Hi Bin, On 6 February 2016 at 09:46, Bin Meng <bmeng...@gmail.com> wrote: > 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.
Except - macronix, spansion, stmicro, sst, winbond configs we can remove remaining flash vendor configs for now, because later configs using common code. -- Jagan. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
