RE: [RFC PATCH v1 1/1] mmc: snps_sdhci: Add sdhci driver support for TH1520 SoC

["Jaehoon Chung" <jh80.chung@samsung.com>]

Hi,

> -----Original Message-----
> From: Maxim Kiselev <bigunclemax@gmail.com>
> Sent: Wednesday, April 3, 2024 3:57 AM
> To: Sean Anderson <sean.anderson@seco.com>
> Cc: Tom Rini <trini@konsulko.com>; Peng Fan <peng.fan@nxp.com>; Jaehoon Chung <jh80.chung@samsung.com>;
> Marek Vasut <marek.vasut+renesas@mailbox.org>; Paul Barker <paul.barker.ct@bp.renesas.com>; Kever Yang
> <kever.yang@rock-chips.com>; Peter Geis <pgwipeout@gmail.com>; Oleksandr Suvorov
> <oleksandr.suvorov@foundries.io>; Stefan Roese <sr@denx.de>; u-boot@lists.denx.de
> Subject: Re: [RFC PATCH v1 1/1] mmc: snps_sdhci: Add sdhci driver support for TH1520 SoC
> 
> Hi, Sean
> 
> вт, 2 апр. 2024 г. в 18:39, Sean Anderson <sean.anderson@seco.com>:
> >
> > On 3/30/24 13:59, Maksim Kiselev wrote:
> > > Add support for DesignWare SDHCI host controller on Alibaba TH1520 SoC
> > >
> > > Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
> > > ---
> > > drivers/mmc/Kconfig | 12 +
> > > drivers/mmc/Makefile | 1 +
> > > drivers/mmc/snps_sdhci.c | 464 +++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 477 insertions(+)
> > > create mode 100644 drivers/mmc/snps_sdhci.c
> > >
> > > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> > > index cef05790dd..6936fa0928 100644
> > > --- a/drivers/mmc/Kconfig
> > > +++ b/drivers/mmc/Kconfig
> > > @@ -671,6 +671,18 @@ config MMC_SDHCI_S5P
> > >
> > > If unsure, say N.
> > >
> > > +config MMC_SDHCI_SNPS
> > > + bool "Synopsys DesignWare SDHCI controller"
> > > + depends on MMC_SDHCI
> > > + depends on DM_MMC
> > > + help
> > > + Support for DesignWare SDHCI host controller on Alibaba TH1520 SoC.
> > > + This is a highly configurable and programmable, high performance
> > > + Mobile Storage Host Controller (MSHC) with AXI as the bus interface
> > > + for data transfer.
> > > +
> > > + If unsure, say N.
> > > +
> >
> > Why not use MMC_DW?
> 
> That driver looks completely different. As I notice in the cover letter,
> the MMC_SDHCI_ROCKCHIP driver is closer to TH1520 driver.
> https://lore.kernel.org/u-boot/20240330175948.80931-1-bigunclemax@gmail.com/
> 
> Their IP blocks look very similar.
> Moreover, on Linux, Rockchip SDHCI controllers (ex. rockchip,rk3588-dwcmshc)
> and the TH1520 controller live in the same driver - sdhci-of-dwcmshc.c

Right. I didn't have its TRM. AFAIK, It's a different IP with DW_MMC.

> 
> We could do the same for U-boot. It will require to get rid of Rockchip
> specific defines that come from asm/arch-rockchip/hardware.h.
> And maybe rename the rockchip_sdhci.c driver to sdhci-of-dwcmshc.c.

In u-boot, rochkip_sdhci.c is including the specific rockchip code. (rockchip header..)
So I think that it can be a huge refactoring to use both SoC.

But if it's possible, I think that it can be reusable.

> 
> That's why I mark this series as RFC. So I'm willing to discuss any option.
> 
> > --Sean
> >
> >
> > [Embedded World 2024, SECO SpA]<https://www.messe-
> ticket.de/Nuernberg/embeddedworld2024/Register/ew24517689>
> 
> Cheers,
> Maksim