[PATCH/RFC,2/2] arm64: dts: renesas: salvator-common: Fix adv7482 decimal unit addresses

Message ID 1528984088-24801-3-git-send-email-geert+renesas@glider.be (mailing list archive)
State Superseded, archived
Headers

Commit Message

Geert Uytterhoeven June 14, 2018, 1:48 p.m. UTC
  With recent dtc and W=1:

    ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@10: graph node unit address error, expected "a"
    ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@11: graph node unit address error, expected "b"

Unit addresses are always hexadecimal (without prefix), while the bases
of reg property values depend on their prefixes.

Fixes: 908001d778eba06e ("arm64: dts: renesas: salvator-common: Add ADV7482 support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Kieran Bingham June 14, 2018, 3:53 p.m. UTC | #1
Hi Geert,

On 14/06/18 14:48, Geert Uytterhoeven wrote:
> With recent dtc and W=1:
> 
>      ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@10: graph node unit address error, expected "a"
>      ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@11: graph node unit address error, expected "b"
> 
> Unit addresses are always hexadecimal (without prefix), while the bases
> of reg property values depend on their prefixes.
> 
> Fixes: 908001d778eba06e ("arm64: dts: renesas: salvator-common: Add ADV7482 support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>   arch/arm64/boot/dts/renesas/salvator-common.dtsi | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> index 320250d708c3bbab..47088206cc052a15 100644
> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> @@ -437,7 +437,7 @@
>   			};
>   		};
>   
> -		port@10 {
> +		port@a {
>   			reg = <10>;

This looks a bit ugly with the different mappings;
If we must move to 'port@a', I think reg needs to be <0xa>, to avoid 
confusion. (but I personally prefer port@10, reg = <10> here)


--
Kieran


>   
>   			adv7482_txa: endpoint {
> @@ -447,7 +447,7 @@
>   			};
>   		};
>   
> -		port@11 {
> +		port@b {
>   			reg = <11>;
>   
>   			adv7482_txb: endpoint {
>
  
Rob Herring (Arm) June 26, 2018, 7:57 p.m. UTC | #2
On Thu, Jun 14, 2018 at 03:48:08PM +0200, Geert Uytterhoeven wrote:
> With recent dtc and W=1:
> 
>     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@10: graph node unit address error, expected "a"
>     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@11: graph node unit address error, expected "b"
> 
> Unit addresses are always hexadecimal (without prefix), while the bases
> of reg property values depend on their prefixes.
> 
> Fixes: 908001d778eba06e ("arm64: dts: renesas: salvator-common: Add ADV7482 support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>
  
Simon Horman June 27, 2018, 3:10 p.m. UTC | #3
On Tue, Jun 26, 2018 at 01:57:47PM -0600, Rob Herring wrote:
> On Thu, Jun 14, 2018 at 03:48:08PM +0200, Geert Uytterhoeven wrote:
> > With recent dtc and W=1:
> > 
> >     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@10: graph node unit address error, expected "a"
> >     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@11: graph node unit address error, expected "b"
> > 
> > Unit addresses are always hexadecimal (without prefix), while the bases
> > of reg property values depend on their prefixes.
> > 
> > Fixes: 908001d778eba06e ("arm64: dts: renesas: salvator-common: Add ADV7482 support")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

Geert, shall I apply this?
  
Geert Uytterhoeven June 27, 2018, 4:40 p.m. UTC | #4
Hi Simon,

On Wed, Jun 27, 2018 at 5:10 PM Simon Horman <horms@verge.net.au> wrote:
> On Tue, Jun 26, 2018 at 01:57:47PM -0600, Rob Herring wrote:
> > On Thu, Jun 14, 2018 at 03:48:08PM +0200, Geert Uytterhoeven wrote:
> > > With recent dtc and W=1:
> > >
> > >     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@10: graph node unit address error, expected "a"
> > >     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@11: graph node unit address error, expected "b"
> > >
> > > Unit addresses are always hexadecimal (without prefix), while the bases
> > > of reg property values depend on their prefixes.
> > >
> > > Fixes: 908001d778eba06e ("arm64: dts: renesas: salvator-common: Add ADV7482 support")
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > >  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
>
> Geert, shall I apply this?

I'd say yes. Thanks!

Gr{oetje,eeting}s,

                        Geert
  
Kieran Bingham June 27, 2018, 4:45 p.m. UTC | #5
On 27/06/18 17:40, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Wed, Jun 27, 2018 at 5:10 PM Simon Horman <horms@verge.net.au> wrote:
>> On Tue, Jun 26, 2018 at 01:57:47PM -0600, Rob Herring wrote:
>>> On Thu, Jun 14, 2018 at 03:48:08PM +0200, Geert Uytterhoeven wrote:
>>>> With recent dtc and W=1:
>>>>
>>>>     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@10: graph node unit address error, expected "a"
>>>>     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@11: graph node unit address error, expected "b"
>>>>
>>>> Unit addresses are always hexadecimal (without prefix), while the bases
>>>> of reg property values depend on their prefixes.
>>>>
>>>> Fixes: 908001d778eba06e ("arm64: dts: renesas: salvator-common: Add ADV7482 support")
>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>> ---
>>>>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>
>> Geert, shall I apply this?
> 
> I'd say yes. Thanks!

I'm happy to throw an

Acked-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

on the patch - but I had a pending question regarding the reg = <10> part.

Shouldn't the reg become hex "reg = <0xa>" to be consistent?

Either way - if there's precedent - take that route and I'm happy.

--
Regards

Kieran


> 
> Gr{oetje,eeting}s,
> 
>                         Geert
>
  
Simon Horman June 28, 2018, 8:47 a.m. UTC | #6
On Wed, Jun 27, 2018 at 05:45:34PM +0100, Kieran Bingham wrote:
> On 27/06/18 17:40, Geert Uytterhoeven wrote:
> > Hi Simon,
> > 
> > On Wed, Jun 27, 2018 at 5:10 PM Simon Horman <horms@verge.net.au> wrote:
> >> On Tue, Jun 26, 2018 at 01:57:47PM -0600, Rob Herring wrote:
> >>> On Thu, Jun 14, 2018 at 03:48:08PM +0200, Geert Uytterhoeven wrote:
> >>>> With recent dtc and W=1:
> >>>>
> >>>>     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@10: graph node unit address error, expected "a"
> >>>>     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@11: graph node unit address error, expected "b"
> >>>>
> >>>> Unit addresses are always hexadecimal (without prefix), while the bases
> >>>> of reg property values depend on their prefixes.
> >>>>
> >>>> Fixes: 908001d778eba06e ("arm64: dts: renesas: salvator-common: Add ADV7482 support")
> >>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>>> ---
> >>>>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 4 ++--
> >>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> Reviewed-by: Rob Herring <robh@kernel.org>
> >>
> >> Geert, shall I apply this?
> > 
> > I'd say yes. Thanks!
> 
> I'm happy to throw an
> 
> Acked-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> on the patch - but I had a pending question regarding the reg = <10> part.
> 
> Shouldn't the reg become hex "reg = <0xa>" to be consistent?
> 
> Either way - if there's precedent - take that route and I'm happy.

Consistency seems good to me, Geert?
  
Geert Uytterhoeven June 28, 2018, 8:52 a.m. UTC | #7
Hi Simon,

On Thu, Jun 28, 2018 at 10:48 AM Simon Horman <horms@verge.net.au> wrote:
> On Wed, Jun 27, 2018 at 05:45:34PM +0100, Kieran Bingham wrote:
> > On 27/06/18 17:40, Geert Uytterhoeven wrote:
> > > On Wed, Jun 27, 2018 at 5:10 PM Simon Horman <horms@verge.net.au> wrote:
> > >> On Tue, Jun 26, 2018 at 01:57:47PM -0600, Rob Herring wrote:
> > >>> On Thu, Jun 14, 2018 at 03:48:08PM +0200, Geert Uytterhoeven wrote:
> > >>>> With recent dtc and W=1:
> > >>>>
> > >>>>     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@10: graph node unit address error, expected "a"
> > >>>>     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@11: graph node unit address error, expected "b"
> > >>>>
> > >>>> Unit addresses are always hexadecimal (without prefix), while the bases
> > >>>> of reg property values depend on their prefixes.
> > >>>>
> > >>>> Fixes: 908001d778eba06e ("arm64: dts: renesas: salvator-common: Add ADV7482 support")
> > >>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >>>> ---
> > >>>>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 4 ++--
> > >>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> > >>>
> > >>> Reviewed-by: Rob Herring <robh@kernel.org>
> > >>
> > >> Geert, shall I apply this?
> > >
> > > I'd say yes. Thanks!
> >
> > I'm happy to throw an
> >
> > Acked-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >
> > on the patch - but I had a pending question regarding the reg = <10> part.
> >
> > Shouldn't the reg become hex "reg = <0xa>" to be consistent?
> >
> > Either way - if there's precedent - take that route and I'm happy.
>
> Consistency seems good to me, Geert?

Typically we use decimal for "small" and hex for "large" numbers.
So far this was mostly relevant for the size parts of "reg"
properties, as the address
parts are usually large (if part of the main memory space).

These are different, as they are not memory-mapped addresses.
If you want to see 0xa and 0xb in the reg properties, I can respin.

Gr{oetje,eeting}s,

                        Geert
  
Simon Horman June 28, 2018, 12:25 p.m. UTC | #8
On Thu, Jun 28, 2018 at 10:52:17AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Thu, Jun 28, 2018 at 10:48 AM Simon Horman <horms@verge.net.au> wrote:
> > On Wed, Jun 27, 2018 at 05:45:34PM +0100, Kieran Bingham wrote:
> > > On 27/06/18 17:40, Geert Uytterhoeven wrote:
> > > > On Wed, Jun 27, 2018 at 5:10 PM Simon Horman <horms@verge.net.au> wrote:
> > > >> On Tue, Jun 26, 2018 at 01:57:47PM -0600, Rob Herring wrote:
> > > >>> On Thu, Jun 14, 2018 at 03:48:08PM +0200, Geert Uytterhoeven wrote:
> > > >>>> With recent dtc and W=1:
> > > >>>>
> > > >>>>     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@10: graph node unit address error, expected "a"
> > > >>>>     ...salvator-x.dtb: Warning (graph_port): /soc/i2c@e66d8000/video-receiver@70/port@11: graph node unit address error, expected "b"
> > > >>>>
> > > >>>> Unit addresses are always hexadecimal (without prefix), while the bases
> > > >>>> of reg property values depend on their prefixes.
> > > >>>>
> > > >>>> Fixes: 908001d778eba06e ("arm64: dts: renesas: salvator-common: Add ADV7482 support")
> > > >>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > >>>> ---
> > > >>>>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 4 ++--
> > > >>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >>>
> > > >>> Reviewed-by: Rob Herring <robh@kernel.org>
> > > >>
> > > >> Geert, shall I apply this?
> > > >
> > > > I'd say yes. Thanks!
> > >
> > > I'm happy to throw an
> > >
> > > Acked-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > >
> > > on the patch - but I had a pending question regarding the reg = <10> part.
> > >
> > > Shouldn't the reg become hex "reg = <0xa>" to be consistent?
> > >
> > > Either way - if there's precedent - take that route and I'm happy.
> >
> > Consistency seems good to me, Geert?
> 
> Typically we use decimal for "small" and hex for "large" numbers.
> So far this was mostly relevant for the size parts of "reg"
> properties, as the address
> parts are usually large (if part of the main memory space).
> 
> These are different, as they are not memory-mapped addresses.
> If you want to see 0xa and 0xb in the reg properties, I can respin.

I'll take this as is. We can decide how we want to address this,
in a consistent manner, without too many puns, later.
  

Patch

diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index 320250d708c3bbab..47088206cc052a15 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -437,7 +437,7 @@ 
 			};
 		};
 
-		port@10 {
+		port@a {
 			reg = <10>;
 
 			adv7482_txa: endpoint {
@@ -447,7 +447,7 @@ 
 			};
 		};
 
-		port@11 {
+		port@b {
 			reg = <11>;
 
 			adv7482_txb: endpoint {