Linux GPIO subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH] gpio: tps65219: don't use CONFIG_DEBUG_GPIO
From: Linus Walleij @ 2023-12-22 17:51 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Tony Lindgren, Andy Shevchenko, linux-omap, linux-gpio,
	linux-kernel, Bartosz Golaszewski
In-Reply-To: <20231221185702.24685-1-brgl@bgdev.pl>

On Thu, Dec 21, 2023 at 7:57 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> CONFIG_DEBUG_GPIO should only be used to enable debug log messages and
> for core GPIOLIB debugging. Don't use it to control the execution of
> potentially buggy code. Just put it under an always-false #if.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Maybe I would have simply deleted the code, but OK. I bet some static
analyzers will promptly return complaints about "no #if 0 in the kernel" :/
In this case it's an OK compromise.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply

* Re: [RFC PATCH 0/6] Add Pinctrl driver for Starfive JH8100 SoC
From: Linus Walleij @ 2023-12-22 17:58 UTC (permalink / raw)
  To: Alex Soo
  Cc: Bartosz Golaszewski, Hal Feng, Ley Foon Tan, Jianlong Huang,
	Emil Renner Berthing, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Drew Fustini, linux-gpio, linux-kernel, devicetree,
	linux-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou
In-Reply-To: <20231221083622.3445726-1-yuklin.soo@starfivetech.com>

Hi Alex,

thanks for your patch!

On Thu, Dec 21, 2023 at 9:36 AM Alex Soo <yuklin.soo@starfivetech.com> wrote:

>   pinctrl: starfive: jh8100: add pinctrl driver for sys_east domain
>   pinctrl: starfive: jh8100: add pinctrl driver for sys_west domain
>   pinctrl: starfive: jh8100: add pinctrl driver for sys_gmac domain
>   pinctrl: starfive: jh8100: add pinctrl driver for AON domain

To my eye it looks like a lot of code is duplicated between the four subdrivers.

The pattern from other pin controllers is to create a file with all the common
code and then subdrivers for each sub-pincontroller that have their own
probe but calls into the library.

C.f.
drivers/pinctrl/qcom/pinctrl-apq8064.c:

static int apq8064_pinctrl_probe(struct platform_device *pdev)
{
        return msm_pinctrl_probe(pdev, &apq8064_pinctrl);
}

And that function is in drivers/pinctrl/qcom/pinctrl-msm.c
and you find great inspiration in the qcom Kconfig and Makefile
and drivers/pinctrl/qcom/pinctrl-msm.h
that you can copypaste to pull this off.

Maybe you should start with a patch that extract the common stuff
from the existing jh7100/jh7110 drivers and then reuse that for
jh8100?

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v2 00/22] Improve IRQ wake capability reporting and update the cros_ec driver to use it
From: Mark Hasemeyer @ 2023-12-22 22:30 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: LKML, chrome-platform, cros-qcom-dts-watchers, devicetree,
	linux-acpi, linux-arm-kernel, linux-arm-msm, linux-gpio,
	linux-i2c, linux-mediatek, linux-rockchip, linux-samsung-soc,
	linux-tegra
In-Reply-To: <ZYRAuY1LGdD8_u5K@smile.fi.intel.com>

> Just wondering if you used --histogram diff algo when preparing patches.

Not knowingly. I use patman which uses 'git format-patch' under the
covers with some added options:
https://github.com/siemens/u-boot/blob/master/tools/patman/gitutil.py#L308

^ permalink raw reply

* [linusw-pinctrl:for-next] BUILD SUCCESS da1294407cb1142719706d77fca569df93f0c67d
From: kernel test robot @ 2023-12-22 23:31 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-gpio

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git for-next
branch HEAD: da1294407cb1142719706d77fca569df93f0c67d  Merge branch 'devel' into for-next

elapsed time: 2314m

configs tested: 80
configs skipped: 2

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha                             allnoconfig   gcc  
alpha                            allyesconfig   gcc  
alpha                               defconfig   gcc  
arc                              allmodconfig   gcc  
arc                               allnoconfig   gcc  
arc                              allyesconfig   gcc  
arc                                 defconfig   gcc  
arm                              allmodconfig   gcc  
arm                               allnoconfig   gcc  
arm                              allyesconfig   gcc  
arm                                 defconfig   clang
arm64                            allmodconfig   clang
arm64                             allnoconfig   gcc  
arm64                               defconfig   gcc  
csky                             allmodconfig   gcc  
csky                              allnoconfig   gcc  
csky                             allyesconfig   gcc  
csky                                defconfig   gcc  
hexagon                           allnoconfig   clang
hexagon                             defconfig   clang
i386                             allmodconfig   clang
i386                              allnoconfig   clang
i386                             allyesconfig   clang
i386                                defconfig   gcc  
loongarch                        allmodconfig   gcc  
loongarch                         allnoconfig   gcc  
loongarch                           defconfig   gcc  
m68k                             allmodconfig   gcc  
m68k                              allnoconfig   gcc  
m68k                             allyesconfig   gcc  
m68k                                defconfig   gcc  
microblaze                       allmodconfig   gcc  
microblaze                        allnoconfig   gcc  
microblaze                       allyesconfig   gcc  
microblaze                          defconfig   gcc  
mips                              allnoconfig   clang
mips                             allyesconfig   gcc  
nios2                            allmodconfig   gcc  
nios2                             allnoconfig   gcc  
nios2                            allyesconfig   gcc  
nios2                               defconfig   gcc  
openrisc                          allnoconfig   gcc  
openrisc                         allyesconfig   gcc  
openrisc                            defconfig   gcc  
parisc                           allmodconfig   gcc  
parisc                            allnoconfig   gcc  
parisc                           allyesconfig   gcc  
parisc                              defconfig   gcc  
parisc64                            defconfig   gcc  
powerpc                          allmodconfig   clang
powerpc                           allnoconfig   gcc  
powerpc                          allyesconfig   clang
riscv                            allmodconfig   gcc  
riscv                             allnoconfig   clang
riscv                            allyesconfig   gcc  
riscv                               defconfig   gcc  
riscv                          rv32_defconfig   clang
s390                             allmodconfig   gcc  
s390                              allnoconfig   gcc  
s390                             allyesconfig   gcc  
s390                                defconfig   gcc  
sh                               allmodconfig   gcc  
sh                                allnoconfig   gcc  
sh                               allyesconfig   gcc  
sh                                  defconfig   gcc  
sparc                            allmodconfig   gcc  
sparc64                          allmodconfig   gcc  
sparc64                          allyesconfig   gcc  
sparc64                             defconfig   gcc  
um                               allmodconfig   clang
um                                allnoconfig   clang
um                               allyesconfig   clang
um                                  defconfig   gcc  
um                             i386_defconfig   gcc  
um                           x86_64_defconfig   gcc  
x86_64                            allnoconfig   gcc  
x86_64                           allyesconfig   clang
x86_64                              defconfig   gcc  
x86_64                          rhel-8.3-rust   clang
xtensa                            allnoconfig   gcc  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [PATCH v1 1/8] dt-bindings: pinctrl: Add thead,th1520-pinctrl bindings
From: Linus Walleij @ 2023-12-23  0:18 UTC (permalink / raw)
  To: Emil Renner Berthing
  Cc: Rob Herring, linux-gpio, devicetree, linux-kernel, linux-riscv,
	Hoan Tran, Serge Semin, Bartosz Golaszewski, Andy Shevchenko,
	Krzysztof Kozlowski, Conor Dooley, Jisheng Zhang, Guo Ren, Fu Wei,
	Paul Walmsley, Palmer Dabbelt
In-Reply-To: <CAJM55Z8SwyNEqw4HWRd7G8Y9rdtOGtKy-KbzDorqohdK3nZg0A@mail.gmail.com>

On Thu, Dec 21, 2023 at 3:07 PM Emil Renner Berthing
<emil.renner.berthing@canonical.com> wrote:
> Linus Walleij wrote:

> > Do the people designing boards using this have better documentation than what
> > you have? Then either get that documentation or just don't give them
> > too much rope.
>
> We can certainly prevent Linux from ever combining the strong pull-up with the
> regular bias, but that doesn't mean that the vendor u-boot can't find a use for
> it and might hand over pins in such states Linux then wouldn't know how to
> handle.

What you are saying is "there might be people who have access to
documentation that I don't have so they do this crazy thing".

Clearly you cannot design for that.

Print a big fat warning and fail probe if it happens.

If U-Boot is using some feature you definitely cannot deal with if this
happens, and then the people doing this can very well write a patch for
the kernel.

> If you think its better we could just postpone that problem to when/if it ever
> happens.

Yes please.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v1 1/1] gpiolib: cdev: Split line_get_debounce_period() and use
From: Kent Gibson @ 2023-12-23  2:08 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Bartosz Golaszewski, Bartosz Golaszewski, Andy Shevchenko,
	linux-gpio, linux-kernel
In-Reply-To: <CACRpkdZrnOJ-Sjj4VpuVU0Gvzca_uGN9Um5Zj=bRMH2df4kRZw@mail.gmail.com>

On Fri, Dec 22, 2023 at 06:49:03PM +0100, Linus Walleij wrote:
> On Fri, Dec 22, 2023 at 3:15 PM Kent Gibson <warthog618@gmail.com> wrote:
> > On Fri, Dec 22, 2023 at 03:09:54PM +0100, Bartosz Golaszewski wrote:
>
> > > I can still split the uAPI files into their own section, make Linus
> > > and myself maintainers and make you a reviewer, how about that?
> >
> > That is closer to the reality, so that would work for me.
>
> Hmm I think of Kent as one of the main architects for UAPI v2
> so I would like you as maintainer, and me to be dropped, I already
> responded to that patch though.
>

There is no escaping that my fingerprints are all over that so it does
make sense to list me over you. Given that patch and git-tree management
will be deferred to the GPIO subsystem/Bart, there isn't much distinction
between a reviewer and a maintainer, so I'm ok with being listed as a
maintainer - I'll just have to pay a bit more attention to the list mails
than I have been.

Cheers,
Kent.

^ permalink raw reply

* Re: [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
From: kernel test robot @ 2023-12-23  2:05 UTC (permalink / raw)
  To: Mark Hasemeyer, LKML
  Cc: oe-kbuild-all, AngeloGioacchino Del Regno, Krzysztof Kozlowski,
	Tzung-Bi Shih, Raul Rangel, Konrad Dybcio, Andy Shevchenko,
	Rob Herring, Sudeep Holla, Mark Hasemeyer, Bartosz Golaszewski,
	Len Brown, Linus Walleij, Mika Westerberg, Rafael J. Wysocki,
	Wolfram Sang, linux-acpi, linux-gpio, linux-i2c
In-Reply-To: <20231220165423.v2.1.Ifd0903f1c351e84376d71dbdadbd43931197f5ea@changeid>

Hi Mark,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on chrome-platform/for-next chrome-platform/for-firmware-next wsa/i2c/for-next driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.7-rc6 next-20231222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mark-Hasemeyer/gpiolib-acpi-Modify-acpi_dev_irq_wake_get_by-to-use-resource/20231222-172104
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20231220165423.v2.1.Ifd0903f1c351e84376d71dbdadbd43931197f5ea%40changeid
patch subject: [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
config: x86_64-randconfig-161-20231222 (https://download.01.org/0day-ci/archive/20231223/202312230907.szXqJyXq-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231223/202312230907.szXqJyXq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312230907.szXqJyXq-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpio/gpiolib-acpi.c:117: warning: Function parameter or member 'shareable' not described in 'acpi_gpio_info'


vim +117 drivers/gpio/gpiolib-acpi.c

aa92b6f689acf1 Mika Westerberg 2014-03-10   93  
b7452d670fdef8 Dmitry Torokhov 2022-11-15   94  /**
b7452d670fdef8 Dmitry Torokhov 2022-11-15   95   * struct acpi_gpio_info - ACPI GPIO specific information
b7452d670fdef8 Dmitry Torokhov 2022-11-15   96   * @adev: reference to ACPI device which consumes GPIO resource
b7452d670fdef8 Dmitry Torokhov 2022-11-15   97   * @flags: GPIO initialization flags
b7452d670fdef8 Dmitry Torokhov 2022-11-15   98   * @gpioint: if %true this GPIO is of type GpioInt otherwise type is GpioIo
b7452d670fdef8 Dmitry Torokhov 2022-11-15   99   * @pin_config: pin bias as provided by ACPI
b7452d670fdef8 Dmitry Torokhov 2022-11-15  100   * @polarity: interrupt polarity as provided by ACPI
b7452d670fdef8 Dmitry Torokhov 2022-11-15  101   * @triggering: triggering type as provided by ACPI
b7452d670fdef8 Dmitry Torokhov 2022-11-15  102   * @wake_capable: wake capability as provided by ACPI
b7452d670fdef8 Dmitry Torokhov 2022-11-15  103   * @debounce: debounce timeout as provided by ACPI
b7452d670fdef8 Dmitry Torokhov 2022-11-15  104   * @quirks: Linux specific quirks as provided by struct acpi_gpio_mapping
b7452d670fdef8 Dmitry Torokhov 2022-11-15  105   */
b7452d670fdef8 Dmitry Torokhov 2022-11-15  106  struct acpi_gpio_info {
b7452d670fdef8 Dmitry Torokhov 2022-11-15  107  	struct acpi_device *adev;
b7452d670fdef8 Dmitry Torokhov 2022-11-15  108  	enum gpiod_flags flags;
b7452d670fdef8 Dmitry Torokhov 2022-11-15  109  	bool gpioint;
b7452d670fdef8 Dmitry Torokhov 2022-11-15  110  	int pin_config;
b7452d670fdef8 Dmitry Torokhov 2022-11-15  111  	int polarity;
b7452d670fdef8 Dmitry Torokhov 2022-11-15  112  	int triggering;
b7452d670fdef8 Dmitry Torokhov 2022-11-15  113  	bool wake_capable;
189f4620fa2d51 Mark Hasemeyer  2023-12-20  114  	bool shareable;
b7452d670fdef8 Dmitry Torokhov 2022-11-15  115  	unsigned int debounce;
b7452d670fdef8 Dmitry Torokhov 2022-11-15  116  	unsigned int quirks;
b7452d670fdef8 Dmitry Torokhov 2022-11-15 @117  };
b7452d670fdef8 Dmitry Torokhov 2022-11-15  118  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* RE: [PATCH 7/7] pinctrl: scmi: implement pinctrl_scmi_imx_dt_node_to_map
From: Peng Fan @ 2023-12-23  2:14 UTC (permalink / raw)
  To: Cristian Marussi, Peng Fan (OSS)
  Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Oleksii Moisieiev, Linus Walleij, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, dl-linux-imx,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-gpio@vger.kernel.org
In-Reply-To: <ZYXK2YVJ1nnETSuI@pluto>

> Subject: Re: [PATCH 7/7] pinctrl: scmi: implement
> pinctrl_scmi_imx_dt_node_to_map
> 
> On Fri, Dec 15, 2023 at 07:56:35PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > i.MX95 System Manager FW supports SCMI PINCTRL protocol, but uses
> OEM
> > Pin Configuration type, so need i.MX specific dt_node_to_map.
> >
> 
> This does not even compile for me, as of now, when configuring the Pinctrl
> SCMI driver as a module with your IMX custom additions. (I think the
> Makefile with the additional pinctrl-imx is wrong in how describes the objects
> composing the pinctrl-scmi module with IMX addons...)
> 
> ERROR: modpost: "pinctrl_scmi_imx_dt_node_to_map"
> [drivers/pinctrl/pinctrl-scmi.ko] undefined!
> make[3]: *** [dev/src/linux/scripts/Makefile.modpost:145: Module.symvers]
> Error 1
> make[2]: *** [dev/src/linux/Makefile:1863: modpost] Error 2
> make[1]: *** [dev/src/linux/Makefile:234: __sub-make] Error 2
> make[1]: Leaving directory dev/out_linux
> make: *** [Makefile:234: __sub-make] Error 2

Oh, sorry for this. I could post a new version if you require. But before
that we may better align on the approach on how to support i.MX.

> 
> More in general, I think that this NXP OEM specific additions, which are in
> general welcome (and indeed as you know part of the spec was modified to
> allow for OEM specific needs), do NOT belong to this generic SCMI Pinctrl
> driver, because the driver from Oleksii/EPAM was born as a generic SCMI
> driver and it fits perfectly with the Generic Pinctrl Linux subsystem and
> related generic bindings parsing: now with this you are trying to stick a
> custom OEM slight varied behaviour (and related binding) on top of a generic
> thing.
> 
> And this choice leads to a number of additional changes in the SCMI core to
> support an even more complex handling of SCMI devices, which is already too
> complex IMO..
> 
> IOW...I dont think that the whole idea of the per-protocol optional
> compatible to be able to select slightly different behaviours/parsing would
> have a great chance to fly sincerely...
> 
> I know there is an issue with having a completely distinct SCMI IMX pinctrl
> driver that uses the same protocol node @19 (without the need for the
> compatible trick) due to the way in which the Pinctrl subsystem searches for
> devices (by of_node)...I'll think about an alternative way to allow this but I am
> not sure (as you saw) that would be so easily doable...

For all protocols supports VENDOR extension attributes, we need a way
to handle I think. 

As Linus wrote in
https://lore.kernel.org/all/CACRpkdaRY+rU+md-r5gVyFH5ATt3Pqp9=M4=+WArYkfVLAFdpw@mail.gmail.com/:

We may need:
protocol@19 {
    compatible = "vendor,soc-scmi-pinctrl";
(...)

> 
> Also, I am wondering if this is really a problem in reality since I would NOT
> expect you to load/ship both the OEM/NXP custom specific SCMI pinctrl
> driver AND the generic one on the same platform (after having made them
> distinct I mean...) am I wrong ?

You are right, but that means the upstream ARM64 defconfig will not 
able to support both i.MX9 and others.

Thanks,
Peng.

> (so you could even made them exclude each other at compile time...far from
> being the best option I agree...)
> 
> Thanks,
> Cristian

^ permalink raw reply

* Re: [PATCH v6 2/2] pinctrl: qcom: sm4450: dd SM4450 pinctrl driver
From: Bjorn Andersson @ 2023-12-23  2:57 UTC (permalink / raw)
  To: Tengfei Fan
  Cc: agross, konrad.dybcio, linus.walleij, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, linux-kernel, linux-arm-msm,
	linux-gpio, devicetree
In-Reply-To: <20231206020840.33228-3-quic_tengfan@quicinc.com>

On Wed, Dec 06, 2023 at 10:08:40AM +0800, Tengfei Fan wrote:
> Add pinctrl driver for TLMM block found in SM4450 SoC.
> Can Guo helped out in reviewing the driver.
> 
> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>

Reviewed-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn

^ permalink raw reply

* Re: [PATCH v6 1/2] dt-bindings: pinctrl: qcom: Add SM4450 pinctrl
From: Bjorn Andersson @ 2023-12-23  3:06 UTC (permalink / raw)
  To: Tengfei Fan
  Cc: agross, konrad.dybcio, linus.walleij, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, linux-kernel, linux-arm-msm,
	linux-gpio, devicetree
In-Reply-To: <20231206020840.33228-2-quic_tengfan@quicinc.com>

On Wed, Dec 06, 2023 at 10:08:39AM +0800, Tengfei Fan wrote:
> Add device tree binding Documentation details for Qualcomm SM4450
> TLMM device.
> 
> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
> ---
>  .../bindings/pinctrl/qcom,sm4450-tlmm.yaml    | 151 ++++++++++++++++++
>  1 file changed, 151 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml
> new file mode 100644
> index 000000000000..bb08ca5a1509
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml
> @@ -0,0 +1,151 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/qcom,sm4450-tlmm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Technologies, Inc. SM4450 TLMM block
> +
> +maintainers:
> +  - Tengfei Fan <quic_tengfan@quicinc.com>
> +
> +description:
> +  Top Level Mode Multiplexer pin controller in Qualcomm SM4450 SoC.
> +
> +allOf:
> +  - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: qcom,sm4450-pinctrl

This should be qcom,sm4450-tlmm

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts: true
> +  interrupt-controller: true
> +  "#interrupt-cells": true
> +  gpio-controller: true
> +
> +  gpio-reserved-ranges:
> +    minItems: 1
> +    maxItems: 68
> +
> +  gpio-line-names:
> +    maxItems: 136
> +
> +  "#gpio-cells": true
> +  gpio-ranges: true
> +  wakeup-parent: true
> +
> +patternProperties:
> +  "-state$":
> +    oneOf:
> +      - $ref: "#/$defs/qcom-sm4450-tlmm-state"
> +      - patternProperties:
> +          "-pins$":
> +            $ref: "#/$defs/qcom-sm4450-tlmm-state"
> +        additionalProperties: false
> +
> +$defs:
> +  qcom-sm4450-tlmm-state:
> +    type: object
> +    description:
> +      Pinctrl node's client devices use subnodes for desired pin configuration.
> +      Client device subnodes use below standard properties.
> +    $ref: qcom,tlmm-common.yaml#/$defs/qcom-tlmm-state
> +    unevaluatedProperties: false
> +
> +    properties:
> +      pins:
> +        description:
> +          List of gpio pins affected by the properties specified in this
> +          subnode.
> +        items:
> +          oneOf:
> +            - pattern: "^gpio([0-9]|[1-9][0-9]|1[0-2][0-9]|13[0-5])$"
> +            - enum: [ sdc2_clk, sdc2_cmd, sdc2_data, ufs_reset ]
> +          minItems: 1
> +          maxItems: 36
> +
> +      function:
> +        description:
> +          Specify the alternative function to be configured for the specified
> +          pins.
> +        enum: [ gpio, atest_char, atest_char0, atest_char1, atest_char2,
> +                atest_char3, atest_usb0, atest_usb00, atest_usb01, atest_usb02,
> +                atest_usb03, audio_ref, cam_mclk, cci_async, cci_i2c,
> +                cci_timer0, cci_timer1, cci_timer2, cci_timer3, cci_timer4,
> +                cmu_rng0, cmu_rng1, cmu_rng2, cmu_rng3, coex_uart1, cri_trng,
> +                cri_trng0, cri_trng1, dbg_out, ddr_bist, ddr_pxi0, ddr_pxi1,
> +                dp0_hot, gcc_gp1, gcc_gp2, gcc_gp3, host2wlan_sol, ibi_i3c,
> +                jitter_bist, mdp_vsync, mdp_vsync0, mdp_vsync1, mdp_vsync2,
> +                mdp_vsync3, mi2s0_data0, mi2s0_data1, mi2s0_sck, mi2s0_ws,
> +                mi2s2_data0, mi2s2_data1, mi2s2_sck, mi2s2_ws, mi2s_mclk0,
> +                mi2s_mclk1, nav_gpio0, nav_gpio1, nav_gpio2, pcie0_clk,
> +                phase_flag0, phase_flag1, phase_flag10, phase_flag11,
> +                phase_flag12, phase_flag13, phase_flag14, phase_flag15,
> +                phase_flag16, phase_flag17, phase_flag18, phase_flag19,
> +                phase_flag2, phase_flag20, phase_flag21, phase_flag22,
> +                phase_flag23, phase_flag24, phase_flag25, phase_flag26,
> +                phase_flag27, phase_flag28, phase_flag29, phase_flag3,
> +                phase_flag30, phase_flag31, phase_flag4, phase_flag5,
> +                phase_flag6, phase_flag7, phase_flag8, phase_flag9,
> +                pll_bist, pll_clk, prng_rosc0, prng_rosc1, prng_rosc2,
> +                prng_rosc3, qdss_cti, qdss_gpio, qdss_gpio0, qdss_gpio1,
> +                qdss_gpio10, qdss_gpio11, qdss_gpio12, qdss_gpio13, qdss_gpio14,
> +                qdss_gpio15, qdss_gpio2, qdss_gpio3, qdss_gpio4, qdss_gpio5,
> +                qdss_gpio6, qdss_gpio7, qdss_gpio8, qdss_gpio9, qlink0_enable,
> +                qlink0_request, qlink0_wmss, qlink1_enable, qlink1_request,
> +                qlink1_wmss, qlink2_enable, qlink2_request, qlink2_wmss,
> +                qup0_se0, qup0_se1, qup0_se2, qup0_se3, qup0_se4, qup0_se5,
> +                qup0_se6, qup0_se7, qup1_se0, qup1_se1, qup1_se2, qup1_se3,
> +                qup1_se4, qup1_se5, qup1_se6, sd_write, tb_trig, tgu_ch0,
> +                tgu_ch1, tgu_ch2, tgu_ch3, tmess_prng0, tmess_prng1,
> +                tmess_prng2, tmess_prng3, tsense_pwm1, tsense_pwm2, uim0_clk,
> +                uim0_data, uim0_present, uim0_reset, uim1_clk, uim1_data,
> +                uim1_present, uim1_reset, usb0_hs, usb0_phy, vfr_0, vfr_1,
> +                vsense_trigger ]

You consolidated the functions in the driver, but you forgot to update
this list accordingly.

> +
> +        required:
> +          - pins
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    tlmm: pinctrl@f100000 {
> +        compatible = "qcom,sm4450-tlmm";

Please feel free to ping me if you're having issues running
dt_binding_check and dtb_check. I believe the prior should complain
about this compatible being unknown.

Regards,
Bjorn

> +        reg = <0x0f100000 0x300000>;
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +        gpio-ranges = <&tlmm 0 0 137>;
> +        interrupt-controller;
> +        #interrupt-cells = <2>;
> +        interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
> +
> +        gpio-wo-state {
> +            pins = "gpio1";
> +            function = "gpio";
> +        };
> +
> +        uart-w-state {
> +            rx-pins {
> +                pins = "gpio23";
> +                function = "qup1_se2";
> +                bias-pull-up;
> +            };
> +
> +            tx-pins {
> +                pins = "gpio22";
> +                function = "qup1_se2";
> +                bias-disable;
> +            };
> +        };
> +    };
> +...
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
From: Mark Hasemeyer @ 2023-12-23  3:09 UTC (permalink / raw)
  To: kernel test robot
  Cc: LKML, oe-kbuild-all, AngeloGioacchino Del Regno,
	Krzysztof Kozlowski, Tzung-Bi Shih, Raul Rangel, Konrad Dybcio,
	Andy Shevchenko, Rob Herring, Sudeep Holla, Bartosz Golaszewski,
	Len Brown, Linus Walleij, Mika Westerberg, Rafael J. Wysocki,
	Wolfram Sang, linux-acpi, linux-gpio, linux-i2c
In-Reply-To: <202312230907.szXqJyXq-lkp@intel.com>

On Fri, Dec 22, 2023 at 7:09 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Mark,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on robh/for-next]
> [also build test WARNING on chrome-platform/for-next chrome-platform/for-firmware-next wsa/i2c/for-next driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.7-rc6 next-20231222]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Mark-Hasemeyer/gpiolib-acpi-Modify-acpi_dev_irq_wake_get_by-to-use-resource/20231222-172104
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
> patch link:    https://lore.kernel.org/r/20231220165423.v2.1.Ifd0903f1c351e84376d71dbdadbd43931197f5ea%40changeid
> patch subject: [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
> config: x86_64-randconfig-161-20231222 (https://download.01.org/0day-ci/archive/20231223/202312230907.szXqJyXq-lkp@intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231223/202312230907.szXqJyXq-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202312230907.szXqJyXq-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/gpio/gpiolib-acpi.c:117: warning: Function parameter or member 'shareable' not described in 'acpi_gpio_info'
>
>
> vim +117 drivers/gpio/gpiolib-acpi.c
>
> aa92b6f689acf1 Mika Westerberg 2014-03-10   93
> b7452d670fdef8 Dmitry Torokhov 2022-11-15   94  /**
> b7452d670fdef8 Dmitry Torokhov 2022-11-15   95   * struct acpi_gpio_info - ACPI GPIO specific information
> b7452d670fdef8 Dmitry Torokhov 2022-11-15   96   * @adev: reference to ACPI device which consumes GPIO resource
> b7452d670fdef8 Dmitry Torokhov 2022-11-15   97   * @flags: GPIO initialization flags
> b7452d670fdef8 Dmitry Torokhov 2022-11-15   98   * @gpioint: if %true this GPIO is of type GpioInt otherwise type is GpioIo
> b7452d670fdef8 Dmitry Torokhov 2022-11-15   99   * @pin_config: pin bias as provided by ACPI
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  100   * @polarity: interrupt polarity as provided by ACPI
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  101   * @triggering: triggering type as provided by ACPI
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  102   * @wake_capable: wake capability as provided by ACPI
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  103   * @debounce: debounce timeout as provided by ACPI
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  104   * @quirks: Linux specific quirks as provided by struct acpi_gpio_mapping
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  105   */
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  106  struct acpi_gpio_info {
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  107          struct acpi_device *adev;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  108          enum gpiod_flags flags;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  109          bool gpioint;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  110          int pin_config;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  111          int polarity;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  112          int triggering;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  113          bool wake_capable;
> 189f4620fa2d51 Mark Hasemeyer  2023-12-20  114          bool shareable;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  115          unsigned int debounce;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  116          unsigned int quirks;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 @117  };
> b7452d670fdef8 Dmitry Torokhov 2022-11-15  118
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

Ack. Missing documentation for acpi_gpio_info.shareable member. Will
add in next version.

^ permalink raw reply

* Re: [PATCH v6 05/40] pinctrl: add a Cirrus ep93xx SoC pin controller
From: Nikita Shubin @ 2023-12-23  8:55 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Linus Walleij, linux-kernel, linux-gpio, Arnd Bergmann
In-Reply-To: <ZXnvHYjgnc3VsXnX@smile.fi.intel.com>

Hello Andy!

On Wed, 2023-12-13 at 19:51 +0200, Andy Shevchenko wrote:
> On Tue, Dec 12, 2023 at 11:20:22AM +0300, Nikita Shubin wrote:
> > Add a pin control (only multiplexing) driver for ep93xx SoC so
> > we can fully convert ep93xx to device tree.
> > 
> > This driver is capable of muxing ep9301/ep9302/ep9307/ep9312/ep9315
> > variants, this is chosen based on "compatible" in device tree.
> 
> Mostly nit-picks below, with the exception to setting device node.
> See below.
> 
> ...
> 
> > +/*
> > + * There are several system configuration options selectable by
> > the DeviceCfg and SysCfg
> > + * registers. These registers provide the selection of several pin
> > multiplexing options and also
> > + * provide software access to the system reset configuration
> > options. Please refer to the
> > + * descriptions of the registers, “DeviceCfg” on page 5-25 and
> > “SysCfg” on page 5-34, for a
> > + * detailed explanation.
> > + */
> > +#define EP93XX_SYSCON_DEVCFG_D1ONG     BIT(30) /* not used */
> > +#define EP93XX_SYSCON_DEVCFG_D0ONG     BIT(29) /* not used */
> > +#define EP93XX_SYSCON_DEVCFG_IONU2     BIT(28) /* not used */
> > +#define EP93XX_SYSCON_DEVCFG_GONK      BIT(27) /* done */
> > +#define EP93XX_SYSCON_DEVCFG_TONG      BIT(26) /* not used */
> > +#define EP93XX_SYSCON_DEVCFG_MONG      BIT(25) /* not used */
> > +#define EP93XX_SYSCON_DEVCFG_A2ONG     BIT(22) /* not used */
> > +#define EP93XX_SYSCON_DEVCFG_A1ONG     BIT(21) /* not used */
> > +#define EP93XX_SYSCON_DEVCFG_HONIDE    BIT(11) /* done */
> > +#define EP93XX_SYSCON_DEVCFG_GONIDE    BIT(10) /* done */
> > +#define EP93XX_SYSCON_DEVCFG_PONG      BIT(9) /* done */
> > +#define EP93XX_SYSCON_DEVCFG_EONIDE    BIT(8) /* done */
> > +#define EP93XX_SYSCON_DEVCFG_I2SONSSP  BIT(7) /* done */
> > +#define EP93XX_SYSCON_DEVCFG_I2SONAC97 BIT(6) /* done */
> > +#define EP93XX_SYSCON_DEVCFG_RASONP3   BIT(4) /* done */
> 
> What are these comments supposed to mean?
> 
> ...
> 
> > +static const struct pinctrl_ops ep93xx_pctrl_ops = {
> > +       .get_groups_count = ep93xx_get_groups_count,
> > +       .get_group_name = ep93xx_get_group_name,
> > +       .get_group_pins = ep93xx_get_group_pins,
> 
> > +       .dt_node_to_map = pinconf_generic_dt_node_to_map_all,
> > +       .dt_free_map = pinconf_generic_dt_free_map,
> 
> Hmm... Don you need to ifdef these fields?

From now on we can't live without CONFIG_OF, so i don't think it's
necessary.

> 
> > +};
> 
> ...
> 
> > +static const struct pinfunction ep93xx_pmx_functions[] = {
> > +       PINCTRL_PINFUNCTION("spi", spigrps, ARRAY_SIZE(spigrps)),
> 
> Is array_size.h being included?
> 
> > +       PINCTRL_PINFUNCTION("ac97", ac97grps,
> > ARRAY_SIZE(ac97grps)),
> > +       PINCTRL_PINFUNCTION("i2s", i2sgrps, ARRAY_SIZE(i2sgrps)),
> > +       PINCTRL_PINFUNCTION("pwm", pwm1grps, ARRAY_SIZE(pwm1grps)),
> > +       PINCTRL_PINFUNCTION("keypad", keypadgrps,
> > ARRAY_SIZE(keypadgrps)),
> > +       PINCTRL_PINFUNCTION("pata", idegrps, ARRAY_SIZE(idegrps)),
> > +       PINCTRL_PINFUNCTION("lcd", rastergrps,
> > ARRAY_SIZE(rastergrps)),
> > +       PINCTRL_PINFUNCTION("gpio", gpiogrps,
> > ARRAY_SIZE(gpiogrps)),
> > +};
> 
> ...
> 
> > +       switch (pmx->model) {
> > +       case EP93XX_9301_PINCTRL:
> > +               grp = &ep9301_pin_groups[group];
> > +               break;
> > +       case EP93XX_9307_PINCTRL:
> > +               grp = &ep9307_pin_groups[group];
> > +               break;
> > +       case EP93XX_9312_PINCTRL:
> > +               grp = &ep9312_pin_groups[group];
> > +               break;
> 
> default?
> 
> > +       }
> 
> ...
> 
> > +       pmx->model = (int)(uintptr_t)id->driver_data;
> 
> Is the model defined as int (signed)?
> 
> Otherwise can we use proper type?
> 
> ...
> 
> > +       /* using parent of_node to match in
> > get_pinctrl_dev_from_of_node() */
> > +       device_set_of_node_from_dev(dev, adev->dev.parent);
> 
> Hmm... This takes references in comparison to device_set_node(). Is
> it intended?

Nope, switched to "device_set_node(dev, dev_fwnode(adev-dev.parent));".

> 
> ...
> 
> > +       pmx->pctl = devm_pinctrl_register(dev, &ep93xx_pmx_desc,
> > pmx);
> > +       if (IS_ERR(pmx->pctl))
> > +               return dev_err_probe(dev, PTR_ERR(pmx->pctl),
> > "could not register pinmux driver\n");
> 
> It can be written as
> 
>         pmx->pctl = devm_pinctrl_register(dev, &ep93xx_pmx_desc,
> pmx);
>         ret = PTR_ERR_OR_ZERO(...);
>         if (ret)
>                 return dev_err_probe(dev, ret, "could not register
> pinmux driver\n");
> 
> (makes line shorter). But it's up to you.
> 


^ permalink raw reply

* Re: [PATCH v6 00/40] ep93xx device tree conversion
From: Nikita Shubin @ 2023-12-23  9:12 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Hartley Sweeten, Alexander Sverdlin, Russell King,
	Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
	Michael Turquette, Stephen Boyd, Sebastian Reichel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Wim Van Sebroeck,
	Guenter Roeck, Thierry Reding, Uwe Kleine-König, Mark Brown,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Damien Le Moal, Sergey Shtylyov, Dmitry Torokhov, Liam Girdwood,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-kernel,
	linux-gpio, linux-clk, linux-pm, devicetree, dmaengine,
	linux-watchdog, linux-pwm, linux-spi, netdev, linux-mtd,
	linux-ide, linux-input, linux-sound, Arnd Bergmann,
	Bartosz Golaszewski, Krzysztof Kozlowski, Andrew Lunn
In-Reply-To: <ZXnxBtqbneUMbvwq@smile.fi.intel.com>

Hello Andy!

On Wed, 2023-12-13 at 19:59 +0200, Andy Shevchenko wrote:
> On Tue, Dec 12, 2023 at 11:20:17AM +0300, Nikita Shubin wrote:
> > No major changes since last version all changes are cometic.
> > 
> > Following patches require attention from Stephen Boyd, as they were
> > converted to aux_dev as suggested:
> > 
> > - ARM: ep93xx: add regmap aux_dev
> > - clk: ep93xx: add DT support for Cirrus EP93xx
> > 
> > DMA related patches still require Acked or Reviewed tags.
> > 
> > got approval LGTM from Miquel:
> > - mtd: rawnand: add support for ts72xx
> > Link: https://lore.kernel.org/lkml/20231004103911.2aa65354@xps-13/
> > 
> > new patches:
> > 
> > ARM: ep93xx:  Add terminator to gpiod_lookup_table
> >   - fixed terminator in gpiod_lockup_table
> > 
> > So mostly all patches got approval.
> > 
> > Patches should be now formated with '--histogram'
> 
> It _feels_ like some tags might be missing.
> In any case I suggest to use `b4` tool to retrieve tags when
> preparing
> the next version:
> 
>         git checkout -b vXX v6.7-rcX
>         b4 am -slt $MSG_ID_OF_v(XX-1)
>         git am ...
>         git rebase --interactive ... # to address comments
> 

I moved to b4 a few iterations ago:

```
Calculating patch-ids from commits, this may take a moment...
Checking change-id "20230605-ep93xx-01c76317e2d2"
Grabbing search results from lore.kernel.org
Grabbing thread from lore.kernel.org/all/20230424123522.18302-1-
nikita.shubin%40maquefel.me/t.mbox.gz
---
NOTE: some trailers ignored due to from/email mismatches:
    ! Trailer: Tested-by: Michael Peters <mpeters@embeddedTS.com>
     Msg From: Kris Bahnsen <kris@embeddedTS.com>
NOTE: Rerun with -S to apply them anyway
No trailer updates found.
```

I haven't found any missing tags, that b4 didn't apply, the ones above
refer to a very old iteration and were given to cover letter and i
don't feel like they need to be included.

^ permalink raw reply

* Re: [PATCH v4 2/2] Add GPIO support for Realtek DHC(Digital Home Center) RTD SoCs.
From: Krzysztof Kozlowski @ 2023-12-23 14:19 UTC (permalink / raw)
  To: Tzuyi Chang, Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-gpio, devicetree, linux-kernel
In-Reply-To: <20231222075812.6540-3-tychang@realtek.com>

On 22/12/2023 08:58, Tzuyi Chang wrote:
> This driver enables configuration of GPIO direction, GPIO values, GPIO
> debounce settings and handles GPIO interrupts.
> 
> Signed-off-by: Tzuyi Chang <tychang@realtek.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---


...

> +static int rtd_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +	struct rtd_gpio *data = gpiochip_get_data(gc);
> +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
> +	u32 mask = BIT(hwirq % 32);
> +	unsigned long flags;
> +	int dp_reg_offset;
> +	bool polarity;
> +	u32 val;
> +
> +	dp_reg_offset = rtd_gpio_dp_offset(data, hwirq);
> +
> +	switch (type & IRQ_TYPE_SENSE_MASK) {
> +	case IRQ_TYPE_EDGE_RISING:
> +		polarity = 1;
> +		break;
> +
> +	case IRQ_TYPE_EDGE_FALLING:
> +		polarity = 0;
> +		break;
> +
> +	case IRQ_TYPE_EDGE_BOTH:
> +		polarity = 1;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	raw_spin_lock_irqsave(&data->lock, flags);

Why are you using raw spinlock? This question applies to entire driver.

> +
> +	val = readl_relaxed(data->base + dp_reg_offset);
> +	if (polarity)
> +		val |= mask;
> +	else
> +		val &= ~mask;
> +	writel_relaxed(val, data->base + dp_reg_offset);
> +
> +	raw_spin_unlock_irqrestore(&data->lock, flags);
> +
> +	return 0;
> +}

...

}
> +
> +module_init(rtd_gpio_init);
> +
> +static void __exit rtd_gpio_exit(void)
> +{
> +	platform_driver_unregister(&rtd_gpio_platform_driver);
> +}
> +module_exit(rtd_gpio_exit);

Why not using module_platform_driver?

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v4 1/2] dt-bindings: gpio: realtek: Add realtek,rtd-gpio
From: Krzysztof Kozlowski @ 2023-12-23 14:19 UTC (permalink / raw)
  To: Tzuyi Chang, Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-gpio, devicetree, linux-kernel
In-Reply-To: <20231222075812.6540-2-tychang@realtek.com>

On 22/12/2023 08:58, Tzuyi Chang wrote:
> Add the device tree bindings for the Realtek DHC(Digital Home Center)
> RTD SoCs GPIO controllers.
> 
> Signed-off-by: Tzuyi Chang <tychang@realtek.com>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply

* [PATCH] pinctrl: samsung: constify iomem pointers
From: Krzysztof Kozlowski @ 2023-12-23 19:19 UTC (permalink / raw)
  To: Tomasz Figa, Krzysztof Kozlowski, Sylwester Nawrocki, Alim Akhtar,
	Linus Walleij, linux-arm-kernel, linux-samsung-soc, linux-gpio,
	linux-kernel

Constify few pointers to iomem, where the destination memory is not
modified, for code safety and readability.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/pinctrl/samsung/pinctrl-exynos.c  | 4 ++--
 drivers/pinctrl/samsung/pinctrl-samsung.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c
index d3d4b5d036c8..871c1eb46ddf 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos.c
+++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
@@ -693,7 +693,7 @@ static void exynos_pinctrl_suspend_bank(
 				struct samsung_pin_bank *bank)
 {
 	struct exynos_eint_gpio_save *save = bank->soc_priv;
-	void __iomem *regs = bank->eint_base;
+	const void __iomem *regs = bank->eint_base;
 
 	save->eint_con = readl(regs + EXYNOS_GPIO_ECON_OFFSET
 						+ bank->eint_offset);
@@ -714,7 +714,7 @@ static void exynosauto_pinctrl_suspend_bank(struct samsung_pinctrl_drv_data *drv
 					    struct samsung_pin_bank *bank)
 {
 	struct exynos_eint_gpio_save *save = bank->soc_priv;
-	void __iomem *regs = bank->eint_base;
+	const void __iomem *regs = bank->eint_base;
 
 	save->eint_con = readl(regs + bank->pctl_offset + bank->eint_con_offset);
 	save->eint_mask = readl(regs + bank->pctl_offset + bank->eint_mask_offset);
diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
index dbf38767f15f..ed07e23e0912 100644
--- a/drivers/pinctrl/samsung/pinctrl-samsung.c
+++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
@@ -565,7 +565,7 @@ static void samsung_gpio_set(struct gpio_chip *gc, unsigned offset, int value)
 /* gpiolib gpio_get callback function */
 static int samsung_gpio_get(struct gpio_chip *gc, unsigned offset)
 {
-	void __iomem *reg;
+	const void __iomem *reg;
 	u32 data;
 	struct samsung_pin_bank *bank = gpiochip_get_data(gc);
 	const struct samsung_pin_bank_type *type = bank->type;
@@ -1204,7 +1204,7 @@ static int __maybe_unused samsung_pinctrl_suspend(struct device *dev)
 
 	for (i = 0; i < drvdata->nr_banks; i++) {
 		struct samsung_pin_bank *bank = &drvdata->pin_banks[i];
-		void __iomem *reg = bank->pctl_base + bank->pctl_offset;
+		const void __iomem *reg = bank->pctl_base + bank->pctl_offset;
 		const u8 *offs = bank->type->reg_offset;
 		const u8 *widths = bank->type->fld_width;
 		enum pincfg_type type;
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH] pinctrl: samsung: constify iomem pointers
From: Sam Protsenko @ 2023-12-24  2:00 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Tomasz Figa, Sylwester Nawrocki, Alim Akhtar, Linus Walleij,
	linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel
In-Reply-To: <20231223191902.22857-1-krzysztof.kozlowski@linaro.org>

On Sat, Dec 23, 2023 at 1:19 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> Constify few pointers to iomem, where the destination memory is not
> modified, for code safety and readability.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

>  drivers/pinctrl/samsung/pinctrl-exynos.c  | 4 ++--
>  drivers/pinctrl/samsung/pinctrl-samsung.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c
> index d3d4b5d036c8..871c1eb46ddf 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos.c
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
> @@ -693,7 +693,7 @@ static void exynos_pinctrl_suspend_bank(
>                                 struct samsung_pin_bank *bank)
>  {
>         struct exynos_eint_gpio_save *save = bank->soc_priv;
> -       void __iomem *regs = bank->eint_base;
> +       const void __iomem *regs = bank->eint_base;
>
>         save->eint_con = readl(regs + EXYNOS_GPIO_ECON_OFFSET
>                                                 + bank->eint_offset);
> @@ -714,7 +714,7 @@ static void exynosauto_pinctrl_suspend_bank(struct samsung_pinctrl_drv_data *drv
>                                             struct samsung_pin_bank *bank)
>  {
>         struct exynos_eint_gpio_save *save = bank->soc_priv;
> -       void __iomem *regs = bank->eint_base;
> +       const void __iomem *regs = bank->eint_base;
>
>         save->eint_con = readl(regs + bank->pctl_offset + bank->eint_con_offset);
>         save->eint_mask = readl(regs + bank->pctl_offset + bank->eint_mask_offset);
> diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
> index dbf38767f15f..ed07e23e0912 100644
> --- a/drivers/pinctrl/samsung/pinctrl-samsung.c
> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
> @@ -565,7 +565,7 @@ static void samsung_gpio_set(struct gpio_chip *gc, unsigned offset, int value)
>  /* gpiolib gpio_get callback function */
>  static int samsung_gpio_get(struct gpio_chip *gc, unsigned offset)
>  {
> -       void __iomem *reg;
> +       const void __iomem *reg;
>         u32 data;
>         struct samsung_pin_bank *bank = gpiochip_get_data(gc);
>         const struct samsung_pin_bank_type *type = bank->type;
> @@ -1204,7 +1204,7 @@ static int __maybe_unused samsung_pinctrl_suspend(struct device *dev)
>
>         for (i = 0; i < drvdata->nr_banks; i++) {
>                 struct samsung_pin_bank *bank = &drvdata->pin_banks[i];
> -               void __iomem *reg = bank->pctl_base + bank->pctl_offset;
> +               const void __iomem *reg = bank->pctl_base + bank->pctl_offset;
>                 const u8 *offs = bank->type->reg_offset;
>                 const u8 *widths = bank->type->fld_width;
>                 enum pincfg_type type;
> --
> 2.34.1
>
>

^ permalink raw reply

* Re: [PATCH v2] pinctrl: Add lock to ensure the state atomization
From: Aiqun Yu (Maria) @ 2023-12-25  6:13 UTC (permalink / raw)
  To: Linus Walleij; +Cc: andersson, kernel, linux-gpio, linux-kernel, linux-arm-msm
In-Reply-To: <CACRpkdb6dkw58GwkqYXTDAQtdLazOLyp1CEjnkxDX2v=TDvvMw@mail.gmail.com>



On 12/20/2023 7:02 PM, Linus Walleij wrote:
> Hi Maria,
> 
> On Tue, Dec 12, 2023 at 10:06 AM Maria Yu <quic_aiquny@quicinc.com> wrote:
> 
>> Currently pinctrl_select_state is an export symbol and don't have
>> effective re-entrance protect design. During async probing of devices
>> it's possible to end up in pinctrl_select_state() from multiple
>> contexts simultaneously, so make it thread safe.
>> More over, when the real racy happened, the system frequently have
>> printk message like:
>>    "not freeing pin xx (xxx) as part of deactivating group xxx - it is
>> already used for some other setting".
>> Finally the system crashed after the flood log.
>> Add per pinctrl lock to ensure the old state and new state transition
>> atomization.
>> Also move dev error print message outside the region with interrupts
>> disabled.
>>
>> Fixes: 4198a9b57106 ("pinctrl: avoid reload of p state in list iteration")
>> Signed-off-by: Maria Yu <quic_aiquny@quicinc.com>
> 
> Overall this looks good!
> 
>> @@ -1262,9 +1263,12 @@ static void pinctrl_link_add(struct pinctrl_dev *pctldev,
>>   static int pinctrl_commit_state(struct pinctrl *p, struct pinctrl_state *state)
>>   {
>>          struct pinctrl_setting *setting, *setting2;
>> -       struct pinctrl_state *old_state = READ_ONCE(p->state);
>> +       struct pinctrl_state *old_state;
>>          int ret;
>> +       unsigned long flags;
>>
>> +       spin_lock_irqsave(&p->lock, flags);
> (...)
>> +       spin_unlock_irqrestore(&p->lock, flags);
> (...)
>> +       spin_unlock_irqrestore(&p->lock, flags);
> 
> Is it possible to use a scoped guard for pinctrl_commit_state()?
Good idea.
I will address this in next patchset.
> 
> #include <linux/cleanup.h>
> guard(spinlock_irqsave)(&p->lock);
> 
> It saves some code (and no need for flags) and avoid possible
> bugs when people add new errorpaths to the code.
> 
> Yours,
> Linus Walleij

-- 
Thx and BRs,
Aiqun(Maria) Yu

^ permalink raw reply

* Re: [PATCH v6 1/2] dt-bindings: pinctrl: qcom: Add SM4450 pinctrl
From: Tengfei Fan @ 2023-12-25  6:50 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: agross, konrad.dybcio, linus.walleij, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, linux-kernel, linux-arm-msm,
	linux-gpio, devicetree
In-Reply-To: <suy4rrpplvhsg2pfblg5amkvy7mrrkzsd3vzizhhpfj5xlog3q@i2n5wvsxb5wa>



On 12/23/2023 11:06 AM, Bjorn Andersson wrote:
> On Wed, Dec 06, 2023 at 10:08:39AM +0800, Tengfei Fan wrote:
>> Add device tree binding Documentation details for Qualcomm SM4450
>> TLMM device.
>>
>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>> ---
>>   .../bindings/pinctrl/qcom,sm4450-tlmm.yaml    | 151 ++++++++++++++++++
>>   1 file changed, 151 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml
>> new file mode 100644
>> index 000000000000..bb08ca5a1509
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml
>> @@ -0,0 +1,151 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/pinctrl/qcom,sm4450-tlmm.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Technologies, Inc. SM4450 TLMM block
>> +
>> +maintainers:
>> +  - Tengfei Fan <quic_tengfan@quicinc.com>
>> +
>> +description:
>> +  Top Level Mode Multiplexer pin controller in Qualcomm SM4450 SoC.
>> +
>> +allOf:
>> +  - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,sm4450-pinctrl
> 
> This should be qcom,sm4450-tlmm

Sure, I will fix it correctly.

> 
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts: true
>> +  interrupt-controller: true
>> +  "#interrupt-cells": true
>> +  gpio-controller: true
>> +
>> +  gpio-reserved-ranges:
>> +    minItems: 1
>> +    maxItems: 68
>> +
>> +  gpio-line-names:
>> +    maxItems: 136
>> +
>> +  "#gpio-cells": true
>> +  gpio-ranges: true
>> +  wakeup-parent: true
>> +
>> +patternProperties:
>> +  "-state$":
>> +    oneOf:
>> +      - $ref: "#/$defs/qcom-sm4450-tlmm-state"
>> +      - patternProperties:
>> +          "-pins$":
>> +            $ref: "#/$defs/qcom-sm4450-tlmm-state"
>> +        additionalProperties: false
>> +
>> +$defs:
>> +  qcom-sm4450-tlmm-state:
>> +    type: object
>> +    description:
>> +      Pinctrl node's client devices use subnodes for desired pin configuration.
>> +      Client device subnodes use below standard properties.
>> +    $ref: qcom,tlmm-common.yaml#/$defs/qcom-tlmm-state
>> +    unevaluatedProperties: false
>> +
>> +    properties:
>> +      pins:
>> +        description:
>> +          List of gpio pins affected by the properties specified in this
>> +          subnode.
>> +        items:
>> +          oneOf:
>> +            - pattern: "^gpio([0-9]|[1-9][0-9]|1[0-2][0-9]|13[0-5])$"
>> +            - enum: [ sdc2_clk, sdc2_cmd, sdc2_data, ufs_reset ]
>> +          minItems: 1
>> +          maxItems: 36
>> +
>> +      function:
>> +        description:
>> +          Specify the alternative function to be configured for the specified
>> +          pins.
>> +        enum: [ gpio, atest_char, atest_char0, atest_char1, atest_char2,
>> +                atest_char3, atest_usb0, atest_usb00, atest_usb01, atest_usb02,
>> +                atest_usb03, audio_ref, cam_mclk, cci_async, cci_i2c,
>> +                cci_timer0, cci_timer1, cci_timer2, cci_timer3, cci_timer4,
>> +                cmu_rng0, cmu_rng1, cmu_rng2, cmu_rng3, coex_uart1, cri_trng,
>> +                cri_trng0, cri_trng1, dbg_out, ddr_bist, ddr_pxi0, ddr_pxi1,
>> +                dp0_hot, gcc_gp1, gcc_gp2, gcc_gp3, host2wlan_sol, ibi_i3c,
>> +                jitter_bist, mdp_vsync, mdp_vsync0, mdp_vsync1, mdp_vsync2,
>> +                mdp_vsync3, mi2s0_data0, mi2s0_data1, mi2s0_sck, mi2s0_ws,
>> +                mi2s2_data0, mi2s2_data1, mi2s2_sck, mi2s2_ws, mi2s_mclk0,
>> +                mi2s_mclk1, nav_gpio0, nav_gpio1, nav_gpio2, pcie0_clk,
>> +                phase_flag0, phase_flag1, phase_flag10, phase_flag11,
>> +                phase_flag12, phase_flag13, phase_flag14, phase_flag15,
>> +                phase_flag16, phase_flag17, phase_flag18, phase_flag19,
>> +                phase_flag2, phase_flag20, phase_flag21, phase_flag22,
>> +                phase_flag23, phase_flag24, phase_flag25, phase_flag26,
>> +                phase_flag27, phase_flag28, phase_flag29, phase_flag3,
>> +                phase_flag30, phase_flag31, phase_flag4, phase_flag5,
>> +                phase_flag6, phase_flag7, phase_flag8, phase_flag9,
>> +                pll_bist, pll_clk, prng_rosc0, prng_rosc1, prng_rosc2,
>> +                prng_rosc3, qdss_cti, qdss_gpio, qdss_gpio0, qdss_gpio1,
>> +                qdss_gpio10, qdss_gpio11, qdss_gpio12, qdss_gpio13, qdss_gpio14,
>> +                qdss_gpio15, qdss_gpio2, qdss_gpio3, qdss_gpio4, qdss_gpio5,
>> +                qdss_gpio6, qdss_gpio7, qdss_gpio8, qdss_gpio9, qlink0_enable,
>> +                qlink0_request, qlink0_wmss, qlink1_enable, qlink1_request,
>> +                qlink1_wmss, qlink2_enable, qlink2_request, qlink2_wmss,
>> +                qup0_se0, qup0_se1, qup0_se2, qup0_se3, qup0_se4, qup0_se5,
>> +                qup0_se6, qup0_se7, qup1_se0, qup1_se1, qup1_se2, qup1_se3,
>> +                qup1_se4, qup1_se5, qup1_se6, sd_write, tb_trig, tgu_ch0,
>> +                tgu_ch1, tgu_ch2, tgu_ch3, tmess_prng0, tmess_prng1,
>> +                tmess_prng2, tmess_prng3, tsense_pwm1, tsense_pwm2, uim0_clk,
>> +                uim0_data, uim0_present, uim0_reset, uim1_clk, uim1_data,
>> +                uim1_present, uim1_reset, usb0_hs, usb0_phy, vfr_0, vfr_1,
>> +                vsense_trigger ]
> 
> You consolidated the functions in the driver, but you forgot to update
> this list accordingly.

Yes, I forgot update this list, I will update it in the future.

> 
>> +
>> +        required:
>> +          - pins
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    tlmm: pinctrl@f100000 {
>> +        compatible = "qcom,sm4450-tlmm";
> 
> Please feel free to ping me if you're having issues running
> dt_binding_check and dtb_check. I believe the prior should complain
> about this compatible being unknown.

There isn't any complain about this compatible when I run 
dt_binding_check and dtb_check.

Here is the commands which I used:

1. make ARCH=arm64  dtbs_check W=1

2. make ARCH=arm64  CROSS_COMPILE=aarch64-linux-gnu- DT_CHECKER_FLAGS=-m 
dt_binding_check 
DT_SCHEMA_FILES=Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml 


> 
> Regards,
> Bjorn
> 
>> +        reg = <0x0f100000 0x300000>;
>> +        gpio-controller;
>> +        #gpio-cells = <2>;
>> +        gpio-ranges = <&tlmm 0 0 137>;
>> +        interrupt-controller;
>> +        #interrupt-cells = <2>;
>> +        interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +        gpio-wo-state {
>> +            pins = "gpio1";
>> +            function = "gpio";
>> +        };
>> +
>> +        uart-w-state {
>> +            rx-pins {
>> +                pins = "gpio23";
>> +                function = "qup1_se2";
>> +                bias-pull-up;
>> +            };
>> +
>> +            tx-pins {
>> +                pins = "gpio22";
>> +                function = "qup1_se2";
>> +                bias-disable;
>> +            };
>> +        };
>> +    };
>> +...
>> -- 
>> 2.17.1
>>

-- 
Thx and BRs,
Tengfei Fan

^ permalink raw reply

* [PATCH v3] pinctrl: Add lock to ensure the state atomization
From: Maria Yu @ 2023-12-25  8:23 UTC (permalink / raw)
  To: andersson, linus.walleij
  Cc: Maria Yu, kernel, linux-gpio, linux-kernel, linux-arm-msm

Currently pinctrl_select_state is an export symbol and don't have
effective re-entrance protect design. During async probing of devices
it's possible to end up in pinctrl_select_state() from multiple
contexts simultaneously, so make it thread safe.
More over, when the real racy happened, the system frequently have
printk message like:
  "not freeing pin xx (xxx) as part of deactivating group xxx - it is
already used for some other setting".
Finally the system crashed after the flood log.
Add per pinctrl lock to ensure the old state and new state transition
atomization.
Also move dev error print message outside the region with interrupts
disabled.
Use scoped guard to simplify the lock protection needed code.

Fixes: 4198a9b57106 ("pinctrl: avoid reload of p state in list iteration")
Signed-off-by: Maria Yu <quic_aiquny@quicinc.com>
---
 drivers/pinctrl/core.c | 144 +++++++++++++++++++++--------------------
 drivers/pinctrl/core.h |   2 +
 2 files changed, 76 insertions(+), 70 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index f2977eb65522..ab158b745486 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -13,6 +13,7 @@
 #define pr_fmt(fmt) "pinctrl core: " fmt
 
 #include <linux/array_size.h>
+#include <linux/cleanup.h>
 #include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -1066,6 +1067,7 @@ static struct pinctrl *create_pinctrl(struct device *dev,
 	p->dev = dev;
 	INIT_LIST_HEAD(&p->states);
 	INIT_LIST_HEAD(&p->dt_maps);
+	spin_lock_init(&p->lock);
 
 	ret = pinctrl_dt_to_map(p, pctldev);
 	if (ret < 0) {
@@ -1262,93 +1264,95 @@ static void pinctrl_link_add(struct pinctrl_dev *pctldev,
 static int pinctrl_commit_state(struct pinctrl *p, struct pinctrl_state *state)
 {
 	struct pinctrl_setting *setting, *setting2;
-	struct pinctrl_state *old_state = READ_ONCE(p->state);
+	struct pinctrl_state *old_state;
 	int ret;
 
-	if (old_state) {
-		/*
-		 * For each pinmux setting in the old state, forget SW's record
-		 * of mux owner for that pingroup. Any pingroups which are
-		 * still owned by the new state will be re-acquired by the call
-		 * to pinmux_enable_setting() in the loop below.
-		 */
-		list_for_each_entry(setting, &old_state->settings, node) {
-			if (setting->type != PIN_MAP_TYPE_MUX_GROUP)
-				continue;
-			pinmux_disable_setting(setting);
+	scoped_guard(spinlock_irqsave, &p->lock) {
+		old_state = p->state;
+		if (old_state) {
+			/*
+			 * For each pinmux setting in the old state, forget SW's record
+			 * of mux owner for that pingroup. Any pingroups which are
+			 * still owned by the new state will be re-acquired by the call
+			 * to pinmux_enable_setting() in the loop below.
+			 */
+			list_for_each_entry(setting, &old_state->settings, node) {
+				if (setting->type != PIN_MAP_TYPE_MUX_GROUP)
+					continue;
+				pinmux_disable_setting(setting);
+			}
 		}
-	}
-
-	p->state = NULL;
 
-	/* Apply all the settings for the new state - pinmux first */
-	list_for_each_entry(setting, &state->settings, node) {
-		switch (setting->type) {
-		case PIN_MAP_TYPE_MUX_GROUP:
-			ret = pinmux_enable_setting(setting);
-			break;
-		case PIN_MAP_TYPE_CONFIGS_PIN:
-		case PIN_MAP_TYPE_CONFIGS_GROUP:
-			ret = 0;
-			break;
-		default:
-			ret = -EINVAL;
-			break;
-		}
+		p->state = NULL;
 
-		if (ret < 0)
-			goto unapply_new_state;
+		/* Apply all the settings for the new state - pinmux first */
+		list_for_each_entry(setting, &state->settings, node) {
+			switch (setting->type) {
+			case PIN_MAP_TYPE_MUX_GROUP:
+				ret = pinmux_enable_setting(setting);
+				break;
+			case PIN_MAP_TYPE_CONFIGS_PIN:
+			case PIN_MAP_TYPE_CONFIGS_GROUP:
+				ret = 0;
+				break;
+			default:
+				ret = -EINVAL;
+				break;
+			}
 
-		/* Do not link hogs (circular dependency) */
-		if (p != setting->pctldev->p)
-			pinctrl_link_add(setting->pctldev, p->dev);
-	}
+			if (ret < 0)
+				goto unapply_new_state;
 
-	/* Apply all the settings for the new state - pinconf after */
-	list_for_each_entry(setting, &state->settings, node) {
-		switch (setting->type) {
-		case PIN_MAP_TYPE_MUX_GROUP:
-			ret = 0;
-			break;
-		case PIN_MAP_TYPE_CONFIGS_PIN:
-		case PIN_MAP_TYPE_CONFIGS_GROUP:
-			ret = pinconf_apply_setting(setting);
-			break;
-		default:
-			ret = -EINVAL;
-			break;
+			/* Do not link hogs (circular dependency) */
+			if (p != setting->pctldev->p)
+				pinctrl_link_add(setting->pctldev, p->dev);
 		}
 
-		if (ret < 0) {
-			goto unapply_new_state;
-		}
+		/* Apply all the settings for the new state - pinconf after */
+		list_for_each_entry(setting, &state->settings, node) {
+			switch (setting->type) {
+			case PIN_MAP_TYPE_MUX_GROUP:
+				ret = 0;
+				break;
+			case PIN_MAP_TYPE_CONFIGS_PIN:
+			case PIN_MAP_TYPE_CONFIGS_GROUP:
+				ret = pinconf_apply_setting(setting);
+				break;
+			default:
+				ret = -EINVAL;
+				break;
+			}
 
-		/* Do not link hogs (circular dependency) */
-		if (p != setting->pctldev->p)
-			pinctrl_link_add(setting->pctldev, p->dev);
-	}
+			if (ret < 0)
+				goto unapply_new_state;
 
-	p->state = state;
+			/* Do not link hogs (circular dependency) */
+			if (p != setting->pctldev->p)
+				pinctrl_link_add(setting->pctldev, p->dev);
+		}
 
-	return 0;
+		p->state = state;
+
+		return 0;
 
 unapply_new_state:
-	dev_err(p->dev, "Error applying setting, reverse things back\n");
 
-	list_for_each_entry(setting2, &state->settings, node) {
-		if (&setting2->node == &setting->node)
-			break;
-		/*
-		 * All we can do here is pinmux_disable_setting.
-		 * That means that some pins are muxed differently now
-		 * than they were before applying the setting (We can't
-		 * "unmux a pin"!), but it's not a big deal since the pins
-		 * are free to be muxed by another apply_setting.
-		 */
-		if (setting2->type == PIN_MAP_TYPE_MUX_GROUP)
-			pinmux_disable_setting(setting2);
+		list_for_each_entry(setting2, &state->settings, node) {
+			if (&setting2->node == &setting->node)
+				break;
+			/*
+			 * All we can do here is pinmux_disable_setting.
+			 * That means that some pins are muxed differently now
+			 * than they were before applying the setting (We can't
+			 * "unmux a pin"!), but it's not a big deal since the pins
+			 * are free to be muxed by another apply_setting.
+			 */
+			if (setting2->type == PIN_MAP_TYPE_MUX_GROUP)
+				pinmux_disable_setting(setting2);
+		}
 	}
 
+	dev_err(p->dev, "Error applying setting, reverse things back\n");
 	/* There's no infinite recursive loop here because p->state is NULL */
 	if (old_state)
 		pinctrl_select_state(p, old_state);
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 530370443c19..86fc41393f7b 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -12,6 +12,7 @@
 #include <linux/list.h>
 #include <linux/mutex.h>
 #include <linux/radix-tree.h>
+#include <linux/spinlock.h>
 #include <linux/types.h>
 
 #include <linux/pinctrl/machine.h>
@@ -91,6 +92,7 @@ struct pinctrl {
 	struct pinctrl_state *state;
 	struct list_head dt_maps;
 	struct kref users;
+	spinlock_t lock;
 };
 
 /**

base-commit: 861deac3b092f37b2c5e6871732f3e11486f7082
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v6 1/2] dt-bindings: pinctrl: qcom: Add SM4450 pinctrl
From: Krzysztof Kozlowski @ 2023-12-25  9:37 UTC (permalink / raw)
  To: Tengfei Fan, Bjorn Andersson
  Cc: agross, konrad.dybcio, linus.walleij, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, linux-kernel, linux-arm-msm,
	linux-gpio, devicetree
In-Reply-To: <8ea2e841-eb9d-47a8-b1ca-3597403a1bc6@quicinc.com>

On 25/12/2023 07:50, Tengfei Fan wrote:
>>> +
>>> +        required:
>>> +          - pins
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +    tlmm: pinctrl@f100000 {
>>> +        compatible = "qcom,sm4450-tlmm";
>>
>> Please feel free to ping me if you're having issues running
>> dt_binding_check and dtb_check. I believe the prior should complain
>> about this compatible being unknown.
> 
> There isn't any complain about this compatible when I run 
> dt_binding_check and dtb_check.
> 
> Here is the commands which I used:
> 
> 1. make ARCH=arm64  dtbs_check W=1
> 
> 2. make ARCH=arm64  CROSS_COMPILE=aarch64-linux-gnu- DT_CHECKER_FLAGS=-m 
> dt_binding_check 
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml 

Something got broken around -m flags for dtschema, so indeed no reports
are printed for unmatched compatibles.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v4 2/2] Add GPIO support for Realtek DHC(Digital Home Center) RTD SoCs.
From: Andy Shevchenko @ 2023-12-25 19:52 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Tzuyi Chang, Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-gpio,
	devicetree, linux-kernel
In-Reply-To: <2a44fe91-b4a5-4842-8abc-f30c532f14e0@linaro.org>

On Sat, Dec 23, 2023 at 4:19 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 22/12/2023 08:58, Tzuyi Chang wrote:

...

> > +     raw_spin_lock_irqsave(&data->lock, flags);
>
> Why are you using raw spinlock? This question applies to entire driver.

If you want to have your IRQ chip to work in the RT kernel, you need a
real spinlock.

> > +     raw_spin_unlock_irqrestore(&data->lock, flags);


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v6 00/40] ep93xx device tree conversion
From: Andy Shevchenko @ 2023-12-25 19:55 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Andy Shevchenko, Hartley Sweeten, Alexander Sverdlin,
	Russell King, Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
	Michael Turquette, Stephen Boyd, Sebastian Reichel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Wim Van Sebroeck,
	Guenter Roeck, Thierry Reding, Uwe Kleine-König, Mark Brown,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Damien Le Moal, Sergey Shtylyov, Dmitry Torokhov, Liam Girdwood,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-kernel,
	linux-gpio, linux-clk, linux-pm, devicetree, dmaengine,
	linux-watchdog, linux-pwm, linux-spi, netdev, linux-mtd,
	linux-ide, linux-input, linux-sound, Arnd Bergmann,
	Bartosz Golaszewski, Krzysztof Kozlowski, Andrew Lunn
In-Reply-To: <d6e898200b96e816ea8c8c9a847307088ec5821c.camel@maquefel.me>

On Sat, Dec 23, 2023 at 11:13 AM Nikita Shubin
<nikita.shubin@maquefel.me> wrote:
> On Wed, 2023-12-13 at 19:59 +0200, Andy Shevchenko wrote:

...

> I haven't found any missing tags, that b4 didn't apply, the ones above
> refer to a very old iteration and were given to cover letter and i
> don't feel like they need to be included.

When somebody gives you a tag against a cover letter, it means the
entire series (if not spelled differently). `b4` even has a parameter
-t for that IIRC.


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* [PATCH AUTOSEL 6.6 06/39] pinctrl: amd: Mask non-wake source pins with interrupt enabled at suspend
From: Sasha Levin @ 2023-12-26  0:18 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Mario Limonciello, Marcus Aram, Mark Herbert, Linus Walleij,
	Sasha Levin, Basavaraj.Natikar, Shyam-sundar.S-k, linux-gpio
In-Reply-To: <20231226002021.4776-1-sashal@kernel.org>

From: Mario Limonciello <mario.limonciello@amd.com>

[ Upstream commit 2fff0b5e1a6b9c577b4dd4958902c877159c856b ]

If a pin isn't marked as a wake source processing any interrupts is
just going to destroy battery life.  The APU may wake up from a hardware
sleep state to process the interrupt but not return control to the OS.

Mask interrupt for all non-wake source pins at suspend. They'll be
re-enabled at resume.

Reported-and-tested-by: Marcus Aram <marcus+oss@oxar.nl>
Reported-and-tested-by: Mark Herbert <mark.herbert42@gmail.com>
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2812
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Link: https://lore.kernel.org/r/20231203032431.30277-3-mario.limonciello@amd.com
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/pinctrl/pinctrl-amd.c | 9 +++++++++
 drivers/pinctrl/pinctrl-amd.h | 5 +++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
index 74241b2ff21e3..86034c457c043 100644
--- a/drivers/pinctrl/pinctrl-amd.c
+++ b/drivers/pinctrl/pinctrl-amd.c
@@ -923,6 +923,15 @@ static int amd_gpio_suspend(struct device *dev)
 
 		raw_spin_lock_irqsave(&gpio_dev->lock, flags);
 		gpio_dev->saved_regs[i] = readl(gpio_dev->base + pin * 4) & ~PIN_IRQ_PENDING;
+
+		/* mask any interrupts not intended to be a wake source */
+		if (!(gpio_dev->saved_regs[i] & WAKE_SOURCE)) {
+			writel(gpio_dev->saved_regs[i] & ~BIT(INTERRUPT_MASK_OFF),
+			       gpio_dev->base + pin * 4);
+			pm_pr_dbg("Disabling GPIO #%d interrupt for suspend.\n",
+				  pin);
+		}
+
 		raw_spin_unlock_irqrestore(&gpio_dev->lock, flags);
 	}
 
diff --git a/drivers/pinctrl/pinctrl-amd.h b/drivers/pinctrl/pinctrl-amd.h
index 34c5c3e71fb26..cf59089f27763 100644
--- a/drivers/pinctrl/pinctrl-amd.h
+++ b/drivers/pinctrl/pinctrl-amd.h
@@ -80,6 +80,11 @@
 #define FUNCTION_MASK		GENMASK(1, 0)
 #define FUNCTION_INVALID	GENMASK(7, 0)
 
+#define WAKE_SOURCE	(BIT(WAKE_CNTRL_OFF_S0I3) | \
+			 BIT(WAKE_CNTRL_OFF_S3)   | \
+			 BIT(WAKE_CNTRL_OFF_S4)   | \
+			 BIT(WAKECNTRL_Z_OFF))
+
 struct amd_function {
 	const char *name;
 	const char * const groups[NSELECTS];
-- 
2.43.0


^ permalink raw reply related

* [PATCH AUTOSEL 6.6 35/39] pinctrl: cy8c95x0: Fix typo
From: Sasha Levin @ 2023-12-26  0:19 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Patrick Rudolph, Linus Walleij, Sasha Levin, linux-gpio
In-Reply-To: <20231226002021.4776-1-sashal@kernel.org>

From: Patrick Rudolph <patrick.rudolph@9elements.com>

[ Upstream commit 47b1fa48116238208c1b1198dba10f56fc1b6eb2 ]

Fix typo to make pinctrl-cy8c95x compile again.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Link: https://lore.kernel.org/r/20231219125120.4028862-1-patrick.rudolph@9elements.com
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/pinctrl/pinctrl-cy8c95x0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 58ca6fac7849a..993b30ebc6841 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -821,7 +821,7 @@ static int cy8c95x0_setup_gpiochip(struct cy8c95x0_pinctrl *chip)
 	gc->get_direction = cy8c95x0_gpio_get_direction;
 	gc->get_multiple = cy8c95x0_gpio_get_multiple;
 	gc->set_multiple = cy8c95x0_gpio_set_multiple;
-	gc->set_config = gpiochip_generic_config,
+	gc->set_config = gpiochip_generic_config;
 	gc->can_sleep = true;
 	gc->add_pin_ranges = cy8c95x0_add_pin_ranges;
 
-- 
2.43.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox