Re: Subject: [PATCH 00/22] gpio: xilinx: update driver to match official xilinx version

[Michal Simek <michal.simek@xxxxxxxxxx>]

On 6.8.2018 15:57, Brandon Maier wrote:
> Hello, we've been trying to integrate with Xilinx's gpio driver. And
> wanted to provide some feedback from issues we've had.
> 
> On Sun, Aug 5, 2018 at 4:52 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>>
>> On Sun, Aug 5, 2018 at 10:06 PM Alexander Hedges <ahedges@xxxxxxx> wrote:
> ...
>>>   Sadly, the gpio-xilinx is currently one out of three devices where
>>>   #gpio-cells is allowed to be larger than 2, so rather an exception.
> ...
>> I do not see it being other than two, which is standard.
> 
> I believe he's talking about the version of the driver as it exists in
> Xilinx's tree. They add a 3rd cell which can be either 0 or
> XGPIO_CHANNEL_OFFSET (0x8). Where 0 == the first "channel" and 0x8 ==
> second "channel". It's so that the devicetree can resolve phandles
> correctly because both chips are registered under a single devicetree
> node.
> 
> For example,
> 
> gpio0: mygpio@x {
>    compatible = "xlnx,xps-gpio-1.00.a"
>    ...
> }
> driver1 {
>     // Gpio #0 of first channel
>     gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
> }
> driver2 {
>     // Gpio #0 of second channel
>     gpios = <&gpio0 XGPIO_CHANNEL_OFFSET GPIO_ACTIVE_HIGH>;
> }
> 
> This design has been problematic though, as we've been trying to
> switch to the new gpio-line-names and gpio-hog features. But they
> don't work well with gpio-xilinx because both gpio_chips are
> registered under the same devicetree node. E.g. if you put
> gpio-line-names into the xilinx-gpio, it will try to register the same
> names to both chips. It would make this driver easier to use if it
> either a) the driver registered each chip under a separate sub-node
> 
> e.g.
> mygpio@x {
>   compatible = "xlnx,xps-gpio-1.00.a"
>   chip0 {
>     gpio-line-names = ...
>   }
>   chip1 {
>     gpio-line-names = ...
>   }
> }
> 
> or b) have the driver register both "channels" as one continuous gpio
> chip, so that "channel 1" becomes gpios 0-31 and "channel 2" gpios
> 32-63. (b) would seem preferable as that's what some of the other
> drivers do, e.g. gpio-zynq does this with its multiple gpio banks.
> However it would require more invasive changes to Xilinx's tree to get
> working.

This 3rd cell format was introduced in 2013 just in xilinx tree and will
never applied to mainline. Maybe we tried to upstream it but it wasn't
accepted but doesn't matter.
I think that the best is to have only 2 cells format with one continues
gpios. But I don't think we need to allocate all pins when they are unused.

From
xlnx,gpio-width = <0x2>;
xlnx,gpio2-width = <0x4>;
you know that the first channel have 2 pins and second 4.
It means driver should allocate just 6 gpio lines and driver translation
function will identify it properly.

It means this translation.

&gpio 0 X - channel 0, pin 0
&gpio 1 X - channel 0, pin 1
&gpio 2 X - channel 1, pin 0
&gpio 3 X - channel 1, pin 1
&gpio 4 X - channel 1, pin 2
&gpio 5 X - channel 1, pin 3

3 cell format can be marked as deprecated in xilinx soc tree but I
believe that this can be detected that both bindings can be supported
for some time in xilinx soc vendor tree.

Thanks,
Michal

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html