[PATCH 1/2] iio/adc: add ad7927 compatibility to ad7923 drivers

[PATCH 1/2] iio/adc: add ad7927 compatibility to ad7923 drivers

[Edmund Berenson]

The ADC ad7927 is compatible with ad7923 driver,
add explicit compatible and select appropriate
channel configuration.

Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
---
 drivers/iio/adc/ad7923.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
index 9d6bf6d0927a..df72e97dc331 100644
--- a/drivers/iio/adc/ad7923.c
+++ b/drivers/iio/adc/ad7923.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * AD7904/AD7914/AD7923/AD7924/AD7908/AD7918/AD7928 SPI ADC driver
+ * AD7904/AD7914/AD7923/AD7924/AD7908/AD7918/AD7927/AD7928 SPI ADC driver
  *
  * Copyright 2011 Analog Devices Inc (from AD7923 Driver)
  * Copyright 2012 CS Systemes d'Information
@@ -367,6 +367,7 @@ static const struct spi_device_id ad7923_id[] = {
 	{"ad7924", AD7924},
 	{"ad7908", AD7908},
 	{"ad7918", AD7918},
+	{"ad7927", AD7928},
 	{"ad7928", AD7928},
 	{}
 };
@@ -379,6 +380,7 @@ static const struct of_device_id ad7923_of_match[] = {
 	{ .compatible = "adi,ad7924", },
 	{ .compatible = "adi,ad7908", },
 	{ .compatible = "adi,ad7918", },
+	{ .compatible = "adi,ad7927", },
 	{ .compatible = "adi,ad7928", },
 	{ },
 };
-- 
2.37.4

[PATCH 2/2] iio/adc: add documentation for adc ad7927

[Edmund Berenson]

Add the ad7927 compatibility string documentation to
ad7923 driver.

Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
---
 Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
index 07f9d1c09c7d..3e9e9a9c18a4 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
@@ -27,6 +27,7 @@ properties:
       - adi,ad7924
       - adi,ad7908
       - adi,ad7918
+      - adi,ad7927
       - adi,ad7928
 
   reg:
-- 
2.37.4

Re: [PATCH 2/2] iio/adc: add documentation for adc ad7927

[Krzysztof Kozlowski]

On 16/11/2022 11:44, Edmund Berenson wrote:
> Add the ad7927 compatibility string documentation to
> ad7923 driver.

Use subject prefixes matching the subsystem (git log --oneline -- ...).

> 
> Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
> Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>


Best regards,
Krzysztof

Re: [PATCH 1/2] iio/adc: add ad7927 compatibility to ad7923 drivers

[Jonathan Cameron]

On Wed, 16 Nov 2022 11:44:52 +0100
Edmund Berenson <edmund.berenson@emlix.com> wrote:

> The ADC ad7927 is compatible with ad7923 driver,
> add explicit compatible and select appropriate
> channel configuration.
> 
> Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
> Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
This should match local style for patch formatting (as with the
dt one that has already been commented on).


iio: adc: ad7928: Add ad7927

The ad7927 is software compatible with the existing AD7928
so just add the ID.

S...

Change itself is fine.

> ---
>  drivers/iio/adc/ad7923.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> index 9d6bf6d0927a..df72e97dc331 100644
> --- a/drivers/iio/adc/ad7923.c
> +++ b/drivers/iio/adc/ad7923.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * AD7904/AD7914/AD7923/AD7924/AD7908/AD7918/AD7928 SPI ADC driver
> + * AD7904/AD7914/AD7923/AD7924/AD7908/AD7918/AD7927/AD7928 SPI ADC driver
>   *
>   * Copyright 2011 Analog Devices Inc (from AD7923 Driver)
>   * Copyright 2012 CS Systemes d'Information
> @@ -367,6 +367,7 @@ static const struct spi_device_id ad7923_id[] = {
>  	{"ad7924", AD7924},
>  	{"ad7908", AD7908},
>  	{"ad7918", AD7918},
> +	{"ad7927", AD7928},
>  	{"ad7928", AD7928},
>  	{}
>  };
> @@ -379,6 +380,7 @@ static const struct of_device_id ad7923_of_match[] = {
>  	{ .compatible = "adi,ad7924", },
>  	{ .compatible = "adi,ad7908", },
>  	{ .compatible = "adi,ad7918", },
> +	{ .compatible = "adi,ad7927", },
>  	{ .compatible = "adi,ad7928", },
>  	{ },
>  };

Re: [PATCH 2/2] iio/adc: add documentation for adc ad7927

[Jonathan Cameron]

On Wed, 16 Nov 2022 11:44:53 +0100
Edmund Berenson <edmund.berenson@emlix.com> wrote:

> Add the ad7927 compatibility string documentation to
> ad7923 driver.
> 
> Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
> Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>

Given you treat it as an ad7928 in the driver, can we have
a fallback compatible for it to that part?  Would allow people
using older kernels (with newer DT) to get support without needing
to patch the driver.

> ---
>  Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> index 07f9d1c09c7d..3e9e9a9c18a4 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> @@ -27,6 +27,7 @@ properties:
>        - adi,ad7924
>        - adi,ad7908
>        - adi,ad7918
> +      - adi,ad7927
>        - adi,ad7928
>  
>    reg:

[PATCH v2 1/2] iio: adc: ad7923: Add ad7927

[Edmund Berenson]

Add ID for ad7927 which is compatible with existing ad7928 driver.

Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
---
 drivers/iio/adc/ad7923.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
index 9d6bf6d0927a..df72e97dc331 100644
--- a/drivers/iio/adc/ad7923.c
+++ b/drivers/iio/adc/ad7923.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * AD7904/AD7914/AD7923/AD7924/AD7908/AD7918/AD7928 SPI ADC driver
+ * AD7904/AD7914/AD7923/AD7924/AD7908/AD7918/AD7927/AD7928 SPI ADC driver
  *
  * Copyright 2011 Analog Devices Inc (from AD7923 Driver)
  * Copyright 2012 CS Systemes d'Information
@@ -367,6 +367,7 @@ static const struct spi_device_id ad7923_id[] = {
 	{"ad7924", AD7924},
 	{"ad7908", AD7908},
 	{"ad7918", AD7918},
+	{"ad7927", AD7928},
 	{"ad7928", AD7928},
 	{}
 };
@@ -379,6 +380,7 @@ static const struct of_device_id ad7923_of_match[] = {
 	{ .compatible = "adi,ad7924", },
 	{ .compatible = "adi,ad7908", },
 	{ .compatible = "adi,ad7918", },
+	{ .compatible = "adi,ad7927", },
 	{ .compatible = "adi,ad7928", },
 	{ },
 };
-- 
2.37.4

[PATCH v2 2/2] dt-bindings: iio: adc: ad7923: adjust documentation

[Edmund Berenson]

- Add the ad7927 compatibility string, with fallback compatibility
to ad7928.
- ad7923 and ad7924 are treated the same in the driver, show
the relationship in the documentation.

Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
---
 .../bindings/iio/adc/adi,ad7923.yaml          | 21 +++++++++++--------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
index 07f9d1c09c7d..165c4b6863e0 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
@@ -11,7 +11,7 @@ maintainers:
 
 description: |
   Analog Devices AD7904, AD7914, AD7923, AD7924 4 Channel ADCs, and AD7908,
-   AD7918, AD7928 8 Channels ADCs.
+   AD7918, AD7927, AD7928 8 Channels ADCs.
 
   Specifications about the part can be found at:
     https://www.analog.com/media/en/technical-documentation/data-sheets/AD7923.pdf
@@ -20,14 +20,17 @@ description: |
 
 properties:
   compatible:
-    enum:
-      - adi,ad7904
-      - adi,ad7914
-      - adi,ad7923
-      - adi,ad7924
-      - adi,ad7908
-      - adi,ad7918
-      - adi,ad7928
+    oneOf:
+      - const: adi,ad7904
+      - const: adi,ad7914
+      - items:
+        - const: adi,ad7923
+        - const: adi,ad7924
+      - const: adi,ad7908
+      - const: adi,ad7918
+      - items:
+        - const: adi,ad7927
+        - const: adi,ad7928
 
   reg:
     maxItems: 1
-- 
2.37.4

Re: [PATCH v2 2/2] dt-bindings: iio: adc: ad7923: adjust documentation

[Rob Herring]

On Sun, 20 Nov 2022 13:45:43 +0100, Edmund Berenson wrote:
> - Add the ad7927 compatibility string, with fallback compatibility
> to ad7928.
> - ad7923 and ad7924 are treated the same in the driver, show
> the relationship in the documentation.
> 
> Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
> Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
> ---
>  .../bindings/iio/adc/adi,ad7923.yaml          | 21 +++++++++++--------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml:27:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml:32:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7923.example.dtb: adc@0: compatible: 'oneOf' conditional failed, one must be fixed:
	['adi,ad7928'] is too short
	'adi,ad7904' was expected
	'adi,ad7914' was expected
	'adi,ad7923' was expected
	'adi,ad7908' was expected
	'adi,ad7918' was expected
	'adi,ad7927' was expected
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7923.example.dtb: adc@0: Unevaluated properties are not allowed ('compatible' was unexpected)
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221120124543.18973-1-edmund.berenson@emlix.com

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.

Re: [PATCH v2 2/2] dt-bindings: iio: adc: ad7923: adjust documentation

[Rob Herring]

On Sun, Nov 20, 2022 at 01:45:43PM +0100, Edmund Berenson wrote:
> - Add the ad7927 compatibility string, with fallback compatibility
> to ad7928.
> - ad7923 and ad7924 are treated the same in the driver, show
> the relationship in the documentation.
> 
> Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
> Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
> ---
>  .../bindings/iio/adc/adi,ad7923.yaml          | 21 +++++++++++--------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> index 07f9d1c09c7d..165c4b6863e0 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> @@ -11,7 +11,7 @@ maintainers:
>  
>  description: |
>    Analog Devices AD7904, AD7914, AD7923, AD7924 4 Channel ADCs, and AD7908,
> -   AD7918, AD7928 8 Channels ADCs.
> +   AD7918, AD7927, AD7928 8 Channels ADCs.
>  
>    Specifications about the part can be found at:
>      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7923.pdf
> @@ -20,14 +20,17 @@ description: |
>  
>  properties:
>    compatible:
> -    enum:
> -      - adi,ad7904
> -      - adi,ad7914
> -      - adi,ad7923
> -      - adi,ad7924
> -      - adi,ad7908
> -      - adi,ad7918
> -      - adi,ad7928
> +    oneOf:
> +      - const: adi,ad7904
> +      - const: adi,ad7914
> +      - items:
> +        - const: adi,ad7923
> +        - const: adi,ad7924
> +      - const: adi,ad7908
> +      - const: adi,ad7918

All the single entry 'const' cases can remain an 'enum'.

> +      - items:
> +        - const: adi,ad7927
> +        - const: adi,ad7928
>  
>    reg:
>      maxItems: 1
> -- 
> 2.37.4
> 
> 

[PATCH v3] dt-bindings: iio: adc: ad7923: adjust documentation

[Edmund Berenson]

- Add the ad7927 compatibility string, with fallback compatibility
to ad7928.
- ad7923 and ad7924 are treated the same in the driver, show
the relationship in the documentation.

Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
---
 .../bindings/iio/adc/adi,ad7923.yaml          | 26 ++++++++++++-------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
index 07f9d1c09c7d..e553853e25d5 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
@@ -11,7 +11,7 @@ maintainers:
 
 description: |
   Analog Devices AD7904, AD7914, AD7923, AD7924 4 Channel ADCs, and AD7908,
-   AD7918, AD7928 8 Channels ADCs.
+   AD7918, AD7927, AD7928 8 Channels ADCs.
 
   Specifications about the part can be found at:
     https://www.analog.com/media/en/technical-documentation/data-sheets/AD7923.pdf
@@ -20,14 +20,22 @@ description: |
 
 properties:
   compatible:
-    enum:
-      - adi,ad7904
-      - adi,ad7914
-      - adi,ad7923
-      - adi,ad7924
-      - adi,ad7908
-      - adi,ad7918
-      - adi,ad7928
+    oneOf:
+      - enum:
+          - adi,ad7904
+          - adi,ad7914
+          - adi,ad7908
+          - adi,ad7918
+          - adi,ad7923
+          - adi,ad7924
+          - adi,ad7927
+          - adi,ad7928
+      - items:
+          - const: adi,ad7923
+          - const: adi,ad7924
+      - items:
+          - const: adi,ad7927
+          - const: adi,ad7928
 
   reg:
     maxItems: 1
-- 
2.37.4

Re: [PATCH v3] dt-bindings: iio: adc: ad7923: adjust documentation

[Krzysztof Kozlowski]

On 20/11/2022 18:06, Edmund Berenson wrote:
> - Add the ad7927 compatibility string, with fallback compatibility
> to ad7928.
> - ad7923 and ad7924 are treated the same in the driver, show
> the relationship in the documentation.
> 
> Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
> Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
> ---
>  .../bindings/iio/adc/adi,ad7923.yaml          | 26 ++++++++++++-------

Do not respond with new patch to some old thread. Each patchset starts a
new thread.

>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> index 07f9d1c09c7d..e553853e25d5 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> @@ -11,7 +11,7 @@ maintainers:
>  
>  description: |
>    Analog Devices AD7904, AD7914, AD7923, AD7924 4 Channel ADCs, and AD7908,
> -   AD7918, AD7928 8 Channels ADCs.
> +   AD7918, AD7927, AD7928 8 Channels ADCs.
>  
>    Specifications about the part can be found at:
>      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7923.pdf
> @@ -20,14 +20,22 @@ description: |
>  
>  properties:
>    compatible:
> -    enum:
> -      - adi,ad7904
> -      - adi,ad7914
> -      - adi,ad7923
> -      - adi,ad7924
> -      - adi,ad7908
> -      - adi,ad7918
> -      - adi,ad7928
> +    oneOf:
> +      - enum:
> +          - adi,ad7904
> +          - adi,ad7914
> +          - adi,ad7908

You already started shuffling the entries, so make them ordered. What's
the point of changing the order from one non-sorted to another non-sorted?

> +          - adi,ad7918
> +          - adi,ad7923
> +          - adi,ad7924

Then deprecate this as alone compatible.

> +          - adi,ad7927> +          - adi,ad7928

Ditto

> +      - items:
> +          - const: adi,ad7923
> +          - const: adi,ad7924

I would expect lower number as fallback.

> +      - items:
> +          - const: adi,ad7927
> +          - const: adi,ad7928

Ditto.

>  
>    reg:
>      maxItems: 1

Best regards,
Krzysztof

Re: [PATCH v3] dt-bindings: iio: adc: ad7923: adjust documentation

[Edmund Berenson]

On Mon, Nov 21, 2022 at 10:13:57AM +0100, Krzysztof Kozlowski wrote:
> On 20/11/2022 18:06, Edmund Berenson wrote:
> > - Add the ad7927 compatibility string, with fallback compatibility
> > to ad7928.
> > - ad7923 and ad7924 are treated the same in the driver, show
> > the relationship in the documentation.
> > 
> > Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
> > Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
> > ---
> >  .../bindings/iio/adc/adi,ad7923.yaml          | 26 ++++++++++++-------
> 
> Do not respond with new patch to some old thread. Each patchset starts a
> new thread.
> 
Sorry I didn't know this is the preferred way. I will send new patch
version as new thread in the future.
> >  1 file changed, 17 insertions(+), 9 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> > index 07f9d1c09c7d..e553853e25d5 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> > @@ -11,7 +11,7 @@ maintainers:
> >  
> >  description: |
> >    Analog Devices AD7904, AD7914, AD7923, AD7924 4 Channel ADCs, and AD7908,
> > -   AD7918, AD7928 8 Channels ADCs.
> > +   AD7918, AD7927, AD7928 8 Channels ADCs.
> >  
> >    Specifications about the part can be found at:
> >      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7923.pdf
> > @@ -20,14 +20,22 @@ description: |
> >  
> >  properties:
> >    compatible:
> > -    enum:
> > -      - adi,ad7904
> > -      - adi,ad7914
> > -      - adi,ad7923
> > -      - adi,ad7924
> > -      - adi,ad7908
> > -      - adi,ad7918
> > -      - adi,ad7928
> > +    oneOf:
> > +      - enum:
> > +          - adi,ad7904
> > +          - adi,ad7914
> > +          - adi,ad7908
> 
> You already started shuffling the entries, so make them ordered. What's
> the point of changing the order from one non-sorted to another non-sorted?
> 
> > +          - adi,ad7918
> > +          - adi,ad7923
> > +          - adi,ad7924
> 
> Then deprecate this as alone compatible.
> 
> > +          - adi,ad7927> +          - adi,ad7928
> 
> Ditto
> 
> > +      - items:
> > +          - const: adi,ad7923
> > +          - const: adi,ad7924
> 
> I would expect lower number as fallback.
If I remove alone compatibility of 7924 and 7927 in the documentation,
I will have to remove explicit compatibility match on the driver side,
correct?
Just want to make sure I don't misunderstand you.
> 
> > +      - items:
> > +          - const: adi,ad7927
> > +          - const: adi,ad7928
> 
> Ditto.
> 
> >  
> >    reg:
> >      maxItems: 1
> 
> Best regards,
> Krzysztof
> 

Thank you and best regards,
Edmund

Re: [PATCH v3] dt-bindings: iio: adc: ad7923: adjust documentation

[Krzysztof Kozlowski]

On 21/11/2022 11:26, Edmund Berenson wrote:
> On Mon, Nov 21, 2022 at 10:13:57AM +0100, Krzysztof Kozlowski wrote:
>> On 20/11/2022 18:06, Edmund Berenson wrote:
>>> - Add the ad7927 compatibility string, with fallback compatibility
>>> to ad7928.
>>> - ad7923 and ad7924 are treated the same in the driver, show
>>> the relationship in the documentation.
>>>
>>> Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
>>> Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
>>> ---
>>>  .../bindings/iio/adc/adi,ad7923.yaml          | 26 ++++++++++++-------
>>
>> Do not respond with new patch to some old thread. Each patchset starts a
>> new thread.
>>
> Sorry I didn't know this is the preferred way. I will send new patch
> version as new thread in the future.
>>>  1 file changed, 17 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
>>> index 07f9d1c09c7d..e553853e25d5 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
>>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
>>> @@ -11,7 +11,7 @@ maintainers:
>>>  
>>>  description: |
>>>    Analog Devices AD7904, AD7914, AD7923, AD7924 4 Channel ADCs, and AD7908,
>>> -   AD7918, AD7928 8 Channels ADCs.
>>> +   AD7918, AD7927, AD7928 8 Channels ADCs.
>>>  
>>>    Specifications about the part can be found at:
>>>      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7923.pdf
>>> @@ -20,14 +20,22 @@ description: |
>>>  
>>>  properties:
>>>    compatible:
>>> -    enum:
>>> -      - adi,ad7904
>>> -      - adi,ad7914
>>> -      - adi,ad7923
>>> -      - adi,ad7924
>>> -      - adi,ad7908
>>> -      - adi,ad7918
>>> -      - adi,ad7928
>>> +    oneOf:
>>> +      - enum:
>>> +          - adi,ad7904
>>> +          - adi,ad7914
>>> +          - adi,ad7908
>>
>> You already started shuffling the entries, so make them ordered. What's
>> the point of changing the order from one non-sorted to another non-sorted?
>>
>>> +          - adi,ad7918
>>> +          - adi,ad7923
>>> +          - adi,ad7924
>>
>> Then deprecate this as alone compatible.
>>
>>> +          - adi,ad7927> +          - adi,ad7928
>>
>> Ditto
>>
>>> +      - items:
>>> +          - const: adi,ad7923
>>> +          - const: adi,ad7924
>>
>> I would expect lower number as fallback.
> If I remove alone compatibility of 7924 and 7927 in the documentation,

I don't understand. 7924 and 7927 are not compatible with each other -
neither in old code nor in new - so what do you want to remove?

> I will have to remove explicit compatibility match on the driver side,
> correct?
> Just want to make sure I don't misunderstand you.

My comment to which you responded was about order of items. Usually
lower number means older device and usually older device is the fallback.

Best regards,
Krzysztof

Re: [PATCH v3] dt-bindings: iio: adc: ad7923: adjust documentation

[Edmund Berenson]

On Mon, Nov 21, 2022 at 11:31:33AM +0100, Krzysztof Kozlowski wrote:
> On 21/11/2022 11:26, Edmund Berenson wrote:
> > On Mon, Nov 21, 2022 at 10:13:57AM +0100, Krzysztof Kozlowski wrote:
> >> On 20/11/2022 18:06, Edmund Berenson wrote:
> >>> - Add the ad7927 compatibility string, with fallback compatibility
> >>> to ad7928.
> >>> - ad7923 and ad7924 are treated the same in the driver, show
> >>> the relationship in the documentation.
> >>>
> >>> Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
> >>> Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
> >>> ---
> >>>  .../bindings/iio/adc/adi,ad7923.yaml          | 26 ++++++++++++-------
> >>
> >> Do not respond with new patch to some old thread. Each patchset starts a
> >> new thread.
> >>
> > Sorry I didn't know this is the preferred way. I will send new patch
> > version as new thread in the future.
> >>>  1 file changed, 17 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> >>> index 07f9d1c09c7d..e553853e25d5 100644
> >>> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> >>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> >>> @@ -11,7 +11,7 @@ maintainers:
> >>>  
> >>>  description: |
> >>>    Analog Devices AD7904, AD7914, AD7923, AD7924 4 Channel ADCs, and AD7908,
> >>> -   AD7918, AD7928 8 Channels ADCs.
> >>> +   AD7918, AD7927, AD7928 8 Channels ADCs.
> >>>  
> >>>    Specifications about the part can be found at:
> >>>      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7923.pdf
> >>> @@ -20,14 +20,22 @@ description: |
> >>>  
> >>>  properties:
> >>>    compatible:
> >>> -    enum:
> >>> -      - adi,ad7904
> >>> -      - adi,ad7914
> >>> -      - adi,ad7923
> >>> -      - adi,ad7924
> >>> -      - adi,ad7908
> >>> -      - adi,ad7918
> >>> -      - adi,ad7928
> >>> +    oneOf:
> >>> +      - enum:
> >>> +          - adi,ad7904
> >>> +          - adi,ad7914
> >>> +          - adi,ad7908
> >>
> >> You already started shuffling the entries, so make them ordered. What's
> >> the point of changing the order from one non-sorted to another non-sorted?
> >>
> >>> +          - adi,ad7918
> >>> +          - adi,ad7923
> >>> +          - adi,ad7924
> >>
> >> Then deprecate this as alone compatible.
> >>
> >>> +          - adi,ad7927> +          - adi,ad7928
> >>
> >> Ditto
> >>
> >>> +      - items:
> >>> +          - const: adi,ad7923
> >>> +          - const: adi,ad7924
> >>
> >> I would expect lower number as fallback.
> > If I remove alone compatibility of 7924 and 7927 in the documentation,
> 
> I don't understand. 7924 and 7927 are not compatible with each other -
> neither in old code nor in new - so what do you want to remove?
> 
> > I will have to remove explicit compatibility match on the driver side,
> > correct?
> > Just want to make sure I don't misunderstand you.
> 
> My comment to which you responded was about order of items. Usually
> lower number means older device and usually older device is the fallback.
My response was meant to respond to both your comment to "deprecate
alone compatibility" and "lower number should be fallback"
Which I understood in the following way: because 7923, 7924 for one and
7927, 7928 are compatible with each other I will remove
7924 compatible string from driver and not add 7927 to the driver and
only add it to the documentation.
> Best regards,
> Krzysztof
> 

Thank you and best regards,
Edmund

Re: [PATCH v3] dt-bindings: iio: adc: ad7923: adjust documentation

[Krzysztof Kozlowski]

On 21/11/2022 11:45, Edmund Berenson wrote:

>>>> I would expect lower number as fallback.
>>> If I remove alone compatibility of 7924 and 7927 in the documentation,
>>
>> I don't understand. 7924 and 7927 are not compatible with each other -
>> neither in old code nor in new - so what do you want to remove?
>>
>>> I will have to remove explicit compatibility match on the driver side,
>>> correct?
>>> Just want to make sure I don't misunderstand you.
>>
>> My comment to which you responded was about order of items. Usually
>> lower number means older device and usually older device is the fallback.
> My response was meant to respond to both your comment to "deprecate
> alone compatibility" and "lower number should be fallback"
> Which I understood in the following way: because 7923, 7924 for one and
> 7927, 7928 are compatible with each other I will remove
> 7924 compatible string from driver and not add 7927 to the driver and
> only add it to the documentation.

That's not what I suggested. I said nothing about driver, I did not even
look there. I *only* asked to mark old variants as "deprecated: true".
Not remove anything from drivers as this would be obvious ABI break.


Best regards,
Krzysztof

Re: [PATCH v3] dt-bindings: iio: adc: ad7923: adjust documentation

[Jonathan Cameron]

On Mon, 21 Nov 2022 11:45:32 +0100
Edmund Berenson <edmund.berenson@emlix.com> wrote:

> On Mon, Nov 21, 2022 at 11:31:33AM +0100, Krzysztof Kozlowski wrote:
> > On 21/11/2022 11:26, Edmund Berenson wrote:  
> > > On Mon, Nov 21, 2022 at 10:13:57AM +0100, Krzysztof Kozlowski wrote:  
> > >> On 20/11/2022 18:06, Edmund Berenson wrote:  
> > >>> - Add the ad7927 compatibility string, with fallback compatibility
> > >>> to ad7928.
> > >>> - ad7923 and ad7924 are treated the same in the driver, show
> > >>> the relationship in the documentation.
> > >>>
> > >>> Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
> > >>> Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
> > >>> ---
> > >>>  .../bindings/iio/adc/adi,ad7923.yaml          | 26 ++++++++++++-------  
> > >>
> > >> Do not respond with new patch to some old thread. Each patchset starts a
> > >> new thread.
> > >>  
> > > Sorry I didn't know this is the preferred way. I will send new patch
> > > version as new thread in the future.  
> > >>>  1 file changed, 17 insertions(+), 9 deletions(-)
> > >>>
> > >>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> > >>> index 07f9d1c09c7d..e553853e25d5 100644
> > >>> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> > >>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> > >>> @@ -11,7 +11,7 @@ maintainers:
> > >>>  
> > >>>  description: |
> > >>>    Analog Devices AD7904, AD7914, AD7923, AD7924 4 Channel ADCs, and AD7908,
> > >>> -   AD7918, AD7928 8 Channels ADCs.
> > >>> +   AD7918, AD7927, AD7928 8 Channels ADCs.
> > >>>  
> > >>>    Specifications about the part can be found at:
> > >>>      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7923.pdf
> > >>> @@ -20,14 +20,22 @@ description: |
> > >>>  
> > >>>  properties:
> > >>>    compatible:
> > >>> -    enum:
> > >>> -      - adi,ad7904
> > >>> -      - adi,ad7914
> > >>> -      - adi,ad7923
> > >>> -      - adi,ad7924
> > >>> -      - adi,ad7908
> > >>> -      - adi,ad7918
> > >>> -      - adi,ad7928
> > >>> +    oneOf:
> > >>> +      - enum:
> > >>> +          - adi,ad7904
> > >>> +          - adi,ad7914
> > >>> +          - adi,ad7908  
> > >>
> > >> You already started shuffling the entries, so make them ordered. What's
> > >> the point of changing the order from one non-sorted to another non-sorted?
> > >>  
> > >>> +          - adi,ad7918
> > >>> +          - adi,ad7923
> > >>> +          - adi,ad7924  
> > >>
> > >> Then deprecate this as alone compatible.
> > >>  
> > >>> +          - adi,ad7927> +          - adi,ad7928  
> > >>
> > >> Ditto
> > >>  
> > >>> +      - items:
> > >>> +          - const: adi,ad7923
> > >>> +          - const: adi,ad7924  
> > >>
> > >> I would expect lower number as fallback.  
> > > If I remove alone compatibility of 7924 and 7927 in the documentation,  
> > 
> > I don't understand. 7924 and 7927 are not compatible with each other -
> > neither in old code nor in new - so what do you want to remove?
> >   
> > > I will have to remove explicit compatibility match on the driver side,
> > > correct?
> > > Just want to make sure I don't misunderstand you.  
> > 
> > My comment to which you responded was about order of items. Usually
> > lower number means older device and usually older device is the fallback.  

Oldest in which sense?  I think it should be oldest in order of having
a binding defined, not in order of part releases (and ADI seem to scramble
part numbers fairly randomly so definitely not generally the case that
ordering of numbers has anything much to do with age of part).




> My response was meant to respond to both your comment to "deprecate
> alone compatibility" and "lower number should be fallback"
> Which I understood in the following way: because 7923, 7924 for one and
> 7927, 7928 are compatible with each other I will remove
> 7924 compatible string from driver and not add 7927 to the driver and
> only add it to the documentation.
> > Best regards,
> > Krzysztof
> >   
> 
> Thank you and best regards,
> Edmund
> 

Re: [PATCH v3] dt-bindings: iio: adc: ad7923: adjust documentation

[Krzysztof Kozlowski]

On 21/11/2022 13:45, Jonathan Cameron wrote:
> On Mon, 21 Nov 2022 11:45:32 +0100
> Edmund Berenson <edmund.berenson@emlix.com> wrote:
> 
>> On Mon, Nov 21, 2022 at 11:31:33AM +0100, Krzysztof Kozlowski wrote:
>>> On 21/11/2022 11:26, Edmund Berenson wrote:  
>>>> On Mon, Nov 21, 2022 at 10:13:57AM +0100, Krzysztof Kozlowski wrote:  
>>>>> On 20/11/2022 18:06, Edmund Berenson wrote:  
>>>>>> - Add the ad7927 compatibility string, with fallback compatibility
>>>>>> to ad7928.
>>>>>> - ad7923 and ad7924 are treated the same in the driver, show
>>>>>> the relationship in the documentation.
>>>>>>
>>>>>> Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com>
>>>>>> Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com>
>>>>>> ---
>>>>>>  .../bindings/iio/adc/adi,ad7923.yaml          | 26 ++++++++++++-------  
>>>>>
>>>>> Do not respond with new patch to some old thread. Each patchset starts a
>>>>> new thread.
>>>>>  
>>>> Sorry I didn't know this is the preferred way. I will send new patch
>>>> version as new thread in the future.  
>>>>>>  1 file changed, 17 insertions(+), 9 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
>>>>>> index 07f9d1c09c7d..e553853e25d5 100644
>>>>>> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
>>>>>> @@ -11,7 +11,7 @@ maintainers:
>>>>>>  
>>>>>>  description: |
>>>>>>    Analog Devices AD7904, AD7914, AD7923, AD7924 4 Channel ADCs, and AD7908,
>>>>>> -   AD7918, AD7928 8 Channels ADCs.
>>>>>> +   AD7918, AD7927, AD7928 8 Channels ADCs.
>>>>>>  
>>>>>>    Specifications about the part can be found at:
>>>>>>      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7923.pdf
>>>>>> @@ -20,14 +20,22 @@ description: |
>>>>>>  
>>>>>>  properties:
>>>>>>    compatible:
>>>>>> -    enum:
>>>>>> -      - adi,ad7904
>>>>>> -      - adi,ad7914
>>>>>> -      - adi,ad7923
>>>>>> -      - adi,ad7924
>>>>>> -      - adi,ad7908
>>>>>> -      - adi,ad7918
>>>>>> -      - adi,ad7928
>>>>>> +    oneOf:
>>>>>> +      - enum:
>>>>>> +          - adi,ad7904
>>>>>> +          - adi,ad7914
>>>>>> +          - adi,ad7908  
>>>>>
>>>>> You already started shuffling the entries, so make them ordered. What's
>>>>> the point of changing the order from one non-sorted to another non-sorted?
>>>>>  
>>>>>> +          - adi,ad7918
>>>>>> +          - adi,ad7923
>>>>>> +          - adi,ad7924  
>>>>>
>>>>> Then deprecate this as alone compatible.
>>>>>  
>>>>>> +          - adi,ad7927> +          - adi,ad7928  
>>>>>
>>>>> Ditto
>>>>>  
>>>>>> +      - items:
>>>>>> +          - const: adi,ad7923
>>>>>> +          - const: adi,ad7924  
>>>>>
>>>>> I would expect lower number as fallback.  
>>>> If I remove alone compatibility of 7924 and 7927 in the documentation,  
>>>
>>> I don't understand. 7924 and 7927 are not compatible with each other -
>>> neither in old code nor in new - so what do you want to remove?
>>>   
>>>> I will have to remove explicit compatibility match on the driver side,
>>>> correct?
>>>> Just want to make sure I don't misunderstand you.  
>>>
>>> My comment to which you responded was about order of items. Usually
>>> lower number means older device and usually older device is the fallback.  
> 
> Oldest in which sense?  I think it should be oldest in order of having
> a binding defined, not in order of part releases (and ADI seem to scramble
> part numbers fairly randomly so definitely not generally the case that
> ordering of numbers has anything much to do with age of part).

Older in a meaning of design by ADI. Of course I have no clue whether
this matches incremental numbers...

Best regards,
Krzysztof