Messages in this thread | | | From | Yoshihiro Shimoda <> | Subject | RE: [PATCH v2] phy: renesas: rcar-gen3-usb2: fix SError happen if DEBUG_SHIRQ is enabled | Date | Fri, 17 Jul 2020 08:16:47 +0000 |
| |
Hello Vinod,
> From: Vinod Koul, Sent: Friday, July 17, 2020 3:39 PM > > hello Yoshihiro, > > On 13-07-20, 21:11, Yoshihiro Shimoda wrote: > > Please consider revising patch subject. It tell me you are fixing an > error but it doesnt tell me what this patch is about :) > > Perhpas :move irq registration to init" maybe a better title which > describes the changes this patch brings in
Thank you for your suggestion! I also think your suggestion is better. So, I will fix it.
<snip> > > @@ -389,12 +390,39 @@ static void rcar_gen3_init_otg(struct rcar_gen3_chan *ch) > > rcar_gen3_device_recognition(ch); > > } > > > > +static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, void *_ch) > > +{ > > + struct rcar_gen3_chan *ch = _ch; > > + void __iomem *usb2_base = ch->base; > > + u32 status = readl(usb2_base + USB2_OBINTSTA); > > + irqreturn_t ret = IRQ_NONE; > > + > > + if (status & USB2_OBINT_BITS) { > > + dev_vdbg(ch->dev, "%s: %08x\n", __func__, status); > > + writel(USB2_OBINT_BITS, usb2_base + USB2_OBINTSTA); > > + rcar_gen3_device_recognition(ch); > > + ret = IRQ_HANDLED; > > + } > > + > > + return ret; > > +} > > + > > static int rcar_gen3_phy_usb2_init(struct phy *p) > > { > > struct rcar_gen3_phy *rphy = phy_get_drvdata(p); > > struct rcar_gen3_chan *channel = rphy->ch; > > void __iomem *usb2_base = channel->base; > > u32 val; > > + int ret; > > + > > + if (!rcar_gen3_is_any_rphy_initialized(channel) && channel->irq >= 0) { > > + INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work); > > + ret = request_irq(channel->irq, rcar_gen3_phy_usb2_irq, > > + IRQF_SHARED, dev_name(channel->dev), channel); > > + if (ret < 0) > > + dev_err(channel->dev, "No irq handler (%d)\n", > > + channel->irq); > > This could be in a single line :)
Yes. We could be 80 over characters in a line now :) I'll fix it.
> Should we continue on error here?
Hmm, maybe it's better if the request_irq() failed because it can avoid unexpected behaviors. But, original code continued on error. In this case, should I make a separated incremental patch to exit on error?
Best regards, Yoshihiro Shimoda
| |