mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/2] string: implement strchrnul
@ 2021-06-19  3:26 Ahmad Fatoum
  2021-06-19  3:26 ` [PATCH 2/2] console: respect baudrate specified in device-tree stdout-path Ahmad Fatoum
  2021-06-21  4:52 ` [PATCH 1/2] string: implement strchrnul Sascha Hauer
  0 siblings, 2 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2021-06-19  3:26 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

We have at least two places opencoding strchrnul, one of them needlessly
iterating twice instead of once over the string. Replace both by calling
a single pass implementation.

Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
---
 commands/hwclock.c     |  8 --------
 drivers/of/base.c      |  5 +----
 include/linux/string.h |  4 ++++
 lib/string.c           | 18 ++++++++++++++++++
 4 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/commands/hwclock.c b/commands/hwclock.c
index 1b5c2cd100ca..7a5600c456c9 100644
--- a/commands/hwclock.c
+++ b/commands/hwclock.c
@@ -9,14 +9,6 @@
 #include <string.h>
 #include <environment.h>
 
-static char *strchrnul(const char *s, int c)
-{
-	while (*s != '\0' && *s != c)
-		s++;
-
-	return (char *)s;
-}
-
 static int sscanf_two_digits(char *s, int *res)
 {
 	char buf[3];
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 6e716fcdcf27..f375fcd67572 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2479,10 +2479,7 @@ struct device_node *of_get_stdoutpath(void)
 	if (!name)
 		return 0;
 
-	/* This could make use of strchrnul if it were available */
-	p = strchr(name, ':');
-	if (!p)
-		p = name + strlen(name);
+	p = strchrnul(name, ':');
 
 	q = xstrndup(name, p - name);
 
diff --git a/include/linux/string.h b/include/linux/string.h
index 55bc905c0e6b..47a27a391f0b 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -25,6 +25,10 @@ extern __kernel_size_t strspn(const char *,const char *);
 	(typeof(&(s1)[0]))(_strstr((s1), (s2))); \
 	})
 
+#ifndef __HAVE_ARCH_STRCHRNUL
+extern char * strchrnul(const char *,int);
+#endif
+
 /*
  * Include machine specific inline routines
  */
diff --git a/lib/string.c b/lib/string.c
index bad186586fac..9aeb1d883022 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -274,6 +274,24 @@ char * _strchr(const char * s, int c)
 #endif
 EXPORT_SYMBOL(_strchr);
 
+#ifndef __HAVE_ARCH_STRCHRNUL
+/**
+ * strchrnul - Find and return a character in a string, or end of string
+ * @s: The string to be searched
+ * @c: The character to search for
+ *
+ * Returns pointer to first occurrence of 'c' in s. If c is not found, then
+ * return a pointer to the null byte at the end of s.
+ */
+char *strchrnul(const char *s, int c)
+{
+	while (*s && *s != (char)c)
+		s++;
+	return (char *)s;
+}
+EXPORT_SYMBOL(strchrnul);
+#endif
+
 #ifndef __HAVE_ARCH_STRRCHR
 /**
  * strrchr - Find the last occurrence of a character in a string
-- 
2.32.0.rc0


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


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

* [PATCH 2/2] console: respect baudrate specified in device-tree stdout-path
  2021-06-19  3:26 [PATCH 1/2] string: implement strchrnul Ahmad Fatoum
@ 2021-06-19  3:26 ` Ahmad Fatoum
  2021-06-21  4:59   ` Sascha Hauer
  2021-06-21  4:52 ` [PATCH 1/2] string: implement strchrnul Sascha Hauer
  1 sibling, 1 reply; 6+ messages in thread
From: Ahmad Fatoum @ 2021-06-19  3:26 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

stdout-path in the device tree can have a suffix indicating line
settings. The baud rate contained within was so far ignored by barebox.

Change this so barebox first consults the stdout-path alias before
falling back to CONFIG_BAUDRATE.

Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
---
This applies on top of the deep probe series.
---
 common/console.c      | 20 ++++++++++----------
 drivers/of/base.c     | 19 ++++++++++++++-----
 drivers/of/platform.c |  2 +-
 include/of.h          |  8 ++++----
 4 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/common/console.c b/common/console.c
index 8a0af75a1ff4..ad1a6aaab2a0 100644
--- a/common/console.c
+++ b/common/console.c
@@ -220,7 +220,7 @@ static void console_init_early(void)
 	initialized = CONSOLE_INITIALIZED_BUFFER;
 }
 
-static void console_set_stdoutpath(struct console_device *cdev)
+static void console_set_stdoutpath(struct console_device *cdev, unsigned baudrate)
 {
 	int id;
 	char *str;
@@ -232,8 +232,7 @@ static void console_set_stdoutpath(struct console_device *cdev)
 	if (id < 0)
 		return;
 
-	str = basprintf("console=%s%d,%dn8", cdev->linux_console_name, id,
-			  cdev->baudrate);
+	str = basprintf("console=%s%d,%dn8", cdev->linux_console_name, id, baudrate);
 
 	globalvar_add_simple("linux.bootargs.console", str);
 
@@ -297,6 +296,7 @@ int console_register(struct console_device *newcdev)
 	struct device_node *serdev_node = console_is_serdev_node(newcdev);
 	struct device_d *dev = &newcdev->class_dev;
 	int activate = 0, ret;
+	unsigned baudrate = CONFIG_BAUDRATE;
 
 	if (!serdev_node && initialized == CONSOLE_UNINITIALIZED)
 		console_init_early();
@@ -327,11 +327,16 @@ int console_register(struct console_device *newcdev)
 	if (serdev_node)
 		return of_platform_populate(serdev_node, NULL, dev);
 
+	if (newcdev->dev && of_device_is_stdout_path(newcdev->dev, &baudrate)) {
+		activate = 1;
+		console_set_stdoutpath(newcdev, baudrate);
+	}
+
 	if (newcdev->setbrg) {
-		ret = newcdev->setbrg(newcdev, CONFIG_BAUDRATE);
+		ret = newcdev->setbrg(newcdev, baudrate);
 		if (ret)
 			return ret;
-		newcdev->baudrate_param = newcdev->baudrate = CONFIG_BAUDRATE;
+		newcdev->baudrate_param = newcdev->baudrate = baudrate;
 		dev_add_param_uint32(dev, "baudrate", console_baudrate_set,
 			NULL, &newcdev->baudrate_param, "%u", newcdev);
 	}
@@ -349,11 +354,6 @@ int console_register(struct console_device *newcdev)
 		activate = 1;
 	}
 
-	if (newcdev->dev && of_device_is_stdout_path(newcdev->dev)) {
-		activate = 1;
-		console_set_stdoutpath(newcdev);
-	}
-
 	list_add_tail(&newcdev->list, &console_list);
 
 	if (activate)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index f375fcd67572..877480d46518 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2465,7 +2465,7 @@ void of_delete_node(struct device_node *node)
 		of_set_root_node(NULL);
 }
 
-struct device_node *of_get_stdoutpath(void)
+struct device_node *of_get_stdoutpath(unsigned int *baudrate)
 {
 	struct device_node *dn;
 	const char *name;
@@ -2487,15 +2487,24 @@ struct device_node *of_get_stdoutpath(void)
 
 	free(q);
 
+	if (baudrate && *p) {
+		unsigned rate = simple_strtoul(p + 1, NULL, 10);
+		if (rate)
+			*baudrate = rate;
+	}
+
 	return dn;
 }
 
-int of_device_is_stdout_path(struct device_d *dev)
+int of_device_is_stdout_path(struct device_d *dev, unsigned int *baudrate)
 {
-	if (!dev->device_node)
-		return 0;
+	unsigned int tmp;
+
+	if (!dev || !dev->device_node || dev->device_node != of_get_stdoutpath(&tmp))
+		return false;
 
-	return dev->device_node == of_get_stdoutpath();
+	*baudrate = tmp;
+	return true;
 }
 
 /**
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 6b6f904573dd..1d025f941682 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -520,7 +520,7 @@ static int of_stdoutpath_init(void)
 {
 	struct device_node *np;
 
-	np = of_get_stdoutpath();
+	np = of_get_stdoutpath(NULL);
 	if (!np)
 		return 0;
 
diff --git a/include/of.h b/include/of.h
index 98de9d848034..689fa54d4c73 100644
--- a/include/of.h
+++ b/include/of.h
@@ -286,8 +286,8 @@ struct cdev *of_parse_partition(struct cdev *cdev, struct device_node *node);
 int of_parse_partitions(struct cdev *cdev, struct device_node *node);
 int of_fixup_partitions(struct device_node *np, struct cdev *cdev);
 int of_partitions_register_fixup(struct cdev *cdev);
-struct device_node *of_get_stdoutpath(void);
-int of_device_is_stdout_path(struct device_d *dev);
+struct device_node *of_get_stdoutpath(unsigned int *);
+int of_device_is_stdout_path(struct device_d *dev, unsigned int *baudrate);
 const char *of_get_model(void);
 void *of_flatten_dtb(struct device_node *node);
 int of_add_memory(struct device_node *node, bool dump);
@@ -324,12 +324,12 @@ static inline int of_partitions_register_fixup(struct cdev *cdev)
 	return -ENOSYS;
 }
 
-static inline struct device_node *of_get_stdoutpath(void)
+static inline struct device_node *of_get_stdoutpath(unsigned int *)
 {
 	return NULL;
 }
 
-static inline int of_device_is_stdout_path(struct device_d *dev)
+static inline int of_device_is_stdout_path(struct device_d *dev, unsigned int *baudrate)
 {
 	return 0;
 }
-- 
2.32.0.rc0


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


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

* Re: [PATCH 1/2] string: implement strchrnul
  2021-06-19  3:26 [PATCH 1/2] string: implement strchrnul Ahmad Fatoum
  2021-06-19  3:26 ` [PATCH 2/2] console: respect baudrate specified in device-tree stdout-path Ahmad Fatoum
@ 2021-06-21  4:52 ` Sascha Hauer
  2021-06-21  5:03   ` Ahmad Fatoum
  1 sibling, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2021-06-21  4:52 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

Hi Ahmad,

On Sat, Jun 19, 2021 at 05:26:53AM +0200, Ahmad Fatoum wrote:
> -}
> -
>  static int sscanf_two_digits(char *s, int *res)
>  {
>  	char buf[3];
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 6e716fcdcf27..f375fcd67572 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2479,10 +2479,7 @@ struct device_node *of_get_stdoutpath(void)

There seems to be a patch missing in this series. I don't have
of_get_stdoutpath() in my tree.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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] 6+ messages in thread

* Re: [PATCH 2/2] console: respect baudrate specified in device-tree stdout-path
  2021-06-19  3:26 ` [PATCH 2/2] console: respect baudrate specified in device-tree stdout-path Ahmad Fatoum
@ 2021-06-21  4:59   ` Sascha Hauer
  2021-06-21  5:04     ` Ahmad Fatoum
  0 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2021-06-21  4:59 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

Hi Ahmad,

On Sat, Jun 19, 2021 at 05:26:54AM +0200, Ahmad Fatoum wrote:
> -static void console_set_stdoutpath(struct console_device *cdev)
> +static void console_set_stdoutpath(struct console_device *cdev, unsigned baudrate)
>  {
>  	int id;
>  	char *str;
> @@ -232,8 +232,7 @@ static void console_set_stdoutpath(struct console_device *cdev)
>  	if (id < 0)
>  		return;
>  
> -	str = basprintf("console=%s%d,%dn8", cdev->linux_console_name, id,
> -			  cdev->baudrate);
> +	str = basprintf("console=%s%d,%dn8", cdev->linux_console_name, id, baudrate);
>  
>  	globalvar_add_simple("linux.bootargs.console", str);
>  
> @@ -297,6 +296,7 @@ int console_register(struct console_device *newcdev)
>  	struct device_node *serdev_node = console_is_serdev_node(newcdev);
>  	struct device_d *dev = &newcdev->class_dev;
>  	int activate = 0, ret;
> +	unsigned baudrate = CONFIG_BAUDRATE;

You initialize baudrate to CONFIG_BAUDRATE here in order to use that as
a default, ...

>  
>  	if (!serdev_node && initialized == CONSOLE_UNINITIALIZED)
>  		console_init_early();
> @@ -327,11 +327,16 @@ int console_register(struct console_device *newcdev)
>  	if (serdev_node)
>  		return of_platform_populate(serdev_node, NULL, dev);
>  
> +	if (newcdev->dev && of_device_is_stdout_path(newcdev->dev, &baudrate)) {

...but of_device_is_stdout_path() unconditionally overwrites &baudrate...

> -int of_device_is_stdout_path(struct device_d *dev)
> +int of_device_is_stdout_path(struct device_d *dev, unsigned int *baudrate)
>  {
> -	if (!dev->device_node)
> -		return 0;
> +	unsigned int tmp;
> +
> +	if (!dev || !dev->device_node || dev->device_node != of_get_stdoutpath(&tmp))
> +		return false;
>  
> -	return dev->device_node == of_get_stdoutpath();
> +	*baudrate = tmp;

...here. This would break boards which don't have the baudrate given in
the devicetree.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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] 6+ messages in thread

* Re: [PATCH 1/2] string: implement strchrnul
  2021-06-21  4:52 ` [PATCH 1/2] string: implement strchrnul Sascha Hauer
@ 2021-06-21  5:03   ` Ahmad Fatoum
  0 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2021-06-21  5:03 UTC (permalink / raw)
  To: Sascha Hauer, Ahmad Fatoum; +Cc: barebox

Hi,

On 21.06.21 06:52, Sascha Hauer wrote:
> Hi Ahmad,
> 
> On Sat, Jun 19, 2021 at 05:26:53AM +0200, Ahmad Fatoum wrote:
>> -}
>> -
>>  static int sscanf_two_digits(char *s, int *res)
>>  {
>>  	char buf[3];
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 6e716fcdcf27..f375fcd67572 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -2479,10 +2479,7 @@ struct device_node *of_get_stdoutpath(void)
> 
> There seems to be a patch missing in this series. I don't have
> of_get_stdoutpath() in my tree.

Sorry, should have mentioned the deep probe series as prerequisite:

https://lore.barebox.org/barebox/20210506092901.25461-8-s.hauer@pengutronix.de/

> 
> Sascha
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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] 6+ messages in thread

* Re: [PATCH 2/2] console: respect baudrate specified in device-tree stdout-path
  2021-06-21  4:59   ` Sascha Hauer
@ 2021-06-21  5:04     ` Ahmad Fatoum
  0 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2021-06-21  5:04 UTC (permalink / raw)
  To: Sascha Hauer, Ahmad Fatoum; +Cc: barebox

Hi,

On 21.06.21 06:59, Sascha Hauer wrote:
> Hi Ahmad,
> 
> On Sat, Jun 19, 2021 at 05:26:54AM +0200, Ahmad Fatoum wrote:
>> -static void console_set_stdoutpath(struct console_device *cdev)
>> +static void console_set_stdoutpath(struct console_device *cdev, unsigned baudrate)
>>  {
>>  	int id;
>>  	char *str;
>> @@ -232,8 +232,7 @@ static void console_set_stdoutpath(struct console_device *cdev)
>>  	if (id < 0)
>>  		return;
>>  
>> -	str = basprintf("console=%s%d,%dn8", cdev->linux_console_name, id,
>> -			  cdev->baudrate);
>> +	str = basprintf("console=%s%d,%dn8", cdev->linux_console_name, id, baudrate);
>>  
>>  	globalvar_add_simple("linux.bootargs.console", str);
>>  
>> @@ -297,6 +296,7 @@ int console_register(struct console_device *newcdev)
>>  	struct device_node *serdev_node = console_is_serdev_node(newcdev);
>>  	struct device_d *dev = &newcdev->class_dev;
>>  	int activate = 0, ret;
>> +	unsigned baudrate = CONFIG_BAUDRATE;
> 
> You initialize baudrate to CONFIG_BAUDRATE here in order to use that as
> a default, ...
> 
>>  
>>  	if (!serdev_node && initialized == CONSOLE_UNINITIALIZED)
>>  		console_init_early();
>> @@ -327,11 +327,16 @@ int console_register(struct console_device *newcdev)
>>  	if (serdev_node)
>>  		return of_platform_populate(serdev_node, NULL, dev);
>>  
>> +	if (newcdev->dev && of_device_is_stdout_path(newcdev->dev, &baudrate)) {
> 
> ...but of_device_is_stdout_path() unconditionally overwrites &baudrate...
> 
>> -int of_device_is_stdout_path(struct device_d *dev)
>> +int of_device_is_stdout_path(struct device_d *dev, unsigned int *baudrate)
>>  {
>> -	if (!dev->device_node)
>> -		return 0;
>> +	unsigned int tmp;
>> +
>> +	if (!dev || !dev->device_node || dev->device_node != of_get_stdoutpath(&tmp))
>> +		return false;
>>  
>> -	return dev->device_node == of_get_stdoutpath();
>> +	*baudrate = tmp;
> 
> ...here. This would break boards which don't have the baudrate given in
> the devicetree.

Ouch. Will fix.

> 
> Sascha
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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] 6+ messages in thread

end of thread, other threads:[~2021-06-21  5:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-19  3:26 [PATCH 1/2] string: implement strchrnul Ahmad Fatoum
2021-06-19  3:26 ` [PATCH 2/2] console: respect baudrate specified in device-tree stdout-path Ahmad Fatoum
2021-06-21  4:59   ` Sascha Hauer
2021-06-21  5:04     ` Ahmad Fatoum
2021-06-21  4:52 ` [PATCH 1/2] string: implement strchrnul Sascha Hauer
2021-06-21  5:03   ` Ahmad Fatoum

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