cleanup StarFive JH7100 GPIO driver and device tree

[pdp7]

This is my initial attempt to clean up the obvious issues with the StarFive JH7100 GPIO driver:

• add StarFive as vendor prefix [I have posted this to the device-tree list]
• rename GPIO driver from VIC to JH7100
• fix checkpatch warnings for GPIO driver
• add SPDX license headers
• rename the board dts from jh7100-starlight to beaglev-starlight-jh7100

@esmil please let me know if you prefer me raising a PR against https://github.com/esmil/linux instead

[esmil]

I love it until here and have already pushed the gpio changes to the starlight branch.

I must admit though, that I really like the scheme that I think is best enforced by arch/arm64/boot/dts/rockchip right now, where the files are named from most generic to most specific. It makes it really obvious which files builds on which when you see

rk3399-gru-kevin.dts
rk3399-gru-scarlet-inx.dts
rk3399-gru-scarlet-kd.dts
rk3399-gru-scarlet.dtsi
rk3399-gru.dtsi
rk3399.dtsi

I know none of arch/riscv/boot/dts uses this scheme and even rockchip has exceptions, but I hope it's not too late have some sort of order in arch/riscv/boot/dts/starfive.

[esmil]

To put it another way. I strongly prefer this

jh7100-starlight.dts
jh7100.dtsi
jh7110-arvsom-app1.dts
jh7110-arvsom-app2.dts
jh7110-arvsom-devboard.dts
jh7110-arvsom.dtsi
jh7110-starlight.dts
jh7110.dtsi
jh71x0.dtsi

..over this

antmicro-arvsom-jh7110.dtsi
antmicro-devboard-jh7110.dts
beaglev-starlight-jh7100.dts
beaglev-starlight-jh7110.dts
firstcompany-app1.dts
jh7100.dtsi
jh7110.dtsi
jh71x0.dtsi
secondcompany-app2.dts

[pdp7]

@esmil

To put it another way. I strongly prefer this

jh7100-starlight.dts
jh7100.dtsi
jh7110-arvsom-app1.dts
jh7110-arvsom-app2.dts
jh7110-arvsom-devboard.dts
jh7110-arvsom.dtsi
jh7110-starlight.dts
jh7110.dtsi
jh71x0.dtsi

..over this

Yes, I agree. I was trying to avoid the bare "starlight" as the board name should be thought of atomic phrase "beaglev-starlight". Thus, I think jh7100-beaglev-starlight.dts sounds like the best name for the current board.

Note, there will be boards with the JH7100 beyond just these beta prototypes so like the naming of jh71100-beaglev-starlight.dts and jh7100-beaglev-starlight.dts

[geertu]

@pdp7 Please make the following change to the example in the GPIO DT bindings:

- reg = <0x0 0x11910000 0x0 0x10000>;
+ reg = <0x11910000 0x10000>;

and drop the surrounding "soc" node and related #{address,size}-cells.
By convention, all examples on the MMIO bus use #{address,size}-cells = <1>.

[pdp7]

@geertu thanks! I will make that change.

That 4 value notion looked weird to be which led me down a long path of reading about reg properties and address ranges :) What you suggested does look easier to understand to me.

Just to make sure my understanding is correct. Does the old 4 value line mean this?

reg = <0x0 0x11910000 0x0 0x10000>;

• Position 1: Bus address is 0x0, this is what the GPIO controller thinks it is
• Position 2: CPU address is 0x11910000, this is how the CPU sees the GPIO controller base register offset
• Position 3: upper 32 bits of 64 bit size which is 0x0
• Position 4: lower 32 bits of 64 bit size which is 0x10000, which is 64KB

[geertu]

@geertu thanks! I will make that change.

That 4 value notion looked weird to be which led me down a long path of reading about reg properties and address ranges :) What you suggested does look easier to understand to me.

Just to make sure my understanding is correct. Does the old 4 value line mean this?

reg = <0x0 0x11910000 0x0 0x10000>;

• Position 1: Bus address is 0x0
• Position 2: CPU address is 0x11910000

Almost correct: as #address-cells was 2, the first two numbers formed a 64-bit physical CPU address.

• Position 3: upper 32 bits of 64 bit size which is 0x0
• Position 4: lower 32 bits of 64 bit size which is 0x10000

Correct: as #size-cells was 2, the second set of two numbers formed a 64-bit size.

[pdp7]

@geertu thanks, I have made those changes and the checks seem to pass ok:

pdp7@x1:~/dev/starfive/linux$ make -j8 ARCH=riscv CROSS_COMPILE=riscv64-linux-gnu- dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.yaml
  CHKDT   Documentation/devicetree/bindings/processed-schema-examples.json
  DTEX    Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.example.dts
/home/pdp7/dev/starfive/linux/Documentation/devicetree/bindings/media/renesas,drif.yaml: properties:clock-names:maxItems: False schema does not allow 1
	hint: "minItems/maxItems" equal to the "items" list length are not necessary
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
  SCHEMA  Documentation/devicetree/bindings/processed-schema-examples.json
/home/pdp7/dev/starfive/linux/Documentation/devicetree/bindings/media/renesas,drif.yaml: ignoring, error in schema: properties: clock-names: maxItems
warning: no schema found in file: ./Documentation/devicetree/bindings/media/renesas,drif.yaml
  DTC     Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.example.dt.yaml
  CHECK   Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.example.dt.yaml
/home/pdp7/dev/starfive/linux/Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.example.dt.yaml: example-0: gpio@11910000:reg:0: [0, 294715392, 0, 65536] is too long
	From schema: /home/pdp7/.local/lib/python3.8/site-packages/dtschema/schemas/reg.yaml

pdp7@x1:~/dev/starfive/linux$ make -j8 ARCH=riscv CROSS_COMPILE=riscv64-linux-gnu- dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.yaml 

The commits are in https://github.com/starfive-tech/linux/tree/fix-dts-gpio-reg-esmil_starlight:

• riscv: dts: starfive-jh7100: change gpio reg property
• dt-bindings: gpio: fix starfive,jh7100-gpio bindings example

[pdp7]

@esmil @geertu pull request here: #25

[geertu]

@pdp7 Seems like you misunderstood:

1. The example should use "reg = <0x11910000 0x10000>;" (examples use #{address,size}-cells = <1>),
2. The actual DTS should use "reg = <0x0 0x11910000 0x0 0x10000>;" (real DTS has #{address,size}-cells = <2>).

DTC Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.example.dt.yaml
CHECK Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.example.dt.yaml
/home/pdp7/dev/starfive/linux/Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.example.dt.yaml: example-0: gpio@11910000:reg:0: [0, 294715392, 0, 65536] is too long

Hence it complains about the example.
There should be another complaint for the DTS ("make dtbs_check").

[pdp7]

@geertu Ah! Thank you for explaining that. I failed to see the example-0: gpio@11910000:reg:0: [0, 294715392, 0, 65536] is too long in the output.

Unrelated, but any idea why I might be seeing those warnings for Documentation/devicetree/bindings/media/renesas,drif.yaml?

[geertu]

Unrelated, but any idea why I might be seeing those warnings for Documentation/devicetree/bindings/media/renesas,drif.yaml?

@pdp7 Because https://lore.kernel.org/r/20210408202436.3706-1-fabrizio.castro.jz@renesas.com hasn't been applied yet.