Re: Optiarc DVD RW AD-5200A audio playing

[Stefan Bader]

Borislav Petkov wrote:
> On Sat, Feb 16, 2008 at 04:24:01PM +0100, Bartlomiej Zolnierkiewicz wrote:
>> Hi,
>>
>> On Saturday 16 February 2008, Borislav Petkov wrote:
>>> On Fri, Feb 15, 2008 at 02:53:27PM -0500, Stefan Bader wrote:
>>>> Hello Borislav,
>>>>
>>>> I worked on a problem with an DVD driver (model=Optiarc DVD RW AD-5200A)
>>>> which obviously has the same problem as some Matshita drives. The
>>>> following patch was reported to enabled audio playing on this drive.
>>>> Would this approach be suitable for upstream or are there other
>>>> solutions to this problem?
>>>>
>>>> Regards,
>>>> Stefan
>>>>
>>>> --- a/drivers/ide/ide-cd.c
>>>> +++ b/drivers/ide/ide-cd.c
>>>> @@ -2988,7 +2988,8 @@ int ide_cdrom_probe_capabilities (ide_drive_t *drive)
>>>>  	if (strcmp(drive->id->model, "MATSHITADVD-ROM SR-8187") == 0 ||
>>>>  	    strcmp(drive->id->model, "MATSHITADVD-ROM SR-8186") == 0 ||
>>>>  	    strcmp(drive->id->model, "MATSHITADVD-ROM SR-8176") == 0 ||
>>>> -	    strcmp(drive->id->model, "MATSHITADVD-ROM SR-8174") == 0)
>>>> +	    strcmp(drive->id->model, "MATSHITADVD-ROM SR-8174") == 0 ||
>>>> +	    strcmp(drive->id->model, "Optiarc DVD RW AD-5200A") == 0)
>>>>  		CDROM_CONFIG_FLAGS(drive)->audio_play = 1;
>>>>
>>>>  #if ! STANDARD_ATAPI
>>> Hi Stefan,
>>>
>>> just to make sure that the audioplay bit is not set in the capabilities page,
>>> can you please try the following patch applied against 2.6.25-rc2 and send me
>>> the output. Thanks!
>>>
>>> @Bart: by the way, this cdi->mask thingy is kinda unintuitive doing double
>>> negation to check whether a feature is supported or not. Yeah, this comes from
>>> "above," i.e. uniform cdrom layer. But still, shouldn't we use a cdi->caps_flags
>>> or something similar instead, which mirrors the caps page bits setting?
>> It seems so (at least having negative flags is very unintuitive) but they
>> might be some reason for this ugliness, Jens?
>>
>> [ Please also remember that since cdrom layer is _uniform_ it may be not
>>   possible and/or desirable to have 1-1 mapping between caps page bits
>>   and the future cdi->caps_flags. ]
>>
>>> commit 435f0f4496a1b32af2d542f43b2370a890fe2f83
>>> Author: Borislav Petkov <petkovbb@xxxxxxxxx>
>>> Date:   Sat Feb 16 09:56:36 2008 +0100
>>>
>>>     ide-cd: Enable audio play quirk for Optiarc DVD RW AD-5200A drive
>>>     
>>>     Reported-by: Stefan Bader <stefan.bader@xxxxxxxxxxxxx>
>>>     Signed-off-by: Borislav Petkov <petkovbb@xxxxxxxxx>
>>>
>>> diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
>>> index f77db6b..2c9d06e 100644
>>> --- a/drivers/ide/ide-cd.c
>>> +++ b/drivers/ide/ide-cd.c
>>> @@ -1750,6 +1750,10 @@ int ide_cdrom_probe_capabilities (ide_drive_t *drive)
>>>  		cdi->mask &= ~(CDC_DVD_RAM | CDC_RAM);
>>>  	if (buf[8 + 3] & 0x10)
>>>  		cdi->mask &= ~CDC_DVD_R;
>>> +	if (!(buf[8 + 4] & 0x01)) {
>> Hmm, shouldn't there be '&& (cd->cd_flags & IDE_CD_FLAG_PLAY_AUDIO_OK)'
>> to prevent false positives?
> 
> I wanted to see whether the caps page reports the audioplay bit off...
> 
>>> +		printk(KERN_INFO "ide-cd: audio play not advertised in caps page,"
>> Would be nice to also printk() the device name.
> 
> ... but printing the device model is actually a good idea and this will rule out
> false positives, so Stefan, please drop the previous patch and test the updated
> one below. Thanks.
> 
> 

Hi Borislav,

the problem is that I don't own this drive myself and the owner is
running a 2.6.22 kernel and is normally not doing any kernel compiles.
But I could provide him a modified patch.
Though, if you just want to know whether the cap bit was really unset, I
think we know this already. When I got the problem report we checked
/proc/sys/dev/cdrom/info and that showed the "Can play audio" bit as 0.
Which is the reason I gave the owner the patch for adding the model to
the excemption list. And from his feedback I take that the drive plays
audio tracks with the patch in use.

Stefan

> commit 6cc44b0ce5c9270b15d456eb9ffa91b855e4e0d0
> Author: Borislav Petkov <petkovbb@xxxxxxxxx>
> Date:   Sat Feb 16 09:56:36 2008 +0100
> 
>     ide-cd: Enable audio play quirk for Optiarc DVD RW AD-5200A drive
>     
>     Reported-by: Stefan Bader <stefan.bader@xxxxxxxxxxxxx>
>     Signed-off-by: Borislav Petkov <petkovbb@xxxxxxxxx>
> 
> diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
> index f77db6b..4c9984f 100644
> --- a/drivers/ide/ide-cd.c
> +++ b/drivers/ide/ide-cd.c
> @@ -1750,6 +1750,11 @@ int ide_cdrom_probe_capabilities (ide_drive_t *drive)
>  		cdi->mask &= ~(CDC_DVD_RAM | CDC_RAM);
>  	if (buf[8 + 3] & 0x10)
>  		cdi->mask &= ~CDC_DVD_R;
> +	if (!(buf[8 + 4] & 0x01)) {
> +		printk(KERN_INFO "ide-cd: audio play not advertised in caps "
> +				"page for drive model [%s], enabling quirk.\n",
> +				drive->id->model);
> +	}
>  	if ((buf[8 + 4] & 0x01) || (cd->cd_flags & IDE_CD_FLAG_PLAY_AUDIO_OK))
>  		cdi->mask &= ~CDC_PLAY_AUDIO;
>  
> @@ -1929,6 +1934,7 @@ static const struct cd_list_entry ide_cd_quirks_list[] = {
>  	{ "MATSHITADVD-ROM SR-8186", NULL,   IDE_CD_FLAG_PLAY_AUDIO_OK	    },
>  	{ "MATSHITADVD-ROM SR-8176", NULL,   IDE_CD_FLAG_PLAY_AUDIO_OK	    },
>  	{ "MATSHITADVD-ROM SR-8174", NULL,   IDE_CD_FLAG_PLAY_AUDIO_OK	    },
> +	{ "Optiarc DVD RW AD-5200A", NULL,   IDE_CD_FLAG_PLAY_AUDIO_OK      },
>  	{ NULL, NULL, 0 }
>  };
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/