Re: [PATCH 3/5] pinctrl: add helpers reading pins, groups & functions from DT

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Thu, Nov 18, 2021 at 4:17 PM Rafał Miłecki <zajec5@xxxxxxxxx> wrote:
> On 18.11.2021 14:57, Andy Shevchenko wrote:
> > On Thu, Nov 18, 2021 at 3:22 PM Rafał Miłecki <zajec5@xxxxxxxxx> wrote:

...

> >> +#include <linux/of.h>
> >
> > I don't like this. This shows not thought through the design of the series.
> >
> > What I rather expect is a proper interfacing layer that you fill with
> > options that can be provided by corresponding underlying
> > implementation, e.g. DT.
> >
> > Moreover, before doing this you probably would need to refactor the
> > pin control core to get rid of DT specifics, i.e. abstract them away
> > first.
>
> Ouch, it seems like pinctrl got into a tricky state. As I understand it
> we need some abstraction layer between DT and pinctrl but noone is
> working on it?

Seems so to me.
To be more specific, I'm talking about these:

  struct pinctrl_ops {
    ...
    int (*dt_node_to_map)...
    void (*dt_free_map)...
  }

and this:

  struct pinctrl {
    ...
    struct list_head dt_maps;
 }

In the latter case it is almost related to renaming dt_maps to
something like fw_maps.
In both cases it is about decoupling DT stuff out from the pin control core.

So, with something like pinctrl_fw_ops to be introduced, the DT stuff
moved to drivers/pinctrl/devicetree.c.

> Does it mean we should consider pinctrl core frozen until
> it's refactored?

Solutions which are related to pin control core without keeping non-DT
providers in mind will be NAKed by me, definitely. Of course it can be
overridden by maintainers.

> It's quite inconvenient for me as I'm not sure if I can handle such
> heavy pinctrl refactoring while at the same time I'd like to add
> those small features to it.

Which effectively means "let give somebody else even more burden and problems".

> Can you point to an example of extra interfacing layer that could be
> used as a reference for what you expect for pinctrl one, please? Some
> solution in another Linux's subsystem?

GPIOLIB decoupled this.
Another example (not sure if it's good enough here) is the fwnode API
(see fwnode ops).

-- 
With Best Regards,
Andy Shevchenko