diff mbox series

[v3,1/5] dt-bindings: aspeed-sgpio: Convert txt bindings to yaml.

Message ID 20210603101822.9645-2-steven_lee@aspeedtech.com
State Superseded
Headers show
Series ASPEED sgpio driver enhancement. | expand

Commit Message

Steven Lee June 3, 2021, 10:18 a.m. UTC
sgpio-aspeed bindings should be converted to yaml format.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
 .../bindings/gpio/aspeed,sgpio.yaml           | 78 +++++++++++++++++++
 .../devicetree/bindings/gpio/sgpio-aspeed.txt | 46 -----------
 2 files changed, 78 insertions(+), 46 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
 delete mode 100644 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt

Comments

Steven Lee June 4, 2021, 3:30 a.m. UTC | #1
The 06/04/2021 07:25, Andrew Jeffery wrote:
> Hi Steven,
> 
> On Thu, 3 Jun 2021, at 19:48, Steven Lee wrote:
> > sgpio-aspeed bindings should be converted to yaml format.
> > 
> > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> > ---
> >  .../bindings/gpio/aspeed,sgpio.yaml           | 78 +++++++++++++++++++
> >  .../devicetree/bindings/gpio/sgpio-aspeed.txt | 46 -----------
> >  2 files changed, 78 insertions(+), 46 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml 
> > b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
> > new file mode 100644
> > index 000000000000..e7c2113cc096
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
> > @@ -0,0 +1,78 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/gpio/aspeed,sgpio.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Aspeed SGPIO controller
> > +
> > +maintainers:
> > +  - Andrew Jeffery <andrew@aj.id.au>
> > +
> > +description:
> > +  This SGPIO controller is for ASPEED AST2400, AST2500 and AST2600 SoC,
> > +  AST2600 have two sgpio master one with 128 pins another one with 80 
> > pins,
> > +  AST2500/AST2400 have one sgpio master with 80 pins. Each of the 
> > Serial
> > +  GPIO pins can be programmed to support the following options
> > +  - Support interrupt option for each input port and various interrupt
> > +    sensitivity option (level-high, level-low, edge-high, edge-low)
> > +  - Support reset tolerance option for each output port
> > +  - Directly connected to APB bus and its shift clock is from APB bus 
> > clock
> > +    divided by a programmable value.
> > +  - Co-work with external signal-chained TTL components 
> > (74LV165/74LV595)
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - aspeed,ast2400-sgpio
> > +      - aspeed,ast2500-sgpio
> > +      - aspeed,ast2600-sgpiom1
> > +      - aspeed,ast2600-sgpiom2
> 
> You should have followed Rob's request here and made two patches for 
> the binding document:
> 
> 1. A 1-to-1 conversion of the text file to dt-schema
> 2. Add your new compatibles for the 2600.
> 

Sorry I forgot to remove compatibles and move them to a new patch.

> From a cursory glance it looks okay except for the new compatibles.
> 
> Regarding the compatibles, I'd prefer we use something a bit more 
> meaningful. What do you think of these?
> 
> - aspeed,ast2600-sgpiom-80
> - aspeed,ast2600-sgpiom-128
> 

Ok, I will change the name as you suggested.

BTW, I and development team have an internal discussion about the
current sgpio design.

In the current design, the base offset of gpio input and output
are calculated by the maximum number of gpio pins that SoC supported.
For instance, in AST2500, max_ngpios is 80(defined in MAX_NR_HW_SGPIO),
if ngpios is 16 in dts, gpio input pin id is from 0 to 15 and
gpio output pin id is from 80 to 95.

We are thinking of removing max_ngpios(and MAX_NR_HW_SGPIO) and
corresponding design to make the gpio input and output pin base
are determined by ngpios.
For instance, in any AST SoC, if ngpios is 16 in dts,
gpio input pin id is from 0 to 15 and gpio output pin id is from 16 to 31.
Thus we don't need to care about the max_ngpios of SoCs, and needn't to
add 2 compatibles for ast2600.

However, it might affect users who update kernel/driver from the
old kernel/driver as they may expect the gpio output pin base is start
from 80(MAX_NR_HW_SGPIO).
I was wondering if it is better to change the design as above.
It would be great to have your suggestion.

Thanks,
Steven

> Cheers,
> 
> Andrew
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
new file mode 100644
index 000000000000..e7c2113cc096
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
@@ -0,0 +1,78 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/aspeed,sgpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Aspeed SGPIO controller
+
+maintainers:
+  - Andrew Jeffery <andrew@aj.id.au>
+
+description:
+  This SGPIO controller is for ASPEED AST2400, AST2500 and AST2600 SoC,
+  AST2600 have two sgpio master one with 128 pins another one with 80 pins,
+  AST2500/AST2400 have one sgpio master with 80 pins. Each of the Serial
+  GPIO pins can be programmed to support the following options
+  - Support interrupt option for each input port and various interrupt
+    sensitivity option (level-high, level-low, edge-high, edge-low)
+  - Support reset tolerance option for each output port
+  - Directly connected to APB bus and its shift clock is from APB bus clock
+    divided by a programmable value.
+  - Co-work with external signal-chained TTL components (74LV165/74LV595)
+
+properties:
+  compatible:
+    enum:
+      - aspeed,ast2400-sgpio
+      - aspeed,ast2500-sgpio
+      - aspeed,ast2600-sgpiom1
+      - aspeed,ast2600-sgpiom2
+
+  reg:
+    maxItems: 1
+
+  gpio-controller: true
+
+  '#gpio-cells':
+    const: 2
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  clocks:
+    maxItems: 1
+
+  ngpios: true
+
+  bus-frequency: true
+
+required:
+  - compatible
+  - reg
+  - gpio-controller
+  - '#gpio-cells'
+  - interrupts
+  - interrupt-controller
+  - ngpios
+  - clocks
+  - bus-frequency
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/aspeed-clock.h>
+    sgpio: sgpio@1e780200 {
+        #gpio-cells = <2>;
+        compatible = "aspeed,ast2500-sgpio";
+        gpio-controller;
+        interrupts = <40>;
+        reg = <0x1e780200 0x0100>;
+        clocks = <&syscon ASPEED_CLK_APB>;
+        interrupt-controller;
+        ngpios = <80>;
+        bus-frequency = <12000000>;
+    };
diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
deleted file mode 100644
index be329ea4794f..000000000000
--- a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
+++ /dev/null
@@ -1,46 +0,0 @@ 
-Aspeed SGPIO controller Device Tree Bindings
---------------------------------------------
-
-This SGPIO controller is for ASPEED AST2500 SoC, it supports up to 80 full
-featured Serial GPIOs. Each of the Serial GPIO pins can be programmed to
-support the following options:
-- Support interrupt option for each input port and various interrupt
-  sensitivity option (level-high, level-low, edge-high, edge-low)
-- Support reset tolerance option for each output port
-- Directly connected to APB bus and its shift clock is from APB bus clock
-  divided by a programmable value.
-- Co-work with external signal-chained TTL components (74LV165/74LV595)
-
-Required properties:
-
-- compatible : Should be one of
-  "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio"
-- #gpio-cells : Should be 2, see gpio.txt
-- reg : Address and length of the register set for the device
-- gpio-controller : Marks the device node as a GPIO controller
-- interrupts : Interrupt specifier, see interrupt-controller/interrupts.txt
-- interrupt-controller : Mark the GPIO controller as an interrupt-controller
-- ngpios : number of *hardware* GPIO lines, see gpio.txt. This will expose
-  2 software GPIOs per hardware GPIO: one for hardware input, one for hardware
-  output. Up to 80 pins, must be a multiple of 8.
-- clocks : A phandle to the APB clock for SGPM clock division
-- bus-frequency : SGPM CLK frequency
-
-The sgpio and interrupt properties are further described in their respective
-bindings documentation:
-
-- Documentation/devicetree/bindings/gpio/gpio.txt
-- Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
-
-  Example:
-	sgpio: sgpio@1e780200 {
-		#gpio-cells = <2>;
-		compatible = "aspeed,ast2500-sgpio";
-		gpio-controller;
-		interrupts = <40>;
-		reg = <0x1e780200 0x0100>;
-		clocks = <&syscon ASPEED_CLK_APB>;
-		interrupt-controller;
-		ngpios = <8>;
-		bus-frequency = <12000000>;
-	};