Re: [PATCH] Add minimal Hexagon target - First in a series of patches - linux-user changes + linux-user/hexagon + skeleton of target/hexagon - Files in target/hexagon/imported are from another project and therefore do not conform to qemu coding standards

[Laurent Vivier]

Le 19/11/2019 à 18:22, Taylor Simpson a écrit :
> Thanks for all the feedback on the patch.  I'll summarize my TODO list here.  
> Please let me know if there's anything I missed.
> - Add a README file in the imported directory to make it clear that the code 
> comes from another project.  Personally, I prefer keeping the name as 
> "imported".  It was suggested by Richard at the meeting.  Also as a heads-up, 
> that is a small subset of the files that will be in that directory 
> eventually.  Right now, it is the minimum needed to build the skeleton target.
> - Work on the .checkpatchignore as Philippe suggested.
> - Split out the "[__SIGRTMAX - 1] = __SIGRTMIN + 1" into a separate patch.
> - Clean up the long subject line.
> - Add license text to the new files.
> - Remove the DEBUG_HEX blocks.  In general the DEBUG_HEX macro controls a 
> bunch of debugging output as you'll see in later patches.  In the long run, I 
> think it should be replaces with a macro that is defined when configured with 
> --enable-debug and then an additional command-line argument.  I haven't 
> looked into this, so any pointers would be appreciated.

You can have a look to the trace infrastructure
(docs/devel/tracing.txt). We have also some qemu_log() macros for low
level debugging.

> - Laurent suggested I split the patch into two parts: linux-user and 
> target/hexagon.  If I do that, which one should contain the changes to common 
> files (e.g., configure)?  Also, note that we won't be able to build until 
> both patches are merged.  Is that OK?

You should add target/hexagon first, and it should not be build as we
don't have any target (hexagon-linux-user or hexagon-softmmu),
then you can add linux-user part that will be built and use the
target/hexagone CPU. I think the configure part should go to the
linux-user part as it enables the build.

I asked to split the patch for review purpose, but this should not break
anything (to allow bisect).

Thanks,
Laurent

> 
> Thanks,
> Taylor
> 
> 
> -----Original Message-----
> From: Philippe Mathieu-Daudé <address@hidden>
> Sent: Tuesday, November 19, 2019 9:19 AM
> To: Taylor Simpson <address@hidden>; address@hidden; address@hidden; 
> address@hidden
> Subject: Re: [PATCH] Add minimal Hexagon target - First in a series of 
> patches - linux-user changes + linux-user/hexagon + skeleton of 
> target/hexagon - Files in target/hexagon/imported are from another project 
> and therefore do not conform to qemu coding standards
> 
> -------------------------------------------------------------------------
> CAUTION: This email originated from outside of the organization.
> -------------------------------------------------------------------------
> 
> On 11/19/19 12:58 AM, Taylor Simpson wrote:
>> Signed-off-by: Taylor Simpson <address@hidden>
>> ---
> [...]
>>   target/hexagon/imported/global_types.h      |  25 +++
>>   target/hexagon/imported/iss_ver_registers.h | 183 +++++++++++++++
>>   target/hexagon/imported/max.h               |  78 +++++++
>>   target/hexagon/imported/regs.h              |  19 ++
> 
> Maybe you can rename this directory as:
> 
> target/hexagon/dsp-sdk/
> 
> and add a README "Files under this directory are imported from the SDK 
> available once registered on developer.qualcomm.com ..."
> 
>