[PATCH] OMAP: use fncpy to copy the PM code functions to SRAM

[Jean Pihet]

Dave, Russell,

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
> type-safe.
>
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'.
>
> Cheers
> ---Dave
>

Thanks,
Jean