mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* Device-tree node renames break barebox/kernel compatibility...
@ 2018-06-12 12:23 David Jander
  2018-06-12 12:47 ` Sascha Hauer
  0 siblings, 1 reply; 6+ messages in thread
From: David Jander @ 2018-06-12 12:23 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Fabio Estevam, Barebox List, Marco Franchi, Shawn Guo


Hi Sascha,

I know this is old already, and I was surprised that I couldn't find any
complaints about this yet, but I recently came across this patch in the kernel:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/patch/?id=df5cc9d0b42d15fa33b30440cca7a11ca7ba35a4

...which was adopted in barebox as this:

https://git.pengutronix.de/cgit/barebox/patch/dts/src/arm/imx6qdl.dtsi?id=2e9cce8fb1f577088e2b20ae2f461130e13ad190

As I don't know the exact reason as to why this was necessary, or why this is
an issue at all, I just wanted to point out the fact that this leads to some
breakage in barebox new and old. The sensible code comes from u-boot
originally so maybe it is there where blame is to be found, but I can't find
this code on latest u-boot anymore...

The specific problem I observed is here:
https://git.pengutronix.de/cgit/barebox/tree/net/eth.c#n299

But I guess this can cause other similar problems elsewhere... If you agree
that all these potential places are broken, I'd like to know what the correct
fix should be and I'd be happy to submit patches.

The thing is: barebox has its own device-tree but nevertheless should patch
the devicetree of any given Linux kernel to boot. In the aforementioned piece
of code, barebox will identify an ethernet node by its full DT-path from it's
own device tree (!) and associate a MAC address with that device. Later, when
a kernel (+ its DT) is loaded, that DT is fixed-up with the MAC address saved
earlier by searching for the ethernet node by full DT-path name.
Due to the above patches, these path names (which in theory should be
immutable, since the ethernet device is still connected to the same instance
of the same bus at the same physical address) just happen to be different
between different versions of the kernel and the bootloader. This is awful,
because now old bootloaders cannot load newer kernels and newer bootloaders
cannot load older kernels... again!

I still hope that someone tells my that my board code is broken and that I
should not do things like this to start. If not, how should this issue be
solved?

Best regards,

-- 
David Jander
Protonic Holland.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Device-tree node renames break barebox/kernel compatibility...
  2018-06-12 12:23 Device-tree node renames break barebox/kernel compatibility David Jander
@ 2018-06-12 12:47 ` Sascha Hauer
  2018-06-12 13:46   ` David Jander
  0 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2018-06-12 12:47 UTC (permalink / raw)
  To: David Jander; +Cc: Fabio Estevam, Barebox List, Marco Franchi, Shawn Guo

Hi David,

On Tue, Jun 12, 2018 at 02:23:08PM +0200, David Jander wrote:
> 
> Hi Sascha,
> 
> I know this is old already, and I was surprised that I couldn't find any
> complaints about this yet, but I recently came across this patch in the kernel:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/patch/?id=df5cc9d0b42d15fa33b30440cca7a11ca7ba35a4
> 
> ...which was adopted in barebox as this:
> 
> https://git.pengutronix.de/cgit/barebox/patch/dts/src/arm/imx6qdl.dtsi?id=2e9cce8fb1f577088e2b20ae2f461130e13ad190
> 
> As I don't know the exact reason as to why this was necessary, or why this is
> an issue at all, I just wanted to point out the fact that this leads to some
> breakage in barebox new and old.

This issue is known and has led to
https://www.mail-archive.com/barebox@lists.infradead.org/msg25189.html

> 
> The specific problem I observed is here:
> https://git.pengutronix.de/cgit/barebox/tree/net/eth.c#n299

of_get_reproducible_name()/of_find_node_by_reproducible_name() is used for
fixing up mtd partitions and state nodes, but not yet for the ethernet
devices because nobody noticed.

To fix you would have to get a reproducible name of the ethernet device
node from the barebox device tree using of_find_node_by_reproducible_name()
and then use of_find_node_by_reproducible_name() on the kernel device
tree.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Device-tree node renames break barebox/kernel compatibility...
  2018-06-12 12:47 ` Sascha Hauer
@ 2018-06-12 13:46   ` David Jander
  2018-06-13  6:42     ` Sascha Hauer
  0 siblings, 1 reply; 6+ messages in thread
From: David Jander @ 2018-06-12 13:46 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Fabio Estevam, Barebox List, Marco Franchi, Shawn Guo

On Tue, 12 Jun 2018 14:47:38 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Hi David,
> 
> On Tue, Jun 12, 2018 at 02:23:08PM +0200, David Jander wrote:
> > 
> > Hi Sascha,
> > 
> > I know this is old already, and I was surprised that I couldn't find any
> > complaints about this yet, but I recently came across this patch in the kernel:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/patch/?id=df5cc9d0b42d15fa33b30440cca7a11ca7ba35a4
> > 
> > ...which was adopted in barebox as this:
> > 
> > https://git.pengutronix.de/cgit/barebox/patch/dts/src/arm/imx6qdl.dtsi?id=2e9cce8fb1f577088e2b20ae2f461130e13ad190
> > 
> > As I don't know the exact reason as to why this was necessary, or why this is
> > an issue at all, I just wanted to point out the fact that this leads to some
> > breakage in barebox new and old.  
> 
> This issue is known and has led to
> https://www.mail-archive.com/barebox@lists.infradead.org/msg25189.html

Ah, thanks for the pointer. I had searched through the list archive since
september-2017 and did not find this... need new glasses anyway.

> > 
> > The specific problem I observed is here:
> > https://git.pengutronix.de/cgit/barebox/tree/net/eth.c#n299  
> 
> of_get_reproducible_name()/of_find_node_by_reproducible_name() is used for
> fixing up mtd partitions and state nodes, but not yet for the ethernet
> devices because nobody noticed.
> 
> To fix you would have to get a reproducible name of the ethernet device
> node from the barebox device tree using of_find_node_by_reproducible_name()
> and then use of_find_node_by_reproducible_name() on the kernel device
> tree.

Ok, this makes sense. My only grief is that we are still left with a forced
bootloader update before being able to move to kernel 4.15+, for a reason
which to me doesn't sound really worth it. Granted, the leading zero's are a
bit ugly and probably never should have made it mainline, but I don't
understand why the DT compiler should suddenly complain about that and can't be
fixed. In a way, the device-tree is an interface from the kernel to the outside
world, and while it is not really "user-space", IMHO it should still be
regarded stable and "set in stone"... but maybe that's just me? Of course if
it really _is_ structurally wrong, it must be fixed, but please be aware that
it _will_ hurt people!
From a manufacturer perspective, I can say that while customers have come
to accept regular kernel updates (due to legitimate security reasons and
whatnot), bootloader upgrades are still a big issue. They usually don't accept
such "risky" upgrades in the field without putting up a fight :-(

Best regards,

-- 
David Jander
Protonic Holland.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Device-tree node renames break barebox/kernel compatibility...
  2018-06-12 13:46   ` David Jander
@ 2018-06-13  6:42     ` Sascha Hauer
  2018-06-13  6:50       ` David Jander
  0 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2018-06-13  6:42 UTC (permalink / raw)
  To: David Jander; +Cc: Fabio Estevam, Barebox List, Marco Franchi, Shawn Guo

On Tue, Jun 12, 2018 at 03:46:06PM +0200, David Jander wrote:
> On Tue, 12 Jun 2018 14:47:38 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > Hi David,
> > 
> > On Tue, Jun 12, 2018 at 02:23:08PM +0200, David Jander wrote:
> > > 
> > > Hi Sascha,
> > > 
> > > I know this is old already, and I was surprised that I couldn't find any
> > > complaints about this yet, but I recently came across this patch in the kernel:
> > > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/patch/?id=df5cc9d0b42d15fa33b30440cca7a11ca7ba35a4
> > > 
> > > ...which was adopted in barebox as this:
> > > 
> > > https://git.pengutronix.de/cgit/barebox/patch/dts/src/arm/imx6qdl.dtsi?id=2e9cce8fb1f577088e2b20ae2f461130e13ad190
> > > 
> > > As I don't know the exact reason as to why this was necessary, or why this is
> > > an issue at all, I just wanted to point out the fact that this leads to some
> > > breakage in barebox new and old.  
> > 
> > This issue is known and has led to
> > https://www.mail-archive.com/barebox@lists.infradead.org/msg25189.html
> 
> Ah, thanks for the pointer. I had searched through the list archive since
> september-2017 and did not find this... need new glasses anyway.
> 
> > > 
> > > The specific problem I observed is here:
> > > https://git.pengutronix.de/cgit/barebox/tree/net/eth.c#n299  
> > 
> > of_get_reproducible_name()/of_find_node_by_reproducible_name() is used for
> > fixing up mtd partitions and state nodes, but not yet for the ethernet
> > devices because nobody noticed.
> > 
> > To fix you would have to get a reproducible name of the ethernet device
> > node from the barebox device tree using of_find_node_by_reproducible_name()
> > and then use of_find_node_by_reproducible_name() on the kernel device
> > tree.
> 
> Ok, this makes sense. My only grief is that we are still left with a forced
> bootloader update before being able to move to kernel 4.15+, for a reason
> which to me doesn't sound really worth it.

You can still revert the offending patch on your Kernel device trees.

> Granted, the leading zero's are a
> bit ugly and probably never should have made it mainline, but I don't
> understand why the DT compiler should suddenly complain about that and can't be
> fixed. In a way, the device-tree is an interface from the kernel to the outside
> world, and while it is not really "user-space", IMHO it should still be
> regarded stable and "set in stone"... but maybe that's just me?

No, it's not just you. Me and other people are with you.

When talking about device tree stability we usually only talk about the
bindings, not about the structure of the trees. The problem here was
probably that people weren't aware that this change breaks stuff.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Device-tree node renames break barebox/kernel compatibility...
  2018-06-13  6:42     ` Sascha Hauer
@ 2018-06-13  6:50       ` David Jander
  2018-06-13 16:57         ` Fabio Estevam
  0 siblings, 1 reply; 6+ messages in thread
From: David Jander @ 2018-06-13  6:50 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Fabio Estevam, Barebox List, Marco Franchi, Shawn Guo

On Wed, 13 Jun 2018 08:42:30 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Tue, Jun 12, 2018 at 03:46:06PM +0200, David Jander wrote:
> > On Tue, 12 Jun 2018 14:47:38 +0200
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >   
> > > Hi David,
> > > 
> > > On Tue, Jun 12, 2018 at 02:23:08PM +0200, David Jander wrote:  
> > > > 
> > > > Hi Sascha,
> > > > 
> > > > I know this is old already, and I was surprised that I couldn't find any
> > > > complaints about this yet, but I recently came across this patch in the kernel:
> > > > 
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/patch/?id=df5cc9d0b42d15fa33b30440cca7a11ca7ba35a4
> > > > 
> > > > ...which was adopted in barebox as this:
> > > > 
> > > > https://git.pengutronix.de/cgit/barebox/patch/dts/src/arm/imx6qdl.dtsi?id=2e9cce8fb1f577088e2b20ae2f461130e13ad190
> > > > 
> > > > As I don't know the exact reason as to why this was necessary, or why this is
> > > > an issue at all, I just wanted to point out the fact that this leads to some
> > > > breakage in barebox new and old.    
> > > 
> > > This issue is known and has led to
> > > https://www.mail-archive.com/barebox@lists.infradead.org/msg25189.html  
> > 
> > Ah, thanks for the pointer. I had searched through the list archive since
> > september-2017 and did not find this... need new glasses anyway.
> >   
> > > > 
> > > > The specific problem I observed is here:
> > > > https://git.pengutronix.de/cgit/barebox/tree/net/eth.c#n299    
> > > 
> > > of_get_reproducible_name()/of_find_node_by_reproducible_name() is used for
> > > fixing up mtd partitions and state nodes, but not yet for the ethernet
> > > devices because nobody noticed.
> > > 
> > > To fix you would have to get a reproducible name of the ethernet device
> > > node from the barebox device tree using of_find_node_by_reproducible_name()
> > > and then use of_find_node_by_reproducible_name() on the kernel device
> > > tree.  
> > 
> > Ok, this makes sense. My only grief is that we are still left with a forced
> > bootloader update before being able to move to kernel 4.15+, for a reason
> > which to me doesn't sound really worth it.  
> 
> You can still revert the offending patch on your Kernel device trees.

I could, but it would be another patch to maintain against mainline...

> > Granted, the leading zero's are a
> > bit ugly and probably never should have made it mainline, but I don't
> > understand why the DT compiler should suddenly complain about that and can't be
> > fixed. In a way, the device-tree is an interface from the kernel to the outside
> > world, and while it is not really "user-space", IMHO it should still be
> > regarded stable and "set in stone"... but maybe that's just me?  
> 
> No, it's not just you. Me and other people are with you.
> 
> When talking about device tree stability we usually only talk about the
> bindings, not about the structure of the trees. The problem here was
> probably that people weren't aware that this change breaks stuff.

That's exactly what I also thought. That's in part also why I sent this e-mail
(the committers are in CC and hopefully listening ;-)
Sorry for the spam in that regard.

Best regards,

-- 
David Jander
Protonic Holland.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Device-tree node renames break barebox/kernel compatibility...
  2018-06-13  6:50       ` David Jander
@ 2018-06-13 16:57         ` Fabio Estevam
  0 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2018-06-13 16:57 UTC (permalink / raw)
  To: David Jander; +Cc: Fabio Estevam, Barebox List, Marco Franchi, Shawn Guo

Hi David,

On Wed, Jun 13, 2018 at 3:50 AM, David Jander <david@protonic.nl> wrote:
o, it's not just you. Me and other people are with you.
>>
>> When talking about device tree stability we usually only talk about the
>> bindings, not about the structure of the trees. The problem here was
>> probably that people weren't aware that this change breaks stuff.
>
> That's exactly what I also thought. That's in part also why I sent this e-mail
> (the committers are in CC and hopefully listening ;-)
> Sorry for the spam in that regard.

I was not aware that removing the leading zeroes would cause any problem, sorry.

Maybe you could also report this problem to Rob Herring and the
devicetree mainling list.

Thanks

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-06-13 16:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-12 12:23 Device-tree node renames break barebox/kernel compatibility David Jander
2018-06-12 12:47 ` Sascha Hauer
2018-06-12 13:46   ` David Jander
2018-06-13  6:42     ` Sascha Hauer
2018-06-13  6:50       ` David Jander
2018-06-13 16:57         ` Fabio Estevam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox