Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 17 Oct 2023 11:32:11 +0900 | From | AKASHI Takahiro <> | Subject | Re: [RFC v2 5/5] dt-bindings: gpio: Add bindings for pinctrl based generic gpio driver |
| |
Hi Linus,
On Thu, Oct 12, 2023 at 09:25:20AM +0200, Linus Walleij wrote: > On Tue, Oct 10, 2023 at 7:25???AM AKASHI Takahiro > <takahiro.akashi@linaro.org> wrote: > > > > We can probably mandate that this has to be inside a pin controller > > > since it is a first. > > > > Yeah, my U-Boot implementation tentatively supports both (inside and > > outside pin controller). But it is not a user's choice, but we should > > decide which way to go. > > OK I have decided we are going to put it inside the pin control node, > as a subnode. (I don't expect anyone to object.)
While I'm still thinking of how I can modify my current implementation to fit into 'inside' syntax, there are a couple of concerns:
1) invoke gpiochip_add_data() at probe function Probably we no longer need "compatible" property, but instead we need to call gpiochip_add_data() explicitly in SCMI pin controller's probe as follows:
scmi_pinctrl_probe() ... devm_pinctrl_register_and_init(dev, ..., pctrldev); pinctrl_enable(pctrldev);
device_for_each_child_node(dev, fwnode) if (fwnode contains "gpio-controller") { /* what pin_control_gpio_probe() does */ gc->get_direction = ...; ... devm_gpiochip_data_add(dev, gc, ...); }
2) gpio-by-pinctrl.c While this file is SCMI-independent now, due to a change at (1), it would be better to move the whole content inside SCMI pin controller driver (because there is no other user for now).
3) Then, pin-control-gpio.yaml may also be put into SCMI binding (i.e. firmware/arm,scmi.yaml). Can we leave the gpio binding outside?
4) phandle in "gpio-ranges" property (As you mentioned) The first element in a tuple of "gpio-ranges" is a phandle to a pin controller node. Now that the gpio node is a sub node of pin controller, the phandle is trivial. But there is no easier way to represent it than using an explicit label: (My U-Boot implementation does this.)
scmi { ... scmi_pinctrl: protocol@19 { ... gpio { gpio-controller; ... gpio-ranges = <&scmi_pinctrl ... >; } } }
I tried: gpio-ranges = <0 ...>; // dtc passed, but '0' might be illegal by spec. gpio-ranges = <(-1) ...>; // dtc passed, but ... gpio-ranges = <&{..} ...>; // dtc error because it's not a full path.
Do you have any other idea? Otherwise, I will modify my RFC with the changes above.
-Takahiro Akashi
> It makes everything easier and clearer for users I think. > > Yours, > Linus Walleij
| ![\](/images/icornerr.gif) |