mbox series

[v8,00/42] ARM: davinci: convert to common clock framework​

Message ID 1521168778-27236-1-git-send-email-david@lechnology.com
Headers show
Series ARM: davinci: convert to common clock framework​ | expand

Message

David Lechner March 16, 2018, 2:52 a.m. UTC
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.

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


Changes:

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:

- "ARM: davinci: fix the GPIO lookup for omapl138-hawk"[1] (in linux-davinci/fixes)
- "drm/tilcdc: Fix setting clock divider for omap-l138"[2] (new)
- "ARM: davinci: DA8XX: fix oops in USB PHY driver due to stack allocated
   platform platform_data"[3] (new)
- "ARM: davinci: DA8XX: simplify CFGCHIP regmap_config"[4] (new)
- "clk: divider: export clk_div_mask() helper"[5] (in clk/fixes)
- "clk: divider: read-only divider can propagate rate change"[6] (in clk/fixes)
- "gpio: Handle deferred probing in of_find_gpio() properly"[7] (in v4.16-rc4)
- "gpiolib: Keep returning EPROBE_DEFER when we should"[8] (in v4.16-rc4)

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git/commit/?h=fixes&id=c4dc56be7e26040bfc60ce73425353516a356955
[2]: https://patchwork.freedesktop.org/patch/210696/
[3]: https://patchwork.kernel.org/patch/10285679/
[4]: https://patchwork.kernel.org/patch/10285605/
[5]: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=e6d3cc7b1fac3d7f1313faf8ac9b23830113e3ec
[6]: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=b15ee490e16324c35b51f04bad54ae45a2cefd29
[7]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ce27fb2c56db6ccfe8099343bb4afdab15e77e7b
[8]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6662ae6af82df10259a70c7569b4c12ea7f3ba93

You can find a working branch with everything included (plus a few extras, like
cpufreq-dt) in the "common-clk-v8" 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                       | 167 ++++
 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                      | 789 ++++--------------
 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                     | 394 ++-------
 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                  | 242 +-----
 drivers/clk/Makefile                               |   1 +
 drivers/clk/davinci/Makefile                       |  21 +
 drivers/clk/davinci/da8xx-cfgchip.c                | 790 ++++++++++++++++++
 drivers/clk/davinci/pll-da830.c                    |  70 ++
 drivers/clk/davinci/pll-da850.c                    | 212 +++++
 drivers/clk/davinci/pll-dm355.c                    |  79 ++
 drivers/clk/davinci/pll-dm365.c                    | 145 ++++
 drivers/clk/davinci/pll-dm644x.c                   |  80 ++
 drivers/clk/davinci/pll-dm646x.c                   |  84 ++
 drivers/clk/davinci/pll.c                          | 901 +++++++++++++++++++++
 drivers/clk/davinci/pll.h                          | 141 ++++
 drivers/clk/davinci/psc-da830.c                    | 116 +++
 drivers/clk/davinci/psc-da850.c                    | 149 ++++
 drivers/clk/davinci/psc-dm355.c                    |  88 ++
 drivers/clk/davinci/psc-dm365.c                    |  96 +++
 drivers/clk/davinci/psc-dm644x.c                   |  83 ++
 drivers/clk/davinci/psc-dm646x.c                   |  80 ++
 drivers/clk/davinci/psc.c                          | 552 +++++++++++++
 drivers/clk/davinci/psc.h                          | 108 +++
 include/linux/platform_data/clk-da8xx-cfgchip.h    |  21 +
 include/linux/platform_data/clk-davinci-pll.h      |  21 +
 66 files changed, 4876 insertions(+), 3836 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
 create mode 100644 include/linux/platform_data/clk-davinci-pll.h

Comments

David Lechner March 16, 2018, 2:59 a.m. UTC | #1
On 03/15/2018 09:52 PM, David Lechner wrote:
> This series converts mach-davinci to use the common clock framework.
> 


Hi Adam,

I think we are getting pretty close to the final product here. If you
have time to test the EVM(s) you have again, I think this is a good
time to do it.

--
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 March 16, 2018, 3:11 a.m. UTC | #2
On 03/15/2018 09:52 PM, David Lechner wrote:
> This adds platform-specific declarations for the PSC clocks on TI DA850/
> OMAP-L138/AM18XX SoCs.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> Reviewed-by: Sekhar Nori <nsekhar@ti.com>
> ---
> 

This Reviewed-by: was supposed to be dropped. This file has had some significant
changes.
--
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 March 16, 2018, 5:20 p.m. UTC | #3
On 03/15/2018 09:52 PM, David Lechner wrote:
> This adds clock provider nodes for da850 and wires them up to all of the
> devices.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

...

This is the mcasp0: mcasp@100000 node...

> @@ -560,6 +720,7 @@
>   			dmas = <&edma0 1 1>,
>   				<&edma0 0 1>;
>   			dma-names = "tx", "rx";
> +			clocks = <&psc1 7>;

After some testing, it looks like it needs to be:

+			power-domains = <&psc1 7>;

instead of

+			clocks = <&psc1 7>;

>   		};
>   
>   		lcdc: display@213000 {
--
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 March 19, 2018, 1:17 p.m. UTC | #4
2018-03-16 3:52 GMT+01:00 David Lechner <david@lechnology.com>:
> 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.
>
> This series has been tested on LEGO MINDSTORMS EV3 (device tree) and TI
> OMAP-L138 LCDK (both device tree and legacy board file).
>

Hi David,

thanks, with the u-boot fix everything seems to work fine. I tested
da850-lcdk and da850-evm boards in DT and board file modes.

I'll be working with this series in the following days and will do
some thorough testing and rebase my aemif series on top of it. I'll
also use the recently merged reset support for board files to get rid
of the exported reset functions.

Thanks for your work!

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
David Lechner March 19, 2018, 3:59 p.m. UTC | #5
On 03/19/2018 08:17 AM, Bartosz Golaszewski wrote:
> 2018-03-16 3:52 GMT+01:00 David Lechner <david@lechnology.com>:
>> 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.
>>
>> This series has been tested on LEGO MINDSTORMS EV3 (device tree) and TI
>> OMAP-L138 LCDK (both device tree and legacy board file).
>>
> 
> Hi David,
> 
> thanks, with the u-boot fix everything seems to work fine. I tested
> da850-lcdk and da850-evm boards in DT and board file modes.
> 
> I'll be working with this series in the following days and will do
> some thorough testing and rebase my aemif series on top of it. I'll
> also use the recently merged reset support for board files to get rid
> of the exported reset functions.
> 
> Thanks for your work!
> 
> Best regards,
> Bartosz Golaszewski
> 

Sounds like a good plan to me. FYI, I've started a common-clk-v9 branch
on my GitHub already with a few minor fixes.
--
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 March 19, 2018, 4:11 p.m. UTC | #6
On Mon, Mar 19, 2018 at 10:59 AM, David Lechner <david@lechnology.com> wrote:
> On 03/19/2018 08:17 AM, Bartosz Golaszewski wrote:
>>
>> 2018-03-16 3:52 GMT+01:00 David Lechner <david@lechnology.com>:
>>>
>>> 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.
>>>
>>> This series has been tested on LEGO MINDSTORMS EV3 (device tree) and TI
>>> OMAP-L138 LCDK (both device tree and legacy board file).
>>>
>>
>> Hi David,
>>
>> thanks, with the u-boot fix everything seems to work fine. I tested
>> da850-lcdk and da850-evm boards in DT and board file modes.
>>

I tested MMC and Ethernet.  Both appear to be operating normally, but
I have having USB issues.

Should I be testing cpufreq?  I noticed that when changing the
frequencies, the USB appears to stop recognizing connections and
disconnections.

I did some testing on the DA850-EVM with USB from your repo, and I'm
getting some crashing on MUSB unplug that do not appear in the
4.16-RC6 from linux-stable:


------------[ cut here ]------------
WARNING: CPU: 0 PID: 13 at drivers/dma/cppi41.c:694
cppi41_stop_chan+0x1f4/0x378 [cppi41]
Modules linked in: evbug evdev mousedev hid_generic usbhid hid
usb_f_ss_lb g_zero libcomposite configfs ofpart cmdlinepart m25p80
spi_nor cppi41 adv7343 tvp514x vpif_display vpif_capture
videobuf2_dma_con
tig videobuf2_memops videobuf2_v4l2 videobuf2_common v4l2_fwnode
v4l2_common snd_soc_simple_card tilcdc spi_davinci
snd_soc_simple_card_utils videodev pwm_tiecap spi_bitbang da8xx
drm_kms_helper phy_gener
ic syscopyarea ohci_da8xx musb_hdrc sysfillrect sysimgblt fb_sys_fops
ohci_hcd udc_core drm usbcore drm_panel_orientation_quirks usb_common
snd_soc_tlv320aic3x vpif snd_soc_davinci_mcasp snd_soc_edma snd_
soc_core snd_pcm_dmaengine snd_pcm rtc_omap snd_timer snd soundcore
phy_da8xx_usb davinci_nand nand nand_ecc mtd pwm_bl backlight
cpufreq_dt ti_aemif
CPU: 0 PID: 13 Comm: kworker/0:1 Not tainted 4.16.0-rc5-g291ba8b-dirty #3
Hardware name: Generic DA850/OMAP-L138/AM18x
Workqueue: usb_hub_wq hub_event [usbcore]
Backtrace:
[<c000df94>] (dump_backtrace) from [<c000e264>] (show_stack+0x18/0x1c)
 r7:00000009 r6:00000000 r5:bf339ad0 r4:00000000
[<c000e24c>] (show_stack) from [<c04864c0>] (dump_stack+0x20/0x28)
[<c04864a0>] (dump_stack) from [<c001ac38>] (__warn+0xd4/0xfc)
[<c001ab64>] (__warn) from [<c001ad7c>] (warn_slowpath_null+0x44/0x50)
 r8:c6c43080 r7:c05f1008 r6:bf338744 r5:000002b6 r4:bf339ad0
[<c001ad38>] (warn_slowpath_null) from [<bf338744>]
(cppi41_stop_chan+0x1f4/0x378 [cppi41])
 r6:c5e59410 r5:c5e59420 r4:c5feca50
[<bf338550>] (cppi41_stop_chan [cppi41]) from [<bf213dac>]
(cppi41_dma_channel_abort+0xe0/0x2a8 [musb_hdrc])
 r8:c6929180 r7:00000000 r6:c5c67290 r5:bf2175a0 r4:00000008
[<bf213ccc>] (cppi41_dma_channel_abort [musb_hdrc]) from [<bf20c4bc>]
(musb_cleanup_urb+0x60/0x210 [musb_hdrc])
 r10:00000001 r9:c5fca010 r8:c5c67290 r7:a0000093 r6:00000080 r5:c55b5100
 r4:c5fca5a8
[<bf20c45c>] (musb_cleanup_urb [musb_hdrc]) from [<bf20ca8c>]
(musb_urb_dequeue+0xfc/0x164 [musb_hdrc])
 r10:00000001 r9:40408280 r8:c5fca010 r7:a0000093 r6:00000000 r5:c55b5380
 r4:c55b5100
[<bf20c990>] (musb_urb_dequeue [musb_hdrc]) from [<bf10fd7c>]
(unlink1+0x34/0x13c [usbcore])
 r10:00000100 r9:c5cb2a00 r8:c55b5100 r7:ffffff94 r6:c5c6c000 r5:c5dd2380
 r4:c55b5100 r3:bf20c990
[<bf10fd48>] (unlink1 [usbcore]) from [<bf112424>]
(usb_hcd_flush_endpoint+0x164/0x1a4 [usbcore])
 r9:c5cb2a00 r8:c55b5100 r7:c5c6c000 r6:c5dd2398 r5:c5dd2380 r4:ffffe000
[<bf1122c0>] (usb_hcd_flush_endpoint [usbcore]) from [<bf11563c>]
(usb_disable_endpoint+0x50/0x94 [usbcore])
 r9:c5cb2a00 r8:bf39e898 r7:00000000 r6:c5c43000 r5:c5c43000 r4:c5dd2380
[<bf1155ec>] (usb_disable_endpoint [usbcore]) from [<bf1156c4>]
(usb_disable_interface+0x44/0x58 [usbcore])
 r5:c5dd2348 r4:00000000
[<bf115680>] (usb_disable_interface [usbcore]) from [<bf118044>]
(usb_unbind_interface+0x1c0/0x268 [usbcore])
 r7:c5cb2c00 r6:bf39e898 r5:c5cb2c20 r4:c5cb2c20
[<bf117e84>] (usb_unbind_interface [usbcore]) from [<c02ce180>]
(device_release_driver_internal+0x160/0x208)
 r10:00000100 r9:c5cb2a00 r8:00000034 r7:00000000 r6:bf39e898 r5:c5cb2c54
 r4:c5cb2c20
[<c02ce020>] (device_release_driver_internal) from [<c02ce240>]
(device_release_driver+0x18/0x1c)
 r9:c5cb2a00 r8:c05f1008 r7:c5c43070 r6:bf12895c r5:c5cb2c20 r4:c5dc91ac
[<c02ce228>] (device_release_driver) from [<c02ccf70>]
(bus_remove_device+0xd0/0x100)
[<c02ccea0>] (bus_remove_device) from [<c02c9c60>] (device_del+0x120/0x378)
 r7:c5c43070 r6:c5cb2c28 r5:00000000 r4:c5cb2c20
[<c02c9b40>] (device_del) from [<bf11576c>]
(usb_disable_device+0x94/0x1d8 [usbcore])
 r10:00000100 r9:c5cb2a00 r8:c5cb2c00 r7:c5c6c000 r6:00000001 r5:00000000
 r4:c5c43000
[<bf1156d8>] (usb_disable_device [usbcore]) from [<bf10bccc>]
(usb_disconnect+0xb4/0x244 [usbcore])
 r9:c5cb2a00 r8:c5c2d600 r7:c5c430a4 r6:c5c43070 r5:c5c43000 r4:00000000
[<bf10bc18>] (usb_disconnect [usbcore]) from [<bf10d788>]
(hub_event+0x678/0x11dc [usbcore])
 r10:00000100 r9:c05f1008 r8:c5c2dcf8 r7:c68fc400 r6:00000001 r5:40000000
 r4:00000000
[<bf10d110>] (hub_event [usbcore]) from [<c0031db0>]
(process_one_work+0x1d8/0x41c)
 r10:00000008 r9:00000000 r8:c060b1d0 r7:00000000 r6:c7ede200 r5:c6882480
 r4:c5c2dcf8
[<c0031bd8>] (process_one_work) from [<c0032030>] (worker_thread+0x3c/0x670)
 r10:00000008 r9:c060b1e4 r8:c061ab60 r7:c6882498 r6:ffffe000 r5:c060b1d0
 r4:c6882480
[<c0031ff4>] (worker_thread) from [<c0038270>] (kthread+0x134/0x14c)
 r10:c683fe90 r9:c0031ff4 r8:c6882480 r7:c6896000 r6:00000000 r5:c687bd80
 r4:c6803e80
[<c003813c>] (kthread) from [<c00090e0>] (ret_from_fork+0x14/0x34)
Exception stack(0xc6897fb0 to 0xc6897ff8)
7fa0:                                     00000000 00000000 00000000 00000000
7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
 r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c003813c
 r4:c687bd80
---[ end trace cb9c7f93aaa3ed36 ]---
------------[ cut here ]------------


adam


>> I'll be working with this series in the following days and will do
>> some thorough testing and rebase my aemif series on top of it. I'll
>> also use the recently merged reset support for board files to get rid
>> of the exported reset functions.
>>
>> Thanks for your work!
>>
>> Best regards,
>> Bartosz Golaszewski
>>
>
> Sounds like a good plan to me. FYI, I've started a common-clk-v9 branch
> on my GitHub already with a few minor fixes.
--
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 March 19, 2018, 4:14 p.m. UTC | #7
2018-03-19 17:11 GMT+01:00 Adam Ford <aford173@gmail.com>:
> On Mon, Mar 19, 2018 at 10:59 AM, David Lechner <david@lechnology.com> wrote:
>> On 03/19/2018 08:17 AM, Bartosz Golaszewski wrote:
>>>
>>> 2018-03-16 3:52 GMT+01:00 David Lechner <david@lechnology.com>:
>>>>
>>>> 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.
>>>>
>>>> This series has been tested on LEGO MINDSTORMS EV3 (device tree) and TI
>>>> OMAP-L138 LCDK (both device tree and legacy board file).
>>>>
>>>
>>> Hi David,
>>>
>>> thanks, with the u-boot fix everything seems to work fine. I tested
>>> da850-lcdk and da850-evm boards in DT and board file modes.
>>>
>
> I tested MMC and Ethernet.  Both appear to be operating normally, but
> I have having USB issues.
>
> Should I be testing cpufreq?  I noticed that when changing the
> frequencies, the USB appears to stop recognizing connections and
> disconnections.
>
> I did some testing on the DA850-EVM with USB from your repo, and I'm
> getting some crashing on MUSB unplug that do not appear in the
> 4.16-RC6 from linux-stable:
>
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 13 at drivers/dma/cppi41.c:694
> cppi41_stop_chan+0x1f4/0x378 [cppi41]
> Modules linked in: evbug evdev mousedev hid_generic usbhid hid
> usb_f_ss_lb g_zero libcomposite configfs ofpart cmdlinepart m25p80
> spi_nor cppi41 adv7343 tvp514x vpif_display vpif_capture
> videobuf2_dma_con
> tig videobuf2_memops videobuf2_v4l2 videobuf2_common v4l2_fwnode
> v4l2_common snd_soc_simple_card tilcdc spi_davinci
> snd_soc_simple_card_utils videodev pwm_tiecap spi_bitbang da8xx
> drm_kms_helper phy_gener
> ic syscopyarea ohci_da8xx musb_hdrc sysfillrect sysimgblt fb_sys_fops
> ohci_hcd udc_core drm usbcore drm_panel_orientation_quirks usb_common
> snd_soc_tlv320aic3x vpif snd_soc_davinci_mcasp snd_soc_edma snd_
> soc_core snd_pcm_dmaengine snd_pcm rtc_omap snd_timer snd soundcore
> phy_da8xx_usb davinci_nand nand nand_ecc mtd pwm_bl backlight
> cpufreq_dt ti_aemif
> CPU: 0 PID: 13 Comm: kworker/0:1 Not tainted 4.16.0-rc5-g291ba8b-dirty #3
> Hardware name: Generic DA850/OMAP-L138/AM18x
> Workqueue: usb_hub_wq hub_event [usbcore]
> Backtrace:
> [<c000df94>] (dump_backtrace) from [<c000e264>] (show_stack+0x18/0x1c)
>  r7:00000009 r6:00000000 r5:bf339ad0 r4:00000000
> [<c000e24c>] (show_stack) from [<c04864c0>] (dump_stack+0x20/0x28)
> [<c04864a0>] (dump_stack) from [<c001ac38>] (__warn+0xd4/0xfc)
> [<c001ab64>] (__warn) from [<c001ad7c>] (warn_slowpath_null+0x44/0x50)
>  r8:c6c43080 r7:c05f1008 r6:bf338744 r5:000002b6 r4:bf339ad0
> [<c001ad38>] (warn_slowpath_null) from [<bf338744>]
> (cppi41_stop_chan+0x1f4/0x378 [cppi41])
>  r6:c5e59410 r5:c5e59420 r4:c5feca50
> [<bf338550>] (cppi41_stop_chan [cppi41]) from [<bf213dac>]
> (cppi41_dma_channel_abort+0xe0/0x2a8 [musb_hdrc])
>  r8:c6929180 r7:00000000 r6:c5c67290 r5:bf2175a0 r4:00000008
> [<bf213ccc>] (cppi41_dma_channel_abort [musb_hdrc]) from [<bf20c4bc>]
> (musb_cleanup_urb+0x60/0x210 [musb_hdrc])
>  r10:00000001 r9:c5fca010 r8:c5c67290 r7:a0000093 r6:00000080 r5:c55b5100
>  r4:c5fca5a8
> [<bf20c45c>] (musb_cleanup_urb [musb_hdrc]) from [<bf20ca8c>]
> (musb_urb_dequeue+0xfc/0x164 [musb_hdrc])
>  r10:00000001 r9:40408280 r8:c5fca010 r7:a0000093 r6:00000000 r5:c55b5380
>  r4:c55b5100
> [<bf20c990>] (musb_urb_dequeue [musb_hdrc]) from [<bf10fd7c>]
> (unlink1+0x34/0x13c [usbcore])
>  r10:00000100 r9:c5cb2a00 r8:c55b5100 r7:ffffff94 r6:c5c6c000 r5:c5dd2380
>  r4:c55b5100 r3:bf20c990
> [<bf10fd48>] (unlink1 [usbcore]) from [<bf112424>]
> (usb_hcd_flush_endpoint+0x164/0x1a4 [usbcore])
>  r9:c5cb2a00 r8:c55b5100 r7:c5c6c000 r6:c5dd2398 r5:c5dd2380 r4:ffffe000
> [<bf1122c0>] (usb_hcd_flush_endpoint [usbcore]) from [<bf11563c>]
> (usb_disable_endpoint+0x50/0x94 [usbcore])
>  r9:c5cb2a00 r8:bf39e898 r7:00000000 r6:c5c43000 r5:c5c43000 r4:c5dd2380
> [<bf1155ec>] (usb_disable_endpoint [usbcore]) from [<bf1156c4>]
> (usb_disable_interface+0x44/0x58 [usbcore])
>  r5:c5dd2348 r4:00000000
> [<bf115680>] (usb_disable_interface [usbcore]) from [<bf118044>]
> (usb_unbind_interface+0x1c0/0x268 [usbcore])
>  r7:c5cb2c00 r6:bf39e898 r5:c5cb2c20 r4:c5cb2c20
> [<bf117e84>] (usb_unbind_interface [usbcore]) from [<c02ce180>]
> (device_release_driver_internal+0x160/0x208)
>  r10:00000100 r9:c5cb2a00 r8:00000034 r7:00000000 r6:bf39e898 r5:c5cb2c54
>  r4:c5cb2c20
> [<c02ce020>] (device_release_driver_internal) from [<c02ce240>]
> (device_release_driver+0x18/0x1c)
>  r9:c5cb2a00 r8:c05f1008 r7:c5c43070 r6:bf12895c r5:c5cb2c20 r4:c5dc91ac
> [<c02ce228>] (device_release_driver) from [<c02ccf70>]
> (bus_remove_device+0xd0/0x100)
> [<c02ccea0>] (bus_remove_device) from [<c02c9c60>] (device_del+0x120/0x378)
>  r7:c5c43070 r6:c5cb2c28 r5:00000000 r4:c5cb2c20
> [<c02c9b40>] (device_del) from [<bf11576c>]
> (usb_disable_device+0x94/0x1d8 [usbcore])
>  r10:00000100 r9:c5cb2a00 r8:c5cb2c00 r7:c5c6c000 r6:00000001 r5:00000000
>  r4:c5c43000
> [<bf1156d8>] (usb_disable_device [usbcore]) from [<bf10bccc>]
> (usb_disconnect+0xb4/0x244 [usbcore])
>  r9:c5cb2a00 r8:c5c2d600 r7:c5c430a4 r6:c5c43070 r5:c5c43000 r4:00000000
> [<bf10bc18>] (usb_disconnect [usbcore]) from [<bf10d788>]
> (hub_event+0x678/0x11dc [usbcore])
>  r10:00000100 r9:c05f1008 r8:c5c2dcf8 r7:c68fc400 r6:00000001 r5:40000000
>  r4:00000000
> [<bf10d110>] (hub_event [usbcore]) from [<c0031db0>]
> (process_one_work+0x1d8/0x41c)
>  r10:00000008 r9:00000000 r8:c060b1d0 r7:00000000 r6:c7ede200 r5:c6882480
>  r4:c5c2dcf8
> [<c0031bd8>] (process_one_work) from [<c0032030>] (worker_thread+0x3c/0x670)
>  r10:00000008 r9:c060b1e4 r8:c061ab60 r7:c6882498 r6:ffffe000 r5:c060b1d0
>  r4:c6882480
> [<c0031ff4>] (worker_thread) from [<c0038270>] (kthread+0x134/0x14c)
>  r10:c683fe90 r9:c0031ff4 r8:c6882480 r7:c6896000 r6:00000000 r5:c687bd80
>  r4:c6803e80
> [<c003813c>] (kthread) from [<c00090e0>] (ret_from_fork+0x14/0x34)
> Exception stack(0xc6897fb0 to 0xc6897ff8)
> 7fa0:                                     00000000 00000000 00000000 00000000
> 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c003813c
>  r4:c687bd80
> ---[ end trace cb9c7f93aaa3ed36 ]---
> ------------[ cut here ]------------
>
>
> adam
>

Hi Adam,

this is a known issue and it's present in mainline and even in TI BSP.
It's been deprioritized for now, but I'll return to debugging it once
the common clock conversion is complete.

Thanks,
Bartosz
--
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 March 19, 2018, 4:15 p.m. UTC | #8
2018-03-19 17:14 GMT+01:00 Bartosz Golaszewski <bgolaszewski@baylibre.com>:
> 2018-03-19 17:11 GMT+01:00 Adam Ford <aford173@gmail.com>:
>> On Mon, Mar 19, 2018 at 10:59 AM, David Lechner <david@lechnology.com> wrote:
>>> On 03/19/2018 08:17 AM, Bartosz Golaszewski wrote:
>>>>
>>>> 2018-03-16 3:52 GMT+01:00 David Lechner <david@lechnology.com>:
>>>>>
>>>>> 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.
>>>>>
>>>>> This series has been tested on LEGO MINDSTORMS EV3 (device tree) and TI
>>>>> OMAP-L138 LCDK (both device tree and legacy board file).
>>>>>
>>>>
>>>> Hi David,
>>>>
>>>> thanks, with the u-boot fix everything seems to work fine. I tested
>>>> da850-lcdk and da850-evm boards in DT and board file modes.
>>>>
>>
>> I tested MMC and Ethernet.  Both appear to be operating normally, but
>> I have having USB issues.
>>
>> Should I be testing cpufreq?  I noticed that when changing the
>> frequencies, the USB appears to stop recognizing connections and
>> disconnections.
>>
>> I did some testing on the DA850-EVM with USB from your repo, and I'm
>> getting some crashing on MUSB unplug that do not appear in the
>> 4.16-RC6 from linux-stable:
>>
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 13 at drivers/dma/cppi41.c:694
>> cppi41_stop_chan+0x1f4/0x378 [cppi41]
>> Modules linked in: evbug evdev mousedev hid_generic usbhid hid
>> usb_f_ss_lb g_zero libcomposite configfs ofpart cmdlinepart m25p80
>> spi_nor cppi41 adv7343 tvp514x vpif_display vpif_capture
>> videobuf2_dma_con
>> tig videobuf2_memops videobuf2_v4l2 videobuf2_common v4l2_fwnode
>> v4l2_common snd_soc_simple_card tilcdc spi_davinci
>> snd_soc_simple_card_utils videodev pwm_tiecap spi_bitbang da8xx
>> drm_kms_helper phy_gener
>> ic syscopyarea ohci_da8xx musb_hdrc sysfillrect sysimgblt fb_sys_fops
>> ohci_hcd udc_core drm usbcore drm_panel_orientation_quirks usb_common
>> snd_soc_tlv320aic3x vpif snd_soc_davinci_mcasp snd_soc_edma snd_
>> soc_core snd_pcm_dmaengine snd_pcm rtc_omap snd_timer snd soundcore
>> phy_da8xx_usb davinci_nand nand nand_ecc mtd pwm_bl backlight
>> cpufreq_dt ti_aemif
>> CPU: 0 PID: 13 Comm: kworker/0:1 Not tainted 4.16.0-rc5-g291ba8b-dirty #3
>> Hardware name: Generic DA850/OMAP-L138/AM18x
>> Workqueue: usb_hub_wq hub_event [usbcore]
>> Backtrace:
>> [<c000df94>] (dump_backtrace) from [<c000e264>] (show_stack+0x18/0x1c)
>>  r7:00000009 r6:00000000 r5:bf339ad0 r4:00000000
>> [<c000e24c>] (show_stack) from [<c04864c0>] (dump_stack+0x20/0x28)
>> [<c04864a0>] (dump_stack) from [<c001ac38>] (__warn+0xd4/0xfc)
>> [<c001ab64>] (__warn) from [<c001ad7c>] (warn_slowpath_null+0x44/0x50)
>>  r8:c6c43080 r7:c05f1008 r6:bf338744 r5:000002b6 r4:bf339ad0
>> [<c001ad38>] (warn_slowpath_null) from [<bf338744>]
>> (cppi41_stop_chan+0x1f4/0x378 [cppi41])
>>  r6:c5e59410 r5:c5e59420 r4:c5feca50
>> [<bf338550>] (cppi41_stop_chan [cppi41]) from [<bf213dac>]
>> (cppi41_dma_channel_abort+0xe0/0x2a8 [musb_hdrc])
>>  r8:c6929180 r7:00000000 r6:c5c67290 r5:bf2175a0 r4:00000008
>> [<bf213ccc>] (cppi41_dma_channel_abort [musb_hdrc]) from [<bf20c4bc>]
>> (musb_cleanup_urb+0x60/0x210 [musb_hdrc])
>>  r10:00000001 r9:c5fca010 r8:c5c67290 r7:a0000093 r6:00000080 r5:c55b5100
>>  r4:c5fca5a8
>> [<bf20c45c>] (musb_cleanup_urb [musb_hdrc]) from [<bf20ca8c>]
>> (musb_urb_dequeue+0xfc/0x164 [musb_hdrc])
>>  r10:00000001 r9:40408280 r8:c5fca010 r7:a0000093 r6:00000000 r5:c55b5380
>>  r4:c55b5100
>> [<bf20c990>] (musb_urb_dequeue [musb_hdrc]) from [<bf10fd7c>]
>> (unlink1+0x34/0x13c [usbcore])
>>  r10:00000100 r9:c5cb2a00 r8:c55b5100 r7:ffffff94 r6:c5c6c000 r5:c5dd2380
>>  r4:c55b5100 r3:bf20c990
>> [<bf10fd48>] (unlink1 [usbcore]) from [<bf112424>]
>> (usb_hcd_flush_endpoint+0x164/0x1a4 [usbcore])
>>  r9:c5cb2a00 r8:c55b5100 r7:c5c6c000 r6:c5dd2398 r5:c5dd2380 r4:ffffe000
>> [<bf1122c0>] (usb_hcd_flush_endpoint [usbcore]) from [<bf11563c>]
>> (usb_disable_endpoint+0x50/0x94 [usbcore])
>>  r9:c5cb2a00 r8:bf39e898 r7:00000000 r6:c5c43000 r5:c5c43000 r4:c5dd2380
>> [<bf1155ec>] (usb_disable_endpoint [usbcore]) from [<bf1156c4>]
>> (usb_disable_interface+0x44/0x58 [usbcore])
>>  r5:c5dd2348 r4:00000000
>> [<bf115680>] (usb_disable_interface [usbcore]) from [<bf118044>]
>> (usb_unbind_interface+0x1c0/0x268 [usbcore])
>>  r7:c5cb2c00 r6:bf39e898 r5:c5cb2c20 r4:c5cb2c20
>> [<bf117e84>] (usb_unbind_interface [usbcore]) from [<c02ce180>]
>> (device_release_driver_internal+0x160/0x208)
>>  r10:00000100 r9:c5cb2a00 r8:00000034 r7:00000000 r6:bf39e898 r5:c5cb2c54
>>  r4:c5cb2c20
>> [<c02ce020>] (device_release_driver_internal) from [<c02ce240>]
>> (device_release_driver+0x18/0x1c)
>>  r9:c5cb2a00 r8:c05f1008 r7:c5c43070 r6:bf12895c r5:c5cb2c20 r4:c5dc91ac
>> [<c02ce228>] (device_release_driver) from [<c02ccf70>]
>> (bus_remove_device+0xd0/0x100)
>> [<c02ccea0>] (bus_remove_device) from [<c02c9c60>] (device_del+0x120/0x378)
>>  r7:c5c43070 r6:c5cb2c28 r5:00000000 r4:c5cb2c20
>> [<c02c9b40>] (device_del) from [<bf11576c>]
>> (usb_disable_device+0x94/0x1d8 [usbcore])
>>  r10:00000100 r9:c5cb2a00 r8:c5cb2c00 r7:c5c6c000 r6:00000001 r5:00000000
>>  r4:c5c43000
>> [<bf1156d8>] (usb_disable_device [usbcore]) from [<bf10bccc>]
>> (usb_disconnect+0xb4/0x244 [usbcore])
>>  r9:c5cb2a00 r8:c5c2d600 r7:c5c430a4 r6:c5c43070 r5:c5c43000 r4:00000000
>> [<bf10bc18>] (usb_disconnect [usbcore]) from [<bf10d788>]
>> (hub_event+0x678/0x11dc [usbcore])
>>  r10:00000100 r9:c05f1008 r8:c5c2dcf8 r7:c68fc400 r6:00000001 r5:40000000
>>  r4:00000000
>> [<bf10d110>] (hub_event [usbcore]) from [<c0031db0>]
>> (process_one_work+0x1d8/0x41c)
>>  r10:00000008 r9:00000000 r8:c060b1d0 r7:00000000 r6:c7ede200 r5:c6882480
>>  r4:c5c2dcf8
>> [<c0031bd8>] (process_one_work) from [<c0032030>] (worker_thread+0x3c/0x670)
>>  r10:00000008 r9:c060b1e4 r8:c061ab60 r7:c6882498 r6:ffffe000 r5:c060b1d0
>>  r4:c6882480
>> [<c0031ff4>] (worker_thread) from [<c0038270>] (kthread+0x134/0x14c)
>>  r10:c683fe90 r9:c0031ff4 r8:c6882480 r7:c6896000 r6:00000000 r5:c687bd80
>>  r4:c6803e80
>> [<c003813c>] (kthread) from [<c00090e0>] (ret_from_fork+0x14/0x34)
>> Exception stack(0xc6897fb0 to 0xc6897ff8)
>> 7fa0:                                     00000000 00000000 00000000 00000000
>> 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>>  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c003813c
>>  r4:c687bd80
>> ---[ end trace cb9c7f93aaa3ed36 ]---
>> ------------[ cut here ]------------
>>
>>
>> adam
>>
>
> Hi Adam,
>
> this is a known issue and it's present in mainline and even in TI BSP.
> It's been deprioritized for now, but I'll return to debugging it once
> the common clock conversion is complete.
>
> Thanks,
> Bartosz

Ugh, -ETOOEARLY

I meant the ohci dying during cpufreq bug, not the crash you posted.

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
Adam Ford March 19, 2018, 5:52 p.m. UTC | #9
On Mon, Mar 19, 2018 at 11:15 AM, Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
> 2018-03-19 17:14 GMT+01:00 Bartosz Golaszewski <bgolaszewski@baylibre.com>:
>> 2018-03-19 17:11 GMT+01:00 Adam Ford <aford173@gmail.com>:
>>> On Mon, Mar 19, 2018 at 10:59 AM, David Lechner <david@lechnology.com> wrote:
>>>> On 03/19/2018 08:17 AM, Bartosz Golaszewski wrote:
>>>>>
>>>>> 2018-03-16 3:52 GMT+01:00 David Lechner <david@lechnology.com>:
>>>>>>
>>>>>> 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.
>>>>>>
>>>>>> This series has been tested on LEGO MINDSTORMS EV3 (device tree) and TI
>>>>>> OMAP-L138 LCDK (both device tree and legacy board file).
>>>>>>


Does anyone have an LCD connected to the LCDC controller with device
tree?  I posted an RFC patch a while ago for the DA850-EVM, but I got
distracted and forgot about it, so I never working on getting the
patch ready for acceptance.

I am trying to test the LCD now, but I cannot get the screen to come
up, but in the process, it appears as if the clocking to the LCD isn't
quite right.  I know it used to work, but I am going to probe some
pins, but I am getting warning messages I have never received before.
The desired clock frequency is 9000000, but when I use the cpufreq in
ondemand mode, I get the following messages:

#  echo ondemand > /sys/devices/system/cpu/cpufreq/policy0/scaling_governor
# tilcdc 1e13000.display: tilcdc_crtc_irq(0x00000161): FIFO underflow
tilcdc 1e13000.display: effective pixel clock rate (50000000Hz) differs from the
 calculated rate (54000000Hz)
tilcdc 1e13000.display: effective pixel clock rate (50000000Hz) differs from the
 calculated rate (54000000Hz)
tilcdc 1e13000.display: tilcdc_crtc_irq(0x00000161): FIFO underflow
tilcdc 1e13000.display: tilcdc_crtc_irq(0x00000161): FIFO underflow
tilcdc 1e13000.display: effective pixel clock rate (50000000Hz) differs from the
 calculated rate (54000000Hz)
tilcdc 1e13000.display: effective pixel clock rate (50000000Hz) differs from the
 calculated rate (54000000Hz)
tilcdc 1e13000.display: tilcdc_crtc_irq(0x00000161): FIFO underflow
tilcdc 1e13000.display: tilcdc_crtc_irq(0x00000161): FIFO underflow
tilcdc 1e13000.display: effective pixel clock rate (50000000Hz) differs from the
 calculated rate (54000000Hz)
tilcdc 1e13000.display: effective pixel clock rate (50000000Hz) differs from the
 calculated rate (54000000Hz)
tilcdc 1e13000.display: tilcdc_crtc_irq(0x00000161): FIFO underflow

As ondemend is used and the processor scaling happens, the above
message appears on and off.

I do not know if it impacts the LCD image since I haven't been able to
get it working yet, but I'll troubleshoot it and when/if I can get the
LCD working, I'll turn on the ondemand again and see how it behaves.

adam


>>>>>
>>>>> Hi David,
>>>>>
>>>>> thanks, with the u-boot fix everything seems to work fine. I tested
>>>>> da850-lcdk and da850-evm boards in DT and board file modes.
>>>>>
>>>
>>> I tested MMC and Ethernet.  Both appear to be operating normally, but
>>> I have having USB issues.
>>>
>>> Should I be testing cpufreq?  I noticed that when changing the
>>> frequencies, the USB appears to stop recognizing connections and
>>> disconnections.
>>>
>>> I did some testing on the DA850-EVM with USB from your repo, and I'm
>>> getting some crashing on MUSB unplug that do not appear in the
>>> 4.16-RC6 from linux-stable:
>>>
>>>
>>> ------------[ cut here ]------------
>>> WARNING: CPU: 0 PID: 13 at drivers/dma/cppi41.c:694
>>> cppi41_stop_chan+0x1f4/0x378 [cppi41]
>>> Modules linked in: evbug evdev mousedev hid_generic usbhid hid
>>> usb_f_ss_lb g_zero libcomposite configfs ofpart cmdlinepart m25p80
>>> spi_nor cppi41 adv7343 tvp514x vpif_display vpif_capture
>>> videobuf2_dma_con
>>> tig videobuf2_memops videobuf2_v4l2 videobuf2_common v4l2_fwnode
>>> v4l2_common snd_soc_simple_card tilcdc spi_davinci
>>> snd_soc_simple_card_utils videodev pwm_tiecap spi_bitbang da8xx
>>> drm_kms_helper phy_gener
>>> ic syscopyarea ohci_da8xx musb_hdrc sysfillrect sysimgblt fb_sys_fops
>>> ohci_hcd udc_core drm usbcore drm_panel_orientation_quirks usb_common
>>> snd_soc_tlv320aic3x vpif snd_soc_davinci_mcasp snd_soc_edma snd_
>>> soc_core snd_pcm_dmaengine snd_pcm rtc_omap snd_timer snd soundcore
>>> phy_da8xx_usb davinci_nand nand nand_ecc mtd pwm_bl backlight
>>> cpufreq_dt ti_aemif
>>> CPU: 0 PID: 13 Comm: kworker/0:1 Not tainted 4.16.0-rc5-g291ba8b-dirty #3
>>> Hardware name: Generic DA850/OMAP-L138/AM18x
>>> Workqueue: usb_hub_wq hub_event [usbcore]
>>> Backtrace:
>>> [<c000df94>] (dump_backtrace) from [<c000e264>] (show_stack+0x18/0x1c)
>>>  r7:00000009 r6:00000000 r5:bf339ad0 r4:00000000
>>> [<c000e24c>] (show_stack) from [<c04864c0>] (dump_stack+0x20/0x28)
>>> [<c04864a0>] (dump_stack) from [<c001ac38>] (__warn+0xd4/0xfc)
>>> [<c001ab64>] (__warn) from [<c001ad7c>] (warn_slowpath_null+0x44/0x50)
>>>  r8:c6c43080 r7:c05f1008 r6:bf338744 r5:000002b6 r4:bf339ad0
>>> [<c001ad38>] (warn_slowpath_null) from [<bf338744>]
>>> (cppi41_stop_chan+0x1f4/0x378 [cppi41])
>>>  r6:c5e59410 r5:c5e59420 r4:c5feca50
>>> [<bf338550>] (cppi41_stop_chan [cppi41]) from [<bf213dac>]
>>> (cppi41_dma_channel_abort+0xe0/0x2a8 [musb_hdrc])
>>>  r8:c6929180 r7:00000000 r6:c5c67290 r5:bf2175a0 r4:00000008
>>> [<bf213ccc>] (cppi41_dma_channel_abort [musb_hdrc]) from [<bf20c4bc>]
>>> (musb_cleanup_urb+0x60/0x210 [musb_hdrc])
>>>  r10:00000001 r9:c5fca010 r8:c5c67290 r7:a0000093 r6:00000080 r5:c55b5100
>>>  r4:c5fca5a8
>>> [<bf20c45c>] (musb_cleanup_urb [musb_hdrc]) from [<bf20ca8c>]
>>> (musb_urb_dequeue+0xfc/0x164 [musb_hdrc])
>>>  r10:00000001 r9:40408280 r8:c5fca010 r7:a0000093 r6:00000000 r5:c55b5380
>>>  r4:c55b5100
>>> [<bf20c990>] (musb_urb_dequeue [musb_hdrc]) from [<bf10fd7c>]
>>> (unlink1+0x34/0x13c [usbcore])
>>>  r10:00000100 r9:c5cb2a00 r8:c55b5100 r7:ffffff94 r6:c5c6c000 r5:c5dd2380
>>>  r4:c55b5100 r3:bf20c990
>>> [<bf10fd48>] (unlink1 [usbcore]) from [<bf112424>]
>>> (usb_hcd_flush_endpoint+0x164/0x1a4 [usbcore])
>>>  r9:c5cb2a00 r8:c55b5100 r7:c5c6c000 r6:c5dd2398 r5:c5dd2380 r4:ffffe000
>>> [<bf1122c0>] (usb_hcd_flush_endpoint [usbcore]) from [<bf11563c>]
>>> (usb_disable_endpoint+0x50/0x94 [usbcore])
>>>  r9:c5cb2a00 r8:bf39e898 r7:00000000 r6:c5c43000 r5:c5c43000 r4:c5dd2380
>>> [<bf1155ec>] (usb_disable_endpoint [usbcore]) from [<bf1156c4>]
>>> (usb_disable_interface+0x44/0x58 [usbcore])
>>>  r5:c5dd2348 r4:00000000
>>> [<bf115680>] (usb_disable_interface [usbcore]) from [<bf118044>]
>>> (usb_unbind_interface+0x1c0/0x268 [usbcore])
>>>  r7:c5cb2c00 r6:bf39e898 r5:c5cb2c20 r4:c5cb2c20
>>> [<bf117e84>] (usb_unbind_interface [usbcore]) from [<c02ce180>]
>>> (device_release_driver_internal+0x160/0x208)
>>>  r10:00000100 r9:c5cb2a00 r8:00000034 r7:00000000 r6:bf39e898 r5:c5cb2c54
>>>  r4:c5cb2c20
>>> [<c02ce020>] (device_release_driver_internal) from [<c02ce240>]
>>> (device_release_driver+0x18/0x1c)
>>>  r9:c5cb2a00 r8:c05f1008 r7:c5c43070 r6:bf12895c r5:c5cb2c20 r4:c5dc91ac
>>> [<c02ce228>] (device_release_driver) from [<c02ccf70>]
>>> (bus_remove_device+0xd0/0x100)
>>> [<c02ccea0>] (bus_remove_device) from [<c02c9c60>] (device_del+0x120/0x378)
>>>  r7:c5c43070 r6:c5cb2c28 r5:00000000 r4:c5cb2c20
>>> [<c02c9b40>] (device_del) from [<bf11576c>]
>>> (usb_disable_device+0x94/0x1d8 [usbcore])
>>>  r10:00000100 r9:c5cb2a00 r8:c5cb2c00 r7:c5c6c000 r6:00000001 r5:00000000
>>>  r4:c5c43000
>>> [<bf1156d8>] (usb_disable_device [usbcore]) from [<bf10bccc>]
>>> (usb_disconnect+0xb4/0x244 [usbcore])
>>>  r9:c5cb2a00 r8:c5c2d600 r7:c5c430a4 r6:c5c43070 r5:c5c43000 r4:00000000
>>> [<bf10bc18>] (usb_disconnect [usbcore]) from [<bf10d788>]
>>> (hub_event+0x678/0x11dc [usbcore])
>>>  r10:00000100 r9:c05f1008 r8:c5c2dcf8 r7:c68fc400 r6:00000001 r5:40000000
>>>  r4:00000000
>>> [<bf10d110>] (hub_event [usbcore]) from [<c0031db0>]
>>> (process_one_work+0x1d8/0x41c)
>>>  r10:00000008 r9:00000000 r8:c060b1d0 r7:00000000 r6:c7ede200 r5:c6882480
>>>  r4:c5c2dcf8
>>> [<c0031bd8>] (process_one_work) from [<c0032030>] (worker_thread+0x3c/0x670)
>>>  r10:00000008 r9:c060b1e4 r8:c061ab60 r7:c6882498 r6:ffffe000 r5:c060b1d0
>>>  r4:c6882480
>>> [<c0031ff4>] (worker_thread) from [<c0038270>] (kthread+0x134/0x14c)
>>>  r10:c683fe90 r9:c0031ff4 r8:c6882480 r7:c6896000 r6:00000000 r5:c687bd80
>>>  r4:c6803e80
>>> [<c003813c>] (kthread) from [<c00090e0>] (ret_from_fork+0x14/0x34)
>>> Exception stack(0xc6897fb0 to 0xc6897ff8)
>>> 7fa0:                                     00000000 00000000 00000000 00000000
>>> 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>> 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>>>  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c003813c
>>>  r4:c687bd80
>>> ---[ end trace cb9c7f93aaa3ed36 ]---
>>> ------------[ cut here ]------------
>>>
>>>
>>> adam
>>>
>>
>> Hi Adam,
>>
>> this is a known issue and it's present in mainline and even in TI BSP.
>> It's been deprioritized for now, but I'll return to debugging it once
>> the common clock conversion is complete.
>>
>> Thanks,
>> Bartosz
>
> Ugh, -ETOOEARLY
>
> I meant the ohci dying during cpufreq bug, not the crash you posted.
>
> 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 March 19, 2018, 10:14 p.m. UTC | #10
On 03/19/2018 11:11 AM, Adam Ford wrote:
> On Mon, Mar 19, 2018 at 10:59 AM, David Lechner <david@lechnology.com> wrote:
>> On 03/19/2018 08:17 AM, Bartosz Golaszewski wrote:
>>>
>>> 2018-03-16 3:52 GMT+01:00 David Lechner <david@lechnology.com>:
>>>>
>>>> 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.
>>>>
>>>> This series has been tested on LEGO MINDSTORMS EV3 (device tree) and TI
>>>> OMAP-L138 LCDK (both device tree and legacy board file).
>>>>
>>>
>>> Hi David,
>>>
>>> thanks, with the u-boot fix everything seems to work fine. I tested
>>> da850-lcdk and da850-evm boards in DT and board file modes.
>>>
> 
> I tested MMC and Ethernet.  Both appear to be operating normally, but
> I have having USB issues.
> 
> Should I be testing cpufreq?  I noticed that when changing the
> frequencies, the USB appears to stop recognizing connections and
> disconnections.
> 
> I did some testing on the DA850-EVM with USB from your repo, and I'm
> getting some crashing on MUSB unplug that do not appear in the
> 4.16-RC6 from linux-stable:
> 
> 
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 13 at drivers/dma/cppi41.c:694
> cppi41_stop_chan+0x1f4/0x378 [cppi41]
> Modules linked in: evbug evdev mousedev hid_generic usbhid hid
> usb_f_ss_lb g_zero libcomposite configfs ofpart cmdlinepart m25p80
> spi_nor cppi41 adv7343 tvp514x vpif_display vpif_capture
> videobuf2_dma_con
> tig videobuf2_memops videobuf2_v4l2 videobuf2_common v4l2_fwnode
> v4l2_common snd_soc_simple_card tilcdc spi_davinci
> snd_soc_simple_card_utils videodev pwm_tiecap spi_bitbang da8xx
> drm_kms_helper phy_gener
> ic syscopyarea ohci_da8xx musb_hdrc sysfillrect sysimgblt fb_sys_fops
> ohci_hcd udc_core drm usbcore drm_panel_orientation_quirks usb_common
> snd_soc_tlv320aic3x vpif snd_soc_davinci_mcasp snd_soc_edma snd_
> soc_core snd_pcm_dmaengine snd_pcm rtc_omap snd_timer snd soundcore
> phy_da8xx_usb davinci_nand nand nand_ecc mtd pwm_bl backlight
> cpufreq_dt ti_aemif
> CPU: 0 PID: 13 Comm: kworker/0:1 Not tainted 4.16.0-rc5-g291ba8b-dirty #3
> Hardware name: Generic DA850/OMAP-L138/AM18x
> Workqueue: usb_hub_wq hub_event [usbcore]
> Backtrace:
> [<c000df94>] (dump_backtrace) from [<c000e264>] (show_stack+0x18/0x1c)
>   r7:00000009 r6:00000000 r5:bf339ad0 r4:00000000
> [<c000e24c>] (show_stack) from [<c04864c0>] (dump_stack+0x20/0x28)
> [<c04864a0>] (dump_stack) from [<c001ac38>] (__warn+0xd4/0xfc)
> [<c001ab64>] (__warn) from [<c001ad7c>] (warn_slowpath_null+0x44/0x50)
>   r8:c6c43080 r7:c05f1008 r6:bf338744 r5:000002b6 r4:bf339ad0
> [<c001ad38>] (warn_slowpath_null) from [<bf338744>]
> (cppi41_stop_chan+0x1f4/0x378 [cppi41])
>   r6:c5e59410 r5:c5e59420 r4:c5feca50
> [<bf338550>] (cppi41_stop_chan [cppi41]) from [<bf213dac>]
> (cppi41_dma_channel_abort+0xe0/0x2a8 [musb_hdrc])
>   r8:c6929180 r7:00000000 r6:c5c67290 r5:bf2175a0 r4:00000008
> [<bf213ccc>] (cppi41_dma_channel_abort [musb_hdrc]) from [<bf20c4bc>]
> (musb_cleanup_urb+0x60/0x210 [musb_hdrc])
>   r10:00000001 r9:c5fca010 r8:c5c67290 r7:a0000093 r6:00000080 r5:c55b5100
>   r4:c5fca5a8
> [<bf20c45c>] (musb_cleanup_urb [musb_hdrc]) from [<bf20ca8c>]
> (musb_urb_dequeue+0xfc/0x164 [musb_hdrc])
>   r10:00000001 r9:40408280 r8:c5fca010 r7:a0000093 r6:00000000 r5:c55b5380
>   r4:c55b5100
> [<bf20c990>] (musb_urb_dequeue [musb_hdrc]) from [<bf10fd7c>]
> (unlink1+0x34/0x13c [usbcore])
>   r10:00000100 r9:c5cb2a00 r8:c55b5100 r7:ffffff94 r6:c5c6c000 r5:c5dd2380
>   r4:c55b5100 r3:bf20c990
> [<bf10fd48>] (unlink1 [usbcore]) from [<bf112424>]
> (usb_hcd_flush_endpoint+0x164/0x1a4 [usbcore])
>   r9:c5cb2a00 r8:c55b5100 r7:c5c6c000 r6:c5dd2398 r5:c5dd2380 r4:ffffe000
> [<bf1122c0>] (usb_hcd_flush_endpoint [usbcore]) from [<bf11563c>]
> (usb_disable_endpoint+0x50/0x94 [usbcore])
>   r9:c5cb2a00 r8:bf39e898 r7:00000000 r6:c5c43000 r5:c5c43000 r4:c5dd2380
> [<bf1155ec>] (usb_disable_endpoint [usbcore]) from [<bf1156c4>]
> (usb_disable_interface+0x44/0x58 [usbcore])
>   r5:c5dd2348 r4:00000000
> [<bf115680>] (usb_disable_interface [usbcore]) from [<bf118044>]
> (usb_unbind_interface+0x1c0/0x268 [usbcore])
>   r7:c5cb2c00 r6:bf39e898 r5:c5cb2c20 r4:c5cb2c20
> [<bf117e84>] (usb_unbind_interface [usbcore]) from [<c02ce180>]
> (device_release_driver_internal+0x160/0x208)
>   r10:00000100 r9:c5cb2a00 r8:00000034 r7:00000000 r6:bf39e898 r5:c5cb2c54
>   r4:c5cb2c20
> [<c02ce020>] (device_release_driver_internal) from [<c02ce240>]
> (device_release_driver+0x18/0x1c)
>   r9:c5cb2a00 r8:c05f1008 r7:c5c43070 r6:bf12895c r5:c5cb2c20 r4:c5dc91ac
> [<c02ce228>] (device_release_driver) from [<c02ccf70>]
> (bus_remove_device+0xd0/0x100)
> [<c02ccea0>] (bus_remove_device) from [<c02c9c60>] (device_del+0x120/0x378)
>   r7:c5c43070 r6:c5cb2c28 r5:00000000 r4:c5cb2c20
> [<c02c9b40>] (device_del) from [<bf11576c>]
> (usb_disable_device+0x94/0x1d8 [usbcore])
>   r10:00000100 r9:c5cb2a00 r8:c5cb2c00 r7:c5c6c000 r6:00000001 r5:00000000
>   r4:c5c43000
> [<bf1156d8>] (usb_disable_device [usbcore]) from [<bf10bccc>]
> (usb_disconnect+0xb4/0x244 [usbcore])
>   r9:c5cb2a00 r8:c5c2d600 r7:c5c430a4 r6:c5c43070 r5:c5c43000 r4:00000000
> [<bf10bc18>] (usb_disconnect [usbcore]) from [<bf10d788>]
> (hub_event+0x678/0x11dc [usbcore])
>   r10:00000100 r9:c05f1008 r8:c5c2dcf8 r7:c68fc400 r6:00000001 r5:40000000
>   r4:00000000
> [<bf10d110>] (hub_event [usbcore]) from [<c0031db0>]
> (process_one_work+0x1d8/0x41c)
>   r10:00000008 r9:00000000 r8:c060b1d0 r7:00000000 r6:c7ede200 r5:c6882480
>   r4:c5c2dcf8
> [<c0031bd8>] (process_one_work) from [<c0032030>] (worker_thread+0x3c/0x670)
>   r10:00000008 r9:c060b1e4 r8:c061ab60 r7:c6882498 r6:ffffe000 r5:c060b1d0
>   r4:c6882480
> [<c0031ff4>] (worker_thread) from [<c0038270>] (kthread+0x134/0x14c)
>   r10:c683fe90 r9:c0031ff4 r8:c6882480 r7:c6896000 r6:00000000 r5:c687bd80
>   r4:c6803e80
> [<c003813c>] (kthread) from [<c00090e0>] (ret_from_fork+0x14/0x34)
> Exception stack(0xc6897fb0 to 0xc6897ff8)
> 7fa0:                                     00000000 00000000 00000000 00000000
> 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>   r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c003813c
>   r4:c687bd80
> ---[ end trace cb9c7f93aaa3ed36 ]---
> ------------[ cut here ]------------
> 
> 

Hi Alexandre,

Do you have any insight on this error? I seem to recall something like this
when you were working on DA8xx MUSB last year.

I am wondering if this warning should be a warning in the kernel since it
seems like stopping CPPI when you unplug a cable even if it is busy is a
legitimate thing to do.
--
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 March 19, 2018, 10:54 p.m. UTC | #11
On 03/19/2018 12:52 PM, Adam Ford wrote:
> On Mon, Mar 19, 2018 at 11:15 AM, Bartosz Golaszewski
> <bgolaszewski@baylibre.com> wrote:
>> 2018-03-19 17:14 GMT+01:00 Bartosz Golaszewski <bgolaszewski@baylibre.com>:
>>> 2018-03-19 17:11 GMT+01:00 Adam Ford <aford173@gmail.com>:
>>>> On Mon, Mar 19, 2018 at 10:59 AM, David Lechner <david@lechnology.com> wrote:
>>>>> On 03/19/2018 08:17 AM, Bartosz Golaszewski wrote:
>>>>>>
>>>>>> 2018-03-16 3:52 GMT+01:00 David Lechner <david@lechnology.com>:
>>>>>>>
>>>>>>> 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.
>>>>>>>
>>>>>>> This series has been tested on LEGO MINDSTORMS EV3 (device tree) and TI
>>>>>>> OMAP-L138 LCDK (both device tree and legacy board file).
>>>>>>>
> 
> 
> Does anyone have an LCD connected to the LCDC controller with device
> tree?  I posted an RFC patch a while ago for the DA850-EVM, but I got
> distracted and forgot about it, so I never working on getting the
> patch ready for acceptance.
> 
> I am trying to test the LCD now, but I cannot get the screen to come
> up, but in the process, it appears as if the clocking to the LCD isn't
> quite right.  I know it used to work, but I am going to probe some
> pins, but I am getting warning messages I have never received before.
> The desired clock frequency is 9000000, but when I use the cpufreq in
> ondemand mode, I get the following messages:
> 
> #  echo ondemand > /sys/devices/system/cpu/cpufreq/policy0/scaling_governor
> # tilcdc 1e13000.display: tilcdc_crtc_irq(0x00000161): FIFO underflow
> tilcdc 1e13000.display: effective pixel clock rate (50000000Hz) differs from the
>   calculated rate (54000000Hz)
> tilcdc 1e13000.display: effective pixel clock rate (50000000Hz) differs from the
>   calculated rate (54000000Hz)
> tilcdc 1e13000.display: tilcdc_crtc_irq(0x00000161): FIFO underflow
> tilcdc 1e13000.display: tilcdc_crtc_irq(0x00000161): FIFO underflow
> tilcdc 1e13000.display: effective pixel clock rate (50000000Hz) differs from the
>   calculated rate (54000000Hz)
> tilcdc 1e13000.display: effective pixel clock rate (50000000Hz) differs from the
>   calculated rate (54000000Hz)
> tilcdc 1e13000.display: tilcdc_crtc_irq(0x00000161): FIFO underflow
> tilcdc 1e13000.display: tilcdc_crtc_irq(0x00000161): FIFO underflow
> tilcdc 1e13000.display: effective pixel clock rate (50000000Hz) differs from the
>   calculated rate (54000000Hz)
> tilcdc 1e13000.display: effective pixel clock rate (50000000Hz) differs from the
>   calculated rate (54000000Hz)
> tilcdc 1e13000.display: tilcdc_crtc_irq(0x00000161): FIFO underflow
> 
> As ondemend is used and the processor scaling happens, the above
> message appears on and off.
> 
> I do not know if it impacts the LCD image since I haven't been able to
> get it working yet, but I'll troubleshoot it and when/if I can get the
> LCD working, I'll turn on the ondemand again and see how it behaves.
> 
> adam
> 
> 

I've just been using the VGA connector on the LCDK since that is the only
hardware I have that uses the LCDC controller and I haven't tried it with
ondemand CPU freq yet.

But, I do know this. The parent clock for the LCDC (PLL0 SYSCLK2) must be
(according to the TRM) set to a fixed ratio to the ARM clock (/2), so it
can only have certain rates. The tilcdc driver then tries to pick a
divider for that rate to get close enough to the requested 54MHz. Also,
this divider must be at least 2. It can't be 1 (or 0).

So, if the CPU throttles down to 100, 200, or 300MHz, then 50Mz is as
close as any integer divider can get. The kernel prints a warning if
the difference between the requested and actual rate is over 5%. There
is a note in the kernel comments that this 5% value is arbitrary, so
maybe it needs to change to 10%?

I haven't dug deep enough to understand why the driver thinks it needs
a 54MHz pixel when you think it should be 9MHz.

I am also occasionally seeing the underflow error when the CPU is busy.
Maybe there is some more tweaking that could be done with the master
priority controller (unrelated to this patch series)? Or maybe if you
want to use the LCDC, then you just need to run at 475MHz all of the
time?


--
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
Stephen Boyd March 20, 2018, 12:53 a.m. UTC | #12
Quoting David Lechner (2018-03-15 19:52:16)
> 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.

Should I apply the first 19 patches to clk tree? Looking over them
nothing stands out except for your self comment about the bad
reviewed-by tag which I can remove.
--
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 March 20, 2018, 1:52 p.m. UTC | #13
Hi Stephen,

On Tuesday 20 March 2018 06:23 AM, Stephen Boyd wrote:
> Quoting David Lechner (2018-03-15 19:52:16)
>> 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.
> 
> Should I apply the first 19 patches to clk tree? Looking over them
> nothing stands out except for your self comment about the bad
> reviewed-by tag which I can remove.

I think it will be a good idea to go ahead merge the drivers/clk/ parts
for v4.17.

I have been traveling, and have not been able to review this version.
But I have closely reviewed previous versions. If there really are
issues, I am sure we can get follow-on patches to fix those.

But getting these into v4.17 will enable platform parts to come in
easily into v4.18

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 March 20, 2018, 4:57 p.m. UTC | #14
On 03/20/2018 11:54 AM, Stephen Boyd wrote:
> Quoting David Lechner (2018-03-15 19:52:34)
>> +
>> +typedef int (*da8xx_cfgchip_init)(struct device *dev, void __iomem *base);
> 
> Should be struct regmap *regmap?

Yes. It looks like this was copied and pasted from one of the other drivers.

Thank you for fixing it.

> 
> I've squashed this in.
> 
> diff --git a/drivers/clk/davinci/da8xx-cfgchip.c b/drivers/clk/davinci/da8xx-cfgchip.c
> index 858d3786b27b..c971111d2601 100644
> --- a/drivers/clk/davinci/da8xx-cfgchip.c
> +++ b/drivers/clk/davinci/da8xx-cfgchip.c
> @@ -736,7 +736,7 @@ static const struct platform_device_id da8xx_cfgchip_id_table[] = {
>   	{ }
>   };
>   
> -typedef int (*da8xx_cfgchip_init)(struct device *dev, void __iomem *base);
> +typedef int (*da8xx_cfgchip_init)(struct device *dev, struct regmap *regmap);
>   
>   static int da8xx_cfgchip_probe(struct platform_device *pdev)
>   {
> 

--
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
Stephen Boyd March 20, 2018, 5:03 p.m. UTC | #15
Quoting David Lechner (2018-03-15 19:52:18)
> This adds a new driver for mach-davinci PLL clocks. This is porting the
> code from arch/arm/mach-davinci/clock.c to the common clock framework.
> Additionally, it adds device tree support for these clocks.
> 
> The ifeq ($(CONFIG_COMMON_CLK), y) in the Makefile is needed to prevent
> compile errors until the clock code in arch/arm/mach-davinci is removed.
> 
> Note: although there are similar clocks for TI Keystone we are not able
> to share the code for a few reasons. The keystone clocks are device tree
> only and use legacy one-node-per-clock bindings. Also the register
> layouts are a bit different, which would add even more if/else mess
> to the keystone clocks. And the keystone PLL driver doesn't support
> setting clock rates.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:03 p.m. UTC | #16
Quoting David Lechner (2018-03-15 19:52:19)
> This adds platform-specific declarations for the PLL clocks on TI DA830/
> OMAP-L137/AM17XX SoCs.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:03 p.m. UTC | #17
Quoting David Lechner (2018-03-15 19:52:20)
> This adds platform-specific declarations for the PLL clocks on TI DA850/
> OMAP-L138/AM18XX SoCs.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:03 p.m. UTC | #18
Quoting David Lechner (2018-03-15 19:52:21)
> This adds platform-specific declarations for the PLL clocks on TI
> DM355 based systems.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:03 p.m. UTC | #19
Quoting David Lechner (2018-03-15 19:52:22)
> This adds platform-specific declarations for the PLL clocks on TI
> DM365 based systems.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:03 p.m. UTC | #20
Quoting David Lechner (2018-03-15 19:52:23)
> This adds platform-specific declarations for the PLL clocks on TI
> DM644x based systems.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:03 p.m. UTC | #21
Quoting David Lechner (2018-03-15 19:52:24)
> This adds platform-specific declarations for the PLL clocks on TI
> DM646x based systems.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:03 p.m. UTC | #22
Quoting David Lechner (2018-03-15 19:52:26)
> This adds a new driver for mach-davinci PSC clocks. This is porting the
> code from arch/arm/mach-davinci/psc.c to the common clock framework and
> is converting it to use regmap to simplify the code. Additionally, it
> adds device tree support for these clocks.
> 
> Note: although there are similar clocks for TI Keystone we are not able
> to share the code for a few reasons. The keystone clocks are device tree
> only and use legacy one-node-per-clock bindings. Also the keystone
> driver makes the assumption that there is only one PSC per SoC and uses
> global variables, but here we have two controllers per SoC.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:04 p.m. UTC | #23
Quoting David Lechner (2018-03-15 19:52:27)
> This adds platform-specific declarations for the PSC clocks on TI DA830/
> OMAP-L137/AM17XX SoCs.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:04 p.m. UTC | #24
Quoting David Lechner (2018-03-15 19:52:28)
> This adds platform-specific declarations for the PSC clocks on TI DA850/
> OMAP-L138/AM18XX SoCs.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> Reviewed-by: Sekhar Nori <nsekhar@ti.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:04 p.m. UTC | #25
Quoting David Lechner (2018-03-15 19:52:29)
> This adds platform-specific declarations for the PSC clocks on TI
> DM355 based systems.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:04 p.m. UTC | #26
Quoting David Lechner (2018-03-15 19:52:30)
> This adds platform-specific declarations for the PSC clocks on TI
> DM365 based systems.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> Reviewed-by: Sekhar Nori <nsekhar@ti.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:04 p.m. UTC | #27
Quoting David Lechner (2018-03-15 19:52:31)
> This adds platform-specific declarations for the PSC clocks on TI
> DM644x based systems.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:04 p.m. UTC | #28
Quoting David Lechner (2018-03-15 19:52:32)
> This adds platform-specific declarations for the PSC clocks on TI
> DM646x based systems.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:04 p.m. UTC | #29
Quoting David Lechner (2018-03-15 19:52:34)
> This adds a new driver for the gate and multiplexer clocks in the
> CFGCHIPn syscon registers on TI DA8XX-type SoCs.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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
Stephen Boyd March 20, 2018, 5:04 p.m. UTC | #30
Quoting David Lechner (2018-03-15 19:52:35)
> This adds a new driver for the USB PHY clocks in the CFGCHIP2 syscon
> register on TI DA8XX-type SoCs.
> 
> The USB0 (USB 2.0) PHY clock is an interesting case because it calls
> clk_enable() in a reentrant way. The USB 2.0 PSC only has to be enabled
> temporarily while we are locking the PLL, which takes place during the
> clk_enable() callback.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---

Applied to clk-next

--
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 April 2, 2018, 11:12 a.m. UTC | #31
On Friday 16 March 2018 10:50 PM, David Lechner wrote:
> On 03/15/2018 09:52 PM, David Lechner wrote:
>> This adds clock provider nodes for da850 and wires them up to all of the
>> devices.
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> ---
> 
> ...
> 
> This is the mcasp0: mcasp@100000 node...
> 
>> @@ -560,6 +720,7 @@
>>               dmas = <&edma0 1 1>,
>>                   <&edma0 0 1>;
>>               dma-names = "tx", "rx";
>> +            clocks = <&psc1 7>;
> 
> After some testing, it looks like it needs to be:
> 
> +            power-domains = <&psc1 7>;
> 
> instead of
> 
> +            clocks = <&psc1 7>;

We should probably have both clocks and power-domains properties for all
PSC clocks. This way, the driver can change without a corresponding DT
update dependency.

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 April 2, 2018, 4:15 p.m. UTC | #32
On 04/02/2018 06:12 AM, Sekhar Nori wrote:
> On Friday 16 March 2018 10:50 PM, David Lechner wrote:
>> On 03/15/2018 09:52 PM, David Lechner wrote:
>>> This adds clock provider nodes for da850 and wires them up to all of the
>>> devices.
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>> ---
>>
>> ...
>>
>> This is the mcasp0: mcasp@100000 node...
>>
>>> @@ -560,6 +720,7 @@
>>>                dmas = <&edma0 1 1>,
>>>                    <&edma0 0 1>;
>>>                dma-names = "tx", "rx";
>>> +            clocks = <&psc1 7>;
>>
>> After some testing, it looks like it needs to be:
>>
>> +            power-domains = <&psc1 7>;
>>
>> instead of
>>
>> +            clocks = <&psc1 7>;
> 
> We should probably have both clocks and power-domains properties for all
> PSC clocks. This way, the driver can change without a corresponding DT
> update dependency.
> 
> Thanks,
> Sekhar
> 

That's fine with me. I just didn't know how people felt about using properties
that are not documented.

--
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 April 3, 2018, 5:43 a.m. UTC | #33
On Monday 02 April 2018 09:45 PM, David Lechner wrote:
> On 04/02/2018 06:12 AM, Sekhar Nori wrote:
>> On Friday 16 March 2018 10:50 PM, David Lechner wrote:
>>> On 03/15/2018 09:52 PM, David Lechner wrote:
>>>> This adds clock provider nodes for da850 and wires them up to all of
>>>> the
>>>> devices.
>>>>
>>>> Signed-off-by: David Lechner <david@lechnology.com>
>>>> ---
>>>
>>> ...
>>>
>>> This is the mcasp0: mcasp@100000 node...
>>>
>>>> @@ -560,6 +720,7 @@
>>>>                dmas = <&edma0 1 1>,
>>>>                    <&edma0 0 1>;
>>>>                dma-names = "tx", "rx";
>>>> +            clocks = <&psc1 7>;
>>>
>>> After some testing, it looks like it needs to be:
>>>
>>> +            power-domains = <&psc1 7>;
>>>
>>> instead of
>>>
>>> +            clocks = <&psc1 7>;
>>
>> We should probably have both clocks and power-domains properties for all
>> PSC clocks. This way, the driver can change without a corresponding DT
>> update dependency.
>>
>> Thanks,
>> Sekhar
>>
> 
> That's fine with me. I just didn't know how people felt about using
> properties
> that are not documented.

Good point. How to ease documentation for generic properties like these
was discussed in the past, but there is no guidance in
Documentation/devicetree/bindings that I can see.

So, in the interest of reduced controversy, its probably better to do
what you already have and only populate properties already documented in
the bindings.

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 April 3, 2018, 10:26 a.m. UTC | #34
On Friday 16 March 2018 08:22 AM, David Lechner wrote:
> +static struct resource dm644x_pll1_resources[] = {
> +	{
> +		.start	= DAVINCI_PLL1_BASE,
> +		.end	= DAVINCI_PLL1_BASE + SZ_4K - 1,

The .end should be DAVINCI_PLL1_BASE + SZ_1K - 1, otherwise it prevents
PLL2 from getting registered.

> +		.flags	= IORESOURCE_MEM,
> +	},
> +};
> +
> +static struct platform_device dm644x_pll1_device = {
> +	.name		= "dm644x-pll1",
> +	.id		= -1,
> +	.resource	= dm644x_pll1_resources,
> +	.num_resources	= ARRAY_SIZE(dm644x_pll1_resources),
> +};
> +
> +static struct resource dm644x_pll2_resources[] = {
> +	{
> +		.start	= DAVINCI_PLL2_BASE,
> +		.end	= DAVINCI_PLL2_BASE + SZ_4K - 1,

And this too should be fixed, else it prevents the PSC from getting
registered.

> +		.flags	= IORESOURCE_MEM,
> +	},
> +};

With these fixed, I still had to enable 'clk_ignore_unused' on DM644x
EVM to get to NFS boot. I think root of the problem is that pm_runtime()
APIs are not working in the legacy boot mode.

This can be seen even on the DA850 LCDK in legacy boot. pm_genpd_summary
in debugfs shows all domains are off and there are no devices registered
under the "da850-psc1: emac" domain. NFS mounting still works on the
DA850 LCDK because clk_summary shows enable and prepare count of 4 for
emac. Not sure how that's happening. But on DM644x EVM, the emac clock
enable count is 0.

Still looking at whats going wrong here. I am testing your v8 branch
with clk-davinci branch from clk-next merged to get the fixes Stephen made.

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 April 3, 2018, 4:30 p.m. UTC | #35
On 04/03/2018 05:26 AM, Sekhar Nori wrote:
> On Friday 16 March 2018 08:22 AM, David Lechner wrote:
>> +static struct resource dm644x_pll1_resources[] = {
>> +	{
>> +		.start	= DAVINCI_PLL1_BASE,
>> +		.end	= DAVINCI_PLL1_BASE + SZ_4K - 1,
> 
> The .end should be DAVINCI_PLL1_BASE + SZ_1K - 1, otherwise it prevents
> PLL2 from getting registered.
> 
>> +		.flags	= IORESOURCE_MEM,
>> +	},
>> +};
>> +
>> +static struct platform_device dm644x_pll1_device = {
>> +	.name		= "dm644x-pll1",
>> +	.id		= -1,
>> +	.resource	= dm644x_pll1_resources,
>> +	.num_resources	= ARRAY_SIZE(dm644x_pll1_resources),
>> +};
>> +
>> +static struct resource dm644x_pll2_resources[] = {
>> +	{
>> +		.start	= DAVINCI_PLL2_BASE,
>> +		.end	= DAVINCI_PLL2_BASE + SZ_4K - 1,
> 
> And this too should be fixed, else it prevents the PSC from getting
> registered.
> 
>> +		.flags	= IORESOURCE_MEM,
>> +	},
>> +};
> 
> With these fixed, I still had to enable 'clk_ignore_unused' on DM644x
> EVM to get to NFS boot. I think root of the problem is that pm_runtime()
> APIs are not working in the legacy boot mode.
> 
> This can be seen even on the DA850 LCDK in legacy boot. pm_genpd_summary
> in debugfs shows all domains are off and there are no devices registered
> under the "da850-psc1: emac" domain. NFS mounting still works on the
> DA850 LCDK because clk_summary shows enable and prepare count of 4 for
> emac. Not sure how that's happening. But on DM644x EVM, the emac clock
> enable count is 0.
> 
> Still looking at whats going wrong here. I am testing your v8 branch
> with clk-davinci branch from clk-next merged to get the fixes Stephen made.
> 

In legacy mode, genpd is not being used. I didn't see any mechanism for
genpd lookup without device tree. So, we are still relying on the
matching in arch/arm/mach-davinci/pm_domain.c.

I suspect we need to fix the clock lookups in
drivers/clk/davinci/psc-dm644x.c.

LPSC_CLKDEV2(emac_clkdev,		NULL,		"davinci_emac.1",
					"fck",		"davinci_mdio.0");

NULL might need to be changed to "fck" to be picked up by pm matching
and "davinci_emac.1" should be verified that it matches the actual EMAC
device name.
--
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 April 4, 2018, 6:47 a.m. UTC | #36
On Tuesday 03 April 2018 10:00 PM, David Lechner wrote:
> On 04/03/2018 05:26 AM, Sekhar Nori wrote:
>> On Friday 16 March 2018 08:22 AM, David Lechner wrote:
>>> +static struct resource dm644x_pll1_resources[] = {
>>> +    {
>>> +        .start    = DAVINCI_PLL1_BASE,
>>> +        .end    = DAVINCI_PLL1_BASE + SZ_4K - 1,
>>
>> The .end should be DAVINCI_PLL1_BASE + SZ_1K - 1, otherwise it prevents
>> PLL2 from getting registered.
>>
>>> +        .flags    = IORESOURCE_MEM,
>>> +    },
>>> +};
>>> +
>>> +static struct platform_device dm644x_pll1_device = {
>>> +    .name        = "dm644x-pll1",
>>> +    .id        = -1,
>>> +    .resource    = dm644x_pll1_resources,
>>> +    .num_resources    = ARRAY_SIZE(dm644x_pll1_resources),
>>> +};
>>> +
>>> +static struct resource dm644x_pll2_resources[] = {
>>> +    {
>>> +        .start    = DAVINCI_PLL2_BASE,
>>> +        .end    = DAVINCI_PLL2_BASE + SZ_4K - 1,
>>
>> And this too should be fixed, else it prevents the PSC from getting
>> registered.
>>
>>> +        .flags    = IORESOURCE_MEM,
>>> +    },
>>> +};
>>
>> With these fixed, I still had to enable 'clk_ignore_unused' on DM644x
>> EVM to get to NFS boot. I think root of the problem is that pm_runtime()
>> APIs are not working in the legacy boot mode.
>>
>> This can be seen even on the DA850 LCDK in legacy boot. pm_genpd_summary
>> in debugfs shows all domains are off and there are no devices registered
>> under the "da850-psc1: emac" domain. NFS mounting still works on the
>> DA850 LCDK because clk_summary shows enable and prepare count of 4 for
>> emac. Not sure how that's happening. But on DM644x EVM, the emac clock
>> enable count is 0.
>>
>> Still looking at whats going wrong here. I am testing your v8 branch
>> with clk-davinci branch from clk-next merged to get the fixes Stephen
>> made.
>>
> 
> In legacy mode, genpd is not being used. I didn't see any mechanism for

Ah, I got stumped by the genpd related debug entries popping up.
Probably something should be done to make sure they don't show up in
legacy boot. And some comments to that effect in psc.c will help.

> genpd lookup without device tree. So, we are still relying on the
> matching in arch/arm/mach-davinci/pm_domain.c.

This is fine. We just need legacy boot to keep working without regressions.

> 
> I suspect we need to fix the clock lookups in
> drivers/clk/davinci/psc-dm644x.c.
> 
> LPSC_CLKDEV2(emac_clkdev,        NULL,        "davinci_emac.1",
>                     "fck",        "davinci_mdio.0");
> 
> NULL might need to be changed to "fck" to be picked up by pm matching
> and "davinci_emac.1" should be verified that it matches the actual EMAC
> device name.

NULL con_id matches what we have for DA850 and also what we had for
DM644x prior to CCF conversion. So, I did not really suspect that. The
device name does match. I will check what else could be going on based
on your input.

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 April 4, 2018, 12:44 p.m. UTC | #37
On Wednesday 04 April 2018 12:17 PM, Sekhar Nori wrote:
> On Tuesday 03 April 2018 10:00 PM, David Lechner wrote:
>> On 04/03/2018 05:26 AM, Sekhar Nori wrote:
>>> On Friday 16 March 2018 08:22 AM, David Lechner wrote:
>>>> +static struct resource dm644x_pll1_resources[] = {
>>>> +    {
>>>> +        .start    = DAVINCI_PLL1_BASE,
>>>> +        .end    = DAVINCI_PLL1_BASE + SZ_4K - 1,
>>>
>>> The .end should be DAVINCI_PLL1_BASE + SZ_1K - 1, otherwise it prevents
>>> PLL2 from getting registered.
>>>
>>>> +        .flags    = IORESOURCE_MEM,
>>>> +    },
>>>> +};
>>>> +
>>>> +static struct platform_device dm644x_pll1_device = {
>>>> +    .name        = "dm644x-pll1",
>>>> +    .id        = -1,
>>>> +    .resource    = dm644x_pll1_resources,
>>>> +    .num_resources    = ARRAY_SIZE(dm644x_pll1_resources),
>>>> +};
>>>> +
>>>> +static struct resource dm644x_pll2_resources[] = {
>>>> +    {
>>>> +        .start    = DAVINCI_PLL2_BASE,
>>>> +        .end    = DAVINCI_PLL2_BASE + SZ_4K - 1,
>>>
>>> And this too should be fixed, else it prevents the PSC from getting
>>> registered.
>>>
>>>> +        .flags    = IORESOURCE_MEM,
>>>> +    },
>>>> +};
>>>
>>> With these fixed, I still had to enable 'clk_ignore_unused' on DM644x
>>> EVM to get to NFS boot. I think root of the problem is that pm_runtime()
>>> APIs are not working in the legacy boot mode.
>>>
>>> This can be seen even on the DA850 LCDK in legacy boot. pm_genpd_summary
>>> in debugfs shows all domains are off and there are no devices registered
>>> under the "da850-psc1: emac" domain. NFS mounting still works on the
>>> DA850 LCDK because clk_summary shows enable and prepare count of 4 for
>>> emac. Not sure how that's happening. But on DM644x EVM, the emac clock
>>> enable count is 0.
>>>
>>> Still looking at whats going wrong here. I am testing your v8 branch
>>> with clk-davinci branch from clk-next merged to get the fixes Stephen
>>> made.
>>>
>>
>> In legacy mode, genpd is not being used. I didn't see any mechanism for
> 
> Ah, I got stumped by the genpd related debug entries popping up.
> Probably something should be done to make sure they don't show up in
> legacy boot. And some comments to that effect in psc.c will help.
> 
>> genpd lookup without device tree. So, we are still relying on the
>> matching in arch/arm/mach-davinci/pm_domain.c.
> 
> This is fine. We just need legacy boot to keep working without regressions.
> 
>>
>> I suspect we need to fix the clock lookups in
>> drivers/clk/davinci/psc-dm644x.c.
>>
>> LPSC_CLKDEV2(emac_clkdev,        NULL,        "davinci_emac.1",
>>                     "fck",        "davinci_mdio.0");
>>
>> NULL might need to be changed to "fck" to be picked up by pm matching
>> and "davinci_emac.1" should be verified that it matches the actual EMAC
>> device name.
> 
> NULL con_id matches what we have for DA850 and also what we had for
> DM644x prior to CCF conversion. So, I did not really suspect that. The
> device name does match. I will check what else could be going on based
> on your input.

This issue is because emac platform device is getting registered really
early on dm644x even before the clocks are ready in postcore_initcall().

I will send a patch fixing that.

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 April 5, 2018, 11:30 a.m. UTC | #38
On Friday 16 March 2018 08:22 AM, David Lechner wrote:
> +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. This currently always happens because platform
> +		 * clocks (i.e PLLs and PSCs) are registered as platform
> +		 * devices and therefore are not available at this point in
> +		 * the boot process.

It seems to me that this is not going to be a temporary problem (or at
least will be around for quite a while). So, I think we can as well just
look for ref_clk directly.

> +		 */
> +		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);
> +	}
> +
> +	davinci_timer_init(clk);
> +
> +	return 0;
> +}
> +TIMER_OF_DECLARE(davinci_timer, "ti,davinci-timer", of_davinci_timer_init);

Here, I think we should use "ti,da850-timer" so we can change the fixed
clock we are looking for based on the SoC. At a minimum, we should have
"ti,da850-timer" in the DT along with "ti,davinci-timer".

BTW, I noticed that "ti,davinci-timer" is not documented. I think we
need to add a binding documentation too.

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 April 5, 2018, 12:12 p.m. UTC | #39
On Friday 16 March 2018 08:22 AM, David Lechner wrote:
> This changes davinci_timer_init() so that we pass the clock as a
> parameter instead of using clk_get(). This is done in preparation
> for converting to the common clock framework.
> 
> It removes the requirement that we have to have a clock with con_id
> of "timer0", which will be good for DT bindings since clock-names =
> "timer0" doesn't really make sense.
> 
> Additionally, when we convert to the common clock framework, most of
> the clocks will be platform devices, which will not be available at
> this point during the boot process, so we will need to pass a clock
> that is available (i.e. ref_clk) instead of the "timer0" clock.
> 
> NB: The comment added in time.c is not entirely true when this patch
> is applied, but it will be correct once the conversion to the common
> clock framework is complete in subsequent patches.

I think its better to add the comment when its actually applicable, even
if it needs to be a separate patch.

> 
> Also, drop use of extern in header file since we are touching the
> definition.
> 
> Signed-off-by: David Lechner <david@lechnology.com>

> -
> -void __init davinci_timer_init(void)
> +void __init davinci_timer_init(struct clk *timer_clk)
>  {
> -	struct clk *timer_clk;
>  	struct davinci_soc_info *soc_info = &davinci_soc_info;
>  	unsigned int clockevent_id;
>  	unsigned int clocksource_id;
> @@ -373,7 +371,14 @@ void __init davinci_timer_init(void)
>  		}
>  	}
>  
> -	timer_clk = clk_get(NULL, "timer0");
> +	/*
> +	 * REVISIT: Currently, timer_clk will be "ref_clk". However, the actual
> +	 * clock for this device comes from a PLL AUXCLK or a PSC clock
> +	 * (depending on the SoC). The PLL and PSC clocks are not registered
> +	 * until later in boot because they are platform devices. We should try
> +	 * again later to get the real clock so that the real clock is not
> +	 * turned off when disabling unused clocks, which would stop the timer.

This disabling later should not happen because you have the timer clocks
marked as LPSC_ALWAYS_ENABLED which translates to CLK_IS_CRITICAL and
that should make sure they are never disabled.

The issue should be documented is that we depend on bootloader leaving
the timer0 enabled on DM355, DM365, DM644x and DM646x. Of these, I have
tested only DM644x so far. I will check others and see the status of
timer0 there.

Its not really a great situation here, but I don't have a good
alternative to suggest as well if having genpd support in PSC means we
cannot be using CLK_OF_DECLARE().

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