Morning Andy, On Mon, 2021-05-10 at 19:54 +0300, Andy Shevchenko wrote: > On Mon, May 10, 2021 at 4:41 PM Matti Vaittinen > <matti.vaittinen@xxxxxxxxxxxxxxxxx> wrote: > > Hi Linus, All, > > > > On Thu, 2021-03-25 at 12:32 +0200, Matti Vaittinen wrote: > > > On Thu, 2021-03-25 at 10:35 +0100, Linus Walleij wrote: > > > > snip > > > > > > It could potentially (like the other Rohm GPIO MFD PMIC > > > > drivers) > > > > make some use of the gpio regmap library, but we have some > > > > pending changes for that so look into it after the next merge > > > > window. > > > > > > > > I.e. for your TODO: look at the GPIO_REGMAP helper. > > > > > > I just took a quick peek at gpio_regmap and it looks pretty good > > > to > > > me! > > > > > > Any particular reason why gpio_regmap is not just part of > > > gpio_chip? > > > I > > > guess providing the 'gpio_regmap_direction_*()', > > > 'gpio_regmap_get()', > > > 'gpio_regmap_set()' as exported helpers and leaving calling the > > > (devm_)gpiochip_add_data() to IC driver would have allowed more > > > flexibility. Drivers could then use the gpio_regamap features > > > which > > > fit > > > the IC (by providing pointers to helper functions in gpio_chip) - > > > and > > > handle potential oddball-features by using pointers to some > > > customized > > > functions in gpio_chip. > > > > So, v5.13-rc1 is out. I started wondering the gpio_regamap - and > > same > > question persists. Why hiding the gpio_chip from gpio_regmap users? > > In general to me this sounds like opening a window for > non-controllable changes vs. controllable. Besides that, struct > gpio_chip has more than a few callbacks. On top of that, opening this > wide window means you won't be able to stop or refactoring become a > burden. I would be on the stricter side here. I kind of fail to see your point Andy. Or yes, I know exposing the gpio_chip to user allows much more flexibility. But what are the options? What would a driver developer do when his HW does almost fir the standard regmap_gpio - but not just quite? Say that for example the changing of gpio direction requires some odd additional register access - but other than that the regmap_gpio operations like setting/getting the value, IRQ options etc. fitted the regmap_gpio logic. If he can not override this one function - then he will need to write wholly new GPIO driver without re-using any of the regmap-gpio stuff. You know, if one can't use regmap-gpio, he's likely to use the already exposed gpio_chip anyways. I'd say this is much more of a pain to maintain. Or maybe you add another work-around option in the gpio_regmap_config to indicate this (and every other) oddball HW - which eventually leads to a mess. But this is all just my thinking - I'm kind of a "bystander" here and that's why I asked for opinions. Thanks for sharing yours, Andy. I do appreciate all the help and discussion. > > 3) The last option would be adding pointer to regmap_gpio to > > gpio_chip > > - and exporting the regmap_gpio functions as helpers - leaving the > > gpio > > registration to be done by the IC driver. That would allow IC > > driver to > > use the regmap_gpio helpers which suit the IC and write own > > functions > > for rest of the stuff. I was trying to describe here the approach that has been taken in use at the regulator subsystem - which has used the regmap helpers for quite a while. I think that approach is scaling quite Ok even for strange HW. Best Regards Matti Vaittinen
Attachment:
signature.asc
Description: This is a digitally signed message part
