| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9f534bee-9d27-f878-f0b8-58d8d9ece9fa@lechnology.com>
Date: Sun, 11 Feb 2018 21:03:40 -0600
From: David Lechner <david@...hnology.com>
To: Michael Turquette <mturquette@...libre.com>,
Bartosz Golaszewski <brgl@...ev.pl>
Cc: linux-clk <linux-clk@...r.kernel.org>,
devicetree <devicetree@...r.kernel.org>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
Stephen Boyd <sboyd@...eaurora.org>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Sekhar Nori <nsekhar@...com>,
Kevin Hilman <khilman@...nel.org>,
Adam Ford <aford173@...il.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 12/44] clk: davinci: Add platform information for TI
DA850 PSC
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@...ev.pl> wrote:
>> 2018-01-08 3:17 GMT+01:00 David Lechner <david@...hnology.com>:
>>> This adds platform-specific declarations for the PSC clocks on TI DA850/
>>> OMAP-L138/AM18XX SoCs.
>>>
>>> Signed-off-by: David Lechner <david@...hnology.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@...hnology.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
Powered by blists - more mailing lists