Skip to contentSkip to conversation

next-2008-0306 scsi/libsas build failure

PATCH In review Randy Dunlap 5 participants 12 messages Mar 6, 2008 – Mar 7, 2008

Subsystems (3)
SCSI SCSI libsas Firmware loader (request_firmware)
lore
Share & tools
lore

Randy Dunlap

author
#0 lore raw

drivers/built-in.o: In function `sas_request_addr':
(.text+0x598fb): undefined reference to request_firmware' drivers/built-in.o: In function sas_request_addr':
(.text+0x5998e): undefined reference to `release_firmware' make[1]: *** [.tmp_vmlinux1] Error 1
make: *** [sub-make] Error 2

drivers/scsi/libsas/sas_scsi_host.c calls request_firmware() and release_firmware() without checking to see if CONFIG_FW_LOADER=y.

---
~Randy

#1 lore raw
Quoted 9 lines · Randy Dunlap

drivers/built-in.o: In function `sas_request_addr':
(.text+0x598fb): undefined reference to request_firmware' drivers/built-in.o: In function sas_request_addr':
(.text+0x5998e): undefined reference to `release_firmware' make[1]: *** [.tmp_vmlinux1] Error 1
make: *** [sub-make] Error 2

drivers/scsi/libsas/sas_scsi_host.c calls request_firmware() and release_firmware() without checking to see if CONFIG_FW_LOADER=y.

Sounds like a bug in the firmware code, it should provide stubs if this config option is not present, right?

thanks,

greg k-h

author
#2 lore raw
Quoted 8 lines · Greg KH
Quoted 7 lines · Randy Dunlap

drivers/built-in.o: In function `sas_request_addr':
(.text+0x598fb): undefined reference to request_firmware' drivers/built-in.o: In function sas_request_addr':
(.text+0x5998e): undefined reference to `release_firmware' make[1]: *** [.tmp_vmlinux1] Error 1
make: *** [sub-make] Error 2

Quoted 5 lines
Quoted 2 lines

drivers/scsi/libsas/sas_scsi_host.c calls request_firmware() and release_firmware() without checking to see if CONFIG_FW_LOADER=y.

Sounds like a bug in the firmware code, it should provide stubs if this config option is not present, right?

That would be one (good) solution.

I'll wait to see if James or Darrick have other ideas...

Signature · 1 line
~Randy
#3 lore raw
Quoted 6 lines
Quoted 2 lines

Sounds like a bug in the firmware code, it should provide stubs if this config option is not present, right?

That would be one (good) solution.

I'll wait to see if James or Darrick have other ideas...

sas_request_addr should only be called in the (possibly) exceptional case that a SAS controller forgot its address and wants a sysadmin to provide one. However, it's not critical to the operation of libsas, so in this particular case we can just return an error code if CONFIG_FW_LOADER=n.

--D

#4 lore raw

PATCH 1/1 libsas: Check that the firmware loader is present in sas_request_addr

In reply to Randy Dunlap

Return an error code in sas_request_addr if the fw loader isn't configured.

Signed-off-by Darrick J. Wong <[email protected]>

---

 drivers/scsi/libsas/sas_scsi_host.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
drivers/scsi/libsas/sas_scsi_host.c +4 -0
index 601ec5b..e44be7a 100644
@@ -1085,6 +1085,7 @@ static void sas_parse_addr(u8 *sas_addr, const char *p)
 
 int sas_request_addr(struct Scsi_Host *shost, u8 *addr)
 {
+#ifdef CONFIG_FW_LOADER
 	int res;
 	const struct firmware *fw;
 

@@ -1102,6 +1103,9 @@ int sas_request_addr(struct Scsi_Host *shost, u8 *addr)
 out:
 	release_firmware(fw);
 	return res;
+#else
+	return -ENOENT;
+#endif
 }
 EXPORT_SYMBOL_GPL(sas_request_addr);
maintainer
#5 lore raw

Re: [PATCH] libsas: Check that the firmware loader is present in sas_request_addr

Quoted 22 lines · Darrick J. Wong

Return an error code in sas_request_addr if the fw loader isn't configured.

Signed-off-by Darrick J. Wong <[email protected]>

---

 drivers/scsi/libsas/sas_scsi_host.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
drivers/scsi/libsas/sas_scsi_host.c +1 -0
index 601ec5b..e44be7a 100644
@@ -1085,6 +1085,7 @@ static void sas_parse_addr(u8 *sas_addr, const char *p)
 
 int sas_request_addr(struct Scsi_Host *shost, u8 *addr)
 {
+#ifdef CONFIG_FW_LOADER
 	int res;
 	const struct firmware *fw;

drivers/scsi/libsas/sas_scsi_host.c int sas_request_addr(struct Scsi_Host *shost, u8 *addr) · line 1110

Quoted 8 lines
drivers/scsi/libsas/sas_scsi_host.c +3 -0
@@ -1102,6 +1103,9 @@ int sas_request_addr(struct Scsi_Host *shost, u8 *addr)
 out:
 	release_firmware(fw);
 	return res;
+#else
+	return -ENOENT;
+#endif
 }
 EXPORT_SYMBOL_GPL(sas_request_addr);

We could do it that way ... I suspect what Greg was asking for was more like this:

James

---

include/linux/firmware.h +23 -0
index 33d8f20..4d10c73 100644
@@ -10,7 +10,10 @@ struct firmware {
 	size_t size;
 	u8 *data;
 };
+
 struct device;
+
+#if defined(CONFIG_FW_LOADER) || defined(CONFIG_FW_LOADER_MODULE)
 int request_firmware(const struct firmware **fw, const char *name,
 		     struct device *device);
 int request_firmware_nowait(

@@ -19,4 +22,24 @@ int request_firmware_nowait(
 	void (*cont)(const struct firmware *fw, void *context));
 
 void release_firmware(const struct firmware *fw);
+#else
+static inline int request_firmware(const struct firmware **fw,
+				   const char *name,
+				   struct device *device)
+{
+	return -EINVAL;
+}
+static inline int request_firmware_nowait(
+	struct module *module, int uevent,
+	const char *name, struct device *device, void *context,
+	void (*cont)(const struct firmware *fw, void *context))
+{
+	return -EINVAL;
+}
+
+static inline void release_firmware(const struct firmware *fw)
+{
+}
+#endif
+
 #endif
author
#6 lore raw

Re: [PATCH] libsas: Check that the firmware loader is present in sas_request_addr

Quoted 22 lines · James Bottomley
Quoted 21 lines · Darrick J. Wong

Return an error code in sas_request_addr if the fw loader isn't configured.

Signed-off-by Darrick J. Wong <[email protected]>

---

 drivers/scsi/libsas/sas_scsi_host.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
drivers/scsi/libsas/sas_scsi_host.c +1 -0
index 601ec5b..e44be7a 100644
@@ -1085,6 +1085,7 @@ static void sas_parse_addr(u8 *sas_addr, const char *p)
 
 int sas_request_addr(struct Scsi_Host *shost, u8 *addr)
 {
+#ifdef CONFIG_FW_LOADER
 	int res;
 	const struct firmware *fw;
Quoted 12 lines

drivers/scsi/libsas/sas_scsi_host.c int sas_request_addr(struct Scsi_Host *shost, u8 *addr) · line 1110

Quoted 9 lines
drivers/scsi/libsas/sas_scsi_host.c +3 -0
@@ -1102,6 +1103,9 @@ int sas_request_addr(struct Scsi_Host *shost, u8 *addr)
 out:
 	release_firmware(fw);
 	return res;
+#else
+	return -ENOENT;
+#endif
 }
 EXPORT_SYMBOL_GPL(sas_request_addr);

We could do it that way ... I suspect what Greg was asking for was more like this:

Yes, it seems that this is needed. Lots of drivers use request_firmware(). Oooooh, and they select FW_LOADER. :( That's how they "get around" this problem.

Quoted 46 lines

James

---

include/linux/firmware.h +23 -0
index 33d8f20..4d10c73 100644
@@ -10,7 +10,10 @@ struct firmware {
 	size_t size;
 	u8 *data;
 };
+
 struct device;
+
+#if defined(CONFIG_FW_LOADER) || defined(CONFIG_FW_LOADER_MODULE)
 int request_firmware(const struct firmware **fw, const char *name,
 		     struct device *device);
 int request_firmware_nowait(

@@ -19,4 +22,24 @@ int request_firmware_nowait(
 	void (*cont)(const struct firmware *fw, void *context));
 
 void release_firmware(const struct firmware *fw);
+#else
+static inline int request_firmware(const struct firmware **fw,
+				   const char *name,
+				   struct device *device)
+{
+	return -EINVAL;
+}
+static inline int request_firmware_nowait(
+	struct module *module, int uevent,
+	const char *name, struct device *device, void *context,
+	void (*cont)(const struct firmware *fw, void *context))
+{
+	return -EINVAL;
+}
+
+static inline void release_firmware(const struct firmware *fw)
+{
+}
+#endif
+
 #endif
Signature · 1 line
~Randy
author Acked-by
#7 lore raw

Re: [PATCH] libsas: Check that the firmware loader is present in sas_request_addr

In reply to James Bottomley
Quoted 4 lines · On Thu, 06 Mar 2008 15:17:13 -0600 James Bottomley wrote:

We could do it that way ... I suspect what Greg was asking for was more like this:

James

Builds cleanly for CONFIG_FW_LOADER=n.
I'm having trouble setting CONFIG_FW_LOADER=m for test building it. Anyway,

Acked-by Randy Dunlap <[email protected]>
Quoted 32 lines

---

include/linux/firmware.h +14 -0
index 33d8f20..4d10c73 100644
@@ -10,7 +10,10 @@ struct firmware {
 	size_t size;
 	u8 *data;
 };
+
 struct device;
+
+#if defined(CONFIG_FW_LOADER) || defined(CONFIG_FW_LOADER_MODULE)
 int request_firmware(const struct firmware **fw, const char *name,
 		     struct device *device);
 int request_firmware_nowait(

@@ -19,4 +22,24 @@ int request_firmware_nowait(
 	void (*cont)(const struct firmware *fw, void *context));
 
 void release_firmware(const struct firmware *fw);
+#else
+static inline int request_firmware(const struct firmware **fw,
+				   const char *name,
+				   struct device *device)
+{
+	return -EINVAL;
+}
+static inline int request_firmware_nowait(
+	struct module *module, int uevent,
+	const char *name, struct device *device, void *context,
+	void (*cont)(const struct firmware *fw, void *context))

include/linux/firmware.h int request_firmware_nowait( · line 45

Quoted 10 lines
include/linux/firmware.h +9 -0
+{
+	return -EINVAL;
+}
+
+static inline void release_firmware(const struct firmware *fw)
+{
+}
+#endif
+
 #endif

---
~Randy

maintainer
#8 lore raw

Re: [PATCH] libsas: Check that the firmware loader is present in sas_request_addr

Quoted 9 lines · Randy Dunlap
Quoted 4 lines · On Thu, 06 Mar 2008 15:17:13 -0600 James Bottomley wrote:

We could do it that way ... I suspect what Greg was asking for was more like this:

James

Builds cleanly for CONFIG_FW_LOADER=n.
I'm having trouble setting CONFIG_FW_LOADER=m for test building it.

Ironically, I can, and have, tested that config ... I just couldn't build with CONFIG_FW_LOADER=n.

I think we've covered all the test cases between us.

Quoted 2 lines

Anyway,

Acked-by Randy Dunlap <[email protected]>

Thanks.

James

#9 lore raw

Re: [PATCH] libsas: Check that the firmware loader is present in sas_request_addr

Quoted 14 lines · James Bottomley
Quoted 9 lines · Randy Dunlap
Quoted 4 lines · On Thu, 06 Mar 2008 15:17:13 -0600 James Bottomley wrote:

We could do it that way ... I suspect what Greg was asking for was more like this:

James

Builds cleanly for CONFIG_FW_LOADER=n.
I'm having trouble setting CONFIG_FW_LOADER=m for test building it.

Ironically, I can, and have, tested that config ... I just couldn't build with CONFIG_FW_LOADER=n.

Quoted 6 lines

I think we've covered all the test cases between us.

Quoted 2 lines

Anyway,

Acked-by Randy Dunlap <[email protected]>

Thanks.

Yes, this is what I was thinking was needed.

Care to resend it with a signed-off-by?

thanks,

greg k-h

maintainer
#10 lore raw

PATCH 1/1 firmware: provide stubs for the FW_LOADER=n case

libsas has a case where it uses the firmware loader to provide services, but doesn't want to select it all the time. This currently causes a compile failure in libsas if FW_LOADER=n. Fix this by providing error stubs for the firmware loader API in the FW_LOADER=n case.

Signed-off-by James Bottomley <[email protected]>

---

Quoted 5 lines · Greg KH

Yes, this is what I was thinking was needed.

Care to resend it with a signed-off-by?

thanks,

Sure, attached.

James

include/linux/firmware.h +23 -0
index 33d8f20..4d10c73 100644
@@ -10,7 +10,10 @@ struct firmware {
 	size_t size;
 	u8 *data;
 };
+
 struct device;
+
+#if defined(CONFIG_FW_LOADER) || defined(CONFIG_FW_LOADER_MODULE)
 int request_firmware(const struct firmware **fw, const char *name,
 		     struct device *device);
 int request_firmware_nowait(

@@ -19,4 +22,24 @@ int request_firmware_nowait(
 	void (*cont)(const struct firmware *fw, void *context));
 
 void release_firmware(const struct firmware *fw);
+#else
+static inline int request_firmware(const struct firmware **fw,
+				   const char *name,
+				   struct device *device)
+{
+	return -EINVAL;
+}
+static inline int request_firmware_nowait(
+	struct module *module, int uevent,
+	const char *name, struct device *device, void *context,
+	void (*cont)(const struct firmware *fw, void *context))
+{
+	return -EINVAL;
+}
+
+static inline void release_firmware(const struct firmware *fw)
+{
+}
+#endif
+
 #endif
Applied
#11 lore raw

patch firmware-provide-stubs-for-the-fw_loader-n-case.patch added to gregkh-2.6 tree

This is a note to let you know that I've just added the patch titled

Subject: firmware: provide stubs for the FW_LOADER=n case

to my gregkh-2.6 tree. Its filename is

firmware-provide-stubs-for-the-fw_loader-n-case.patch

This tree can be found at
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/

Quoted 1 line

From [email protected] Fri Mar 7 08:30:11 2008

From: James Bottomley <[email protected]>
Date: Fri, 07 Mar 2008 08:57:54 -0600
Subject: firmware: provide stubs for the FW_LOADER=n case
To: Greg KH <[email protected]>
Cc: Randy Dunlap <[email protected]>, "Darrick J. Wong" <[email protected]>, [email protected], scsi <[email protected]>
Message-ID: <[email protected]>

libsas has a case where it uses the firmware loader to provide services, but doesn't want to select it all the time. This currently causes a compile failure in libsas if FW_LOADER=n. Fix this by providing error stubs for the firmware loader API in the FW_LOADER=n case.

Signed-off-by James Bottomley <[email protected]>
Cc Randy Dunlap <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 include/linux/firmware.h |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
include/linux/firmware.h +23 -0
@@ -10,7 +10,10 @@ struct firmware {
 	size_t size;
 	u8 *data;
 };
+
 struct device;
+
+#if defined(CONFIG_FW_LOADER) || defined(CONFIG_FW_LOADER_MODULE)
 int request_firmware(const struct firmware **fw, const char *name,
 		     struct device *device);
 int request_firmware_nowait(

@@ -19,4 +22,24 @@ int request_firmware_nowait(
 	void (*cont)(const struct firmware *fw, void *context));
 
 void release_firmware(const struct firmware *fw);
+#else
+static inline int request_firmware(const struct firmware **fw,
+				   const char *name,
+				   struct device *device)
+{
+	return -EINVAL;
+}
+static inline int request_firmware_nowait(
+	struct module *module, int uevent,
+	const char *name, struct device *device, void *context,
+	void (*cont)(const struct firmware *fw, void *context))
+{
+	return -EINVAL;
+}
+
+static inline void release_firmware(const struct firmware *fw)
+{
+}
+#endif
+
 #endif

Patches currently in gregkh-2.6 which might be from [email protected] are

pci/pci-remove-parisc-consumer-of-the-pci-global_list.patch usb/usb-enable-usb-persist-by-default.patch driver-core/firmware-provide-stubs-for-the-fw_loader-n-case.patch