Re: [PATCH v7 1/1] i2c: lpi2c: use clk notifier for rate changes

From: Wolfram Sang <wsa@kernel.org>
To: Alexander Stein <alexander.stein@ew.tq-group.com>
Cc: Andi Shyti <andi.shyti@kernel.org>,
	Dong Aisheng <aisheng.dong@nxp.com>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	Alexander Sverdlin <alexander.sverdlin@siemens.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	NXP Linux Team <linux-imx@nxp.com>,
	linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v7 1/1] i2c: lpi2c: use clk notifier for rate changes
Date: Tue, 19 Dec 2023 17:56:15 +0100	[thread overview]
Message-ID: <ZYHLL8fiLKJNEJ+8@shikoro> (raw)
In-Reply-To: <2912069.e9J7NaK4W3@steina-w>

[-- Attachment #1: Type: text/plain, Size: 915 bytes --]

Hi,

> > ... improve the commit message, reporting the real deadlock case
> > instead of a lockdep warning and...
> 
> I've improved the commit message about an actual deadlock.

That means a v8 is in your queue?

> > ... find a more appropriate error number, I will ack this patch.
> 
> Thinking about this again, I think EINVAL is an appropriate error code.
> The parent clock frequency is also an input for the i2c transfer. So if, for 
> whatever reason, that clock frequency is 0, it is an invalid value (argument).
> I've checked other drivers what they do if that clock is 0. Unfortunately most 
> don't consider this case at all. But some do, so e.g. i2c_lpc2k_probe() or 
> dc_i2c_init_hw() both return EINVAL if the clk or a calculated divider is 0.

IMHO, the return value doesn't matter that much as we have a descriptive
text accompanying it anyhow.

Happy hacking,

   Wolfram

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]