On 12/05/2021 12:24:26-0400, Krzysztof Kozlowski wrote: > On 12/05/2021 12:13, Alexandre Belloni wrote: > > On 10/05/2021 08:20:52-0400, Krzysztof Kozlowski wrote: > >> On 09/05/2021 17:06, Alexandre Belloni wrote: > >>> On 08/05/2021 18:06:03-0600, Edmundo Carmona Antoranz wrote: > >>>> On Sat, May 8, 2021 at 10:59 AM Christophe JAILLET > >>>> <christophe.jaillet@xxxxxxxxxx> wrote: > >>>>> > >>>>>> > >>>>>> Following the recent conversations, I think it might make sense to do > >>>>>> dev_err(&pdev->dev, "Failed to register RTC device: %pe\n", info->rtc_dev); > >>>>>> > >>>>>> Is that right? > >>>>>> > >>>>> > >>>>> Yes, it is right, but it should be done in another patch. > >>>>> > >>>>> Would you like to give it a try? > >>>>> > >>>> Sure, I'll have the patch ready to send it when I see yours on next. > >>> > >>> Does it make sense to print anything at all? Who would use the output? > >>> Is anyone actually going to read it? > >> > >> If the RTC core does not print the message, it should be > >> dev_err_probe(). However the first is recently preferred - RTC core > >> should do it for all drivers. I find such error messages useful - helps > >> easily spotting regressions via dmesg -l err. > >> > > > > The only error path that will not print a message by default (it is > > dev_dbg) is when rtc-ops is NULL which I don't expect would regress > > anyway. > > Then the message in the driver is useless and could be removed. > > > A better way to remove the dead code would be to switch to > > devm_rtc_allocate_device/devm_rtc_register_device. And even better would > > be to take that opportunity to set range_min and range_max ;) > > > > The driver already uses devm_rtc_device_register() so I think I don't > follow that part. devm_rtc_device_register is different from devm_rtc_register_device. > > Best regards, > Krzysztof -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
