mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* State patches
@ 2017-03-31  7:03 Sascha Hauer
  2017-03-31  7:03 ` [PATCH 01/42] state: Make pointing to the backend using a phandle the only supported method Sascha Hauer
                   ` (42 more replies)
  0 siblings, 43 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

Hi All,

Here is a ton of patches working on the state framework:

- make code easier to follow
  - Drop cached backend, replace with open coded more direct code
  - drop backend as extra struct type
  - drop lazy init code
- update documentation
- Add keystore command
  Using authenticated state requires hardware support which currently
  is only available for i.MX6. At least for testing purposes a keystore
  command to set keys is useful. This way authenticated state can be
  tested without hardware support
- more robust conversion of the backend path to a device node
- make work better with NOR flash: The current code ended up reading
  from NOR Flash byte by byte which was painfully slow

----------------------------------------------------------------
Sascha Hauer (42):
      state: Make pointing to the backend using a phandle the only supported method
      state: Use positive logic
      state: backend: remove .get_packed_len
      state: backend: remove len_hint argument from state_storage_read
      state: Drop backend as extra struct type
      state: merge backend.c into state.c
      state: open code state_backend_init in caller
      state: remove unnecessary argument from state_format_init
      state: pass struct state * to storage functions
      state: storage: initialize variable once outside loop
      state: backend_circular: Read whole PEB
      state: drop lazy_init
      state: simplify direct backend
      state: replace len_hint logic
      state: Convert all bufs to void *
      state: Drop cache bucket
      state: backend-direct: Fix max_size
      state: bucket: Make output more informative
      state: backend_bucket_direct: max_size is always given
      state: backend: Add more fields to struct state_backend_storage
      state: backend_circular: remove unnecessary warning
      state: storage: direct: do not close file that is not opened
      state: backend: Add some documentation
      state: backend_circular: default to circular storage
      state: backend_circular: rewrite function doc
      state: backend_storage: Rename variable nr_copies to n_buckets
      state: backend_storage: Rename variable desired_copies to desired_buckets
      state: backend_storage: rewrite function doc
      state: backend_storage: make locally used variable static
      state: backend_storage: rename more variables
      keystore: implement forgetting secrets
      commands: implement keystore command
      commands: state: allow loading state with -l
      crypto: digest: initialize earlier
      state: backend_raw: alloc digest only when needed
      state: backend_circular: Set minumum writesize to 8
      state: backend bucket circular: Explain metadata
      state: Allow to load without authentification
      state: Update documentation
      state: Do not load state during state_new_from_node
      state: Remove -EUCLEAN check from userspace tool
      state: find device node from device path, not from device node path

 .../devicetree/bindings/barebox/barebox,state.rst  |  33 +-
 Documentation/user/state.rst                       |  33 +-
 commands/Kconfig                                   |   6 +
 commands/Makefile                                  |   1 +
 commands/keystore.c                                | 100 +++++
 commands/state.c                                   |  21 +-
 common/state/Makefile                              |   2 -
 common/state/backend.c                             | 189 ---------
 common/state/backend_bucket_cached.c               | 155 -------
 common/state/backend_bucket_circular.c             |  96 ++---
 common/state/backend_bucket_direct.c               |  20 +-
 common/state/backend_format_dtb.c                  |  13 +-
 common/state/backend_format_raw.c                  | 127 +++---
 common/state/backend_storage.c                     | 456 ++++++++-------------
 common/state/state.c                               | 188 +++++++--
 common/state/state.h                               |  71 ++--
 crypto/hmac.c                                      |   2 +-
 crypto/keystore.c                                  |  53 ++-
 crypto/sha1.c                                      |   2 +-
 crypto/sha2.c                                      |   2 +-
 crypto/sha4.c                                      |   2 +-
 drivers/misc/state.c                               |   6 +
 include/crypto/keystore.h                          |   4 +
 include/state.h                                    |   1 +
 24 files changed, 713 insertions(+), 870 deletions(-)
 create mode 100644 commands/keystore.c
 delete mode 100644 common/state/backend.c
 delete mode 100644 common/state/backend_bucket_cached.c

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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 01/42] state: Make pointing to the backend using a phandle the only supported method
  2017-03-31  7:03 State patches Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-05-15  9:18   ` Jan Remmet
  2017-03-31  7:03 ` [PATCH 02/42] state: Use positive logic Sascha Hauer
                   ` (41 subsequent siblings)
  42 siblings, 1 reply; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

All other methods are broken for some time already: When starting the
kernel the state code rewrites the state node in the device tree and
replaced the "backend" property with a phandle - even when the target
can't be described as a phandle. Since using phandles is the nicest way
to point to the storage device anyway remove the other methods.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../devicetree/bindings/barebox/barebox,state.rst  | 27 ++++++++++++++++------
 common/state/state.c                               | 21 ++++++++---------
 2 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/Documentation/devicetree/bindings/barebox/barebox,state.rst b/Documentation/devicetree/bindings/barebox/barebox,state.rst
index 438cc434a2..e9daa65f1a 100644
--- a/Documentation/devicetree/bindings/barebox/barebox,state.rst
+++ b/Documentation/devicetree/bindings/barebox/barebox,state.rst
@@ -29,7 +29,8 @@ Required properties:
 
 * ``compatible``: should be ``barebox,state``;
 * ``magic``: A 32bit number used as a magic to identify the state
-* ``backend``: describes where the data for this state is stored
+* ``backend``: contains a phandle to the device/partition which holds the
+  actual state data.
 * ``backend-type``: should be ``raw`` or ``dtb``.
 
 Optional properties:
@@ -77,19 +78,31 @@ Example::
   	magic = <0x27031977>;
   	compatible = "barebox,state";
   	backend-type = "raw";
-  	backend = &eeprom, "partname:state";
+  	backend = &state_part;
 
   	foo {
-		reg = <0x00 0x4>;
-		type = "uint32";
+  		reg = <0x00 0x4>;
+  		type = "uint32";
   		default = <0x0>;
   	};
 
   	bar {
-		reg = <0x10 0x4>;
-		type = "enum32";
+  		reg = <0x10 0x4>;
+  		type = "enum32";
   		names = "baz", "qux";
-		default = <1>;
+  		default = <1>;
+  	};
+  };
+
+  &nand_flash {
+  	partitions {
+  		compatible = "fixed-partitions";
+  		#address-cells = <1>;
+  		#size-cells = <1>;
+  		state_part: state@10000 {
+  			label = "state";
+  			reg = <0x10000 0x10000>;
+  		};
   	};
   };
 
diff --git a/common/state/state.c b/common/state/state.c
index 02bb1bb24a..6e6b3a6f08 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -452,19 +452,16 @@ struct state *state_new_from_node(struct device_node *node, char *path,
 	}
 
 	if (!path) {
-		/* guess if of_path is a path, not a phandle */
-		if (of_path[0] == '/' && len > 1) {
-			ret = of_find_path(node, "backend", &path, 0);
-		} else {
-			struct device_node *partition_node;
-
-			partition_node = of_parse_phandle(node, "backend", 0);
-			if (!partition_node)
-				goto out_release_state;
-
-			of_path = partition_node->full_name;
-			ret = of_find_path_by_node(partition_node, &path, 0);
+		struct device_node *partition_node;
+
+		partition_node = of_parse_phandle(node, "backend", 0);
+		if (!partition_node) {
+			dev_err(&state->dev, "Cannot resolve \"backend\" phandle\n");
+			goto out_release_state;
 		}
+
+		of_path = partition_node->full_name;
+		ret = of_find_path_by_node(partition_node, &path, 0);
 		if (ret) {
 			if (ret != -EPROBE_DEFER)
 				dev_err(&state->dev, "state failed to parse path to backend: %s\n",
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 02/42] state: Use positive logic
  2017-03-31  7:03 State patches Sascha Hauer
  2017-03-31  7:03 ` [PATCH 01/42] state: Make pointing to the backend using a phandle the only supported method Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 03/42] state: backend: remove .get_packed_len Sascha Hauer
                   ` (40 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

bools with "non" in the names are rather confusing. Switch to
positive logic.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_storage.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 5dc8c50267..d7bd147441 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -249,7 +249,7 @@ const int desired_copies = 3;
  * @param storage Storage object
  * @param meminfo Info about the mtd device
  * @param path Path to the device
- * @param non_circular Use non-circular mode to write data that is compatible with the old on-flash format
+ * @param circular If false, use non-circular mode to write data that is compatible with the old on-flash format
  * @param dev_offset Offset to start at in the device.
  * @param max_size Maximum size to use for data. May be 0 for infinite.
  * @return 0 on success, -errno otherwise
@@ -262,7 +262,7 @@ const int desired_copies = 3;
  */
 static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 					  struct mtd_info_user *meminfo,
-					  const char *path, bool non_circular,
+					  const char *path, bool circular,
 					  off_t dev_offset, size_t max_size)
 {
 	struct state_backend_storage_bucket *bucket;
@@ -285,7 +285,7 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 		unsigned int eraseblock = offset / meminfo->erasesize;
 		bool lazy_init = true;
 
-		if (non_circular)
+		if (!circular)
 			writesize = meminfo->erasesize;
 
 		ret = state_backend_bucket_circular_create(storage->dev, path,
@@ -483,16 +483,16 @@ int state_storage_init(struct state_backend_storage *storage,
 		ret = mtd_get_meminfo(path, &meminfo);
 
 	if (!ret && !(meminfo.flags & MTD_NO_ERASE)) {
-		bool non_circular = false;
+		bool circular = true;
 		if (!storagetype) {
-			non_circular = true;
+			circular = false;
 		} else if (strcmp(storagetype, "circular")) {
 			dev_warn(storage->dev, "Unknown storagetype '%s', falling back to old format circular storage type.\n",
 				 storagetype);
-			non_circular = true;
+			circular = false;
 		}
 		return state_storage_mtd_buckets_init(storage, &meminfo, path,
-						      non_circular, offset,
+						      circular, offset,
 						      max_size);
 	} else {
 		return state_storage_file_buckets_init(storage, path, offset,
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 03/42] state: backend: remove .get_packed_len
  2017-03-31  7:03 State patches Sascha Hauer
  2017-03-31  7:03 ` [PATCH 01/42] state: Make pointing to the backend using a phandle the only supported method Sascha Hauer
  2017-03-31  7:03 ` [PATCH 02/42] state: Use positive logic Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 04/42] state: backend: remove len_hint argument from state_storage_read Sascha Hauer
                   ` (39 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

.get_packed_len isn't implemented by any backend, so remove the
hook and its potential caller.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend.c | 3 ---
 common/state/state.h   | 2 --
 2 files changed, 5 deletions(-)

diff --git a/common/state/backend.c b/common/state/backend.c
index 5235bb0283..6b06c845f0 100644
--- a/common/state/backend.c
+++ b/common/state/backend.c
@@ -74,9 +74,6 @@ int state_load(struct state *state)
 	int ret;
 	struct state_backend *backend = &state->backend;
 
-	if (backend->format->get_packed_len)
-		len_hint = backend->format->get_packed_len(backend->format,
-							   state);
 	ret = state_storage_read(&backend->storage, backend->format,
 				 state->magic, &buf, &len, len_hint);
 	if (ret) {
diff --git a/common/state/state.h b/common/state/state.h
index bc6917de61..0197cb839a 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -53,8 +53,6 @@ struct state_backend_format {
 		     uint8_t ** buf, ssize_t * len);
 	int (*unpack) (struct state_backend_format * format,
 		       struct state * state, const uint8_t * buf, ssize_t len);
-	ssize_t(*get_packed_len) (struct state_backend_format * format,
-				   struct state * state);
 	void (*free) (struct state_backend_format * format);
 	const char *name;
 };
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 04/42] state: backend: remove len_hint argument from state_storage_read
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (2 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 03/42] state: backend: remove .get_packed_len Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 05/42] state: Drop backend as extra struct type Sascha Hauer
                   ` (38 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

The argument is 0 in the only caller, so remove the argument.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend.c         | 3 +--
 common/state/backend_storage.c | 5 ++---
 common/state/state.h           | 3 +--
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/common/state/backend.c b/common/state/backend.c
index 6b06c845f0..803390e2aa 100644
--- a/common/state/backend.c
+++ b/common/state/backend.c
@@ -70,12 +70,11 @@ int state_load(struct state *state)
 {
 	uint8_t *buf;
 	ssize_t len;
-	ssize_t len_hint = 0;
 	int ret;
 	struct state_backend *backend = &state->backend;
 
 	ret = state_storage_read(&backend->storage, backend->format,
-				 state->magic, &buf, &len, len_hint);
+				 state->magic, &buf, &len);
 	if (ret) {
 		dev_err(&state->dev, "Failed to read state with format %s, %d\n",
 			backend->format->name, ret);
diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index d7bd147441..5481f27df9 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -129,14 +129,13 @@ int state_storage_restore_consistency(struct state_backend_storage *storage,
  */
 int state_storage_read(struct state_backend_storage *storage,
 		       struct state_backend_format *format,
-		       uint32_t magic, uint8_t ** buf, ssize_t * len,
-		       ssize_t len_hint)
+		       uint32_t magic, uint8_t ** buf, ssize_t * len)
 {
 	struct state_backend_storage_bucket *bucket;
 	int ret;
 
 	list_for_each_entry(bucket, &storage->buckets, bucket_list) {
-		*len = len_hint;
+		*len = 0;
 		ret = bucket_lazy_init(bucket);
 		if (ret) {
 			dev_warn(storage->dev, "Failed to init bucket/read state backend bucket, %d\n",
diff --git a/common/state/state.h b/common/state/state.h
index 0197cb839a..ddf45239eb 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -234,8 +234,7 @@ int state_storage_restore_consistency(struct state_backend_storage
 				      ssize_t len);
 int state_storage_read(struct state_backend_storage *storage,
 		       struct state_backend_format *format,
-		       uint32_t magic, uint8_t **buf, ssize_t *len,
-		       ssize_t len_hint);
+		       uint32_t magic, uint8_t **buf, ssize_t *len);
 
 static inline struct state_uint32 *to_state_uint32(struct state_variable *s)
 {
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 05/42] state: Drop backend as extra struct type
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (3 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 04/42] state: backend: remove len_hint argument from state_storage_read Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 06/42] state: merge backend.c into state.c Sascha Hauer
                   ` (37 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

struct state_backend is embedded into struct state. This additional
indirection does not have any real gain. Drop it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend.c | 48 +++++++++++++++++++++++-------------------------
 common/state/state.c   | 34 +++++++++++++++++-----------------
 common/state/state.h   | 23 ++++++-----------------
 3 files changed, 46 insertions(+), 59 deletions(-)

diff --git a/common/state/backend.c b/common/state/backend.c
index 803390e2aa..1773392e8d 100644
--- a/common/state/backend.c
+++ b/common/state/backend.c
@@ -31,19 +31,18 @@ int state_save(struct state *state)
 	uint8_t *buf;
 	ssize_t len;
 	int ret;
-	struct state_backend *backend = &state->backend;
 
 	if (!state->dirty)
 		return 0;
 
-	ret = backend->format->pack(backend->format, state, &buf, &len);
+	ret = state->format->pack(state->format, state, &buf, &len);
 	if (ret) {
 		dev_err(&state->dev, "Failed to pack state with backend format %s, %d\n",
-			backend->format->name, ret);
+			state->format->name, ret);
 		return ret;
 	}
 
-	ret = state_storage_write(&backend->storage, buf, len);
+	ret = state_storage_write(&state->storage, buf, len);
 	if (ret) {
 		dev_err(&state->dev, "Failed to write packed state, %d\n", ret);
 		goto out;
@@ -71,20 +70,19 @@ int state_load(struct state *state)
 	uint8_t *buf;
 	ssize_t len;
 	int ret;
-	struct state_backend *backend = &state->backend;
 
-	ret = state_storage_read(&backend->storage, backend->format,
+	ret = state_storage_read(&state->storage, state->format,
 				 state->magic, &buf, &len);
 	if (ret) {
 		dev_err(&state->dev, "Failed to read state with format %s, %d\n",
-			backend->format->name, ret);
+			state->format->name, ret);
 		return ret;
 	}
 
-	ret = backend->format->unpack(backend->format, state, buf, len);
+	ret = state->format->unpack(state->format, state, buf, len);
 	if (ret) {
 		dev_err(&state->dev, "Failed to unpack read data with format %s although verified, %d\n",
-			backend->format->name, ret);
+			state->format->name, ret);
 		goto out;
 	}
 
@@ -95,17 +93,17 @@ out:
 	return ret;
 }
 
-static int state_format_init(struct state_backend *backend,
+static int state_format_init(struct state *state,
 			     struct device_d *dev, const char *backend_format,
 			     struct device_node *node, const char *state_name)
 {
 	int ret;
 
 	if (!strcmp(backend_format, "raw")) {
-		ret = backend_format_raw_create(&backend->format, node,
+		ret = backend_format_raw_create(&state->format, node,
 						state_name, dev);
 	} else if (!strcmp(backend_format, "dtb")) {
-		ret = backend_format_dtb_create(&backend->format, dev);
+		ret = backend_format_dtb_create(&state->format, dev);
 	} else {
 		dev_err(dev, "Invalid backend format %s\n",
 			backend_format);
@@ -143,7 +141,7 @@ static void state_format_free(struct state_backend_format *format)
  * autoselect some backwardscompatible backend options
  * @return 0 on success, -errno otherwise
  */
-int state_backend_init(struct state_backend *backend, struct device_d *dev,
+int state_backend_init(struct state *state, struct device_d *dev,
 		       struct device_node *node, const char *backend_format,
 		       const char *storage_path, const char *state_name, const
 		       char *of_path, off_t offset, size_t max_size,
@@ -151,35 +149,35 @@ int state_backend_init(struct state_backend *backend, struct device_d *dev,
 {
 	int ret;
 
-	ret = state_format_init(backend, dev, backend_format, node, state_name);
+	ret = state_format_init(state, dev, backend_format, node, state_name);
 	if (ret)
 		return ret;
 
-	ret = state_storage_init(&backend->storage, dev, storage_path, offset,
+	ret = state_storage_init(&state->storage, dev, storage_path, offset,
 				 max_size, stridesize, storagetype);
 	if (ret)
 		goto out_free_format;
 
-	backend->of_path = xstrdup(of_path);
+	state->of_backend_path = xstrdup(of_path);
 
 	return 0;
 
 out_free_format:
-	state_format_free(backend->format);
-	backend->format = NULL;
+	state_format_free(state->format);
+	state->format = NULL;
 
 	return ret;
 }
 
-void state_backend_set_readonly(struct state_backend *backend)
+void state_backend_set_readonly(struct state *state)
 {
-	state_storage_set_readonly(&backend->storage);
+	state_storage_set_readonly(&state->storage);
 }
 
-void state_backend_free(struct state_backend *backend)
+void state_backend_free(struct state *state)
 {
-	state_storage_free(&backend->storage);
-	if (backend->format)
-		state_format_free(backend->format);
-	free(backend->of_path);
+	state_storage_free(&state->storage);
+	if (state->format)
+		state_format_free(state->format);
+	free(state->of_path);
 }
diff --git a/common/state/state.c b/common/state/state.c
index 6e6b3a6f08..62357fb248 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -328,21 +328,21 @@ static int of_state_fixup(struct device_node *root, void *ctx)
 	}
 
 	/* backend-type */
-	if (!state->backend.format) {
+	if (!state->format) {
 		ret = -ENODEV;
 		goto out;
 	}
 
 	p = of_new_property(new_node, "backend-type",
-			    state->backend.format->name,
-			    strlen(state->backend.format->name) + 1);
+			    state->format->name,
+			    strlen(state->format->name) + 1);
 	if (!p) {
 		ret = -ENOMEM;
 		goto out;
 	}
 
 	/* backend phandle */
-	backend_node = of_find_node_by_path_from(root, state->backend.of_path);
+	backend_node = of_find_node_by_path_from(root, state->of_backend_path);
 	if (!backend_node) {
 		ret = -ENODEV;
 		goto out;
@@ -353,9 +353,9 @@ static int of_state_fixup(struct device_node *root, void *ctx)
 	if (ret)
 		goto out;
 
-	if (!strcmp("raw", state->backend.format->name)) {
+	if (!strcmp("raw", state->format->name)) {
 		struct digest *digest =
-		    state_backend_format_raw_get_digest(state->backend.format);
+		    state_backend_format_raw_get_digest(state->format);
 		if (digest) {
 			p = of_new_property(new_node, "algo",
 					    digest_name(digest),
@@ -367,19 +367,19 @@ static int of_state_fixup(struct device_node *root, void *ctx)
 		}
 	}
 
-	if (state->backend.storage.name) {
+	if (state->storage.name) {
 		p = of_new_property(new_node, "backend-storage-type",
-				    state->backend.storage.name,
-				    strlen(state->backend.storage.name) + 1);
+				    state->storage.name,
+				    strlen(state->storage.name) + 1);
 		if (!p) {
 			ret = -ENOMEM;
 			goto out;
 		}
 	}
 
-	if (state->backend.storage.stridesize) {
+	if (state->storage.stridesize) {
 		ret = of_property_write_u32(new_node, "backend-stridesize",
-				state->backend.storage.stridesize);
+				state->storage.stridesize);
 		if (ret)
 			goto out;
 	}
@@ -408,7 +408,7 @@ void state_release(struct state *state)
 	of_unregister_fixup(of_state_fixup, state);
 	list_del(&state->list);
 	unregister_device(&state->dev);
-	state_backend_free(&state->backend);
+	state_backend_free(state);
 	free(state->of_path);
 	free(state);
 }
@@ -487,14 +487,14 @@ struct state *state_new_from_node(struct device_node *node, char *path,
 		dev_info(&state->dev, "No backend-storage-type found, using default.\n");
 	}
 
-	ret = state_backend_init(&state->backend, &state->dev, node,
+	ret = state_backend_init(state, &state->dev, node,
 				 backend_type, path, alias, of_path, offset,
 				 max_size, stridesize, storage_type);
 	if (ret)
 		goto out_release_state;
 
 	if (readonly)
-		state_backend_set_readonly(&state->backend);
+		state_backend_set_readonly(state);
 
 	ret = state_from_node(state, node, 1);
 	if (ret) {
@@ -569,10 +569,10 @@ void state_info(void)
 
 	list_for_each_entry(state, &state_list, list) {
 		printf("%-20s ", state->name);
-		if (state->backend.format)
+		if (state->format)
 			printf("(backend: %s, path: %s)\n",
-			       state->backend.format->name,
-			       state->backend.of_path);
+			       state->format->name,
+			       state->of_backend_path);
 		else
 			printf("(no backend)\n");
 	}
diff --git a/common/state/state.h b/common/state/state.h
index ddf45239eb..cb248fc609 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -75,19 +75,6 @@ struct state_backend_storage {
 	bool readonly;
 };
 
-/**
- * state_backend - State Backend object
- *
- * @format Backend format object
- * @storage Backend storage object
- * @of_path Path to the DT node
- */
-struct state_backend {
-	struct state_backend_format *format;
-	struct state_backend_storage storage;
-	char *of_path;
-};
-
 struct state {
 	struct list_head list; /* Entry to enqueue on list of states */
 
@@ -100,7 +87,9 @@ struct state {
 	unsigned int dirty;
 	unsigned int save_on_shutdown;
 
-	struct state_backend backend;
+	struct state_backend_format *format;
+	struct state_backend_storage storage;
+	char *of_backend_path;
 };
 
 enum state_convert {
@@ -216,13 +205,13 @@ int state_backend_bucket_cached_create(struct device_d *dev,
 struct state_variable *state_find_var(struct state *state, const char *name);
 struct digest *state_backend_format_raw_get_digest(struct state_backend_format
 						   *format);
-int state_backend_init(struct state_backend *backend, struct device_d *dev,
+int state_backend_init(struct state *state, struct device_d *dev,
 		       struct device_node *node, const char *backend_format,
 		       const char *storage_path, const char *state_name, const
 		       char *of_path, off_t offset, size_t max_size,
 		       uint32_t stridesize, const char *storagetype);
-void state_backend_set_readonly(struct state_backend *backend);
-void state_backend_free(struct state_backend *backend);
+void state_backend_set_readonly(struct state *state);
+void state_backend_free(struct state *state);
 void state_storage_free(struct state_backend_storage *storage);
 int state_backend_bucket_direct_create(struct device_d *dev, const char *path,
 				       struct state_backend_storage_bucket **bucket,
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 06/42] state: merge backend.c into state.c
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (4 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 05/42] state: Drop backend as extra struct type Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 07/42] state: open code state_backend_init in caller Sascha Hauer
                   ` (36 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

The code in backend.c is too small to justify an extra file. Merge it
into state.c.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/Makefile  |   1 -
 common/state/backend.c | 183 -------------------------------------------------
 common/state/state.c   | 161 +++++++++++++++++++++++++++++++++++++++++++
 common/state/state.h   |   6 --
 4 files changed, 161 insertions(+), 190 deletions(-)
 delete mode 100644 common/state/backend.c

diff --git a/common/state/Makefile b/common/state/Makefile
index 3e0e2c6e55..d3518feab4 100644
--- a/common/state/Makefile
+++ b/common/state/Makefile
@@ -1,6 +1,5 @@
 obj-y += state.o
 obj-y += state_variables.o
-obj-y += backend.o
 obj-y += backend_format_dtb.o
 obj-y += backend_format_raw.o
 obj-y += backend_storage.o
diff --git a/common/state/backend.c b/common/state/backend.c
deleted file mode 100644
index 1773392e8d..0000000000
--- a/common/state/backend.c
+++ /dev/null
@@ -1,183 +0,0 @@
-/*
- * Copyright (C) 2016 Pengutronix, Markus Pargmann <mpa@pengutronix.de>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- */
-
-#include <linux/kernel.h>
-#include <linux/list.h>
-#include <linux/string.h>
-#include <malloc.h>
-#include <printk.h>
-
-#include "state.h"
-
-
-/**
- * Save the state
- * @param state
- * @return
- */
-int state_save(struct state *state)
-{
-	uint8_t *buf;
-	ssize_t len;
-	int ret;
-
-	if (!state->dirty)
-		return 0;
-
-	ret = state->format->pack(state->format, state, &buf, &len);
-	if (ret) {
-		dev_err(&state->dev, "Failed to pack state with backend format %s, %d\n",
-			state->format->name, ret);
-		return ret;
-	}
-
-	ret = state_storage_write(&state->storage, buf, len);
-	if (ret) {
-		dev_err(&state->dev, "Failed to write packed state, %d\n", ret);
-		goto out;
-	}
-
-	state->dirty = 0;
-
-out:
-	free(buf);
-	return ret;
-}
-
-/**
- * state_load - Loads a state from the backend
- * @param state The state that should be updated to contain the loaded data
- * @return 0 on success, -errno on failure. If no state is loaded the previous
- * values remain in the state.
- *
- * This function uses the registered storage backend to read data. All data that
- * we read is checked for integrity by the formatter. After that we unpack the
- * data into our state.
- */
-int state_load(struct state *state)
-{
-	uint8_t *buf;
-	ssize_t len;
-	int ret;
-
-	ret = state_storage_read(&state->storage, state->format,
-				 state->magic, &buf, &len);
-	if (ret) {
-		dev_err(&state->dev, "Failed to read state with format %s, %d\n",
-			state->format->name, ret);
-		return ret;
-	}
-
-	ret = state->format->unpack(state->format, state, buf, len);
-	if (ret) {
-		dev_err(&state->dev, "Failed to unpack read data with format %s although verified, %d\n",
-			state->format->name, ret);
-		goto out;
-	}
-
-	state->dirty = 0;
-
-out:
-	free(buf);
-	return ret;
-}
-
-static int state_format_init(struct state *state,
-			     struct device_d *dev, const char *backend_format,
-			     struct device_node *node, const char *state_name)
-{
-	int ret;
-
-	if (!strcmp(backend_format, "raw")) {
-		ret = backend_format_raw_create(&state->format, node,
-						state_name, dev);
-	} else if (!strcmp(backend_format, "dtb")) {
-		ret = backend_format_dtb_create(&state->format, dev);
-	} else {
-		dev_err(dev, "Invalid backend format %s\n",
-			backend_format);
-		return -EINVAL;
-	}
-
-	if (ret && ret != -EPROBE_DEFER)
-		dev_err(dev, "Failed to initialize format %s, %d\n",
-			backend_format, ret);
-
-	return ret;
-}
-
-static void state_format_free(struct state_backend_format *format)
-{
-	if (format->free)
-		format->free(format);
-}
-
-/**
- * state_backend_init - Initiates the backend storage and format using the
- * passed arguments
- * @param backend state backend
- * @param dev Device pointer used for prints
- * @param node the DT device node corresponding to the state
- * @param backend_format a string describing the format. Valid values are 'raw'
- * and 'dtb' currently
- * @param storage_path Path to the backend storage file/device/partition/...
- * @param state_name Name of the state
- * @param of_path Path in the devicetree
- * @param stridesize stridesize in case we have a medium without eraseblocks.
- * stridesize describes how far apart copies of the same data should be stored.
- * For blockdevices it makes sense to align them on blocksize.
- * @param storagetype Type of the storage backend. This may be NULL where we
- * autoselect some backwardscompatible backend options
- * @return 0 on success, -errno otherwise
- */
-int state_backend_init(struct state *state, struct device_d *dev,
-		       struct device_node *node, const char *backend_format,
-		       const char *storage_path, const char *state_name, const
-		       char *of_path, off_t offset, size_t max_size,
-		       uint32_t stridesize, const char *storagetype)
-{
-	int ret;
-
-	ret = state_format_init(state, dev, backend_format, node, state_name);
-	if (ret)
-		return ret;
-
-	ret = state_storage_init(&state->storage, dev, storage_path, offset,
-				 max_size, stridesize, storagetype);
-	if (ret)
-		goto out_free_format;
-
-	state->of_backend_path = xstrdup(of_path);
-
-	return 0;
-
-out_free_format:
-	state_format_free(state->format);
-	state->format = NULL;
-
-	return ret;
-}
-
-void state_backend_set_readonly(struct state *state)
-{
-	state_storage_set_readonly(&state->storage);
-}
-
-void state_backend_free(struct state *state)
-{
-	state_storage_free(&state->storage);
-	if (state->format)
-		state_format_free(state->format);
-	free(state->of_path);
-}
diff --git a/common/state/state.c b/common/state/state.c
index 62357fb248..570fa498c5 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -34,6 +34,167 @@
 /* list of all registered state instances */
 static LIST_HEAD(state_list);
 
+/**
+ * Save the state
+ * @param state
+ * @return
+ */
+int state_save(struct state *state)
+{
+	uint8_t *buf;
+	ssize_t len;
+	int ret;
+
+	if (!state->dirty)
+		return 0;
+
+	ret = state->format->pack(state->format, state, &buf, &len);
+	if (ret) {
+		dev_err(&state->dev, "Failed to pack state with backend format %s, %d\n",
+			state->format->name, ret);
+		return ret;
+	}
+
+	ret = state_storage_write(&state->storage, buf, len);
+	if (ret) {
+		dev_err(&state->dev, "Failed to write packed state, %d\n", ret);
+		goto out;
+	}
+
+	state->dirty = 0;
+
+out:
+	free(buf);
+	return ret;
+}
+
+/**
+ * state_load - Loads a state from the backend
+ * @param state The state that should be updated to contain the loaded data
+ * @return 0 on success, -errno on failure. If no state is loaded the previous
+ * values remain in the state.
+ *
+ * This function uses the registered storage backend to read data. All data that
+ * we read is checked for integrity by the formatter. After that we unpack the
+ * data into our state.
+ */
+int state_load(struct state *state)
+{
+	uint8_t *buf;
+	ssize_t len;
+	int ret;
+
+	ret = state_storage_read(&state->storage, state->format,
+				 state->magic, &buf, &len);
+	if (ret) {
+		dev_err(&state->dev, "Failed to read state with format %s, %d\n",
+			state->format->name, ret);
+		return ret;
+	}
+
+	ret = state->format->unpack(state->format, state, buf, len);
+	if (ret) {
+		dev_err(&state->dev, "Failed to unpack read data with format %s although verified, %d\n",
+			state->format->name, ret);
+		goto out;
+	}
+
+	state->dirty = 0;
+
+out:
+	free(buf);
+	return ret;
+}
+
+static int state_format_init(struct state *state,
+			     struct device_d *dev, const char *backend_format,
+			     struct device_node *node, const char *state_name)
+{
+	int ret;
+
+	if (!backend_format || !strcmp(backend_format, "raw")) {
+		ret = backend_format_raw_create(&state->format, node,
+						state_name, dev);
+	} else if (!strcmp(backend_format, "dtb")) {
+		ret = backend_format_dtb_create(&state->format, dev);
+	} else {
+		dev_err(dev, "Invalid backend format %s\n",
+			backend_format);
+		return -EINVAL;
+	}
+
+	if (ret && ret != -EPROBE_DEFER)
+		dev_err(dev, "Failed to initialize format %s, %d\n",
+			backend_format, ret);
+
+	return ret;
+}
+
+static void state_format_free(struct state_backend_format *format)
+{
+	if (format->free)
+		format->free(format);
+}
+
+/**
+ * state_backend_init - Initiates the backend storage and format using the
+ * passed arguments
+ * @param backend state backend
+ * @param dev Device pointer used for prints
+ * @param node the DT device node corresponding to the state
+ * @param backend_format a string describing the format. Valid values are 'raw'
+ * and 'dtb' currently
+ * @param storage_path Path to the backend storage file/device/partition/...
+ * @param state_name Name of the state
+ * @param of_path Path in the devicetree
+ * @param stridesize stridesize in case we have a medium without eraseblocks.
+ * stridesize describes how far apart copies of the same data should be stored.
+ * For blockdevices it makes sense to align them on blocksize.
+ * @param storagetype Type of the storage backend. This may be NULL where we
+ * autoselect some backwardscompatible backend options
+ * @return 0 on success, -errno otherwise
+ */
+static int state_backend_init(struct state *state, struct device_d *dev,
+		       struct device_node *node, const char *backend_format,
+		       const char *storage_path, const char *state_name, const
+		       char *of_path, off_t offset, size_t max_size,
+		       uint32_t stridesize, const char *storagetype)
+{
+	int ret;
+
+	ret = state_format_init(state, dev, backend_format, node, state_name);
+	if (ret)
+		return ret;
+
+	ret = state_storage_init(&state->storage, dev, storage_path, offset,
+				 max_size, stridesize, storagetype);
+	if (ret)
+		goto out_free_format;
+
+	state->of_backend_path = xstrdup(of_path);
+
+	return 0;
+
+out_free_format:
+	state_format_free(state->format);
+	state->format = NULL;
+
+	return ret;
+}
+
+void state_backend_set_readonly(struct state *state)
+{
+	state_storage_set_readonly(&state->storage);
+}
+
+void state_backend_free(struct state *state)
+{
+	state_storage_free(&state->storage);
+	if (state->format)
+		state_format_free(state->format);
+	free(state->of_path);
+}
+
 static struct state *state_new(const char *name)
 {
 	struct state *state;
diff --git a/common/state/state.h b/common/state/state.h
index cb248fc609..254ff31a06 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -205,13 +205,7 @@ int state_backend_bucket_cached_create(struct device_d *dev,
 struct state_variable *state_find_var(struct state *state, const char *name);
 struct digest *state_backend_format_raw_get_digest(struct state_backend_format
 						   *format);
-int state_backend_init(struct state *state, struct device_d *dev,
-		       struct device_node *node, const char *backend_format,
-		       const char *storage_path, const char *state_name, const
-		       char *of_path, off_t offset, size_t max_size,
-		       uint32_t stridesize, const char *storagetype);
 void state_backend_set_readonly(struct state *state);
-void state_backend_free(struct state *state);
 void state_storage_free(struct state_backend_storage *storage);
 int state_backend_bucket_direct_create(struct device_d *dev, const char *path,
 				       struct state_backend_storage_bucket **bucket,
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 07/42] state: open code state_backend_init in caller
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (5 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 06/42] state: merge backend.c into state.c Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 08/42] state: remove unnecessary argument from state_format_init Sascha Hauer
                   ` (35 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

Safes a lot of argument passing to a function that is used
only once.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/state.c | 70 ++++++++++------------------------------------------
 1 file changed, 13 insertions(+), 57 deletions(-)

diff --git a/common/state/state.c b/common/state/state.c
index 570fa498c5..fce8fcc943 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -132,69 +132,18 @@ static int state_format_init(struct state *state,
 
 static void state_format_free(struct state_backend_format *format)
 {
+	if (!format)
+		return;
+
 	if (format->free)
 		format->free(format);
 }
 
-/**
- * state_backend_init - Initiates the backend storage and format using the
- * passed arguments
- * @param backend state backend
- * @param dev Device pointer used for prints
- * @param node the DT device node corresponding to the state
- * @param backend_format a string describing the format. Valid values are 'raw'
- * and 'dtb' currently
- * @param storage_path Path to the backend storage file/device/partition/...
- * @param state_name Name of the state
- * @param of_path Path in the devicetree
- * @param stridesize stridesize in case we have a medium without eraseblocks.
- * stridesize describes how far apart copies of the same data should be stored.
- * For blockdevices it makes sense to align them on blocksize.
- * @param storagetype Type of the storage backend. This may be NULL where we
- * autoselect some backwardscompatible backend options
- * @return 0 on success, -errno otherwise
- */
-static int state_backend_init(struct state *state, struct device_d *dev,
-		       struct device_node *node, const char *backend_format,
-		       const char *storage_path, const char *state_name, const
-		       char *of_path, off_t offset, size_t max_size,
-		       uint32_t stridesize, const char *storagetype)
-{
-	int ret;
-
-	ret = state_format_init(state, dev, backend_format, node, state_name);
-	if (ret)
-		return ret;
-
-	ret = state_storage_init(&state->storage, dev, storage_path, offset,
-				 max_size, stridesize, storagetype);
-	if (ret)
-		goto out_free_format;
-
-	state->of_backend_path = xstrdup(of_path);
-
-	return 0;
-
-out_free_format:
-	state_format_free(state->format);
-	state->format = NULL;
-
-	return ret;
-}
-
 void state_backend_set_readonly(struct state *state)
 {
 	state_storage_set_readonly(&state->storage);
 }
 
-void state_backend_free(struct state *state)
-{
-	state_storage_free(&state->storage);
-	if (state->format)
-		state_format_free(state->format);
-	free(state->of_path);
-}
-
 static struct state *state_new(const char *name)
 {
 	struct state *state;
@@ -569,7 +518,9 @@ void state_release(struct state *state)
 	of_unregister_fixup(of_state_fixup, state);
 	list_del(&state->list);
 	unregister_device(&state->dev);
-	state_backend_free(state);
+	state_storage_free(&state->storage);
+	state_format_free(state->format);
+	free(state->of_backend_path);
 	free(state->of_path);
 	free(state);
 }
@@ -648,12 +599,17 @@ struct state *state_new_from_node(struct device_node *node, char *path,
 		dev_info(&state->dev, "No backend-storage-type found, using default.\n");
 	}
 
-	ret = state_backend_init(state, &state->dev, node,
-				 backend_type, path, alias, of_path, offset,
+	ret = state_format_init(state, &state->dev, backend_type, node, alias);
+	if (ret)
+		goto out_release_state;
+
+	ret = state_storage_init(&state->storage, &state->dev, path, offset,
 				 max_size, stridesize, storage_type);
 	if (ret)
 		goto out_release_state;
 
+	state->of_backend_path = xstrdup(of_path);
+
 	if (readonly)
 		state_backend_set_readonly(state);
 
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 08/42] state: remove unnecessary argument from state_format_init
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (6 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 07/42] state: open code state_backend_init in caller Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 09/42] state: pass struct state * to storage functions Sascha Hauer
                   ` (34 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

The device pointer is already in struct state, no need to pass it
around when a struct state * is already passed.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/state.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/common/state/state.c b/common/state/state.c
index fce8fcc943..87b704feba 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -106,25 +106,24 @@ out:
 	return ret;
 }
 
-static int state_format_init(struct state *state,
-			     struct device_d *dev, const char *backend_format,
+static int state_format_init(struct state *state, const char *backend_format,
 			     struct device_node *node, const char *state_name)
 {
 	int ret;
 
 	if (!backend_format || !strcmp(backend_format, "raw")) {
 		ret = backend_format_raw_create(&state->format, node,
-						state_name, dev);
+						state_name, &state->dev);
 	} else if (!strcmp(backend_format, "dtb")) {
-		ret = backend_format_dtb_create(&state->format, dev);
+		ret = backend_format_dtb_create(&state->format, &state->dev);
 	} else {
-		dev_err(dev, "Invalid backend format %s\n",
+		dev_err(&state->dev, "Invalid backend format %s\n",
 			backend_format);
 		return -EINVAL;
 	}
 
 	if (ret && ret != -EPROBE_DEFER)
-		dev_err(dev, "Failed to initialize format %s, %d\n",
+		dev_err(&state->dev, "Failed to initialize format %s, %d\n",
 			backend_format, ret);
 
 	return ret;
@@ -599,7 +598,7 @@ struct state *state_new_from_node(struct device_node *node, char *path,
 		dev_info(&state->dev, "No backend-storage-type found, using default.\n");
 	}
 
-	ret = state_format_init(state, &state->dev, backend_type, node, alias);
+	ret = state_format_init(state, backend_type, node, alias);
 	if (ret)
 		goto out_release_state;
 
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 09/42] state: pass struct state * to storage functions
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (7 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 08/42] state: remove unnecessary argument from state_format_init Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 10/42] state: storage: initialize variable once outside loop Sascha Hauer
                   ` (33 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

We can get a state_backend_storage * and the device * from struct state,
so pass this to the storage functions rather than the two pointers.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_storage.c | 7 +++----
 common/state/state.c           | 2 +-
 common/state/state.h           | 3 +--
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 5481f27df9..e1869830bd 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -454,7 +454,6 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
 
 /**
  * state_storage_init - Init backend storage
- * @param storage Storage object
  * @param path Path to the backend storage file
  * @param dev_offset Offset in the device to start writing at.
  * @param max_size Maximum size of the data. May be 0 for infinite.
@@ -465,16 +464,16 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
  *
  * Depending on the filetype, we create mtd buckets or normal file buckets.
  */
-int state_storage_init(struct state_backend_storage *storage,
-		       struct device_d *dev, const char *path,
+int state_storage_init(struct state *state, const char *path,
 		       off_t offset, size_t max_size, uint32_t stridesize,
 		       const char *storagetype)
 {
+	struct state_backend_storage *storage = &state->storage;
 	int ret = -ENODEV;
 	struct mtd_info_user meminfo;
 
 	INIT_LIST_HEAD(&storage->buckets);
-	storage->dev = dev;
+	storage->dev = &state->dev;
 	storage->name = storagetype;
 	storage->stridesize = stridesize;
 
diff --git a/common/state/state.c b/common/state/state.c
index 87b704feba..8d2dca2523 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -602,7 +602,7 @@ struct state *state_new_from_node(struct device_node *node, char *path,
 	if (ret)
 		goto out_release_state;
 
-	ret = state_storage_init(&state->storage, &state->dev, path, offset,
+	ret = state_storage_init(state, path, offset,
 				 max_size, stridesize, storage_type);
 	if (ret)
 		goto out_release_state;
diff --git a/common/state/state.h b/common/state/state.h
index 254ff31a06..4ef46693c6 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -186,8 +186,7 @@ int backend_format_raw_create(struct state_backend_format **format,
 			      struct device_d *dev);
 int backend_format_dtb_create(struct state_backend_format **format,
 			      struct device_d *dev);
-int state_storage_init(struct state_backend_storage *storage,
-		       struct device_d *dev, const char *path,
+int state_storage_init(struct state *state, const char *path,
 		       off_t offset, size_t max_size, uint32_t stridesize,
 		       const char *storagetype);
 void state_storage_set_readonly(struct state_backend_storage *storage);
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 10/42] state: storage: initialize variable once outside loop
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (8 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 09/42] state: pass struct state * to storage functions Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 11/42] state: backend_circular: Read whole PEB Sascha Hauer
                   ` (32 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

writesize is initialized with the same value in each loop iteration,
Instead, initialize it once outside the loop.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_storage.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index e1869830bd..0808c5c0b4 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -268,6 +268,7 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 	ssize_t end = dev_offset + max_size;
 	int nr_copies = 0;
 	off_t offset;
+	ssize_t writesize;
 
 	if (!end || end > meminfo->size)
 		end = meminfo->size;
@@ -278,15 +279,16 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 		return -EINVAL;
 	}
 
+	if (circular)
+		writesize = meminfo->writesize;
+	else
+		writesize = meminfo->erasesize;
+
 	for (offset = dev_offset; offset < end; offset += meminfo->erasesize) {
 		int ret;
-		ssize_t writesize = meminfo->writesize;
 		unsigned int eraseblock = offset / meminfo->erasesize;
 		bool lazy_init = true;
 
-		if (!circular)
-			writesize = meminfo->erasesize;
-
 		ret = state_backend_bucket_circular_create(storage->dev, path,
 							   &bucket,
 							   eraseblock,
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 11/42] state: backend_circular: Read whole PEB
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (9 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 10/42] state: storage: initialize variable once outside loop Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-04-15  8:40   ` Sam Ravnborg
  2017-03-31  7:03 ` [PATCH 12/42] state: drop lazy_init Sascha Hauer
                   ` (31 subsequent siblings)
  42 siblings, 1 reply; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

When the circular backend searches for the last page written in the
eraseblock, it iterates backwards pagewise from the end of the block.
This is ok for NAND flash, but on NOR flash, which does not have pages,
the code ends up iterating bytewise backwards, calling into mtd each
time. This is very time consuming, so optimize this by reading the whole
eraseblock once and just iterate over the buffer in memory.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_bucket_circular.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/common/state/backend_bucket_circular.c b/common/state/backend_bucket_circular.c
index 0bce900d0d..53c2aae803 100644
--- a/common/state/backend_bucket_circular.c
+++ b/common/state/backend_bucket_circular.c
@@ -379,26 +379,24 @@ static int state_backend_bucket_circular_init(
 	int sub_offset;
 	uint32_t written_length = 0;
 	uint8_t *buf;
+	int ret;
 
-	buf = xmalloc(circ->writesize);
+	buf = xmalloc(circ->max_size);
 	if (!buf)
 		return -ENOMEM;
 
+	ret = state_mtd_peb_read(circ, buf, 0, circ->max_size);
+	if (ret && ret != -EUCLEAN)
+		return ret;
+
 	for (sub_offset = circ->max_size - circ->writesize; sub_offset >= 0;
 	     sub_offset -= circ->writesize) {
-		int ret;
-
-		ret = state_mtd_peb_read(circ, buf, sub_offset,
-					 circ->writesize);
-		if (ret && ret != -EUCLEAN)
-			return ret;
-
-		ret = mtd_buf_all_ff(buf, circ->writesize);
+		ret = mtd_buf_all_ff(buf + sub_offset, circ->writesize);
 		if (!ret) {
 			struct state_backend_storage_bucket_circular_meta *meta;
 
 			meta = (struct state_backend_storage_bucket_circular_meta *)
-					(buf + circ->writesize - sizeof(*meta));
+					(buf + sub_offset + circ->writesize - sizeof(*meta));
 
 			if (meta->magic != circular_magic)
 				written_length = 0;
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 12/42] state: drop lazy_init
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (10 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 11/42] state: backend_circular: Read whole PEB Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 13/42] state: simplify direct backend Sascha Hauer
                   ` (30 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

lazy_init is an optimization that makes it possible to read only up to
the first valid bucket when starting. However, when restoring consistency,
immediately afterwards we have we have to initialize all buckets anyway,
so being lazy doesn't give us any gain. Remove it to simplify the code.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_bucket_cached.c   | 14 --------------
 common/state/backend_bucket_circular.c | 13 ++++---------
 common/state/backend_storage.c         | 34 +---------------------------------
 common/state/state.h                   |  5 +----
 4 files changed, 6 insertions(+), 60 deletions(-)

diff --git a/common/state/backend_bucket_cached.c b/common/state/backend_bucket_cached.c
index ba0af7f373..5e7f44c8f8 100644
--- a/common/state/backend_bucket_cached.c
+++ b/common/state/backend_bucket_cached.c
@@ -110,19 +110,6 @@ static int state_backend_bucket_cache_write(struct state_backend_storage_bucket
 	return 0;
 }
 
-static int state_backend_bucket_cache_init(
-		struct state_backend_storage_bucket *bucket)
-{
-	struct state_backend_storage_bucket_cache *cache =
-			get_bucket_cache(bucket);
-
-	if (cache->raw->init) {
-		return cache->raw->init(cache->raw);
-	}
-
-	return 0;
-}
-
 static void state_backend_bucket_cache_free(
 		struct state_backend_storage_bucket *bucket)
 {
@@ -147,7 +134,6 @@ int state_backend_bucket_cached_create(struct device_d *dev,
 	cache->bucket.free = state_backend_bucket_cache_free;
 	cache->bucket.read = state_backend_bucket_cache_read;
 	cache->bucket.write = state_backend_bucket_cache_write;
-	cache->bucket.init = state_backend_bucket_cache_init;
 
 	*out = &cache->bucket;
 
diff --git a/common/state/backend_bucket_circular.c b/common/state/backend_bucket_circular.c
index 53c2aae803..8eae86694a 100644
--- a/common/state/backend_bucket_circular.c
+++ b/common/state/backend_bucket_circular.c
@@ -456,8 +456,7 @@ int state_backend_bucket_circular_create(struct device_d *dev, const char *path,
 					 struct state_backend_storage_bucket **bucket,
 					 unsigned int eraseblock,
 					 ssize_t writesize,
-					 struct mtd_info_user *mtd_uinfo,
-					 bool lazy_init)
+					 struct mtd_info_user *mtd_uinfo)
 {
 	struct state_backend_storage_bucket_circular *circ;
 	int ret;
@@ -493,13 +492,9 @@ int state_backend_bucket_circular_create(struct device_d *dev, const char *path,
 	circ->bucket.free = state_backend_bucket_circular_free;
 	*bucket = &circ->bucket;
 
-	if (!lazy_init) {
-		ret = state_backend_bucket_circular_init(*bucket);
-		if (ret)
-			goto out_free;
-	} else {
-		circ->bucket.init = state_backend_bucket_circular_init;
-	}
+	ret = state_backend_bucket_circular_init(*bucket);
+	if (ret)
+		goto out_free;
 
 	return 0;
 
diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 0808c5c0b4..04d9d8f0a7 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -27,23 +27,6 @@
 
 const unsigned int min_copies_written = 1;
 
-static int bucket_lazy_init(struct state_backend_storage_bucket *bucket)
-{
-	int ret;
-
-	if (bucket->initialized)
-		return 0;
-
-	if (bucket->init) {
-		ret = bucket->init(bucket);
-		if (ret)
-			return ret;
-	}
-	bucket->initialized = true;
-
-	return 0;
-}
-
 /**
  * state_storage_write - Writes the given data to the storage
  * @param storage Storage object
@@ -69,13 +52,6 @@ int state_storage_write(struct state_backend_storage *storage,
 		return 0;
 
 	list_for_each_entry(bucket, &storage->buckets, bucket_list) {
-		ret = bucket_lazy_init(bucket);
-		if (ret) {
-			dev_warn(storage->dev, "Failed to init bucket/write state backend bucket, %d\n",
-				 ret);
-			continue;
-		}
-
 		ret = bucket->write(bucket, buf, len);
 		if (ret) {
 			dev_warn(storage->dev, "Failed to write state backend bucket, %d\n",
@@ -136,12 +112,6 @@ int state_storage_read(struct state_backend_storage *storage,
 
 	list_for_each_entry(bucket, &storage->buckets, bucket_list) {
 		*len = 0;
-		ret = bucket_lazy_init(bucket);
-		if (ret) {
-			dev_warn(storage->dev, "Failed to init bucket/read state backend bucket, %d\n",
-				 ret);
-			continue;
-		}
 
 		ret = bucket->read(bucket, buf, len);
 		if (ret) {
@@ -287,14 +257,12 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 	for (offset = dev_offset; offset < end; offset += meminfo->erasesize) {
 		int ret;
 		unsigned int eraseblock = offset / meminfo->erasesize;
-		bool lazy_init = true;
 
 		ret = state_backend_bucket_circular_create(storage->dev, path,
 							   &bucket,
 							   eraseblock,
 							   writesize,
-							   meminfo,
-							   lazy_init);
+							   meminfo);
 		if (ret) {
 			dev_warn(storage->dev, "Failed to create bucket at '%s' eraseblock %u\n",
 				 path, eraseblock);
diff --git a/common/state/state.h b/common/state/state.h
index 4ef46693c6..6f5de31dff 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -20,14 +20,12 @@ struct mtd_info_user;
  * @bucket_list A list element struct to attach this bucket to a list
  */
 struct state_backend_storage_bucket {
-	int (*init) (struct state_backend_storage_bucket * bucket);
 	int (*write) (struct state_backend_storage_bucket * bucket,
 		      const uint8_t * buf, ssize_t len);
 	int (*read) (struct state_backend_storage_bucket * bucket,
 		     uint8_t ** buf, ssize_t * len_hint);
 	void (*free) (struct state_backend_storage_bucket * bucket);
 
-	bool initialized;
 	struct list_head bucket_list;
 };
 
@@ -196,8 +194,7 @@ int state_backend_bucket_circular_create(struct device_d *dev, const char *path,
 					 struct state_backend_storage_bucket **bucket,
 					 unsigned int eraseblock,
 					 ssize_t writesize,
-					 struct mtd_info_user *mtd_uinfo,
-					 bool lazy_init);
+					 struct mtd_info_user *mtd_uinfo);
 int state_backend_bucket_cached_create(struct device_d *dev,
 				       struct state_backend_storage_bucket *raw,
 				       struct state_backend_storage_bucket **out);
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 13/42] state: simplify direct backend
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (11 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 12/42] state: drop lazy_init Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 14/42] state: replace len_hint logic Sascha Hauer
                   ` (29 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

- drop support for regular files. This, if at all, is only useful for
  debugging. For the debugging case still a file of sufficient size
  can be created manually.
- make stridesize mandatory. Makes the code simpler.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_storage.c | 145 ++++-------------------------------------
 1 file changed, 14 insertions(+), 131 deletions(-)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 04d9d8f0a7..52f4aedee7 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -156,60 +156,6 @@ static int mtd_get_meminfo(const char *path, struct mtd_info_user *meminfo)
 	return ret;
 }
 
-#ifdef __BAREBOX__
-#define STAT_GIVES_SIZE(s) (S_ISREG(s.st_mode) || S_ISCHR(s.st_mode))
-#define BLKGET_GIVES_SIZE(s) 0
-#else
-#define STAT_GIVES_SIZE(s) (S_ISREG(s.st_mode))
-#define BLKGET_GIVES_SIZE(s) (S_ISBLK(s.st_mode))
-#endif
-#ifndef BLKGETSIZE64
-#define BLKGETSIZE64 -1
-#endif
-
-static int state_backend_storage_get_size(const char *path, size_t * out_size)
-{
-	struct mtd_info_user meminfo;
-	struct stat s;
-	int ret;
-
-	ret = stat(path, &s);
-	if (ret)
-		return -errno;
-
-	/*
-	 * under Linux, stat() gives the size only on regular files
-	 * under barebox, it works on char dev, too
-	 */
-	if (STAT_GIVES_SIZE(s)) {
-		*out_size = s.st_size;
-		return 0;
-	}
-
-	/* this works under Linux on block devs */
-	if (BLKGET_GIVES_SIZE(s)) {
-		int fd;
-
-		fd = open(path, O_RDONLY);
-		if (fd < 0)
-			return -errno;
-
-		ret = ioctl(fd, BLKGETSIZE64, out_size);
-		close(fd);
-		if (!ret)
-			return 0;
-	}
-
-	/* try mtd next */
-	ret = mtd_get_meminfo(path, &meminfo);
-	if (!ret) {
-		*out_size = meminfo.size;
-		return 0;
-	}
-
-	return ret;
-}
-
 /* Number of copies that should be allocated */
 const int desired_copies = 3;
 
@@ -292,41 +238,6 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 	return 0;
 }
 
-static int state_storage_file_create(struct device_d *dev, const char *path,
-				     size_t fd_size)
-{
-	int fd;
-	uint8_t *buf;
-	int ret;
-
-	fd = open(path, O_RDWR | O_CREAT, 0600);
-	if (fd < 0) {
-		dev_err(dev, "Failed to open/create file '%s', %d\n", path,
-			-errno);
-		return -errno;
-	}
-
-	buf = xzalloc(fd_size);
-	if (!buf) {
-		ret = -ENOMEM;
-		goto out_close;
-	}
-
-	ret = write_full(fd, buf, fd_size);
-	if (ret < 0) {
-		dev_err(dev, "Failed to initialize empty file '%s', %d\n", path,
-			ret);
-		goto out_free;
-	}
-	ret = 0;
-
-out_free:
-	free(buf);
-out_close:
-	close(fd);
-	return ret;
-}
-
 /**
  * state_storage_file_buckets_init - Create buckets for a conventional file descriptor
  * @param storage Storage object
@@ -345,53 +256,25 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
 					   size_t max_size, uint32_t stridesize)
 {
 	struct state_backend_storage_bucket *bucket;
-	size_t fd_size = 0;
-	int ret;
+	int ret, n;
 	off_t offset;
 	int nr_copies = 0;
 
-	ret = state_backend_storage_get_size(path, &fd_size);
-	if (ret) {
-		if (ret != -ENOENT) {
-			dev_err(storage->dev, "Failed to get the filesize of '%s', %d\n",
-				path, ret);
-			return ret;
-		}
-		if (!stridesize) {
-			dev_err(storage->dev, "File '%s' does not exist and no information about the needed size. Please specify stridesize\n",
-				path);
-			return ret;
-		}
-
-		if (max_size)
-			fd_size = min(dev_offset + stridesize * desired_copies,
-				      dev_offset + max_size);
-		else
-			fd_size = dev_offset + stridesize * desired_copies;
-		dev_info(storage->dev, "File '%s' does not exist, creating file of size %zd\n",
-			 path, fd_size);
-		ret = state_storage_file_create(storage->dev, path, fd_size);
-		if (ret) {
-			dev_info(storage->dev, "Failed to create file '%s', %d\n",
-				 path, ret);
-			return ret;
-		}
-	} else if (max_size) {
-		fd_size = min(fd_size, (size_t)dev_offset + max_size);
-	}
-
 	if (!stridesize) {
-		dev_warn(storage->dev, "WARNING, no stridesize given although we use a direct file write. Starting in degraded mode\n");
-		stridesize = fd_size;
+		dev_err(storage->dev, "stridesize unspecified\n");
+		return -EINVAL;
 	}
 
-	for (offset = dev_offset; offset < fd_size; offset += stridesize) {
-		size_t maxsize = min((size_t)stridesize,
-				     (size_t)(fd_size - offset));
+	if (max_size && max_size < desired_copies * stridesize) {
+		dev_err(storage->dev, "device is too small to hold %d copies\n", desired_copies);
+		return -EINVAL;
+	}
 
+	for (n = 0; n < desired_copies; n++) {
+		offset = dev_offset + n * stridesize;
 		ret = state_backend_bucket_direct_create(storage->dev, path,
 							 &bucket, offset,
-							 maxsize);
+							 stridesize);
 		if (ret) {
 			dev_warn(storage->dev, "Failed to create direct bucket at '%s' offset %ld\n",
 				 path, offset);
@@ -407,16 +290,16 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
 
 		list_add_tail(&bucket->bucket_list, &storage->buckets);
 		++nr_copies;
-		if (nr_copies >= desired_copies)
-			return 0;
 	}
 
 	if (!nr_copies) {
 		dev_err(storage->dev, "Failed to initialize any state direct storage bucket\n");
 		return -EIO;
 	}
-	dev_warn(storage->dev, "Failed to initialize desired amount of direct buckets, only %d of %d succeeded\n",
-		 nr_copies, desired_copies);
+
+	if (nr_copies < desired_copies)
+		dev_warn(storage->dev, "Failed to initialize desired amount of direct buckets, only %d of %d succeeded\n",
+			nr_copies, desired_copies);
 
 	return 0;
 }
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 14/42] state: replace len_hint logic
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (12 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 13/42] state: simplify direct backend Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 15/42] state: Convert all bufs to void * Sascha Hauer
                   ` (28 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

The len_hint mechanism is rather hard to understand as it's not clear
from where to where the hint is passed and also it's not clear what
happens if the hint is empty or wrong.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_bucket_circular.c |  4 ++--
 common/state/backend_bucket_direct.c   | 11 +++--------
 common/state/backend_format_dtb.c      |  7 +++++--
 common/state/backend_format_raw.c      |  5 ++++-
 common/state/backend_storage.c         |  9 ++++++---
 common/state/state.h                   |  2 +-
 6 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/common/state/backend_bucket_circular.c b/common/state/backend_bucket_circular.c
index 8eae86694a..0b8286f9cc 100644
--- a/common/state/backend_bucket_circular.c
+++ b/common/state/backend_bucket_circular.c
@@ -227,7 +227,7 @@ static int state_mtd_peb_erase(struct state_backend_storage_bucket_circular *cir
 
 static int state_backend_bucket_circular_read(struct state_backend_storage_bucket *bucket,
 					      uint8_t ** buf_out,
-					      ssize_t * len_hint)
+					      ssize_t * len_out)
 {
 	struct state_backend_storage_bucket_circular *circ =
 	    get_bucket_circular(bucket);
@@ -282,7 +282,7 @@ static int state_backend_bucket_circular_read(struct state_backend_storage_bucke
 	}
 
 	*buf_out = buf;
-	*len_hint = read_len - sizeof(struct state_backend_storage_bucket_circular_meta);
+	*len_out = read_len - sizeof(struct state_backend_storage_bucket_circular_meta);
 
 	return ret;
 }
diff --git a/common/state/backend_bucket_direct.c b/common/state/backend_bucket_direct.c
index 5225433ec5..06a5433c45 100644
--- a/common/state/backend_bucket_direct.c
+++ b/common/state/backend_bucket_direct.c
@@ -47,7 +47,7 @@ static inline struct state_backend_storage_bucket_direct
 
 static int state_backend_bucket_direct_read(struct state_backend_storage_bucket
 					    *bucket, uint8_t ** buf_out,
-					    ssize_t * len_hint)
+					    ssize_t * len_out)
 {
 	struct state_backend_storage_bucket_direct *direct =
 	    get_bucket_direct(bucket);
@@ -69,18 +69,13 @@ static int state_backend_bucket_direct_read(struct state_backend_storage_bucket
 	if (meta.magic == direct_magic) {
 		read_len = meta.written_length;
 	} else {
-		if (*len_hint)
-			read_len = *len_hint;
-		else
-			read_len = direct->max_size;
+		read_len = direct->max_size;
 		ret = lseek(direct->fd, direct->offset, SEEK_SET);
 		if (ret < 0) {
 			dev_err(direct->dev, "Failed to seek file, %d\n", ret);
 			return ret;
 		}
 	}
-	if (direct->max_size)
-		read_len = min(read_len, direct->max_size);
 
 	buf = xmalloc(read_len);
 	if (!buf)
@@ -94,7 +89,7 @@ static int state_backend_bucket_direct_read(struct state_backend_storage_bucket
 	}
 
 	*buf_out = buf;
-	*len_hint = read_len;
+	*len_out = read_len;
 
 	return 0;
 }
diff --git a/common/state/backend_format_dtb.c b/common/state/backend_format_dtb.c
index dc19c888e5..abf8921dc4 100644
--- a/common/state/backend_format_dtb.c
+++ b/common/state/backend_format_dtb.c
@@ -40,12 +40,13 @@ static inline struct state_backend_format_dtb *get_format_dtb(struct
 
 static int state_backend_format_dtb_verify(struct state_backend_format *format,
 					   uint32_t magic, const uint8_t * buf,
-					   ssize_t len)
+					   ssize_t *lenp)
 {
 	struct state_backend_format_dtb *fdtb = get_format_dtb(format);
 	struct device_node *root;
 	struct fdt_header *fdt = (struct fdt_header *)buf;
 	size_t dtb_len = fdt32_to_cpu(fdt->totalsize);
+	size_t len = *lenp;
 
 	if (dtb_len > len) {
 		dev_err(fdtb->dev, "Error, stored DTB length (%d) longer than read buffer (%d)\n",
@@ -67,6 +68,8 @@ static int state_backend_format_dtb_verify(struct state_backend_format *format,
 
 	fdtb->root = root;
 
+	*lenp = be32_to_cpu(fdt->totalsize);
+
 	return 0;
 }
 
@@ -78,7 +81,7 @@ static int state_backend_format_dtb_unpack(struct state_backend_format *format,
 	int ret;
 
 	if (!fdtb->root) {
-		state_backend_format_dtb_verify(format, 0, buf, len);
+		state_backend_format_dtb_verify(format, 0, buf, &len);
 	}
 
 	ret = state_from_node(state, fdtb->root, 0);
diff --git a/common/state/backend_format_raw.c b/common/state/backend_format_raw.c
index e028ea616c..3c8956f8ef 100644
--- a/common/state/backend_format_raw.c
+++ b/common/state/backend_format_raw.c
@@ -55,13 +55,14 @@ static inline struct state_backend_format_raw *get_format_raw(
 
 static int backend_format_raw_verify(struct state_backend_format *format,
 				     uint32_t magic, const uint8_t * buf,
-				     ssize_t len)
+				     ssize_t *lenp)
 {
 	uint32_t crc;
 	struct backend_raw_header *header;
 	int d_len = 0;
 	int ret;
 	const uint8_t *data;
+	ssize_t len = *lenp;
 	struct state_backend_format_raw *backend_raw = get_format_raw(format);
 	ssize_t complete_len;
 
@@ -105,6 +106,8 @@ static int backend_format_raw_verify(struct state_backend_format *format,
 		return -EINVAL;
 	}
 
+	*lenp = header->data_len + sizeof(*header);
+
 	if (backend_raw->digest) {
 		struct digest *d = backend_raw->digest;
 		const void *hmac = data + header->data_len;
diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 52f4aedee7..f1b3f5a6b2 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -111,15 +111,18 @@ int state_storage_read(struct state_backend_storage *storage,
 	int ret;
 
 	list_for_each_entry(bucket, &storage->buckets, bucket_list) {
-		*len = 0;
-
 		ret = bucket->read(bucket, buf, len);
 		if (ret) {
 			dev_warn(storage->dev, "Failed to read from state backend bucket, trying next, %d\n",
 				 ret);
 			continue;
 		}
-		ret = format->verify(format, magic, *buf, *len);
+
+		/*
+		 * Verify the buffer crcs. The buffer length is passed in the len argument,
+		 * .verify overwrites it with the length actually used.
+		 */
+		ret = format->verify(format, magic, *buf, len);
 		if (!ret) {
 			goto found;
 		}
diff --git a/common/state/state.h b/common/state/state.h
index 6f5de31dff..0b1a7e5ec2 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -46,7 +46,7 @@ struct state_backend_storage_bucket {
  */
 struct state_backend_format {
 	int (*verify) (struct state_backend_format * format, uint32_t magic,
-		       const uint8_t * buf, ssize_t len);
+		       const uint8_t * buf, ssize_t *lenp);
 	int (*pack) (struct state_backend_format * format, struct state * state,
 		     uint8_t ** buf, ssize_t * len);
 	int (*unpack) (struct state_backend_format * format,
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 15/42] state: Convert all bufs to void *
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (13 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 14/42] state: replace len_hint logic Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 16/42] state: Drop cache bucket Sascha Hauer
                   ` (27 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

A void * is a much better type for a buffer than a u8 * as it
can be casted to any other type implicitly. Convert all buffers
used by the state framework to void *.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_bucket_cached.c   |  6 +++---
 common/state/backend_bucket_circular.c | 16 ++++++++--------
 common/state/backend_bucket_direct.c   |  6 +++---
 common/state/backend_format_dtb.c      |  6 +++---
 common/state/backend_format_raw.c      | 10 +++++-----
 common/state/backend_storage.c         |  6 +++---
 common/state/state.c                   |  4 ++--
 common/state/state.h                   | 16 ++++++++--------
 8 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/common/state/backend_bucket_cached.c b/common/state/backend_bucket_cached.c
index 5e7f44c8f8..f8a7785387 100644
--- a/common/state/backend_bucket_cached.c
+++ b/common/state/backend_bucket_cached.c
@@ -20,7 +20,7 @@ struct state_backend_storage_bucket_cache {
 
 	struct state_backend_storage_bucket *raw;
 
-	u8 *data;
+	void *data;
 	ssize_t data_len;
 	bool force_write;
 
@@ -61,7 +61,7 @@ static int state_backend_bucket_cache_fill(
 }
 
 static int state_backend_bucket_cache_read(struct state_backend_storage_bucket *bucket,
-					   uint8_t ** buf_out,
+					   void ** buf_out,
 					   ssize_t * len_hint)
 {
 	struct state_backend_storage_bucket_cache *cache =
@@ -85,7 +85,7 @@ static int state_backend_bucket_cache_read(struct state_backend_storage_bucket *
 }
 
 static int state_backend_bucket_cache_write(struct state_backend_storage_bucket *bucket,
-					    const uint8_t * buf, ssize_t len)
+					    const void * buf, ssize_t len)
 {
 	struct state_backend_storage_bucket_cache *cache =
 			get_bucket_cache(bucket);
diff --git a/common/state/backend_bucket_circular.c b/common/state/backend_bucket_circular.c
index 0b8286f9cc..53ee0c3f57 100644
--- a/common/state/backend_bucket_circular.c
+++ b/common/state/backend_bucket_circular.c
@@ -65,7 +65,7 @@ static inline struct state_backend_storage_bucket_circular
 
 #ifdef __BAREBOX__
 static int state_mtd_peb_read(struct state_backend_storage_bucket_circular *circ,
-			      char *buf, int offset, int len)
+			      void *buf, int offset, int len)
 {
 	int ret;
 
@@ -102,7 +102,7 @@ static int state_mtd_peb_read(struct state_backend_storage_bucket_circular *circ
 }
 
 static int state_mtd_peb_write(struct state_backend_storage_bucket_circular *circ,
-			       const char *buf, int offset, int len)
+			       const void *buf, int offset, int len)
 {
 	int ret;
 
@@ -133,7 +133,7 @@ static int state_mtd_peb_erase(struct state_backend_storage_bucket_circular *cir
 }
 #else
 static int state_mtd_peb_read(struct state_backend_storage_bucket_circular *circ,
-			      char *buf, int suboffset, int len)
+			      void *buf, int suboffset, int len)
 {
 	int ret;
 	off_t offset = suboffset;
@@ -185,7 +185,7 @@ static int state_mtd_peb_read(struct state_backend_storage_bucket_circular *circ
 }
 
 static int state_mtd_peb_write(struct state_backend_storage_bucket_circular *circ,
-			       const char *buf, int suboffset, int len)
+			       const void *buf, int suboffset, int len)
 {
 	int ret;
 	off_t offset = suboffset;
@@ -226,14 +226,14 @@ static int state_mtd_peb_erase(struct state_backend_storage_bucket_circular *cir
 #endif
 
 static int state_backend_bucket_circular_read(struct state_backend_storage_bucket *bucket,
-					      uint8_t ** buf_out,
+					      void ** buf_out,
 					      ssize_t * len_out)
 {
 	struct state_backend_storage_bucket_circular *circ =
 	    get_bucket_circular(bucket);
 	ssize_t read_len;
 	off_t offset;
-	uint8_t *buf;
+	void *buf;
 	int ret;
 
 	/* Storage is empty */
@@ -288,7 +288,7 @@ static int state_backend_bucket_circular_read(struct state_backend_storage_bucke
 }
 
 static int state_backend_bucket_circular_write(struct state_backend_storage_bucket *bucket,
-					       const uint8_t * buf,
+					       const void * buf,
 					       ssize_t len)
 {
 	struct state_backend_storage_bucket_circular *circ =
@@ -297,7 +297,7 @@ static int state_backend_bucket_circular_write(struct state_backend_storage_buck
 	struct state_backend_storage_bucket_circular_meta *meta;
 	uint32_t written_length = ALIGN(len + sizeof(*meta), circ->writesize);
 	int ret;
-	uint8_t *write_buf;
+	void *write_buf;
 
 	if (written_length > circ->max_size) {
 		dev_err(circ->dev, "Error, state data too big to be written, to write: %zd, writesize: %zd, length: %zd, available: %zd\n",
diff --git a/common/state/backend_bucket_direct.c b/common/state/backend_bucket_direct.c
index 06a5433c45..246cb499c4 100644
--- a/common/state/backend_bucket_direct.c
+++ b/common/state/backend_bucket_direct.c
@@ -46,14 +46,14 @@ static inline struct state_backend_storage_bucket_direct
 }
 
 static int state_backend_bucket_direct_read(struct state_backend_storage_bucket
-					    *bucket, uint8_t ** buf_out,
+					    *bucket, void ** buf_out,
 					    ssize_t * len_out)
 {
 	struct state_backend_storage_bucket_direct *direct =
 	    get_bucket_direct(bucket);
 	struct state_backend_storage_bucket_direct_meta meta;
 	ssize_t read_len;
-	uint8_t *buf;
+	void *buf;
 	int ret;
 
 	ret = lseek(direct->fd, direct->offset, SEEK_SET);
@@ -95,7 +95,7 @@ static int state_backend_bucket_direct_read(struct state_backend_storage_bucket
 }
 
 static int state_backend_bucket_direct_write(struct state_backend_storage_bucket
-					     *bucket, const uint8_t * buf,
+					     *bucket, const void * buf,
 					     ssize_t len)
 {
 	struct state_backend_storage_bucket_direct *direct =
diff --git a/common/state/backend_format_dtb.c b/common/state/backend_format_dtb.c
index abf8921dc4..d7b01729bc 100644
--- a/common/state/backend_format_dtb.c
+++ b/common/state/backend_format_dtb.c
@@ -39,7 +39,7 @@ static inline struct state_backend_format_dtb *get_format_dtb(struct
 }
 
 static int state_backend_format_dtb_verify(struct state_backend_format *format,
-					   uint32_t magic, const uint8_t * buf,
+					   uint32_t magic, const void * buf,
 					   ssize_t *lenp)
 {
 	struct state_backend_format_dtb *fdtb = get_format_dtb(format);
@@ -75,7 +75,7 @@ static int state_backend_format_dtb_verify(struct state_backend_format *format,
 
 static int state_backend_format_dtb_unpack(struct state_backend_format *format,
 					   struct state *state,
-					   const uint8_t * buf, ssize_t len)
+					   const void * buf, ssize_t len)
 {
 	struct state_backend_format_dtb *fdtb = get_format_dtb(format);
 	int ret;
@@ -92,7 +92,7 @@ static int state_backend_format_dtb_unpack(struct state_backend_format *format,
 }
 
 static int state_backend_format_dtb_pack(struct state_backend_format *format,
-					 struct state *state, uint8_t ** buf,
+					 struct state *state, void ** buf,
 					 ssize_t * len)
 {
 	struct state_backend_format_dtb *fdtb = get_format_dtb(format);
diff --git a/common/state/backend_format_raw.c b/common/state/backend_format_raw.c
index 3c8956f8ef..a425b636d0 100644
--- a/common/state/backend_format_raw.c
+++ b/common/state/backend_format_raw.c
@@ -54,14 +54,14 @@ static inline struct state_backend_format_raw *get_format_raw(
 }
 
 static int backend_format_raw_verify(struct state_backend_format *format,
-				     uint32_t magic, const uint8_t * buf,
+				     uint32_t magic, const void * buf,
 				     ssize_t *lenp)
 {
 	uint32_t crc;
 	struct backend_raw_header *header;
 	int d_len = 0;
 	int ret;
-	const uint8_t *data;
+	const void *data;
 	ssize_t len = *lenp;
 	struct state_backend_format_raw *backend_raw = get_format_raw(format);
 	ssize_t complete_len;
@@ -139,12 +139,12 @@ static int backend_format_raw_verify(struct state_backend_format *format,
 }
 
 static int backend_format_raw_unpack(struct state_backend_format *format,
-				     struct state *state, const uint8_t * buf,
+				     struct state *state, const void * buf,
 				     ssize_t len)
 {
 	struct state_variable *sv;
 	const struct backend_raw_header *header;
-	const uint8_t *data;
+	const void *data;
 	struct state_backend_format_raw *backend_raw = get_format_raw(format);
 
 	header = (const struct backend_raw_header *)buf;
@@ -163,7 +163,7 @@ static int backend_format_raw_unpack(struct state_backend_format *format,
 }
 
 static int backend_format_raw_pack(struct state_backend_format *format,
-				   struct state *state, uint8_t ** buf_out,
+				   struct state *state, void ** buf_out,
 				   ssize_t * len_out)
 {
 	struct state_backend_format_raw *backend_raw = get_format_raw(format);
diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index f1b3f5a6b2..6ec58a0c97 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -42,7 +42,7 @@ const unsigned int min_copies_written = 1;
  * error.
  */
 int state_storage_write(struct state_backend_storage *storage,
-		        const uint8_t * buf, ssize_t len)
+		        const void * buf, ssize_t len)
 {
 	struct state_backend_storage_bucket *bucket;
 	int ret;
@@ -82,7 +82,7 @@ int state_storage_write(struct state_backend_storage *storage,
  * low, can read their own copy first and compare it.
  */
 int state_storage_restore_consistency(struct state_backend_storage *storage,
-				      const uint8_t * buf, ssize_t len)
+				      const void * buf, ssize_t len)
 {
 	return state_storage_write(storage, buf, len);
 }
@@ -105,7 +105,7 @@ int state_storage_restore_consistency(struct state_backend_storage *storage,
  */
 int state_storage_read(struct state_backend_storage *storage,
 		       struct state_backend_format *format,
-		       uint32_t magic, uint8_t ** buf, ssize_t * len)
+		       uint32_t magic, void **buf, ssize_t *len)
 {
 	struct state_backend_storage_bucket *bucket;
 	int ret;
diff --git a/common/state/state.c b/common/state/state.c
index 8d2dca2523..476e3ee08e 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -41,7 +41,7 @@ static LIST_HEAD(state_list);
  */
 int state_save(struct state *state)
 {
-	uint8_t *buf;
+	void *buf;
 	ssize_t len;
 	int ret;
 
@@ -80,7 +80,7 @@ out:
  */
 int state_load(struct state *state)
 {
-	uint8_t *buf;
+	void *buf;
 	ssize_t len;
 	int ret;
 
diff --git a/common/state/state.h b/common/state/state.h
index 0b1a7e5ec2..52d332e27d 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -21,9 +21,9 @@ struct mtd_info_user;
  */
 struct state_backend_storage_bucket {
 	int (*write) (struct state_backend_storage_bucket * bucket,
-		      const uint8_t * buf, ssize_t len);
+		      const void * buf, ssize_t len);
 	int (*read) (struct state_backend_storage_bucket * bucket,
-		     uint8_t ** buf, ssize_t * len_hint);
+		     void ** buf, ssize_t * len_hint);
 	void (*free) (struct state_backend_storage_bucket * bucket);
 
 	struct list_head bucket_list;
@@ -46,11 +46,11 @@ struct state_backend_storage_bucket {
  */
 struct state_backend_format {
 	int (*verify) (struct state_backend_format * format, uint32_t magic,
-		       const uint8_t * buf, ssize_t *lenp);
+		       const void * buf, ssize_t *lenp);
 	int (*pack) (struct state_backend_format * format, struct state * state,
-		     uint8_t ** buf, ssize_t * len);
+		     void ** buf, ssize_t * len);
 	int (*unpack) (struct state_backend_format * format,
-		       struct state * state, const uint8_t * buf, ssize_t len);
+		       struct state * state, const void * buf, ssize_t len);
 	void (*free) (struct state_backend_format * format);
 	const char *name;
 };
@@ -207,13 +207,13 @@ int state_backend_bucket_direct_create(struct device_d *dev, const char *path,
 				       struct state_backend_storage_bucket **bucket,
 				       off_t offset, ssize_t max_size);
 int state_storage_write(struct state_backend_storage *storage,
-			const uint8_t * buf, ssize_t len);
+			const void * buf, ssize_t len);
 int state_storage_restore_consistency(struct state_backend_storage
-				      *storage, const uint8_t * buf,
+				      *storage, const void * buf,
 				      ssize_t len);
 int state_storage_read(struct state_backend_storage *storage,
 		       struct state_backend_format *format,
-		       uint32_t magic, uint8_t **buf, ssize_t *len);
+		       uint32_t magic, void **buf, ssize_t *len);
 
 static inline struct state_uint32 *to_state_uint32(struct state_variable *s)
 {
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 16/42] state: Drop cache bucket
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (14 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 15/42] state: Convert all bufs to void * Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-04-15  8:53   ` Sam Ravnborg
  2017-03-31  7:03 ` [PATCH 17/42] state: backend-direct: Fix max_size Sascha Hauer
                   ` (26 subsequent siblings)
  42 siblings, 1 reply; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

The cache bucket sits between the storage functions and the backend
storage. We only read from the storage once, so there is no need to cache
anything. The real purpose of the cache bucket is to keep the -EUCLEAN
information when a NAND block needs to be rewritten and to keep the read
buffers as long as the backend iterates over all buckets trying to find
the one we want to use.

This can be coded easier and more obvious in the backend code, so drop
the cache bucket.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/Makefile                |   1 -
 common/state/backend_bucket_cached.c | 141 -----------------------------------
 common/state/backend_storage.c       | 112 +++++++++++++++++-----------
 common/state/state.h                 |   7 +-
 4 files changed, 71 insertions(+), 190 deletions(-)
 delete mode 100644 common/state/backend_bucket_cached.c

diff --git a/common/state/Makefile b/common/state/Makefile
index d3518feab4..fcf9add52c 100644
--- a/common/state/Makefile
+++ b/common/state/Makefile
@@ -5,4 +5,3 @@ obj-y += backend_format_raw.o
 obj-y += backend_storage.o
 obj-y += backend_bucket_direct.o
 obj-$(CONFIG_MTD) += backend_bucket_circular.o
-obj-y += backend_bucket_cached.o
diff --git a/common/state/backend_bucket_cached.c b/common/state/backend_bucket_cached.c
deleted file mode 100644
index f8a7785387..0000000000
--- a/common/state/backend_bucket_cached.c
+++ /dev/null
@@ -1,141 +0,0 @@
-/*
- * Copyright (C) 2016 Pengutronix, Markus Pargmann <mpa@pengutronix.de>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- */
-
-#include <common.h>
-#include "state.h"
-
-struct state_backend_storage_bucket_cache {
-	struct state_backend_storage_bucket bucket;
-
-	struct state_backend_storage_bucket *raw;
-
-	void *data;
-	ssize_t data_len;
-	bool force_write;
-
-	/* For outputs */
-	struct device_d *dev;
-};
-
-static inline struct state_backend_storage_bucket_cache
-    *get_bucket_cache(struct state_backend_storage_bucket *bucket)
-{
-	return container_of(bucket,
-			    struct state_backend_storage_bucket_cache,
-			    bucket);
-}
-
-static inline void state_backend_bucket_cache_drop(
-		struct state_backend_storage_bucket_cache *cache)
-{
-	if (cache->data) {
-		free(cache->data);
-		cache->data = NULL;
-		cache->data_len = 0;
-	}
-}
-
-static int state_backend_bucket_cache_fill(
-		struct state_backend_storage_bucket_cache *cache)
-{
-	int ret;
-
-	ret = cache->raw->read(cache->raw, &cache->data, &cache->data_len);
-	if (ret == -EUCLEAN) {
-		cache->force_write = true;
-		ret = 0;
-	}
-
-	return ret;
-}
-
-static int state_backend_bucket_cache_read(struct state_backend_storage_bucket *bucket,
-					   void ** buf_out,
-					   ssize_t * len_hint)
-{
-	struct state_backend_storage_bucket_cache *cache =
-			get_bucket_cache(bucket);
-	int ret;
-
-	if (!cache->data) {
-		ret = state_backend_bucket_cache_fill(cache);
-		if (ret)
-			return ret;
-	}
-
-	if (cache->data) {
-		*buf_out = xmemdup(cache->data, cache->data_len);
-		if (!*buf_out)
-			return -ENOMEM;
-		*len_hint = cache->data_len;
-	}
-
-	return 0;
-}
-
-static int state_backend_bucket_cache_write(struct state_backend_storage_bucket *bucket,
-					    const void * buf, ssize_t len)
-{
-	struct state_backend_storage_bucket_cache *cache =
-			get_bucket_cache(bucket);
-	int ret;
-
-	if (!cache->force_write) {
-		if (!cache->data)
-			ret = state_backend_bucket_cache_fill(cache);
-
-		if (cache->data_len == len && !memcmp(cache->data, buf, len))
-			return 0;
-	}
-
-	state_backend_bucket_cache_drop(cache);
-
-	ret = cache->raw->write(cache->raw, buf, len);
-	if (ret)
-		return ret;
-
-	cache->data = xmemdup(buf, len);
-	cache->data_len = len;
-	return 0;
-}
-
-static void state_backend_bucket_cache_free(
-		struct state_backend_storage_bucket *bucket)
-{
-	struct state_backend_storage_bucket_cache *cache =
-			get_bucket_cache(bucket);
-
-	state_backend_bucket_cache_drop(cache);
-	cache->raw->free(cache->raw);
-	free(cache);
-}
-
-int state_backend_bucket_cached_create(struct device_d *dev,
-				       struct state_backend_storage_bucket *raw,
-				       struct state_backend_storage_bucket **out)
-{
-	struct state_backend_storage_bucket_cache *cache;
-
-	cache = xzalloc(sizeof(*cache));
-	cache->raw = raw;
-	cache->dev = dev;
-
-	cache->bucket.free = state_backend_bucket_cache_free;
-	cache->bucket.read = state_backend_bucket_cache_read;
-	cache->bucket.write = state_backend_bucket_cache_write;
-
-	*out = &cache->bucket;
-
-	return 0;
-}
diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 6ec58a0c97..218c67f2d7 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -69,22 +69,29 @@ int state_storage_write(struct state_backend_storage *storage,
 	return -EIO;
 }
 
-/**
- * state_storage_restore_consistency - Restore consistency on all storage backends
- * @param storage Storage object
- * @param buf Buffer with valid data that should be on all buckets after this operation
- * @param len Length of the buffer
- * @return 0 on success, -errno otherwise
- *
- * This function brings valid data onto all buckets we have to ensure that all
- * data copies are in sync. In the current implementation we just write the data
- * to all buckets. Bucket implementations that need to keep the number of writes
- * low, can read their own copy first and compare it.
- */
-int state_storage_restore_consistency(struct state_backend_storage *storage,
-				      const void * buf, ssize_t len)
+static int bucket_refresh(struct state_backend_storage *storage,
+			  struct state_backend_storage_bucket *bucket, void *buf, ssize_t len)
 {
-	return state_storage_write(storage, buf, len);
+	int ret;
+
+	if (bucket->needs_refresh)
+		goto refresh;
+
+	if (bucket->len != len)
+		goto refresh;
+
+	if (memcmp(bucket->buf, buf, len))
+		goto refresh;
+
+	return 0;
+
+refresh:
+	ret = bucket->write(bucket, buf, len);
+
+	if (ret)
+		dev_warn(storage->dev, "Failed to restore bucket\n");
+
+	return ret;
 }
 
 /**
@@ -94,7 +101,6 @@ int state_storage_restore_consistency(struct state_backend_storage *storage,
  * @param magic state magic value
  * @param buf The newly allocated data area will be stored in this pointer
  * @param len The resulting length of the buffer
- * @param len_hint Hint of how big the data may be.
  * @return 0 on success, -errno otherwise. buf and len will be set to valid
  * values on success.
  *
@@ -107,12 +113,18 @@ int state_storage_read(struct state_backend_storage *storage,
 		       struct state_backend_format *format,
 		       uint32_t magic, void **buf, ssize_t *len)
 {
-	struct state_backend_storage_bucket *bucket;
+	struct state_backend_storage_bucket *bucket, *bucket_used = NULL;
 	int ret;
 
+	/*
+	 * Iterate over all buckets. The first valid one we find is the
+	 * one we want to use.
+	 */
 	list_for_each_entry(bucket, &storage->buckets, bucket_list) {
-		ret = bucket->read(bucket, buf, len);
-		if (ret) {
+		ret = bucket->read(bucket, &bucket->buf, &bucket->len);
+		if (ret == -EUCLEAN) {
+			bucket->needs_refresh = 1;
+		} else if (ret) {
 			dev_warn(storage->dev, "Failed to read from state backend bucket, trying next, %d\n",
 				 ret);
 			continue;
@@ -122,22 +134,46 @@ int state_storage_read(struct state_backend_storage *storage,
 		 * Verify the buffer crcs. The buffer length is passed in the len argument,
 		 * .verify overwrites it with the length actually used.
 		 */
-		ret = format->verify(format, magic, *buf, len);
-		if (!ret) {
-			goto found;
-		}
-		free(*buf);
-		dev_warn(storage->dev, "Failed to verify read copy, trying next bucket, %d\n",
-			 ret);
+		ret = format->verify(format, magic, bucket->buf, &bucket->len);
+		if (!ret && !bucket_used)
+			bucket_used = bucket;
+
+		if (ret)
+			dev_warn(storage->dev, "Failed to verify read copy, trying next bucket, %d\n",
+				 ret);
+	}
+
+	if (!bucket_used) {
+		dev_err(storage->dev, "Failed to find any valid state copy in any bucket\n");
+
+		return -ENOENT;
 	}
 
-	dev_err(storage->dev, "Failed to find any valid state copy in any bucket\n");
+	/*
+	 * Restore/refresh all buckets except the one we currently use (in case
+	 * it's the only usable bucket at the moment)
+	 */
+	list_for_each_entry(bucket, &storage->buckets, bucket_list) {
+		if (bucket == bucket_used)
+			continue;
+
+		ret = bucket_refresh(storage, bucket, bucket_used->buf, bucket_used->len);
+
+		/* Free buffer from the unused buckets */
+		free(bucket->buf);
+		bucket->buf = NULL;
+	}
 
-	return -ENOENT;
+	/*
+	 * Restore/refresh the bucket we currently use
+	 */
+	ret = bucket_refresh(storage, bucket_used, bucket_used->buf, bucket_used->len);
 
-found:
-	/* A failed restore consistency is not a failure of reading the state */
-	state_storage_restore_consistency(storage, *buf, *len);
+	*buf = bucket_used->buf;
+	*len = bucket_used->len;
+
+	/* buffer from the used bucket is passed to the caller, do not free */
+	bucket_used->buf = NULL;
 
 	return 0;
 }
@@ -218,13 +254,6 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 			continue;
 		}
 
-		ret = state_backend_bucket_cached_create(storage->dev, bucket,
-							 &bucket);
-		if (ret) {
-			dev_warn(storage->dev, "Failed to setup cache bucket, continuing without cache, %d\n",
-				 ret);
-		}
-
 		list_add_tail(&bucket->bucket_list, &storage->buckets);
 		++nr_copies;
 		if (nr_copies >= desired_copies)
@@ -284,13 +313,6 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
 			continue;
 		}
 
-		ret = state_backend_bucket_cached_create(storage->dev, bucket,
-							 &bucket);
-		if (ret) {
-			dev_warn(storage->dev, "Failed to setup cache bucket, continuing without cache, %d\n",
-				 ret);
-		}
-
 		list_add_tail(&bucket->bucket_list, &storage->buckets);
 		++nr_copies;
 	}
diff --git a/common/state/state.h b/common/state/state.h
index 52d332e27d..62544a207c 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -27,6 +27,10 @@ struct state_backend_storage_bucket {
 	void (*free) (struct state_backend_storage_bucket * bucket);
 
 	struct list_head bucket_list;
+
+	void *buf;
+	ssize_t len;
+	bool needs_refresh;
 };
 
 /**
@@ -208,9 +212,6 @@ int state_backend_bucket_direct_create(struct device_d *dev, const char *path,
 				       off_t offset, ssize_t max_size);
 int state_storage_write(struct state_backend_storage *storage,
 			const void * buf, ssize_t len);
-int state_storage_restore_consistency(struct state_backend_storage
-				      *storage, const void * buf,
-				      ssize_t len);
 int state_storage_read(struct state_backend_storage *storage,
 		       struct state_backend_format *format,
 		       uint32_t magic, void **buf, ssize_t *len);
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 17/42] state: backend-direct: Fix max_size
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (15 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 16/42] state: Drop cache bucket Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 18/42] state: bucket: Make output more informative Sascha Hauer
                   ` (25 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

The max_size in the direct backend includes the meta data, so
substract its size when determing the max data size we can store.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_bucket_direct.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/state/backend_bucket_direct.c b/common/state/backend_bucket_direct.c
index 246cb499c4..9996ac2919 100644
--- a/common/state/backend_bucket_direct.c
+++ b/common/state/backend_bucket_direct.c
@@ -103,7 +103,7 @@ static int state_backend_bucket_direct_write(struct state_backend_storage_bucket
 	int ret;
 	struct state_backend_storage_bucket_direct_meta meta;
 
-	if (direct->max_size && len > direct->max_size)
+	if (direct->max_size && len > direct->max_size - sizeof(meta))
 		return -E2BIG;
 
 	ret = lseek(direct->fd, direct->offset, SEEK_SET);
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 18/42] state: bucket: Make output more informative
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (16 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 17/42] state: backend-direct: Fix max_size Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 19/42] state: backend_bucket_direct: max_size is always given Sascha Hauer
                   ` (24 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

Print offset and number of the bucket along with the bucket
specific messages to give a hint which bucket a message is for.

Also it's pretty much expected that buckets sometimes have no
data or need cleanup, so instead of complaining loudly, only
write which bucket is used and which buckets are cleaned up.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_storage.c | 25 +++++++++++++++----------
 common/state/state.h           |  3 +++
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 218c67f2d7..4e0548af85 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -89,7 +89,11 @@ refresh:
 	ret = bucket->write(bucket, buf, len);
 
 	if (ret)
-		dev_warn(storage->dev, "Failed to restore bucket\n");
+		dev_warn(storage->dev, "Failed to restore bucket %d@0x%08lx\n",
+			 bucket->num, bucket->offset);
+	else
+		dev_info(storage->dev, "restored bucket %d@0x%08lx\n",
+			 bucket->num, bucket->offset);
 
 	return ret;
 }
@@ -122,13 +126,10 @@ int state_storage_read(struct state_backend_storage *storage,
 	 */
 	list_for_each_entry(bucket, &storage->buckets, bucket_list) {
 		ret = bucket->read(bucket, &bucket->buf, &bucket->len);
-		if (ret == -EUCLEAN) {
+		if (ret == -EUCLEAN)
 			bucket->needs_refresh = 1;
-		} else if (ret) {
-			dev_warn(storage->dev, "Failed to read from state backend bucket, trying next, %d\n",
-				 ret);
+		else if (ret)
 			continue;
-		}
 
 		/*
 		 * Verify the buffer crcs. The buffer length is passed in the len argument,
@@ -137,10 +138,6 @@ int state_storage_read(struct state_backend_storage *storage,
 		ret = format->verify(format, magic, bucket->buf, &bucket->len);
 		if (!ret && !bucket_used)
 			bucket_used = bucket;
-
-		if (ret)
-			dev_warn(storage->dev, "Failed to verify read copy, trying next bucket, %d\n",
-				 ret);
 	}
 
 	if (!bucket_used) {
@@ -149,6 +146,8 @@ int state_storage_read(struct state_backend_storage *storage,
 		return -ENOENT;
 	}
 
+	dev_info(storage->dev, "Using bucket %d@0x%08lx\n", bucket_used->num, bucket_used->offset);
+
 	/*
 	 * Restore/refresh all buckets except the one we currently use (in case
 	 * it's the only usable bucket at the moment)
@@ -254,6 +253,9 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 			continue;
 		}
 
+		bucket->offset = offset;
+		bucket->num = nr_copies;
+
 		list_add_tail(&bucket->bucket_list, &storage->buckets);
 		++nr_copies;
 		if (nr_copies >= desired_copies)
@@ -313,6 +315,9 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
 			continue;
 		}
 
+		bucket->offset = offset;
+		bucket->num = nr_copies;
+
 		list_add_tail(&bucket->bucket_list, &storage->buckets);
 		++nr_copies;
 	}
diff --git a/common/state/state.h b/common/state/state.h
index 62544a207c..f6ab2009cc 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -26,6 +26,9 @@ struct state_backend_storage_bucket {
 		     void ** buf, ssize_t * len_hint);
 	void (*free) (struct state_backend_storage_bucket * bucket);
 
+	int num;
+	off_t offset;
+
 	struct list_head bucket_list;
 
 	void *buf;
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 19/42] state: backend_bucket_direct: max_size is always given
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (17 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 18/42] state: bucket: Make output more informative Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 20/42] state: backend: Add more fields to struct state_backend_storage Sascha Hauer
                   ` (23 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

max_size is always != 0, so if(direct->max_size) can be skipped.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_bucket_direct.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/state/backend_bucket_direct.c b/common/state/backend_bucket_direct.c
index 9996ac2919..b72e8adb96 100644
--- a/common/state/backend_bucket_direct.c
+++ b/common/state/backend_bucket_direct.c
@@ -103,7 +103,7 @@ static int state_backend_bucket_direct_write(struct state_backend_storage_bucket
 	int ret;
 	struct state_backend_storage_bucket_direct_meta meta;
 
-	if (direct->max_size && len > direct->max_size - sizeof(meta))
+	if (len > direct->max_size - sizeof(meta))
 		return -E2BIG;
 
 	ret = lseek(direct->fd, direct->offset, SEEK_SET);
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 20/42] state: backend: Add more fields to struct state_backend_storage
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (18 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 19/42] state: backend_bucket_direct: max_size is always given Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 21/42] state: backend_circular: remove unnecessary warning Sascha Hauer
                   ` (22 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

To save a few function arguments.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_storage.c | 48 ++++++++++++++++++------------------------
 common/state/state.h           |  6 ++++++
 2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 4e0548af85..17013855b4 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -201,10 +201,7 @@ const int desired_copies = 3;
  * state_storage_mtd_buckets_init - Creates storage buckets for mtd devices
  * @param storage Storage object
  * @param meminfo Info about the mtd device
- * @param path Path to the device
  * @param circular If false, use non-circular mode to write data that is compatible with the old on-flash format
- * @param dev_offset Offset to start at in the device.
- * @param max_size Maximum size to use for data. May be 0 for infinite.
  * @return 0 on success, -errno otherwise
  *
  * Starting from offset 0 this function tries to create circular buckets on
@@ -214,12 +211,10 @@ const int desired_copies = 3;
  * Circular buckets write new data always in the next free space.
  */
 static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
-					  struct mtd_info_user *meminfo,
-					  const char *path, bool circular,
-					  off_t dev_offset, size_t max_size)
+					  struct mtd_info_user *meminfo, bool circular)
 {
 	struct state_backend_storage_bucket *bucket;
-	ssize_t end = dev_offset + max_size;
+	ssize_t end = storage->offset + storage->max_size;
 	int nr_copies = 0;
 	off_t offset;
 	ssize_t writesize;
@@ -227,9 +222,9 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 	if (!end || end > meminfo->size)
 		end = meminfo->size;
 
-	if (!IS_ALIGNED(dev_offset, meminfo->erasesize)) {
+	if (!IS_ALIGNED(storage->offset, meminfo->erasesize)) {
 		dev_err(storage->dev, "Offset within the device is not aligned to eraseblocks. Offset is %ld, erasesize %zu\n",
-			dev_offset, meminfo->erasesize);
+			storage->offset, meminfo->erasesize);
 		return -EINVAL;
 	}
 
@@ -238,18 +233,18 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 	else
 		writesize = meminfo->erasesize;
 
-	for (offset = dev_offset; offset < end; offset += meminfo->erasesize) {
+	for (offset = storage->offset; offset < end; offset += meminfo->erasesize) {
 		int ret;
 		unsigned int eraseblock = offset / meminfo->erasesize;
 
-		ret = state_backend_bucket_circular_create(storage->dev, path,
+		ret = state_backend_bucket_circular_create(storage->dev, storage->path,
 							   &bucket,
 							   eraseblock,
 							   writesize,
 							   meminfo);
 		if (ret) {
 			dev_warn(storage->dev, "Failed to create bucket at '%s' eraseblock %u\n",
-				 path, eraseblock);
+				 storage->path, eraseblock);
 			continue;
 		}
 
@@ -275,24 +270,19 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 /**
  * state_storage_file_buckets_init - Create buckets for a conventional file descriptor
  * @param storage Storage object
- * @param path Path to file/device
- * @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 How far apart the different data copies are placed. If
- * stridesize is 0, only one copy can be created.
  * @return 0 on success, -errno otherwise
  *
  * For blockdevices and other regular files we create direct buckets beginning
  * at offset 0. Direct buckets are simple and write data always to offset 0.
  */
-static int state_storage_file_buckets_init(struct state_backend_storage *storage,
-					   const char *path, off_t dev_offset,
-					   size_t max_size, uint32_t stridesize)
+static int state_storage_file_buckets_init(struct state_backend_storage *storage)
 {
 	struct state_backend_storage_bucket *bucket;
 	int ret, n;
 	off_t offset;
 	int nr_copies = 0;
+	uint32_t stridesize = storage->stridesize;
+	size_t max_size = storage->max_size;
 
 	if (!stridesize) {
 		dev_err(storage->dev, "stridesize unspecified\n");
@@ -305,13 +295,13 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
 	}
 
 	for (n = 0; n < desired_copies; n++) {
-		offset = dev_offset + n * stridesize;
-		ret = state_backend_bucket_direct_create(storage->dev, path,
+		offset = storage->offset + n * stridesize;
+		ret = state_backend_bucket_direct_create(storage->dev, storage->path,
 							 &bucket, offset,
 							 stridesize);
 		if (ret) {
 			dev_warn(storage->dev, "Failed to create direct bucket at '%s' offset %ld\n",
-				 path, offset);
+				 storage->path, offset);
 			continue;
 		}
 
@@ -359,6 +349,9 @@ int state_storage_init(struct state *state, const char *path,
 	storage->dev = &state->dev;
 	storage->name = storagetype;
 	storage->stridesize = stridesize;
+	storage->offset = offset;
+	storage->max_size = max_size;
+	storage->path = xstrdup(path);
 
 	if (IS_ENABLED(CONFIG_MTD))
 		ret = mtd_get_meminfo(path, &meminfo);
@@ -372,12 +365,9 @@ int state_storage_init(struct state *state, const char *path,
 				 storagetype);
 			circular = false;
 		}
-		return state_storage_mtd_buckets_init(storage, &meminfo, path,
-						      circular, offset,
-						      max_size);
+		return state_storage_mtd_buckets_init(storage, &meminfo, circular);
 	} else {
-		return state_storage_file_buckets_init(storage, path, offset,
-						       max_size, stridesize);
+		return state_storage_file_buckets_init(storage);
 	}
 
 	dev_err(storage->dev, "storage init done\n");
@@ -405,4 +395,6 @@ void state_storage_free(struct state_backend_storage *storage)
 		list_del(&bucket->bucket_list);
 		bucket->free(bucket);
 	}
+
+	free(storage->path);
 }
diff --git a/common/state/state.h b/common/state/state.h
index f6ab2009cc..a2737abeb6 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -66,6 +66,9 @@ struct state_backend_format {
  * state_backend_storage - Storage backend of the state.
  *
  * @buckets List of storage buckets that are available
+ * @stridesize The distance between copies
+ * @offset Offset in the backend device where the data starts
+ * @max_size The maximum size of the data we can use
  */
 struct state_backend_storage {
 	struct list_head buckets;
@@ -76,6 +79,9 @@ struct state_backend_storage {
 	const char *name;
 
 	uint32_t stridesize;
+	off_t offset;
+	size_t max_size;
+	char *path;
 
 	bool readonly;
 };
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 21/42] state: backend_circular: remove unnecessary warning
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (19 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 20/42] state: backend: Add more fields to struct state_backend_storage Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 22/42] state: storage: direct: do not close file that is not opened Sascha Hauer
                   ` (21 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

It's expected that NAND flashes contain bad blocks, do not warn
about them.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_storage.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 17013855b4..deae9c325b 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -242,11 +242,8 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 							   eraseblock,
 							   writesize,
 							   meminfo);
-		if (ret) {
-			dev_warn(storage->dev, "Failed to create bucket at '%s' eraseblock %u\n",
-				 storage->path, eraseblock);
+		if (ret)
 			continue;
-		}
 
 		bucket->offset = offset;
 		bucket->num = nr_copies;
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 22/42] state: storage: direct: do not close file that is not opened
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (20 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 21/42] state: backend_circular: remove unnecessary warning Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 23/42] state: backend: Add some documentation Sascha Hauer
                   ` (20 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

When open failed to not try to close the invalid fd afterwards.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_bucket_direct.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/common/state/backend_bucket_direct.c b/common/state/backend_bucket_direct.c
index b72e8adb96..4465ed0e41 100644
--- a/common/state/backend_bucket_direct.c
+++ b/common/state/backend_bucket_direct.c
@@ -156,7 +156,6 @@ int state_backend_bucket_direct_create(struct device_d *dev, const char *path,
 	fd = open(path, O_RDWR);
 	if (fd < 0) {
 		dev_err(dev, "Failed to open file '%s', %d\n", path, -errno);
-		close(fd);
 		return -errno;
 	}
 
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 23/42] state: backend: Add some documentation
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (21 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 22/42] state: storage: direct: do not close file that is not opened Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 24/42] state: backend_circular: default to circular storage Sascha Hauer
                   ` (19 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

Write some sentences to make the concepts clearer.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_bucket_circular.c | 16 +++++++++++++++-
 common/state/backend_storage.c         | 22 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/common/state/backend_bucket_circular.c b/common/state/backend_bucket_circular.c
index 53ee0c3f57..58fffd18e3 100644
--- a/common/state/backend_bucket_circular.c
+++ b/common/state/backend_bucket_circular.c
@@ -25,7 +25,21 @@
 
 #include "state.h"
 
-
+/*
+ * The circular backend bucket code. The circular backend bucket is intended
+ * for mtd devices which need an erase operation.
+ *
+ * Erasing blocks is an operation that should be avoided. On NOR flashes erasing
+ * blocks is very time consuming and on NAND flashes each block only has a limited
+ * number of erase cycles allowed. For this reason we continuously write more data
+ * into each eraseblock and only erase it when no more free space is available.
+ * Don't confuse these multiple writes into a single eraseblock with buckets. A bucket
+ * is the whole eraseblock, we just happen to reuse the same bucket for storing
+ * new data.
+ *
+ * If your device is a mtd device, but does not have eraseblocks, like MRAMs, then
+ * the direct bucket is used instead.
+ */
 struct state_backend_storage_bucket_circular {
 	struct state_backend_storage_bucket bucket;
 
diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index deae9c325b..f9e8151670 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -25,6 +25,28 @@
 
 #include "state.h"
 
+/*
+ * The state framework stores data in so called buckets. A bucket is
+ * exactly one copy of the state we want to store. On flash type media
+ * a bucket corresponds to a single eraseblock. On media which do not
+ * need an erase operation a bucket corresponds to a storage area of
+ * @stridesize bytes.
+ *
+ * For redundancy and to make sure that we have valid data on the storage
+ * device at any time the state framework stores multiple buckets. The strategy
+ * is as follows:
+ *
+ * When loading the state from the storage we iterate over the buckets. We
+ * take the first one we find which has valid crcs. The next step is to
+ * restore consistency between the different buckets. This means rewriting
+ * a bucket when it signalled it needs refresh (i.e. returned -EUCLEAN)
+ * or when contains data different from the bucket we use.
+ *
+ * When the state backend initialized successfully we already restored
+ * consistency which means all buckets contain the same data. This means
+ * when storing a new state we can just write all buckets in order.
+ */
+
 const unsigned int min_copies_written = 1;
 
 /**
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 24/42] state: backend_circular: default to circular storage
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (22 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 23/42] state: backend: Add some documentation Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 25/42] state: backend_circular: rewrite function doc Sascha Hauer
                   ` (18 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

Default to the new circular storage format which saves erase
cycles. The old format can still be selected with
backend-storage-type = "noncircular".

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../devicetree/bindings/barebox/barebox,state.rst          |  6 +++---
 common/state/backend_storage.c                             | 14 ++++++++------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/barebox/barebox,state.rst b/Documentation/devicetree/bindings/barebox/barebox,state.rst
index e9daa65f1a..00fb592614 100644
--- a/Documentation/devicetree/bindings/barebox/barebox,state.rst
+++ b/Documentation/devicetree/bindings/barebox/barebox,state.rst
@@ -40,9 +40,9 @@ Optional properties:
   e.g. ``hmac(sha256)``. Only used for ``raw``.
 * ``backend-stridesize``: Maximum size per copy of the data. Only important for
   non-MTD devices
-* ``backend-storage-type``: Type of the storage. This has two options at the
-  moment. For MTD with erasing the correct type is ``circular``. For all other
-  devices and files, ``direct`` is the needed type.
+* ``backend-storage-type``: Normally the correct storage type is detected auto-
+  matically. The circular backend supports the option ``noncircular`` to fall
+  back to an old storage format.
 
 Variable nodes
 --------------
diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index f9e8151670..036204c188 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -376,13 +376,15 @@ int state_storage_init(struct state *state, const char *path,
 		ret = mtd_get_meminfo(path, &meminfo);
 
 	if (!ret && !(meminfo.flags & MTD_NO_ERASE)) {
-		bool circular = true;
-		if (!storagetype) {
-			circular = false;
-		} else if (strcmp(storagetype, "circular")) {
-			dev_warn(storage->dev, "Unknown storagetype '%s', falling back to old format circular storage type.\n",
-				 storagetype);
+		bool circular;
+		if (!storagetype || !strcmp(storagetype, "circular")) {
+			circular = true;
+		} else if (!strcmp(storagetype, "noncircular")) {
+			dev_warn(storage->dev, "using old format circular storage type.\n");
 			circular = false;
+		} else {
+			dev_warn(storage->dev, "unknown storage type '%s'\n", storagetype);
+			return -EINVAL;
 		}
 		return state_storage_mtd_buckets_init(storage, &meminfo, circular);
 	} else {
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 25/42] state: backend_circular: rewrite function doc
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (23 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 24/42] state: backend_circular: default to circular storage Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 26/42] state: backend_storage: Rename variable nr_copies to n_buckets Sascha Hauer
                   ` (17 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

The commment talks about copies where buckets are meant and also
claims we start at offset 0, which may not be true. Rewrite comment.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_storage.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 036204c188..d29db36cca 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -226,11 +226,9 @@ const int desired_copies = 3;
  * @param circular If false, use non-circular mode to write data that is compatible with the old on-flash format
  * @return 0 on success, -errno otherwise
  *
- * Starting from offset 0 this function tries to create circular buckets on
- * different offsets in the device. Different copies of the data are located in
- * different eraseblocks.
- * For MTD devices we use circular buckets to minimize the number of erases.
- * Circular buckets write new data always in the next free space.
+ * This function iterates over the eraseblocks and creates one bucket on
+ * each eraseblock until we have the number of desired buckets. Bad blocks
+ * will be skipped and the next block will be used.
  */
 static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 					  struct mtd_info_user *meminfo, bool circular)
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 26/42] state: backend_storage: Rename variable nr_copies to n_buckets
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (24 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 25/42] state: backend_circular: rewrite function doc Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 27/42] state: backend_storage: Rename variable desired_copies to desired_buckets Sascha Hauer
                   ` (16 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

We defined what a bucket is, so use n_buckets when counting buckets,
and not nr_copies.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_storage.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index d29db36cca..c29004071b 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -235,7 +235,7 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 {
 	struct state_backend_storage_bucket *bucket;
 	ssize_t end = storage->offset + storage->max_size;
-	int nr_copies = 0;
+	int n_buckets = 0;
 	off_t offset;
 	ssize_t writesize;
 
@@ -266,21 +266,21 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 			continue;
 
 		bucket->offset = offset;
-		bucket->num = nr_copies;
+		bucket->num = n_buckets;
 
 		list_add_tail(&bucket->bucket_list, &storage->buckets);
-		++nr_copies;
-		if (nr_copies >= desired_copies)
+		++n_buckets;
+		if (n_buckets >= desired_copies)
 			return 0;
 	}
 
-	if (!nr_copies) {
+	if (!n_buckets) {
 		dev_err(storage->dev, "Failed to initialize any state storage bucket\n");
 		return -EIO;
 	}
 
 	dev_warn(storage->dev, "Failed to initialize desired amount of buckets, only %d of %d succeeded\n",
-		 nr_copies, desired_copies);
+		 n_buckets, desired_copies);
 	return 0;
 }
 
@@ -297,7 +297,7 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
 	struct state_backend_storage_bucket *bucket;
 	int ret, n;
 	off_t offset;
-	int nr_copies = 0;
+	int n_buckets = 0;
 	uint32_t stridesize = storage->stridesize;
 	size_t max_size = storage->max_size;
 
@@ -323,20 +323,20 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
 		}
 
 		bucket->offset = offset;
-		bucket->num = nr_copies;
+		bucket->num = n_buckets;
 
 		list_add_tail(&bucket->bucket_list, &storage->buckets);
-		++nr_copies;
+		++n_buckets;
 	}
 
-	if (!nr_copies) {
+	if (!n_buckets) {
 		dev_err(storage->dev, "Failed to initialize any state direct storage bucket\n");
 		return -EIO;
 	}
 
-	if (nr_copies < desired_copies)
+	if (n_buckets < desired_copies)
 		dev_warn(storage->dev, "Failed to initialize desired amount of direct buckets, only %d of %d succeeded\n",
-			nr_copies, desired_copies);
+			n_buckets, desired_copies);
 
 	return 0;
 }
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 27/42] state: backend_storage: Rename variable desired_copies to desired_buckets
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (25 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 26/42] state: backend_storage: Rename variable nr_copies to n_buckets Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 28/42] state: backend_storage: rewrite function doc Sascha Hauer
                   ` (15 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

We defined what a bucket is, so rename the variable that holds the
number of desired buckets from desired_copies to desired_buckets.

While at it, make locally used variable static.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_storage.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index c29004071b..1310531dd1 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -216,8 +216,8 @@ static int mtd_get_meminfo(const char *path, struct mtd_info_user *meminfo)
 	return ret;
 }
 
-/* Number of copies that should be allocated */
-const int desired_copies = 3;
+/* Number of buckets that should be used */
+static const int desired_buckets = 3;
 
 /**
  * state_storage_mtd_buckets_init - Creates storage buckets for mtd devices
@@ -270,7 +270,7 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 
 		list_add_tail(&bucket->bucket_list, &storage->buckets);
 		++n_buckets;
-		if (n_buckets >= desired_copies)
+		if (n_buckets >= desired_buckets)
 			return 0;
 	}
 
@@ -280,7 +280,7 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
 	}
 
 	dev_warn(storage->dev, "Failed to initialize desired amount of buckets, only %d of %d succeeded\n",
-		 n_buckets, desired_copies);
+		 n_buckets, desired_buckets);
 	return 0;
 }
 
@@ -306,12 +306,12 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
 		return -EINVAL;
 	}
 
-	if (max_size && max_size < desired_copies * stridesize) {
-		dev_err(storage->dev, "device is too small to hold %d copies\n", desired_copies);
+	if (max_size && max_size < desired_buckets * stridesize) {
+		dev_err(storage->dev, "device is too small to hold %d copies\n", desired_buckets);
 		return -EINVAL;
 	}
 
-	for (n = 0; n < desired_copies; n++) {
+	for (n = 0; n < desired_buckets; n++) {
 		offset = storage->offset + n * stridesize;
 		ret = state_backend_bucket_direct_create(storage->dev, storage->path,
 							 &bucket, offset,
@@ -334,9 +334,9 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
 		return -EIO;
 	}
 
-	if (n_buckets < desired_copies)
+	if (n_buckets < desired_buckets)
 		dev_warn(storage->dev, "Failed to initialize desired amount of direct buckets, only %d of %d succeeded\n",
-			n_buckets, desired_copies);
+			n_buckets, desired_buckets);
 
 	return 0;
 }
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 28/42] state: backend_storage: rewrite function doc
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (26 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 27/42] state: backend_storage: Rename variable desired_copies to desired_buckets Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 29/42] state: backend_storage: make locally used variable static Sascha Hauer
                   ` (14 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

The function documentation for state_storage_file_buckets_init() is not
very accurate. Rewrite it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_storage.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 1310531dd1..bc531ef991 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -289,8 +289,8 @@ static int state_storage_mtd_buckets_init(struct state_backend_storage *storage,
  * @param storage Storage object
  * @return 0 on success, -errno otherwise
  *
- * For blockdevices and other regular files we create direct buckets beginning
- * at offset 0. Direct buckets are simple and write data always to offset 0.
+ * direct buckets are simpler than circular buckets and can be used on blockdevices
+ * and mtd devices that don't need erase (MRAM). Also used for EEPROMs.
  */
 static int state_storage_file_buckets_init(struct state_backend_storage *storage)
 {
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 29/42] state: backend_storage: make locally used variable static
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (27 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 28/42] state: backend_storage: rewrite function doc Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 30/42] state: backend_storage: rename more variables Sascha Hauer
                   ` (13 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

Signed-off-by: Sascha Hauer <s.hauer@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 bc531ef991..8d0f285f77 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -47,7 +47,7 @@
  * when storing a new state we can just write all buckets in order.
  */
 
-const unsigned int min_copies_written = 1;
+static const unsigned int min_copies_written = 1;
 
 /**
  * state_storage_write - Writes the given data to the storage
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 30/42] state: backend_storage: rename more variables
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (28 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 29/42] state: backend_storage: make locally used variable static Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 31/42] keystore: implement forgetting secrets Sascha Hauer
                   ` (12 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

Use "buckets" rather than "copies" in variable names.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_storage.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index 8d0f285f77..a5688aa424 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -47,7 +47,7 @@
  * when storing a new state we can just write all buckets in order.
  */
 
-static const unsigned int min_copies_written = 1;
+static const unsigned int min_buckets_written = 1;
 
 /**
  * state_storage_write - Writes the given data to the storage
@@ -60,7 +60,7 @@ static const unsigned int min_copies_written = 1;
  * operation on all of them. Writes are always in the same sequence. This
  * ensures, that reading in the same sequence will always return the latest
  * written valid data first.
- * We try to at least write min_copies_written. If this fails we return with an
+ * We try to at least write min_buckets_written. If this fails we return with an
  * error.
  */
 int state_storage_write(struct state_backend_storage *storage,
@@ -68,7 +68,7 @@ int state_storage_write(struct state_backend_storage *storage,
 {
 	struct state_backend_storage_bucket *bucket;
 	int ret;
-	int copies_written = 0;
+	int buckets_written = 0;
 
 	if (storage->readonly)
 		return 0;
@@ -79,15 +79,15 @@ int state_storage_write(struct state_backend_storage *storage,
 			dev_warn(storage->dev, "Failed to write state backend bucket, %d\n",
 				 ret);
 		} else {
-			++copies_written;
+			++buckets_written;
 		}
 	}
 
-	if (copies_written >= min_copies_written)
+	if (buckets_written >= min_buckets_written)
 		return 0;
 
 	dev_err(storage->dev, "Failed to write state to at least %d buckets. Successfully written to %d buckets\n",
-		min_copies_written, copies_written);
+		min_buckets_written, buckets_written);
 	return -EIO;
 }
 
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 31/42] keystore: implement forgetting secrets
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (29 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 30/42] state: backend_storage: rename more variables Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 32/42] commands: implement keystore command Sascha Hauer
                   ` (11 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

To be able to change secrets add a function to forget secrets.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 crypto/keystore.c         | 53 +++++++++++++++++++++++++++++++++--------------
 include/crypto/keystore.h |  4 ++++
 2 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/crypto/keystore.c b/crypto/keystore.c
index 90b470fe67..f2b25ca6c9 100644
--- a/crypto/keystore.c
+++ b/crypto/keystore.c
@@ -16,8 +16,8 @@ static LIST_HEAD(keystore_list);
 
 struct keystore_key {
 	struct list_head list;
-	const char *name;
-	const u8 *secret;
+	char *name;
+	u8 *secret;
 	int secret_len;
 };
 
@@ -29,6 +29,17 @@ static int keystore_compare(struct list_head *a, struct list_head *b)
 	return strcmp(na, nb);
 }
 
+static struct keystore_key *get_key(const char *name)
+{
+	struct keystore_key *key;
+
+	for_each_key(key)
+		if (!strcmp(name, key->name))
+			return key;
+
+	return NULL;
+};
+
 /**
  * @param[in] name Name of the secret to get
  * @param[out] secret Double pointer to memory representing the secret, do _not_ free() after use
@@ -38,19 +49,17 @@ int keystore_get_secret(const char *name, const u8 **secret, int *secret_len)
 {
 	struct keystore_key *key;
 
-	for_each_key(key) {
-		if (!strcmp(name, key->name)) {
-			if (!secret || !secret_len)
-				return 0;
+	if (!secret || !secret_len)
+		return 0;
 
-			*secret = key->secret;
-			*secret_len = key->secret_len;
+	key = get_key(name);
+	if (!key)
+		return -ENOENT;
 
-			return 0;
-		}
-	}
+	*secret = key->secret;
+	*secret_len = key->secret_len;
 
-	return -ENOENT;
+	return 0;
 }
 
 /**
@@ -61,11 +70,10 @@ int keystore_get_secret(const char *name, const u8 **secret, int *secret_len)
 int keystore_set_secret(const char *name, const u8 *secret, int secret_len)
 {
 	struct keystore_key *key;
-	int ret;
 
 	/* check if key is already in store */
-	ret = keystore_get_secret(name, NULL, NULL);
-	if (!ret)
+	key = get_key(name);
+	if (key)
 		return -EBUSY;
 
 	key = xzalloc(sizeof(*key));
@@ -78,3 +86,18 @@ int keystore_set_secret(const char *name, const u8 *secret, int secret_len)
 
 	return 0;
 }
+
+void keystore_forget_secret(const char *name)
+{
+	struct keystore_key *key;
+
+	key = get_key(name);
+	if (!key)
+		return;
+
+	list_del(&key->list);
+
+	free(key->name);
+	free(key->secret);
+	free(key);
+}
diff --git a/include/crypto/keystore.h b/include/crypto/keystore.h
index 29915854b8..89d962628b 100644
--- a/include/crypto/keystore.h
+++ b/include/crypto/keystore.h
@@ -12,6 +12,7 @@
 #ifdef CONFIG_CRYPTO_KEYSTORE
 int keystore_get_secret(const char *name, const u8 **secret, int *secret_len);
 int keystore_set_secret(const char *name, const u8 *secret, int secret_len);
+void keystore_forget_secret(const char *name);
 #else
 static inline int keystore_get_secret(const char *name, const u8 **secret, int *secret_len)
 {
@@ -21,6 +22,9 @@ static inline int keystore_set_secret(const char *name, const u8 *secret, int se
 {
 	return 0;
 }
+static inline void keystore_forget_secret(const char *name)
+{
+}
 #endif
 
 #endif
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 32/42] commands: implement keystore command
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (30 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 31/42] keystore: implement forgetting secrets Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 33/42] commands: state: allow loading state with -l Sascha Hauer
                   ` (10 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

The keystore command provides access to the barebox keystore.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/Kconfig    |   6 ++++
 commands/Makefile   |   1 +
 commands/keystore.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+)
 create mode 100644 commands/keystore.c

diff --git a/commands/Kconfig b/commands/Kconfig
index bc0885c69d..6bb47d6363 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -1967,6 +1967,12 @@ config CMD_FIRMWARELOAD
 	  Provides the "firmwareload" command which deals with devices which need
 	  firmware to work. It is also used to upload firmware to FPGA devices.
 
+config CMD_KEYSTORE
+	depends on CRYPTO_KEYSTORE
+	bool
+	prompt "keystore"
+	help
+	  keystore provides access to the barebox keystore.
 
 config CMD_LINUX_EXEC
 	bool "linux exec"
diff --git a/commands/Makefile b/commands/Makefile
index 601f15fc38..a20c675929 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -92,6 +92,7 @@ obj-$(CONFIG_CMD_READLINK)	+= readlink.o
 obj-$(CONFIG_CMD_LET)		+= let.o
 obj-$(CONFIG_CMD_LN)		+= ln.o
 obj-$(CONFIG_CMD_CLK)		+= clk.o
+obj-$(CONFIG_CMD_KEYSTORE)	+= keystore.o
 obj-$(CONFIG_CMD_TFTP)		+= tftp.o
 obj-$(CONFIG_CMD_FILETYPE)	+= filetype.o
 obj-$(CONFIG_CMD_BAREBOX_UPDATE)+= barebox-update.o
diff --git a/commands/keystore.c b/commands/keystore.c
new file mode 100644
index 0000000000..52c4be2639
--- /dev/null
+++ b/commands/keystore.c
@@ -0,0 +1,100 @@
+#include <common.h>
+#include <command.h>
+#include <getopt.h>
+#include <libfile.h>
+#include <crypto/keystore.h>
+#include <linux/kernel.h>
+#include <fs.h>
+
+static int do_keystore(int argc, char *argv[])
+{
+	int opt;
+	int ret;
+	int do_remove = 0;
+	const char *name;
+	const char *file = NULL;
+	char *secret_str = NULL;
+	void *secret;
+	int s_len;
+
+	while ((opt = getopt(argc, argv, "rs:f:")) > 0) {
+		switch (opt) {
+		case 'r':
+			do_remove = 1;
+			break;
+		case 's':
+			secret_str = optarg;
+			break;
+		case 'f':
+			file = optarg;
+			break;
+		default:
+			return COMMAND_ERROR_USAGE;
+		}
+	}
+
+	if (argc == optind)
+		return COMMAND_ERROR_USAGE;
+
+	if (!do_remove && !file && !secret_str)
+		return COMMAND_ERROR_USAGE;
+
+	if (file && secret_str)
+		return COMMAND_ERROR_USAGE;
+
+	name = argv[optind];
+
+	if (do_remove) {
+		keystore_forget_secret(name);
+		printf("forgotten secret for key %s\n", name);
+		return 0;
+	}
+
+	if (file) {
+		ret = read_file_2(file, &s_len, (void *)&secret_str, FILESIZE_MAX);
+		if (ret) {
+			printf("Cannot open %s: %s\n", file, strerror(-ret));
+			return 1;
+		}
+	} else if (secret_str) {
+		s_len = strlen(secret_str);
+	}
+
+	if (s_len & 1) {
+		printf("invalid secret len. Must be whole bytes\n");
+		return 1;
+	}
+
+	secret = xzalloc(s_len / 2);
+	ret = hex2bin(secret, secret_str, s_len / 2);
+	if (ret) {
+		printf("Cannot convert %s to binary: %s\n", secret_str, strerror(-ret));
+		return 1;
+	}
+
+	ret = keystore_set_secret(name, secret, s_len / 2);
+	if (ret)
+		printf("cannot set secret for key %s: %s\n", name, strerror(-ret));
+	else
+		printf("Added secret for key %s\n", name);
+
+	free(secret);
+
+	return ret ? 1 : 0;
+}
+
+BAREBOX_CMD_HELP_START(keystore)
+BAREBOX_CMD_HELP_TEXT("")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT("-r", "remove a key from the keystore")
+BAREBOX_CMD_HELP_OPT("-s <key>", "set a key in the keystore")
+BAREBOX_CMD_HELP_OPT("-f <keyfile>", "set a key in the keystore, read secret from file")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(keystore)
+	.cmd	= do_keystore,
+	BAREBOX_CMD_DESC("manage keys")
+	BAREBOX_CMD_OPTS("[-rsf] <keyname>")
+	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
+	BAREBOX_CMD_HELP(cmd_keystore_help)
+BAREBOX_CMD_END
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 33/42] commands: state: allow loading state with -l
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (31 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 32/42] commands: implement keystore command Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 34/42] crypto: digest: initialize earlier Sascha Hauer
                   ` (9 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

At least for testing purposes it's useful to be able to
manually load a state. Add -l option for this.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/state.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/commands/state.c b/commands/state.c
index 4b51759e3e..aded6e71e2 100644
--- a/commands/state.c
+++ b/commands/state.c
@@ -21,20 +21,23 @@ static int do_state(int argc, char *argv[])
 {
 	int opt, ret = 0;
 	struct state *state = NULL;
-	int do_save = 0;
+	int do_save = 0, do_load = 0;
 	const char *statename = "state";
 
-	while ((opt = getopt(argc, argv, "s")) > 0) {
+	while ((opt = getopt(argc, argv, "sl")) > 0) {
 		switch (opt) {
 		case 's':
 			do_save = 1;
 			break;
+		case 'l':
+			do_load = 1;
+			break;
 		default:
 			return COMMAND_ERROR_USAGE;
 		}
 	}
 
-	if (!do_save) {
+	if (!do_save && !do_load) {
 		state_info();
 		return 0;
 	}
@@ -48,7 +51,9 @@ static int do_state(int argc, char *argv[])
 		return -ENOENT;
 	}
 
-	if (do_save)
+	if (do_load)
+		ret = state_load(state);
+	else if (do_save)
 		ret = state_save(state);
 
 	return ret;
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 34/42] crypto: digest: initialize earlier
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (32 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 33/42] commands: state: allow loading state with -l Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 35/42] state: backend_raw: alloc digest only when needed Sascha Hauer
                   ` (8 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

Digests have dependencies and are needed for state which initializes
at device_initcall level. To make sure the digests are available
for state register them earlier, at coredevice_initcall level.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 crypto/hmac.c | 2 +-
 crypto/sha1.c | 2 +-
 crypto/sha2.c | 2 +-
 crypto/sha4.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/crypto/hmac.c b/crypto/hmac.c
index 05b9b50f12..37b270df7a 100644
--- a/crypto/hmac.c
+++ b/crypto/hmac.c
@@ -196,4 +196,4 @@ static int digest_hmac_initcall(void)
 
 	return 0;
 }
-crypto_initcall(digest_hmac_initcall);
+coredevice_initcall(digest_hmac_initcall);
diff --git a/crypto/sha1.c b/crypto/sha1.c
index cbde4d28e4..f4b2ded0b0 100644
--- a/crypto/sha1.c
+++ b/crypto/sha1.c
@@ -303,4 +303,4 @@ static int sha1_digest_register(void)
 {
 	return digest_algo_register(&m);
 }
-device_initcall(sha1_digest_register);
+coredevice_initcall(sha1_digest_register);
diff --git a/crypto/sha2.c b/crypto/sha2.c
index cb0f11c77e..c62ddb8d25 100644
--- a/crypto/sha2.c
+++ b/crypto/sha2.c
@@ -372,4 +372,4 @@ static int sha256_digest_register(void)
 
 	return digest_algo_register(&m256);
 }
-device_initcall(sha256_digest_register);
+coredevice_initcall(sha256_digest_register);
diff --git a/crypto/sha4.c b/crypto/sha4.c
index 4ce37b73e4..aad8081fa5 100644
--- a/crypto/sha4.c
+++ b/crypto/sha4.c
@@ -292,4 +292,4 @@ static int sha512_digest_register(void)
 
 	return digest_algo_register(&m512);
 }
-device_initcall(sha512_digest_register);
+coredevice_initcall(sha512_digest_register);
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 35/42] state: backend_raw: alloc digest only when needed
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (33 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 34/42] crypto: digest: initialize earlier Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 36/42] state: backend_circular: Set minumum writesize to 8 Sascha Hauer
                   ` (7 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

Instead of deferring probe just allocate the digest when
it's needed. This way the credentials can be added later,
possibly on the commandline.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_format_raw.c | 112 ++++++++++++++++++++------------------
 1 file changed, 59 insertions(+), 53 deletions(-)

diff --git a/common/state/backend_format_raw.c b/common/state/backend_format_raw.c
index a425b636d0..43a5523152 100644
--- a/common/state/backend_format_raw.c
+++ b/common/state/backend_format_raw.c
@@ -35,6 +35,10 @@ struct state_backend_format_raw {
 
 	/* For outputs */
 	struct device_d *dev;
+
+	char *secret_name;
+	int needs_secret;
+	char *algo;
 };
 
 struct __attribute__((__packed__)) backend_raw_header {
@@ -53,6 +57,43 @@ static inline struct state_backend_format_raw *get_format_raw(
 	return container_of(format, struct state_backend_format_raw, format);
 }
 
+static int backend_raw_digest_init(struct state_backend_format_raw *raw)
+{
+	const unsigned char *key;
+	int key_len;
+	int ret;
+
+	if (!raw->digest) {
+		raw->digest = digest_alloc(raw->algo);
+		if (!raw->digest) {
+			dev_err(raw->dev, "algo %s not found\n",
+				raw->algo);
+			return -ENODEV;
+		}
+		raw->digest_length = digest_length(raw->digest);
+	}
+
+	ret = keystore_get_secret(raw->secret_name, &key, &key_len);
+	if (ret) {
+		dev_err(raw->dev, "Could not get secret '%s'\n",
+			raw->secret_name);
+		return ret;
+	}
+
+	ret = digest_set_key(raw->digest, key, key_len);
+	if (ret)
+		return ret;
+
+	ret = digest_init(raw->digest);
+	if (ret) {
+		dev_err(raw->dev, "Failed to initialize digest: %s\n",
+			strerror(-ret));
+		return ret;
+	}
+
+	return 0;
+}
+
 static int backend_format_raw_verify(struct state_backend_format *format,
 				     uint32_t magic, const void * buf,
 				     ssize_t *lenp)
@@ -86,7 +127,11 @@ static int backend_format_raw_verify(struct state_backend_format *format,
 		return -EINVAL;
 	}
 
-	if (backend_raw->digest) {
+	if (backend_raw->algo) {
+		ret = backend_raw_digest_init(backend_raw);
+		if (ret)
+			return ret;
+
 		d_len = digest_length(backend_raw->digest);
 	}
 
@@ -108,26 +153,18 @@ static int backend_format_raw_verify(struct state_backend_format *format,
 
 	*lenp = header->data_len + sizeof(*header);
 
-	if (backend_raw->digest) {
-		struct digest *d = backend_raw->digest;
+	if (backend_raw->algo) {
 		const void *hmac = data + header->data_len;
 
-		ret = digest_init(d);
-		if (ret) {
-			dev_err(backend_raw->dev, "Failed to initialize digest, %d\n",
-				ret);
-			return ret;
-		}
-
 		/* hmac over header and data */
-		ret = digest_update(d, buf, sizeof(*header) + header->data_len);
+		ret = digest_update(backend_raw->digest, buf, sizeof(*header) + header->data_len);
 		if (ret) {
 			dev_err(backend_raw->dev, "Failed to update digest, %d\n",
 				ret);
 			return ret;
 		}
 
-		ret = digest_verify(d, hmac);
+		ret = digest_verify(backend_raw->digest, hmac);
 		if (ret < 0) {
 			dev_err(backend_raw->dev, "Failed to verify data, hmac, %d\n",
 				ret);
@@ -195,25 +232,20 @@ static int backend_format_raw_pack(struct state_backend_format *format,
 	header->header_crc = crc32(0, header,
 				   sizeof(*header) - sizeof(uint32_t));
 
-	if (backend_raw->digest) {
-		struct digest *d = backend_raw->digest;
-
-		ret = digest_init(d);
-		if (ret) {
-			dev_err(backend_raw->dev, "Failed to initialize digest for packing, %d\n",
-				ret);
-			goto out_free;
-		}
+	if (backend_raw->algo) {
+		ret = backend_raw_digest_init(backend_raw);
+		if (ret)
+			return ret;
 
 		/* hmac over header and data */
-		ret = digest_update(d, buf, sizeof(*header) + size_data);
+		ret = digest_update(backend_raw->digest, buf, sizeof(*header) + size_data);
 		if (ret) {
 			dev_err(backend_raw->dev, "Failed to update digest for packing, %d\n",
 				ret);
 			goto out_free;
 		}
 
-		ret = digest_final(d, hmac);
+		ret = digest_final(backend_raw->digest, hmac);
 		if (ret < 0) {
 			dev_err(backend_raw->dev, "Failed to finish digest for packing, %d\n",
 				ret);
@@ -243,11 +275,9 @@ static int backend_format_raw_init_digest(struct state_backend_format_raw *raw,
 					  struct device_node *root,
 					  const char *secret_name)
 {
-	struct digest *digest;
 	struct property *p;
 	const char *algo;
-	const unsigned char *key;
-	int key_len, ret;
+	int ret;
 
 	p = of_find_property(root, "algo", NULL);
 	if (!p)			/* does not exist */
@@ -263,30 +293,7 @@ static int backend_format_raw_init_digest(struct state_backend_format_raw *raw,
 		return -EINVAL;
 	}
 
-	ret = keystore_get_secret(secret_name, &key, &key_len);
-	if (ret == -ENOENT) {	/* -ENOENT == does not exist */
-		dev_info(raw->dev, "Could not get secret '%s' - probe deferred\n",
-			 secret_name);
-		return -EPROBE_DEFER;
-	} else if (ret) {
-		return ret;
-	}
-
-	digest = digest_alloc(algo);
-	if (!digest) {
-		dev_info(raw->dev, "algo %s not found - probe deferred\n",
-			 algo);
-		return -EPROBE_DEFER;
-	}
-
-	ret = digest_set_key(digest, key, key_len);
-	if (ret) {
-		digest_free(digest);
-		return ret;
-	}
-
-	raw->digest = digest;
-	raw->digest_length = digest_length(digest);
+	raw->algo = xstrdup(algo);
 
 	return 0;
 }
@@ -304,15 +311,14 @@ int backend_format_raw_create(struct state_backend_format **format,
 
 	raw->dev = dev;
 	ret = backend_format_raw_init_digest(raw, node, secret_name);
-	if (ret == -EPROBE_DEFER) {
-		return ret;
-	} else if (ret) {
+	if (ret) {
 		dev_err(raw->dev, "Failed initializing digest for raw format, %d\n",
 			ret);
 		free(raw);
 		return ret;
 	}
 
+	raw->secret_name = xstrdup(secret_name);
 	raw->format.pack = backend_format_raw_pack;
 	raw->format.unpack = backend_format_raw_unpack;
 	raw->format.verify = backend_format_raw_verify;
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 36/42] state: backend_circular: Set minumum writesize to 8
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (34 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 35/42] state: backend_raw: alloc digest only when needed Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 37/42] state: backend bucket circular: Explain metadata Sascha Hauer
                   ` (6 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

NOR flashes have a write size of 1. With this the metadata may
end up on non-4-byte-aligned offsets. Force the minimum writesize
to 8 so that the metadata is always at aligned offsets.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_bucket_circular.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/common/state/backend_bucket_circular.c b/common/state/backend_bucket_circular.c
index 58fffd18e3..5a525f68e1 100644
--- a/common/state/backend_bucket_circular.c
+++ b/common/state/backend_bucket_circular.c
@@ -475,6 +475,9 @@ int state_backend_bucket_circular_create(struct device_d *dev, const char *path,
 	struct state_backend_storage_bucket_circular *circ;
 	int ret;
 
+	if (writesize < 8)
+		writesize = 8;
+
 	circ = xzalloc(sizeof(*circ));
 	circ->eraseblock = eraseblock;
 	circ->writesize = writesize;
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 37/42] state: backend bucket circular: Explain metadata
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (35 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 36/42] state: backend_circular: Set minumum writesize to 8 Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 38/42] state: Allow to load without authentification Sascha Hauer
                   ` (5 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

Explain why we have metadata and where it is used.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_bucket_circular.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/common/state/backend_bucket_circular.c b/common/state/backend_bucket_circular.c
index 5a525f68e1..d99ba39126 100644
--- a/common/state/backend_bucket_circular.c
+++ b/common/state/backend_bucket_circular.c
@@ -61,6 +61,11 @@ struct state_backend_storage_bucket_circular {
 	struct device_d *dev;
 };
 
+/*
+ * The metadata will be written directly before writesize aligned offsets.
+ * When searching backwards through the pages it allows us to find the
+ * beginning of the data.
+ */
 struct __attribute__((__packed__)) state_backend_storage_bucket_circular_meta {
 	uint32_t magic;
 	uint32_t written_length;
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 38/42] state: Allow to load without authentification
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (36 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 37/42] state: backend bucket circular: Explain metadata Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 39/42] state: Update documentation Sascha Hauer
                   ` (4 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

Sometimes it's useful to be able to load a state even when it
can't be authentificated. Add an option for this.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/state.c                  | 16 ++++++++++++----
 common/state/backend_format_dtb.c |  4 ++--
 common/state/backend_format_raw.c |  6 +++---
 common/state/backend_storage.c    |  6 ++++--
 common/state/state.c              | 14 ++++++++++++--
 common/state/state.h              |  9 +++++++--
 include/state.h                   |  1 +
 7 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/commands/state.c b/commands/state.c
index aded6e71e2..c57a906ff0 100644
--- a/commands/state.c
+++ b/commands/state.c
@@ -23,8 +23,9 @@ static int do_state(int argc, char *argv[])
 	struct state *state = NULL;
 	int do_save = 0, do_load = 0;
 	const char *statename = "state";
+	int no_auth = 0;
 
-	while ((opt = getopt(argc, argv, "sl")) > 0) {
+	while ((opt = getopt(argc, argv, "sln")) > 0) {
 		switch (opt) {
 		case 's':
 			do_save = 1;
@@ -32,6 +33,9 @@ static int do_state(int argc, char *argv[])
 		case 'l':
 			do_load = 1;
 			break;
+		case 'n':
+			no_auth = 1;
+			break;
 		default:
 			return COMMAND_ERROR_USAGE;
 		}
@@ -51,10 +55,14 @@ static int do_state(int argc, char *argv[])
 		return -ENOENT;
 	}
 
-	if (do_load)
-		ret = state_load(state);
-	else if (do_save)
+	if (do_load) {
+		if (no_auth)
+			ret = state_load_no_auth(state);
+		else
+			ret = state_load(state);
+	} else if (do_save) {
 		ret = state_save(state);
+	}
 
 	return ret;
 }
diff --git a/common/state/backend_format_dtb.c b/common/state/backend_format_dtb.c
index d7b01729bc..55fa1fc597 100644
--- a/common/state/backend_format_dtb.c
+++ b/common/state/backend_format_dtb.c
@@ -40,7 +40,7 @@ static inline struct state_backend_format_dtb *get_format_dtb(struct
 
 static int state_backend_format_dtb_verify(struct state_backend_format *format,
 					   uint32_t magic, const void * buf,
-					   ssize_t *lenp)
+					   ssize_t *lenp, enum state_flags flags)
 {
 	struct state_backend_format_dtb *fdtb = get_format_dtb(format);
 	struct device_node *root;
@@ -81,7 +81,7 @@ static int state_backend_format_dtb_unpack(struct state_backend_format *format,
 	int ret;
 
 	if (!fdtb->root) {
-		state_backend_format_dtb_verify(format, 0, buf, &len);
+		state_backend_format_dtb_verify(format, 0, buf, &len, 0);
 	}
 
 	ret = state_from_node(state, fdtb->root, 0);
diff --git a/common/state/backend_format_raw.c b/common/state/backend_format_raw.c
index 43a5523152..232856a209 100644
--- a/common/state/backend_format_raw.c
+++ b/common/state/backend_format_raw.c
@@ -96,7 +96,7 @@ static int backend_raw_digest_init(struct state_backend_format_raw *raw)
 
 static int backend_format_raw_verify(struct state_backend_format *format,
 				     uint32_t magic, const void * buf,
-				     ssize_t *lenp)
+				     ssize_t *lenp, enum state_flags flags)
 {
 	uint32_t crc;
 	struct backend_raw_header *header;
@@ -127,7 +127,7 @@ static int backend_format_raw_verify(struct state_backend_format *format,
 		return -EINVAL;
 	}
 
-	if (backend_raw->algo) {
+	if (backend_raw->algo && !(flags & STATE_FLAG_NO_AUTHENTIFICATION)) {
 		ret = backend_raw_digest_init(backend_raw);
 		if (ret)
 			return ret;
@@ -153,7 +153,7 @@ static int backend_format_raw_verify(struct state_backend_format *format,
 
 	*lenp = header->data_len + sizeof(*header);
 
-	if (backend_raw->algo) {
+	if (backend_raw->algo && !(flags & STATE_FLAG_NO_AUTHENTIFICATION)) {
 		const void *hmac = data + header->data_len;
 
 		/* hmac over header and data */
diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index a5688aa424..9ed6ad79ac 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -127,6 +127,7 @@ refresh:
  * @param magic state magic value
  * @param buf The newly allocated data area will be stored in this pointer
  * @param len The resulting length of the buffer
+ * @param flags flags controlling how to load state
  * @return 0 on success, -errno otherwise. buf and len will be set to valid
  * values on success.
  *
@@ -137,7 +138,8 @@ refresh:
  */
 int state_storage_read(struct state_backend_storage *storage,
 		       struct state_backend_format *format,
-		       uint32_t magic, void **buf, ssize_t *len)
+		       uint32_t magic, void **buf, ssize_t *len,
+		       enum state_flags flags)
 {
 	struct state_backend_storage_bucket *bucket, *bucket_used = NULL;
 	int ret;
@@ -157,7 +159,7 @@ int state_storage_read(struct state_backend_storage *storage,
 		 * Verify the buffer crcs. The buffer length is passed in the len argument,
 		 * .verify overwrites it with the length actually used.
 		 */
-		ret = format->verify(format, magic, bucket->buf, &bucket->len);
+		ret = format->verify(format, magic, bucket->buf, &bucket->len, flags);
 		if (!ret && !bucket_used)
 			bucket_used = bucket;
 	}
diff --git a/common/state/state.c b/common/state/state.c
index 476e3ee08e..1232ff3207 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -78,14 +78,14 @@ out:
  * we read is checked for integrity by the formatter. After that we unpack the
  * data into our state.
  */
-int state_load(struct state *state)
+static int state_do_load(struct state *state, enum state_flags flags)
 {
 	void *buf;
 	ssize_t len;
 	int ret;
 
 	ret = state_storage_read(&state->storage, state->format,
-				 state->magic, &buf, &len);
+				 state->magic, &buf, &len, flags);
 	if (ret) {
 		dev_err(&state->dev, "Failed to read state with format %s, %d\n",
 			state->format->name, ret);
@@ -106,6 +106,16 @@ out:
 	return ret;
 }
 
+int state_load(struct state *state)
+{
+	return state_do_load(state, 0);
+}
+
+int state_load_no_auth(struct state *state)
+{
+	return state_do_load(state, STATE_FLAG_NO_AUTHENTIFICATION);
+}
+
 static int state_format_init(struct state *state, const char *backend_format,
 			     struct device_node *node, const char *state_name)
 {
diff --git a/common/state/state.h b/common/state/state.h
index a2737abeb6..5e240c4773 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -5,6 +5,10 @@
 struct state;
 struct mtd_info_user;
 
+enum state_flags {
+	STATE_FLAG_NO_AUTHENTIFICATION = (1 << 0),
+};
+
 /**
  * state_backend_storage_bucket - This class describes a single backend storage
  * object copy
@@ -53,7 +57,7 @@ struct state_backend_storage_bucket {
  */
 struct state_backend_format {
 	int (*verify) (struct state_backend_format * format, uint32_t magic,
-		       const void * buf, ssize_t *lenp);
+		       const void * buf, ssize_t *lenp, enum state_flags flags);
 	int (*pack) (struct state_backend_format * format, struct state * state,
 		     void ** buf, ssize_t * len);
 	int (*unpack) (struct state_backend_format * format,
@@ -223,7 +227,8 @@ int state_storage_write(struct state_backend_storage *storage,
 			const void * buf, ssize_t len);
 int state_storage_read(struct state_backend_storage *storage,
 		       struct state_backend_format *format,
-		       uint32_t magic, void **buf, ssize_t *len);
+		       uint32_t magic, void **buf, ssize_t *len,
+		       enum state_flags flags);
 
 static inline struct state_uint32 *to_state_uint32(struct state_variable *s)
 {
diff --git a/include/state.h b/include/state.h
index bc9a574093..63164f92e5 100644
--- a/include/state.h
+++ b/include/state.h
@@ -18,6 +18,7 @@ struct state *state_by_name(const char *name);
 struct state *state_by_node(const struct device_node *node);
 int state_get_name(const struct state *state, char const **name);
 
+int state_load_no_auth(struct state *state);
 int state_load(struct state *state);
 int state_save(struct state *state);
 void state_info(void);
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 39/42] state: Update documentation
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (37 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 38/42] state: Allow to load without authentification Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 40/42] state: Do not load state during state_new_from_node Sascha Hauer
                   ` (3 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

- explain what buckets are
- rework text about storage backends
- explain redundancy concept

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/user/state.rst | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/Documentation/user/state.rst b/Documentation/user/state.rst
index 5dd5c486e2..73c4be8159 100644
--- a/Documentation/user/state.rst
+++ b/Documentation/user/state.rst
@@ -23,16 +23,35 @@ available, ``raw`` and ``dtb``. Both format the state data differently.
 Basically these are serializers. The raw serializer additionally supports a
 HMAC algorithm to detect manipulations.
 
+The data is always stored in a logical unit called ``bucket``. A ``bucket`` has
+its own size depending on some external contraints. These contraints are listed
+in more detail below depending on the used memory type and storage backend. A
+``bucket`` stores exactly one state. A default number of three buckets is used
+to store data redundantely.
+
+Redundancy
+----------
+
+The state framework is safe against powerfailures during write operations. To
+archieve that multiple buckets are stored to disk. When writing all buckets are
+written in order. When reading, the buckets are read in order and the first
+one found that passes CRC tests is used. When all data is read the buckets
+containing invalid or outdated data are written with the data just read. Also
+NAND blocks need cleanup due to excessive bitflips are rewritten in this step.
+With this it is made sure that after successful initialization of a state the
+data on the storage device is consistent and redundant.
+
 Storage Backends
 ----------------
 
-The serialized data can be stored to different backends which are automatically
-selected depending on the defined backend in the devicetree. Currently two
-implementations exist, ``circular`` and ``direct``. ``circular`` writes the
-data sequentially on the backend storage device. Each save is appended until
-the storage area is full. It then erases the block and starts from offset 0.
-``circular`` is used for MTD devices with erase functionality. ``direct``
-writes the data directly to the file without erasing.
+The serialized data can be stored to different backends. Currently two
+implementations exist, ``circular`` and ``direct``. The state framework automatically
+selects the correct backend depending on the storage medium. Media requiring
+erase operations (NAND, NOR flash) use the ``circular`` backend, others use the ``direct``
+backend. The purpose of the ``circular`` backend is to save erase cycles which may
+wear out the flash blocks. It continuously fills eraseblocks with updated data
+and only when an eraseblock if fully written erases it and starts over writing
+new data to the same eraseblock again.
 
 For all backends multiple copies are written to handle read errors.
 
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 40/42] state: Do not load state during state_new_from_node
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (38 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 39/42] state: Update documentation Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 41/42] state: Remove -EUCLEAN check from userspace tool Sascha Hauer
                   ` (2 subsequent siblings)
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

The caller of state_new_from_node() may have it's own ideas what to
do when loading the state fails, so do not load it in the initialization
function, but instead let the caller do it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/state.c | 5 -----
 drivers/misc/state.c | 6 ++++++
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/common/state/state.c b/common/state/state.c
index 1232ff3207..bdeda2e5a1 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -632,11 +632,6 @@ struct state *state_new_from_node(struct device_node *node, char *path,
 		goto out_release_state;
 	}
 
-	ret = state_load(state);
-	if (ret) {
-		dev_warn(&state->dev, "Failed to load persistent state, continuing with defaults, %d\n", ret);
-	}
-
 	dev_info(&state->dev, "New state registered '%s'\n", alias);
 
 	return state;
diff --git a/drivers/misc/state.c b/drivers/misc/state.c
index b43aee60fe..98ed42e757 100644
--- a/drivers/misc/state.c
+++ b/drivers/misc/state.c
@@ -26,6 +26,7 @@ static int state_probe(struct device_d *dev)
 	struct device_node *np = dev->device_node;
 	struct state *state;
 	bool readonly = false;
+	int ret;
 
 	state = state_new_from_node(np, NULL, 0, 0, readonly);
 	if (IS_ERR(state)) {
@@ -35,6 +36,11 @@ static int state_probe(struct device_d *dev)
 		return ret;
 	}
 
+	ret = state_load(state);
+	if (ret)
+		dev_warn(dev, "Failed to load persistent state, continuing with defaults, %d\n",
+			 ret);
+
 	return 0;
 }
 
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 41/42] state: Remove -EUCLEAN check from userspace tool
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (39 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 40/42] state: Do not load state during state_new_from_node Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-03-31  7:03 ` [PATCH 42/42] state: find device node from device path, not from device node path Sascha Hauer
  2017-04-03 20:15 ` State patches Sam Ravnborg
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

The state code is used for the userspace tool aswell, kept in sync
manually. This patch only introduces a change for the userspace
tool, not for barebox.

In Linux userspace there is no direct possibility to check for -EUCLEAN.
To indirectly check for -EUCLEAN the state tool reads the number of
corrected bits before and after reading a block. Unfortunately it does
not take the number of acceptable bitflips into account, but instead
returns -EUCLEAN even when only a single bitflip occurred on a whole
page. To be correct the algorithm must be more complicated: We would
have to read the bitflip_threshold from sysfs. This value is per ECC
step (often 512 byte), not per page. We would have to read the page
in ECC step size chunks, testing for bitflips lower than the threshold
for each chunk. Even if we would do that, there's still another issue.
The eccstats ioctl delivers the eccstats for the whole device, so a
concurrent reader would falsify the result.

Let's decide that this is not worth the hassle and assume that no
device has enough uptime that a cleanup in barebox is not sufficient.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/backend_bucket_circular.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/common/state/backend_bucket_circular.c b/common/state/backend_bucket_circular.c
index d99ba39126..5279ec9ce2 100644
--- a/common/state/backend_bucket_circular.c
+++ b/common/state/backend_bucket_circular.c
@@ -171,33 +171,12 @@ static int state_mtd_peb_read(struct state_backend_storage_bucket_circular *circ
 	dev_dbg(circ->dev, "Read state from %ld length %zd\n", offset,
 		len);
 
-	ret = ioctl(circ->fd, ECCGETSTATS, &stat1);
-	if (ret)
-		nostats = true;
 
 	ret = read_full(circ->fd, buf, len);
 	if (ret < 0) {
 		dev_err(circ->dev, "Failed to read circular storage len %zd, %d\n",
 			len, ret);
 		free(buf);
-		return ret;
-	}
-
-	if (nostats)
-		return 0;
-
-	ret = ioctl(circ->fd, ECCGETSTATS, &stat2);
-	if (ret)
-		return 0;
-
-	if (stat2.failed - stat1.failed > 0) {
-		ret = -EUCLEAN;
-		dev_dbg(circ->dev, "PEB %u has ECC error, forcing rewrite\n",
-			circ->eraseblock);
-	} else if (stat2.corrected - stat1.corrected > 0) {
-		ret = -EUCLEAN;
-		dev_dbg(circ->dev, "PEB %u is unclean, forcing rewrite\n",
-			circ->eraseblock);
 	}
 
 	return ret;
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* [PATCH 42/42] state: find device node from device path, not from device node path
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (40 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 41/42] state: Remove -EUCLEAN check from userspace tool Sascha Hauer
@ 2017-03-31  7:03 ` Sascha Hauer
  2017-04-03 20:15 ` State patches Sam Ravnborg
  42 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-03-31  7:03 UTC (permalink / raw)
  To: Barebox List

The device node path may change from the internal device tree to the
one Linux is started with, so using this path to fixup the tree is
not very robust. Instead, use of_find_node_by_devpath() which has
been created for exactly this purpose.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/state/state.c | 10 +++++-----
 common/state/state.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/common/state/state.c b/common/state/state.c
index bdeda2e5a1..94a423b825 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -461,7 +461,7 @@ static int of_state_fixup(struct device_node *root, void *ctx)
 	}
 
 	/* backend phandle */
-	backend_node = of_find_node_by_path_from(root, state->of_backend_path);
+	backend_node = of_find_node_by_devpath(root, state->backend_path);
 	if (!backend_node) {
 		ret = -ENODEV;
 		goto out;
@@ -529,7 +529,7 @@ void state_release(struct state *state)
 	unregister_device(&state->dev);
 	state_storage_free(&state->storage);
 	state_format_free(state->format);
-	free(state->of_backend_path);
+	free(state->backend_path);
 	free(state->of_path);
 	free(state);
 }
@@ -591,6 +591,8 @@ struct state *state_new_from_node(struct device_node *node, char *path,
 		}
 	}
 
+	state->backend_path = xstrdup(path);
+
 	ret = of_property_read_string(node, "backend-type", &backend_type);
 	if (ret) {
 		goto out_release_state;
@@ -617,8 +619,6 @@ struct state *state_new_from_node(struct device_node *node, char *path,
 	if (ret)
 		goto out_release_state;
 
-	state->of_backend_path = xstrdup(of_path);
-
 	if (readonly)
 		state_backend_set_readonly(state);
 
@@ -693,7 +693,7 @@ void state_info(void)
 		if (state->format)
 			printf("(backend: %s, path: %s)\n",
 			       state->format->name,
-			       state->of_backend_path);
+			       state->backend_path);
 		else
 			printf("(no backend)\n");
 	}
diff --git a/common/state/state.h b/common/state/state.h
index 5e240c4773..ead8cc88c1 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -104,7 +104,7 @@ struct state {
 
 	struct state_backend_format *format;
 	struct state_backend_storage storage;
-	char *of_backend_path;
+	char *backend_path;
 };
 
 enum state_convert {
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* Re: State patches
  2017-03-31  7:03 State patches Sascha Hauer
                   ` (41 preceding siblings ...)
  2017-03-31  7:03 ` [PATCH 42/42] state: find device node from device path, not from device node path Sascha Hauer
@ 2017-04-03 20:15 ` Sam Ravnborg
  2017-04-04  6:19   ` Sascha Hauer
  42 siblings, 1 reply; 52+ messages in thread
From: Sam Ravnborg @ 2017-04-03 20:15 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

Hi Sasha.

On Fri, Mar 31, 2017 at 09:03:04AM +0200, Sascha Hauer wrote:
> Hi All,
> 
> Here is a ton of patches working on the state framework:
> 
> - make code easier to follow
>   - Drop cached backend, replace with open coded more direct code
>   - drop backend as extra struct type
>   - drop lazy init code
> - update documentation
> - Add keystore command
>   Using authenticated state requires hardware support which currently
>   is only available for i.MX6. At least for testing purposes a keystore
>   command to set keys is useful. This way authenticated state can be
>   tested without hardware support
> - more robust conversion of the backend path to a device node
> - make work better with NOR flash: The current code ended up reading
>   from NOR Flash byte by byte which was painfully slow

From browsing the patches this also brings a nice boot time improvement.
We avoid earsing the whole block (or whatever the right name is) until
we have filled it up with state entries.
I expect this tospeed up booting with at least a second on our target.

Will this require a newer version of dt-utils to work?
(When we for example update the sate from linux userspace)

	Sam

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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* Re: State patches
  2017-04-03 20:15 ` State patches Sam Ravnborg
@ 2017-04-04  6:19   ` Sascha Hauer
  0 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-04-04  6:19 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: Barebox List

Hi Sam,

On Mon, Apr 03, 2017 at 10:15:11PM +0200, Sam Ravnborg wrote:
> Hi Sasha.
> 
> On Fri, Mar 31, 2017 at 09:03:04AM +0200, Sascha Hauer wrote:
> > Hi All,
> > 
> > Here is a ton of patches working on the state framework:
> > 
> > - make code easier to follow
> >   - Drop cached backend, replace with open coded more direct code
> >   - drop backend as extra struct type
> >   - drop lazy init code
> > - update documentation
> > - Add keystore command
> >   Using authenticated state requires hardware support which currently
> >   is only available for i.MX6. At least for testing purposes a keystore
> >   command to set keys is useful. This way authenticated state can be
> >   tested without hardware support
> > - more robust conversion of the backend path to a device node
> > - make work better with NOR flash: The current code ended up reading
> >   from NOR Flash byte by byte which was painfully slow
> 
> From browsing the patches this also brings a nice boot time improvement.
> We avoid earsing the whole block (or whatever the right name is) until
> we have filled it up with state entries.
> I expect this tospeed up booting with at least a second on our target.
> 
> Will this require a newer version of dt-utils to work?
> (When we for example update the sate from linux userspace)

No new tools required, the binary format is still the same.

The circular format which fills up blocks before erasing them existed before
this series, but this format is the default now. So if you end up with
the new format because you haven't specified the format before, then
this format change is still passed to userspace in the device tree and
and the userspace utility will do the right thing.

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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* Re: [PATCH 11/42] state: backend_circular: Read whole PEB
  2017-03-31  7:03 ` [PATCH 11/42] state: backend_circular: Read whole PEB Sascha Hauer
@ 2017-04-15  8:40   ` Sam Ravnborg
  0 siblings, 0 replies; 52+ messages in thread
From: Sam Ravnborg @ 2017-04-15  8:40 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On Fri, Mar 31, 2017 at 09:03:15AM +0200, Sascha Hauer wrote:
> When the circular backend searches for the last page written in the
> eraseblock, it iterates backwards pagewise from the end of the block.
> This is ok for NAND flash, but on NOR flash, which does not have pages,
> the code ends up iterating bytewise backwards, calling into mtd each
> time. This is very time consuming, so optimize this by reading the whole
> eraseblock once and just iterate over the buffer in memory.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  common/state/backend_bucket_circular.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/common/state/backend_bucket_circular.c b/common/state/backend_bucket_circular.c
> index 0bce900d0d..53c2aae803 100644
> --- a/common/state/backend_bucket_circular.c
> +++ b/common/state/backend_bucket_circular.c
> @@ -379,26 +379,24 @@ static int state_backend_bucket_circular_init(
>  	int sub_offset;
>  	uint32_t written_length = 0;
>  	uint8_t *buf;
> +	int ret;
>  
> -	buf = xmalloc(circ->writesize);
> +	buf = xmalloc(circ->max_size);
>  	if (!buf)
>  		return -ENOMEM;
>  
> +	ret = state_mtd_peb_read(circ, buf, 0, circ->max_size);
> +	if (ret && ret != -EUCLEAN)
> +		return ret;

We leak buf here.
But ownership of buf is vague.

In one implementation of state_mtd_peb_read() we free buf
in case of errors, but not in all paths.

This is the version of state_mtd_peb_read() outside the

   #ifdef __BAREBOX__

block.

The version of state_mtd_peb_read() inside the

   #ifdef __BAREBOX__

will not free buf, so there ownership is more clear.
But in both cases we may leak buf.

This is as such unrelated to this patch - but noticed while browsing these
patches.

	Sam

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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* Re: [PATCH 16/42] state: Drop cache bucket
  2017-03-31  7:03 ` [PATCH 16/42] state: Drop cache bucket Sascha Hauer
@ 2017-04-15  8:53   ` Sam Ravnborg
  2017-04-19  8:22     ` Sascha Hauer
  0 siblings, 1 reply; 52+ messages in thread
From: Sam Ravnborg @ 2017-04-15  8:53 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

Hi Sasha.

> diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
> index 6ec58a0c97..218c67f2d7 100644
> --- a/common/state/backend_storage.c
> +++ b/common/state/backend_storage.c
> @@ -69,22 +69,29 @@ int state_storage_write(struct state_backend_storage *storage,
>  	return -EIO;
>  }
>  
> -/**
> - * state_storage_restore_consistency - Restore consistency on all storage backends
> - * @param storage Storage object
> - * @param buf Buffer with valid data that should be on all buckets after this operation
> - * @param len Length of the buffer
> - * @return 0 on success, -errno otherwise
> - *
> - * This function brings valid data onto all buckets we have to ensure that all
> - * data copies are in sync. In the current implementation we just write the data
> - * to all buckets. Bucket implementations that need to keep the number of writes
> - * low, can read their own copy first and compare it.
> - */
> -int state_storage_restore_consistency(struct state_backend_storage *storage,
> -				      const void * buf, ssize_t len)
> +static int bucket_refresh(struct state_backend_storage *storage,
> +			  struct state_backend_storage_bucket *bucket, void *buf, ssize_t len)
>  {
> -	return state_storage_write(storage, buf, len);
> +	int ret;
> +
> +	if (bucket->needs_refresh)
> +		goto refresh;
> +
> +	if (bucket->len != len)
> +		goto refresh;
> +
> +	if (memcmp(bucket->buf, buf, len))
> +		goto refresh;
> +
> +	return 0;
> +
> +refresh:
> +	ret = bucket->write(bucket, buf, len);

In the code above we check needs_refresh and len.
But needs_refresh is never set back to 0 so it looks like we will refresh it too often.
The same with the len argument.

> +	/*
> +	 * Iterate over all buckets. The first valid one we find is the
> +	 * one we want to use.
> +	 */
>  	list_for_each_entry(bucket, &storage->buckets, bucket_list) {
> -		ret = bucket->read(bucket, buf, len);
> -		if (ret) {
> +		ret = bucket->read(bucket, &bucket->buf, &bucket->len);
> +		if (ret == -EUCLEAN) {
> +			bucket->needs_refresh = 1;
> +		} else if (ret) {
This is where we mark it as need_refresh.

	Sam

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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* Re: [PATCH 16/42] state: Drop cache bucket
  2017-04-15  8:53   ` Sam Ravnborg
@ 2017-04-19  8:22     ` Sascha Hauer
  0 siblings, 0 replies; 52+ messages in thread
From: Sascha Hauer @ 2017-04-19  8:22 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: Barebox List

Hi Sam,

On Sat, Apr 15, 2017 at 10:53:25AM +0200, Sam Ravnborg wrote:
> > +	if (memcmp(bucket->buf, buf, len))
> > +		goto refresh;
> > +
> > +	return 0;
> > +
> > +refresh:
> > +	ret = bucket->write(bucket, buf, len);
> 
> In the code above we check needs_refresh and len.
> But needs_refresh is never set back to 0 so it looks like we will refresh it too often.
> The same with the len argument.

You are right, I just sent a patch.

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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* Re: [PATCH 01/42] state: Make pointing to the backend using a phandle the only supported method
  2017-03-31  7:03 ` [PATCH 01/42] state: Make pointing to the backend using a phandle the only supported method Sascha Hauer
@ 2017-05-15  9:18   ` Jan Remmet
  2017-05-15 10:14     ` Jan Remmet
  0 siblings, 1 reply; 52+ messages in thread
From: Jan Remmet @ 2017-05-15  9:18 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On Fri, Mar 31, 2017 at 09:03:05AM +0200, Sascha Hauer wrote:
> All other methods are broken for some time already: When starting the
> kernel the state code rewrites the state node in the device tree and
> replaced the "backend" property with a phandle - even when the target
> can't be described as a phandle. Since using phandles is the nicest way
> to point to the storage device anyway remove the other methods.
Is there a way to use block devices as storage? On eMMC we use mtd style
partitios inside the sd/mmc host node to access barebox and
barebox-environment. But using this barebox-state didn't find the backend
path.

Jan

> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  .../devicetree/bindings/barebox/barebox,state.rst  | 27 ++++++++++++++++------
>  common/state/state.c                               | 21 ++++++++---------
>  2 files changed, 29 insertions(+), 19 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/barebox/barebox,state.rst b/Documentation/devicetree/bindings/barebox/barebox,state.rst
> index 438cc434a2..e9daa65f1a 100644
> --- a/Documentation/devicetree/bindings/barebox/barebox,state.rst
> +++ b/Documentation/devicetree/bindings/barebox/barebox,state.rst
> @@ -29,7 +29,8 @@ Required properties:
>  
>  * ``compatible``: should be ``barebox,state``;
>  * ``magic``: A 32bit number used as a magic to identify the state
> -* ``backend``: describes where the data for this state is stored
> +* ``backend``: contains a phandle to the device/partition which holds the
> +  actual state data.
>  * ``backend-type``: should be ``raw`` or ``dtb``.
>  
>  Optional properties:
> @@ -77,19 +78,31 @@ Example::
>    	magic = <0x27031977>;
>    	compatible = "barebox,state";
>    	backend-type = "raw";
> -  	backend = &eeprom, "partname:state";
> +  	backend = &state_part;
>  
>    	foo {
> -		reg = <0x00 0x4>;
> -		type = "uint32";
> +  		reg = <0x00 0x4>;
> +  		type = "uint32";
>    		default = <0x0>;
>    	};
>  
>    	bar {
> -		reg = <0x10 0x4>;
> -		type = "enum32";
> +  		reg = <0x10 0x4>;
> +  		type = "enum32";
>    		names = "baz", "qux";
> -		default = <1>;
> +  		default = <1>;
> +  	};
> +  };
> +
> +  &nand_flash {
> +  	partitions {
> +  		compatible = "fixed-partitions";
> +  		#address-cells = <1>;
> +  		#size-cells = <1>;
> +  		state_part: state@10000 {
> +  			label = "state";
> +  			reg = <0x10000 0x10000>;
> +  		};
>    	};
>    };
>  
> diff --git a/common/state/state.c b/common/state/state.c
> index 02bb1bb24a..6e6b3a6f08 100644
> --- a/common/state/state.c
> +++ b/common/state/state.c
> @@ -452,19 +452,16 @@ struct state *state_new_from_node(struct device_node *node, char *path,
>  	}
>  
>  	if (!path) {
> -		/* guess if of_path is a path, not a phandle */
> -		if (of_path[0] == '/' && len > 1) {
> -			ret = of_find_path(node, "backend", &path, 0);
> -		} else {
> -			struct device_node *partition_node;
> -
> -			partition_node = of_parse_phandle(node, "backend", 0);
> -			if (!partition_node)
> -				goto out_release_state;
> -
> -			of_path = partition_node->full_name;
> -			ret = of_find_path_by_node(partition_node, &path, 0);
> +		struct device_node *partition_node;
> +
> +		partition_node = of_parse_phandle(node, "backend", 0);
> +		if (!partition_node) {
> +			dev_err(&state->dev, "Cannot resolve \"backend\" phandle\n");
> +			goto out_release_state;
>  		}
> +
> +		of_path = partition_node->full_name;
> +		ret = of_find_path_by_node(partition_node, &path, 0);
>  		if (ret) {
>  			if (ret != -EPROBE_DEFER)
>  				dev_err(&state->dev, "state failed to parse path to backend: %s\n",
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* Re: [PATCH 01/42] state: Make pointing to the backend using a phandle the only supported method
  2017-05-15  9:18   ` Jan Remmet
@ 2017-05-15 10:14     ` Jan Remmet
  2017-05-16  5:33       ` Sascha Hauer
  0 siblings, 1 reply; 52+ messages in thread
From: Jan Remmet @ 2017-05-15 10:14 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On Mon, May 15, 2017 at 11:18:00AM +0200, Jan Remmet wrote:
> On Fri, Mar 31, 2017 at 09:03:05AM +0200, Sascha Hauer wrote:
> > All other methods are broken for some time already: When starting the
> > kernel the state code rewrites the state node in the device tree and
> > replaced the "backend" property with a phandle - even when the target
> > can't be described as a phandle. Since using phandles is the nicest way
> > to point to the storage device anyway remove the other methods.
> Is there a way to use block devices as storage? On eMMC we use mtd style
> partitios inside the sd/mmc host node to access barebox and
> barebox-environment. But using this barebox-state didn't find the backend
> path.
Using barebox-state 2016.12.0 the states from the mtd style partition is found
now.

Jan

> 
> Jan
> 
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  .../devicetree/bindings/barebox/barebox,state.rst  | 27 ++++++++++++++++------
> >  common/state/state.c                               | 21 ++++++++---------
> >  2 files changed, 29 insertions(+), 19 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/barebox/barebox,state.rst b/Documentation/devicetree/bindings/barebox/barebox,state.rst
> > index 438cc434a2..e9daa65f1a 100644
> > --- a/Documentation/devicetree/bindings/barebox/barebox,state.rst
> > +++ b/Documentation/devicetree/bindings/barebox/barebox,state.rst
> > @@ -29,7 +29,8 @@ Required properties:
> >  
> >  * ``compatible``: should be ``barebox,state``;
> >  * ``magic``: A 32bit number used as a magic to identify the state
> > -* ``backend``: describes where the data for this state is stored
> > +* ``backend``: contains a phandle to the device/partition which holds the
> > +  actual state data.
> >  * ``backend-type``: should be ``raw`` or ``dtb``.
> >  
> >  Optional properties:
> > @@ -77,19 +78,31 @@ Example::
> >    	magic = <0x27031977>;
> >    	compatible = "barebox,state";
> >    	backend-type = "raw";
> > -  	backend = &eeprom, "partname:state";
> > +  	backend = &state_part;
> >  
> >    	foo {
> > -		reg = <0x00 0x4>;
> > -		type = "uint32";
> > +  		reg = <0x00 0x4>;
> > +  		type = "uint32";
> >    		default = <0x0>;
> >    	};
> >  
> >    	bar {
> > -		reg = <0x10 0x4>;
> > -		type = "enum32";
> > +  		reg = <0x10 0x4>;
> > +  		type = "enum32";
> >    		names = "baz", "qux";
> > -		default = <1>;
> > +  		default = <1>;
> > +  	};
> > +  };
> > +
> > +  &nand_flash {
> > +  	partitions {
> > +  		compatible = "fixed-partitions";
> > +  		#address-cells = <1>;
> > +  		#size-cells = <1>;
> > +  		state_part: state@10000 {
> > +  			label = "state";
> > +  			reg = <0x10000 0x10000>;
> > +  		};
> >    	};
> >    };
> >  
> > diff --git a/common/state/state.c b/common/state/state.c
> > index 02bb1bb24a..6e6b3a6f08 100644
> > --- a/common/state/state.c
> > +++ b/common/state/state.c
> > @@ -452,19 +452,16 @@ struct state *state_new_from_node(struct device_node *node, char *path,
> >  	}
> >  
> >  	if (!path) {
> > -		/* guess if of_path is a path, not a phandle */
> > -		if (of_path[0] == '/' && len > 1) {
> > -			ret = of_find_path(node, "backend", &path, 0);
> > -		} else {
> > -			struct device_node *partition_node;
> > -
> > -			partition_node = of_parse_phandle(node, "backend", 0);
> > -			if (!partition_node)
> > -				goto out_release_state;
> > -
> > -			of_path = partition_node->full_name;
> > -			ret = of_find_path_by_node(partition_node, &path, 0);
> > +		struct device_node *partition_node;
> > +
> > +		partition_node = of_parse_phandle(node, "backend", 0);
> > +		if (!partition_node) {
> > +			dev_err(&state->dev, "Cannot resolve \"backend\" phandle\n");
> > +			goto out_release_state;
> >  		}
> > +
> > +		of_path = partition_node->full_name;
> > +		ret = of_find_path_by_node(partition_node, &path, 0);
> >  		if (ret) {
> >  			if (ret != -EPROBE_DEFER)
> >  				dev_err(&state->dev, "state failed to parse path to backend: %s\n",
> > -- 
> > 2.11.0
> > 
> > 
> > _______________________________________________
> > barebox mailing list
> > barebox@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/barebox
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* Re: [PATCH 01/42] state: Make pointing to the backend using a phandle the only supported method
  2017-05-15 10:14     ` Jan Remmet
@ 2017-05-16  5:33       ` Sascha Hauer
  2017-05-17  9:13         ` Jan Remmet
  0 siblings, 1 reply; 52+ messages in thread
From: Sascha Hauer @ 2017-05-16  5:33 UTC (permalink / raw)
  To: Jan Remmet; +Cc: Barebox List

On Mon, May 15, 2017 at 12:14:28PM +0200, Jan Remmet wrote:
> On Mon, May 15, 2017 at 11:18:00AM +0200, Jan Remmet wrote:
> > On Fri, Mar 31, 2017 at 09:03:05AM +0200, Sascha Hauer wrote:
> > > All other methods are broken for some time already: When starting the
> > > kernel the state code rewrites the state node in the device tree and
> > > replaced the "backend" property with a phandle - even when the target
> > > can't be described as a phandle. Since using phandles is the nicest way
> > > to point to the storage device anyway remove the other methods.
> > Is there a way to use block devices as storage? On eMMC we use mtd style
> > partitios inside the sd/mmc host node to access barebox and
> > barebox-environment. But using this barebox-state didn't find the backend
> > path.
> Using barebox-state 2016.12.0 the states from the mtd style partition is found
> now.

There's v2017.03.0 already. Or was that the version you tried before and
failed with?

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

^ permalink raw reply	[flat|nested] 52+ messages in thread
* Re: [PATCH 01/42] state: Make pointing to the backend using a phandle the only supported method
  2017-05-16  5:33       ` Sascha Hauer
@ 2017-05-17  9:13         ` Jan Remmet
  0 siblings, 0 replies; 52+ messages in thread
From: Jan Remmet @ 2017-05-17  9:13 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On Tue, May 16, 2017 at 07:33:58AM +0200, Sascha Hauer wrote:
> On Mon, May 15, 2017 at 12:14:28PM +0200, Jan Remmet wrote:
> > On Mon, May 15, 2017 at 11:18:00AM +0200, Jan Remmet wrote:
> > > On Fri, Mar 31, 2017 at 09:03:05AM +0200, Sascha Hauer wrote:
> > > > All other methods are broken for some time already: When starting the
> > > > kernel the state code rewrites the state node in the device tree and
> > > > replaced the "backend" property with a phandle - even when the target
> > > > can't be described as a phandle. Since using phandles is the nicest way
> > > > to point to the storage device anyway remove the other methods.
> > > Is there a way to use block devices as storage? On eMMC we use mtd style
> > > partitios inside the sd/mmc host node to access barebox and
> > > barebox-environment. But using this barebox-state didn't find the backend
> > > path.
> > Using barebox-state 2016.12.0 the states from the mtd style partition is found
> > now.
> 
> There's v2017.03.0 already. Or was that the version you tried before and
> failed with?
I was still on 2016.08.0 with barebox_2016.11.0. 

Jan

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

^ permalink raw reply	[flat|nested] 52+ messages in thread
end of thread, other threads:[~2017-05-17  9:14 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-31  7:03 State patches Sascha Hauer
2017-03-31  7:03 ` [PATCH 01/42] state: Make pointing to the backend using a phandle the only supported method Sascha Hauer
2017-05-15  9:18   ` Jan Remmet
2017-05-15 10:14     ` Jan Remmet
2017-05-16  5:33       ` Sascha Hauer
2017-05-17  9:13         ` Jan Remmet
2017-03-31  7:03 ` [PATCH 02/42] state: Use positive logic Sascha Hauer
2017-03-31  7:03 ` [PATCH 03/42] state: backend: remove .get_packed_len Sascha Hauer
2017-03-31  7:03 ` [PATCH 04/42] state: backend: remove len_hint argument from state_storage_read Sascha Hauer
2017-03-31  7:03 ` [PATCH 05/42] state: Drop backend as extra struct type Sascha Hauer
2017-03-31  7:03 ` [PATCH 06/42] state: merge backend.c into state.c Sascha Hauer
2017-03-31  7:03 ` [PATCH 07/42] state: open code state_backend_init in caller Sascha Hauer
2017-03-31  7:03 ` [PATCH 08/42] state: remove unnecessary argument from state_format_init Sascha Hauer
2017-03-31  7:03 ` [PATCH 09/42] state: pass struct state * to storage functions Sascha Hauer
2017-03-31  7:03 ` [PATCH 10/42] state: storage: initialize variable once outside loop Sascha Hauer
2017-03-31  7:03 ` [PATCH 11/42] state: backend_circular: Read whole PEB Sascha Hauer
2017-04-15  8:40   ` Sam Ravnborg
2017-03-31  7:03 ` [PATCH 12/42] state: drop lazy_init Sascha Hauer
2017-03-31  7:03 ` [PATCH 13/42] state: simplify direct backend Sascha Hauer
2017-03-31  7:03 ` [PATCH 14/42] state: replace len_hint logic Sascha Hauer
2017-03-31  7:03 ` [PATCH 15/42] state: Convert all bufs to void * Sascha Hauer
2017-03-31  7:03 ` [PATCH 16/42] state: Drop cache bucket Sascha Hauer
2017-04-15  8:53   ` Sam Ravnborg
2017-04-19  8:22     ` Sascha Hauer
2017-03-31  7:03 ` [PATCH 17/42] state: backend-direct: Fix max_size Sascha Hauer
2017-03-31  7:03 ` [PATCH 18/42] state: bucket: Make output more informative Sascha Hauer
2017-03-31  7:03 ` [PATCH 19/42] state: backend_bucket_direct: max_size is always given Sascha Hauer
2017-03-31  7:03 ` [PATCH 20/42] state: backend: Add more fields to struct state_backend_storage Sascha Hauer
2017-03-31  7:03 ` [PATCH 21/42] state: backend_circular: remove unnecessary warning Sascha Hauer
2017-03-31  7:03 ` [PATCH 22/42] state: storage: direct: do not close file that is not opened Sascha Hauer
2017-03-31  7:03 ` [PATCH 23/42] state: backend: Add some documentation Sascha Hauer
2017-03-31  7:03 ` [PATCH 24/42] state: backend_circular: default to circular storage Sascha Hauer
2017-03-31  7:03 ` [PATCH 25/42] state: backend_circular: rewrite function doc Sascha Hauer
2017-03-31  7:03 ` [PATCH 26/42] state: backend_storage: Rename variable nr_copies to n_buckets Sascha Hauer
2017-03-31  7:03 ` [PATCH 27/42] state: backend_storage: Rename variable desired_copies to desired_buckets Sascha Hauer
2017-03-31  7:03 ` [PATCH 28/42] state: backend_storage: rewrite function doc Sascha Hauer
2017-03-31  7:03 ` [PATCH 29/42] state: backend_storage: make locally used variable static Sascha Hauer
2017-03-31  7:03 ` [PATCH 30/42] state: backend_storage: rename more variables Sascha Hauer
2017-03-31  7:03 ` [PATCH 31/42] keystore: implement forgetting secrets Sascha Hauer
2017-03-31  7:03 ` [PATCH 32/42] commands: implement keystore command Sascha Hauer
2017-03-31  7:03 ` [PATCH 33/42] commands: state: allow loading state with -l Sascha Hauer
2017-03-31  7:03 ` [PATCH 34/42] crypto: digest: initialize earlier Sascha Hauer
2017-03-31  7:03 ` [PATCH 35/42] state: backend_raw: alloc digest only when needed Sascha Hauer
2017-03-31  7:03 ` [PATCH 36/42] state: backend_circular: Set minumum writesize to 8 Sascha Hauer
2017-03-31  7:03 ` [PATCH 37/42] state: backend bucket circular: Explain metadata Sascha Hauer
2017-03-31  7:03 ` [PATCH 38/42] state: Allow to load without authentification Sascha Hauer
2017-03-31  7:03 ` [PATCH 39/42] state: Update documentation Sascha Hauer
2017-03-31  7:03 ` [PATCH 40/42] state: Do not load state during state_new_from_node Sascha Hauer
2017-03-31  7:03 ` [PATCH 41/42] state: Remove -EUCLEAN check from userspace tool Sascha Hauer
2017-03-31  7:03 ` [PATCH 42/42] state: find device node from device path, not from device node path Sascha Hauer
2017-04-03 20:15 ` State patches Sam Ravnborg
2017-04-04  6:19   ` Sascha Hauer

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