RE: [PATCH v4 02/13] dt-bindings: usb: add documentation for typec port controller(TCPCI)
Hi,
> -Original Message-
> From: Mats Karrman [mailto:mats.dev.l...@gmail.com]
> Sent: 2018年4月30日 15:41
> To: Jun Li
> Cc: robh...@kernel.org; gre...@linuxfoundation.org;
> heikki.kroge...@linux.intel.com; li...@roeck-us.net; a.ha...@samsung.com;
> shufan_...@richtek.com; Peter Chen ;
> devicet...@vger.kernel.org; linux-usb@vger.kernel.org; dl-linux-imx
> ; de...@driverdev.osuosl.org
> Subject: Re: [PATCH v4 02/13] dt-bindings: usb: add documentation for typec
> port controller(TCPCI)
>
> Hi Li Jun,
>
> Are you working on an updated version of this patch series?
> I'm pondering other changes that builds on these patches (the documentation
> and the fwnode added to the tcpc_dev and tcpm primarily).
I am on a vacation and will be back tomorrow, I will post a new version soon.
>
> Btw, there is a semi-colon missing in your example below.
Thanks, I will add it.
Li Jun
>
> BR // Mats
>
> On 2018-03-28 18:06, Li Jun wrote:
>
> > TCPCI stands for typec port controller interface, its implementation
> > has full typec port control with power delivery support, it's a
> > standard i2c slave with GPIO input as irq interface, detail see spec
> > "Universal Serial Bus Type-C Port Controller Interface Specification
> > Revision 1.0, Version 1.1"
> >
> > Signed-off-by: Li Jun
> > ---
> > .../devicetree/bindings/usb/typec-tcpci.txt| 33
> ++
> > 1 file changed, 33 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> > b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> > new file mode 100644
> > index 000..7a7a8e0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> > @@ -0,0 +1,33 @@
> > +TCPCI(Typec port cotroller interface) binding
> > +-
> > +
> > +Required properties:
> > +- compatible: should be "usb-tcpci,chip-specific-string".
> > +- reg: the i2c slave address of typec port controller device.
> > +- interrupt-parent: the phandle to the interrupt controller which provides
> > +the interrupt.
> > +- interrupts: interrupt specification for tcpci alert.
> > +
> > +Required sub-node:
> > +- connector: The "usb-c-connector" attached to the tcpci chip, the
> > +bindings
> > + of connector node are specified in
> > + Documentation/devicetree/bindings/connector/usb-connector.txt
> > +
> > +Example:
> > +
> > +ptn5110@50 {
> > + compatible = "usb-tcpci,ptn5110";
> > + reg = <0x50>;
> > + interrupt-parent = <&gpio3>;
> > + interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> > +
> > + usb_con: connector {
> > + compatible = "usb-c-connector";
> > + label = "USB-C";
> > + port-type = "dual";
> > + try-power-role = "sink"
>
> Here!
>
> > + source-pdos = <0x380190c8>;
> > + sink-pdos = <0x380190c8 0x3802d0c8>;
> > + op-sink-microwatt-hours = <900>;
> > + };
> > +};
> >
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
RE: [PATCH v4 01/13] dt-bindings: connector: add properties for typec
Hi > -Original Message- > From: Heikki Krogerus [mailto:heikki.kroge...@linux.intel.com] > Sent: 2018年4月30日 19:24 > To: Jun Li > Cc: robh...@kernel.org; gre...@linuxfoundation.org; li...@roeck-us.net; > a.ha...@samsung.com; shufan_...@richtek.com; Peter Chen > ; devicet...@vger.kernel.org; > linux-usb@vger.kernel.org; dl-linux-imx ; > de...@driverdev.osuosl.org > Subject: Re: [PATCH v4 01/13] dt-bindings: connector: add properties for typec > > Hi, > > On Thu, Mar 29, 2018 at 12:06:06AM +0800, Li Jun wrote: > > +Optional properties for usb-c-connector: > > +- power-type: should be one of "source", "sink" or "dual"(DRP) if typec > > + connector has power support. > > +- try-power-role: preferred power role if "dual"(DRP) can support Try.SNK > > + or Try.SRC, should be "sink" for Try.SNK or "source" for Try.SRC. > > +- data-type: should be one of "host", "device", "dual"(DRD) if typec > > + connector supports USB data. > > Please use "power-role" and "data-role". OK. I will update these 2 property names as you suggested. Li Jun > > > Thanks, > > -- > heikki
Re: [PATCH 1/4] platform: x86: intel_cht_int33fe: Fix dependencies
Hi, On 30-04-18 14:41, Heikki Krogerus wrote: The driver will not probe unless bq24190 is loaded, so making it a dependency. Hmm, the dependency is pure run-time, with this Kconfig change if a user now decides to builtin the intel_cht_int33fe driver, the bq24190 driver must also be builtin, I'm not sure if that is a good thing. Regards, Hans Signed-off-by: Heikki Krogerus Cc: Wolfram Sang Cc: Darren Hart Cc: Andy Shevchenko --- drivers/i2c/busses/Kconfig | 3 +-- drivers/platform/x86/Kconfig | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 8d21b9825d71..fce9f2ca0570 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -202,8 +202,7 @@ config I2C_CHT_WC Note this controller is hooked up to a TI bq24292i charger-IC, combined with a FUSB302 Type-C port-controller as such it is advised - to also select CONFIG_CHARGER_BQ24190=m and CONFIG_TYPEC_FUSB302=m - (the fusb302 driver currently is in drivers/staging). + to also select CONFIG_TYPEC_FUSB302=m. config I2C_NFORCE2 tristate "Nvidia nForce2, nForce3 and nForce4" diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 39d06dd1f63a..1dbd9d547e60 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -866,6 +866,7 @@ config ACPI_CMPC config INTEL_CHT_INT33FE tristate "Intel Cherry Trail ACPI INT33FE Driver" depends on X86 && ACPI && I2C && REGULATOR + depends on CHARGER_BQ24190 ---help--- This driver add support for the INT33FE ACPI device found on some Intel Cherry Trail devices. @@ -877,8 +878,7 @@ config INTEL_CHT_INT33FE i2c drivers for these chips can bind to the them. If you enable this driver it is advised to also select - CONFIG_TYPEC_FUSB302=m, CONFIG_CHARGER_BQ24190=m and - CONFIG_BATTERY_MAX17042=m. + CONFIG_TYPEC_FUSB302=m and CONFIG_BATTERY_MAX17042=m. config INTEL_INT0002_VGPIO tristate "Intel ACPI INT0002 Virtual GPIO driver" -- 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 2/4] usb: typec: tcpm: Release the role mux when exiting
Hi,
On 30-04-18 14:41, Heikki Krogerus wrote:
The ref count for the USB role switch device must be
released after we are done using the switch.
Fixes: c6962c29729c ("usb: typec: tcpm: Set USB role switch to device mode when
configured as such")
Signed-off-by: Heikki Krogerus
Makes sense:
Reviewed-by: Hans de Goede
Regards,
Hans
---
drivers/usb/typec/tcpm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 1ee259bc14a5..66dc0773b9bf 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -4652,6 +4652,7 @@ void tcpm_unregister_port(struct tcpm_port *port)
for (i = 0; i < ARRAY_SIZE(port->port_altmode); i++)
typec_unregister_altmode(port->port_altmode[i]);
typec_unregister_port(port->typec_port);
+ usb_role_switch_put(port->role_sw);
tcpm_debugfs_exit(port);
destroy_workqueue(port->wq);
}
--
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 4/4] usb: roles: intel_xhci: Always allow user control
Hi,
On 30-04-18 14:41, Heikki Krogerus wrote:
Trying to determine the USB port type with this mux is very
difficult. To simplify the situation, always allowing user
control, even if the port is USB Type-C port.
Signed-off-by: Heikki Krogerus > ---
drivers/usb/roles/intel-xhci-usb-role-switch.c | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 28102127b9d5..6482eee6fd45 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -43,15 +43,6 @@ struct intel_xhci_acpi_match {
int hrv;
};
-/*
- * ACPI IDs for PMICs which do not support separate data and power role
- * detection (USB ACA detection for micro USB OTG), we allow userspace to
- * change the role manually on these.
- */
-static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = {
- { "INT33F4", 3 }, /* X-Powers AXP288 PMIC */
-};
-
static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
{
struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
@@ -137,7 +128,6 @@ static int intel_xhci_usb_probe(struct platform_device
*pdev)
struct device *dev = &pdev->dev;
struct intel_xhci_usb_data *data;
struct resource *res;
- int i;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
@@ -150,10 +140,7 @@ static int intel_xhci_usb_probe(struct platform_device
*pdev)
if (!data->base)
return -ENOMEM;
- for (i = 0; i < ARRAY_SIZE(allow_userspace_ctrl_ids); i++)
- if (acpi_dev_present(allow_userspace_ctrl_ids[i].hid, "1",
-allow_userspace_ctrl_ids[i].hrv))
- sw_desc.allow_userspace_control = true;
+ sw_desc.allow_userspace_control = true;
It would be better to add:
.allow_userspace_control = true,
To the initialization of the struct and also make the struct const since
we are now no longer changing it and usb_role_switch_register() accepts
it being const.
With that changed:
Reviewed-by: Hans de Goede
Regards,
Hans
--
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: musb: Support gadget mode when the port is set to dual role
On Mon, Apr 30, 2018 at 11:08:42PM +0200, Paul Kocialkowski wrote: > Hi, > > Le samedi 21 avril 2018 à 09:34 -0500, Bin Liu a écrit : > > Okay, this came down to an argument that whether we should require > > loading a gadget driver on a dual-role port to work in host mode, > > which is currently required on musb since a long long time ago. > > > > I understand the requirement is kinda unnecessary, but since it > > already > > exists on musb stack for a long time, I don't plan to change it. > > Because I > > cannot think of a use case in real products that doesn't automatically > > load a gadget function on the dual-role port. > > > > If you can explain a use case in real world (not a engineering lab) > > that the gadget driver will not be loaded at linux booting up, but > > later based on user's input, I will reconsider my decision. To remove > > this requirement from musb stack, the work is more than this patch. > > My use case here is to support common GNU/Linux-based distributions, not > use-case-specific varieties of GNU/Linux-based rootfs. So my point here > would be that most distros will (and probably should) ship g_ether as a > module but without any particular reason to autoload it, or any other > gadget module in particular, since the system is general-purpose. This is the case I called it "in a engineering lab", not a real product. > Then, imagine a user wants to plug a USB device through OTG (say, > because it's the only USB port available at all on the tablet they're > using), it simply won't work. It won't be obvious to that user that this > is because no gadget is loaded, since what they want to do does not > involve using gadget mode at any point. If a tablet has a dual-role usb port, it is designed to use a gadget driver, which has to be loaded at some point. In the case you described above, when the gadget driver will be loaded? and how? If a gadget driver will never be used, a host-only port should be on the board, not a dual-role port. > Do you think this is a valid use case? It surely is a common one and > perfectly depicts my situation. As I explained above, I don't think so. > Note that in addition to Allwinner devices, I also have omap3/4/5 > devices for testing things. I don't think I have other MUSB-enabled Much more than what I have ;) > devices in my collection though, but I would be willing to test fixes to > this issue on the ones I have. Appreciated it, but someone has to make the patches first. The one you posted might be a good start, but it is not complete. The first problem I see is that musb_start() will be called twice, one in the place you patched, the other is when the gadget driver is bound to the UDC. Regards, -Bin. -- 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: musb: Support gadget mode when the port is set to dual role
Hi, Le mardi 01 mai 2018 à 07:25 -0500, Bin Liu a écrit : > On Mon, Apr 30, 2018 at 11:08:42PM +0200, Paul Kocialkowski wrote: > > Hi, > > > > Le samedi 21 avril 2018 à 09:34 -0500, Bin Liu a écrit : > > > Okay, this came down to an argument that whether we should require > > > loading a gadget driver on a dual-role port to work in host mode, > > > which is currently required on musb since a long long time ago. > > > > > > I understand the requirement is kinda unnecessary, but since it > > > already > > > exists on musb stack for a long time, I don't plan to change it. > > > Because I > > > cannot think of a use case in real products that doesn't > > > automatically > > > load a gadget function on the dual-role port. > > > > > > If you can explain a use case in real world (not a engineering > > > lab) > > > that the gadget driver will not be loaded at linux booting up, but > > > later based on user's input, I will reconsider my decision. To > > > remove > > > this requirement from musb stack, the work is more than this > > > patch. > > > > My use case here is to support common GNU/Linux-based distributions, > > not > > use-case-specific varieties of GNU/Linux-based rootfs. So my point > > here > > would be that most distros will (and probably should) ship g_ether > > as a > > module but without any particular reason to autoload it, or any > > other > > gadget module in particular, since the system is general-purpose. > > This is the case I called it "in a engineering lab", not a real > product. To me, this sounds more like "daily use with upstream like on any laptop/desktop" rather than an engineering lab, but that's not the main point here. > > Then, imagine a user wants to plug a USB device through OTG (say, > > because it's the only USB port available at all on the tablet > > they're > > using), it simply won't work. It won't be obvious to that user that > > this > > is because no gadget is loaded, since what they want to do does not > > involve using gadget mode at any point. > > If a tablet has a dual-role usb port, it is designed to use a gadget > driver, I don't understand the logic behind this assertion. If it has a dual- role USB port, then its hardware allows both use cases. It's obvious that the use case is up to the user of the device since it can be switched by software and is not fixed at design time. > which has to be loaded at some point. In the case you described > above, when the gadget driver will be loaded? and how? Again, loading a gadget driver is not part of the use case. In what I described, the user only wants to use the dual-role port for its host capability and does not care about gadget at all. When the device is plugged into a host, it will simply charge and not propose any USB device features. > If a gadget driver will never be used, a host-only port should be on > the board, not a dual-role port. Here as well, I think the use case is separate from the hardware design. I crafted this patch because I was in the use case I described, with a tablet that only features a micro B USB OTG port. The form factor simply does not allow having a full USB A female host-only port. > > Do you think this is a valid use case? It surely is a common one and > > perfectly depicts my situation. > > As I explained above, I don't think so. I am really surprised that using regular upstream GNU/Linux distributions out of the box is not a valid use case for the MUSB driver. The situation I'm describing is exactly the same as buying a laptop with a preinstalled OS and replacing it with a regular distro. In my case, that's what I did with the tablet (that had an old Android version that did expose gadget features via USB) and I installed upstream Linux and a distro on it. > > Note that in addition to Allwinner devices, I also have omap3/4/5 > > devices for testing things. I don't think I have other MUSB-enabled > > Much more than what I have ;) > > > devices in my collection though, but I would be willing to test > > fixes to > > this issue on the ones I have. > > Appreciated it, but someone has to make the patches first. The one you > posted might be a good start, but it is not complete. The first > problem Oh, I am definitely up for making the changes as well, I mentioned testing to show what level of test coverage I could bring to the table, since this will probably require making sure that it doesn't break specific platforms, glue layers, etc. > I see is that musb_start() will be called twice, one in the place you > patched, the other is when the gadget driver is bound to the UDC. Okay, I will look into this and make sure there is only a single call to musb_start in all scenarios. Are there other things that should be modified as well? Cheers, -- Paul Kocialkowski, developer of free digital technology and hardware support. Website: https://www.paulk.fr/ Coding blog: https://code.paulk.fr/ Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/ signature.asc
Re: [PATCH v2 1/2] usb: misc: xapea00x: add driver for Xaptum ENF Access Card
On Mon, Apr 30, 2018 at 11:05 AM, Greg Kroah-Hartman
wrote:
>
> On Mon, Apr 30, 2018 at 07:54:17AM -0500, David R. Bild wrote:
> > This commit adds a driver for the Xaptum ENF Access Card, a TPM2.0
> > hardware module for authenticating IoT devices and gateways.
> >
> Overall it looks good to me, but you should
> also get the TPM maintainers/developers to review it as I will require
> their review before I can take it in the USB tree.
Thanks for the review.
> > --- /dev/null
> > +++ b/drivers/usb/misc/xapea00x/Kconfig
> > @@ -0,0 +1,16 @@
> > +config USB_XAPEA00X
> > +tristate "Xaptum ENF Access card support (XAP-EA-00x)"
> > +depends on USB_SUPPORT
> > +select SPI
> > +select TCG_TPM
> > +select TCG_TIS_SPI
>
> Do you really want to 'select' these? Why not just depend on them?
Here's my reasoning, but I'm happy to change to "depends" if that
better follows common practice.
The device is a USB device and thus will only work on a machine with a
USB host. That's a property of the machine, so we just "depend" on it.
The SPI and TPM support requirements are internal to the device driver
(the device is SPI master, SPI device, and TPM). This driver needs
those systems enabled, but doesn't require any explicit SPI or TPM
support on the machine (e.g., a physical SPI master). Someone might
want to build a kernel with this driver, but would have no reason to
explicitly enable the SPI or TPM subsystems (after all, the machine
may not have any SPI or TPM hardware). So we "select" those here.
(I can also see using logic to argue for depending on USB_SUPPORT and
TCG_TPM, but selecting SPI and TCG_TIS_SPI...)
In the absence of any other guidance, that was my logic. I'm happy to
change to "depends".
> > +struct xapea00x_device {
> > + struct kref kref;
> > +
> > + struct usb_device *udev;
> > + /*
> > + * The interface pointer will be set NULL when the device
> > + * disconnects. Accessing it safe only while holding the
> > + * usb_mutex.
> > + */
> > + struct usb_interface *interface;
> > + /*
> > + * Th usb_mutex must be held while synchronous USB requests are
> > + * in progress. It is acquired during disconnect to be sure
> > + * that there is not an outstanding request.
> > + */
> > + struct mutex usb_mutex;
> > +
> > + struct usb_endpoint_descriptor *bulk_in;
> > + struct usb_endpoint_descriptor *bulk_out;
> > +
> > + u16 pid;
> > + u16 vid;
>
> Why do you care about these vid/pid values? You set them and never use
> them. If you really needed them, you can get them from the pointer to
> the interface above.
They are used in the TPM initialization code (`xapea00x-tpm.c`).
There are a couple of models of xap-ea-00x with differing
initialization requirements.
Is there a shorter way to get the pid from the usb_device than
__le16_to_cpu(udev->descriptor.idProduct);
?
I found it cleaner to pull that out into a variable once, rather than
fill the TPM-specific code with that very USB-specific code.
Thanks,
David
--
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] musb: fix remote wakeup racing with suspend
Hi,
On Fri, Apr 27, 2018 at 03:00:05PM +0200, Daniel Glöckner wrote:
> It has been observed that writing 0xF2 to the power register while it
> reads as 0xF4 results in the register having the value 0xF0, i.e. clearing
> RESUME and setting SUSPENDM in one go does not work. It might also violate
> the USB spec to transition directly from resume to suspend, especially
> when not taking T_DRSMDN into account. But this is what happens when a
> remote wakeup occurs between SetPortFeature USB_PORT_FEAT_SUSPEND on the
> root hub and musb_bus_suspend being called.
>
> This commit returns -EBUSY when musb_bus_suspend is called while remote
> wakeup is signalled and thus avoids to reset the RESUME bit. Remember that
> resume can be signalled only when the bus was already suspended, so it is
> safe to skip the following steps even when musb_hub_control ignores the
what do you mean by "skip the following steps"?
> error returned by musb_port_suspend.
>
> The resume part of musb_port_suspend is modified to also accept a pending
> remote wakeup, to bring it to the end after T_DRSMDN has passed.
Can you please explain more here? I am not sure when musb_port_suspend()
is involved in resume by remote wakeup, and what case is a 'pending'
remote wakeup?
>
> Signed-off-by: Daniel Glöckner
> ---
> drivers/usb/musb/musb_host.c| 5 -
> drivers/usb/musb/musb_host.h| 7 +--
> drivers/usb/musb/musb_virthub.c | 27 ---
> 3 files changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> index 3a8451a..d05cb03 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -2522,8 +2522,11 @@ static int musb_bus_suspend(struct usb_hcd *hcd)
> {
> struct musb *musb = hcd_to_musb(hcd);
> u8 devctl;
> + int ret;
>
> - musb_port_suspend(musb, true);
> + ret = musb_port_suspend(musb, true);
> + if (ret)
> + return ret;
>
> if (!is_host_active(musb))
> return 0;
> diff --git a/drivers/usb/musb/musb_host.h b/drivers/usb/musb/musb_host.h
> index 72392bb..2999845 100644
> --- a/drivers/usb/musb/musb_host.h
> +++ b/drivers/usb/musb/musb_host.h
> @@ -67,7 +67,7 @@ extern void musb_host_rx(struct musb *, u8);
> extern void musb_root_disconnect(struct musb *musb);
> extern void musb_host_resume_root_hub(struct musb *musb);
> extern void musb_host_poke_root_hub(struct musb *musb);
> -extern void musb_port_suspend(struct musb *musb, bool do_suspend);
> +extern int musb_port_suspend(struct musb *musb, bool do_suspend);
> extern void musb_port_reset(struct musb *musb, bool do_reset);
> extern void musb_host_finish_resume(struct work_struct *work);
> #else
> @@ -99,7 +99,10 @@ static inline void musb_root_disconnect(struct musb *musb)
> {}
> static inline void musb_host_resume_root_hub(struct musb *musb) {}
> static inline void musb_host_poll_rh_status(struct musb *musb) {}
> static inline void musb_host_poke_root_hub(struct musb *musb){}
> -static inline void musb_port_suspend(struct musb *musb, bool do_suspend) {}
> +static inline int musb_port_suspend(struct musb *musb, bool do_suspend)
> +{
> + return 0;
> +}
> static inline void musb_port_reset(struct musb *musb, bool do_reset) {}
> static inline void musb_host_finish_resume(struct work_struct *work) {}
> #endif
> diff --git a/drivers/usb/musb/musb_virthub.c b/drivers/usb/musb/musb_virthub.c
> index 5165d2b..41b44ce 100644
> --- a/drivers/usb/musb/musb_virthub.c
> +++ b/drivers/usb/musb/musb_virthub.c
> @@ -48,14 +48,14 @@ void musb_host_finish_resume(struct work_struct *work)
> spin_unlock_irqrestore(&musb->lock, flags);
> }
>
> -void musb_port_suspend(struct musb *musb, bool do_suspend)
> +int musb_port_suspend(struct musb *musb, bool do_suspend)
> {
> struct usb_otg *otg = musb->xceiv->otg;
> u8 power;
> void __iomem*mbase = musb->mregs;
>
> if (!is_host_active(musb))
> - return;
> + return 0;
>
> /* NOTE: this doesn't necessarily put PHY into low power mode,
>* turning off its clock; that's a function of PHY integration and
> @@ -66,16 +66,20 @@ void musb_port_suspend(struct musb *musb, bool do_suspend)
> if (do_suspend) {
> int retries = 1;
>
> - power &= ~MUSB_POWER_RESUME;
> - power |= MUSB_POWER_SUSPENDM;
> - musb_writeb(mbase, MUSB_POWER, power);
> + if (power & MUSB_POWER_RESUME)
> + return -EBUSY;
>
> - /* Needed for OPT A tests */
> - power = musb_readb(mbase, MUSB_POWER);
> - while (power & MUSB_POWER_SUSPENDM) {
> + if (!(power & MUSB_POWER_SUSPENDM)) {
> + power |= MUSB_POWER_SUSPENDM;
> + musb_writeb(mbase, MUSB_POWER, power);
> +
> +
Re: [RFC 01/10] ARM: dts: exynos: Remove Exynos5440
On Tue, Apr 24, 2018 at 10:32:30PM +0200, Krzysztof Kozlowski wrote: > The Exynos5440 (quad-core A15 with GMAC, PCIe, SATA) was targeting > server platforms but it did not make it to the market really. There are > no development boards with it and probably there are no real products > neither. The development for Exynos5440 ended in 2013 and since then > the platform is in maintenance mode. > > Remove all Device Tree sources for Exynos5440, as first step of removal > of the platform to simplify the code and drivers. > > Signed-off-by: Krzysztof Kozlowski > --- > .../bindings/arm/samsung/samsung-boards.txt| 2 - > arch/arm/boot/dts/Makefile | 2 - > arch/arm/boot/dts/exynos5440-sd5v1.dts | 42 --- > arch/arm/boot/dts/exynos5440-ssdk5440.dts | 81 - > arch/arm/boot/dts/exynos5440-tmu-sensor-conf.dtsi | 20 -- > arch/arm/boot/dts/exynos5440-trip-points.dtsi | 21 -- > arch/arm/boot/dts/exynos5440.dtsi | 355 > - > 7 files changed, 523 deletions(-) > delete mode 100644 arch/arm/boot/dts/exynos5440-sd5v1.dts > delete mode 100644 arch/arm/boot/dts/exynos5440-ssdk5440.dts > delete mode 100644 arch/arm/boot/dts/exynos5440-tmu-sensor-conf.dtsi > delete mode 100644 arch/arm/boot/dts/exynos5440-trip-points.dtsi > delete mode 100644 arch/arm/boot/dts/exynos5440.dtsi Reviewed-by: Rob Herring -- 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 02/10] ata: ahci-platform: Remove support for Exynos5440
On Tue, Apr 24, 2018 at 10:32:31PM +0200, Krzysztof Kozlowski wrote: > The Exynos5440 is not actively developed, there are no development > boards available and probably there are no real products with it. > Remove wide-tree support for Exynos5440. > > Signed-off-by: Krzysztof Kozlowski > --- > Documentation/devicetree/bindings/ata/ahci-platform.txt | 1 - > drivers/ata/ahci_platform.c | 1 - > 2 files changed, 2 deletions(-) Reviewed-by: Rob Herring -- 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 1/2] usb: dwc3: use local copy of resource to fix-up register offset
Hi Felipe,
2018-04-19 20:03 GMT+09:00 Masahiro Yamada :
> It is not a good idea to directly modify the resource of a platform
> device. Modify its local copy, and pass it to devm_ioremap_resource()
> so that we do not need to restore it in the failure path and the remove
> hook.
>
> Signed-off-by: Masahiro Yamada
> Reviewed-by: Masami Hiramatsu
I want this patch applied first
unless you are opposed to this clean-up.
I'd like to avoid re-sending a trivial patch like this.
> ---
>
> Changes in v2: None
>
> drivers/usb/dwc3/core.c | 32
> 1 file changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index a15648d..8e66edd 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1245,7 +1245,7 @@ static void dwc3_check_params(struct dwc3 *dwc)
> static int dwc3_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> - struct resource *res;
> + struct resource *res, dwc_res;
> struct dwc3 *dwc;
>
> int ret;
> @@ -1270,20 +1270,19 @@ static int dwc3_probe(struct platform_device *pdev)
> dwc->xhci_resources[0].flags = res->flags;
> dwc->xhci_resources[0].name = res->name;
>
> - res->start += DWC3_GLOBALS_REGS_START;
> -
> /*
> * Request memory region but exclude xHCI regs,
> * since it will be requested by the xhci-plat driver.
> */
> - regs = devm_ioremap_resource(dev, res);
> - if (IS_ERR(regs)) {
> - ret = PTR_ERR(regs);
> - goto err0;
> - }
> + dwc_res = *res;
> + dwc_res.start += DWC3_GLOBALS_REGS_START;
> +
> + regs = devm_ioremap_resource(dev, &dwc_res);
> + if (IS_ERR(regs))
> + return PTR_ERR(regs);
>
> dwc->regs = regs;
> - dwc->regs_size = resource_size(res);
> + dwc->regs_size = resource_size(&dwc_res);
>
> dwc3_get_properties(dwc);
>
> @@ -1350,29 +1349,14 @@ static int dwc3_probe(struct platform_device *pdev)
> pm_runtime_put_sync(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
>
> -err0:
> - /*
> -* restore res->start back to its original value so that, in case the
> -* probe is deferred, we don't end up getting error in request the
> -* memory region the next time probe is called.
> -*/
> - res->start -= DWC3_GLOBALS_REGS_START;
> -
> return ret;
> }
>
> static int dwc3_remove(struct platform_device *pdev)
> {
> struct dwc3 *dwc = platform_get_drvdata(pdev);
> - struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> pm_runtime_get_sync(&pdev->dev);
> - /*
> -* restore res->start back to its original value so that, in case the
> -* probe is deferred, we don't end up getting error in request the
> -* memory region the next time probe is called.
> -*/
> - res->start -= DWC3_GLOBALS_REGS_START;
>
> dwc3_debugfs_exit(dwc);
> dwc3_core_exit_mode(dwc);
> --
> 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
--
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 03/10] cpufreq: exynos: Remove support for Exynos5440
On Tue, Apr 24, 2018 at 10:32:32PM +0200, Krzysztof Kozlowski wrote: > The Exynos5440 is not actively developed, there are no development > boards available and probably there are no real products with it. > Remove wide-tree support for Exynos5440. > > Signed-off-by: Krzysztof Kozlowski > --- > .../bindings/cpufreq/cpufreq-exynos5440.txt| 28 -- > drivers/cpufreq/Kconfig.arm| 14 - > drivers/cpufreq/Makefile | 1 - > drivers/cpufreq/exynos5440-cpufreq.c | 452 > - > 4 files changed, 495 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/cpufreq/cpufreq-exynos5440.txt > delete mode 100644 drivers/cpufreq/exynos5440-cpufreq.c Reviewed-by: Rob Herring -- 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 1/2] dt-bindings: add MediaTek XS-PHY binding
On Wed, Apr 25, 2018 at 03:45:28PM +0800, Chunfeng Yun wrote:
> Add a DT binding documentation of XS-PHY for MediaTek SoCs
> with USB3.1 GEN2 controller
>
> Signed-off-by: Chunfeng Yun
> ---
> .../devicetree/bindings/phy/phy-mtk-xsphy.txt | 127
>
> 1 file changed, 127 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt
>
> diff --git a/Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt
> b/Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt
> new file mode 100644
> index 000..23c51a0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt
> @@ -0,0 +1,127 @@
> +MediaTek XS-PHY binding
> +--
> +
> +The XS-PHY controller supports physical layer functionality for USB3.1
> +GEN2 controller on MediaTek SoCs.
> +
> +Required properties (controller (parent) node):
> + - compatible: should be "mediatek,-xsphy",
> "mediatek,xsphy",
> + soc-model is the name of SoC, such as mt2712 etc;
Please list all valid SoCs.
> + when using "mediatek,xsphy" compatible string, you need SoC
> specific
> + ones in addition.
> +
> +Required nodes : a sub-node is required for each port the controller
> + provides. Address range information including the usual
> + 'reg' property is used inside these nodes to describe
> + the controller's topology.
This should go afer the parent optional properties.
> +
> +Optional properties (controller (parent) node):
> + - reg : offset and length of register shared by multiple U3
> ports,
> + exclude port's private register, if only U2 ports provided,
> + shouldn't use the property.
> + - mediatek,src-ref-clk-mhz : u32, frequency of reference clock for slew
> rate
> + calibrate
> + - mediatek,src-coef : u32, coefficient for slew rate calibrate, depends on
> + SoC process
> +
> +Required properties (port (child) node):
> +- reg: address and length of the register set for the port.
> +- clocks : a list of phandle + clock-specifier pairs, one for each
> + entry in clock-names
> +- clock-names: must contain
> + "ref": 48M reference clock for HighSpeed analog phy; and 26M
> + reference clock for SuperSpeedPlus analog phy,
> sometimes is
> + 24M, 25M or 27M, depended on platform.
> +- #phy-cells : should be 1 (See second example)
> + cell after port phandle is phy type from:
> + - PHY_TYPE_USB2
> + - PHY_TYPE_USB3
Is the type a property of the connection to the host? If not, this
shouldn't be part of the phy cells. I would add compatible strings to
the child nodes to define this. I think that would simplify the driver
too as then you can parse DT properties in probe rather than phy_init.
> +
> +The following optional properties are only for debug or HQA test
> +Optional properties (PHY_TYPE_USB2 port (child) node):
> +- mediatek,eye-src : u32, the value of slew rate calibrate
> +- mediatek,eye-vrt : u32, the selection of VRT reference voltage
> +- mediatek,eye-term : u32, the selection of HS_TX TERM reference voltage
> +- mediatek,efuse-intr: u32, the selection of Internal Resistor
> +
> +Optional properties (PHY_TYPE_USB3 port (child) node):
> +- mediatek,efuse-intr: u32, the selection of Internal Resistor
> +- mediatek,efuse-tx-imp : u32, the selection of TX Impedance
> +- mediatek,efuse-rx-imp : u32, the selection of RX Impedance
> +
> +Example:
> +
> +u3phy: usb-phy@11c4 {
> + compatible = "mediatek,mt36xx-xsphy", "mediatek,xsphy";
Is mt36xx a specific SoC? Don't use wildcards in compatible strings.
> + reg = <0 0x11c43000 0 0x0200>;
> + mediatek,src-ref-clk-mhz = <26>;
> + mediatek,src-coef = <17>;
> + #address-cells = <2>;
> + #size-cells = <2>;
Really need 64-bit sizes?
> + ranges;
Limiting the range to only the phy addresses is preferred.
> + status = "disabled";
Don't show status in examples.
> +
> + u2port0: usb-phy@11c4 {
> + reg = <0 0x11c4 0 0x0400>;
> + clocks = <&clk48m>;
> + clock-names = "ref";
> + mediatek,eye-src = <4>;
> + #phy-cells = <1>;
> + status = "okay";
> + };
> +
> + u3port0: usb-phy@11c43000 {
> + reg = <0 0x11c43400 0 0x0500>;
> + clocks = <&clk26m>;
> + clock-names = "ref";
> + mediatek,efuse-intr = <28>;
> + #phy-cells = <1>;
> + status = "okay";
> + };
> +};
> +
> +Specifying phy control of devices
> +-
> +
> +Device nodes should specify the configuration required in their "phys"
> +property, containing a phandle to the phy port node and a de
Re: [PATCH] thermal: samsung: Remove support for Exynos5440
On Thu, Apr 26, 2018 at 01:21:12PM +0200, Bartlomiej Zolnierkiewicz wrote: > From: Krzysztof Kozlowski > Subject: [PATCH] thermal: samsung: Remove support for Exynos5440 > > The Exynos5440 is not actively developed, there are no development > boards available and probably there are no real products with it. > Remove wide-tree support for Exynos5440. > > Signed-off-by: Krzysztof Kozlowski > [b.zolnierkie: ported over driver changes] > Signed-off-by: Bartlomiej Zolnierkiewicz > --- > Eduardo, here is a version which applies on top of pending Exynos > thermal changes (https://lkml.org/lkml/2018/4/16/256). > > Documentation/devicetree/bindings/thermal/exynos-thermal.txt | 14 > drivers/thermal/samsung/exynos_tmu.c | 161 > --- > 2 files changed, 4 insertions(+), 171 deletions(-) Reviewed-by: Rob Herring -- 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: musb: Support gadget mode when the port is set to dual role
On Tue, May 01, 2018 at 03:26:57PM +0200, Paul Kocialkowski wrote: > Hi, > > Le mardi 01 mai 2018 à 07:25 -0500, Bin Liu a écrit : > > On Mon, Apr 30, 2018 at 11:08:42PM +0200, Paul Kocialkowski wrote: > > > Hi, > > > > > > Le samedi 21 avril 2018 à 09:34 -0500, Bin Liu a écrit : > > > > Okay, this came down to an argument that whether we should require > > > > loading a gadget driver on a dual-role port to work in host mode, > > > > which is currently required on musb since a long long time ago. > > > > > > > > I understand the requirement is kinda unnecessary, but since it > > > > already > > > > exists on musb stack for a long time, I don't plan to change it. > > > > Because I > > > > cannot think of a use case in real products that doesn't > > > > automatically > > > > load a gadget function on the dual-role port. > > > > > > > > If you can explain a use case in real world (not a engineering > > > > lab) > > > > that the gadget driver will not be loaded at linux booting up, but > > > > later based on user's input, I will reconsider my decision. To > > > > remove > > > > this requirement from musb stack, the work is more than this > > > > patch. > > > > > > My use case here is to support common GNU/Linux-based distributions, > > > not > > > use-case-specific varieties of GNU/Linux-based rootfs. So my point > > > here > > > would be that most distros will (and probably should) ship g_ether > > > as a > > > module but without any particular reason to autoload it, or any > > > other > > > gadget module in particular, since the system is general-purpose. > > > > This is the case I called it "in a engineering lab", not a real > > product. > > To me, this sounds more like "daily use with upstream like on any > laptop/desktop" rather than an engineering lab, but that's not the main > point here. > > > > Then, imagine a user wants to plug a USB device through OTG (say, > > > because it's the only USB port available at all on the tablet > > > they're > > > using), it simply won't work. It won't be obvious to that user that > > > this > > > is because no gadget is loaded, since what they want to do does not > > > involve using gadget mode at any point. > > > > If a tablet has a dual-role usb port, it is designed to use a gadget > > driver, > > I don't understand the logic behind this assertion. If it has a dual- > role USB port, then its hardware allows both use cases. It's obvious > that the use case is up to the user of the device since it can be > switched by software and is not fixed at design time. My view is the whole (embedded) system, not just Linux itself. If the hardware designs a dual-role port, a gadget driver has to be used. Otherwise, define the port as host-only, either in the hardware design, or at least in device tree. > > which has to be loaded at some point. In the case you described > > above, when the gadget driver will be loaded? and how? > > Again, loading a gadget driver is not part of the use case. In what I > described, the user only wants to use the dual-role port for its host > capability and does not care about gadget at all. When the device is > plugged into a host, it will simply charge and not propose any USB > device features. It sounds to me a hacking to an existing product, not designing a new product. If so, please hack it completely, define the port dr_mode to host in the board device tree, then the port should work for you. > > If a gadget driver will never be used, a host-only port should be on > > the board, not a dual-role port. > > Here as well, I think the use case is separate from the hardware design. > I crafted this patch because I was in the use case I described, with a > tablet that only features a micro B USB OTG port. The form factor simply I guess you meant micro-AB port, microB doesn't have an ID pin, cannot make MUSB to work in host mode. > does not allow having a full USB A female host-only port. > > > > Do you think this is a valid use case? It surely is a common one and > > > perfectly depicts my situation. > > > > As I explained above, I don't think so. > > I am really surprised that using regular upstream GNU/Linux > distributions out of the box is not a valid use case for the MUSB > driver. The situation I'm describing is exactly the same as buying a > laptop with a preinstalled OS and replacing it with a regular distro. In > my case, that's what I did with the tablet (that had an old Android > version that did expose gadget features via USB) and I installed > upstream Linux and a distro on it. Embedded system is different than PC, I don't expect to just drop in a distro without any modification to work, especially in this case you change the function of the product originally designed - dual-role port to host-only port. You would have to at least modify the board device tree for your new purpose. > > > > Note that in addition to Allwinner devices, I also have omap3/4/5 > > > devices for testing things. I don't think I have othe
Re: [RFC 04/10] clk: samsung: Remove support for Exynos5440
Quoting Krzysztof Kozlowski (2018-04-24 13:32:33) > The Exynos5440 is not actively developed, there are no development > boards available and probably there are no real products with it. > Remove wide-tree support for Exynos5440. > > Signed-off-by: Krzysztof Kozlowski > --- Acked-by: Stephen Boyd -- 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
[RFC PATCH 4/7] dt-bindings: usb: Add general documentation for USB Type-C muxes
This patch adds documentation of properties allowing the supported
set of mux modes to be restricted in situations where a bad mode
selection may have negative effects.
Signed-off-by: Mats Karrman
---
Documentation/devicetree/bindings/usb/typec-mux.txt | 18 ++
1 file changed, 18 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/typec-mux.txt
diff --git a/Documentation/devicetree/bindings/usb/typec-mux.txt
b/Documentation/devicetree/bindings/usb/typec-mux.txt
new file mode 100644
index 000..1e93973
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/typec-mux.txt
@@ -0,0 +1,18 @@
+USB Type-C Multiplexer/DeMultiplexer Switch
+
+Optional properties:
+ - have-2ch-usbss : The hardware has support for 2ch USB SS.
+ - have-4ch-am : The hardware has support for 4ch Alternate Mode.
+ - have-2ch-usbss-2ch-am : The hardware has support for 2ch USB SS + 2ch
+ Alternate Mode.
+ - have-4ch-usbss : The hardware has support for 4ch USB SS.
+ - have-2ch-usbss-2ch-am-b : The hardware has support for 2ch USB SS + 2ch
+ Alternate Mode, alternate pair.
+
+Example :
+pi3usb30532@00 {
+ compatible = "pericom,pi3usb30532";
+ reg = <0x00>;
+ have-2ch-usbss;
+ have-4ch-am;
+};
--
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
[RFC PATCH 2/7] usb: typec: Move mux mode type definition to limit dependency
To avoid unnecessary dependencies on tcpm.h and to pave the way
for comming patches, move the tcpc_mux_mode enum from tcpm.h to
typec.h and change its name to typec_mux_mode (the enum constants
already follow this naming).
Signed-off-by: Mats Karrman
---
drivers/usb/typec/mux/pi3usb30532.c | 6 +++---
drivers/usb/typec/tcpm.c| 2 +-
include/linux/usb/tcpm.h| 12
include/linux/usb/typec.h | 11 +++
include/linux/usb/typec_mux.h | 4 ++--
5 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/typec/mux/pi3usb30532.c
b/drivers/usb/typec/mux/pi3usb30532.c
index 279f3c3..d995883 100644
--- a/drivers/usb/typec/mux/pi3usb30532.c
+++ b/drivers/usb/typec/mux/pi3usb30532.c
@@ -9,7 +9,7 @@
#include
#include
#include
-#include
+#include
#include
#define PI3USB30532_CONF 0x00
@@ -73,7 +73,7 @@ static int pi3usb30532_sw_set(struct typec_switch *sw,
return ret;
}
-static int pi3usb30532_mux_set(struct typec_mux *mux, int state)
+static int pi3usb30532_mux_set(struct typec_mux *mux, enum typec_mux_mode mode)
{
struct pi3usb30532 *pi = container_of(mux, struct pi3usb30532, mux);
u8 new_conf;
@@ -82,7 +82,7 @@ static int pi3usb30532_mux_set(struct typec_mux *mux, int
state)
mutex_lock(&pi->lock);
new_conf = pi->conf;
- switch (state) {
+ switch (mode) {
default:
case TYPEC_MUX_NONE:
new_conf = PI3USB30532_CONF_OPEN;
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 0451ea0..aaf6d57 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -644,7 +644,7 @@ void tcpm_pd_transmit_complete(struct tcpm_port *port,
}
EXPORT_SYMBOL_GPL(tcpm_pd_transmit_complete);
-static int tcpm_mux_set(struct tcpm_port *port, enum tcpc_mux_mode mode,
+static int tcpm_mux_set(struct tcpm_port *port, enum typec_mux_mode mode,
enum usb_role usb_role,
enum typec_orientation orientation)
{
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index 3518965..e641e21 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -93,18 +93,6 @@ struct tcpc_config {
const struct typec_altmode_desc *alt_modes;
};
-/* Mux modes */
-enum tcpc_mux_mode {
- TYPEC_MUX_NONE, /* Open switch */
- TYPEC_MUX_2CH_USBSS,/* 2ch USB SS */
- TYPEC_MUX_4CH_AM, /* 4ch Alt Mode */
- TYPEC_MUX_2CH_USBSS_2CH_AM, /* 2ch USB SS + 2ch Alt Mode */
-
- // Example of additional modes that may be needed in future:
- TYPEC_MUX_4CH_USBSS,/* 4ch USB SS */
- TYPEC_MUX_2CH_USBSS_2CH_AM_B, /* 2ch USB SS + 2ch Alt Mode
(e.g. DP GPU2) */
-};
-
/**
* struct tcpc_dev - Port configuration and callback functions
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 672b39b..d356577 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -72,6 +72,17 @@ enum typec_orientation {
TYPEC_ORIENTATION_REVERSE,
};
+enum typec_mux_mode {
+ TYPEC_MUX_NONE, /* Open switch */
+ TYPEC_MUX_2CH_USBSS,/* 2ch USB SS */
+ TYPEC_MUX_4CH_AM, /* 4ch Alt Mode */
+ TYPEC_MUX_2CH_USBSS_2CH_AM, /* 2ch USB SS + 2ch Alt Mode */
+
+ // Example of additional modes that may be needed in future:
+ TYPEC_MUX_4CH_USBSS,/* 4ch USB SS */
+ TYPEC_MUX_2CH_USBSS_2CH_AM_B, /* 2ch USB SS + 2ch Alt Mode
(e.g. DP GPU2) */
+};
+
/*
* struct usb_pd_identity - USB Power Delivery identity data
* @id_header: ID Header VDO
diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
index 12c1b05..e44137d 100644
--- a/include/linux/usb/typec_mux.h
+++ b/include/linux/usb/typec_mux.h
@@ -29,7 +29,7 @@ struct typec_switch {
* struct typec_switch - USB Type-C connector pin mux
* @dev: Mux device
* @entry: List entry
- * @set: Callback to the driver for setting the state of the mux
+ * @set: Callback to the driver for setting the mode of the mux
*
* Pin Multiplexer/DeMultiplexer switch routing the USB Type-C connector pins
to
* different components depending on the requested mode of operation. Used with
@@ -39,7 +39,7 @@ struct typec_mux {
struct device *dev;
struct list_head entry;
- int (*set)(struct typec_mux *mux, int state);
+ int (*set)(struct typec_mux *mux, enum typec_mux_mode mode);
};
struct typec_switch *typec_switch_get(struct device *dev);
--
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
[RFC PATCH 6/7] dt-bindings: usb: typec-mux: Add property for default mux mode
For some devices it is necessary to specify a different initial
mux mode to use after connection. E.g. some devices may not have
USB SS support but just USB HS and an alternate mode and thus
prefer TYPEC_MUX_NONE as default mode.
Signed-off-by: Mats Karrman
---
Documentation/devicetree/bindings/usb/typec-mux.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/typec-mux.txt
b/Documentation/devicetree/bindings/usb/typec-mux.txt
index 1e93973..ca960fc 100644
--- a/Documentation/devicetree/bindings/usb/typec-mux.txt
+++ b/Documentation/devicetree/bindings/usb/typec-mux.txt
@@ -8,6 +8,7 @@ Optional properties:
- have-4ch-usbss : The hardware has support for 4ch USB SS.
- have-2ch-usbss-2ch-am-b : The hardware has support for 2ch USB SS + 2ch
Alternate Mode, alternate pair.
+ - default-mux-mode: Mux mode to use after initial connection.
Example :
pi3usb30532@00 {
@@ -15,4 +16,5 @@ pi3usb30532@00 {
reg = <0x00>;
have-2ch-usbss;
have-4ch-am;
+ default-mux-mode = <2ch-usbss>;
};
--
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
[RFC PATCH 1/7] usb: typec: Generalize mux mode names
The current naming used for tcpc_mux_mode constants makes
too much assumptioms about the usage of the signals.
This patch replaces the names with generic names more closely
tied to the Type-C specifications and also adds some new ones.
At the same time TCPC_MUX_* defines are removed as they do not
fit the new concept and currently have no in-tree users.
Signed-off-by: Mats Karrman
---
drivers/usb/typec/mux/pi3usb30532.c | 7 ---
drivers/usb/typec/tcpm.c| 2 +-
include/linux/usb/tcpm.h| 21 ++---
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/typec/mux/pi3usb30532.c
b/drivers/usb/typec/mux/pi3usb30532.c
index b0e88db..279f3c3 100644
--- a/drivers/usb/typec/mux/pi3usb30532.c
+++ b/drivers/usb/typec/mux/pi3usb30532.c
@@ -83,18 +83,19 @@ static int pi3usb30532_mux_set(struct typec_mux *mux, int
state)
new_conf = pi->conf;
switch (state) {
+ default:
case TYPEC_MUX_NONE:
new_conf = PI3USB30532_CONF_OPEN;
break;
- case TYPEC_MUX_USB:
+ case TYPEC_MUX_2CH_USBSS:
new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
PI3USB30532_CONF_USB3;
break;
- case TYPEC_MUX_DP:
+ case TYPEC_MUX_4CH_AM:
new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
PI3USB30532_CONF_4LANE_DP;
break;
- case TYPEC_MUX_DOCK:
+ case TYPEC_MUX_2CH_USBSS_2CH_AM:
new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
PI3USB30532_CONF_USB3_AND_2LANE_DP;
break;
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 7ee417a..0451ea0 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -790,7 +790,7 @@ static int tcpm_set_roles(struct tcpm_port *port, bool
attached,
else
usb_role = USB_ROLE_DEVICE;
- ret = tcpm_mux_set(port, TYPEC_MUX_USB, usb_role, orientation);
+ ret = tcpm_mux_set(port, TYPEC_MUX_2CH_USBSS, usb_role, orientation);
if (ret < 0)
return ret;
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index b231b93..3518965 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -93,20 +93,19 @@ struct tcpc_config {
const struct typec_altmode_desc *alt_modes;
};
-/* Mux state attributes */
-#define TCPC_MUX_USB_ENABLED BIT(0) /* USB enabled */
-#define TCPC_MUX_DP_ENABLEDBIT(1) /* DP enabled */
-#define TCPC_MUX_POLARITY_INVERTED BIT(2) /* Polarity inverted */
-
-/* Mux modes, decoded to attributes */
+/* Mux modes */
enum tcpc_mux_mode {
- TYPEC_MUX_NONE = 0,/* Open switch */
- TYPEC_MUX_USB = TCPC_MUX_USB_ENABLED, /* USB only */
- TYPEC_MUX_DP= TCPC_MUX_DP_ENABLED, /* DP only */
- TYPEC_MUX_DOCK = TCPC_MUX_USB_ENABLED |/* Both USB and DP */
- TCPC_MUX_DP_ENABLED,
+ TYPEC_MUX_NONE, /* Open switch */
+ TYPEC_MUX_2CH_USBSS,/* 2ch USB SS */
+ TYPEC_MUX_4CH_AM, /* 4ch Alt Mode */
+ TYPEC_MUX_2CH_USBSS_2CH_AM, /* 2ch USB SS + 2ch Alt Mode */
+
+ // Example of additional modes that may be needed in future:
+ TYPEC_MUX_4CH_USBSS,/* 4ch USB SS */
+ TYPEC_MUX_2CH_USBSS_2CH_AM_B, /* 2ch USB SS + 2ch Alt Mode
(e.g. DP GPU2) */
};
+
/**
* struct tcpc_dev - Port configuration and callback functions
* @config:Pointer to port configuration
--
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
[RFC PATCH 0/7] Fixe some issues with Type-C muxes
This series is based on usb-next commit 252427037a36 ("typec: tcpm: Fix
incorrect 'and' operator").
In my work trying to adapt my project to the latest kernel I have ran into
a number of issues. Consider the following series an initial poll on the
acceptance of the solutions I have found.
In short the issues are as follows:
- TYPEC_MUX_ constants have a lot of prejudice in their names. E.g. other
alternate modes than DisplayPort may appear as well as other combinations.
- It should be possible to limit the configurations the mux driver is allowed
to set, in case the hardware does not support all possible modes of a
specific mux device/driver.
- Some devices may need a default mux mode after connection different than
2ch USBSS, e.g. if the port does not support USBSS but only AM, then the
default mode could be set to _NONE or if the port supports _4CH_USBSS maybe
that should be the default.
Mats Karrman (7):
usb: typec: Generalize mux mode names
usb: typec: Move mux mode type definition to limit dependency
usb: typec: Add API to find mux mode from its string representation
dt-bindings: usb: Add general documentation for USB Type-C muxes
usb: typec: mux: pi3usb30532: Add support for supported mode
properties
dt-bindings: usb: typec-mux: Add property for default mux mode
usb: typec: mux: Allow default mux mode to be specified
.../devicetree/bindings/usb/typec-mux.txt | 20
drivers/usb/typec/class.c | 22
drivers/usb/typec/mux/pi3usb30532.c| 60 +-
drivers/usb/typec/tcpm.c | 4 +-
include/linux/usb/tcpm.h | 13 -
include/linux/usb/typec.h | 13 +
include/linux/usb/typec_mux.h | 4 +-
7 files changed, 107 insertions(+), 29 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/typec-mux.txt
--
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
[RFC PATCH 3/7] usb: typec: Add API to find mux mode from its string representation
Signed-off-by: Mats Karrman
---
drivers/usb/typec/class.c | 22 ++
include/linux/usb/typec.h | 1 +
2 files changed, 23 insertions(+)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 53df10d..c5432f4 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1255,6 +1255,28 @@ EXPORT_SYMBOL_GPL(typec_set_pwr_opmode);
/* -- */
/* API for Multiplexer/DeMultiplexer Switches */
+static const char * const typec_mux_modes[] = {
+ [TYPEC_MUX_NONE] = "none",
+ [TYPEC_MUX_2CH_USBSS] = "2ch-usbss",
+ [TYPEC_MUX_4CH_AM] = "4ch-am",
+ [TYPEC_MUX_2CH_USBSS_2CH_AM] = "2ch-usbss-2ch-am",
+ [TYPEC_MUX_4CH_USBSS] = "4ch-usbss",
+ [TYPEC_MUX_2CH_USBSS_2CH_AM_B] = "2ch-usbss-2ch-am-b",
+};
+
+/**
+ * typec_find_mux_mode - Get the typec mux mode from its string representation
+ * @name: mux mode string
+ *
+ * Returns typec_mux_mode if success, otherwise negative error code.
+ */
+int typec_find_mux_mode(const char *name)
+{
+ return match_string(typec_mux_modes, ARRAY_SIZE(typec_mux_modes),
+ name);
+}
+EXPORT_SYMBOL_GPL(typec_find_mux_mode);
+
/**
* typec_set_orientation - Set USB Type-C cable plug orientation
* @port: USB Type-C Port
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index d356577..72cd4a7 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -274,6 +274,7 @@ void typec_set_pwr_role(struct typec_port *port, enum
typec_role role);
void typec_set_vconn_role(struct typec_port *port, enum typec_role role);
void typec_set_pwr_opmode(struct typec_port *port, enum typec_pwr_opmode mode);
+int typec_find_mux_mode(const char *name);
int typec_set_orientation(struct typec_port *port,
enum typec_orientation orientation);
int typec_set_mode(struct typec_port *port, int mode);
--
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
[RFC PATCH 5/7] usb: typec: mux: pi3usb30532: Add support for supported mode properties
Signed-off-by: Mats Karrman
---
drivers/usb/typec/mux/pi3usb30532.c | 29 +++--
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/typec/mux/pi3usb30532.c
b/drivers/usb/typec/mux/pi3usb30532.c
index d995883..3e564e6 100644
--- a/drivers/usb/typec/mux/pi3usb30532.c
+++ b/drivers/usb/typec/mux/pi3usb30532.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -25,6 +26,7 @@ struct pi3usb30532 {
struct mutex lock; /* protects the cached conf register */
struct typec_switch sw;
struct typec_mux mux;
+ u8 mode_support; /* Modes supported by hardware as bit flags */
u8 conf;
};
@@ -88,16 +90,19 @@ static int pi3usb30532_mux_set(struct typec_mux *mux, enum
typec_mux_mode mode)
new_conf = PI3USB30532_CONF_OPEN;
break;
case TYPEC_MUX_2CH_USBSS:
- new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
- PI3USB30532_CONF_USB3;
+ if (pi->mode_support & (0x1 << TYPEC_MUX_2CH_USBSS))
+ new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
+ PI3USB30532_CONF_USB3;
break;
case TYPEC_MUX_4CH_AM:
- new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
- PI3USB30532_CONF_4LANE_DP;
+ if (pi->mode_support & (0x1 << TYPEC_MUX_4CH_AM))
+ new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
+ PI3USB30532_CONF_4LANE_DP;
break;
case TYPEC_MUX_2CH_USBSS_2CH_AM:
- new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
- PI3USB30532_CONF_USB3_AND_2LANE_DP;
+ if (pi->mode_support & (0x1 << TYPEC_MUX_2CH_USBSS_2CH_AM))
+ new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
+ PI3USB30532_CONF_USB3_AND_2LANE_DP;
break;
}
@@ -124,6 +129,18 @@ static int pi3usb30532_probe(struct i2c_client *client)
pi->mux.set = pi3usb30532_mux_set;
mutex_init(&pi->lock);
+ if (device_property_present(dev, "have-2ch-usbss"))
+ pi->mode_support |= 0x1 << TYPEC_MUX_2CH_USBSS;
+ if (device_property_present(dev, "have-4ch-am"))
+ pi->mode_support |= 0x1 << TYPEC_MUX_4CH_AM;
+ if (device_property_present(dev, "have-2ch-usbss-2ch-am"))
+ pi->mode_support |= 0x1 << TYPEC_MUX_2CH_USBSS_2CH_AM;
+
+ if (!pi->mode_support) {
+ dev_warn(dev, "No mode support found, assuming full support\n");
+ pi->mode_support = (u8)-1;
+ }
+
ret = i2c_smbus_read_byte_data(client, PI3USB30532_CONF);
if (ret < 0) {
dev_err(dev, "Error reading config register %d\n", ret);
--
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
[RFC PATCH 7/7] usb: typec: mux: Allow default mux mode to be specified
This patch allows the default mux mode to be specified using
a firmware property.
Signed-off-by: Mats Karrman
---
drivers/usb/typec/mux/pi3usb30532.c | 18 ++
drivers/usb/typec/tcpm.c| 2 +-
include/linux/usb/typec.h | 1 +
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/typec/mux/pi3usb30532.c
b/drivers/usb/typec/mux/pi3usb30532.c
index 3e564e6..d1de12c 100644
--- a/drivers/usb/typec/mux/pi3usb30532.c
+++ b/drivers/usb/typec/mux/pi3usb30532.c
@@ -26,6 +26,7 @@ struct pi3usb30532 {
struct mutex lock; /* protects the cached conf register */
struct typec_switch sw;
struct typec_mux mux;
+ enum typec_mux_mode default_mux_mode; /* Mode for TYPEC_MUX_DEFAULT */
u8 mode_support; /* Modes supported by hardware as bit flags */
u8 conf;
};
@@ -84,6 +85,9 @@ static int pi3usb30532_mux_set(struct typec_mux *mux, enum
typec_mux_mode mode)
mutex_lock(&pi->lock);
new_conf = pi->conf;
+ if (mode == TYPEC_MUX_DEFAULT)
+ mode = pi->default_mux_mode;
+
switch (mode) {
default:
case TYPEC_MUX_NONE:
@@ -116,6 +120,7 @@ static int pi3usb30532_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
struct pi3usb30532 *pi;
+ const char *mode_str;
int ret;
pi = devm_kzalloc(dev, sizeof(*pi), GFP_KERNEL);
@@ -129,6 +134,19 @@ static int pi3usb30532_probe(struct i2c_client *client)
pi->mux.set = pi3usb30532_mux_set;
mutex_init(&pi->lock);
+ if (!device_property_present(dev, "default-mux-mode")) {
+ pi->default_mux_mode = TYPEC_MUX_2CH_USBSS;
+ } else {
+ ret = device_property_read_string(dev, "default-mux-mode",
+ &mode_str);
+ if (ret)
+ return ret;
+ ret = typec_find_mux_mode(mode_str);
+ if (ret < 0)
+ return ret;
+ pi->default_mux_mode = ret;
+ }
+
if (device_property_present(dev, "have-2ch-usbss"))
pi->mode_support |= 0x1 << TYPEC_MUX_2CH_USBSS;
if (device_property_present(dev, "have-4ch-am"))
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index aaf6d57..8a9dc1a 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -790,7 +790,7 @@ static int tcpm_set_roles(struct tcpm_port *port, bool
attached,
else
usb_role = USB_ROLE_DEVICE;
- ret = tcpm_mux_set(port, TYPEC_MUX_2CH_USBSS, usb_role, orientation);
+ ret = tcpm_mux_set(port, TYPEC_MUX_DEFAULT, usb_role, orientation);
if (ret < 0)
return ret;
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 72cd4a7..85df816 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -74,6 +74,7 @@ enum typec_orientation {
enum typec_mux_mode {
TYPEC_MUX_NONE, /* Open switch */
+ TYPEC_MUX_DEFAULT, /* Initial mode after connect */
TYPEC_MUX_2CH_USBSS,/* 2ch USB SS */
TYPEC_MUX_4CH_AM, /* 4ch Alt Mode */
TYPEC_MUX_2CH_USBSS_2CH_AM, /* 2ch USB SS + 2ch Alt Mode */
--
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] musb: fix remote wakeup racing with suspend
Hi, On Tue, May 01, 2018 at 08:48:11AM -0500, Bin Liu wrote: > On Fri, Apr 27, 2018 at 03:00:05PM +0200, Daniel Glöckner wrote: > > It has been observed that writing 0xF2 to the power register while it > > reads as 0xF4 results in the register having the value 0xF0, i.e. clearing > > RESUME and setting SUSPENDM in one go does not work. It might also violate > > the USB spec to transition directly from resume to suspend, especially > > when not taking T_DRSMDN into account. But this is what happens when a > > remote wakeup occurs between SetPortFeature USB_PORT_FEAT_SUSPEND on the > > root hub and musb_bus_suspend being called. > > > > This commit returns -EBUSY when musb_bus_suspend is called while remote > > wakeup is signalled and thus avoids to reset the RESUME bit. Remember that > > resume can be signalled only when the bus was already suspended, so it is > > safe to skip the following steps even when musb_hub_control ignores the > > what do you mean by "skip the following steps"? Setting USB_PORT_STAT_SUSPEND in musb->port1_status, changing musb->xceiv->otg->state, setting musb->is_active, etc. > > error returned by musb_port_suspend. > > > > The resume part of musb_port_suspend is modified to also accept a pending > > remote wakeup, to bring it to the end after T_DRSMDN has passed. > > Can you please explain more here? I am not sure when musb_port_suspend() > is involved in resume by remote wakeup, and what case is a 'pending' > remote wakeup? With 'pending' I was referring to the situation when MUSB_POWER_RESUME has been set by the controller in the power register as a result of of a detected remote wakeup. I see... finish_resume_work is already scheduled by musb_stage0_irq. So the condition of the if statement probably does not need to be changed. I'll test without that part and make a v2 patch if it works. Btw., do you know why that 1 iterations while loop is needed in musb_port_suspend to pass the OTG Protocol Test as indicated by the comment? Best regards, Daniel -- Dipl.-Math. Daniel Glöckner, emlix GmbH, http://www.emlix.com Fon +49 551 30664-0, Fax +49 551 30664-11, Gothaer Platz 3, 37083 Göttingen, Germany Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160 Geschäftsführung: Heike Jordan, Dr. Uwe Kracke Ust-IdNr.: DE 205 198 055 emlix - your embedded linux partner -- 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: dwc2: fix isoc split in transfer issue
This patch fix dma unaligned problem and data lost problem for isoc split in transfer. Test on rk3288 platform, use an usb hs Hub (GL852G-12) and an usb fs audio device (Plantronics headset) to capture and playback. William Wu (2): usb: dwc2: alloc dma aligned buffer for isoc split in usb: dwc2: fix isoc split in transfer with no data drivers/usb/dwc2/hcd.c | 63 +--- drivers/usb/dwc2/hcd.h | 10 +++ drivers/usb/dwc2/hcd_intr.c | 10 ++- drivers/usb/dwc2/hcd_queue.c | 8 +- 4 files changed, 86 insertions(+), 5 deletions(-) -- 2.0.0 -- 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/2] usb: dwc2: alloc dma aligned buffer for isoc split in
The commit 3bc04e28a030 ("usb: dwc2: host: Get aligned DMA in
a more supported way") rips out a lot of code to simply the
allocation of aligned DMA. However, it also introduces a new
issue when use isoc split in transfer.
In my test case, I connect the dwc2 controller with an usb hs
Hub (GL852G-12), and plug an usb fs audio device (Plantronics
headset) into the downstream port of Hub. Then use the usb mic
to record, we can find noise when playback.
It's because that the usb Hub uses an MDATA for the first
transaction and a DATA0 for the second transaction for the isoc
split in transaction. An typical isoc split in transaction sequence
like this:
- SSPLIT IN transaction
- CSPLIT IN transaction
- MDATA packet
- CSPLIT IN transaction
- DATA0 packet
The DMA address of MDATA (urb->dma) is always DWORD-aligned, but
the DMA address of DATA0 (urb->dma + qtd->isoc_split_offset) may
not be DWORD-aligned, it depends on the qtd->isoc_split_offset (the
length of MDATA). In my test case, the length of MDATA is usually
unaligned, this casue DATA0 packet transmission error.
This patch base on the old way of aligned DMA allocation in the
dwc2 driver to get aligned DMA for isoc split in.
Signed-off-by: William Wu
---
Changes in v2:
- None
drivers/usb/dwc2/hcd.c | 63 +---
drivers/usb/dwc2/hcd.h | 10 +++
drivers/usb/dwc2/hcd_intr.c | 8 ++
drivers/usb/dwc2/hcd_queue.c | 8 +-
4 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 190f959..8c2b35f 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -1562,11 +1562,20 @@ static void dwc2_hc_start_transfer(struct dwc2_hsotg
*hsotg,
}
if (hsotg->params.host_dma) {
- dwc2_writel((u32)chan->xfer_dma,
- hsotg->regs + HCDMA(chan->hc_num));
+ dma_addr_t dma_addr;
+
+ if (chan->align_buf) {
+ if (dbg_hc(chan))
+ dev_vdbg(hsotg->dev, "align_buf\n");
+ dma_addr = chan->align_buf;
+ } else {
+ dma_addr = chan->xfer_dma;
+ }
+ dwc2_writel((u32)dma_addr, hsotg->regs + HCDMA(chan->hc_num));
+
if (dbg_hc(chan))
dev_vdbg(hsotg->dev, "Wrote %08lx to HCDMA(%d)\n",
-(unsigned long)chan->xfer_dma, chan->hc_num);
+(unsigned long)dma_addr, chan->hc_num);
}
/* Start the split */
@@ -2620,6 +2629,33 @@ static void dwc2_hc_init_xfer(struct dwc2_hsotg *hsotg,
}
}
+static int dwc2_alloc_split_dma_aligned_buf(struct dwc2_hsotg *hsotg,
+ struct dwc2_qh *qh,
+ struct dwc2_host_chan *chan)
+{
+ if (!qh->dw_align_buf) {
+ qh->dw_align_buf = kmalloc(chan->max_packet,
+ GFP_ATOMIC | GFP_DMA);
+ if (!qh->dw_align_buf)
+ return -ENOMEM;
+
+ qh->dw_align_buf_size = chan->max_packet;
+ }
+
+ qh->dw_align_buf_dma = dma_map_single(hsotg->dev, qh->dw_align_buf,
+ qh->dw_align_buf_size,
+ DMA_FROM_DEVICE);
+
+ if (dma_mapping_error(hsotg->dev, qh->dw_align_buf_dma)) {
+ dev_err(hsotg->dev, "can't map align_buf\n");
+ chan->align_buf = 0;
+ return -EINVAL;
+ }
+
+ chan->align_buf = qh->dw_align_buf_dma;
+ return 0;
+}
+
#define DWC2_USB_DMA_ALIGN 4
struct dma_aligned_buffer {
@@ -2797,6 +2833,27 @@ static int dwc2_assign_and_init_hc(struct dwc2_hsotg
*hsotg, struct dwc2_qh *qh)
/* Set the transfer attributes */
dwc2_hc_init_xfer(hsotg, chan, qtd);
+ /* For non-dword aligned buffers */
+ if (hsotg->params.host_dma > 0 && qh->do_split &&
+ chan->ep_is_in && (chan->xfer_dma & 0x3)) {
+ dev_vdbg(hsotg->dev, "Non-aligned buffer\n");
+ if (dwc2_alloc_split_dma_aligned_buf(hsotg, qh, chan)) {
+ dev_err(hsotg->dev,
+ "%s: Failed to allocate memory to handle
non-dword aligned buffer\n",
+ __func__);
+ /* Add channel back to free list */
+ chan->align_buf = 0;
+ chan->multi_count = 0;
+ list_add_tail(&chan->hc_list_entry,
+ &hsotg->free_hc_list);
+ qtd->in_process = 0;
+ qh->channel = NULL;
+ return -ENOMEM;
+ }
+ } else {
+ chan->align_buf = 0;
+ }
+
if (chan->ep_type == USB_ENDPO
[PATCH v2 2/2] usb: dwc2: fix isoc split in transfer with no data
If isoc split in transfer with no data (the length of DATA0
packet is zero), we can't simply return immediately. Because
the DATA0 can be the first transaction or the second transaction
for the isoc split in transaction. If the DATA0 packet with no
data is in the first transaction, we can return immediately.
But if the DATA0 packet with no data is in the second transaction
of isoc split in transaction sequence, we need to increase the
qtd->isoc_frame_index and giveback urb to device driver if needed,
otherwise, the MDATA packet will be lost.
A typical test case is that connect the dwc2 controller with an
usb hs Hub (GL852G-12), and plug an usb fs audio device (Plantronics
headset) into the downstream port of Hub. Then use the usb mic
to record, we can find noise when playback.
In the case, the isoc split in transaction sequence like this:
- SSPLIT IN transaction
- CSPLIT IN transaction
- MDATA packet (176 bytes)
- CSPLIT IN transaction
- DATA0 packet (0 byte)
This patch use both the length of DATA0 and qtd->isoc_split_offset
to check if the DATA0 is in the second transaction.
Signed-off-by: William Wu
---
Changes in v2:
- Modify the commit message
drivers/usb/dwc2/hcd_intr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
index 5e2378f..479f628 100644
--- a/drivers/usb/dwc2/hcd_intr.c
+++ b/drivers/usb/dwc2/hcd_intr.c
@@ -930,7 +930,7 @@ static int dwc2_xfercomp_isoc_split_in(struct dwc2_hsotg
*hsotg,
frame_desc = &qtd->urb->iso_descs[qtd->isoc_frame_index];
len = dwc2_get_actual_xfer_length(hsotg, chan, chnum, qtd,
DWC2_HC_XFER_COMPLETE, NULL);
- if (!len) {
+ if (!len && !qtd->isoc_split_offset) {
qtd->complete_split = 0;
qtd->isoc_split_offset = 0;
return 0;
--
2.0.0
--
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 1/2] usb: dwc2: alloc dma aligned buffer for isoc split in
Hi,
On Tue, May 1, 2018 at 8:04 PM, William Wu wrote:
> The commit 3bc04e28a030 ("usb: dwc2: host: Get aligned DMA in
> a more supported way") rips out a lot of code to simply the
> allocation of aligned DMA. However, it also introduces a new
> issue when use isoc split in transfer.
>
> In my test case, I connect the dwc2 controller with an usb hs
> Hub (GL852G-12), and plug an usb fs audio device (Plantronics
> headset) into the downstream port of Hub. Then use the usb mic
> to record, we can find noise when playback.
>
> It's because that the usb Hub uses an MDATA for the first
> transaction and a DATA0 for the second transaction for the isoc
> split in transaction. An typical isoc split in transaction sequence
> like this:
>
> - SSPLIT IN transaction
> - CSPLIT IN transaction
> - MDATA packet
> - CSPLIT IN transaction
> - DATA0 packet
>
> The DMA address of MDATA (urb->dma) is always DWORD-aligned, but
> the DMA address of DATA0 (urb->dma + qtd->isoc_split_offset) may
> not be DWORD-aligned, it depends on the qtd->isoc_split_offset (the
> length of MDATA). In my test case, the length of MDATA is usually
> unaligned, this casue DATA0 packet transmission error.
>
> This patch base on the old way of aligned DMA allocation in the
> dwc2 driver to get aligned DMA for isoc split in.
>
> Signed-off-by: William Wu
> ---
> Changes in v2:
> - None
>
> drivers/usb/dwc2/hcd.c | 63
> +---
> drivers/usb/dwc2/hcd.h | 10 +++
> drivers/usb/dwc2/hcd_intr.c | 8 ++
> drivers/usb/dwc2/hcd_queue.c | 8 +-
> 4 files changed, 85 insertions(+), 4 deletions(-)
A word of warning that I'm pretty rusty on dwc2 and even when I was
making lots of patches I still considered myself a bit clueless.
...so if something seems wrong, please call me on it...
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 190f959..8c2b35f 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -1562,11 +1562,20 @@ static void dwc2_hc_start_transfer(struct dwc2_hsotg
> *hsotg,
> }
>
> if (hsotg->params.host_dma) {
> - dwc2_writel((u32)chan->xfer_dma,
> - hsotg->regs + HCDMA(chan->hc_num));
> + dma_addr_t dma_addr;
> +
> + if (chan->align_buf) {
> + if (dbg_hc(chan))
> + dev_vdbg(hsotg->dev, "align_buf\n");
> + dma_addr = chan->align_buf;
> + } else {
> + dma_addr = chan->xfer_dma;
> + }
> + dwc2_writel((u32)dma_addr, hsotg->regs + HCDMA(chan->hc_num));
> +
> if (dbg_hc(chan))
> dev_vdbg(hsotg->dev, "Wrote %08lx to HCDMA(%d)\n",
> -(unsigned long)chan->xfer_dma, chan->hc_num);
> +(unsigned long)dma_addr, chan->hc_num);
> }
>
> /* Start the split */
> @@ -2620,6 +2629,33 @@ static void dwc2_hc_init_xfer(struct dwc2_hsotg *hsotg,
> }
> }
>
> +static int dwc2_alloc_split_dma_aligned_buf(struct dwc2_hsotg *hsotg,
> + struct dwc2_qh *qh,
> + struct dwc2_host_chan *chan)
> +{
> + if (!qh->dw_align_buf) {
> + qh->dw_align_buf = kmalloc(chan->max_packet,
So you're potentially doing a bounce buffer atop a bounce buffer now,
right? That seems pretty non-optimal. You're also back to doing a
kmalloc at interrupt time which I found was pretty bad for
performance.
Is there really no way you can do your memory allocation in
dwc2_alloc_dma_aligned_buffer() instead of here? For input packets,
if you could just allocate an extra 3 bytes in the original bounce
buffer you could just re-use the original bounce buffer together with
a memmove? AKA:
transfersize = 13 + 64;
buf = alloc(16 + 64);
// Do the first transfer, no problems.
dma_into(buf, 13);
// 2nd transfer isn't aligned, so align.
// we allocated a little extra to account for this
dma_into(buf + 16, 64);
// move back where it belongs.
memmove(buf + 13, buf + 16, 64);
To make the above work you'd need to still allocate a bounce buffer
even if the original "urb->transfer_buffer" was already aligned.
Ideally you'd be able to know when dwc2_alloc_dma_aligned_buffer()
that this is one of the special cases where you need a slightly
oversized bounce buffer.
---
If you somehow need to do something for output, you'd do the opposite.
You'd copy backwards top already transferred data to align.
> + GFP_ATOMIC | GFP_DMA);
> + if (!qh->dw_align_buf)
> + return -ENOMEM;
> +
> + qh->dw_align_buf_size = chan->max_packet;
> + }
> +
> + qh->dw_align_buf_dma = dma_map_single(hsotg->dev, qh->dw_align_buf,
> + qh->dw
Re: [PATCH v2 2/2] usb: dwc2: fix isoc split in transfer with no data
Hi,
On Tue, May 1, 2018 at 8:04 PM, William Wu wrote:
> If isoc split in transfer with no data (the length of DATA0
> packet is zero), we can't simply return immediately. Because
> the DATA0 can be the first transaction or the second transaction
> for the isoc split in transaction. If the DATA0 packet with no
> data is in the first transaction, we can return immediately.
> But if the DATA0 packet with no data is in the second transaction
> of isoc split in transaction sequence, we need to increase the
> qtd->isoc_frame_index and giveback urb to device driver if needed,
> otherwise, the MDATA packet will be lost.
>
> A typical test case is that connect the dwc2 controller with an
> usb hs Hub (GL852G-12), and plug an usb fs audio device (Plantronics
> headset) into the downstream port of Hub. Then use the usb mic
> to record, we can find noise when playback.
>
> In the case, the isoc split in transaction sequence like this:
>
> - SSPLIT IN transaction
> - CSPLIT IN transaction
> - MDATA packet (176 bytes)
> - CSPLIT IN transaction
> - DATA0 packet (0 byte)
>
> This patch use both the length of DATA0 and qtd->isoc_split_offset
> to check if the DATA0 is in the second transaction.
>
> Signed-off-by: William Wu
> ---
> Changes in v2:
> - Modify the commit message
>
> drivers/usb/dwc2/hcd_intr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
> index 5e2378f..479f628 100644
> --- a/drivers/usb/dwc2/hcd_intr.c
> +++ b/drivers/usb/dwc2/hcd_intr.c
> @@ -930,7 +930,7 @@ static int dwc2_xfercomp_isoc_split_in(struct dwc2_hsotg
> *hsotg,
> frame_desc = &qtd->urb->iso_descs[qtd->isoc_frame_index];
> len = dwc2_get_actual_xfer_length(hsotg, chan, chnum, qtd,
> DWC2_HC_XFER_COMPLETE, NULL);
> - if (!len) {
> + if (!len && !qtd->isoc_split_offset) {
> qtd->complete_split = 0;
> qtd->isoc_split_offset = 0;
> return 0;
I don't think my USB-fu is strong enough to do a real review of this
patch, but one small nitpick is that you can remove
"qtd->isoc_split_offset = 0" in the if test now. AKA:
if (!len && !qtd->isoc_split_offset) {
qtd->complete_split = 0;
return 0;
}
...since you only enter the "if" test now when isoc_split_offset is
already 0... Hopefully John Youn will have some time to comment on
this patch with more real USB knowledge...
-Doug
--
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 v9 1/2] usb/gadget: Constify usb_gadget_get_string "table" argument
The table is never modified by the function. This allows us
to use it on a statically defined table that is marked const.
Signed-off-by: Benjamin Herrenschmidt
---
drivers/usb/gadget/usbstring.c | 2 +-
include/linux/usb/gadget.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/usbstring.c b/drivers/usb/gadget/usbstring.c
index 566ab261e8b7..7c24d1ce1088 100644
--- a/drivers/usb/gadget/usbstring.c
+++ b/drivers/usb/gadget/usbstring.c
@@ -33,7 +33,7 @@
* characters (which are also widely used in C strings).
*/
int
-usb_gadget_get_string (struct usb_gadget_strings *table, int id, u8 *buf)
+usb_gadget_get_string (const struct usb_gadget_strings *table, int id, u8 *buf)
{
struct usb_string *s;
int len;
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 847f423ad9b3..e5cd84a0f84a 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -763,7 +763,7 @@ struct usb_gadget_string_container {
};
/* put descriptor for string with that id into buf (buflen >= 256) */
-int usb_gadget_get_string(struct usb_gadget_strings *table, int id, u8 *buf);
+int usb_gadget_get_string(const struct usb_gadget_strings *table, int id, u8
*buf);
/*-*/
--
2.17.0
--
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 v9 2/2] usb/gadget: Add driver for Aspeed SoC virtual hub
The Aspeed BMC SoCs support a "virtual hub" function. It provides some HW support for a top-level USB2 hub behind which sit 5 gadget "ports". This driver adds support for the full functionality, emulating the hub standard requests and exposing 5 UDC gadget drivers corresponding to the ports. The hub itself has HW provided dedicated EP0 and EP1 (the latter for hub interrupts). It also has dedicated EP0s for each function. For other endpoints, there's a pool of 15 "generic" endpoints that are shared among the ports. The driver relies on my previous patch adding a "dispose" EP op to handle EP allocation between ports. EPs are allocated from the shared pool in the UDC "match_ep" callback and assigned to the UDC instance (added to the gadget ep_list). When the composite driver gets unbound, the new hook will allow the UDC to clean things up and return those EPs to the shared pool. Signed-off-by: Benjamin Herrenschmidt --- v9. - Fix lock recursion issues when replying to standard SETUP packets - Fix lifetime of the port_dev struct device v8. - Rework ast_vhub_rep_desc() to avoid incorrect built error when fortify is enabled - Rework ast_vhub_rep_string() to use usb_gadget_get_string() instead of a local copy of ascii2desc - Whitespace fixes v7. - Fix OF match list - Remove unused variable in hub code v6. - Fix bug with 0-length packets on IN endpoints in desc mode v5. - Remove #ifdef's and hard coded values - Remove module parameters - Honor maximum speed device-tree property - Remove useless list_empty test - Cosmetic/spaces fixes - Add SPDX licence identifiers v4. - Fix missing unlock ast_vhub_udc_wakeup() error path - Make "irq" signed to deal with error from platform_get_irq - Fix Makefile for module builds - Fix a missing endian conversion v3. - Rebased - Add clk stuff v2. - Cosmetic fixes - Properly "allocate" device addresses instead of using a never reset counter - Move .dtsi updates to a different patch foo --- drivers/usb/gadget/udc/Kconfig | 2 + drivers/usb/gadget/udc/Makefile | 1 + drivers/usb/gadget/udc/aspeed-vhub/Kconfig | 7 + drivers/usb/gadget/udc/aspeed-vhub/Makefile | 4 + drivers/usb/gadget/udc/aspeed-vhub/core.c | 425 ++ drivers/usb/gadget/udc/aspeed-vhub/dev.c| 589 ++ drivers/usb/gadget/udc/aspeed-vhub/ep0.c| 486 +++ drivers/usb/gadget/udc/aspeed-vhub/epn.c| 843 drivers/usb/gadget/udc/aspeed-vhub/hub.c| 819 +++ drivers/usb/gadget/udc/aspeed-vhub/vhub.h | 514 10 files changed, 3690 insertions(+) create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/Kconfig create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/Makefile create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/core.c create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/dev.c create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/ep0.c create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/epn.c create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/hub.c create mode 100644 drivers/usb/gadget/udc/aspeed-vhub/vhub.h diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig index 0875d38476ee..b838cae1 100644 --- a/drivers/usb/gadget/udc/Kconfig +++ b/drivers/usb/gadget/udc/Kconfig @@ -438,6 +438,8 @@ config USB_GADGET_XILINX dynamically linked module called "udc-xilinx" and force all gadget drivers to also be dynamically linked. +source "drivers/usb/gadget/udc/aspeed-vhub/Kconfig" + # # LAST -- dummy/emulated controller # diff --git a/drivers/usb/gadget/udc/Makefile b/drivers/usb/gadget/udc/Makefile index ce865b129fd6..897f648f3cf1 100644 --- a/drivers/usb/gadget/udc/Makefile +++ b/drivers/usb/gadget/udc/Makefile @@ -39,4 +39,5 @@ obj-$(CONFIG_USB_MV_U3D) += mv_u3d_core.o obj-$(CONFIG_USB_GR_UDC) += gr_udc.o obj-$(CONFIG_USB_GADGET_XILINX)+= udc-xilinx.o obj-$(CONFIG_USB_SNP_UDC_PLAT) += snps_udc_plat.o +obj-$(CONFIG_USB_ASPEED_VHUB) += aspeed-vhub/ obj-$(CONFIG_USB_BDC_UDC) += bdc/ diff --git a/drivers/usb/gadget/udc/aspeed-vhub/Kconfig b/drivers/usb/gadget/udc/aspeed-vhub/Kconfig new file mode 100644 index ..f0cdf89b8503 --- /dev/null +++ b/drivers/usb/gadget/udc/aspeed-vhub/Kconfig @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0+ +config USB_ASPEED_VHUB + tristate "Aspeed vHub UDC driver" + depends on ARCH_ASPEED || COMPILE_TEST + help + USB peripheral controller for the Aspeed AST2500 family + SoCs supporting the "vHub" functionality and USB2.0 diff --git a/drivers/usb/gadget/udc/aspeed-vhub/Makefile b/drivers/usb/gadget/udc/aspeed-vhub/Makefile new file mode 100644 index ..9f3add605f8e --- /dev/null +++ b/drivers/usb/gadget/udc/aspeed-vhub/Makefile @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0+ +obj-$(CONFIG_USB_ASPEED_VHUB) += aspeed-vhub.
Re: [GIT PULL] USB fixes for v4.17-rc3
Hi, Greg Kroah-Hartman writes: > On Fri, Apr 27, 2018 at 11:09:50AM +0300, Felipe Balbi wrote: >> >> Hi Greg, >> >> here's a tiny, 7 fixes only, pull request for the current -rc cycle. Let >> me know if you want anything to be changed. >> >> cheers >> >> The following changes since commit 6d08b06e67cd117f6992c46611dfb4ce267cd71e: >> >> Linux 4.17-rc2 (2018-04-22 19:20:09 -0700) >> >> are available in the Git repository at: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git >> tags/fixes-for-v4.17-rc3 >> >> for you to fetch changes up to ed769520727edbf526e0f510e6c501fc6ba3824a: >> >> usb: gadget: composite Allow for larger configuration descriptors >> (2018-04-27 10:17:10 +0300) > > Sorry these missed my last pull request to Linus. I've taken them now > and will send them off to him with my next round in a week or so. sure thing, no issues :-) -- balbi -- 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 3/4] usb: typec: fusb302: Fix debugfs issue
On Mon, Apr 30, 2018 at 06:34:03AM -0700, Guenter Roeck wrote:
> On 04/30/2018 05:41 AM, Heikki Krogerus wrote:
> > Removing the "fusb302" debugfs directory when unloading
> > the driver. That allows the driver to be loaded more then
> > ones.
> >
> > This fixes an issue where the driver, if unloaded, can't be
> > re-loaded, as the "fusb302" debugfs directory already
> > exists. While here, removing useless condition when creating
> > the debugfs directory.
> >
> > Fixes: 76f0c53d08b9 ("usb: typec: fusb302: Move out of staging")
> > Signed-off-by: Heikki Krogerus
> > ---
> > drivers/usb/typec/fusb302/fusb302.c | 9 -
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/typec/fusb302/fusb302.c
> > b/drivers/usb/typec/fusb302/fusb302.c
> > index eba6bb890b17..0e5d0aa052f5 100644
> > --- a/drivers/usb/typec/fusb302/fusb302.c
> > +++ b/drivers/usb/typec/fusb302/fusb302.c
> > @@ -218,11 +218,9 @@ static struct dentry *rootdir;
> > static int fusb302_debugfs_init(struct fusb302_chip *chip)
> > {
> > mutex_init(&chip->logbuffer_lock);
> > - if (!rootdir) {
> > - rootdir = debugfs_create_dir("fusb302", NULL);
> > - if (!rootdir)
> > - return -ENOMEM;
> > - }
>
> I think the idea here was to permit more than one instance of the driver,
> and have all debugfs file entries under the fusb302 directory. Loading the
> second instance will fail after this patch is applied.
OK, need to come up with something else for this issue then.
> > + rootdir = debugfs_create_dir("fusb302", NULL);
> > + if (!rootdir)
> > + return -ENOMEM;
> > chip->dentry = debugfs_create_file(dev_name(chip->dev),
> >S_IFREG | 0444, rootdir,
> > @@ -234,6 +232,7 @@ static int fusb302_debugfs_init(struct fusb302_chip
> > *chip)
> > static void fusb302_debugfs_exit(struct fusb302_chip *chip)
> > {
> > debugfs_remove(chip->dentry);
> > + debugfs_remove(rootdir); > }
> > #else
> >
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: [PATCH 4/4] usb: roles: intel_xhci: Always allow user control
On Tue, May 01, 2018 at 11:57:55AM +0200, Hans de Goede wrote:
> Hi,
>
> On 30-04-18 14:41, Heikki Krogerus wrote:
> > Trying to determine the USB port type with this mux is very
> > difficult. To simplify the situation, always allowing user
> > control, even if the port is USB Type-C port.
> >
> > Signed-off-by: Heikki Krogerus > ---
> > drivers/usb/roles/intel-xhci-usb-role-switch.c | 15 +--
> > 1 file changed, 1 insertion(+), 14 deletions(-)
> >
> > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > index 28102127b9d5..6482eee6fd45 100644
> > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > @@ -43,15 +43,6 @@ struct intel_xhci_acpi_match {
> > int hrv;
> > };
> > -/*
> > - * ACPI IDs for PMICs which do not support separate data and power role
> > - * detection (USB ACA detection for micro USB OTG), we allow userspace to
> > - * change the role manually on these.
> > - */
> > -static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = {
> > - { "INT33F4", 3 }, /* X-Powers AXP288 PMIC */
> > -};
> > -
> > static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> > {
> > struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
> > @@ -137,7 +128,6 @@ static int intel_xhci_usb_probe(struct platform_device
> > *pdev)
> > struct device *dev = &pdev->dev;
> > struct intel_xhci_usb_data *data;
> > struct resource *res;
> > - int i;
> > data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > if (!data)
> > @@ -150,10 +140,7 @@ static int intel_xhci_usb_probe(struct platform_device
> > *pdev)
> > if (!data->base)
> > return -ENOMEM;
> > - for (i = 0; i < ARRAY_SIZE(allow_userspace_ctrl_ids); i++)
> > - if (acpi_dev_present(allow_userspace_ctrl_ids[i].hid, "1",
> > -allow_userspace_ctrl_ids[i].hrv))
> > - sw_desc.allow_userspace_control = true;
> > + sw_desc.allow_userspace_control = true;
>
> It would be better to add:
>
> .allow_userspace_control = true,
>
> To the initialization of the struct and also make the struct const since
> we are now no longer changing it and usb_role_switch_register() accepts
> it being const.
Makes sense.
> With that changed:
>
> Reviewed-by: Hans de Goede
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
