Re: [PATCH v1 8/9] mmc: sdhci-cadence: add Cadence SD6HC support

[Adrian Hunter]

On 21/05/2026 20:28, Kathpalia, Tanmay wrote:
> Hi Adrian,
> 
> Thanks for your feedback.
> 
> On 5/19/2026 8:59 PM, Adrian Hunter wrote:
>> On 11/05/2026 23:21, Tanmay Kathpalia wrote:
>>> The Cadence SD6HC is the sixth-generation SD Host Controller used in
>>> Agilex5 SoCs. Its PHY differs substantially from the SD4HC: it
>>> requires per-speed-mode IO cell timing parameters and a DLL-based
>>> delay line to achieve correct signal margins across all speed grades
>>> from Default Speed to HS400.
>>>
>>> Support is implemented in a new sdhci-cadence6.c alongside the
>>> existing v4 code, now in sdhci-cadence4.c. Shared structures and the
>>> v6 function interface are declared in a new sdhci-cadence.h header.
>>> The common driver paths select between v4 and v6 PHY operations based
>>> on the SDHCI specification version reported by the controller.
>>>
>>> The new compatible string "cdns,sd6hc" identifies SD6HC hardware in
>>> device tree.
>>>
>>> Signed-off-by: Tanmay Kathpalia <tanmay.kathpalia@xxxxxxxxxx>
>>> ---
>>>   MAINTAINERS                                   |    7 +
>>>   drivers/mmc/host/Makefile                     |    3 +-
>>>   drivers/mmc/host/sdhci-cadence.h              |  113 ++
>>>   .../{sdhci-cadence.c => sdhci-cadence4.c}     |   92 +-
>>>   drivers/mmc/host/sdhci-cadence6.c             | 1051 +++++++++++++++++
>>>   5 files changed, 1228 insertions(+), 38 deletions(-)
>>>   create mode 100644 drivers/mmc/host/sdhci-cadence.h
>>>   rename drivers/mmc/host/{sdhci-cadence.c => sdhci-cadence4.c} (91%)
>>>   create mode 100644 drivers/mmc/host/sdhci-cadence6.c
>> The new file names seem slightly misleading since sdhci-cadence4.c
>> is the main driver as well as supporting V4, whereas sdhci-cadence6.c
>> is most of V6 support.
>>
>> There doesn't seem much to gain from having a separate sdhci-cadence6.c.
>> Isn't it simpler to just put it all in 1 source file.
> 
> I believe keeping them separate improves maintainability for the
> following reasons:
> 
> 1. V4 and V6 PHY programming models are fundamentally different:
>     a. SD4HC reads pre-computed delay values directly from DT and
>     writes them one-to-one into PHY registers.
>     b. SD6HC takes only a few timing parameters (IO cell delay, delay
>     element size) from DT and derives all PHY and SDHC register
>     programming values inside the driver through a calibration
>     sequence.
> 2. The V6 PHY logic alone is more than 1000 lines. Merging it into
> the existing file would make sdhci-cadence4.c significantly harder
> to review, and maintain.
> 3. Keep code structure aligned with U-Boot, where V6 handling is
> also separated.
> 
> That said, I am open to merging into one file if you think that is
> better for maintainability in this driver. I can do that in v2.

At the very least, the file names need to be more explicit:

  The core part of the driver:	sdhci-cadence-core.c
  The V6 PHY implementation:	sdhci-cadence-phy-v6.c