Messages in this thread | | | Date | Fri, 21 May 2021 12:46:29 +0200 | From | Michael Walle <> | Subject | Re: [PATCH v2 1/3] gpio: regmap: Support few IC specific operations |
| |
Am 2021-05-21 12:25, schrieb Vaittinen, Matti: > On Fri, 2021-05-21 at 12:19 +0200, Michael Walle wrote: >> Am 2021-05-21 12:09, schrieb Andy Shevchenko: >> > On Fri, May 21, 2021 at 12:53 PM Matti Vaittinen >> > <matti.vaittinen@fi.rohmeurope.com> wrote: >> > > Changelog v2: (based on suggestions by Michael Walle) >> > > - drop gpio_regmap_set_drvdata() >> > >> > But why do we have gpio_regmap_get_drvdata() and why is it >> > different >> > now to the new member handling? >> >> Eg. the reg_mask_xlate() callback is just passed a "struct >> gpio_regmap*". >> If someone needs to access private data there, >> gpio_regmap_get_drvdata() >> is used. At least that was its intention. > > I would help the IC driver here too and just directly provide the > drvdata pointer as argument. I don't see much value in providing the > regmap_gpio pointer as IC driver can not dereference it.
What is it with the "it's useless if one cannot dereference it"? You're also passing "struct regmap *" which you cannot dereference. It's an opaque pointer you need to pass to gpio_regmap to call a function there.
What is the problem with letting gpio_regmap derefence its internal data structure and return the value for you?
Adding the drvdata to reg_mask_xlate() highlights my former concern; you need to keep chaning the users to add another parameter. What if xlate() needs the regmap, too? Then you need to change it again. Granted this is a silly example, but you should get my point. It is by far more easy to just add another new gpio_regmap_*(struct gpio_regmap *) function and you don't have to change existing users.
Also what if gpio_regmap provides some useful helper function in the future, it will likely need its internal data struct.
-michael
| |