Re: [PATCH 09/10] [media] tvp5150: Initialize the chip on probe

2016-01-06 Thread Javier Martinez Canillas
Hello Laurent,

On 01/06/2016 07:59 AM, Laurent Pinchart wrote:
> Hi Javier,
> 
> Thankk you for the patch.
>

Thanks a lot for your feedback.
 
> On Monday 04 January 2016 09:25:31 Javier Martinez Canillas wrote:
>> After power-up, the tvp5150 decoder is in a unknown state until the
>> RESETB pin is driven LOW which reset all the registers and restarts
>> the chip's internal state machine.
>>
>> The init sequence has some timing constraints and the RESETB signal
>> can only be used if the PDN (Power-down) pin is first released.
>>
>> So, the initialization sequence is as follows:
>>
>> 1- PDN (active-low) is driven HIGH so the chip is power-up
>> 2- A 20 ms delay is needed before sending a RESETB (active-low) signal.
>> 3- The RESETB pulse duration is 500 ns.
>> 4- A 200 us delay is needed for the I2C client to be active after reset.
>>
>> This patch used as a reference the logic in the IGEPv2 board file from
>> the ISEE 2.6.37 vendor tree.
>>
>> Signed-off-by: Javier Martinez Canillas 
>> ---
>>
>>  drivers/media/i2c/tvp5150.c | 35 +++
>>  1 file changed, 35 insertions(+)
>>
>> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
>> index caac96a577f8..fed89a811ab7 100644
>> --- a/drivers/media/i2c/tvp5150.c
>> +++ b/drivers/media/i2c/tvp5150.c
>> @@ -5,6 +5,7 @@
>>   * This code is placed under the terms of the GNU General Public License v2
>> */
>>
>> +#include 
> 
> Let's keep the headers sorted alphabetically if you don't mind :-)
>

Right, sorry about that.
 
>>  #include 
>>  #include 
>>  #include 
>> @@ -1197,6 +1198,36 @@ static int tvp5150_detect_version(struct tvp5150
>> *core) return 0;
>>  }
>>
>> +static inline int tvp5150_init(struct i2c_client *c)
>> +{
>> +struct gpio_desc *pdn_gpio;
>> +struct gpio_desc *reset_gpio;
>> +
>> +pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", 
>> GPIOD_OUT_HIGH);
>> +if (IS_ERR(pdn_gpio))
>> +return PTR_ERR(pdn_gpio);
>> +
>> +if (pdn_gpio) {
>> +gpiod_set_value_cansleep(pdn_gpio, 0);
>> +/* Delay time between power supplies active and reset */
>> +msleep(20);
> 
> How about usleep_range() instead ?
>

Documentation/timers/timers-howto.txt says that usleep_range() should be
used for 1ms - 20ms and msleep() for 20ms+ so since this was a boundary
value, I chose for msleep() but I can use usleep_range() if you want.

I've no strong opinion on this.

>> +}
>> +
>> +reset_gpio = devm_gpiod_get_optional(>dev, "reset", GPIOD_OUT_HIGH);
>> +if (IS_ERR(reset_gpio))
>> +return PTR_ERR(reset_gpio);
>> +
>> +if (reset_gpio) {
>> +/* RESETB pulse duration */
>> +ndelay(500);
> 
> Is the timing so sensitive that we need a delay, or could we use 
> usleep_range() ?
>

According to my tests, it is a minimum value but I chose to do a delay since
the time is very short and also Documentation/timers/timers-howto.txt says
that using usleep for very short periods may not be worth it due the overhead
of setting up the hrtimers for usleep.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 09/10] [media] tvp5150: Initialize the chip on probe

2016-01-06 Thread Laurent Pinchart
Hi Javier,

Thankk you for the patch.

On Monday 04 January 2016 09:25:31 Javier Martinez Canillas wrote:
> After power-up, the tvp5150 decoder is in a unknown state until the
> RESETB pin is driven LOW which reset all the registers and restarts
> the chip's internal state machine.
> 
> The init sequence has some timing constraints and the RESETB signal
> can only be used if the PDN (Power-down) pin is first released.
> 
> So, the initialization sequence is as follows:
> 
> 1- PDN (active-low) is driven HIGH so the chip is power-up
> 2- A 20 ms delay is needed before sending a RESETB (active-low) signal.
> 3- The RESETB pulse duration is 500 ns.
> 4- A 200 us delay is needed for the I2C client to be active after reset.
> 
> This patch used as a reference the logic in the IGEPv2 board file from
> the ISEE 2.6.37 vendor tree.
> 
> Signed-off-by: Javier Martinez Canillas 
> ---
> 
>  drivers/media/i2c/tvp5150.c | 35 +++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index caac96a577f8..fed89a811ab7 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -5,6 +5,7 @@
>   * This code is placed under the terms of the GNU General Public License v2
> */
> 
> +#include 

Let's keep the headers sorted alphabetically if you don't mind :-)

>  #include 
>  #include 
>  #include 
> @@ -1197,6 +1198,36 @@ static int tvp5150_detect_version(struct tvp5150
> *core) return 0;
>  }
> 
> +static inline int tvp5150_init(struct i2c_client *c)
> +{
> + struct gpio_desc *pdn_gpio;
> + struct gpio_desc *reset_gpio;
> +
> + pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", 
> GPIOD_OUT_HIGH);
> + if (IS_ERR(pdn_gpio))
> + return PTR_ERR(pdn_gpio);
> +
> + if (pdn_gpio) {
> + gpiod_set_value_cansleep(pdn_gpio, 0);
> + /* Delay time between power supplies active and reset */
> + msleep(20);

How about usleep_range() instead ?

> + }
> +
> + reset_gpio = devm_gpiod_get_optional(>dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(reset_gpio))
> + return PTR_ERR(reset_gpio);
> +
> + if (reset_gpio) {
> + /* RESETB pulse duration */
> + ndelay(500);

Is the timing so sensitive that we need a delay, or could we use 
usleep_range() ?

> + gpiod_set_value_cansleep(reset_gpio, 0);
> + /* Delay time between end of reset to I2C active */
> + usleep_range(200, 250);
> + }
> +
> + return 0;
> +}
> +
>  static int tvp5150_probe(struct i2c_client *c,
>const struct i2c_device_id *id)
>  {
> @@ -1209,6 +1240,10 @@ static int tvp5150_probe(struct i2c_client *c,
>I2C_FUNC_SMBUS_READ_BYTE | I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
>   return -EIO;
> 
> + res = tvp5150_init(c);
> + if (res)
> + return res;
> +
>   core = devm_kzalloc(>dev, sizeof(*core), GFP_KERNEL);
>   if (!core)
>   return -ENOMEM;

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 09/10] [media] tvp5150: Initialize the chip on probe

2016-01-06 Thread Laurent Pinchart
Hi Javier,

Thankk you for the patch.

On Monday 04 January 2016 09:25:31 Javier Martinez Canillas wrote:
> After power-up, the tvp5150 decoder is in a unknown state until the
> RESETB pin is driven LOW which reset all the registers and restarts
> the chip's internal state machine.
> 
> The init sequence has some timing constraints and the RESETB signal
> can only be used if the PDN (Power-down) pin is first released.
> 
> So, the initialization sequence is as follows:
> 
> 1- PDN (active-low) is driven HIGH so the chip is power-up
> 2- A 20 ms delay is needed before sending a RESETB (active-low) signal.
> 3- The RESETB pulse duration is 500 ns.
> 4- A 200 us delay is needed for the I2C client to be active after reset.
> 
> This patch used as a reference the logic in the IGEPv2 board file from
> the ISEE 2.6.37 vendor tree.
> 
> Signed-off-by: Javier Martinez Canillas 
> ---
> 
>  drivers/media/i2c/tvp5150.c | 35 +++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index caac96a577f8..fed89a811ab7 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -5,6 +5,7 @@
>   * This code is placed under the terms of the GNU General Public License v2
> */
> 
> +#include 

Let's keep the headers sorted alphabetically if you don't mind :-)

>  #include 
>  #include 
>  #include 
> @@ -1197,6 +1198,36 @@ static int tvp5150_detect_version(struct tvp5150
> *core) return 0;
>  }
> 
> +static inline int tvp5150_init(struct i2c_client *c)
> +{
> + struct gpio_desc *pdn_gpio;
> + struct gpio_desc *reset_gpio;
> +
> + pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", 
> GPIOD_OUT_HIGH);
> + if (IS_ERR(pdn_gpio))
> + return PTR_ERR(pdn_gpio);
> +
> + if (pdn_gpio) {
> + gpiod_set_value_cansleep(pdn_gpio, 0);
> + /* Delay time between power supplies active and reset */
> + msleep(20);

How about usleep_range() instead ?

> + }
> +
> + reset_gpio = devm_gpiod_get_optional(>dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(reset_gpio))
> + return PTR_ERR(reset_gpio);
> +
> + if (reset_gpio) {
> + /* RESETB pulse duration */
> + ndelay(500);

Is the timing so sensitive that we need a delay, or could we use 
usleep_range() ?

> + gpiod_set_value_cansleep(reset_gpio, 0);
> + /* Delay time between end of reset to I2C active */
> + usleep_range(200, 250);
> + }
> +
> + return 0;
> +}
> +
>  static int tvp5150_probe(struct i2c_client *c,
>const struct i2c_device_id *id)
>  {
> @@ -1209,6 +1240,10 @@ static int tvp5150_probe(struct i2c_client *c,
>I2C_FUNC_SMBUS_READ_BYTE | I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
>   return -EIO;
> 
> + res = tvp5150_init(c);
> + if (res)
> + return res;
> +
>   core = devm_kzalloc(>dev, sizeof(*core), GFP_KERNEL);
>   if (!core)
>   return -ENOMEM;

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 09/10] [media] tvp5150: Initialize the chip on probe

2016-01-06 Thread Javier Martinez Canillas
Hello Laurent,

On 01/06/2016 07:59 AM, Laurent Pinchart wrote:
> Hi Javier,
> 
> Thankk you for the patch.
>

Thanks a lot for your feedback.
 
> On Monday 04 January 2016 09:25:31 Javier Martinez Canillas wrote:
>> After power-up, the tvp5150 decoder is in a unknown state until the
>> RESETB pin is driven LOW which reset all the registers and restarts
>> the chip's internal state machine.
>>
>> The init sequence has some timing constraints and the RESETB signal
>> can only be used if the PDN (Power-down) pin is first released.
>>
>> So, the initialization sequence is as follows:
>>
>> 1- PDN (active-low) is driven HIGH so the chip is power-up
>> 2- A 20 ms delay is needed before sending a RESETB (active-low) signal.
>> 3- The RESETB pulse duration is 500 ns.
>> 4- A 200 us delay is needed for the I2C client to be active after reset.
>>
>> This patch used as a reference the logic in the IGEPv2 board file from
>> the ISEE 2.6.37 vendor tree.
>>
>> Signed-off-by: Javier Martinez Canillas 
>> ---
>>
>>  drivers/media/i2c/tvp5150.c | 35 +++
>>  1 file changed, 35 insertions(+)
>>
>> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
>> index caac96a577f8..fed89a811ab7 100644
>> --- a/drivers/media/i2c/tvp5150.c
>> +++ b/drivers/media/i2c/tvp5150.c
>> @@ -5,6 +5,7 @@
>>   * This code is placed under the terms of the GNU General Public License v2
>> */
>>
>> +#include 
> 
> Let's keep the headers sorted alphabetically if you don't mind :-)
>

Right, sorry about that.
 
>>  #include 
>>  #include 
>>  #include 
>> @@ -1197,6 +1198,36 @@ static int tvp5150_detect_version(struct tvp5150
>> *core) return 0;
>>  }
>>
>> +static inline int tvp5150_init(struct i2c_client *c)
>> +{
>> +struct gpio_desc *pdn_gpio;
>> +struct gpio_desc *reset_gpio;
>> +
>> +pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", 
>> GPIOD_OUT_HIGH);
>> +if (IS_ERR(pdn_gpio))
>> +return PTR_ERR(pdn_gpio);
>> +
>> +if (pdn_gpio) {
>> +gpiod_set_value_cansleep(pdn_gpio, 0);
>> +/* Delay time between power supplies active and reset */
>> +msleep(20);
> 
> How about usleep_range() instead ?
>

Documentation/timers/timers-howto.txt says that usleep_range() should be
used for 1ms - 20ms and msleep() for 20ms+ so since this was a boundary
value, I chose for msleep() but I can use usleep_range() if you want.

I've no strong opinion on this.

>> +}
>> +
>> +reset_gpio = devm_gpiod_get_optional(>dev, "reset", GPIOD_OUT_HIGH);
>> +if (IS_ERR(reset_gpio))
>> +return PTR_ERR(reset_gpio);
>> +
>> +if (reset_gpio) {
>> +/* RESETB pulse duration */
>> +ndelay(500);
> 
> Is the timing so sensitive that we need a delay, or could we use 
> usleep_range() ?
>

According to my tests, it is a minimum value but I chose to do a delay since
the time is very short and also Documentation/timers/timers-howto.txt says
that using usleep for very short periods may not be worth it due the overhead
of setting up the hrtimers for usleep.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 09/10] [media] tvp5150: Initialize the chip on probe

2016-01-04 Thread Javier Martinez Canillas
Hello,

On 01/04/2016 09:40 AM, kbuild test robot wrote:
> Hi Javier,
> 
> [auto build test ERROR on linuxtv-media/master]
> [also build test ERROR on v4.4-rc8 next-20151231]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improving the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Javier-Martinez-Canillas/tvp5150-add-MC-and-DT-support/20160104-203224
> base:   git://linuxtv.org/media_tree.git master
> config: x86_64-randconfig-x008-01040711 (attached as .config)
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>drivers/media/i2c/tvp5150.c: In function 'tvp5150_init':
>>> drivers/media/i2c/tvp5150.c:1206:13: error: implicit declaration of 
>>> function 'devm_gpiod_get_optional' [-Werror=implicit-function-declaration]
>  pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", GPIOD_OUT_HIGH);
> ^
>>> drivers/media/i2c/tvp5150.c:1206:59: error: 'GPIOD_OUT_HIGH' undeclared 
>>> (first use in this function)
>  pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", GPIOD_OUT_HIGH);
>   ^
>drivers/media/i2c/tvp5150.c:1206:59: note: each undeclared identifier is 
> reported only once for each function it appears in
>>> drivers/media/i2c/tvp5150.c:1211:3: error: implicit declaration of function 
>>> 'gpiod_set_value_cansleep' [-Werror=implicit-function-declaration]
>   gpiod_set_value_cansleep(pdn_gpio, 0);
>

Sigh, it's caused by a missing include for the  header.

Thanks for reporting, I'll wait a couple of days to see if I get more feedback
and then post a v2 fixing this.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 09/10] [media] tvp5150: Initialize the chip on probe

2016-01-04 Thread kbuild test robot
Hi Javier,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.4-rc8 next-20151231]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Javier-Martinez-Canillas/tvp5150-add-MC-and-DT-support/20160104-203224
base:   git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-x008-01040711 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/media/i2c/tvp5150.c: In function 'tvp5150_init':
>> drivers/media/i2c/tvp5150.c:1206:13: error: implicit declaration of function 
>> 'devm_gpiod_get_optional' [-Werror=implicit-function-declaration]
 pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", GPIOD_OUT_HIGH);
^
>> drivers/media/i2c/tvp5150.c:1206:59: error: 'GPIOD_OUT_HIGH' undeclared 
>> (first use in this function)
 pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", GPIOD_OUT_HIGH);
  ^
   drivers/media/i2c/tvp5150.c:1206:59: note: each undeclared identifier is 
reported only once for each function it appears in
>> drivers/media/i2c/tvp5150.c:1211:3: error: implicit declaration of function 
>> 'gpiod_set_value_cansleep' [-Werror=implicit-function-declaration]
  gpiod_set_value_cansleep(pdn_gpio, 0);
  ^
   cc1: some warnings being treated as errors

vim +/devm_gpiod_get_optional +1206 drivers/media/i2c/tvp5150.c

  1200  
  1201  static inline int tvp5150_init(struct i2c_client *c)
  1202  {
  1203  struct gpio_desc *pdn_gpio;
  1204  struct gpio_desc *reset_gpio;
  1205  
> 1206  pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", 
> GPIOD_OUT_HIGH);
  1207  if (IS_ERR(pdn_gpio))
  1208  return PTR_ERR(pdn_gpio);
  1209  
  1210  if (pdn_gpio) {
> 1211  gpiod_set_value_cansleep(pdn_gpio, 0);
  1212  /* Delay time between power supplies active and reset */
  1213  msleep(20);
  1214  }

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data


[PATCH 09/10] [media] tvp5150: Initialize the chip on probe

2016-01-04 Thread Javier Martinez Canillas
After power-up, the tvp5150 decoder is in a unknown state until the
RESETB pin is driven LOW which reset all the registers and restarts
the chip's internal state machine.

The init sequence has some timing constraints and the RESETB signal
can only be used if the PDN (Power-down) pin is first released.

So, the initialization sequence is as follows:

1- PDN (active-low) is driven HIGH so the chip is power-up
2- A 20 ms delay is needed before sending a RESETB (active-low) signal.
3- The RESETB pulse duration is 500 ns.
4- A 200 us delay is needed for the I2C client to be active after reset.

This patch used as a reference the logic in the IGEPv2 board file from
the ISEE 2.6.37 vendor tree.

Signed-off-by: Javier Martinez Canillas 
---

 drivers/media/i2c/tvp5150.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index caac96a577f8..fed89a811ab7 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -5,6 +5,7 @@
  * This code is placed under the terms of the GNU General Public License v2
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -1197,6 +1198,36 @@ static int tvp5150_detect_version(struct tvp5150 *core)
return 0;
 }
 
+static inline int tvp5150_init(struct i2c_client *c)
+{
+   struct gpio_desc *pdn_gpio;
+   struct gpio_desc *reset_gpio;
+
+   pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", 
GPIOD_OUT_HIGH);
+   if (IS_ERR(pdn_gpio))
+   return PTR_ERR(pdn_gpio);
+
+   if (pdn_gpio) {
+   gpiod_set_value_cansleep(pdn_gpio, 0);
+   /* Delay time between power supplies active and reset */
+   msleep(20);
+   }
+
+   reset_gpio = devm_gpiod_get_optional(>dev, "reset", GPIOD_OUT_HIGH);
+   if (IS_ERR(reset_gpio))
+   return PTR_ERR(reset_gpio);
+
+   if (reset_gpio) {
+   /* RESETB pulse duration */
+   ndelay(500);
+   gpiod_set_value_cansleep(reset_gpio, 0);
+   /* Delay time between end of reset to I2C active */
+   usleep_range(200, 250);
+   }
+
+   return 0;
+}
+
 static int tvp5150_probe(struct i2c_client *c,
 const struct i2c_device_id *id)
 {
@@ -1209,6 +1240,10 @@ static int tvp5150_probe(struct i2c_client *c,
 I2C_FUNC_SMBUS_READ_BYTE | I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
return -EIO;
 
+   res = tvp5150_init(c);
+   if (res)
+   return res;
+
core = devm_kzalloc(>dev, sizeof(*core), GFP_KERNEL);
if (!core)
return -ENOMEM;
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 09/10] [media] tvp5150: Initialize the chip on probe

2016-01-04 Thread Javier Martinez Canillas
After power-up, the tvp5150 decoder is in a unknown state until the
RESETB pin is driven LOW which reset all the registers and restarts
the chip's internal state machine.

The init sequence has some timing constraints and the RESETB signal
can only be used if the PDN (Power-down) pin is first released.

So, the initialization sequence is as follows:

1- PDN (active-low) is driven HIGH so the chip is power-up
2- A 20 ms delay is needed before sending a RESETB (active-low) signal.
3- The RESETB pulse duration is 500 ns.
4- A 200 us delay is needed for the I2C client to be active after reset.

This patch used as a reference the logic in the IGEPv2 board file from
the ISEE 2.6.37 vendor tree.

Signed-off-by: Javier Martinez Canillas 
---

 drivers/media/i2c/tvp5150.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index caac96a577f8..fed89a811ab7 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -5,6 +5,7 @@
  * This code is placed under the terms of the GNU General Public License v2
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -1197,6 +1198,36 @@ static int tvp5150_detect_version(struct tvp5150 *core)
return 0;
 }
 
+static inline int tvp5150_init(struct i2c_client *c)
+{
+   struct gpio_desc *pdn_gpio;
+   struct gpio_desc *reset_gpio;
+
+   pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", 
GPIOD_OUT_HIGH);
+   if (IS_ERR(pdn_gpio))
+   return PTR_ERR(pdn_gpio);
+
+   if (pdn_gpio) {
+   gpiod_set_value_cansleep(pdn_gpio, 0);
+   /* Delay time between power supplies active and reset */
+   msleep(20);
+   }
+
+   reset_gpio = devm_gpiod_get_optional(>dev, "reset", GPIOD_OUT_HIGH);
+   if (IS_ERR(reset_gpio))
+   return PTR_ERR(reset_gpio);
+
+   if (reset_gpio) {
+   /* RESETB pulse duration */
+   ndelay(500);
+   gpiod_set_value_cansleep(reset_gpio, 0);
+   /* Delay time between end of reset to I2C active */
+   usleep_range(200, 250);
+   }
+
+   return 0;
+}
+
 static int tvp5150_probe(struct i2c_client *c,
 const struct i2c_device_id *id)
 {
@@ -1209,6 +1240,10 @@ static int tvp5150_probe(struct i2c_client *c,
 I2C_FUNC_SMBUS_READ_BYTE | I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
return -EIO;
 
+   res = tvp5150_init(c);
+   if (res)
+   return res;
+
core = devm_kzalloc(>dev, sizeof(*core), GFP_KERNEL);
if (!core)
return -ENOMEM;
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 09/10] [media] tvp5150: Initialize the chip on probe

2016-01-04 Thread Javier Martinez Canillas
Hello,

On 01/04/2016 09:40 AM, kbuild test robot wrote:
> Hi Javier,
> 
> [auto build test ERROR on linuxtv-media/master]
> [also build test ERROR on v4.4-rc8 next-20151231]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improving the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Javier-Martinez-Canillas/tvp5150-add-MC-and-DT-support/20160104-203224
> base:   git://linuxtv.org/media_tree.git master
> config: x86_64-randconfig-x008-01040711 (attached as .config)
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>drivers/media/i2c/tvp5150.c: In function 'tvp5150_init':
>>> drivers/media/i2c/tvp5150.c:1206:13: error: implicit declaration of 
>>> function 'devm_gpiod_get_optional' [-Werror=implicit-function-declaration]
>  pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", GPIOD_OUT_HIGH);
> ^
>>> drivers/media/i2c/tvp5150.c:1206:59: error: 'GPIOD_OUT_HIGH' undeclared 
>>> (first use in this function)
>  pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", GPIOD_OUT_HIGH);
>   ^
>drivers/media/i2c/tvp5150.c:1206:59: note: each undeclared identifier is 
> reported only once for each function it appears in
>>> drivers/media/i2c/tvp5150.c:1211:3: error: implicit declaration of function 
>>> 'gpiod_set_value_cansleep' [-Werror=implicit-function-declaration]
>   gpiod_set_value_cansleep(pdn_gpio, 0);
>

Sigh, it's caused by a missing include for the  header.

Thanks for reporting, I'll wait a couple of days to see if I get more feedback
and then post a v2 fixing this.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 09/10] [media] tvp5150: Initialize the chip on probe

2016-01-04 Thread kbuild test robot
Hi Javier,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.4-rc8 next-20151231]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Javier-Martinez-Canillas/tvp5150-add-MC-and-DT-support/20160104-203224
base:   git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-x008-01040711 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/media/i2c/tvp5150.c: In function 'tvp5150_init':
>> drivers/media/i2c/tvp5150.c:1206:13: error: implicit declaration of function 
>> 'devm_gpiod_get_optional' [-Werror=implicit-function-declaration]
 pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", GPIOD_OUT_HIGH);
^
>> drivers/media/i2c/tvp5150.c:1206:59: error: 'GPIOD_OUT_HIGH' undeclared 
>> (first use in this function)
 pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", GPIOD_OUT_HIGH);
  ^
   drivers/media/i2c/tvp5150.c:1206:59: note: each undeclared identifier is 
reported only once for each function it appears in
>> drivers/media/i2c/tvp5150.c:1211:3: error: implicit declaration of function 
>> 'gpiod_set_value_cansleep' [-Werror=implicit-function-declaration]
  gpiod_set_value_cansleep(pdn_gpio, 0);
  ^
   cc1: some warnings being treated as errors

vim +/devm_gpiod_get_optional +1206 drivers/media/i2c/tvp5150.c

  1200  
  1201  static inline int tvp5150_init(struct i2c_client *c)
  1202  {
  1203  struct gpio_desc *pdn_gpio;
  1204  struct gpio_desc *reset_gpio;
  1205  
> 1206  pdn_gpio = devm_gpiod_get_optional(>dev, "powerdown", 
> GPIOD_OUT_HIGH);
  1207  if (IS_ERR(pdn_gpio))
  1208  return PTR_ERR(pdn_gpio);
  1209  
  1210  if (pdn_gpio) {
> 1211  gpiod_set_value_cansleep(pdn_gpio, 0);
  1212  /* Delay time between power supplies active and reset */
  1213  msleep(20);
  1214  }

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data