[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Marcus Comstedt]

Commit ed0ef37 broke compatibility with HiFive Unleashed with SiFive
FSBL 2018-03-20.  This caused ethernet to stop working.  Restore
compatibility by adding back the needed compatible string, and
removing the special handling of rtcclk which was dead code anyway
since no __prci_clock actually names "rtcclk" as its parent.

Signed-off-by: Marcus Comstedt <marcus@mc.pp.se>
Cc: Anup Patel <Anup.Patel@wdc.com>
---
 drivers/clk/sifive/fu540-prci.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c
index ce0769f2d1..78a9fe3770 100644
--- a/drivers/clk/sifive/fu540-prci.c
+++ b/drivers/clk/sifive/fu540-prci.c
@@ -160,7 +160,6 @@
 struct __prci_data {
 	void *va;
 	struct clk parent_hfclk;
-	struct clk parent_rtcclk;
 };
 
 /**
@@ -538,10 +537,7 @@ static ulong sifive_fu540_prci_parent_rate(struct __prci_clock *pc)
 		return p->ops->recalc_rate(p, sifive_fu540_prci_parent_rate(p));
 	}
 
-	if (strcmp(pc->parent_name, "rtcclk") == 0)
-		parent_rate = clk_get_rate(&pc->pd->parent_rtcclk);
-	else
-		parent_rate = clk_get_rate(&pc->pd->parent_hfclk);
+	parent_rate = clk_get_rate(&pc->pd->parent_hfclk);
 
 	return parent_rate;
 }
@@ -593,10 +589,6 @@ static int sifive_fu540_prci_probe(struct udevice *dev)
 	if (err)
 		return err;
 
-	err = clk_get_by_index(dev, 1, &pd->parent_rtcclk);
-	if (err)
-		return err;
-
 	for (i = 0; i < ARRAY_SIZE(__prci_init_clocks); ++i) {
 		pc = &__prci_init_clocks[i];
 		pc->pd = pd;
@@ -614,6 +606,7 @@ static struct clk_ops sifive_fu540_prci_ops = {
 
 static const struct udevice_id sifive_fu540_prci_ids[] = {
 	{ .compatible = "sifive,fu540-c000-prci" },
+	{ .compatible = "sifive,aloeprci0" },
 	{ }
 };
 
-- 
2.21.0

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Bin Meng]

Hi Marcus,

On Sun, Sep 8, 2019 at 5:36 PM Marcus Comstedt <marcus@mc.pp.se> wrote:
>
> Commit ed0ef37 broke compatibility with HiFive Unleashed with SiFive
> FSBL 2018-03-20.  This caused ethernet to stop working.  Restore
> compatibility by adding back the needed compatible string, and
> removing the special handling of rtcclk which was dead code anyway
> since no __prci_clock actually names "rtcclk" as its parent.
>
> Signed-off-by: Marcus Comstedt <marcus@mc.pp.se>
> Cc: Anup Patel <Anup.Patel@wdc.com>
> ---
>  drivers/clk/sifive/fu540-prci.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
>

I believe the officially supported booting workflow is to use the
latest DT from Linux and compile it into OpenSBI, then boot to U-Boot.
The original DT shipped by FSBL is not used by OpenSBI or U-Boot.

Regards,
Bin

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Marcus Comstedt]

Hi Bin,

Bin Meng <bmeng.cn@gmail.com> writes:

> I believe the officially supported booting workflow is to use the
> latest DT from Linux and compile it into OpenSBI, then boot to U-Boot.
> The original DT shipped by FSBL is not used by OpenSBI or U-Boot.

Ok, if U-Boot uses a DT compiled into OpenSBI then the incompatibility
is with OpenSBI v0.4 (the latest release) because that's what I'm
running.  In any case it is a regression since the same setup worked
with U-Boot v2019.07.


  // Marcus

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Bin Meng]

Hi Marcus,

On Sun, Sep 8, 2019 at 8:54 PM Marcus Comstedt <marcus@mc.pp.se> wrote:
>
>
> Hi Bin,
>
> Bin Meng <bmeng.cn@gmail.com> writes:
>
> > I believe the officially supported booting workflow is to use the
> > latest DT from Linux and compile it into OpenSBI, then boot to U-Boot.
> > The original DT shipped by FSBL is not used by OpenSBI or U-Boot.
>
> Ok, if U-Boot uses a DT compiled into OpenSBI then the incompatibility
> is with OpenSBI v0.4 (the latest release) because that's what I'm
> running.  In any case it is a regression since the same setup worked
> with U-Boot v2019.07.
>

OpenSBI v0.4 works fine if you supply the the Linux DTB via
FW_PAYLOAD_FDT_PATH when building OpenSBI.

Regards,
Bin

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Marcus Comstedt]

Bin Meng <bmeng.cn@gmail.com> writes:

> OpenSBI v0.4 works fine if you supply the the Linux DTB via
> FW_PAYLOAD_FDT_PATH when building OpenSBI.

And which "Linux DTB" should that be?  Since RISC-V seems to be going
the same annoying way as ARM with a requirement that you switch DTB
every time you switch kernel version even though the hardware the DTB
describes stays the same, because the syntax of the nodes keep
changing, it's necessary to decide on a specific firmware DTB which is
decoupled from the Linux DTB (which can be loaded separately from
U-Boot as long as the ethernet/MMC drivers are working :-).

If U-Boot needs a specific DTB, shouldn't it be shipped in
u-boot/arch/riscv/dts/?


  // Marcus

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Bin Meng]

Hi Marcus,

On Sun, Sep 8, 2019 at 9:19 PM Marcus Comstedt <marcus@mc.pp.se> wrote:
>
>
> Bin Meng <bmeng.cn@gmail.com> writes:
>
> > OpenSBI v0.4 works fine if you supply the the Linux DTB via
> > FW_PAYLOAD_FDT_PATH when building OpenSBI.
>
> And which "Linux DTB" should that be?  Since RISC-V seems to be going

The one in arch/riscv/boot/dts/sifive.

> the same annoying way as ARM with a requirement that you switch DTB
> every time you switch kernel version even though the hardware the DTB
> describes stays the same, because the syntax of the nodes keep
> changing, it's necessary to decide on a specific firmware DTB which is
> decoupled from the Linux DTB (which can be loaded separately from
> U-Boot as long as the ethernet/MMC drivers are working :-).

Yes, it's annoying for ARM. Hence I believer on RISC-V the goal is to
use the firmware provided DTB for all the software on the boot chain:
OpenSBI, U-Boot, kernel. That's why U-Boot RISC-V is currently using
OF_PRIOR_STAGE.

As for the syntax of the nodes keep changing, I would say that's
inevitable because of the upstreaming process. Everything in vendor's
DT that does not get merged in Linux will remain potentially changed
by later upstreaming reviews.

>
> If U-Boot needs a specific DTB, shouldn't it be shipped in
> u-boot/arch/riscv/dts/?
>

This can be done by adding OF_SEPERATE support to RISC-V. But so far
OF_PRIOR_STAGE already works hence adding that would be of low
priority.

Regards,
Bin

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Marcus Comstedt]

Hi Bin,

Bin Meng <bmeng.cn@gmail.com> writes:

>> And which "Linux DTB" should that be?  Since RISC-V seems to be going
>
> The one in arch/riscv/boot/dts/sifive.

Yes, but in which Linux?  This whole thing started because U-Boot will
no longer accept the DTB used with Linux 4.14.  If each Linux version is
going to have its own incompatible DTB then U-Boot either needs to
decide which one to work with, or work with the vendor DTB (which
would be perferable in my opinion).


> As for the syntax of the nodes keep changing, I would say that's
> inevitable because of the upstreaming process. Everything in vendor's
> DT that does not get merged in Linux will remain potentially changed
> by later upstreaming reviews.

An upstreaming review that suggests changing a driver so that it no
longer works with the vendor's DTB is IMHO contraproductive and
against the very idea of DTB, but I suppose there's little that can be
done to affect the review process over at Linux land...  It would be
nice if this weirdness did not spill over on U-Boot/OpenSBI though.


  // Marcus

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Bin Meng]

Hi Marcus,

On Sun, Sep 8, 2019 at 10:48 PM Marcus Comstedt <marcus@mc.pp.se> wrote:
>
>
> Hi Bin,
>
> Bin Meng <bmeng.cn@gmail.com> writes:
>
> >> And which "Linux DTB" should that be?  Since RISC-V seems to be going
> >
> > The one in arch/riscv/boot/dts/sifive.
>
> Yes, but in which Linux?  This whole thing started because U-Boot will

The latest Linux kernel v5.3.

> no longer accept the DTB used with Linux 4.14.  If each Linux version is

I checked kernel 4.14 and found it did not ship the SiFive Unleased
board DTS in the kernel tree. Are you sure it's 4.14?

> going to have its own incompatible DTB then U-Boot either needs to
> decide which one to work with, or work with the vendor DTB (which
> would be perferable in my opinion).
>
>
> > As for the syntax of the nodes keep changing, I would say that's
> > inevitable because of the upstreaming process. Everything in vendor's
> > DT that does not get merged in Linux will remain potentially changed
> > by later upstreaming reviews.
>
> An upstreaming review that suggests changing a driver so that it no
> longer works with the vendor's DTB is IMHO contraproductive and
> against the very idea of DTB, but I suppose there's little that can be

I don't think Linux is to be blamed for this case. They have a policy
to maintain the the backward compatibility for the DT land. The issue
what you were facing here is an out of tree DTB (the vendors') is
incompatible with the kernel's. Once the kernel accepts a DTS,
bringing in any out-of-tree DT with changes reviewed to in-tree,
future compatibility will be assured.

> done to affect the review process over at Linux land...  It would be
> nice if this weirdness did not spill over on U-Boot/OpenSBI though.

Regards,
Bin

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Marcus Comstedt]

Hi Bin,

Bin Meng <bmeng.cn@gmail.com> writes:

>> Yes, but in which Linux?  This whole thing started because U-Boot will
>
> The latest Linux kernel v5.3.

Thanks.  I'll try it later.  As if to prove my point, the one from
5.2.11 (which is the Linux version I'm currently running) did not work
(I got MMC but no ethernet)...


> I checked kernel 4.14 and found it did not ship the SiFive Unleased
> board DTS in the kernel tree. Are you sure it's 4.14?

It's the one shipped with the H5U, which I'm pretty sure ran Linux
4.14 at the time (as part of the "Freedom U SDK") since 4.15 was not
out yet.  It looks like SiFive never added the DTS to their own Linux
tree (https://github.com/sifive/riscv-linux).


> I don't think Linux is to be blamed for this case. They have a policy
> to maintain the the backward compatibility for the DT land. The issue
> what you were facing here is an out of tree DTB (the vendors') is
> incompatible with the kernel's. Once the kernel accepts a DTS,
> bringing in any out-of-tree DT with changes reviewed to in-tree,
> future compatibility will be assured.

Isn't the reason that the DTS accepted by the kernel is different from
the out-of-tree vendor one that the review process caused changes in
the DTS then?  It seems unlikely that the vendor (SiFive) would
deliberately submit an incompatible DTS for inclusion...


  // Marcus

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Bin Meng]

Hi Marcus,

On Tue, Sep 10, 2019 at 4:52 AM Marcus Comstedt <marcus@mc.pp.se> wrote:
>
>
> Hi Bin,
>
> Bin Meng <bmeng.cn@gmail.com> writes:
>
> >> Yes, but in which Linux?  This whole thing started because U-Boot will
> >
> > The latest Linux kernel v5.3.
>
> Thanks.  I'll try it later.  As if to prove my point, the one from
> 5.2.11 (which is the Linux version I'm currently running) did not work
> (I got MMC but no ethernet)...

Maybe Anup could chime in for various SiFive kernel driver upstream
history. My memory was that before all these drivers were upstreamed,
we were using SiFive's out-of-tree kernel and drivers.

>
>
> > I checked kernel 4.14 and found it did not ship the SiFive Unleased
> > board DTS in the kernel tree. Are you sure it's 4.14?
>
> It's the one shipped with the H5U, which I'm pretty sure ran Linux
> 4.14 at the time (as part of the "Freedom U SDK") since 4.15 was not
> out yet.  It looks like SiFive never added the DTS to their own Linux
> tree (https://github.com/sifive/riscv-linux).
>

So 4.14 definitely was an out-of-tree kernel since I did not see the
SiFive DTS in the mainline kernel. For the DTS it was passed all the
way from FSBL to BBL to kernel.

>
> > I don't think Linux is to be blamed for this case. They have a policy
> > to maintain the the backward compatibility for the DT land. The issue
> > what you were facing here is an out of tree DTB (the vendors') is
> > incompatible with the kernel's. Once the kernel accepts a DTS,
> > bringing in any out-of-tree DT with changes reviewed to in-tree,
> > future compatibility will be assured.
>
> Isn't the reason that the DTS accepted by the kernel is different from
> the out-of-tree vendor one that the review process caused changes in
> the DTS then?  It seems unlikely that the vendor (SiFive) would
> deliberately submit an incompatible DTS for inclusion...

Well, that's the normal upstream process we have to live with. It's
nothing to do with whether that's a DT change, or a feature design
change. No one can guarantee an out-of-tree implementation will be
keeping compatible after it's accepted in-tree. Reviewers/maintainers
may have different view from the author on what's the best to be
included in the kernel mainline and this may require the author to
change the implementation and break something ...

Regards,
Bin

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Marcus Comstedt]

Hi Bin,

Bin Meng <bmeng.cn@gmail.com> writes:

> So 4.14 definitely was an out-of-tree kernel

Everything before 5.2 was out-of-tree.


> No one can guarantee an out-of-tree implementation will be
> keeping compatible after it's accepted in-tree.

Reviewers/maintainers can guarantee compatibility with existing
hardware and DT by not instisting on breaking changes.

It's not really about keeping in-tree and out-of-tree compatible with
each other, but about keeping both of them compatible with the actual
hardware and DT of the system the OS is supposed to run on.


> Reviewers/maintainers
> may have different view from the author on what's the best

A reviewer/maintainer could for example have the view that a certain
register in a piece of hardware should really be two registers with
the bits divided between them based on some logical partitioning.  And
they might be right.  But the hardware is what it is, and if they
insist that the driver access two different registers the driver will
not work the hardware.  You'll have a nice driver that works on
nothing (at least until the vendor makes a new spin of the hardware
with the two registers).

My opinion is that the DT should be treated the same way.  It is part
of the hardware (sort of the "metadata" for the hardware).  Sure you
can have some idea of how things could be expressed better and add
support for that, but you need to also keep compat with the actual
hardware platform that the driver is there to interface against,
otherwise the driver won't work.


  // Marcus

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Bin Meng]

Hi Marcus,

On Tue, Sep 10, 2019 at 11:53 PM Marcus Comstedt <marcus@mc.pp.se> wrote:
>
>
> Hi Bin,
>
> Bin Meng <bmeng.cn@gmail.com> writes:
>
> > So 4.14 definitely was an out-of-tree kernel
>
> Everything before 5.2 was out-of-tree.
>
>
> > No one can guarantee an out-of-tree implementation will be
> > keeping compatible after it's accepted in-tree.
>
> Reviewers/maintainers can guarantee compatibility with existing
> hardware and DT by not instisting on breaking changes.
>
> It's not really about keeping in-tree and out-of-tree compatible with
> each other, but about keeping both of them compatible with the actual
> hardware and DT of the system the OS is supposed to run on.
>

Per my understanding kernel reviewers/maintainers want to have a clean
implementation from the start. That's why it's being done this way.

>
> > Reviewers/maintainers
> > may have different view from the author on what's the best
>
> A reviewer/maintainer could for example have the view that a certain
> register in a piece of hardware should really be two registers with
> the bits divided between them based on some logical partitioning.  And
> they might be right.  But the hardware is what it is, and if they
> insist that the driver access two different registers the driver will
> not work the hardware.  You'll have a nice driver that works on
> nothing (at least until the vendor makes a new spin of the hardware
> with the two registers).

The original hardware vendor's DT may be written in a way that
violates the DT specs. Hence I believe it's really hard to keep the
comparability for both.

For this specific FU540 ethernet, what you did in this patch does not
completely bring back the compatibility. Things were changed a lot in
the upstream kernel.

For example, the U-Boot GEM GXL-MGMT driver was needed to work with SiFive's DT
https://lists.denx.de/pipermail/u-boot/2019-May/369577.html

But later, it was dropped due to kernel mainline DT
reviewers/maintainers thought it was a misuse of clock node and the
GXL-MGMT module should be part of the GEM spec.
https://lists.denx.de/pipermail/u-boot/2019-June/373306.html

>
> My opinion is that the DT should be treated the same way.  It is part
> of the hardware (sort of the "metadata" for the hardware).  Sure you
> can have some idea of how things could be expressed better and add
> support for that, but you need to also keep compat with the actual
> hardware platform that the driver is there to interface against,
> otherwise the driver won't work.
>

While I agree with you to some extent that, that's now how things are
done with current kernel practice.

Regards,
Bin

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Anup Patel]

On Wed, Sep 11, 2019 at 7:28 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Marcus,
>
> On Tue, Sep 10, 2019 at 11:53 PM Marcus Comstedt <marcus@mc.pp.se> wrote:
> >
> >
> > Hi Bin,
> >
> > Bin Meng <bmeng.cn@gmail.com> writes:
> >
> > > So 4.14 definitely was an out-of-tree kernel
> >
> > Everything before 5.2 was out-of-tree.
> >
> >
> > > No one can guarantee an out-of-tree implementation will be
> > > keeping compatible after it's accepted in-tree.
> >
> > Reviewers/maintainers can guarantee compatibility with existing
> > hardware and DT by not instisting on breaking changes.
> >
> > It's not really about keeping in-tree and out-of-tree compatible with
> > each other, but about keeping both of them compatible with the actual
> > hardware and DT of the system the OS is supposed to run on.
> >
>
> Per my understanding kernel reviewers/maintainers want to have a clean
> implementation from the start. That's why it's being done this way.
>
> >
> > > Reviewers/maintainers
> > > may have different view from the author on what's the best
> >
> > A reviewer/maintainer could for example have the view that a certain
> > register in a piece of hardware should really be two registers with
> > the bits divided between them based on some logical partitioning.  And
> > they might be right.  But the hardware is what it is, and if they
> > insist that the driver access two different registers the driver will
> > not work the hardware.  You'll have a nice driver that works on
> > nothing (at least until the vendor makes a new spin of the hardware
> > with the two registers).
>
> The original hardware vendor's DT may be written in a way that
> violates the DT specs. Hence I believe it's really hard to keep the
> comparability for both.
>
> For this specific FU540 ethernet, what you did in this patch does not
> completely bring back the compatibility. Things were changed a lot in
> the upstream kernel.
>
> For example, the U-Boot GEM GXL-MGMT driver was needed to work with SiFive's DT
> https://lists.denx.de/pipermail/u-boot/2019-May/369577.html
>
> But later, it was dropped due to kernel mainline DT
> reviewers/maintainers thought it was a misuse of clock node and the
> GXL-MGMT module should be part of the GEM spec.
> https://lists.denx.de/pipermail/u-boot/2019-June/373306.html
>
> >
> > My opinion is that the DT should be treated the same way.  It is part
> > of the hardware (sort of the "metadata" for the hardware).  Sure you
> > can have some idea of how things could be expressed better and add
> > support for that, but you need to also keep compat with the actual
> > hardware platform that the driver is there to interface against,
> > otherwise the driver won't work.
> >
>
> While I agree with you to some extent that, that's now how things are
> done with current kernel practice.

Sorry for the slow response...

The DT bindings for most of the Linux drivers accepted upstream have
changed in a backward in-compatible way for SiFive Unleashed. The
Linux clock driver implementation also changed quite a bit.

Most of the recent U-Boot changes for SiFive Unleashed were to sync-up
DT bindings followed by U-Boot drivers with SiFive Unleashed drivers
accepted in Linux. The built-in DTB passed by FSBL to OpenSBI + U-Boot
is totally out-of-date. Going forward SiFive folks will use upstreamed Linux
DT bindings for newer platforms so it made sense to update U-Boot drivers.

The root of the problem is delay in freezing DT bindings by SOC vendors.

I think the SOC vendors should get DT bindings reviewed very early before
sending patches to Linux or U-Boot. The Linux kernel is still a central place
for all DT bindings documentation across architectures.

May be things would be different if DT bindings documentation was
maintained as separate project ???

For now, we can insist SOC vendors to get DT bindings reviewed/froozen
in LKML before sending U-Boot driver patches. Suggestions ??

Regards,
Anup

>
> Regards,
> Bin
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

Re: [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Bin Meng]

+devicetree

On Wed, Sep 11, 2019 at 2:25 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Wed, Sep 11, 2019 at 7:28 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Marcus,
> >
> > On Tue, Sep 10, 2019 at 11:53 PM Marcus Comstedt <marcus@mc.pp.se> wrote:
> > >
> > >
> > > Hi Bin,
> > >
> > > Bin Meng <bmeng.cn@gmail.com> writes:
> > >
> > > > So 4.14 definitely was an out-of-tree kernel
> > >
> > > Everything before 5.2 was out-of-tree.
> > >
> > >
> > > > No one can guarantee an out-of-tree implementation will be
> > > > keeping compatible after it's accepted in-tree.
> > >
> > > Reviewers/maintainers can guarantee compatibility with existing
> > > hardware and DT by not instisting on breaking changes.
> > >
> > > It's not really about keeping in-tree and out-of-tree compatible with
> > > each other, but about keeping both of them compatible with the actual
> > > hardware and DT of the system the OS is supposed to run on.
> > >
> >
> > Per my understanding kernel reviewers/maintainers want to have a clean
> > implementation from the start. That's why it's being done this way.
> >
> > >
> > > > Reviewers/maintainers
> > > > may have different view from the author on what's the best
> > >
> > > A reviewer/maintainer could for example have the view that a certain
> > > register in a piece of hardware should really be two registers with
> > > the bits divided between them based on some logical partitioning.  And
> > > they might be right.  But the hardware is what it is, and if they
> > > insist that the driver access two different registers the driver will
> > > not work the hardware.  You'll have a nice driver that works on
> > > nothing (at least until the vendor makes a new spin of the hardware
> > > with the two registers).
> >
> > The original hardware vendor's DT may be written in a way that
> > violates the DT specs. Hence I believe it's really hard to keep the
> > comparability for both.
> >
> > For this specific FU540 ethernet, what you did in this patch does not
> > completely bring back the compatibility. Things were changed a lot in
> > the upstream kernel.
> >
> > For example, the U-Boot GEM GXL-MGMT driver was needed to work with SiFive's DT
> > https://lists.denx.de/pipermail/u-boot/2019-May/369577.html
> >
> > But later, it was dropped due to kernel mainline DT
> > reviewers/maintainers thought it was a misuse of clock node and the
> > GXL-MGMT module should be part of the GEM spec.
> > https://lists.denx.de/pipermail/u-boot/2019-June/373306.html
> >
> > >
> > > My opinion is that the DT should be treated the same way.  It is part
> > > of the hardware (sort of the "metadata" for the hardware).  Sure you
> > > can have some idea of how things could be expressed better and add
> > > support for that, but you need to also keep compat with the actual
> > > hardware platform that the driver is there to interface against,
> > > otherwise the driver won't work.
> > >
> >
> > While I agree with you to some extent that, that's now how things are
> > done with current kernel practice.
>
> Sorry for the slow response...
>
> The DT bindings for most of the Linux drivers accepted upstream have
> changed in a backward in-compatible way for SiFive Unleashed. The
> Linux clock driver implementation also changed quite a bit.
>
> Most of the recent U-Boot changes for SiFive Unleashed were to sync-up
> DT bindings followed by U-Boot drivers with SiFive Unleashed drivers
> accepted in Linux. The built-in DTB passed by FSBL to OpenSBI + U-Boot
> is totally out-of-date. Going forward SiFive folks will use upstreamed Linux
> DT bindings for newer platforms so it made sense to update U-Boot drivers.
>
> The root of the problem is delay in freezing DT bindings by SOC vendors.
>
> I think the SOC vendors should get DT bindings reviewed very early before

The question is when we can call that "very early"? Normally SoC
vendors will ship their own software stack (bootloaders, kernels, dtb)
on their evaluation board and provided forked git repos of these
software for their customers, aka SDK. If we expect bringing DT
bindings in a very early phase to fix such compatibility problem, that
means SoC vendors cannot ship their SDK to their customers before
every DT bindings used in their SoC get reviewed and approved. That
sounds unrealistic.

> sending patches to Linux or U-Boot. The Linux kernel is still a central place
> for all DT bindings documentation across architectures.
>
> May be things would be different if DT bindings documentation was
> maintained as separate project ???

I vaguely remember a similar proposal was once discussed in the LKML.
If we manage to go with that direction, I think we should also request
all DTS files to be dual licensed with one BSD-style permissive
license model for other projects to use.

> For now, we can insist SOC vendors to get DT bindings reviewed/froozen
> in LKML before sending U-Boot driver patches. Suggestions ??
>

So U-Boot upstream drivers always work with upstream DT in Linux
kernel. Sounds good to me. But again this does not help with the
out-of-tree U-Boot fork provided by the SoC vendor to their customers
...

Regards,
Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] clk: sifive: Fix ethernet regression on HiFive Unleashed

[Bin Meng]

+devicetree

On Wed, Sep 11, 2019 at 2:25 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Wed, Sep 11, 2019 at 7:28 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Marcus,
> >
> > On Tue, Sep 10, 2019 at 11:53 PM Marcus Comstedt <marcus@mc.pp.se> wrote:
> > >
> > >
> > > Hi Bin,
> > >
> > > Bin Meng <bmeng.cn@gmail.com> writes:
> > >
> > > > So 4.14 definitely was an out-of-tree kernel
> > >
> > > Everything before 5.2 was out-of-tree.
> > >
> > >
> > > > No one can guarantee an out-of-tree implementation will be
> > > > keeping compatible after it's accepted in-tree.
> > >
> > > Reviewers/maintainers can guarantee compatibility with existing
> > > hardware and DT by not instisting on breaking changes.
> > >
> > > It's not really about keeping in-tree and out-of-tree compatible with
> > > each other, but about keeping both of them compatible with the actual
> > > hardware and DT of the system the OS is supposed to run on.
> > >
> >
> > Per my understanding kernel reviewers/maintainers want to have a clean
> > implementation from the start. That's why it's being done this way.
> >
> > >
> > > > Reviewers/maintainers
> > > > may have different view from the author on what's the best
> > >
> > > A reviewer/maintainer could for example have the view that a certain
> > > register in a piece of hardware should really be two registers with
> > > the bits divided between them based on some logical partitioning.  And
> > > they might be right.  But the hardware is what it is, and if they
> > > insist that the driver access two different registers the driver will
> > > not work the hardware.  You'll have a nice driver that works on
> > > nothing (at least until the vendor makes a new spin of the hardware
> > > with the two registers).
> >
> > The original hardware vendor's DT may be written in a way that
> > violates the DT specs. Hence I believe it's really hard to keep the
> > comparability for both.
> >
> > For this specific FU540 ethernet, what you did in this patch does not
> > completely bring back the compatibility. Things were changed a lot in
> > the upstream kernel.
> >
> > For example, the U-Boot GEM GXL-MGMT driver was needed to work with SiFive's DT
> > https://lists.denx.de/pipermail/u-boot/2019-May/369577.html
> >
> > But later, it was dropped due to kernel mainline DT
> > reviewers/maintainers thought it was a misuse of clock node and the
> > GXL-MGMT module should be part of the GEM spec.
> > https://lists.denx.de/pipermail/u-boot/2019-June/373306.html
> >
> > >
> > > My opinion is that the DT should be treated the same way.  It is part
> > > of the hardware (sort of the "metadata" for the hardware).  Sure you
> > > can have some idea of how things could be expressed better and add
> > > support for that, but you need to also keep compat with the actual
> > > hardware platform that the driver is there to interface against,
> > > otherwise the driver won't work.
> > >
> >
> > While I agree with you to some extent that, that's now how things are
> > done with current kernel practice.
>
> Sorry for the slow response...
>
> The DT bindings for most of the Linux drivers accepted upstream have
> changed in a backward in-compatible way for SiFive Unleashed. The
> Linux clock driver implementation also changed quite a bit.
>
> Most of the recent U-Boot changes for SiFive Unleashed were to sync-up
> DT bindings followed by U-Boot drivers with SiFive Unleashed drivers
> accepted in Linux. The built-in DTB passed by FSBL to OpenSBI + U-Boot
> is totally out-of-date. Going forward SiFive folks will use upstreamed Linux
> DT bindings for newer platforms so it made sense to update U-Boot drivers.
>
> The root of the problem is delay in freezing DT bindings by SOC vendors.
>
> I think the SOC vendors should get DT bindings reviewed very early before

The question is when we can call that "very early"? Normally SoC
vendors will ship their own software stack (bootloaders, kernels, dtb)
on their evaluation board and provided forked git repos of these
software for their customers, aka SDK. If we expect bringing DT
bindings in a very early phase to fix such compatibility problem, that
means SoC vendors cannot ship their SDK to their customers before
every DT bindings used in their SoC get reviewed and approved. That
sounds unrealistic.

> sending patches to Linux or U-Boot. The Linux kernel is still a central place
> for all DT bindings documentation across architectures.
>
> May be things would be different if DT bindings documentation was
> maintained as separate project ???

I vaguely remember a similar proposal was once discussed in the LKML.
If we manage to go with that direction, I think we should also request
all DTS files to be dual licensed with one BSD-style permissive
license model for other projects to use.

> For now, we can insist SOC vendors to get DT bindings reviewed/froozen
> in LKML before sending U-Boot driver patches. Suggestions ??
>

So U-Boot upstream drivers always work with upstream DT in Linux
kernel. Sounds good to me. But again this does not help with the
out-of-tree U-Boot fork provided by the SoC vendor to their customers
...

Regards,
Bin