2018-02-22 00:07:15

by Laurent Pinchart

[permalink] [raw]
Subject: [PATCH v5 0/8] R-Car DU: Convert LVDS code to bridge driver

Hello,

This patch series addresses a design mistake that dates back from the initial
DU support. Support for the LVDS encoders, which are IP cores separate from
the DU, was bundled in the DU driver. Worse, both the DU and LVDS were
described through a single DT node.

To fix the, patches 1/8 and 2/8 define new DT bindings for the LVDS
encoders, and deprecate their description inside the DU bindings. To retain
backward compatibility with existing DT, patches 3/8 to 7/8 then patch the
device tree at runtime to convert the legacy bindings to the new ones.

With the DT side addressed, patch 8/8 converts the LVDS support code to a
separate bridge driver.

I decided to go for live DT patching in patch 7/8 because implementing
support for both the legacy and new bindings in the driver would have been
very intrusive, and prevented further cleanups. This version relies more
heavily on overlays to avoid touching the internals of the OF core compared to
v2, even if manual fixes to the device tree are still needed.

As all the patches but the last one have been acked, I plan to send a pull
request by the end of the week if no new issue is discovered.

Compared to v4, the patch "of: changeset: Add of_changeset_node_move method"
has been dropped as it's not needed. The patches that update the DT sources to
the new DU and LVDS bindings have been dropped as well to avoid spamming the
lists as they depend on the driver changes going in first to avoid
regressions and haven't been changed.

Compared to v3, this series uses the OF changeset API to update properties
instead of accessing the internals of the property structure. This removes the
local implementation of functions to look up nodes by path and update
properties. In order to do this, I pulled in Pantelis' patch series titled
"[PATCH v2 0/5] of: dynamic: Changesets helpers & fixes" at Rob's request, and
rebased it while taking two small review comments into account.

Rob, I'd like this series to be merged in v4.17. As the changeset helpers are
now a dependency, I'd need you to merge them early (ideally on top of
v4.16-rc1) and provide a stable branch, or get your ack to merge them through
Dave's tree if they don't conflict with what you have and will queue for
v4.17.

This version also drops the small fix to the Porter board device tree that has
been queued for v4.17 already.

Compared to v2, the biggest change is in patch 7/8. Following Rob's and
Frank's reviews it was clear that modifying the unflattened DT structure of
the overlay before applying it wasn't popular. I have thus decided to use one
overlay source per SoC to move as much of the DT changes to the overlay as
possible, and only perform manual modifications (that are still needed as some
of the information is board-specific) on the system DT after applying the
overlay. As a result the overlay is parsed and applied without being modified.

Compared to v1, this series update the r8a7792 and r8a7794 device tree sources
and incorporate review feedback as described by the changelogs of individual
patches.

Laurent Pinchart (4):
dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings
dt-bindings: display: renesas: Deprecate LVDS support in the DU
bindings
drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
drm: rcar-du: Convert LVDS encoder code to bridge driver

Pantelis Antoniou (4):
of: dynamic: Add __of_node_dupv()
of: changesets: Introduce changeset helper methods
of: unittest: changeset helpers
i2c: demux: Use changeset helpers for clarity

.../bindings/display/bridge/renesas,lvds.txt | 56 +++
.../devicetree/bindings/display/renesas,du.txt | 31 +-
MAINTAINERS | 1 +
drivers/gpu/drm/rcar-du/Kconfig | 6 +-
drivers/gpu/drm/rcar-du/Makefile | 10 +-
drivers/gpu/drm/rcar-du/rcar_du_drv.c | 21 +-
drivers/gpu/drm/rcar-du/rcar_du_drv.h | 5 -
drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 175 +------
drivers/gpu/drm/rcar-du/rcar_du_encoder.h | 12 -
drivers/gpu/drm/rcar-du/rcar_du_kms.c | 14 +-
drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 93 ----
drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h | 24 -
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 238 ----------
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h | 64 ---
drivers/gpu/drm/rcar-du/rcar_du_of.c | 307 ++++++++++++
drivers/gpu/drm/rcar-du/rcar_du_of.h | 20 +
.../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts | 79 ++++
.../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts | 53 +++
.../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts | 53 +++
.../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts | 53 +++
.../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts | 53 +++
drivers/gpu/drm/rcar-du/rcar_lvds.c | 524 +++++++++++++++++++++
drivers/i2c/muxes/i2c-demux-pinctrl.c | 12 +-
drivers/of/dynamic.c | 251 +++++++++-
drivers/of/unittest.c | 54 +++
include/linux/of.h | 329 +++++++++++++
26 files changed, 1886 insertions(+), 652 deletions(-)
create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h
delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c
create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h
create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts
create mode 100644 drivers/gpu/drm/rcar-du/rcar_lvds.c

--
Regards,

Laurent Pinchart



2018-02-22 00:05:57

by Laurent Pinchart

[permalink] [raw]
Subject: [PATCH v5 5/8] of: unittest: changeset helpers

From: Pantelis Antoniou <[email protected]>

Add a unitest specific for the new changeset helpers.

Signed-off-by: Pantelis Antoniou <[email protected]>
[Use IS_ENABLED instead of #ifdef]
Signed-off-by: Laurent Pinchart <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
drivers/of/unittest.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 7a9abaae874d..1b21d2c549a8 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -609,6 +609,59 @@ static void __init of_unittest_changeset(void)
#endif
}

+static void __init of_unittest_changeset_helper(void)
+{
+#ifdef CONFIG_OF_DYNAMIC
+ struct device_node *n1, *n2, *n21, *parent, *np;
+ struct of_changeset chgset;
+
+ of_changeset_init(&chgset);
+
+ parent = of_find_node_by_path("/testcase-data/changeset");
+
+ unittest(parent, "testcase setup failure\n");
+ n1 = of_changeset_create_device_node(&chgset,
+ parent, "/testcase-data/changeset/n1");
+ unittest(n1, "testcase setup failure\n");
+ n2 = of_changeset_create_device_node(&chgset,
+ parent, "/testcase-data/changeset/n2");
+ unittest(n2, "testcase setup failure\n");
+ n21 = of_changeset_create_device_node(&chgset, n2, "%s/%s",
+ "/testcase-data/changeset/n2", "n21");
+ unittest(n21, "testcase setup failure\n");
+
+ unittest(!of_changeset_add_property_string(&chgset, parent,
+ "prop-add", "foo"), "fail add prop\n");
+
+ unittest(!of_changeset_attach_node(&chgset, n1), "fail n1 attach\n");
+ unittest(!of_changeset_attach_node(&chgset, n2), "fail n2 attach\n");
+ unittest(!of_changeset_attach_node(&chgset, n21), "fail n21 attach\n");
+
+ unittest(!of_changeset_apply(&chgset), "apply failed\n");
+
+ /* Make sure node names are constructed correctly */
+ np = of_find_node_by_path("/testcase-data/changeset/n1");
+ unittest(np, "'%s' not added\n", n1->full_name);
+ of_node_put(np);
+
+ /* Make sure node names are constructed correctly */
+ np = of_find_node_by_path("/testcase-data/changeset/n2");
+ unittest(np, "'%s' not added\n", n2->full_name);
+ of_node_put(np);
+
+ np = of_find_node_by_path("/testcase-data/changeset/n2/n21");
+ unittest(np, "'%s' not added\n", n21->full_name);
+ of_node_put(np);
+
+ unittest(!of_changeset_revert(&chgset), "revert failed\n");
+
+ of_changeset_destroy(&chgset);
+
+ of_node_put(parent);
+#endif
+}
+
+
static void __init of_unittest_parse_interrupts(void)
{
struct device_node *np;
@@ -2363,6 +2416,7 @@ static int __init of_unittest(void)
of_unittest_property_string();
of_unittest_property_copy();
of_unittest_changeset();
+ of_unittest_changeset_helper();
of_unittest_parse_interrupts();
of_unittest_parse_interrupts_extended();
of_unittest_match_node();
--
Regards,

Laurent Pinchart


2018-02-22 00:06:28

by Laurent Pinchart

[permalink] [raw]
Subject: [PATCH v5 4/8] of: changesets: Introduce changeset helper methods

From: Pantelis Antoniou <[email protected]>

Changesets are very powerful, but the lack of a helper API
makes using them cumbersome. Introduce a simple copy based
API that makes things considerably easier.

To wit, adding a property using the raw API.

struct property *prop;
prop = kzalloc(sizeof(*prop)), GFP_KERNEL);
prop->name = kstrdup("compatible");
prop->value = kstrdup("foo,bar");
prop->length = strlen(prop->value) + 1;
of_changeset_add_property(ocs, np, prop);

while using the helper API

of_changeset_add_property_string(ocs, np, "compatible",
"foo,bar");

Signed-off-by: Pantelis Antoniou <[email protected]>
[Fixed memory leak in __of_changeset_add_update_property_copy()]
[Fixed cpu to be32 conversion sparse warnings]
[Move include <linux/slab.h> to fix compilation when !CONFIG_OF_DYNAMIC]
Signed-off-by: Laurent Pinchart <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
drivers/of/dynamic.c | 222 ++++++++++++++++++++++++++++++++++
include/linux/of.h | 329 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 551 insertions(+)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index a2f0c45836f9..f62083921df2 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -910,3 +910,225 @@ int of_changeset_action(struct of_changeset *ocs, unsigned long action,
return 0;
}
EXPORT_SYMBOL_GPL(of_changeset_action);
+
+/* changeset helpers */
+
+/**
+ * of_changeset_create_device_node - Create an empty device node
+ *
+ * @ocs: changeset pointer
+ * @parent: parent device node
+ * @fmt: format string for the node's full_name
+ * @args: argument list for the format string
+ *
+ * Create an empty device node, marking it as detached and allocated.
+ *
+ * Returns a device node on success, an error encoded pointer otherwise
+ */
+struct device_node *of_changeset_create_device_nodev(
+ struct of_changeset *ocs, struct device_node *parent,
+ const char *fmt, va_list vargs)
+{
+ struct device_node *node;
+
+ node = __of_node_dupv(NULL, fmt, vargs);
+ if (!node)
+ return ERR_PTR(-ENOMEM);
+
+ node->parent = parent;
+ return node;
+}
+EXPORT_SYMBOL_GPL(of_changeset_create_device_nodev);
+
+/**
+ * of_changeset_create_device_node - Create an empty device node
+ *
+ * @ocs: changeset pointer
+ * @parent: parent device node
+ * @fmt: Format string for the node's full_name
+ * ... Arguments
+ *
+ * Create an empty device node, marking it as detached and allocated.
+ *
+ * Returns a device node on success, an error encoded pointer otherwise
+ */
+__printf(3, 4) struct device_node *
+of_changeset_create_device_node(struct of_changeset *ocs,
+ struct device_node *parent, const char *fmt, ...)
+{
+ va_list vargs;
+ struct device_node *node;
+
+ va_start(vargs, fmt);
+ node = of_changeset_create_device_nodev(ocs, parent, fmt, vargs);
+ va_end(vargs);
+ return node;
+}
+EXPORT_SYMBOL_GPL(of_changeset_create_device_node);
+
+/**
+ * __of_changeset_add_property_copy - Create/update a new property copying
+ * name & value
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @value: pointer to the value data
+ * @length: length of the value in bytes
+ * @update: True on update operation
+ *
+ * Adds/updates a property to the changeset by making copies of the name & value
+ * entries. The @update parameter controls whether an add or update takes place.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+int __of_changeset_add_update_property_copy(struct of_changeset *ocs,
+ struct device_node *np, const char *name, const void *value,
+ int length, bool update)
+{
+ struct property *prop;
+ int ret = -ENOMEM;
+
+ prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+ if (!prop)
+ return -ENOMEM;
+
+ prop->name = kstrdup(name, GFP_KERNEL);
+ if (!prop->name)
+ goto out_err;
+
+ /*
+ * NOTE: There is no check for zero length value.
+ * In case of a boolean property, this will allocate a value
+ * of zero bytes. We do this to work around the use
+ * of of_get_property() calls on boolean values.
+ */
+ prop->value = kmemdup(value, length, GFP_KERNEL);
+ if (!prop->value)
+ goto out_err;
+
+ of_property_set_flag(prop, OF_DYNAMIC);
+
+ prop->length = length;
+
+ if (!update)
+ ret = of_changeset_add_property(ocs, np, prop);
+ else
+ ret = of_changeset_update_property(ocs, np, prop);
+
+ if (!ret)
+ return 0;
+
+out_err:
+ kfree(prop->value);
+ kfree(prop->name);
+ kfree(prop);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(__of_changeset_add_update_property_copy);
+
+/**
+ * of_changeset_add_property_stringf - Create a new formatted string property
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @fmt: format of string property
+ * ... arguments of the format string
+ *
+ * Adds a string property to the changeset by making copies of the name
+ * and the formatted value.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+__printf(4, 5) int of_changeset_add_property_stringf(
+ struct of_changeset *ocs, struct device_node *np,
+ const char *name, const char *fmt, ...)
+{
+ va_list vargs;
+ int ret;
+
+ va_start(vargs, fmt);
+ ret = __of_changeset_add_update_property_stringv(ocs, np, name, fmt,
+ vargs, false);
+ va_end(vargs);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(of_changeset_add_property_stringf);
+
+/**
+ * of_changeset_update_property_stringf - Update formatted string property
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @fmt: format of string property
+ * ... arguments of the format string
+ *
+ * Updates a string property to the changeset by making copies of the name
+ * and the formatted value.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+int of_changeset_update_property_stringf(
+ struct of_changeset *ocs, struct device_node *np,
+ const char *name, const char *fmt, ...)
+{
+ va_list vargs;
+ int ret;
+
+ va_start(vargs, fmt);
+ ret = __of_changeset_add_update_property_stringv(ocs, np, name, fmt,
+ vargs, true);
+ va_end(vargs);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(of_changeset_update_property_stringf);
+
+/**
+ * __of_changeset_add_update_property_string_list - Create/update a string
+ * list property
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @strs: pointer to the string list
+ * @count: string count
+ * @update: True on update operation
+ *
+ * Adds a string list property to the changeset.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+int __of_changeset_add_update_property_string_list(
+ struct of_changeset *ocs, struct device_node *np,
+ const char *name, const char **strs, int count, bool update)
+{
+ int total = 0, i, ret;
+ char *value, *s;
+
+ for (i = 0; i < count; i++) {
+ /* check if it's NULL */
+ if (!strs[i])
+ return -EINVAL;
+ total += strlen(strs[i]) + 1;
+ }
+
+ value = kmalloc(total, GFP_KERNEL);
+ if (!value)
+ return -ENOMEM;
+
+ for (i = 0, s = value; i < count; i++) {
+ /* no need to check for NULL, check above */
+ strcpy(s, strs[i]);
+ s += strlen(strs[i]) + 1;
+ }
+
+ ret = __of_changeset_add_update_property_copy(ocs, np, name, value,
+ total, update);
+
+ kfree(value);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(__of_changeset_add_update_property_string_list);
diff --git a/include/linux/of.h b/include/linux/of.h
index da1ee95241c1..f8decae8bd31 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -16,6 +16,7 @@
#include <linux/errno.h>
#include <linux/kobject.h>
#include <linux/mod_devicetable.h>
+#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/topology.h>
#include <linux/notifier.h>
@@ -1309,6 +1310,23 @@ static inline int of_changeset_update_property(struct of_changeset *ocs,
{
return of_changeset_action(ocs, OF_RECONFIG_UPDATE_PROPERTY, np, prop);
}
+
+struct device_node *of_changeset_create_device_nodev(
+ struct of_changeset *ocs, struct device_node *parent,
+ const char *fmt, va_list vargs);
+
+__printf(3, 4) struct device_node *
+of_changeset_create_device_node(struct of_changeset *ocs,
+ struct device_node *parent, const char *fmt, ...);
+
+int __of_changeset_add_update_property_copy(struct of_changeset *ocs,
+ struct device_node *np, const char *name, const void *value,
+ int length, bool update);
+
+int __of_changeset_add_update_property_string_list(
+ struct of_changeset *ocs, struct device_node *np,
+ const char *name, const char **strs, int count, bool update);
+
#else /* CONFIG_OF_DYNAMIC */
static inline int of_reconfig_notifier_register(struct notifier_block *nb)
{
@@ -1328,8 +1346,319 @@ static inline int of_reconfig_get_state_change(unsigned long action,
{
return -EINVAL;
}
+
+static inline struct device_node *of_changeset_create_device_nodev(
+ struct of_changeset *ocs, struct device_node *parent,
+ const char *fmt, va_list vargs)
+{
+ return ERR_PTR(-EINVAL);
+}
+
+static inline __printf(3, 4) struct device_node *
+of_changeset_create_device_node(struct of_changeset *ocs,
+ struct device_node *parent, const char *fmt, ...)
+{
+ return ERR_PTR(-EINVAL);
+}
+
+static inline int __of_changeset_add_update_property_copy(
+ struct of_changeset *ocs, struct device_node *np,
+ const char *name, const void *value, int length, bool update)
+{
+ return -EINVAL;
+}
+
+static inline __printf(4, 5) int of_changeset_add_property_stringf(
+ struct of_changeset *ocs, struct device_node *np,
+ const char *name, const char *fmt, ...)
+{
+ return -EINVAL;
+}
+
+static inline int of_changeset_update_property_stringf(
+ struct of_changeset *ocs, struct device_node *np,
+ const char *name, const char *fmt, ...)
+{
+ return -EINVAL;
+}
+
+static inline int __of_changeset_add_update_property_string_list(
+ struct of_changeset *ocs, struct device_node *np,
+ const char *name, const char **strs, int count, bool update)
+{
+ return -EINVAL;
+}
+
#endif /* CONFIG_OF_DYNAMIC */

+/**
+ * of_changeset_add_property_copy - Create a new property copying name & value
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @value: pointer to the value data
+ * @length: length of the value in bytes
+ *
+ * Adds a property to the changeset by making copies of the name & value
+ * entries.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_add_property_copy(struct of_changeset *ocs,
+ struct device_node *np, const char *name,
+ const void *value, int length)
+{
+ return __of_changeset_add_update_property_copy(ocs, np, name, value,
+ length, false);
+}
+
+/**
+ * of_changeset_update_property_copy - Update a property copying name & value
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @value: pointer to the value data
+ * @length: length of the value in bytes
+ *
+ * Update a property to the changeset by making copies of the name & value
+ * entries.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_update_property_copy(struct of_changeset *ocs,
+ struct device_node *np, const char *name,
+ const void *value, int length)
+{
+ return __of_changeset_add_update_property_copy(ocs, np, name, value,
+ length, true);
+}
+
+/**
+ * __of_changeset_add_update_property_string - Create/update a string property
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @str: string property value
+ * @update: True on update operation
+ *
+ * Adds/updates a string property to the changeset by making copies of the name
+ * and the given value. The @update parameter controls whether an add or
+ * update takes place.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int __of_changeset_add_update_property_string(
+ struct of_changeset *ocs, struct device_node *np, const char *name,
+ const char *str, bool update)
+{
+ return __of_changeset_add_update_property_copy(ocs, np, name, str,
+ strlen(str) + 1, update);
+}
+
+/**
+ * __of_changeset_add_update_property_stringv - Create/update a formatted
+ * string property
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @fmt: format of string property
+ * @vargs: arguments of the format string
+ * @update: True on update operation
+ *
+ * Adds/updates a string property to the changeset by making copies of the name
+ * and the formatted value. The @update parameter controls whether an add or
+ * update takes place.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int __of_changeset_add_update_property_stringv(
+ struct of_changeset *ocs, struct device_node *np, const char *name,
+ const char *fmt, va_list vargs, bool update)
+{
+ char *str;
+ int ret;
+
+ str = kvasprintf(GFP_KERNEL, fmt, vargs);
+ if (!str)
+ return -ENOMEM;
+ ret = __of_changeset_add_update_property_string(ocs, np, name, str,
+ update);
+ kfree(str);
+
+ return ret;
+}
+
+/**
+ * of_changeset_add_property_string_list - Create a new string list property
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @strs: pointer to the string list
+ * @count: string count
+ *
+ * Adds a string list property to the changeset.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_add_property_string_list(
+ struct of_changeset *ocs, struct device_node *np, const char *name,
+ const char **strs, int count)
+{
+ return __of_changeset_add_update_property_string_list(ocs, np, name,
+ strs, count, false);
+}
+
+/**
+ * of_changeset_update_property_string_list - Update string list property
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @strs: pointer to the string list
+ * @count: string count
+ *
+ * Updates a string list property to the changeset.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_update_property_string_list(
+ struct of_changeset *ocs, struct device_node *np,
+ const char *name, const char **strs, int count)
+{
+ return __of_changeset_add_update_property_string_list(ocs, np, name,
+ strs, count, true);
+}
+
+/**
+ * of_changeset_add_property_string - Adds a string property
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @str: string property
+ *
+ * Adds a string property to the changeset by making copies of the name
+ * and the string value.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_add_property_string(
+ struct of_changeset *ocs, struct device_node *np,
+ const char *name, const char *str)
+{
+ return __of_changeset_add_update_property_string(ocs, np, name, str,
+ false);
+}
+
+/**
+ * of_changeset_update_property_string - Update a string property
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @str: string property
+ *
+ * Updates a string property to the changeset by making copies of the name
+ * and the string value.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_update_property_string(
+ struct of_changeset *ocs, struct device_node *np,
+ const char *name, const char *str)
+{
+ return __of_changeset_add_update_property_string(ocs, np, name, str,
+ true);
+}
+
+/**
+ * of_changeset_add_property_u32 - Create a new u32 property
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @val: value in host endian format
+ *
+ * Adds a u32 property to the changeset.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_add_property_u32(struct of_changeset *ocs,
+ struct device_node *np, const char *name, u32 val)
+{
+ __be32 v = cpu_to_be32(val);
+
+ return __of_changeset_add_update_property_copy(ocs, np, name, &v,
+ sizeof(v), false);
+}
+
+/**
+ * of_changeset_update_property_u32 - Update u32 property
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ * @val: value in host endian format
+ *
+ * Updates a u32 property to the changeset.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_update_property_u32(
+ struct of_changeset *ocs, struct device_node *np,
+ const char *name, u32 val)
+{
+ __be32 v = cpu_to_be32(val);
+
+ return __of_changeset_add_update_property_copy(ocs, np, name, &v,
+ sizeof(v), true);
+}
+
+/**
+ * of_changeset_add_property_bool - Create a new u32 property
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ *
+ * Adds a bool property to the changeset. Note that there is
+ * no option to set the value to false, since the property
+ * existing sets it to true.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_add_property_bool(
+ struct of_changeset *ocs, struct device_node *np, const char *name)
+{
+ return __of_changeset_add_update_property_copy(ocs, np, name, "", 0,
+ false);
+}
+
+/**
+ * of_changeset_update_property_bool - Update a bool property
+ *
+ * @ocs: changeset pointer
+ * @np: device node pointer
+ * @name: name of the property
+ *
+ * Updates a property to the changeset. Note that there is
+ * no option to set the value to false, since the property
+ * existing sets it to true.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+static inline int of_changeset_update_property_bool(struct of_changeset *ocs,
+ struct device_node *np, const char *name)
+{
+ return __of_changeset_add_update_property_copy(ocs, np, name, "", 0,
+ true);
+}
+
/**
* of_device_is_system_power_controller - Tells if system-power-controller is found for device_node
* @np: Pointer to the given device_node
--
Regards,

Laurent Pinchart


2018-02-22 00:07:06

by Laurent Pinchart

[permalink] [raw]
Subject: [PATCH v5 6/8] i2c: demux: Use changeset helpers for clarity

From: Pantelis Antoniou <[email protected]>

The changeset helpers are easier to use, use them instead of
using the static property.

Signed-off-by: Pantelis Antoniou <[email protected]>
Acked-by: Wolfram Sang <[email protected]>
["okay" -> "ok"]
Signed-off-by: Laurent Pinchart <[email protected]>
---
drivers/i2c/muxes/i2c-demux-pinctrl.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c b/drivers/i2c/muxes/i2c-demux-pinctrl.c
index 33ce032cb701..0f0046831492 100644
--- a/drivers/i2c/muxes/i2c-demux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
@@ -220,10 +220,7 @@ static int i2c_demux_pinctrl_probe(struct platform_device *pdev)

priv = devm_kzalloc(&pdev->dev, sizeof(*priv)
+ num_chan * sizeof(struct i2c_demux_pinctrl_chan), GFP_KERNEL);
-
- props = devm_kcalloc(&pdev->dev, num_chan, sizeof(*props), GFP_KERNEL);
-
- if (!priv || !props)
+ if (!priv)
return -ENOMEM;

err = of_property_read_string(np, "i2c-bus-name", &priv->bus_name);
@@ -241,12 +238,9 @@ static int i2c_demux_pinctrl_probe(struct platform_device *pdev)
}
priv->chan[i].parent_np = adap_np;

- props[i].name = devm_kstrdup(&pdev->dev, "status", GFP_KERNEL);
- props[i].value = devm_kstrdup(&pdev->dev, "ok", GFP_KERNEL);
- props[i].length = 3;
-
of_changeset_init(&priv->chan[i].chgset);
- of_changeset_update_property(&priv->chan[i].chgset, adap_np, &props[i]);
+ of_changeset_update_property_string(&priv->chan[i].chgset,
+ adap_np, "status", "ok");
}

priv->num_chan = num_chan;
--
Regards,

Laurent Pinchart


2018-02-22 00:07:23

by Laurent Pinchart

[permalink] [raw]
Subject: [PATCH v5 3/8] of: dynamic: Add __of_node_dupv()

From: Pantelis Antoniou <[email protected]>

Add an __of_node_dupv() private method and make __of_node_dup() use it.
This is required for the subsequent changeset accessors which will
make use of it.

Signed-off-by: Pantelis Antoniou <[email protected]>
[Make __of_node_dupv() static]
Signed-off-by: Laurent Pinchart <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
drivers/of/dynamic.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 7bb33d22b4e2..a2f0c45836f9 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -382,8 +382,9 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
}

/**
- * __of_node_dup() - Duplicate or create an empty device node dynamically.
- * @fmt: Format string (plus vargs) for new full name of the device node
+ * __of_node_dupv() - Duplicate or create an empty device node dynamically.
+ * @fmt: Format string for new full name of the device node
+ * @vargs: va_list containing the arugments for the node full name
*
* Create an device tree node, either by duplicating an empty node or by allocating
* an empty one suitable for further modification. The node data are
@@ -391,17 +392,15 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
* OF_DETACHED bits set. Returns the newly allocated node or NULL on out of
* memory error.
*/
-struct device_node *__of_node_dup(const struct device_node *np, const char *fmt, ...)
+static struct device_node *__of_node_dupv(const struct device_node *np,
+ const char *fmt, va_list vargs)
{
- va_list vargs;
struct device_node *node;

node = kzalloc(sizeof(*node), GFP_KERNEL);
if (!node)
return NULL;
- va_start(vargs, fmt);
node->full_name = kvasprintf(GFP_KERNEL, fmt, vargs);
- va_end(vargs);
if (!node->full_name) {
kfree(node);
return NULL;
@@ -433,6 +432,24 @@ struct device_node *__of_node_dup(const struct device_node *np, const char *fmt,
return NULL;
}

+/**
+ * __of_node_dup() - Duplicate or create an empty device node dynamically.
+ * @fmt: Format string (plus vargs) for new full name of the device node
+ *
+ * See: __of_node_dupv()
+ */
+struct device_node *__of_node_dup(const struct device_node *np,
+ const char *fmt, ...)
+{
+ va_list vargs;
+ struct device_node *node;
+
+ va_start(vargs, fmt);
+ node = __of_node_dupv(np, fmt, vargs);
+ va_end(vargs);
+ return node;
+}
+
static void __of_changeset_entry_destroy(struct of_changeset_entry *ce)
{
of_node_put(ce->np);
--
Regards,

Laurent Pinchart


2018-02-22 09:28:15

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v5 6/8] i2c: demux: Use changeset helpers for clarity

Hi Laurent,

On Thu, Feb 22, 2018 at 1:05 AM, Laurent Pinchart
<[email protected]> wrote:
> From: Pantelis Antoniou <[email protected]>
>
> The changeset helpers are easier to use, use them instead of
> using the static property.
>
> Signed-off-by: Pantelis Antoniou <[email protected]>
> Acked-by: Wolfram Sang <[email protected]>
> ["okay" -> "ok"]

Why? ePAPR says "okay", "disabled", "fail", or "fail-sss".

Sorry for missing this in the previous round.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2018-02-22 10:20:31

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v5 6/8] i2c: demux: Use changeset helpers for clarity

Hi Geert,

On Thursday, 22 February 2018 11:26:44 EET Geert Uytterhoeven wrote:
> On Thu, Feb 22, 2018 at 1:05 AM, Laurent Pinchart wrote:
> > From: Pantelis Antoniou <[email protected]>
> >
> > The changeset helpers are easier to use, use them instead of
> > using the static property.
> >
> > Signed-off-by: Pantelis Antoniou <[email protected]>
> > Acked-by: Wolfram Sang <[email protected]>
> > ["okay" -> "ok"]
>
> Why? ePAPR says "okay", "disabled", "fail", or "fail-sss".
>
> Sorry for missing this in the previous round.

That was per Wolfram's request, and because the existing code uses "ok". I'm
personally fine with any.

--
Regards,

Laurent Pinchart


2018-02-22 11:08:54

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH v5 6/8] i2c: demux: Use changeset helpers for clarity


> > Why? ePAPR says "okay", "disabled", "fail", or "fail-sss".
> >
> > Sorry for missing this in the previous round.
>
> That was per Wolfram's request, and because the existing code uses "ok". I'm
> personally fine with any.

I did? Well, today I don't have a strong preference. Any is fine with
me, too.


Attachments:
(No filename) (322.00 B)
signature.asc (849.00 B)
Download all attachments