Agilex SDMMC clock enable/disable API #808
Conversation
1ff31a0 to
269be5d
Compare
Update Agilex clock IDs to the latest and correct ID numbers Signed-off-by: Alif Zakuan Yuslaimi <alif.zakuan.yuslaimi@altera.com>
2642992 to
77322b8
Compare
drivers/mmc/socfpga_dw_mmc.c
Outdated
| struct udevice *clkmgr_dev; | ||
| int ret = uclass_get_device_by_name(UCLASS_CLK, "clock-controller@ffd10000", &clkmgr_dev); | ||
|
|
||
| ret = clk_get_by_name(dev, "ciu", &priv->mmc_clk_ciu); |
There was a problem hiding this comment.
clk_get_by_name(dev, "ciu", &priv->mmc_clk_ciu) will only work if the MMC node has clock-names = "ciu" and the provider exports that name. Using clk_get_by_index(dev, 0, ...) (or clk_get_optional...) is usually more robust across DTS variants. If you keep name-based lookup, ensure the DTS matches.
There was a problem hiding this comment.
the target clock index is not 0 by default in agilex dt -
the same can be observed in n5x dt in downstream - https://github.com/altera-innersource/applications.fpga.soc.uboot-socfpga-dev/blob/dcb7e01595e33ac087f1991709d3e27da2a93eec/arch/arm/dts/socfpga_n5x.dtsi#L315
my plan is to make sure all supported families to use "ciu" naming to point to sdmmc clock as we upstream all of our families to external.
| CLKMGR_PERPLLGRP_EN_SDMMCCLK_MASK); | ||
| ret = clk_disable(&priv->mmc_clk_ciu); | ||
| if (ret) { | ||
| printf("%s: Failed to disable SDMMC clock\n", __func__); |
There was a problem hiding this comment.
Prefer dev_err(dev, ...) / dev_dbg(dev, ...) over bare printf() for driver-model devices. It keeps logs consistent and can be filtered per device.
printf()
Global: Always prints unconditionally to the console (UART or log).
No context: Doesn’t automatically include which device/driver emitted the message.
No log level: It’s treated like a raw string — you can’t filter or categorize it.
Legacy: Older code uses printf(), but driver-model code is expected to use dev_*() helpers.
dev_err(dev, ...) / dev_dbg(dev, ...)
Device-aware logging:
These helpers automatically prefix messages with the device’s name/driver instance (e.g. mmc@ff808000: error: clock not found). This makes logs much clearer when you have multiple instances of the same IP (say, multiple MMC controllers).
Log levels:
dev_err(dev, ...) → error log, always shown.
dev_warn(dev, ...) → warning.
dev_info(dev, ...) → informational.
dev_dbg(dev, ...) → debug, compiled in only if DEBUG or CONFIG_DEBUG_DEVRES (depending on subsystem) is enabled. This way you don’t flood normal builds with debug spam.
Integration with log framework:
U-Boot has a log command and runtime loglevel filtering. Messages from dev_*() follow that framework. With printf() you lose that.
Consistency:
Maintainers prefer dev_*() in new DM-based drivers because it keeps all output structured and filterable.
There was a problem hiding this comment.
i see some sections of the code not touched by this patch series still using printf, example - https://github.com/u-boot/u-boot/blame/dca578a9c9decb85271665de8086b8f41731d388/drivers/mmc/socfpga_dw_mmc.c#L84, and https://github.com/u-boot/u-boot/blame/dca578a9c9decb85271665de8086b8f41731d388/drivers/mmc/socfpga_dw_mmc.c#L120.
do you want me to update these as well?
98fefc2 to
02d562c
Compare
Update Agilex clock driver to support enabling or disabling the peripheral clocks via clock driver model APIs. The caller will pass the clock ID to this driver and the driver will then proceed to manipulate the desired bit in the Agilex clock manager peripheral PLL register based on the given clock ID. Signed-off-by: Alif Zakuan Yuslaimi <alif.zakuan.yuslaimi@altera.com>
Update the driver to enable or disable the SDMMC clock via clock driver model API instead of doing it in the driver itself. This allows for scalability of the driver for various SoCFPGA devices. Signed-off-by: Alif Zakuan Yuslaimi <alif.zakuan.yuslaimi@altera.com>
02d562c to
a780b2c
Compare
Pipeline test for the patch series upstream.
Tested on fm61linear board. Verified that the clock manager peripheral PLL register (HPS address map link) is able to be manipulated successfully.
Able to boot to Linux via SDMMC image - SPETC link
checkpatch.pl scan clean.