linux-next.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* linux-next: build warning after merge of the i2c tree
@ 2019-06-11  0:25 Stephen Rothwell
  2019-06-12  8:19 ` Wolfram Sang
  0 siblings, 1 reply; 13+ messages in thread
From: Stephen Rothwell @ 2019-06-11  0:25 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linux Next Mailing List, Linux Kernel Mailing List,
	Ruslan Babayev, Andrew de Quincey, Mauro Carvalho Chehab

[-- Attachment #1: Type: text/plain, Size: 1114 bytes --]

Hi Wolfram,

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.
-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: build warning after merge of the i2c tree
  2019-06-11  0:25 linux-next: build warning after merge of the i2c tree Stephen Rothwell
@ 2019-06-12  8:19 ` Wolfram Sang
  2019-06-12 11:02   ` Mauro Carvalho Chehab
  2019-06-12 11:25   ` [PATCH] media: tua6100: Remove some ugly defines Mauro Carvalho Chehab
  0 siblings, 2 replies; 13+ messages in thread
From: Wolfram Sang @ 2019-06-12  8:19 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Linux Next Mailing List, Linux Kernel Mailing List,
	Ruslan Babayev, Andrew de Quincey, Mauro Carvalho Chehab

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

On Tue, Jun 11, 2019 at 10:25:28AM +1000, Stephen Rothwell wrote:
> Hi Wolfram,
> 
> 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.

I suggest to fix the driver by adding a TUA6100_ prefix to the defines.
I can cook up a patch for that.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: build warning after merge of the i2c tree
  2019-06-12  8:19 ` Wolfram Sang
@ 2019-06-12 11:02   ` Mauro Carvalho Chehab
  2019-06-12 11:09     ` Wolfram Sang
  2019-06-12 11:25   ` [PATCH] media: tua6100: Remove some ugly defines Mauro Carvalho Chehab
  1 sibling, 1 reply; 13+ messages in thread
From: Mauro Carvalho Chehab @ 2019-06-12 11:02 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Stephen Rothwell, Linux Next Mailing List,
	Linux Kernel Mailing List, Ruslan Babayev, Andrew de Quincey

Em Wed, 12 Jun 2019 10:19:29 +0200
Wolfram Sang <wsa@the-dreams.de> escreveu:

> On Tue, Jun 11, 2019 at 10:25:28AM +1000, Stephen Rothwell wrote:
> > Hi Wolfram,
> > 
> > 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.  
> 
> I suggest to fix the driver by adding a TUA6100_ prefix to the defines.
> I can cook up a patch for that.
> 

That entire use of _P, _R and _ri looks weird into my eyes. The code there
do things like:

#define _P 32

...

        if (_P == 64)
                reg1[1] |= 0x40;


It sounds to me that _P and _R are actually some sort of setup
with depends on how the device is wired. 

A quick read on its datasheet at page 19 - downloaded from:

	http://www.datasheetcatalog.com/datasheets_pdf/T/U/A/6/TUA6100.shtml

Shows that:
	P:  divide ratio of the prescaler.
	R:  divide ratio of the R-counter.
	ri: reference frequency input (XTAL osc).

IMO, the right fix would be to change struct tua6100_priv, in order to
add those  parameters, and initialize them with the current values
at tua6100_attach.

That would allow changing those values during device initialization,
in the case we ever need to support a hardware with the same chipset,
with, for example, a different XTAL.

I'll work on a patch to address it.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: build warning after merge of the i2c tree
  2019-06-12 11:02   ` Mauro Carvalho Chehab
@ 2019-06-12 11:09     ` Wolfram Sang
  2019-06-12 11:48       ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 13+ messages in thread
From: Wolfram Sang @ 2019-06-12 11:09 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Stephen Rothwell, Linux Next Mailing List,
	Linux Kernel Mailing List, Ruslan Babayev, Andrew de Quincey

[-- Attachment #1: Type: text/plain, Size: 505 bytes --]

Hi Mauro,

> That entire use of _P, _R and _ri looks weird into my eyes. The code there

Yes.

> do things like:
> 
> #define _P 32
> 
> ...
> 
>         if (_P == 64)
>                 reg1[1] |= 0x40;

Yup, I saw this, too, but didn't feel like refactoring the driver.
Thanks for stepping up!

> I'll work on a patch to address it.

OK, so that means I should send my pull request after yours in the next
merge window? To avoid the build breakage?

Kind regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] media: tua6100: Remove some ugly defines
  2019-06-12  8:19 ` Wolfram Sang
  2019-06-12 11:02   ` Mauro Carvalho Chehab
@ 2019-06-12 11:25   ` Mauro Carvalho Chehab
  2019-06-12 12:01     ` Wolfram Sang
                       ` (2 more replies)
  1 sibling, 3 replies; 13+ messages in thread
From: Mauro Carvalho Chehab @ 2019-06-12 11:25 UTC (permalink / raw)
  To: Linux Media Mailing List
  Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Wolfram Sang,
	Linux Next Mailing List, Linux Kernel Mailing List,
	Ruslan Babayev, Andrew de Quincey, Michael Buesch,
	Stephen Rothwell

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>
---
 drivers/media/dvb-frontends/tua6100.c | 38 ++++++++++++++++-----------
 1 file changed, 23 insertions(+), 15 deletions(-)

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: linux-next: build warning after merge of the i2c tree
  2019-06-12 11:09     ` Wolfram Sang
@ 2019-06-12 11:48       ` Mauro Carvalho Chehab
  2019-06-12 12:04         ` Wolfram Sang
  0 siblings, 1 reply; 13+ messages in thread
From: Mauro Carvalho Chehab @ 2019-06-12 11:48 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Stephen Rothwell, Linux Next Mailing List,
	Linux Kernel Mailing List, Ruslan Babayev, Andrew de Quincey

Em Wed, 12 Jun 2019 13:09:04 +0200
Wolfram Sang <wsa@the-dreams.de> escreveu:

> Hi Mauro,
> 
> > That entire use of _P, _R and _ri looks weird into my eyes. The code there  
> 
> Yes.
> 
> > do things like:
> > 
> > #define _P 32
> > 
> > ...
> > 
> >         if (_P == 64)
> >                 reg1[1] |= 0x40;  
> 
> Yup, I saw this, too, but didn't feel like refactoring the driver.
> Thanks for stepping up!
> 
> > I'll work on a patch to address it.  
> 
> OK, so that means I should send my pull request after yours in the next
> merge window? To avoid the build breakage?

Either that or you can apply my patch on your tree before the
patch that caused the breakage. 

Just let me know what works best for you.

> 
> Kind regards,
> 
>    Wolfram
> 



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] media: tua6100: Remove some ugly defines
  2019-06-12 11:25   ` [PATCH] media: tua6100: Remove some ugly defines Mauro Carvalho Chehab
@ 2019-06-12 12:01     ` Wolfram Sang
  2019-06-18 21:38     ` Wolfram Sang
  2019-07-08  9:25     ` Wolfram Sang
  2 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2019-06-12 12:01 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
	Linux Next Mailing List, Linux Kernel Mailing List,
	Ruslan Babayev, Andrew de Quincey, Michael Buesch,
	Stephen Rothwell

[-- Attachment #1: Type: text/plain, Size: 2079 bytes --]

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>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: build warning after merge of the i2c tree
  2019-06-12 11:48       ` Mauro Carvalho Chehab
@ 2019-06-12 12:04         ` Wolfram Sang
  2019-06-12 12:32           ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 13+ messages in thread
From: Wolfram Sang @ 2019-06-12 12:04 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Stephen Rothwell, Linux Next Mailing List,
	Linux Kernel Mailing List, Ruslan Babayev, Andrew de Quincey

[-- Attachment #1: Type: text/plain, Size: 411 bytes --]


> > OK, so that means I should send my pull request after yours in the next
> > merge window? To avoid the build breakage?
> 
> Either that or you can apply my patch on your tree before the
> patch that caused the breakage. 
> 
> Just let me know what works best for you.

Hmm, the offending patch is already in -next and I don't rebase my tree.
So, I guess it's the merge window dependency then.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: build warning after merge of the i2c tree
  2019-06-12 12:04         ` Wolfram Sang
@ 2019-06-12 12:32           ` Mauro Carvalho Chehab
  2019-06-12 13:15             ` Stephen Rothwell
  0 siblings, 1 reply; 13+ messages in thread
From: Mauro Carvalho Chehab @ 2019-06-12 12:32 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Stephen Rothwell, Linux Next Mailing List,
	Linux Kernel Mailing List, Ruslan Babayev, Andrew de Quincey

Em Wed, 12 Jun 2019 14:04:39 +0200
Wolfram Sang <wsa@the-dreams.de> escreveu:

> > > OK, so that means I should send my pull request after yours in the next
> > > merge window? To avoid the build breakage?  
> > 
> > Either that or you can apply my patch on your tree before the
> > patch that caused the breakage. 
> > 
> > Just let me know what works best for you.  
> 
> Hmm, the offending patch is already in -next and I don't rebase my tree.
> So, I guess it's the merge window dependency then.
> 
Ok, I'll merge it through my tree then.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: build warning after merge of the i2c tree
  2019-06-12 12:32           ` Mauro Carvalho Chehab
@ 2019-06-12 13:15             ` Stephen Rothwell
  2019-06-12 13:24               ` Wolfram Sang
  0 siblings, 1 reply; 13+ messages in thread
From: Stephen Rothwell @ 2019-06-12 13:15 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Wolfram Sang, Linux Next Mailing List, Linux Kernel Mailing List,
	Ruslan Babayev, Andrew de Quincey

[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]

Hi all,

On Wed, 12 Jun 2019 09:32:29 -0300 Mauro Carvalho Chehab <mchehab@infradead.org> wrote:
>
> Em Wed, 12 Jun 2019 14:04:39 +0200
> Wolfram Sang <wsa@the-dreams.de> escreveu:
> 
> > > > OK, so that means I should send my pull request after yours in the next
> > > > merge window? To avoid the build breakage?    
> > > 
> > > Either that or you can apply my patch on your tree before the
> > > patch that caused the breakage. 
> > > 
> > > Just let me know what works best for you.    
> > 
> > Hmm, the offending patch is already in -next and I don't rebase my tree.
> > So, I guess it's the merge window dependency then.
> >   
> Ok, I'll merge it through my tree then.

It should go into the i2c tree (since that is where the *warning* was
introduced).  It is only a warning and there won't be many patches
between the patch that introduced the warning and this one that fixes
it.  This patch could then have a Fixes tag that makes sense i.e. it
will reference a previous commit.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: build warning after merge of the i2c tree
  2019-06-12 13:15             ` Stephen Rothwell
@ 2019-06-12 13:24               ` Wolfram Sang
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2019-06-12 13:24 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Mauro Carvalho Chehab, Linux Next Mailing List,
	Linux Kernel Mailing List, Ruslan Babayev, Andrew de Quincey

[-- Attachment #1: Type: text/plain, Size: 1380 bytes --]

On Wed, Jun 12, 2019 at 11:15:35PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> On Wed, 12 Jun 2019 09:32:29 -0300 Mauro Carvalho Chehab <mchehab@infradead.org> wrote:
> >
> > Em Wed, 12 Jun 2019 14:04:39 +0200
> > Wolfram Sang <wsa@the-dreams.de> escreveu:
> > 
> > > > > OK, so that means I should send my pull request after yours in the next
> > > > > merge window? To avoid the build breakage?    
> > > > 
> > > > Either that or you can apply my patch on your tree before the
> > > > patch that caused the breakage. 
> > > > 
> > > > Just let me know what works best for you.    
> > > 
> > > Hmm, the offending patch is already in -next and I don't rebase my tree.
> > > So, I guess it's the merge window dependency then.
> > >   
> > Ok, I'll merge it through my tree then.
> 
> It should go into the i2c tree (since that is where the *warning* was
> introduced).  It is only a warning and there won't be many patches
> between the patch that introduced the warning and this one that fixes
> it.  This patch could then have a Fixes tag that makes sense i.e. it
> will reference a previous commit.

I can apply the patch to my i2c/for-next branch, but not to my
i2c/for-5.3 branch (which I merge into i2c/for-next). This way, the
warning will go away, but the media patch still goes to Linus via the
media tree. Is that suitable for you?



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] media: tua6100: Remove some ugly defines
  2019-06-12 11:25   ` [PATCH] media: tua6100: Remove some ugly defines Mauro Carvalho Chehab
  2019-06-12 12:01     ` Wolfram Sang
@ 2019-06-18 21:38     ` Wolfram Sang
  2019-07-08  9:25     ` Wolfram Sang
  2 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2019-06-18 21:38 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
	Linux Next Mailing List, Linux Kernel Mailing List,
	Ruslan Babayev, Andrew de Quincey, Michael Buesch,
	Stephen Rothwell

[-- Attachment #1: Type: text/plain, Size: 2170 bytes --]

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?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] media: tua6100: Remove some ugly defines
  2019-06-12 11:25   ` [PATCH] media: tua6100: Remove some ugly defines Mauro Carvalho Chehab
  2019-06-12 12:01     ` Wolfram Sang
  2019-06-18 21:38     ` Wolfram Sang
@ 2019-07-08  9:25     ` Wolfram Sang
  2 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2019-07-08  9:25 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
	Linux Next Mailing List, Linux Kernel Mailing List,
	Ruslan Babayev, Andrew de Quincey, Michael Buesch,
	Stephen Rothwell

[-- Attachment #1: Type: text/plain, Size: 2625 bytes --]

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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-07-08  9:25 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-11  0:25 linux-next: build warning after merge of the i2c tree Stephen Rothwell
2019-06-12  8:19 ` Wolfram Sang
2019-06-12 11:02   ` Mauro Carvalho Chehab
2019-06-12 11:09     ` Wolfram Sang
2019-06-12 11:48       ` Mauro Carvalho Chehab
2019-06-12 12:04         ` Wolfram Sang
2019-06-12 12:32           ` Mauro Carvalho Chehab
2019-06-12 13:15             ` Stephen Rothwell
2019-06-12 13:24               ` Wolfram Sang
2019-06-12 11:25   ` [PATCH] media: tua6100: Remove some ugly defines Mauro Carvalho Chehab
2019-06-12 12:01     ` Wolfram Sang
2019-06-18 21:38     ` Wolfram Sang
2019-07-08  9:25     ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).