[PATCH 1/4] phy-sun4i-usb: Fix sun8i_r40_cfg
From: qianfan Zhao qianfanguijin@163.com
The address of sun8i_r40's phyctrl is 0x01c13404, also fixed enable_pmu and dual_route.
Signed-off-by: qianfan Zhao qianfanguijin@163.com --- drivers/phy/allwinner/phy-sun4i-usb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c index 5723c98032..608ba46242 100644 --- a/drivers/phy/allwinner/phy-sun4i-usb.c +++ b/drivers/phy/allwinner/phy-sun4i-usb.c @@ -587,10 +587,10 @@ static const struct sun4i_usb_phy_cfg sun8i_r40_cfg = { .num_phys = 3, .type = sun8i_r40_phy, .disc_thresh = 3, - .phyctl_offset = REG_PHYCTL_A33, + .phyctl_offset = REG_PHYCTL_A10, .dedicated_clocks = true, - .enable_pmu_unk1 = true, - .phy0_dual_route = true, + .enable_pmu_unk1 = false, + .phy0_dual_route = false, };
static const struct sun4i_usb_phy_cfg sun8i_v3s_cfg = {
From: qianfan Zhao qianfanguijin@163.com
R40 has 8 user-configurable endpoints and 8KB FIFO for EPs.
Signed-off-by: qianfan Zhao qianfanguijin@163.com --- drivers/usb/musb-new/sunxi.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c index fea4105f3d..e03299ea5b 100644 --- a/drivers/usb/musb-new/sunxi.c +++ b/drivers/usb/musb-new/sunxi.c @@ -263,7 +263,6 @@ static int sunxi_musb_enable(struct musb *musb) }
USBC_ForceVbusValidToHigh(musb->mregs); - enabled = true; return 0; } @@ -438,6 +437,30 @@ static struct musb_hdrc_config musb_config_h3 = { .ram_bits = SUNXI_MUSB_RAM_BITS, };
+/* R40/A40 OTG supports only 4 endpoints */ +#define SUNXI_MUSB_MAX_EP_NUM_R40 5 + +static struct musb_fifo_cfg sunxi_musb_mode_cfg_r40[] = { + MUSB_EP_FIFO_SINGLE(1, FIFO_TX, 512), + MUSB_EP_FIFO_SINGLE(1, FIFO_RX, 512), + MUSB_EP_FIFO_SINGLE(2, FIFO_TX, 512), + MUSB_EP_FIFO_SINGLE(2, FIFO_RX, 512), + MUSB_EP_FIFO_SINGLE(3, FIFO_TX, 512), + MUSB_EP_FIFO_SINGLE(3, FIFO_RX, 512), + MUSB_EP_FIFO_SINGLE(4, FIFO_TX, 512), + MUSB_EP_FIFO_SINGLE(4, FIFO_RX, 512), +}; + +static struct musb_hdrc_config musb_config_r40 = { + .fifo_cfg = sunxi_musb_mode_cfg_r40, + .fifo_cfg_size = ARRAY_SIZE(sunxi_musb_mode_cfg_r40), + .multipoint = true, + .dyn_fifo = true, + .soft_con = true, + .num_eps = SUNXI_MUSB_MAX_EP_NUM_R40, + .ram_bits = SUNXI_MUSB_RAM_BITS, +}; + static int musb_usb_probe(struct udevice *dev) { struct sunxi_glue *glue = dev_get_priv(dev); @@ -527,6 +550,10 @@ static const struct sunxi_musb_config sun8i_h3_cfg = { .config = &musb_config_h3, };
+static const struct sunxi_musb_config sun8i_r40_cfg = { + .config = &musb_config_r40, +}; + static const struct udevice_id sunxi_musb_ids[] = { { .compatible = "allwinner,sun4i-a10-musb", .data = (ulong)&sun4i_a10_cfg }, @@ -536,6 +563,8 @@ static const struct udevice_id sunxi_musb_ids[] = { .data = (ulong)&sun6i_a31_cfg }, { .compatible = "allwinner,sun8i-h3-musb", .data = (ulong)&sun8i_h3_cfg }, + { .compatible = "allwinner,sun8i-r40-musb", + .data = (ulong)&sun8i_r40_cfg }, { } };
On Wed, 16 Jun 2021 10:33:24 +0800 qianfanguijin@163.com (qianfanguijin@163.com) wrote:
Hi,
From: qianfan Zhao qianfanguijin@163.com
R40 has 8 user-configurable endpoints and 8KB FIFO for EPs.
This means the MUSB controller is fully compatible to the H3, so this whole patch is not needed:
Signed-off-by: qianfan Zhao qianfanguijin@163.com
drivers/usb/musb-new/sunxi.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c index fea4105f3d..e03299ea5b 100644 --- a/drivers/usb/musb-new/sunxi.c +++ b/drivers/usb/musb-new/sunxi.c @@ -263,7 +263,6 @@ static int sunxi_musb_enable(struct musb *musb) }
USBC_ForceVbusValidToHigh(musb->mregs);
- enabled = true; return 0;
} @@ -438,6 +437,30 @@ static struct musb_hdrc_config musb_config_h3 = { .ram_bits = SUNXI_MUSB_RAM_BITS, };
+/* R40/A40 OTG supports only 4 endpoints */ +#define SUNXI_MUSB_MAX_EP_NUM_R40 5
+static struct musb_fifo_cfg sunxi_musb_mode_cfg_r40[] = {
- MUSB_EP_FIFO_SINGLE(1, FIFO_TX, 512),
- MUSB_EP_FIFO_SINGLE(1, FIFO_RX, 512),
- MUSB_EP_FIFO_SINGLE(2, FIFO_TX, 512),
- MUSB_EP_FIFO_SINGLE(2, FIFO_RX, 512),
- MUSB_EP_FIFO_SINGLE(3, FIFO_TX, 512),
- MUSB_EP_FIFO_SINGLE(3, FIFO_RX, 512),
- MUSB_EP_FIFO_SINGLE(4, FIFO_TX, 512),
- MUSB_EP_FIFO_SINGLE(4, FIFO_RX, 512),
+};
+static struct musb_hdrc_config musb_config_r40 = {
- .fifo_cfg = sunxi_musb_mode_cfg_r40,
- .fifo_cfg_size = ARRAY_SIZE(sunxi_musb_mode_cfg_r40),
- .multipoint = true,
- .dyn_fifo = true,
- .soft_con = true,
- .num_eps = SUNXI_MUSB_MAX_EP_NUM_R40,
- .ram_bits = SUNXI_MUSB_RAM_BITS,
+};
Apart from changing the name, this is identical to the H3, so you won't need a separate structure for that.
static int musb_usb_probe(struct udevice *dev) { struct sunxi_glue *glue = dev_get_priv(dev); @@ -527,6 +550,10 @@ static const struct sunxi_musb_config sun8i_h3_cfg = { .config = &musb_config_h3, };
+static const struct sunxi_musb_config sun8i_r40_cfg = {
- .config = &musb_config_r40,
+};
static const struct udevice_id sunxi_musb_ids[] = { { .compatible = "allwinner,sun4i-a10-musb", .data = (ulong)&sun4i_a10_cfg }, @@ -536,6 +563,8 @@ static const struct udevice_id sunxi_musb_ids[] = { .data = (ulong)&sun6i_a31_cfg }, { .compatible = "allwinner,sun8i-h3-musb", .data = (ulong)&sun8i_h3_cfg },
- { .compatible = "allwinner,sun8i-r40-musb",
.data = (ulong)&sun8i_r40_cfg },
And since there is no difference to the H3, you don't need a separate compatible string either. Just use "allwinner,sun8i-r40-musb", "allwinner,sun8i-h3-musb" in the DT, and drop this whole patch here.
Cheers, Andre
{ } };
From: qianfan Zhao qianfanguijin@163.com
bpi-m2u has a hardware usb_otg, let's enable it in dts.
Signed-off-by: qianfan Zhao qianfanguijin@163.com --- arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts | 4 ++++ arch/arm/dts/sun8i-r40.dtsi | 13 +++++++++++++ 2 files changed, 17 insertions(+)
diff --git a/arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts b/arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts index a6a1087a0c..828ddc63ae 100644 --- a/arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts +++ b/arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts @@ -117,6 +117,10 @@ status = "okay"; };
+&usb_otg { + status = "okay"; +}; + &ehci1 { status = "okay"; }; diff --git a/arch/arm/dts/sun8i-r40.dtsi b/arch/arm/dts/sun8i-r40.dtsi index d5ad3b9efd..7c7a9cd9f8 100644 --- a/arch/arm/dts/sun8i-r40.dtsi +++ b/arch/arm/dts/sun8i-r40.dtsi @@ -363,6 +363,19 @@ #size-cells = <0>; };
+ usb_otg: usb@1c13000 { + compatible = "allwinner,sun8i-r40-musb"; + reg = <0x01c13000 0x0400>; + clocks = <&ccu CLK_BUS_OTG>; + resets = <&ccu RST_BUS_OTG>; + interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "mc"; + phys = <&usbphy 0>; + phy-names = "usb"; + extcon = <&usbphy 0>; + status = "disabled"; + }; + usbphy: phy@1c13400 { compatible = "allwinner,sun8i-r40-usb-phy"; reg = <0x01c13400 0x14>,
On Wed, 16 Jun 2021 10:33:25 +0800 qianfanguijin@163.com (qianfanguijin@163.com) wrote:
Hi,
in general this patch looks mostly alright (see below), but this needs to go through Linux first, since we only sync DTs this way.
Some comments below, for when you send this to Linux:
From: qianfan Zhao qianfanguijin@163.com
bpi-m2u has a hardware usb_otg, let's enable it in dts.
Signed-off-by: qianfan Zhao qianfanguijin@163.com
arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts | 4 ++++ arch/arm/dts/sun8i-r40.dtsi | 13 +++++++++++++ 2 files changed, 17 insertions(+)
diff --git a/arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts b/arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts index a6a1087a0c..828ddc63ae 100644 --- a/arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts +++ b/arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts @@ -117,6 +117,10 @@ status = "okay"; };
+&usb_otg {
Please keep this sorted alphabetically, so it belongs just above the usbphy node below. And please put the dr_mode property here (even though U-Boot ignores this, AFAIK). Also we would need the USB-ID and power supply in the PHY node below, compare other DTs with a micro-USB socket and proper VBUS support.
- status = "okay";
+};
&ehci1 { status = "okay"; }; diff --git a/arch/arm/dts/sun8i-r40.dtsi b/arch/arm/dts/sun8i-r40.dtsi index d5ad3b9efd..7c7a9cd9f8 100644 --- a/arch/arm/dts/sun8i-r40.dtsi +++ b/arch/arm/dts/sun8i-r40.dtsi @@ -363,6 +363,19 @@ #size-cells = <0>; };
usb_otg: usb@1c13000 {compatible = "allwinner,sun8i-r40-musb";
Since this is fully compatible with the H3, please use: compatible = "allwinner,sun8i-r40-musb", "allwinner,sun8i-h3-musb";
This way we won't need code changes to the MUSB driver.
The rest (address, interrupt) look alright.
Cheers, Andre
reg = <0x01c13000 0x0400>;clocks = <&ccu CLK_BUS_OTG>;resets = <&ccu RST_BUS_OTG>;interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;interrupt-names = "mc";phys = <&usbphy 0>;phy-names = "usb";extcon = <&usbphy 0>;status = "disabled";};- usbphy: phy@1c13400 { compatible = "allwinner,sun8i-r40-usb-phy"; reg = <0x01c13400 0x14>,
From: qianfan Zhao qianfanguijin@163.com
Since the usb otg driver support R40 device, we enable usb gadget functions and ums.
Signed-off-by: qianfan Zhao qianfanguijin@163.com --- configs/Bananapi_M2_Ultra_defconfig | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/configs/Bananapi_M2_Ultra_defconfig b/configs/Bananapi_M2_Ultra_defconfig index 37bcb3d7bf..a0869d56ae 100644 --- a/configs/Bananapi_M2_Ultra_defconfig +++ b/configs/Bananapi_M2_Ultra_defconfig @@ -7,12 +7,13 @@ CONFIG_MACPWR="PA17" CONFIG_MMC0_CD_PIN="PH13" CONFIG_MMC_SUNXI_SLOT_EXTRA=2 CONFIG_USB1_VBUS_PIN="PH23" -CONFIG_USB2_VBUS_PIN="PH23" CONFIG_DEFAULT_DEVICE_TREE="sun8i-r40-bananapi-m2-ultra" CONFIG_AHCI=y # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set CONFIG_SPL_I2C_SUPPORT=y +CONFIG_CMD_USB_MASS_STORAGE=y CONFIG_SCSI_AHCI=y +# CONFIG_USB_FUNCTION_FASTBOOT is not set CONFIG_RGMII=y CONFIG_SUN8I_EMAC=y CONFIG_AXP_DLDO4_VOLT=2500 @@ -20,3 +21,5 @@ CONFIG_AXP_ELDO3_VOLT=1200 CONFIG_SCSI=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_OHCI_HCD=y +CONFIG_USB_MUSB_GADGET=y +CONFIG_USB_GADGET_DOWNLOAD=y
On Wed, 16 Jun 2021 10:33:26 +0800 qianfanguijin@163.com (qianfanguijin@163.com) wrote:
Hi,
From: qianfan Zhao qianfanguijin@163.com
Since the usb otg driver support R40 device, we enable usb gadget functions and ums.
Signed-off-by: qianfan Zhao qianfanguijin@163.com
configs/Bananapi_M2_Ultra_defconfig | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/configs/Bananapi_M2_Ultra_defconfig b/configs/Bananapi_M2_Ultra_defconfig index 37bcb3d7bf..a0869d56ae 100644 --- a/configs/Bananapi_M2_Ultra_defconfig +++ b/configs/Bananapi_M2_Ultra_defconfig @@ -7,12 +7,13 @@ CONFIG_MACPWR="PA17" CONFIG_MMC0_CD_PIN="PH13" CONFIG_MMC_SUNXI_SLOT_EXTRA=2 CONFIG_USB1_VBUS_PIN="PH23" -CONFIG_USB2_VBUS_PIN="PH23" CONFIG_DEFAULT_DEVICE_TREE="sun8i-r40-bananapi-m2-ultra" CONFIG_AHCI=y # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set CONFIG_SPL_I2C_SUPPORT=y +CONFIG_CMD_USB_MASS_STORAGE=y
So in general we don't have those kind of command enables in the defconfig. It might be worth discussing whether we should enable the really useful "ums" command by default, for all Allwinner devices enabling USB gadget mode, but this would be done differently.
CONFIG_SCSI_AHCI=y +# CONFIG_USB_FUNCTION_FASTBOOT is not set
Why is this explicitly disabled? From what I can see, it doesn't hurt to leave it enabled (through Kconfig's default), it does not interfere with other gadgets like UMS or Ethernet.
CONFIG_RGMII=y CONFIG_SUN8I_EMAC=y CONFIG_AXP_DLDO4_VOLT=2500 @@ -20,3 +21,5 @@ CONFIG_AXP_ELDO3_VOLT=1200 CONFIG_SCSI=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_OHCI_HCD=y +CONFIG_USB_MUSB_GADGET=y +CONFIG_USB_GADGET_DOWNLOAD=y
This last symbol is enabled automatically by the FASTBOOT option.
Cheers, Andre
On Wed, 16 Jun 2021 10:33:23 +0800 qianfanguijin@163.com (qianfanguijin@163.com) wrote:
Hi,
first many thanks for sending this! Indeed OTG support was broken/missing on the R40, also in Linux. So it seems you have the found the problem: the missing PHY multiplex. Many thanks for that! This indeed enables OTG functionality for me (although with some changes). That means that actually a similar patch needs to go through Linux. Do you plan on enabling support in Linux as well?
From: qianfan Zhao qianfanguijin@163.com
The address of sun8i_r40's phyctrl is 0x01c13404,
But this isn't quite right, is it? See below.
enable_pmu and dual_route.
Ah, of course! The R40 is closer to the A33/A23 here, with not having separate EHCI0/OHCI0 controllers, instead relying on the MUSB host IP. So indeed we don't have the PHY multiplex for PHY0. I think this is the root cause of the non-working OTG support so far!
Signed-off-by: qianfan Zhao qianfanguijin@163.com
drivers/phy/allwinner/phy-sun4i-usb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c index 5723c98032..608ba46242 100644 --- a/drivers/phy/allwinner/phy-sun4i-usb.c +++ b/drivers/phy/allwinner/phy-sun4i-usb.c @@ -587,10 +587,10 @@ static const struct sun4i_usb_phy_cfg sun8i_r40_cfg = { .num_phys = 3, .type = sun8i_r40_phy, .disc_thresh = 3,
- .phyctl_offset = REG_PHYCTL_A33,
- .phyctl_offset = REG_PHYCTL_A10,
So this doesn't work for me, no device appearing on the host. Also writing anything to this register (+0x04) reads back as 0, so it's not implemented, as in the H3. The register at +0x10 however works, and if I keep the A33 line, OTG indeed works for me. Same in Linux, btw.
.dedicated_clocks = true,
- .enable_pmu_unk1 = true,
- .phy0_dual_route = true,
- .enable_pmu_unk1 = false,
- .phy0_dual_route = false,
If they are false, you don't need to list them, the default of 0 will take care of this.
Cheers, Andre
};
static const struct sun4i_usb_phy_cfg sun8i_v3s_cfg = {
在 2021/6/21 8:33, Andre Przywara 写道:
On Wed, 16 Jun 2021 10:33:23 +0800 qianfanguijin@163.com (qianfanguijin@163.com) wrote:
Hi,
first many thanks for sending this! Indeed OTG support was broken/missing on the R40, also in Linux. So it seems you have the found the problem: the missing PHY multiplex. Many thanks for that! This indeed enables OTG functionality for me (although with some changes). That means that actually a similar patch needs to go through Linux. Do you plan on enabling support in Linux as well?
sure.
From: qianfan Zhao qianfanguijin@163.com
The address of sun8i_r40's phyctrl is 0x01c13404,
But this isn't quite right, is it? See below.
Yes, the right address of R40 is 0x01c13410. I had checked the bsp code of allwinner: #if defined (CONFIG_ARCH_SUN50I) || defined (CONFIG_ARCH_SUN8IW10) || defined (CONFIG_ARCH_SUN8IW11)#define USBPHYC_REG_o_PHYCTL 0x0410#else#define USBPHYC_REG_o_PHYCTL 0x0404#endif
But I had no idea why the addres 0x01c13404 can work fine, maybe I load u-boot by using sunxi-tools, that the usb otg is already init by IBR.
enable_pmu and dual_route.
Ah, of course! The R40 is closer to the A33/A23 here, with not having separate EHCI0/OHCI0 controllers, instead relying on the MUSB host IP. So indeed we don't have the PHY multiplex for PHY0. I think this is the root cause of the non-working OTG support so far!
Signed-off-by: qianfan Zhao qianfanguijin@163.com
drivers/phy/allwinner/phy-sun4i-usb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c index 5723c98032..608ba46242 100644 --- a/drivers/phy/allwinner/phy-sun4i-usb.c +++ b/drivers/phy/allwinner/phy-sun4i-usb.c @@ -587,10 +587,10 @@ static const struct sun4i_usb_phy_cfg sun8i_r40_cfg = { .num_phys = 3, .type = sun8i_r40_phy, .disc_thresh = 3,
- .phyctl_offset = REG_PHYCTL_A33,
- .phyctl_offset = REG_PHYCTL_A10,
So this doesn't work for me, no device appearing on the host. Also writing anything to this register (+0x04) reads back as 0, so it's not implemented, as in the H3. The register at +0x10 however works, and if I keep the A33 line, OTG indeed works for me. Same in Linux, btw.
.dedicated_clocks = true,
- .enable_pmu_unk1 = true,
- .phy0_dual_route = true,
- .enable_pmu_unk1 = false,
- .phy0_dual_route = false,
If they are false, you don't need to list them, the default of 0 will take care of this.
Thanks for yours guide, I will make a change later.
Cheers, Andre
};
static const struct sun4i_usb_phy_cfg sun8i_v3s_cfg = {
participants (3)
-
Andre Przywara -
qianfan -
qianfanguijin＠163.com