On 09.05.2023 17:18:06, Judith Mendez wrote:
[...]
> >> + if (!mcan_class->polling && irq < 0) {
> >> + ret = -ENXIO;
> >> + dev_err_probe(mcan_class->dev, ret, "IRQ int0 not found and polling not activated\n");
> >> + goto probe_fail;
> >> + }
> >> +
> >> + if (mcan_class->polling) {
> >> + if (irq > 0) {
> >> + mcan_class->polling = 0;
> >
> > false
> >
> >> + dev_dbg(mcan_class->dev, "Polling enabled and hardware IRQ found, use hardware IRQ\n");
> >
> > "...using hardware IRQ"
> >
> > Use dev_info(), as there is something not 100% correct with the DT.
>
> Is it dev_info or dev_dbg?
dev_info() - But without an explicit "poll-interval' in the DT, this
code path doesn't exist anymore.
> I used to have dev_info since it was nice to see when polling was
> enabled.
Re-read your code, this is not about enabling polling. This message
handles the case where an IRQ was given _and_ "poll-interval" was
specified. So there is something not 100% correct with the DT (IRQ _and_
polling), but this is obsolete now.
> Also, I had seen this print and the next as informative prints, hence the dev_info().
We don't print messages when IRQs are enabled, so enabling polling
should be a dev_dbg(), too.
> However, I was told in this review process to change to dev_dbg. Which is correct?
Driver works correct -> dev_dbg()
Something is strange -> dev_info()
Hope that helps,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Attachment:
signature.asc
Description: PGP signature
