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