All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] Introduce builtin_platform_driver for non modules
@ 2015-07-01  1:19 Paul Gortmaker
  2015-07-01  1:24 ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Gortmaker @ 2015-07-01  1:19 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, gregkh

We see an increasing number of non-modular drivers using
modular_driver() type register functions.  There are several
downsides to letting this continue unchecked:

1) The code can appear modular to a reader of the code, and they
   won't know if the code really is modular without checking the
   Makefile and Kconfig to see if compilation is governed by a
   bool or tristate.
2) Coders of drivers may be tempted to code up an __exit function
   that is never used, just in order to satisfy the required three
   args of the modular registration function.
3) Non-modular code ends up including the <module.h> which increases
   CPP overhead that they don't need.
4) It hinders us from performing better separation of the module
   init code and the generic init code.

So here we introduce similar macros for builtin drivers.  Then we 
convert builtin drivers (controlled by a bool Kconfig) by making the
following type of mapping:

  module_platform_driver()       --->  builtin_platform_driver()
  module_platform_driver_probe() --->  builtin_platform_driver_probe().

The set of drivers that are converted here are just the ones that
showed up as relying on an implicit include of <module.h> during
a pending header cleanup.  So we convert them here vs. adding
an include of <module.h> to non-modular code to avoid compile fails.
Additonal conversions can be done asynchronously at any time.

Once again, an unused module_exit function that is removed here appears
in the diffstat as an outlier wrt. all the other changes.

Original posting:
   "Introduce builtin_driver and use it for non-modular code"
   https://lkml.kernel.org/r/1431287385-1526-1-git-send-email-paul.gortmaker@windriver.com

Thanks,
Paul.
---

The following changes since commit 0f57d86787d8b1076ea8f9cbdddda2a46d534a27:

  Linux 4.1-rc8 (2015-06-14 15:51:10 -1000)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux.git tags/module-builtin_driver-v4.1-rc8

for you to fetch changes up to 77459a0feca4ae8757a905fd1791f039479e8e1e:

  drivers/clk: convert sunxi/clk-mod0.c to use builtin_platform_driver (2015-06-16 14:12:39 -0400)

----------------------------------------------------------------
Replace module_platform_driver with builtin_platform driver in non modules.

----------------------------------------------------------------
Paul Gortmaker (8):
      platform_device: better support builtin boilerplate avoidance
      drivers/platform: Convert non-modular pdev_bus to use builtin_platform_driver
      drivers/cpuidle: Convert non-modular drivers to use builtin_platform_driver
      drivers/cpufreq: Convert non-modular s5pv210-cpufreq.c to use builtin_platform_driver
      drivers/soc: Convert non-modular tegra/pmc to use builtin_platform_driver
      drivers/soc: Convert non-modular soc-realview to use builtin_platform_driver
      drivers/power: Convert non-modular syscon-reboot to use builtin_platform_driver
      drivers/clk: convert sunxi/clk-mod0.c to use builtin_platform_driver

 drivers/clk/sunxi/clk-mod0.c         |  2 +-
 drivers/cpufreq/s5pv210-cpufreq.c    |  2 +-
 drivers/cpuidle/cpuidle-at91.c       |  3 +--
 drivers/cpuidle/cpuidle-calxeda.c    |  3 +--
 drivers/cpuidle/cpuidle-zynq.c       |  3 +--
 drivers/platform/goldfish/pdev_bus.c | 12 +-----------
 drivers/power/reset/syscon-reboot.c  |  2 +-
 drivers/soc/tegra/pmc.c              |  2 +-
 drivers/soc/versatile/soc-realview.c |  2 +-
 include/linux/device.h               | 22 ++++++++++++++++++++++
 include/linux/platform_device.h      | 23 +++++++++++++++++++++++
 11 files changed, 54 insertions(+), 22 deletions(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [GIT PULL] Introduce builtin_platform_driver for non modules
  2015-07-01  1:19 [GIT PULL] Introduce builtin_platform_driver for non modules Paul Gortmaker
@ 2015-07-01  1:24 ` Greg KH
  2015-07-01  6:53   ` Stephen Rothwell
  2015-07-01 15:33   ` Paul Gortmaker
  0 siblings, 2 replies; 6+ messages in thread
From: Greg KH @ 2015-07-01  1:24 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: torvalds, linux-kernel

On Tue, Jun 30, 2015 at 09:19:09PM -0400, Paul Gortmaker wrote:
> We see an increasing number of non-modular drivers using
> modular_driver() type register functions.  There are several
> downsides to letting this continue unchecked:
> 
> 1) The code can appear modular to a reader of the code, and they
>    won't know if the code really is modular without checking the
>    Makefile and Kconfig to see if compilation is governed by a
>    bool or tristate.
> 2) Coders of drivers may be tempted to code up an __exit function
>    that is never used, just in order to satisfy the required three
>    args of the modular registration function.
> 3) Non-modular code ends up including the <module.h> which increases
>    CPP overhead that they don't need.
> 4) It hinders us from performing better separation of the module
>    init code and the generic init code.
> 
> So here we introduce similar macros for builtin drivers.  Then we 
> convert builtin drivers (controlled by a bool Kconfig) by making the
> following type of mapping:
> 
>   module_platform_driver()       --->  builtin_platform_driver()
>   module_platform_driver_probe() --->  builtin_platform_driver_probe().
> 
> The set of drivers that are converted here are just the ones that
> showed up as relying on an implicit include of <module.h> during
> a pending header cleanup.  So we convert them here vs. adding
> an include of <module.h> to non-modular code to avoid compile fails.
> Additonal conversions can be done asynchronously at any time.
> 
> Once again, an unused module_exit function that is removed here appears
> in the diffstat as an outlier wrt. all the other changes.
> 
> Original posting:
>    "Introduce builtin_driver and use it for non-modular code"
>    https://lkml.kernel.org/r/1431287385-1526-1-git-send-email-paul.gortmaker@windriver.com
> 
> Thanks,
> Paul.
> ---
> 
> The following changes since commit 0f57d86787d8b1076ea8f9cbdddda2a46d534a27:
> 
>   Linux 4.1-rc8 (2015-06-14 15:51:10 -1000)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux.git tags/module-builtin_driver-v4.1-rc8
> 
> for you to fetch changes up to 77459a0feca4ae8757a905fd1791f039479e8e1e:
> 
>   drivers/clk: convert sunxi/clk-mod0.c to use builtin_platform_driver (2015-06-16 14:12:39 -0400)

Was this ever in linux-next?  I saw you post this once, don't recall any
real discussion about it.  Ideally some subsystem people would ack it...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [GIT PULL] Introduce builtin_platform_driver for non modules
  2015-07-01  1:24 ` Greg KH
@ 2015-07-01  6:53   ` Stephen Rothwell
  2015-07-01 15:29     ` Greg KH
  2015-07-01 15:33   ` Paul Gortmaker
  1 sibling, 1 reply; 6+ messages in thread
From: Stephen Rothwell @ 2015-07-01  6:53 UTC (permalink / raw)
  To: Greg KH; +Cc: Paul Gortmaker, torvalds, linux-kernel

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

Hi Greg,

On Tue, 30 Jun 2015 18:24:58 -0700 Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > The following changes since commit 0f57d86787d8b1076ea8f9cbdddda2a46d534a27:
> > 
> >   Linux 4.1-rc8 (2015-06-14 15:51:10 -1000)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux.git tags/module-builtin_driver-v4.1-rc8
> > 
> > for you to fetch changes up to 77459a0feca4ae8757a905fd1791f039479e8e1e:
> > 
> >   drivers/clk: convert sunxi/clk-mod0.c to use builtin_platform_driver (2015-06-16 14:12:39 -0400)
> 
> Was this ever in linux-next?

Yes, since next-20150617.

>  Ideally some subsystem people would ack it...

Most of the patches have at least one Ack
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [GIT PULL] Introduce builtin_platform_driver for non modules
  2015-07-01  6:53   ` Stephen Rothwell
@ 2015-07-01 15:29     ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2015-07-01 15:29 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: Paul Gortmaker, torvalds, linux-kernel

On Wed, Jul 01, 2015 at 04:53:55PM +1000, Stephen Rothwell wrote:
> Hi Greg,
> 
> On Tue, 30 Jun 2015 18:24:58 -0700 Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > > The following changes since commit 0f57d86787d8b1076ea8f9cbdddda2a46d534a27:
> > > 
> > >   Linux 4.1-rc8 (2015-06-14 15:51:10 -1000)
> > > 
> > > are available in the git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux.git tags/module-builtin_driver-v4.1-rc8
> > > 
> > > for you to fetch changes up to 77459a0feca4ae8757a905fd1791f039479e8e1e:
> > > 
> > >   drivers/clk: convert sunxi/clk-mod0.c to use builtin_platform_driver (2015-06-16 14:12:39 -0400)
> > 
> > Was this ever in linux-next?
> 
> Yes, since next-20150617.

Ah good, missed that, sorry.

> >  Ideally some subsystem people would ack it...
> 
> Most of the patches have at least one Ack

Ok, I was looking at the one that touched the driver core :)

Anyway, this is fine with me, sorry for the noise.

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [GIT PULL] Introduce builtin_platform_driver for non modules
  2015-07-01  1:24 ` Greg KH
  2015-07-01  6:53   ` Stephen Rothwell
@ 2015-07-01 15:33   ` Paul Gortmaker
  2015-07-01 15:39     ` Greg KH
  1 sibling, 1 reply; 6+ messages in thread
From: Paul Gortmaker @ 2015-07-01 15:33 UTC (permalink / raw)
  To: Greg KH; +Cc: torvalds, linux-kernel, sfr

[Re: [GIT PULL] Introduce builtin_platform_driver for non modules] On 30/06/2015 (Tue 18:24) Greg KH wrote:

[...]

> > 
> > The following changes since commit 0f57d86787d8b1076ea8f9cbdddda2a46d534a27:
> > 
> >   Linux 4.1-rc8 (2015-06-14 15:51:10 -1000)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux.git tags/module-builtin_driver-v4.1-rc8
> > 
> > for you to fetch changes up to 77459a0feca4ae8757a905fd1791f039479e8e1e:
> > 
> >   drivers/clk: convert sunxi/clk-mod0.c to use builtin_platform_driver (2015-06-16 14:12:39 -0400)
> 
> Was this ever in linux-next?

It was added to linux-next a month ago, and all the commits and their
baseline have been unchanged for the last two weeks (the date Stephen
indicated) as that was when the last Acks etc stopped trickling in.

I've also been proactively monitoring linux-next looking for any merge
issues, which is why I sent you the (now mainline) commits fc368ea1ea00c
and 5a6a7cd05c039 -- in both commits I mentioned how we'd like to change
to use this very infrastructure here, once it is present in tree.

> I saw you post this once, don't recall any real discussion about it. 

I thought the lack of discussion wasn't surprising, given that it was a
mundane and trivial extension of the modular ones to a non-modular use
case, and the ugly alternative is to let everyone open code their own :(

That said, it was posted with a sensible Cc list and it also did get
wider opportunity for possible discussion if needed, thanks to LWN:
	https://lwn.net/Articles/643854/

The only other thread of discussion I can think of was where another
subsystem maintainer looped me into the review of a new driver, because
they were looking forward to having this in tree, due to the additional
clarity it would add between modular and non modular code:
	https://lkml.kernel.org/r/20150620180435.GG16386@windriver.com

> Ideally some subsystem people would ack it...

Yes a good many of the deployment patches themselves are Ack'd.  For the
core macro introduction itself, you were Cc'd on it [and the 0/7 intro].

Given my above mentioned commits that you'd read and merged that
mentioned this, I didn't want to burn karma nagging you for an explicit
ack for this one basic commit itself, given how busy you are with
stable, staging, etc.  But I did explicitly put you on the Cc for this
pull figuring it would be an opportunity to keep you in the loop and
provide a last chance opportunity for a "No, don't do this because..."

If I have to burn karma nagging you about something, I'd rather it be
something more important, like adding this (unrelated) clk_add_alias fix
to staging -- since its absence has been breaking powerpc, s390, parisc,
cris, ... etc. builds in linux-next for quite some time now.  :)

	https://lkml.org/lkml/2015/6/25/365

Thanks,
Paul.
--

> 
> thanks,
> 
> greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [GIT PULL] Introduce builtin_platform_driver for non modules
  2015-07-01 15:33   ` Paul Gortmaker
@ 2015-07-01 15:39     ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2015-07-01 15:39 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: torvalds, linux-kernel, sfr

On Wed, Jul 01, 2015 at 11:33:21AM -0400, Paul Gortmaker wrote:
> [Re: [GIT PULL] Introduce builtin_platform_driver for non modules] On 30/06/2015 (Tue 18:24) Greg KH wrote:
> 
> [...]
> 
> > > 
> > > The following changes since commit 0f57d86787d8b1076ea8f9cbdddda2a46d534a27:
> > > 
> > >   Linux 4.1-rc8 (2015-06-14 15:51:10 -1000)
> > > 
> > > are available in the git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux.git tags/module-builtin_driver-v4.1-rc8
> > > 
> > > for you to fetch changes up to 77459a0feca4ae8757a905fd1791f039479e8e1e:
> > > 
> > >   drivers/clk: convert sunxi/clk-mod0.c to use builtin_platform_driver (2015-06-16 14:12:39 -0400)
> > 
> > Was this ever in linux-next?
> 
> It was added to linux-next a month ago, and all the commits and their
> baseline have been unchanged for the last two weeks (the date Stephen
> indicated) as that was when the last Acks etc stopped trickling in.
> 
> I've also been proactively monitoring linux-next looking for any merge
> issues, which is why I sent you the (now mainline) commits fc368ea1ea00c
> and 5a6a7cd05c039 -- in both commits I mentioned how we'd like to change
> to use this very infrastructure here, once it is present in tree.
> 
> > I saw you post this once, don't recall any real discussion about it. 
> 
> I thought the lack of discussion wasn't surprising, given that it was a
> mundane and trivial extension of the modular ones to a non-modular use
> case, and the ugly alternative is to let everyone open code their own :(
> 
> That said, it was posted with a sensible Cc list and it also did get
> wider opportunity for possible discussion if needed, thanks to LWN:
> 	https://lwn.net/Articles/643854/
> 
> The only other thread of discussion I can think of was where another
> subsystem maintainer looped me into the review of a new driver, because
> they were looking forward to having this in tree, due to the additional
> clarity it would add between modular and non modular code:
> 	https://lkml.kernel.org/r/20150620180435.GG16386@windriver.com
> 
> > Ideally some subsystem people would ack it...
> 
> Yes a good many of the deployment patches themselves are Ack'd.  For the
> core macro introduction itself, you were Cc'd on it [and the 0/7 intro].
> 
> Given my above mentioned commits that you'd read and merged that
> mentioned this, I didn't want to burn karma nagging you for an explicit
> ack for this one basic commit itself, given how busy you are with
> stable, staging, etc.  But I did explicitly put you on the Cc for this
> pull figuring it would be an opportunity to keep you in the loop and
> provide a last chance opportunity for a "No, don't do this because..."

Fair enough, thanks for that, I don't have any objections to this pull
request.

> If I have to burn karma nagging you about something, I'd rather it be
> something more important, like adding this (unrelated) clk_add_alias fix
> to staging -- since its absence has been breaking powerpc, s390, parisc,
> cris, ... etc. builds in linux-next for quite some time now.  :)
> 
> 	https://lkml.org/lkml/2015/6/25/365

Heh, it's not nagging, I'll queue that up after 4.2-rc1 is out, along
with other fixes.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-07-01 15:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-01  1:19 [GIT PULL] Introduce builtin_platform_driver for non modules Paul Gortmaker
2015-07-01  1:24 ` Greg KH
2015-07-01  6:53   ` Stephen Rothwell
2015-07-01 15:29     ` Greg KH
2015-07-01 15:33   ` Paul Gortmaker
2015-07-01 15:39     ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.