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

Reply via email to