> -----Original Message-----
> From: Peng Fan <peng....@oss.nxp.com>
> Sent: Tuesday, August 5, 2025 12:08 PM
> To: Yuslaimi, Alif Zakuan <alif.zakuan.yusla...@altera.com>
> Cc: u-boot@lists.denx.de; Peng Fan <peng....@nxp.com>; Jaehoon Chung
> <jh80.ch...@samsung.com>; Tom Rini <tr...@konsulko.com>; Chee, Tien Fong
> <tien.fong.c...@altera.com>; Marek Vasut <ma...@denx.de>; Simon
> Goldschmidt <simon.k.r.goldschm...@gmail.com>
> Subject: Re: [PATCH v1] mmc: socfpga_dw_mmc: Retrieve clock manager
> address via clocks phandle
>
> [CAUTION: This email is from outside your organization. Unless you trust the
> sender, do not click on links or open attachments as it may be a fraudulent
> email
> attempting to steal your information and/or compromise your computer.]
>
> On Sun, Aug 03, 2025 at 05:47:10PM -0700, alif.zakuan.yusla...@altera.com
> wrote:
> >From: Alif Zakuan Yuslaimi <alif.zakuan.yusla...@altera.com>
> >
> >Improve the current clock manager base address retrieval by probing the
> >clocks phandle under the mmc node in the device tree instead of probing
> >the clock manager node name in the device tree.
> >
> >This will help to make the driver more scalable for more families as we
> >no longer need to rely on the exact node name in the device tree to
> >probe the clock manager driver for its base address.
> >
> >Signed-off-by: Alif Zakuan Yuslaimi <alif.zakuan.yusla...@altera.com>
> >---
> > drivers/mmc/socfpga_dw_mmc.c | 19 +++++++++++++++----
> > 1 file changed, 15 insertions(+), 4 deletions(-)
> >
> >diff --git a/drivers/mmc/socfpga_dw_mmc.c
> >b/drivers/mmc/socfpga_dw_mmc.c index 3b86bc9b18c..ff1dd0055f8 100644
> >--- a/drivers/mmc/socfpga_dw_mmc.c
> >+++ b/drivers/mmc/socfpga_dw_mmc.c
> >@@ -54,17 +54,28 @@ static int socfpga_dwmci_clksel(struct dwmci_host
> *host)
> > u32 sdmmc_mask = ((priv->smplsel & 0x7) <<
> SYSMGR_SDMMC_SMPLSEL_SHIFT) |
> > ((priv->drvsel & 0x7) <<
> >SYSMGR_SDMMC_DRVSEL_SHIFT);
> >
> >- /* Get clock manager base address */
> >+ struct udevice *mmc_dev = host->mmc->dev;
> >+ struct ofnode_phandle_args clkmgr_args;
> > struct udevice *clkmgr_dev;
> >- int ret = uclass_get_device_by_name(UCLASS_CLK, "clock-
> controller@ffd10000", &clkmgr_dev);
> >+ fdt_addr_t clkmgr_base;
> >+ int ret;
> >
> >+ /*
> >+ * Get the clkmgr device from the first phandle in the "clocks"
> >property.
> >+ */
> >+ ret = dev_read_phandle_with_args(mmc_dev, "clocks",
> >+ "#clock-cells", 0, 0, &clkmgr_args);
> > if (ret) {
> >- printf("Failed to get clkmgr device: %d\n", ret);
> >+ printf("Failed to parse clocks property: %d\n", ret);
> > return ret;
> > }
> >
> >- fdt_addr_t clkmgr_base = dev_read_addr(clkmgr_dev);
> >+ ret = uclass_get_device_by_ofnode(UCLASS_CLK, clkmgr_args.node,
> &clkmgr_dev);
> >+ if (ret) {
> >+ printf("Failed to get clkmgr device: %d\n", ret);
> >+ return ret;
> >+ }
> >
> >+ clkmgr_base = dev_read_addr(clkmgr_dev);
>
> I see this driver use clk api, so why still directly map the clk manager base
> and
> configure it. Shouldn't clk api be used or assigned-clock-parents be used?
>
> Thanks,
> Peng
Hi Peng,
The direct access to the clock manager base and offset in this driver was
inherited from the legacy implementation before we had full driver model clock
support. Currently, both Agilex and Agilex5 have their own clock drivers under
the driver model, but the `clk_enable()` and `clk_disable()` operations are
still not implemented.
Because of that, we retained the base + offset logic for disabling a particular
clock in the SDMMC driver to preserve existing functionality. Once proper
enable/disable API support is added to the clock drivers, we can clean this up
to fully rely on the clock API or use `assigned-clock-parents` in device tree
where applicable.
Thanks,
Alif
>
> > if (clkmgr_base == FDT_ADDR_T_NONE) {
> > printf("Failed to read base address from clkmgr DT node\n");
> > return -EINVAL;
> >--
> >2.35.3
> >
