[U-Boot] [PATCH 3/3] mtd: spi: Clean up usage of CONFIG_SPI_FLASH_MTD

Jagan Teki jagan at amarulasolutions.com
Wed Oct 23 19:26:13 UTC 2019


On Wed, Oct 23, 2019 at 12:57 PM Stefan Roese <sr at denx.de> wrote:
>
> On 23.10.19 09:09, Jagan Teki wrote:
> > On Wed, Oct 23, 2019 at 12:35 PM Schrempf Frieder
> > <frieder.schrempf at kontron.de> wrote:
> >>
> >> Hi Jagan,
> >>
> >> On 22.10.19 20:16, Jagan Teki wrote:
> >>> On Sat, Sep 14, 2019 at 4:14 AM Schrempf Frieder
> >>> <frieder.schrempf at kontron.de> wrote:
> >>>>
> >>>> From: Frieder Schrempf <frieder.schrempf at kontron.de>
> >>>>
> >>>> Most boards currently use SPI_FLASH_MTD only in U-Boot proper, not in
> >>>> SPL. They often rely on hacks in the board header files to include
> >>>> this option conditionally. To be able to fix this, we previously
> >>>> introduced a separate option SPL_SPI_FLASH_MTD.
> >>>>
> >>>> Therefore we can now adjust the Makefile and change the code in
> >>>> sf_probe.c and sf_internal.h to use CONFIG_IS_ENABLED(SPI_FLASH_MTD).
> >>>>
> >>>> We also need to move all occurences of CONFIG_SPI_FLASH_MTD from the
> >>>> header files to the according defconfigs. The affected boards are
> >>>> socfpga, aristainetos, cm_fx6, display5, ventana, rcar-gen2, dh_imx6
> >>>> and da850evm.
> >>>>
> >>>> We do this all in one patch to guarantee bisectibility.
> >>>>
> >>>> This change was tested with buildman to make sure it does not
> >>>> introduce any regressions by comparing the resulting binary sizes.
> >>>>
> >>>> Signed-off-by: Frieder Schrempf <frieder.schrempf at kontron.de>
> >>>> ---
> >>>>    configs/aristainetos2_defconfig        |  1 +
> >>>>    configs/aristainetos2b_defconfig       |  1 +
> >>>>    configs/aristainetos_defconfig         |  1 +
> >>>>    configs/cm_fx6_defconfig               |  1 +
> >>>>    configs/display5_defconfig             |  1 +
> >>>>    configs/display5_factory_defconfig     |  1 +
> >>>>    configs/socfpga_arria5_defconfig       |  1 +
> >>>>    configs/socfpga_cyclone5_defconfig     |  1 +
> >>>>    configs/socfpga_dbm_soc1_defconfig     |  1 +
> >>>>    configs/socfpga_de0_nano_soc_defconfig |  1 +
> >>>>    configs/socfpga_de10_nano_defconfig    |  1 +
> >>>>    configs/socfpga_is1_defconfig          |  1 +
> >>>>    configs/socfpga_mcvevk_defconfig       |  1 +
> >>>>    configs/socfpga_sockit_defconfig       |  1 +
> >>>>    configs/socfpga_socrates_defconfig     |  1 +
> >>>>    configs/socfpga_sr1500_defconfig       |  1 +
> >>>>    configs/socfpga_vining_fpga_defconfig  |  1 +
> >>>>    drivers/mtd/spi/Makefile               |  2 +-
> >>>>    drivers/mtd/spi/sf_internal.h          |  2 +-
> >>>>    drivers/mtd/spi/sf_probe.c             |  6 +++---
> >>>>    include/configs/aristainetos-common.h  |  1 -
> >>>>    include/configs/cm_fx6.h               |  7 -------
> >>>>    include/configs/da850evm.h             |  7 +------
> >>>>    include/configs/dh_imx6.h              |  1 -
> >>>>    include/configs/display5.h             |  4 ----
> >>>>    include/configs/gw_ventana.h           | 10 +---------
> >>>>    include/configs/rcar-gen2-common.h     |  4 +---
> >>>>    include/configs/socfpga_common.h       |  4 ----
> >>>>    28 files changed, 25 insertions(+), 40 deletions(-)
> >>>>
> >>>> diff --git a/configs/aristainetos2_defconfig b/configs/aristainetos2_defconfig
> >>>> index 18ef5d2dce..0bfc117762 100644
> >>>> --- a/configs/aristainetos2_defconfig
> >>>> +++ b/configs/aristainetos2_defconfig
> >>>> @@ -44,6 +44,7 @@ CONFIG_SF_DEFAULT_CS=1
> >>>>    CONFIG_SF_DEFAULT_MODE=0
> >>>>    CONFIG_SF_DEFAULT_SPEED=20000000
> >>>>    CONFIG_SPI_FLASH_STMICRO=y
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_MTD_UBI_FASTMAP=y
> >>>>    CONFIG_MTD_UBI_FASTMAP_AUTOCONVERT=1
> >>>>    CONFIG_PHYLIB=y
> >>>> diff --git a/configs/aristainetos2b_defconfig b/configs/aristainetos2b_defconfig
> >>>> index 1054c05d8c..e2da747a8f 100644
> >>>> --- a/configs/aristainetos2b_defconfig
> >>>> +++ b/configs/aristainetos2b_defconfig
> >>>> @@ -42,6 +42,7 @@ CONFIG_SPI_FLASH=y
> >>>>    CONFIG_SF_DEFAULT_MODE=0
> >>>>    CONFIG_SF_DEFAULT_SPEED=20000000
> >>>>    CONFIG_SPI_FLASH_STMICRO=y
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_MTD_UBI_FASTMAP=y
> >>>>    CONFIG_MTD_UBI_FASTMAP_AUTOCONVERT=1
> >>>>    CONFIG_PHYLIB=y
> >>>> diff --git a/configs/aristainetos_defconfig b/configs/aristainetos_defconfig
> >>>> index 4080a7b310..5caf95c22f 100644
> >>>> --- a/configs/aristainetos_defconfig
> >>>> +++ b/configs/aristainetos_defconfig
> >>>> @@ -43,6 +43,7 @@ CONFIG_SF_DEFAULT_BUS=3
> >>>>    CONFIG_SF_DEFAULT_MODE=0
> >>>>    CONFIG_SF_DEFAULT_SPEED=20000000
> >>>>    CONFIG_SPI_FLASH_STMICRO=y
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_MTD_UBI_FASTMAP=y
> >>>>    CONFIG_MTD_UBI_FASTMAP_AUTOCONVERT=1
> >>>>    CONFIG_PHYLIB=y
> >>>> diff --git a/configs/cm_fx6_defconfig b/configs/cm_fx6_defconfig
> >>>> index fd0db4db5c..15be7db027 100644
> >>>> --- a/configs/cm_fx6_defconfig
> >>>> +++ b/configs/cm_fx6_defconfig
> >>>> @@ -72,6 +72,7 @@ CONFIG_SPI_FLASH_SPANSION=y
> >>>>    CONFIG_SPI_FLASH_STMICRO=y
> >>>>    CONFIG_SPI_FLASH_SST=y
> >>>>    CONFIG_SPI_FLASH_WINBOND=y
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_PHYLIB=y
> >>>>    CONFIG_MII=y
> >>>>    CONFIG_DM_PMIC=y
> >>>> diff --git a/configs/display5_defconfig b/configs/display5_defconfig
> >>>> index 8609cd5a8c..5a4cc772be 100644
> >>>> --- a/configs/display5_defconfig
> >>>> +++ b/configs/display5_defconfig
> >>>> @@ -75,6 +75,7 @@ CONFIG_SF_DEFAULT_MODE=0
> >>>>    CONFIG_SF_DEFAULT_SPEED=50000000
> >>>>    CONFIG_SPI_FLASH_SPANSION=y
> >>>>    CONFIG_SPI_FLASH_STMICRO=y
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_PHYLIB=y
> >>>>    CONFIG_PHY_MARVELL=y
> >>>>    CONFIG_FEC_MXC=y
> >>>> diff --git a/configs/display5_factory_defconfig b/configs/display5_factory_defconfig
> >>>> index 70c64260d8..66c68e5ea9 100644
> >>>> --- a/configs/display5_factory_defconfig
> >>>> +++ b/configs/display5_factory_defconfig
> >>>> @@ -74,6 +74,7 @@ CONFIG_SF_DEFAULT_MODE=0
> >>>>    CONFIG_SF_DEFAULT_SPEED=50000000
> >>>>    CONFIG_SPI_FLASH_SPANSION=y
> >>>>    CONFIG_SPI_FLASH_STMICRO=y
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_PHYLIB=y
> >>>>    CONFIG_FEC_MXC=y
> >>>>    CONFIG_MII=y
> >>>> diff --git a/configs/socfpga_arria5_defconfig b/configs/socfpga_arria5_defconfig
> >>>> index 89e5ff8c71..30c2d19941 100644
> >>>> --- a/configs/socfpga_arria5_defconfig
> >>>> +++ b/configs/socfpga_arria5_defconfig
> >>>> @@ -47,6 +47,7 @@ CONFIG_SPI_FLASH=y
> >>>>    CONFIG_SPI_FLASH_SPANSION=y
> >>>>    CONFIG_SPI_FLASH_STMICRO=y
> >>>>    # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_PHY_MICREL=y
> >>>>    CONFIG_PHY_MICREL_KSZ90X1=y
> >>>>    CONFIG_DM_ETH=y
> >>>> diff --git a/configs/socfpga_cyclone5_defconfig b/configs/socfpga_cyclone5_defconfig
> >>>> index 00f2104276..dfe011b959 100644
> >>>> --- a/configs/socfpga_cyclone5_defconfig
> >>>> +++ b/configs/socfpga_cyclone5_defconfig
> >>>> @@ -48,6 +48,7 @@ CONFIG_SPI_FLASH_MACRONIX=y
> >>>>    CONFIG_SPI_FLASH_SPANSION=y
> >>>>    CONFIG_SPI_FLASH_STMICRO=y
> >>>>    # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_PHY_MICREL=y
> >>>>    CONFIG_PHY_MICREL_KSZ90X1=y
> >>>>    CONFIG_DM_ETH=y
> >>>> diff --git a/configs/socfpga_dbm_soc1_defconfig b/configs/socfpga_dbm_soc1_defconfig
> >>>> index 1877010a19..ee693f3def 100644
> >>>> --- a/configs/socfpga_dbm_soc1_defconfig
> >>>> +++ b/configs/socfpga_dbm_soc1_defconfig
> >>>> @@ -46,6 +46,7 @@ CONFIG_SYS_I2C_DW=y
> >>>>    CONFIG_DM_MMC=y
> >>>>    CONFIG_MMC_DW=y
> >>>>    CONFIG_MTD_DEVICE=y
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_DM_ETH=y
> >>>>    CONFIG_PHY_GIGE=y
> >>>>    CONFIG_ETH_DESIGNWARE=y
> >>>> diff --git a/configs/socfpga_de0_nano_soc_defconfig b/configs/socfpga_de0_nano_soc_defconfig
> >>>> index de50f17174..e91d6f62f8 100644
> >>>> --- a/configs/socfpga_de0_nano_soc_defconfig
> >>>> +++ b/configs/socfpga_de0_nano_soc_defconfig
> >>>> @@ -43,6 +43,7 @@ CONFIG_SYS_I2C_DW=y
> >>>>    CONFIG_DM_MMC=y
> >>>>    CONFIG_MMC_DW=y
> >>>>    CONFIG_MTD_DEVICE=y
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_PHY_MICREL=y
> >>>>    CONFIG_PHY_MICREL_KSZ90X1=y
> >>>>    CONFIG_DM_ETH=y
> >>>> diff --git a/configs/socfpga_de10_nano_defconfig b/configs/socfpga_de10_nano_defconfig
> >>>> index 03961195ac..ffe9d6c10c 100644
> >>>> --- a/configs/socfpga_de10_nano_defconfig
> >>>> +++ b/configs/socfpga_de10_nano_defconfig
> >>>> @@ -39,6 +39,7 @@ CONFIG_SYS_I2C_DW=y
> >>>>    CONFIG_DM_MMC=y
> >>>>    CONFIG_MMC_DW=y
> >>>>    CONFIG_MTD_DEVICE=y
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_PHY_MICREL=y
> >>>>    CONFIG_PHY_MICREL_KSZ90X1=y
> >>>>    CONFIG_DM_ETH=y
> >>>> diff --git a/configs/socfpga_is1_defconfig b/configs/socfpga_is1_defconfig
> >>>> index 6ea06c1104..76ab87250b 100644
> >>>> --- a/configs/socfpga_is1_defconfig
> >>>> +++ b/configs/socfpga_is1_defconfig
> >>>> @@ -43,6 +43,7 @@ CONFIG_SYS_I2C_DW=y
> >>>>    CONFIG_MTD_DEVICE=y
> >>>>    CONFIG_SPI_FLASH=y
> >>>>    CONFIG_SPI_FLASH_STMICRO=y
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_PHY_MICREL=y
> >>>>    CONFIG_PHY_MICREL_KSZ90X1=y
> >>>>    CONFIG_DM_ETH=y
> >>>> diff --git a/configs/socfpga_mcvevk_defconfig b/configs/socfpga_mcvevk_defconfig
> >>>> index 161bd6fca3..4d3caaa8ad 100644
> >>>> --- a/configs/socfpga_mcvevk_defconfig
> >>>> +++ b/configs/socfpga_mcvevk_defconfig
> >>>> @@ -42,6 +42,7 @@ CONFIG_DM_I2C=y
> >>>>    CONFIG_SYS_I2C_DW=y
> >>>>    CONFIG_DM_MMC=y
> >>>>    CONFIG_MMC_DW=y
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_DM_ETH=y
> >>>>    CONFIG_PHY_GIGE=y
> >>>>    CONFIG_ETH_DESIGNWARE=y
> >>>> diff --git a/configs/socfpga_sockit_defconfig b/configs/socfpga_sockit_defconfig
> >>>> index 8ec1c05571..0ffcfda21e 100644
> >>>> --- a/configs/socfpga_sockit_defconfig
> >>>> +++ b/configs/socfpga_sockit_defconfig
> >>>> @@ -48,6 +48,7 @@ CONFIG_SPI_FLASH_MACRONIX=y
> >>>>    CONFIG_SPI_FLASH_SPANSION=y
> >>>>    CONFIG_SPI_FLASH_STMICRO=y
> >>>>    # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_PHY_MICREL=y
> >>>>    CONFIG_PHY_MICREL_KSZ90X1=y
> >>>>    CONFIG_DM_ETH=y
> >>>> diff --git a/configs/socfpga_socrates_defconfig b/configs/socfpga_socrates_defconfig
> >>>> index 15f81d1a4b..193af0b0df 100644
> >>>> --- a/configs/socfpga_socrates_defconfig
> >>>> +++ b/configs/socfpga_socrates_defconfig
> >>>> @@ -48,6 +48,7 @@ CONFIG_SPI_FLASH=y
> >>>>    CONFIG_SPI_FLASH_MACRONIX=y
> >>>>    CONFIG_SPI_FLASH_SPANSION=y
> >>>>    CONFIG_SPI_FLASH_STMICRO=y
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_PHY_MICREL=y
> >>>>    CONFIG_PHY_MICREL_KSZ90X1=y
> >>>>    CONFIG_DM_ETH=y
> >>>> diff --git a/configs/socfpga_sr1500_defconfig b/configs/socfpga_sr1500_defconfig
> >>>> index 941bf1124a..899dd8396b 100644
> >>>> --- a/configs/socfpga_sr1500_defconfig
> >>>> +++ b/configs/socfpga_sr1500_defconfig
> >>>> @@ -49,6 +49,7 @@ CONFIG_SPI_FLASH=y
> >>>>    CONFIG_SF_DEFAULT_SPEED=100000000
> >>>>    CONFIG_SPI_FLASH_STMICRO=y
> >>>>    # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_PHY_MARVELL=y
> >>>>    CONFIG_DM_ETH=y
> >>>>    CONFIG_PHY_GIGE=y
> >>>> diff --git a/configs/socfpga_vining_fpga_defconfig b/configs/socfpga_vining_fpga_defconfig
> >>>> index 96f806ab5f..f96f536169 100644
> >>>> --- a/configs/socfpga_vining_fpga_defconfig
> >>>> +++ b/configs/socfpga_vining_fpga_defconfig
> >>>> @@ -72,6 +72,7 @@ CONFIG_SPI_FLASH=y
> >>>>    CONFIG_SPI_FLASH_SPANSION=y
> >>>>    CONFIG_SPI_FLASH_STMICRO=y
> >>>>    # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
> >>>> +CONFIG_SPI_FLASH_MTD=y
> >>>>    CONFIG_MTD_UBI_FASTMAP=y
> >>>>    CONFIG_PHY_MICREL=y
> >>>>    CONFIG_PHY_MICREL_KSZ90X1=y
> >>>> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
> >>>> index f99f6cb16e..fb67ba32c6 100644
> >>>> --- a/drivers/mtd/spi/Makefile
> >>>> +++ b/drivers/mtd/spi/Makefile
> >>>> @@ -19,5 +19,5 @@ endif
> >>>>
> >>>>    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_$(SPL_)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 a6bf734830..8542b03685 100644
> >>>> --- a/drivers/mtd/spi/sf_internal.h
> >>>> +++ b/drivers/mtd/spi/sf_internal.h
> >>>> @@ -94,7 +94,7 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
> >>>>    int spi_flash_cmd_get_sw_write_prot(struct spi_flash *flash);
> >>>>
> >>>>
> >>>> -#ifdef CONFIG_SPI_FLASH_MTD
> >>>> +#if CONFIG_IS_ENABLED(SPI_FLASH_MTD)
> >>>>    int spi_flash_mtd_register(struct spi_flash *flash);
> >>>>    void spi_flash_mtd_unregister(void);
> >>>>    #endif
> >>>> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> >>>> index 73297e1a0a..f051e473ff 100644
> >>>> --- a/drivers/mtd/spi/sf_probe.c
> >>>> +++ b/drivers/mtd/spi/sf_probe.c
> >>>> @@ -44,7 +44,7 @@ static int spi_flash_probe_slave(struct spi_flash *flash)
> >>>>           if (ret)
> >>>>                   goto err_read_id;
> >>>>
> >>>> -#ifdef CONFIG_SPI_FLASH_MTD
> >>>> +#if CONFIG_IS_ENABLED(SPI_FLASH_MTD)
> >>>>           ret = spi_flash_mtd_register(flash);
> >>>>    #endif
> >>>>
> >>>> @@ -83,7 +83,7 @@ struct spi_flash *spi_flash_probe(unsigned int busnum, unsigned int cs,
> >>>>
> >>>>    void spi_flash_free(struct spi_flash *flash)
> >>>>    {
> >>>> -#ifdef CONFIG_SPI_FLASH_MTD
> >>>> +#if CONFIG_IS_ENABLED(SPI_FLASH_MTD)
> >>>>           spi_flash_mtd_unregister();
> >>>>    #endif
> >>>>           spi_free_slave(flash->spi);
> >>>> @@ -152,7 +152,7 @@ static int spi_flash_std_probe(struct udevice *dev)
> >>>>
> >>>>    static int spi_flash_std_remove(struct udevice *dev)
> >>>>    {
> >>>> -#ifdef CONFIG_SPI_FLASH_MTD
> >>>> +#if CONFIG_IS_ENABLED(SPI_FLASH_MTD)
> >>>
> >>> These ifdef changes look unrelated wrt actual patch. any comments?
> >>
> >> No, they are not unrelated. This is the actual reason for having this
> >> patch after all. We want to be able to enable/disable the code for
> >> CONFIG_SPI_FLASH_MTD per build target (SPL, U-Boot proper).
> >> These changes need to be done in a single patch together with the
> >> defconfig and Makefile changes to sustain bisectibility.
> >
> > What I'm trying to say here is, #ifdef CONFIG_SPI_FLASH_MTD and #if
> > CONFIG_IS_ENABLED(SPI_FLASH_MTD) is the same
>
> This is not correct. CONFIG_IS_ENABLED(SPI_FLASH_MTD) also handles
> CONFIG_SPL_SPI_FLASH_MTD & CONFIG_TPL_SPI_FLASH_MTD.

Got it Stefan, thanks.


More information about the U-Boot mailing list