Re: [PATCH 2/3] mmc: sdhci-dwcmshc: Add Canaan K230 DWCMSHC controller support
From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2026-02-07 09:33:01
Also in:
linux-mmc, linux-riscv, lkml
On 07/02/2026 09:45, Jiayu Du wrote:
On Fri, Feb 06, 2026 at 02:26:40PM +0100, Krzysztof Kozlowski wrote:quoted
On 04/02/2026 09:29, Jiayu Du wrote:quoted
+static int dwcmshc_k230_init(struct device *dev, struct sdhci_host *host, + struct dwcmshc_priv *dwc_priv) +{ + static const char * const clk_ids[] = {"base", "timer", "ahb"}; + struct device_node *usb_phy_node; + struct k230_priv *k230_priv; + u32 data; + int ret; + + k230_priv = devm_kzalloc(dev, sizeof(struct k230_priv), GFP_KERNEL); + if (!k230_priv) + return -ENOMEM; + dwc_priv->priv = k230_priv; + + usb_phy_node = of_find_compatible_node(NULL, NULL, "canaan,k230-usb-phy");Hm? You should use phandles, not look for various nodes.Only one usbphy node has the canaan, k230-usb-phy compatibility. So in this situation, is it ok to continue using of_find_compatible_node?
Amount of nodes does not matter. This is not how you express links/dependencies between devices. Phandle is for this. This is wrong on many levels, including missing device links, bypassing kernel API/layers.
quoted
quoted
+ if (!usb_phy_node) {Please follow Linux coding style.I will fix it in next version.quoted
quoted
+ return dev_err_probe(dev, -ENODEV, + "Failed to find k230-usb-phy node\n"); + } + + k230_priv->hi_sys_regmap = device_node_to_regmap(usb_phy_node); + of_node_put(usb_phy_node); + if (IS_ERR(k230_priv->hi_sys_regmap)) { + return dev_err_probe(dev, PTR_ERR(k230_priv->hi_sys_regmap), + "Failed to get k230-usb-phy regmap\n"); + } + + ret = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv, + ARRAY_SIZE(clk_ids), clk_ids); + if (ret) { + return dev_err_probe(dev, ret, + "Failed to get/enable k230 mmc other clocks\n"); + } + + if (of_device_is_compatible(dev->of_node, "canaan,k230-sdio")) {Driver match data is for this.What you mean is that I shouldn't use of_find_compatible_node, but I can use device_get_match_data instead? Then I can continue to distinguish between SDIO and eMMC to do parameter configuration Or do you mean that I should put the parameters to be adjusted into the pdata structure? But currently, the dwcmshc structure is not suitable for containing vendor-specific properties.
Parameters should go to driver match data. I already requested this for some other driver and this has to be fixed. Best regards, Krzysztof