Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1344077 > unrolled thread

Re: [PATCH] mtd: bcm47xxsflash: use ioremap_cachable() instead of KSEG0ADDR()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] mtd: bcm47xxsflash: use ioremap_cachable() instead of KSEG0ADDR() Rafał Miłecki <zajec5@gmail.com> - 2016-02-26 11:50 +0100
    Re: [PATCH] mtd: bcm47xxsflash: use ioremap_cachable() instead of  KSEG0ADDR() "Maciej W. Rozycki" <macro@imgtec.com> - 2016-02-26 15:40 +0100
      Re: [PATCH] mtd: bcm47xxsflash: use ioremap_cachable() instead of KSEG0ADDR() Rafał Miłecki <zajec5@gmail.com> - 2016-02-26 17:40 +0100

#1344077 — Re: [PATCH] mtd: bcm47xxsflash: use ioremap_cachable() instead of KSEG0ADDR()

On 24 January 2016 at 21:17, Maciej W. Rozycki <macro@imgtec.com> wrote:
> On Sun, 17 Jan 2016, Rafał Miłecki wrote:
>
>> KSEG0ADDR was translating 0x1c000000 into 0x9c000000. With
>> ioremap_cachable we use MIPS's __ioremap (and remap_area_pages). This
>> results in different address (e.g. 0xc0080000) but it still should be
>> cached as expected and it was successfully tested with BCM47186B0.
>
>  This is due to this piece:
>
>         /*
>          * Map uncached objects in the low 512mb of address space using KSEG1,
>          * otherwise map using page tables.
>          */
>         if (IS_LOW512(phys_addr) && IS_LOW512(last_addr) &&
>             flags == _CACHE_UNCACHED)
>                 return (void __iomem *) CKSEG1ADDR(phys_addr);
>
> special-casing uncached mapping only (replicated in 2 places).  I think
> there will really be no harm from returning a KSEG0 mapping for calls
> requesting a caching mode equal to `_page_cachable_default', which --
> depending on the cache architecture -- will have been either hardwired or
> prearranged via Config.K0.  I think there's really no need to put pressure
> on the TLB, which may be small, in cases where a fixed mapping will do.

No, it isn't hitting condition you pointed. We call ioremap_cachable
which uses _page_cachable_default as a flag. This flag
(_page_cachable_default) isn't equal to the _CACHE_UNCACHED.

Moreover code you pointed uses CKSEG1ADDR which would result in
setting bit KSEG1 (0xa0000000). As I pointed in the commit message
address it ORed with KSEG2 (0xc0000000).

So what really happens is what my commit message says:
ioremap_cachable -> __ioremap -> remap_area_pages

[toc] | [next] | [standalone]

#1344307 — Re: [PATCH] mtd: bcm47xxsflash: use ioremap_cachable() instead of KSEG0ADDR()

On Fri, 26 Feb 2016, Rafał Miłecki wrote:

> >> KSEG0ADDR was translating 0x1c000000 into 0x9c000000. With
> >> ioremap_cachable we use MIPS's __ioremap (and remap_area_pages). This
> >> results in different address (e.g. 0xc0080000) but it still should be
> >> cached as expected and it was successfully tested with BCM47186B0.
> >
> >  This is due to this piece:
> >
> >         /*
> >          * Map uncached objects in the low 512mb of address space using KSEG1,
> >          * otherwise map using page tables.
> >          */
> >         if (IS_LOW512(phys_addr) && IS_LOW512(last_addr) &&
> >             flags == _CACHE_UNCACHED)
> >                 return (void __iomem *) CKSEG1ADDR(phys_addr);
> >
> > special-casing uncached mapping only (replicated in 2 places).  I think
> > there will really be no harm from returning a KSEG0 mapping for calls
> > requesting a caching mode equal to `_page_cachable_default', which --
> > depending on the cache architecture -- will have been either hardwired or
> > prearranged via Config.K0.  I think there's really no need to put pressure
> > on the TLB, which may be small, in cases where a fixed mapping will do.
> 
> No, it isn't hitting condition you pointed. We call ioremap_cachable
> which uses _page_cachable_default as a flag. This flag
> (_page_cachable_default) isn't equal to the _CACHE_UNCACHED.

 That's exactly what I wrote: code I quoted is "special-casing uncached 
mapping only" -- which as you have correctly observed does not apply after 
your change anymore.  Which is why previously you got an address in the 
unmapped KSEG1 segment and now you get an address in the mapped KSEG2 
rather than the unmapped KSEG0 segment.

> Moreover code you pointed uses CKSEG1ADDR which would result in
> setting bit KSEG1 (0xa0000000). As I pointed in the commit message
> address it ORed with KSEG2 (0xc0000000).

 It's not merely ORed, it's actually mapped via the TLB.

 I hope this makes things clear.

  Maciej

[toc] | [prev] | [next] | [standalone]

#1344424

On 26 February 2016 at 15:18, Maciej W. Rozycki <macro@imgtec.com> wrote:
> On Fri, 26 Feb 2016, Rafał Miłecki wrote:
>
>> >> KSEG0ADDR was translating 0x1c000000 into 0x9c000000. With
>> >> ioremap_cachable we use MIPS's __ioremap (and remap_area_pages). This
>> >> results in different address (e.g. 0xc0080000) but it still should be
>> >> cached as expected and it was successfully tested with BCM47186B0.
>> >
>> >  This is due to this piece:
>> >
>> >         /*
>> >          * Map uncached objects in the low 512mb of address space using KSEG1,
>> >          * otherwise map using page tables.
>> >          */
>> >         if (IS_LOW512(phys_addr) && IS_LOW512(last_addr) &&
>> >             flags == _CACHE_UNCACHED)
>> >                 return (void __iomem *) CKSEG1ADDR(phys_addr);
>> >
>> > special-casing uncached mapping only (replicated in 2 places).  I think
>> > there will really be no harm from returning a KSEG0 mapping for calls
>> > requesting a caching mode equal to `_page_cachable_default', which --
>> > depending on the cache architecture -- will have been either hardwired or
>> > prearranged via Config.K0.  I think there's really no need to put pressure
>> > on the TLB, which may be small, in cases where a fixed mapping will do.
>>
>> No, it isn't hitting condition you pointed. We call ioremap_cachable
>> which uses _page_cachable_default as a flag. This flag
>> (_page_cachable_default) isn't equal to the _CACHE_UNCACHED.
>
>  That's exactly what I wrote: code I quoted is "special-casing uncached
> mapping only" -- which as you have correctly observed does not apply after
> your change anymore.  Which is why previously you got an address in the
> unmapped KSEG1 segment and now you get an address in the mapped KSEG2
> rather than the unmapped KSEG0 segment.
>
>> Moreover code you pointed uses CKSEG1ADDR which would result in
>> setting bit KSEG1 (0xa0000000). As I pointed in the commit message
>> address it ORed with KSEG2 (0xc0000000).
>
>  It's not merely ORed, it's actually mapped via the TLB.
>
>  I hope this makes things clear.

Ah, sorry, I missed the point of your explanation. Now it's clear, thanks!

-- 
Rafał

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web