* [PATCH 0/2] media: Add regulator support to ov9282
@ 2022-10-05 15:20 Dave Stevenson
2022-10-05 15:20 ` [PATCH 1/2] dt-bindings: media: ovti,ov9282: Add optional regulators Dave Stevenson
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Dave Stevenson @ 2022-10-05 15:20 UTC (permalink / raw)
To: paul.j.murphy, daniele.alessandrelli, linux-media, robh+dt, devicetree
Cc: Dave Stevenson
The sensor takes 3 voltage supply rails, so add the relevant
configuration to the device tree bindings and driver.
Dave Stevenson (2):
dt-bindings: media: ovti,ov9282: Add optional regulators
media: i2c: ov9282: Add support for regulators.
.../bindings/media/i2c/ovti,ov9282.yaml | 9 +++++
drivers/media/i2c/ov9282.c | 38 +++++++++++++++++++
2 files changed, 47 insertions(+)
--
2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] dt-bindings: media: ovti,ov9282: Add optional regulators 2022-10-05 15:20 [PATCH 0/2] media: Add regulator support to ov9282 Dave Stevenson @ 2022-10-05 15:20 ` Dave Stevenson 2022-10-06 19:08 ` Rob Herring 2022-10-05 15:20 ` [PATCH 2/2] media: i2c: ov9282: Add support for regulators Dave Stevenson 2022-11-16 16:50 ` [PATCH 0/2] media: Add regulator support to ov9282 Dave Stevenson 2 siblings, 1 reply; 9+ messages in thread From: Dave Stevenson @ 2022-10-05 15:20 UTC (permalink / raw) To: paul.j.murphy, daniele.alessandrelli, linux-media, robh+dt, devicetree Cc: Dave Stevenson The OV9282 image sensor takes 3 voltage supplies, so define the relevant regulators. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> --- .../devicetree/bindings/media/i2c/ovti,ov9282.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml index bf115ab9d926..652b4cfeded7 100644 --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml @@ -36,6 +36,15 @@ properties: description: Reference to the GPIO connected to the XCLR pin, if any. maxItems: 1 + avdd-supply: + description: Analog voltage supply, 2.8 volts + + dvdd-supply: + description: Digital core voltage supply, 1.2 volts + + dovdd-supply: + description: Digital I/O voltage supply, 1.8 volts + port: additionalProperties: false $ref: /schemas/graph.yaml#/$defs/port-base -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] dt-bindings: media: ovti,ov9282: Add optional regulators 2022-10-05 15:20 ` [PATCH 1/2] dt-bindings: media: ovti,ov9282: Add optional regulators Dave Stevenson @ 2022-10-06 19:08 ` Rob Herring 0 siblings, 0 replies; 9+ messages in thread From: Rob Herring @ 2022-10-06 19:08 UTC (permalink / raw) To: Dave Stevenson Cc: linux-media, devicetree, daniele.alessandrelli, robh+dt, paul.j.murphy On Wed, 05 Oct 2022 16:20:17 +0100, Dave Stevenson wrote: > The OV9282 image sensor takes 3 voltage supplies, so > define the relevant regulators. > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > --- > .../devicetree/bindings/media/i2c/ovti,ov9282.yaml | 9 +++++++++ > 1 file changed, 9 insertions(+) > Acked-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] media: i2c: ov9282: Add support for regulators. 2022-10-05 15:20 [PATCH 0/2] media: Add regulator support to ov9282 Dave Stevenson 2022-10-05 15:20 ` [PATCH 1/2] dt-bindings: media: ovti,ov9282: Add optional regulators Dave Stevenson @ 2022-10-05 15:20 ` Dave Stevenson 2022-11-24 9:31 ` Alexander Stein 2022-11-16 16:50 ` [PATCH 0/2] media: Add regulator support to ov9282 Dave Stevenson 2 siblings, 1 reply; 9+ messages in thread From: Dave Stevenson @ 2022-10-05 15:20 UTC (permalink / raw) To: paul.j.murphy, daniele.alessandrelli, linux-media, robh+dt, devicetree Cc: Dave Stevenson The sensor takes 3 supply rails - AVDD, DVDD, and DOVDD. Add hooks into the regulator framework for each of these regulators. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> --- drivers/media/i2c/ov9282.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c index 2e0b315801e5..699fc5b753b4 100644 --- a/drivers/media/i2c/ov9282.c +++ b/drivers/media/i2c/ov9282.c @@ -11,6 +11,7 @@ #include <linux/i2c.h> #include <linux/module.h> #include <linux/pm_runtime.h> +#include <linux/regulator/consumer.h> #include <media/v4l2-ctrls.h> #include <media/v4l2-fwnode.h> @@ -55,6 +56,14 @@ #define OV9282_REG_MIN 0x00 #define OV9282_REG_MAX 0xfffff +static const char * const ov9282_supply_names[] = { + "avdd", /* Analog power */ + "dovdd", /* Digital I/O power */ + "dvdd", /* Digital core power */ +}; + +#define OV9282_NUM_SUPPLIES ARRAY_SIZE(ov9282_supply_names) + /** * struct ov9282_reg - ov9282 sensor register * @address: Register address @@ -128,6 +137,7 @@ struct ov9282 { struct media_pad pad; struct gpio_desc *reset_gpio; struct clk *inclk; + struct regulator_bulk_data supplies[OV9282_NUM_SUPPLIES]; struct v4l2_ctrl_handler ctrl_handler; struct v4l2_ctrl *link_freq_ctrl; struct v4l2_ctrl *pclk_ctrl; @@ -767,6 +777,18 @@ static int ov9282_detect(struct ov9282 *ov9282) return 0; } +static int ov9282_configure_regulators(struct ov9282 *ov9282) +{ + unsigned int i; + + for (i = 0; i < OV9282_NUM_SUPPLIES; i++) + ov9282->supplies[i].supply = ov9282_supply_names[i]; + + return devm_regulator_bulk_get(ov9282->dev, + OV9282_NUM_SUPPLIES, + ov9282->supplies); +} + /** * ov9282_parse_hw_config() - Parse HW configuration and check if supported * @ov9282: pointer to ov9282 device @@ -803,6 +825,12 @@ static int ov9282_parse_hw_config(struct ov9282 *ov9282) return PTR_ERR(ov9282->inclk); } + ret = ov9282_configure_regulators(ov9282); + if (ret) { + dev_err(ov9282->dev, "Failed to get power regulators\n"); + return ret; + } + rate = clk_get_rate(ov9282->inclk); if (rate != OV9282_INCLK_RATE) { dev_err(ov9282->dev, "inclk frequency mismatch"); @@ -874,6 +902,12 @@ static int ov9282_power_on(struct device *dev) struct ov9282 *ov9282 = to_ov9282(sd); int ret; + ret = regulator_bulk_enable(OV9282_NUM_SUPPLIES, ov9282->supplies); + if (ret < 0) { + dev_err(dev, "Failed to enable regulators\n"); + return ret; + } + usleep_range(400, 600); gpiod_set_value_cansleep(ov9282->reset_gpio, 1); @@ -891,6 +925,8 @@ static int ov9282_power_on(struct device *dev) error_reset: gpiod_set_value_cansleep(ov9282->reset_gpio, 0); + regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies); + return ret; } @@ -909,6 +945,8 @@ static int ov9282_power_off(struct device *dev) clk_disable_unprepare(ov9282->inclk); + regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies); + return 0; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] media: i2c: ov9282: Add support for regulators. 2022-10-05 15:20 ` [PATCH 2/2] media: i2c: ov9282: Add support for regulators Dave Stevenson @ 2022-11-24 9:31 ` Alexander Stein 2022-11-24 11:58 ` Dave Stevenson 0 siblings, 1 reply; 9+ messages in thread From: Alexander Stein @ 2022-11-24 9:31 UTC (permalink / raw) To: Dave Stevenson Cc: paul.j.murphy, daniele.alessandrelli, linux-media, robh+dt, devicetree, Dave Stevenson Hello Dave, Am Mittwoch, 5. Oktober 2022, 17:20:18 CET schrieb Dave Stevenson: > The sensor takes 3 supply rails - AVDD, DVDD, and DOVDD. > > Add hooks into the regulator framework for each of these > regulators. > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > --- > drivers/media/i2c/ov9282.c | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c > index 2e0b315801e5..699fc5b753b4 100644 > --- a/drivers/media/i2c/ov9282.c > +++ b/drivers/media/i2c/ov9282.c > @@ -11,6 +11,7 @@ > #include <linux/i2c.h> > #include <linux/module.h> > #include <linux/pm_runtime.h> > +#include <linux/regulator/consumer.h> > > #include <media/v4l2-ctrls.h> > #include <media/v4l2-fwnode.h> > @@ -55,6 +56,14 @@ > #define OV9282_REG_MIN 0x00 > #define OV9282_REG_MAX 0xfffff > > +static const char * const ov9282_supply_names[] = { > + "avdd", /* Analog power */ > + "dovdd", /* Digital I/O power */ > + "dvdd", /* Digital core power */ > +}; > + > +#define OV9282_NUM_SUPPLIES ARRAY_SIZE(ov9282_supply_names) > + > /** > * struct ov9282_reg - ov9282 sensor register > * @address: Register address > @@ -128,6 +137,7 @@ struct ov9282 { > struct media_pad pad; > struct gpio_desc *reset_gpio; > struct clk *inclk; > + struct regulator_bulk_data supplies[OV9282_NUM_SUPPLIES]; Please add documentation for supplies. > struct v4l2_ctrl_handler ctrl_handler; > struct v4l2_ctrl *link_freq_ctrl; > struct v4l2_ctrl *pclk_ctrl; > @@ -767,6 +777,18 @@ static int ov9282_detect(struct ov9282 *ov9282) > return 0; > } > > +static int ov9282_configure_regulators(struct ov9282 *ov9282) > +{ > + unsigned int i; > + > + for (i = 0; i < OV9282_NUM_SUPPLIES; i++) > + ov9282->supplies[i].supply = ov9282_supply_names[i]; > + > + return devm_regulator_bulk_get(ov9282->dev, > + OV9282_NUM_SUPPLIES, > + ov9282->supplies); > +} > + > /** > * ov9282_parse_hw_config() - Parse HW configuration and check if supported > * @ov9282: pointer to ov9282 device > @@ -803,6 +825,12 @@ static int ov9282_parse_hw_config(struct ov9282 > *ov9282) return PTR_ERR(ov9282->inclk); > } > > + ret = ov9282_configure_regulators(ov9282); > + if (ret) { > + dev_err(ov9282->dev, "Failed to get power regulators\n"); dev_err_probe seems sensible here. > + return ret; > + } > + > rate = clk_get_rate(ov9282->inclk); > if (rate != OV9282_INCLK_RATE) { > dev_err(ov9282->dev, "inclk frequency mismatch"); > @@ -874,6 +902,12 @@ static int ov9282_power_on(struct device *dev) > struct ov9282 *ov9282 = to_ov9282(sd); > int ret; > > + ret = regulator_bulk_enable(OV9282_NUM_SUPPLIES, ov9282->supplies); > + if (ret < 0) { > + dev_err(dev, "Failed to enable regulators\n"); > + return ret; > + } > + > usleep_range(400, 600); > > gpiod_set_value_cansleep(ov9282->reset_gpio, 1); > @@ -891,6 +925,8 @@ static int ov9282_power_on(struct device *dev) > error_reset: > gpiod_set_value_cansleep(ov9282->reset_gpio, 0); > > + regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies); > + > return ret; > } > > @@ -909,6 +945,8 @@ static int ov9282_power_off(struct device *dev) > > clk_disable_unprepare(ov9282->inclk); > > + regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies); > + > return 0; > } Despite the nits above Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] media: i2c: ov9282: Add support for regulators. 2022-11-24 9:31 ` Alexander Stein @ 2022-11-24 11:58 ` Dave Stevenson 2022-11-24 12:06 ` Alexander Stein 0 siblings, 1 reply; 9+ messages in thread From: Dave Stevenson @ 2022-11-24 11:58 UTC (permalink / raw) To: Alexander Stein Cc: paul.j.murphy, daniele.alessandrelli, linux-media, robh+dt, devicetree Hi Alexander Thanks for the review. Sakari has already picked this up and included it in a pull to Mauro for 6.2. https://www.spinics.net/lists/linux-media/msg222346.html On Thu, 24 Nov 2022 at 09:31, Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > > Hello Dave, > > Am Mittwoch, 5. Oktober 2022, 17:20:18 CET schrieb Dave Stevenson: > > The sensor takes 3 supply rails - AVDD, DVDD, and DOVDD. > > > > Add hooks into the regulator framework for each of these > > regulators. > > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > --- > > drivers/media/i2c/ov9282.c | 38 ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 38 insertions(+) > > > > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c > > index 2e0b315801e5..699fc5b753b4 100644 > > --- a/drivers/media/i2c/ov9282.c > > +++ b/drivers/media/i2c/ov9282.c > > @@ -11,6 +11,7 @@ > > #include <linux/i2c.h> > > #include <linux/module.h> > > #include <linux/pm_runtime.h> > > +#include <linux/regulator/consumer.h> > > > > #include <media/v4l2-ctrls.h> > > #include <media/v4l2-fwnode.h> > > @@ -55,6 +56,14 @@ > > #define OV9282_REG_MIN 0x00 > > #define OV9282_REG_MAX 0xfffff > > > > +static const char * const ov9282_supply_names[] = { > > + "avdd", /* Analog power */ > > + "dovdd", /* Digital I/O power */ > > + "dvdd", /* Digital core power */ > > +}; > > + > > +#define OV9282_NUM_SUPPLIES ARRAY_SIZE(ov9282_supply_names) > > + > > /** > > * struct ov9282_reg - ov9282 sensor register > > * @address: Register address > > @@ -128,6 +137,7 @@ struct ov9282 { > > struct media_pad pad; > > struct gpio_desc *reset_gpio; > > struct clk *inclk; > > + struct regulator_bulk_data supplies[OV9282_NUM_SUPPLIES]; > > Please add documentation for supplies. Is it the place for the driver to document the supplies beyond the comments in ov9282_supply_names with regard to which sensor rail they relate to? Some drivers include the typical values for each supply, but those are technically inaccurate as each will have a min and max value. Anyone interfacing with a sensor is going to have the datasheet for it and should be referring to that for the characteristics of supply rails. Duplicating some of that in the driver seems redundant, and has the potential to be incorrect. > > struct v4l2_ctrl_handler ctrl_handler; > > struct v4l2_ctrl *link_freq_ctrl; > > struct v4l2_ctrl *pclk_ctrl; > > @@ -767,6 +777,18 @@ static int ov9282_detect(struct ov9282 *ov9282) > > return 0; > > } > > > > +static int ov9282_configure_regulators(struct ov9282 *ov9282) > > +{ > > + unsigned int i; > > + > > + for (i = 0; i < OV9282_NUM_SUPPLIES; i++) > > + ov9282->supplies[i].supply = ov9282_supply_names[i]; > > + > > + return devm_regulator_bulk_get(ov9282->dev, > > + OV9282_NUM_SUPPLIES, > > + ov9282->supplies); > > +} > > + > > /** > > * ov9282_parse_hw_config() - Parse HW configuration and check if supported > > * @ov9282: pointer to ov9282 device > > @@ -803,6 +825,12 @@ static int ov9282_parse_hw_config(struct ov9282 > > *ov9282) return PTR_ERR(ov9282->inclk); > > } > > > > + ret = ov9282_configure_regulators(ov9282); > > + if (ret) { > > + dev_err(ov9282->dev, "Failed to get power regulators\n"); > > dev_err_probe seems sensible here. That would have been good - sorry. I must get into the habit of remembering to use dev_err_probe. Dave > > + return ret; > > + } > > + > > rate = clk_get_rate(ov9282->inclk); > > if (rate != OV9282_INCLK_RATE) { > > dev_err(ov9282->dev, "inclk frequency mismatch"); > > @@ -874,6 +902,12 @@ static int ov9282_power_on(struct device *dev) > > struct ov9282 *ov9282 = to_ov9282(sd); > > int ret; > > > > + ret = regulator_bulk_enable(OV9282_NUM_SUPPLIES, ov9282->supplies); > > + if (ret < 0) { > > + dev_err(dev, "Failed to enable regulators\n"); > > + return ret; > > + } > > + > > usleep_range(400, 600); > > > > gpiod_set_value_cansleep(ov9282->reset_gpio, 1); > > @@ -891,6 +925,8 @@ static int ov9282_power_on(struct device *dev) > > error_reset: > > gpiod_set_value_cansleep(ov9282->reset_gpio, 0); > > > > + regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies); > > + > > return ret; > > } > > > > @@ -909,6 +945,8 @@ static int ov9282_power_off(struct device *dev) > > > > clk_disable_unprepare(ov9282->inclk); > > > > + regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies); > > + > > return 0; > > } > > Despite the nits above > Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] media: i2c: ov9282: Add support for regulators. 2022-11-24 11:58 ` Dave Stevenson @ 2022-11-24 12:06 ` Alexander Stein 0 siblings, 0 replies; 9+ messages in thread From: Alexander Stein @ 2022-11-24 12:06 UTC (permalink / raw) To: Dave Stevenson Cc: paul.j.murphy, daniele.alessandrelli, linux-media, robh+dt, devicetree Hi Dave, Am Donnerstag, 24. November 2022, 12:58:08 CET schrieb Dave Stevenson: > Hi Alexander > > Thanks for the review. > > Sakari has already picked this up and included it in a pull to Mauro for > 6.2. https://www.spinics.net/lists/linux-media/msg222346.html A quite recent, I wasn't aware of that. Thanks for the hint. > On Thu, 24 Nov 2022 at 09:31, Alexander Stein > > <alexander.stein@ew.tq-group.com> wrote: > > Hello Dave, > > > > Am Mittwoch, 5. Oktober 2022, 17:20:18 CET schrieb Dave Stevenson: > > > The sensor takes 3 supply rails - AVDD, DVDD, and DOVDD. > > > > > > Add hooks into the regulator framework for each of these > > > regulators. > > > > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > --- > > > > > > drivers/media/i2c/ov9282.c | 38 ++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 38 insertions(+) > > > > > > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c > > > index 2e0b315801e5..699fc5b753b4 100644 > > > --- a/drivers/media/i2c/ov9282.c > > > +++ b/drivers/media/i2c/ov9282.c > > > @@ -11,6 +11,7 @@ > > > > > > #include <linux/i2c.h> > > > #include <linux/module.h> > > > #include <linux/pm_runtime.h> > > > > > > +#include <linux/regulator/consumer.h> > > > > > > #include <media/v4l2-ctrls.h> > > > #include <media/v4l2-fwnode.h> > > > > > > @@ -55,6 +56,14 @@ > > > > > > #define OV9282_REG_MIN 0x00 > > > #define OV9282_REG_MAX 0xfffff > > > > > > +static const char * const ov9282_supply_names[] = { > > > + "avdd", /* Analog power */ > > > + "dovdd", /* Digital I/O power */ > > > + "dvdd", /* Digital core power */ > > > +}; > > > + > > > +#define OV9282_NUM_SUPPLIES ARRAY_SIZE(ov9282_supply_names) > > > + > > > > > > /** > > > > > > * struct ov9282_reg - ov9282 sensor register > > > * @address: Register address > > > > > > @@ -128,6 +137,7 @@ struct ov9282 { > > > > > > struct media_pad pad; > > > struct gpio_desc *reset_gpio; > > > struct clk *inclk; > > > > > > + struct regulator_bulk_data supplies[OV9282_NUM_SUPPLIES]; > > > > Please add documentation for supplies. > > Is it the place for the driver to document the supplies beyond the > comments in ov9282_supply_names with regard to which sensor rail they > relate to? > Some drivers include the typical values for each supply, but those are > technically inaccurate as each will have a min and max value. > > Anyone interfacing with a sensor is going to have the datasheet for it > and should be referring to that for the characteristics of supply > rails. Duplicating some of that in the driver seems redundant, and has > the potential to be incorrect. What I meant was adding " @supplies: power supply regulators" to the doxygen (?) documentation directly above. I agree that no details about those supplies should be added to driver code though. Alexander > > > > struct v4l2_ctrl_handler ctrl_handler; > > > struct v4l2_ctrl *link_freq_ctrl; > > > struct v4l2_ctrl *pclk_ctrl; > > > > > > @@ -767,6 +777,18 @@ static int ov9282_detect(struct ov9282 *ov9282) > > > > > > return 0; > > > > > > } > > > > > > +static int ov9282_configure_regulators(struct ov9282 *ov9282) > > > +{ > > > + unsigned int i; > > > + > > > + for (i = 0; i < OV9282_NUM_SUPPLIES; i++) > > > + ov9282->supplies[i].supply = ov9282_supply_names[i]; > > > + > > > + return devm_regulator_bulk_get(ov9282->dev, > > > + OV9282_NUM_SUPPLIES, > > > + ov9282->supplies); > > > +} > > > + > > > > > > /** > > > > > > * ov9282_parse_hw_config() - Parse HW configuration and check if > > > supported > > > > > > * @ov9282: pointer to ov9282 device > > > @@ -803,6 +825,12 @@ static int ov9282_parse_hw_config(struct ov9282 > > > *ov9282) return PTR_ERR(ov9282->inclk); > > > > > > } > > > > > > + ret = ov9282_configure_regulators(ov9282); > > > + if (ret) { > > > + dev_err(ov9282->dev, "Failed to get power regulators\n"); > > > > dev_err_probe seems sensible here. > > That would have been good - sorry. I must get into the habit of > remembering to use dev_err_probe. > > Dave > > > > + return ret; > > > + } > > > + > > > > > > rate = clk_get_rate(ov9282->inclk); > > > if (rate != OV9282_INCLK_RATE) { > > > > > > dev_err(ov9282->dev, "inclk frequency mismatch"); > > > > > > @@ -874,6 +902,12 @@ static int ov9282_power_on(struct device *dev) > > > > > > struct ov9282 *ov9282 = to_ov9282(sd); > > > int ret; > > > > > > + ret = regulator_bulk_enable(OV9282_NUM_SUPPLIES, > > > ov9282->supplies); > > > + if (ret < 0) { > > > + dev_err(dev, "Failed to enable regulators\n"); > > > + return ret; > > > + } > > > + > > > > > > usleep_range(400, 600); > > > > > > gpiod_set_value_cansleep(ov9282->reset_gpio, 1); > > > > > > @@ -891,6 +925,8 @@ static int ov9282_power_on(struct device *dev) > > > > > > error_reset: > > > gpiod_set_value_cansleep(ov9282->reset_gpio, 0); > > > > > > + regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies); > > > + > > > > > > return ret; > > > > > > } > > > > > > @@ -909,6 +945,8 @@ static int ov9282_power_off(struct device *dev) > > > > > > clk_disable_unprepare(ov9282->inclk); > > > > > > + regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies); > > > + > > > > > > return 0; > > > > > > } > > > > Despite the nits above > > Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] media: Add regulator support to ov9282 2022-10-05 15:20 [PATCH 0/2] media: Add regulator support to ov9282 Dave Stevenson 2022-10-05 15:20 ` [PATCH 1/2] dt-bindings: media: ovti,ov9282: Add optional regulators Dave Stevenson 2022-10-05 15:20 ` [PATCH 2/2] media: i2c: ov9282: Add support for regulators Dave Stevenson @ 2022-11-16 16:50 ` Dave Stevenson 2022-11-17 9:40 ` Sakari Ailus 2 siblings, 1 reply; 9+ messages in thread From: Dave Stevenson @ 2022-11-16 16:50 UTC (permalink / raw) To: paul.j.murphy, daniele.alessandrelli, linux-media; +Cc: Sakari Ailus On Wed, 5 Oct 2022 at 16:21, Dave Stevenson <dave.stevenson@raspberrypi.com> wrote: > > The sensor takes 3 voltage supply rails, so add the relevant > configuration to the device tree bindings and driver. A gentle ping to this patchset. The other patchset for ov9282 that added functionality has been merged and made a pull request already, and Patchwork was reporting this one as "Under review" for longer than that one was. The DT side is ack'ed, and the driver changes are pretty trivial. Cheers, Dave > Dave Stevenson (2): > dt-bindings: media: ovti,ov9282: Add optional regulators > media: i2c: ov9282: Add support for regulators. > > .../bindings/media/i2c/ovti,ov9282.yaml | 9 +++++ > drivers/media/i2c/ov9282.c | 38 +++++++++++++++++++ > 2 files changed, 47 insertions(+) > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] media: Add regulator support to ov9282 2022-11-16 16:50 ` [PATCH 0/2] media: Add regulator support to ov9282 Dave Stevenson @ 2022-11-17 9:40 ` Sakari Ailus 0 siblings, 0 replies; 9+ messages in thread From: Sakari Ailus @ 2022-11-17 9:40 UTC (permalink / raw) To: Dave Stevenson; +Cc: paul.j.murphy, daniele.alessandrelli, linux-media Hi Dave, On Wed, Nov 16, 2022 at 04:50:27PM +0000, Dave Stevenson wrote: > On Wed, 5 Oct 2022 at 16:21, Dave Stevenson > <dave.stevenson@raspberrypi.com> wrote: > > > > The sensor takes 3 voltage supply rails, so add the relevant > > configuration to the device tree bindings and driver. > > A gentle ping to this patchset. > The other patchset for ov9282 that added functionality has been merged > and made a pull request already, and Patchwork was reporting this one > as "Under review" for longer than that one was. > The DT side is ack'ed, and the driver changes are pretty trivial. Oops. Thanks for the ping. These are in my tree now. -- Kind regards, Sakari Ailus ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-11-24 12:07 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-05 15:20 [PATCH 0/2] media: Add regulator support to ov9282 Dave Stevenson 2022-10-05 15:20 ` [PATCH 1/2] dt-bindings: media: ovti,ov9282: Add optional regulators Dave Stevenson 2022-10-06 19:08 ` Rob Herring 2022-10-05 15:20 ` [PATCH 2/2] media: i2c: ov9282: Add support for regulators Dave Stevenson 2022-11-24 9:31 ` Alexander Stein 2022-11-24 11:58 ` Dave Stevenson 2022-11-24 12:06 ` Alexander Stein 2022-11-16 16:50 ` [PATCH 0/2] media: Add regulator support to ov9282 Dave Stevenson 2022-11-17 9:40 ` Sakari Ailus
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).