Re: [PATCH] usb: dwc3: Fix assignment of EP transfer resources
Hi,
John Youn writes:
>> John Youn writes:
>>> The assignement of EP transfer resources was not handled properly in the
>>> dwc3 driver. Commit aebda6187181 ("usb: dwc3: Reset the transfer
>>> resource index on SET_INTERFACE") previously fixed one aspect of this
>>> where resources may be exhausted with multiple calls to SET_INTERFACE.
>>> However, it introduced an issue where composite devices with multiple
>>> interfaces can be assigned the same transfer resources for different
>>> endpoints.
>>>
>>> This patch solves both issues.
>>>
>>> The assigning of transfer resource should go as follows:
>>>
>>> Each hardware endpoint requires a transfer resource before it can
>>> perform any USB transfer. The transfer resources are allocated using two
>>> commands: DEPSTARTCFG and DEPXFERCFG.
>>>
>>> In the controller, there is a transfer resource index that is set by the
>>> DEPSTARTCFG command. The DEPXFERCFG command assigns the resource to an
>>> endpoint and increments the transfer resource index.
>>>
>>> Upon startup of the driver, the transfer resource index should be set to
>>> 0 by issuing DEPSTARTCFG(0). EP0-out and EP0-in are then assigned a
>>> resource by DEPXFERCFG. They are assigned resource indexes 0 and 1.
>>>
>>> Every time a SET_CONFIGURATION usb request occurs the DEPSTARTCFG(2)
>>> command should be issued to reset the index to 2. Transfer resources 0
>>> and 1 remain assigned to EP0-out and EP0-in.
>>>
>>> Then for every endpoint in the configuration (endpoints that will be
>>> enabled by the upper layer) call DEPXFERCFG to assign the next
>>> resource. On SET_INTERFACE, the same or different endpoints may be
>>> enabled. If the endpoint already has an assigned transfer resource,
>>> don't assign a new one.
>>
>> very nice and thorough commit log, thanks.
>>
>>> Fixes: aebda6187181 ("usb: dwc3: Reset the transfer resource index on
>>> SET_INTERFACE")
>>
>> You need to Cc stable here:
>>
>> Cc: # v3.2+
>>
>> The reason for v3.2+, instead of v4.3+ is that usb: dwc3: Reset the
>> transfer resource index on SET_INTERFACE has been backported to v3.2+,
>> so we want to fix all those kernels too.
>>
>>> Reported-by: Ravi Babu
>>> Signed-off-by: John Youn
>>> ---
>>> Hi Ravi,
>>>
>>> Here is a patch that should solve your issue. Can you review and test
>>> it out?
>>>
>>> I have tested with multiple SET_INTERFACE for a single interface.
>>>
>>> Thanks,
>>> John
>>>
>>>
>>>
>>> drivers/usb/dwc3/core.h | 3 +++
>>> drivers/usb/dwc3/ep0.c| 4
>>> drivers/usb/dwc3/gadget.c | 36 +---
>>> 3 files changed, 36 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>>> index 2913068..7d5d3a2 100644
>>> --- a/drivers/usb/dwc3/core.h
>>> +++ b/drivers/usb/dwc3/core.h
>>> @@ -453,6 +453,8 @@ struct dwc3_event_buffer {
>>> * @flags: endpoint flags (wedged, stalled, ...)
>>> * @number: endpoint number (1 - 15)
>>> * @type: set to bmAttributes & USB_ENDPOINT_XFERTYPE_MASK
>>> + * @resource_assigned: indicates that a transfer resource is assigned
>>> + * to this endpoint
>>> * @resource_index: Resource transfer index
>>> * @interval: the interval on which the ISOC transfer is started
>>> * @name: a human readable name e.g. ep1out-bulk
>>> @@ -485,6 +487,7 @@ struct dwc3_ep {
>>>
>>> u8 number;
>>> u8 type;
>>> + unsignedresource_assigned:1;
>>
>> I would prefer to use up another bit from our 'flags' bitfield. Looks
>> like that would be:
>>
>> #define DWC3_EP_RESOURCE_ASSIGNED (1 << 7)
>>
>>> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
>>> index 3a9354a..878b40e 100644
>>> --- a/drivers/usb/dwc3/ep0.c
>>> +++ b/drivers/usb/dwc3/ep0.c
>>> @@ -737,10 +737,6 @@ static int dwc3_ep0_std_request(struct dwc3 *dwc,
>>> struct usb_ctrlrequest *ctrl)
>>> dwc3_trace(trace_dwc3_ep0, "USB_REQ_SET_ISOCH_DELAY");
>>> ret = dwc3_ep0_set_isoch_delay(dwc, ctrl);
>>> break;
>>> - case USB_REQ_SET_INTERFACE:
>>> - dwc3_trace(trace_dwc3_ep0, "USB_REQ_SET_INTERFACE");
>>> - dwc->start_config_issued = false;
>>> - /* Fall through */
>>> default:
>>> dwc3_trace(trace_dwc3_ep0, "Forwarding to gadget driver");
>>> ret = dwc3_ep0_delegate_req(dwc, ctrl);
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 7d1dd82..1aeea8f 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -385,6 +385,30 @@ static void dwc3_free_trb_pool(struct dwc3_ep *dep)
>>> dep->trb_pool_dma = 0;
>>> }
>>>
>>> +static void dwc3_gadget_reset_xfer_resource_for_ep(struct dwc3 *dwc,
>>
>> it would be nicer if this would receive struct dwc3_ep *dep as argument.
>>
>>> + int num,
>>> + int direction)
>>
>> this is an
[PATCH] cdc-acm: implement put_char() and flush_chars()
This should cut down latencies and waste if the tty layer writes single bytes.
Signed-off-by: Oliver Neukum >oneu...@suse.com>
---
drivers/usb/class/cdc-acm.c | 67 +
drivers/usb/class/cdc-acm.h | 1 +
2 files changed, 68 insertions(+)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index b30e742..1cb3124 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -712,9 +712,20 @@ static int acm_tty_write(struct tty_struct *tty,
}
if (acm->susp_count) {
+ if (acm->putbuffer) {
+ /* now to preserve order */
+ usb_anchor_urb(acm->putbuffer->urb, &acm->delayed);
+ acm->putbuffer = NULL;
+ }
usb_anchor_urb(wb->urb, &acm->delayed);
spin_unlock_irqrestore(&acm->write_lock, flags);
return count;
+ } else {
+ if (acm->putbuffer) {
+ /* at this point there is no good way to handle errors
*/
+ acm_start_wb(acm, acm->putbuffer);
+ acm->putbuffer = NULL;
+ }
}
stat = acm_start_wb(acm, wb);
@@ -725,6 +736,60 @@ static int acm_tty_write(struct tty_struct *tty,
return count;
}
+static void acm_tty_flush_chars(struct tty_struct *tty)
+{
+ struct acm *acm = tty->driver_data;
+ struct acm_wb *cur = acm->putbuffer;
+ int err;
+ unsigned long flags;
+
+ acm->putbuffer = NULL;
+ err = usb_autopm_get_interface_async(acm->control);
+ spin_lock_irqsave(&acm->write_lock, flags);
+ if (err < 0) {
+ cur->use = 0;
+ goto out;
+ }
+
+ if (acm->susp_count)
+ usb_anchor_urb(cur->urb, &acm->delayed);
+ else
+ acm_start_wb(acm, cur);
+out:
+ spin_unlock_irqrestore(&acm->write_lock, flags);
+ return;
+}
+
+static int acm_tty_put_char(struct tty_struct *tty, unsigned char ch)
+{
+ struct acm *acm = tty->driver_data;
+ struct acm_wb *cur;
+ int wbn;
+ unsigned long flags;
+
+overflow:
+ cur = acm->putbuffer;
+ if (!cur) {
+ spin_lock_irqsave(&acm->write_lock, flags);
+ wbn = acm_wb_alloc(acm);
+ if (wbn >= 0) {
+ cur = &acm->wb[wbn];
+ acm->putbuffer = cur;
+ }
+ spin_unlock_irqrestore(&acm->write_lock, flags);
+ if (!cur)
+ return 0;
+ }
+
+ if (cur->len == acm->writesize) {
+ acm_tty_flush_chars(tty);
+ goto overflow;
+ }
+
+ cur->buf[cur->len++] = ch;
+ return 1;
+}
+
static int acm_tty_write_room(struct tty_struct *tty)
{
struct acm *acm = tty->driver_data;
@@ -1888,6 +1953,8 @@ static const struct tty_operations acm_ops = {
.cleanup = acm_tty_cleanup,
.hangup = acm_tty_hangup,
.write =acm_tty_write,
+ .put_char = acm_tty_put_char,
+ .flush_chars = acm_tty_flush_chars,
.write_room = acm_tty_write_room,
.ioctl =acm_tty_ioctl,
.throttle = acm_tty_throttle,
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index dd9af38..648a6f7 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -94,6 +94,7 @@ struct acm {
unsigned long read_urbs_free;
struct urb *read_urbs[ACM_NR];
struct acm_rb read_buffers[ACM_NR];
+ struct acm_wb *putbuffer; /* for
acm_tty_put_char() */
int rx_buflimit;
int rx_endpoint;
spinlock_t read_lock;
--
2.1.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] usb: dwc3: Fix assignment of EP transfer resources
On 2/10/2016 12:44 AM, Felipe Balbi wrote:
>
> Hi,
>
> John Youn writes:
>>> John Youn writes:
The assignement of EP transfer resources was not handled properly in the
dwc3 driver. Commit aebda6187181 ("usb: dwc3: Reset the transfer
resource index on SET_INTERFACE") previously fixed one aspect of this
where resources may be exhausted with multiple calls to SET_INTERFACE.
However, it introduced an issue where composite devices with multiple
interfaces can be assigned the same transfer resources for different
endpoints.
This patch solves both issues.
The assigning of transfer resource should go as follows:
Each hardware endpoint requires a transfer resource before it can
perform any USB transfer. The transfer resources are allocated using two
commands: DEPSTARTCFG and DEPXFERCFG.
In the controller, there is a transfer resource index that is set by the
DEPSTARTCFG command. The DEPXFERCFG command assigns the resource to an
endpoint and increments the transfer resource index.
Upon startup of the driver, the transfer resource index should be set to
0 by issuing DEPSTARTCFG(0). EP0-out and EP0-in are then assigned a
resource by DEPXFERCFG. They are assigned resource indexes 0 and 1.
Every time a SET_CONFIGURATION usb request occurs the DEPSTARTCFG(2)
command should be issued to reset the index to 2. Transfer resources 0
and 1 remain assigned to EP0-out and EP0-in.
Then for every endpoint in the configuration (endpoints that will be
enabled by the upper layer) call DEPXFERCFG to assign the next
resource. On SET_INTERFACE, the same or different endpoints may be
enabled. If the endpoint already has an assigned transfer resource,
don't assign a new one.
>>>
>>> very nice and thorough commit log, thanks.
>>>
Fixes: aebda6187181 ("usb: dwc3: Reset the transfer resource index on
SET_INTERFACE")
>>>
>>> You need to Cc stable here:
>>>
>>> Cc: # v3.2+
>>>
>>> The reason for v3.2+, instead of v4.3+ is that usb: dwc3: Reset the
>>> transfer resource index on SET_INTERFACE has been backported to v3.2+,
>>> so we want to fix all those kernels too.
>>>
Reported-by: Ravi Babu
Signed-off-by: John Youn
---
Hi Ravi,
Here is a patch that should solve your issue. Can you review and test
it out?
I have tested with multiple SET_INTERFACE for a single interface.
Thanks,
John
drivers/usb/dwc3/core.h | 3 +++
drivers/usb/dwc3/ep0.c| 4
drivers/usb/dwc3/gadget.c | 36 +---
3 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 2913068..7d5d3a2 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -453,6 +453,8 @@ struct dwc3_event_buffer {
* @flags: endpoint flags (wedged, stalled, ...)
* @number: endpoint number (1 - 15)
* @type: set to bmAttributes & USB_ENDPOINT_XFERTYPE_MASK
+ * @resource_assigned: indicates that a transfer resource is assigned
+ *to this endpoint
* @resource_index: Resource transfer index
* @interval: the interval on which the ISOC transfer is started
* @name: a human readable name e.g. ep1out-bulk
@@ -485,6 +487,7 @@ struct dwc3_ep {
u8 number;
u8 type;
+ unsignedresource_assigned:1;
>>>
>>> I would prefer to use up another bit from our 'flags' bitfield. Looks
>>> like that would be:
>>>
>>> #define DWC3_EP_RESOURCE_ASSIGNED (1 << 7)
>>>
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 3a9354a..878b40e 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -737,10 +737,6 @@ static int dwc3_ep0_std_request(struct dwc3 *dwc,
struct usb_ctrlrequest *ctrl)
dwc3_trace(trace_dwc3_ep0, "USB_REQ_SET_ISOCH_DELAY");
ret = dwc3_ep0_set_isoch_delay(dwc, ctrl);
break;
- case USB_REQ_SET_INTERFACE:
- dwc3_trace(trace_dwc3_ep0, "USB_REQ_SET_INTERFACE");
- dwc->start_config_issued = false;
- /* Fall through */
default:
dwc3_trace(trace_dwc3_ep0, "Forwarding to gadget driver");
ret = dwc3_ep0_delegate_req(dwc, ctrl);
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 7d1dd82..1aeea8f 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -385,6 +385,30 @@ static void dwc3_free_trb_pool(struct dwc3_ep *dep)
dep->trb_pool_dma = 0;
}
+static void dwc3_gadget_reset_xfer_resource_for_ep(struct dwc3 *dwc,
>>>
>>> it would be nicer if this would receive struct dwc3_ep *dep as argu
Re: [PATCH] usb: dwc3: Fix assignment of EP transfer resources
Hi,
John Youn writes:
>> If it wasn't for that flag, we would always allocate transfer resource 3
>> for every newly enabled endpoint. Can you check with your RTL engineers
>> if it's valid to *always* issue DEPSTARTCFG with a proper parameter
>> depending on endpoint number ? Basically, something like below:
>>
>> @@ -306,20 +306,14 @@ static int dwc3_gadget_start_config(struct dwc3 *dwc,
>> struct dwc3_ep *dep)
>>
>> memset(¶ms, 0x00, sizeof(params));
>>
>> -if (dep->number != 1) {
>> -cmd = DWC3_DEPCMD_DEPSTARTCFG;
>> -/* XferRscIdx == 0 for ep0 and 2 for the remaining */
>> -if (dep->number > 1) {
>> -if (dwc->start_config_issued)
>> -return 0;
>> -dwc->start_config_issued = true;
>> -cmd |= DWC3_DEPCMD_PARAM(2);
>> -}
>> -
>> -return dwc3_send_gadget_ep_cmd(dwc, 0, cmd, ¶ms);
>> +cmd = DWC3_DEPCMD_DEPSTARTCFG;
>> +/* XferRscIdx == 0 for ep0 and 2 for the remaining */
>> +if (dep->number > 1) {
>> +dwc->start_config_issued = true;
>> +cmd |= DWC3_DEPCMD_PARAM(dwc->current_resource);
>> }
>>
>> -return 0;
>> +return dwc3_send_gadget_ep_cmd(dwc, 0, cmd, ¶ms);
>> }
>>
>> static int dwc3_gadget_set_ep_config(struct dwc3 *dwc, struct dwc3_ep *dep,
>> @@ -388,13 +382,20 @@ static int dwc3_gadget_set_ep_config(struct dwc3 *dwc,
>> struct dwc3_ep *dep,
>> static int dwc3_gadget_set_xfer_resource(struct dwc3 *dwc, struct dwc3_ep
>> *dep)
>> {
>> struct dwc3_gadget_ep_cmd_params params;
>> +int ret;
>>
>> memset(¶ms, 0x00, sizeof(params));
>>
>> params.param0 = DWC3_DEPXFERCFG_NUM_XFER_RES(1);
>>
>> -return dwc3_send_gadget_ep_cmd(dwc, dep->number,
>> +ret = dwc3_send_gadget_ep_cmd(dwc, dep->number,
>> DWC3_DEPCMD_SETTRANSFRESOURCE, ¶ms);
>> +if (ret)
>> +return ret;
>> +
>> +dwc->current_resource++;
>> +
>> +return 0;
>> }
>>
>> /**
>>
>> With this we will *always* use DEPSTARTCFG any time we're enabling an
>> endpoint which hasn't been enabled, but we will always keep transfer
>> resources around. (Note that this won't really work as is, I haven't
>> defined current_resource nor have I made sure to decrement
>> current_resource whenever we disable an endpoint. Also, it might be that
>> using a 32-bit flag instead of a counter might be better, dunno)
>>
>
> Something like this might work too.
>
> I actually have a patch now which *greatly* simplifies all of this
> code and eliminates the start_config_issued flag. But I need the RTL
> engineers to confirm. It should be ok as it relies on the same
> behavior that this current patch does.
>
> Basically assign all the resources in advance.
I thought about that, but wouldn't this, essentially, enable all
endpoints unconditionally ? This could, potentially, increase power
consumption on some systems, right ? This could also cause "spurious"
interrupts if a bogus host tries to move data on an endpoint which
hasn't been enabled yet.
Not sure this is a good approach here.
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH 1/2] usb: dwc3: drop FIFO resizing logic
Hi,
Kishon Vijay Abraham I writes:
> Hi Felipe,
>
> On Thursday 04 February 2016 05:48 PM, Felipe Balbi wrote:
>> That FIFO resizing logic was added to support OMAP5
>> ES1.0 which had a bogus default FIFO size. I can't
>> remember the exact size of default FIFO, but it was
>> less than one bulk superspeed packet (<1024) which
>> would prevent USB3 from ever working on OMAP5 ES1.0.
>>
>> However, OMAP5 ES1.0 support has been dropped by
>> commit aa2f4b16f830 ("ARM: OMAP5: id: Remove ES1.0
>> support") which renders FIFO resizing unnecessary.
>>
>> Signed-off-by: Felipe Balbi
>
> tested this series on both dra7-evm and dra72-evm using mass storage gadget
> and
> msc.sh
both HS and SS ?
> dra72-evm: http://pastebin.ubuntu.com/14887997/
> dra7-evm: http://pastebin.ubuntu.com/14887975/
>
> Tested-by: Kishon Vijay Abraham I
>
> Let me know if you want me to do any other testing on dra7.
yeah, run testusb for a week or so, that usually catches odd bugs.
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH 1/2] usb: dwc3: drop FIFO resizing logic
Hi,
On Wednesday 10 February 2016 03:43 PM, Felipe Balbi wrote:
>
> Hi,
>
> Kishon Vijay Abraham I writes:
>> Hi Felipe,
>>
>> On Thursday 04 February 2016 05:48 PM, Felipe Balbi wrote:
>>> That FIFO resizing logic was added to support OMAP5
>>> ES1.0 which had a bogus default FIFO size. I can't
>>> remember the exact size of default FIFO, but it was
>>> less than one bulk superspeed packet (<1024) which
>>> would prevent USB3 from ever working on OMAP5 ES1.0.
>>>
>>> However, OMAP5 ES1.0 support has been dropped by
>>> commit aa2f4b16f830 ("ARM: OMAP5: id: Remove ES1.0
>>> support") which renders FIFO resizing unnecessary.
>>>
>>> Signed-off-by: Felipe Balbi
>>
>> tested this series on both dra7-evm and dra72-evm using mass storage gadget
>> and
>> msc.sh
>
> both HS and SS ?
yes.. but the logs here were for only SS.
>
>> dra72-evm: http://pastebin.ubuntu.com/14887997/
>> dra7-evm: http://pastebin.ubuntu.com/14887975/
>>
>> Tested-by: Kishon Vijay Abraham I
>>
>> Let me know if you want me to do any other testing on dra7.
>
> yeah, run testusb for a week or so, that usually catches odd bugs.
Okay. I'll start the test before I go for vacation next week.
Thanks
Kishon
--
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/3] usb: type-c: UCSI ACPI driver
On Tue, Feb 09, 2016 at 10:22:46AM -0800, Greg KH wrote: > On Tue, Feb 09, 2016 at 07:01:23PM +0200, Heikki Krogerus wrote: > > Driver for ACPI enumerated UCSI devices. > > What does this mean? > > What does the driver do? Why would we care? > > > > > Signed-off-by: Heikki Krogerus > > --- > > drivers/usb/type-c/Kconfig | 10 > > drivers/usb/type-c/Makefile| 1 + > > drivers/usb/type-c/ucsi_acpi.c | 133 > > + > > 3 files changed, 144 insertions(+) > > create mode 100644 drivers/usb/type-c/ucsi_acpi.c > > > > diff --git a/drivers/usb/type-c/Kconfig b/drivers/usb/type-c/Kconfig > > index 02abd74..72b002e 100644 > > --- a/drivers/usb/type-c/Kconfig > > +++ b/drivers/usb/type-c/Kconfig > > @@ -12,4 +12,14 @@ config TYPEC_UCSI > > registers and data structures used to interface with the USB Type-C > > connectors on a system. > > > > +if TYPEC_UCSI > > + > > +config TYPEC_UCSI_ACPI > > + tristate "UCSI ACPI Driver" > > + depends on ACPI > > + help > > + Driver for ACPI enumerated UCSI devices. > > Worst help text ever :( I'm sorry for that. I was not planning to leave it like that. I was more consearned with the class then these drivers, and forgot finish these parts. 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 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Tue, Feb 09, 2016 at 10:21:55AM -0800, Greg KH wrote: > On Tue, Feb 09, 2016 at 07:01:22PM +0200, Heikki Krogerus wrote: > > USB Type-C Connector System Software Interface (UCSI) is a > > specification that defines registers and data structures > > used to interface with the USB Type-C connectors on a system. > > > > The specification is public and available at: > > http://www.intel.com/content/www/us/en/io/universal-serial-bus/usb-type-c-ucsi-spec.html > > > > What does this driver / code actually do? Why is it needed? What > interface to the rest of the kernel / userspace does it provide? I will fix this describe these things in the commit message. I'll just but some UCSI background in case somebody is interested. So UCSI is in practice a standard for USB Type-C controllers.. UCSI is the control interface for USB Type-C connectors (regardless was USB PD supported or not) in MS Windows, so most likely all new HW platforms designed to work also with Windows that are equipped with USB Type-C will have UCSI device for controlling the USB Type-C ports. In some cases the hardware for Type-C will be just a PHY like fusb30x on these platforms (it's cheaper then USB PD or complete USB Type-C controller), but in those cases the PHY is probable attached to an EC or is completely controlled by system FW like BIOS together with any USB PD communication in cases where USB PD is supported, and is in any case not visible to the OS. Instead UCSI device is exposed to the OS to give it means to apply its policies to the USB Type-C port. > Why would we care about this? I'll try to explain why it's important to export the control of USB Type-C ports to the user space in my answer to your comments to the first patch of this series, the one introducing the class. But surely everybody agrees that decision about the policies regarding USB Type-C ports, like which data role to use, do we charge or are we letting the other end charge, etc., belongs to the user? If you plug your phone to your desktop, I would imagine that you want to see the phone primarily as the USB device and the desktop as host, and to achieve the device role, you don't want to be forced to unplug/replug your phone from the desktop until you achieve device role, right? > You need to describe this a lot better than you did... Sure thing. I'm sorry about the poor description. I send these out too hastily. 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
[PATCH] usb: retry reset if a device times out
Some devices I got show an inability to operate right after power on if they are already connected. They are beyond recovery if the descriptors are requested multiple times. So in case of a timeout we rather bail early and reset again. But it must be done only on the first loop lest we get into a reset/time out spiral that can be overcome with a retry. This patch is a rework of a patch that fell through the cracks. http://www.spinics.net/lists/linux-usb/msg103263.html Signed-off-by: Oliver Neukum CC: sta...@vger.kernel.org --- drivers/usb/core/hub.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index f912fe6..e4e46f6 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -4496,7 +4496,13 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, r = -EPROTO; break; } - if (r == 0) + /* +* Some devices time out if they are powered on +* when already connected. They need a second +* reset. But only on the first attempt, +* lest we get into a time out/reset loop +*/ + if (r == 0 || (r == -ETIMEDOUT && j == 0)) break; } udev->descriptor.bMaxPacketSize0 = -- 2.1.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 1/3] usb: USB Type-C Connector Class
On Tue, Feb 09, 2016 at 10:20:45AM -0800, Greg KH wrote: > On Tue, Feb 09, 2016 at 07:01:21PM +0200, Heikki Krogerus wrote: > > The purpose of this class is to provide unified interface > > for user space to get the status and basic information about > > USB Type-C Connectors in the system, control data role > > swapping, and when USB PD is available, also power role > > swapping and Altenate Modes. > > > > The class will export the following interfaces for every > > USB Type-C Connector in the system to sysfs: > > > > 1. connected - Connection status of the connector > > 2. alternate_mode - The current Alternate Mode > > 3. alternate_modes - Lists all Alternate Modes the connector supports > > 4. partner_alt_modes - Lists partner's Alternate Modes when connected > > 5. partner_type - Can be USB, Charger, Alt Mode or Accessory > > 6. data_role - The current data role, host or device > > 7. data_roles - Data roles supported by the connector > > 8. power_role - Connector's current power role, source or sink > > 9. power_roles - Power roles supported by the connector > > 10. power_operation_mode - The current power level in use > > 11. usb_pd - yes if the connector supports USB PD. > > 12. audio_accessory - yes if the connector supports Audio Accessory > > 13. debug_accessory - yes if the connector supports Debug Accessory > > You forgot to document these sysfs files in Documenataion/ABI :( Man, it's almost like I enjoy making these stupid mistakes :( > And what is userspace going to do with these files? Why does it care? The OS policy regarding USB Type-C ports needs to come from user space. The user must be allowed to select the USB data role, and when USB PD is supported, also the power role, and at the same time we need to export all the relevant information about the USB Type-C ports to the user space, like connection status, the type of partner etc. And all that from a single interface. I'm pretty sure that this is exactly what distributions like Ubuntu, RedHat etc. want, an I know for fact that Chrome OS and Android will expect the user to be in control over the roles and get that information about the ports one way or the other. > > The data_role, power_role and alternate_mode are also > > writable and can be used for executing role swapping and > > entering modes. When USB PD is not supported by the > > connector or partner, power_role will reflect the value of > > the data_role, and is not swappable independently. > > How does this implementation differ from those in other drivers that we > have seen, but not submitted for merging? I'm referring to the code > from Fairchild for their USB Type C driver: > https://github.com/gregkh/fusb30x > and the driver that is in the latest Nexus 6 Android release (don't have > the link to the android kernel tree at the moment sorry, but it's public > and I think Linaro is working on cleaning it up...) That would be USB PD stack and driver for fusb30x USB Type-C/PD PHYs. It's the second USB PD stack I've seen (and sadly also second driver for fusb30x), but I just know there are more. My class is just about exporting control of USB Type-C ports to the user space, and note, USB Type-C *not* USB PD. I don't thing that my little class and the USB PD stack, where ever it ends up coming from, conflict with each other. The only difference is that I'm clearly separating USB Type-C from USB PD (and actually everything else), which is the correct thing to do. USB Type-C is not the same thing as USB PD. USB Type-C does not always come with USB PD. I did not go through that code, but I'm guessing the guys have for example exported similar role swapping controls to user space from some part of their stack. So those guys would just need to register their fusb30x with this class, let the class take care of exporting those controls and probable continue using their USB PD stack exactly like they have done before. I hope I was able to explain myself. 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 1/3] usb: USB Type-C Connector Class
On Tue, 2016-02-09 at 19:01 +0200, Heikki Krogerus wrote:
> The purpose of this class is to provide unified interface
> for user space to get the status and basic information about
> USB Type-C Connectors in the system, control data role
> swapping, and when USB PD is available, also power role
> swapping and Altenate Modes.
>
> The class will export the following interfaces for every
> USB Type-C Connector in the system to sysfs:
>
> 1. connected - Connection status of the connector
> 2. alternate_mode - The current Alternate Mode
> 3. alternate_modes - Lists all Alternate Modes the connector supports
These names are a bit problematic, as they are too similar.
How about
current_alternate_mode
potential_alternate_modes
> 4. partner_alt_modes - Lists partner's Alternate Modes when connected
> 5. partner_type - Can be USB, Charger, Alt Mode or Accessory
> 6. data_role - The current data role, host or device
> 7. data_roles - Data roles supported by the connector
> 8. power_role - Connector's current power role, source or sink
> 9. power_roles - Power roles supported by the connector
> 10. power_operation_mode - The current power level in use
> 11. usb_pd - yes if the connector supports USB PD.
> 12. audio_accessory - yes if the connector supports Audio Accessory
> 13. debug_accessory - yes if the connector supports Debug Accessory
>
> The data_role, power_role and alternate_mode are also
> writable and can be used for executing role swapping and
> entering modes. When USB PD is not supported by the
> connector or partner, power_role will reflect the value of
> the data_role, and is not swappable independently.
>
> Signed-off-by: Heikki Krogerus
> ---
> drivers/usb/Kconfig | 2 +
> drivers/usb/Makefile| 2 +
> drivers/usb/type-c/Kconfig | 7 +
> drivers/usb/type-c/Makefile | 1 +
> drivers/usb/type-c/typec.c | 446
>
> include/linux/usb/typec.h | 114 +++
> 6 files changed, 572 insertions(+)
> create mode 100644 drivers/usb/type-c/Kconfig
> create mode 100644 drivers/usb/type-c/Makefile
> create mode 100644 drivers/usb/type-c/typec.c
> create mode 100644 include/linux/usb/typec.h
>
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 8ed451d..0c45547 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -151,6 +151,8 @@ source "drivers/usb/phy/Kconfig"
>
> source "drivers/usb/gadget/Kconfig"
>
> +source "drivers/usb/type-c/Kconfig"
> +
> config USB_LED_TRIG
> bool "USB LED Triggers"
> depends on LEDS_CLASS && USB_COMMON && LEDS_TRIGGERS
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index d5c57f1..4d712ee 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -61,3 +61,5 @@ obj-$(CONFIG_USB_GADGET)+= gadget/
> obj-$(CONFIG_USB_COMMON) += common/
>
> obj-$(CONFIG_USBIP_CORE) += usbip/
> +
> +obj-$(CONFIG_TYPEC) += type-c/
> diff --git a/drivers/usb/type-c/Kconfig b/drivers/usb/type-c/Kconfig
> new file mode 100644
> index 000..b229fb9
> --- /dev/null
> +++ b/drivers/usb/type-c/Kconfig
> @@ -0,0 +1,7 @@
> +
> +menu "USB PD and Type-C drivers"
> +
> +config TYPEC
> + tristate
> +
> +endmenu
> diff --git a/drivers/usb/type-c/Makefile b/drivers/usb/type-c/Makefile
> new file mode 100644
> index 000..1012a8b
> --- /dev/null
> +++ b/drivers/usb/type-c/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_TYPEC) += typec.o
> diff --git a/drivers/usb/type-c/typec.c b/drivers/usb/type-c/typec.c
> new file mode 100644
> index 000..e425955
> --- /dev/null
> +++ b/drivers/usb/type-c/typec.c
> @@ -0,0 +1,446 @@
> +/*
> + * USB Type-C class
> + *
> + * Copyright (C) 2016, Intel Corporation
> + * Author: Heikki Krogerus
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#define to_typec_port(p) container_of(p, struct typec_port, dev)
> +
> +static DEFINE_IDA(typec_index_ida);
> +
> +/* */
> +
> +int typec_connect(struct typec_port *port)
> +{
> + kobject_uevent(&port->dev.kobj, KOBJ_CHANGE);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(typec_connect);
> +
> +void typec_disconnect(struct typec_port *port)
> +{
> + kobject_uevent(&port->dev.kobj, KOBJ_CHANGE);
> +}
> +EXPORT_SYMBOL_GPL(typec_disconnect);
> +
> +/* */
> +
> +static ssize_t alternate_mode_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct typec_port *port = to_typec_port(dev);
> + struct typec_alt_mode alt_mode;
> + int ret;
> +
> + if (!port->cap->set_alt_mode) {
> + dev_warn(dev, "entering Alternate Modes not suppor
Re: [PATCH 1/3] usb: USB Type-C Connector Class
On Wed, Feb 10, 2016 at 12:49 PM, Oliver Neukum wrote: > On Tue, 2016-02-09 at 19:01 +0200, Heikki Krogerus wrote: >> The purpose of this class is to provide unified interface >> for user space to get the status and basic information about >> USB Type-C Connectors in the system, control data role >> swapping, and when USB PD is available, also power role >> swapping and Altenate Modes. >> >> The class will export the following interfaces for every >> USB Type-C Connector in the system to sysfs: >> >> 1. connected - Connection status of the connector >> 2. alternate_mode - The current Alternate Mode >> 3. alternate_modes - Lists all Alternate Modes the connector supports > > These names are a bit problematic, as they are too similar. > How about > > current_alternate_mode > potential_alternate_modes I would vote for supported_* >> 4. partner_alt_modes - Lists partner's Alternate Modes when connected >> 5. partner_type - Can be USB, Charger, Alt Mode or Accessory >> 6. data_role - The current data role, host or device >> 7. data_roles - Data roles supported by the connector >> 8. power_role - Connector's current power role, source or sink >> 9. power_roles - Power roles supported by the connector >> 10. power_operation_mode - The current power level in use >> 11. usb_pd - yes if the connector supports USB PD. >> 12. audio_accessory - yes if the connector supports Audio Accessory >> 13. debug_accessory - yes if the connector supports Debug Accessory -- With Best Regards, Andy Shevchenko -- 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/3] usb: USB Type-C Connector Class
On Wed, Feb 10, 2016 at 01:05:27PM +0200, Andy Shevchenko wrote: > On Wed, Feb 10, 2016 at 12:49 PM, Oliver Neukum wrote: > > On Tue, 2016-02-09 at 19:01 +0200, Heikki Krogerus wrote: > >> The purpose of this class is to provide unified interface > >> for user space to get the status and basic information about > >> USB Type-C Connectors in the system, control data role > >> swapping, and when USB PD is available, also power role > >> swapping and Altenate Modes. > >> > >> The class will export the following interfaces for every > >> USB Type-C Connector in the system to sysfs: > >> > >> 1. connected - Connection status of the connector > >> 2. alternate_mode - The current Alternate Mode > >> 3. alternate_modes - Lists all Alternate Modes the connector supports > > > > These names are a bit problematic, as they are too similar. > > How about > > > > current_alternate_mode That works for me. > > potential_alternate_modes > > I would vote for supported_* How about connector_alternate_modes? 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 2/3] usb: type-c: USB Type-C Connector System Software Interface
> +static int ucsi_run_cmd(struct ucsi *ucsi, void *data, size_t size)
> +{
> + int status;
> + int ret;
> +
> + dev_vdbg(ucsi->dev, "%s control 0x%llx\n", __func__,
> + ucsi->ppm->data->control);
> +
> + ret = ucsi->ppm->cmd(ucsi->ppm);
> + if (ret)
> + return ret;
> +
> + /* REVISIT: We may need to set UCSI_CCI_CMD_COMPLETE flag here */
> + wait_for_completion(&ucsi->complete);
> +
> + status = ucsi->status;
> + if (status != UCSI_ERROR && size)
> + memcpy(data, ucsi->ppm->data->message_in, size);
> +
> + ret = ucsi_ack(ucsi, UCSI_ACK_CMD);
> + if (ret)
> + goto out;
> +
> + if (status == UCSI_ERROR) {
> + u16 error;
> +
> + ucsi->ppm->data->control = UCSI_GET_ERROR_STATUS;
> + ret = ucsi->ppm->cmd(ucsi->ppm);
> + if (ret)
> + goto out;
> +
> + wait_for_completion(&ucsi->complete);
> +
> + /* Something has really gone wrong */
> + if (ucsi->status == UCSI_ERROR) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + memcpy(&error, ucsi->ppm->data->message_in, sizeof(error));
> +
> + ret = ucsi_ack(ucsi, UCSI_ACK_CMD);
> + if (ret)
> + goto out;
> +
> + switch (error) {
> + case UCSI_ERROR_INVALID_CON_NUM:
> + ret = -ENXIO;
> + break;
> + case UCSI_ERROR_INCOMPATIBLE_PARTNER:
> + case UCSI_ERROR_CC_COMMUNICATION_ERR:
> + case UCSI_ERROR_CONTRACT_NEGOTIATION_FAIL:
> + ret = -EIO;
> + break;
This looks like you mapped all the interesting error condition
on a single error code and gave out other error codes to
conditions of lesser importance.
> + case UCSI_ERROR_DEAD_BATTERY:
> + dev_warn(ucsi->dev, "Dead Battery Condition!\n");
> + ret = -EPERM;
> + break;
> + case UCSI_ERROR_UNREGONIZED_CMD:
> + case UCSI_ERROR_INVALID_CMD_ARGUMENT:
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + }
> +out:
> + ucsi->ppm->data->control = 0;
> + return ret;
> +}
[..]
> +/**
> + * ucsi_init - Initialize an UCSI Interface
> + * @ucsi: The UCSI Interface
> + *
> + * Registers all the USB Type-C ports governed by the PPM of @ucsi and
> enables
> + * all the notifications from the PPM.
> + */
> +int ucsi_init(struct ucsi *ucsi)
> +{
> + struct ucsi_control *ctrl = (void *)&ucsi->ppm->data->control;
> + struct ucsi_connector *con;
> + int ret;
> + int i;
> +
> + /* Enable basic notifications */
> + ctrl->cmd = UCSI_SET_NOTIFICATION_ENABLE;
> + ctrl->data = UCSI_ENABLE_NTFY_CMD_COMPLETE | UCSI_ENABLE_NTFY_ERROR;
> + ret = ucsi_run_cmd(ucsi, NULL, 0);
> + if (ret)
> + return ret;
> +
> + /* Get PPM capabilities */
> + ctrl->cmd = UCSI_GET_CAPABILITY;
> + ret = ucsi_run_cmd(ucsi, &ucsi->cap, sizeof(ucsi->cap));
> + if (ret)
> + return ret;
> +
> + ucsi->connector = kcalloc(ucsi->cap.num_connectors,
> + sizeof(struct ucsi_connector), GFP_KERNEL);
> + if (!ucsi->connector)
> + return -ENOMEM;
> +
> + for (i = 0, con = ucsi->connector; i < ucsi->cap.num_connectors;
> + i++, con++) {
> + struct typec_capability *cap = &con->typec_cap;
> + struct ucsi_connector_status constat;
> +
> + /* Get connector capability */
> + ctrl->cmd = UCSI_GET_CONNECTOR_CAPABILITY;
> + ctrl->data = i + 1;
> + ret = ucsi_run_cmd(ucsi, &con->cap, sizeof(con->cap));
> + if (ret)
> + goto err;
> +
> + /* Register the connector */
> +
> + if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DRP)
> + cap->type = TYPEC_PORT_DRP;
> + else if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DFP)
> + cap->type = TYPEC_PORT_DFP;
> + else if (con->cap.op_mode & UCSI_CONCAP_OPMODE_UFP)
> + cap->type = TYPEC_PORT_UFP;
> +
> + cap->usb_pd = !!(ucsi->cap.attributes &
> +UCSI_CAP_ATTR_USB_PD);
> + cap->audio_accessory = !!(con->cap.op_mode &
> + UCSI_CONCAP_OPMODE_AUDIO_ACCESSORY);
> + cap->debug_accessory = !!(con->cap.op_mode &
> + UCSI_CONCAP_OPMODE_DEBUG_ACCESSORY);
> +
> + /* TODO: Alt modes */
> +
> + cap->dr_swap = ucsi_dr_swap;
> + cap->pr_swap = ucsi_pr_swap;
> +
> + con->port = typec_register_port(ucsi->dev, cap);
> + if (IS
Re: [PATCH 1/3] usb: USB Type-C Connector Class
Hi Oliver,
> > +static ssize_t alternate_mode_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t size)
> > +{
> > + struct typec_port *port = to_typec_port(dev);
> > + struct typec_alt_mode alt_mode;
> > + int ret;
> > +
> > + if (!port->cap->set_alt_mode) {
> > + dev_warn(dev, "entering Alternate Modes not supported\n");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + if (!port->connected)
> > + return -ENXIO;
>
> Doesn't this need locking?
Yes, I need to fix the locking.
> And why wouldn't user space want to preselect a mode?
That is tricky, as we would need to keep a list of the preselected
modes and for all SVIDs the connector supports. I don't think it would
be practical to do from this file as we would then use it differently
when connected and not connected, so the preselected modes would
probable be better to give from a separate file.
That is certainly doable, but is it really useful? I not really
against adding that support, but I would like to keep this interface
as simple as possible.
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 1/2] usb: dwc3: drop FIFO resizing logic
Hi,
Kishon Vijay Abraham I writes:
>> Kishon Vijay Abraham I writes:
>>> Hi Felipe,
>>>
>>> On Thursday 04 February 2016 05:48 PM, Felipe Balbi wrote:
That FIFO resizing logic was added to support OMAP5
ES1.0 which had a bogus default FIFO size. I can't
remember the exact size of default FIFO, but it was
less than one bulk superspeed packet (<1024) which
would prevent USB3 from ever working on OMAP5 ES1.0.
However, OMAP5 ES1.0 support has been dropped by
commit aa2f4b16f830 ("ARM: OMAP5: id: Remove ES1.0
support") which renders FIFO resizing unnecessary.
Signed-off-by: Felipe Balbi
>>>
>>> tested this series on both dra7-evm and dra72-evm using mass storage gadget
>>> and
>>> msc.sh
>>
>> both HS and SS ?
>
> yes.. but the logs here were for only SS.
cool thanks. Did you notice any speed regression or is it all the same ?
>>> Let me know if you want me to do any other testing on dra7.
>>
>> yeah, run testusb for a week or so, that usually catches odd bugs.
>
> Okay. I'll start the test before I go for vacation next week.
great
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH 1/1] Drivers: USB: DA8xx MUSB: added DT support
On 09.02.2016 11:50, Sergei Shtylyov wrote: Doesn't usb_get_dr_mode() work for you? I can't really say because I can't test it. Is this the replacement for of_usb_get_dr_mode() ? Seems so. The call in musb_dsps.c was just replaced. Thanks, Sergei. Yesterday I submitted the (hopefully) final version version of the patch based on Felipe's "next" branch and incorporating all you guys' feedback. Also the binding was submitted separately. Please let me know if anything else needs to be amended. Thanks Petr -- 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/3] usb: USB Type-C Connector Class
On Wed, Feb 10, 2016 at 1:11 PM, Heikki Krogerus wrote: > On Wed, Feb 10, 2016 at 01:05:27PM +0200, Andy Shevchenko wrote: >> On Wed, Feb 10, 2016 at 12:49 PM, Oliver Neukum wrote: >> > On Tue, 2016-02-09 at 19:01 +0200, Heikki Krogerus wrote: >> >> The purpose of this class is to provide unified interface >> >> for user space to get the status and basic information about >> >> USB Type-C Connectors in the system, control data role >> >> swapping, and when USB PD is available, also power role >> >> swapping and Altenate Modes. >> >> >> >> The class will export the following interfaces for every >> >> USB Type-C Connector in the system to sysfs: >> >> >> >> 1. connected - Connection status of the connector >> >> 2. alternate_mode - The current Alternate Mode >> >> 3. alternate_modes - Lists all Alternate Modes the connector supports >> > >> > These names are a bit problematic, as they are too similar. >> > How about >> > >> > current_alternate_mode > > That works for me. > >> > potential_alternate_modes >> >> I would vote for supported_* > > How about connector_alternate_modes? Would be fine as well. -- With Best Regards, Andy Shevchenko -- 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: [PATCHv2 1/2] usb: dwc3: pci: use build-in properties instead of platform data
Hi John, > Hi Heikki, > > The properties are now set using your patch. > > However I get a use-after-free error when I unload the driver: OK. I need to prepare proper fixes for the fwnode handling, but I think these patches are in any case OK. Though they now depend on those fixes of course. 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: ULPI Driver help
Hi Sudhakar, On Mon, Feb 08, 2016 at 11:20:44PM -0800, Sudhakar Krishnan wrote: > Hi Heikki, > > First of all I would like to apologize for sending you random email. I > found your name from ULPI driver change list > > I am hobbyist developer using baytrail tablet (atom Z3537) and trying to > port linux in to it. Having trouble in bringing up PMIC AXP288 since its > failing to load due to USB phy detection failure. Could you share more details about your platform, and perhaps share your kernel cofing file and dmesg output. And just in case, which kernel are you using? Why would PMIC driver complain about USB PHY? I'm going to add the Linux usb mailing list. I'm sure there are people more familiar with your platform then I am. > When I was searching for a solution found lot of thread talking about > ULPI+baytrail > > https://lkml.org/lkml/2015/3/18/271 > > The communication is so big I am finding difficult to get the required > patches. > > Could you please let me know the changes up-streamed? or any github I can > refer to get the latest changes. Yes, those patches were upstreamed and are available in mainline. To use them just make sure you have enabled DWC3 ULPI support and the tusb1210 PHY driver (though not all Baytrails used that particular USB2 PHY). Cheers, -- 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 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Tue, Feb 9, 2016 at 7:01 PM, Heikki Krogerus
wrote:
> USB Type-C Connector System Software Interface (UCSI) is a
> specification that defines registers and data structures
> used to interface with the USB Type-C connectors on a system.
>
> The specification is public and available at:
> http://www.intel.com/content/www/us/en/io/universal-serial-bus/usb-type-c-ucsi-spec.html
+#define to_ucsi_connector(_port_) container_of(_port_->cap,
\
+ struct ucsi_connector, \
+ typec_cap)
Perhaps
static inline ...
+
+#define cci_to_connector(_ucsi_, cci) (_ucsi_->connector +\
+ UCSI_CCI_CONNECTOR_CHANGE(cci) - 1)
+
If you start you macro on the next line it will look better.
> +static int
> +ucsi_connect(struct ucsi_connector *con, struct ucsi_connector_status
> *constat)
> +{
> + struct typec_port *port = con->port;
> +
> + port->connected = true;
> +
> + if (constat->partner_flags & UCSI_CONSTAT_PARTNER_FLAG_ALT_MODE)
> + port->partner_type = TYPEC_PARTNER_ALTMODE;
> + else
> + port->partner_type = TYPEC_PARTNER_USB;
> +
> + switch (constat->partner_type) {
> + case UCSI_CONSTAT_PARTNER_TYPE_CABLE_NO_UFP:
> + /* REVISIT: We don't care about just the cable for now */
> + return 0;
> + case UCSI_CONSTAT_PARTNER_TYPE_DFP:
> + case UCSI_CONSTAT_PARTNER_TYPE_CABLE_AND_UFP:
> + port->pwr_role = TYPEC_PWR_SINK;
> + port->data_role = TYPEC_DEVICE;
> + break;
> + case UCSI_CONSTAT_PARTNER_TYPE_UFP:
> + port->pwr_role = TYPEC_PWR_SOURCE;
> + port->data_role = TYPEC_HOST;
> + break;
> + case UCSI_CONSTAT_PARTNER_TYPE_DEBUG:
> + port->partner_type = TYPEC_PARTNER_DEBUG;
> + goto out;
> + case UCSI_CONSTAT_PARTNER_TYPE_AUDIO:
> + port->partner_type = TYPEC_PARTNER_AUDIO;
> + goto out;
> + }
> +
> + switch (constat->pwr_op_mode) {
> + case UCSI_CONSTAT_PWR_OPMODE_NONE:
> + case UCSI_CONSTAT_PWR_OPMODE_DEFAULT:
> + port->pwr_opmode = TYPEC_PWR_MODE_USB;
> + break;
> + case UCSI_CONSTAT_PWR_OPMODE_BC:
> + port->partner_type = TYPEC_PARTNER_CHARGER;
> + port->pwr_opmode = TYPEC_PWR_MODE_BC1_2;
> + break;
> + case UCSI_CONSTAT_PWR_OPMODE_PD:
> + port->pwr_opmode = TYPEC_PWR_MODE_PD;
> + break;
> + case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_3:
> + port->pwr_opmode = TYPEC_PWR_MODE_1_5A;
> + break;
> + case UCSI_CONSTAT_PWR_OPMODE_TYPEC3_0:
> + port->pwr_opmode = TYPEC_PWR_MODE_3_0A;
> + break;
> + default:
> + break;
> + }
> +out:
CodingStyle suggests to do a better label naming.
> + return typec_connect(port);
> +}
> +static void ucsi_connector_change(struct work_struct *work)
> +{
> + struct ucsi_connector *con = container_of(work, struct ucsi_connector,
> + work);
> +}
> +int ucsi_init(struct ucsi *ucsi)
> +{
> + struct ucsi_control *ctrl = (void *)&ucsi->ppm->data->control;
> + struct ucsi_connector *con;
> + int ret;
> + int i;
> + for (i = 0, con = ucsi->connector; i < ucsi->cap.num_connectors;
> +i++, con++) {
> +err:
> + if (i > 0)
> + for (; i >= 0; i--, con--)
> + typec_unregister_port(con->port);
Perhaps
while (--i >= 0) {
...
}
But...
> +
> + kfree(ucsi->connector);
> + return ret;
> +}
> +struct ucsi *ucsi_register_ppm(struct device *dev, struct ucsi_ppm *ppm)
> +{
> + struct ucsi *ucsi;
> +
> + ucsi = kzalloc(sizeof(*ucsi), GFP_KERNEL);
> + if (!ucsi)
> + return ERR_PTR(-ENOMEM);
> +
> + init_completion(&ucsi->complete);
> + ucsi->dev = dev;
> + ucsi->ppm = ppm;
> +
> + return ucsi;
> +}
> +EXPORT_SYMBOL_GPL(ucsi_register_ppm);
> +
> +/**
> + * ucsi_unregister_ppm - Unregister UCSI PPM Interface
> + * @ucsi: struct ucsi associated with the PPM
> + *
> + * Unregister an UCSI PPM that was created with ucsi_register().
> + */
> +void ucsi_unregister_ppm(struct ucsi *ucsi)
> +{
> + struct ucsi_connector *con;
> + int i;
> +
> + /* Disable all notifications */
> + ucsi->ppm->data->control = UCSI_SET_NOTIFICATION_ENABLE;
> + ucsi->ppm->cmd(ucsi->ppm);
> +
> + for (i = 0, con = ucsi->connector; i < ucsi->cap.num_connectors;
> +i++, con++)
> + typec_unregister_port(con->port);
...looks a code duplication.
> +
> + kfree(ucsi->connector);
> + kfree(ucsi);
> +}
> +EXPORT_SYMBOL_GPL(ucsi_un
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Wed, Feb 10, 2016 at 12:19:53PM +0100, Oliver Neukum wrote:
>
> > +static int ucsi_run_cmd(struct ucsi *ucsi, void *data, size_t size)
> > +{
> > + int status;
> > + int ret;
> > +
> > + dev_vdbg(ucsi->dev, "%s control 0x%llx\n", __func__,
> > +ucsi->ppm->data->control);
> > +
> > + ret = ucsi->ppm->cmd(ucsi->ppm);
> > + if (ret)
> > + return ret;
> > +
> > + /* REVISIT: We may need to set UCSI_CCI_CMD_COMPLETE flag here */
> > + wait_for_completion(&ucsi->complete);
> > +
> > + status = ucsi->status;
> > + if (status != UCSI_ERROR && size)
> > + memcpy(data, ucsi->ppm->data->message_in, size);
> > +
> > + ret = ucsi_ack(ucsi, UCSI_ACK_CMD);
> > + if (ret)
> > + goto out;
> > +
> > + if (status == UCSI_ERROR) {
> > + u16 error;
> > +
> > + ucsi->ppm->data->control = UCSI_GET_ERROR_STATUS;
> > + ret = ucsi->ppm->cmd(ucsi->ppm);
> > + if (ret)
> > + goto out;
> > +
> > + wait_for_completion(&ucsi->complete);
> > +
> > + /* Something has really gone wrong */
> > + if (ucsi->status == UCSI_ERROR) {
> > + ret = -ENODEV;
> > + goto out;
> > + }
> > +
> > + memcpy(&error, ucsi->ppm->data->message_in, sizeof(error));
> > +
> > + ret = ucsi_ack(ucsi, UCSI_ACK_CMD);
> > + if (ret)
> > + goto out;
> > +
> > + switch (error) {
> > + case UCSI_ERROR_INVALID_CON_NUM:
> > + ret = -ENXIO;
> > + break;
> > + case UCSI_ERROR_INCOMPATIBLE_PARTNER:
> > + case UCSI_ERROR_CC_COMMUNICATION_ERR:
> > + case UCSI_ERROR_CONTRACT_NEGOTIATION_FAIL:
> > + ret = -EIO;
> > + break;
>
> This looks like you mapped all the interesting error condition
> on a single error code and gave out other error codes to
> conditions of lesser importance.
OK, I'll fix those.
> > + case UCSI_ERROR_DEAD_BATTERY:
> > + dev_warn(ucsi->dev, "Dead Battery Condition!\n");
> > + ret = -EPERM;
> > + break;
> > + case UCSI_ERROR_UNREGONIZED_CMD:
> > + case UCSI_ERROR_INVALID_CMD_ARGUMENT:
> > + default:
> > + ret = -EINVAL;
> > + break;
> > + }
> > + }
> > +out:
> > + ucsi->ppm->data->control = 0;
> > + return ret;
> > +}
>
> [..]
>
> > +/**
> > + * ucsi_init - Initialize an UCSI Interface
> > + * @ucsi: The UCSI Interface
> > + *
> > + * Registers all the USB Type-C ports governed by the PPM of @ucsi and
> > enables
> > + * all the notifications from the PPM.
> > + */
> > +int ucsi_init(struct ucsi *ucsi)
> > +{
> > + struct ucsi_control *ctrl = (void *)&ucsi->ppm->data->control;
> > + struct ucsi_connector *con;
> > + int ret;
> > + int i;
> > +
> > + /* Enable basic notifications */
> > + ctrl->cmd = UCSI_SET_NOTIFICATION_ENABLE;
> > + ctrl->data = UCSI_ENABLE_NTFY_CMD_COMPLETE | UCSI_ENABLE_NTFY_ERROR;
> > + ret = ucsi_run_cmd(ucsi, NULL, 0);
> > + if (ret)
> > + return ret;
> > +
> > + /* Get PPM capabilities */
> > + ctrl->cmd = UCSI_GET_CAPABILITY;
> > + ret = ucsi_run_cmd(ucsi, &ucsi->cap, sizeof(ucsi->cap));
> > + if (ret)
> > + return ret;
> > +
> > + ucsi->connector = kcalloc(ucsi->cap.num_connectors,
> > + sizeof(struct ucsi_connector), GFP_KERNEL);
> > + if (!ucsi->connector)
> > + return -ENOMEM;
> > +
> > + for (i = 0, con = ucsi->connector; i < ucsi->cap.num_connectors;
> > +i++, con++) {
> > + struct typec_capability *cap = &con->typec_cap;
> > + struct ucsi_connector_status constat;
> > +
> > + /* Get connector capability */
> > + ctrl->cmd = UCSI_GET_CONNECTOR_CAPABILITY;
> > + ctrl->data = i + 1;
> > + ret = ucsi_run_cmd(ucsi, &con->cap, sizeof(con->cap));
> > + if (ret)
> > + goto err;
> > +
> > + /* Register the connector */
> > +
> > + if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DRP)
> > + cap->type = TYPEC_PORT_DRP;
> > + else if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DFP)
> > + cap->type = TYPEC_PORT_DFP;
> > + else if (con->cap.op_mode & UCSI_CONCAP_OPMODE_UFP)
> > + cap->type = TYPEC_PORT_UFP;
> > +
> > + cap->usb_pd = !!(ucsi->cap.attributes &
> > + UCSI_CAP_ATTR_USB_PD);
> > + cap->audio_accessory = !!(con->cap.op_mode &
> > + UCSI_CONCAP_OPMODE_AUDIO_ACCESSORY);
> > + cap->debug_accessory = !!(con->cap.op_mode &
> > + UCSI_CONCAP_OPMODE_DEBUG_ACCESSORY);
> > +
> > + /* TODO: Alt modes */
> > +
> > +
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Tue, 2016-02-09 at 19:01 +0200, Heikki Krogerus wrote: > +#define UCSI_CONSTAT_BC_NOT_CHARGING 0 > +#define UCSI_CONSTAT_BC_NOMINAL_CHARGING 1 > +#define UCSI_CONSTAT_BC_SLOW_CHARGING 2 > +#define UCSI_CONSTAT_BC_TRICLE_CHARGING3 typo. It is spelled TRICKLE Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Wed, 2016-02-10 at 13:56 +0200, Andy Shevchenko wrote: > > +out: > > CodingStyle suggests to do a better label naming. Names coming from specs are what they are. There is no place for coding style here. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Wed, Feb 10, 2016 at 3:21 PM, Oliver Neukum wrote: > On Wed, 2016-02-10 at 13:56 +0200, Andy Shevchenko wrote: >> > +out: >> >> CodingStyle suggests to do a better label naming. > > Names coming from specs are what they are. There is > no place for coding style here. Yes, and how is it related to C label names? -- With Best Regards, Andy Shevchenko -- 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/2] USB: musb: DA8xx: Add DT support for the DA8xx driver
Hello.
On 2/9/2016 3:23 PM, Petr Kulhavy wrote:
Adds DT support for the TI DA830 and DA850 USB driver.
Signed-off-by: Petr Kulhavy
---
drivers/usb/musb/da8xx.c | 136 +++
drivers/usb/musb/musb_core.c | 24
drivers/usb/musb/musb_core.h | 2 +
3 files changed, 162 insertions(+)
diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index b03d3b8..6573600 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -1,6 +1,9 @@
/*
* Texas Instruments DA8xx/OMAP-L1x "glue layer"
*
+ * DT support
+ * Copyright (c) 2016 Petr Kulhavy, Barix AG
+ *
Could you place this after MV's copyright?
* Copyright (c) 2008-2009 MontaVista Software, Inc.
*
* Based on the DaVinci "glue layer" code.
@@ -36,6 +39,7 @@
#include
#include
+#include
#include "musb_core.h"
@@ -134,6 +138,35 @@ static inline void phy_off(void)
__raw_writel(cfgchip2, CFGCHIP2);
}
+/* converts PHY refclk frequency in Hz into USB0REF_FREQ config value
+ * on unsupported frequency returns -1
+ */
+static inline int phy_refclk_cfg(int frequency)
+{
+ switch (frequency) {
+ case 1200:
+ return 0x01;
There's a macro for this.
+ case 1300:
+ return 0x06;
+ case 1920:
+ return 0x05;
+ case 2000:
+ return 0x08;
+ case 2400:
+ return 0x02;
And for this.
+ case 2600:
+ return 0x07;
+ case 3840:
+ return 0x05;
+ case 4000:
+ return 0x09;
+ case 4800:
+ return 0x03;
And for this.
+ default:
+ return -1;
I suggest that you first add the macros for the ones not #define'd (in a
separate patch).
[...]
@@ -515,6 +551,90 @@ static int da8xx_probe(struct platform_device *pdev)
glue->dev= &pdev->dev;
glue->clk= clk;
+ if (IS_ENABLED(CONFIG_OF) && np) {
+ const struct of_device_id *match;
+ const struct musb_hdrc_config *matched_config;
+ struct musb_hdrc_config *config;
+ struct musb_hdrc_platform_data *data;
+ u32 phy20_refclock_freq, phy20_clkmux_cfg;
+ u32 cfgchip2;
+ unsigned power_ma;
+ int ret;
+
+ match = of_match_device(da8xx_id_table, &pdev->dev);
+ if (!match) {
+ ret = -ENODEV;
+ goto err5;
+ }
+ matched_config = match->data;
There's no dire need for initializing the DT device data yet (and there
will hardly be one I think).
+
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ ret = -ENOMEM;
+ goto err5;
+ }
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
Why duplicate the platform data?!
+ if (!data) {
+ ret = -ENOMEM;
+ goto err5;
+ }
+
+ config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
Why not just use the static config?!
+ if (!config) {
+ ret = -ENOMEM;
+ goto err5;
+ }
+
+ config->num_eps = matched_config->num_eps;
+ config->ram_bits = matched_config->ram_bits;
+ config->multipoint = matched_config->multipoint;
+ pdata->config= config;
+ pdata->board_data= data;
What?! Why store a pointer to self?!
+ pdata->mode = musb_get_dr_mode(&pdev->dev);
+
+ ret = of_property_read_u32(np, "mentor,power", &power_ma);
I told you this is MUSB generic prop and so should be parsed in musb_core.c.
+ if (ret)
+ power_ma = 0;
+
+ /* the "mentor,power" value is in milli-amps, whereas
milli-Ampers, no?
+* the mentor configuration is in 2mA units
+*/
+ pdata->power = power_ma / 2;
+
+ /* optional parameter reference clock frequency
+* true = use PLL, false = use external clock pin
+*/
+ phy20_clkmux_cfg =
+ of_property_read_bool(np, "ti,phy20-clkmux-pll") ?
+ CFGCHIP2_USB2PHYCLKMUX : 0;
No dire need for this variable...
+
+ cfgchip2 = __raw_readl(CFGCHIP2);
+ cfgchip2 &= ~CFGCHIP2_USB2PHYCLKMUX;
+ cfgchip2 |= phy20_clkmux_cfg;
+ __raw_writel(cfgchip2, CFGCHIP2);
+
+ /* optional parameter reference clock frequency */
+ if
Re: [PATCH 1/1] Drivers: USB: DA8xx MUSB: added DT support
On 2/10/2016 2:26 PM, Petr Kulhavy wrote: Doesn't usb_get_dr_mode() work for you? I can't really say because I can't test it. Is this the replacement for of_usb_get_dr_mode() ? Seems so. The call in musb_dsps.c was just replaced. Thanks, Sergei. Yesterday I submitted the (hopefully) final version version Hope dies last. :-) There's too much cut&pasted code in your patch still... of the patch based on Felipe's "next" branch and incorporating all you guys' feedback. Also the binding was submitted separately. I'll go find it, haven't seen yet. Please let me know if anything else needs to be amended. I've just reviewed the glue patch. Thanks Petr MBR, Sergei -- 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/2] USB: musb: DA8xx: Add DT support for the DA8xx driver
On 2/10/2016 5:05 PM, Sergei Shtylyov wrote: Adds DT support for the TI DA830 and DA850 USB driver. And I'd write this as DAA8xx/OMAP-L1x/AM17xx/AM18xx. No need to be specific here. Signed-off-by: Petr Kulhavy [...] MBR, Sergei -- 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/1] Drivers: USB: DA8xx MUSB: added DT support
On 2/10/2016 5:07 PM, Sergei Shtylyov wrote: [...] Thanks, Sergei. Yesterday I submitted the (hopefully) final version version Hope dies last. :-) There's too much cut&pasted code in your patch still... of the patch based on Felipe's "next" branch and incorporating all you guys' feedback. Also the binding was submitted separately. I'll go find it, haven't seen yet. I just haven't received it. Please make sure to CC at least linux-usb next time you post it. Thanks Petr MBR, Sergei -- 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/3] usb: type-c: USB Type-C Connector System Software Interface
On Wed, 2016-02-10 at 13:56 +0200, Andy Shevchenko wrote:
> > +err:
> > + if (i > 0)
> > + for (; i >= 0; i--, con--)
> > + typec_unregister_port(con->port);
>
> Perhaps
>
> while (--i >= 0) {
> ...
> }
While we are at it. No we should not change the semantics
of conditionals for the sake of appearance.
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Wed, Feb 10, 2016 at 4:15 PM, Oliver Neukum wrote:
> On Wed, 2016-02-10 at 13:56 +0200, Andy Shevchenko wrote:
>> > +err:
>> > + if (i > 0)
>> > + for (; i >= 0; i--, con--)
>> > + typec_unregister_port(con->port);
>>
>> Perhaps
>>
>> while (--i >= 0) {
>> ...
>> }
>
> While we are at it. No we should not change the semantics
> of conditionals for the sake of appearance.
I'm sorry I didn't get you.
How this more or less standard pattern to clean up stuff on error path
does with conditional semantics?
I also noticed that this code might be factored out to helper which
will do same things (only number of loops is different) in both cases.
What did I miss?
--
With Best Regards,
Andy Shevchenko
--
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/1] Drivers: USB: DA8xx MUSB: added DT support
On 10.02.2016 15:12, Sergei Shtylyov wrote: of the patch based on Felipe's "next" branch and incorporating all you guys' feedback. Also the binding was submitted separately. I'll go find it, haven't seen yet. I just haven't received it. Please make sure to CC at least linux-usb next time you post it. I've just followed the instructions in Documentation/devicetree/bindings/submitting-patches.txt as you hinted me last time. It doesn't say the subsystem mailing list should be CCed as well. Neither does the get_maintainer.pl print the linux-usb list. I'm sorry! :-7 But sure, next time I'll copy the list as well. Petr -- 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 02/21] usb: add HAS_IOMEM dependency to USB_NET2272
drivers/usb/gadget/udc/net2272.c: In function ‘net2272_remove’: drivers/usb/gadget/udc/net2272.c:2232:2: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration] iounmap(dev->base_addr); ^ drivers/usb/gadget/udc/net2272.c: In function ‘net2272_plat_probe’: drivers/usb/gadget/udc/net2272.c:2650:2: error: implicit declaration of function ‘ioremap_nocache’ [-Werror=implicit-function-declaration] dev->base_addr = ioremap_nocache(base, len); ^ drivers/usb/gadget/udc/net2272.c:2650:17: warning: assignment makes pointer from integer without a cast [enabled by default] dev->base_addr = ioremap_nocache(base, len); ^ Signed-off-by: Vegard Nossum --- drivers/usb/gadget/udc/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig index 753c29b..ca19f6f 100644 --- a/drivers/usb/gadget/udc/Kconfig +++ b/drivers/usb/gadget/udc/Kconfig @@ -287,6 +287,7 @@ config USB_FSL_QE dynamically linked module called "fsl_qe_udc". config USB_NET2272 + depends on HAS_IOMEM tristate "PLX NET2272" help PLX NET2272 is a USB peripheral controller which supports -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/21] usb: Add HAS_IOMEM dependency to USB_M66592
drivers/usb/gadget/udc/m66592-udc.c: In function ‘m66592_remove’: drivers/usb/gadget/udc/m66592-udc.c:1538:2: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration] iounmap(m66592->reg); ^ drivers/usb/gadget/udc/m66592-udc.c: In function ‘m66592_probe’: drivers/usb/gadget/udc/m66592-udc.c:1577:2: error: implicit declaration of function ‘ioremap’ [-Werror=implicit-function-declaration] reg = ioremap(res->start, resource_size(res)); ^ drivers/usb/gadget/udc/m66592-udc.c:1577:6: warning: assignment makes pointer from integer without a cast [enabled by default] reg = ioremap(res->start, resource_size(res)); ^ Signed-off-by: Vegard Nossum --- drivers/usb/gadget/udc/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig index ca19f6f..e06efa3 100644 --- a/drivers/usb/gadget/udc/Kconfig +++ b/drivers/usb/gadget/udc/Kconfig @@ -244,6 +244,7 @@ config USB_MV_U3D config USB_M66592 tristate "Renesas M66592 USB Peripheral Controller" + depends on HAS_IOMEM help M66592 is a discrete USB peripheral controller chip that supports both full and high speed USB 2.0 data transfers. -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 19/21] usb: add HAS_IOMEM dependency to USB_ISP1362_HCD
drivers/built-in.o: In function `isp1362_probe': /home/vegard/linux/drivers/usb/host/isp1362-hcd.c:2668: undefined reference to `devm_ioremap_resource' Signed-off-by: Vegard Nossum --- drivers/usb/host/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 85230d6..438dcf6 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -357,6 +357,7 @@ config USB_ISP116X_HCD config USB_ISP1362_HCD tristate "ISP1362 HCD support" + depends on HAS_IOMEM ---help--- Supports the Philips ISP1362 chip as a host controller -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/21] usb: add HAS_IOMEM dependency to USB_ISP116X_HCD
drivers/usb/host/isp116x-hcd.c: In function ‘isp116x_remove’: drivers/usb/host/isp116x-hcd.c:1552:2: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration] iounmap(isp116x->data_reg); ^ drivers/usb/host/isp116x-hcd.c: In function ‘isp116x_probe’: drivers/usb/host/isp116x-hcd.c:1604:2: error: implicit declaration of function ‘ioremap’ [-Werror=implicit-function-declaration] addr_reg = ioremap(addr->start, resource_size(addr)); ^ drivers/usb/host/isp116x-hcd.c:1604:11: warning: assignment makes pointer from integer without a cast [enabled by default] addr_reg = ioremap(addr->start, resource_size(addr)); ^ drivers/usb/host/isp116x-hcd.c:1613:11: warning: assignment makes pointer from integer without a cast [enabled by default] data_reg = ioremap(data->start, resource_size(data)); ^ Signed-off-by: Vegard Nossum --- drivers/usb/host/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 1f117c3..64d78b1 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -340,6 +340,7 @@ config USB_OXU210HP_HCD config USB_ISP116X_HCD tristate "ISP116X HCD support" + depends on HAS_IOMEM ---help--- The ISP1160 and ISP1161 chips are USB host controllers. Enable this option if your board has this chip. If unsure, say N. -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 21/21] usb: remove HAS_IOMEM dependency from USB_SUPPORT
Am 10.02.2016 um 15:29 schrieb Vegard Nossum: > USB has not been usable on UML since this commit: > > commit e25df1205f37c7bff3ab14fdfc8a5249f3c69c82 > Author: Martin Schwidefsky > Date: Thu May 10 15:45:57 2007 +0200 > > [S390] Kconfig: menus with depends on HAS_IOMEM. > > Add "depends on HAS_IOMEM" to a number of menus to make them > disappear for s390 which does not have I/O memory. > > Signed-off-by: Martin Schwidefsky > > With hopefully all USB Host Controller Drivers that need it now > depending on HAS_IOMEM, we can remove the dependency from USB_SUPPORT > itself. This makes it possible to include USB support in UML builds > again. How do you use USB on uml? Or is it just for build coverage? Thanks, //richard -- 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 20/21] usb: support building without CONFIG_HAS_DMA
Some platforms don't have DMA, but we should still be able to build
USB drivers for these platforms. They could still be used through
vhci_hcd, usbip_host, or maybe something like USB passthrough in UML
from a capable host.
This is admittedly ugly with all the #ifdefs, but it is necessary to
get around linker errors like these:
drivers/built-in.o: In function `dma_unmap_sg_attrs':
include/linux/dma-mapping.h:183: undefined reference to `bad_dma_ops'
drivers/built-in.o: In function `dma_unmap_single_attrs':
include/linux/dma-mapping.h:148: undefined reference to `bad_dma_ops'
drivers/built-in.o: In function `dma_map_sg_attrs':
include/linux/dma-mapping.h:168: undefined reference to `bad_dma_ops'
drivers/built-in.o: In function `dma_map_page':
include/linux/dma-mapping.h:196: undefined reference to `bad_dma_ops'
drivers/built-in.o: In function `dma_mapping_error':
include/linux/dma-mapping.h:430: undefined reference to `bad_dma_ops'
drivers/built-in.o:include/linux/dma-mapping.h:131: more undefined references
to `bad_dma_ops' follow
If any of the new warnings trigger, the correct solution is almost
certainly to add a CONFIG_HAS_DMA dependency in the Kconfig menu for
the responsible driver.
Signed-off-by: Vegard Nossum
---
drivers/usb/core/buffer.c | 17 +
drivers/usb/core/hcd.c| 45 +++--
include/linux/usb/hcd.h | 8
3 files changed, 64 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index 89f2e77..427b131 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -59,13 +59,16 @@ void __init usb_init_pool_max(void)
*/
int hcd_buffer_create(struct usb_hcd *hcd)
{
+#ifdef CONFIG_HAS_DMA
charname[16];
int i, size;
+#endif
if (!hcd->self.controller->dma_mask &&
!(hcd->driver->flags & HCD_LOCAL_MEM))
return 0;
+#ifdef CONFIG_HAS_DMA
for (i = 0; i < HCD_BUFFER_POOLS; i++) {
size = pool_max[i];
if (!size)
@@ -78,6 +81,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
return -ENOMEM;
}
}
+#endif
return 0;
}
@@ -91,6 +95,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
*/
void hcd_buffer_destroy(struct usb_hcd *hcd)
{
+#ifdef CONFIG_HAS_DMA
int i;
for (i = 0; i < HCD_BUFFER_POOLS; i++) {
@@ -101,6 +106,7 @@ void hcd_buffer_destroy(struct usb_hcd *hcd)
hcd->pool[i] = NULL;
}
}
+#endif
}
@@ -116,7 +122,9 @@ void *hcd_buffer_alloc(
)
{
struct usb_hcd *hcd = bus_to_hcd(bus);
+#ifdef CONFIG_HAS_DMA
int i;
+#endif
/* some USB hosts just use PIO */
if (!bus->controller->dma_mask &&
@@ -125,11 +133,16 @@ void *hcd_buffer_alloc(
return kmalloc(size, mem_flags);
}
+#ifdef CONFIG_HAS_DMA
for (i = 0; i < HCD_BUFFER_POOLS; i++) {
if (size <= pool_max[i])
return dma_pool_alloc(hcd->pool[i], mem_flags, dma);
}
return dma_alloc_coherent(hcd->self.controller, size, dma, mem_flags);
+#else
+ WARN_ON_NO_DMA();
+ return NULL;
+#endif
}
void hcd_buffer_free(
@@ -140,7 +153,9 @@ void hcd_buffer_free(
)
{
struct usb_hcd *hcd = bus_to_hcd(bus);
+#ifdef CONFIG_HAS_DMA
int i;
+#endif
if (!addr)
return;
@@ -151,6 +166,7 @@ void hcd_buffer_free(
return;
}
+#ifdef CONFIG_HAS_DMA
for (i = 0; i < HCD_BUFFER_POOLS; i++) {
if (size <= pool_max[i]) {
dma_pool_free(hcd->pool[i], addr, dma);
@@ -158,4 +174,5 @@ void hcd_buffer_free(
}
}
dma_free_coherent(hcd->self.controller, size, addr, dma);
+#endif
}
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index df0e3b9..1eb214d 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1408,12 +1408,15 @@ static void hcd_free_coherent(struct usb_bus *bus,
dma_addr_t *dma_handle,
void usb_hcd_unmap_urb_setup_for_dma(struct usb_hcd *hcd, struct urb *urb)
{
- if (urb->transfer_flags & URB_SETUP_MAP_SINGLE)
+ if (urb->transfer_flags & URB_SETUP_MAP_SINGLE) {
+#ifdef CONFIG_HAS_DMA
dma_unmap_single(hcd->self.controller,
urb->setup_dma,
sizeof(struct usb_ctrlrequest),
DMA_TO_DEVICE);
- else if (urb->transfer_flags & URB_SETUP_MAP_LOCAL)
+#endif
+ WARN_ON_NO_DMA();
+ } else if (urb->transfer_flags & URB_SETUP_MAP_LOCAL)
hcd_free_coherent(urb->dev->bus,
&urb->setup_dma,
(void **) &urb->setup_packet,
@@ -1440,27 +1443,37 @@ vo
[PATCH 10/21] usb: add HAS_IOMEM dependency to USB_EHCI_HCD
drivers/built-in.o: In function `ehci_platform_probe': /home/vegard/linux/drivers/usb/host/ehci-platform.c:282: undefined reference to `devm_ioremap_resource' drivers/built-in.o: In function `oxu_drv_probe': /home/vegard/linux/drivers/usb/host/oxu210hp-hcd.c:3821: undefined reference to `devm_ioremap_resource' drivers/built-in.o: In function `isp1362_probe': /home/vegard/linux/drivers/usb/host/isp1362-hcd.c:2668: undefined reference to `devm_ioremap_resource' Signed-off-by: Vegard Nossum --- drivers/usb/host/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 96221c4..4c2e38a 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -72,6 +72,7 @@ endif # USB_XHCI_HCD config USB_EHCI_HCD tristate "EHCI HCD (USB 2.0) support" + depends on HAS_IOMEM ---help--- The Enhanced Host Controller Interface (EHCI) is standard for USB 2.0 "high speed" (480 Mbit/sec, 60 Mbyte/sec) host controller hardware. -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 15/21] usb: add HAS_IOMEM dependency to USB_APPLEDISPLAY
warning: (USB_APPLEDISPLAY) selects BACKLIGHT_LCD_SUPPORT which has unmet direct dependencies (HAS_IOMEM) Signed-off-by: Vegard Nossum --- drivers/usb/misc/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig index f7a7fc2..ea10059 100644 --- a/drivers/usb/misc/Kconfig +++ b/drivers/usb/misc/Kconfig @@ -150,6 +150,7 @@ config USB_FTDI_ELAN config USB_APPLEDISPLAY tristate "Apple Cinema Display support" + depends on HAS_IOMEM select BACKLIGHT_LCD_SUPPORT select BACKLIGHT_CLASS_DEVICE help -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/21] usb: add HAS_IOMEM dependency to USB_FOTG210_HCD
drivers/built-in.o: In function `fotg210_hcd_probe': /home/vegard/linux/drivers/usb/host/fotg210-hcd.c:5637: undefined reference to `devm_ioremap_resource' Signed-off-by: Vegard Nossum --- drivers/usb/host/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 90cb8d5..89f592d 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -367,6 +367,7 @@ config USB_ISP1362_HCD config USB_FOTG210_HCD tristate "FOTG210 HCD support" depends on USB + depends on HAS_IOMEM ---help--- Faraday FOTG210 is an OTG controller which can be configured as an USB2.0 host. It is designed to meet USB2.0 EHCI specification -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 18/21] usb: add HAS_IOMEM dependency to USB_OXU210HP_HCD
drivers/built-in.o: In function `oxu_drv_probe': /home/vegard/linux/drivers/usb/host/oxu210hp-hcd.c:3821: undefined reference to `devm_ioremap_resource' Signed-off-by: Vegard Nossum --- drivers/usb/host/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index af62016..85230d6 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -332,6 +332,7 @@ endif # USB_EHCI_HCD config USB_OXU210HP_HCD tristate "OXU210HP HCD support" + depends on HAS_IOMEM ---help--- The OXU210HP is an USB host/OTG/device controller. Enable this option if your board has this chip. If unsure, say N. -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 21/21] usb: remove HAS_IOMEM dependency from USB_SUPPORT
USB has not been usable on UML since this commit: commit e25df1205f37c7bff3ab14fdfc8a5249f3c69c82 Author: Martin Schwidefsky Date: Thu May 10 15:45:57 2007 +0200 [S390] Kconfig: menus with depends on HAS_IOMEM. Add "depends on HAS_IOMEM" to a number of menus to make them disappear for s390 which does not have I/O memory. Signed-off-by: Martin Schwidefsky With hopefully all USB Host Controller Drivers that need it now depending on HAS_IOMEM, we can remove the dependency from USB_SUPPORT itself. This makes it possible to include USB support in UML builds again. Signed-off-by: Vegard Nossum --- drivers/usb/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig index 8ed451d..93ba109 100644 --- a/drivers/usb/Kconfig +++ b/drivers/usb/Kconfig @@ -21,7 +21,6 @@ config USB_EHCI_BIG_ENDIAN_DESC menuconfig USB_SUPPORT bool "USB support" - depends on HAS_IOMEM default y ---help--- This option adds core support for Universal Serial Bus (USB). -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 17/21] usb: add HAS_IOMEM dependency to USB_PXA27X
drivers/built-in.o: In function `pxa_udc_probe': /home/vegard/linux/drivers/usb/gadget/udc/pxa27x_udc.c:2430: undefined reference to `devm_ioremap_resource' Signed-off-by: Vegard Nossum --- drivers/usb/gadget/udc/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig index 70feaf2..d6ad7e6 100644 --- a/drivers/usb/gadget/udc/Kconfig +++ b/drivers/usb/gadget/udc/Kconfig @@ -188,6 +188,7 @@ config USB_RENESAS_USB3 config USB_PXA27X tristate "PXA 27x" + depends on HAS_IOMEM help Intel's PXA 27x series XScale ARM v5TE processors include an integrated full speed USB 1.1 device controller. -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 14/21] usb: add HAS_IOMEM dependency to USB_PXA25X
drivers/built-in.o: In function `pxa_udc_probe': /home/vegard/linux/drivers/usb/gadget/udc/pxa27x_udc.c:2430: undefined reference to `devm_ioremap_resource' Signed-off-by: Vegard Nossum --- drivers/usb/gadget/udc/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig index e06efa3..70feaf2 100644 --- a/drivers/usb/gadget/udc/Kconfig +++ b/drivers/usb/gadget/udc/Kconfig @@ -128,6 +128,7 @@ config USB_OMAP config USB_PXA25X tristate "PXA 25x or IXP 4xx" depends on (ARCH_PXA && PXA25x) || ARCH_IXP4XX + depends on HAS_IOMEM help Intel's PXA 25x series XScale ARM-5TE processors include an integrated full speed USB 1.1 device controller. The -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/21] usb: add HAS_IOMEM dependency to USB_XHCI_MVEBU
drivers/usb/host/xhci-mvebu.c: In function ‘xhci_mvebu_mbus_init_quirk’: drivers/usb/host/xhci-mvebu.c:58:2: error: implicit declaration of function ‘ioremap’ [-Werror=implicit-function-declaration] base = ioremap(res->start, resource_size(res)); ^ drivers/usb/host/xhci-mvebu.c:58:7: warning: assignment makes pointer from integer without a cast [enabled by default] base = ioremap(res->start, resource_size(res)); ^ drivers/usb/host/xhci-mvebu.c:69:2: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration] iounmap(base); ^ Signed-off-by: Vegard Nossum --- drivers/usb/host/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 64d78b1..bf68bd8 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -53,6 +53,7 @@ config USB_XHCI_MTK config USB_XHCI_MVEBU tristate "xHCI support for Marvell Armada 375/38x" select USB_XHCI_PLATFORM + depends on HAS_IOMEM depends on ARCH_MVEBU || COMPILE_TEST ---help--- Say 'Y' to enable the support for the xHCI host controller -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/21] usb: add HAS_IOMEM dependency to USB_C67X00_HCD
CC drivers/usb/c67x00/c67x00-drv.o drivers/usb/c67x00/c67x00-drv.c: In function ‘c67x00_drv_probe’: drivers/usb/c67x00/c67x00-drv.c:148:2: error: implicit declaration of function ‘ioremap’ [-Werror=implicit-function-declaration] c67x00->hpi.base = ioremap(res->start, resource_size(res)); ^ drivers/usb/c67x00/c67x00-drv.c:148:19: warning: assignment makes pointer from integer without a cast [enabled by default] c67x00->hpi.base = ioremap(res->start, resource_size(res)); ^ drivers/usb/c67x00/c67x00-drv.c:185:2: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration] iounmap(c67x00->hpi.base); ^ Signed-off-by: Vegard Nossum --- drivers/usb/host/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index af20d93..e781fb1 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -5,6 +5,7 @@ comment "USB Host Controller Drivers" config USB_C67X00_HCD tristate "Cypress C67x00 HCD support" + depends on HAS_IOMEM help The Cypress C67x00 (EZ-Host/EZ-OTG) chips are dual-role host/peripheral/OTG USB controllers. -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/21] usb: add HAS_IOMEM dependency to USB_DWC2
drivers/built-in.o: In function `dwc2_driver_probe': /home/vegard/linux/drivers/usb/dwc2/platform.c:491: undefined reference to `devm_ioremap_resource' Signed-off-by: Vegard Nossum --- drivers/usb/dwc2/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig index fd95ba6..b56920a 100644 --- a/drivers/usb/dwc2/Kconfig +++ b/drivers/usb/dwc2/Kconfig @@ -1,6 +1,7 @@ config USB_DWC2 tristate "DesignWare USB2 DRD Core Support" depends on USB || USB_GADGET + depends on HAS_IOMEM help Say Y here if your system has a Dual Role Hi-Speed USB controller based on the DesignWare HSOTG IP Core. -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 16/21] usb: add HAS_IOMEM dependency to USB_OHCI_HCD
drivers/built-in.o: In function `ohci_platform_probe': /home/vegard/linux/drivers/usb/host/ohci-platform.c:246: undefined reference to `devm_ioremap_resource' Signed-off-by: Vegard Nossum --- drivers/usb/host/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 89f592d..af62016 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -389,6 +389,7 @@ config USB_MAX3421_HCD config USB_OHCI_HCD tristate "OHCI HCD (USB 1.1) support" + depends on HAS_IOMEM ---help--- The Open Host Controller Interface (OHCI) is a standard for accessing USB 1.1 host controller hardware. It does more in hardware than Intel's -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/21] usb: add HAS_IOMEM dependency to USB_XHCI_HCD
drivers/built-in.o: In function `xhci_plat_probe': /home/vegard/linux/drivers/usb/host/xhci-plat.c:160: undefined reference to `devm_ioremap_resource' Signed-off-by: Vegard Nossum --- drivers/usb/host/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 4c2e38a..90cb8d5 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -18,6 +18,7 @@ config USB_C67X00_HCD config USB_XHCI_HCD tristate "xHCI HCD (USB 3.0) support" + depends on HAS_IOMEM ---help--- The eXtensible Host Controller Interface (xHCI) is standard for USB 3.0 "SuperSpeed" host controller hardware. -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/21] usb: add HAS_IOMEM dependency to USB_MUSB_TUSB6010
CC drivers/usb/musb/tusb6010.o drivers/usb/musb/tusb6010.c: In function ‘tusb_musb_init’: drivers/usb/musb/tusb6010.c:1133:2: error: implicit declaration of function ‘ioremap’ [-Werror=implicit-function-declaration] sync = ioremap(mem->start, resource_size(mem)); ^ drivers/usb/musb/tusb6010.c:1133:7: warning: assignment makes pointer from integer without a cast [enabled by default] sync = ioremap(mem->start, resource_size(mem)); ^ drivers/usb/musb/tusb6010.c:1162:4: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration] iounmap(sync); ^ Signed-off-by: Vegard Nossum --- drivers/usb/musb/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig index 45c83ba..0401573 100644 --- a/drivers/usb/musb/Kconfig +++ b/drivers/usb/musb/Kconfig @@ -85,6 +85,7 @@ config USB_MUSB_DA8XX config USB_MUSB_TUSB6010 tristate "TUSB6010" + depends on HAS_IOMEM depends on ARCH_OMAP2PLUS || COMPILE_TEST depends on NOP_USB_XCEIV = USB_MUSB_HDRC # both built-in or both modules -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/21] usb: add HAS_IOMEM dependency to USB_SL811_HCD
CC drivers/usb/host/xhci-mtk.o drivers/usb/host/xhci-mtk.c:135:12: warning: ‘xhci_mtk_host_disable’ defined but not used [-Wunused-function] static int xhci_mtk_host_disable(struct xhci_hcd_mtk *mtk) ^ drivers/usb/host/xhci-mtk.c:313:13: warning: ‘usb_wakeup_enable’ defined but not used [-Wunused-function] static void usb_wakeup_enable(struct xhci_hcd_mtk *mtk) ^ drivers/usb/host/xhci-mtk.c:321:13: warning: ‘usb_wakeup_disable’ defined but not used [-Wunused-function] static void usb_wakeup_disable(struct xhci_hcd_mtk *mtk) ^ CC drivers/usb/host/sl811-hcd.o drivers/usb/host/sl811-hcd.c: In function ‘sl811h_remove’: drivers/usb/host/sl811-hcd.c:1607:3: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration] iounmap(sl811->data_reg); ^ drivers/usb/host/sl811-hcd.c: In function ‘sl811h_probe’: drivers/usb/host/sl811-hcd.c:1669:3: error: implicit declaration of function ‘ioremap’ [-Werror=implicit-function-declaration] addr_reg = ioremap(addr->start, 1); ^ drivers/usb/host/sl811-hcd.c:1669:12: warning: assignment makes pointer from integer without a cast [enabled by default] addr_reg = ioremap(addr->start, 1); ^ drivers/usb/host/sl811-hcd.c:1675:12: warning: assignment makes pointer from integer without a cast [enabled by default] data_reg = ioremap(data->start, 1); ^ Signed-off-by: Vegard Nossum --- drivers/usb/host/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index e781fb1..96221c4 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -671,6 +671,7 @@ config USB_U132_HCD config USB_SL811_HCD tristate "SL811HS HCD support" + depends on HAS_IOMEM help The SL811HS is a single-port USB controller that supports either host side or peripheral side roles. Enable this option if your -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/21] usb: add HAS_IOMEM dependency to USB_MUSB_HDRC
drivers/built-in.o: In function `musb_probe': /home/vegard/linux/drivers/usb/musb/musb_core.c:2304: undefined reference to `devm_ioremap_resource' Signed-off-by: Vegard Nossum --- drivers/usb/musb/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig index 0401573..886526b 100644 --- a/drivers/usb/musb/Kconfig +++ b/drivers/usb/musb/Kconfig @@ -7,6 +7,7 @@ config USB_MUSB_HDRC tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, AW, ...)' depends on (USB || USB_GADGET) + depends on HAS_IOMEM help Say Y here if your system has a dual role high speed USB controller based on the Mentor Graphics silicon IP. Then -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/21] usb: add HAS_IOMEM dependency to USB_R8A66597_HCD
CC drivers/usb/host/r8a66597-hcd.o drivers/usb/host/r8a66597-hcd.c: In function ‘r8a66597_remove’: drivers/usb/host/r8a66597-hcd.c:2401:2: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration] iounmap(r8a66597->reg); ^ drivers/usb/host/r8a66597-hcd.c: In function ‘r8a66597_probe’: drivers/usb/host/r8a66597-hcd.c:2447:2: error: implicit declaration of function ‘ioremap’ [-Werror=implicit-function-declaration] reg = ioremap(res->start, resource_size(res)); ^ drivers/usb/host/r8a66597-hcd.c:2447:6: warning: assignment makes pointer from integer without a cast [enabled by default] reg = ioremap(res->start, resource_size(res)); ^ Signed-off-by: Vegard Nossum --- drivers/usb/host/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index bf68bd8..af20d93 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -701,6 +701,7 @@ config USB_SL811_CS config USB_R8A66597_HCD tristate "R8A66597 HCD support" + depends on HAS_IOMEM help The R8A66597 is a USB 2.0 host and peripheral controller. -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 21/21] usb: remove HAS_IOMEM dependency from USB_SUPPORT
On 02/10/2016 03:35 PM, Richard Weinberger wrote: Am 10.02.2016 um 15:29 schrieb Vegard Nossum: USB has not been usable on UML since this commit: commit e25df1205f37c7bff3ab14fdfc8a5249f3c69c82 Author: Martin Schwidefsky Date: Thu May 10 15:45:57 2007 +0200 [S390] Kconfig: menus with depends on HAS_IOMEM. Add "depends on HAS_IOMEM" to a number of menus to make them disappear for s390 which does not have I/O memory. Signed-off-by: Martin Schwidefsky With hopefully all USB Host Controller Drivers that need it now depending on HAS_IOMEM, we can remove the dependency from USB_SUPPORT itself. This makes it possible to include USB support in UML builds again. How do you use USB on uml? Or is it just for build coverage? You can use usbip_host (USB over ip) to connect with real devices or gadgetfs (e.g. dummy_hcd) to emulate a device in the UML userspace which connects to the USB driver in the UML kernel. James McMechan at one time had some patches for passing through USB devices on the host to the UML kernel but I don't think it was ever merged. Anyway, that might be desirable to bring back at some point in the future. My specific use case is using gadgetfs inside UML for USB device driver fuzzing. Vegard -- 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/3] usb: type-c: USB Type-C Connector System Software Interface
On Wed, 2016-02-10 at 16:24 +0200, Andy Shevchenko wrote:
> On Wed, Feb 10, 2016 at 4:15 PM, Oliver Neukum wrote:
> > On Wed, 2016-02-10 at 13:56 +0200, Andy Shevchenko wrote:
> >> > +err:
> >> > + if (i > 0)
> >> > + for (; i >= 0; i--, con--)
> >> > + typec_unregister_port(con->port);
> >>
> >> Perhaps
> >>
> >> while (--i >= 0) {
> >> ...
> >> }
> >
> > While we are at it. No we should not change the semantics
> > of conditionals for the sake of appearance.
>
> I'm sorry I didn't get you.
> How this more or less standard pattern to clean up stuff on error path
> does with conditional semantics?
You change a postdecrement to a predecrement. The highest
number the loop is executed for is changed.
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
Andy Shevchenko writes: > On Wed, Feb 10, 2016 at 3:21 PM, Oliver Neukum wrote: >> On Wed, 2016-02-10 at 13:56 +0200, Andy Shevchenko wrote: >>> > +out: >>> >>> CodingStyle suggests to do a better label naming. >> >> Names coming from specs are what they are. There is >> no place for coding style here. > > Yes, and how is it related to C label names? It did appear as if you were commenting on the case labels since you quoted two full switch blocks. That's how I read your comment as well. It's now clear that you somehow mean than "out:" is in conflict with CodingStyle. It is still very unclear how, and it does not seem like you intend to make it any clearer since you did not take the opportunity to explain yourself. FWIW, I read the CodingStyle recommendation as: use descriptive labels instead of "foo1", "foo2" etc, where "foo" is typically "err". I do not see this as conflicting with the use of "err" or "out" when there is a single such label in a function. The meaning of those labels are very clear IMHO. Exactly what is it about "out" that is unclear to you here? Could you propose a better alternative if you seriously mean that this needs to be changed? Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] USB: musb: DA8xx: Add DT support for the DA8xx driver
Hello Sergei,
On 10.02.2016 15:05, Sergei Shtylyov wrote:
@@ -1,6 +1,9 @@
/*
* Texas Instruments DA8xx/OMAP-L1x "glue layer"
*
+ * DT support
+ * Copyright (c) 2016 Petr Kulhavy, Barix AG
+ *
Could you place this after MV's copyright?
I was trying to preserve the descending date order in the file.
Do you think 2008, 2016, 2005 makes more sense?
+static inline int phy_refclk_cfg(int frequency)
+{
+switch (frequency) {
+case 1200:
+return 0x01;
There's a macro for this.
[...]
And for this.
[...]
And for this.
Would you mind pointing me to those macros?
Apart of arch specific macros like e.g. CLOCK_12M in arch/mips/lantiq/clk.h
I can't find any such macros in the include/ directory.
+if (!data) {
+ret = -ENOMEM;
+goto err5;
+}
+
+config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
Why not just use the static config?!
I would say bad example, again :-)
BTW the config pointer in struct musb_hdrc_platform_data should be const
to make sure the musb core doesn't alter it.
I'll clean up that section.
+if (ret)
+power_ma = 0;
+
+/* the "mentor,power" value is in milli-amps, whereas
milli-Ampers, no?
Wrong. Correct is "milliamperes" - if the comment is worth the time
spent arguing about it.
However "milliamps" is also commonly used :-)
+/* optional parameter reference clock frequency */
+if (!of_property_read_u32(np, "ti,phy20-refclock-frequency",
Actually, this one smells like mandatory prop... U-Boot doesn't
program CFGCHIP2, so REFFREQ is left 0.
Yes, you might be right. I thought this parameter is valid only if the
external clock is selected.
But reading the clock section in the manual again, it seems the value
must be set in both cases.
I'll make it a mandatory parameter.
+static const struct musb_hdrc_config da830_config = {
+.ram_bits = 10,
+.num_eps = 5,
+.multipoint = 1,
+
+static const struct of_device_id da8xx_id_table[] = {
+{
+.compatible = "ti,da830-musb",
+.data = &da830_config,
I don't think you need to init. 'data' at this stage, keep it simple.
OK.
+enum musb_mode musb_get_dr_mode(struct device *dev)
I'd call it just musb_get_mode()...
If you like :-)
And please add this function in a separate (preceding) patch.
OK.
Petr
--
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 02/21] usb: add HAS_IOMEM dependency to USB_NET2272
On Wed, Feb 10, 2016 at 03:29:37PM +0100, Vegard Nossum wrote: > drivers/usb/gadget/udc/net2272.c: In function ‘net2272_remove’: > drivers/usb/gadget/udc/net2272.c:2232:2: error: implicit declaration of > function ‘iounmap’ [-Werror=implicit-function-declaration] > iounmap(dev->base_addr); > ^ > drivers/usb/gadget/udc/net2272.c: In function ‘net2272_plat_probe’: > drivers/usb/gadget/udc/net2272.c:2650:2: error: implicit declaration of > function ‘ioremap_nocache’ [-Werror=implicit-function-declaration] > dev->base_addr = ioremap_nocache(base, len); > ^ > drivers/usb/gadget/udc/net2272.c:2650:17: warning: assignment makes pointer > from integer without a cast [enabled by default] > dev->base_addr = ioremap_nocache(base, len); > ^ > > Signed-off-by: Vegard Nossum > --- > drivers/usb/gadget/udc/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig > index 753c29b..ca19f6f 100644 > --- a/drivers/usb/gadget/udc/Kconfig > +++ b/drivers/usb/gadget/udc/Kconfig > @@ -287,6 +287,7 @@ config USB_FSL_QE > dynamically linked module called "fsl_qe_udc". > > config USB_NET2272 > + depends on HAS_IOMEM Why not fix the root of the problem and provide the correct functions for this when HAS_IOMEM is not enabled? thanks, greg k-h -- 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 02/21] usb: add HAS_IOMEM dependency to USB_NET2272
Am 10.02.2016 um 17:28 schrieb Vegard Nossum:
> I don't think there is a "correct function" for when HAS_IOMEM is not
> enabled. There is no IO address space on UML, so it doesn't make sense
> to compile these drivers in the first place.
>
> Or do you mean to use the dummy implementation from asm-generic/io.h? (I
> have to admit I don't know how that would work.)
We already had an discussion of having an ioremap() like this for UML and S390:
static inline void __iomem *ioremap(phys_addr_t offset, size_t size)
{
BUG();
return NULL;
}
But IMHO we should just fix the driver dependencies instead of providing dummy
interfaces just for the sake of making things somehow build...
Thanks,
//richard
--
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 02/21] usb: add HAS_IOMEM dependency to USB_NET2272
On 02/10/2016 05:15 PM, Greg Kroah-Hartman wrote: On Wed, Feb 10, 2016 at 03:29:37PM +0100, Vegard Nossum wrote: drivers/usb/gadget/udc/net2272.c: In function ‘net2272_remove’: drivers/usb/gadget/udc/net2272.c:2232:2: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration] iounmap(dev->base_addr); ^ drivers/usb/gadget/udc/net2272.c: In function ‘net2272_plat_probe’: drivers/usb/gadget/udc/net2272.c:2650:2: error: implicit declaration of function ‘ioremap_nocache’ [-Werror=implicit-function-declaration] dev->base_addr = ioremap_nocache(base, len); ^ drivers/usb/gadget/udc/net2272.c:2650:17: warning: assignment makes pointer from integer without a cast [enabled by default] dev->base_addr = ioremap_nocache(base, len); ^ Signed-off-by: Vegard Nossum --- drivers/usb/gadget/udc/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig index 753c29b..ca19f6f 100644 --- a/drivers/usb/gadget/udc/Kconfig +++ b/drivers/usb/gadget/udc/Kconfig @@ -287,6 +287,7 @@ config USB_FSL_QE dynamically linked module called "fsl_qe_udc". config USB_NET2272 + depends on HAS_IOMEM Why not fix the root of the problem and provide the correct functions for this when HAS_IOMEM is not enabled? I don't think there is a "correct function" for when HAS_IOMEM is not enabled. There is no IO address space on UML, so it doesn't make sense to compile these drivers in the first place. Or do you mean to use the dummy implementation from asm-generic/io.h? (I have to admit I don't know how that would work.) Vegard -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: host: xhci: Replace bus lock with host controller lock
On 05.02.2016 17:14, Chris Bainbridge wrote: Running task list at fail point: ... Some of the functions appear to be inlined, the exact call chain is: hub_port_init usb_get_device_descriptor usb_get_descriptor usb_control_msg usb_internal_control_msg usb_start_wait_urb usb_submit_urb / wait_for_completion_timeout / usb_kill_urb hub_port_init hub_set_address xhci_address_device xhci_setup_device hub_port_reset() will end up moving the corresponding xhci device slot to default state. As hub_port_reset() is called several times in hub_port_init() It sounds reasonable that we could end up with two threads having their xhci device slots in default state at the same time, which according to xhci 4.5.3 specs still is a big no no. So both threads fail at their next task after this. One fails to read the descriptor, and the other fails addressing the device. Nice catch btw. So xhci_setup_device is entered while there is an outstanding URB on the other bus. Unless anyone can think of a better way to fix this I'll make the requested changes and resend my patch. For what it's wort I think that this suggested controller mutex sounds like a good idea. Should work for xhci at least. -Mathias -- 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: core: hub: hub_port_init lock controller instead of bus
On 08.02.2016 15:49, Chris Bainbridge wrote: The XHCI controller presents two USB buses to the system - one for USB 2 and one for USB 3. When only one bus is locked there is a race condition with two threads in hub_port_init: [8.984500] Call Trace: [8.985698] [] schedule+0x37/0x90 [8.986918] [] usb_kill_urb+0x8d/0xd0 [8.988130] [] ? wake_up_atomic_t+0x30/0x30 [8.989343] [] usb_start_wait_urb+0xbe/0x150 [8.990561] [] usb_control_msg+0xbc/0xf0 [8.991766] [] hub_port_init+0x51e/0xb70 [8.992964] [] hub_event+0x817/0x1570 [8.994156] [] process_one_work+0x1ff/0x620 [8.995342] [] ? process_one_work+0x15f/0x620 [8.996528] [] worker_thread+0x64/0x4b0 [8.997707] [] ? rescuer_thread+0x390/0x390 [8.998883] [] kthread+0x105/0x120 [9.56] [] ? kthread_create_on_node+0x200/0x200 [9.001241] [] ret_from_fork+0x3f/0x70 [9.002420] [] ? kthread_create_on_node+0x200/0x200 [9.870837] Call Trace: [9.875664] [] xhci_setup_device+0x53d/0xa40 [9.876871] [] xhci_address_device+0xe/0x10 [9.878068] [] hub_port_init+0x1bf/0xb70 [9.879262] [] ? trace_hardirqs_on+0xd/0x10 [9.880465] [] hub_event+0x817/0x1570 [9.881668] [] process_one_work+0x1ff/0x620 [9.882869] [] ? process_one_work+0x15f/0x620 [9.884074] [] worker_thread+0x64/0x4b0 [9.885268] [] ? rescuer_thread+0x390/0x390 [9.886457] [] kthread+0x105/0x120 [9.887634] [] ? kthread_create_on_node+0x200/0x200 [9.17] [] ret_from_fork+0x3f/0x70 [9.889995] [] ? kthread_create_on_node+0x200/0x200 Which results from the two call chains: hub_port_init usb_get_device_descriptor usb_get_descriptor usb_control_msg usb_internal_control_msg usb_start_wait_urb usb_submit_urb / wait_for_completion_timeout / usb_kill_urb hub_port_init hub_set_address xhci_address_device xhci_setup_device The logged kernel errors are: [8.034843] xhci_hcd :00:14.0: Timeout while waiting for setup device command [ 13.183701] usb 3-3: device descriptor read/all, error -110 On a test system this failure occurred on 6% of all boots. Hypothetically, it should perhaps be possible to set the address of the hub on one bus while the hub on the other bus already has an outstanding descriptor read request. But as this is not working reliably, fix it by locking the controller in hub_port_init to prevent parallel initialisation of both buses. Most likely xhci is messed up after two device slots are in default state at the same time. This happens when both threads are in hub_port_init() have called hub_port_reset() The issue becomes visible when the the descriptor read and set address both fail after the port resets. xhci specs 4.5.3 has one tiny note about this: "Note: Software shall not transition more than one Device Slot to the Default State at a time" So to me, and from xhci pov this patch looks like the correct solution, but I might be missing some usb core side details. -Mathias -- 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/3] usb: type-c: USB Type-C Connector System Software Interface
On Wed, Feb 10, 2016 at 12:30:42PM +0200, Heikki Krogerus wrote: > On Tue, Feb 09, 2016 at 10:21:55AM -0800, Greg KH wrote: > > On Tue, Feb 09, 2016 at 07:01:22PM +0200, Heikki Krogerus wrote: > > > USB Type-C Connector System Software Interface (UCSI) is a > > > specification that defines registers and data structures > > > used to interface with the USB Type-C connectors on a system. > > > > > > The specification is public and available at: > > > http://www.intel.com/content/www/us/en/io/universal-serial-bus/usb-type-c-ucsi-spec.html > > > > > > > What does this driver / code actually do? Why is it needed? What > > interface to the rest of the kernel / userspace does it provide? > > I will fix this describe these things in the commit message. I'll just > but some UCSI background in case somebody is interested. So UCSI is in > practice a standard for USB Type-C controllers.. > > UCSI is the control interface for USB Type-C connectors (regardless > was USB PD supported or not) in MS Windows, so most likely all new HW > platforms designed to work also with Windows that are equipped with > USB Type-C will have UCSI device for controlling the USB Type-C ports. There's many millions of devices with type-C without Windows on them, so don't count on this being everywhere :) > In some cases the hardware for Type-C will be just a PHY like fusb30x > on these platforms (it's cheaper then USB PD or complete USB Type-C > controller), but in those cases the PHY is probable attached to an EC > or is completely controlled by system FW like BIOS together with any > USB PD communication in cases where USB PD is supported, and is in any > case not visible to the OS. Instead UCSI device is exposed to the OS > to give it means to apply its policies to the USB Type-C port. > > > Why would we care about this? > > I'll try to explain why it's important to export the control of USB > Type-C ports to the user space in my answer to your comments to the > first patch of this series, the one introducing the class. > > But surely everybody agrees that decision about the policies regarding > USB Type-C ports, like which data role to use, do we charge or are we > letting the other end charge, etc., belongs to the user? No, I don't agree. It's still unknown if userspace can react fast though to these types of "policy" changes. I've heard from some manufacturers that the response time needed is something that we can't leave to userspace. And along those lines, do you have a working userspace user of this interface? We don't create interfaces without a user, especially given that it takes a long time to ensure that a user/kernel api actually is correct. We would need to see that to ensure that this kernel implementation is "correct" and working properly. > If you plug > your phone to your desktop, I would imagine that you want to see the > phone primarily as the USB device and the desktop as host, and to > achieve the device role, you don't want to be forced to unplug/replug > your phone from the desktop until you achieve device role, right? Why is unplugging somehow required? thanks, greg k-h -- 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/3] usb: USB Type-C Connector Class
On Wed, Feb 10, 2016 at 12:38:40PM +0200, Heikki Krogerus wrote: > > And what is userspace going to do with these files? Why does it care? > > The OS policy regarding USB Type-C ports needs to come from user > space. What drives this "need"? > The user must be allowed to select the USB data role, and when > USB PD is supported, also the power role, and at the same time we need > to export all the relevant information about the USB Type-C ports to > the user space, like connection status, the type of partner etc. And > all that from a single interface. Again, what drives this "need" to be in a "single interface"? > I'm pretty sure that this is exactly what distributions like Ubuntu, > RedHat etc. want, an I know for fact that Chrome OS and Android will > expect the user to be in control over the roles and get that > information about the ports one way or the other. Given that ChromeOS and Android already do this type of thing today, why not work with those developers to ensure that this really is the interface they want / expect? > > > The data_role, power_role and alternate_mode are also > > > writable and can be used for executing role swapping and > > > entering modes. When USB PD is not supported by the > > > connector or partner, power_role will reflect the value of > > > the data_role, and is not swappable independently. > > > > How does this implementation differ from those in other drivers that we > > have seen, but not submitted for merging? I'm referring to the code > > from Fairchild for their USB Type C driver: > > https://github.com/gregkh/fusb30x > > and the driver that is in the latest Nexus 6 Android release (don't have > > the link to the android kernel tree at the moment sorry, but it's public > > and I think Linaro is working on cleaning it up...) > > That would be USB PD stack and driver for fusb30x USB Type-C/PD PHYs. > It's the second USB PD stack I've seen (and sadly also second driver > for fusb30x), but I just know there are more. Oh, there's more than just 2 drivers for that fusb30x chip floating around. My repo is not the latest version and it's a truly horrid piece of code, never to be run on any hardware you actually care about power as it doesn't care. > My class is just about exporting control of USB Type-C ports to the > user space, and note, USB Type-C *not* USB PD. I don't thing that my > little class and the USB PD stack, where ever it ends up coming from, > conflict with each other. But we need to ensure that it doesn't conflict, and given that you are already using the same directory those stacks are using, perhaps you can look at them to see that you aren't duplicating any work? > The only difference is that I'm clearly separating USB Type-C from USB > PD (and actually everything else), which is the correct thing to do. > USB Type-C is not the same thing as USB PD. USB Type-C does not always > come with USB PD. I agree, keeping them separate seems good, but I worry when you have to do both how that is going to look. > I did not go through that code, but I'm guessing the guys have for > example exported similar role swapping controls to user space from > some part of their stack. So those guys would just need to register > their fusb30x with this class, let the class take care of exporting > those controls and probable continue using their USB PD stack exactly > like they have done before. the fusb30x code does it all within kernel space, no userspace interactions needed due to timing requirements (so they say). I'm not saying that this is a good idea / design, just that I'm getting conflicting requirements from different camps at the moment and it's really hard to sort it all out :( thanks, greg k-h -- 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: usb: musb: slower system resume
Hi,
On Tue, Feb 9, 2016 at 3:51 PM, Vishal Thanki wrote:
> On Mon, Feb 08, 2016 at 08:44:19PM +0200, Felipe Balbi wrote:
>>
>> Hi,
>>
>> Vishal Thanki writes:
>> > It is vanilla kernel v4.0, except for some ASoC patches and v5 of Dave's PM
>> > series from kernel v3.19-rc5 rebased to it.
>>
>> Care to try *real* vanilla v4.4 instead ? v4.5-rc3 would be even
>> better. Take *no* extra patches.
>>
>
> I tried on v4.4. I have to take the patches from Dave so that PM can work
> on am33xx platform. The patches are taken from here, and rebased to v4.4
> vanilla kernel.
>
> https://github.com/dgerlach/linux-pm/commits/pm-v4.3-rc1-amx3-suspend
>
>> > If a USB storage device is plugged in before suspend and keep is plugged in
>> > during resume, the resume is taking ~15+ seconds. I noticed that it fails
>> > while
>> > sending USB control messages in hub_port_init():
>> >
>> > http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L4390
>> >
>> > After the failure, the USB device is logically disconnected and
>> > rediscovered
>> > again. So I can see the device mounted once the system is resumed, but it
>> > takes more time during resume.
>> >
>> > I observed that during system resume, there is a CONNECT interrupt received
>> > by MUSB controller:
>> >
>> > http://lxr.free-electrons.com/source/drivers/usb/musb/musb_core.c#L772
>> >
>> > If the hub_port_init() is started before the CONNECT interrupt is
>> > served, I am hitting the issue. Almost every time the CONNECT
>> > interrupt is occurring ~150ms after musb_start() is invoked from
>> > musb_resume(). If I add a wait of ~200ms in musb_resume() just to make
>> > sure that CONNECT interrupt is received, I never hit the issue.
>>
>> interesting, sounds like a bug in the ordering of calls in
>> musb_resume(). Can you see if you're falling in either of these branches
>> on the failing case ?
>>
>> mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV;
>> if ((devctl & mask) != (musb->context.devctl & mask))
>> musb->port1_status = 0;
>> if (musb->need_finish_resume) {
>> musb->need_finish_resume = 0;
>> schedule_delayed_work(&musb->finish_resume_work,
>> msecs_to_jiffies(USB_RESUME_TIMEOUT));
>> }
>>
>> Any differences in this regard on the working case?
>
> In both, working and non-working case, the execution is not entering into any
> of the "if" blocks. I have made sure that by putting prints as can be
> seen in the attached patch.
>
>>
>> > I see that hub_port_init() is calling hub_port_reset before actually
>> > sending the USB
>> > control messages. The hub_port_reset() internally sets the RESET bit
>> > in MUSB POWER
>> > register, but I am not sure if that is a valid operation before
>> > getting the CONNECT interrupt.
>>
>> hub_port_reset(), IMO, shouldn't run before it knows there are devices
>> connected to the bus...
>>
>
> Hmm, I have attached the logs for kernel v4.4 for working and
> non-working case. I noticed that the CONNECT interrupt now comes a
> little late (not within ~200 ms). However in working case, it always
> occurs before the hub_port_reset().
>
Just to add information, the MUSB controller is acting as host and
is using the TI DSPS (musb_dsps.c) glue layer driver.
With that said, I noticed that commit "869c59782981" added a flag to
deassert the RESET on resume. But actually the MUSB_POWER_RESET
is not set while calling musb_bus_resume(). Does a de-assert of
RESET really required even if MUSB_POWER_RESET is not set.
If I add a condition for MUSB_POWER_RESET to be to call the deassert reset,
I do not hit the issue, because in that case the musb->port1_status does not
contain valid flags and hub_port_reset() fails, hence the device is logically
disconnected and rediscovered again.
Thanks,
Vishal
>> --
>> 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 2/2] USB: musb: DA8xx: Add DT support for the DA8xx driver
Hello.
On 02/10/2016 07:01 PM, Petr Kulhavy wrote:
@@ -1,6 +1,9 @@
/*
* Texas Instruments DA8xx/OMAP-L1x "glue layer"
*
+ * DT support
+ * Copyright (c) 2016 Petr Kulhavy, Barix AG
+ *
Could you place this after MV's copyright?
I was trying to preserve the descending date order in the file.
The TI's copyright was copied from the davinci.c.
Do you think 2008, 2016, 2005 makes more sense?
Yes.
+static inline int phy_refclk_cfg(int frequency)
+{
+switch (frequency) {
+case 1200:
+return 0x01;
There's a macro for this.
[...]
And for this.
[...]
And for this.
Would you mind pointing me to those macros?
See CFGCHIP2_REFFREQ_*MHZ in include/linux/platfrom_data/usb-davinci.h.
+if (!data) {
+ret = -ENOMEM;
+goto err5;
+}
+
+config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
Why not just use the static config?!
I would say bad example, again :-)
You're supposed to think a bit, not just copy&paste. ;-)
BTW the config pointer in struct musb_hdrc_platform_data should be const to
make sure the musb core doesn't alter it.
I'll clean up that section.
+/* optional parameter reference clock frequency */
+if (!of_property_read_u32(np, "ti,phy20-refclock-frequency",
Actually, this one smells like mandatory prop... U-Boot doesn't program
CFGCHIP2, so REFFREQ is left 0.
And 0 is reserved.
Yes, you might be right. I thought this parameter is valid only if the
external clock is selected.
But reading the clock section in the manual again, it seems the value must be
set in both cases.
I'll make it a mandatory parameter.
Yes, please.
+enum musb_mode musb_get_dr_mode(struct device *dev)
I'd call it just musb_get_mode()...
If you like :-)
Note that there's MUSB_PORT_MODE_* in musb_core.h and musb_dsps.c uses
those in such function (incorrectly). I'll look into this once the generic
function is there.
And please add this function in a separate (preceding) patch.
OK.
Unless Felipe objects, that is.
Petr
MBR, Sergei
--
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: dwc3: Fix assignment of EP transfer resources
On 2/10/2016 2:11 AM, Felipe Balbi wrote:
>
> Hi,
>
> John Youn writes:
>
>
>>> If it wasn't for that flag, we would always allocate transfer resource 3
>>> for every newly enabled endpoint. Can you check with your RTL engineers
>>> if it's valid to *always* issue DEPSTARTCFG with a proper parameter
>>> depending on endpoint number ? Basically, something like below:
>>>
>>> @@ -306,20 +306,14 @@ static int dwc3_gadget_start_config(struct dwc3 *dwc,
>>> struct dwc3_ep *dep)
>>>
>>> memset(¶ms, 0x00, sizeof(params));
>>>
>>> - if (dep->number != 1) {
>>> - cmd = DWC3_DEPCMD_DEPSTARTCFG;
>>> - /* XferRscIdx == 0 for ep0 and 2 for the remaining */
>>> - if (dep->number > 1) {
>>> - if (dwc->start_config_issued)
>>> - return 0;
>>> - dwc->start_config_issued = true;
>>> - cmd |= DWC3_DEPCMD_PARAM(2);
>>> - }
>>> -
>>> - return dwc3_send_gadget_ep_cmd(dwc, 0, cmd, ¶ms);
>>> + cmd = DWC3_DEPCMD_DEPSTARTCFG;
>>> + /* XferRscIdx == 0 for ep0 and 2 for the remaining */
>>> + if (dep->number > 1) {
>>> + dwc->start_config_issued = true;
>>> + cmd |= DWC3_DEPCMD_PARAM(dwc->current_resource);
>>> }
>>>
>>> - return 0;
>>> + return dwc3_send_gadget_ep_cmd(dwc, 0, cmd, ¶ms);
>>> }
>>>
>>> static int dwc3_gadget_set_ep_config(struct dwc3 *dwc, struct dwc3_ep *dep,
>>> @@ -388,13 +382,20 @@ static int dwc3_gadget_set_ep_config(struct dwc3
>>> *dwc, struct dwc3_ep *dep,
>>> static int dwc3_gadget_set_xfer_resource(struct dwc3 *dwc, struct dwc3_ep
>>> *dep)
>>> {
>>> struct dwc3_gadget_ep_cmd_params params;
>>> + int ret;
>>>
>>> memset(¶ms, 0x00, sizeof(params));
>>>
>>> params.param0 = DWC3_DEPXFERCFG_NUM_XFER_RES(1);
>>>
>>> - return dwc3_send_gadget_ep_cmd(dwc, dep->number,
>>> + ret = dwc3_send_gadget_ep_cmd(dwc, dep->number,
>>> DWC3_DEPCMD_SETTRANSFRESOURCE, ¶ms);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + dwc->current_resource++;
>>> +
>>> + return 0;
>>> }
>>>
>>> /**
>>>
>>> With this we will *always* use DEPSTARTCFG any time we're enabling an
>>> endpoint which hasn't been enabled, but we will always keep transfer
>>> resources around. (Note that this won't really work as is, I haven't
>>> defined current_resource nor have I made sure to decrement
>>> current_resource whenever we disable an endpoint. Also, it might be that
>>> using a 32-bit flag instead of a counter might be better, dunno)
>>>
>>
>> Something like this might work too.
>>
>> I actually have a patch now which *greatly* simplifies all of this
>> code and eliminates the start_config_issued flag. But I need the RTL
>> engineers to confirm. It should be ok as it relies on the same
>> behavior that this current patch does.
>>
>> Basically assign all the resources in advance.
>
> I thought about that, but wouldn't this, essentially, enable all
> endpoints unconditionally ? This could, potentially, increase power
> consumption on some systems, right ? This could also cause "spurious"
> interrupts if a bogus host tries to move data on an endpoint which
> hasn't been enabled yet.
No, I mean to just assign resources withouth configuring or enabling
the endpoint. I have tested this approach and it works. But I still
need to verify that it won't conflict with anything, such as streams.
>
> Not sure this is a good approach here.
>
In any case, I will also resend the cleaned up version of this patch
as well.
Regards,
John
--
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: dwc3: Fix assignment of EP transfer resources
John Youn writes: >>> Basically assign all the resources in advance. >> >> I thought about that, but wouldn't this, essentially, enable all >> endpoints unconditionally ? This could, potentially, increase power >> consumption on some systems, right ? This could also cause "spurious" >> interrupts if a bogus host tries to move data on an endpoint which >> hasn't been enabled yet. > > No, I mean to just assign resources withouth configuring or enabling > the endpoint. I have tested this approach and it works. But I still oh ok. > need to verify that it won't conflict with anything, such as streams. yeah, we would probably have an issue with streams. IIRC, we allocate one transfer resource per stream, right ? >> Not sure this is a good approach here. > > In any case, I will also resend the cleaned up version of this patch > as well. cool -- balbi signature.asc Description: PGP signature
Re: [PATCH 21/21] usb: remove HAS_IOMEM dependency from USB_SUPPORT
Hi Vegard,
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v4.5-rc3 next-20160210]
[if your patch is applied to the wrong git tree, please drop us a note to help
improving the system]
url:
https://github.com/0day-ci/linux/commits/Vegard-Nossum/usb-add-HAS_IOMEM-dependency-to-USB_ISP116X_HCD/20160210-223436
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
usb-testing
config: um-allyesconfig (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=um
All warnings (new ones prefixed by >>):
In file included from include/linux/linkage.h:4:0,
from include/linux/preempt.h:9,
from include/linux/spinlock.h:50,
from drivers/staging/rtl8723au/include/osdep_service.h:22,
from drivers/staging/rtl8723au/core/rtw_cmd.c:17:
include/asm-generic/fixmap.h: In function 'fix_to_virt':
>> include/asm-generic/fixmap.h:31:19: warning: comparison of unsigned
>> expression >= 0 is always true [-Wtype-limits]
BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
^
include/linux/compiler.h:481:19: note: in definition of macro
'__compiletime_assert'
bool __cond = !(condition);\
^
include/linux/compiler.h:501:2: note: in expansion of macro
'_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^
include/linux/bug.h:50:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^
include/linux/bug.h:74:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^
>> include/asm-generic/fixmap.h:31:2: note: in expansion of macro 'BUILD_BUG_ON'
BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
^
In file included from include/net/checksum.h:25:0,
from include/linux/skbuff.h:31,
from include/linux/if_ether.h:23,
from include/uapi/linux/ethtool.h:17,
from include/linux/ethtool.h:16,
from include/linux/netdevice.h:42,
from drivers/staging/rtl8723au/include/osdep_service.h:30,
from drivers/staging/rtl8723au/core/rtw_cmd.c:17:
arch/um/include/asm/uaccess.h: In function '__access_ok':
>> arch/um/include/asm/uaccess.h:18:29: warning: comparison of unsigned
>> expression >= 0 is always true [-Wtype-limits]
(((unsigned long) (addr) >= FIXADDR_USER_START) && \
^
>> arch/um/include/asm/uaccess.h:48:3: note: in expansion of macro
>> '__access_ok_vsyscall'
__access_ok_vsyscall(addr, size) ||
^
vim +31 include/asm-generic/fixmap.h
d57c33c5 Mark Salter 2014-01-23 15 #ifndef __ASM_GENERIC_FIXMAP_H
d57c33c5 Mark Salter 2014-01-23 16 #define __ASM_GENERIC_FIXMAP_H
d57c33c5 Mark Salter 2014-01-23 17
d57c33c5 Mark Salter 2014-01-23 18 #include
d57c33c5 Mark Salter 2014-01-23 19
d57c33c5 Mark Salter 2014-01-23 20 #define __fix_to_virt(x) (FIXADDR_TOP -
((x) << PAGE_SHIFT))
d57c33c5 Mark Salter 2014-01-23 21 #define __virt_to_fix(x) ((FIXADDR_TOP -
((x)&PAGE_MASK)) >> PAGE_SHIFT)
d57c33c5 Mark Salter 2014-01-23 22
d57c33c5 Mark Salter 2014-01-23 23 #ifndef __ASSEMBLY__
d57c33c5 Mark Salter 2014-01-23 24 /*
d57c33c5 Mark Salter 2014-01-23 25 * 'index to address' translation. If
anyone tries to use the idx
d57c33c5 Mark Salter 2014-01-23 26 * directly without translation, we catch
the bug with a NULL-deference
d57c33c5 Mark Salter 2014-01-23 27 * kernel oops. Illegal ranges of incoming
indices are caught too.
d57c33c5 Mark Salter 2014-01-23 28 */
d57c33c5 Mark Salter 2014-01-23 29 static __always_inline unsigned long
fix_to_virt(const unsigned int idx)
d57c33c5 Mark Salter 2014-01-23 30 {
d57c33c5 Mark Salter 2014-01-23 @31 BUILD_BUG_ON(idx >=
__end_of_fixed_addresses);
d57c33c5 Mark Salter 2014-01-23 32 return __fix_to_virt(idx);
d57c33c5 Mark Salter 2014-01-23 33 }
d57c33c5 Mark Salter 2014-01-23 34
d57c33c5 Mark Salter 2014-01-23 35 static inline unsigned long
virt_to_fix(const unsigned long vaddr)
d57c33c5 Mark Salter 2014-01-23 36 {
d57c33c5 Mark Salter 2014-01-23 37 BUG_ON(vaddr >= FIXADDR_TOP || vaddr <
FIXADDR_START);
d57c33c5 Mark Salter 2014-01-23 38 return __virt_to_fix(vaddr);
d57c33c5 Mark Salter 2014-01-23 39 }
:: The code at line 31 was first introduced by commit
:: d57c33c5daa4efa9e4d303bd0faf868080b532be add generic fixmap.h
:: TO: Mark Salter
:: CC: Linus Torvalds
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: Binary data
[PATCH] usb: storage: make US_DEBUGPX print with LOGLEVEL_DEBUG
The US_DEBUGPX macro uses printk without specifying a kernel log level, so
the default kernel log level is used, which may not match LOGLEVEL_DEBUG
used in usb_stor_dbg. Use printk_emit with LOGLEVEL_DEBUG instead.
Signed-off-by: Victor Dodon
---
drivers/usb/storage/debug.h | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/storage/debug.h b/drivers/usb/storage/debug.h
index f525203..fc1c02f 100644
--- a/drivers/usb/storage/debug.h
+++ b/drivers/usb/storage/debug.h
@@ -43,6 +43,7 @@
#define _DEBUG_H_
#include
+#include
#define USB_STORAGE "usb-storage: "
@@ -53,7 +54,8 @@ void usb_stor_show_sense(const struct us_data *us, unsigned
char key,
__printf(2, 3) void usb_stor_dbg(const struct us_data *us,
const char *fmt, ...);
-#define US_DEBUGPX(fmt, ...) printk(fmt, ##__VA_ARGS__)
+#define US_DEBUGPX(fmt, ...) \
+ printk_emit(0, LOGLEVEL_DEBUG, NULL, 0, fmt, ##__VA_ARGS__)
#define US_DEBUG(x)x
#else
__printf(2, 3)
@@ -64,7 +66,11 @@ static inline void _usb_stor_dbg(const struct us_data *us,
#define usb_stor_dbg(us, fmt, ...) \
do { if (0) _usb_stor_dbg(us, fmt, ##__VA_ARGS__); } while (0)
#define US_DEBUGPX(fmt, ...) \
- do { if (0) printk(fmt, ##__VA_ARGS__); } while (0)
+ do {\
+ if (0) \
+ printk_emit(0, LOGLEVEL_DEBUG, NULL, 0, \
+ fmt, ##__VA_ARGS__);\
+ } while (0)
#define US_DEBUG(x)
#endif
--
2.7.0.rc3.207.g0ac5344
--
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
Business Partnership
Hello, I am Mr. LAURENT EYADEMA from Republic of Togo.please read the attached proposal. Thanks in anticipation of your urgent response, LAURENT EYADEMA proposal.docx Description: Binary data
Business Partnership
Hello, I am Mr. LAURENT EYADEMA from Republic of Togo.please read the attached proposal. Thanks in anticipation of your urgent response, LAURENT EYADEMA proposal.docx Description: Binary data
Re: [PATCH] cdc-acm: implement put_char() and flush_chars()
On Wed, 2016-02-10 at 11:58 -0800, Peter Hurley wrote: > > I was just suggesting that sometimes a > > temporary hack which makes things work and can later be removed when > > there is a proper fix for the problem is better than having it > broken > > for five years straight. > > My preference would be for more engineers/developers/hackers to invest > the time and effort to learn and contribute to this important area. > I wasn't born knowing the tty and serial cores. It raises a question. If I understand Sven's use case correctly, he uses cdc-acm as a connection between systems. A data pipe not a true serial device. Now you may call this an abuse but cdc-acm has the whole acm logic, so my first instinct that Sven should use a driver of his own is not going to fly. So are we trying to fix the tty layer for a connection that shouldn't use it? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Business Partnership
Hello, I am Mr. LAURENT EYADEMA from Republic of Togo.please read the attached proposal. Thanks in anticipation of your urgent response, LAURENT EYADEMA proposal.docx Description: Binary data
