Hi Jagan, On Fri, Dec 4, 2015 at 2:57 AM, Simon Glass <s...@chromium.org> wrote: > 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'? >
Can you comment on this CONFIG_SPI_FLASH_GENERIC as Simon suggested? [snip] Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
