[PATCH 1/7] state: print name of property of size mismatch is detected

[PATCH 1/7] state: print name of property of size mismatch is detected

[Marc Kleine-Budde]

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 common/state.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/state.c b/common/state.c
index cc44cecb484f..d4ce7313be97 100644
--- a/common/state.c
+++ b/common/state.c
@@ -517,8 +517,8 @@ static int state_convert_node_variable(struct state *state,
 
 		if (start_size[1] != sv->size) {
 			dev_err(&state->dev,
-				"size mismatch: type=%s(size=%u) size=%u\n",
-			       type_name, sv->size, start_size[1]);
+				"%s: size mismatch: type=%s(size=%u) size=%u\n",
+				name, type_name, sv->size, start_size[1]);
 			ret = -EOVERFLOW;
 			goto out_free;
 		}
-- 
2.1.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/7] state: print proper error message, if reg property is not found

[Marc Kleine-Budde]

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 common/state.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/common/state.c b/common/state.c
index d4ce7313be97..5642f0d41855 100644
--- a/common/state.c
+++ b/common/state.c
@@ -512,8 +512,11 @@ static int state_convert_node_variable(struct state *state,
 
 		ret = of_property_read_u32_array(node, "reg", start_size,
 						 ARRAY_SIZE(start_size));
-		if (ret)
+		if (ret) {
+			dev_err(&state->dev,
+				"%s: reg property not found\n", name);
 			goto out_free;
+		}
 
 		if (start_size[1] != sv->size) {
 			dev_err(&state->dev,
-- 
2.1.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/7] state: make state_release() non static

[Marc Kleine-Budde]

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 common/state.c  | 2 +-
 include/state.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/common/state.c b/common/state.c
index 5642f0d41855..66620a892074 100644
--- a/common/state.c
+++ b/common/state.c
@@ -426,7 +426,7 @@ static struct state *state_new(const char *name)
 	return state;
 }
 
-static void state_release(struct state *state)
+void state_release(struct state *state)
 {
 	list_del(&state->list);
 	unregister_device(&state->dev);
diff --git a/include/state.h b/include/state.h
index d321451e1490..116ec6301c1c 100644
--- a/include/state.h
+++ b/include/state.h
@@ -10,6 +10,7 @@ int state_backend_raw_file(struct state *state, const char *path,
 		off_t offset, size_t size);
 
 struct state *state_new_from_node(const char *name, struct device_node *node);
+void state_release(struct state *state);
 
 struct state *state_by_name(const char *name);
 struct state *state_by_node(const struct device_node *node);
-- 
2.1.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/7] state: add functionality to export state description

[Marc Kleine-Budde]

This patch adds the functionality to export the description of the state, but
not the contents of the variables to a DT node. This will be used in a later
patch to "copy" the state description to the kernel device tree during DT
fixup.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 common/state.c | 61 ++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 38 insertions(+), 23 deletions(-)

diff --git a/common/state.c b/common/state.c
index 66620a892074..744a6f8250c4 100644
--- a/common/state.c
+++ b/common/state.c
@@ -74,12 +74,20 @@ struct state_variable {
 	void *raw;
 };
 
+enum state_convert {
+	STATE_CONVERT_FROM_NODE,
+	STATE_CONVERT_FROM_NODE_CREATE,
+	STATE_CONVERT_TO_NODE,
+	STATE_CONVERT_FIXUP,
+};
+
 /* A variable type (uint32, enum32) */
 struct variable_type {
 	enum state_variable_type type;
 	const char *type_name;
 	struct list_head list;
-	int (*export)(struct state_variable *, struct device_node *);
+	int (*export)(struct state_variable *, struct device_node *,
+			enum state_convert);
 	int (*import)(struct state_variable *, const struct device_node *);
 	struct state_variable *(*create)(struct state *state,
 			const char *name, struct device_node *);
@@ -126,15 +134,15 @@ static inline struct state_uint32 *to_state_uint32(struct state_variable *s)
 }
 
 static int state_uint32_export(struct state_variable *var,
-		struct device_node *node)
+		struct device_node *node, enum state_convert conv)
 {
 	struct state_uint32 *su32 = to_state_uint32(var);
 	int ret;
 
-	if (su32->value_default) {
+	if (su32->value_default || conv == STATE_CONVERT_FIXUP) {
 		ret = of_property_write_u32(node, "default",
 					    su32->value_default);
-		if (ret)
+		if (ret || conv == STATE_CONVERT_FIXUP)
 			return ret;
 	}
 
@@ -193,16 +201,16 @@ static inline struct state_enum32 *to_state_enum32(struct state_variable *s)
 }
 
 static int state_enum32_export(struct state_variable *var,
-		struct device_node *node)
+		struct device_node *node, enum state_convert conv)
 {
 	struct state_enum32 *enum32 = to_state_enum32(var);
 	int ret, i, len;
 	char *prop, *str;
 
-	if (enum32->value_default) {
+	if (enum32->value_default || conv == STATE_CONVERT_FIXUP) {
 		ret = of_property_write_u32(node, "default",
 					    enum32->value_default);
-		if (ret)
+		if (ret || conv == STATE_CONVERT_FIXUP)
 			return ret;
 	}
 
@@ -309,14 +317,14 @@ static inline struct state_mac *to_state_mac(struct state_variable *s)
 }
 
 static int state_mac_export(struct state_variable *var,
-		struct device_node *node)
+		struct device_node *node, enum state_convert conv)
 {
 	struct state_mac *mac = to_state_mac(var);
 	int ret;
 
 	ret = of_property_write_u8_array(node, "default", mac->value_default,
 					 ARRAY_SIZE(mac->value_default));
-	if (ret)
+	if (ret || conv == STATE_CONVERT_FIXUP)
 		return ret;
 
 	return of_property_write_u8_array(node, "value", mac->value,
@@ -446,12 +454,6 @@ static struct state_variable *state_find_var(struct state *state,
 	return ERR_PTR(-ENOENT);
 }
 
-enum state_convert {
-	STATE_CONVERT_FROM_NODE,
-	STATE_CONVERT_FROM_NODE_CREATE,
-	STATE_CONVERT_TO_NODE,
-};
-
 static int state_convert_node_variable(struct state *state,
 		struct device_node *node, struct device_node *parent,
 		const char *parent_name, enum state_convert conv)
@@ -476,7 +478,8 @@ static int state_convert_node_variable(struct state *state,
 			parent_name, parent_name[0] ? "." : "", short_name);
 	free(short_name);
 
-	if (conv == STATE_CONVERT_TO_NODE)
+	if ((conv == STATE_CONVERT_TO_NODE) ||
+	    (conv == STATE_CONVERT_FIXUP))
 		new_node = of_new_node(parent, node->name);
 
 	for_each_child_of_node(node, child) {
@@ -489,6 +492,15 @@ static int state_convert_node_variable(struct state *state,
 	/* parents are allowed to have no type */
 	ret = of_property_read_string(node, "type", &type_name);
 	if (!list_empty(&node->children) && ret == -EINVAL) {
+		if (conv == STATE_CONVERT_FIXUP) {
+			ret = of_property_write_u32(new_node, "#address-cells", 1);
+			if (ret)
+				goto out_free;
+
+			ret = of_property_write_u32(new_node, "#size-cells", 1);
+			if (ret)
+				goto out_free;
+		}
 		ret = 0;
 		goto out_free;
 	} else if (ret) {
@@ -542,7 +554,8 @@ static int state_convert_node_variable(struct state *state,
 		}
 		free(name);
 
-		if (conv == STATE_CONVERT_TO_NODE) {
+		if ((conv == STATE_CONVERT_TO_NODE) ||
+		    (conv == STATE_CONVERT_FIXUP)) {
 			ret = of_set_property(new_node, "type",
 					      vtype->type_name,
 					      strlen(vtype->type_name) + 1, 1);
@@ -559,8 +572,9 @@ static int state_convert_node_variable(struct state *state,
 		}
 	}
 
-	if (conv == STATE_CONVERT_TO_NODE)
-		ret = vtype->export(sv, new_node);
+	if ((conv == STATE_CONVERT_TO_NODE) ||
+	    (conv == STATE_CONVERT_FIXUP))
+		ret = vtype->export(sv, new_node, conv);
 	else
 		ret = vtype->import(sv, node);
 
@@ -574,20 +588,21 @@ out:
 	return ret;
 }
 
-static struct device_node *state_to_node(struct state *state)
+static struct device_node *state_to_node(struct state *state, struct device_node *parent,
+					 enum state_convert conv)
 {
 	struct device_node *child;
 	struct device_node *root;
 	int ret;
 
-	root = of_new_node(NULL, NULL);
+	root = of_new_node(parent, state->root->name);
 	ret = of_property_write_u32(root, "magic", state->magic);
 	if (ret)
 		goto out;
 
 	for_each_child_of_node(state->root, child) {
 		ret = state_convert_node_variable(state, child, root, "",
-						  STATE_CONVERT_TO_NODE);
+						  conv);
 		if (ret)
 			goto out;
 	}
@@ -845,7 +860,7 @@ static int state_backend_dtb_save(struct state_backend *backend,
 	struct device_node *root;
 	struct fdt_header *fdt;
 
-	root = state_to_node(state);
+	root = state_to_node(state, NULL, STATE_CONVERT_TO_NODE);
 	if (IS_ERR(root))
 		return PTR_ERR(root);
 
-- 
2.1.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/7] state: backend: support phandle and of_path references

[Marc Kleine-Budde]

This patch improves the backend property, it can be either a phandle or a
of_path. During probe() of the state driver the backend property is
dereferenced and the resulting of_path is saved in the state context. In a
later patch it will be used to generate a phandle reference to the backend
during DT fixup.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 common/state.c       |  9 ++++---
 drivers/misc/state.c | 70 ++++++++++++++++++++++++++++++++++++++++++----------
 include/state.h      |  7 +++---
 3 files changed, 67 insertions(+), 19 deletions(-)

diff --git a/common/state.c b/common/state.c
index 744a6f8250c4..d6060bb7dff4 100644
--- a/common/state.c
+++ b/common/state.c
@@ -54,6 +54,7 @@ struct state_backend {
 	int (*load)(struct state_backend *backend, struct state *state);
 	int (*save)(struct state_backend *backend, struct state *state);
 	const char *name;
+	const char *of_path;
 	const char *path;
 };
 
@@ -903,7 +904,7 @@ out:
  * @state	The state instance to work on
  * @path	The path where the state will be stored to
  */
-int state_backend_dtb_file(struct state *state, const char *path)
+int state_backend_dtb_file(struct state *state, const char *of_path, const char *path)
 {
 	struct state_backend_dtb *backend_dtb;
 	struct state_backend *backend;
@@ -918,6 +919,7 @@ int state_backend_dtb_file(struct state *state, const char *path)
 
 	backend->load = state_backend_dtb_load;
 	backend->save = state_backend_dtb_save;
+	backend->of_path = xstrdup(of_path);
 	backend->path = xstrdup(path);
 	backend->name = "dtb";
 
@@ -1133,8 +1135,8 @@ out_free:
  * device @size may be 0. The two copies are spread to different
  * eraseblocks if approriate for this device.
  */
-int state_backend_raw_file(struct state *state, const char *path, off_t offset,
-		size_t size)
+int state_backend_raw_file(struct state *state, const char *of_path,
+		const char *path, off_t offset, size_t size)
 {
 	struct state_backend_raw *backend_raw;
 	struct state_backend *backend;
@@ -1159,6 +1161,7 @@ int state_backend_raw_file(struct state *state, const char *path, off_t offset,
 
 	backend->load = state_backend_raw_load;
 	backend->save = state_backend_raw_save;
+	backend->of_path = xstrdup(of_path);
 	backend->path = xstrdup(path);
 	backend->name = "raw";
 
diff --git a/drivers/misc/state.c b/drivers/misc/state.c
index 3b07bb93d725..9e8a40255348 100644
--- a/drivers/misc/state.c
+++ b/drivers/misc/state.c
@@ -24,10 +24,12 @@
 static int state_probe(struct device_d *dev)
 {
 	struct device_node *np = dev->device_node;
+	struct device_node *partition_node;
 	struct state *state;
 	const char *alias;
 	const char *backend_type = NULL;
 	int ret;
+	const char *of_path;
 	char *path;
 
 	if (!np)
@@ -41,28 +43,70 @@ static int state_probe(struct device_d *dev)
 	if (IS_ERR(state))
 		return PTR_ERR(state);
 
-	ret = of_find_path(np, "backend", &path, 0);
-	if (ret)
-		return ret;
+	of_path = of_get_property(np, "backend", NULL);
+	if (!of_path) {
+		ret = -ENODEV;
+		goto out_release;
+	}
+
+	/* guess if of_path is a path, not a phandle */
+	if (of_path[0] == '/') {
+		ret = of_find_path(np, "backend", &path, 0);
+		if (ret)
+			goto out_release;
+	} else {
+		struct device_d *dev;
+		struct cdev *cdev;
+
+		partition_node = of_parse_phandle(np, "backend", 0);
+		if (!partition_node) {
+			ret = -ENODEV;
+			goto out_release;
+		}
+
+		dev = of_find_device_by_node(partition_node);
+		if (!list_is_singular(&dev->cdevs)) {
+			ret = -ENODEV;
+			goto out_release;
+		}
+
+		cdev = list_first_entry(&dev->cdevs, struct cdev, devices_list);
+		if (!cdev) {
+			ret = -ENODEV;
+			goto out_release;
+		}
 
-	dev_info(dev, "outpath: %s\n", path);
+		path = asprintf("/dev/%s", cdev->name);
+		of_path = partition_node->full_name;
+	}
 
 	ret = of_property_read_string(np, "backend-type", &backend_type);
-	if (ret)
-		return ret;
-	else if (!strcmp(backend_type, "raw"))
-		ret = state_backend_raw_file(state, path, 0, 0);
-	else if (!strcmp(backend_type, "dtb"))
-		ret = state_backend_dtb_file(state, path);
-	else
+	if (ret) {
+		goto out_free;
+	} else if (!strcmp(backend_type, "raw")) {
+		ret = state_backend_raw_file(state, of_path, path, 0, 0);
+	} else if (!strcmp(backend_type, "dtb")) {
+		ret = state_backend_dtb_file(state, of_path, path);
+	} else {
 		dev_warn(dev, "invalid backend type: %s\n", backend_type);
+		ret = -ENODEV;
+		goto out_free;
+	}
 
 	if (ret)
-		return ret;
+		goto out_free;
 
-	state_load(state);
+	dev_info(dev, "backend: %s, path: %s, of_path: %s\n", backend_type, path, of_path);
+	free(path);
 
+	state_load(state);
 	return 0;
+
+ out_free:
+	free(path);
+ out_release:
+	state_release(state);
+	return ret;
 }
 
 static __maybe_unused struct of_device_id state_ids[] = {
diff --git a/include/state.h b/include/state.h
index 116ec6301c1c..08c4e8654b9e 100644
--- a/include/state.h
+++ b/include/state.h
@@ -5,9 +5,10 @@
 
 struct state;
 
-int state_backend_dtb_file(struct state *state, const char *path);
-int state_backend_raw_file(struct state *state, const char *path,
-		off_t offset, size_t size);
+int state_backend_dtb_file(struct state *state, const char *of_path,
+		const char *path);
+int state_backend_raw_file(struct state *state, const char *of_path,
+		const char *path, off_t offset, size_t size);
 
 struct state *state_new_from_node(const char *name, struct device_node *node);
 void state_release(struct state *state);
-- 
2.1.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 5/7] state: backend: support phandle and of_path references

[Sascha Hauer]

On Wed, May 13, 2015 at 12:12:30PM +0200, Marc Kleine-Budde wrote:
> This patch improves the backend property, it can be either a phandle or a
> of_path. During probe() of the state driver the backend property is
> dereferenced and the resulting of_path is saved in the state context. In a
> later patch it will be used to generate a phandle reference to the backend
> during DT fixup.
> 
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  common/state.c       |  9 ++++---
>  drivers/misc/state.c | 70 ++++++++++++++++++++++++++++++++++++++++++----------
>  include/state.h      |  7 +++---
>  3 files changed, 67 insertions(+), 19 deletions(-)
> 
> diff --git a/common/state.c b/common/state.c
> index 744a6f8250c4..d6060bb7dff4 100644
> --- a/common/state.c
> +++ b/common/state.c
> @@ -54,6 +54,7 @@ struct state_backend {
>  	int (*load)(struct state_backend *backend, struct state *state);
>  	int (*save)(struct state_backend *backend, struct state *state);
>  	const char *name;
> +	const char *of_path;
>  	const char *path;
>  };
>  
> @@ -903,7 +904,7 @@ out:
>   * @state	The state instance to work on
>   * @path	The path where the state will be stored to
>   */
> -int state_backend_dtb_file(struct state *state, const char *path)
> +int state_backend_dtb_file(struct state *state, const char *of_path, const char *path)
>  {
>  	struct state_backend_dtb *backend_dtb;
>  	struct state_backend *backend;
> @@ -918,6 +919,7 @@ int state_backend_dtb_file(struct state *state, const char *path)
>  
>  	backend->load = state_backend_dtb_load;
>  	backend->save = state_backend_dtb_save;
> +	backend->of_path = xstrdup(of_path);
>  	backend->path = xstrdup(path);
>  	backend->name = "dtb";
>  
> @@ -1133,8 +1135,8 @@ out_free:
>   * device @size may be 0. The two copies are spread to different
>   * eraseblocks if approriate for this device.
>   */
> -int state_backend_raw_file(struct state *state, const char *path, off_t offset,
> -		size_t size)
> +int state_backend_raw_file(struct state *state, const char *of_path,
> +		const char *path, off_t offset, size_t size)
>  {
>  	struct state_backend_raw *backend_raw;
>  	struct state_backend *backend;
> @@ -1159,6 +1161,7 @@ int state_backend_raw_file(struct state *state, const char *path, off_t offset,
>  
>  	backend->load = state_backend_raw_load;
>  	backend->save = state_backend_raw_save;
> +	backend->of_path = xstrdup(of_path);
>  	backend->path = xstrdup(path);
>  	backend->name = "raw";
>  
> diff --git a/drivers/misc/state.c b/drivers/misc/state.c
> index 3b07bb93d725..9e8a40255348 100644
> --- a/drivers/misc/state.c
> +++ b/drivers/misc/state.c
> @@ -24,10 +24,12 @@
>  static int state_probe(struct device_d *dev)
>  {
>  	struct device_node *np = dev->device_node;
> +	struct device_node *partition_node;
>  	struct state *state;
>  	const char *alias;
>  	const char *backend_type = NULL;
>  	int ret;
> +	const char *of_path;
>  	char *path;
>  
>  	if (!np)
> @@ -41,28 +43,70 @@ static int state_probe(struct device_d *dev)
>  	if (IS_ERR(state))
>  		return PTR_ERR(state);
>  
> -	ret = of_find_path(np, "backend", &path, 0);
> -	if (ret)
> -		return ret;
> +	of_path = of_get_property(np, "backend", NULL);
> +	if (!of_path) {
> +		ret = -ENODEV;
> +		goto out_release;
> +	}
> +
> +	/* guess if of_path is a path, not a phandle */
> +	if (of_path[0] == '/') {

Since this also matches to the 47th phandle in the device tree I made
this:

	if (of_path[0] == '/' && len > 1)

And applied up to 6/7

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 5/7] state: backend: support phandle and of_path references

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 4067 bytes --]

On 05/15/2015 07:05 AM, Sascha Hauer wrote:
> On Wed, May 13, 2015 at 12:12:30PM +0200, Marc Kleine-Budde wrote:
>> This patch improves the backend property, it can be either a phandle or a
>> of_path. During probe() of the state driver the backend property is
>> dereferenced and the resulting of_path is saved in the state context. In a
>> later patch it will be used to generate a phandle reference to the backend
>> during DT fixup.
>>
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> ---
>>  common/state.c       |  9 ++++---
>>  drivers/misc/state.c | 70 ++++++++++++++++++++++++++++++++++++++++++----------
>>  include/state.h      |  7 +++---
>>  3 files changed, 67 insertions(+), 19 deletions(-)
>>
>> diff --git a/common/state.c b/common/state.c
>> index 744a6f8250c4..d6060bb7dff4 100644
>> --- a/common/state.c
>> +++ b/common/state.c
>> @@ -54,6 +54,7 @@ struct state_backend {
>>  	int (*load)(struct state_backend *backend, struct state *state);
>>  	int (*save)(struct state_backend *backend, struct state *state);
>>  	const char *name;
>> +	const char *of_path;
>>  	const char *path;
>>  };
>>  
>> @@ -903,7 +904,7 @@ out:
>>   * @state	The state instance to work on
>>   * @path	The path where the state will be stored to
>>   */
>> -int state_backend_dtb_file(struct state *state, const char *path)
>> +int state_backend_dtb_file(struct state *state, const char *of_path, const char *path)
>>  {
>>  	struct state_backend_dtb *backend_dtb;
>>  	struct state_backend *backend;
>> @@ -918,6 +919,7 @@ int state_backend_dtb_file(struct state *state, const char *path)
>>  
>>  	backend->load = state_backend_dtb_load;
>>  	backend->save = state_backend_dtb_save;
>> +	backend->of_path = xstrdup(of_path);
>>  	backend->path = xstrdup(path);
>>  	backend->name = "dtb";
>>  
>> @@ -1133,8 +1135,8 @@ out_free:
>>   * device @size may be 0. The two copies are spread to different
>>   * eraseblocks if approriate for this device.
>>   */
>> -int state_backend_raw_file(struct state *state, const char *path, off_t offset,
>> -		size_t size)
>> +int state_backend_raw_file(struct state *state, const char *of_path,
>> +		const char *path, off_t offset, size_t size)
>>  {
>>  	struct state_backend_raw *backend_raw;
>>  	struct state_backend *backend;
>> @@ -1159,6 +1161,7 @@ int state_backend_raw_file(struct state *state, const char *path, off_t offset,
>>  
>>  	backend->load = state_backend_raw_load;
>>  	backend->save = state_backend_raw_save;
>> +	backend->of_path = xstrdup(of_path);
>>  	backend->path = xstrdup(path);
>>  	backend->name = "raw";
>>  
>> diff --git a/drivers/misc/state.c b/drivers/misc/state.c
>> index 3b07bb93d725..9e8a40255348 100644
>> --- a/drivers/misc/state.c
>> +++ b/drivers/misc/state.c
>> @@ -24,10 +24,12 @@
>>  static int state_probe(struct device_d *dev)
>>  {
>>  	struct device_node *np = dev->device_node;
>> +	struct device_node *partition_node;
>>  	struct state *state;
>>  	const char *alias;
>>  	const char *backend_type = NULL;
>>  	int ret;
>> +	const char *of_path;
>>  	char *path;
>>  
>>  	if (!np)
>> @@ -41,28 +43,70 @@ static int state_probe(struct device_d *dev)
>>  	if (IS_ERR(state))
>>  		return PTR_ERR(state);
>>  
>> -	ret = of_find_path(np, "backend", &path, 0);
>> -	if (ret)
>> -		return ret;
>> +	of_path = of_get_property(np, "backend", NULL);
>> +	if (!of_path) {
>> +		ret = -ENODEV;
>> +		goto out_release;
>> +	}
>> +
>> +	/* guess if of_path is a path, not a phandle */
>> +	if (of_path[0] == '/') {
> 
> Since this also matches to the 47th phandle in the device tree I made
> this:
> 
> 	if (of_path[0] == '/' && len > 1)

Makes sense.

> And applied up to 6/7

Thanks,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 6/7] state: add fixup to copy state from barebox to kernel device tree

[Marc Kleine-Budde]

This patch registers a DT fixup, that copies the state nodes from the active
to the kernel DT. The backend reference will be a phandles.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 common/state.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/common/state.c b/common/state.c
index d6060bb7dff4..ca691d2827fa 100644
--- a/common/state.c
+++ b/common/state.c
@@ -676,6 +676,93 @@ static int state_from_node(struct state *state, struct device_node *node,
 	return ret;
 }
 
+static int of_state_fixup(struct device_node *root, void *ctx)
+{
+	struct state *state = ctx;
+	const char *compatible = "barebox,state";
+	struct device_node *new_node, *node, *parent, *backend_node;
+	struct property *p;
+	int ret;
+	phandle phandle;
+
+	node = of_find_node_by_path_from(root, state->root->full_name);
+	if (node) {
+		/* replace existing node - it will be deleted later */
+		parent = node->parent;
+	} else {
+		char *of_path, *c;
+
+		/* look for parent, remove last '/' from path */
+		of_path = xstrdup(state->root->full_name);
+		c = strrchr(of_path, '/');
+		if (!c)
+			return -ENODEV;
+		*c = '0';
+		parent = of_find_node_by_path(of_path);
+		if (!parent)
+			parent = root;
+
+		free(of_path);
+	}
+
+	/* serialize variable definitions */
+	new_node = state_to_node(state, parent, STATE_CONVERT_FIXUP);
+	if (IS_ERR(new_node))
+		return PTR_ERR(new_node);
+
+	/* compatible */
+	p = of_new_property(new_node, "compatible", compatible,
+			    strlen(compatible) + 1);
+	if (!p) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/* backend-type */
+	if (!state->backend) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	p = of_new_property(new_node, "backend-type", state->backend->name,
+			    strlen(state->backend->name) + 1);
+	if (!p) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/* backend phandle */
+	backend_node = of_find_node_by_path_from(root, state->backend->of_path);
+	if (!backend_node) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	phandle = of_node_create_phandle(backend_node);
+	ret = of_property_write_u32(new_node, "backend", phandle);
+	if (ret)
+		goto out;
+
+	/* address-cells + size-cells */
+	ret = of_property_write_u32(root, "#address-cells", 1);
+	if (ret)
+		goto out;
+
+	ret = of_property_write_u32(root, "#size-cells", 1);
+	if (ret)
+		goto out;
+
+	/* delete existing node */
+	if (node)
+		of_delete_node(node);
+
+	return 0;
+
+ out:
+	of_delete_node(new_node);
+	return ret;
+}
+
 /*
  * state_new_from_node - create a new state instance from a device_node
  *
@@ -697,6 +784,11 @@ struct state *state_new_from_node(const char *name, struct device_node *node)
 		return ERR_PTR(ret);
 	}
 
+	ret = of_register_fixup(of_state_fixup, state);
+	if (ret) {
+		state_release(state);
+		return ERR_PTR(ret);
+	}
 
 	return state;
 }
-- 
2.1.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 6/7] state: add fixup to copy state from barebox to kernel device tree

[Uwe Kleine-König]

On Wed, May 13, 2015 at 12:12:31PM +0200, Marc Kleine-Budde wrote:
> This patch registers a DT fixup, that copies the state nodes from the active
> to the kernel DT. The backend reference will be a phandles.
> 
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  common/state.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
> 
> diff --git a/common/state.c b/common/state.c
> index d6060bb7dff4..ca691d2827fa 100644
> --- a/common/state.c
> +++ b/common/state.c
> @@ -676,6 +676,93 @@ static int state_from_node(struct state *state, struct device_node *node,
>  	return ret;
>  }
>  
> +static int of_state_fixup(struct device_node *root, void *ctx)
> +{
> +	struct state *state = ctx;
> +	const char *compatible = "barebox,state";
> +	struct device_node *new_node, *node, *parent, *backend_node;
> +	struct property *p;
> +	int ret;
> +	phandle phandle;
> +
> +	node = of_find_node_by_path_from(root, state->root->full_name);
> +	if (node) {
> +		/* replace existing node - it will be deleted later */
> +		parent = node->parent;
> +	} else {
> +		char *of_path, *c;
> +
> +		/* look for parent, remove last '/' from path */
> +		of_path = xstrdup(state->root->full_name);
> +		c = strrchr(of_path, '/');
> +		if (!c)
> +			return -ENODEV;
> +		*c = '0';
> +		parent = of_find_node_by_path(of_path);
> +		if (!parent)
> +			parent = root;
> +
> +		free(of_path);
> +	}
> +
> +	/* serialize variable definitions */
> +	new_node = state_to_node(state, parent, STATE_CONVERT_FIXUP);
> +	if (IS_ERR(new_node))
> +		return PTR_ERR(new_node);
> +
> +	/* compatible */
> +	p = of_new_property(new_node, "compatible", compatible,
> +			    strlen(compatible) + 1);
> +	if (!p) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	/* backend-type */
> +	if (!state->backend) {
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +
> +	p = of_new_property(new_node, "backend-type", state->backend->name,
> +			    strlen(state->backend->name) + 1);
> +	if (!p) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	/* backend phandle */
> +	backend_node = of_find_node_by_path_from(root, state->backend->of_path);
> +	if (!backend_node) {
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +
> +	phandle = of_node_create_phandle(backend_node);
> +	ret = of_property_write_u32(new_node, "backend", phandle);
> +	if (ret)
> +		goto out;
> +
> +	/* address-cells + size-cells */
> +	ret = of_property_write_u32(root, "#address-cells", 1);
> +	if (ret)
> +		goto out;
> +
> +	ret = of_property_write_u32(root, "#size-cells", 1);
> +	if (ret)
> +		goto out;
> +
> +	/* delete existing node */
> +	if (node)
> +		of_delete_node(node);
> +
> +	return 0;
> +
> + out:
> +	of_delete_node(new_node);
> +	return ret;
> +}
> +
>  /*
>   * state_new_from_node - create a new state instance from a device_node
>   *
> @@ -697,6 +784,11 @@ struct state *state_new_from_node(const char *name, struct device_node *node)
>  		return ERR_PTR(ret);
>  	}
>  
> +	ret = of_register_fixup(of_state_fixup, state);
> +	if (ret) {
> +		state_release(state);
> +		return ERR_PTR(ret);
> +	}
This is broken. state_new_from_node is called in state_probe:

	static int state_probe(struct device_d *dev)
	{
		...
		state = state_new_from_node(alias, np);
		...
		ret = of_find_path(np, "backend", &path, 0);
		if (ret)
			return ret;
		...

So if there is an error with the backend (or something else later that
makes the state invalid) the fixup is still registered. So when the
fixup is actually called state might be invalid resulting in
of_state_fixup using possibly overwritten memory.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH] fixup! state: add fixup to copy state from barebox to kernel device tree

[Uwe Kleine-König]

of_unregister_fixup must also be called in the release function.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
This depends on the patch "of: add a function to remove an of_fixup" I
just sent to the list
---
 common/state.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/common/state.c b/common/state.c
index fb13ae272db9..bd9c5a712aac 100644
--- a/common/state.c
+++ b/common/state.c
@@ -435,13 +435,6 @@ static struct state *state_new(const char *name)
 	return state;
 }
 
-void state_release(struct state *state)
-{
-	list_del(&state->list);
-	unregister_device(&state->dev);
-	free(state);
-}
-
 static struct state_variable *state_find_var(struct state *state,
 					     const char *name)
 {
@@ -760,6 +753,14 @@ static int of_state_fixup(struct device_node *root, void *ctx)
 	return ret;
 }
 
+void state_release(struct state *state)
+{
+	of_unregister_fixup(of_state_fixup, state);
+	list_del(&state->list);
+	unregister_device(&state->dev);
+	free(state);
+}
+
 /*
  * state_new_from_node - create a new state instance from a device_node
  *
-- 
2.1.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] fixup! state: add fixup to copy state from barebox to kernel device tree

[Sascha Hauer]

On Sat, May 16, 2015 at 12:15:27PM +0200, Uwe Kleine-König wrote:
> of_unregister_fixup must also be called in the release function.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> This depends on the patch "of: add a function to remove an of_fixup" I
> just sent to the list
> ---
>  common/state.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)

Applied with said patch beforehand.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 7/7] state: return -EPROBE_DEFER if the backend isn't available

[Marc Kleine-Budde]

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/misc/state.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/state.c b/drivers/misc/state.c
index 9e8a40255348..dbe88715a103 100644
--- a/drivers/misc/state.c
+++ b/drivers/misc/state.c
@@ -52,6 +52,8 @@ static int state_probe(struct device_d *dev)
 	/* guess if of_path is a path, not a phandle */
 	if (of_path[0] == '/') {
 		ret = of_find_path(np, "backend", &path, 0);
+		if (ret == -ENODEV)
+			ret = -EPROBE_DEFER;
 		if (ret)
 			goto out_release;
 	} else {
@@ -66,7 +68,7 @@ static int state_probe(struct device_d *dev)
 
 		dev = of_find_device_by_node(partition_node);
 		if (!list_is_singular(&dev->cdevs)) {
-			ret = -ENODEV;
+			ret = -EPROBE_DEFER;
 			goto out_release;
 		}
 
-- 
2.1.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 7/7] state: return -EPROBE_DEFER if the backend isn't available

[Uwe Kleine-König]

Hello,

On Wed, May 13, 2015 at 12:12:32PM +0200, Marc Kleine-Budde wrote:
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
BTW, I didn't add this S-o-b on purpose because I didn't consider this
patch done.

> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/misc/state.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/state.c b/drivers/misc/state.c
> index 9e8a40255348..dbe88715a103 100644
> --- a/drivers/misc/state.c
> +++ b/drivers/misc/state.c
> @@ -52,6 +52,8 @@ static int state_probe(struct device_d *dev)
>  	/* guess if of_path is a path, not a phandle */
>  	if (of_path[0] == '/') {
>  		ret = of_find_path(np, "backend", &path, 0);
> +		if (ret == -ENODEV)
> +			ret = -EPROBE_DEFER;
>  		if (ret)
>  			goto out_release;
>  	} else {
In my testcase I only use this hunk.

> @@ -66,7 +68,7 @@ static int state_probe(struct device_d *dev)
>  
>  		dev = of_find_device_by_node(partition_node);
>  		if (!list_is_singular(&dev->cdevs)) {
> -			ret = -ENODEV;
> +			ret = -EPROBE_DEFER;
>  			goto out_release;
>  		}
This one was done without testing, there is another if below that might
need adaption too (or instead of this one):

                cdev = list_first_entry(&dev->cdevs, struct cdev, devices_list);
                if (!cdev) {
                        ret = -ENODEV;
                        goto out_release;
                }

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox