[U-Boot] [PATCH] ARM: mvebu: db-88f6820-amc: enable SPI_FLASH_BAR

[U-Boot] [PATCH] ARM: mvebu: db-88f6820-amc: enable SPI_FLASH_BAR

[Chris Packham]

This board uses Micron N25Q256A SPI flash. Enable SPI_FLASH_BAR to allow
us to access the whole chip.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
Cc: Vignesh R <vigneshr@ti.com>
---

 configs/db-88f6820-amc_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/db-88f6820-amc_defconfig b/configs/db-88f6820-amc_defconfig
index f5d35229d11c..319d9dbd9c3f 100644
--- a/configs/db-88f6820-amc_defconfig
+++ b/configs/db-88f6820-amc_defconfig
@@ -53,6 +53,7 @@ CONFIG_SYS_I2C_MVTWSI=y
 CONFIG_NAND=y
 CONFIG_NAND_PXA3XX=y
 CONFIG_SPI_FLASH=y
+CONFIG_SPI_FLASH_BAR=y
 CONFIG_SPI_FLASH_MACRONIX=y
 CONFIG_SPI_FLASH_STMICRO=y
 CONFIG_PHY_MARVELL=y
-- 
2.21.0

[U-Boot] [PATCH] ARM: mvebu: db-88f6820-amc: enable SPI_FLASH_BAR

[Vignesh Raghavendra]

On 02-Mar-19 1:15 PM, Chris Packham wrote:
> This board uses Micron N25Q256A SPI flash. Enable SPI_FLASH_BAR to allow
> us to access the whole chip.
> 
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> Cc: Vignesh R <vigneshr@ti.com>
> ---
> 

Could add imply SPI_FLASH_BAR to KIRKWOOD_SPI instead? So that we don't
have to fix every defconfig that using kirkwood_spi.c.

Looks like kernel driver does support 3/4 byte addressing, but U-Boot
version of the driver doesn't.

>  configs/db-88f6820-amc_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configs/db-88f6820-amc_defconfig b/configs/db-88f6820-amc_defconfig
> index f5d35229d11c..319d9dbd9c3f 100644
> --- a/configs/db-88f6820-amc_defconfig
> +++ b/configs/db-88f6820-amc_defconfig
> @@ -53,6 +53,7 @@ CONFIG_SYS_I2C_MVTWSI=y
>  CONFIG_NAND=y
>  CONFIG_NAND_PXA3XX=y
>  CONFIG_SPI_FLASH=y
> +CONFIG_SPI_FLASH_BAR=y
>  CONFIG_SPI_FLASH_MACRONIX=y
>  CONFIG_SPI_FLASH_STMICRO=y
>  CONFIG_PHY_MARVELL=y
> 

[U-Boot] [PATCH] ARM: mvebu: db-88f6820-amc: enable SPI_FLASH_BAR

[Stefan Roese]

Hi Chris,
Hi Vignesh,

On 02.03.19 09:36, Vignesh Raghavendra wrote:
> 
> 
> On 02-Mar-19 1:15 PM, Chris Packham wrote:
>> This board uses Micron N25Q256A SPI flash. Enable SPI_FLASH_BAR to allow
>> us to access the whole chip.
>>
>> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>> Cc: Vignesh R <vigneshr@ti.com>
>> ---
>>
> 
> Could add imply SPI_FLASH_BAR to KIRKWOOD_SPI instead? So that we don't
> have to fix every defconfig that using kirkwood_spi.c.

Did you use BAR before the newly introduced SPI NOR restructuring?
If not, then I would rather oppose to using it now, since option
should only be used by those platforms that used it earlier as well.
BAR mode is not supported by Linux and an U-Boot special.
  
> Looks like kernel driver does support 3/4 byte addressing, but U-Boot
> version of the driver doesn't.

Could you please double check, if and how addressing of the whole
32MiB was done before? And perhaps its possible to enhance the SPI
driver to support 3 and 4 byte addressing instead.

Thanks,
Stefan

[U-Boot] [PATCH] ARM: mvebu: db-88f6820-amc: enable SPI_FLASH_BAR

[Vignesh Raghavendra]

Hi Stefan,

On 02-Mar-19 3:56 PM, Stefan Roese wrote:
> Hi Chris,
> Hi Vignesh,
> 
> On 02.03.19 09:36, Vignesh Raghavendra wrote:
>>
>>
>> On 02-Mar-19 1:15 PM, Chris Packham wrote:
>>> This board uses Micron N25Q256A SPI flash. Enable SPI_FLASH_BAR to allow
>>> us to access the whole chip.
>>>
>>> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>>> Cc: Vignesh R <vigneshr@ti.com>
>>> ---
>>>
>>
>> Could add imply SPI_FLASH_BAR to KIRKWOOD_SPI instead? So that we don't
>> have to fix every defconfig that using kirkwood_spi.c.
> 
> Did you use BAR before the newly introduced SPI NOR restructuring?
> If not, then I would rather oppose to using it now, since option
> should only be used by those platforms that used it earlier as well.

configs/db-88f6820-amc_defconfig did use SPI_FLASH_BAR before SPI NOR
re-work

> BAR mode is not supported by Linux and an U-Boot special.
>   
>> Looks like kernel driver does support 3/4 byte addressing, but U-Boot
>> version of the driver doesn't.
> 
> Could you please double check, if and how addressing of the whole
> 32MiB was done before? And perhaps its possible to enhance the SPI
> driver to support 3 and 4 byte addressing instead.
> 

It would be great if driver could be modified to support 3 and 4 byte
addressing modes.

[U-Boot] [PATCH] ARM: mvebu: db-88f6820-amc: enable SPI_FLASH_BAR

[Stefan Roese]

Hi Vignesh,

On 02.03.19 11:50, Vignesh Raghavendra wrote:
> Hi Stefan,
> 
> On 02-Mar-19 3:56 PM, Stefan Roese wrote:
>> Hi Chris,
>> Hi Vignesh,
>>
>> On 02.03.19 09:36, Vignesh Raghavendra wrote:
>>>
>>>
>>> On 02-Mar-19 1:15 PM, Chris Packham wrote:
>>>> This board uses Micron N25Q256A SPI flash. Enable SPI_FLASH_BAR to allow
>>>> us to access the whole chip.
>>>>
>>>> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>>>> Cc: Vignesh R <vigneshr@ti.com>
>>>> ---
>>>>
>>>
>>> Could add imply SPI_FLASH_BAR to KIRKWOOD_SPI instead? So that we don't
>>> have to fix every defconfig that using kirkwood_spi.c.
>>
>> Did you use BAR before the newly introduced SPI NOR restructuring?
>> If not, then I would rather oppose to using it now, since option
>> should only be used by those platforms that used it earlier as well.
> 
> configs/db-88f6820-amc_defconfig did use SPI_FLASH_BAR before SPI NOR
> re-work

I see. Thanks for checking.
  
>> BAR mode is not supported by Linux and an U-Boot special.
>>    
>>> Looks like kernel driver does support 3/4 byte addressing, but U-Boot
>>> version of the driver doesn't.
>>
>> Could you please double check, if and how addressing of the whole
>> 32MiB was done before? And perhaps its possible to enhance the SPI
>> driver to support 3 and 4 byte addressing instead.
>>
> 
> It would be great if driver could be modified to support 3 and 4 byte
> addressing modes.

I fail to see, what really is needed to make the SPI driver support
3 and 4 byte addressing modes. I recently added the MT7688 SPI
driver (mt7621_spi.c) which supports 3 and 4 byte mode and I remember
no driver specials that I needed to implement for this feature. Its
address mode agnostic AFAICT and just xfers a buffer of data to and
from the SPI device.

Could you please let me know, what exactly is missing for this 4 byte
mode?

Thanks,
Stefan

[U-Boot] [PATCH] ARM: mvebu: db-88f6820-amc: enable SPI_FLASH_BAR

[Vignesh Raghavendra]

On 02-Mar-19 5:02 PM, Stefan Roese wrote:
[...]
>>
>> It would be great if driver could be modified to support 3 and 4 byte
>> addressing modes.
> 
> I fail to see, what really is needed to make the SPI driver support
> 3 and 4 byte addressing modes. I recently added the MT7688 SPI
> driver (mt7621_spi.c) which supports 3 and 4 byte mode and I remember
> no driver specials that I needed to implement for this feature. Its
> address mode agnostic AFAICT and just xfers a buffer of data to and
> from the SPI device.
> 

Certain SPI controllers have special HW support to support SPI flashes
and therefore need knowledge of flash specific information such as
opcode, no. of address bytes etc. More details in this blog[1] Such
controllers should ideally use spi-mem interface like [2]
I see this line in kirkwood_spi.c[3] which seems to hard code address
bytes to 3? Can't really be sure w/o looking at the datasheet of SPI
controllers.

[1]
https://bootlin.com/blog/spi-mem-bringing-some-consistency-to-the-spi-memory-ecosystem/
[2] https://patchwork.ozlabs.org/patch/1039718/
[3]
https://elixir.bootlin.com/u-boot/latest/source/drivers/spi/kirkwood_spi.c#L132

> Could you please let me know, what exactly is missing for this 4 byte
> mode?
> 
> Thanks,
> Stefan
> 

[U-Boot] [PATCH] ARM: mvebu: db-88f6820-amc: enable SPI_FLASH_BAR

[Stefan Roese]

On 02.03.19 12:53, Vignesh Raghavendra wrote:
> 
> 
> On 02-Mar-19 5:02 PM, Stefan Roese wrote:
> [...]
>>>
>>> It would be great if driver could be modified to support 3 and 4 byte
>>> addressing modes.
>>
>> I fail to see, what really is needed to make the SPI driver support
>> 3 and 4 byte addressing modes. I recently added the MT7688 SPI
>> driver (mt7621_spi.c) which supports 3 and 4 byte mode and I remember
>> no driver specials that I needed to implement for this feature. Its
>> address mode agnostic AFAICT and just xfers a buffer of data to and
>> from the SPI device.
>>
> 
> Certain SPI controllers have special HW support to support SPI flashes
> and therefore need knowledge of flash specific information such as
> opcode, no. of address bytes etc. More details in this blog[1] Such
> controllers should ideally use spi-mem interface like [2]
> I see this line in kirkwood_spi.c[3] which seems to hard code address
> bytes to 3? Can't really be sure w/o looking at the datasheet of SPI
> controllers.

I remember working on the Marvell SPI controller for Armada XP a few
years ago. This 3BYTE mode is only for the special "direct mode", that
is also supported by this controller. AFAIR, we don't use this direct
mode in U-Boot at all. So setting this register to 3BYTE mode should be
a "no-op" right now.
  
> [1]
> https://bootlin.com/blog/spi-mem-bringing-some-consistency-to-the-spi-memory-ecosystem/
> [2] https://patchwork.ozlabs.org/patch/1039718/
> [3]
> https://elixir.bootlin.com/u-boot/latest/source/drivers/spi/kirkwood_spi.c#L132
> 
>> Could you please let me know, what exactly is missing for this 4 byte
>> mode?
>>
>> Thanks,
>> Stefan
>>

Thanks,
Stefan

[U-Boot] [PATCH] ARM: mvebu: db-88f6820-amc: enable SPI_FLASH_BAR

[Stefan Roese]

On 02.03.19 08:45, Chris Packham wrote:
> This board uses Micron N25Q256A SPI flash. Enable SPI_FLASH_BAR to allow
> us to access the whole chip.
> 
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> Cc: Vignesh R <vigneshr@ti.com>

Applied to u-boot-marvell/master.

Thanks,
Stefan