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 <mrip...@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
>
--
cheers,
-roger
