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

On Thu, Nov 18, 2021 at 4:17 PM Rafał Miłecki [off-list ref] wrote:

On 18.11.2021 14:57, Andy Shevchenko wrote:

quoted

On Thu, Nov 18, 2021 at 3:22 PM Rafał Miłecki [off-list ref] wrote:

...

quoted

quoted

+#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