Re: [PATCH 0/2] mtd: hyperbus: Add support for Infineon S26Hx-T

[Tudor Ambarus <tudor.ambarus@xxxxxxxxxx>]

On 1/22/24 06:25, Raghavendra, Vignesh wrote:
> 
> 
> On 1/22/2024 11:41 AM, Tudor Ambarus wrote:
>> + Sergei, Geert, Mark & linux-spi
>>
>> Hi, Sergei,
>>
>> On 23.05.2023 07:22, Tudor Ambarus wrote:
>>> Hi, Takahiro, Vignesh,
>>>
>>>
>>> On 07.04.2023 09:11, tkuw584924@xxxxxxxxx wrote:
>>>> From: Takahiro Kuwano <Takahiro.Kuwano@xxxxxxxxxxxx>
>>>>
>>>> This sereis adds support for Infineon S26HL-T/S26HS-T flash family.
>>>> https://www.infineon.com/dgdl/Infineon-S26HS01GTGABHM020-DataSheet-v01_00-EN.pdf?fileId=8ac78c8c7f2a768a017f52f2f5182c91
>>>>
>>>> This family supports two interface modes, SPI mode and Hyperbus mode. The mode
>>>> can be switched at rutime. The default mode is selected by ordering option
>>>> and non-volatile user configuration. In hyperbus mode, the device is compatible
>>>> with S26KL-S/S26KS-S hyperflash family that supports hyperbus only so one of
>>>> use cases of S26Hx-T is replacement of (or migration from) S26Kx-S. This patch
>>>> set focuses on particular usage that the device is pre-configured as hyperbus
>>>> mode for compatibility with S26Kx-S.
>>>
>>> I'm questioning the overall hyperbus software architecture, not your
>>> patches per se. IMO hyperbus framework should have been written on top
>>> of SPIMEM and the controllers be placed in drivers/spi/. So I'd first
>>> address the SPIMEM adoption before adding/accepting new support. Would
>>> love to hear more from Vignesh.
>>>
>>
>> RPC is the only multi IO SPI controller that's upstreamed and capable of
>> dealing with hyperflashes, but there are others which are not upstreamed
>> yet (microchip).
>>
>> Struct ``struct rpcif_op`` [1] duplicates the contents of ``struct
>> spi_mem_op`` [2] which could have been avoided if hyperflash driver was
>> extended with SPI MEM support. This way the RPC hyperbus driver, which
>> is an SPI controller, could have been moved to drivers/spi.
>>
>> Sergei, do you remember why we haven't used SPI MEM for hyberbus since
>> the beginning? Was it something that we aimed for in a future patch set?
>>
>> Thanks,
>> ta
>>
>> [1]
>> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/tree/include/memory/renesas-rpc-if.h?h=mtd/for-6.8#n22
>>
>> [2]
>> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/tree/include/linux/spi/spi-mem.h?h=mtd/for-6.8#n99
>>
> 
> The initial hyperflash predates opening up of HyperBus protocol and
> inclusion of it in xSPI spec. First gen Flashes followed CFI specification
> and hence made sense to make use of cfi_cmdset_0002.c
> 
> We did have a discussion on extending spi_mem to support xSPI profile 
> 2.0 during the RPC_IF [3] [4].
> 
> Overall, both Controllers and Flashes have moved away from CFI parallel 
> flash protocol over to xSPI / SPI NOR flash protocol (profile 2.0), so I 
> agree with Tudor's assessment that we need to move towards spi_mem in 
> longer term. So
> 

Good, thanks Vignesh! I'll study a bit more and let you know about the
progress on this topic.

Cheers,
ta

> a) Extend spi_mem_op to support xSPI profile 2.0 transaction template
> b) HyperBus layer can then either be a adapter from CFI to spi_mem for CFI
> compliant devices. And  be subsumed completely within SPI NOR for SFDP
> compliant devices.
> c) Move the existing controllers over to new framework.
> 
> 
> [3] https://lore.kernel.org/all/b8224f46-fc2e-de35-0a90-a2a86cacb489@xxxxxx/
> [4] https://lore.kernel.org/all/20200220084927.7a411d40@xxxxxxxxxxxxx/
>