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

[Bin Meng]

Hi Stefan, Simon,

On Wed, Feb 22, 2017 at 3:07 PM, Stefan Roese <sr at denx.de> wrote:
> 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.
>

What's next step for this patch? I see Stefan has done some patches on dm core.

Regards,
Bin