[v2,1/3] dt-bindings: media: camss: qcom,msm8996-camss: Add CAMSS power domain

Message ID 20230526180712.8481-2-y.oudjana@protonmail.com
State New
Headers
Series media: camss: Link CAMSS power domain on MSM8996 |

Commit Message

Yassine Oudjana May 26, 2023, 6:07 p.m. UTC
  From: Yassine Oudjana <y.oudjana@protonmail.com>

Add the CAMSS power domain which is needed for the proper operation of
CAMSS, and add power-domain-names to ease fetching it as well as the other
power domains.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../bindings/media/qcom,msm8996-camss.yaml          | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
  

Comments

Conor Dooley May 26, 2023, 7:46 p.m. UTC | #1
Yo Yassine,

On Fri, May 26, 2023 at 09:07:10PM +0300, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Add the CAMSS power domain which is needed for the proper operation of
> CAMSS, and add power-domain-names to ease fetching it as well as the other
> power domains.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  .../bindings/media/qcom,msm8996-camss.yaml          | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,msm8996-camss.yaml b/Documentation/devicetree/bindings/media/qcom,msm8996-camss.yaml
> index 8a10aa1cafc5..27c9a11f0df9 100644
> --- a/Documentation/devicetree/bindings/media/qcom,msm8996-camss.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,msm8996-camss.yaml
> @@ -85,6 +85,13 @@ properties:
>      items:
>        - description: VFE0 GDSC - Video Front End, Global Distributed Switch Controller.
>        - description: VFE1 GDSC - Video Front End, Global Distributed Switch Controller.
> +      - description: CAMSS GDSC - Camera Subsystem, Global Distributed Switch Controller.
> +
> +  power-domain-names:
> +    items:
> +      - const: vfe0
> +      - const: vfe1
> +      - const: camss
>  
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -209,6 +216,7 @@ required:
>    - interrupts
>    - iommus
>    - power-domains
> +  - power-domain-names

Why is this now required?

Thanks,
Conor.

>    - reg
>    - reg-names
>    - vdda-supply
> @@ -326,7 +334,10 @@ examples:
>           <&vfe_smmu 3>;
>  
>        power-domains = <&mmcc VFE0_GDSC>,
> -        <&mmcc VFE1_GDSC>;
> +        <&mmcc VFE1_GDSC>,
> +        <&mmcc CAMSS_GDSC>;
> +
> +      power-domain-names = "vfe0", "vfe1", "camss";
>  
>        reg = <0x00a34000 0x1000>,
>          <0x00a00030 0x4>,
> -- 
> 2.40.1
>
  
Bryan O'Donoghue May 26, 2023, 8:05 p.m. UTC | #2
On 26/05/2023 20:46, Conor Dooley wrote:
>> +  - power-domain-names
> Why is this now required?
> 
> Thanks,
> Conor.
> 

Its an accurate description of the power/clock tree to have the top 
power domain be switched on prior to the clocks that depend on it.

I think Yassine, you could probably include the majority of your 
cover-letter text in this commit to explain this change a bit better.

---
bod
  
Conor Dooley May 26, 2023, 8:19 p.m. UTC | #3
On Fri, May 26, 2023 at 09:05:47PM +0100, Bryan O'Donoghue wrote:
> On 26/05/2023 20:46, Conor Dooley wrote:
> > > +  - power-domain-names
> > Why is this now required?
> > 
> > Thanks,
> > Conor.
> > 
> 
> Its an accurate description of the power/clock tree to have the top power
> domain be switched on prior to the clocks that depend on it.

But what does that have to do with the *names* now being required?

> I think Yassine, you could probably include the majority of your
> cover-letter text in this commit to explain this change a bit better.

I think it would be good to have that regardless.

Cheers,
Conor.

> bod
  ^^^ I've been trying not to think about rugby since the weekend :(
  
Bryan O'Donoghue May 26, 2023, 8:21 p.m. UTC | #4
On 26/05/2023 21:19, Conor Dooley wrote:
> On Fri, May 26, 2023 at 09:05:47PM +0100, Bryan O'Donoghue wrote:
>> On 26/05/2023 20:46, Conor Dooley wrote:
>>>> +  - power-domain-names
>>> Why is this now required?
>>>
>>> Thanks,
>>> Conor.
>>>
>>
>> Its an accurate description of the power/clock tree to have the top power
>> domain be switched on prior to the clocks that depend on it.
> 
> But what does that have to do with the *names* now being required?

oh the names

no toss that

> 
>> I think Yassine, you could probably include the majority of your
>> cover-letter text in this commit to explain this change a bit better.
> 
> I think it would be good to have that regardless.
> 
> Cheers,
> Conor.
> 
>> bod
>    ^^^ I've been trying not to think about rugby since the weekend :(

Pockets O'Gara should have his paddy papers rescinded
  
Konrad Dybcio May 26, 2023, 8:36 p.m. UTC | #5
On 26.05.2023 22:21, Bryan O'Donoghue wrote:
> On 26/05/2023 21:19, Conor Dooley wrote:
>> On Fri, May 26, 2023 at 09:05:47PM +0100, Bryan O'Donoghue wrote:
>>> On 26/05/2023 20:46, Conor Dooley wrote:
>>>>> +  - power-domain-names
>>>> Why is this now required?
>>>>
>>>> Thanks,
>>>> Conor.
>>>>
>>>
>>> Its an accurate description of the power/clock tree to have the top power
>>> domain be switched on prior to the clocks that depend on it.
>>
>> But what does that have to do with the *names* now being required?
> 
> oh the names
> 
> no toss that
this should be

if:properties:compatible:blahblahmsm8996:then:required:power-domain-names

Konrad
> 
>>
>>> I think Yassine, you could probably include the majority of your
>>> cover-letter text in this commit to explain this change a bit better.
>>
>> I think it would be good to have that regardless.
>>
>> Cheers,
>> Conor.
>>
>>> bod
>>    ^^^ I've been trying not to think about rugby since the weekend :(
> 
> Pockets O'Gara should have his paddy papers rescinded
  
Bryan O'Donoghue May 26, 2023, 8:40 p.m. UTC | #6
On 26/05/2023 21:36, Konrad Dybcio wrote:
>> oh the names
>>
>> no toss that
> this should be
> 
> if:properties:compatible:blahblahmsm8996:then:required:power-domain-names
> 
> Konrad

Hmm, we don't depend on the names though.

---
bod
  
Konrad Dybcio May 26, 2023, 8:43 p.m. UTC | #7
On 26.05.2023 22:40, Bryan O'Donoghue wrote:
> On 26/05/2023 21:36, Konrad Dybcio wrote:
>>> oh the names
>>>
>>> no toss that
>> this should be
>>
>> if:properties:compatible:blahblahmsm8996:then:required:power-domain-names
>>
>> Konrad
> 
> Hmm, we don't depend on the names though.
Check patch 3!

Konrad
> 
> ---
> bod
  
Bryan O'Donoghue May 26, 2023, 8:47 p.m. UTC | #8
On 26/05/2023 21:43, Konrad Dybcio wrote:
> 
> 
> On 26.05.2023 22:40, Bryan O'Donoghue wrote:
>> On 26/05/2023 21:36, Konrad Dybcio wrote:
>>>> oh the names
>>>>
>>>> no toss that
>>> this should be
>>>
>>> if:properties:compatible:blahblahmsm8996:then:required:power-domain-names
>>>
>>> Konrad
>>
>> Hmm, we don't depend on the names though.
> Check patch 3!

Hmm but we already count the number of power domains in 
camss_configure_pd().

There's no logic in counting it twice using two different methods.
  
Yassine Oudjana May 27, 2023, 6:05 a.m. UTC | #9
On Fri, May 26 2023 at 10:36:32 PM +02:00:00, Konrad Dybcio 
<konrad.dybcio@linaro.org> wrote:
> 
> 
> On 26.05.2023 22:21, Bryan O'Donoghue wrote:
>>  On 26/05/2023 21:19, Conor Dooley wrote:
>>>  On Fri, May 26, 2023 at 09:05:47PM +0100, Bryan O'Donoghue wrote:
>>>>  On 26/05/2023 20:46, Conor Dooley wrote:
>>>>>>  +  - power-domain-names
>>>>>  Why is this now required?
>>>>> 
>>>>>  Thanks,
>>>>>  Conor.
>>>>> 
>>>> 
>>>>  Its an accurate description of the power/clock tree to have the 
>>>> top power
>>>>  domain be switched on prior to the clocks that depend on it.
>>> 
>>>  But what does that have to do with the *names* now being required?
>> 
>>  oh the names
>> 
>>  no toss that
> this should be
> 
> if:properties:compatible:blahblahmsm8996:then:required:power-domain-names

The only compatible in this binding is qcom,msm8996-camss, so what 
would this achieve?

> 
> Konrad
>> 
>>> 
>>>>  I think Yassine, you could probably include the majority of your
>>>>  cover-letter text in this commit to explain this change a bit 
>>>> better.
>>> 
>>>  I think it would be good to have that regardless.
>>> 
>>>  Cheers,
>>>  Conor.
>>> 
>>>>  bod
>>>     ^^^ I've been trying not to think about rugby since the weekend 
>>> :(
>> 
>>  Pockets O'Gara should have his paddy papers rescinded
  
Konrad Dybcio May 27, 2023, 3:52 p.m. UTC | #10
On 27.05.2023 08:05, Yassine Oudjana wrote:
> 
> On Fri, May 26 2023 at 10:36:32 PM +02:00:00, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>>
>> On 26.05.2023 22:21, Bryan O'Donoghue wrote:
>>>  On 26/05/2023 21:19, Conor Dooley wrote:
>>>>  On Fri, May 26, 2023 at 09:05:47PM +0100, Bryan O'Donoghue wrote:
>>>>>  On 26/05/2023 20:46, Conor Dooley wrote:
>>>>>>>  +  - power-domain-names
>>>>>>  Why is this now required?
>>>>>>
>>>>>>  Thanks,
>>>>>>  Conor.
>>>>>>
>>>>>
>>>>>  Its an accurate description of the power/clock tree to have the top power
>>>>>  domain be switched on prior to the clocks that depend on it.
>>>>
>>>>  But what does that have to do with the *names* now being required?
>>>
>>>  oh the names
>>>
>>>  no toss that
>> this should be
>>
>> if:properties:compatible:blahblahmsm8996:then:required:power-domain-names
> 
> The only compatible in this binding is qcom,msm8996-camss, so what would this achieve?
You're right - I didn't notice and assumed it was a camss-common one.

Konrad
> 
>>
>> Konrad
>>>
>>>>
>>>>>  I think Yassine, you could probably include the majority of your
>>>>>  cover-letter text in this commit to explain this change a bit better.
>>>>
>>>>  I think it would be good to have that regardless.
>>>>
>>>>  Cheers,
>>>>  Conor.
>>>>
>>>>>  bod
>>>>     ^^^ I've been trying not to think about rugby since the weekend :(
>>>
>>>  Pockets O'Gara should have his paddy papers rescinded
> 
>
  

Patch

diff --git a/Documentation/devicetree/bindings/media/qcom,msm8996-camss.yaml b/Documentation/devicetree/bindings/media/qcom,msm8996-camss.yaml
index 8a10aa1cafc5..27c9a11f0df9 100644
--- a/Documentation/devicetree/bindings/media/qcom,msm8996-camss.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,msm8996-camss.yaml
@@ -85,6 +85,13 @@  properties:
     items:
       - description: VFE0 GDSC - Video Front End, Global Distributed Switch Controller.
       - description: VFE1 GDSC - Video Front End, Global Distributed Switch Controller.
+      - description: CAMSS GDSC - Camera Subsystem, Global Distributed Switch Controller.
+
+  power-domain-names:
+    items:
+      - const: vfe0
+      - const: vfe1
+      - const: camss
 
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
@@ -209,6 +216,7 @@  required:
   - interrupts
   - iommus
   - power-domains
+  - power-domain-names
   - reg
   - reg-names
   - vdda-supply
@@ -326,7 +334,10 @@  examples:
          <&vfe_smmu 3>;
 
       power-domains = <&mmcc VFE0_GDSC>,
-        <&mmcc VFE1_GDSC>;
+        <&mmcc VFE1_GDSC>,
+        <&mmcc CAMSS_GDSC>;
+
+      power-domain-names = "vfe0", "vfe1", "camss";
 
       reg = <0x00a34000 0x1000>,
         <0x00a00030 0x4>,