Quoting Sakari Ailus (2023-10-11 12:06:19)
> Hi Kieran,
>
> On Wed, Oct 11, 2023 at 10:41:59AM +0100, Kieran Bingham wrote:
> > Quoting Sakari Ailus (2023-10-10 07:12:08)
> > > Hi Kieran,
> > >
> > > On Tue, Oct 10, 2023 at 01:51:23AM +0100, Kieran Bingham wrote:
> > > > Provide support for enabling and disabling regulator supplies to control
> > > > power to the camera sensor.
> > > >
> > > > Signed-off-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
> > > > ---
> > > > drivers/media/i2c/imx335.c | 41 ++++++++++++++++++++++++++++++++++++--
> > > > 1 file changed, 39 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c
> > > > index ec729126274b..bf12b9b69fce 100644
> > > > --- a/drivers/media/i2c/imx335.c
> > > > +++ b/drivers/media/i2c/imx335.c
> > > > @@ -75,6 +75,19 @@ struct imx335_reg_list {
> > > > const struct imx335_reg *regs;
> > > > };
> > > >
> > > > +static const char * const imx335_supply_name[] = {
> > > > + /*
> > > > + * Turn on the power supplies so that they rise in order of
> > > > + * 1.2v,-> 1.8 -> 2.9v
> > >
> > > This won't happen with regulator_bulk_enable(). Does the spec require this?
> >
> > Every camera I've ever seen handles this in hardware. (I know that's not
> > an excuse as somewhere there could be a device that routes each of these
> > separately).
> >
> > Perhaps I shouldn't have added the comment ;-) But I thought it was
> > useful while I was working through anyway, and could be important for
> > other devices indeed.
> >
> > The datasheet states:
> >
> > ```
> > 1. Turn On the power supplies so that the power supplies rise in order
> > of 1.2 V power supply (DVDD1, DVDD2) → 1.8 V power supply (OVDD) → 2.9 V
> > power supply (AVDD1, AVDD2). In addition, all power supplies should
> > finish rising within 200 ms.
>
> That's an annoying requirement but I'd presume this to work just fine in
> practice. The device is reset in any case (as you describe below). Some
> boards might not wire the reset GPIO though, and then it's when it gets
> interesting.
Correct - this board does not expose the reset/XCLR to me anyway, so I
can not control that.
> To literally implement the documented sequence, you should separately
> enable the regulators one by one.
>
> Although I don't object just removing the comment either.
Given that neither the existing user, nor this user (me) need this,
*and* the schematics of my (working) camera module show that it's fine
to enable all regulators at the same time - I'll remove the comment.
--
Thanks
Kieran
>
> --
> Regards,
>
> Sakari Ailus
