sr cd->device->writeable without purpose
Subsystems (2)
PATCH 1/1 sr cd->device->writeable without purpose
Quoted 4 lines
http://marc.theaimsgroup.com/?l=linux-scsi&m=106813641032686
List: linux-scsi Subject: Re: [PATCH] CDC_MMC_WR Date: 2003-11-06 16:32:35
The inline text patch of this email concretely demoes why I think the sr cd->device->writeable bit becomes vestigial - has no remaining purpose - once we branch on CDC_MMC_WR instead of cd->device->writeable to decide to pass thru or reject writes.
This patch differs from that simpler CDC_MMC_WR patch by updating sr cd->device->writeable bit whenever cdrom corrects the sr guess of CDC_MMC_WR via op x46 GPCMD_GET_CONFIGURATION.
But as far as I can tell, Linux works no better when I add this complication. Can anyone educate me differently? I need to learn something new before I can understand that offline suggestion of "have the ->writeable bit actually be set if the device is writeable".
Running this code against ATAPI/USB DVD/CD of various kinds, I notice sr_cdi_notify always clears cd->device->writeable. This consequence arises naturally from the fact that this patch calls cdo->cdi_notify only after cdi->mask |= CDC_MMC_WR.
Pat LaVarre x4402
drivers/cdrom/cdrom.c +56 -1
@@ -313,6 +313,7 @@#define CHECKAUDIO if ((ret=check_for_audio_disc(cdi, cdo))) return ret/* Not-exported routines. */ +static void cdrom_get_cdc (struct cdrom_device_info *cdi);static int open_for_data (struct cdrom_device_info * cdi);static int check_for_audio_disc (struct cdrom_device_info * cdi,struct cdrom_device_ops * cdo);
@@ -365,6 +366,7 @@ENSURE (audio_ioctl,CDC_PLAY_AUDIO );ENSURE (dev_ioctl,CDC_IOCTLS );ENSURE (generic_packet,CDC_GENERIC_PACKET ); +/* ENSURE(cdi_notify, CDC_); */ cdi->mc_flags =0 ; cdo->n_minors =0 ; cdi->options =CDO_USE_FFLAGS ;
@@ -381,6 +383,8 @@cdinfo (CD_REG_UNREG ,"drive \"/dev/%s\" registered\n" , cdi->name); cdi->next = topCdromPtr; topCdromPtr = cdi; + +cdrom_get_cdc (cdi);return 0 ; }#undef ENSURE
@@ -408,6 +412,52 @@return 0 ; } +static int cdrom_get_configuration (struct cdrom_device_info *cdi) +{ +struct cdrom_generic_command cgc; +struct cdrom_device_ops *cdo = cdi->ops; +u_char buf[8 ]; +int ret; +init_cdrom_command (&cgc, buf,sizeof (buf),CGC_DATA_READ ); + cgc.cmd[0 ] =GPCMD_GET_CONFIGURATION ; + cgc.cmd[7 ] = cgc.buflen >>8 ; + cgc.cmd[8 ] = cgc.buflen &0xff ; + cgc.data_direction =CGC_DATA_READ ; + ret = cdo->generic_packet (cdi, &cgc); +if (!ret) { +int profile = buf[6 ] <<8 | buf[7 ]; +cdinfo (CD_REG_UNREG ,"/dev/%s: " +"MMC profile x%04X\n" , cdi->name, profile); +switch (profile) { +case 0x0001 :/* Non-Removable Disk */ +case 0x0002 :/* Removable Disk */ +case 0x0003 :/* Magneto-Optical Erasable */ +case 0x0005 :/* AS-MO */ +case 0x0012 :/* DVD-RAM */ +case 0x0022 :/* DDCD-RW */ +return 0 ; +case 0x001A :/* DVD+RW = not much rewrite in place */ +default : +break ; + } + } + cdi->mask |=CDC_MMC_WR ; +if (cdo->cdi_notify !=NULL ) cdo->cdi_notify (cdi); +return ret;/* ret may here be zero */ +} + +static void cdrom_get_cdc (struct cdrom_device_info *cdi) +{ +if (CDROM_CAN (CDC_MMC_WR ) && !CDROM_CAN (CDC_DVD_RAM )) { +cdinfo (CD_REG_UNREG ,"/dev/%s: " +"cdrom_get_configuration\n" , cdi->name); +if (cdrom_get_configuration (cdi)) { +cdinfo (CD_REG_UNREG ,"/dev/%s: " +"cdrom_get_configuration not\n" , cdi->name); + } + } +} +/* We use the open-option O_NONBLOCK to indicate that the * purpose of opening is only for subsequent ioctl() calls; no device * integrity checks are performed.
@@ -426,8 +476,9 @@if ((fp->f_flags &O_NONBLOCK ) && (cdi->options &CDO_USE_FFLAGS )) ret = cdi->ops->open (cdi,1 );else { -if ((fp->f_mode &FMODE_WRITE ) && !CDROM_CAN (CDC_DVD_RAM )) +if ((fp->f_mode &FMODE_WRITE ) && !CDROM_CAN (CDC_MMC_WR )) {return -EROFS ; + } ret =open_for_data (cdi); }
@@ -2406,6 +2457,10 @@for (cdi=topCdromPtr;cdi!=NULL ;cdi=cdi->next) pos +=sprintf (info+pos,"\t%d" ,CDROM_CAN (CDC_DVD_RAM ) !=0 ); + pos +=sprintf (info+pos,"\nTolerates random write:" ); +for (cdi=topCdromPtr;cdi!=NULL ;cdi=cdi->next) + pos +=sprintf (info+pos,"\t%d" ,CDROM_CAN (CDC_MMC_WR ) !=0 ); +strcpy (info+pos,"\n\n" );return proc_dostring (ctl, write, filp, buffer, lenp);
include/linux/cdrom.h +2 -0
@@ -388,6 +388,7 @@#define CDC_DVD_R 0x10000/* drive can write DVD-R */ #define CDC_DVD_RAM 0x20000/* drive can write DVD-RAM */ #define CDC_MO_DRIVE 0x40000/* drive is an MO device */ +#define CDC_MMC_WR 0x80000/* profile includes random write */ /* drive status possibilities returned by CDROM_DRIVE_STATUS ioctl */ #define CDS_NO_INFO 0/* if not implemented */
@@ -773,6 +774,7 @@/* handle uniform packets for scsi type devices (scsi,atapi) */ int (*generic_packet) (struct cdrom_device_info *,struct cdrom_generic_command *); +void (*cdi_notify) (struct cdrom_device_info *); };/* the general block_device operations structure: */
drivers/ide/ide-cd.c +2 -1
@@ -2822,7 +2822,8 @@CDC_MEDIA_CHANGED |CDC_PLAY_AUDIO |CDC_RESET |CDC_IOCTLS |CDC_DRIVE_STATUS |CDC_CD_R |CDC_CD_RW |CDC_DVD |CDC_DVD_R |CDC_DVD_RAM | - CDC_GENERIC_PACKET | CDC_MO_DRIVE, + CDC_GENERIC_PACKET | CDC_MO_DRIVE | CDC_MMC_WR, + .cdi_notify = NULL, .generic_packet = ide_cdrom_packet, };
drivers/scsi/sr.c +20 -4
@@ -67,7 +67,8 @@ (CDC_CLOSE_TRAY |CDC_OPEN_TRAY |CDC_LOCK |CDC_SELECT_SPEED | \CDC_SELECT_DISC |CDC_MULTI_SESSION |CDC_MCN |CDC_MEDIA_CHANGED | \CDC_PLAY_AUDIO |CDC_RESET |CDC_IOCTLS |CDC_DRIVE_STATUS | \ - CDC_CD_R|CDC_CD_RW|CDC_DVD|CDC_DVD_R|CDC_DVD_RAM|CDC_GENERIC_PACKET) +CDC_CD_R |CDC_CD_RW |CDC_DVD |CDC_DVD_R |CDC_DVD_RAM |CDC_GENERIC_PACKET | \ + CDC_MMC_WR)static int sr_probe (struct device *);static int sr_remove (struct device *);
@@ -94,6 +95,7 @@static int sr_media_change (struct cdrom_device_info *,int );static int sr_packet (struct cdrom_device_info *,struct cdrom_generic_command *); +static void sr_cdi_notify (struct cdrom_device_info *);static struct cdrom_device_ops sr_dops = { .open = sr_open,
@@ -110,9 +112,23 @@ .dev_ioctl = sr_dev_ioctl, .capability = SR_CAPABILITIES, .generic_packet = sr_packet, + .cdi_notify = sr_cdi_notify, };/* +* sr_cdi_notify carelessly accepts any cdi change whatsoever, +* and copies CDC_MMC_WR-not-masked into writeable more often than useful. +*/ + +static void sr_cdi_notify (struct cdrom_device_info *cdi) +{ +struct scsi_cd *cd = cdi->handle; + cd->device->writeable = ((cd->cdi.mask &CDC_MMC_WR ) ==0 ); +printk ("/dev/%s: sr_cdi_notify: writeable %d\n" , + cdi->name, cd->device->writeable); +} + +/* * This function checks to see if the media has been changed in the * CDROM drive. It is possible that we have already sensed a change, * or the drive may have sensed one and not yet reported it. We must
@@ -327,8 +343,9 @@ }if (rq_data_dir (SCpnt->request) ==WRITE ) { -if (!cd->device->writeable) +if ((cd->cdi.mask &CDC_MMC_WR ) !=0 ) {return 0 ; + } SCpnt->cmnd[0 ] =WRITE_10 ; SCpnt->sc_data_direction =SCSI_DATA_WRITE ; }else if (rq_data_dir (SCpnt->request) ==READ ) {
@@ -557,6 +574,7 @@ sdev->sector_size =2048 ;/* A guess, just in case */ /* FIXME: need to handle a get_capabilities failure properly ?? */ + cd->device->writeable =1 ;/* ((cd->cdi.mask & CDC_MMC_WR) == 0); */ get_capabilities (cd);sr_vendor_init (cd);
@@ -788,8 +806,6 @@if ((buffer[n +3 ] &0x20 ) ==0 ) {/* can't write DVD-RAM media */ cd->cdi.mask |=CDC_DVD_RAM ; - }else { - cd->device->writeable =1 ; }if ((buffer[n +3 ] &0x10 ) ==0 )/* can't write DVD-R media */
Kindly offline I hear:
Quoted 3 lines
I think the sr
cd->device->writeablebit becomes vestigial - has no remaining purpose - once we branch onCDC_MMC_WRinstead ofcd->device->writeableto decide to pass thru or reject writes.
That might or might not be true. However,
Quoted 3 lines
This patch differs from that simpler
CDC_MMC_WRpatch by updating srcd->device->writeablebit whenever cdrom corrects the sr guess ofCDC_MMC_WRvia op x46GPCMD_GET_CONFIGURATION.
Ouch, rather silly of me. In that patch, me the newbie, I foolishly:
a) grew cdrom_device_ops to call back into sr ...
b) just before cdrom_get_configuration returns to cdrom_get_cdc, ...
c) just before cdrom_get_cdc returns to register_cdrom, ...
d) just before register_cdrom returns to sr.sr_probe.
That is, I can run the same sr code at almost precisely the same time if only I place that code immediately after the call to register_cdrom in sr.sr_probe.
D'oh.
Pat LaVarre