Am 28.06.2017 um 12:50 schrieb Sylwester Nawrocki [off-list ref]:

On 06/28/2017 11:12 AM, H. Nikolaus Schaller wrote:

quoted

quoted

Am 28.06.2017 um 00:57 schrieb Sylwester Nawrocki [off-list ref]:
On 06/27/2017 07:48 AM, H. Nikolaus Schaller wrote:

quoted

quoted

Am 26.06.2017 um 22:04 schrieb Sylwester Nawrocki [off-list ref]:
On 06/26/2017 12:35 PM, Hugues FRUCHET wrote:

quoted

quoted

What I am missing to support the GTA04 camera is the control of the optional "vana-supply".
So the driver does not power up the camera module when needed and therefore probing fails.

   - vana-supply: a regulator to power up the camera module.

Driver code is not complex to add:

quoted

Yes, I saw it in your code, but as I don't have any programmable power
supply on my setup, I have not pushed this commit.

Since you are about to add voltage supplies to the DT binding I'd suggest
to include all three voltage supplies of the sensor chip. Looking at the OV9650
and the OV9655 datasheet there are following names used for the voltage supply
pins:

AVDD - Analog power supply,
DVDD - Power supply for digital core logic,
DOVDD - Digital power supply for I/O.

The latter two are usually not independently switchable from the SoC power
the module is connected to.

And sometimes DVDD and DOVDD are connected together.

So the driver can't make much use of knowing or requesting them because the
1.8V supply is always active, even during suspend.

quoted

I doubt the sensor can work without any of these voltage supplies, thus
regulator_get_optional() should not be used. I would just use the regulator
bulk API to handle all three power supplies.

The digital part works with AVDD turned off. So the LDO supplying AVDD should
be switchable to save power (&vaux3 on the GTA04 device).>
But not all designs can switch it off. Hence the idea to define it as an
/optional/ regulator. If it is not defined by DT, the driver simply assumes
it is always powered on.

I didn't say we can't define regulator supply properties as optional in the DT
binding.  If we define them as such and any of these *-supply properties is
missing in DT with regulator_get() the regulator core will use dummy regulator
for that particular voltage supply.  While with regulator_get_optional()
-ENODEV is returned when the regulator cannot be found.

Ah, ok. I see.

I had thought that it is the right thing to do like devm_gpiod_get_optional().

That one it is described as:

"* This is equivalent to gpiod_get(), except that when no GPIO was assigned to
 * the requested function it will return NULL. This is convenient for drivers
 * that need to handle optional GPIOs."

Seems to be inconsistent definition of what "optional" means.

Indeed, this commit explains it further:

commit de1dd9fd2156874b45803299b3b27e65d5defdd9
regulator: core: Provide hints to the core about optional supplies

quoted

So we indeed should use devm_regulator_get() in this case. Thanks for > pointing out!

quoted

quoted

quoted

So in summary we only need AVDD switched for the GTA04 - but it does not
matter if the others are optional properties. We would not use them.

It does matter if they are mandatory because it adds DT complexity (size
and processing) without added function.

We should not be defining DT binding only with selected use cases/board
designs in mind.  IMO all three voltage supplies should be listed in the
binding, presumably all can be made optional, with an assumption that when
the property is missing selected pin is hooked up to a fixed regulator.

Ok, then it should just be defined in the bindings but not used by 
the driver?

Yes, I think so. So we have a possibly complete binding right from the 
beginning. I someone needs handling other supplies than AVDD they could
update the driver in future.

Fine! I have sent some patches to Hughues so that he can integrate it in
his next version of the patch series.

BR and thanks,
Nikolaus