[PATCH 1/5] media: dt-bindings: media: imx335: Add supply bindings

[Kieran Bingham]

Quoting Sakari Ailus (2023-10-11 12:01:23)
> Hi Kieran,
> 
> On Tue, Oct 10, 2023 at 02:25:09PM +0100, Kieran Bingham wrote:
> > Hi Sakari,
> > 
> > Quoting Sakari Ailus (2023-10-10 07:06:26)
> > > Hi Kieran,
> > > 
> > > On Tue, Oct 10, 2023 at 01:51:22AM +0100, Kieran Bingham wrote:
> > > > Add the bindings for the supply references used on the IMX335.
> > > > 
> > > > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > > > ---
> > > >  .../bindings/media/i2c/sony,imx335.yaml          | 16 ++++++++++++++++
> > > >  1 file changed, 16 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> > > > index a167dcdb3a32..1863b5608a5c 100644
> > > > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> > > > @@ -32,6 +32,15 @@ properties:
> > > >      description: Clock frequency from 6 to 27 MHz, 37.125MHz, 74.25MHz
> > > >      maxItems: 1
> > > >  
> > > > +  avdd-supply:
> > > > +    description: Analog power supply (2.9V)
> > > > +
> > > > +  ovdd-supply:
> > > > +    description: Interface power supply (1.8V)
> > > > +
> > > > +  dvdd-supply:
> > > > +    description: Digital power supply (1.2V)
> > > 
> > > I wonder what's the policy in this case --- some of the regulators are
> > > often hard-wired and the bindings didn't have them previously either (I
> > > wonder why, maybe they were all hard wired in the board??).
> > > 
> > > Could they be optional? The driver will need to be able to do without these
> > > in any case.
> > 
> > Indeed - many devices do not need to define how they are powered up.
> > 
> > But Krzysztof stated that supplies should be required by the bindings on
> > my recent posting for a VCM driver:
> > 
> >  - https://lore.kernel.org/all/6e163f4d-061d-3c20-4c2e-44c74d529f10@linaro.org/
> > 
> > So based on that I have made these 'required'.
> 
> I guess it's good to align bindings regarding this, in practice the driver
> will need to work without regulators (or with dummies), too.
> 
> > 
> > Even in my case here, with a camera module that is compatible with the
> > Raspberry Pi camera connector - there isn't really 3 supplies. It's just
> > a single gpio enable pin to bring this device up for me. Of course
> > that's specific to the module not the sensor.
> 
> How do you declare that in DT? One of the regulators will be a GPIO one?

I have the following as an imx335.dtsi which I include.
It /should/ be an overlay / dtbo - but the current bootloader on the
baord I have doesn't support applying overlays - so I just include it
directly for now.


```
/ {
	/* 24 MHz Crystal on the camera module */
	imx335_inclk_1: imx335_inclk_24m {
		compatible = "fixed-clock";
		#clock-cells = <0>;
		status = "okay";
		clock-frequency = <24000000>;
	};

	reg_imx335_1_3v3: regulator-imx335_1-vdd3v3 {
		compatible = "regulator-fixed";
		pinctrl-names = "default";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
		regulator-name = "IMX335_1_POWER_EN";
		gpio = <&expander0 4 GPIO_ACTIVE_HIGH>;
		vin-supply = <&reg_csi2_3v3>;
		startup-delay-us = <300000>;
		enable-active-high;
	};
};

&i2c3 {
	imx335_0: sensor at 1a {
		compatible = "sony,imx335";
		reg = <0x1a>;

		clocks = <&imx335_inclk_1>;
		clock-names = "xclk";

		rotation = <180>;
		orientation = <0>;

		status = "okay";

		/* The IMX335 module uses *only* the 3v3 line */
		avdd-supply = <&reg_imx335_1_3v3>;
		ovdd-supply = <&reg_imx335_1_3v3>;
		dvdd-supply = <&reg_imx335_1_3v3>;

		port {
			sensor_1_out: endpoint {
				remote-endpoint = <&mipi_csi_1_in>;
				clock-lanes = <0>;
				data-lanes = <1 2 3 4>;
				link-frequencies = /bits/ 64 <594000000>;
			};
		};
	};
};

&mipi_csi_1 {
	status = "okay";

	ports {
		port at 0 {
			mipi_csi_1_in: endpoint {
				remote-endpoint = <&sensor_1_out>;
				clock-lanes = <0>;
				data-lanes = <1 2 3 4>;
			};
		};
	};
};

```

We could argue that the reg_imx335_1_3v3, should be 3 separate
regulators each targetting vin-supply = <&reg_csi2_3v3>;

But they are all wired up to the same enable pin, and I think they would
then fail to probe if they all tried to control that gpio - while a
regulator-fixed can be shared and handles this for us.

The gpio at:

 &reg_imx335_1_3v3 {
 	gpio = <&expander0 4 GPIO_ACTIVE_HIGH>;
 };

connects to the enable line of all three regulators on the camera
module.


In fact - looking at the schematics of the camera module - they all
power up at 'the same time'. There are no hardware delays introduced on
this module, so that might answer the regulator-bulk question on the
driver.
--
Kieran



> 
> -- 
> Regards,
> 
> Sakari Ailus