Re: [RFC PATCHv2 1/4] drivers/otp: add initial support for OTP memory

[Jamie Iles <jamie@jamieiles.com>]

On Sat, Mar 26, 2011 at 12:02:19AM +0100, Arnd Bergmann wrote:
> 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.

OK, you've convinced me :-)

The only use of an ioctl() at the moment is for locking portions of the 
OTP so perhaps we add a new region op .lock(unsigned long start_addr, 
size_t nr_words).

For the actual ioctl() we should assume byte addressing rather than 
words though and do the conversion in the driver so we can cope with 
devices that don't have 64-bit words and do the locking on a looping 
word-by-word basis.

	struct otp_lock_req {
		__u32	start_addr;
		__u32	byte_count;
	};

The start_addr would be relative to the start of the region.

Mike, would this be OK with you if we used a different ioctl() to the 
one bfin-otp is using currently?  I notice that it's using the OTPLOCK 
ioctl() from MTD but I think it's using the argument in a different way.

Jamie