[PATCH v4 0/6] Support ROCKCHIP SPI new feature

[PATCH v4 0/6] Support ROCKCHIP SPI new feature

[Jon Lin]

Changes in v4:
- Adjust the order patches
- Simply commit massage like redundancy "application" content

Changes in v3:
- Fix compile error which is find by Sascha in [v2,2/8]

Jon Lin (6):
  dt-bindings: spi: spi-rockchip: add description for rv1126 and rk3568
  spi: rockchip: add compatible string for rv1126 and rk3568
  spi: rockchip: Set rx_fifo interrupt waterline base on transfer item
  spi: rockchip: Wait for STB status in slave mode tx_xfer
  spi: rockchip: Support cs-gpio
  spi: rockchip: Support SPI_CS_HIGH

 .../devicetree/bindings/spi/spi-rockchip.yaml |  2 +
 drivers/spi/spi-rockchip.c                    | 96 +++++++++++++++----
 2 files changed, 82 insertions(+), 16 deletions(-)

-- 
2.17.1

[PATCH v4 1/6] dt-bindings: spi: spi-rockchip: add description for rv1126 and rk3568

[Jon Lin]

The description below will be used for rv1126.dtsi or rk3568.dtsi in
the future

Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v4:
- Adjust the order patches
- Simply commit massage like redundancy "application" content

Changes in v3:
- Fix compile error which is find by Sascha in [v2,2/8]

 Documentation/devicetree/bindings/spi/spi-rockchip.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
index 1e6cf29e6388..2d7957f9ae0a 100644
--- a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
@@ -27,12 +27,14 @@ properties:
       - items:
           - enum:
               - rockchip,px30-spi
+              - rockchip,rv1126-spi
               - rockchip,rk3188-spi
               - rockchip,rk3288-spi
               - rockchip,rk3308-spi
               - rockchip,rk3328-spi
               - rockchip,rk3368-spi
               - rockchip,rk3399-spi
+              - rockchip,rk3568-spi
           - const: rockchip,rk3066-spi
 
   reg:
-- 
2.17.1

Re: [PATCH v4 1/6] dt-bindings: spi: spi-rockchip: add description for rv1126 and rk3568

[Johan Jonker]

Hi Jon,

On 6/7/21 8:34 AM, Jon Lin wrote:
> The description below will be used for rv1126.dtsi or rk3568.dtsi in
> the future
> 
> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> ---
> 
> Changes in v4:
> - Adjust the order patches
> - Simply commit massage like redundancy "application" content
> 
> Changes in v3:
> - Fix compile error which is find by Sascha in [v2,2/8]
> 
>  Documentation/devicetree/bindings/spi/spi-rockchip.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
> index 1e6cf29e6388..2d7957f9ae0a 100644
> --- a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
> @@ -27,12 +27,14 @@ properties:
>        - items:
>            - enum:
>                - rockchip,px30-spi

> +              - rockchip,rv1126-spi

This list is sort alphabetically.
Move "rockchip,rv1126-spi" below "rockchip,rk3568-spi"

>                - rockchip,rk3188-spi
>                - rockchip,rk3288-spi
>                - rockchip,rk3308-spi
>                - rockchip,rk3328-spi
>                - rockchip,rk3368-spi
>                - rockchip,rk3399-spi
> +              - rockchip,rk3568-spi


>            - const: rockchip,rk3066-spi
>  
>    reg:
> 

===

Your comment in [PATCH v3 3/8]:
>> Adding "rockchip,rv1126-spi" to rockchip_spi_dt_match[] is strictly not
>> needed when using "rockchip,rk3066-spi" as fall back string.
>> Could a maintainer advise?
>>
>> Maybe this bug of mine should revert too?? Or is it legacy?
>> spi: rockchip: add compatible string for px30 rk3308 rk3328
>> https://lore.kernel.org/r/20200309151004.7780-1-jbx6244@gmail.com

> I agree with you. If the maintainer doesn't have any comments, I will use
> "rockchip,spi" as compatible names for the subsequent rk platform.

Compatibility strings are supposed to be SoC orientated.
So generic ones like in the manufacturer tree can't be used here.

===

Johan

Re: [PATCH v4 1/6] dt-bindings: spi: spi-rockchip: add description for rv1126 and rk3568

[Heiko Stübner]

Am Montag, 7. Juni 2021, 10:15:30 CEST schrieb Johan Jonker:
> Hi Jon,
> 
> On 6/7/21 8:34 AM, Jon Lin wrote:
> > The description below will be used for rv1126.dtsi or rk3568.dtsi in
> > the future
> > 
> > Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> > ---
> > 
> > Changes in v4:
> > - Adjust the order patches
> > - Simply commit massage like redundancy "application" content
> > 
> > Changes in v3:
> > - Fix compile error which is find by Sascha in [v2,2/8]
> > 
> >  Documentation/devicetree/bindings/spi/spi-rockchip.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
> > index 1e6cf29e6388..2d7957f9ae0a 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
> > @@ -27,12 +27,14 @@ properties:
> >        - items:
> >            - enum:
> >                - rockchip,px30-spi
> 
> > +              - rockchip,rv1126-spi
> 
> This list is sort alphabetically.
> Move "rockchip,rv1126-spi" below "rockchip,rk3568-spi"
> 
> >                - rockchip,rk3188-spi
> >                - rockchip,rk3288-spi
> >                - rockchip,rk3308-spi
> >                - rockchip,rk3328-spi
> >                - rockchip,rk3368-spi
> >                - rockchip,rk3399-spi
> > +              - rockchip,rk3568-spi
> 
> 
> >            - const: rockchip,rk3066-spi
> >  
> >    reg:
> > 
> 
> ===
> 
> Your comment in [PATCH v3 3/8]:
> >> Adding "rockchip,rv1126-spi" to rockchip_spi_dt_match[] is strictly not
> >> needed when using "rockchip,rk3066-spi" as fall back string.
> >> Could a maintainer advise?
> >>
> >> Maybe this bug of mine should revert too?? Or is it legacy?
> >> spi: rockchip: add compatible string for px30 rk3308 rk3328
> >> https://lore.kernel.org/r/20200309151004.7780-1-jbx6244@gmail.com
> 
> > I agree with you. If the maintainer doesn't have any comments, I will use
> > "rockchip,spi" as compatible names for the subsequent rk platform.
> 
> Compatibility strings are supposed to be SoC orientated.
> So generic ones like in the manufacturer tree can't be used here.

Johan ist right :-) .

rockchip,spi won't work at all, especially as these controllers always change
over time. [0]

Best example is the iommu. We started with "rockchip,iommu" thinking this
won't change over time, but with the rk3568 we get a new slightly different
iommu.

The vendor-kernel then introduces somewhat random "-vX" additions to
distinguish them, but often they do seem to be very software-centric.

Meaning, hardware-designers moved stuff around and software-developers
then invented the versioning to differentiate between versions.

The devicetree is supposed to describe the hardware though, so going with
the relevant soc-specific compatible gives us the necessary hardware-centric
differentiation.

Also this allows to catch later issues with specific soc implementations ;-)
Like 6 monts down the road we discover some special behaviour on the
rk3568 and devicetree is supposed to be stable.

So having the relevant compatibles in place allows us to just add driver
fixes and have those apply on the rk3568 if that is need at some point.

Heiko

Re: [PATCH v4 1/6] dt-bindings: spi: spi-rockchip: add description for rv1126 and rk3568

[Jon Lin]

On 6/7/21 5:04 PM, Heiko Stübner wrote:
> Am Montag, 7. Juni 2021, 10:15:30 CEST schrieb Johan Jonker:
>> Hi Jon,
>>
>> On 6/7/21 8:34 AM, Jon Lin wrote:
>>> The description below will be used for rv1126.dtsi or rk3568.dtsi in
>>> the future
>>>
>>> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
>>> ---
>>>
>>> Changes in v4:
>>> - Adjust the order patches
>>> - Simply commit massage like redundancy "application" content
>>>
>>> Changes in v3:
>>> - Fix compile error which is find by Sascha in [v2,2/8]
>>>
>>>   Documentation/devicetree/bindings/spi/spi-rockchip.yaml | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
>>> index 1e6cf29e6388..2d7957f9ae0a 100644
>>> --- a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
>>> @@ -27,12 +27,14 @@ properties:
>>>         - items:
>>>             - enum:
>>>                 - rockchip,px30-spi
>>> +              - rockchip,rv1126-spi
>> This list is sort alphabetically.
>> Move "rockchip,rv1126-spi" below "rockchip,rk3568-spi"
>>
>>>                 - rockchip,rk3188-spi
>>>                 - rockchip,rk3288-spi
>>>                 - rockchip,rk3308-spi
>>>                 - rockchip,rk3328-spi
>>>                 - rockchip,rk3368-spi
>>>                 - rockchip,rk3399-spi
>>> +              - rockchip,rk3568-spi
>>
>>>             - const: rockchip,rk3066-spi
>>>   
>>>     reg:
>>>
>> ===
>>
>> Your comment in [PATCH v3 3/8]:
>>>> Adding "rockchip,rv1126-spi" to rockchip_spi_dt_match[] is strictly not
>>>> needed when using "rockchip,rk3066-spi" as fall back string.
>>>> Could a maintainer advise?
With consulting to my colleague，we plane to:
1.If new soc's spi ip is compatible with the fall back one, we wont add 
new compatible id to the code.
2.I will add new fall back string stand for new generation ip: 
rockchip,rv1126-spi
>>>>
>>>> Maybe this bug of mine should revert too?? Or is it legacy?
>>>> spi: rockchip: add compatible string for px30 rk3308 rk3328
>>>> https://lore.kernel.org/r/20200309151004.7780-1-jbx6244@gmail.com
>>> I agree with you. If the maintainer doesn't have any comments, I will use
>>> "rockchip,spi" as compatible names for the subsequent rk platform.
>> Compatibility strings are supposed to be SoC orientated.
>> So generic ones like in the manufacturer tree can't be used here.
> Johan ist right :-) .
>
> rockchip,spi won't work at all, especially as these controllers always change
> over time. [0]
>
> Best example is the iommu. We started with "rockchip,iommu" thinking this
> won't change over time, but with the rk3568 we get a new slightly different
> iommu.
>
> The vendor-kernel then introduces somewhat random "-vX" additions to
> distinguish them, but often they do seem to be very software-centric.
>
> Meaning, hardware-designers moved stuff around and software-developers
> then invented the versioning to differentiate between versions.
>
> The devicetree is supposed to describe the hardware though, so going with
> the relevant soc-specific compatible gives us the necessary hardware-centric
> differentiation.
>
> Also this allows to catch later issues with specific soc implementations ;-)
> Like 6 monts down the road we discover some special behaviour on the
> rk3568 and devicetree is supposed to be stable.
>
> So having the relevant compatibles in place allows us to just add driver
> fixes and have those apply on the rk3568 if that is need at some point.
>
> Heiko
>
After the explain from you and Johan, I found that the idea 
"rockchip,spi" was immature.
>
>
>
>

Re: [PATCH v4 1/6] dt-bindings: spi: spi-rockchip: add description for rv1126 and rk3568

[Kever Yang]

Hi Heiko, Johan,

On 2021/6/7 下午5:04, Heiko Stübner wrote:
> Your comment in [PATCH v3 3/8]:
>>>> Adding "rockchip,rv1126-spi" to rockchip_spi_dt_match[] is strictly not
>>>> needed when using "rockchip,rk3066-spi" as fall back string.
>>>> Could a maintainer advise?
>>>>
>>>> Maybe this bug of mine should revert too?? Or is it legacy?
>>>> spi: rockchip: add compatible string for px30 rk3308 rk3328
>>>> https://lore.kernel.org/r/20200309151004.7780-1-jbx6244@gmail.com
>>> I agree with you. If the maintainer doesn't have any comments, I will use
>>> "rockchip,spi" as compatible names for the subsequent rk platform.
>> Compatibility strings are supposed to be SoC orientated.
>> So generic ones like in the manufacturer tree can't be used here.
> Johan ist right :-) .
>
> rockchip,spi won't work at all, especially as these controllers always change
> over time. [0]
>
> Best example is the iommu. We started with "rockchip,iommu" thinking this
> won't change over time, but with the rk3568 we get a new slightly different
> iommu.


Rockchip SPI and SFC controller can use a generic compatible string, 
because there is a version

register inside the IP, and all the feature update will have a new IP 
version, so the driver is

used for the SPI/SFC IP  in all SoCs, we don't need to care which SoC is 
using this driver.

If we have to use the compatible string "rockchip,rk3066-spi" and each 
for a new soc, then we

have to update the driver compatible id list and document for each soc 
which is totally not need

and not correct  to do it.

The example "iommu" is different, because there is no version register 
inside the IP and the IP

can not identify itself, which need a software define "-vX".


Thanks,

- Kever

> The vendor-kernel then introduces somewhat random "-vX" additions to
> distinguish them, but often they do seem to be very software-centric.
>
> Meaning, hardware-designers moved stuff around and software-developers
> then invented the versioning to differentiate between versions.
>
> The devicetree is supposed to describe the hardware though, so going with
> the relevant soc-specific compatible gives us the necessary hardware-centric
> differentiation.
>
> Also this allows to catch later issues with specific soc implementations ;-)
> Like 6 monts down the road we discover some special behaviour on the
> rk3568 and devicetree is supposed to be stable.
>
> So having the relevant compatibles in place allows us to just add driver
> fixes and have those apply on the rk3568 if that is need at some point.
>
> Heiko
>
>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>