From: Corey Minyard <minyard@acm.org>
To: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>,
emoenke@gwdg.de, minyard@wf-rch.cirr.com,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] use mutex instead of binary semaphore in CDU-31A driver
Date: Tue, 24 Apr 2007 07:44:33 -0500 [thread overview]
Message-ID: <462DFBB1.3020603@acm.org> (raw)
In-Reply-To: <20070423071537.GA10339@traven>
This is fine with me; I don't see any problems in it and the mutex
is better. However, I cannot test this as I have no hardware
left capable of hosting the drive.
-corey
Matthias Kaehlcke wrote:
> use mutex instead of binary semaphore in CDU-31A driver
>
> Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
>
> --
> diff --git a/drivers/cdrom/cdu31a.c b/drivers/cdrom/cdu31a.c
> index 2157c58..d3649e4 100644
> --- a/drivers/cdrom/cdu31a.c
> +++ b/drivers/cdrom/cdu31a.c
> @@ -263,7 +263,7 @@ static int sony_toc_read = 0; /* Has the TOC been read for
> static struct s_sony_subcode last_sony_subcode; /* Points to the last
> subcode address read */
>
> -static DECLARE_MUTEX(sony_sem); /* Semaphore for drive hardware access */
> +static DEFINE_MUTEX(sony_mtx); /* Mutex for drive hardware access */
>
> static int is_double_speed = 0; /* does the drive support double speed ? */
>
> @@ -339,11 +339,11 @@ static int scd_drive_status(struct cdrom_device_info *cdi, int slot_nr)
> return -EINVAL;
> if (sony_spun_up)
> return CDS_DISC_OK;
> - if (down_interruptible(&sony_sem))
> + if (mutex_lock_interruptible(&sony_mtx))
> return -ERESTARTSYS;
> if (scd_spinup() == 0)
> sony_spun_up = 1;
> - up(&sony_sem);
> + mutex_unlock(&sony_mtx);
> return sony_spun_up ? CDS_DISC_OK : CDS_DRIVE_NOT_READY;
> }
>
> @@ -452,7 +452,7 @@ static int scd_reset(struct cdrom_device_info *cdi)
> {
> unsigned long retry_count;
>
> - if (down_interruptible(&sony_sem))
> + if (mutex_lock_interruptible(&sony_mtx))
> return -ERESTARTSYS;
> reset_drive();
>
> @@ -461,7 +461,7 @@ static int scd_reset(struct cdrom_device_info *cdi)
> sony_sleep();
> }
>
> - up(&sony_sem);
> + mutex_unlock(&sony_mtx);
> return 0;
> }
>
> @@ -655,10 +655,10 @@ static int scd_select_speed(struct cdrom_device_info *cdi, int speed)
> else
> sony_speed = speed - 1;
>
> - if (down_interruptible(&sony_sem))
> + if (mutex_lock_interruptible(&sony_mtx))
> return -ERESTARTSYS;
> set_drive_params(sony_speed);
> - up(&sony_sem);
> + mutex_unlock(&sony_mtx);
> return 0;
> }
>
> @@ -673,10 +673,10 @@ static int scd_lock_door(struct cdrom_device_info *cdi, int lock)
> } else {
> is_auto_eject = 0;
> }
> - if (down_interruptible(&sony_sem))
> + if (mutex_lock_interruptible(&sony_mtx))
> return -ERESTARTSYS;
> set_drive_params(sony_speed);
> - up(&sony_sem);
> + mutex_unlock(&sony_mtx);
> return 0;
> }
>
> @@ -1143,7 +1143,7 @@ static void handle_abort_timeout(unsigned long data)
> {
> pr_debug(PFX "Entering %s\n", __FUNCTION__);
> /* If it is in use, ignore it. */
> - if (down_trylock(&sony_sem) == 0) {
> + if (mutex_trylock(&sony_mtx) == 0) {
> /* We can't use abort_read(), because it will sleep
> or schedule in the timer interrupt. Just start
> the operation, finish it on the next access to
> @@ -1154,7 +1154,7 @@ static void handle_abort_timeout(unsigned long data)
>
> sony_blocks_left = 0;
> abort_read_started = 1;
> - up(&sony_sem);
> + mutex_unlock(&sony_mtx);
> }
> pr_debug(PFX "Leaving %s\n", __FUNCTION__);
> }
> @@ -1300,7 +1300,7 @@ static void do_cdu31a_request(request_queue_t * q)
> pr_debug(PFX "Entering %s\n", __FUNCTION__);
>
> spin_unlock_irq(q->queue_lock);
> - if (down_interruptible(&sony_sem)) {
> + if (mutex_lock_interruptible(&sony_mtx)) {
> spin_lock_irq(q->queue_lock);
> return;
> }
> @@ -1435,7 +1435,7 @@ static void do_cdu31a_request(request_queue_t * q)
> add_timer(&cdu31a_abort_timer);
> #endif
>
> - up(&sony_sem);
> + mutex_unlock(&sony_mtx);
> spin_lock_irq(q->queue_lock);
> pr_debug(PFX "Leaving %s at %d\n", __FUNCTION__, __LINE__);
> }
> @@ -1906,10 +1906,10 @@ static int scd_get_last_session(struct cdrom_device_info *cdi,
> return 1;
>
> if (!sony_toc_read) {
> - if (down_interruptible(&sony_sem))
> + if (mutex_lock_interruptible(&sony_mtx))
> return -ERESTARTSYS;
> sony_get_toc();
> - up(&sony_sem);
> + mutex_unlock(&sony_mtx);
> }
>
> ms_info->addr_format = CDROM_LBA;
> @@ -1988,11 +1988,11 @@ scd_get_mcn(struct cdrom_device_info *cdi, struct cdrom_mcn *mcn)
> unsigned int res_size;
>
> memset(mcn->medium_catalog_number, 0, 14);
> - if (down_interruptible(&sony_sem))
> + if (mutex_lock_interruptible(&sony_mtx))
> return -ERESTARTSYS;
> do_sony_cd_cmd(SONY_REQ_UPC_EAN_CMD,
> NULL, 0, resbuffer, &res_size);
> - up(&sony_sem);
> + mutex_unlock(&sony_mtx);
> if ((res_size < 2) || ((resbuffer[0] & 0xf0) == 0x20));
> else {
> /* packed bcd to single ASCII digits */
> @@ -2207,7 +2207,7 @@ static int read_audio(struct cdrom_read_audio *ra)
> unsigned int res_size;
> unsigned int cframe;
>
> - if (down_interruptible(&sony_sem))
> + if (mutex_lock_interruptible(&sony_mtx))
> return -ERESTARTSYS;
> if (!sony_spun_up)
> scd_spinup();
> @@ -2343,7 +2343,7 @@ static int read_audio(struct cdrom_read_audio *ra)
> }
>
> out_up:
> - up(&sony_sem);
> + mutex_unlock(&sony_mtx);
>
> return retval;
> }
> @@ -2373,7 +2373,7 @@ static int scd_tray_move(struct cdrom_device_info *cdi, int position)
> {
> int retval;
>
> - if (down_interruptible(&sony_sem))
> + if (mutex_lock_interruptible(&sony_mtx))
> return -ERESTARTSYS;
> if (position == 1 /* open tray */ ) {
> unsigned char res_reg[12];
> @@ -2392,7 +2392,7 @@ static int scd_tray_move(struct cdrom_device_info *cdi, int position)
> sony_spun_up = 1;
> retval = 0;
> }
> - up(&sony_sem);
> + mutex_unlock(&sony_mtx);
> return retval;
> }
>
> @@ -2407,7 +2407,7 @@ static int scd_audio_ioctl(struct cdrom_device_info *cdi,
> unsigned char params[7];
> int i, retval;
>
> - if (down_interruptible(&sony_sem))
> + if (mutex_lock_interruptible(&sony_mtx))
> return -ERESTARTSYS;
> switch (cmd) {
> case CDROMSTART: /* Spin up the drive */
> @@ -2665,7 +2665,7 @@ static int scd_audio_ioctl(struct cdrom_device_info *cdi,
> retval = -EINVAL;
> break;
> }
> - up(&sony_sem);
> + mutex_unlock(&sony_mtx);
> return retval;
> }
>
> @@ -2675,7 +2675,7 @@ static int scd_read_audio(struct cdrom_device_info *cdi,
> void __user *argp = (void __user *)arg;
> int retval;
>
> - if (down_interruptible(&sony_sem))
> + if (mutex_lock_interruptible(&sony_mtx))
> return -ERESTARTSYS;
> switch (cmd) {
> case CDROMREADAUDIO: /* Read 2352 byte audio tracks and 2340 byte
> @@ -2749,7 +2749,7 @@ static int scd_read_audio(struct cdrom_device_info *cdi,
> default:
> retval = -EINVAL;
> }
> - up(&sony_sem);
> + mutex_unlock(&sony_mtx);
> return retval;
> }
>
>
prev parent reply other threads:[~2007-04-24 12:44 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-04-23 7:15 [PATCH] use mutex instead of binary semaphore in CDU-31A driver Matthias Kaehlcke
2007-04-24 12:44 ` Corey Minyard [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=462DFBB1.3020603@acm.org \
--to=minyard@acm.org \
--cc=emoenke@gwdg.de \
--cc=linux-kernel@vger.kernel.org \
--cc=matthias.kaehlcke@gmail.com \
--cc=minyard@wf-rch.cirr.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox