On 17-07-20, 08:16, Yoshihiro Shimoda wrote: > 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?
Yes that would be better :), Always, a patch per change -- ~Vinod