| Message ID | 20210701002037.912625-1-drew@beagleboard.org |
|---|---|
| Headers | show |
| Series | gpio: starfive-jh7100: Add StarFive JH7100 GPIO bindings and driver | expand |
Hi Drew, On Thu, Jul 1, 2021 at 2:22 AM Drew Fustini <drew@beagleboard.org> wrote: > Add bindings for the GPIO controller in the StarFive JH7100 SoC [1]. > > [1] https://github.com/starfive-tech/beaglev_doc > > Signed-off-by: Drew Fustini <drew@beagleboard.org> > Signed-off-by: Huan Feng <huan.feng@starfivetech.com> Thanks for your patch! > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.yaml > @@ -0,0 +1,60 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/gpio/starfive,jh7100-gpio.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: StarFive JH7100 GPIO controller > + > +maintainers: > + - Huan Feng <huan.feng@starfivetech.com> > + - Drew Fustini <drew@beagleboard.org> > + > +properties: > + compatible: > + items: > + - const: starfive,jh7100-gpio > + > + reg: > + maxItems: 1 > + > + interrupts: > + description: > + Interrupt mapping, one per GPIO. Maximum 32 GPIOs. > + minItems: 1 > + maxItems: 32 What about clocks and resets? > + > + gpio-controller: true > + > + "#gpio-cells": > + const: 2 > + > + interrupt-controller: true > + > + "#interrupt-cells": > + const: 2 > + > +required: > + - compatible > + - reg > + - interrupts > + - interrupt-controller > + - "#interrupt-cells" > + - "#gpio-cells" > + - gpio-controller > + > +additionalProperties: false > + > +examples: > + - | > + gpio@11910000 { > + compatible = "starfive,jh7100-gpio"; > + reg = <0x11910000 0x10000>; > + gpio-controller; > + #gpio-cells = <2>; > + interrupt-controller; > + #interrupt-cells = <2>; > + interrupts = <32>; > + }; > + > +... Gr{oetje,eeting}s, Geert
On Thu, Jul 01, 2021 at 10:34:56AM +0200, Geert Uytterhoeven wrote: > Hi Drew, > > On Thu, Jul 1, 2021 at 2:22 AM Drew Fustini <drew@beagleboard.org> wrote: > > Add bindings for the GPIO controller in the StarFive JH7100 SoC [1]. > > > > [1] https://github.com/starfive-tech/beaglev_doc > > > > Signed-off-by: Drew Fustini <drew@beagleboard.org> > > Signed-off-by: Huan Feng <huan.feng@starfivetech.com> > > Thanks for your patch! > > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.yaml > > @@ -0,0 +1,60 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/gpio/starfive,jh7100-gpio.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: StarFive JH7100 GPIO controller > > + > > +maintainers: > > + - Huan Feng <huan.feng@starfivetech.com> > > + - Drew Fustini <drew@beagleboard.org> > > + > > +properties: > > + compatible: > > + items: > > + - const: starfive,jh7100-gpio > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + description: > > + Interrupt mapping, one per GPIO. Maximum 32 GPIOs. > > + minItems: 1 > > + maxItems: 32 > > What about clocks and resets? Thank you for your feedback, Geert. GPIO controller uses clk_apb1_bus under dom0_sys. I believe the device tree node would use something like this: clocks = <&clkgen JH7100_CLK_APB1>; I see the sifive-gpio.yaml has: clocks: maxItems: 1 Would that be the correct way to do it for the starfive gpio yaml? The reset for GPIO controller is presetn under dom_sys. Do you think know you know an example that has reset in the YAML? Is there some code that would actually make use of that information? > > > + > > + gpio-controller: true > > + > > + "#gpio-cells": > > + const: 2 > > + > > + interrupt-controller: true > > + > > + "#interrupt-cells": > > + const: 2 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - interrupt-controller > > + - "#interrupt-cells" > > + - "#gpio-cells" > > + - gpio-controller Do you think I should add 'clocks' to 'required:'? > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + gpio@11910000 { > > + compatible = "starfive,jh7100-gpio"; > > + reg = <0x11910000 0x10000>; > > + gpio-controller; > > + #gpio-cells = <2>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + interrupts = <32>; I would add: clocks = <&clkgen JH7100_CLK_APB1>; But I am not sure how reset would work? Thank you, Drew
Hi Drew, On Fri, Jul 2, 2021 at 10:56 PM Drew Fustini <drew@beagleboard.org> wrote: > On Thu, Jul 01, 2021 at 10:34:56AM +0200, Geert Uytterhoeven wrote: > > On Thu, Jul 1, 2021 at 2:22 AM Drew Fustini <drew@beagleboard.org> wrote: > > > Add bindings for the GPIO controller in the StarFive JH7100 SoC [1]. > > > > > > [1] https://github.com/starfive-tech/beaglev_doc > > > > > > Signed-off-by: Drew Fustini <drew@beagleboard.org> > > > Signed-off-by: Huan Feng <huan.feng@starfivetech.com> > > > > Thanks for your patch! > > > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.yaml > > > @@ -0,0 +1,60 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/gpio/starfive,jh7100-gpio.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: StarFive JH7100 GPIO controller > > > + > > > +maintainers: > > > + - Huan Feng <huan.feng@starfivetech.com> > > > + - Drew Fustini <drew@beagleboard.org> > > > + > > > +properties: > > > + compatible: > > > + items: > > > + - const: starfive,jh7100-gpio > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + interrupts: > > > + description: > > > + Interrupt mapping, one per GPIO. Maximum 32 GPIOs. > > > + minItems: 1 > > > + maxItems: 32 > > > > What about clocks and resets? > > Thank you for your feedback, Geert. > > GPIO controller uses clk_apb1_bus under dom0_sys. I believe the device > tree node would use something like this: > > clocks = <&clkgen JH7100_CLK_APB1>; > > I see the sifive-gpio.yaml has: > > clocks: > maxItems: 1 > > Would that be the correct way to do it for the starfive gpio yaml? Yep. > The reset for GPIO controller is presetn under dom_sys. Do you think > know you know an example that has reset in the YAML? Is there some code > that would actually make use of that information? > > > > > > + > > > + gpio-controller: true > > > + > > > + "#gpio-cells": > > > + const: 2 > > > + > > > + interrupt-controller: true > > > + > > > + "#interrupt-cells": > > > + const: 2 > > > + > > > +required: > > > + - compatible > > > + - reg > > > + - interrupts > > > + - interrupt-controller > > > + - "#interrupt-cells" > > > + - "#gpio-cells" > > > + - gpio-controller > > Do you think I should add 'clocks' to 'required:'? I'm still having issues with i2c if the GPIO block lists a clock, due to fw_devlink dependencies. > > > + > > > +additionalProperties: false > > > + > > > +examples: > > > + - | > > > + gpio@11910000 { > > > + compatible = "starfive,jh7100-gpio"; > > > + reg = <0x11910000 0x10000>; > > > + gpio-controller; > > > + #gpio-cells = <2>; > > > + interrupt-controller; > > > + #interrupt-cells = <2>; > > > + interrupts = <32>; > > I would add: > > clocks = <&clkgen JH7100_CLK_APB1>; > > But I am not sure how reset would work? That should become "resets = <&rstgen JH7100_RSTN_GPIO_APB>", but we don't have the reset controller in Linux yet (we do in barebox). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Fri, Jul 02, 2021 at 11:03:56PM +0200, Geert Uytterhoeven wrote: > Hi Drew, > > On Fri, Jul 2, 2021 at 10:56 PM Drew Fustini <drew@beagleboard.org> wrote: > > On Thu, Jul 01, 2021 at 10:34:56AM +0200, Geert Uytterhoeven wrote: > > > On Thu, Jul 1, 2021 at 2:22 AM Drew Fustini <drew@beagleboard.org> wrote: > > > > Add bindings for the GPIO controller in the StarFive JH7100 SoC [1]. > > > > > > > > [1] https://github.com/starfive-tech/beaglev_doc > > > > > > > > Signed-off-by: Drew Fustini <drew@beagleboard.org> > > > > Signed-off-by: Huan Feng <huan.feng@starfivetech.com> > > > > > > Thanks for your patch! > > > > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.yaml > > > > @@ -0,0 +1,60 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/gpio/starfive,jh7100-gpio.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: StarFive JH7100 GPIO controller > > > > + > > > > +maintainers: > > > > + - Huan Feng <huan.feng@starfivetech.com> > > > > + - Drew Fustini <drew@beagleboard.org> > > > > + > > > > +properties: > > > > + compatible: > > > > + items: > > > > + - const: starfive,jh7100-gpio > > > > + > > > > + reg: > > > > + maxItems: 1 > > > > + > > > > + interrupts: > > > > + description: > > > > + Interrupt mapping, one per GPIO. Maximum 32 GPIOs. > > > > + minItems: 1 > > > > + maxItems: 32 > > > > > > What about clocks and resets? > > > > Thank you for your feedback, Geert. > > > > GPIO controller uses clk_apb1_bus under dom0_sys. I believe the device > > tree node would use something like this: > > > > clocks = <&clkgen JH7100_CLK_APB1>; > > > > I see the sifive-gpio.yaml has: > > > > clocks: > > maxItems: 1 > > > > Would that be the correct way to do it for the starfive gpio yaml? > > Yep. > > > The reset for GPIO controller is presetn under dom_sys. Do you think > > know you know an example that has reset in the YAML? Is there some code > > that would actually make use of that information? > > > > > > > > > + > > > > + gpio-controller: true > > > > + > > > > + "#gpio-cells": > > > > + const: 2 > > > > + > > > > + interrupt-controller: true > > > > + > > > > + "#interrupt-cells": > > > > + const: 2 > > > > + > > > > +required: > > > > + - compatible > > > > + - reg > > > > + - interrupts > > > > + - interrupt-controller > > > > + - "#interrupt-cells" > > > > + - "#gpio-cells" > > > > + - gpio-controller > > > > Do you think I should add 'clocks' to 'required:'? > > I'm still having issues with i2c if the GPIO block lists a clock, due to > fw_devlink dependencies. > > > > > + > > > > +additionalProperties: false > > > > + > > > > +examples: > > > > + - | > > > > + gpio@11910000 { > > > > + compatible = "starfive,jh7100-gpio"; > > > > + reg = <0x11910000 0x10000>; > > > > + gpio-controller; > > > > + #gpio-cells = <2>; > > > > + interrupt-controller; > > > > + #interrupt-cells = <2>; > > > > + interrupts = <32>; > > > > I would add: > > > > clocks = <&clkgen JH7100_CLK_APB1>; > > > > But I am not sure how reset would work? > > That should become "resets = <&rstgen JH7100_RSTN_GPIO_APB>", > but we don't have the reset controller in Linux yet (we do in barebox). Do you think I should add reset item like this? resets: maxItems: 1 I suppose this is supposed to describe the hardware and it shouldn't matter whether or not Linux uses the property, right? Thank you, Drew
Hi Drew, On Sat, Jul 3, 2021 at 8:46 AM Drew Fustini <drew@beagleboard.org> wrote: > On Fri, Jul 02, 2021 at 11:03:56PM +0200, Geert Uytterhoeven wrote: > > On Fri, Jul 2, 2021 at 10:56 PM Drew Fustini <drew@beagleboard.org> wrote: > > > On Thu, Jul 01, 2021 at 10:34:56AM +0200, Geert Uytterhoeven wrote: > > > > On Thu, Jul 1, 2021 at 2:22 AM Drew Fustini <drew@beagleboard.org> wrote: > > > > > Add bindings for the GPIO controller in the StarFive JH7100 SoC [1]. > > > > > > > > > > [1] https://github.com/starfive-tech/beaglev_doc > > > > > > > > > > Signed-off-by: Drew Fustini <drew@beagleboard.org> > > > > > Signed-off-by: Huan Feng <huan.feng@starfivetech.com> > > > > > > > > Thanks for your patch! > > > > > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/gpio/starfive,jh7100-gpio.yaml > > > > > @@ -0,0 +1,60 @@ > > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > > > +%YAML 1.2 > > > > > +--- > > > > > +$id: http://devicetree.org/schemas/gpio/starfive,jh7100-gpio.yaml# > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > + > > > > > +title: StarFive JH7100 GPIO controller > > > > > + > > > > > +maintainers: > > > > > + - Huan Feng <huan.feng@starfivetech.com> > > > > > + - Drew Fustini <drew@beagleboard.org> > > > > > + > > > > > +properties: > > > > > + compatible: > > > > > + items: > > > > > + - const: starfive,jh7100-gpio > > > > > + > > > > > + reg: > > > > > + maxItems: 1 > > > > > + > > > > > + interrupts: > > > > > + description: > > > > > + Interrupt mapping, one per GPIO. Maximum 32 GPIOs. > > > > > + minItems: 1 > > > > > + maxItems: 32 > > > > > > > > What about clocks and resets? > > > But I am not sure how reset would work? > > > > That should become "resets = <&rstgen JH7100_RSTN_GPIO_APB>", > > but we don't have the reset controller in Linux yet (we do in barebox). > > Do you think I should add reset item like this? > > resets: > maxItems: 1 > > I suppose this is supposed to describe the hardware and it shouldn't > matter whether or not Linux uses the property, right? Exactly. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds