Re: [PATCH v17 2/3] usb: USB Type-C connector class
On Tue, Apr 25, 2017 at 7:40 PM, Guenter Roeck wrote:
> On 04/25/2017 01:26 AM, Rajaram R wrote:
>>
>> On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan
>> wrote:
>>>
>>> On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R
>>> wrote:
>>>>
>>>> On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck
>>>> wrote:
>>>>>
>>>>> On Fri, Apr 21, 2017 at 07:57:52PM +0530, Rajaram R wrote:
>>>>>>
>>>>>> On Fri, Apr 21, 2017 at 1:16 AM, Badhri Jagan Sridharan
>>>>>> wrote:
>>>>>>>
>>>>>>> Thanks for the responses :)
>>>>>>>
>>>>>>> So seems like we have a plan.
>>>>>>>
>>>>>>> In Type-C connector class the checks for TYPEC_PWR_MODE_PD
>>>>>>> and pd_revision for both the port and the partner will be removed in
>>>>>>> power_role_store and the data_role_store and will be delegated
>>>>>>> to the low level drivers.
>>>>>>
>>>>>>
>>>>>> It is important to remember what USB Type-C provide is mechanisms for
>>>>>> "TRYing" to become a particular role and not guaranteeing.
>>>>>>
>>>>>> With what device combination do you fore see we could get the desired
>>>>>> role with this change ?
>>>>>>
>>>>>
>>>>> If the partner is not PD capable, if a preferred role is specified,
>>>>> if the current cole does not match the preferred role, and if the
>>>>> request
>>>>> is to set the role to match the preferred role, I think it is
>>>>> reasonable
>>>>> to expect that re-establishing the connection would accomplish that if
>>>>> the
>>>>> partner supports it.
>>>>>
>>>> In this context I believe we have two different inputs as follows:
>>>>
>>>> /sys/class/typec//supported_power_roles
>>>> /sys/class/typec//preferred_role
>>>>
>>>> The need of preferred role is required when DRP is set in
>>>> supported_power_roles option.
>>>> Ideally a battery powered device will TRY to be SNK and a a/c plugged
>>>> device will TRY to be SRC
>>>>
>>>> We need to understand which non-PD device will set to DRP? In the
>>>
>>>
>>> Android Phones (actually it could be any phone which has a type-c port)
>>> since it can act as usb gadget (when connected to PC) or Usb host
>>> when connected to peripherals such as thumb drives, keyboard etc.
>>> Phones with smaller form factors might be thermally limited to charge
>>> above 15W, therefore supporting PD might be an overkill for them.
>>>
>>>> current ecosystem all legacy devices
>>>> will sit behind adapters which either present an Rp or Rd.
>>>>
>>>> If it is a power adapter in 5V range can either present Rp or DRP with
>>>> TRY.SRC and there is no role swap requirement.
>>>>
>>>> If it is a laptop port or similar with non-PD (??) DRP there is no
>>>> guaranteed role swap in a non-PD mode.
>>>
>>>
>>> This is true, but following a Try.SRC or Try.SNK state machine can
>>> increase the chances of landing in the desired role/preferred role.
>>
>>
>> Agree and as indicated it increases only chances.
>>
>
> FWIW, this is pretty much the same as requesting a role change using PD.
> Based on my experience with various devices, the chance for that to succeed
> isn't really that high either.
>
> I am not really sure I understand your problem with using Try.{SRC,SNK}
> to trigger (or attempt to trigger) a role change. Can we take a step back,
> and can you explain ?
The parameters required for a type-c connection is defined as follows
and will have a default value.
/sys/class/typec//supported_power_roles
/sys/class/typec//preferred_role
When two DRP devices are connected and for which we have
preferred_role which provides input on the preference, In a DRP mode
we have randomness in the mode of connection and hence we require role
swap mechanisms. A Type-C only device cannot role swap as this is
valid only in PD operation.
# Question was how to choose a particular role in non-PD mode. Only
way to have a deterministic role in a non-PD mode is to set expected
supported_role of choice rather than DRP.
# As part of the solution suggested, checking of roles and triggering
role swaps has to be done by the policy manager(PM) and delinked from
Policy Engine. I guess the policy manager is at user space?.
I do not have the complete git repo and may be i could be missing
something. If this is in any public git please let me know
>
> Thanks,
> Guenter
>
--
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 v17 2/3] usb: USB Type-C connector class
On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan wrote: > On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R > wrote: >> On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck wrote: >>> On Fri, Apr 21, 2017 at 07:57:52PM +0530, Rajaram R wrote: >>>> On Fri, Apr 21, 2017 at 1:16 AM, Badhri Jagan Sridharan >>>> wrote: >>>> > Thanks for the responses :) >>>> > >>>> > So seems like we have a plan. >>>> > >>>> > In Type-C connector class the checks for TYPEC_PWR_MODE_PD >>>> > and pd_revision for both the port and the partner will be removed in >>>> > power_role_store and the data_role_store and will be delegated >>>> > to the low level drivers. >>>> >>>> It is important to remember what USB Type-C provide is mechanisms for >>>> "TRYing" to become a particular role and not guaranteeing. >>>> >>>> With what device combination do you fore see we could get the desired >>>> role with this change ? >>>> >>> >>> If the partner is not PD capable, if a preferred role is specified, >>> if the current cole does not match the preferred role, and if the request >>> is to set the role to match the preferred role, I think it is reasonable >>> to expect that re-establishing the connection would accomplish that if the >>> partner supports it. >>> >> In this context I believe we have two different inputs as follows: >> >> /sys/class/typec//supported_power_roles >> /sys/class/typec//preferred_role >> >> The need of preferred role is required when DRP is set in >> supported_power_roles option. >> Ideally a battery powered device will TRY to be SNK and a a/c plugged >> device will TRY to be SRC >> >> We need to understand which non-PD device will set to DRP? In the > > Android Phones (actually it could be any phone which has a type-c port) > since it can act as usb gadget (when connected to PC) or Usb host > when connected to peripherals such as thumb drives, keyboard etc. > Phones with smaller form factors might be thermally limited to charge > above 15W, therefore supporting PD might be an overkill for them. > >> current ecosystem all legacy devices >> will sit behind adapters which either present an Rp or Rd. >> >> If it is a power adapter in 5V range can either present Rp or DRP with >> TRY.SRC and there is no role swap requirement. >> >> If it is a laptop port or similar with non-PD (??) DRP there is no >> guaranteed role swap in a non-PD mode. > > This is true, but following a Try.SRC or Try.SNK state machine can > increase the chances of landing in the desired role/preferred role. Agree and as indicated it increases only chances. > >> So we need to understand what non PD device will fit into this scenario ? > Answered above. > >> >>> Of course, that won't change anything if the partner does not support the >>> desired role, but it is better than doing nothing. This is also comparable >>> to requesting a role change from the partner if it does support PD. >> >> All I am highlighting is that we can only TRY and there is no >> guaranteed role swap with Type-C >> >>> Do you have a better idea ? >>> >> If need a guaranteed role in a non-PD mode we need to set the required >> role in supported_power_roles. >> An understanding of scenario will help take better approach. > > The current Type-c connector class interface defines the support_*_roles as > read-only nodes. Leaving that apart, I think what you are trying to say is > that > instead of running through the state machine again by switching to > Try.SRC or Try.SNK, you are suggesting that switch from DRP to source/host > (or) sink/device to make sure that CC is either pulled up through Rp or > grounded through Rd so that it increases the chances of settling in the > desired > role. I do agree this, but, there is a pitfall here. Say when a DRP is > connected to > a pure sink/device, when the DRP switches to being a pure sink as well, then > the port roles would not resolve at all as both would be asserting Rd on CC > and If it is a pure SNK(presenting Rd), there is no conflict and a DRP be it TRY SRC or TRY SNK will endup as SRC. > therefore it might not be possible to detect a disconnect unless we have > a VCONN powered cable. Following Try.SRC, Try.SNK state machine actually VConn does not play any role in detection. > takes care of this for you. When in Try.SRC or Try.SNK state, CC would either > be pulled up or down for a specific amount of
Re: [PATCH v17 2/3] usb: USB Type-C connector class
On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck wrote: > On Fri, Apr 21, 2017 at 07:57:52PM +0530, Rajaram R wrote: >> On Fri, Apr 21, 2017 at 1:16 AM, Badhri Jagan Sridharan >> wrote: >> > Thanks for the responses :) >> > >> > So seems like we have a plan. >> > >> > In Type-C connector class the checks for TYPEC_PWR_MODE_PD >> > and pd_revision for both the port and the partner will be removed in >> > power_role_store and the data_role_store and will be delegated >> > to the low level drivers. >> >> It is important to remember what USB Type-C provide is mechanisms for >> "TRYing" to become a particular role and not guaranteeing. >> >> With what device combination do you fore see we could get the desired >> role with this change ? >> > > If the partner is not PD capable, if a preferred role is specified, > if the current cole does not match the preferred role, and if the request > is to set the role to match the preferred role, I think it is reasonable > to expect that re-establishing the connection would accomplish that if the > partner supports it. > In this context I believe we have two different inputs as follows: /sys/class/typec//supported_power_roles /sys/class/typec//preferred_role The need of preferred role is required when DRP is set in supported_power_roles option. Ideally a battery powered device will TRY to be SNK and a a/c plugged device will TRY to be SRC We need to understand which non-PD device will set to DRP? In the current ecosystem all legacy devices will sit behind adapters which either present an Rp or Rd. If it is a power adapter in 5V range can either present Rp or DRP with TRY.SRC and there is no role swap requirement. If it is a laptop port or similar with non-PD (??) DRP there is no guaranteed role swap in a non-PD mode. So we need to understand what non PD device will fit into this scenario ? > Of course, that won't change anything if the partner does not support the > desired role, but it is better than doing nothing. This is also comparable > to requesting a role change from the partner if it does support PD. All I am highlighting is that we can only TRY and there is no guaranteed role swap with Type-C > Do you have a better idea ? > If need a guaranteed role in a non-PD mode we need to set the required role in supported_power_roles. An understanding of scenario will help take better approach. > Thanks, > Guenter > >> >> > >> > TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if >> > current_role is not same as the preferred_role. >> > > > ... if the partner is not PD capable. > >> > I am going to make changes in my local kernel code base to start >> > making the corresponding changes in userspace. >> > Should I post-back the local kernel changes or Heikki and Geunter >> > you are planning to upload them ? >> > >> > Thanks for the support !! >> > Badhri. >> > >> > On Thu, Apr 20, 2017 at 5:24 AM, Heikki Krogerus >> > wrote: >> >> On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote: >> >>> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck >> >>> wrote: >> >>> > On Wed, Apr 19, 2017 at 07:45:00AM -0700, Badhri Jagan Sridharan wrote: >> >>> >> On Wed, Apr 19, 2017 at 4:23 AM, Heikki Krogerus >> >>> >> wrote: >> >>> >> > Hi, >> >>> >> > >> >>> >> > On Tue, Apr 18, 2017 at 11:52:33AM -0700, Badhri Jagan Sridharan >> >>> >> > wrote: >> >>> >> >> Hi Heikki, >> >>> >> >> >> >>> >> >> I have a question regarding the preferred_role node. >> >>> >> >> >> >>> >> >> +What: /sys/class/typec//preferred_role >> >>> >> >> +Date: March 2017 >> >>> >> >> +Contact: Heikki Krogerus >> >>> >> >> +Description: >> >>> >> >> + The user space can notify the driver about the >> >>> >> >> preferred role. >> >>> >> >> + It should be handled as enabling of Try.SRC or >> >>> >> >> Try.SNK, as >> >>> >> >> + defined in USB Type-C specification, in the port >> >>> >> >> drivers. By >> >>> >> >> + default the preferred role should come from the >> >>&
Re: [PATCH v17 2/3] usb: USB Type-C connector class
On Fri, Apr 21, 2017 at 1:16 AM, Badhri Jagan Sridharan
wrote:
> Thanks for the responses :)
>
> So seems like we have a plan.
>
> In Type-C connector class the checks for TYPEC_PWR_MODE_PD
> and pd_revision for both the port and the partner will be removed in
> power_role_store and the data_role_store and will be delegated
> to the low level drivers.
It is important to remember what USB Type-C provide is mechanisms for
"TRYing" to become a particular role and not guaranteeing.
With what device combination do you fore see we could get the desired
role with this change ?
>
> TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
> current_role is not same as the preferred_role.
>
> I am going to make changes in my local kernel code base to start
> making the corresponding changes in userspace.
> Should I post-back the local kernel changes or Heikki and Geunter
> you are planning to upload them ?
>
> Thanks for the support !!
> Badhri.
>
> On Thu, Apr 20, 2017 at 5:24 AM, Heikki Krogerus
> wrote:
>> On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote:
>>> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck wrote:
>>> > On Wed, Apr 19, 2017 at 07:45:00AM -0700, Badhri Jagan Sridharan wrote:
>>> >> On Wed, Apr 19, 2017 at 4:23 AM, Heikki Krogerus
>>> >> wrote:
>>> >> > Hi,
>>> >> >
>>> >> > On Tue, Apr 18, 2017 at 11:52:33AM -0700, Badhri Jagan Sridharan wrote:
>>> >> >> Hi Heikki,
>>> >> >>
>>> >> >> I have a question regarding the preferred_role node.
>>> >> >>
>>> >> >> +What: /sys/class/typec//preferred_role
>>> >> >> +Date: March 2017
>>> >> >> +Contact: Heikki Krogerus
>>> >> >> +Description:
>>> >> >> + The user space can notify the driver about the
>>> >> >> preferred role.
>>> >> >> + It should be handled as enabling of Try.SRC or
>>> >> >> Try.SNK, as
>>> >> >> + defined in USB Type-C specification, in the port
>>> >> >> drivers. By
>>> >> >> + default the preferred role should come from the
>>> >> >> platform.
>>> >> >> +
>>> >> >> + Valid values: source, sink, none (to remove
>>> >> >> preference)
>>> >> >>
>>> >> >> What is the expected behavior when the userspace changes the
>>> >> >> preferred_role node when the port is in connected state ?
>>> >> >>
>>> >> >> 1. the state machine re-resolves the port roles right away based on
>>> >> >> the new state machine in place ? (or)
>>> >> >
>>> >> > No! There are separate attributes for sending role swap requests.
>>> >>
>>> >> Right. But, that might not be helpful in cases when PD is not
>>> >> implemented.
>>> >> and Implementing PD is not mandatory according the spec :/
>>> >>
>>> >> FYI quoting from the Type-C specification release(page 24),
>>> >> role swaps are not limited to devices that only support PD.
>>> >>
>>> >> "Two independent set of mechanisms are defined to allow a USB Type-C
>>> >> DRP to functionally swap power and data roles. When USB PD is
>>> >> supported, power and data role swapping is performed as a subsequent
>>> >> step following the initial connection process. For non-PD
>>> >> implementations,
>>> >> power/data role swapping can optionally be dealt with as part of the
>>> >> initial
>>> >> connection process."
>>> >>
>>> >> But, the current interface definition actually prevents current/data role
>>> >> swaps for non-pd devices.
>>> >>
>>>
>>> > This is correct for the attribute definition, but it is not implemented
>>> > that way. Writing the attribute is only read-only for non-DRP ports.
>>>
>>> i.e. tcpm_dr_set/tcpm_pr_set at tcpm.c would return EINVAL when type
>>> is not TYPEC_PORT_DRP, is that what you are referring to ?
>>>
>>> if (port->typec_caps.type != TYPEC_PORT_DRP) {
>>> ret = -EINVAL;
>>> goto port_unlock;
>>> }
>>>
>>> I do agree that this is actually correct. I am referring to the case
>>> where port is
>>> dual-role-power and dual-role-data but NOT PD capable.
>>>
>>> > Given the standard, I would consider that to be intentional; it might
>>> > make sense to update the description accordingly.
>>> >
>>> > How about implementing a mechanism in the dr_set and pr_set code in tcpm
>>> > which would handle that situation ? Something along the line of
>>> >
>>> > if (!port->pd_capable && connected && current role != desired
>>> > role) {
>>> > reset_port();
>>> > goto done;
>>> > }
>>>
>>> By "desired role" you are referring to preferred_role right ?
>>>
>>> If so yes, That's a good idea as well and it might work as long as
>>> type-c connector
>>> class allows the call to reach tcpm code :) But the current connector
>>> class code does
>>> not allow that because the power_role and data_role nodes are defined that
>>> way.
>>
>> Well, the data_role does not limit the requests from reaching the low
>> level drivers, but..
>>
>>> port->cap->pd_revision and the port->pwr_opmode check i
Re: [PATCHv3 1/2] usb: USB Type-C connector class
On Wed, Jun 29, 2016 at 4:00 PM, Heikki Krogerus wrote: > On Wed, Jun 29, 2016 at 02:21:10PM +0530, Rajaram R wrote: >> On Mon, Jun 27, 2016 at 5:43 PM, Heikki Krogerus >> wrote: >> > Hi, >> > >> > On Mon, Jun 27, 2016 at 03:51:08PM +0530, Rajaram R wrote: >> >> May be I am missing user or usage of the driver.. I see this driver is >> >> providing limited information of the Type-C connectors or the port >> >> partner >> > >> > Yes, this interface can't provide directly information received from >> > PD commands like Discover Identity. We will have to present the >> > partners even when USB PD is not supported and in a consistent >> > fashion. Some details will be available in any case indirectly. Like >> > if there are modes, there will be devices presenting them, and the >> > product type in case of partners will be the partner type. >> >> Agree. What is the end use of this driver? IMO end use case will >> decide what attributes to be shared. Since we are terming this as a >> universal representation for user space we may need to expose details >> such as Discovery details say Vendor ID, Product ID, Super Speed >> support etc which are not related to alt mode. In the legacy drivers >> complete descriptors of the device is available for user space to >> build applications. > > The details about the USB connection are out side the scope the this > class, and in most cases the port driver will not even have them at > their disposal. We can determine that the connector is in USB mode, > and that's about it. > > But those details will in any case be exposed by the USB subsystem, so > why should we duplicate them? The user space has been so far relying > on getting the details from the normal interfaces the USB subsystem > provides and that should not change. Apologize for bringing in USB example. I used it to as an example to say that complete device details are exposed to user space by other drivers. Sticking to the current topic/context more details of Type-C port/partner, a detailed information(with restrictions) will help build more user applications. > > > 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: [PATCHv3 1/2] usb: USB Type-C connector class
On Mon, Jun 27, 2016 at 5:43 PM, Heikki Krogerus wrote: > Hi, > > On Mon, Jun 27, 2016 at 03:51:08PM +0530, Rajaram R wrote: >> May be I am missing user or usage of the driver.. I see this driver is >> providing limited information of the Type-C connectors or the port >> partner > > Yes, this interface can't provide directly information received from > PD commands like Discover Identity. We will have to present the > partners even when USB PD is not supported and in a consistent > fashion. Some details will be available in any case indirectly. Like > if there are modes, there will be devices presenting them, and the > product type in case of partners will be the partner type. Agree. What is the end use of this driver? IMO end use case will decide what attributes to be shared. Since we are terming this as a universal representation for user space we may need to expose details such as Discovery details say Vendor ID, Product ID, Super Speed support etc which are not related to alt mode. In the legacy drivers complete descriptors of the device is available for user space to build applications. > > But there are a couple of attributes I have been thinking about adding > for the partners: > > supported_data_roles > supports_usb_power_delivery > > The supported data roles would respond bits 30 and 31 of the ID Header > VDO. But when the partner does not support USB PD, we will have to > report "unknown" in it. > > Oliver, Guenter! How do you guys feel about those? Is there any use > for them? > > >> On Mon, Jun 27, 2016 at 3:21 PM, Heikki Krogerus >> wrote: >> > On Fri, Jun 24, 2016 at 07:54:12PM +0530, Rajaram R wrote: >> >> On Tue, Jun 21, 2016 at 8:21 PM, Heikki Krogerus >> >> wrote: >> >> > The purpose of USB Type-C connector class is to provide >> >> > unified interface for the user space to get the status and >> >> > basic information about USB Type-C connectors on a system, >> >> >> >> Since we are defining this is as a unified interface for user space, >> >> will the interface include identity details of local port and peer. >> >> Or am I over looking something ? >> > >> > By peer, do you mean the partners? Sorry but could you elaborate the >> > question? > > > 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: USB host port {,performance} issues with Beaglebone
Victor On Tue, Mar 8, 2016 at 9:05 AM, Victor Dodon wrote: > > Hi all, > > I have some performance issues with the host port on a Beaglebone > board. I tested with kernel 3.8.13, 3.14.55 and 4.1.18 and the issue > still persists. Running a fio test with 64k random reads from a USB > flash drive yields a maximum of 14402.01 KiB/s (115216.08 Kb/s). The > 3.14 and 4.1 kernels where build with CONFIG_TI_CPPI41_DMA=y. I was > able to get a much better performance on the client USB port by > enabling fifo double buffering. Iperf over a gigabit connection and a Ethernet > to USB adapter plugged in the host port gives a maximum of 180Mbit/s with fifo > double buffering enabled for the ep1 and ep2. Do you have answer for the following - What is the raw throughput of musb driver ? ( With no class driver) - What is the performance you are expecting ? performance of MSC depends on storage medium as well. May be you should use high performance storage device - For Ethernet, did you try NCM ? I believe that gives better results With little bit of experience in musb, I have seen only similar performances in 3.0 Kernel. Any tweaks for improvement would help the community. > > Are there any known performance issues in the musb driver? For my use > case I need > a higher bandwidth and I would like to improve the host controller, > but I'm a beginner in Kernel hacking and I would appreciate some help, > tips or any cues to start. > > I also found a few problems with the host port. For example: > Using the setup described above (gigabit connection and a Ethernet > to USB adapter plugged in the host port and with a running iSCSI > initiator on the BB, > in usb/musb/musb_core.c if I change mode_4_cfg to enable double > buffering, and I restart the board while doing a dd from the disk > mounted with iSCSI, the kernel stops at: > > > Kind regards, > Victor Dodon. > -- > 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 -- 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 Thu, Feb 18, 2016 at 4:17 PM, Heikki Krogerus wrote: > On Thu, Feb 18, 2016 at 04:07:54PM +0530, Rajaram R wrote: >> On Wed, Feb 17, 2016 at 7:58 PM, Heikki Krogerus >> wrote: >> > On Wed, Feb 17, 2016 at 03:36:46PM +0200, Felipe Balbi wrote: >> >> >> >> Hi, >> >> >> >> Heikki Krogerus writes: >> >> > On Wed, Feb 17, 2016 at 11:36:52AM +0100, Oliver Neukum wrote: >> >> >> On Wed, 2016-02-17 at 12:29 +0200, Felipe Balbi wrote: >> >> >> > Hi, >> >> >> > >> >> >> > Oliver Neukum writes: >> >> >> > > On Wed, 2016-02-17 at 09:58 +0200, Heikki Krogerus wrote: >> >> >> > >> On Tue, Feb 16, 2016 at 02:39:47PM +0100, Oliver Neukum wrote: >> >> >> >> >> >> > >> > Yes, but we need an API. We can't keep adding to it. So if that >> >> >> > >> > is to be supported, it needs to be defined now. >> >> >> > >> >> >> >> > >> When you say API, do you mean the API the class provides to the >> >> >> > >> drivers? Or did you mean ABI which would be the sysfs in this >> >> >> > >> case? >> >> >> > > >> >> >> > > The API to user space. That is the point. We cannot break user >> >> >> > > space. >> >> >> > > Once this sysfs API is upstream we are stuck with it. >> >> >> > >> >> >> > yeah, in fact I have been wondering if sysfs is the best interface to >> >> >> >> >> >> That is the discussion we must have. >> >> >> >> >> >> > userspace. I talked with Heikki a few days back about this; I was >> >> >> > wondering if something like what the NFC folks did with netlink >> >> >> > would be >> >> >> > better here. >> >> >> >> >> >> I doubt that, because the main user is likely to be udev scripts. >> >> >> They can easily deal with sysfs attributes. >> >> > >> >> > IMHO for high level interface like this, sysfs is ideal because of the >> >> > simple fact that you only need a shell to access the files. netlink >> >> > would make us depend on custom software, no? >> >> > >> >> > I'm not against using netlink, but what would be the benefit from it >> >> > in this case? >> >> >> >> With HW we see nowadays, CC stack is hidden on some microcontroller, but >> >> is it too far-fetched to consider a system where this is not the case ? >> > >> > There already are several USB PD stacks out there, like also Greg >> > pointed out. >> > >> >> Specially when we consider things like power delivery which, I know, you >> >> wanted to keep it out of this interface, however we would have two >> >> 'stacks' competing for access to the same pins, right ? >> > >> > No. This class would be the top layer for the coming stack, where ever >> > it ends up coming. The class is only the interface to the user space >> > and nothing else. >> > >> > By saying we need to keep USB Type-C separate from USB PD I meant that >> > the userspace access can not be mixed somewhere in layers of the USB >> > PD/CC stack like it has been in the USB PD stacks I've seen so far. >> > They assume that we always use the software USB PD stack with USB >> > Type-C, which as we can see is not true when the stack is implemented >> > in EC or firmware or some complex USB PD controller or what ever. >> > However, the operations the userspace needs to do are exactly the same >> > in both cases. >> > >> > - data role swapping >> > - power role swapping (depends on USB PD) >> > - Alternate Modes (depends on USB PD) >> > >> > And we really should not forget that we actually also have USB Type-C >> > PHYs that can't do any USB PD communication over the CC pin, so USB PD >> > is simply not always going to be available. But the data role swapping >> > and also accessories are still available with them, as the do not need >> > USB PD. >> > >> > This was the whole point with the class. It allows the different ways >> > of dealing with Type-C ports to be exposed to userspace in the same >> > way.
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Wed, Feb 17, 2016 at 7:58 PM, Heikki Krogerus wrote: > On Wed, Feb 17, 2016 at 03:36:46PM +0200, Felipe Balbi wrote: >> >> Hi, >> >> Heikki Krogerus writes: >> > On Wed, Feb 17, 2016 at 11:36:52AM +0100, Oliver Neukum wrote: >> >> On Wed, 2016-02-17 at 12:29 +0200, Felipe Balbi wrote: >> >> > Hi, >> >> > >> >> > Oliver Neukum writes: >> >> > > On Wed, 2016-02-17 at 09:58 +0200, Heikki Krogerus wrote: >> >> > >> On Tue, Feb 16, 2016 at 02:39:47PM +0100, Oliver Neukum wrote: >> >> >> >> > >> > Yes, but we need an API. We can't keep adding to it. So if that >> >> > >> > is to be supported, it needs to be defined now. >> >> > >> >> >> > >> When you say API, do you mean the API the class provides to the >> >> > >> drivers? Or did you mean ABI which would be the sysfs in this case? >> >> > > >> >> > > The API to user space. That is the point. We cannot break user space. >> >> > > Once this sysfs API is upstream we are stuck with it. >> >> > >> >> > yeah, in fact I have been wondering if sysfs is the best interface to >> >> >> >> That is the discussion we must have. >> >> >> >> > userspace. I talked with Heikki a few days back about this; I was >> >> > wondering if something like what the NFC folks did with netlink would be >> >> > better here. >> >> >> >> I doubt that, because the main user is likely to be udev scripts. >> >> They can easily deal with sysfs attributes. >> > >> > IMHO for high level interface like this, sysfs is ideal because of the >> > simple fact that you only need a shell to access the files. netlink >> > would make us depend on custom software, no? >> > >> > I'm not against using netlink, but what would be the benefit from it >> > in this case? >> >> With HW we see nowadays, CC stack is hidden on some microcontroller, but >> is it too far-fetched to consider a system where this is not the case ? > > There already are several USB PD stacks out there, like also Greg > pointed out. > >> Specially when we consider things like power delivery which, I know, you >> wanted to keep it out of this interface, however we would have two >> 'stacks' competing for access to the same pins, right ? > > No. This class would be the top layer for the coming stack, where ever > it ends up coming. The class is only the interface to the user space > and nothing else. > > By saying we need to keep USB Type-C separate from USB PD I meant that > the userspace access can not be mixed somewhere in layers of the USB > PD/CC stack like it has been in the USB PD stacks I've seen so far. > They assume that we always use the software USB PD stack with USB > Type-C, which as we can see is not true when the stack is implemented > in EC or firmware or some complex USB PD controller or what ever. > However, the operations the userspace needs to do are exactly the same > in both cases. > > - data role swapping > - power role swapping (depends on USB PD) > - Alternate Modes (depends on USB PD) > > And we really should not forget that we actually also have USB Type-C > PHYs that can't do any USB PD communication over the CC pin, so USB PD > is simply not always going to be available. But the data role swapping > and also accessories are still available with them, as the do not need > USB PD. > > This was the whole point with the class. It allows the different ways > of dealing with Type-C ports to be exposed to userspace in the same > way. > >> IIRC mode and role negotiation goes via CC pins using the power delivery >> protocol. If I misunderstand anything, let me know. > > The data role swap with USB Type-C connectors is in no way tied to USB > Power Delivery. The USB Type-C spec defines that when USB PD is Its not data role swap i guess its dual role, A Data role swap is tied with USB PD, > available, DR_Swap USB PD function is used to swap the role, otherwise > emulated disconnect will do the trick. I doubt a USB host with no device capability implement DRP ?? Also emulated trick(??) is not spec requirement rt ? > > Data role swapping is a must thing to have with USB Type-C connectors I guess you are referring to Dual role (DRP) and not data role (DRD). > because of the fact that the role is selected randomly. Regardless was > USB PD supported or not. > > > 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 -- 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 0/3] usb: USB Type-C Class and driver for UCSI
On Tue, Feb 9, 2016 at 10:31 PM, Heikki Krogerus wrote: > Hi, > > The OS, or more precisely the user space, needs to be able to control > a few things regarding USB Type-C ports. The first thing that must be > allowed to be controlled is the data role. USB Type-C ports will > select the data role randomly with DRP ports. When USB PD is > supported, also independent (from data role) power role swapping can > be supported together with Alternate Mode control. > > I'm proposing with this set a Class for the Type-C connectors that > gives the user space control over those things on top of getting basic > details about the USB Type-C connectors and also partners. The details > include the capabilities of the port, the supported data and power > roles, supported accessories (audio and debug), supported Alternate > Modes, USB PD support and of course the type of the partner (USB, Alt > Mode, Accessory or Charger), and more or less the same details about > the partner. > > I'm not considering cables with this Class, and I have deliberately Since we have capability details of ports in user space, I believe cable capability is also necessary for policy decision(power, alt mode). Is that something we are cautiously leaving out ? pls explain > left out some more technical details, like cable orientation, firstly > because I did not see much use for the user space from knowing that > an secondly because that kind of details are not always available for > example with UCSI. > > So the interface to the user space is kept as simple as I dared to > make it. > > NOTE: In case there is somebody wondering, this is not adding USB PD > support to Linux kernel. This is just about USB Type-C. > > > Heikki Krogerus (3): > usb: USB Type-C Connector Class > usb: type-c: USB Type-C Connector System Software Interface > usb: type-c: UCSI ACPI driver > > drivers/usb/Kconfig| 2 + > drivers/usb/Makefile | 2 + > drivers/usb/type-c/Kconfig | 25 +++ > drivers/usb/type-c/Makefile| 3 + > drivers/usb/type-c/typec.c | 446 > drivers/usb/type-c/ucsi.c | 450 > + > drivers/usb/type-c/ucsi.h | 219 > drivers/usb/type-c/ucsi_acpi.c | 133 > include/linux/usb/typec.h | 114 +++ > 9 files changed, 1394 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 drivers/usb/type-c/ucsi.c > create mode 100644 drivers/usb/type-c/ucsi.h > create mode 100644 drivers/usb/type-c/ucsi_acpi.c > create mode 100644 include/linux/usb/typec.h > > -- > 2.7.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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: FX3 on Linux
Please find reply inline On Fri, Jun 12, 2015 at 11:48 PM, wrote: > > Knowing that you have experts in USB 3.0 support for Linux, I am writing to > your organization hoping that you may be able to offer some direction on a > problem I have encountered. > > I am trying to track down a problem I am having with a USB 3.0 PCIe-x1 card > on Linux while trying to talk to a Cypress FX3 explorer board. I posted the > following to the Cypress forums. However, after further investigation, I > think it is the ASM1042 controller on the PCIe card. Other than going to a > different USB 3.0 card, is there anything from the Linux side that I might be > able to do to get by the error condition described below? I am doing > asynchronous reads through libusb. Thanks for your help and insight. > > Cypress Post > I have run into an odd problem using the FX3 (CYUSB3KIT-003) on Linux using > libusb. > > I have a java test app that calls down through C++ DLLs (so, dylib) to > communicate > with the FX3 (which is running the default bulk src and sink firmware that > came > programmed on the board). It does a simple sequence of operations: open, > write block, > read block, single huge read of data, and close. Prior to each read, I fill > the target > buffer with a set pattern. The FX3 firmware, on a read, fills the target with > 0xAA, > overwriting my pattern as it should. The blue LED on the board is blinking, > so I know > we are using USB 3.0. > > For Windows, these operations go through WinUSB. There are no problems. > Everything > works well. Likewise everything works well with the Mac OSX using libusb. The > test programs can be run any number of times and the operations and incoming > data > are fine. > > With Linux, the first time through the test program, everything is fine. > However, > running the program a second time results in the very first read either > timing out with > the target buffer being filled with 0x00 (on one of my test machines) or (on > another > test machine) having the read "succeed" (result indicates the number of bytes > to be > read were read without error) but not having anything change in the target > buffer. > If the read is immediately retried, it works fine (or if subsequent different > reads > are done, they work fine). Please provide usbmon logs and dmesg prints from xHCI to understand what is happening @ driver level. > > It stays in this state until I either unplug and replug the board or reset the > system. On a couple of occasions, after unplugging and replugging the board, > the > blue LED on the board does not even come on at all. At that point, I have to > reboot > the system in order for it to even recognize that the FX3 is plugged in. > > When I plug the FX3 board into a USB 2.0 only socket on the Linux machine > (blue LED > on board is on solid), I see no problems. It is obviously slower, but > everything > works as expected. > > At this point, I am suspecting that either the Linux USB 3.0 handling has a > problem, > libusb has a problem (though it worked fine on the 2.0 port), or the FX3 has a > problem (unlikely). > > Are there know issues in using the FX3 on a USB 3.0 port on Linux? There are no known issue. FX3 works with other platforms. This could be xHCI limitation. >I am running a > 64-bit quad core Xeon Intel machine running Mint 16. I have a PPA Int'l USB > 3.0 SuperSpeed > PCIe-x1 card that provides two USB 3.0 ports (per PPA's support group, the > GT50PCIEUSB3 USB 3.0 controller uses the ASM1042 chipset). > > Thanks. > End Cypress Post > > > > lspci -vv (output for ASM1042 controller) > > 02:00.0 USB controller: ASMedia Technology Inc. ASM1042 SuperSpeed USB Host > Controller (prog-if 30 [XHCI]) > Subsystem: Device 1234:5678 > Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- > Stepping- SERR- FastB2B- DisINTx+ > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- > SERR- Latency: 0, Cache Line Size: 64 bytes > Interrupt: pin A routed to IRQ 16 > Region 0: Memory at d410 (64-bit, non-prefetchable) [size=32K] > Capabilities: > Kernel driver in use: xhci_hcd > > > lspci | grep USB > 00:1a.0 USB controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI > Controller #4 > 00:1a.1 USB controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI > Controller #5 > 00:1a.7 USB controller: Intel Corporation 82801JI (ICH10 Family) USB2 EHCI > Controller #2 > 00:1d.0 USB controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI > Controller #1 > 00:1d.1 USB controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI > Controller #2 > 00:1d.2 USB controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI > Controller #3 > 00:1d.3 USB controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI > Controller #6 > 00:1d.7 USB controller: Intel Corporation 82801JI (ICH10 Family) USB2 EHCI > Controller #1 > 02:
Re: disable VBUS?
On Wed, May 14, 2014 at 6:04 AM, Grant wrote: > Can I disable VBUS while keeping the rest of USB functional for a > device that does not require bus power? Can you please elaborate the question of why VBus should go off ? Is this question in the context of any new USB specification ? > > - Grant > -- > 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 -- 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 Device stops working after 200001 interrupts
Josh Could you provide kernel logs (dmesg) which include logs from IR module ( http://lxr.free-electrons.com/source/drivers/media/rc/mceusb.c) Cheers On Wed, Jan 22, 2014 at 11:15 PM, Josh Bendavid wrote: > I have a usb infrared remote receiver (Windows media center variety) which > works correctly at boot, but stops working sometime later. > > Kernel gives: > [ 1101.490321] irq 21: nobody cared (try booting with the "irqpoll" option) > [ 1101.490328] CPU: 0 PID: 0 Comm: swapper/0 Tainted: P O 3.13.0-rc8 #1 > [ 1101.490330] Hardware name: System manufacturer System Product > Name/P5N7A-VM, BIOS 0407 11/18/2008 > [ 1101.490333] 88011ae2e584 81704b5d > 88011ae2e500 > [ 1101.490337] 8105c584 88011ae2e500 > 0015 > [ 1101.490341] 8105c930 88011ae2e500 > 88011ae2e500 > [ 1101.490344] Call Trace: > [ 1101.490346] [] ? dump_stack+0x41/0x51 > [ 1101.490358] [] ? __report_bad_irq+0x1e/0xbb > [ 1101.490361] [] ? note_interrupt+0x163/0x1ef > [ 1101.490366] [] ? handle_irq_event_percpu+0xfd/0x114 > [ 1101.490370] [] ? handle_irq_event+0x2e/0x4d > [ 1101.490373] [] ? handle_fasteoi_irq+0x71/0xa3 > [ 1101.490377] [] ? handle_irq+0x15/0x20 > [ 1101.490380] [] ? do_IRQ+0x41/0x97 > [ 1101.490384] [] ? common_interrupt+0x67/0x67 > [ 1101.490498] [] ? rm_isr+0x93/0x129 [nvidia] > [ 1101.490502] [] ? __do_softirq+0x92/0x1d9 > [ 1101.490506] [] ? __do_softirq+0x57/0x1d9 > [ 1101.490509] [] ? get_vtime_delta+0xd/0x59 > [ 1101.490513] [] ? irq_exit+0x51/0xbb > [ 1101.490516] [] ? do_IRQ+0x81/0x97 > [ 1101.490519] [] ? common_interrupt+0x67/0x67 > [ 1101.490521] [] ? tick_broadcast_oneshot_control+0x146/0x166 > [ 1101.490529] [] ? cpuidle_enter_state+0x43/0xa6 > [ 1101.490532] [] ? cpuidle_enter_state+0x3c/0xa6 > [ 1101.490535] [] ? cpuidle_idle_call+0xc3/0x109 > [ 1101.490539] [] ? arch_cpu_idle+0x6/0x17 > [ 1101.490542] [] ? cpu_startup_entry+0xf1/0x170 > [ 1101.490546] [] ? start_kernel+0x3bd/0x3c8 > [ 1101.490549] [] ? repair_env_string+0x57/0x57 > [ 1101.490551] handlers: > [ 1101.490554] [] usb_hcd_irq > [ 1101.490556] Disabling IRQ #21 > > and the output of cat /proc/interrupts: > 0: 2963382 0 IO-APIC-edge timer > 1: 3 0 IO-APIC-edge i8042 > 7: 1 0 IO-APIC-edge > > 8: 1 0 IO-APIC-edge rtc0 > 9: 0 0 IO-APIC-fasteoi acpi > 12: 4 0 IO-APIC-edge i8042 > 20: 427944 0 IO-APIC-fasteoi ehci_hcd:usb2, eth0 > 21: 21 0 IO-APIC-fasteoi ohci_hcd:usb4 > 22: 2 0 IO-APIC-fasteoi ehci_hcd:usb1 > 23: 3193 0 IO-APIC-fasteoi ohci_hcd:usb3, snd_hda_intel > 40: 379222 0 PCI-MSI-edge ahci > 41: 2658415 0 PCI-MSI-edge nvidia > NMI: 3636 547 Non-maskable interrupts > LOC: 1016378 1512153 Local timer interrupts > SPU: 0 0 Spurious interrupts > PMI: 3636 547 Performance monitoring interrupts > IWI: 81542 67824 IRQ work interrupts > RTR: 0 0 APIC ICR read retries > RES: 24020 59830 Rescheduling interrupts > CAL: 64 103 Function call interrupts > TLB: 6505 1455 TLB shootdowns > ERR: 1 > MIS: 0 > > output of lspci: > lspci > 00:00.0 Host bridge: NVIDIA Corporation MCP79 Host Bridge (rev b1) > 00:00.1 RAM memory: NVIDIA Corporation MCP79 Memory Controller (rev b1) > 00:03.0 ISA bridge: NVIDIA Corporation MCP79 LPC Bridge (rev b2) > 00:03.1 RAM memory: NVIDIA Corporation MCP79 Memory Controller (rev b1) > 00:03.2 SMBus: NVIDIA Corporation MCP79 SMBus (rev b1) > 00:03.3 RAM memory: NVIDIA Corporation MCP79 Memory Controller (rev b1) > 00:03.4 RAM memory: NVIDIA Corporation Device 0a98 (rev b1) > 00:03.5 Co-processor: NVIDIA Corporation MCP79 Co-processor (rev b1) > 00:04.0 USB controller: NVIDIA Corporation MCP79 OHCI USB 1.1 Controller > (rev b1) > 00:04.1 USB controller: NVIDIA Corporation MCP79 EHCI USB 2.0 Controller > (rev b1) > 00:06.0 USB controller: NVIDIA Corporation MCP79 OHCI USB 1.1 Controller > (rev b1) > 00:06.1 USB controller: NVIDIA Corporation MCP79 EHCI USB 2.0 Controller > (rev b1) > 00:08.0 Audio device: NVIDIA Corporation MCP79 High Definition Audio (rev b1) > 00:09.0 PCI bridge: NVIDIA Corporation MCP79 PCI Bridge (rev b1) > 00:0a.0 Ethernet controller: NVIDIA Corporation MCP79 Ethernet (rev b1) > 00:0b.0 SATA controller: NVIDIA Corporation MCP79 AHCI Controller (rev b1) > 00:0c.0 PCI bridge: NVIDIA Corporation MCP79 PCI Express Bridge (rev b1) > 00:10.0 PCI bridge: NVIDIA Corporation MCP79 PCI Express Bridge (rev b1) > 00:15.0 PCI bridge: NVIDIA Corporation MCP79 PCI Express Bridge (rev b1) > 03:00.0 VGA compatible controller: NVIDIA Corporation C79 [GeForce 9300 / > nForce 730i] (rev b1) > > > output of lsusb -v > Bus 003 Device 002: ID 046d:c517 Logitech, Inc. LX710 Cordless Desktop Laser > Device Descriptor: > bLength18 > bDescriptorType 1 > bcdUSB 1.10 > bDeviceClass0 (Defined at Interface level) > bDeviceSubClass 0 > bDeviceProtocol 0 > bMaxPacketSize0 8 > idVendor 0x046d Logitech, Inc. > idProduct 0xc517 L
Re: EG20T USB Gadget Performance
On Tue, Jan 14, 2014 at 9:26 PM, Alan Ott wrote: > On 01/14/2014 09:13 AM, Felipe Balbi wrote: >> >> On Mon, Jan 13, 2014 at 11:25:16PM -0500, Alan Ott wrote: >>> >>> On 01/13/2014 09:01 PM, Alan Stern wrote: On Mon, 13 Jan 2014, Felipe Balbi wrote: > On Mon, Jan 13, 2014 at 03:20:31PM -0500, Alan Ott wrote: >> >> I have an EG20T-based board and have some issues with performance on >> the USB device interface. >> >> >> I'll shoot in the dark here and assume current pch_udc only starts >> request N after N-1 has been givenback, and that's probably what's >> causing the extra delay. > > > Yes, that's what it looks like from my analysis. The DMAs work on the > transfer level though (one DMA per USB transfer), so making larger transfers > increases the performance significantly. > > >> Just as a debugging effort, can you move the call to giveback to a >> workqueue or something like that just so it gets scheduled into the >> future ? This wouldn't be an acceptable patch, but just to see if my >> statement is valid. Generally this delay caused due to processing rather than the pdc driver. I blv in this case checking of data and memset could be the cause for delay. Had tried workqueue for a different PDC driver but didnt change things because mostly completion is necessary before taking the next action. > > > Do you mean move the starting of the next transfer (after a transfer > completes) to a workqueue to delay it further? (for testing of course). > > > Alan. > > -- > 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 -- 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: EG20T USB Gadget Performance
Hi
I guess it is something to do with the buffer size of the gadget
driver. Could you please try change the buffer size to 16K and confirm
if the delay is shifting ? In this case your delay should be after 31
transfers...
===
66 static struct usb_zero_options gzero_options = {
67 .isoc_interval = 4,
68 .isoc_maxpacket = 1024,
69 .bulk_buflen = 4096, => change to 16K
70 .qlen = 32,
71 };
===
Rajaram
On Tue, Jan 14, 2014 at 1:50 AM, Alan Ott wrote:
> Hi all,
>
> I have an EG20T-based board and have some issues with performance on the USB
> device interface.
>
> I made a libusb test program (using the async interface)[0] to read data
> from the EG20T's USB device port which has the gadget zero source/sink
> function bound. In theory, one would hope this would give the fastest
> real-world results for the hardware connected.
>
> The test program submits 32 IN transfers and re-submits on transfer
> completion, counting received packets.
>
> From running my test program for a few minutes I get the following:
> elapsed: 548.468416 seconds
> packets: 21503334
> packets/sec: 39206.148199
> bytes/sec: 20073547.877732
> MBit/sec: 160.588383
>
> 160MBit/sec isn't terrible, but I hoped for better. A USB analyzer shows 7
> transactions happening quickly (with about 14us separating them), but every
> 8th transaction, the EG20T will NAK between 20-80 times[1], losing
> 50-100us[2].
>
> This delay happens every 8th transaction without fail[3].
>
> I've looked at the following:
> 1. The f_sourcesink.c function it queues up 8 responses at the beginning.
> Changing this number up or down had no effect.
> 2. Analysis of pch_udc.c doesn't show anything which would obviously cause a
> delay every 8th packet.
> 3. f_eem seems to have roughly the same performance with ping -f -s 64000
> (160Mbit/sec).
>
> The CPU load of the gadget-side Atom PC sits very close to zero.
>
> System Details:
> Linux 3.13.0-rc7 (With a defconfig from Yocto for Intel Crownbay)
> Intel Atom E680 with EG20T
>
> I seem to have eliminated everything on the host side, since the host is
> asking for data, and the device is saying it doesn't have any for up to
> 100us at a time.
>
> What am I missing?
>
> Alan.
>
> [0] http://www.signal11.us/~alan/recv.c
> [1] The number of NAKs not important and doesn't correlate to the amount of
> time spent NAK-ing.
> [2] Analyzer screenshots:
> http://www.signal11.us/~alan/nak.png
> http://www.signal11.us/~alan/nak_open.png
> [3] Often it looks like it's aligned to a SOF packet, but that's only
> because when you delay 100us, the odds are good you will be NAK-ing when a
> SOF packet arrives (every 125us). Sometimes the NAK-ing will start before
> the SOF and sometimes after.
>
> --
> 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
--
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: Linux USB power delivery
On Thu, Aug 8, 2013 at 7:55 PM, Sarah Sharp wrote: > I've heard there's little to do at the software level, but I haven't > been following the spec closely. As I see it at least there has to be some framework that needs to support USB PD specific class requests and also some framework to manage USB-PD (may not be part of usb drivers). > Are you volunteering to add support? > Sure > Sarah Sharp > > On Thu, Aug 08, 2013 at 06:16:54PM +0530, Rajaram R wrote: >> Hi All >> >> Can someone share thoughts if there is a work in progress or plan on >> USB power delivery support for Linux ? >> >> Rajaram -- 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
Linux USB power delivery
Hi All Can someone share thoughts if there is a work in progress or plan on USB power delivery support for Linux ? Rajaram -- 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: Kernel panic with Gadget CDC/ECM
On Wed, Jul 24, 2013 at 7:08 AM, tong li wrote: > Hello, > > I run our board(not a common one) as a Gadget CDC/ECM device with > linux-3.0.39. > At the outset it is running normally when I plug it into > HostPC(Ubuntu12.04),I test it with iperf. > But if I leave it connected for a long time(actually not certain), > I unplug/plug the usb cable again I get the kernel panic. > And it happened infrequently which made me difficult to find the real reason. > Has anyone else seen this or know what could be causing it? During 3.0 PM framework changes got integrated and it was work in progress.That could be a reason. What is the controller driver below ? As Greg said you might want to go for some newer stable kernel. > > Best regards > > LiTong > > [ 590.481517] g_ether.0 gadget: high speed config #1: CDC Ethernet (ECM) > [ 2870.342555] g_ether.0 gadget: high speed config #1: CDC Ethernet (ECM) > [ 2871.535677] [ cut here ] > [ 2871.540373] WARNING: at /work/linux-3.0.39/net/core/dev.c:2925 > net_tx_action+0x6c/0x1a8() > [ 2871.548872] Modules linked in: g_ether > [ 2871.552776] Backtrace: > [ 2871.555318] [<80032b04>] (dump_backtrace+0x0/0x10c) from > [<80295c4c>] (dump_stack+0x18/0x1c) > [ 2871.563898] r6:80331ebf r5:0b6d r4: r3:bf832000 > [ 2871.569671] [<80295c34>] (dump_stack+0x0/0x1c) from [<8004a6ac>] > (warn_slowpath_common+0x54/0x6c) > [ 2871.578706] [<8004a658>] (warn_slowpath_common+0x0/0x6c) from > [<8004a6e8>] (warn_slowpath_null+0x24/0x2c) > [ 2871.588402] r8: r7:00b69000 r6:80b92080 r5:80029080 r4:bf039f00 > [ 2871.595095] r3:0009 > [ 2871.597773] [<8004a6c4>] (warn_slowpath_null+0x0/0x2c) from > [<80212300>] (net_tx_action+0x6c/0x1a8) > [ 2871.606967] [<80212294>] (net_tx_action+0x0/0x1a8) from > [<8004fd04>] (__do_softirq+0xb4/0x15c) > [ 2871.615707] r8:000a r7:0101 r6:80356048 r5:0001 r4:bf832000 > [ 2871.622283] r3:80212294 > [ 2871.625048] [<8004fc50>] (__do_softirq+0x0/0x15c) from [<8004fe78>] > (run_ksoftirqd+0xcc/0x1ec) > [ 2871.633833] [<8004fdac>] (run_ksoftirqd+0x0/0x1ec) from > [<80063914>] (kthread+0x90/0x98) > [ 2871.641960] r7:0013 r6:8004fdac r5: r4:bf82ff20 > [ 2871.647925] [<80063884>] (kthread+0x0/0x98) from [<8004d7c4>] > (do_exit+0x0/0x64c) > [ 2871.667897] r6:8004d7c4 r5:80063884 r4:bf82ff20 > [ 2871.672651] ---[ end trace 3f7079ce04858111 ]--- > [ 2871.682535] [ cut here ] > [ 2871.687242] WARNING: at /work/linux-3.0.39-B/net/core/dev.c:2925 > net_tx_action+0x6c/0x1a8() > [ 2871.695721] Modules linked in: g_ether > [ 2871.699505] Backtrace: > [ 2871.702066] [<80032b04>] (dump_backtrace+0x0/0x10c) from > [<80295c4c>] (dump_stack+0x18/0x1c) > [ 2871.710642] r6:80331ebf r5:0b6d r4: r3:bf8fa000 > [ 2871.716504] [<80295c34>] (dump_stack+0x0/0x1c) from [<8004a6ac>] > (warn_slowpath_common+0x54/0x6c) > [ 2871.725523] [<8004a658>] (warn_slowpath_common+0x0/0x6c) from > [<8004a6e8>] (warn_slowpath_null+0x24/0x2c) > [ 2871.735226] r8:bf107480 r7:00b69000 r6:80b92080 r5:80029080 r4:bf107a20 > [ 2871.741801] r3:0009 > [ 2871.744585] [<8004a6c4>] (warn_slowpath_null+0x0/0x2c) from > [<80212300>] (net_tx_action+0x6c/0x1a8) > [ 2871.753810] [<80212294>] (net_tx_action+0x0/0x1a8) from > [<8004fd04>] (__do_softirq+0xb4/0x15c) > [ 2871.762650] r8:000a r7:0103 r6:80356048 r5:0081 r4:bf8fa000 > [ 2871.769232] r3:80212294 > [ 2871.771890] [<8004fc50>] (__do_softirq+0x0/0x15c) from [<80050224>] > (irq_exit+0x50/0xac) > [ 2871.780155] [<800501d4>] (irq_exit+0x0/0xac) from [<8002a298>] > (do_local_timer+0x64/0x94) > [ 2871.788601] r4: r3:029c > [ 2871.792978] [<8002a234>] (do_local_timer+0x0/0x94) from > [<8002f60c>] (__irq_svc+0x4c/0xe0) > [ 2871.805491] Unable to handle kernel paging request at virtual > address 00100104 > [ 2871.812744] pgd = 80004000 > [ 2871.815457] [00100104] *pgd= > [ 2871.819056] Internal error: Oops: 817 [#1] PREEMPT SMP > [ 2871.824210] Modules linked in: g_ether > [ 2871.827985] CPU: 0Tainted: GW(3.0.39 #50) > [ 2871.833444] PC is at gether_disconnect+0x108/0x1a0 [g_ether] > [ 2871.839144] LR is at _raw_spin_lock+0x10/0x14 > [ 2871.843520] pc : [<7f003a68>]lr : [<8029a8ac>]psr: 8193 > [ 2871.843530] sp : bf8fba90 ip : 00200200 fp : bf8fbaac > [ 2871.855038] r10: r9 : bf8fa000 r8 : bf9b0400 > [ 2871.860279] r7 : beca0238 r6 : bf9fc3b4 r5 : bf970b40 r4 : bf9fc3a0 > [ 2871.866825] r3 : 00200200 r2 : 00100100 r1 : 0001 r0 : bf9fc3b4 > [ 2871.873373] Flags: Nzcv IRQs off FIQs on Mode SVC_32 ISA ARM > Segment kernel > [ 2871.880797] Control: 10c5387d Table: 7f0d004a DAC: 0015 > [ 2871.886563] Process kworker/0:1 (pid: 17, stack limit = 0xbf8fa2f0) > [ 2871.892850] Stack: (0xbf8fba90 to 0xbf8fc000) > [ 2871.897225] ba80: bf970b40 > bf0fff60 0001 > [ 2871.905436] baa0: bf8fbac4 bf8fbab0 7f003cc0 7f00396c 7f003c9c > bf
Re: [RFC] ux500 dma & short transfers on MUSB
On Fri, Jul 19, 2013 at 2:51 PM, Sebastian Andrzej Siewior wrote: > On 07/19/2013 11:16 AM, Rajaram R wrote: >> That was work in progress. You may wish to look at this patch >> http://permalink.gmane.org/gmane.linux.usb.general/79858 >> >> If a device sends 513 bytes to a musb device. 512 bytes will be >> received in DMA mode and 1 byte in FIFO mode. >> >> FIFO will always handle data < 512. > > Okay. Let me rephrase this: Lets say there is a read request for 4096 > bytes and your maxpacket is 512 and total of 4096 bytes will be sent by > the device. How many interrupts / read requests will there be? > 8 like every maxpacket size or is it possible to stuff more data and > transfer 4096 in one go? Not 100% sure, but as i remember it will be 8 times in the present driver (atleast host mode). But yes we can do 4096 at a time. > >>>> Cheers >>> > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] ux500 dma & short transfers on MUSB
On Fri, Jul 19, 2013 at 2:23 PM, Sebastian Andrzej Siewior wrote: > On 07/19/2013 10:26 AM, Rajaram R wrote: >> We program the DMA only when we receive RX interrupt and when the >> length of data is known. When you submit URB for RX the flow would be >> something like musb_start_urb==>musb_ep_program, Note here we do not >> program the DMA. DMA is programmed musb_host_rx using >> "channel_program" callback. Note here we pass the length of data >> received. >> >> Now in ux500_dma file 'ux500_dma_is_compatible' fails the dma >> operation and the driver switches to FIFO mode in the same function >> (musb_host_rx). > > Okay. This is completely different from what I expected. That means > while the URB is submitting you only program the musb controller for > receive and nothing else. The next interrupt will notify that the > musb's endpoint fifo is filled with X bytes and you program the dma > engine to transfer X bytes. And then you wait for another interrupt > which says DMA transfer completed. So in that case you don't have that > problem I though you do. Thank you for clearing that up. > > ux500_dma_is_compatible() is testing "length < 512" and "length & 3". > This looks like the fifo can be filled with more than 512 bytes. Do you > know on top of your head how much that can be? That was work in progress. You may wish to look at this patch http://permalink.gmane.org/gmane.linux.usb.general/79858 If a device sends 513 bytes to a musb device. 512 bytes will be received in DMA mode and 1 byte in FIFO mode. FIFO will always handle data < 512. > >> >> >> Cheers > > Sebastian > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] ux500 dma & short transfers on MUSB
On Fri, Jul 19, 2013 at 1:36 PM, Sebastian Andrzej Siewior wrote: > On 07/19/2013 09:59 AM, Rajaram R wrote: >>> Okay. musb offloads the actual transfer to the DMA engine it is using. >>> Once it does so, it relies on whatever comes back from dma engine >>> regarding transfer complete, transferred size etc. >> >> AFAIK ux500 musb dma code handles data which is multiple of max packet >> size in DMA. 1 byte should be in PIO mode. Which version of kernel you >> are using ? > > I am looking at v3.11-rc1 right now and I don't have the HW. > > As I said: The URB scheduled for receive is 256 bytes in size, max > packet size is 64. So DMA should be chosen, right? > The UART on other side sends just one byte so the DMA receives just one > byte regardless what has been requested. > My question is how musb gets notified of this one byte. It might happen > someplace but I don't see it. We program the DMA only when we receive RX interrupt and when the length of data is known. When you submit URB for RX the flow would be something like musb_start_urb==>musb_ep_program, Note here we do not program the DMA. DMA is programmed musb_host_rx using "channel_program" callback. Note here we pass the length of data received. Now in ux500_dma file 'ux500_dma_is_compatible' fails the dma operation and the driver switches to FIFO mode in the same function (musb_host_rx). Cheers > > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] ux500 dma & short transfers on MUSB
On Thu, Jul 11, 2013 at 10:44 PM, Sebastian Andrzej Siewior wrote: > > On 07/11/2013 06:58 PM, Greg KH wrote: > >> following scenario: > >> you attach an UART-TO-USB adapter to your musb port running ux500-dma > >> code. The USB UARt driver queues 1x RX URB with the size of 256 bytes > >> (example) and the max packet size is 64. > >> > >> The other side sends only one byte because it is mean. > > > > That's actually quite common, why is it "mean"? > > Hehe. Because it is the unexpected one. common depends on the use case. > Mass storage doesn't do this. Not sure if network notices the > difference, maybe ncm. > > >> Now, the way I understand it is, you tell musb that the complete > >> transfer of 256 bytes has ended instead one byte that really > >> happened. Is my assumption wrong? > > > > What do you mean by "tell musb"? Of course the transfer has completed, > > that's all the device sent to the host controller, so it has to complete > > the transfer and send that on up to the driver that requested the urb. > > > > I don't understand the question/problem you are asking here, care to be > > more descriptive? > > Okay. musb offloads the actual transfer to the DMA engine it is using. > Once it does so, it relies on whatever comes back from dma engine > regarding transfer complete, transferred size etc. AFAIK ux500 musb dma code handles data which is multiple of max packet size in DMA. 1 byte should be in PIO mode. Which version of kernel you are using ? > > In case of ux500-dma (as far as I can tell) musb forwards the RX > request to the DMA engine, which will receive one byte instead of the > requested 256bytes. Since the DMA engine did not inform musb about the > correct transfer size, musb will complete that URB with 256 bytes. > > If you take a look on ux500_dma_callback() you will see the line: >ux500_channel->channel.actual_len = ux500_channel->cur_len; > > ->actual_len is what musb thinks got transferred. ->cur_len is what > musb asked to transfer. I don't see where the case of a shorter > transfer is considered. Again I have no HW I was just browsing. > > > thanks, > > > > greg k-h > > Sebastian > -- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: ux500: fix 'musbid' undeclared error in ux500_remove()
On Wed, Oct 31, 2012 at 8:00 PM, Felipe Balbi wrote:
> Hi,
>
> On Wed, Oct 31, 2012 at 07:45:53PM +0530, Rajaram R wrote:
>> On Tue, Oct 23, 2012 at 11:42 AM, Wei Yongjun wrote:
>> > commit 65b3d52d02a558fbfe08e43688e15390c5ab3067
>> > (usb: musb: add musb_ida for multi instance support)
>> > used musbid in ux500_remove() but nerver declared it.
>>
>> The above message doesnot match with your fix. You say ux500_remove is
>> using musbhid but build error is in probe.
>>
>> I also dont see anywhere in the code we use this variable musbid.
>
> -ECONFUSED:
:-) No.. This patch could have been avoided if a build test was done
when the original patch pushed this change... !!
Anyway I can send a patch correcting the text if required...
>
> static int __devinit ux500_probe(struct platform_device *pdev)
> {
> struct musb_hdrc_platform_data *pdata = pdev->dev.platform_data;
> struct platform_device *musb;
> struct ux500_glue *glue;
> struct clk *clk;
> int musbid;
> int ret = -ENOMEM;
>
> glue = kzalloc(sizeof(*glue), GFP_KERNEL);
> if (!glue) {
> dev_err(&pdev->dev, "failed to allocate glue context\n");
> goto err0;
> }
>
> /* get the musb id */
> musbid = musb_get_id(&pdev->dev, GFP_KERNEL);
> if (musbid < 0) {
> dev_err(&pdev->dev, "failed to allocate musb id\n");
> ret = -ENOMEM;
> goto err1;
> }
>
> musb = platform_device_alloc("musb-hdrc", musbid);
> if (!musb) {
> dev_err(&pdev->dev, "failed to allocate musb device\n");
> goto err2;
> }
>
> clk = clk_get(&pdev->dev, "usb");
> if (IS_ERR(clk)) {
> dev_err(&pdev->dev, "failed to get clock\n");
> ret = PTR_ERR(clk);
> goto err3;
> }
>
> ret = clk_enable(clk);
> if (ret) {
> dev_err(&pdev->dev, "failed to enable clock\n");
> goto err4;
> }
>
> musb->id= musbid;
> musb->dev.parent= &pdev->dev;
> musb->dev.dma_mask = pdev->dev.dma_mask;
> musb->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
>
> glue->dev = &pdev->dev;
> glue->musb = musb;
> glue->clk = clk;
>
> pdata->platform_ops = &ux500_ops;
>
> platform_set_drvdata(pdev, glue);
>
> ret = platform_device_add_resources(musb, pdev->resource,
> pdev->num_resources);
> if (ret) {
> dev_err(&pdev->dev, "failed to add resources\n");
> goto err5;
> }
>
> ret = platform_device_add_data(musb, pdata, sizeof(*pdata));
> if (ret) {
> dev_err(&pdev->dev, "failed to add platform_data\n");
> goto err5;
> }
>
> ret = platform_device_add(musb);
> if (ret) {
> dev_err(&pdev->dev, "failed to register musb device\n");
> goto err5;
> }
>
> return 0;
>
> err5:
> clk_disable(clk);
>
> err4:
> clk_put(clk);
>
> err3:
> platform_device_put(musb);
>
> err2:
> musb_put_id(&pdev->dev, musbid);
>
> err1:
> kfree(glue);
>
> err0:
> return ret;
> }
>
> --
> 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: [GIT PULL] USB fixes for v3.7-rc4
Hi Felipie/Greg
On Tue, Oct 30, 2012 at 6:17 PM, Felipe Balbi wrote:
> Hi Greg,
>
> Judging by how things are going, this is likely to be my last pull request
> for v3.7-rc cycle, unless someone finds a big regression which we really must
> fix during v3.7-rc.
>
> It's rather small; just a revert, a Kconfig change and the addition of a
> variable to a function. In fact, it's so small that I've added the full
> combined diff below FYI.
>
> Let me know if you want me to change anything.
>
> cheers
>
> The following changes since commit 1cb60156defa4f23d5318ea1ddd400f25b2d0ce5:
>
> usb: renesas_usbhs: fixup dma transfer stall (2012-10-23 09:44:37 +0300)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
> tags/fixes-for-v3.7-rc4
>
> for you to fetch changes up to f6bc8c29383456b89ac1b6f1aa768d195670fb53:
>
> usb: otg: Fix build errors if USB_MUSB_OMAP2PLUS is selected as module
> (2012-10-30 14:37:07 +0200)
>
>
> usb: fixes for v3.7-rc4
>
> We're reverting MUSB Mode 1 DMA patch which caused many regressions. Meanwhile
> Roger is cooking a better version of that patch, which will hopefully be ready
> for v3.8 merge window.
>
> We also fix an undeclared error in ux5000_remove() and another build error
> when we try to build USB_MUSB_OMAP2PLUS as a module.
>
>
> Felipe Balbi (1):
> Revert "usb: musb: use DMA mode 1 whenever possible"
>
> Roger Quadros (1):
> usb: otg: Fix build errors if USB_MUSB_OMAP2PLUS is selected as module
>
> Wei Yongjun (1):
> usb: musb: ux500: fix 'musbid' undeclared error in ux500_remove()
This patch is incorrect. Please dont merge this patch.
>
> drivers/usb/musb/musb_gadget.c | 30 ++
> drivers/usb/musb/ux500.c | 2 +-
> drivers/usb/otg/Kconfig| 4 ++--
> 3 files changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index d0b87e7..b6b84da 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -707,11 +707,12 @@ static void rxstate(struct musb *musb, struct
> musb_request *req)
> fifo_count = musb_readw(epio, MUSB_RXCOUNT);
>
> /*
> -* use mode 1 only if we expect data of at least ep packet_sz
> -* and have not yet received a short packet
> +* Enable Mode 1 on RX transfers only when short_not_ok flag
> +* is set. Currently short_not_ok flag is set only from
> +* file_storage and f_mass_storage drivers
> */
> - if ((request->length - request->actual >= musb_ep->packet_sz)
> &&
> - (fifo_count >= musb_ep->packet_sz))
> +
> + if (request->short_not_ok && fifo_count == musb_ep->packet_sz)
> use_mode_1 = 1;
> else
> use_mode_1 = 0;
> @@ -727,6 +728,27 @@ static void rxstate(struct musb *musb, struct
> musb_request *req)
> c = musb->dma_controller;
> channel = musb_ep->dma;
>
> + /* We use DMA Req mode 0 in rx_csr, and DMA controller operates in
> +* mode 0 only. So we do not get endpoint interrupts due to DMA
> +* completion. We only get interrupts from DMA controller.
> +*
> +* We could operate in DMA mode 1 if we knew the size of the tranfer
> +* in advance. For mass storage class, request->length = what the host
> +* sends, so that'd work. But for pretty much everything else,
> +* request->length is routinely more than what the host sends. For
> +* most these gadgets, end of is signified either by a short packet,
> +* or filling the last byte of the buffer. (Sending extra data in
> +* that last pckate should trigger an overflow fault.) But in mode 1,
> +* we don't get DMA completion interrupt for short packets.
> +*
> +* Theoretically, we could enable DMAReq irq (MUSB_RXCSR_DMAMODE = 1),
> +* to get endpoint interrupt on every DMA req, but that didn't seem
> +* to work reliably.
> +*
> +* REVISIT an updated g_file_storage can set req->short_not_ok, which
> +* then becomes usable as a runtime "use mode 1" hint...
> +*/
> +
> /* Experimental: Mode1 works with mass
> storage use cases */
> if (use_mode_1) {
> csr |= MUSB_RXCSR_AUTOCLEAR;
> diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
> index d62a91f..0e62f50 100644
> --- a/drivers/usb/musb/ux500.c
> +++ b/drivers/usb/musb/ux500.c
> @@ -65,7 +65,7 @@ static int __devinit ux500_probe(struct platf
Re: [PATCH] usb: musb: ux500: fix 'musbid' undeclared error in ux500_remove()
Hi On Tue, Oct 23, 2012 at 11:42 AM, Wei Yongjun wrote: > commit 65b3d52d02a558fbfe08e43688e15390c5ab3067 > (usb: musb: add musb_ida for multi instance support) > used musbid in ux500_remove() but nerver declared it. The above message doesnot match with your fix. You say ux500_remove is using musbhid but build error is in probe. I also dont see anywhere in the code we use this variable musbid. > > I found this in x86_64 platform, but not sure whether > this is a error on the correct ARCH. > > $ make drivers/usb/musb/ux500.o > make[1]: Nothing to be done for `all'. > make[1]: Nothing to be done for `relocs'. > CHK include/generated/uapi/linux/version.h > CHK include/generated/utsrelease.h > UPD include/generated/utsrelease.h > CALLscripts/checksyscalls.sh > CC drivers/usb/musb/ux500.o > drivers/usb/musb/ux500.c: In function 'ux500_probe': > drivers/usb/musb/ux500.c:78:2: error: 'musbid' undeclared (first use in this > function) > drivers/usb/musb/ux500.c:78:2: note: each undeclared identifier is reported > only once for each function it appears in > make[1]: *** [drivers/usb/musb/ux500.o] Error 1 > make: *** [drivers/usb/musb/ux500.o] Error 2 > > Signed-off-by: Wei Yongjun > --- > drivers/usb/musb/ux500.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c > index d62a91f..0e62f50 100644 > --- a/drivers/usb/musb/ux500.c > +++ b/drivers/usb/musb/ux500.c > @@ -65,7 +65,7 @@ static int __devinit ux500_probe(struct platform_device > *pdev) > struct platform_device *musb; > struct ux500_glue *glue; > struct clk *clk; > - > + int musbid; > int ret = -ENOMEM; > > glue = kzalloc(sizeof(*glue), GFP_KERNEL); > -- > 1.7.11.7 > > > -- > 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 -- 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: set hub's default autosuspend delay as 0
On Thu, Sep 20, 2012 at 9:38 PM, Ming Lei wrote: > > On Thu, Sep 20, 2012 at 11:23 PM, Ming Lei wrote: > > > > I will try to study 'lsusb' to see if there is one problem and try to > > solve it. > > If what you concern is about accessing device via libusb, it should be > OK since the device will be resumed in its open() and suspended in > its close(). > > Could you let me know if there is other thing which may cause 'lsusb' > mess if we change hub's autosuspend delay as 0? Did you try a fast connect and disconnect of a USB Stick( Mass Storage) ? Does it enumerate/mount properly everytime? > > Thanks > -- > Ming Lei > -- > 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 -- 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: gadget: Add Interface Association Descriptor to ECM
On Sat, Sep 8, 2012 at 3:34 AM, Linus Walleij wrote:
>
> On Fri, Sep 7, 2012 at 8:51 PM, Sebastian Andrzej Siewior
> wrote:
> > On Wed, Jan 25, 2012 at 11:02:03AM +0100, Linus Walleij wrote:
>
> >> static struct usb_descriptor_header *ecm_fs_function[] = {
> >> /* CDC ECM control descriptors */
> >> + (struct usb_descriptor_header *) &ecm_iad_descriptor,
> >> (struct usb_descriptor_header *) &ecm_control_intf,
> >> (struct usb_descriptor_header *) &ecm_header_desc,
> >> (struct usb_descriptor_header *) &ecm_union_desc,
> >> @@ -247,6 +262,7 @@ static struct usb_endpoint_descriptor
> >> hs_ecm_out_desc = {
> >>
> >> static struct usb_descriptor_header *ecm_hs_function[] = {
> >> /* CDC ECM control descriptors */
> >> + (struct usb_descriptor_header *) &ecm_iad_descriptor,
> >> (struct usb_descriptor_header *) &ecm_control_intf,
> >> (struct usb_descriptor_header *) &ecm_header_desc,
> >> (struct usb_descriptor_header *) &ecm_union_desc,
> >
> > Why did you add this IAD thingy to FS and HS and not SS descriptors? I
> > don't
> > see a reason so I would guess you just forgot :)
>
> Argh, Praveena, save me here ... I was being patch secretary
> this time, but to my untrained USB-eyes it looks like you're right,
> interested in patching it?
This patch was generated based on Kernel 3.0 and then Super Speed was
not fully into gadget framework!
>
> Yours,
> Linus Walleij
> --
> 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
--
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: musb: gadget: implement proper ->reset handling
On Thu, Aug 16, 2012 at 11:51 AM, Felipe Balbi wrote:
> After introducing the new ->reset method on
> struct usb_gadget_driver, UDC drivers are
> required to implement proper handling for it.
>
> This patch adds proper ->reset handling for
> musb driver.
>
> Signed-off-by: Felipe Balbi
> ---
> drivers/usb/musb/musb_gadget.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index d0b87e7..30fca10 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -2163,9 +2163,16 @@ __acquires(musb->lock)
> : NULL
> );
>
> - /* report disconnect, if we didn't already (flushing EP state) */
> - if (musb->g.speed != USB_SPEED_UNKNOWN)
> - musb_g_disconnect(musb);
> + /* report reset, if we didn't already (flushing EP state) */
> + if (musb->gadget_driver && musb->gadget_driver->reset) {
> + spin_unlock(&musb->lock);
> + musb->gadget_driver->reset(&musb->g);
What will this call back do ?
> + spin_lock(&musb->lock);
> + } else if (musb->gadget_driver && musb->gadget_driver->disconnect) {
> + spin_unlock(&musb->lock);
> + musb->gadget_driver->disconnect(&musb->g);
> + spin_lock(&musb->lock);
> + }
>
> /* clear HR */
> else if (devctl & MUSB_DEVCTL_HR)
> --
> 1.7.12.rc2
>
> --
> 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
--
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: musb: use DMA mode 1 whenever possible
On Tue, Aug 7, 2012 at 6:39 PM, Roger Quadros wrote:
> Do not rely on any hints from gadget drivers and use DMA mode 1
> whenever we expect data of at least the endpoint's packet size and
> have not yet received a short packet.
Could you please let us know what all combination this was tested ?
What will happen if the request length is 513 ?
>
> The last packet if short is always transferred using DMA mode 0.
>
> This patch fixes USB throughput issues in mass storage mode for
> host to device transfers.
>
> Signed-off-by: Roger Quadros
> ---
> drivers/usb/musb/musb_gadget.c | 30 --
> 1 files changed, 4 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index 8d2cce1..5c4392b 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -707,12 +707,11 @@ static void rxstate(struct musb *musb, struct
> musb_request *req)
> fifo_count = musb_readw(epio, MUSB_RXCOUNT);
>
> /*
> -* Enable Mode 1 on RX transfers only when short_not_ok flag
> -* is set. Currently short_not_ok flag is set only from
> -* file_storage and f_mass_storage drivers
> +* use mode 1 only if we expect data of at least ep packet_sz
> +* and have not yet received a short packet
> */
> -
> - if (request->short_not_ok && fifo_count == musb_ep->packet_sz)
> + if ((request->length - request->actual >= musb_ep->packet_sz)
> &&
> + (fifo_count >= musb_ep->packet_sz))
> use_mode_1 = 1;
> else
> use_mode_1 = 0;
> @@ -727,27 +726,6 @@ static void rxstate(struct musb *musb, struct
> musb_request *req)
> c = musb->dma_controller;
> channel = musb_ep->dma;
>
> - /* We use DMA Req mode 0 in rx_csr, and DMA controller operates in
> -* mode 0 only. So we do not get endpoint interrupts due to DMA
> -* completion. We only get interrupts from DMA controller.
> -*
> -* We could operate in DMA mode 1 if we knew the size of the tranfer
> -* in advance. For mass storage class, request->length = what the host
> -* sends, so that'd work. But for pretty much everything else,
> -* request->length is routinely more than what the host sends. For
> -* most these gadgets, end of is signified either by a short packet,
> -* or filling the last byte of the buffer. (Sending extra data in
> -* that last pckate should trigger an overflow fault.) But in mode 1,
> -* we don't get DMA completion interrupt for short packets.
> -*
> -* Theoretically, we could enable DMAReq irq (MUSB_RXCSR_DMAMODE = 1),
> -* to get endpoint interrupt on every DMA req, but that didn't seem
> -* to work reliably.
> -*
> -* REVISIT an updated g_file_storage can set req->short_not_ok, which
> -* then becomes usable as a runtime "use mode 1" hint...
> -*/
> -
> /* Experimental: Mode1 works with mass
> storage use cases */
> if (use_mode_1) {
> csr |= MUSB_RXCSR_AUTOCLEAR;
> --
> 1.7.4.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 RFC] usb: musb: use DMA mode 1 whenever possible
On Tue, Aug 7, 2012 at 5:26 PM, Rajaram R wrote:
> Hi Robert/Felipie
>
> On Thu, Aug 2, 2012 at 1:41 PM, Roger Quadros wrote:
>>
>> Do not rely on any hints from gadget drivers and use DMA mode 1
>> whenever we expect more data than the endpoint's packet size and
>> have not yet received a short packet.
>>
>> The last packet if short is always transferred using DMA mode 0.
>>
>> This patch fixes USB throughput issues in mass storage mode for
>> host to device transfers.
>>
>> Signed-off-by: Roger Quadros
>> ---
>> drivers/usb/musb/musb_gadget.c | 30 --
>> 1 files changed, 4 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
>> index f7194cf..9c94655 100644
>> --- a/drivers/usb/musb/musb_gadget.c
>> +++ b/drivers/usb/musb/musb_gadget.c
>> @@ -707,12 +707,11 @@ static void rxstate(struct musb *musb, struct
>> musb_request *req)
>> len = musb_readw(epio, MUSB_RXCOUNT);
>>
>> /*
>> -* Enable Mode 1 on RX transfers only when short_not_ok flag
>> -* is set. Currently short_not_ok flag is set only from
>> -* file_storage and f_mass_storage drivers
>> +* use mode 1 only if we expect data larger than ep
>> packet_sz
>> +* and we have not yet received a short packet
>> */
>> -
>> - if (request->short_not_ok && len == musb_ep->packet_sz)
>> + if ((request->length - request->actual > musb_ep->packet_sz)
>> &&
>> + (len >= musb_ep->packet_sz))
>
> If the request length is 512 it can go in mode. Please add that while pushing.
If the request length is 512 it can go in mode1. Please add that while pushing.
>
>
>> use_mode_1 = 1;
>> else
>> use_mode_1 = 0;
>> @@ -727,27 +726,6 @@ static void rxstate(struct musb *musb, struct
>> musb_request *req)
>> c = musb->dma_controller;
>> channel = musb_ep->dma;
>>
>> - /* We use DMA Req mode 0 in rx_csr, and DMA controller operates in
>> -* mode 0 only. So we do not get endpoint interrupts due to DMA
>> -* completion. We only get interrupts from DMA controller.
>> -*
>> -* We could operate in DMA mode 1 if we knew the size of the tranfer
>> -* in advance. For mass storage class, request->length = what the
>> host
>> -* sends, so that'd work. But for pretty much everything else,
>> -* request->length is routinely more than what the host sends. For
>> -* most these gadgets, end of is signified either by a short packet,
>> -* or filling the last byte of the buffer. (Sending extra data in
>> -* that last pckate should trigger an overflow fault.) But in mode
>> 1,
>> -* we don't get DMA completion interrupt for short packets.
>> -*
>> -* Theoretically, we could enable DMAReq irq (MUSB_RXCSR_DMAMODE =
>> 1),
>> -* to get endpoint interrupt on every DMA req, but that didn't seem
>> -* to work reliably.
>> -*
>> -* REVISIT an updated g_file_storage can set req->short_not_ok, which
>> -* then becomes usable as a runtime "use mode 1" hint...
>> -*/
>> -
>> /* Experimental: Mode1 works with mass
>> storage use cases */
>> if (use_mode_1) {
>> csr |= MUSB_RXCSR_AUTOCLEAR;
>> --
>> 1.7.4.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
--
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 RFC] usb: musb: use DMA mode 1 whenever possible
Hi Robert/Felipie
On Thu, Aug 2, 2012 at 1:41 PM, Roger Quadros wrote:
>
> Do not rely on any hints from gadget drivers and use DMA mode 1
> whenever we expect more data than the endpoint's packet size and
> have not yet received a short packet.
>
> The last packet if short is always transferred using DMA mode 0.
>
> This patch fixes USB throughput issues in mass storage mode for
> host to device transfers.
>
> Signed-off-by: Roger Quadros
> ---
> drivers/usb/musb/musb_gadget.c | 30 --
> 1 files changed, 4 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index f7194cf..9c94655 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -707,12 +707,11 @@ static void rxstate(struct musb *musb, struct
> musb_request *req)
> len = musb_readw(epio, MUSB_RXCOUNT);
>
> /*
> -* Enable Mode 1 on RX transfers only when short_not_ok flag
> -* is set. Currently short_not_ok flag is set only from
> -* file_storage and f_mass_storage drivers
> +* use mode 1 only if we expect data larger than ep packet_sz
> +* and we have not yet received a short packet
> */
> -
> - if (request->short_not_ok && len == musb_ep->packet_sz)
> + if ((request->length - request->actual > musb_ep->packet_sz)
> &&
> + (len >= musb_ep->packet_sz))
If the request length is 512 it can go in mode. Please add that while pushing.
> use_mode_1 = 1;
> else
> use_mode_1 = 0;
> @@ -727,27 +726,6 @@ static void rxstate(struct musb *musb, struct
> musb_request *req)
> c = musb->dma_controller;
> channel = musb_ep->dma;
>
> - /* We use DMA Req mode 0 in rx_csr, and DMA controller operates in
> -* mode 0 only. So we do not get endpoint interrupts due to DMA
> -* completion. We only get interrupts from DMA controller.
> -*
> -* We could operate in DMA mode 1 if we knew the size of the tranfer
> -* in advance. For mass storage class, request->length = what the host
> -* sends, so that'd work. But for pretty much everything else,
> -* request->length is routinely more than what the host sends. For
> -* most these gadgets, end of is signified either by a short packet,
> -* or filling the last byte of the buffer. (Sending extra data in
> -* that last pckate should trigger an overflow fault.) But in mode 1,
> -* we don't get DMA completion interrupt for short packets.
> -*
> -* Theoretically, we could enable DMAReq irq (MUSB_RXCSR_DMAMODE = 1),
> -* to get endpoint interrupt on every DMA req, but that didn't seem
> -* to work reliably.
> -*
> -* REVISIT an updated g_file_storage can set req->short_not_ok, which
> -* then becomes usable as a runtime "use mode 1" hint...
> -*/
> -
> /* Experimental: Mode1 works with mass
> storage use cases */
> if (use_mode_1) {
> csr |= MUSB_RXCSR_AUTOCLEAR;
> --
> 1.7.4.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
--
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: (Query): [PATCH v4 2/4] usb gadget: Configure endpoint according to gadget speed.
On Mon, Jul 30, 2012 at 2:57 AM, Alan Stern wrote: > > On Sun, 29 Jul 2012, Rajaram R wrote: > > > > The ep list doesn't belong to the gadget driver; it belongs to the UDC > > > driver. The maxpacket has to be adjusted to match the value stored in > > > the descriptor so that the UDC will tell the hardware to use the right > > > maxpacket value. > > > > The ep list is owned by UDC and thus the function or class should not > > update it. > > This will create problem when we switch functions > > > > For example: > > > > Lets assume the UDC has an interrupt EP in the ep-list with maxpktsize > > say 32. When for the 1st time a class a requires interrupt EP, chooses > > this EP and adjusts the size to say 8. Then when we switch to a > > function that requies an interrupt EP of size say 10. This time though > > the UDC is capable of supporting this requirement, EP autoconf will > > not be successful. > > > > So IMO, classes should not update eplist. This patch has to be > > corrected and pls let me know if I am missing something? > > Ah, I see what you mean. But the problem is even worse than you think, > because several of the UDC drivers adjust ep->maxpacket in this same > way when the endpoint is enabled. OK. In this case, the patch is doing it in composite framework and will affect all UDC drivers irrespective of whether they update or not. Will send out a patch to correct the composite driver initially. > > The documentation in include/linux/usb/gadget.h should be updated to > indicate that the maxpacket value is the largest allowed by the > hardware, not the endpoint's current setting. The same is probably > true for maxburst and mult. > > Alan Stern > -- 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: (Query): [PATCH v4 2/4] usb gadget: Configure endpoint according to gadget speed.
On Thu, Jul 26, 2012 at 10:26 PM, Alan Stern wrote:
> On Thu, 26 Jul 2012, Rajaram R wrote:
>
>> Hi Felipe/Alan
>>
>> Any comments for the below query ?
>>
>> On Thu, Jul 12, 2012 at 4:53 PM, Rajaram R
>> wrote:
>> > Hi
>> >
>> >
>> > On Thu, Nov 18, 2010 at 6:17 PM, Tatyana Brokhman
>> > wrote:
>> >>
>> >> Add config_ep_by_speed() to configure the endpoint according to the gadget
>> >> speed. Using this function will spare the FDs from handling the endpoint
>> >> chosen descriptor.
>> >>
>> >> Signed-off-by: Tatyana Brokhman
>> >> ---
>> >> drivers/usb/gadget/composite.c | 76
>> >> +++
>> >> drivers/usb/gadget/epautoconf.c |1 +
>> >> include/linux/usb/composite.h | 21 +++
>> >> include/linux/usb/gadget.h |3 ++
>> >> 4 files changed, 101 insertions(+), 0 deletions(-)
>> >>
>> > ---cut---
>> >
>> >> + */
>> >> +int config_ep_by_speed(struct usb_gadget *g,
>> >> + struct usb_function *f,
>> >> + struct usb_ep *_ep)
>> >> +{
>
> ...
>
>> >> +ep_found:
>> >> + /* commit results */
>> >> + _ep->maxpacket = le16_to_cpu(chosen_desc->wMaxPacketSize);
>> >> + _ep->desc = chosen_desc;
>>
>> Could you please comment on why do we need to update gadget's ep list
>> with function's maxpacket ? Will this not affect when we switch
>> functions ?
>
> The ep list doesn't belong to the gadget driver; it belongs to the UDC
> driver. The maxpacket has to be adjusted to match the value stored in
> the descriptor so that the UDC will tell the hardware to use the right
> maxpacket value.
The ep list is owned by UDC and thus the function or class should not update it.
This will create problem when we switch functions
For example:
Lets assume the UDC has an interrupt EP in the ep-list with maxpktsize
say 32. When for the 1st time a class a requires interrupt EP, chooses
this EP and adjusts the size to say 8. Then when we switch to a
function that requies an interrupt EP of size say 10. This time though
the UDC is capable of supporting this requirement, EP autoconf will
not be successful.
So IMO, classes should not update eplist. This patch has to be
corrected and pls let me know if I am missing something?
>
> When you switch functions, the new function should call
> config_ep_by_speed().
>
> Alan Stern
>
--
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
(Query): [PATCH v4 2/4] usb gadget: Configure endpoint according to gadget speed.
Hi Felipe/Alan
Any comments for the below query ?
On Thu, Jul 12, 2012 at 4:53 PM, Rajaram R wrote:
> Hi
>
>
> On Thu, Nov 18, 2010 at 6:17 PM, Tatyana Brokhman
> wrote:
>>
>> Add config_ep_by_speed() to configure the endpoint according to the gadget
>> speed. Using this function will spare the FDs from handling the endpoint
>> chosen descriptor.
>>
>> Signed-off-by: Tatyana Brokhman
>> ---
>> drivers/usb/gadget/composite.c | 76
>> +++
>> drivers/usb/gadget/epautoconf.c |1 +
>> include/linux/usb/composite.h | 21 +++
>> include/linux/usb/gadget.h |3 ++
>> 4 files changed, 101 insertions(+), 0 deletions(-)
>>
> ---cut---
>
>> + */
>> +int config_ep_by_speed(struct usb_gadget *g,
>> + struct usb_function *f,
>> + struct usb_ep *_ep)
>> +{
>> + struct usb_endpoint_descriptor *chosen_desc = NULL;
>> + struct usb_descriptor_header **speed_desc = NULL;
>> +
>> + struct usb_descriptor_header **d_spd; /* cursor for speed desc */
>> +
>> + if (!g || !f || !_ep)
>> + return -EIO;
>> +
>> + /* select desired speed */
>> + switch (g->speed) {
>> + case USB_SPEED_HIGH:
>> + if (gadget_is_dualspeed(g)) {
>> + speed_desc = f->hs_descriptors;
>> + break;
>> + }
>> + /* else: fall through */
>> + default:
>> + speed_desc = f->descriptors;
>> + }
>> + /* find descriptors */
>> + for (d_spd = next_ep_desc(speed_desc); d_spd;
>> + d_spd = next_ep_desc(d_spd+1)) {
>> + chosen_desc = (struct usb_endpoint_descriptor *)*d_spd;
>> + if (chosen_desc->bEndpointAddress == _ep->bEndpointAddress)
>> + goto ep_found;
>> + }
>> + return -EIO;
>> +
>> +ep_found:
>> + /* commit results */
>> + _ep->maxpacket = le16_to_cpu(chosen_desc->wMaxPacketSize);
>> + _ep->desc = chosen_desc;
Could you please comment on why do we need to update gadget's ep list
with function's maxpacket ? Will this not affect when we switch
functions ?
>
>> +
>> + return 0;
>> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb : mass storage : short_not_ok for non usb3 udc
On Thu, Jul 26, 2012 at 2:33 AM, Alan Stern wrote: > On Wed, 25 Jul 2012, Paul Zimmerman wrote: > >> > From: Alan Stern [mailto:st...@rowland.harvard.edu] >> > Sent: Wednesday, July 25, 2012 12:48 PM >> > To: Paul Zimmerman >> > Cc: Rajaram R; Michal Nazarewicz; linux-usb@vger.kernel.org >> > Subject: RE: usb : mass storage : short_not_ok for non usb3 udc >> > >> > On Wed, 25 Jul 2012, Paul Zimmerman wrote: >> > >> > > > > > I don't understand this either. What's wrong with setting >> > > > > > short_not_ok while at SuperSpeed? It shouldn't force the use of a >> > > > bounce buffer. >> > > > > > >> > > > > >> > > > > I have just brought back some code removed by patch "usb: fix mass >> > > > > storage gadgets to work with Synopsys UDC". >> > > > >> > > > Maybe while bringing it back you can remove the checks for SuperSpeed. >> > > > Is there any reason to keep them? >> > > >> > > Hi Alan, >> > > >> > > The problem only arises because at super-speed the MaxPacket size is >> > > 1024, which is greater than the sector size of 512. So the whole point of >> > > the patch is to fix the SuperSpeed operation, while allowing high-speed >> > > devices to operate as they always did. >> > >> > Don't the mass-storage drivers already operate correctly at SuperSpeed? >> > If not, what goes wrong? >> > >> > Besides, how could the patch possibly affect SuperSpeed operation? >> > Everything it does is protected by >> > >> > if (!gadget_is_superspeed(gadget)) >> > >> > So I'm afraid I don't understand your point. >> >> Sorry for not being clear. The original patch: >> >> http://www.spinics.net/lists/linux-usb/msg52549.html >> >> was required to make mass-storage work with DWC3 at SuperSpeed. >> However, it apparently broke platforms like musb which depend on >> short-not-ok being set. The current patch fixes that, by setting >> short-not-ok if not running SuperSpeed. DWC3 will still work fine >> with this latest patch, since its problem was only with SuperSpeed. > > Okay, that's fine. But it doesn't answer my earlier question: Is there > really any need for the SuperSpeed checks in this patch? > > In other words, will DWC3 continue to work correctly if short_not_ok is > set? Yes, that was my question earlier to this patch. > > Alan Stern > -- 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 : mass storage : short_not_ok for non usb3 udc
On Wed, Jul 25, 2012 at 10:51 PM, Alan Stern wrote: > On Wed, 25 Jul 2012, Rajaram R wrote: > >> >> > There's one thing I don't get. The message talks about musb but the >> >> > code >> >> > checks for non Super Speed devices. So maybe the code is correct, maybe >> >> > it's not, but the message does not really explain it (at least to me). >> >> > >> >> Please let me know if this thread sets the context ? >> >> >> >> http://www.spinics.net/lists/linux-usb/msg64938.html >> > >> > I don't understand this either. What's wrong with setting short_not_ok >> > while at SuperSpeed? It shouldn't force the use of a bounce buffer. >> > >> >> I have just brought back some code removed by patch "usb: fix mass >> storage gadgets to work with Synopsys UDC". > > Maybe while bringing it back you can remove the checks for SuperSpeed. > Is there any reason to keep them? Looks like it is required for DWC. > > Alan Stern > -- 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 : mass storage : short_not_ok for non usb3 udc
Hi Alan On Wed, Jul 25, 2012 at 9:07 PM, Alan Stern wrote: > On Wed, 25 Jul 2012, Rajaram R wrote: > >> Hi >> >> On Wed, Jul 25, 2012 at 6:33 PM, Michal Nazarewicz wrote: >> > On Wed, 25 Jul 2012 14:53:00 +0200, Rajaram REGUPATHY >> > wrote: >> >> >> >> The short_not_ok field is used by class drivers to indicate udc whether >> >> short packet is expected during a particular transfer. >> >> In case of mass storage, during command and status phase this field is set >> >> as false and set to true during data phase. >> >> musb driver uses this field to decide whether to program DMA for mode1. >> >> This code is essential for musb driver to program DMA. >> > >> > >> > There's one thing I don't get. The message talks about musb but the code >> > checks for non Super Speed devices. So maybe the code is correct, maybe >> > it's not, but the message does not really explain it (at least to me). >> > >> Please let me know if this thread sets the context ? >> >> http://www.spinics.net/lists/linux-usb/msg64938.html > > I don't understand this either. What's wrong with setting short_not_ok > while at SuperSpeed? It shouldn't force the use of a bounce buffer. > I have just brought back some code removed by patch "usb: fix mass storage gadgets to work with Synopsys UDC". > In any case, the patch description should be improved to explain more > clearly what the real problem is. It should also be more clear about Sure. Will update the description > what the existing code does and what changes the patch makes; your > description above seems to say that the existing code sets short_not_ok > during the data phase. > > Alan Stern > -- 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 : mass storage : short_not_ok for non usb3 udc
Hi
On Wed, Jul 25, 2012 at 6:33 PM, Michal Nazarewicz wrote:
> On Wed, 25 Jul 2012 14:53:00 +0200, Rajaram REGUPATHY
> wrote:
>>
>> The short_not_ok field is used by class drivers to indicate udc whether
>> short packet is expected during a particular transfer.
>> In case of mass storage, during command and status phase this field is set
>> as false and set to true during data phase.
>> musb driver uses this field to decide whether to program DMA for mode1.
>> This code is essential for musb driver to program DMA.
>
>
> There's one thing I don't get. The message talks about musb but the code
> checks for non Super Speed devices. So maybe the code is correct, maybe
> it's not, but the message does not really explain it (at least to me).
>
Please let me know if this thread sets the context ?
http://www.spinics.net/lists/linux-usb/msg64938.html
> (Also, you probably should wrap the message before 80th column.)
>
>
>> Signed-off-by: Balakumar Rajendran
>> Signed-off-by: Rajaram R
>> ---
>> drivers/usb/gadget/f_mass_storage.c |9 +
>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/f_mass_storage.c
>> b/drivers/usb/gadget/f_mass_storage.c
>> index f67b453..db8bf4a 100644
>> --- a/drivers/usb/gadget/f_mass_storage.c
>> +++ b/drivers/usb/gadget/f_mass_storage.c
>> @@ -878,6 +878,7 @@ static int do_write(struct fsg_common *common)
>> unsigned intamount;
>> ssize_t nwritten;
>> int rc;
>> + struct usb_gadget *gadget = common->cdev->gadget;
>> if (curlun->ro) {
>> curlun->sense_data = SS_WRITE_PROTECTED; @@ -960,6 +961,8
>> @@ static int do_write(struct fsg_common *common)
>> * the bulk-out maxpacket size.
>> */
>> set_bulk_out_req_length(common, bh, amount);
>> + if (!gadget_is_superspeed(gadget))
>> + bh->outreq->short_not_ok = 1;
>> if (!start_out_transfer(common, bh))
>> /* Dunno what to do if common->fsg is NULL
>> */
>> return -EIO;
>> @@ -1584,6 +1587,7 @@ static int throw_away_data(struct fsg_common
>> *common)
>> struct fsg_buffhd *bh;
>> u32 amount;
>> int rc;
>> + struct usb_gadget *gadget = common->cdev->gadget;
>> for (bh = common->next_buffhd_to_drain;
>> bh->state != BUF_STATE_EMPTY || common->usb_amount_left > 0;
>> @@ -1617,6 +1621,8 @@ static int throw_away_data(struct fsg_common *common)
>> * the bulk-out maxpacket size.
>> */
>> set_bulk_out_req_length(common, bh, amount);
>> + if (!gadget_is_superspeed(gadget))
>> + bh->outreq->short_not_ok = 1;
>> if (!start_out_transfer(common, bh))
>> /* Dunno what to do if common->fsg is NULL
>> */
>> return -EIO;
>> @@ -2292,6 +2298,7 @@ static int get_next_command(struct fsg_common
>> *common) {
>> struct fsg_buffhd *bh;
>> int rc = 0;
>> + struct usb_gadget *gadget = common->cdev->gadget;
>> /* Wait for the next buffer to become available */
>> bh = common->next_buffhd_to_fill;
>> @@ -2303,6 +2310,8 @@ static int get_next_command(struct fsg_common
>> *common)
>> /* Queue a request to read a Bulk-only CBW */
>> set_bulk_out_req_length(common, bh, US_BULK_CB_WRAP_LEN);
>> + if (!gadget_is_superspeed(gadget))
>> + bh->outreq->short_not_ok = 0;
>> if (!start_out_transfer(common, bh))
>> /* Don't know what to do if common->fsg is NULL */
>> return -EIO;
>
>
> --
> Best regards, _ _
> .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
> ..o | Computer Science, Michał “mina86” Nazarewicz(o o)
> ooo +--ooO--(_)--Ooo--
>
> --
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/4] usb gadget: Configure endpoint according to gadget speed.
Hi Felipe/Tatyana
Could you please comment ?
On Thu, Jul 12, 2012 at 4:53 PM, Rajaram R wrote:
> Hi
>
>
> On Thu, Nov 18, 2010 at 6:17 PM, Tatyana Brokhman
> wrote:
>>
>> Add config_ep_by_speed() to configure the endpoint according to the gadget
>> speed. Using this function will spare the FDs from handling the endpoint
>> chosen descriptor.
>>
>> Signed-off-by: Tatyana Brokhman
>> ---
>> drivers/usb/gadget/composite.c | 76
>> +++
>> drivers/usb/gadget/epautoconf.c |1 +
>> include/linux/usb/composite.h | 21 +++
>> include/linux/usb/gadget.h |3 ++
>> 4 files changed, 101 insertions(+), 0 deletions(-)
>>
> ---cut---
>
>> + */
>> +int config_ep_by_speed(struct usb_gadget *g,
>> + struct usb_function *f,
>> + struct usb_ep *_ep)
>> +{
>> + struct usb_endpoint_descriptor *chosen_desc = NULL;
>> + struct usb_descriptor_header **speed_desc = NULL;
>> +
>> + struct usb_descriptor_header **d_spd; /* cursor for speed desc */
>> +
>> + if (!g || !f || !_ep)
>> + return -EIO;
>> +
>> + /* select desired speed */
>> + switch (g->speed) {
>> + case USB_SPEED_HIGH:
>> + if (gadget_is_dualspeed(g)) {
>> + speed_desc = f->hs_descriptors;
>> + break;
>> + }
>> + /* else: fall through */
>> + default:
>> + speed_desc = f->descriptors;
>> + }
>> + /* find descriptors */
>> + for (d_spd = next_ep_desc(speed_desc); d_spd;
>> + d_spd = next_ep_desc(d_spd+1)) {
>> + chosen_desc = (struct usb_endpoint_descriptor *)*d_spd;
>> + if (chosen_desc->bEndpointAddress == _ep->bEndpointAddress)
>> + goto ep_found;
>> + }
>> + return -EIO;
>> +
>> +ep_found:
>> + /* commit results */
>> + _ep->maxpacket = le16_to_cpu(chosen_desc->wMaxPacketSize);
>> + _ep->desc = chosen_desc;
>
> Could you please comment on why do we need to update gadget's ep list
> with function's maxpacket ? Will this not affect when we switch
> functions ?
>
>> +
>> + return 0;
>> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/4] usb gadget: Configure endpoint according to gadget speed.
Hi
On Thu, Nov 18, 2010 at 6:17 PM, Tatyana Brokhman
wrote:
>
> Add config_ep_by_speed() to configure the endpoint according to the gadget
> speed. Using this function will spare the FDs from handling the endpoint
> chosen descriptor.
>
> Signed-off-by: Tatyana Brokhman
> ---
> drivers/usb/gadget/composite.c | 76
> +++
> drivers/usb/gadget/epautoconf.c |1 +
> include/linux/usb/composite.h | 21 +++
> include/linux/usb/gadget.h |3 ++
> 4 files changed, 101 insertions(+), 0 deletions(-)
>
---cut---
> + */
> +int config_ep_by_speed(struct usb_gadget *g,
> + struct usb_function *f,
> + struct usb_ep *_ep)
> +{
> + struct usb_endpoint_descriptor *chosen_desc = NULL;
> + struct usb_descriptor_header **speed_desc = NULL;
> +
> + struct usb_descriptor_header **d_spd; /* cursor for speed desc */
> +
> + if (!g || !f || !_ep)
> + return -EIO;
> +
> + /* select desired speed */
> + switch (g->speed) {
> + case USB_SPEED_HIGH:
> + if (gadget_is_dualspeed(g)) {
> + speed_desc = f->hs_descriptors;
> + break;
> + }
> + /* else: fall through */
> + default:
> + speed_desc = f->descriptors;
> + }
> + /* find descriptors */
> + for (d_spd = next_ep_desc(speed_desc); d_spd;
> + d_spd = next_ep_desc(d_spd+1)) {
> + chosen_desc = (struct usb_endpoint_descriptor *)*d_spd;
> + if (chosen_desc->bEndpointAddress == _ep->bEndpointAddress)
> + goto ep_found;
> + }
> + return -EIO;
> +
> +ep_found:
> + /* commit results */
> + _ep->maxpacket = le16_to_cpu(chosen_desc->wMaxPacketSize);
> + _ep->desc = chosen_desc;
Could you please comment on why do we need to update gadget's ep list
with function's maxpacket ? Will this not affect when we switch
functions ?
> +
> + return 0;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
