Re: [PATCH v7 00/42] ARM: davinci: convert to common clock frameworkâ

From: Bartosz Golaszewski
Date: Tue Feb 20 2018 - 08:33:21 EST

Next message: jacopo mondi: "Re: [PATCH 11/15] gpio: rcar: Add R-Car M3-N compatible string"
Previous message: Corey Minyard: "Re: [PATCH ipmi/kcs_bmc v2] ipmi: kcs_bmc: make the code be more clean"
In reply to: David Lechner: "[PATCH v7 42/42] ARM: dts: da850: Add clocks"
Next in thread: David Lechner: "Re: [PATCH v7 00/42] ARM: davinci: convert to common clock frameworkâ"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

2018-02-19 21:21 GMT+01:00 David Lechner <david@xxxxxxxxxxxxxx>:
> This series converts mach-davinci to use the common clock framework.
>
> The series works like this, the first 19 patches create new clock drivers
> using the common clock framework. There are basically 3 groups of clocks -
> PLL, PSC and CFGCHIP (syscon). There are six different SoCs that each have
> unique init data, which is the reason for so many patches.
>
> Then, starting with "ARM: davinci: pass clock as parameter to
> davinci_timer_init()", we get the mach code ready for the switch by adding the
> code needed for the new clock drivers and adding #ifndef CONFIG_COMMON_CLK
> around the legacy clocks so that we can switch easily between the old and the
> new.
>
> "ARM: davinci: switch to common clock framework" actually flips the switch
> to start using the new clock drivers. Then the next 8 patches remove all
> of the old clock code.
>
> The final three patches add device tree clock support to the one SoC that
> supports it.
>
> ---
>
> The change to make all of the clocks platform devices in v7 was a pretty
> major change, so unfortunately I had to drop quite a few Reviewed-bys and
> this series will need more close review and testing again.
>
> v7 changes (also see individual patches for details):
> - Rebased on linux-davinci/master (v4.16-rc)
> - Convert clock drivers to platform devices
> - New patch "ARM: davinci: pass clock as parameter to davinci_timer_init()"
> - Fix issues with lcdk and aemif clock lookups and power domains
> - Fixed other minor issues brought up in v6 review
>
> v6 changes (also see individual patches for details):
> - All of the device tree bindings are changed
> - All of the clock drivers are changed significantly
> - Fixed issues brought up during review of v5
> - "ARM: davinci: move davinci_clk_init() to init_time" is removed from this
> series and submitted separately
>
> v5 changes:
> - Basically, this is an entirely new series
> - Patches are broken up into bite-sized pieces
> - Converted PSC clock driver to use regmap
> - Restored "force" flag for certain DA850 clocks
> - Added device tree bindings
> - Moved more of the clock init to drivers/clk
> - Fixed frequency scaling (maybe*)
>
> * I have frequency scaling using cpufreq-dt, so I know the clocks are doing
> what they need to do to make this work, but I haven't figured out how to
> test davinci-cpufreq driver yet. (Patches to make cpufreq-dt work will be
> sent separately after this series has landed.)
>
> Dependencies:
>
> Most of the dependencies have landed in mainline already in v4.16-rc1 or
> are in linux-davinci/master.
>
> There are no build dependencies any more, but you will need to pickup a
> couple of patches to get CPU frequency scaling working because of a divider
> clock bug.
>
> - https://patchwork.kernel.org/patch/10218941/
> - https://patchwork.kernel.org/patch/10218927/
>
> You can find a working branch with everything included in the "common-clk-v7"
> branch of https://github.com/dlech/ev3dev-kernel.git.
>
>
> Testing/debugging for the uninitiated:
>
> I only have one device to test with, which is based on da850, so I will
> have to rely on others to do some testing here. Since we are dealing with
> clocks, if something isn't working, you most likely won't see output on
> the serial port. To figure out what is going on, you need to enable...
>
> CONFIG_DEBUG_LL=y
> CONFIG_EARLY_PRINTK=y
>
> and add "earlyprintk clk_ignore_unused" to the kernel command line options.
> You may need to select a different UART for this depending on your board. I
> think UART1 is the default in the kernel configuration.
>
> On da850 devices comment out the lines:
>
> else
> clk_set_parent(clk, parent->clk);
>
> in da850.c or, if using device tree, comment out the lines:
>
> assigned-clocks = <&async3_clk>;
> assigned-clock-parents = <&pll1_sysclk 2>;
>
> in da850.dtsi when doing earlyprintk, otherwise the UART1 and UART2 clock
> source will change during boot and cause garbled output after a point.
>
> David Lechner (42):
> dt-bindings: clock: Add new bindings for TI Davinci PLL clocks
> clk: davinci: New driver for davinci PLL clocks
> clk: davinci: Add platform information for TI DA830 PLL
> clk: davinci: Add platform information for TI DA850 PLL
> clk: davinci: Add platform information for TI DM355 PLL
> clk: davinci: Add platform information for TI DM365 PLL
> clk: davinci: Add platform information for TI DM644x PLL
> clk: davinci: Add platform information for TI DM646x PLL
> dt-bindings: clock: New bindings for TI Davinci PSC
> clk: davinci: New driver for davinci PSC clocks
> clk: davinci: Add platform information for TI DA830 PSC
> clk: davinci: Add platform information for TI DA850 PSC
> clk: davinci: Add platform information for TI DM355 PSC
> clk: davinci: Add platform information for TI DM365 PSC
> clk: davinci: Add platform information for TI DM644x PSC
> clk: davinci: Add platform information for TI DM646x PSC
> dt-bindings: clock: Add bindings for DA8XX CFGCHIP clocks
> clk: davinci: New driver for TI DA8XX CFGCHIP clocks
> clk: davinci: cfgchip: Add TI DA8XX USB PHY clocks
> ARM: davinci: pass clock as parameter to davinci_timer_init()
> ARM: davinci: da830: add new clock init using common clock framework
> ARM: davinci: da850: add new clock init using common clock framework
> ARM: davinci: dm355: add new clock init using common clock framework
> ARM: davinci: dm365: add new clock init using common clock framework
> ARM: davinci: dm644x: add new clock init using common clock framework
> ARM: davinci: dm646x: add new clock init using common clock framework
> ARM: davinci: da8xx: add new USB PHY clock init using common clock
> framework
> ARM: davinci: da8xx: add new sata_refclk init using common clock
> framework
> ARM: davinci: remove CONFIG_DAVINCI_RESET_CLOCKS
> ARM: davinci_all_defconfig: remove CONFIG_DAVINCI_RESET_CLOCKS
> ARM: davinci: switch to common clock framework
> ARM: davinci: da830: Remove legacy clock init
> ARM: davinci: da850: Remove legacy clock init
> ARM: davinci: dm355: Remove legacy clock init
> ARM: davinci: dm365: Remove legacy clock init
> ARM: davinci: dm644x: Remove legacy clock init
> ARM: davinci: dm646x: Remove legacy clock init
> ARM: davinci: da8xx: Remove legacy USB and SATA clock init
> ARM: davinci: remove legacy clocks
> ARM: davinci: add device tree support to timer
> ARM: davinci: da8xx-dt: switch to device tree clocks
> ARM: dts: da850: Add clocks
>
> .../bindings/clock/ti/davinci/da8xx-cfgchip.txt | 93 +++
> .../devicetree/bindings/clock/ti/davinci/pll.txt | 96 +++
> .../devicetree/bindings/clock/ti/davinci/psc.txt | 71 ++
> MAINTAINERS | 7 +
> arch/arm/Kconfig | 5 +-
> arch/arm/boot/dts/da850-enbw-cmc.dts | 4 +
> arch/arm/boot/dts/da850-evm.dts | 4 +
> arch/arm/boot/dts/da850-lcdk.dts | 9 +
> arch/arm/boot/dts/da850-lego-ev3.dts | 4 +
> arch/arm/boot/dts/da850.dtsi | 159 ++++
> arch/arm/configs/davinci_all_defconfig | 1 -
> arch/arm/mach-davinci/Kconfig | 13 +-
> arch/arm/mach-davinci/Makefile | 4 +-
> arch/arm/mach-davinci/board-da830-evm.c | 12 +-
> arch/arm/mach-davinci/board-da850-evm.c | 2 +
> arch/arm/mach-davinci/board-dm355-evm.c | 2 +
> arch/arm/mach-davinci/board-dm355-leopard.c | 2 +
> arch/arm/mach-davinci/board-dm365-evm.c | 2 +
> arch/arm/mach-davinci/board-dm644x-evm.c | 2 +
> arch/arm/mach-davinci/board-dm646x-evm.c | 2 +
> arch/arm/mach-davinci/board-mityomapl138.c | 2 +
> arch/arm/mach-davinci/board-neuros-osd2.c | 2 +
> arch/arm/mach-davinci/board-omapl138-hawk.c | 11 +-
> arch/arm/mach-davinci/board-sffsdr.c | 2 +
> arch/arm/mach-davinci/clock.c | 745 -----------------
> arch/arm/mach-davinci/clock.h | 76 --
> arch/arm/mach-davinci/common.c | 3 -
> arch/arm/mach-davinci/da830.c | 469 ++---------
> arch/arm/mach-davinci/da850.c | 766 +++---------------
> arch/arm/mach-davinci/da8xx-dt.c | 60 --
> arch/arm/mach-davinci/davinci.h | 8 +
> arch/arm/mach-davinci/devices-da8xx.c | 43 +-
> arch/arm/mach-davinci/devices.c | 1 -
> arch/arm/mach-davinci/dm355.c | 425 ++--------
> arch/arm/mach-davinci/dm365.c | 517 ++----------
> arch/arm/mach-davinci/dm644x.c | 363 ++-------
> arch/arm/mach-davinci/dm646x.c | 391 ++-------
> arch/arm/mach-davinci/include/mach/clock.h | 3 -
> arch/arm/mach-davinci/include/mach/common.h | 11 +-
> arch/arm/mach-davinci/include/mach/da8xx.h | 6 +-
> arch/arm/mach-davinci/pm_domain.c | 5 +
> arch/arm/mach-davinci/psc.c | 137 ----
> arch/arm/mach-davinci/psc.h | 12 -
> arch/arm/mach-davinci/time.c | 46 +-
> arch/arm/mach-davinci/usb-da8xx.c | 250 +-----
> drivers/clk/Makefile | 1 +
> drivers/clk/davinci/Makefile | 21 +
> drivers/clk/davinci/da8xx-cfgchip.c | 786 ++++++++++++++++++
> drivers/clk/davinci/pll-da830.c | 58 ++
> drivers/clk/davinci/pll-da850.c | 178 +++++
> drivers/clk/davinci/pll-dm355.c | 75 ++
> drivers/clk/davinci/pll-dm365.c | 119 +++
> drivers/clk/davinci/pll-dm644x.c | 76 ++
> drivers/clk/davinci/pll-dm646x.c | 78 ++
> drivers/clk/davinci/pll.c | 877 +++++++++++++++++++++
> drivers/clk/davinci/pll.h | 141 ++++
> drivers/clk/davinci/psc-da830.c | 89 +++
> drivers/clk/davinci/psc-da850.c | 109 +++
> drivers/clk/davinci/psc-dm355.c | 72 ++
> drivers/clk/davinci/psc-dm365.c | 77 ++
> drivers/clk/davinci/psc-dm644x.c | 67 ++
> drivers/clk/davinci/psc-dm646x.c | 62 ++
> drivers/clk/davinci/psc.c | 505 ++++++++++++
> drivers/clk/davinci/psc.h | 101 +++
> include/linux/platform_data/clk-da8xx-cfgchip.h | 21 +
> 65 files changed, 4515 insertions(+), 3846 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/clock/ti/davinci/da8xx-cfgchip.txt
> create mode 100644 Documentation/devicetree/bindings/clock/ti/davinci/pll.txt
> create mode 100644 Documentation/devicetree/bindings/clock/ti/davinci/psc.txt
> delete mode 100644 arch/arm/mach-davinci/clock.c
> delete mode 100644 arch/arm/mach-davinci/psc.c
> create mode 100644 drivers/clk/davinci/Makefile
> create mode 100644 drivers/clk/davinci/da8xx-cfgchip.c
> create mode 100644 drivers/clk/davinci/pll-da830.c
> create mode 100644 drivers/clk/davinci/pll-da850.c
> create mode 100644 drivers/clk/davinci/pll-dm355.c
> create mode 100644 drivers/clk/davinci/pll-dm365.c
> create mode 100644 drivers/clk/davinci/pll-dm644x.c
> create mode 100644 drivers/clk/davinci/pll-dm646x.c
> create mode 100644 drivers/clk/davinci/pll.c
> create mode 100644 drivers/clk/davinci/pll.h
> create mode 100644 drivers/clk/davinci/psc-da830.c
> create mode 100644 drivers/clk/davinci/psc-da850.c
> create mode 100644 drivers/clk/davinci/psc-dm355.c
> create mode 100644 drivers/clk/davinci/psc-dm365.c
> create mode 100644 drivers/clk/davinci/psc-dm644x.c
> create mode 100644 drivers/clk/davinci/psc-dm646x.c
> create mode 100644 drivers/clk/davinci/psc.c
> create mode 100644 drivers/clk/davinci/psc.h
> create mode 100644 include/linux/platform_data/clk-da8xx-cfgchip.h
>
> --
> 2.7.4
>

Hi David,

just some quick results from today's playing with v7.

I started out with da850-lcdk with my standard rootfs over NFS. I was
not able to boot to console so far. The first problem is that mdio
fails to probe:

libphy: Fixed MDIO Bus: probed
davinci_mdio 1e24000.mdio: davinci mdio revision 1.5, bus freq 2200000
davinci_mdio 1e24000.mdio: no live phy, scanning all
davinci_mdio: probe of 1e24000.mdio failed with error -5

After some digging I noticed that the supplied clock rate differs
between mainline (114000000Hz) vs common-clock-v7 (18000000). Since
we're not setting the rate in mdio, using LPSC_SET_RATE_PARENT would
not help like with lcdc.

After that, the boot just hangs without ever getting to emac's probe.

Once I set the emac clock to always enabled (a workaround that was
necessary with v6, but could be dropped with my first
genpd-in-a-separate-driver attempt), I'm getting a rather strange NULL
pointer dereference:

Backtrace:
[<c049d464>] (strlen) from [<c01f32a8>] (start_creating+0x58/0xc0)
[<c01f3250>] (start_creating) from [<c01f34c8>] (debugfs_create_dir+0x14/0xd8)
r7:c04dadbc r6:c0567480 r5:c0656274 r4:c68a9300
[<c01f34b4>] (debugfs_create_dir) from [<c0296838>]
(clk_debug_create_one+0x28/0x1d0)
r5:c0656274 r4:c68a9300
[<c0296810>] (clk_debug_create_one) from [<c05cdf84>]
(clk_debug_init+0x100/0x15c)
r5:c0656274 r4:c68a9300
[<c05cde84>] (clk_debug_init) from [<c000a54c>] (do_one_initcall+0x50/0x19c)
r7:c05e3834 r6:ffffe000 r5:c05f7008 r4:c05cde84
[<c000a4fc>] (do_one_initcall) from [<c05b6eb4>]
(kernel_init_freeable+0x110/0x1cc)
r9:c05b4584 r8:c05b6614 r7:c05e3834 r6:c063cc80 r5:00000073 r4:c05f2354
[<c05b6da4>] (kernel_init_freeable) from [<c04a2ce4>] (kernel_init+0x10/0xfc)
r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c04a2cd4
r4:00000000
[<c04a2cd4>] (kernel_init) from [<c00090e0>] (ret_from_fork+0x14/0x34)

It turns out that the emac clock is correctly registered in
__davinci_psc_register_clocks() and a corresponding clk_core structure
is added to the list and core->name is correctly assigned, but then
later when clk_debug_init() is called, the name in emac clk_core is
NULL. This is the direct reason for the panic. Interestingly other
members of clk_core seem to be fine.

I'll continue on debugging it. Let me know if you have any ideas.

Best regards,
Bartosz Golaszewski

Next message: jacopo mondi: "Re: [PATCH 11/15] gpio: rcar: Add R-Car M3-N compatible string"
Previous message: Corey Minyard: "Re: [PATCH ipmi/kcs_bmc v2] ipmi: kcs_bmc: make the code be more clean"
In reply to: David Lechner: "[PATCH v7 42/42] ARM: dts: da850: Add clocks"
Next in thread: David Lechner: "Re: [PATCH v7 00/42] ARM: davinci: convert to common clock frameworkâ"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]