On Thu, 30 Jun 2022 16:15:05 +0200 Uwe Kleine-König <ukleinek@xxxxxxxxxx> wrote: > On 6/30/22 12:26, Andy Shevchenko wrote: > > On Thu, Jun 30, 2022 at 12:25 PM Uwe Kleine-König <ukleinek@xxxxxxxxxx> wrote: > >> On 6/30/22 09:17, Andy Shevchenko wrote: > >>> On Thu, Jun 30, 2022 at 8:58 AM Uwe Kleine-König <ukleinek@xxxxxxxxxx> wrote: > >>>> > >>>> Don't emit a message for -ENOMEM, the kernel is already loud enough in > >>>> this case. Add a message if getting the GPIO or registering the iio > >>>> device fails and use dev_err_probe for improved behaviour on > >>>> -EPROBE_DEFER. > >>> > >>> Why do we need additional messages? > >> > >> I don't understand the question. Do you really wonder why there is a > >> benefit of an error message if a resource allocation in probe fails? > >> > >> Current state is that for Yves-Alexis (on Cc:) the driver is silent but > >> unbound. I guess that's because gpiod_get fails, but seeing a > >> confirmation in the kernel log would be nice. Giving that example in the patch description makes for a very strong reasoning. Mostly I want this captured so that others who might consider similar patches and see this, understand the reasoning. Messages for things that can happen in real life are definitely a good thing! > >> > >>> They should be split in a separate patch, perhaps, with the explanation. > >>> > >>> Actually the rest I would split to two: converting to dev_err_probe() > >>> in the case where it's not right now, and dropping ENOMEM message. > >> > >> So we're at three patches: > >> > >> - drop ENOMEM message > >> - convert existing messages to dev_err_probe() > >> - introduce errors for devm_gpiod_get() and devm_iio_device_register() That lines up with how I'd prefer it split. If I'd not wanted the patch description additions above anyway I 'might' have just picked it up. Given we are going around again, let's set a nice example. In particularly it will make the description easier to read as separates -ENOMEM case from the new additions. Jonathan > >> > >> I can rework accordingly, but for me this looks a bit over engineered. > > > > Perhaps, but they three are about different stuff. > > The different things could be named as follows: > > - Improve error reporting in .probe() for devm_iio_device_alloc() > - Improve error reporting in .probe() for devm_gpiod_get() > - Improve error reporting in .probe() for gpiod_to_irq() > - Improve error reporting in .probe() for devm_iio_device_register() > > Even after the discussion I think it's ok to summarize that to a single > patch "Improve error reporting in .probe()". I'd wait for Jonathan's > feedback with his opinion before reworking the patch. > > Best regards > Uwe
