mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/2] state: use the given backend storage type name
@ 2017-08-17  9:32 Juergen Borleis
  2017-08-17  9:32 ` [PATCH 2/2] state: don't use uninitialized variable Juergen Borleis
  2017-09-06 12:11 ` [PATCH 1/2] state: use the given backend storage type name Sascha Hauer
  0 siblings, 2 replies; 3+ messages in thread
From: Juergen Borleis @ 2017-08-17  9:32 UTC (permalink / raw)
  To: barebox

Change 119f92b27e131a0cb506fe8d8bffe8010fb14a3d already tried to fix it, but
forgets the 'direct' usecase.

The 'backend-storage-type' node is optional. Its default depends on the
capability of the used backend memory, which means "circular" or NULL.
The latter defaults to 'direct' in the routines.
If it is NULL, the devicetree fixup routine skips exporting a
'backend-storage-type' node to the kernel's devicetree.

But currently if the 'backend-storage-type' node is explicitly given as
'direct', it will be skipped silently and set to NULL instead. In this
case the user of the 'barebox-state' tool then ends up with the warning:

"No backend-storage-type found, using default"

which is annoying, because it was given.

Storing the given value will still use a NULL if the
'backend-storage-type' node isn't defined, but stores everything else if
it is defined. Then the 'backend-storage-type' node is present in the
kernel's devicetree as well.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 Documentation/devicetree/bindings/barebox/barebox,state.rst | 6 ++++--
 common/state/backend_storage.c                              | 9 +++++----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/barebox/barebox,state.rst b/Documentation/devicetree/bindings/barebox/barebox,state.rst
index 4b1aade66..cebb5f828 100644
--- a/Documentation/devicetree/bindings/barebox/barebox,state.rst
+++ b/Documentation/devicetree/bindings/barebox/barebox,state.rst
@@ -48,8 +48,10 @@ Optional Properties
 ###################
 
 * ``backend-stridesize``: stride counted in bytes. See note below.
-* ``backend-storage-type``: Defines the backend storage type to ``direct`` or
-  ``circular``. Defaults to ``circular`` for media which requires erase cycles.
+* ``backend-storage-type``: Defines the backend storage type to ``direct``,
+  ``circular`` or ``noncircular``. If the backend memory needs to be erased
+  prior a write it defaults to the ``circular`` storage backend type, for backend
+  memories like RAMs or EEPROMs it defaults to the ``direct`` storage backend type.
 * ``algo``: A HMAC algorithm used to detect manipulation of the data
   or header, sensible values follow this pattern ``hmac(<HASH>)``,
   e.g. ``hmac(sha256)``. Only available for the ``backend-type`` ``raw``.
diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 2e2478cb6..825db824b 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -357,11 +357,12 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
  * @param dev_offset Offset in the device to start writing at.
  * @param max_size Maximum size of the data. May be 0 for infinite.
  * @param stridesize Distance between two copies of the data. Not relevant for MTD
- * @param storagetype Type of the storage backend. This may be NULL where we
- * autoselect some backwardscompatible backend options
+ * @param storagetype Type of the storage backend. May be NULL for autoselection.
  * @return 0 on success, -errno otherwise
  *
- * Depending on the filetype, we create mtd buckets or normal file buckets.
+ * If the backend memory needs to be erased prior a write, the @b storagetype
+ * defaults to 'circular' storage backend type, for backend memories like RAMs
+ * or EEPROMs @b storagetype defaults to the 'direct' storage backend type.
  */
 int state_storage_init(struct state *state, const char *path,
 		       off_t offset, size_t max_size, uint32_t stridesize,
@@ -373,6 +374,7 @@ int state_storage_init(struct state *state, const char *path,
 
 	INIT_LIST_HEAD(&storage->buckets);
 	storage->dev = &state->dev;
+	storage->name = storagetype;
 	storage->stridesize = stridesize;
 	storage->offset = offset;
 	storage->max_size = max_size;
@@ -387,7 +389,6 @@ int state_storage_init(struct state *state, const char *path,
 			storage->name = "circular";
 			circular = true;
 		} else if (!strcmp(storagetype, "noncircular")) {
-			storage->name = "noncircular";
 			dev_warn(storage->dev, "using old format circular storage type.\n");
 			circular = false;
 		} else {
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/2] state: don't use uninitialized variable
  2017-08-17  9:32 [PATCH 1/2] state: use the given backend storage type name Juergen Borleis
@ 2017-08-17  9:32 ` Juergen Borleis
  2017-09-06 12:11 ` [PATCH 1/2] state: use the given backend storage type name Sascha Hauer
  1 sibling, 0 replies; 3+ messages in thread
From: Juergen Borleis @ 2017-08-17  9:32 UTC (permalink / raw)
  To: barebox

Printing 'ret' makes no sense.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 common/state/backend_storage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 825db824b..c6ebe8624 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -214,7 +214,7 @@ static int mtd_get_meminfo(const char *path, struct mtd_info_user *meminfo)
 
 	fd = open(path, O_RDONLY);
 	if (fd < 0) {
-		pr_err("Failed to open '%s', %d\n", path, ret);
+		pr_err("Failed to open '%s', %d\n", path, fd);
 		return fd;
 	}
 
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] state: use the given backend storage type name
  2017-08-17  9:32 [PATCH 1/2] state: use the given backend storage type name Juergen Borleis
  2017-08-17  9:32 ` [PATCH 2/2] state: don't use uninitialized variable Juergen Borleis
@ 2017-09-06 12:11 ` Sascha Hauer
  1 sibling, 0 replies; 3+ messages in thread
From: Sascha Hauer @ 2017-09-06 12:11 UTC (permalink / raw)
  To: Juergen Borleis; +Cc: barebox

On Thu, Aug 17, 2017 at 11:32:32AM +0200, Juergen Borleis wrote:
> Change 119f92b27e131a0cb506fe8d8bffe8010fb14a3d already tried to fix it, but
> forgets the 'direct' usecase.
> 
> The 'backend-storage-type' node is optional. Its default depends on the
> capability of the used backend memory, which means "circular" or NULL.
> The latter defaults to 'direct' in the routines.
> If it is NULL, the devicetree fixup routine skips exporting a
> 'backend-storage-type' node to the kernel's devicetree.
> 
> But currently if the 'backend-storage-type' node is explicitly given as
> 'direct', it will be skipped silently and set to NULL instead. In this
> case the user of the 'barebox-state' tool then ends up with the warning:
> 
> "No backend-storage-type found, using default"
> 
> which is annoying, because it was given.
> 
> Storing the given value will still use a NULL if the
> 'backend-storage-type' node isn't defined, but stores everything else if
> it is defined. Then the 'backend-storage-type' node is present in the
> kernel's devicetree as well.
> 
> Signed-off-by: Juergen Borleis <jbe@pengutronix.de>

Applied, thanks

Sascha

> ---
>  Documentation/devicetree/bindings/barebox/barebox,state.rst | 6 ++++--
>  common/state/backend_storage.c                              | 9 +++++----
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/barebox/barebox,state.rst b/Documentation/devicetree/bindings/barebox/barebox,state.rst
> index 4b1aade66..cebb5f828 100644
> --- a/Documentation/devicetree/bindings/barebox/barebox,state.rst
> +++ b/Documentation/devicetree/bindings/barebox/barebox,state.rst
> @@ -48,8 +48,10 @@ Optional Properties
>  ###################
>  
>  * ``backend-stridesize``: stride counted in bytes. See note below.
> -* ``backend-storage-type``: Defines the backend storage type to ``direct`` or
> -  ``circular``. Defaults to ``circular`` for media which requires erase cycles.
> +* ``backend-storage-type``: Defines the backend storage type to ``direct``,
> +  ``circular`` or ``noncircular``. If the backend memory needs to be erased
> +  prior a write it defaults to the ``circular`` storage backend type, for backend
> +  memories like RAMs or EEPROMs it defaults to the ``direct`` storage backend type.
>  * ``algo``: A HMAC algorithm used to detect manipulation of the data
>    or header, sensible values follow this pattern ``hmac(<HASH>)``,
>    e.g. ``hmac(sha256)``. Only available for the ``backend-type`` ``raw``.
> diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
> index 2e2478cb6..825db824b 100644
> --- a/common/state/backend_storage.c
> +++ b/common/state/backend_storage.c
> @@ -357,11 +357,12 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
>   * @param dev_offset Offset in the device to start writing at.
>   * @param max_size Maximum size of the data. May be 0 for infinite.
>   * @param stridesize Distance between two copies of the data. Not relevant for MTD
> - * @param storagetype Type of the storage backend. This may be NULL where we
> - * autoselect some backwardscompatible backend options
> + * @param storagetype Type of the storage backend. May be NULL for autoselection.
>   * @return 0 on success, -errno otherwise
>   *
> - * Depending on the filetype, we create mtd buckets or normal file buckets.
> + * If the backend memory needs to be erased prior a write, the @b storagetype
> + * defaults to 'circular' storage backend type, for backend memories like RAMs
> + * or EEPROMs @b storagetype defaults to the 'direct' storage backend type.
>   */
>  int state_storage_init(struct state *state, const char *path,
>  		       off_t offset, size_t max_size, uint32_t stridesize,
> @@ -373,6 +374,7 @@ int state_storage_init(struct state *state, const char *path,
>  
>  	INIT_LIST_HEAD(&storage->buckets);
>  	storage->dev = &state->dev;
> +	storage->name = storagetype;
>  	storage->stridesize = stridesize;
>  	storage->offset = offset;
>  	storage->max_size = max_size;
> @@ -387,7 +389,6 @@ int state_storage_init(struct state *state, const char *path,
>  			storage->name = "circular";
>  			circular = true;
>  		} else if (!strcmp(storagetype, "noncircular")) {
> -			storage->name = "noncircular";
>  			dev_warn(storage->dev, "using old format circular storage type.\n");
>  			circular = false;
>  		} else {
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-09-06 12:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-17  9:32 [PATCH 1/2] state: use the given backend storage type name Juergen Borleis
2017-08-17  9:32 ` [PATCH 2/2] state: don't use uninitialized variable Juergen Borleis
2017-09-06 12:11 ` [PATCH 1/2] state: use the given backend storage type name Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox