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

[Krzysztof Kozlowski <krzk@kernel.org>]

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