All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Add GPIO support for MStar/SigmaStar ARMv7
@ 2020-10-11  2:48 ` Daniel Palmer
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-11  2:48 UTC (permalink / raw)
  To: linux-gpio; +Cc: devicetree, linux-arm-kernel, linux-kernel, Daniel Palmer

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.

Daniel Palmer (5):
  dt-bindings: gpio: Binding for MStar MSC313 GPIO controller
  dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver
  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      |  69 ++++
 MAINTAINERS                                   |   3 +
 arch/arm/boot/dts/mstar-infinity.dtsi         |  16 +
 arch/arm/boot/dts/mstar-v7.dtsi               |   7 +
 drivers/gpio/Kconfig                          |   9 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-msc313.c                    | 341 ++++++++++++++++++
 include/dt-bindings/gpio/msc313-gpio.h        |  95 +++++
 8 files changed, 541 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.27.0


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 0/5] Add GPIO support for MStar/SigmaStar ARMv7
@ 2020-10-11  2:48 ` Daniel Palmer
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-11  2:48 UTC (permalink / raw)
  To: linux-gpio; +Cc: devicetree, Daniel Palmer, linux-kernel, linux-arm-kernel

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.

Daniel Palmer (5):
  dt-bindings: gpio: Binding for MStar MSC313 GPIO controller
  dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver
  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      |  69 ++++
 MAINTAINERS                                   |   3 +
 arch/arm/boot/dts/mstar-infinity.dtsi         |  16 +
 arch/arm/boot/dts/mstar-v7.dtsi               |   7 +
 drivers/gpio/Kconfig                          |   9 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-msc313.c                    | 341 ++++++++++++++++++
 include/dt-bindings/gpio/msc313-gpio.h        |  95 +++++
 8 files changed, 541 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.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 1/5] dt-bindings: gpio: Binding for MStar MSC313 GPIO controller
  2020-10-11  2:48 ` Daniel Palmer
@ 2020-10-11  2:48   ` Daniel Palmer
  -1 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-11  2:48 UTC (permalink / raw)
  To: linux-gpio; +Cc: devicetree, linux-arm-kernel, linux-kernel, Daniel Palmer

Add a binding description for the MStar/SigmaStar GPIO controller
found in the MSC313 and later ARMv7 SoCs.

Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 .../bindings/gpio/mstar,msc313-gpio.yaml      | 69 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml

diff --git a/Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml b/Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
new file mode 100644
index 000000000000..07ef463273d2
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/mstar,msc313-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MStar/SigmaStar GPIO controller
+
+maintainers:
+  - Daniel Palmer <daniel@thingy.jp>
+
+properties:
+  $nodename:
+    pattern: "^gpio@[0-9a-f]+$"
+
+  compatible:
+    const: mstar,msc313-gpio
+
+  reg:
+    maxItems: 1
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-ranges: true
+
+  gpio-ranges-group-names:
+    $ref: /schemas/types.yaml#/definitions/string-array
+
+  interrupts: true
+
+  interrupt-names:
+    description: |
+      The interrupt name should match the pin that the interrupt
+      is connected to.
+
+required:
+  - compatible
+  - reg
+  - gpio-controller
+  - "#gpio-cells"
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/gpio/msc313-gpio.h>
+
+    gpio: gpio@207800 {
+      compatible = "mstar,msc313e-gpio";
+      #gpio-cells = <2>;
+      reg = <0x207800 0x200>;
+      gpio-controller;
+      gpio-ranges = <&pinctrl 0 36 22>,
+                    <&pinctrl 22 63 4>,
+                    <&pinctrl 26 68 6>;
+      interrupt-parent = <&intc_fiq>;
+      interrupt-names = MSC313_PINNAME_SPI0_CZ,
+                        MSC313_PINNAME_SPI0_CK,
+                        MSC313_PINNAME_SPI0_DI,
+                        MSC313_PINNAME_SPI0_DO;
+      interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+      status = "okay";
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 75b04ba10f21..4594b70f2e3a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2155,6 +2155,7 @@ L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 W:	http://linux-chenxing.org/
 F:	Documentation/devicetree/bindings/arm/mstar/*
+F:	Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
 F:	arch/arm/boot/dts/mstar-*
 F:	arch/arm/mach-mstar/
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 1/5] dt-bindings: gpio: Binding for MStar MSC313 GPIO controller
@ 2020-10-11  2:48   ` Daniel Palmer
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-11  2:48 UTC (permalink / raw)
  To: linux-gpio; +Cc: devicetree, Daniel Palmer, linux-kernel, linux-arm-kernel

Add a binding description for the MStar/SigmaStar GPIO controller
found in the MSC313 and later ARMv7 SoCs.

Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 .../bindings/gpio/mstar,msc313-gpio.yaml      | 69 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml

diff --git a/Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml b/Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
new file mode 100644
index 000000000000..07ef463273d2
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/mstar,msc313-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MStar/SigmaStar GPIO controller
+
+maintainers:
+  - Daniel Palmer <daniel@thingy.jp>
+
+properties:
+  $nodename:
+    pattern: "^gpio@[0-9a-f]+$"
+
+  compatible:
+    const: mstar,msc313-gpio
+
+  reg:
+    maxItems: 1
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-ranges: true
+
+  gpio-ranges-group-names:
+    $ref: /schemas/types.yaml#/definitions/string-array
+
+  interrupts: true
+
+  interrupt-names:
+    description: |
+      The interrupt name should match the pin that the interrupt
+      is connected to.
+
+required:
+  - compatible
+  - reg
+  - gpio-controller
+  - "#gpio-cells"
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/gpio/msc313-gpio.h>
+
+    gpio: gpio@207800 {
+      compatible = "mstar,msc313e-gpio";
+      #gpio-cells = <2>;
+      reg = <0x207800 0x200>;
+      gpio-controller;
+      gpio-ranges = <&pinctrl 0 36 22>,
+                    <&pinctrl 22 63 4>,
+                    <&pinctrl 26 68 6>;
+      interrupt-parent = <&intc_fiq>;
+      interrupt-names = MSC313_PINNAME_SPI0_CZ,
+                        MSC313_PINNAME_SPI0_CK,
+                        MSC313_PINNAME_SPI0_DI,
+                        MSC313_PINNAME_SPI0_DO;
+      interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+      status = "okay";
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 75b04ba10f21..4594b70f2e3a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2155,6 +2155,7 @@ L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 W:	http://linux-chenxing.org/
 F:	Documentation/devicetree/bindings/arm/mstar/*
+F:	Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
 F:	arch/arm/boot/dts/mstar-*
 F:	arch/arm/mach-mstar/
 
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 2/5] dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver
  2020-10-11  2:48 ` Daniel Palmer
@ 2020-10-11  2:48   ` Daniel Palmer
  -1 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-11  2:48 UTC (permalink / raw)
  To: linux-gpio; +Cc: devicetree, linux-arm-kernel, linux-kernel, Daniel Palmer

The driver uses the pin names to find the right interrupt for
a pin from the device tree so this header reduces the need to
have multiple copies of the same string all over the place.

This header also 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 <daniel@0x0f.com>
---
 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 4594b70f2e3a..ec5b49b9955f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2158,6 +2158,7 @@ F:	Documentation/devicetree/bindings/arm/mstar/*
 F:	Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
 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 <mkpetch@internode.on.net>
diff --git a/include/dt-bindings/gpio/msc313-gpio.h b/include/dt-bindings/gpio/msc313-gpio.h
new file mode 100644
index 000000000000..655fe03de519
--- /dev/null
+++ b/include/dt-bindings/gpio/msc313-gpio.h
@@ -0,0 +1,95 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * GPIO definitions for MStar/SigmaStar MSC313 and later SoCs
+ *
+ * Copyright (C) 2020 Daniel Palmer <daniel@thingy.jp>
+ */
+
+#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.27.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 2/5] dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver
@ 2020-10-11  2:48   ` Daniel Palmer
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-11  2:48 UTC (permalink / raw)
  To: linux-gpio; +Cc: devicetree, Daniel Palmer, linux-kernel, linux-arm-kernel

The driver uses the pin names to find the right interrupt for
a pin from the device tree so this header reduces the need to
have multiple copies of the same string all over the place.

This header also 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 <daniel@0x0f.com>
---
 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 4594b70f2e3a..ec5b49b9955f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2158,6 +2158,7 @@ F:	Documentation/devicetree/bindings/arm/mstar/*
 F:	Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
 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 <mkpetch@internode.on.net>
diff --git a/include/dt-bindings/gpio/msc313-gpio.h b/include/dt-bindings/gpio/msc313-gpio.h
new file mode 100644
index 000000000000..655fe03de519
--- /dev/null
+++ b/include/dt-bindings/gpio/msc313-gpio.h
@@ -0,0 +1,95 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * GPIO definitions for MStar/SigmaStar MSC313 and later SoCs
+ *
+ * Copyright (C) 2020 Daniel Palmer <daniel@thingy.jp>
+ */
+
+#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.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
  2020-10-11  2:48 ` Daniel Palmer
@ 2020-10-11  2:48   ` Daniel Palmer
  -1 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-11  2:48 UTC (permalink / raw)
  To: linux-gpio; +Cc: devicetree, linux-arm-kernel, linux-kernel, Daniel Palmer

This adds a driver that supports the GPIO block found in
MStar/SigmaStar ARMv7 SoCs.

The controller seems to support 128 lines but where they
are wired up differs between chips and no currently known
chip uses anywhere near 128 lines so there needs to be some
per-chip data to collect together what lines actually have
physical pins attached and map the right names to them.

The core peripherals seem to use the same lines on the
currently known chips but the lines used for the sensor
interface, lcd controller etc pins seem to be totally
different between the infinity and mercury chips

The code tries to collect all of the re-usable names,
offsets etc together so that it's easy to build the extra
per-chip data for other chips in the future.

So far this only supports the MSC313 and MSC313E chips.

Support for the SSC8336N (mercury5) is trivial to add once
all of the lines have been mapped out.

Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 MAINTAINERS                |   1 +
 drivers/gpio/Kconfig       |   9 +
 drivers/gpio/Makefile      |   1 +
 drivers/gpio/gpio-msc313.c | 341 +++++++++++++++++++++++++++++++++++++
 4 files changed, 352 insertions(+)
 create mode 100644 drivers/gpio/gpio-msc313.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ec5b49b9955f..d20e8935dd4c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2158,6 +2158,7 @@ F:	Documentation/devicetree/bindings/arm/mstar/*
 F:	Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
 F:	arch/arm/boot/dts/mstar-*
 F:	arch/arm/mach-mstar/
+F:	drivers/gpio/gpio-msc313.c
 F:	include/dt-bindings/gpio/msc313-gpio.h
 
 ARM/NEC MOBILEPRO 900/c MACHINE SUPPORT
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 8030fd91a3cc..d85226cb2a07 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -712,6 +712,15 @@ config GPIO_AMD_FCH
 	  Note: This driver doesn't registers itself automatically, as it
 	  needs to be provided with platform specific configuration.
 	  (See eg. CONFIG_PCENGINES_APU2.)
+
+config GPIO_MSC313
+	bool "MStar MSC313 GPIO support"
+	default y if ARCH_MSTARV7
+	depends on ARCH_MSTARV7
+	select GPIO_GENERIC
+	help
+	  Say Y here to support GPIO on MStar MSC313 and later SoCs.
+
 endmenu
 
 menu "Port-mapped I/O GPIO drivers"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4f9abff4f2dc..7c675b502cc4 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -102,6 +102,7 @@ obj-$(CONFIG_GPIO_MOCKUP)		+= gpio-mockup.o
 obj-$(CONFIG_GPIO_MOXTET)		+= gpio-moxtet.o
 obj-$(CONFIG_GPIO_MPC5200)		+= gpio-mpc5200.o
 obj-$(CONFIG_GPIO_MPC8XXX)		+= gpio-mpc8xxx.o
+obj-$(CONFIG_GPIO_MSC313)		+= gpio-msc313.o
 obj-$(CONFIG_GPIO_MSIC)			+= gpio-msic.o
 obj-$(CONFIG_GPIO_MT7621)		+= gpio-mt7621.o
 obj-$(CONFIG_GPIO_MVEBU)		+= gpio-mvebu.o
diff --git a/drivers/gpio/gpio-msc313.c b/drivers/gpio/gpio-msc313.c
new file mode 100644
index 000000000000..6bdab77674ae
--- /dev/null
+++ b/drivers/gpio/gpio-msc313.c
@@ -0,0 +1,341 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Daniel Palmer<daniel@thingy.jp>
+ */
+
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/gpio/driver.h>
+
+#include <dt-bindings/gpio/msc313-gpio.h>
+
+#define DRIVER_NAME "gpio-msc313"
+
+#define MSC313_GPIO_IN  BIT(0)
+#define MSC313_GPIO_OUT BIT(4)
+#define MSC313_GPIO_OEN BIT(5)
+
+#define MSC313_GPIO_BITSTOSAVE (MSC313_GPIO_OUT | MSC313_GPIO_OEN)
+
+#define FUART_NAMES			\
+	MSC313_PINNAME_FUART_RX,	\
+	MSC313_PINNAME_FUART_TX,	\
+	MSC313_PINNAME_FUART_CTS,	\
+	MSC313_PINNAME_FUART_RTS
+
+#define OFF_FUART_RX	0x50
+#define OFF_FUART_TX	0x54
+#define OFF_FUART_CTS	0x58
+#define OFF_FUART_RTS	0x5c
+
+#define FUART_OFFSETS	\
+	OFF_FUART_RX,	\
+	OFF_FUART_TX,	\
+	OFF_FUART_CTS,	\
+	OFF_FUART_RTS
+
+#define SR_NAMES		\
+	MSC313_PINNAME_SR_IO2,	\
+	MSC313_PINNAME_SR_IO3,	\
+	MSC313_PINNAME_SR_IO4,	\
+	MSC313_PINNAME_SR_IO5,	\
+	MSC313_PINNAME_SR_IO6,	\
+	MSC313_PINNAME_SR_IO7,	\
+	MSC313_PINNAME_SR_IO8,	\
+	MSC313_PINNAME_SR_IO9,	\
+	MSC313_PINNAME_SR_IO10,	\
+	MSC313_PINNAME_SR_IO11,	\
+	MSC313_PINNAME_SR_IO12,	\
+	MSC313_PINNAME_SR_IO13,	\
+	MSC313_PINNAME_SR_IO14,	\
+	MSC313_PINNAME_SR_IO15,	\
+	MSC313_PINNAME_SR_IO16,	\
+	MSC313_PINNAME_SR_IO17
+
+#define OFF_SR_IO2	0x88
+#define OFF_SR_IO3	0x8c
+#define OFF_SR_IO4	0x90
+#define OFF_SR_IO5	0x94
+#define OFF_SR_IO6	0x98
+#define OFF_SR_IO7	0x9c
+#define OFF_SR_IO8	0xa0
+#define OFF_SR_IO9	0xa4
+#define OFF_SR_IO10	0xa8
+#define OFF_SR_IO11	0xac
+#define OFF_SR_IO12	0xb0
+#define OFF_SR_IO13	0xb4
+#define OFF_SR_IO14	0xb8
+#define OFF_SR_IO15	0xbc
+#define OFF_SR_IO16	0xc0
+#define OFF_SR_IO17	0xc4
+
+#define SR_OFFSETS	\
+	OFF_SR_IO2,	\
+	OFF_SR_IO3,	\
+	OFF_SR_IO4,	\
+	OFF_SR_IO5,	\
+	OFF_SR_IO6,	\
+	OFF_SR_IO7,	\
+	OFF_SR_IO8,	\
+	OFF_SR_IO9,	\
+	OFF_SR_IO10,	\
+	OFF_SR_IO11,	\
+	OFF_SR_IO12,	\
+	OFF_SR_IO13,	\
+	OFF_SR_IO14,	\
+	OFF_SR_IO15,	\
+	OFF_SR_IO16,	\
+	OFF_SR_IO17
+
+#define SD_NAMES		\
+	MSC313_PINNAME_SD_CLK,	\
+	MSC313_PINNAME_SD_CMD,	\
+	MSC313_PINNAME_SD_D0,	\
+	MSC313_PINNAME_SD_D1,	\
+	MSC313_PINNAME_SD_D2,	\
+	MSC313_PINNAME_SD_D3
+
+#define OFF_SD_CLK	0x140
+#define OFF_SD_CMD	0x144
+#define OFF_SD_D0	0x148
+#define OFF_SD_D1	0x14c
+#define OFF_SD_D2	0x150
+#define OFF_SD_D3	0x154
+
+#define SD_OFFSETS	\
+	OFF_SD_CLK,	\
+	OFF_SD_CMD,	\
+	OFF_SD_D0,	\
+	OFF_SD_D1,	\
+	OFF_SD_D2,	\
+	OFF_SD_D3
+
+#define I2C1_NAMES			\
+	MSC313_PINNAME_I2C1_SCL,	\
+	MSC313_PINNAME_I2C1_SCA
+
+#define OFF_I2C1_SCL	0x188
+#define OFF_I2C1_SCA	0x18c
+
+#define I2C1_OFFSETS	\
+	OFF_I2C1_SCL,	\
+	OFF_I2C1_SCA
+
+#define SPI0_NAMES		\
+	MSC313_PINNAME_SPI0_CZ,	\
+	MSC313_PINNAME_SPI0_CK,	\
+	MSC313_PINNAME_SPI0_DI,	\
+	MSC313_PINNAME_SPI0_DO
+
+#define OFF_SPI0_CZ	0x1c0
+#define OFF_SPI0_CK	0x1c4
+#define OFF_SPI0_DI	0x1c8
+#define OFF_SPI0_DO	0x1cc
+
+#define SPI0_OFFSETS	\
+	OFF_SPI0_CZ,	\
+	OFF_SPI0_CK,	\
+	OFF_SPI0_DI,	\
+	OFF_SPI0_DO
+
+struct msc313_gpio_data {
+	const char * const *names;
+	const unsigned int *offsets;
+	const unsigned int num;
+};
+
+#define MSC313_GPIO_CHIPDATA(_chip) \
+static const struct msc313_gpio_data _chip##_data = { \
+	.names = _chip##_names, \
+	.offsets = _chip##_offsets, \
+	.num = ARRAY_SIZE(_chip##_offsets), \
+}
+
+#ifdef CONFIG_MACH_INFINITY
+static const char * const msc313_names[] = {
+	FUART_NAMES,
+	SR_NAMES,
+	SD_NAMES,
+	I2C1_NAMES,
+	SPI0_NAMES,
+};
+
+static const unsigned int msc313_offsets[] = {
+	FUART_OFFSETS,
+	SR_OFFSETS,
+	SD_OFFSETS,
+	I2C1_OFFSETS,
+	SPI0_OFFSETS,
+};
+
+MSC313_GPIO_CHIPDATA(msc313);
+#endif
+
+struct msc313_gpio {
+	void __iomem *base;
+	const struct msc313_gpio_data *gpio_data;
+	int *irqs;
+	u8 *saved;
+};
+
+static void msc313_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
+{
+	struct msc313_gpio *gpio = gpiochip_get_data(chip);
+	u8 gpioreg = readb_relaxed(gpio->base + gpio->gpio_data->offsets[offset]);
+
+	if (value)
+		gpioreg |= MSC313_GPIO_OUT;
+	else
+		gpioreg &= ~MSC313_GPIO_OUT;
+
+	writeb_relaxed(gpioreg, gpio->base + gpio->gpio_data->offsets[offset]);
+}
+
+static int msc313_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct msc313_gpio *gpio = gpiochip_get_data(chip);
+
+	return readb_relaxed(gpio->base + gpio->gpio_data->offsets[offset])
+			& MSC313_GPIO_IN;
+}
+
+static int msc313_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+	struct msc313_gpio *gpio = gpiochip_get_data(chip);
+	u8 gpioreg = readb_relaxed(gpio->base + gpio->gpio_data->offsets[offset]);
+
+	gpioreg |= MSC313_GPIO_OEN;
+	writeb_relaxed(gpioreg, gpio->base + gpio->gpio_data->offsets[offset]);
+
+	return 0;
+}
+
+static int msc313_gpio_direction_output(struct gpio_chip *chip, unsigned int offset, int value)
+{
+	struct msc313_gpio *gpio = gpiochip_get_data(chip);
+	u8 gpioreg = readb_relaxed(gpio->base + gpio->gpio_data->offsets[offset]);
+
+	gpioreg &= ~MSC313_GPIO_OEN;
+	if (value)
+		gpioreg |= MSC313_GPIO_OUT;
+	else
+		gpioreg &= ~MSC313_GPIO_OUT;
+	writeb_relaxed(gpioreg, gpio->base + gpio->gpio_data->offsets[offset]);
+
+	return 0;
+}
+
+static int msc313_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
+{
+	struct msc313_gpio *gpio = gpiochip_get_data(chip);
+
+	return gpio->irqs[offset];
+}
+
+static int msc313_gpio_probe(struct platform_device *pdev)
+{
+	int i, ret;
+	const struct msc313_gpio_data *match_data;
+	struct msc313_gpio *gpio;
+	struct resource *res;
+	struct gpio_chip *gpiochip;
+
+	match_data = of_device_get_match_data(&pdev->dev);
+	if (!match_data)
+		return -EINVAL;
+
+	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
+	if (!gpio)
+		return -ENOMEM;
+
+	gpio->gpio_data = match_data;
+
+	gpio->irqs = devm_kzalloc(&pdev->dev, gpio->gpio_data->num * sizeof(*gpio->irqs), GFP_KERNEL);
+	if (!gpio->irqs)
+		return -ENOMEM;
+
+	gpio->saved = devm_kzalloc(&pdev->dev, gpio->gpio_data->num * sizeof(*gpio->saved), GFP_KERNEL);
+	if (!gpio->saved)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, gpio);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	gpio->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(gpio->base))
+		return PTR_ERR(gpio->base);
+
+	gpiochip = devm_kzalloc(&pdev->dev, sizeof(*gpiochip), GFP_KERNEL);
+	if (!gpiochip)
+		return -ENOMEM;
+
+	gpiochip->label = DRIVER_NAME;
+	gpiochip->parent = &pdev->dev;
+	gpiochip->request = gpiochip_generic_request;
+	gpiochip->free = gpiochip_generic_free;
+	gpiochip->direction_input = msc313_gpio_direction_input;
+	gpiochip->direction_output = msc313_gpio_direction_output;
+	gpiochip->get = msc313_gpio_get;
+	gpiochip->set = msc313_gpio_set;
+	gpiochip->to_irq = msc313_gpio_to_irq;
+	gpiochip->base = -1;
+	gpiochip->ngpio = gpio->gpio_data->num;
+	gpiochip->names = gpio->gpio_data->names;
+
+	for (i = 0; i < gpiochip->ngpio; i++)
+		gpio->irqs[i] = of_irq_get_byname(pdev->dev.of_node, gpio->gpio_data->names[i]);
+
+	ret = gpiochip_add_data(gpiochip, gpio);
+	return ret;
+}
+
+static const struct of_device_id msc313_gpio_of_match[] = {
+#ifdef CONFIG_MACH_INFINITY
+	{
+		.compatible = "mstar,msc313-gpio",
+		.data = &msc313_data,
+	},
+#endif
+	{ }
+};
+
+/* The GPIO controller loses the state of the registers when the
+ * SoC goes into "DRAM self-refresh low power" mode so we need to
+ * save the direction and value before suspending and put it back
+ * when resuming.
+ */
+
+static int __maybe_unused msc313_gpio_suspend(struct device *dev)
+{
+	struct msc313_gpio *gpio = dev_get_drvdata(dev);
+	int i;
+
+	for (i = 0; i < gpio->gpio_data->num; i++)
+		gpio->saved[i] = readb_relaxed(gpio->base + gpio->gpio_data->offsets[i]) & MSC313_GPIO_BITSTOSAVE;
+
+	return 0;
+}
+
+static int __maybe_unused msc313_gpio_resume(struct device *dev)
+{
+	struct msc313_gpio *gpio = dev_get_drvdata(dev);
+	int i;
+
+	for (i = 0; i < gpio->gpio_data->num; i++)
+		writeb_relaxed(gpio->saved[i], gpio->base + gpio->gpio_data->offsets[i]);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(msc313_gpio_ops, msc313_gpio_suspend, msc313_gpio_resume);
+
+static struct platform_driver msc313_gpio_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = msc313_gpio_of_match,
+		.pm = &msc313_gpio_ops,
+	},
+	.probe = msc313_gpio_probe,
+};
+
+builtin_platform_driver(msc313_gpio_driver);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
@ 2020-10-11  2:48   ` Daniel Palmer
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-11  2:48 UTC (permalink / raw)
  To: linux-gpio; +Cc: devicetree, Daniel Palmer, linux-kernel, linux-arm-kernel

This adds a driver that supports the GPIO block found in
MStar/SigmaStar ARMv7 SoCs.

The controller seems to support 128 lines but where they
are wired up differs between chips and no currently known
chip uses anywhere near 128 lines so there needs to be some
per-chip data to collect together what lines actually have
physical pins attached and map the right names to them.

The core peripherals seem to use the same lines on the
currently known chips but the lines used for the sensor
interface, lcd controller etc pins seem to be totally
different between the infinity and mercury chips

The code tries to collect all of the re-usable names,
offsets etc together so that it's easy to build the extra
per-chip data for other chips in the future.

So far this only supports the MSC313 and MSC313E chips.

Support for the SSC8336N (mercury5) is trivial to add once
all of the lines have been mapped out.

Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 MAINTAINERS                |   1 +
 drivers/gpio/Kconfig       |   9 +
 drivers/gpio/Makefile      |   1 +
 drivers/gpio/gpio-msc313.c | 341 +++++++++++++++++++++++++++++++++++++
 4 files changed, 352 insertions(+)
 create mode 100644 drivers/gpio/gpio-msc313.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ec5b49b9955f..d20e8935dd4c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2158,6 +2158,7 @@ F:	Documentation/devicetree/bindings/arm/mstar/*
 F:	Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
 F:	arch/arm/boot/dts/mstar-*
 F:	arch/arm/mach-mstar/
+F:	drivers/gpio/gpio-msc313.c
 F:	include/dt-bindings/gpio/msc313-gpio.h
 
 ARM/NEC MOBILEPRO 900/c MACHINE SUPPORT
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 8030fd91a3cc..d85226cb2a07 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -712,6 +712,15 @@ config GPIO_AMD_FCH
 	  Note: This driver doesn't registers itself automatically, as it
 	  needs to be provided with platform specific configuration.
 	  (See eg. CONFIG_PCENGINES_APU2.)
+
+config GPIO_MSC313
+	bool "MStar MSC313 GPIO support"
+	default y if ARCH_MSTARV7
+	depends on ARCH_MSTARV7
+	select GPIO_GENERIC
+	help
+	  Say Y here to support GPIO on MStar MSC313 and later SoCs.
+
 endmenu
 
 menu "Port-mapped I/O GPIO drivers"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4f9abff4f2dc..7c675b502cc4 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -102,6 +102,7 @@ obj-$(CONFIG_GPIO_MOCKUP)		+= gpio-mockup.o
 obj-$(CONFIG_GPIO_MOXTET)		+= gpio-moxtet.o
 obj-$(CONFIG_GPIO_MPC5200)		+= gpio-mpc5200.o
 obj-$(CONFIG_GPIO_MPC8XXX)		+= gpio-mpc8xxx.o
+obj-$(CONFIG_GPIO_MSC313)		+= gpio-msc313.o
 obj-$(CONFIG_GPIO_MSIC)			+= gpio-msic.o
 obj-$(CONFIG_GPIO_MT7621)		+= gpio-mt7621.o
 obj-$(CONFIG_GPIO_MVEBU)		+= gpio-mvebu.o
diff --git a/drivers/gpio/gpio-msc313.c b/drivers/gpio/gpio-msc313.c
new file mode 100644
index 000000000000..6bdab77674ae
--- /dev/null
+++ b/drivers/gpio/gpio-msc313.c
@@ -0,0 +1,341 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Daniel Palmer<daniel@thingy.jp>
+ */
+
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/gpio/driver.h>
+
+#include <dt-bindings/gpio/msc313-gpio.h>
+
+#define DRIVER_NAME "gpio-msc313"
+
+#define MSC313_GPIO_IN  BIT(0)
+#define MSC313_GPIO_OUT BIT(4)
+#define MSC313_GPIO_OEN BIT(5)
+
+#define MSC313_GPIO_BITSTOSAVE (MSC313_GPIO_OUT | MSC313_GPIO_OEN)
+
+#define FUART_NAMES			\
+	MSC313_PINNAME_FUART_RX,	\
+	MSC313_PINNAME_FUART_TX,	\
+	MSC313_PINNAME_FUART_CTS,	\
+	MSC313_PINNAME_FUART_RTS
+
+#define OFF_FUART_RX	0x50
+#define OFF_FUART_TX	0x54
+#define OFF_FUART_CTS	0x58
+#define OFF_FUART_RTS	0x5c
+
+#define FUART_OFFSETS	\
+	OFF_FUART_RX,	\
+	OFF_FUART_TX,	\
+	OFF_FUART_CTS,	\
+	OFF_FUART_RTS
+
+#define SR_NAMES		\
+	MSC313_PINNAME_SR_IO2,	\
+	MSC313_PINNAME_SR_IO3,	\
+	MSC313_PINNAME_SR_IO4,	\
+	MSC313_PINNAME_SR_IO5,	\
+	MSC313_PINNAME_SR_IO6,	\
+	MSC313_PINNAME_SR_IO7,	\
+	MSC313_PINNAME_SR_IO8,	\
+	MSC313_PINNAME_SR_IO9,	\
+	MSC313_PINNAME_SR_IO10,	\
+	MSC313_PINNAME_SR_IO11,	\
+	MSC313_PINNAME_SR_IO12,	\
+	MSC313_PINNAME_SR_IO13,	\
+	MSC313_PINNAME_SR_IO14,	\
+	MSC313_PINNAME_SR_IO15,	\
+	MSC313_PINNAME_SR_IO16,	\
+	MSC313_PINNAME_SR_IO17
+
+#define OFF_SR_IO2	0x88
+#define OFF_SR_IO3	0x8c
+#define OFF_SR_IO4	0x90
+#define OFF_SR_IO5	0x94
+#define OFF_SR_IO6	0x98
+#define OFF_SR_IO7	0x9c
+#define OFF_SR_IO8	0xa0
+#define OFF_SR_IO9	0xa4
+#define OFF_SR_IO10	0xa8
+#define OFF_SR_IO11	0xac
+#define OFF_SR_IO12	0xb0
+#define OFF_SR_IO13	0xb4
+#define OFF_SR_IO14	0xb8
+#define OFF_SR_IO15	0xbc
+#define OFF_SR_IO16	0xc0
+#define OFF_SR_IO17	0xc4
+
+#define SR_OFFSETS	\
+	OFF_SR_IO2,	\
+	OFF_SR_IO3,	\
+	OFF_SR_IO4,	\
+	OFF_SR_IO5,	\
+	OFF_SR_IO6,	\
+	OFF_SR_IO7,	\
+	OFF_SR_IO8,	\
+	OFF_SR_IO9,	\
+	OFF_SR_IO10,	\
+	OFF_SR_IO11,	\
+	OFF_SR_IO12,	\
+	OFF_SR_IO13,	\
+	OFF_SR_IO14,	\
+	OFF_SR_IO15,	\
+	OFF_SR_IO16,	\
+	OFF_SR_IO17
+
+#define SD_NAMES		\
+	MSC313_PINNAME_SD_CLK,	\
+	MSC313_PINNAME_SD_CMD,	\
+	MSC313_PINNAME_SD_D0,	\
+	MSC313_PINNAME_SD_D1,	\
+	MSC313_PINNAME_SD_D2,	\
+	MSC313_PINNAME_SD_D3
+
+#define OFF_SD_CLK	0x140
+#define OFF_SD_CMD	0x144
+#define OFF_SD_D0	0x148
+#define OFF_SD_D1	0x14c
+#define OFF_SD_D2	0x150
+#define OFF_SD_D3	0x154
+
+#define SD_OFFSETS	\
+	OFF_SD_CLK,	\
+	OFF_SD_CMD,	\
+	OFF_SD_D0,	\
+	OFF_SD_D1,	\
+	OFF_SD_D2,	\
+	OFF_SD_D3
+
+#define I2C1_NAMES			\
+	MSC313_PINNAME_I2C1_SCL,	\
+	MSC313_PINNAME_I2C1_SCA
+
+#define OFF_I2C1_SCL	0x188
+#define OFF_I2C1_SCA	0x18c
+
+#define I2C1_OFFSETS	\
+	OFF_I2C1_SCL,	\
+	OFF_I2C1_SCA
+
+#define SPI0_NAMES		\
+	MSC313_PINNAME_SPI0_CZ,	\
+	MSC313_PINNAME_SPI0_CK,	\
+	MSC313_PINNAME_SPI0_DI,	\
+	MSC313_PINNAME_SPI0_DO
+
+#define OFF_SPI0_CZ	0x1c0
+#define OFF_SPI0_CK	0x1c4
+#define OFF_SPI0_DI	0x1c8
+#define OFF_SPI0_DO	0x1cc
+
+#define SPI0_OFFSETS	\
+	OFF_SPI0_CZ,	\
+	OFF_SPI0_CK,	\
+	OFF_SPI0_DI,	\
+	OFF_SPI0_DO
+
+struct msc313_gpio_data {
+	const char * const *names;
+	const unsigned int *offsets;
+	const unsigned int num;
+};
+
+#define MSC313_GPIO_CHIPDATA(_chip) \
+static const struct msc313_gpio_data _chip##_data = { \
+	.names = _chip##_names, \
+	.offsets = _chip##_offsets, \
+	.num = ARRAY_SIZE(_chip##_offsets), \
+}
+
+#ifdef CONFIG_MACH_INFINITY
+static const char * const msc313_names[] = {
+	FUART_NAMES,
+	SR_NAMES,
+	SD_NAMES,
+	I2C1_NAMES,
+	SPI0_NAMES,
+};
+
+static const unsigned int msc313_offsets[] = {
+	FUART_OFFSETS,
+	SR_OFFSETS,
+	SD_OFFSETS,
+	I2C1_OFFSETS,
+	SPI0_OFFSETS,
+};
+
+MSC313_GPIO_CHIPDATA(msc313);
+#endif
+
+struct msc313_gpio {
+	void __iomem *base;
+	const struct msc313_gpio_data *gpio_data;
+	int *irqs;
+	u8 *saved;
+};
+
+static void msc313_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
+{
+	struct msc313_gpio *gpio = gpiochip_get_data(chip);
+	u8 gpioreg = readb_relaxed(gpio->base + gpio->gpio_data->offsets[offset]);
+
+	if (value)
+		gpioreg |= MSC313_GPIO_OUT;
+	else
+		gpioreg &= ~MSC313_GPIO_OUT;
+
+	writeb_relaxed(gpioreg, gpio->base + gpio->gpio_data->offsets[offset]);
+}
+
+static int msc313_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct msc313_gpio *gpio = gpiochip_get_data(chip);
+
+	return readb_relaxed(gpio->base + gpio->gpio_data->offsets[offset])
+			& MSC313_GPIO_IN;
+}
+
+static int msc313_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+	struct msc313_gpio *gpio = gpiochip_get_data(chip);
+	u8 gpioreg = readb_relaxed(gpio->base + gpio->gpio_data->offsets[offset]);
+
+	gpioreg |= MSC313_GPIO_OEN;
+	writeb_relaxed(gpioreg, gpio->base + gpio->gpio_data->offsets[offset]);
+
+	return 0;
+}
+
+static int msc313_gpio_direction_output(struct gpio_chip *chip, unsigned int offset, int value)
+{
+	struct msc313_gpio *gpio = gpiochip_get_data(chip);
+	u8 gpioreg = readb_relaxed(gpio->base + gpio->gpio_data->offsets[offset]);
+
+	gpioreg &= ~MSC313_GPIO_OEN;
+	if (value)
+		gpioreg |= MSC313_GPIO_OUT;
+	else
+		gpioreg &= ~MSC313_GPIO_OUT;
+	writeb_relaxed(gpioreg, gpio->base + gpio->gpio_data->offsets[offset]);
+
+	return 0;
+}
+
+static int msc313_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
+{
+	struct msc313_gpio *gpio = gpiochip_get_data(chip);
+
+	return gpio->irqs[offset];
+}
+
+static int msc313_gpio_probe(struct platform_device *pdev)
+{
+	int i, ret;
+	const struct msc313_gpio_data *match_data;
+	struct msc313_gpio *gpio;
+	struct resource *res;
+	struct gpio_chip *gpiochip;
+
+	match_data = of_device_get_match_data(&pdev->dev);
+	if (!match_data)
+		return -EINVAL;
+
+	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
+	if (!gpio)
+		return -ENOMEM;
+
+	gpio->gpio_data = match_data;
+
+	gpio->irqs = devm_kzalloc(&pdev->dev, gpio->gpio_data->num * sizeof(*gpio->irqs), GFP_KERNEL);
+	if (!gpio->irqs)
+		return -ENOMEM;
+
+	gpio->saved = devm_kzalloc(&pdev->dev, gpio->gpio_data->num * sizeof(*gpio->saved), GFP_KERNEL);
+	if (!gpio->saved)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, gpio);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	gpio->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(gpio->base))
+		return PTR_ERR(gpio->base);
+
+	gpiochip = devm_kzalloc(&pdev->dev, sizeof(*gpiochip), GFP_KERNEL);
+	if (!gpiochip)
+		return -ENOMEM;
+
+	gpiochip->label = DRIVER_NAME;
+	gpiochip->parent = &pdev->dev;
+	gpiochip->request = gpiochip_generic_request;
+	gpiochip->free = gpiochip_generic_free;
+	gpiochip->direction_input = msc313_gpio_direction_input;
+	gpiochip->direction_output = msc313_gpio_direction_output;
+	gpiochip->get = msc313_gpio_get;
+	gpiochip->set = msc313_gpio_set;
+	gpiochip->to_irq = msc313_gpio_to_irq;
+	gpiochip->base = -1;
+	gpiochip->ngpio = gpio->gpio_data->num;
+	gpiochip->names = gpio->gpio_data->names;
+
+	for (i = 0; i < gpiochip->ngpio; i++)
+		gpio->irqs[i] = of_irq_get_byname(pdev->dev.of_node, gpio->gpio_data->names[i]);
+
+	ret = gpiochip_add_data(gpiochip, gpio);
+	return ret;
+}
+
+static const struct of_device_id msc313_gpio_of_match[] = {
+#ifdef CONFIG_MACH_INFINITY
+	{
+		.compatible = "mstar,msc313-gpio",
+		.data = &msc313_data,
+	},
+#endif
+	{ }
+};
+
+/* The GPIO controller loses the state of the registers when the
+ * SoC goes into "DRAM self-refresh low power" mode so we need to
+ * save the direction and value before suspending and put it back
+ * when resuming.
+ */
+
+static int __maybe_unused msc313_gpio_suspend(struct device *dev)
+{
+	struct msc313_gpio *gpio = dev_get_drvdata(dev);
+	int i;
+
+	for (i = 0; i < gpio->gpio_data->num; i++)
+		gpio->saved[i] = readb_relaxed(gpio->base + gpio->gpio_data->offsets[i]) & MSC313_GPIO_BITSTOSAVE;
+
+	return 0;
+}
+
+static int __maybe_unused msc313_gpio_resume(struct device *dev)
+{
+	struct msc313_gpio *gpio = dev_get_drvdata(dev);
+	int i;
+
+	for (i = 0; i < gpio->gpio_data->num; i++)
+		writeb_relaxed(gpio->saved[i], gpio->base + gpio->gpio_data->offsets[i]);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(msc313_gpio_ops, msc313_gpio_suspend, msc313_gpio_resume);
+
+static struct platform_driver msc313_gpio_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = msc313_gpio_of_match,
+		.pm = &msc313_gpio_ops,
+	},
+	.probe = msc313_gpio_probe,
+};
+
+builtin_platform_driver(msc313_gpio_driver);
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 4/5] ARM: mstar: Add gpio controller to MStar base dtsi
  2020-10-11  2:48 ` Daniel Palmer
@ 2020-10-11  2:48   ` Daniel Palmer
  -1 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-11  2:48 UTC (permalink / raw)
  To: linux-gpio; +Cc: devicetree, linux-arm-kernel, linux-kernel, Daniel Palmer

The GPIO controller is at the same address in all of the
currently known chips so create a node for it in the base
dtsi.

Some extra properties are needed to actually use it so
disable it by default.

Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 arch/arm/boot/dts/mstar-v7.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/mstar-v7.dtsi b/arch/arm/boot/dts/mstar-v7.dtsi
index f07880561e11..669aada6f286 100644
--- a/arch/arm/boot/dts/mstar-v7.dtsi
+++ b/arch/arm/boot/dts/mstar-v7.dtsi
@@ -109,6 +109,13 @@ l3bridge: l3bridge@204400 {
 				reg = <0x204400 0x200>;
 			};
 
+			gpio: gpio@207800 {
+				#gpio-cells = <2>;
+				reg = <0x207800 0x200>;
+				gpio-controller;
+				status = "disabled";
+			};
+
 			pm_uart: uart@221000 {
 				compatible = "ns16550a";
 				reg = <0x221000 0x100>;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 4/5] ARM: mstar: Add gpio controller to MStar base dtsi
@ 2020-10-11  2:48   ` Daniel Palmer
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-11  2:48 UTC (permalink / raw)
  To: linux-gpio; +Cc: devicetree, Daniel Palmer, linux-kernel, linux-arm-kernel

The GPIO controller is at the same address in all of the
currently known chips so create a node for it in the base
dtsi.

Some extra properties are needed to actually use it so
disable it by default.

Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 arch/arm/boot/dts/mstar-v7.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/mstar-v7.dtsi b/arch/arm/boot/dts/mstar-v7.dtsi
index f07880561e11..669aada6f286 100644
--- a/arch/arm/boot/dts/mstar-v7.dtsi
+++ b/arch/arm/boot/dts/mstar-v7.dtsi
@@ -109,6 +109,13 @@ l3bridge: l3bridge@204400 {
 				reg = <0x204400 0x200>;
 			};
 
+			gpio: gpio@207800 {
+				#gpio-cells = <2>;
+				reg = <0x207800 0x200>;
+				gpio-controller;
+				status = "disabled";
+			};
+
 			pm_uart: uart@221000 {
 				compatible = "ns16550a";
 				reg = <0x221000 0x100>;
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 5/5] ARM: mstar: Fill in GPIO controller properties for infinity
  2020-10-11  2:48 ` Daniel Palmer
@ 2020-10-11  2:48   ` Daniel Palmer
  -1 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-11  2:48 UTC (permalink / raw)
  To: linux-gpio; +Cc: devicetree, linux-arm-kernel, linux-kernel, Daniel Palmer

Fill in the properties needed to use the GPIO controller
in the infinity and infinity3 chips.

Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 arch/arm/boot/dts/mstar-infinity.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/mstar-infinity.dtsi b/arch/arm/boot/dts/mstar-infinity.dtsi
index cd911adef014..6432b2976c2c 100644
--- a/arch/arm/boot/dts/mstar-infinity.dtsi
+++ b/arch/arm/boot/dts/mstar-infinity.dtsi
@@ -6,6 +6,22 @@
 
 #include "mstar-v7.dtsi"
 
+#include <dt-bindings/gpio/msc313-gpio.h>
+
 &imi {
 	reg = <0xa0000000 0x16000>;
 };
+
+&gpio {
+	compatible = "mstar,msc313-gpio";
+	interrupt-parent = <&intc_fiq>;
+	interrupt-names = MSC313_PINNAME_SPI0_CZ,
+			  MSC313_PINNAME_SPI0_CK,
+			  MSC313_PINNAME_SPI0_DI,
+			  MSC313_PINNAME_SPI0_DO;
+	interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+	status = "okay";
+};
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 5/5] ARM: mstar: Fill in GPIO controller properties for infinity
@ 2020-10-11  2:48   ` Daniel Palmer
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-11  2:48 UTC (permalink / raw)
  To: linux-gpio; +Cc: devicetree, Daniel Palmer, linux-kernel, linux-arm-kernel

Fill in the properties needed to use the GPIO controller
in the infinity and infinity3 chips.

Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 arch/arm/boot/dts/mstar-infinity.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/mstar-infinity.dtsi b/arch/arm/boot/dts/mstar-infinity.dtsi
index cd911adef014..6432b2976c2c 100644
--- a/arch/arm/boot/dts/mstar-infinity.dtsi
+++ b/arch/arm/boot/dts/mstar-infinity.dtsi
@@ -6,6 +6,22 @@
 
 #include "mstar-v7.dtsi"
 
+#include <dt-bindings/gpio/msc313-gpio.h>
+
 &imi {
 	reg = <0xa0000000 0x16000>;
 };
+
+&gpio {
+	compatible = "mstar,msc313-gpio";
+	interrupt-parent = <&intc_fiq>;
+	interrupt-names = MSC313_PINNAME_SPI0_CZ,
+			  MSC313_PINNAME_SPI0_CK,
+			  MSC313_PINNAME_SPI0_DI,
+			  MSC313_PINNAME_SPI0_DO;
+	interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+	status = "okay";
+};
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH 1/5] dt-bindings: gpio: Binding for MStar MSC313 GPIO controller
  2020-10-11  2:48   ` Daniel Palmer
@ 2020-10-12 16:10     ` Rob Herring
  -1 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2020-10-12 16:10 UTC (permalink / raw)
  To: Daniel Palmer; +Cc: linux-gpio, devicetree, linux-arm-kernel, linux-kernel

On Sun, 11 Oct 2020 11:48:27 +0900, Daniel Palmer wrote:
> Add a binding description for the MStar/SigmaStar GPIO controller
> found in the MSC313 and later ARMv7 SoCs.
> 
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>
> ---
>  .../bindings/gpio/mstar,msc313-gpio.yaml      | 69 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.example.dts:21:18: fatal error: dt-bindings/gpio/msc313-gpio.h: No such file or directory
   21 |         #include <dt-bindings/gpio/msc313-gpio.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:342: Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1366: dt_binding_check] Error 2


See https://patchwork.ozlabs.org/patch/1380236

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 1/5] dt-bindings: gpio: Binding for MStar MSC313 GPIO controller
@ 2020-10-12 16:10     ` Rob Herring
  0 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2020-10-12 16:10 UTC (permalink / raw)
  To: Daniel Palmer; +Cc: linux-gpio, linux-kernel, linux-arm-kernel, devicetree

On Sun, 11 Oct 2020 11:48:27 +0900, Daniel Palmer wrote:
> Add a binding description for the MStar/SigmaStar GPIO controller
> found in the MSC313 and later ARMv7 SoCs.
> 
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>
> ---
>  .../bindings/gpio/mstar,msc313-gpio.yaml      | 69 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.example.dts:21:18: fatal error: dt-bindings/gpio/msc313-gpio.h: No such file or directory
   21 |         #include <dt-bindings/gpio/msc313-gpio.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:342: Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1366: dt_binding_check] Error 2


See https://patchwork.ozlabs.org/patch/1380236

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 2/5] dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver
  2020-10-11  2:48   ` Daniel Palmer
@ 2020-10-12 16:11     ` Rob Herring
  -1 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2020-10-12 16:11 UTC (permalink / raw)
  To: Daniel Palmer; +Cc: linux-gpio, devicetree, linux-arm-kernel, linux-kernel

On Sun, Oct 11, 2020 at 11:48:28AM +0900, Daniel Palmer wrote:
> The driver uses the pin names to find the right interrupt for
> a pin from the device tree so this header reduces the need to
> have multiple copies of the same string all over the place.
> 
> This header also 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 <daniel@0x0f.com>
> ---
>  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

This should be part of the previous patch to avoid the error.

> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4594b70f2e3a..ec5b49b9955f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2158,6 +2158,7 @@ F:	Documentation/devicetree/bindings/arm/mstar/*
>  F:	Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
>  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 <mkpetch@internode.on.net>
> diff --git a/include/dt-bindings/gpio/msc313-gpio.h b/include/dt-bindings/gpio/msc313-gpio.h
> new file mode 100644
> index 000000000000..655fe03de519
> --- /dev/null
> +++ b/include/dt-bindings/gpio/msc313-gpio.h
> @@ -0,0 +1,95 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */

Don't use DT on non-GPL systems?

> +/*
> + * GPIO definitions for MStar/SigmaStar MSC313 and later SoCs
> + *
> + * Copyright (C) 2020 Daniel Palmer <daniel@thingy.jp>
> + */
> +
> +#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.27.0
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 2/5] dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver
@ 2020-10-12 16:11     ` Rob Herring
  0 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2020-10-12 16:11 UTC (permalink / raw)
  To: Daniel Palmer; +Cc: linux-gpio, linux-kernel, linux-arm-kernel, devicetree

On Sun, Oct 11, 2020 at 11:48:28AM +0900, Daniel Palmer wrote:
> The driver uses the pin names to find the right interrupt for
> a pin from the device tree so this header reduces the need to
> have multiple copies of the same string all over the place.
> 
> This header also 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 <daniel@0x0f.com>
> ---
>  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

This should be part of the previous patch to avoid the error.

> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4594b70f2e3a..ec5b49b9955f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2158,6 +2158,7 @@ F:	Documentation/devicetree/bindings/arm/mstar/*
>  F:	Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml
>  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 <mkpetch@internode.on.net>
> diff --git a/include/dt-bindings/gpio/msc313-gpio.h b/include/dt-bindings/gpio/msc313-gpio.h
> new file mode 100644
> index 000000000000..655fe03de519
> --- /dev/null
> +++ b/include/dt-bindings/gpio/msc313-gpio.h
> @@ -0,0 +1,95 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */

Don't use DT on non-GPL systems?

> +/*
> + * GPIO definitions for MStar/SigmaStar MSC313 and later SoCs
> + *
> + * Copyright (C) 2020 Daniel Palmer <daniel@thingy.jp>
> + */
> +
> +#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.27.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 2/5] dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver
  2020-10-12 16:11     ` Rob Herring
@ 2020-10-14  9:45       ` Daniel Palmer
  -1 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-14  9:45 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-gpio, DTML, linux-arm-kernel, linux-kernel

Hi Rob,

On Tue, 13 Oct 2020 at 01:11, Rob Herring <robh@kernel.org> wrote:
<snip>
> >  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
>
> This should be part of the previous patch to avoid the error.

Would reordering the patches to make this header before the yaml file
be acceptable?
The commit message might be pretty big with them squashed into one.

<snip>
> > @@ -0,0 +1,95 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
>
> Don't use DT on non-GPL systems?

Good point. I didn't really think about the header also being used for
something like FreeBSD.
I'll fix that.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 2/5] dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver
@ 2020-10-14  9:45       ` Daniel Palmer
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-14  9:45 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-gpio, linux-kernel, linux-arm-kernel, DTML

Hi Rob,

On Tue, 13 Oct 2020 at 01:11, Rob Herring <robh@kernel.org> wrote:
<snip>
> >  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
>
> This should be part of the previous patch to avoid the error.

Would reordering the patches to make this header before the yaml file
be acceptable?
The commit message might be pretty big with them squashed into one.

<snip>
> > @@ -0,0 +1,95 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
>
> Don't use DT on non-GPL systems?

Good point. I didn't really think about the header also being used for
something like FreeBSD.
I'll fix that.

Cheers,

Daniel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 2/5] dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver
  2020-10-14  9:45       ` Daniel Palmer
@ 2020-10-14 12:17         ` Rob Herring
  -1 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2020-10-14 12:17 UTC (permalink / raw)
  To: Daniel Palmer
  Cc: open list:GPIO SUBSYSTEM, DTML, linux-arm-kernel, linux-kernel

On Wed, Oct 14, 2020 at 4:46 AM Daniel Palmer <daniel@0x0f.com> wrote:
>
> Hi Rob,
>
> On Tue, 13 Oct 2020 at 01:11, Rob Herring <robh@kernel.org> wrote:
> <snip>
> > >  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
> >
> > This should be part of the previous patch to avoid the error.
>
> Would reordering the patches to make this header before the yaml file
> be acceptable?

Sure.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 2/5] dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver
@ 2020-10-14 12:17         ` Rob Herring
  0 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2020-10-14 12:17 UTC (permalink / raw)
  To: Daniel Palmer
  Cc: open list:GPIO SUBSYSTEM, linux-kernel, linux-arm-kernel, DTML

On Wed, Oct 14, 2020 at 4:46 AM Daniel Palmer <daniel@0x0f.com> wrote:
>
> Hi Rob,
>
> On Tue, 13 Oct 2020 at 01:11, Rob Herring <robh@kernel.org> wrote:
> <snip>
> > >  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
> >
> > This should be part of the previous patch to avoid the error.
>
> Would reordering the patches to make this header before the yaml file
> be acceptable?

Sure.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 1/5] dt-bindings: gpio: Binding for MStar MSC313 GPIO controller
  2020-10-11  2:48   ` Daniel Palmer
@ 2020-10-16 16:36     ` Linus Walleij
  -1 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2020-10-16 16:36 UTC (permalink / raw)
  To: Daniel Palmer, Krzysztof Kozlowski
  Cc: open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linux-kernel

On Sun, Oct 11, 2020 at 4:48 AM Daniel Palmer <daniel@0x0f.com> wrote:

> Add a binding description for the MStar/SigmaStar GPIO controller
> found in the MSC313 and later ARMv7 SoCs.
>
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>

I think Krzysztof is working on some generic bindings that
will make it easier to write YAML GPIO controller bindings,
but I don't know the status of them. I would be happy to merge
them early for v5.11 though.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 1/5] dt-bindings: gpio: Binding for MStar MSC313 GPIO controller
@ 2020-10-16 16:36     ` Linus Walleij
  0 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2020-10-16 16:36 UTC (permalink / raw)
  To: Daniel Palmer, Krzysztof Kozlowski
  Cc: open list:GPIO SUBSYSTEM, linux-kernel, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Sun, Oct 11, 2020 at 4:48 AM Daniel Palmer <daniel@0x0f.com> wrote:

> Add a binding description for the MStar/SigmaStar GPIO controller
> found in the MSC313 and later ARMv7 SoCs.
>
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>

I think Krzysztof is working on some generic bindings that
will make it easier to write YAML GPIO controller bindings,
but I don't know the status of them. I would be happy to merge
them early for v5.11 though.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
  2020-10-11  2:48   ` Daniel Palmer
@ 2020-10-16 16:56     ` Linus Walleij
  -1 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2020-10-16 16:56 UTC (permalink / raw)
  To: Daniel Palmer
  Cc: open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linux-kernel

Hi Daniel,

thanks for your patch!

Some comments below, we need some work but keep at it.

On Sun, Oct 11, 2020 at 4:48 AM Daniel Palmer <daniel@0x0f.com> wrote:

> This adds a driver that supports the GPIO block found in
> MStar/SigmaStar ARMv7 SoCs.
>
> The controller seems to support 128 lines but where they
> are wired up differs between chips and no currently known
> chip uses anywhere near 128 lines so there needs to be some
> per-chip data to collect together what lines actually have
> physical pins attached and map the right names to them.
>
> The core peripherals seem to use the same lines on the
> currently known chips but the lines used for the sensor
> interface, lcd controller etc pins seem to be totally
> different between the infinity and mercury chips
>
> The code tries to collect all of the re-usable names,
> offsets etc together so that it's easy to build the extra
> per-chip data for other chips in the future.
>
> So far this only supports the MSC313 and MSC313E chips.
>
> Support for the SSC8336N (mercury5) is trivial to add once
> all of the lines have been mapped out.
>
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>

(...)

> +config GPIO_MSC313
> +       bool "MStar MSC313 GPIO support"
> +       default y if ARCH_MSTARV7
> +       depends on ARCH_MSTARV7
> +       select GPIO_GENERIC

Selecting GPIO_GENERIC, that is good.
But you're not using it, because you can't.
This chip does not have the bits lined up nicely
in one register, instead there seems to be something
like one register per line, right?
So skip GPIO_GENERIC.

> +#define MSC313_GPIO_IN  BIT(0)
> +#define MSC313_GPIO_OUT BIT(4)
> +#define MSC313_GPIO_OEN BIT(5)
> +
> +#define MSC313_GPIO_BITSTOSAVE (MSC313_GPIO_OUT | MSC313_GPIO_OEN)

Some comment here telling us why these need saving and
not others.

> +#define FUART_NAMES                    \
> +       MSC313_PINNAME_FUART_RX,        \
> +       MSC313_PINNAME_FUART_TX,        \
> +       MSC313_PINNAME_FUART_CTS,       \
> +       MSC313_PINNAME_FUART_RTS
> +
> +#define OFF_FUART_RX   0x50
> +#define OFF_FUART_TX   0x54
> +#define OFF_FUART_CTS  0x58
> +#define OFF_FUART_RTS  0x5c
> +
> +#define FUART_OFFSETS  \
> +       OFF_FUART_RX,   \
> +       OFF_FUART_TX,   \
> +       OFF_FUART_CTS,  \
> +       OFF_FUART_RTS

This looks a bit strange. The GPIO driver should not really
have to know about any other use cases for pins than
GPIO. But I guess it is intuitive for the driver.

> +#define SD_NAMES               \
> +       MSC313_PINNAME_SD_CLK,  \
> +       MSC313_PINNAME_SD_CMD,  \
> +       MSC313_PINNAME_SD_D0,   \
> +       MSC313_PINNAME_SD_D1,   \
> +       MSC313_PINNAME_SD_D2,   \
> +       MSC313_PINNAME_SD_D3
> +
> +#define OFF_SD_CLK     0x140
> +#define OFF_SD_CMD     0x144
> +#define OFF_SD_D0      0x148
> +#define OFF_SD_D1      0x14cchild_to_parent_hwirq
> +#define OFF_SD_D2      0x150
> +#define OFF_SD_D3      0x154
> +
> +#define SD_OFFSETS     \
> +       OFF_SD_CLK,     \
> +       OFF_SD_CMD,     \
> +       OFF_SD_D0,      \
> +       OFF_SD_D1,      \
> +       OFF_SD_D2,      \
> +       OFF_SD_D3
> +
> +#define I2C1_NAMES                     \
> +       MSC313_PINNAME_I2C1_SCL,        \
> +       MSC313_PINNAME_I2C1_SCA
> +
> +#define OFF_I2C1_SCL   0x188
> +#define OFF_I2C1_SCA   0x18c
> +
> +#define I2C1_OFFSETS   \
> +       OFF_I2C1_SCL,   \
> +       OFF_I2C1_SCA
> +
> +#define SPI0_NAMES             \
> +       MSC313_PINNAME_SPI0_CZ, \
> +       MSC313_PINNAME_SPI0_CK, \
> +       MSC313_PINNAME_SPI0_DI, \
> +       MSC313_PINNAME_SPI0_DO
> +
> +#define OFF_SPI0_CZ    0x1c0
> +#define OFF_SPI0_CK    0x1c4
> +#define OFF_SPI0_DI    0x1c8
> +#define OFF_SPI0_DO    0x1cc
> +
> +#define SPI0_OFFSETS   \
> +       OFF_SPI0_CZ,    \
> +       OFF_SPI0_CK,    \
> +       OFF_SPI0_DI,    \
> +       OFF_SPI0_DO

Same with all these. I suppose it is the offsets of stuff
that would be there unless we were using it for GPIO.

> +static int msc313_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct msc313_gpio *gpio = gpiochip_get_data(chip);
> +> +

> +       return gpio->irqs[offset];
> +}

Please do not use custom IRQ handling like this.
As there seems to be one IRQ per line, look into using

        select GPIOLIB_IRQCHIP
        select IRQ_DOMAIN_HIERARCHY

See for example in gpio-ixp4xx.c how we deal with
hiearchical GPIO IRQs.

> +       gpiochip->to_irq = msc313_gpio_to_irq;
> +       gpiochip->base = -1;
> +       gpiochip->ngpio = gpio->gpio_data->num;
> +       gpiochip->names = gpio->gpio_data->names;
> +
> +       for (i = 0; i < gpiochip->ngpio; i++)
> +               gpio->irqs[i] = of_irq_get_byname(pdev->dev.of_node, gpio->gpio_data->names[i]);

Use hierarchical generic GPIO IRQs for these.

Assign ->fwnode, ->parent_domain, ->child_to_parent_hwirq,
and probably also ->handler on the struct gpio_irq_chip *.

Skip assigning gpiochip->to_irq, the generic code will
handle that.

Again see gpio-ixp4xx.c for an example.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
@ 2020-10-16 16:56     ` Linus Walleij
  0 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2020-10-16 16:56 UTC (permalink / raw)
  To: Daniel Palmer
  Cc: open list:GPIO SUBSYSTEM, linux-kernel, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Daniel,

thanks for your patch!

Some comments below, we need some work but keep at it.

On Sun, Oct 11, 2020 at 4:48 AM Daniel Palmer <daniel@0x0f.com> wrote:

> This adds a driver that supports the GPIO block found in
> MStar/SigmaStar ARMv7 SoCs.
>
> The controller seems to support 128 lines but where they
> are wired up differs between chips and no currently known
> chip uses anywhere near 128 lines so there needs to be some
> per-chip data to collect together what lines actually have
> physical pins attached and map the right names to them.
>
> The core peripherals seem to use the same lines on the
> currently known chips but the lines used for the sensor
> interface, lcd controller etc pins seem to be totally
> different between the infinity and mercury chips
>
> The code tries to collect all of the re-usable names,
> offsets etc together so that it's easy to build the extra
> per-chip data for other chips in the future.
>
> So far this only supports the MSC313 and MSC313E chips.
>
> Support for the SSC8336N (mercury5) is trivial to add once
> all of the lines have been mapped out.
>
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>

(...)

> +config GPIO_MSC313
> +       bool "MStar MSC313 GPIO support"
> +       default y if ARCH_MSTARV7
> +       depends on ARCH_MSTARV7
> +       select GPIO_GENERIC

Selecting GPIO_GENERIC, that is good.
But you're not using it, because you can't.
This chip does not have the bits lined up nicely
in one register, instead there seems to be something
like one register per line, right?
So skip GPIO_GENERIC.

> +#define MSC313_GPIO_IN  BIT(0)
> +#define MSC313_GPIO_OUT BIT(4)
> +#define MSC313_GPIO_OEN BIT(5)
> +
> +#define MSC313_GPIO_BITSTOSAVE (MSC313_GPIO_OUT | MSC313_GPIO_OEN)

Some comment here telling us why these need saving and
not others.

> +#define FUART_NAMES                    \
> +       MSC313_PINNAME_FUART_RX,        \
> +       MSC313_PINNAME_FUART_TX,        \
> +       MSC313_PINNAME_FUART_CTS,       \
> +       MSC313_PINNAME_FUART_RTS
> +
> +#define OFF_FUART_RX   0x50
> +#define OFF_FUART_TX   0x54
> +#define OFF_FUART_CTS  0x58
> +#define OFF_FUART_RTS  0x5c
> +
> +#define FUART_OFFSETS  \
> +       OFF_FUART_RX,   \
> +       OFF_FUART_TX,   \
> +       OFF_FUART_CTS,  \
> +       OFF_FUART_RTS

This looks a bit strange. The GPIO driver should not really
have to know about any other use cases for pins than
GPIO. But I guess it is intuitive for the driver.

> +#define SD_NAMES               \
> +       MSC313_PINNAME_SD_CLK,  \
> +       MSC313_PINNAME_SD_CMD,  \
> +       MSC313_PINNAME_SD_D0,   \
> +       MSC313_PINNAME_SD_D1,   \
> +       MSC313_PINNAME_SD_D2,   \
> +       MSC313_PINNAME_SD_D3
> +
> +#define OFF_SD_CLK     0x140
> +#define OFF_SD_CMD     0x144
> +#define OFF_SD_D0      0x148
> +#define OFF_SD_D1      0x14cchild_to_parent_hwirq
> +#define OFF_SD_D2      0x150
> +#define OFF_SD_D3      0x154
> +
> +#define SD_OFFSETS     \
> +       OFF_SD_CLK,     \
> +       OFF_SD_CMD,     \
> +       OFF_SD_D0,      \
> +       OFF_SD_D1,      \
> +       OFF_SD_D2,      \
> +       OFF_SD_D3
> +
> +#define I2C1_NAMES                     \
> +       MSC313_PINNAME_I2C1_SCL,        \
> +       MSC313_PINNAME_I2C1_SCA
> +
> +#define OFF_I2C1_SCL   0x188
> +#define OFF_I2C1_SCA   0x18c
> +
> +#define I2C1_OFFSETS   \
> +       OFF_I2C1_SCL,   \
> +       OFF_I2C1_SCA
> +
> +#define SPI0_NAMES             \
> +       MSC313_PINNAME_SPI0_CZ, \
> +       MSC313_PINNAME_SPI0_CK, \
> +       MSC313_PINNAME_SPI0_DI, \
> +       MSC313_PINNAME_SPI0_DO
> +
> +#define OFF_SPI0_CZ    0x1c0
> +#define OFF_SPI0_CK    0x1c4
> +#define OFF_SPI0_DI    0x1c8
> +#define OFF_SPI0_DO    0x1cc
> +
> +#define SPI0_OFFSETS   \
> +       OFF_SPI0_CZ,    \
> +       OFF_SPI0_CK,    \
> +       OFF_SPI0_DI,    \
> +       OFF_SPI0_DO

Same with all these. I suppose it is the offsets of stuff
that would be there unless we were using it for GPIO.

> +static int msc313_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct msc313_gpio *gpio = gpiochip_get_data(chip);
> +> +

> +       return gpio->irqs[offset];
> +}

Please do not use custom IRQ handling like this.
As there seems to be one IRQ per line, look into using

        select GPIOLIB_IRQCHIP
        select IRQ_DOMAIN_HIERARCHY

See for example in gpio-ixp4xx.c how we deal with
hiearchical GPIO IRQs.

> +       gpiochip->to_irq = msc313_gpio_to_irq;
> +       gpiochip->base = -1;
> +       gpiochip->ngpio = gpio->gpio_data->num;
> +       gpiochip->names = gpio->gpio_data->names;
> +
> +       for (i = 0; i < gpiochip->ngpio; i++)
> +               gpio->irqs[i] = of_irq_get_byname(pdev->dev.of_node, gpio->gpio_data->names[i]);

Use hierarchical generic GPIO IRQs for these.

Assign ->fwnode, ->parent_domain, ->child_to_parent_hwirq,
and probably also ->handler on the struct gpio_irq_chip *.

Skip assigning gpiochip->to_irq, the generic code will
handle that.

Again see gpio-ixp4xx.c for an example.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
  2020-10-16 16:56     ` Linus Walleij
@ 2020-10-17  1:57       ` Daniel Palmer
  -1 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-17  1:57 UTC (permalink / raw)
  To: Linus Walleij
  Cc: open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linux-kernel

Hi Linus

On Sat, 17 Oct 2020 at 01:56, Linus Walleij <linus.walleij@linaro.org> wrote:
> (...)
>
> > +config GPIO_MSC313
> > +       bool "MStar MSC313 GPIO support"
> > +       default y if ARCH_MSTARV7
> > +       depends on ARCH_MSTARV7
> > +       select GPIO_GENERIC
>
> Selecting GPIO_GENERIC, that is good.
> But you're not using it, because you can't.
> This chip does not have the bits lined up nicely
> in one register, instead there seems to be something
> like one register per line, right?
> So skip GPIO_GENERIC.

Well spotted. Copy/paste fail on my side :).

> > +#define MSC313_GPIO_IN  BIT(0)
> > +#define MSC313_GPIO_OUT BIT(4)
> > +#define MSC313_GPIO_OEN BIT(5)
> > +
> > +#define MSC313_GPIO_BITSTOSAVE (MSC313_GPIO_OUT | MSC313_GPIO_OEN)
>
> Some comment here telling us why these need saving and
> not others.

There is a comment near to the save function that explains it I think.
When the hardware goes into low power mode with the CPU turned off
the register contents are lost and those two bits are the only ones that are
writable from what I can tell. I'll add an extra comment above that line.

> > +#define FUART_NAMES                    \
> > +       MSC313_PINNAME_FUART_RX,        \
> > +       MSC313_PINNAME_FUART_TX,        \
> > +       MSC313_PINNAME_FUART_CTS,       \
> > +       MSC313_PINNAME_FUART_RTS
> > +
> > +#define OFF_FUART_RX   0x50
> > +#define OFF_FUART_TX   0x54
> > +#define OFF_FUART_CTS  0x58
> > +#define OFF_FUART_RTS  0x5c
> > +
> > +#define FUART_OFFSETS  \
> > +       OFF_FUART_RX,   \
> > +       OFF_FUART_TX,   \
> > +       OFF_FUART_CTS,  \
> > +       OFF_FUART_RTS
>
> This looks a bit strange. The GPIO driver should not really
> have to know about any other use cases for pins than
> GPIO. But I guess it is intuitive for the driver.
>
<snip>
>
> Same with all these. I suppose it is the offsets of stuff
> that would be there unless we were using it for GPIO.

The pad FUART_RX can't move but the function FUART_RX can.
If the function FUART_RX (or another function) isn't on the pad/pin
FUART_RX it's connected to the GPIO block.
Even more confusingly some of the other chips (SSD201/SSD202)
have pads called GPIO1, GPIO2 etc that only have GPIO functionality
but the offsets of the registers to control the GPIO on those pads might
not have a relation to the name.
GPIO1 isn't gpio_base + (1 * 4) and instead some random address.

Basically using the pad name as the name of the GPIO made sense
because it's fixed and the pad name and offset are the same with all
of the chips I've seen so far.

> > +static int msc313_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
> > +{
> > +       struct msc313_gpio *gpio = gpiochip_get_data(chip);
> > +> +
>
> > +       return gpio->irqs[offset];
> > +}
>
> Please do not use custom IRQ handling like this.
> As there seems to be one IRQ per line, look into using
>
>         select GPIOLIB_IRQCHIP
>         select IRQ_DOMAIN_HIERARCHY
>
> See for example in gpio-ixp4xx.c how we deal with
> hiearchical GPIO IRQs.

<snip>

> Use hierarchical generic GPIO IRQs for these.
>
> Assign ->fwnode, ->parent_domain, ->child_to_parent_hwirq,
> and probably also ->handler on the struct gpio_irq_chip *.
>
> Skip assigning gpiochip->to_irq, the generic code will
> handle that.
>
> Again see gpio-ixp4xx.c for an example.

I'll look into this.
I don't have datasheets so I'm working from some crusty header
files from the vendor kernel but there isn't one irq per line from
what I can tell.
There seems to have been 4 spare lines on an interrupt controller
so they wired GPIOs to them.

Thank you for the comments. I'll send a v2 in a few days.

Thanks,

Daniel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
@ 2020-10-17  1:57       ` Daniel Palmer
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-17  1:57 UTC (permalink / raw)
  To: Linus Walleij
  Cc: open list:GPIO SUBSYSTEM, linux-kernel, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Linus

On Sat, 17 Oct 2020 at 01:56, Linus Walleij <linus.walleij@linaro.org> wrote:
> (...)
>
> > +config GPIO_MSC313
> > +       bool "MStar MSC313 GPIO support"
> > +       default y if ARCH_MSTARV7
> > +       depends on ARCH_MSTARV7
> > +       select GPIO_GENERIC
>
> Selecting GPIO_GENERIC, that is good.
> But you're not using it, because you can't.
> This chip does not have the bits lined up nicely
> in one register, instead there seems to be something
> like one register per line, right?
> So skip GPIO_GENERIC.

Well spotted. Copy/paste fail on my side :).

> > +#define MSC313_GPIO_IN  BIT(0)
> > +#define MSC313_GPIO_OUT BIT(4)
> > +#define MSC313_GPIO_OEN BIT(5)
> > +
> > +#define MSC313_GPIO_BITSTOSAVE (MSC313_GPIO_OUT | MSC313_GPIO_OEN)
>
> Some comment here telling us why these need saving and
> not others.

There is a comment near to the save function that explains it I think.
When the hardware goes into low power mode with the CPU turned off
the register contents are lost and those two bits are the only ones that are
writable from what I can tell. I'll add an extra comment above that line.

> > +#define FUART_NAMES                    \
> > +       MSC313_PINNAME_FUART_RX,        \
> > +       MSC313_PINNAME_FUART_TX,        \
> > +       MSC313_PINNAME_FUART_CTS,       \
> > +       MSC313_PINNAME_FUART_RTS
> > +
> > +#define OFF_FUART_RX   0x50
> > +#define OFF_FUART_TX   0x54
> > +#define OFF_FUART_CTS  0x58
> > +#define OFF_FUART_RTS  0x5c
> > +
> > +#define FUART_OFFSETS  \
> > +       OFF_FUART_RX,   \
> > +       OFF_FUART_TX,   \
> > +       OFF_FUART_CTS,  \
> > +       OFF_FUART_RTS
>
> This looks a bit strange. The GPIO driver should not really
> have to know about any other use cases for pins than
> GPIO. But I guess it is intuitive for the driver.
>
<snip>
>
> Same with all these. I suppose it is the offsets of stuff
> that would be there unless we were using it for GPIO.

The pad FUART_RX can't move but the function FUART_RX can.
If the function FUART_RX (or another function) isn't on the pad/pin
FUART_RX it's connected to the GPIO block.
Even more confusingly some of the other chips (SSD201/SSD202)
have pads called GPIO1, GPIO2 etc that only have GPIO functionality
but the offsets of the registers to control the GPIO on those pads might
not have a relation to the name.
GPIO1 isn't gpio_base + (1 * 4) and instead some random address.

Basically using the pad name as the name of the GPIO made sense
because it's fixed and the pad name and offset are the same with all
of the chips I've seen so far.

> > +static int msc313_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
> > +{
> > +       struct msc313_gpio *gpio = gpiochip_get_data(chip);
> > +> +
>
> > +       return gpio->irqs[offset];
> > +}
>
> Please do not use custom IRQ handling like this.
> As there seems to be one IRQ per line, look into using
>
>         select GPIOLIB_IRQCHIP
>         select IRQ_DOMAIN_HIERARCHY
>
> See for example in gpio-ixp4xx.c how we deal with
> hiearchical GPIO IRQs.

<snip>

> Use hierarchical generic GPIO IRQs for these.
>
> Assign ->fwnode, ->parent_domain, ->child_to_parent_hwirq,
> and probably also ->handler on the struct gpio_irq_chip *.
>
> Skip assigning gpiochip->to_irq, the generic code will
> handle that.
>
> Again see gpio-ixp4xx.c for an example.

I'll look into this.
I don't have datasheets so I'm working from some crusty header
files from the vendor kernel but there isn't one irq per line from
what I can tell.
There seems to have been 4 spare lines on an interrupt controller
so they wired GPIOs to them.

Thank you for the comments. I'll send a v2 in a few days.

Thanks,

Daniel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 1/5] dt-bindings: gpio: Binding for MStar MSC313 GPIO controller
  2020-10-16 16:36     ` Linus Walleij
@ 2020-10-19 16:13       ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 40+ messages in thread
From: Krzysztof Kozlowski @ 2020-10-19 16:13 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Daniel Palmer, open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linux-kernel

On Fri, 16 Oct 2020 at 18:36, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Sun, Oct 11, 2020 at 4:48 AM Daniel Palmer <daniel@0x0f.com> wrote:
>
> > Add a binding description for the MStar/SigmaStar GPIO controller
> > found in the MSC313 and later ARMv7 SoCs.
> >
> > Signed-off-by: Daniel Palmer <daniel@0x0f.com>
>
> I think Krzysztof is working on some generic bindings that
> will make it easier to write YAML GPIO controller bindings,
> but I don't know the status of them. I would be happy to merge
> them early for v5.11 though.

Hi,

The generic GPIO controller dtschema got dropped because Rob wants it
to be part of dtschema (outside of kernel) and then
relicensing/rewriting property descriptions plays a role. Only the
GPIO hogs went to common dtschema package.

Therefore as of now, one should include all generic properties
directly in the GPIO controller bindings.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 1/5] dt-bindings: gpio: Binding for MStar MSC313 GPIO controller
@ 2020-10-19 16:13       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 40+ messages in thread
From: Krzysztof Kozlowski @ 2020-10-19 16:13 UTC (permalink / raw)
  To: Linus Walleij
  Cc: open list:GPIO SUBSYSTEM, Daniel Palmer, linux-kernel, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, 16 Oct 2020 at 18:36, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Sun, Oct 11, 2020 at 4:48 AM Daniel Palmer <daniel@0x0f.com> wrote:
>
> > Add a binding description for the MStar/SigmaStar GPIO controller
> > found in the MSC313 and later ARMv7 SoCs.
> >
> > Signed-off-by: Daniel Palmer <daniel@0x0f.com>
>
> I think Krzysztof is working on some generic bindings that
> will make it easier to write YAML GPIO controller bindings,
> but I don't know the status of them. I would be happy to merge
> them early for v5.11 though.

Hi,

The generic GPIO controller dtschema got dropped because Rob wants it
to be part of dtschema (outside of kernel) and then
relicensing/rewriting property descriptions plays a role. Only the
GPIO hogs went to common dtschema package.

Therefore as of now, one should include all generic properties
directly in the GPIO controller bindings.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
  2020-10-16 16:56     ` Linus Walleij
@ 2020-10-21 11:07       ` Daniel Palmer
  -1 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-21 11:07 UTC (permalink / raw)
  To: Linus Walleij
  Cc: open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linux-kernel

Hi Linus,

Sorry to pester you again...

On Sat, 17 Oct 2020 at 01:56, Linus Walleij <linus.walleij@linaro.org> wrote:

> > +       gpiochip->to_irq = msc313_gpio_to_irq;
> > +       gpiochip->base = -1;
> > +       gpiochip->ngpio = gpio->gpio_data->num;
> > +       gpiochip->names = gpio->gpio_data->names;
> > +
> > +       for (i = 0; i < gpiochip->ngpio; i++)
> > +               gpio->irqs[i] = of_irq_get_byname(pdev->dev.of_node, gpio->gpio_data->names[i]);
>
> Use hierarchical generic GPIO IRQs for these.
>
> Assign ->fwnode, ->parent_domain, ->child_to_parent_hwirq,
> and probably also ->handler on the struct gpio_irq_chip *.
>
> Skip assigning gpiochip->to_irq, the generic code will
> handle that.
>
> Again see gpio-ixp4xx.c for an example.

I sent a v2 with this conversion already and it looks a lot better.
Based on Andy Shevchenko's comments[0] I'll be sending a v3 that fixes
up all style and other issues he found.
Before I do that I have a question that maybe you could help me with:
Andy noted a few times that I have this driver as a built in driver
and not a module.
The gpio-ixp4xx.c driver is also a built in driver. Is there a reason
why it's ok there but not this driver?
I've actually changed it to allow building as a module already but I
don't want to push a v3 if something like the interrupt handling means
it should actually be a built in and I'm just missing something.

Thanks,

Daniel

0 - https://lore.kernel.org/linux-gpio/CAHp75Vf5iUzKp32CqBbv_5MRo8q8CyBPsBcgzKsww6BFtGJwUA@mail.gmail.com/

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
@ 2020-10-21 11:07       ` Daniel Palmer
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-10-21 11:07 UTC (permalink / raw)
  To: Linus Walleij
  Cc: open list:GPIO SUBSYSTEM, linux-kernel, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Linus,

Sorry to pester you again...

On Sat, 17 Oct 2020 at 01:56, Linus Walleij <linus.walleij@linaro.org> wrote:

> > +       gpiochip->to_irq = msc313_gpio_to_irq;
> > +       gpiochip->base = -1;
> > +       gpiochip->ngpio = gpio->gpio_data->num;
> > +       gpiochip->names = gpio->gpio_data->names;
> > +
> > +       for (i = 0; i < gpiochip->ngpio; i++)
> > +               gpio->irqs[i] = of_irq_get_byname(pdev->dev.of_node, gpio->gpio_data->names[i]);
>
> Use hierarchical generic GPIO IRQs for these.
>
> Assign ->fwnode, ->parent_domain, ->child_to_parent_hwirq,
> and probably also ->handler on the struct gpio_irq_chip *.
>
> Skip assigning gpiochip->to_irq, the generic code will
> handle that.
>
> Again see gpio-ixp4xx.c for an example.

I sent a v2 with this conversion already and it looks a lot better.
Based on Andy Shevchenko's comments[0] I'll be sending a v3 that fixes
up all style and other issues he found.
Before I do that I have a question that maybe you could help me with:
Andy noted a few times that I have this driver as a built in driver
and not a module.
The gpio-ixp4xx.c driver is also a built in driver. Is there a reason
why it's ok there but not this driver?
I've actually changed it to allow building as a module already but I
don't want to push a v3 if something like the interrupt handling means
it should actually be a built in and I'm just missing something.

Thanks,

Daniel

0 - https://lore.kernel.org/linux-gpio/CAHp75Vf5iUzKp32CqBbv_5MRo8q8CyBPsBcgzKsww6BFtGJwUA@mail.gmail.com/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 1/5] dt-bindings: gpio: Binding for MStar MSC313 GPIO controller
  2020-10-19 16:13       ` Krzysztof Kozlowski
@ 2020-11-05  9:13         ` Linus Walleij
  -1 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2020-11-05  9:13 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring
  Cc: Daniel Palmer, open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linux-kernel

On Mon, Oct 19, 2020 at 6:13 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:

> The generic GPIO controller dtschema got dropped because Rob wants it
> to be part of dtschema (outside of kernel) and then
> relicensing/rewriting property descriptions plays a role. Only the
> GPIO hogs went to common dtschema package.
>
> Therefore as of now, one should include all generic properties
> directly in the GPIO controller bindings.

Oh now I am confused.

Rob, what is the plan here?

Am I *not* to create say gpio-controller.yaml for $ref:in into
other controllers?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 1/5] dt-bindings: gpio: Binding for MStar MSC313 GPIO controller
@ 2020-11-05  9:13         ` Linus Walleij
  0 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2020-11-05  9:13 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring
  Cc: open list:GPIO SUBSYSTEM, Daniel Palmer, linux-kernel, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Mon, Oct 19, 2020 at 6:13 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:

> The generic GPIO controller dtschema got dropped because Rob wants it
> to be part of dtschema (outside of kernel) and then
> relicensing/rewriting property descriptions plays a role. Only the
> GPIO hogs went to common dtschema package.
>
> Therefore as of now, one should include all generic properties
> directly in the GPIO controller bindings.

Oh now I am confused.

Rob, what is the plan here?

Am I *not* to create say gpio-controller.yaml for $ref:in into
other controllers?

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
  2020-10-21 11:07       ` Daniel Palmer
@ 2020-11-05  9:21         ` Linus Walleij
  -1 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2020-11-05  9:21 UTC (permalink / raw)
  To: Daniel Palmer
  Cc: open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linux-kernel

On Wed, Oct 21, 2020 at 1:07 PM Daniel Palmer <daniel@0x0f.com> wrote:

> Sorry to pester you again...

Don't worry. I'm more worried that my replies are slow.

> Before I do that I have a question that maybe you could help me with:
> Andy noted a few times that I have this driver as a built in driver
> and not a module.
> The gpio-ixp4xx.c driver is also a built in driver. Is there a reason
> why it's ok there but not this driver?

Not that I know of. There is a lot of push for modularization right
now because Android (and other distributions) likes it, so if your
SoC could be used by Android or Fedora or Debian etc it is
generally a good idea to modularize.

These distributions use the generic ARM (etc) kernel and try
to load as many drivers as possible as modules.

It is not always possible because some GPIOs might be needed
very early, such as on-chip GPIO. So you better make sure
that the platform can get to userspace also without this driver
compiled in, otherwise it *MUST* be bool so people don't get
ammunition to shoot themselves in the foot and configure a
non-bootable kernel just because they could modularize this
driver.

If your SoC is only used by OpenWrt (like ixp4xx) then it is fine
to just use bool because that distribution is always built with an
image for a specific hardware, whereas distributions are generic.

So it actually depends a bit on the usecase of the SoC.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
@ 2020-11-05  9:21         ` Linus Walleij
  0 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2020-11-05  9:21 UTC (permalink / raw)
  To: Daniel Palmer
  Cc: open list:GPIO SUBSYSTEM, linux-kernel, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Wed, Oct 21, 2020 at 1:07 PM Daniel Palmer <daniel@0x0f.com> wrote:

> Sorry to pester you again...

Don't worry. I'm more worried that my replies are slow.

> Before I do that I have a question that maybe you could help me with:
> Andy noted a few times that I have this driver as a built in driver
> and not a module.
> The gpio-ixp4xx.c driver is also a built in driver. Is there a reason
> why it's ok there but not this driver?

Not that I know of. There is a lot of push for modularization right
now because Android (and other distributions) likes it, so if your
SoC could be used by Android or Fedora or Debian etc it is
generally a good idea to modularize.

These distributions use the generic ARM (etc) kernel and try
to load as many drivers as possible as modules.

It is not always possible because some GPIOs might be needed
very early, such as on-chip GPIO. So you better make sure
that the platform can get to userspace also without this driver
compiled in, otherwise it *MUST* be bool so people don't get
ammunition to shoot themselves in the foot and configure a
non-bootable kernel just because they could modularize this
driver.

If your SoC is only used by OpenWrt (like ixp4xx) then it is fine
to just use bool because that distribution is always built with an
image for a specific hardware, whereas distributions are generic.

So it actually depends a bit on the usecase of the SoC.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
  2020-11-05  9:21         ` Linus Walleij
@ 2020-11-05  9:31           ` Willy Tarreau
  -1 siblings, 0 replies; 40+ messages in thread
From: Willy Tarreau @ 2020-11-05  9:31 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Daniel Palmer, open list:GPIO SUBSYSTEM, linux-kernel, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Thu, Nov 05, 2020 at 10:21:27AM +0100, Linus Walleij wrote:
> If your SoC is only used by OpenWrt (like ixp4xx) then it is fine
> to just use bool because that distribution is always built with an
> image for a specific hardware, whereas distributions are generic.

Speaking for myself (since I have a few now), I'm not running OpenWRT
on mine but my own distro, and I guess most users will run either
Buildroot or their own distro. It's unlikely that we'll see very
generic distros there given the limited storage you'd typically have
in an SPI NOR (16-32 MB) and the small RAM (64MB) which tends to
discourage anyone from booting a regular distro over other storage
anyway.

Thus my guess is that most users will keep building their own kernels.

But this just emphasizes your points :-)

Just my two cents,
Willy

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
@ 2020-11-05  9:31           ` Willy Tarreau
  0 siblings, 0 replies; 40+ messages in thread
From: Willy Tarreau @ 2020-11-05  9:31 UTC (permalink / raw)
  To: Linus Walleij
  Cc: open list:GPIO SUBSYSTEM, Daniel Palmer, linux-kernel, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Thu, Nov 05, 2020 at 10:21:27AM +0100, Linus Walleij wrote:
> If your SoC is only used by OpenWrt (like ixp4xx) then it is fine
> to just use bool because that distribution is always built with an
> image for a specific hardware, whereas distributions are generic.

Speaking for myself (since I have a few now), I'm not running OpenWRT
on mine but my own distro, and I guess most users will run either
Buildroot or their own distro. It's unlikely that we'll see very
generic distros there given the limited storage you'd typically have
in an SPI NOR (16-32 MB) and the small RAM (64MB) which tends to
discourage anyone from booting a regular distro over other storage
anyway.

Thus my guess is that most users will keep building their own kernels.

But this just emphasizes your points :-)

Just my two cents,
Willy

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
  2020-11-05  9:31           ` Willy Tarreau
@ 2020-11-05  9:42             ` Linus Walleij
  -1 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2020-11-05  9:42 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Daniel Palmer, open list:GPIO SUBSYSTEM, linux-kernel, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Thu, Nov 5, 2020 at 10:31 AM Willy Tarreau <w@1wt.eu> wrote:
> On Thu, Nov 05, 2020 at 10:21:27AM +0100, Linus Walleij wrote:

> > If your SoC is only used by OpenWrt (like ixp4xx) then it is fine
> > to just use bool because that distribution is always built with an
> > image for a specific hardware, whereas distributions are generic.
>
> Speaking for myself (since I have a few now), I'm not running OpenWRT
> on mine but my own distro, and I guess most users will run either
> Buildroot or their own distro. It's unlikely that we'll see very
> generic distros there given the limited storage you'd typically have
> in an SPI NOR (16-32 MB) and the small RAM (64MB) which tends to
> discourage anyone from booting a regular distro over other storage
> anyway.
>
> Thus my guess is that most users will keep building their own kernels.
>
> But this just emphasizes your points :-)

I think that is a good argument to keep this as bool.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
@ 2020-11-05  9:42             ` Linus Walleij
  0 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2020-11-05  9:42 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: open list:GPIO SUBSYSTEM, Daniel Palmer, linux-kernel, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Thu, Nov 5, 2020 at 10:31 AM Willy Tarreau <w@1wt.eu> wrote:
> On Thu, Nov 05, 2020 at 10:21:27AM +0100, Linus Walleij wrote:

> > If your SoC is only used by OpenWrt (like ixp4xx) then it is fine
> > to just use bool because that distribution is always built with an
> > image for a specific hardware, whereas distributions are generic.
>
> Speaking for myself (since I have a few now), I'm not running OpenWRT
> on mine but my own distro, and I guess most users will run either
> Buildroot or their own distro. It's unlikely that we'll see very
> generic distros there given the limited storage you'd typically have
> in an SPI NOR (16-32 MB) and the small RAM (64MB) which tends to
> discourage anyone from booting a regular distro over other storage
> anyway.
>
> Thus my guess is that most users will keep building their own kernels.
>
> But this just emphasizes your points :-)

I think that is a good argument to keep this as bool.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
  2020-11-05  9:42             ` Linus Walleij
@ 2020-11-05 15:39               ` Daniel Palmer
  -1 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-11-05 15:39 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Willy Tarreau, open list:GPIO SUBSYSTEM, linux-kernel, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Linus,

Thanks for all of the comments.

On Thu, 5 Nov 2020 at 18:42, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Nov 5, 2020 at 10:31 AM Willy Tarreau <w@1wt.eu> wrote:
> > On Thu, Nov 05, 2020 at 10:21:27AM +0100, Linus Walleij wrote:
>
> > > If your SoC is only used by OpenWrt (like ixp4xx) then it is fine
> > > to just use bool because that distribution is always built with an
> > > image for a specific hardware, whereas distributions are generic.
> >
.. snip ..
>> It's unlikely that we'll see very
> > generic distros there given the limited storage you'd typically have
> > in an SPI NOR (16-32 MB) and the small RAM (64MB) which tends to
> > discourage anyone from booting a regular distro over other storage
> > anyway.
> >
> > Thus my guess is that most users will keep building their own kernels.
> >
> > But this just emphasizes your points :-)
>
> I think that is a good argument to keep this as bool.

Thanks. I did change it to a tristate for v3 but I'll change it back.

Just a heads up:
There is another GPIO driver for this chip (same functionality,
totally different register layout for no reason) that'll look pretty
similar to this that'll follow soon.
It might be similar enough that people confuse the two series as the same thing.

Thanks,

Daniel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] gpio: msc313: MStar MSC313 GPIO driver
@ 2020-11-05 15:39               ` Daniel Palmer
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Palmer @ 2020-11-05 15:39 UTC (permalink / raw)
  To: Linus Walleij
  Cc: open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Willy Tarreau, Linux ARM, linux-kernel

Hi Linus,

Thanks for all of the comments.

On Thu, 5 Nov 2020 at 18:42, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Nov 5, 2020 at 10:31 AM Willy Tarreau <w@1wt.eu> wrote:
> > On Thu, Nov 05, 2020 at 10:21:27AM +0100, Linus Walleij wrote:
>
> > > If your SoC is only used by OpenWrt (like ixp4xx) then it is fine
> > > to just use bool because that distribution is always built with an
> > > image for a specific hardware, whereas distributions are generic.
> >
.. snip ..
>> It's unlikely that we'll see very
> > generic distros there given the limited storage you'd typically have
> > in an SPI NOR (16-32 MB) and the small RAM (64MB) which tends to
> > discourage anyone from booting a regular distro over other storage
> > anyway.
> >
> > Thus my guess is that most users will keep building their own kernels.
> >
> > But this just emphasizes your points :-)
>
> I think that is a good argument to keep this as bool.

Thanks. I did change it to a tristate for v3 but I'll change it back.

Just a heads up:
There is another GPIO driver for this chip (same functionality,
totally different register layout for no reason) that'll look pretty
similar to this that'll follow soon.
It might be similar enough that people confuse the two series as the same thing.

Thanks,

Daniel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2020-11-05 15:39 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-11  2:48 [PATCH 0/5] Add GPIO support for MStar/SigmaStar ARMv7 Daniel Palmer
2020-10-11  2:48 ` Daniel Palmer
2020-10-11  2:48 ` [PATCH 1/5] dt-bindings: gpio: Binding for MStar MSC313 GPIO controller Daniel Palmer
2020-10-11  2:48   ` Daniel Palmer
2020-10-12 16:10   ` Rob Herring
2020-10-12 16:10     ` Rob Herring
2020-10-16 16:36   ` Linus Walleij
2020-10-16 16:36     ` Linus Walleij
2020-10-19 16:13     ` Krzysztof Kozlowski
2020-10-19 16:13       ` Krzysztof Kozlowski
2020-11-05  9:13       ` Linus Walleij
2020-11-05  9:13         ` Linus Walleij
2020-10-11  2:48 ` [PATCH 2/5] dt-bindings: gpio: Add a binding header for the MSC313 GPIO driver Daniel Palmer
2020-10-11  2:48   ` Daniel Palmer
2020-10-12 16:11   ` Rob Herring
2020-10-12 16:11     ` Rob Herring
2020-10-14  9:45     ` Daniel Palmer
2020-10-14  9:45       ` Daniel Palmer
2020-10-14 12:17       ` Rob Herring
2020-10-14 12:17         ` Rob Herring
2020-10-11  2:48 ` [PATCH 3/5] gpio: msc313: MStar " Daniel Palmer
2020-10-11  2:48   ` Daniel Palmer
2020-10-16 16:56   ` Linus Walleij
2020-10-16 16:56     ` Linus Walleij
2020-10-17  1:57     ` Daniel Palmer
2020-10-17  1:57       ` Daniel Palmer
2020-10-21 11:07     ` Daniel Palmer
2020-10-21 11:07       ` Daniel Palmer
2020-11-05  9:21       ` Linus Walleij
2020-11-05  9:21         ` Linus Walleij
2020-11-05  9:31         ` Willy Tarreau
2020-11-05  9:31           ` Willy Tarreau
2020-11-05  9:42           ` Linus Walleij
2020-11-05  9:42             ` Linus Walleij
2020-11-05 15:39             ` Daniel Palmer
2020-11-05 15:39               ` Daniel Palmer
2020-10-11  2:48 ` [PATCH 4/5] ARM: mstar: Add gpio controller to MStar base dtsi Daniel Palmer
2020-10-11  2:48   ` Daniel Palmer
2020-10-11  2:48 ` [PATCH 5/5] ARM: mstar: Fill in GPIO controller properties for infinity Daniel Palmer
2020-10-11  2:48   ` Daniel Palmer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.