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@xxxxxxxxxxxxxxxx> > > > > --- > > > > .../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@xxxxxxxxxx/ > > > > 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 = <®_csi2_3v3>; startup-delay-us = <300000>; enable-active-high; }; }; &i2c3 { imx335_0: sensor@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 = <®_imx335_1_3v3>; ovdd-supply = <®_imx335_1_3v3>; dvdd-supply = <®_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@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 = <®_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: ®_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
