Re: [PATCH v6] i2c: imx: support DMA defer probing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> > > @@ -1802,6 +1803,18 @@ static int i2c_imx_probe(struct platform_device
> > *pdev)
> > >       if (ret == -EPROBE_DEFER)
> > >               goto clk_notifier_unregister;
> > >
> > > +     /* As we can always fall back to PIO, let's ignore the error setting up
> > DMA. */
> > > +     ret = i2c_imx_dma_request(i2c_imx, phy_addr);
> > > +     if (ret) {
> > > +             if (ret == -EPROBE_DEFER)
> > > +                     goto clk_notifier_unregister;
> > > +             else if (ret == -ENODEV)
> > > +                     dev_dbg(&pdev->dev, "Only use PIO mode\n");
> > > +             else
> > > +                     dev_err(&pdev->dev, "Failed to setup DMA (%pe),
> > only use PIO mode\n",
> > > +                             ERR_PTR(ret));
> > 
> > My question here is not just about the use of dev_err vs dev_err_probe, but why
> > don't we exit the probe if we get an error.
> > 
> > We should use PIO only in case of ENODEV, in all the other cases I think we
> > should just leave. E.g. why don't we exit if we meet ret == -ENOMEM?
> 
> Hi, Andi
> 
> Thank you! From my point, I2C is critical bus so it should be available as much as possible.
> -ENOMEM or other unknown errors all are from i2c_imx_dma_request(). So error happened in enable DMA mode process.

OK, makes sense, it's the idea of "let things fail on their own,
I'll move forward as much as I can"; we need to be aware of
the choice. Please add a comment above.

But then it's not an error, but a warning. With errors we bail
out, with warnings we tell users that something went wrong.

Sorry for keeping you on this point for so long, but do you mind
swapping this dev_err in dev_warn, with a comment explaining the
reason we decided not to leave?

Thanks,
Andi




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux