Re: [Uclinux-dist-devel] [PATCH 1/4] extend ad1938 codec driver to ad193x supporting ad1936/7/8/9

[Barry Song <21cnbao@gmail.com>]

On Fri, Mar 19, 2010 at 8:24 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Mar 19, 2010 at 03:07:33PM +0800, Barry Song wrote:
>> It seems "git-format-patch -M" fails to detect this patch as a
>> rename(maybe due to too many changed lines?), so I commit two times to
>> get a readable patch and attach them.
>> 1. rename ad1938 to ad193x
>> 2. extend ad1938 codec driver to ad193x supporting ad1936/7/8/9
>
> Please don't top post and please always follow the patch submission
> procedure documented in Documentation/SubmittingPatches.  Even if
> problems with your MUA make it difficult to submit patches without using
> attachments you should always send one patch per message.  Not following
> these rules makes your patches much more difficult to handle.
>
> Since the patches aren't in line it's difficult to quote things when
> commenting but a few issues:
>
>  - Your rename patch didn't update the Makefile and Kconfig, which would
>   cause build breakage if applied alone.  You should always try to
>   ensure that builds work even with a partially applied patch series
>   since this allows things like bisection which step through the
>   history to work.
Yes. if i split into two patches, i should change Makefile and Kconfig
for the 1st one.

>
>  - You've left the bus_probe() functions exported - now you've merged
>   everything into one file this is no longer needed.
Yes. i noticed this too after i sent the patch.
>
>  - The way you've factored out the bus probe and removal functions so
>   that there's no code in the individual I2C and SPI functions means
>   that the register() and unregister() functions could just be squashed
>   into the bus_probe() and bus_remove() functions - all that the
>   register and unregister functions are is the code that's shared
>   between the bus
if so, it likes register/unregister should be moved into probe/remove
even for codecs with single codec?
>
> I've fixed the first issue by squashing the two patches together and the
> last one with a patch.  The last issue isn't important, it's more for
> information.
>
Thanks for your help to fix issues.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel