Hi,
On 13-Dec-24 10:04 AM, Sakari Ailus wrote:
> Hi Hans,
>
> Thanks for the set.
>
> On Thu, Nov 28, 2024 at 04:23:38PM +0100, Hans de Goede wrote:
>> On some designs the regulators for the AVDD / DOVDD / DVDD power rails
>> are controlled by Linux.
>>
>> Add support to the driver for getting regulators for these 3 rails and
>> for enabling these regulators when necessary.
>>
>> The datasheet specifies a delay of 0ns between enabling the regulators,
>> IOW they can all 3 be enabled at the same time. This allows using the bulk
>> regulator API.
>>
>> The regulator core will provide dummy regulators for the 3 power-rails
>> when necessary.
>>
>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>> ---
>> drivers/media/i2c/ov2740.c | 30 +++++++++++++++++++++++++++---
>> 1 file changed, 27 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
>> index 14d0a0588cc2..c766c1f83e12 100644
>> --- a/drivers/media/i2c/ov2740.c
>> +++ b/drivers/media/i2c/ov2740.c
>> @@ -11,6 +11,7 @@
>> #include <linux/pm_runtime.h>
>> #include <linux/nvmem-provider.h>
>> #include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>> #include <media/v4l2-ctrls.h>
>> #include <media/v4l2-device.h>
>> #include <media/v4l2-fwnode.h>
>> @@ -76,6 +77,14 @@
>> /* OTP registers from sensor */
>> #define OV2740_REG_OTP_CUSTOMER 0x7010
>>
>> +static const char * const ov2740_supply_name[] = {
>> + "AVDD",
>> + "DOVDD",
>> + "DVDD",
>> +};
>> +
>> +#define OV2740_NUM_SUPPLIES ARRAY_SIZE(ov2740_supply_name)
>> +
>> struct nvm_data {
>> struct nvmem_device *nvmem;
>> struct regmap *regmap;
>> @@ -523,10 +532,11 @@ struct ov2740 {
>> struct v4l2_ctrl *hblank;
>> struct v4l2_ctrl *exposure;
>>
>> - /* GPIOs, clocks */
>> + /* GPIOs, clocks, regulators */
>> struct gpio_desc *reset_gpio;
>> struct gpio_desc *powerdown_gpio;
>> struct clk *clk;
>> + struct regulator_bulk_data supplies[OV2740_NUM_SUPPLIES];
>
> This would be cleaner with plain ARRAY_SIZE(). Same below. I.e. the macro
> is redundant.
Ok, I will change this for v2.
>
>>
>> /* Current mode */
>> const struct ov2740_mode *cur_mode;
>> @@ -1309,6 +1319,7 @@ static int ov2740_suspend(struct device *dev)
>> gpiod_set_value_cansleep(ov2740->reset_gpio, 1);
>> gpiod_set_value_cansleep(ov2740->powerdown_gpio, 1);
>> clk_disable_unprepare(ov2740->clk);
>> + regulator_bulk_disable(OV2740_NUM_SUPPLIES, ov2740->supplies);
>> return 0;
>> }
>>
>> @@ -1318,10 +1329,16 @@ static int ov2740_resume(struct device *dev)
>> struct ov2740 *ov2740 = to_ov2740(sd);
>> int ret;
>>
>> - ret = clk_prepare_enable(ov2740->clk);
>> + ret = regulator_bulk_enable(OV2740_NUM_SUPPLIES, ov2740->supplies);
>> if (ret)
>> return ret;
>>
>> + ret = clk_prepare_enable(ov2740->clk);
>> + if (ret) {
>> + regulator_bulk_disable(OV2740_NUM_SUPPLIES, ov2740->supplies);
>> + return ret;
>> + }
>> +
>> gpiod_set_value_cansleep(ov2740->powerdown_gpio, 0);
>> gpiod_set_value_cansleep(ov2740->reset_gpio, 0);
>> msleep(20);
>> @@ -1334,7 +1351,7 @@ static int ov2740_probe(struct i2c_client *client)
>> struct device *dev = &client->dev;
>> struct ov2740 *ov2740;
>> bool full_power;
>> - int ret;
>> + int i, ret;
>
> unsigned int i
ack.
Regards,
Hans
