2024-01-29 11:11:15

by Abel Vesa

[permalink] [raw]
Subject: [PATCH v2 0/2] PCI: qcom: Add PCIe support for X1E80100

Add support for PCIe controllers found on X1E80100 platform.

Signed-off-by: Abel Vesa <[email protected]>
---
Changes in v2:
- Documented the compatible
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Abel Vesa (2):
dt-bindings: PCI: qcom: Document the X1E80100 PCIe Controller
PCI: qcom: Add X1E80100 PCIe support

.../devicetree/bindings/pci/qcom,pcie.yaml | 29 ++++++++++++++++++++++
drivers/pci/controller/dwc/pcie-qcom.c | 1 +
2 files changed, 30 insertions(+)
---
base-commit: 01af33cc9894b4489fb68fa35c40e9fe85df63dc
change-id: 20231201-x1e80100-pci-e3ad9158bb24

Best regards,
--
Abel Vesa <[email protected]>



2024-01-29 11:11:23

by Abel Vesa

[permalink] [raw]
Subject: [PATCH v2 1/2] dt-bindings: PCI: qcom: Document the X1E80100 PCIe Controller

Document the PCIe Controllers on the X1E80100 platform. They are similar
to the ones found on SM8550, but they don't have SF QTB clock.

Signed-off-by: Abel Vesa <[email protected]>
---
.../devicetree/bindings/pci/qcom,pcie.yaml | 29 ++++++++++++++++++++++
1 file changed, 29 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index a93ab3b54066..7381e38b7398 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -41,6 +41,7 @@ properties:
- qcom,pcie-sm8450-pcie0
- qcom,pcie-sm8450-pcie1
- qcom,pcie-sm8550
+ - qcom,pcie-x1e80100
- items:
- enum:
- qcom,pcie-sm8650
@@ -227,6 +228,7 @@ allOf:
- qcom,pcie-sm8450-pcie0
- qcom,pcie-sm8450-pcie1
- qcom,pcie-sm8550
+ - qcom,pcie-x1e80100
then:
properties:
reg:
@@ -826,6 +828,32 @@ allOf:
items:
- const: pci # PCIe core reset

+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,pcie-x1e80100
+ then:
+ properties:
+ clocks:
+ maxItems: 7
+ clock-names:
+ items:
+ - const: aux # Auxiliary clock
+ - const: cfg # Configuration clock
+ - const: bus_master # Master AXI clock
+ - const: bus_slave # Slave AXI clock
+ - const: slave_q2a # Slave Q2A clock
+ - const: noc_aggr # Aggre NoC PCIe AXI clock
+ - const: cnoc_sf_axi # Config NoC PCIe1 AXI clock
+ resets:
+ maxItems: 2
+ reset-names:
+ items:
+ - const: pci # PCIe core reset
+ - const: link_down # PCIe link down reset
+
- if:
properties:
compatible:
@@ -884,6 +912,7 @@ allOf:
- qcom,pcie-sm8450-pcie0
- qcom,pcie-sm8450-pcie1
- qcom,pcie-sm8550
+ - qcom,pcie-x1e80100
then:
oneOf:
- properties:

--
2.34.1


2024-01-29 11:11:49

by Abel Vesa

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

Add the compatible and the driver data for X1E80100.

Signed-off-by: Abel Vesa <[email protected]>
---
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


2024-01-29 14:21:49

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: PCI: qcom: Document the X1E80100 PCIe Controller

On 29/01/2024 12:10, Abel Vesa wrote:
> Document the PCIe Controllers on the X1E80100 platform. They are similar
> to the ones found on SM8550, but they don't have SF QTB clock.
>
> Signed-off-by: Abel Vesa <[email protected]>
> ---

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts_getmaintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, use mainline), work on fork of kernel (don't, use
mainline) or you ignore some maintainers (really don't). Just use b4 and
all the problems go away.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.

Please kindly resend and include all necessary To/Cc entries.


Best regards,
Krzysztof


2024-01-29 14:54:32

by Abel Vesa

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: PCI: qcom: Document the X1E80100 PCIe Controller

On 24-01-29 15:19:46, Krzysztof Kozlowski wrote:
> On 29/01/2024 12:10, Abel Vesa wrote:
> > Document the PCIe Controllers on the X1E80100 platform. They are similar
> > to the ones found on SM8550, but they don't have SF QTB clock.
> >
> > Signed-off-by: Abel Vesa <[email protected]>
> > ---
>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.
>
> Tools like b4 or scripts_getmaintainer.pl provide you proper list of
> people, so fix your workflow. Tools might also fail if you work on some
> ancient tree (don't, use mainline), work on fork of kernel (don't, use
> mainline) or you ignore some maintainers (really don't). Just use b4 and
> all the problems go away.
>
> You missed at least devicetree list (maybe more), so this won't be
> tested by automated tooling. Performing review on untested code might be
> a waste of time.

Oups, forgot to run "b4 prep --auto-to-cc" after I added the bindings
patch.

Sorry about that. Will resend v2.

>
> Please kindly resend and include all necessary To/Cc entries.
>
>
> Best regards,
> Krzysztof
>

2024-02-01 19:20:56

by Konrad Dybcio

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

On 29.01.2024 12:10, Abel Vesa wrote:
> Add the compatible and the driver data for X1E80100.
>
> Signed-off-by: Abel Vesa <[email protected]>
> ---
> 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

2024-02-02 08:35:38

by Neil Armstrong

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

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 <[email protected]>
>> ---
>> 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
>


2024-02-02 08:41:38

by Abel Vesa

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

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 <[email protected]>
> > ---
> > 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

2024-02-02 08:51:37

by Neil Armstrong

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

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 <[email protected]>
>>> ---
>>> 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
>


2024-02-02 08:52:42

by Manivannan Sadhasivam

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

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 <[email protected]>
> > > ---
> > > 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

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

2024-02-02 08:54:08

by Abel Vesa

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

On 24-02-02 09:44:23, [email protected] 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 <[email protected]>
> > > > ---
> > > > 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
> >
>

2024-02-02 08:54:35

by Manivannan Sadhasivam

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

On Fri, Feb 02, 2024 at 09:13:25AM +0100, [email protected] 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 <[email protected]>
> > > ---
> > > 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

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

2024-02-02 08:55:42

by Abel Vesa

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

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 <[email protected]>
> > > > ---
> > > > 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
>
> --
> மணிவண்ணன் சதாசிவம்

2024-02-02 10:11:31

by Abel Vesa

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

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 <[email protected]>
> > > > > ---
> > > > > 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
> >
> > --
> > மணிவண்ணன் சதாசிவம்

2024-02-02 10:15:20

by Manivannan Sadhasivam

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

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 <[email protected]>
> > > > > > ---
> > > > > > 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

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

2024-02-02 10:31:45

by Abel Vesa

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

On 24-02-02 14:11:57, Manivannan Sadhasivam wrote:
> On Fri, Feb 02, 2024 at 09:13:25AM +0100, [email protected] 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 <[email protected]>
> > > > ---
> > > > 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
>
> --
> மணிவண்ணன் சதாசிவம்

2024-02-02 12:44:30

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: PCI: qcom: Document the X1E80100 PCIe Controller

On Mon, Jan 29, 2024 at 01:10:26PM +0200, Abel Vesa wrote:
> Document the PCIe Controllers on the X1E80100 platform. They are similar
> to the ones found on SM8550, but they don't have SF QTB clock.
>
> Signed-off-by: Abel Vesa <[email protected]>

Acked-by: Manivannan Sadhasivam <[email protected]>

- Mani

> ---
> .../devicetree/bindings/pci/qcom,pcie.yaml | 29 ++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index a93ab3b54066..7381e38b7398 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -41,6 +41,7 @@ properties:
> - qcom,pcie-sm8450-pcie0
> - qcom,pcie-sm8450-pcie1
> - qcom,pcie-sm8550
> + - qcom,pcie-x1e80100
> - items:
> - enum:
> - qcom,pcie-sm8650
> @@ -227,6 +228,7 @@ allOf:
> - qcom,pcie-sm8450-pcie0
> - qcom,pcie-sm8450-pcie1
> - qcom,pcie-sm8550
> + - qcom,pcie-x1e80100
> then:
> properties:
> reg:
> @@ -826,6 +828,32 @@ allOf:
> items:
> - const: pci # PCIe core reset
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,pcie-x1e80100
> + then:
> + properties:
> + clocks:
> + maxItems: 7
> + clock-names:
> + items:
> + - const: aux # Auxiliary clock
> + - const: cfg # Configuration clock
> + - const: bus_master # Master AXI clock
> + - const: bus_slave # Slave AXI clock
> + - const: slave_q2a # Slave Q2A clock
> + - const: noc_aggr # Aggre NoC PCIe AXI clock
> + - const: cnoc_sf_axi # Config NoC PCIe1 AXI clock
> + resets:
> + maxItems: 2
> + reset-names:
> + items:
> + - const: pci # PCIe core reset
> + - const: link_down # PCIe link down reset
> +
> - if:
> properties:
> compatible:
> @@ -884,6 +912,7 @@ allOf:
> - qcom,pcie-sm8450-pcie0
> - qcom,pcie-sm8450-pcie1
> - qcom,pcie-sm8550
> + - qcom,pcie-x1e80100
> then:
> oneOf:
> - properties:
>
> --
> 2.34.1
>

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

2024-02-02 12:51:43

by Manivannan Sadhasivam

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

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, [email protected] 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 <[email protected]>
> > > > > ---
> > > > > 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

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

2024-02-02 12:54:49

by Manivannan Sadhasivam

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

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 <[email protected]>

Reviewed-by: Manivannan Sadhasivam <[email protected]>

- 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
>

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