[PATCH 0/4] siox: Move some complexity into the core

Uwe Kleine-König posted 4 patches 1 year, 11 months ago
drivers/siox/siox-bus-gpio.c | 62 ++++++++++++------------------------
drivers/siox/siox-core.c     | 45 ++++++++++++++++++++++++++
drivers/siox/siox.h          |  4 +++
3 files changed, 69 insertions(+), 42 deletions(-)
[PATCH 0/4] siox: Move some complexity into the core
Posted by Uwe Kleine-König 1 year, 11 months ago
Hello,

the reference handling in siox is a bit strange. With
siox_master_alloc() to get a reference on the master that is passed to
the core calling siox_master_register(). So until siox_master_register()
is called successfully the driver has to call siox_master_put() in the
error path, but on remove siox_master_unregister cares for that. While
that technically works, it's unusual and surprising to use. This serie's
first patch cleans that up and then introduces devm functions to make it
even easier to use.

A nice (and intended) side effect is that the gpio bus driver gets rid
of it's remove callback, so I don't have to adapt it for my quest that
changes the prototype of .remove().

Best regards
Uwe

Uwe Kleine-König (4):
  siox: Don't pass the reference on a master in siox_master_register()
  siox: Provide a devm variant of siox_master_alloc()
  siox: Provide a devm variant of siox_master_register()
  siox: bus-gpio: Simplify using devm_siox_* functions

 drivers/siox/siox-bus-gpio.c | 62 ++++++++++++------------------------
 drivers/siox/siox-core.c     | 45 ++++++++++++++++++++++++++
 drivers/siox/siox.h          |  4 +++
 3 files changed, 69 insertions(+), 42 deletions(-)


base-commit: d37e1e4c52bc60578969f391fb81f947c3e83118
-- 
2.43.0

Re: [PATCH 0/4] siox: Move some complexity into the core
Posted by Thorsten Scherer 1 year, 11 months ago
Hello Uwe, Hello Greg,

On Mon, Feb 19, 2024 at 08:46:28AM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> the reference handling in siox is a bit strange. With
> siox_master_alloc() to get a reference on the master that is passed to
> the core calling siox_master_register(). So until siox_master_register()
> is called successfully the driver has to call siox_master_put() in the
> error path, but on remove siox_master_unregister cares for that. While
> that technically works, it's unusual and surprising to use. This serie's
> first patch cleans that up and then introduces devm functions to make it
> even easier to use.
> 
> A nice (and intended) side effect is that the gpio bus driver gets rid
> of it's remove callback, so I don't have to adapt it for my quest that
> changes the prototype of .remove().

I only compile tested this as I currently do not have access to test
hardware.

The series looks sensible.

Acked-by: Thorsten Scherer <t.scherer@eckelmann.de>

@gregkh: Would you please pick up Uwe's series as well?

Thank you both.

Best regards
Thorsten

> Best regards
> Uwe
> 
> Uwe Kleine-König (4):
>   siox: Don't pass the reference on a master in siox_master_register()
>   siox: Provide a devm variant of siox_master_alloc()
>   siox: Provide a devm variant of siox_master_register()
>   siox: bus-gpio: Simplify using devm_siox_* functions
> 
>  drivers/siox/siox-bus-gpio.c | 62 ++++++++++++------------------------
>  drivers/siox/siox-core.c     | 45 ++++++++++++++++++++++++++
>  drivers/siox/siox.h          |  4 +++
>  3 files changed, 69 insertions(+), 42 deletions(-)
> 
> 
> base-commit: d37e1e4c52bc60578969f391fb81f947c3e83118
> -- 
> 2.43.0
> 
siox patches for next development cycle [Re: [PATCH 0/4] siox: Move some complexity into the core]
Posted by Uwe Kleine-König 1 year, 11 months ago
Hello,

On Tue, Feb 27, 2024 at 11:21:24AM +0100, Thorsten Scherer wrote:
> On Mon, Feb 19, 2024 at 08:46:28AM +0100, Uwe Kleine-König wrote:
> The series looks sensible.
> 
> Acked-by: Thorsten Scherer <t.scherer@eckelmann.de>
> 
> @gregkh: Would you please pick up Uwe's series as well?

There are currently six patches for drivers/siox waiting to be applied.
(Two by Ricardo and four by me.) Thorsten asked Greg to do so. Greg
didn't pick these up yet though. So I collected them and put them to a
branch at:

	https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git siox/for-next

I'd like to get them in during the next development cycle.

Greg, what is easiest for you? Are they still on your list of open
patches and we (I) need just a bit more patience? Or should I send a PR
to Linus when the merge window opens?

Stephen: It would be nice to get this above branch into next, no matter
if the patches make it in via Greg or me. Could you please add this
branch to your list for next? If Greg will apply them, I'll empty this
branch to not get duplicates.

Thanks to all involved people,
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
Re: siox patches for next development cycle [Re: [PATCH 0/4] siox: Move some complexity into the core]
Posted by Greg Kroah-Hartman 1 year, 11 months ago
On Wed, Mar 06, 2024 at 07:24:38PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> On Tue, Feb 27, 2024 at 11:21:24AM +0100, Thorsten Scherer wrote:
> > On Mon, Feb 19, 2024 at 08:46:28AM +0100, Uwe Kleine-König wrote:
> > The series looks sensible.
> > 
> > Acked-by: Thorsten Scherer <t.scherer@eckelmann.de>
> > 
> > @gregkh: Would you please pick up Uwe's series as well?
> 
> There are currently six patches for drivers/siox waiting to be applied.
> (Two by Ricardo and four by me.) Thorsten asked Greg to do so. Greg
> didn't pick these up yet though. So I collected them and put them to a
> branch at:
> 
> 	https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git siox/for-next
> 
> I'd like to get them in during the next development cycle.
> 
> Greg, what is easiest for you? Are they still on your list of open
> patches and we (I) need just a bit more patience? Or should I send a PR
> to Linus when the merge window opens?

Yes, they are on my list, but I am way behind, sorry.  But hey, a pull
request is faster, I'll go take this now, thanks!

Oops, nope, I get the following build error with this tree:
	ERROR: modpost: "devm_siox_master_alloc" [drivers/siox/siox-bus-gpio.ko] undefined!

So are you sure you tested this?

thanks,

greg k-h
Re: siox patches for next development cycle [Re: [PATCH 0/4] siox: Move some complexity into the core]
Posted by Uwe Kleine-König 1 year, 11 months ago
Hey Greg,

On Wed, Mar 06, 2024 at 10:46:43PM +0000, Greg Kroah-Hartman wrote:
> On Wed, Mar 06, 2024 at 07:24:38PM +0100, Uwe Kleine-König wrote:
> > On Tue, Feb 27, 2024 at 11:21:24AM +0100, Thorsten Scherer wrote:
> > > On Mon, Feb 19, 2024 at 08:46:28AM +0100, Uwe Kleine-König wrote:
> > > The series looks sensible.
> > > 
> > > Acked-by: Thorsten Scherer <t.scherer@eckelmann.de>
> > > 
> > > @gregkh: Would you please pick up Uwe's series as well?
> > 
> > There are currently six patches for drivers/siox waiting to be applied.
> > (Two by Ricardo and four by me.) Thorsten asked Greg to do so. Greg
> > didn't pick these up yet though. So I collected them and put them to a
> > branch at:
> > 
> > 	https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git siox/for-next
> > 
> > I'd like to get them in during the next development cycle.
> > 
> > Greg, what is easiest for you? Are they still on your list of open
> > patches and we (I) need just a bit more patience? Or should I send a PR
> > to Linus when the merge window opens?
> 
> Yes, they are on my list, but I am way behind, sorry.  But hey, a pull
> request is faster, I'll go take this now, thanks!
> 
> Oops, nope, I get the following build error with this tree:
> 	ERROR: modpost: "devm_siox_master_alloc" [drivers/siox/siox-bus-gpio.ko] undefined!
> 
> So are you sure you tested this?

Dang, Stephen notices that, too. TIL if I only do:

	make allmodconfig drivers/siox/

a missing EXPORT_SYMBOL doesn't make the build fail.

I squashed 

diff --git a/drivers/siox/siox-core.c b/drivers/siox/siox-core.c
index ae103349c91a..24a45920a240 100644
--- a/drivers/siox/siox-core.c
+++ b/drivers/siox/siox-core.c
@@ -730,6 +730,7 @@ struct siox_master *devm_siox_master_alloc(struct device *dev,
 
 	return smaster;
 }
+EXPORT_SYMBOL_GPL(devm_siox_master_alloc);
 
 int siox_master_register(struct siox_master *smaster)
 {

into the offending commit in my above branch and did some more extensive
build testing.

Sorry
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
Re: siox patches for next development cycle [Re: [PATCH 0/4] siox: Move some complexity into the core]
Posted by Stephen Rothwell 1 year, 11 months ago
Hi Uwe,

On Wed, 6 Mar 2024 19:24:38 +0100 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
>
> On Tue, Feb 27, 2024 at 11:21:24AM +0100, Thorsten Scherer wrote:
> > On Mon, Feb 19, 2024 at 08:46:28AM +0100, Uwe Kleine-König wrote:
> > The series looks sensible.
> > 
> > Acked-by: Thorsten Scherer <t.scherer@eckelmann.de>
> > 
> > @gregkh: Would you please pick up Uwe's series as well?  
> 
> There are currently six patches for drivers/siox waiting to be applied.
> (Two by Ricardo and four by me.) Thorsten asked Greg to do so. Greg
> didn't pick these up yet though. So I collected them and put them to a
> branch at:
> 
> 	https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git siox/for-next
> 
> I'd like to get them in during the next development cycle.
> 
> Greg, what is easiest for you? Are they still on your list of open
> patches and we (I) need just a bit more patience? Or should I send a PR
> to Linus when the merge window opens?
> 
> Stephen: It would be nice to get this above branch into next, no matter
> if the patches make it in via Greg or me. Could you please add this
> branch to your list for next? If Greg will apply them, I'll empty this
> branch to not get duplicates.

Added from today.

Thanks for adding your subsystem tree as a participant of linux-next.  As
you may know, this is not a judgement of your code.  The purpose of
linux-next is for integration testing and to lower the impact of
conflicts between subsystems in the next merge window. 

You will need to ensure that the patches/commits in your tree/series have
been:
     * submitted under GPL v2 (or later) and include the Contributor's
        Signed-off-by,
     * posted to the relevant mailing list,
     * reviewed by you (or another maintainer of your subsystem tree),
     * successfully unit tested, and 
     * destined for the current or next Linux merge window.

Basically, this should be just what you would send to Linus (or ask him
to fetch).  It is allowed to be rebased if you deem it necessary.

-- 
Cheers,
Stephen Rothwell 
sfr@canb.auug.org.au

-- 
Cheers,
Stephen Rothwell