Hi Jagan, Simon, On Thu, Jan 21, 2016 at 1:53 PM, Bin Meng <bmeng...@gmail.com> wrote: > Hi, > > On Tue, Dec 15, 2015 at 2:27 PM, Jagan Teki <jt...@openedev.com> wrote: >> >> On 15 December 2015 at 11:48, Bin Meng <bmeng...@gmail.com> wrote: >> > 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. >> >> So your patch did that change? let me look at it what Simon commenting. >> > > I don't see any further comments on this patch. Can we close this? >
A gentle ping .. I still would like to clean these up unless there is something changes in the SF (as Simon proposed) to be planned. Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
