Hi Matt&Michal,

Thank you for your advice. We are updating our codes in order to submit the 
source code successfully.

We are doing the following changes. Next patch will be in the end of this 
month. (by the way, shall I make a new patch based on older source code or 
based on last patch?)
1. > configure.ac                    |   81 +-
>There are a lot of useless changes in configure.ac, like lowering the 
>AC_PREREQ number and changing the bugzilla link. Also, the version number in 
>configure.ac is wrong.

We are updating the file. 

2. > man/Makefile.am                 |   64 +-
>You replaced the Oracle copyright notice with the old Sun copyright notice. 
>Clearly wrong, and it makes it clear that this wasn't well reviewed or rebased.

We've replaced old Sun copyright notice with the Oracle copyright notice. Is it 
OK?

3. > src/CALLMAP                     |   22 +
>This is a useless file that was probably removed from git a long time ago. 
>Don't add it back.

The file has been deleted.

4. > src/Imakefile                   |  116 +
>WTF? No. Just no.
>I see that the driver has RANDR support. No version of the X server ever had 
>an Imake build system and also RANDR support. Remove this Imakefile.

The file has been deleted.

5. 
+*         Copyright (c) 2007 by Silicon Motion, Inc. (SMI)
+*
+*  All rights are reserved. Reproduction or in part is prohibited
+*  without the written consent of the copyright owner.
>That doesn't look good.

We will use the same old format:    Copyright (C) YYYY Silicon Motion, Inc.  
All Rights Reserved. 
Is this OK?


>I guess this is not really a requirement but makes the code needlessly 
>unattractive.

>Since this is an initial submission it is the right time to change this.

>If you look at the other drivers they have rather simple structure with all 
>files in a single directory. Large part of the code is shared. When support 
>for new chipset family is added a few files specific to that chipset are 
>typically required which have the chipset name in their file name but much of 
>the rest is just updated slightly.

>Now you add support for half dozen of chips and I have no idea how they are 
>related to each other and to the chips previously supported by the smi driver. 
>You should know the best since you wrote the code.

>However, the file naming suggests that you started with separate driver for 
>every chip and just added hooks in the old driver to call one or the other 
>with very little sharing.

>This has both advantages and drawbacks but with few people working on the code 
>the less needs to be maintained the better. Then sharing as much code as 
>reasonably possible makes sense, and making bazillion subdirectories does not 
>help that.

We've talked about this issue. We prefer not to change the driver's structure. 
There is a lots of reason why we decide to separate the ddk file for each chip, 
and we decide to separate the code in the beginning of the driver design. So we 
need to confirm one thing: Is this driver ok to be added into the X.Org source 
tree or we must change the codes structure otherwise we cannot be accepted.

Is there anything else we need to change to be accepted by the X.Org. We really 
appreciate your advice. Thank you so much!

Aaron

-----邮件原件-----
发件人: Michal Suchanek [mailto:[email protected]] 
发送时间: 2012年7月17日 20:15
收件人: Aaron.Chen 陈俊杰
抄送: Matt Turner; [email protected]; caesar.qiu 裘赛海; Ilena.Zhou周菁华
主题: Re: 答复: [PATCH]new driver for siliconmotion

On 17 July 2012 10:31, Aaron.Chen  陈俊杰 <[email protected]> wrote:
> Hi Matt,
>
> We really appreciate your advice! The project is very important to us! We 
> have worked for the project for two years. It can support all the SMI 
> graphics chips and works OK on FC, SUSE, Ubuntu Red Hat, etc. for both 32 and 
> 64 bit OS. The code contains two different types of driver. One is support 
> XRandr and the other is not which is for multi-adapter.
> So it may remain some old structure files.
>
> And one more question asked by our develop team:
>>" Do we really have to prefix all these files and directories with 'ddk'?"
>
> So, We'd better change all the name of files named "ddk*". Is that right?
> Aaron

I guess this is not really a requirement but makes the code needlessly 
unattractive.

Since this is an initial submission it is the right time to change this.

If you look at the other drivers they have rather simple structure with all 
files in a single directory. Large part of the code is shared. When support for 
new chipset family is added a few files specific to that chipset are typically 
required which have the chipset name in their file name but much of the rest is 
just updated slightly.

Now you add support for half dozen of chips and I have no idea how they are 
related to each other and to the chips previously supported by the smi driver. 
You should know the best since you wrote the code.

However, the file naming suggests that you started with separate driver for 
every chip and just added hooks in the old driver to call one or the other with 
very little sharing.

This has both advantages and drawbacks but with few people working on the code 
the less needs to be maintained the better. Then sharing as much code as 
reasonably possible makes sense, and making bazillion subdirectories does not 
help that.

Thanks

Michal
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to