Hi, On 3 December 2015 at 06:27, Bin Meng <bmeng...@gmail.com> wrote: > Hi Jagan, > > On Thu, Dec 3, 2015 at 6:24 PM, Jagan Teki <jt...@openedev.com> wrote: >> Hi Bin, >> >> On 3 December 2015 at 10:14, Bin Meng <bmeng...@gmail.com> wrote: >>> Hi Simon, >>> >>> On Thu, Dec 3, 2015 at 5:05 AM, Simon Glass <s...@chromium.org> wrote: >>>> +Jagan >>>> >>>> Hi Bin, >>>> >>>> On 1 December 2015 at 18:41, Bin Meng <bmeng...@gmail.com> wrote: >>>>> >>>>> Hi Simon, >>>>> >>>>> On Wed, Dec 2, 2015 at 12:32 AM, Simon Glass <s...@chromium.org> wrote: >>>>> > Hi Bin, >>>>> > >>>>> > On 28 November 2015 at 05:45, Bin Meng <bmeng...@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...@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. >> >> Are you talking about flash vendor config or CONFIG_SPI_FLASH? >> > > Flash vendor config, as you see in this patch. > >>>> >>>> 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. >> >> But there is some flash vendor specific code like quad enable bit, >> locking ops and finally about spi_flash_params table. >> > > I know. That's probably why adding all these SPI flash drivers don't > help at all because only one code path will take effect. And what I > did in this patch is to select one type of flash per board.
So how about we group together 3-4 of the common ones, with no special features, into a 'CONFIG_SPI_FLASH_GENERIC'? > >>> >>>>> >>>>> > >>>>> > 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. >> >> thanks! >> -- > > Regards, > Bin Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
