Re: [RFCv2 1/8] [media] si4713: switch to devm regulator API
Em Tue, 11 Nov 2014 18:59:31 +0100
Hans Verkuil escreveu:
> Hi Mauro,
>
> On 11/11/2014 12:07 PM, Mauro Carvalho Chehab wrote:
> > Em Tue, 21 Oct 2014 17:07:00 +0200
> > Sebastian Reichel escreveu:
> >
> >> This switches back to the normal regulator API (but use
> >> managed variant) in preparation for device tree support.
> >
> > This patch broke compilation. Please be sure that none of the patches in
> > the series would break it, as otherwise git bisect would be broken.
>
> Weird, as reported by Sebastian, it works for me.
Weird. Not sure what happened here.
>
> However, after applying this patch I get these new warnings:
>
> CC drivers/media/radio/si4713/radio-usb-si4713.o
> drivers/media/radio/si4713/si4713.c: In function ‘si4713_probe’:
> drivers/media/radio/si4713/si4713.c:1617:1: warning: label ‘free_gpio’
> defined but not used [-Wunused-label]
> free_gpio:
> ^
> drivers/media/radio/si4713/si4713.c:1451:12: warning: unused variable ‘i’
> [-Wunused-variable]
> int rval, i;
> ^
>
> So it's probably not a good idea to merge this patch anyway until this is
> fixed.
Agreed. Also, v3 of this series apparently came after the pull request.
Regards,
Mauro
> Sebastian, can you fix these warnings and repost?
>
> Thanks!
>
> Hans
>
> >
> > Thanks,
> > Mauro
> >
> > drivers/media/radio/si4713/si4713.c: In function 'si4713_powerup':
> > drivers/media/radio/si4713/si4713.c:369:10: error: 'struct si4713_device'
> > has no member named 'supplies'
> >
> > ^
> > drivers/media/radio/si4713/si4713.c:370:35: error: 'struct si4713_device'
> > has no member named 'supplies'
> > if (sdev->vdd) {
> >^
> > drivers/media/radio/si4713/si4713.c:370:51: error: 'struct si4713_device'
> > has no member named 'supply_data'
> > if (sdev->vdd) {
> >^
> > drivers/media/radio/si4713/si4713.c:402:10: error: 'struct si4713_device'
> > has no member named 'supplies'
> >v4l2_dbg(1, debug, &sdev->sd, "Device in power up mode\n");
> > ^
> > drivers/media/radio/si4713/si4713.c:403:36: error: 'struct si4713_device'
> > has no member named 'supplies'
> >sdev->power_state = POWER_ON;
> > ^
> > drivers/media/radio/si4713/si4713.c:403:52: error: 'struct si4713_device'
> > has no member named 'supply_data'
> >sdev->power_state = POWER_ON;
> > ^
> > drivers/media/radio/si4713/si4713.c: In function 'si4713_powerdown':
> > drivers/media/radio/si4713/si4713.c:435:11: error: 'struct si4713_device'
> > has no member named 'supplies'
> > int err;
> >^
> > drivers/media/radio/si4713/si4713.c:436:37: error: 'struct si4713_device'
> > has no member named 'supplies'
> > u8 resp[SI4713_PWDN_NRESP];
> > ^
> > drivers/media/radio/si4713/si4713.c:437:16: error: 'struct si4713_device'
> > has no member named 'supply_data'
> >
> > ^
> > drivers/media/radio/si4713/si4713.c: In function 'si4713_probe':
> > drivers/media/radio/si4713/si4713.c:1444:7: error: 'struct si4713_device'
> > has no member named 'supplies'
> > /* si4713_probe - probe for the device */
> >^
> > drivers/media/radio/si4713/si4713.c:1447:22: error: 'struct si4713_device'
> > has no member named 'supplies'
> > {
> > ^
> > drivers/media/radio/si4713/si4713.c:1448:7: error: 'struct si4713_device'
> > has no member named 'supply_data'
> > struct si4713_device *sdev;
> >^
> > drivers/media/radio/si4713/si4713.c:1450:46: error: 'struct si4713_device'
> > has no member named 'supplies'
> > struct v4l2_ctrl_handler *hdl;
> > ^
> > drivers/media/radio/si4713/si4713.c:1451:11: error: 'struct si4713_device'
> > has no member named 'supply_data'
> > int rval, i;
> >^
> > drivers/media/radio/si4713/si4713.c:1583:26: error: 'struct si4713_device'
> > has no member named 'supplies'
> >
> > ^
> > drivers/media/radio/si4713/si4713.c:1583:42: error: 'struct si4713_device'
> > has no member named 'supply_data'
> >
> > ^
> > drivers/media/radio/si4713/si4713.c: In function 'si4713_remove':
> > drivers/media/radio/si4713/si4713.c:1607:26: error: 'struct si4713_device'
> > has no member named 'supplies'
> >goto free_irq;
> > ^
> > drivers/media/radio/si4713/si4713.c:1607:42: error: 'struct si4713_device'
> > has no member named 'supply_data'
> >goto free_irq;
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-media" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a
Re: [RFCv2 1/8] [media] si4713: switch to devm regulator API
Hi Mauro,
On 11/11/2014 12:07 PM, Mauro Carvalho Chehab wrote:
> Em Tue, 21 Oct 2014 17:07:00 +0200
> Sebastian Reichel escreveu:
>
>> This switches back to the normal regulator API (but use
>> managed variant) in preparation for device tree support.
>
> This patch broke compilation. Please be sure that none of the patches in
> the series would break it, as otherwise git bisect would be broken.
Weird, as reported by Sebastian, it works for me.
However, after applying this patch I get these new warnings:
CC drivers/media/radio/si4713/radio-usb-si4713.o
drivers/media/radio/si4713/si4713.c: In function ‘si4713_probe’:
drivers/media/radio/si4713/si4713.c:1617:1: warning: label ‘free_gpio’ defined
but not used [-Wunused-label]
free_gpio:
^
drivers/media/radio/si4713/si4713.c:1451:12: warning: unused variable ‘i’
[-Wunused-variable]
int rval, i;
^
So it's probably not a good idea to merge this patch anyway until this is fixed.
Sebastian, can you fix these warnings and repost?
Thanks!
Hans
>
> Thanks,
> Mauro
>
> drivers/media/radio/si4713/si4713.c: In function 'si4713_powerup':
> drivers/media/radio/si4713/si4713.c:369:10: error: 'struct si4713_device' has
> no member named 'supplies'
>
> ^
> drivers/media/radio/si4713/si4713.c:370:35: error: 'struct si4713_device' has
> no member named 'supplies'
> if (sdev->vdd) {
>^
> drivers/media/radio/si4713/si4713.c:370:51: error: 'struct si4713_device' has
> no member named 'supply_data'
> if (sdev->vdd) {
>^
> drivers/media/radio/si4713/si4713.c:402:10: error: 'struct si4713_device' has
> no member named 'supplies'
>v4l2_dbg(1, debug, &sdev->sd, "Device in power up mode\n");
> ^
> drivers/media/radio/si4713/si4713.c:403:36: error: 'struct si4713_device' has
> no member named 'supplies'
>sdev->power_state = POWER_ON;
> ^
> drivers/media/radio/si4713/si4713.c:403:52: error: 'struct si4713_device' has
> no member named 'supply_data'
>sdev->power_state = POWER_ON;
> ^
> drivers/media/radio/si4713/si4713.c: In function 'si4713_powerdown':
> drivers/media/radio/si4713/si4713.c:435:11: error: 'struct si4713_device' has
> no member named 'supplies'
> int err;
>^
> drivers/media/radio/si4713/si4713.c:436:37: error: 'struct si4713_device' has
> no member named 'supplies'
> u8 resp[SI4713_PWDN_NRESP];
> ^
> drivers/media/radio/si4713/si4713.c:437:16: error: 'struct si4713_device' has
> no member named 'supply_data'
>
> ^
> drivers/media/radio/si4713/si4713.c: In function 'si4713_probe':
> drivers/media/radio/si4713/si4713.c:1444:7: error: 'struct si4713_device' has
> no member named 'supplies'
> /* si4713_probe - probe for the device */
>^
> drivers/media/radio/si4713/si4713.c:1447:22: error: 'struct si4713_device'
> has no member named 'supplies'
> {
> ^
> drivers/media/radio/si4713/si4713.c:1448:7: error: 'struct si4713_device' has
> no member named 'supply_data'
> struct si4713_device *sdev;
>^
> drivers/media/radio/si4713/si4713.c:1450:46: error: 'struct si4713_device'
> has no member named 'supplies'
> struct v4l2_ctrl_handler *hdl;
> ^
> drivers/media/radio/si4713/si4713.c:1451:11: error: 'struct si4713_device'
> has no member named 'supply_data'
> int rval, i;
>^
> drivers/media/radio/si4713/si4713.c:1583:26: error: 'struct si4713_device'
> has no member named 'supplies'
>
> ^
> drivers/media/radio/si4713/si4713.c:1583:42: error: 'struct si4713_device'
> has no member named 'supply_data'
>
> ^
> drivers/media/radio/si4713/si4713.c: In function 'si4713_remove':
> drivers/media/radio/si4713/si4713.c:1607:26: error: 'struct si4713_device'
> has no member named 'supplies'
>goto free_irq;
> ^
> drivers/media/radio/si4713/si4713.c:1607:42: error: 'struct si4713_device'
> has no member named 'supply_data'
>goto free_irq;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv2 1/8] [media] si4713: switch to devm regulator API
Hi Mauro, On Tue, Nov 11, 2014 at 09:07:10AM -0200, Mauro Carvalho Chehab wrote: > Em Tue, 21 Oct 2014 17:07:00 +0200 > Sebastian Reichel escreveu: > > > This switches back to the normal regulator API (but use > > managed variant) in preparation for device tree support. > > This patch broke compilation. Please be sure that none of the patches in > the series would break it, as otherwise git bisect would be broken. > > [...] mh, the errors seem to be from the old code (without the patch applied to drivers/media/radio/si4713/si4713.c) and the inlined code fragment displayed by the compiler seems to be the new code (with the patch applied to drivers/media/radio/si4713/si4713.c). Possible reasons I can think of: * You are using some kind of object cache, which assumed it could link the previously compiled si4713.o * You started the kernel compilation before merging the patch and the commit was only half applied when the compilation reached the si4713 driver. -- Sebastian signature.asc Description: Digital signature
Re: [RFCv2 1/8] [media] si4713: switch to devm regulator API
Em Tue, 21 Oct 2014 17:07:00 +0200
Sebastian Reichel escreveu:
> This switches back to the normal regulator API (but use
> managed variant) in preparation for device tree support.
This patch broke compilation. Please be sure that none of the patches in
the series would break it, as otherwise git bisect would be broken.
Thanks,
Mauro
drivers/media/radio/si4713/si4713.c: In function 'si4713_powerup':
drivers/media/radio/si4713/si4713.c:369:10: error: 'struct si4713_device' has
no member named 'supplies'
^
drivers/media/radio/si4713/si4713.c:370:35: error: 'struct si4713_device' has
no member named 'supplies'
if (sdev->vdd) {
^
drivers/media/radio/si4713/si4713.c:370:51: error: 'struct si4713_device' has
no member named 'supply_data'
if (sdev->vdd) {
^
drivers/media/radio/si4713/si4713.c:402:10: error: 'struct si4713_device' has
no member named 'supplies'
v4l2_dbg(1, debug, &sdev->sd, "Device in power up mode\n");
^
drivers/media/radio/si4713/si4713.c:403:36: error: 'struct si4713_device' has
no member named 'supplies'
sdev->power_state = POWER_ON;
^
drivers/media/radio/si4713/si4713.c:403:52: error: 'struct si4713_device' has
no member named 'supply_data'
sdev->power_state = POWER_ON;
^
drivers/media/radio/si4713/si4713.c: In function 'si4713_powerdown':
drivers/media/radio/si4713/si4713.c:435:11: error: 'struct si4713_device' has
no member named 'supplies'
int err;
^
drivers/media/radio/si4713/si4713.c:436:37: error: 'struct si4713_device' has
no member named 'supplies'
u8 resp[SI4713_PWDN_NRESP];
^
drivers/media/radio/si4713/si4713.c:437:16: error: 'struct si4713_device' has
no member named 'supply_data'
^
drivers/media/radio/si4713/si4713.c: In function 'si4713_probe':
drivers/media/radio/si4713/si4713.c:1444:7: error: 'struct si4713_device' has
no member named 'supplies'
/* si4713_probe - probe for the device */
^
drivers/media/radio/si4713/si4713.c:1447:22: error: 'struct si4713_device' has
no member named 'supplies'
{
^
drivers/media/radio/si4713/si4713.c:1448:7: error: 'struct si4713_device' has
no member named 'supply_data'
struct si4713_device *sdev;
^
drivers/media/radio/si4713/si4713.c:1450:46: error: 'struct si4713_device' has
no member named 'supplies'
struct v4l2_ctrl_handler *hdl;
^
drivers/media/radio/si4713/si4713.c:1451:11: error: 'struct si4713_device' has
no member named 'supply_data'
int rval, i;
^
drivers/media/radio/si4713/si4713.c:1583:26: error: 'struct si4713_device' has
no member named 'supplies'
^
drivers/media/radio/si4713/si4713.c:1583:42: error: 'struct si4713_device' has
no member named 'supply_data'
^
drivers/media/radio/si4713/si4713.c: In function 'si4713_remove':
drivers/media/radio/si4713/si4713.c:1607:26: error: 'struct si4713_device' has
no member named 'supplies'
goto free_irq;
^
drivers/media/radio/si4713/si4713.c:1607:42: error: 'struct si4713_device' has
no member named 'supply_data'
goto free_irq;
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv2 1/8] [media] si4713: switch to devm regulator API
This switches back to the normal regulator API (but use
managed variant) in preparation for device tree support.
Signed-off-by: Sebastian Reichel
---
drivers/media/radio/si4713/si4713.c | 80 ++---
drivers/media/radio/si4713/si4713.h | 6 +--
2 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/drivers/media/radio/si4713/si4713.c
b/drivers/media/radio/si4713/si4713.c
index b576555..b335093 100644
--- a/drivers/media/radio/si4713/si4713.c
+++ b/drivers/media/radio/si4713/si4713.c
@@ -23,6 +23,7 @@
#include
#include
+#include
#include
#include
#include
@@ -366,13 +367,22 @@ static int si4713_powerup(struct si4713_device *sdev)
if (sdev->power_state)
return 0;
- if (sdev->supplies) {
- err = regulator_bulk_enable(sdev->supplies, sdev->supply_data);
+ if (sdev->vdd) {
+ err = regulator_enable(sdev->vdd);
if (err) {
- v4l2_err(&sdev->sd, "Failed to enable supplies: %d\n",
err);
+ v4l2_err(&sdev->sd, "Failed to enable vdd: %d\n", err);
return err;
}
}
+
+ if (sdev->vio) {
+ err = regulator_enable(sdev->vio);
+ if (err) {
+ v4l2_err(&sdev->sd, "Failed to enable vio: %d\n", err);
+ return err;
+ }
+ }
+
if (gpio_is_valid(sdev->gpio_reset)) {
udelay(50);
gpio_set_value(sdev->gpio_reset, 1);
@@ -399,11 +409,18 @@ static int si4713_powerup(struct si4713_device *sdev)
}
if (gpio_is_valid(sdev->gpio_reset))
gpio_set_value(sdev->gpio_reset, 0);
- if (sdev->supplies) {
- err = regulator_bulk_disable(sdev->supplies, sdev->supply_data);
+
+
+ if (sdev->vdd) {
+ err = regulator_disable(sdev->vdd);
if (err)
- v4l2_err(&sdev->sd,
-"Failed to disable supplies: %d\n", err);
+ v4l2_err(&sdev->sd, "Failed to disable vdd: %d\n", err);
+ }
+
+ if (sdev->vio) {
+ err = regulator_disable(sdev->vio);
+ if (err)
+ v4l2_err(&sdev->sd, "Failed to disable vio: %d\n", err);
}
return err;
@@ -432,12 +449,21 @@ static int si4713_powerdown(struct si4713_device *sdev)
v4l2_dbg(1, debug, &sdev->sd, "Device in reset mode\n");
if (gpio_is_valid(sdev->gpio_reset))
gpio_set_value(sdev->gpio_reset, 0);
- if (sdev->supplies) {
- err = regulator_bulk_disable(sdev->supplies,
-sdev->supply_data);
- if (err)
+
+ if (sdev->vdd) {
+ err = regulator_disable(sdev->vdd);
+ if (err) {
+ v4l2_err(&sdev->sd,
+ "Failed to disable vdd: %d\n", err);
+ }
+ }
+
+ if (sdev->vio) {
+ err = regulator_disable(sdev->vio);
+ if (err) {
v4l2_err(&sdev->sd,
-"Failed to disable supplies: %d\n",
err);
+ "Failed to disable vio: %d\n", err);
+ }
}
sdev->power_state = POWER_OFF;
}
@@ -1441,17 +1467,26 @@ static int si4713_probe(struct i2c_client *client,
}
sdev->gpio_reset = pdata->gpio_reset;
gpio_direction_output(sdev->gpio_reset, 0);
- sdev->supplies = pdata->supplies;
}
- for (i = 0; i < sdev->supplies; i++)
- sdev->supply_data[i].supply = pdata->supply_names[i];
+ sdev->vdd = devm_regulator_get_optional(&client->dev, "vdd");
+ if (IS_ERR(sdev->vdd)) {
+ rval = PTR_ERR(sdev->vdd);
+ if (rval == -EPROBE_DEFER)
+ goto exit;
+
+ dev_dbg(&client->dev, "no vdd regulator found: %d\n", rval);
+ sdev->vdd = NULL;
+ }
+
+ sdev->vio = devm_regulator_get_optional(&client->dev, "vio");
+ if (IS_ERR(sdev->vio)) {
+ rval = PTR_ERR(sdev->vio);
+ if (rval == -EPROBE_DEFER)
+ goto exit;
- rval = regulator_bulk_get(&client->dev, sdev->supplies,
- sdev->supply_data);
- if (rval) {
- dev_err(&client->dev, "Cannot get regulators: %d\n", rval);
- goto free_gpio;
+ dev_dbg(&client->dev, "no vio regulator found: %d\n", rval);
+ sdev->vio = NULL;
}
v4l2_i2c_subdev_init(&sdev->sd, cl
