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.

thanks!
--
Jagan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to