Re: [PATCH] gpio: zynq: add a to_irq implementation

[Jean-Francois Dagenais <jeff.dagenais@xxxxxxxxx>]

Thank you for your thoughts!

> On May 20, 2019, at 02:38, Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx> wrote:
> 
>> 
>> Any thoughts?
>> 
> 
> Yes! Don't use sysfs and especially don't add your own buggy
> interfaces?

aawww... but I like adding a bunch of "buggy" interfaces here and there to make
sure my customers get a challenge when using my product. :P

BTW, I've always been fully aware, when I made this change in gpiolib, that I
would not be upstreaming it. It's been years and we're quite happy with what it
allowed us to do. Only now, because I needed to use the "edge" attribute, have I
had any problems with this approach. So it has served us well this far. I was
just long-shot probing you the other day.

> Is there any reason you can't use libgpiod and the
> character device?

To be honest, I had only glanced quite a while back at gpiolib and thought it
was a bit overkill at that stage of our development to include another C library
and methods into the mix. Later, I forgot about it. Looking at it again here I
can see the command line tools do satisfy a great part of the "simplicity"
argument. I will most likely add gpiolib to my base yocto image, thanks!

> What does your own class provide that none of the
> upstream interfaces do?

I have not fully explored gpiolib's API yet so I may be missing something but
the hog/auto-export I use allows for the hardware pin to be described
functionally all the way to userspace and abstract what chip a line is on and
whether it is active_low or high.

So for example a line name could be MYSUBCIRCUIT_RESET. From the DTS, that line
name is given, but in the hog/auto-export, it would be shown at
/sys/class/gpio/mysubcircuit_enable. So a functional name is given
(mysubcircuit_enable), along with ACTIVE_LOW to abstract the line polarity. That
changes this particular gpio's function to a "cleaner" api (an enable gpio
rather than a reset gpio). All along, userspace is none the wiser in that
hardware "details", other than the gpio functional name, need not be present in
the userspace consumer.

This is akin to having positive boolean in a program. Hardware designers have
different physical reasons for making "not" signals (inverted). This should stay
in the DTS where we already describe the hardware.

If you can describe a pin "functionally" from DTS and still use gpiolib, I will
stand corrected.

Thanks for your feedback!