diff mbox series

[v2,16/21] dt-bindings: spi: document support for SA8255p

Message ID 20240903220240.2594102-17-quic_nkela@quicinc.com (mailing list archive)
State Not Applicable
Headers show
Series [v2,01/21] dt-bindings: arm: qcom: add the SoC ID for SA8255P | expand

Commit Message

Nikunj Kela Sept. 3, 2024, 10:02 p.m. UTC
Add compatible representing spi support on SA8255p.

Clocks and interconnects are being configured in firmware VM
on SA8255p platform, therefore making them optional.

CC: Praveen Talari <quic_ptalari@quicinc.com>
Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
---
 .../bindings/spi/qcom,spi-geni-qcom.yaml      | 60 +++++++++++++++++--
 1 file changed, 56 insertions(+), 4 deletions(-)

Comments

Krzysztof Kozlowski Sept. 4, 2024, 6:34 a.m. UTC | #1
On Tue, Sep 03, 2024 at 03:02:35PM -0700, Nikunj Kela wrote:
> Add compatible representing spi support on SA8255p.
> 
> Clocks and interconnects are being configured in firmware VM
> on SA8255p platform, therefore making them optional.
> 

Please use standard email subjects, so with the PATCH keyword in the
title.  helps here to create proper versioned patches.
Another useful tool is b4. Skipping the PATCH keyword makes filtering of
emails more difficult thus making the review process less convenient.


> CC: Praveen Talari <quic_ptalari@quicinc.com>
> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
> ---
>  .../bindings/spi/qcom,spi-geni-qcom.yaml      | 60 +++++++++++++++++--
>  1 file changed, 56 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
> index 2e20ca313ec1..75b52c0a7440 100644
> --- a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
> +++ b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
> @@ -25,10 +25,45 @@ description:
>  
>  allOf:
>    - $ref: /schemas/spi/spi-controller.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,sa8255p-geni-spi

Not much improved. All my previous (v1) and other patch (i2c) comments
apply.

> +    then:
> +      required:
> +        - power-domains
> +        - power-domain-names
> +
> +      properties:
> +        power-domains:
> +          minItems: 2
> +
> +    else:
> +      required:
> +        - clocks
> +        - clock-names
> +
> +      properties:
> +        power-domains:
> +          maxItems: 1
> +
> +        interconnects:
> +          minItems: 2
> +          maxItems: 3
> +
> +        interconnect-names:
> +          minItems: 2
> +          items:
> +            - const: qup-core
> +            - const: qup-config
> +            - const: qup-memory
>  
>  properties:
>    compatible:
> -    const: qcom,geni-spi
> +    enum:
> +      - qcom,geni-spi
> +      - qcom,sa8255p-geni-spi

You have entire commit msg to explain why this device's programming
model is not compatible with existing generic compatible which must
cover all variants (because it is crazy generic).

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 4, 2024, 7:48 a.m. UTC | #2
On 04/09/2024 00:02, Nikunj Kela wrote:
> Add compatible representing spi support on SA8255p.
> 
> Clocks and interconnects are being configured in firmware VM
> on SA8255p platform, therefore making them optional.
> 
> CC: Praveen Talari <quic_ptalari@quicinc.com>
> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>

Also this is incomplete - adding compatible without driver change is not
expected. It cannot even work.

Best regards,
Krzysztof
Nikunj Kela Sept. 4, 2024, 12:48 p.m. UTC | #3
On 9/3/2024 11:34 PM, Krzysztof Kozlowski wrote:
> On Tue, Sep 03, 2024 at 03:02:35PM -0700, Nikunj Kela wrote:
>> Add compatible representing spi support on SA8255p.
>>
>> Clocks and interconnects are being configured in firmware VM
>> on SA8255p platform, therefore making them optional.
>>
> Please use standard email subjects, so with the PATCH keyword in the
> title.  helps here to create proper versioned patches.
Where did I miss PATCH keyword in the subject here? It says "[PATCH v2
16/21] dt-bindings: spi: document support for SA8255p"
> Another useful tool is b4. Skipping the PATCH keyword makes filtering of
> emails more difficult thus making the review process less convenient.
>
>
>> CC: Praveen Talari <quic_ptalari@quicinc.com>
>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
>> ---
>>  .../bindings/spi/qcom,spi-geni-qcom.yaml      | 60 +++++++++++++++++--
>>  1 file changed, 56 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
>> index 2e20ca313ec1..75b52c0a7440 100644
>> --- a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
>> +++ b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
>> @@ -25,10 +25,45 @@ description:
>>  
>>  allOf:
>>    - $ref: /schemas/spi/spi-controller.yaml#
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: qcom,sa8255p-geni-spi
> Not much improved. All my previous (v1) and other patch (i2c) comments
> apply.
>> +    then:
>> +      required:
>> +        - power-domains
>> +        - power-domain-names
>> +
>> +      properties:
>> +        power-domains:
>> +          minItems: 2
>> +
>> +    else:
>> +      required:
>> +        - clocks
>> +        - clock-names
>> +
>> +      properties:
>> +        power-domains:
>> +          maxItems: 1
>> +
>> +        interconnects:
>> +          minItems: 2
>> +          maxItems: 3
>> +
>> +        interconnect-names:
>> +          minItems: 2
>> +          items:
>> +            - const: qup-core
>> +            - const: qup-config
>> +            - const: qup-memory
>>  
>>  properties:
>>    compatible:
>> -    const: qcom,geni-spi
>> +    enum:
>> +      - qcom,geni-spi
>> +      - qcom,sa8255p-geni-spi
> You have entire commit msg to explain why this device's programming
> model is not compatible with existing generic compatible which must
> cover all variants (because it is crazy generic).
>
> Best regards,
> Krzysztof

I will put more details in the description of the patch, though, I had
put the description in the cover letter for this entire series.


>
Nikunj Kela Sept. 4, 2024, 12:49 p.m. UTC | #4
On 9/4/2024 12:48 AM, Krzysztof Kozlowski wrote:
> On 04/09/2024 00:02, Nikunj Kela wrote:
>> Add compatible representing spi support on SA8255p.
>>
>> Clocks and interconnects are being configured in firmware VM
>> on SA8255p platform, therefore making them optional.
>>
>> CC: Praveen Talari <quic_ptalari@quicinc.com>
>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
> Also this is incomplete - adding compatible without driver change is not
> expected. It cannot even work.
>
> Best regards,
> Krzysztof

Link for CLO branch is provided in I2C patch series. The driver changes
will soon follow.
Krzysztof Kozlowski Sept. 4, 2024, 1:21 p.m. UTC | #5
On 04/09/2024 14:48, Nikunj Kela wrote:
> 
> On 9/3/2024 11:34 PM, Krzysztof Kozlowski wrote:
>> On Tue, Sep 03, 2024 at 03:02:35PM -0700, Nikunj Kela wrote:
>>> Add compatible representing spi support on SA8255p.
>>>
>>> Clocks and interconnects are being configured in firmware VM
>>> on SA8255p platform, therefore making them optional.
>>>
>> Please use standard email subjects, so with the PATCH keyword in the
>> title.  helps here to create proper versioned patches.
> Where did I miss PATCH keyword in the subject here? It says "[PATCH v2
> 16/21] dt-bindings: spi: document support for SA8255p"

Oh, wrong template. It was about spi prefix, should be this one:

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

>>

Best regards,
Krzysztof
Nikunj Kela Sept. 4, 2024, 4:14 p.m. UTC | #6
On 9/4/2024 6:21 AM, Krzysztof Kozlowski wrote:
> On 04/09/2024 14:48, Nikunj Kela wrote:
>> On 9/3/2024 11:34 PM, Krzysztof Kozlowski wrote:
>>> On Tue, Sep 03, 2024 at 03:02:35PM -0700, Nikunj Kela wrote:
>>>> Add compatible representing spi support on SA8255p.
>>>>
>>>> Clocks and interconnects are being configured in firmware VM
>>>> on SA8255p platform, therefore making them optional.
>>>>
>>> Please use standard email subjects, so with the PATCH keyword in the
>>> title.  helps here to create proper versioned patches.
>> Where did I miss PATCH keyword in the subject here? It says "[PATCH v2
>> 16/21] dt-bindings: spi: document support for SA8255p"
> Oh, wrong template. It was about spi prefix, should be this one:

Sorry, didn't realize SPI uses different subject format than other
subsystems. Will fix in v3. Thanks


> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
>
> Best regards,
> Krzysztof
>
Andrew Lunn Sept. 4, 2024, 4:58 p.m. UTC | #7
> Sorry, didn't realize SPI uses different subject format than other
> subsystems. Will fix in v3. Thanks

Each subsystem is free to use its own form. e.g for netdev you will
want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:

This is another reason why you should be splitting these patches per
subsystem, and submitting both the DT bindings and the code changes as
a two patch patchset. You can then learn how each subsystem names its
patches.

Please pick one victim subsystem and work on the patches for just that
subsystem. Once you have them correct, you can use everything you
learned to fixup all your other patches, one by one.

	Andrew
Nikunj Kela Sept. 4, 2024, 9:06 p.m. UTC | #8
On 9/4/2024 9:58 AM, Andrew Lunn wrote:
>> Sorry, didn't realize SPI uses different subject format than other
>> subsystems. Will fix in v3. Thanks
> Each subsystem is free to use its own form. e.g for netdev you will
> want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:
of course they are! No one is disputing that.
>
> This is another reason why you should be splitting these patches per
> subsystem, and submitting both the DT bindings and the code changes as
> a two patch patchset. You can then learn how each subsystem names its
> patches.

Qualcomm QUPs chips have serial engines that can be configured as
UART/I2C/SPI so QUPs changes require to be pushed in one series for all
3 subsystems as they all are dependent.


>
> Please pick one victim subsystem and work on the patches for just that
> subsystem. Once you have them correct, you can use everything you
> learned to fixup all your other patches, one by one.
>
> 	Andrew
Andrew Lunn Sept. 4, 2024, 9:49 p.m. UTC | #9
> Qualcomm QUPs chips have serial engines that can be configured as
> UART/I2C/SPI so QUPs changes require to be pushed in one series for all
> 3 subsystems as they all are dependent.

So leave that until later. And when you do, explicit mention why you
are cross posting to three subsystems, because the hardware is
designed like that. And suggest a way it could be merged, which
subsystem should take the lead, and the others just need to provide
Acked-by. The Maintainers might disagree, want to do it differently,
but i find it always helps to state this from the beginning, otherwise
sometimes no Maintainer take the lead role.

But this patchset appears to be much more than QUPs. You should be
able the break the rest up into smaller patchsets, one per subsystem.

	Andrew
Krzysztof Kozlowski Sept. 5, 2024, 8:04 a.m. UTC | #10
On 04/09/2024 23:06, Nikunj Kela wrote:
> 
> On 9/4/2024 9:58 AM, Andrew Lunn wrote:
>>> Sorry, didn't realize SPI uses different subject format than other
>>> subsystems. Will fix in v3. Thanks
>> Each subsystem is free to use its own form. e.g for netdev you will
>> want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:
> of course they are! No one is disputing that.
>>
>> This is another reason why you should be splitting these patches per
>> subsystem, and submitting both the DT bindings and the code changes as
>> a two patch patchset. You can then learn how each subsystem names its
>> patches.
> 
> Qualcomm QUPs chips have serial engines that can be configured as
> UART/I2C/SPI so QUPs changes require to be pushed in one series for all
> 3 subsystems as they all are dependent.

No, they are not dependent. They have never been. Look how all other
upstreaming process worked in the past.


Best regards,
Krzysztof
Dmitry Baryshkov Sept. 5, 2024, 1:21 p.m. UTC | #11
On Wed, Sep 04, 2024 at 05:48:35AM GMT, Nikunj Kela wrote:
> 
> On 9/3/2024 11:34 PM, Krzysztof Kozlowski wrote:
> > On Tue, Sep 03, 2024 at 03:02:35PM -0700, Nikunj Kela wrote:
> >> Add compatible representing spi support on SA8255p.
> >>
> >> Clocks and interconnects are being configured in firmware VM
> >> on SA8255p platform, therefore making them optional.
> >>
> > Please use standard email subjects, so with the PATCH keyword in the
> > title.  helps here to create proper versioned patches.
> Where did I miss PATCH keyword in the subject here? It says "[PATCH v2
> 16/21] dt-bindings: spi: document support for SA8255p"
> > Another useful tool is b4. Skipping the PATCH keyword makes filtering of
> > emails more difficult thus making the review process less convenient.
> >
> >
> >> CC: Praveen Talari <quic_ptalari@quicinc.com>
> >> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
> >> ---
> >>  .../bindings/spi/qcom,spi-geni-qcom.yaml      | 60 +++++++++++++++++--
> >>  1 file changed, 56 insertions(+), 4 deletions(-)

> >>  
> >>  properties:
> >>    compatible:
> >> -    const: qcom,geni-spi
> >> +    enum:
> >> +      - qcom,geni-spi
> >> +      - qcom,sa8255p-geni-spi
> > You have entire commit msg to explain why this device's programming
> > model is not compatible with existing generic compatible which must
> > cover all variants (because it is crazy generic).
> >
> > Best regards,
> > Krzysztof
> 
> I will put more details in the description of the patch, though, I had
> put the description in the cover letter for this entire series.

Cover letters do not land in the git repo, so the next person coming to
perform modifications can not understand what was so special about this
platform. Please always provide all reasoning for a change in the commit
message.
Dmitry Baryshkov Sept. 5, 2024, 1:22 p.m. UTC | #12
On Wed, Sep 04, 2024 at 05:49:40AM GMT, Nikunj Kela wrote:
> 
> On 9/4/2024 12:48 AM, Krzysztof Kozlowski wrote:
> > On 04/09/2024 00:02, Nikunj Kela wrote:
> >> Add compatible representing spi support on SA8255p.
> >>
> >> Clocks and interconnects are being configured in firmware VM
> >> on SA8255p platform, therefore making them optional.
> >>
> >> CC: Praveen Talari <quic_ptalari@quicinc.com>
> >> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
> > Also this is incomplete - adding compatible without driver change is not
> > expected. It cannot even work.
> >
> > Best regards,
> > Krzysztof
> 
> Link for CLO branch is provided in I2C patch series. The driver changes
> will soon follow.

So, what's the point of posting the dt-bindings without corresponding
driver changes?
Nikunj Kela Sept. 5, 2024, 2:03 p.m. UTC | #13
On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote:
> On 04/09/2024 23:06, Nikunj Kela wrote:
>> On 9/4/2024 9:58 AM, Andrew Lunn wrote:
>>>> Sorry, didn't realize SPI uses different subject format than other
>>>> subsystems. Will fix in v3. Thanks
>>> Each subsystem is free to use its own form. e.g for netdev you will
>>> want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:
>> of course they are! No one is disputing that.
>>> This is another reason why you should be splitting these patches per
>>> subsystem, and submitting both the DT bindings and the code changes as
>>> a two patch patchset. You can then learn how each subsystem names its
>>> patches.
>> Qualcomm QUPs chips have serial engines that can be configured as
>> UART/I2C/SPI so QUPs changes require to be pushed in one series for all
>> 3 subsystems as they all are dependent.
> No, they are not dependent. They have never been. Look how all other
> upstreaming process worked in the past.

Top level QUP node(patch#18) includes i2c,spi,uart nodes.
soc/qcom/qcom,geni-se.yaml validate those subnodes against respective
yaml. The example that is added in YAML file for QUP node will not find
sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not
included in the same series.


>
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Sept. 5, 2024, 2:09 p.m. UTC | #14
On 05/09/2024 16:03, Nikunj Kela wrote:
> 
> On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote:
>> On 04/09/2024 23:06, Nikunj Kela wrote:
>>> On 9/4/2024 9:58 AM, Andrew Lunn wrote:
>>>>> Sorry, didn't realize SPI uses different subject format than other
>>>>> subsystems. Will fix in v3. Thanks
>>>> Each subsystem is free to use its own form. e.g for netdev you will
>>>> want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:
>>> of course they are! No one is disputing that.
>>>> This is another reason why you should be splitting these patches per
>>>> subsystem, and submitting both the DT bindings and the code changes as
>>>> a two patch patchset. You can then learn how each subsystem names its
>>>> patches.
>>> Qualcomm QUPs chips have serial engines that can be configured as
>>> UART/I2C/SPI so QUPs changes require to be pushed in one series for all
>>> 3 subsystems as they all are dependent.
>> No, they are not dependent. They have never been. Look how all other
>> upstreaming process worked in the past.
> 
> Top level QUP node(patch#18) includes i2c,spi,uart nodes.
> soc/qcom/qcom,geni-se.yaml validate those subnodes against respective
> yaml. The example that is added in YAML file for QUP node will not find
> sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not
> included in the same series.
> 

So where is the dependency? I don't see it. Anyway, if you insist,
provide reasons why this should be the only one patchset - from all
SoCs, all companies, all developers - getting an exception from standard
merging practice and from explicit rule about driver change. See
submitting bindings.

This was re-iterated over and over, but you keep claiming you need some
sort of special treatment. If so, please provide arguments WHY this
requires special treatment and *all* other contributions are fine with it.

Best regards,
Krzysztof
Nikunj Kela Sept. 5, 2024, 2:15 p.m. UTC | #15
On 9/5/2024 7:09 AM, Krzysztof Kozlowski wrote:
> On 05/09/2024 16:03, Nikunj Kela wrote:
>> On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote:
>>> On 04/09/2024 23:06, Nikunj Kela wrote:
>>>> On 9/4/2024 9:58 AM, Andrew Lunn wrote:
>>>>>> Sorry, didn't realize SPI uses different subject format than other
>>>>>> subsystems. Will fix in v3. Thanks
>>>>> Each subsystem is free to use its own form. e.g for netdev you will
>>>>> want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:
>>>> of course they are! No one is disputing that.
>>>>> This is another reason why you should be splitting these patches per
>>>>> subsystem, and submitting both the DT bindings and the code changes as
>>>>> a two patch patchset. You can then learn how each subsystem names its
>>>>> patches.
>>>> Qualcomm QUPs chips have serial engines that can be configured as
>>>> UART/I2C/SPI so QUPs changes require to be pushed in one series for all
>>>> 3 subsystems as they all are dependent.
>>> No, they are not dependent. They have never been. Look how all other
>>> upstreaming process worked in the past.
>> Top level QUP node(patch#18) includes i2c,spi,uart nodes.
>> soc/qcom/qcom,geni-se.yaml validate those subnodes against respective
>> yaml. The example that is added in YAML file for QUP node will not find
>> sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not
>> included in the same series.
>>
> So where is the dependency? I don't see it. 

Ok, what is your suggestion on dt-schema check failure in that case as I
mentioned above? Shall we remove examples from yaml that we added?


> Anyway, if you insist,
> provide reasons why this should be the only one patchset - from all
> SoCs, all companies, all developers - getting an exception from standard
> merging practice and from explicit rule about driver change. See
> submitting bindings.
>
> This was re-iterated over and over, but you keep claiming you need some
> sort of special treatment. If so, please provide arguments WHY this
> requires special treatment and *all* other contributions are fine with it.
>
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Sept. 5, 2024, 2:39 p.m. UTC | #16
On 05/09/2024 16:15, Nikunj Kela wrote:
> 
> On 9/5/2024 7:09 AM, Krzysztof Kozlowski wrote:
>> On 05/09/2024 16:03, Nikunj Kela wrote:
>>> On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote:
>>>> On 04/09/2024 23:06, Nikunj Kela wrote:
>>>>> On 9/4/2024 9:58 AM, Andrew Lunn wrote:
>>>>>>> Sorry, didn't realize SPI uses different subject format than other
>>>>>>> subsystems. Will fix in v3. Thanks
>>>>>> Each subsystem is free to use its own form. e.g for netdev you will
>>>>>> want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:
>>>>> of course they are! No one is disputing that.
>>>>>> This is another reason why you should be splitting these patches per
>>>>>> subsystem, and submitting both the DT bindings and the code changes as
>>>>>> a two patch patchset. You can then learn how each subsystem names its
>>>>>> patches.
>>>>> Qualcomm QUPs chips have serial engines that can be configured as
>>>>> UART/I2C/SPI so QUPs changes require to be pushed in one series for all
>>>>> 3 subsystems as they all are dependent.
>>>> No, they are not dependent. They have never been. Look how all other
>>>> upstreaming process worked in the past.
>>> Top level QUP node(patch#18) includes i2c,spi,uart nodes.
>>> soc/qcom/qcom,geni-se.yaml validate those subnodes against respective
>>> yaml. The example that is added in YAML file for QUP node will not find
>>> sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not
>>> included in the same series.
>>>
>> So where is the dependency? I don't see it. 
> 
> Ok, what is your suggestion on dt-schema check failure in that case as I
> mentioned above? Shall we remove examples from yaml that we added?

I don't understand what sort of failure you want to fix and why examples
have any problem here. I said it multiple times already but I think you
never confirmed. Do you understand how patches are merged? That they go
via different trees but everything must be 100% bisectable?

Best regards,
Krzysztof
Andrew Lunn Sept. 5, 2024, 2:46 p.m. UTC | #17
> Ok, what is your suggestion on dt-schema check failure in that case as I
> mentioned above? Shall we remove examples from yaml that we added?

As Krzysztof keeps saying, Commit message. You have an unlimited
amount of space to document why this SoC is special, how it is
special, maybe include some ASCII art showing how it is special.
Justify it being special. Once it is clear it is special, has
dependencies which are real, we are likely to accept the patches. We
know SoC vendors do weird things, and sometimes mainline processes
just don't work. But you need to clear, upfront, and state, the
process does not work because... in your commit message. Maybe put it
below the ---.

Something i often say to Mainline newbies. The code is easy, it is the
processes which are hard. The commit message is part of the
process. You want to try to anticipate all the questions Reviewers are
going to ask and answer them in the commit message, before they ask
them. It is process that you split patches by subsystem. It is process
that binding changes and driver changes go together in the
patchset. Your 'code review' should include all this, not just the
lines of actual code. And to begin with, process is probably a lot
more important than the actual code. So please concentrate on
processes, get them right.

	Andrew
Krzysztof Kozlowski Sept. 5, 2024, 2:49 p.m. UTC | #18
On 05/09/2024 16:15, Nikunj Kela wrote:
> 
> On 9/5/2024 7:09 AM, Krzysztof Kozlowski wrote:
>> On 05/09/2024 16:03, Nikunj Kela wrote:
>>> On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote:
>>>> On 04/09/2024 23:06, Nikunj Kela wrote:
>>>>> On 9/4/2024 9:58 AM, Andrew Lunn wrote:
>>>>>>> Sorry, didn't realize SPI uses different subject format than other
>>>>>>> subsystems. Will fix in v3. Thanks
>>>>>> Each subsystem is free to use its own form. e.g for netdev you will
>>>>>> want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:
>>>>> of course they are! No one is disputing that.
>>>>>> This is another reason why you should be splitting these patches per
>>>>>> subsystem, and submitting both the DT bindings and the code changes as
>>>>>> a two patch patchset. You can then learn how each subsystem names its
>>>>>> patches.
>>>>> Qualcomm QUPs chips have serial engines that can be configured as
>>>>> UART/I2C/SPI so QUPs changes require to be pushed in one series for all
>>>>> 3 subsystems as they all are dependent.
>>>> No, they are not dependent. They have never been. Look how all other
>>>> upstreaming process worked in the past.
>>> Top level QUP node(patch#18) includes i2c,spi,uart nodes.
>>> soc/qcom/qcom,geni-se.yaml validate those subnodes against respective
>>> yaml. The example that is added in YAML file for QUP node will not find
>>> sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not
>>> included in the same series.
>>>
>> So where is the dependency? I don't see it. 
> 
> Ok, what is your suggestion on dt-schema check failure in that case as I
> mentioned above? Shall we remove examples from yaml that we added?
> 
> 
>> Anyway, if you insist,
>> provide reasons why this should be the only one patchset - from all
>> SoCs, all companies, all developers - getting an exception from standard
>> merging practice and from explicit rule about driver change. See
>> submitting bindings.
>>
>> This was re-iterated over and over, but you keep claiming you need some
>> sort of special treatment. If so, please provide arguments WHY this
>> requires special treatment and *all* other contributions are fine with it.

You did not respond to above about explaining why this patchset needs
special treatment, so I assume there is no exception here to be granted
so any new version will follow standard process (see submitting bindings
/ writing bindings).

Best regards,
Krzysztof
Nikunj Kela Sept. 5, 2024, 3:43 p.m. UTC | #19
On 9/5/2024 7:49 AM, Krzysztof Kozlowski wrote:
> On 05/09/2024 16:15, Nikunj Kela wrote:
>> On 9/5/2024 7:09 AM, Krzysztof Kozlowski wrote:
>>> On 05/09/2024 16:03, Nikunj Kela wrote:
>>>> On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote:
>>>>> On 04/09/2024 23:06, Nikunj Kela wrote:
>>>>>> On 9/4/2024 9:58 AM, Andrew Lunn wrote:
>>>>>>>> Sorry, didn't realize SPI uses different subject format than other
>>>>>>>> subsystems. Will fix in v3. Thanks
>>>>>>> Each subsystem is free to use its own form. e.g for netdev you will
>>>>>>> want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:
>>>>>> of course they are! No one is disputing that.
>>>>>>> This is another reason why you should be splitting these patches per
>>>>>>> subsystem, and submitting both the DT bindings and the code changes as
>>>>>>> a two patch patchset. You can then learn how each subsystem names its
>>>>>>> patches.
>>>>>> Qualcomm QUPs chips have serial engines that can be configured as
>>>>>> UART/I2C/SPI so QUPs changes require to be pushed in one series for all
>>>>>> 3 subsystems as they all are dependent.
>>>>> No, they are not dependent. They have never been. Look how all other
>>>>> upstreaming process worked in the past.
>>>> Top level QUP node(patch#18) includes i2c,spi,uart nodes.
>>>> soc/qcom/qcom,geni-se.yaml validate those subnodes against respective
>>>> yaml. The example that is added in YAML file for QUP node will not find
>>>> sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not
>>>> included in the same series.
>>>>
>>> So where is the dependency? I don't see it. 
>> Ok, what is your suggestion on dt-schema check failure in that case as I
>> mentioned above? Shall we remove examples from yaml that we added?
>>
>>
>>> Anyway, if you insist,
>>> provide reasons why this should be the only one patchset - from all
>>> SoCs, all companies, all developers - getting an exception from standard
>>> merging practice and from explicit rule about driver change. See
>>> submitting bindings.
>>>
>>> This was re-iterated over and over, but you keep claiming you need some
>>> sort of special treatment. If so, please provide arguments WHY this
>>> requires special treatment and *all* other contributions are fine with it.
> You did not respond to above about explaining why this patchset needs
> special treatment, so I assume there is no exception here to be granted
> so any new version will follow standard process (see submitting bindings
> / writing bindings).
>
> Best regards,
> Krzysztof

Things will be clear after you see the driver changes. Without looking
at the code, this discussion won't lead to anything constructive. So I
deferred the QUP related discussion until driver patches are posted.

Thanks,

-Nikunj


>
Nikunj Kela Sept. 5, 2024, 4:08 p.m. UTC | #20
On 9/5/2024 7:39 AM, Krzysztof Kozlowski wrote:
> On 05/09/2024 16:15, Nikunj Kela wrote:
>> On 9/5/2024 7:09 AM, Krzysztof Kozlowski wrote:
>>> On 05/09/2024 16:03, Nikunj Kela wrote:
>>>> On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote:
>>>>> On 04/09/2024 23:06, Nikunj Kela wrote:
>>>>>> On 9/4/2024 9:58 AM, Andrew Lunn wrote:
>>>>>>>> Sorry, didn't realize SPI uses different subject format than other
>>>>>>>> subsystems. Will fix in v3. Thanks
>>>>>>> Each subsystem is free to use its own form. e.g for netdev you will
>>>>>>> want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:
>>>>>> of course they are! No one is disputing that.
>>>>>>> This is another reason why you should be splitting these patches per
>>>>>>> subsystem, and submitting both the DT bindings and the code changes as
>>>>>>> a two patch patchset. You can then learn how each subsystem names its
>>>>>>> patches.
>>>>>> Qualcomm QUPs chips have serial engines that can be configured as
>>>>>> UART/I2C/SPI so QUPs changes require to be pushed in one series for all
>>>>>> 3 subsystems as they all are dependent.
>>>>> No, they are not dependent. They have never been. Look how all other
>>>>> upstreaming process worked in the past.
>>>> Top level QUP node(patch#18) includes i2c,spi,uart nodes.
>>>> soc/qcom/qcom,geni-se.yaml validate those subnodes against respective
>>>> yaml. The example that is added in YAML file for QUP node will not find
>>>> sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not
>>>> included in the same series.
>>>>
>>> So where is the dependency? I don't see it. 
>> Ok, what is your suggestion on dt-schema check failure in that case as I
>> mentioned above? Shall we remove examples from yaml that we added?
> I don't understand what sort of failure you want to fix and why examples
> have any problem here. 

If the QUPs yaml changes are not included in the same series with
i2c,serial yaml changes, you see these errors:

/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:compatible:0: 'qcom,sa8255p-geni-uart' is not one of ['qcom,geni-uart', 'qcom,geni-debug-uart']
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000:compatible:0: 'qcom,sa8255p-geni-i2c' is not one of ['qcom,geni-i2c', 'qcom,geni-i2c-master-hub']

> I said it multiple times already but I think you
> never confirmed. Do you understand how patches are merged? That they go
> via different trees but everything must be 100% bisectable?
>
> Best regards,
> Krzysztof
>
Andrew Lunn Sept. 5, 2024, 4:23 p.m. UTC | #21
> If the QUPs yaml changes are not included in the same series with
> i2c,serial yaml changes, you see these errors:
> 
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:compatible:0: 'qcom,sa8255p-geni-uart' is not one of ['qcom,geni-uart', 'qcom,geni-debug-uart']
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000:compatible:0: 'qcom,sa8255p-geni-i2c' is not one of ['qcom,geni-i2c', 'qcom,geni-i2c-master-hub']

So you have a couple of options:

1) It sounds like you should get the QUP changes merged first. Then
   submit the i2c,serial changes. Is there a reason you cannot do
   this? Is there a mutual dependency between these two series, or
   just a one way dependency?

2) Explain in the commit message that following errors are expected
   because ... And explain in detail why the dependency cannot be
   broken to avoid the errors.

Andrew
Nikunj Kela Sept. 5, 2024, 4:39 p.m. UTC | #22
On 9/5/2024 9:23 AM, Andrew Lunn wrote:
>> If the QUPs yaml changes are not included in the same series with
>> i2c,serial yaml changes, you see these errors:
>>
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:compatible:0: 'qcom,sa8255p-geni-uart' is not one of ['qcom,geni-uart', 'qcom,geni-debug-uart']
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000:compatible:0: 'qcom,sa8255p-geni-i2c' is not one of ['qcom,geni-i2c', 'qcom,geni-i2c-master-hub']
> So you have a couple of options:
>
> 1) It sounds like you should get the QUP changes merged first. Then
>    submit the i2c,serial changes. Is there a reason you cannot do
>    this? Is there a mutual dependency between these two series, or
>    just a one way dependency?

The ask in this thread is to create new yaml files since existing one is
using generic compatibles. With new yaml, we would need to provide
example and can't avoid it. If we have to provide example of QUP node,
IMO, we should provide a few subnodes as well since just QUP node
without subnodes(i2c/serial/spi)  will not be very useful.

We can possibly skip all 3 subnode and only keep one subsystem(e.g.
serial) so QUP and UART yaml can go together(still need two subsystems)
while SPI and I2C can go independently after QUP series is accepted. Not
sure if that is acceptable to maintainers though. QUP node in actual DT
will have all 3 types of subnodes(i2c,spi, serial) so example in this
case won't be complete.

>
> 2) Explain in the commit message that following errors are expected
>    because ... And explain in detail why the dependency cannot be
>    broken to avoid the errors.
>
> Andrew
Krzysztof Kozlowski Sept. 5, 2024, 4:56 p.m. UTC | #23
On 05/09/2024 18:08, Nikunj Kela wrote:
> 
> On 9/5/2024 7:39 AM, Krzysztof Kozlowski wrote:
>> On 05/09/2024 16:15, Nikunj Kela wrote:
>>> On 9/5/2024 7:09 AM, Krzysztof Kozlowski wrote:
>>>> On 05/09/2024 16:03, Nikunj Kela wrote:
>>>>> On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote:
>>>>>> On 04/09/2024 23:06, Nikunj Kela wrote:
>>>>>>> On 9/4/2024 9:58 AM, Andrew Lunn wrote:
>>>>>>>>> Sorry, didn't realize SPI uses different subject format than other
>>>>>>>>> subsystems. Will fix in v3. Thanks
>>>>>>>> Each subsystem is free to use its own form. e.g for netdev you will
>>>>>>>> want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:
>>>>>>> of course they are! No one is disputing that.
>>>>>>>> This is another reason why you should be splitting these patches per
>>>>>>>> subsystem, and submitting both the DT bindings and the code changes as
>>>>>>>> a two patch patchset. You can then learn how each subsystem names its
>>>>>>>> patches.
>>>>>>> Qualcomm QUPs chips have serial engines that can be configured as
>>>>>>> UART/I2C/SPI so QUPs changes require to be pushed in one series for all
>>>>>>> 3 subsystems as they all are dependent.
>>>>>> No, they are not dependent. They have never been. Look how all other
>>>>>> upstreaming process worked in the past.
>>>>> Top level QUP node(patch#18) includes i2c,spi,uart nodes.
>>>>> soc/qcom/qcom,geni-se.yaml validate those subnodes against respective
>>>>> yaml. The example that is added in YAML file for QUP node will not find
>>>>> sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not
>>>>> included in the same series.
>>>>>
>>>> So where is the dependency? I don't see it. 
>>> Ok, what is your suggestion on dt-schema check failure in that case as I
>>> mentioned above? Shall we remove examples from yaml that we added?
>> I don't understand what sort of failure you want to fix and why examples
>> have any problem here. 
> 
> If the QUPs yaml changes are not included in the same series with

They cannot be included in the same series. You just think that
including here solves the problem so go ahead, simulate the merging:
1. Bjorn applies soc/qcom/qcom,geni-se.yaml patch and tests. His tree
MUST build, so it also must pass dt_binding_check.
Does it pass? No.

2. SPI maintainer... ah, no point even going there.

> i2c,serial yaml changes, you see these errors:
> 
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:compatible:0: 'qcom,sa8255p-geni-uart' is not one of ['qcom,geni-uart', 'qcom,geni-debug-uart']
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000:compatible:0: 'qcom,sa8255p-geni-i2c' is not one of ['qcom,geni-i2c', 'qcom,geni-i2c-master-hub']

Don't grow examples if not needed. Or create dependencies and ask
maintainers to cross-merge.

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 5, 2024, 5 p.m. UTC | #24
On 05/09/2024 18:56, Krzysztof Kozlowski wrote:
> On 05/09/2024 18:08, Nikunj Kela wrote:
>>
>> On 9/5/2024 7:39 AM, Krzysztof Kozlowski wrote:
>>> On 05/09/2024 16:15, Nikunj Kela wrote:
>>>> On 9/5/2024 7:09 AM, Krzysztof Kozlowski wrote:
>>>>> On 05/09/2024 16:03, Nikunj Kela wrote:
>>>>>> On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote:
>>>>>>> On 04/09/2024 23:06, Nikunj Kela wrote:
>>>>>>>> On 9/4/2024 9:58 AM, Andrew Lunn wrote:
>>>>>>>>>> Sorry, didn't realize SPI uses different subject format than other
>>>>>>>>>> subsystems. Will fix in v3. Thanks
>>>>>>>>> Each subsystem is free to use its own form. e.g for netdev you will
>>>>>>>>> want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:
>>>>>>>> of course they are! No one is disputing that.
>>>>>>>>> This is another reason why you should be splitting these patches per
>>>>>>>>> subsystem, and submitting both the DT bindings and the code changes as
>>>>>>>>> a two patch patchset. You can then learn how each subsystem names its
>>>>>>>>> patches.
>>>>>>>> Qualcomm QUPs chips have serial engines that can be configured as
>>>>>>>> UART/I2C/SPI so QUPs changes require to be pushed in one series for all
>>>>>>>> 3 subsystems as they all are dependent.
>>>>>>> No, they are not dependent. They have never been. Look how all other
>>>>>>> upstreaming process worked in the past.
>>>>>> Top level QUP node(patch#18) includes i2c,spi,uart nodes.
>>>>>> soc/qcom/qcom,geni-se.yaml validate those subnodes against respective
>>>>>> yaml. The example that is added in YAML file for QUP node will not find
>>>>>> sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not
>>>>>> included in the same series.
>>>>>>
>>>>> So where is the dependency? I don't see it. 
>>>> Ok, what is your suggestion on dt-schema check failure in that case as I
>>>> mentioned above? Shall we remove examples from yaml that we added?
>>> I don't understand what sort of failure you want to fix and why examples
>>> have any problem here. 
>>
>> If the QUPs yaml changes are not included in the same series with
> 
> They cannot be included in the same series. You just think that
> including here solves the problem so go ahead, simulate the merging:
> 1. Bjorn applies soc/qcom/qcom,geni-se.yaml patch and tests. His tree
> MUST build, so it also must pass dt_binding_check.
> Does it pass? No.
> 
> 2. SPI maintainer... ah, no point even going there.
> 
>> i2c,serial yaml changes, you see these errors:
>>
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:compatible:0: 'qcom,sa8255p-geni-uart' is not one of ['qcom,geni-uart', 'qcom,geni-debug-uart']
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000:compatible:0: 'qcom,sa8255p-geni-i2c' is not one of ['qcom,geni-i2c', 'qcom,geni-i2c-master-hub']
> 
> Don't grow examples if not needed. Or create dependencies and ask
> maintainers to cross-merge.

Or soc/geni-se binding could be also converted to just list compatibles
instead of referencing other schema, just like MDSS.

Best regards,
Krzysztof
Andrew Lunn Sept. 5, 2024, 5:35 p.m. UTC | #25
On Thu, Sep 05, 2024 at 09:39:54AM -0700, Nikunj Kela wrote:
> 
> On 9/5/2024 9:23 AM, Andrew Lunn wrote:
> >> If the QUPs yaml changes are not included in the same series with
> >> i2c,serial yaml changes, you see these errors:
> >>
> >> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:compatible:0: 'qcom,sa8255p-geni-uart' is not one of ['qcom,geni-uart', 'qcom,geni-debug-uart']
> >> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000:compatible:0: 'qcom,sa8255p-geni-i2c' is not one of ['qcom,geni-i2c', 'qcom,geni-i2c-master-hub']
> > So you have a couple of options:
> >
> > 1) It sounds like you should get the QUP changes merged first. Then
> >    submit the i2c,serial changes. Is there a reason you cannot do
> >    this? Is there a mutual dependency between these two series, or
> >    just a one way dependency?
> 
> The ask in this thread is to create new yaml files since existing one is
> using generic compatibles. With new yaml, we would need to provide
> example and can't avoid it. If we have to provide example of QUP node,
> IMO, we should provide a few subnodes as well since just QUP node
> without subnodes(i2c/serial/spi)  will not be very useful.

Does it need to be useful, at the beginning? Was the development done
all at once, i2c, serial and spi all mixed together, inseparable? More
likely, you have a set of patches adding some sort of base, and
hopefully a DT binding patch for that base. Then you add a driver in
drivers/tty/serial, with patches which extend the DT binding with the
serial port. You then add a driver in driver/i2c/busses and extend the
DT binding for I2C. And then add a driver for SPI in drivers/spi,
which again extends the DT binding?

This would be typical for how an MFD would be posted. Please go search
the lists for examples of MFDs you might be able to follow.

	Andrew
Nikunj Kela Sept. 9, 2024, 8:29 p.m. UTC | #26
On 9/4/2024 6:21 AM, Krzysztof Kozlowski wrote:
> On 04/09/2024 14:48, Nikunj Kela wrote:
>> On 9/3/2024 11:34 PM, Krzysztof Kozlowski wrote:
>>> On Tue, Sep 03, 2024 at 03:02:35PM -0700, Nikunj Kela wrote:
>>>> Add compatible representing spi support on SA8255p.
>>>>
>>>> Clocks and interconnects are being configured in firmware VM
>>>> on SA8255p platform, therefore making them optional.
>>>>
>>> Please use standard email subjects, so with the PATCH keyword in the
>>> title.  helps here to create proper versioned patches.
>> Where did I miss PATCH keyword in the subject here? It says "[PATCH v2
>> 16/21] dt-bindings: spi: document support for SA8255p"
> Oh, wrong template. It was about spi prefix, 

These are the latest 4 commits in linux-next for spi:

12736adc43b7 dt-bindings: spi: nxp-fspi: add imx8ulp support
b0cdf9cc0895 spi: dt-bindings: Add rockchip,rk3576-spi compatible
d6d0af1b9eff dt-bindings: spi: add PIC64GX SPI/QSPI compatibility to
MPFS SPI/QSPI bindings
1c4d834e4e81 spi: dt-bindings: convert spi-sc18is602.txt to yaml format

Now I am confused which prefix format shall I use? first spi or first
dt-bindings?


> should be this one:
>
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
>
> Best regards,
> Krzysztof
>
Mark Brown Sept. 9, 2024, 10 p.m. UTC | #27
On Mon, Sep 09, 2024 at 01:29:37PM -0700, Nikunj Kela wrote:

> Now I am confused which prefix format shall I use? first spi or first
> dt-bindings?

spi: first.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
index 2e20ca313ec1..75b52c0a7440 100644
--- a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
+++ b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
@@ -25,10 +25,45 @@  description:
 
 allOf:
   - $ref: /schemas/spi/spi-controller.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,sa8255p-geni-spi
+    then:
+      required:
+        - power-domains
+        - power-domain-names
+
+      properties:
+        power-domains:
+          minItems: 2
+
+    else:
+      required:
+        - clocks
+        - clock-names
+
+      properties:
+        power-domains:
+          maxItems: 1
+
+        interconnects:
+          minItems: 2
+          maxItems: 3
+
+        interconnect-names:
+          minItems: 2
+          items:
+            - const: qup-core
+            - const: qup-config
+            - const: qup-memory
 
 properties:
   compatible:
-    const: qcom,geni-spi
+    enum:
+      - qcom,geni-spi
+      - qcom,sa8255p-geni-spi
 
   clocks:
     maxItems: 1
@@ -61,15 +96,19 @@  properties:
   operating-points-v2: true
 
   power-domains:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  power-domain-names:
+    items:
+      - const: power
+      - const: perf
 
   reg:
     maxItems: 1
 
 required:
   - compatible
-  - clocks
-  - clock-names
   - interrupts
   - reg
 
@@ -116,3 +155,16 @@  examples:
         #address-cells = <1>;
         #size-cells = <0>;
     };
+
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    spi@888000 {
+        compatible = "qcom,sa8255p-geni-spi";
+        reg = <0x888000 0x4000>;
+        interrupts = <GIC_SPI 584 IRQ_TYPE_LEVEL_HIGH>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        power-domains = <&scmi10_pd 16>, <&scmi10_dvfs 16>;
+        power-domain-names = "power", "perf";
+    };