[pinmux,scripts,1/3] Update kernel driver template
diff mbox series

Message ID 20190620170055.19771-1-thierry.reding@gmail.com
State New
Headers show
Series
  • [pinmux,scripts,1/3] Update kernel driver template
Related show

Commit Message

Thierry Reding June 20, 2019, 5 p.m. UTC
From: Thierry Reding <treding@nvidia.com>

Some changes in recent years have modified the upstream kernel driver in
some ways that make it incompatible with the current template. Update
the template to take into account changes introduced by the following
commits:

	commit e3d2160f12d6aa7a87d9db09d8458b4a3492cd45
	Author: Paul Gortmaker <paul.gortmaker@windriver.com>
	Date:   Mon May 22 16:56:47 2017 -0400

	    pinctrl: tegra: clean up modular vs. non-modular distinctions

	    None of the Kconfigs for any of these drivers are tristate,
	    meaning that they currently are not being built as a module by anyone.

	    Lets remove the modular code that is essentially orphaned, so that
	    when reading the drivers there is no doubt they are builtin-only.  All
	    drivers get similar changes, so they are handled in batch.

	    We remove module.h from code that isn't doing anything modular at
	    all;  if they have __init sections, then replace it with init.h.

	    A couple drivers have module_exit() code that is essentially orphaned,
	    and so we remove that.

	    Quite a few bool drivers (hence non-modular) are converted over to
	    to builtin_platform_driver().

	    Since module_platform_driver() uses the same init level priority as
	    builtin_platform_driver() the init ordering remains unchanged with
	    this commit.

	    Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.

	    We also delete the MODULE_LICENSE tag etc. since all that information
	    was (or is now) contained at the top of the file in the comments.

	    Cc: Linus Walleij <linus.walleij@linaro.org>
	    Cc: Stephen Warren <swarren@wwwdotorg.org>
	    Cc: Thierry Reding <thierry.reding@gmail.com>
	    Cc: Alexandre Courbot <gnurou@gmail.com>
	    Cc: Pritesh Raithatha <praithatha@nvidia.com>
	    Cc: Ashwini Ghuge <aghuge@nvidia.com>
	    Cc: linux-gpio@vger.kernel.org
	    Cc: linux-tegra@vger.kernel.org
	    Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
	    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

	commit 3c94d2d08a032d911bbe34f2edb24cb63a63644a
	Author: Stefan Agner <stefan@agner.ch>
	Date:   Thu Jul 26 17:40:24 2018 +0200

	    pinctrl: tegra: define GPIO compatible node per SoC

	    Tegra 2 uses a different GPIO controller which uses "tegra20-gpio" as
	    compatible string.

	    Make the compatible string the GPIO node is using a SoC specific
	    property. This prevents the kernel from registering the GPIO range
	    twice in case the GPIO range is specified in the device tree.

	    Fixes: 9462510ce31e ("pinctrl: tegra: Only set the gpio range if needed")
	    Signed-off-by: Stefan Agner <stefan@agner.ch>
	    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

	commit 1e0813ee5599932c856bda64a568895ed7a33d3a
	Author: Dmitry Osipenko <digetx@gmail.com>
	Date:   Thu Aug 2 14:11:43 2018 +0300

	    pinctrl: tegra: Move drivers registration to arch_init level

	    There is a bug in regards to deferred probing within the drivers core
	    that causes GPIO-driver to suspend after its users. The bug appears if
	    GPIO-driver probe is getting deferred, which happens after introducing
	    dependency on PINCTRL-driver for the GPIO-driver by defining "gpio-ranges"
	    property in device-tree. The bug in the drivers core is old (more than 4
	    years now) and is well known, unfortunately there is no easy fix for it.
	    The good news is that we can workaround the deferred probe issue by
	    changing GPIO / PINCTRL drivers registration order and hence by moving
	    PINCTRL driver registration to the arch_init level and GPIO to the
	    subsys_init.

	    Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
	    Acked-by: Stefan Agner <stefan@agner.ch>
	    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Note that the last one is something that we probably should fix
correctly by using device links rather than working around it by playing
init level tricks.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 soc-to-kernel-pinctrl-driver.py | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

Comments

Thierry Reding June 20, 2019, 5:08 p.m. UTC | #1
On Thu, Jun 20, 2019 at 07:00:53PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Some changes in recent years have modified the upstream kernel driver in
> some ways that make it incompatible with the current template. Update
> the template to take into account changes introduced by the following
> commits:
> 
> 	commit e3d2160f12d6aa7a87d9db09d8458b4a3492cd45
> 	Author: Paul Gortmaker <paul.gortmaker@windriver.com>
> 	Date:   Mon May 22 16:56:47 2017 -0400
> 
> 	    pinctrl: tegra: clean up modular vs. non-modular distinctions
> 
> 	    None of the Kconfigs for any of these drivers are tristate,
> 	    meaning that they currently are not being built as a module by anyone.
> 
> 	    Lets remove the modular code that is essentially orphaned, so that
> 	    when reading the drivers there is no doubt they are builtin-only.  All
> 	    drivers get similar changes, so they are handled in batch.
> 
> 	    We remove module.h from code that isn't doing anything modular at
> 	    all;  if they have __init sections, then replace it with init.h.
> 
> 	    A couple drivers have module_exit() code that is essentially orphaned,
> 	    and so we remove that.
> 
> 	    Quite a few bool drivers (hence non-modular) are converted over to
> 	    to builtin_platform_driver().
> 
> 	    Since module_platform_driver() uses the same init level priority as
> 	    builtin_platform_driver() the init ordering remains unchanged with
> 	    this commit.
> 
> 	    Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.
> 
> 	    We also delete the MODULE_LICENSE tag etc. since all that information
> 	    was (or is now) contained at the top of the file in the comments.
> 
> 	    Cc: Linus Walleij <linus.walleij@linaro.org>
> 	    Cc: Stephen Warren <swarren@wwwdotorg.org>
> 	    Cc: Thierry Reding <thierry.reding@gmail.com>
> 	    Cc: Alexandre Courbot <gnurou@gmail.com>
> 	    Cc: Pritesh Raithatha <praithatha@nvidia.com>
> 	    Cc: Ashwini Ghuge <aghuge@nvidia.com>
> 	    Cc: linux-gpio@vger.kernel.org
> 	    Cc: linux-tegra@vger.kernel.org
> 	    Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> 	    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> 
> 	commit 3c94d2d08a032d911bbe34f2edb24cb63a63644a
> 	Author: Stefan Agner <stefan@agner.ch>
> 	Date:   Thu Jul 26 17:40:24 2018 +0200
> 
> 	    pinctrl: tegra: define GPIO compatible node per SoC
> 
> 	    Tegra 2 uses a different GPIO controller which uses "tegra20-gpio" as
> 	    compatible string.
> 
> 	    Make the compatible string the GPIO node is using a SoC specific
> 	    property. This prevents the kernel from registering the GPIO range
> 	    twice in case the GPIO range is specified in the device tree.
> 
> 	    Fixes: 9462510ce31e ("pinctrl: tegra: Only set the gpio range if needed")
> 	    Signed-off-by: Stefan Agner <stefan@agner.ch>
> 	    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> 
> 	commit 1e0813ee5599932c856bda64a568895ed7a33d3a
> 	Author: Dmitry Osipenko <digetx@gmail.com>
> 	Date:   Thu Aug 2 14:11:43 2018 +0300
> 
> 	    pinctrl: tegra: Move drivers registration to arch_init level
> 
> 	    There is a bug in regards to deferred probing within the drivers core
> 	    that causes GPIO-driver to suspend after its users. The bug appears if
> 	    GPIO-driver probe is getting deferred, which happens after introducing
> 	    dependency on PINCTRL-driver for the GPIO-driver by defining "gpio-ranges"
> 	    property in device-tree. The bug in the drivers core is old (more than 4
> 	    years now) and is well known, unfortunately there is no easy fix for it.
> 	    The good news is that we can workaround the deferred probe issue by
> 	    changing GPIO / PINCTRL drivers registration order and hence by moving
> 	    PINCTRL driver registration to the arch_init level and GPIO to the
> 	    subsys_init.
> 
> 	    Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> 	    Acked-by: Stefan Agner <stefan@agner.ch>
> 	    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Note that the last one is something that we probably should fix
> correctly by using device links rather than working around it by playing
> init level tricks.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  soc-to-kernel-pinctrl-driver.py | 27 +++++++++++----------------
>  1 file changed, 11 insertions(+), 16 deletions(-)

There was supposed to be a cover letter for this... Anyway, patches 1
and 2 basically get the kernel driver template up to date with the
latest kernel driver. Patch 3 is Sowjanya's patch from yesterday but
with one suggestion worked in to make the parked_bitmask field take 0
as the special value meaning "unsupported".

I've also sent out two patches that are the result of regenerating the
various SoC drivers from the updated script and tables.

Thierry

> 
> diff --git a/soc-to-kernel-pinctrl-driver.py b/soc-to-kernel-pinctrl-driver.py
> index 65e4c604f1c9..37f34b15db2b 100755
> --- a/soc-to-kernel-pinctrl-driver.py
> +++ b/soc-to-kernel-pinctrl-driver.py
> @@ -41,22 +41,16 @@ if dbg: print(args)
>  soc = tegra_pmx_soc_parser.load_soc(args.soc)
>  
>  print('''\
> +// SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * Pinctrl data for the NVIDIA %s pinmux
>   *
> - * Copyright (c) %s, NVIDIA CORPORATION.  All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms and conditions of the GNU General Public License,
> - * version 2, as published by the Free Software Foundation.
> + * Author: %s
>   *
> - * This program is distributed in the hope it will be useful, but WITHOUT
> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> - * more details.
> + * Copyright (c) %s, NVIDIA CORPORATION.  All rights reserved.
>   */
>  
> -#include <linux/module.h>
> +#include <linux/init.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/pinctrl/pinctrl.h>
> @@ -68,7 +62,7 @@ print('''\
>   * Most pins affected by the pinmux can also be GPIOs. Define these first.
>   * These must match how the GPIO driver names/numbers its pins.
>   */
> -''' % (soc.titlename, soc.kernel_copyright_years), end='')
> +''' % (soc.titlename, soc.kernel_author, soc.kernel_copyright_years), end='')
>  
>  # Do not add any more exceptions here; new SoCs should be formatted correctly
>  if soc.name == 'tegra30':
> @@ -615,6 +609,7 @@ print('''\
>  
>  static const struct tegra_pinctrl_soc_data %(soc)s_pinctrl = {
>  	.ngpios = NUM_GPIOS,
> +	.gpio_compatible = "nvidia,%(soc)s-gpio",
>  	.pins = %(soc)s_pins,
>  	.npins = ARRAY_SIZE(%(soc)s_pins),
>  	.functions = %(soc)s_functions,
> @@ -635,7 +630,6 @@ static const struct of_device_id %(soc)s_pinctrl_of_match[] = {
>  	{ .compatible = "nvidia,%(soc)s-pinmux", },
>  	{ },
>  };
> -MODULE_DEVICE_TABLE(of, %(soc)s_pinctrl_of_match);
>  
>  static struct platform_driver %(soc)s_pinctrl_driver = {
>  	.driver = {
> @@ -644,9 +638,10 @@ static struct platform_driver %(soc)s_pinctrl_driver = {
>  	},
>  	.probe = %(soc)s_pinctrl_probe,
>  };
> -module_platform_driver(%(soc)s_pinctrl_driver);
>  
> -MODULE_AUTHOR("%(author)s");
> -MODULE_DESCRIPTION("NVIDIA %(usoc)s pinctrl driver");
> -MODULE_LICENSE("GPL v2");
> +static int __init %(soc)s_pinctrl_init(void)
> +{
> +	return platform_driver_register(&%(soc)s_pinctrl_driver);
> +}
> +arch_initcall(%(soc)s_pinctrl_init);
>  ''' % socvars, end='')
> -- 
> 2.21.0
>
Dmitry Osipenko June 20, 2019, 10:40 p.m. UTC | #2
20.06.2019 20:08, Thierry Reding пишет:
> On Thu, Jun 20, 2019 at 07:00:53PM +0200, Thierry Reding wrote:
>> From: Thierry Reding <treding@nvidia.com>
>>
>> Some changes in recent years have modified the upstream kernel driver in
>> some ways that make it incompatible with the current template. Update
>> the template to take into account changes introduced by the following
>> commits:
>>
>> 	commit e3d2160f12d6aa7a87d9db09d8458b4a3492cd45
>> 	Author: Paul Gortmaker <paul.gortmaker@windriver.com>
>> 	Date:   Mon May 22 16:56:47 2017 -0400
>>
>> 	    pinctrl: tegra: clean up modular vs. non-modular distinctions
>>
>> 	    None of the Kconfigs for any of these drivers are tristate,
>> 	    meaning that they currently are not being built as a module by anyone.
>>
>> 	    Lets remove the modular code that is essentially orphaned, so that
>> 	    when reading the drivers there is no doubt they are builtin-only.  All
>> 	    drivers get similar changes, so they are handled in batch.
>>
>> 	    We remove module.h from code that isn't doing anything modular at
>> 	    all;  if they have __init sections, then replace it with init.h.
>>
>> 	    A couple drivers have module_exit() code that is essentially orphaned,
>> 	    and so we remove that.
>>
>> 	    Quite a few bool drivers (hence non-modular) are converted over to
>> 	    to builtin_platform_driver().
>>
>> 	    Since module_platform_driver() uses the same init level priority as
>> 	    builtin_platform_driver() the init ordering remains unchanged with
>> 	    this commit.
>>
>> 	    Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.
>>
>> 	    We also delete the MODULE_LICENSE tag etc. since all that information
>> 	    was (or is now) contained at the top of the file in the comments.
>>
>> 	    Cc: Linus Walleij <linus.walleij@linaro.org>
>> 	    Cc: Stephen Warren <swarren@wwwdotorg.org>
>> 	    Cc: Thierry Reding <thierry.reding@gmail.com>
>> 	    Cc: Alexandre Courbot <gnurou@gmail.com>
>> 	    Cc: Pritesh Raithatha <praithatha@nvidia.com>
>> 	    Cc: Ashwini Ghuge <aghuge@nvidia.com>
>> 	    Cc: linux-gpio@vger.kernel.org
>> 	    Cc: linux-tegra@vger.kernel.org
>> 	    Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>> 	    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> 	commit 3c94d2d08a032d911bbe34f2edb24cb63a63644a
>> 	Author: Stefan Agner <stefan@agner.ch>
>> 	Date:   Thu Jul 26 17:40:24 2018 +0200
>>
>> 	    pinctrl: tegra: define GPIO compatible node per SoC
>>
>> 	    Tegra 2 uses a different GPIO controller which uses "tegra20-gpio" as
>> 	    compatible string.
>>
>> 	    Make the compatible string the GPIO node is using a SoC specific
>> 	    property. This prevents the kernel from registering the GPIO range
>> 	    twice in case the GPIO range is specified in the device tree.
>>
>> 	    Fixes: 9462510ce31e ("pinctrl: tegra: Only set the gpio range if needed")
>> 	    Signed-off-by: Stefan Agner <stefan@agner.ch>
>> 	    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> 	commit 1e0813ee5599932c856bda64a568895ed7a33d3a
>> 	Author: Dmitry Osipenko <digetx@gmail.com>
>> 	Date:   Thu Aug 2 14:11:43 2018 +0300
>>
>> 	    pinctrl: tegra: Move drivers registration to arch_init level
>>
>> 	    There is a bug in regards to deferred probing within the drivers core
>> 	    that causes GPIO-driver to suspend after its users. The bug appears if
>> 	    GPIO-driver probe is getting deferred, which happens after introducing
>> 	    dependency on PINCTRL-driver for the GPIO-driver by defining "gpio-ranges"
>> 	    property in device-tree. The bug in the drivers core is old (more than 4
>> 	    years now) and is well known, unfortunately there is no easy fix for it.
>> 	    The good news is that we can workaround the deferred probe issue by
>> 	    changing GPIO / PINCTRL drivers registration order and hence by moving
>> 	    PINCTRL driver registration to the arch_init level and GPIO to the
>> 	    subsys_init.
>>
>> 	    Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> 	    Acked-by: Stefan Agner <stefan@agner.ch>
>> 	    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> Note that the last one is something that we probably should fix
>> correctly by using device links rather than working around it by playing
>> init level tricks.

I'd leave the init level as-is because it also resolves deferred probing
of the drivers which may come back if you'll move all drivers to the
default boot level. Drivers core should be aware of the hardware
topology to be able to probe drivers in the right order, but that's not
supported for the device tree.

Indeed, will be nice to have an explicit dependency on the PINCTRL
driver for the GPIO.

>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>> ---
>>  soc-to-kernel-pinctrl-driver.py | 27 +++++++++++----------------
>>  1 file changed, 11 insertions(+), 16 deletions(-)
> 
> There was supposed to be a cover letter for this... Anyway, patches 1
> and 2 basically get the kernel driver template up to date with the
> latest kernel driver. Patch 3 is Sowjanya's patch from yesterday but
> with one suggestion worked in to make the parked_bitmask field take 0
> as the special value meaning "unsupported".
> 
> I've also sent out two patches that are the result of regenerating the
> various SoC drivers from the updated script and tables.

This all is looking good to me! Thank you very much!
Stephen Warren July 1, 2019, 11:43 p.m. UTC | #3
On 6/20/19 11:00 AM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Some changes in recent years have modified the upstream kernel driver in
> some ways that make it incompatible with the current template. Update
> the template to take into account changes introduced by the following
> commits:

All 3 patches applied.

Patch
diff mbox series

diff --git a/soc-to-kernel-pinctrl-driver.py b/soc-to-kernel-pinctrl-driver.py
index 65e4c604f1c9..37f34b15db2b 100755
--- a/soc-to-kernel-pinctrl-driver.py
+++ b/soc-to-kernel-pinctrl-driver.py
@@ -41,22 +41,16 @@  if dbg: print(args)
 soc = tegra_pmx_soc_parser.load_soc(args.soc)
 
 print('''\
+// SPDX-License-Identifier: GPL-2.0-only
 /*
  * Pinctrl data for the NVIDIA %s pinmux
  *
- * Copyright (c) %s, NVIDIA CORPORATION.  All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
+ * Author: %s
  *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
+ * Copyright (c) %s, NVIDIA CORPORATION.  All rights reserved.
  */
 
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pinctrl/pinctrl.h>
@@ -68,7 +62,7 @@  print('''\
  * Most pins affected by the pinmux can also be GPIOs. Define these first.
  * These must match how the GPIO driver names/numbers its pins.
  */
-''' % (soc.titlename, soc.kernel_copyright_years), end='')
+''' % (soc.titlename, soc.kernel_author, soc.kernel_copyright_years), end='')
 
 # Do not add any more exceptions here; new SoCs should be formatted correctly
 if soc.name == 'tegra30':
@@ -615,6 +609,7 @@  print('''\
 
 static const struct tegra_pinctrl_soc_data %(soc)s_pinctrl = {
 	.ngpios = NUM_GPIOS,
+	.gpio_compatible = "nvidia,%(soc)s-gpio",
 	.pins = %(soc)s_pins,
 	.npins = ARRAY_SIZE(%(soc)s_pins),
 	.functions = %(soc)s_functions,
@@ -635,7 +630,6 @@  static const struct of_device_id %(soc)s_pinctrl_of_match[] = {
 	{ .compatible = "nvidia,%(soc)s-pinmux", },
 	{ },
 };
-MODULE_DEVICE_TABLE(of, %(soc)s_pinctrl_of_match);
 
 static struct platform_driver %(soc)s_pinctrl_driver = {
 	.driver = {
@@ -644,9 +638,10 @@  static struct platform_driver %(soc)s_pinctrl_driver = {
 	},
 	.probe = %(soc)s_pinctrl_probe,
 };
-module_platform_driver(%(soc)s_pinctrl_driver);
 
-MODULE_AUTHOR("%(author)s");
-MODULE_DESCRIPTION("NVIDIA %(usoc)s pinctrl driver");
-MODULE_LICENSE("GPL v2");
+static int __init %(soc)s_pinctrl_init(void)
+{
+	return platform_driver_register(&%(soc)s_pinctrl_driver);
+}
+arch_initcall(%(soc)s_pinctrl_init);
 ''' % socvars, end='')