Re: [Xen-devel] [PATCH v01 2/3] xen/arm: add platform specific definitions for DRA7 evm board

[Andrii Tseglytskyi <andrii.tseglytskyi@xxxxxxxxxxxxxxx>]

Hi Ian,

On Fri, Jun 27, 2014 at 3:17 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Thu, 2014-06-26 at 14:15 +0300, Andrii Tseglytskyi wrote:
>
> This new file is about 90% the same as xen/arch/arm/platforms/omap5.c.
> Are they really different SoCs or just variants?
>

They are really different SoCs. A lot of changes were dropped from
DRA7 - voltagedomains, powerdomains, clocks are really different.
In TI kernels OMAP5 and DRA7 definitions live separately in different
files. In kernel 3.4 - separate HW specific defs in *.c files, in
kernel 3.8 and later - separate device tree blobs.


> If they are variants then I think they should be in the same file and
> this new one should be added to the compatible list there, and perhaps
> some is_compatible put where the variations differ.
>
> If they are completely different SoCs which just happen to have some
> omap related similarities then I think platforms/omap-common.[ch] should
> be created to hold the stuff which can be reused.

This sounds reasonable. Thank you.
>
>
>> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@xxxxxxxxxxxxxxx>
>> ---
>>  xen/arch/arm/platforms/Makefile        |    1 +
>>  xen/arch/arm/platforms/dra7xx.c        |  158 
>> +++++++++++++++++++++++++++++++
>>  xen/include/asm-arm/platforms/dra7xx.h |  163 
>> ++++++++++++++++++++++++++++++++
>
> Please put whatever definitions you need into the .c file, this will
> stop platform details getting leaked into the remainder of Xen.
>
> (the existing platform headers were a historical mistake IMHO)
>
> If you go the omap-common route then put the header in
> xen/arch/arm/platforms/ and #include "omap-common.h" rather than <>.
>

OK.

>> +    /* OMAP Linux kernel handles devices with status "disabled" in a
>> +     * weird manner - tries to reset them. While their memory ranges
>> +     * are not mapped, this leads to data aborts, so skip these devices
>> +     * from DT for dom0.
>> +     */
>> +    DT_MATCH_NOT_AVAILABLE(),
>
> I think this should be done in common code, either by default (if that
> makes sense) or using a new quirk flag.
>

OK.
> Ian.
>


Regards

-- 

Andrii Tseglytskyi | Embedded Dev
GlobalLogic
www.globallogic.com

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel