* next-2008-0306 scsi/libsas build failure @ 2008-03-06 17:09 Randy Dunlap 2008-03-06 18:16 ` Greg KH 0 siblings, 1 reply; 12+ messages in thread From: Randy Dunlap @ 2008-03-06 17:09 UTC (permalink / raw) To: linux-next, scsi; +Cc: jejb 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: next-2008-0306 scsi/libsas build failure 2008-03-06 17:09 next-2008-0306 scsi/libsas build failure Randy Dunlap @ 2008-03-06 18:16 ` Greg KH 2008-03-06 18:22 ` Randy Dunlap 0 siblings, 1 reply; 12+ messages in thread From: Greg KH @ 2008-03-06 18:16 UTC (permalink / raw) To: Randy Dunlap; +Cc: linux-next, scsi, jejb On Thu, Mar 06, 2008 at 09:09:04AM -0800, Randy Dunlap wrote: > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: next-2008-0306 scsi/libsas build failure 2008-03-06 18:16 ` Greg KH @ 2008-03-06 18:22 ` Randy Dunlap 2008-03-06 21:01 ` Darrick J. Wong 2008-03-06 21:02 ` [PATCH] libsas: Check that the firmware loader is present in sas_request_addr Darrick J. Wong 0 siblings, 2 replies; 12+ messages in thread From: Randy Dunlap @ 2008-03-06 18:22 UTC (permalink / raw) To: Greg KH; +Cc: linux-next, scsi, jejb Greg KH wrote: > On Thu, Mar 06, 2008 at 09:09:04AM -0800, Randy Dunlap wrote: >> 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? That would be one (good) solution. I'll wait to see if James or Darrick have other ideas... -- ~Randy ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: next-2008-0306 scsi/libsas build failure 2008-03-06 18:22 ` Randy Dunlap @ 2008-03-06 21:01 ` Darrick J. Wong 2008-03-06 21:02 ` [PATCH] libsas: Check that the firmware loader is present in sas_request_addr Darrick J. Wong 1 sibling, 0 replies; 12+ messages in thread From: Darrick J. Wong @ 2008-03-06 21:01 UTC (permalink / raw) To: Randy Dunlap; +Cc: Greg KH, linux-next, scsi, jejb >> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] libsas: Check that the firmware loader is present in sas_request_addr 2008-03-06 18:22 ` Randy Dunlap 2008-03-06 21:01 ` Darrick J. Wong @ 2008-03-06 21:02 ` Darrick J. Wong 2008-03-06 21:17 ` James Bottomley 1 sibling, 1 reply; 12+ messages in thread From: Darrick J. Wong @ 2008-03-06 21:02 UTC (permalink / raw) To: Randy Dunlap; +Cc: Greg KH, linux-next, scsi, jejb Return an error code in sas_request_addr if the fw loader isn't configured. Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> --- drivers/scsi/libsas/sas_scsi_host.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c index 601ec5b..e44be7a 100644 --- a/drivers/scsi/libsas/sas_scsi_host.c +++ b/drivers/scsi/libsas/sas_scsi_host.c @@ -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); ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] libsas: Check that the firmware loader is present in sas_request_addr 2008-03-06 21:02 ` [PATCH] libsas: Check that the firmware loader is present in sas_request_addr Darrick J. Wong @ 2008-03-06 21:17 ` James Bottomley 2008-03-06 21:20 ` Randy Dunlap 2008-03-06 22:04 ` Randy Dunlap 0 siblings, 2 replies; 12+ messages in thread From: James Bottomley @ 2008-03-06 21:17 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Randy Dunlap, Greg KH, linux-next, scsi On Thu, 2008-03-06 at 13:02 -0800, Darrick J. Wong wrote: > Return an error code in sas_request_addr if the fw loader isn't > configured. > > Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> > --- > > drivers/scsi/libsas/sas_scsi_host.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c > index 601ec5b..e44be7a 100644 > --- a/drivers/scsi/libsas/sas_scsi_host.c > +++ b/drivers/scsi/libsas/sas_scsi_host.c > @@ -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); We could do it that way ... I suspect what Greg was asking for was more like this: James --- diff --git a/include/linux/firmware.h b/include/linux/firmware.h index 33d8f20..4d10c73 100644 --- a/include/linux/firmware.h +++ b/include/linux/firmware.h @@ -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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] libsas: Check that the firmware loader is present in sas_request_addr 2008-03-06 21:17 ` James Bottomley @ 2008-03-06 21:20 ` Randy Dunlap 2008-03-06 22:04 ` Randy Dunlap 1 sibling, 0 replies; 12+ messages in thread From: Randy Dunlap @ 2008-03-06 21:20 UTC (permalink / raw) To: James Bottomley; +Cc: Darrick J. Wong, Greg KH, linux-next, scsi James Bottomley wrote: > On Thu, 2008-03-06 at 13:02 -0800, Darrick J. Wong wrote: >> Return an error code in sas_request_addr if the fw loader isn't >> configured. >> >> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> >> --- >> >> drivers/scsi/libsas/sas_scsi_host.c | 4 ++++ >> 1 files changed, 4 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c >> index 601ec5b..e44be7a 100644 >> --- a/drivers/scsi/libsas/sas_scsi_host.c >> +++ b/drivers/scsi/libsas/sas_scsi_host.c >> @@ -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); > > 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. > James > > --- > > diff --git a/include/linux/firmware.h b/include/linux/firmware.h > index 33d8f20..4d10c73 100644 > --- a/include/linux/firmware.h > +++ b/include/linux/firmware.h > @@ -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 > > -- ~Randy ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] libsas: Check that the firmware loader is present in sas_request_addr 2008-03-06 21:17 ` James Bottomley 2008-03-06 21:20 ` Randy Dunlap @ 2008-03-06 22:04 ` Randy Dunlap 2008-03-06 22:16 ` James Bottomley 1 sibling, 1 reply; 12+ messages in thread From: Randy Dunlap @ 2008-03-06 22:04 UTC (permalink / raw) To: James Bottomley; +Cc: Darrick J. Wong, Greg KH, linux-next, scsi 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 <randy.dunlap@oracle.com> > --- > > diff --git a/include/linux/firmware.h b/include/linux/firmware.h > index 33d8f20..4d10c73 100644 > --- a/include/linux/firmware.h > +++ b/include/linux/firmware.h > @@ -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 --- ~Randy ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] libsas: Check that the firmware loader is present in sas_request_addr 2008-03-06 22:04 ` Randy Dunlap @ 2008-03-06 22:16 ` James Bottomley 2008-03-07 4:54 ` Greg KH 0 siblings, 1 reply; 12+ messages in thread From: James Bottomley @ 2008-03-06 22:16 UTC (permalink / raw) To: Randy Dunlap; +Cc: Darrick J. Wong, Greg KH, linux-next, scsi On Thu, 2008-03-06 at 14:04 -0800, Randy Dunlap wrote: > 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. > Anyway, > Acked-by: Randy Dunlap <randy.dunlap@oracle.com> Thanks. James ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] libsas: Check that the firmware loader is present in sas_request_addr 2008-03-06 22:16 ` James Bottomley @ 2008-03-07 4:54 ` Greg KH 2008-03-07 14:57 ` [PATCH] firmware: provide stubs for the FW_LOADER=n case James Bottomley 0 siblings, 1 reply; 12+ messages in thread From: Greg KH @ 2008-03-07 4:54 UTC (permalink / raw) To: James Bottomley; +Cc: Randy Dunlap, Darrick J. Wong, linux-next, scsi On Thu, Mar 06, 2008 at 04:16:21PM -0600, James Bottomley wrote: > On Thu, 2008-03-06 at 14:04 -0800, Randy Dunlap wrote: > > 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. > > > Anyway, > > Acked-by: Randy Dunlap <randy.dunlap@oracle.com> > > Thanks. Yes, this is what I was thinking was needed. Care to resend it with a signed-off-by? thanks, greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] firmware: provide stubs for the FW_LOADER=n case 2008-03-07 4:54 ` Greg KH @ 2008-03-07 14:57 ` James Bottomley 2008-03-07 17:08 ` patch firmware-provide-stubs-for-the-fw_loader-n-case.patch added to gregkh-2.6 tree gregkh 0 siblings, 1 reply; 12+ messages in thread From: James Bottomley @ 2008-03-07 14:57 UTC (permalink / raw) To: Greg KH; +Cc: Randy Dunlap, Darrick J. Wong, linux-next, scsi 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 <James.Bottomley@HansenPartnership.com> --- On Thu, 2008-03-06 at 20:54 -0800, Greg KH wrote: > Yes, this is what I was thinking was needed. > > Care to resend it with a signed-off-by? > > thanks, Sure, attached. James diff --git a/include/linux/firmware.h b/include/linux/firmware.h index 33d8f20..4d10c73 100644 --- a/include/linux/firmware.h +++ b/include/linux/firmware.h @@ -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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* patch firmware-provide-stubs-for-the-fw_loader-n-case.patch added to gregkh-2.6 tree 2008-03-07 14:57 ` [PATCH] firmware: provide stubs for the FW_LOADER=n case James Bottomley @ 2008-03-07 17:08 ` gregkh 0 siblings, 0 replies; 12+ messages in thread From: gregkh @ 2008-03-07 17:08 UTC (permalink / raw) To: James.Bottomley, djwong, greg, gregkh, linux-scsi, randy.dunlap 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/ >From James.Bottomley@HansenPartnership.com Fri Mar 7 08:30:11 2008 From: James Bottomley <James.Bottomley@HansenPartnership.com> Date: Fri, 07 Mar 2008 08:57:54 -0600 Subject: firmware: provide stubs for the FW_LOADER=n case To: Greg KH <greg@kroah.com> Cc: Randy Dunlap <randy.dunlap@oracle.com>, "Darrick J. Wong" <djwong@us.ibm.com>, linux-next@vger.kernel.org, scsi <linux-scsi@vger.kernel.org> Message-ID: <1204901874.2889.3.camel@localhost.localdomain> 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 <James.Bottomley@HansenPartnership.com> Cc: Randy Dunlap <randy.dunlap@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> --- include/linux/firmware.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) --- a/include/linux/firmware.h +++ b/include/linux/firmware.h @@ -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 James.Bottomley@HansenPartnership.com 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2008-03-07 17:09 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-03-06 17:09 next-2008-0306 scsi/libsas build failure Randy Dunlap 2008-03-06 18:16 ` Greg KH 2008-03-06 18:22 ` Randy Dunlap 2008-03-06 21:01 ` Darrick J. Wong 2008-03-06 21:02 ` [PATCH] libsas: Check that the firmware loader is present in sas_request_addr Darrick J. Wong 2008-03-06 21:17 ` James Bottomley 2008-03-06 21:20 ` Randy Dunlap 2008-03-06 22:04 ` Randy Dunlap 2008-03-06 22:16 ` James Bottomley 2008-03-07 4:54 ` Greg KH 2008-03-07 14:57 ` [PATCH] firmware: provide stubs for the FW_LOADER=n case James Bottomley 2008-03-07 17:08 ` patch firmware-provide-stubs-for-the-fw_loader-n-case.patch added to gregkh-2.6 tree gregkh
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).