* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 1:27 ` Chen Gang
@ 2014-07-17 1:58 ` Guenter Roeck
2014-07-17 8:37 ` Thierry Reding
2014-07-17 9:20 ` Arnd Bergmann
2 siblings, 0 replies; 70+ messages in thread
From: Guenter Roeck @ 2014-07-17 1:58 UTC (permalink / raw)
To: Chen Gang, Richard Weinberger
Cc: Lars-Peter Clausen, Greg Kroah-Hartman, dmitry.torokhov,
linux-iio, Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23, arnd,
Geert Uytterhoeven
On 07/16/2014 06:27 PM, Chen Gang wrote:
>
>
> On 07/15/2014 10:38 PM, Chen Gang wrote:
>> On 07/15/2014 09:11 AM, Chen Gang wrote:
>>>
>>>
>>> On 07/15/2014 08:53 AM, Guenter Roeck wrote:
>>>> On 07/14/2014 05:34 PM, Chen Gang wrote:
>>>>> On 07/14/2014 05:22 PM, Chen Gang wrote:
>>>>>>
>>>>>> 在 2014年7月14日，下午4:57，Richard Weinberger <richard@nod.at> 写道：
>>>>>>
>>>>>>> Am 14.07.2014 10:48, schrieb Lars-Peter Clausen:
>>>>>>>> On 07/14/2014 10:31 AM, Richard Weinberger wrote:
>>>>>>>>> Am 13.07.2014 22:17, schrieb Greg Kroah-Hartman:
>>>>>>>>>> On Sun, Jul 13, 2014 at 09:33:38PM +0200, Richard Weinberger wrote:
>>>>>>>>>>> Maybe we could add COMPILE_TEST to the version string too?
>>>>>>>>>>> Just to detect such kernels fast in user bug reports...
>>>>>>>>>>
>>>>>>>>>> What kind of bug report are you going to get?
>>>>>>>>>
>>>>>>>>> User manages to enable CONFIG_FOO by selecting COMPILE_TEST and
>>>>>>>>> complains that it does not work. :)
>>>>>>>>
>>>>>>>> These drivers are typically drivers for some SoC peripheral and the
>>>>>>>> device will simply physically not exist on a platform that does not
>>>>>>>> provide HAS_IOMEM. This is not really any
>>>>>>>> different from making the driver selectable via COMPILE_TEST for
>>>>>>>> any other platform. To hit the issue you'd have to instantiate a
>>>>>>>> device driver instance for a device that
>>>>>>>> physically does not exist. This will always result in a failure.
>>>>>>>
>>>>>>> Okay, you have convinced me. :)
>>>>>>>
>>>>>
>>>>> After search the history patches, I found one related patch which made
>>>>> by myself (when I am in Asianux):
>>>>>
>>>>> "https://lkml.org/lkml/2013/7/1/641"
>>>>>
>>>>> For me, it is a long discussion, and forced many members have to join
>>>>> in. Please help check again.
>>>>>
>>>>
>>>> One thing you could try would be to return NULL (or where appropriate
>>>> an error) in the #else case of CONFIG_HAS_IOMEM and CONFIG_HAS_IOPORT,
>>>> ie dont take COMPILE_TEST into account at all. Obviously that means
>>>> you won't be able to dump a warning message in the COMPILE_TEST
>>>> case, but at least the code would compile. The rejection of above patch
>>>> would make a good case for this approach.
>>>>
>
> For me, only let 'devm_io*map*' support COMPILE_TEST is OK, that can fix
> all related issues:
>
>
> [PATCH] lib: devres: Add dumy functions to support COMPILE_TEST when no IOMEM
>
> For some architectures which no IOMEM, 'devres' will be skipped. But
> many drivers may still want COMPILE_TEST, so let 'devres' support it.
>
> The related error (with allmodconfig under score):
>
> MODPOST 1365 modules
> ERROR: "devm_ioremap_resource" [drivers/watchdog/tegra_wdt.ko] undefined!
> ERROR: "devm_ioremap_resource" [drivers/watchdog/of_xilinx_wdt.ko] undefined!
> ERROR: "devm_ioremap_resource" [drivers/staging/iio/adc/mxs-lradc.ko] undefined!
> ERROR: "devm_ioremap_resource" [drivers/pwm/pwm-clps711x.ko] undefined!
> ERROR: "devm_ioremap_resource" [drivers/input/serio/olpc_apsp.ko] undefined!
> ERROR: "devm_ioremap_resource" [drivers/input/serio/arc_ps2.ko] undefined!
> ERROR: "devm_ioremap_resource" [drivers/rtc/rtc-xgene.ko] undefined!
> ERROR: "devm_ioremap_resource" [drivers/rtc/rtc-stk17ta8.ko] undefined!
> ERROR: "devm_ioremap_resource" [drivers/rtc/rtc-ds1742.ko] undefined!
> ERROR: "devm_ioremap_resource" [drivers/rtc/rtc-ds1553.ko] undefined!
> ERROR: "devm_ioremap_resource" [drivers/rtc/rtc-ds1511.ko] undefined!
> ERROR: "devm_ioremap_resource" [drivers/rtc/rtc-ds1286.ko] undefined!
> ERROR: "devm_ioremap" [drivers/rtc/rtc-rp5c01.ko] undefined!
> ERROR: "devm_ioremap" [drivers/rtc/rtc-msm6242.ko] undefined!
> ERROR: "devm_ioremap" [drivers/rtc/rtc-m48t59.ko] undefined!
> ERROR: "devm_ioremap" [drivers/rtc/rtc-m48t35.ko] undefined!
> ERROR: "devm_ioremap" [drivers/rtc/rtc-bq4802.ko] undefined!
>
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
> include/linux/device.h | 9 +++++++++
> include/linux/io.h | 30 +++++++++++++++++++++++++++++-
> 2 files changed, 38 insertions(+), 1 deletion(-)
>
> 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)
I would make it #else
> +static inline void __iomem *devm_ioremap_resource(struct device *dev,
> + struct resource *res)
> +{
> + pr_warn("no hardware io memory, only for COMPILE_TEST\n");
dev_warn
> + return (__force void __iomem *)ERR_PTR(-ENXIO);
> +}
> +#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)
> +
Same as above - I would suggest to use #else.
> +static inline void __iomem *devm_ioremap(struct device *dev,
> + resource_size_t offset, unsigned long size)
> +{
> + 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");
dev_warn
Guenter
> + return NULL;
> +}
> +
> +static inline void devm_iounmap(struct device *dev, void __iomem *addr)
> +{
> +}
> +
> +static inline void devm_ioremap_release(struct device *dev, void *res)
> +{
> +}
> +
> +#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */
> +
> int check_signature(const volatile void __iomem *io_addr,
> const unsigned char *signature, int length);
> -void devm_ioremap_release(struct device *dev, void *res);
>
> /*
> * Some systems do not have legacy ISA devices.
>
^ permalink raw reply [flat|nested] 70+ messages in thread* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 1:27 ` Chen Gang
2014-07-17 1:58 ` Guenter Roeck
@ 2014-07-17 8:37 ` Thierry Reding
2014-07-17 8:59 ` Chen Gang
2014-07-17 9:20 ` Arnd Bergmann
2 siblings, 1 reply; 70+ messages in thread
From: Thierry Reding @ 2014-07-17 8:37 UTC (permalink / raw)
To: Chen Gang
Cc: Guenter Roeck, Richard Weinberger, Lars-Peter Clausen,
Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Lennox Wu, Marek Vasut, Liqin Chen,
msalter, linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, arnd, Geert Uytterhoeven
[-- Attachment #1: Type: text/plain, Size: 2872 bytes --]
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.
> + return (__force void __iomem *)ERR_PTR(-ENXIO);
There's apparently an IOMEM_ERR_PTR() for this nowadays...
> +}
> +#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)
> +{
> + 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
?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 70+ messages in thread* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 8:37 ` Thierry Reding
@ 2014-07-17 8:59 ` Chen Gang
2014-07-17 9:16 ` Dan Carpenter
0 siblings, 1 reply; 70+ messages in thread
From: Chen Gang @ 2014-07-17 8:59 UTC (permalink / raw)
To: Thierry Reding
Cc: Guenter Roeck, Richard Weinberger, Lars-Peter Clausen,
Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Lennox Wu, Marek Vasut, Liqin Chen,
msalter, linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, arnd, Geert Uytterhoeven
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
^ permalink raw reply [flat|nested] 70+ messages in thread* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 8:59 ` Chen Gang
@ 2014-07-17 9:16 ` Dan Carpenter
2014-07-17 9:19 ` Chen Gang
0 siblings, 1 reply; 70+ messages in thread
From: Dan Carpenter @ 2014-07-17 9:16 UTC (permalink / raw)
To: Chen Gang
Cc: Thierry Reding, linux-iio, Benjamin Herrenschmidt, teg, Lennox Wu,
Marek Vasut, Liqin Chen, Lars-Peter Clausen, Richard Weinberger,
Geert Uytterhoeven, msalter, Guenter Roeck, linux-pwm, devel,
linux-watchdog, arnd, linux-input, Greg Kroah-Hartman,
dmitry.torokhov, linux-kernel@vger.kernel.org, knaack.h,
Martin Schwidefsky, Mischa.Jonker, jic23
On Thu, Jul 17, 2014 at 04:59:09PM +0800, Chen Gang wrote:
> >> + 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.
Yes. Move it there. That is the right place.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 9:16 ` Dan Carpenter
@ 2014-07-17 9:19 ` Chen Gang
2014-07-23 11:09 ` Chen Gang
0 siblings, 1 reply; 70+ messages in thread
From: Chen Gang @ 2014-07-17 9:19 UTC (permalink / raw)
To: Dan Carpenter
Cc: Thierry Reding, linux-iio, Benjamin Herrenschmidt, teg, Lennox Wu,
Marek Vasut, Liqin Chen, Lars-Peter Clausen, Richard Weinberger,
Geert Uytterhoeven, msalter, Guenter Roeck, linux-pwm, devel,
linux-watchdog, arnd, linux-input, Greg Kroah-Hartman,
dmitry.torokhov, linux-kernel@vger.kernel.org, knaack.h,
Martin Schwidefsky, Mischa.Jonker, jic23
On 07/17/2014 05:16 PM, Dan Carpenter wrote:
> On Thu, Jul 17, 2014 at 04:59:09PM +0800, Chen Gang wrote:
>>>> + 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.
>
> Yes. Move it there. That is the right place.
>
OK, thanks, if no another objections within 2 days, I shall send another
related patch for it.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 9:19 ` Chen Gang
@ 2014-07-23 11:09 ` Chen Gang
2014-07-23 11:30 ` Dan Carpenter
0 siblings, 1 reply; 70+ messages in thread
From: Chen Gang @ 2014-07-23 11:09 UTC (permalink / raw)
To: Dan Carpenter
Cc: Thierry Reding, linux-iio, Benjamin Herrenschmidt, teg, Lennox Wu,
Marek Vasut, Liqin Chen, Lars-Peter Clausen, Richard Weinberger,
Geert Uytterhoeven, msalter, Guenter Roeck, linux-pwm, devel,
linux-watchdog, arnd, linux-input, Greg Kroah-Hartman,
dmitry.torokhov, linux-kernel@vger.kernel.org, knaack.h,
Martin Schwidefsky, Mischa.Jonker, jic23
On 07/17/2014 05:19 PM, Chen Gang wrote:
>
>
> On 07/17/2014 05:16 PM, Dan Carpenter wrote:
>> On Thu, Jul 17, 2014 at 04:59:09PM +0800, Chen Gang wrote:
>>>>> + 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.
>>
>> Yes. Move it there. That is the right place.
>>
Sorry for replying late about IOMEM_ERR_PTR().
Although no objections within 2 days, for me, when move something to
generic header file, it is not only for future using, but also for
solving current issue, or it has to be suspended for waiting 'trigger'.
So for me, the patch about IOMEM_ERR_PTR() need be suspended, until some
member find an issue which may be related with IOMEM_ERR_PTR(), more or
less.
>
> OK, thanks, if no another objections within 2 days, I shall send another
> related patch for it.
>
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-23 11:09 ` Chen Gang
@ 2014-07-23 11:30 ` Dan Carpenter
2014-07-23 11:37 ` Chen Gang
0 siblings, 1 reply; 70+ messages in thread
From: Dan Carpenter @ 2014-07-23 11:30 UTC (permalink / raw)
To: Chen Gang
Cc: linux-iio, Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, Lars-Peter Clausen, Richard Weinberger,
Geert Uytterhoeven, msalter, Guenter Roeck, linux-pwm, devel,
linux-watchdog, arnd, linux-input, Greg Kroah-Hartman,
dmitry.torokhov, linux-kernel@vger.kernel.org, knaack.h,
Martin Schwidefsky, Mischa.Jonker, jic23
On Wed, Jul 23, 2014 at 07:09:22PM +0800, Chen Gang wrote:
>
>
> On 07/17/2014 05:19 PM, Chen Gang wrote:
> >
> >
> > On 07/17/2014 05:16 PM, Dan Carpenter wrote:
> >> On Thu, Jul 17, 2014 at 04:59:09PM +0800, Chen Gang wrote:
> >>>>> + 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.
> >>
> >> Yes. Move it there. That is the right place.
> >>
>
> Sorry for replying late about IOMEM_ERR_PTR().
>
> Although no objections within 2 days, for me, when move something to
> generic header file, it is not only for future using, but also for
> solving current issue, or it has to be suspended for waiting 'trigger'.
>
> So for me, the patch about IOMEM_ERR_PTR() need be suspended, until some
> member find an issue which may be related with IOMEM_ERR_PTR(), more or
> less.
Matthias Brugger already moved it to include/linux/io.h on Friday.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-23 11:30 ` Dan Carpenter
@ 2014-07-23 11:37 ` Chen Gang
0 siblings, 0 replies; 70+ messages in thread
From: Chen Gang @ 2014-07-23 11:37 UTC (permalink / raw)
To: Dan Carpenter
Cc: linux-iio, Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, Lars-Peter Clausen, Richard Weinberger,
Geert Uytterhoeven, msalter, Guenter Roeck, linux-pwm, devel,
linux-watchdog, arnd, linux-input, Greg Kroah-Hartman,
dmitry.torokhov, linux-kernel@vger.kernel.org, knaack.h,
Martin Schwidefsky, Mischa.Jonker, jic23
On 07/23/2014 07:30 PM, Dan Carpenter wrote:
> On Wed, Jul 23, 2014 at 07:09:22PM +0800, Chen Gang wrote:
>>
>>
>> On 07/17/2014 05:19 PM, Chen Gang wrote:
>>>
>>>
>>> On 07/17/2014 05:16 PM, Dan Carpenter wrote:
>>>> On Thu, Jul 17, 2014 at 04:59:09PM +0800, Chen Gang wrote:
>>>>>>> + 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.
>>>>
>>>> Yes. Move it there. That is the right place.
>>>>
>>
>> Sorry for replying late about IOMEM_ERR_PTR().
>>
>> Although no objections within 2 days, for me, when move something to
>> generic header file, it is not only for future using, but also for
>> solving current issue, or it has to be suspended for waiting 'trigger'.
>>
>> So for me, the patch about IOMEM_ERR_PTR() need be suspended, until some
>> member find an issue which may be related with IOMEM_ERR_PTR(), more or
>> less.
>
> Matthias Brugger already moved it to include/linux/io.h on Friday.
>
OK, thank you for your work.
And for me, this 'long discussion' can be finished now (keeping current
status no touch). But if any members have additional suggestions and
ideas, still welcome to reply directly.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 1:27 ` Chen Gang
2014-07-17 1:58 ` Guenter Roeck
2014-07-17 8:37 ` Thierry Reding
@ 2014-07-17 9:20 ` Arnd Bergmann
2014-07-17 9:26 ` Richard Weinberger
` (3 more replies)
2 siblings, 4 replies; 70+ messages in thread
From: Arnd Bergmann @ 2014-07-17 9:20 UTC (permalink / raw)
To: Chen Gang
Cc: Guenter Roeck, Richard Weinberger, Lars-Peter Clausen,
Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
On Thursday 17 July 2014 09:27:58 Chen Gang wrote:
> 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");
> + return (__force void __iomem *)ERR_PTR(-ENXIO);
> +}
> +#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */
>
> /* allows to add/remove a custom action to devres stack */
To be honest, I think it's a bad idea to introduce wrappers functions
that are only available when CONFIG_COMPILE_TEST is set.
COMPILE_TEST is a great tool in general, but it has its limits.
In particular, the case for !CONFIG_IOMEM is completely obscure
and we won't find any bugs by allowing more drivers to be built
in those configurations, but attempting to do it would cause
endless churn by changing each instance of 'depends on HAS_IOMEM'
to 'depends on HAS_IOMEM || COMPILE_TEST'.
Note that s390 no has gained support for IOMEM, tile has it most
of the time (when PCI is enabled, so you get it in half the
test builds already), score should set HAS_IOMEM and doesn't
even have public compilers, and uml doesn't even compile in
latest mainline. Nothing else ever sets NO_IOMEM.
Arnd
^ permalink raw reply [flat|nested] 70+ messages in thread* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 9:20 ` Arnd Bergmann
@ 2014-07-17 9:26 ` Richard Weinberger
2014-07-17 10:28 ` Arnd Bergmann
2014-07-17 9:29 ` Chen Gang
` (2 subsequent siblings)
3 siblings, 1 reply; 70+ messages in thread
From: Richard Weinberger @ 2014-07-17 9:26 UTC (permalink / raw)
To: Arnd Bergmann, Chen Gang
Cc: Guenter Roeck, Lars-Peter Clausen, Greg Kroah-Hartman,
dmitry.torokhov, linux-iio, Benjamin Herrenschmidt, teg,
Thierry Reding, Lennox Wu, Marek Vasut, Liqin Chen, msalter,
linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, Geert Uytterhoeven
Am 17.07.2014 11:20, schrieb Arnd Bergmann:
> On Thursday 17 July 2014 09:27:58 Chen Gang wrote:
>> 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");
>> + return (__force void __iomem *)ERR_PTR(-ENXIO);
>> +}
>> +#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */
>>
>> /* allows to add/remove a custom action to devres stack */
>
> To be honest, I think it's a bad idea to introduce wrappers functions
> that are only available when CONFIG_COMPILE_TEST is set.
>
> COMPILE_TEST is a great tool in general, but it has its limits.
> In particular, the case for !CONFIG_IOMEM is completely obscure
> and we won't find any bugs by allowing more drivers to be built
> in those configurations, but attempting to do it would cause
> endless churn by changing each instance of 'depends on HAS_IOMEM'
> to 'depends on HAS_IOMEM || COMPILE_TEST'.
>
> Note that s390 no has gained support for IOMEM, tile has it most
> of the time (when PCI is enabled, so you get it in half the
> test builds already), score should set HAS_IOMEM and doesn't
> even have public compilers, and uml doesn't even compile in
> latest mainline. Nothing else ever sets NO_IOMEM.
Huh? UML (v3.16-rc5-143-gb6603fe) builds fine here. What build issue are you facing?
Thanks,
//richard
^ permalink raw reply [flat|nested] 70+ messages in thread* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 9:26 ` Richard Weinberger
@ 2014-07-17 10:28 ` Arnd Bergmann
2014-07-17 10:58 ` Richard Weinberger
0 siblings, 1 reply; 70+ messages in thread
From: Arnd Bergmann @ 2014-07-17 10:28 UTC (permalink / raw)
To: Richard Weinberger
Cc: Chen Gang, Guenter Roeck, Lars-Peter Clausen, Greg Kroah-Hartman,
dmitry.torokhov, linux-iio, Benjamin Herrenschmidt, teg,
Thierry Reding, Lennox Wu, Marek Vasut, Liqin Chen, msalter,
linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, Geert Uytterhoeven
On Thursday 17 July 2014 11:26:57 Richard Weinberger wrote:
> Am 17.07.2014 11:20, schrieb Arnd Bergmann:
> > On Thursday 17 July 2014 09:27:58 Chen Gang wrote:
> >> 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");
> >> + return (__force void __iomem *)ERR_PTR(-ENXIO);
> >> +}
> >> +#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */
> >>
> >> /* allows to add/remove a custom action to devres stack */
> >
> > To be honest, I think it's a bad idea to introduce wrappers functions
> > that are only available when CONFIG_COMPILE_TEST is set.
> >
> > COMPILE_TEST is a great tool in general, but it has its limits.
> > In particular, the case for !CONFIG_IOMEM is completely obscure
> > and we won't find any bugs by allowing more drivers to be built
> > in those configurations, but attempting to do it would cause
> > endless churn by changing each instance of 'depends on HAS_IOMEM'
> > to 'depends on HAS_IOMEM || COMPILE_TEST'.
> >
> > Note that s390 no has gained support for IOMEM, tile has it most
> > of the time (when PCI is enabled, so you get it in half the
> > test builds already), score should set HAS_IOMEM and doesn't
> > even have public compilers, and uml doesn't even compile in
> > latest mainline. Nothing else ever sets NO_IOMEM.
>
> Huh? UML (v3.16-rc5-143-gb6603fe) builds fine here. What build issue are you facing?
This is what I got upon trying earlier. I have not attempted to look into
why this is happening. Note this is on linux-next from yesterday,
not mainline as I incorrectly stated above.
In file included from ../arch/um/include/asm/fixmap.h:58:0,
from ../arch/um/include/asm/pgtable.h:11,
from ../include/linux/mm.h:51,
from ../kernel/uid16.c:6:
../include/asm-generic/fixmap.h: In function 'fix_to_virt':
../include/asm-generic/fixmap.h:31:2: error: size of unnamed array is negative
BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
Arnd
^ permalink raw reply [flat|nested] 70+ messages in thread* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 10:28 ` Arnd Bergmann
@ 2014-07-17 10:58 ` Richard Weinberger
2014-07-17 11:24 ` Arnd Bergmann
2014-07-17 11:32 ` Chen Gang
0 siblings, 2 replies; 70+ messages in thread
From: Richard Weinberger @ 2014-07-17 10:58 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Chen Gang, Guenter Roeck, Lars-Peter Clausen, Greg Kroah-Hartman,
dmitry.torokhov, linux-iio, Benjamin Herrenschmidt, teg,
Thierry Reding, Lennox Wu, Marek Vasut, Liqin Chen, msalter,
linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, Geert Uytterhoeven
Am 17.07.2014 12:28, schrieb Arnd Bergmann:
> On Thursday 17 July 2014 11:26:57 Richard Weinberger wrote:
>> Am 17.07.2014 11:20, schrieb Arnd Bergmann:
>>> On Thursday 17 July 2014 09:27:58 Chen Gang wrote:
>>>> 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");
>>>> + return (__force void __iomem *)ERR_PTR(-ENXIO);
>>>> +}
>>>> +#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */
>>>>
>>>> /* allows to add/remove a custom action to devres stack */
>>>
>>> To be honest, I think it's a bad idea to introduce wrappers functions
>>> that are only available when CONFIG_COMPILE_TEST is set.
>>>
>>> COMPILE_TEST is a great tool in general, but it has its limits.
>>> In particular, the case for !CONFIG_IOMEM is completely obscure
>>> and we won't find any bugs by allowing more drivers to be built
>>> in those configurations, but attempting to do it would cause
>>> endless churn by changing each instance of 'depends on HAS_IOMEM'
>>> to 'depends on HAS_IOMEM || COMPILE_TEST'.
>>>
>>> Note that s390 no has gained support for IOMEM, tile has it most
>>> of the time (when PCI is enabled, so you get it in half the
>>> test builds already), score should set HAS_IOMEM and doesn't
>>> even have public compilers, and uml doesn't even compile in
>>> latest mainline. Nothing else ever sets NO_IOMEM.
>>
>> Huh? UML (v3.16-rc5-143-gb6603fe) builds fine here. What build issue are you facing?
>
> This is what I got upon trying earlier. I have not attempted to look into
> why this is happening. Note this is on linux-next from yesterday,
> not mainline as I incorrectly stated above.
>
> In file included from ../arch/um/include/asm/fixmap.h:58:0,
> from ../arch/um/include/asm/pgtable.h:11,
> from ../include/linux/mm.h:51,
> from ../kernel/uid16.c:6:
> ../include/asm-generic/fixmap.h: In function 'fix_to_virt':
> ../include/asm-generic/fixmap.h:31:2: error: size of unnamed array is negative
> BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
So, this is next-20140716?
I don't see the fixmap issue you're reporting, also not on the most recent next.
All I see is the known ext4 issue with CONFIG_SMP=n:
fs/ext4/super.c: In function ‘ext4_commit_super’:
fs/ext4/super.c:4547:41: error: ‘struct percpu_counter’ has no member named ‘counters’
if (EXT4_SB(sb)->s_freeclusters_counter.counters)
^
fs/ext4/super.c:4551:39: error: ‘struct percpu_counter’ has no member named ‘counters’
if (EXT4_SB(sb)->s_freeinodes_counter.counters)
Thanks,
//richard
^ permalink raw reply [flat|nested] 70+ messages in thread* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 10:58 ` Richard Weinberger
@ 2014-07-17 11:24 ` Arnd Bergmann
2014-07-17 11:32 ` Chen Gang
1 sibling, 0 replies; 70+ messages in thread
From: Arnd Bergmann @ 2014-07-17 11:24 UTC (permalink / raw)
To: Richard Weinberger
Cc: Chen Gang, Guenter Roeck, Lars-Peter Clausen, Greg Kroah-Hartman,
dmitry.torokhov, linux-iio, Benjamin Herrenschmidt, teg,
Thierry Reding, Lennox Wu, Marek Vasut, Liqin Chen, msalter,
linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, Geert Uytterhoeven
On Thursday 17 July 2014 12:58:55 Richard Weinberger wrote:
> > This is what I got upon trying earlier. I have not attempted to look into
> > why this is happening. Note this is on linux-next from yesterday,
> > not mainline as I incorrectly stated above.
> >
> > In file included from ../arch/um/include/asm/fixmap.h:58:0,
> > from ../arch/um/include/asm/pgtable.h:11,
> > from ../include/linux/mm.h:51,
> > from ../kernel/uid16.c:6:
> > ../include/asm-generic/fixmap.h: In function 'fix_to_virt':
> > ../include/asm-generic/fixmap.h:31:2: error: size of unnamed array is negative
> > BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
>
> So, this is next-20140716?
> I don't see the fixmap issue you're reporting, also not on the most recent next.
Sorry, nevermind. I had a workaround for a gcc-4.9 bug applied that
turned off optimization for uid16.c, which fixed the build for ARM for
me but happened to break x86 including uml.
Without that patch, I don't see this problem.
Arnd
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 10:58 ` Richard Weinberger
2014-07-17 11:24 ` Arnd Bergmann
@ 2014-07-17 11:32 ` Chen Gang
1 sibling, 0 replies; 70+ messages in thread
From: Chen Gang @ 2014-07-17 11:32 UTC (permalink / raw)
To: Richard Weinberger, Arnd Bergmann
Cc: Guenter Roeck, Lars-Peter Clausen, Greg Kroah-Hartman,
dmitry.torokhov, linux-iio, Benjamin Herrenschmidt, teg,
Thierry Reding, Lennox Wu, Marek Vasut, Liqin Chen, msalter,
linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, Geert Uytterhoeven
On 07/17/2014 06:58 PM, Richard Weinberger wrote:
> Am 17.07.2014 12:28, schrieb Arnd Bergmann:
>> On Thursday 17 July 2014 11:26:57 Richard Weinberger wrote:
>>> Am 17.07.2014 11:20, schrieb Arnd Bergmann:
>>>> On Thursday 17 July 2014 09:27:58 Chen Gang wrote:
>>>>> 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");
>>>>> + return (__force void __iomem *)ERR_PTR(-ENXIO);
>>>>> +}
>>>>> +#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */
>>>>>
>>>>> /* allows to add/remove a custom action to devres stack */
>>>>
>>>> To be honest, I think it's a bad idea to introduce wrappers functions
>>>> that are only available when CONFIG_COMPILE_TEST is set.
>>>>
>>>> COMPILE_TEST is a great tool in general, but it has its limits.
>>>> In particular, the case for !CONFIG_IOMEM is completely obscure
>>>> and we won't find any bugs by allowing more drivers to be built
>>>> in those configurations, but attempting to do it would cause
>>>> endless churn by changing each instance of 'depends on HAS_IOMEM'
>>>> to 'depends on HAS_IOMEM || COMPILE_TEST'.
>>>>
>>>> Note that s390 no has gained support for IOMEM, tile has it most
>>>> of the time (when PCI is enabled, so you get it in half the
>>>> test builds already), score should set HAS_IOMEM and doesn't
>>>> even have public compilers, and uml doesn't even compile in
>>>> latest mainline. Nothing else ever sets NO_IOMEM.
>>>
>>> Huh? UML (v3.16-rc5-143-gb6603fe) builds fine here. What build issue are you facing?
>>
>> This is what I got upon trying earlier. I have not attempted to look into
>> why this is happening. Note this is on linux-next from yesterday,
>> not mainline as I incorrectly stated above.
>>
>> In file included from ../arch/um/include/asm/fixmap.h:58:0,
>> from ../arch/um/include/asm/pgtable.h:11,
>> from ../include/linux/mm.h:51,
>> from ../kernel/uid16.c:6:
>> ../include/asm-generic/fixmap.h: In function 'fix_to_virt':
>> ../include/asm-generic/fixmap.h:31:2: error: size of unnamed array is negative
>> BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
>
> So, this is next-20140716?
> I don't see the fixmap issue you're reporting, also not on the most recent next.
>
> All I see is the known ext4 issue with CONFIG_SMP=n:
>
> fs/ext4/super.c: In function ‘ext4_commit_super’:
> fs/ext4/super.c:4547:41: error: ‘struct percpu_counter’ has no member named ‘counters’
> if (EXT4_SB(sb)->s_freeclusters_counter.counters)
> ^
> fs/ext4/super.c:4551:39: error: ‘struct percpu_counter’ has no member named ‘counters’
> if (EXT4_SB(sb)->s_freeinodes_counter.counters)
>
Yeah, and I have notified to ext4 related maintainer, yesterday, and he
has already send fix patch for it, I guess it will be integrated into
main line soon.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 9:20 ` Arnd Bergmann
2014-07-17 9:26 ` Richard Weinberger
@ 2014-07-17 9:29 ` Chen Gang
2014-07-17 9:51 ` Thierry Reding
2014-07-17 10:38 ` Arnd Bergmann
2014-07-17 9:56 ` Thierry Reding
2014-07-17 10:40 ` Lars-Peter Clausen
3 siblings, 2 replies; 70+ messages in thread
From: Chen Gang @ 2014-07-17 9:29 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Guenter Roeck, Richard Weinberger, Lars-Peter Clausen,
Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
On 07/17/2014 05:20 PM, Arnd Bergmann wrote:
> On Thursday 17 July 2014 09:27:58 Chen Gang wrote:
>> 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");
>> + return (__force void __iomem *)ERR_PTR(-ENXIO);
>> +}
>> +#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */
>>
>> /* allows to add/remove a custom action to devres stack */
>
> To be honest, I think it's a bad idea to introduce wrappers functions
> that are only available when CONFIG_COMPILE_TEST is set.
>
> COMPILE_TEST is a great tool in general, but it has its limits.
> In particular, the case for !CONFIG_IOMEM is completely obscure
> and we won't find any bugs by allowing more drivers to be built
> in those configurations, but attempting to do it would cause
> endless churn by changing each instance of 'depends on HAS_IOMEM'
> to 'depends on HAS_IOMEM || COMPILE_TEST'.
>
Architecture members and driver members really have different tastes,
they are different roles. It really need additional discussion.
For me, I only want to change devm_io*map*, not touch so much.
Welcome any other members' idea or suggestions.
> Note that s390 no has gained support for IOMEM, tile has it most
> of the time (when PCI is enabled, so you get it in half the
> test builds already), score should set HAS_IOMEM and doesn't
> even have public compilers, and uml doesn't even compile in
> latest mainline. Nothing else ever sets NO_IOMEM.
>
In latest gcc and binutils, can compile score cross compiler
successfully for building kernel (but I am not quite sure whether the
compiling result are really OK, but I guess so).
And next (maybe after finish allmodconfig for microblaze), I shall try
to let uml pass allmodconfig for linux-next tree.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 9:29 ` Chen Gang
@ 2014-07-17 9:51 ` Thierry Reding
2014-07-17 10:38 ` Arnd Bergmann
1 sibling, 0 replies; 70+ messages in thread
From: Thierry Reding @ 2014-07-17 9:51 UTC (permalink / raw)
To: Chen Gang
Cc: Arnd Bergmann, Guenter Roeck, Richard Weinberger,
Lars-Peter Clausen, Greg Kroah-Hartman, dmitry.torokhov,
linux-iio, Benjamin Herrenschmidt, teg, Lennox Wu, Marek Vasut,
Liqin Chen, msalter, linux-pwm, devel, linux-watchdog,
linux-input, linux-kernel@vger.kernel.org, knaack.h,
Martin Schwidefsky, Mischa.Jonker, jic23, Geert Uytterhoeven
[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]
On Thu, Jul 17, 2014 at 05:29:31PM +0800, Chen Gang wrote:
>
>
> On 07/17/2014 05:20 PM, Arnd Bergmann wrote:
> > On Thursday 17 July 2014 09:27:58 Chen Gang wrote:
> >> 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");
> >> + return (__force void __iomem *)ERR_PTR(-ENXIO);
> >> +}
> >> +#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */
> >>
> >> /* allows to add/remove a custom action to devres stack */
> >
> > To be honest, I think it's a bad idea to introduce wrappers functions
> > that are only available when CONFIG_COMPILE_TEST is set.
> >
> > COMPILE_TEST is a great tool in general, but it has its limits.
> > In particular, the case for !CONFIG_IOMEM is completely obscure
> > and we won't find any bugs by allowing more drivers to be built
> > in those configurations, but attempting to do it would cause
> > endless churn by changing each instance of 'depends on HAS_IOMEM'
> > to 'depends on HAS_IOMEM || COMPILE_TEST'.
> >
>
> Architecture members and driver members really have different tastes,
> they are different roles. It really need additional discussion.
>
> For me, I only want to change devm_io*map*, not touch so much.
>
> Welcome any other members' idea or suggestions.
>
> > Note that s390 no has gained support for IOMEM, tile has it most
> > of the time (when PCI is enabled, so you get it in half the
> > test builds already), score should set HAS_IOMEM and doesn't
> > even have public compilers, and uml doesn't even compile in
> > latest mainline. Nothing else ever sets NO_IOMEM.
> >
>
> In latest gcc and binutils, can compile score cross compiler
> successfully for building kernel (but I am not quite sure whether the
> compiling result are really OK, but I guess so).
I was only able to ever build a partial bare-metal toolchain for score.
There's no uClibc, glibc or newlib support, so it becomes rather useless
as a Linux architecture.
Also when I run the cross-compiler on a score defconfig build, I get a
bunch of these:
score-unknown-elf-gcc-4.9.0: internal compiler error: Segmentation fault (program as)
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 70+ messages in thread* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 9:29 ` Chen Gang
2014-07-17 9:51 ` Thierry Reding
@ 2014-07-17 10:38 ` Arnd Bergmann
2014-07-17 11:46 ` Chen Gang
1 sibling, 1 reply; 70+ messages in thread
From: Arnd Bergmann @ 2014-07-17 10:38 UTC (permalink / raw)
To: Chen Gang
Cc: Guenter Roeck, Richard Weinberger, Lars-Peter Clausen,
Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
On Thursday 17 July 2014 17:29:31 Chen Gang wrote:
> >
> > COMPILE_TEST is a great tool in general, but it has its limits.
> > In particular, the case for !CONFIG_IOMEM is completely obscure
> > and we won't find any bugs by allowing more drivers to be built
> > in those configurations, but attempting to do it would cause
> > endless churn by changing each instance of 'depends on HAS_IOMEM'
> > to 'depends on HAS_IOMEM || COMPILE_TEST'.
> >
>
> Architecture members and driver members really have different tastes,
> they are different roles. It really need additional discussion.
>
> For me, I only want to change devm_io*map*, not touch so much.
But what do you gain from that? All drivers that need these
functions should already 'depends on HAS_IOMEM' and if they don't,
we should fix /that/ instead. I don't see this dependency as any
different from a lot of others (PCI, DMAENGINE, HAVE_CLK, ...)
that we use to intentionally annotate drivers that need a particular
feature to be present for compilation. Do you want to do the
same hack to those?
> Welcome any other members' idea or suggestions.
> > Note that s390 no has gained support for IOMEM, tile has it most
> > of the time (when PCI is enabled, so you get it in half the
> > test builds already), score should set HAS_IOMEM and doesn't
> > even have public compilers, and uml doesn't even compile in
> > latest mainline. Nothing else ever sets NO_IOMEM.
> >
>
> In latest gcc and binutils, can compile score cross compiler
> successfully for building kernel (but I am not quite sure whether the
> compiling result are really OK, but I guess so).
Ok. Would you mind sending a patch that enables HAS_IOMEM on
score?
> And next (maybe after finish allmodconfig for microblaze), I shall try
> to let uml pass allmodconfig for linux-next tree.
That is a fair goal, but it seems better to do that by ensuring
we don't build any code that tries to call the MMIO functions
rather than trying to make them build.
Arnd
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 10:38 ` Arnd Bergmann
@ 2014-07-17 11:46 ` Chen Gang
0 siblings, 0 replies; 70+ messages in thread
From: Chen Gang @ 2014-07-17 11:46 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Guenter Roeck, Richard Weinberger, Lars-Peter Clausen,
Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, jic23, Geert Uytterhoeven,
Liqin Chen, Lennox Wu
On 07/17/2014 06:38 PM, Arnd Bergmann wrote:
> On Thursday 17 July 2014 17:29:31 Chen Gang wrote:
>>>
>>> COMPILE_TEST is a great tool in general, but it has its limits.
>>> In particular, the case for !CONFIG_IOMEM is completely obscure
>>> and we won't find any bugs by allowing more drivers to be built
>>> in those configurations, but attempting to do it would cause
>>> endless churn by changing each instance of 'depends on HAS_IOMEM'
>>> to 'depends on HAS_IOMEM || COMPILE_TEST'.
>>>
>>
>> Architecture members and driver members really have different tastes,
>> they are different roles. It really need additional discussion.
>>
>> For me, I only want to change devm_io*map*, not touch so much.
>
> But what do you gain from that? All drivers that need these
> functions should already 'depends on HAS_IOMEM' and if they don't,
> we should fix /that/ instead. I don't see this dependency as any
> different from a lot of others (PCI, DMAENGINE, HAVE_CLK, ...)
> that we use to intentionally annotate drivers that need a particular
> feature to be present for compilation. Do you want to do the
> same hack to those?
>
>> Welcome any other members' idea or suggestions.
>
>>> Note that s390 no has gained support for IOMEM, tile has it most
>>> of the time (when PCI is enabled, so you get it in half the
>>> test builds already), score should set HAS_IOMEM and doesn't
>>> even have public compilers, and uml doesn't even compile in
>>> latest mainline. Nothing else ever sets NO_IOMEM.
>>>
>>
I guess, we are just discussing about them in another threads, so I skip
them. If it is still necessary to reply (e.g. I misunderstand), please
let me know, thanks.
>> In latest gcc and binutils, can compile score cross compiler
>> successfully for building kernel (but I am not quite sure whether the
>> compiling result are really OK, but I guess so).
>
> Ok. Would you mind sending a patch that enables HAS_IOMEM on
> score?
>
For me, welcome the score related maintainers' idea and suggestions.
>> And next (maybe after finish allmodconfig for microblaze), I shall try
>> to let uml pass allmodconfig for linux-next tree.
>
> That is a fair goal, but it seems better to do that by ensuring
> we don't build any code that tries to call the MMIO functions
> rather than trying to make them build.
>
When I am performing uml, I will try and also communicate with the
related maintainers for it (their suggestions and ideas are valuable).
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 9:20 ` Arnd Bergmann
2014-07-17 9:26 ` Richard Weinberger
2014-07-17 9:29 ` Chen Gang
@ 2014-07-17 9:56 ` Thierry Reding
2014-07-17 10:33 ` Arnd Bergmann
2014-07-17 10:40 ` Lars-Peter Clausen
3 siblings, 1 reply; 70+ messages in thread
From: Thierry Reding @ 2014-07-17 9:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Chen Gang, Guenter Roeck, Richard Weinberger, Lars-Peter Clausen,
Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Lennox Wu, Marek Vasut, Liqin Chen,
msalter, linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, Geert Uytterhoeven
[-- Attachment #1: Type: text/plain, Size: 451 bytes --]
On Thu, Jul 17, 2014 at 11:20:36AM +0200, Arnd Bergmann wrote:
[...]
> score should set HAS_IOMEM and doesn't
> even have public compilers
This begs an interesting question. Should it be made a requirement to
have publicly available compilers for new architectures so that they can
at least be compile-tested? Preferably this would of course be in source
form so that there aren't any dependencies on the distribution.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 9:56 ` Thierry Reding
@ 2014-07-17 10:33 ` Arnd Bergmann
2014-07-17 10:55 ` Thierry Reding
0 siblings, 1 reply; 70+ messages in thread
From: Arnd Bergmann @ 2014-07-17 10:33 UTC (permalink / raw)
To: Thierry Reding
Cc: Chen Gang, Guenter Roeck, Richard Weinberger, Lars-Peter Clausen,
Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Lennox Wu, Marek Vasut, Liqin Chen,
msalter, linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, Geert Uytterhoeven
On Thursday 17 July 2014 11:56:58 Thierry Reding wrote:
> On Thu, Jul 17, 2014 at 11:20:36AM +0200, Arnd Bergmann wrote:
> [...]
> > score should set HAS_IOMEM and doesn't
> > even have public compilers
>
> This begs an interesting question. Should it be made a requirement to
> have publicly available compilers for new architectures so that they can
> at least be compile-tested? Preferably this would of course be in source
> form so that there aren't any dependencies on the distribution.
The question has come up a few times. I wouldn't mandate that the port
has an upstream gcc (you've got to start mainlining one of them first
after all), but having compilers available for download should probably be
required. It's hard to ask for a particular quality of that gcc port
though, or to expect it to stay available online.
Where did you find the gcc port for score?
Arnd
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 10:33 ` Arnd Bergmann
@ 2014-07-17 10:55 ` Thierry Reding
2014-07-17 11:20 ` Chen Gang
0 siblings, 1 reply; 70+ messages in thread
From: Thierry Reding @ 2014-07-17 10:55 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Chen Gang, Guenter Roeck, Richard Weinberger, Lars-Peter Clausen,
Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Lennox Wu, Marek Vasut, Liqin Chen,
msalter, linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, Geert Uytterhoeven
[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]
On Thu, Jul 17, 2014 at 12:33:32PM +0200, Arnd Bergmann wrote:
> On Thursday 17 July 2014 11:56:58 Thierry Reding wrote:
> > On Thu, Jul 17, 2014 at 11:20:36AM +0200, Arnd Bergmann wrote:
> > [...]
> > > score should set HAS_IOMEM and doesn't
> > > even have public compilers
> >
> > This begs an interesting question. Should it be made a requirement to
> > have publicly available compilers for new architectures so that they can
> > at least be compile-tested? Preferably this would of course be in source
> > form so that there aren't any dependencies on the distribution.
>
> The question has come up a few times. I wouldn't mandate that the port
> has an upstream gcc (you've got to start mainlining one of them first
> after all), but having compilers available for download should probably be
> required. It's hard to ask for a particular quality of that gcc port
> though, or to expect it to stay available online.
>
> Where did you find the gcc port for score?
It's upstream, though marked obsolete and to be removed in the next
release... =)
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 10:55 ` Thierry Reding
@ 2014-07-17 11:20 ` Chen Gang
0 siblings, 0 replies; 70+ messages in thread
From: Chen Gang @ 2014-07-17 11:20 UTC (permalink / raw)
To: Thierry Reding, Arnd Bergmann
Cc: Guenter Roeck, Richard Weinberger, Lars-Peter Clausen,
Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Lennox Wu, Marek Vasut, Liqin Chen,
msalter, linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, Geert Uytterhoeven, Liqin Chen, Lennox Wu
On 07/17/2014 06:55 PM, Thierry Reding wrote:
> On Thu, Jul 17, 2014 at 12:33:32PM +0200, Arnd Bergmann wrote:
>> On Thursday 17 July 2014 11:56:58 Thierry Reding wrote:
>>> On Thu, Jul 17, 2014 at 11:20:36AM +0200, Arnd Bergmann wrote:
>>> [...]
>>>> score should set HAS_IOMEM and doesn't
>>>> even have public compilers
>>>
>>> This begs an interesting question. Should it be made a requirement to
>>> have publicly available compilers for new architectures so that they can
>>> at least be compile-tested? Preferably this would of course be in source
>>> form so that there aren't any dependencies on the distribution.
>>
>> The question has come up a few times. I wouldn't mandate that the port
>> has an upstream gcc (you've got to start mainlining one of them first
>> after all), but having compilers available for download should probably be
>> required. It's hard to ask for a particular quality of that gcc port
>> though, or to expect it to stay available online.
>>
>> Where did you find the gcc port for score?
>
> It's upstream, though marked obsolete and to be removed in the next
> release... =)
>
For me, I get the latest gcc version and binutils source code, and fix 2
bugs (one for gas, which always generate core dump, the other for gcc
c-decl, fix it together with the other gcc members).
And I only finish compiling raw cross-compiler (--without-headers),
after make some patches, can let score pass allmodconfig.
At present, it seems the score cross-compiler still contents some issue
which I shall try to analyse (it is about link symbols), and maybe need
communicate with gcc/binutils members.
At present, the related score maintainers are still active in upstream
kernel, so we also need the related maintainers' ideas and suggestions.
Thanks
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 9:20 ` Arnd Bergmann
` (2 preceding siblings ...)
2014-07-17 9:56 ` Thierry Reding
@ 2014-07-17 10:40 ` Lars-Peter Clausen
2014-07-17 10:48 ` Arnd Bergmann
3 siblings, 1 reply; 70+ messages in thread
From: Lars-Peter Clausen @ 2014-07-17 10:40 UTC (permalink / raw)
To: Arnd Bergmann, Chen Gang
Cc: Guenter Roeck, Richard Weinberger, Greg Kroah-Hartman,
dmitry.torokhov, linux-iio, Benjamin Herrenschmidt, teg,
Thierry Reding, Lennox Wu, Marek Vasut, Liqin Chen, msalter,
linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, Geert Uytterhoeven
On 07/17/2014 11:20 AM, Arnd Bergmann wrote:
> On Thursday 17 July 2014 09:27:58 Chen Gang wrote:
>> 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");
>> + return (__force void __iomem *)ERR_PTR(-ENXIO);
>> +}
>> +#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */
>>
>> /* allows to add/remove a custom action to devres stack */
>
> To be honest, I think it's a bad idea to introduce wrappers functions
> that are only available when CONFIG_COMPILE_TEST is set.
>
> COMPILE_TEST is a great tool in general, but it has its limits.
> In particular, the case for !CONFIG_IOMEM is completely obscure
> and we won't find any bugs by allowing more drivers to be built
> in those configurations, but attempting to do it would cause
> endless churn by changing each instance of 'depends on HAS_IOMEM'
> to 'depends on HAS_IOMEM || COMPILE_TEST'.
The point of this exercise is that we do not have to replace a good chunk of
'depends on COMPILE_TEST' with 'depends on COMPILE_TEST && HAS_IOMEM'
E.g. the typical Kconfig entry for your random SoC peripheral driver looks like
config ARCH_FOOBAR_DRIVER
depends on ARCH_FOOBAR || COMPILE_TEST
...
Now when COMPILE_TEST is not set there is a implicit dependency on HAS_IOMEM
since the architecture will provide it. If COMPILE_TEST is selected the
driver will also be build-able on architectures that do no have HAS_IOMEM
and hence linking the driver fails. One way to fix this is of course to
replace the COMPILE_TEST with (COMPILE_TEST && HAS_IOMEM). But this is very
often overlooked and only noticed later on when somebody actually builds a
allyesconfig on an architecture that does not provide HAS_IOMEM. To avoid
these kinds of build errors and tedious fixup patches the idea is to provide
a stub function when HAS_IOMEM is not enabled, but COMPILE_TEST is enabled.
^ permalink raw reply [flat|nested] 70+ messages in thread* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 10:40 ` Lars-Peter Clausen
@ 2014-07-17 10:48 ` Arnd Bergmann
2014-07-17 11:28 ` Chen Gang
2014-07-17 18:09 ` Richard Weinberger
0 siblings, 2 replies; 70+ messages in thread
From: Arnd Bergmann @ 2014-07-17 10:48 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Chen Gang, Guenter Roeck, Richard Weinberger, Greg Kroah-Hartman,
dmitry.torokhov, linux-iio, Benjamin Herrenschmidt, teg,
Thierry Reding, Lennox Wu, Marek Vasut, Liqin Chen, msalter,
linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, Geert Uytterhoeven
On Thursday 17 July 2014 12:40:25 Lars-Peter Clausen wrote:
> On 07/17/2014 11:20 AM, Arnd Bergmann wrote:
> > On Thursday 17 July 2014 09:27:58 Chen Gang wrote:
> >> 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");
> >> + return (__force void __iomem *)ERR_PTR(-ENXIO);
> >> +}
> >> +#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */
> >>
> >> /* allows to add/remove a custom action to devres stack */
> >
> > To be honest, I think it's a bad idea to introduce wrappers functions
> > that are only available when CONFIG_COMPILE_TEST is set.
> >
> > COMPILE_TEST is a great tool in general, but it has its limits.
> > In particular, the case for !CONFIG_IOMEM is completely obscure
> > and we won't find any bugs by allowing more drivers to be built
> > in those configurations, but attempting to do it would cause
> > endless churn by changing each instance of 'depends on HAS_IOMEM'
> > to 'depends on HAS_IOMEM || COMPILE_TEST'.
>
> The point of this exercise is that we do not have to replace a good chunk of
> 'depends on COMPILE_TEST' with 'depends on COMPILE_TEST && HAS_IOMEM'
Ok, I see.
> E.g. the typical Kconfig entry for your random SoC peripheral driver looks like
>
> config ARCH_FOOBAR_DRIVER
> depends on ARCH_FOOBAR || COMPILE_TEST
> ...
>
> Now when COMPILE_TEST is not set there is a implicit dependency on HAS_IOMEM
> since the architecture will provide it. If COMPILE_TEST is selected the
> driver will also be build-able on architectures that do no have HAS_IOMEM
> and hence linking the driver fails. One way to fix this is of course to
> replace the COMPILE_TEST with (COMPILE_TEST && HAS_IOMEM). But this is very
> often overlooked and only noticed later on when somebody actually builds a
> allyesconfig on an architecture that does not provide HAS_IOMEM. To avoid
> these kinds of build errors and tedious fixup patches the idea is to provide
> a stub function when HAS_IOMEM is not enabled, but COMPILE_TEST is enabled.
AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
to build on UML seems pointless to me and we special-case it in a number of
places already.
Arnd
^ permalink raw reply [flat|nested] 70+ messages in thread* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 10:48 ` Arnd Bergmann
@ 2014-07-17 11:28 ` Chen Gang
2014-07-17 20:41 ` Chris Metcalf
2014-07-17 18:09 ` Richard Weinberger
1 sibling, 1 reply; 70+ messages in thread
From: Chen Gang @ 2014-07-17 11:28 UTC (permalink / raw)
To: Arnd Bergmann, Lars-Peter Clausen
Cc: Guenter Roeck, Richard Weinberger, Greg Kroah-Hartman,
dmitry.torokhov, linux-iio, Benjamin Herrenschmidt, teg,
Thierry Reding, Lennox Wu, Marek Vasut, Liqin Chen, msalter,
linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, Geert Uytterhoeven, cmetcalf
On 07/17/2014 06:48 PM, Arnd Bergmann wrote:
> On Thursday 17 July 2014 12:40:25 Lars-Peter Clausen wrote:
>> On 07/17/2014 11:20 AM, Arnd Bergmann wrote:
>>> On Thursday 17 July 2014 09:27:58 Chen Gang wrote:
>>>> 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");
>>>> + return (__force void __iomem *)ERR_PTR(-ENXIO);
>>>> +}
>>>> +#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */
>>>>
>>>> /* allows to add/remove a custom action to devres stack */
>>>
>>> To be honest, I think it's a bad idea to introduce wrappers functions
>>> that are only available when CONFIG_COMPILE_TEST is set.
>>>
>>> COMPILE_TEST is a great tool in general, but it has its limits.
>>> In particular, the case for !CONFIG_IOMEM is completely obscure
>>> and we won't find any bugs by allowing more drivers to be built
>>> in those configurations, but attempting to do it would cause
>>> endless churn by changing each instance of 'depends on HAS_IOMEM'
>>> to 'depends on HAS_IOMEM || COMPILE_TEST'.
>>
>> The point of this exercise is that we do not have to replace a good chunk of
>> 'depends on COMPILE_TEST' with 'depends on COMPILE_TEST && HAS_IOMEM'
>
> Ok, I see.
>
>> E.g. the typical Kconfig entry for your random SoC peripheral driver looks like
>>
>> config ARCH_FOOBAR_DRIVER
>> depends on ARCH_FOOBAR || COMPILE_TEST
>> ...
>>
>> Now when COMPILE_TEST is not set there is a implicit dependency on HAS_IOMEM
>> since the architecture will provide it. If COMPILE_TEST is selected the
>> driver will also be build-able on architectures that do no have HAS_IOMEM
>> and hence linking the driver fails. One way to fix this is of course to
>> replace the COMPILE_TEST with (COMPILE_TEST && HAS_IOMEM). But this is very
>> often overlooked and only noticed later on when somebody actually builds a
>> allyesconfig on an architecture that does not provide HAS_IOMEM. To avoid
>> these kinds of build errors and tedious fixup patches the idea is to provide
>> a stub function when HAS_IOMEM is not enabled, but COMPILE_TEST is enabled.
>
> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
> to build on UML seems pointless to me and we special-case it in a number of
> places already.
>
According to current source code, tile still has chance to choose
NO_IOMEM, for me, welcome the tile's maintainer's ideas or suggestions.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 11:28 ` Chen Gang
@ 2014-07-17 20:41 ` Chris Metcalf
2014-07-17 21:05 ` Arnd Bergmann
0 siblings, 1 reply; 70+ messages in thread
From: Chris Metcalf @ 2014-07-17 20:41 UTC (permalink / raw)
To: Chen Gang, Arnd Bergmann, Lars-Peter Clausen
Cc: Guenter Roeck, Richard Weinberger, Greg Kroah-Hartman,
dmitry.torokhov, linux-iio, Benjamin Herrenschmidt, teg,
Thierry Reding, Lennox Wu, Marek Vasut, Liqin Chen, msalter,
linux-pwm, devel, linux-watchdog, linux-input,
linux-kernel@vger.kernel.org, knaack.h, Martin Schwidefsky,
Mischa.Jonker, jic23, Geert Uytterhoeven
On 7/17/2014 7:28 AM, Chen Gang wrote:
> On 07/17/2014 06:48 PM, Arnd Bergmann wrote:
>> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
>> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
>> to build on UML seems pointless to me and we special-case it in a number of
>> places already.
>>
> According to current source code, tile still has chance to choose
> NO_IOMEM, for me, welcome the tile's maintainer's ideas or suggestions.
I'm not really sure. It's true that on tile, if you don't enable PCI
support there's no other I/O memory (or I/O port) space you can use.
We pretty much always enable PCI support in our kernel, though. I'm
kind of surprised that other architectures don't also have the model
that IOMEM requires PCI, but perhaps most architectures just don't
encode that in the Kconfig file?
My observation is just that if I remove the "NO_IOMEM if !PCI" from
arch/tile/Kconfig, my build fails with ioremap() undefined. No doubt I
could work around that, but my assumption was that NO_IOMEM was exactly the
right thing to express the fact that without PCI there is no I/O memory :-)
--
Chris Metcalf, Tilera Corp.
http://www.tilera.com
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 20:41 ` Chris Metcalf
@ 2014-07-17 21:05 ` Arnd Bergmann
2014-07-18 0:26 ` Chen Gang
2014-07-31 20:09 ` Chris Metcalf
0 siblings, 2 replies; 70+ messages in thread
From: Arnd Bergmann @ 2014-07-17 21:05 UTC (permalink / raw)
To: Chris Metcalf
Cc: Chen Gang, Lars-Peter Clausen, Guenter Roeck, Richard Weinberger,
Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
On Thursday 17 July 2014 16:41:14 Chris Metcalf wrote:
> On 7/17/2014 7:28 AM, Chen Gang wrote:
> > On 07/17/2014 06:48 PM, Arnd Bergmann wrote:
> >> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
> >> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
> >> to build on UML seems pointless to me and we special-case it in a number of
> >> places already.
> >>
> > According to current source code, tile still has chance to choose
> > NO_IOMEM, for me, welcome the tile's maintainer's ideas or suggestions.
>
> I'm not really sure. It's true that on tile, if you don't enable PCI
> support there's no other I/O memory (or I/O port) space you can use.
> We pretty much always enable PCI support in our kernel, though. I'm
> kind of surprised that other architectures don't also have the model
> that IOMEM requires PCI, but perhaps most architectures just don't
> encode that in the Kconfig file?
Only s390 as far as I know. Most architectures have integrated
peripherals that use MMIO without PCI.
> My observation is just that if I remove the "NO_IOMEM if !PCI" from
> arch/tile/Kconfig, my build fails with ioremap() undefined. No doubt I
> could work around that, but my assumption was that NO_IOMEM was exactly the
> right thing to express the fact that without PCI there is no I/O memory
Your assumption is correct.
For tile by itself it would certainly be best to leave this
dependency, it makes no sense to enable IOMEM without PCI.
That doesn't solve the problem of COMPILE_TEST enabling drivers
that require IOMEM though. An easy hack for that would be to
make COMPILE_TEST depend on HAS_IOMEM, but it gets into hacky territory
there, and it's not clear if this is any better than the original patch
to provide fallbacks for ioremap and friends. Definitely simpler
though.
Arnd
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 21:05 ` Arnd Bergmann
@ 2014-07-18 0:26 ` Chen Gang
2014-07-31 20:09 ` Chris Metcalf
1 sibling, 0 replies; 70+ messages in thread
From: Chen Gang @ 2014-07-18 0:26 UTC (permalink / raw)
To: Arnd Bergmann, Chris Metcalf
Cc: Lars-Peter Clausen, Guenter Roeck, Richard Weinberger,
Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
On 07/18/2014 05:05 AM, Arnd Bergmann wrote:
> On Thursday 17 July 2014 16:41:14 Chris Metcalf wrote:
>> On 7/17/2014 7:28 AM, Chen Gang wrote:
>>> On 07/17/2014 06:48 PM, Arnd Bergmann wrote:
>>>> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
>>>> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
>>>> to build on UML seems pointless to me and we special-case it in a number of
>>>> places already.
>>>>
>>> According to current source code, tile still has chance to choose
>>> NO_IOMEM, for me, welcome the tile's maintainer's ideas or suggestions.
>>
>> I'm not really sure. It's true that on tile, if you don't enable PCI
>> support there's no other I/O memory (or I/O port) space you can use.
>> We pretty much always enable PCI support in our kernel, though. I'm
>> kind of surprised that other architectures don't also have the model
>> that IOMEM requires PCI, but perhaps most architectures just don't
>> encode that in the Kconfig file?
>
> Only s390 as far as I know. Most architectures have integrated
> peripherals that use MMIO without PCI.
>
>> My observation is just that if I remove the "NO_IOMEM if !PCI" from
>> arch/tile/Kconfig, my build fails with ioremap() undefined. No doubt I
>> could work around that, but my assumption was that NO_IOMEM was exactly the
>> right thing to express the fact that without PCI there is no I/O memory
>
> Your assumption is correct.
>
> For tile by itself it would certainly be best to leave this
> dependency, it makes no sense to enable IOMEM without PCI.
>
> That doesn't solve the problem of COMPILE_TEST enabling drivers
> that require IOMEM though. An easy hack for that would be to
> make COMPILE_TEST depend on HAS_IOMEM, but it gets into hacky territory
> there, and it's not clear if this is any better than the original patch
> to provide fallbacks for ioremap and friends. Definitely simpler
> though.
>
OK, thank all of you, tile just likes most of architectures to support
IOMEM, and at present, we can focus score and uml only.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 21:05 ` Arnd Bergmann
2014-07-18 0:26 ` Chen Gang
@ 2014-07-31 20:09 ` Chris Metcalf
1 sibling, 0 replies; 70+ messages in thread
From: Chris Metcalf @ 2014-07-31 20:09 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Chen Gang, Lars-Peter Clausen, Guenter Roeck, Richard Weinberger,
Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
On 7/17/2014 5:05 PM, Arnd Bergmann wrote:
> On Thursday 17 July 2014 16:41:14 Chris Metcalf wrote:
>> On 7/17/2014 7:28 AM, Chen Gang wrote:
>>> According to current source code, tile still has chance to choose
>>> NO_IOMEM, for me, welcome the tile's maintainer's ideas or suggestions.
>> I'm not really sure. It's true that on tile, if you don't enable PCI
>> support there's no other I/O memory (or I/O port) space you can use.
>> We pretty much always enable PCI support in our kernel, though. I'm
>> kind of surprised that other architectures don't also have the model
>> that IOMEM requires PCI, but perhaps most architectures just don't
>> encode that in the Kconfig file?
> Only s390 as far as I know. Most architectures have integrated
> peripherals that use MMIO without PCI.
Yes, and tilegx has these too (quite a few of them). The memory-mapped
devices are accessed by specifying a shim x,y coordinate in the high bits
of the physical address, in conjunction with an MMIO type in the TLB entry.
Various tilegx drivers set up these kinds of mappings in the page table.
The issue with ioremap() is that it takes a generic resource_size_t
"physical address", and we don't have any general-purpose layer that maps
particular PAs to shim coordinates, other than for TRIO (our PCI peripheral).
Right now we just check the PCI root complexes that we have configured in
the kernel, and if the pseudo physical address requested is in a range that
we're associating with one of the root complexes, we will use the appropriate
mapping against the appropriate TRIO shim to set up its x,y coordinate in
the page table.
So it makes some kind of sense to condition HAS_IOMEM on PCI, even though
naively it seems like it shouldn't be strictly related.
--
Chris Metcalf, Tilera Corp.
http://www.tilera.com
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 10:48 ` Arnd Bergmann
2014-07-17 11:28 ` Chen Gang
@ 2014-07-17 18:09 ` Richard Weinberger
2014-07-18 0:36 ` Chen Gang
1 sibling, 1 reply; 70+ messages in thread
From: Richard Weinberger @ 2014-07-17 18:09 UTC (permalink / raw)
To: Arnd Bergmann, Lars-Peter Clausen
Cc: Chen Gang, Guenter Roeck, Greg Kroah-Hartman, dmitry.torokhov,
linux-iio, Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
Am 17.07.2014 12:48, schrieb Arnd Bergmann:
> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
> to build on UML seems pointless to me and we special-case it in a number of
> places already.
If UML is the only arch without io memory the dependency on !UML seems
reasonable to me. :-)
Thanks,
//richard
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-17 18:09 ` Richard Weinberger
@ 2014-07-18 0:36 ` Chen Gang
2014-07-18 7:35 ` Richard Weinberger
0 siblings, 1 reply; 70+ messages in thread
From: Chen Gang @ 2014-07-18 0:36 UTC (permalink / raw)
To: Richard Weinberger, Arnd Bergmann, Lars-Peter Clausen
Cc: Guenter Roeck, Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
On 07/18/2014 02:09 AM, Richard Weinberger wrote:
> Am 17.07.2014 12:48, schrieb Arnd Bergmann:
>> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
>> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
>> to build on UML seems pointless to me and we special-case it in a number of
>> places already.
>
> If UML is the only arch without io memory the dependency on !UML seems
> reasonable to me. :-)
>
For me, if only uml left, I suggest to implement dummy functions within
uml instead of let CONFIG_UML appear in generic include directory. And
then remove all HAS_IOMEM and NO_IOMEM from kernel.
If score sticks to NO_IOMEM, and can not remove score from kernel only
because of this reason. I also suggest implement dummy functions within
score itself.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-18 0:36 ` Chen Gang
@ 2014-07-18 7:35 ` Richard Weinberger
2014-07-18 10:44 ` Chen Gang
0 siblings, 1 reply; 70+ messages in thread
From: Richard Weinberger @ 2014-07-18 7:35 UTC (permalink / raw)
To: Chen Gang, Arnd Bergmann, Lars-Peter Clausen
Cc: Guenter Roeck, Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
Am 18.07.2014 02:36, schrieb Chen Gang:
>
> On 07/18/2014 02:09 AM, Richard Weinberger wrote:
>> Am 17.07.2014 12:48, schrieb Arnd Bergmann:
>>> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
>>> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
>>> to build on UML seems pointless to me and we special-case it in a number of
>>> places already.
>>
>> If UML is the only arch without io memory the dependency on !UML seems
>> reasonable to me. :-)
>>
>
> For me, if only uml left, I suggest to implement dummy functions within
> uml instead of let CONFIG_UML appear in generic include directory. And
> then remove all HAS_IOMEM and NO_IOMEM from kernel.
Erm, this is something completely different.
I thought we're focusing on COMPILE_TEST?
Thanks,
//richard
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-18 7:35 ` Richard Weinberger
@ 2014-07-18 10:44 ` Chen Gang
2014-07-18 10:51 ` Richard Weinberger
0 siblings, 1 reply; 70+ messages in thread
From: Chen Gang @ 2014-07-18 10:44 UTC (permalink / raw)
To: Richard Weinberger, Arnd Bergmann, Lars-Peter Clausen
Cc: Guenter Roeck, Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
On 07/18/2014 03:35 PM, Richard Weinberger wrote:
> Am 18.07.2014 02:36, schrieb Chen Gang:
>>
>> On 07/18/2014 02:09 AM, Richard Weinberger wrote:
>>> Am 17.07.2014 12:48, schrieb Arnd Bergmann:
>>>> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
>>>> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
>>>> to build on UML seems pointless to me and we special-case it in a number of
>>>> places already.
>>>
>>> If UML is the only arch without io memory the dependency on !UML seems
>>> reasonable to me. :-)
>>>
>>
>> For me, if only uml left, I suggest to implement dummy functions within
>> uml instead of let CONFIG_UML appear in generic include directory. And
>> then remove all HAS_IOMEM and NO_IOMEM from kernel.
>
> Erm, this is something completely different.
> I thought we're focusing on COMPILE_TEST?
>
COMPILE_TEST is none-architecture specific, but UML is. So in generic
include folder, if we're focusing on choosing whether COMPILE_TEST or
UML, for me, I will choose COMPILE_TEST.
If we're not only focusing on COMPILE_TEST, for me, if something only
depend on one architecture, I'd like to put them under "arch/*/" folder.
Especially, after that, we can remove all HAS_IOMEM and NO_IOMEM, nobody
has to think of them again. :-)
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-18 10:44 ` Chen Gang
@ 2014-07-18 10:51 ` Richard Weinberger
2014-07-18 15:37 ` Lennox Wu
0 siblings, 1 reply; 70+ messages in thread
From: Richard Weinberger @ 2014-07-18 10:51 UTC (permalink / raw)
To: Chen Gang, Arnd Bergmann, Lars-Peter Clausen
Cc: Guenter Roeck, Greg Kroah-Hartman, dmitry.torokhov, linux-iio,
Benjamin Herrenschmidt, teg, Thierry Reding, Lennox Wu,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
Am 18.07.2014 12:44, schrieb Chen Gang:
> On 07/18/2014 03:35 PM, Richard Weinberger wrote:
>> Am 18.07.2014 02:36, schrieb Chen Gang:
>>>
>>> On 07/18/2014 02:09 AM, Richard Weinberger wrote:
>>>> Am 17.07.2014 12:48, schrieb Arnd Bergmann:
>>>>> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
>>>>> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
>>>>> to build on UML seems pointless to me and we special-case it in a number of
>>>>> places already.
>>>>
>>>> If UML is the only arch without io memory the dependency on !UML seems
>>>> reasonable to me. :-)
>>>>
>>>
>>> For me, if only uml left, I suggest to implement dummy functions within
>>> uml instead of let CONFIG_UML appear in generic include directory. And
>>> then remove all HAS_IOMEM and NO_IOMEM from kernel.
>>
>> Erm, this is something completely different.
>> I thought we're focusing on COMPILE_TEST?
>>
>
> COMPILE_TEST is none-architecture specific, but UML is. So in generic
> include folder, if we're focusing on choosing whether COMPILE_TEST or
> UML, for me, I will choose COMPILE_TEST.
>
> If we're not only focusing on COMPILE_TEST, for me, if something only
> depend on one architecture, I'd like to put them under "arch/*/" folder.
>
> Especially, after that, we can remove all HAS_IOMEM and NO_IOMEM, nobody
> has to think of them again. :-)
And then we end up with a solution that on UML a lot of completely useless
drivers are build which fail in various interesting manners because you'll
add stubs for all kinds of io memory related functions to arch/um/?
We had this kind of discussion already. You'll need more than ioremap...
I like Arnd's idea *much* more to make COMPILE_TEST depend on !UML.
Thanks,
//richard
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-18 10:51 ` Richard Weinberger
@ 2014-07-18 15:37 ` Lennox Wu
2014-07-18 18:02 ` Chen Gang
0 siblings, 1 reply; 70+ messages in thread
From: Lennox Wu @ 2014-07-18 15:37 UTC (permalink / raw)
To: Richard Weinberger
Cc: Chen Gang, Arnd Bergmann, Lars-Peter Clausen, Guenter Roeck,
Greg Kroah-Hartman, Dmitry Torokhov, linux-iio,
Benjamin Herrenschmidt, Tom Gundersen, Thierry Reding,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
Score can provide dummy functions if HAS_IOMEM and NO_IOMEM will be
removed, even if we indeed have no IOMEM.
Best,
Lennox
2014-07-18 18:51 GMT+08:00 Richard Weinberger <richard@nod.at>:
> Am 18.07.2014 12:44, schrieb Chen Gang:
>> On 07/18/2014 03:35 PM, Richard Weinberger wrote:
>>> Am 18.07.2014 02:36, schrieb Chen Gang:
>>>>
>>>> On 07/18/2014 02:09 AM, Richard Weinberger wrote:
>>>>> Am 17.07.2014 12:48, schrieb Arnd Bergmann:
>>>>>> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
>>>>>> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
>>>>>> to build on UML seems pointless to me and we special-case it in a number of
>>>>>> places already.
>>>>>
>>>>> If UML is the only arch without io memory the dependency on !UML seems
>>>>> reasonable to me. :-)
>>>>>
>>>>
>>>> For me, if only uml left, I suggest to implement dummy functions within
>>>> uml instead of let CONFIG_UML appear in generic include directory. And
>>>> then remove all HAS_IOMEM and NO_IOMEM from kernel.
>>>
>>> Erm, this is something completely different.
>>> I thought we're focusing on COMPILE_TEST?
>>>
>>
>> COMPILE_TEST is none-architecture specific, but UML is. So in generic
>> include folder, if we're focusing on choosing whether COMPILE_TEST or
>> UML, for me, I will choose COMPILE_TEST.
>>
>> If we're not only focusing on COMPILE_TEST, for me, if something only
>> depend on one architecture, I'd like to put them under "arch/*/" folder.
>>
>> Especially, after that, we can remove all HAS_IOMEM and NO_IOMEM, nobody
>> has to think of them again. :-)
>
> And then we end up with a solution that on UML a lot of completely useless
> drivers are build which fail in various interesting manners because you'll
> add stubs for all kinds of io memory related functions to arch/um/?
> We had this kind of discussion already. You'll need more than ioremap...
>
> I like Arnd's idea *much* more to make COMPILE_TEST depend on !UML.
>
> Thanks,
> //richard
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-18 15:37 ` Lennox Wu
@ 2014-07-18 18:02 ` Chen Gang
2014-07-20 8:38 ` Chen Gang
0 siblings, 1 reply; 70+ messages in thread
From: Chen Gang @ 2014-07-18 18:02 UTC (permalink / raw)
To: Lennox Wu
Cc: Richard Weinberger, Arnd Bergmann, Lars-Peter Clausen,
Guenter Roeck, Greg Kroah-Hartman, Dmitry Torokhov, linux-iio,
Benjamin Herrenschmidt, Tom Gundersen, Thierry Reding,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
On 07/18/2014 11:37 PM, Lennox Wu wrote:
> Score can provide dummy functions if HAS_IOMEM and NO_IOMEM will be
> removed, even if we indeed have no IOMEM.
>
Thank you for your reply, for score, your ideas is OK to me.
And for the COMPILE_TEST needs still discussing below:
> 2014-07-18 18:51 GMT+08:00 Richard Weinberger <richard@nod.at>:
>> Am 18.07.2014 12:44, schrieb Chen Gang:
>>> On 07/18/2014 03:35 PM, Richard Weinberger wrote:
>>>> Am 18.07.2014 02:36, schrieb Chen Gang:
>>>>>
>>>>> On 07/18/2014 02:09 AM, Richard Weinberger wrote:
>>>>>> Am 17.07.2014 12:48, schrieb Arnd Bergmann:
>>>>>>> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
>>>>>>> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
>>>>>>> to build on UML seems pointless to me and we special-case it in a number of
>>>>>>> places already.
>>>>>>
>>>>>> If UML is the only arch without io memory the dependency on !UML seems
>>>>>> reasonable to me. :-)
>>>>>>
>>>>>
>>>>> For me, if only uml left, I suggest to implement dummy functions within
>>>>> uml instead of let CONFIG_UML appear in generic include directory. And
>>>>> then remove all HAS_IOMEM and NO_IOMEM from kernel.
>>>>
>>>> Erm, this is something completely different.
>>>> I thought we're focusing on COMPILE_TEST?
>>>>
>>>
>>> COMPILE_TEST is none-architecture specific, but UML is. So in generic
>>> include folder, if we're focusing on choosing whether COMPILE_TEST or
>>> UML, for me, I will choose COMPILE_TEST.
>>>
>>> If we're not only focusing on COMPILE_TEST, for me, if something only
>>> depend on one architecture, I'd like to put them under "arch/*/" folder.
>>>
>>> Especially, after that, we can remove all HAS_IOMEM and NO_IOMEM, nobody
>>> has to think of them again. :-)
>>
>> And then we end up with a solution that on UML a lot of completely useless
>> drivers are build which fail in various interesting manners because you'll
>> add stubs for all kinds of io memory related functions to arch/um/?
>> We had this kind of discussion already. You'll need more than ioremap...
>>
>> I like Arnd's idea *much* more to make COMPILE_TEST depend on !UML.
>>
That will let UML itself against COMPILE_TEST (but all the other
architectures not).
And if let COMPILE_TEST depend on !UML, can we still remove all
HAS_IOMEM and NO_IOMEM from kernel? (I guess so).
If we can remove them, we can send related patch firstly -- that will
let current discussion be in UML architecture wide instead of kernel
wide.
Thanks.
--
Chen Gang
Open share and attitude like air water and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-18 18:02 ` Chen Gang
@ 2014-07-20 8:38 ` Chen Gang
2014-07-20 9:45 ` Richard Weinberger
2014-07-20 9:45 ` Chen Gang
0 siblings, 2 replies; 70+ messages in thread
From: Chen Gang @ 2014-07-20 8:38 UTC (permalink / raw)
To: Lennox Wu
Cc: Richard Weinberger, Arnd Bergmann, Lars-Peter Clausen,
Guenter Roeck, Greg Kroah-Hartman, Dmitry Torokhov, linux-iio,
Benjamin Herrenschmidt, Tom Gundersen, Thierry Reding,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
On 07/19/2014 02:02 AM, Chen Gang wrote:
>> 2014-07-18 18:51 GMT+08:00 Richard Weinberger <richard@nod.at>:
>>> Am 18.07.2014 12:44, schrieb Chen Gang:
>>>> On 07/18/2014 03:35 PM, Richard Weinberger wrote:
>>>>> Am 18.07.2014 02:36, schrieb Chen Gang:
>>>>>>
>>>>>> On 07/18/2014 02:09 AM, Richard Weinberger wrote:
>>>>>>> Am 17.07.2014 12:48, schrieb Arnd Bergmann:
>>>>>>>> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
>>>>>>>> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
>>>>>>>> to build on UML seems pointless to me and we special-case it in a number of
>>>>>>>> places already.
>>>>>>>
>>>>>>> If UML is the only arch without io memory the dependency on !UML seems
>>>>>>> reasonable to me. :-)
>>>>>>>
>>>>>>
>>>>>> For me, if only uml left, I suggest to implement dummy functions within
>>>>>> uml instead of let CONFIG_UML appear in generic include directory. And
>>>>>> then remove all HAS_IOMEM and NO_IOMEM from kernel.
>>>>>
>>>>> Erm, this is something completely different.
>>>>> I thought we're focusing on COMPILE_TEST?
>>>>>
>>>>
>>>> COMPILE_TEST is none-architecture specific, but UML is. So in generic
>>>> include folder, if we're focusing on choosing whether COMPILE_TEST or
>>>> UML, for me, I will choose COMPILE_TEST.
>>>>
>>>> If we're not only focusing on COMPILE_TEST, for me, if something only
>>>> depend on one architecture, I'd like to put them under "arch/*/" folder.
>>>>
>>>> Especially, after that, we can remove all HAS_IOMEM and NO_IOMEM, nobody
>>>> has to think of them again. :-)
>>>
>>> And then we end up with a solution that on UML a lot of completely useless
>>> drivers are build which fail in various interesting manners because you'll
>>> add stubs for all kinds of io memory related functions to arch/um/?
>>> We had this kind of discussion already. You'll need more than ioremap...
>>>
>>> I like Arnd's idea *much* more to make COMPILE_TEST depend on !UML.
>>>
>
> That will let UML itself against COMPILE_TEST (but all the other
> architectures not).
>
> And if let COMPILE_TEST depend on !UML, can we still remove all
> HAS_IOMEM and NO_IOMEM from kernel? (I guess so).
>
> If we can remove them, we can send related patch firstly -- that will
> let current discussion be in UML architecture wide instead of kernel
> wide.
>
Next, I shall:
- Remove HAS_IOMEM and NO_IOMEM from kernel, firstly.
- Try to make dummy IOMEM functions for score architecture.
- Continue discussing with UML for it.
By the way: how about HAS_DMA? can we treat it as HAS_IOMEM (remove
it from kernel)? (for me, I guess we can not).
At present, I shall finish sending patch for removing IOMEM today, and
continue to welcome any ideas, suggestions or completions for IOMEM or
DMA.
Thanks.
--
Chen Gang
Open share and attitude like air water and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-20 8:38 ` Chen Gang
@ 2014-07-20 9:45 ` Richard Weinberger
2014-07-20 9:51 ` Chen Gang
2014-07-20 9:45 ` Chen Gang
1 sibling, 1 reply; 70+ messages in thread
From: Richard Weinberger @ 2014-07-20 9:45 UTC (permalink / raw)
To: Chen Gang, Lennox Wu
Cc: Arnd Bergmann, Lars-Peter Clausen, Guenter Roeck,
Greg Kroah-Hartman, Dmitry Torokhov, linux-iio,
Benjamin Herrenschmidt, Tom Gundersen, Thierry Reding,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven
Am 20.07.2014 10:38, schrieb Chen Gang:
> On 07/19/2014 02:02 AM, Chen Gang wrote:
>>> 2014-07-18 18:51 GMT+08:00 Richard Weinberger <richard@nod.at>:
>>>> Am 18.07.2014 12:44, schrieb Chen Gang:
>>>>> On 07/18/2014 03:35 PM, Richard Weinberger wrote:
>>>>>> Am 18.07.2014 02:36, schrieb Chen Gang:
>>>>>>>
>>>>>>> On 07/18/2014 02:09 AM, Richard Weinberger wrote:
>>>>>>>> Am 17.07.2014 12:48, schrieb Arnd Bergmann:
>>>>>>>>> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
>>>>>>>>> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
>>>>>>>>> to build on UML seems pointless to me and we special-case it in a number of
>>>>>>>>> places already.
>>>>>>>>
>>>>>>>> If UML is the only arch without io memory the dependency on !UML seems
>>>>>>>> reasonable to me. :-)
>>>>>>>>
>>>>>>>
>>>>>>> For me, if only uml left, I suggest to implement dummy functions within
>>>>>>> uml instead of let CONFIG_UML appear in generic include directory. And
>>>>>>> then remove all HAS_IOMEM and NO_IOMEM from kernel.
>>>>>>
>>>>>> Erm, this is something completely different.
>>>>>> I thought we're focusing on COMPILE_TEST?
>>>>>>
>>>>>
>>>>> COMPILE_TEST is none-architecture specific, but UML is. So in generic
>>>>> include folder, if we're focusing on choosing whether COMPILE_TEST or
>>>>> UML, for me, I will choose COMPILE_TEST.
>>>>>
>>>>> If we're not only focusing on COMPILE_TEST, for me, if something only
>>>>> depend on one architecture, I'd like to put them under "arch/*/" folder.
>>>>>
>>>>> Especially, after that, we can remove all HAS_IOMEM and NO_IOMEM, nobody
>>>>> has to think of them again. :-)
>>>>
>>>> And then we end up with a solution that on UML a lot of completely useless
>>>> drivers are build which fail in various interesting manners because you'll
>>>> add stubs for all kinds of io memory related functions to arch/um/?
>>>> We had this kind of discussion already. You'll need more than ioremap...
>>>>
>>>> I like Arnd's idea *much* more to make COMPILE_TEST depend on !UML.
>>>>
>>
>> That will let UML itself against COMPILE_TEST (but all the other
>> architectures not).
>>
>> And if let COMPILE_TEST depend on !UML, can we still remove all
>> HAS_IOMEM and NO_IOMEM from kernel? (I guess so).
>>
>> If we can remove them, we can send related patch firstly -- that will
>> let current discussion be in UML architecture wide instead of kernel
>> wide.
>>
>
> Next, I shall:
>
> - Remove HAS_IOMEM and NO_IOMEM from kernel, firstly.
This needs to be last, otherwise lot's of stuff will break.
> - Try to make dummy IOMEM functions for score architecture.
>
> - Continue discussing with UML for it.
If you find sane dummy functions for both UML and score I'm fine with it.
Thanks,
//richard
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-20 9:45 ` Richard Weinberger
@ 2014-07-20 9:51 ` Chen Gang
2014-07-20 9:56 ` Chen Gang
0 siblings, 1 reply; 70+ messages in thread
From: Chen Gang @ 2014-07-20 9:51 UTC (permalink / raw)
To: Richard Weinberger
Cc: Lennox Wu, Arnd Bergmann, Lars-Peter Clausen, Guenter Roeck,
Greg Kroah-Hartman, Dmitry Torokhov, linux-iio,
Benjamin Herrenschmidt, Tom Gundersen, Thierry Reding,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven, cmetcalf, Martin Schwidefsky, heiko.carstens
On 07/20/2014 05:45 PM, Richard Weinberger wrote:
>
>
> Am 20.07.2014 10:38, schrieb Chen Gang:
>> On 07/19/2014 02:02 AM, Chen Gang wrote:
>>>> 2014-07-18 18:51 GMT+08:00 Richard Weinberger <richard@nod.at>:
>>>>> Am 18.07.2014 12:44, schrieb Chen Gang:
>>>>>> On 07/18/2014 03:35 PM, Richard Weinberger wrote:
>>>>>>> Am 18.07.2014 02:36, schrieb Chen Gang:
>>>>>>>>
>>>>>>>> On 07/18/2014 02:09 AM, Richard Weinberger wrote:
>>>>>>>>> Am 17.07.2014 12:48, schrieb Arnd Bergmann:
>>>>>>>>>> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
>>>>>>>>>> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
>>>>>>>>>> to build on UML seems pointless to me and we special-case it in a number of
>>>>>>>>>> places already.
>>>>>>>>>
>>>>>>>>> If UML is the only arch without io memory the dependency on !UML seems
>>>>>>>>> reasonable to me. :-)
>>>>>>>>>
>>>>>>>>
>>>>>>>> For me, if only uml left, I suggest to implement dummy functions within
>>>>>>>> uml instead of let CONFIG_UML appear in generic include directory. And
>>>>>>>> then remove all HAS_IOMEM and NO_IOMEM from kernel.
>>>>>>>
>>>>>>> Erm, this is something completely different.
>>>>>>> I thought we're focusing on COMPILE_TEST?
>>>>>>>
>>>>>>
>>>>>> COMPILE_TEST is none-architecture specific, but UML is. So in generic
>>>>>> include folder, if we're focusing on choosing whether COMPILE_TEST or
>>>>>> UML, for me, I will choose COMPILE_TEST.
>>>>>>
>>>>>> If we're not only focusing on COMPILE_TEST, for me, if something only
>>>>>> depend on one architecture, I'd like to put them under "arch/*/" folder.
>>>>>>
>>>>>> Especially, after that, we can remove all HAS_IOMEM and NO_IOMEM, nobody
>>>>>> has to think of them again. :-)
>>>>>
>>>>> And then we end up with a solution that on UML a lot of completely useless
>>>>> drivers are build which fail in various interesting manners because you'll
>>>>> add stubs for all kinds of io memory related functions to arch/um/?
>>>>> We had this kind of discussion already. You'll need more than ioremap...
>>>>>
>>>>> I like Arnd's idea *much* more to make COMPILE_TEST depend on !UML.
>>>>>
>>>
>>> That will let UML itself against COMPILE_TEST (but all the other
>>> architectures not).
>>>
>>> And if let COMPILE_TEST depend on !UML, can we still remove all
>>> HAS_IOMEM and NO_IOMEM from kernel? (I guess so).
>>>
>>> If we can remove them, we can send related patch firstly -- that will
>>> let current discussion be in UML architecture wide instead of kernel
>>> wide.
>>>
>>
>> Next, I shall:
>>
>> - Remove HAS_IOMEM and NO_IOMEM from kernel, firstly.
>
> This needs to be last, otherwise lot's of stuff will break.
>
OK, thanks, that sounds reasonable to me.
>> - Try to make dummy IOMEM functions for score architecture.
>>
>> - Continue discussing with UML for it.
>
> If you find sane dummy functions for both UML and score I'm fine with it.
>
For me, what you said is necessary, tile and s390 also need dummy IOMEM
when !PCI.
So for me, I shall implement them in "include/asm-generic", and let uml,
score, tile and s390 use dummy IOMEM when they need.
Welcome any other members ideas, suggestions and completions.
Thanks.
--
Chen Gang
Open share and attitude like air water and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-20 9:51 ` Chen Gang
@ 2014-07-20 9:56 ` Chen Gang
0 siblings, 0 replies; 70+ messages in thread
From: Chen Gang @ 2014-07-20 9:56 UTC (permalink / raw)
To: Richard Weinberger
Cc: Lennox Wu, Arnd Bergmann, Lars-Peter Clausen, Guenter Roeck,
Greg Kroah-Hartman, Dmitry Torokhov, linux-iio,
Benjamin Herrenschmidt, Tom Gundersen, Thierry Reding,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Mischa.Jonker, jic23,
Geert Uytterhoeven, cmetcalf, heiko.carstens
On 07/20/2014 05:51 PM, Chen Gang wrote:
> On 07/20/2014 05:45 PM, Richard Weinberger wrote:
>>
>>
>> Am 20.07.2014 10:38, schrieb Chen Gang:
>>> On 07/19/2014 02:02 AM, Chen Gang wrote:
>>>>> 2014-07-18 18:51 GMT+08:00 Richard Weinberger <richard@nod.at>:
>>>>>> Am 18.07.2014 12:44, schrieb Chen Gang:
>>>>>>> On 07/18/2014 03:35 PM, Richard Weinberger wrote:
>>>>>>>> Am 18.07.2014 02:36, schrieb Chen Gang:
>>>>>>>>>
>>>>>>>>> On 07/18/2014 02:09 AM, Richard Weinberger wrote:
>>>>>>>>>> Am 17.07.2014 12:48, schrieb Arnd Bergmann:
>>>>>>>>>>> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
>>>>>>>>>>> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
>>>>>>>>>>> to build on UML seems pointless to me and we special-case it in a number of
>>>>>>>>>>> places already.
>>>>>>>>>>
>>>>>>>>>> If UML is the only arch without io memory the dependency on !UML seems
>>>>>>>>>> reasonable to me. :-)
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> For me, if only uml left, I suggest to implement dummy functions within
>>>>>>>>> uml instead of let CONFIG_UML appear in generic include directory. And
>>>>>>>>> then remove all HAS_IOMEM and NO_IOMEM from kernel.
>>>>>>>>
>>>>>>>> Erm, this is something completely different.
>>>>>>>> I thought we're focusing on COMPILE_TEST?
>>>>>>>>
>>>>>>>
>>>>>>> COMPILE_TEST is none-architecture specific, but UML is. So in generic
>>>>>>> include folder, if we're focusing on choosing whether COMPILE_TEST or
>>>>>>> UML, for me, I will choose COMPILE_TEST.
>>>>>>>
>>>>>>> If we're not only focusing on COMPILE_TEST, for me, if something only
>>>>>>> depend on one architecture, I'd like to put them under "arch/*/" folder.
>>>>>>>
>>>>>>> Especially, after that, we can remove all HAS_IOMEM and NO_IOMEM, nobody
>>>>>>> has to think of them again. :-)
>>>>>>
>>>>>> And then we end up with a solution that on UML a lot of completely useless
>>>>>> drivers are build which fail in various interesting manners because you'll
>>>>>> add stubs for all kinds of io memory related functions to arch/um/?
>>>>>> We had this kind of discussion already. You'll need more than ioremap...
>>>>>>
>>>>>> I like Arnd's idea *much* more to make COMPILE_TEST depend on !UML.
>>>>>>
>>>>
>>>> That will let UML itself against COMPILE_TEST (but all the other
>>>> architectures not).
>>>>
>>>> And if let COMPILE_TEST depend on !UML, can we still remove all
>>>> HAS_IOMEM and NO_IOMEM from kernel? (I guess so).
>>>>
>>>> If we can remove them, we can send related patch firstly -- that will
>>>> let current discussion be in UML architecture wide instead of kernel
>>>> wide.
>>>>
>>>
>>> Next, I shall:
>>>
>>> - Remove HAS_IOMEM and NO_IOMEM from kernel, firstly.
>>
>> This needs to be last, otherwise lot's of stuff will break.
>>
>
> OK, thanks, that sounds reasonable to me.
>
>>> - Try to make dummy IOMEM functions for score architecture.
>>>
>>> - Continue discussing with UML for it.
>>
>> If you find sane dummy functions for both UML and score I'm fine with it.
>>
>
> For me, what you said is necessary, tile and s390 also need dummy IOMEM
> when !PCI.
>
> So for me, I shall implement them in "include/asm-generic", and let uml,
> score, tile and s390 use dummy IOMEM when they need.
>
> Welcome any other members ideas, suggestions and completions.
>
>
And sorry, I can not finish this discussion and send patch for it within
this week, for it is really a long necessary discussion.
But, hope we can finish this discussion and send patch for it within
this month (but in current condition, I am not quite sure).
And after finish discussion, welcome any other members help sending
related patch for it (e.g. implement dummy IOMEM in "asm-generic",
remove all IOMEM in kernel wide).
Thanks.
--
Chen Gang
Open share and attitude like air water and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-20 8:38 ` Chen Gang
2014-07-20 9:45 ` Richard Weinberger
@ 2014-07-20 9:45 ` Chen Gang
2014-07-22 10:32 ` Arnd Bergmann
1 sibling, 1 reply; 70+ messages in thread
From: Chen Gang @ 2014-07-20 9:45 UTC (permalink / raw)
To: Lennox Wu
Cc: Richard Weinberger, Arnd Bergmann, Lars-Peter Clausen,
Guenter Roeck, Greg Kroah-Hartman, Dmitry Torokhov, linux-iio,
Benjamin Herrenschmidt, Tom Gundersen, Thierry Reding,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, Martin Schwidefsky, jic23,
Geert Uytterhoeven, cmetcalf, heiko.carstens
On 07/20/2014 04:38 PM, Chen Gang wrote:
> On 07/19/2014 02:02 AM, Chen Gang wrote:
>>> 2014-07-18 18:51 GMT+08:00 Richard Weinberger <richard@nod.at>:
>>>> Am 18.07.2014 12:44, schrieb Chen Gang:
>>>>> On 07/18/2014 03:35 PM, Richard Weinberger wrote:
>>>>>> Am 18.07.2014 02:36, schrieb Chen Gang:
>>>>>>>
>>>>>>> On 07/18/2014 02:09 AM, Richard Weinberger wrote:
>>>>>>>> Am 17.07.2014 12:48, schrieb Arnd Bergmann:
>>>>>>>>> AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
>>>>>>>>> a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
>>>>>>>>> to build on UML seems pointless to me and we special-case it in a number of
>>>>>>>>> places already.
>>>>>>>>
>>>>>>>> If UML is the only arch without io memory the dependency on !UML seems
>>>>>>>> reasonable to me. :-)
>>>>>>>>
>>>>>>>
>>>>>>> For me, if only uml left, I suggest to implement dummy functions within
>>>>>>> uml instead of let CONFIG_UML appear in generic include directory. And
>>>>>>> then remove all HAS_IOMEM and NO_IOMEM from kernel.
>>>>>>
>>>>>> Erm, this is something completely different.
>>>>>> I thought we're focusing on COMPILE_TEST?
>>>>>>
>>>>>
>>>>> COMPILE_TEST is none-architecture specific, but UML is. So in generic
>>>>> include folder, if we're focusing on choosing whether COMPILE_TEST or
>>>>> UML, for me, I will choose COMPILE_TEST.
>>>>>
>>>>> If we're not only focusing on COMPILE_TEST, for me, if something only
>>>>> depend on one architecture, I'd like to put them under "arch/*/" folder.
>>>>>
>>>>> Especially, after that, we can remove all HAS_IOMEM and NO_IOMEM, nobody
>>>>> has to think of them again. :-)
>>>>
>>>> And then we end up with a solution that on UML a lot of completely useless
>>>> drivers are build which fail in various interesting manners because you'll
>>>> add stubs for all kinds of io memory related functions to arch/um/?
>>>> We had this kind of discussion already. You'll need more than ioremap...
>>>>
>>>> I like Arnd's idea *much* more to make COMPILE_TEST depend on !UML.
>>>>
>>
>> That will let UML itself against COMPILE_TEST (but all the other
>> architectures not).
>>
>> And if let COMPILE_TEST depend on !UML, can we still remove all
>> HAS_IOMEM and NO_IOMEM from kernel? (I guess so).
>>
>> If we can remove them, we can send related patch firstly -- that will
>> let current discussion be in UML architecture wide instead of kernel
>> wide.
>>
>
> Next, I shall:
>
> - Remove HAS_IOMEM and NO_IOMEM from kernel, firstly.
>
> - Try to make dummy IOMEM functions for score architecture.
>
> - Continue discussing with UML for it.
>
Oh, sorry, I forgot, after remove IOMEM from kernel, also s390 and tile
need implement dummy IOMEM if !PCI.
If possible, I shall try to implement the dummy IOMEM in 'asm-generic',
and let uml, score, s390 and tile use them when they need.
>
> By the way: how about HAS_DMA? can we treat it as HAS_IOMEM (remove
> it from kernel)? (for me, I guess we can not).
>
>
> At present, I shall finish sending patch for removing IOMEM today, and
> continue to welcome any ideas, suggestions or completions for IOMEM or
> DMA.
>
>
Thanks.
--
Chen Gang
Open share and attitude like air water and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-20 9:45 ` Chen Gang
@ 2014-07-22 10:32 ` Arnd Bergmann
2014-07-22 11:29 ` Chen Gang
0 siblings, 1 reply; 70+ messages in thread
From: Arnd Bergmann @ 2014-07-22 10:32 UTC (permalink / raw)
To: Chen Gang
Cc: Lennox Wu, Richard Weinberger, Lars-Peter Clausen, Guenter Roeck,
Greg Kroah-Hartman, Dmitry Torokhov, linux-iio,
Benjamin Herrenschmidt, Tom Gundersen, Thierry Reding,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, jic23, Geert Uytterhoeven, cmetcalf,
heiko.carstens
On Sunday 20 July 2014 17:45:40 Chen Gang wrote:
> >
> > Next, I shall:
> >
> > - Remove HAS_IOMEM and NO_IOMEM from kernel, firstly.
> >
> > - Try to make dummy IOMEM functions for score architecture.
> >
> > - Continue discussing with UML for it.
> >
>
> Oh, sorry, I forgot, after remove IOMEM from kernel, also s390 and tile
> need implement dummy IOMEM if !PCI.
>
> If possible, I shall try to implement the dummy IOMEM in 'asm-generic',
> and let uml, score, s390 and tile use them when they need.
Sorry for going round in circles, but looking back at the original patches,
adding the extra 'depends on HAS_IOMEM' does seem much better than the
other suggestions that came afterwards.
In particular, removing HAS_IOMEM and NO_IOMEM sounds like an awful idea
to me. I'd rather add a HAS_IOPORT in addition to also catch architectures
that have no support for PC-style PIO.
Arnd
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'
2014-07-22 10:32 ` Arnd Bergmann
@ 2014-07-22 11:29 ` Chen Gang
0 siblings, 0 replies; 70+ messages in thread
From: Chen Gang @ 2014-07-22 11:29 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Lennox Wu, Richard Weinberger, Lars-Peter Clausen, Guenter Roeck,
Greg Kroah-Hartman, Dmitry Torokhov, linux-iio,
Benjamin Herrenschmidt, Tom Gundersen, Thierry Reding,
Marek Vasut, Liqin Chen, msalter, linux-pwm, devel,
linux-watchdog, linux-input, linux-kernel@vger.kernel.org,
knaack.h, Martin Schwidefsky, jic23, Geert Uytterhoeven, cmetcalf,
heiko.carstens
On 07/22/2014 06:32 PM, Arnd Bergmann wrote:
> On Sunday 20 July 2014 17:45:40 Chen Gang wrote:
>>>
>>> Next, I shall:
>>>
>>> - Remove HAS_IOMEM and NO_IOMEM from kernel, firstly.
>>>
>>> - Try to make dummy IOMEM functions for score architecture.
>>>
>>> - Continue discussing with UML for it.
>>>
>>
>> Oh, sorry, I forgot, after remove IOMEM from kernel, also s390 and tile
>> need implement dummy IOMEM if !PCI.
>>
>> If possible, I shall try to implement the dummy IOMEM in 'asm-generic',
>> and let uml, score, s390 and tile use them when they need.
>
> Sorry for going round in circles, but looking back at the original patches,
> adding the extra 'depends on HAS_IOMEM' does seem much better than the
> other suggestions that came afterwards.
>
> In particular, removing HAS_IOMEM and NO_IOMEM sounds like an awful idea
> to me. I'd rather add a HAS_IOPORT in addition to also catch architectures
> that have no support for PC-style PIO.
>
Welcome any other members (especially driver members) ideas and
suggestions -- driver members and architecture members have different
tastes and different roles.
For me, if no additional reply, I prefer to keep current status, and
still add 'depends on HAS_IOMEM' for each driver which need it, but I am
not sure whether driver members can bear it.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 70+ messages in thread