[U-Boot] [PATCH] x86: Clean up SPI flash drivers in defconfig

[Bin Meng]

Hi Simon,

On Thu, Dec 3, 2015 at 5:05 AM, Simon Glass <sjg at chromium.org> wrote:
> +Jagan
>
> Hi Bin,
>
> On 1 December 2015 at 18:41, Bin Meng <bmeng.cn at gmail.com> wrote:
>>
>> Hi Simon,
>>
>> On Wed, Dec 2, 2015 at 12:32 AM, Simon Glass <sjg at chromium.org> wrote:
>> > Hi Bin,
>> >
>> > On 28 November 2015 at 05:45, Bin Meng <bmeng.cn at gmail.com> wrote:
>> >> Every board has one dedicated type of SPI flash, hence it is
>> >> unnecessary to include multiple SPI flash drivers.
>> >>
>> >> For QEMU and coreboot (default build of coreboot is also QEMU),
>> >> SPI flash is not supported. Remove those SPI flash drivers.
>> >>
>> >> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>> >> ---
>> >>
>> >>  configs/bayleybay_defconfig         | 2 --
>> >>  configs/chromebook_link_defconfig   | 2 --
>> >>  configs/chromebox_panther_defconfig | 2 --
>> >>  configs/coreboot-x86_defconfig      | 4 ----
>> >>  configs/crownbay_defconfig          | 3 ---
>> >>  configs/galileo_defconfig           | 2 --
>> >>  configs/minnowmax_defconfig         | 3 ---
>> >>  configs/qemu-x86_defconfig          | 4 ----
>> >>  8 files changed, 22 deletions(-)
>> >
>> > What is the benefit of this? I see it removes a few lines in a data
>> > table. Does it matter?
>>
>> Maybe we should ask the other way around, why do we create so many
>> flash driver Kconfig option? I believe the intention was footprint.
>> Besides the footprint issue, having just one flash driver in each
>> board makes it very clear instead of causing confusion. Looks other
>> board defconfig files only select one.
>
> They are a hangover from when we had a separate driver for each one.
> Jagan put a lot of effort into removing all the semi-duplicated code.
>
> Maybe we should prune down these options?
>

But if we already spent a lot of effort into removing all the
semi-duplicated code, we should not have converted those flash driver
to Kconfig options before.

See commit d5af92315bb48740f16bf8817f38e227d3076905 "sf: kconfig: add
kconfig options for spi flashes"

I suspect we may remove most of these SPI flash macros, but at least
SST flash macro should be kept since right now it is mixed in the
generic driver with a special byte program and word program which is
incompatible with other vendors' flashes.

>>
>> >
>> > For all of these platforms we can use the dediprog em100 which I
>> > typically set to use winbond as the manufacturer, regardless of which
>> > chip is actually on the board.
>> >
>>
>> I think that's because emulator can emulate flash from various vendors.
>
> Yes, and also for convenience.
>
>>
>> > For U-Boot on coreboot, why is SPI flash not supported? It certainly
>> > works with link.
>>
>> Yes, booting from coreboot does support SPI flash. However since we
>> decided to use QEMU as the default build target for coreboot, and QEMU
>> does not support SPI flash yet, these config options are removed. One
>> can certainly adjust these Kconfig options via 'make menuconfig', eg:
>> adding SD/MMC support which is not in coreboot's defconfig either.
>
> Well this breaks booting on link, since the SPI flash stops working.
> I'm really not keen on having to specially select the SPI flash when
> you want to use link.
>

Do you mean booting U-Boot on link from coreboot? Without SPI flash it
should still boot. It looks to me your preference is to include all
the available drivers into coreboot's defconfig, yes? If so, we may
add some other drivers Kconfig in coreboot-x86_defconfig too, like
SD/MMC drivers, all the available ethernet drivers even they only
exist on some boards.

Regards,
Bin