dts: Add initial support of USB PHYs #10359

ydamigos · 2018-10-03T14:40:39Z

Add initial support of USB PHYs.

ydamigos · 2018-10-03T14:45:20Z

codecov-io · 2018-10-03T15:08:09Z

Codecov Report

Merging #10359 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #10359   +/-   ##
=======================================
  Coverage   48.37%   48.37%           
=======================================
  Files         265      265           
  Lines       42188    42188           
  Branches    10137    10137           
=======================================
  Hits        20408    20408           
  Misses      17703    17703           
  Partials     4077     4077

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a933297...918015b. Read the comment docs.

galak

When phys has multiple handles (like on stm32f723.dtsi) what does that mean?

ydamigos · 2018-10-03T17:42:38Z

When phys has multiple handles (like on stm32f723.dtsi) what does that mean?

The available PHYs for the USB controller. In case of stm32f723 the available PHYs are:
– An on-chip full-speed PHY
– A UTMI interface for internal HS PHY

erwango · 2018-10-04T10:04:30Z

@ydamigos , looks nice, but it is not clear to me what issue this addition is going to solve, and no application is provided to demonstrate.
Any pointer on a discussion that could help to understand the context?

ydamigos · 2018-10-04T12:40:04Z

Any pointer on a discussion that could help to understand the context?

@erwango It needs PR #9062 to get merged. Then, we could use DT to get hw information for USBPHYC and not stm32cube in stm32 usb driver.

galak · 2018-10-09T17:00:00Z

@erwango It needs PR #9062 to get merged. Then, we could use DT to get hw information for USBPHYC and not stm32cube in stm32 usb driver.

merged #9062 :)

aurel32 · 2018-10-10T20:28:52Z

Thanks @ydamigos for working on that. I have to say I don't necessary understand all the logic parsing the device tree to produce an header file so I still find a bit difficult to understand how the new DT entries are going to be used in practice. Anyway I already have a few comments/questions:

The STM32F723 OTG_HS interface only has a single HS PHY (that can work in FS mode). The documentation about HS PHY is quite confusing, and even wrong in some aspects. Therefore I believe the usbotg_hs phys entry has to be changed to:
phys = <&usbphyc>;
The usbotg_fs_phy basically has no address and no property, so the same is used for both the usbotg_hs and usbotg_fs instances. Is it a problem? In other part of the device tree we tend to have one instance per device, post-fixed with a number.
I guess in the future we can add support for an external PHY in the device tree, which in that case will be outside of the soc hierarchy. Is that correct?
When compiling with your patches applied (after fixing the conflicts), I get the following warnings:

stm32f723e_disco.dts_compiled: Warning (phys_property): /soc/usb@50000000: Missing property '#phy-cells' in node /soc/usbphy or bad phandle (referred from phys[0])
stm32f723e_disco.dts_compiled: Warning (phys_property): /soc/usb@40040000: Missing property '#phy-cells' in node /soc/usbphy or bad phandle (referred from phys[0])

ydamigos · 2018-10-11T17:24:02Z

@aurel32 Thanks for the review

The STM32F723 OTG_HS interface only has a single HS PHY (that can work in FS mode). The documentation about HS PHY is quite confusing, and even wrong in some aspects. Therefore I believe the usbotg_hs phys entry has to be changed to:
phys = <&usbphyc>;

You are right. I updated it.

The usbotg_fs_phy basically has no address and no property, so the same is used for both the usbotg_hs and usbotg_fs instances. Is it a problem? In other part of the device tree we tend to have one instance per device, post-fixed with a number.

Yes, I should use a different one. It is a different embedded phy in a different controller.

I guess in the future we can add support for an external PHY in the device tree, which in that case will be outside of the soc hierarchy. Is that correct?

Yes, on board level.

When compiling with your patches applied (after fixing the conflicts)

I have seen the warnings and I'll fix them. I thought #phy-cells was used in linux only but it is needed by DT compiler.

ydamigos · 2018-10-17T17:02:39Z

When phys has multiple handles (like on stm32f723.dtsi) what does that mean?

I had a better look at linux phy bindings and a controller has multiple phys if it can use simultaneously.

erwango

Great work.
Only adaptation would be to use CONFIG_DT_COMPAT_XX flags instead of CONFIG_XXX_BASE_ADDRESS

erwango · 2018-11-07T08:18:06Z

drivers/usb/device/usb_dc_stm32.c

@@ -258,14 +258,14 @@ static int usb_dc_stm32_clock_enable(void)
 #ifdef CONFIG_USB_HS_BASE_ADDRESS


-#ifdef USB_HS_PHYC
+#ifdef CONFIG_USBPHYC_BASE_ADDRESS


We're now generating CONFIG_DT_COMPAT_XXXX flags (more details here: #9406).
These should be used for driver specific implementations/variations. Since you've set the ground with phys nodes it would fit perfectly here.
Please use this instead of 'CONFIG_USBPHYC_BASE_ADDRESS'.
Apply everywhere.

I changed it to DT_COMPAT_ST_STM32_USBPHYC.

galak

Curious, why the use of "zephyr,usb-nop-xceiv" as a compatible (specifically the 'zephyr' bit?)

ydamigos · 2018-11-13T15:35:43Z

Curious, why the use of "zephyr,usb-nop-xceiv" as a compatible (specifically the 'zephyr' bit?)

@galak I borrowed the idea from linux, where usb-nop-xceiv is used by all the usb transceivers which are built-in with USB IP. I added the 'zephyr' part to highlight that it is a zephyr "implementation".

galak · 2018-11-13T17:24:02Z

@galak I borrowed the idea from linux, where usb-nop-xceiv is used by all the usb transceivers which are built-in with USB IP. I added the 'zephyr' part to highlight that it is a zephyr "implementation".

what compat does linux use?

ydamigos · 2018-11-13T20:17:53Z

what compat does linux use?

usb-nop-xceiv

galak · 2018-11-14T12:55:29Z

usb-nop-xceiv

Any reason not to use the same, is there some zephyr specific semantic we have?

Add yaml file for initial support of USB PHYs. It adds also yaml files for STM32 USBPHYC and USB controllers with embedded PHY. PHY refers to the physical layer which is used to connect a device to the physical medium e.g. USB controller. Origin: original Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com>

Add phys property, which specifies USB PHY provider, to STM32 USB yaml files. Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com>

Add USB phy nodes and phys property in USB nodes. Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com>

Use DT to check if SoC has a USBPHYC controller. Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com>

ydamigos · 2018-11-14T17:32:48Z

Any reason not to use the same, is there some zephyr specific semantic we have?

No, there is nothing zephyr specific. I changed it to usb-nop-xceiv.

ydamigos added area: Devicetree platform: STM32 ST Micro STM32 DNM This PR should not be merged (Do Not Merge) labels Oct 3, 2018

ydamigos requested review from aurel32, galak and erwango October 3, 2018 14:40

ydamigos changed the title ~~dts: Add initial support of USB PHYs~~ [DNM] dts: Add initial support of USB PHYs Oct 3, 2018

ydamigos mentioned this pull request Oct 3, 2018

dts: bindings: usb: Add maximum-speed property #9062

Merged

galak reviewed Oct 3, 2018

View reviewed changes

ydamigos force-pushed the usb-phys branch from 6a84067 to 8a2031e Compare October 11, 2018 17:10

ydamigos force-pushed the usb-phys branch from 8a2031e to 7958f21 Compare October 11, 2018 18:24

ydamigos changed the title ~~[DNM] dts: Add initial support of USB PHYs~~ [WIP] dts: Add initial support of USB PHYs Oct 12, 2018

ydamigos force-pushed the usb-phys branch 2 times, most recently from c2bd153 to 0f36059 Compare October 15, 2018 18:58

ydamigos requested a review from loicpoulain as a code owner October 15, 2018 18:58

ydamigos force-pushed the usb-phys branch 2 times, most recently from 0c51f42 to 2df02d0 Compare October 17, 2018 16:52

ydamigos force-pushed the usb-phys branch from 2df02d0 to 6c73a44 Compare October 17, 2018 18:26

ydamigos removed the DNM This PR should not be merged (Do Not Merge) label Oct 17, 2018

ydamigos changed the title ~~[WIP] dts: Add initial support of USB PHYs~~ dts: Add initial support of USB PHYs Oct 17, 2018

erwango requested changes Nov 7, 2018

View reviewed changes

ydamigos force-pushed the usb-phys branch from 6c73a44 to cd58dcc Compare November 7, 2018 19:14

erwango approved these changes Nov 12, 2018

View reviewed changes

galak reviewed Nov 13, 2018

View reviewed changes

ydamigos force-pushed the usb-phys branch from cd58dcc to 00d4114 Compare November 13, 2018 20:24

ydamigos added 4 commits November 14, 2018 19:23

dts/bindings/usb: Add phys property to STM32 USB yaml

a81f583

Add phys property, which specifies USB PHY provider, to STM32 USB yaml files. Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com>

dts: st: Add USB PHY nodes

69c528f

Add USB phy nodes and phys property in USB nodes. Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com>

drivers: usb_dc_stm32: Use DT to check if USBPHYC exists

918015b

Use DT to check if SoC has a USBPHYC controller. Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com>

ydamigos force-pushed the usb-phys branch from 00d4114 to 918015b Compare November 14, 2018 17:31

galak approved these changes Nov 15, 2018

View reviewed changes

galak merged commit 954a943 into zephyrproject-rtos:master Nov 15, 2018

ydamigos deleted the usb-phys branch November 15, 2018 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dts: Add initial support of USB PHYs #10359

dts: Add initial support of USB PHYs #10359

ydamigos commented Oct 3, 2018

ydamigos commented Oct 3, 2018

codecov-io commented Oct 3, 2018 •

edited

Loading

galak left a comment

ydamigos commented Oct 3, 2018

erwango commented Oct 4, 2018

ydamigos commented Oct 4, 2018

galak commented Oct 9, 2018 •

edited

Loading

aurel32 commented Oct 10, 2018

ydamigos commented Oct 11, 2018

ydamigos commented Oct 17, 2018

erwango left a comment

erwango Nov 7, 2018

ydamigos Nov 7, 2018

galak left a comment

ydamigos commented Nov 13, 2018

galak commented Nov 13, 2018

ydamigos commented Nov 13, 2018

galak commented Nov 14, 2018

ydamigos commented Nov 14, 2018

dts: Add initial support of USB PHYs #10359

dts: Add initial support of USB PHYs #10359

Conversation

ydamigos commented Oct 3, 2018

ydamigos commented Oct 3, 2018

codecov-io commented Oct 3, 2018 • edited Loading

Codecov Report

galak left a comment

Choose a reason for hiding this comment

ydamigos commented Oct 3, 2018

erwango commented Oct 4, 2018

ydamigos commented Oct 4, 2018

galak commented Oct 9, 2018 • edited Loading

aurel32 commented Oct 10, 2018

ydamigos commented Oct 11, 2018

ydamigos commented Oct 17, 2018

erwango left a comment

Choose a reason for hiding this comment

erwango Nov 7, 2018

Choose a reason for hiding this comment

ydamigos Nov 7, 2018

Choose a reason for hiding this comment

galak left a comment

Choose a reason for hiding this comment

ydamigos commented Nov 13, 2018

galak commented Nov 13, 2018

ydamigos commented Nov 13, 2018

galak commented Nov 14, 2018

ydamigos commented Nov 14, 2018

codecov-io commented Oct 3, 2018 •

edited

Loading

galak commented Oct 9, 2018 •

edited

Loading