[PATCH] OMAP: use fncpy to copy the PM code functions to SRAM
jean.pihet at newoldbits.com
Tue Jan 18 07:05:49 EST 2011
On Mon, Jan 17, 2011 at 4:46 PM, Dave Martin <dave.martin at linaro.org> wrote:
> On Mon, Jan 17, 2011 at 2:01 PM, Jean Pihet <jean.pihet at newoldbits.com> wrote:
>> On Fri, Jan 14, 2011 at 6:34 PM, Russell King - ARM Linux
>> <linux at arm.linux.org.uk> wrote:
>>> On Fri, Jan 14, 2011 at 05:13:01PM +0100, Jean Pihet wrote:
>>>> Is the name 'omap_sram_push' wrong then?
>>>> What about the following?
>>>> @@ -251,9 +251,8 @@ void * omap_sram_push(void * start, unsigned long size)
>>>> omap_sram_ceil -= size;
>>>> omap_sram_ceil = ROUND_DOWN(omap_sram_ceil, sizeof(void *));
>>>> - memcpy((void *)omap_sram_ceil, start, size);
>>>> - flush_icache_range((unsigned long)omap_sram_ceil,
>>>> - (unsigned long)(omap_sram_ceil + size));
>>>> - return (void *)omap_sram_ceil;
>>>> + return fncpy((void *)omap_sram_ceil, start, size);
>>> It's more correct, but still missing out on the type safety which we've
>>> tried to provide with fncpy.
>> IIUC the type of the function is propagated from the 2nd argument
>> (funcp) to the return value, which is fine here. The (void)* is here
>> only to avoid a warning thrown by memcpy.
> I think Russell's argument was that the compiler will not notice if
> you mismatch the return type and the function to be copied, because
> the casts to/from void * make the compiler blind to the real types
> involved. So:
> int f(int);
> size_t size_of_f;
> void *buffer;
> int (*_copied_f1)(int);
> char *(*_copied_f2)(char *);
> _copied_f1 = fncpy(buffer, f, size_of_f); /* OK */
> _copied_f2 = fncpy(buffer, f, size_of_f); /* compile error */
> _copied_f1 = omap_sram_push(f, size_of_f); /* OK */
> _copied_f2 = opam_sram_push(f, size_of_f); /* type mismatch, but
> compilation succeeds */
> One way to work around this is would be to make omap_sram_push() a macro:
> #define omap_sram_push(funcp, size) \
> (typeof(funcp))_do_omap_sram_push((void *)(funcp), size)
> ... where the definition of _do_omap_sram_push() is the same is the
> existing definition of omap_sram_push(). Providing
> _do_omap_sram_push() is not called directly, this should now be
Ok I reworked the patch from your suggestions. Indeed a few functions
types mismatch have been spotted and corrected using the fncpy API.
New patch sent as '[PATCH v2] OMAP: use fncpy to copy the PM code
functions to SRAM'.
More information about the linux-arm-kernel