linux-kernel - Re: [PATCH v4] i2c: npcm: Add clock toggle recovery

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 for Android: free password hash cracker in your pocket

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

Message-ID: <npt666jrmw4qhesdwup4khufkxczy3zkf4sd4ygjx6dl6joas2@pb2jumja7cgb>
Date: Thu, 17 Apr 2025 18:11:34 +0200
From: Andi Shyti <andi.shyti@...nel.org>
To: Mohammed Elbadry <mohammed.0.elbadry@...il.com>
Cc: Tali Perry <tali.perry1@...il.com>, 
	Avi Fishman <avifishman70@...il.com>, Tomer Maimon <tmaimon77@...il.com>, 
	Patrick Venture <venture@...gle.com>, Nancy Yuen <yuenn@...gle.com>, 
	Benjamin Fair <benjaminfair@...gle.com>, openbmc@...ts.ozlabs.org, linux-i2c@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] i2c: npcm: Add clock toggle recovery

Hi Mohammed,

...

> > > +                              npcm_i2c_get_SDA(&bus->adap), npcm_i2c_get_SCL(&bus->adap));
> > > +             if (npcm_i2c_recovery_tgclk(&bus->adap)) {
> > > +                     dev_err(bus->dev, "I2C%d init fail: SDA=%d SCL=%d\n",
> > > +                             bus->num, npcm_i2c_get_SDA(&bus->adap),
> > > +                             npcm_i2c_get_SCL(&bus->adap));
> > > +                     return -ENXIO;
> >
> > why don't we return the error coming from
> > npcm_i2c_recovery_tgclk() instead of forcing it to ENXIO?
> 
> The error that comes from the current driver is only -ENOTRECOVERABLE.
> I do not see why we cannot return that error instead. I understand the
> difference between both states; however, from the driver's
> perspective, on initialization, ENXIO and ENOTRECOVERABLE behavior is
> identical in that state.

I think it makes more sense to return the error we receive from
npcm_i2c_recovery_tgclk(). Then we can discuss whether that error
is correct or not, but that's a different topic.

> ENXIO was what the driver returned by default without trying to
> recover. Do you recommend changing it? Let me know. I am happy to send
> the v5 patch with the change and address the above comments, too.

Yes, please do.

Andi