Re: [patch 2/7] m68k: Remove BKL from rtc implementations

[Geert Uytterhoeven]

On Thu, Oct 15, 2009 at 09:42, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> m68k does not support SMP. The access to the rtc is already serialized
> with local_irq_save/restore which is sufficient on UP.
>
> The open() protection in arch/m68k/mvme16x/rtc.c is not pretty but
> sufficient on UP and safe w/o the BKL.
>
> open() in arch/m68k/bvme6000/rtc.c can do with the same atomic logic
> as arch/m68k/mvme16x/rtc.c

Thx, applied.

> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> ---
> Âarch/m68k/bvme6000/rtc.c | Â 29 +++++++++--------------------
> Âarch/m68k/mvme16x/rtc.c Â| Â 19 +++++--------------
> Â2 files changed, 14 insertions(+), 34 deletions(-)
>
> Index: linux-2.6-tip/arch/m68k/bvme6000/rtc.c
> ===================================================================
> --- linux-2.6-tip.orig/arch/m68k/bvme6000/rtc.c
> +++ linux-2.6-tip/arch/m68k/bvme6000/rtc.c
> @@ -10,7 +10,6 @@
> Â#include <linux/errno.h>
> Â#include <linux/miscdevice.h>
> Â#include <linux/slab.h>
> -#include <linux/smp_lock.h>
> Â#include <linux/ioport.h>
> Â#include <linux/capability.h>
> Â#include <linux/fcntl.h>
> @@ -36,10 +35,9 @@
> Âstatic unsigned char days_in_mo[] =
> Â{0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
>
> -static char rtc_status;
> +static atomic_t rtc_status = ATOMIC_INIT(1);
>
> -static int rtc_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
> - Â Â Â Â Â Â Â Â Â Âunsigned long arg)
> +static long rtc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> Â{
> Â Â Â Âvolatile RtcPtr_t rtc = (RtcPtr_t)BVME_RTC_BASE;
> Â Â Â Âunsigned char msr;
> @@ -133,29 +131,20 @@ static int rtc_ioctl(struct inode *inode
> Â}
>
> Â/*
> - * Â Â We enforce only one user at a time here with the open/close.
> - * Â Â Also clear the previous interrupt data on an open, and clean
> - * Â Â up things on a close.
> + * We enforce only one user at a time here with the open/close.
> Â*/
> -
> Âstatic int rtc_open(struct inode *inode, struct file *file)
> Â{
> - Â Â Â lock_kernel();
> - Â Â Â if(rtc_status) {
> - Â Â Â Â Â Â Â unlock_kernel();
> + Â Â Â if (!atomic_dec_and_test(&rtc_status)) {
> + Â Â Â Â Â Â Â atomic_inc(&rtc_status);
> Â Â Â Â Â Â Â Âreturn -EBUSY;
> Â Â Â Â}
> -
> - Â Â Â rtc_status = 1;
> - Â Â Â unlock_kernel();
> Â Â Â Âreturn 0;
> Â}
>
> Âstatic int rtc_release(struct inode *inode, struct file *file)
> Â{
> - Â Â Â lock_kernel();
> - Â Â Â rtc_status = 0;
> - Â Â Â unlock_kernel();
> + Â Â Â atomic_inc(&rtc_status);
> Â Â Â Âreturn 0;
> Â}
>
> @@ -164,9 +153,9 @@ static int rtc_release(struct inode *ino
> Â*/
>
> Âstatic const struct file_operations rtc_fops = {
> - Â Â Â .ioctl = Â Â Â Ârtc_ioctl,
> - Â Â Â .open = Â Â Â Â rtc_open,
> - Â Â Â .release = Â Â Ârtc_release,
> + Â Â Â .unlocked_ioctl = rtc_ioctl,
> +    .open      = rtc_open,
> +    .release    Â= rtc_release,
> Â};
>
> Âstatic struct miscdevice rtc_dev = {
> Index: linux-2.6-tip/arch/m68k/mvme16x/rtc.c
> ===================================================================
> --- linux-2.6-tip.orig/arch/m68k/mvme16x/rtc.c
> +++ linux-2.6-tip/arch/m68k/mvme16x/rtc.c
> @@ -10,7 +10,6 @@
> Â#include <linux/errno.h>
> Â#include <linux/miscdevice.h>
> Â#include <linux/slab.h>
> -#include <linux/smp_lock.h>
> Â#include <linux/ioport.h>
> Â#include <linux/capability.h>
> Â#include <linux/fcntl.h>
> @@ -37,8 +36,7 @@ static const unsigned char days_in_mo[]
>
> Âstatic atomic_t rtc_ready = ATOMIC_INIT(1);
>
> -static int rtc_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
> - Â Â Â Â Â Â Â Â Â Âunsigned long arg)
> +static long rtc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> Â{
> Â Â Â Âvolatile MK48T08ptr_t rtc = (MK48T08ptr_t)MVME_RTC_BASE;
> Â Â Â Âunsigned long flags;
> @@ -121,22 +119,15 @@ static int rtc_ioctl(struct inode *inode
> Â}
>
> Â/*
> - * Â Â We enforce only one user at a time here with the open/close.
> - * Â Â Also clear the previous interrupt data on an open, and clean
> - * Â Â up things on a close.
> + * We enforce only one user at a time here with the open/close.
> Â*/
> -
> Âstatic int rtc_open(struct inode *inode, struct file *file)
> Â{
> - Â Â Â lock_kernel();
> Â Â Â Âif( !atomic_dec_and_test(&rtc_ready) )
> Â Â Â Â{
> Â Â Â Â Â Â Â Âatomic_inc( &rtc_ready );
> - Â Â Â Â Â Â Â unlock_kernel();
> Â Â Â Â Â Â Â Âreturn -EBUSY;
> Â Â Â Â}
> - Â Â Â unlock_kernel();
> -
> Â Â Â Âreturn 0;
> Â}
>
> @@ -151,9 +142,9 @@ static int rtc_release(struct inode *ino
> Â*/
>
> Âstatic const struct file_operations rtc_fops = {
> - Â Â Â .ioctl = Â Â Â Ârtc_ioctl,
> - Â Â Â .open = Â Â Â Â rtc_open,
> - Â Â Â .release = Â Â Ârtc_release,
> + Â Â Â .unlocked_ioctl = rtc_ioctl,
> +    .open      = rtc_open,
> +    .release    Â= rtc_release,
> Â};
>
> Âstatic struct miscdevice rtc_dev=

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds
--
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/