From: Sowjanya Komatineni <skomatineni@nvidia.com>
To: Sakari Ailus <sakari.ailus@iki.fi>
Cc: <thierry.reding@gmail.com>, <jonathanh@nvidia.com>,
<frankc@nvidia.com>, <hverkuil@xs4all.nl>,
<luca@lucaceresoli.net>, <leonl@leopardimaging.com>,
<robh+dt@kernel.org>, <lgirdwood@gmail.com>, <broonie@kernel.org>,
<linux-media@vger.kernel.org>, <devicetree@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 3/3] media: i2c: imx274: Add IMX274 power on and off sequence
Date: Thu, 27 Aug 2020 15:55:52 -0700 [thread overview]
Message-ID: <824ced0f-7493-9d2f-10af-5242c7997631@nvidia.com> (raw)
In-Reply-To: <20200813220147.GJ840@valkosipuli.retiisi.org.uk>
On 8/13/20 3:01 PM, Sakari Ailus wrote:
> Hi Sowjanya,
>
> On Fri, Jul 31, 2020 at 09:34:15AM -0700, Sowjanya Komatineni wrote:
>> On 7/31/20 9:26 AM, Sakari Ailus wrote:
>>> Hi Sowjanya,
>>>
>>> Thanks for the patch.
>>>
>>> On Mon, Jul 20, 2020 at 10:01:34AM -0700, Sowjanya Komatineni wrote:
>>>> IMX274 has VANA analog 2.8V supply, VDIG digital core 1.8V supply,
>>>> and VDDL digital io 1.2V supply which are optional based on camera
>>>> module design.
>>>>
>>>> IMX274 also need external 24Mhz clock and is optional based on
>>>> camera module design.
>>> The sensor appears to be able to use other frequencies, too. Could you
>>> check in the driver the frequency is correct? This should be found in DT
>>> bindings, too.
>> External input clock is not in DT. So added it as part of this series.
>>
>> We are mostly using 24Mhz I/P with IMX274 on designs we have and also on
>> leopard module which has onboard XTAL for 24Mhz
> Yes. This information still should be found in DT as the xtal isn't part of
> the sensor.
>
>>>> This patch adds support for IMX274 power on and off to enable and
>>>> disable these supplies and external clock.
>>>>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>> drivers/media/i2c/imx274.c | 132 +++++++++++++++++++++++++++++++++++++++++++--
>>>> 1 file changed, 129 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
>>>> index 55869ff..7157b1d 100644
>>>> --- a/drivers/media/i2c/imx274.c
>>>> +++ b/drivers/media/i2c/imx274.c
>>>> @@ -19,6 +19,7 @@
>>>> #include <linux/module.h>
>>>> #include <linux/of_gpio.h>
>>>> #include <linux/regmap.h>
>>>> +#include <linux/regulator/consumer.h>
>>>> #include <linux/slab.h>
>>>> #include <linux/v4l2-mediabus.h>
>>>> #include <linux/videodev2.h>
>>>> @@ -131,6 +132,15 @@
>>>> #define IMX274_TABLE_WAIT_MS 0
>>>> #define IMX274_TABLE_END 1
>>>> +/* regulator supplies */
>>>> +static const char * const imx274_supply_names[] = {
>>>> + "VDDL", /* IF (1.2V) supply */
>>>> + "VDIG", /* Digital Core (1.8V) supply */
>>>> + "VANA", /* Analog (2.8V) supply */
>>>> +};
>>>> +
>>>> +#define IMX274_NUM_SUPPLIES ARRAY_SIZE(imx274_supply_names)
>>> Please use ARRAY_SIZE() directly.
>>>
>>>> +
>>>> /*
>>>> * imx274 I2C operation related structure
>>>> */
>>>> @@ -501,6 +511,8 @@ struct imx274_ctrls {
>>>> * @frame_rate: V4L2 frame rate structure
>>>> * @regmap: Pointer to regmap structure
>>>> * @reset_gpio: Pointer to reset gpio
>>>> + * @supplies: imx274 analog and digital supplies
>>>> + * @inck: input clock to imx274
>>>> * @lock: Mutex structure
>>>> * @mode: Parameters for the selected readout mode
>>>> */
>>>> @@ -514,6 +526,8 @@ struct stimx274 {
>>>> struct v4l2_fract frame_interval;
>>>> struct regmap *regmap;
>>>> struct gpio_desc *reset_gpio;
>>>> + struct regulator *supplies[IMX274_NUM_SUPPLIES];
>>>> + struct clk *inck;
>>>> struct mutex lock; /* mutex lock for operations */
>>>> const struct imx274_mode *mode;
>>>> };
>>>> @@ -767,6 +781,99 @@ static void imx274_reset(struct stimx274 *priv, int rst)
>>>> usleep_range(IMX274_RESET_DELAY1, IMX274_RESET_DELAY2);
>>>> }
>>>> +/*
>>>> + * imx274_power_on - Function called to power on the sensor
>>>> + * @imx274: Pointer to device structure
>>>> + */
>>>> +static int imx274_power_on(struct device *dev)
>>>> +{
>>>> + struct i2c_client *client = to_i2c_client(dev);
>>>> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
>>>> + struct stimx274 *imx274 = to_imx274(sd);
>>>> + int i, ret;
>>>> +
>>>> + ret = clk_prepare_enable(imx274->inck);
>>>> + if (ret) {
>>>> + dev_err(&imx274->client->dev,
>>>> + "Failed to enable input clock: %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> +
>>> Could you use regulator_bulk_enable() instead? Same for disable.
>> Using regulator_bulk_enable() makes these regulators mandatory.
> How? regulator_bulk_enable() simply does call regulator_enable() on all the
> regulators.
>
regulator_bulk APIs uses regulator_bulk_data and so had to retrieve
regulators from DT with devm_regulator_bulk_get() which don't use
optional regulator get.
next prev parent reply other threads:[~2020-08-27 22:56 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-20 17:01 [PATCH v3 1/3] media: i2c: imx274: Fix Y_OUT_SIZE register setting Sowjanya Komatineni
2020-07-20 17:01 ` [PATCH v3 2/3] dt-bindings: media: imx274: Add optional input clock and supplies Sowjanya Komatineni
2020-07-31 16:19 ` Sakari Ailus
2020-07-31 16:28 ` Mark Brown
2020-07-20 17:01 ` [PATCH v3 3/3] media: i2c: imx274: Add IMX274 power on and off sequence Sowjanya Komatineni
2020-07-21 14:03 ` Luca Ceresoli
2020-07-21 14:28 ` Sowjanya Komatineni
2020-07-31 16:26 ` Sakari Ailus
2020-07-31 16:34 ` Sowjanya Komatineni
2020-08-13 22:01 ` Sakari Ailus
2020-08-27 22:55 ` Sowjanya Komatineni [this message]
2020-08-31 16:25 ` Sowjanya Komatineni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=824ced0f-7493-9d2f-10af-5242c7997631@nvidia.com \
--to=skomatineni@nvidia.com \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=frankc@nvidia.com \
--cc=hverkuil@xs4all.nl \
--cc=jonathanh@nvidia.com \
--cc=leonl@leopardimaging.com \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=luca@lucaceresoli.net \
--cc=robh+dt@kernel.org \
--cc=sakari.ailus@iki.fi \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).