mbox series

[0/5] Implement setup_inteface() in the DaVinci NAND controller

Message ID 20241030104717.88688-1-bastien.curutchet@bootlin.com
Headers show
Series Implement setup_inteface() in the DaVinci NAND controller | expand

Message

Bastien Curutchet Oct. 30, 2024, 10:47 a.m. UTC 
Hi all,

This patch series aims to implement the setup_interface() operation in
the DaVinci NAND controller to enable the use of all ONFI modes and
improve the NAND access speed.

This NAND controller is present in the DaVinci (OMAP L138) and Keystone2
SoCs and functions as a 'child' of the AEMIF controller. So its timings
are set by the AEMIF controller itself from device-tree properties.
Implementing the setup_interface() callback implies being able to update
dynamically these timings, so the first two patches of the series modify
the AEMIF driver to provide its 'children' a way to modify their chip
select timing configuration. To do so, I add a ti-aemif.h header, I'm not
sure whether this header should be located in include/memory or in
include/linux/memory. I put it in include/memory because the folder
already exists while include/linux/memory doesn't.

The remaining patches implement the setup_interface() operation.
The computation of the register's contents is directly based on
ยง20.3.2.3 of the OMAP-L138 TRM [1]

This has been tested on two platforms based upon the DaVinci SoC. One is
interfaced with a Macronix MX30UF4G18AC NAND, the other with a Toshiba
NAND.

[1] : https://www.ti.com/lit/ug/spruh77c/spruh77c.pdf

Bastien Curutchet (5):
  memory: ti-aemif: Create aemif_set_cs_timings()
  memory: ti-aemif: export aemif_set_cs_timing()
  mtd: rawnand: davinci: Order headers alphabetically
  mtd: rawnand: davinci: Add clock resource
  mtd: rawnand: davinci: Implement setup_interface() operation

 drivers/memory/ti-aemif.c           | 102 +++++++++++++++++++---------
 drivers/mtd/nand/raw/davinci_nand.c |  95 ++++++++++++++++++++++++--
 include/memory/ti-aemif.h           |  31 +++++++++
 3 files changed, 191 insertions(+), 37 deletions(-)
 create mode 100644 include/memory/ti-aemif.h

Comments

Krzysztof Kozlowski Oct. 30, 2024, 11:17 a.m. UTC | #1 
On 30/10/2024 11:47, Bastien Curutchet wrote:
> Hi all,
> 
> This patch series aims to implement the setup_interface() operation in
> the DaVinci NAND controller to enable the use of all ONFI modes and
> improve the NAND access speed.
> 

Your changelog is supposed to explain also merging dependencies. Within
patchset or external.

> This NAND controller is present in the DaVinci (OMAP L138) and Keystone2
> SoCs and functions as a 'child' of the AEMIF controller. So its timings
> are set by the AEMIF controller itself from device-tree properties.
> Implementing the setup_interface() callback implies being able to update
> dynamically these timings, so the first two patches of the series modify
> the AEMIF driver to provide its 'children' a way to modify their chip
> select timing configuration. To do so, I add a ti-aemif.h header, I'm not
> sure whether this header should be located in include/memory or in
> include/linux/memory. I put it in include/memory because the folder
> already exists while include/linux/memory doesn't.

All Linux headers go to include/linux/, so this one should as well.



Best regards,
Krzysztof
Bastien Curutchet Oct. 30, 2024, 12:39 p.m. UTC | #2 
Hi Krzysztof,

On 10/30/24 12:17 PM, Krzysztof Kozlowski wrote:
> On 30/10/2024 11:47, Bastien Curutchet wrote:
>> Hi all,
>>
>> This patch series aims to implement the setup_interface() operation in
>> the DaVinci NAND controller to enable the use of all ONFI modes and
>> improve the NAND access speed.
>>
> 
> Your changelog is supposed to explain also merging dependencies. Within
> patchset or external.

I'm not sure I understand what you mean here. Do you mean that I need to 
explicitly state that the patches in the 
drivers/mtd/nand/raw/davinci_nand.c depend on the ones in 
drivers/memory/ti-aemif.c ?

There isn't any external dependency on this patch series. The ONFI modes 
are already managed by the NAND core driver (in 
drivers/mtd/nand/raw/nand_base.c). If a NAND controller wants to benefit 
from all the ONFI modes, it needs to implement the setup_interface() 
operation; otherwise it can only use the mode 0 which is the slowest.

> 
>> This NAND controller is present in the DaVinci (OMAP L138) and Keystone2
>> SoCs and functions as a 'child' of the AEMIF controller. So its timings
>> are set by the AEMIF controller itself from device-tree properties.
>> Implementing the setup_interface() callback implies being able to update
>> dynamically these timings, so the first two patches of the series modify
>> the AEMIF driver to provide its 'children' a way to modify their chip
>> select timing configuration. To do so, I add a ti-aemif.h header, I'm not
>> sure whether this header should be located in include/memory or in
>> include/linux/memory. I put it in include/memory because the folder
>> already exists while include/linux/memory doesn't.
> 
> All Linux headers go to include/linux/, so this one should as well.
> 

Ok thank you, I'll move it there in V2.


Best regards,
Bastien
Krzysztof Kozlowski Oct. 30, 2024, 2:25 p.m. UTC | #3 
On 30/10/2024 13:39, Bastien Curutchet wrote:
> Hi Krzysztof,
> 
> On 10/30/24 12:17 PM, Krzysztof Kozlowski wrote:
>> On 30/10/2024 11:47, Bastien Curutchet wrote:
>>> Hi all,
>>>
>>> This patch series aims to implement the setup_interface() operation in
>>> the DaVinci NAND controller to enable the use of all ONFI modes and
>>> improve the NAND access speed.
>>>
>>
>> Your changelog is supposed to explain also merging dependencies. Within
>> patchset or external.
> 
> I'm not sure I understand what you mean here. Do you mean that I need to 
> explicitly state that the patches in the 
> drivers/mtd/nand/raw/davinci_nand.c depend on the ones in 
> drivers/memory/ti-aemif.c ?

Well, you target different subsystems, so maintainers need to know what
they can take and what do you expect of them. Otherwise what do you
expect of me exactly and how can I guess it?


Best regards,
Krzysztof