dm: Introduce Rockchip RK3288 support

There are a few minor tweaks needed to get this to build. Add them so that the simple pinctrl implementation can be used.

Signed-off-by Simon Glass <[email protected]>

---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/pinctrl/pinctrl-uclass.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/pinctrl/pinctrl-uclass.c +4 -5

index 853d779..01b0cf8 100644
@@ -6,21 +6,21 @@
 
 #include <common.h>
 #include <libfdt.h>
-#include <linux/err.h>
-#include <linux/list.h>
 #include <dm/device.h>
 #include <dm/lists.h>
 #include <dm/pinctrl.h>
 #include <dm/uclass.h>
+#include <linux/err.h>
+#include <linux/list.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
 #if CONFIG_IS_ENABLED(PINCTRL_SIMPLE)
 
-static int pinctrl_select_state(struct udevice *dev, const char *ignored)
+int pinctrl_select_state(struct udevice *dev, const char *ignored)
 {
        struct udevice *pctldev;
-       struct pinconf_ops *ops;
+       const struct pinctrl_ops *ops;
        int ret;
 
        ret = uclass_get_device(UCLASS_PINCTRL, 0, &pctldev);
--

2.5.0.457.gab17608

+Masahiro

Quoted 42 lines · Simon Glass

There are a few minor tweaks needed to get this to build. Add them so that the simple pinctrl implementation can be used.

Signed-off-by Simon Glass <[email protected]>

---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/pinctrl/pinctrl-uclass.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/pinctrl/pinctrl-uclass.c +4 -4

index 853d779..01b0cf8 100644
@@ -6,21 +6,21 @@
 
 #include <common.h>
 #include <libfdt.h>
-#include <linux/err.h>
-#include <linux/list.h>
 #include <dm/device.h>
 #include <dm/lists.h>
 #include <dm/pinctrl.h>
 #include <dm/uclass.h>
+#include <linux/err.h>
+#include <linux/list.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
 #if CONFIG_IS_ENABLED(PINCTRL_SIMPLE)
 
-static int pinctrl_select_state(struct udevice *dev, const char *ignored)
+int pinctrl_select_state(struct udevice *dev, const char *ignored)
 {
        struct udevice *pctldev;
-       struct pinconf_ops *ops;
+       const struct pinctrl_ops *ops;
        int ret;
 

drivers/pinctrl/pinctrl-uclass.c line 26

Quoted 1 line

drivers/pinctrl/pinctrl-uclass.c +0 -0

        ret = uclass_get_device(UCLASS_PINCTRL, 0, &pctldev);

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 <[email protected]>

---

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(+)

drivers/pinctrl/pinctrl-uclass.c +15 -0

index 01b0cf8..1d90fb0 100644
@@ -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);
+}

drivers/pinctrl/pinctrl-uclass.c +1 -0

 UCLASS_DRIVER(pinctrl) = {
        .id = UCLASS_PINCTRL,
        .name = "pinctrl",
+       .post_bind      = pinctrl_post_bind,
 };
 
 #else

include/dm/pinctrl.h +52 -0

index b2ba0b4..2f9bf67 100644
@@ -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

Hi Simon,

2015-08-25 0:12 GMT+09:00 Simon Glass <[email protected]>:

Quoted 68 lines

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 <[email protected]>

---

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(+)

drivers/pinctrl/pinctrl-uclass.c +21 -0

index 01b0cf8..1d90fb0 100644
@@ -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)

drivers/pinctrl/pinctrl-uclass.c int pinctrl_select_state(struct udevice *dev, const char *ignored) · line 63

Quoted 4 lines

drivers/pinctrl/pinctrl-uclass.c +4 -0

+               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>/ ?

drivers/pinctrl/pinctrl-uclass.c int pinctrl_select_state(struct udevice *dev, const char *ignored) · line 69

Quoted 9 lines

drivers/pinctrl/pinctrl-uclass.c +5 -0

+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.

Quoted 83 lines

drivers/pinctrl/pinctrl-uclass.c +1 -0

 UCLASS_DRIVER(pinctrl) = {
        .id = UCLASS_PINCTRL,
        .name = "pinctrl",
+       .post_bind      = pinctrl_post_bind,
 };
 
 #else

include/dm/pinctrl.h +52 -0

index b2ba0b4..2f9bf67 100644
@@ -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

include/dm/pinctrl.h static inline int pinctrl_generic_set_state(struct udevice *pctldev, · line 281

Quoted 10 lines

include/dm/pinctrl.h +9 -0

+ * 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).

Hi Masahiro,

Quoted 73 lines · Masahiro Yamada

Hi Simon,

2015-08-25 0:12 GMT+09:00 Simon Glass <[email protected]>:

Quoted 68 lines

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 <[email protected]>

---

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(+)

drivers/pinctrl/pinctrl-uclass.c +21 -0

index 01b0cf8..1d90fb0 100644
@@ -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)

Quoted 8 lines

drivers/pinctrl/pinctrl-uclass.c int pinctrl_select_state(struct udevice *dev, const char *ignored) · line 63

Quoted 4 lines

drivers/pinctrl/pinctrl-uclass.c +4 -0

+               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>/ ?

Yes.

Quoted 13 lines

drivers/pinctrl/pinctrl-uclass.c int pinctrl_select_state(struct udevice *dev, const char *ignored) · line 69

Quoted 9 lines

drivers/pinctrl/pinctrl-uclass.c +5 -0

+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.

Quoted 4 lines

dm_scan_fdt_node() requires child nodes to be compatible to something for binding.

Pin configuration nodes do not have '.compatible' properties.

True in many cases, in which case it does not harm, but Rockchip has them:

http://git.denx.de/?p=u-boot-dm.git;a=blob;f=arch/arm/dts/rk3288.dtsi;h=0f497099679470ea39078d6ca9e5b1ae550995b0;hb=refs/heads/rockchip-working

Quoted 98 lines

Quoted 82 lines

drivers/pinctrl/pinctrl-uclass.c +1 -0

 UCLASS_DRIVER(pinctrl) = {
        .id = UCLASS_PINCTRL,
        .name = "pinctrl",
+       .post_bind      = pinctrl_post_bind,
 };
 
 #else

include/dm/pinctrl.h +52 -0

index b2ba0b4..2f9bf67 100644
@@ -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

include/dm/pinctrl.h static inline int pinctrl_generic_set_state(struct udevice *pctldev, · line 281

Quoted 10 lines

include/dm/pinctrl.h +9 -0

+ * 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?

Quoted 2 lines

I expect callbacks are invoked from the core framework (uclass).

They can be involved from other places. In particular you need to call get_periph_id() to get the peripheral ID for use with clocks.

2015-08-26 13:38 GMT+09:00 Simon Glass <[email protected]>:

Quoted 101 lines

Hi Masahiro,

Quoted 73 lines · Masahiro Yamada

Hi Simon,

2015-08-25 0:12 GMT+09:00 Simon Glass <[email protected]>:

Quoted 68 lines

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 <[email protected]>

---

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(+)

drivers/pinctrl/pinctrl-uclass.c +21 -0

index 01b0cf8..1d90fb0 100644
@@ -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)

Quoted 8 lines

drivers/pinctrl/pinctrl-uclass.c int pinctrl_select_state(struct udevice *dev, const char *ignored) · line 63

Quoted 4 lines

drivers/pinctrl/pinctrl-uclass.c +4 -0

+               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>/ ?

Yes.

Quoted 13 lines

drivers/pinctrl/pinctrl-uclass.c int pinctrl_select_state(struct udevice *dev, const char *ignored) · line 69

Quoted 9 lines

drivers/pinctrl/pinctrl-uclass.c +5 -0

+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.

Quoted 8 lines

Quoted 4 lines

dm_scan_fdt_node() requires child nodes to be compatible to something for binding.

Pin configuration nodes do not have '.compatible' properties.

True in many cases, in which case it does not harm, but Rockchip has them:

http://git.denx.de/?p=u-boot-dm.git;a=blob;f=arch/arm/dts/rk3288.dtsi;h=0f497099679470ea39078d6ca9e5b1ae550995b0;hb=refs/heads/rockchip-working

My bad.

It is possible a pinctrl device can have other devices under it (in this case, GPIO).

So, it turned out my patch should be fixed. device_bind_driver_to_node() should be called for devices that have no .compatible.

Quoted 102 lines

Quoted 102 lines

Quoted 86 lines

drivers/pinctrl/pinctrl-uclass.c +1 -0

 UCLASS_DRIVER(pinctrl) = {
        .id = UCLASS_PINCTRL,
        .name = "pinctrl",
+       .post_bind      = pinctrl_post_bind,
 };
 
 #else

include/dm/pinctrl.h +52 -0

index b2ba0b4..2f9bf67 100644
@@ -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

include/dm/pinctrl.h static inline int pinctrl_generic_set_state(struct udevice *pctldev, · line 281

Quoted 10 lines

include/dm/pinctrl.h +9 -0

+ * 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?

Quoted 6 lines

Quoted 2 lines

I expect callbacks are invoked from the core framework (uclass).

They can be involved from other places. In particular you need to call get_periph_id() to get the peripheral ID for use with clocks.

In the first place, pinctrl_get_periph_id() should not be defined in a pinctrl driver.

The first argument, pinctrl device, is not needed to calculate the peripheral ID,
and the actual definition is placed under mach-<soc>, right?

Hi Masahiro,

Quoted 117 lines · Masahiro Yamada

2015-08-26 13:38 GMT+09:00 Simon Glass <[email protected]>:

Quoted 101 lines

Hi Masahiro,

Quoted 73 lines · Masahiro Yamada

Hi Simon,

2015-08-25 0:12 GMT+09:00 Simon Glass <[email protected]>:

Quoted 68 lines

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 <[email protected]>

---

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(+)

drivers/pinctrl/pinctrl-uclass.c +21 -0

index 01b0cf8..1d90fb0 100644
@@ -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)

Quoted 8 lines

drivers/pinctrl/pinctrl-uclass.c int pinctrl_select_state(struct udevice *dev, const char *ignored) · line 63

Quoted 4 lines

drivers/pinctrl/pinctrl-uclass.c +4 -0

+               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>/ ?

Yes.

Quoted 13 lines

drivers/pinctrl/pinctrl-uclass.c int pinctrl_select_state(struct udevice *dev, const char *ignored) · line 69

Quoted 9 lines

drivers/pinctrl/pinctrl-uclass.c +5 -0

+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.

Quoted 8 lines

Quoted 4 lines

dm_scan_fdt_node() requires child nodes to be compatible to something for binding.

Pin configuration nodes do not have '.compatible' properties.

True in many cases, in which case it does not harm, but Rockchip has them:

http://git.denx.de/?p=u-boot-dm.git;a=blob;f=arch/arm/dts/rk3288.dtsi;h=0f497099679470ea39078d6ca9e5b1ae550995b0;hb=refs/heads/rockchip-working

My bad.

It is possible a pinctrl device can have other devices under it (in this case, GPIO).

Quoted 4 lines

So, it turned out my patch should be fixed. device_bind_driver_to_node() should be called for devices that have no .compatible.

OK.

Quoted 112 lines

Quoted 101 lines

Quoted 101 lines

Quoted 85 lines

drivers/pinctrl/pinctrl-uclass.c +1 -0

 UCLASS_DRIVER(pinctrl) = {
        .id = UCLASS_PINCTRL,
        .name = "pinctrl",
+       .post_bind      = pinctrl_post_bind,
 };
 
 #else

include/dm/pinctrl.h +52 -0

index b2ba0b4..2f9bf67 100644
@@ -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

include/dm/pinctrl.h static inline int pinctrl_generic_set_state(struct udevice *pctldev, · line 281

Quoted 10 lines

include/dm/pinctrl.h +9 -0

+ * 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?

Quoted 6 lines

Quoted 2 lines

I expect callbacks are invoked from the core framework (uclass).

They can be involved from other places. In particular you need to call get_periph_id() to get the peripheral ID for use with clocks.

In the first place, pinctrl_get_periph_id() should not be defined in a pinctrl driver.

Quoted 3 lines

The first argument, pinctrl device, is not needed to calculate the peripheral ID,
and the actual definition is placed under mach-<soc>, right?

So are you suggesting that each SoC provides a function (not in the driver model framework) to get the peripheral ID in the simple pinctrl case? This might be similar to clock_decode_periph_id(const void *blob, int node) in tegra.

In fact if we implement a full clock controller binding (similar to how you have implemented pinctrl) then we can avoid the need for peripheral IDs once driver model is fully up. The need then exists only in SPL, due to code size/memory constraints. It may perhaps also be needed before relocation in U-Boot proper due to execution time constraints but we can limit the number of devices brought up using u-boot,dm-pre-reloc.

It would be good to limit the spread of the peripheral ID and only use it when needed.

I'll take a look at creating a function outside of pinctrl which returns a peripheral ID. That deals with the get_periph_id() method in pinctrl.

However this still leaves the request() method - which takes a peripheral ID. This is needed I think because:the device being set up might not have a udevice in SPL. What do you think?

Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

2015-08-26 22:20 GMT+09:00 Simon Glass <[email protected]>:

Quoted 126 lines

Hi Masahiro,

Quoted 121 lines · Masahiro Yamada

2015-08-26 13:38 GMT+09:00 Simon Glass <[email protected]>:

Quoted 101 lines

Hi Masahiro,

Quoted 73 lines · Masahiro Yamada

Hi Simon,

2015-08-25 0:12 GMT+09:00 Simon Glass <[email protected]>:

Quoted 68 lines

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 <[email protected]>

---

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(+)

drivers/pinctrl/pinctrl-uclass.c +21 -0

index 01b0cf8..1d90fb0 100644
@@ -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)

Quoted 8 lines

drivers/pinctrl/pinctrl-uclass.c int pinctrl_select_state(struct udevice *dev, const char *ignored) · line 63

Quoted 4 lines

drivers/pinctrl/pinctrl-uclass.c +4 -0

+               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>/ ?

Yes.

Quoted 13 lines

drivers/pinctrl/pinctrl-uclass.c int pinctrl_select_state(struct udevice *dev, const char *ignored) · line 69

Quoted 9 lines

drivers/pinctrl/pinctrl-uclass.c +5 -0

+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.

Quoted 8 lines

Quoted 4 lines

dm_scan_fdt_node() requires child nodes to be compatible to something for binding.

Pin configuration nodes do not have '.compatible' properties.

True in many cases, in which case it does not harm, but Rockchip has them:

http://git.denx.de/?p=u-boot-dm.git;a=blob;f=arch/arm/dts/rk3288.dtsi;h=0f497099679470ea39078d6ca9e5b1ae550995b0;hb=refs/heads/rockchip-working

My bad.

It is possible a pinctrl device can have other devices under it (in this case, GPIO).

So, it turned out my patch should be fixed. device_bind_driver_to_node() should be called for devices that have no .compatible.

Quoted 1 line

OK.

So, dm_scan_fdt_node() should be moved to rk3288_pinctrl_bind(), I think.

Having GPIO nodes under the pinctrl node is a rockchip-specific matter.

Quoted 105 lines

Quoted 105 lines

Quoted 94 lines

Quoted 94 lines

Quoted 78 lines

drivers/pinctrl/pinctrl-uclass.c +1 -0

 UCLASS_DRIVER(pinctrl) = {
        .id = UCLASS_PINCTRL,
        .name = "pinctrl",
+       .post_bind      = pinctrl_post_bind,
 };
 
 #else

include/dm/pinctrl.h +52 -0

index b2ba0b4..2f9bf67 100644
@@ -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

include/dm/pinctrl.h static inline int pinctrl_generic_set_state(struct udevice *pctldev, · line 281

Quoted 10 lines

include/dm/pinctrl.h +9 -0

+ * 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?

Quoted 6 lines

Quoted 2 lines

I expect callbacks are invoked from the core framework (uclass).

They can be involved from other places. In particular you need to call get_periph_id() to get the peripheral ID for use with clocks.

In the first place, pinctrl_get_periph_id() should not be defined in a pinctrl driver.

Quoted 8 lines

Quoted 3 lines

The first argument, pinctrl device, is not needed to calculate the peripheral ID,
and the actual definition is placed under mach-<soc>, right?

So are you suggesting that each SoC provides a function (not in the driver model framework) to get the peripheral ID in the simple pinctrl case? This might be similar to clock_decode_periph_id(const void *blob, int node) in tegra.

Only for those SoCs that use pinctrl-simple and clk-simple.

Given that the peripheral ID is driver-wide concept that is shared between pinctrl-simple and clk-simple,
the ID decoder should be placed in common location, so that pinctrl-simple should not depends on clk-simple, and vice versa.

Quoted 22 lines

In fact if we implement a full clock controller binding (similar to how you have implemented pinctrl) then we can avoid the need for peripheral IDs once driver model is fully up. The need then exists only in SPL, due to code size/memory constraints. It may perhaps also be needed before relocation in U-Boot proper due to execution time constraints but we can limit the number of devices brought up using u-boot,dm-pre-reloc.

It would be good to limit the spread of the peripheral ID and only use it when needed.

I'll take a look at creating a function outside of pinctrl which returns a peripheral ID. That deals with the get_periph_id() method in pinctrl.

However this still leaves the request() method - which takes a peripheral ID. This is needed I think because:the device being set up might not have a udevice in SPL. What do you think?

Quoted 6 lines

Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

--
Best Regards
Masahiro Yamada

Some SoCs want to adjust the input clock to the DWMMC block as a way of controlling the MMC bus clock. Update the get_mmc_clk() method to support this.

Signed-off-by Simon Glass <[email protected]>

---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/mmc/dw_mmc.c        |  2 +-
 drivers/mmc/exynos_dw_mmc.c |  2 +-
 include/dwmmc.h             | 16 +++++++++++++++-
 3 files changed, 17 insertions(+), 3 deletions(-)

drivers/mmc/dw_mmc.c +1 -1

index 77b87e0..1117fed 100644
@@ -266,7 +266,7 @@ static int dwmci_setup_bus(struct dwmci_host *host, u32 freq)
 	 * host->bus_hz should be set by user.
 	 */
 	if (host->get_mmc_clk)
-		sclk = host->get_mmc_clk(host);
+		sclk = host->get_mmc_clk(host, freq);
 	else if (host->bus_hz)
 		sclk = host->bus_hz;
 	else {

drivers/mmc/exynos_dw_mmc.c +1 -1

index cde2ba7..863bbb3 100644
@@ -39,7 +39,7 @@ static void exynos_dwmci_clksel(struct dwmci_host *host)
 	dwmci_writel(host, DWMCI_CLKSEL, priv->sdr_timing);
 }
 
-unsigned int exynos_dwmci_get_clk(struct dwmci_host *host)
+unsigned int exynos_dwmci_get_clk(struct dwmci_host *host, uint freq)
 {
 	unsigned long sclk;
 	int8_t clk_div;

include/dwmmc.h +15 -1

index 7a7555a..25cf42c 100644
@@ -163,7 +163,21 @@ struct dwmci_host {
 
 	void (*clksel)(struct dwmci_host *host);
 	void (*board_init)(struct dwmci_host *host);
-	unsigned int (*get_mmc_clk)(struct dwmci_host *host);
+
+	/**
+	 * Get / set a particular MMC clock frequency
+	 *
+	 * This is used to request the current clock frequency of the clock
+	 * that drives the DWMMC peripheral. The caller will then use this
+	 * information to work out the divider it needs to achieve the
+	 * required MMC bus clock frequency. If you want to handle the
+	 * clock external to DWMMC, use @freq to select the frequency and
+	 * return that value too. Then DWMMC will put itself in bypass mode.
+	 *
+	 * @host:	DWMMC host
+	 * @freq:	Frequency the host is trying to achieve
+	 */
+	unsigned int (*get_mmc_clk)(struct dwmci_host *host, uint freq);
 
 	struct mmc_config cfg;
 };

At present there is an arbitrary limit of 4KB for padding. Rockchip needs more than that, so remove this restriction.

Signed-off-by Simon Glass <[email protected]>

Allow the image handler to store the original input file size so that it can reference it later.

Signed-off-by Simon Glass <[email protected]>