| Message ID | fa93fecaa9d8e33f7d3b335872e9082893b775ae.1560338665.git.mchehab+samsung@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: tua6100: Remove some ugly defines | expand |
On Wed, Jun 12, 2019 at 08:25:03AM -0300, Mauro Carvalho Chehab wrote: > As reported by Stephen: > > > After merging the i2c tree, today's linux-next build (x86_64 allmodconfig) > > produced this warning: > > > > drivers/media/dvb-frontends/tua6100.c: In function 'tua6100_set_params': > > drivers/media/dvb-frontends/tua6100.c:71: warning: "_P" redefined > > #define _P 32 > > > > In file included from include/acpi/platform/aclinux.h:54, > > from include/acpi/platform/acenv.h:152, > > from include/acpi/acpi.h:22, > > from include/linux/acpi.h:21, > > from include/linux/i2c.h:17, > > from drivers/media/dvb-frontends/tua6100.h:22, > > from drivers/media/dvb-frontends/tua6100.c:24: > > include/linux/ctype.h:14: note: this is the location of the previous definition > > #define _P 0x10 /* punct */ > > > > Exposed by commit > > > > 5213d7efc8ec ("i2c: acpi: export i2c_acpi_find_adapter_by_handle") > > > > Since that included <linux/acpi.h> from <linux/i2c.h> > > > > Originally introduced by commit > > > > 00be2e7c6415 ("V4L/DVB (4606): Add driver for TUA6100") > > > > The _P in <linux/ctype.h> has existed since before git. > > The addition of include <linux/ctype.h> at the I2C code caused a > breakage at the tua6100 driver. The reason is that the code there > used defines for 3 parameters used at the calculus for the > divide ratio. > > In thesis, those are board-dependent, but, as there's just one > driver using it (ttpci/budget-av), there was no need to make > the code more generic. While it sounds unlikely that this old > DVB-S frontend would ever be used on new projects, one might > some day come with a variant using a different configuration. So, > let's do the right thing and store those values at its private > struct. > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> Acked-by: Wolfram Sang <wsa@the-dreams.de>
On Wed, Jun 12, 2019 at 08:25:03AM -0300, Mauro Carvalho Chehab wrote: > As reported by Stephen: > > > After merging the i2c tree, today's linux-next build (x86_64 allmodconfig) > > produced this warning: > > > > drivers/media/dvb-frontends/tua6100.c: In function 'tua6100_set_params': > > drivers/media/dvb-frontends/tua6100.c:71: warning: "_P" redefined > > #define _P 32 > > > > In file included from include/acpi/platform/aclinux.h:54, > > from include/acpi/platform/acenv.h:152, > > from include/acpi/acpi.h:22, > > from include/linux/acpi.h:21, > > from include/linux/i2c.h:17, > > from drivers/media/dvb-frontends/tua6100.h:22, > > from drivers/media/dvb-frontends/tua6100.c:24: > > include/linux/ctype.h:14: note: this is the location of the previous definition > > #define _P 0x10 /* punct */ > > > > Exposed by commit > > > > 5213d7efc8ec ("i2c: acpi: export i2c_acpi_find_adapter_by_handle") > > > > Since that included <linux/acpi.h> from <linux/i2c.h> > > > > Originally introduced by commit > > > > 00be2e7c6415 ("V4L/DVB (4606): Add driver for TUA6100") > > > > The _P in <linux/ctype.h> has existed since before git. > > The addition of include <linux/ctype.h> at the I2C code caused a > breakage at the tua6100 driver. The reason is that the code there > used defines for 3 parameters used at the calculus for the > divide ratio. > > In thesis, those are board-dependent, but, as there's just one > driver using it (ttpci/budget-av), there was no need to make > the code more generic. While it sounds unlikely that this old > DVB-S frontend would ever be used on new projects, one might > some day come with a variant using a different configuration. So, > let's do the right thing and store those values at its private > struct. > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> I see there is a build-fix from davem now in linux-next. Shall I still apply this patch to my i2c tree once Mauro applied it to his?
On Wed, Jun 12, 2019 at 08:25:03AM -0300, Mauro Carvalho Chehab wrote: > As reported by Stephen: > > > After merging the i2c tree, today's linux-next build (x86_64 allmodconfig) > > produced this warning: > > > > drivers/media/dvb-frontends/tua6100.c: In function 'tua6100_set_params': > > drivers/media/dvb-frontends/tua6100.c:71: warning: "_P" redefined > > #define _P 32 > > > > In file included from include/acpi/platform/aclinux.h:54, > > from include/acpi/platform/acenv.h:152, > > from include/acpi/acpi.h:22, > > from include/linux/acpi.h:21, > > from include/linux/i2c.h:17, > > from drivers/media/dvb-frontends/tua6100.h:22, > > from drivers/media/dvb-frontends/tua6100.c:24: > > include/linux/ctype.h:14: note: this is the location of the previous definition > > #define _P 0x10 /* punct */ > > > > Exposed by commit > > > > 5213d7efc8ec ("i2c: acpi: export i2c_acpi_find_adapter_by_handle") > > > > Since that included <linux/acpi.h> from <linux/i2c.h> > > > > Originally introduced by commit > > > > 00be2e7c6415 ("V4L/DVB (4606): Add driver for TUA6100") > > > > The _P in <linux/ctype.h> has existed since before git. > > The addition of include <linux/ctype.h> at the I2C code caused a > breakage at the tua6100 driver. The reason is that the code there > used defines for 3 parameters used at the calculus for the > divide ratio. > > In thesis, those are board-dependent, but, as there's just one > driver using it (ttpci/budget-av), there was no need to make > the code more generic. While it sounds unlikely that this old > DVB-S frontend would ever be used on new projects, one might > some day come with a variant using a different configuration. So, > let's do the right thing and store those values at its private > struct. > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> Mauro, what do we do here? The merge window is open and I can't send my I2C pull request unless the above issue is fixed. I see two options: 1) This patch gets fast-tracked into the media-tree (or I can pick it into my I2C tree if you are fine with that) 2) I pick the intermediate fix from David Miller in linux-next into my tree, but I'd need your ack for this patch (can be given here): https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/drivers/media/dvb-frontends/tua6100.c?id=621ccc6cc5f8d6730b740d31d4818227866c93c9 Kind regards, Wolfram
diff --git a/drivers/media/dvb-frontends/tua6100.c b/drivers/media/dvb-frontends/tua6100.c index b233b7be0b84..2990eb3fd475 100644 --- a/drivers/media/dvb-frontends/tua6100.c +++ b/drivers/media/dvb-frontends/tua6100.c @@ -31,11 +31,24 @@ #include "tua6100.h" +/** + * struct tua6100_priv - tuner's private data + * + * @i2c_address: I2C address + * @i2c: pointer to struct i2c_adapter + * @frequency: tuned frequency + * @prescaler_div: divide ratio of the prescaler (32 or 64) + * @ref_div: reference frequency divider + * @ref_inp: reference frequency input (XTAL osc) + */ struct tua6100_priv { /* i2c details */ int i2c_address; struct i2c_adapter *i2c; u32 frequency; + int prescaler_div; + int ref_div; + u32 ref_inp; }; static void tua6100_release(struct dvb_frontend *fe) @@ -75,10 +88,6 @@ static int tua6100_set_params(struct dvb_frontend *fe) struct i2c_msg msg1 = { .addr = priv->i2c_address, .flags = 0, .buf = reg1, .len = 4 }; struct i2c_msg msg2 = { .addr = priv->i2c_address, .flags = 0, .buf = reg2, .len = 3 }; -#define _R 4 -#define _P 32 -#define _ri 4000000 - // setup register 0 if (c->frequency < 2000000) reg0[1] = 0x03; @@ -91,14 +100,14 @@ static int tua6100_set_params(struct dvb_frontend *fe) else reg1[1] = 0x0c; - if (_P == 64) + if (priv->prescaler_div == 64) reg1[1] |= 0x40; if (c->frequency >= 1525000) reg1[1] |= 0x80; // register 2 - reg2[1] = (_R >> 8) & 0x03; - reg2[2] = _R; + reg2[1] = (priv->ref_div >> 8) & 0x03; + reg2[2] = priv->ref_div; if (c->frequency < 1455000) reg2[1] |= 0x1c; else if (c->frequency < 1630000) @@ -110,19 +119,15 @@ static int tua6100_set_params(struct dvb_frontend *fe) * The N divisor ratio (note: c->frequency is in kHz, but we * need it in Hz) */ - prediv = (c->frequency * _R) / (_ri / 1000); - div = prediv / _P; + prediv = (c->frequency * priv->ref_div) / (priv->ref_inp / 1000); + div = prediv / priv->prescaler_div; reg1[1] |= (div >> 9) & 0x03; reg1[2] = div >> 1; reg1[3] = (div << 7); - priv->frequency = ((div * _P) * (_ri / 1000)) / _R; + priv->frequency = ((div * priv->prescaler_div) * (priv->ref_inp / 1000)) / priv->ref_div; // Finally, calculate and store the value for A - reg1[3] |= (prediv - (div*_P)) & 0x7f; - -#undef _R -#undef _P -#undef _ri + reg1[3] |= (prediv - (div * priv->prescaler_div)) & 0x7f; if (fe->ops.i2c_gate_ctrl) fe->ops.i2c_gate_ctrl(fe, 1); @@ -189,6 +194,9 @@ struct dvb_frontend *tua6100_attach(struct dvb_frontend *fe, int addr, struct i2 priv->i2c_address = addr; priv->i2c = i2c; + priv->ref_div = 4; + priv->prescaler_div = 32; + priv->ref_inp = 4000000; memcpy(&fe->ops.tuner_ops, &tua6100_tuner_ops, sizeof(struct dvb_tuner_ops)); fe->tuner_priv = priv;