| Message ID | 20170125185207.23902-2-paul@crapouillou.net |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, Jan 25, 2017 at 07:51:54PM +0100, Paul Cercueil wrote: > This commit adds documentation for the devicetree bidings of the > pinctrl-ingenic driver, which handles pin configuration and pin > muxing of the Ingenic SoCs currently supported by the Linux kernel. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > --- > .../bindings/pinctrl/ingenic,pinctrl.txt | 77 ++++++++++++++++++++++ > 1 file changed, 77 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt > > v2: Rewrote the documentation for the new pinctrl-ingenic driver > v3: No changes > > diff --git a/Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt > new file mode 100644 > index 000000000000..ead5b01ad471 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt > @@ -0,0 +1,77 @@ > +Ingenic jz47xx pin controller > + > +Please refer to pinctrl-bindings.txt in this directory for details of the > +common pinctrl bindings used by client devices, including the meaning of the > +phrase "pin configuration node". > + > +For the jz47xx SoCs, pin control is tightly bound with GPIO ports. All pins may > +be used as GPIOs, multiplexed device functions are configured within the > +GPIO port configuration registers and it is typical to refer to pins using the > +naming scheme "PxN" where x is a character identifying the GPIO port with > +which the pin is associated and N is an integer from 0 to 31 identifying the > +pin within that GPIO port. For example PA0 is the first pin in GPIO port A, and > +PB31 is the last pin in GPIO port B. The jz4740 contains 4 GPIO ports, PA to > +PD, for a total of 128 pins. The jz4780 contains 6 GPIO ports, PA to PF, for a > +total of 192 pins. From the overlapping register addresses in the examples and this description, it looks like the pinctrlr and gpio controller are 1 block. If so, then there should only be 1 node. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, > From the overlapping register addresses in the examples and this > description, it looks like the pinctrlr and gpio controller are 1 > block. > If so, then there should only be 1 node. Well, that's what I had until Linus W. just told me to do the opposite: > Just pull all these down two levels and make them one device > each instead of having them inside the pin controller node > like this. -Paul -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 31, 2017 at 11:31 AM, Paul Cercueil <paul@crapouillou.net> wrote: > [Rob]: >> From the overlapping register addresses in the examples and this >> description, it looks like the pinctrlr and gpio controller are 1 block. >> If so, then there should only be 1 node. > > Well, that's what I had until Linus W. just told me to do the opposite: > >> Just pull all these down two levels and make them one device >> each instead of having them inside the pin controller node >> like this. I guess the argument is that they are in the same coherent memory range so they should be one device node. That is how we handle e.g. system controllers so it makes some sense. So can the two GPIO controllers be modeled as two subnodes of the pin controller then? Subnodes are certainly OK, we have that for many other devices such as interrupt controllers on PCI bridges and what not. So when the probing of the pin controller is ready it can just walk down and populate the GPIO subdevices with of_platform_default_populate() or simply by registering the device directly with platform_device_add_data() just like an MFD device does? This is nice because we want to use the standard gpio ranges to map pins to GPIO lines. I'm sorry about the unclarities here, but it's essentially an intrinsic problem with GPIO that has been with us for years: do we model each "bank" as a device or do we just register each bank as a gpiochip, or do we even make one gpiochip to cover all the banks. All solutions can be found in the kernel... also the different DT bindings: one node for a whole slew of GPIO controllers, or seveal nodes and I bet also several nodes for memory ranges in close proximity. I don't know for sure what is the most elegant solution, we might need to build some consensus here for the future so it doesn't get to heterogeneous. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le 2017-01-31 14:09, Linus Walleij a écrit : > On Tue, Jan 31, 2017 at 11:31 AM, Paul Cercueil <paul@crapouillou.net> > wrote: >> [Rob]: >>> From the overlapping register addresses in the examples and this >>> description, it looks like the pinctrlr and gpio controller are 1 >>> block. >>> If so, then there should only be 1 node. >> >> Well, that's what I had until Linus W. just told me to do the >> opposite: >> >>> Just pull all these down two levels and make them one device >>> each instead of having them inside the pin controller node >>> like this. > > I guess the argument is that they are in the same coherent memory > range so they should be one device node. That is how we handle > e.g. system controllers so it makes some sense. > > So can the two GPIO controllers be modeled as two subnodes of > the pin controller then? > > Subnodes are certainly OK, we have that for many other devices > such as interrupt controllers on PCI bridges and what not. > > So when the probing of the pin controller is ready it can just > walk down and populate the GPIO subdevices with > of_platform_default_populate() or simply by registering the > device directly with platform_device_add_data() just like an > MFD device does? > > This is nice because we want to use the standard gpio ranges > to map pins to GPIO lines. > > I'm sorry about the unclarities here, but it's essentially an intrinsic > problem with GPIO that has been with us for years: do we model > each "bank" as a device or do we just register each bank as a > gpiochip, or do we even make one gpiochip to cover all the banks. > All solutions can be found in the kernel... also the different DT > bindings: > one node for a whole slew of GPIO controllers, or seveal nodes > and I bet also several nodes for memory ranges in close proximity. > > I don't know for sure what is the most elegant solution, we might > need to build some consensus here for the future so it doesn't > get to heterogeneous. > > Yours, > Linus Walleij I was thinking that instead of having one pinctrl-ingenic instance covering 0x600 of register space, and 6 instances of gpio-ingenic having 0x100 each, I could just have 6 instances of pinctrl-ingenic, each one with an instance of gpio-ingenic declared as a sub-node, each handling just 0x100 of memory space. Then I can make pinctrl-ingenic and gpio-ingenic share a regmap (through syscon), which would be a good idea anyway since the two drivers poke to the very same registers (in theory not at the same time, but it's never safe to assume things like this). Problem is, that in that case the pin functions/groups (and ingenic,pull-ups) would have to be in DTS because we would have 6 instances with different pin groups, and I know you hate that. Thoughts? -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 9, 2017 at 6:28 PM, Paul Cercueil <paul@crapouillou.net> wrote: > I was thinking that instead of having one pinctrl-ingenic instance covering > 0x600 of register space, and 6 instances of gpio-ingenic having 0x100 each, > I could just have 6 instances of pinctrl-ingenic, each one with an instance > of gpio-ingenic declared as a sub-node, each handling just 0x100 of memory > space. My head is spinning, but I think I get it. What is wrong with the solution I proposed with one pin control instance covering the whole 0x600 and with 6 subnodes of GPIO? The GPIO nodes do not even have to have an address range associated with them you know, that can be distributed out with regmap code accessing the parent regmap. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le 2017-02-20 14:56, Linus Walleij a écrit : > On Thu, Feb 9, 2017 at 6:28 PM, Paul Cercueil <paul@crapouillou.net> > wrote: > >> I was thinking that instead of having one pinctrl-ingenic instance >> covering >> 0x600 of register space, and 6 instances of gpio-ingenic having 0x100 >> each, >> I could just have 6 instances of pinctrl-ingenic, each one with an >> instance >> of gpio-ingenic declared as a sub-node, each handling just 0x100 of >> memory >> space. > > My head is spinning, but I think I get it. What is wrong with the > solution > I proposed with one pin control instance covering the whole 0x600 and > with 6 > subnodes of GPIO? > > The GPIO nodes do not even have to have an address range associated > with > them you know, that can be distributed out with regmap code accessing > the parent regmap. OK, but then each GPIO chip 'X' still need to know its offset in the register area, which is (pinctrl_base + X * 0x100). What's the best way to pass that info to the driver? (I assume it's not with a custom DT binding...). Regards, -Paul -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 21, 2017 at 12:20 PM, Paul Cercueil <paul@crapouillou.net> wrote: > Le 2017-02-20 14:56, Linus Walleij a écrit : >> >> On Thu, Feb 9, 2017 at 6:28 PM, Paul Cercueil <paul@crapouillou.net> >> wrote: >> >>> I was thinking that instead of having one pinctrl-ingenic instance >>> covering >>> 0x600 of register space, and 6 instances of gpio-ingenic having 0x100 >>> each, >>> I could just have 6 instances of pinctrl-ingenic, each one with an >>> instance >>> of gpio-ingenic declared as a sub-node, each handling just 0x100 of >>> memory >>> space. >> >> >> My head is spinning, but I think I get it. What is wrong with the >> solution >> I proposed with one pin control instance covering the whole 0x600 and with >> 6 >> subnodes of GPIO? >> >> The GPIO nodes do not even have to have an address range associated with >> them you know, that can be distributed out with regmap code accessing >> the parent regmap. > > > OK, but then each GPIO chip 'X' still need to know its offset in the > register > area, which is (pinctrl_base + X * 0x100). > What's the best way to pass that info to the driver? (I assume it's not with > a custom DT binding...). I do not really understand what driver you are referring to. If the pin controller node is overarching and spawning children for the gpiochips, you use the design pattern from MFD to pass data from parents to children, e.g.: #include <linux/regmap.h> pinctrl driver: struct regmap_config mapconf = { .reg_bits = 32, .val_bits = 32, .reg_stride = 4, }; struct regmap *map; map = regmap_init_mmio(dev, base, &mapconf); if (IS_ERR(map)) .... dev_set_drvdata(dev, map); of_populate_children(dev,).. (can also use platform_device_add_data() or "simple-bus" etc) gpio subdrivers: struct regmap *map; map = dev_get_drvdata(dev->parent); There are examples of drivers passing more complex things to their children than a regmap, just put some struct in a <linux/*/*.h> file and pass it with drvdata as per above. PS i2c_set_drvdata(), platform_set_drvdata() are just aliases for dev_set_drvdata(). Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
This is the v4 of my patchset that introduces the pinctrl-ingenic and gpio-ingenic drivers. The drivers were rewritten based on the feedback received on the v3 version. The pinctrl-ingenic driver now contains the pinmux/pinconf information of each compatible SoC, so the devicetree bindings have been simplified greatly. The driver now uses regmap for accessing the registers. This regmap is shared to optional instances of the gpio-ingenic driver, that are instanciated as MFD cells of the pinctrl-ingenic driver. -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt new file mode 100644 index 000000000000..ead5b01ad471 --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt @@ -0,0 +1,77 @@ +Ingenic jz47xx pin controller + +Please refer to pinctrl-bindings.txt in this directory for details of the +common pinctrl bindings used by client devices, including the meaning of the +phrase "pin configuration node". + +For the jz47xx SoCs, pin control is tightly bound with GPIO ports. All pins may +be used as GPIOs, multiplexed device functions are configured within the +GPIO port configuration registers and it is typical to refer to pins using the +naming scheme "PxN" where x is a character identifying the GPIO port with +which the pin is associated and N is an integer from 0 to 31 identifying the +pin within that GPIO port. For example PA0 is the first pin in GPIO port A, and +PB31 is the last pin in GPIO port B. The jz4740 contains 4 GPIO ports, PA to +PD, for a total of 128 pins. The jz4780 contains 6 GPIO ports, PA to PF, for a +total of 192 pins. + + +Pin controller node +=================== + +Required properties: +- compatible: One of: + - "ingenic,jz4740-pinctrl" + - "ingenic,jz4780-pinctrl" + +Optional properties: +- ingenic,pull-ups: A list of 32-bit bit fields, where each bit set tells the + driver that a pull-up resistor is available for this pin. + By default, the driver considers that all pins feature a pull-up resistor. +- ingenic,pull-downs: A list of 32-bit bit fields, where each bit set tells + the driver that a pull-down resistor is available for this pin. + By default, the driver considers that all pins feature a pull-down + resistor. + + +'functions' sub-node +==================== + +The 'functions' node will contain sub-nodes that correspond to pin function +nodes, and no properties. Pin function nodes will contain sub-nodes that +correspond to pin groups, and no properties. + +The names of the pin function nodes will end up being the available functions +provided by the pinctrl driver. +The names of the pin group nodes will end up being the available groups +provided by the pinctrl driver. + +Required properties for pin groups: +- ingenic,pins: <pin mode [pin mode ...]>; + where 'pin' is the number of the pin, and 'mode' is the function mode of the + pin that should be enabled for this group. + + +Example: +======= + +pinctrl: ingenic-pinctrl@10010000 { + compatible = "ingenic,jz4740-pinctrl"; + reg = <0x10010000 0x400>; + + ingenic,pull-ups = <0xffffffff 0xffffffff 0xffffffff 0xdfffffff>; + ingenic,pull-downs = <0x00000000 0x00000000 0x00000000 0x00000000>; + + functions { + mmc { + mmc-1bit { + /* CLK, CMD, D0 */ + ingenic,pins = <0x69 0 0x68 0 0x6a 0>; + }; + + mmc-4bit { + /* D1, D2, D3 */ + ingenic,pins = <0x6b 0 0x6c 0 0x6d 0>; + }; + }; + }; +};
This commit adds documentation for the devicetree bidings of the pinctrl-ingenic driver, which handles pin configuration and pin muxing of the Ingenic SoCs currently supported by the Linux kernel. Signed-off-by: Paul Cercueil <paul@crapouillou.net> --- .../bindings/pinctrl/ingenic,pinctrl.txt | 77 ++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt v2: Rewrote the documentation for the new pinctrl-ingenic driver v3: No changes