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 |
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.
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 >
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 >> >
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 > >> > >
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
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
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
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 >>>> >>> >
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 > >>>> > >>> > >
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
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 --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:
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(-)