2023-01-10 08:40:04

by Jim Liu

[permalink] [raw]
Subject: [PATCH v4 3/3] dt-bindings: gpio: add NPCM sgpio driver bindings

Add dt-bindings document for the Nuvoton NPCM7xx and NPCM8xx sgpio driver

Signed-off-by: Jim Liu <[email protected]>
---
Changes for v4:
- modify in/out property
- modify bus-frequency property
Changes for v3:
- modify description
- modify in/out property name
Changes for v2:
- modify description
---
.../bindings/gpio/nuvoton,sgpio.yaml | 92 +++++++++++++++++++
1 file changed, 92 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml

diff --git a/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml b/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
new file mode 100644
index 000000000000..3c01ce61f8d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/nuvoton,sgpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton SGPIO controller
+
+maintainers:
+ - Jim LIU <[email protected]>
+
+description:
+ This SGPIO controller is for NUVOTON NPCM7xx and NPCM8xx SoC.
+ Nuvoton NPCM7xx SGPIO module is combine serial to parallel IC (HC595)
+ and parallel to serial IC (HC165), and use APB3 clock to control it.
+ This interface has 4 pins (D_out , D_in, S_CLK, LDSH).
+ NPCM7xx/NPCM8xx have two sgpio module each module can support up
+ to 64 output pins,and up to 64 input pin, the pin is only for gpi or gpo.
+ GPIO pins have sequential, First half is gpo and second half is gpi.
+ 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)
+ - ngpios is number of nuvoton,input-ngpios GPIO lines and nuvoton,output-ngpios GPIO lines.
+ nuvoton,input-ngpios GPIO lines is only for gpi.
+ nuvoton,output-ngpios GPIO lines is only for gpo.
+
+properties:
+ compatible:
+ enum:
+ - nuvoton,npcm750-sgpio
+ - nuvoton,npcm845-sgpio
+
+ reg:
+ maxItems: 1
+
+ gpio-controller: true
+
+ '#gpio-cells':
+ const: 2
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ nuvoton,input-ngpios:
+ description: The numbers of GPIO's exposed.
+ GPIO lines is only for gpi.
+ minimum: 0
+ maximum: 64
+
+ nuvoton,output-ngpios:
+ description: The numbers of GPIO's exposed.
+ GPIO lines is only for gpo.
+ minimum: 0
+ maximum: 64
+
+ bus-frequency:
+ description: Directly connected to APB bus and
+ its shift clock is from APB bus clock divided by a programmable value.
+ default: 8000000
+
+required:
+ - compatible
+ - reg
+ - gpio-controller
+ - '#gpio-cells'
+ - interrupts
+ - nuvoton,input-ngpios
+ - nuvoton,output-ngpios
+ - clocks
+ - bus-frequency
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/nuvoton,npcm7xx-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ gpio8: gpio@101000 {
+ compatible = "nuvoton,npcm750-sgpio";
+ reg = <0x101000 0x200>;
+ clocks = <&clk NPCM7XX_CLK_APB3>;
+ interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+ bus-frequency = <8000000>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ nuvoton,input-ngpios = <64>;
+ nuvoton,output-ngpios = <64>;
+ status = "disabled";
+ };
--
2.17.1


2023-01-10 10:49:29

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] dt-bindings: gpio: add NPCM sgpio driver bindings

On 10/01/2023 09:32, Jim Liu wrote:
> Add dt-bindings document for the Nuvoton NPCM7xx and NPCM8xx sgpio driver
>
> Signed-off-by: Jim Liu <[email protected]>
> ---
> Changes for v4:
> - modify in/out property
> - modify bus-frequency property
> Changes for v3:
> - modify description
> - modify in/out property name
> Changes for v2:
> - modify description
> ---
> .../bindings/gpio/nuvoton,sgpio.yaml | 92 +++++++++++++++++++
> 1 file changed, 92 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
>
> diff --git a/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml b/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
> new file mode 100644
> index 000000000000..3c01ce61f8d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
> @@ -0,0 +1,92 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/nuvoton,sgpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton SGPIO controller
> +
> +maintainers:
> + - Jim LIU <[email protected]>
> +
> +description:
> + This SGPIO controller is for NUVOTON NPCM7xx and NPCM8xx SoC.
> + Nuvoton NPCM7xx SGPIO module is combine serial to parallel IC (HC595)
> + and parallel to serial IC (HC165), and use APB3 clock to control it.
> + This interface has 4 pins (D_out , D_in, S_CLK, LDSH).
> + NPCM7xx/NPCM8xx have two sgpio module each module can support up
> + to 64 output pins,and up to 64 input pin, the pin is only for gpi or gpo.
> + GPIO pins have sequential, First half is gpo and second half is gpi.
> + 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)
> + - ngpios is number of nuvoton,input-ngpios GPIO lines and nuvoton,output-ngpios GPIO lines.
> + nuvoton,input-ngpios GPIO lines is only for gpi.
> + nuvoton,output-ngpios GPIO lines is only for gpo.
> +
> +properties:
> + compatible:
> + enum:
> + - nuvoton,npcm750-sgpio
> + - nuvoton,npcm845-sgpio
> +
> + reg:
> + maxItems: 1
> +
> + gpio-controller: true
> +
> + '#gpio-cells':
> + const: 2
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + nuvoton,input-ngpios:
> + description: The numbers of GPIO's exposed.
> + GPIO lines is only for gpi.
> + minimum: 0
> + maximum: 64
> +
> + nuvoton,output-ngpios:
> + description: The numbers of GPIO's exposed.
> + GPIO lines is only for gpo.
> + minimum: 0
> + maximum: 64
> +
> + bus-frequency:
> + description: Directly connected to APB bus and
> + its shift clock is from APB bus clock divided by a programmable value.

The bus frequency is derived from input clocks, isn't it? We already
questioned this property and this does not help justify it existence.
Drop it.

> + default: 8000000
> +
> +required:
> + - compatible
> + - reg
> + - gpio-controller
> + - '#gpio-cells'
> + - interrupts
> + - nuvoton,input-ngpios
> + - nuvoton,output-ngpios
> + - clocks
> + - bus-frequency
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/nuvoton,npcm7xx-clock.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + gpio8: gpio@101000 {
> + compatible = "nuvoton,npcm750-sgpio";
> + reg = <0x101000 0x200>;
> + clocks = <&clk NPCM7XX_CLK_APB3>;
> + interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
> + bus-frequency = <8000000>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + nuvoton,input-ngpios = <64>;
> + nuvoton,output-ngpios = <64>;
> + status = "disabled";

I reminded you about this twice. So this is third time. Or maybe even
fourth?

Best regards,
Krzysztof

2023-01-10 13:38:17

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] dt-bindings: gpio: add NPCM sgpio driver bindings


On Tue, 10 Jan 2023 16:32:38 +0800, Jim Liu wrote:
> Add dt-bindings document for the Nuvoton NPCM7xx and NPCM8xx sgpio driver
>
> Signed-off-by: Jim Liu <[email protected]>
> ---
> Changes for v4:
> - modify in/out property
> - modify bus-frequency property
> Changes for v3:
> - modify description
> - modify in/out property name
> Changes for v2:
> - modify description
> ---
> .../bindings/gpio/nuvoton,sgpio.yaml | 92 +++++++++++++++++++
> 1 file changed, 92 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml: properties:nuvoton,input-ngpios: 'oneOf' conditional failed, one must be fixed:
'type' is a required property
hint: A vendor boolean property can use "type: boolean"
Additional properties are not allowed ('maximum', 'minimum' were unexpected)
hint: A vendor boolean property can use "type: boolean"
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml: properties:nuvoton,input-ngpios: 'oneOf' conditional failed, one must be fixed:
'enum' is a required property
'const' is a required property
hint: A vendor string property with exact values has an implicit type
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml: properties:nuvoton,input-ngpios: 'oneOf' conditional failed, one must be fixed:
'$ref' is a required property
'allOf' is a required property
hint: A vendor property needs a $ref to types.yaml
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml: properties:nuvoton,output-ngpios: 'oneOf' conditional failed, one must be fixed:
'type' is a required property
hint: A vendor boolean property can use "type: boolean"
Additional properties are not allowed ('maximum', 'minimum' were unexpected)
hint: A vendor boolean property can use "type: boolean"
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml: properties:nuvoton,output-ngpios: 'oneOf' conditional failed, one must be fixed:
'enum' is a required property
'const' is a required property
hint: A vendor string property with exact values has an implicit type
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml: properties:nuvoton,output-ngpios: 'oneOf' conditional failed, one must be fixed:
'$ref' is a required property
'allOf' is a required property
hint: A vendor property needs a $ref to types.yaml
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

2023-03-13 08:59:29

by Jim Liu

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] dt-bindings: gpio: add NPCM sgpio driver bindings

Hi Krzysztof

sorry for reply late.
I think I need to explain more details about the clock.

The NPCM7xx / NPCM8xx SGPIO feature have 4 pins.
picture is as below:
https://drive.google.com/file/d/1E9i_Avh-AZV9IEZO1HLMT4EtgCBe46OV/view?usp=sharing

The clock is generated from npcm7xx APB.
The bus frequency is derived from npcm7xx APB not HC595/HC165.
Users can connect 1~8 HC595 on DOUT pin to decode the serial data for
HC595 A~H pin
and can connect 1~8 HC165 on DIN pin to encode the serial data to
send to NPCM7xx.

The test device is as below:
https://pdf1.alldatasheet.com/datasheet-pdf/view/345467/TI/SN74HC595N.html
https://pdf1.alldatasheet.com/datasheet-pdf/view/27899/TI/SN74HC165N.html

NPCM7xx/NPCM8xx have two sgpio modules;
each module can support up to 64 output pins,and up to 64 input pins.
If the user needs 64 output pins , user needs to connect 8 HC595.
If the user needs 64 input pins , user needs to connect 8 HC165.

the HC595 and HC165 connect is as below:
NPCM7xx_DOUT -> HC595 SER pin
NPCM7xx_SCLK -> HC595 SRCLK pin
NPCM7xx_LDSH -> HC595 RCLK pin

NPCM7xx_SCLK -> HC165 CLK pin
NPCM7xx_LDSH -> HC165 SH/LD pin
NPCM7xx_DIN -> HC165 QH pin

The frequency is not derived from the input clock. so i think maybe
the yaml needs to describe it.
if yaml file still didn't need please let me know.

And if you have any confusion about the sgpio feature or the connect
test please let me know.


Best regards,
Jim

On Tue, Jan 10, 2023 at 9:29 PM Rob Herring <[email protected]> wrote:
>
>
> On Tue, 10 Jan 2023 16:32:38 +0800, Jim Liu wrote:
> > Add dt-bindings document for the Nuvoton NPCM7xx and NPCM8xx sgpio driver
> >
> > Signed-off-by: Jim Liu <[email protected]>
> > ---
> > Changes for v4:
> > - modify in/out property
> > - modify bus-frequency property
> > Changes for v3:
> > - modify description
> > - modify in/out property name
> > Changes for v2:
> > - modify description
> > ---
> > .../bindings/gpio/nuvoton,sgpio.yaml | 92 +++++++++++++++++++
> > 1 file changed, 92 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml: properties:nuvoton,input-ngpios: 'oneOf' conditional failed, one must be fixed:
> 'type' is a required property
> hint: A vendor boolean property can use "type: boolean"
> Additional properties are not allowed ('maximum', 'minimum' were unexpected)
> hint: A vendor boolean property can use "type: boolean"
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml: properties:nuvoton,input-ngpios: 'oneOf' conditional failed, one must be fixed:
> 'enum' is a required property
> 'const' is a required property
> hint: A vendor string property with exact values has an implicit type
> from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml: properties:nuvoton,input-ngpios: 'oneOf' conditional failed, one must be fixed:
> '$ref' is a required property
> 'allOf' is a required property
> hint: A vendor property needs a $ref to types.yaml
> from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
> hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
> from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml: properties:nuvoton,output-ngpios: 'oneOf' conditional failed, one must be fixed:
> 'type' is a required property
> hint: A vendor boolean property can use "type: boolean"
> Additional properties are not allowed ('maximum', 'minimum' were unexpected)
> hint: A vendor boolean property can use "type: boolean"
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml: properties:nuvoton,output-ngpios: 'oneOf' conditional failed, one must be fixed:
> 'enum' is a required property
> 'const' is a required property
> hint: A vendor string property with exact values has an implicit type
> from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml: properties:nuvoton,output-ngpios: 'oneOf' conditional failed, one must be fixed:
> '$ref' is a required property
> 'allOf' is a required property
> hint: A vendor property needs a $ref to types.yaml
> from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
> hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
> from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>

2023-03-13 09:23:23

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] dt-bindings: gpio: add NPCM sgpio driver bindings

On 13/03/2023 09:54, Jim Liu wrote:
> Hi Krzysztof
>
> sorry for reply late.
> I think I need to explain more details about the clock.

You top posted to Rob's bot's email, so I have no clue what do you refer to.

Best regards,
Krzysztof


2023-03-13 10:38:44

by Jim Liu

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] dt-bindings: gpio: add NPCM sgpio driver bindings

Hi Krzysztof

Sorry for the mistake.
I think I need to explain more details about the clock.

The NPCM7xx / NPCM8xx SGPIO feature have 4 pins.
picture is as below:
https://drive.google.com/file/d/1E9i_Avh-AZV9IEZO1HLMT4EtgCBe46OV/view?usp=sharing

The clock is generated from npcm7xx APB.
The bus frequency is derived from npcm7xx APB not HC595/HC165.
Users can connect 1~8 HC595 on DOUT pin to decode the serial data for
HC595 A~H pin
and can connect 1~8 HC165 on DIN pin to encode the serial data to
send to NPCM7xx.

The test device is as below:
https://pdf1.alldatasheet.com/datasheet-pdf/view/345467/TI/SN74HC595N.html
https://pdf1.alldatasheet.com/datasheet-pdf/view/27899/TI/SN74HC165N.html

NPCM7xx/NPCM8xx have two sgpio modules;
each module can support up to 64 output pins,and up to 64 input pins.
If the user needs 64 output pins , user needs to connect 8 HC595.
If the user needs 64 input pins , user needs to connect 8 HC165.

the HC595 and HC165 connect is as below:
NPCM7xx_DOUT -> HC595 SER pin
NPCM7xx_SCLK -> HC595 SRCLK pin
NPCM7xx_LDSH -> HC595 RCLK pin

NPCM7xx_SCLK -> HC165 CLK pin
NPCM7xx_LDSH -> HC165 SH/LD pin
NPCM7xx_DIN -> HC165 QH pin

The frequency is not derived from the input clock. so i think maybe
the yaml needs to describe it.
if yaml file still didn't need please let me know.

And if you have any confusion about the sgpio feature or the connect
test please let me know.


Best regards,
Jim

On Tue, Jan 10, 2023 at 6:35 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 10/01/2023 09:32, Jim Liu wrote:
> > Add dt-bindings document for the Nuvoton NPCM7xx and NPCM8xx sgpio driver
> >
> > Signed-off-by: Jim Liu <[email protected]>
> > ---
> > Changes for v4:
> > - modify in/out property
> > - modify bus-frequency property
> > Changes for v3:
> > - modify description
> > - modify in/out property name
> > Changes for v2:
> > - modify description
> > ---
> > .../bindings/gpio/nuvoton,sgpio.yaml | 92 +++++++++++++++++++
> > 1 file changed, 92 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml b/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
> > new file mode 100644
> > index 000000000000..3c01ce61f8d9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
> > @@ -0,0 +1,92 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/gpio/nuvoton,sgpio.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Nuvoton SGPIO controller
> > +
> > +maintainers:
> > + - Jim LIU <[email protected]>
> > +
> > +description:
> > + This SGPIO controller is for NUVOTON NPCM7xx and NPCM8xx SoC.
> > + Nuvoton NPCM7xx SGPIO module is combine serial to parallel IC (HC595)
> > + and parallel to serial IC (HC165), and use APB3 clock to control it.
> > + This interface has 4 pins (D_out , D_in, S_CLK, LDSH).
> > + NPCM7xx/NPCM8xx have two sgpio module each module can support up
> > + to 64 output pins,and up to 64 input pin, the pin is only for gpi or gpo.
> > + GPIO pins have sequential, First half is gpo and second half is gpi.
> > + 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)
> > + - ngpios is number of nuvoton,input-ngpios GPIO lines and nuvoton,output-ngpios GPIO lines.
> > + nuvoton,input-ngpios GPIO lines is only for gpi.
> > + nuvoton,output-ngpios GPIO lines is only for gpo.
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - nuvoton,npcm750-sgpio
> > + - nuvoton,npcm845-sgpio
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + gpio-controller: true
> > +
> > + '#gpio-cells':
> > + const: 2
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + nuvoton,input-ngpios:
> > + description: The numbers of GPIO's exposed.
> > + GPIO lines is only for gpi.
> > + minimum: 0
> > + maximum: 64
> > +
> > + nuvoton,output-ngpios:
> > + description: The numbers of GPIO's exposed.
> > + GPIO lines is only for gpo.
> > + minimum: 0
> > + maximum: 64
> > +
> > + bus-frequency:
> > + description: Directly connected to APB bus and
> > + its shift clock is from APB bus clock divided by a programmable value.
>
> The bus frequency is derived from input clocks, isn't it? We already
> questioned this property and this does not help justify it existence.
> Drop it.
>
> > + default: 8000000
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - gpio-controller
> > + - '#gpio-cells'
> > + - interrupts
> > + - nuvoton,input-ngpios
> > + - nuvoton,output-ngpios
> > + - clocks
> > + - bus-frequency
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/nuvoton,npcm7xx-clock.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + gpio8: gpio@101000 {
> > + compatible = "nuvoton,npcm750-sgpio";
> > + reg = <0x101000 0x200>;
> > + clocks = <&clk NPCM7XX_CLK_APB3>;
> > + interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
> > + bus-frequency = <8000000>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + nuvoton,input-ngpios = <64>;
> > + nuvoton,output-ngpios = <64>;
> > + status = "disabled";
>
> I reminded you about this twice. So this is third time. Or maybe even
> fourth?
>
> Best regards,
> Krzysztof
>

2023-03-13 12:30:28

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] dt-bindings: gpio: add NPCM sgpio driver bindings

On 13/03/2023 11:38, Jim Liu wrote:
> Hi Krzysztof
>
> Sorry for the mistake.
> I think I need to explain more details about the clock.

It's still top-posting.

>
> The NPCM7xx / NPCM8xx SGPIO feature have 4 pins.
> picture is as below:
> https://drive.google.com/file/d/1E9i_Avh-AZV9IEZO1HLMT4EtgCBe46OV/view?usp=sharing
>
> The clock is generated from npcm7xx APB.
> The bus frequency is derived from npcm7xx APB not HC595/HC165.
> Users can connect 1~8 HC595 on DOUT pin to decode the serial data for
> HC595 A~H pin
> and can connect 1~8 HC165 on DIN pin to encode the serial data to
> send to NPCM7xx.
>
> The test device is as below:
> https://pdf1.alldatasheet.com/datasheet-pdf/view/345467/TI/SN74HC595N.html
> https://pdf1.alldatasheet.com/datasheet-pdf/view/27899/TI/SN74HC165N.html
>
> NPCM7xx/NPCM8xx have two sgpio modules;
> each module can support up to 64 output pins,and up to 64 input pins.
> If the user needs 64 output pins , user needs to connect 8 HC595.
> If the user needs 64 input pins , user needs to connect 8 HC165.
>
> the HC595 and HC165 connect is as below:
> NPCM7xx_DOUT -> HC595 SER pin
> NPCM7xx_SCLK -> HC595 SRCLK pin
> NPCM7xx_LDSH -> HC595 RCLK pin
>
> NPCM7xx_SCLK -> HC165 CLK pin
> NPCM7xx_LDSH -> HC165 SH/LD pin
> NPCM7xx_DIN -> HC165 QH pin
>
> The frequency is not derived from the input clock. so i think maybe
> the yaml needs to describe it.

That's not what your code was saying. It said:
"Directly connected to APB bus and its shift clock is from APB bus clock
divided by a programmable value."

> if yaml file still didn't need please let me know.

Now read the description of bus-frequency:
"Legacy property for fixed bus frequencies"

Don't add legacy properties to new bindings. You have
assigned-clock-rates and clocks properties.

Best regards,
Krzysztof