* [PATCH v5 1/4] media: i2c: imx412: Fix reset GPIO polarity
2022-04-15 11:59 [PATCH v5 0/4] media: i2c: imx412: Add regulator control to imx412 Bryan O'Donoghue
@ 2022-04-15 11:59 ` Bryan O'Donoghue
2022-04-25 14:41 ` Alessandrelli, Daniele
2022-04-15 11:59 ` [PATCH v5 2/4] media: i2c: imx412: Fix power_off ordering Bryan O'Donoghue
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Bryan O'Donoghue @ 2022-04-15 11:59 UTC (permalink / raw)
To: sakari.ailus, jacopo, paul.j.murphy, daniele.alessandrelli,
mchehab, linux-media, devicetree
Cc: robert.foss, hfink, jgrahsl, dmitry.baryshkov,
vladimir.zapolskiy, bryan.odonoghue, stable
The imx412/imx577 sensor has a reset line that is active low not active
high. Currently the logic for this is inverted.
The right way to define the reset line is to declare it active low in the
DTS and invert the logic currently contained in the driver.
The DTS should represent the hardware does i.e. reset is active low.
So:
+ reset-gpios = <&tlmm 78 GPIO_ACTIVE_LOW>;
not:
- reset-gpios = <&tlmm 78 GPIO_ACTIVE_HIGH>;
I was a bit reticent about changing this logic since I thought it might
negatively impact @intel.com users. Googling a bit though I believe this
sensor is used on "Keem Bay" which is clearly a DTS based system and is not
upstream yet.
Fixes: 9214e86c0cc1 ("media: i2c: Add imx412 camera sensor driver")
Cc: stable@vger.kernel.org
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/media/i2c/imx412.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
index be3f6ea55559..e6be6b4250f5 100644
--- a/drivers/media/i2c/imx412.c
+++ b/drivers/media/i2c/imx412.c
@@ -1011,7 +1011,7 @@ static int imx412_power_on(struct device *dev)
struct imx412 *imx412 = to_imx412(sd);
int ret;
- gpiod_set_value_cansleep(imx412->reset_gpio, 1);
+ gpiod_set_value_cansleep(imx412->reset_gpio, 0);
ret = clk_prepare_enable(imx412->inclk);
if (ret) {
@@ -1024,7 +1024,7 @@ static int imx412_power_on(struct device *dev)
return 0;
error_reset:
- gpiod_set_value_cansleep(imx412->reset_gpio, 0);
+ gpiod_set_value_cansleep(imx412->reset_gpio, 1);
return ret;
}
@@ -1040,7 +1040,7 @@ static int imx412_power_off(struct device *dev)
struct v4l2_subdev *sd = dev_get_drvdata(dev);
struct imx412 *imx412 = to_imx412(sd);
- gpiod_set_value_cansleep(imx412->reset_gpio, 0);
+ gpiod_set_value_cansleep(imx412->reset_gpio, 1);
clk_disable_unprepare(imx412->inclk);
--
2.35.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v5 1/4] media: i2c: imx412: Fix reset GPIO polarity
2022-04-15 11:59 ` [PATCH v5 1/4] media: i2c: imx412: Fix reset GPIO polarity Bryan O'Donoghue
@ 2022-04-25 14:41 ` Alessandrelli, Daniele
0 siblings, 0 replies; 10+ messages in thread
From: Alessandrelli, Daniele @ 2022-04-25 14:41 UTC (permalink / raw)
To: Murphy, Paul J, jacopo, mchehab, bryan.odonoghue, linux-media,
devicetree, sakari.ailus
Cc: hfink, robert.foss, vladimir.zapolskiy, dmitry.baryshkov, stable,
jgrahsl
Hi Bryan,
On Fri, 2022-04-15 at 12:59 +0100, Bryan O'Donoghue wrote:
> The imx412/imx577 sensor has a reset line that is active low not active
> high. Currently the logic for this is inverted.
>
> The right way to define the reset line is to declare it active low in the
> DTS and invert the logic currently contained in the driver.
>
> The DTS should represent the hardware does i.e. reset is active low.
> So:
> + reset-gpios = <&tlmm 78 GPIO_ACTIVE_LOW>;
> not:
> - reset-gpios = <&tlmm 78 GPIO_ACTIVE_HIGH>;
>
> I was a bit reticent about changing this logic since I thought it might
> negatively impact @intel.com users. Googling a bit though I believe this
> sensor is used on "Keem Bay" which is clearly a DTS based system and is not
> upstream yet.
Thanks for the fix and sorry for the late reply (I've been off for the
last 2 weeks).
The fix looks good to me and it will not negatively affect us (I
appreciate the consideration!).
Reviewed-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
>
> Fixes: 9214e86c0cc1 ("media: i2c: Add imx412 camera sensor driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> drivers/media/i2c/imx412.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
> index be3f6ea55559..e6be6b4250f5 100644
> --- a/drivers/media/i2c/imx412.c
> +++ b/drivers/media/i2c/imx412.c
> @@ -1011,7 +1011,7 @@ static int imx412_power_on(struct device *dev)
> struct imx412 *imx412 = to_imx412(sd);
> int ret;
>
> - gpiod_set_value_cansleep(imx412->reset_gpio, 1);
> + gpiod_set_value_cansleep(imx412->reset_gpio, 0);
>
> ret = clk_prepare_enable(imx412->inclk);
> if (ret) {
> @@ -1024,7 +1024,7 @@ static int imx412_power_on(struct device *dev)
> return 0;
>
> error_reset:
> - gpiod_set_value_cansleep(imx412->reset_gpio, 0);
> + gpiod_set_value_cansleep(imx412->reset_gpio, 1);
>
> return ret;
> }
> @@ -1040,7 +1040,7 @@ static int imx412_power_off(struct device *dev)
> struct v4l2_subdev *sd = dev_get_drvdata(dev);
> struct imx412 *imx412 = to_imx412(sd);
>
> - gpiod_set_value_cansleep(imx412->reset_gpio, 0);
> + gpiod_set_value_cansleep(imx412->reset_gpio, 1);
>
> clk_disable_unprepare(imx412->inclk);
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 2/4] media: i2c: imx412: Fix power_off ordering
2022-04-15 11:59 [PATCH v5 0/4] media: i2c: imx412: Add regulator control to imx412 Bryan O'Donoghue
2022-04-15 11:59 ` [PATCH v5 1/4] media: i2c: imx412: Fix reset GPIO polarity Bryan O'Donoghue
@ 2022-04-15 11:59 ` Bryan O'Donoghue
2022-04-25 14:43 ` Alessandrelli, Daniele
2022-04-15 11:59 ` [PATCH v5 3/4] media: dt-bindings: imx412: Add regulator descriptions Bryan O'Donoghue
2022-04-15 11:59 ` [PATCH v5 4/4] media: i2c: imx412: Add bulk regulator support Bryan O'Donoghue
3 siblings, 1 reply; 10+ messages in thread
From: Bryan O'Donoghue @ 2022-04-15 11:59 UTC (permalink / raw)
To: sakari.ailus, jacopo, paul.j.murphy, daniele.alessandrelli,
mchehab, linux-media, devicetree
Cc: robert.foss, hfink, jgrahsl, dmitry.baryshkov,
vladimir.zapolskiy, bryan.odonoghue, stable
The enable path does
- gpio
- clock
The disable path does
- gpio
- clock
Fix the order on the power-off path so that power-off and power-on have the
same ordering for clock and gpio.
Fixes: 9214e86c0cc1 ("media: i2c: Add imx412 camera sensor driver")
Cc: stable@vger.kernel.org
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/media/i2c/imx412.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
index e6be6b4250f5..84279a680873 100644
--- a/drivers/media/i2c/imx412.c
+++ b/drivers/media/i2c/imx412.c
@@ -1040,10 +1040,10 @@ static int imx412_power_off(struct device *dev)
struct v4l2_subdev *sd = dev_get_drvdata(dev);
struct imx412 *imx412 = to_imx412(sd);
- gpiod_set_value_cansleep(imx412->reset_gpio, 1);
-
clk_disable_unprepare(imx412->inclk);
+ gpiod_set_value_cansleep(imx412->reset_gpio, 1);
+
return 0;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v5 2/4] media: i2c: imx412: Fix power_off ordering
2022-04-15 11:59 ` [PATCH v5 2/4] media: i2c: imx412: Fix power_off ordering Bryan O'Donoghue
@ 2022-04-25 14:43 ` Alessandrelli, Daniele
0 siblings, 0 replies; 10+ messages in thread
From: Alessandrelli, Daniele @ 2022-04-25 14:43 UTC (permalink / raw)
To: Murphy, Paul J, jacopo, mchehab, bryan.odonoghue, linux-media,
devicetree, sakari.ailus
Cc: hfink, robert.foss, vladimir.zapolskiy, dmitry.baryshkov, stable,
jgrahsl
On Fri, 2022-04-15 at 12:59 +0100, Bryan O'Donoghue wrote:
> The enable path does
> - gpio
> - clock
>
> The disable path does
> - gpio
> - clock
>
> Fix the order on the power-off path so that power-off and power-on have
> the
> same ordering for clock and gpio.
>
> Fixes: 9214e86c0cc1 ("media: i2c: Add imx412 camera sensor driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> drivers/media/i2c/imx412.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
> index e6be6b4250f5..84279a680873 100644
> --- a/drivers/media/i2c/imx412.c
> +++ b/drivers/media/i2c/imx412.c
> @@ -1040,10 +1040,10 @@ static int imx412_power_off(struct device *dev)
> struct v4l2_subdev *sd = dev_get_drvdata(dev);
> struct imx412 *imx412 = to_imx412(sd);
>
> - gpiod_set_value_cansleep(imx412->reset_gpio, 1);
> -
> clk_disable_unprepare(imx412->inclk);
>
> + gpiod_set_value_cansleep(imx412->reset_gpio, 1);
> +
> return 0;
> }
>
LGTM, thanks for the patch!
Reviewed-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 3/4] media: dt-bindings: imx412: Add regulator descriptions
2022-04-15 11:59 [PATCH v5 0/4] media: i2c: imx412: Add regulator control to imx412 Bryan O'Donoghue
2022-04-15 11:59 ` [PATCH v5 1/4] media: i2c: imx412: Fix reset GPIO polarity Bryan O'Donoghue
2022-04-15 11:59 ` [PATCH v5 2/4] media: i2c: imx412: Fix power_off ordering Bryan O'Donoghue
@ 2022-04-15 11:59 ` Bryan O'Donoghue
2022-04-15 11:59 ` [PATCH v5 4/4] media: i2c: imx412: Add bulk regulator support Bryan O'Donoghue
3 siblings, 0 replies; 10+ messages in thread
From: Bryan O'Donoghue @ 2022-04-15 11:59 UTC (permalink / raw)
To: sakari.ailus, jacopo, paul.j.murphy, daniele.alessandrelli,
mchehab, linux-media, devicetree
Cc: robert.foss, hfink, jgrahsl, dmitry.baryshkov,
vladimir.zapolskiy, bryan.odonoghue, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski
The imx412 like many I2C camera sensors has three voltage rails which
depending on platform may be necessary to switch power onto directly.
Add in as optional rails so as not to break anything for existing users.
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
.../devicetree/bindings/media/i2c/sony,imx412.yaml | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml
index afcf70947f7e..26d1807d0bb6 100644
--- a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml
@@ -32,6 +32,15 @@ properties:
description: Clock frequency 6MHz, 12MHz, 18MHz, 24MHz or 27MHz
maxItems: 1
+ dovdd-supply:
+ description: Interface power supply.
+
+ avdd-supply:
+ description: Analog power supply.
+
+ dvdd-supply:
+ description: Digital power supply.
+
reset-gpios:
description: Reference to the GPIO connected to the XCLR pin, if any.
maxItems: 1
--
2.35.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 4/4] media: i2c: imx412: Add bulk regulator support
2022-04-15 11:59 [PATCH v5 0/4] media: i2c: imx412: Add regulator control to imx412 Bryan O'Donoghue
` (2 preceding siblings ...)
2022-04-15 11:59 ` [PATCH v5 3/4] media: dt-bindings: imx412: Add regulator descriptions Bryan O'Donoghue
@ 2022-04-15 11:59 ` Bryan O'Donoghue
2022-04-15 15:12 ` Jacopo Mondi
` (2 more replies)
3 siblings, 3 replies; 10+ messages in thread
From: Bryan O'Donoghue @ 2022-04-15 11:59 UTC (permalink / raw)
To: sakari.ailus, jacopo, paul.j.murphy, daniele.alessandrelli,
mchehab, linux-media, devicetree
Cc: robert.foss, hfink, jgrahsl, dmitry.baryshkov,
vladimir.zapolskiy, bryan.odonoghue
Depending on the platform we may need to enable and disable three separate
regulators for the imx412.
- DOVDD
Digital I/O power
- AVDD
Analog power
- DVDD
Digital core power
The addition of these regulators shouldn't affect existing users using
fixed-on/firmware-controlled regulators.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/media/i2c/imx412.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
index 84279a680873..1795a6180d60 100644
--- a/drivers/media/i2c/imx412.c
+++ b/drivers/media/i2c/imx412.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>
@@ -101,6 +102,12 @@ struct imx412_mode {
struct imx412_reg_list reg_list;
};
+static const char * const imx412_supply_names[] = {
+ "dovdd", /* Digital I/O power */
+ "avdd", /* Analog power */
+ "dvdd", /* Digital core power */
+};
+
/**
* struct imx412 - imx412 sensor device structure
* @dev: Pointer to generic device
@@ -128,6 +135,7 @@ struct imx412 {
struct media_pad pad;
struct gpio_desc *reset_gpio;
struct clk *inclk;
+ struct regulator_bulk_data supplies[ARRAY_SIZE(imx412_supply_names)];
struct v4l2_ctrl_handler ctrl_handler;
struct v4l2_ctrl *link_freq_ctrl;
struct v4l2_ctrl *pclk_ctrl;
@@ -946,6 +954,16 @@ static int imx412_parse_hw_config(struct imx412 *imx412)
return -EINVAL;
}
+ /* Get optional DT defined regulators */
+ for (i = 0; i < ARRAY_SIZE(imx412_supply_names); i++)
+ imx412->supplies[i].supply = imx412_supply_names[i];
+
+ ret = devm_regulator_bulk_get(imx412->dev,
+ ARRAY_SIZE(imx412_supply_names),
+ imx412->supplies);
+ if (ret)
+ return ret;
+
ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
if (!ep)
return -ENXIO;
@@ -1011,6 +1029,13 @@ static int imx412_power_on(struct device *dev)
struct imx412 *imx412 = to_imx412(sd);
int ret;
+ ret = regulator_bulk_enable(ARRAY_SIZE(imx412_supply_names),
+ imx412->supplies);
+ if (ret < 0) {
+ dev_err(dev, "failed to enable regulators\n");
+ return ret;
+ }
+
gpiod_set_value_cansleep(imx412->reset_gpio, 0);
ret = clk_prepare_enable(imx412->inclk);
@@ -1025,6 +1050,8 @@ static int imx412_power_on(struct device *dev)
error_reset:
gpiod_set_value_cansleep(imx412->reset_gpio, 1);
+ regulator_bulk_disable(ARRAY_SIZE(imx412_supply_names),
+ imx412->supplies);
return ret;
}
@@ -1044,6 +1071,9 @@ static int imx412_power_off(struct device *dev)
gpiod_set_value_cansleep(imx412->reset_gpio, 1);
+ regulator_bulk_disable(ARRAY_SIZE(imx412_supply_names),
+ imx412->supplies);
+
return 0;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v5 4/4] media: i2c: imx412: Add bulk regulator support
2022-04-15 11:59 ` [PATCH v5 4/4] media: i2c: imx412: Add bulk regulator support Bryan O'Donoghue
@ 2022-04-15 15:12 ` Jacopo Mondi
2022-04-26 16:52 ` Alessandrelli, Daniele
2022-05-06 14:26 ` Sakari Ailus
2 siblings, 0 replies; 10+ messages in thread
From: Jacopo Mondi @ 2022-04-15 15:12 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: sakari.ailus, paul.j.murphy, daniele.alessandrelli, mchehab,
linux-media, devicetree, robert.foss, hfink, jgrahsl,
dmitry.baryshkov, vladimir.zapolskiy
Hello Bryan
On Fri, Apr 15, 2022 at 12:59:54PM +0100, Bryan O'Donoghue wrote:
> Depending on the platform we may need to enable and disable three separate
> regulators for the imx412.
>
> - DOVDD
> Digital I/O power
>
> - AVDD
> Analog power
>
> - DVDD
> Digital core power
>
> The addition of these regulators shouldn't affect existing users using
> fixed-on/firmware-controlled regulators.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Thanks
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Cheers
j
> ---
> drivers/media/i2c/imx412.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
> index 84279a680873..1795a6180d60 100644
> --- a/drivers/media/i2c/imx412.c
> +++ b/drivers/media/i2c/imx412.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>
> @@ -101,6 +102,12 @@ struct imx412_mode {
> struct imx412_reg_list reg_list;
> };
>
> +static const char * const imx412_supply_names[] = {
> + "dovdd", /* Digital I/O power */
> + "avdd", /* Analog power */
> + "dvdd", /* Digital core power */
> +};
> +
> /**
> * struct imx412 - imx412 sensor device structure
> * @dev: Pointer to generic device
> @@ -128,6 +135,7 @@ struct imx412 {
> struct media_pad pad;
> struct gpio_desc *reset_gpio;
> struct clk *inclk;
> + struct regulator_bulk_data supplies[ARRAY_SIZE(imx412_supply_names)];
> struct v4l2_ctrl_handler ctrl_handler;
> struct v4l2_ctrl *link_freq_ctrl;
> struct v4l2_ctrl *pclk_ctrl;
> @@ -946,6 +954,16 @@ static int imx412_parse_hw_config(struct imx412 *imx412)
> return -EINVAL;
> }
>
> + /* Get optional DT defined regulators */
> + for (i = 0; i < ARRAY_SIZE(imx412_supply_names); i++)
> + imx412->supplies[i].supply = imx412_supply_names[i];
> +
> + ret = devm_regulator_bulk_get(imx412->dev,
> + ARRAY_SIZE(imx412_supply_names),
> + imx412->supplies);
> + if (ret)
> + return ret;
> +
> ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
> if (!ep)
> return -ENXIO;
> @@ -1011,6 +1029,13 @@ static int imx412_power_on(struct device *dev)
> struct imx412 *imx412 = to_imx412(sd);
> int ret;
>
> + ret = regulator_bulk_enable(ARRAY_SIZE(imx412_supply_names),
> + imx412->supplies);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable regulators\n");
> + return ret;
> + }
> +
> gpiod_set_value_cansleep(imx412->reset_gpio, 0);
>
> ret = clk_prepare_enable(imx412->inclk);
> @@ -1025,6 +1050,8 @@ static int imx412_power_on(struct device *dev)
>
> error_reset:
> gpiod_set_value_cansleep(imx412->reset_gpio, 1);
> + regulator_bulk_disable(ARRAY_SIZE(imx412_supply_names),
> + imx412->supplies);
>
> return ret;
> }
> @@ -1044,6 +1071,9 @@ static int imx412_power_off(struct device *dev)
>
> gpiod_set_value_cansleep(imx412->reset_gpio, 1);
>
> + regulator_bulk_disable(ARRAY_SIZE(imx412_supply_names),
> + imx412->supplies);
> +
> return 0;
> }
>
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 4/4] media: i2c: imx412: Add bulk regulator support
2022-04-15 11:59 ` [PATCH v5 4/4] media: i2c: imx412: Add bulk regulator support Bryan O'Donoghue
2022-04-15 15:12 ` Jacopo Mondi
@ 2022-04-26 16:52 ` Alessandrelli, Daniele
2022-05-06 14:26 ` Sakari Ailus
2 siblings, 0 replies; 10+ messages in thread
From: Alessandrelli, Daniele @ 2022-04-26 16:52 UTC (permalink / raw)
To: Murphy, Paul J, jacopo, mchehab, bryan.odonoghue, linux-media,
devicetree, sakari.ailus
Cc: hfink, robert.foss, vladimir.zapolskiy, dmitry.baryshkov, jgrahsl
On Fri, 2022-04-15 at 12:59 +0100, Bryan O'Donoghue wrote:
> Depending on the platform we may need to enable and disable three separate
> regulators for the imx412.
>
> - DOVDD
> Digital I/O power
>
> - AVDD
> Analog power
>
> - DVDD
> Digital core power
>
> The addition of these regulators shouldn't affect existing users using
> fixed-on/firmware-controlled regulators.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> ---
> drivers/media/i2c/imx412.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
> index 84279a680873..1795a6180d60 100644
> --- a/drivers/media/i2c/imx412.c
> +++ b/drivers/media/i2c/imx412.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>
> @@ -101,6 +102,12 @@ struct imx412_mode {
> struct imx412_reg_list reg_list;
> };
>
> +static const char * const imx412_supply_names[] = {
> + "dovdd", /* Digital I/O power */
> + "avdd", /* Analog power */
> + "dvdd", /* Digital core power */
> +};
> +
> /**
> * struct imx412 - imx412 sensor device structure
> * @dev: Pointer to generic device
> @@ -128,6 +135,7 @@ struct imx412 {
> struct media_pad pad;
> struct gpio_desc *reset_gpio;
> struct clk *inclk;
> + struct regulator_bulk_data supplies[ARRAY_SIZE(imx412_supply_names)];
> struct v4l2_ctrl_handler ctrl_handler;
> struct v4l2_ctrl *link_freq_ctrl;
> struct v4l2_ctrl *pclk_ctrl;
> @@ -946,6 +954,16 @@ static int imx412_parse_hw_config(struct imx412 *imx412)
> return -EINVAL;
> }
>
> + /* Get optional DT defined regulators */
> + for (i = 0; i < ARRAY_SIZE(imx412_supply_names); i++)
> + imx412->supplies[i].supply = imx412_supply_names[i];
> +
> + ret = devm_regulator_bulk_get(imx412->dev,
> + ARRAY_SIZE(imx412_supply_names),
> + imx412->supplies);
> + if (ret)
> + return ret;
> +
> ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
> if (!ep)
> return -ENXIO;
> @@ -1011,6 +1029,13 @@ static int imx412_power_on(struct device *dev)
> struct imx412 *imx412 = to_imx412(sd);
> int ret;
>
> + ret = regulator_bulk_enable(ARRAY_SIZE(imx412_supply_names),
> + imx412->supplies);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable regulators\n");
> + return ret;
> + }
> +
> gpiod_set_value_cansleep(imx412->reset_gpio, 0);
>
> ret = clk_prepare_enable(imx412->inclk);
> @@ -1025,6 +1050,8 @@ static int imx412_power_on(struct device *dev)
>
> error_reset:
> gpiod_set_value_cansleep(imx412->reset_gpio, 1);
> + regulator_bulk_disable(ARRAY_SIZE(imx412_supply_names),
> + imx412->supplies);
>
> return ret;
> }
> @@ -1044,6 +1071,9 @@ static int imx412_power_off(struct device *dev)
>
> gpiod_set_value_cansleep(imx412->reset_gpio, 1);
>
> + regulator_bulk_disable(ARRAY_SIZE(imx412_supply_names),
> + imx412->supplies);
> +
> return 0;
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 4/4] media: i2c: imx412: Add bulk regulator support
2022-04-15 11:59 ` [PATCH v5 4/4] media: i2c: imx412: Add bulk regulator support Bryan O'Donoghue
2022-04-15 15:12 ` Jacopo Mondi
2022-04-26 16:52 ` Alessandrelli, Daniele
@ 2022-05-06 14:26 ` Sakari Ailus
2 siblings, 0 replies; 10+ messages in thread
From: Sakari Ailus @ 2022-05-06 14:26 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: jacopo, paul.j.murphy, daniele.alessandrelli, mchehab,
linux-media, devicetree, robert.foss, hfink, jgrahsl,
dmitry.baryshkov, vladimir.zapolskiy
On Fri, Apr 15, 2022 at 12:59:54PM +0100, Bryan O'Donoghue wrote:
> Depending on the platform we may need to enable and disable three separate
> regulators for the imx412.
>
> - DOVDD
> Digital I/O power
>
> - AVDD
> Analog power
>
> - DVDD
> Digital core power
>
> The addition of these regulators shouldn't affect existing users using
> fixed-on/firmware-controlled regulators.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Squashed this bit into the patch:
diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
index 1795a6180d60d..a1394d6c14320 100644
--- a/drivers/media/i2c/imx412.c
+++ b/drivers/media/i2c/imx412.c
@@ -116,6 +116,7 @@ static const char * const imx412_supply_names[] = {
* @pad: Media pad. Only one pad supported
* @reset_gpio: Sensor reset gpio
* @inclk: Sensor input clock
+ * @supplies: Regulator supplies
* @ctrl_handler: V4L2 control handler
* @link_freq_ctrl: Pointer to link frequency control
* @pclk_ctrl: Pointer to pixel clock control
--
Sakari Ailus
^ permalink raw reply related [flat|nested] 10+ messages in thread