mbox series

[v10,00/27] ARM: davinci: convert to common clock framework​

Message ID 20180509172606.29387-1-david@lechnology.com
Headers show
Series ARM: davinci: convert to common clock framework​ | expand

Message

David Lechner May 9, 2018, 5:25 p.m. UTC
This series converts mach-davinci to use the common clock framework.

The series works like this, the first 3 patches fix some issues with the clock
drivers that have already been accepted into the mainline kernel.

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 four patches add device tree clock support to the one SoC that
supports it.

This series has been tested on TI OMAP-L138 LCDK (both device tree and legacy
board file).


Changes:

v10 changes (also see individual patches for details):
- Reworked device tree bindings for DaVinci timer.
- Dropped helper functions to conditionally call devm_* versions of functions
- Fix some typos
- Fix some rebasing issues introduced in v9

v9 changes (also see individual patches for details):
- Rebased on linux-davnci/master (f5e3203bb775)
- Dropped drivers/clk patches that landed in v4.17
- New drivers/clk patches for early boot special case
- New patch for ti,davinci-timer device tree bindings
- Updated mach/davinci patches to register clocks in early boot when needed

v8 changes (also see individual patches for details):
- Rebased on linux-davinci/master
- Dropped use of __init and __initconst attributes in clk drivers
- Add clkdev lookups for PLL SYSCLKs
- Fix genpd clock reference counting issue
- Fix PSC clock driver loading order issue
- Fix typo in device tree and add more power-domains properties

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:

There are still some outstanding fixes to get everything working correctly.
These are all just runtime dependencies and only needed for certain platforms.

- "drm/tilcdc: Fix setting clock divider for omap-l138"[1]
- "clk: davinci: pll-dm355: fix SYSCLKn parent names"[2]
- "remoteproc/davinci: common clock framework related fixes"[3]

[1]: https://patchwork.freedesktop.org/patch/210696/
[2]: https://lkml.org/lkml/2018/5/9/626
[3]: https://lkml.org/lkml/2018/5/2/201

You can find a working branch with everything included (plus a few extras, like
cpufreq-dt) in the "common-clk-v10" 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:

	/* pll1_sysclk2 is not affected by CPU scaling, so use it for async3 */
	parent = clk_hw_get_parent_by_index(&mux->hw, 1);
	if (parent)
		clk_set_parent(mux->hw.clk, parent->clk);
	else
		dev_warn(dev, "Failed to find async3 parent clock\n");

in da8xx-cfgchip.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 (27):
  clk: davinci: pll: allow dev == NULL
  clk: davinci: da850-pll: change PLL0 to CLK_OF_DECLARE
  clk: davinci: psc: allow for dev == NULL
  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
  dt-bindings: timer: new bindings for TI DaVinci timer
  ARM: davinci: add device tree support to timer
  ARM: davinci: da8xx-dt: switch to device tree clocks
  ARM: dts: da850: Add clocks

 .../bindings/timer/ti,davinci-timer.txt       |  37 +
 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                  | 168 ++++
 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                 | 462 ++---------
 arch/arm/mach-davinci/da850.c                 | 778 +++---------------
 arch/arm/mach-davinci/da8xx-dt.c              |  66 --
 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                 | 406 ++-------
 arch/arm/mach-davinci/dm365.c                 | 485 +----------
 arch/arm/mach-davinci/dm644x.c                | 344 +-------
 arch/arm/mach-davinci/dm646x.c                | 372 +--------
 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                  |  39 +-
 arch/arm/mach-davinci/usb-da8xx.c             | 242 +-----
 drivers/clk/davinci/pll-da830.c               |   4 +-
 drivers/clk/davinci/pll-da850.c               |  36 +-
 drivers/clk/davinci/pll-dm355.c               |   8 +-
 drivers/clk/davinci/pll-dm365.c               |   8 +-
 drivers/clk/davinci/pll-dm644x.c              |   8 +-
 drivers/clk/davinci/pll-dm646x.c              |   8 +-
 drivers/clk/davinci/pll.c                     | 110 +--
 drivers/clk/davinci/pll.h                     |  30 +-
 drivers/clk/davinci/psc-dm355.c               |   2 +-
 drivers/clk/davinci/psc-dm365.c               |   2 +-
 drivers/clk/davinci/psc-dm644x.c              |   2 +-
 drivers/clk/davinci/psc-dm646x.c              |   2 +-
 drivers/clk/davinci/psc.c                     |  27 +-
 include/linux/clk/davinci.h                   |  29 +
 56 files changed, 860 insertions(+), 3950 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/timer/ti,davinci-timer.txt
 delete mode 100644 arch/arm/mach-davinci/clock.c
 delete mode 100644 arch/arm/mach-davinci/psc.c
 create mode 100644 include/linux/clk/davinci.h

Comments

Sekhar Nori May 11, 2018, 3:26 p.m. UTC | #1
Hi David,

On Wednesday 09 May 2018 10:55 PM, David Lechner wrote:
> This series converts mach-davinci to use the common clock framework.
> 
> The series works like this, the first 3 patches fix some issues with the clock
> drivers that have already been accepted into the mainline kernel.

I have not yet looked at the patches, but I got a bunch of W=1 warnings 
and some sparse warnings when building your branch. Please take a look 
at these. Unfortunately the output is mixed between sparse and compiler.
The "expression using sizeof(void)" can be ignored as its a known issue
with sparse, I believe.

Thanks,
Sekhar

drivers/clk/davinci/pll-da830.c:39:5: warning: symbol 'da830_pll_init' was not declared. Should it be static?
drivers/clk/davinci/pll.c:142:16: warning: expression using sizeof(void)
drivers/clk/davinci/pll.c:142:16: warning: expression using sizeof(void)
drivers/clk/davinci/pll-da850.c:87:5: warning: symbol 'da850_pll0_init' was not declared. Should it be static?
drivers/clk/davinci/pll-da830.c:39:5: warning: no previous prototype for ‘da830_pll_init’ [-Wmissing-prototypes]
 int da830_pll_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
     ^~~~~~~~~~~~~~
drivers/clk/davinci/pll-da850.c:87:5: warning: no previous prototype for ‘da850_pll0_init’ [-Wmissing-prototypes]
 int da850_pll0_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
     ^~~~~~~~~~~~~~~
drivers/clk/davinci/pll-dm355.c:30:5: warning: symbol 'dm355_pll1_init' was not declared. Should it be static?
drivers/clk/davinci/pll-dm365.c:59:5: warning: symbol 'dm365_pll1_init' was not declared. Should it be static?
drivers/clk/davinci/pll-dm365.c:122:5: warning: symbol 'dm365_pll2_init' was not declared. Should it be static?
drivers/clk/davinci/da8xx-cfgchip.c:581: warning: Function parameter or member 'dev' not described in 'da8xx_cfgchip_register_usb1_clk48'
drivers/clk/davinci/pll-dm646x.c:32:5: warning: symbol 'dm646x_pll1_init' was not declared. Should it be static?
drivers/clk/davinci/pll-dm644x.c:30:5: warning: symbol 'dm644x_pll1_init' was not declared. Should it be static?
drivers/clk/davinci/pll-dm365.c:59:5: warning: no previous prototype for ‘dm365_pll1_init’ [-Wmissing-prototypes]
 int dm365_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
     ^~~~~~~~~~~~~~~
drivers/clk/davinci/pll-dm365.c:122:5: warning: no previous prototype for ‘dm365_pll2_init’ [-Wmissing-prototypes]
 int dm365_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
     ^~~~~~~~~~~~~~~
drivers/clk/davinci/pll-dm355.c:30:5: warning: no previous prototype for ‘dm355_pll1_init’ [-Wmissing-prototypes]
 int dm355_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
     ^~~~~~~~~~~~~~~
drivers/clk/davinci/psc.c:310:5: warning: symbol 'davinci_clk_reset_assert' was not declared. Should it be static?
drivers/clk/davinci/psc.c:316:5: warning: symbol 'davinci_clk_reset_deassert' was not declared. Should it be static?
drivers/clk/davinci/psc-dm644x.c:66:5: warning: symbol 'dm644x_psc_init' was not declared. Should it be static?
drivers/clk/davinci/psc-dm355.c:71:5: warning: symbol 'dm355_psc_init' was not declared. Should it be static?
drivers/clk/davinci/pll-dm646x.c:32:5: warning: no previous prototype for ‘dm646x_pll1_init’ [-Wmissing-prototypes]
 int dm646x_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
     ^~~~~~~~~~~~~~~~
drivers/clk/davinci/psc-dm365.c:76:5: warning: symbol 'dm365_psc_init' was not declared. Should it be static?
drivers/clk/davinci/psc-dm646x.c:61:5: warning: symbol 'dm646x_psc_init' was not declared. Should it be static?
drivers/clk/davinci/pll-dm644x.c:30:5: warning: no previous prototype for ‘dm644x_pll1_init’ [-Wmissing-prototypes]
 int dm644x_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
     ^~~~~~~~~~~~~~~~
drivers/clk/davinci/psc-dm355.c:71:5: warning: no previous prototype for ‘dm355_psc_init’ [-Wmissing-prototypes]
 int dm355_psc_init(struct device *dev, void __iomem *base)
     ^~~~~~~~~~~~~~
In file included from drivers/clk/davinci/psc-dm355.c:15:0:
drivers/clk/davinci/psc-dm355.c:26:14: warning: ‘mcbsp0_clkdev’ defined but not used [-Wunused-const-variable=]
 LPSC_CLKDEV1(mcbsp0_clkdev,  NULL,  "davinci-mcbsp.0");
              ^
drivers/clk/davinci/psc.h:31:46: note: in definition of macro ‘LPSC_CLKDEV1’
 static const struct davinci_lpsc_clkdev_info n[] __initconst = { \
                                              ^
drivers/clk/davinci/psc-dm355.c:21:14: warning: ‘mcbsp1_clkdev’ defined but not used [-Wunused-const-variable=]
 LPSC_CLKDEV1(mcbsp1_clkdev,  NULL,  "davinci-mcbsp.1");
              ^
drivers/clk/davinci/psc.h:31:46: note: in definition of macro ‘LPSC_CLKDEV1’
 static const struct davinci_lpsc_clkdev_info n[] __initconst = { \
                                              ^
drivers/clk/davinci/psc-dm365.c:76:5: warning: no previous prototype for ‘dm365_psc_init’ [-Wmissing-prototypes]
 int dm365_psc_init(struct device *dev, void __iomem *base)
     ^~~~~~~~~~~~~~
drivers/clk/davinci/psc-dm646x.c:61:5: warning: no previous prototype for ‘dm646x_psc_init’ [-Wmissing-prototypes]
 int dm646x_psc_init(struct device *dev, void __iomem *base)
     ^~~~~~~~~~~~~~~
drivers/clk/davinci/psc-dm644x.c:66:5: warning: no previous prototype for ‘dm644x_psc_init’ [-Wmissing-prototypes]
 int dm644x_psc_init(struct device *dev, void __iomem *base)
     ^~~~~~~~~~~~~~~
drivers/clk/davinci/pll.c:496: warning: Function parameter or member 'dev' not described in 'davinci_pll_auxclk_register'
drivers/clk/davinci/psc.c:310:5: warning: no previous prototype for ‘davinci_clk_reset_assert’ [-Wmissing-prototypes]
 int davinci_clk_reset_assert(struct clk *clk)
     ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/clk/davinci/psc.c:316:5: warning: no previous prototype for ‘davinci_clk_reset_deassert’ [-Wmissing-prototypes]
 int davinci_clk_reset_deassert(struct clk *clk)
     ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/clk/davinci/pll.c:509: warning: Function parameter or member 'dev' not described in 'davinci_pll_sysclkbp_clk_register'
drivers/clk/davinci/pll.c:524: warning: Function parameter or member 'dev' not described in 'davinci_pll_obsclk_register'
drivers/clk/davinci/pll.c:605: warning: Function parameter or member 'dev' not described in 'davinci_pll_sysclk_register'

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Lechner May 12, 2018, 9:11 p.m. UTC | #2
On 05/11/2018 10:26 AM, Sekhar Nori wrote:
> Hi David,
> 
> On Wednesday 09 May 2018 10:55 PM, David Lechner wrote:
>> This series converts mach-davinci to use the common clock framework.
>>
>> The series works like this, the first 3 patches fix some issues with the clock
>> drivers that have already been accepted into the mainline kernel.
> 
> I have not yet looked at the patches, but I got a bunch of W=1 warnings
> and some sparse warnings when building your branch. Please take a look
> at these. Unfortunately the output is mixed between sparse and compiler.
> The "expression using sizeof(void)" can be ignored as its a known issue
> with sparse, I believe.
> 

I've started a common-clk-v11 branch on my GitHub that fixes most of these.
Also submitted "clk: davinci: psc-dm355: fix ASP0/1 clkdev lookups" that
fixes a couple more. I've purposely not fixed the davinci_clk_reset_* functions
since there is already a patch that will remove those functions in the future.

I'll wait a bit longer for DT review before re-sending v11 of this series.



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adam Ford May 14, 2018, 12:40 a.m. UTC | #3
On Wed, May 9, 2018 at 12:25 PM, David Lechner <david@lechnology.com> wrote:
> This series converts mach-davinci to use the common clock framework.
>
> The series works like this, the first 3 patches fix some issues with the clock
> drivers that have already been accepted into the mainline kernel.
>
> 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 four patches add device tree clock support to the one SoC that
> supports it.
>
> This series has been tested on TI OMAP-L138 LCDK (both device tree and legacy
> board file).
>

I am not sure if I did something wrong, but I attempted to build and I
wasn't able to boot the da850-evm.dtb your repo common-clk-v11,
however the legacy board file boot was OK.

make davinci_all_defconfig ARCH=arm
make zImage modules da850-evm.dtb ARCH=arm CROSS_COMPILE=arm-linux- -j8

3140416 bytes read in 1464 ms (2 MiB/s)
20353 bytes read in 15 ms (1.3 MiB/s)
## Flattened Device Tree blob at c0600000
   Booting using the fdt blob at 0xc0600000
   Loading Device Tree to c7e57000, end c7e5ef80 ... OK

Starting kernel ...

Uncompressing Linux... done, booting the kernel.

(and hang)

If you have some suggestions, I am try them as I get time.

adam

>
> Changes:
>
> v10 changes (also see individual patches for details):
> - Reworked device tree bindings for DaVinci timer.
> - Dropped helper functions to conditionally call devm_* versions of functions
> - Fix some typos
> - Fix some rebasing issues introduced in v9
>
> v9 changes (also see individual patches for details):
> - Rebased on linux-davnci/master (f5e3203bb775)
> - Dropped drivers/clk patches that landed in v4.17
> - New drivers/clk patches for early boot special case
> - New patch for ti,davinci-timer device tree bindings
> - Updated mach/davinci patches to register clocks in early boot when needed
>
> v8 changes (also see individual patches for details):
> - Rebased on linux-davinci/master
> - Dropped use of __init and __initconst attributes in clk drivers
> - Add clkdev lookups for PLL SYSCLKs
> - Fix genpd clock reference counting issue
> - Fix PSC clock driver loading order issue
> - Fix typo in device tree and add more power-domains properties
>
> 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:
>
> There are still some outstanding fixes to get everything working correctly.
> These are all just runtime dependencies and only needed for certain platforms.
>
> - "drm/tilcdc: Fix setting clock divider for omap-l138"[1]
> - "clk: davinci: pll-dm355: fix SYSCLKn parent names"[2]
> - "remoteproc/davinci: common clock framework related fixes"[3]
>
> [1]: https://patchwork.freedesktop.org/patch/210696/
> [2]: https://lkml.org/lkml/2018/5/9/626
> [3]: https://lkml.org/lkml/2018/5/2/201
>
> You can find a working branch with everything included (plus a few extras, like
> cpufreq-dt) in the "common-clk-v10" 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:
>
>         /* pll1_sysclk2 is not affected by CPU scaling, so use it for async3 */
>         parent = clk_hw_get_parent_by_index(&mux->hw, 1);
>         if (parent)
>                 clk_set_parent(mux->hw.clk, parent->clk);
>         else
>                 dev_warn(dev, "Failed to find async3 parent clock\n");
>
> in da8xx-cfgchip.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 (27):
>   clk: davinci: pll: allow dev == NULL
>   clk: davinci: da850-pll: change PLL0 to CLK_OF_DECLARE
>   clk: davinci: psc: allow for dev == NULL
>   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
>   dt-bindings: timer: new bindings for TI DaVinci timer
>   ARM: davinci: add device tree support to timer
>   ARM: davinci: da8xx-dt: switch to device tree clocks
>   ARM: dts: da850: Add clocks
>
>  .../bindings/timer/ti,davinci-timer.txt       |  37 +
>  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                  | 168 ++++
>  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                 | 462 ++---------
>  arch/arm/mach-davinci/da850.c                 | 778 +++---------------
>  arch/arm/mach-davinci/da8xx-dt.c              |  66 --
>  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                 | 406 ++-------
>  arch/arm/mach-davinci/dm365.c                 | 485 +----------
>  arch/arm/mach-davinci/dm644x.c                | 344 +-------
>  arch/arm/mach-davinci/dm646x.c                | 372 +--------
>  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                  |  39 +-
>  arch/arm/mach-davinci/usb-da8xx.c             | 242 +-----
>  drivers/clk/davinci/pll-da830.c               |   4 +-
>  drivers/clk/davinci/pll-da850.c               |  36 +-
>  drivers/clk/davinci/pll-dm355.c               |   8 +-
>  drivers/clk/davinci/pll-dm365.c               |   8 +-
>  drivers/clk/davinci/pll-dm644x.c              |   8 +-
>  drivers/clk/davinci/pll-dm646x.c              |   8 +-
>  drivers/clk/davinci/pll.c                     | 110 +--
>  drivers/clk/davinci/pll.h                     |  30 +-
>  drivers/clk/davinci/psc-dm355.c               |   2 +-
>  drivers/clk/davinci/psc-dm365.c               |   2 +-
>  drivers/clk/davinci/psc-dm644x.c              |   2 +-
>  drivers/clk/davinci/psc-dm646x.c              |   2 +-
>  drivers/clk/davinci/psc.c                     |  27 +-
>  include/linux/clk/davinci.h                   |  29 +
>  56 files changed, 860 insertions(+), 3950 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/timer/ti,davinci-timer.txt
>  delete mode 100644 arch/arm/mach-davinci/clock.c
>  delete mode 100644 arch/arm/mach-davinci/psc.c
>  create mode 100644 include/linux/clk/davinci.h
>
> --
> 2.17.0
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Lechner May 14, 2018, 1:50 a.m. UTC | #4
On 05/13/2018 07:40 PM, Adam Ford wrote:
> On Wed, May 9, 2018 at 12:25 PM, David Lechner <david@lechnology.com> wrote:
>> This series converts mach-davinci to use the common clock framework.
>>
>> The series works like this, the first 3 patches fix some issues with the clock
>> drivers that have already been accepted into the mainline kernel.
>>
>> 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 four patches add device tree clock support to the one SoC that
>> supports it.
>>
>> This series has been tested on TI OMAP-L138 LCDK (both device tree and legacy
>> board file).
>>
> 
> I am not sure if I did something wrong, but I attempted to build and I
> wasn't able to boot the da850-evm.dtb your repo common-clk-v11,
> however the legacy board file boot was OK.
> 
> make davinci_all_defconfig ARCH=arm
> make zImage modules da850-evm.dtb ARCH=arm CROSS_COMPILE=arm-linux- -j8
> 
> 3140416 bytes read in 1464 ms (2 MiB/s)
> 20353 bytes read in 15 ms (1.3 MiB/s)
> ## Flattened Device Tree blob at c0600000
>     Booting using the fdt blob at 0xc0600000
>     Loading Device Tree to c7e57000, end c7e5ef80 ... OK
> 
> Starting kernel ...
> 
> Uncompressing Linux... done, booting the kernel.
> 
> (and hang)
> 
> If you have some suggestions, I am try them as I get time.
> 

As with each revision of this series, I have included debugging tips
in the cover letter, e.g. earlyprink and not disabling unused clocks.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartosz Golaszewski May 15, 2018, 9:25 a.m. UTC | #5
2018-05-14 2:40 GMT+02:00 Adam Ford <aford173@gmail.com>:
> On Wed, May 9, 2018 at 12:25 PM, David Lechner <david@lechnology.com> wrote:
>> This series converts mach-davinci to use the common clock framework.
>>
>> The series works like this, the first 3 patches fix some issues with the clock
>> drivers that have already been accepted into the mainline kernel.
>>
>> 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 four patches add device tree clock support to the one SoC that
>> supports it.
>>
>> This series has been tested on TI OMAP-L138 LCDK (both device tree and legacy
>> board file).
>>
>
> I am not sure if I did something wrong, but I attempted to build and I
> wasn't able to boot the da850-evm.dtb your repo common-clk-v11,
> however the legacy board file boot was OK.
>
> make davinci_all_defconfig ARCH=arm
> make zImage modules da850-evm.dtb ARCH=arm CROSS_COMPILE=arm-linux- -j8
>
> 3140416 bytes read in 1464 ms (2 MiB/s)
> 20353 bytes read in 15 ms (1.3 MiB/s)
> ## Flattened Device Tree blob at c0600000
>    Booting using the fdt blob at 0xc0600000
>    Loading Device Tree to c7e57000, end c7e5ef80 ... OK
>
> Starting kernel ...
>
> Uncompressing Linux... done, booting the kernel.
>
> (and hang)
>
> If you have some suggestions, I am try them as I get time.
>
> adam
>

Runs fine on da850-lcdk and dm365-evm. I'll test the da850-evm
tomorrow when I'll have access to it.

Bart
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori May 15, 2018, 1:31 p.m. UTC | #6
On Wednesday 09 May 2018 10:55 PM, David Lechner wrote:
> +void of_da850_pll0_init(struct device_node *node)
>  {
> -	return of_davinci_pll_init(dev, dev->of_node, &da850_pll0_info,
> -				   &da850_pll0_obsclk_info,
> -				   da850_pll0_sysclk_info, 7, base, cfgchip);
> +	void __iomem *base;
> +	struct regmap *cfgchip;
> +
> +	base = of_iomap(node, 0);
> +	if (!base) {
> +		pr_err("%s: ioremap failed\n", __func__);
> +		return;
> +	}
> +
> +	cfgchip = syscon_regmap_lookup_by_compatible("ti,da830-cfgchip");

It will be nice to handle the error case here.

> +
> +	of_davinci_pll_init(NULL, node, &da850_pll0_info,
> +			    &da850_pll0_obsclk_info,
> +			    da850_pll0_sysclk_info, 7, base, cfgchip);

Apart from that, it looks good to me.

Reviewed-by: Sekhar Nori <nsekhar@ti.com>

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori May 15, 2018, 1:42 p.m. UTC | #7
On Wednesday 09 May 2018 10:55 PM, David Lechner wrote:
> @@ -261,10 +263,14 @@ davinci_lpsc_clk_register(struct device *dev, const char *name,
>  	lpsc->pd = pd;
>  	lpsc->flags = flags;
>  
> -	ret = devm_clk_hw_register(dev, &lpsc->hw);
> +	ret = clk_hw_register(dev, &lpsc->hw);
>  	if (ret < 0)
>  		return ERR_PTR(ret);
>  
> +	/* for now, genpd is only registered when using device-tree */
> +	if (!dev || !dev->of_node)
> +		return lpsc;
> +
>  	/* genpd attach needs a way to look up this clock */
>  	ret = clk_hw_register_clkdev(&lpsc->hw, name, best_dev_name(dev));
>  
> @@ -378,11 +384,11 @@ __davinci_psc_register_clocks(struct device *dev,
>  	struct regmap *regmap;
>  	int i, ret;
>  
> -	psc = devm_kzalloc(dev, sizeof(*psc), GFP_KERNEL);
> +	psc = kzalloc(sizeof(*psc), GFP_KERNEL);
>  	if (!psc)
>  		return ERR_PTR(-ENOMEM);
>  
> -	clks = devm_kmalloc_array(dev, num_clks, sizeof(*clks), GFP_KERNEL);
> +	clks = kmalloc_array(num_clks, sizeof(*clks), GFP_KERNEL);
>  	if (!clks)
>  		return ERR_PTR(-ENOMEM);
>  
> @@ -396,14 +402,14 @@ __davinci_psc_register_clocks(struct device *dev,
>  	for (i = 0; i < num_clks; i++)
>  		clks[i] = ERR_PTR(-ENOENT);
>  
> -	pm_domains = devm_kcalloc(dev, num_clks, sizeof(*pm_domains), GFP_KERNEL);
> +	pm_domains = kcalloc(num_clks, sizeof(*pm_domains), GFP_KERNEL);
>  	if (!pm_domains)
>  		return ERR_PTR(-ENOMEM);
>  
>  	psc->pm_data.domains = pm_domains;
>  	psc->pm_data.num_domains = num_clks;
>  
> -	regmap = devm_regmap_init_mmio(dev, base, &davinci_psc_regmap_config);
> +	regmap = regmap_init_mmio(dev, base, &davinci_psc_regmap_config);
>  	if (IS_ERR(regmap))
>  		return ERR_CAST(regmap);

Here and in the PLL driver, you have dropped the devm_* variants (like
agreed upon), but not added any error path handling. For the clocks
needed for boot, its probably fine, but the same code path is used for
non-essential clocks too. Just from a code completeness perspective, it
will be nice to see the error path correctly handled.

Apart from that both 1/27 and 3/27 look good to me.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Lechner May 15, 2018, 3:42 p.m. UTC | #8
On 05/15/2018 08:31 AM, Sekhar Nori wrote:
> On Wednesday 09 May 2018 10:55 PM, David Lechner wrote:
>> +void of_da850_pll0_init(struct device_node *node)
>>   {
>> -	return of_davinci_pll_init(dev, dev->of_node, &da850_pll0_info,
>> -				   &da850_pll0_obsclk_info,
>> -				   da850_pll0_sysclk_info, 7, base, cfgchip);
>> +	void __iomem *base;
>> +	struct regmap *cfgchip;
>> +
>> +	base = of_iomap(node, 0);
>> +	if (!base) {
>> +		pr_err("%s: ioremap failed\n", __func__);
>> +		return;
>> +	}
>> +
>> +	cfgchip = syscon_regmap_lookup_by_compatible("ti,da830-cfgchip");

In your previous review, you pointed out that the error did not need to
be handled here because it is handled later in davinci_pll_clk_register().

We get a warning there because cfgchip is only needed for unlocking the
PLL for CPU frequency scaling and is not critical for operation of the
clocks.

> 
> It will be nice to handle the error case here.
> 
>> +
>> +	of_davinci_pll_init(NULL, node, &da850_pll0_info,
>> +			    &da850_pll0_obsclk_info,
>> +			    da850_pll0_sysclk_info, 7, base, cfgchip);
> 
> Apart from that, it looks good to me.
> 
> Reviewed-by: Sekhar Nori <nsekhar@ti.com>
> 
> Thanks,
> Sekhar
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adam Ford May 15, 2018, 10:44 p.m. UTC | #9
On Tue, May 15, 2018 at 4:25 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 2018-05-14 2:40 GMT+02:00 Adam Ford <aford173@gmail.com>:
>> On Wed, May 9, 2018 at 12:25 PM, David Lechner <david@lechnology.com> wrote:
>>> This series converts mach-davinci to use the common clock framework.
>>>
>>> The series works like this, the first 3 patches fix some issues with the clock
>>> drivers that have already been accepted into the mainline kernel.
>>>
>>> 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 four patches add device tree clock support to the one SoC that
>>> supports it.
>>>
>>> This series has been tested on TI OMAP-L138 LCDK (both device tree and legacy
>>> board file).
>>>
>>
>> I am not sure if I did something wrong, but I attempted to build and I
>> wasn't able to boot the da850-evm.dtb your repo common-clk-v11,
>> however the legacy board file boot was OK.
>>
>> make davinci_all_defconfig ARCH=arm
>> make zImage modules da850-evm.dtb ARCH=arm CROSS_COMPILE=arm-linux- -j8
>>
>> 3140416 bytes read in 1464 ms (2 MiB/s)
>> 20353 bytes read in 15 ms (1.3 MiB/s)
>> ## Flattened Device Tree blob at c0600000
>>    Booting using the fdt blob at 0xc0600000
>>    Loading Device Tree to c7e57000, end c7e5ef80 ... OK
>>
>> Starting kernel ...
>>
>> Uncompressing Linux... done, booting the kernel.
>>
>> (and hang)
>>
>> If you have some suggestions, I am try them as I get time.
>>
>> adam
>>
>
> Runs fine on da850-lcdk and dm365-evm. I'll test the da850-evm
> tomorrow when I'll have access to it.

I set the bootargs to: bootargs=console=ttyS2,115200n8
clk_ignore_unused root=/dev/mmcblk0p2 rw rootfstype=ext4 rootwait

I enabled DEBUG_LL and EARLY_PRINTK, yet when it loads, I only get:

## Flattened Device Tree blob at c0600000
   Booting using the fdt blob at 0xc0600000
   Loading Device Tree to c7e57000, end c7e5ef35 ... OK

Starting kernel ...

Uncompressing Linux... done, booting the kernel.


I am doing this at my home, so I don't have a debugger for the
DA850-EVM.  I am using a SOM that is an AM1808, but I vaguely remember
something about enabling a DSP clock somewhere, but I cannot seem to
find the e-mail.  I know its counter intuitive that we'd need to
enable a clock that runs the DSP since it doesn't exist on the AM1808,
but I would have thought the clk_ignore_unused would have worked
around that issue.

If someone else has a DA850-EVM or suggestions, I'm willing to try
them as I have time.

adam
>
> Bart
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Lechner May 16, 2018, 12:31 a.m. UTC | #10
On 5/15/18 5:44 PM, Adam Ford wrote:
> On Tue, May 15, 2018 at 4:25 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> 2018-05-14 2:40 GMT+02:00 Adam Ford <aford173@gmail.com>:
>>> On Wed, May 9, 2018 at 12:25 PM, David Lechner <david@lechnology.com> wrote:
>>>> This series converts mach-davinci to use the common clock framework.
>>>>
>>>> The series works like this, the first 3 patches fix some issues with the clock
>>>> drivers that have already been accepted into the mainline kernel.
>>>>
>>>> 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 four patches add device tree clock support to the one SoC that
>>>> supports it.
>>>>
>>>> This series has been tested on TI OMAP-L138 LCDK (both device tree and legacy
>>>> board file).
>>>>
>>>
>>> I am not sure if I did something wrong, but I attempted to build and I
>>> wasn't able to boot the da850-evm.dtb your repo common-clk-v11,
>>> however the legacy board file boot was OK.
>>>
>>> make davinci_all_defconfig ARCH=arm
>>> make zImage modules da850-evm.dtb ARCH=arm CROSS_COMPILE=arm-linux- -j8
>>>
>>> 3140416 bytes read in 1464 ms (2 MiB/s)
>>> 20353 bytes read in 15 ms (1.3 MiB/s)
>>> ## Flattened Device Tree blob at c0600000
>>>     Booting using the fdt blob at 0xc0600000
>>>     Loading Device Tree to c7e57000, end c7e5ef80 ... OK
>>>
>>> Starting kernel ...
>>>
>>> Uncompressing Linux... done, booting the kernel.
>>>
>>> (and hang)
>>>
>>> If you have some suggestions, I am try them as I get time.
>>>
>>> adam
>>>
>>
>> Runs fine on da850-lcdk and dm365-evm. I'll test the da850-evm
>> tomorrow when I'll have access to it.
> 
> I set the bootargs to: bootargs=console=ttyS2,115200n8
> clk_ignore_unused root=/dev/mmcblk0p2 rw rootfstype=ext4 rootwait

It looks like you forgot earlyprintk in your bootargs.

> 
> I enabled DEBUG_LL and EARLY_PRINTK, yet when it loads, I only get:
> 
> ## Flattened Device Tree blob at c0600000
>     Booting using the fdt blob at 0xc0600000
>     Loading Device Tree to c7e57000, end c7e5ef35 ... OK
> 
> Starting kernel ...
> 
> Uncompressing Linux... done, booting the kernel.
> 
> 
> I am doing this at my home, so I don't have a debugger for the
> DA850-EVM.  I am using a SOM that is an AM1808, but I vaguely remember
> something about enabling a DSP clock somewhere, but I cannot seem to
> find the e-mail.  I know its counter intuitive that we'd need to
> enable a clock that runs the DSP since it doesn't exist on the AM1808,
> but I would have thought the clk_ignore_unused would have worked
> around that issue.
> 
> If someone else has a DA850-EVM or suggestions, I'm willing to try
> them as I have time.
> 
> adam
>>
>> Bart
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori May 16, 2018, 5:51 a.m. UTC | #11
On Tuesday 15 May 2018 09:12 PM, David Lechner wrote:
> On 05/15/2018 08:31 AM, Sekhar Nori wrote:
>> On Wednesday 09 May 2018 10:55 PM, David Lechner wrote:
>>> +void of_da850_pll0_init(struct device_node *node)
>>>   {
>>> -    return of_davinci_pll_init(dev, dev->of_node, &da850_pll0_info,
>>> -                   &da850_pll0_obsclk_info,
>>> -                   da850_pll0_sysclk_info, 7, base, cfgchip);
>>> +    void __iomem *base;
>>> +    struct regmap *cfgchip;
>>> +
>>> +    base = of_iomap(node, 0);
>>> +    if (!base) {
>>> +        pr_err("%s: ioremap failed\n", __func__);
>>> +        return;
>>> +    }
>>> +
>>> +    cfgchip = syscon_regmap_lookup_by_compatible("ti,da830-cfgchip");
> 
> In your previous review, you pointed out that the error did not need to
> be handled here because it is handled later in davinci_pll_clk_register().>
> We get a warning there because cfgchip is only needed for unlocking the
> PLL for CPU frequency scaling and is not critical for operation of the
> clocks.

Oops, forgot about that :)

Reviewed-by: Sekhar Nori <nsekhar@ti.com>

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartosz Golaszewski May 16, 2018, 7:47 a.m. UTC | #12
2018-05-16 0:44 GMT+02:00 Adam Ford <aford173@gmail.com>:
> On Tue, May 15, 2018 at 4:25 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> 2018-05-14 2:40 GMT+02:00 Adam Ford <aford173@gmail.com>:
>>> On Wed, May 9, 2018 at 12:25 PM, David Lechner <david@lechnology.com> wrote:
>>>> This series converts mach-davinci to use the common clock framework.
>>>>
>>>> The series works like this, the first 3 patches fix some issues with the clock
>>>> drivers that have already been accepted into the mainline kernel.
>>>>
>>>> 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 four patches add device tree clock support to the one SoC that
>>>> supports it.
>>>>
>>>> This series has been tested on TI OMAP-L138 LCDK (both device tree and legacy
>>>> board file).
>>>>
>>>
>>> I am not sure if I did something wrong, but I attempted to build and I
>>> wasn't able to boot the da850-evm.dtb your repo common-clk-v11,
>>> however the legacy board file boot was OK.
>>>
>>> make davinci_all_defconfig ARCH=arm
>>> make zImage modules da850-evm.dtb ARCH=arm CROSS_COMPILE=arm-linux- -j8
>>>
>>> 3140416 bytes read in 1464 ms (2 MiB/s)
>>> 20353 bytes read in 15 ms (1.3 MiB/s)
>>> ## Flattened Device Tree blob at c0600000
>>>    Booting using the fdt blob at 0xc0600000
>>>    Loading Device Tree to c7e57000, end c7e5ef80 ... OK
>>>
>>> Starting kernel ...
>>>
>>> Uncompressing Linux... done, booting the kernel.
>>>
>>> (and hang)
>>>
>>> If you have some suggestions, I am try them as I get time.
>>>
>>> adam
>>>
>>
>> Runs fine on da850-lcdk and dm365-evm. I'll test the da850-evm
>> tomorrow when I'll have access to it.
>
> I set the bootargs to: bootargs=console=ttyS2,115200n8
> clk_ignore_unused root=/dev/mmcblk0p2 rw rootfstype=ext4 rootwait
>
> I enabled DEBUG_LL and EARLY_PRINTK, yet when it loads, I only get:
>
> ## Flattened Device Tree blob at c0600000
>    Booting using the fdt blob at 0xc0600000
>    Loading Device Tree to c7e57000, end c7e5ef35 ... OK
>
> Starting kernel ...
>
> Uncompressing Linux... done, booting the kernel.
>
>
> I am doing this at my home, so I don't have a debugger for the
> DA850-EVM.  I am using a SOM that is an AM1808, but I vaguely remember
> something about enabling a DSP clock somewhere, but I cannot seem to
> find the e-mail.  I know its counter intuitive that we'd need to
> enable a clock that runs the DSP since it doesn't exist on the AM1808,
> but I would have thought the clk_ignore_unused would have worked
> around that issue.
>
> If someone else has a DA850-EVM or suggestions, I'm willing to try
> them as I have time.
>
> adam

Hi Adam,

everything works fine for me both when booting the DTB and in legacy
mode on da850-evm.

I'm using the following bootargs:
    ip=dhcp console=ttyS2,115200n8 root=/dev/nfs rw nfsroot=<snip!>,v3
nfsrootdebug

Regular davinci_all_defconfig on David's common-clk-v11 branch.

Best regards,
Bartosz Golaszewski
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adam Ford May 17, 2018, 12:46 a.m. UTC | #13
On Wed, May 9, 2018 at 12:25 PM, David Lechner <david@lechnology.com> wrote:
> This series converts mach-davinci to use the common clock framework.
>
> The series works like this, the first 3 patches fix some issues with the clock
> drivers that have already been accepted into the mainline kernel.
>
> 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 four patches add device tree clock support to the one SoC that
> supports it.
>
> This series has been tested on TI OMAP-L138 LCDK (both device tree and legacy
> board file).
>
I am find.  I don't know what I did wrong, but it's working fine.  If
you want to add my 'tested-by' go ahead.

Tested-by: Adam Ford <aford173@gmail.com> #da850-evm, ethernet, spi
flash, SD, UART

>
> Changes:
>
> v10 changes (also see individual patches for details):
> - Reworked device tree bindings for DaVinci timer.
> - Dropped helper functions to conditionally call devm_* versions of functions
> - Fix some typos
> - Fix some rebasing issues introduced in v9
>
> v9 changes (also see individual patches for details):
> - Rebased on linux-davnci/master (f5e3203bb775)
> - Dropped drivers/clk patches that landed in v4.17
> - New drivers/clk patches for early boot special case
> - New patch for ti,davinci-timer device tree bindings
> - Updated mach/davinci patches to register clocks in early boot when needed
>
> v8 changes (also see individual patches for details):
> - Rebased on linux-davinci/master
> - Dropped use of __init and __initconst attributes in clk drivers
> - Add clkdev lookups for PLL SYSCLKs
> - Fix genpd clock reference counting issue
> - Fix PSC clock driver loading order issue
> - Fix typo in device tree and add more power-domains properties
>
> 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:
>
> There are still some outstanding fixes to get everything working correctly.
> These are all just runtime dependencies and only needed for certain platforms.
>
> - "drm/tilcdc: Fix setting clock divider for omap-l138"[1]
> - "clk: davinci: pll-dm355: fix SYSCLKn parent names"[2]
> - "remoteproc/davinci: common clock framework related fixes"[3]
>
> [1]: https://patchwork.freedesktop.org/patch/210696/
> [2]: https://lkml.org/lkml/2018/5/9/626
> [3]: https://lkml.org/lkml/2018/5/2/201
>
> You can find a working branch with everything included (plus a few extras, like
> cpufreq-dt) in the "common-clk-v10" 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:
>
>         /* pll1_sysclk2 is not affected by CPU scaling, so use it for async3 */
>         parent = clk_hw_get_parent_by_index(&mux->hw, 1);
>         if (parent)
>                 clk_set_parent(mux->hw.clk, parent->clk);
>         else
>                 dev_warn(dev, "Failed to find async3 parent clock\n");
>
> in da8xx-cfgchip.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 (27):
>   clk: davinci: pll: allow dev == NULL
>   clk: davinci: da850-pll: change PLL0 to CLK_OF_DECLARE
>   clk: davinci: psc: allow for dev == NULL
>   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
>   dt-bindings: timer: new bindings for TI DaVinci timer
>   ARM: davinci: add device tree support to timer
>   ARM: davinci: da8xx-dt: switch to device tree clocks
>   ARM: dts: da850: Add clocks
>
>  .../bindings/timer/ti,davinci-timer.txt       |  37 +
>  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                  | 168 ++++
>  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                 | 462 ++---------
>  arch/arm/mach-davinci/da850.c                 | 778 +++---------------
>  arch/arm/mach-davinci/da8xx-dt.c              |  66 --
>  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                 | 406 ++-------
>  arch/arm/mach-davinci/dm365.c                 | 485 +----------
>  arch/arm/mach-davinci/dm644x.c                | 344 +-------
>  arch/arm/mach-davinci/dm646x.c                | 372 +--------
>  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                  |  39 +-
>  arch/arm/mach-davinci/usb-da8xx.c             | 242 +-----
>  drivers/clk/davinci/pll-da830.c               |   4 +-
>  drivers/clk/davinci/pll-da850.c               |  36 +-
>  drivers/clk/davinci/pll-dm355.c               |   8 +-
>  drivers/clk/davinci/pll-dm365.c               |   8 +-
>  drivers/clk/davinci/pll-dm644x.c              |   8 +-
>  drivers/clk/davinci/pll-dm646x.c              |   8 +-
>  drivers/clk/davinci/pll.c                     | 110 +--
>  drivers/clk/davinci/pll.h                     |  30 +-
>  drivers/clk/davinci/psc-dm355.c               |   2 +-
>  drivers/clk/davinci/psc-dm365.c               |   2 +-
>  drivers/clk/davinci/psc-dm644x.c              |   2 +-
>  drivers/clk/davinci/psc-dm646x.c              |   2 +-
>  drivers/clk/davinci/psc.c                     |  27 +-
>  include/linux/clk/davinci.h                   |  29 +
>  56 files changed, 860 insertions(+), 3950 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/timer/ti,davinci-timer.txt
>  delete mode 100644 arch/arm/mach-davinci/clock.c
>  delete mode 100644 arch/arm/mach-davinci/psc.c
>  create mode 100644 include/linux/clk/davinci.h
>
> --
> 2.17.0
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori May 17, 2018, 2:35 p.m. UTC | #14
Hi David,

On Wednesday 09 May 2018 10:56 PM, David Lechner wrote:
> This adds device tree support to the davinci timer so that when clocks
> are moved to device tree, the timer will still work.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

> +static int __init of_davinci_timer_init(struct device_node *np)
> +{
> +	struct clk *clk;
> +
> +	clk = of_clk_get(np, 0);
> +	if (IS_ERR(clk)) {
> +		struct of_phandle_args clkspec;
> +
> +		/*
> +		 * Fall back to using ref_clk if the actual clock is not
> +		 * available. There will be problems later if the real clock
> +		 * source is disabled.
> +		 */
> +
> +		pr_warn("%s: falling back to ref_clk\n", __func__);
> +
> +		clkspec.np = of_find_node_by_name(NULL, "ref_clk");
> +		if (IS_ERR(clkspec.np)) {
> +			pr_err("%s: No clock available for timer!\n", __func__);
> +			return PTR_ERR(clkspec.np);
> +		}
> +		clk = of_clk_get_from_provider(&clkspec);
> +		of_node_put(clkspec.np);
> +	}

Do we need this error path now?

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Lechner May 17, 2018, 3:09 p.m. UTC | #15
On 05/17/2018 09:35 AM, Sekhar Nori wrote:
> Hi David,
> 
> On Wednesday 09 May 2018 10:56 PM, David Lechner wrote:
>> This adds device tree support to the davinci timer so that when clocks
>> are moved to device tree, the timer will still work.
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> ---
> 
>> +static int __init of_davinci_timer_init(struct device_node *np)
>> +{
>> +	struct clk *clk;
>> +
>> +	clk = of_clk_get(np, 0);
>> +	if (IS_ERR(clk)) {
>> +		struct of_phandle_args clkspec;
>> +
>> +		/*
>> +		 * Fall back to using ref_clk if the actual clock is not
>> +		 * available. There will be problems later if the real clock
>> +		 * source is disabled.
>> +		 */
>> +
>> +		pr_warn("%s: falling back to ref_clk\n", __func__);
>> +
>> +		clkspec.np = of_find_node_by_name(NULL, "ref_clk");
>> +		if (IS_ERR(clkspec.np)) {
>> +			pr_err("%s: No clock available for timer!\n", __func__);
>> +			return PTR_ERR(clkspec.np);
>> +		}
>> +		clk = of_clk_get_from_provider(&clkspec);
>> +		of_node_put(clkspec.np);
>> +	}
> 
> Do we need this error path now?
> 
> Thanks,
> Sekhar
> 

No, not really.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori May 18, 2018, 6:05 a.m. UTC | #16
On Thursday 17 May 2018 08:39 PM, David Lechner wrote:
> On 05/17/2018 09:35 AM, Sekhar Nori wrote:
>> Hi David,
>>
>> On Wednesday 09 May 2018 10:56 PM, David Lechner wrote:
>>> This adds device tree support to the davinci timer so that when clocks
>>> are moved to device tree, the timer will still work.
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>> ---
>>
>>> +static int __init of_davinci_timer_init(struct device_node *np)
>>> +{
>>> +    struct clk *clk;
>>> +
>>> +    clk = of_clk_get(np, 0);
>>> +    if (IS_ERR(clk)) {
>>> +        struct of_phandle_args clkspec;
>>> +
>>> +        /*
>>> +         * Fall back to using ref_clk if the actual clock is not
>>> +         * available. There will be problems later if the real clock
>>> +         * source is disabled.
>>> +         */
>>> +
>>> +        pr_warn("%s: falling back to ref_clk\n", __func__);
>>> +
>>> +        clkspec.np = of_find_node_by_name(NULL, "ref_clk");
>>> +        if (IS_ERR(clkspec.np)) {
>>> +            pr_err("%s: No clock available for timer!\n", __func__);
>>> +            return PTR_ERR(clkspec.np);
>>> +        }
>>> +        clk = of_clk_get_from_provider(&clkspec);
>>> +        of_node_put(clkspec.np);
>>> +    }
>>
>> Do we need this error path now?
>>
>> Thanks,
>> Sekhar
>>
> 
> No, not really.

Then lets just print an error and return the error number.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Lechner May 18, 2018, 3:35 p.m. UTC | #17
On 05/18/2018 01:05 AM, Sekhar Nori wrote:
> On Thursday 17 May 2018 08:39 PM, David Lechner wrote:
>> On 05/17/2018 09:35 AM, Sekhar Nori wrote:
>>> Hi David,
>>>
>>> On Wednesday 09 May 2018 10:56 PM, David Lechner wrote:
>>>> This adds device tree support to the davinci timer so that when clocks
>>>> are moved to device tree, the timer will still work.
>>>>
>>>> Signed-off-by: David Lechner <david@lechnology.com>
>>>> ---
>>>
>>>> +static int __init of_davinci_timer_init(struct device_node *np)
>>>> +{
>>>> +    struct clk *clk;
>>>> +
>>>> +    clk = of_clk_get(np, 0);
>>>> +    if (IS_ERR(clk)) {
>>>> +        struct of_phandle_args clkspec;
>>>> +
>>>> +        /*
>>>> +         * Fall back to using ref_clk if the actual clock is not
>>>> +         * available. There will be problems later if the real clock
>>>> +         * source is disabled.
>>>> +         */
>>>> +
>>>> +        pr_warn("%s: falling back to ref_clk\n", __func__);
>>>> +
>>>> +        clkspec.np = of_find_node_by_name(NULL, "ref_clk");
>>>> +        if (IS_ERR(clkspec.np)) {
>>>> +            pr_err("%s: No clock available for timer!\n", __func__);
>>>> +            return PTR_ERR(clkspec.np);
>>>> +        }
>>>> +        clk = of_clk_get_from_provider(&clkspec);
>>>> +        of_node_put(clkspec.np);
>>>> +    }
>>>
>>> Do we need this error path now?
>>>
>>> Thanks,
>>> Sekhar
>>>
>>
>> No, not really.
> 
> Then lets just print an error and return the error number.
> 

OK. FYI, timer_probe() prints the error if we return and error, so
I will just return the error.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html