Hi Grant, Thomas, do you see any potential problems with this patch? It adds sysfs interface to change / set-up trigger edge from the kernel (currently not possible).
BR, Drasko On Fri, Oct 5, 2012 at 3:16 PM, Drasko DRASKOVIC <drasko.drasko...@gmail.com> wrote: > On Fri, Oct 5, 2012 at 2:40 PM, Linus Walleij <linus.wall...@linaro.org> > wrote: >> On Fri, Oct 5, 2012 at 2:20 PM, Drasko DRASKOVIC >> Why not put the above text into the patch commit blurb? > > OK, I can add this easily. Makes sense. > > >> If I understand correctly the below more or less exports >> struct irq_chip to userspace, >> trying to hide it by instead exposing a property of the >> containing struct gpio_chip and it worries me. > > No, it should not. It operates only on already exported gpiochip > (similar to gpio_export_link()). > It just helps exported GPIO be configured in "interrupt" and not in > "normal" mode. > >> >> One possible comment is that you shouldn't >> add this to gpiolib, if you want to mess around with the >> irq_chip you should create sysfs entries for the irq_chip >> per se, then we can have a symbolic link from the >> gpio_chip to its irq_chip in sysfs, and you can follow that >> to change trigger flanks, right? > > I did not found any correlation between kernel space irq_chip and > gpiolib's internal stuctures describing interrupt. > > This patch implementation seems like reasonable solution to me, but > still leaves possibility for someone to configure GPIO interrupt mode > internally (within driver) different than it is declared lately to > sysfs by calling my function... > > Otherwise, a pointer to an edge set-up kernel function can be added to > the gpio_chip structure, but I think it will be slightly more > complicated, and will fall back to the same thing. > >> >> Part of me doesn't like it when userspace is messing >> around with this kind of thing. Why can't this be set >> up from the kernel itself by some jam table? >> >> I can understand it if this is some lab board with GPIO >> or so, if it's some embedded GPIO controller within >> a laptop or something it surely should be in kernelspace. > > Yes, it is intended for embedded devices, where most (if not all) of > GPIO configuration should be done by the kernel, but also this > configuration should be appropriately exported to sysfs, so that > user-space applications could start using it right after the boot. > > >> So please detail your usecase a bit... what is the code >> daemon etc in userspace poking around at these pins? >> What is is doing and why? > > Right now, sysfs exposes interface like this for GPIO IRQ type configuration : > > # cat /sys/class/gpio/gpioX/edge > none > # echo rising > /sys/class/gpio/gpioX/edge > > This example puts GPIO pin X into "interrupt" mode, and declares it's > "type" i.e. that it triggers on rising edge. > > In the world of embedded, system configuratio which tells which GPIO > pins should be configured in "interrupt" mode (and what is their > trigger type) is kept in some format usually known only to the kernel > driver. We have no need to export this format to the user space, so > that rc scripts for example would know to parse GPIO configuration and > execute operations I mentioned above. > > To avoid that this is done from the user space, a function accesible > to kernel is needed. > > BR, > Drasko -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
