[v2] PCI: qcom: Add PCIe support for X1E80100

[PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by Abel Vesa 1 year, 10 months ago

Add the compatible and the driver data for X1E80100.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 10f2d0bb86be..2a6000e457bc 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
 	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
 	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
 	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },
 	{ }
 };
 

-- 
2.34.1

Re: [PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by Manivannan Sadhasivam 1 year, 10 months ago

On Mon, Jan 29, 2024 at 01:10:27PM +0200, Abel Vesa wrote:
> Add the compatible and the driver data for X1E80100.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 10f2d0bb86be..2a6000e457bc 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
>  	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
>  	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
>  	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },
>  	{ }
>  };
>  
> 
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by Konrad Dybcio 1 year, 10 months ago

On 29.01.2024 12:10, Abel Vesa wrote:
> Add the compatible and the driver data for X1E80100.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 10f2d0bb86be..2a6000e457bc 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
>  	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
>  	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
>  	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },

I swear I'm not delaying everything related to x1 on purpose..

But..

Would a "qcom,pcie-v1.9.0" generic match string be a good idea?

Konrad

Re: [PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by Abel Vesa 1 year, 10 months ago

On 24-02-01 20:20:40, Konrad Dybcio wrote:
> On 29.01.2024 12:10, Abel Vesa wrote:
> > Add the compatible and the driver data for X1E80100.
> > 
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 10f2d0bb86be..2a6000e457bc 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> >  	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
> >  	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
> >  	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
> > +	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },
> 
> I swear I'm not delaying everything related to x1 on purpose..
> 

No worries.

> But..
> 
> Would a "qcom,pcie-v1.9.0" generic match string be a good idea?

Sure. So that means this would be fallback compatible for all the following platforms:

- sa8540p
- sa8775p
- sc7280
- sc8180x
- sc8280xp
- sdx55
- sm8150
- sm8250
- sm8350
- sm8450-pcie0
- sm8450-pcie1
- sm8550
- x1e80100

Will prepare a patchset.

> 
> Konrad

Re: [PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by Manivannan Sadhasivam 1 year, 10 months ago

On Fri, Feb 02, 2024 at 10:41:03AM +0200, Abel Vesa wrote:
> On 24-02-01 20:20:40, Konrad Dybcio wrote:
> > On 29.01.2024 12:10, Abel Vesa wrote:
> > > Add the compatible and the driver data for X1E80100.
> > > 
> > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index 10f2d0bb86be..2a6000e457bc 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> > >  	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
> > >  	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
> > >  	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
> > > +	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },
> > 
> > I swear I'm not delaying everything related to x1 on purpose..
> > 
> 
> No worries.
> 
> > But..
> > 
> > Would a "qcom,pcie-v1.9.0" generic match string be a good idea?
> 
> Sure. So that means this would be fallback compatible for all the following platforms:
> 
> - sa8540p
> - sa8775p
> - sc7280
> - sc8180x
> - sc8280xp
> - sdx55
> - sm8150
> - sm8250
> - sm8350
> - sm8450-pcie0
> - sm8450-pcie1
> - sm8550
> - x1e80100
> 
> Will prepare a patchset.
> 

NO. Fallback should be based on the base SoC for this platform.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by Abel Vesa 1 year, 10 months ago

On 24-02-02 14:18:06, Manivannan Sadhasivam wrote:
> On Fri, Feb 02, 2024 at 10:41:03AM +0200, Abel Vesa wrote:
> > On 24-02-01 20:20:40, Konrad Dybcio wrote:
> > > On 29.01.2024 12:10, Abel Vesa wrote:
> > > > Add the compatible and the driver data for X1E80100.
> > > > 
> > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > > ---
> > > >  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > index 10f2d0bb86be..2a6000e457bc 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > @@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> > > >  	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
> > > >  	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
> > > >  	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
> > > > +	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },
> > > 
> > > I swear I'm not delaying everything related to x1 on purpose..
> > > 
> > 
> > No worries.
> > 
> > > But..
> > > 
> > > Would a "qcom,pcie-v1.9.0" generic match string be a good idea?
> > 
> > Sure. So that means this would be fallback compatible for all the following platforms:
> > 
> > - sa8540p
> > - sa8775p
> > - sc7280
> > - sc8180x
> > - sc8280xp
> > - sdx55
> > - sm8150
> > - sm8250
> > - sm8350
> > - sm8450-pcie0
> > - sm8450-pcie1
> > - sm8550
> > - x1e80100
> > 
> > Will prepare a patchset.
> > 
> 
> NO. Fallback should be based on the base SoC for this platform.

Right, so since the SM8250 is the one that has the core version 1.9.0,
should we just the sm8550 compatible as fallback for all other ones.

Yes, I know that there is SM8150, which has core version 1.5.0, but it
is still 1.9.0 compatible.

Or maybe we should rename the config to 1_5_0 and have the sm8150
compatible as fallback for all these platforms.

> 
> - Mani
> 
> -- 
> மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by Abel Vesa 1 year, 10 months ago

On 24-02-02 10:55:28, Abel Vesa wrote:
> On 24-02-02 14:18:06, Manivannan Sadhasivam wrote:
> > On Fri, Feb 02, 2024 at 10:41:03AM +0200, Abel Vesa wrote:
> > > On 24-02-01 20:20:40, Konrad Dybcio wrote:
> > > > On 29.01.2024 12:10, Abel Vesa wrote:
> > > > > Add the compatible and the driver data for X1E80100.
> > > > > 
> > > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > > > ---
> > > > >  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > index 10f2d0bb86be..2a6000e457bc 100644
> > > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > @@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> > > > >  	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
> > > > >  	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
> > > > >  	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
> > > > > +	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },
> > > > 
> > > > I swear I'm not delaying everything related to x1 on purpose..
> > > > 
> > > 
> > > No worries.
> > > 
> > > > But..
> > > > 
> > > > Would a "qcom,pcie-v1.9.0" generic match string be a good idea?
> > > 
> > > Sure. So that means this would be fallback compatible for all the following platforms:
> > > 
> > > - sa8540p
> > > - sa8775p
> > > - sc7280
> > > - sc8180x
> > > - sc8280xp
> > > - sdx55
> > > - sm8150
> > > - sm8250
> > > - sm8350
> > > - sm8450-pcie0
> > > - sm8450-pcie1
> > > - sm8550
> > > - x1e80100
> > > 
> > > Will prepare a patchset.
> > > 
> > 
> > NO. Fallback should be based on the base SoC for this platform.
> 
> Right, so since the SM8250 is the one that has the core version 1.9.0,
> should we just the sm8550 compatible as fallback for all other ones.
> 
> Yes, I know that there is SM8150, which has core version 1.5.0, but it
> is still 1.9.0 compatible.
> 
> Or maybe we should rename the config to 1_5_0 and have the sm8150
> compatible as fallback for all these platforms.
> 

Actually no, that's a bad idea. I would break DT backwards compatibility.

I'll just drop the compatible from driver and add fallback in DT for
X1E80100.

> > 
> > - Mani
> > 
> > -- 
> > மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by Manivannan Sadhasivam 1 year, 10 months ago

On Fri, Feb 02, 2024 at 11:31:50AM +0200, Abel Vesa wrote:
> On 24-02-02 10:55:28, Abel Vesa wrote:
> > On 24-02-02 14:18:06, Manivannan Sadhasivam wrote:
> > > On Fri, Feb 02, 2024 at 10:41:03AM +0200, Abel Vesa wrote:
> > > > On 24-02-01 20:20:40, Konrad Dybcio wrote:
> > > > > On 29.01.2024 12:10, Abel Vesa wrote:
> > > > > > Add the compatible and the driver data for X1E80100.
> > > > > > 
> > > > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > > > > ---
> > > > > >  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
> > > > > >  1 file changed, 1 insertion(+)
> > > > > > 
> > > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > > index 10f2d0bb86be..2a6000e457bc 100644
> > > > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > > @@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> > > > > >  	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
> > > > > >  	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
> > > > > >  	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
> > > > > > +	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },
> > > > > 
> > > > > I swear I'm not delaying everything related to x1 on purpose..
> > > > > 
> > > > 
> > > > No worries.
> > > > 
> > > > > But..
> > > > > 
> > > > > Would a "qcom,pcie-v1.9.0" generic match string be a good idea?
> > > > 
> > > > Sure. So that means this would be fallback compatible for all the following platforms:
> > > > 
> > > > - sa8540p
> > > > - sa8775p
> > > > - sc7280
> > > > - sc8180x
> > > > - sc8280xp
> > > > - sdx55
> > > > - sm8150
> > > > - sm8250
> > > > - sm8350
> > > > - sm8450-pcie0
> > > > - sm8450-pcie1
> > > > - sm8550
> > > > - x1e80100
> > > > 
> > > > Will prepare a patchset.
> > > > 
> > > 
> > > NO. Fallback should be based on the base SoC for this platform.
> > 
> > Right, so since the SM8250 is the one that has the core version 1.9.0,
> > should we just the sm8550 compatible as fallback for all other ones.
> > 
> > Yes, I know that there is SM8150, which has core version 1.5.0, but it
> > is still 1.9.0 compatible.
> > 
> > Or maybe we should rename the config to 1_5_0 and have the sm8150
> > compatible as fallback for all these platforms.
> > 
> 
> Actually no, that's a bad idea. I would break DT backwards compatibility.
> 

Yes!

> I'll just drop the compatible from driver and add fallback in DT for
> X1E80100.
> 

Sounds good.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by neil.armstrong@linaro.org 1 year, 10 months ago

On 02/02/2024 09:41, Abel Vesa wrote:
> On 24-02-01 20:20:40, Konrad Dybcio wrote:
>> On 29.01.2024 12:10, Abel Vesa wrote:
>>> Add the compatible and the driver data for X1E80100.
>>>
>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>> ---
>>>   drivers/pci/controller/dwc/pcie-qcom.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>> index 10f2d0bb86be..2a6000e457bc 100644
>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>> @@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
>>>   	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
>>>   	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
>>>   	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
>>> +	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },
>>
>> I swear I'm not delaying everything related to x1 on purpose..
>>
> 
> No worries.
> 
>> But..
>>
>> Would a "qcom,pcie-v1.9.0" generic match string be a good idea?
> 
> Sure. So that means this would be fallback compatible for all the following platforms:
> 
> - sa8540p
> - sa8775p
> - sc7280
> - sc8180x
> - sc8280xp
> - sdx55
> - sm8150
> - sm8250
> - sm8350
> - sm8450-pcie0
> - sm8450-pcie1
> - sm8550
> - x1e80100
> 
> Will prepare a patchset.

Honestly I don't know from where comes the 1_9_0 here, I didn't find a match... none of the IP version matches.

So I consider this "1_9_0" is a software implementation, not a proper IP version so I'm against using this.

But, using close cousins as fallback that are known to share 99% of IP design is ok to me, this is why I used the sm8550 as fallback because the IP *behaves* like the one in sm8550.

Neil

> 
>>
>> Konrad
>

Re: [PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by Abel Vesa 1 year, 10 months ago

On 24-02-02 09:44:23, neil.armstrong@linaro.org wrote:
> On 02/02/2024 09:41, Abel Vesa wrote:
> > On 24-02-01 20:20:40, Konrad Dybcio wrote:
> > > On 29.01.2024 12:10, Abel Vesa wrote:
> > > > Add the compatible and the driver data for X1E80100.
> > > > 
> > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > > ---
> > > >   drivers/pci/controller/dwc/pcie-qcom.c | 1 +
> > > >   1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > index 10f2d0bb86be..2a6000e457bc 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > @@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> > > >   	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
> > > >   	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
> > > >   	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
> > > > +	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },
> > > 
> > > I swear I'm not delaying everything related to x1 on purpose..
> > > 
> > 
> > No worries.
> > 
> > > But..
> > > 
> > > Would a "qcom,pcie-v1.9.0" generic match string be a good idea?
> > 
> > Sure. So that means this would be fallback compatible for all the following platforms:
> > 
> > - sa8540p
> > - sa8775p
> > - sc7280
> > - sc8180x
> > - sc8280xp
> > - sdx55
> > - sm8150
> > - sm8250
> > - sm8350
> > - sm8450-pcie0
> > - sm8450-pcie1
> > - sm8550
> > - x1e80100
> > 
> > Will prepare a patchset.
> 
> Honestly I don't know from where comes the 1_9_0 here, I didn't find a match... none of the IP version matches.
> 
> So I consider this "1_9_0" is a software implementation, not a proper IP version so I'm against using this.

This is the core version starting with SM8250. All the other ones are
compatible with it, from configuration point of view.

> 
> But, using close cousins as fallback that are known to share 99% of IP design is ok to me, this is why I used the sm8550 as fallback because the IP *behaves* like the one in sm8550.
> 
> Neil
> 
> > 
> > > 
> > > Konrad
> > 
>

Re: [PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by neil.armstrong@linaro.org 1 year, 10 months ago

On 01/02/2024 20:20, Konrad Dybcio wrote:
> On 29.01.2024 12:10, Abel Vesa wrote:
>> Add the compatible and the driver data for X1E80100.
>>
>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 10f2d0bb86be..2a6000e457bc 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
>>   	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
>>   	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
>>   	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
>> +	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },
> 
> I swear I'm not delaying everything related to x1 on purpose..
> 
> But..
> 
> Would a "qcom,pcie-v1.9.0" generic match string be a good idea?

Yes as fallback, this is why I used qcom,pcie-sm8550 as fallback for SM8650.

> 
> Konrad
>

Re: [PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by Manivannan Sadhasivam 1 year, 10 months ago

On Fri, Feb 02, 2024 at 09:13:25AM +0100, neil.armstrong@linaro.org wrote:
> On 01/02/2024 20:20, Konrad Dybcio wrote:
> > On 29.01.2024 12:10, Abel Vesa wrote:
> > > Add the compatible and the driver data for X1E80100.
> > > 
> > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > ---
> > >   drivers/pci/controller/dwc/pcie-qcom.c | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index 10f2d0bb86be..2a6000e457bc 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> > >   	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
> > >   	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
> > >   	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
> > > +	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },
> > 
> > I swear I'm not delaying everything related to x1 on purpose..
> > 
> > But..
> > 
> > Would a "qcom,pcie-v1.9.0" generic match string be a good idea?
> 
> Yes as fallback, this is why I used qcom,pcie-sm8550 as fallback for SM8650.
> 

Right. Fallback should be used here also.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by Abel Vesa 1 year, 10 months ago

On 24-02-02 14:11:57, Manivannan Sadhasivam wrote:
> On Fri, Feb 02, 2024 at 09:13:25AM +0100, neil.armstrong@linaro.org wrote:
> > On 01/02/2024 20:20, Konrad Dybcio wrote:
> > > On 29.01.2024 12:10, Abel Vesa wrote:
> > > > Add the compatible and the driver data for X1E80100.
> > > > 
> > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > > ---
> > > >   drivers/pci/controller/dwc/pcie-qcom.c | 1 +
> > > >   1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > index 10f2d0bb86be..2a6000e457bc 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > @@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> > > >   	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
> > > >   	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
> > > >   	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
> > > > +	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },
> > > 
> > > I swear I'm not delaying everything related to x1 on purpose..
> > > 
> > > But..
> > > 
> > > Would a "qcom,pcie-v1.9.0" generic match string be a good idea?
> > 
> > Yes as fallback, this is why I used qcom,pcie-sm8550 as fallback for SM8650.
> > 
> 
> Right. Fallback should be used here also.

So after digging a bit more ...

Nope. Fallback approach doesn't work for X1E80100.

The ddrss_sf_qtb clock is, on this platform, under RPMH control,
and therefore not registered by the GCC. This implies this clock cannot
be provided to the pcie controller node in DT, which implies the
bindings are different when compared to sm8550. So dedicated compatible
is needed.

So this patchset should remain as is.

> 
> - Mani
> 
> -- 
> மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 2/2] PCI: qcom: Add X1E80100 PCIe support

Posted by Manivannan Sadhasivam 1 year, 10 months ago

On Fri, Feb 02, 2024 at 12:31:32PM +0200, Abel Vesa wrote:
> On 24-02-02 14:11:57, Manivannan Sadhasivam wrote:
> > On Fri, Feb 02, 2024 at 09:13:25AM +0100, neil.armstrong@linaro.org wrote:
> > > On 01/02/2024 20:20, Konrad Dybcio wrote:
> > > > On 29.01.2024 12:10, Abel Vesa wrote:
> > > > > Add the compatible and the driver data for X1E80100.
> > > > > 
> > > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > > > ---
> > > > >   drivers/pci/controller/dwc/pcie-qcom.c | 1 +
> > > > >   1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > index 10f2d0bb86be..2a6000e457bc 100644
> > > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > @@ -1642,6 +1642,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> > > > >   	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
> > > > >   	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
> > > > >   	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
> > > > > +	{ .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 },
> > > > 
> > > > I swear I'm not delaying everything related to x1 on purpose..
> > > > 
> > > > But..
> > > > 
> > > > Would a "qcom,pcie-v1.9.0" generic match string be a good idea?
> > > 
> > > Yes as fallback, this is why I used qcom,pcie-sm8550 as fallback for SM8650.
> > > 
> > 
> > Right. Fallback should be used here also.
> 
> So after digging a bit more ...
> 
> Nope. Fallback approach doesn't work for X1E80100.
> 
> The ddrss_sf_qtb clock is, on this platform, under RPMH control,
> and therefore not registered by the GCC. This implies this clock cannot
> be provided to the pcie controller node in DT, which implies the
> bindings are different when compared to sm8550. So dedicated compatible
> is needed.
> 
> So this patchset should remain as is.
> 

Apologies! I just went with the conversation without cross checking the DT
binding. You have already listed it as a separate entry.

- Mani

-- 
மணிவண்ணன் சதாசிவம்