Re: Gadgetfs - adding support for delegation of setup requests
Hi, Binyamin Sharet writes: > I think this will cause existing implementation over gadgetfs to fail > with this > special kernel (as now it will delegate everything all of the time). How > about > using a ioctl to configure it, but wrapping this ioctl with Kconfig? > This way > gadgetfs will operate as always unless a user activates "delegate all" > in runtime. > > Sounds reasonable? You mean a ep0 ioctl? Makes sense to me. Then we can add more features later. Perhaps just add a "Get supported features" IOCTL which returns a struct with 256 bits (4x uint64_t). I doubt we will ever need that many bits, but better safe than sorry, I guess. >>> Don't we need some "set feature" IOCTL as well? >> Why? Features are enabled at kernel compile time. Userspace issues ioctl >> to ask the kernel "which features do you have enabled for >> GadgetFS?". Note that we have a backwards compatibility problem here. If >> "which features do you have enabled for GadgetFS" ioctl fails, we will >> assume that kernel is older than the first kernel where this ioctl was >> added. >> >> To help userland a little more, we can make sure that, from now on, the >> ioctl itself is always enabled and bit0 out of the 256 bits we have MUST >> always be set. This means that "Forward all requests to userspace" >> feature has to be, at least, bit1. >> >> This mimics what USB descriptors where bit7 of config descriptor's >> bmAttributes must always be set. We're just using bit0 instead. >> > What I mean is - when the gadgetfs compiled with "Forward all requests to > userspace" feature, it should behave as before until an ep0 ioctl is issued > to actually enable this feature (e.g. forward the messages) otherwise, this > version of gadgetfs will break any current userspace application that uses > it. > Am I missing something? No, you're not. I misunderstood what you meant ;-) Yeah, I agree, we also need an IOCTL to enable specific features after discovering they exist. -- balbi signature.asc Description: PGP signature
Re: [PATCH 1/1] ARM: dma: fix dma_max_pfn()
Hi Santosh,
On 19/08/16 05:01, Santosh Shilimkar wrote:
> On 8/18/2016 4:07 PM, Russell King - ARM Linux wrote:
>> On Thu, Aug 18, 2016 at 09:55:55AM -0700, Santosh Shilimkar wrote:
>>> Hi Russell,
>>>
>>> On 8/18/2016 7:24 AM, Russell King - ARM Linux wrote:
On Wed, Aug 17, 2016 at 03:05:17PM +0300, Roger Quadros wrote:
> Since commit 6ce0d2001692 ("ARM: dma: Use dma_pfn_offset for dma address
> translation"),
> dma_to_pfn() already returns the PFN with the physical memory start offset
> so we don't need to add it again.
>
> This fixes USB mass storage lock-up problem on systems that can't do DMA
> over the entire physical memory range (e.g.) Keystone 2 systems with 4GB
> RAM
> can only do DMA over the first 2GB. [K2E-EVM].
>
> What happens there is that without this patch SCSI layer sets a wrong
> bounce buffer limit in scsi_calculate_bounce_limit() for the USB mass
> storage device. dma_max_pfn() evaluates to 0x8f and bounce_limit
> is set to 0x8f000 whereas maximum DMA'ble physical memory on Keystone
> 2
> is 0x87fff. This results in non DMA'ble pages being given to the
> USB controller and hence the lock-up.
>
> NOTE: in the above case, USB-SCSI-device's dma_pfn_offset was showing as
> 0.
> This should have really been 0x78 as on K2e, LOWMEM_START is
> 0x8000
> and HIGHMEM_START is 0x8. DMA zone is 2GB so dma_max_pfn should be
> 0x87. The incorrect dma_pfn_offset for the USB storage device is
> because
> USB devices are not correctly inheriting the dma_pfn_offset from the
> USB host controller. This will be fixed by a separate patch.
I'd like to hear from Santosh, as the author of the original change.
The original commit doesn't mention which platform it was intended for
or what the problem was, which would've been helpful.
>>> From what I recollect, we did these changes to make the max pfn behave
>>> same on ARM arch as other archs. This patch was evolved as part of
>>> fixing the max*pfn assumption.
>>
>> To me, the proposed patch _looks_ correct, because...
>>
> diff --git a/arch/arm/include/asm/dma-mapping.h
> b/arch/arm/include/asm/dma-mapping.h
> index d009f79..bf02dbd 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -111,7 +111,7 @@ static inline dma_addr_t virt_to_dma(struct device
> *dev, void *addr)
> /* The ARM override for dma_max_pfn() */
> static inline unsigned long dma_max_pfn(struct device *dev)
> {
> -return PHYS_PFN_OFFSET + dma_to_pfn(dev, *dev->dma_mask);
> +return dma_to_pfn(dev, *dev->dma_mask);
> }
> #define dma_max_pfn(dev) dma_max_pfn(dev)
>>> By doing this change I hope we don't break other drivers on Keystone so
>>> am not sure about the change.
>>
>> dma_to_pfn() returns the page frame number referenced from physical
>> address zero - the default implementation of dma_to_pfn() is
>> bus_to_pfn(), which is __phys_to_pfn(x), which is just x >> PAGE_SHIFT.
>> The other thing about dma_to_pfn() is that it should return a
>> zero-referenced PFN number, where PFN 0 = physical address 0.
>>
>> If there is some offset for keystone2, that should be taken care of
>> via "dev->dma_pfn_offset", and not offsetting the return value from
>> dma_to_pfn().
>>
>> So I'm 99.9% convinced that the proposed change is correct.
>>
> I will got with that then :-) and take my objection back. Just
> saying that if there other breakages which I can't recollect now,
> those drivers needs to be patched as well.
>
I was able to boot the Keystone2 Edison EVM over NFS with the $subject patch.
Boot log is below. Do you see anything suspicious?
---
U-Boot SPL 2016.05-00100-g052f3bf-dirty (Aug 10 2016 - 12:28:40)
Trying to boot from SPI
U-Boot 2016.05-00100-g052f3bf-dirty (Aug 10 2016 - 12:28:40 +0300)
CPU: 66AK2Ex SR1.0
I2C: ready
DRAM: DDR3A Speed will be configured for 1600 Operation.
Detected SO-DIMM [18KSF51272HZ-1G6K2]
DDR3 speed 1600
DRAM: 4 GiB
Clear entire DDR3 memory to enable ECC
2 GiB
NAND: 512 MiB
Net: eth0: netcp@2400
Hit any key to stop autoboot: 0
netcp@2400 Waiting for SGMII auto negotiation to complete. done
Using netcp@2400 device
TFTP from server 192.168.1.36; our IP address is 192.168.1.90
Filename '/tftpboot/k2-fw-initrd.cpio.gz'.
Load address: 0x8808
Loading: ##
958 KiB/s
done
Bytes transferred = 48117 (bbf5 hex)
netcp@2400 Waiting for SGMII auto negotiation to complete. done
Using netcp@2400 device
TFTP from server 192.168.1.36; our IP address is 192.168.1.90
Filename 'keystone-k2e-evm.dtb'.
Load address: 0x8800
Loading: #
884.8 KiB/s
done
Bytes transferred = 22660 (5884 hex)
netcp@2400 Waiting for SGMII auto negotiation to complete. done
Using netcp@2400 device
TFTP from server 192.168.1.36; our IP address is 192.1
Re: [PATCH 2/4] usb: host: xhci: Introduce one new 'usb3_slow_suspend' member for xhci private data
Hi Felipe, On 18 August 2016 at 21:42, Felipe Balbi wrote: > > Hi Baolin, > > Baolin Wang writes: >> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c >> index e2e2487..162f17c 100644 >> --- a/drivers/usb/host/xhci-plat.c >> +++ b/drivers/usb/host/xhci-plat.c >> @@ -250,6 +250,9 @@ static int xhci_plat_probe(struct platform_device >> *pdev) >> (pdata && pdata->usb3_lpm_capable)) >> xhci->quirks |= XHCI_LPM_SUPPORT; >> >> + if (pdata && pdata->usb3_slow_suspend) >> + xhci->quirks |= XHCI_SLOW_SUSPEND; > > I remember having a discussion about this with Paul Z and it turned out > that we really didn't need SLOW_SUSPEND. Can you describe further in > what situation you need this quirk? On my dwc3 platform, xhci suspend will be failed if we have not enabled XHCI_SLOW_SUSPEND quirk. >>> >>> fail how? What error do you see? Do you have some traces of what's >>> happening? Did you try figuring out if this is, perhaps, caused by some >>> call ordering which is wrong? Perhaps disabling PHYs too early or >>> something like that? >> >> It shows the warning "WARN: xHC CMD_RUN timeout" when running >> xhci_suspend(). If I enbale XHCI_SLOW_SUSPEND quirk, then it can work >> well. I did not try to figure out other things, due to I think the >> dwc3 need XHCI_SLOW_SUSPEND quirk. But I can re-try to figure out if >> there are other issues if you still believe that dwc3 does not need >> XHCI_SLOW_SUSPEND quirk. Thanks. > > When I discussed this with Paul Z, he told me there was no known > DWC3 SoC that really needed SLOW_SUSPEND, so it's likely to be a bug in > either xhci or dwc3. > > Please try to track this down. I would start looking at ordering with > PHY poweroff or something like that. OK. Thanks. -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: xHCI problem? [was Re: Erratic USB device behavior and device loss]
On 18.08.2016 17:33, Ritesh Raj Sarraf wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA512 Hello Mathias, Hi Can you please confirm of the erratic behavior we've discussed so far, on this thread ? It'd help to know if it really is a bug or something else. Could you add xhci debugging?. The current logs mainly show usb core reporting errors on urbs returned (or timeout) from xhci. xhci debugging can be added with echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control Make sure debugfs is mounted before. There is a scattergather fix in 4.8-rc1, but it has another regression, so waiting for 4.8-rc3 and retrying with it could make sense. Not sure if that fix is related, but should be fairly easy to try before digging too deep into this -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: gadget: f_ncm: add support for scatter/gather SKB to enable GSO
Hi,
Jussi Kivilinna writes:
>> Felipe Balbi writes:
>> Enabling SG allows enabling GSO (generic segmentation offload) feature
>> of linux networking layer. This increases TCP throughput with NCM
>> on Cortex-A15+USB3380 based device from 300 Mbit/s to 1.1 Gbit/s.
>>
>> Signed-off-by: Jussi Kivilinna
>
> this is AWESOME!! :-) But here's the thing, any chance we can build this
> in u_ether.c ? Also, NETIF_F_SG should be conditional on
> gadget->sg_supported so that we don't break UDCs that don't support
> sglists.
>
Actually, no sglists are passed to UDC. Reason why this work
with minimal changes for NCM is that NCM does tx buffering
in its wrap function.. 'ncm_wrap_ntb' copies input skbuffs to
larger skbuff, so enabling SG is only matter of changing that
skbuff data copy from 'memcpy' to 'skb_copy_bits' (and changing
CRC calculation work with skbuff fragments). Since NCM already
does copying, SG can be enabled for NCM without extra overhead.
>>>
>>> aha, understood. Now what if we skip copying altogether? If we have an
>>> sg and a UDC that supports sg (gadget->sg_supported = 1), then we can
>>> avoid copying, right?
>>
>> perhaps this is the crud of the change (still need to check for
>> sg_supported)?
>>
>> diff --git a/drivers/usb/gadget/function/u_ether.c
>> b/drivers/usb/gadget/function/u_ether.c
>> index 5f562c1ec795..f3497cba32ec 100644
>> --- a/drivers/usb/gadget/function/u_ether.c
>> +++ b/drivers/usb/gadget/function/u_ether.c
>> @@ -235,7 +235,7 @@ rx_submit(struct eth_dev *dev, struct usb_request *req,
>> gfp_t gfp_flags)
>> */
>> skb_reserve(skb, NET_IP_ALIGN);
>>
>> - req->buf = skb->data;
>> + req->num_sgs = skb_to_sgvec(skb, req->sg, 0, skb->len);
>> req->length = size;
>> req->complete = rx_complete;
>> req->context = skb;
>>
>
> I did experiment with this, but cannot fully test it without
> sg_supported UDC. Also, wrapper functions need to be reviewed
> to make sure they work with non-linear skbuffs. Seems that EEM
> wrapper had problem with SG, probably because it tries to add
> CRC at the skbuff tail. Below is patch which I've tested for
> now. It applies on top of the earlier NCM SG patch.
>
> -Jussi
>
> ---
> [PATCH] [RFC] usb: gadget: u_ether: enable NETIF_F_SG for networking gadgets
>
> Signed-off-by: Jussi Kivilinna
I tested this patch on top of previous two patches. It works fine for a
while, but then starts failing. No idea yet why it fails. I'll try to
debug this for a while longer.
> @@ -363,6 +364,14 @@ static int prealloc(struct list_head *list, struct
> usb_ep *ep, unsigned n)
> }
> while (i--) {
> req = usb_ep_alloc_request(ep, GFP_ATOMIC);
> + if (alloc_sg && req) {
> + req->sg = kmalloc(sizeof(*req->sg) *
> + (MAX_SKB_FRAGS + 2), GFP_ATOMIC);
how about:
kmalloc_array(MAX_SKB_FRAGS + 2, sizeof(*req->sg), GFP_ATOMIC)
> @@ -376,6 +385,8 @@ extra:
>
> next = req->list.next;
> list_del(&req->list);
> + if (req->sg)
> + kfree(req->sg);
the check here is unnecessary. Just kfree(req->sg) :-)
> @@ -391,10 +402,11 @@ static int alloc_requests(struct eth_dev *dev, struct
> gether *link, unsigned n)
> int status;
>
> spin_lock(&dev->req_lock);
> - status = prealloc(&dev->tx_reqs, link->in_ep, n);
> + status = prealloc(&dev->tx_reqs, link->in_ep, n,
> + dev->gadget->sg_supported);
> if (status < 0)
> goto fail;
> - status = prealloc(&dev->rx_reqs, link->out_ep, n);
> + status = prealloc(&dev->rx_reqs, link->out_ep, n, false);
why don't we want sglist for rx?
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH 2/2] usb: gadget: f_ncm: add support for scatter/gather SKB to enable GSO
Hi, Felipe Balbi writes: > Jussi Kivilinna writes: >>> Felipe Balbi writes: >>> Enabling SG allows enabling GSO (generic segmentation offload) feature >>> of linux networking layer. This increases TCP throughput with NCM >>> on Cortex-A15+USB3380 based device from 300 Mbit/s to 1.1 Gbit/s. >>> >>> Signed-off-by: Jussi Kivilinna >> >> this is AWESOME!! :-) But here's the thing, any chance we can build this >> in u_ether.c ? Also, NETIF_F_SG should be conditional on >> gadget->sg_supported so that we don't break UDCs that don't support >> sglists. >> > > Actually, no sglists are passed to UDC. Reason why this work > with minimal changes for NCM is that NCM does tx buffering > in its wrap function.. 'ncm_wrap_ntb' copies input skbuffs to > larger skbuff, so enabling SG is only matter of changing that > skbuff data copy from 'memcpy' to 'skb_copy_bits' (and changing > CRC calculation work with skbuff fragments). Since NCM already > does copying, SG can be enabled for NCM without extra overhead. aha, understood. Now what if we skip copying altogether? If we have an sg and a UDC that supports sg (gadget->sg_supported = 1), then we can avoid copying, right? >>> >>> perhaps this is the crud of the change (still need to check for >>> sg_supported)? >>> >>> diff --git a/drivers/usb/gadget/function/u_ether.c >>> b/drivers/usb/gadget/function/u_ether.c >>> index 5f562c1ec795..f3497cba32ec 100644 >>> --- a/drivers/usb/gadget/function/u_ether.c >>> +++ b/drivers/usb/gadget/function/u_ether.c >>> @@ -235,7 +235,7 @@ rx_submit(struct eth_dev *dev, struct usb_request *req, >>> gfp_t gfp_flags) >>> */ >>> skb_reserve(skb, NET_IP_ALIGN); >>> >>> - req->buf = skb->data; >>> + req->num_sgs = skb_to_sgvec(skb, req->sg, 0, skb->len); >>> req->length = size; >>> req->complete = rx_complete; >>> req->context = skb; >>> >> >> I did experiment with this, but cannot fully test it without >> sg_supported UDC. Also, wrapper functions need to be reviewed >> to make sure they work with non-linear skbuffs. Seems that EEM >> wrapper had problem with SG, probably because it tries to add >> CRC at the skbuff tail. Below is patch which I've tested for >> now. It applies on top of the earlier NCM SG patch. >> >> -Jussi >> >> --- >> [PATCH] [RFC] usb: gadget: u_ether: enable NETIF_F_SG for networking gadgets >> >> Signed-off-by: Jussi Kivilinna > > I tested this patch on top of previous two patches. It works fine for a > while, but then starts failing. No idea yet why it fails. I'll try to > debug this for a while longer. nothing interesting on dwc3's traces, but I see pending softirq: [ 28.978622] IPv6: ADDRCONF(NETDEV_UP): usb0: link is not ready [ 39.633641] configfs-gadget gadget: high-speed config #1: c [ 39.650761] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready [ 39.701440] NOHZ: local_softirq_pending 08 [ 39.721752] NOHZ: local_softirq_pending 08 [ 40.228248] NOHZ: local_softirq_pending 08 [ 40.610264] NOHZ: local_softirq_pending 08 [ 41.233350] NOHZ: local_softirq_pending 08 [ 41.233577] NOHZ: local_softirq_pending 08 [ 41.233729] NOHZ: local_softirq_pending 08 [ 41.241269] NOHZ: local_softirq_pending 08 [ 41.294759] NOHZ: local_softirq_pending 08 [ 41.350932] NOHZ: local_softirq_pending 08 I don't get why our networking gadgets are always so flakey. Guess I should really dig details about the networking stack to fix these bugs once and for all :-( -- balbi signature.asc Description: PGP signature
[PATCH] usb: chipidea: usb2: delete the redundant setting default DMA mask code
Similar as commit 2b2fe36def08 ("usb: chipidea: imx: delete the
redundant setting default DMA mask code"), the ci_hdrc_usb2 platform
device is also created by device tree, the default DMA mask should be
already set by of_dma_configure when the device are created. So delete
the redundant code at driver.
Signed-off-by: Jisheng Zhang
---
drivers/usb/chipidea/ci_hdrc_usb2.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c
b/drivers/usb/chipidea/ci_hdrc_usb2.c
index 4456d2c..d162cc0 100644
--- a/drivers/usb/chipidea/ci_hdrc_usb2.c
+++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
@@ -74,10 +74,6 @@ static int ci_hdrc_usb2_probe(struct platform_device *pdev)
}
}
- ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
- if (ret)
- goto clk_err;
-
ci_pdata->name = dev_name(dev);
priv->ci_pdev = ci_hdrc_add_device(dev, pdev->resource,
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: chipidea: delete an useless header file
is for net phy drivers, we don't need it. Signed-off-by: Jisheng Zhang --- drivers/usb/chipidea/core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 69426e6..ae12595 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -62,7 +62,6 @@ #include #include #include -#include #include #include -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: chipidea: support generic phy in PM code path
Support generic phy in PM code path: call phy_power_off/phy_power_on
in ci_controller_suspend/ci_controller_resume.
Signed-off-by: Jisheng Zhang
---
drivers/usb/chipidea/core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index ae12595..ef9fb0b 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -1116,6 +1116,7 @@ static void ci_controller_suspend(struct ci_hdrc *ci)
usleep_range(ci->platdata->phy_clkgate_delay_us,
ci->platdata->phy_clkgate_delay_us + 50);
usb_phy_set_suspend(ci->usb_phy, 1);
+ phy_power_off(ci->phy);
ci->in_lpm = true;
enable_irq(ci->irq);
}
@@ -1132,9 +1133,10 @@ static int ci_controller_resume(struct device *dev)
}
ci_hdrc_enter_lpm(ci, false);
- if (ci->usb_phy) {
+ if (ci->usb_phy || ci->phy) {
usb_phy_set_suspend(ci->usb_phy, 0);
usb_phy_set_wakeup(ci->usb_phy, false);
+ phy_power_on(ci->phy);
hw_wait_phy_stable();
}
--
2.9.3
--
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: chipidea: delete an useless header file
Hello. On 8/19/2016 3:05 PM, Jisheng Zhang wrote: is for net phy drivers, we don't need it. Signed-off-by: Jisheng Zhang --- drivers/usb/chipidea/core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 69426e6..ae12595 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -62,7 +62,6 @@ #include #include #include -#include #include #include You're deleting an #include, not a file (as you say in the subject). MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: chipidea: delete an useless header include
is for net phy drivers, we don't need it. Signed-off-by: Jisheng Zhang --- Since v1: - fix commit subject. Thank Sergei for pointing it out. drivers/usb/chipidea/core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 69426e6..ae12595 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -62,7 +62,6 @@ #include #include #include -#include #include #include -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/7] phy-sun4i-usb: Add support for phy_set_mode
Hi,
On Thursday 18 August 2016 03:47 PM, Felipe Balbi wrote:
>
> Hi,
>
> Hans de Goede writes:
>
> [...]
>
>> void sun4i_usb_phy_set_squelch_detect(struct phy *_phy, bool enabled)
>> {
>> struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
> [...]
>
> $ scripts/checkpatch.pl
> ~/patches/phy-sun4i-usb-Add-support-for-phy_set_mode.patch
> ERROR: trailing statements should be on next line
> #29: FILE: drivers/phy/phy-sun4i-usb.c:439:
> +case PHY_MODE_USB_HOST: data->dr_mode = USB_DR_MODE_HOST; break;
>
> ERROR: trailing statements should be on next line
> #30: FILE: drivers/phy/phy-sun4i-usb.c:440:
> +case PHY_MODE_USB_DEVICE: data->dr_mode = USB_DR_MODE_PERIPHERAL;
> break;
>
> ERROR: trailing statements should be on next line
> #31: FILE: drivers/phy/phy-sun4i-usb.c:441:
> +case PHY_MODE_USB_OTG:data->dr_mode = USB_DR_MODE_OTG; break;
This is normal codeing style for a switch-case assigning a single value
per case,
but checkpatch does not know this.
>>>
>>> I don't see that in CodingStyle
>>
>> It is an exception to the rule as such it is not listed, but this
>> really is quite a normal thing to do in C code.
>>
>>> and it's quite ugly.
>>
>> So this is ugly:
>>
>> switch (mode) {
>> case PHY_MODE_USB_HOST: data->dr_mode = USB_DR_MODE_HOST; break;
>> case PHY_MODE_USB_DEVICE: data->dr_mode = USB_DR_MODE_PERIPHERAL; break;
>> case PHY_MODE_USB_OTG:data->dr_mode = USB_DR_MODE_OTG; break;
>> default:
>> return -EINVAL;
>> }
>>
>> Where as this is not:
>>
>> switch (mode) {
>> case PHY_MODE_USB_HOST:
>> data->dr_mode = USB_DR_MODE_HOST;
>> break;
>> case PHY_MODE_USB_DEVICE:
>> data->dr_mode = USB_DR_MODE_PERIPHERAL;
>> break;
>> case PHY_MODE_USB_OTG:
>> data->dr_mode = USB_DR_MODE_OTG;
>> break;
>> default:
>> return -EINVAL;
>> }
>>
>> ???
>>
>> IMHO the original version is much easier to read / makes it much
>> clearer what the code is doing.
>>
>> But if you insist I can do a v3 changing the coding style to
>> the (IMHO) uglier version.
>>
>> Also note that the real ugliness is that we've 3 different enums
>> for host / device / dual-role. For some reason the musb code has
>> 2 all of its own and then there is "enum phy_mode".
>>
>> Anyways let me know if you want a v3 with check-patch warnings
>> fixed.
>
> I see it's somewhat common even in drivers/usb:
>
> $ git grep -ce "case \w+:.*break;" -- drivers/usb/
> drivers/usb/gadget/udc/net2272.c:4
> drivers/usb/host/ehci-hcd.c:3
> drivers/usb/host/isp116x.h:2
> drivers/usb/host/ohci-dbg.c:14
> drivers/usb/host/sl811-hcd.c:7
> drivers/usb/host/uhci-debug.c:8
> drivers/usb/image/microtek.c:64
> drivers/usb/mon/mon_text.c:6
> drivers/usb/musb/musb_gadget.c:2
> drivers/usb/serial/digi_acceleport.c:23
> drivers/usb/serial/ftdi_sio.c:10
> drivers/usb/serial/mct_u232.c:10
> drivers/usb/serial/spcp8x5.c:17
> drivers/usb/serial/whiteheat.c:4
> drivers/usb/storage/debug.c:86
>
> so I'm okay either way. Kishon has the final say here since he's
> drivers/phy/ maintainer.
hmm.. I'd prefer without checkpatch errors or warnings.
Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation: dt: dwc3: note the supported phy-names
On Thu, Aug 18, 2016 at 02:37:16PM -0700, Brian Norris wrote: > The dwc3 driver expicitly looks for "usb2-phy" or "usb3-phy", but we > never noted these names in the documentation. > > Signed-off-by: Brian Norris > --- > Documentation/devicetree/bindings/usb/dwc3.txt | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Acked-by: Rob Herring -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How the EHCI HC driver make the decision to suspend some USB devices?
On Fri, 19 Aug 2016, ludeng wrote: > Hi, > How the EHCI HC driver make the decision to suspend some USB devices, > but not to suspend some others? We notice that for some USB Video > Cameras, when they are enumerated and there is no data to transfer, > the EHCI HC driver will suspend them by setting the Suspend bit of > the PORTSC register. But for some other Cameras, the EHCI HC driver > will not suspend them. The drvier make the decision based on what > kind of information of the device ? The EHCI driver does not make this decision. The decision is made by the video camera driver and by the user. 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
[GIT PULL] USB driver fixes for 4.8-rc3
The following changes since commit 29b4817d4018df78086157ea3a55c1d9424a7cfc: Linux 4.8-rc1 (2016-08-07 18:18:00 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ tags/usb-4.8-rc3 for you to fetch changes up to f1f6d9a8b540df22b87a5bf6bc104edaade81f47: xhci: don't dereference a xhci member after removing xhci (2016-08-16 09:42:47 +0200) USB fixes for 4.8-rc3 Here are a number of USB fixes for reported issues for your tree. The normal amount of gadget fixes, xhci fixes, new device ids, and a few other minor things. All of them have been in linux-next for a while, the full details are in the shortlog below. Signed-off-by: Greg Kroah-Hartman Alan Stern (4): USB: hub: fix up early-exit pathway in hub_activate USB: hub: change the locking in hub_activate USB: validate wMaxPacketValue entries in endpoint descriptors USB: remove race condition in usbfs/libusb when using reap-after-disconnect Alban Browaeys (1): xhci: really enqueue zero length TRBs. Alexey Klimov (1): USB: serial: fix memleak in driver-registration error path Binyamin Sharet (1): usb: gadget: fix check in sync read from ep in gadgetfs Christophe JAILLET (2): usb: gadget: uvc: Fix return value in case of error usb: gadget: composite: Fix return value in case of error Dan Carpenter (1): usb: gadget: fsl_qe_udc: off by one in setup_received_handle() Daniele Palmas (1): USB: serial: option: add support for Telit LE920A4 Felipe Balbi (4): usb: dwc3: gadget: increment request->actual once usb: dwc3: gadget: fix for short pkts during chained xfers usb: dwc3: gadget: always cleanup all TRBs usb: dwc3: gadget: stop processing on HWO set Gavin Li (1): cdc-acm: fix wrong pipe type on rx interrupt xfers Greg Kroah-Hartman (2): Merge tag 'fixes-for-v4.8-rc2' of git://git.kernel.org/.../balbi/usb into usb-linus Merge tag 'usb-serial-4.8-rc2' of git://git.kernel.org/.../johan/usb-serial into usb-linus Heikki Krogerus (1): usb: dwc3: pci: add Intel Kabylake PCI ID Jaewon Kim (1): usb: host: max3421-hcd: fix mask of IO control register Janusz Dziedzic (1): usb: dwc3: don't set last bit for ISOC endpoints Jim Lin (1): usb: xhci: Fix panic if disconnect Jiri Slaby (1): usb: devio, do not warn when allocation fails Lu Baolu (1): usb: misc: usbtest: add fix for driver hang Lubomir Rintel (1): USB: serial: option: add D-Link DWM-156/A3 Marc Ohlf (1): usb: ehci: change order of register cleanup during shutdown Mathias Nyman (2): xhci: always handle "Command Ring Stopped" events xhci: don't dereference a xhci member after removing xhci Mathieu Laurendeau (1): usb/gadget: fix gadgetfs aio support. Peter Chen (5): usb: misc: usbtest: usbtest_do_ioctl may return positive integer usb: gadget: composite: fix dereference after null check coverify warning usb: gadget: u_ether: fix dereference after null check coverify warning usb: misc: usbtest: usbtest_do_ioctl may return positive integer usb: udc: core: fix error handling Robert Deliën (1): USB: serial: ftdi_sio: add PIDs for Ivium Technologies devices Sheng-Hui J. Chu (1): USB: serial: ftdi_sio: add device ID for WICED USB UART dev board Viresh Kumar (1): usb: hub: Fix unbalanced reference count/memory leak/deadlocks Wei Yongjun (2): usb: phy: omap-otg: Fix missing platform_set_drvdata() in omap_otg_probe() usb: dwc3: fix missing platform_set_drvdata() in dwc3_of_simple_probe() Winter Wang (1): usb: gadget: configfs: add mutex lock before unregister gadget Xerox Lin (1): usb: gadget: rndis: free response queue during REMOTE_NDIS_RESET_MSG Xiao Han (1): usb: misc: ftdi-elan: Fix off-by-one memory corruptions Yoshihiro Shimoda (3): usb: renesas_usbhs: Fix receiving data corrupt on R-Car Gen3 with dmac usb: renesas_usbhs: clear the BRDYSTS in usbhsg_ep_enable() usb: renesas_usbhs: Use dmac only if the pipe type is bulk drivers/usb/class/cdc-acm.c| 5 +-- drivers/usb/class/cdc-acm.h| 1 - drivers/usb/core/config.c | 66 -- drivers/usb/core/devio.c | 7 +++- drivers/usb/core/hub.c | 23 --- drivers/usb/dwc3/dwc3-of-simple.c | 1 + drivers/usb/dwc3/dwc3-pci.c| 2 + drivers/usb/dwc3/gadget.c | 55 +++-- drivers/usb/gadget/composite.c | 6 ++- drivers/usb/gadget/configfs.c | 2 + drivers/usb/gadget/function/rndis.c| 6 +++ drivers/usb/gadget/function/u_ether.c | 3 +- drivers/usb/gadget/function/uvc_conf
[PATCH] USB: ohci-omap - avoid including mach/irqs.h
ohci-omap doesn't need to include mach/irqs.h - nothing within this driver needs anything from this header file. Remove this include. Signed-off-by: Russell King --- drivers/usb/host/ohci-omap.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/host/ohci-omap.c b/drivers/usb/host/ohci-omap.c index de7c68602a7e..495c1454b9e8 100644 --- a/drivers/usb/host/ohci-omap.c +++ b/drivers/usb/host/ohci-omap.c @@ -36,7 +36,6 @@ #include #include -#include #include -- 2.1.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 1/1] ARM: dma: fix dma_max_pfn()
On 8/19/2016 12:30 AM, Roger Quadros wrote: Hi Santosh, So I'm 99.9% convinced that the proposed change is correct. I will got with that then :-) and take my objection back. Just saying that if there other breakages which I can't recollect now, those drivers needs to be patched as well. I was able to boot the Keystone2 Edison EVM over NFS with the $subject patch. Boot log is below. Do you see anything suspicious? Logs looks ok to me. Probably do some tests where DMA and bounce buffers etc gets tested. Running it through your internal regression suit will be good idea as well if thats possible. Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 0/5] da8xx USB PHY platform devices and clocks (was "da8xx UBS clocks")
David, On Wed, Aug 17, 2016 at 4:35 AM, Kishon Vijay Abraham I wrote: > > Hi Kevin, > > On Saturday 13 August 2016 02:54 AM, Kevin Hilman wrote: > > On Wed, May 25, 2016 at 6:18 AM, Sekhar Nori wrote: > >> On Monday 23 May 2016 08:44 PM, David Lechner wrote: > >>> On 05/09/2016 06:46 PM, David Lechner wrote: > v5 changes: renamed "usbphy" to "usb_phy" or "usb-phy" as appropriate > > > > > [...] > > > >>> > >>> What should I be doing to keep this moving along? > >> > >> We need the related driver changes to be applied first. I could then use > >> an immutable branch to push the platform changes against. > >> > >> I did take a look at the patches and they look good to me. Except the > >> one comment from Sergei which I just now indicated that I agree with. > > > > Just checking on the status of this. I'm not seeing the driver > > changes in mainline yet. > > > > Any update? > > phy driver is already merged which actually introduced a compilation error. > The > fix for it is currently queued in linux-phy -fixes and it should be merged in > this -rc cycle. > I think it would be better if David Lechner re-spins the series re-based on > top > of latest mainline kernel and then merged by Bin/Alan. Does this work for you? Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 0/5] da8xx USB PHY platform devices and clocks (was "da8xx UBS clocks")
On 08/19/2016 11:40 AM, Kevin Hilman wrote: David, On Wed, Aug 17, 2016 at 4:35 AM, Kishon Vijay Abraham I wrote: Hi Kevin, On Saturday 13 August 2016 02:54 AM, Kevin Hilman wrote: On Wed, May 25, 2016 at 6:18 AM, Sekhar Nori wrote: On Monday 23 May 2016 08:44 PM, David Lechner wrote: On 05/09/2016 06:46 PM, David Lechner wrote: v5 changes: renamed "usbphy" to "usb_phy" or "usb-phy" as appropriate [...] What should I be doing to keep this moving along? We need the related driver changes to be applied first. I could then use an immutable branch to push the platform changes against. I did take a look at the patches and they look good to me. Except the one comment from Sergei which I just now indicated that I agree with. Just checking on the status of this. I'm not seeing the driver changes in mainline yet. Any update? phy driver is already merged which actually introduced a compilation error. The fix for it is currently queued in linux-phy -fixes and it should be merged in this -rc cycle. I think it would be better if David Lechner re-spins the series re-based on top of latest mainline kernel and then merged by Bin/Alan. Does this work for you? Kevin Yes, I try to get this done in the next few days. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: dwc3: Add revision numbers for the USB 3.0 IP
Add revision number constants for the 3.00a and 3.10a releases.
Signed-off-by: John Youn
---
drivers/usb/dwc3/core.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 3d94acd..002e647 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -902,6 +902,8 @@ struct dwc3 {
#define DWC3_REVISION_260A 0x5533260a
#define DWC3_REVISION_270A 0x5533270a
#define DWC3_REVISION_280A 0x5533280a
+#define DWC3_REVISION_300A 0x5533300a
+#define DWC3_REVISION_310A 0x5533310a
/*
* NOTICE: we're using bit 31 as a "is usb 3.1" flag. This is really
--
2.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: dwc3: Add ENDXFER command polling
ENDXFER polling is available on version 3.10a and later of the
DWC_usb3 (USB 3.0) controller. With this feature, the software can poll
the CMDACT bit in the DEPCMD register after issuing an ENDXFER command.
This feature is enabled by writing GUCTL2[14].
This feature is NOT available on the DWC_usb31 (USB 3.1) IP.
Signed-off-by: John Youn
---
drivers/usb/dwc3/core.c | 11 +++
drivers/usb/dwc3/core.h | 4
drivers/usb/dwc3/gadget.c | 31 ++-
3 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 71ac7c2..057739d 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -704,6 +704,17 @@ static int dwc3_core_init(struct dwc3 *dwc)
break;
}
+ /*
+* ENDXFER polling is available on version 3.10a and later of
+* the DWC_usb3 controller. It is NOT available in the
+* DWC_usb31 controller.
+*/
+ if (!dwc3_is_usb31(dwc) && dwc->revision >= DWC3_REVISION_310A) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUCTL2);
+ reg |= DWC3_GUCTL2_RST_ACTBITLATER;
+ dwc3_writel(dwc->regs, DWC3_GUCTL2, reg);
+ }
+
return 0;
err4:
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 002e647..b2317e7 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -109,6 +109,7 @@
#define DWC3_GPRTBIMAP_HS1 0xc184
#define DWC3_GPRTBIMAP_FS0 0xc188
#define DWC3_GPRTBIMAP_FS1 0xc18c
+#define DWC3_GUCTL20xc19c
#define DWC3_VER_NUMBER0xc1a0
#define DWC3_VER_TYPE 0xc1a4
@@ -288,6 +289,9 @@
#define DWC3_GFLADJ_30MHZ_SDBND_SEL(1 << 7)
#define DWC3_GFLADJ_30MHZ_MASK 0x3f
+/* Global User Control Register 2 */
+#define DWC3_GUCTL2_RST_ACTBITLATER(1 << 14)
+
/* Device Configuration Register */
#define DWC3_DCFG_DEVADDR(addr)((addr) << 3)
#define DWC3_DCFG_DEVADDR_MASK DWC3_DCFG_DEVADDR(0x7f)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index c6fe9a3..823e65d 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2202,6 +2202,7 @@ static void dwc3_stop_active_transfer(struct dwc3 *dwc,
u32 epnum, bool force)
struct dwc3_gadget_ep_cmd_params params;
u32 cmd;
int ret;
+ unsigned int timeout;
dep = dwc->eps[epnum];
@@ -2225,6 +2226,14 @@ static void dwc3_stop_active_transfer(struct dwc3 *dwc,
u32 epnum, bool force)
*
* - Issue EndTransfer WITH CMDIOC bit set
* - Wait 100us
+*
+* As of IP version 3.10a of the DWC_usb3 IP, the controller
+* supports a mode to work around the above limitation. The
+* software can poll the CMDACT bit in the DEPCMD register
+* after issuing a EndTransfer command. This mode is enabled
+* by writing GUCTL2[14].
+*
+* This mode is NOT available on the DWC_usb31 IP.
*/
cmd = DWC3_DEPCMD_ENDTRANSFER;
@@ -2236,7 +2245,27 @@ static void dwc3_stop_active_transfer(struct dwc3 *dwc,
u32 epnum, bool force)
WARN_ON_ONCE(ret);
dep->resource_index = 0;
dep->flags &= ~DWC3_EP_BUSY;
- udelay(100);
+
+ if (dwc3_is_usb31(dwc) || dwc->revision < DWC3_REVISION_310A) {
+ udelay(100);
+ return;
+ }
+
+ timeout = 100;
+
+ do {
+ cmd = dwc3_readl(dep->regs, DWC3_DEPCMD);
+ if (!(cmd & DWC3_DEPCMD_CMDACT))
+ break;
+
+ timeout--;
+ if (!timeout) {
+ dev_err(dwc->dev, "EndTransfer didn't complete\n");
+ break;
+ }
+
+ udelay(1);
+ } while (1);
}
static void dwc3_stop_active_transfers(struct dwc3 *dwc)
--
2.9.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 10/10] usb: gadget: f_hid: use alloc_ep_req()
On 8/8/2016 1:30 PM, Felipe F. Tonello wrote: > Use gadget's framework allocation function instead of directly calling > usb_ep_alloc_request(). > > Signed-off-by: Felipe F. Tonello > --- > drivers/usb/gadget/function/f_hid.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_hid.c > b/drivers/usb/gadget/function/f_hid.c > index a010496e4e05..89d2e9a5a04f 100644 > --- a/drivers/usb/gadget/function/f_hid.c > +++ b/drivers/usb/gadget/function/f_hid.c > @@ -611,14 +611,10 @@ static int hidg_bind(struct usb_configuration *c, > struct usb_function *f) > > /* preallocate request and buffer */ > status = -ENOMEM; > - hidg->req = usb_ep_alloc_request(hidg->in_ep, GFP_KERNEL); > + hidg->req = alloc_ep_req(hidg->in_ep, hidg->report_length); > if (!hidg->req) > goto fail; > > - hidg->req->buf = kmalloc(hidg->report_length, GFP_KERNEL); > - if (!hidg->req->buf) > - goto fail; > - > /* set descriptor dynamic values */ > hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass; > hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol; > Hi Felipe, This commit on your testing/next breaks compilation. ../drivers/usb/gadget/function/f_hid.c: In function ‘hidg_bind’: ../drivers/usb/gadget/function/f_hid.c:620:14: error: too few arguments to function ‘alloc_ep_req’ hidg->req = alloc_ep_req(hidg->in_ep, hidg->report_length); ^ In file included from ../drivers/usb/gadget/function/f_hid.c:24:0: ../drivers/usb/gadget/u_f.h:63:21: note: declared here struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len, int default_len); ^ Regards, John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/7] musb: sunxi: Add support for run-time changing dr-mode through sysfs
Hi, On Mon, Aug 15, 2016 at 09:21:25PM +0200, Hans de Goede wrote: > Hi All, > > Here is a patch series which implements run-time changing the dr-mode > of sunxi musb controllers through the (already existing) musb "mode" > sysfs attribute. > > This is useful on boards where there is no id pin, e.g. some tv-boxes > use the musb controller to get an extra usb A port without needing > a hub chip. Except for the missing id pin when using a usb A<->A cable > these ports can do peripheral mode just fine. This series makes it > possible to do e.g. this by doing echo "peripheral" > mode before > plugging in the usb A<->A cable. Well, this is an illegal usecase. A-A cable is invalid by USB Spec. With type-A receptacle the controller should be in host-only mode, switching to peripheral mode should not be allowed. > > This series has both sun4i-usb-phy driver and sunxi-musb-glue changes, > both are necessary for the run-time changing to work, but they can be > merged independently without breaking anything. > > Changed in v2: > > After sleeping on it a night I realized that always passing port_mode = > DUAL_ROLE to the musb-core was wrong. There is a distintion between > the id-pin not working properly which we can work around with software > mode switching (and we want to register both the host and udc driver > in this case) vs cases where we really only want to register a host > (wifi module connected to musb soldered onto the PCB). > > So v2 of this series drops the > "musb: sunxi: Always register both host and udc when build with dual-role > support" > patch. > > Instead systems which are dual-role capable, but lack an id-pin should use > dr_mode = "otg" in the dts file. There is one problem with this, some > systems are dual-role capable but use a female USB-A connector connected > to the musb controller. These should come up in host mode by default, This is not a problem. With a type-A connector, the dual-role controller should work in host-only mode. Role switching should only be allowed if an AB connector is used. Using the sysfs entry to switch roles for generic purpose is really a bad idea, it opens up ton of problems. For systems which lack of id-pin should use a discrete circuit (for example GPIO) to detect the id pin in the AB receptacle, then the USB driver will handle the role switching transparently. > rather then peripheral mode which is the default for systems which lack an > id-pin. This patch set introduces: > > "phy-sun4i-usb: Add "allwinner,usb0-usb-a-connector" dt property" > > Which allows specifying the use of a female USB-A connector for the > musb controller in the phy dt node, the presence of this dt property > changes the default to host mode. This is unnecessary, if using a type-A connector, dr_mode should be "host" in DT. > > Please review (and if no issues are found merge). > > Thanks & Regards, > > Hans Regards, -Bin. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/7] musb: sunxi: Add support for platform_set_mode
Hi,
On Mon, Aug 15, 2016 at 09:21:32PM +0200, Hans de Goede wrote:
> This allows run-time dr_mode switching support via the "mode" musb
> sysfs attribute.
>
> Signed-off-by: Hans de Goede
> ---
> drivers/usb/musb/sunxi.c | 52
>
> 1 file changed, 48 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> index c6ee166..1fe7451 100644
> --- a/drivers/usb/musb/sunxi.c
> +++ b/drivers/usb/musb/sunxi.c
> @@ -74,6 +74,7 @@
> #define SUNXI_MUSB_FL_HAS_SRAM 5
> #define SUNXI_MUSB_FL_HAS_RESET 6
> #define SUNXI_MUSB_FL_NO_CONFIGDATA 7
> +#define SUNXI_MUSB_FL_PHY_MODE_PEND 8
>
> /* Our read/write methods need access and do not get passed in a musb ref :|
> */
> static struct musb *sunxi_musb;
> @@ -87,6 +88,7 @@ struct sunxi_glue {
> struct phy *phy;
> struct platform_device *usb_phy;
> struct usb_phy *xceiv;
> + enum phy_mode phy_mode;
> unsigned long flags;
> struct work_struct work;
> struct extcon_dev *extcon;
> @@ -140,6 +142,9 @@ static void sunxi_musb_work(struct work_struct *work)
> clear_bit(SUNXI_MUSB_FL_PHY_ON, &glue->flags);
> }
> }
> +
> + if (test_and_clear_bit(SUNXI_MUSB_FL_PHY_MODE_PEND, &glue->flags))
> + phy_set_mode(glue->phy, glue->phy_mode);
> }
>
> static void sunxi_musb_set_vbus(struct musb *musb, int is_on)
> @@ -341,6 +346,41 @@ static void sunxi_musb_dma_controller_destroy(struct
> dma_controller *c)
> {
> }
>
> +static int sunxi_musb_set_mode(struct musb *musb, u8 mode)
> +{
> + struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
> + enum phy_mode new_mode;
> +
> + switch (mode) {
> + case MUSB_HOST: new_mode = PHY_MODE_USB_HOST; break;
> + case MUSB_PERIPHERAL: new_mode = PHY_MODE_USB_DEVICE; break;
> + case MUSB_OTG: new_mode = PHY_MODE_USB_OTG; break;
Please fix the code style as commented in patch 4/7.
> + default:
> + dev_err(musb->controller->parent,
> + "Error requested mode not supported by this kernel\n");
> + return -EINVAL;
> + }
> +
> + if (glue->phy_mode == new_mode)
> + return 0;
> +
> + if (musb->port_mode != MUSB_PORT_MODE_DUAL_ROLE) {
> + dev_err(musb->controller->parent,
> + "Error changing modes is only supported in dual role
> mode\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * phy_set_mode may sleep, and we're called with a spinlock held,
> + * so let sunxi_musb_work deal with it.
> + */
> + glue->phy_mode = new_mode;
> + set_bit(SUNXI_MUSB_FL_PHY_MODE_PEND, &glue->flags);
> + schedule_work(&glue->work);
When switching from host to peripheral mode, if an usb device is still
plugged and enumerated, how do you handle the device disconnect?
Regards,
-Bin.
> +
> + return 0;
> +}
> +
> /*
> * sunxi musb register layout
> * 0x00 - 0x17 fifo regs, 1 long per fifo
> @@ -568,6 +608,7 @@ static const struct musb_platform_ops sunxi_musb_ops = {
> .writew = sunxi_musb_writew,
> .dma_init = sunxi_musb_dma_controller_create,
> .dma_exit = sunxi_musb_dma_controller_destroy,
> + .set_mode = sunxi_musb_set_mode,
> .set_vbus = sunxi_musb_set_vbus,
> .pre_root_reset_end = sunxi_musb_pre_root_reset_end,
> .post_root_reset_end = sunxi_musb_post_root_reset_end,
> @@ -614,21 +655,28 @@ static int sunxi_musb_probe(struct platform_device
> *pdev)
> return -EINVAL;
> }
>
> + glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
> + if (!glue)
> + return -ENOMEM;
> +
> memset(&pdata, 0, sizeof(pdata));
> switch (usb_get_dr_mode(&pdev->dev)) {
> #if defined CONFIG_USB_MUSB_DUAL_ROLE || defined CONFIG_USB_MUSB_HOST
> case USB_DR_MODE_HOST:
> pdata.mode = MUSB_PORT_MODE_HOST;
> + glue->phy_mode = PHY_MODE_USB_HOST;
> break;
> #endif
> #if defined CONFIG_USB_MUSB_DUAL_ROLE || defined CONFIG_USB_MUSB_GADGET
> case USB_DR_MODE_PERIPHERAL:
> pdata.mode = MUSB_PORT_MODE_GADGET;
> + glue->phy_mode = PHY_MODE_USB_DEVICE;
> break;
> #endif
> #ifdef CONFIG_USB_MUSB_DUAL_ROLE
> case USB_DR_MODE_OTG:
> pdata.mode = MUSB_PORT_MODE_DUAL_ROLE;
> + glue->phy_mode = PHY_MODE_USB_OTG;
> break;
> #endif
> default:
> @@ -638,10 +686,6 @@ static int sunxi_musb_probe(struct platform_device *pdev)
> pdata.platform_ops = &sunxi_musb_ops;
> pdata.config= &sunxi_musb_hdrc_config;
>
> - glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
> - if (
Re: [PATCH v2 6/7] phy-sun4i-usb: Add "allwinner,usb0-usb-a-connector" dt property
Hi,
On Mon, Aug 15, 2016 at 09:21:31PM +0200, Hans de Goede wrote:
> On some devices the musb otg controller can be used in both device and
> host mode, but requires software mode switching since there is no id pin
> connected. The usb0 phy code will default to device mode in this case.
>
> On some systems usb0 is connected to a female USB-A port. It can still
> be used in device mode when using software mode switching and a USB
> A to A cable. To configure the controller to support both modes we must
> set its "dr_mode" dt property to "otg" (*). For these setups the device
> mode default is wrong, for a female USB-A port the default should be
> host mode.
>
> This commit adds support to the sun4i phy code for a new
> "allwinner,usb0-usb-a-connector" dt property which can be used in dt
> files to indicate this scenario and when present it changes the default
> mode to host mode.
As I commented in patch 0/7, this dt prop is unneccesary.
Regards,
-Bin.
>
> *) dr_mode = "host" is used when device mode is _never_ used. E.g.
> a wifi module soldered onto the PCB is connected to the musb controller.
>
> Signed-off-by: Hans de Goede
> ---
> Changes in v2:
> -New patch in v2 of this patchset
> ---
> Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt | 2 ++
> drivers/phy/phy-sun4i-usb.c | 9 -
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> index 287150d..8646b53 100644
> --- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
> @@ -35,6 +35,8 @@ Optional properties:
> - usb0_vbus-supply : regulator phandle for controller usb0 vbus
> - usb1_vbus-supply : regulator phandle for controller usb1 vbus
> - usb2_vbus-supply : regulator phandle for controller usb2 vbus
> +- allwinner,usb0-usb-a-connector: usb0 is connected to an USB-A connector,
> + rather then an USB-B connector as one would expect
> (bool)
>
> Example:
> usbphy: phy@0x01c13400 {
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> index c17b099..82fb46a 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -137,6 +137,7 @@ struct sun4i_usb_phy_data {
> int vbus_det_irq;
> int id_det;
> int vbus_det;
> + int id_det_default;
> struct delayed_work detect;
> };
>
> @@ -328,7 +329,7 @@ static int sun4i_usb_phy0_get_id_det(struct
> sun4i_usb_phy_data *data)
> if (data->id_det_gpio)
> return gpiod_get_value_cansleep(data->id_det_gpio);
> else
> - return 1; /* Fallback to peripheral mode */
> + return data->id_det_default;
> case USB_DR_MODE_HOST:
> return 0;
> case USB_DR_MODE_PERIPHERAL:
> @@ -621,6 +622,12 @@ static int sun4i_usb_phy_probe(struct platform_device
> *pdev)
> if (IS_ERR(data->base))
> return PTR_ERR(data->base);
>
> + /* Set id-det default for when there is no id-det gpio */
> + if (of_property_read_bool(np, "allwinner,usb0-usb-a-connector"))
> + data->id_det_default = 0; /* Host (USB-A connector) */
> + else
> + data->id_det_default = 1; /* Device (USB-B connector) */
> +
> data->id_det_gpio = devm_gpiod_get_optional(dev, "usb0_id_det",
> GPIOD_IN);
> if (IS_ERR(data->id_det_gpio))
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Add PM runtime support for cppi41 DMA
Hi all, Here are two patches to add PM runtime support to cppi41 DMA. I've tested these on am335x using the musb dsps glue layer. If anybody has other test cases, please test if you can. Regards, Tony Tony Lindgren (2): dmaengine: cppi41: Prepare to add PM runtime support dmaengine: cppi41: Add basic PM runtime support drivers/dma/cppi41.c | 134 +++ 1 file changed, 114 insertions(+), 20 deletions(-) -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] dmaengine: cppi41: Prepare to add PM runtime support
Let's just move code from cppi41_dma_issue_pending() to
push_desc_queue() as that's the only call to push_desc_queue().
We want to do this for PM runtime as we need to call push_desc_queue()
also for pending queued transfers from PM runtime resume.
No functional changes, just moves code around.
Signed-off-by: Tony Lindgren
---
drivers/dma/cppi41.c | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 3b4c842..66b84fe 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -386,21 +386,6 @@ static void push_desc_queue(struct cppi41_channel *c)
u32 desc_phys;
u32 reg;
- desc_phys = lower_32_bits(c->desc_phys);
- desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
- WARN_ON(cdd->chan_busy[desc_num]);
- cdd->chan_busy[desc_num] = c;
-
- reg = (sizeof(struct cppi41_desc) - 24) / 4;
- reg |= desc_phys;
- cppi_writel(reg, cdd->qmgr_mem + QMGR_QUEUE_D(c->q_num));
-}
-
-static void cppi41_dma_issue_pending(struct dma_chan *chan)
-{
- struct cppi41_channel *c = to_cpp41_chan(chan);
- u32 reg;
-
c->residue = 0;
reg = GCR_CHAN_ENABLE;
@@ -418,6 +403,21 @@ static void cppi41_dma_issue_pending(struct dma_chan *chan)
* before starting the dma engine.
*/
__iowmb();
+
+ desc_phys = lower_32_bits(c->desc_phys);
+ desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
+ WARN_ON(cdd->chan_busy[desc_num]);
+ cdd->chan_busy[desc_num] = c;
+
+ reg = (sizeof(struct cppi41_desc) - 24) / 4;
+ reg |= desc_phys;
+ cppi_writel(reg, cdd->qmgr_mem + QMGR_QUEUE_D(c->q_num));
+}
+
+static void cppi41_dma_issue_pending(struct dma_chan *chan)
+{
+ struct cppi41_channel *c = to_cpp41_chan(chan);
+
push_desc_queue(c);
}
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] dmaengine: cppi41: Add basic PM runtime support
Let's keep the device enabled between cppi41_dma_issue_pending()
and dmaengine_desc_get_callback_invoke() and rely on the PM runtime
autoidle timeout elsewhere.
As the PM runtime is for whole device, not for each channel,
we need to queue pending transfers if the device is PM runtime
suspended. Then we start the pending transfers in PM runtime
resume.
Signed-off-by: Tony Lindgren
---
drivers/dma/cppi41.c | 104 ---
1 file changed, 99 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 66b84fe..ce8739f 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -108,6 +108,8 @@ struct cppi41_channel {
unsigned td_queued:1;
unsigned td_seen:1;
unsigned td_desc_seen:1;
+
+ struct list_head node; /* Node for pending list */
};
struct cppi41_desc {
@@ -146,6 +148,9 @@ struct cppi41_dd {
const struct chan_queues *queues_tx;
struct chan_queues td_queue;
+ struct list_head pending; /* Pending queued transfers */
+ spinlock_t lock;/* Lock for pending list */
+
/* context for suspend/resume */
unsigned int dma_tdfdq;
};
@@ -332,6 +337,10 @@ static irqreturn_t cppi41_irq(int irq, void *data)
c->residue = pd_trans_len(c->desc->pd6) - len;
dma_cookie_complete(&c->txd);
dmaengine_desc_get_callback_invoke(&c->txd, NULL);
+
+ /* Paired with cppi41_dma_issue_pending */
+ pm_runtime_mark_last_busy(cdd->ddev.dev);
+ pm_runtime_put_autosuspend(cdd->ddev.dev);
}
}
return IRQ_HANDLED;
@@ -349,6 +358,12 @@ static dma_cookie_t cppi41_tx_submit(struct
dma_async_tx_descriptor *tx)
static int cppi41_dma_alloc_chan_resources(struct dma_chan *chan)
{
struct cppi41_channel *c = to_cpp41_chan(chan);
+ struct cppi41_dd *cdd = c->cdd;
+ int error;
+
+ error = pm_runtime_get_sync(cdd->ddev.dev);
+ if (error < 0)
+ return error;
dma_cookie_init(chan);
dma_async_tx_descriptor_init(&c->txd, chan);
@@ -357,11 +372,26 @@ static int cppi41_dma_alloc_chan_resources(struct
dma_chan *chan)
if (!c->is_tx)
cppi_writel(c->q_num, c->gcr_reg + RXHPCRA0);
+ pm_runtime_mark_last_busy(cdd->ddev.dev);
+ pm_runtime_put_autosuspend(cdd->ddev.dev);
+
return 0;
}
static void cppi41_dma_free_chan_resources(struct dma_chan *chan)
{
+ struct cppi41_channel *c = to_cpp41_chan(chan);
+ struct cppi41_dd *cdd = c->cdd;
+ int error;
+
+ error = pm_runtime_get_sync(cdd->ddev.dev);
+ if (error < 0)
+ return;
+
+ WARN_ON(!list_empty(&cdd->pending));
+
+ pm_runtime_mark_last_busy(cdd->ddev.dev);
+ pm_runtime_put_autosuspend(cdd->ddev.dev);
}
static enum dma_status cppi41_dma_tx_status(struct dma_chan *chan,
@@ -414,11 +444,35 @@ static void push_desc_queue(struct cppi41_channel *c)
cppi_writel(reg, cdd->qmgr_mem + QMGR_QUEUE_D(c->q_num));
}
+static void pending_desc(struct cppi41_channel *c)
+{
+ struct cppi41_dd *cdd = c->cdd;
+ unsigned long flags;
+
+ spin_lock_irqsave(&cdd->lock, flags);
+ list_add_tail(&c->node, &cdd->pending);
+ spin_unlock_irqrestore(&cdd->lock, flags);
+}
+
static void cppi41_dma_issue_pending(struct dma_chan *chan)
{
struct cppi41_channel *c = to_cpp41_chan(chan);
+ struct cppi41_dd *cdd = c->cdd;
+ int error;
+
+ /* PM runtime paired with dmaengine_desc_get_callback_invoke */
+ error = pm_runtime_get(cdd->ddev.dev);
+ if (error < 0) {
+ dev_err(cdd->ddev.dev, "Failed to pm_runtime_get: %i\n",
+ error);
- push_desc_queue(c);
+ return;
+ }
+
+ if (likely(pm_runtime_active(cdd->ddev.dev)))
+ push_desc_queue(c);
+ else
+ pending_desc(c);
}
static u32 get_host_pd0(u32 length)
@@ -940,12 +994,18 @@ static int cppi41_dma_probe(struct platform_device *pdev)
cdd->ctrl_mem = of_iomap(dev->of_node, 1);
cdd->sched_mem = of_iomap(dev->of_node, 2);
cdd->qmgr_mem = of_iomap(dev->of_node, 3);
+ spin_lock_init(&cdd->lock);
+ INIT_LIST_HEAD(&cdd->pending);
+
+ platform_set_drvdata(pdev, cdd);
if (!cdd->usbss_mem || !cdd->ctrl_mem || !cdd->sched_mem ||
!cdd->qmgr_mem)
return -ENXIO;
pm_runtime_enable(dev);
+ pm_runtime_set_autosuspend_delay(dev, 100);
+ pm_runtime_use_autosuspend(dev);
ret = pm_runtime_get_sync(dev);
if (ret < 0)
goto err_get_sync;
@@ -985,7 +1045,9 @@ static int cppi41_dma_probe(struct platform_device *pdev)
if (ret)
goto err_of;
- p
cdc_acm bug? read buffer bytes shifted
Hi, I have noticed a problem using a usb device managed by the cdc_acm driver. The data received from the device and copied to userspace ends up being shifted by one byte again and again after some amount of calls to read() and most importantly with previously read data. usbmon shows the usb data payload is correct. A small shell script is enough to make it happen fastly: https://gist.github.com/Julio-Guerra/b6529994f814771c825649bdb8d927c2 It prints the buffer read with my device (a relay) and gives me things like: > 00 01 01 01 00 01 00 01 ... > 01 00 01 01 01 00 01 00 ... > 00 01 00 01 01 01 00 01 after a random number of times, while usbmon shows the usb payload "00 01 01 01 00 01 00 01" Another example: > 00 00 00 00 00 00 00 01 ... > 01 00 00 00 00 00 00 00 ... > 00 01 00 00 00 00 00 00 ... > 00 00 01 00 00 00 00 00 after a random number of times, while usbmon shows the usb payload "00 00 00 00 00 00 00 01". Any idea? I currently use versions 4.5.0 and 4.7.1 to execute this script. Thanks you -- Julio Guerra -- 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
