24.05.2017 02:25, Andrey Smirnov wrote: > On Mon, May 22, 2017 at 11:52 PM, Nikita Yushchenko > <nikita.yoush@xxxxxxxxxxxxxxxxxx> wrote: >>> + ret = of_property_read_u32(chip_np, "#gpio-cells", &gpio_cells); >>> + if (ret) >>> + return ret; >>> + >>> + if (WARN_ON(gpio_cells != 2)) >>> + return -ENOTSUPP; >>> + >>> + ret = of_property_read_u32_index(np, "gpios", idx * gpio_cells, >>> + &gpio_num); >>> + if (ret) >>> + return ret; >>> + >>> + ret = of_property_read_u32_index(np, "gpios", idx * gpio_cells + 1, >>> + &gpio_flags); >>> + if (ret) >>> + return ret; >> >> Doesn't this hardcode interpretation of device tree words in gpio >> specification - while this is intended to be gpio-provider specific and >> that's why #gpio-cells exist? >> > > It does and yes that's my understanding of the purpose of #gpio-cells > as well. The reason I did in such a primitive way was because > Barebox's GPIO subsystem doesn't have any translation plumbing to be > able to handle anything more than a simple one dimensional offset. > Given the fact that of_get_named_gpio_flags() make similar assumption > I thought that there are no real consumers of that functionality and > left proper implementation as a future improvement that can be made > once the need arises. Maybe then at least make this [wrong] thing done in single place? I.e. extract relevant code from of_get_named_gpio_flags() into separate routine and call it from two places? (And add a comment there, that it is a stub assuming dump representation) >>> +static int of_gpiochip_scan_gpios(struct gpio_chip *chip) >> >> Not best choice of name for routine that scans hogs? >> >> (although I understand that it comes from linux counterpart) >> > > Eh, I don't have any strong opinion on this one, I am more than happy > to rename it if you think there are better alternatives. of_gpiochip_scan_hogs() ? _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox
