[PATCH 0/2] dw9807 binding and driver compatible changes

[PATCH 0/2] dw9807 binding and driver compatible changes

[Sakari Ailus]

Hello everyone,

These two patches convert dw9807 bindings to json-schema and add a new
compatible string to the dw9807-vcm driver in order to support existing
ACPI firmware.

Sakari Ailus (2):
  dt-bindings: Convert Dongwoon dw9807-vcm bindings to json-schema
  dw9807-vcm: Add "dongwoon,dw9807" compatible string

 .../media/i2c/dongwoon,dw9807-vcm.txt         |  9 ----
 .../media/i2c/dongwoon,dw9807-vcm.yaml        | 43 +++++++++++++++++++
 drivers/media/i2c/dw9807-vcm.c                |  2 +
 3 files changed, 45 insertions(+), 9 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.yaml

-- 
2.30.2

[PATCH 1/2] dt-bindings: Convert Dongwoon dw9807-vcm bindings to json-schema

[Sakari Ailus]

Convert the old text based dw9807-vcm chip DT bindings to json-schema.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../media/i2c/dongwoon,dw9807-vcm.txt         |  9 ----
 .../media/i2c/dongwoon,dw9807-vcm.yaml        | 43 +++++++++++++++++++
 2 files changed, 43 insertions(+), 9 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.txt b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.txt
deleted file mode 100644
index c4701f1eaaf63..0000000000000
--- a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.txt
+++ /dev/null
@@ -1,9 +0,0 @@
-Dongwoon Anatech DW9807 voice coil lens driver
-
-DW9807 is a 10-bit DAC with current sink capability. It is intended for
-controlling voice coil lenses.
-
-Mandatory properties:
-
-- compatible: "dongwoon,dw9807-vcm"
-- reg: I2C slave address
diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.yaml b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.yaml
new file mode 100644
index 0000000000000..0cf153ed547fe
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2018, 2021 Intel Corporation
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9807-vcm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dongwoon Anatech DW9807 voice coil lens driver
+
+maintainers:
+  - Sakari Ailus <sakari.ailus@linux.intel.com>
+
+description: |
+  DW9807 is a 10-bit DAC with current sink capability. It is intended for
+  controlling voice coil lenses.
+
+properties:
+
+  compatible:
+    const: "dongwoon,dw9807-vcm"
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        lens@e {
+            compatible = "dongwoon,dw9807-vcm";
+            reg = <0x0e>;
+        };
+    };
+...
-- 
2.30.2

Re: [PATCH 1/2] dt-bindings: Convert Dongwoon dw9807-vcm bindings to json-schema

[Krzysztof Kozlowski]

On 18/03/2022 17:51, Sakari Ailus wrote:
> Convert the old text based dw9807-vcm chip DT bindings to json-schema.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  .../media/i2c/dongwoon,dw9807-vcm.txt         |  9 ----
>  .../media/i2c/dongwoon,dw9807-vcm.yaml        | 43 +++++++++++++++++++
>  2 files changed, 43 insertions(+), 9 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.txt
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.txt b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.txt
> deleted file mode 100644
> index c4701f1eaaf63..0000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.txt
> +++ /dev/null
> @@ -1,9 +0,0 @@
> -Dongwoon Anatech DW9807 voice coil lens driver
> -
> -DW9807 is a 10-bit DAC with current sink capability. It is intended for
> -controlling voice coil lenses.
> -
> -Mandatory properties:
> -
> -- compatible: "dongwoon,dw9807-vcm"
> -- reg: I2C slave address
> diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.yaml b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.yaml
> new file mode 100644
> index 0000000000000..0cf153ed547fe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807-vcm.yaml
> @@ -0,0 +1,43 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2018, 2021 Intel Corporation
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9807-vcm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Dongwoon Anatech DW9807 voice coil lens driver
> +
> +maintainers:
> +  - Sakari Ailus <sakari.ailus@linux.intel.com>
> +
> +description: |
> +  DW9807 is a 10-bit DAC with current sink capability. It is intended for
> +  controlling voice coil lenses.
> +
> +properties:
> +

No blank line.

> +  compatible:
> +    const: "dongwoon,dw9807-vcm"

No quotes.

> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +

No blank line.


With the changes above:

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: Convert Dongwoon dw9807-vcm bindings to json-schema

[Sakari Ailus]

Hi Krzysztof,

On Sun, Mar 20, 2022 at 12:56:48PM +0100, Krzysztof Kozlowski wrote:
> On 18/03/2022 17:51, Sakari Ailus wrote:
> > +$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9807-vcm.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Dongwoon Anatech DW9807 voice coil lens driver
> > +
> > +maintainers:
> > +  - Sakari Ailus <sakari.ailus@linux.intel.com>
> > +
> > +description: |
> > +  DW9807 is a 10-bit DAC with current sink capability. It is intended for
> > +  controlling voice coil lenses.
> > +
> > +properties:
> > +
> 
> No blank line.
> 
> > +  compatible:
> > +    const: "dongwoon,dw9807-vcm"
> 
> No quotes.

Hmm. Strings generally need to be quoted, including the compatible
strings.

I'll remove the extra blank lines.

-- 
Sakari Ailus

Re: [PATCH 1/2] dt-bindings: Convert Dongwoon dw9807-vcm bindings to json-schema

[Krzysztof Kozlowski]

On 23/03/2022 15:54, Sakari Ailus wrote:
> Hi Krzysztof,
> 
> On Sun, Mar 20, 2022 at 12:56:48PM +0100, Krzysztof Kozlowski wrote:
>> On 18/03/2022 17:51, Sakari Ailus wrote:
>>> +$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9807-vcm.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Dongwoon Anatech DW9807 voice coil lens driver
>>> +
>>> +maintainers:
>>> +  - Sakari Ailus <sakari.ailus@linux.intel.com>
>>> +
>>> +description: |
>>> +  DW9807 is a 10-bit DAC with current sink capability. It is intended for
>>> +  controlling voice coil lenses.
>>> +
>>> +properties:
>>> +
>>
>> No blank line.
>>
>>> +  compatible:
>>> +    const: "dongwoon,dw9807-vcm"
>>
>> No quotes.
> 
> Hmm. Strings generally need to be quoted, including the compatible
> strings.

No, not in dtschema. Almost none of strings are ever quoted. If you need
reference - you example-schema.yaml.


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: Convert Dongwoon dw9807-vcm bindings to json-schema

[Sakari Ailus]

On Wed, Mar 23, 2022 at 03:55:50PM +0100, Krzysztof Kozlowski wrote:
> On 23/03/2022 15:54, Sakari Ailus wrote:
> > Hi Krzysztof,
> > 
> > On Sun, Mar 20, 2022 at 12:56:48PM +0100, Krzysztof Kozlowski wrote:
> >> On 18/03/2022 17:51, Sakari Ailus wrote:
> >>> +$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9807-vcm.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Dongwoon Anatech DW9807 voice coil lens driver
> >>> +
> >>> +maintainers:
> >>> +  - Sakari Ailus <sakari.ailus@linux.intel.com>
> >>> +
> >>> +description: |
> >>> +  DW9807 is a 10-bit DAC with current sink capability. It is intended for
> >>> +  controlling voice coil lenses.
> >>> +
> >>> +properties:
> >>> +
> >>
> >> No blank line.
> >>
> >>> +  compatible:
> >>> +    const: "dongwoon,dw9807-vcm"
> >>
> >> No quotes.
> > 
> > Hmm. Strings generally need to be quoted, including the compatible
> > strings.
> 
> No, not in dtschema. Almost none of strings are ever quoted. If you need
> reference - you example-schema.yaml.

Good point. I suppose it's because YAML doesn't require that.

I'll drop the quotes in v2.

-- 
Sakari Ailus

[PATCH 2/2] dw9807-vcm: Add "dongwoon,dw9807" compatible string

[Sakari Ailus]

There is firmware out there that uses "dongwoon,dw9807" compatible string
that never made it to upstream as-is. Add it to the driver to make it load
on such systems.

The chip also has an EEPROM part which is AT24 compatible (for reading
purposes) on a separate I²C address. Adding possible support for this in
the future is not affected by this change.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/dw9807-vcm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/i2c/dw9807-vcm.c b/drivers/media/i2c/dw9807-vcm.c
index 95e06f13bc9ed..ada8e467a0450 100644
--- a/drivers/media/i2c/dw9807-vcm.c
+++ b/drivers/media/i2c/dw9807-vcm.c
@@ -295,6 +295,8 @@ static int  __maybe_unused dw9807_vcm_resume(struct device *dev)
 
 static const struct of_device_id dw9807_of_table[] = {
 	{ .compatible = "dongwoon,dw9807-vcm" },
+	/* Compatibility for older firmware */
+	{ .compatible = "dongwoon,dw9807" },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, dw9807_of_table);
-- 
2.30.2

Re: [PATCH 2/2] dw9807-vcm: Add "dongwoon,dw9807" compatible string

[Krzysztof Kozlowski]

On 18/03/2022 17:51, Sakari Ailus wrote:
> There is firmware out there that uses "dongwoon,dw9807" compatible string
> that never made it to upstream as-is. Add it to the driver to make it load
> on such systems.
> 
> The chip also has an EEPROM part which is AT24 compatible (for reading
> purposes) on a separate I²C address. Adding possible support for this in
> the future is not affected by this change.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/dw9807-vcm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/i2c/dw9807-vcm.c b/drivers/media/i2c/dw9807-vcm.c
> index 95e06f13bc9ed..ada8e467a0450 100644
> --- a/drivers/media/i2c/dw9807-vcm.c
> +++ b/drivers/media/i2c/dw9807-vcm.c
> @@ -295,6 +295,8 @@ static int  __maybe_unused dw9807_vcm_resume(struct device *dev)
>  
>  static const struct of_device_id dw9807_of_table[] = {
>  	{ .compatible = "dongwoon,dw9807-vcm" },
> +	/* Compatibility for older firmware */
> +	{ .compatible = "dongwoon,dw9807" },

You need to add it to the bindings as well.

Best regards,
Krzysztof

Re: [PATCH 2/2] dw9807-vcm: Add "dongwoon,dw9807" compatible string

[Sakari Ailus]

Hi Krysztof,

On Sun, Mar 20, 2022 at 12:58:08PM +0100, Krzysztof Kozlowski wrote:
> On 18/03/2022 17:51, Sakari Ailus wrote:
> > There is firmware out there that uses "dongwoon,dw9807" compatible string
> > that never made it to upstream as-is. Add it to the driver to make it load
> > on such systems.
> > 
> > The chip also has an EEPROM part which is AT24 compatible (for reading
> > purposes) on a separate I²C address. Adding possible support for this in
> > the future is not affected by this change.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/i2c/dw9807-vcm.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/media/i2c/dw9807-vcm.c b/drivers/media/i2c/dw9807-vcm.c
> > index 95e06f13bc9ed..ada8e467a0450 100644
> > --- a/drivers/media/i2c/dw9807-vcm.c
> > +++ b/drivers/media/i2c/dw9807-vcm.c
> > @@ -295,6 +295,8 @@ static int  __maybe_unused dw9807_vcm_resume(struct device *dev)
> >  
> >  static const struct of_device_id dw9807_of_table[] = {
> >  	{ .compatible = "dongwoon,dw9807-vcm" },
> > +	/* Compatibility for older firmware */
> > +	{ .compatible = "dongwoon,dw9807" },
> 
> You need to add it to the bindings as well.

Why? Generally things that are there for binary compatibility but
deprecated are not documented in bindings.

-- 
Sakari Ailus

Re: [PATCH 2/2] dw9807-vcm: Add "dongwoon,dw9807" compatible string

[Krzysztof Kozlowski]

On 23/03/2022 15:50, Sakari Ailus wrote:
> Hi Krysztof,
> 
> On Sun, Mar 20, 2022 at 12:58:08PM +0100, Krzysztof Kozlowski wrote:
>> On 18/03/2022 17:51, Sakari Ailus wrote:
>>> There is firmware out there that uses "dongwoon,dw9807" compatible string
>>> that never made it to upstream as-is. Add it to the driver to make it load
>>> on such systems.
>>>
>>> The chip also has an EEPROM part which is AT24 compatible (for reading
>>> purposes) on a separate I²C address. Adding possible support for this in
>>> the future is not affected by this change.
>>>
>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>> ---
>>>  drivers/media/i2c/dw9807-vcm.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/media/i2c/dw9807-vcm.c b/drivers/media/i2c/dw9807-vcm.c
>>> index 95e06f13bc9ed..ada8e467a0450 100644
>>> --- a/drivers/media/i2c/dw9807-vcm.c
>>> +++ b/drivers/media/i2c/dw9807-vcm.c
>>> @@ -295,6 +295,8 @@ static int  __maybe_unused dw9807_vcm_resume(struct device *dev)
>>>  
>>>  static const struct of_device_id dw9807_of_table[] = {
>>>  	{ .compatible = "dongwoon,dw9807-vcm" },
>>> +	/* Compatibility for older firmware */
>>> +	{ .compatible = "dongwoon,dw9807" },
>>
>> You need to add it to the bindings as well.
> 
> Why? Generally things that are there for binary compatibility but
> deprecated are not documented in bindings.

Because:
1. Otherwise it will get removed by someone as undocumented, unknown
stuff. Someone might treat it as mistake.
2. Putting it in the driver code is already self-documenting, so someone
might start using it. This should be clearly discouraged and bindings
help in it - the compatible is marked as deprecated with the comment you
mentioned.
3. Bindings are used beyond Linux kernel, so they might document
something more than we use here in DT.


Best regards,
Krzysztof

Re: [PATCH 2/2] dw9807-vcm: Add "dongwoon,dw9807" compatible string

[Sakari Ailus]

On Wed, Mar 23, 2022 at 03:59:03PM +0100, Krzysztof Kozlowski wrote:
> On 23/03/2022 15:50, Sakari Ailus wrote:
> > Hi Krysztof,
> > 
> > On Sun, Mar 20, 2022 at 12:58:08PM +0100, Krzysztof Kozlowski wrote:
> >> On 18/03/2022 17:51, Sakari Ailus wrote:
> >>> There is firmware out there that uses "dongwoon,dw9807" compatible string
> >>> that never made it to upstream as-is. Add it to the driver to make it load
> >>> on such systems.
> >>>
> >>> The chip also has an EEPROM part which is AT24 compatible (for reading
> >>> purposes) on a separate I²C address. Adding possible support for this in
> >>> the future is not affected by this change.
> >>>
> >>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >>> ---
> >>>  drivers/media/i2c/dw9807-vcm.c | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/media/i2c/dw9807-vcm.c b/drivers/media/i2c/dw9807-vcm.c
> >>> index 95e06f13bc9ed..ada8e467a0450 100644
> >>> --- a/drivers/media/i2c/dw9807-vcm.c
> >>> +++ b/drivers/media/i2c/dw9807-vcm.c
> >>> @@ -295,6 +295,8 @@ static int  __maybe_unused dw9807_vcm_resume(struct device *dev)
> >>>  
> >>>  static const struct of_device_id dw9807_of_table[] = {
> >>>  	{ .compatible = "dongwoon,dw9807-vcm" },
> >>> +	/* Compatibility for older firmware */
> >>> +	{ .compatible = "dongwoon,dw9807" },
> >>
> >> You need to add it to the bindings as well.
> > 
> > Why? Generally things that are there for binary compatibility but
> > deprecated are not documented in bindings.
> 
> Because:
> 1. Otherwise it will get removed by someone as undocumented, unknown
> stuff. Someone might treat it as mistake.
> 2. Putting it in the driver code is already self-documenting, so someone
> might start using it. This should be clearly discouraged and bindings
> help in it - the compatible is marked as deprecated with the comment you
> mentioned.

That's why there's a comment. I could add a warning telling not to use it
anywhere.

This really does not belong to bindings.

> 3. Bindings are used beyond Linux kernel, so they might document
> something more than we use here in DT.

-- 
Sakari Ailus