From: Arnd Bergmann <arnd@arndb.de>
To: Jamie Iles <jamie@jamieiles.com>
Subject: Re: [RFC PATCHv2 1/4] drivers/otp: add initial support for OTP memory
Date: Sat, 26 Mar 2011 00:02:19 +0100
User-Agent: KMail/1.13.5 (Linux/2.6.38+; KDE/4.5.1; x86_64; ; )
Cc: Mike Frysinger <vapier@gentoo.org>, linux-kernel@vger.kernel.org,
        gregkh@suse.de
References: <1300980071-24645-1-git-send-email-jamie@jamieiles.com> <AANLkTinQ_M92zDZx_Jfe=rvPO0Gr4kHhXZUATvgVBywh@mail.gmail.com> <20110325225220.GV3130@pulham.picochip.com>
In-Reply-To: <20110325225220.GV3130@pulham.picochip.com>
MIME-Version: 1.0
Content-Type: Text/Plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Message-Id: <201103260002.19291.arnd@arndb.de>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1341
Lines: 37

On Friday 25 March 2011 23:52:20 Jamie Iles wrote:
> That's what I intended but I guess it isn't that clear from the code.  
> Perhaps instead of:
> 
>         mutex_lock();
>         region->ops->ioctl();
>         mutex_unlock();
> 
> if I change it to:
> 
>         mutex_lock();
>         switch (cmd) {
>         default:
>                 region->ops->ioctl();
>         }
>         mutex_unlock();
> 
> then that's a bit clearer.  If there's stuff common across different OTP 
> implementations then we can add it to the region ops and decode it here 
> and if not fall back to the specific implementation.

This will make it a lot harder to support compat_ioctl as well.
Just drop the ->ioctl callback into the driver and make one
callback per command. Any command that could be reused on
another driver will then be defined in the common header file
already and just work on new drivers.

If you are tempted to add commands that cannot possibly work
on other hardware, that is usually a good indication that this
command should not be added, or should be generalized in some way.

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