Patchwork [U-Boot] tegra: trimslice: fix a couple typos

login
register
mail settings
Submitter Stephen Warren
Date May 30, 2012, 4:45 p.m.
Message ID <1338396350-16492-1-git-send-email-swarren@wwwdotorg.org>
Download mbox | patch
Permalink /patch/162036/
State Accepted
Commit 1e7e716e80f7a8f599390dd5aa4ae7dce465a1da
Headers show

Comments

Stephen Warren - May 30, 2012, 4:45 p.m.
From: Stephen Warren <swarren@nvidia.com>

Fix the .dts file USB unit addresses not to duplicate each-other.

Fix the board name string to indicate the vendor is Compulab not NVIDIA.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 board/compulab/dts/tegra2-trimslice.dts |    2 +-
 include/configs/trimslice.h             |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
Igor Grinberg - May 31, 2012, 8:47 a.m.
On 05/30/12 19:45, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Fix the .dts file USB unit addresses not to duplicate each-other.
> 
> Fix the board name string to indicate the vendor is Compulab not NVIDIA.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Acked-by: Igor Grinberg <grinberg@compulab.co.il>

Thanks!

> ---
>  board/compulab/dts/tegra2-trimslice.dts |    2 +-
>  include/configs/trimslice.h             |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/board/compulab/dts/tegra2-trimslice.dts b/board/compulab/dts/tegra2-trimslice.dts
> index c707eb8..db79e77 100644
> --- a/board/compulab/dts/tegra2-trimslice.dts
> +++ b/board/compulab/dts/tegra2-trimslice.dts
> @@ -47,7 +47,7 @@
>  		status = "disabled";
>  	};
>  
> -	usb@c5004000 {
> +	usb@c5000000 {
>  		status = "disabled";
>  	};
>  
> diff --git a/include/configs/trimslice.h b/include/configs/trimslice.h
> index dec9125..39c9866 100644
> --- a/include/configs/trimslice.h
> +++ b/include/configs/trimslice.h
> @@ -34,7 +34,7 @@
>  
>  /* High-level configuration options */
>  #define V_PROMPT		"Tegra2 (TrimSlice) # "
> -#define CONFIG_TEGRA2_BOARD_STRING	"NVIDIA Trimslice"
> +#define CONFIG_TEGRA2_BOARD_STRING	"Compulab Trimslice"
>  
>  /* Board-specific serial config */
>  #define CONFIG_SERIAL_MULTI
Marek Vasut - May 31, 2012, 10:13 a.m.
Dear Igor Grinberg,

> On 05/30/12 19:45, Stephen Warren wrote:
> > From: Stephen Warren <swarren@nvidia.com>
> > 
> > Fix the .dts file USB unit addresses not to duplicate each-other.
> > 
> > Fix the board name string to indicate the vendor is Compulab not NVIDIA.
> > 
> > Signed-off-by: Stephen Warren <swarren@nvidia.com>
> 
> Acked-by: Igor Grinberg <grinberg@compulab.co.il>

Do we have one copy of the dts files here and one in Linux kernel tree? Are they 
the same?

M

> Thanks!
> 
> > ---
> > 
> >  board/compulab/dts/tegra2-trimslice.dts |    2 +-
> >  include/configs/trimslice.h             |    2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/board/compulab/dts/tegra2-trimslice.dts
> > b/board/compulab/dts/tegra2-trimslice.dts index c707eb8..db79e77 100644
> > --- a/board/compulab/dts/tegra2-trimslice.dts
> > +++ b/board/compulab/dts/tegra2-trimslice.dts
> > @@ -47,7 +47,7 @@
> > 
> >  		status = "disabled";
> >  	
> >  	};
> > 
> > -	usb@c5004000 {
> > +	usb@c5000000 {
> > 
> >  		status = "disabled";
> >  	
> >  	};
> > 
> > diff --git a/include/configs/trimslice.h b/include/configs/trimslice.h
> > index dec9125..39c9866 100644
> > --- a/include/configs/trimslice.h
> > +++ b/include/configs/trimslice.h
> > @@ -34,7 +34,7 @@
> > 
> >  /* High-level configuration options */
> >  #define V_PROMPT		"Tegra2 (TrimSlice) # "
> > 
> > -#define CONFIG_TEGRA2_BOARD_STRING	"NVIDIA Trimslice"
> > +#define CONFIG_TEGRA2_BOARD_STRING	"Compulab Trimslice"
> > 
> >  /* Board-specific serial config */
> >  #define CONFIG_SERIAL_MULTI

Best regards,
Marek Vasut
Stephen Warren - May 31, 2012, 4:50 p.m.
On 05/31/2012 04:13 AM, Marek Vasut wrote:
> Dear Igor Grinberg,
> 
>> On 05/30/12 19:45, Stephen Warren wrote:
>>> From: Stephen Warren <swarren@nvidia.com>
>>>
>>> Fix the .dts file USB unit addresses not to duplicate each-other.
>>>
>>> Fix the board name string to indicate the vendor is Compulab not NVIDIA.
>>>
>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>
>> Acked-by: Igor Grinberg <grinberg@compulab.co.il>
> 
> Do we have one copy of the dts files here and one in Linux kernel tree? Are they 
> the same?

Both U-Boot and the kernel have their own copies of the .dts files.

In general, the U-Boot copy would be identical to what's in the kernel,
or a pure subset since mostly the kernel's driver support is more
advanced, so we've added more nodes to the DT.

That said, there are unfortunately some bizarre quirks in the way the
U-Boot parses the device tree, such as requiring the /aliases node in
order to enumerate at least some devices, the use of the Tegra clock
binding that hasn't been incorporated into the kernel yet and is used
for both clock and module reset functionality even though it's really
only intended for clock functionality, and various other small
properties that are U-Boot specific (although I forget if we managed to
eliminate these all or not). These all end up causing differences
between the two device tree files:-(
Igor Grinberg - June 1, 2012, 6:38 a.m.
On 05/31/12 19:50, Stephen Warren wrote:
> On 05/31/2012 04:13 AM, Marek Vasut wrote:
>> Dear Igor Grinberg,
>>
>>> On 05/30/12 19:45, Stephen Warren wrote:
>>>> From: Stephen Warren <swarren@nvidia.com>
>>>>
>>>> Fix the .dts file USB unit addresses not to duplicate each-other.
>>>>
>>>> Fix the board name string to indicate the vendor is Compulab not NVIDIA.
>>>>
>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>>
>>> Acked-by: Igor Grinberg <grinberg@compulab.co.il>
>>
>> Do we have one copy of the dts files here and one in Linux kernel tree? Are they 
>> the same?
> 
> Both U-Boot and the kernel have their own copies of the .dts files.
> 
> In general, the U-Boot copy would be identical to what's in the kernel,
> or a pure subset since mostly the kernel's driver support is more
> advanced, so we've added more nodes to the DT.
> 
> That said, there are unfortunately some bizarre quirks in the way the
> U-Boot parses the device tree, such as requiring the /aliases node in
> order to enumerate at least some devices, the use of the Tegra clock
> binding that hasn't been incorporated into the kernel yet and is used
> for both clock and module reset functionality even though it's really
> only intended for clock functionality, and various other small
> properties that are U-Boot specific (although I forget if we managed to
> eliminate these all or not). These all end up causing differences
> between the two device tree files:-(

Thanks for the information.

I don't see any problem with having differences between the .dts files
in kernel and U-Boot, because the way I see it:
The .dts file we have in kernel should provide a way for a DT aware kernel
to boot on even non-DT aware U-Boot (as the DT blob can be appended
to the kernel binary).
The .dts file in U-Boot can have a basic settings and the binary DT blob
can be updated by U-Boot at run time, just before loading kernel.

Also, IIRC, the intension was to remove the kernel .dts files after
"all bootloaders" know to boot the DT kernel...
Stephen Warren - June 1, 2012, 2:50 p.m.
On 06/01/2012 12:38 AM, Igor Grinberg wrote:
> On 05/31/12 19:50, Stephen Warren wrote:
>> On 05/31/2012 04:13 AM, Marek Vasut wrote:
>>> Dear Igor Grinberg,
>>>
>>>> On 05/30/12 19:45, Stephen Warren wrote:
>>>>> From: Stephen Warren <swarren@nvidia.com>
>>>>>
>>>>> Fix the .dts file USB unit addresses not to duplicate each-other.
>>>>>
>>>>> Fix the board name string to indicate the vendor is Compulab not NVIDIA.
>>>>>
>>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>>>
>>>> Acked-by: Igor Grinberg <grinberg@compulab.co.il>
>>>
>>> Do we have one copy of the dts files here and one in Linux kernel tree? Are they 
>>> the same?
>>
>> Both U-Boot and the kernel have their own copies of the .dts files.
>>
>> In general, the U-Boot copy would be identical to what's in the kernel,
>> or a pure subset since mostly the kernel's driver support is more
>> advanced, so we've added more nodes to the DT.
>>
>> That said, there are unfortunately some bizarre quirks in the way the
>> U-Boot parses the device tree, such as requiring the /aliases node in
>> order to enumerate at least some devices, the use of the Tegra clock
>> binding that hasn't been incorporated into the kernel yet and is used
>> for both clock and module reset functionality even though it's really
>> only intended for clock functionality, and various other small
>> properties that are U-Boot specific (although I forget if we managed to
>> eliminate these all or not). These all end up causing differences
>> between the two device tree files:-(
> 
> Thanks for the information.
> 
> I don't see any problem with having differences between the .dts files
> in kernel and U-Boot, because the way I see it:

The issue isn't so much the duplicate files, but differing content.

The whole point about DT is that it's a pure representation of the
hardware; there should be no software-dependent design or data in it.
Put another way, both U-Boot and the Linux kernel (and indeed anything
else) should expect the DT to be written according to the same
"bindings" design. This doesn't preclude the U-Boot DT file being a
strict subset of the kernel file it it needs less information, but what
is in both should match.

> Also, IIRC, the intension was to remove the kernel .dts files after
> "all bootloaders" know to boot the DT kernel...

I don't believe it's anything to do with bootloaders. Bootloaders are
already (in the main) expected to provide the DTB to the kernel as a
separate entity, irrespective of whether the DTB is built by the kernel
boot process or from some other repository. (Although there is
CONFIG_APPENDED_DTB to support cases where this isn't possible, it's
much preferred not to use this). Moving the .dts files out of the kernel
is more purely about finding a place to put them I think.
Marek Vasut - June 1, 2012, 3:45 p.m.
Dear Stephen Warren,

> On 06/01/2012 12:38 AM, Igor Grinberg wrote:
> > On 05/31/12 19:50, Stephen Warren wrote:
> >> On 05/31/2012 04:13 AM, Marek Vasut wrote:
> >>> Dear Igor Grinberg,
> >>> 
> >>>> On 05/30/12 19:45, Stephen Warren wrote:
> >>>>> From: Stephen Warren <swarren@nvidia.com>
> >>>>> 
> >>>>> Fix the .dts file USB unit addresses not to duplicate each-other.
> >>>>> 
> >>>>> Fix the board name string to indicate the vendor is Compulab not
> >>>>> NVIDIA.
> >>>>> 
> >>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >>>> 
> >>>> Acked-by: Igor Grinberg <grinberg@compulab.co.il>
> >>> 
> >>> Do we have one copy of the dts files here and one in Linux kernel tree?
> >>> Are they the same?
> >> 
> >> Both U-Boot and the kernel have their own copies of the .dts files.
> >> 
> >> In general, the U-Boot copy would be identical to what's in the kernel,
> >> or a pure subset since mostly the kernel's driver support is more
> >> advanced, so we've added more nodes to the DT.
> >> 
> >> That said, there are unfortunately some bizarre quirks in the way the
> >> U-Boot parses the device tree, such as requiring the /aliases node in
> >> order to enumerate at least some devices, the use of the Tegra clock
> >> binding that hasn't been incorporated into the kernel yet and is used
> >> for both clock and module reset functionality even though it's really
> >> only intended for clock functionality, and various other small
> >> properties that are U-Boot specific (although I forget if we managed to
> >> eliminate these all or not). These all end up causing differences
> >> between the two device tree files:-(
> > 
> > Thanks for the information.
> > 
> > I don't see any problem with having differences between the .dts files
> 
> > in kernel and U-Boot, because the way I see it:
> The issue isn't so much the duplicate files, but differing content.
> 
> The whole point about DT is that it's a pure representation of the
> hardware; there should be no software-dependent design or data in it.
> Put another way, both U-Boot and the Linux kernel (and indeed anything
> else) should expect the DT to be written according to the same
> "bindings" design. This doesn't preclude the U-Boot DT file being a
> strict subset of the kernel file it it needs less information, but what
> is in both should match.

Thanks for clearing it up!

> 
> > Also, IIRC, the intension was to remove the kernel .dts files after
> > "all bootloaders" know to boot the DT kernel...
> 
> I don't believe it's anything to do with bootloaders. Bootloaders are
> already (in the main) expected to provide the DTB to the kernel as a
> separate entity, irrespective of whether the DTB is built by the kernel
> boot process or from some other repository. (Although there is
> CONFIG_APPENDED_DTB to support cases where this isn't possible, it's
> much preferred not to use this). Moving the .dts files out of the kernel
> is more purely about finding a place to put them I think.
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Best regards,
Marek Vasut
Igor Grinberg - June 3, 2012, 5:50 a.m.
On 06/01/12 18:45, Marek Vasut wrote:
> Dear Stephen Warren,
> 
>> On 06/01/2012 12:38 AM, Igor Grinberg wrote:
>>> On 05/31/12 19:50, Stephen Warren wrote:
>>>> On 05/31/2012 04:13 AM, Marek Vasut wrote:
>>>>> Dear Igor Grinberg,
>>>>>
>>>>>> On 05/30/12 19:45, Stephen Warren wrote:
>>>>>>> From: Stephen Warren <swarren@nvidia.com>
>>>>>>>
>>>>>>> Fix the .dts file USB unit addresses not to duplicate each-other.
>>>>>>>
>>>>>>> Fix the board name string to indicate the vendor is Compulab not
>>>>>>> NVIDIA.
>>>>>>>
>>>>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>>>>>
>>>>>> Acked-by: Igor Grinberg <grinberg@compulab.co.il>
>>>>>
>>>>> Do we have one copy of the dts files here and one in Linux kernel tree?
>>>>> Are they the same?
>>>>
>>>> Both U-Boot and the kernel have their own copies of the .dts files.
>>>>
>>>> In general, the U-Boot copy would be identical to what's in the kernel,
>>>> or a pure subset since mostly the kernel's driver support is more
>>>> advanced, so we've added more nodes to the DT.
>>>>
>>>> That said, there are unfortunately some bizarre quirks in the way the
>>>> U-Boot parses the device tree, such as requiring the /aliases node in
>>>> order to enumerate at least some devices, the use of the Tegra clock
>>>> binding that hasn't been incorporated into the kernel yet and is used
>>>> for both clock and module reset functionality even though it's really
>>>> only intended for clock functionality, and various other small
>>>> properties that are U-Boot specific (although I forget if we managed to
>>>> eliminate these all or not). These all end up causing differences
>>>> between the two device tree files:-(
>>>
>>> Thanks for the information.
>>>
>>> I don't see any problem with having differences between the .dts files
>>
>>> in kernel and U-Boot, because the way I see it:
>> The issue isn't so much the duplicate files, but differing content.
>>
>> The whole point about DT is that it's a pure representation of the
>> hardware; there should be no software-dependent design or data in it.

Right, what I was saying is not that the design or the bindings can differ, but
that the DT blob can be adjusted with data in run time to better represent the
"run time detected" hardware (e.g. extension boards and devices on them,
revisions, serial number, etc.)

>> Put another way, both U-Boot and the Linux kernel (and indeed anything
>> else) should expect the DT to be written according to the same
>> "bindings" design. This doesn't preclude the U-Boot DT file being a
>> strict subset of the kernel file it it needs less information, but what
>> is in both should match.

That is understood completely and of course agreed!

Patch

diff --git a/board/compulab/dts/tegra2-trimslice.dts b/board/compulab/dts/tegra2-trimslice.dts
index c707eb8..db79e77 100644
--- a/board/compulab/dts/tegra2-trimslice.dts
+++ b/board/compulab/dts/tegra2-trimslice.dts
@@ -47,7 +47,7 @@ 
 		status = "disabled";
 	};
 
-	usb@c5004000 {
+	usb@c5000000 {
 		status = "disabled";
 	};
 
diff --git a/include/configs/trimslice.h b/include/configs/trimslice.h
index dec9125..39c9866 100644
--- a/include/configs/trimslice.h
+++ b/include/configs/trimslice.h
@@ -34,7 +34,7 @@ 
 
 /* High-level configuration options */
 #define V_PROMPT		"Tegra2 (TrimSlice) # "
-#define CONFIG_TEGRA2_BOARD_STRING	"NVIDIA Trimslice"
+#define CONFIG_TEGRA2_BOARD_STRING	"Compulab Trimslice"
 
 /* Board-specific serial config */
 #define CONFIG_SERIAL_MULTI