| Message ID | 20230720-ti-mdio-pinmux-v1-1-0bd3bd1cf759@kernel.org |
|---|---|
| State | Superseded |
| Delegated to: | Ramon Fried |
| Headers | show |
| Series | net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl | expand |
On 20/07/2023 12:55, Maxime Ripard wrote: > Some drivers might not follow entirely the driver/device association, > and thus might support what should be multiple devices in a single > driver. > > Such a driver is am65-cpsw-nuss, where the MAC and MDIO controllers are > different device tree nodes, with their own resources (including pinctrl > pins) but supported by a single driver tied to the MAC device in U-Boot. > > In order to get the proper pinctrl state, we would need to get the > state from the MDIO device tree node, but tie it to the MAC device since > it's the only thing we have access to. > > Signed-off-by: Maxime Ripard <mripard@kernel.org> > --- > drivers/pinctrl/pinctrl-uclass.c | 15 +++++++++------ > include/dm/pinctrl.h | 26 ++++++++++++++++++++------ > 2 files changed, 29 insertions(+), 12 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c > index 73dd7b1038bb..9e28f1858cbd 100644 > --- a/drivers/pinctrl/pinctrl-uclass.c > +++ b/drivers/pinctrl/pinctrl-uclass.c > @@ -53,7 +53,8 @@ static int pinctrl_config_one(struct udevice *config) > * @statename: state name, like "default" > * @return: 0 on success, or negative error code on failure > */ > -static int pinctrl_select_state_full(struct udevice *dev, const char *statename) > +static int pinctrl_select_state_full(struct udevice *dev, ofnode node, > + const char *statename) > { > char propname[32]; /* long enough */ > const fdt32_t *list; > @@ -61,7 +62,7 @@ static int pinctrl_select_state_full(struct udevice *dev, const char *statename) > struct udevice *config; > int state, size, i, ret; > > - state = dev_read_stringlist_search(dev, "pinctrl-names", statename); > + state = ofnode_stringlist_search(node, "pinctrl-names", statename); > if (state < 0) { > char *end; > /* > @@ -74,7 +75,7 @@ static int pinctrl_select_state_full(struct udevice *dev, const char *statename) > } > > snprintf(propname, sizeof(propname), "pinctrl-%d", state); > - list = dev_read_prop(dev, propname, &size); > + list = ofnode_get_property(node, propname, &size); > if (!list) > return -ENOSYS; > > @@ -293,20 +294,22 @@ static int pinctrl_select_state_simple(struct udevice *dev) > return ops->set_state_simple(pctldev, dev); > } > > -int pinctrl_select_state(struct udevice *dev, const char *statename) > +int pinctrl_select_state_by_ofnode(struct udevice *dev, ofnode node, > + const char *statename) > { > /* > * Some device which is logical like mmc.blk, do not have > * a valid ofnode. > */ > - if (!dev_has_ofnode(dev)) > + if (!ofnode_valid(node)) > return 0; > + > /* > * Try full-implemented pinctrl first. > * If it fails or is not implemented, try simple one. > */ > if (CONFIG_IS_ENABLED(PINCTRL_FULL)) > - return pinctrl_select_state_full(dev, statename); > + return pinctrl_select_state_full(dev, node, statename); > > return pinctrl_select_state_simple(dev); > } > diff --git a/include/dm/pinctrl.h b/include/dm/pinctrl.h > index e3e50afeaff0..be4679b7f20d 100644 > --- a/include/dm/pinctrl.h > +++ b/include/dm/pinctrl.h > @@ -502,6 +502,24 @@ static inline int pinctrl_generic_set_state(struct udevice *pctldev, > #endif > > #if CONFIG_IS_ENABLED(PINCTRL) > +/** > + * pinctrl_select_state_by_ofnode() - Set a device to a given state using the given ofnode > + * @dev: Peripheral device > + * @ofnode: Device Tree node to get the state from > + * @statename: State name, like "default" > + * > + * Return: 0 on success, or negative error code on failure > + */ > +int pinctrl_select_state_by_ofnode(struct udevice *dev, ofnode node, const char *statename); > +#else > +static inline int pinctrl_select_state_by_ofnode(struct udevice *dev, > + ofnode node, > + const char *statename) > +{ > + return -ENOSYS; > +} This allows & encourages one device driver to change another devices pinctrl state which doesn't look like a good idea to me. Please see if an alternative solution pointed out in patch2 works so we don't have to need this patch. > +#endif > + > /** > * pinctrl_select_state() - Set a device to a given state > * @dev: Peripheral device > @@ -509,14 +527,10 @@ static inline int pinctrl_generic_set_state(struct udevice *pctldev, > * > * Return: 0 on success, or negative error code on failure > */ > -int pinctrl_select_state(struct udevice *dev, const char *statename); > -#else > -static inline int pinctrl_select_state(struct udevice *dev, > - const char *statename) > +static inline int pinctrl_select_state(struct udevice *dev, const char *statename) > { > - return -ENOSYS; > + return pinctrl_select_state_by_ofnode(dev, dev_ofnode(dev), statename); > } > -#endif > > /** > * pinctrl_request() - Request a particular pinctrl function >
diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c index 73dd7b1038bb..9e28f1858cbd 100644 --- a/drivers/pinctrl/pinctrl-uclass.c +++ b/drivers/pinctrl/pinctrl-uclass.c @@ -53,7 +53,8 @@ static int pinctrl_config_one(struct udevice *config) * @statename: state name, like "default" * @return: 0 on success, or negative error code on failure */ -static int pinctrl_select_state_full(struct udevice *dev, const char *statename) +static int pinctrl_select_state_full(struct udevice *dev, ofnode node, + const char *statename) { char propname[32]; /* long enough */ const fdt32_t *list; @@ -61,7 +62,7 @@ static int pinctrl_select_state_full(struct udevice *dev, const char *statename) struct udevice *config; int state, size, i, ret; - state = dev_read_stringlist_search(dev, "pinctrl-names", statename); + state = ofnode_stringlist_search(node, "pinctrl-names", statename); if (state < 0) { char *end; /* @@ -74,7 +75,7 @@ static int pinctrl_select_state_full(struct udevice *dev, const char *statename) } snprintf(propname, sizeof(propname), "pinctrl-%d", state); - list = dev_read_prop(dev, propname, &size); + list = ofnode_get_property(node, propname, &size); if (!list) return -ENOSYS; @@ -293,20 +294,22 @@ static int pinctrl_select_state_simple(struct udevice *dev) return ops->set_state_simple(pctldev, dev); } -int pinctrl_select_state(struct udevice *dev, const char *statename) +int pinctrl_select_state_by_ofnode(struct udevice *dev, ofnode node, + const char *statename) { /* * Some device which is logical like mmc.blk, do not have * a valid ofnode. */ - if (!dev_has_ofnode(dev)) + if (!ofnode_valid(node)) return 0; + /* * Try full-implemented pinctrl first. * If it fails or is not implemented, try simple one. */ if (CONFIG_IS_ENABLED(PINCTRL_FULL)) - return pinctrl_select_state_full(dev, statename); + return pinctrl_select_state_full(dev, node, statename); return pinctrl_select_state_simple(dev); } diff --git a/include/dm/pinctrl.h b/include/dm/pinctrl.h index e3e50afeaff0..be4679b7f20d 100644 --- a/include/dm/pinctrl.h +++ b/include/dm/pinctrl.h @@ -502,6 +502,24 @@ static inline int pinctrl_generic_set_state(struct udevice *pctldev, #endif #if CONFIG_IS_ENABLED(PINCTRL) +/** + * pinctrl_select_state_by_ofnode() - Set a device to a given state using the given ofnode + * @dev: Peripheral device + * @ofnode: Device Tree node to get the state from + * @statename: State name, like "default" + * + * Return: 0 on success, or negative error code on failure + */ +int pinctrl_select_state_by_ofnode(struct udevice *dev, ofnode node, const char *statename); +#else +static inline int pinctrl_select_state_by_ofnode(struct udevice *dev, + ofnode node, + const char *statename) +{ + return -ENOSYS; +} +#endif + /** * pinctrl_select_state() - Set a device to a given state * @dev: Peripheral device @@ -509,14 +527,10 @@ static inline int pinctrl_generic_set_state(struct udevice *pctldev, * * Return: 0 on success, or negative error code on failure */ -int pinctrl_select_state(struct udevice *dev, const char *statename); -#else -static inline int pinctrl_select_state(struct udevice *dev, - const char *statename) +static inline int pinctrl_select_state(struct udevice *dev, const char *statename) { - return -ENOSYS; + return pinctrl_select_state_by_ofnode(dev, dev_ofnode(dev), statename); } -#endif /** * pinctrl_request() - Request a particular pinctrl function
Some drivers might not follow entirely the driver/device association, and thus might support what should be multiple devices in a single driver. Such a driver is am65-cpsw-nuss, where the MAC and MDIO controllers are different device tree nodes, with their own resources (including pinctrl pins) but supported by a single driver tied to the MAC device in U-Boot. In order to get the proper pinctrl state, we would need to get the state from the MDIO device tree node, but tie it to the MAC device since it's the only thing we have access to. Signed-off-by: Maxime Ripard <mripard@kernel.org> --- drivers/pinctrl/pinctrl-uclass.c | 15 +++++++++------ include/dm/pinctrl.h | 26 ++++++++++++++++++++------ 2 files changed, 29 insertions(+), 12 deletions(-)