Re: [PATCH v3 2/3] PCI: qcom: Add support for multi-root port

[Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>]

On 4/23/2025 2:15 AM, Konrad Dybcio wrote:
> On 4/19/25 7:19 AM, Krishna Chaitanya Chundru wrote:
>> Move phy, perst handling to root port and provide a way to have multi-port
>> logic.
>>
>> Currently, qcom controllers only support single port, and all properties
>> are present in the controller node itself. This is incorrect, as
>> properties like phy, perst, wake, etc. can vary per port and should be
>> present in the root port node.
>>
>> To maintain DT backwards compatibility, fallback to the legacy method of
>> parsing the controller node if the port parsing fails.
>>
>> pci-bus-common.yaml uses reset-gpios property for representing PERST, use
>> same property instead of perst-gpios.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> -static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
>> +static void qcom_perst_assert_deassert(struct qcom_pcie *pcie, bool assert)
>>   {
>> -	gpiod_set_value_cansleep(pcie->reset, 1);
>> +	struct qcom_pcie_port *port, *tmp;
>> +	int val = assert ? 1 : 0;
> 
> assert is already a boolean - are some checkers complaining?
Ack, I will remove this in next patch.
> 
> [...]
> 
>> +	/*
>> +	 * In the case of failure in parsing the port nodes, fallback to the
>> +	 * legacy method of parsing the controller node. This is to maintain DT
>> +	 * backwards compatibility.
> 
> It'd be simpler to call qcom_pcie_parse_port on the PCIe controller's
> OF node, removing the need for the if-else-s throughout the patch
> 
There is difference in perst property name for controller's OF node and
the root port OF node. controller use perst-gpios, where as the root
port node uses the pci-bus-common.yaml defined way of perst i.e
reset-gpios.

It's better to have this way then having if else condition in the
qcom_pcie_parse_port.

- Krishna Chaitanya.
> Konrad
> 
>