| Message ID | 67b53f91ede9e9ffdda913c818065095a726b92e.1559157595.git.mchehab+samsung@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] media: mt9m111: add regulator support | expand |
2019年5月30日(木) 4:25 Mauro Carvalho Chehab <mchehab+samsung@kernel.org>: > > From: Robert Jarzmik <robert.jarzmik@free.fr> > > In the soc_camera removal, the board specific power callback was > dropped. This at least will remove the power optimization from ezx and > em-x270 pxa based boards. > > As to recreate the same level of functionality, make the mt9m111 have a > regulator providing it its power, so that board designers can plug in a > gpio based or ldo regulator, mimicking their former soc_camera power > hook. > > Fixes: 5c10113cc668 ("media: mt9m111: make a standalone v4l2 subdevice") > > [mchehab+samsung@kernel.org: check return values for regulator_enable and > fix a build warning] > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> > --- > > This is a respin of this patch: > > http://patchwork.linuxtv.org/patch/37950/ > > rebased (and fixed) to apply on the top of upstream. > > While checking old patches at the ML, I noticed that this patch > was never applied: > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1238720.html > > The first patch of this series got applied, though: > > c771f42fed7f ("[media] media: platform: pxa_camera: add missing sensor power on") > > So, I'm closing the original patch as obsoleted and I'm sending this > one to the ML for tests. > > Can anyone test this patch and send a tested-by? In my devicetree, vdd-supply is not defined. So it falls back to the dummy regulator and works fine. Tested-by: Akinobu Mita <akinobu.mita@gmail.com>
Hi Mauro, On Wed, May 29, 2019 at 03:25:18PM -0400, Mauro Carvalho Chehab wrote: > From: Robert Jarzmik <robert.jarzmik@free.fr> > > In the soc_camera removal, the board specific power callback was > dropped. This at least will remove the power optimization from ezx and > em-x270 pxa based boards. > > As to recreate the same level of functionality, make the mt9m111 have a > regulator providing it its power, so that board designers can plug in a > gpio based or ldo regulator, mimicking their former soc_camera power > hook. > > Fixes: 5c10113cc668 ("media: mt9m111: make a standalone v4l2 subdevice") > > [mchehab+samsung@kernel.org: check return values for regulator_enable and > fix a build warning] > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> > --- > > This is a respin of this patch: > > http://patchwork.linuxtv.org/patch/37950/ > > rebased (and fixed) to apply on the top of upstream. > > While checking old patches at the ML, I noticed that this patch > was never applied: > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1238720.html > > The first patch of this series got applied, though: > > c771f42fed7f ("[media] media: platform: pxa_camera: add missing sensor power on") > > So, I'm closing the original patch as obsoleted and I'm sending this > one to the ML for tests. > > Can anyone test this patch and send a tested-by? > > drivers/media/i2c/mt9m111.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c > index 5168bb5880c4..746d1345b505 100644 > --- a/drivers/media/i2c/mt9m111.c > +++ b/drivers/media/i2c/mt9m111.c > @@ -13,6 +13,7 @@ > #include <linux/log2.h> > #include <linux/gpio.h> > #include <linux/delay.h> > +#include <linux/regulator/consumer.h> > #include <linux/v4l2-mediabus.h> > #include <linux/module.h> > #include <linux/property.h> > @@ -243,6 +244,7 @@ struct mt9m111 { > int power_count; > const struct mt9m111_datafmt *fmt; > int lastpage; /* PageMap cache value */ > + struct regulator *regulator; > bool is_streaming; > /* user point of view - 0: falling 1: rising edge */ > unsigned int pclk_sample:1; > @@ -982,6 +984,12 @@ static int mt9m111_power_on(struct mt9m111 *mt9m111) > if (ret < 0) > return ret; > > + if (mt9m111->regulator) { > + ret = regulator_enable(mt9m111->regulator); > + if (ret < 0) > + return ret; > + } > + > ret = mt9m111_resume(mt9m111); > if (ret < 0) { > dev_err(&client->dev, "Failed to resume the sensor: %d\n", ret); > @@ -994,6 +1002,8 @@ static int mt9m111_power_on(struct mt9m111 *mt9m111) > static void mt9m111_power_off(struct mt9m111 *mt9m111) > { > mt9m111_suspend(mt9m111); > + if (mt9m111->regulator) You could omit this check, same for the above. As Mita-san explained, it falls back to using the dummy regulator if there isn't one defined. > + regulator_disable(mt9m111->regulator); > v4l2_clk_disable(mt9m111->clk); > } > > @@ -1256,6 +1266,13 @@ static int mt9m111_probe(struct i2c_client *client, > if (IS_ERR(mt9m111->clk)) > return PTR_ERR(mt9m111->clk); > > + mt9m111->regulator = devm_regulator_get(&client->dev, "vdd"); > + if (IS_ERR(mt9m111->regulator)) { > + dev_err(&client->dev, "regulator not found: %ld\n", > + PTR_ERR(mt9m111->regulator)); > + return PTR_ERR(mt9m111->regulator); > + } > + > /* Default HIGHPOWER context */ > mt9m111->ctx = &context_b; >
Hi Sakari, Em Fri, 31 May 2019 14:27:24 +0300 Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > Hi Mauro, > > On Wed, May 29, 2019 at 03:25:18PM -0400, Mauro Carvalho Chehab wrote: > > From: Robert Jarzmik <robert.jarzmik@free.fr> As stated here, this patch is not really mine. It is a rebased version of a patch that was delegated to a sub-maintainer, being on his queue for more than 2 years. > > > > In the soc_camera removal, the board specific power callback was > > dropped. This at least will remove the power optimization from ezx and > > em-x270 pxa based boards. > > > > As to recreate the same level of functionality, make the mt9m111 have a > > regulator providing it its power, so that board designers can plug in a > > gpio based or ldo regulator, mimicking their former soc_camera power > > hook. > > > > Fixes: 5c10113cc668 ("media: mt9m111: make a standalone v4l2 subdevice") > > > > [mchehab+samsung@kernel.org: check return values for regulator_enable and > > fix a build warning] > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> > > --- > > > > This is a respin of this patch: > > > > http://patchwork.linuxtv.org/patch/37950/ > > > > rebased (and fixed) to apply on the top of upstream. > > > > While checking old patches at the ML, I noticed that this patch > > was never applied: > > > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1238720.html > > > > The first patch of this series got applied, though: > > > > c771f42fed7f ("[media] media: platform: pxa_camera: add missing sensor power on") > > > > So, I'm closing the original patch as obsoleted and I'm sending this > > one to the ML for tests. > > > > Can anyone test this patch and send a tested-by? > > > > drivers/media/i2c/mt9m111.c | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c > > index 5168bb5880c4..746d1345b505 100644 > > --- a/drivers/media/i2c/mt9m111.c > > +++ b/drivers/media/i2c/mt9m111.c > > @@ -13,6 +13,7 @@ > > #include <linux/log2.h> > > #include <linux/gpio.h> > > #include <linux/delay.h> > > +#include <linux/regulator/consumer.h> > > #include <linux/v4l2-mediabus.h> > > #include <linux/module.h> > > #include <linux/property.h> > > @@ -243,6 +244,7 @@ struct mt9m111 { > > int power_count; > > const struct mt9m111_datafmt *fmt; > > int lastpage; /* PageMap cache value */ > > + struct regulator *regulator; > > bool is_streaming; > > /* user point of view - 0: falling 1: rising edge */ > > unsigned int pclk_sample:1; > > @@ -982,6 +984,12 @@ static int mt9m111_power_on(struct mt9m111 *mt9m111) > > if (ret < 0) > > return ret; > > > > + if (mt9m111->regulator) { > > + ret = regulator_enable(mt9m111->regulator); > > + if (ret < 0) > > + return ret; > > + } > > + > > ret = mt9m111_resume(mt9m111); > > if (ret < 0) { > > dev_err(&client->dev, "Failed to resume the sensor: %d\n", ret); > > @@ -994,6 +1002,8 @@ static int mt9m111_power_on(struct mt9m111 *mt9m111) > > static void mt9m111_power_off(struct mt9m111 *mt9m111) > > { > > mt9m111_suspend(mt9m111); > > + if (mt9m111->regulator) > > You could omit this check, same for the above. As Mita-san explained, it > falls back to using the dummy regulator if there isn't one defined. > > > + regulator_disable(mt9m111->regulator); Makes sense to me. Feel free to remove it and apply on your tree. > > v4l2_clk_disable(mt9m111->clk); > > } > > > > @@ -1256,6 +1266,13 @@ static int mt9m111_probe(struct i2c_client *client, > > if (IS_ERR(mt9m111->clk)) > > return PTR_ERR(mt9m111->clk); > > > > + mt9m111->regulator = devm_regulator_get(&client->dev, "vdd"); > > + if (IS_ERR(mt9m111->regulator)) { > > + dev_err(&client->dev, "regulator not found: %ld\n", > > + PTR_ERR(mt9m111->regulator)); > > + return PTR_ERR(mt9m111->regulator); > > + } > > + > > /* Default HIGHPOWER context */ > > mt9m111->ctx = &context_b; > > > Thanks, Mauro
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> writes: > Hi Sakari, > > Em Fri, 31 May 2019 14:27:24 +0300 > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > >> Hi Mauro, >> >> On Wed, May 29, 2019 at 03:25:18PM -0400, Mauro Carvalho Chehab wrote: >> > From: Robert Jarzmik <robert.jarzmik@free.fr> > > As stated here, this patch is not really mine. It is a rebased version of a > patch that was delegated to a sub-maintainer, being on his queue for more > than 2 years. Hi Mauro, And if you need it, I can respin this patch for a v3, as I'm the original author AFAIR. And as soon as my brain recovers from my flu, I can also test it if need be. You can ask whatever you need, I will help. Cheers. -- Robert
Akinobu Mita <akinobu.mita@gmail.com> writes: >> Can anyone test this patch and send a tested-by? > > In my devicetree, vdd-supply is not defined. So it falls back to the dummy > regulator and works fine. Would that work also in a non devicetree build, ie. in a platform_data based one (as this is one of the mach-pxa targets) ? Cheers.
2019年6月1日(土) 4:43 Robert Jarzmik <robert.jarzmik@free.fr>: > > Akinobu Mita <akinobu.mita@gmail.com> writes: > > >> Can anyone test this patch and send a tested-by? > > > > In my devicetree, vdd-supply is not defined. So it falls back to the dummy > > regulator and works fine. > Would that work also in a non devicetree build, ie. in a platform_data based one > (as this is one of the mach-pxa targets) ? I don't have board_info based system, so I can't test without devicetree.
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> writes: > From: Robert Jarzmik <robert.jarzmik@free.fr> > > In the soc_camera removal, the board specific power callback was > dropped. This at least will remove the power optimization from ezx and > em-x270 pxa based boards. > > As to recreate the same level of functionality, make the mt9m111 have a > regulator providing it its power, so that board designers can plug in a > gpio based or ldo regulator, mimicking their former soc_camera power > hook. > > Fixes: 5c10113cc668 ("media: mt9m111: make a standalone v4l2 subdevice") > > [mchehab+samsung@kernel.org: check return values for regulator_enable and > fix a build warning] > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> Tested-by: Robert Jarzmik <robert.jarzmik@free.fr> (on mioa701, pxa architecture, platform data based, on top of 5.0-rc1) Cheers. -- Robert
On Fri, May 31, 2019 at 09:29:57PM +0200, Robert Jarzmik wrote: > Mauro Carvalho Chehab <mchehab+samsung@kernel.org> writes: > > > Hi Sakari, > > > > Em Fri, 31 May 2019 14:27:24 +0300 > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > >> Hi Mauro, > >> > >> On Wed, May 29, 2019 at 03:25:18PM -0400, Mauro Carvalho Chehab wrote: > >> > From: Robert Jarzmik <robert.jarzmik@free.fr> > > > > As stated here, this patch is not really mine. It is a rebased version of a > > patch that was delegated to a sub-maintainer, being on his queue for more > > than 2 years. > Hi Mauro, > > And if you need it, I can respin this patch for a v3, as I'm the original author > AFAIR. And as soon as my brain recovers from my flu, I can also test it if need > be. > > You can ask whatever you need, I will help. I wrote a few additional patches to address some of the issues. I'll post them separately.
diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c index 5168bb5880c4..746d1345b505 100644 --- a/drivers/media/i2c/mt9m111.c +++ b/drivers/media/i2c/mt9m111.c @@ -13,6 +13,7 @@ #include <linux/log2.h> #include <linux/gpio.h> #include <linux/delay.h> +#include <linux/regulator/consumer.h> #include <linux/v4l2-mediabus.h> #include <linux/module.h> #include <linux/property.h> @@ -243,6 +244,7 @@ struct mt9m111 { int power_count; const struct mt9m111_datafmt *fmt; int lastpage; /* PageMap cache value */ + struct regulator *regulator; bool is_streaming; /* user point of view - 0: falling 1: rising edge */ unsigned int pclk_sample:1; @@ -982,6 +984,12 @@ static int mt9m111_power_on(struct mt9m111 *mt9m111) if (ret < 0) return ret; + if (mt9m111->regulator) { + ret = regulator_enable(mt9m111->regulator); + if (ret < 0) + return ret; + } + ret = mt9m111_resume(mt9m111); if (ret < 0) { dev_err(&client->dev, "Failed to resume the sensor: %d\n", ret); @@ -994,6 +1002,8 @@ static int mt9m111_power_on(struct mt9m111 *mt9m111) static void mt9m111_power_off(struct mt9m111 *mt9m111) { mt9m111_suspend(mt9m111); + if (mt9m111->regulator) + regulator_disable(mt9m111->regulator); v4l2_clk_disable(mt9m111->clk); } @@ -1256,6 +1266,13 @@ static int mt9m111_probe(struct i2c_client *client, if (IS_ERR(mt9m111->clk)) return PTR_ERR(mt9m111->clk); + mt9m111->regulator = devm_regulator_get(&client->dev, "vdd"); + if (IS_ERR(mt9m111->regulator)) { + dev_err(&client->dev, "regulator not found: %ld\n", + PTR_ERR(mt9m111->regulator)); + return PTR_ERR(mt9m111->regulator); + } + /* Default HIGHPOWER context */ mt9m111->ctx = &context_b;