From patchwork Mon Feb 9 03:59:48 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Bottomley X-Patchwork-Id: 22647 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from ozlabs.org (localhost [127.0.0.1]) by ozlabs.org (Postfix) with ESMTP id 25DE9DDDB5 for ; Mon, 9 Feb 2009 15:07:20 +1100 (EST) X-Original-To: linuxppc-dev@ozlabs.org Delivered-To: linuxppc-dev@ozlabs.org X-Greylist: delayed 322 seconds by postgrey-1.31 at ozlabs; Mon, 09 Feb 2009 15:05:15 EST Received: from accolon.hansenpartnership.com (accolon.hansenpartnership.com [76.243.235.52]) by ozlabs.org (Postfix) with ESMTP id 4487BDDDA5 for ; Mon, 9 Feb 2009 15:05:15 +1100 (EST) Received: from localhost (localhost [127.0.0.1]) by accolon.hansenpartnership.com (Postfix) with ESMTP id 282E382EE; Sun, 8 Feb 2009 21:59:49 -0600 (CST) Received: from accolon.hansenpartnership.com ([127.0.0.1]) by localhost (redscar.int.hansenpartnership.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xlvcGzK48Aql; Sun, 8 Feb 2009 21:59:47 -0600 (CST) Received: from [153.66.150.222] (mulgrave-w.int.hansenpartnership.com [153.66.150.222]) by accolon.hansenpartnership.com (Postfix) with ESMTP id 487217F95; Sun, 8 Feb 2009 21:59:47 -0600 (CST) Subject: Re: [patch] powerpc/ps3: Use hard coded values for LV1 device type From: James Bottomley To: Benjamin Herrenschmidt In-Reply-To: <1234138267.31963.77.camel@pasglop> References: <498C6C49.1010200@in.ibm.com> <498CF52E.4040903@am.sony.com> <1234092590.10240.4.camel@localhost> <1234138267.31963.77.camel@pasglop> Date: Sun, 08 Feb 2009 21:59:48 -0600 Message-Id: <1234151988.8776.4.camel@localhost.localdomain> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 (2.22.3.1-1.fc9) Cc: linux-scsi , Mel Gorman , Kamalesh Babulal , linuxppc-dev@ozlabs.org, Jens Axboe X-BeenThere: linuxppc-dev@ozlabs.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@ozlabs.org Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@ozlabs.org On Mon, 2009-02-09 at 11:11 +1100, Benjamin Herrenschmidt wrote: > On Sun, 2009-02-08 at 22:29 +1100, Michael Ellerman wrote: > > On Fri, 2009-02-06 at 18:42 -0800, Geoff Levand wrote: > > > Change the PS3 platform code to use hard coded numbers for its > > > LV1 device types. > > > > > > The PS3 platform code was incorrectly using some scsi block > > > constants for the device type returned from the LV1 hypervisor. > > > > > > Fixes build errors like these when CONFIG_BLOCK=n: > > > > > > In file included from include/scsi/scsi.h:12, > > > from arch/powerpc/platforms/ps3/platform.h:25, > > > from arch/powerpc/platforms/ps3/setup.c:36: > > > include/scsi/scsi_cmnd.h:27:25: warning: "BLK_MAX_CDB" is not defined > > > include/scsi/scsi_cmnd.h:28:3: error: #error MAX_COMMAND_SIZE can not be bigger than BLK_MAX_CDB > > Adding Jens and James on CC since I think a proper fix lies in blkdev.h > or scsi*.h And cc'd linux-scsi > So basically, the whole of blkdev.h is inside a big ifdef > CONFIG_BLOCK... which means that scsi_cmnd.h can't build which in turn > makes scsi.h fail. Well, look at it from our point of view; it's impossible to build SCSI without block, so a little interdependence is easy to get. > The PS3 platform code wants to use some of the standard SCSI types from > there though, as they are part of the hypervisor ABI. (And in fact it > can be argued that non-block devices using SCSI do exist, such as > scanners, no ?) > > Any reason other than pre-historical to have blkdev.h shielded like > that ? Actually, I think the fix lies in scsi.h ... we can make that into a nicely independent protocol header file. Your current woes come because it pulls in scsi_cmnd.h ... perhaps just getting rid of this will fix it. Can the rest of linux-scsi verify that the fix below doesn't break something else? I found one cockup: block/cmd-filter.c is apparently not including linuc/blkdev.h directly but via scsi/scsi.h ... I fixed this up. > Cheers, > Ben. > > > > Signed-off-by: Geoff Levand > > > --- > > > Ben, > > > > > > Please send upstream for 2.6.29. > > > > > > -Geoff > > > > > > arch/powerpc/platforms/ps3/platform.h | 8 +++----- > > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > > > --- a/arch/powerpc/platforms/ps3/platform.h > > > +++ b/arch/powerpc/platforms/ps3/platform.h > > > @@ -22,8 +22,6 @@ > > > #define _PS3_PLATFORM_H > > > > > > #include > > > -#include > > > - > > > #include > > > > > > /* htab */ > > > @@ -83,12 +81,12 @@ enum ps3_bus_type { > > > }; > > > > > > enum ps3_dev_type { > > > - PS3_DEV_TYPE_STOR_DISK = TYPE_DISK, /* 0 */ > > > + PS3_DEV_TYPE_STOR_DISK = 0, /* TYPE_DISK */ > > > PS3_DEV_TYPE_SB_GELIC = 3, > > > PS3_DEV_TYPE_SB_USB = 4, > > > - PS3_DEV_TYPE_STOR_ROM = TYPE_ROM, /* 5 */ > > > + PS3_DEV_TYPE_STOR_ROM = 5, /* TYPE_ROM */ > > > PS3_DEV_TYPE_SB_GPIO = 6, > > > - PS3_DEV_TYPE_STOR_FLASH = TYPE_RBC, /* 14 */ > > > + PS3_DEV_TYPE_STOR_FLASH = 14, /* TYPE_RBC */ > > > > This looks like you're just papering over the bug, by hardcoding the > > same values that are in the scsi header. Or are they really independent, > > in which case I'd say the comments are confusing. > > > > cheers James Acked-by: Geoff Levand diff --git a/block/cmd-filter.c b/block/cmd-filter.c index 504b275..572bbc2 100644 --- a/block/cmd-filter.c +++ b/block/cmd-filter.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h index 80d7f60..084478e 100644 --- a/include/scsi/scsi.h +++ b/include/scsi/scsi.h @@ -9,7 +9,8 @@ #define _SCSI_SCSI_H #include -#include + +struct scsi_cmnd; /* * The maximum number of SG segments that we will put inside a @@ -439,22 +440,6 @@ static inline int scsi_is_wlun(unsigned int lun) #define host_byte(result) (((result) >> 16) & 0xff) #define driver_byte(result) (((result) >> 24) & 0xff) -static inline void set_msg_byte(struct scsi_cmnd *cmd, char status) -{ - cmd->result |= status << 8; -} - -static inline void set_host_byte(struct scsi_cmnd *cmd, char status) -{ - cmd->result |= status << 16; -} - -static inline void set_driver_byte(struct scsi_cmnd *cmd, char status) -{ - cmd->result |= status << 24; -} - - #define sense_class(sense) (((sense) >> 4) & 0x7) #define sense_error(sense) ((sense) & 0xf) #define sense_valid(sense) ((sense) & 0x80); diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h index 855bf95..43b50d3 100644 --- a/include/scsi/scsi_cmnd.h +++ b/include/scsi/scsi_cmnd.h @@ -291,4 +291,19 @@ static inline struct scsi_data_buffer *scsi_prot(struct scsi_cmnd *cmd) #define scsi_for_each_prot_sg(cmd, sg, nseg, __i) \ for_each_sg(scsi_prot_sglist(cmd), sg, nseg, __i) +static inline void set_msg_byte(struct scsi_cmnd *cmd, char status) +{ + cmd->result |= status << 8; +} + +static inline void set_host_byte(struct scsi_cmnd *cmd, char status) +{ + cmd->result |= status << 16; +} + +static inline void set_driver_byte(struct scsi_cmnd *cmd, char status) +{ + cmd->result |= status << 24; +} + #endif /* _SCSI_SCSI_CMND_H */