Re: [PATCH 1/8] staging: tidspbridge - remove req_addr from proc_map - Russell King

From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: "Guzman Lugo, Fernando" <fernando.lugo@ti.com>,
	"gregkh@suse.de" <gregkh@suse.de>,
	"hiroshi.doyu@nokia.com" <hiroshi.doyu@nokia.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Felipe Contreras <felipe.contreras@nokia.com>,
	"andy.shevchenko@gmail.com" <andy.shevchenko@gmail.com>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/8] staging: tidspbridge - remove req_addr from proc_map
Date: Thu, 28 Oct 2010 16:56:35 +0100	[thread overview]
Message-ID: <20101028155635.GF3122@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <AANLkTimwbNV227yhPy-5E5rdAL2LciKeCSouWGW9H+WG@mail.gmail.com>

On Wed, Oct 27, 2010 at 11:19:47AM +0300, Felipe Contreras wrote:
> Yes, but this has to be done on every library/app that is using the dsp.
> It's much easier to do it on kernel side.
> 
> Besides, at least on gst-dsp we want it to work for _all_ bridge
> versions, so it would be:
> 
> @@ -829,7 +829,9 @@ struct map_mem {
>         void *proc_handle;
>         void *mpu_addr;
>         unsigned long size;
> +#if DSP_API >= 3
>         void *req_addr;
> +#endif
>         void **ret_map_addr;
>         unsigned long attr;
>  };

This thread is getting really stupid.  There is a process for changing
the arguments to ioctl.

The first thing is that ioctl numbers are supposed to be defined in a
standard form - using _IO(), _IOR(), _IOW() and _IOWR().  Amongst the
things that these macros take is the structure which is being passed.

What this means is that the ioctl number generated by these macros is
directly related to the size of the structure.  This immediately gives
a way to deal with a limited set of changes to the structure.

The steps are as follows:

1. Rename the structure and all uses of it from "struct map_mem" to
   "struct old_map_mem"
2. Rename the ioctl definition names and all uses to have an OLD prefix.

So at this stage, the original ioctl number and structure are still
present, and importantly are unchanged except by name.

3. Add the new "struct map_mem".
4. Add the new ioctl name definition using "struct map_mem" to identify
   it.
5. Implement the new ioctl number.  Optionally, implement the old ioctl
   functionality via the new implementation if this is appropriate - but
   make sure that the old ioctl still works.
6. arrange for users of the old ioctl to receive a warning that it's
   deprecated.

This allows existing userspace to continue working, but with warnings so
that people know that they need to rebuild their libraries against the
new structures.

There's no need for a DSP_API define using this method, provided you
don't end up with these structures shrinking and then re-growing back
to a size that they were before.