Re: [PATCH v3 1/2] dt-bindings: usb: add documentation for typec switch via GPIO
On Mon, Mar 18, 2019 at 10:59:31AM +, Jun Li wrote: > > > > -Original Message- > > From: Heikki Krogerus > > Sent: 2019年3月13日 17:36 > > To: Rob Herring > > Cc: Jun Li ; gre...@linuxfoundation.org; > > hdego...@redhat.com; > > andy.shevche...@gmail.com; linux-usb@vger.kernel.org; > > devicet...@vger.kernel.org; dl-linux-imx > > Subject: Re: [PATCH v3 1/2] dt-bindings: usb: add documentation for typec > > switch > > via GPIO > > > > On Tue, Mar 12, 2019 at 09:45:27AM -0500, Rob Herring wrote: > > > On Mon, Mar 11, 2019 at 10:40:09AM +, Jun Li wrote: > > > > Some typec super speed active channel switch can be controlled via a > > > > GPIO, this binding can be used to specify the switch node by a GPIO > > > > and the remote endpoint of its consumer. > > > > > > > > Signed-off-by: Li Jun > > > > --- > > > > .../devicetree/bindings/usb/typec-switch-gpio.txt | 30 > > > > ++ > > > > 1 file changed, 30 insertions(+) > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/usb/typec-switch-gpio.txt > > > > b/Documentation/devicetree/bindings/usb/typec-switch-gpio.txt > > > > new file mode 100644 > > > > index 000..4ef76cf > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/usb/typec-switch-gpio.txt > > > > @@ -0,0 +1,30 @@ > > > > +Typec orientation switch via a GPIO > > > > +--- > > > > + > > > > +Required properties: > > > > +- compatible: should be set one of following: > > > > + - "nxp,ptn36043" for NXP Type-C SuperSpeed active switch. > > > > + > > > > +- gpios: the GPIO used to switch the super speed active channel, > > > > > > Perhaps switch-gpios in case there are other gpios needed. > > > > > > > + GPIO_ACTIVE_HIGH: GPIO state high for cc1; > > > > + GPIO_ACTIVE_LOW: GPIO state low for cc1. > > > > +- orientation-switch: must be present. > > > > > > Why is this needed? The compatible can't imply this? > > > > I think Jun Li is added that based on the comment I put to > > drivers/usb/typec/mux.c, > > so I'm to blame here. If we can handle this with the compatible like I > > guess we can, > > I'm happy. > > Hi Heikki > > Can I just remove the original bool property check? i.e, match OK if the > remote > parent node is in switch_list. No. If typec_switch_get() is called before the mux device is registered, we need to return -EPROBE_DEFER. That means we need to be able to identify the mux device node. I think we should just use the compatible like Rob suggested. The Type-C muxes should probable have their own bindings file where it's defined for these muxes. thanks, -- heikki
Re: [PATCH] [PATCH] usb: gadget: composite: Fix double free memory bug
Hi,
On 2019-03-15 16:37, Chandana Kishori Chiluveru wrote:
configfs_dev_cleanup function can double free os_desc
and buffer when called from different context. For
example, this can be called from composite_unbind() and
when composite_bind() fails.
Shouldn't we instead fix the error path handling of composite_bind
and configfs_composite_bind?
Fix this issue by setting
request and buffer pointer to NULL after kfree.
Signed-off-by: Chandana Kishori Chiluveru
---
drivers/usb/gadget/composite.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/gadget/composite.c
b/drivers/usb/gadget/composite.c
index b8a1584..992f1e2 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2155,14 +2155,18 @@ void composite_dev_cleanup(struct
usb_composite_dev *cdev)
usb_ep_dequeue(cdev->gadget->ep0, cdev->os_desc_req);
kfree(cdev->os_desc_req->buf);
+ cdev->os_desc_req->buf = NULL;
usb_ep_free_request(cdev->gadget->ep0, cdev->os_desc_req);
+ cdev->os_desc_req = NULL;
Better to move os_desc_cleanup handling to a different function
say - composite_os_desc_req_cleanup (to match
composite_os_desc_req_prepare())
}
if (cdev->req) {
if (cdev->setup_pending)
usb_ep_dequeue(cdev->gadget->ep0, cdev->req);
kfree(cdev->req->buf);
+ cdev->req->buf = NULL;
usb_ep_free_request(cdev->gadget->ep0, cdev->req);
+ cdev->req = NULL;
}
cdev->next_string_id = 0;
device_remove_file(&cdev->gadget->dev, &dev_attr_suspended);
Re: [PATCH] USB: serial: option: Add support for Quectel EM12
On Sat, Mar 02, 2019 at 01:35:53PM +0100, Kristian Evensen wrote:
> The Quectel EM12 is a Cat. 12 LTE modem. It behaves in the exactly the
> same way as the EP06 (including the dynamic configuration behavior), so
> the same checkes on reserved interfaces, etc. are needed.
>
> Signed-off-by: Kristian Evensen
> ---
> drivers/usb/serial/option.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index aef15497ff31..cc8932658e72 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -246,6 +246,7 @@ static void option_instat_callback(struct urb *urb);
> #define QUECTEL_PRODUCT_EC25 0x0125
> #define QUECTEL_PRODUCT_BG96 0x0296
> #define QUECTEL_PRODUCT_EP06 0x0306
> +#define QUECTEL_PRODUCT_EM12 0x0512
>
> #define CMOTECH_VENDOR_ID0x16d8
> #define CMOTECH_PRODUCT_6001 0x6001
> @@ -1086,7 +1087,10 @@ static const struct usb_device_id option_ids[] = {
> .driver_info = RSVD(4) },
> { USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID,
> QUECTEL_PRODUCT_EP06, 0xff, 0xff, 0xff),
> .driver_info = RSVD(1) | RSVD(2) | RSVD(3) | RSVD(4) | NUMEP2 },
> + { USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID,
> QUECTEL_PRODUCT_EM12, 0xff, 0xff, 0xff),
> + .driver_info = RSVD(1) | RSVD(2) | RSVD(3) | RSVD(4) | NUMEP2 },
> { USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID,
> QUECTEL_PRODUCT_EP06, 0xff, 0, 0) },
> + { USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID,
> QUECTEL_PRODUCT_EM12, 0xff, 0, 0) },
I think we should keep both entries together, so reordered above.
> { USB_DEVICE(CMOTECH_VENDOR_ID, CMOTECH_PRODUCT_6001) },
> { USB_DEVICE(CMOTECH_VENDOR_ID, CMOTECH_PRODUCT_CMU_300) },
> { USB_DEVICE(CMOTECH_VENDOR_ID, CMOTECH_PRODUCT_6003),
Now applied, but could you please post the output of usb-devices (with
all interfaces enabled) for this device for reference?
Thanks,
Johan
Re: [PATCH 4/4] usb/serial: Convert serial_minors to XArray
On Tue, Mar 19, 2019 at 07:14:09AM +0100, Greg KH wrote: > On Mon, Mar 18, 2019 at 02:17:13PM -0700, Matthew Wilcox wrote: > > Signed-off-by: Matthew Wilcox > > --- > > drivers/usb/serial/usb-serial.c | 21 ++--- > > 1 file changed, 10 insertions(+), 11 deletions(-) > > I really do not like taking patches without any changelog text at all :( I'm really not sure what to say. Something extolling the superiority of the new API? I've got 279 patches here and some of them are really just 1:1 replacements like this one. What would you want to read if you came across this patch in git log in five years time?
Re: [PATCH 4/4] usb/serial: Convert serial_minors to XArray
On Di, 2019-03-19 at 04:35 -0700, Matthew Wilcox wrote: > On Tue, Mar 19, 2019 at 07:14:09AM +0100, Greg KH wrote: > > On Mon, Mar 18, 2019 at 02:17:13PM -0700, Matthew Wilcox wrote: > > > Signed-off-by: Matthew Wilcox > > > --- > > > drivers/usb/serial/usb-serial.c | 21 ++--- > > > 1 file changed, 10 insertions(+), 11 deletions(-) > > > > I really do not like taking patches without any changelog text at all :( > > I'm really not sure what to say. Something extolling the superiority > of the new API? Yes. Something like XARRAy is a replacement for radix trees. Convert this subsystem. Part of a project for the whole kernel. Regards Oliver
Re: [PATCH 4/4] usb/serial: Convert serial_minors to XArray
On Tue, Mar 19, 2019 at 12:36:21PM +0100, Oliver Neukum wrote: > On Di, 2019-03-19 at 04:35 -0700, Matthew Wilcox wrote: > > On Tue, Mar 19, 2019 at 07:14:09AM +0100, Greg KH wrote: > > > On Mon, Mar 18, 2019 at 02:17:13PM -0700, Matthew Wilcox wrote: > > > > Signed-off-by: Matthew Wilcox > > > > --- > > > > drivers/usb/serial/usb-serial.c | 21 ++--- > > > > 1 file changed, 10 insertions(+), 11 deletions(-) > > > > > > I really do not like taking patches without any changelog text at all :( > > > > I'm really not sure what to say. Something extolling the superiority > > of the new API? > > Yes. Something like > > XARRAy is a replacement for radix trees. Convert this subsystem. > Part of a project for the whole kernel. Yes, that's perfect. Matthew, just add it to a script and away you go :) thanks, greg k-h
Re: [PATCH 2/4] cdc-acm: Convert acm_minors to XArray
On Mon, Mar 18, 2019 at 02:17:11PM -0700, Matthew Wilcox wrote:
> Signed-off-by: Matthew Wilcox
Also something here saying acm_minors was an idr structure that is being
converted to xarray.
> ---
> drivers/usb/class/cdc-acm.c | 33 ++---
> 1 file changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> index 739f8960811a..28eb9a898b4a 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -47,7 +47,7 @@
> static struct usb_driver acm_driver;
> static struct tty_driver *acm_tty_driver;
>
> -static DEFINE_IDR(acm_minors);
> +static DEFINE_XARRAY_ALLOC(acm_minors);
> static DEFINE_MUTEX(acm_minors_lock);
>
> static void acm_tty_set_termios(struct tty_struct *tty,
> @@ -66,7 +66,7 @@ static struct acm *acm_get_by_minor(unsigned int minor)
> struct acm *acm;
>
> mutex_lock(&acm_minors_lock);
> - acm = idr_find(&acm_minors, minor);
> + acm = xa_load(&acm_minors, minor);
> if (acm) {
> mutex_lock(&acm->mutex);
> if (acm->disconnected) {
> @@ -86,20 +86,15 @@ static struct acm *acm_get_by_minor(unsigned int minor)
> */
> static int acm_alloc_minor(struct acm *acm)
> {
> - int minor;
> -
> - mutex_lock(&acm_minors_lock);
> - minor = idr_alloc(&acm_minors, acm, 0, ACM_TTY_MINORS, GFP_KERNEL);
> - mutex_unlock(&acm_minors_lock);
> -
> - return minor;
> + return xa_alloc(&acm_minors, &acm->minor, acm,
> + XA_LIMIT(0, ACM_TTY_MINORS - 1), GFP_KERNEL);
So, the only thing "better" here is that you don't need a lock anymore?
Why not just replace the idr api "underneath" with xarray and then drop
all of the locks over time?
idr was just a wrapper on top of ida, what's one more :)
Anyway, no objection from me, I like tree-wide changes, and appreciate
the effort that goes into it. Thanks for doing it. If you fix up the
changelog entries for the ones that have none, I'll be glad to queue
these up.
thanks,
greg k-h
Re: [PATCH 1/4] usb: Convert xhci-mem to XArray
On Mon, Mar 18, 2019 at 02:17:10PM -0700, Matthew Wilcox wrote: > The XArray API is a better fit for xhci than the radix tree API was, > to the point where we can remove the wrappers around the radix tree > and just call the XArray APIs directly. > > Signed-off-by: Matthew Wilcox > --- > drivers/usb/host/xhci-mem.c | 86 + > drivers/usb/host/xhci.h | 5 ++- > 2 files changed, 33 insertions(+), 58 deletions(-) At first glance this looks good to me, did you test it? thanks, greg k-h
Re: [PATCH 2/4] cdc-acm: Convert acm_minors to XArray
On Mo, 2019-03-18 at 14:17 -0700, Matthew Wilcox wrote: > Signed-off-by: Matthew Wilcox Straight replacement. Looks fine to me. But without a commit message Greg would dismember both of us. Regards Oliver
Re: [PATCH 2/4] cdc-acm: Convert acm_minors to XArray
On Di, 2019-03-19 at 13:14 +0100, Greg KH wrote:
> On Mon, Mar 18, 2019 at 02:17:11PM -0700, Matthew Wilcox wrote:
> > Signed-off-by: Matthew Wilcox
[..]
> > static int acm_alloc_minor(struct acm *acm)
> > {
> > - int minor;
> > -
> > - mutex_lock(&acm_minors_lock);
> > - minor = idr_alloc(&acm_minors, acm, 0, ACM_TTY_MINORS, GFP_KERNEL);
> > - mutex_unlock(&acm_minors_lock);
> > -
> > - return minor;
> > + return xa_alloc(&acm_minors, &acm->minor, acm,
> > + XA_LIMIT(0, ACM_TTY_MINORS - 1), GFP_KERNEL);
>
> So, the only thing "better" here is that you don't need a lock anymore?
Once this is accepted I can reduce the locking to a per instance
spinlock. But to start with a straight conversion is best.
Regards
Oliver
[PATCH] usb: typec: wcove: Provide fwnode for the port
By registering a software fwnode for the port, we can supply
the connector capabilities to the tcpm using the common USB
connector device properties instead of relying on platform
data (struct tcpc_config).
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/tcpm/wcove.c | 29 -
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/typec/tcpm/wcove.c b/drivers/usb/typec/tcpm/wcove.c
index 423208e19383..53f00ad9aa92 100644
--- a/drivers/usb/typec/tcpm/wcove.c
+++ b/drivers/usb/typec/tcpm/wcove.c
@@ -587,17 +587,14 @@ static const u32 snk_pdo[] = {
PDO_VAR(5000, 12000, 3000),
};
-static struct tcpc_config wcove_typec_config = {
- .src_pdo = src_pdo,
- .nr_src_pdo = ARRAY_SIZE(src_pdo),
- .snk_pdo = snk_pdo,
- .nr_snk_pdo = ARRAY_SIZE(snk_pdo),
-
- .operating_snk_mw = 15000,
-
- .type = TYPEC_PORT_DRP,
- .data = TYPEC_PORT_DRD,
- .default_role = TYPEC_SINK,
+static const struct property_entry wcove_props[] = {
+ PROPERTY_ENTRY_STRING("data-role", "dual"),
+ PROPERTY_ENTRY_STRING("power-role", "dual"),
+ PROPERTY_ENTRY_STRING("try-power-role", "sink"),
+ PROPERTY_ENTRY_U32_ARRAY("source-pdos", src_pdo),
+ PROPERTY_ENTRY_U32_ARRAY("sink-pdos", snk_pdo),
+ PROPERTY_ENTRY_U32("op-sink-microwatt", 15000),
+ { }
};
static int wcove_typec_probe(struct platform_device *pdev)
@@ -643,17 +640,22 @@ static int wcove_typec_probe(struct platform_device *pdev)
wcove->tcpc.set_roles = wcove_set_roles;
wcove->tcpc.pd_transmit = wcove_pd_transmit;
- wcove->tcpc.config = &wcove_typec_config;
+ wcove->tcpc.fwnode = fwnode_create_software_node(wcove_props, NULL);
+ if (IS_ERR(wcove->tcpc.fwnode))
+ return PTR_ERR(wcove->tcpc.fwnode);
wcove->tcpm = tcpm_register_port(wcove->dev, &wcove->tcpc);
- if (IS_ERR(wcove->tcpm))
+ if (IS_ERR(wcove->tcpm)) {
+ fwnode_remove_software_node(wcove->tcpc.fwnode);
return PTR_ERR(wcove->tcpm);
+ }
ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
wcove_typec_irq, IRQF_ONESHOT,
"wcove_typec", wcove);
if (ret) {
tcpm_unregister_port(wcove->tcpm);
+ fwnode_remove_software_node(wcove->tcpc.fwnode);
return ret;
}
@@ -673,6 +675,7 @@ static int wcove_typec_remove(struct platform_device *pdev)
regmap_write(wcove->regmap, USBC_IRQMASK2, val | USBC_IRQMASK2_ALL);
tcpm_unregister_port(wcove->tcpm);
+ fwnode_remove_software_node(wcove->tcpc.fwnode);
return 0;
}
--
2.20.1
Re: [PATCH 2/4] cdc-acm: Convert acm_minors to XArray
On Tue, Mar 19, 2019 at 01:14:20PM +0100, Greg KH wrote:
> On Mon, Mar 18, 2019 at 02:17:11PM -0700, Matthew Wilcox wrote:
> > Signed-off-by: Matthew Wilcox
>
> Also something here saying acm_minors was an idr structure that is being
> converted to xarray.
ACK.
> > @@ -86,20 +86,15 @@ static struct acm *acm_get_by_minor(unsigned int minor)
> > */
> > static int acm_alloc_minor(struct acm *acm)
> > {
> > - int minor;
> > -
> > - mutex_lock(&acm_minors_lock);
> > - minor = idr_alloc(&acm_minors, acm, 0, ACM_TTY_MINORS, GFP_KERNEL);
> > - mutex_unlock(&acm_minors_lock);
> > -
> > - return minor;
> > + return xa_alloc(&acm_minors, &acm->minor, acm,
> > + XA_LIMIT(0, ACM_TTY_MINORS - 1), GFP_KERNEL);
>
> So, the only thing "better" here is that you don't need a lock anymore?
As far as this driver is concerned, I think that's the only advantage.
Other drivers see more advantages.
> Why not just replace the idr api "underneath" with xarray and then drop
> all of the locks over time?
>
> idr was just a wrapper on top of ida, what's one more :)
umm, IDR was never a wrapper over IDA. Originally, it was its own data
structure, then IDA was grafted on top of IDR, then I merged the IDR
and radix tree, now I'm replacing both the IDR and the radix tree.
> Anyway, no objection from me, I like tree-wide changes, and appreciate
> the effort that goes into it. Thanks for doing it. If you fix up the
> changelog entries for the ones that have none, I'll be glad to queue
> these up.
Thanks.
Re: [PATCH v2] usb: typec: tcpm: Try PD-2.0 if sink does not respond to 3.0 source-caps
On Sat, Mar 16, 2019 at 04:57:12PM +0100, Hans de Goede wrote:
> PD 2.0 sinks are supposed to accept src-capabilities with a 3.0 header and
> simply ignore any src PDOs which the sink does not understand such as PPS
> but some 2.0 sinks instead ignore the entire PD_DATA_SOURCE_CAP message,
> causing contract negotiation to fail.
>
> This commit fixes such sinks not working by re-trying the contract
> negotiation with PD-2.0 source-caps messages if we don't have a contract
> after PD_N_HARD_RESET_COUNT hard-reset attempts.
>
> The problem fixed by this commit was noticed with a Type-C to VGA dongle.
>
> Signed-off-by: Hans de Goede
Reviewed-by: Heikki Krogerus
Also applied to my typec-next branch:
https://github.com/krohei/linux/commit/35e8c23e398ad03e7d9268a11a0408f8642da6d0
> ---
> The Type-C to VGA dongle on which this encountered looks like this one:
> https://www.aliexpress.com/item/Male-USB-3-1-Type-C-USB-C-to-Female-VGA-Adapter-Cable-10Gbps-for-New/32898274476.html
> ---
> Changes in v2:
> -Refactor the patch to avoid a check vs use race wrt negotiated_rev
> ---
> drivers/usb/typec/tcpm/tcpm.c | 27 ++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index f1c39a3c7534..d34e945e5d09 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -37,6 +37,7 @@
> S(SRC_ATTACHED),\
> S(SRC_STARTUP), \
> S(SRC_SEND_CAPABILITIES), \
> + S(SRC_SEND_CAPABILITIES_TIMEOUT), \
> S(SRC_NEGOTIATE_CAPABILITIES), \
> S(SRC_TRANSITION_SUPPLY), \
> S(SRC_READY), \
> @@ -2966,10 +2967,34 @@ static void run_state_machine(struct tcpm_port *port)
> /* port->hard_reset_count = 0; */
> port->caps_count = 0;
> port->pd_capable = true;
> - tcpm_set_state_cond(port, hard_reset_state(port),
> + tcpm_set_state_cond(port, SRC_SEND_CAPABILITIES_TIMEOUT,
> PD_T_SEND_SOURCE_CAP);
> }
> break;
> + case SRC_SEND_CAPABILITIES_TIMEOUT:
> + /*
> + * Error recovery for a PD_DATA_SOURCE_CAP reply timeout.
> + *
> + * PD 2.0 sinks are supposed to accept src-capabilities with a
> + * 3.0 header and simply ignore any src PDOs which the sink does
> + * not understand such as PPS but some 2.0 sinks instead ignore
> + * the entire PD_DATA_SOURCE_CAP message, causing contract
> + * negotiation to fail.
> + *
> + * After PD_N_HARD_RESET_COUNT hard-reset attempts, we try
> + * sending src-capabilities with a lower PD revision to
> + * make these broken sinks work.
> + */
> + if (port->hard_reset_count < PD_N_HARD_RESET_COUNT) {
> + tcpm_set_state(port, HARD_RESET_SEND, 0);
> + } else if (port->negotiated_rev > PD_REV20) {
> + port->negotiated_rev--;
> + port->hard_reset_count = 0;
> + tcpm_set_state(port, SRC_SEND_CAPABILITIES, 0);
> + } else {
> + tcpm_set_state(port, hard_reset_state(port), 0);
> + }
> + break;
> case SRC_NEGOTIATE_CAPABILITIES:
> ret = tcpm_pd_check_request(port);
> if (ret < 0) {
> --
> 2.20.1
thanks,
--
heikki
Re: [PATCH] usb: typec: pi3usb30532: Keep orientation when setting mux to safe mode
On Fri, Mar 15, 2019 at 04:36:13PM +0200, Heikki Krogerus wrote:
> On Fri, Feb 22, 2019 at 08:22:39PM +0100, Hans de Goede wrote:
> > Keep the orientation value when setting the mux to safe mode, this
> > fixes the orientation getting reset when switching alt-modes.
> >
> > Signed-off-by: Hans de Goede
>
> Acked-by: Heikki Krogerus
Also applied to my typec-next branch:
https://github.com/krohei/linux/commit/4002f81b327b4537dead79ed6117dcb8065d2dca
> > ---
> > drivers/usb/typec/mux/pi3usb30532.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/typec/mux/pi3usb30532.c
> > b/drivers/usb/typec/mux/pi3usb30532.c
> > index 64eb5983e17a..9294e85fd34b 100644
> > --- a/drivers/usb/typec/mux/pi3usb30532.c
> > +++ b/drivers/usb/typec/mux/pi3usb30532.c
> > @@ -84,7 +84,8 @@ static int pi3usb30532_mux_set(struct typec_mux *mux, int
> > state)
> >
> > switch (state) {
> > case TYPEC_STATE_SAFE:
> > - new_conf = PI3USB30532_CONF_OPEN;
> > + new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
> > + PI3USB30532_CONF_OPEN;
> > break;
> > case TYPEC_STATE_USB:
> > new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
> > --
> > 2.20.1
thanks,
--
heikki
Re: [PATCH v3 0/8] usb: typec: fusb302: Various fixes
On Fri, Mar 15, 2019 at 04:34:59PM +0200, Heikki Krogerus wrote: > On Mon, Mar 11, 2019 at 11:48:10AM +0100, Hans de Goede wrote: > > Hi All, > > > > Here is v3 of my fusb302 bug-fix series, the main fix in this series > > makes active adapters like Type-C to HDMI (often HDMI + USB-3-A) adapters > > work when they are to be powered by the Type-C port and thus present both > > an Ra and a Rd resistor on their Cc pins (patch 5). > > > > Changes in v3: > > > > [PATCH v3 4/8] usb: typec: fusb302: Check vconn is off when we start > > -Use WARN with a message describing the problem, instead of WARN_ON > > > > [PATCH v3 5/8] usb: typec: fusb302: Fix fusb302_handle_togdone_src Ra > > -Keep the delays for measuring Rd / Ra the same as before instead of > > multiplying them by a factor 100 > > > > [PATCH v3 7/8] usb: typec: fusb302: Improve suspend/resume handling > > -Change devm_request_irq to a regular request_irq free_irq, so that the work > > can be properly stopped on remove > > I tested these, and all of them look good to me: > > Acked-by: Heikki Krogerus I've also applied the entire series to my typec-next branch: https://github.com/krohei/linux/commits/typec-next thanks, -- heikki
Re: [PATCH v5 1/2] usb: typec: ucsi: add get_fw_info function
On Thu, Feb 07, 2019 at 11:18:12AM -0800, Ajay Gupta wrote: > From: Ajay Gupta > > Function is to get the details of ccg firmware and device version. > It will be useful in debugging and also during firmware update. > > Signed-off-by: Ajay Gupta Applied: https://github.com/krohei/linux/commit/21dcedbca01c9e8532449a50861900e06c518ad2 thanks, -- heikki
Re: [PATCH v5 2/2] usb: typec: ucsi: add firmware flashing support
On Thu, Feb 07, 2019 at 11:18:13AM -0800, Ajay Gupta wrote: > From: Ajay Gupta > > CCGx has two copies of the firmware in addition to the bootloader. > If the device is running FW1, FW2 can be updated with the new version. > Dual firmware mode allows the CCG device to stay in a PD contract and > support USB PD and Type-C functionality while a firmware update is in > progress. > > First we read the currently flashed firmware version of both > primary and secondary firmware and then compare it with > version of firmware file to determine if flashing is required. > > Command framework is added to support sending commands to CCGx > controller. We wait for response after sending the command and then > read the response from RAB_RESPONSE register. > > Below commands are supported, > - ENTER_FLASHING > - RESET > - PDPORT_ENABLE > - JUMP_TO_BOOT > - FLASH_ROW_RW > - VALIDATE_FW > > Command specific mutex lock is also added to sync between driver > and user threads. > > PD port number information is added which is required while sending > PD_PORT_ENABLE command > > Signed-off-by: Ajay Gupta Applied: https://github.com/krohei/linux/commit/6acc0b6b0b5041432016cd1c7adc8de20e80bf86 thanks, -- heikki
Re: [PATCH v2 1/3] usb: gadget: net2280: Fix overrun of OUT messages
On Mon, 18 Mar 2019 gu...@kiener-muenchen.de wrote: > Zitat von Alan Stern : > > > On Mon, 18 Mar 2019, Guido Kiener wrote: > > > >> The OUT endpoint normally blocks (NAK) subsequent packets when a > >> short packet was received and returns an incomplete queue entry to > >> the gadget driver. Thereby the gadget driver can detect a short packet > >> when reading queue entries with a length that is not equal to a > >> multiple of packet size. > >> > >> The start_queue() function enables receiving OUT packets regardless > >> of the content of the OUT FIFO. This results in a problem: > >> When receiving is enabled again, more OUT packets are appended to > >> the last short packet and the gadget driver cannot determine the end > >> of a short packet anymore. Furthermore the remaining data in the OUT > >> FIFO is not delivered to the gadget driver immediately and can produce > >> timeout errors. > >> > >> This fix stops OUT naking only when OUT naking is enabled and when the > >> OUT FIFO is empty. It ensures that all received data is delivered to > >> the gadget driver which can detect a short packet now before new > >> packets overrun the last short packet. > > > > This description should explain the race which causes the problem. > > With the current code, it's possible that the "!ep->is_in && > > (readl(&ep->regs->ep_stat) & BIT(NAK_OUT_PACKETS))" test will fail, > > then a short packet will be received, and then start_queue() will call > > stop_out_naking(). That's what we don't want -- OUT naking gets turned > > off while there is data in the FIFO. With the patch, this race can't > > occur because the FIFO's state is tested after we know that OUT naking > > is already turned on. > > > > Thanks. Please feel free to change my wording/spelling. I'm not offended. > Does this description meet your request: > > The OUT endpoint normally blocks (NAK) subsequent packets when a > short packet was received and returns an incomplete queue entry to > the gadget driver. Thereby the gadget driver can detect a short packet > when reading queue entries with a length that is not equal to a > multiple of packet size. > > The start_queue() function enables receiving OUT packets regardless > of the content of the OUT FIFO. This results in a race: > With the current code, it's possible that the "!ep->is_in && > (readl(&ep->regs->ep_stat) & BIT(NAK_OUT_PACKETS))" test will fail, > then a short packet will be received, and then start_queue() will call > stop_out_naking(). That's what we don't want -- OUT naking gets turned > off while there is data in the FIFO. With the patch, this race can't > occur because the FIFO's state is tested after we know that OUT naking > is already turned on. Here are a few more slight changes to the text. Submit the patch again with this description and it will be okay: The OUT endpoint normally blocks (NAK) subsequent packets when a short packet was received and returns an incomplete queue entry to the gadget driver. Thereby the gadget driver can detect a short packet when reading queue entries with a length that is not equal to a multiple of packet size. The start_queue() function enables receiving OUT packets regardless of the content of the OUT FIFO. This results in a race: With the current code, it's possible that the "!ep->is_in && (readl(&ep->regs->ep_stat) & BIT(NAK_OUT_PACKETS))" test in start_dma() will fail, then a short packet will be received, and then start_queue() will call stop_out_naking(). That's what we don't want (OUT naking gets turned off while there is data in the FIFO) because then the next driver request might receive a mixture of old and new packets. With the patch, this race can't occur because the FIFO's state is tested after we know that OUT naking is already turned on, and OUT naking is stopped only when both of the conditions are met. This ensures that all received data is delivered to the gadget driver, which can detect a short packet now before new packets are appended to the last short packet. Alan Stern
Re: [PATCH 3/4] usb: Convert usb_bus_idr to XArray
On Mon, 18 Mar 2019, Matthew Wilcox wrote: > Remove the usb_bus_idr_lock as it doesn't appear to be protecting > anything more than the built-in XArray lock does. > > Signed-off-by: Matthew Wilcox > --- > drivers/usb/core/devices.c | 10 +++-- > drivers/usb/core/hcd.c | 40 - > drivers/usb/core/usb.c | 1 - > drivers/usb/host/r8a66597-hcd.c | 4 +--- > drivers/usb/mon/mon_main.c | 7 +++--- > include/linux/usb/hcd.h | 3 +-- > 6 files changed, 17 insertions(+), 48 deletions(-) This certainly looks correct, but I haven't tested it. If you get the same data in /sys/kernel/debug/usb/devices with the patch and without it, feel free to add: Acked-by: Alan Stern Alan Stern
Re: [PATCH v2 1/3] usb: gadget: net2280: Fix overrun of OUT messages
Zitat von Alan Stern : Here are a few more slight changes to the text. Submit the patch again with this description and it will be okay: The OUT endpoint normally blocks (NAK) subsequent packets when a short packet was received and returns an incomplete queue entry to the gadget driver. Thereby the gadget driver can detect a short packet when reading queue entries with a length that is not equal to a multiple of packet size. The start_queue() function enables receiving OUT packets regardless of the content of the OUT FIFO. This results in a race: With the current code, it's possible that the "!ep->is_in && (readl(&ep->regs->ep_stat) & BIT(NAK_OUT_PACKETS))" test in start_dma() will fail, then a short packet will be received, and then start_queue() will call stop_out_naking(). That's what we don't want (OUT naking gets turned off while there is data in the FIFO) because then the next driver request might receive a mixture of old and new packets. With the patch, this race can't occur because the FIFO's state is tested after we know that OUT naking is already turned on, and OUT naking is stopped only when both of the conditions are met. This ensures that all received data is delivered to the gadget driver, which can detect a short packet now before new packets are appended to the last short packet. Thank you. Sounds good. Guido
Re: [PATCH] usb: typec: wcove: Provide fwnode for the port
On Tue, Mar 19, 2019 at 03:53:24PM +0300, Heikki Krogerus wrote:
> By registering a software fwnode for the port, we can supply
> the connector capabilities to the tcpm using the common USB
> connector device properties instead of relying on platform
> data (struct tcpc_config).
>
> Signed-off-by: Heikki Krogerus
Reviewed-by: Guenter Roeck
> ---
> drivers/usb/typec/tcpm/wcove.c | 29 -
> 1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/wcove.c b/drivers/usb/typec/tcpm/wcove.c
> index 423208e19383..53f00ad9aa92 100644
> --- a/drivers/usb/typec/tcpm/wcove.c
> +++ b/drivers/usb/typec/tcpm/wcove.c
> @@ -587,17 +587,14 @@ static const u32 snk_pdo[] = {
> PDO_VAR(5000, 12000, 3000),
> };
>
> -static struct tcpc_config wcove_typec_config = {
> - .src_pdo = src_pdo,
> - .nr_src_pdo = ARRAY_SIZE(src_pdo),
> - .snk_pdo = snk_pdo,
> - .nr_snk_pdo = ARRAY_SIZE(snk_pdo),
> -
> - .operating_snk_mw = 15000,
> -
> - .type = TYPEC_PORT_DRP,
> - .data = TYPEC_PORT_DRD,
> - .default_role = TYPEC_SINK,
> +static const struct property_entry wcove_props[] = {
> + PROPERTY_ENTRY_STRING("data-role", "dual"),
> + PROPERTY_ENTRY_STRING("power-role", "dual"),
> + PROPERTY_ENTRY_STRING("try-power-role", "sink"),
> + PROPERTY_ENTRY_U32_ARRAY("source-pdos", src_pdo),
> + PROPERTY_ENTRY_U32_ARRAY("sink-pdos", snk_pdo),
> + PROPERTY_ENTRY_U32("op-sink-microwatt", 15000),
> + { }
> };
>
> static int wcove_typec_probe(struct platform_device *pdev)
> @@ -643,17 +640,22 @@ static int wcove_typec_probe(struct platform_device
> *pdev)
> wcove->tcpc.set_roles = wcove_set_roles;
> wcove->tcpc.pd_transmit = wcove_pd_transmit;
>
> - wcove->tcpc.config = &wcove_typec_config;
> + wcove->tcpc.fwnode = fwnode_create_software_node(wcove_props, NULL);
> + if (IS_ERR(wcove->tcpc.fwnode))
> + return PTR_ERR(wcove->tcpc.fwnode);
>
> wcove->tcpm = tcpm_register_port(wcove->dev, &wcove->tcpc);
> - if (IS_ERR(wcove->tcpm))
> + if (IS_ERR(wcove->tcpm)) {
> + fwnode_remove_software_node(wcove->tcpc.fwnode);
> return PTR_ERR(wcove->tcpm);
> + }
>
> ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> wcove_typec_irq, IRQF_ONESHOT,
> "wcove_typec", wcove);
> if (ret) {
> tcpm_unregister_port(wcove->tcpm);
> + fwnode_remove_software_node(wcove->tcpc.fwnode);
> return ret;
> }
>
> @@ -673,6 +675,7 @@ static int wcove_typec_remove(struct platform_device
> *pdev)
> regmap_write(wcove->regmap, USBC_IRQMASK2, val | USBC_IRQMASK2_ALL);
>
> tcpm_unregister_port(wcove->tcpm);
> + fwnode_remove_software_node(wcove->tcpc.fwnode);
>
> return 0;
> }
> --
> 2.20.1
>
[PATCH v3] usb: gadget: net2280: Fix overrun of OUT messages
The OUT endpoint normally blocks (NAK) subsequent packets when a short packet was received and returns an incomplete queue entry to the gadget driver. Thereby the gadget driver can detect a short packet when reading queue entries with a length that is not equal to a multiple of packet size. The start_queue() function enables receiving OUT packets regardless of the content of the OUT FIFO. This results in a race: With the current code, it's possible that the "!ep->is_in && (readl(&ep->regs->ep_stat) & BIT(NAK_OUT_PACKETS))" test in start_dma() will fail, then a short packet will be received, and then start_queue() will call stop_out_naking(). That's what we don't want (OUT naking gets turned off while there is data in the FIFO) because then the next driver request might receive a mixture of old and new packets. With the patch, this race can't occur because the FIFO's state is tested after we know that OUT naking is already turned on, and OUT naking is stopped only when both of the conditions are met. This ensures that all received data is delivered to the gadget driver, which can detect a short packet now before new packets are appended to the last short packet. Signed-off-by: Guido Kiener --- drivers/usb/gadget/udc/net2280.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c index f63f82450bf4..e0b413e9e532 100644 --- a/drivers/usb/gadget/udc/net2280.c +++ b/drivers/usb/gadget/udc/net2280.c @@ -866,9 +866,6 @@ static void start_queue(struct net2280_ep *ep, u32 dmactl, u32 td_dma) (void) readl(&ep->dev->pci->pcimstctl); writel(BIT(DMA_START), &dma->dmastat); - - if (!ep->is_in) - stop_out_naking(ep); } static void start_dma(struct net2280_ep *ep, struct net2280_request *req) @@ -907,6 +904,7 @@ static void start_dma(struct net2280_ep *ep, struct net2280_request *req) writel(BIT(DMA_START), &dma->dmastat); return; } + stop_out_naking(ep); } tmp = dmactl_default; -- 2.17.1
Re: [PATCH v3] usb: gadget: net2280: Fix overrun of OUT messages
On Tue, 19 Mar 2019, Guido Kiener wrote: > The OUT endpoint normally blocks (NAK) subsequent packets when a > short packet was received and returns an incomplete queue entry to > the gadget driver. Thereby the gadget driver can detect a short packet > when reading queue entries with a length that is not equal to a > multiple of packet size. > > The start_queue() function enables receiving OUT packets regardless of > the content of the OUT FIFO. This results in a race: With the current > code, it's possible that the "!ep->is_in && (readl(&ep->regs->ep_stat) > & BIT(NAK_OUT_PACKETS))" test in start_dma() will fail, then a short > packet will be received, and then start_queue() will call > stop_out_naking(). That's what we don't want (OUT naking gets turned > off while there is data in the FIFO) because then the next driver > request might receive a mixture of old and new packets. > > With the patch, this race can't occur because the FIFO's state is > tested after we know that OUT naking is already turned on, and OUT > naking is stopped only when both of the conditions are met. This > ensures that all received data is delivered to the gadget driver, > which can detect a short packet now before new packets are appended > to the last short packet. > > Signed-off-by: Guido Kiener > --- > drivers/usb/gadget/udc/net2280.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/usb/gadget/udc/net2280.c > b/drivers/usb/gadget/udc/net2280.c > index f63f82450bf4..e0b413e9e532 100644 > --- a/drivers/usb/gadget/udc/net2280.c > +++ b/drivers/usb/gadget/udc/net2280.c > @@ -866,9 +866,6 @@ static void start_queue(struct net2280_ep *ep, u32 > dmactl, u32 td_dma) > (void) readl(&ep->dev->pci->pcimstctl); > > writel(BIT(DMA_START), &dma->dmastat); > - > - if (!ep->is_in) > - stop_out_naking(ep); > } > > static void start_dma(struct net2280_ep *ep, struct net2280_request *req) > @@ -907,6 +904,7 @@ static void start_dma(struct net2280_ep *ep, struct > net2280_request *req) > writel(BIT(DMA_START), &dma->dmastat); > return; > } > + stop_out_naking(ep); > } > > tmp = dmactl_default; Acked-by: Alan Stern
Re: [PATCH v2 3/3] usb: gadget: net2272: Fix net2272_dequeue()
Zitat von Guido Kiener :
Restore the status of ep->stopped in function net2272_dequeue().
When the given request is not found in the endpoint queue
the function returns -EINVAL without restoring the state of
ep->stopped. Thus the endpoint keeps blocked and does not transfer
any data anymore.
This fix is only compile-tested, since we do not have a
corresponding hardware. An analogous fix was tested in the sibling
driver. See "usb: gadget: net2280: Fix net2280_dequeue()"
Signed-off-by: Guido Kiener
---
drivers/usb/gadget/udc/net2272.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/udc/net2272.c
b/drivers/usb/gadget/udc/net2272.c
index b77f3126580e..c2011cd7df8c 100644
--- a/drivers/usb/gadget/udc/net2272.c
+++ b/drivers/usb/gadget/udc/net2272.c
@@ -945,6 +945,7 @@ net2272_dequeue(struct usb_ep *_ep, struct
usb_request *_req)
break;
}
if (&req->req != _req) {
+ ep->stopped = stopped;
spin_unlock_irqrestore(&ep->dev->lock, flags);
return -EINVAL;
}
--
2.17.1
Alan,
do you want to add an acknowledgement for the sibling driver, too?
Guido
Re: [PATCH v2 3/3] usb: gadget: net2272: Fix net2272_dequeue()
On Tue, 19 Mar 2019 gu...@kiener-muenchen.de wrote:
> Zitat von Guido Kiener :
>
> > Restore the status of ep->stopped in function net2272_dequeue().
> >
> > When the given request is not found in the endpoint queue
> > the function returns -EINVAL without restoring the state of
> > ep->stopped. Thus the endpoint keeps blocked and does not transfer
> > any data anymore.
> >
> > This fix is only compile-tested, since we do not have a
> > corresponding hardware. An analogous fix was tested in the sibling
> > driver. See "usb: gadget: net2280: Fix net2280_dequeue()"
> >
> > Signed-off-by: Guido Kiener
> > ---
> > drivers/usb/gadget/udc/net2272.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/usb/gadget/udc/net2272.c
> > b/drivers/usb/gadget/udc/net2272.c
> > index b77f3126580e..c2011cd7df8c 100644
> > --- a/drivers/usb/gadget/udc/net2272.c
> > +++ b/drivers/usb/gadget/udc/net2272.c
> > @@ -945,6 +945,7 @@ net2272_dequeue(struct usb_ep *_ep, struct
> > usb_request *_req)
> > break;
> > }
> > if (&req->req != _req) {
> > + ep->stopped = stopped;
> > spin_unlock_irqrestore(&ep->dev->lock, flags);
> > return -EINVAL;
> > }
> > --
> > 2.17.1
>
> Alan,
>
> do you want to add an acknowledgement for the sibling driver, too?
>
> Guido
Yes, okay. I can't test this patch either, but for what it's worth:
Acked-by: Alan Stern
Alan Stern
[PATCH] usb-serial: fix mos_parport refcount imbalance on error path
write_parport_ref_nonblock increase mos_parport refcount without
decrease it when return -ENOMEM code, so need a decrement before function
return -ENOMEM.
Signed-off-by: Lin Yi
---
drivers/usb/serial/mos7720.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index fc52ac7..6abb335 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -371,12 +371,14 @@ static int write_parport_reg_nonblock(struct
mos7715_parport *mos_parport,
urbtrack->urb = usb_alloc_urb(0, GFP_ATOMIC);
if (!urbtrack->urb) {
kfree(urbtrack);
+ kref_put(&mos_parport->ref_count, destroy_mos_parport);
return -ENOMEM;
}
urbtrack->setup = kmalloc(sizeof(*urbtrack->setup), GFP_ATOMIC);
if (!urbtrack->setup) {
usb_free_urb(urbtrack->urb);
kfree(urbtrack);
+ kref_put(&mos_parport->ref_count, destroy_mos_parport);
return -ENOMEM;
}
urbtrack->setup->bRequestType = (__u8)0x40;
--
1.9.1
Re: [PATCH] [v2]USB:serial:pl2303:add new Pull-Up mode to support PL2303HXD (TYPE_HX)
Hi Greg / Johan, Is there free time to check the patch? Or I have to continue wait. Thanks! Charles Greg KH 於 2019年3月6日 週三 下午1:25寫道： > > On Wed, Mar 06, 2019 at 12:42:29PM +0800, Charles Yeh wrote: > > Hi Johan / Greg, > > > > Do you have time to review the patch? > > If there is no time.. Who else can review this patch? > > Because my boss has been asking me to update the Linux patch... > > It is the middle of the merge window and we can not accept any new > patches until after 5.1-rc1 is out in 1 1/2 weeks. Please wait until > thne before we can get to reviewing this. > > thanks, > > greg k-h
[PATCH] usb: dwc3: pci: add support for Comet Lake PCH ID
This patch simply adds a new PCI Device ID
Signed-off-by: Felipe Balbi
---
drivers/usb/dwc3/dwc3-pci.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index fdc6e4e403e8..8cced3609e24 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -29,6 +29,7 @@
#define PCI_DEVICE_ID_INTEL_BXT_M 0x1aaa
#define PCI_DEVICE_ID_INTEL_APL0x5aaa
#define PCI_DEVICE_ID_INTEL_KBP0xa2b0
+#define PCI_DEVICE_ID_INTEL_CMLH 0x02ee
#define PCI_DEVICE_ID_INTEL_GLK0x31aa
#define PCI_DEVICE_ID_INTEL_CNPLP 0x9dee
#define PCI_DEVICE_ID_INTEL_CNPH 0xa36e
@@ -305,6 +306,9 @@ static const struct pci_device_id dwc3_pci_id_table[] = {
{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_MRFLD),
(kernel_ulong_t) &dwc3_pci_mrfld_properties, },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_CMLH),
+ (kernel_ulong_t) &dwc3_pci_intel_properties, },
+
{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SPTLP),
(kernel_ulong_t) &dwc3_pci_intel_properties, },
--
2.21.0
