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

Attachments:

(No filename) (2.52 kB)
(No filename) (819.00 B)
Download all attachments

2014-07-17 09:57:19

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

2014-07-31 20:09:53

by Chris Metcalf

[permalink] [raw]

Subject: Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'

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