linux-kernel - Re: [PATCH 08/10] phy: exynos: Add PCIe PHY support for FSD SoC

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 for Android: free password hash cracker in your pocket

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

Message-ID: <a42f7c93-4c26-489e-a680-ad20a8b8a0a6@kernel.org>
Date: Wed, 28 May 2025 09:21:27 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Shradha Todi <shradha.t@...sung.com>
Cc: linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.or,
 linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org,
 manivannan.sadhasivam@...aro.org, lpieralisi@...nel.org, kw@...ux.com,
 robh@...nel.org, bhelgaas@...gle.com, jingoohan1@...il.com,
 krzk+dt@...nel.org, conor+dt@...nel.org, alim.akhtar@...sung.com,
 vkoul@...nel.org, kishon@...nel.org, arnd@...db.de,
 m.szyprowski@...sung.com, jh80.chung@...sung.com
Subject: Re: [PATCH 08/10] phy: exynos: Add PCIe PHY support for FSD SoC

On 27/05/2025 12:45, Shradha Todi wrote:
>>>
>>> -	generic_phy = devm_phy_create(dev, dev->of_node, &exynos5433_phy_ops);
>>> +	generic_phy = devm_phy_create(dev, dev->of_node, drv_data->phy_ops);
>>>  	if (IS_ERR(generic_phy)) {
>>>  		dev_err(dev, "failed to create PHY\n");
>>>  		return PTR_ERR(generic_phy);
>>>  	}
>>>
>>> +	exynos_phy->pcs_base = devm_platform_ioremap_resource(pdev, 1);
>>> +	exynos_phy->phy_id = of_alias_get_id(dev->of_node, "pciephy");
>>
>> Where did you document aliases?
>>
> 
> Will add it to dt bindings.
> 
>> Anyway, all this looks because you have completely buggy way of handling MMIO via syscon. That's a no-go. Use proper address
>> ranges assigned to ddevices. If you ever need to use syscon, you should pass the offset as argument - just like other devices are
>> doing.
>>
> 
> Alias is used for 2 reasons.

So if on my board the PCI slots are named differently and I use
different alias, everything will stop working, right? Usually aliases
for exposable buses are matching what is physically labeled on the
exposed interface.

> 1. Each of the 2 PHYs in FSD have different initializing sequence due to channel length, etc. We need the alias to select the init sequence accordingly

So devices are different? What is channel length? Number of lanes?


> 2. The syscon offset can be passed via DT but the bit field also varies according to instance. (common reset is bit 8 in PHY0 and bit 1 in PHY1).

You did not address the main problem here: you use MMIO but do not
define any MMIO. Syscon is not a replacement for MMIO.


Best regards,
Krzysztof