RE: [PATCHv3 2/2] regmap: add DT endianness binding support.
> > + * of_regmap_endian_by_type() - Parse and lookup the endian referenced
> > + * by a device node
> > + * @np: pointer to clock consumer node
>
> This is not the clock consumer, right?
>
Yes, you are right.
I will fix it.
> > + * @type: type of consumer's endian input
> > + *
> > + * This function parses the device endian property, and uses them to
> > + * determine the endian of the registers and values.
> > + */
> > +static int of_regmap_endian_by_type(struct device_node *np,
> > + enum regmap_endian_type type,
> > + enum regmap_endian *endian)
> > +{
> > + if (!endian)
> > + return -EINVAL;
> > +
> > + switch (type) {
> > + case REGMAP_ENDIAN_REG:
> > + if (of_property_read_bool(np, "big-endian-reg"))
> > + *endian = REGMAP_ENDIAN_BIG;
> > + else if (of_property_read_bool(np, "little-endian-reg"))
> > + *endian = REGMAP_ENDIAN_LITTLE;
> > + else
> > + *endian = REGMAP_ENDIAN_NATIVE;
>
> You could also return an error code here as there was no DT property
> found. This doesn't change to much in the code, just an idea.
>
The properties are all Boolean ones, and returning an error code here
is not proper. Isn't it ?
Returning _NATIVE here means to indicate that the the CPU and DEV are in
the same endianness mode...
Thanks very much,
BRs
Xiubo
> Regards,
>
> Markus
>
> > + break;
> > + case REGMAP_ENDIAN_VAL:
> > + if (of_property_read_bool(np, "big-endian-val"))
> > + *endian = REGMAP_ENDIAN_BIG;
> > + else if (of_property_read_bool(np, "little-endian-val"))
> > + *endian = REGMAP_ENDIAN_LITTLE;
> > + else
> > + *endian = REGMAP_ENDIAN_NATIVE;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int of_regmap_get_endian(struct device *dev,
> > + const struct regmap_bus *bus,
> > + const struct regmap_config *config,
> > + enum regmap_endian_type type,
> > + enum regmap_endian *endian)
> > +{
> > + int ret;
> > +
> > + if (!endian || !config)
> > + return -EINVAL;
> > +
> > + /*
> > +* Firstly, try to parse the endian from driver's config,
> > +* this is to be compatible with the none DT or the old drivers.
> > +* From the driver's config the endian value maybe:
> > +* REGMAP_ENDIAN_BIG,
> > +* REGMAP_ENDIAN_LITTLE,
> > +* REGMAP_ENDIAN_NATIVE,
> > +* REGMAP_ENDIAN_DEFAULT.
> > +*/
> > + switch (type) {
> > + case REGMAP_ENDIAN_REG:
> > + *endian = config->reg_format_endian;
> > + break;
> > + case REGMAP_ENDIAN_VAL:
> > + *endian = config->val_format_endian;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + /*
> > +* If the endian parsing from driver's config is REGMAP_ENDIAN_DEFAULT,
> > +* that means maybe we are using the DT node to specify the endianness.
> > +*/
> > + if (*endian != REGMAP_ENDIAN_DEFAULT)
> > + return 0;
> > +
> > + /*
> > +* Secondly, try to parse the endian from DT node if the
> > +* driver config does not specify it.
> > +* From the DT node the endian value maybe:
> > +* REGMAP_ENDIAN_BIG,
> > +* REGMAP_ENDIAN_LITTLE,
> > +* REGMAP_ENDIAN_NATIVE,
> > +*/
> > + if (dev) {
> > + ret = of_regmap_endian_by_type(dev->of_node, type, endian);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + /*
> > +* If the endian parsing from DT node is REGMAP_ENDIAN_NATIVE, that
> > +* maybe means the DT does not care the endianness or it should use
> > +* the regmap bus's default endianness, then we should try to check
> > +* whether the regmap bus has specified the default endianess.
> > +*/
> > + if (*endian != REGMAP_ENDIAN_NATIVE)
> > + return 0;
> > +
> > + /*
> > +* Finally, try to parse the endian from regmap bus config
> > +* if in device's DT node the endian property is absent.
> > +*/
> > + switch (type) {
> > + case REGMAP_ENDIAN_REG:
> > + if (bus && bus->reg_format_endian_default)
> > + *endian = bus->reg_format_endian_default;
> > + break;
> > + case REGMAP_ENDIAN_VAL:
> > + if (bus && bus->val_format_endian_default)
> > + *endian = bus->val_format_endian_default;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > /**
> > * regmap_init(): Initialise register map
> > *
> > @@ -518,17 +645,15 @@ struct regmap *regmap_init(struct device *dev,
> > map->reg_read = _regmap_bus_read;
> >
Re: [PATCHv3 2/2] regmap: add DT endianness binding support.
Hi,
On Wed, Apr 30, 2014 at 12:43:49PM +0800, Xiubo Li wrote:
[...]
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index 8e8cea1..946e901 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -422,6 +423,132 @@ static void regmap_range_exit(struct regmap *map)
> kfree(map->selector_work_buf);
> }
>
> +enum regmap_endian_type {
> + REGMAP_ENDIAN_REG,
> + REGMAP_ENDIAN_VAL,
> +};
> +
> +/**
> + * of_regmap_endian_by_type() - Parse and lookup the endian referenced
> + * by a device node
> + * @np: pointer to clock consumer node
This is not the clock consumer, right?
> + * @type: type of consumer's endian input
> + *
> + * This function parses the device endian property, and uses them to
> + * determine the endian of the registers and values.
> + */
> +static int of_regmap_endian_by_type(struct device_node *np,
> + enum regmap_endian_type type,
> + enum regmap_endian *endian)
> +{
> + if (!endian)
> + return -EINVAL;
> +
> + switch (type) {
> + case REGMAP_ENDIAN_REG:
> + if (of_property_read_bool(np, "big-endian-reg"))
> + *endian = REGMAP_ENDIAN_BIG;
> + else if (of_property_read_bool(np, "little-endian-reg"))
> + *endian = REGMAP_ENDIAN_LITTLE;
> + else
> + *endian = REGMAP_ENDIAN_NATIVE;
You could also return an error code here as there was no DT property
found. This doesn't change to much in the code, just an idea.
Regards,
Markus
> + break;
> + case REGMAP_ENDIAN_VAL:
> + if (of_property_read_bool(np, "big-endian-val"))
> + *endian = REGMAP_ENDIAN_BIG;
> + else if (of_property_read_bool(np, "little-endian-val"))
> + *endian = REGMAP_ENDIAN_LITTLE;
> + else
> + *endian = REGMAP_ENDIAN_NATIVE;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int of_regmap_get_endian(struct device *dev,
> + const struct regmap_bus *bus,
> + const struct regmap_config *config,
> + enum regmap_endian_type type,
> + enum regmap_endian *endian)
> +{
> + int ret;
> +
> + if (!endian || !config)
> + return -EINVAL;
> +
> + /*
> + * Firstly, try to parse the endian from driver's config,
> + * this is to be compatible with the none DT or the old drivers.
> + * From the driver's config the endian value maybe:
> + * REGMAP_ENDIAN_BIG,
> + * REGMAP_ENDIAN_LITTLE,
> + * REGMAP_ENDIAN_NATIVE,
> + * REGMAP_ENDIAN_DEFAULT.
> + */
> + switch (type) {
> + case REGMAP_ENDIAN_REG:
> + *endian = config->reg_format_endian;
> + break;
> + case REGMAP_ENDIAN_VAL:
> + *endian = config->val_format_endian;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + /*
> + * If the endian parsing from driver's config is REGMAP_ENDIAN_DEFAULT,
> + * that means maybe we are using the DT node to specify the endianness.
> + */
> + if (*endian != REGMAP_ENDIAN_DEFAULT)
> + return 0;
> +
> + /*
> + * Secondly, try to parse the endian from DT node if the
> + * driver config does not specify it.
> + * From the DT node the endian value maybe:
> + * REGMAP_ENDIAN_BIG,
> + * REGMAP_ENDIAN_LITTLE,
> + * REGMAP_ENDIAN_NATIVE,
> + */
> + if (dev) {
> + ret = of_regmap_endian_by_type(dev->of_node, type, endian);
> + if (ret < 0)
> + return ret;
> + }
> +
> + /*
> + * If the endian parsing from DT node is REGMAP_ENDIAN_NATIVE, that
> + * maybe means the DT does not care the endianness or it should use
> + * the regmap bus's default endianness, then we should try to check
> + * whether the regmap bus has specified the default endianess.
> + */
> + if (*endian != REGMAP_ENDIAN_NATIVE)
> + return 0;
> +
> + /*
> + * Finally, try to parse the endian from regmap bus config
> + * if in device's DT node the endian property is absent.
> + */
> + switch (type) {
> + case REGMAP_ENDIAN_REG:
> + if (bus && bus->reg_format_endian_default)
> + *endian = bus->reg_format_endian_default;
> + break;
> + case REGMAP_ENDIAN_VAL:
> + if (bus && bus->val_format_endian_default)
> + *endian = bus->val_format_endian_default;
> + break;
> + default:
> + return -EINVAL;
>
Re: [PATCHv3 2/2] regmap: add DT endianness binding support.
Hi,
On Wed, Apr 30, 2014 at 12:43:49PM +0800, Xiubo Li wrote:
[...]
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 8e8cea1..946e901 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -15,6 +15,7 @@
#include linux/export.h
#include linux/mutex.h
#include linux/err.h
+#include linux/of.h
#include linux/rbtree.h
#include linux/sched.h
@@ -422,6 +423,132 @@ static void regmap_range_exit(struct regmap *map)
kfree(map-selector_work_buf);
}
+enum regmap_endian_type {
+ REGMAP_ENDIAN_REG,
+ REGMAP_ENDIAN_VAL,
+};
+
+/**
+ * of_regmap_endian_by_type() - Parse and lookup the endian referenced
+ * by a device node
+ * @np: pointer to clock consumer node
This is not the clock consumer, right?
+ * @type: type of consumer's endian input
+ *
+ * This function parses the device endian property, and uses them to
+ * determine the endian of the registers and values.
+ */
+static int of_regmap_endian_by_type(struct device_node *np,
+ enum regmap_endian_type type,
+ enum regmap_endian *endian)
+{
+ if (!endian)
+ return -EINVAL;
+
+ switch (type) {
+ case REGMAP_ENDIAN_REG:
+ if (of_property_read_bool(np, big-endian-reg))
+ *endian = REGMAP_ENDIAN_BIG;
+ else if (of_property_read_bool(np, little-endian-reg))
+ *endian = REGMAP_ENDIAN_LITTLE;
+ else
+ *endian = REGMAP_ENDIAN_NATIVE;
You could also return an error code here as there was no DT property
found. This doesn't change to much in the code, just an idea.
Regards,
Markus
+ break;
+ case REGMAP_ENDIAN_VAL:
+ if (of_property_read_bool(np, big-endian-val))
+ *endian = REGMAP_ENDIAN_BIG;
+ else if (of_property_read_bool(np, little-endian-val))
+ *endian = REGMAP_ENDIAN_LITTLE;
+ else
+ *endian = REGMAP_ENDIAN_NATIVE;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int of_regmap_get_endian(struct device *dev,
+ const struct regmap_bus *bus,
+ const struct regmap_config *config,
+ enum regmap_endian_type type,
+ enum regmap_endian *endian)
+{
+ int ret;
+
+ if (!endian || !config)
+ return -EINVAL;
+
+ /*
+ * Firstly, try to parse the endian from driver's config,
+ * this is to be compatible with the none DT or the old drivers.
+ * From the driver's config the endian value maybe:
+ * REGMAP_ENDIAN_BIG,
+ * REGMAP_ENDIAN_LITTLE,
+ * REGMAP_ENDIAN_NATIVE,
+ * REGMAP_ENDIAN_DEFAULT.
+ */
+ switch (type) {
+ case REGMAP_ENDIAN_REG:
+ *endian = config-reg_format_endian;
+ break;
+ case REGMAP_ENDIAN_VAL:
+ *endian = config-val_format_endian;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /*
+ * If the endian parsing from driver's config is REGMAP_ENDIAN_DEFAULT,
+ * that means maybe we are using the DT node to specify the endianness.
+ */
+ if (*endian != REGMAP_ENDIAN_DEFAULT)
+ return 0;
+
+ /*
+ * Secondly, try to parse the endian from DT node if the
+ * driver config does not specify it.
+ * From the DT node the endian value maybe:
+ * REGMAP_ENDIAN_BIG,
+ * REGMAP_ENDIAN_LITTLE,
+ * REGMAP_ENDIAN_NATIVE,
+ */
+ if (dev) {
+ ret = of_regmap_endian_by_type(dev-of_node, type, endian);
+ if (ret 0)
+ return ret;
+ }
+
+ /*
+ * If the endian parsing from DT node is REGMAP_ENDIAN_NATIVE, that
+ * maybe means the DT does not care the endianness or it should use
+ * the regmap bus's default endianness, then we should try to check
+ * whether the regmap bus has specified the default endianess.
+ */
+ if (*endian != REGMAP_ENDIAN_NATIVE)
+ return 0;
+
+ /*
+ * Finally, try to parse the endian from regmap bus config
+ * if in device's DT node the endian property is absent.
+ */
+ switch (type) {
+ case REGMAP_ENDIAN_REG:
+ if (bus bus-reg_format_endian_default)
+ *endian = bus-reg_format_endian_default;
+ break;
+ case REGMAP_ENDIAN_VAL:
+ if (bus bus-val_format_endian_default)
+ *endian = bus-val_format_endian_default;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
/**
* regmap_init(): Initialise register map
RE: [PATCHv3 2/2] regmap: add DT endianness binding support.
+ * of_regmap_endian_by_type() - Parse and lookup the endian referenced
+ * by a device node
+ * @np: pointer to clock consumer node
This is not the clock consumer, right?
Yes, you are right.
I will fix it.
+ * @type: type of consumer's endian input
+ *
+ * This function parses the device endian property, and uses them to
+ * determine the endian of the registers and values.
+ */
+static int of_regmap_endian_by_type(struct device_node *np,
+ enum regmap_endian_type type,
+ enum regmap_endian *endian)
+{
+ if (!endian)
+ return -EINVAL;
+
+ switch (type) {
+ case REGMAP_ENDIAN_REG:
+ if (of_property_read_bool(np, big-endian-reg))
+ *endian = REGMAP_ENDIAN_BIG;
+ else if (of_property_read_bool(np, little-endian-reg))
+ *endian = REGMAP_ENDIAN_LITTLE;
+ else
+ *endian = REGMAP_ENDIAN_NATIVE;
You could also return an error code here as there was no DT property
found. This doesn't change to much in the code, just an idea.
The properties are all Boolean ones, and returning an error code here
is not proper. Isn't it ?
Returning _NATIVE here means to indicate that the the CPU and DEV are in
the same endianness mode...
Thanks very much,
BRs
Xiubo
Regards,
Markus
+ break;
+ case REGMAP_ENDIAN_VAL:
+ if (of_property_read_bool(np, big-endian-val))
+ *endian = REGMAP_ENDIAN_BIG;
+ else if (of_property_read_bool(np, little-endian-val))
+ *endian = REGMAP_ENDIAN_LITTLE;
+ else
+ *endian = REGMAP_ENDIAN_NATIVE;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int of_regmap_get_endian(struct device *dev,
+ const struct regmap_bus *bus,
+ const struct regmap_config *config,
+ enum regmap_endian_type type,
+ enum regmap_endian *endian)
+{
+ int ret;
+
+ if (!endian || !config)
+ return -EINVAL;
+
+ /*
+* Firstly, try to parse the endian from driver's config,
+* this is to be compatible with the none DT or the old drivers.
+* From the driver's config the endian value maybe:
+* REGMAP_ENDIAN_BIG,
+* REGMAP_ENDIAN_LITTLE,
+* REGMAP_ENDIAN_NATIVE,
+* REGMAP_ENDIAN_DEFAULT.
+*/
+ switch (type) {
+ case REGMAP_ENDIAN_REG:
+ *endian = config-reg_format_endian;
+ break;
+ case REGMAP_ENDIAN_VAL:
+ *endian = config-val_format_endian;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /*
+* If the endian parsing from driver's config is REGMAP_ENDIAN_DEFAULT,
+* that means maybe we are using the DT node to specify the endianness.
+*/
+ if (*endian != REGMAP_ENDIAN_DEFAULT)
+ return 0;
+
+ /*
+* Secondly, try to parse the endian from DT node if the
+* driver config does not specify it.
+* From the DT node the endian value maybe:
+* REGMAP_ENDIAN_BIG,
+* REGMAP_ENDIAN_LITTLE,
+* REGMAP_ENDIAN_NATIVE,
+*/
+ if (dev) {
+ ret = of_regmap_endian_by_type(dev-of_node, type, endian);
+ if (ret 0)
+ return ret;
+ }
+
+ /*
+* If the endian parsing from DT node is REGMAP_ENDIAN_NATIVE, that
+* maybe means the DT does not care the endianness or it should use
+* the regmap bus's default endianness, then we should try to check
+* whether the regmap bus has specified the default endianess.
+*/
+ if (*endian != REGMAP_ENDIAN_NATIVE)
+ return 0;
+
+ /*
+* Finally, try to parse the endian from regmap bus config
+* if in device's DT node the endian property is absent.
+*/
+ switch (type) {
+ case REGMAP_ENDIAN_REG:
+ if (bus bus-reg_format_endian_default)
+ *endian = bus-reg_format_endian_default;
+ break;
+ case REGMAP_ENDIAN_VAL:
+ if (bus bus-val_format_endian_default)
+ *endian = bus-val_format_endian_default;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
/**
* regmap_init(): Initialise register map
*
@@ -518,17 +645,15 @@ struct regmap *regmap_init(struct device *dev,
map-reg_read = _regmap_bus_read;
}
- reg_endian = config-reg_format_endian;
- if (reg_endian == REGMAP_ENDIAN_DEFAULT)
- reg_endian = bus-reg_format_endian_default;
- if (reg_endian == REGMAP_ENDIAN_DEFAULT)
- reg_endian = REGMAP_ENDIAN_BIG;
-
- val_endian =
