| Message ID | 20191021033913.220758-70-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Bin Meng |
| Headers | show |
| Series | x86: Add initial support for apollolake | expand |
+Vignesh On Mon, Oct 21, 2019 at 11:40 AM Simon Glass <sjg@chromium.org> wrote: > > We don't normally need this on x86 unless the size of SPI flash devices is > larger than 16MB. This can be enabled by particular SoCs as needed, since > it adds to code size. > > Drop the default enabling of this option on x86. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > Changes in v3: None > Changes in v2: None > > drivers/spi/Kconfig | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig > index b8ca2bdedd5..320baeeb3eb 100644 > --- a/drivers/spi/Kconfig > +++ b/drivers/spi/Kconfig > @@ -125,7 +125,6 @@ config FSL_DSPI > > config ICH_SPI > bool "Intel ICH SPI driver" > - imply SPI_FLASH_BAR This was added via: commit 6d82517836418f984b7b4c05cf1427d7b49b1169 Author: Vignesh R <vigneshr@ti.com> Date: Tue Feb 5 11:29:28 2019 +0530 configs: Don't use SPI_FLASH_BAR as default Now that new SPI NOR layer uses stateless 4 byte opcodes by default, don't enable SPI_FLASH_BAR. For SPI controllers that cannot support 4-byte addressing, (stm32_qspi.c, fsl_qspi.c, mtk_qspi.c, ich.c, renesas_rpc_spi.c) add an imply clause to enable SPI_FLASH_BAR so as to not break functionality. The commit message says: SPI_FLASH_BAR was added so as to not break functionality. I don't have board to test right now. Vignesh could you please have a review? Thanks! > help > Enable the Intel ICH SPI driver. This driver can be used to > access the SPI NOR flash on platforms embedding this Intel > -- Regards, Bin
Hi, On 19/11/19 7:03 PM, Bin Meng wrote: > +Vignesh > > On Mon, Oct 21, 2019 at 11:40 AM Simon Glass <sjg@chromium.org> wrote: >> >> We don't normally need this on x86 unless the size of SPI flash devices is >> larger than 16MB. This can be enabled by particular SoCs as needed, since >> it adds to code size. >> >> Drop the default enabling of this option on x86. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- >> >> Changes in v3: None >> Changes in v2: None >> >> drivers/spi/Kconfig | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig >> index b8ca2bdedd5..320baeeb3eb 100644 >> --- a/drivers/spi/Kconfig >> +++ b/drivers/spi/Kconfig >> @@ -125,7 +125,6 @@ config FSL_DSPI >> >> config ICH_SPI >> bool "Intel ICH SPI driver" >> - imply SPI_FLASH_BAR > > This was added via: > > commit 6d82517836418f984b7b4c05cf1427d7b49b1169 > Author: Vignesh R <vigneshr@ti.com> > Date: Tue Feb 5 11:29:28 2019 +0530 > > configs: Don't use SPI_FLASH_BAR as default > > Now that new SPI NOR layer uses stateless 4 byte opcodes by default, > don't enable SPI_FLASH_BAR. For SPI controllers that cannot support > 4-byte addressing, (stm32_qspi.c, fsl_qspi.c, mtk_qspi.c, ich.c, > renesas_rpc_spi.c) add an imply clause to enable SPI_FLASH_BAR so as to > not break functionality. > > The commit message says: SPI_FLASH_BAR was added so as to not break > functionality. > At the time of introducing new SPI NOR layer, I found that (by code inspection) ich.c did not support 4 byte addressing. Hence, "imply SPI_FLASH_BAR" was added for ich.c driver. But if driver has been improved to support 4 byte addressing or if there is no plan to support flashes > 16MiB by default, then it should be okay to remove SPI_FLASH_BAR. > I don't have board to test right now. Vignesh could you please have a > review? Thanks! > >> help >> Enable the Intel ICH SPI driver. This driver can be used to >> access the SPI NOR flash on platforms embedding this Intel >> -- > > Regards, > Bin >
Hi Vignesh, On Tue, 19 Nov 2019 at 22:21, Vignesh Raghavendra <vigneshr@ti.com> wrote: > > Hi, > > On 19/11/19 7:03 PM, Bin Meng wrote: > > +Vignesh > > > > On Mon, Oct 21, 2019 at 11:40 AM Simon Glass <sjg@chromium.org> wrote: > >> > >> We don't normally need this on x86 unless the size of SPI flash devices is > >> larger than 16MB. This can be enabled by particular SoCs as needed, since > >> it adds to code size. > >> > >> Drop the default enabling of this option on x86. > >> > >> Signed-off-by: Simon Glass <sjg@chromium.org> > >> --- > >> > >> Changes in v3: None > >> Changes in v2: None > >> > >> drivers/spi/Kconfig | 1 - > >> 1 file changed, 1 deletion(-) > >> > >> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig > >> index b8ca2bdedd5..320baeeb3eb 100644 > >> --- a/drivers/spi/Kconfig > >> +++ b/drivers/spi/Kconfig > >> @@ -125,7 +125,6 @@ config FSL_DSPI > >> > >> config ICH_SPI > >> bool "Intel ICH SPI driver" > >> - imply SPI_FLASH_BAR > > > > This was added via: > > > > commit 6d82517836418f984b7b4c05cf1427d7b49b1169 > > Author: Vignesh R <vigneshr@ti.com> > > Date: Tue Feb 5 11:29:28 2019 +0530 > > > > configs: Don't use SPI_FLASH_BAR as default > > > > Now that new SPI NOR layer uses stateless 4 byte opcodes by default, > > don't enable SPI_FLASH_BAR. For SPI controllers that cannot support > > 4-byte addressing, (stm32_qspi.c, fsl_qspi.c, mtk_qspi.c, ich.c, > > renesas_rpc_spi.c) add an imply clause to enable SPI_FLASH_BAR so as to > > not break functionality. > > > > The commit message says: SPI_FLASH_BAR was added so as to not break > > functionality. > > > > At the time of introducing new SPI NOR layer, I found that (by code > inspection) ich.c did not support 4 byte addressing. Hence, "imply > SPI_FLASH_BAR" was added for ich.c driver. > But if driver has been improved to support 4 byte addressing or if there > is no plan to support flashes > 16MiB by default, then it should be okay > to remove SPI_FLASH_BAR. Well the problem is that it adds almost 3KB to the code size in TPL, so I think we need the option to turn it off. Also current platforms supported in U-Boot don't have more than 8 or 16MB of SPI flash, so this should be safe. > > > I don't have board to test right now. Vignesh could you please have a > > review? Thanks! > > > >> help > >> Enable the Intel ICH SPI driver. This driver can be used to > >> access the SPI NOR flash on platforms embedding this Intel > >> -- Regards, Simon
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index b8ca2bdedd5..320baeeb3eb 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -125,7 +125,6 @@ config FSL_DSPI config ICH_SPI bool "Intel ICH SPI driver" - imply SPI_FLASH_BAR help Enable the Intel ICH SPI driver. This driver can be used to access the SPI NOR flash on platforms embedding this Intel
We don't normally need this on x86 unless the size of SPI flash devices is larger than 16MB. This can be enabled by particular SoCs as needed, since it adds to code size. Drop the default enabling of this option on x86. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v3: None Changes in v2: None drivers/spi/Kconfig | 1 - 1 file changed, 1 deletion(-)