[PATCH] USB: serial: mos7840: Add a product ID for the new product
From: JackyChou
Add a new PID 0x7843 to the driver.
Let the new products be able to set up 3 serial ports with the driver.
Because the development of new product is based on 4 serial ports,
but some users only need 3 serial ports. There is no way to set it from
the hardware, so let the driver set 3 serial ports with PID 0x7843.
Signed-off-by: JackyChou
---
drivers/usb/serial/mos7840.c | 45 ++--
1 file changed, 22 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index b42bad85097a..57ef25a2f7bb 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -94,6 +94,7 @@
/* The native mos7840/7820 component */
#define USB_VENDOR_ID_MOSCHIP 0x9710
#define MOSCHIP_DEVICE_ID_7840 0x7840
+#define MOSCHIP_DEVICE_ID_7843 0x7843
#define MOSCHIP_DEVICE_ID_7820 0x7820
#define MOSCHIP_DEVICE_ID_7810 0x7810
/* The native component can have its vendor/device id's overridden
@@ -176,6 +177,7 @@ enum mos7840_flag {
static const struct usb_device_id id_table[] = {
{USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7840)},
+ {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7843)},
{USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7820)},
{USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7810)},
{USB_DEVICE(USB_VENDOR_ID_BANDB, BANDB_DEVICE_ID_USO9ML2_2)},
@@ -298,15 +300,10 @@ static int mos7840_set_uart_reg(struct usb_serial_port
*port, __u16 reg,
val = val & 0x00ff;
/* For the UART control registers, the application number need
to be Or'ed */
- if (port->serial->num_ports == 4) {
+ if (port->serial->num_ports == 2 && port->port_number != 0)
+ val |= ((__u16)port->port_number + 2) << 8;
+ else
val |= ((__u16)port->port_number + 1) << 8;
- } else {
- if (port->port_number == 0) {
- val |= ((__u16)port->port_number + 1) << 8;
- } else {
- val |= ((__u16)port->port_number + 2) << 8;
- }
- }
dev_dbg(&port->dev, "%s application number is %x\n", __func__, val);
return usb_control_msg(dev, usb_sndctrlpipe(dev, 0), MCS_WRREQ,
MCS_WR_RTYPE, val, reg, NULL, 0,
@@ -332,15 +329,10 @@ static int mos7840_get_uart_reg(struct usb_serial_port
*port, __u16 reg,
return -ENOMEM;
/* Wval is same as application number */
- if (port->serial->num_ports == 4) {
+ if (port->serial->num_ports == 2 && port->port_number != 0)
+ Wval = ((__u16)port->port_number + 2) << 8;
+ else
Wval = ((__u16)port->port_number + 1) << 8;
- } else {
- if (port->port_number == 0) {
- Wval = ((__u16)port->port_number + 1) << 8;
- } else {
- Wval = ((__u16)port->port_number + 2) << 8;
- }
- }
dev_dbg(&port->dev, "%s application number is %x\n", __func__,
Wval);
ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), MCS_RDREQ,
MCS_RD_RTYPE, Wval, reg, buf,
VENDOR_READ_LENGTH,
@@ -2071,12 +2063,16 @@ static int mos7840_probe(struct usb_serial *serial,
VENDOR_READ_LENGTH, MOS_WDR_TIMEOUT);
/* For a MCS7840 device GPIO0 must be set to 1 */
- if (buf[0] & 0x01)
- device_type = MOSCHIP_DEVICE_ID_7840;
- else if (mos7810_check(serial))
+ if (buf[0] & 0x01) {
+ if (product == MOSCHIP_DEVICE_ID_7843)
+ device_type = MOSCHIP_DEVICE_ID_7843;
+ else
+ device_type = MOSCHIP_DEVICE_ID_7840;
+ } else if (mos7810_check(serial)) {
device_type = MOSCHIP_DEVICE_ID_7810;
- else
+ } else {
device_type = MOSCHIP_DEVICE_ID_7820;
+ }
kfree(buf);
out:
@@ -2091,7 +2087,10 @@ static int mos7840_calc_num_ports(struct usb_serial
*serial,
int device_type = (unsigned long)usb_get_serial_data(serial);
int num_ports;
- num_ports = (device_type >> 4) & 0x000F;
+ if (device_type == MOSCHIP_DEVICE_ID_7843)
+ num_ports = 3;
+ else
+ num_ports = (device_type >> 4) & 0x000F;
/*
* num_ports is currently never zero as device_type is one of
@@ -2146,7 +2145,7 @@ static int mos7840_port_probe(struct usb_serial_port
*port)
mos7840_port->SpRegOffset = 0x0;
mos7840_port->ControlRegOffset = 0x1;
mos7840_port->DcrRegOffset = 0x4;
- } else if ((mos7840_port->port_num == 2) && (serial->num_ports ==
4)) {
+ } else if ((mos7840_port->port_num == 2) && (serial->num_ports > 2))
{
mos7840_port->SpRegOffset = 0x8;
[PATCH v3 2/2] usb: dwc3: gadget: Report isoc transfer frame number
Implement the new frame_number API to report the isochronous interval
frame number. This patch checks and reports the interval in which the
isoc transfer was transmitted or received via the Isoc-First TRB SOF
number field.
Signed-off-by: Thinh Nguyen
---
Change in v3:
- Implement the change with the redefined frame_number meaning
Change in v2:
- Capture frame number at request cleanup
drivers/usb/dwc3/core.h | 1 +
drivers/usb/dwc3/gadget.c | 13 +
2 files changed, 14 insertions(+)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index ed0359d1216d..2c9f7a93147a 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -777,6 +777,7 @@ enum dwc3_link_state {
#define DWC3_TRB_CTRL_ISP_IMI BIT(10)
#define DWC3_TRB_CTRL_IOC BIT(11)
#define DWC3_TRB_CTRL_SID_SOFN(n) (((n) & 0x) << 14)
+#define DWC3_TRB_CTRL_GET_SID_SOFN(n) (((n) & (0x << 14)) >> 14)
#define DWC3_TRBCTL_TYPE(n)((n) & (0x3f << 4))
#define DWC3_TRBCTL_NORMAL DWC3_TRB_CTRL_TRBCTL(1)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 679c12e14522..2de563124fc1 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2254,6 +2254,19 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct
dwc3_ep *dep,
if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO))
trb->ctrl &= ~DWC3_TRB_CTRL_HWO;
+ /*
+* For isochronous transfers, the first TRB in a service interval must
+* have the Isoc-First type. Track and report its interval frame number.
+*/
+ if (usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
+ (trb->ctrl & DWC3_TRBCTL_ISOCHRONOUS_FIRST)) {
+ unsigned int frame_number;
+
+ frame_number = DWC3_TRB_CTRL_GET_SID_SOFN(trb->ctrl);
+ frame_number &= ~(dep->interval - 1);
+ req->request.frame_number = frame_number;
+ }
+
/*
* If we're dealing with unaligned size OUT transfer, we will be left
* with one TRB pending in the ring. We need to manually clear HWO bit
--
2.11.0
[PATCH v3 1/2] usb: gadget: Introduce frame_number to usb_request
Add a field frame_number to the usb_request to report the interval
number in (micro)frames in which the isochronous transfer was
transmitted or received. The gadget driver can use this knowledge to
synchronize with the host. Also, this option is useful for debugging
purposes.
Signed-off-by: Thinh Nguyen
---
Change in v3:
- Rename the "start_frame" field to "frame_number"
- Redefine the frame_number meaning and purpose
Change in v2:
- None
include/linux/usb/gadget.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index e5cd84a0f84a..7595056b96c1 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -61,6 +61,8 @@ struct usb_ep;
* invalidated by the error may first be dequeued.
* @context: For use by the completion callback
* @list: For use by the gadget driver.
+ * @frame_number: Reports the interval number in (micro)frame in which the
+ * isochronous transfer was transmitted or received.
* @status: Reports completion code, zero or a negative errno.
* Normally, faults block the transfer queue from advancing until
* the completion callback returns.
@@ -112,6 +114,8 @@ struct usb_request {
void*context;
struct list_headlist;
+ unsignedframe_number; /* ISO ONLY */
+
int status;
unsignedactual;
};
--
2.11.0
Re: [PATCH v3 2/2] Embedded USB Debugger (EUD) driver
On Thu, Nov 15, 2018 at 11:32:54AM -0800, Prakruthi Deepak Heragu wrote:
> +struct device_attribute eud_attribute = {
> + .attr.name = "enable",
> + .attr.mode = 0644,
> + .show = eud_enable_show,
> + .store = eud_enable_store,
> +};
Please use:
static DEVICE_ATTR_RW(enable);
instead of open-coding all of that mess.
Also it makes it static, this should not be a global symbol.
> +
> +static void eud_event_notifier(struct work_struct *eud_work)
> +{
> + struct eud_chip *chip = container_of(eud_work, struct eud_chip,
> + eud_work);
> + int ret;
> +
> + if (chip->int_status == EUD_INT_VBUS) {
> + ret = extcon_set_state_sync(chip->extcon, chip->extcon_id,
> + chip->usb_attach);
> + if (ret)
> + return;
> + } else if (chip->int_status == EUD_INT_CHGR) {
> + ret = extcon_set_state_sync(chip->extcon, chip->extcon_id,
> + chip->chgr_enable);
> + if (ret)
> + return;
> + }
> +}
> +
> +static void usb_attach_detach(struct eud_chip *chip)
> +{
> + u32 reg;
> +
> + chip->extcon_id = EXTCON_USB;
> + /* read ctl_out_1[4] to find USB attach or detach event */
> + reg = readl_relaxed(chip->eud_reg_base + EUD_REG_CTL_OUT_1);
> + if (reg & BIT(4))
> + chip->usb_attach = true;
> + else
> + chip->usb_attach = false;
> +
> + schedule_work(&chip->eud_work);
> +
> + /* set and clear vbus_int_clr[0] to clear interrupt */
> + writel_relaxed(BIT(0), chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
> + /* Ensure Register Writes Complete */
> + wmb();
> + writel_relaxed(0, chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
> +}
> +
> +static void chgr_enable_disable(struct eud_chip *chip)
> +{
> + u32 reg;
> +
> + chip->extcon_id = EXTCON_CHG_USB_SDP;
> + /* read ctl_out_1[6] to find charger enable or disable event */
> + reg = readl_relaxed(chip->eud_reg_base + EUD_REG_CTL_OUT_1);
> + if (reg & BIT(6))
> + chip->chgr_enable = true;
> + else
> + chip->chgr_enable = false;
> +
> + schedule_work(&chip->eud_work);
> +
> + /* set and clear chgr_int_clr[0] to clear interrupt */
> + writel_relaxed(BIT(0), chip->eud_reg_base + EUD_REG_CHGR_INT_CLR);
> + /* Ensure Register Writes Complete */
> + wmb();
> + writel_relaxed(0, chip->eud_reg_base + EUD_REG_CHGR_INT_CLR);
> +}
> +
> +static void pet_eud(struct eud_chip *chip)
> +{
> + u32 reg;
> +
> + /* read sw_attach_det[0] to find attach/detach event */
> + reg = readl_relaxed(chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
> + if (reg & BIT(0)) {
> + /* Detach & Attach pet for EUD */
> + writel_relaxed(0, chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
> + /* Ensure Register Writes Complete */
> + wmb();
> + /* Delay to make sure detach pet is done before attach pet */
> + udelay(100);
> + writel_relaxed(BIT(0), chip->eud_reg_base +
> + EUD_REG_SW_ATTACH_DET);
> + /* Ensure Register Writes Complete */
> + wmb();
> + } else {
> + /* Attach pet for EUD */
> + writel_relaxed(BIT(0), chip->eud_reg_base +
> + EUD_REG_SW_ATTACH_DET);
> + /* Ensure Register Writes Complete */
> + wmb();
> + }
> +}
> +
> +static irqreturn_t handle_eud_irq(int irq, void *data)
> +{
> + struct eud_chip *chip = data;
> + u32 reg;
> +
> + /* read status register and find out which interrupt triggered */
> + reg = readl_relaxed(chip->eud_reg_base + EUD_REG_INT_STATUS_1);
> + switch (reg & EUD_INT_ALL) {
> + case EUD_INT_VBUS:
> + chip->int_status = EUD_INT_VBUS;
> + usb_attach_detach(chip);
> + break;
> + case EUD_INT_CHGR:
> + chip->int_status = EUD_INT_CHGR;
> + chgr_enable_disable(chip);
> + break;
> + case EUD_INT_SAFE_MODE:
> + pet_eud(chip);
> + break;
> + default:
> + return IRQ_NONE;
> + }
> + return IRQ_HANDLED;
> +}
> +
> +static int msm_eud_probe(struct platform_device *pdev)
> +{
> + struct eud_chip *chip;
> + struct resource *res;
> + int ret;
> +
> + chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
> + if (!chip)
> + return -ENOMEM;
> +
> + chip->dev = &pdev->dev;
> + platform_set_drvdata(pdev, chip);
> +
> + ret = device_create_file(&pdev->dev, &eud_attribute);
> + if (ret)
> + return ret;
You just raced with userspace and lost :(
Isn't there a way to add the attribute to the platform device as a group
to have it correctly added by the driver core?
Also, you are adding an attri
Re: [PATCH v3 2/2] Embedded USB Debugger (EUD) driver
On Thu, Nov 15, 2018 at 11:32:54AM -0800, Prakruthi Deepak Heragu wrote: > Add support for control peripheral of EUD (Embedded USB Debugger) to > listen to events such as USB attach/detach, charger enable/disable, pet > EUD to indicate software is functional. Reusing the platform device kobj, > sysfs entry 'enable' is created to enable or disable EUD. If you add/remove/change a sysfs file, you need to also have a Documentation/ABI/ file update as well. Please do that here. thanks, greg k-h
[PATCH v3 1/2] dt-bindings: Documentation for qcom,eud
Documentation for Embedded USB Debugger (EUD) device tree bindings.
Signed-off-by: Satya Durga Srinivasu Prabhala
Signed-off-by: Prakruthi Deepak Heragu
---
.../devicetree/bindings/soc/qcom/qcom,msm-eud.txt | 43 ++
1 file changed, 43 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
b/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
new file mode 100644
index 000..6d339e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
@@ -0,0 +1,43 @@
+* Qualcomm Technologies Inc Embedded USB Debugger (EUD)
+
+The EUD (Embedded USB Debugger) is a mini-USB hub implemented
+on chip to support the USB-based debug and trace capabilities.
+
+Required properties:
+
+ - compatible: Should be "qcom,msm-eud"
+ - interrupts: Interrupt number
+ - reg: Should be address and size of EUD register space
+
+EUD notifies clients for VBUS attach/detach and charger enable/disable
+events. The link between client and EUD is established via a directed
+graph. EUD has one endpoint of the graph link mentioning EUD as an
+output link and clients of EUD should have the other endpoint of the
+graph link as an input link.
+
+An example for EUD device node:
+
+ eud: qcom,msm-eud@88e {
+ compatible = "qcom,msm-eud";
+ interrupts = ;
+ reg = <0x88e 0x4000>;
+ usb_con: connector {
+compatible = "usb-c-connector";
+label = "USB-C";
+port {
+ eud_usb_output: endpoint {
+ remote-endpoint = <&eud_usb3_input>;
+};
+ };
+};
+ };
+
+An example for EUD client:
+
+ usb3 {
+ port {
+eud_usb3_input: endpoint {
+remote-endpoint = <&eud_usb_output>;
+};
+};
+ };
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v3 2/2] Embedded USB Debugger (EUD) driver
Add support for control peripheral of EUD (Embedded USB Debugger) to
listen to events such as USB attach/detach, charger enable/disable, pet
EUD to indicate software is functional. Reusing the platform device kobj,
sysfs entry 'enable' is created to enable or disable EUD.
Signed-off-by: Satya Durga Srinivasu Prabhala
Signed-off-by: Prakruthi Deepak Heragu
---
drivers/soc/qcom/Kconfig | 12 ++
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/eud.c| 344 ++
3 files changed, 357 insertions(+)
create mode 100644 drivers/soc/qcom/eud.c
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 5856e79..12669ec 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -136,4 +136,16 @@ config QCOM_APR
application processor and QDSP6. APR is
used by audio driver to configure QDSP6
ASM, ADM and AFE modules.
+
+config QCOM_EUD
+ tristate "QTI Embedded USB Debugger (EUD)"
+ depends on ARCH_QCOM
+ help
+ The Embedded USB Debugger (EUD) driver is a driver for the
+ control peripheral which waits on events like USB attach/detach
+ and charger enable/disable. The control peripheral further helps
+ support the USB-based debug and trace capabilities.
+ This module enables support for Qualcomm Technologies, Inc.
+ Embedded USB Debugger (EUD).
+ If unsure, say N.
endmenu
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 19dcf95..dd4701b 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_QCOM_SMP2P) += smp2p.o
obj-$(CONFIG_QCOM_SMSM)+= smsm.o
obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
obj-$(CONFIG_QCOM_APR) += apr.o
+obj-$(CONFIG_QCOM_EUD) += eud.o
diff --git a/drivers/soc/qcom/eud.c b/drivers/soc/qcom/eud.c
new file mode 100644
index 000..bd885d6
--- /dev/null
+++ b/drivers/soc/qcom/eud.c
@@ -0,0 +1,344 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define EUD_ENABLE_CMD 1
+#define EUD_DISABLE_CMD 0
+
+#define EUD_REG_INT1_EN_MASK 0x0024
+#define EUD_REG_INT_STATUS_1 0x0044
+#define EUD_REG_CTL_OUT_1 0x0074
+#define EUD_REG_VBUS_INT_CLR 0x0080
+#define EUD_REG_CHGR_INT_CLR 0x0084
+#define EUD_REG_CSR_EUD_EN 0x1014
+#define EUD_REG_SW_ATTACH_DET 0x1018
+
+#define EUD_INT_VBUS BIT(2)
+#define EUD_INT_CHGR BIT(3)
+#define EUD_INT_SAFE_MODE BIT(4)
+#define EUD_INT_ALL(EUD_INT_VBUS|EUD_INT_CHGR|\
+ EUD_INT_SAFE_MODE)
+
+struct eud_chip {
+ struct device *dev;
+ int eud_irq;
+ unsigned intextcon_id;
+ unsigned intint_status;
+ boolusb_attach;
+ boolchgr_enable;
+ void __iomem*eud_reg_base;
+ struct extcon_dev *extcon;
+ int enable;
+ struct work_struct eud_work;
+};
+
+static const unsigned int eud_extcon_cable[] = {
+ EXTCON_USB,
+ EXTCON_CHG_USB_SDP,
+ EXTCON_NONE,
+};
+
+static int enable_eud(struct eud_chip *priv)
+{
+ struct power_supply *usb_psy = NULL;
+ union power_supply_propval pval = {0};
+ union power_supply_propval tval = {0};
+ int ret;
+
+ usb_psy = power_supply_get_by_name("usb");
+ if (!usb_psy)
+ return -1;
+
+ ret = power_supply_get_property(usb_psy,
+ POWER_SUPPLY_PROP_PRESENT, &pval);
+ if (ret)
+ return ret;
+
+ ret = power_supply_get_property(usb_psy,
+ POWER_SUPPLY_PROP_REAL_TYPE, &tval);
+ if (ret)
+ return ret;
+
+ if (!pval.intval || (tval.intval != POWER_SUPPLY_TYPE_USB &&
+ tval.intval != POWER_SUPPLY_TYPE_USB_CDP))
+ return -1;
+
+ /* write into CSR to enable EUD */
+ writel_relaxed(BIT(0), priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
+ /* Enable vbus, chgr & safe mode warning interrupts */
+ writel_relaxed(EUD_INT_VBUS | EUD_INT_CHGR | EUD_INT_SAFE_MODE,
+ priv->eud_reg_base + EUD_REG_INT1_EN_MASK);
+
+ /* Ensure Register Writes Complete */
+ wmb();
+
+ /*
+* Set the default cable state to usb connect and charger
+* enable
+*/
+ ret = extcon_set_state_sync(priv->extcon, EXTCON_USB, true);
+ if (ret)
+ return ret;
+ ret = extcon_set_state_sync(priv->extcon,
+ EXTCON_CHG_USB_SDP, true);
+ if (ret
[PATCH v3 0/2] Add Embedded USB Debugger (EUD) driver
This is a series of patches that implements a driver for the control peripheral, EUD (Embedded USB Debugger). The EUD is a mini-USB hub implemented on chip to support the USB-based debug and trace capabilities. Apart from debug capabilities, EUD has a control peripheral. Control Peripheral is on when EUD is on and gets signals like USB attach, pet EUD, charge phone etc. EUD driver listens to events like USB attach or detach and charger enable or disable and then notifies the USB driver or PMIC driver respectively about these events via EXTCON. At regular intervals, the EUD driver receives an interrupt to pet the driver indicating that the software is functional. Changes since v2: - Remove module_param and add sysfs support instead - Simplify if-else condition - Change if-elseif to switch case - Return -ENOMEM - Got rid of unnecessary checks in sysfs store function - Updated the dt-binding Changes since v1: - Remove EUD_NR as it is an unused macro Changes since v0: - Remove select SERIAL_CORE from Kconfig as this patch doesn't involve anything related to serial console - Changed the dt-bindings to remove extcon and replace it with graphs to represent a connection with client Prakruthi Deepak Heragu (2): dt-bindings: Documentation for qcom,eud Embedded USB Debugger (EUD) driver .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt | 41 +++ drivers/soc/qcom/Kconfig | 12 + drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/eud.c | 338 + 4 files changed, 392 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt create mode 100644 drivers/soc/qcom/eud.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH] usb: hub: add I/O error retry & reset routine
On Thu, 15 Nov 2018, Nicolas Saenz Julienne wrote:
> An URB submission error in the HUB's endpoint completion function
> renders the whole HUB device unresponsive. This patch introduces a
> routine that retries the submission for 1s to then, as a last resort,
> reset the whole device.
>
> The implementation is based on usbhid/hid_core.c's, which implements the
> same functionality. It was tested with the help of BCC's error injection
> tool (inject.py).
>
> Signed-off-by: Nicolas Saenz Julienne
Why do you think this is needed? Are you experiencing these
sorts of URB submission errors?
Why do you handle only errors during submission but not during
completion? And if you keep on getting errors during submission, why
would resetting the hub make any difference?
The patch doesn't delete the io_retry timer when the hub is removed.
Alan Stern
> ---
> drivers/usb/core/hub.c | 43 +-
> drivers/usb/core/hub.h | 3 +++
> 2 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index d9bd7576786a..1447bdba59ec 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -607,6 +607,44 @@ static int hub_port_status(struct usb_hub *hub, int
> port1,
> status, change, NULL);
> }
>
> +static void hub_io_error(struct usb_hub *hub)
> +{
> + /*
> + * If it has been a while since the last error, we'll assume
> + * this a brand new error and reset the retry timeout.
> + */
> + if (time_after(jiffies, hub->stop_retry + HZ/2))
> + hub->retry_delay = 0;
> +
> + /* When an error occurs, retry at increasing intervals */
> + if (hub->retry_delay == 0) {
> + hub->retry_delay = 13; /* Then 26, 52, 104, 104, ... */
> + hub->stop_retry = jiffies + msecs_to_jiffies(1000);
> + } else if (hub->retry_delay < 100)
> + hub->retry_delay *= 2;
> +
> + if (time_after(jiffies, hub->stop_retry)) {
> + /* Retries failed, so do a port reset */
> + usb_queue_reset_device(to_usb_interface(hub->intfdev));
> + return;
> + }
> +
> + mod_timer(&hub->io_retry,
> + jiffies + msecs_to_jiffies(hub->retry_delay));
> +}
> +
> +static void hub_retry_timeout(struct timer_list *t)
> +{
> + struct usb_hub *hub = from_timer(hub, t, io_retry);
> +
> + if (hub->disconnected || hub->quiescing)
> + return;
> +
> + dev_err(hub->intfdev, "retrying int urb\n");
> + if (usb_submit_urb(hub->urb, GFP_ATOMIC))
> + hub_io_error(hub);
> +}
> +
> static void kick_hub_wq(struct usb_hub *hub)
> {
> struct usb_interface *intf;
> @@ -713,8 +751,10 @@ static void hub_irq(struct urb *urb)
> return;
>
> status = usb_submit_urb(hub->urb, GFP_ATOMIC);
> - if (status != 0 && status != -ENODEV && status != -EPERM)
> + if (status != 0 && status != -ENODEV && status != -EPERM) {
> dev_err(hub->intfdev, "resubmit --> %d\n", status);
> + hub_io_error(hub);
> + }
> }
>
> /* USB 2.0 spec Section 11.24.2.3 */
> @@ -1800,6 +1840,7 @@ static int hub_probe(struct usb_interface *intf, const
> struct usb_device_id *id)
> INIT_DELAYED_WORK(&hub->leds, led_work);
> INIT_DELAYED_WORK(&hub->init_work, NULL);
> INIT_WORK(&hub->events, hub_event);
> + timer_setup(&hub->io_retry, hub_retry_timeout, 0);
> usb_get_intf(intf);
> usb_get_dev(hdev);
>
> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> index 4accfb63f7dc..7dbd19c2c8d9 100644
> --- a/drivers/usb/core/hub.h
> +++ b/drivers/usb/core/hub.h
> @@ -69,6 +69,9 @@ struct usb_hub {
> struct delayed_work leds;
> struct delayed_work init_work;
> struct work_struct events;
> + struct timer_list io_retry;
> + unsigned long stop_retry;
> + unsigned intretry_delay;
> struct usb_port **ports;
> };
>
>
RE: SMSC95xx driver updates (round 1)
Hi Ben, > -Original Message- > From: netdev-ow...@vger.kernel.org On Behalf > Of Ben Dooks > Sent: Wednesday, November 14, 2018 6:50 AM > To: net...@vger.kernel.org > Cc: oneu...@suse.com; da...@davemloft.net; linux-usb@vger.kernel.org; linux- > ker...@vger.kernel.org; steve.glendinn...@shawell.net; > linux-ker...@lists.codethink.co.uk > Subject: SMSC95xx driver updates (round 1) > > This is a series of a few driver cleanups and some fixups of the code > for the SMSC95XX driver. There have been a few reviews, and the issues > have been fixed so this should be ready for merging. > > I will work on the tx-alignment and the other bits of usbnet changes > and produce at least two more patch series for this later. Some reason, maintainer email of unglinuxdri...@microchip.com is NOT included in CC. Please add it in following patch series you are creating to have a chance to review by proper reviewers. USB SMSC95XX ETHERNET DRIVER M: Steve Glendinning M: Microchip Linux Driver Support L: net...@vger.kernel.org S: Maintained F: drivers/net/usb/smsc95xx.* Best Regards, Woojung
[PATCH] usb: hub: add I/O error retry & reset routine
An URB submission error in the HUB's endpoint completion function
renders the whole HUB device unresponsive. This patch introduces a
routine that retries the submission for 1s to then, as a last resort,
reset the whole device.
The implementation is based on usbhid/hid_core.c's, which implements the
same functionality. It was tested with the help of BCC's error injection
tool (inject.py).
Signed-off-by: Nicolas Saenz Julienne
---
drivers/usb/core/hub.c | 43 +-
drivers/usb/core/hub.h | 3 +++
2 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d9bd7576786a..1447bdba59ec 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -607,6 +607,44 @@ static int hub_port_status(struct usb_hub *hub, int port1,
status, change, NULL);
}
+static void hub_io_error(struct usb_hub *hub)
+{
+ /*
+* If it has been a while since the last error, we'll assume
+* this a brand new error and reset the retry timeout.
+*/
+ if (time_after(jiffies, hub->stop_retry + HZ/2))
+ hub->retry_delay = 0;
+
+ /* When an error occurs, retry at increasing intervals */
+ if (hub->retry_delay == 0) {
+ hub->retry_delay = 13; /* Then 26, 52, 104, 104, ... */
+ hub->stop_retry = jiffies + msecs_to_jiffies(1000);
+ } else if (hub->retry_delay < 100)
+ hub->retry_delay *= 2;
+
+ if (time_after(jiffies, hub->stop_retry)) {
+ /* Retries failed, so do a port reset */
+ usb_queue_reset_device(to_usb_interface(hub->intfdev));
+ return;
+ }
+
+ mod_timer(&hub->io_retry,
+ jiffies + msecs_to_jiffies(hub->retry_delay));
+}
+
+static void hub_retry_timeout(struct timer_list *t)
+{
+ struct usb_hub *hub = from_timer(hub, t, io_retry);
+
+ if (hub->disconnected || hub->quiescing)
+ return;
+
+ dev_err(hub->intfdev, "retrying int urb\n");
+ if (usb_submit_urb(hub->urb, GFP_ATOMIC))
+ hub_io_error(hub);
+}
+
static void kick_hub_wq(struct usb_hub *hub)
{
struct usb_interface *intf;
@@ -713,8 +751,10 @@ static void hub_irq(struct urb *urb)
return;
status = usb_submit_urb(hub->urb, GFP_ATOMIC);
- if (status != 0 && status != -ENODEV && status != -EPERM)
+ if (status != 0 && status != -ENODEV && status != -EPERM) {
dev_err(hub->intfdev, "resubmit --> %d\n", status);
+ hub_io_error(hub);
+ }
}
/* USB 2.0 spec Section 11.24.2.3 */
@@ -1800,6 +1840,7 @@ static int hub_probe(struct usb_interface *intf, const
struct usb_device_id *id)
INIT_DELAYED_WORK(&hub->leds, led_work);
INIT_DELAYED_WORK(&hub->init_work, NULL);
INIT_WORK(&hub->events, hub_event);
+ timer_setup(&hub->io_retry, hub_retry_timeout, 0);
usb_get_intf(intf);
usb_get_dev(hdev);
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 4accfb63f7dc..7dbd19c2c8d9 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -69,6 +69,9 @@ struct usb_hub {
struct delayed_work leds;
struct delayed_work init_work;
struct work_struct events;
+ struct timer_list io_retry;
+ unsigned long stop_retry;
+ unsigned intretry_delay;
struct usb_port **ports;
};
--
2.19.1
Re: Query on usb/core/devio.c
On Thu, 15 Nov 2018, Oliver Neukum wrote: > On Do, 2018-11-15 at 12:45 +, Mayuresh Kulkarni wrote: > > > > Understood this for remote-wake part. > > > > But still unclear about step (1) for host-wake as below (please note, I am > > refering to host-wake and remote-wake as per my previous comment) - > > Pre-condition: device in suspend and link in L2. > > Use-case: end user wants to sends a control message to device. > > Assumption: end user is NOT doing any activity that causes remote-wake. > > That is just an assumption you cannot make. > Users are devious creatures. If it is physically > possible, you have to be able to deal with it. That's right. But let's suppose that the user hasn't interacted with the device. In that case it's simple: The app should submit an URB to the device (or do pretty much any other ioctl to the usbfs device file descriptor). When usbfs gets an ioctl, it will automatically wake up the device by incrementing the runtime PM usage counter. Alan Stern
Re: [PATCH v2 1/3] staging: typec: fusb302: Rename fcs,extcon-name to linux,extcon-name
On Thu, Nov 15, 2018 at 03:16:19PM +0200, Andy Shevchenko wrote:
> Since we are going to use the same in Designware USB 3 driver,
> rename the property to be consistent across the drivers.
>
> No functional change intended.
>
> Signed-off-by: Andy Shevchenko
> Cc: Hans de Goede
> Cc: Guenter Roeck
> Acked-by: Hans de Goede
> Acked-by: Guenter Roeck
FWIW, the series:
Reviewed-by: Heikki Krogerus
> ---
> drivers/platform/x86/intel_cht_int33fe.c | 2 +-
> drivers/usb/typec/tcpm/fusb302.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/intel_cht_int33fe.c
> b/drivers/platform/x86/intel_cht_int33fe.c
> index 464fe93657b5..87cbf18d0305 100644
> --- a/drivers/platform/x86/intel_cht_int33fe.c
> +++ b/drivers/platform/x86/intel_cht_int33fe.c
> @@ -79,7 +79,7 @@ static const struct property_entry max17047_props[] = {
> };
>
> static const struct property_entry fusb302_props[] = {
> - PROPERTY_ENTRY_STRING("fcs,extcon-name", "cht_wcove_pwrsrc"),
> + PROPERTY_ENTRY_STRING("linux,extcon-name", "cht_wcove_pwrsrc"),
> PROPERTY_ENTRY_U32("fcs,max-sink-microvolt", 1200),
> PROPERTY_ENTRY_U32("fcs,max-sink-microamp", 300),
> PROPERTY_ENTRY_U32("fcs,max-sink-microwatt", 3600),
> diff --git a/drivers/usb/typec/tcpm/fusb302.c
> b/drivers/usb/typec/tcpm/fusb302.c
> index 43b64d9309d0..e9344997329c 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -1765,7 +1765,7 @@ static int fusb302_probe(struct i2c_client *client,
>* to be set by the platform code which also registers the i2c client
>* for the fusb302.
>*/
> - if (device_property_read_string(dev, "fcs,extcon-name", &name) == 0) {
> + if (device_property_read_string(dev, "linux,extcon-name", &name) == 0) {
> chip->extcon = extcon_get_extcon_dev(name);
> if (!chip->extcon)
> return -EPROBE_DEFER;
> --
> 2.19.1
--
heikki
Re: [PATCH] Improve the accuracy of the baud rate generator by using round-to-closest instead of truncating when calculating baud rate divisors.
On 11/15/18 9:24 AM, Johan Hovold wrote: On Tue, Nov 13, 2018 at 08:19:44PM +0100, Nikolaj Fogh wrote: On 11/12/18 10:54 AM, Johan Hovold wrote: On Wed, Oct 31, 2018 at 09:16:48PM +0100, Nikolaj Fogh wrote: I have experienced that the ftdi_sio driver gives less-than-optimal baud rates as the driver truncates instead of rounds to nearest during baud rate divisor calculation. This patch improves on the baud rate generation. The generated baud rate corresponds to the optimal baud rate achievable with the chip. This is what the windows driver gives as well. How did you verify this? Did you trace and compare the divisors actually requested by the Windows driver, or did you measure the resulting rates using a scope? I verified it by scope. Granted, I only verified it for one baud rate (961200). Whether it gives the same as the Windows driver in general, I'm not sure. However, I would think that rounding instead of flooring would always yield the most accurate result. I'm not so sure in this case. The driver uses "sub-integer" divisors and looks like it depends on truncation rather than rounding. Some background here: https://www.ftdichip.com/Support/Knowledgebase/index.html?whatbaudratesareachieveabl.htm I have had a closer look at this (and the driver code), and it seemsthat each bit in the divisorcorresponds to 1/8th (0.125) in the calculation. It is shuffled around a bit in the code (for legacy reasons I expect), and put in the higher order bits, but prior to that, I see no reason that rounding should not be used instead of truncating. I don't see how it "depends" on truncation. If you want to change these calculations you need to make a stronger case for it and verify that we don't mess up some other rate inadvertently. I have done a calculation which compares the error of the baud rate calculation going all the way from 1 to 3MBaud where it can be seen that the rounding (as expected) halves the maximum error. Whereas the old method went up to 12% baud rate error, the new method reaches 6%, so the range of baud rates where communication will be successful should increase. Also, the new method is always better or as-accurate as the old. I guess that image attachments are not welcome in the mailing list, so I will refrain from attaching it. Let me know if I should send it to you. I am using it in a system which uses a baud rate of 961200 (and not the standard 921600). Here the old calculation gave an error of 4.03% and the new gave 0.12% error. I will try to verify the numbers I have calculated with a logic analyzer to make sure that it corresponds with the real world. I can also try to compare it to the windows driver outputs. As I only have a FT232RT (232bm) to test with, the patch should probably be limited to the changes in the ftdi_232bm_baud_base_to_divisor() function. Best regards Nikolaj Fogh Thanks, Johan
[PATCH] usb: dwc3: don't print error message on probe defer
dwc3_core_init() is requesting the PHYs, which may probe later than the
USB host controller. This is a normal system operation state and thus
should not print an error message into the logs.
Signed-off-by: Lucas Stach
---
drivers/usb/dwc3/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index becfbb87f791..48fc59cdbff7 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1482,7 +1482,8 @@ static int dwc3_probe(struct platform_device *pdev)
ret = dwc3_core_init(dwc);
if (ret) {
- dev_err(dev, "failed to initialize core\n");
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "failed to initialize core\n");
goto err4;
}
--
2.19.1
Re: Query on usb/core/devio.c
On Do, 2018-11-15 at 12:45 +, Mayuresh Kulkarni wrote: > > Understood this for remote-wake part. > > But still unclear about step (1) for host-wake as below (please note, I am > refering to host-wake and remote-wake as per my previous comment) - > Pre-condition: device in suspend and link in L2. > Use-case: end user wants to sends a control message to device. > Assumption: end user is NOT doing any activity that causes remote-wake. That is just an assumption you cannot make. Users are devious creatures. If it is physically possible, you have to be able to deal with it. Regards Oliver
[PATCH v2 2/3] usb: dwc3: drd: Switch to device property for 'extcon' handling
Switch to device property for 'extcon' handling. No functional change intended. Signed-off-by: Andy Shevchenko Acked-by: Hans de Goede --- drivers/usb/dwc3/drd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c index 218371f985ca..2401bd504891 100644 --- a/drivers/usb/dwc3/drd.c +++ b/drivers/usb/dwc3/drd.c @@ -10,6 +10,7 @@ #include #include #include +#include #include "debug.h" #include "core.h" @@ -446,8 +447,8 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc) struct device_node *np_phy, *np_conn; struct extcon_dev *edev; - if (of_property_read_bool(dev->of_node, "extcon")) - return extcon_get_edev_by_phandle(dwc->dev, 0); + if (device_property_read_bool(dev, "extcon")) + return extcon_get_edev_by_phandle(dev, 0); np_phy = of_parse_phandle(dev->of_node, "phys", 0); np_conn = of_graph_get_remote_node(np_phy, -1, -1); -- 2.19.1
[PATCH v2 3/3] usb: dwc3: drd: Add support for DR detection through extcon
Allow extcon device, found by name, to provide DR status for USB.
This is needed, for example, in case of Intel Merrifield platform,
where the Intel Basin Cove PMIC provides an extcon device to communicate
the detected role.
Note, that the "linux,extcon-name" property name is only for kernel
internal use by X86/ACPI platform code and as such is not documented
in the device tree bindings.
Signed-off-by: Andy Shevchenko
Acked-by: Hans de Goede
---
drivers/usb/dwc3/drd.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 2401bd504891..726100d1ac0d 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -446,10 +446,25 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3
*dwc)
struct device *dev = dwc->dev;
struct device_node *np_phy, *np_conn;
struct extcon_dev *edev;
+ const char *name;
if (device_property_read_bool(dev, "extcon"))
return extcon_get_edev_by_phandle(dev, 0);
+ /*
+* Device tree platforms should get extcon via phandle.
+* On ACPI platforms, we get the name from a device property.
+* This device property is for kernel internal use only and
+* is expected to be set by the glue code.
+*/
+ if (device_property_read_string(dev, "linux,extcon-name", &name) == 0) {
+ edev = extcon_get_extcon_dev(name);
+ if (!edev)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ return edev;
+ }
+
np_phy = of_parse_phandle(dev->of_node, "phys", 0);
np_conn = of_graph_get_remote_node(np_phy, -1, -1);
--
2.19.1
[PATCH v2 1/3] staging: typec: fusb302: Rename fcs,extcon-name to linux,extcon-name
Since we are going to use the same in Designware USB 3 driver,
rename the property to be consistent across the drivers.
No functional change intended.
Signed-off-by: Andy Shevchenko
Cc: Hans de Goede
Cc: Guenter Roeck
Acked-by: Hans de Goede
Acked-by: Guenter Roeck
---
drivers/platform/x86/intel_cht_int33fe.c | 2 +-
drivers/usb/typec/tcpm/fusb302.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/intel_cht_int33fe.c
b/drivers/platform/x86/intel_cht_int33fe.c
index 464fe93657b5..87cbf18d0305 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -79,7 +79,7 @@ static const struct property_entry max17047_props[] = {
};
static const struct property_entry fusb302_props[] = {
- PROPERTY_ENTRY_STRING("fcs,extcon-name", "cht_wcove_pwrsrc"),
+ PROPERTY_ENTRY_STRING("linux,extcon-name", "cht_wcove_pwrsrc"),
PROPERTY_ENTRY_U32("fcs,max-sink-microvolt", 1200),
PROPERTY_ENTRY_U32("fcs,max-sink-microamp", 300),
PROPERTY_ENTRY_U32("fcs,max-sink-microwatt", 3600),
diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 43b64d9309d0..e9344997329c 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -1765,7 +1765,7 @@ static int fusb302_probe(struct i2c_client *client,
* to be set by the platform code which also registers the i2c client
* for the fusb302.
*/
- if (device_property_read_string(dev, "fcs,extcon-name", &name) == 0) {
+ if (device_property_read_string(dev, "linux,extcon-name", &name) == 0) {
chip->extcon = extcon_get_extcon_dev(name);
if (!chip->extcon)
return -EPROBE_DEFER;
--
2.19.1
Re: Query on usb/core/devio.c
On Mon, 2018-11-12 at 15:36 +0100, Oliver Neukum wrote: > On Mo, 2018-11-12 at 12:04 +, Mayuresh Kulkarni wrote: > > > > I think I now understand the disconnect between us this point. Below is an > > attempt to bridge that, so please bear with me: > > 1. In our use-case(s), the end user can "interact" with composite USB device > > either by physically interacting with or via an app on Android. > > 2. When the interaction is "physical" (e.g.: something that is sensed by a > > sensor or a button press), we need "remote-wake" from USB device to host. > > 3. When the interaction is via app (e.g.: to toggle some control), we need > > "host-wake" from host to USB device. > All of these events can occur at the same time. > And you cannot prevent that. > > > > > And this is where, the knowledge of wake-up source and difference in > > handling > > comes into picture. > > For (2), the host needs to first wake-up and then queue a request to read- > > out > > "what" happened. > > For (3), the host needs to first wake-up the device and then queue the > > command(s) to the take action asked by the end user. > > > > I agree that in both of above cases, "a USB request" needs to be send from > > user- > > space to kernel to device. But in my opinion, each of the them has a > > different > > context associated with it. E.g.: for (2), a single read request is > > sufficient > > to "know" what happened while for (3), a write and a read request is needed. > There is a natural race condition between remote and host wake up. > You cannot rule out that your app wakes the device up, exactly when > user interaction would have woken it anyway. > > Your app will know what it wants to do with the device. And you can > detect that. > Hence the correct algorithm is: > > 1. wake up the device > 2. query it for a cause of wake up > 3. If the host wants to do IO, do all of it. > Understood this for remote-wake part. But still unclear about step (1) for host-wake as below (please note, I am refering to host-wake and remote-wake as per my previous comment) - Pre-condition: device in suspend and link in L2. Use-case: end user wants to sends a control message to device. Assumption: end user is NOT doing any activity that causes remote-wake. So how would (1) happen i.e.: which ioctls the user-space should execute? Could you please explain a bit more? > Regards > Oliver
Re: [PATCH] Improve the accuracy of the baud rate generator by using round-to-closest instead of truncating when calculating baud rate divisors.
On Thu, Nov 15, 2018 at 09:24:49AM +0100, Johan Hovold wrote: > On Tue, Nov 13, 2018 at 08:19:44PM +0100, Nikolaj Fogh wrote: > > On 11/12/18 10:54 AM, Johan Hovold wrote: > > > On Wed, Oct 31, 2018 at 09:16:48PM +0100, Nikolaj Fogh wrote: > > >> I have experienced that the ftdi_sio driver gives less-than-optimal > > >> baud rates as the driver truncates instead of rounds to nearest > > >> during baud rate divisor calculation. > > > >> This patch improves on the baud rate generation. The generated baud > > >> rate corresponds to the optimal baud rate achievable with the chip. > > >> This is what the windows driver gives as well. > > > > > > How did you verify this? Did you trace and compare the divisors > > > actually requested by the Windows driver, or did you measure the > > > resulting rates using a scope? > > > I verified it by scope. Granted, I only verified it for one baud rate > > (961200). Whether it gives the same as the Windows driver in general, > > I'm not sure. However, I would think that rounding instead of flooring > > would always yield the most accurate result. > > I'm not so sure in this case. The driver uses "sub-integer" divisors and > looks like it depends on truncation rather than rounding. Some > background here: > > > https://www.ftdichip.com/Support/Knowledgebase/index.html?whatbaudratesareachieveabl.htm After taking a closer look at the driver, I think your patch may be fine. It shouldn't change 921600 baud, though, but I see now that you wrote *961200* above. Was that intentional? > If you want to change these calculations you need to make a stronger > case for it and verify that we don't mess up some other rate > inadvertently. This still stands though, you need a better description of why this is needed and correct. Mention which device and rate that was off, and by how much. Determining the theoretical difference may be interesting too, while making sure we don't introduce larger errors for other rates. Thanks, Johan
[PATCH 1/1] xhci: Prevent bus suspend if a port connect change or polling state is detected
USB3 roothub might autosuspend before a plugged USB3 device is detected,
causing USB3 device enumeration failure.
USB3 devices don't show up as connected and enabled until USB3 link trainig
completes. On a fast booting platform with a slow USB3 link training the
link might reach the connected enabled state just as the bus is suspending.
If this device is discovered first time by the xhci_bus_suspend() routine
it will be put to U3 suspended state like the other ports which failed to
suspend earlier.
The hub thread will notice the connect change and resume the bus,
moving the port back to U0
This U0 -> U3 -> U0 transition right after being connected seems to be
too much for some devices, causing them to first go to SS.Inactive state,
and finally end up stuck in a polling state with reset asserted
Fix this by failing the bus suspend if a port has a connect change or is
in a polling state in xhci_bus_suspend().
Don't do any port changes until all ports are checked, buffer all port
changes and only write them in the end if suspend can proceed
Cc: sta...@vger.kernel.org
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-hub.c | 60 ++---
1 file changed, 46 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index da98a11..94aca1b 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1474,15 +1474,18 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
unsigned long flags;
struct xhci_hub *rhub;
struct xhci_port **ports;
+ u32 portsc_buf[USB_MAXCHILDREN];
+ bool wake_enabled;
rhub = xhci_get_rhub(hcd);
ports = rhub->ports;
max_ports = rhub->num_ports;
bus_state = &xhci->bus_state[hcd_index(hcd)];
+ wake_enabled = hcd->self.root_hub->do_remote_wakeup;
spin_lock_irqsave(&xhci->lock, flags);
- if (hcd->self.root_hub->do_remote_wakeup) {
+ if (wake_enabled) {
if (bus_state->resuming_ports ||/* USB2 */
bus_state->port_remote_wakeup) {/* USB3 */
spin_unlock_irqrestore(&xhci->lock, flags);
@@ -1490,26 +1493,36 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
return -EBUSY;
}
}
-
- port_index = max_ports;
+ /*
+* Prepare ports for suspend, but don't write anything before all ports
+* are checked and we know bus suspend can proceed
+*/
bus_state->bus_suspended = 0;
+ port_index = max_ports;
while (port_index--) {
- /* suspend the port if the port is not suspended */
u32 t1, t2;
- int slot_id;
t1 = readl(ports[port_index]->addr);
t2 = xhci_port_state_to_neutral(t1);
+ portsc_buf[port_index] = 0;
- if ((t1 & PORT_PE) && !(t1 & PORT_PLS_MASK)) {
- xhci_dbg(xhci, "port %d not suspended\n", port_index);
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- port_index + 1);
- if (slot_id) {
+ /* Bail out if a USB3 port has a new device in link training */
+ if ((t1 & PORT_PLS_MASK) == XDEV_POLLING) {
+ bus_state->bus_suspended = 0;
+ spin_unlock_irqrestore(&xhci->lock, flags);
+ xhci_dbg(xhci, "Bus suspend bailout, port in
polling\n");
+ return -EBUSY;
+ }
+
+ /* suspend ports in U0, or bail out for new connect changes */
+ if ((t1 & PORT_PE) && (t1 & PORT_PLS_MASK) == XDEV_U0) {
+ if ((t1 & PORT_CSC) && wake_enabled) {
+ bus_state->bus_suspended = 0;
spin_unlock_irqrestore(&xhci->lock, flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(&xhci->lock, flags);
+ xhci_dbg(xhci, "Bus suspend bailout, port
connect change\n");
+ return -EBUSY;
}
+ xhci_dbg(xhci, "port %d not suspended\n", port_index);
t2 &= ~PORT_PLS_MASK;
t2 |= PORT_LINK_STROBE | XDEV_U3;
set_bit(port_index, &bus_state->bus_suspended);
@@ -1518,7 +1531,7 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
* including the USB 3.0 roothub, but only if CONFIG_PM
* is enabled, so also enable remote wake here.
*/
- if (hcd->self.root_hub->do_remote_wakeup) {
+ if (wake_enabled) {
if (t1 & PORT_CONNECT) {
t2 |= PORT_WKOC_E | PORT_WKDISC_E;
[PATCH 0/1] xhci fix for usb-linus
Hi Greg A case was found where USB3 devices fail to enumerate at boot. If a USB3 link is trained slowly then the roothub might autosuspend while device is just getting enabled. This patch fixes the issue -Mathias Mathias Nyman (1): xhci: Prevent bus suspend if a port connect change or polling state is detected drivers/usb/host/xhci-hub.c | 60 ++--- 1 file changed, 46 insertions(+), 14 deletions(-) -- 2.7.4
Re: [balbi-usb:testing/next 17/22] drivers/usb/dwc3/drd.c:604: undefined reference to `usb_role_switch_unregister'
Hi, Heikki Krogerus writes: > On Thu, Nov 15, 2018 at 09:46:26AM +0200, Felipe Balbi wrote: >> >> Hi Heikki, >> >> kbuild test robot writes: >> >> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git >> > testing/next >> > head: bad6c1502dac79c80ad5a7149fa308849c0191dd >> > commit: 24e2238d8c102f242ece57f45fbeb4014929aad4 [17/22] usb: dwc3: drd: >> > Register a USB role switch >> > config: x86_64-randconfig-s4-11151335 (attached as .config) >> > compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 >> > reproduce: >> > git checkout 24e2238d8c102f242ece57f45fbeb4014929aad4 >> > # save the attached .config to linux build tree >> > make ARCH=x86_64 >> > >> > All errors (new ones prefixed by >>): >> > >> >drivers/usb/dwc3/drd.o: In function `dwc3_drd_exit': >> >>> drivers/usb/dwc3/drd.c:604: undefined reference to >> >>> `usb_role_switch_unregister' >> >drivers/usb/dwc3/drd.o: In function `dwc3_drd_init': >> >>> drivers/usb/dwc3/drd.c:563: undefined reference to >> >>> `usb_role_switch_register' >> >>> drivers/usb/dwc3/drd.c:563: undefined reference to >> >>> `usb_role_switch_register' >> >> care to fix this? > > Have you picked the RFC patch that registers the drd switch in > drivers/usb/dwc3/drd.c? I have not even tried to compile that code :-) > > I really did not want that patch to be picked by you. I was just > presenting the idea to Chen Yu. It was probable wrong of me to send it > as a patch, even though I did use the magic "RFC" letters. Sorry for > that. don't worry, I missed the RFC :-p > Is it possible for you to drop the patch, or do you prefer that we > just fix it? I don't think reverting the patch is a good idea. If > dropping the patch at this stage is not possible anymore, then I think > it's probable better to just fix the situation. Yeah, I'll drop it, no worries. -- balbi
Re: [balbi-usb:testing/next 17/22] drivers/usb/dwc3/drd.c:604: undefined reference to `usb_role_switch_unregister'
On Thu, Nov 15, 2018 at 09:46:26AM +0200, Felipe Balbi wrote: > > Hi Heikki, > > kbuild test robot writes: > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git > > testing/next > > head: bad6c1502dac79c80ad5a7149fa308849c0191dd > > commit: 24e2238d8c102f242ece57f45fbeb4014929aad4 [17/22] usb: dwc3: drd: > > Register a USB role switch > > config: x86_64-randconfig-s4-11151335 (attached as .config) > > compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 > > reproduce: > > git checkout 24e2238d8c102f242ece57f45fbeb4014929aad4 > > # save the attached .config to linux build tree > > make ARCH=x86_64 > > > > All errors (new ones prefixed by >>): > > > >drivers/usb/dwc3/drd.o: In function `dwc3_drd_exit': > >>> drivers/usb/dwc3/drd.c:604: undefined reference to > >>> `usb_role_switch_unregister' > >drivers/usb/dwc3/drd.o: In function `dwc3_drd_init': > >>> drivers/usb/dwc3/drd.c:563: undefined reference to > >>> `usb_role_switch_register' > >>> drivers/usb/dwc3/drd.c:563: undefined reference to > >>> `usb_role_switch_register' > > care to fix this? Have you picked the RFC patch that registers the drd switch in drivers/usb/dwc3/drd.c? I have not even tried to compile that code :-) I really did not want that patch to be picked by you. I was just presenting the idea to Chen Yu. It was probable wrong of me to send it as a patch, even though I did use the magic "RFC" letters. Sorry for that. Is it possible for you to drop the patch, or do you prefer that we just fix it? I don't think reverting the patch is a good idea. If dropping the patch at this stage is not possible anymore, then I think it's probable better to just fix the situation. thanks, -- heikki
Re: [PATCH] Improve the accuracy of the baud rate generator by using round-to-closest instead of truncating when calculating baud rate divisors.
On Tue, Nov 13, 2018 at 08:19:44PM +0100, Nikolaj Fogh wrote: > On 11/12/18 10:54 AM, Johan Hovold wrote: > > On Wed, Oct 31, 2018 at 09:16:48PM +0100, Nikolaj Fogh wrote: > >> I have experienced that the ftdi_sio driver gives less-than-optimal > >> baud rates as the driver truncates instead of rounds to nearest > >> during baud rate divisor calculation. > >> This patch improves on the baud rate generation. The generated baud > >> rate corresponds to the optimal baud rate achievable with the chip. > >> This is what the windows driver gives as well. > > > > How did you verify this? Did you trace and compare the divisors > > actually requested by the Windows driver, or did you measure the > > resulting rates using a scope? > I verified it by scope. Granted, I only verified it for one baud rate > (961200). Whether it gives the same as the Windows driver in general, > I'm not sure. However, I would think that rounding instead of flooring > would always yield the most accurate result. I'm not so sure in this case. The driver uses "sub-integer" divisors and looks like it depends on truncation rather than rounding. Some background here: https://www.ftdichip.com/Support/Knowledgebase/index.html?whatbaudratesareachieveabl.htm If you want to change these calculations you need to make a stronger case for it and verify that we don't mess up some other rate inadvertently. Thanks, Johan
Re: [PATCH v2 RESEND] usb: dwc3: core: Fix ULPI PHYs and prevent phy_get/ulpi_init during suspend/resume
Hi, Todor Tomov writes: > Hello, > > After I apply this patch on 4.14 (or receive it with 4.14.70) I see a > regression with > the Qualcomm QUSB2 phy driver. I'm testing on a Dragonboard 820c. > In boot log I get: > > [4.525502] phy phy-7412000.phy.6: QUSB2PHY pll lock failed: status reg = 0 > [4.529232] phy phy-7412000.phy.6: phy init failed --> -16 > [4.536170] dwc3 760.dwc3: failed to initialize core > [4.541743] dwc3: probe of 760.dwc3 failed with error -16 > [4.549979] phy phy-7411000.phy.5: QUSB2PHY pll lock failed: status reg = 0 > [4.552843] phy phy-7411000.phy.5: phy init failed --> -16 > [4.559606] dwc3 6a0.dwc3: failed to initialize core > [4.565181] dwc3: probe of 6a0.dwc3 failed with error -16 > > I can provide a full log if needed. please do. Also, try mainline and let us know if the problem persists. Why do you get -EBUSY from the PHY driver? - balbi
