[PATCH 1/2] Documentation: dt-bindings: media: i2c: Add ROHM BU64754 bindings

[PATCH 1/2] Documentation: dt-bindings: media: i2c: Add ROHM BU64754 bindings

[Kieran Bingham]

Add YAML device tree bindings for the ROHM BU64754 VCM Motor Driver for
Camera Autofocus.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 .../bindings/media/i2c/rohm,bu64754.yaml      | 48 +++++++++++++++++++
 MAINTAINERS                                   |  7 +++
 2 files changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/rohm,bu64754.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/rohm,bu64754.yaml b/Documentation/devicetree/bindings/media/i2c/rohm,bu64754.yaml
new file mode 100644
index 000000000000..22da4a46bb0c
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/rohm,bu64754.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2023 Ideas on Board Oy.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/rohm,bu64754.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BU64754 Actuator Driver for Camera Autofocus
+
+maintainers:
+  - Kieran Bingham <kieran.bingham@ideasonboard.com>
+
+description: |
+  The BU64754 is an actuator driver IC which can control the actuator
+  position precisely using an internal Hall Sensor.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - rohm,bu64754
+
+  reg:
+    maxItems: 1
+
+  vdd-supply:
+    description:
+      Definition of the regulator used as VDD power supply to the driver.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        lens@76 {
+            compatible = "rohm,bu64754";
+            reg = <0x76>;
+            vdd-supply = <&cam1_reg>;
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 90f13281d297..f43e0ffcaf56 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18570,6 +18570,13 @@ S:	Supported
 F:	drivers/iio/light/rohm-bu27008.c
 F:	drivers/iio/light/rohm-bu27034.c
 
+ROHM BU64754 MOTOR DRIVER FOR CAMERA AUTOFOCUS
+M:	Kieran Bingham <kieran.bingham@ideasonboard.com>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+T:	git git://linuxtv.org/media_tree.git
+F:	Documentation/devicetree/bindings/media/i2c/rohm,bu64754.yaml
+
 ROHM MULTIFUNCTION BD9571MWV-M PMIC DEVICE DRIVERS
 M:	Marek Vasut <marek.vasut+renesas@gmail.com>
 L:	linux-kernel@vger.kernel.org
-- 
2.34.1

Re: [PATCH 1/2] Documentation: dt-bindings: media: i2c: Add ROHM BU64754 bindings

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1689 bytes --]

Yo,

On Fri, Sep 15, 2023 at 05:59:45PM +0100, Kieran Bingham wrote:
> Add YAML device tree bindings for the ROHM BU64754 VCM Motor Driver for
> Camera Autofocus.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  .../bindings/media/i2c/rohm,bu64754.yaml      | 48 +++++++++++++++++++
>  MAINTAINERS                                   |  7 +++
>  2 files changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/rohm,bu64754.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/rohm,bu64754.yaml b/Documentation/devicetree/bindings/media/i2c/rohm,bu64754.yaml
> new file mode 100644
> index 000000000000..22da4a46bb0c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/rohm,bu64754.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2023 Ideas on Board Oy.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/rohm,bu64754.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ROHM BU64754 Actuator Driver for Camera Autofocus
> +
> +maintainers:
> +  - Kieran Bingham <kieran.bingham@ideasonboard.com>
> +
> +description: |

That | is surplus to requirements since you have no formatting to
preserve.

> +  The BU64754 is an actuator driver IC which can control the actuator
> +  position precisely using an internal Hall Sensor.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - rohm,bu64754

Why such a complicated pattern when you only have one item here?
Why not just const: rohm,bu64754?

Otherwise, looks fine to me.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] Documentation: dt-bindings: media: i2c: Add ROHM BU64754 bindings

[Kieran Bingham]

Quoting Conor Dooley (2023-09-15 22:13:40)
> Yo,
> 
> On Fri, Sep 15, 2023 at 05:59:45PM +0100, Kieran Bingham wrote:
> > Add YAML device tree bindings for the ROHM BU64754 VCM Motor Driver for
> > Camera Autofocus.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  .../bindings/media/i2c/rohm,bu64754.yaml      | 48 +++++++++++++++++++
> >  MAINTAINERS                                   |  7 +++
> >  2 files changed, 55 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/rohm,bu64754.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/rohm,bu64754.yaml b/Documentation/devicetree/bindings/media/i2c/rohm,bu64754.yaml
> > new file mode 100644
> > index 000000000000..22da4a46bb0c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/rohm,bu64754.yaml
> > @@ -0,0 +1,48 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2023 Ideas on Board Oy.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/rohm,bu64754.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ROHM BU64754 Actuator Driver for Camera Autofocus
> > +
> > +maintainers:
> > +  - Kieran Bingham <kieran.bingham@ideasonboard.com>
> > +
> > +description: |
> 
> That | is surplus to requirements since you have no formatting to
> preserve.
> 
> > +  The BU64754 is an actuator driver IC which can control the actuator
> > +  position precisely using an internal Hall Sensor.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - rohm,bu64754
> 
> Why such a complicated pattern when you only have one item here?
> Why not just const: rohm,bu64754?

These devices are really simple. I'd expect other versions/variants to
be able to be covered by the same driver in the future, but I can remove
this for now and simplify.

> 
> Otherwise, looks fine to me.


Thanks for the review.

--
Kieran

Re: [PATCH 1/2] Documentation: dt-bindings: media: i2c: Add ROHM BU64754 bindings

[Krzysztof Kozlowski]

On 15/09/2023 18:59, Kieran Bingham wrote:
> Add YAML device tree bindings for the ROHM BU64754 VCM Motor Driver for
> Camera Autofocus.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---

...

> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - rohm,bu64754
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply:

If the supply is not required, how the driver gets any power?

> +    description:
> +      Definition of the regulator used as VDD power supply to the driver.
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        lens@76 {
> +            compatible = "rohm,bu64754";
> +            reg = <0x76>;
> +            vdd-supply = <&cam1_reg>;
> +        };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 90f13281d297..f43e0ffcaf56 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18570,6 +18570,13 @@ S:	Supported
>  F:	drivers/iio/light/rohm-bu27008.c
>  F:	drivers/iio/light/rohm-bu27034.c
>  
> +ROHM BU64754 MOTOR DRIVER FOR CAMERA AUTOFOCUS
> +M:	Kieran Bingham <kieran.bingham@ideasonboard.com>
> +L:	linux-media@vger.kernel.org
> +S:	Maintained
> +T:	git git://linuxtv.org/media_tree.git

Do you have push rights there? The git entry is for entries where you
maintain, which means: you accept and apply patches. This does not look
like the case, so copying subsystem Git entry is pointless. Subsystem
maintainer defines the git tree, not you.

Best regards,
Krzysztof

Re: [PATCH 1/2] Documentation: dt-bindings: media: i2c: Add ROHM BU64754 bindings

[Kieran Bingham]

Quoting Krzysztof Kozlowski (2023-09-17 10:37:07)
> On 15/09/2023 18:59, Kieran Bingham wrote:
> > Add YAML device tree bindings for the ROHM BU64754 VCM Motor Driver for
> > Camera Autofocus.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> 
> ...
> 
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - rohm,bu64754
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  vdd-supply:
> 
> If the supply is not required, how the driver gets any power?

It may not be controllable. It could be wired to be always on.

The device has a low power mode which is controllable by software, so it
could be always powered on and controlled via software only if desired.
The driver handles that already as if the regulator isn't specified the
power down register bits will be set to put it in low power mode.


> 
> > +    description:
> > +      Definition of the regulator used as VDD power supply to the driver.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        lens@76 {
> > +            compatible = "rohm,bu64754";
> > +            reg = <0x76>;
> > +            vdd-supply = <&cam1_reg>;
> > +        };
> > +    };
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 90f13281d297..f43e0ffcaf56 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -18570,6 +18570,13 @@ S:   Supported
> >  F:   drivers/iio/light/rohm-bu27008.c
> >  F:   drivers/iio/light/rohm-bu27034.c
> >  
> > +ROHM BU64754 MOTOR DRIVER FOR CAMERA AUTOFOCUS
> > +M:   Kieran Bingham <kieran.bingham@ideasonboard.com>
> > +L:   linux-media@vger.kernel.org
> > +S:   Maintained
> > +T:   git git://linuxtv.org/media_tree.git
> 
> Do you have push rights there? The git entry is for entries where you
> maintain, which means: you accept and apply patches. This does not look
> like the case, so copying subsystem Git entry is pointless. Subsystem
> maintainer defines the git tree, not you.

I'll drop the line.

> 
> Best regards,
> Krzysztof
>

Re: [PATCH 1/2] Documentation: dt-bindings: media: i2c: Add ROHM BU64754 bindings

[Krzysztof Kozlowski]

On 17/09/2023 12:33, Kieran Bingham wrote:
> Quoting Krzysztof Kozlowski (2023-09-17 10:37:07)
>> On 15/09/2023 18:59, Kieran Bingham wrote:
>>> Add YAML device tree bindings for the ROHM BU64754 VCM Motor Driver for
>>> Camera Autofocus.
>>>
>>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>> ---
>>
>> ...
>>
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - rohm,bu64754
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  vdd-supply:
>>
>> If the supply is not required, how the driver gets any power?
> 
> It may not be controllable. It could be wired to be always on.

That's different. You talk now about board, I am talking about device.
Is the supply required for the device? If yes, then it should be
required by the bindings.

> 
> The device has a low power mode which is controllable by software, so it
> could be always powered on and controlled via software only if desired.
> The driver handles that already as if the regulator isn't specified the
> power down register bits will be set to put it in low power mode.

Best regards,
Krzysztof