On 07/17/2014 04:37 PM, Thierry Reding wrote:
> On Thu, Jul 17, 2014 at 09:27:58AM +0800, Chen Gang wrote:
> [...]
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index c2421e0..a7500c3 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -630,7 +630,16 @@ extern unsigned long devm_get_free_pages(struct device
>> *dev,
>> gfp_t gfp_mask, unsigned int order);
>> extern void devm_free_pages(struct device *dev, unsigned long addr);
>>
>> +#ifdef CONFIG_HAS_IOMEM
>> void __iomem *devm_ioremap_resource(struct device *dev, struct resource
>> *res);
>> +#elif defined(CONFIG_COMPILE_TEST)
>> +static inline void __iomem *devm_ioremap_resource(struct device *dev,
>> + struct resource *res)
>> +{
>> + pr_warn("no hardware io memory, only for COMPILE_TEST\n");
>
> Maybe: "Hardware doesn't support memory-mapped I/O"? I'm not sure if
> it's useful to include the reference to COMPILE_TEST, especially since
> the #elif will be dropped in favour of a simple #else.
>
OK, thanks. I will use the comments which you provide.
And also use #else instead of #elif, use 'dev_warn' instead of 'pr_warn'
which Guenter suggests.
>> + return (__force void __iomem *)ERR_PTR(-ENXIO);
>
> There's apparently an IOMEM_ERR_PTR() for this nowadays...
>
IOMEM_ERR_PTR() is defined within "lib/devres.c", not in "./include".
But may we move it from "lib/devres.c" to "./include/linux/err.h"?
For me, I am not quite sure, it may need additional discussion, but at
least, that will be another patch.
>> +}
>> +#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */
>>
>> /* allows to add/remove a custom action to devres stack */
>> int devm_add_action(struct device *dev, void (*action)(void *), void *data);
>> diff --git a/include/linux/io.h b/include/linux/io.h
>> index b76e6e5..59128aa 100644
>> --- a/include/linux/io.h
>> +++ b/include/linux/io.h
>> @@ -58,14 +58,42 @@ static inline void devm_ioport_unmap(struct device *dev,
>> void __iomem *addr)
>> }
>> #endif
>>
>> +#ifdef CONFIG_HAS_IOMEM
>> +
>> void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
>> unsigned long size);
>> void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t
>> offset,
>> unsigned long size);
>> void devm_iounmap(struct device *dev, void __iomem *addr);
>> +void devm_ioremap_release(struct device *dev, void *res);
>> +
>> +#elif defined(CONFIG_COMPILE_TEST)
>> +
>> +static inline void __iomem *devm_ioremap(struct device *dev,
>> + resource_size_t offset, unsigned long size)
>
> For consistency with other functions above, please keep arguments on
> subsequent lines aligned with the first argument on the first line, like
> so:
>
> static inline void __iomem *devm_ioremap(struct device *dev,
> resource_size_t offset,
> unsigned long size)
>
That sounds fine to me, thanks.
>> +{
>> + pr_warn("no hardware io memory, only for COMPILE_TEST\n");
>> + return NULL;
>> +}
>> +static inline void __iomem *devm_ioremap_nocache(struct device *dev,
>> + resource_size_t offset, unsigned long size)
>> +{
>> + pr_warn("no hardware io memory, only for COMPILE_TEST\n");
>> + return NULL;
>> +}
>
> Perhaps this should call devm_ioremap() so we don't have to repeat the
> same error message? Or maybe make it:
>
> #define devm_ioremap_nocache devm_ioremap
>
That sounds fine to me, thanks.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
