[PATCH] of/base: Replace alias if it already exists

[PATCH] of/base: Replace alias if it already exists

[Ivaylo Dimitrov]

The current code unconditionally adds aliases without check if it already
exists, so it is not possible to alter an alias, from board DT file for
example. Fix that by replacing an alias if it already exists

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 drivers/of/base.c |   34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 03e7fc6..99215f0 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2036,6 +2036,23 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np,
 		 ap->alias, ap->stem, ap->id, of_node_full_name(np));
 }
 
+static int of_alias_replace(struct device_node *np, int id, const char *stem,
+			    int stem_len)
+{
+	struct alias_prop *ap;
+
+	list_for_each_entry(ap, &aliases_lookup, link) {
+		if (strncmp(ap->stem, stem, stem_len))
+			continue;
+
+		if (np == ap->np) {
+			ap->id = id;
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
 /**
  * of_alias_scan - Scan all properties of 'aliases' node
  *
@@ -2092,13 +2109,16 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
 		if (kstrtoint(end, 10, &id) < 0)
 			continue;
 
-		/* Allocate an alias_prop with enough space for the stem */
-		ap = dt_alloc(sizeof(*ap) + len + 1, 4);
-		if (!ap)
-			continue;
-		memset(ap, 0, sizeof(*ap) + len + 1);
-		ap->alias = start;
-		of_alias_add(ap, np, id, start, len);
+		if (of_alias_replace(np, id, start, len)) {
+			/* Allocate an alias_prop with enough space for the stem
+			 */
+			ap = dt_alloc(sizeof(*ap) + len + 1, 4);
+			if (!ap)
+				continue;
+			memset(ap, 0, sizeof(*ap) + len + 1);
+			ap->alias = start;
+			of_alias_add(ap, np, id, start, len);
+		}
 	}
 }
 
-- 
1.7.9.5

Re: [PATCH] of/base: Replace alias if it already exists

[Grant Likely]

On Sun,  1 Jun 2014 15:01:23 +0300, Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> wrote:
> The current code unconditionally adds aliases without check if it already
> exists, so it is not possible to alter an alias, from board DT file for
> example. Fix that by replacing an alias if it already exists
> 

Can you describe a more detailed use-case for altering an alias?

g.

> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> ---
>  drivers/of/base.c |   34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 03e7fc6..99215f0 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2036,6 +2036,23 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np,
>  		 ap->alias, ap->stem, ap->id, of_node_full_name(np));
>  }
>  
> +static int of_alias_replace(struct device_node *np, int id, const char *stem,
> +			    int stem_len)
> +{
> +	struct alias_prop *ap;
> +
> +	list_for_each_entry(ap, &aliases_lookup, link) {
> +		if (strncmp(ap->stem, stem, stem_len))
> +			continue;
> +
> +		if (np == ap->np) {
> +			ap->id = id;
> +			return 0;
> +		}
> +	}
> +
> +	return -ENODEV;
> +}
>  /**
>   * of_alias_scan - Scan all properties of 'aliases' node
>   *
> @@ -2092,13 +2109,16 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
>  		if (kstrtoint(end, 10, &id) < 0)
>  			continue;
>  
> -		/* Allocate an alias_prop with enough space for the stem */
> -		ap = dt_alloc(sizeof(*ap) + len + 1, 4);
> -		if (!ap)
> -			continue;
> -		memset(ap, 0, sizeof(*ap) + len + 1);
> -		ap->alias = start;
> -		of_alias_add(ap, np, id, start, len);
> +		if (of_alias_replace(np, id, start, len)) {
> +			/* Allocate an alias_prop with enough space for the stem
> +			 */
> +			ap = dt_alloc(sizeof(*ap) + len + 1, 4);
> +			if (!ap)
> +				continue;
> +			memset(ap, 0, sizeof(*ap) + len + 1);
> +			ap->alias = start;
> +			of_alias_add(ap, np, id, start, len);
> +		}
>  	}
>  }
>  
> -- 
> 1.7.9.5
> 

Re: [PATCH] of/base: Replace alias if it already exists

[Ivaylo Dimitrov]

On  2.06.2014 17:59, Grant Likely wrote:
> On Sun,  1 Jun 2014 15:01:23 +0300, Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> wrote:
>> The current code unconditionally adds aliases without check if it already
>> exists, so it is not possible to alter an alias, from board DT file for
>> example. Fix that by replacing an alias if it already exists
>>
>
> Can you describe a more detailed use-case for altering an alias?
>
> g.
>

That one http://www.spinics.net/lists/linux-omap/msg107869.html

Ivo

Re: [PATCH] of/base: Replace alias if it already exists

[Grant Likely]

On Mon, 02 Jun 2014 19:07:01 +0300, Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> wrote:
> 
> 
> On  2.06.2014 17:59, Grant Likely wrote:
> > On Sun,  1 Jun 2014 15:01:23 +0300, Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> wrote:
> >> The current code unconditionally adds aliases without check if it already
> >> exists, so it is not possible to alter an alias, from board DT file for
> >> example. Fix that by replacing an alias if it already exists
> >>
> >
> > Can you describe a more detailed use-case for altering an alias?
> >
> > g.
> >
> 
> That one http://www.spinics.net/lists/linux-omap/msg107869.html

So, if I understand correctly, the .dtsi file sets up aliases like so:

aliases {
	i2c0 = &i2c1;
	i2c1 = &i2c2;
	i2c2 = &i2c3;
	i2c3 = &i2c4;
	i2c4 = &i2c5;
};

and the board file does this:


aliases {
	i2c1 = &i2c1;
	i2c2 = &i2c2;
	i2c3 = &i2c3;
	i2c4 = &i2c4;
	i2c5 = &i2c5;
};

Which results in a dtb that looks like this:

aliases {
	i2c0 = &i2c1;
	i2c1 = &i2c1;
	i2c2 = &i2c2;
	i2c3 = &i2c3;
	i2c4 = &i2c4;
	i2c5 = &i2c5;
};

Do I understand correctly?

The way aliases works, it is completely valid to have multiple aliases
pointing at the same node. There are also no ordering guarantees about
which comes first, so though this patch might work for you at the
moment, there is no guarantee that it will continue to do so. If the
i2c0 alias is invalid for that platform, then it must be removed.

Unfortunately, the overlays have no mechanism for removing properties at
this time. Can you try putting the following into your board dts file
and see if it works?

aliases {
	i2c0;		/* Deactivate the old alias by making it blank */
	i2c1 = &i2c1;
	i2c2 = &i2c2;
	i2c3 = &i2c3;
	i2c4 = &i2c4;
	i2c5 = &i2c5;
};

g.

Re: [PATCH] of/base: Replace alias if it already exists

[Ivaylo Dimitrov]

On  3.06.2014 11:53, Grant Likely wrote:

> So, if I understand correctly, the .dtsi file sets up aliases like so:
>
> aliases {
> 	i2c0 = &i2c1;
> 	i2c1 = &i2c2;
> 	i2c2 = &i2c3;
> 	i2c3 = &i2c4;
> 	i2c4 = &i2c5;
> };
>
> and the board file does this:
>
>
> aliases {
> 	i2c1 = &i2c1;
> 	i2c2 = &i2c2;
> 	i2c3 = &i2c3;
> 	i2c4 = &i2c4;
> 	i2c5 = &i2c5;
> };
>
> Which results in a dtb that looks like this:
>
> aliases {
> 	i2c0 = &i2c1;
> 	i2c1 = &i2c1;
> 	i2c2 = &i2c2;
> 	i2c3 = &i2c3;
> 	i2c4 = &i2c4;
> 	i2c5 = &i2c5;
> };
>
> Do I understand correctly?
>

I guess this is the resulting dtb, at least kernel assigns "incorrect" 
index(0) only to i2c1 bus.

> The way aliases works, it is completely valid to have multiple aliases
> pointing at the same node. There are also no ordering guarantees about
> which comes first, so though this patch might work for you at the
> moment, there is no guarantee that it will continue to do so. If the
> i2c0 alias is invalid for that platform, then it must be removed.
>

I see.

> Unfortunately, the overlays have no mechanism for removing properties at
> this time. Can you try putting the following into your board dts file
> and see if it works?
>
> aliases {
> 	i2c0;		/* Deactivate the old alias by making it blank */
> 	i2c1 = &i2c1;
> 	i2c2 = &i2c2;
> 	i2c3 = &i2c3;
> 	i2c4 = &i2c4;
> 	i2c5 = &i2c5;
> };
>
> g.
>

Will do, as soon as I get back home.

Thanks,
Ivo

Re: [PATCH] of/base: Replace alias if it already exists

[Ivaylo Dimitrov]

On  3.06.2014 12:58, Ivaylo Dimitrov wrote:
>
>
> On  3.06.2014 11:53, Grant Likely wrote:
>
[...] Can you try putting the following into your board dts file
>> and see if it works?
>>
>> aliases {
>>     i2c0;        /* Deactivate the old alias by making it blank */
>>     i2c1 = &i2c1;
>>     i2c2 = &i2c2;
>>     i2c3 = &i2c3;
>>     i2c4 = &i2c4;
>>     i2c5 = &i2c5;
>> };
>>
>> g.
>>
>
> Will do, as soon as I get back home.
>
> Thanks,
> Ivo

Seems to fix the problem (tested it with qemu), will send a proper patch.

Thanks!,
Ivo

Re: [PATCH] of/base: Replace alias if it already exists

[Grant Likely]

On Tue, 03 Jun 2014 20:09:24 +0300, Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> wrote:
> 
> 
> On  3.06.2014 12:58, Ivaylo Dimitrov wrote:
> >
> >
> > On  3.06.2014 11:53, Grant Likely wrote:
> >
> [...] Can you try putting the following into your board dts file
> >> and see if it works?
> >>
> >> aliases {
> >>     i2c0;        /* Deactivate the old alias by making it blank */
> >>     i2c1 = &i2c1;
> >>     i2c2 = &i2c2;
> >>     i2c3 = &i2c3;
> >>     i2c4 = &i2c4;
> >>     i2c5 = &i2c5;
> >> };
> >>
> >> g.
> >>
> >
> > Will do, as soon as I get back home.
> >
> > Thanks,
> > Ivo
> 
> Seems to fix the problem (tested it with qemu), will send a proper patch.

Great, I'm glad it worked.

g.