Hi Simon,
2015-08-25 0:12 GMT+09:00 Simon Glass <s...@chromium.org>:
> My original pinctrl patch operating using a peripheral ID enum. This was
> shared between pinmux and clock and provides an easy way to specify a device
> that needs to be controlled, even it is does not (yet) have a driver within
> driver model.
>
> Masahiro's new simple pinctrl gets around this by providing a
> set_state_simple() pinctrl method. By passing a device to that call the
> peripheral ID becomes unnecessary. If the driver needs it, it can calculate
> it itself and use it internally.
>
> However this does not solve the problem for peripheral clocks. The 'pure'
> solution would be to pass a driver to the clock uclass also. But this
> requires that all devices should have a driver, and a struct udevide. Also
> a key optimisation of the clock uclass is allowing a peripheral clock to
> be set even when there is no device for that clock.
>
> There may be a better way to achive the same goal, but for now it seems
> expedient to add in peripheral ID to the pinctrl uclass. Two methods are
> added - one to get the peripheral ID and one to select it. The existing
> set_state_simple() is effectively the union of these.
>
> Signed-off-by: Simon Glass <s...@chromium.org>
> ---
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
> drivers/pinctrl/pinctrl-uclass.c | 33 ++++++++++++++++++++++
> include/dm/pinctrl.h | 61
> ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 94 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-uclass.c
> b/drivers/pinctrl/pinctrl-uclass.c
> index 01b0cf8..1d90fb0 100644
> --- a/drivers/pinctrl/pinctrl-uclass.c
> +++ b/drivers/pinctrl/pinctrl-uclass.c
> @@ -9,6 +9,7 @@
> #include <dm/device.h>
> #include <dm/lists.h>
> #include <dm/pinctrl.h>
> +#include <dm/root.h>
> #include <dm/uclass.h>
> #include <linux/err.h>
> #include <linux/list.h>
> @@ -36,9 +37,41 @@ int pinctrl_select_state(struct udevice *dev, const char
> *ignored)
> return ops->set_state_simple(pctldev, dev);
> }
>
> +int pinctrl_request(struct udevice *dev, int func, int flags)
> +{
> + struct pinctrl_ops *ops = pinctrl_get_ops(dev);
> +
> + if (!ops->request)
> + return -ENOSYS;
> +
> + return ops->request(dev, func, flags);
> +}
> +
> +int pinctrl_request_noflags(struct udevice *dev, int func)
> +{
> + return pinctrl_request(dev, func, 0);
> +}
> +
> +int pinctrl_get_periph_id(struct udevice *dev, struct udevice *periph)
> +{
> + struct pinctrl_ops *ops = pinctrl_get_ops(dev);
> +
> + if (!ops->get_periph_id)
> + return -ENOSYS;
> +
> + return ops->get_periph_id(dev, periph);
> +}
If _get_periph_id() is shared between clk and pinctrl (that what I understood
from your statement), where is it defined? Under arch/arm/mach-<soc>/ ?
> +static int pinctrl_post_bind(struct udevice *dev)
> +{
> + /* Scan the bus for devices */
> + return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false);
> +}
I do not think this work.
dm_scan_fdt_node() requires child nodes to be compatible to something
for binding.
Pin configuration nodes do not have '.compatible' properties.
> UCLASS_DRIVER(pinctrl) = {
> .id = UCLASS_PINCTRL,
> .name = "pinctrl",
> + .post_bind = pinctrl_post_bind,
> };
>
> #else
> diff --git a/include/dm/pinctrl.h b/include/dm/pinctrl.h
> index b2ba0b4..2f9bf67 100644
> --- a/include/dm/pinctrl.h
> +++ b/include/dm/pinctrl.h
> @@ -85,10 +85,37 @@ struct pinctrl_ops {
> int (*pinconf_group_set)(struct udevice *dev, unsigned group_selector,
> unsigned param, unsigned argument);
> int (*set_state)(struct udevice *dev, struct udevice *config);
> +
> /* for pinctrl-simple */
> int (*set_state_simple)(struct udevice *dev, struct udevice *periph);
> + /**
> + * request() - Request a particular pinctrl function
> + *
> + * This activates the selected function.
> + *
> + * @dev: Device to adjust (UCLASS_PINCTRL)
> + * @func: Function number (driver-specific)
> + * @return 0 if OK, -ve on error
> + */
> + int (*request)(struct udevice *dev, int func, int flags);
> +
> + /**
> + * get_periph_id() - get the peripheral ID for a device
> + *
> + * This generally looks at the peripheral's device tree node to work
> + * out the peripheral ID. The return value is normally interpreted as
> + * enum periph_id. so long as this is defined by the platform (which it
> + * should be).
> + *
> + * @dev: Pinctrl device to use for decoding
> + * @periph: Device to check
> + * @return peripheral ID of @periph, or -ENOENT on error
> + */
> + int (*get_periph_id)(struct udevice *dev, struct udevice *periph);
> };
>
> +#define pinctrl_get_ops(dev) ((struct pinctrl_ops *)(dev)->driver->ops)
> +
> /**
> * Generic pin configuration paramters
> *
> @@ -217,4 +244,38 @@ static inline int pinctrl_generic_set_state(struct
> udevice *pctldev,
> }
> #endif
>
> +/**
> + * pinctrl_request() - Request a particular pinctrl function
> + *
> + * @dev: Device to check (UCLASS_PINCTRL)
> + * @func: Function number (driver-specific)
> + * @flags: Flags (driver-specific)
> + * @return 0 if OK, -ve on error
> + */
> +int pinctrl_request(struct udevice *dev, int func, int flags);
> +
> +/**
> + * pinctrl_request_noflags() - Request a particular pinctrl function
> + *
> + * This is similar to pinctrl_request() but uses 0 for @flags.
> + *
> + * @dev: Device to check (UCLASS_PINCTRL)
> + * @func: Function number (driver-specific)
> + * @return 0 if OK, -ve on error
> + */
> +int pinctrl_request_noflags(struct udevice *dev, int func);
> +
> +/**
> + * pinctrl_get_periph_id() - get the peripheral ID for a device
> + *
> + * This generally looks at the peripheral's device tree node to work out the
> + * peripheral ID. The return value is normally interpreted as enum periph_id.
> + * so long as this is defined by the platform (which it should be).
> + *
> + * @dev: Pinctrl device to use for decoding
> + * @periph: Device to check
> + * @return peripheral ID of @periph, or -ENOENT on error
> + */
> +int pinctrl_get_periph_id(struct udevice *dev, struct udevice *periph);
> +
> #endif /* __PINCTRL_H */
I am guessing these functions are only invoked from .set_state_simple(),
If so, should they be exported to struct pinctrl_ops?
I expect callbacks are invoked from the core framework (uclass).
--
Best Regards
Masahiro Yamada
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
