[U-Boot] [PATCH v3] spi: ich: Configure SPI BIOS parameters

[Stefan Roese]

Hi Simon,

On 22.02.2017 04:59, Simon Glass wrote:

<snip>

>>>> What if the board mounts a flash with a different SPI flash command
>>>> set? Will this work?
>>>
>>>
>>> Frankly, I can't tell for sure but its very likely. As you might
>>> have guessed, these defines are taken from coreboot where they are
>>> usually board specific and written in the last stage before booting
>>> into the OS (IIRC). It would definitely make sense to add a mechanism
>>> to configure these BIOS parameters in a board-specific way. So that
>>> boards can choose to (optionally) provide different values.
>>>
>>> An easy way to do this, would be to make the newly created function
>>> spi_controller_config() a wear default. This way, board can always
>>> overwrite these default values (and / or do other stuff) in their
>>> board specific code. This could also be added in a follow up patch
>>> once this is needed though. I still have to see such a case of an
>>> "incompatible" SPI flash on x86, and IIRC all values in coreboot
>>> were identical.
>>
>> I am OK with the weak implementation, but I know Simon dislikes weak
>> :) Maybe he has a better idea.
>>
>> Let's hear what others think.
>
> Well how about crossing that bridge when we come to it? For now,
> perhaps this is good enough.

Thanks.

> You are right, I'm not keen on weak
> functions as I see them as ad-hoc APIs. Better, I believe, to think
> about it and define a real API. For example in this case I wonder if
> the remove() method of the SPI driver could do something? Or perhaps
> add another method like finalise()?

Okay, lets think about this in more detail, once its necessary.

Thanks,
Stefan