diff mbox series

[3/4] dt-bindings: soc: samsung: exynos-sysreg: add dedicated SYSREG compatibles to Exynos5433

Message ID 20221125112201.240178-3-krzysztof.kozlowski@linaro.org
State New
Headers
Series [1/4] arm64: dts: exynos: add dedicated SYSREG compatibles to Exynos5433 |

Commit Message

Krzysztof Kozlowski Nov. 25, 2022, 11:22 a.m. UTC 
  Exynos5433 has several different SYSREGs, so use dedicated compatibles
for them.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Cc: Sriranjani P <sriranjani.p@samsung.com>
Cc: Chanho Park <chanho61.park@samsung.com>
Cc: Sam Protsenko <semen.protsenko@linaro.org>
---
 .../bindings/soc/samsung/samsung,exynos-sysreg.yaml | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Sam Protsenko Nov. 25, 2022, 2:22 p.m. UTC | #1 
On Fri, 25 Nov 2022 at 05:22, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> Exynos5433 has several different SYSREGs, so use dedicated compatibles
> for them.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> ---
>
> Cc: Sriranjani P <sriranjani.p@samsung.com>
> Cc: Chanho Park <chanho61.park@samsung.com>
> Cc: Sam Protsenko <semen.protsenko@linaro.org>
> ---

Hi Krzysztof,

Just curious: what is the rationale for adding those more specific
sysregs? AFAIR, e.g. in Exynos850, different SysReg instances have
pretty much the same register layout.

Other than that:

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

>  .../bindings/soc/samsung/samsung,exynos-sysreg.yaml | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
> index 68064a5e339c..42357466005e 100644
> --- a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
> +++ b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
> @@ -17,10 +17,21 @@ properties:
>                - samsung,exynos3-sysreg
>                - samsung,exynos4-sysreg
>                - samsung,exynos5-sysreg
> -              - samsung,exynos5433-sysreg
>                - samsung,exynos850-sysreg
>                - samsung,exynosautov9-sysreg
>            - const: syscon
> +      - items:
> +          - enum:
> +              - samsung,exynos5433-cam0-sysreg
> +              - samsung,exynos5433-cam1-sysreg
> +              - samsung,exynos5433-disp-sysreg
> +              - samsung,exynos5433-fsys-sysreg
> +          - const: samsung,exynos5433-sysreg
> +          - const: syscon
> +      - items:
> +          - const: samsung,exynos5433-sysreg
> +          - const: syscon
> +        deprecated: true
>
>    reg:
>      maxItems: 1
> --
> 2.34.1
>
Sriranjani P Nov. 25, 2022, 2:33 p.m. UTC | #2 
> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> Sent: 25 November 2022 16:52
> To: Lee Jones <lee@kernel.org>; Rob Herring <robh+dt@kernel.org>;
Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Alim Akhtar
> <alim.akhtar@samsung.com>; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
linux-samsung-
> soc@vger.kernel.org
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>; Sriranjani P
> <sriranjani.p@samsung.com>; Chanho Park <chanho61.park@samsung.com>;
> Sam Protsenko <semen.protsenko@linaro.org>
> Subject: [PATCH 3/4] dt-bindings: soc: samsung: exynos-sysreg: add
dedicated
> SYSREG compatibles to Exynos5433
> 
> Exynos5433 has several different SYSREGs, so use dedicated compatibles for
> them.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> Cc: Sriranjani P <sriranjani.p@samsung.com>
> Cc: Chanho Park <chanho61.park@samsung.com>
> Cc: Sam Protsenko <semen.protsenko@linaro.org>
> ---

Reviewed-by: Sriranjani P <sriranjani.p@samsung.com>

>  .../bindings/soc/samsung/samsung,exynos-sysreg.yaml | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
> sysreg.yaml
> b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
> sysreg.yaml
> index 68064a5e339c..42357466005e 100644
> --- a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
> sysreg.yaml
> +++ b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
> sysre
> +++ g.yaml
> @@ -17,10 +17,21 @@ properties:
>                - samsung,exynos3-sysreg
>                - samsung,exynos4-sysreg
>                - samsung,exynos5-sysreg
> -              - samsung,exynos5433-sysreg
>                - samsung,exynos850-sysreg
>                - samsung,exynosautov9-sysreg
>            - const: syscon
> +      - items:
> +          - enum:
> +              - samsung,exynos5433-cam0-sysreg
> +              - samsung,exynos5433-cam1-sysreg
> +              - samsung,exynos5433-disp-sysreg
> +              - samsung,exynos5433-fsys-sysreg
> +          - const: samsung,exynos5433-sysreg
> +          - const: syscon
> +      - items:
> +          - const: samsung,exynos5433-sysreg
> +          - const: syscon
> +        deprecated: true
> 
>    reg:
>      maxItems: 1
> --
> 2.34.1
Krzysztof Kozlowski Nov. 25, 2022, 2:47 p.m. UTC | #3 
On 25/11/2022 15:22, Sam Protsenko wrote:
> On Fri, 25 Nov 2022 at 05:22, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> Exynos5433 has several different SYSREGs, so use dedicated compatibles
>> for them.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> ---
>>
>> Cc: Sriranjani P <sriranjani.p@samsung.com>
>> Cc: Chanho Park <chanho61.park@samsung.com>
>> Cc: Sam Protsenko <semen.protsenko@linaro.org>
>> ---
> 
> Hi Krzysztof,
> 
> Just curious: what is the rationale for adding those more specific
> sysregs? AFAIR, e.g. in Exynos850, different SysReg instances have
> pretty much the same register layout.
> 

On Exynos5433 all these blocks have different registers. Are you saying
that Exynos850 has four (or more) sysregs which are exactly the same?
Same registers? Why would they duplicate it?

Best regards,
Krzysztof
Sam Protsenko Nov. 25, 2022, 2:57 p.m. UTC | #4 
On Fri, 25 Nov 2022 at 08:47, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 25/11/2022 15:22, Sam Protsenko wrote:
> > On Fri, 25 Nov 2022 at 05:22, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> Exynos5433 has several different SYSREGs, so use dedicated compatibles
> >> for them.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>
> >> ---
> >>
> >> Cc: Sriranjani P <sriranjani.p@samsung.com>
> >> Cc: Chanho Park <chanho61.park@samsung.com>
> >> Cc: Sam Protsenko <semen.protsenko@linaro.org>
> >> ---
> >
> > Hi Krzysztof,
> >
> > Just curious: what is the rationale for adding those more specific
> > sysregs? AFAIR, e.g. in Exynos850, different SysReg instances have
> > pretty much the same register layout.
> >
>
> On Exynos5433 all these blocks have different registers. Are you saying
> that Exynos850 has four (or more) sysregs which are exactly the same?
> Same registers? Why would they duplicate it?
>

Ah, no, you are right. Just checked it, they are different. Just first
couple of registers are similar between blocks, that's why I memorized
it wrong.

So as I understand, adding those new compatibles follows "describe HW,
not a driver" rule? Because AFAIU, right now it'll fallback to
"syscon" compatible anyway.

> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Nov. 25, 2022, 3:01 p.m. UTC | #5 
On 25/11/2022 15:57, Sam Protsenko wrote:
> On Fri, 25 Nov 2022 at 08:47, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 25/11/2022 15:22, Sam Protsenko wrote:
>>> On Fri, 25 Nov 2022 at 05:22, Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> Exynos5433 has several different SYSREGs, so use dedicated compatibles
>>>> for them.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>
>>>> ---
>>>>
>>>> Cc: Sriranjani P <sriranjani.p@samsung.com>
>>>> Cc: Chanho Park <chanho61.park@samsung.com>
>>>> Cc: Sam Protsenko <semen.protsenko@linaro.org>
>>>> ---
>>>
>>> Hi Krzysztof,
>>>
>>> Just curious: what is the rationale for adding those more specific
>>> sysregs? AFAIR, e.g. in Exynos850, different SysReg instances have
>>> pretty much the same register layout.
>>>
>>
>> On Exynos5433 all these blocks have different registers. Are you saying
>> that Exynos850 has four (or more) sysregs which are exactly the same?
>> Same registers? Why would they duplicate it?
>>
> 
> Ah, no, you are right. Just checked it, they are different. Just first
> couple of registers are similar between blocks, that's why I memorized
> it wrong.
> 
> So as I understand, adding those new compatibles follows "describe HW,
> not a driver" rule? Because AFAIU, right now it'll fallback to
> "syscon" compatible anyway.

Yes, they describe hardware. Of course all of these sysregs are similar
as they are just bunch of SFR/MMIO-region, but they have different
roles/features. For example some other devices (users) of syscon/sysreg
should reference specific device, not any sysreg.

On several other architectures we use specific compatibles, so I think
for Samsung we should do the same.

Different case was for Exynos 3/4/5 where there was only one SYSREG.

Best regards,
Krzysztof
Alim Akhtar Nov. 25, 2022, 4:40 p.m. UTC | #6 
>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
>Sent: Friday, November 25, 2022 8:31 PM
>To: Sam Protsenko <semen.protsenko@linaro.org>
>Cc: Lee Jones <lee@kernel.org>; Rob Herring <robh+dt@kernel.org>;
>Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Alim Akhtar
><alim.akhtar@samsung.com>; devicetree@vger.kernel.org; linux-
>kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>samsung-soc@vger.kernel.org; Sriranjani P <sriranjani.p@samsung.com>;
>Chanho Park <chanho61.park@samsung.com>
>Subject: Re: [PATCH 3/4] dt-bindings: soc: samsung: exynos-sysreg: add
>dedicated SYSREG compatibles to Exynos5433
>
>On 25/11/2022 15:57, Sam Protsenko wrote:
>> On Fri, 25 Nov 2022 at 08:47, Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>> On 25/11/2022 15:22, Sam Protsenko wrote:
>>>> On Fri, 25 Nov 2022 at 05:22, Krzysztof Kozlowski
>>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>>
>>>>> Exynos5433 has several different SYSREGs, so use dedicated
>>>>> compatibles for them.
>>>>>
>>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>
>>>>> ---
>>>>>
>>>>> Cc: Sriranjani P <sriranjani.p@samsung.com>
>>>>> Cc: Chanho Park <chanho61.park@samsung.com>
>>>>> Cc: Sam Protsenko <semen.protsenko@linaro.org>
>>>>> ---
>>>>
>>>> Hi Krzysztof,
>>>>
>>>> Just curious: what is the rationale for adding those more specific
>>>> sysregs? AFAIR, e.g. in Exynos850, different SysReg instances have
>>>> pretty much the same register layout.
>>>>
>>>
>>> On Exynos5433 all these blocks have different registers. Are you
>>> saying that Exynos850 has four (or more) sysregs which are exactly the
>same?
>>> Same registers? Why would they duplicate it?
>>>
>>
>> Ah, no, you are right. Just checked it, they are different. Just first
>> couple of registers are similar between blocks, that's why I memorized
>> it wrong.
>>
>> So as I understand, adding those new compatibles follows "describe HW,
>> not a driver" rule? Because AFAIU, right now it'll fallback to
>> "syscon" compatible anyway.
>
>Yes, they describe hardware. Of course all of these sysregs are similar as they
>are just bunch of SFR/MMIO-region, but they have different roles/features.
>For example some other devices (users) of syscon/sysreg should reference
>specific device, not any sysreg.
>
Yes, these are dedicated / extended SFR region to provide IP/Block specific side-band signals / configurations.

>On several other architectures we use specific compatibles, so I think for
>Samsung we should do the same.
>
Yes, most of the SoC's sysreg are dedicated/included in the IP block itself now a day, so make sense to have a dedicated compatible.

>Different case was for Exynos 3/4/5 where there was only one SYSREG.
>
AFAIR, this is correct.

>Best regards,
>Krzysztof
Alim Akhtar Nov. 25, 2022, 5:06 p.m. UTC | #7 
>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
>Sent: Friday, November 25, 2022 4:52 PM
>To: Lee Jones <lee@kernel.org>; Rob Herring <robh+dt@kernel.org>;
>Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Alim Akhtar
><alim.akhtar@samsung.com>; devicetree@vger.kernel.org; linux-
>kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>samsung-soc@vger.kernel.org
>Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>; Sriranjani P
><sriranjani.p@samsung.com>; Chanho Park <chanho61.park@samsung.com>;
>Sam Protsenko <semen.protsenko@linaro.org>
>Subject: [PATCH 3/4] dt-bindings: soc: samsung: exynos-sysreg: add
dedicated
>SYSREG compatibles to Exynos5433
>
>Exynos5433 has several different SYSREGs, so use dedicated compatibles for
>them.
>
>Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
>---
>
>Cc: Sriranjani P <sriranjani.p@samsung.com>
>Cc: Chanho Park <chanho61.park@samsung.com>
>Cc: Sam Protsenko <semen.protsenko@linaro.org>
>---
> .../bindings/soc/samsung/samsung,exynos-sysreg.yaml | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
>diff --git
>a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
>sysreg.yaml
>b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
>sysreg.yaml
>index 68064a5e339c..42357466005e 100644
>--- a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
>sysreg.yaml
>+++ b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
>sysre
>+++ g.yaml
>@@ -17,10 +17,21 @@ properties:
>               - samsung,exynos3-sysreg
>               - samsung,exynos4-sysreg
>               - samsung,exynos5-sysreg
>-              - samsung,exynos5433-sysreg
>               - samsung,exynos850-sysreg
>               - samsung,exynosautov9-sysreg
>           - const: syscon
>+      - items:
>+          - enum:
>+              - samsung,exynos5433-cam0-sysreg
>+              - samsung,exynos5433-cam1-sysreg
>+              - samsung,exynos5433-disp-sysreg
>+              - samsung,exynos5433-fsys-sysreg
>+          - const: samsung,exynos5433-sysreg
>+          - const: syscon
>+      - items:
>+          - const: samsung,exynos5433-sysreg
>+          - const: syscon
>+        deprecated: true
Any reason to add "deprecated: true" here for above compatible?

>
>   reg:
>     maxItems: 1
>--
>2.34.1
Krzysztof Kozlowski Nov. 26, 2022, 1:08 p.m. UTC | #8 
On 25/11/2022 18:06, Alim Akhtar wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
>> Sent: Friday, November 25, 2022 4:52 PM
>> To: Lee Jones <lee@kernel.org>; Rob Herring <robh+dt@kernel.org>;
>> Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Alim Akhtar
>> <alim.akhtar@samsung.com>; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>> samsung-soc@vger.kernel.org
>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>; Sriranjani P
>> <sriranjani.p@samsung.com>; Chanho Park <chanho61.park@samsung.com>;
>> Sam Protsenko <semen.protsenko@linaro.org>
>> Subject: [PATCH 3/4] dt-bindings: soc: samsung: exynos-sysreg: add
> dedicated
>> SYSREG compatibles to Exynos5433
>>
>> Exynos5433 has several different SYSREGs, so use dedicated compatibles for
>> them.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> ---
>>
>> Cc: Sriranjani P <sriranjani.p@samsung.com>
>> Cc: Chanho Park <chanho61.park@samsung.com>
>> Cc: Sam Protsenko <semen.protsenko@linaro.org>
>> ---
>> .../bindings/soc/samsung/samsung,exynos-sysreg.yaml | 13 ++++++++++++-
>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
>> sysreg.yaml
>> b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
>> sysreg.yaml
>> index 68064a5e339c..42357466005e 100644
>> --- a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
>> sysreg.yaml
>> +++ b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
>> sysre
>> +++ g.yaml
>> @@ -17,10 +17,21 @@ properties:
>>               - samsung,exynos3-sysreg
>>               - samsung,exynos4-sysreg
>>               - samsung,exynos5-sysreg
>> -              - samsung,exynos5433-sysreg
>>               - samsung,exynos850-sysreg
>>               - samsung,exynosautov9-sysreg
>>           - const: syscon
>> +      - items:
>> +          - enum:
>> +              - samsung,exynos5433-cam0-sysreg
>> +              - samsung,exynos5433-cam1-sysreg
>> +              - samsung,exynos5433-disp-sysreg
>> +              - samsung,exynos5433-fsys-sysreg
>> +          - const: samsung,exynos5433-sysreg
>> +          - const: syscon
>> +      - items:
>> +          - const: samsung,exynos5433-sysreg
>> +          - const: syscon
>> +        deprecated: true
> Any reason to add "deprecated: true" here for above compatible?

Because it should be used alone as unspecific.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
index 68064a5e339c..42357466005e 100644
--- a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
+++ b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
@@ -17,10 +17,21 @@  properties:
               - samsung,exynos3-sysreg
               - samsung,exynos4-sysreg
               - samsung,exynos5-sysreg
-              - samsung,exynos5433-sysreg
               - samsung,exynos850-sysreg
               - samsung,exynosautov9-sysreg
           - const: syscon
+      - items:
+          - enum:
+              - samsung,exynos5433-cam0-sysreg
+              - samsung,exynos5433-cam1-sysreg
+              - samsung,exynos5433-disp-sysreg
+              - samsung,exynos5433-fsys-sysreg
+          - const: samsung,exynos5433-sysreg
+          - const: syscon
+      - items:
+          - const: samsung,exynos5433-sysreg
+          - const: syscon
+        deprecated: true
 
   reg:
     maxItems: 1