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? Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
