Hi Jagan, On Tue, Dec 15, 2015 at 2:15 PM, Jagan Teki <jt...@openedev.com> wrote: > Hi Bin, > > > On Tuesday 15 December 2015 11:37 AM, Bin Meng wrote: >> >> Hi Jagan, >> >> On Tue, Dec 15, 2015 at 1:51 PM, Jagan Teki <jt...@openedev.com> wrote: >>> >>> Hi Bin, >>> >>> On 15 December 2015 at 10:48, Bin Meng <bmeng...@gmail.com> wrote: >>>> >>>> Hi Jagan, Simon, >>>> >>>> On Tue, Dec 8, 2015 at 11:44 PM, Jagan Teki <jt...@openedev.com> wrote: >>>>> >>>>> On 8 December 2015 at 17:27, Bin Meng <bmeng...@gmail.com> wrote: >>>>>> >>>>>> 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? >>>>> >>>>> >>>>> Good idea, but if we don't find enough foot-print difference on no >>>>> feature flags may be we can remove those config items and I have a >>>>> plan to re-arrange the sf_param_table which suits Linux may be I will >>>>> come back about these things. >>>>> >>>> >>>> Can you please suggest which way should we go for this patch? I still >>>> prefer one board with one SPI flash macro. >>> >>> >>> Sorry, I didn't get you what do you mean by one board with one SPI >>> flash macro? Suppose if board have one controller connected with micro >>> flash then the board file include CONFIG_SPI_FLASH_STMICRO and if >>> another board having two controllers one connected with spansion and >>> other connected with micro then the board file include >>> CONFIG_SPI_FLASH_STMICRO, CONFIG_SPI_FLASH_SPANSION. It's entirely up >>> to board that connected flash devices. >>> >> >> Yes, your understanding is the same as mine. I wasn't clear in my >> previous question. >> >> Right now this patch is doing exactly as what you and I understand, >> that we just want to select the specific flash macro for a specific >> x86 board. But Simon wanted to enable all of the flash macros for one >> board for convenience. Thus I came to ask for what's our direction. > > > So, does this board supports or connected all flash variants? in that case > it is true right? "for convenience" here means for testing then ie also true > because this particular board is meant for testing all flash devices, and > also it is up to this particular board config over-head rather than the > generic spi-flash. >
No, each board only connects one specific type of SPI flash, as described in the board device tree file. Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
