Board revision dependent settings in device tree/override tree?
Dear coreboot folks, Change-set *mb/google/nissa/var/craaskov: Disable external fivr* [1] moves the settings for the external fully integrated voltage regulator (FIVR) out of device tree/override tree to only apply it to board revision with an external FIVR. Is there a way to support conditions in device tree? Kind regards, Paul [1]: https://review.coreboot.org/c/coreboot/+/81607
hi Paul, I think the patch does it the right way, we want to minimize the amount of conditionals in the devicetree, it's supposed to be a static representation of the board hardware. These are just FSP UPDs that are being set anyway. In this case, the board ID is read from an EEPROM (CBI) by the EC, so I'm not sure that would be available early enough to conditionally set the devicetree params anyway. regards, Matt On Tue, Apr 2, 2024 at 4:38 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
Dear coreboot folks,
Change-set *mb/google/nissa/var/craaskov: Disable external fivr* [1] moves the settings for the external fully integrated voltage regulator (FIVR) out of device tree/override tree to only apply it to board revision with an external FIVR.
Is there a way to support conditions in device tree?
Kind regards,
Paul
[1]: https://review.coreboot.org/c/coreboot/+/81607 _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
I'll probably regret mentioning this, but recall that the register code is, by design, C. So if you look at that CL, you see this .vnn_supported_voltage_bitmap = FIVR_VOLTAGE_MIN_ACTIVE, that could also be a macro. As Matt mentions, it's not something you can know at build time, so it's hard to see a way to use it. On Tue, Apr 2, 2024 at 5:39 AM Matt DeVillier <matt.devillier@gmail.com> wrote:
hi Paul,
I think the patch does it the right way, we want to minimize the amount of conditionals in the devicetree, it's supposed to be a static representation of the board hardware. These are just FSP UPDs that are being set anyway.
In this case, the board ID is read from an EEPROM (CBI) by the EC, so I'm not sure that would be available early enough to conditionally set the devicetree params anyway.
regards, Matt
On Tue, Apr 2, 2024 at 4:38 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
Dear coreboot folks,
Change-set *mb/google/nissa/var/craaskov: Disable external fivr* [1] moves the settings for the external fully integrated voltage regulator (FIVR) out of device tree/override tree to only apply it to board revision with an external FIVR.
Is there a way to support conditions in device tree?
Kind regards,
Paul
[1]: https://review.coreboot.org/c/coreboot/+/81607 _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
_______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
Hi all,We actually use Kconfig options and macros in the register settings now, though not widely: `register "usb2_ports[CONFIG_CCD_PORT]" = "USB2_PORT_MID(OC_SKIP)"` We've also discussed running the devicetrees through the C preprocessor, or introducing another method to allow for more configurability, though that's only for updating the generation of the data structures, not anything configurable at runtime. Another thing that has been discussed, though probably not as much is having multiple devicetrees, or even parts of devicetrees and selecting between them at runtime. This could be useful to make board variants that all work with a single firmware image. Adding any sort of runtime configuration to devicetree would be significantly more difficult. The devicetree structure is just data, with no way to have it run any code. It's used in all of the various stages, some of which can be XIP, so it's not like it could even just contain a pointer to a function. Typically, if there's something based on the devicetree that might need to be updated at runtime, we just use a flag for an "auto" or "default" setting that gets the value from somewhere else when we're running code based on the devicetree. I think that's probably what we want to continue for any sort of runtime configuration. Martin Apr 2, 2024, 09:12 by rminnich@gmail.com:
I'll probably regret mentioning this, but recall that the register code is, by design, C. So if you look at that CL, you see this .vnn_supported_voltage_bitmap = FIVR_VOLTAGE_MIN_ACTIVE, that could also be a macro.
As Matt mentions, it's not something you can know at build time, so it's hard to see a way to use it.
On Tue, Apr 2, 2024 at 5:39 AM Matt DeVillier <> matt.devillier@gmail.com> > wrote:
hi Paul,
I think the patch does it the right way, we want to minimize the amount of conditionals in the devicetree, it's supposed to be a static representation of the board hardware. These are just FSP UPDs that are being set anyway.
In this case, the board ID is read from an EEPROM (CBI) by the EC, so I'm not sure that would be available early enough to conditionally set the devicetree params anyway.
regards, Matt
On Tue, Apr 2, 2024 at 4:38 AM Paul Menzel <>> pmenzel@molgen.mpg.de>> > wrote:
Dear coreboot folks,
Change-set *mb/google/nissa/var/craaskov: Disable external fivr* [1] moves the settings for the external fully integrated voltage regulator (FIVR) out of device tree/override tree to only apply it to board revision with an external FIVR.
Is there a way to support conditions in device tree?
Kind regards,
Paul
[1]: >>> https://review.coreboot.org/c/coreboot/+/81607 _______________________________________________ coreboot mailing list -- >>> coreboot@coreboot.org To unsubscribe send an email to >>> coreboot-leave@coreboot.org
_______________________________________________ coreboot mailing list -- >> coreboot@coreboot.org To unsubscribe send an email to >> coreboot-leave@coreboot.org
yes, the devicetree was designed in the first place (this is ca 2000) to be all about static initialization. On Tue, Apr 2, 2024 at 8:53 AM Martin Roth <gaumless@tutanota.com> wrote:
Hi all,We actually use Kconfig options and macros in the register settings now, though not widely: `register "usb2_ports[CONFIG_CCD_PORT]" = "USB2_PORT_MID(OC_SKIP)"`
We've also discussed running the devicetrees through the C preprocessor, or introducing another method to allow for more configurability, though that's only for updating the generation of the data structures, not anything configurable at runtime.
Another thing that has been discussed, though probably not as much is having multiple devicetrees, or even parts of devicetrees and selecting between them at runtime. This could be useful to make board variants that all work with a single firmware image. Adding any sort of runtime configuration to devicetree would be significantly more difficult. The devicetree structure is just data, with no way to have it run any code. It's used in all of the various stages, some of which can be XIP, so it's not like it could even just contain a pointer to a function.
Typically, if there's something based on the devicetree that might need to be updated at runtime, we just use a flag for an "auto" or "default" setting that gets the value from somewhere else when we're running code based on the devicetree. I think that's probably what we want to continue for any sort of runtime configuration. Martin Apr 2, 2024, 09:12 by rminnich@gmail.com:
I'll probably regret mentioning this, but recall that the register code is, by design, C. So if you look at that CL, you see this .vnn_supported_voltage_bitmap = FIVR_VOLTAGE_MIN_ACTIVE, that could also be a macro.
As Matt mentions, it's not something you can know at build time, so it's hard to see a way to use it.
On Tue, Apr 2, 2024 at 5:39 AM Matt DeVillier <> matt.devillier@gmail.com> > wrote:
hi Paul,
I think the patch does it the right way, we want to minimize the amount of conditionals in the devicetree, it's supposed to be a static representation of the board hardware. These are just FSP UPDs that are being set anyway.
In this case, the board ID is read from an EEPROM (CBI) by the EC, so I'm not sure that would be available early enough to conditionally set the devicetree params anyway.
regards, Matt
On Tue, Apr 2, 2024 at 4:38 AM Paul Menzel <>> pmenzel@molgen.mpg.de>> wrote:
Dear coreboot folks,
Change-set *mb/google/nissa/var/craaskov: Disable external fivr* [1] moves the settings for the external fully integrated voltage regulator (FIVR) out of device tree/override tree to only apply it to board revision with an external FIVR.
Is there a way to support conditions in device tree?
Kind regards,
Paul
[1]: >>> https://review.coreboot.org/c/coreboot/+/81607 _______________________________________________ coreboot mailing list -- >>> coreboot@coreboot.org To unsubscribe send an email to >>> coreboot-leave@coreboot.org
_______________________________________________ coreboot mailing list -- >> coreboot@coreboot.org To unsubscribe send an email to >> coreboot-leave@coreboot.org
Hi Paul, On 02.04.24 11:38, Paul Menzel wrote:
Change-set *mb/google/nissa/var/craaskov: Disable external fivr* [1] moves the settings for the external fully integrated voltage regulator (FIVR) out of device tree/override tree to only apply it to board revision with an external FIVR.
Is there a way to support conditions in device tree?
as others pointed out this is about runtime conditions in particular. There's actually one runtime mechanism implemented currently: the `probe` statement. It does a simple comparison and if that fails, the device with the probe gets disabled. This doesn't seem to be imple- mented for the board_id values atm. But extending the mechanism should be easy. However there's a bigger problem in [1], that a monolithic, FSP-style struct is used instead of modeling individual devices. Even with a mechanism to disable an FIVR device at runtime, it would need more changes to configure FSP with an actual device*tree*. Nico
participants (5)
-
Martin Roth -
Matt DeVillier -
Nico Huber -
Paul Menzel -
ron minnich