[U-Boot] [PATCH v3.1 03/13] rockchip: dts: rk3399: Create initial rk3399-u-boot.dtsi

[U-Boot] [PATCH v3.1 03/13] rockchip: dts: rk3399: Create initial rk3399-u-boot.dtsi

[Jagan Teki]

u-boot,dm-pre-reloc is required for SDMMC booted rk3399 boards and
which is U-Boot specific devicetrees binding.

Move it on global rk3399-u-boot.dtsi file and rest of the U-Boot
bindings will move it future based on the requirement.

This would help to sync the devicetrees from Linux whenever required
instead of adding specific nodes.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
Changes for v3.1:
- exclude changes for other dts files, since they handle separately.

 arch/arm/dts/rk3399-u-boot.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)
 create mode 100644 arch/arm/dts/rk3399-u-boot.dtsi

diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi
new file mode 100644
index 0000000000..f533ed95eb
--- /dev/null
+++ b/arch/arm/dts/rk3399-u-boot.dtsi
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Jagan Teki <jagan@amarulasolutions.com>
+ */
+
+&sdmmc {
+	u-boot,dm-pre-reloc;
+};
-- 
2.18.0.321.gffc6fa0e3

[U-Boot] [PATCH v3.1 03/13] rockchip: dts: rk3399: Create initial rk3399-u-boot.dtsi

[Paul Kocialkowski]

Hi,

On Fri, 2019-04-26 at 18:48 +0530, Jagan Teki wrote:
> u-boot,dm-pre-reloc is required for SDMMC booted rk3399 boards and
> which is U-Boot specific devicetrees binding.
> 
> Move it on global rk3399-u-boot.dtsi file and rest of the U-Boot
> bindings will move it future based on the requirement.
> 
> This would help to sync the devicetrees from Linux whenever required
> instead of adding specific nodes.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Since your patch has changed significantly, you can no longer include a
Reviewed-by that was given on a previous iteration.

It looks like we are still missing bits to have rk3399-u-boot.dtsi
included, so this patch does not get my Reviewed-by tag.

Cheers,

Paul

> ---
> Changes for v3.1:
> - exclude changes for other dts files, since they handle separately.
> 
>  arch/arm/dts/rk3399-u-boot.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
>  create mode 100644 arch/arm/dts/rk3399-u-boot.dtsi
> 
> diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi
> new file mode 100644
> index 0000000000..f533ed95eb
> --- /dev/null
> +++ b/arch/arm/dts/rk3399-u-boot.dtsi
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Jagan Teki <jagan@amarulasolutions.com>
> + */
> +
> +&sdmmc {
> +	u-boot,dm-pre-reloc;
> +};
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[U-Boot] [PATCH v3.1 03/13] rockchip: dts: rk3399: Create initial rk3399-u-boot.dtsi

[Jagan Teki]

On Fri, Apr 26, 2019 at 7:04 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> On Fri, 2019-04-26 at 18:48 +0530, Jagan Teki wrote:
> > u-boot,dm-pre-reloc is required for SDMMC booted rk3399 boards and
> > which is U-Boot specific devicetrees binding.
> >
> > Move it on global rk3399-u-boot.dtsi file and rest of the U-Boot
> > bindings will move it future based on the requirement.
> >
> > This would help to sync the devicetrees from Linux whenever required
> > instead of adding specific nodes.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>
> Since your patch has changed significantly, you can no longer include a
> Reviewed-by that was given on a previous iteration.
>
> It looks like we are still missing bits to have rk3399-u-boot.dtsi
> included, so this patch does not get my Reviewed-by tag.

It will include rockchip-u-boot.dtsi automatically and I made the
receptive changes to include other files in this patch [1] and sure I
will ask Philipp to remove the reviewed-by tag, thanks.

[1] https://patchwork.ozlabs.org/patch/1091543/

[U-Boot] [PATCH v3.1 03/13] rockchip: dts: rk3399: Create initial rk3399-u-boot.dtsi

[Paul Kocialkowski]

Hi,

On Fri, 2019-04-26 at 19:07 +0530, Jagan Teki wrote:
> On Fri, Apr 26, 2019 at 7:04 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> > Hi,
> > 
> > On Fri, 2019-04-26 at 18:48 +0530, Jagan Teki wrote:
> > > u-boot,dm-pre-reloc is required for SDMMC booted rk3399 boards and
> > > which is U-Boot specific devicetrees binding.
> > > 
> > > Move it on global rk3399-u-boot.dtsi file and rest of the U-Boot
> > > bindings will move it future based on the requirement.
> > > 
> > > This would help to sync the devicetrees from Linux whenever required
> > > instead of adding specific nodes.
> > > 
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > 
> > Since your patch has changed significantly, you can no longer include a
> > Reviewed-by that was given on a previous iteration.
> > 
> > It looks like we are still missing bits to have rk3399-u-boot.dtsi
> > included, so this patch does not get my Reviewed-by tag.
> 
> It will include rockchip-u-boot.dtsi automatically and I made the
> receptive changes to include other files in this patch [1] and sure I
> will ask Philipp to remove the reviewed-by tag, thanks.

Most importantly (now that I see the new series), you should certainly
have sent that patch as part of your new series because we now have an
inter-dependency between both series.

Could you clean that up and make a proper standalone series that
creates and populates rk3399-u-boot.dtsi?

Cheers,

Paul

> [1] https://patchwork.ozlabs.org/patch/1091543/
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[U-Boot] [PATCH v3.1 03/13] rockchip: dts: rk3399: Create initial rk3399-u-boot.dtsi

[Jagan Teki]

On Fri, Apr 26, 2019 at 7:12 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> On Fri, 2019-04-26 at 19:07 +0530, Jagan Teki wrote:
> > On Fri, Apr 26, 2019 at 7:04 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > > Hi,
> > >
> > > On Fri, 2019-04-26 at 18:48 +0530, Jagan Teki wrote:
> > > > u-boot,dm-pre-reloc is required for SDMMC booted rk3399 boards and
> > > > which is U-Boot specific devicetrees binding.
> > > >
> > > > Move it on global rk3399-u-boot.dtsi file and rest of the U-Boot
> > > > bindings will move it future based on the requirement.
> > > >
> > > > This would help to sync the devicetrees from Linux whenever required
> > > > instead of adding specific nodes.
> > > >
> > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > >
> > > Since your patch has changed significantly, you can no longer include a
> > > Reviewed-by that was given on a previous iteration.
> > >
> > > It looks like we are still missing bits to have rk3399-u-boot.dtsi
> > > included, so this patch does not get my Reviewed-by tag.
> >
> > It will include rockchip-u-boot.dtsi automatically and I made the
> > receptive changes to include other files in this patch [1] and sure I
> > will ask Philipp to remove the reviewed-by tag, thanks.
>
> Most importantly (now that I see the new series), you should certainly
> have sent that patch as part of your new series because we now have an
> inter-dependency between both series.

It is not like an inter-dependency the previous rk3399-u-boot.dtsi is
not included by these files so they would work as before. and now this
series would need this change to include rk3399-u-boot.dtsi since the
goal here to add binman node to common for all rk3399 dts files.

[U-Boot] [PATCH v3.1 03/13] rockchip: dts: rk3399: Create initial rk3399-u-boot.dtsi

[Paul Kocialkowski]

Hi,

On Fri, 2019-04-26 at 19:20 +0530, Jagan Teki wrote:
> On Fri, Apr 26, 2019 at 7:12 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> > Hi,
> > 
> > On Fri, 2019-04-26 at 19:07 +0530, Jagan Teki wrote:
> > > On Fri, Apr 26, 2019 at 7:04 PM Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.com> wrote:
> > > > Hi,
> > > > 
> > > > On Fri, 2019-04-26 at 18:48 +0530, Jagan Teki wrote:
> > > > > u-boot,dm-pre-reloc is required for SDMMC booted rk3399 boards and
> > > > > which is U-Boot specific devicetrees binding.
> > > > > 
> > > > > Move it on global rk3399-u-boot.dtsi file and rest of the U-Boot
> > > > > bindings will move it future based on the requirement.
> > > > > 
> > > > > This would help to sync the devicetrees from Linux whenever required
> > > > > instead of adding specific nodes.
> > > > > 
> > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > 
> > > > Since your patch has changed significantly, you can no longer include a
> > > > Reviewed-by that was given on a previous iteration.
> > > > 
> > > > It looks like we are still missing bits to have rk3399-u-boot.dtsi
> > > > included, so this patch does not get my Reviewed-by tag.
> > > 
> > > It will include rockchip-u-boot.dtsi automatically and I made the
> > > receptive changes to include other files in this patch [1] and sure I
> > > will ask Philipp to remove the reviewed-by tag, thanks.
> > 
> > Most importantly (now that I see the new series), you should certainly
> > have sent that patch as part of your new series because we now have an
> > inter-dependency between both series.
> 
> It is not like an inter-dependency the previous rk3399-u-boot.dtsi is
> not included by these files so they would work as before. and now this
> series would need this change to include rk3399-u-boot.dtsi since the
> goal here to add binman node to common for all rk3399 dts files.

Well, what I mean is that the latest series you sent mentions a
dependency on the first one, when it really should be the other way
round: you need the rework to make things in your first series work at
all, since that dm-pre-reloc is necessary.

We don't want to merge one broken series and then the fix for it later,
even if it would still build in all cases. Here there is a clear
logical dependency, in the reverse ordre to the one you are describing.

In order to get this right, you need to take that v3.1 patch from the
first series and stick first in the new one. When that's done, I'll be
happy to move on to reviewing the code!

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[U-Boot] [PATCH v3.1 03/13] rockchip: dts: rk3399: Create initial rk3399-u-boot.dtsi

[Jagan Teki]

On Fri, Apr 26, 2019 at 7:28 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> On Fri, 2019-04-26 at 19:20 +0530, Jagan Teki wrote:
> > On Fri, Apr 26, 2019 at 7:12 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > > Hi,
> > >
> > > On Fri, 2019-04-26 at 19:07 +0530, Jagan Teki wrote:
> > > > On Fri, Apr 26, 2019 at 7:04 PM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > Hi,
> > > > >
> > > > > On Fri, 2019-04-26 at 18:48 +0530, Jagan Teki wrote:
> > > > > > u-boot,dm-pre-reloc is required for SDMMC booted rk3399 boards and
> > > > > > which is U-Boot specific devicetrees binding.
> > > > > >
> > > > > > Move it on global rk3399-u-boot.dtsi file and rest of the U-Boot
> > > > > > bindings will move it future based on the requirement.
> > > > > >
> > > > > > This would help to sync the devicetrees from Linux whenever required
> > > > > > instead of adding specific nodes.
> > > > > >
> > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > >
> > > > > Since your patch has changed significantly, you can no longer include a
> > > > > Reviewed-by that was given on a previous iteration.
> > > > >
> > > > > It looks like we are still missing bits to have rk3399-u-boot.dtsi
> > > > > included, so this patch does not get my Reviewed-by tag.
> > > >
> > > > It will include rockchip-u-boot.dtsi automatically and I made the
> > > > receptive changes to include other files in this patch [1] and sure I
> > > > will ask Philipp to remove the reviewed-by tag, thanks.
> > >
> > > Most importantly (now that I see the new series), you should certainly
> > > have sent that patch as part of your new series because we now have an
> > > inter-dependency between both series.
> >
> > It is not like an inter-dependency the previous rk3399-u-boot.dtsi is
> > not included by these files so they would work as before. and now this
> > series would need this change to include rk3399-u-boot.dtsi since the
> > goal here to add binman node to common for all rk3399 dts files.
>
> Well, what I mean is that the latest series you sent mentions a
> dependency on the first one, when it really should be the other way
> round: you need the rework to make things in your first series work at
> all, since that dm-pre-reloc is necessary.
>
> We don't want to merge one broken series and then the fix for it later,
> even if it would still build in all cases. Here there is a clear
> logical dependency, in the reverse ordre to the one you are describing.

I don't understand your point, sorry.

This patch [1] will make use of the new boards added on that series it
will not effect existing or previous rk3399 boards. in that case why
it is broken patch or series could you elaborate? the existing boards
are already supported dm-pre-reloc on their respective devicetree
files.

[1] https://patchwork.ozlabs.org/patch/1091534/

[U-Boot] [PATCH v3.1 03/13] rockchip: dts: rk3399: Create initial rk3399-u-boot.dtsi

[Paul Kocialkowski]

Hi,

On Fri, 2019-04-26 at 19:32 +0530, Jagan Teki wrote:
> On Fri, Apr 26, 2019 at 7:28 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> > Hi,
> > 
> > On Fri, 2019-04-26 at 19:20 +0530, Jagan Teki wrote:
> > > On Fri, Apr 26, 2019 at 7:12 PM Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.com> wrote:
> > > > Hi,
> > > > 
> > > > On Fri, 2019-04-26 at 19:07 +0530, Jagan Teki wrote:
> > > > > On Fri, Apr 26, 2019 at 7:04 PM Paul Kocialkowski
> > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > On Fri, 2019-04-26 at 18:48 +0530, Jagan Teki wrote:
> > > > > > > u-boot,dm-pre-reloc is required for SDMMC booted rk3399 boards and
> > > > > > > which is U-Boot specific devicetrees binding.
> > > > > > > 
> > > > > > > Move it on global rk3399-u-boot.dtsi file and rest of the U-Boot
> > > > > > > bindings will move it future based on the requirement.
> > > > > > > 
> > > > > > > This would help to sync the devicetrees from Linux whenever required
> > > > > > > instead of adding specific nodes.
> > > > > > > 
> > > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > > 
> > > > > > Since your patch has changed significantly, you can no longer include a
> > > > > > Reviewed-by that was given on a previous iteration.
> > > > > > 
> > > > > > It looks like we are still missing bits to have rk3399-u-boot.dtsi
> > > > > > included, so this patch does not get my Reviewed-by tag.
> > > > > 
> > > > > It will include rockchip-u-boot.dtsi automatically and I made the
> > > > > receptive changes to include other files in this patch [1] and sure I
> > > > > will ask Philipp to remove the reviewed-by tag, thanks.
> > > > 
> > > > Most importantly (now that I see the new series), you should certainly
> > > > have sent that patch as part of your new series because we now have an
> > > > inter-dependency between both series.
> > > 
> > > It is not like an inter-dependency the previous rk3399-u-boot.dtsi is
> > > not included by these files so they would work as before. and now this
> > > series would need this change to include rk3399-u-boot.dtsi since the
> > > goal here to add binman node to common for all rk3399 dts files.
> > 
> > Well, what I mean is that the latest series you sent mentions a
> > dependency on the first one, when it really should be the other way
> > round: you need the rework to make things in your first series work at
> > all, since that dm-pre-reloc is necessary.
> > 
> > We don't want to merge one broken series and then the fix for it later,
> > even if it would still build in all cases. Here there is a clear
> > logical dependency, in the reverse ordre to the one you are describing.
> 
> I don't understand your point, sorry.
> 
> This patch [1] will make use of the new boards added on that series it
> will not effect existing or previous rk3399 boards. in that case why
> it is broken patch or series could you elaborate? the existing boards
> are already supported dm-pre-reloc on their respective devicetree
> files.

On looks like I still didn't really get what you are trying to do.
Let's try and recap the situation:

- Currently, we have rk3399 boards with u-boot,dm-pre-reloc in their
shared dtsi;
- Your v3.1 adds rk3399-u-boot.dtsi but keeps the u-boot,dm-pre-reloc
in each device since rk3399-u-boot won't be included.
- Your new series allows rk3399-u-boot.dtsi to be included
automatically, so we can move the per-common-device-dts u-boot,dm-pre-
reloc to rk3399-u-boot.dtsi

If that's correct, then it indeed makes sense the way it is proposed.
Your commit messages just lack a clear explanation of what is going on
here, since it's not quite trivial for anyone but you.

Cheers,

Paul

> [1] https://patchwork.ozlabs.org/patch/1091534/
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[U-Boot] [PATCH v3.1 03/13] rockchip: dts: rk3399: Create initial rk3399-u-boot.dtsi

[Jagan Teki]

On Fri, Apr 26, 2019 at 7:46 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> On Fri, 2019-04-26 at 19:32 +0530, Jagan Teki wrote:
> > On Fri, Apr 26, 2019 at 7:28 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > > Hi,
> > >
> > > On Fri, 2019-04-26 at 19:20 +0530, Jagan Teki wrote:
> > > > On Fri, Apr 26, 2019 at 7:12 PM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > Hi,
> > > > >
> > > > > On Fri, 2019-04-26 at 19:07 +0530, Jagan Teki wrote:
> > > > > > On Fri, Apr 26, 2019 at 7:04 PM Paul Kocialkowski
> > > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Fri, 2019-04-26 at 18:48 +0530, Jagan Teki wrote:
> > > > > > > > u-boot,dm-pre-reloc is required for SDMMC booted rk3399 boards and
> > > > > > > > which is U-Boot specific devicetrees binding.
> > > > > > > >
> > > > > > > > Move it on global rk3399-u-boot.dtsi file and rest of the U-Boot
> > > > > > > > bindings will move it future based on the requirement.
> > > > > > > >
> > > > > > > > This would help to sync the devicetrees from Linux whenever required
> > > > > > > > instead of adding specific nodes.
> > > > > > > >
> > > > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > > >
> > > > > > > Since your patch has changed significantly, you can no longer include a
> > > > > > > Reviewed-by that was given on a previous iteration.
> > > > > > >
> > > > > > > It looks like we are still missing bits to have rk3399-u-boot.dtsi
> > > > > > > included, so this patch does not get my Reviewed-by tag.
> > > > > >
> > > > > > It will include rockchip-u-boot.dtsi automatically and I made the
> > > > > > receptive changes to include other files in this patch [1] and sure I
> > > > > > will ask Philipp to remove the reviewed-by tag, thanks.
> > > > >
> > > > > Most importantly (now that I see the new series), you should certainly
> > > > > have sent that patch as part of your new series because we now have an
> > > > > inter-dependency between both series.
> > > >
> > > > It is not like an inter-dependency the previous rk3399-u-boot.dtsi is
> > > > not included by these files so they would work as before. and now this
> > > > series would need this change to include rk3399-u-boot.dtsi since the
> > > > goal here to add binman node to common for all rk3399 dts files.
> > >
> > > Well, what I mean is that the latest series you sent mentions a
> > > dependency on the first one, when it really should be the other way
> > > round: you need the rework to make things in your first series work at
> > > all, since that dm-pre-reloc is necessary.
> > >
> > > We don't want to merge one broken series and then the fix for it later,
> > > even if it would still build in all cases. Here there is a clear
> > > logical dependency, in the reverse ordre to the one you are describing.
> >
> > I don't understand your point, sorry.
> >
> > This patch [1] will make use of the new boards added on that series it
> > will not effect existing or previous rk3399 boards. in that case why
> > it is broken patch or series could you elaborate? the existing boards
> > are already supported dm-pre-reloc on their respective devicetree
> > files.
>
> On looks like I still didn't really get what you are trying to do.
> Let's try and recap the situation:
>
> - Currently, we have rk3399 boards with u-boot,dm-pre-reloc in their
> shared dtsi;

No, the existing boards don't have shared dtsi that have
u-boot,dm-pre-reloc instead they included on respective dts files.

> - Your v3.1 adds rk3399-u-boot.dtsi but keeps the u-boot,dm-pre-reloc
> in each device since rk3399-u-boot won't be included.

This would keep sdmmc node only which is included by the new boards on
that series only.

> - Your new series allows rk3399-u-boot.dtsi to be included
> automatically, so we can move the per-common-device-dts u-boot,dm-pre-
> reloc to rk3399-u-boot.dtsi

No, automatically I'm explicit including on respective -u-boot.dtsi

>
> If that's correct, then it indeed makes sense the way it is proposed.
> Your commit messages just lack a clear explanation of what is going on
> here, since it's not quite trivial for anyone but you.

Please don't through stone, explain where it lacks. thanks.

Look like you seems confusing or unclear, here is bit more
explanation. The existing boards have u-boot,dm-pre-reloc for sdmmc on
their individual dts(i) files, my series add added initial
rk3399-u-boot.dtsi by including sdmmc node to use u-boot,dm-pre-reloc
which is what in v3.1 and the same included by board dts files which I
added on next patches.

Jagan.

[U-Boot] [PATCH v3.1 03/13] rockchip: dts: rk3399: Create initial rk3399-u-boot.dtsi

[Paul Kocialkowski]

Hi,

On Fri, 2019-04-26 at 19:55 +0530, Jagan Teki wrote:
> On Fri, Apr 26, 2019 at 7:46 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> > Hi,
> > 
> > On Fri, 2019-04-26 at 19:32 +0530, Jagan Teki wrote:
> > > On Fri, Apr 26, 2019 at 7:28 PM Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.com> wrote:
> > > > Hi,
> > > > 
> > > > On Fri, 2019-04-26 at 19:20 +0530, Jagan Teki wrote:
> > > > > On Fri, Apr 26, 2019 at 7:12 PM Paul Kocialkowski
> > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > On Fri, 2019-04-26 at 19:07 +0530, Jagan Teki wrote:
> > > > > > > On Fri, Apr 26, 2019 at 7:04 PM Paul Kocialkowski
> > > > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > On Fri, 2019-04-26 at 18:48 +0530, Jagan Teki wrote:
> > > > > > > > > u-boot,dm-pre-reloc is required for SDMMC booted rk3399 boards and
> > > > > > > > > which is U-Boot specific devicetrees binding.
> > > > > > > > > 
> > > > > > > > > Move it on global rk3399-u-boot.dtsi file and rest of the U-Boot
> > > > > > > > > bindings will move it future based on the requirement.
> > > > > > > > > 
> > > > > > > > > This would help to sync the devicetrees from Linux whenever required
> > > > > > > > > instead of adding specific nodes.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > > > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > > > > 
> > > > > > > > Since your patch has changed significantly, you can no longer include a
> > > > > > > > Reviewed-by that was given on a previous iteration.
> > > > > > > > 
> > > > > > > > It looks like we are still missing bits to have rk3399-u-boot.dtsi
> > > > > > > > included, so this patch does not get my Reviewed-by tag.
> > > > > > > 
> > > > > > > It will include rockchip-u-boot.dtsi automatically and I made the
> > > > > > > receptive changes to include other files in this patch [1] and sure I
> > > > > > > will ask Philipp to remove the reviewed-by tag, thanks.
> > > > > > 
> > > > > > Most importantly (now that I see the new series), you should certainly
> > > > > > have sent that patch as part of your new series because we now have an
> > > > > > inter-dependency between both series.
> > > > > 
> > > > > It is not like an inter-dependency the previous rk3399-u-boot.dtsi is
> > > > > not included by these files so they would work as before. and now this
> > > > > series would need this change to include rk3399-u-boot.dtsi since the
> > > > > goal here to add binman node to common for all rk3399 dts files.
> > > > 
> > > > Well, what I mean is that the latest series you sent mentions a
> > > > dependency on the first one, when it really should be the other way
> > > > round: you need the rework to make things in your first series work at
> > > > all, since that dm-pre-reloc is necessary.
> > > > 
> > > > We don't want to merge one broken series and then the fix for it later,
> > > > even if it would still build in all cases. Here there is a clear
> > > > logical dependency, in the reverse ordre to the one you are describing.
> > > 
> > > I don't understand your point, sorry.
> > > 
> > > This patch [1] will make use of the new boards added on that series it
> > > will not effect existing or previous rk3399 boards. in that case why
> > > it is broken patch or series could you elaborate? the existing boards
> > > are already supported dm-pre-reloc on their respective devicetree
> > > files.
> > 
> > On looks like I still didn't really get what you are trying to do.
> > Let's try and recap the situation:
> > 
> > - Currently, we have rk3399 boards with u-boot,dm-pre-reloc in their
> > shared dtsi;
> 
> No, the existing boards don't have shared dtsi that have
> u-boot,dm-pre-reloc instead they included on respective dts files.

I mean shared with Linux, so respective per-device dts files yes.
Sorry, that was definitely unclear wording on my side.

> > - Your v3.1 adds rk3399-u-boot.dtsi but keeps the u-boot,dm-pre-reloc
> > in each device since rk3399-u-boot won't be included.
> 
> This would keep sdmmc node only which is included by the new boards on
> that series only.
> 
> > - Your new series allows rk3399-u-boot.dtsi to be included
> > automatically, so we can move the per-common-device-dts u-boot,dm-pre-
> > reloc to rk3399-u-boot.dtsi
> 
> No, automatically I'm explicit including on respective -u-boot.dtsi

Yes you're right, though the result is the same.

> > If that's correct, then it indeed makes sense the way it is proposed.
> > Your commit messages just lack a clear explanation of what is going on
> > here, since it's not quite trivial for anyone but you.
> 
> Please don't through stone, explain where it lacks. thanks.

It's the same issue as before, you are not putting enough context in
the descriptions and they are only clear to someone who has already
understood the issue. You need to make sure that someone who did not
know about the issue has a clear idea of it after reading the commit
log.

I'm already trying to help by discussing the issue and presenting my
understanding so you can get an idea of what point where unclear, but
it's ultimately up to you to provide understandable commit messages for
reviewers.

> Look like you seems confusing or unclear, here is bit more
> explanation. The existing boards have u-boot,dm-pre-reloc for sdmmc on
> their individual dts(i) files, my series add added initial
> rk3399-u-boot.dtsi by including sdmmc node to use u-boot,dm-pre-reloc
> which is what in v3.1 and the same included by board dts files which I
> added on next patches.

Okay so I think I have a clear idea of what is going on now.

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com