All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* Unable to implement board fdt-fixup for imx8m CPU
@ 2023-04-24 20:37 Hugo Villeneuve
  2023-04-24 21:01 ` Simon Glass
  0 siblings, 1 reply; 8+ messages in thread
From: Hugo Villeneuve @ 2023-04-24 20:37 UTC (permalink / raw
  To: u-boot

Hi,
according to this document:

    doc/develop/driver-model/fdt-fixup.rst

it is suggested that boards implement the board_fix_fdt() function to tweak the device tree. I am trying to do just that, but unfortunately I cannot because the following source file already has an implementation of board_fix_fdt():

    arch/arm/mach-imx/imx8m/soc.c

For all boards using a imx8m CPU, how can we implement board_fix_fdt()?

Thank you.

-- 
Hugo Villeneuve <hugo@hugovil.com>

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

* Re: Unable to implement board fdt-fixup for imx8m CPU
  2023-04-24 20:37 Unable to implement board fdt-fixup for imx8m CPU Hugo Villeneuve
@ 2023-04-24 21:01 ` Simon Glass
  2023-04-24 21:57   ` Hugo Villeneuve
  0 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2023-04-24 21:01 UTC (permalink / raw
  To: Hugo Villeneuve; +Cc: u-boot

Hi Hugo,

On Mon, 24 Apr 2023 at 14:53, Hugo Villeneuve <hugo@hugovil.com> wrote:
>
> Hi,
> according to this document:
>
>     doc/develop/driver-model/fdt-fixup.rst
>
> it is suggested that boards implement the board_fix_fdt() function to tweak the device tree. I am trying to do just that, but unfortunately I cannot because the following source file already has an implementation of board_fix_fdt():
>
>     arch/arm/mach-imx/imx8m/soc.c
>
> For all boards using a imx8m CPU, how can we implement board_fix_fdt()?

If you enable CONFIG_EVENT you can use EVT_FT_FIXUP to add as many
fixup functions as you need.

Migration to use that mechanism everywhere hasn't really started, but
I think it would be useful.

Regards,
Simon

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

* Re: Unable to implement board fdt-fixup for imx8m CPU
  2023-04-24 21:01 ` Simon Glass
@ 2023-04-24 21:57   ` Hugo Villeneuve
  2023-04-24 23:33     ` Simon Glass
  2023-04-25  1:09     ` Adam Ford
  0 siblings, 2 replies; 8+ messages in thread
From: Hugo Villeneuve @ 2023-04-24 21:57 UTC (permalink / raw
  To: Simon Glass; +Cc: u-boot

On Mon, 24 Apr 2023 15:01:35 -0600
Simon Glass <sjg@chromium.org> wrote:

> Hi Hugo,
> 
> On Mon, 24 Apr 2023 at 14:53, Hugo Villeneuve <hugo@hugovil.com> wrote:
> >
> > Hi,
> > according to this document:
> >
> >     doc/develop/driver-model/fdt-fixup.rst
> >
> > it is suggested that boards implement the board_fix_fdt() function to tweak the device tree. I am trying to do just that, but unfortunately I cannot because the following source file already has an implementation of board_fix_fdt():
> >
> >     arch/arm/mach-imx/imx8m/soc.c
> >
> > For all boards using a imx8m CPU, how can we implement board_fix_fdt()?
> 
> If you enable CONFIG_EVENT you can use EVT_FT_FIXUP to add as many
> fixup functions as you need.
> 
> Migration to use that mechanism everywhere hasn't really started, but
> I think it would be useful.

Hi Simon,
to be more precise, I need to modify the FDT used by U-Boot to tweak some ethernet PHY properties before initializing the ethernet controller. I do _not_ need to fix the FDT before booting in the OS.

With that in mind, do you still think using CONFIG_EVENT is the way to go? I am not sure from my reading of the source code because it seems that fixup events are called just before booting into the OS.

Thank you,
Hugo.

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

* Re: Unable to implement board fdt-fixup for imx8m CPU
  2023-04-24 21:57   ` Hugo Villeneuve
@ 2023-04-24 23:33     ` Simon Glass
  2023-04-25  1:09     ` Adam Ford
  1 sibling, 0 replies; 8+ messages in thread
From: Simon Glass @ 2023-04-24 23:33 UTC (permalink / raw
  To: Hugo Villeneuve; +Cc: u-boot

Hi Hugo,

On Mon, 24 Apr 2023 at 15:57, Hugo Villeneuve <hugo@hugovil.com> wrote:
>
> On Mon, 24 Apr 2023 15:01:35 -0600
> Simon Glass <sjg@chromium.org> wrote:
>
> > Hi Hugo,
> >
> > On Mon, 24 Apr 2023 at 14:53, Hugo Villeneuve <hugo@hugovil.com> wrote:
> > >
> > > Hi,
> > > according to this document:
> > >
> > >     doc/develop/driver-model/fdt-fixup.rst
> > >
> > > it is suggested that boards implement the board_fix_fdt() function to tweak the device tree. I am trying to do just that, but unfortunately I cannot because the following source file already has an implementation of board_fix_fdt():
> > >
> > >     arch/arm/mach-imx/imx8m/soc.c
> > >
> > > For all boards using a imx8m CPU, how can we implement board_fix_fdt()?
> >
> > If you enable CONFIG_EVENT you can use EVT_FT_FIXUP to add as many
> > fixup functions as you need.
> >
> > Migration to use that mechanism everywhere hasn't really started, but
> > I think it would be useful.
>
> Hi Simon,
> to be more precise, I need to modify the FDT used by U-Boot to tweak some ethernet PHY properties before initializing the ethernet controller. I do _not_ need to fix the FDT before booting in the OS.
>
> With that in mind, do you still think using CONFIG_EVENT is the way to go? I am not sure from my reading of the source code because it seems that fixup events are called just before booting into the OS.

Well you could create a new event type and send it at a different
type, if that helps. In general these weak functions are a pain IMO,
since it is hard to know which one is called, and you can ultimately
only have one main function and a backup. So I am encouraging the use
of events instead.

Regards,
Simon

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

* Re: Unable to implement board fdt-fixup for imx8m CPU
  2023-04-24 21:57   ` Hugo Villeneuve
  2023-04-24 23:33     ` Simon Glass
@ 2023-04-25  1:09     ` Adam Ford
  2023-04-25  2:17       ` Hugo Villeneuve
  1 sibling, 1 reply; 8+ messages in thread
From: Adam Ford @ 2023-04-25  1:09 UTC (permalink / raw
  To: Hugo Villeneuve; +Cc: Simon Glass, u-boot

On Mon, Apr 24, 2023 at 4:57 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
>
> On Mon, 24 Apr 2023 15:01:35 -0600
> Simon Glass <sjg@chromium.org> wrote:
>
> > Hi Hugo,
> >
> > On Mon, 24 Apr 2023 at 14:53, Hugo Villeneuve <hugo@hugovil.com> wrote:
> > >
> > > Hi,
> > > according to this document:
> > >
> > >     doc/develop/driver-model/fdt-fixup.rst
> > >
> > > it is suggested that boards implement the board_fix_fdt() function to tweak the device tree. I am trying to do just that, but unfortunately I cannot because the following source file already has an implementation of board_fix_fdt():
> > >
> > >     arch/arm/mach-imx/imx8m/soc.c
> > >
> > > For all boards using a imx8m CPU, how can we implement board_fix_fdt()?
> >
> > If you enable CONFIG_EVENT you can use EVT_FT_FIXUP to add as many
> > fixup functions as you need.
> >
> > Migration to use that mechanism everywhere hasn't really started, but
> > I think it would be useful.
>
> Hi Simon,
> to be more precise, I need to modify the FDT used by U-Boot to tweak some ethernet PHY properties before initializing the ethernet controller. I do _not_ need to fix the FDT before booting in the OS.

Can you modify the -u-boot.dtsi file?  You can add additional device
tree stuff to that.  I think a few boards do this already specifically
for the Ethernet stuff.

adam
>
> With that in mind, do you still think using CONFIG_EVENT is the way to go? I am not sure from my reading of the source code because it seems that fixup events are called just before booting into the OS.
>
> Thank you,
> Hugo.

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

* Re: Unable to implement board fdt-fixup for imx8m CPU
  2023-04-25  1:09     ` Adam Ford
@ 2023-04-25  2:17       ` Hugo Villeneuve
  2023-04-25 12:19         ` Rasmus Villemoes
  0 siblings, 1 reply; 8+ messages in thread
From: Hugo Villeneuve @ 2023-04-25  2:17 UTC (permalink / raw
  To: Adam Ford; +Cc: Simon Glass, u-boot

On Mon, 24 Apr 2023 20:09:53 -0500
Adam Ford <aford173@gmail.com> wrote:

> On Mon, Apr 24, 2023 at 4:57 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> >
> > On Mon, 24 Apr 2023 15:01:35 -0600
> > Simon Glass <sjg@chromium.org> wrote:
> >
> > > Hi Hugo,
> > >
> > > On Mon, 24 Apr 2023 at 14:53, Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > >
> > > > Hi,
> > > > according to this document:
> > > >
> > > >     doc/develop/driver-model/fdt-fixup.rst
> > > >
> > > > it is suggested that boards implement the board_fix_fdt() function to tweak the device tree. I am trying to do just that, but unfortunately I cannot because the following source file already has an implementation of board_fix_fdt():
> > > >
> > > >     arch/arm/mach-imx/imx8m/soc.c
> > > >
> > > > For all boards using a imx8m CPU, how can we implement board_fix_fdt()?
> > >
> > > If you enable CONFIG_EVENT you can use EVT_FT_FIXUP to add as many
> > > fixup functions as you need.
> > >
> > > Migration to use that mechanism everywhere hasn't really started, but
> > > I think it would be useful.
> >
> > Hi Simon,
> > to be more precise, I need to modify the FDT used by U-Boot to tweak some ethernet PHY properties before initializing the ethernet controller. I do _not_ need to fix the FDT before booting in the OS.
> 
> Can you modify the -u-boot.dtsi file?  You can add additional device
> tree stuff to that.  I think a few boards do this already specifically
> for the Ethernet stuff.
> 
> adam

Hi Adam,
I need to detect at runtime the SOM configuration from an EEPROM, and based on that information adjust the ethernet PHY property inside the device tree.

I have already done that with success, it works great and as a temporary workaround I simply call my own function imx8m_board_fix_fdt() at the end of the function board_fix_fdt() of arch/arm/mach-imx/imx8m/soc.c.

But I don't think that a "board" level function should have been put in a SOC/arch source file in arch/arm/mach-imx/imx8m/soc.c in the first place. If it were not for that, there wouldn't be any need to define something new (event or other).

Maybe we should try to remove that board level function from arch/arm/mach-imx/imx8m/soc.c and use some other already existing mechanism for SOC-level device tree fixups?

Hugo.

> >
> > With that in mind, do you still think using CONFIG_EVENT is the way to go? I am not sure from my reading of the source code because it seems that fixup events are called just before booting into the OS.
> >
> > Thank you,
> > Hugo.

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

* Re: Unable to implement board fdt-fixup for imx8m CPU
  2023-04-25  2:17       ` Hugo Villeneuve
@ 2023-04-25 12:19         ` Rasmus Villemoes
  2023-04-25 14:08           ` Hugo Villeneuve
  0 siblings, 1 reply; 8+ messages in thread
From: Rasmus Villemoes @ 2023-04-25 12:19 UTC (permalink / raw
  To: Hugo Villeneuve, Adam Ford; +Cc: Simon Glass, u-boot, Peng Fan, Tom Rini

On 25/04/2023 04.17, Hugo Villeneuve wrote:

> I need to detect at runtime the SOM configuration from an EEPROM, and based on that information adjust the ethernet PHY property inside the device tree.
> 
> I have already done that with success, it works great and as a temporary workaround I simply call my own function imx8m_board_fix_fdt() at the end of the function board_fix_fdt() of arch/arm/mach-imx/imx8m/soc.c.
> 
> But I don't think that a "board" level function should have been put in a SOC/arch source file in arch/arm/mach-imx/imx8m/soc.c in the first place. If it were not for that, there wouldn't be any need to define something new (event or other).
> 
> Maybe we should try to remove that board level function from arch/arm/mach-imx/imx8m/soc.c and use some other already existing mechanism for SOC-level device tree fixups?

AFAICT, you/we can just nuke that function completely.

(1) As you say, it's not appropriate for arch/SOC to provide that in the
first place.
(2) It's completely dead and useless code:
(2a) No in-tree imx8m-based board seems to set CONFIG_OF_BOARD_FIXUP
(2b) The nodes which that function wants to disable don't even exist in
the U-Boot copy of imx8mp.dtsi.

Commit 35bb60787b88 should never have been applied in mainline U-Boot,
it's some random import of code from downstream NXP U-Boot, with a
commit message that makes no sense in upstream context.

Rasmus


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

* Re: Unable to implement board fdt-fixup for imx8m CPU
  2023-04-25 12:19         ` Rasmus Villemoes
@ 2023-04-25 14:08           ` Hugo Villeneuve
  0 siblings, 0 replies; 8+ messages in thread
From: Hugo Villeneuve @ 2023-04-25 14:08 UTC (permalink / raw
  To: Rasmus Villemoes; +Cc: Adam Ford, Simon Glass, u-boot, Peng Fan, Tom Rini

On Tue, 25 Apr 2023 14:19:14 +0200
Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:

> On 25/04/2023 04.17, Hugo Villeneuve wrote:
> 
> > I need to detect at runtime the SOM configuration from an EEPROM, and based on that information adjust the ethernet PHY property inside the device tree.
> > 
> > I have already done that with success, it works great and as a temporary workaround I simply call my own function imx8m_board_fix_fdt() at the end of the function board_fix_fdt() of arch/arm/mach-imx/imx8m/soc.c.
> > 
> > But I don't think that a "board" level function should have been put in a SOC/arch source file in arch/arm/mach-imx/imx8m/soc.c in the first place. If it were not for that, there wouldn't be any need to define something new (event or other).
> > 
> > Maybe we should try to remove that board level function from arch/arm/mach-imx/imx8m/soc.c and use some other already existing mechanism for SOC-level device tree fixups?
> 
> AFAICT, you/we can just nuke that function completely.
> 
> (1) As you say, it's not appropriate for arch/SOC to provide that in the
> first place.
> (2) It's completely dead and useless code:
> (2a) No in-tree imx8m-based board seems to set CONFIG_OF_BOARD_FIXUP
> (2b) The nodes which that function wants to disable don't even exist in
> the U-Boot copy of imx8mp.dtsi.
> 
> Commit 35bb60787b88 should never have been applied in mainline U-Boot,
> it's some random import of code from downstream NXP U-Boot, with a
> commit message that makes no sense in upstream context.
> 
> Rasmus

Hi Rasmus,
I agree with your analysis, and I will submit a patch to remove that code.

Thank you.

-- 
Hugo Villeneuve

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

end of thread, other threads:[~2023-04-25 14:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-24 20:37 Unable to implement board fdt-fixup for imx8m CPU Hugo Villeneuve
2023-04-24 21:01 ` Simon Glass
2023-04-24 21:57   ` Hugo Villeneuve
2023-04-24 23:33     ` Simon Glass
2023-04-25  1:09     ` Adam Ford
2023-04-25  2:17       ` Hugo Villeneuve
2023-04-25 12:19         ` Rasmus Villemoes
2023-04-25 14:08           ` Hugo Villeneuve

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.