Re: [PATCH 0/9] iio: Remove duplicated error reporting in .remove()

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

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

Hello Jonathan,

On Sat, May 14, 2022 at 02:31:51PM +0100, Jonathan Cameron wrote:
> On Fri, 13 May 2022 09:24:24 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > On Sat, May 07, 2022 at 03:38:55PM +0100, Jonathan Cameron wrote:
> > > On Sun, 1 May 2022 18:51:23 +0100
> > > Jonathan Cameron <jic23@kernel.org> wrote:
> > >   
> > > > On Sun, 1 May 2022 18:41:49 +0100
> > > > Jonathan Cameron <jic23@kernel.org> wrote:
> > > >   
> > > > > On Sat, 30 Apr 2022 10:15:58 +0200
> > > > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > > > >     
> > > > > > Hello,
> > > > > > 
> > > > > > this series adapts several i2c drivers that emit two error messages if
> > > > > > something in their remove function fails. The relevant issue is that the
> > > > > > i2c core emits an error message if the remove callback returns a
> > > > > > non-zero value but the drivers already emit a (better) message. So
> > > > > > these patches change the drivers to return 0 even after an error. Note
> > > > > > there is no further error handling in the i2c core, if a remove callback
> > > > > > returns an error code, the device is removed anyhow, so the only effect
> > > > > > of making the return value zero is that the error message is suppressed.
> > > > > > 
> > > > > > The motivation for this series is to eventually change the prototype of
> > > > > > the i2c remove callback to return void. As a preparation all remove
> > > > > > functions should return 0 such that changing the prototype doesn't
> > > > > > change behaviour of individual drivers.      
> > > > > 
> > > > > I think I'd rather have seen these called out as simply moving towards
> > > > > this second change as it feels wrong to deliberately not report an error
> > > > > so as to avoid repeated error messages!
> > > > > 
> > > > > Meh, I don't care that strongly and you call out the real reason in each
> > > > > patch.    
> > > > 
> > > > Series looks fine to me, but I'll leave the on list for a few days to let
> > > > others have time to take a look.
> > > > 
> > > > Worth noting that some of these are crying out for use
> > > > of devm_add_action_or_reset() and getting rid of the remove functions
> > > > entirely now you've dropped the oddity of them returning non 0.
> > > > 
> > > > Low hanging fruit for any newbies who want to do it, or maybe I will
> > > > if I get bored :)  
> > > 
> > > Series applied to the togreg branch of iio.git and pushed out as testing for
> > > 0-day to see if it can find anything we missed.  
> > 
> > They are in testing
> > (https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=testing)
> > but not in togreg
> > (https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=togreg).
> > 
> > Not sure if that is how it's supposed to be? Only togreg seems to be in
> > next.
> Yup. That's intentional because I don't rebase the togreg branch unless
> something goes wrong when it hits next.  The intent being it's a stable
> base to build upon.  Normally there is a delay of up to a week to let
> 0-day take a look at testing and for me to happen to get time sat at
> the right computer, but sometimes it's longer :(
> 
> Right now I'm waiting on a pull request being picked up by Greg KH,
> after which I'll fast forward the togreg branch as I have some patches
> waiting to be queued up that are dependent on things that have reached
> char-misc-next via other paths.

Not sure I understood that correctly. Do you let Greg pull the togreg
branch? If you instead let him pull tags, you don't have to wait in such
a situation to apply new patches to a for-next branch. (Or just don't
use "togreg" for both, sending PRs to Greg and put patches into next.)

> Unfortunately I'm doubtful about whether I can squeeze in a second
> pull request this cycle, so they may get queued up for next cycle.
> A bit of bad timing :(

It's not a big problem for me. There is still much to do (also a bit in
drivers/iio) before I tackle the final bits of my quest and actually
change struct i2c_driver (and so depend on these patches having hit
mainline). The only downside for you is, that you might have to endure
me asking again for the state of these patches ;-)

Thanks for your feedback. Compared to pinging repeatedly and getting no
maintainer reply, knowing about your problems is much appreciated.

Best regards and have a nice week-end,
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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