Hello,
On 06/28/2010 10:43 AM, su henry wrote:
>> Can you please align fields? Also, single struct for the whole
>> system? I haven't read the spec but I don't think anybody would be
>> defining things like this system-wide. Right?
>>
>
> My original though is to put this structure to cdrom_device_info, but
> this structure should be firstly used by acpi_walk_namespace in
> sr_init, and cdrom_device_info is allocated in sr_probe, that's why I
> define a separate structure for the zero power odd.
What prevents you from walking the acpi device tree from sr_probe()?
Or even if you need to walk it from sr_init(), you still need to store
the result and associate it with a specific cdrom device. It is
something which is specific to single device. You can't use single
global data structure for all devices like this.
>> bool?
>
> The return value of sr_test_unit_ready is int.
I would still go for bool but it's a peripheral issue.
>> This thing definitely belongs to struct scsi_cd. What are the
>> synchronization rules here? Also, what happens if async notification
>> is in use? How does the timer get activated then?
>
> This is a problem, any suggestions? Especially when the system goes to
> S3/S4 state.
Associate with specific device and using timer should work.
>>> + default:
>>> + /*tray/drawer/pop-up type*/
>>> + /* 3a/01(asc/ascq) means MEDIUM NOT PRESENT - TRAY CLOSED */
>>> + if (isvalid && (sshdr->asc == 0x3a &&
>>> + sshdr->ascq == 0x01)) {
>>> +
>>> + /* Eject the tray for a tray type drive*/
>>> + if (sr_zpodd_device.flags & SR_ZPODD_NEED_EJECT) {
>>> + sr_tray_move(cdi, 1);
>>> + sr_zpodd_device.flags &= ~SR_ZPODD_NEED_EJECT;
>>> + } else if (!(sr_zpodd_device.flags &
>>> + SR_ZPODD_NO_DISK)) {
>>> + sr_zpodd_device.flags |= SR_ZPODD_NO_DISK;
>>> + sr_zpodd_device.last_jiffies = jiffies;
>>> + } else if (time_after(jiffies,
>>> + sr_zpodd_device.last_jiffies
>>> + + SR_NO_MEDIA_TIMEOUT)) {
>>> + acpi_bus_set_power(sr_zpodd_device.handle,
>>> + ACPI_STATE_D3);
>>> + }
>>> + } else
>>> + sr_zpodd_device.flags &= ~SR_ZPODD_NO_DISK;
>>> + break;
>>> + }
>>
>> It would probably be better to separate decision making and updating
>> states.
>
> When user pressing the button on a tray type drive, driver should set
> a flag used to eject the tray in sr_media_change, if we use that
> states here, we should a one more state "NEED_EJECT" for drive status.
Oh, I meant the code. ie. instead of
switch (device type) {
type a:
something something
break;
default:
something something slightly differently
break;
}
do something like the following,
switch (device type) {
type a:
determine what to do
break;
default:
determine what to do slightly differently;
break;
}
do what has been determined;
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
