[PATCH v2 2/7] fdt: Add function to allow aliases to refer to multiple nodes

[PATCH v2 2/7] fdt: Add function to allow aliases to refer to multiple nodes

[Simon Glass]

Some devices can deal with multiple compatible properties. The devices
need to know which nodes to bind to which features. For example an
I2C driver which supports two different controller types will want to
know which type it is dealing with in each case.

The new fdtdec_add_aliases_for_id() function deals with this by allowing
the driver to search for additional compatible nodes for a different ID.
It can then detect the new ones and perform appropriate processing.

Another option considered was to return a tuple (node offset, compat id)
and have the function be passed a list of compatible IDs. This is more
overhead for the common case though. We may add such a function later if
more drivers in U-Boot require it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/fdtdec.h |   15 +++++++++++++++
 lib/fdtdec.c     |   16 ++++++++++++----
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/include/fdtdec.h b/include/fdtdec.h
index bd2222c..e6d63f9 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -200,6 +200,21 @@ int fdtdec_find_aliases_for_id(const void *blob, const char *name,
 			enum fdt_compat_id id, int *node_list, int maxcount);
 
 /*
+ * This function is similar to fdtdec_find_aliases_for_id() except that it
+ * adds to the node_list that is passed in. Any 0 elements are considered
+ * available for allocation - others are considered already used and are
+ * skipped.
+ *
+ * You can use this by calling fdtdec_find_aliases_for_id() with an
+ * uninitialised array, then setting the elements that are returned to -1,
+ * say, then calling this function, perhaps with a different compat id.
+ * Any elements you get back that are >0 are new nodes added by the call
+ * to this function.
+ */
+int fdtdec_add_aliases_for_id(const void *blob, const char *name,
+			enum fdt_compat_id id, int *node_list, int maxcount);
+
+/*
  * Get the name for a compatible ID
  *
  * @param id		Compatible ID to look for
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index daf2c7e..ddd56b9 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -151,10 +151,18 @@ int fdtdec_next_alias(const void *blob, const char *name,
 	return node;
 }
 
-/* TODO: Can we tighten this code up a little? */
 int fdtdec_find_aliases_for_id(const void *blob, const char *name,
 			enum fdt_compat_id id, int *node_list, int maxcount)
 {
+	memset(node_list, '\0', sizeof(*node_list) * maxcount);
+
+	return fdtdec_add_aliases_for_id(blob, name, id, node_list, maxcount);
+}
+
+/* TODO: Can we tighten this code up a little? */
+int fdtdec_add_aliases_for_id(const void *blob, const char *name,
+			enum fdt_compat_id id, int *node_list, int maxcount)
+{
 	int name_len = strlen(name);
 	int nodes[maxcount];
 	int num_found = 0;
@@ -184,8 +192,6 @@ int fdtdec_find_aliases_for_id(const void *blob, const char *name,
 		       __func__, name);
 
 	/* Now find all the aliases */
-	memset(node_list, '\0', sizeof(*node_list) * maxcount);
-
 	for (offset = fdt_first_property_offset(blob, alias_node);
 			offset > 0;
 			offset = fdt_next_property_offset(blob, offset)) {
@@ -232,11 +238,13 @@ int fdtdec_find_aliases_for_id(const void *blob, const char *name,
 		 * it as done.
 		 */
 		if (fdtdec_get_is_enabled(blob, node)) {
+			if (node_list[number])
+				continue;
 			node_list[number] = node;
 			if (number >= num_found)
 				num_found = number + 1;
 		}
-		nodes[j] = 0;
+		nodes[found] = 0;
 	}
 
 	/* Add any nodes not mentioned by an alias */
-- 
1.7.7.3

Re: [PATCH v2 2/7] fdt: Add function to allow aliases to refer to multiple nodes

[Stephen Warren]

On 01/12/2012 12:00 PM, Simon Glass wrote:
> Some devices can deal with multiple compatible properties. The devices
> need to know which nodes to bind to which features. For example an
> I2C driver which supports two different controller types will want to
> know which type it is dealing with in each case.
> 
> The new fdtdec_add_aliases_for_id() function deals with this by allowing
> the driver to search for additional compatible nodes for a different ID.
> It can then detect the new ones and perform appropriate processing.
> 
> Another option considered was to return a tuple (node offset, compat id)
> and have the function be passed a list of compatible IDs. This is more
> overhead for the common case though. We may add such a function later if
> more drivers in U-Boot require it.

> +int fdtdec_add_aliases_for_id(const void *blob, const char *name,
> +			enum fdt_compat_id id, int *node_list, int maxcount)
...
> @@ -232,11 +238,13 @@ int fdtdec_find_aliases_for_id(const void *blob, const char *name,
>  		 * it as done.
>  		 */
>  		if (fdtdec_get_is_enabled(blob, node)) {
> +			if (node_list[number])
> +				continue;
>  			node_list[number] = node;
>  			if (number >= num_found)
>  				num_found = number + 1;
>  		}
> -		nodes[j] = 0;
> +		nodes[found] = 0;
>  	}
>  
>  	/* Add any nodes not mentioned by an alias */

Aren't those two changes really bug-fixes to the underlying patch rather
than part of this enhancement?

I'm not convinced this patch is correct in all cases. It'll probably
work fine for simply device-trees though. Consider the following case:

4 device nodes
A: compat=i2c alias for usb0
B: compat=i2c no alias
C: compat=i2c alias for usb2
D: compat=i2cx alias for usb1

First, we search for all compat=i2c, the list comes back:

0=A
1=B
2=C

Then we search for all compat=i2cx, the list comes back:

-1
-1
-1
D

So D's alias of 1 isn't honored even though if both IDs were searched
for together, it would have been.

Also, what about the scenario where a driver searches for both
nvidia,tegra20-i2c then later nvidia,tegra30-i2c, given that all the DT
nodes are probably compatible with both?

-- 
nvpublic

Re: [PATCH v2 2/7] fdt: Add function to allow aliases to refer to multiple nodes

[Simon Glass]

Hi Stephen,

On Thu, Jan 19, 2012 at 12:49 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/12/2012 12:00 PM, Simon Glass wrote:
>> Some devices can deal with multiple compatible properties. The devices
>> need to know which nodes to bind to which features. For example an
>> I2C driver which supports two different controller types will want to
>> know which type it is dealing with in each case.
>>
>> The new fdtdec_add_aliases_for_id() function deals with this by allowing
>> the driver to search for additional compatible nodes for a different ID.
>> It can then detect the new ones and perform appropriate processing.
>>
>> Another option considered was to return a tuple (node offset, compat id)
>> and have the function be passed a list of compatible IDs. This is more
>> overhead for the common case though. We may add such a function later if
>> more drivers in U-Boot require it.
>
>> +int fdtdec_add_aliases_for_id(const void *blob, const char *name,
>> +                     enum fdt_compat_id id, int *node_list, int maxcount)
> ...
>> @@ -232,11 +238,13 @@ int fdtdec_find_aliases_for_id(const void *blob, const char *name,
>>                * it as done.
>>                */
>>               if (fdtdec_get_is_enabled(blob, node)) {
>> +                     if (node_list[number])
>> +                             continue;
>>                       node_list[number] = node;
>>                       if (number >= num_found)
>>                               num_found = number + 1;
>>               }
>> -             nodes[j] = 0;
>> +             nodes[found] = 0;
>>       }
>>
>>       /* Add any nodes not mentioned by an alias */
>
> Aren't those two changes really bug-fixes to the underlying patch rather
> than part of this enhancement?

Clean-ups maybe. The first change can change behaviour when someone
has two aliases the same. The second is equivalent (j == found) but
nicer I think.

>
> I'm not convinced this patch is correct in all cases. It'll probably
> work fine for simply device-trees though. Consider the following case:
>
> 4 device nodes
> A: compat=i2c alias for usb0
> B: compat=i2c no alias
> C: compat=i2c alias for usb2
> D: compat=i2cx alias for usb1
>
> First, we search for all compat=i2c, the list comes back:
>
> 0=A
> 1=B
> 2=C
>
> Then we search for all compat=i2cx, the list comes back:
>
> -1
> -1
> -1
> D
>
> So D's alias of 1 isn't honored even though if both IDs were searched
> for together, it would have been.

Do we really want that behaviour? Overall I think it is a bit odd to
have devices with no aliases if you actually care about the ordering.
At least in U-Boot ordering is important. The simple fix above is to
provide an alias for everything, which is what we do. If we do want
the behaviour you suggest then I will need the change to doing it in
one pass.

>
> Also, what about the scenario where a driver searches for both
> nvidia,tegra20-i2c then later nvidia,tegra30-i2c, given that all the DT
> nodes are probably compatible with both?

If all DT notes are compatible with both, why are you searching for
one and then the other? Why not just search for one?

It sounds like a decision here as to whether we want a one-pass
algorithm with a list of compatible nodes, or the current additive
priorty-based approach.

Regards,
Simon

>
> --
> nvpublic

Re: [PATCH v2 2/7] fdt: Add function to allow aliases to refer to multiple nodes

[Stephen Warren]

On 01/19/2012 04:45 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Thu, Jan 19, 2012 at 12:49 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> On 01/12/2012 12:00 PM, Simon Glass wrote:
>>> Some devices can deal with multiple compatible properties. The devices
>>> need to know which nodes to bind to which features. For example an
>>> I2C driver which supports two different controller types will want to
>>> know which type it is dealing with in each case.
>>>
>>> The new fdtdec_add_aliases_for_id() function deals with this by allowing
>>> the driver to search for additional compatible nodes for a different ID.
>>> It can then detect the new ones and perform appropriate processing.
>>>
>>> Another option considered was to return a tuple (node offset, compat id)
>>> and have the function be passed a list of compatible IDs. This is more
>>> overhead for the common case though. We may add such a function later if
>>> more drivers in U-Boot require it.
...
>> I'm not convinced this patch is correct in all cases. It'll probably
>> work fine for simply device-trees though. Consider the following case:
>>
>> 4 device nodes
>> A: compat=i2c alias for usb0
>> B: compat=i2c no alias
>> C: compat=i2c alias for usb2
>> D: compat=i2cx alias for usb1
>>
>> First, we search for all compat=i2c, the list comes back:
>>
>> 0=A
>> 1=B
>> 2=C
>>
>> Then we search for all compat=i2cx, the list comes back:
>>
>> -1
>> -1
>> -1
>> D
>>
>> So D's alias of 1 isn't honored even though if both IDs were searched
>> for together, it would have been.
> 
> Do we really want that behaviour? Overall I think it is a bit odd to
> have devices with no aliases if you actually care about the ordering.
> At least in U-Boot ordering is important. The simple fix above is to
> provide an alias for everything, which is what we do. If we do want
> the behaviour you suggest then I will need the change to doing it in
> one pass.

I think that behaviour is the intended given that DT content. Despite
the fact that content is admittedly a little twisted and perhaps not
commonly useful, since this is user-supplied content in general, I don't
think we can simply not accept it or not implement the fairly obvious
intent.

>> Also, what about the scenario where a driver searches for both
>> nvidia,tegra20-i2c then later nvidia,tegra30-i2c, given that all the DT
>> nodes are probably compatible with both?
> 
> If all DT notes are compatible with both, why are you searching for
> one and then the other? Why not just search for one?

I suppose one /shouldn't/ need to.

I was thinking of supporting the case where somebody only wrote
compatible = "nvidia,tegra30-i2c" and left out the Tegra20 variant.
Still, is arguably a bug, and that won't work in the kernel either since
we're not going around adding all the new compatible values into the
drivers when the devices are compatible with the existing HW anyway.

I guess if we do get some legitimate case that trips this issue, we can
just fix it then, since it's entirely isolated to the code and doesn't
impact anything externally visible etc.

-- 
nvpublic

Re: [PATCH v2 2/7] fdt: Add function to allow aliases to refer to multiple nodes

[Simon Glass]

Hi Stephen,

On Thu, Jan 19, 2012 at 4:17 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/19/2012 04:45 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Thu, Jan 19, 2012 at 12:49 PM, Stephen Warren <swarren@nvidia.com> wrote:
>>> On 01/12/2012 12:00 PM, Simon Glass wrote:
>>>> Some devices can deal with multiple compatible properties. The devices
>>>> need to know which nodes to bind to which features. For example an
>>>> I2C driver which supports two different controller types will want to
>>>> know which type it is dealing with in each case.
>>>>
>>>> The new fdtdec_add_aliases_for_id() function deals with this by allowing
>>>> the driver to search for additional compatible nodes for a different ID.
>>>> It can then detect the new ones and perform appropriate processing.
>>>>
>>>> Another option considered was to return a tuple (node offset, compat id)
>>>> and have the function be passed a list of compatible IDs. This is more
>>>> overhead for the common case though. We may add such a function later if
>>>> more drivers in U-Boot require it.
> ...
>>> I'm not convinced this patch is correct in all cases. It'll probably
>>> work fine for simply device-trees though. Consider the following case:
>>>
>>> 4 device nodes
>>> A: compat=i2c alias for usb0
>>> B: compat=i2c no alias
>>> C: compat=i2c alias for usb2
>>> D: compat=i2cx alias for usb1
>>>
>>> First, we search for all compat=i2c, the list comes back:
>>>
>>> 0=A
>>> 1=B
>>> 2=C
>>>
>>> Then we search for all compat=i2cx, the list comes back:
>>>
>>> -1
>>> -1
>>> -1
>>> D
>>>
>>> So D's alias of 1 isn't honored even though if both IDs were searched
>>> for together, it would have been.
>>
>> Do we really want that behaviour? Overall I think it is a bit odd to
>> have devices with no aliases if you actually care about the ordering.
>> At least in U-Boot ordering is important. The simple fix above is to
>> provide an alias for everything, which is what we do. If we do want
>> the behaviour you suggest then I will need the change to doing it in
>> one pass.
>
> I think that behaviour is the intended given that DT content. Despite
> the fact that content is admittedly a little twisted and perhaps not
> commonly useful, since this is user-supplied content in general, I don't
> think we can simply not accept it or not implement the fairly obvious
> intent.
>
>>> Also, what about the scenario where a driver searches for both
>>> nvidia,tegra20-i2c then later nvidia,tegra30-i2c, given that all the DT
>>> nodes are probably compatible with both?
>>
>> If all DT notes are compatible with both, why are you searching for
>> one and then the other? Why not just search for one?
>
> I suppose one /shouldn't/ need to.
>
> I was thinking of supporting the case where somebody only wrote
> compatible = "nvidia,tegra30-i2c" and left out the Tegra20 variant.
> Still, is arguably a bug, and that won't work in the kernel either since
> we're not going around adding all the new compatible values into the
> drivers when the devices are compatible with the existing HW anyway.
>
> I guess if we do get some legitimate case that trips this issue, we can
> just fix it then, since it's entirely isolated to the code and doesn't
> impact anything externally visible etc.

Hmmm well perhaps I should add a code comment and see how we go. It
would be nice if this situation could be spotted. I am concerned about
anything that is non-obvious here. Will take a look but otherwise it
sounds like we can punt this until later (unless we decide to move to
the one-pass option).

Anyway this is in a place by itself with unit tests so we should be
able to fiddle if needed.

Regards,
Simon

>
> --
> nvpublic

[PATCH v2 3/7] tegra: fdt: Add extra I2C bindings for U-Boot

[Simon Glass]

Add U-Boot's peripheral clock information to the Tegra20 device tree file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Adjust definitions to fit new peripheral clock bindings
- Change 'speed' to 'clock-frequency'
- Remove u-boot,pinmux binding (sadly)

 arch/arm/dts/tegra20.dtsi |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
index ca7b523..963cf27 100644
--- a/arch/arm/dts/tegra20.dtsi
+++ b/arch/arm/dts/tegra20.dtsi
@@ -45,6 +45,8 @@
 		compatible = "nvidia,tegra20-i2c";
 		reg = <0x7000C000 0x100>;
 		interrupts = < 70 >;
+		clock-frequency = <100000>;
+		clocks = <&periph_clk 12>;	// PERIPH_ID_I2C1
 	};
 
 	i2c@7000c400 {
@@ -53,6 +55,8 @@
 		compatible = "nvidia,tegra20-i2c";
 		reg = <0x7000C400 0x100>;
 		interrupts = < 116 >;
+		clock-frequency = <100000>;
+		clocks = <&periph_clk 54>;	// PERIPH_ID_I2C2
 	};
 
 	i2c@7000c500 {
@@ -61,14 +65,18 @@
 		compatible = "nvidia,tegra20-i2c";
 		reg = <0x7000C500 0x100>;
 		interrupts = < 124 >;
+		clock-frequency = <100000>;
+		clocks = <&periph_clk 67>;	// PERIPH_ID_I2C3
 	};
 
 	i2c@7000d000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "nvidia,tegra20-i2c";
+		compatible = "nvidia,tegra20-i2c-dvc";
 		reg = <0x7000D000 0x200>;
 		interrupts = < 85 >;
+		clock-frequency = <100000>;
+		clocks = <&periph_clk 47>;	// PERIPH_ID_DVC_I2C
 	};
 
 	i2s@70002800 {
-- 
1.7.7.3

Re: [PATCH v2 3/7] tegra: fdt: Add extra I2C bindings for U-Boot

[Heiko Schocher]

Hello Simon,

Simon Glass wrote:
> Add U-Boot's peripheral clock information to the Tegra20 device tree file.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Adjust definitions to fit new peripheral clock bindings
> - Change 'speed' to 'clock-frequency'
> - Remove u-boot,pinmux binding (sadly)
> 
>  arch/arm/dts/tegra20.dtsi |   10 +++++++++-

Hmm.. we have no such a directory in the u-boot source tree ... ?
I think, this patch is for the linux tree?

[...]

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Re: [U-Boot] [PATCH v2 3/7] tegra: fdt: Add extra I2C bindings for U-Boot

[Simon Glass]

Hi Heiko,

On Thu, Jan 12, 2012 at 10:31 PM, Heiko Schocher <hs-ynQEQJNshbs@public.gmane.org> wrote:
> Hello Simon,
>
> Simon Glass wrote:
>> Add U-Boot's peripheral clock information to the Tegra20 device tree file.
>>
>> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> ---
>> Changes in v2:
>> - Adjust definitions to fit new peripheral clock bindings
>> - Change 'speed' to 'clock-frequency'
>> - Remove u-boot,pinmux binding (sadly)
>>
>>  arch/arm/dts/tegra20.dtsi |   10 +++++++++-
>
> Hmm.. we have no such a directory in the u-boot source tree ... ?
> I think, this patch is for the linux tree?

No, it's for a future U-Boot :-) It builds on USB series (which brings
in the basic device tree file).

Regards,
Simon

>
> [...]
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Re: [PATCH v2 3/7] tegra: fdt: Add extra I2C bindings for U-Boot

[Stephen Warren]

On 01/12/2012 12:00 PM, Simon Glass wrote:
> Add U-Boot's peripheral clock information to the Tegra20 device tree file.

> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
> index ca7b523..963cf27 100644
> --- a/arch/arm/dts/tegra20.dtsi
> +++ b/arch/arm/dts/tegra20.dtsi
> @@ -45,6 +45,8 @@
>  		compatible = "nvidia,tegra20-i2c";
>  		reg = <0x7000C000 0x100>;
>  		interrupts = < 70 >;
> +		clock-frequency = <100000>;

That's board-specific information. I'd rather not include it in the SoC
.dtsi file even as a default; that way, it forces people to think about
the correct value and put it in their board file, whereas including a
default means people probably won't think about it.

> +		clocks = <&periph_clk 12>;	// PERIPH_ID_I2C1

Of course, that'll need updating based on the resolution of the clock
binding thread I started.

-- 
nvpublic

Re: [PATCH v2 3/7] tegra: fdt: Add extra I2C bindings for U-Boot

[Simon Glass]

Hi Stephen,

On Thu, Jan 19, 2012 at 12:51 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/12/2012 12:00 PM, Simon Glass wrote:
>> Add U-Boot's peripheral clock information to the Tegra20 device tree file.
>
>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
>> index ca7b523..963cf27 100644
>> --- a/arch/arm/dts/tegra20.dtsi
>> +++ b/arch/arm/dts/tegra20.dtsi
>> @@ -45,6 +45,8 @@
>>               compatible = "nvidia,tegra20-i2c";
>>               reg = <0x7000C000 0x100>;
>>               interrupts = < 70 >;
>> +             clock-frequency = <100000>;
>
> That's board-specific information. I'd rather not include it in the SoC
> .dtsi file even as a default; that way, it forces people to think about
> the correct value and put it in their board file, whereas including a
> default means people probably won't think about it.

OK I moved this into the board file - sadly this means everyone will
need to specify it here. Or should we have a default?

>
>> +             clocks = <&periph_clk 12>;      // PERIPH_ID_I2C1
>
> Of course, that'll need updating based on the resolution of the clock
> binding thread I started.

Yes we can come back to this when resolved.

Regards,
Simon

>
> --
> nvpublic

Re: [PATCH v2 3/7] tegra: fdt: Add extra I2C bindings for U-Boot

[Stephen Warren]

On 01/22/2012 10:41 AM, Simon Glass wrote:
> Hi Stephen,
> 
> On Thu, Jan 19, 2012 at 12:51 PM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> On 01/12/2012 12:00 PM, Simon Glass wrote:
>>> Add U-Boot's peripheral clock information to the Tegra20 device tree file.
>>
>>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
>>> index ca7b523..963cf27 100644
>>> --- a/arch/arm/dts/tegra20.dtsi
>>> +++ b/arch/arm/dts/tegra20.dtsi
>>> @@ -45,6 +45,8 @@
>>>               compatible = "nvidia,tegra20-i2c";
>>>               reg = <0x7000C000 0x100>;
>>>               interrupts = < 70 >;
>>> +             clock-frequency = <100000>;
>>
>> That's board-specific information. I'd rather not include it in the SoC
>> .dtsi file even as a default; that way, it forces people to think about
>> the correct value and put it in their board file, whereas including a
>> default means people probably won't think about it.
> 
> OK I moved this into the board file - sadly this means everyone will
> need to specify it here. Or should we have a default?

I think I'd rather not have the default. That way, people need to think
about the correct value to use in their board file, rather than having
it magically work without having validated the value.

If people disagree, we'd need to update the kernel's tegra[23]0.dtsi,
since they assume ${board}.dts will set this property.

-- 
nvpublic

Re: [PATCH v2 3/7] tegra: fdt: Add extra I2C bindings for U-Boot

[Simon Glass]

Hi Stephen,

On Mon, Jan 23, 2012 at 10:25 AM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/22/2012 10:41 AM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Thu, Jan 19, 2012 at 12:51 PM, Stephen Warren <swarren@nvidia.com> wrote:
>>> On 01/12/2012 12:00 PM, Simon Glass wrote:
>>>> Add U-Boot's peripheral clock information to the Tegra20 device tree file.
>>>
>>>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
>>>> index ca7b523..963cf27 100644
>>>> --- a/arch/arm/dts/tegra20.dtsi
>>>> +++ b/arch/arm/dts/tegra20.dtsi
>>>> @@ -45,6 +45,8 @@
>>>>               compatible = "nvidia,tegra20-i2c";
>>>>               reg = <0x7000C000 0x100>;
>>>>               interrupts = < 70 >;
>>>> +             clock-frequency = <100000>;
>>>
>>> That's board-specific information. I'd rather not include it in the SoC
>>> .dtsi file even as a default; that way, it forces people to think about
>>> the correct value and put it in their board file, whereas including a
>>> default means people probably won't think about it.
>>
>> OK I moved this into the board file - sadly this means everyone will
>> need to specify it here. Or should we have a default?
>
> I think I'd rather not have the default. That way, people need to think
> about the correct value to use in their board file, rather than having
> it magically work without having validated the value.
>
> If people disagree, we'd need to update the kernel's tegra[23]0.dtsi,
> since they assume ${board}.dts will set this property.

OK that's fine with me.

Regards,
Simon

>
> --
> nvpublic