Re: [PATCH] usb: dwc3: Prevent indefinite sleep in _dwc3_set_mode during suspend/resume
Hi,
Roger Quadros writes:
> In the following test we get stuck by sleeping forever in _dwc3_set_mode()
> after which dual-role switching doesn't work.
>
> On dra7-evm's dual-role port,
> - Load g_zero gadget driver and enumerate to host
> - suspend to mem
> - disconnect USB cable to host and connect otg cable with Pen drive in it.
> - resume system
> - we sleep indefinitely in _dwc3_set_mode due to.
> dwc3_gadget_exit()->usb_del_gadget_udc()->udc_stop()->
> dwc3_gadget_stop()->wait_event_lock_irq()
>
> Let's clear the DWC3_EP_END_TRANSFER_PENDING flag on all endpoints
> so we don't wait in dwc3_gadget_stop().
>
> Signed-off-by: Roger Quadros
> ---
> drivers/usb/dwc3/gadget.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 2bda4eb..0a360da 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3273,6 +3273,20 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>
> void dwc3_gadget_exit(struct dwc3 *dwc)
> {
> + int epnum;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dwc->lock, flags);
> + for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
> + struct dwc3_ep *dep = dwc->eps[epnum];
> +
> + if (!dep)
> + continue;
> +
> + dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
> + }
> + spin_unlock_irqrestore(&dwc->lock, flags);
> +
> usb_del_gadget_udc(&dwc->gadget);
> dwc3_gadget_free_endpoints(dwc);
free endpoints is a better place for this. It's already going to free
the memory anyway. Might as well clear all flags to 0 there.
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH v4 4/6] arm64: dts: exynos: add OF graph between MHL and USB connector
On 27.02.2018 22:24, Rob Herring wrote:
> On Wed, Feb 21, 2018 at 2:55 AM, Andrzej Hajda wrote:
>> OF graph describes MHL data lanes between MHL and respective USB
>> connector.
>>
>> Signed-off-by: Andrzej Hajda
>> ---
>> v4:
>> - added missing reg property in connector's port node (Krzysztof)
>> ---
>> .../boot/dts/exynos/exynos5433-tm2-common.dtsi | 32
>> --
>> 1 file changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> index f604f6b1a9c2..2ed506df94d0 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> @@ -817,9 +817,22 @@
>> clocks = <&pmu_system_controller 0>;
>> clock-names = "xtal";
>>
>> - port {
>> - mhl_to_hdmi: endpoint {
>> - remote-endpoint = <&hdmi_to_mhl>;
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> + mhl_to_hdmi: endpoint {
>> + remote-endpoint = <&hdmi_to_mhl>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> + mhl_to_musb_con: endpoint {
>> + remote-endpoint = <&musb_con_to_mhl>;
>> + };
> These ports are mutually exclusive, right? If so, it should be 1 port
> with 2 endpoints. Ports should represent independent data flows.
> Something muxed or replicated (1 to many connection) should be be
> endpoints.
No, this is HDMI -> MHL bridge, so port 0 is HDMI input, and port 1 is
MHL output.
Regards
Andrzej
>
> Rob
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
回覆: [PATCH] staging: typec: handle vendor defined part and modify drp toggling flow
Hi Jun,
For the questions of drp_toggling, our test is as following:
According to TCPCI 4.4.5.2
It is recommended the TCPM write ROLE_CONTROL.DRP=0 before writing to
POWER_CONTROL.AutoDischargeDisconnect and starting the DRP toggling using
COMMAND.Look4Connection.
We've encounter a situation while testing with RT1711H as following:
We repeatedly plug in/out a device (with Rd), and not to provide VBUS(5V)
while plugging in.
If we plug out the device right after TCPC detects it and stops toggling,
TCPM will try to restart drp_toggling.
Here, we re-plug in the device before TCPM calls drp_toggling. Under this
circumstance, RT1711H reports open/open after drp_toggling is called.
For current TCPM, it stops if open/open is reported at drp_toggling state.
The detailed flow that causes RT1711H reporting open/open is described as
following:
1. RT1711H detects the device, stops toggling and reports to TCPM.
2. Rd is plugged out before set_cc is called. So, ROLE_CONTROL.DRP is still 1.
3. Plug in the device before TCPM restarts drp_toggling
4. The device is detected by RT1711H's internal firmware again(TCPC chip's
internal firmware).
5. TCPM calls drp_toggling before cc_change triggered by step 4 is handled.
6. TCPM sets ROLE_CONTROL.DRP = 1, Rd/Rd and start drp_toggling.
According to TCPCI 4.4.5.2
The TCPM shall write B6 (DRP) = 0b and B3..0 (CC1/CC2) if it wishes to control
the Rp/Rd
directly instead of having the TCPC perform DRP toggling autonomousl.
So, Rd/Rd set in step 6 will not work. (Because ROLE_CONTROL.DRP is 1 since the
first step.)
7. RT1711H will stop toggling immediately (Because it's internal firmware
already detects device at step 4) and report open/open (Because CC1/CC2 will be
changed to Rd by RT1711H after LOOK4CONNECTION is set. RT1711H sets to Rd and
device is Rd so open is reported)
8. TCPCI reports open/open to TCPM at drp_toggling state
That's why we always set ROLE_CONTROL.DRP to 0 while setting Rd/Rd.
If this circumstance is not a general case, we can also use a vendor ops to
replace it.
I don't catch the point of how you handle private events by above change,
maybe post your RT1711H part as a user in one series can make it clear,
I assume this can be done in existing tcpci_irq like above vender specific
handling as well:
For every glue driver, it registers its own irq handler and calls tcpci_irq in
the handler.
Take RT1711H as an example:
It registers its own irq handler in probe function and handle vendor defined
interrupts before calling general tcpci_irq:
static irqreturn_t _tcpci_irq(int irq, void *dev_id)
{
struct rt1711h_chip *chip = dev_id;
/* handle vendor defined interrupts here */
/* call general tcpci_irq */
return tcpci_irq(chip->tcpci);
}
static int rt1711h_probe(struct i2c_client *client, const struct i2c_device_id
*i2c_id)
{
ret = devm_request_threaded_irq(chip->dev, client->irq, NULL,
_tcpci_irq,
IRQF_ONESHOT | IRQF_TRIGGER_LOW,
dev_name(&client->dev), chip);
}
Best Regards,
*
Shu-Fan Lee
Richtek Technology Corporation
TEL: +886-3-5526789 #2359
FAX: +886-3-5526612
*
寄件者: Jun Li
寄件日期: 2018年2月22日 下午 06:16
收件者: ShuFanLee; heikki.kroge...@linux.intel.com; li...@roeck-us.net;
g...@kroah.com
副本: shufan_lee(李書帆); cy_huang(黃啟原); linux-ker...@vger.kernel.org;
linux-usb@vger.kernel.org; dl-linux-imx
主旨: RE: [PATCH] staging: typec: handle vendor defined part and modify drp
toggling flow
Hi,
> -Original Message-
> From: linux-usb-ow...@vger.kernel.org
> [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of ShuFanLee
> Sent: 2018年2月21日 23:02
> To: heikki.kroge...@linux.intel.com; li...@roeck-us.net; g...@kroah.com
> Cc: shufan_...@richtek.com; cy_hu...@richtek.com;
> linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org
> Subject: [PATCH] staging: typec: handle vendor defined part and modify drp
> toggling flow
>
> From: ShuFanLee
>
> Handle vendor defined behavior in tcpci_init, tcpci_set_vconn and export
> tcpci_irq.
> More operations can be extended in tcpci_data if needed.
> According to TCPCI specification, 4.4.5.2 ROLE_CONTROL, TCPC shall not
> start DRP toggling until subsequently the TCPM writes to the COMMAND
> register to start DRP toggling.
My understanding of above statement is TCPM(this Linux driver) can enable
DRP and CC1/CC2 in one shot, but TCPC(your typec chip internal firmware)
needs wait until TCPM writes to COMMAND register to start drp toggling.
> DRP toggling flow is chagned as following:
> - Write DRP = 0 & Rd/Rd
Why fixed to be Rd/Rd? in this case, it means the starting value:
Tcpci 4.4.5.2:
"When initiating a
Re: [PATCH] usb: dwc3: Prevent indefinite sleep in _dwc3_set_mode during suspend/resume
Hi Roger,
On 27 February 2018 at 19:22, Roger Quadros wrote:
> In the following test we get stuck by sleeping forever in _dwc3_set_mode()
> after which dual-role switching doesn't work.
>
> On dra7-evm's dual-role port,
> - Load g_zero gadget driver and enumerate to host
> - suspend to mem
> - disconnect USB cable to host and connect otg cable with Pen drive in it.
> - resume system
> - we sleep indefinitely in _dwc3_set_mode due to.
> dwc3_gadget_exit()->usb_del_gadget_udc()->udc_stop()->
> dwc3_gadget_stop()->wait_event_lock_irq()
>
> Let's clear the DWC3_EP_END_TRANSFER_PENDING flag on all endpoints
> so we don't wait in dwc3_gadget_stop().
I am curious why the DWC3_DEPEVT_EPCMDCMPLT event was not triggered
any more when you executed the DWC3_DEPCMD_ENDTRANSFER command?
>
> Signed-off-by: Roger Quadros
> ---
> drivers/usb/dwc3/gadget.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 2bda4eb..0a360da 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3273,6 +3273,20 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>
> void dwc3_gadget_exit(struct dwc3 *dwc)
> {
> + int epnum;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dwc->lock, flags);
> + for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
> + struct dwc3_ep *dep = dwc->eps[epnum];
> +
> + if (!dep)
> + continue;
> +
> + dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
> + }
> + spin_unlock_irqrestore(&dwc->lock, flags);
> +
> usb_del_gadget_udc(&dwc->gadget);
> dwc3_gadget_free_endpoints(dwc);
> dma_free_coherent(dwc->sysdev, DWC3_BOUNCE_SIZE, dwc->bounce,
> --
--
Baolin.wang
Best Regards
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
回覆: [PATCH] staging: typec: handle vendor defined part and modify drp toggling flow
Hi Guenter,
regmap_write returns a negative return code or 0, thus this can be
simplified to
return regmap_write(...);
Ok, I'll modify it in v4
Hmm, normally I'd expect this function _after_ the function it calls.
Guess that doesn't matter much here, so I am fine with it as long
as Greg is ok with it as well.
Do you mean it maybe better to call vendor's set_vconn after normal set_vconn
is finished?
If I understand correctly, I can also modify it in v4. For RT1711H, it also
works by enabling/disabling idle mode after set_vconn.
Best Regards,
*
Shu-Fan Lee
Richtek Technology Corporation
TEL: +886-3-5526789 #2359
FAX: +886-3-5526612
*
寄件者: Guenter Roeck 代表 Guenter Roeck
寄件日期: 2018年2月22日 上午 06:15
收件者: ShuFanLee
副本: heikki.kroge...@linux.intel.com; g...@kroah.com; shufan_lee(李書帆);
cy_huang(黃啟原); linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org
主旨: Re: [PATCH] staging: typec: handle vendor defined part and modify drp
toggling flow
On Wed, Feb 21, 2018 at 11:02:23PM +0800, ShuFanLee wrote:
> From: ShuFanLee
>
> Handle vendor defined behavior in tcpci_init, tcpci_set_vconn and export
> tcpci_irq.
> More operations can be extended in tcpci_data if needed.
> According to TCPCI specification, 4.4.5.2 ROLE_CONTROL,
> TCPC shall not start DRP toggling until subsequently the TCPM
> writes to the COMMAND register to start DRP toggling.
> DRP toggling flow is chagned as following:
> - Write DRP = 0 & Rd/Rd
> - Write DRP = 1
> - Set LOOK4CONNECTION command
>
> Signed-off-by: ShuFanLee
Mostly loooks good to me. Couple of nitpicks below.
Guenter
> ---
> drivers/staging/typec/tcpci.c | 128
> +-
> drivers/staging/typec/tcpci.h | 13 +
> 2 files changed, 115 insertions(+), 26 deletions(-)
>
> patch changelogs between v1 & v2
> - Remove unnecessary i2c_client in the structure of tcpci
> - Rename structure of tcpci_vendor_data to tcpci_data
> - Not exporting tcpci read/write wrappers but register i2c regmap in glue
> driver
> - Add set_vconn ops in tcpci_data
>(It is necessary for RT1711H to enable/disable idle mode before
> disabling/enabling vconn)
> - Export tcpci_irq so that vendor can call it in their own IRQ handler
>
> patch changelogs between v2 & v3
> - Change the return type of tcpci_irq from int to irqreturn_t
>
> diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
> index 9bd4412..4959c69 100644
> --- a/drivers/staging/typec/tcpci.c
> +++ b/drivers/staging/typec/tcpci.c
> @@ -21,7 +21,6 @@
>
> struct tcpci {
> struct device *dev;
> - struct i2c_client *client;
>
> struct tcpm_port *port;
>
> @@ -30,6 +29,12 @@ struct tcpci {
> bool controls_vbus;
>
> struct tcpc_dev tcpc;
> + struct tcpci_data *data;
> +};
> +
> +struct tcpci_chip {
> + struct tcpci *tcpci;
> + struct tcpci_data data;
> };
>
> static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc)
> @@ -37,8 +42,7 @@ static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev
> *tcpc)
> return container_of(tcpc, struct tcpci, tcpc);
> }
>
> -static int tcpci_read16(struct tcpci *tcpci, unsigned int reg,
> - u16 *val)
> +static int tcpci_read16(struct tcpci *tcpci, unsigned int reg, u16 *val)
> {
> return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u16));
> }
> @@ -98,8 +102,10 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum
> typec_cc_status cc)
> static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
> enum typec_cc_status cc)
> {
> + int ret;
> struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> - unsigned int reg = TCPC_ROLE_CTRL_DRP;
> + unsigned int reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
> +(TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
>
> switch (cc) {
> default:
> @@ -117,7 +123,19 @@ static int tcpci_start_drp_toggling(struct tcpc_dev
> *tcpc,
> break;
> }
>
> - return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> + if (ret < 0)
> + return ret;
> + usleep_range(500, 1000);
> + reg |= TCPC_ROLE_CTRL_DRP;
> + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> + if (ret < 0)
> + return ret;
> + ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
> +TCPC_CMD_LOOK4CONNECTION);
> + if (ret < 0)
> + return ret;
> + return 0;
regmap_write returns a negative return code or 0, thus this can be
simplified to
return regmap_write(...);
> }
>
> static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink)
> @@ -178,6 +196,16 @@ static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool
> enable)
> struct
Re: [PATCH] usbip: tools usbip_attach: Fix cryptic error messages
On 02/27/2018 03:45 PM, Krzysztof Opasiak wrote:
>
>
> On 02/27/2018 11:23 PM, Shuah Khan wrote:
>> Attach device error message is cryptic and useless. Fix it to be
>> informative.
>>
>> Signed-off-by: Shuah Khan
>> ---
>> tools/usb/usbip/src/usbip_attach.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/usb/usbip/src/usbip_attach.c
>> b/tools/usb/usbip/src/usbip_attach.c
>> index 7f07b2d50f59..f60738735398 100644
>> --- a/tools/usb/usbip/src/usbip_attach.c
>> +++ b/tools/usb/usbip/src/usbip_attach.c
>> @@ -195,7 +195,8 @@ static int attach_device(char *host, char *busid)
>> rhport = query_import_device(sockfd, busid);
>> if (rhport < 0) {
>> - err("query");
>> + err("Attach request for Device %s. Is this device exported?",
>> + busid);
>> return -1;
>> }
>
> The code itself is ok and you may put my:
>
> Reviewed-by: Krzysztof Opasiak
>
> but just because of my curiosity, there is a number of places in usbip tools
> where the same level of descriptiveness is used for error message. Why did
> you choose to fix this one not any other or all others?;)
>
> Best regards,
Yes there are several very cryptic and useless error message all over the
place in usbip tools that could use refinement. This just happens to be the
one that I seem to run into very frequently. It frustrated me enough that
I decided to fix this one first. :)
thanks,
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usbip: tools usbip_attach: Fix cryptic error messages
On 02/27/2018 11:23 PM, Shuah Khan wrote:
Attach device error message is cryptic and useless. Fix it to be
informative.
Signed-off-by: Shuah Khan
---
tools/usb/usbip/src/usbip_attach.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/usb/usbip/src/usbip_attach.c
b/tools/usb/usbip/src/usbip_attach.c
index 7f07b2d50f59..f60738735398 100644
--- a/tools/usb/usbip/src/usbip_attach.c
+++ b/tools/usb/usbip/src/usbip_attach.c
@@ -195,7 +195,8 @@ static int attach_device(char *host, char *busid)
rhport = query_import_device(sockfd, busid);
if (rhport < 0) {
- err("query");
+ err("Attach request for Device %s. Is this device exported?",
+ busid);
return -1;
}
The code itself is ok and you may put my:
Reviewed-by: Krzysztof Opasiak
but just because of my curiosity, there is a number of places in usbip
tools where the same level of descriptiveness is used for error message.
Why did you choose to fix this one not any other or all others?;)
Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/1] usb: musb: call pm_runtime_{get,put}_sync before reading vbus registers
Without pm_runtime_{get,put}_sync calls in place, reading
vbus status via /sys causes the following error:
Unhandled fault: external abort on non-linefetch (0x1028) at 0xfa0ab060
pgd = b333e822
[fa0ab060] *pgd=48011452(bad)
[] (musb_default_readb) from [] (musb_vbus_show+0x58/0xe4)
[] (musb_vbus_show) from [] (dev_attr_show+0x20/0x44)
[] (dev_attr_show) from [] (sysfs_kf_seq_show+0x80/0xdc)
[] (sysfs_kf_seq_show) from [] (seq_read+0x250/0x448)
[] (seq_read) from [] (__vfs_read+0x1c/0x118)
[] (__vfs_read) from [] (vfs_read+0x90/0x144)
[] (vfs_read) from [] (SyS_read+0x3c/0x74)
[] (SyS_read) from [] (ret_fast_syscall+0x0/0x54)
Solution was suggested by Tony Lindgren .
Signed-off-by: Merlijn Wajer
---
drivers/usb/musb/musb_core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index eef4ad578b31..c344ef4e5355 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1756,6 +1756,7 @@ vbus_show(struct device *dev, struct device_attribute
*attr, char *buf)
int vbus;
u8 devctl;
+ pm_runtime_get_sync(dev);
spin_lock_irqsave(&musb->lock, flags);
val = musb->a_wait_bcon;
vbus = musb_platform_get_vbus_status(musb);
@@ -1769,6 +1770,7 @@ vbus_show(struct device *dev, struct device_attribute
*attr, char *buf)
vbus = 0;
}
spin_unlock_irqrestore(&musb->lock, flags);
+ pm_runtime_put_sync(dev);
return sprintf(buf, "Vbus %s, timeout %lu msec\n",
vbus ? "on" : "off", val);
--
2.16.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/1] usb: musb: fix vbus_show
Hi,
This fixes reading vbus status in musb.
v2 places the pm_runtime_{get,put}_sync calls outside of the spinlocks, because
pm_runtime_{get,put}_sync would otherwise (sometimes, depending on the platform)
cause deadlocks.
Hopefully this fixes the deadlock issue that Bin Liu ran into. I've tested this
patch on my Nokia N900 and it works for me.
Cheers,
Merlijn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL
Hi,
On 2018년 02월 27일 21:05, Andrzej Hajda wrote:
> On 27.02.2018 12:08, Chanwoo Choi wrote:
>> Hi,
>>
>> On 2018년 02월 27일 16:11, Andrzej Hajda wrote:
>>> From: Maciej Purski
>>>
>>> Currently MHL chip must be turned on permanently to detect MHL cable. It
>>> duplicates micro-USB controller's (MUIC) functionality and consumes
>>> unnecessary power. Lets use extcon attached to MUIC to enable MHL chip
>>> only if it detects MHL cable.
>>>
>>> Signed-off-by: Maciej Purski
>>> Signed-off-by: Andrzej Hajda
>>> ---
>>> v5: updated extcon API
>>>
>>> This is rework of the patch by Maciej with following changes:
>>> - use micro-USB port bindings to get extcon, instead of extcon property,
>>> - fixed remove sequence,
>>> - fixed extcon get state logic.
>>>
>>> Code finding extcon node is hacky IMO, I guess ultimately it should be done
>>> via some framework (maybe even extcon), or at least via helper, I hope it
>>> can stay as is until the proper solution will be merged.
>>>
>>> Signed-off-by: Andrzej Hajda
>>> ---
>>> drivers/gpu/drm/bridge/sil-sii8620.c | 97
>>> ++--
>>> 1 file changed, 94 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
>>> b/drivers/gpu/drm/bridge/sil-sii8620.c
>>> index 9e785b8e0ea2..62b0adabcac2 100644
>>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>>> @@ -17,6 +17,7 @@
>>>
>>> #include
>>> #include
>>> +#include
>>> #include
>>> #include
>>> #include
>>> @@ -25,6 +26,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> #include
>>> #include
>>>
>>> @@ -81,6 +83,10 @@ struct sii8620 {
>>> struct edid *edid;
>>> unsigned int gen2_write_burst:1;
>>> enum sii8620_mt_state mt_state;
>>> + struct extcon_dev *extcon;
>>> + struct notifier_block extcon_nb;
>>> + struct work_struct extcon_wq;
>>> + int cable_state;
>>> struct list_head mt_queue;
>>> struct {
>>> int r_size;
>>> @@ -2175,6 +2181,77 @@ static void sii8620_init_rcp_input_dev(struct
>>> sii8620 *ctx)
>>> ctx->rc_dev = rc_dev;
>>> }
>>>
>>> +static void sii8620_cable_out(struct sii8620 *ctx)
>>> +{
>>> + disable_irq(to_i2c_client(ctx->dev)->irq);
>>> + sii8620_hw_off(ctx);
>>> +}
>>> +
>>> +static void sii8620_extcon_work(struct work_struct *work)
>>> +{
>>> + struct sii8620 *ctx =
>>> + container_of(work, struct sii8620, extcon_wq);
>>> + int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
>>> +
>>> + if (state == ctx->cable_state)
>>> + return;
>>> +
>>> + ctx->cable_state = state;
>>> +
>>> + if (state > 0)
>>> + sii8620_cable_in(ctx);
>>> + else
>>> + sii8620_cable_out(ctx);
>>> +}
>>> +
>>> +static int sii8620_extcon_notifier(struct notifier_block *self,
>>> + unsigned long event, void *ptr)
>>> +{
>>> + struct sii8620 *ctx =
>>> + container_of(self, struct sii8620, extcon_nb);
>>> +
>>> + schedule_work(&ctx->extcon_wq);
>>> +
>>> + return NOTIFY_DONE;
>>> +}
>>> +
>>> +static int sii8620_extcon_init(struct sii8620 *ctx)
>>> +{
>>> + struct extcon_dev *edev;
>>> + struct device_node *musb, *muic;
>>> + int ret;
>>> +
>>> + /* get micro-USB connector node */
>>> + musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
>>> + /* next get micro-USB Interface Controller node */
>>> + muic = of_get_next_parent(musb);
>>> +
>>> + if (!muic) {
>>> + dev_info(ctx->dev, "no extcon found, switching to 'always on'
>>> mode\n");
>>> + return 0;
>>> + }
>>> +
>>> + edev = extcon_find_edev_by_node(muic);
>>> + of_node_put(muic);
>>> + if (IS_ERR(edev)) {
>>> + if (PTR_ERR(edev) == -EPROBE_DEFER)
>>> + return -EPROBE_DEFER;
>>> + dev_err(ctx->dev, "Invalid or missing extcon\n");
>>> + return PTR_ERR(edev);
>>> + }
>>> +
>>> + ctx->extcon = edev;
>>> + ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
>>> + INIT_WORK(&ctx->extcon_wq, sii8620_extcon_work);
>>> + ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
>> You better to use devm_extcon_register_notifier().
>
> With devm version I risk that in case of device unbind notification will
> be called after .remove callback, it seems to me quite dangerous. Or am
> I missing something?
If you use the cancel_work_sync() in remove() instead of flush_work(),
you can use the 'devm_extcon_*'.
>
> Regards
> Andrzej
>
>>
>>> + if (ret) {
>>> + dev_err(ctx->dev, "failed to register notifier for MHL\n");
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge)
>>> {
>>> return container_of(bridge, struct sii8620, bridge);
>>> @@ -2307,13 +2384,20 @@ static int sii8620_probe(struct i2c_client *client,
>>> if (ret)
>>> return ret;
>>>
>>> + r
[PATCH] usbip: tools usbip_network: Fix cryptic error messages
Kernel and tool version mismatch message is cryptic. Fix it to be
informative.
Signed-off-by: Shuah Khan
---
tools/usb/usbip/src/usbip_network.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/usb/usbip/src/usbip_network.c
b/tools/usb/usbip/src/usbip_network.c
index b4c37e76a6e0..90325fa8bc38 100644
--- a/tools/usb/usbip/src/usbip_network.c
+++ b/tools/usb/usbip/src/usbip_network.c
@@ -179,8 +179,8 @@ int usbip_net_recv_op_common(int sockfd, uint16_t *code)
PACK_OP_COMMON(0, &op_common);
if (op_common.version != USBIP_VERSION) {
- dbg("version mismatch: %d %d", op_common.version,
- USBIP_VERSION);
+ err("USBIP Kernel and tool version mismatch: %d %d:",
+ op_common.version, USBIP_VERSION);
goto err;
}
--
2.14.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usbip: tools usbip_attach: Fix cryptic error messages
Attach device error message is cryptic and useless. Fix it to be
informative.
Signed-off-by: Shuah Khan
---
tools/usb/usbip/src/usbip_attach.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/usb/usbip/src/usbip_attach.c
b/tools/usb/usbip/src/usbip_attach.c
index 7f07b2d50f59..f60738735398 100644
--- a/tools/usb/usbip/src/usbip_attach.c
+++ b/tools/usb/usbip/src/usbip_attach.c
@@ -195,7 +195,8 @@ static int attach_device(char *host, char *busid)
rhport = query_import_device(sockfd, busid);
if (rhport < 0) {
- err("query");
+ err("Attach request for Device %s. Is this device exported?",
+ busid);
return -1;
}
--
2.14.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/6] arm64: dts: exynos: add OF graph between MHL and USB connector
On Wed, Feb 21, 2018 at 2:55 AM, Andrzej Hajda wrote:
> OF graph describes MHL data lanes between MHL and respective USB
> connector.
>
> Signed-off-by: Andrzej Hajda
> ---
> v4:
> - added missing reg property in connector's port node (Krzysztof)
> ---
> .../boot/dts/exynos/exynos5433-tm2-common.dtsi | 32
> --
> 1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> index f604f6b1a9c2..2ed506df94d0 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> @@ -817,9 +817,22 @@
> clocks = <&pmu_system_controller 0>;
> clock-names = "xtal";
>
> - port {
> - mhl_to_hdmi: endpoint {
> - remote-endpoint = <&hdmi_to_mhl>;
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + mhl_to_hdmi: endpoint {
> + remote-endpoint = <&hdmi_to_mhl>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + mhl_to_musb_con: endpoint {
> + remote-endpoint = <&musb_con_to_mhl>;
> + };
These ports are mutually exclusive, right? If so, it should be 1 port
with 2 endpoints. Ports should represent independent data flows.
Something muxed or replicated (1 to many connection) should be be
endpoints.
Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/1] drivers/usb/storage: NULL pointer dereference [null-pointer-deref] (CWE 476) problem
On 2/27/2018 2:05 PM, Greg KH wrote:
On Tue, Feb 27, 2018 at 01:22:24PM -0500, Joe Moriarty wrote:
On 2/27/2018 1:14 PM, Greg KH wrote:
On Tue, Feb 27, 2018 at 09:59:40AM -0500, Joe Moriarty wrote:
On 2/26/2018 2:35 PM, Greg KH wrote:
On Mon, Feb 26, 2018 at 02:08:08PM -0500, Joe Moriarty wrote:
On 2/26/2018 1:12 PM, Greg KH wrote:
On Mon, Feb 26, 2018 at 12:10:02PM -0500, Joe Moriarty wrote:
The Parfait (version 2.1.0) static code analysis tool found the
following NULL pointer dereference problem.
What is the "CWE 476" thing in the subject line for?
[JDM]
(CWE 476) stands for Common Weakness Enumeration.
https://secur1ty.com/cwe/CWE-476/
It is the type of security flaw related to a NULL pointer dereference
Ok, why put that in the subject line and not just the body of the
changelog if you really want to call something out like this?
dev_to_shost() in include/scsi/scsi_host.h has the ability to return
NULL if the scsi host device does not have the Scsi_host->parent
field set. With the possibilty of a NULL pointer being set for
the Scsi_Host->parent field, calls to host_to_us() have to make
sure the return pointer is not null. Changes were made to check
for a return value of NULL on calls to host_to_us().
Signed-off-by: Joe Moriarty
Reviewed-by: Steven Sistare
Acked-by: Hakon Bugge
---
drivers/usb/storage/scsiglue.c | 53
--
1 file changed, 46 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index c267f2812a04..94af609d49bf 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -66,6 +66,9 @@ static int slave_alloc (struct scsi_device *sdev)
{
struct us_data *us = host_to_us(sdev->host);
+ if (!us)
+ pr_warn("Error in %s: us = NULL\n", __func__);
It is a driver, you should never use pr_* calls, but rather dev_* calls.
Also, you don't exit, are you sure the code keeps working after this
happens?
And what is a user supposed to do with this warning message?
[JDM]
??? You need a better email client, inline comments are the norm.
- The messages are targeted for a developer and not the end user. I can
change it to dev_ calls.
But an end user sees "KERN_WARNING" messages, right? If this is a
debugging-only thing, then make it as such, and properly recover from it
as it could be hit during normal operation.
+
/*
* Set the INQUIRY transfer length to 36. We don't use any of
* the extra data and many devices choke if asked for more or
@@ -102,6 +105,11 @@ static int slave_configure(struct scsi_device *sdev)
{
struct us_data *us = host_to_us(sdev->host);
+ if (!us) {
+ pr_warn("Error in %s: us = NULL\n", __func__);
+ return 0;
Why are you returning a success?
[JDM]
- Yes, I need to return -ENXIO for the slave_alloc routine.
+ }
+
/*
* Many devices have trouble transferring more than 32KB at a time,
* while others have trouble with more than 64K. At this time we
@@ -331,6 +339,11 @@ static int target_alloc(struct scsi_target *starget)
{
struct us_data *us = host_to_us(dev_to_shost(starget->dev.parent));
Can host_to_us() handle NULL?
Nope, just looked at it, this will never cause the return value to be
NULL, your checker needs some more work :(
[JDM]
This what the static code checker is catching. host_to_us will return NULL
if the following occurs.
'dev_to_shost()' in include/scsi/scsi_host.h line 757.
This is done everytime a slave device is created at the following code
'target_alloc()' in drivers/usb/storage/scsiglue.c linue 340.
This means that any call to host_to_us in the scsiglue module will have the
possibility of setting the return value of NULL. In fact, I would need to
split out the following embedded call in 'target_alloc()' to avoid a
possible NULL pointer dereference.
struct us_data *us = host_to_us(dev_to_shost(starget->dev.parent));
Exactly, your change did nothing, and us is probably not NULL here.
The question becomes, Can a slave device ever have it's parent field set to
NULL (ie: dev->parent).
I don't think it can, do you see how?
Ok, I believe we went off the original problem this patch solves. Since you
and I both agree that a slave device can never have it's parent field set to
NULL (ie: dev->parent) then this patch boils down to the following one
change.
include/scsi/scsi_host.h
static inline struct Scsi_Host *dev_to_shost(struct device *dev)
{
while (!scsi_is_host_device(dev)) {
if (!dev->parent)
- return NULL;
+ BUG();
No, never crash the kernel.
There is Bug() throughout the kernel. So that is invalid.
Nope, we are trying to get rid of them. If you see new ones being
added, please point it out.
Kernel Developers will put in Bug() statements anywhere the code is
not su
Re: [PATCH] cdc_ether: flag the Cinterion PLS8 modem by gemalto as WWAN
From: Bassem Boubaker Date: Tue, 27 Feb 2018 14:04:44 +0100 > The Cinterion PL8 is an LTE modem with 2 possible WWAN interfaces. > > The modem is controlled via AT commands through the exposed TTYs. > > AT^SWWAN write command can be used to activate or deactivate a WWAN > connection for a PDP context defined with AT+CGDCONT. UE supports > two WWAN adapter. Both WWAN adapters can be activated a the same time > > Signed-off-by: Bassem Boubaker Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/1] drivers/usb/storage: NULL pointer dereference [null-pointer-deref] (CWE 476) problem
On Tue, Feb 27, 2018 at 01:22:24PM -0500, Joe Moriarty wrote:
> On 2/27/2018 1:14 PM, Greg KH wrote:
> > On Tue, Feb 27, 2018 at 09:59:40AM -0500, Joe Moriarty wrote:
> > > On 2/26/2018 2:35 PM, Greg KH wrote:
> > > > On Mon, Feb 26, 2018 at 02:08:08PM -0500, Joe Moriarty wrote:
> > > > > On 2/26/2018 1:12 PM, Greg KH wrote:
> > > > > > On Mon, Feb 26, 2018 at 12:10:02PM -0500, Joe Moriarty wrote:
> > > > > > > The Parfait (version 2.1.0) static code analysis tool found the
> > > > > > > following NULL pointer dereference problem.
> > > > > >
> > > > > > What is the "CWE 476" thing in the subject line for?
> > > > > >
> > > > > [JDM]
> > > > > (CWE 476) stands for Common Weakness Enumeration.
> > > > > https://secur1ty.com/cwe/CWE-476/
> > > > >
> > > > > It is the type of security flaw related to a NULL pointer dereference
> > > >
> > > > Ok, why put that in the subject line and not just the body of the
> > > > changelog if you really want to call something out like this?
> > > >
> > > > > > > dev_to_shost() in include/scsi/scsi_host.h has the ability to
> > > > > > > return
> > > > > > > NULL if the scsi host device does not have the Scsi_host->parent
> > > > > > > field set. With the possibilty of a NULL pointer being set for
> > > > > > > the Scsi_Host->parent field, calls to host_to_us() have to make
> > > > > > > sure the return pointer is not null. Changes were made to check
> > > > > > > for a return value of NULL on calls to host_to_us().
> > > > > > >
> > > > > > > Signed-off-by: Joe Moriarty
> > > > > > > Reviewed-by: Steven Sistare
> > > > > > > Acked-by: Hakon Bugge
> > > > > > > ---
> > > > > > > drivers/usb/storage/scsiglue.c | 53
> > > > > > > --
> > > > > > > 1 file changed, 46 insertions(+), 7 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/usb/storage/scsiglue.c
> > > > > > > b/drivers/usb/storage/scsiglue.c
> > > > > > > index c267f2812a04..94af609d49bf 100644
> > > > > > > --- a/drivers/usb/storage/scsiglue.c
> > > > > > > +++ b/drivers/usb/storage/scsiglue.c
> > > > > > > @@ -66,6 +66,9 @@ static int slave_alloc (struct scsi_device
> > > > > > > *sdev)
> > > > > > > {
> > > > > > > struct us_data *us = host_to_us(sdev->host);
> > > > > > > + if (!us)
> > > > > > > + pr_warn("Error in %s: us = NULL\n", __func__);
> > > > > >
> > > > > > It is a driver, you should never use pr_* calls, but rather dev_*
> > > > > > calls.
> > > > > >
> > > > > > Also, you don't exit, are you sure the code keeps working after this
> > > > > > happens?
> > > > > >
> > > > > > And what is a user supposed to do with this warning message?
> > > > >
> > > > > [JDM]
> > > >
> > > > ??? You need a better email client, inline comments are the norm.
> > > >
> > > > > - The messages are targeted for a developer and not the end user. I
> > > > > can
> > > > > change it to dev_ calls.
> > > >
> > > > But an end user sees "KERN_WARNING" messages, right? If this is a
> > > > debugging-only thing, then make it as such, and properly recover from it
> > > > as it could be hit during normal operation.
> > > >
> > > > > > > +
> > > > > > > /*
> > > > > > >* Set the INQUIRY transfer length to 36. We don't use
> > > > > > > any of
> > > > > > >* the extra data and many devices choke if asked for
> > > > > > > more or
> > > > > > > @@ -102,6 +105,11 @@ static int slave_configure(struct
> > > > > > > scsi_device *sdev)
> > > > > > > {
> > > > > > > struct us_data *us = host_to_us(sdev->host);
> > > > > > > + if (!us) {
> > > > > > > + pr_warn("Error in %s: us = NULL\n", __func__);
> > > > > > > + return 0;
> > > > > >
> > > > > > Why are you returning a success?
> > > > > [JDM]
> > > > > - Yes, I need to return -ENXIO for the slave_alloc routine.
> > > > >
> > > > > >
> > > > > > > + }
> > > > > > > +
> > > > > > > /*
> > > > > > >* Many devices have trouble transferring more than
> > > > > > > 32KB at a time,
> > > > > > >* while others have trouble with more than 64K. At
> > > > > > > this time we
> > > > > > > @@ -331,6 +339,11 @@ static int target_alloc(struct scsi_target
> > > > > > > *starget)
> > > > > > > {
> > > > > > > struct us_data *us =
> > > > > > > host_to_us(dev_to_shost(starget->dev.parent));
> > > > > >
> > > > > > Can host_to_us() handle NULL?
> > > > > >
> > > > > > Nope, just looked at it, this will never cause the return value to
> > > > > > be
> > > > > > NULL, your checker needs some more work :(
> > > > > [JDM]
> > > > > This what the static code checker is catching. host_to_us will
> > > > > return NULL
> > > > > if the following occurs.
> > > > >
> > > > > 'dev_to_shost()' in include/scsi/scsi_host.h line 757.
> > > > >
> > > > > This is done everytime a slave device is created at the following code
> > > > > 'target_alloc()' in drivers/usb/storage/scsiglue.c lin
Re: [PATCH v1 1/1] drivers/usb/storage: NULL pointer dereference [null-pointer-deref] (CWE 476) problem
On 2/27/2018 1:14 PM, Greg KH wrote:
On Tue, Feb 27, 2018 at 09:59:40AM -0500, Joe Moriarty wrote:
On 2/26/2018 2:35 PM, Greg KH wrote:
On Mon, Feb 26, 2018 at 02:08:08PM -0500, Joe Moriarty wrote:
On 2/26/2018 1:12 PM, Greg KH wrote:
On Mon, Feb 26, 2018 at 12:10:02PM -0500, Joe Moriarty wrote:
The Parfait (version 2.1.0) static code analysis tool found the
following NULL pointer dereference problem.
What is the "CWE 476" thing in the subject line for?
[JDM]
(CWE 476) stands for Common Weakness Enumeration.
https://secur1ty.com/cwe/CWE-476/
It is the type of security flaw related to a NULL pointer dereference
Ok, why put that in the subject line and not just the body of the
changelog if you really want to call something out like this?
dev_to_shost() in include/scsi/scsi_host.h has the ability to return
NULL if the scsi host device does not have the Scsi_host->parent
field set. With the possibilty of a NULL pointer being set for
the Scsi_Host->parent field, calls to host_to_us() have to make
sure the return pointer is not null. Changes were made to check
for a return value of NULL on calls to host_to_us().
Signed-off-by: Joe Moriarty
Reviewed-by: Steven Sistare
Acked-by: Hakon Bugge
---
drivers/usb/storage/scsiglue.c | 53
--
1 file changed, 46 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index c267f2812a04..94af609d49bf 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -66,6 +66,9 @@ static int slave_alloc (struct scsi_device *sdev)
{
struct us_data *us = host_to_us(sdev->host);
+ if (!us)
+ pr_warn("Error in %s: us = NULL\n", __func__);
It is a driver, you should never use pr_* calls, but rather dev_* calls.
Also, you don't exit, are you sure the code keeps working after this
happens?
And what is a user supposed to do with this warning message?
[JDM]
??? You need a better email client, inline comments are the norm.
- The messages are targeted for a developer and not the end user. I can
change it to dev_ calls.
But an end user sees "KERN_WARNING" messages, right? If this is a
debugging-only thing, then make it as such, and properly recover from it
as it could be hit during normal operation.
+
/*
* Set the INQUIRY transfer length to 36. We don't use any of
* the extra data and many devices choke if asked for more or
@@ -102,6 +105,11 @@ static int slave_configure(struct scsi_device *sdev)
{
struct us_data *us = host_to_us(sdev->host);
+ if (!us) {
+ pr_warn("Error in %s: us = NULL\n", __func__);
+ return 0;
Why are you returning a success?
[JDM]
- Yes, I need to return -ENXIO for the slave_alloc routine.
+ }
+
/*
* Many devices have trouble transferring more than 32KB at a time,
* while others have trouble with more than 64K. At this time we
@@ -331,6 +339,11 @@ static int target_alloc(struct scsi_target *starget)
{
struct us_data *us = host_to_us(dev_to_shost(starget->dev.parent));
Can host_to_us() handle NULL?
Nope, just looked at it, this will never cause the return value to be
NULL, your checker needs some more work :(
[JDM]
This what the static code checker is catching. host_to_us will return NULL
if the following occurs.
'dev_to_shost()' in include/scsi/scsi_host.h line 757.
This is done everytime a slave device is created at the following code
'target_alloc()' in drivers/usb/storage/scsiglue.c linue 340.
This means that any call to host_to_us in the scsiglue module will have the
possibility of setting the return value of NULL. In fact, I would need to
split out the following embedded call in 'target_alloc()' to avoid a
possible NULL pointer dereference.
struct us_data *us = host_to_us(dev_to_shost(starget->dev.parent));
Exactly, your change did nothing, and us is probably not NULL here.
The question becomes, Can a slave device ever have it's parent field set to
NULL (ie: dev->parent).
I don't think it can, do you see how?
Ok, I believe we went off the original problem this patch solves. Since you
and I both agree that a slave device can never have it's parent field set to
NULL (ie: dev->parent) then this patch boils down to the following one
change.
include/scsi/scsi_host.h
static inline struct Scsi_Host *dev_to_shost(struct device *dev)
{
while (!scsi_is_host_device(dev)) {
if (!dev->parent)
- return NULL;
+ BUG();
No, never crash the kernel.
There is Bug() throughout the kernel. So that is invalid. Kernel
Developers will put in Bug() statements anywhere the code is not suppose
to go. It is easier to debug and then trying to backtrace from a side
effect issue.
Why not ask the scsi developers about this? They put that line there
for a good reason, right?
I submitted
Re: [PATCH v1 1/1] drivers/usb/storage: NULL pointer dereference [null-pointer-deref] (CWE 476) problem
On Tue, Feb 27, 2018 at 09:59:40AM -0500, Joe Moriarty wrote:
> On 2/26/2018 2:35 PM, Greg KH wrote:
> > On Mon, Feb 26, 2018 at 02:08:08PM -0500, Joe Moriarty wrote:
> > > On 2/26/2018 1:12 PM, Greg KH wrote:
> > > > On Mon, Feb 26, 2018 at 12:10:02PM -0500, Joe Moriarty wrote:
> > > > > The Parfait (version 2.1.0) static code analysis tool found the
> > > > > following NULL pointer dereference problem.
> > > >
> > > > What is the "CWE 476" thing in the subject line for?
> > > >
> > > [JDM]
> > > (CWE 476) stands for Common Weakness Enumeration.
> > > https://secur1ty.com/cwe/CWE-476/
> > >
> > > It is the type of security flaw related to a NULL pointer dereference
> >
> > Ok, why put that in the subject line and not just the body of the
> > changelog if you really want to call something out like this?
> >
> > > > > dev_to_shost() in include/scsi/scsi_host.h has the ability to return
> > > > > NULL if the scsi host device does not have the Scsi_host->parent
> > > > > field set. With the possibilty of a NULL pointer being set for
> > > > > the Scsi_Host->parent field, calls to host_to_us() have to make
> > > > > sure the return pointer is not null. Changes were made to check
> > > > > for a return value of NULL on calls to host_to_us().
> > > > >
> > > > > Signed-off-by: Joe Moriarty
> > > > > Reviewed-by: Steven Sistare
> > > > > Acked-by: Hakon Bugge
> > > > > ---
> > > > >drivers/usb/storage/scsiglue.c | 53
> > > > > --
> > > > >1 file changed, 46 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/drivers/usb/storage/scsiglue.c
> > > > > b/drivers/usb/storage/scsiglue.c
> > > > > index c267f2812a04..94af609d49bf 100644
> > > > > --- a/drivers/usb/storage/scsiglue.c
> > > > > +++ b/drivers/usb/storage/scsiglue.c
> > > > > @@ -66,6 +66,9 @@ static int slave_alloc (struct scsi_device *sdev)
> > > > >{
> > > > > struct us_data *us = host_to_us(sdev->host);
> > > > > + if (!us)
> > > > > + pr_warn("Error in %s: us = NULL\n", __func__);
> > > >
> > > > It is a driver, you should never use pr_* calls, but rather dev_* calls.
> > > >
> > > > Also, you don't exit, are you sure the code keeps working after this
> > > > happens?
> > > >
> > > > And what is a user supposed to do with this warning message?
> > >
> > > [JDM]
> >
> > ??? You need a better email client, inline comments are the norm.
> >
> > > - The messages are targeted for a developer and not the end user. I can
> > > change it to dev_ calls.
> >
> > But an end user sees "KERN_WARNING" messages, right? If this is a
> > debugging-only thing, then make it as such, and properly recover from it
> > as it could be hit during normal operation.
> >
> > > > > +
> > > > > /*
> > > > >* Set the INQUIRY transfer length to 36. We don't use any of
> > > > >* the extra data and many devices choke if asked for more or
> > > > > @@ -102,6 +105,11 @@ static int slave_configure(struct scsi_device
> > > > > *sdev)
> > > > >{
> > > > > struct us_data *us = host_to_us(sdev->host);
> > > > > + if (!us) {
> > > > > + pr_warn("Error in %s: us = NULL\n", __func__);
> > > > > + return 0;
> > > >
> > > > Why are you returning a success?
> > > [JDM]
> > > - Yes, I need to return -ENXIO for the slave_alloc routine.
> > >
> > > >
> > > > > + }
> > > > > +
> > > > > /*
> > > > >* Many devices have trouble transferring more than 32KB at a
> > > > > time,
> > > > >* while others have trouble with more than 64K. At this time we
> > > > > @@ -331,6 +339,11 @@ static int target_alloc(struct scsi_target
> > > > > *starget)
> > > > >{
> > > > > struct us_data *us =
> > > > > host_to_us(dev_to_shost(starget->dev.parent));
> > > >
> > > > Can host_to_us() handle NULL?
> > > >
> > > > Nope, just looked at it, this will never cause the return value to be
> > > > NULL, your checker needs some more work :(
> > > [JDM]
> > > This what the static code checker is catching. host_to_us will return
> > > NULL
> > > if the following occurs.
> > >
> > > 'dev_to_shost()' in include/scsi/scsi_host.h line 757.
> > >
> > > This is done everytime a slave device is created at the following code
> > > 'target_alloc()' in drivers/usb/storage/scsiglue.c linue 340.
> > >
> > > This means that any call to host_to_us in the scsiglue module will have
> > > the
> > > possibility of setting the return value of NULL. In fact, I would need to
> > > split out the following embedded call in 'target_alloc()' to avoid a
> > > possible NULL pointer dereference.
> > >
> > > struct us_data *us = host_to_us(dev_to_shost(starget->dev.parent));
> >
> > Exactly, your change did nothing, and us is probably not NULL here.
> >
> > > The question becomes, Can a slave device ever have it's parent field set
> > > to
> > > NULL (ie: dev->parent).
> >
> > I don't think it can, do you see how?
> >
> Ok, I believ
Re: [RFC PATCH] usb: hcd: complete URBs in threaded-IRQ context instead of tasklet
Hi Sebastian,
Em Fri, 16 Feb 2018 18:04:50 +0100
Sebastian Andrzej Siewior escreveu:
> I've been going over Frederic's softirq patches and it seems that there
> were two problems. One was network related, the other was Mauro's USB
> dvb-[stc] device which was not able to stream properly over time.
>
> Here is an attempt to let the URB complete in the threaded handler
> instead of going to the tasklet. In case the system is SMP then the
> patch [0] would be required in order to have the IRQ and its thread on
> the same CPU.
>
> Mauro, would you mind giving it a shot?
Sorry for taking some time to test it, has been busy those days...
Anyway, I tested it today. Didn't work. It keep losing data.
Regards,
>
> [0] genirq: Let irq thread follow the effective hard irq affinity
> https://git.kernel.org/tip/cbf866a6e7f2f674b3a2a4cef9f666ff613e
>
> ---
>
> The urb->complete callback was initially invoked in IRQ context after
> the HCD dropped its lock because the callback could re-queue the URB
> again. Later this completion was deferred to the tasklet allowing the
> HCD hold the lock. Also the BH handler can be interrupted by the IRQ
> handler adding more "completed" requests to its queue.
> While this batching is good in general, the softirq defers its doing for
> short period of time if it is running constantly without a break. This
> breaks some use cases where constant USB throughput is required.
> As an alternative approach to tasklet handling, I defer the URB
> completion to the HCD's threaded handler. There are two lists for
> "high-prio" proccessing and lower priority (to mimic current behaviour).
> The URBs in the high-priority list are always preffered over the URBs
> in the low-priority list.
> The URBs from the root-hub never create an interrupt so I currently
> process them in a workqueue (I'm not sure if an URB-enqueue in the
> completion handler would break something).
>
> Signed-off-by: Sebastian Andrzej Siewior
> ---
> drivers/usb/core/hcd.c | 131
>
> include/linux/usb/hcd.h | 14 +++---
> 2 files changed, 95 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index fc32391a34d5..8d6dd4d3cbe7 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -1775,33 +1775,75 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
> usb_put_urb(urb);
> }
>
> -static void usb_giveback_urb_bh(unsigned long param)
> +static void usb_hcd_rh_gb_urb(struct work_struct *work)
> {
> - struct giveback_urb_bh *bh = (struct giveback_urb_bh *)param;
> - struct list_head local_list;
> + struct giveback_urb *bh = container_of(work, struct giveback_urb,
> +rh_compl);
> + struct list_headurb_list;
>
> spin_lock_irq(&bh->lock);
> - bh->running = true;
> - restart:
> - list_replace_init(&bh->head, &local_list);
> + list_replace_init(&bh->rh_head, &urb_list);
> spin_unlock_irq(&bh->lock);
>
> - while (!list_empty(&local_list)) {
> + while (!list_empty(&urb_list)) {
> struct urb *urb;
>
> - urb = list_entry(local_list.next, struct urb, urb_list);
> + urb = list_first_entry(&urb_list, struct urb, urb_list);
> list_del_init(&urb->urb_list);
> - bh->completing_ep = urb->ep;
> __usb_hcd_giveback_urb(urb);
> - bh->completing_ep = NULL;
> + }
> +}
> +
> +
> +#define URB_PRIO_HIGH0
> +#define URB_PRIO_LOW 1
> +
> +static irqreturn_t usb_hcd_gb_urb(int irq, void *__hcd)
> +{
> + struct usb_hcd *hcd = __hcd;
> + struct giveback_urb *bh = &hcd->gb_urb;
> + struct list_headurb_list[2];
> + int i;
> +
> + INIT_LIST_HEAD(&urb_list[URB_PRIO_HIGH]);
> + INIT_LIST_HEAD(&urb_list[URB_PRIO_LOW]);
> +
> + spin_lock_irq(&bh->lock);
> + restart:
> + list_splice_tail_init(&bh->prio_hi_head, &urb_list[URB_PRIO_HIGH]);
> + list_splice_tail_init(&bh->prio_lo_head, &urb_list[URB_PRIO_LOW]);
> + spin_unlock_irq(&bh->lock);
> +
> + for (i = 0; i < ARRAY_SIZE(urb_list); i++) {
> + while (!list_empty(&urb_list[i])) {
> + struct urb *urb;
> +
> + urb = list_first_entry(&urb_list[i],
> +struct urb, urb_list);
> + list_del_init(&urb->urb_list);
> + if (i == URB_PRIO_HIGH)
> + bh->completing_ep = urb->ep;
> +
> + __usb_hcd_giveback_urb(urb);
> +
> + if (i == URB_PRIO_HIGH)
> + bh->completing_ep = NULL;
> +
> + if (i == URB_PRIO_LOW &&
> + !list_empty_careful(&urb_list[URB_PRIO_HIGH])) {
> + spin_lock_irq(&bh->lock);
> +
Re: [PATCH v1 1/1] drivers/usb/storage: NULL pointer dereference [null-pointer-deref] (CWE 476) problem
On 2/27/2018 9:59 AM, Joe Moriarty wrote:
On 2/26/2018 2:35 PM, Greg KH wrote:
On Mon, Feb 26, 2018 at 02:08:08PM -0500, Joe Moriarty wrote:
On 2/26/2018 1:12 PM, Greg KH wrote:
On Mon, Feb 26, 2018 at 12:10:02PM -0500, Joe Moriarty wrote:
The Parfait (version 2.1.0) static code analysis tool found the
following NULL pointer dereference problem.
What is the "CWE 476" thing in the subject line for?
[JDM]
(CWE 476) stands for Common Weakness Enumeration.
https://secur1ty.com/cwe/CWE-476/
It is the type of security flaw related to a NULL pointer dereference
Ok, why put that in the subject line and not just the body of the
changelog if you really want to call something out like this?
dev_to_shost() in include/scsi/scsi_host.h has the ability to return
NULL if the scsi host device does not have the Scsi_host->parent
field set. With the possibilty of a NULL pointer being set for
the Scsi_Host->parent field, calls to host_to_us() have to make
sure the return pointer is not null. Changes were made to check
for a return value of NULL on calls to host_to_us().
Signed-off-by: Joe Moriarty
Reviewed-by: Steven Sistare
Acked-by: Hakon Bugge
---
drivers/usb/storage/scsiglue.c | 53
--
1 file changed, 46 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/storage/scsiglue.c
b/drivers/usb/storage/scsiglue.c
index c267f2812a04..94af609d49bf 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -66,6 +66,9 @@ static int slave_alloc (struct scsi_device *sdev)
{
struct us_data *us = host_to_us(sdev->host);
+ if (!us)
+ pr_warn("Error in %s: us = NULL\n", __func__);
It is a driver, you should never use pr_* calls, but rather dev_*
calls.
Also, you don't exit, are you sure the code keeps working after this
happens?
And what is a user supposed to do with this warning message?
[JDM]
??? You need a better email client, inline comments are the norm.
- The messages are targeted for a developer and not the end user. I can
change it to dev_ calls.
But an end user sees "KERN_WARNING" messages, right? If this is a
debugging-only thing, then make it as such, and properly recover from it
as it could be hit during normal operation.
+
/*
* Set the INQUIRY transfer length to 36. We don't use any of
* the extra data and many devices choke if asked for more or
@@ -102,6 +105,11 @@ static int slave_configure(struct scsi_device
*sdev)
{
struct us_data *us = host_to_us(sdev->host);
+ if (!us) {
+ pr_warn("Error in %s: us = NULL\n", __func__);
+ return 0;
Why are you returning a success?
[JDM]
- Yes, I need to return -ENXIO for the slave_alloc routine.
+ }
+
/*
* Many devices have trouble transferring more than 32KB at
a time,
* while others have trouble with more than 64K. At this
time we
@@ -331,6 +339,11 @@ static int target_alloc(struct scsi_target
*starget)
{
struct us_data *us =
host_to_us(dev_to_shost(starget->dev.parent));
Can host_to_us() handle NULL?
Nope, just looked at it, this will never cause the return value to be
NULL, your checker needs some more work :(
[JDM]
This what the static code checker is catching. host_to_us will
return NULL
if the following occurs.
'dev_to_shost()' in include/scsi/scsi_host.h line 757.
This is done everytime a slave device is created at the following code
'target_alloc()' in drivers/usb/storage/scsiglue.c linue 340.
This means that any call to host_to_us in the scsiglue module will
have the
possibility of setting the return value of NULL. In fact, I would
need to
split out the following embedded call in 'target_alloc()' to avoid a
possible NULL pointer dereference.
struct us_data *us = host_to_us(dev_to_shost(starget->dev.parent));
Exactly, your change did nothing, and us is probably not NULL here.
The question becomes, Can a slave device ever have it's parent field
set to
NULL (ie: dev->parent).
I don't think it can, do you see how?
Ok, I believe we went off the original problem this patch solves. Since
you and I both agree that a slave device can never have it's parent
field set to NULL (ie: dev->parent) then this patch boils down to the
following one change.
include/scsi/scsi_host.h
static inline struct Scsi_Host *dev_to_shost(struct device *dev)
{
while (!scsi_is_host_device(dev)) {
if (!dev->parent)
- return NULL;
+ BUG();
dev = dev->parent;
}
return container_of(dev, struct Scsi_Host, shost_gendev);
}
The above code change cleared up an additional 49 NULL pointer
dereference instances found from the parfait static analysis. I checked
a few of them out and notices that none of them were checking for a NULL
pointer being returned from dev_to_shost(). This looks very much like a
code path that is never taken and is never believe to occur. In fact,
if the code path was ta
Re: inconsistent lock state with usbnet/asix usb ethernet and xhci
Am Dienstag, den 27.02.2018, 07:13 -0800 schrieb Eric Dumazet: > On Tue, 2018-02-27 at 07:09 -0800, Eric Dumazet wrote: > > > > > > Note that for this one, it seems we also could perform stats updates in > > BH context, since skb is queued via defer_bh() > > > > But simplicity wins I guess. > > Thinking more about this, I am not sure we have any guarantee that TX > and RX can not run on multiple cpus. > > Using an unique syncp is not going to be safe, even if we make lockdep > happy enough with the local_irq save/restore. Unfortunately you are right. It is not guaranteed for some hardware. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Wakeup from USB on i.MX6S
Hi Peter, here is an overview of the output voltages of all voltage regulators I could find on that system: for i in $(find / -name microvolts);do echo $i -- $(cat $i);done /sys/devices/soc0/soc/200.aips-bus/20c8000.anatop/20c8000.anatop:regulator-3p0/regulator/regulator.2/microvolts -- 300 /sys/devices/soc0/soc/200.aips-bus/20c8000.anatop/20c8000.anatop:regulator-vddsoc/regulator/regulator.6/microvolts -- 1175000 /sys/devices/soc0/soc/200.aips-bus/20c8000.anatop/20c8000.anatop:regulator-1p1/regulator/regulator.1/microvolts -- 110 /sys/devices/soc0/soc/200.aips-bus/20c8000.anatop/20c8000.anatop:regulator-vddcore/regulator/regulator.4/microvolts -- 1175000 /sys/devices/soc0/soc/200.aips-bus/20c8000.anatop/20c8000.anatop:regulator-2p5/regulator/regulator.3/microvolts -- 240 /sys/devices/soc0/soc/200.aips-bus/20c8000.anatop/20c8000.anatop:regulator-vddpu/regulator/regulator.5/microvolts -- 1175000 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.26/microvolts -- 180 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.16/microvolts -- 180 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.24/microvolts -- 250 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.14/microvolts -- 130 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.22/microvolts -- 250 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.20/microvolts -- 120 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.29/microvolts -- 280 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.19/microvolts -- 170 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.27/microvolts -- 330 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.17/microvolts -- 180 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.25/microvolts -- 250 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.15/microvolts -- 150 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.23/microvolts -- 330 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.13/microvolts -- 130 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.21/microvolts -- 250 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.28/microvolts -- 120 /sys/devices/soc0/soc/210.aips-bus/21a.i2c/i2c-0/0-0058/da9063-regulators/regulator/regulator.18/microvolts -- 330 /sys/devices/soc0/supplies/supplies:supply@0/regulator/regulator.9/microvolts -- 500 /sys/devices/soc0/supplies/supplies:supply@1/regulator/regulator.10/microvolts -- 330 /sys/devices/soc0/supplies/supplies:otgvbus/regulator/regulator.12/microvolts -- 500 /sys/devices/soc0/supplies/supplies:supply@2/regulator/regulator.11/microvolts -- 150 /sys/devices/soc0/regulators/regulators:regulator@1/regulator/regulator.8/microvolts -- 500 /sys/devices/soc0/regulators/regulators:regulator@0/regulator/regulator.7/microvolts -- 500 On 27 February 2018 2:45 AM, Peter Chen wrote: > Currently, the wakeup only occurs entering "mem" low power mode, > it means the 2P5 is changed during "entering" process, so you may > need to measure it if 2P5 is here when the system is at "mem" low > power mode. Another way to check if it is vbus wakeup, you can > disable vbus wakeup by not setting MX6\_BM\_VBUS\_WAKEUP at usbmisc\_imx.c (...) > VBUS should be there at runtime due to you can find the HUB. Like the > 2P5, I hope you can measure it too. I've searched the whole board with my multimeter for something that uses the 2P5. According to the emCON-MX6x-Manual [3], there are several connectors that use 2.5V: SATA, PCIE, LVDS, HDMI, MIPI. I've found Pin 32 of the miniPCIe Socket to have ~2.30V. It is called I2C1_SDA. I wonder if this is a proper Pin to test 2P5? I couldn't find any information about what is connected to the 2P5 and what's not. Pin32(I2C_SDA) is powered in standby, indeed. Without enabling any additional wakup sources (after boot, only the wakeup button is enabled). Also the VBUS of the USB Port is on ~5,07V. During mem-standby without enabling additional wakeup sources (only wakeup button (gpio) is enabled) > To check if the wakeup signal is from the HUB, you may not enable wakeup > for below entry: > /sys
Re: inconsistent lock state with usbnet/asix usb ethernet and xhci
On Tue, 2018-02-27 at 07:09 -0800, Eric Dumazet wrote: > > Note that for this one, it seems we also could perform stats updates in > BH context, since skb is queued via defer_bh() > > But simplicity wins I guess. Thinking more about this, I am not sure we have any guarantee that TX and RX can not run on multiple cpus. Using an unique syncp is not going to be safe, even if we make lockdep happy enough with the local_irq save/restore. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: inconsistent lock state with usbnet/asix usb ethernet and xhci
On Tue, 2018-02-27 at 15:42 +0100, Marek Szyprowski wrote:
> Hi Eric,
>
> On 2018-02-27 15:07, Eric Dumazet wrote:
> > On Tue, 2018-02-27 at 08:26 +0100, Marek Szyprowski wrote:
> > > I've noticed that USBnet/ASIX AX88772B USB driver produces deplock kernel
> > > warning ("inconsistent lock state") on Chromebook2 Peach-PIT board. No
> > > special activity is needed to reproduce this issue, it happens almost
> > > on every boot. ASIX USB ethernet is connected to XHCI USB host controller
> > > on that board. Is it a known issue? Frankly I have no idea where to look
> > > to fix it. The same adapter connected to EHCI ports on other boards based
> > > on the same SoC works fine without any warnings.
> > >
> > > Here are some more information from that board:
> > > # lsusb
> > > Bus 006 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> > > Bus 005 Device 002: ID 0b95:772b ASIX Electronics Corp. AX88772B
> > > Bus 005 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> > > Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> > > Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> > > Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> > > Bus 001 Device 002: ID 2232:1056 Silicon Motion
> > > Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> > >
> > > # lsusb -t
> > > /: Bus 06.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M
> > > /: Bus 05.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 480M
> > > |__ Port 1: Dev 2, If 0, Class=Vendor Specific Class, Driver=asix,
> > > 480M
> > > /: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M
> > > /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 480M
> > > /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=exynos-ohci/3p, 12M
> > > /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=exynos-ehci/3p, 480M
> > > |__ Port 1: Dev 2, If 0, Class=Video, Driver=, 480M
> > > |__ Port 1: Dev 2, If 1, Class=Video, Driver=, 480M
> > >
> > >
> > > And the log with mentioned warning:
> > >
> > > [ 17.768040]
> > > [ 17.772239] WARNING: inconsistent lock state
> > > [ 17.776511] 4.16.0-rc3-next-20180227-7-g876c53a7493c #453 Not
> > > tainted
> > > [ 17.783329]
> > > [ 17.787580] inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
> > > [ 17.793607] swapper/0/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
> > > [ 17.798751] (&syncp->seq#5){?.-.}, at: [<9b22e5f0>]
> > > asix_rx_fixup_internal+0x188/0x288
> > > [ 17.806790] {IN-HARDIRQ-W} state was registered at:
> > > [ 17.811677] tx_complete+0x100/0x208
> > > [ 17.815319] __usb_hcd_giveback_urb+0x60/0xf0
> > > [ 17.819770] xhci_giveback_urb_in_irq+0xa8/0x240
> > > [ 17.824469] xhci_td_cleanup+0xf4/0x16c
> > > [ 17.828367] xhci_irq+0xe74/0x2240
> > > [ 17.831827] usb_hcd_irq+0x24/0x38
> > > [ 17.835343] __handle_irq_event_percpu+0x98/0x510
> > > [ 17.840111] handle_irq_event_percpu+0x1c/0x58
> > > [ 17.844623] handle_irq_event+0x38/0x5c
> > > [ 17.848519] handle_fasteoi_irq+0xa4/0x138
> > > [ 17.852681] generic_handle_irq+0x18/0x28
> > > [ 17.856760] __handle_domain_irq+0x6c/0xe4
> > > [ 17.860941] gic_handle_irq+0x54/0xa0
> > > [ 17.864666] __irq_svc+0x70/0xb0
> > > [ 17.867964] arch_cpu_idle+0x20/0x3c
> > > [ 17.871578] arch_cpu_idle+0x20/0x3c
> > > [ 17.875190] do_idle+0x144/0x218
> > > [ 17.878468] cpu_startup_entry+0x18/0x1c
> > > [ 17.882454] start_kernel+0x394/0x400
> > > [ 17.886177] irq event stamp: 161912
> > > [ 17.889616] hardirqs last enabled at (161912): [<7bedfacf>]
> > > __netdev_alloc_skb+0xcc/0x140
> > > [ 17.897893] hardirqs last disabled at (161911): []
> > > __netdev_alloc_skb+0x94/0x140
> > > [ 17.904903] exynos5-hsi2c 12ca.i2c: tx timeout
> > > [ 17.906116] softirqs last enabled at (161904): [<387102ff>]
> > > irq_enter+0x78/0x80
> > > [ 17.906123] softirqs last disabled at (161905): []
> > > irq_exit+0x134/0x158
> > > [ 17.925722].
> > > [ 17.925722] other info that might help us debug this:
> > > [ 17.933435] Possible unsafe locking scenario:
> > > [ 17.933435].
> > &g
Re: [PATCH v1 1/1] drivers/usb/storage: NULL pointer dereference [null-pointer-deref] (CWE 476) problem
On 2/26/2018 2:35 PM, Greg KH wrote:
On Mon, Feb 26, 2018 at 02:08:08PM -0500, Joe Moriarty wrote:
On 2/26/2018 1:12 PM, Greg KH wrote:
On Mon, Feb 26, 2018 at 12:10:02PM -0500, Joe Moriarty wrote:
The Parfait (version 2.1.0) static code analysis tool found the
following NULL pointer dereference problem.
What is the "CWE 476" thing in the subject line for?
[JDM]
(CWE 476) stands for Common Weakness Enumeration.
https://secur1ty.com/cwe/CWE-476/
It is the type of security flaw related to a NULL pointer dereference
Ok, why put that in the subject line and not just the body of the
changelog if you really want to call something out like this?
dev_to_shost() in include/scsi/scsi_host.h has the ability to return
NULL if the scsi host device does not have the Scsi_host->parent
field set. With the possibilty of a NULL pointer being set for
the Scsi_Host->parent field, calls to host_to_us() have to make
sure the return pointer is not null. Changes were made to check
for a return value of NULL on calls to host_to_us().
Signed-off-by: Joe Moriarty
Reviewed-by: Steven Sistare
Acked-by: Hakon Bugge
---
drivers/usb/storage/scsiglue.c | 53
--
1 file changed, 46 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index c267f2812a04..94af609d49bf 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -66,6 +66,9 @@ static int slave_alloc (struct scsi_device *sdev)
{
struct us_data *us = host_to_us(sdev->host);
+ if (!us)
+ pr_warn("Error in %s: us = NULL\n", __func__);
It is a driver, you should never use pr_* calls, but rather dev_* calls.
Also, you don't exit, are you sure the code keeps working after this
happens?
And what is a user supposed to do with this warning message?
[JDM]
??? You need a better email client, inline comments are the norm.
- The messages are targeted for a developer and not the end user. I can
change it to dev_ calls.
But an end user sees "KERN_WARNING" messages, right? If this is a
debugging-only thing, then make it as such, and properly recover from it
as it could be hit during normal operation.
+
/*
* Set the INQUIRY transfer length to 36. We don't use any of
* the extra data and many devices choke if asked for more or
@@ -102,6 +105,11 @@ static int slave_configure(struct scsi_device *sdev)
{
struct us_data *us = host_to_us(sdev->host);
+ if (!us) {
+ pr_warn("Error in %s: us = NULL\n", __func__);
+ return 0;
Why are you returning a success?
[JDM]
- Yes, I need to return -ENXIO for the slave_alloc routine.
+ }
+
/*
* Many devices have trouble transferring more than 32KB at a time,
* while others have trouble with more than 64K. At this time we
@@ -331,6 +339,11 @@ static int target_alloc(struct scsi_target *starget)
{
struct us_data *us = host_to_us(dev_to_shost(starget->dev.parent));
Can host_to_us() handle NULL?
Nope, just looked at it, this will never cause the return value to be
NULL, your checker needs some more work :(
[JDM]
This what the static code checker is catching. host_to_us will return NULL
if the following occurs.
'dev_to_shost()' in include/scsi/scsi_host.h line 757.
This is done everytime a slave device is created at the following code
'target_alloc()' in drivers/usb/storage/scsiglue.c linue 340.
This means that any call to host_to_us in the scsiglue module will have the
possibility of setting the return value of NULL. In fact, I would need to
split out the following embedded call in 'target_alloc()' to avoid a
possible NULL pointer dereference.
struct us_data *us = host_to_us(dev_to_shost(starget->dev.parent));
Exactly, your change did nothing, and us is probably not NULL here.
The question becomes, Can a slave device ever have it's parent field set to
NULL (ie: dev->parent).
I don't think it can, do you see how?
Ok, I believe we went off the original problem this patch solves.
Since you and I both agree that a slave device can never have it's
parent field set to NULL (ie: dev->parent) then this patch boils down
to the following one change.
include/scsi/scsi_host.h
static inline struct Scsi_Host *dev_to_shost(struct device *dev)
{
while (!scsi_is_host_device(dev)) {
if (!dev->parent)
- return NULL;
+ BUG();
dev = dev->parent;
}
return container_of(dev, struct Scsi_Host, shost_gendev);
}
If you agree, then I can revert all the other changes and resubmit a
version 2 of this patch with this change. If you do not agree, then I
will mark it as a false positive and move on, or we can continue to work
on the initial patch to get it to your satisfaction. Let me know how
you would like to proceed here.
Joe
Can it? If not, this becomes a
Re: inconsistent lock state with usbnet/asix usb ethernet and xhci
Hi Eric,
On 2018-02-27 15:07, Eric Dumazet wrote:
On Tue, 2018-02-27 at 08:26 +0100, Marek Szyprowski wrote:
I've noticed that USBnet/ASIX AX88772B USB driver produces deplock kernel
warning ("inconsistent lock state") on Chromebook2 Peach-PIT board. No
special activity is needed to reproduce this issue, it happens almost
on every boot. ASIX USB ethernet is connected to XHCI USB host controller
on that board. Is it a known issue? Frankly I have no idea where to look
to fix it. The same adapter connected to EHCI ports on other boards based
on the same SoC works fine without any warnings.
Here are some more information from that board:
# lsusb
Bus 006 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 005 Device 002: ID 0b95:772b ASIX Electronics Corp. AX88772B
Bus 005 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 001 Device 002: ID 2232:1056 Silicon Motion
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
# lsusb -t
/: Bus 06.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M
/: Bus 05.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 480M
|__ Port 1: Dev 2, If 0, Class=Vendor Specific Class, Driver=asix, 480M
/: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M
/: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 480M
/: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=exynos-ohci/3p, 12M
/: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=exynos-ehci/3p, 480M
|__ Port 1: Dev 2, If 0, Class=Video, Driver=, 480M
|__ Port 1: Dev 2, If 1, Class=Video, Driver=, 480M
And the log with mentioned warning:
[ 17.768040]
[ 17.772239] WARNING: inconsistent lock state
[ 17.776511] 4.16.0-rc3-next-20180227-7-g876c53a7493c #453 Not tainted
[ 17.783329]
[ 17.787580] inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
[ 17.793607] swapper/0/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
[ 17.798751] (&syncp->seq#5){?.-.}, at: [<9b22e5f0>]
asix_rx_fixup_internal+0x188/0x288
[ 17.806790] {IN-HARDIRQ-W} state was registered at:
[ 17.811677] tx_complete+0x100/0x208
[ 17.815319] __usb_hcd_giveback_urb+0x60/0xf0
[ 17.819770] xhci_giveback_urb_in_irq+0xa8/0x240
[ 17.824469] xhci_td_cleanup+0xf4/0x16c
[ 17.828367] xhci_irq+0xe74/0x2240
[ 17.831827] usb_hcd_irq+0x24/0x38
[ 17.835343] __handle_irq_event_percpu+0x98/0x510
[ 17.840111] handle_irq_event_percpu+0x1c/0x58
[ 17.844623] handle_irq_event+0x38/0x5c
[ 17.848519] handle_fasteoi_irq+0xa4/0x138
[ 17.852681] generic_handle_irq+0x18/0x28
[ 17.856760] __handle_domain_irq+0x6c/0xe4
[ 17.860941] gic_handle_irq+0x54/0xa0
[ 17.864666] __irq_svc+0x70/0xb0
[ 17.867964] arch_cpu_idle+0x20/0x3c
[ 17.871578] arch_cpu_idle+0x20/0x3c
[ 17.875190] do_idle+0x144/0x218
[ 17.878468] cpu_startup_entry+0x18/0x1c
[ 17.882454] start_kernel+0x394/0x400
[ 17.886177] irq event stamp: 161912
[ 17.889616] hardirqs last enabled at (161912): [<7bedfacf>]
__netdev_alloc_skb+0xcc/0x140
[ 17.897893] hardirqs last disabled at (161911): []
__netdev_alloc_skb+0x94/0x140
[ 17.904903] exynos5-hsi2c 12ca.i2c: tx timeout
[ 17.906116] softirqs last enabled at (161904): [<387102ff>]
irq_enter+0x78/0x80
[ 17.906123] softirqs last disabled at (161905): []
irq_exit+0x134/0x158
[ 17.925722].
[ 17.925722] other info that might help us debug this:
[ 17.933435] Possible unsafe locking scenario:
[ 17.933435].
[ 17.940331] CPU0
[ 17.942488]
[ 17.944894] lock(&syncp->seq#5);
[ 17.948274]
[ 17.950847] lock(&syncp->seq#5);
[ 17.954386].
[ 17.954386] *** DEADLOCK ***
[ 17.954386].
[ 17.962422] no locks held by swapper/0/0.
[ 17.966011].
[ 17.966011] stack backtrace:
[ 17.971333] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.16.0-rc3-next-20180227-7-g876c53a7493c #453
[ 17.980312] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[ 17.986380] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[ 17.994128] [] (show_stack) from []
(dump_stack+0x90/0xc8)
[ 18.001339] [] (dump_stack) from []
(print_usage_bug+0x25c/0x2cc)
[ 18.009161] [] (print_usage_bug) from []
(mark_lock+0x290/0x698)
[ 18.014952] exynos5-hsi2c 12ca.i2c: tx timeout
[ 18.016899] [] (mark_lock) from []
(__lock_acquire+0x454/0x1850)
[ 18.029449] [] (__lock_acquire) from []
(lock_acquire+0xc8/0x2b8)
[ 18.037272] [] (lock_acquire) from []
(usbnet_skb_return+0x7c/0x1a0)
[ 18.045356] [] (usbnet_skb_return) from []
(asix_rx_fixup_internal+0x188/0x288)
[ 18.054420] [] (asix_rx_fixup_internal) from []
(usbnet_bh+0xf8/0x2e4)
[ 18.062694] [] (usbnet_bh) from []
(tasklet_action+0x8c/0x13c)
Re: inconsistent lock state with usbnet/asix usb ethernet and xhci
On Tue, 2018-02-27 at 08:26 +0100, Marek Szyprowski wrote:
> Hi
>
> I've noticed that USBnet/ASIX AX88772B USB driver produces deplock kernel
> warning ("inconsistent lock state") on Chromebook2 Peach-PIT board. No
> special activity is needed to reproduce this issue, it happens almost
> on every boot. ASIX USB ethernet is connected to XHCI USB host controller
> on that board. Is it a known issue? Frankly I have no idea where to look
> to fix it. The same adapter connected to EHCI ports on other boards based
> on the same SoC works fine without any warnings.
>
> Here are some more information from that board:
> # lsusb
> Bus 006 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 005 Device 002: ID 0b95:772b ASIX Electronics Corp. AX88772B
> Bus 005 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 001 Device 002: ID 2232:1056 Silicon Motion
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
>
> # lsusb -t
> /: Bus 06.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M
> /: Bus 05.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 480M
> |__ Port 1: Dev 2, If 0, Class=Vendor Specific Class, Driver=asix, 480M
> /: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M
> /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 480M
> /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=exynos-ohci/3p, 12M
> /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=exynos-ehci/3p, 480M
> |__ Port 1: Dev 2, If 0, Class=Video, Driver=, 480M
> |__ Port 1: Dev 2, If 1, Class=Video, Driver=, 480M
>
>
> And the log with mentioned warning:
>
> [ 17.768040] ====
> [ 17.772239] WARNING: inconsistent lock state
> [ 17.776511] 4.16.0-rc3-next-20180227-7-g876c53a7493c #453 Not tainted
> [ 17.783329]
> [ 17.787580] inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
> [ 17.793607] swapper/0/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
> [ 17.798751] (&syncp->seq#5){?.-.}, at: [<9b22e5f0>]
> asix_rx_fixup_internal+0x188/0x288
> [ 17.806790] {IN-HARDIRQ-W} state was registered at:
> [ 17.811677] tx_complete+0x100/0x208
> [ 17.815319] __usb_hcd_giveback_urb+0x60/0xf0
> [ 17.819770] xhci_giveback_urb_in_irq+0xa8/0x240
> [ 17.824469] xhci_td_cleanup+0xf4/0x16c
> [ 17.828367] xhci_irq+0xe74/0x2240
> [ 17.831827] usb_hcd_irq+0x24/0x38
> [ 17.835343] __handle_irq_event_percpu+0x98/0x510
> [ 17.840111] handle_irq_event_percpu+0x1c/0x58
> [ 17.844623] handle_irq_event+0x38/0x5c
> [ 17.848519] handle_fasteoi_irq+0xa4/0x138
> [ 17.852681] generic_handle_irq+0x18/0x28
> [ 17.856760] __handle_domain_irq+0x6c/0xe4
> [ 17.860941] gic_handle_irq+0x54/0xa0
> [ 17.864666] __irq_svc+0x70/0xb0
> [ 17.867964] arch_cpu_idle+0x20/0x3c
> [ 17.871578] arch_cpu_idle+0x20/0x3c
> [ 17.875190] do_idle+0x144/0x218
> [ 17.878468] cpu_startup_entry+0x18/0x1c
> [ 17.882454] start_kernel+0x394/0x400
> [ 17.886177] irq event stamp: 161912
> [ 17.889616] hardirqs last enabled at (161912): [<7bedfacf>]
> __netdev_alloc_skb+0xcc/0x140
> [ 17.897893] hardirqs last disabled at (161911): []
> __netdev_alloc_skb+0x94/0x140
> [ 17.904903] exynos5-hsi2c 12ca.i2c: tx timeout
> [ 17.906116] softirqs last enabled at (161904): [<387102ff>]
> irq_enter+0x78/0x80
> [ 17.906123] softirqs last disabled at (161905): []
> irq_exit+0x134/0x158
> [ 17.925722].
> [ 17.925722] other info that might help us debug this:
> [ 17.933435] Possible unsafe locking scenario:
> [ 17.933435].
> [ 17.940331] CPU0
> [ 17.942488]
> [ 17.944894] lock(&syncp->seq#5);
> [ 17.948274]
> [ 17.950847] lock(&syncp->seq#5);
> [ 17.954386].
> [ 17.954386] *** DEADLOCK ***
> [ 17.954386].
> [ 17.962422] no locks held by swapper/0/0.
> [ 17.966011].
> [ 17.966011] stack backtrace:
> [ 17.971333] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 4.16.0-rc3-next-20180227-7-g876c53a7493c #453
> [ 17.980312] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [ 17.986380] [] (unwind_backtrace) from []
> (show_stack+0x10/0x14)
> [ 17.994128] [] (show_stack) from []
> (dump_stack+0x90/0xc8)
> [ 18.001339] [] (dump_stack) from []
> (print_usage_bug+0x25c/0x2cc)
> [ 18.009161] [] (print_usage_bug) from []
> (mark_lock+0x290/0x698)
> [ 18.014952] exynos5-h
Re: [RFC PATCH] drivers: use 'depends on MFD_SYSCON' instead of 'select MFD_SYSCON'
On Tue, Feb 27, 2018 at 11:22 AM, Masahiro Yamada wrote: > 2018-02-27 18:03 GMT+09:00 Arnd Bergmann : >> On Tue, Feb 27, 2018 at 1:46 AM, Masahiro Yamada >> wrote: >>> But, we need to decide what the right solution is. >> >> I think for consistency, we should change the existing >> 'depends on MFD_SYSCON' to 'select MFD_SYSCON'. This >> matches what we do with REGMAP_MMIO. >> >> MFD_SYSCON is really a thin wrapper around REGMAP_MMIO, >> so I would keep using the same conventions here, even though >> we normally prefer to not 'select' any user-visible options. >> >> It might be possible to make MFD_SYSCON a silent symbol >> as well, but we'd have to make sure that all users select the symbol >> then. >> >> Arnd > > > If we agree, I can send the following three patches. > > > [1] Add "depends on HAS_IOMEM" > to all drivers selecting MFD_SYSCON > (Unmet dependencies will be fixed by this) > > [2] For consistency, convert existing "depends on MFD_SYSCON" > to "select MFD_SYSCON" + "depends on HAS_IOMEM" Those sound good. > [3] Change MFD_SYSCON to user-unconfigurable option. > But, for COMPILE_TEST, allow users to enable it independently. > Like follows: > > config MFD_SYSCON > bool "System Controller Register R/W Based on Regmap" if COMPILE_TEST > select REGMAP_MMIO > help > Select this option to enable accessing system control registers > via regmap. > > > Is this OK? I'm unsure about the third one, since we have drivers that can optionally use syscon, depending on the platform. With this change, any user that manually enabled syscon to use that with a driver that requires it on their platform but not on others will see a regression. If we do make MFD_SYSCON a silent option like that, we should remove the #else section in include/linux/mfd/syscon.h to force a build error, and require all drivers to 'select MFD_SYSCON' if they are able to use it. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usbip: vudc: fix null pointer dereference on udc->lock
On 02/26/2018 05:40 PM, Shuah Khan wrote:
On 02/22/2018 10:39 AM, Colin King wrote:
From: Colin Ian King
Currently the driver attempts to spin lock on udc->lock before a NULL
pointer check is performed on udc, hence there is a potential null
pointer dereference on udc->lock. Fix this by moving the null check
on udc before the lock occurs.
Fixes: ea6873a45a22 ("usbip: vudc: Add SysFS infrastructure for VUDC")
Signed-off-by: Colin Ian King
---
drivers/usb/usbip/vudc_sysfs.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c
index d86f7291..6dcd3ff655c3 100644
--- a/drivers/usb/usbip/vudc_sysfs.c
+++ b/drivers/usb/usbip/vudc_sysfs.c
@@ -105,10 +105,14 @@ static ssize_t usbip_sockfd_store(struct device *dev,
struct device_attribute *a
if (rv != 0)
return -EINVAL;
+ if (!udc) {
+ dev_err(dev, "no device");
+ return -ENODEV;
+ }
spin_lock_irqsave(&udc->lock, flags);
/* Don't export what we don't have */
- if (!udc || !udc->driver || !udc->pullup) {
- dev_err(dev, "no device or gadget not bound");
+ if (!udc->driver || !udc->pullup) {
+ dev_err(dev, "gadget not bound");
ret = -ENODEV;
goto unlock;
}
Thanks for the patch. Looks good to me.
Acked-by: Shuah Khan
Reviewed-by: Krzysztof Opasiak
but you could fix also a similar bug one one function above
(dev_desc_read()) ;)
Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cdc_ether: flag the Cinterion PLS8 modem by gemalto as WWAN
Am Dienstag, den 27.02.2018, 14:04 +0100 schrieb Bassem Boubaker: > The Cinterion PL8 is an LTE modem with 2 possible WWAN interfaces. > > The modem is controlled via AT commands through the exposed TTYs. > > AT^SWWAN write command can be used to activate or deactivate a WWAN > connection for a PDP context defined with AT+CGDCONT. UE supports > two WWAN adapter. Both WWAN adapters can be activated a the same time > > Signed-off-by: Bassem Boubaker Acked-by: Oliver Neukum -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] cdc_ether: flag the Cinterion PLS8 modem by gemalto as WWAN
The Cinterion PL8 is an LTE modem with 2 possible WWAN interfaces.
The modem is controlled via AT commands through the exposed TTYs.
AT^SWWAN write command can be used to activate or deactivate a WWAN
connection for a PDP context defined with AT+CGDCONT. UE supports
two WWAN adapter. Both WWAN adapters can be activated a the same time
Signed-off-by: Bassem Boubaker
---
drivers/net/usb/cdc_ether.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 05dca3e..fff4b13 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -896,6 +896,12 @@ static const struct usb_device_id products[] = {
USB_CDC_PROTO_NONE),
.driver_info = (unsigned long)&wwan_info,
}, {
+ /* Cinterion PLS8 modem by GEMALTO */
+ USB_DEVICE_AND_INTERFACE_INFO(0x1e2d, 0x0061, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET,
+ USB_CDC_PROTO_NONE),
+ .driver_info = (unsigned long)&wwan_info,
+}, {
USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_ETHERNET,
USB_CDC_PROTO_NONE),
.driver_info = (unsigned long) &cdc_info,
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cdc_ether: flag the Cinterion PLS8 modem by gemalto as WWAN
Am Dienstag, den 27.02.2018, 13:29 +0100 schrieb Bjørn Mork : > Bassem Boubaker writes: > > > > > +#define GEMALTO_VENDOR_ID 0x1e2d > > This is defined as CINTERION_VENDOR_ID in drivers/usb/serial/option.c. > > I have no idea which defintion is most correct, but I believe the macros > should be kept identical whatever you decide. Anything else is just > unnecessarily confusing. > > IMHO the company name tracking macros have grown beyond useful a long > time ago. They just make it harder to grep for the IDs without adding > any useful information whatsoever. And because you have cases like this > where the same number end up having different names, they sometimes hide > information which a plain number would have revealed. Hi, I concur. Could you redo the patch and just use a plain number? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cdc_ether: flag the Cinterion PLS8 modem by gemalto as WWAN
Bassem Boubaker writes: > +#define GEMALTO_VENDOR_ID0x1e2d This is defined as CINTERION_VENDOR_ID in drivers/usb/serial/option.c. I have no idea which defintion is most correct, but I believe the macros should be kept identical whatever you decide. Anything else is just unnecessarily confusing. IMHO the company name tracking macros have grown beyond useful a long time ago. They just make it harder to grep for the IDs without adding any useful information whatsoever. And because you have cases like this where the same number end up having different names, they sometimes hide information which a plain number would have revealed. Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 5/6] extcon: add possibility to get extcon device by OF node
Since extcon property is not allowed in DT, extcon subsystem requires
another way to get extcon device. Lets try the simplest approach - get
edev by of_node.
Signed-off-by: Andrzej Hajda
Acked-by: Chanwoo Choi
---
v5: function renamed to extcon_find_edev_by_node (Andy)
v2: changed label to follow local convention (Chanwoo)
---
drivers/extcon/extcon.c | 44 ++--
include/linux/extcon.h | 6 ++
2 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index cb38c2747684..8bff5fd18185 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1336,6 +1336,28 @@ void extcon_dev_unregister(struct extcon_dev *edev)
EXPORT_SYMBOL_GPL(extcon_dev_unregister);
#ifdef CONFIG_OF
+
+/*
+ * extcon_find_edev_by_node - Find the extcon device from devicetree.
+ * @node : OF node identifying edev
+ *
+ * Return the pointer of extcon device if success or ERR_PTR(err) if fail.
+ */
+struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
+{
+ struct extcon_dev *edev;
+
+ mutex_lock(&extcon_dev_list_lock);
+ list_for_each_entry(edev, &extcon_dev_list, entry)
+ if (edev->dev.parent && edev->dev.parent->of_node == node)
+ goto out;
+ edev = ERR_PTR(-EPROBE_DEFER);
+out:
+ mutex_unlock(&extcon_dev_list_lock);
+
+ return edev;
+}
+
/*
* extcon_get_edev_by_phandle - Get the extcon device from devicetree.
* @dev: the instance to the given device
@@ -1363,25 +1385,27 @@ struct extcon_dev *extcon_get_edev_by_phandle(struct
device *dev, int index)
return ERR_PTR(-ENODEV);
}
- mutex_lock(&extcon_dev_list_lock);
- list_for_each_entry(edev, &extcon_dev_list, entry) {
- if (edev->dev.parent && edev->dev.parent->of_node == node) {
- mutex_unlock(&extcon_dev_list_lock);
- of_node_put(node);
- return edev;
- }
- }
- mutex_unlock(&extcon_dev_list_lock);
+ edev = extcon_find_edev_by_node(node);
of_node_put(node);
- return ERR_PTR(-EPROBE_DEFER);
+ return edev;
}
+
#else
+
+struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev, int index)
{
return ERR_PTR(-ENOSYS);
}
+
#endif /* CONFIG_OF */
+
+EXPORT_SYMBOL_GPL(extcon_find_edev_by_node);
EXPORT_SYMBOL_GPL(extcon_get_edev_by_phandle);
/**
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 6d94e82c8ad9..7f033b1ea568 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -230,6 +230,7 @@ extern void devm_extcon_unregister_notifier_all(struct
device *dev,
* Following APIs get the extcon_dev from devicetree or by through extcon name.
*/
extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
+extern struct extcon_dev *extcon_find_edev_by_node(struct device_node *node);
extern struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev,
int index);
@@ -283,6 +284,11 @@ static inline struct extcon_dev
*extcon_get_extcon_dev(const char *extcon_name)
return ERR_PTR(-ENODEV);
}
+static inline struct extcon_dev *extcon_find_edev_by_node(struct device_node
*node)
+{
+ return ERR_PTR(-ENODEV);
+}
+
static inline struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev,
int index)
{
--
2.16.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL
On 27.02.2018 12:08, Chanwoo Choi wrote:
> Hi,
>
> On 2018년 02월 27일 16:11, Andrzej Hajda wrote:
>> From: Maciej Purski
>>
>> Currently MHL chip must be turned on permanently to detect MHL cable. It
>> duplicates micro-USB controller's (MUIC) functionality and consumes
>> unnecessary power. Lets use extcon attached to MUIC to enable MHL chip
>> only if it detects MHL cable.
>>
>> Signed-off-by: Maciej Purski
>> Signed-off-by: Andrzej Hajda
>> ---
>> v5: updated extcon API
>>
>> This is rework of the patch by Maciej with following changes:
>> - use micro-USB port bindings to get extcon, instead of extcon property,
>> - fixed remove sequence,
>> - fixed extcon get state logic.
>>
>> Code finding extcon node is hacky IMO, I guess ultimately it should be done
>> via some framework (maybe even extcon), or at least via helper, I hope it
>> can stay as is until the proper solution will be merged.
>>
>> Signed-off-by: Andrzej Hajda
>> ---
>> drivers/gpu/drm/bridge/sil-sii8620.c | 97
>> ++--
>> 1 file changed, 94 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
>> b/drivers/gpu/drm/bridge/sil-sii8620.c
>> index 9e785b8e0ea2..62b0adabcac2 100644
>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>> @@ -17,6 +17,7 @@
>>
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -25,6 +26,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>>
>> @@ -81,6 +83,10 @@ struct sii8620 {
>> struct edid *edid;
>> unsigned int gen2_write_burst:1;
>> enum sii8620_mt_state mt_state;
>> +struct extcon_dev *extcon;
>> +struct notifier_block extcon_nb;
>> +struct work_struct extcon_wq;
>> +int cable_state;
>> struct list_head mt_queue;
>> struct {
>> int r_size;
>> @@ -2175,6 +2181,77 @@ static void sii8620_init_rcp_input_dev(struct sii8620
>> *ctx)
>> ctx->rc_dev = rc_dev;
>> }
>>
>> +static void sii8620_cable_out(struct sii8620 *ctx)
>> +{
>> +disable_irq(to_i2c_client(ctx->dev)->irq);
>> +sii8620_hw_off(ctx);
>> +}
>> +
>> +static void sii8620_extcon_work(struct work_struct *work)
>> +{
>> +struct sii8620 *ctx =
>> +container_of(work, struct sii8620, extcon_wq);
>> +int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
>> +
>> +if (state == ctx->cable_state)
>> +return;
>> +
>> +ctx->cable_state = state;
>> +
>> +if (state > 0)
>> +sii8620_cable_in(ctx);
>> +else
>> +sii8620_cable_out(ctx);
>> +}
>> +
>> +static int sii8620_extcon_notifier(struct notifier_block *self,
>> +unsigned long event, void *ptr)
>> +{
>> +struct sii8620 *ctx =
>> +container_of(self, struct sii8620, extcon_nb);
>> +
>> +schedule_work(&ctx->extcon_wq);
>> +
>> +return NOTIFY_DONE;
>> +}
>> +
>> +static int sii8620_extcon_init(struct sii8620 *ctx)
>> +{
>> +struct extcon_dev *edev;
>> +struct device_node *musb, *muic;
>> +int ret;
>> +
>> +/* get micro-USB connector node */
>> +musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
>> +/* next get micro-USB Interface Controller node */
>> +muic = of_get_next_parent(musb);
>> +
>> +if (!muic) {
>> +dev_info(ctx->dev, "no extcon found, switching to 'always on'
>> mode\n");
>> +return 0;
>> +}
>> +
>> +edev = extcon_find_edev_by_node(muic);
>> +of_node_put(muic);
>> +if (IS_ERR(edev)) {
>> +if (PTR_ERR(edev) == -EPROBE_DEFER)
>> +return -EPROBE_DEFER;
>> +dev_err(ctx->dev, "Invalid or missing extcon\n");
>> +return PTR_ERR(edev);
>> +}
>> +
>> +ctx->extcon = edev;
>> +ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
>> +INIT_WORK(&ctx->extcon_wq, sii8620_extcon_work);
>> +ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
> You better to use devm_extcon_register_notifier().
With devm version I risk that in case of device unbind notification will
be called after .remove callback, it seems to me quite dangerous. Or am
I missing something?
Regards
Andrzej
>
>> +if (ret) {
>> +dev_err(ctx->dev, "failed to register notifier for MHL\n");
>> +return ret;
>> +}
>> +
>> +return 0;
>> +}
>> +
>> static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge)
>> {
>> return container_of(bridge, struct sii8620, bridge);
>> @@ -2307,13 +2384,20 @@ static int sii8620_probe(struct i2c_client *client,
>> if (ret)
>> return ret;
>>
>> +ret = sii8620_extcon_init(ctx);
>> +if (ret < 0) {
>> +dev_err(ctx->dev, "failed to initialize EXTCON\n");
>> +return ret;
>> +}
>> +
>> i2c_set_clientdata(client, ctx);
>>
>> ctx->bridge.funcs = &sii8620_bridge_funcs;
>
[PATCH] cdc_ether: flag the Cinterion PLS8 modem by gemalto as WWAN
The Cinterion PL8 is an LTE modem with 2 possible WWAN interfaces.
The modem is controlled via AT commands through the exposed TTYs.
AT^SWWAN write command can be used to activate or deactivate a WWAN
connection for a PDP context defined with AT+CGDCONT. UE supports
two WWAN adapter. Both WWAN adapters can be activated a the same time
Signed-off-by: Bassem Boubaker
---
drivers/net/usb/cdc_ether.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 05dca3e..65621fe 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -562,6 +562,7 @@ static const struct driver_info wwan_info = {
#define MICROSOFT_VENDOR_ID0x045e
#define UBLOX_VENDOR_ID0x1546
#define TPLINK_VENDOR_ID 0x2357
+#define GEMALTO_VENDOR_ID 0x1e2d
static const struct usb_device_id products[] = {
/* BLACKLIST !!
@@ -896,6 +897,12 @@ static const struct usb_device_id products[] = {
USB_CDC_PROTO_NONE),
.driver_info = (unsigned long)&wwan_info,
}, {
+ /* Cinterion PLS8 modem by GEMALTO */
+ USB_DEVICE_AND_INTERFACE_INFO(GEMALTO_VENDOR_ID, 0x0061, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET,
+ USB_CDC_PROTO_NONE),
+ .driver_info = (unsigned long)&wwan_info,
+}, {
USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_ETHERNET,
USB_CDC_PROTO_NONE),
.driver_info = (unsigned long) &cdc_info,
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] usb: dwc3: add dual role support using OTG block
This is useful on platforms (e.g. TI AM437x) that don't
have ID available on a GPIO but do have the OTG block.
We can obtain the ID state via the OTG block and use it
for dual-role switching.
Signed-off-by: Roger Quadros
---
drivers/usb/dwc3/core.c | 67 ++-
drivers/usb/dwc3/core.h | 29 +++
drivers/usb/dwc3/drd.c | 489 ++--
3 files changed, 557 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index df4569d..397a4d4 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -89,10 +89,7 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc)
return 0;
}
-static void dwc3_event_buffers_cleanup(struct dwc3 *dwc);
-static int dwc3_event_buffers_setup(struct dwc3 *dwc);
-
-static void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
+void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
{
u32 reg;
@@ -110,16 +107,19 @@ static void __dwc3_set_mode(struct work_struct *work)
unsigned long flags;
int ret;
- if (!dwc->desired_dr_role)
+ if (dwc->dr_mode != USB_DR_MODE_OTG)
return;
- if (dwc->desired_dr_role == dwc->current_dr_role)
+ if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_OTG)
+ dwc3_otg_update(dwc, 0);
+
+ if (!dwc->desired_dr_role)
return;
- if (dwc->dr_mode != USB_DR_MODE_OTG)
+ if (dwc->desired_dr_role == dwc->current_dr_role)
return;
- if (dwc->desired_dr_role == DWC3_GCTL_PRTCAP_OTG)
+ if (dwc->desired_dr_role == DWC3_GCTL_PRTCAP_OTG && dwc->edev)
return;
switch (dwc->current_dr_role) {
@@ -130,6 +130,13 @@ static void __dwc3_set_mode(struct work_struct *work)
dwc3_gadget_exit(dwc);
dwc3_event_buffers_cleanup(dwc);
break;
+ case DWC3_GCTL_PRTCAP_OTG:
+ dwc3_otg_exit(dwc);
+ spin_lock_irqsave(&dwc->lock, flags);
+ dwc->desired_otg_role = DWC3_OTG_ROLE_IDLE;
+ spin_unlock_irqrestore(&dwc->lock, flags);
+ dwc3_otg_update(dwc, 1);
+ break;
default:
break;
}
@@ -165,9 +172,14 @@ static void __dwc3_set_mode(struct work_struct *work)
if (ret)
dev_err(dwc->dev, "failed to initialize peripheral\n");
break;
+ case DWC3_GCTL_PRTCAP_OTG:
+ dwc3_otg_init(dwc);
+ dwc3_otg_update(dwc, 0);
+ break;
default:
break;
}
+
}
void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
@@ -351,7 +363,7 @@ static int dwc3_alloc_event_buffers(struct dwc3 *dwc,
unsigned length)
*
* Returns 0 on success otherwise negative errno.
*/
-static int dwc3_event_buffers_setup(struct dwc3 *dwc)
+int dwc3_event_buffers_setup(struct dwc3 *dwc)
{
struct dwc3_event_buffer*evt;
@@ -368,7 +380,7 @@ static int dwc3_event_buffers_setup(struct dwc3 *dwc)
return 0;
}
-static void dwc3_event_buffers_cleanup(struct dwc3 *dwc)
+void dwc3_event_buffers_cleanup(struct dwc3 *dwc)
{
struct dwc3_event_buffer*evt;
@@ -1329,6 +1341,20 @@ static int dwc3_suspend_common(struct dwc3 *dwc,
pm_message_t msg)
if (!PMSG_IS_AUTO(msg))
dwc3_core_exit(dwc);
break;
+ case DWC3_GCTL_PRTCAP_OTG:
+ /* do nothing during runtime_suspend */
+ if (PMSG_IS_AUTO(msg))
+ break;
+
+ if (dwc->current_otg_role == DWC3_OTG_ROLE_DEVICE) {
+ spin_lock_irqsave(&dwc->lock, flags);
+ dwc3_gadget_suspend(dwc);
+ spin_unlock_irqrestore(&dwc->lock, flags);
+ }
+
+ dwc3_otg_exit(dwc);
+ dwc3_core_exit(dwc);
+ break;
default:
/* do nothing */
break;
@@ -1360,6 +1386,27 @@ static int dwc3_resume_common(struct dwc3 *dwc,
pm_message_t msg)
return ret;
}
break;
+ case DWC3_GCTL_PRTCAP_OTG:
+ /* nothing to do on runtime_resume */
+ if (PMSG_IS_AUTO(msg))
+ break;
+
+ ret = dwc3_core_init(dwc);
+ if (ret)
+ return ret;
+
+ dwc3_set_prtcap(dwc, dwc->current_dr_role);
+
+ dwc3_otg_init(dwc);
+ if (dwc->current_otg_role == DWC3_OTG_ROLE_HOST) {
+ dwc3_otg_host_init(dwc);
+ } else if (dwc->current_otg_role == DWC3_OTG_ROLE_DEVICE) {
+ spin_lock_irqsave(&dwc->lock, flags);
+ dwc3_gadget_resume(dwc);
+ spin_unlock_irqrestore(&dwc->lock, flags);
+ }
+
+ break;
[PATCH v2 1/2] usb: dwc3: core.h: add some register definitions
Add OTG and GHWPARAMS6 register definitions Signed-off-by: Roger Quadros --- drivers/usb/dwc3/core.h | 82 + 1 file changed, 82 insertions(+) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 860d2bc..0d4c698 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -201,6 +201,15 @@ #define DWC3_GUCTL1_TX_IPGAP_LINECHECK_DIS BIT(28) #define DWC3_GUCTL1_DEV_L1_EXIT_BY_HW BIT(24) +/* Global Status Register */ +#define DWC3_GSTS_OTG_IP BIT(10) +#define DWC3_GSTS_BC_IPBIT(9) +#define DWC3_GSTS_ADP_IP BIT(8) +#define DWC3_GSTS_HOST_IP BIT(7) +#define DWC3_GSTS_DEVICE_IPBIT(6) +#define DWC3_GSTS_CSR_TIMEOUT BIT(5) +#define DWC3_GSTS_BUS_ERR_ADDR_VLD BIT(4) + /* Global USB2 PHY Configuration Register */ #define DWC3_GUSB2PHYCFG_PHYSOFTRSTBIT(31) #define DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS BIT(30) @@ -286,6 +295,11 @@ #define DWC3_MAX_HIBER_SCRATCHBUFS 15 /* Global HWPARAMS6 Register */ +#define DWC3_GHWPARAMS6_BCSUPPORT BIT(14) +#define DWC3_GHWPARAMS6_OTG3SUPPORTBIT(13) +#define DWC3_GHWPARAMS6_ADPSUPPORT BIT(12) +#define DWC3_GHWPARAMS6_HNPSUPPORT BIT(11) +#define DWC3_GHWPARAMS6_SRPSUPPORT BIT(10) #define DWC3_GHWPARAMS6_EN_FPGABIT(7) /* Global HWPARAMS7 Register */ @@ -467,6 +481,74 @@ #define DWC3_DEV_IMOD_INTERVAL_SHIFT 0 #define DWC3_DEV_IMOD_INTERVAL_MASK(0x << 0) +/* OTG Configuration Register */ +#define DWC3_OCFG_DISPWRCUTTOFFBIT(5) +#define DWC3_OCFG_HIBDISMASK BIT(4) +#define DWC3_OCFG_SFTRSTMASK BIT(3) +#define DWC3_OCFG_OTGVERSION BIT(2) +#define DWC3_OCFG_HNPCAP BIT(1) +#define DWC3_OCFG_SRPCAP BIT(0) + +/* OTG CTL Register */ +#define DWC3_OCTL_OTG3GOERRBIT(7) +#define DWC3_OCTL_PERIMODE BIT(6) +#define DWC3_OCTL_PRTPWRCTLBIT(5) +#define DWC3_OCTL_HNPREQ BIT(4) +#define DWC3_OCTL_SESREQ BIT(3) +#define DWC3_OCTL_TERMSELIDPULSE BIT(2) +#define DWC3_OCTL_DEVSETHNPEN BIT(1) +#define DWC3_OCTL_HSTSETHNPEN BIT(0) + +/* OTG Event Register */ +#define DWC3_OEVT_DEVICEMODE BIT(31) +#define DWC3_OEVT_XHCIRUNSTPSETBIT(27) +#define DWC3_OEVT_DEVRUNSTPSET BIT(26) +#define DWC3_OEVT_HIBENTRY BIT(25) +#define DWC3_OEVT_CONIDSTSCHNG BIT(24) +#define DWC3_OEVT_HRRCONFNOTIF BIT(23) +#define DWC3_OEVT_HRRINITNOTIF BIT(22) +#define DWC3_OEVT_ADEVIDLE BIT(21) +#define DWC3_OEVT_ADEVBHOSTEND BIT(20) +#define DWC3_OEVT_ADEVHOST BIT(19) +#define DWC3_OEVT_ADEVHNPCHNG BIT(18) +#define DWC3_OEVT_ADEVSRPDET BIT(17) +#define DWC3_OEVT_ADEVSESSENDDET BIT(16) +#define DWC3_OEVT_BDEVBHOSTEND BIT(11) +#define DWC3_OEVT_BDEVHNPCHNG BIT(10) +#define DWC3_OEVT_BDEVSESSVLDDET BIT(9) +#define DWC3_OEVT_BDEVVBUSCHNG BIT(8) +#define DWC3_OEVT_BSESSVLD BIT(3) +#define DWC3_OEVT_HSTNEGSTSBIT(2) +#define DWC3_OEVT_SESREQSTSBIT(1) +#define DWC3_OEVT_ERRORBIT(0) + +/* OTG Event Enable Register */ +#define DWC3_OEVTEN_XHCIRUNSTPSETENBIT(27) +#define DWC3_OEVTEN_DEVRUNSTPSETEN BIT(26) +#define DWC3_OEVTEN_HIBENTRYEN BIT(25) +#define DWC3_OEVTEN_CONIDSTSCHNGEN BIT(24) +#define DWC3_OEVTEN_HRRCONFNOTIFEN BIT(23) +#define DWC3_OEVTEN_HRRINITNOTIFEN BIT(22) +#define DWC3_OEVTEN_ADEVIDLEEN BIT(21) +#define DWC3_OEVTEN_ADEVBHOSTENDEN BIT(20) +#define DWC3_OEVTEN_ADEVHOSTEN BIT(19) +#define DWC3_OEVTEN_ADEVHNPCHNGEN BIT(18) +#define DWC3_OEVTEN_ADEVSRPDETEN BIT(17) +#define DWC3_OEVTEN_ADEVSESSENDDETEN BIT(16) +#define DWC3_OEVTEN_BDEVBHOSTENDEN BIT(11) +#define DWC3_OEVTEN_BDEVHNPCHNGEN BIT(10) +#define DWC3_OEVTEN_BDEVSESSVLDDETEN BIT(9) +#define DWC3_OEVTEN_BDEVVBUSCHNGEN BIT(8) + +/* OTG Status Register */ +#define DWC3_OSTS_DEVRUNSTPBIT(13) +#define DWC3_OSTS_XHCIRUNSTP BIT(12) +#define DWC3_OSTS_PERIPHERALSTATE BIT(4) +#define DWC3_OSTS_XHCIPRTPOWER BIT(3) +#define DWC3_OSTS_BSESVLD BIT(2) +#define DWC3_OSTS_VBUSVLD BIT(1) +#define DWC3_OSTS_CONIDSTS BIT(0) + /* Structures */ struct dwc3_trb; -- cheers, -roger Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/2] usb: dwc3: Add dual-role support using OTG core
Hi Felipe, Some platforms (e.g. TI's AM437x) don't have USB ID pin state available over GPIO/extcon but need to rely on the DWC3 core's OTG block to get the ID pin state instead. This series implements simple dual-role functionality using DWC3's OTG block. Debugfs 'mode' override is also functional so user can switch between "otg", "host" or "device" modes for debug. Although system suspend/resume isn't working yet in mainline for AM437x, I've tested this series for system suspend/resume using a local tree. Roger Quadros (2): usb: dwc3: core.h: add some register definitions usb: dwc3: add dual role support using OTG block drivers/usb/dwc3/core.c | 67 ++- drivers/usb/dwc3/core.h | 111 +++ drivers/usb/dwc3/drd.c | 489 ++-- 3 files changed, 639 insertions(+), 28 deletions(-) -- cheers, -roger Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc3: Prevent indefinite sleep in _dwc3_set_mode during suspend/resume
In the following test we get stuck by sleeping forever in _dwc3_set_mode()
after which dual-role switching doesn't work.
On dra7-evm's dual-role port,
- Load g_zero gadget driver and enumerate to host
- suspend to mem
- disconnect USB cable to host and connect otg cable with Pen drive in it.
- resume system
- we sleep indefinitely in _dwc3_set_mode due to.
dwc3_gadget_exit()->usb_del_gadget_udc()->udc_stop()->
dwc3_gadget_stop()->wait_event_lock_irq()
Let's clear the DWC3_EP_END_TRANSFER_PENDING flag on all endpoints
so we don't wait in dwc3_gadget_stop().
Signed-off-by: Roger Quadros
---
drivers/usb/dwc3/gadget.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 2bda4eb..0a360da 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3273,6 +3273,20 @@ int dwc3_gadget_init(struct dwc3 *dwc)
void dwc3_gadget_exit(struct dwc3 *dwc)
{
+ int epnum;
+ unsigned long flags;
+
+ spin_lock_irqsave(&dwc->lock, flags);
+ for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
+ struct dwc3_ep *dep = dwc->eps[epnum];
+
+ if (!dep)
+ continue;
+
+ dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
+ }
+ spin_unlock_irqrestore(&dwc->lock, flags);
+
usb_del_gadget_udc(&dwc->gadget);
dwc3_gadget_free_endpoints(dwc);
dma_free_coherent(dwc->sysdev, DWC3_BOUNCE_SIZE, dwc->bounce,
--
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL
Hi,
On 2018년 02월 27일 16:11, Andrzej Hajda wrote:
> From: Maciej Purski
>
> Currently MHL chip must be turned on permanently to detect MHL cable. It
> duplicates micro-USB controller's (MUIC) functionality and consumes
> unnecessary power. Lets use extcon attached to MUIC to enable MHL chip
> only if it detects MHL cable.
>
> Signed-off-by: Maciej Purski
> Signed-off-by: Andrzej Hajda
> ---
> v5: updated extcon API
>
> This is rework of the patch by Maciej with following changes:
> - use micro-USB port bindings to get extcon, instead of extcon property,
> - fixed remove sequence,
> - fixed extcon get state logic.
>
> Code finding extcon node is hacky IMO, I guess ultimately it should be done
> via some framework (maybe even extcon), or at least via helper, I hope it
> can stay as is until the proper solution will be merged.
>
> Signed-off-by: Andrzej Hajda
> ---
> drivers/gpu/drm/bridge/sil-sii8620.c | 97
> ++--
> 1 file changed, 94 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
> b/drivers/gpu/drm/bridge/sil-sii8620.c
> index 9e785b8e0ea2..62b0adabcac2 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -17,6 +17,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -25,6 +26,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -81,6 +83,10 @@ struct sii8620 {
> struct edid *edid;
> unsigned int gen2_write_burst:1;
> enum sii8620_mt_state mt_state;
> + struct extcon_dev *extcon;
> + struct notifier_block extcon_nb;
> + struct work_struct extcon_wq;
> + int cable_state;
> struct list_head mt_queue;
> struct {
> int r_size;
> @@ -2175,6 +2181,77 @@ static void sii8620_init_rcp_input_dev(struct sii8620
> *ctx)
> ctx->rc_dev = rc_dev;
> }
>
> +static void sii8620_cable_out(struct sii8620 *ctx)
> +{
> + disable_irq(to_i2c_client(ctx->dev)->irq);
> + sii8620_hw_off(ctx);
> +}
> +
> +static void sii8620_extcon_work(struct work_struct *work)
> +{
> + struct sii8620 *ctx =
> + container_of(work, struct sii8620, extcon_wq);
> + int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
> +
> + if (state == ctx->cable_state)
> + return;
> +
> + ctx->cable_state = state;
> +
> + if (state > 0)
> + sii8620_cable_in(ctx);
> + else
> + sii8620_cable_out(ctx);
> +}
> +
> +static int sii8620_extcon_notifier(struct notifier_block *self,
> + unsigned long event, void *ptr)
> +{
> + struct sii8620 *ctx =
> + container_of(self, struct sii8620, extcon_nb);
> +
> + schedule_work(&ctx->extcon_wq);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int sii8620_extcon_init(struct sii8620 *ctx)
> +{
> + struct extcon_dev *edev;
> + struct device_node *musb, *muic;
> + int ret;
> +
> + /* get micro-USB connector node */
> + musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
> + /* next get micro-USB Interface Controller node */
> + muic = of_get_next_parent(musb);
> +
> + if (!muic) {
> + dev_info(ctx->dev, "no extcon found, switching to 'always on'
> mode\n");
> + return 0;
> + }
> +
> + edev = extcon_find_edev_by_node(muic);
> + of_node_put(muic);
> + if (IS_ERR(edev)) {
> + if (PTR_ERR(edev) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + dev_err(ctx->dev, "Invalid or missing extcon\n");
> + return PTR_ERR(edev);
> + }
> +
> + ctx->extcon = edev;
> + ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
> + INIT_WORK(&ctx->extcon_wq, sii8620_extcon_work);
> + ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
You better to use devm_extcon_register_notifier().
> + if (ret) {
> + dev_err(ctx->dev, "failed to register notifier for MHL\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge)
> {
> return container_of(bridge, struct sii8620, bridge);
> @@ -2307,13 +2384,20 @@ static int sii8620_probe(struct i2c_client *client,
> if (ret)
> return ret;
>
> + ret = sii8620_extcon_init(ctx);
> + if (ret < 0) {
> + dev_err(ctx->dev, "failed to initialize EXTCON\n");
> + return ret;
> + }
> +
> i2c_set_clientdata(client, ctx);
>
> ctx->bridge.funcs = &sii8620_bridge_funcs;
> ctx->bridge.of_node = dev->of_node;
> drm_bridge_add(&ctx->bridge);
>
> - sii8620_cable_in(ctx);
> + if (!ctx->extcon)
> + sii8620_cable_in(ctx);
>
> return 0;
> }
> @@ -2322,8 +2406,15 @@ static int sii8620_remove(struct i2c_client *client)
> {
> struct sii8620
Re: [PATCH v5 5/6] extcon: add possibility to get extcon device by OF node
Hi,
On 2018년 02월 27일 16:11, Andrzej Hajda wrote:
> Since extcon property is not allowed in DT, extcon subsystem requires
> another way to get extcon device. Lets try the simplest approach - get
> edev by of_node.
>
> Signed-off-by: Andrzej Hajda
> Acked-by: Chanwoo Choi
> ---
> v5: function renamed to extcon_find_edev_by_node (Andy)
> v2: changed label to follow local convention (Chanwoo)
> ---
> drivers/extcon/extcon.c | 44 ++--
> include/linux/extcon.h | 6 ++
> 2 files changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index cb38c2747684..47a5ca9eb86d 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -1336,6 +1336,28 @@ void extcon_dev_unregister(struct extcon_dev *edev)
> EXPORT_SYMBOL_GPL(extcon_dev_unregister);
>
> #ifdef CONFIG_OF
> +
> +/*
> + * extcon_find_edev_by_node - Find the extcon device from devicetree.
> + * @node : OF node identyfying edev
s/identyfying/identifying
> + *
> + * Return the pointer of extcon device if success or ERR_PTR(err) if fail.
> + */
> +struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
> +{
> + struct extcon_dev *edev;
> +
> + mutex_lock(&extcon_dev_list_lock);
> + list_for_each_entry(edev, &extcon_dev_list, entry)
> + if (edev->dev.parent && edev->dev.parent->of_node == node)
> + goto out;
> + edev = ERR_PTR(-EPROBE_DEFER);
> +out:
> + mutex_unlock(&extcon_dev_list_lock);
> +
> + return edev;
> +}
> +
> /*
> * extcon_get_edev_by_phandle - Get the extcon device from devicetree.
> * @dev : the instance to the given device
> @@ -1363,25 +1385,27 @@ struct extcon_dev *extcon_get_edev_by_phandle(struct
> device *dev, int index)
> return ERR_PTR(-ENODEV);
> }
>
> - mutex_lock(&extcon_dev_list_lock);
> - list_for_each_entry(edev, &extcon_dev_list, entry) {
> - if (edev->dev.parent && edev->dev.parent->of_node == node) {
> - mutex_unlock(&extcon_dev_list_lock);
> - of_node_put(node);
> - return edev;
> - }
> - }
> - mutex_unlock(&extcon_dev_list_lock);
> + edev = extcon_find_edev_by_node(node);
> of_node_put(node);
>
> - return ERR_PTR(-EPROBE_DEFER);
> + return edev;
> }
> +
> #else
> +
> +struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
> +
> struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev, int index)
> {
> return ERR_PTR(-ENOSYS);
> }
> +
> #endif /* CONFIG_OF */
> +
> +EXPORT_SYMBOL_GPL(extcon_find_edev_by_node);
> EXPORT_SYMBOL_GPL(extcon_get_edev_by_phandle);
>
> /**
> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
> index 6d94e82c8ad9..7f033b1ea568 100644
> --- a/include/linux/extcon.h
> +++ b/include/linux/extcon.h
> @@ -230,6 +230,7 @@ extern void devm_extcon_unregister_notifier_all(struct
> device *dev,
> * Following APIs get the extcon_dev from devicetree or by through extcon
> name.
> */
> extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
> +extern struct extcon_dev *extcon_find_edev_by_node(struct device_node *node);
> extern struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev,
>int index);
>
> @@ -283,6 +284,11 @@ static inline struct extcon_dev
> *extcon_get_extcon_dev(const char *extcon_name)
> return ERR_PTR(-ENODEV);
> }
>
> +static inline struct extcon_dev *extcon_find_edev_by_node(struct device_node
> *node)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +
> static inline struct extcon_dev *extcon_get_edev_by_phandle(struct device
> *dev,
> int index)
> {
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/12] staging: typec: tcpci: support port config passed via dt
Hi,
On Mon, Feb 26, 2018 at 02:30:53PM +, Jun Li wrote:
> > > + child = of_get_child_by_name(tcpci->dev->of_node, "connector");
> > > + if (!child) {
> > > + dev_err(tcpci->dev, "failed to get connector node.\n");
> > > + return -EINVAL;
> > > + }
> >
> > Why do you need separate child node for the connector? You will always
> > have only one connector per tcpc, i.e. the tcpci already represents the
> > connector and all its capabilities.
> >
> This is my original idea, my understanding is Rob expects those properties
> should
> apply for a common usb connector node[1], that way I need add a child node
> for it,
> sorry I didn't make the dt-binding patches come first in this series, please
> see
> patch 10,11.
>
> [1] https://patchwork.kernel.org/patch/10231447/
But was the idea really to put properties like the TCPC capabilities
under the usb connector child node? That will force us to extract
the same properties in two different methods in every USB Type-C
driver. One extracting them from DT, and another from other FW
interfaces and build-in properties.
To avoid that, let's just expect to get these properties in the node
for tcpc, not the usb connector child.
Thanks,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: inconsistent lock state with usbnet/asix usb ethernet and xhci
Hi Oliver,
On 2018-02-27 11:37, Oliver Neukum wrote:
Am Dienstag, den 27.02.2018, 08:26 +0100 schrieb Marek Szyprowski:
I've noticed that USBnet/ASIX AX88772B USB driver produces deplock kernel
warning ("inconsistent lock state") on Chromebook2 Peach-PIT board. No
Is that 32 bit?
Yes. ARM 32bit. Exynos5420 SoC (4 x CortexA15 + 4 x CortexA7 big.LITTLE
CPUs).
special activity is needed to reproduce this issue, it happens almost
on every boot. ASIX USB ethernet is connected to XHCI USB host controller
on that board. Is it a known issue? Frankly I have no idea where to look
No, it is not known.
to fix it. The same adapter connected to EHCI ports on other boards based
on the same SoC works fine without any warnings.
Odd.
And the log with mentioned warning:
[ 17.768040]
[ 17.772239] WARNING: inconsistent lock state
[ 17.776511] 4.16.0-rc3-next-20180227-7-g876c53a7493c #453 Not tainted
[ 17.783329]
[ 17.787580] inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
[ 17.793607] swapper/0/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
[ 17.798751] (&syncp->seq#5){?.-.}, at: [<9b22e5f0>]
asix_rx_fixup_internal+0x188/0x288
Looks like this triggers (in usbnet):
u64_stats_update_begin(&stats64->syncp);
stats64->rx_packets++;
stats64->rx_bytes += skb->len;
u64_stats_update_end(&stats64->syncp);
That I considered to be called under lock.
Could you comment this out for testing?
Yes, commenting this out indeed hides the deplock warning.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc3: prevent setting PRTCAP to OTG from debugfs
We don't support PRTCAP == OTG yet, so prevent user from
setting it via debugfs.
Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly")
Cc: # v4.12+
Signed-off-by: Roger Quadros
---
drivers/usb/dwc3/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index e94bf91..df4569d 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -119,6 +119,9 @@ static void __dwc3_set_mode(struct work_struct *work)
if (dwc->dr_mode != USB_DR_MODE_OTG)
return;
+ if (dwc->desired_dr_role == DWC3_GCTL_PRTCAP_OTG)
+ return;
+
switch (dwc->current_dr_role) {
case DWC3_GCTL_PRTCAP_HOST:
dwc3_host_exit(dwc);
--
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc3: Fix lock-up on ID change during system suspend/resume
To reproduce the lock up do the following
- connect otg host adapter and a USB device to the dual-role port
so that it is in host mode.
- suspend to mem.
- disconnect otg adapter.
- resume the system.
If we call dwc3_host_exit() before tasks are thawed
xhci_plat_remove() seems to lock up at the second usb_remove_hcd() call.
To work around this we queue the _dwc3_set_mode() work on
the system_freezable_wq.
Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly")
Cc: # v4.12+
Suggested-by: Manu Gautam
Signed-off-by: Roger Quadros
---
drivers/usb/dwc3/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index f1d838a..e94bf91 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -175,7 +175,7 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
dwc->desired_dr_role = mode;
spin_unlock_irqrestore(&dwc->lock, flags);
- queue_work(system_power_efficient_wq, &dwc->drd_work);
+ queue_work(system_freezable_wq, &dwc->drd_work);
}
u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type)
--
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: inconsistent lock state with usbnet/asix usb ethernet and xhci
Am Dienstag, den 27.02.2018, 08:26 +0100 schrieb Marek Szyprowski:
Hi,
> I've noticed that USBnet/ASIX AX88772B USB driver produces deplock kernel
> warning ("inconsistent lock state") on Chromebook2 Peach-PIT board. No
Is that 32 bit?
> special activity is needed to reproduce this issue, it happens almost
> on every boot. ASIX USB ethernet is connected to XHCI USB host controller
> on that board. Is it a known issue? Frankly I have no idea where to look
No, it is not known.
> to fix it. The same adapter connected to EHCI ports on other boards based
> on the same SoC works fine without any warnings.
Odd.
> And the log with mentioned warning:
>
> [ 17.768040]
> [ 17.772239] WARNING: inconsistent lock state
> [ 17.776511] 4.16.0-rc3-next-20180227-7-g876c53a7493c #453 Not tainted
> [ 17.783329]
> [ 17.787580] inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
> [ 17.793607] swapper/0/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
> [ 17.798751] (&syncp->seq#5){?.-.}, at: [<9b22e5f0>]
> asix_rx_fixup_internal+0x188/0x288
Looks like this triggers (in usbnet):
u64_stats_update_begin(&stats64->syncp);
stats64->rx_packets++;
stats64->rx_bytes += skb->len;
u64_stats_update_end(&stats64->syncp);
That I considered to be called under lock.
Could you comment this out for testing?
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] drivers: use 'depends on MFD_SYSCON' instead of 'select MFD_SYSCON'
2018-02-27 18:03 GMT+09:00 Arnd Bergmann : > On Tue, Feb 27, 2018 at 1:46 AM, Masahiro Yamada > wrote: >> 2018-02-26 21:43 GMT+09:00 Arnd Bergmann : >>> On Mon, Feb 26, 2018 at 12:53 PM, Masahiro Yamada >>> wrote: 2018-02-26 17:43 GMT+09:00 Arnd Bergmann : > On Sat, Feb 24, 2018 at 3:50 PM, Masahiro Yamada > wrote: >> As Documentation/kbuild/kconfig-language.txt notes, 'select' should be >> used with care - it forces a lower limit of another symbol, ignoring >> the dependency. >> >> MFD_SYSCON depends on HAS_IOMEM, but several drivers with COMPILE_TEST >> select it. >> >> This causes unmet dependencies for architecture without HAS_IOMEM. >> >> $ make ARCH=score randconfig >> scripts/kconfig/conf --randconfig Kconfig >> KCONFIG_SEED=0x27C47F43 >> warning: (HWSPINLOCK_QCOM && AHCI_MTK && STMMAC_PLATFORM && ...) >> selects MFD_SYSCON which has unmet direct dependencies (HAS_IOMEM) >> >> Use 'depends on' to observe the dependency. >> >> This commit was created by the following command: >> >> $ find drivers -name 'Kconfig*' | xargs sed -i -e \ >> 's/select MFD_SYSCON$/depends on MFD_SYSCON/' >> >> Then, COMMON_CLK_NXP and S3C2410_WATCHDOG were fixed up manually. >> >> Also, make MFD_SYSCON 'default y' because some defconfig files may >> rely on someone select's MFD_SYSCON. >> >> Signed-off-by: Masahiro Yamada >> --- >> >> If you have a better idea to fix 'unmet dependencies', >> please suggest. > > Changing 'select MFD_SYSCON' to 'depends on' will definitely break lots > of defconfig configurations, I'd rather not do that. Could you explain why? I set 'default y' for MFD_SYSCON. Would it still break defconfig configurations? >>> >>> No, you are right, that would not break defconfigs, it would just mean one >>> useless driver being enabled for many configurations that don't need it. >> >> If we are unhappy about this, >> we can send per-arch patches >> to add CONFIG_MTD_SYSCON=y to defconfigs that need it. >> >> But, we need to decide what the right solution is. > > I think for consistency, we should change the existing > 'depends on MFD_SYSCON' to 'select MFD_SYSCON'. This > matches what we do with REGMAP_MMIO. > > MFD_SYSCON is really a thin wrapper around REGMAP_MMIO, > so I would keep using the same conventions here, even though > we normally prefer to not 'select' any user-visible options. > > It might be possible to make MFD_SYSCON a silent symbol > as well, but we'd have to make sure that all users select the symbol > then. > > Arnd If we agree, I can send the following three patches. [1] Add "depends on HAS_IOMEM" to all drivers selecting MFD_SYSCON (Unmet dependencies will be fixed by this) [2] For consistency, convert existing "depends on MFD_SYSCON" to "select MFD_SYSCON" + "depends on HAS_IOMEM" [3] Change MFD_SYSCON to user-unconfigurable option. But, for COMPILE_TEST, allow users to enable it independently. Like follows: config MFD_SYSCON bool "System Controller Register R/W Based on Regmap" if COMPILE_TEST select REGMAP_MMIO help Select this option to enable accessing system control registers via regmap. Is this OK? -- Best Regards Masahiro Yamada -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] drivers: use 'depends on MFD_SYSCON' instead of 'select MFD_SYSCON'
On Tue, Feb 27, 2018 at 1:46 AM, Masahiro Yamada wrote: > 2018-02-26 21:43 GMT+09:00 Arnd Bergmann : >> On Mon, Feb 26, 2018 at 12:53 PM, Masahiro Yamada >> wrote: >>> 2018-02-26 17:43 GMT+09:00 Arnd Bergmann : On Sat, Feb 24, 2018 at 3:50 PM, Masahiro Yamada wrote: > As Documentation/kbuild/kconfig-language.txt notes, 'select' should be > used with care - it forces a lower limit of another symbol, ignoring > the dependency. > > MFD_SYSCON depends on HAS_IOMEM, but several drivers with COMPILE_TEST > select it. > > This causes unmet dependencies for architecture without HAS_IOMEM. > > $ make ARCH=score randconfig > scripts/kconfig/conf --randconfig Kconfig > KCONFIG_SEED=0x27C47F43 > warning: (HWSPINLOCK_QCOM && AHCI_MTK && STMMAC_PLATFORM && ...) > selects MFD_SYSCON which has unmet direct dependencies (HAS_IOMEM) > > Use 'depends on' to observe the dependency. > > This commit was created by the following command: > > $ find drivers -name 'Kconfig*' | xargs sed -i -e \ > 's/select MFD_SYSCON$/depends on MFD_SYSCON/' > > Then, COMMON_CLK_NXP and S3C2410_WATCHDOG were fixed up manually. > > Also, make MFD_SYSCON 'default y' because some defconfig files may > rely on someone select's MFD_SYSCON. > > Signed-off-by: Masahiro Yamada > --- > > If you have a better idea to fix 'unmet dependencies', > please suggest. Changing 'select MFD_SYSCON' to 'depends on' will definitely break lots of defconfig configurations, I'd rather not do that. >>> >>> >>> Could you explain why? >>> >>> I set 'default y' for MFD_SYSCON. >>> >>> Would it still break defconfig configurations? >> >> No, you are right, that would not break defconfigs, it would just mean one >> useless driver being enabled for many configurations that don't need it. > > If we are unhappy about this, > we can send per-arch patches > to add CONFIG_MTD_SYSCON=y to defconfigs that need it. > > But, we need to decide what the right solution is. I think for consistency, we should change the existing 'depends on MFD_SYSCON' to 'select MFD_SYSCON'. This matches what we do with REGMAP_MMIO. MFD_SYSCON is really a thin wrapper around REGMAP_MMIO, so I would keep using the same conventions here, even though we normally prefer to not 'select' any user-visible options. It might be possible to make MFD_SYSCON a silent symbol as well, but we'd have to make sure that all users select the symbol then. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery
On 26.02.2018 12:49, Li Jun wrote:
> In case of usb-c-connector with power delivery support, add bingdings
> supported by current typec driver, so user can pass all those properties
> via dt.
>
> Signed-off-by: Li Jun
> ---
> Changes for v2:
> - Added typec properties are based on general usb connector bindings[1]
> proposed by Andrzej Hajda.
> - Use the standard unit suffixes as defined in property-units.txt.
>
> [1] https://patchwork.kernel.org/patch/10231447/
>
> .../bindings/connector/usb-connector.txt | 43
> ++
> 1 file changed, 43 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt
> b/Documentation/devicetree/bindings/connector/usb-connector.txt
> index e1463f1..242f6df 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> @@ -15,6 +15,30 @@ Optional properties:
> - type: size of the connector, should be specified in case of USB-A, USB-B
>non-fullsize connectors: "mini", "micro".
>
> +Required properties for usb-c-connector with power delivery support:
> +- port-type: should be one of "source", "sink" or "dual".
> +- default-role: preferred power role if port-type is "dual"(drp), should be
> + "sink" or "source".
Since port carries data and power, it would be better to explicitly
mention it in names of properties which can be ambiguous.
Maybe instead of 'port-type' you can use 'power-role', for example.
Other thing is that default-role is required only in case of DRP, so
maybe better would be to squash it in power-role, for example:
power-role: should be on of "source", "sink", "dual-source",
"dual-sink", in case of dual roles suffix determines preferred role.
> +- src-pdos: An array of u32 with each entry providing supported power
> + source data object(PDO), the detailed bit definitions of PDO can be found
> + in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.2
> + Source_Capabilities Message, the order of each entry(PDO) should follow
> + the PD spec chapter 6.4.1. Required for power source and power dual role.
> +- snk-pdos: An array of u32 with each entry providing supported power
> + sink data object(PDO), the detailed bit definitions of PDO can be found in
> + "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3 Sink
> + Capabilities Message, the order of each entry(PDO) should follow the PD
> + spec chapter 6.4.1. Required for power sink and power dual role.
We should avoid magic numbers, magic numbers are bad :)
If we really need to use PDOs here, I think we can re-use PDO_* macros
[1] - DTC should be able to parse it, maybe some lifting will be necessary.
[1]:
https://elixir.bootlin.com/linux/v4.16-rc3/source/include/linux/usb/pd.h#L161
> +- max-snk-microvolt: The max voltage the sink can support in micro volts,
> + required for power sink and power dual role.
> +- max-snk-microamp: The max current the sink can support in micro amps,
> + required for power sink and power dual role.
> +- max-snk-microwatt-hours: The max power the sink can support in micro
> + Watt-hours, required for power sink and power dual role.
> +- op-snk-microwatt-hours: Sink required operating power in micro Watt-hours,
> + if source offered power is less then it, Capability Mismatch is set,
> + required for power sink and power dual role.
What is the relation between above properties and PDOs specified earlier?
Are you sure all these props are required?
And general remark regarding all above properties. For me, most of them
are specific not to the port but to devices responsible for power
management: chargers, pmics,
In many cases these properties are redundant with devices capabilities.
On the other side I guess in many cases it may be convenient to put them
here, so I am not sure what is better :)
Regards
Andrzej
> +
> Required nodes:
> - any data bus to the connector should be modeled using the OF graph bindings
>specified in bindings/graph.txt, unless the bus is between parent node and
> @@ -73,3 +97,22 @@ ccic: s2mm005@33 {
> };
> };
> };
> +
> +3. USB-C connector attached to a typec port controller(ptn5110), which has
> +power delivery support and enables drp.
> +
> +typec: ptn5110@50 {
> + ...
> + usb_con: connector {
> + compatible = "usb-c-connector";
> + label = "USB-C";
> + port-type = "dual";
> + default-role = "sink";
> + src-pdos = <0x380190c8>;
> + snk-pdos = <0x380190c8 0x3802d0c8>;
> + max-snk-microvolt = <9000>;
> + max-snk-microamp = <2000>;
> + max-snk-microwatt-hours = <18000>;
> + op-snk-microwatt-hours = <9000>;
> + };
> +};
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/
[PATCH 2/2] usb: gadget: udc: renesas_usb3: add binging for r8a77965
This patch adds binding for r8a77965 (R-Car M3-N). Signed-off-by: Yoshihiro Shimoda --- Documentation/devicetree/bindings/usb/renesas_usb3.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt index 87a45e2..2c071bb 100644 --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt @@ -4,6 +4,7 @@ Required properties: - compatible: Must contain one of the following: - "renesas,r8a7795-usb3-peri" - "renesas,r8a7796-usb3-peri" + - "renesas,r8a77965-usb3-peri" - "renesas,rcar-gen3-usb3-peri" for a generic R-Car Gen3 compatible device -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: renesas_usbhs: add binding for r8a77965
This patch adds binding for r8a77965 (R-Car M3-N). Signed-off-by: Yoshihiro Shimoda --- Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt index d060172..43960fa 100644 --- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt +++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt @@ -12,6 +12,7 @@ Required properties: - "renesas,usbhs-r8a7794" for r8a7794 (R-Car E2) compatible device - "renesas,usbhs-r8a7795" for r8a7795 (R-Car H3) compatible device - "renesas,usbhs-r8a7796" for r8a7796 (R-Car M3-W) compatible device + - "renesas,usbhs-r8a77965" for r8a77965 (R-Car M3-N) compatible device - "renesas,usbhs-r8a77995" for r8a77995 (R-Car D3) compatible device - "renesas,usbhs-r7s72100" for r7s72100 (RZ/A1) compatible device - "renesas,rcar-gen2-usbhs" for R-Car Gen2 or RZ/G1 compatible devices -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] usb: add USB2.0/3.0 peripheral bindings for r8a77965
This patch set is based on the latest Felipe's usb.git / testing/next branch (commit id = 892b3e4a3df3f8fa18053aba65acd5a3dee05325). Yoshihiro Shimoda (2): usb: renesas_usbhs: add binding for r8a77965 usb: gadget: udc: renesas_usb3: add binging for r8a77965 Documentation/devicetree/bindings/usb/renesas_usb3.txt | 1 + Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 1 + 2 files changed, 2 insertions(+) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: host: xhci-rcar: add support for r8a77965
This patch adds support for r8a77965 (R-Car M3-N).
Signed-off-by: Yoshihiro Shimoda
---
Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
drivers/usb/host/xhci-rcar.c | 4
2 files changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index e2ea59b..1651483a 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -13,6 +13,7 @@ Required properties:
- "renesas,xhci-r8a7793" for r8a7793 SoC
- "renesas,xhci-r8a7795" for r8a7795 SoC
- "renesas,xhci-r8a7796" for r8a7796 SoC
+- "renesas,xhci-r8a77965" for r8a77965 SoC
- "renesas,rcar-gen2-xhci" for a generic R-Car Gen2 or RZ/G1 compatible
device
- "renesas,rcar-gen3-xhci" for a generic R-Car Gen3 compatible device
diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
index f0b5596..f33ffc2 100644
--- a/drivers/usb/host/xhci-rcar.c
+++ b/drivers/usb/host/xhci-rcar.c
@@ -83,6 +83,10 @@
.soc_id = "r8a7796",
.data = (void *)RCAR_XHCI_FIRMWARE_V3,
},
+ {
+ .soc_id = "r8a77965",
+ .data = (void *)RCAR_XHCI_FIRMWARE_V3,
+ },
{ /* sentinel */ },
};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: host: xhci-rcar: add support for r8a77965
Hi,
I'm afraid but I will re-submit this patch because we should get review by
device tree maintainers.
Best regards,
Yoshihiro Shimoda
> From: Yoshihiro Shimoda, Sent: Tuesday, February 27, 2018 5:12 PM
>
> This patch adds support for r8a77965 (R-Car M3-N).
>
> Signed-off-by: Yoshihiro Shimoda
> ---
> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
> drivers/usb/host/xhci-rcar.c | 4
> 2 files changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt
> b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> index e2ea59b..1651483a 100644
> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> @@ -13,6 +13,7 @@ Required properties:
> - "renesas,xhci-r8a7793" for r8a7793 SoC
> - "renesas,xhci-r8a7795" for r8a7795 SoC
> - "renesas,xhci-r8a7796" for r8a7796 SoC
> +- "renesas,xhci-r8a77965" for r8a77965 SoC
> - "renesas,rcar-gen2-xhci" for a generic R-Car Gen2 or RZ/G1 compatible
>device
> - "renesas,rcar-gen3-xhci" for a generic R-Car Gen3 compatible device
> diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
> index f0b5596..f33ffc2 100644
> --- a/drivers/usb/host/xhci-rcar.c
> +++ b/drivers/usb/host/xhci-rcar.c
> @@ -83,6 +83,10 @@
> .soc_id = "r8a7796",
> .data = (void *)RCAR_XHCI_FIRMWARE_V3,
> },
> + {
> + .soc_id = "r8a77965",
> + .data = (void *)RCAR_XHCI_FIRMWARE_V3,
> + },
> { /* sentinel */ },
> };
>
> --
> 1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: host: xhci-rcar: add support for r8a77965
This patch adds support for r8a77965 (R-Car M3-N).
Signed-off-by: Yoshihiro Shimoda
---
Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
drivers/usb/host/xhci-rcar.c | 4
2 files changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index e2ea59b..1651483a 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -13,6 +13,7 @@ Required properties:
- "renesas,xhci-r8a7793" for r8a7793 SoC
- "renesas,xhci-r8a7795" for r8a7795 SoC
- "renesas,xhci-r8a7796" for r8a7796 SoC
+- "renesas,xhci-r8a77965" for r8a77965 SoC
- "renesas,rcar-gen2-xhci" for a generic R-Car Gen2 or RZ/G1 compatible
device
- "renesas,rcar-gen3-xhci" for a generic R-Car Gen3 compatible device
diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
index f0b5596..f33ffc2 100644
--- a/drivers/usb/host/xhci-rcar.c
+++ b/drivers/usb/host/xhci-rcar.c
@@ -83,6 +83,10 @@
.soc_id = "r8a7796",
.data = (void *)RCAR_XHCI_FIRMWARE_V3,
},
+ {
+ .soc_id = "r8a77965",
+ .data = (void *)RCAR_XHCI_FIRMWARE_V3,
+ },
{ /* sentinel */ },
};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
