diff mbox series

[1/4] dt-bindings: media: qcom,sm8550-iris: update power domain name

Message ID 20250311-dtbinding-v1-1-5c807d33f7ae@quicinc.com (mailing list archive)
State New
Headers show
Series media: qcom: iris: add support for SA8775P | expand

Commit Message

Vikash Garodia March 11, 2025, 12:03 p.m. UTC
Not all platforms has a collapsible mx, so use the more generic naming
of mx in the binding.

Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
---
 Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rob Herring (Arm) March 11, 2025, 2:52 p.m. UTC | #1
On Tue, 11 Mar 2025 17:33:53 +0530, Vikash Garodia wrote:
> Not all platforms has a collapsible mx, so use the more generic naming
> of mx in the binding.
> 
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ---
>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sm8550-iris.example.dtb: video-codec@aa00000: power-domain-names:2: 'mx' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sm8550-iris.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250311-dtbinding-v1-1-5c807d33f7ae@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Dmitry Baryshkov March 11, 2025, 3:07 p.m. UTC | #2
On Tue, Mar 11, 2025 at 05:33:53PM +0530, Vikash Garodia wrote:
> Not all platforms has a collapsible mx, so use the more generic naming
> of mx in the binding.

I guess, it wasn't even tested...

> 
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ---
>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> index e424ea84c211f473a799481fd5463a16580187ed..440a0d7cdfe19a1ccedefc207d96b26eed5d6630 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> @@ -28,7 +28,7 @@ properties:
>      items:
>        - const: venus
>        - const: vcodec0
> -      - const: mxc
> +      - const: mx
>        - const: mmcx
>  
>    clocks:
> 
> -- 
> 2.34.1
>
Vikash Garodia March 11, 2025, 3:11 p.m. UTC | #3
On 3/11/2025 8:37 PM, Dmitry Baryshkov wrote:
> On Tue, Mar 11, 2025 at 05:33:53PM +0530, Vikash Garodia wrote:
>> Not all platforms has a collapsible mx, so use the more generic naming
>> of mx in the binding.
> 
> I guess, it wasn't even tested...
Not sure what made you guess so, let me check why my binding checker did not
catch the bot reported warning.
Regards,
Vikash
> 
>>
>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>> ---
>>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> index e424ea84c211f473a799481fd5463a16580187ed..440a0d7cdfe19a1ccedefc207d96b26eed5d6630 100644
>> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> @@ -28,7 +28,7 @@ properties:
>>      items:
>>        - const: venus
>>        - const: vcodec0
>> -      - const: mxc
>> +      - const: mx
>>        - const: mmcx
>>  
>>    clocks:
>>
>> -- 
>> 2.34.1
>>
>
Dmitry Baryshkov March 11, 2025, 3:35 p.m. UTC | #4
On Tue, Mar 11, 2025 at 08:41:01PM +0530, Vikash Garodia wrote:
> 
> On 3/11/2025 8:37 PM, Dmitry Baryshkov wrote:
> > On Tue, Mar 11, 2025 at 05:33:53PM +0530, Vikash Garodia wrote:
> >> Not all platforms has a collapsible mx, so use the more generic naming
> >> of mx in the binding.
> > 
> > I guess, it wasn't even tested...
> Not sure what made you guess so, let me check why my binding checker did not
> catch the bot reported warning.

Obvious: you are changing the bindings in a non-backwards compatible
way, but you are not changing the example in the same file (and
obviously you are not changing the DTs), which means that this wasn't
tested.

Hint: you can use enum [mx, mxc] instead of const. That would make it
backwards compatible.

> Regards,
> Vikash
> > 
> >>
> >> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> >> ---
> >>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >> index e424ea84c211f473a799481fd5463a16580187ed..440a0d7cdfe19a1ccedefc207d96b26eed5d6630 100644
> >> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >> @@ -28,7 +28,7 @@ properties:
> >>      items:
> >>        - const: venus
> >>        - const: vcodec0
> >> -      - const: mxc
> >> +      - const: mx
> >>        - const: mmcx
> >>  
> >>    clocks:
> >>
> >> -- 
> >> 2.34.1
> >>
> >
Konrad Dybcio March 11, 2025, 4:24 p.m. UTC | #5
On 3/11/25 4:11 PM, Vikash Garodia wrote:
> 
> On 3/11/2025 8:37 PM, Dmitry Baryshkov wrote:
>> On Tue, Mar 11, 2025 at 05:33:53PM +0530, Vikash Garodia wrote:
>>> Not all platforms has a collapsible mx, so use the more generic naming
>>> of mx in the binding.
>>
>> I guess, it wasn't even tested...
> Not sure what made you guess so, let me check why my binding checker did not
> catch the bot reported warning.

You probably checked the compiled DTBs (make dtbs_check / CHECK_DTBS=1), but you
also need to test the YAML (make dt_binding_check)

This change can't be accepted as-is, because there are already expectations
about the naming (and order) of the entries.

Because there's a difference, you would normally be expected to add a whole new
list, but maybe the dt-bindings maintainers will agree to Dmitry's solution of
adding a sneaky enum inside the list

Konrad
Krzysztof Kozlowski March 11, 2025, 5:33 p.m. UTC | #6
On 11/03/2025 13:03, Vikash Garodia wrote:
> Not all platforms has a collapsible mx, so use the more generic naming
> of mx in the binding.
> 

No, neither tested, nor justified. Read the file. How many platforms do
you have there? One. Out of this one platform you claim not all of them
have MX collapsible, so you want MX?

Best regards,
Krzysztof
Vikash Garodia March 11, 2025, 5:47 p.m. UTC | #7
On 3/11/2025 11:03 PM, Krzysztof Kozlowski wrote:
> On 11/03/2025 13:03, Vikash Garodia wrote:
>> Not all platforms has a collapsible mx, so use the more generic naming
>> of mx in the binding.
>>
> 
> No, neither tested, nor justified. Read the file. How many platforms do
> you have there? One. Out of this one platform you claim not all of them
> have MX collapsible, so you want MX?
Let say we have one which is non-collapsible, what should be the way in that
case to use the bindings which differ only in the MX/MXC part ?
Regards,
Vikash
> 
> Best regards,
> Krzysztof
Vikash Garodia March 11, 2025, 7:19 p.m. UTC | #8
On 3/11/2025 9:05 PM, Dmitry Baryshkov wrote:
> On Tue, Mar 11, 2025 at 08:41:01PM +0530, Vikash Garodia wrote:
>>
>> On 3/11/2025 8:37 PM, Dmitry Baryshkov wrote:
>>> On Tue, Mar 11, 2025 at 05:33:53PM +0530, Vikash Garodia wrote:
>>>> Not all platforms has a collapsible mx, so use the more generic naming
>>>> of mx in the binding.
>>>
>>> I guess, it wasn't even tested...
>> Not sure what made you guess so, let me check why my binding checker did not
>> catch the bot reported warning.
> 
> Obvious: you are changing the bindings in a non-backwards compatible
> way, but you are not changing the example in the same file (and
> obviously you are not changing the DTs), which means that this wasn't
> tested.
> 
> Hint: you can use enum [mx, mxc] instead of const. That would make it
> backwards compatible.
Currently there are no user of this binding. Given that either of MX or MXC are
same connection to video hardware, just that one is collapsible, it would be
good to replace the existing element instead of enum.

Regards,
Vikash
> 
>> Regards,
>> Vikash
>>>
>>>>
>>>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>>>> index e424ea84c211f473a799481fd5463a16580187ed..440a0d7cdfe19a1ccedefc207d96b26eed5d6630 100644
>>>> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>>>> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>>>> @@ -28,7 +28,7 @@ properties:
>>>>      items:
>>>>        - const: venus
>>>>        - const: vcodec0
>>>> -      - const: mxc
>>>> +      - const: mx
>>>>        - const: mmcx
>>>>  
>>>>    clocks:
>>>>
>>>> -- 
>>>> 2.34.1
>>>>
>>>
>
Dmitry Baryshkov March 11, 2025, 10:13 p.m. UTC | #9
On Tue, 11 Mar 2025 at 21:19, Vikash Garodia <quic_vgarodia@quicinc.com> wrote:
>
>
> On 3/11/2025 9:05 PM, Dmitry Baryshkov wrote:
> > On Tue, Mar 11, 2025 at 08:41:01PM +0530, Vikash Garodia wrote:
> >>
> >> On 3/11/2025 8:37 PM, Dmitry Baryshkov wrote:
> >>> On Tue, Mar 11, 2025 at 05:33:53PM +0530, Vikash Garodia wrote:
> >>>> Not all platforms has a collapsible mx, so use the more generic naming
> >>>> of mx in the binding.
> >>>
> >>> I guess, it wasn't even tested...
> >> Not sure what made you guess so, let me check why my binding checker did not
> >> catch the bot reported warning.
> >
> > Obvious: you are changing the bindings in a non-backwards compatible
> > way, but you are not changing the example in the same file (and
> > obviously you are not changing the DTs), which means that this wasn't
> > tested.
> >
> > Hint: you can use enum [mx, mxc] instead of const. That would make it
> > backwards compatible.
> Currently there are no user of this binding. Given that either of MX or MXC are
> same connection to video hardware, just that one is collapsible, it would be
> good to replace the existing element instead of enum.

This obviously should go to the commit message.

>
> Regards,
> Vikash
> >
> >> Regards,
> >> Vikash
> >>>
> >>>>
> >>>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> >>>> ---
> >>>>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >>>> index e424ea84c211f473a799481fd5463a16580187ed..440a0d7cdfe19a1ccedefc207d96b26eed5d6630 100644
> >>>> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >>>> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >>>> @@ -28,7 +28,7 @@ properties:
> >>>>      items:
> >>>>        - const: venus
> >>>>        - const: vcodec0
> >>>> -      - const: mxc
> >>>> +      - const: mx
> >>>>        - const: mmcx
> >>>>
> >>>>    clocks:
> >>>>
> >>>> --
> >>>> 2.34.1
> >>>>
> >>>
> >
Krzysztof Kozlowski March 12, 2025, 8:44 a.m. UTC | #10
On 11/03/2025 18:47, Vikash Garodia wrote:
> 
> On 3/11/2025 11:03 PM, Krzysztof Kozlowski wrote:
>> On 11/03/2025 13:03, Vikash Garodia wrote:
>>> Not all platforms has a collapsible mx, so use the more generic naming
>>> of mx in the binding.
>>>
>>
>> No, neither tested, nor justified. Read the file. How many platforms do
>> you have there? One. Out of this one platform you claim not all of them
>> have MX collapsible, so you want MX?
> Let say we have one which is non-collapsible, what should be the way in that
> case to use the bindings which differ only in the MX/MXC part ?


I don't care about imaginary things. Send patches for real hardware. How
does collapsibility of the domain change the real hardware interface?

Best regards,
Krzysztof
Vikash Garodia March 12, 2025, 12:55 p.m. UTC | #11
On 3/12/2025 2:14 PM, Krzysztof Kozlowski wrote:
> On 11/03/2025 18:47, Vikash Garodia wrote:
>>
>> On 3/11/2025 11:03 PM, Krzysztof Kozlowski wrote:
>>> On 11/03/2025 13:03, Vikash Garodia wrote:
>>>> Not all platforms has a collapsible mx, so use the more generic naming
>>>> of mx in the binding.
>>>>
>>>
>>> No, neither tested, nor justified. Read the file. How many platforms do
>>> you have there? One. Out of this one platform you claim not all of them
>>> have MX collapsible, so you want MX?
>> Let say we have one which is non-collapsible, what should be the way in that
>> case to use the bindings which differ only in the MX/MXC part ?
> 
> 
> I don't care about imaginary things. Send patches for real hardware. How
> does collapsibility of the domain change the real hardware interface?
It does not. I am now thinking to drop this patch altogether, and continue to
use MXC as defined in bindings, irrespective of connection to hardware as MX or
MXC. For ex SM8550/SA8775P have MXC, while QCS8300 have MX, but again, as you
mentioned, these difference just alters some property in DT, binding can remain
same.
Regards,
Vikash
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
index e424ea84c211f473a799481fd5463a16580187ed..440a0d7cdfe19a1ccedefc207d96b26eed5d6630 100644
--- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
@@ -28,7 +28,7 @@  properties:
     items:
       - const: venus
       - const: vcodec0
-      - const: mxc
+      - const: mx
       - const: mmcx
 
   clocks: