Re: [PATCH 3/4] davinci: da850: add support for SATA interface

[Sergei Shtylyov <sshtylyov@mvista.com>]

Hello.

Nori, Sekhar wrote:

>>>>> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
>>>>> index 68fe4c2..276199d 100644
>>>>> --- a/arch/arm/mach-davinci/da850.c
>>>>> +++ b/arch/arm/mach-davinci/da850.c
>>>>> @@ -373,6 +373,14 @@ static struct clk spi1_clk = {
>>>>>    	.flags		= DA850_CLK_ASYNC3,
>>>>>    };
>>>>>
>>>>> +static struct clk sata_clk = {
>>>>> +	.name		= "sata",
>>>>> +	.parent		=&pll0_sysclk2,
>>>>> +	.lpsc		= DA850_LPSC1_SATA,
>>>>> +	.gpsc		= 1,
>>>>> +	.flags		= PSC_FORCE,
>>>>> +};
>>>>> +
>>>>>    static struct clk_lookup da850_clks[] = {
>>>>>    	CLK(NULL,		"ref",		&ref_clk),
>>>>>    	CLK(NULL,		"pll0",		&pll0_clk),
>>>>> @@ -419,6 +427,7 @@ static struct clk_lookup da850_clks[] = {
>>>>>    	CLK(NULL,		"usb20",	&usb20_clk),
>>>>>    	CLK("spi_davinci.0",	NULL,		&spi0_clk),
>>>>>    	CLK("spi_davinci.1",	NULL,		&spi1_clk),
>>>>> +	CLK("ahci",		NULL,		&sata_clk),
>>>>>    	CLK(NULL,		NULL,		NULL),
>>>>>    };

>>>>      I'd put the above into a separate patch...

>>> Why should addition of clock data not be in the same patch
>>> as the one which adds platform resources etc? It is not a big
>>> deal to change, but I would like to know why you request this.

>>     I didn't request anything, I just said what I'd have done. :-)

> Okay. I guess I will keep it as is.

>> I think modifying the DA8xx-common and DA850-specific files should better be 
>> done separately. Although in this case we're adding DA850 only device, so 
>> perhaps the added code in devices-da8xx.c should be enclosed into #ifdef?

> Good point. Will add the #ifdef.

    Or perhaps the device should just be placed in da850.c instead?

WBR, Sergei