On Thu, Mar 4, 2021 at 5:44 PM Álvaro Fernández Rojas [off-list ref] wrote:

quoted

El 4 mar 2021, a las 16:28, Andy Shevchenko [off-list ref] escribió:
On Thu, Mar 4, 2021 at 5:24 PM Álvaro Fernández Rojas [off-list ref] wrote:

quoted

quoted

El 4 mar 2021, a las 16:17, Andy Shevchenko [off-list ref] escribió:
On Thu, Mar 4, 2021 at 5:06 PM Álvaro Fernández Rojas [off-list ref] wrote:

quoted

quoted

El 4 mar 2021, a las 11:35, Andy Shevchenko [off-list ref] escribió:
On Thu, Mar 4, 2021 at 10:57 AM Álvaro Fernández Rojas
[off-list ref] wrote:

quoted

quoted

quoted

+ * @of_node:           (Optional) The device node

quoted

+       struct device_node *of_node;

Can we use fwnode from day 1, please?

Could you explain this? I haven’t dealt with fwnode never :$
BTW, this is done to fix this check when parsing gpio ranges:
https://github.com/torvalds/linux/blob/f69d02e37a85645aa90d18cacfff36dba370f797/drivers/gpio/gpiolib-of.c#L933-L934

Use struct fwnode_handle pointer instead of OF-specific one.

But is that compatible with the current gpiolib-of code? :$

Yes (after a bit of amendment I have sent today as v2:
https://lore.kernel.org/linux-gpio/20210304150215.80652-1-andriy.shevchenko@linux.intel.com/T/#u (local)).

Well that doesn’t fulfill my definition of “current gpiolib-of code”…
@Linus what should I do about this?

Well, fwnode is a generic, and I strongly against spreading
OF-specific code when we have fwnode working. But let's hear Linus
out, of course!

But it seems you are right and the library needs a few more amendments.

quoted

quoted

quoted

Also here is the question, why do you need to have that field in the
regmap config structure and can't simply use the parent's fwnode?
Also I'm puzzled why it's not working w/o this patch: GPIO library
effectively assigns parent's fwnode (okay, of_node right now).

Because gpio regmap a child node of the pin controller, which is the one probed (gpio regmap is probed from the pin controller).
Therefore the parent’s fwnode is useless, since the correct gpio_chip node is the child's one (we have pin-ranges declared in the child node, referencing the parent pinctrl node).

I see. Can you point me out to the code where we get the node and
where it's being retrieved / filled?

Sure, this is where the child node is searched: https://github.com/Noltari/linux/blob/6d1ebb8ff26ed54592eef1fcd3b58834acb48c04/drivers/pinctrl/bcm/pinctrl-bcm63xx.c#L100-L109
Then the gpio child node is probed and assigned here: https://github.com/Noltari/linux/blob/6d1ebb8ff26ed54592eef1fcd3b58834acb48c04/drivers/pinctrl/bcm/pinctrl-bcm63xx.c#L51

So, this is not (*yet) in upstream, correct?

So, why not to switch to fwnode API in that driver as well?

When you do that and supply fwnode thru the regmap configuration, in
the gpio-regmap we may assign it to of_node (via to_of_node() API).

Basically, I based that part of the code on the ingenic pin controller: https://github.com/torvalds/linux/blob/f69d02e37a85645aa90d18cacfff36dba370f797/drivers/pinctrl/pinctrl-ingenic.c#L2485-L2491
https://github.com/torvalds/linux/blob/f69d02e37a85645aa90d18cacfff36dba370f797/Documentation/devicetree/bindings/pinctrl/ingenic%2Cpinctrl.yaml#L155-L176

This doesn't use remgap GPIO.

-- 
With Best Regards,
Andy Shevchenko