Re: [U-Boot] [PATCH v3 16/20] mtd: spi: Add lightweight SPI flash stack for SPL
On Fri, Feb 1, 2019 at 10:35 PM Vignesh R wrote: > > > > On 01-Feb-19 9:18 PM, Jagan Teki wrote: > > On Thu, Jan 31, 2019 at 11:20 PM Vignesh R wrote: > >> > [...] > >>> > >>> This doesn't look good to me, this change is part of 08/20 and now it > >>> removed. better do the same change in 08/20 by adding new file > >>> spi-nor-ids.c > >>> > >> > >> This is intentional. Patch 8-11 clearly shows what all is being sync'd > >> from Kernel and I would like to keep that as is. > >> Merging U-Boot specific changes with those patches does not provide a > >> clean history > >> spi-nor-ids table is moved out of spi-nor-core.c in this patch because > >> we need it for two separate compilation units (spi-nor-tiny and > >> spi-nor-core). > > > > Understand this point, but since it's not a direct commit sync and we > > even add changes wrt u-boot and remove unneeded changes related to > > Linux. > > This is a direct sync. I removed code not related to U-Boot on your > insistence. But, code organization is same as Linux. > I had no intention of splitting ID table into a separate file (see RFC > v1 where it was still in the original file) as there was no other user > of ID table and keeping table in same file allows compiler to carry out > compile time optimizations. > > It's fine to create -ids.c file in the same commit, otherwise > > it is simply adding code and removing the same in following commit > > doesn't suit for bisectable. > > > > Sorry, I do not agree on this.. This patch clearly captures the _need_ > to move ID table out of spi-nor-core.c. Code is being moved out of > spi-nor-core.c to _support_ tiny stack. At no point in this series > bisectablility is broken. > > Redoing patch 8/20 is non trivial and would be painful rework with no > gain. I will have to rework patch 8/20 and 14/20 so as to not break > compilation. OK. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 16/20] mtd: spi: Add lightweight SPI flash stack for SPL
On 01-Feb-19 9:18 PM, Jagan Teki wrote: > On Thu, Jan 31, 2019 at 11:20 PM Vignesh R wrote: >> [...] >>> >>> This doesn't look good to me, this change is part of 08/20 and now it >>> removed. better do the same change in 08/20 by adding new file >>> spi-nor-ids.c >>> >> >> This is intentional. Patch 8-11 clearly shows what all is being sync'd >> from Kernel and I would like to keep that as is. >> Merging U-Boot specific changes with those patches does not provide a >> clean history >> spi-nor-ids table is moved out of spi-nor-core.c in this patch because >> we need it for two separate compilation units (spi-nor-tiny and >> spi-nor-core). > > Understand this point, but since it's not a direct commit sync and we > even add changes wrt u-boot and remove unneeded changes related to > Linux. This is a direct sync. I removed code not related to U-Boot on your insistence. But, code organization is same as Linux. I had no intention of splitting ID table into a separate file (see RFC v1 where it was still in the original file) as there was no other user of ID table and keeping table in same file allows compiler to carry out compile time optimizations. It's fine to create -ids.c file in the same commit, otherwise > it is simply adding code and removing the same in following commit > doesn't suit for bisectable. > Sorry, I do not agree on this.. This patch clearly captures the _need_ to move ID table out of spi-nor-core.c. Code is being moved out of spi-nor-core.c to _support_ tiny stack. At no point in this series bisectablility is broken. Redoing patch 8/20 is non trivial and would be painful rework with no gain. I will have to rework patch 8/20 and 14/20 so as to not break compilation. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 16/20] mtd: spi: Add lightweight SPI flash stack for SPL
On Thu, Jan 31, 2019 at 11:20 PM Vignesh R wrote:
>
>
>
> On 31/01/19 5:36 PM, Jagan Teki wrote:
> > On Tue, Jan 29, 2019 at 11:29 AM Vignesh R wrote:
> >>
> >> Add a tiny SPI flash stack that just supports reading data/images from
> >> SPI flash. This is useful for boards that have SPL size constraints and
> >> would need to use SPI flash framework just to read images/data from
> >> flash. There is approximately 1.5 to 2KB savings with this.
> >>
> >> Based on prior work of reducing spi flash id table by
> >> Simon Goldschmidt
> >>
> >> Signed-off-by: Vignesh R
> >> Tested-by: Simon Goldschmidt
> >> Tested-by: Stefan Roese
> >> Tested-by: Horatiu Vultur
> >> ---
> >> common/spl/Kconfig | 11 +-
> >> drivers/mtd/spi/Makefile | 10 +-
> >> drivers/mtd/spi/sf_internal.h | 2 +
> >> drivers/mtd/spi/spi-nor-core.c | 266 +--
> >> drivers/mtd/spi/spi-nor-ids.c | 297
> >> drivers/mtd/spi/spi-nor-tiny.c | 810 +
> >> 6 files changed, 1132 insertions(+), 264 deletions(-)
> >> create mode 100644 drivers/mtd/spi/spi-nor-ids.c
> >> create mode 100644 drivers/mtd/spi/spi-nor-tiny.c
> >>
> >> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> >> index 2e1dd2705a62..416f5933b0d9 100644
> >> --- a/common/spl/Kconfig
> >> +++ b/common/spl/Kconfig
> >> @@ -732,9 +732,18 @@ config SPL_SPI_FLASH_SUPPORT
> >>
> >> if SPL_SPI_FLASH_SUPPORT
> >>
> >> +config SPL_SPI_FLASH_TINY
> >> + bool "Enable low footprint SPL SPI Flash support"
> >> + depends on !SPI_FLASH_BAR
> >> + help
> >> +Enable lightweight SPL SPI Flash support that supports just
> >> reading
> >> +data/images from flash. No support to write/erase flash. Enable
> >> +this if you have SPL size limitations and don't need full
> >> +fledged SPI flash support.
> >> +
> >> config SPL_SPI_FLASH_SFDP_SUPPORT
> >> bool "SFDP table parsing support for SPI NOR flashes"
> >> - depends on !SPI_FLASH_BAR
> >> + depends on !SPI_FLASH_BAR && !SPL_SPI_FLASH_TINY
> >> help
> >> Enable support for parsing and auto discovery of parameters for
> >> SPI NOR flashes using Serial Flash Discoverable Parameters (SFDP)
> >> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
> >> index 70058d3df2b9..f99f6cb16e29 100644
> >> --- a/drivers/mtd/spi/Makefile
> >> +++ b/drivers/mtd/spi/Makefile
> >> @@ -4,12 +4,20 @@
> >> # Wolfgang Denk, DENX Software Engineering, w...@denx.de.
> >>
> >> obj-$(CONFIG_DM_SPI_FLASH) += sf-uclass.o
> >> +spi-nor-y := sf_probe.o spi-nor-ids.o
> >>
> >> ifdef CONFIG_SPL_BUILD
> >> obj-$(CONFIG_SPL_SPI_BOOT) += fsl_espi_spl.o
> >> +ifeq ($(CONFIG_SPL_SPI_FLASH_TINY),y)
> >> +spi-nor-y += spi-nor-tiny.o
> >> +else
> >> +spi-nor-y += spi-nor-core.o
> >> +endif
> >> +else
> >> +spi-nor-y += spi-nor-core.o
> >> endif
> >>
> >> -obj-$(CONFIG_SPI_FLASH) += sf_probe.o spi-nor-core.o
> >> +obj-$(CONFIG_SPI_FLASH) += spi-nor.o
> >> obj-$(CONFIG_SPI_FLASH_DATAFLASH) += sf_dataflash.o sf.o
> >> obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
> >> obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
> >> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> >> index fd00e0d1b23b..a6bf734830a7 100644
> >> --- a/drivers/mtd/spi/sf_internal.h
> >> +++ b/drivers/mtd/spi/sf_internal.h
> >> @@ -16,7 +16,9 @@
> >> #define SPI_NOR_MAX_ADDR_WIDTH 4
> >>
> >> struct flash_info {
> >> +#if !CONFIG_IS_ENABLED(SPI_FLASH_TINY)
> >> char*name;
> >> +#endif
> >>
> >> /*
> >> * This array stores the ID bytes.
> >> diff --git a/drivers/mtd/spi/spi-nor-core.c
> >> b/drivers/mtd/spi/spi-nor-core.c
> >> index dbaaf45c7e1e..80633f8fd070 100644
> >> --- a/drivers/mtd/spi/spi-nor-core.c
> >> +++ b/drivers/mtd/spi/spi-nor-core.c
> >> @@ -879,284 +879,26 @@ static int stm_is_locked(struct spi_nor *nor,
> >> loff_t ofs, uint64_t len)
> >> }
> >> #endif /* CONFIG_SPI_FLASH_STMICRO */
> >>
> >> -/* Used when the "_ext_id" is two bytes at most */
> >> -#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \
> >> - .id = { \
> >> - ((_jedec_id) >> 16) & 0xff, \
> >> - ((_jedec_id) >> 8) & 0xff, \
> >> - (_jedec_id) & 0xff, \
> >> - ((_ext_id) >> 8) & 0xff,\
> >> - (_ext_id) & 0xff, \
> >> - }, \
> >> - .id_len = (!(_jedec_id) ? 0 : (3 + ((_ext_id) ? 2 : 0))),
> >> \
> >> - .sector_size = (_sector_size), \
> >> - .n_sectors = (_n_sectors), \
> >> - .page_size =
Re: [U-Boot] [PATCH v3 16/20] mtd: spi: Add lightweight SPI flash stack for SPL
On 31/01/19 5:36 PM, Jagan Teki wrote:
> On Tue, Jan 29, 2019 at 11:29 AM Vignesh R wrote:
>>
>> Add a tiny SPI flash stack that just supports reading data/images from
>> SPI flash. This is useful for boards that have SPL size constraints and
>> would need to use SPI flash framework just to read images/data from
>> flash. There is approximately 1.5 to 2KB savings with this.
>>
>> Based on prior work of reducing spi flash id table by
>> Simon Goldschmidt
>>
>> Signed-off-by: Vignesh R
>> Tested-by: Simon Goldschmidt
>> Tested-by: Stefan Roese
>> Tested-by: Horatiu Vultur
>> ---
>> common/spl/Kconfig | 11 +-
>> drivers/mtd/spi/Makefile | 10 +-
>> drivers/mtd/spi/sf_internal.h | 2 +
>> drivers/mtd/spi/spi-nor-core.c | 266 +--
>> drivers/mtd/spi/spi-nor-ids.c | 297
>> drivers/mtd/spi/spi-nor-tiny.c | 810 +
>> 6 files changed, 1132 insertions(+), 264 deletions(-)
>> create mode 100644 drivers/mtd/spi/spi-nor-ids.c
>> create mode 100644 drivers/mtd/spi/spi-nor-tiny.c
>>
>> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
>> index 2e1dd2705a62..416f5933b0d9 100644
>> --- a/common/spl/Kconfig
>> +++ b/common/spl/Kconfig
>> @@ -732,9 +732,18 @@ config SPL_SPI_FLASH_SUPPORT
>>
>> if SPL_SPI_FLASH_SUPPORT
>>
>> +config SPL_SPI_FLASH_TINY
>> + bool "Enable low footprint SPL SPI Flash support"
>> + depends on !SPI_FLASH_BAR
>> + help
>> +Enable lightweight SPL SPI Flash support that supports just reading
>> +data/images from flash. No support to write/erase flash. Enable
>> +this if you have SPL size limitations and don't need full
>> +fledged SPI flash support.
>> +
>> config SPL_SPI_FLASH_SFDP_SUPPORT
>> bool "SFDP table parsing support for SPI NOR flashes"
>> - depends on !SPI_FLASH_BAR
>> + depends on !SPI_FLASH_BAR && !SPL_SPI_FLASH_TINY
>> help
>> Enable support for parsing and auto discovery of parameters for
>> SPI NOR flashes using Serial Flash Discoverable Parameters (SFDP)
>> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
>> index 70058d3df2b9..f99f6cb16e29 100644
>> --- a/drivers/mtd/spi/Makefile
>> +++ b/drivers/mtd/spi/Makefile
>> @@ -4,12 +4,20 @@
>> # Wolfgang Denk, DENX Software Engineering, w...@denx.de.
>>
>> obj-$(CONFIG_DM_SPI_FLASH) += sf-uclass.o
>> +spi-nor-y := sf_probe.o spi-nor-ids.o
>>
>> ifdef CONFIG_SPL_BUILD
>> obj-$(CONFIG_SPL_SPI_BOOT) += fsl_espi_spl.o
>> +ifeq ($(CONFIG_SPL_SPI_FLASH_TINY),y)
>> +spi-nor-y += spi-nor-tiny.o
>> +else
>> +spi-nor-y += spi-nor-core.o
>> +endif
>> +else
>> +spi-nor-y += spi-nor-core.o
>> endif
>>
>> -obj-$(CONFIG_SPI_FLASH) += sf_probe.o spi-nor-core.o
>> +obj-$(CONFIG_SPI_FLASH) += spi-nor.o
>> obj-$(CONFIG_SPI_FLASH_DATAFLASH) += sf_dataflash.o sf.o
>> obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
>> obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
>> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
>> index fd00e0d1b23b..a6bf734830a7 100644
>> --- a/drivers/mtd/spi/sf_internal.h
>> +++ b/drivers/mtd/spi/sf_internal.h
>> @@ -16,7 +16,9 @@
>> #define SPI_NOR_MAX_ADDR_WIDTH 4
>>
>> struct flash_info {
>> +#if !CONFIG_IS_ENABLED(SPI_FLASH_TINY)
>> char*name;
>> +#endif
>>
>> /*
>> * This array stores the ID bytes.
>> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
>> index dbaaf45c7e1e..80633f8fd070 100644
>> --- a/drivers/mtd/spi/spi-nor-core.c
>> +++ b/drivers/mtd/spi/spi-nor-core.c
>> @@ -879,284 +879,26 @@ static int stm_is_locked(struct spi_nor *nor, loff_t
>> ofs, uint64_t len)
>> }
>> #endif /* CONFIG_SPI_FLASH_STMICRO */
>>
>> -/* Used when the "_ext_id" is two bytes at most */
>> -#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \
>> - .id = { \
>> - ((_jedec_id) >> 16) & 0xff, \
>> - ((_jedec_id) >> 8) & 0xff, \
>> - (_jedec_id) & 0xff, \
>> - ((_ext_id) >> 8) & 0xff,\
>> - (_ext_id) & 0xff, \
>> - }, \
>> - .id_len = (!(_jedec_id) ? 0 : (3 + ((_ext_id) ? 2 : 0))),
>>\
>> - .sector_size = (_sector_size), \
>> - .n_sectors = (_n_sectors), \
>> - .page_size = 256, \
>> - .flags = (_flags),
>> -
>> -#define INFO6(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)\
>> - .id = { \
>> - ((_jedec_id) >> 16) &
Re: [U-Boot] [PATCH v3 16/20] mtd: spi: Add lightweight SPI flash stack for SPL
On Tue, Jan 29, 2019 at 11:29 AM Vignesh R wrote:
>
> Add a tiny SPI flash stack that just supports reading data/images from
> SPI flash. This is useful for boards that have SPL size constraints and
> would need to use SPI flash framework just to read images/data from
> flash. There is approximately 1.5 to 2KB savings with this.
>
> Based on prior work of reducing spi flash id table by
> Simon Goldschmidt
>
> Signed-off-by: Vignesh R
> Tested-by: Simon Goldschmidt
> Tested-by: Stefan Roese
> Tested-by: Horatiu Vultur
> ---
> common/spl/Kconfig | 11 +-
> drivers/mtd/spi/Makefile | 10 +-
> drivers/mtd/spi/sf_internal.h | 2 +
> drivers/mtd/spi/spi-nor-core.c | 266 +--
> drivers/mtd/spi/spi-nor-ids.c | 297
> drivers/mtd/spi/spi-nor-tiny.c | 810 +
> 6 files changed, 1132 insertions(+), 264 deletions(-)
> create mode 100644 drivers/mtd/spi/spi-nor-ids.c
> create mode 100644 drivers/mtd/spi/spi-nor-tiny.c
>
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index 2e1dd2705a62..416f5933b0d9 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -732,9 +732,18 @@ config SPL_SPI_FLASH_SUPPORT
>
> if SPL_SPI_FLASH_SUPPORT
>
> +config SPL_SPI_FLASH_TINY
> + bool "Enable low footprint SPL SPI Flash support"
> + depends on !SPI_FLASH_BAR
> + help
> +Enable lightweight SPL SPI Flash support that supports just reading
> +data/images from flash. No support to write/erase flash. Enable
> +this if you have SPL size limitations and don't need full
> +fledged SPI flash support.
> +
> config SPL_SPI_FLASH_SFDP_SUPPORT
> bool "SFDP table parsing support for SPI NOR flashes"
> - depends on !SPI_FLASH_BAR
> + depends on !SPI_FLASH_BAR && !SPL_SPI_FLASH_TINY
> help
> Enable support for parsing and auto discovery of parameters for
> SPI NOR flashes using Serial Flash Discoverable Parameters (SFDP)
> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
> index 70058d3df2b9..f99f6cb16e29 100644
> --- a/drivers/mtd/spi/Makefile
> +++ b/drivers/mtd/spi/Makefile
> @@ -4,12 +4,20 @@
> # Wolfgang Denk, DENX Software Engineering, w...@denx.de.
>
> obj-$(CONFIG_DM_SPI_FLASH) += sf-uclass.o
> +spi-nor-y := sf_probe.o spi-nor-ids.o
>
> ifdef CONFIG_SPL_BUILD
> obj-$(CONFIG_SPL_SPI_BOOT) += fsl_espi_spl.o
> +ifeq ($(CONFIG_SPL_SPI_FLASH_TINY),y)
> +spi-nor-y += spi-nor-tiny.o
> +else
> +spi-nor-y += spi-nor-core.o
> +endif
> +else
> +spi-nor-y += spi-nor-core.o
> endif
>
> -obj-$(CONFIG_SPI_FLASH) += sf_probe.o spi-nor-core.o
> +obj-$(CONFIG_SPI_FLASH) += spi-nor.o
> obj-$(CONFIG_SPI_FLASH_DATAFLASH) += sf_dataflash.o sf.o
> obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
> obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index fd00e0d1b23b..a6bf734830a7 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -16,7 +16,9 @@
> #define SPI_NOR_MAX_ADDR_WIDTH 4
>
> struct flash_info {
> +#if !CONFIG_IS_ENABLED(SPI_FLASH_TINY)
> char*name;
> +#endif
>
> /*
> * This array stores the ID bytes.
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index dbaaf45c7e1e..80633f8fd070 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -879,284 +879,26 @@ static int stm_is_locked(struct spi_nor *nor, loff_t
> ofs, uint64_t len)
> }
> #endif /* CONFIG_SPI_FLASH_STMICRO */
>
> -/* Used when the "_ext_id" is two bytes at most */
> -#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \
> - .id = { \
> - ((_jedec_id) >> 16) & 0xff, \
> - ((_jedec_id) >> 8) & 0xff, \
> - (_jedec_id) & 0xff, \
> - ((_ext_id) >> 8) & 0xff,\
> - (_ext_id) & 0xff, \
> - }, \
> - .id_len = (!(_jedec_id) ? 0 : (3 + ((_ext_id) ? 2 : 0))),
> \
> - .sector_size = (_sector_size), \
> - .n_sectors = (_n_sectors), \
> - .page_size = 256, \
> - .flags = (_flags),
> -
> -#define INFO6(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)\
> - .id = { \
> - ((_jedec_id) >> 16) & 0xff, \
> - ((_jedec_id) >> 8) & 0xff, \
> - (_jedec_id) & 0xff,
