| Message ID | 1490026491-21742-3-git-send-email-jacopo+renesas@jmondi.org |
|---|---|
| State | New |
| Headers | show |
Hi Jacopo, On Mon, Mar 20, 2017 at 5:14 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote: > Add device tree bindings documentation for Renesas RZ/A1 gpio and pin for the Renesas ... > controller. > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > --- > .../bindings/pinctrl/renesas,rza1-pinctrl.txt | 144 +++++++++++++++++++++ > 1 file changed, 144 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt > > diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt > new file mode 100644 > index 0000000..0474860 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt > @@ -0,0 +1,144 @@ > +Renesas RZ/A1 combined Pin and GPIO controller > + > +Renesas SoCs of RZ/A1 family feature a combined Pin and GPIO controller the RZ/A1 family > +hardware controller, named "Ports" in the hardware reference manual. bogus "hardware controller" > +Sub-nodes > +--------- > + > +The child nodes of the pin controller node describe a pin multiplexing > +function or a gpio controller alternatively. > + > +- Pin multiplexing sub-nodes: > + A pin multiplexing sub-node describes how to configure a set of > + (or a single) pin in some desired alternate function mode. > + A single sub-node may define several pin configurations groups. > + > + Required properties: > + - renesas,pins Just "pins"? > + describes an array of pin multiplexing configurations. > + When a pin has to be configured in alternate function mode, use this > + property to identify the pin by its global index, and provide its > + alternate function configuration description along with it. > + When multiple pins are required to be configured as part of the same > + alternate function (odds are single-pin alternate functions exist) they > + shall be specified as members of the same argument list of a single > + "renesas-pins" property. > + Helper macros to ease calculating the pin index from its position > + (port where it sits on and pin offset), and alternate function > + configuration options are provided in pin controller header file at: the pin ... file > + include/dt-bindings/pinctrl/r7s72100-pinctrl.h Hence I'd include that file in this patch, as it's part of the bindings. > + Example: > + A serial communication interface with a TX output pin and a RX input pin. an RX > + > + &pinctrl { > + scif2_pins: serial2 { > + renesas,pins = <PIN(3, 0) 6>, > + <PIN(3, 2) 4>; Single line? > + }; > + } > + > + Pin #0 on port #3 is configured in alternate function #6. > + Pin #2 on port #3 is configured in alternate function #4. as alternate function > + > + Example 2: > + I2c master: both SDA and SCL pins need bi-directional operations > + > + &pinctrl { > + i2c2_pins: i2c2 { > + renesas,pins = <PIN(1, 4) (1 | BI_DIR)>, > + <PIN(1, 5) (1 | BI_DIR)>; > + }; > + } > + > + Pin #4 on port #1 is configured in alternate function #1. > + Pin #5 on port #1 is configured in alternate function #1. as alternate function > + Both need to work in bi-directional mode. > + > + Example 3: > + Multi-function timer input and output compare pins. > + The hardware manual prescribes this pins to have input/output direction > + specified by software. Configure TIOC0A as input and TIOC0B as output. > + > + &pinctrl { > + tioc0_pins: tioc0 { > + renesas,pins = <PIN(4, 0) (2 | SWIO_IN)>, > + <PIN(4, 1) (2 | SWIO_OUT)>; > + }; > + } > + > + Pin #0 on port #4 is configured in alternate function #2 with IO direction > + specified by software as input. > + Pin #1 on port #4 is configured in alternate function #1 with IO direction > + specified by software as output. as alternate function > +- GPIO controller sub-nodes: > + Each port of r7s72100 pin controller hardware is itself a gpio controller. the r7s72100 pin controller hardware > + Different SoCs have different number of available pins per port, but numbers of > + generally speaking, each of them can be configured in GPIO ("port") mode > + on this hardware. > + Describe gpio-controllers using sub-nodes with the following properties. > + > + Required properties: > + - gpio-controller > + empty property as defined by gpio bindings documentation. the gpio bindings documentation > + - #gpio-cells > + number of cells required to identify and configure a GPIO. > + Shall be 2. > + - gpio-ranges > + Describes a gpio controller specifying its specific pin base, the pin > + base in the global pin numbering space, and the number of controlled > + pins, as defined by gpio bindings documentation. Refer to this file the gpio bindings documentation 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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 22, 2017 at 11:33 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Mon, Mar 20, 2017 at 5:14 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote: >> Add device tree bindings documentation for Renesas RZ/A1 gpio and pin >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt ulating the pin index from its position >> + (port where it sits on and pin offset), and alternate function >> + configuration options are provided in pin controller header file at: > > the pin ... file > >> + include/dt-bindings/pinctrl/r7s72100-pinctrl.h > > Hence I'd include that file in this patch, as it's part of the bindings. Ah, that would create a hard dependency between the DTS files and the DT bindings, which typically go in through different trees. As the driver replicates the definitions from the header, the include file can go in with the DTS updates. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert,
thanks for reviews
On Wed, Mar 22, 2017 at 02:20:08PM +0100, Geert Uytterhoeven wrote:
> On Wed, Mar 22, 2017 at 11:33 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Mon, Mar 20, 2017 at 5:14 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> >> Add device tree bindings documentation for Renesas RZ/A1 gpio and pin
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
>
> ulating the pin index from its position
> >> + (port where it sits on and pin offset), and alternate function
> >> + configuration options are provided in pin controller header file at:
> >
> > the pin ... file
> >
> >> + include/dt-bindings/pinctrl/r7s72100-pinctrl.h
> >
> > Hence I'd include that file in this patch, as it's part of the bindings.
>
> Ah, that would create a hard dependency between the DTS files and the
> DT bindings, which typically go in through different trees.
> As the driver replicates the definitions from the header, the include file
> can go in with the DTS updates.
>
Sorry, got confused by multiple reviews here..
Are you suggesting to squash [03/07] in [04/07] here?
Thanks
j
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jacopo, On Wed, Mar 22, 2017 at 4:36 PM, jacopo <jacopo@jmondi.org> wrote: > On Wed, Mar 22, 2017 at 02:20:08PM +0100, Geert Uytterhoeven wrote: >> On Wed, Mar 22, 2017 at 11:33 AM, Geert Uytterhoeven >> <geert@linux-m68k.org> wrote: >> > On Mon, Mar 20, 2017 at 5:14 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote: >> >> Add device tree bindings documentation for Renesas RZ/A1 gpio and pin >> >> --- /dev/null >> >> +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt >> >> ulating the pin index from its position >> >> + (port where it sits on and pin offset), and alternate function >> >> + configuration options are provided in pin controller header file at: >> > >> > the pin ... file >> > >> >> + include/dt-bindings/pinctrl/r7s72100-pinctrl.h >> > >> > Hence I'd include that file in this patch, as it's part of the bindings. >> >> Ah, that would create a hard dependency between the DTS files and the >> DT bindings, which typically go in through different trees. >> As the driver replicates the definitions from the header, the include file >> can go in with the DTS updates. >> > > Sorry, got confused by multiple reviews here.. > Are you suggesting to squash [03/07] in [04/07] here? No, I suggested to squash [03/07] into [02/07]. But upon second thought, that's not such a good idea, as it creates an additional dependency. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert, thanks for review On Wed, Mar 22, 2017 at 11:33:50AM +0100, Geert Uytterhoeven wrote: > Hi Jacopo, > > On Mon, Mar 20, 2017 at 5:14 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote: > > Add device tree bindings documentation for Renesas RZ/A1 gpio and pin > > for the Renesas ... > > > controller. > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > --- > > .../bindings/pinctrl/renesas,rza1-pinctrl.txt | 144 +++++++++++++++++++++ > > 1 file changed, 144 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt > > > > diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt > > new file mode 100644 > > index 0000000..0474860 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt > > @@ -0,0 +1,144 @@ > > +Renesas RZ/A1 combined Pin and GPIO controller > > + > > +Renesas SoCs of RZ/A1 family feature a combined Pin and GPIO controller > > the RZ/A1 family > > > +hardware controller, named "Ports" in the hardware reference manual. > > bogus "hardware controller" > > > +Sub-nodes > > +--------- > > + > > +The child nodes of the pin controller node describe a pin multiplexing > > +function or a gpio controller alternatively. > > + > > +- Pin multiplexing sub-nodes: > > + A pin multiplexing sub-node describes how to configure a set of > > + (or a single) pin in some desired alternate function mode. > > + A single sub-node may define several pin configurations groups. > > + > > + Required properties: > > + - renesas,pins > > Just "pins"? > You know, I've been thinking about this, bu the "pins" property definition in pinctrl-bidings is the following one: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt --- - pins takes a list of pin names or IDs as a required argument. The specific binding for the hardware defines: - Whether the entries are integers or strings, and their meaning. --- And all examples there assume one "pin name" or "ID" per pin. Now, we use 2 values per each pin (the pin ID and the alternate function number), so to me this is different from what the generic binding describes. Also, pinctrl-single, and pinctrl-imx which have and ABI similar to the one this driver define, use "pinctrl-single,pins" and "fsl,pins" respectively as property names. So either they have to be updated yet, or we should keep using "renesas,pins" for our own defined ABI. Maybe Linus or other pinctrl people can give some suggestion here. Thanks j > > + describes an array of pin multiplexing configurations. > > + When a pin has to be configured in alternate function mode, use this > > + property to identify the pin by its global index, and provide its > > + alternate function configuration description along with it. > > + When multiple pins are required to be configured as part of the same > > + alternate function (odds are single-pin alternate functions exist) they > > + shall be specified as members of the same argument list of a single > > + "renesas-pins" property. > > + Helper macros to ease calculating the pin index from its position > > + (port where it sits on and pin offset), and alternate function > > + configuration options are provided in pin controller header file at: > > the pin ... file > > > + include/dt-bindings/pinctrl/r7s72100-pinctrl.h > > Hence I'd include that file in this patch, as it's part of the bindings. > > > + Example: > > + A serial communication interface with a TX output pin and a RX input pin. > > an RX > > > + > > + &pinctrl { > > + scif2_pins: serial2 { > > + renesas,pins = <PIN(3, 0) 6>, > > + <PIN(3, 2) 4>; > > Single line? > > > + }; > > + } > > + > > + Pin #0 on port #3 is configured in alternate function #6. > > + Pin #2 on port #3 is configured in alternate function #4. > > as alternate function > > > + > > + Example 2: > > + I2c master: both SDA and SCL pins need bi-directional operations > > + > > + &pinctrl { > > + i2c2_pins: i2c2 { > > + renesas,pins = <PIN(1, 4) (1 | BI_DIR)>, > > + <PIN(1, 5) (1 | BI_DIR)>; > > + }; > > + } > > + > > + Pin #4 on port #1 is configured in alternate function #1. > > + Pin #5 on port #1 is configured in alternate function #1. > > as alternate function > > > + Both need to work in bi-directional mode. > > + > > + Example 3: > > + Multi-function timer input and output compare pins. > > + The hardware manual prescribes this pins to have input/output direction > > + specified by software. Configure TIOC0A as input and TIOC0B as output. > > + > > + &pinctrl { > > + tioc0_pins: tioc0 { > > + renesas,pins = <PIN(4, 0) (2 | SWIO_IN)>, > > + <PIN(4, 1) (2 | SWIO_OUT)>; > > + }; > > + } > > + > > + Pin #0 on port #4 is configured in alternate function #2 with IO direction > > + specified by software as input. > > + Pin #1 on port #4 is configured in alternate function #1 with IO direction > > + specified by software as output. > > as alternate function > > > +- GPIO controller sub-nodes: > > + Each port of r7s72100 pin controller hardware is itself a gpio controller. > > the r7s72100 pin controller hardware > > > + Different SoCs have different number of available pins per port, but > > numbers of > > > + generally speaking, each of them can be configured in GPIO ("port") mode > > + on this hardware. > > + Describe gpio-controllers using sub-nodes with the following properties. > > + > > + Required properties: > > + - gpio-controller > > + empty property as defined by gpio bindings documentation. > > the gpio bindings documentation > > > + - #gpio-cells > > + number of cells required to identify and configure a GPIO. > > + Shall be 2. > > + - gpio-ranges > > + Describes a gpio controller specifying its specific pin base, the pin > > + base in the global pin numbering space, and the number of controlled > > + pins, as defined by gpio bindings documentation. Refer to this file > > the gpio bindings documentation > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 23, 2017 at 5:02 PM, jacopo <jacopo@jmondi.org> wrote: >> > + Required properties: >> > + - renesas,pins >> >> Just "pins"? >> > > You know, I've been thinking about this, bu the "pins" property > definition in pinctrl-bidings is the following one: > > Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt > --- > - pins takes a list of pin names or IDs as a required argument. The > specific binding for the hardware defines: > - Whether the entries are integers or strings, and their > meaning. > --- > > And all examples there assume one "pin name" or "ID" per pin. > > Now, we use 2 values per each pin (the pin ID and the alternate > function number), so to me this is different from what the generic > binding describes. > Also, pinctrl-single, and pinctrl-imx which have and ABI similar to > the one this driver define, use "pinctrl-single,pins" and "fsl,pins" > respectively as property names. > So either they have to be updated yet, or we should keep using > "renesas,pins" for our own defined ABI. > > Maybe Linus or other pinctrl people can give some suggestion here. To me as subsystem maintainer any "necessarily different" bindings are just a big confusion for the head. Since you're adding a new driver, try to stick to the generic bindings even if it deviates from what you are used to for Renesas, because even if it may be more work for you guys or make you annoyed that now a certain Renesas is different from all other Renesas platforms, for the community this makes things easier to maintain because we can look at the driver and its bindings and say "ah I know this". The fact that historically all the early adopters of pinctrl in device tree have these funky custom bindings is unfortunate but just something that we need to live with. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Linus, On 2017-03-28 11:46, Linus Walleij wrote: > On Thu, Mar 23, 2017 at 5:02 PM, jacopo <jacopo@jmondi.org> wrote: > >>> > + Required properties: >>> > + - renesas,pins >>> >>> Just "pins"? >>> >> >> You know, I've been thinking about this, bu the "pins" property >> definition in pinctrl-bidings is the following one: >> >> Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt >> --- >> - pins takes a list of pin names or IDs as a required argument. The >> specific binding for the hardware defines: >> - Whether the entries are integers or strings, and their >> meaning. >> --- >> >> And all examples there assume one "pin name" or "ID" per pin. >> >> Now, we use 2 values per each pin (the pin ID and the alternate >> function number), so to me this is different from what the generic >> binding describes. >> Also, pinctrl-single, and pinctrl-imx which have and ABI similar to >> the one this driver define, use "pinctrl-single,pins" and "fsl,pins" >> respectively as property names. >> So either they have to be updated yet, or we should keep using >> "renesas,pins" for our own defined ABI. >> >> Maybe Linus or other pinctrl people can give some suggestion here. > > To me as subsystem maintainer any "necessarily different" bindings > are just a big confusion for the head. > Understandable :) > Since you're adding a new driver, try to stick to the generic bindings > even if it deviates from what you are used to for Renesas, because > even if it may be more work for you guys or make you annoyed that > now a certain Renesas is different from all other Renesas platforms, > for the community this makes things easier to maintain because > we can look at the driver and its bindings and say "ah I know this". > > The fact that historically all the early adopters of pinctrl in device > tree > have these funky custom bindings is unfortunate but just something > that we need to live with. > To avoid any confusion, please bear with me and clarify this once and for all, since I'm not certain I fully got you here. Are you suggesting: 1) Use "pins" property with the currently implemented ABI (which slightly differs from the standard documented one as explained above. Not sure it is fine overriding it or not) 2) Use "pins" property and change our ABI to match the documented one: one ID (integer or string) per pin, not 2 as we're doing now. Both solutions are easily implementable; 2) requires some more work to make pin id, function number and pin configuration fit in one single integer, but is achievable for sure. Thanks j > Yours, > Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 28, 2017 at 4:38 PM, <jacopo@jmondi.org> wrote: >> The fact that historically all the early adopters of pinctrl in device >> tree >> have these funky custom bindings is unfortunate but just something >> that we need to live with. >> > > To avoid any confusion, please bear with me and clarify this once and for > all, > since I'm not certain I fully got you here. > > Are you suggesting: > > 1) Use "pins" property with the currently implemented ABI (which slightly > differs > from the standard documented one as explained above. Not sure it is fine > overriding > it or not) Correction: you should be using the property "pinmux", because you are setting group and function at the same time. See for example: include/dt-bindings/pinctrl/mt65xx.h And how that is used in: arch/arm/boot/dts/mt2701-pinfunc.h arch/arm/boot/dts/mt2701-evb.dts The docs are here: Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt I'm sorry that "pinmux" is not part of the generic documentation, it'd be great if you would like to add it with a patch. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Linus, On Wed, Mar 29, 2017 at 4:30 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Tue, Mar 28, 2017 at 4:38 PM, <jacopo@jmondi.org> wrote: >>> The fact that historically all the early adopters of pinctrl in device >>> tree >>> have these funky custom bindings is unfortunate but just something >>> that we need to live with. >> >> To avoid any confusion, please bear with me and clarify this once and for >> all, >> since I'm not certain I fully got you here. >> >> Are you suggesting: >> >> 1) Use "pins" property with the currently implemented ABI (which slightly >> differs >> from the standard documented one as explained above. Not sure it is fine >> overriding >> it or not) > > Correction: you should be using the property "pinmux", because you > are setting group and function at the same time. OK. > See for example: > include/dt-bindings/pinctrl/mt65xx.h > > And how that is used in: > arch/arm/boot/dts/mt2701-pinfunc.h > arch/arm/boot/dts/mt2701-evb.dts > > The docs are here: > Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt All of the above pack the information for a pin into a single 32-bit integer. Is that what you want us to use, too? Currently we use two integers: 1) pin index, and 2) function/flags combo. > I'm sorry that "pinmux" is not part of the generic documentation, it'd be > great if you would like to add it with a patch. That would mean pinmux could be an array of single values, or tuples. Is that OK? Thanks! 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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 29, 2017 at 9:35 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Me: >> See for example: >> include/dt-bindings/pinctrl/mt65xx.h >> >> And how that is used in: >> arch/arm/boot/dts/mt2701-pinfunc.h >> arch/arm/boot/dts/mt2701-evb.dts >> >> The docs are here: >> Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt > > All of the above pack the information for a pin into a single 32-bit integer. > Is that what you want us to use, too? > Currently we use two integers: 1) pin index, and 2) function/flags combo. I don't really know what you need, sorry. But some kind of figure, yes. I would say whatever makes sense. 16+16 bits makes sense in most combinatorial spaces does it not? If you split 32 bits in 16 bits for pin and 16 bits for function, do you have more than 2^16 pins or 2^16 functions? If you really do we may need to go for u64 but ... really? Is there a rational reason for that other than "we did it like this first"? I do not understand the notion of "flags" here. I hope that is not referring to pin config, because I expect that to use the standard pin config bindings outside of the pinmux value which should just define the pin+function combo: node { pinmux = <PIN_NUMBER_PINMUX>; GENERIC_PINCONFIG; }; Example from Mediatek: i2c1_pins_a: i2c1@0 { pins { pinmux = <MT8135_PIN_195_SDA1__FUNC_SDA1>, <MT8135_PIN_196_SCL1__FUNC_SCL1>; bias-pull-up = <55>; }; }; So this allows for a combine pin+function number but pull ups, bias etc are not baked into the thing, they have to be added on separately with the generic bindings, which is nice and very readable. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Linus, On Wed, Mar 29, 2017 at 12:15 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Wed, Mar 29, 2017 at 9:35 AM, Geert Uytterhoeven > <geert@linux-m68k.org> wrote: >>> See for example: >>> include/dt-bindings/pinctrl/mt65xx.h >>> >>> And how that is used in: >>> arch/arm/boot/dts/mt2701-pinfunc.h >>> arch/arm/boot/dts/mt2701-evb.dts >>> >>> The docs are here: >>> Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt >> >> All of the above pack the information for a pin into a single 32-bit integer. >> Is that what you want us to use, too? >> Currently we use two integers: 1) pin index, and 2) function/flags combo. > > I don't really know what you need, sorry. But some kind of figure, yes. > I would say whatever makes sense. 16+16 bits makes sense in most > combinatorial spaces does it not? If you split 32 bits in 16 bits for > pin and 16 bits for function, do you have more than 2^16 pins or 2^16 > functions? > > If you really do we may need to go for u64 but ... really? Is there > a rational reason for that other than "we did it like this first"? > > I do not understand the notion of "flags" here. I hope that is not referring Flags refers to BI_DIR, SWIO_IN, and SWIO_OUT, from https://patchwork.kernel.org/patch/9643047/ 32-bit should be enough to cover pins, function, and flags. > to pin config, because I expect that to use the standard pin config > bindings outside of the pinmux value which should just define the > pin+function combo: > > node { > pinmux = <PIN_NUMBER_PINMUX>; > GENERIC_PINCONFIG; > }; > > Example from Mediatek: > > i2c1_pins_a: i2c1@0 { > pins { > pinmux = <MT8135_PIN_195_SDA1__FUNC_SDA1>, > <MT8135_PIN_196_SCL1__FUNC_SCL1>; If we follow this example, then we can list all combinations in include/dt-bindings/pinctrl/r7s72100-pinctrl.h, instead of creating the value by combining the bits using a macro where we need it in the DTS. It's gonna be a long list, though... > bias-pull-up = <55>; > }; > }; > > So this allows for a combine pin+function number but pull ups, > bias etc are not baked into the thing, they have to be added on > separately with the generic bindings, which is nice and very readable. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert, Linus On Wed, Mar 29, 2017 at 01:20:39PM +0200, Geert Uytterhoeven wrote: > Hi Linus, > > On Wed, Mar 29, 2017 at 12:15 PM, Linus Walleij > <linus.walleij@linaro.org> wrote: > > On Wed, Mar 29, 2017 at 9:35 AM, Geert Uytterhoeven > > <geert@linux-m68k.org> wrote: > >>> See for example: > >>> include/dt-bindings/pinctrl/mt65xx.h > >>> > >>> And how that is used in: > >>> arch/arm/boot/dts/mt2701-pinfunc.h > >>> arch/arm/boot/dts/mt2701-evb.dts > >>> > >>> The docs are here: > >>> Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt > >> > >> All of the above pack the information for a pin into a single 32-bit integer. > >> Is that what you want us to use, too? > >> Currently we use two integers: 1) pin index, and 2) function/flags combo. > > > > I don't really know what you need, sorry. But some kind of figure, yes. > > I would say whatever makes sense. 16+16 bits makes sense in most > > combinatorial spaces does it not? If you split 32 bits in 16 bits for > > pin and 16 bits for function, do you have more than 2^16 pins or 2^16 > > functions? > > > > If you really do we may need to go for u64 but ... really? Is there > > a rational reason for that other than "we did it like this first"? > > > > I do not understand the notion of "flags" here. I hope that is not referring > > Flags refers to BI_DIR, SWIO_IN, and SWIO_OUT, from > https://patchwork.kernel.org/patch/9643047/ > > 32-bit should be enough to cover pins, function, and flags. > Geert already replied, but to avoid any confusion I'll try to remove from driver the use of "pin config" when referring to this three flags, which are just additional informations the pin controller needs to perform pin muxing properly. They're not related the standard pin config properties (pull-up/down, bias etc.. actually our hardware does not even support these natively) > > to pin config, because I expect that to use the standard pin config > > bindings outside of the pinmux value which should just define the > > pin+function combo: > > > > node { > > pinmux = <PIN_NUMBER_PINMUX>; > > GENERIC_PINCONFIG; > > }; > > > > Example from Mediatek: > > > > i2c1_pins_a: i2c1@0 { > > pins { > > pinmux = <MT8135_PIN_195_SDA1__FUNC_SDA1>, > > <MT8135_PIN_196_SCL1__FUNC_SCL1>; > > If we follow this example, then we can list all combinations in > include/dt-bindings/pinctrl/r7s72100-pinctrl.h, instead of creating the value > by combining the bits using a macro where we need it in the DTS. > > It's gonna be a long list, though... > I'm strongly in favour of something like pinmux = <PIN(1, 4) | FUNC# | FLAGS>, .... ; opposed to pinmux = <PIN1_4_FUNC#_FLAGS>, ... ; Not only because it will save use from having a loong list(*) of macros that has to be kept up to date when/if new RZ hardware will arrive, but also because of readability and simplicity for down-stream and BSP users. Speaking of which, I would like to know what does Chris think of this. Is there any strong preference between the two for you Linus? Thanks j (*) 12 ports, 16 pins, 8 functions, 3 flags: you can do the math yourselves and see this is going to be hard to maintain. > > bias-pull-up = <55>; > > }; > > }; > > > > So this allows for a combine pin+function number but pull ups, > > bias etc are not baked into the thing, they have to be added on > > separately with the generic bindings, which is nice and very readable. > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Adding my 2 cents here: On Wednesday, March 29, 2017, jacopo wrote: > > > If you really do we may need to go for u64 but ... really? Is there > > > a rational reason for that other than "we did it like this first"? > > > > > > I do not understand the notion of "flags" here. I hope that is not > > > referring > > > > Flags refers to BI_DIR, SWIO_IN, and SWIO_OUT, from > > https://patchwork.kernel.org/patch/9643047/ > > > > 32-bit should be enough to cover pins, function, and flags. > > > > Geert already replied, but to avoid any confusion I'll try to remove from > driver the use of "pin config" when referring to this three flags, which > are just additional informations the pin controller needs to perform pin > muxing properly. They're not related the standard pin config properties > (pull-up/down, bias etc.. actually our hardware does not even support > these natively) > > > > to pin config, because I expect that to use the standard pin config > > > bindings outside of the pinmux value which should just define the > > > pin+function combo: > > > > > > node { > > > pinmux = <PIN_NUMBER_PINMUX>; > > > GENERIC_PINCONFIG; > > > }; > > > > > > Example from Mediatek: > > > > > > i2c1_pins_a: i2c1@0 { > > > pins { > > > pinmux = <MT8135_PIN_195_SDA1__FUNC_SDA1>, > > > <MT8135_PIN_196_SCL1__FUNC_SCL1>; > > > > If we follow this example, then we can list all combinations in > > include/dt-bindings/pinctrl/r7s72100-pinctrl.h, instead of creating > > the value by combining the bits using a macro where we need it in the > DTS. > > > > It's gonna be a long list, though... > > > > I'm strongly in favour of something like > pinmux = <PIN(1, 4) | FUNC# | FLAGS>, .... ; > > opposed to > pinmux = <PIN1_4_FUNC#_FLAGS>, ... ; I agree. I like "<PIN(1, 4) | FUNC# | FLAGS>". > Not only because it will save use from having a loong list(*) of macros > that has to be kept up to date when/if new RZ hardware will arrive, but > also because of readability and simplicity for down-stream and BSP users. > Speaking of which, I would like to know what does Chris think of this. The list of macros would be very long, especially against the different packaging version of the RZ/A1 series. 11 ports, 16-pins for each port, 8 different function options for each pin....2 different package/pin variations. And at the end of the day....there is no benefit for the user over just using a macro. A little about "this controller" for the RZ/A1: In my opinion it's a one-shot usage. I don't foresee future RZ/A SoCs using this exact pin controller. The reason for the "FLAGS" is to work around a quirky hardware design (in my opinion). However, future RZ/A SoCs will use a 'similar' type controller where each pin has 8 function options (but the FLAGs won't be needed anymore...as far as I can see...). So I foresee the DT interface staying more or less the same, but the underlying register setup in the driver Will change (become more simple). Now...if we can only convince the R-Car guys to move to a more simple pin-mux controller... Chris
On Wed, Mar 29, 2017 at 1:20 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >> I do not understand the notion of "flags" here. I hope that is not referring > > Flags refers to BI_DIR, SWIO_IN, and SWIO_OUT, from > https://patchwork.kernel.org/patch/9643047/ Aha I will go in and review that closer because it doesn't seem right. Sorry for missing it. >> i2c1_pins_a: i2c1@0 { >> pins { >> pinmux = <MT8135_PIN_195_SDA1__FUNC_SDA1>, >> <MT8135_PIN_196_SCL1__FUNC_SCL1>; > > If we follow this example, then we can list all combinations in > include/dt-bindings/pinctrl/r7s72100-pinctrl.h, instead of creating the value > by combining the bits using a macro where we need it in the DTS. > > It's gonna be a long list, though... Size is not the issue, readability is the issue. I don't see why you would need to list "all combinations" since the trees go through the C preprocessor so you can use macros and bit | OR to build them? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 29, 2017 at 2:38 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote: >> I'm strongly in favour of something like >> pinmux = <PIN(1, 4) | FUNC# | FLAGS>, .... ; >> >> opposed to >> pinmux = <PIN1_4_FUNC#_FLAGS>, ... ; > > > I agree. I like "<PIN(1, 4) | FUNC# | FLAGS>". I don't know if you have seen the Mediatek things really. include/dt-bindings/pinctrl/mt6397-pinfunc.h Example: #define MT6397_PIN_2_SRCLKEN_PERI__FUNC_GPIO2 (MTK_PIN_NO(2) | 0) If you prefer to use preprocessor macros or whatever to make the bitmasks or how you want to organize the constants in your include files is not my concern, do whatever you seem fit, just pack it into a 32bit thing somehow which makes sense from a maintenance point of view. >> Not only because it will save use from having a loong list(*) of macros >> that has to be kept up to date when/if new RZ hardware will arrive, but >> also because of readability and simplicity for down-stream and BSP users. >> Speaking of which, I would like to know what does Chris think of this. > > The list of macros would be very long, especially against the different > packaging version of the RZ/A1 series. 11 ports, 16-pins for each port, 8 different > function options for each pin....2 different package/pin variations. > > And at the end of the day....there is no benefit for the user over just using > a macro. I don't know who has this idea that you could not use macros, certainly not me. Some misunderstanding must be going on. For what I'm concerned you can write hex numbers in the pinmux = <0x12345678>; > The reason for the "FLAGS" is to work around a quirky hardware design (in my opinion). The flags I don't like at all, and think they should be converted to generic pin config because they have nothing to do with muxing. But I will point that out in the specific patch adding them. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, March 29, 2017, Linus Walleij: > If you prefer to use preprocessor macros or whatever to make the bitmasks > or how you want to organize the constants in your include files is not my > concern, do whatever you seem fit, just pack it into a 32bit thing somehow > which makes sense from a maintenance point of view. OK, I think everyone agrees that a single 32-bit value is fine because macros will also for good readability and maintenance. > >> Not only because it will save use from having a loong list(*) of > >> macros that has to be kept up to date when/if new RZ hardware will > >> arrive, but also because of readability and simplicity for down-stream > and BSP users. > >> Speaking of which, I would like to know what does Chris think of this. > > > > The list of macros would be very long, especially against the > > different packaging version of the RZ/A1 series. 11 ports, 16-pins for > > each port, 8 different function options for each pin....2 different > package/pin variations. > > > > And at the end of the day....there is no benefit for the user over > > just using a macro. > > I don't know who has this idea that you could not use macros, certainly > not me. Some misunderstanding must be going on. For what I'm concerned you > can write hex numbers in the pinmux = <0x12345678>; > > > The reason for the "FLAGS" is to work around a quirky hardware design > (in my opinion). > > The flags I don't like at all, and think they should be converted to > generic pin config because they have nothing to do with muxing. > > But I will point that out in the specific patch adding them. OK, I think I understand your issue a little better of mixing user-defined config with generic pinmux. From our perspective, the FLAGS (BI_DIR, SWIO_IN, and SWIO_OUT ) were not really optional when selecting what you want the pin to do....so we considered it part of the pin-mux. In the hardware manual, there are tables that say that for 'some' certain pins/functions "just setting the pin to a function is not enough...you need to make another register setting (that may or may not make sense), otherwise it's not going to work". So, trying to get the driver to be that smart across all the different pin/package variations seems to be way too ugly (and a maintenance nightmare). Simply putting that in the DT binding was much more cleaner. As for how to pass this HW info into the driver, I'll move over to the other email thread where you started to give some suggestions... Chris
diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt new file mode 100644 index 0000000..0474860 --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt @@ -0,0 +1,144 @@ +Renesas RZ/A1 combined Pin and GPIO controller + +Renesas SoCs of RZ/A1 family feature a combined Pin and GPIO controller +hardware controller, named "Ports" in the hardware reference manual. +Pin multiplexing and GPIO configuration is performed on a per-pin basis +writing configuration values to per-port register sets. +Each "port" features up to 16 pins, each of them configurable for GPIO +function (port mode) or in alternate function mode. +Up to 8 different alternate function modes exist for each single pin. + +Pin controller node +------------------- + +Required properties: + - compatible + this shall be "renesas,r7s72100-ports". + + - #pinctrl-cells + as defined by pinctrl-bindings.txt, this is the number + of cells (in addition to pin index) required to configure a single pin. + Shall be set to 1. + + - reg + address base and length of the memory area where pin controller + hardware is mapped to. + +Example: +Pin controller node for RZ/A1H SoC (r7s72100) + +pinctrl: pinctrl@fcfe3000 { + compatible = "renesas,r7s72100-ports"; + + #pinctrl-cells = <1>; + + reg = <0xfcfe3000 0x4248>; +}; + +Sub-nodes +--------- + +The child nodes of the pin controller node describe a pin multiplexing +function or a gpio controller alternatively. + +- Pin multiplexing sub-nodes: + A pin multiplexing sub-node describes how to configure a set of + (or a single) pin in some desired alternate function mode. + A single sub-node may define several pin configurations groups. + + Required properties: + - renesas,pins + describes an array of pin multiplexing configurations. + When a pin has to be configured in alternate function mode, use this + property to identify the pin by its global index, and provide its + alternate function configuration description along with it. + When multiple pins are required to be configured as part of the same + alternate function (odds are single-pin alternate functions exist) they + shall be specified as members of the same argument list of a single + "renesas-pins" property. + Helper macros to ease calculating the pin index from its position + (port where it sits on and pin offset), and alternate function + configuration options are provided in pin controller header file at: + include/dt-bindings/pinctrl/r7s72100-pinctrl.h + + Example: + A serial communication interface with a TX output pin and a RX input pin. + + &pinctrl { + scif2_pins: serial2 { + renesas,pins = <PIN(3, 0) 6>, + <PIN(3, 2) 4>; + }; + } + + Pin #0 on port #3 is configured in alternate function #6. + Pin #2 on port #3 is configured in alternate function #4. + + Example 2: + I2c master: both SDA and SCL pins need bi-directional operations + + &pinctrl { + i2c2_pins: i2c2 { + renesas,pins = <PIN(1, 4) (1 | BI_DIR)>, + <PIN(1, 5) (1 | BI_DIR)>; + }; + } + + Pin #4 on port #1 is configured in alternate function #1. + Pin #5 on port #1 is configured in alternate function #1. + Both need to work in bi-directional mode. + + Example 3: + Multi-function timer input and output compare pins. + The hardware manual prescribes this pins to have input/output direction + specified by software. Configure TIOC0A as input and TIOC0B as output. + + &pinctrl { + tioc0_pins: tioc0 { + renesas,pins = <PIN(4, 0) (2 | SWIO_IN)>, + <PIN(4, 1) (2 | SWIO_OUT)>; + }; + } + + Pin #0 on port #4 is configured in alternate function #2 with IO direction + specified by software as input. + Pin #1 on port #4 is configured in alternate function #1 with IO direction + specified by software as output. + +- GPIO controller sub-nodes: + Each port of r7s72100 pin controller hardware is itself a gpio controller. + Different SoCs have different number of available pins per port, but + generally speaking, each of them can be configured in GPIO ("port") mode + on this hardware. + Describe gpio-controllers using sub-nodes with the following properties. + + Required properties: + - gpio-controller + empty property as defined by gpio bindings documentation. + - #gpio-cells + number of cells required to identify and configure a GPIO. + Shall be 2. + - gpio-ranges + Describes a gpio controller specifying its specific pin base, the pin + base in the global pin numbering space, and the number of controlled + pins, as defined by gpio bindings documentation. Refer to this file + for a more detailed description. + + Example: + A gpio controller node, controlling 16 pins indexed from 0. + The gpio controller base in the global pin indexing space is pin 48, thus + pins [0 - 15] on this controller map to pins [48 - 63] in the global pin + indexing space. + + port3: gpio-3 { + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 48 16>; + }; + + A device node willing to use pins controlled by this gpio controller, shall + refer to it as follows: + + led1 { + gpios = <&port3 10 GPIO_ACTIVE_LOW>; + };
Add device tree bindings documentation for Renesas RZ/A1 gpio and pin controller. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- .../bindings/pinctrl/renesas,rza1-pinctrl.txt | 144 +++++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt