[PATCH] fix sata_sil compilation on non-DMI platforms

[PATCH] fix sata_sil compilation on non-DMI platforms

[Konstantinos Margaritis]

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

(not subscribed please CC me)

I tried to compile sata_sil on a 2.6.27 kernel on powerpc32 and I found that 
it failed to compile -lots of dmi related errors. I found that I had to 
include the broken_systems handling code in #ifdef CONFIG_DMI (DMI is not 
supported on platforms other than i386/x86_64). 

Lennert on #mklinux told me that this commit broke the non-dmi support, and 
that a similar patch to mine is used on ARM systems :

commit e57db7bde7bff95ae812736ca00c73bd5271455b
SATA Sil: Blacklist system that spins off disks during ACPI power off

With this patch, sata_sil compiles on ppc (and I guess on other platforms). 
I'm using it for a while with no problems with a Delock 4-port SATA PCI card.

Regards

-- 
Konstantinos Margaritis
Codex
http://www.codex.gr

[-- Attachment #2: sata_sil.c.patch --]
[-- Type: text/x-patch, Size: 458 bytes --]

--- sata_sil.c.orig	2009-05-06 20:03:16.472876188 +0300
+++ sata_sil.c	2009-05-06 20:04:22.693209638 +0300
@@ -698,6 +698,7 @@
 
 static bool sil_broken_system_poweroff(struct pci_dev *pdev)
 {
+#ifdef CONFIG_DMI
 	static const struct dmi_system_id broken_systems[] = {
 		{
 			.ident = "HP Compaq nx6325",
@@ -718,7 +719,7 @@
 		/* apply the quirk only to on-board controllers */
 		return slot == PCI_SLOT(pdev->devfn);
 	}
-
+#endif
 	return false;
 }
 

Re: [PATCH] fix sata_sil compilation on non-DMI platforms

[Jeff Garzik]

Konstantinos Margaritis wrote:
> (not subscribed please CC me)
> 
> I tried to compile sata_sil on a 2.6.27 kernel on powerpc32 and I found that 
> it failed to compile -lots of dmi related errors. I found that I had to 
> include the broken_systems handling code in #ifdef CONFIG_DMI (DMI is not 
> supported on platforms other than i386/x86_64). 
> 
> Lennert on #mklinux told me that this commit broke the non-dmi support, and 
> that a similar patch to mine is used on ARM systems :
> 
> commit e57db7bde7bff95ae812736ca00c73bd5271455b
> SATA Sil: Blacklist system that spins off disks during ACPI power off
> 
> With this patch, sata_sil compiles on ppc (and I guess on other platforms). 
> I'm using it for a while with no problems with a Delock 4-port SATA PCI card.

(CC'ing various Lennerts)

What is the breakage?

Ideally the DMI subsystem should be provided wrappers for platforms 
without DMI, rendering patches like this unnecessary.

	Jeff

Re: [PATCH] fix sata_sil compilation on non-DMI platforms

[Lennert Buytenhek]

On Mon, May 11, 2009 at 02:12:06PM -0400, Jeff Garzik wrote:

> > I tried to compile sata_sil on a 2.6.27 kernel on powerpc32 and I found 
> > that it failed to compile -lots of dmi related errors. I found that I had 
> > to include the broken_systems handling code in #ifdef CONFIG_DMI (DMI is 
> > not supported on platforms other than i386/x86_64). 
> >
> > Lennert on #mklinux told me that this commit broke the non-dmi support, 
> > and that a similar patch to mine is used on ARM systems :
> >
> > commit e57db7bde7bff95ae812736ca00c73bd5271455b
> > SATA Sil: Blacklist system that spins off disks during ACPI power off
> >
> > With this patch, sata_sil compiles on ppc (and I guess on other 
> > platforms). I'm using it for a while with no problems with a Delock 4-port 
> > SATA PCI card.
> 
> (CC'ing various Lennerts)

That was moi.


> What is the breakage?
> 
> Ideally the DMI subsystem should be provided wrappers for platforms 
> without DMI, rendering patches like this unnecessary.

It seems that the commit below should fix it.  Since that only went
into 2.6.29, that would explain why 2.6.27 is broken.


	commit d8204ee2ad1c9babd7e33d4c118ec99a78a8442e
	Author: Kumar Gala <galak@kernel.crashing.org>
	Date:   Wed Jan 28 00:07:20 2009 -0600

	    dmi: Fix build breakage

	    [...]

Re: [PATCH] fix sata_sil compilation on non-DMI platforms

[Alan Cox]

> Ideally the DMI subsystem should be provided wrappers for platforms 
> without DMI, rendering patches like this unnecessary.

Interesting question - is the PPC OpenFirmware machine information
mappable onto the DMI space ? or the various static bits of name
information in the various ARM and the like machine descriptions.

If not which bits are similar enough we could replace dmi at the high
level with an abstract interface for system/vendor/... that was ?

(

Re: [PATCH] fix sata_sil compilation on non-DMI platforms

[Benjamin Herrenschmidt]

On Mon, 2009-05-11 at 19:44 +0100, Alan Cox wrote:
> > Ideally the DMI subsystem should be provided wrappers for platforms 
> > without DMI, rendering patches like this unnecessary.
> 
> Interesting question - is the PPC OpenFirmware machine information
> mappable onto the DMI space ? or the various static bits of name
> information in the various ARM and the like machine descriptions.
> 
> If not which bits are similar enough we could replace dmi at the high
> level with an abstract interface for system/vendor/... that was ?

The one thing we could try to map would be the device-tree "compatible"
property which would contain vendor,system tuples from more specific to
more generic with which the machine is compatible with.

Of course I would argue the other way around and create a device-tree
from the DMI data :-)

Cheers,
Ben.

Re: [PATCH] fix sata_sil compilation on non-DMI platforms

[Lennart Sorensen]

On Mon, May 11, 2009 at 02:12:06PM -0400, Jeff Garzik wrote:
> Konstantinos Margaritis wrote:
>> (not subscribed please CC me)
>>
>> I tried to compile sata_sil on a 2.6.27 kernel on powerpc32 and I found 
>> that it failed to compile -lots of dmi related errors. I found that I 
>> had to include the broken_systems handling code in #ifdef CONFIG_DMI 
>> (DMI is not supported on platforms other than i386/x86_64). 
>>
>> Lennert on #mklinux told me that this commit broke the non-dmi support, 
>> and that a similar patch to mine is used on ARM systems :
>>
>> commit e57db7bde7bff95ae812736ca00c73bd5271455b
>> SATA Sil: Blacklist system that spins off disks during ACPI power off
>>
>> With this patch, sata_sil compiles on ppc (and I guess on other 
>> platforms). I'm using it for a while with no problems with a Delock 
>> 4-port SATA PCI card.
>
> (CC'ing various Lennerts)

I am not a Lennert, I am a Lennart. :)  I didn't do it.

> What is the breakage?
>
> Ideally the DMI subsystem should be provided wrappers for platforms  
> without DMI, rendering patches like this unnecessary.

-- 
Len Sorensen

Re: [PATCH] fix sata_sil compilation on non-DMI platforms

[Mikael Pettersson]

Jeff Garzik writes:
 > Konstantinos Margaritis wrote:
 > > (not subscribed please CC me)
 > > 
 > > I tried to compile sata_sil on a 2.6.27 kernel on powerpc32 and I found that 
 > > it failed to compile -lots of dmi related errors. I found that I had to 
 > > include the broken_systems handling code in #ifdef CONFIG_DMI (DMI is not 
 > > supported on platforms other than i386/x86_64). 
 > > 
 > > Lennert on #mklinux told me that this commit broke the non-dmi support, and 
 > > that a similar patch to mine is used on ARM systems :
 > > 
 > > commit e57db7bde7bff95ae812736ca00c73bd5271455b
 > > SATA Sil: Blacklist system that spins off disks during ACPI power off
 > > 
 > > With this patch, sata_sil compiles on ppc (and I guess on other platforms). 
 > > I'm using it for a while with no problems with a Delock 4-port SATA PCI card.
 > 
 > (CC'ing various Lennerts)

That would be Lennert Buytenhek.

 > What is the breakage?

Compile-time error because sata_sil is used on !x86 platforms and the
unconditional references to x86-only DMI stuff simply aren't valid.

I have an ARM + sata_sil NAS box that did have this problem a few
kernel releases back, but it got resolved before that kernel's -final.
(I can look up the details on Thursday when I'm back to my home network.)

 > Ideally the DMI subsystem should be provided wrappers for platforms 
 > without DMI, rendering patches like this unnecessary.

Another ARM NAS of mine uses pata_artop with a short 40-wire cable.
To correct the speed setting I need a cable type override function with
an "is it $this platform" check. The lack of a unified infrastructure
for these checks forces me to use ARM-specific code wrapped with
#ifdef CONFIG_ARM. So I agree with your comment, except I don't have
any opinion on whether a unified platform identification layer should
look like DMI, OF, or something else.

/Mikael

Re: [PATCH] fix sata_sil compilation on non-DMI platforms

[Jeff Garzik]

Mikael Pettersson wrote:
> Jeff Garzik writes:
>  > Konstantinos Margaritis wrote:
>  > > (not subscribed please CC me)
>  > > 
>  > > I tried to compile sata_sil on a 2.6.27 kernel on powerpc32 and I found that 
>  > > it failed to compile -lots of dmi related errors. I found that I had to 
>  > > include the broken_systems handling code in #ifdef CONFIG_DMI (DMI is not 
>  > > supported on platforms other than i386/x86_64). 
>  > > 
>  > > Lennert on #mklinux told me that this commit broke the non-dmi support, and 
>  > > that a similar patch to mine is used on ARM systems :
>  > > 
>  > > commit e57db7bde7bff95ae812736ca00c73bd5271455b
>  > > SATA Sil: Blacklist system that spins off disks during ACPI power off
>  > > 
>  > > With this patch, sata_sil compiles on ppc (and I guess on other platforms). 
>  > > I'm using it for a while with no problems with a Delock 4-port SATA PCI card.
>  > 
>  > (CC'ing various Lennerts)
> 
> That would be Lennert Buytenhek.
> 
>  > What is the breakage?
> 
> Compile-time error because sata_sil is used on !x86 platforms and the
> unconditional references to x86-only DMI stuff simply aren't valid.
> 
> I have an ARM + sata_sil NAS box that did have this problem a few
> kernel releases back, but it got resolved before that kernel's -final.
> (I can look up the details on Thursday when I'm back to my home network.)

Yes, I was asking for specific details of the breakage.

It is common practice to create no-op stubs for the !CONFIG_FEATURE 
case, thereby saving individual drivers from being littered with 
CONFIG_xxx ifdefs all over the place.

You can see this in the bottom half of include/linux/dmi.h, which is 
clearly intended to support !CONFIG_DMI platforms.

Therefore, it is an open question of what _specifically_ is the 
breakage, because include/linux/dmi.h is not working as intended on 
non-DMI platforms.

	Jeff

Re: [PATCH] fix sata_sil compilation on non-DMI platforms

[Mikael Pettersson]

Jeff Garzik writes:
 > Mikael Pettersson wrote:
 > > Jeff Garzik writes:
 > >  > Konstantinos Margaritis wrote:
 > >  > > (not subscribed please CC me)
 > >  > > 
 > >  > > I tried to compile sata_sil on a 2.6.27 kernel on powerpc32 and I found that 
 > >  > > it failed to compile -lots of dmi related errors. I found that I had to 
 > >  > > include the broken_systems handling code in #ifdef CONFIG_DMI (DMI is not 
 > >  > > supported on platforms other than i386/x86_64). 
 > >  > > 
 > >  > > Lennert on #mklinux told me that this commit broke the non-dmi support, and 
 > >  > > that a similar patch to mine is used on ARM systems :
 > >  > > 
 > >  > > commit e57db7bde7bff95ae812736ca00c73bd5271455b
 > >  > > SATA Sil: Blacklist system that spins off disks during ACPI power off
 > >  > > 
 > >  > > With this patch, sata_sil compiles on ppc (and I guess on other platforms). 
 > >  > > I'm using it for a while with no problems with a Delock 4-port SATA PCI card.
 > >  > 
 > >  > (CC'ing various Lennerts)
 > > 
 > > That would be Lennert Buytenhek.
 > > 
 > >  > What is the breakage?
 > > 
 > > Compile-time error because sata_sil is used on !x86 platforms and the
 > > unconditional references to x86-only DMI stuff simply aren't valid.
 > > 
 > > I have an ARM + sata_sil NAS box that did have this problem a few
 > > kernel releases back, but it got resolved before that kernel's -final.
 > > (I can look up the details on Thursday when I'm back to my home network.)
 > 
 > Yes, I was asking for specific details of the breakage.

Ok I've checked now, and the breakage is that the commit referenced
above added dmi stuff to sata_sil.c without an #include <linux/dmi.h>,
and that broke !x86 builds:

drivers/ata/sata_sil.c: In function 'sil_broken_system_poweroff':
drivers/ata/sata_sil.c:713: error: implicit declaration of function 'dmi_first_match'
drivers/ata/sata_sil.c:713: warning: initialization makes pointer from integer without a cast

x86 builds work because they drag in dmi.h indirectly via some other header.

The follow-up commit 1737ef7598d3515fdc11cb9ba7e054f334404e04
"sata_sil: Fix build breakage" added #include <linux/dmi.h> which
made !x86 builds work again. 2.6.29-rc3 was released in the window
between these two commits so it had this build breakage.

So the problem is not that <linux/dmi.h> doesn't work, but that
someone apparently backported e57db7bde7bff95ae812736ca00c73bd5271455b
to 2.6.27 but not 1737ef7598d3515fdc11cb9ba7e054f334404e04.

/Mikael

Re: [PATCH] fix sata_sil compilation on non-DMI platforms

[Jeff Garzik]

Mikael Pettersson wrote:
> Ok I've checked now, and the breakage is that the commit referenced
> above added dmi stuff to sata_sil.c without an #include <linux/dmi.h>,
> and that broke !x86 builds:

Great!  Thanks for checking.


> drivers/ata/sata_sil.c: In function 'sil_broken_system_poweroff':
> drivers/ata/sata_sil.c:713: error: implicit declaration of function 'dmi_first_match'
> drivers/ata/sata_sil.c:713: warning: initialization makes pointer from integer without a cast
> 
> x86 builds work because they drag in dmi.h indirectly via some other header.
> 
> The follow-up commit 1737ef7598d3515fdc11cb9ba7e054f334404e04
> "sata_sil: Fix build breakage" added #include <linux/dmi.h> which
> made !x86 builds work again. 2.6.29-rc3 was released in the window
> between these two commits so it had this build breakage.
> 
> So the problem is not that <linux/dmi.h> doesn't work, but that
> someone apparently backported e57db7bde7bff95ae812736ca00c73bd5271455b
> to 2.6.27 but not 1737ef7598d3515fdc11cb9ba7e054f334404e04.

OK, so it sounds like upstream is OK, and 2.6.27.x needs 
1737ef7598d3515fdc11cb9ba7e054f334404e04

Thanks,

	Jeff

Re: [PATCH] fix sata_sil compilation on non-DMI platforms

[Mikael Pettersson]

Jeff Garzik writes:
 > Mikael Pettersson wrote:
 > > Ok I've checked now, and the breakage is that the commit referenced
 > > above added dmi stuff to sata_sil.c without an #include <linux/dmi.h>,
 > > and that broke !x86 builds:
 > 
 > Great!  Thanks for checking.
 > 
 > 
 > > drivers/ata/sata_sil.c: In function 'sil_broken_system_poweroff':
 > > drivers/ata/sata_sil.c:713: error: implicit declaration of function 'dmi_first_match'
 > > drivers/ata/sata_sil.c:713: warning: initialization makes pointer from integer without a cast
 > > 
 > > x86 builds work because they drag in dmi.h indirectly via some other header.
 > > 
 > > The follow-up commit 1737ef7598d3515fdc11cb9ba7e054f334404e04
 > > "sata_sil: Fix build breakage" added #include <linux/dmi.h> which
 > > made !x86 builds work again. 2.6.29-rc3 was released in the window
 > > between these two commits so it had this build breakage.
 > > 
 > > So the problem is not that <linux/dmi.h> doesn't work, but that
 > > someone apparently backported e57db7bde7bff95ae812736ca00c73bd5271455b
 > > to 2.6.27 but not 1737ef7598d3515fdc11cb9ba7e054f334404e04.
 > 
 > OK, so it sounds like upstream is OK, and 2.6.27.x needs 
 > 1737ef7598d3515fdc11cb9ba7e054f334404e04

Upstream is ok, but so is 2.6.27.x. 2.6.27 vanilla sata_sil.c has no
dmi stuff in it, and 2.6.27.23 (current latest -stable for 2.6.27)
has no updates to sata_sil.c. I just compiled both those kernels for
ARM with sata_sil enabled and there were no errors.

Looks like the original poster used a patched 2.6.27...