Patchwork [11/15] x86, olpc: Use a correct version when making up a battery node

login
register
mail settings
Submitter Lubomir Rintel
Date Oct. 10, 2018, 5:22 p.m.
Message ID <20181010172300.317643-12-lkundrak@v3.sk>
Download mbox | patch
Permalink /patch/632587/
State New
Headers show

Comments

Lubomir Rintel - Oct. 10, 2018, 5:22 p.m.
The XO-1 and XO-1.5 batteries apparently differ in an ability to report
ambient temperature. Add a different compatible string to the 1.5
battery.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 arch/x86/platform/olpc/olpc_dt.c | 59 +++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 17 deletions(-)
Andy Shevchenko - Oct. 19, 2018, 1:43 p.m.
On Wed, Oct 10, 2018 at 8:23 PM Lubomir Rintel <lkundrak@v3.sk> wrote:
>
> The XO-1 and XO-1.5 batteries apparently differ in an ability to report
> ambient temperature. Add a different compatible string to the 1.5
> battery.

> +int olpc_dt_compatible_match(phandle node, const char *compat)
>  {
>         char buf[64];
> +       int plen;
> +       char *p;
> +       int len;
> +
> +       plen = olpc_dt_getproperty(node, "compatible", buf, sizeof(buf));
> +       if (plen <= 0)
> +               return 0;
> +
> +       len = strlen(compat);
> +       for (p = buf; p < buf + plen; p += strlen(p) + 1) {
> +               if (strcmp(p, compat) == 0)
> +                       return 1;
> +       }
> +
> +       return 0;
> +}

DT doesn't still have a helper for that?!
I hardly believe in that.

> +               olpc_dt_interpret("\" /battery@0\" find-device"
> +                       " \" olpc,xo1.5-battery\" +compatible"
> +                       " device-end");

Please, don't split string literals.

>                 olpc_dt_interpret("\" /pci/display@1\" find-device"
>                         " new-device"
>                         " \" dcon\" device-name \" olpc,xo1-dcon\" +compatible"
>                         " finish-device device-end");

Yeah, this and similar also needs to be fixed.
Pavel Machek - Nov. 4, 2018, 12:34 p.m.
On Wed 2018-10-10 19:22:56, Lubomir Rintel wrote:
> The XO-1 and XO-1.5 batteries apparently differ in an ability to report
> ambient temperature. Add a different compatible string to the 1.5
> battery.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

Acked-by: Pavel Machek <pavel@ucw.cz>

> ---
>  arch/x86/platform/olpc/olpc_dt.c | 59 +++++++++++++++++++++++---------
>  1 file changed, 42 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/platform/olpc/olpc_dt.c b/arch/x86/platform/olpc/olpc_dt.c
> index d6ee92986920..6e54e116b0c5 100644
> --- a/arch/x86/platform/olpc/olpc_dt.c
> +++ b/arch/x86/platform/olpc/olpc_dt.c
> @@ -218,10 +218,28 @@ static u32 __init olpc_dt_get_board_revision(void)
>  	return be32_to_cpu(rev);
>  }
>  
> -void __init olpc_dt_fixup(void)
> +int olpc_dt_compatible_match(phandle node, const char *compat)
>  {
> -	int r;
>  	char buf[64];
> +	int plen;
> +	char *p;
> +	int len;
> +
> +	plen = olpc_dt_getproperty(node, "compatible", buf, sizeof(buf));
> +	if (plen <= 0)
> +		return 0;
> +
> +	len = strlen(compat);
> +	for (p = buf; p < buf + plen; p += strlen(p) + 1) {
> +		if (strcmp(p, compat) == 0)
> +			return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +void __init olpc_dt_fixup(void)
> +{
>  	phandle node;
>  	u32 board_rev;
>  
> @@ -229,32 +247,33 @@ void __init olpc_dt_fixup(void)
>  	if (!node)
>  		return;
>  
> -	/*
> -	 * If the battery node has a compatible property, we are running a new
> -	 * enough firmware and don't have fixups to make.
> -	 */
> -	r = olpc_dt_getproperty(node, "compatible", buf, sizeof(buf));
> -	if (r > 0)
> -		return;
> -
> -	pr_info("PROM DT: Old firmware detected, applying fixes\n");
> -
> -	/* Add olpc,xo1-battery compatible marker to battery node */
> -	olpc_dt_interpret("\" /battery@0\" find-device"
> -		" \" olpc,xo1-battery\" +compatible"
> -		" device-end");
> -
>  	board_rev = olpc_dt_get_board_revision();
>  	if (!board_rev)
>  		return;
>  
>  	if (board_rev >= olpc_board_pre(0xd0)) {
> +		if (olpc_dt_compatible_match(node, "olpc,xo1.5-battery"))
> +			return;
> +
> +		/* Add olpc,xo1.5-battery compatible marker to battery node */
> +		olpc_dt_interpret("\" /battery@0\" find-device"
> +			" \" olpc,xo1.5-battery\" +compatible"
> +			" device-end");
> +
> +		/* We're running a very old firmware if this is missing. */
> +		if (olpc_dt_compatible_match(node, "olpc,xo1-battery"))
> +			return;
> +
>  		/* XO-1.5: add dcon device */
>  		olpc_dt_interpret("\" /pci/display@1\" find-device"
>  			" new-device"
>  			" \" dcon\" device-name \" olpc,xo1-dcon\" +compatible"
>  			" finish-device device-end");
>  	} else {
> +		/* We're running a very old firmware if this is missing. */
> +		if (olpc_dt_compatible_match(node, "olpc,xo1-battery"))
> +			return;
> +
>  		/* XO-1: add dcon device, mark RTC as olpc,xo1-rtc */
>  		olpc_dt_interpret("\" /pci/display@1,1\" find-device"
>  			" new-device"
> @@ -264,6 +283,11 @@ void __init olpc_dt_fixup(void)
>  			" \" olpc,xo1-rtc\" +compatible"
>  			" device-end");
>  	}
> +
> +	/* Add olpc,xo1-battery compatible marker to battery node */
> +	olpc_dt_interpret("\" /battery@0\" find-device"
> +		" \" olpc,xo1-battery\" +compatible"
> +		" device-end");
>  }
>  
>  void __init olpc_dt_build_devicetree(void)
> @@ -289,6 +313,7 @@ void __init olpc_dt_build_devicetree(void)
>  /* A list of DT node/bus matches that we want to expose as platform devices */
>  static struct of_device_id __initdata of_ids[] = {
>  	{ .compatible = "olpc,xo1-battery" },
> +	{ .compatible = "olpc,xo1.5-battery" },
>  	{ .compatible = "olpc,xo1-dcon" },
>  	{ .compatible = "olpc,xo1-rtc" },
>  	{},
Lubomir Rintel - Nov. 14, 2018, 4:49 p.m.
Hello,

On Fri, 2018-10-19 at 16:43 +0300, Andy Shevchenko wrote:
> On Wed, Oct 10, 2018 at 8:23 PM Lubomir Rintel <lkundrak@v3.sk>
> wrote:
> > The XO-1 and XO-1.5 batteries apparently differ in an ability to
> > report
> > ambient temperature. Add a different compatible string to the 1.5
> > battery.
> > +int olpc_dt_compatible_match(phandle node, const char *compat)
> >  {
> >         char buf[64];
> > +       int plen;
> > +       char *p;
> > +       int len;
> > +
> > +       plen = olpc_dt_getproperty(node, "compatible", buf,
> > sizeof(buf));
> > +       if (plen <= 0)
> > +               return 0;
> > +
> > +       len = strlen(compat);
> > +       for (p = buf; p < buf + plen; p += strlen(p) + 1) {
> > +               if (strcmp(p, compat) == 0)
> > +                       return 1;
> > +       }
> > +
> > +       return 0;
> > +}
> 
> DT doesn't still have a helper for that?!
> I hardly believe in that.

There indeed is of_device_compatible_match() (and
of_find_node_by_phandle() for that matter).

However, these fixups are executed before the DT is built with
of_pdt_build_devicetree(), so I don't think any of_*() routines can be
used at that point.

> > +               olpc_dt_interpret("\" /battery@0\" find-device"
> > +                       " \" olpc,xo1.5-battery\" +compatible"
> > +                       " device-end");
> 
> Please, don't split string literals.
> 
> >                 olpc_dt_interpret("\" /pci/display@1\" find-device"
> >                         " new-device"
> >                         " \" dcon\" device-name \" olpc,xo1-dcon\"
> > +compatible"
> >                         " finish-device device-end");
> 
> Yeah, this and similar also needs to be fixed.

Okay.

Thank you
Lubo

Patch

diff --git a/arch/x86/platform/olpc/olpc_dt.c b/arch/x86/platform/olpc/olpc_dt.c
index d6ee92986920..6e54e116b0c5 100644
--- a/arch/x86/platform/olpc/olpc_dt.c
+++ b/arch/x86/platform/olpc/olpc_dt.c
@@ -218,10 +218,28 @@  static u32 __init olpc_dt_get_board_revision(void)
 	return be32_to_cpu(rev);
 }
 
-void __init olpc_dt_fixup(void)
+int olpc_dt_compatible_match(phandle node, const char *compat)
 {
-	int r;
 	char buf[64];
+	int plen;
+	char *p;
+	int len;
+
+	plen = olpc_dt_getproperty(node, "compatible", buf, sizeof(buf));
+	if (plen <= 0)
+		return 0;
+
+	len = strlen(compat);
+	for (p = buf; p < buf + plen; p += strlen(p) + 1) {
+		if (strcmp(p, compat) == 0)
+			return 1;
+	}
+
+	return 0;
+}
+
+void __init olpc_dt_fixup(void)
+{
 	phandle node;
 	u32 board_rev;
 
@@ -229,32 +247,33 @@  void __init olpc_dt_fixup(void)
 	if (!node)
 		return;
 
-	/*
-	 * If the battery node has a compatible property, we are running a new
-	 * enough firmware and don't have fixups to make.
-	 */
-	r = olpc_dt_getproperty(node, "compatible", buf, sizeof(buf));
-	if (r > 0)
-		return;
-
-	pr_info("PROM DT: Old firmware detected, applying fixes\n");
-
-	/* Add olpc,xo1-battery compatible marker to battery node */
-	olpc_dt_interpret("\" /battery@0\" find-device"
-		" \" olpc,xo1-battery\" +compatible"
-		" device-end");
-
 	board_rev = olpc_dt_get_board_revision();
 	if (!board_rev)
 		return;
 
 	if (board_rev >= olpc_board_pre(0xd0)) {
+		if (olpc_dt_compatible_match(node, "olpc,xo1.5-battery"))
+			return;
+
+		/* Add olpc,xo1.5-battery compatible marker to battery node */
+		olpc_dt_interpret("\" /battery@0\" find-device"
+			" \" olpc,xo1.5-battery\" +compatible"
+			" device-end");
+
+		/* We're running a very old firmware if this is missing. */
+		if (olpc_dt_compatible_match(node, "olpc,xo1-battery"))
+			return;
+
 		/* XO-1.5: add dcon device */
 		olpc_dt_interpret("\" /pci/display@1\" find-device"
 			" new-device"
 			" \" dcon\" device-name \" olpc,xo1-dcon\" +compatible"
 			" finish-device device-end");
 	} else {
+		/* We're running a very old firmware if this is missing. */
+		if (olpc_dt_compatible_match(node, "olpc,xo1-battery"))
+			return;
+
 		/* XO-1: add dcon device, mark RTC as olpc,xo1-rtc */
 		olpc_dt_interpret("\" /pci/display@1,1\" find-device"
 			" new-device"
@@ -264,6 +283,11 @@  void __init olpc_dt_fixup(void)
 			" \" olpc,xo1-rtc\" +compatible"
 			" device-end");
 	}
+
+	/* Add olpc,xo1-battery compatible marker to battery node */
+	olpc_dt_interpret("\" /battery@0\" find-device"
+		" \" olpc,xo1-battery\" +compatible"
+		" device-end");
 }
 
 void __init olpc_dt_build_devicetree(void)
@@ -289,6 +313,7 @@  void __init olpc_dt_build_devicetree(void)
 /* A list of DT node/bus matches that we want to expose as platform devices */
 static struct of_device_id __initdata of_ids[] = {
 	{ .compatible = "olpc,xo1-battery" },
+	{ .compatible = "olpc,xo1.5-battery" },
 	{ .compatible = "olpc,xo1-dcon" },
 	{ .compatible = "olpc,xo1-rtc" },
 	{},