| Message ID | 20180804231114.21420-5-erosca@de.adit-jv.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Simon Horman |
| Headers | show |
| Series | Add minimal DTS support for M3-N Starter Kit | expand |
Hi Eugeniu, Thank you for the patch. On Sunday, 5 August 2018 02:11:04 EEST Eugeniu Rosca wrote: > Following the recent change in dt-bindings [1], switch from > "renesas,h3ulcb" to "renesas,ulcb" compatible string. > > [1] Documentation/devicetree/bindings/arm/shmobile.txt > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > --- > arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts > b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts index > 06deb67c36c8..7a5b1dc64090 100644 > --- a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts > +++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts > @@ -14,6 +14,6 @@ > > / { > model = "Renesas H3ULCB Kingfisher board based on r8a7795 ES1.x"; > - compatible = "shimafuji,kingfisher", "renesas,h3ulcb", > + compatible = "shimafuji,kingfisher", "renesas,ulcb", > "renesas,r8a7795"; This is unrelated to your patch, but due to the reason explained in my review of 02/14, I think "shimafuji,kingfisher" should include the SoC name. This brings up the topic of how to describe boards that are made of an SoC "module" board plugged into an expansion "motherboard". > };
Hi Laurent, Eugeniu, On 06/08/18 11:42, Laurent Pinchart wrote: > Hi Eugeniu, > > Thank you for the patch. > > On Sunday, 5 August 2018 02:11:04 EEST Eugeniu Rosca wrote: >> Following the recent change in dt-bindings [1], switch from >> "renesas,h3ulcb" to "renesas,ulcb" compatible string. >> >> [1] Documentation/devicetree/bindings/arm/shmobile.txt >> >> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> >> --- >> arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts >> b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts index >> 06deb67c36c8..7a5b1dc64090 100644 >> --- a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts >> +++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts >> @@ -14,6 +14,6 @@ >> >> / { >> model = "Renesas H3ULCB Kingfisher board based on r8a7795 ES1.x"; >> - compatible = "shimafuji,kingfisher", "renesas,h3ulcb", >> + compatible = "shimafuji,kingfisher", "renesas,ulcb", >> "renesas,r8a7795"; > > This is unrelated to your patch, but due to the reason explained in my review > of 02/14, I think "shimafuji,kingfisher" should include the SoC name. > > This brings up the topic of how to describe boards that are made of an SoC > "module" board plugged into an expansion "motherboard". Isn't it the point that the shimafuji board is agnostic to the SoC on the ULCB? I presume the Kingfisher board is just the expansion board which would be identical regardless of if it was put on an H3 ULCB, or an M3 ULCB? > >> }; > Regards Kieran
On 06/08/18 11:45, Kieran Bingham wrote: > Hi Laurent, Eugeniu, > > On 06/08/18 11:42, Laurent Pinchart wrote: >> Hi Eugeniu, >> >> Thank you for the patch. >> >> On Sunday, 5 August 2018 02:11:04 EEST Eugeniu Rosca wrote: >>> Following the recent change in dt-bindings [1], switch from >>> "renesas,h3ulcb" to "renesas,ulcb" compatible string. >>> >>> [1] Documentation/devicetree/bindings/arm/shmobile.txt >>> >>> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> >>> --- >>> arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts >>> b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts index >>> 06deb67c36c8..7a5b1dc64090 100644 >>> --- a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts >>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts >>> @@ -14,6 +14,6 @@ >>> >>> / { >>> model = "Renesas H3ULCB Kingfisher board based on r8a7795 ES1.x"; >>> - compatible = "shimafuji,kingfisher", "renesas,h3ulcb", >>> + compatible = "shimafuji,kingfisher", "renesas,ulcb", >>> "renesas,r8a7795"; >> >> This is unrelated to your patch, but due to the reason explained in my review >> of 02/14, I think "shimafuji,kingfisher" should include the SoC name. >> >> This brings up the topic of how to describe boards that are made of an SoC >> "module" board plugged into an expansion "motherboard". > > Isn't it the point that the shimafuji board is agnostic to the SoC on > the ULCB? > > I presume the Kingfisher board is just the expansion board which would > be identical regardless of if it was put on an H3 ULCB, or an M3 ULCB? In fact possibly the interesting point is that the kingfisher as an expansion board is surely an 'overlay', rather than the board itself ? >> >>> }; Regards Kieran
Hi Kieran, On Monday, 6 August 2018 13:49:59 EEST Kieran Bingham wrote: > On 06/08/18 11:45, Kieran Bingham wrote: > > On 06/08/18 11:42, Laurent Pinchart wrote: > >> On Sunday, 5 August 2018 02:11:04 EEST Eugeniu Rosca wrote: > >>> Following the recent change in dt-bindings [1], switch from > >>> "renesas,h3ulcb" to "renesas,ulcb" compatible string. > >>> > >>> [1] Documentation/devicetree/bindings/arm/shmobile.txt > >>> > >>> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > >>> --- > >>> > >>> arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts > >>> b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts index > >>> 06deb67c36c8..7a5b1dc64090 100644 > >>> --- a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts > >>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts > >>> @@ -14,6 +14,6 @@ > >>> > >>> / { > >>> model = "Renesas H3ULCB Kingfisher board based on r8a7795 ES1.x"; > >>> > >>> - compatible = "shimafuji,kingfisher", "renesas,h3ulcb", > >>> + compatible = "shimafuji,kingfisher", "renesas,ulcb", > >>> "renesas,r8a7795"; > >> > >> This is unrelated to your patch, but due to the reason explained in my > >> review of 02/14, I think "shimafuji,kingfisher" should include the SoC > >> name. > >> > >> This brings up the topic of how to describe boards that are made of an > >> SoC "module" board plugged into an expansion "motherboard". > > > > Isn't it the point that the shimafuji board is agnostic to the SoC on > > the ULCB? > > > > I presume the Kingfisher board is just the expansion board which would > > be identical regardless of if it was put on an H3 ULCB, or an M3 ULCB? > > In fact possibly the interesting point is that the kingfisher as an > expansion board is surely an 'overlay', rather than the board itself ? In a way it's both. From an SoC point of view, the KingFisher board can be seen as an expansion board. From a system point of view, it's a KingFisher system. If you built a car around an H3 ULCB, would you consider that to be an H3 with a car extension, or a car with an H3 inside ? :-) > >>> };
Hi Laurent, [Adding Rob and DT ML] On 08/06/2018 01:42 PM, Laurent Pinchart wrote: > Hi Eugeniu, > > Thank you for the patch. > > On Sunday, 5 August 2018 02:11:04 EEST Eugeniu Rosca wrote: >> Following the recent change in dt-bindings [1], switch from >> "renesas,h3ulcb" to "renesas,ulcb" compatible string. >> >> [1] Documentation/devicetree/bindings/arm/shmobile.txt >> >> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> >> --- >> arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts >> b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts index >> 06deb67c36c8..7a5b1dc64090 100644 >> --- a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts >> +++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts >> @@ -14,6 +14,6 @@ >> >> / { >> model = "Renesas H3ULCB Kingfisher board based on r8a7795 ES1.x"; >> - compatible = "shimafuji,kingfisher", "renesas,h3ulcb", >> + compatible = "shimafuji,kingfisher", "renesas,ulcb", >> "renesas,r8a7795"; > > This is unrelated to your patch, but due to the reason explained in my review > of 02/14, I think "shimafuji,kingfisher" should include the SoC name. > > This brings up the topic of how to describe boards that are made of an SoC > "module" board plugged into an expansion "motherboard". > Diving into (probably shatteredly recollected by me) history, a board 'compatible' property appears as a handle to deal with incomplete board description represented in DTB, so that arch code or drivers can catch on it and fill some board specific missing parts in runtime, commonly the related code takes a shape of quirks. If we accept it, SoC specific drivers would take their interest in SoC model and revision, and out-of-SoC/platform/board drivers and legacy arch board code can look at a PCB board model variant. Note that for both separated but comparably large and important pieces of board/platform knowledge there is a single compatible property in use, namely the compatible property of the root node. Also note that both ePAPR and Devicetree Specification describe 'compatible' property of the root node as a special one, as "a list of platform architectures with which this platform is compatible". In my opinion a better board representation would be to add a 'soc' device node as a child of the root node, and the former one has its own fixed compatible property, but a protocol based on such view has to be agreed and accepted firstly, and it falls into "forever unrealistic tasks" category. Today the SoC part of compatible property value is still in active use, and PCB\SoC part is almost abandoned, so I would propose to diminish the asset of the latter. Since both board/platform descriptions are alloyed in one list of values, I'd like to see a better description of acceptable values of 'compatible' property of the root node. The common practice is to put SoC specific values into the right tail, and place (kind of optional) board specific values on the left, let's continue to follow it, but unrestrict SoC agnostic string names of boards and further board extensions, if PCB\SoC (or PCB\PCB for extension boards) are about identical. Anyway those values are mainly unused nowadays, so nothing is lost but another dimension in board/platform description and naming is avoided. I do understand that likely most of PCBs are very SoC centric, and the proposal shared above should not be considered as a rule, but rather as a reasonable and valid exception. -- Best wishes, Vladimir
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts index 06deb67c36c8..7a5b1dc64090 100644 --- a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts +++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts @@ -14,6 +14,6 @@ / { model = "Renesas H3ULCB Kingfisher board based on r8a7795 ES1.x"; - compatible = "shimafuji,kingfisher", "renesas,h3ulcb", + compatible = "shimafuji,kingfisher", "renesas,ulcb", "renesas,r8a7795"; };
Following the recent change in dt-bindings [1], switch from "renesas,h3ulcb" to "renesas,ulcb" compatible string. [1] Documentation/devicetree/bindings/arm/shmobile.txt Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> --- arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)