diff mbox

[v5,12/44] clk: davinci: Add platform information for TI DA850 PSC

Message ID 1515377863-20358-13-git-send-email-david@lechnology.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Lechner Jan. 8, 2018, 2:17 a.m. UTC
This adds platform-specific declarations for the PSC clocks on TI DA850/
OMAP-L138/AM18XX SoCs.

Signed-off-by: David Lechner <david@lechnology.com>
---
 drivers/clk/davinci/Makefile    |   1 +
 drivers/clk/davinci/psc-da850.c | 117 ++++++++++++++++++++++++++++++++++++++++
 include/linux/clk/davinci.h     |   1 +
 3 files changed, 119 insertions(+)
 create mode 100644 drivers/clk/davinci/psc-da850.c

Comments

Sekhar Nori Jan. 16, 2018, 2 p.m. UTC | #1
On Monday 08 January 2018 07:47 AM, David Lechner wrote:
> +void __init da850_psc_clk_init(void __iomem *psc0, void __iomem *psc1)
> +{
> +	struct clk_onecell_data *clk_data;
> +
> +	clk_data = davinci_psc_register_clocks(psc0, da850_psc0_info, 16);
> +	if (!clk_data)
> +		return;
> +
> +	clk_register_clkdev(clk_data->clks[3], NULL, "ti-aemif");
> +	clk_register_clkdev(clk_data->clks[3], "aemif", "davinci-nand.0");
> +	clk_register_clkdev(clk_data->clks[4], NULL, "spi_davinci.0");
> +	clk_register_clkdev(clk_data->clks[5], NULL, "da830-mmc.0");
> +	clk_register_clkdev(clk_data->clks[9], NULL, "serial8250.0");
> +	clk_register_clkdev(clk_data->clks[14], "arm", NULL);
> +	clk_register_clkdev(clk_data->clks[15], NULL, "davinci-rproc.0");
> +
> +	clk_free_onecell_data(clk_data);
> +
> +	clk_data = davinci_psc_register_clocks(psc1, da850_psc1_info, 32);
> +	if (!clk_data)
> +		return;
> +
> +	clk_register_clkdev(clk_data->clks[1], "usb20_psc_clk", NULL);

Is this con_id really needed now? Searching for "usb20_psc_clk" in your
tree results in only this one hit.

> +	clk_register_clkdev(clk_data->clks[1], NULL, "musb-da8xx");
> +	clk_register_clkdev(clk_data->clks[1], NULL, "cppi41-dmaengine");

I guess multiple dev_id matches like these are another hurdle in moving
them to davinci_psc_clk_info[] table? If its too cumbersome to keep
multiple entries in the table, they can be handled as an exception at
the end of processing the table? Still they are not the norm so I hope
the normal case will still benefit.

> +	clk_register_clkdev(clk_data->clks[2], NULL, "ohci-da8xx");
> +	clk_register_clkdev(clk_data->clks[3], "gpio", NULL);
> +	clk_register_clkdev(clk_data->clks[5], NULL, "davinci_emac.1");
> +	clk_register_clkdev(clk_data->clks[5], "fck", "davinci_mdio.0");
> +	clk_register_clkdev(clk_data->clks[7], NULL, "davinci-mcasp.0");
> +	clk_register_clkdev(clk_data->clks[8], "fck", "ahci_da850");
> +	clk_register_clkdev(clk_data->clks[9], NULL, "vpif");
> +	clk_register_clkdev(clk_data->clks[10], NULL, "spi_davinci.1");
> +	clk_register_clkdev(clk_data->clks[11], NULL, "i2c_davinci.2");
> +	clk_register_clkdev(clk_data->clks[12], NULL, "serial8250.1");
> +	clk_register_clkdev(clk_data->clks[13], NULL, "serial8250.2");
> +	clk_register_clkdev(clk_data->clks[14], NULL, "davinci-mcbsp.0");
> +	clk_register_clkdev(clk_data->clks[15], NULL, "davinci-mcbsp.1");
> +	clk_register_clkdev(clk_data->clks[16], "fck", "da8xx_lcdc.0");
> +	clk_register_clkdev(clk_data->clks[17], "fck", "ehrpwm.0");
> +	clk_register_clkdev(clk_data->clks[17], "fck", "ehrpwm.1");
> +	clk_register_clkdev(clk_data->clks[18], NULL, "da830-mmc.1");
> +	clk_register_clkdev(clk_data->clks[20], "fck", "ecap.0");
> +	clk_register_clkdev(clk_data->clks[20], "fck", "ecap.1");
> +	clk_register_clkdev(clk_data->clks[20], "fck", "ecap.2");
> +
> +	clk_free_onecell_data(clk_data);
> +}

Thanks,
Sekhar
David Lechner Jan. 16, 2018, 5:21 p.m. UTC | #2
On 01/16/2018 08:00 AM, Sekhar Nori wrote:
> On Monday 08 January 2018 07:47 AM, David Lechner wrote:
>> +void __init da850_psc_clk_init(void __iomem *psc0, void __iomem *psc1)
>> +{
>> +	struct clk_onecell_data *clk_data;
>> +
>> +	clk_data = davinci_psc_register_clocks(psc0, da850_psc0_info, 16);
>> +	if (!clk_data)
>> +		return;
>> +
>> +	clk_register_clkdev(clk_data->clks[3], NULL, "ti-aemif");
>> +	clk_register_clkdev(clk_data->clks[3], "aemif", "davinci-nand.0");
>> +	clk_register_clkdev(clk_data->clks[4], NULL, "spi_davinci.0");
>> +	clk_register_clkdev(clk_data->clks[5], NULL, "da830-mmc.0");
>> +	clk_register_clkdev(clk_data->clks[9], NULL, "serial8250.0");
>> +	clk_register_clkdev(clk_data->clks[14], "arm", NULL);
>> +	clk_register_clkdev(clk_data->clks[15], NULL, "davinci-rproc.0");
>> +
>> +	clk_free_onecell_data(clk_data);
>> +
>> +	clk_data = davinci_psc_register_clocks(psc1, da850_psc1_info, 32);
>> +	if (!clk_data)
>> +		return;
>> +
>> +	clk_register_clkdev(clk_data->clks[1], "usb20_psc_clk", NULL);
> 
> Is this con_id really needed now? Searching for "usb20_psc_clk" in your
> tree results in only this one hit.

Yes, this is left over from previous attempts.

> 
>> +	clk_register_clkdev(clk_data->clks[1], NULL, "musb-da8xx");
>> +	clk_register_clkdev(clk_data->clks[1], NULL, "cppi41-dmaengine");
> 
> I guess multiple dev_id matches like these are another hurdle in moving
> them to davinci_psc_clk_info[] table? If its too cumbersome to keep
> multiple entries in the table, they can be handled as an exception at
> the end of processing the table? Still they are not the norm so I hope
> the normal case will still benefit.

Right, as I mentioned in the reply to the previous patch, instead of
assigning a con_id and dev_id to each clock, we would need to assign
an array with a list of clocks. I think that would work better than
trying to handle the extras as an exception since there, on average,
about 5 per SoC.

> 
>> +	clk_register_clkdev(clk_data->clks[2], NULL, "ohci-da8xx");
>> +	clk_register_clkdev(clk_data->clks[3], "gpio", NULL);
>> +	clk_register_clkdev(clk_data->clks[5], NULL, "davinci_emac.1");
>> +	clk_register_clkdev(clk_data->clks[5], "fck", "davinci_mdio.0");
>> +	clk_register_clkdev(clk_data->clks[7], NULL, "davinci-mcasp.0");
>> +	clk_register_clkdev(clk_data->clks[8], "fck", "ahci_da850");
>> +	clk_register_clkdev(clk_data->clks[9], NULL, "vpif");
>> +	clk_register_clkdev(clk_data->clks[10], NULL, "spi_davinci.1");
>> +	clk_register_clkdev(clk_data->clks[11], NULL, "i2c_davinci.2");
>> +	clk_register_clkdev(clk_data->clks[12], NULL, "serial8250.1");
>> +	clk_register_clkdev(clk_data->clks[13], NULL, "serial8250.2");
>> +	clk_register_clkdev(clk_data->clks[14], NULL, "davinci-mcbsp.0");
>> +	clk_register_clkdev(clk_data->clks[15], NULL, "davinci-mcbsp.1");
>> +	clk_register_clkdev(clk_data->clks[16], "fck", "da8xx_lcdc.0");
>> +	clk_register_clkdev(clk_data->clks[17], "fck", "ehrpwm.0");
>> +	clk_register_clkdev(clk_data->clks[17], "fck", "ehrpwm.1");
>> +	clk_register_clkdev(clk_data->clks[18], NULL, "da830-mmc.1");
>> +	clk_register_clkdev(clk_data->clks[20], "fck", "ecap.0");
>> +	clk_register_clkdev(clk_data->clks[20], "fck", "ecap.1");
>> +	clk_register_clkdev(clk_data->clks[20], "fck", "ecap.2");
>> +
>> +	clk_free_onecell_data(clk_data);
>> +}
> 
> Thanks,
> Sekhar
>
Sekhar Nori Jan. 17, 2018, 11:57 a.m. UTC | #3
On Tuesday 16 January 2018 10:51 PM, David Lechner wrote:
> On 01/16/2018 08:00 AM, Sekhar Nori wrote:
>> On Monday 08 January 2018 07:47 AM, David Lechner wrote:
>>> +void __init da850_psc_clk_init(void __iomem *psc0, void __iomem *psc1)
>>> +{
>>> +    struct clk_onecell_data *clk_data;
>>> +
>>> +    clk_data = davinci_psc_register_clocks(psc0, da850_psc0_info, 16);
>>> +    if (!clk_data)
>>> +        return;
>>> +
>>> +    clk_register_clkdev(clk_data->clks[3], NULL, "ti-aemif");
>>> +    clk_register_clkdev(clk_data->clks[3], "aemif", "davinci-nand.0");
>>> +    clk_register_clkdev(clk_data->clks[4], NULL, "spi_davinci.0");
>>> +    clk_register_clkdev(clk_data->clks[5], NULL, "da830-mmc.0");
>>> +    clk_register_clkdev(clk_data->clks[9], NULL, "serial8250.0");
>>> +    clk_register_clkdev(clk_data->clks[14], "arm", NULL);
>>> +    clk_register_clkdev(clk_data->clks[15], NULL, "davinci-rproc.0");
>>> +
>>> +    clk_free_onecell_data(clk_data);
>>> +
>>> +    clk_data = davinci_psc_register_clocks(psc1, da850_psc1_info, 32);
>>> +    if (!clk_data)
>>> +        return;
>>> +
>>> +    clk_register_clkdev(clk_data->clks[1], "usb20_psc_clk", NULL);
>>
>> Is this con_id really needed now? Searching for "usb20_psc_clk" in your
>> tree results in only this one hit.
> 
> Yes, this is left over from previous attempts.
> 
>>
>>> +    clk_register_clkdev(clk_data->clks[1], NULL, "musb-da8xx");
>>> +    clk_register_clkdev(clk_data->clks[1], NULL, "cppi41-dmaengine");
>>
>> I guess multiple dev_id matches like these are another hurdle in moving
>> them to davinci_psc_clk_info[] table? If its too cumbersome to keep
>> multiple entries in the table, they can be handled as an exception at
>> the end of processing the table? Still they are not the norm so I hope
>> the normal case will still benefit.
> 
> Right, as I mentioned in the reply to the previous patch, instead of
> assigning a con_id and dev_id to each clock, we would need to assign
> an array with a list of clocks. I think that would work better than
> trying to handle the extras as an exception since there, on average,
> about 5 per SoC.

Okay, are you going to try this to see how it looks? It looks like
samsung (clk-s3c2410.c) and tegra (clk-tegra20.c) use such tables
(although both use separate tables mapping just the gate number to
con_id/dev_id).

Others like u8540_clk.c and clk-mmp2.c have multiple calls in code to
clk_register_clkdev() like you have, but they keep them right after the
gate clock registration which makes it easy to see the mapping.

clk-imx35.c has multiple clk_register_clkdev() calls, but uses an enum
for the gates so its easy to see the mapping. This approach looks fine
to me as well.

So looks like there is a whole gamut of ways people have approached
this. But I do think we need to change the scheme you have currently
since it is difficult to review and audit (believe me on this one :))

Thanks,
Sekhar
David Lechner Jan. 17, 2018, 5:33 p.m. UTC | #4
On 01/17/2018 05:57 AM, Sekhar Nori wrote:
> On Tuesday 16 January 2018 10:51 PM, David Lechner wrote:
>> On 01/16/2018 08:00 AM, Sekhar Nori wrote:
>>> On Monday 08 January 2018 07:47 AM, David Lechner wrote:
>>>> +void __init da850_psc_clk_init(void __iomem *psc0, void __iomem *psc1)
>>>> +{
>>>> +    struct clk_onecell_data *clk_data;
>>>> +
>>>> +    clk_data = davinci_psc_register_clocks(psc0, da850_psc0_info, 16);
>>>> +    if (!clk_data)
>>>> +        return;
>>>> +
>>>> +    clk_register_clkdev(clk_data->clks[3], NULL, "ti-aemif");
>>>> +    clk_register_clkdev(clk_data->clks[3], "aemif", "davinci-nand.0");
>>>> +    clk_register_clkdev(clk_data->clks[4], NULL, "spi_davinci.0");
>>>> +    clk_register_clkdev(clk_data->clks[5], NULL, "da830-mmc.0");
>>>> +    clk_register_clkdev(clk_data->clks[9], NULL, "serial8250.0");
>>>> +    clk_register_clkdev(clk_data->clks[14], "arm", NULL);
>>>> +    clk_register_clkdev(clk_data->clks[15], NULL, "davinci-rproc.0");
>>>> +
>>>> +    clk_free_onecell_data(clk_data);
>>>> +
>>>> +    clk_data = davinci_psc_register_clocks(psc1, da850_psc1_info, 32);
>>>> +    if (!clk_data)
>>>> +        return;
>>>> +
>>>> +    clk_register_clkdev(clk_data->clks[1], "usb20_psc_clk", NULL);
>>>
>>> Is this con_id really needed now? Searching for "usb20_psc_clk" in your
>>> tree results in only this one hit.
>>
>> Yes, this is left over from previous attempts.
>>
>>>
>>>> +    clk_register_clkdev(clk_data->clks[1], NULL, "musb-da8xx");
>>>> +    clk_register_clkdev(clk_data->clks[1], NULL, "cppi41-dmaengine");
>>>
>>> I guess multiple dev_id matches like these are another hurdle in moving
>>> them to davinci_psc_clk_info[] table? If its too cumbersome to keep
>>> multiple entries in the table, they can be handled as an exception at
>>> the end of processing the table? Still they are not the norm so I hope
>>> the normal case will still benefit.
>>
>> Right, as I mentioned in the reply to the previous patch, instead of
>> assigning a con_id and dev_id to each clock, we would need to assign
>> an array with a list of clocks. I think that would work better than
>> trying to handle the extras as an exception since there, on average,
>> about 5 per SoC.
> 
> Okay, are you going to try this to see how it looks? It looks like
> samsung (clk-s3c2410.c) and tegra (clk-tegra20.c) use such tables
> (although both use separate tables mapping just the gate number to
> con_id/dev_id).
> 
> Others like u8540_clk.c and clk-mmp2.c have multiple calls in code to
> clk_register_clkdev() like you have, but they keep them right after the
> gate clock registration which makes it easy to see the mapping.
> 
> clk-imx35.c has multiple clk_register_clkdev() calls, but uses an enum
> for the gates so its easy to see the mapping. This approach looks fine
> to me as well.
> 
> So looks like there is a whole gamut of ways people have approached
> this. But I do think we need to change the scheme you have currently
> since it is difficult to review and audit (believe me on this one :))
> 

OK, I'll figure out something here.
David Lechner Jan. 17, 2018, 7:08 p.m. UTC | #5
On 01/17/2018 05:57 AM, Sekhar Nori wrote:
> On Tuesday 16 January 2018 10:51 PM, David Lechner wrote:
>> On 01/16/2018 08:00 AM, Sekhar Nori wrote:
>>> On Monday 08 January 2018 07:47 AM, David Lechner wrote:
>>>> +void __init da850_psc_clk_init(void __iomem *psc0, void __iomem *psc1)
>>>> +{
>>>> +    struct clk_onecell_data *clk_data;
>>>> +
>>>> +    clk_data = davinci_psc_register_clocks(psc0, da850_psc0_info, 16);
>>>> +    if (!clk_data)
>>>> +        return;
>>>> +
>>>> +    clk_register_clkdev(clk_data->clks[3], NULL, "ti-aemif");
>>>> +    clk_register_clkdev(clk_data->clks[3], "aemif", "davinci-nand.0");
>>>> +    clk_register_clkdev(clk_data->clks[4], NULL, "spi_davinci.0");
>>>> +    clk_register_clkdev(clk_data->clks[5], NULL, "da830-mmc.0");
>>>> +    clk_register_clkdev(clk_data->clks[9], NULL, "serial8250.0");
>>>> +    clk_register_clkdev(clk_data->clks[14], "arm", NULL);
>>>> +    clk_register_clkdev(clk_data->clks[15], NULL, "davinci-rproc.0");
>>>> +
>>>> +    clk_free_onecell_data(clk_data);
>>>> +
>>>> +    clk_data = davinci_psc_register_clocks(psc1, da850_psc1_info, 32);
>>>> +    if (!clk_data)
>>>> +        return;
>>>> +
>>>> +    clk_register_clkdev(clk_data->clks[1], "usb20_psc_clk", NULL);
>>>
>>> Is this con_id really needed now? Searching for "usb20_psc_clk" in your
>>> tree results in only this one hit.
>>
>> Yes, this is left over from previous attempts.
>>
>>>
>>>> +    clk_register_clkdev(clk_data->clks[1], NULL, "musb-da8xx");
>>>> +    clk_register_clkdev(clk_data->clks[1], NULL, "cppi41-dmaengine");
>>>
>>> I guess multiple dev_id matches like these are another hurdle in moving
>>> them to davinci_psc_clk_info[] table? If its too cumbersome to keep
>>> multiple entries in the table, they can be handled as an exception at
>>> the end of processing the table? Still they are not the norm so I hope
>>> the normal case will still benefit.
>>
>> Right, as I mentioned in the reply to the previous patch, instead of
>> assigning a con_id and dev_id to each clock, we would need to assign
>> an array with a list of clocks. I think that would work better than
>> trying to handle the extras as an exception since there, on average,
>> about 5 per SoC.
> 
> Okay, are you going to try this to see how it looks?

It is looking like this:


static const struct davinci_psc_clkdev_info emfia_clkdev[] __initconst = {
	LPSC_CLKDEV(NULL,	"ti-aemif"),
	LPSC_CLKDEV("aemif",	"davinci-nand.0"),
	{ }
};

static const struct davinci_psc_clkdev_info spi0_clkdev[] __initconst = {
	LPSC_CLKDEV(NULL,	"spi_davinci.0"),
	{ }
};

static const struct davinci_psc_clkdev_info mmcsd0_clkdev[] __initconst = {
	LPSC_CLKDEV(NULL,	"da830-mmc.0"),
	{ }
};

static const struct davinci_psc_clkdev_info uart0_clkdev[] __initconst = {
	LPSC_CLKDEV(NULL,	"serial8250.0"),
	{ }
};

static const struct davinci_psc_clkdev_info arm_clkdev[] __initconst = {
	/*
	 * REVISIT: cpufreq-davinci should be modified to use dev_id and drop
	 * use of con_id.
	 */
	LPSC_CLKDEV("arm",	NULL),
	{ }
};

static const struct davinci_psc_clkdev_info dsp_clkdev[] __initconst = {
	LPSC_CLKDEV(NULL,	"davinci-rproc.0"),
	{ }
};

static const struct davinci_psc_clk_info da850_psc0_info[] __initconst = {
	LPSC(0,  0, tpcc0,   pll0_sysclk2, NULL,          LPSC_ALWAYS_ENABLED),
	LPSC(1,  0, tptc0,   pll0_sysclk2, NULL,          LPSC_ALWAYS_ENABLED),
	LPSC(2,  0, tptc1,   pll0_sysclk2, NULL,          LPSC_ALWAYS_ENABLED),
	LPSC(3,  0, emifa,   async1,       emfia_clkdev,  0),
	LPSC(4,  0, spi0,    pll0_sysclk2, spi0_clkdev,   0),
	LPSC(5,  0, mmcsd0,  pll0_sysclk2, mmcsd0_clkdev, 0),
	LPSC(6,  0, aintc,   pll0_sysclk4, NULL,          LPSC_ALWAYS_ENABLED),
	LPSC(7,  0, arm_rom, pll0_sysclk2, NULL,          LPSC_ALWAYS_ENABLED),
	LPSC(9,  0, uart0,   pll0_sysclk2, uart0_clkdev,  0),
	LPSC(13, 0, pruss,   pll0_sysclk2, NULL,          0),
	LPSC(14, 0, arm,     pll0_sysclk6, arm_clkdev,    LPSC_ALWAYS_ENABLED |
							  LPSC_ARM_RATE),
	LPSC(15, 1, dsp,     pll0_sysclk1, dsp_clkdev,    LPSC_FORCE |
							  LPSC_LOCAL_RESET),
	{ }
};
Sekhar Nori Jan. 18, 2018, 6:37 a.m. UTC | #6
On Thursday 18 January 2018 12:38 AM, David Lechner wrote:
> It is looking like this:
> 
> 
> static const struct davinci_psc_clkdev_info emfia_clkdev[] __initconst = {
>     LPSC_CLKDEV(NULL,    "ti-aemif"),
>     LPSC_CLKDEV("aemif",    "davinci-nand.0"),
>     { }
> };
> 
> static const struct davinci_psc_clkdev_info spi0_clkdev[] __initconst = {
>     LPSC_CLKDEV(NULL,    "spi_davinci.0"),
>     { }
> };
> 
> static const struct davinci_psc_clkdev_info mmcsd0_clkdev[] __initconst = {
>     LPSC_CLKDEV(NULL,    "da830-mmc.0"),
>     { }
> };
> 
> static const struct davinci_psc_clkdev_info uart0_clkdev[] __initconst = {
>     LPSC_CLKDEV(NULL,    "serial8250.0"),
>     { }
> };
> 
> static const struct davinci_psc_clkdev_info arm_clkdev[] __initconst = {
>     /*
>      * REVISIT: cpufreq-davinci should be modified to use dev_id and drop
>      * use of con_id.
>      */
>     LPSC_CLKDEV("arm",    NULL),
>     { }
> };
> 
> static const struct davinci_psc_clkdev_info dsp_clkdev[] __initconst = {
>     LPSC_CLKDEV(NULL,    "davinci-rproc.0"),
>     { }
> };
> 
> static const struct davinci_psc_clk_info da850_psc0_info[] __initconst = {
>     LPSC(0,  0, tpcc0,   pll0_sysclk2, NULL,          LPSC_ALWAYS_ENABLED),
>     LPSC(1,  0, tptc0,   pll0_sysclk2, NULL,          LPSC_ALWAYS_ENABLED),
>     LPSC(2,  0, tptc1,   pll0_sysclk2, NULL,          LPSC_ALWAYS_ENABLED),
>     LPSC(3,  0, emifa,   async1,       emfia_clkdev,  0),
>     LPSC(4,  0, spi0,    pll0_sysclk2, spi0_clkdev,   0),
>     LPSC(5,  0, mmcsd0,  pll0_sysclk2, mmcsd0_clkdev, 0),
>     LPSC(6,  0, aintc,   pll0_sysclk4, NULL,          LPSC_ALWAYS_ENABLED),
>     LPSC(7,  0, arm_rom, pll0_sysclk2, NULL,          LPSC_ALWAYS_ENABLED),
>     LPSC(9,  0, uart0,   pll0_sysclk2, uart0_clkdev,  0),
>     LPSC(13, 0, pruss,   pll0_sysclk2, NULL,          0),
>     LPSC(14, 0, arm,     pll0_sysclk6, arm_clkdev,    LPSC_ALWAYS_ENABLED |
>                               LPSC_ARM_RATE),
>     LPSC(15, 1, dsp,     pll0_sysclk1, dsp_clkdev,    LPSC_FORCE |
>                               LPSC_LOCAL_RESET),
>     { }
> };

This looks good to me!

Regards,
Sekhar
Bartosz Golaszewski Feb. 9, 2018, 4:22 p.m. UTC | #7
2018-01-08 3:17 GMT+01:00 David Lechner <david@lechnology.com>:
> This adds platform-specific declarations for the PSC clocks on TI DA850/
> OMAP-L138/AM18XX SoCs.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  drivers/clk/davinci/Makefile    |   1 +
>  drivers/clk/davinci/psc-da850.c | 117 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/clk/davinci.h     |   1 +
>  3 files changed, 119 insertions(+)
>  create mode 100644 drivers/clk/davinci/psc-da850.c
>
> diff --git a/drivers/clk/davinci/Makefile b/drivers/clk/davinci/Makefile
> index fb14c8c..aef0390 100644
> --- a/drivers/clk/davinci/Makefile
> +++ b/drivers/clk/davinci/Makefile
> @@ -11,4 +11,5 @@ obj-$(CONFIG_ARCH_DAVINCI_DM646x)     += pll-dm646x.o
>
>  obj-y += psc.o
>  obj-$(CONFIG_ARCH_DAVINCI_DA830)       += psc-da830.o
> +obj-$(CONFIG_ARCH_DAVINCI_DA850)       += psc-da850.o
>  endif
> diff --git a/drivers/clk/davinci/psc-da850.c b/drivers/clk/davinci/psc-da850.c
> new file mode 100644
> index 0000000..3b4583d
> --- /dev/null
> +++ b/drivers/clk/davinci/psc-da850.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PSC clock descriptions for TI DA850/OMAP-L138/AM18XX
> + *
> + * Copyright (C) 2017 David Lechner <david@lechnology.com>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/init.h>
> +#include <linux/of.h>
> +#include <linux/types.h>
> +
> +#include "psc.h"
> +
> +static const struct davinci_psc_clk_info da850_psc0_info[] __initconst = {
> +       LPSC(0, 0, tpcc0, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
> +       LPSC(1, 0, tptc0, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
> +       LPSC(2, 0, tptc1, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
> +       LPSC(3, 0, aemif, pll0_sysclk3, 0),
> +       LPSC(4, 0, spi0, pll0_sysclk2, 0),
> +       LPSC(5, 0, mmcsd0, pll0_sysclk2, 0),
> +       LPSC(6, 0, aintc, pll0_sysclk4, LPSC_ALWAYS_ENABLED),
> +       LPSC(7, 0, arm_rom, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
> +       LPSC(9, 0, uart0, pll0_sysclk2, 0),
> +       LPSC(13, 0, pruss, pll0_sysclk2, 0),
> +       LPSC(14, 0, arm, pll0_sysclk6, LPSC_ALWAYS_ENABLED),
> +       LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET),
> +       { }
> +};
> +
> +static const struct davinci_psc_clk_info da850_psc1_info[] __initconst = {
> +       LPSC(0, 0, tpcc1, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
> +       LPSC(1, 0, usb0, pll0_sysclk2, 0),
> +       LPSC(2, 0, usb1, pll0_sysclk4, 0),
> +       LPSC(3, 0, gpio, pll0_sysclk4, 0),
> +       LPSC(5, 0, emac, pll0_sysclk4, 0),
> +       LPSC(6, 0, emif3, pll0_sysclk5, LPSC_ALWAYS_ENABLED),
> +       LPSC(7, 0, mcasp0, async3, 0),
> +       LPSC(8, 0, sata, pll0_sysclk2, LPSC_FORCE),
> +       LPSC(9, 0, vpif, pll0_sysclk2, 0),
> +       LPSC(10, 0, spi1, async3, 0),
> +       LPSC(11, 0, i2c1, pll0_sysclk4, 0),
> +       LPSC(12, 0, uart1, async3, 0),
> +       LPSC(13, 0, uart2, async3, 0),
> +       LPSC(14, 0, mcbsp0, async3, 0),
> +       LPSC(15, 0, mcbsp1, async3, 0),
> +       LPSC(16, 0, lcdc, pll0_sysclk2, 0),
> +       LPSC(17, 0, ehrpwm, async3, 0),
> +       LPSC(18, 0, mmcsd1, pll0_sysclk2, 0),
> +       LPSC(20, 0, ecap, async3, 0),
> +       LPSC(21, 0, tptc2, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
> +       { }
> +};
> +
> +void __init da850_psc_clk_init(void __iomem *psc0, void __iomem *psc1)
> +{
> +       struct clk_onecell_data *clk_data;
> +
> +       clk_data = davinci_psc_register_clocks(psc0, da850_psc0_info, 16);
> +       if (!clk_data)
> +               return;
> +
> +       clk_register_clkdev(clk_data->clks[3], NULL, "ti-aemif");
> +       clk_register_clkdev(clk_data->clks[3], "aemif", "davinci-nand.0");
> +       clk_register_clkdev(clk_data->clks[4], NULL, "spi_davinci.0");
> +       clk_register_clkdev(clk_data->clks[5], NULL, "da830-mmc.0");
> +       clk_register_clkdev(clk_data->clks[9], NULL, "serial8250.0");
> +       clk_register_clkdev(clk_data->clks[14], "arm", NULL);
> +       clk_register_clkdev(clk_data->clks[15], NULL, "davinci-rproc.0");
> +
> +       clk_free_onecell_data(clk_data);
> +
> +       clk_data = davinci_psc_register_clocks(psc1, da850_psc1_info, 32);
> +       if (!clk_data)
> +               return;
> +
> +       clk_register_clkdev(clk_data->clks[1], "usb20_psc_clk", NULL);
> +       clk_register_clkdev(clk_data->clks[1], NULL, "musb-da8xx");
> +       clk_register_clkdev(clk_data->clks[1], NULL, "cppi41-dmaengine");
> +       clk_register_clkdev(clk_data->clks[2], NULL, "ohci-da8xx");
> +       clk_register_clkdev(clk_data->clks[3], "gpio", NULL);
> +       clk_register_clkdev(clk_data->clks[5], NULL, "davinci_emac.1");
> +       clk_register_clkdev(clk_data->clks[5], "fck", "davinci_mdio.0");
> +       clk_register_clkdev(clk_data->clks[7], NULL, "davinci-mcasp.0");
> +       clk_register_clkdev(clk_data->clks[8], "fck", "ahci_da850");
> +       clk_register_clkdev(clk_data->clks[9], NULL, "vpif");
> +       clk_register_clkdev(clk_data->clks[10], NULL, "spi_davinci.1");
> +       clk_register_clkdev(clk_data->clks[11], NULL, "i2c_davinci.2");
> +       clk_register_clkdev(clk_data->clks[12], NULL, "serial8250.1");
> +       clk_register_clkdev(clk_data->clks[13], NULL, "serial8250.2");
> +       clk_register_clkdev(clk_data->clks[14], NULL, "davinci-mcbsp.0");
> +       clk_register_clkdev(clk_data->clks[15], NULL, "davinci-mcbsp.1");
> +       clk_register_clkdev(clk_data->clks[16], "fck", "da8xx_lcdc.0");
> +       clk_register_clkdev(clk_data->clks[17], "fck", "ehrpwm.0");
> +       clk_register_clkdev(clk_data->clks[17], "fck", "ehrpwm.1");
> +       clk_register_clkdev(clk_data->clks[18], NULL, "da830-mmc.1");
> +       clk_register_clkdev(clk_data->clks[20], "fck", "ecap.0");
> +       clk_register_clkdev(clk_data->clks[20], "fck", "ecap.1");
> +       clk_register_clkdev(clk_data->clks[20], "fck", "ecap.2");
> +
> +       clk_free_onecell_data(clk_data);
> +}
> +
> +#ifdef CONFIG_OF
> +static void __init of_da850_psc0_clk_init(struct device_node *node)
> +{
> +       of_davinci_psc_clk_init(node, da850_psc0_info, 16);
> +}
> +CLK_OF_DECLARE(da850_psc0_clk, "ti,da850-psc0", of_da850_psc0_clk_init);
> +
> +static void __init of_da850_psc1_clk_init(struct device_node *node)
> +{
> +       of_davinci_psc_clk_init(node, da850_psc1_info, 32);
> +}
> +CLK_OF_DECLARE(da850_psc1_clk, "ti,da850-psc1", of_da850_psc1_clk_init);
> +#endif
> diff --git a/include/linux/clk/davinci.h b/include/linux/clk/davinci.h
> index 3ec8100..3d8bdfa 100644
> --- a/include/linux/clk/davinci.h
> +++ b/include/linux/clk/davinci.h
> @@ -17,5 +17,6 @@ void dm644x_pll_clk_init(void __iomem *pll1, void __iomem *pll2);
>  void dm646x_pll_clk_init(void __iomem *pll1, void __iomem *pll2);
>
>  void da830_psc_clk_init(void __iomem *psc0, void __iomem *psc1);
> +void da850_psc_clk_init(void __iomem *psc0, void __iomem *psc1);
>
>  #endif /* __LINUX_CLK_DAVINCI_H__ */
> --
> 2.7.4
>

Hi David,

I've been working on moving the genpd code from its own driver to the
psc one. I couldn't get the system to boot though and problems
happened very early in the boot sequence. I struggled to figure out
what's happening, but eventually I noticed that psc uses
CLK_OF_DECLARE() to initialize clocks. The functions registered this
way are called very early in the boot sequence, way before
late_initcall() in which the genpd framework is initialized. This of
course makes it impossible to register genpd at the same time we
register PSCs.

Mike, Stephen - it would be great if you could confirm, but I believe
the clock framework now only accepts clock drivers which create real
platform drivers, in which case this series would need to be modified.

David: FYI I quickly converted the functions in psc-da850.c to actual
probe callbacks and it worked just fine on a da850-lcdk board.

Best regards,
Bartosz Golaszewski
Michael Turquette Feb. 9, 2018, 4:48 p.m. UTC | #8
Hi Bartosz, all,

On Fri, Feb 9, 2018 at 8:22 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 2018-01-08 3:17 GMT+01:00 David Lechner <david@lechnology.com>:
>> This adds platform-specific declarations for the PSC clocks on TI DA850/
>> OMAP-L138/AM18XX SoCs.
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> ---
>>  drivers/clk/davinci/Makefile    |   1 +
>>  drivers/clk/davinci/psc-da850.c | 117 ++++++++++++++++++++++++++++++++++++++++
>>  include/linux/clk/davinci.h     |   1 +
>>  3 files changed, 119 insertions(+)
>>  create mode 100644 drivers/clk/davinci/psc-da850.c
>>
>> diff --git a/drivers/clk/davinci/Makefile b/drivers/clk/davinci/Makefile
>> index fb14c8c..aef0390 100644
>> --- a/drivers/clk/davinci/Makefile
>> +++ b/drivers/clk/davinci/Makefile
>> @@ -11,4 +11,5 @@ obj-$(CONFIG_ARCH_DAVINCI_DM646x)     += pll-dm646x.o
>>
>>  obj-y += psc.o
>>  obj-$(CONFIG_ARCH_DAVINCI_DA830)       += psc-da830.o
>> +obj-$(CONFIG_ARCH_DAVINCI_DA850)       += psc-da850.o
>>  endif
>> diff --git a/drivers/clk/davinci/psc-da850.c b/drivers/clk/davinci/psc-da850.c
>> new file mode 100644
>> index 0000000..3b4583d
>> --- /dev/null
>> +++ b/drivers/clk/davinci/psc-da850.c
>> @@ -0,0 +1,117 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * PSC clock descriptions for TI DA850/OMAP-L138/AM18XX
>> + *
>> + * Copyright (C) 2017 David Lechner <david@lechnology.com>
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/init.h>
>> +#include <linux/of.h>
>> +#include <linux/types.h>
>> +
>> +#include "psc.h"
>> +
>> +static const struct davinci_psc_clk_info da850_psc0_info[] __initconst = {
>> +       LPSC(0, 0, tpcc0, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>> +       LPSC(1, 0, tptc0, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>> +       LPSC(2, 0, tptc1, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>> +       LPSC(3, 0, aemif, pll0_sysclk3, 0),
>> +       LPSC(4, 0, spi0, pll0_sysclk2, 0),
>> +       LPSC(5, 0, mmcsd0, pll0_sysclk2, 0),
>> +       LPSC(6, 0, aintc, pll0_sysclk4, LPSC_ALWAYS_ENABLED),
>> +       LPSC(7, 0, arm_rom, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>> +       LPSC(9, 0, uart0, pll0_sysclk2, 0),
>> +       LPSC(13, 0, pruss, pll0_sysclk2, 0),
>> +       LPSC(14, 0, arm, pll0_sysclk6, LPSC_ALWAYS_ENABLED),
>> +       LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET),
>> +       { }
>> +};
>> +
>> +static const struct davinci_psc_clk_info da850_psc1_info[] __initconst = {
>> +       LPSC(0, 0, tpcc1, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>> +       LPSC(1, 0, usb0, pll0_sysclk2, 0),
>> +       LPSC(2, 0, usb1, pll0_sysclk4, 0),
>> +       LPSC(3, 0, gpio, pll0_sysclk4, 0),
>> +       LPSC(5, 0, emac, pll0_sysclk4, 0),
>> +       LPSC(6, 0, emif3, pll0_sysclk5, LPSC_ALWAYS_ENABLED),
>> +       LPSC(7, 0, mcasp0, async3, 0),
>> +       LPSC(8, 0, sata, pll0_sysclk2, LPSC_FORCE),
>> +       LPSC(9, 0, vpif, pll0_sysclk2, 0),
>> +       LPSC(10, 0, spi1, async3, 0),
>> +       LPSC(11, 0, i2c1, pll0_sysclk4, 0),
>> +       LPSC(12, 0, uart1, async3, 0),
>> +       LPSC(13, 0, uart2, async3, 0),
>> +       LPSC(14, 0, mcbsp0, async3, 0),
>> +       LPSC(15, 0, mcbsp1, async3, 0),
>> +       LPSC(16, 0, lcdc, pll0_sysclk2, 0),
>> +       LPSC(17, 0, ehrpwm, async3, 0),
>> +       LPSC(18, 0, mmcsd1, pll0_sysclk2, 0),
>> +       LPSC(20, 0, ecap, async3, 0),
>> +       LPSC(21, 0, tptc2, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>> +       { }
>> +};
>> +
>> +void __init da850_psc_clk_init(void __iomem *psc0, void __iomem *psc1)
>> +{
>> +       struct clk_onecell_data *clk_data;
>> +
>> +       clk_data = davinci_psc_register_clocks(psc0, da850_psc0_info, 16);
>> +       if (!clk_data)
>> +               return;
>> +
>> +       clk_register_clkdev(clk_data->clks[3], NULL, "ti-aemif");
>> +       clk_register_clkdev(clk_data->clks[3], "aemif", "davinci-nand.0");
>> +       clk_register_clkdev(clk_data->clks[4], NULL, "spi_davinci.0");
>> +       clk_register_clkdev(clk_data->clks[5], NULL, "da830-mmc.0");
>> +       clk_register_clkdev(clk_data->clks[9], NULL, "serial8250.0");
>> +       clk_register_clkdev(clk_data->clks[14], "arm", NULL);
>> +       clk_register_clkdev(clk_data->clks[15], NULL, "davinci-rproc.0");
>> +
>> +       clk_free_onecell_data(clk_data);
>> +
>> +       clk_data = davinci_psc_register_clocks(psc1, da850_psc1_info, 32);
>> +       if (!clk_data)
>> +               return;
>> +
>> +       clk_register_clkdev(clk_data->clks[1], "usb20_psc_clk", NULL);
>> +       clk_register_clkdev(clk_data->clks[1], NULL, "musb-da8xx");
>> +       clk_register_clkdev(clk_data->clks[1], NULL, "cppi41-dmaengine");
>> +       clk_register_clkdev(clk_data->clks[2], NULL, "ohci-da8xx");
>> +       clk_register_clkdev(clk_data->clks[3], "gpio", NULL);
>> +       clk_register_clkdev(clk_data->clks[5], NULL, "davinci_emac.1");
>> +       clk_register_clkdev(clk_data->clks[5], "fck", "davinci_mdio.0");
>> +       clk_register_clkdev(clk_data->clks[7], NULL, "davinci-mcasp.0");
>> +       clk_register_clkdev(clk_data->clks[8], "fck", "ahci_da850");
>> +       clk_register_clkdev(clk_data->clks[9], NULL, "vpif");
>> +       clk_register_clkdev(clk_data->clks[10], NULL, "spi_davinci.1");
>> +       clk_register_clkdev(clk_data->clks[11], NULL, "i2c_davinci.2");
>> +       clk_register_clkdev(clk_data->clks[12], NULL, "serial8250.1");
>> +       clk_register_clkdev(clk_data->clks[13], NULL, "serial8250.2");
>> +       clk_register_clkdev(clk_data->clks[14], NULL, "davinci-mcbsp.0");
>> +       clk_register_clkdev(clk_data->clks[15], NULL, "davinci-mcbsp.1");
>> +       clk_register_clkdev(clk_data->clks[16], "fck", "da8xx_lcdc.0");
>> +       clk_register_clkdev(clk_data->clks[17], "fck", "ehrpwm.0");
>> +       clk_register_clkdev(clk_data->clks[17], "fck", "ehrpwm.1");
>> +       clk_register_clkdev(clk_data->clks[18], NULL, "da830-mmc.1");
>> +       clk_register_clkdev(clk_data->clks[20], "fck", "ecap.0");
>> +       clk_register_clkdev(clk_data->clks[20], "fck", "ecap.1");
>> +       clk_register_clkdev(clk_data->clks[20], "fck", "ecap.2");
>> +
>> +       clk_free_onecell_data(clk_data);
>> +}
>> +
>> +#ifdef CONFIG_OF
>> +static void __init of_da850_psc0_clk_init(struct device_node *node)
>> +{
>> +       of_davinci_psc_clk_init(node, da850_psc0_info, 16);
>> +}
>> +CLK_OF_DECLARE(da850_psc0_clk, "ti,da850-psc0", of_da850_psc0_clk_init);
>> +
>> +static void __init of_da850_psc1_clk_init(struct device_node *node)
>> +{
>> +       of_davinci_psc_clk_init(node, da850_psc1_info, 32);
>> +}
>> +CLK_OF_DECLARE(da850_psc1_clk, "ti,da850-psc1", of_da850_psc1_clk_init);
>> +#endif
>> diff --git a/include/linux/clk/davinci.h b/include/linux/clk/davinci.h
>> index 3ec8100..3d8bdfa 100644
>> --- a/include/linux/clk/davinci.h
>> +++ b/include/linux/clk/davinci.h
>> @@ -17,5 +17,6 @@ void dm644x_pll_clk_init(void __iomem *pll1, void __iomem *pll2);
>>  void dm646x_pll_clk_init(void __iomem *pll1, void __iomem *pll2);
>>
>>  void da830_psc_clk_init(void __iomem *psc0, void __iomem *psc1);
>> +void da850_psc_clk_init(void __iomem *psc0, void __iomem *psc1);
>>
>>  #endif /* __LINUX_CLK_DAVINCI_H__ */
>> --
>> 2.7.4
>>
>
> Hi David,
>
> I've been working on moving the genpd code from its own driver to the
> psc one. I couldn't get the system to boot though and problems
> happened very early in the boot sequence. I struggled to figure out
> what's happening, but eventually I noticed that psc uses
> CLK_OF_DECLARE() to initialize clocks. The functions registered this
> way are called very early in the boot sequence, way before
> late_initcall() in which the genpd framework is initialized. This of
> course makes it impossible to register genpd at the same time we
> register PSCs.
>
> Mike, Stephen - it would be great if you could confirm, but I believe
> the clock framework now only accepts clock drivers which create real
> platform drivers, in which case this series would need to be modified.

You are correct. All new uses of CLK_OF_DECLARE are met with high
suspicion, and all new drivers should be platform drivers.

There are cases when CLK_OF_DECLARE is necessary (sometimes
temporarily while things are reworked), but it seems this is not the
case for this driver based on your testing of the platform driver
approach. Please use the platform driver approach instead.

Thanks,
Mike

>
> David: FYI I quickly converted the functions in psc-da850.c to actual
> probe callbacks and it worked just fine on a da850-lcdk board.
>
> Best regards,
> Bartosz Golaszewski
David Lechner Feb. 12, 2018, 3:03 a.m. UTC | #9
On 02/09/2018 10:48 AM, Michael Turquette wrote:
> Hi Bartosz, all,
> 
> On Fri, Feb 9, 2018 at 8:22 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> 2018-01-08 3:17 GMT+01:00 David Lechner <david@lechnology.com>:
>>> This adds platform-specific declarations for the PSC clocks on TI DA850/
>>> OMAP-L138/AM18XX SoCs.
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>> ---
>>>   drivers/clk/davinci/Makefile    |   1 +
>>>   drivers/clk/davinci/psc-da850.c | 117 ++++++++++++++++++++++++++++++++++++++++
>>>   include/linux/clk/davinci.h     |   1 +
>>>   3 files changed, 119 insertions(+)
>>>   create mode 100644 drivers/clk/davinci/psc-da850.c
>>>
>>> diff --git a/drivers/clk/davinci/Makefile b/drivers/clk/davinci/Makefile
>>> index fb14c8c..aef0390 100644
>>> --- a/drivers/clk/davinci/Makefile
>>> +++ b/drivers/clk/davinci/Makefile
>>> @@ -11,4 +11,5 @@ obj-$(CONFIG_ARCH_DAVINCI_DM646x)     += pll-dm646x.o
>>>
>>>   obj-y += psc.o
>>>   obj-$(CONFIG_ARCH_DAVINCI_DA830)       += psc-da830.o
>>> +obj-$(CONFIG_ARCH_DAVINCI_DA850)       += psc-da850.o
>>>   endif
>>> diff --git a/drivers/clk/davinci/psc-da850.c b/drivers/clk/davinci/psc-da850.c
>>> new file mode 100644
>>> index 0000000..3b4583d
>>> --- /dev/null
>>> +++ b/drivers/clk/davinci/psc-da850.c
>>> @@ -0,0 +1,117 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * PSC clock descriptions for TI DA850/OMAP-L138/AM18XX
>>> + *
>>> + * Copyright (C) 2017 David Lechner <david@lechnology.com>
>>> + */
>>> +
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/clkdev.h>
>>> +#include <linux/init.h>
>>> +#include <linux/of.h>
>>> +#include <linux/types.h>
>>> +
>>> +#include "psc.h"
>>> +
>>> +static const struct davinci_psc_clk_info da850_psc0_info[] __initconst = {
>>> +       LPSC(0, 0, tpcc0, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>>> +       LPSC(1, 0, tptc0, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>>> +       LPSC(2, 0, tptc1, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>>> +       LPSC(3, 0, aemif, pll0_sysclk3, 0),
>>> +       LPSC(4, 0, spi0, pll0_sysclk2, 0),
>>> +       LPSC(5, 0, mmcsd0, pll0_sysclk2, 0),
>>> +       LPSC(6, 0, aintc, pll0_sysclk4, LPSC_ALWAYS_ENABLED),
>>> +       LPSC(7, 0, arm_rom, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>>> +       LPSC(9, 0, uart0, pll0_sysclk2, 0),
>>> +       LPSC(13, 0, pruss, pll0_sysclk2, 0),
>>> +       LPSC(14, 0, arm, pll0_sysclk6, LPSC_ALWAYS_ENABLED),
>>> +       LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET),
>>> +       { }
>>> +};
>>> +
>>> +static const struct davinci_psc_clk_info da850_psc1_info[] __initconst = {
>>> +       LPSC(0, 0, tpcc1, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>>> +       LPSC(1, 0, usb0, pll0_sysclk2, 0),
>>> +       LPSC(2, 0, usb1, pll0_sysclk4, 0),
>>> +       LPSC(3, 0, gpio, pll0_sysclk4, 0),
>>> +       LPSC(5, 0, emac, pll0_sysclk4, 0),
>>> +       LPSC(6, 0, emif3, pll0_sysclk5, LPSC_ALWAYS_ENABLED),
>>> +       LPSC(7, 0, mcasp0, async3, 0),
>>> +       LPSC(8, 0, sata, pll0_sysclk2, LPSC_FORCE),
>>> +       LPSC(9, 0, vpif, pll0_sysclk2, 0),
>>> +       LPSC(10, 0, spi1, async3, 0),
>>> +       LPSC(11, 0, i2c1, pll0_sysclk4, 0),
>>> +       LPSC(12, 0, uart1, async3, 0),
>>> +       LPSC(13, 0, uart2, async3, 0),
>>> +       LPSC(14, 0, mcbsp0, async3, 0),
>>> +       LPSC(15, 0, mcbsp1, async3, 0),
>>> +       LPSC(16, 0, lcdc, pll0_sysclk2, 0),
>>> +       LPSC(17, 0, ehrpwm, async3, 0),
>>> +       LPSC(18, 0, mmcsd1, pll0_sysclk2, 0),
>>> +       LPSC(20, 0, ecap, async3, 0),
>>> +       LPSC(21, 0, tptc2, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>>> +       { }
>>> +};
>>> +
>>> +void __init da850_psc_clk_init(void __iomem *psc0, void __iomem *psc1)
>>> +{
>>> +       struct clk_onecell_data *clk_data;
>>> +
>>> +       clk_data = davinci_psc_register_clocks(psc0, da850_psc0_info, 16);
>>> +       if (!clk_data)
>>> +               return;
>>> +
>>> +       clk_register_clkdev(clk_data->clks[3], NULL, "ti-aemif");
>>> +       clk_register_clkdev(clk_data->clks[3], "aemif", "davinci-nand.0");
>>> +       clk_register_clkdev(clk_data->clks[4], NULL, "spi_davinci.0");
>>> +       clk_register_clkdev(clk_data->clks[5], NULL, "da830-mmc.0");
>>> +       clk_register_clkdev(clk_data->clks[9], NULL, "serial8250.0");
>>> +       clk_register_clkdev(clk_data->clks[14], "arm", NULL);
>>> +       clk_register_clkdev(clk_data->clks[15], NULL, "davinci-rproc.0");
>>> +
>>> +       clk_free_onecell_data(clk_data);
>>> +
>>> +       clk_data = davinci_psc_register_clocks(psc1, da850_psc1_info, 32);
>>> +       if (!clk_data)
>>> +               return;
>>> +
>>> +       clk_register_clkdev(clk_data->clks[1], "usb20_psc_clk", NULL);
>>> +       clk_register_clkdev(clk_data->clks[1], NULL, "musb-da8xx");
>>> +       clk_register_clkdev(clk_data->clks[1], NULL, "cppi41-dmaengine");
>>> +       clk_register_clkdev(clk_data->clks[2], NULL, "ohci-da8xx");
>>> +       clk_register_clkdev(clk_data->clks[3], "gpio", NULL);
>>> +       clk_register_clkdev(clk_data->clks[5], NULL, "davinci_emac.1");
>>> +       clk_register_clkdev(clk_data->clks[5], "fck", "davinci_mdio.0");
>>> +       clk_register_clkdev(clk_data->clks[7], NULL, "davinci-mcasp.0");
>>> +       clk_register_clkdev(clk_data->clks[8], "fck", "ahci_da850");
>>> +       clk_register_clkdev(clk_data->clks[9], NULL, "vpif");
>>> +       clk_register_clkdev(clk_data->clks[10], NULL, "spi_davinci.1");
>>> +       clk_register_clkdev(clk_data->clks[11], NULL, "i2c_davinci.2");
>>> +       clk_register_clkdev(clk_data->clks[12], NULL, "serial8250.1");
>>> +       clk_register_clkdev(clk_data->clks[13], NULL, "serial8250.2");
>>> +       clk_register_clkdev(clk_data->clks[14], NULL, "davinci-mcbsp.0");
>>> +       clk_register_clkdev(clk_data->clks[15], NULL, "davinci-mcbsp.1");
>>> +       clk_register_clkdev(clk_data->clks[16], "fck", "da8xx_lcdc.0");
>>> +       clk_register_clkdev(clk_data->clks[17], "fck", "ehrpwm.0");
>>> +       clk_register_clkdev(clk_data->clks[17], "fck", "ehrpwm.1");
>>> +       clk_register_clkdev(clk_data->clks[18], NULL, "da830-mmc.1");
>>> +       clk_register_clkdev(clk_data->clks[20], "fck", "ecap.0");
>>> +       clk_register_clkdev(clk_data->clks[20], "fck", "ecap.1");
>>> +       clk_register_clkdev(clk_data->clks[20], "fck", "ecap.2");
>>> +
>>> +       clk_free_onecell_data(clk_data);
>>> +}
>>> +
>>> +#ifdef CONFIG_OF
>>> +static void __init of_da850_psc0_clk_init(struct device_node *node)
>>> +{
>>> +       of_davinci_psc_clk_init(node, da850_psc0_info, 16);
>>> +}
>>> +CLK_OF_DECLARE(da850_psc0_clk, "ti,da850-psc0", of_da850_psc0_clk_init);
>>> +
>>> +static void __init of_da850_psc1_clk_init(struct device_node *node)
>>> +{
>>> +       of_davinci_psc_clk_init(node, da850_psc1_info, 32);
>>> +}
>>> +CLK_OF_DECLARE(da850_psc1_clk, "ti,da850-psc1", of_da850_psc1_clk_init);
>>> +#endif
>>> diff --git a/include/linux/clk/davinci.h b/include/linux/clk/davinci.h
>>> index 3ec8100..3d8bdfa 100644
>>> --- a/include/linux/clk/davinci.h
>>> +++ b/include/linux/clk/davinci.h
>>> @@ -17,5 +17,6 @@ void dm644x_pll_clk_init(void __iomem *pll1, void __iomem *pll2);
>>>   void dm646x_pll_clk_init(void __iomem *pll1, void __iomem *pll2);
>>>
>>>   void da830_psc_clk_init(void __iomem *psc0, void __iomem *psc1);
>>> +void da850_psc_clk_init(void __iomem *psc0, void __iomem *psc1);
>>>
>>>   #endif /* __LINUX_CLK_DAVINCI_H__ */
>>> --
>>> 2.7.4
>>>
>>
>> Hi David,
>>
>> I've been working on moving the genpd code from its own driver to the
>> psc one. I couldn't get the system to boot though and problems
>> happened very early in the boot sequence. I struggled to figure out
>> what's happening, but eventually I noticed that psc uses
>> CLK_OF_DECLARE() to initialize clocks. The functions registered this
>> way are called very early in the boot sequence, way before
>> late_initcall() in which the genpd framework is initialized. This of
>> course makes it impossible to register genpd at the same time we
>> register PSCs.
>>
>> Mike, Stephen - it would be great if you could confirm, but I believe
>> the clock framework now only accepts clock drivers which create real
>> platform drivers, in which case this series would need to be modified.
> 
> You are correct. All new uses of CLK_OF_DECLARE are met with high
> suspicion, and all new drivers should be platform drivers.

Oh boy. I'll try to get a v7 out this week with these changes.



> 
> There are cases when CLK_OF_DECLARE is necessary (sometimes
> temporarily while things are reworked), but it seems this is not the
> case for this driver based on your testing of the platform driver
> approach. Please use the platform driver approach instead.
> 
> Thanks,
> Mike
> 
>>
>> David: FYI I quickly converted the functions in psc-da850.c to actual
>> probe callbacks and it worked just fine on a da850-lcdk board.
>>
>> Best regards,
>> Bartosz Golaszewski
Sekhar Nori April 5, 2018, 1:09 p.m. UTC | #10
Hi Bartosz,

On Friday 09 February 2018 10:18 PM, Michael Turquette wrote:
> On Fri, Feb 9, 2018 at 8:22 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> 2018-01-08 3:17 GMT+01:00 David Lechner <david@lechnology.com>:

>> Hi David,
>>
>> I've been working on moving the genpd code from its own driver to the
>> psc one. I couldn't get the system to boot though and problems
>> happened very early in the boot sequence. I struggled to figure out
>> what's happening, but eventually I noticed that psc uses
>> CLK_OF_DECLARE() to initialize clocks. The functions registered this
>> way are called very early in the boot sequence, way before
>> late_initcall() in which the genpd framework is initialized. This of

late_initcall() is too late for genpd to be initialized. As you may have
seen with the latest set of patches, we have problems with timer
initialization. After converting to platform devices, PSC and PLL clocks
get initialized post time_init(). We are working that around using
fixed-clocks, which hopefully will work (I still need to test many of
the affected platforms).

Can you please reply with the exact issue you faced with genpd framework
initialization so we do have that on record.

Thanks,
Sekhar

>> course makes it impossible to register genpd at the same time we
>> register PSCs.
>>
>> Mike, Stephen - it would be great if you could confirm, but I believe
>> the clock framework now only accepts clock drivers which create real
>> platform drivers, in which case this series would need to be modified.
> 
> You are correct. All new uses of CLK_OF_DECLARE are met with high
> suspicion, and all new drivers should be platform drivers.
> 
> There are cases when CLK_OF_DECLARE is necessary (sometimes
> temporarily while things are reworked), but it seems this is not the
> case for this driver based on your testing of the platform driver
> approach. Please use the platform driver approach instead.
> 
> Thanks,
> Mike
> 
>>
>> David: FYI I quickly converted the functions in psc-da850.c to actual
>> probe callbacks and it worked just fine on a da850-lcdk board.
>>
>> Best regards,
>> Bartosz Golaszewski
Bartosz Golaszewski April 5, 2018, 1:44 p.m. UTC | #11
2018-04-05 15:09 GMT+02:00 Sekhar Nori <nsekhar@ti.com>:
> Hi Bartosz,
>
> On Friday 09 February 2018 10:18 PM, Michael Turquette wrote:
>> On Fri, Feb 9, 2018 at 8:22 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>> 2018-01-08 3:17 GMT+01:00 David Lechner <david@lechnology.com>:
>
>>> Hi David,
>>>
>>> I've been working on moving the genpd code from its own driver to the
>>> psc one. I couldn't get the system to boot though and problems
>>> happened very early in the boot sequence. I struggled to figure out
>>> what's happening, but eventually I noticed that psc uses
>>> CLK_OF_DECLARE() to initialize clocks. The functions registered this
>>> way are called very early in the boot sequence, way before
>>> late_initcall() in which the genpd framework is initialized. This of
>
> late_initcall() is too late for genpd to be initialized. As you may have
> seen with the latest set of patches, we have problems with timer
> initialization. After converting to platform devices, PSC and PLL clocks
> get initialized post time_init(). We are working that around using
> fixed-clocks, which hopefully will work (I still need to test many of
> the affected platforms).
>
> Can you please reply with the exact issue you faced with genpd framework
> initialization so we do have that on record.
>

The exact issue manifested itself in a NULL-pointer dereference panic
when I tried moving the genpd code I had initially implemented as a
separate platform driver to what I believe was v6 or v7 of David's
series (before the psc driver became a platform driver, when it was
still using CLK_OF_DECLARE()). When I had tested a simple conversion
of that version to a platform_driver, genpd worked fine.

I don't have the stack traces from these panics, but I recall some
debugfs functions being involved and the genpd late_initcalls are
related to debugfs. Looking at it now I don't see how exactly it could
fail though.

Best regards,
Bartosz
Sekhar Nori April 5, 2018, 2:36 p.m. UTC | #12
On Thursday 05 April 2018 07:14 PM, Bartosz Golaszewski wrote:
> 2018-04-05 15:09 GMT+02:00 Sekhar Nori <nsekhar@ti.com>:
>> Hi Bartosz,
>>
>> On Friday 09 February 2018 10:18 PM, Michael Turquette wrote:
>>> On Fri, Feb 9, 2018 at 8:22 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>>> 2018-01-08 3:17 GMT+01:00 David Lechner <david@lechnology.com>:
>>
>>>> Hi David,
>>>>
>>>> I've been working on moving the genpd code from its own driver to the
>>>> psc one. I couldn't get the system to boot though and problems
>>>> happened very early in the boot sequence. I struggled to figure out
>>>> what's happening, but eventually I noticed that psc uses
>>>> CLK_OF_DECLARE() to initialize clocks. The functions registered this
>>>> way are called very early in the boot sequence, way before
>>>> late_initcall() in which the genpd framework is initialized. This of
>>
>> late_initcall() is too late for genpd to be initialized. As you may have
>> seen with the latest set of patches, we have problems with timer
>> initialization. After converting to platform devices, PSC and PLL clocks
>> get initialized post time_init(). We are working that around using
>> fixed-clocks, which hopefully will work (I still need to test many of
>> the affected platforms).
>>
>> Can you please reply with the exact issue you faced with genpd framework
>> initialization so we do have that on record.
>>
> 
> The exact issue manifested itself in a NULL-pointer dereference panic
> when I tried moving the genpd code I had initially implemented as a
> separate platform driver to what I believe was v6 or v7 of David's
> series (before the psc driver became a platform driver, when it was
> still using CLK_OF_DECLARE()). When I had tested a simple conversion
> of that version to a platform_driver, genpd worked fine.
> 
> I don't have the stack traces from these panics, but I recall some
> debugfs functions being involved and the genpd late_initcalls are
> related to debugfs. Looking at it now I don't see how exactly it could
> fail though.

Do you have the code where you faced the problem stashed somewhere? I am
not (yet) advocating going back to CLK_OF_DECLARE(). But there is a
definite issue with timer being ready when not using CLK_OF_DECLARE().
So, I want to make sure there the reason why we are going down the
platform device path is a amply clear.

Thanks,
Sekhar
David Lechner April 5, 2018, 3:37 p.m. UTC | #13
On 04/05/2018 09:36 AM, Sekhar Nori wrote:
> On Thursday 05 April 2018 07:14 PM, Bartosz Golaszewski wrote:
>> 2018-04-05 15:09 GMT+02:00 Sekhar Nori <nsekhar@ti.com>:
>>> Hi Bartosz,
>>>
>>> On Friday 09 February 2018 10:18 PM, Michael Turquette wrote:
>>>> On Fri, Feb 9, 2018 at 8:22 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>>>> 2018-01-08 3:17 GMT+01:00 David Lechner <david@lechnology.com>:
>>>
>>>>> Hi David,
>>>>>
>>>>> I've been working on moving the genpd code from its own driver to the
>>>>> psc one. I couldn't get the system to boot though and problems
>>>>> happened very early in the boot sequence. I struggled to figure out
>>>>> what's happening, but eventually I noticed that psc uses
>>>>> CLK_OF_DECLARE() to initialize clocks. The functions registered this
>>>>> way are called very early in the boot sequence, way before
>>>>> late_initcall() in which the genpd framework is initialized. This of
>>>
>>> late_initcall() is too late for genpd to be initialized. As you may have
>>> seen with the latest set of patches, we have problems with timer
>>> initialization. After converting to platform devices, PSC and PLL clocks
>>> get initialized post time_init(). We are working that around using
>>> fixed-clocks, which hopefully will work (I still need to test many of
>>> the affected platforms).
>>>
>>> Can you please reply with the exact issue you faced with genpd framework
>>> initialization so we do have that on record.
>>>
>>
>> The exact issue manifested itself in a NULL-pointer dereference panic
>> when I tried moving the genpd code I had initially implemented as a
>> separate platform driver to what I believe was v6 or v7 of David's
>> series (before the psc driver became a platform driver, when it was
>> still using CLK_OF_DECLARE()). When I had tested a simple conversion
>> of that version to a platform_driver, genpd worked fine.
>>
>> I don't have the stack traces from these panics, but I recall some
>> debugfs functions being involved and the genpd late_initcalls are
>> related to debugfs. Looking at it now I don't see how exactly it could
>> fail though.
> 
> Do you have the code where you faced the problem stashed somewhere? I am
> not (yet) advocating going back to CLK_OF_DECLARE(). But there is a
> definite issue with timer being ready when not using CLK_OF_DECLARE().
> So, I want to make sure there the reason why we are going down the
> platform device path is a amply clear.
> 
> Thanks,
> Sekhar
> 
This is the thread that led to the conversion to platform devices:

https://lkml.org/lkml/2018/2/9/541
Bartosz Golaszewski April 5, 2018, 3:51 p.m. UTC | #14
2018-04-05 16:36 GMT+02:00 Sekhar Nori <nsekhar@ti.com>:
> On Thursday 05 April 2018 07:14 PM, Bartosz Golaszewski wrote:
>> 2018-04-05 15:09 GMT+02:00 Sekhar Nori <nsekhar@ti.com>:
>>> Hi Bartosz,
>>>
>>> On Friday 09 February 2018 10:18 PM, Michael Turquette wrote:
>>>> On Fri, Feb 9, 2018 at 8:22 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>>>> 2018-01-08 3:17 GMT+01:00 David Lechner <david@lechnology.com>:
>>>
>>>>> Hi David,
>>>>>
>>>>> I've been working on moving the genpd code from its own driver to the
>>>>> psc one. I couldn't get the system to boot though and problems
>>>>> happened very early in the boot sequence. I struggled to figure out
>>>>> what's happening, but eventually I noticed that psc uses
>>>>> CLK_OF_DECLARE() to initialize clocks. The functions registered this
>>>>> way are called very early in the boot sequence, way before
>>>>> late_initcall() in which the genpd framework is initialized. This of
>>>
>>> late_initcall() is too late for genpd to be initialized. As you may have
>>> seen with the latest set of patches, we have problems with timer
>>> initialization. After converting to platform devices, PSC and PLL clocks
>>> get initialized post time_init(). We are working that around using
>>> fixed-clocks, which hopefully will work (I still need to test many of
>>> the affected platforms).
>>>
>>> Can you please reply with the exact issue you faced with genpd framework
>>> initialization so we do have that on record.
>>>
>>
>> The exact issue manifested itself in a NULL-pointer dereference panic
>> when I tried moving the genpd code I had initially implemented as a
>> separate platform driver to what I believe was v6 or v7 of David's
>> series (before the psc driver became a platform driver, when it was
>> still using CLK_OF_DECLARE()). When I had tested a simple conversion
>> of that version to a platform_driver, genpd worked fine.
>>
>> I don't have the stack traces from these panics, but I recall some
>> debugfs functions being involved and the genpd late_initcalls are
>> related to debugfs. Looking at it now I don't see how exactly it could
>> fail though.
>
> Do you have the code where you faced the problem stashed somewhere? I am
> not (yet) advocating going back to CLK_OF_DECLARE(). But there is a
> definite issue with timer being ready when not using CLK_OF_DECLARE().
> So, I want to make sure there the reason why we are going down the
> platform device path is a amply clear.
>
> Thanks,
> Sekhar

Yes, you can still find it on my github[1].

Bart

[1] github.com:brgl/linux.git  topic/davinci-genpd-final-v2
diff mbox

Patch

diff --git a/drivers/clk/davinci/Makefile b/drivers/clk/davinci/Makefile
index fb14c8c..aef0390 100644
--- a/drivers/clk/davinci/Makefile
+++ b/drivers/clk/davinci/Makefile
@@ -11,4 +11,5 @@  obj-$(CONFIG_ARCH_DAVINCI_DM646x)	+= pll-dm646x.o
 
 obj-y += psc.o
 obj-$(CONFIG_ARCH_DAVINCI_DA830)	+= psc-da830.o
+obj-$(CONFIG_ARCH_DAVINCI_DA850)	+= psc-da850.o
 endif
diff --git a/drivers/clk/davinci/psc-da850.c b/drivers/clk/davinci/psc-da850.c
new file mode 100644
index 0000000..3b4583d
--- /dev/null
+++ b/drivers/clk/davinci/psc-da850.c
@@ -0,0 +1,117 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PSC clock descriptions for TI DA850/OMAP-L138/AM18XX
+ *
+ * Copyright (C) 2017 David Lechner <david@lechnology.com>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/init.h>
+#include <linux/of.h>
+#include <linux/types.h>
+
+#include "psc.h"
+
+static const struct davinci_psc_clk_info da850_psc0_info[] __initconst = {
+	LPSC(0, 0, tpcc0, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
+	LPSC(1, 0, tptc0, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
+	LPSC(2, 0, tptc1, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
+	LPSC(3, 0, aemif, pll0_sysclk3, 0),
+	LPSC(4, 0, spi0, pll0_sysclk2, 0),
+	LPSC(5, 0, mmcsd0, pll0_sysclk2, 0),
+	LPSC(6, 0, aintc, pll0_sysclk4, LPSC_ALWAYS_ENABLED),
+	LPSC(7, 0, arm_rom, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
+	LPSC(9, 0, uart0, pll0_sysclk2, 0),
+	LPSC(13, 0, pruss, pll0_sysclk2, 0),
+	LPSC(14, 0, arm, pll0_sysclk6, LPSC_ALWAYS_ENABLED),
+	LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET),
+	{ }
+};
+
+static const struct davinci_psc_clk_info da850_psc1_info[] __initconst = {
+	LPSC(0, 0, tpcc1, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
+	LPSC(1, 0, usb0, pll0_sysclk2, 0),
+	LPSC(2, 0, usb1, pll0_sysclk4, 0),
+	LPSC(3, 0, gpio, pll0_sysclk4, 0),
+	LPSC(5, 0, emac, pll0_sysclk4, 0),
+	LPSC(6, 0, emif3, pll0_sysclk5, LPSC_ALWAYS_ENABLED),
+	LPSC(7, 0, mcasp0, async3, 0),
+	LPSC(8, 0, sata, pll0_sysclk2, LPSC_FORCE),
+	LPSC(9, 0, vpif, pll0_sysclk2, 0),
+	LPSC(10, 0, spi1, async3, 0),
+	LPSC(11, 0, i2c1, pll0_sysclk4, 0),
+	LPSC(12, 0, uart1, async3, 0),
+	LPSC(13, 0, uart2, async3, 0),
+	LPSC(14, 0, mcbsp0, async3, 0),
+	LPSC(15, 0, mcbsp1, async3, 0),
+	LPSC(16, 0, lcdc, pll0_sysclk2, 0),
+	LPSC(17, 0, ehrpwm, async3, 0),
+	LPSC(18, 0, mmcsd1, pll0_sysclk2, 0),
+	LPSC(20, 0, ecap, async3, 0),
+	LPSC(21, 0, tptc2, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
+	{ }
+};
+
+void __init da850_psc_clk_init(void __iomem *psc0, void __iomem *psc1)
+{
+	struct clk_onecell_data *clk_data;
+
+	clk_data = davinci_psc_register_clocks(psc0, da850_psc0_info, 16);
+	if (!clk_data)
+		return;
+
+	clk_register_clkdev(clk_data->clks[3], NULL, "ti-aemif");
+	clk_register_clkdev(clk_data->clks[3], "aemif", "davinci-nand.0");
+	clk_register_clkdev(clk_data->clks[4], NULL, "spi_davinci.0");
+	clk_register_clkdev(clk_data->clks[5], NULL, "da830-mmc.0");
+	clk_register_clkdev(clk_data->clks[9], NULL, "serial8250.0");
+	clk_register_clkdev(clk_data->clks[14], "arm", NULL);
+	clk_register_clkdev(clk_data->clks[15], NULL, "davinci-rproc.0");
+
+	clk_free_onecell_data(clk_data);
+
+	clk_data = davinci_psc_register_clocks(psc1, da850_psc1_info, 32);
+	if (!clk_data)
+		return;
+
+	clk_register_clkdev(clk_data->clks[1], "usb20_psc_clk", NULL);
+	clk_register_clkdev(clk_data->clks[1], NULL, "musb-da8xx");
+	clk_register_clkdev(clk_data->clks[1], NULL, "cppi41-dmaengine");
+	clk_register_clkdev(clk_data->clks[2], NULL, "ohci-da8xx");
+	clk_register_clkdev(clk_data->clks[3], "gpio", NULL);
+	clk_register_clkdev(clk_data->clks[5], NULL, "davinci_emac.1");
+	clk_register_clkdev(clk_data->clks[5], "fck", "davinci_mdio.0");
+	clk_register_clkdev(clk_data->clks[7], NULL, "davinci-mcasp.0");
+	clk_register_clkdev(clk_data->clks[8], "fck", "ahci_da850");
+	clk_register_clkdev(clk_data->clks[9], NULL, "vpif");
+	clk_register_clkdev(clk_data->clks[10], NULL, "spi_davinci.1");
+	clk_register_clkdev(clk_data->clks[11], NULL, "i2c_davinci.2");
+	clk_register_clkdev(clk_data->clks[12], NULL, "serial8250.1");
+	clk_register_clkdev(clk_data->clks[13], NULL, "serial8250.2");
+	clk_register_clkdev(clk_data->clks[14], NULL, "davinci-mcbsp.0");
+	clk_register_clkdev(clk_data->clks[15], NULL, "davinci-mcbsp.1");
+	clk_register_clkdev(clk_data->clks[16], "fck", "da8xx_lcdc.0");
+	clk_register_clkdev(clk_data->clks[17], "fck", "ehrpwm.0");
+	clk_register_clkdev(clk_data->clks[17], "fck", "ehrpwm.1");
+	clk_register_clkdev(clk_data->clks[18], NULL, "da830-mmc.1");
+	clk_register_clkdev(clk_data->clks[20], "fck", "ecap.0");
+	clk_register_clkdev(clk_data->clks[20], "fck", "ecap.1");
+	clk_register_clkdev(clk_data->clks[20], "fck", "ecap.2");
+
+	clk_free_onecell_data(clk_data);
+}
+
+#ifdef CONFIG_OF
+static void __init of_da850_psc0_clk_init(struct device_node *node)
+{
+	of_davinci_psc_clk_init(node, da850_psc0_info, 16);
+}
+CLK_OF_DECLARE(da850_psc0_clk, "ti,da850-psc0", of_da850_psc0_clk_init);
+
+static void __init of_da850_psc1_clk_init(struct device_node *node)
+{
+	of_davinci_psc_clk_init(node, da850_psc1_info, 32);
+}
+CLK_OF_DECLARE(da850_psc1_clk, "ti,da850-psc1", of_da850_psc1_clk_init);
+#endif
diff --git a/include/linux/clk/davinci.h b/include/linux/clk/davinci.h
index 3ec8100..3d8bdfa 100644
--- a/include/linux/clk/davinci.h
+++ b/include/linux/clk/davinci.h
@@ -17,5 +17,6 @@  void dm644x_pll_clk_init(void __iomem *pll1, void __iomem *pll2);
 void dm646x_pll_clk_init(void __iomem *pll1, void __iomem *pll2);
 
 void da830_psc_clk_init(void __iomem *psc0, void __iomem *psc1);
+void da850_psc_clk_init(void __iomem *psc0, void __iomem *psc1);
 
 #endif /* __LINUX_CLK_DAVINCI_H__ */