Thread (13 messages) 13 messages, 4 authors, 2026-02-08

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help