Re: [PATCH] ARM: S5PV210: Add Torbreck board support

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

 



On Tue, Sep 28, 2010 at 2:55 PM, Kukjin Kim <kgene.kim@xxxxxxxxxxx> wrote:
> ìêí wrote:
> Hi,
>
> Welcome to Linux mainline ;-)
> I have some comments about your patches.
>
> Firstly could you please use English character in the representing name in e-mail client not Korean character :-)
> And make sure it's text type.
>
>>Hi,
>>Thank you for your interesting.
>>On Mon, Sep 27, 2010 at 11:58 AM, Kyungmin Park <kmpark@xxxxxxxxxxxxx> wrote:
>
> (snip)
>
>>> +#define TORBRECK_UFCON_DEFAULT (S3C2410_UFCON_FIFOMODE | Â Â Â \
>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂS5PV210_UFCON_TXTRIG4 | Â Â Â Â\
>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂS5PV210_UFCON_RXTRIG4)
>>Any reason to use TRIG4? just use the full trigger e.g., 256.
>>
>>Okay, I'll fix it.
>>
> Hmm...Kyungmin, any reason to use full trigger here?
>
> It depends on board...so it doesn't matter TRIG4 or anything else if there is no problem on your board.
> It means the maximum value is not best condition...only depends on your situation/condition.

So I ask ANY REASON to use TRIG4?
And my opinions,
First. it's not true for all UARTs. it's only valid on UART0.
Second, "the maximum value is not best condition" then why Spec said
"increase the FIFO size for performance".

>
>>> +
>>> +static struct s3c2410_uartcfg torbreck_uartcfgs[] __initdata = {
>>> + Â Â Â [0] = {
>>> +        .hwport     = 0,
>>> +        .flags     Â= 0,
>>There's no code for flags, please remove it all.
>>
>>Okay, I'll remove it.
>>
> I think no need to modify it.
> Actually I said many times about this...
>
> And as Ben Dooks said in other patch, the format will be changed soon.

I always listen "will be changed" so until that time use the correct code.
Don't add the meaningless codes.

Thank you,
Kyungmin Park
>
>>> +        .ucon      = TORBRECK_UCON_DEFAULT,
>>> +        .ulcon     Â= TORBRECK_ULCON_DEFAULT,
>>> +        .ufcon     Â= TORBRECK_UFCON_DEFAULT,
>>> + Â Â Â },
>>> + Â Â Â [1] = {
>>> +        .hwport     = 1,
>>> +        .flags     Â= 0,
>>> +        .ucon      = TORBRECK_UCON_DEFAULT,
>>> +        .ulcon     Â= TORBRECK_ULCON_DEFAULT,
>>> +        .ufcon     Â= TORBRECK_UFCON_DEFAULT,
>>> + Â Â Â },
>>> + Â Â Â [2] = {
>>> +        .hwport     = 2,
>>> +        .flags     Â= 0,
>>> +        .ucon      = TORBRECK_UCON_DEFAULT,
>>> +        .ulcon     Â= TORBRECK_ULCON_DEFAULT,
>>> +        .ufcon     Â= TORBRECK_UFCON_DEFAULT,
>>> + Â Â Â },
>>> + Â Â Â [3] = {
>>> +        .hwport     = 3,
>>> +        .flags     Â= 0,
>>> +        .ucon      = TORBRECK_UCON_DEFAULT,
>>> +        .ulcon     Â= TORBRECK_ULCON_DEFAULT,
>>> +        .ufcon     Â= TORBRECK_UFCON_DEFAULT,
>>> + Â Â Â },
>>> +};
>>> +
>
> (snip)
>
>>> --
>>> 1.5.6.3
>>>
> If possible, please use later version git.
> It doesn't mean latest git is best...
> This is just private opinion. :-)
>
> Thanks.
>
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim@xxxxxxxxxxx>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux