Hi Markos,
2014-08-12 0:23 GMT-07:00 Markos Chandras <markos.chandras@xxxxxxxxxx>:
> Hi Florian,
>
> On Mon, Aug 04, 2014 at 03:54:49PM -0700, Florian Fainelli wrote:
>> Hi Markos,
>>
>> On 07/18/2014 02:51 AM, Markos Chandras wrote:
>> > Different cores use different CCA values to achieve write-combine
>> > memory writes. For cores that do not support write-combine we
>> > set the default value to CCA:2 (uncached, non-coherent) which is the
>> > default value as set by the kernel.
>> >
>> > Signed-off-by: Markos Chandras <markos.chandras@xxxxxxxxxx>
>> > ---
>> [snip]
>> break;
>> > @@ -765,67 +767,83 @@ static inline void cpu_probe_legacy(struct cpuinfo_mips *c, unsigned int cpu)
>> >
>> > static inline void cpu_probe_mips(struct cpuinfo_mips *c, unsigned int cpu)
>> > {
>> > + c->writecombine = _CACHE_UNCACHED_ACCELERATED;
>>
>> Why do we set this writecombine setting by default, when later we are
>> going to override writecombine on a per-cpu basic.
>>
>> In the end, we have the following:
>>
>> cpu_probe()
>> c->writecombine = _CACHE_UNCACHED;
>>
>> cpu_probe_mips()
>> c->writecombine = _CACHE_UNCACHED_ACCELERATED:
>> ... per-cpu case ...
>> c->writecombine = _CACHE_UNCACHED;
>>
>> Can't we just eliminate the various assignments in cpu_probe_mips() and
>> only override c->writecombine if _CACHE_UNCACHED is not suitable?
>>
> The reason I did it like this, is that new cores (eg *Aptiv family) will use
> _CACHE_UNCACHED_ACCELERATED and that's why it's the 'default' option for
> the MIPS cores. _CACHE_UNCACHED is only suitable for old cores.
> The way it is right now, allows us to not have to set this option whenever we
> add support for a new core since it will inherit the default option.
Ok, that makes sense, although we currently have more _CACHE_UNCACHED
platforms supported than _CACHE_UNCACHED_ACCELERATED, so maybe once
you reach a critical number of Aptiv cores or with similar caching
settings, we can reverse the tendency by then? This is not a strong
objection, I was just looking at a way to minimize the changes.
--
Florian
