diff mbox series

[1/2] dt-bindings: media: qcom,x1e80100-camss: Sort interconnect alphabetically

Message ID 20250610083318.2773727-1-vladimir.zapolskiy@linaro.org
State New
Headers show
Series [1/2] dt-bindings: media: qcom,x1e80100-camss: Sort interconnect alphabetically | expand

Commit Message

Vladimir Zapolskiy June 10, 2025, 8:33 a.m. UTC
Sort the entries of interconnect and interconnect-names lists in
alphabetical order.

Fixes: 2ab7f87a7f4b ("dt-bindings: media: Add qcom,x1e80100-camss")
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
Another fix on top of https://lore.kernel.org/all/20250502204142.2064496-1-vladimir.zapolskiy@linaro.org/

 .../devicetree/bindings/media/qcom,x1e80100-camss.yaml | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Dmitry Baryshkov June 10, 2025, 11:11 a.m. UTC | #1
On Tue, Jun 10, 2025 at 11:33:17AM +0300, Vladimir Zapolskiy wrote:
> Sort the entries of interconnect and interconnect-names lists in
> alphabetical order.

This looks like an ABI change. At least you should explain the reason
for the patch.

> 
> Fixes: 2ab7f87a7f4b ("dt-bindings: media: Add qcom,x1e80100-camss")
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
> Another fix on top of https://lore.kernel.org/all/20250502204142.2064496-1-vladimir.zapolskiy@linaro.org/
Vladimir Zapolskiy June 10, 2025, 12:45 p.m. UTC | #2
On 6/10/25 14:14, Bryan O'Donoghue wrote:
> On 10/06/2025 09:33, Vladimir Zapolskiy wrote:
>> Sort the entries of interconnect and interconnect-names lists in
>> alphabetical order.
>>
>> Fixes: 2ab7f87a7f4b ("dt-bindings: media: Add qcom,x1e80100-camss")
>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---
>> Another fix on top of https://lore.kernel.org/all/20250502204142.2064496-1-vladimir.zapolskiy@linaro.org/
>>
>>    .../devicetree/bindings/media/qcom,x1e80100-camss.yaml | 10 +++++-----
>>    1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
>> index c101e42f22ac..7d4e6ef57bf8 100644
>> --- a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
>> +++ b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
>> @@ -100,8 +100,8 @@ properties:
>>        items:
>>          - const: ahb
>>          - const: hf_mnoc
>> -      - const: sf_mnoc
>>          - const: sf_icp_mnoc
>> +      - const: sf_mnoc
>>    
>>      iommus:
>>        maxItems: 8
>> @@ -321,15 +321,15 @@ examples:
>>                                 &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
>>                                <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS
>>                                 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>> -                            <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS
>> -                             &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>>                                <&mmss_noc MASTER_CAMNOC_ICP QCOM_ICC_TAG_ALWAYS
>> +                             &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>> +                            <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS
>>                                 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
>>    
>>                interconnect-names = "ahb",
>>                                     "hf_mnoc",
>> -                                 "sf_mnoc",
>> -                                 "sf_icp_mnoc";
>> +                                 "sf_icp_mnoc",
>> +                                 "sf_mnoc";
>>    
>>                iommus = <&apps_smmu 0x800 0x60>,
>>                         <&apps_smmu 0x860 0x60>,
> 
> How is this a Fixes: ?

I call it the fix to the dt-bindings documentation, then what is this
change, if it's not a fix?..

Anyway, if there is a strong disagreement about if it's a fix or not,
the Fixes tag can be dropped from the change, since it's so secondary.

--
Best wishes,
Vladimir
Vladimir Zapolskiy June 10, 2025, 3:01 p.m. UTC | #3
On 6/10/25 17:44, Dmitry Baryshkov wrote:
> On Tue, Jun 10, 2025 at 03:42:05PM +0300, Vladimir Zapolskiy wrote:
>> On 6/10/25 14:11, Dmitry Baryshkov wrote:
>>> On Tue, Jun 10, 2025 at 11:33:17AM +0300, Vladimir Zapolskiy wrote:
>>>> Sort the entries of interconnect and interconnect-names lists in
>>>> alphabetical order.
>>>
>>> This looks like an ABI change. At least you should explain the reason
>>> for the patch.
>>
>> There was a number of comments and notes on the mailing list that
>> any changes to dt bindings without users are acceptable, i.e. no
>> users implies no ABI change.
> 
> It is still an ABI change, but the one which usually has a waiver. And
> that's why it should be explained in the commit message.

I can resend the change with an update in its commit message stating
that it's an acceptable ABI change.

At once the Fixes tags could be removed, let it be a non-fix ABI change :)

>>
>> Also it was used as a justification to accept dt binding documentation
>> changes without the correspondent .dtsi changes, like in this particular
>> case. So, I believe the room for fixes is still open.
> 
> Yes
> 
>>
>>>>
>>>> Fixes: 2ab7f87a7f4b ("dt-bindings: media: Add qcom,x1e80100-camss")
>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>> ---
>>>> Another fix on top of https://lore.kernel.org/all/20250502204142.2064496-1-vladimir.zapolskiy@linaro.org/
>>>

--
Best wishes,
Vladimir
Vladimir Zapolskiy June 10, 2025, 3:10 p.m. UTC | #4
On 6/10/25 18:02, Bryan O'Donoghue wrote:
> On 10/06/2025 13:45, Vladimir Zapolskiy wrote:
>>>
>>> How is this a Fixes: ?
>>
>> I call it the fix to the dt-bindings documentation, then what is this
>> change, if it's not a fix?..
>>
>> Anyway, if there is a strong disagreement about if it's a fix or not,
>> the Fixes tag can be dropped from the change, since it's so secondary.
> 
> Since we don't have a committed upstream user I don't think this is an
> ABI break.

Well, Dmitry says it's an ABI break... It would be beneficial to come to
a common understanding here.

> But I also don't think it warrants a Fixes: tag either, there's no bug.

There is no bug, but there are Documentation/ changes with Fixes tags,
it's okay.

I will resend the changes with whatever updates requested by both of you,
if they do not contradict to each other.

--
Best wishes,
Vladimir
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
index c101e42f22ac..7d4e6ef57bf8 100644
--- a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
@@ -100,8 +100,8 @@  properties:
     items:
       - const: ahb
       - const: hf_mnoc
-      - const: sf_mnoc
       - const: sf_icp_mnoc
+      - const: sf_mnoc
 
   iommus:
     maxItems: 8
@@ -321,15 +321,15 @@  examples:
                              &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
                             <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS
                              &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
-                            <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS
-                             &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
                             <&mmss_noc MASTER_CAMNOC_ICP QCOM_ICC_TAG_ALWAYS
+                             &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+                            <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS
                              &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
 
             interconnect-names = "ahb",
                                  "hf_mnoc",
-                                 "sf_mnoc",
-                                 "sf_icp_mnoc";
+                                 "sf_icp_mnoc",
+                                 "sf_mnoc";
 
             iommus = <&apps_smmu 0x800 0x60>,
                      <&apps_smmu 0x860 0x60>,