From: Hans Verkuil <hansverk@cisco.com>
To: "edubezval@gmail.com" <edubezval@gmail.com>
Cc: Dinesh Ram <dinesh.ram@cern.ch>,
Linux-Media <linux-media@vger.kernel.org>,
Hans Verkuil <hverkuil@xs4all.nl>,
d ram <dinesh.ram086@gmail.com>,
Hans Verkuil <hans.verkuil@cisco.com>
Subject: Re: [REVIEW PATCH 8/9] si4713: move supply list to si4713_platform_data
Date: Tue, 05 Nov 2013 09:38:18 +0100 [thread overview]
Message-ID: <5278AE7A.5010000@cisco.com> (raw)
In-Reply-To: <CAC-25o9YZjLnwUmt_q17V1Xiu8wubgrn=uLpX31Zu_H6PwF73A@mail.gmail.com>
Hi,
On 11/04/13 15:07, edubezval@gmail.com wrote:
> Hi,
>
> On Tue, Oct 15, 2013 at 11:24 AM, Dinesh Ram <dinesh.ram@cern.ch> wrote:
>> The supply list is needed by the platform driver, but not by the usb driver.
>> So this information belongs to the platform data and should not be hardcoded
>> in the subdevice driver.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>
> Dinesh, could you please sign this patch too?
>
>> ---
>> arch/arm/mach-omap2/board-rx51-peripherals.c | 7 ++++
>> drivers/media/radio/si4713/si4713.c | 52 +++++++++++++-------------
>> drivers/media/radio/si4713/si4713.h | 3 +-
>> include/media/si4713.h | 2 +
>> 4 files changed, 37 insertions(+), 27 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c b/arch/arm/mach-omap2/board-rx51-peripherals.c
>> index f6fe388..eae73f7 100644
>> --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
>> +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
>> @@ -776,7 +776,14 @@ static struct regulator_init_data rx51_vintdig = {
>> },
>> };
>>
>> +static const char * const si4713_supply_names[SI4713_NUM_SUPPLIES] = {
>
> This patch produces the following compilation error:
> arch/arm/mach-omap2/board-rx51-peripherals.c:779:47: error:
> 'SI4713_NUM_SUPPLIES' undeclared here (not in a function)
Hmm, I thought I had compile-tested this, apparently not. Does it compile if
you just remove SI4713_NUM_SUPPLIES? It's not necessary here.
Regards,
Hans
> arch/arm/mach-omap2/board-rx51-peripherals.c:785:14: error: bit-field
> '<anonymous>' width not an integer constant
> arch/arm/mach-omap2/board-rx51-peripherals.c:779:27: warning:
> 'si4713_supply_names' defined but not used [-Wunused-variable]
> make[1]: *** [arch/arm/mach-omap2/board-rx51-peripherals.o] Error 1
> make: *** [arch/arm/mach-omap2] Error 2
> make: *** Waiting for unfinished jobs....
>
>
>> + "vio",
>> + "vdd",
>> +};
>> +
>> static struct si4713_platform_data rx51_si4713_i2c_data __initdata_or_module = {
>> + .supplies = ARRAY_SIZE(si4713_supply_names),
>> + .supply_names = si4713_supply_names,
>> .gpio_reset = RX51_FMTX_RESET_GPIO,
>> };
>>
>> diff --git a/drivers/media/radio/si4713/si4713.c b/drivers/media/radio/si4713/si4713.c
>> index d297a5b..920dfa5 100644
>> --- a/drivers/media/radio/si4713/si4713.c
>> +++ b/drivers/media/radio/si4713/si4713.c
>> @@ -44,11 +44,6 @@ MODULE_AUTHOR("Eduardo Valentin <eduardo.valentin@nokia.com>");
>> MODULE_DESCRIPTION("I2C driver for Si4713 FM Radio Transmitter");
>> MODULE_VERSION("0.0.1");
>>
>> -static const char *si4713_supply_names[SI4713_NUM_SUPPLIES] = {
>> - "vio",
>> - "vdd",
>> -};
>> -
>> #define DEFAULT_RDS_PI 0x00
>> #define DEFAULT_RDS_PTY 0x00
>> #define DEFAULT_RDS_DEVIATION 0x00C8
>> @@ -368,11 +363,12 @@ static int si4713_powerup(struct si4713_device *sdev)
>> if (sdev->power_state)
>> return 0;
>>
>> - err = regulator_bulk_enable(ARRAY_SIZE(sdev->supplies),
>> - sdev->supplies);
>> - if (err) {
>> - v4l2_err(&sdev->sd, "Failed to enable supplies: %d\n", err);
>> - return err;
>> + if (sdev->supplies) {
>> + err = regulator_bulk_enable(sdev->supplies, sdev->supply_data);
>> + if (err) {
>> + v4l2_err(&sdev->sd, "Failed to enable supplies: %d\n", err);
>> + return err;
>> + }
>> }
>> if (gpio_is_valid(sdev->gpio_reset)) {
>> udelay(50);
>> @@ -396,11 +392,12 @@ static int si4713_powerup(struct si4713_device *sdev)
>> if (client->irq)
>> err = si4713_write_property(sdev, SI4713_GPO_IEN,
>> SI4713_STC_INT | SI4713_CTS);
>> - } else {
>> - if (gpio_is_valid(sdev->gpio_reset))
>> - gpio_set_value(sdev->gpio_reset, 0);
>> - err = regulator_bulk_disable(ARRAY_SIZE(sdev->supplies),
>> - sdev->supplies);
>> + return err;
>> + }
>> + 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)
>> v4l2_err(&sdev->sd,
>> "Failed to disable supplies: %d\n", err);
>> @@ -432,11 +429,13 @@ 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);
>> - err = regulator_bulk_disable(ARRAY_SIZE(sdev->supplies),
>> - sdev->supplies);
>> - if (err)
>> - v4l2_err(&sdev->sd,
>> - "Failed to disable supplies: %d\n", err);
>> + if (sdev->supplies) {
>> + err = regulator_bulk_disable(sdev->supplies,
>> + sdev->supply_data);
>> + if (err)
>> + v4l2_err(&sdev->sd,
>> + "Failed to disable supplies: %d\n", err);
>> + }
>> sdev->power_state = POWER_OFF;
>> }
>>
>> @@ -1381,13 +1380,14 @@ 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 < ARRAY_SIZE(sdev->supplies); i++)
>> - sdev->supplies[i].supply = si4713_supply_names[i];
>> + for (i = 0; i < sdev->supplies; i++)
>> + sdev->supply_data[i].supply = pdata->supply_names[i];
>>
>> - rval = regulator_bulk_get(&client->dev, ARRAY_SIZE(sdev->supplies),
>> - sdev->supplies);
>> + 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;
>> @@ -1500,7 +1500,7 @@ free_irq:
>> free_ctrls:
>> v4l2_ctrl_handler_free(hdl);
>> put_reg:
>> - regulator_bulk_free(ARRAY_SIZE(sdev->supplies), sdev->supplies);
>> + regulator_bulk_free(sdev->supplies, sdev->supply_data);
>> free_gpio:
>> if (gpio_is_valid(sdev->gpio_reset))
>> gpio_free(sdev->gpio_reset);
>> @@ -1524,7 +1524,7 @@ static int si4713_remove(struct i2c_client *client)
>>
>> v4l2_device_unregister_subdev(sd);
>> v4l2_ctrl_handler_free(sd->ctrl_handler);
>> - regulator_bulk_free(ARRAY_SIZE(sdev->supplies), sdev->supplies);
>> + regulator_bulk_free(sdev->supplies, sdev->supply_data);
>> if (gpio_is_valid(sdev->gpio_reset))
>> gpio_free(sdev->gpio_reset);
>> kfree(sdev);
>> diff --git a/drivers/media/radio/si4713/si4713.h b/drivers/media/radio/si4713/si4713.h
>> index dc0ce66..986e27b 100644
>> --- a/drivers/media/radio/si4713/si4713.h
>> +++ b/drivers/media/radio/si4713/si4713.h
>> @@ -227,7 +227,8 @@ struct si4713_device {
>> struct v4l2_ctrl *tune_ant_cap;
>> };
>> struct completion work;
>> - struct regulator_bulk_data supplies[SI4713_NUM_SUPPLIES];
>> + unsigned supplies;
>> + struct regulator_bulk_data supply_data[SI4713_NUM_SUPPLIES];
>> int gpio_reset;
>> u32 power_state;
>> u32 rds_enabled;
>> diff --git a/include/media/si4713.h b/include/media/si4713.h
>> index ed7353e..f98a0a7 100644
>> --- a/include/media/si4713.h
>> +++ b/include/media/si4713.h
>> @@ -23,6 +23,8 @@
>> * Platform dependent definition
>> */
>> struct si4713_platform_data {
>> + const char * const *supply_names;
>> + unsigned supplies;
>> int gpio_reset; /* < 0 if not used */
>> };
>>
>> --
>> 1.7.9.5
>>
>
>
>
next prev parent reply other threads:[~2013-11-05 8:49 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-15 15:24 [Review Patch 0/9] si4713 usb device driver Dinesh Ram
2013-10-15 15:24 ` [REVIEW PATCH 1/9] si4713 : Reorganized drivers/media/radio directory Dinesh Ram
2013-10-15 15:24 ` [REVIEW PATCH 2/9] si4713 : Modified i2c driver to handle cases where interrupts are not used Dinesh Ram
2013-12-03 15:35 ` Mauro Carvalho Chehab
2013-12-03 17:06 ` Hans Verkuil
[not found] ` <1386129496.79520.YahooMailNeo@web190906.mail.sg3.yahoo.com>
2013-12-04 17:42 ` Mauro Carvalho Chehab
2013-12-05 7:39 ` Hans Verkuil
2013-10-15 15:24 ` [REVIEW PATCH 3/9] si4713 : Reorganized includes in si4713.c/h Dinesh Ram
2013-10-15 15:24 ` [REVIEW PATCH 4/9] si4713 : Bug fix for si4713_tx_tune_power() method in the i2c driver Dinesh Ram
2013-10-15 15:24 ` [REVIEW PATCH 5/9] si4713 : HID blacklist Si4713 USB development board Dinesh Ram
2013-10-15 15:24 ` [REVIEW PATCH 6/9] si4713 : Added the USB driver for Si4713 Dinesh Ram
2013-11-05 14:18 ` edubezval
2013-11-07 7:40 ` Hans Verkuil
2013-11-11 12:34 ` edubezval
2013-11-18 14:47 ` edubezval
2013-10-15 15:24 ` [REVIEW PATCH 7/9] si4713 : Added MAINTAINERS entry for radio-usb-si4713 driver Dinesh Ram
2013-10-15 15:24 ` [REVIEW PATCH 8/9] si4713: move supply list to si4713_platform_data Dinesh Ram
2013-11-04 14:07 ` edubezval
2013-11-05 8:38 ` Hans Verkuil [this message]
2013-11-05 14:14 ` edubezval
2013-10-15 15:24 ` [REVIEW PATCH 9/9] si4713: si4713_set_rds_radio_text overwrites terminating \0 Dinesh Ram
2013-12-03 15:39 ` [REVIEW PATCH 1/9] si4713 : Reorganized drivers/media/radio directory Mauro Carvalho Chehab
[not found] ` <CAP_RhzeRgLir1FGL6UN2-yXXaS-1knsS2BP20MjfMJRAEyDqeg@mail.gmail.com>
2013-12-03 16:06 ` FW: " Dinesh Ram
2013-12-03 16:58 ` Hans Verkuil
2013-12-04 17:41 ` Mauro Carvalho Chehab
2013-10-15 17:37 ` [Review Patch 0/9] si4713 usb device driver edubezval
2013-11-04 9:33 ` Hans Verkuil
2013-11-04 14:09 ` edubezval
2013-11-04 14:13 ` Hans Verkuil
[not found] ` <CAP_RhzfWKc8y27uU4VXFu6cAt87NvO=BnLNq9WeGG_kpxihTKQ@mail.gmail.com>
2013-11-04 14:39 ` edubezval
2013-11-18 14:31 ` edubezval
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=5278AE7A.5010000@cisco.com \
--to=hansverk@cisco.com \
--cc=dinesh.ram086@gmail.com \
--cc=dinesh.ram@cern.ch \
--cc=edubezval@gmail.com \
--cc=hans.verkuil@cisco.com \
--cc=hverkuil@xs4all.nl \
--cc=linux-media@vger.kernel.org \
/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