2020-10-20 02:55:54

by Daniel Palmer

[permalink] [raw]
Subject: [PATCH v2 0/5] Add GPIO support for MStar/SigmaStar ARMv7

At the moment the MStar/SigmaStar support is only really
capable of shell from an initramfs and not much else.

Most of the interesting drivers are blocked on clock and pinctrl
drivers and those are going to take me a little while to get cleaned
up.

Clock and pinctrl aren't needed for basic GPIO to work (all pins
start off as GPIOs..) and it makes it possible to actually do something
so this series adds everything that is needed for the main GPIO
block in these chips.

Changes since v1:

- Moves the binding header commit before the yaml commit

- Fixes the license on the binding header to include BSD-2-Clause

- The driver has been reworked to use the gpiolib irqchip functionality
as suggested by Linus[0]. I think I got this right. The gpio controller
doesn't actually do anything with interrupts itself.. It just happens
to have 4 lines that are also wired to lines on one of the interrupt
controllers.

- Now that the driver is an interrupt controller in it's own right for
the gpio lines that have associated interrupts the binding description
has been updated to add the interrupt-controller bits and remove the
description of the interrupt-names that described how the interrupts
used to be passed in.

Daniel Palmer (5):
dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver
dt-bindings: gpio: Binding for MStar MSC313 GPIO controller
gpio: msc313: MStar MSC313 GPIO driver
ARM: mstar: Add gpio controller to MStar base dtsi
ARM: mstar: Fill in GPIO controller properties for infinity

.../bindings/gpio/mstar,msc313-gpio.yaml | 61 +++
MAINTAINERS | 3 +
arch/arm/boot/dts/mstar-infinity.dtsi | 7 +
arch/arm/boot/dts/mstar-v7.dtsi | 10 +
drivers/gpio/Kconfig | 9 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-msc313.c | 406 ++++++++++++++++++
include/dt-bindings/gpio/msc313-gpio.h | 95 ++++
8 files changed, 592 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
create mode 100644 drivers/gpio/gpio-msc313.c
create mode 100644 include/dt-bindings/gpio/msc313-gpio.h

--
2.28.0


2020-10-20 02:57:04

by Daniel Palmer

[permalink] [raw]
Subject: [PATCH v2 1/5] dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver

Header adds defines for the gpio number of each pin
from the driver view. The gpio block seems to support 128 lines
but what line is mapped to a physical pin depends on the chip.
The driver itself uses the index of a pin's offset in an array
of the possible offsets for a chip as the gpio number.

The defines remove the need to work out that index to consume
a pin in the device tree.

Signed-off-by: Daniel Palmer <[email protected]>
---
MAINTAINERS | 1 +
include/dt-bindings/gpio/msc313-gpio.h | 95 ++++++++++++++++++++++++++
2 files changed, 96 insertions(+)
create mode 100644 include/dt-bindings/gpio/msc313-gpio.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 3f345f36c22c..a188fae8c04e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2132,6 +2132,7 @@ W: http://linux-chenxing.org/
F: Documentation/devicetree/bindings/arm/mstar/*
F: arch/arm/boot/dts/mstar-*
F: arch/arm/mach-mstar/
+F: include/dt-bindings/gpio/msc313-gpio.h

ARM/NEC MOBILEPRO 900/c MACHINE SUPPORT
M: Michael Petchkovsky <[email protected]>
diff --git a/include/dt-bindings/gpio/msc313-gpio.h b/include/dt-bindings/gpio/msc313-gpio.h
new file mode 100644
index 000000000000..9b8cd6ffb7c4
--- /dev/null
+++ b/include/dt-bindings/gpio/msc313-gpio.h
@@ -0,0 +1,95 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * GPIO definitions for MStar/SigmaStar MSC313 and later SoCs
+ *
+ * Copyright (C) 2020 Daniel Palmer <[email protected]>
+ */
+
+#ifndef _DT_BINDINGS_MSC313_GPIO_H
+#define _DT_BINDINGS_MSC313_GPIO_H
+
+/* pin names for fuart, same for all SoCs so far */
+#define MSC313_PINNAME_FUART_RX "fuart_rx"
+#define MSC313_PINNAME_FUART_TX "fuart_tx"
+#define MSC313_PINNAME_FUART_CTS "fuart_cts"
+#define MSC313_PINNAME_FUART_RTS "fuart_rts"
+
+/* pin names for sr, mercury5 is different */
+#define MSC313_PINNAME_SR_IO2 "sr_io2"
+#define MSC313_PINNAME_SR_IO3 "sr_io3"
+#define MSC313_PINNAME_SR_IO4 "sr_io4"
+#define MSC313_PINNAME_SR_IO5 "sr_io5"
+#define MSC313_PINNAME_SR_IO6 "sr_io6"
+#define MSC313_PINNAME_SR_IO7 "sr_io7"
+#define MSC313_PINNAME_SR_IO8 "sr_io8"
+#define MSC313_PINNAME_SR_IO9 "sr_io9"
+#define MSC313_PINNAME_SR_IO10 "sr_io10"
+#define MSC313_PINNAME_SR_IO11 "sr_io11"
+#define MSC313_PINNAME_SR_IO12 "sr_io12"
+#define MSC313_PINNAME_SR_IO13 "sr_io13"
+#define MSC313_PINNAME_SR_IO14 "sr_io14"
+#define MSC313_PINNAME_SR_IO15 "sr_io15"
+#define MSC313_PINNAME_SR_IO16 "sr_io16"
+#define MSC313_PINNAME_SR_IO17 "sr_io17"
+
+/* pin names for sd, same for all SoCs so far */
+#define MSC313_PINNAME_SD_CLK "sd_clk"
+#define MSC313_PINNAME_SD_CMD "sd_cmd"
+#define MSC313_PINNAME_SD_D0 "sd_d0"
+#define MSC313_PINNAME_SD_D1 "sd_d1"
+#define MSC313_PINNAME_SD_D2 "sd_d2"
+#define MSC313_PINNAME_SD_D3 "sd_d3"
+
+/* pin names for i2c1, same for all SoCs so for */
+#define MSC313_PINNAME_I2C1_SCL "i2c1_scl"
+#define MSC313_PINNAME_I2C1_SCA "i2c1_sda"
+
+/* pin names for spi0, same for all SoCs so far */
+#define MSC313_PINNAME_SPI0_CZ "spi0_cz"
+#define MSC313_PINNAME_SPI0_CK "spi0_ck"
+#define MSC313_PINNAME_SPI0_DI "spi0_di"
+#define MSC313_PINNAME_SPI0_DO "spi0_do"
+
+#define MSC313_GPIO_FUART 0
+#define MSC313_GPIO_FUART_RX (MSC313_GPIO_FUART + 0)
+#define MSC313_GPIO_FUART_TX (MSC313_GPIO_FUART + 1)
+#define MSC313_GPIO_FUART_CTS (MSC313_GPIO_FUART + 2)
+#define MSC313_GPIO_FUART_RTS (MSC313_GPIO_FUART + 3)
+
+#define MSC313_GPIO_SR (MSC313_GPIO_FUART_RTS + 1)
+#define MSC313_GPIO_SR_IO2 (MSC313_GPIO_SR + 0)
+#define MSC313_GPIO_SR_IO3 (MSC313_GPIO_SR + 1)
+#define MSC313_GPIO_SR_IO4 (MSC313_GPIO_SR + 2)
+#define MSC313_GPIO_SR_IO5 (MSC313_GPIO_SR + 3)
+#define MSC313_GPIO_SR_IO6 (MSC313_GPIO_SR + 4)
+#define MSC313_GPIO_SR_IO7 (MSC313_GPIO_SR + 5)
+#define MSC313_GPIO_SR_IO8 (MSC313_GPIO_SR + 6)
+#define MSC313_GPIO_SR_IO9 (MSC313_GPIO_SR + 7)
+#define MSC313_GPIO_SR_IO10 (MSC313_GPIO_SR + 8)
+#define MSC313_GPIO_SR_IO11 (MSC313_GPIO_SR + 9)
+#define MSC313_GPIO_SR_IO12 (MSC313_GPIO_SR + 10)
+#define MSC313_GPIO_SR_IO13 (MSC313_GPIO_SR + 11)
+#define MSC313_GPIO_SR_IO14 (MSC313_GPIO_SR + 12)
+#define MSC313_GPIO_SR_IO15 (MSC313_GPIO_SR + 13)
+#define MSC313_GPIO_SR_IO16 (MSC313_GPIO_SR + 14)
+#define MSC313_GPIO_SR_IO17 (MSC313_GPIO_SR + 15)
+
+#define MSC313_GPIO_SD (MSC313_GPIO_SR_IO17 + 1)
+#define MSC313_GPIO_SD_CLK (MSC313_GPIO_SD + 0)
+#define MSC313_GPIO_SD_CMD (MSC313_GPIO_SD + 1)
+#define MSC313_GPIO_SD_D0 (MSC313_GPIO_SD + 2)
+#define MSC313_GPIO_SD_D1 (MSC313_GPIO_SD + 3)
+#define MSC313_GPIO_SD_D2 (MSC313_GPIO_SD + 4)
+#define MSC313_GPIO_SD_D3 (MSC313_GPIO_SD + 5)
+
+#define MSC313_GPIO_I2C1 (MSC313_GPIO_SD_D3 + 1)
+#define MSC313_GPIO_I2C1_SCL (MSC313_GPIO_I2C1 + 0)
+#define MSC313_GPIO_I2C1_SDA (MSC313_GPIO_I2C1 + 1)
+
+#define MSC313_GPIO_SPI0 (MSC313_GPIO_I2C1_SDA + 1)
+#define MSC313_GPIO_SPI0_CZ (MSC313_GPIO_SPI0 + 0)
+#define MSC313_GPIO_SPI0_CK (MSC313_GPIO_SPI0 + 1)
+#define MSC313_GPIO_SPI0_DI (MSC313_GPIO_SPI0 + 2)
+#define MSC313_GPIO_SPI0_DO (MSC313_GPIO_SPI0 + 3)
+
+#endif /* _DT_BINDINGS_MSC313_GPIO_H */
--
2.28.0

2020-10-26 14:20:05

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver

On Mon, Oct 19, 2020 at 11:10:04PM +0900, Daniel Palmer wrote:
> Header adds defines for the gpio number of each pin
> from the driver view. The gpio block seems to support 128 lines
> but what line is mapped to a physical pin depends on the chip.
> The driver itself uses the index of a pin's offset in an array
> of the possible offsets for a chip as the gpio number.
>
> The defines remove the need to work out that index to consume
> a pin in the device tree.

I'd expect the DT to have 0-127 numbering... If you need to map that to
another number, then an array property in DT could handle that.

>
> Signed-off-by: Daniel Palmer <[email protected]>
> ---
> MAINTAINERS | 1 +
> include/dt-bindings/gpio/msc313-gpio.h | 95 ++++++++++++++++++++++++++
> 2 files changed, 96 insertions(+)
> create mode 100644 include/dt-bindings/gpio/msc313-gpio.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3f345f36c22c..a188fae8c04e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2132,6 +2132,7 @@ W: http://linux-chenxing.org/
> F: Documentation/devicetree/bindings/arm/mstar/*
> F: arch/arm/boot/dts/mstar-*
> F: arch/arm/mach-mstar/
> +F: include/dt-bindings/gpio/msc313-gpio.h
>
> ARM/NEC MOBILEPRO 900/c MACHINE SUPPORT
> M: Michael Petchkovsky <[email protected]>
> diff --git a/include/dt-bindings/gpio/msc313-gpio.h b/include/dt-bindings/gpio/msc313-gpio.h
> new file mode 100644
> index 000000000000..9b8cd6ffb7c4
> --- /dev/null
> +++ b/include/dt-bindings/gpio/msc313-gpio.h
> @@ -0,0 +1,95 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * GPIO definitions for MStar/SigmaStar MSC313 and later SoCs
> + *
> + * Copyright (C) 2020 Daniel Palmer <[email protected]>
> + */
> +
> +#ifndef _DT_BINDINGS_MSC313_GPIO_H
> +#define _DT_BINDINGS_MSC313_GPIO_H
> +
> +/* pin names for fuart, same for all SoCs so far */
> +#define MSC313_PINNAME_FUART_RX "fuart_rx"
> +#define MSC313_PINNAME_FUART_TX "fuart_tx"
> +#define MSC313_PINNAME_FUART_CTS "fuart_cts"
> +#define MSC313_PINNAME_FUART_RTS "fuart_rts"

Defines for strings serves no purpose.

> +
> +/* pin names for sr, mercury5 is different */
> +#define MSC313_PINNAME_SR_IO2 "sr_io2"
> +#define MSC313_PINNAME_SR_IO3 "sr_io3"
> +#define MSC313_PINNAME_SR_IO4 "sr_io4"
> +#define MSC313_PINNAME_SR_IO5 "sr_io5"
> +#define MSC313_PINNAME_SR_IO6 "sr_io6"
> +#define MSC313_PINNAME_SR_IO7 "sr_io7"
> +#define MSC313_PINNAME_SR_IO8 "sr_io8"
> +#define MSC313_PINNAME_SR_IO9 "sr_io9"
> +#define MSC313_PINNAME_SR_IO10 "sr_io10"
> +#define MSC313_PINNAME_SR_IO11 "sr_io11"
> +#define MSC313_PINNAME_SR_IO12 "sr_io12"
> +#define MSC313_PINNAME_SR_IO13 "sr_io13"
> +#define MSC313_PINNAME_SR_IO14 "sr_io14"
> +#define MSC313_PINNAME_SR_IO15 "sr_io15"
> +#define MSC313_PINNAME_SR_IO16 "sr_io16"
> +#define MSC313_PINNAME_SR_IO17 "sr_io17"
> +
> +/* pin names for sd, same for all SoCs so far */
> +#define MSC313_PINNAME_SD_CLK "sd_clk"
> +#define MSC313_PINNAME_SD_CMD "sd_cmd"
> +#define MSC313_PINNAME_SD_D0 "sd_d0"
> +#define MSC313_PINNAME_SD_D1 "sd_d1"
> +#define MSC313_PINNAME_SD_D2 "sd_d2"
> +#define MSC313_PINNAME_SD_D3 "sd_d3"
> +
> +/* pin names for i2c1, same for all SoCs so for */
> +#define MSC313_PINNAME_I2C1_SCL "i2c1_scl"
> +#define MSC313_PINNAME_I2C1_SCA "i2c1_sda"
> +
> +/* pin names for spi0, same for all SoCs so far */
> +#define MSC313_PINNAME_SPI0_CZ "spi0_cz"
> +#define MSC313_PINNAME_SPI0_CK "spi0_ck"
> +#define MSC313_PINNAME_SPI0_DI "spi0_di"
> +#define MSC313_PINNAME_SPI0_DO "spi0_do"
> +
> +#define MSC313_GPIO_FUART 0
> +#define MSC313_GPIO_FUART_RX (MSC313_GPIO_FUART + 0)
> +#define MSC313_GPIO_FUART_TX (MSC313_GPIO_FUART + 1)
> +#define MSC313_GPIO_FUART_CTS (MSC313_GPIO_FUART + 2)
> +#define MSC313_GPIO_FUART_RTS (MSC313_GPIO_FUART + 3)

We don't normally have defines for GPIO numbers either.

> +
> +#define MSC313_GPIO_SR (MSC313_GPIO_FUART_RTS + 1)
> +#define MSC313_GPIO_SR_IO2 (MSC313_GPIO_SR + 0)
> +#define MSC313_GPIO_SR_IO3 (MSC313_GPIO_SR + 1)
> +#define MSC313_GPIO_SR_IO4 (MSC313_GPIO_SR + 2)
> +#define MSC313_GPIO_SR_IO5 (MSC313_GPIO_SR + 3)
> +#define MSC313_GPIO_SR_IO6 (MSC313_GPIO_SR + 4)
> +#define MSC313_GPIO_SR_IO7 (MSC313_GPIO_SR + 5)
> +#define MSC313_GPIO_SR_IO8 (MSC313_GPIO_SR + 6)
> +#define MSC313_GPIO_SR_IO9 (MSC313_GPIO_SR + 7)
> +#define MSC313_GPIO_SR_IO10 (MSC313_GPIO_SR + 8)
> +#define MSC313_GPIO_SR_IO11 (MSC313_GPIO_SR + 9)
> +#define MSC313_GPIO_SR_IO12 (MSC313_GPIO_SR + 10)
> +#define MSC313_GPIO_SR_IO13 (MSC313_GPIO_SR + 11)
> +#define MSC313_GPIO_SR_IO14 (MSC313_GPIO_SR + 12)
> +#define MSC313_GPIO_SR_IO15 (MSC313_GPIO_SR + 13)
> +#define MSC313_GPIO_SR_IO16 (MSC313_GPIO_SR + 14)
> +#define MSC313_GPIO_SR_IO17 (MSC313_GPIO_SR + 15)
> +
> +#define MSC313_GPIO_SD (MSC313_GPIO_SR_IO17 + 1)
> +#define MSC313_GPIO_SD_CLK (MSC313_GPIO_SD + 0)
> +#define MSC313_GPIO_SD_CMD (MSC313_GPIO_SD + 1)
> +#define MSC313_GPIO_SD_D0 (MSC313_GPIO_SD + 2)
> +#define MSC313_GPIO_SD_D1 (MSC313_GPIO_SD + 3)
> +#define MSC313_GPIO_SD_D2 (MSC313_GPIO_SD + 4)
> +#define MSC313_GPIO_SD_D3 (MSC313_GPIO_SD + 5)
> +
> +#define MSC313_GPIO_I2C1 (MSC313_GPIO_SD_D3 + 1)
> +#define MSC313_GPIO_I2C1_SCL (MSC313_GPIO_I2C1 + 0)
> +#define MSC313_GPIO_I2C1_SDA (MSC313_GPIO_I2C1 + 1)
> +
> +#define MSC313_GPIO_SPI0 (MSC313_GPIO_I2C1_SDA + 1)
> +#define MSC313_GPIO_SPI0_CZ (MSC313_GPIO_SPI0 + 0)
> +#define MSC313_GPIO_SPI0_CK (MSC313_GPIO_SPI0 + 1)
> +#define MSC313_GPIO_SPI0_DI (MSC313_GPIO_SPI0 + 2)
> +#define MSC313_GPIO_SPI0_DO (MSC313_GPIO_SPI0 + 3)
> +
> +#endif /* _DT_BINDINGS_MSC313_GPIO_H */
> --
> 2.28.0
>

2020-10-26 18:09:47

by Daniel Palmer

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver

Hi Rob,

On Mon, 26 Oct 2020 at 22:46, Rob Herring <[email protected]> wrote:
>
> On Mon, Oct 19, 2020 at 11:10:04PM +0900, Daniel Palmer wrote:
> > Header adds defines for the gpio number of each pin
> > from the driver view. The gpio block seems to support 128 lines
> > but what line is mapped to a physical pin depends on the chip.
> > The driver itself uses the index of a pin's offset in an array
> > of the possible offsets for a chip as the gpio number.
> >
> > The defines remove the need to work out that index to consume
> > a pin in the device tree.
>
> I'd expect the DT to have 0-127 numbering... If you need to map that to
> another number, then an array property in DT could handle that.
>

Thank you for the comments on this header and the binding description.

Thinking about this again I'm thinking about having the GPIO numbers
be 0-127 like you say but supplying the valid offsets for that
specific chip and the pad/pin names to make visible to the user via an
array/arrays that contains the pin register offsets and the pin names.
Basically my per-chip table moves out of the driver and into the DT.
Does that sound acceptable? The main thing I want to avoid is
presenting the user with 128 gpios when the actually chip only has <10
of them wired up.

Thanks,

Daniel