2021-03-04 11:08:58

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH v3 00/14] pinctrl: add BCM63XX pincontrol support

First of all, I've based this on the patches sent by Jonas Gorski back in
2016:
https://www.spinics.net/lists/linux-gpio/msg15983.html
http://patchwork.ozlabs.org/project/linux-gpio/patch/[email protected]/

I've tried to address all coments from Linus Walleij, but I know that
this may still need some other modifications

This patchset adds appropriate binding documentation and drivers for
pin controller cores found in the BCM63XX MIPS SoCs currently supported.

While the GPIO part is always the same, the pinmux part varies quite a
lot between different SoCs. Sometimes they have defined groups which
can be muxed into different functions, sometimes each function has a
different group. Sometimes you can mux individual pins. Often it is a
combination of single pins and groups.

Some core versions require the GPIO direction to be set according to the
function, most do not. Sometimes the mux register(s) contain bits for
unrelated other functions.

v3: introduce new files for shared code and add more changes suggested by
Linus Walleij. Also add a new patch needed for properly parsing gpio-ranges.
v2: introduce changes suggested by Linus Walleij and remove interrupts
- In order to use GPIO_REGMAP, the need to get gpio_chip from gpio_regmap
and use it for pinctrl_add_gpio_range() and gpio_chip.direction_input()
and gpio_chip.direction_output().

Álvaro Fernández Rojas (14):
gpio: regmap: set gpio_chip of_node
pinctrl: bcm: add bcm63xx base code
Documentation: add BCM6328 pincontroller binding documentation
pinctrl: add a pincontrol driver for BCM6328
Documentation: add BCM6358 pincontroller binding documentation
pinctrl: add a pincontrol driver for BCM6358
Documentation: add BCM6362 pincontroller binding documentation
pinctrl: add a pincontrol driver for BCM6362
Documentation: add BCM6368 pincontroller binding documentation
pinctrl: add a pincontrol driver for BCM6368
Documentation: add BCM63268 pincontroller binding documentation
pinctrl: add a pincontrol driver for BCM63268
Documentation: add BCM6318 pincontroller binding documentation
pinctrl: add a pincontrol driver for BCM6318

.../pinctrl/brcm,bcm6318-pinctrl.yaml | 187 +++++
.../pinctrl/brcm,bcm63268-pinctrl.yaml | 208 ++++++
.../pinctrl/brcm,bcm6328-pinctrl.yaml | 171 +++++
.../pinctrl/brcm,bcm6358-pinctrl.yaml | 137 ++++
.../pinctrl/brcm,bcm6362-pinctrl.yaml | 250 +++++++
.../pinctrl/brcm,bcm6368-pinctrl.yaml | 261 +++++++
drivers/gpio/gpio-regmap.c | 1 +
drivers/pinctrl/bcm/Kconfig | 57 ++
drivers/pinctrl/bcm/Makefile | 7 +
drivers/pinctrl/bcm/pinctrl-bcm6318.c | 496 ++++++++++++++
drivers/pinctrl/bcm/pinctrl-bcm63268.c | 643 ++++++++++++++++++
drivers/pinctrl/bcm/pinctrl-bcm6328.c | 403 +++++++++++
drivers/pinctrl/bcm/pinctrl-bcm6358.c | 369 ++++++++++
drivers/pinctrl/bcm/pinctrl-bcm6362.c | 617 +++++++++++++++++
drivers/pinctrl/bcm/pinctrl-bcm6368.c | 523 ++++++++++++++
drivers/pinctrl/bcm/pinctrl-bcm63xx.c | 113 +++
drivers/pinctrl/bcm/pinctrl-bcm63xx.h | 46 ++
include/linux/gpio/regmap.h | 3 +
18 files changed, 4492 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm6318-pinctrl.yaml
create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm63268-pinctrl.yaml
create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml
create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm6358-pinctrl.yaml
create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm6362-pinctrl.yaml
create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm6368-pinctrl.yaml
create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm6318.c
create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63268.c
create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm6328.c
create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm6358.c
create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm6362.c
create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm6368.c
create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63xx.c
create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63xx.h

--
2.20.1


2021-03-04 11:11:03

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH v3 01/14] gpio: regmap: set gpio_chip of_node

This is needed for properly registering gpio regmap as a child of a regmap
pin controller.

Signed-off-by: Álvaro Fernández Rojas <[email protected]>
---
v3: introduce patch needed for properly parsing gpio-ranges.

drivers/gpio/gpio-regmap.c | 1 +
include/linux/gpio/regmap.h | 3 +++
2 files changed, 4 insertions(+)

diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index 5412cb3b0b2a..752ccd780b7d 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -249,6 +249,7 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config

chip = &gpio->gpio_chip;
chip->parent = config->parent;
+ chip->of_node = config->of_node;
chip->base = -1;
chip->ngpio = config->ngpio;
chip->names = config->names;
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index ad76f3d0a6ba..f6e638e32d2a 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -4,6 +4,7 @@
#define _LINUX_GPIO_REGMAP_H

struct device;
+struct device_node;
struct gpio_regmap;
struct irq_domain;
struct regmap;
@@ -14,6 +15,7 @@ struct regmap;
/**
* struct gpio_regmap_config - Description of a generic regmap gpio_chip.
* @parent: The parent device
+ * @of_node: The device node
* @regmap: The regmap used to access the registers
* given, the name of the device is used
* @label: (Optional) Descriptive name for GPIO controller.
@@ -56,6 +58,7 @@ struct regmap;
*/
struct gpio_regmap_config {
struct device *parent;
+ struct device_node *of_node;
struct regmap *regmap;

const char *label;
--
2.20.1

2021-03-04 11:58:16

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v3 01/14] gpio: regmap: set gpio_chip of_node

Am 2021-03-03 16:27, schrieb Linus Walleij:
> On Wed, Mar 3, 2021 at 3:23 PM Álvaro Fernández Rojas
> <[email protected]> wrote:
>
>> This is needed for properly registering gpio regmap as a child of a
>> regmap
>> pin controller.
>>
>> Signed-off-by: Álvaro Fernández Rojas <[email protected]>
>> ---
>> v3: introduce patch needed for properly parsing gpio-ranges.
>
> Oops a little bug. I suggest that I merge this into the pinctrl tree
> together with the rest of the patches when we are done with review.

Ha, I've just debugged this because it puzzled me why it was working
for me.

I was about to suggesting using the following instead:
chip->of_node = config->of_node ?: dev_of_node(config->parent);

It turns out this is already done in of_gpio_dev_init():
https://elixir.bootlin.com/linux/v5.12-rc1/source/drivers/gpio/gpiolib-of.c#L1043

So config->of_node is still optional. But I'm not sure if we
should add the line above for clarity in gpio-regmap.c.

-michael

2021-03-04 12:07:50

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v3 01/14] gpio: regmap: set gpio_chip of_node

Am 2021-03-03 15:22, schrieb Álvaro Fernández Rojas:
> This is needed for properly registering gpio regmap as a child of a
> regmap
> pin controller.
>
> Signed-off-by: Álvaro Fernández Rojas <[email protected]>
> ---
> v3: introduce patch needed for properly parsing gpio-ranges.
>
> drivers/gpio/gpio-regmap.c | 1 +
> include/linux/gpio/regmap.h | 3 +++
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index 5412cb3b0b2a..752ccd780b7d 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
> @@ -249,6 +249,7 @@ struct gpio_regmap *gpio_regmap_register(const
> struct gpio_regmap_config *config
>
> chip = &gpio->gpio_chip;
> chip->parent = config->parent;
> + chip->of_node = config->of_node;

chip->of_node = config->of_node ?: dev_of_node(config->parent);

As mentioned in my previous reply in this thread, for clarity
reasons.

> chip->base = -1;
> chip->ngpio = config->ngpio;
> chip->names = config->names;
> diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
> index ad76f3d0a6ba..f6e638e32d2a 100644
> --- a/include/linux/gpio/regmap.h
> +++ b/include/linux/gpio/regmap.h
> @@ -4,6 +4,7 @@
> #define _LINUX_GPIO_REGMAP_H
>
> struct device;
> +struct device_node;
> struct gpio_regmap;
> struct irq_domain;
> struct regmap;
> @@ -14,6 +15,7 @@ struct regmap;
> /**
> * struct gpio_regmap_config - Description of a generic regmap
> gpio_chip.
> * @parent: The parent device
> + * @of_node: The device node

Please add "(Optional)" and move it below @regmap. This should also
mention that if not supplied parent->of_node is used.

> * @regmap: The regmap used to access the registers
> * given, the name of the device is used
> * @label: (Optional) Descriptive name for GPIO controller.
> @@ -56,6 +58,7 @@ struct regmap;
> */
> struct gpio_regmap_config {
> struct device *parent;
> + struct device_node *of_node;
> struct regmap *regmap;
>
> const char *label;

With these changes:
Reviewed-by: Michael Walle <[email protected]>

-michael

2021-03-04 12:08:58

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: Re: [PATCH v3 00/14] pinctrl: add BCM63XX pincontrol support

Hi Linus,

> El 3 mar 2021, a las 16:29, Linus Walleij <[email protected]> escribió:
>
> On Wed, Mar 3, 2021 at 3:23 PM Álvaro Fernández Rojas <[email protected]> wrote:
>
>> v3: introduce new files for shared code and add more changes suggested by
>> Linus Walleij. Also add a new patch needed for properly parsing gpio-ranges.
>
> This looks very appetizing, I am ready to merge this once we cut some
> slack for DT review (a week or two).
>
> I'd like to merge it soon so you can start working on the IRQ add-on.
>
> I'd probably drop the IRQ-related selects from Kconfig
> when applying though (no big deal, no need to resend over that).

About that, it seems that GPIO_REGMAP should select GPIOLIB_IRQCHIP, since I couldn’t build the kernel due to the following error when I removed the IRQ-related selects:
LD vmlinux.o
MODPOST vmlinux.symvers
MODINFO modules.builtin.modinfo
GEN modules.builtin
LD .tmp_vmlinux.kallsyms1
mips-linux-gnu-ld: drivers/gpio/gpio-regmap.o: in function `gpio_regmap_register':
gpio-regmap.c:(.text+0x704): undefined reference to `gpiochip_irqchip_add_domain'
make: *** [Makefile:1197: vmlinux] Error 1

Or maybe we could guard these lines of gpio-regmap.c with #ifdef GPIOLIB_IRQCHIP:
https://github.com/torvalds/linux/blob/f69d02e37a85645aa90d18cacfff36dba370f797/drivers/gpio/gpio-regmap.c#L282-L286

>
> Yours,
> Linus Walleij

Best regards,
Álvaro.

2021-03-04 13:50:19

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v3 00/14] pinctrl: add BCM63XX pincontrol support

On Wed, Mar 3, 2021 at 5:23 PM Álvaro Fernández Rojas <[email protected]> wrote:

> Or maybe we could guard these lines of gpio-regmap.c with #ifdef GPIOLIB_IRQCHIP:
> https://github.com/torvalds/linux/blob/f69d02e37a85645aa90d18cacfff36dba370f797/drivers/gpio/gpio-regmap.c#L282-L286

That's the best approach. I wasn't a big fan of this ability to insert
an external
irqdomain in the first place, so it should be as optional as possible.

Yours,
Linus Walleij

2021-03-04 15:29:31

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v3 01/14] gpio: regmap: set gpio_chip of_node

On Wed, Mar 3, 2021 at 5:12 PM Álvaro Fernández Rojas <[email protected]> wrote:

> Do you want me to send v4 with these changes?
> Or maybe just this single patch?

It's usually better to resend the series because then the b4 tool
will pick it all up properly.

Yours,
Linus Walleij

2021-03-04 15:34:00

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: Re: [PATCH v3 01/14] gpio: regmap: set gpio_chip of_node

Hi Linus,

> El 4 mar 2021, a las 9:13, Linus Walleij <[email protected]> escribió:
>
> On Wed, Mar 3, 2021 at 5:12 PM Álvaro Fernández Rojas <[email protected]> wrote:
>
>> Do you want me to send v4 with these changes?
>> Or maybe just this single patch?
>
> It's usually better to resend the series because then the b4 tool
> will pick it all up properly.

Ok, I didn’t know that :$.
Sorry for sending it as a separate patch.

>
> Yours,
> Linus Walleij

Best regards,
Álvaro.

2021-03-04 23:21:52

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH v3 05/14] Documentation: add BCM6358 pincontroller binding documentation

Add binding documentation for the pincontrol core found in BCM6358 SoCs.

Signed-off-by: Álvaro Fernández Rojas <[email protected]>
Signed-off-by: Jonas Gorski <[email protected]>
---
v3: add new gpio node
v2: remove interrupts

.../pinctrl/brcm,bcm6358-pinctrl.yaml | 137 ++++++++++++++++++
1 file changed, 137 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm6358-pinctrl.yaml

diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm6358-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6358-pinctrl.yaml
new file mode 100644
index 000000000000..eb14dd4cdaaa
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6358-pinctrl.yaml
@@ -0,0 +1,137 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/brcm,bcm6358-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM6358 pin controller
+
+maintainers:
+ - Álvaro Fernández Rojas <[email protected]>
+ - Jonas Gorski <[email protected]>
+
+description: |+
+ The pin controller node should be the child of a syscon node.
+
+ Refer to the the bindings described in
+ Documentation/devicetree/bindings/mfd/syscon.yaml
+
+properties:
+ compatible:
+ const: brcm,bcm6358-pinctrl
+
+patternProperties:
+ '^gpio$':
+ type: object
+ properties:
+ compatible:
+ const: brcm,bcm6358-gpio
+
+ data:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Offset in the register map for the data register (in bytes).
+
+ dirout:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Offset in the register map for the dirout register (in bytes).
+
+ gpio-controller: true
+
+ "#gpio-cells":
+ const: 2
+
+ gpio-ranges:
+ maxItems: 1
+
+ required:
+ - gpio-controller
+ - gpio-ranges
+ - '#gpio-cells'
+
+ '^.*$':
+ if:
+ type: object
+ then:
+ properties:
+ function:
+ $ref: "/schemas/types.yaml#/definitions/string"
+ enum: [ ebi_cs, uart1, serial_led, legacy_led, led, spi_cs, utopia,
+ pwm_syn_clk, sys_irq ]
+
+ pins:
+ $ref: "/schemas/types.yaml#/definitions/string"
+ enum: [ ebi_cs_grp, uart1_grp, serial_led_grp, legacy_led_grp,
+ led_grp, spi_cs_grp, utopia_grp, pwm_syn_clk, sys_irq_grp ]
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ gpio_cntl@fffe0080 {
+ compatible = "syscon", "simple-mfd";
+ reg = <0xfffe0080 0x80>;
+
+ pinctrl: pinctrl {
+ compatible = "brcm,bcm6358-pinctrl";
+
+ gpio {
+ compatible = "brcm,bcm6358-gpio";
+ data = <0xc>;
+ dirout = <0x4>;
+
+ gpio-controller;
+ gpio-ranges = <&pinctrl 0 0 40>;
+ #gpio-cells = <2>;
+ };
+
+ pinctrl_ebi_cs: ebi_cs {
+ function = "ebi_cs";
+ groups = "ebi_cs_grp";
+ };
+
+ pinctrl_uart1: uart1 {
+ function = "uart1";
+ groups = "uart1_grp";
+ };
+
+ pinctrl_serial_led: serial_led {
+ function = "serial_led";
+ groups = "serial_led_grp";
+ };
+
+ pinctrl_legacy_led: legacy_led {
+ function = "legacy_led";
+ groups = "legacy_led_grp";
+ };
+
+ pinctrl_led: led {
+ function = "led";
+ groups = "led_grp";
+ };
+
+ pinctrl_spi_cs_23: spi_cs {
+ function = "spi_cs";
+ groups = "spi_cs_grp";
+ };
+
+ pinctrl_utopia: utopia {
+ function = "utopia";
+ groups = "utopia_grp";
+ };
+
+ pinctrl_pwm_syn_clk: pwm_syn_clk {
+ function = "pwm_syn_clk";
+ groups = "pwm_syn_clk_grp";
+ };
+
+ pinctrl_sys_irq: sys_irq {
+ function = "sys_irq";
+ groups = "sys_irq_grp";
+ };
+ };
+ };
--
2.20.1

2021-03-04 23:23:57

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v3 00/14] pinctrl: add BCM63XX pincontrol support

On Wed, Mar 3, 2021 at 3:23 PM Álvaro Fernández Rojas <[email protected]> wrote:

> v3: introduce new files for shared code and add more changes suggested by
> Linus Walleij. Also add a new patch needed for properly parsing gpio-ranges.

This looks very appetizing, I am ready to merge this once we cut some
slack for DT review (a week or two).

I'd like to merge it soon so you can start working on the IRQ add-on.

I'd probably drop the IRQ-related selects from Kconfig
when applying though (no big deal, no need to resend over that).

Yours,
Linus Walleij

2021-03-04 23:24:16

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v3 01/14] gpio: regmap: set gpio_chip of_node

On Wed, Mar 3, 2021 at 3:23 PM Álvaro Fernández Rojas <[email protected]> wrote:

> This is needed for properly registering gpio regmap as a child of a regmap
> pin controller.
>
> Signed-off-by: Álvaro Fernández Rojas <[email protected]>
> ---
> v3: introduce patch needed for properly parsing gpio-ranges.

Oops a little bug. I suggest that I merge this into the pinctrl tree
together with the rest of the patches when we are done with review.

Yours.
Linus Walleij

2021-03-04 23:24:51

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: Re: [PATCH v3 01/14] gpio: regmap: set gpio_chip of_node

Hi Linus,

Do you want me to send v4 with these changes?
Or maybe just this single patch?

Best regards,
Álvaro.

> El 3 mar 2021, a las 17:08, Michael Walle <[email protected]> escribió:
>
> Am 2021-03-03 15:22, schrieb Álvaro Fernández Rojas:
>> This is needed for properly registering gpio regmap as a child of a regmap
>> pin controller.
>> Signed-off-by: Álvaro Fernández Rojas <[email protected]>
>> ---
>> v3: introduce patch needed for properly parsing gpio-ranges.
>> drivers/gpio/gpio-regmap.c | 1 +
>> include/linux/gpio/regmap.h | 3 +++
>> 2 files changed, 4 insertions(+)
>> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
>> index 5412cb3b0b2a..752ccd780b7d 100644
>> --- a/drivers/gpio/gpio-regmap.c
>> +++ b/drivers/gpio/gpio-regmap.c
>> @@ -249,6 +249,7 @@ struct gpio_regmap *gpio_regmap_register(const
>> struct gpio_regmap_config *config
>> chip = &gpio->gpio_chip;
>> chip->parent = config->parent;
>> + chip->of_node = config->of_node;
>
> chip->of_node = config->of_node ?: dev_of_node(config->parent);
>
> As mentioned in my previous reply in this thread, for clarity
> reasons.
>
>> chip->base = -1;
>> chip->ngpio = config->ngpio;
>> chip->names = config->names;
>> diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
>> index ad76f3d0a6ba..f6e638e32d2a 100644
>> --- a/include/linux/gpio/regmap.h
>> +++ b/include/linux/gpio/regmap.h
>> @@ -4,6 +4,7 @@
>> #define _LINUX_GPIO_REGMAP_H
>> struct device;
>> +struct device_node;
>> struct gpio_regmap;
>> struct irq_domain;
>> struct regmap;
>> @@ -14,6 +15,7 @@ struct regmap;
>> /**
>> * struct gpio_regmap_config - Description of a generic regmap gpio_chip.
>> * @parent: The parent device
>> + * @of_node: The device node
>
> Please add "(Optional)" and move it below @regmap. This should also
> mention that if not supplied parent->of_node is used.
>
>> * @regmap: The regmap used to access the registers
>> * given, the name of the device is used
>> * @label: (Optional) Descriptive name for GPIO controller.
>> @@ -56,6 +58,7 @@ struct regmap;
>> */
>> struct gpio_regmap_config {
>> struct device *parent;
>> + struct device_node *of_node;
>> struct regmap *regmap;
>> const char *label;
>
> With these changes:
> Reviewed-by: Michael Walle <[email protected]>
>
> -michael