Skip to contentSkip to conversation

USB: chipidea misc patches

PATCH Superseded Peter Chen 3 participants 17 messages Aug 3, 2015 – Sep 22, 2015

Subsystems (4)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Peter Chen

author
#0 lore raw

v2 cover USB: chipidea misc patches

Hi all,

In this series, I add several new interfaces for chipidea driver, they are mainly for system configuration adjustment. The USB performance may be improved with these configuration changing, but each vendor driver owner needs to consult with your IC owner which configuration parameters are suitable for your system before changing it, and only change it if necessary.

The other changes are for imx, it shows how imx uses these interfaces. Shawn, the patch [5/10] and [9/10] are dts changes, help to review it please.

Changes for v2:
- Use the bit fields which are not used. [Patch 2/10, 6/10, 10/10]
- According to Greg and Sascha comments, format patch 2/10's commit log.
- using non-burst instead of unburst for patch 7/10

Peter Chen (10):
usb: chipidea: udc: zero-length packet is only needed for TX
usb: chipidea: define stream mode disable for both roles
usb: chipidea: imx: add stream mode enable for device mode at
imx6sl/imx6sx
Doc: usb: ci-hdrc-usb2: add ahb-burst-config for binding doc
ARM: imx6: set ahb-burst-config as 0 for USB
usb: chipidea: add ahb burst configuration interface
usb: chipidea: usbmisc_imx: add non-burst setting for imx6
Doc: usb: ci-hdrc-usb2: add tx(rx)-burst-config-dword for binding doc
ARM: imx6: change default burst size for USB
usb: chipidea: add tx/rx burst size configuration interface

 .../devicetree/bindings/usb/ci-hdrc-usb2.txt       | 10 ++++
 arch/arm/boot/dts/imx6qdl.dtsi                     | 12 +++++
 arch/arm/boot/dts/imx6sl.dtsi                      |  9 ++++
 arch/arm/boot/dts/imx6sx.dtsi                      |  9 ++++
 drivers/usb/chipidea/bits.h                        |  7 +++
 drivers/usb/chipidea/ci.h                          |  1 +
 drivers/usb/chipidea/ci_hdrc_imx.c                 | 16 +++---
 drivers/usb/chipidea/core.c                        | 62 +++++++++++++++++++++-
 drivers/usb/chipidea/udc.c                         |  2 +-
 drivers/usb/chipidea/usbmisc_imx.c                 | 12 ++++-
 include/linux/usb/chipidea.h                       | 11 +++-
 11 files changed, 140 insertions(+), 11 deletions(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH v2 1/10 usb: chipidea: udc: zero-length packet is only needed for TX

The zero-length packet is the sendor tells the receiver that there is no more data, so it is only needed at the TX side.

---
 drivers/usb/chipidea/udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/chipidea/udc.c +1 -1
index 06b7224..51e66a1 100644
@@ -445,7 +445,7 @@ static int _hardware_enqueue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
 		rest -= count;
 	}
 
-	if (hwreq->req.zero && hwreq->req.length
+	if (hwreq->req.zero && hwreq->req.length && hwep->dir == TX
 	    && (hwreq->req.length % hwep->ep.maxpacket == 0))
 		add_td_to_list(hwep, hwreq, 0);
 
Signature · 1 line
1.9.1
author
#2 lore raw

PATCH v2 2/10 usb: chipidea: define stream mode disable for both roles

In reply to Peter Chen

The system bus and chipidea IP have different limitations for both host and device mode.
For example, with below errata, we need to enable SDIS(Stream Disable Mode) at host mode. But we don't want it for device mode at the same system.

TAR 9000378958
Title: Non-Double Word Aligned Buffer Address Sometimes Causes Host to Hang on OUT Retry
Impacted Configuration: Host mode, all transfer types Description:
The host core operating in streaming mode may under run while sending the data packet of an OUT transaction. This under run can occur if there are unexpected system delays in fetching the remaining packet data from memory. The host forces a bad CRC on the packet, the device detects the error and discards the packet. The host then retries a Bulk, Interrupt, or Control transfer if an under run occurs according to the USB specification. During simulations, it was found that the host does not issue the retry of the failed bulk OUT. It does not issue any other transactions except SOF packets that have incorrect frame numbers.

The second failure mode occurs if the under run occurs on an ISO OUT transaction and the next ISO transaction is a zero byte packet. The host does not issue any transactions (including SOFs). The device detects a Suspend condition, reverts to full speed, and waits for resume signaling.

A third failure mode occurs when the host under runs on an ISO OUT and the next ISO in the schedule is an ISO OUT with two max packets of 1024 bytes each. The host should issue MDATA for the first OUT followed by DATA1 for the second. However, it drops the MDATA transaction, and issues the DATA1 transaction.

The system impact of this bug is the same regardless of the failure mode observed. The host core hangs, the ehci_ctrl state machine waits for the protocol engine to send the completion status for the corrupted transaction, which never occurs. No indication is sent to the host controller driver, no register bits change and no interrupts occur. Eventually the requesting application times out.

Detailed internal behavior:
The EHCI control state machine (ehci_ctrl) in the DMA block is responsible for parsing the schedules and initiating all transactions. The ehci_ctrl state machine passes the transaction details to the protocol block by writing the transaction information in to the TxFIFO. It then asserts the pe_hst_run_pkt signal to inform the host protocol state machine (pe_hst_state) that there is a packet in the TxFIFO.
A tag of 0x0 indicates a start of packet with the data providing the following information:

35:32 Tag
31:30 Reserved
29:23 Endpoint (lowest 4 bits)
22:16 Address
15:10 Reserved
9:8 Endpoint speed
7:6 Endpoint type
5:6 Data Toggle
3:0 PID
The pe_hst_state reads the packet information and constructs the packet and issues it to the PHY interface.
The ehci_ctrl state machine writes the start transaction information in to the TxFIFO as 0x03002910c for the OUT packet that had the under run error. However, it writes 0xC3002910C for the retry of the Out transaction, which is incorrect.
The pe_hst_state enters a bus timeout state after sending the bad CRC for the packet that under ran. It then purges any data that was back filled in to the TxFIFO for the packet that under ran. The pe_hst_state machine stops purging the TxFIFO when it is empty or if it reads a location that has a tag of 0x0, indicating a start of packet command.

The pe_hst_state reads 0xC3002910C and discards it as it does not decode to a start of packet command. It continues to purge the OUT data that has been pre-buffered for the OUT retry . The pe_hst_state detects the hst_packet_run signal and attempts to read the PID and address information from the TxFIFO. This location has packet data and so does not decode to a valid PID and so falls through to the PE_HST_SOF_LOAD state where the frame_num_counter is updated. The frame_num_counter is updated with the data in the TxFIFO. In this case, the data is incorrect as the ehci_ctrl state machine did not initiate the load. The hst_pe_state machine detects the SOF request signal and sends an SOF with the bad frame number. Meanwhile, the ehci_ctrl state machine waits indefinitely in the run_pkt state waiting for the completion status from pe_hst_state machine, which will never happen.

The ISO failure case is similar except that there is no retry for ISO. The ehci_ctrl state machine moves to the next transfer in the periodic schedule. If the under run occurs on the last entry of the periodic list then it moves to the Async schedule.

In the case of ISO OUT simulations, the next ISO is a zero byte OUT and again the start of packet command gets corrupted. The TxFIFO is empty when the hst_pe_state attempts to read the Address and PID information as the transaction is a zero byte packet. This results in the hst_pe_state machine staying in the GET_PID state, which means that it does not issue any transactions (including SOFs). The device detects a Suspend condition and reverts to full speed mode and waits for a Resume or Reset signal.

The EHCI specification allows a Non-DoubleWord (32 bits) offset to be used as a current offset for Buffer Pointer Page 0 of the qTD. In Non-DoubleWord aligned cases, the core reads the packet data from the AHB memory, performs the alignment operation before writing it in to the TxFIFO as a 32 bit data word. An End Of Packet tag (EOP) is written to the TxFIFO after all the packet data has been written in to the TxFIFO. The alignment function is reset to Idle by the EOP tag. The corruption of the start of packet command arises because the packet buffer for the OUT transaction that under ran is not aligned to a DoubleWord, and hence no EOP tag is written to the TxFIFO. The alignment function is still active when the start packet information is written in to the TxFIFO for the retry of the bulk packet or for the next transaction in the case of an under run on an ISO. This results in the corruption of the start tag and the transaction information.

Click for waveform showing the command 0x 0000300291 being written in to the TX FIFO for the Out that under ran.
Click for waveform showing the command 0xC3002910C written to the TxFIFO instead of 0x 0000300291
Versions affected: Versions 2.10a and previous versions How discovered: Customer simulation

Workaround:
1- The EHCI specification allows a non-DoubleWord offset to be used as a current offset for Buffer Pointer Page 0 of the qTD. However, if a DoubleWord offset is used then this issue does not arise. 2- Use non streaming mode to eliminate under runs.

Resolution:
The fix involves changes to the traffic state machine in the vusb_hs_dma_traf block. The ehci_ctrl state machine updates the context information by encoding the transaction results on the hst_op_context_update signals at the end of a transaction. The signal hst_op_context_update is added to the traffic state machine, and the tx_fifo_under_ran_r signal is generated if the transaction results in an under run error. Click for waveform

The traffic state machine then traverses to the do_eop states if the tx_fifo_under_ran error is asserted. Thus an EOP tag is written in to the TxFIFO as shown in this waveform .

The EOP tag resets the align state machine to the Idle state ensuring that the next command written by the echi_ctrl state machine does not get corrupted.

File(s) modified:
RTL code fixed: ?..
Method of reproducing: This failure cannot be reproduced in the current test bench.
Date Found: March 2010
Date Fixed: June 2010
Update information:
Added the RTL code fix

---
 drivers/usb/chipidea/core.c  | 13 ++++++++++++-
 include/linux/usb/chipidea.h |  5 ++++-
 2 files changed, 16 insertions(+), 2 deletions(-)
drivers/usb/chipidea/core.c +12 -1
index 9cbb846..38bd036 100644
@@ -64,6 +64,7 @@
 #include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/regulator/consumer.h>
+#include <linux/usb/ehci_def.h>
 
 #include "ci.h"
 #include "udc.h"
@@ -413,7 +414,17 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
  */
 void ci_platform_configure(struct ci_hdrc *ci)
 {
-	if (ci->platdata->flags & CI_HDRC_DISABLE_STREAMING)
+	bool is_device_mode, is_host_mode;
+
+	is_device_mode = hw_read(ci, OP_USBMODE, USBMODE_CM) == USBMODE_CM_DC;
+	is_host_mode = hw_read(ci, OP_USBMODE, USBMODE_CM) == USBMODE_CM_HC;
+
+	if (is_device_mode &&
+		(ci->platdata->flags & CI_HDRC_DISABLE_DEVICE_STREAMING))
+		hw_write(ci, OP_USBMODE, USBMODE_CI_SDIS, USBMODE_CI_SDIS);
+
+	if (is_host_mode &&
+		(ci->platdata->flags & CI_HDRC_DISABLE_HOST_STREAMING))
 		hw_write(ci, OP_USBMODE, USBMODE_CI_SDIS, USBMODE_CI_SDIS);
 
 	if (ci->platdata->flags & CI_HDRC_FORCE_FULLSPEED) {
include/linux/usb/chipidea.h +4 -1
index aa49177..034f7e1 100644
@@ -19,8 +19,11 @@ struct ci_hdrc_platform_data {
 	enum usb_phy_interface phy_mode;
 	unsigned long	 flags;
 #define CI_HDRC_REGS_SHARED		BIT(0)
+#define CI_HDRC_DISABLE_DEVICE_STREAMING	BIT(1)
 #define CI_HDRC_SUPPORTS_RUNTIME_PM	BIT(2)
-#define CI_HDRC_DISABLE_STREAMING	BIT(3)
+#define CI_HDRC_DISABLE_HOST_STREAMING	BIT(3)
+#define CI_HDRC_DISABLE_STREAMING (CI_HDRC_DISABLE_DEVICE_STREAMING |	\
+		CI_HDRC_DISABLE_HOST_STREAMING)
 	/*
 	 * Only set it when DCCPARAMS.DC==1 and DCCPARAMS.HC==1,
 	 * but otg is not supported (no register otgsc).
Signature · 1 line
1.9.1
author
#3 lore raw

PATCH v2 3/10 usb: chipidea: imx: add stream mode enable for device mode at imx6sl/imx6sx

In reply to Peter Chen

Stream mode enable is known for better performance, this stream mode enable patch has been passed with stress tests at device mode for imx6sl and imx6sx, and no issue is found.

---
 drivers/usb/chipidea/ci_hdrc_imx.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/usb/chipidea/ci_hdrc_imx.c +10 -6
index 6d2a85a..3f41679 100644
@@ -29,26 +29,31 @@ struct ci_hdrc_imx_platform_flag {
 };
 
 static const struct ci_hdrc_imx_platform_flag imx27_usb_data = {
+		CI_HDRC_DISABLE_STREAMING,
 };
 
 static const struct ci_hdrc_imx_platform_flag imx28_usb_data = {
 	.flags = CI_HDRC_IMX28_WRITE_FIX |
-		CI_HDRC_TURN_VBUS_EARLY_ON,
+		CI_HDRC_TURN_VBUS_EARLY_ON |
+		CI_HDRC_DISABLE_STREAMING,
 };
 
 static const struct ci_hdrc_imx_platform_flag imx6q_usb_data = {
 	.flags = CI_HDRC_SUPPORTS_RUNTIME_PM |
-		CI_HDRC_TURN_VBUS_EARLY_ON,
+		CI_HDRC_TURN_VBUS_EARLY_ON |
+		CI_HDRC_DISABLE_STREAMING,
 };
 
 static const struct ci_hdrc_imx_platform_flag imx6sl_usb_data = {
 	.flags = CI_HDRC_SUPPORTS_RUNTIME_PM |
-		CI_HDRC_TURN_VBUS_EARLY_ON,
+		CI_HDRC_TURN_VBUS_EARLY_ON |
+		CI_HDRC_DISABLE_HOST_STREAMING,
 };
 
 static const struct ci_hdrc_imx_platform_flag imx6sx_usb_data = {
 	.flags = CI_HDRC_SUPPORTS_RUNTIME_PM |
-		CI_HDRC_TURN_VBUS_EARLY_ON,
+		CI_HDRC_TURN_VBUS_EARLY_ON |
+		CI_HDRC_DISABLE_HOST_STREAMING,
 };
 
 static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
@@ -126,8 +131,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 	struct ci_hdrc_platform_data pdata = {
 		.name		= dev_name(&pdev->dev),
 		.capoffset	= DEF_CAPOFFSET,
-		.flags		= CI_HDRC_DISABLE_STREAMING |
-					CI_HDRC_SET_NON_ZERO_TTHA,
+		.flags		= CI_HDRC_SET_NON_ZERO_TTHA,
 	};
 	int ret;
 	const struct of_device_id *of_id =
Signature · 1 line
1.9.1
author
#4 lore raw

PATCH v2 4/10 Doc: usb: ci-hdrc-usb2: add ahb-burst-config for binding doc

In reply to Peter Chen

It is used to change ahb burst configuration for platforms, it is vendor specific.

---
 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 4 ++++
 1 file changed, 4 insertions(+)
Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +4 -0
index 4159c8c..3c377a3 100644
@@ -32,6 +32,8 @@ Optional properties:
 - external-vbus-divider: (FSL only) enables off-chip resistor divider for Vbus
 - itc-setting: interrupt threshold control register control, the setting
   should be aligned with ITC bits at register USBCMD.
+- ahb-burst-config: it is vendor dependent, the required value should be
+  aligned with AHBBRST at SBUSCFG, the range is from 0x0 to 0x7.
 
 Example:
 
@@ -44,4 +46,6 @@ Example:
 		phy-names = "usb-phy";
 		vbus-supply = <&reg_usb0_vbus>;
 		gadget-itc-setting = <0x4>; /* 4 micro-frames */
+		 /* Incremental burst of unspecified length */
+		ahb-burst-config = <0x0>;
 	};
Signature · 1 line
1.9.1
#5 lore raw

PATCH v2 4/10 Doc: usb: ci-hdrc-usb2: add ahb-burst-config for binding doc

Quoted 13 lines · Peter Chen

It is used to change ahb burst configuration for platforms, it is vendor specific.

---
 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 4 ++++
 1 file changed, 4 insertions(+)
Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +0 -0
index 4159c8c..3c377a3 100644

Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt Optional properties: · line 36

Quoted 5 lines
Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +2 -0
@@ -32,6 +32,8 @@ Optional properties:
 - external-vbus-divider: (FSL only) enables off-chip resistor divider for Vbus
 - itc-setting: interrupt threshold control register control, the setting
   should be aligned with ITC bits at register USBCMD.
+- ahb-burst-config: it is vendor dependent, the required value should be
+  aligned with AHBBRST at SBUSCFG, the range is from 0x0 to 0x7.

What do the values mean and what happens if the property is not present? Please define that here.

Rob

Quoted 13 lines
Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +2 -1
 Example:
 
@@ -44,4 +46,6 @@ Example:
                phy-names = "usb-phy";
                vbus-supply = <&reg_usb0_vbus>;
                gadget-itc-setting = <0x4>; /* 4 micro-frames */
+                /* Incremental burst of unspecified length */
+               ahb-burst-config = <0x0>;
        };
--

1.9.1

author
#6 lore raw

PATCH v2 5/10 ARM: imx6: set ahb-burst-config as 0 for USB

In reply to Peter Chen

After setting ahb burst configuration as 0, we can increase tx/rx burst size, it will improve the USB performance

---
 arch/arm/boot/dts/imx6qdl.dtsi | 4 ++++
 arch/arm/boot/dts/imx6sl.dtsi  | 3 +++
 arch/arm/boot/dts/imx6sx.dtsi  | 3 +++
 3 files changed, 10 insertions(+)
arch/arm/boot/dts/imx6qdl.dtsi +4 -0
index e6d1359..42d62b0 100644
@@ -853,6 +853,7 @@
 				clocks = <&clks IMX6QDL_CLK_USBOH3>;
 				fsl,usbphy = <&usbphy1>;
 				fsl,usbmisc = <&usbmisc 0>;
+				ahb-burst-config = <0x0>;
 				status = "disabled";
 			};
 
@@ -864,6 +865,7 @@
 				fsl,usbphy = <&usbphy2>;
 				fsl,usbmisc = <&usbmisc 1>;
 				dr_mode = "host";
+				ahb-burst-config = <0x0>;
 				status = "disabled";
 			};
 
@@ -874,6 +876,7 @@
 				clocks = <&clks IMX6QDL_CLK_USBOH3>;
 				fsl,usbmisc = <&usbmisc 2>;
 				dr_mode = "host";
+				ahb-burst-config = <0x0>;
 				status = "disabled";
 			};
 
@@ -884,6 +887,7 @@
 				clocks = <&clks IMX6QDL_CLK_USBOH3>;
 				fsl,usbmisc = <&usbmisc 3>;
 				dr_mode = "host";
+				ahb-burst-config = <0x0>;
 				status = "disabled";
 			};
 
arch/arm/boot/dts/imx6sl.dtsi +3 -0
index a78e715..066fac9 100644
@@ -688,6 +688,7 @@
 				clocks = <&clks IMX6SL_CLK_USBOH3>;
 				fsl,usbphy = <&usbphy1>;
 				fsl,usbmisc = <&usbmisc 0>;
+				ahb-burst-config = <0x0>;
 				status = "disabled";
 			};
 
@@ -698,6 +699,7 @@
 				clocks = <&clks IMX6SL_CLK_USBOH3>;
 				fsl,usbphy = <&usbphy2>;
 				fsl,usbmisc = <&usbmisc 1>;
+				ahb-burst-config = <0x0>;
 				status = "disabled";
 			};
 
@@ -708,6 +710,7 @@
 				clocks = <&clks IMX6SL_CLK_USBOH3>;
 				fsl,usbmisc = <&usbmisc 2>;
 				dr_mode = "host";
+				ahb-burst-config = <0x0>;
 				status = "disabled";
 			};
 
arch/arm/boot/dts/imx6sx.dtsi +3 -0
index 708175d..3656a1d 100644
@@ -746,6 +746,7 @@
 				fsl,usbphy = <&usbphy1>;
 				fsl,usbmisc = <&usbmisc 0>;
 				fsl,anatop = <&anatop>;
+				ahb-burst-config = <0x0>;
 				status = "disabled";
 			};
 
@@ -756,6 +757,7 @@
 				clocks = <&clks IMX6SX_CLK_USBOH3>;
 				fsl,usbphy = <&usbphy2>;
 				fsl,usbmisc = <&usbmisc 1>;
+				ahb-burst-config = <0x0>;
 				status = "disabled";
 			};
 
@@ -768,6 +770,7 @@
 				phy_type = "hsic";
 				fsl,anatop = <&anatop>;
 				dr_mode = "host";
+				ahb-burst-config = <0x0>;
 				status = "disabled";
 			};
 
Signature · 1 line
1.9.1
author
#7 lore raw

PATCH v2 6/10 usb: chipidea: add ahb burst configuration interface

In reply to Peter Chen

The users can change it through dts or platform data if they want to change the default value.

---
 drivers/usb/chipidea/bits.h  |  3 +++
 drivers/usb/chipidea/core.c  | 14 ++++++++++++++
 include/linux/usb/chipidea.h |  2 ++
 3 files changed, 19 insertions(+)
drivers/usb/chipidea/bits.h +3 -0
index 831a8f6..462ad02 100644
@@ -25,6 +25,9 @@
 #define VERSION		      (0xF << 25)
 #define CIVERSION	      (0x7 << 29)
 
+/* SBUSCFG */
+#define AHBBRST_MASK		0x7
+
 /* HCCPARAMS */
 #define HCCPARAMS_LEN         BIT(17)
 
drivers/usb/chipidea/core.c +14 -0
index 38bd036..f60cbf6 100644
@@ -439,6 +439,9 @@ void ci_platform_configure(struct ci_hdrc *ci)
 
 	hw_write(ci, OP_USBCMD, 0xff0000, ci->platdata->itc_setting << 16);
 
+	if (ci->platdata->flags & CI_HDRC_OVERRIDE_AHB_BURST)
+		hw_write_id_reg(ci, ID_SBUSCFG, AHBBRST_MASK,
+			ci->platdata->ahb_burst_config);
 }
 
 /**
@@ -647,6 +650,17 @@ static int ci_get_platdata(struct device *dev,
 		}
 	}
 
+	if (of_find_property(dev->of_node, "ahb-burst-config", NULL)) {
+		ret = of_property_read_u32(dev->of_node, "ahb-burst-config",
+			&platdata->ahb_burst_config);
+		if (ret) {
+			dev_err(dev,
+				"failed to get ahb-burst-config\n");
+			return ret;
+		}
+		platdata->flags |= CI_HDRC_OVERRIDE_AHB_BURST;
+	}
+
 	return 0;
 }
 
include/linux/usb/chipidea.h +2 -0
index 034f7e1..fe3c8eb 100644
@@ -33,6 +33,7 @@ struct ci_hdrc_platform_data {
 #define CI_HDRC_FORCE_FULLSPEED		BIT(6)
 #define CI_HDRC_TURN_VBUS_EARLY_ON	BIT(7)
 #define CI_HDRC_SET_NON_ZERO_TTHA	BIT(8)
+#define CI_HDRC_OVERRIDE_AHB_BURST	BIT(9)
 	enum usb_dr_mode	dr_mode;
 #define CI_HDRC_CONTROLLER_RESET_EVENT		0
 #define CI_HDRC_CONTROLLER_STOPPED_EVENT	1
@@ -42,6 +43,7 @@ struct ci_hdrc_platform_data {
 	bool			tpl_support;
 	/* interrupt threshold setting */
 	u32			itc_setting;
+	u32			ahb_burst_config;
 };
 
 /* Default offset of capability registers */
Signature · 1 line
1.9.1
#8 lore raw

PATCH v2 6/10 usb: chipidea: add ahb burst configuration interface

Quoted 43 lines · Peter Chen

The users can change it through dts or platform data if they want to change the default value.

---
 drivers/usb/chipidea/bits.h  |  3 +++
 drivers/usb/chipidea/core.c  | 14 ++++++++++++++
 include/linux/usb/chipidea.h |  2 ++
 3 files changed, 19 insertions(+)
drivers/usb/chipidea/bits.h +3 -0
index 831a8f6..462ad02 100644
@@ -25,6 +25,9 @@
 #define VERSION                      (0xF << 25)
 #define CIVERSION            (0x7 << 29)
 
+/* SBUSCFG */
+#define AHBBRST_MASK           0x7
+
 /* HCCPARAMS */
 #define HCCPARAMS_LEN         BIT(17)
 
drivers/usb/chipidea/core.c +4 -0
index 38bd036..f60cbf6 100644
@@ -439,6 +439,9 @@ void ci_platform_configure(struct ci_hdrc *ci)
 
        hw_write(ci, OP_USBCMD, 0xff0000, ci->platdata->itc_setting << 16);
 
+       if (ci->platdata->flags & CI_HDRC_OVERRIDE_AHB_BURST)
+               hw_write_id_reg(ci, ID_SBUSCFG, AHBBRST_MASK,
+                       ci->platdata->ahb_burst_config);
 }
 
 /**
@@ -647,6 +650,17 @@ static int ci_get_platdata(struct device *dev,
                }
        }
 
+       if (of_find_property(dev->of_node, "ahb-burst-config", NULL)) {

drivers/usb/chipidea/core.c static int ci_get_platdata(struct device *dev, · line 663

Quoted 10 lines
drivers/usb/chipidea/core.c +10 -0
+               ret = of_property_read_u32(dev->of_node, "ahb-burst-config",
+                       &platdata->ahb_burst_config);
+               if (ret) {
+                       dev_err(dev,
+                               "failed to get ahb-burst-config\n");
+                       return ret;
+               }
+               platdata->flags |= CI_HDRC_OVERRIDE_AHB_BURST;
+       }
+

This can be simplified to:

ret = of_property_read_u32(dev->of_node, "ahb-burst-config", &platdata->ahb_burst_config);
if (!ret)
platdata->flags |= CI_HDRC_OVERRIDE_AHB_BURST;

The error condition can't really happen.

Rob

author
#9 lore raw

PATCH v2 6/10 usb: chipidea: add ahb burst configuration interface

Quoted 57 lines · Rob Herring
Quoted 43 lines · Peter Chen

The users can change it through dts or platform data if they want to change the default value.

---
 drivers/usb/chipidea/bits.h  |  3 +++
 drivers/usb/chipidea/core.c  | 14 ++++++++++++++
 include/linux/usb/chipidea.h |  2 ++
 3 files changed, 19 insertions(+)
drivers/usb/chipidea/bits.h +3 -0
index 831a8f6..462ad02 100644
@@ -25,6 +25,9 @@
 #define VERSION                      (0xF << 25)
 #define CIVERSION            (0x7 << 29)
 
+/* SBUSCFG */
+#define AHBBRST_MASK           0x7
+
 /* HCCPARAMS */
 #define HCCPARAMS_LEN         BIT(17)
 
drivers/usb/chipidea/core.c +4 -0
index 38bd036..f60cbf6 100644
@@ -439,6 +439,9 @@ void ci_platform_configure(struct ci_hdrc *ci)
 
        hw_write(ci, OP_USBCMD, 0xff0000, ci->platdata->itc_setting << 16);
 
+       if (ci->platdata->flags & CI_HDRC_OVERRIDE_AHB_BURST)
+               hw_write_id_reg(ci, ID_SBUSCFG, AHBBRST_MASK,
+                       ci->platdata->ahb_burst_config);
 }
 
 /**
@@ -647,6 +650,17 @@ static int ci_get_platdata(struct device *dev,
                }
        }
 
+       if (of_find_property(dev->of_node, "ahb-burst-config", NULL)) {

drivers/usb/chipidea/core.c static int ci_get_platdata(struct device *dev, · line 663

Quoted 10 lines
drivers/usb/chipidea/core.c +10 -0
+               ret = of_property_read_u32(dev->of_node, "ahb-burst-config",
+                       &platdata->ahb_burst_config);
+               if (ret) {
+                       dev_err(dev,
+                               "failed to get ahb-burst-config\n");
+                       return ret;
+               }
+               platdata->flags |= CI_HDRC_OVERRIDE_AHB_BURST;
+       }
+

This can be simplified to:

Quoted 8 lines

ret = of_property_read_u32(dev->of_node, "ahb-burst-config", &platdata->ahb_burst_config);
if (!ret)
platdata->flags |= CI_HDRC_OVERRIDE_AHB_BURST;

The error condition can't really happen.

Rob

Thanks, I will change others accordingly.

Signature · 2 lines
Best Regards,
Peter Chen
author
#10 lore raw

PATCH v2 6/10 usb: chipidea: add ahb burst configuration interface

In reply to Rob Herring
Quoted 57 lines · Rob Herring
Quoted 43 lines · Peter Chen

The users can change it through dts or platform data if they want to change the default value.

---
 drivers/usb/chipidea/bits.h  |  3 +++
 drivers/usb/chipidea/core.c  | 14 ++++++++++++++
 include/linux/usb/chipidea.h |  2 ++
 3 files changed, 19 insertions(+)
drivers/usb/chipidea/bits.h +3 -0
index 831a8f6..462ad02 100644
@@ -25,6 +25,9 @@
 #define VERSION                      (0xF << 25)
 #define CIVERSION            (0x7 << 29)
 
+/* SBUSCFG */
+#define AHBBRST_MASK           0x7
+
 /* HCCPARAMS */
 #define HCCPARAMS_LEN         BIT(17)
 
drivers/usb/chipidea/core.c +4 -0
index 38bd036..f60cbf6 100644
@@ -439,6 +439,9 @@ void ci_platform_configure(struct ci_hdrc *ci)
 
        hw_write(ci, OP_USBCMD, 0xff0000, ci->platdata->itc_setting << 16);
 
+       if (ci->platdata->flags & CI_HDRC_OVERRIDE_AHB_BURST)
+               hw_write_id_reg(ci, ID_SBUSCFG, AHBBRST_MASK,
+                       ci->platdata->ahb_burst_config);
 }
 
 /**
@@ -647,6 +650,17 @@ static int ci_get_platdata(struct device *dev,
                }
        }
 
+       if (of_find_property(dev->of_node, "ahb-burst-config", NULL)) {

drivers/usb/chipidea/core.c static int ci_get_platdata(struct device *dev, · line 663

Quoted 10 lines
drivers/usb/chipidea/core.c +10 -0
+               ret = of_property_read_u32(dev->of_node, "ahb-burst-config",
+                       &platdata->ahb_burst_config);
+               if (ret) {
+                       dev_err(dev,
+                               "failed to get ahb-burst-config\n");
+                       return ret;
+               }
+               platdata->flags |= CI_HDRC_OVERRIDE_AHB_BURST;
+       }
+

This can be simplified to:

Quoted 7 lines

ret = of_property_read_u32(dev->of_node, "ahb-burst-config", &platdata->ahb_burst_config);
if (!ret)
platdata->flags |= CI_HDRC_OVERRIDE_AHB_BURST;

The error condition can't really happen.

Why the error condition can't really happen? If the value of this property at dts is invalid, we would like return error.

For non-exist property, it is normal case; for invalid property, we would like return error, and quit.

Signature · 2 lines
Best Regards,
Peter Chen
author
#11 lore raw

PATCH v2 7/10 usb: chipidea: usbmisc_imx: add non-burst setting for imx6

In reply to Peter Chen

With this setting and AHBBRST at SBUSCFG as "Incremental burst of unspecified length", each non-burst size can be taken as single transfer. It is benefit for non-burst size transfer.

---
 drivers/usb/chipidea/usbmisc_imx.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
drivers/usb/chipidea/usbmisc_imx.c +10 -2
index 3cefd49..5ddab30 100644
@@ -54,6 +54,7 @@
 #define MX53_USB_PHYCTRL1_PLLDIV_MASK	0x3
 #define MX53_USB_PLL_DIV_24_MHZ		0x01
 
+#define MX6_BM_NON_BURST_SETTING	BIT(1)
 #define MX6_BM_OVER_CUR_DIS		BIT(7)
 #define MX6_BM_WAKEUP_ENABLE		BIT(10)
 #define MX6_BM_ID_WAKEUP		BIT(16)
@@ -255,14 +256,21 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
 	if (data->index > 3)
 		return -EINVAL;
 
+	spin_lock_irqsave(&usbmisc->lock, flags);
+
 	if (data->disable_oc) {
-		spin_lock_irqsave(&usbmisc->lock, flags);
 		reg = readl(usbmisc->base + data->index * 4);
 		writel(reg | MX6_BM_OVER_CUR_DIS,
 			usbmisc->base + data->index * 4);
-		spin_unlock_irqrestore(&usbmisc->lock, flags);
 	}
 
+	/* SoC non-burst setting */
+	reg = readl(usbmisc->base + data->index * 4);
+	writel(reg | MX6_BM_NON_BURST_SETTING,
+			usbmisc->base + data->index * 4);
+
+	spin_unlock_irqrestore(&usbmisc->lock, flags);
+
 	usbmisc_imx6q_set_wakeup(data, false);
 
 	return 0;
Signature · 1 line
1.9.1
author
#12 lore raw

PATCH v2 8/10 Doc: usb: ci-hdrc-usb2: add tx(rx)-burst-config-dword for binding doc

In reply to Peter Chen

It is used to override the default setting for burst size, changing burst size takes effect only when the SBUSCFG.AHBBRST = 0.

---
 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 6 ++++++
 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +6 -0
index 3c377a3..0f0929c 100644
@@ -34,6 +34,10 @@ Optional properties:
   should be aligned with ITC bits at register USBCMD.
 - ahb-burst-config: it is vendor dependent, the required value should be
   aligned with AHBBRST at SBUSCFG, the range is from 0x0 to 0x7.
+- tx-burst-size-dword: it is vendor dependent, the tx burst size in dword
+  (4 bytes).
+- rx-burst-size-dword: it is vendor dependent, the rx burst size in dword
+  (4 bytes).
 
 Example:
 
@@ -48,4 +52,6 @@ Example:
 		gadget-itc-setting = <0x4>; /* 4 micro-frames */
 		 /* Incremental burst of unspecified length */
 		ahb-burst-config = <0x0>;
+		tx-burst-size-dword = <0x10>; /* 64 bytes */
+		rx-burst-size-dword = <0x10>;
 	};
Signature · 1 line
1.9.1
author
#13 lore raw

PATCH v2 9/10 ARM: imx6: change default burst size for USB

In reply to Peter Chen

It can improve the USB performance when choosing larger burst size at some systems (bus size is larger), there is no side effect if this burst size is larger than bus size.

---
 arch/arm/boot/dts/imx6qdl.dtsi | 8 ++++++++
 arch/arm/boot/dts/imx6sl.dtsi  | 6 ++++++
 arch/arm/boot/dts/imx6sx.dtsi  | 6 ++++++
 3 files changed, 20 insertions(+)
arch/arm/boot/dts/imx6qdl.dtsi +8 -0
index 42d62b0..b9f2e3a 100644
@@ -854,6 +854,8 @@
 				fsl,usbphy = <&usbphy1>;
 				fsl,usbmisc = <&usbmisc 0>;
 				ahb-burst-config = <0x0>;
+				tx-burst-size-dword = <0x10>;
+				rx-burst-size-dword = <0x10>;
 				status = "disabled";
 			};
 
@@ -866,6 +868,8 @@
 				fsl,usbmisc = <&usbmisc 1>;
 				dr_mode = "host";
 				ahb-burst-config = <0x0>;
+				tx-burst-size-dword = <0x10>;
+				rx-burst-size-dword = <0x10>;
 				status = "disabled";
 			};
 
@@ -877,6 +881,8 @@
 				fsl,usbmisc = <&usbmisc 2>;
 				dr_mode = "host";
 				ahb-burst-config = <0x0>;
+				tx-burst-size-dword = <0x10>;
+				rx-burst-size-dword = <0x10>;
 				status = "disabled";
 			};
 
@@ -888,6 +894,8 @@
 				fsl,usbmisc = <&usbmisc 3>;
 				dr_mode = "host";
 				ahb-burst-config = <0x0>;
+				tx-burst-size-dword = <0x10>;
+				rx-burst-size-dword = <0x10>;
 				status = "disabled";
 			};
 
arch/arm/boot/dts/imx6sl.dtsi +6 -0
index 066fac9..627549f 100644
@@ -689,6 +689,8 @@
 				fsl,usbphy = <&usbphy1>;
 				fsl,usbmisc = <&usbmisc 0>;
 				ahb-burst-config = <0x0>;
+				tx-burst-size-dword = <0x10>;
+				rx-burst-size-dword = <0x10>;
 				status = "disabled";
 			};
 
@@ -700,6 +702,8 @@
 				fsl,usbphy = <&usbphy2>;
 				fsl,usbmisc = <&usbmisc 1>;
 				ahb-burst-config = <0x0>;
+				tx-burst-size-dword = <0x10>;
+				rx-burst-size-dword = <0x10>;
 				status = "disabled";
 			};
 
@@ -711,6 +715,8 @@
 				fsl,usbmisc = <&usbmisc 2>;
 				dr_mode = "host";
 				ahb-burst-config = <0x0>;
+				tx-burst-size-dword = <0x10>;
+				rx-burst-size-dword = <0x10>;
 				status = "disabled";
 			};
 
arch/arm/boot/dts/imx6sx.dtsi +6 -0
index 3656a1d..6844e9e 100644
@@ -747,6 +747,8 @@
 				fsl,usbmisc = <&usbmisc 0>;
 				fsl,anatop = <&anatop>;
 				ahb-burst-config = <0x0>;
+				tx-burst-size-dword = <0x10>;
+				rx-burst-size-dword = <0x10>;
 				status = "disabled";
 			};
 
@@ -758,6 +760,8 @@
 				fsl,usbphy = <&usbphy2>;
 				fsl,usbmisc = <&usbmisc 1>;
 				ahb-burst-config = <0x0>;
+				tx-burst-size-dword = <0x10>;
+				rx-burst-size-dword = <0x10>;
 				status = "disabled";
 			};
 
@@ -771,6 +775,8 @@
 				fsl,anatop = <&anatop>;
 				dr_mode = "host";
 				ahb-burst-config = <0x0>;
+				tx-burst-size-dword = <0x10>;
+				rx-burst-size-dword = <0x10>;
 				status = "disabled";
 			};
 
Signature · 1 line
1.9.1
author
#14 lore raw

PATCH v2 10/10 usb: chipidea: add tx/rx burst size configuration interface

In reply to Peter Chen

The user can adjust it through dts or platform data

---
 drivers/usb/chipidea/bits.h  |  4 ++++
 drivers/usb/chipidea/ci.h    |  1 +
 drivers/usb/chipidea/core.c  | 35 +++++++++++++++++++++++++++++++++++
 include/linux/usb/chipidea.h |  4 ++++
 4 files changed, 44 insertions(+)
drivers/usb/chipidea/bits.h +4 -0
index 462ad02..e462f55 100644
@@ -61,6 +61,10 @@
 /* Set non-zero value for internal TT Hub address representation */
 #define TTCTRL_TTHA		(0x7fUL << 24)
 
+/* BURSTSIZE */
+#define RX_BURST_MASK		0xff
+#define TX_BURST_MASK		0xff00
+
 /* PORTSC */
 #define PORTSC_CCS            BIT(0)
 #define PORTSC_CSC            BIT(1)
drivers/usb/chipidea/ci.h +1 -0
index b562544..41d7cf6 100644
@@ -51,6 +51,7 @@ enum ci_hw_regs {
 	OP_DEVICEADDR,
 	OP_ENDPTLISTADDR,
 	OP_TTCTRL,
+	OP_BURSTSIZE,
 	OP_PORTSC,
 	OP_DEVLC,
 	OP_OTGSC,
drivers/usb/chipidea/core.c +35 -0
index f60cbf6..0375412 100644
@@ -86,6 +86,7 @@ static const u8 ci_regs_nolpm[] = {
 	[OP_DEVICEADDR]		= 0x14U,
 	[OP_ENDPTLISTADDR]	= 0x18U,
 	[OP_TTCTRL]		= 0x1CU,
+	[OP_BURSTSIZE]		= 0x20U,
 	[OP_PORTSC]		= 0x44U,
 	[OP_DEVLC]		= 0x84U,
 	[OP_OTGSC]		= 0x64U,
@@ -109,6 +110,7 @@ static const u8 ci_regs_lpm[] = {
 	[OP_DEVICEADDR]		= 0x14U,
 	[OP_ENDPTLISTADDR]	= 0x18U,
 	[OP_TTCTRL]		= 0x1CU,
+	[OP_BURSTSIZE]		= 0x20U,
 	[OP_PORTSC]		= 0x44U,
 	[OP_DEVLC]		= 0x84U,
 	[OP_OTGSC]		= 0xC4U,
@@ -442,6 +444,17 @@ void ci_platform_configure(struct ci_hdrc *ci)
 	if (ci->platdata->flags & CI_HDRC_OVERRIDE_AHB_BURST)
 		hw_write_id_reg(ci, ID_SBUSCFG, AHBBRST_MASK,
 			ci->platdata->ahb_burst_config);
+
+	/* override burst size, take effect only when ahb_burst_config is 0 */
+	if (!hw_read_id_reg(ci, ID_SBUSCFG, AHBBRST_MASK)) {
+		if (ci->platdata->flags & CI_HDRC_OVERRIDE_TX_BURST)
+			hw_write(ci, OP_BURSTSIZE, TX_BURST_MASK,
+			ci->platdata->tx_burst_size << __ffs(TX_BURST_MASK));
+
+		if (ci->platdata->flags & CI_HDRC_OVERRIDE_RX_BURST)
+			hw_write(ci, OP_BURSTSIZE, RX_BURST_MASK,
+				ci->platdata->rx_burst_size);
+	}
 }
 
 /**
@@ -661,6 +674,28 @@ static int ci_get_platdata(struct device *dev,
 		platdata->flags |= CI_HDRC_OVERRIDE_AHB_BURST;
 	}
 
+	if (of_find_property(dev->of_node, "tx-burst-size-dword", NULL)) {
+		ret = of_property_read_u32(dev->of_node, "tx-burst-size-dword",
+			&platdata->tx_burst_size);
+		if (ret) {
+			dev_err(dev,
+				"failed to get tx-burst-size-dword\n");
+			return ret;
+		}
+		platdata->flags |= CI_HDRC_OVERRIDE_TX_BURST;
+	}
+
+	if (of_find_property(dev->of_node, "rx-burst-size-dword", NULL)) {
+		ret = of_property_read_u32(dev->of_node, "rx-burst-size-dword",
+			&platdata->rx_burst_size);
+		if (ret) {
+			dev_err(dev,
+				"failed to get rx-burst-size-dword\n");
+			return ret;
+		}
+		platdata->flags |= CI_HDRC_OVERRIDE_RX_BURST;
+	}
+
 	return 0;
 }
 
include/linux/usb/chipidea.h +4 -0
index fe3c8eb..a41833c 100644
@@ -34,6 +34,8 @@ struct ci_hdrc_platform_data {
 #define CI_HDRC_TURN_VBUS_EARLY_ON	BIT(7)
 #define CI_HDRC_SET_NON_ZERO_TTHA	BIT(8)
 #define CI_HDRC_OVERRIDE_AHB_BURST	BIT(9)
+#define CI_HDRC_OVERRIDE_TX_BURST	BIT(10)
+#define CI_HDRC_OVERRIDE_RX_BURST	BIT(11)
 	enum usb_dr_mode	dr_mode;
 #define CI_HDRC_CONTROLLER_RESET_EVENT		0
 #define CI_HDRC_CONTROLLER_STOPPED_EVENT	1
@@ -44,6 +46,8 @@ struct ci_hdrc_platform_data {
 	/* interrupt threshold setting */
 	u32			itc_setting;
 	u32			ahb_burst_config;
+	u32			tx_burst_size;
+	u32			rx_burst_size;
 };
 
 /* Default offset of capability registers */
Signature · 1 line
1.9.1
#15 lore raw

v2 cover USB: chipidea misc patches

In reply to Peter Chen
Quoted 9 lines · Peter Chen

Hi all,

In this series, I add several new interfaces for chipidea driver, they are mainly for system configuration adjustment. The USB performance may be improved with these configuration changing, but each vendor driver owner needs to consult with your IC owner which configuration parameters are suitable for your system before changing it, and only change it if necessary.

Quoted 3 lines

The other changes are for imx, it shows how imx uses these interfaces. Shawn, the patch [5/10] and [9/10] are dts changes, help to review it please.

Ping me, after driver and bindings are accepted. Then I will pick up the dts changes.

Shawn

author
#16 lore raw

v2 cover USB: chipidea misc patches

Quoted 10 lines · Shawn Guo
Quoted 9 lines · Peter Chen

Hi all,

In this series, I add several new interfaces for chipidea driver, they are mainly for system configuration adjustment. The USB performance may be improved with these configuration changing, but each vendor driver owner needs to consult with your IC owner which configuration parameters are suitable for your system before changing it, and only change it if necessary.

Quoted 7 lines
Quoted 3 lines

The other changes are for imx, it shows how imx uses these interfaces. Shawn, the patch [5/10] and [9/10] are dts changes, help to review it please.

Ping me, after driver and bindings are accepted. Then I will pick up the dts changes.

Shawn, The driver and bingings have been accepted. And I send v2 changes for you, which apply changes for imx6ul.

Signature · 2 lines
Best Regards,
Peter Chen