linux-kernel - Re: [PATCH v4 2/4] can: m_can: Add hrtimer to generate software interrupt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 10 May 2023 09:21:16 +0200
From:   Marc Kleine-Budde <mkl@...gutronix.de>
To:     Judith Mendez <jm@...com>
Cc:     Chandrasekar Ramakrishnan <rcsekar@...sung.com>,
        Wolfgang Grandegger <wg@...ndegger.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, linux-can@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Schuyler Patton <spatton@...com>, Nishanth Menon <nm@...com>,
        Vignesh Raghavendra <vigneshr@...com>,
        Tero Kristo <kristo@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
        Oliver Hartkopp <socketcan@...tkopp.net>,
        Simon Horman <simon.horman@...igine.com>
Subject: Re: [PATCH v4 2/4] can: m_can: Add hrtimer to generate software
 interrupt

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   |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)