mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Pass reset and boot sources via /chosen node
@ 2018-05-16 20:18 Andrey Smirnov
  2018-05-16 20:18 ` [PATCH v2 1/4] of: Make of_property_get_value() public Andrey Smirnov
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Andrey Smirnov @ 2018-05-16 20:18 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Everyone:

The following patches are really usefull for Linux userspace
applications that need to know the details about reset and boot
sources.

Feedback is welcome!

Changes since [v1]:

	- Series reworked to pass bootsource as a pointer to a
          devicetree node corresponding to detected bootsource

[v1] http://lists.infradead.org/pipermail/barebox/2018-May/032814.html

Andrey Smirnov (4):
  of: Make of_property_get_value() public
  bootsource: Add bootsource alias name API
  common: oftree: Pass bootsource and bootsource instance to kernel
  common: oftree: Pass reset source and reset source instance to kernel

 common/bootsource.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++
 common/oftree.c      | 51 +++++++++++++++++++++++++++++-
 drivers/of/base.c    |  5 ---
 include/bootsource.h |  2 ++
 include/of.h         |  6 ++++
 5 files changed, 132 insertions(+), 6 deletions(-)

-- 
2.17.0


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

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

* [PATCH v2 1/4] of: Make of_property_get_value() public
  2018-05-16 20:18 [PATCH v2 0/4] Pass reset and boot sources via /chosen node Andrey Smirnov
@ 2018-05-16 20:18 ` Andrey Smirnov
  2018-05-16 20:18 ` [PATCH v2 2/4] bootsource: Add bootsource alias name API Andrey Smirnov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Andrey Smirnov @ 2018-05-16 20:18 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Make of_property_get_value() public, so it can be used in other part
of the system.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/of/base.c | 5 -----
 include/of.h      | 6 ++++++
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 4bcc11364..fc01a99ef 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -129,11 +129,6 @@ struct property *of_find_property(const struct device_node *np,
 }
 EXPORT_SYMBOL(of_find_property);
 
-static const void *of_property_get_value(struct property *pp)
-{
-	return pp->value ? pp->value : pp->value_const;
-}
-
 static void of_alias_add(struct alias_prop *ap, struct device_node *np,
 			 int id, const char *stem, int stem_len)
 {
diff --git a/include/of.h b/include/of.h
index fec51bb94..7fc4b7791 100644
--- a/include/of.h
+++ b/include/of.h
@@ -94,6 +94,12 @@ static inline void of_write_number(void *__cell, u64 val, int size)
 	}
 }
 
+static inline const void *of_property_get_value(struct property *pp)
+{
+	return pp->value ? pp->value : pp->value_const;
+}
+
+
 void of_print_property(const void *data, int len);
 void of_print_cmdline(struct device_node *root);
 
-- 
2.17.0


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

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

* [PATCH v2 2/4] bootsource: Add bootsource alias name API
  2018-05-16 20:18 [PATCH v2 0/4] Pass reset and boot sources via /chosen node Andrey Smirnov
  2018-05-16 20:18 ` [PATCH v2 1/4] of: Make of_property_get_value() public Andrey Smirnov
@ 2018-05-16 20:18 ` Andrey Smirnov
  2018-05-17  9:43   ` Sascha Hauer
  2018-05-16 20:18 ` [PATCH v2 3/4] common: oftree: Pass bootsource and bootsource instance to kernel Andrey Smirnov
  2018-05-16 20:18 ` [PATCH v2 4/4] common: oftree: Pass reset source and reset source " Andrey Smirnov
  3 siblings, 1 reply; 9+ messages in thread
From: Andrey Smirnov @ 2018-05-16 20:18 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Add API allowing to query and override the name of the alias pointing
at DTB node representing current bootsource.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/bootsource.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++
 include/bootsource.h |  2 ++
 2 files changed, 76 insertions(+)

diff --git a/common/bootsource.c b/common/bootsource.c
index 78ecd8267..ed435c051 100644
--- a/common/bootsource.c
+++ b/common/bootsource.c
@@ -41,6 +41,80 @@ static const char *bootsource_str[] = {
 
 static enum bootsource bootsource = BOOTSOURCE_UNKNOWN;
 static int bootsource_instance = BOOTSOURCE_INSTANCE_UNKNOWN;
+const char *bootsource_alias_name = NULL;
+
+/**
+ * bootsource_get_alias_name() - Get the name of the bootsource alias
+ *
+ * This function will return newly allocated string containing name of
+ * the alias that is expected to point to DTB node corresponding to
+ * detected bootsource
+ *
+ * NOTE: Caller is expected to free() the string allocated by this
+ * function
+ */
+char *bootsource_get_alias_name(void)
+{
+	int instance = bootsource_instance;
+	const char *stem;
+
+	/*
+	 * If alias name was overridden via
+	 * bootsource_set_alias_name() return that value without
+	 * asking any questions.
+	 *
+	 * Note that we have to strdup() the result to make it
+	 * free-able.
+	 */
+	if (bootsource_alias_name)
+		return strdup(bootsource_alias_name);
+
+	if (bootsource >= ARRAY_SIZE(bootsource_str) ||
+	    bootsource == BOOTSOURCE_UNKNOWN)
+		return NULL;
+
+	switch (bootsource) {
+	case BOOTSOURCE_I2C_EEPROM: /* FALLTHROUGH */
+	case BOOTSOURCE_SPI_EEPROM:
+		/*
+		 * For I2C and SPI EEPROMs we set the stem to be 'i2c'
+		 * and 'spi' correspondingly. The resulting alias will
+		 * be pointing at the controller said EEPROM is
+		 * attached to.
+		 *
+		 * NOTE: This code assumes single bootable EEPROM per
+		 * controller
+		 */
+		BUILD_BUG_ON(BOOTSOURCE_SPI != BOOTSOURCE_SPI_EEPROM - 1);
+		BUILD_BUG_ON(BOOTSOURCE_I2C != BOOTSOURCE_I2C_EEPROM - 1);
+
+		stem = bootsource_str[bootsource - 1];
+		break;
+	case BOOTSOURCE_SERIAL:	/* FALLTHROUGH */
+	case BOOTSOURCE_I2C:	/* FALLTHROUGH */
+	case BOOTSOURCE_MMC:	/* FALLTHROUGH */
+	case BOOTSOURCE_SPI:	/* FALLTHROUGH */
+	case BOOTSOURCE_CAN:
+		stem = bootsource_str[bootsource];
+		break;
+	default:
+		return NULL;
+	}
+
+	/*
+	 * Assume 0 if instance was not specified by bootsource
+	 * detection code.
+	 */
+	if (instance == BOOTSOURCE_INSTANCE_UNKNOWN)
+		instance = 0;
+
+	return basprintf("%s%d", stem, instance);
+}
+
+void bootsource_set_alias_name(const char *name)
+{
+	bootsource_alias_name = name;
+}
 
 void bootsource_set(enum bootsource src)
 {
diff --git a/include/bootsource.h b/include/bootsource.h
index 064f6b9a2..29347aaeb 100644
--- a/include/bootsource.h
+++ b/include/bootsource.h
@@ -25,5 +25,7 @@ enum bootsource bootsource_get(void);
 int bootsource_get_instance(void);
 void bootsource_set(enum bootsource src);
 void bootsource_set_instance(int instance);
+void bootsource_set_alias_name(const char *name);
+char *bootsource_get_alias_name(void);
 
 #endif	/* __BOOTSOURCE_H__ */
-- 
2.17.0


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

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

* [PATCH v2 3/4] common: oftree: Pass bootsource and bootsource instance to kernel
  2018-05-16 20:18 [PATCH v2 0/4] Pass reset and boot sources via /chosen node Andrey Smirnov
  2018-05-16 20:18 ` [PATCH v2 1/4] of: Make of_property_get_value() public Andrey Smirnov
  2018-05-16 20:18 ` [PATCH v2 2/4] bootsource: Add bootsource alias name API Andrey Smirnov
@ 2018-05-16 20:18 ` Andrey Smirnov
  2018-05-17  9:52   ` Sascha Hauer
  2018-05-16 20:18 ` [PATCH v2 4/4] common: oftree: Pass reset source and reset source " Andrey Smirnov
  3 siblings, 1 reply; 9+ messages in thread
From: Andrey Smirnov @ 2018-05-16 20:18 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Pass barebox-detected bootsource to Linux to make it availible to
Linux userspace. That information is passed as full path to the node
corresponding to the bootsource and is placed under /chosen/bootsource
and it can be read under Linux in

/sys/firmware/devicetree/base/chosen/bootsource

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/oftree.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/common/oftree.c b/common/oftree.c
index 8a2ede4c6..5ab6bf089 100644
--- a/common/oftree.c
+++ b/common/oftree.c
@@ -11,6 +11,7 @@
 #include <getopt.h>
 #include <init.h>
 #include <boot.h>
+#include <bootsource.h>
 #include <i2c/i2c.h>
 
 #define MAX_LEVEL	32		/* how deeply nested we will go */
@@ -114,6 +115,47 @@ void of_print_cmdline(struct device_node *root)
 	printf("commandline: %s\n", cmdline);
 }
 
+static int of_fixup_bootargs_bootsource(struct device_node *root,
+					struct device_node *chosen)
+{
+	char *alias_name = bootsource_get_alias_name();
+	struct device_node *bootsource;
+	struct device_node *aliasnp;
+	struct property *p;
+	int ret;
+
+	if (!alias_name)
+		return 0;
+
+	bootsource = of_find_node_by_alias(root, alias_name);
+	/*
+	 * If kerenel DTB doesn't have the appropriate alias set up,
+	 * give up and exit early. No error is reported.
+	 */
+	if (!bootsource) {
+		ret = 0;
+		goto exit;
+	}
+
+	aliasnp = of_find_node_by_path_from(root, "/aliases");
+	if (!aliasnp) {
+		ret = -ENOENT;
+		goto exit;
+	}
+
+	p = of_find_property(aliasnp, alias_name, NULL);
+	if (!p) {
+		ret = -ENOENT;
+		goto exit;
+	}
+
+	ret = of_set_property(chosen, "bootsource", of_property_get_value(p),
+			      p->length, true);
+exit:
+	free(alias_name);
+	return ret;
+}
+
 static int of_fixup_bootargs(struct device_node *root, void *unused)
 {
 	struct device_node *node;
@@ -131,8 +173,10 @@ static int of_fixup_bootargs(struct device_node *root, void *unused)
 	of_property_write_string(node, "barebox-version", release_string);
 
 	err = of_property_write_string(node, "bootargs", str);
+	if (err)
+		return err;
 
-	return err;
+	return of_fixup_bootargs_bootsource(root, node);
 }
 
 static int of_register_bootargs_fixup(void)
-- 
2.17.0


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

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

* [PATCH v2 4/4] common: oftree: Pass reset source and reset source instance to kernel
  2018-05-16 20:18 [PATCH v2 0/4] Pass reset and boot sources via /chosen node Andrey Smirnov
                   ` (2 preceding siblings ...)
  2018-05-16 20:18 ` [PATCH v2 3/4] common: oftree: Pass bootsource and bootsource instance to kernel Andrey Smirnov
@ 2018-05-16 20:18 ` Andrey Smirnov
  3 siblings, 0 replies; 9+ messages in thread
From: Andrey Smirnov @ 2018-05-16 20:18 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Detecting reset reason is, in some cases, a destructive operation and
in such cases it is impossible to obtain that information in the
kernel without some help from barebox.

Pass reset source and reset source instance to kernel to Linux to make
it availible to Linux userspace. This info is placeed under
/chosen/bootsource and it can be read under Linux in

/sys/firmware/devicetree/base/chosen/reset-source.

and

/sys/firmware/devicetree/base/chosen/reset-source-instance.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/oftree.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/common/oftree.c b/common/oftree.c
index 5ab6bf089..5b592fb07 100644
--- a/common/oftree.c
+++ b/common/oftree.c
@@ -13,6 +13,7 @@
 #include <boot.h>
 #include <bootsource.h>
 #include <i2c/i2c.h>
+#include <reset_source.h>
 
 #define MAX_LEVEL	32		/* how deeply nested we will go */
 
@@ -176,6 +177,10 @@ static int of_fixup_bootargs(struct device_node *root, void *unused)
 	if (err)
 		return err;
 
+	of_property_write_string(node, "reset-source", reset_source_name());
+	of_property_write_u32(node, "reset-source-instance",
+			      reset_source_get_instance());
+
 	return of_fixup_bootargs_bootsource(root, node);
 }
 
-- 
2.17.0


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

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

* Re: [PATCH v2 2/4] bootsource: Add bootsource alias name API
  2018-05-16 20:18 ` [PATCH v2 2/4] bootsource: Add bootsource alias name API Andrey Smirnov
@ 2018-05-17  9:43   ` Sascha Hauer
  2018-05-17 19:35     ` Andrey Smirnov
  0 siblings, 1 reply; 9+ messages in thread
From: Sascha Hauer @ 2018-05-17  9:43 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: barebox

On Wed, May 16, 2018 at 01:18:25PM -0700, Andrey Smirnov wrote:
> Add API allowing to query and override the name of the alias pointing
> at DTB node representing current bootsource.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  common/bootsource.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++
>  include/bootsource.h |  2 ++
>  2 files changed, 76 insertions(+)
> 
> diff --git a/common/bootsource.c b/common/bootsource.c
> index 78ecd8267..ed435c051 100644
> --- a/common/bootsource.c
> +++ b/common/bootsource.c
> @@ -41,6 +41,80 @@ static const char *bootsource_str[] = {
>  
>  static enum bootsource bootsource = BOOTSOURCE_UNKNOWN;
>  static int bootsource_instance = BOOTSOURCE_INSTANCE_UNKNOWN;
> +const char *bootsource_alias_name = NULL;
> +
> +/**
> + * bootsource_get_alias_name() - Get the name of the bootsource alias
> + *
> + * This function will return newly allocated string containing name of
> + * the alias that is expected to point to DTB node corresponding to
> + * detected bootsource
> + *
> + * NOTE: Caller is expected to free() the string allocated by this
> + * function
> + */
> +char *bootsource_get_alias_name(void)
> +{
> +	int instance = bootsource_instance;
> +	const char *stem;
> +
> +	/*
> +	 * If alias name was overridden via
> +	 * bootsource_set_alias_name() return that value without
> +	 * asking any questions.
> +	 *
> +	 * Note that we have to strdup() the result to make it
> +	 * free-able.
> +	 */
> +	if (bootsource_alias_name)
> +		return strdup(bootsource_alias_name);
> +
> +	if (bootsource >= ARRAY_SIZE(bootsource_str) ||
> +	    bootsource == BOOTSOURCE_UNKNOWN)
> +		return NULL;

This is already handled by the default path of the switch/case below.

> +
> +	switch (bootsource) {
> +	case BOOTSOURCE_I2C_EEPROM: /* FALLTHROUGH */
> +	case BOOTSOURCE_SPI_EEPROM:
> +		/*
> +		 * For I2C and SPI EEPROMs we set the stem to be 'i2c'
> +		 * and 'spi' correspondingly. The resulting alias will
> +		 * be pointing at the controller said EEPROM is
> +		 * attached to.
> +		 *
> +		 * NOTE: This code assumes single bootable EEPROM per
> +		 * controller
> +		 */
> +		BUILD_BUG_ON(BOOTSOURCE_SPI != BOOTSOURCE_SPI_EEPROM - 1);
> +		BUILD_BUG_ON(BOOTSOURCE_I2C != BOOTSOURCE_I2C_EEPROM - 1);
> +
> +		stem = bootsource_str[bootsource - 1];

Can we make this conversion more explicit?

	case BOOTSOURCE_I2C_EEPROM:
		stem = bootsource_str[BOOTSOURCE_I2C];
		break;
	case BOOTSOURCE_SPI_EEPROM:
		stem = bootsource_str[BOOTSOURCE_SPI];
		break;

It makes it more obvious what happens here.

> +		break;
> +	case BOOTSOURCE_SERIAL:	/* FALLTHROUGH */
> +	case BOOTSOURCE_I2C:	/* FALLTHROUGH */
> +	case BOOTSOURCE_MMC:	/* FALLTHROUGH */
> +	case BOOTSOURCE_SPI:	/* FALLTHROUGH */
> +	case BOOTSOURCE_CAN:
> +		stem = bootsource_str[bootsource];
> +		break;
> +	default:
> +		return NULL;
> +	}
> +
> +	/*
> +	 * Assume 0 if instance was not specified by bootsource
> +	 * detection code.
> +	 */
> +	if (instance == BOOTSOURCE_INSTANCE_UNKNOWN)
> +		instance = 0;

Hm, if some SoC has forgotten to setup the instance I think we should
just bail out here and fix the SoC code instead.

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] 9+ messages in thread

* Re: [PATCH v2 3/4] common: oftree: Pass bootsource and bootsource instance to kernel
  2018-05-16 20:18 ` [PATCH v2 3/4] common: oftree: Pass bootsource and bootsource instance to kernel Andrey Smirnov
@ 2018-05-17  9:52   ` Sascha Hauer
  2018-05-17 20:20     ` Andrey Smirnov
  0 siblings, 1 reply; 9+ messages in thread
From: Sascha Hauer @ 2018-05-17  9:52 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: barebox

On Wed, May 16, 2018 at 01:18:26PM -0700, Andrey Smirnov wrote:
> Pass barebox-detected bootsource to Linux to make it availible to
> Linux userspace. That information is passed as full path to the node
> corresponding to the bootsource and is placed under /chosen/bootsource
> and it can be read under Linux in
> 
> /sys/firmware/devicetree/base/chosen/bootsource
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  common/oftree.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/common/oftree.c b/common/oftree.c
> index 8a2ede4c6..5ab6bf089 100644
> --- a/common/oftree.c
> +++ b/common/oftree.c
> @@ -11,6 +11,7 @@
>  #include <getopt.h>
>  #include <init.h>
>  #include <boot.h>
> +#include <bootsource.h>
>  #include <i2c/i2c.h>
>  
>  #define MAX_LEVEL	32		/* how deeply nested we will go */
> @@ -114,6 +115,47 @@ void of_print_cmdline(struct device_node *root)
>  	printf("commandline: %s\n", cmdline);
>  }
>  
> +static int of_fixup_bootargs_bootsource(struct device_node *root,
> +					struct device_node *chosen)
> +{
> +	char *alias_name = bootsource_get_alias_name();
> +	struct device_node *bootsource;
> +	struct device_node *aliasnp;
> +	struct property *p;
> +	int ret;
> +
> +	if (!alias_name)
> +		return 0;
> +
> +	bootsource = of_find_node_by_alias(root, alias_name);
> +	/*
> +	 * If kerenel DTB doesn't have the appropriate alias set up,
> +	 * give up and exit early. No error is reported.
> +	 */

s/kerenel/kernel/

> +	if (!bootsource) {
> +		ret = 0;
> +		goto exit;
> +	}
> +
> +	aliasnp = of_find_node_by_path_from(root, "/aliases");
> +	if (!aliasnp) {
> +		ret = -ENOENT;
> +		goto exit;
> +	}
> +
> +	p = of_find_property(aliasnp, alias_name, NULL);
> +	if (!p) {
> +		ret = -ENOENT;
> +		goto exit;
> +	}

This seems unnecessary. Pass bootsource->full_name to of_set_property
below.

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] 9+ messages in thread

* Re: [PATCH v2 2/4] bootsource: Add bootsource alias name API
  2018-05-17  9:43   ` Sascha Hauer
@ 2018-05-17 19:35     ` Andrey Smirnov
  0 siblings, 0 replies; 9+ messages in thread
From: Andrey Smirnov @ 2018-05-17 19:35 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On Thu, May 17, 2018 at 2:43 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Wed, May 16, 2018 at 01:18:25PM -0700, Andrey Smirnov wrote:
>> Add API allowing to query and override the name of the alias pointing
>> at DTB node representing current bootsource.
>>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  common/bootsource.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++
>>  include/bootsource.h |  2 ++
>>  2 files changed, 76 insertions(+)
>>
>> diff --git a/common/bootsource.c b/common/bootsource.c
>> index 78ecd8267..ed435c051 100644
>> --- a/common/bootsource.c
>> +++ b/common/bootsource.c
>> @@ -41,6 +41,80 @@ static const char *bootsource_str[] = {
>>
>>  static enum bootsource bootsource = BOOTSOURCE_UNKNOWN;
>>  static int bootsource_instance = BOOTSOURCE_INSTANCE_UNKNOWN;
>> +const char *bootsource_alias_name = NULL;
>> +
>> +/**
>> + * bootsource_get_alias_name() - Get the name of the bootsource alias
>> + *
>> + * This function will return newly allocated string containing name of
>> + * the alias that is expected to point to DTB node corresponding to
>> + * detected bootsource
>> + *
>> + * NOTE: Caller is expected to free() the string allocated by this
>> + * function
>> + */
>> +char *bootsource_get_alias_name(void)
>> +{
>> +     int instance = bootsource_instance;
>> +     const char *stem;
>> +
>> +     /*
>> +      * If alias name was overridden via
>> +      * bootsource_set_alias_name() return that value without
>> +      * asking any questions.
>> +      *
>> +      * Note that we have to strdup() the result to make it
>> +      * free-able.
>> +      */
>> +     if (bootsource_alias_name)
>> +             return strdup(bootsource_alias_name);
>> +
>> +     if (bootsource >= ARRAY_SIZE(bootsource_str) ||
>> +         bootsource == BOOTSOURCE_UNKNOWN)
>> +             return NULL;
>
> This is already handled by the default path of the switch/case below.
>

Good point. Will remove in v3.

>> +
>> +     switch (bootsource) {
>> +     case BOOTSOURCE_I2C_EEPROM: /* FALLTHROUGH */
>> +     case BOOTSOURCE_SPI_EEPROM:
>> +             /*
>> +              * For I2C and SPI EEPROMs we set the stem to be 'i2c'
>> +              * and 'spi' correspondingly. The resulting alias will
>> +              * be pointing at the controller said EEPROM is
>> +              * attached to.
>> +              *
>> +              * NOTE: This code assumes single bootable EEPROM per
>> +              * controller
>> +              */
>> +             BUILD_BUG_ON(BOOTSOURCE_SPI != BOOTSOURCE_SPI_EEPROM - 1);
>> +             BUILD_BUG_ON(BOOTSOURCE_I2C != BOOTSOURCE_I2C_EEPROM - 1);
>> +
>> +             stem = bootsource_str[bootsource - 1];
>
> Can we make this conversion more explicit?
>
>         case BOOTSOURCE_I2C_EEPROM:
>                 stem = bootsource_str[BOOTSOURCE_I2C];
>                 break;
>         case BOOTSOURCE_SPI_EEPROM:
>                 stem = bootsource_str[BOOTSOURCE_SPI];
>                 break;
>
> It makes it more obvious what happens here.
>

Sure, will do in v3.

>> +             break;
>> +     case BOOTSOURCE_SERIAL: /* FALLTHROUGH */
>> +     case BOOTSOURCE_I2C:    /* FALLTHROUGH */
>> +     case BOOTSOURCE_MMC:    /* FALLTHROUGH */
>> +     case BOOTSOURCE_SPI:    /* FALLTHROUGH */
>> +     case BOOTSOURCE_CAN:
>> +             stem = bootsource_str[bootsource];
>> +             break;
>> +     default:
>> +             return NULL;
>> +     }
>> +
>> +     /*
>> +      * Assume 0 if instance was not specified by bootsource
>> +      * detection code.
>> +      */
>> +     if (instance == BOOTSOURCE_INSTANCE_UNKNOWN)
>> +             instance = 0;
>
> Hm, if some SoC has forgotten to setup the instance I think we should
> just bail out here and fix the SoC code instead.
>

OK, will do in v3.

Thanks,
Andrey Smirnov

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

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

* Re: [PATCH v2 3/4] common: oftree: Pass bootsource and bootsource instance to kernel
  2018-05-17  9:52   ` Sascha Hauer
@ 2018-05-17 20:20     ` Andrey Smirnov
  0 siblings, 0 replies; 9+ messages in thread
From: Andrey Smirnov @ 2018-05-17 20:20 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On Thu, May 17, 2018 at 2:52 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Wed, May 16, 2018 at 01:18:26PM -0700, Andrey Smirnov wrote:
>> Pass barebox-detected bootsource to Linux to make it availible to
>> Linux userspace. That information is passed as full path to the node
>> corresponding to the bootsource and is placed under /chosen/bootsource
>> and it can be read under Linux in
>>
>> /sys/firmware/devicetree/base/chosen/bootsource
>>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  common/oftree.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/oftree.c b/common/oftree.c
>> index 8a2ede4c6..5ab6bf089 100644
>> --- a/common/oftree.c
>> +++ b/common/oftree.c
>> @@ -11,6 +11,7 @@
>>  #include <getopt.h>
>>  #include <init.h>
>>  #include <boot.h>
>> +#include <bootsource.h>
>>  #include <i2c/i2c.h>
>>
>>  #define MAX_LEVEL    32              /* how deeply nested we will go */
>> @@ -114,6 +115,47 @@ void of_print_cmdline(struct device_node *root)
>>       printf("commandline: %s\n", cmdline);
>>  }
>>
>> +static int of_fixup_bootargs_bootsource(struct device_node *root,
>> +                                     struct device_node *chosen)
>> +{
>> +     char *alias_name = bootsource_get_alias_name();
>> +     struct device_node *bootsource;
>> +     struct device_node *aliasnp;
>> +     struct property *p;
>> +     int ret;
>> +
>> +     if (!alias_name)
>> +             return 0;
>> +
>> +     bootsource = of_find_node_by_alias(root, alias_name);
>> +     /*
>> +      * If kerenel DTB doesn't have the appropriate alias set up,
>> +      * give up and exit early. No error is reported.
>> +      */
>
> s/kerenel/kernel/
>

Will fix in v3.

>> +     if (!bootsource) {
>> +             ret = 0;
>> +             goto exit;
>> +     }
>> +
>> +     aliasnp = of_find_node_by_path_from(root, "/aliases");
>> +     if (!aliasnp) {
>> +             ret = -ENOENT;
>> +             goto exit;
>> +     }
>> +
>> +     p = of_find_property(aliasnp, alias_name, NULL);
>> +     if (!p) {
>> +             ret = -ENOENT;
>> +             goto exit;
>> +     }
>
> This seems unnecessary. Pass bootsource->full_name to of_set_property
> below.
>

Haven't thought about doing that. Will do in v3.

Thanks,
Andrey Smirnov

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

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

end of thread, other threads:[~2018-05-17 20:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-16 20:18 [PATCH v2 0/4] Pass reset and boot sources via /chosen node Andrey Smirnov
2018-05-16 20:18 ` [PATCH v2 1/4] of: Make of_property_get_value() public Andrey Smirnov
2018-05-16 20:18 ` [PATCH v2 2/4] bootsource: Add bootsource alias name API Andrey Smirnov
2018-05-17  9:43   ` Sascha Hauer
2018-05-17 19:35     ` Andrey Smirnov
2018-05-16 20:18 ` [PATCH v2 3/4] common: oftree: Pass bootsource and bootsource instance to kernel Andrey Smirnov
2018-05-17  9:52   ` Sascha Hauer
2018-05-17 20:20     ` Andrey Smirnov
2018-05-16 20:18 ` [PATCH v2 4/4] common: oftree: Pass reset source and reset source " Andrey Smirnov

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