Re: [patch] powerpc/ps3: Use hard coded values for LV1 device type

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

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

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.

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 ?

Cheers,
Ben.

> > Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
> > ---
> > 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 <linux/rtc.h>
> > -#include <scsi/scsi.h>
> > -
> >  #include <asm/ps3.h>
> >  
> >  /* 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
>