linux-kernel - Re: [PATCH 2/3] mmc: sdhci-dwcmshc: Add Canaan K230 DWCMSHC controller support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <def032d1-b1c5-4a75-88de-cbb7c1293e61@kernel.org>
Date: Sat, 7 Feb 2026 10:32:55 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Jiayu Du <jiayu.riscv@...c.iscas.ac.cn>, ulf.hansson@...aro.org,
 adrian.hunter@...el.com, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org
Cc: pjw@...nel.org, palmer@...belt.com, aou@...s.berkeley.edu,
 linux-mmc@...r.kernel.org, devicetree@...r.kernel.org,
 linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
 gaohan@...as.ac.cn, me@...ao.cc
Subject: Re: [PATCH 2/3] mmc: sdhci-dwcmshc: Add Canaan K230 DWCMSHC
 controller support

On 07/02/2026 09:45, Jiayu Du wrote:
> On Fri, Feb 06, 2026 at 02:26:40PM +0100, Krzysztof Kozlowski wrote:
>> On 04/02/2026 09:29, Jiayu Du wrote:
>>> +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.


> 
>>> +	if (!usb_phy_node) {
>>
>> Please follow Linux coding style.
> 
> I will fix it in next version.
> 
>>> +		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