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

[Tudor Ambarus <tudor.ambarus@xxxxxxxxxx>]

On 2/1/24 12:23, Patrice CHOTARD wrote:
> 
> 
> On 2/1/24 13:02, Tudor Ambarus wrote:
>>
>>
>> On 2/1/24 10:46, Patrice CHOTARD wrote:
>>>
>>>
>>> On 1/22/24 09:13, Tudor Ambarus wrote:
>>>>
>>>>
>>>> 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.
>>>
>>> Hi All
>>
>> Hi,
>>
>>>
>>> At STMicroelectronics we got an OSPI block which is supporting both OSPI and HyperBus protocol
>>> similarly to the mentioned RPC-IF.
>>>
>>> This means that we intend to split our implementation in 3 drivers as RPC-IF:
>>>   _ backend driver including common source code to OSPI and HyperBus
>>>   _ OSPI frontend driver
>>>   _ HyperBus frontend driver
>>>
>>> Following this discussion thread, we are wondering if it will be the right direction to 
>>> choose in order to propose this implementation to MTD mailing list.
>>
>> I think the right direction would be to have hyperflash on top of
>> SPIMEM, and the SPI controller under drivers/spi.
> 
> 
> To be more precise, when you say "would be to have", you mean you will NAK 
> an implementation in 3 drivers as RPC-IF ?
> 

Yes. I think we should move towards SPIMEM adoption, don't you?

> 
>>>
>>> Have you an idea about time scale regarding the HyperBus migration over spi-mem ?
>>>
>>
>> No, sorry. It's at best effort from my side. Happy to review proposals
>> though.
>>
>>> Thanks
>>> Patrice
>>>
>>>
>>>>
>>>> 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/
>>>>>
>>>>
>>>> ______________________________________________________
>>>> Linux MTD discussion mailing list
>>>> http://lists.infradead.org/mailman/listinfo/linux-mtd/