On Wed, Jun 19, 2019 at 10:33:08AM +0200, Thierry Reding wrote:
> On Tue, Jun 18, 2019 at 09:41:03AM -0600, Stephen Warren wrote:
> > On 6/18/19 3:30 AM, Dmitry Osipenko wrote:
> > > 18.06.2019 12:22, Dmitry Osipenko пишет:
> > > > 18.06.2019 10:46, Sowjanya Komatineni пишет:
> > > > > This patch adds suspend and resume support for Tegra pinctrl driver
> > > > > and registers them to syscore so the pinmux settings are restored
> > > > > before the devices resume.
> > > > > 
> > > > > Signed-off-by: Sowjanya Komatineni <skomatin...@nvidia.com>
> > > > > ---
> > > > >   drivers/pinctrl/tegra/pinctrl-tegra.c    | 62 
> > > > > ++++++++++++++++++++++++++++++++
> > > > >   drivers/pinctrl/tegra/pinctrl-tegra.h    |  5 +++
> > > > >   drivers/pinctrl/tegra/pinctrl-tegra114.c |  1 +
> > > > >   drivers/pinctrl/tegra/pinctrl-tegra124.c |  1 +
> > > > >   drivers/pinctrl/tegra/pinctrl-tegra20.c  |  1 +
> > > > >   drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
> > > > >   drivers/pinctrl/tegra/pinctrl-tegra30.c  |  1 +
> > > > >   7 files changed, 84 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c 
> > > > > b/drivers/pinctrl/tegra/pinctrl-tegra.c
> > > > > index 34596b246578..ceced30d8bd1 100644
> > > > > --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> > > > > +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> > > > > @@ -20,11 +20,16 @@
> > > > >   #include <linux/pinctrl/pinmux.h>
> > > > >   #include <linux/pinctrl/pinconf.h>
> > > > >   #include <linux/slab.h>
> > > > > +#include <linux/syscore_ops.h>
> > > > >   #include "../core.h"
> > > > >   #include "../pinctrl-utils.h"
> > > > >   #include "pinctrl-tegra.h"
> > > > > +#define EMMC2_PAD_CFGPADCTRL_0                       0x1c8
> > > > > +#define EMMC4_PAD_CFGPADCTRL_0                       0x1e0
> > > > > +#define EMMC_DPD_PARKING                     (0x1fff << 14)
> > > > > +
> > > > >   static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 
> > > > > reg)
> > > > >   {
> > > > >       return readl(pmx->regs[bank] + reg);
> > > > > @@ -619,6 +624,48 @@ static void 
> > > > > tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
> > > > >                       pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
> > > > >               }
> > > > >       }
> > > > > +
> > > > > +     if (pmx->soc->has_park_padcfg) {
> > > > > +             val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
> > > > > +             val &= ~EMMC_DPD_PARKING;
> > > > > +             pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
> > > > > +
> > > > > +             val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
> > > > > +             val &= ~EMMC_DPD_PARKING;
> > > > > +             pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
> > > > > +     }
> > > > > +}
> > > > 
> > > > Is there any reason why parked_bit can't be changed to parked_bitmask 
> > > > like I was
> > > > asking in a comment to v2?
> > > > 
> > > > I suppose that it's more preferable to keep pinctrl-tegra.c 
> > > > platform-agnostic for
> > > > consistency when possible, hence adding platform specifics here should 
> > > > be discouraged.
> > > > And then the parked_bitmask will also result in a proper hardware 
> > > > description in the code.
> > > > 
> > > 
> > > I'm now also vaguely recalling that Stephen Warren had some kind of a 
> > > "code generator"
> > > for the pinctrl drivers. So I guess all those tables were auto-generated 
> > > initially.
> > > 
> > > Stephen, maybe you could adjust the generator to take into account the 
> > > bitmask (of
> > > course if that's a part of the generated code) and then re-gen it all for 
> > > Sowjanya?
> > 
> > https://github.com/NVIDIA/tegra-pinmux-scripts holds the scripts that
> > generate tegra-pinctrlNNN.c. See    soc-to-kernel-pinctrl-driver.py. IIRC,
> > tegra-pinctrl.c (the core file) isn't auto-generated. Sowjanya is welcome to
> > send a patch to that repo if the code needs to be updated.
> 
> If we want to do that, we may need to start off by bringing the pinmux
> scripts up to date with the latest version of the generated files. There
> have been a number of changes in the meantime that cause the scripts to
> generate a bit of diff with regards to what's currently upstream. Sounds
> like something fairly trivial, though.

Something like the below should do the trick.

Thierry

--- >8 ---
From 9a684d2ad3c0e0c7b4dbda5904db1fda3757072b Mon Sep 17 00:00:00 2001
From: Thierry Reding <tred...@nvidia.com>
Date: Wed, 19 Jun 2019 10:50:57 +0200
Subject: [pinmux scripts PATCH] Update kernel driver template

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.gortma...@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.wall...@linaro.org>
            Cc: Stephen Warren <swar...@wwwdotorg.org>
            Cc: Thierry Reding <thierry.red...@gmail.com>
            Cc: Alexandre Courbot <gnu...@gmail.com>
            Cc: Pritesh Raithatha <praitha...@nvidia.com>
            Cc: Ashwini Ghuge <agh...@nvidia.com>
            Cc: linux-g...@vger.kernel.org
            Cc: linux-te...@vger.kernel.org
            Signed-off-by: Paul Gortmaker <paul.gortma...@windriver.com>
            Signed-off-by: Linus Walleij <linus.wall...@linaro.org>

        commit 3c94d2d08a032d911bbe34f2edb24cb63a63644a
        Author: Stefan Agner <ste...@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 <ste...@agner.ch>
            Signed-off-by: Linus Walleij <linus.wall...@linaro.org>

        commit 1e0813ee5599932c856bda64a568895ed7a33d3a
        Author: Dmitry Osipenko <dig...@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 <dig...@gmail.com>
            Acked-by: Stefan Agner <ste...@agner.ch>
            Signed-off-by: Linus Walleij <linus.wall...@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 <tred...@nvidia.com>
---
 soc-to-kernel-pinctrl-driver.py | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

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

Attachment: signature.asc
Description: PGP signature

Reply via email to