Skip to contentSkip to conversation

fix sata_sil compilation on non-DMI platforms

PATCH In review Konstantinos Margaritis 7 participants 11 messages May 6, 2009 – May 15, 2009 libata

lore
Share & tools
lore

Konstantinos Margaritis

author
#0 lore raw

PATCH 1/1 fix sata_sil compilation on non-DMI platforms

(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

Signature · 3 lines
Konstantinos Margaritis
Codex
http://www.codex.gr
sata_sil.c renamed from sata_sil.c.orig +2 -1
@@ -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;
 }
#1 lore raw
Quoted 10 lines · Konstantinos Margaritis

(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

#2 lore raw
Quoted 8 lines · Jeff Garzik
Quoted 8 lines

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 :

Quoted 8 lines
Quoted 6 lines

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.

Quoted 4 lines

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 <[email protected]>
	Date:   Wed Jan 28 00:07:20 2009 -0600

	    dmi: Fix build breakage

	    [...]
#3 lore raw
In reply to Jeff Garzik
Quoted 2 lines

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 ?

(

#4 lore raw
Quoted 9 lines · Alan Cox
Quoted 2 lines

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.

#5 lore raw
In reply to Jeff Garzik
Quoted 11 lines · Jeff Garzik
Quoted 10 lines · Konstantinos Margaritis

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

Quoted 8 lines
Quoted 6 lines

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.

Quoted 4 lines

What is the breakage?

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

Signature · 1 line
Len Sorensen
#6 lore raw
In reply to Jeff Garzik
Quoted 11 lines · Jeff Garzik
Quoted 10 lines · Konstantinos Margaritis

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

Quoted 7 lines
Quoted 5 lines

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.

Quoted 1 line

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

Quoted 2 lines

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

#7 lore raw
Quoted 22 lines · Mikael Pettersson
Quoted 11 lines · Jeff Garzik
Quoted 10 lines · Konstantinos Margaritis

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

Quoted 7 lines
Quoted 5 lines

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.

Quoted 8 lines
Quoted 1 line

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

#8 lore raw
Quoted 25 lines · Jeff Garzik
Quoted 24 lines · Mikael Pettersson
Quoted 11 lines · Jeff Garzik
Quoted 10 lines · Konstantinos Margaritis

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

Quoted 7 lines
Quoted 5 lines

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.

Quoted 1 line

What is the breakage?

Quoted 8 lines
Quoted 6 lines

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

#9 lore raw
Quoted 3 lines · Mikael Pettersson

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.

Quoted 6 lines

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.

Quoted 8 lines

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

#10 lore raw
Quoted 19 lines · Jeff Garzik
Quoted 3 lines · Mikael Pettersson

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.

Quoted 11 lines

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.

Quoted 6 lines
Quoted 3 lines

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