On Wed, Oct 21, 2020 at 4:54 PM Nico Huber <
nico.h@gmx.de> wrote:
On 22.10.20 00:29, Tim Wawrzynczak wrote:
> On Wed, Oct 21, 2020 at 4:00 PM Nico Huber <nico.h@gmx.de> wrote:
>
>> On 21.10.20 21:19, Tim Wawrzynczak via coreboot wrote:
>>> Currently there are 3 different "strapping" entries in the coreboot
>> tables,
>>> and with the recent addition of fw_config (
>>> https://review.coreboot.org/c/coreboot/+/41209), we would also like to
>> add
>>> the 64-bit fw_config field (updated here
>>> https://review.coreboot.org/c/coreboot/+/45939) to the coreboot table as
>>> well.
>>>
>>> In this patch (https://review.coreboot.org/c/coreboot/+/46605), I am
>>> proposing to deprecate the 3 current "strapping" entries (board ID, ram
>>> code and SKU ID), and add them all to 1 entry, containing board ID, ram
>>> code, SKU ID as well as fw_config. This saves the overhead of parsing 4
>>> different entries to obtain board configuration information.
>>>
>>> Would like to hear any thoughts on this,
>>
>> I'm actually very confused about these things and how they are supposed
>> to be used. Is it correct to say that there are / would be coreboot
>> table entries with board-specific encodings?
>
>
> There already are :) Board ID, SKU ID, and RAM Code are inherently
> mainboard (family) or vendor specific
> conventions. I see FW_CONFIG as yet another one of these strapping fields.
>
>
>> Wouldn't it be better to
>> decode the infos first and put that into tables so generic drivers can
>> consume them?
>
>
> That's an interesting thought, Nico. Can I assume you're talking about the
> coreboot table here?
Yes? Are you? ;)
I wasn't sure if you were implying I decode the ACPI tables :P (which I obviously
can't do on ARM, although I guess they have a DT).
>
> What I'm trying to accomplish here is to be able to pass the FW_CONFIG
> value from coreboot
> to the payload, in my case, obviously depthcharge. You can see some of our
> uses for fw_config
> in coreboot already, in mb/google/volteer for example. Some of the fields
> are distinguishing which
> daughterboard or audio device is on a given mainboard, which in these cases
> is not enumerable
> information, hence my thought to pass the fw_config value to the payload.
> This alleviates needing
> the payload to know where this information came from, as coreboot has
> already done the work
> to figure that out.
The question is, does depthcharge need to know the daughterboard id? or
does it actually need to know something that is implied by that id and
could be written into the table explicitly?
Sure, the ID contains implicit information. It would be possible to add a whole bunch of
new entry types into the coreboot table, but which way to take that?
Let's look at volteer as a strawman; I want to to able to tell depthcharge
which audio chip & interface (I2S vs. SNDW) it has. I could go two different ways with that:
1) Create an audio device table, which just contains board- or vendor- specific IDs in it, and doesn't really shift the issue away from board-specific encodings
2) reinvent ACPI / DT and encode the device structure & properties in a binary structure
or even add ACPI / DT parsing support to the payload, but that seems like overkill.
>> Generally, I wouldn't assume / want board-specific drivers outside of
>> coreboot. It seems board-specific table entries would invite people to
>> write such
>>
>
> I don't think this is encouraging board-specific drivers; just trying to
> pass information to the payload here, that's all.
Wouldn't that payload then naturally contain a board-specific driver? I
mean is it just displaying or storing the information or is it acting on
it?
The information has to live somewhere; it can live in a declarative table, or it can live in code.
>> Sorry if I completely misunderstood the intention of these entries.
>>
>
> I just hope I have explained myself well enough for others to understand :)
I guess I understand what you are doing. I just don't think it's a good
idea.
I am always open to suggestions on a better way to do something :)
Nico