[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <48F316C4.1070603@gmail.com>
Date: Mon, 13 Oct 2008 11:37:08 +0200
From: Jiri Slaby <jirislaby@...il.com>
To: Bryan Wu <cooloney@...nel.org>
CC: sam@...nborg.org, linux-kernel@...r.kernel.org,
Mike Frysinger <vapier.adi@...il.com>
Subject: Re: [PATCH 1/1] Blackfin OTP Char Driver: add writing support of
OTP
On 10/13/2008 11:13 AM, Bryan Wu wrote:
> From: Mike Frysinger <vapier.adi@...il.com>
Some changelog please.
> Signed-off-by: Mike Frysinger <vapier.adi@...il.com>
> Signed-off-by: Bryan Wu <cooloney@...nel.org>
> ---
[...]
> @@ -123,18 +132,95 @@ static ssize_t bfin_otp_write(struct file *filp, const char __user *buff, size_t
> if (mutex_lock_interruptible(&bfin_otp_lock))
> return -ERESTARTSYS;
>
> - /* need otp_init() documentation before this can be implemented */
> + stampit();
> +
> + timing = bfin_otp_init_timing();
> + if (timing == 0) {
> + mutex_unlock(&bfin_otp_lock);
> + return -EIO;
> + }
> +
> + base_flags = OTP_CHECK_FOR_PREV_WRITE;
> +
> + bytes_done = 0;
> + page = *pos / (sizeof(u64) * 2);
> + while (bytes_done < count) {
> + flags = base_flags | (*pos % (sizeof(u64) * 2) ? OTP_UPPER_HALF : OTP_LOWER_HALF);
> + stamp("processing page %i (0x%x:%s) from %p", page, flags,
> + (flags & OTP_UPPER_HALF ? "upper" : "lower"), buff + bytes_done);
> + if (copy_from_user(&content, buff + bytes_done, sizeof(content))) {
> + bytes_done = -EFAULT;
> + break;
> + }
> + ret = bfrom_OtpWrite(page, flags, &content);
> + if (ret & OTP_MASTER_ERROR) {
> + stamp("error from otp: 0x%x", ret);
> + bytes_done = -EIO;
> + break;
> + }
> + if (flags & OTP_UPPER_HALF)
> + ++page;
> + bytes_done += sizeof(content);
> + *pos += sizeof(content);
What happens to pos if it fails later?
> + }
> +
> + bfin_otp_deinit_timing(timing);
>
> mutex_unlock(&bfin_otp_lock);
>
> + return bytes_done;
> +}
> +
> +static int bfin_otp_ioctl(struct inode *inode, struct file *filp,
> + unsigned cmd, unsigned long arg)
> +{
> + stampit();
> +
> + switch (cmd) {
> + case OTPLOCK: {
> + u32 timing;
> + int ret = -EIO;
> +
> + if (!allow_writes)
> + return -EACCES;
> +
> + if (mutex_lock_interruptible(&bfin_otp_lock))
> + return -ERESTARTSYS;
> +
> + timing = bfin_otp_init_timing();
> + if (timing) {
> + u32 otp_result = bfrom_OtpWrite(arg, OTP_LOCK, NULL);
> + stamp("locking page %i resulted in 0x%x", arg, otp_result);
> + if (!(otp_result & OTP_MASTER_ERROR))
> + ret = 0;
> +
> + bfin_otp_deinit_timing(timing);
> + }
> +
> + mutex_unlock(&bfin_otp_lock);
> +
> + return ret;
> + }
> +
> + case MEMLOCK:
> + allow_writes = false;
> + return 0;
> +
> + case MEMUNLOCK:
> + allow_writes = true;
> + return 0;
Please switch to unlocked_ioctl. It should be pretty easy. You should change
(and check) allow_writes under the mutex anyway.
> + }
> +
> return -EINVAL;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists