Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling
Hi,
Peter Chen writes:
> On Tue, Nov 01, 2016 at 01:29:59PM +0200, Felipe Balbi wrote:
>> According to Dave Miller "the networking stack has a
>> hard requirement that all SKBs which are transmitted
>> must have their completion signalled in a fininte
>> amount of time. This is because, until the SKB is
>> freed by the driver, it holds onto socket,
>> netfilter, and other subsystem resources."
>>
>> In summary, this means that using TX IRQ throttling
>> for the networking gadgets is, at least, complex and
>> we should avoid it for the time being.
>>
>> Cc:
>> Reported-by: Ville Syrjälä
>> Suggested-by: David Miller
>> Signed-off-by: Felipe Balbi
>> ---
>> drivers/usb/gadget/function/u_ether.c | 8
>> 1 file changed, 8 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/u_ether.c
>> b/drivers/usb/gadget/function/u_ether.c
>> index f4a640216913..119a2e5848e8 100644
>> --- a/drivers/usb/gadget/function/u_ether.c
>> +++ b/drivers/usb/gadget/function/u_ether.c
>> @@ -589,14 +589,6 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
>>
>> req->length = length;
>>
>> -/* throttle high/super speed IRQ rate back slightly */
>> -if (gadget_is_dualspeed(dev->gadget))
>> -req->no_interrupt = (((dev->gadget->speed == USB_SPEED_HIGH ||
>> - dev->gadget->speed == USB_SPEED_SUPER))
>> &&
>> -!list_empty(&dev->tx_reqs))
>> -? ((atomic_read(&dev->tx_qlen) % dev->qmult) != 0)
>> -: 0;
>> -
>> retval = usb_ep_queue(in, req, GFP_ATOMIC);
>> switch (retval) {
>> default:
>> --
>
> Felipe, it may increase cpu utilization since more interrupts will be there,
> it may affect the SoC which has lower cpu frequency. This code existed
> many years, why this problem has only reported at dwc3 recently?
No idea, but at least for networking gadgets we shouldn't throttle. This
has been a bug since the beginning. Read Dave Miller's explanation at
[1]
moreover, dwc3 seems to be the only one actually throttling IRQ. Here's
a rundown of a few of the UDCs:
- chipidea: uses TD_IOC conditionally, but always sets TD_TERMINATE
lastnode->ptr->next = cpu_to_le32(TD_TERMINATE);
if (!hwreq->req.no_interrupt)
lastnode->ptr->token |= cpu_to_le32(TD_IOC);
I'm guessing TD_TERMINATE works similar to dwc3's LST bit. If
it's set, it will force an interrupt.
- musb: no_interrupt only used for tracing
- atmel_usba_udc: no_interrupt only used for tracing
- mv_u3d_core: probably throttles interrupt, but probably exhibits same
behavior. It's just that it hasn't been reported.
- fsl_udc_core: probably throttles interrupt, but probably exhibits same
behavior. It's just that it hasn't been reported.
and there are the only uses for the no_interrupt flag.
[1] https://marc.info/?l=linux-usb&m=147793992922064&w=2
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling
On Wed, Nov 02, 2016 at 09:55:44AM +0200, Felipe Balbi wrote:
>
> Hi,
>
> Peter Chen writes:
> > On Tue, Nov 01, 2016 at 01:29:59PM +0200, Felipe Balbi wrote:
> >> According to Dave Miller "the networking stack has a
> >> hard requirement that all SKBs which are transmitted
> >> must have their completion signalled in a fininte
> >> amount of time. This is because, until the SKB is
> >> freed by the driver, it holds onto socket,
> >> netfilter, and other subsystem resources."
> >>
> >> In summary, this means that using TX IRQ throttling
> >> for the networking gadgets is, at least, complex and
> >> we should avoid it for the time being.
> >>
> >> Cc:
> >> Reported-by: Ville Syrjälä
> >> Suggested-by: David Miller
> >> Signed-off-by: Felipe Balbi
> >> ---
> >> drivers/usb/gadget/function/u_ether.c | 8
> >> 1 file changed, 8 deletions(-)
> >>
> >> diff --git a/drivers/usb/gadget/function/u_ether.c
> >> b/drivers/usb/gadget/function/u_ether.c
> >> index f4a640216913..119a2e5848e8 100644
> >> --- a/drivers/usb/gadget/function/u_ether.c
> >> +++ b/drivers/usb/gadget/function/u_ether.c
> >> @@ -589,14 +589,6 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
> >>
> >>req->length = length;
> >>
> >> - /* throttle high/super speed IRQ rate back slightly */
> >> - if (gadget_is_dualspeed(dev->gadget))
> >> - req->no_interrupt = (((dev->gadget->speed == USB_SPEED_HIGH ||
> >> - dev->gadget->speed == USB_SPEED_SUPER))
> >> &&
> >> - !list_empty(&dev->tx_reqs))
> >> - ? ((atomic_read(&dev->tx_qlen) % dev->qmult) != 0)
> >> - : 0;
> >> -
> >>retval = usb_ep_queue(in, req, GFP_ATOMIC);
> >>switch (retval) {
> >>default:
> >> --
> >
> > Felipe, it may increase cpu utilization since more interrupts will be there,
> > it may affect the SoC which has lower cpu frequency. This code existed
> > many years, why this problem has only reported at dwc3 recently?
>
> No idea, but at least for networking gadgets we shouldn't throttle. This
> has been a bug since the beginning. Read Dave Miller's explanation at
> [1]
>
> moreover, dwc3 seems to be the only one actually throttling IRQ. Here's
> a rundown of a few of the UDCs:
>
> - chipidea: uses TD_IOC conditionally, but always sets TD_TERMINATE
>
> lastnode->ptr->next = cpu_to_le32(TD_TERMINATE);
> if (!hwreq->req.no_interrupt)
> lastnode->ptr->token |= cpu_to_le32(TD_IOC);
>
> I'm guessing TD_TERMINATE works similar to dwc3's LST bit. If
> it's set, it will force an interrupt.
No, TD_TERMINATE just stands for it is the last TD, and this pointer will
be updated when the new request is added. The interrupt is only triggered
by IOC (Interrupt On Complete) bit at TD.
I am not sure if dwc3 supports ITC (Interrupt Threshold Control)
software control, it is an EHCI compliant register entry, and
the device mode is supported for chipidea too. It is a timeout
mechanism from controller side for pending requests.
The interrupt will be triggered either the request has completed for TD
which IOC bit is set or the ITC is fired (125us currently) and the
request has completed, so the problem David described should not exist,
at least for chipidea.
If DWC3 has similar ITC bits, would you try to tune it? The default ITC
value for chipidea is not enough, and we tuned it before.
>
> - musb: no_interrupt only used for tracing
>
> - atmel_usba_udc: no_interrupt only used for tracing
>
> - mv_u3d_core: probably throttles interrupt, but probably exhibits same
> behavior. It's just that it hasn't been reported.
>
> - fsl_udc_core: probably throttles interrupt, but probably exhibits same
> behavior. It's just that it hasn't been reported.
The above two uses chipidea IP core too.
--
Best Regards,
Peter Chen
--
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 3/6] usb: ehci: fsl: use bus->sysdev for DMA configuration
Hi Arnd,
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.9-rc3 next-20161028]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
[Suggest to use git(>=2.9.0) format-patch --base= (or --base=auto for
convenience) to record what (public, well-known) commit your patch series was
built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url:
https://github.com/0day-ci/linux/commits/Sriram-Dash/inherit-dma-configuration-from-parent-dev/20161102-141353
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
usb-testing
config: powerpc-mpc8313_rdb_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
drivers/usb/host/ehci-fsl.c: In function 'fsl_ehci_drv_probe':
>> drivers/usb/host/ehci-fsl.c:99:56: error: invalid type argument of '->'
>> (have 'struct device')
hcd = __usb_create_hcd(&fsl_ehci_hc_driver, &pdev->dev->parent,
^~
vim +99 drivers/usb/host/ehci-fsl.c
93 "Found HC with no IRQ. Check %s setup!\n",
94 dev_name(&pdev->dev));
95 return -ENODEV;
96 }
97 irq = res->start;
98
> 99 hcd = __usb_create_hcd(&fsl_ehci_hc_driver, &pdev->dev->parent,
100 &pdev->dev, dev_name(&pdev->dev), NULL);
101 if (!hcd) {
102 retval = -ENOMEM;
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling
Hi,
Peter Chen writes:
>> >> diff --git a/drivers/usb/gadget/function/u_ether.c
>> >> b/drivers/usb/gadget/function/u_ether.c
>> >> index f4a640216913..119a2e5848e8 100644
>> >> --- a/drivers/usb/gadget/function/u_ether.c
>> >> +++ b/drivers/usb/gadget/function/u_ether.c
>> >> @@ -589,14 +589,6 @@ static netdev_tx_t eth_start_xmit(struct sk_buff
>> >> *skb,
>> >>
>> >> req->length = length;
>> >>
>> >> - /* throttle high/super speed IRQ rate back slightly */
>> >> - if (gadget_is_dualspeed(dev->gadget))
>> >> - req->no_interrupt = (((dev->gadget->speed == USB_SPEED_HIGH ||
>> >> -dev->gadget->speed == USB_SPEED_SUPER))
>> >> &&
>> >> - !list_empty(&dev->tx_reqs))
>> >> - ? ((atomic_read(&dev->tx_qlen) % dev->qmult) != 0)
>> >> - : 0;
>> >> -
>> >> retval = usb_ep_queue(in, req, GFP_ATOMIC);
>> >> switch (retval) {
>> >> default:
>> >> --
>> >
>> > Felipe, it may increase cpu utilization since more interrupts will be
>> > there,
>> > it may affect the SoC which has lower cpu frequency. This code existed
>> > many years, why this problem has only reported at dwc3 recently?
>>
>> No idea, but at least for networking gadgets we shouldn't throttle. This
>> has been a bug since the beginning. Read Dave Miller's explanation at
>> [1]
>>
>> moreover, dwc3 seems to be the only one actually throttling IRQ. Here's
>> a rundown of a few of the UDCs:
>>
>> - chipidea: uses TD_IOC conditionally, but always sets TD_TERMINATE
>>
>> lastnode->ptr->next = cpu_to_le32(TD_TERMINATE);
>> if (!hwreq->req.no_interrupt)
>> lastnode->ptr->token |= cpu_to_le32(TD_IOC);
>>
>> I'm guessing TD_TERMINATE works similar to dwc3's LST bit. If
>> it's set, it will force an interrupt.
>
> No, TD_TERMINATE just stands for it is the last TD, and this pointer will
> be updated when the new request is added. The interrupt is only triggered
> by IOC (Interrupt On Complete) bit at TD.
>
> I am not sure if dwc3 supports ITC (Interrupt Threshold Control)
> software control, it is an EHCI compliant register entry, and
> the device mode is supported for chipidea too. It is a timeout
> mechanism from controller side for pending requests.
>
> The interrupt will be triggered either the request has completed for TD
> which IOC bit is set or the ITC is fired (125us currently) and the
> request has completed, so the problem David described should not exist,
> at least for chipidea.
In other words, you don't *really* throttle interrupt as they'll fire
after the micro-frame expires :-p
> If DWC3 has similar ITC bits, would you try to tune it? The default ITC
> value for chipidea is not enough, and we tuned it before.
there's no such thing in dwc3
>> - musb: no_interrupt only used for tracing
>>
>> - atmel_usba_udc: no_interrupt only used for tracing
>>
>> - mv_u3d_core: probably throttles interrupt, but probably exhibits same
>> behavior. It's just that it hasn't been reported.
>>
>> - fsl_udc_core: probably throttles interrupt, but probably exhibits same
>> behavior. It's just that it hasn't been reported.
>
> The above two uses chipidea IP core too.
so why do we still have these drivers in tree? Might as well remove them
already.
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH V11 1/1] usb:serial: Add Fintek F81532/534 driver
On Fri, Oct 14, 2016 at 04:20:46PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> This driver is for Fintek F81532/F81534 USB to Serial Ports IC.
>
> F81532 spec:
> https://drive.google.com/file/d/0B8vRwwYO7aMFOTRRMmhWQVNvajQ/view?usp=
> sharing
>
> F81534 spec:
> https://drive.google.com/file/d/0B8vRwwYO7aMFV29pQWJqbVBNc00/view?usp=
> sharing
>
> Features:
> 1. F81532 is 1-to-2 & F81534 is 1-to-4 serial ports IC
> 2. Support Baudrate from B50 to B115200.
>
> Reviewed-by: Johan Hovold
You must never add other peoples' Reviewed-by tags unless you've
explicitly been given permission to do so (e.g. "fix this minor thing up
and then you can add...").
Please make sure to read the section about Reviewed-by tags in
Documentation/SubmittingPatches.
> Signed-off-by: Ji-Ze Hong (Peter Hong)
> ---
> Changelog:
> V11
> 1. Reduce F81534_MAX_BUS_RETRY from 2000 to 20. We are only using
>internal SPI bus to read flash when attach() & calc_num_ports()
>and never read flash when the F81532/534 in full loading, so we
>can reduce retry count.
Does this mean you can remove that retry mechanism completely?
> 2. Modify attach() & calc_num_ports() read default value only when
>can't find the custom setting.
> 3. Change tx_empty protect method from spinlock to set_bit()/
>clear_bit()/test_and_clear_bit().
> 4. Move calculate tty_idx[i] from port_probe() to attach().
> 5. Add f81534_tx_empty()
> +#define DRIVER_DESC "Fintek F81532/F81534"
> +#define FINTEK_VENDOR_ID_1 0x1934
> +#define FINTEK_VENDOR_ID_2 0x2C42
> +#define FINTEK_DEVICE_ID 0x1202
> +#define F81534_MAX_TX_SIZE 100
You never replies to my question about why this is not 124 as for rx.
> +#define F81534_MAX_RX_SIZE 124
> +#define F81534_RECEIVE_BLOCK_SIZE128
> +
> +#define F81534_TOKEN_RECEIVE 0x01
> +#define F81534_TOKEN_WRITE 0x02
> +#define F81534_TOKEN_TX_EMPTY0x03
> +#define F81534_TOKEN_MSR_CHANGE 0x04
> +
> +/*
> + * We used interal SPI bus to access FLASH section. We must wait the SPI bus
> to
> + * idle if we performed any command.
> + *
> + * SPI Bus status register: F81534_BUS_REG_STATUS
> + * Bit 0/1 : BUSY
> + * Bit 2 : IDLE
> + */
> +#define F81534_BUS_BUSY (BIT(0) | BIT(1))
> +#define F81534_BUS_IDLE BIT(2)
> +#define F81534_BUS_READ_DATA 0x1004
> +#define F81534_BUS_REG_STATUS0x1003
> +#define F81534_BUS_REG_START 0x1002
> +#define F81534_BUS_REG_END 0x1001
> +
> +#define F81534_CMD_READ 0x03
> +
> +#define F81534_DEFAULT_BAUD_RATE 9600
> +#define F81534_MAX_BAUDRATE 115200
> +
> +#define F81534_PORT_CONF_DISABLE_PORTBIT(3)
> +#define F81534_PORT_CONF_NOT_EXIST_PORT BIT(7)
> +#define F81534_PORT_UNAVAILABLE \
> + (F81534_PORT_CONF_DISABLE_PORT | F81534_PORT_CONF_NOT_EXIST_PORT)
> +
> +#define F81534_1X_RXTRIGGER 0xc3
> +#define F81534_8X_RXTRIGGER 0xcf
> +
> +static const struct usb_device_id f81534_id_table[] = {
> + {USB_DEVICE(FINTEK_VENDOR_ID_1, FINTEK_DEVICE_ID)},
> + {USB_DEVICE(FINTEK_VENDOR_ID_2, FINTEK_DEVICE_ID)},
Add a space after { and before }.
> + {} /* Terminating entry */
> +};
> +
> +#define F81534_TX_EMPTY_BIT 0
> +
> +struct f81534_serial_private {
> + u8 conf_data[F81534_DEF_CONF_SIZE];
> + int tty_idx[F81534_NUM_PORT];
> + u8 setting_idx;
> + int opened_port;
> + struct mutex urb_mutex;
> +};
> +
> +struct f81534_port_private {
> + struct mutex mcr_mutex;
> + unsigned long tx_empty;
> + spinlock_t msr_lock;
> + u8 shadow_mcr;
> + u8 shadow_msr;
> + u8 phy_num;
> +};
> +static int f81534_set_normal_register(struct usb_serial *serial, u16 reg,
What do mean by "normal" here? Could you give this a more descriptive
name?
Perhaps just call this one f81534_set_register and add a "port" or
"uart" infix to the current f81534_set_register below (e.g. rename it
f81534_set_uart_register, and similar for get).
Or simply replace "normal" with "generic" above.
> + u8 data)
> +{
> + struct usb_interface *interface = serial->interface;
> + struct usb_device *dev = serial->dev;
> + size_t count = F81534_USB_MAX_RETRY;
> + int status;
> + u8 *tmp;
> +
> + tmp = kmalloc(sizeof(u8), GFP_KERNEL);
> + if (!tmp)
> + return -ENOMEM;
> +
> + *tmp = data;
> +
> + /*
> + * Our device maybe not reply when heavily loading, We'll retry for
> + * F81534_USB_MAX_RETRY times.
> + */
> + while (count--) {
> + status = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
> + F81534_SET_GET_REGISTER,
> + USB_TYPE_VENDOR | USB_DIR_OUT,
> +
[PATCH 3/3] usb: ohci-da8xx: rename driver to ohci-da8xx
To be consistent on the usb driver for the davinci platform follow the example of musb, and add the "-da8xx" postfix to the driver name. Signed-off-by: Axel Haslam --- drivers/usb/host/ohci-da8xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c index bd6cf3c..b3de8bc 100644 --- a/drivers/usb/host/ohci-da8xx.c +++ b/drivers/usb/host/ohci-da8xx.c @@ -27,7 +27,7 @@ #include "ohci.h" #define DRIVER_DESC "DA8XX" -#define DRV_NAME "ohci" +#define DRV_NAME "ohci-da8xx" static struct hc_driver __read_mostly ohci_da8xx_hc_driver; -- 2.10.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] fix ohci phy name
The usb ohci clock match is not working because the usb clock is registered as "ohci" instead of "ohci.0" But since there is only a single ohci instance, lets pass -1 to the platform data id parameter and avoid the extra ".0" matching. while we are fixing this, rename the driver to "ohci-da8xx" to be consistent with davinci musb and other usb drivers. Axel Haslam (3): ARM: davinci: da8xx: Fix ohci driver name phy: da8xx-usb: rename the ohci device to ohci-da8xx usb: ohci-da8xx: rename driver to ohci-da8xx arch/arm/mach-davinci/da830.c | 2 +- arch/arm/mach-davinci/da850.c | 2 +- arch/arm/mach-davinci/da8xx-dt.c | 2 +- arch/arm/mach-davinci/usb-da8xx.c | 4 ++-- drivers/phy/phy-da8xx-usb.c | 5 +++-- drivers/usb/host/ohci-da8xx.c | 2 +- 6 files changed, 9 insertions(+), 8 deletions(-) -- 2.10.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] phy: da8xx-usb: rename the ohci device to ohci-da8xx
There is only one ohci on the da8xx series of chips,
so remove the ".0" when creating the phy. Also add
the "-da8xx" postfix to be consistent across davinci
usb drivers.
Signed-off-by: Axel Haslam
---
drivers/phy/phy-da8xx-usb.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/phy-da8xx-usb.c b/drivers/phy/phy-da8xx-usb.c
index 32ae78c..c85fb0b 100644
--- a/drivers/phy/phy-da8xx-usb.c
+++ b/drivers/phy/phy-da8xx-usb.c
@@ -198,7 +198,8 @@ static int da8xx_usb_phy_probe(struct platform_device *pdev)
} else {
int ret;
- ret = phy_create_lookup(d_phy->usb11_phy, "usb-phy", "ohci.0");
+ ret = phy_create_lookup(d_phy->usb11_phy, "usb-phy",
+ "ohci-da8xx");
if (ret)
dev_warn(dev, "Failed to create usb11 phy lookup\n");
ret = phy_create_lookup(d_phy->usb20_phy, "usb-phy",
@@ -216,7 +217,7 @@ static int da8xx_usb_phy_remove(struct platform_device
*pdev)
if (!pdev->dev.of_node) {
phy_remove_lookup(d_phy->usb20_phy, "usb-phy", "musb-da8xx");
- phy_remove_lookup(d_phy->usb11_phy, "usb-phy", "ohci.0");
+ phy_remove_lookup(d_phy->usb11_phy, "usb-phy", "ohci-da8xx");
}
return 0;
--
2.10.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] ARM: davinci: da8xx: Fix ohci driver name
There is a single instance of the ohci driver,
while the clk lookup table is making reference to "ohci"
other subsystems (such as phy) are looking for "ohci.0"
Since there is a single ohci instance, change the dev id
to -1, and add the "-da8xx" for consitancy with the musb
driver name.
Signed-off-by: Axel Haslam
---
arch/arm/mach-davinci/da830.c | 2 +-
arch/arm/mach-davinci/da850.c | 2 +-
arch/arm/mach-davinci/da8xx-dt.c | 2 +-
arch/arm/mach-davinci/usb-da8xx.c | 4 ++--
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
index 41459bd..073c458 100644
--- a/arch/arm/mach-davinci/da830.c
+++ b/arch/arm/mach-davinci/da830.c
@@ -420,7 +420,7 @@ static struct clk_lookup da830_clks[] = {
CLK("davinci_mdio.0", "fck", &emac_clk),
CLK(NULL, "gpio", &gpio_clk),
CLK("i2c_davinci.2",NULL, &i2c1_clk),
- CLK("ohci", "usb11",&usb11_clk),
+ CLK("ohci-da8xx", "usb11",&usb11_clk),
CLK(NULL, "emif3",&emif3_clk),
CLK(NULL, "arm", &arm_clk),
CLK(NULL, "rmii", &rmii_clk),
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 196e262..3961556 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -503,7 +503,7 @@ static struct clk_lookup da850_clks[] = {
CLK("da830-mmc.1", NULL, &mmcsd1_clk),
CLK("ti-aemif", NULL, &aemif_clk),
CLK(NULL, "aemif",&aemif_clk),
- CLK("ohci", "usb11",&usb11_clk),
+ CLK("ohci-da8xx", "usb11",&usb11_clk),
CLK("musb-da8xx", "usb20",&usb20_clk),
CLK("spi_davinci.0",NULL, &spi0_clk),
CLK("spi_davinci.1",NULL, &spi1_clk),
diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index 92ae093..2afb067 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -39,7 +39,7 @@ static struct of_dev_auxdata da850_auxdata_lookup[]
__initdata = {
OF_DEV_AUXDATA("ti,da830-mcasp-audio", 0x01d0, "davinci-mcasp.0",
NULL),
OF_DEV_AUXDATA("ti,da850-aemif", 0x6800, "ti-aemif", NULL),
OF_DEV_AUXDATA("ti,da850-tilcdc", 0x01e13000, "da8xx_lcdc.0", NULL),
- OF_DEV_AUXDATA("ti,da830-ohci", 0x01e25000, "ohci", NULL),
+ OF_DEV_AUXDATA("ti,da830-ohci", 0x01e25000, "ohci-da8xx", NULL),
OF_DEV_AUXDATA("ti,da830-musb", 0x01e0, "musb-da8xx", NULL),
OF_DEV_AUXDATA("ti,da830-usb-phy", 0x01c1417c, "da8xx-usb-phy", NULL),
{}
diff --git a/arch/arm/mach-davinci/usb-da8xx.c
b/arch/arm/mach-davinci/usb-da8xx.c
index b010e5f..c6feecf 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -109,8 +109,8 @@ static struct resource da8xx_usb11_resources[] = {
static u64 da8xx_usb11_dma_mask = DMA_BIT_MASK(32);
static struct platform_device da8xx_usb11_device = {
- .name = "ohci",
- .id = 0,
+ .name = "ohci-da8xx",
+ .id = -1,
.dev = {
.dma_mask = &da8xx_usb11_dma_mask,
.coherent_dma_mask = DMA_BIT_MASK(32),
--
2.10.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] phy: rcar-gen3-usb2: add sysfs for usb role swap
This patch adds sysfs "role" for usb role swap. This parameter can be
read and write. If you use this file as the following, you can swap
the usb role.
For example:
1) Connect a usb cable using 2 Salvator-x boards
2) On A-Device (ID pin is low), you input the following command:
# echo peripheral > /sys/devices/platform/soc/ee080200.usb-phy/role
3) On B-Device (ID pin is high), you input the following command:
# echo host > /sys/devices/platform/soc/ee080200.usb-phy/role
Then, the A-device acts as a peripheral and the B-device acts as a host.
Please note that A-Device must input the following command if you
want the board to act as a host again. (even if you disconnect the usb
cable, since id state may be the same, the A-Device keeps to act as
peripheral.)
# echo host > /sys/devices/platform/soc/ee080200.usb-phy/role
Signed-off-by: Yoshihiro Shimoda
---
This patch is based on the latest linux-phy.git / next branch.
(commit id = 7809cd2ce6abd4f431e4b14e6b1276a7cc842ac4)
Since this patch is related to usb, I added email addresses of Greg, Felipe,
Peter and USB ML as CC. (This patch doesn't use USB OTG FSM though.)
Changes from v2:
- Modify the sysfs file name to "role", and the argument is "host" or
"peripheral". Peter suggested this. Thank you!
Changes from v1:
- rebase the latest next branch.
.../ABI/testing/sysfs-platform-phy-rcar-gen3-usb2 | 15 +++
drivers/phy/phy-rcar-gen3-usb2.c | 118 -
2 files changed, 132 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ABI/testing/sysfs-platform-phy-rcar-gen3-usb2
diff --git a/Documentation/ABI/testing/sysfs-platform-phy-rcar-gen3-usb2
b/Documentation/ABI/testing/sysfs-platform-phy-rcar-gen3-usb2
new file mode 100644
index 000..6212697
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-phy-rcar-gen3-usb2
@@ -0,0 +1,15 @@
+What: /sys/devices/platform//role
+Date: October 2016
+KernelVersion: 4.10
+Contact: Yoshihiro Shimoda
+Description:
+ This file can be read and write.
+ The file can show/change the phy mode for role swap of usb.
+
+ Write the following strings to change the mode:
+"host" - switching mode from peripheral to host.
+"peripheral" - switching mode from host to peripheral.
+
+ Read the file, then it shows the following strings:
+"host" - The mode is host now.
+"peripheral" - The mode is peripheral now.
diff --git a/drivers/phy/phy-rcar-gen3-usb2.c b/drivers/phy/phy-rcar-gen3-usb2.c
index 3d97ead..cfc9956 100644
--- a/drivers/phy/phy-rcar-gen3-usb2.c
+++ b/drivers/phy/phy-rcar-gen3-usb2.c
@@ -70,6 +70,7 @@
#define USB2_LINECTRL1_DP_RPD BIT(18)
#define USB2_LINECTRL1_DMRPD_ENBIT(17)
#define USB2_LINECTRL1_DM_RPD BIT(16)
+#define USB2_LINECTRL1_OPMODE_NODRVBIT(6)
/* ADPCTRL */
#define USB2_ADPCTRL_OTGSESSVLDBIT(20)
@@ -161,6 +162,43 @@ static void rcar_gen3_init_for_peri(struct rcar_gen3_chan
*ch)
schedule_work(&ch->work);
}
+static void rcar_gen3_init_for_b_host(struct rcar_gen3_chan *ch)
+{
+ void __iomem *usb2_base = ch->base;
+ u32 val;
+
+ val = readl(usb2_base + USB2_LINECTRL1);
+ writel(val | USB2_LINECTRL1_OPMODE_NODRV, usb2_base + USB2_LINECTRL1);
+
+ rcar_gen3_set_linectrl(ch, 1, 1);
+ rcar_gen3_set_host_mode(ch, 1);
+ rcar_gen3_enable_vbus_ctrl(ch, 0);
+
+ val = readl(usb2_base + USB2_LINECTRL1);
+ writel(val & ~USB2_LINECTRL1_OPMODE_NODRV, usb2_base + USB2_LINECTRL1);
+}
+
+static void rcar_gen3_init_for_a_peri(struct rcar_gen3_chan *ch)
+{
+ rcar_gen3_set_linectrl(ch, 0, 1);
+ rcar_gen3_set_host_mode(ch, 0);
+ rcar_gen3_enable_vbus_ctrl(ch, 1);
+}
+
+static void rcar_gen3_init_from_a_peri_to_a_host(struct rcar_gen3_chan *ch)
+{
+ void __iomem *usb2_base = ch->base;
+ u32 val;
+
+ val = readl(usb2_base + USB2_OBINTEN);
+ writel(val & ~USB2_OBINT_BITS, usb2_base + USB2_OBINTEN);
+
+ rcar_gen3_enable_vbus_ctrl(ch, 0);
+ rcar_gen3_init_for_host(ch);
+
+ writel(val | USB2_OBINT_BITS, usb2_base + USB2_OBINTEN);
+}
+
static bool rcar_gen3_check_id(struct rcar_gen3_chan *ch)
{
return !!(readl(ch->base + USB2_ADPCTRL) & USB2_ADPCTRL_IDDIG);
@@ -174,6 +212,65 @@ static void rcar_gen3_device_recognition(struct
rcar_gen3_chan *ch)
rcar_gen3_init_for_peri(ch);
}
+static bool rcar_gen3_is_host(struct rcar_gen3_chan *ch)
+{
+ return !(readl(ch->base + USB2_COMMCTRL) & USB2_COMMCTRL_OTG_PERI);
+}
+
+static ssize_t role_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct rcar_gen3_chan *ch = dev_get_drvdata(dev);
+ bool is_b_device, is_host, new_mode_is_host;
+
+ if (!ch->has_otg || !ch->phy->init_count)
+
[PATCH] cdc-acm: fix uninitialized variable
variable struct usb_cdc_parsed_header h may be used
uninitialized in acm_probe.
In kernel 4.8.
/* handle quirks deadly to normal probing*/
if (quirks == NO_UNION_NORMAL)
...
goto skip_normal_probe;
}
we bypass call to
cdc_parse_cdc_header(&h, intf, buffer, buflen);
but later use h in
if (h.usb_cdc_country_functional_desc) { /* export the country data */
Signed-off-by: Oliver Neukum
CC: sta...@vger.kernel.org
Reported-by: Victor Sologoubov
---
drivers/usb/class/cdc-acm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 78f0f85..4ad4ca4 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1161,6 +1161,8 @@ static int acm_probe(struct usb_interface *intf,
if (quirks == IGNORE_DEVICE)
return -ENODEV;
+ memset(&h, 0x00, sizeof(struct usb_cdc_parsed_header));
+
num_rx_buf = (quirks == SINGLE_RX_URB) ? 1 : ACM_NR;
/* handle quirks deadly to normal probing*/
--
2.6.2
--
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: Bug in cdc-acm: variable struct usb_cdc_parsed_header h may be used uninitialized in acm_probe.
On Sat, 2016-10-29 at 15:48 +0300, Victor Sologoubov wrote: > Bug in cdc-acm: variable struct usb_cdc_parsed_header h may be used > uninitialized in acm_probe. Hi, good catch. Apatch with credits to you has been submitted. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling
From: Peter Chen Date: Wed, 2 Nov 2016 14:02:02 +0800 > Felipe, it may increase cpu utilization since more interrupts will be there, > it may affect the SoC which has lower cpu frequency. This code existed > many years, why this problem has only reported at dwc3 recently? It's a bug, and it's going to cause TCP sockets to potentially hang. You will need to find a valid way to decrease cpu utilization if it in fact becomes an issue because of this revert. But it is not an argument for not doing this revert. -- 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 hub_probe: refactor ugly goto-into-compound-statement
Rework smelling code (goto inside compound statement). Perhaps this is legacy.
Anyway such code is not appropriate for Linux kernel.
---
drivers/usb/core/hub.c | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index cbb1467..abd786b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1802,23 +1802,21 @@ static int hub_probe(struct usb_interface *intf, const
struct usb_device_id *id)
/* Some hubs have a subclass of 1, which AFAICT according to the */
/* specs is not defined, but it works */
- if ((desc->desc.bInterfaceSubClass != 0) &&
- (desc->desc.bInterfaceSubClass != 1)) {
-descriptor_error:
+
+ /* Reject in following cases:
+* - Interface subclass is not 0 or 1
+* - Multiple endpoints
+* - Not an interrupt in endpoint
+*/
+ endpoint = &desc->endpoint[0].desc;
+ if ((desc->desc.bInterfaceSubClass != 0 &&
+desc->desc.bInterfaceSubClass != 1) ||
+desc->desc.bNumEndpoints != 1 ||
+!usb_endpoint_is_int_in(endpoint)) {
dev_err(&intf->dev, "bad descriptor, ignoring hub\n");
return -EIO;
}
- /* Multiple endpoints? What kind of mutant ninja-hub is this? */
- if (desc->desc.bNumEndpoints != 1)
- goto descriptor_error;
-
- endpoint = &desc->endpoint[0].desc;
-
- /* If it's not an interrupt in endpoint, we'd better punt! */
- if (!usb_endpoint_is_int_in(endpoint))
- goto descriptor_error;
-
/* We found a hub */
dev_info(&intf->dev, "USB hub found\n");
--
2.10.2
--
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
Ich werde auf Ihre Antwort warten
Lieber Freund, Wie geht es Ihnen heute? Ich habe eine Investitionsmöglichkeit mit Ihnen zu teilen, die die Übertragung einer großen Geldsumme zum gegenseitigen Nutzen für beide von uns betreffen. Mein Name ist Andrew Hau Chung, ich in einem Finanzinstitut arbeiten hier in Hong Kong. Wenn Sie interessiert sind, kontaktieren Sie mich durch meine private E-Mail-Adresse:(andrewhauch...@gmail.com) Ihre prompte Antwort wird geschätzt. Dein, Andrew Hau Chung -- 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 hub_probe: remove ugly goto-into-compound-statement
Rework smelling code (goto inside compound statement). Perhaps this is legacy.
Anyway such code is not appropriate for Linux kernel.
Signed-off-by: Eugene Korenevsky
---
drivers/usb/core/hub.c | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index cbb1467..4081672 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1802,23 +1802,21 @@ static int hub_probe(struct usb_interface *intf, const
struct usb_device_id *id)
/* Some hubs have a subclass of 1, which AFAICT according to the */
/* specs is not defined, but it works */
- if ((desc->desc.bInterfaceSubClass != 0) &&
- (desc->desc.bInterfaceSubClass != 1)) {
-descriptor_error:
+
+ /* Reject in following cases:
+* - Interface subclass is not 0 or 1
+* - Multiple endpoints
+* - Not an interrupt in endpoint
+*/
+ endpoint = &desc->endpoint[0].desc;
+ if ((desc->desc.bInterfaceSubClass != 0 &&
+desc->desc.bInterfaceSubClass != 1) ||
+ desc->desc.bNumEndpoints != 1 ||
+ !usb_endpoint_is_int_in(endpoint)) {
dev_err(&intf->dev, "bad descriptor, ignoring hub\n");
return -EIO;
}
- /* Multiple endpoints? What kind of mutant ninja-hub is this? */
- if (desc->desc.bNumEndpoints != 1)
- goto descriptor_error;
-
- endpoint = &desc->endpoint[0].desc;
-
- /* If it's not an interrupt in endpoint, we'd better punt! */
- if (!usb_endpoint_is_int_in(endpoint))
- goto descriptor_error;
-
/* We found a hub */
dev_info(&intf->dev, "USB hub found\n");
--
2.10.2
--
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/PATCH 27/45] usb: musb: make use of new usb_endpoint_maxp_mult()
On Tue, Nov 01, 2016 at 12:55:18PM +0200, Felipe Balbi wrote:
>
> Hi,
>
> Bin Liu writes:
> > On Wed, Sep 28, 2016 at 04:05:36PM +0300, Felipe Balbi wrote:
> >> We have introduced a helper to calculate multiplier
> >> value from wMaxPacketSize. Start using it.
> >>
> >> Cc: Bin Liu
> >> Signed-off-by: Felipe Balbi
> >> ---
> >> drivers/usb/musb/musb_gadget.c | 6 +++---
> >> drivers/usb/musb/musb_host.c | 2 +-
> >> 2 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/usb/musb/musb_gadget.c
> >> b/drivers/usb/musb/musb_gadget.c
> >> index bff4869a57cd..8a0882cc446d 100644
> >> --- a/drivers/usb/musb/musb_gadget.c
> >> +++ b/drivers/usb/musb/musb_gadget.c
> >> @@ -974,8 +974,8 @@ static int musb_gadget_enable(struct usb_ep *ep,
> >>goto fail;
> >>
> >>/* REVISIT this rules out high bandwidth periodic transfers */
> >> - tmp = usb_endpoint_maxp(desc);
> >
> > The bit 10~0 in tmp is also needed in the original code,
>
> no it's not
>
> >> - if (tmp & ~0x07ff) {
> >> + tmp = usb_endpoint_maxp_mult(desc) - 1;
> >
> > but here bit 10~0 is lost.
>
> here's the whole thing unpatched:
>
> > /* REVISIT this rules out high bandwidth periodic transfers */
> > tmp = usb_endpoint_maxp(desc);
> > if (tmp & ~0x07ff) {
>
> so, if we have a mult setting
>
> > int ok;
> >
> > if (usb_endpoint_dir_in(desc))
> > ok = musb->hb_iso_tx;
> > else
> > ok = musb->hb_iso_rx;
> >
> > if (!ok) {
> > musb_dbg(musb, "no support for high bandwidth ISO");
> > goto fail;
> > }
> > musb_ep->hb_mult = (tmp >> 11) & 3;
>
> then we save the zero-based value in hb_mult
>
> > } else {
> > musb_ep->hb_mult = 0;
>
> otherwise, set it to 0.
>
> IOW, this could be rewritten as:
>
> int ok;
>
> [...]
>
> /* REVISIT this rules out high bandwidth periodic transfers */
> tmp = usb_endpoint_maxp_mult(desc) - 1;
> if (usb_endpoint_dir_in(desc))
> ok = musb->hb_iso_tx & tmp;
> else
> ok = musb->hb_iso_rx & tmp;
>
> if (!ok) {
> musb_dbg(musb, "no support for high bandwidth ISO");
> goto fail;
> }
> musb_ep->hb_mult = tmp;
I like this new version, one level less in indentation.
>
> And nothing would change. There is, however, one small problem with this
> patch, but that's a few lines after this chunk. Here's the incremental
> patch to fix it:
>
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index 0e3404ce9465..47304560f105 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -992,7 +992,7 @@ static int musb_gadget_enable(struct usb_ep *ep,
> musb_ep->hb_mult = 0;
> }
>
> - musb_ep->packet_sz = tmp & 0x7ff;
This is the problem I meant, tmp is used here again.
> + musb_ep->packet_sz = usb_endpoint_maxp(desc);
This should fix it.
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 00/82] usb: patch bomb
On Tue, Nov 01, 2016 at 10:17:37AM +0200, Felipe Balbi wrote: > > Hi, > > Bin Liu writes: > > Hi Felipe, > > > > On Mon, Oct 31, 2016 at 12:47:52PM +0200, Felipe Balbi wrote: > >> Hi guys, > >> > >> Sorry for the patch bomb, but I wanted to make sure everyoby knows which > >> patches are already queued up for the next window. They are still > >> sitting in my testing/next branch, so I can still change any of them. > >> > >> Please make sure to go through each one of them. This very branch has > >> been tested on Intel SKL and I couldn't find any problems so far. > > > > It seems you missed my comments in the RFC set, basically the set breaks > > MUSB and other controllers. > > Okay, I'll go look for the original comments. I also commented in the RFC for two more issues. - some udc drivers use 0-based mult, but the new usb_endpoint_maxp_mult() makes it 1-based. - some udc drivers keeps bit 11 & 12 when calling the old usb_endpoint_maxp(), but the new version masked bit 11 & 12 already. And here is my comment in RFC/PATCH 01/45. " It seems the instances which use 1 based value is less than those use 0 based. To me it seems make sense to just return 0 based value here. Some controllers like musb writes the 0 based value to a register. " 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 1/3] ARM: davinci: da8xx: Fix ohci driver name
On 11/02/2016 07:44 AM, Axel Haslam wrote: There is a single instance of the ohci driver, while the clk lookup table is making reference to "ohci" other subsystems (such as phy) are looking for "ohci.0" This patch changes the "device" name, not the "driver" name. You use "driver" above and in the subject. Since there is a single ohci instance, change the dev id to -1, and add the "-da8xx" for consitancy with the musb driver name. It would be more accurate to say that you are adding "-da8xx" because you are also changing the ohci _driver_ name in a separate patch. Signed-off-by: Axel Haslam --- -- 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] phy: da8xx-usb: rename the ohci device to ohci-da8xx
On 11/02/2016 07:44 AM, Axel Haslam wrote: There is only one ohci on the da8xx series of chips, so remove the ".0" when creating the phy. Also add the "-da8xx" postfix to be consistent across davinci usb drivers. It would be more accurate to say that the device name is being changed in the mach board configuration files, so it is being changed here in the lookup table to match the new name. Signed-off-by: Axel Haslam --- -- 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: ohci-da8xx: rename driver to ohci-da8xx
On 11/02/2016 07:44 AM, Axel Haslam wrote: To be consistent on the usb driver for the davinci platform follow the example of musb, and add the "-da8xx" postfix to the driver name. It is probably worth mentioning that the existing driver name is "ohci" which is a bit too generic. -- 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: da8xx: Don't print phy error on -EPROBE_DEFER
Hi,
On Tue, Oct 25, 2016 at 02:02:50PM -0500, David Lechner wrote:
> This suppresses printing the error message "failed to get phy" in the
> kernel log when the error is -EPROBE_DEFER. This prevents usless noise
> in the kernel log.
>
> Signed-off-by: David Lechner
> ---
> drivers/usb/musb/da8xx.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> index 481d786..f8a1591 100644
> --- a/drivers/usb/musb/da8xx.c
> +++ b/drivers/usb/musb/da8xx.c
> @@ -516,7 +516,8 @@ static int da8xx_probe(struct platform_device *pdev)
>
> glue->phy = devm_phy_get(&pdev->dev, "usb-phy");
> if (IS_ERR(glue->phy)) {
> - dev_err(&pdev->dev, "failed to get phy\n");
> + if (PTR_ERR(glue->phy) != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "failed to get phy\n");
What about something like this?
dev_printk(PTR_ERR(glue->phy) == -EPROBE_DEFER ? KERN_DEBUG : KERN_ERR, ...
At least it outputs something if debug is enabled, making debugging easier.
ladis
> return PTR_ERR(glue->phy);
> }
>
--
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 3/6] usb: ehci: fsl: use bus->sysdev for DMA configuration
>From: Sriram Dash [mailto:sriram.d...@nxp.com]
>From: Arnd Bergmann
>
>For the dual role ehci fsl driver, sysdev will handle the dma config.
>
>Signed-off-by: Arnd Bergmann
>Signed-off-by: Sriram Dash
>---
>Changes in v2:
> - fix compile warnings
>
>
> drivers/usb/host/ehci-fsl.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c index
>9f5ffb6..4dde500 100644
>--- a/drivers/usb/host/ehci-fsl.c
>+++ b/drivers/usb/host/ehci-fsl.c
>@@ -96,8 +96,8 @@ static int fsl_ehci_drv_probe(struct platform_device *pdev)
> }
> irq = res->start;
>
>- hcd = usb_create_hcd(&fsl_ehci_hc_driver, &pdev->dev,
>- dev_name(&pdev->dev));
>+ hcd = __usb_create_hcd(&fsl_ehci_hc_driver, &pdev->dev->parent,
Will correct it to "pdev->dev.parent".
>+ &pdev->dev, dev_name(&pdev->dev), NULL);
> if (!hcd) {
> retval = -ENOMEM;
> goto err1;
>--
>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] usb: gadget: u_ether: remove interrupt throttling
On Wed, Nov 02, 2016 at 11:22:54AM -0400, David Miller wrote: > From: Peter Chen > Date: Wed, 2 Nov 2016 14:02:02 +0800 > > > Felipe, it may increase cpu utilization since more interrupts will be there, > > it may affect the SoC which has lower cpu frequency. This code existed > > many years, why this problem has only reported at dwc3 recently? > > It's a bug, and it's going to cause TCP sockets to potentially hang. > For some controllers, it is, so we need to add parameter for user to see if interrupt migration is supported or not. But just like some ethernet controllers, some USB controllers support hardware timeout mechanism which interrupt will be triggered after some uFrame occurs if the transaction has completed but not required to interrupt, it is used to support interrupt migration like ethernet. -- Best Regards, Peter Chen -- 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
STRICTLY CONFIDENTIAL
I have important transaction for you as next of kin to claim US$18.37m Mail me on my private email: chimwia...@gmail.com so I can send you more details Thanks Mr.Chim Wai Kim ==MOVE TO INBOX=== DISCLAIMER: This email and any files it contains are confidential and intended for the use of the recipient(s) only. If you are not the intended recipient you should notify the sender immediately and destroy the material from your system. -- 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: u_ether: remove interrupt throttling
On Wed, Nov 02, 2016 at 01:02:59PM +0200, Felipe Balbi wrote:
>
> Hi,
>
> Peter Chen writes:
> >> >> diff --git a/drivers/usb/gadget/function/u_ether.c
> >> >> b/drivers/usb/gadget/function/u_ether.c
> >> >> index f4a640216913..119a2e5848e8 100644
> >> >> --- a/drivers/usb/gadget/function/u_ether.c
> >> >> +++ b/drivers/usb/gadget/function/u_ether.c
> >> >> @@ -589,14 +589,6 @@ static netdev_tx_t eth_start_xmit(struct sk_buff
> >> >> *skb,
> >> >>
> >> >> req->length = length;
> >> >>
> >> >> - /* throttle high/super speed IRQ rate back slightly */
> >> >> - if (gadget_is_dualspeed(dev->gadget))
> >> >> - req->no_interrupt = (((dev->gadget->speed ==
> >> >> USB_SPEED_HIGH ||
> >> >> - dev->gadget->speed ==
> >> >> USB_SPEED_SUPER)) &&
> >> >> - !list_empty(&dev->tx_reqs))
> >> >> - ? ((atomic_read(&dev->tx_qlen) % dev->qmult) !=
> >> >> 0)
> >> >> - : 0;
> >> >> -
> >> >> retval = usb_ep_queue(in, req, GFP_ATOMIC);
> >> >> switch (retval) {
> >> >> default:
> >> >> --
> >> >
> >> > Felipe, it may increase cpu utilization since more interrupts will be
> >> > there,
> >> > it may affect the SoC which has lower cpu frequency. This code existed
> >> > many years, why this problem has only reported at dwc3 recently?
> >>
> >> No idea, but at least for networking gadgets we shouldn't throttle. This
> >> has been a bug since the beginning. Read Dave Miller's explanation at
> >> [1]
> >>
> >> moreover, dwc3 seems to be the only one actually throttling IRQ. Here's
> >> a rundown of a few of the UDCs:
> >>
> >> - chipidea: uses TD_IOC conditionally, but always sets TD_TERMINATE
> >>
> >>lastnode->ptr->next = cpu_to_le32(TD_TERMINATE);
> >>if (!hwreq->req.no_interrupt)
> >>lastnode->ptr->token |= cpu_to_le32(TD_IOC);
> >>
> >>I'm guessing TD_TERMINATE works similar to dwc3's LST bit. If
> >>it's set, it will force an interrupt.
> >
> > No, TD_TERMINATE just stands for it is the last TD, and this pointer will
> > be updated when the new request is added. The interrupt is only triggered
> > by IOC (Interrupt On Complete) bit at TD.
> >
> > I am not sure if dwc3 supports ITC (Interrupt Threshold Control)
> > software control, it is an EHCI compliant register entry, and
> > the device mode is supported for chipidea too. It is a timeout
> > mechanism from controller side for pending requests.
> >
> > The interrupt will be triggered either the request has completed for TD
> > which IOC bit is set or the ITC is fired (125us currently) and the
> > request has completed, so the problem David described should not exist,
> > at least for chipidea.
>
> In other words, you don't *really* throttle interrupt as they'll fire
> after the micro-frame expires :-p
No, even in one uFrame, there are at most ~10 packets for bulk at USB2.
At least, you can throttle interrupt within SoF, it is useful for
high throughout use case.
>
> > If DWC3 has similar ITC bits, would you try to tune it? The default ITC
> > value for chipidea is not enough, and we tuned it before.
>
> there's no such thing in dwc3
>
So, how about add another parameter to support throttling interrupt
separately. Current parameter 'mult' combined user request number
and throttle interrupt together.
--
Best Regards,
Peter Chen
--
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 v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation
On Tue, Nov 01 2016, Baolin Wang wrote: >> So I won't be responding on this topic any further until I see a genuine >> attempt to understand and resolve the inconsistencies with >> usb_register_notifier(). > > Any better solution? I'm not sure exactly what you are asking, so I'll assume you are asking the question I want to answer :-) 1/ Liase with the extcon developers to resolve the inconsistencies with USB connector types. e.g. current there is both "EXTCON_USB" and "EXTCON_CHG_USB_SDP" which both seem to suggest a standard downstream port. There is no documentation describing how these relate, and no consistent practice to copy. I suspect the intention is that EXTCON_USB and EXTCON_USB_HOST indicated that data capabilities of the cable, while EXTCON_CHG_USB* indicate the power capabilities of the cable. So EXTCON_CHG_USB_SDP should always appear together with EXTCON_USB while EXTCON_CHG_USB_DCP would not, and EXTCON_CHG_USB_ACA would normally appear with EXTCON_USB_HOST (I think). Some drivers follow this model, particularly extcon-max14577.c but it is not consistent. This policy should be well documented and possibly existing drivers should be updated to follow it. At the same time it would make sense to resolve EXTCON_CHG_USB_SLOW and EXTCON_CHG_USB_FAST. These names don't mean much. They were recently removed from drivers/power/axp288_charger.c which is good, but are still used in drivers/extcon/extcon-max* Possibly they should be changed to names from the standard, or possibly they should be renamed to identify the current they are expected to provide. e.g. EXTCON_CHG_USB_500MA and EXTCON_CHG_USB_1A 2/ Change all usb phys to register an extcon and to send appropriate notifications. Many already do, but I don't think it is universal. It is probable that the extcon should be registered using common code instead of each phy driver having its own extcon_get_edev_by_phandle() or whatever. If the usb phy driver needs to look at battery charger registers to know what sort of cable was connected (which I believe is the case for the chips you are interested in), then it should do that. 3/ Currently some USB controllers discover that a cable was connected by listening on an extcon, and some by registering for a usb_notifier (described below) ... though there seem to only be 2 left which do that. Now that all USB phys send connection information via extcon (see 2), the USB controllers should be changed to all find out about the cable using extcon. 4/ struct usb_phy contains: /* for notification of usb_phy_events */ struct atomic_notifier_head notifier; This is used inconsistently. Sometimes the argument passed is NULL, sometimes it is a pointer to 'vbus_draw' - the current limited negotiated via USB, sometimes it is a pointer the the gadget though as far as I can tell, that last one is never used. This should be changed to be consistent. This notifier is no longer needed to tell the USB controller that a cable was connected (extcon now does that, see 3) so it is only used to communicate the 'vbus_draw' information. So it should be changed to *only* send a notification when vbus_draw is known, and it should carry that information. This should probably be done in common code, and removed from individual drivers. 5/ Now that all cable connection notifications are sent over extcon and all vbus_draw notifications are sent over the usb_phy notifier, write some support code that a power supply client can use to be told what power is available. e.g. a battery charger driver would call: register_power_client(.) or similar, providing a phandle (or similar) for the usb phy and a function to call back when the available current changes (or maybe a work_struct containing the function pointer) register_power_client() would then register with extcon and separately with the usb_phy notifier. When the different events arrive it calculates what ranges of currents are expected and calls the call-back function with those details. 6/ Any battery charger that needs to know the available current can now call register_power_client() and get the information delivered. NeilBrown signature.asc Description: PGP signature
[PATCH 1/1] usb: xhci: cleanup cmd_completion in xhci_virt_device
cmd_completion in struct xhci_virt_device is legacy. With command
strutcture and command queue introduced in xhci, cmd_completion is
not used any more. This patch removes it.
Signed-off-by: Lu Baolu
---
drivers/usb/host/xhci-mem.c | 1 -
drivers/usb/host/xhci.h | 1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6afe323..8102a86 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1032,7 +1032,6 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int
slot_id,
goto fail;
dev->num_rings_cached = 0;
- init_completion(&dev->cmd_completion);
dev->udev = udev;
/* Point to output device context in dcbaa. */
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index f945380..e2b62e0 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -997,7 +997,6 @@ struct xhci_virt_device {
int num_rings_cached;
#defineXHCI_MAX_RINGS_CACHED 31
struct xhci_virt_ep eps[31];
- struct completion cmd_completion;
u8 fake_port;
u8 real_port;
struct xhci_interval_bw_table *bw_table;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
