Messages in this thread |  | | From | Aidan MacDonald <> | Subject | Re: [PATCH v4 12/15] pinctrl: Add AXP192 pin control driver | Date | Sat, 02 Jul 2022 20:22:58 +0100 |
| |
Andy Shevchenko <andy.shevchenko@gmail.com> writes:
> On Fri, Jul 1, 2022 at 5:36 PM Aidan MacDonald > <aidanmacdonald.0x0@gmail.com> wrote: >> Andy Shevchenko <andy.shevchenko@gmail.com> writes: >> > On Wed, Jun 29, 2022 at 4:30 PM Aidan MacDonald >> > <aidanmacdonald.0x0@gmail.com> wrote: > > ... > >> >> +struct axp192_pctl_function { >> >> + const char *name; >> >> + /* Mux value written to the control register to select the function (-1 if unsupported) */ >> >> + const u8 *muxvals; >> >> + const char * const *groups; >> >> + unsigned int ngroups; >> >> +}; >> > >> > Can it be replaced by struct function_desc? >> > https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/pinmux.h#L130 >> >> That'd work, but using the generic infrastructure doesn't allow me to >> simplify anything -- I can eliminate three trivial functions, but the >> generic code is higher overhead (extra allocations, radix trees, ...) > > I really don't see how it gets into extra allocations. Either way you > have a pointer to opaque data or in your current code it's called > muxvals. Other fields seem 1:1 what is in struct function_desc. The > code will be probably the same. > > I.o.w. I'm talking of eliminating data type, and not about simplifying > the code by fully switching to generic infrastructure.
struct function_desc is hidden behind an #ifdef, so I can't use it without enabling the generic pinmux helpers. It doesn't make a lot of sense to enable them if they're not going to be used.
More generally, why would I use a type from an API I'm not using just because it happens to look like a type I defined locally? That would be misleading. Given that the code is the same either way, a local type is preferable because it clearly communicates that I'm not using the generic API, and guarantees that the type isn't referenced elsewhere.
|  |