Re: [PATCH] tda10071: make sure both tuner and demod i2c addresses are specified

2013-01-04 Thread Michael Krufky
On Tue, Jan 1, 2013 at 10:39 AM, Antti Palosaari cr...@iki.fi wrote:
 On 12/17/2012 03:12 AM, Michael Krufky wrote:

 display an error message if either tuner_i2c_addr or demod_i2c_addr
 are not specified in the tda10071_config structure


 Nack.

 I don't see it necessary at all to check correctness of driver configuration
 values explicitly like that. Those are values which user cannot never change
 and when driver developer pass wrong values he will find it out very soon as
 hardware is not working.

 Adding comments to configuration struct which says possible values is
 advisable and enough IMHO.

 Maybe you should open own topic for discussion if you really would like to
 add this kind of checks. It is not tda10071 driver specific change, it
 affects about all drivers.


Antti,

I don't believe that your NACK has any technical merit- we are trying
to enforce good programming practice here, and when we find that a
piece of silicon has a configurable option, we like to expose that
option.  We try not to make default values unless its necessary, but
when there are differing i2c addresses, it is always best to have that
listed explicitly in the configuration structures.  It's called
self-documenting code- It's the way we try to write code in the Linux
kernel.

In this case, there was one previous user of the tda10071 driver and
it was a small change to add both tuner and demod i2c addresses to the
configuration structure.

We did speak about this on IRC on the sixteenth of December, and you
seemed OK with the idea:

[18:21] mkrufky dunno if you saw i asked earlier, but if you
want to review the patches, i can include your credentials on the pull
request
[18:21] mkrufky tda10071 patch:
http://git.linuxtv.org/mkrufky/hauppauge.git/commitdiff/4244d8155324263df37a8b2e067e7b9b81faec29
[18:22] mkrufky cx23885 patch:
http://git.linuxtv.org/mkrufky/hauppauge.git/commitdiff/eb658993b7a834911a3e8aabc7729b2505eef7d0
[18:22] mkrufky and i'll add another patch error out if the
tuner_i2c_addr is not specified, as devin recommended
[18:22] mkrufky (and also change the other tda10071 user to specify
0x14 for tuner_i2c_addr)
[18:23] crope Acked-by: Antti Palosaari cr...@iki.fi
[18:23] crope Reviewed-by: Antti Palosaari cr...@iki.fi
[18:23] mkrufky cool, thanks :-)
[18:24] crope I think 14 is the default, it is hard to understand
why ever one wants to use some other address in case of it is
controlled by demod :] but feel free to do that
[18:24] mkrufky ok
[18:25] mkrufky thats why i'll do it in a separate patch -- no need
to confuse a board addition with other cleanups
[18:25] crope eg cx24116/cx24118 does not specify tuner address at
all (as far as I see) = I think it is hard coded even to firmware
[18:26] mkrufky yea ur right

...the tuner and demod addresses can be changed, so we represent that
in the configuration structure.  I see no reason why you should have
any complaints about this patch.

Best regards,

Mike


 regards
 Antti




 Signed-off-by: Michael Krufky mkru...@linuxtv.org
 ---
   drivers/media/dvb-frontends/tda10071.c  |   18 +++---
   drivers/media/dvb-frontends/tda10071.h  |4 ++--
   drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
   drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
   4 files changed, 20 insertions(+), 7 deletions(-)

 diff --git a/drivers/media/dvb-frontends/tda10071.c
 b/drivers/media/dvb-frontends/tda10071.c
 index 7103629..02f9234 100644
 --- a/drivers/media/dvb-frontends/tda10071.c
 +++ b/drivers/media/dvb-frontends/tda10071.c
 @@ -30,7 +30,7 @@ static int tda10071_wr_regs(struct tda10071_priv *priv,
 u8 reg, u8 *val,
 u8 buf[len+1];
 struct i2c_msg msg[1] = {
 {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = 0,
 .len = sizeof(buf),
 .buf = buf,
 @@ -59,12 +59,12 @@ static int tda10071_rd_regs(struct tda10071_priv
 *priv, u8 reg, u8 *val,
 u8 buf[len];
 struct i2c_msg msg[2] = {
 {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = 0,
 .len = 1,
 .buf = reg,
 }, {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = I2C_M_RD,
 .len = sizeof(buf),
 .buf = buf,
 @@ -1202,6 +1202,18 @@ struct dvb_frontend *tda10071_attach(const struct
 tda10071_config *config,
 goto error;
 }

 +   /* make sure demod i2c address is specified */
 +   if (!config-demod_i2c_addr) {
 +   dev_dbg(i2c-dev, %s: invalid demod i2c address!\n,
 __func__);
 +   goto error;
 +   }
 +
 +   /* make sure tuner i2c address 

Re: [PATCH] tda10071: make sure both tuner and demod i2c addresses are specified

2013-01-04 Thread Antti Palosaari

On 01/04/2013 03:33 PM, Michael Krufky wrote:

On Tue, Jan 1, 2013 at 10:39 AM, Antti Palosaari cr...@iki.fi wrote:

On 12/17/2012 03:12 AM, Michael Krufky wrote:


display an error message if either tuner_i2c_addr or demod_i2c_addr
are not specified in the tda10071_config structure



Nack.

I don't see it necessary at all to check correctness of driver configuration
values explicitly like that. Those are values which user cannot never change
and when driver developer pass wrong values he will find it out very soon as
hardware is not working.

Adding comments to configuration struct which says possible values is
advisable and enough IMHO.

Maybe you should open own topic for discussion if you really would like to
add this kind of checks. It is not tda10071 driver specific change, it
affects about all drivers.



Antti,

I don't believe that your NACK has any technical merit- we are trying
to enforce good programming practice here, and when we find that a
piece of silicon has a configurable option, we like to expose that
option.  We try not to make default values unless its necessary, but
when there are differing i2c addresses, it is always best to have that
listed explicitly in the configuration structures.  It's called
self-documenting code- It's the way we try to write code in the Linux
kernel.


I am against checking explicitly that kind of I2C address correctness! 
Address is set compile time and cannot change during device operation. 
It is just stupid check, adding few lines more code to driver complexity 
and making bloated binary without a good reason.


It is *logging without a good reason/need*. Someone could put BOG_ON or 
WARN_ON in such places, but really, I would like to leave it totally 
without and let it fail for I/O error. My opinion is we should think if 
we really need to add all kind of logging, if not, do not log.



I could maybe agree that situation if we make general rule to add as 
much as checking for all the configuration parameters for all the 
drivers. I don't like it was added to my driver, whilst about 100% other 
drivers are just without currently.




In this case, there was one previous user of the tda10071 driver and
it was a small change to add both tuner and demod i2c addresses to the
configuration structure.


I was (and I am) fine for those two patches.


We did speak about this on IRC on the sixteenth of December, and you
seemed OK with the idea:

[18:21] mkrufky dunno if you saw i asked earlier, but if you
want to review the patches, i can include your credentials on the pull
request
[18:21] mkrufky tda10071 patch:
http://git.linuxtv.org/mkrufky/hauppauge.git/commitdiff/4244d8155324263df37a8b2e067e7b9b81faec29
[18:22] mkrufky cx23885 patch:
http://git.linuxtv.org/mkrufky/hauppauge.git/commitdiff/eb658993b7a834911a3e8aabc7729b2505eef7d0
[18:22] mkrufky and i'll add another patch error out if the
tuner_i2c_addr is not specified, as devin recommended
[18:22] mkrufky (and also change the other tda10071 user to specify
0x14 for tuner_i2c_addr)
[18:23] crope Acked-by: Antti Palosaari cr...@iki.fi
[18:23] crope Reviewed-by: Antti Palosaari cr...@iki.fi
[18:23] mkrufky cool, thanks :-)
[18:24] crope I think 14 is the default, it is hard to understand
why ever one wants to use some other address in case of it is
controlled by demod :] but feel free to do that
[18:24] mkrufky ok
[18:25] mkrufky thats why i'll do it in a separate patch -- no need
to confuse a board addition with other cleanups
[18:25] crope eg cx24116/cx24118 does not specify tuner address at
all (as far as I see) = I think it is hard coded even to firmware
[18:26] mkrufky yea ur right

...the tuner and demod addresses can be changed, so we represent that
in the configuration structure.  I see no reason why you should have
any complaints about this patch.

Best regards,

Mike



regards
Antti


Antti







Signed-off-by: Michael Krufky mkru...@linuxtv.org
---
   drivers/media/dvb-frontends/tda10071.c  |   18 +++---
   drivers/media/dvb-frontends/tda10071.h  |4 ++--
   drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
   drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
   4 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/media/dvb-frontends/tda10071.c
b/drivers/media/dvb-frontends/tda10071.c
index 7103629..02f9234 100644
--- a/drivers/media/dvb-frontends/tda10071.c
+++ b/drivers/media/dvb-frontends/tda10071.c
@@ -30,7 +30,7 @@ static int tda10071_wr_regs(struct tda10071_priv *priv,
u8 reg, u8 *val,
 u8 buf[len+1];
 struct i2c_msg msg[1] = {
 {
-   .addr = priv-cfg.i2c_address,
+   .addr = priv-cfg.demod_i2c_addr,
 .flags = 0,
 .len = sizeof(buf),
 .buf = buf,
@@ -59,12 +59,12 @@ static int tda10071_rd_regs(struct tda10071_priv
*priv, u8 reg, u8 *val,
 u8 buf[len];
 struct i2c_msg msg[2] = {

Re: [PATCH] tda10071: make sure both tuner and demod i2c addresses are specified

2013-01-01 Thread Antti Palosaari

On 12/17/2012 03:12 AM, Michael Krufky wrote:

display an error message if either tuner_i2c_addr or demod_i2c_addr
are not specified in the tda10071_config structure


Nack.

I don't see it necessary at all to check correctness of driver 
configuration values explicitly like that. Those are values which user 
cannot never change and when driver developer pass wrong values he will 
find it out very soon as hardware is not working.


Adding comments to configuration struct which says possible values is 
advisable and enough IMHO.


Maybe you should open own topic for discussion if you really would like 
to add this kind of checks. It is not tda10071 driver specific change, 
it affects about all drivers.



regards
Antti




Signed-off-by: Michael Krufky mkru...@linuxtv.org
---
  drivers/media/dvb-frontends/tda10071.c  |   18 +++---
  drivers/media/dvb-frontends/tda10071.h  |4 ++--
  drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
  drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
  4 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/media/dvb-frontends/tda10071.c 
b/drivers/media/dvb-frontends/tda10071.c
index 7103629..02f9234 100644
--- a/drivers/media/dvb-frontends/tda10071.c
+++ b/drivers/media/dvb-frontends/tda10071.c
@@ -30,7 +30,7 @@ static int tda10071_wr_regs(struct tda10071_priv *priv, u8 
reg, u8 *val,
u8 buf[len+1];
struct i2c_msg msg[1] = {
{
-   .addr = priv-cfg.i2c_address,
+   .addr = priv-cfg.demod_i2c_addr,
.flags = 0,
.len = sizeof(buf),
.buf = buf,
@@ -59,12 +59,12 @@ static int tda10071_rd_regs(struct tda10071_priv *priv, u8 
reg, u8 *val,
u8 buf[len];
struct i2c_msg msg[2] = {
{
-   .addr = priv-cfg.i2c_address,
+   .addr = priv-cfg.demod_i2c_addr,
.flags = 0,
.len = 1,
.buf = reg,
}, {
-   .addr = priv-cfg.i2c_address,
+   .addr = priv-cfg.demod_i2c_addr,
.flags = I2C_M_RD,
.len = sizeof(buf),
.buf = buf,
@@ -1202,6 +1202,18 @@ struct dvb_frontend *tda10071_attach(const struct 
tda10071_config *config,
goto error;
}

+   /* make sure demod i2c address is specified */
+   if (!config-demod_i2c_addr) {
+   dev_dbg(i2c-dev, %s: invalid demod i2c address!\n, 
__func__);
+   goto error;
+   }
+
+   /* make sure tuner i2c address is specified */
+   if (!config-tuner_i2c_addr) {
+   dev_dbg(i2c-dev, %s: invalid tuner i2c address!\n, 
__func__);
+   goto error;
+   }
+
/* setup the priv */
priv-i2c = i2c;
memcpy(priv-cfg, config, sizeof(struct tda10071_config));
diff --git a/drivers/media/dvb-frontends/tda10071.h 
b/drivers/media/dvb-frontends/tda10071.h
index a20d5c4..bff1c38 100644
--- a/drivers/media/dvb-frontends/tda10071.h
+++ b/drivers/media/dvb-frontends/tda10071.h
@@ -28,10 +28,10 @@ struct tda10071_config {
 * Default: none, must set
 * Values: 0x55,
 */
-   u8 i2c_address;
+   u8 demod_i2c_addr;

/* Tuner I2C address.
-* Default: 0x14
+* Default: none, must set
 * Values: 0x14, 0x54, ...
 */
u8 tuner_i2c_addr;
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c 
b/drivers/media/pci/cx23885/cx23885-dvb.c
index cf84c53..a1aae56 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -662,7 +662,7 @@ static struct mt2063_config terratec_mt2063_config[] = {
  };

  static const struct tda10071_config hauppauge_tda10071_config = {
-   .i2c_address = 0x05,
+   .demod_i2c_addr = 0x05,
.tuner_i2c_addr = 0x54,
.i2c_wr_max = 64,
.ts_mode = TDA10071_TS_SERIAL,
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c 
b/drivers/media/usb/em28xx/em28xx-dvb.c
index 63f2e70..e800881 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -714,7 +714,8 @@ static struct tda18271_config 
em28xx_cxd2820r_tda18271_config = {
  };

  static const struct tda10071_config em28xx_tda10071_config = {
-   .i2c_address = 0x55, /* (0xaa  1) */
+   .demod_i2c_addr = 0x55, /* (0xaa  1) */
+   .tuner_i2c_addr = 0x14,
.i2c_wr_max = 64,
.ts_mode = TDA10071_TS_SERIAL,
.spec_inv = 0,




--
http://palosaari.fi/
--
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


Re: [PATCH] tda10071: make sure both tuner and demod i2c addresses are specified

2012-12-21 Thread Michael Krufky
rebased against today's tip, as per your request :-)


pwclient update -s 'superseded' 15923
pwclient update -s 'superseded' 15930


The following changes since commit 1b5901331ff3af4bdc1b998a056a248c9924e2d1:

  [media] exynos-gsc: modify number of output/capture buffers
(2012-12-21 10:26:44 -0200)

are available in the git repository at:

  git://git.linuxtv.org/mkrufky/tuners tda10071

for you to fetch changes up to d562d77132333f0a7b1d704edc992a092d6d6bbe:

  tda10071: make sure both tuner and demod i2c addresses are specified
(2012-12-21 08:29:49 -0500)

Cheers,

Mike

On Fri, Dec 21, 2012 at 8:32 AM, Michael Krufky mkru...@kernellabs.com wrote:
 rebased against today's tip, as per your request :-)


 pwclient update -s 'superseded' 15923
 pwclient update -s 'superseded' 15930


 The following changes since commit 1b5901331ff3af4bdc1b998a056a248c9924e2d1:

   [media] exynos-gsc: modify number of output/capture buffers
 (2012-12-21 10:26:44 -0200)

 are available in the git repository at:

   git://git.linuxtv.org/mkrufky/tuners tda10071

 for you to fetch changes up to d562d77132333f0a7b1d704edc992a092d6d6bbe:

   tda10071: make sure both tuner and demod i2c addresses are specified
 (2012-12-21 08:29:49 -0500)

 
 Michael Krufky (1):
   tda10071: make sure both tuner and demod i2c addresses are specified

  drivers/media/dvb-frontends/tda10071.c  |   18 +++---
  drivers/media/dvb-frontends/tda10071.h  |4 ++--
  drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
  drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
  4 files changed, 20 insertions(+), 7 deletions(-)

 Cheers,

 Mike

 On Mon, Dec 17, 2012 at 10:10 AM, Michael Krufky mkru...@linuxtv.org wrote:
 As discussed on irc, the following pwclient commands should update the
 status of the patches in patchwork to correspond with this merge
 request:

 pwclient update -s 'superseded' 15923
 pwclient update -s 'accepted' 15930


 Cheers,

 Mike

 On Mon, Dec 17, 2012 at 10:09 AM, Michael Krufky mkru...@linuxtv.org wrote:
 Mauro,

 Please merge:

 The following changes since commit 4c8e64232d4a71e68d68b9093506966c0244a526:

   cx23885: add basic DVB-S2 support for Hauppauge HVR-4400 (2012-12-16
 12:27:25 -0500)

 are available in the git repository at:

   git://linuxtv.org/mkrufky/tuners tda10071

 for you to fetch changes up to 326e65af0104faf8a243e534eb8bfdb35b73f4ed:

   tda10071: make sure both tuner and demod i2c addresses are specified
 (2012-12-16 18:05:02 -0500)

 
 Michael Krufky (1):
   tda10071: make sure both tuner and demod i2c addresses are specified

  drivers/media/dvb-frontends/tda10071.c  |   18 +++---
  drivers/media/dvb-frontends/tda10071.h  |4 ++--
  drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
  drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
  4 files changed, 20 insertions(+), 7 deletions(-)

 Cheers,

 Mike

 On Sun, Dec 16, 2012 at 8:12 PM, Michael Krufky mkru...@linuxtv.org wrote:
 display an error message if either tuner_i2c_addr or demod_i2c_addr
 are not specified in the tda10071_config structure

 Signed-off-by: Michael Krufky mkru...@linuxtv.org
 ---
  drivers/media/dvb-frontends/tda10071.c  |   18 +++---
  drivers/media/dvb-frontends/tda10071.h  |4 ++--
  drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
  drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
  4 files changed, 20 insertions(+), 7 deletions(-)

 diff --git a/drivers/media/dvb-frontends/tda10071.c 
 b/drivers/media/dvb-frontends/tda10071.c
 index 7103629..02f9234 100644
 --- a/drivers/media/dvb-frontends/tda10071.c
 +++ b/drivers/media/dvb-frontends/tda10071.c
 @@ -30,7 +30,7 @@ static int tda10071_wr_regs(struct tda10071_priv *priv, 
 u8 reg, u8 *val,
 u8 buf[len+1];
 struct i2c_msg msg[1] = {
 {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = 0,
 .len = sizeof(buf),
 .buf = buf,
 @@ -59,12 +59,12 @@ static int tda10071_rd_regs(struct tda10071_priv 
 *priv, u8 reg, u8 *val,
 u8 buf[len];
 struct i2c_msg msg[2] = {
 {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = 0,
 .len = 1,
 .buf = reg,
 }, {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = I2C_M_RD,
 .len = sizeof(buf),
 .buf = buf,
 @@ -1202,6 +1202,18 @@ struct dvb_frontend *tda10071_attach(const struct 
 tda10071_config *config,
 goto error;
 }

 +   /* make sure demod 

Re: [PATCH] tda10071: make sure both tuner and demod i2c addresses are specified

2012-12-21 Thread Michael Krufky
rebased against today's tip, as per your request :-)


pwclient update -s 'superseded' 15923
pwclient update -s 'superseded' 15930


The following changes since commit 1b5901331ff3af4bdc1b998a056a248c9924e2d1:

  [media] exynos-gsc: modify number of output/capture buffers
(2012-12-21 10:26:44 -0200)

are available in the git repository at:

  git://git.linuxtv.org/mkrufky/tuners tda10071

for you to fetch changes up to d562d77132333f0a7b1d704edc992a092d6d6bbe:

  tda10071: make sure both tuner and demod i2c addresses are specified
(2012-12-21 08:29:49 -0500)


Michael Krufky (1):
  tda10071: make sure both tuner and demod i2c addresses are specified

 drivers/media/dvb-frontends/tda10071.c  |   18 +++---
 drivers/media/dvb-frontends/tda10071.h  |4 ++--
 drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
 drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
 4 files changed, 20 insertions(+), 7 deletions(-)

Cheers,

Mike

On Mon, Dec 17, 2012 at 10:10 AM, Michael Krufky mkru...@linuxtv.org wrote:
 As discussed on irc, the following pwclient commands should update the
 status of the patches in patchwork to correspond with this merge
 request:

 pwclient update -s 'superseded' 15923
 pwclient update -s 'accepted' 15930


 Cheers,

 Mike

 On Mon, Dec 17, 2012 at 10:09 AM, Michael Krufky mkru...@linuxtv.org wrote:
 Mauro,

 Please merge:

 The following changes since commit 4c8e64232d4a71e68d68b9093506966c0244a526:

   cx23885: add basic DVB-S2 support for Hauppauge HVR-4400 (2012-12-16
 12:27:25 -0500)

 are available in the git repository at:

   git://linuxtv.org/mkrufky/tuners tda10071

 for you to fetch changes up to 326e65af0104faf8a243e534eb8bfdb35b73f4ed:

   tda10071: make sure both tuner and demod i2c addresses are specified
 (2012-12-16 18:05:02 -0500)

 
 Michael Krufky (1):
   tda10071: make sure both tuner and demod i2c addresses are specified

  drivers/media/dvb-frontends/tda10071.c  |   18 +++---
  drivers/media/dvb-frontends/tda10071.h  |4 ++--
  drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
  drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
  4 files changed, 20 insertions(+), 7 deletions(-)

 Cheers,

 Mike

 On Sun, Dec 16, 2012 at 8:12 PM, Michael Krufky mkru...@linuxtv.org wrote:
 display an error message if either tuner_i2c_addr or demod_i2c_addr
 are not specified in the tda10071_config structure

 Signed-off-by: Michael Krufky mkru...@linuxtv.org
 ---
  drivers/media/dvb-frontends/tda10071.c  |   18 +++---
  drivers/media/dvb-frontends/tda10071.h  |4 ++--
  drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
  drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
  4 files changed, 20 insertions(+), 7 deletions(-)

 diff --git a/drivers/media/dvb-frontends/tda10071.c 
 b/drivers/media/dvb-frontends/tda10071.c
 index 7103629..02f9234 100644
 --- a/drivers/media/dvb-frontends/tda10071.c
 +++ b/drivers/media/dvb-frontends/tda10071.c
 @@ -30,7 +30,7 @@ static int tda10071_wr_regs(struct tda10071_priv *priv, 
 u8 reg, u8 *val,
 u8 buf[len+1];
 struct i2c_msg msg[1] = {
 {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = 0,
 .len = sizeof(buf),
 .buf = buf,
 @@ -59,12 +59,12 @@ static int tda10071_rd_regs(struct tda10071_priv *priv, 
 u8 reg, u8 *val,
 u8 buf[len];
 struct i2c_msg msg[2] = {
 {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = 0,
 .len = 1,
 .buf = reg,
 }, {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = I2C_M_RD,
 .len = sizeof(buf),
 .buf = buf,
 @@ -1202,6 +1202,18 @@ struct dvb_frontend *tda10071_attach(const struct 
 tda10071_config *config,
 goto error;
 }

 +   /* make sure demod i2c address is specified */
 +   if (!config-demod_i2c_addr) {
 +   dev_dbg(i2c-dev, %s: invalid demod i2c address!\n, 
 __func__);
 +   goto error;
 +   }
 +
 +   /* make sure tuner i2c address is specified */
 +   if (!config-tuner_i2c_addr) {
 +   dev_dbg(i2c-dev, %s: invalid tuner i2c address!\n, 
 __func__);
 +   goto error;
 +   }
 +
 /* setup the priv */
 priv-i2c = i2c;
 memcpy(priv-cfg, config, sizeof(struct tda10071_config));
 diff --git a/drivers/media/dvb-frontends/tda10071.h 
 b/drivers/media/dvb-frontends/tda10071.h
 index a20d5c4..bff1c38 100644
 --- 

Re: [PATCH] tda10071: make sure both tuner and demod i2c addresses are specified

2012-12-17 Thread Michael Krufky
Mauro,

Please merge:

The following changes since commit 4c8e64232d4a71e68d68b9093506966c0244a526:

  cx23885: add basic DVB-S2 support for Hauppauge HVR-4400 (2012-12-16
12:27:25 -0500)

are available in the git repository at:

  git://linuxtv.org/mkrufky/tuners tda10071

for you to fetch changes up to 326e65af0104faf8a243e534eb8bfdb35b73f4ed:

  tda10071: make sure both tuner and demod i2c addresses are specified
(2012-12-16 18:05:02 -0500)


Michael Krufky (1):
  tda10071: make sure both tuner and demod i2c addresses are specified

 drivers/media/dvb-frontends/tda10071.c  |   18 +++---
 drivers/media/dvb-frontends/tda10071.h  |4 ++--
 drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
 drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
 4 files changed, 20 insertions(+), 7 deletions(-)

Cheers,

Mike

On Sun, Dec 16, 2012 at 8:12 PM, Michael Krufky mkru...@linuxtv.org wrote:
 display an error message if either tuner_i2c_addr or demod_i2c_addr
 are not specified in the tda10071_config structure

 Signed-off-by: Michael Krufky mkru...@linuxtv.org
 ---
  drivers/media/dvb-frontends/tda10071.c  |   18 +++---
  drivers/media/dvb-frontends/tda10071.h  |4 ++--
  drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
  drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
  4 files changed, 20 insertions(+), 7 deletions(-)

 diff --git a/drivers/media/dvb-frontends/tda10071.c 
 b/drivers/media/dvb-frontends/tda10071.c
 index 7103629..02f9234 100644
 --- a/drivers/media/dvb-frontends/tda10071.c
 +++ b/drivers/media/dvb-frontends/tda10071.c
 @@ -30,7 +30,7 @@ static int tda10071_wr_regs(struct tda10071_priv *priv, u8 
 reg, u8 *val,
 u8 buf[len+1];
 struct i2c_msg msg[1] = {
 {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = 0,
 .len = sizeof(buf),
 .buf = buf,
 @@ -59,12 +59,12 @@ static int tda10071_rd_regs(struct tda10071_priv *priv, 
 u8 reg, u8 *val,
 u8 buf[len];
 struct i2c_msg msg[2] = {
 {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = 0,
 .len = 1,
 .buf = reg,
 }, {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = I2C_M_RD,
 .len = sizeof(buf),
 .buf = buf,
 @@ -1202,6 +1202,18 @@ struct dvb_frontend *tda10071_attach(const struct 
 tda10071_config *config,
 goto error;
 }

 +   /* make sure demod i2c address is specified */
 +   if (!config-demod_i2c_addr) {
 +   dev_dbg(i2c-dev, %s: invalid demod i2c address!\n, 
 __func__);
 +   goto error;
 +   }
 +
 +   /* make sure tuner i2c address is specified */
 +   if (!config-tuner_i2c_addr) {
 +   dev_dbg(i2c-dev, %s: invalid tuner i2c address!\n, 
 __func__);
 +   goto error;
 +   }
 +
 /* setup the priv */
 priv-i2c = i2c;
 memcpy(priv-cfg, config, sizeof(struct tda10071_config));
 diff --git a/drivers/media/dvb-frontends/tda10071.h 
 b/drivers/media/dvb-frontends/tda10071.h
 index a20d5c4..bff1c38 100644
 --- a/drivers/media/dvb-frontends/tda10071.h
 +++ b/drivers/media/dvb-frontends/tda10071.h
 @@ -28,10 +28,10 @@ struct tda10071_config {
  * Default: none, must set
  * Values: 0x55,
  */
 -   u8 i2c_address;
 +   u8 demod_i2c_addr;

 /* Tuner I2C address.
 -* Default: 0x14
 +* Default: none, must set
  * Values: 0x14, 0x54, ...
  */
 u8 tuner_i2c_addr;
 diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c 
 b/drivers/media/pci/cx23885/cx23885-dvb.c
 index cf84c53..a1aae56 100644
 --- a/drivers/media/pci/cx23885/cx23885-dvb.c
 +++ b/drivers/media/pci/cx23885/cx23885-dvb.c
 @@ -662,7 +662,7 @@ static struct mt2063_config terratec_mt2063_config[] = {
  };

  static const struct tda10071_config hauppauge_tda10071_config = {
 -   .i2c_address = 0x05,
 +   .demod_i2c_addr = 0x05,
 .tuner_i2c_addr = 0x54,
 .i2c_wr_max = 64,
 .ts_mode = TDA10071_TS_SERIAL,
 diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c 
 b/drivers/media/usb/em28xx/em28xx-dvb.c
 index 63f2e70..e800881 100644
 --- a/drivers/media/usb/em28xx/em28xx-dvb.c
 +++ b/drivers/media/usb/em28xx/em28xx-dvb.c
 @@ -714,7 +714,8 @@ static struct tda18271_config 
 em28xx_cxd2820r_tda18271_config = {
  };

  static const struct tda10071_config em28xx_tda10071_config = {
 -   .i2c_address = 0x55, /* (0xaa  1) */
 +   .demod_i2c_addr = 0x55, /* (0xaa  1) */
 +   

Re: [PATCH] tda10071: make sure both tuner and demod i2c addresses are specified

2012-12-17 Thread Michael Krufky
As discussed on irc, the following pwclient commands should update the
status of the patches in patchwork to correspond with this merge
request:

pwclient update -s 'superseded' 15923
pwclient update -s 'accepted' 15930


Cheers,

Mike

On Mon, Dec 17, 2012 at 10:09 AM, Michael Krufky mkru...@linuxtv.org wrote:
 Mauro,

 Please merge:

 The following changes since commit 4c8e64232d4a71e68d68b9093506966c0244a526:

   cx23885: add basic DVB-S2 support for Hauppauge HVR-4400 (2012-12-16
 12:27:25 -0500)

 are available in the git repository at:

   git://linuxtv.org/mkrufky/tuners tda10071

 for you to fetch changes up to 326e65af0104faf8a243e534eb8bfdb35b73f4ed:

   tda10071: make sure both tuner and demod i2c addresses are specified
 (2012-12-16 18:05:02 -0500)

 
 Michael Krufky (1):
   tda10071: make sure both tuner and demod i2c addresses are specified

  drivers/media/dvb-frontends/tda10071.c  |   18 +++---
  drivers/media/dvb-frontends/tda10071.h  |4 ++--
  drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
  drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
  4 files changed, 20 insertions(+), 7 deletions(-)

 Cheers,

 Mike

 On Sun, Dec 16, 2012 at 8:12 PM, Michael Krufky mkru...@linuxtv.org wrote:
 display an error message if either tuner_i2c_addr or demod_i2c_addr
 are not specified in the tda10071_config structure

 Signed-off-by: Michael Krufky mkru...@linuxtv.org
 ---
  drivers/media/dvb-frontends/tda10071.c  |   18 +++---
  drivers/media/dvb-frontends/tda10071.h  |4 ++--
  drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
  drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
  4 files changed, 20 insertions(+), 7 deletions(-)

 diff --git a/drivers/media/dvb-frontends/tda10071.c 
 b/drivers/media/dvb-frontends/tda10071.c
 index 7103629..02f9234 100644
 --- a/drivers/media/dvb-frontends/tda10071.c
 +++ b/drivers/media/dvb-frontends/tda10071.c
 @@ -30,7 +30,7 @@ static int tda10071_wr_regs(struct tda10071_priv *priv, u8 
 reg, u8 *val,
 u8 buf[len+1];
 struct i2c_msg msg[1] = {
 {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = 0,
 .len = sizeof(buf),
 .buf = buf,
 @@ -59,12 +59,12 @@ static int tda10071_rd_regs(struct tda10071_priv *priv, 
 u8 reg, u8 *val,
 u8 buf[len];
 struct i2c_msg msg[2] = {
 {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = 0,
 .len = 1,
 .buf = reg,
 }, {
 -   .addr = priv-cfg.i2c_address,
 +   .addr = priv-cfg.demod_i2c_addr,
 .flags = I2C_M_RD,
 .len = sizeof(buf),
 .buf = buf,
 @@ -1202,6 +1202,18 @@ struct dvb_frontend *tda10071_attach(const struct 
 tda10071_config *config,
 goto error;
 }

 +   /* make sure demod i2c address is specified */
 +   if (!config-demod_i2c_addr) {
 +   dev_dbg(i2c-dev, %s: invalid demod i2c address!\n, 
 __func__);
 +   goto error;
 +   }
 +
 +   /* make sure tuner i2c address is specified */
 +   if (!config-tuner_i2c_addr) {
 +   dev_dbg(i2c-dev, %s: invalid tuner i2c address!\n, 
 __func__);
 +   goto error;
 +   }
 +
 /* setup the priv */
 priv-i2c = i2c;
 memcpy(priv-cfg, config, sizeof(struct tda10071_config));
 diff --git a/drivers/media/dvb-frontends/tda10071.h 
 b/drivers/media/dvb-frontends/tda10071.h
 index a20d5c4..bff1c38 100644
 --- a/drivers/media/dvb-frontends/tda10071.h
 +++ b/drivers/media/dvb-frontends/tda10071.h
 @@ -28,10 +28,10 @@ struct tda10071_config {
  * Default: none, must set
  * Values: 0x55,
  */
 -   u8 i2c_address;
 +   u8 demod_i2c_addr;

 /* Tuner I2C address.
 -* Default: 0x14
 +* Default: none, must set
  * Values: 0x14, 0x54, ...
  */
 u8 tuner_i2c_addr;
 diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c 
 b/drivers/media/pci/cx23885/cx23885-dvb.c
 index cf84c53..a1aae56 100644
 --- a/drivers/media/pci/cx23885/cx23885-dvb.c
 +++ b/drivers/media/pci/cx23885/cx23885-dvb.c
 @@ -662,7 +662,7 @@ static struct mt2063_config terratec_mt2063_config[] = {
  };

  static const struct tda10071_config hauppauge_tda10071_config = {
 -   .i2c_address = 0x05,
 +   .demod_i2c_addr = 0x05,
 .tuner_i2c_addr = 0x54,
 .i2c_wr_max = 64,
 .ts_mode = TDA10071_TS_SERIAL,
 diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c 
 b/drivers/media/usb/em28xx/em28xx-dvb.c
 index 63f2e70..e800881 100644
 --- 

[PATCH] tda10071: make sure both tuner and demod i2c addresses are specified

2012-12-16 Thread Michael Krufky
display an error message if either tuner_i2c_addr or demod_i2c_addr
are not specified in the tda10071_config structure

Signed-off-by: Michael Krufky mkru...@linuxtv.org
---
 drivers/media/dvb-frontends/tda10071.c  |   18 +++---
 drivers/media/dvb-frontends/tda10071.h  |4 ++--
 drivers/media/pci/cx23885/cx23885-dvb.c |2 +-
 drivers/media/usb/em28xx/em28xx-dvb.c   |3 ++-
 4 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/media/dvb-frontends/tda10071.c 
b/drivers/media/dvb-frontends/tda10071.c
index 7103629..02f9234 100644
--- a/drivers/media/dvb-frontends/tda10071.c
+++ b/drivers/media/dvb-frontends/tda10071.c
@@ -30,7 +30,7 @@ static int tda10071_wr_regs(struct tda10071_priv *priv, u8 
reg, u8 *val,
u8 buf[len+1];
struct i2c_msg msg[1] = {
{
-   .addr = priv-cfg.i2c_address,
+   .addr = priv-cfg.demod_i2c_addr,
.flags = 0,
.len = sizeof(buf),
.buf = buf,
@@ -59,12 +59,12 @@ static int tda10071_rd_regs(struct tda10071_priv *priv, u8 
reg, u8 *val,
u8 buf[len];
struct i2c_msg msg[2] = {
{
-   .addr = priv-cfg.i2c_address,
+   .addr = priv-cfg.demod_i2c_addr,
.flags = 0,
.len = 1,
.buf = reg,
}, {
-   .addr = priv-cfg.i2c_address,
+   .addr = priv-cfg.demod_i2c_addr,
.flags = I2C_M_RD,
.len = sizeof(buf),
.buf = buf,
@@ -1202,6 +1202,18 @@ struct dvb_frontend *tda10071_attach(const struct 
tda10071_config *config,
goto error;
}
 
+   /* make sure demod i2c address is specified */
+   if (!config-demod_i2c_addr) {
+   dev_dbg(i2c-dev, %s: invalid demod i2c address!\n, 
__func__);
+   goto error;
+   }
+
+   /* make sure tuner i2c address is specified */
+   if (!config-tuner_i2c_addr) {
+   dev_dbg(i2c-dev, %s: invalid tuner i2c address!\n, 
__func__);
+   goto error;
+   }
+
/* setup the priv */
priv-i2c = i2c;
memcpy(priv-cfg, config, sizeof(struct tda10071_config));
diff --git a/drivers/media/dvb-frontends/tda10071.h 
b/drivers/media/dvb-frontends/tda10071.h
index a20d5c4..bff1c38 100644
--- a/drivers/media/dvb-frontends/tda10071.h
+++ b/drivers/media/dvb-frontends/tda10071.h
@@ -28,10 +28,10 @@ struct tda10071_config {
 * Default: none, must set
 * Values: 0x55,
 */
-   u8 i2c_address;
+   u8 demod_i2c_addr;
 
/* Tuner I2C address.
-* Default: 0x14
+* Default: none, must set
 * Values: 0x14, 0x54, ...
 */
u8 tuner_i2c_addr;
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c 
b/drivers/media/pci/cx23885/cx23885-dvb.c
index cf84c53..a1aae56 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -662,7 +662,7 @@ static struct mt2063_config terratec_mt2063_config[] = {
 };
 
 static const struct tda10071_config hauppauge_tda10071_config = {
-   .i2c_address = 0x05,
+   .demod_i2c_addr = 0x05,
.tuner_i2c_addr = 0x54,
.i2c_wr_max = 64,
.ts_mode = TDA10071_TS_SERIAL,
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c 
b/drivers/media/usb/em28xx/em28xx-dvb.c
index 63f2e70..e800881 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -714,7 +714,8 @@ static struct tda18271_config 
em28xx_cxd2820r_tda18271_config = {
 };
 
 static const struct tda10071_config em28xx_tda10071_config = {
-   .i2c_address = 0x55, /* (0xaa  1) */
+   .demod_i2c_addr = 0x55, /* (0xaa  1) */
+   .tuner_i2c_addr = 0x14,
.i2c_wr_max = 64,
.ts_mode = TDA10071_TS_SERIAL,
.spec_inv = 0,
-- 
1.7.10.4

--
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