The first register bank should be the 'main' register bank, in this case
the CSID wrapper register is responsible for muxing PHY/TPG inputs directly
to CSID or to other blocks such as the Sensor Front End.

commit f4792eeaa971 ("dt-bindings: media: qcom,x1e80100-camss: Fix isp unit address")
assigned the address to the first register bank "csid0" whereas what we
should have done is retained the unit address and moved csid_wrapper to be
the first listed bank.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../devicetree/bindings/media/qcom,x1e80100-camss.yaml       | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
index 2d1662ef522b7..9aaed897f7e0e 100644
--- a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
@@ -21,12 +21,12 @@ properties:
 
   reg-names:
     items:
+      - const: csid_wrapper
       - const: csid0
       - const: csid1
       - const: csid2
       - const: csid_lite0
       - const: csid_lite1
-      - const: csid_wrapper
       - const: csiphy0
       - const: csiphy1
       - const: csiphy2
@@ -190,15 +190,15 @@ examples:
         #address-cells = <2>;
         #size-cells = <2>;
 
-        camss: isp@acb7000 {
+        camss: isp@acb6000 {
             compatible = "qcom,x1e80100-camss";
 
-            reg = <0 0x0acb7000 0 0x2000>,
+            reg = <0 0x0acb6000 0 0x1000>,
+                  <0 0x0acb7000 0 0x2000>,
                   <0 0x0acb9000 0 0x2000>,
                   <0 0x0acbb000 0 0x2000>,
                   <0 0x0acc6000 0 0x1000>,
                   <0 0x0acca000 0 0x1000>,
-                  <0 0x0acb6000 0 0x1000>,
                   <0 0x0ace4000 0 0x1000>,
                   <0 0x0ace6000 0 0x1000>,
                   <0 0x0ace8000 0 0x1000>,
@@ -211,12 +211,12 @@ examples:
                   <0 0x0acc7000 0 0x2000>,
                   <0 0x0accb000 0 0x2000>;
 
-            reg-names = "csid0",
+            reg-names = "csid_wrapper",
+                        "csid0",
                         "csid1",
                         "csid2",
                         "csid_lite0",
                         "csid_lite1",
-                        "csid_wrapper",
                         "csiphy0",
                         "csiphy1",
                         "csiphy2",

-- 
2.52.0

Hi Bryan,

On Wed, 2026-02-25 at 15:11 +0000, Bryan O'Donoghue wrote:
> The first register bank should be the 'main' register bank, in this case
> the CSID wrapper register is responsible for muxing PHY/TPG inputs directly
> to CSID or to other blocks such as the Sensor Front End.
> 
> commit f4792eeaa971 ("dt-bindings: media: qcom,x1e80100-camss: Fix isp unit address")
> assigned the address to the first register bank "csid0" whereas what we
> should have done is retained the unit address and moved csid_wrapper to be
> the first listed bank.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Reviewed-by: Christopher Obbard <christopher.obbard@linaro.org>

> ---
>  .../devicetree/bindings/media/qcom,x1e80100-camss.yaml       | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
> index 2d1662ef522b7..9aaed897f7e0e 100644
> --- a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
> @@ -21,12 +21,12 @@ properties:
>  
>    reg-names:
>      items:
> +      - const: csid_wrapper
>        - const: csid0
>        - const: csid1
>        - const: csid2
>        - const: csid_lite0
>        - const: csid_lite1
> -      - const: csid_wrapper
>        - const: csiphy0
>        - const: csiphy1
>        - const: csiphy2
> @@ -190,15 +190,15 @@ examples:
>          #address-cells = <2>;
>          #size-cells = <2>;
>  
> -        camss: isp@acb7000 {
> +        camss: isp@acb6000 {
>              compatible = "qcom,x1e80100-camss";
>  
> -            reg = <0 0x0acb7000 0 0x2000>,
> +            reg = <0 0x0acb6000 0 0x1000>,
> +                  <0 0x0acb7000 0 0x2000>,
>                    <0 0x0acb9000 0 0x2000>,
>                    <0 0x0acbb000 0 0x2000>,
>                    <0 0x0acc6000 0 0x1000>,
>                    <0 0x0acca000 0 0x1000>,
> -                  <0 0x0acb6000 0 0x1000>,
>                    <0 0x0ace4000 0 0x1000>,
>                    <0 0x0ace6000 0 0x1000>,
>                    <0 0x0ace8000 0 0x1000>,
> @@ -211,12 +211,12 @@ examples:
>                    <0 0x0acc7000 0 0x2000>,
>                    <0 0x0accb000 0 0x2000>;
>  
> -            reg-names = "csid0",
> +            reg-names = "csid_wrapper",
> +                        "csid0",
>                          "csid1",
>                          "csid2",
>                          "csid_lite0",
>                          "csid_lite1",
> -                        "csid_wrapper",
>                          "csiphy0",
>                          "csiphy1",
>                          "csiphy2",

On Wed, Feb 25, 2026 at 03:11:18PM +0000, Bryan O'Donoghue wrote:
> The first register bank should be the 'main' register bank, in this case
> the CSID wrapper register is responsible for muxing PHY/TPG inputs directly
> to CSID or to other blocks such as the Sensor Front End.
> 
> commit f4792eeaa971 ("dt-bindings: media: qcom,x1e80100-camss: Fix isp unit address")

There is no such commit. At least I could not find it.

Also, please wrap in the commit msg according to patch style.

> assigned the address to the first register bank "csid0" whereas what we
> should have done is retained the unit address and moved csid_wrapper to be
> the first listed bank.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  .../devicetree/bindings/media/qcom,x1e80100-camss.yaml       | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
> index 2d1662ef522b7..9aaed897f7e0e 100644
> --- a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
> @@ -21,12 +21,12 @@ properties:
>  
>    reg-names:
>      items:
> +      - const: csid_wrapper

No, that's ABI break for no real reason. This ABI was shipped and there
is upstream (and maybe other) user of it. You cannot change it.

Best regards,
Krzysztof

On 26/02/2026 07:04, Krzysztof Kozlowski wrote:
> No, that's ABI break for no real reason. This ABI was shipped and there
> is upstream (and maybe other) user of it. You cannot change it.

We don't have an upstream user. The dtsi for CAMSS hasn't been committed.

So this is changeable right ?

---
bod

On 26/02/2026 10:25, Bryan O'Donoghue wrote:
> On 26/02/2026 07:04, Krzysztof Kozlowski wrote:
>> No, that's ABI break for no real reason. This ABI was shipped and there
>> is upstream (and maybe other) user of it. You cannot change it.
> 
> We don't have an upstream user. The dtsi for CAMSS hasn't been committed.
> 
> So this is changeable right ?

No. You have user since v6.16 (or v6.15, don't remember). This is
released ABI.

Best regards,
Krzysztof

On 26/02/2026 09:32, Krzysztof Kozlowski wrote:
> On 26/02/2026 10:25, Bryan O'Donoghue wrote:
>> On 26/02/2026 07:04, Krzysztof Kozlowski wrote:
>>> No, that's ABI break for no real reason. This ABI was shipped and there
>>> is upstream (and maybe other) user of it. You cannot change it.
>>
>> We don't have an upstream user. The dtsi for CAMSS hasn't been committed.
>>
>> So this is changeable right ?
> 
> No. You have user since v6.16 (or v6.15, don't remember). This is
> released ABI.

Yes but I distinctly remember us saying if we could prove there was no 
actual _user_ then a change was possible.

Is that not the case ?

---
bod

On 26/02/2026 10:35, Bryan O'Donoghue wrote:
> On 26/02/2026 09:32, Krzysztof Kozlowski wrote:
>> On 26/02/2026 10:25, Bryan O'Donoghue wrote:
>>> On 26/02/2026 07:04, Krzysztof Kozlowski wrote:
>>>> No, that's ABI break for no real reason. This ABI was shipped and there
>>>> is upstream (and maybe other) user of it. You cannot change it.
>>>
>>> We don't have an upstream user. The dtsi for CAMSS hasn't been committed.
>>>
>>> So this is changeable right ?
>>
>> No. You have user since v6.16 (or v6.15, don't remember). This is
>> released ABI.
> 
> Yes but I distinctly remember us saying if we could prove there was no 
> actual _user_ then a change was possible.

I just told you - there is actual mainline user in v6.16 - so whatever
you claim here is incorrect in first assumption. Even if such rule was
true - you can change ABI without user - then you failed here, because
you have user since v6.16.

Just use git grep.

> Is that not the case ?

It WAS NEVER the case. Just read any of multiple replies from me or Rob.

Best regards,
Krzysztof

On 26/02/2026 09:38, Krzysztof Kozlowski wrote:
> I just told you - there is actual mainline user in v6.16 - so whatever
> you claim here is incorrect in first assumption.

I thought we had discussed @ Amsterdam or perhaps Tokyo that if you 
could show there was no external user - i.e. prove no dependency then 
you could change the YAML ?

Specifically check u-boot, FreeBSD etc.

---
bod