Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1570106 > unrolled thread

Re: [PATCH v3 01/14] Documentation: dt/bindings: Document pinctrl-ingenic

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 01/14] Documentation: dt/bindings: Document  pinctrl-ingenic Rob Herring <robh@kernel.org> - 2017-01-30 21:40 +0100
    Re: [PATCH v3 01/14] Documentation: dt/bindings: Document  pinctrl-ingenic Paul Cercueil <paul@crapouillou.net> - 2017-01-31 11:40 +0100
      Re: [PATCH v3 01/14] Documentation: dt/bindings: Document pinctrl-ingenic Linus Walleij <linus.walleij@linaro.org> - 2017-01-31 14:20 +0100
        Re: [PATCH v3 01/14] Documentation: dt/bindings: Document  pinctrl-ingenic Paul Cercueil <paul@crapouillou.net> - 2017-02-09 18:30 +0100

#1570106 — Re: [PATCH v3 01/14] Documentation: dt/bindings: Document pinctrl-ingenic

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

[toc] | [next] | [standalone]

#1570584

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

[toc] | [prev] | [next] | [standalone]

#1570726 — Re: [PATCH v3 01/14] Documentation: dt/bindings: Document pinctrl-ingenic

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

[toc] | [prev] | [next] | [standalone]

#1577809

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?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web