[RFC,00/13] Add tuning algorithm for delay chain

  This patch series introduces a new tuning algorithm for
mmc. The new algorithm should be used when delay chain is
enabled. The ITAPDLY is selected from the largest passing
window and the buffer is not viewed as a circular buffer.
The new tuning algorithm is implemented as per the paper
published here [0] and has been tested on the following
platforms: AM62x SK, AM62A SK, AM62p SK, AM64x SK, and AM64x
EVM.

The series also includes a few fixes in the sdhci_am654
driver on OTAPDLYEN/ITAPDLYEN and ITAPDELSEL. There are
also device tree node fixes for missing mmc nodes,
modifying DLL properties, and fixes for OTAP/ITAP delay
values. 

MMC0/MMC2 nodes are introduced for AM62ax in this series.

This series is sent as a RFC mostly to get some feedback
and/or comments on the new tuning algorithm implementation.

[0] https://www.ti.com/lit/an/spract9/spract9.pdf

Judith Mendez (11):
  drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain
  drivers: mmc: host: sdhci_am654: Write ITAPDLY for DDR52 timing
  drivers: mmc: host: sdhci_am654: Add missing OTAP/ITAP enable
  drivers: mmc: host: sdhci_am654: Add ITAPDLYSEL in
    sdhci_j721e_4bit_set_clock
  drivers: mmc: host: sdhci_am654: Fix ITAPDLY for HS400 timing
  arm64: dts: ti: k3-am62a-main: Add sdhci2 instance
  arm64: dts: ti: k3-am64-main: Update ITAP/OTAP values for MMC
  arm64: dts: ti: k3-am62-main: Update ITAP/OTAP values for MMC
  arm64: dts: ti: k3-am62p: Add missing properties for MMC
  arm64: dts: ti: k3-am6*: Remove DLL properties for soft phys
  arm64: dts: ti: k3-am6*: Reorganize MMC properties

Nitin Yadav (2):
  arm64: dts: ti: k3-am62a-main: Add sdhci0 instance
  arm64: dts: ti: k3-am62a7-sk: Enable eMMC support

 arch/arm64/boot/dts/ti/k3-am62-main.dtsi      |  57 +++--
 .../arm64/boot/dts/ti/k3-am625-beagleplay.dts |   5 -
 arch/arm64/boot/dts/ti/k3-am62a-main.dtsi     |  45 +++-
 arch/arm64/boot/dts/ti/k3-am62a7-sk.dts       |  27 ++-
 arch/arm64/boot/dts/ti/k3-am62p-main.dtsi     |  44 +++-
 arch/arm64/boot/dts/ti/k3-am62p5-sk.dts       |   7 +-
 .../arm64/boot/dts/ti/k3-am62x-sk-common.dtsi |   4 +-
 arch/arm64/boot/dts/ti/k3-am64-main.dtsi      |  17 +-
 arch/arm64/boot/dts/ti/k3-am642-evm.dts       |   4 +-
 arch/arm64/boot/dts/ti/k3-am642-sk.dts        |   2 -
 drivers/mmc/host/sdhci_am654.c                | 215 ++++++++++++++----
 11 files changed, 321 insertions(+), 106 deletions(-)
  

Hi,

On 1/31/2024 6:07 AM, Judith Mendez wrote:
> This patch series introduces a new tuning algorithm for
> mmc. The new algorithm should be used when delay chain is
> enabled. The ITAPDLY is selected from the largest passing
> window and the buffer is not viewed as a circular buffer.
> The new tuning algorithm is implemented as per the paper
> published here [0] and has been tested on the following
> platforms: AM62x SK, AM62A SK, AM62p SK, AM64x SK, and AM64x
> EVM.
> 
> The series also includes a few fixes in the sdhci_am654
> driver on OTAPDLYEN/ITAPDLYEN and ITAPDELSEL. There are
> also device tree node fixes for missing mmc nodes,
> modifying DLL properties, and fixes for OTAP/ITAP delay
> values. 
> 
> MMC0/MMC2 nodes are introduced for AM62ax in this series.
> 
> This series is sent as a RFC mostly to get some feedback
> and/or comments on the new tuning algorithm implementation.
> 
> [0] https://www.ti.com/lit/an/spract9/spract9.pdf
> 


> Judith Mendez (11):
>   drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain
>   drivers: mmc: host: sdhci_am654: Write ITAPDLY for DDR52 timing
>   drivers: mmc: host: sdhci_am654: Add missing OTAP/ITAP enable
>   drivers: mmc: host: sdhci_am654: Add ITAPDLYSEL in
>     sdhci_j721e_4bit_set_clock
>   drivers: mmc: host: sdhci_am654: Fix ITAPDLY for HS400 timing

These patches needs to have Fixes: tag as they are bug fixes IMO.

>   arm64: dts: ti: k3-am62a-main: Add sdhci2 instance
>   arm64: dts: ti: k3-am64-main: Update ITAP/OTAP values for MMC
>   arm64: dts: ti: k3-am62-main: Update ITAP/OTAP values for MMC
>   arm64: dts: ti: k3-am62p: Add missing properties for MMC
>   arm64: dts: ti: k3-am6*: Remove DLL properties for soft phys
>   arm64: dts: ti: k3-am6*: Reorganize MMC properties
> 
> Nitin Yadav (2):
>   arm64: dts: ti: k3-am62a-main: Add sdhci0 instance
>   arm64: dts: ti: k3-am62a7-sk: Enable eMMC support
> 

Can the driver changes be merged independent of DT changes? Or are they
meant to go together? Latter would be problematic as it creates cross
tree dependencies.

>  arch/arm64/boot/dts/ti/k3-am62-main.dtsi      |  57 +++--
>  .../arm64/boot/dts/ti/k3-am625-beagleplay.dts |   5 -
>  arch/arm64/boot/dts/ti/k3-am62a-main.dtsi     |  45 +++-
>  arch/arm64/boot/dts/ti/k3-am62a7-sk.dts       |  27 ++-
>  arch/arm64/boot/dts/ti/k3-am62p-main.dtsi     |  44 +++-
>  arch/arm64/boot/dts/ti/k3-am62p5-sk.dts       |   7 +-
>  .../arm64/boot/dts/ti/k3-am62x-sk-common.dtsi |   4 +-
>  arch/arm64/boot/dts/ti/k3-am64-main.dtsi      |  17 +-
>  arch/arm64/boot/dts/ti/k3-am642-evm.dts       |   4 +-
>  arch/arm64/boot/dts/ti/k3-am642-sk.dts        |   2 -
>  drivers/mmc/host/sdhci_am654.c                | 215 ++++++++++++++----
>  11 files changed, 321 insertions(+), 106 deletions(-)
>
  

On 31/01/2024 14:35, Raghavendra, Vignesh wrote:
>> Judith Mendez (11):
>>   drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain
>>   drivers: mmc: host: sdhci_am654: Write ITAPDLY for DDR52 timing
>>   drivers: mmc: host: sdhci_am654: Add missing OTAP/ITAP enable
>>   drivers: mmc: host: sdhci_am654: Add ITAPDLYSEL in
>>     sdhci_j721e_4bit_set_clock
>>   drivers: mmc: host: sdhci_am654: Fix ITAPDLY for HS400 timing
> 
> These patches needs to have Fixes: tag as they are bug fixes IMO.
> 
>>   arm64: dts: ti: k3-am62a-main: Add sdhci2 instance
>>   arm64: dts: ti: k3-am64-main: Update ITAP/OTAP values for MMC
>>   arm64: dts: ti: k3-am62-main: Update ITAP/OTAP values for MMC
>>   arm64: dts: ti: k3-am62p: Add missing properties for MMC
>>   arm64: dts: ti: k3-am6*: Remove DLL properties for soft phys
>>   arm64: dts: ti: k3-am6*: Reorganize MMC properties
>>
>> Nitin Yadav (2):
>>   arm64: dts: ti: k3-am62a-main: Add sdhci0 instance
>>   arm64: dts: ti: k3-am62a7-sk: Enable eMMC support
>>
> 
> Can the driver changes be merged independent of DT changes? Or are they
> meant to go together? Latter would be problematic as it creates cross
> tree dependencies.

DTS cannot depend on driver changes, because that would mean hardware
description is not really hardware but OS. So the answer to your
question must be "yes, can be merged independently".

Best regards,
Krzysztof
  

On 1/31/2024 7:11 PM, Krzysztof Kozlowski wrote:
> On 31/01/2024 14:35, Raghavendra, Vignesh wrote:
>>> Judith Mendez (11):
>>>   drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain
>>>   drivers: mmc: host: sdhci_am654: Write ITAPDLY for DDR52 timing
>>>   drivers: mmc: host: sdhci_am654: Add missing OTAP/ITAP enable
>>>   drivers: mmc: host: sdhci_am654: Add ITAPDLYSEL in
>>>     sdhci_j721e_4bit_set_clock
>>>   drivers: mmc: host: sdhci_am654: Fix ITAPDLY for HS400 timing
>>
>> These patches needs to have Fixes: tag as they are bug fixes IMO.
>>
>>>   arm64: dts: ti: k3-am62a-main: Add sdhci2 instance
>>>   arm64: dts: ti: k3-am64-main: Update ITAP/OTAP values for MMC
>>>   arm64: dts: ti: k3-am62-main: Update ITAP/OTAP values for MMC
>>>   arm64: dts: ti: k3-am62p: Add missing properties for MMC
>>>   arm64: dts: ti: k3-am6*: Remove DLL properties for soft phys
>>>   arm64: dts: ti: k3-am6*: Reorganize MMC properties
>>>
>>> Nitin Yadav (2):
>>>   arm64: dts: ti: k3-am62a-main: Add sdhci0 instance
>>>   arm64: dts: ti: k3-am62a7-sk: Enable eMMC support
>>>
>>
>> Can the driver changes be merged independent of DT changes? Or are they
>> meant to go together? Latter would be problematic as it creates cross
>> tree dependencies.
> 
> DTS cannot depend on driver changes, because that would mean hardware
> description is not really hardware but OS. So the answer to your
> question must be "yes, can be merged independently".
> 

Normally yes, but here I see update to tuning algorithm and timing
paramaters to the algorithm. DT updates seem to be nature of bug fixes
where in parameters have been refined due to more HW char
data/understanding being available.

I get nervous when anyone posts both driver and DT changes in a single
series, because they may not have tested driver changes standalone and
may have broken the DT backward compatibility

> Best regards,
> Krzysztof
> 

Regards
Vignesh
  

On 1/31/24 7:35 AM, Raghavendra, Vignesh wrote:
> Hi,
> 
> On 1/31/2024 6:07 AM, Judith Mendez wrote:
>> This patch series introduces a new tuning algorithm for
>> mmc. The new algorithm should be used when delay chain is
>> enabled. The ITAPDLY is selected from the largest passing
>> window and the buffer is not viewed as a circular buffer.
>> The new tuning algorithm is implemented as per the paper
>> published here [0] and has been tested on the following
>> platforms: AM62x SK, AM62A SK, AM62p SK, AM64x SK, and AM64x
>> EVM.
>>
>> The series also includes a few fixes in the sdhci_am654
>> driver on OTAPDLYEN/ITAPDLYEN and ITAPDELSEL. There are
>> also device tree node fixes for missing mmc nodes,
>> modifying DLL properties, and fixes for OTAP/ITAP delay
>> values.
>>
>> MMC0/MMC2 nodes are introduced for AM62ax in this series.
>>
>> This series is sent as a RFC mostly to get some feedback
>> and/or comments on the new tuning algorithm implementation.
>>
>> [0] https://www.ti.com/lit/an/spract9/spract9.pdf
>>
> 
> 
>> Judith Mendez (11):
>>    drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain
>>    drivers: mmc: host: sdhci_am654: Write ITAPDLY for DDR52 timing
>>    drivers: mmc: host: sdhci_am654: Add missing OTAP/ITAP enable
>>    drivers: mmc: host: sdhci_am654: Add ITAPDLYSEL in
>>      sdhci_j721e_4bit_set_clock
>>    drivers: mmc: host: sdhci_am654: Fix ITAPDLY for HS400 timing
> 
> These patches needs to have Fixes: tag as they are bug fixes IMO.

Understood, will add.

> 
>>    arm64: dts: ti: k3-am62a-main: Add sdhci2 instance
>>    arm64: dts: ti: k3-am64-main: Update ITAP/OTAP values for MMC
>>    arm64: dts: ti: k3-am62-main: Update ITAP/OTAP values for MMC
>>    arm64: dts: ti: k3-am62p: Add missing properties for MMC
>>    arm64: dts: ti: k3-am6*: Remove DLL properties for soft phys
>>    arm64: dts: ti: k3-am6*: Reorganize MMC properties
>>
>> Nitin Yadav (2):
>>    arm64: dts: ti: k3-am62a-main: Add sdhci0 instance
>>    arm64: dts: ti: k3-am62a7-sk: Enable eMMC support
>>
> 
> Can the driver changes be merged independent of DT changes? Or are they
> meant to go together? Latter would be problematic as it creates cross
> tree dependencies.

The driver changes can be merged independently.

> 
>>   arch/arm64/boot/dts/ti/k3-am62-main.dtsi      |  57 +++--
>>   .../arm64/boot/dts/ti/k3-am625-beagleplay.dts |   5 -
>>   arch/arm64/boot/dts/ti/k3-am62a-main.dtsi     |  45 +++-
>>   arch/arm64/boot/dts/ti/k3-am62a7-sk.dts       |  27 ++-
>>   arch/arm64/boot/dts/ti/k3-am62p-main.dtsi     |  44 +++-
>>   arch/arm64/boot/dts/ti/k3-am62p5-sk.dts       |   7 +-
>>   .../arm64/boot/dts/ti/k3-am62x-sk-common.dtsi |   4 +-
>>   arch/arm64/boot/dts/ti/k3-am64-main.dtsi      |  17 +-
>>   arch/arm64/boot/dts/ti/k3-am642-evm.dts       |   4 +-
>>   arch/arm64/boot/dts/ti/k3-am642-sk.dts        |   2 -
>>   drivers/mmc/host/sdhci_am654.c                | 215 ++++++++++++++----
>>   11 files changed, 321 insertions(+), 106 deletions(-)
>>
  

On 31/01/2024 14:53, Raghavendra, Vignesh wrote:
> 
> 
> On 1/31/2024 7:11 PM, Krzysztof Kozlowski wrote:
>> On 31/01/2024 14:35, Raghavendra, Vignesh wrote:
>>>> Judith Mendez (11):
>>>>   drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain
>>>>   drivers: mmc: host: sdhci_am654: Write ITAPDLY for DDR52 timing
>>>>   drivers: mmc: host: sdhci_am654: Add missing OTAP/ITAP enable
>>>>   drivers: mmc: host: sdhci_am654: Add ITAPDLYSEL in
>>>>     sdhci_j721e_4bit_set_clock
>>>>   drivers: mmc: host: sdhci_am654: Fix ITAPDLY for HS400 timing
>>>
>>> These patches needs to have Fixes: tag as they are bug fixes IMO.
>>>
>>>>   arm64: dts: ti: k3-am62a-main: Add sdhci2 instance
>>>>   arm64: dts: ti: k3-am64-main: Update ITAP/OTAP values for MMC
>>>>   arm64: dts: ti: k3-am62-main: Update ITAP/OTAP values for MMC
>>>>   arm64: dts: ti: k3-am62p: Add missing properties for MMC
>>>>   arm64: dts: ti: k3-am6*: Remove DLL properties for soft phys
>>>>   arm64: dts: ti: k3-am6*: Reorganize MMC properties
>>>>
>>>> Nitin Yadav (2):
>>>>   arm64: dts: ti: k3-am62a-main: Add sdhci0 instance
>>>>   arm64: dts: ti: k3-am62a7-sk: Enable eMMC support
>>>>
>>>
>>> Can the driver changes be merged independent of DT changes? Or are they
>>> meant to go together? Latter would be problematic as it creates cross
>>> tree dependencies.
>>
>> DTS cannot depend on driver changes, because that would mean hardware
>> description is not really hardware but OS. So the answer to your
>> question must be "yes, can be merged independently".
>>
> 
> Normally yes, but here I see update to tuning algorithm and timing
> paramaters to the algorithm. DT updates seem to be nature of bug fixes
> where in parameters have been refined due to more HW char
> data/understanding being available.

Then the patchset should be probably split into fixes and new features,
so it would be clear that new DTS features do not depend on driver code.


Best regards,
Krzysztof