qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 00/20] Add Faraday A36x SoC platform support


From: Kuo-Jung Su
Subject: Re: [Qemu-devel] [PATCH v2 00/20] Add Faraday A36x SoC platform support
Date: Mon, 28 Jan 2013 16:33:55 +0800




2013/1/28 Andreas Färber <address@hidden>
Am 25.01.2013 09:19, schrieb Kuo-Jung Su:
> From: Kuo-Jung Su <address@hidden>
>
> These patches introduce Faraday A36x SoC platform support.
>
> Faraday provides ARMv4/ARMv5TE compatible solutions,
> and focus on ASIC design service.
>
> Here are some public documents for you reference.
>
> http://www.faraday-tech.com/html/documentation/index.html
>
> The pre-built images are also available at my Google Drive:
>
> https://docs.google.com/folder/d/0BwfiewvSmUgAX2pTTmtUMGFCZW8/edit
>
> Here is the image file list:
>
> 1. android-4.0.4/zImage: A369 linux-3.0.31
> 2. android-4.0.4/romfs-4.0.4.tar.bz2: android-4.0.4
> 3. nand.img.bz2: A369 nand flash image
> 4. rom.img.bz2: A369 embedded ROM image
> 5. u-boot: A369 u-boot-2012.10 ELF file
> 6. zImage: A369 linux-3.4.12 + initramfs
> 7. README

Thank you for posting these test images. Could you also publish your
patches on some public git branch so that one doesn't need to manually
apply 20 or more patches to test it? (e.g., GitHub, repo.or.cz, ...)

Another thing you could look into is qemu-jeos.git support - could you
supply configs for building an upstream Linux kernel for testing? You
can look at arm-vexpress.cfg as template.


Sorry, there are lot of patches, and most of them have coding styles.
Becuase the entire open source release (I mean 'mine' open source release) was
initialated 2 weeks ago.
And yes, you might have noticed. The QEMU is my very 1st open source release....
 
And there are somehing make things even worst ....
For example:

In u-boot, there are already some bad code / or heavily un-tested code
have release to its git tree.
i.e.
1. The ftsdc010_esdhc contributed by Andes Technology (a subsidiary of Faraday)
    simply doesn't work at Faraday A369 in u-boot-2013.01.
    One of my colleague has ever make it work in u-boot-2010.09,
    but its performanceis only 1/10 of mine ...
   
2. The ftgmac100 is somehow contributed by someone should do that with a
    untested version, although it seemds to work correctly with MMU disabled.
    
All of these, means after applying my patch, you would have multi-pile drivers
against the same hardware....

If none of these were problem to you, I'll tried to make them open source later.

And for a record, in my original open source schedule, the following items
would be release in turn.

1) qemu (a36x)
2) u-boot-2013.01
3) linux-3.4.12
4) falinux (It's a combination of uClinux and build-root, was my personal
    linux BSP, and now seems to be an official one in Faraday)
5) android-4.0.4
6) linux-3.0.31 for android
7) android-dist (A clone of falinux, which is designed to genrated andoird images)
8) contiki-2.6
9) contiki-2.6-dist (A clone of falinux, which is designed to genrated contiki images)
10) ecos-3.0
11) ecos-dist (A clone of falinux, which is designed to genrated contiki images)
12) virgil (A referenced ROM code for my projects, which is also a RTOS/FreeRTOS)

Because Faraday is actually a ASIC desgin service company, it usualy cares nothing
about software and intends to make them all open sourced to catch more customers.

> Changes for v2:
>
>    1. coding style fixes (verified with checkpatch.pl)
>    2. add Faraday A360 support
>    3. add Faraday USB 2.0 EHCI support
>    4. merge a369_scu.c into a369.c
>    5. introduce QOM coding style
>    6. remove lowercase Macros: min(), max()
>    7. name all struct as CamelCase style
>    8. move function prototypes from .c to faraday.h
>    9. use switch instead of if statement in a369_ahbc_write
>   10. remove debug prints
>   11. update all uarts in A36x to DEVICE_LITTLE_ENDIAN.
>   12. move the variable definitions to the start of the function,
>       instead of inside a do { } while(0)
>   13. remove disabled and commented out code.
>   14. use hw_error() and exit() upon pflash register failed.
>   15. add const prior to TypeInfo (i.e. static *const*
>       TypeInfo ftrtc011_info)
>   16. add le32_to_cpu/cpu_to_le32 to the descriptor processing in
>       FTGMAC100/FTMAC110.
>   17. update the GPL license to GPL v2 (except for FTLCDC200,
>       it's based on pl110.c which is LGPL.)
>   18. add const to src_* in the DMA controllers (FTAPBBRG020/FTDMAC020)
>
>   Note:
>   1. Thanks to the comments from KONRAD, Andreas, Paul and Blue.
>      But if there is still something missed, please kindly remind me to fix.
>      Thanks!

Your detailed change log is very helpful, thank you. When you post a v3
however, please post the patch series as a top-level patchset (i.e., the
cover letter not as a reply, just the actual patches to the cover
letter), the change log then should be sufficient to understand its
historical context and that makes it a little easier for humans or
Anthony's scripts to find the latest patch versions.


Sorry, I can't get you very well...
I guess I'm not supposed to use --in-reply-to=<identifier> with git send-email
and change log should not be removed.
Any way I'll try to find some examples from the email and google for some more to see how to do that....
 
For all your new SysBus devices (i.e., types with .parent =
SYS_BUS_DEVICE) please use a QOM realizefn instead of a qdev initfn -
not just the function naming but dc->realize rather than sdc->init. This
avoids one of us having to go through your code later and doing the
conversion. (I showed you how to do that for EHCI as an example.)

What I'm still missing in v2 is qtest test cases for the RTC and a
libqos driver for your I2C master. If you have questions how to go about
that, feel free to ask on IRC.

Sorry, IRC is banned from my company, and I usually don't work at home....
My wife would get crazzy on me.

P.S:
My real work is to help customers to build new their own new SoC platform
and it's extremely easy so I usually have lots of private time at work.


>   2. There are still two coding style errors reported by checkpatch.pl,
>      but I think they might be false alarms, here is the error log:
>
>   ERROR: need consistent spacing around '*' (ctx:WxV)
>   #74: FILE: hw/a360.c:32:
>   +    i2c_bus      *i2c[2];                ^
>
>   ERROR: open brace '{' following function declarations go on the next line
>   #4680: FILE: hw/ftlcdc200_template.h:59:
>   +static drawfn glue(ftlcdc200_draw_fn_, BITS)[48] = {

Yes, your analysis looks correct, you can ignore both "errors".

got you, thanks
 

Regards,
Andreas

--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



--
Best wishes,
Kuo-Jung Su

reply via email to

[Prev in Thread] Current Thread [Next in Thread]