Re: linux-next: Tree for April 4 [BROKEN thinkpad_acpi]

Re: linux-next: Tree for April 4 [BROKEN thinkpad_acpi]

[Sedat Dilek]

On Mon, Apr 4, 2011 at 6:35 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi all,
>
> Changes since 20110401:
>
> Dropped tree: xen
>
> The net tree gained a build failure so I used the version from
> next-20110401.
>

Hi,

My build breaked today.
Looks like the issue was introduced by commit
2af915b1eda9a671973256540035f8a6da1d57b4:
"thinkpad_acpi: Convert printks to pr_<level>"

- Sedat -

P.S.: Attached excerpt from my build.log

[...]
  CC [M]  drivers/platform/x86/thinkpad_acpi.o
[...]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:
In function 'hotkey_init':
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:3233:2:
error: implicit declaration of function 'str_supported'
[-Werror=implicit-function-declaration]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:3233:2:
warning: format '%s' expects argument of type 'char *', but argument 2
has type 'int' [-Wformat]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:3289:2:
warning: format '%s' expects argument of type 'char *', but argument 2
has type 'int' [-Wformat]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:
In function 'bluetooth_init':
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:3991:2:
warning: format '%s' expects argument of type 'char *', but argument 2
has type 'int' [-Wformat]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:
In function 'wan_init':
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:4179:2:
warning: format '%s' expects argument of type 'char *', but argument 2
has type 'int' [-Wformat]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:
In function 'uwb_init':
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:4319:2:
warning: format '%s' expects argument of type 'char *', but argument 2
has type 'int' [-Wformat]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:
In function 'video_init':
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:4419:2:
warning: format '%s' expects argument of type 'char *', but argument 2
has type 'int' [-Wformat]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:
In function 'light_init':
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:4803:2:
warning: format '%s' expects argument of type 'char *', but argument 2
has type 'int' [-Wformat]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:4803:2:
warning: format '%s' expects argument of type 'char *', but argument 3
has type 'int' [-Wformat]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:
In function 'cmos_init':
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:4910:2:
warning: format '%s' expects argument of type 'char *', but argument 2
has type 'int' [-Wformat]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:
In function 'led_init':
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:5281:2:
warning: format '%s' expects argument of type 'char *', but argument 2
has type 'int' [-Wformat]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:
In function 'beep_init':
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:5404:2:
warning: format '%s' expects argument of type 'char *', but argument 2
has type 'int' [-Wformat]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:
In function 'thermal_init':
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:5737:2:
warning: format '%s' expects argument of type 'char *', but argument 2
has type 'int' [-Wformat]
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:
In function 'volume_init':
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:6901:2:
warning: format '%s' expects argument of type 'char *', but argument 2
has type 'int' [-Wformat]
  CC [M]  net/sched/sch_sfb.o
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:
In function 'fan_init':
/home/sd/src/linux-2.6/linux-2.6.39-rc1/debian/build/source_i386_none/drivers/platform/x86/thinkpad_acpi.c:8001:2:
warning: format '%s' expects argument of type 'char *', but argument 2
has type 'int' [-Wformat]
  CC [M]  drivers/media/dvb/ttusb-dec/ttusbdecfe.o
cc1: some warnings being treated as errors

make[7]: *** [drivers/platform/x86/thinkpad_acpi.o] Error 1
make[6]: *** [drivers/platform/x86] Error 2
make[5]: *** [drivers/platform] Error 2

- Sedat -

Re: linux-next: Tree for April 4 [BROKEN thinkpad_acpi]

[Matthew Garrett]

On Mon, Apr 04, 2011 at 11:21:43AM +0200, Sedat Dilek wrote:
> My build breaked today.
> Looks like the issue was introduced by commit
> 2af915b1eda9a671973256540035f8a6da1d57b4:
> "thinkpad_acpi: Convert printks to pr_<level>"

Yes, this seems to break the non-debug case. Joe, can you take a look? 
I'll drop this for the moment.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: linux-next: Tree for April 4 [BROKEN thinkpad_acpi]

[Joe Perches]

On Mon, 2011-04-04 at 13:22 +0100, Matthew Garrett wrote:
> On Mon, Apr 04, 2011 at 11:21:43AM +0200, Sedat Dilek wrote:
> > My build breaked today.
> > Looks like the issue was introduced by commit
> > 2af915b1eda9a671973256540035f8a6da1d57b4:
> > "thinkpad_acpi: Convert printks to pr_<level>"
> Yes, this seems to break the non-debug case. Joe, can you take a look? 

Sorry 'bout that.  I only compiled it allyesconfig.

It's vdbg_printk no_printk verification.

#ifdef CONFIG_THINKPAD_ACPI_DEBUG
#define vdbg_printk dbg_printk
static const char *str_supported(int is_supported);
#else
#define vdbg_printk(a_dbg_level, format, arg...)	\
	no_printk(format, ##arg)
#endif

Two ways to handle this.

1: add
static inline const char *str_supported(int is_supported) { return ""; }
to the #else

2: Remove no_printk verification and return it to do {} while (0)

Do you have a preference?

Re: linux-next: Tree for April 4 [BROKEN thinkpad_acpi]

[Matthew Garrett]

On Mon, Apr 04, 2011 at 05:49:29AM -0700, Joe Perches wrote:

> It's vdbg_printk no_printk verification.
> 
> #ifdef CONFIG_THINKPAD_ACPI_DEBUG
> #define vdbg_printk dbg_printk
> static const char *str_supported(int is_supported);
> #else
> #define vdbg_printk(a_dbg_level, format, arg...)	\
> 	no_printk(format, ##arg)
> #endif
> 
> Two ways to handle this.
> 
> 1: add
> static inline const char *str_supported(int is_supported) { return ""; }
> to the #else
> 
> 2: Remove no_printk verification and return it to do {} while (0)
> 
> Do you have a preference?

I don't. Henrique?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: linux-next: Tree for April 4 [BROKEN thinkpad_acpi]

[Sedat Dilek]

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

On Mon, Apr 4, 2011 at 3:00 PM, Matthew Garrett <mjg@redhat.com> wrote:
> On Mon, Apr 04, 2011 at 05:49:29AM -0700, Joe Perches wrote:
>
>> It's vdbg_printk no_printk verification.
>>
>> #ifdef CONFIG_THINKPAD_ACPI_DEBUG
>> #define vdbg_printk dbg_printk
>> static const char *str_supported(int is_supported);
>> #else
>> #define vdbg_printk(a_dbg_level, format, arg...)      \
>>       no_printk(format, ##arg)
>> #endif
>>
>> Two ways to handle this.
>>
>> 1: add
>> static inline const char *str_supported(int is_supported) { return ""; }
>> to the #else
>>
>> 2: Remove no_printk verification and return it to do {} while (0)
>>
>> Do you have a preference?
>
> I don't. Henrique?
>
> --
> Matthew Garrett | mjg59@srcf.ucam.org
>

I have tried with solution #2 as it partially restores old behaviour
(patch is attached).

- Sedat -

[-- Attachment #2: thinkpad_acpi-fix.patch --]
[-- Type: text/x-patch, Size: 466 bytes --]

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index d8f3e65..983e42f 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -391,7 +391,7 @@ do {									\
 static const char *str_supported(int is_supported);
 #else
 #define vdbg_printk(a_dbg_level, format, arg...)	\
-	no_printk(format, ##arg)
+	do { } while (0)
 #endif
 
 static void tpacpi_log_usertask(const char * const what)

Re: linux-next: Tree for April 4 [BROKEN thinkpad_acpi]

[Sedat Dilek]

On Mon, Apr 4, 2011 at 3:44 PM, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> On Mon, Apr 4, 2011 at 3:00 PM, Matthew Garrett <mjg@redhat.com> wrote:
>> On Mon, Apr 04, 2011 at 05:49:29AM -0700, Joe Perches wrote:
>>
>>> It's vdbg_printk no_printk verification.
>>>
>>> #ifdef CONFIG_THINKPAD_ACPI_DEBUG
>>> #define vdbg_printk dbg_printk
>>> static const char *str_supported(int is_supported);
>>> #else
>>> #define vdbg_printk(a_dbg_level, format, arg...)      \
>>>       no_printk(format, ##arg)
>>> #endif
>>>
>>> Two ways to handle this.
>>>
>>> 1: add
>>> static inline const char *str_supported(int is_supported) { return ""; }
>>> to the #else
>>>
>>> 2: Remove no_printk verification and return it to do {} while (0)
>>>
>>> Do you have a preference?
>>
>> I don't. Henrique?
>>
>> --
>> Matthew Garrett | mjg59@srcf.ucam.org
>>
>
> I have tried with solution #2 as it partially restores old behaviour
> (patch is attached).
>
> - Sedat -
>

Please feel free to add a:

     Reported-by: Sedat Dilek <sedat.dilek@gmail.com>

For solution #2 :

     Tested-by: Sedat Dilek <sedat.dilek@gmail.com>

- Sedat -

Re: linux-next: Tree for April 4 [BROKEN thinkpad_acpi]

[Joe Perches]

On Mon, 2011-04-04 at 16:44 +0200, Sedat Dilek wrote:
> > I have tried with solution #2 as it partially restores old behaviour
> > (patch is attached).

Partially?  Please explain.

Re: linux-next: Tree for April 4 [BROKEN thinkpad_acpi]

[Sedat Dilek]

On Mon, Apr 4, 2011 at 5:02 PM, Joe Perches <joe@perches.com> wrote:
> On Mon, 2011-04-04 at 16:44 +0200, Sedat Dilek wrote:
>> > I have tried with solution #2 as it partially restores old behaviour
>> > (patch is attached).
>
> Partially?  Please explain.
>
>

From [1] (platform-drivers-x86.git#linux-next):
...
 #ifdef CONFIG_THINKPAD_ACPI_DEBUG
 #define vdbg_printk dbg_printk
 static const char *str_supported(int is_supported);
 #else
-#define vdbg_printk(a_dbg_level, format, arg...) \
-       do { } while (0)
+#define vdbg_printk(a_dbg_level, format, arg...)       \
+       no_printk(format, ##arg)
 #endif
...

- Sedat -

[1] http://git.kernel.org/?p=linux/kernel/git/mjg59/platform-drivers-x86.git;a=blobdiff;f=drivers/platform/x86/thinkpad_acpi.c;h=d8f3e653e96d8d5ad2081e4794a8f70f852b20de;hp=a08561f5349ef038decaf8edcd73528f30f056a9;hb=2af915b1eda9a671973256540035f8a6da1d57b4;hpb=1ba77972006a7ad5fed0811aac3fed724e7c5a04

Re: linux-next: Tree for April 4 [BROKEN thinkpad_acpi]

[Joe Perches]

On Mon, 2011-04-04 at 17:08 +0200, Sedat Dilek wrote:
> On Mon, Apr 4, 2011 at 5:02 PM, Joe Perches <joe@perches.com> wrote:
> > On Mon, 2011-04-04 at 16:44 +0200, Sedat Dilek wrote:
> >> > I have tried with solution #2 as it partially restores old behaviour
> >> > (patch is attached).
> > Partially?  Please explain.
> -#define vdbg_printk(a_dbg_level, format, arg...) \
> -       do { } while (0)
> +#define vdbg_printk(a_dbg_level, format, arg...)       \
> +       no_printk(format, ##arg)

I understand the reversed patch.

My point was focused on the word "partially".

Re: linux-next: Tree for April 4 [BROKEN thinkpad_acpi]

[Henrique de Moraes Holschuh]

On Mon, 04 Apr 2011 14:00 +0100, "Matthew Garrett" <mjg@redhat.com> wrote:
> On Mon, Apr 04, 2011 at 05:49:29AM -0700, Joe Perches wrote:
> 
> > It's vdbg_printk no_printk verification.
> > 
> > #ifdef CONFIG_THINKPAD_ACPI_DEBUG
> > #define vdbg_printk dbg_printk
> > static const char *str_supported(int is_supported);
> > #else
> > #define vdbg_printk(a_dbg_level, format, arg...)	\
> > 	no_printk(format, ##arg)
> > #endif
> > 
> > Two ways to handle this.
> > 
> > 1: add
> > static inline const char *str_supported(int is_supported) { return ""; }
> > to the #else
> > 
> > 2: Remove no_printk verification and return it to do {} while (0)
> > 
> > Do you have a preference?
> 
> I don't. Henrique?

No set preference, either way is fine.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh