Hi Roman, Thanks for this work. Comments below.
On Thu, Feb 26, 2009 at 7:24 AM, Roman Fietze <roman.fie...@telemotive.de> wrote: > Hello, > > I've got a target derived from the Lite5200 that needs to use simple > interrupt GPIO pins. I created a patch to support this kind of GPIO. > > I would need your opinion and like to hear any criticism. Esp. the > facts that I ad to split up struct mpc52xx_gpio and that the GPIO > numbering might get mixed up concern me. While I understand why this patch is written the way it is, I don't think it is the right approach. This patch changes the mpc5200 gpio binding to adapt to a Linux internal implementation detail. Specifically, the of_mm infrastructure only allows a 1:1 relationship between a 'struct of_gpio_chip' and a 'struct gpio_chip'. When working with device trees, this is the wrong way around. The device tree describes the hardware, not the Linux implementation details. An argument could be made that the current binding isn't ideal and that it would be better to split the simple, interrupt and output-only gpio pins into separate nodes, but that pretty much comes down to a matter of opinion as the existing binding describes the hardware just fine. I'm actually of the opinion that it would be better to fewer gpio nodes, not more, by merging the simple and wakeup pins into a single node, but what's done is done and there is no technical reason for changing the current binding. So, that leaves the problem working with the of_mm infrastructure. I think the correct solution is to modify of_gpio_chip to hold an array of struct gpio_chip and to change of_gpio_simple_xlate() to handle it. Maybe something like: struct of_gpio_chip { int gpio_cells; int (*xlate)(struct of_gpio_chip *of_gc, struct device_node *np, const void *gpio_spec, enum of_gpio_flags *flags); struct gpio_chip gc[1]; }; and add an allocator function that takes care of allocating the required size of the gc array. There are details to work out of course, but I thing this would be the more robust solution. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev