Re: [RFC] usb-phy-generic: Add support to SMSC USB3315

2017-06-05 Thread Peter Chen
On Mon, Jun 05, 2017 at 11:52:26AM +0200, Fabien Lahoudere wrote:
> On Mon, 2017-06-05 at 17:43 +0800, Peter Chen wrote:
> > On Mon, Jun 05, 2017 at 10:57:00AM +0200, Fabien Lahoudere wrote:
> > > On Fri, 2017-06-02 at 15:00 -0700, Stephen Boyd wrote:
> > > > On 05/26, Fabien Lahoudere wrote:
> > > > > Hello
> > > > > 
> > > > > I modify ci_hrdc_imx_probe to bypass "data->phy = 
> > > > > devm_usb_get_phy_by_phandle(>dev,
> > > > > "fsl,usbphy", 0);". Everything works as expected and call 
> > > > > ci_ulpi_init.
> > > > > 
> > > > > The problem is that in ci_ulpi_init, before calling "ci->ulpi = 
> > > > > ulpi_register_interface(ci-
> > > > > >dev,
> > > > > >ulpi_ops);" (to initialize our phy), "hw_phymode_configure(ci);" 
> > > > > is called which is the
> > > > > original function that make our system to hang.
> > > > > 
> > > > > Our phy is not initialised before calling ulpi_register_interface so 
> > > > > I don't understand how
> > > > > the
> > > > > phy
> > > > > can reply if it is not out of reset state.
> > > > 
> > > > I haven't see any problem in hw_phymode_configure(). What's the
> > > > value of ci->platdata->phy_mode? USBPHY_INTERFACE_MODE_ULPI? If
> > > > you phy needs to be taken out of reset to reply to the ulpi reads
> > > > of the vendor/product ids, then it sounds like you have a similar
> > > > situation to what I had. I needed to turn on some regulators to
> > > > get those reads to work, otherwise they would fail, but knowing
> > > > what needed to be turned on basically meant I needed to probe the
> > > > ulpi driver so probing the ids wasn't going to be useful. So on
> > > > my device the reads for the ids go through, but they get all
> > > > zeroes back, which is actually ok because there aren't any bits
> > > > set on my devices anyway. After the reads see 0, we fallback to
> > > > DT matching, which avoids the "bring it out of reset/power it on"
> > > > sorts of problems entirely.
> > > > 
> > > 
> > > Yes the phy mode is configured to USBPHY_INTERFACE_MODE_ULPI.
> > > Indeed, this phy need to be out of reset to work. For example everything 
> > > works fine if I call 
> > > "_ci_usb_phy_init(ci);" before calling "hw_phymode_configure(ci);"
> > > This function only init reset GPIO and clock.
> > > 
> > > For information, the original patch I have to fix the issue:
> > > 
> > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > > index 79ad8e9..21aaff1 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -391,6 +391,7 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
> > >   case USBPHY_INTERFACE_MODE_UTMI:
> > >   case USBPHY_INTERFACE_MODE_UTMIW:
> > >   case USBPHY_INTERFACE_MODE_HSIC:
> > > + case USBPHY_INTERFACE_MODE_ULPI:
> > >   ret = _ci_usb_phy_init(ci);
> > >   if (!ret)
> > >   hw_wait_phy_stable();
> > > @@ -398,7 +399,6 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
> > >   return ret;
> > >   hw_phymode_configure(ci);
> > >   break;
> > > - case USBPHY_INTERFACE_MODE_ULPI:
> > >   case USBPHY_INTERFACE_MODE_SERIAL:
> > >   hw_phymode_configure(ci);
> > >   ret = _ci_usb_phy_init(ci);
> > > -- 
> > 
> > Currently, the hw_phymode_configure is called twice for ULPI PHY, the
> > two execution are between _ci_usb_phy_init, would you test which one
> > causes hang? If the second causes hang, you can make a patch for
> > hw_phymode_configure that if the required PORTSC_PTS is the same
> > the value in register, do noop.
> 
> The first one hangs, _ci_usb_phy_init is not called due to hang.
> 

So, you need to comment out hw_phymode_configure at ci_ulpi_init, and you
can't get vid/pid correctly, right? If it is, we may need to add power on
sequence at chipidea core driver (ci_hdrc_probe) for clock and reset things.

http://www.spinics.net/lists/linux-usb/msg157134.html

I am wondering if we can call ci_usb_phy_init before calling ci_ulpi_init,
since we need to let hardware be ready before reading vid/pid.
Stephen & Fabien, does that work for you?

-- 

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] usb: dwc2: add support for the DWC2 controller on Meson8 SoCs

2017-06-05 Thread John Youn
On 5/25/2017 10:39 AM, Martin Blumenstingl wrote:
> Hi John,
>
> On Sat, May 6, 2017 at 7:37 PM, Martin Blumenstingl
>  wrote:
>> USB support in the Meson8 SoCs is provided by a DWC2 controller which
>> works with the same settings as Meson8b and GXBB. Using the generic
>> "snps,dwc2" binding results in an endless stream of "Overcurrent change
>> detected" messages.
>>
>> Signed-off-by: Martin Blumenstingl 
> do you want me to re-send this with Rob's ACK?
> also please let me know if something is still missing in this patch
>
>> ---
>>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>>  drivers/usb/dwc2/params.c  | 2 ++
>>  2 files changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
>> b/Documentation/devicetree/bindings/usb/dwc2.txt
>> index 6c7c2bce6d0c..b55be381ae85 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>> @@ -10,6 +10,7 @@ Required properties:
>>- "rockchip,rk3288-usb", "rockchip,rk3066-usb", "snps,dwc2": for rk3288 
>> Soc;
>>- "lantiq,arx100-usb": The DWC2 USB controller instance in Lantiq ARX 
>> SoCs;
>>- "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX 
>> SoCs;
>> +  - "amlogic,meson8-usb": The DWC2 USB controller instance in Amlogic 
>> Meson8 SoCs;
>>- "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic 
>> Meson8b SoCs;
>>- "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic 
>> S905 SoCs;
>>- "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 
>> 460EX SoCs;
>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
>> index 2990c347289f..0d6290ed66ea 100644
>> --- a/drivers/usb/dwc2/params.c
>> +++ b/drivers/usb/dwc2/params.c
>> @@ -128,6 +128,8 @@ const struct of_device_id dwc2_of_match_table[] = {
>> { .compatible = "lantiq,xrx200-usb", .data = dwc2_set_ltq_params },
>> { .compatible = "snps,dwc2" },
>> { .compatible = "samsung,s3c6400-hsotg" },
>> +   { .compatible = "amlogic,meson8-usb",
>> + .data = dwc2_set_amlogic_params },
>> { .compatible = "amlogic,meson8b-usb",
>>   .data = dwc2_set_amlogic_params },
>> { .compatible = "amlogic,meson-gxbb-usb",
>> --
>> 2.12.2
>>
>

Adding Felipe


Acked-by: John Youn 

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 v14 4/7] usb: core: add power sequence handling for USB devices

2017-06-05 Thread Peter Chen
On Mon, Jun 05, 2017 at 10:16:34AM -0400, Alan Stern wrote:
> On Mon, 5 Jun 2017, Peter Chen wrote:
> 
> > On Thu, May 18, 2017 at 08:49:00AM +0800, Peter Chen wrote:
> > > Some hard-wired USB devices need to do power sequence to let the
> > > device work normally, the typical power sequence like: enable USB
> > > PHY clock, toggle reset pin, etc. But current Linux USB driver
> > > lacks of such code to do it, it may cause some hard-wired USB devices
> > > works abnormal or can't be recognized by controller at all.
> > > 
> > > In this patch, it calls power sequence library APIs to finish
> > > the power sequence events. It will do power on sequence at hub's
> > > probe for all devices under this hub (includes root hub).
> > > At hub_disconnect, it will do power off sequence which is at powered
> > > on list.
> > > 
> > 
> > Greg, Alan, would you please help on reviewing it? It seems Rafael
> > is waiting for USB MAINTAINERS's comments or ack. It resolves some
> > USB HUB issues for several persons.
> > 
> > Peter
> > 
> > > Signed-off-by: Peter Chen 
> > > Tested-by Joshua Clayton 
> > > Tested-by: Maciej S. Szmigiero 
> > > Reviewed-by: Vaibhav Hiremath 
> 
> Acked-by: Alan Stern 
> 
> > > ---
> > >  drivers/usb/Kconfig|  1 +
> > >  drivers/usb/core/hub.c | 49 
> > > +
> > >  drivers/usb/core/hub.h |  1 +
> > >  3 files changed, 47 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> > > index 939a63b..b6f626e 100644
> > > --- a/drivers/usb/Kconfig
> > > +++ b/drivers/usb/Kconfig
> > > @@ -39,6 +39,7 @@ config USB
> > >   tristate "Support for Host-side USB"
> > >   depends on USB_ARCH_HAS_HCD
> > >   select USB_COMMON
> > > + select POWER_SEQUENCE
> > >   select NLS  # for UTF-8 strings
> > >   ---help---
> > > Universal Serial Bus (USB) is a specification for a serial bus
> > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > > index 9dca59e..7a67296 100644
> > > --- a/drivers/usb/core/hub.c
> > > +++ b/drivers/usb/core/hub.c
> > > @@ -28,6 +28,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >  
> > >  #include 
> > >  #include 
> > > @@ -1619,6 +1620,7 @@ static void hub_disconnect(struct usb_interface 
> > > *intf)
> > >   hub->error = 0;
> > >   hub_quiesce(hub, HUB_DISCONNECT);
> > >  
> > > + of_pwrseq_off_list(>pwrseq_on_list);
> 
> Things like this look a little peculiar -- you are passing an "on_list" 
> to a function named "...off_list".
> 
> How about naming the new field hub->pwrseq_list instead?
> 

Thanks, Alan. I will change it at next revision.

-- 

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: g_mass_storage emulation of flash drive - difficulties with passing vendor/product ID

2017-06-05 Thread Alan Robertson
On Mon, Jun 5, 2017 at 9:52 PM, Alan Stern  wrote:
> [Please use Reply-To-All so that your message gets sent to the mailing
> list as well as to me.]

Oops, apologies - now done!

> On Mon, 5 Jun 2017, Alan Robertson wrote:
>
>> On Mon, Jun 5, 2017 at 3:02 PM, Alan Stern  wrote:
>> > On Sun, 4 Jun 2017, Alan Robertson wrote:
>> >
>> >> Hi
>> >>
>> >> Apologies for my first post here being a request for help, but after
>> >> extensive searching online I've not yet been able to find and answer
>> >> and am hopeful the expertise here may be able to help!
>> >>
>> >> I have a Raspberry Pi Zero W that I'm using in OTG/gadget mode. I've
>> >> activated dwc2 USB, partitioned and formatted the file that is to be
>> >> used as filesystem and loaded g_mass_storage.
>> >>
>> >> It is working fine in Windows - I can connect and see the contents,
>> >> add/delete files, eject and reconnect seeing changes fine. I can mount
>> >> it in Raspbian to check contents and can also attach it to other
>> >> devices and interact with it fine.
>> >>
>> >> However, I've got some devices that are very picky about what USB
>> >> devices they will connect to (solely flash drives). The problem is the
>> >> OTG mass storage gadget is being identified as "File-Stor Gadget (Rev:
>> >> 0404)" for device type, which is causing it to be rejected.
>> >>
>> >> I can successfully use a USB flash drive in the target devices and
>> >> have scraped the vendor/product ID, etc from this flash drive to see
>> >> if I can mimic them via g_mass_storage.
>> >>
>> >> I have adjusted modprobe to the following:
>> >>
>> >> $ sudo modprobe g_mass_storage file=/home/pi/piusb.bin stall=0
>> >> removable=1 idVendor=0x0781 idProduct=0x5572 bcdDevice=0x011a
>
> I should point out that this bcdDevice value is not valid (it isn't a
> binary-coded-decimal value).

Thanks, now corrected to 0x0126 - this seems to show up OK in lsusb as 1.26 now

>> >> iManufacturer="SanDisk" iProduct="Cruzer Switch"
>> >> iSerialNumber="123456789012"
>> >>
>> >> Unfortunately this doesn't seem to have made any difference and it is
>> >> still rejected by the target devices.
>> >>
>> >> As far as I can tell the above parameters are getting passed through 
>> >> correctly
>> >> $ ls /sys/module/g_mass_storage/parameters/*
>> >> /sys/module/g_mass_storage/parameters/bcdDevice
>> >> /sys/module/g_mass_storage/parameters/cdrom
>> >> /sys/module/g_mass_storage/parameters/file
>> >> /sys/module/g_mass_storage/parameters/idProduct
>> >> /sys/module/g_mass_storage/parameters/idVendor
>> >> /sys/module/g_mass_storage/parameters/iManufacturer
>> >> /sys/module/g_mass_storage/parameters/iProduct
>> >> /sys/module/g_mass_storage/parameters/iSerialNumber
>> >> /sys/module/g_mass_storage/parameters/luns
>> >> /sys/module/g_mass_storage/parameters/nofua
>> >> /sys/module/g_mass_storage/parameters/removable
>> >> /sys/module/g_mass_storage/parameters/ro
>> >> /sys/module/g_mass_storage/parameters/stall
>> >>
>> >> $ cat /sys/module/g_mass_storage/parameters/*
>> >> 282
>> >> /home/pi/piusb.bin
>> >> 21874
>> >> 1921
>> >> SanDisk
>> >> Cruzer_Switch
>> >> 123456789012
>> >> 0
>> >> Y
>> >> N
>> >>
>> >> However when connecting to Windows, here's what the USB mass storage
>> >> displays - it appears that the iSerialNumber is making it through, but
>> >> nothing else is being altered.  My router shows similar issues,
>> >> identifying it as "File-Stor Gadget (Rev: 0404)"
>> >>
>> >> USBSTOR\DISK_LINUX_FILE-STOR_GADGET_0404\123456789012&0
>
> It looks like Windows is remembering an earlier version of your gadget
> and matching it with the current version based on the serial number
> string.

I've changed the SN and still getting it showing as 'File-Stor Gadget'
on Linux, Windows, router

>> >> USBSTOR\DiskLinux___File-Stor_Gadget0404
>> >> USBSTOR\DiskLinux___File-Stor_Gadget
>> >> USBSTOR\DiskLinux___
>> >> USBSTOR\Linux___File-Stor_Gadget0
>> >> Linux___File-Stor_Gadget0
>> >> USBSTOR\GenDisk
>> >> GenDisk
>> >>
>> >> By comparison, here's what SanDisk shows:
>> >>
>> >> USBSTOR\DISK_SANDISK_CRUZER_SWITCH_1.26\4C532015741508522393&0
>> >> USBSTOR\DiskSanDisk_Cruzer_Switch___1.26
>> >> USBSTOR\DiskSanDisk_Cruzer_Switch___
>> >> USBSTOR\DiskSanDisk_
>> >> USBSTOR\SanDisk_Cruzer_Switch___1
>> >> SanDisk_Cruzer_Switch___1
>> >> USBSTOR\GenDisk
>> >> GenDisk
>> >
>> > Never mind Windows -- if you connect your gadget to a computer running
>> > Linux, what does "lsusb -v" show?
>> >
>> > Alan Stern
>> >
>> Thanks for the reply, Alan.
>>
>> Can provide the full output if it would help, but relevant part seems
>> to be the below:
>>
>> Bus 001 Device 004: ID 0781:5572 SanDisk Corp.
>> Device Descriptor:
>>   bLength18
>>   bDescriptorType 1
>>   bcdUSB   2.00
>>   bDeviceClass0 (Defined at Interface level)
>>   bDeviceSubClass 0
>>   bDeviceProtocol 0
>>   bMaxPacketSize064
>>   

Re: g_mass_storage emulation of flash drive - difficulties with passing vendor/product ID

2017-06-05 Thread Alan Stern
[Please use Reply-To-All so that your message gets sent to the mailing 
list as well as to me.]

On Mon, 5 Jun 2017, Alan Robertson wrote:

> On Mon, Jun 5, 2017 at 3:02 PM, Alan Stern  wrote:
> > On Sun, 4 Jun 2017, Alan Robertson wrote:
> >
> >> Hi
> >>
> >> Apologies for my first post here being a request for help, but after
> >> extensive searching online I've not yet been able to find and answer
> >> and am hopeful the expertise here may be able to help!
> >>
> >> I have a Raspberry Pi Zero W that I'm using in OTG/gadget mode. I've
> >> activated dwc2 USB, partitioned and formatted the file that is to be
> >> used as filesystem and loaded g_mass_storage.
> >>
> >> It is working fine in Windows - I can connect and see the contents,
> >> add/delete files, eject and reconnect seeing changes fine. I can mount
> >> it in Raspbian to check contents and can also attach it to other
> >> devices and interact with it fine.
> >>
> >> However, I've got some devices that are very picky about what USB
> >> devices they will connect to (solely flash drives). The problem is the
> >> OTG mass storage gadget is being identified as "File-Stor Gadget (Rev:
> >> 0404)" for device type, which is causing it to be rejected.
> >>
> >> I can successfully use a USB flash drive in the target devices and
> >> have scraped the vendor/product ID, etc from this flash drive to see
> >> if I can mimic them via g_mass_storage.
> >>
> >> I have adjusted modprobe to the following:
> >>
> >> $ sudo modprobe g_mass_storage file=/home/pi/piusb.bin stall=0
> >> removable=1 idVendor=0x0781 idProduct=0x5572 bcdDevice=0x011a

I should point out that this bcdDevice value is not valid (it isn't a 
binary-coded-decimal value).

> >> iManufacturer="SanDisk" iProduct="Cruzer Switch"
> >> iSerialNumber="123456789012"
> >>
> >> Unfortunately this doesn't seem to have made any difference and it is
> >> still rejected by the target devices.
> >>
> >> As far as I can tell the above parameters are getting passed through 
> >> correctly
> >> $ ls /sys/module/g_mass_storage/parameters/*
> >> /sys/module/g_mass_storage/parameters/bcdDevice
> >> /sys/module/g_mass_storage/parameters/cdrom
> >> /sys/module/g_mass_storage/parameters/file
> >> /sys/module/g_mass_storage/parameters/idProduct
> >> /sys/module/g_mass_storage/parameters/idVendor
> >> /sys/module/g_mass_storage/parameters/iManufacturer
> >> /sys/module/g_mass_storage/parameters/iProduct
> >> /sys/module/g_mass_storage/parameters/iSerialNumber
> >> /sys/module/g_mass_storage/parameters/luns
> >> /sys/module/g_mass_storage/parameters/nofua
> >> /sys/module/g_mass_storage/parameters/removable
> >> /sys/module/g_mass_storage/parameters/ro
> >> /sys/module/g_mass_storage/parameters/stall
> >>
> >> $ cat /sys/module/g_mass_storage/parameters/*
> >> 282
> >> /home/pi/piusb.bin
> >> 21874
> >> 1921
> >> SanDisk
> >> Cruzer_Switch
> >> 123456789012
> >> 0
> >> Y
> >> N
> >>
> >> However when connecting to Windows, here's what the USB mass storage
> >> displays - it appears that the iSerialNumber is making it through, but
> >> nothing else is being altered.  My router shows similar issues,
> >> identifying it as "File-Stor Gadget (Rev: 0404)"
> >>
> >> USBSTOR\DISK_LINUX_FILE-STOR_GADGET_0404\123456789012&0

It looks like Windows is remembering an earlier version of your gadget
and matching it with the current version based on the serial number
string.

> >> USBSTOR\DiskLinux___File-Stor_Gadget0404
> >> USBSTOR\DiskLinux___File-Stor_Gadget
> >> USBSTOR\DiskLinux___
> >> USBSTOR\Linux___File-Stor_Gadget0
> >> Linux___File-Stor_Gadget0
> >> USBSTOR\GenDisk
> >> GenDisk
> >>
> >> By comparison, here's what SanDisk shows:
> >>
> >> USBSTOR\DISK_SANDISK_CRUZER_SWITCH_1.26\4C532015741508522393&0
> >> USBSTOR\DiskSanDisk_Cruzer_Switch___1.26
> >> USBSTOR\DiskSanDisk_Cruzer_Switch___
> >> USBSTOR\DiskSanDisk_
> >> USBSTOR\SanDisk_Cruzer_Switch___1
> >> SanDisk_Cruzer_Switch___1
> >> USBSTOR\GenDisk
> >> GenDisk
> >
> > Never mind Windows -- if you connect your gadget to a computer running
> > Linux, what does "lsusb -v" show?
> >
> > Alan Stern
> >
> Thanks for the reply, Alan.
> 
> Can provide the full output if it would help, but relevant part seems
> to be the below:
> 
> Bus 001 Device 004: ID 0781:5572 SanDisk Corp.
> Device Descriptor:
>   bLength18
>   bDescriptorType 1
>   bcdUSB   2.00
>   bDeviceClass0 (Defined at Interface level)
>   bDeviceSubClass 0
>   bDeviceProtocol 0
>   bMaxPacketSize064
>   idVendor   0x0781 SanDisk Corp.
>   idProduct  0x5572
>   bcdDevice1.1a
>   iManufacturer   3 SanDisk
>   iProduct4 Cruzer_Switch
>   iSerial 5 123456789012
>   bNumConfigurations  1
>   Configuration Descriptor:
> bLength 9
> bDescriptorType 2
> wTotalLength   32
> bNumInterfaces  1
> 

[PATCH v6 6/6] platform/x86: intel_bxtwc_tmu: Remove first level IRQ unmask

2017-06-05 Thread sathyanarayanan . kuppuswamy
From: Kuppuswamy Sathyanarayanan 

Currently in WCOVE PMIC MFD driver, all second level IRQ chips
are chained to the respective first level IRQs. So there is no
need for explicitly unmasking the first level IRQ in this
driver. This patches removes this level 1 IRQ unmask support.

Signed-off-by: Kuppuswamy Sathyanarayanan 

Reviewed-by: Darren Hart (VMware) 
Reviewed-by: Andy Shevchenko 
---
 drivers/platform/x86/intel_bxtwc_tmu.c | 4 
 1 file changed, 4 deletions(-)

Changes since v1:
 * None

Changes since v2:
 * Rebased on top of latest release.

Changes since v3:
 * None

Changes since v4:
 * Changed commit subject from "platform: x86:" "to platform/x86:"

Changes since v5:
 * Changed irq->IRQ, mfd->MFD.

diff --git a/drivers/platform/x86/intel_bxtwc_tmu.c 
b/drivers/platform/x86/intel_bxtwc_tmu.c
index e202abd..ea865d4 100644
--- a/drivers/platform/x86/intel_bxtwc_tmu.c
+++ b/drivers/platform/x86/intel_bxtwc_tmu.c
@@ -92,10 +92,6 @@ static int bxt_wcove_tmu_probe(struct platform_device *pdev)
}
wctmu->irq = virq;
 
-   /* Enable TMU interrupts */
-   regmap_update_bits(wctmu->regmap, BXTWC_MIRQLVL1,
- BXTWC_MIRQLVL1_MTMU, 0);
-
/* Unmask TMU second level Wake & System alarm */
regmap_update_bits(wctmu->regmap, BXTWC_MTMUIRQ_REG,
  BXTWC_TMU_ALRM_MASK, 0);
-- 
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 v6 1/6] mfd: intel_soc_pmic_bxtwc: Fix TMU interrupt index

2017-06-05 Thread sathyanarayanan . kuppuswamy
From: Kuppuswamy Sathyanarayanan 

TMU interrupts are registered as a separate interrupt chip, and
hence it should start its interrupt index(BXTWC_TMU_IRQ) number
from 0. But currently, BXTWC_TMU_IRQ is defined as part of enum
bxtwc_irqs_level2 and its index value is 11. Since this index
value is used when calculating .num_irqs of regmap_irq_chip_tmu,
it incorrectly reports number of IRQs as 12 instead of actual
value of 1.

This patch fixes this issue by creating new enum of tmu IRQs and
resetting its starting index to 0.

Signed-off-by: Kuppuswamy Sathyanarayanan 

Acked-for-MFD-by: Lee Jones 
---
 drivers/mfd/intel_soc_pmic_bxtwc.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

Changes since v1: 
 * Removed code from commit message.

Changes since v2: 
 * Rebased on top of latest release.

Changes sinve v3:
 * Rebased on top of latest release.

Changes sinve v5:
 * Changed irq to IRQ.

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c 
b/drivers/mfd/intel_soc_pmic_bxtwc.c
index 8c3cbf6..7cbaf1e 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -95,7 +95,10 @@ enum bxtwc_irqs_level2 {
BXTWC_GPIO0_IRQ,
BXTWC_GPIO1_IRQ,
BXTWC_CRIT_IRQ,
-   BXTWC_TMU_IRQ,
+};
+
+enum bxtwc_irqs_tmu {
+   BXTWC_TMU_IRQ = 0,
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs[] = {
-- 
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 v6 3/6] mfd: intel_soc_pmic_bxtwc: Remove second level IRQ for gpio device

2017-06-05 Thread sathyanarayanan . kuppuswamy
From: Kuppuswamy Sathyanarayanan 

Currently all PMIC GPIO domain IRQs are consumed by the same
device(bxt_wcove_gpio), so there is no need to export them as
separate interrupts. We can just export only the first level
GPIO IRQ(BXTWC_GPIO_LVL1_IRQ) as an IRQ resource and let the
GPIO device driver(bxt_wcove_gpio) handle the GPIO sub domain
IRQs based on status value of GPIO level2 interrupt status
register. Also, just using only the first level IRQ will eliminate
the bug involved in requesting only the second level IRQ and not
explicitly enable the first level IRQ. For more info on this
issue please read the details at,

https://lkml.org/lkml/2017/2/27/148

This patch also makes relevant change in Whiskey cove GPIO driver to
use only first level PMIC GPIO IRQ.

Signed-off-by: Kuppuswamy Sathyanarayanan 

Acked-for-MFD-by: Lee Jones 
Acked-by: Linus Walleij 
---
 drivers/gpio/gpio-wcove.c  | 14 +-
 drivers/mfd/intel_soc_pmic_bxtwc.c |  7 +--
 2 files changed, 14 insertions(+), 7 deletions(-)

Changes since v1:
 * None

Changes since v2: 
 * Rebased on top of latest release.

Changes since v3:
 * None

Changes since v5:
 * Squashed gpio driver patch.
 * Changes irq -> IRQ.

diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c
index 7b1bc20..bba7704 100644
--- a/drivers/gpio/gpio-wcove.c
+++ b/drivers/gpio/gpio-wcove.c
@@ -401,7 +401,7 @@ static int wcove_gpio_probe(struct platform_device *pdev)
if (!wg)
return -ENOMEM;
 
-   wg->regmap_irq_chip = pmic->irq_chip_data_level2;
+   wg->regmap_irq_chip = pmic->irq_chip_data;
 
platform_set_drvdata(pdev, wg);
 
@@ -449,6 +449,18 @@ static int wcove_gpio_probe(struct platform_device *pdev)
 
gpiochip_set_nested_irqchip(>chip, _irqchip, virq);
 
+   /* Enable GPIO0 interrupts */
+   ret = regmap_update_bits(wg->regmap, IRQ_MASK_BASE, GPIO_IRQ0_MASK,
+0x00);
+   if (ret)
+   return ret;
+
+   /* Enable GPIO1 interrupts */
+   ret = regmap_update_bits(wg->regmap, IRQ_MASK_BASE + 1, GPIO_IRQ1_MASK,
+0x00);
+   if (ret)
+   return ret;
+
return 0;
 }
 
diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c 
b/drivers/mfd/intel_soc_pmic_bxtwc.c
index 7c1ed27..af11c43 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -89,8 +89,6 @@ enum bxtwc_irqs_level2 {
BXTWC_USBC_IRQ,
BXTWC_CHGR0_IRQ,
BXTWC_CHGR1_IRQ,
-   BXTWC_GPIO0_IRQ,
-   BXTWC_GPIO1_IRQ,
BXTWC_CRIT_IRQ,
 };
 
@@ -116,8 +114,6 @@ static const struct regmap_irq bxtwc_regmap_irqs_level2[] = 
{
REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
-   REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 4, 0xff),
-   REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 5, 0x3f),
REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
 };
 
@@ -153,8 +149,7 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
 };
 
 static struct resource gpio_resources[] = {
-   DEFINE_RES_IRQ_NAMED(BXTWC_GPIO0_IRQ, "GPIO0"),
-   DEFINE_RES_IRQ_NAMED(BXTWC_GPIO1_IRQ, "GPIO1"),
+   DEFINE_RES_IRQ_NAMED(BXTWC_GPIO_LVL1_IRQ, "GPIO"),
 };
 
 static struct resource adc_resources[] = {
-- 
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 v6 2/6] mfd: intel_soc_pmic_bxtwc: Remove thermal second level IRQs

2017-06-05 Thread sathyanarayanan . kuppuswamy
From: Kuppuswamy Sathyanarayanan 

Since all second level thermal IRQs are consumed by the same
device(bxt_wcove_thermal), there is no need to expose them as separate
interrupts. We can just export only the first level IRQs for thermal and
let the device(bxt_wcove_thermal) driver handle the second level IRQs
based on thermal interrupt status register. Also, just using only the
first level IRQ will eliminate the bug involved in requesting only the
second level IRQ and not explicitly enable the first level IRQ. For
more info on this issue please read the details at,

https://lkml.org/lkml/2017/2/27/148

This patch also makes relevant change in bxt_wcove_thermal driver to use
only first level PMIC thermal IRQ.

Signed-off-by: Kuppuswamy Sathyanarayanan 

Acked-for-MFD-by: Lee Jones 
Acked-by: Zhang Rui 
---
 drivers/mfd/intel_soc_pmic_bxtwc.c   | 32 
 drivers/thermal/intel_bxt_pmic_thermal.c |  2 +-
 2 files changed, 13 insertions(+), 21 deletions(-)

Changes since v1:
 * None

Changes since v2: 
 * Rebased on top of latest release.

Changes since v3:
 * Rebased on top of latest release.

Changes since v5:
 * Squashed intel_bxt_pmic_thermal driver patch.
 * Changed irq -> IRQ


diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c 
b/drivers/mfd/intel_soc_pmic_bxtwc.c
index 7cbaf1e..7c1ed27 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -84,10 +84,7 @@ enum bxtwc_irqs {
 
 enum bxtwc_irqs_level2 {
/* Level 2 */
-   BXTWC_THRM0_IRQ = 0,
-   BXTWC_THRM1_IRQ,
-   BXTWC_THRM2_IRQ,
-   BXTWC_BCU_IRQ,
+   BXTWC_BCU_IRQ = 0,
BXTWC_ADC_IRQ,
BXTWC_USBC_IRQ,
BXTWC_CHGR0_IRQ,
@@ -114,17 +111,14 @@ static const struct regmap_irq bxtwc_regmap_irqs[] = {
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
-   REGMAP_IRQ_REG(BXTWC_THRM0_IRQ, 0, 0xff),
-   REGMAP_IRQ_REG(BXTWC_THRM1_IRQ, 1, 0xbf),
-   REGMAP_IRQ_REG(BXTWC_THRM2_IRQ, 2, 0xff),
-   REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 3, 0x1f),
-   REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 4, 0xff),
-   REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 5, BIT(5)),
-   REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x1f),
-   REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 6, 0x1f),
-   REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 7, 0xff),
-   REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 8, 0x3f),
-   REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 9, 0x03),
+   REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 0, 0x1f),
+   REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 1, 0xff),
+   REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
+   REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
+   REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
+   REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 4, 0xff),
+   REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 5, 0x3f),
+   REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
@@ -142,8 +136,8 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
 
 static struct regmap_irq_chip bxtwc_regmap_irq_chip_level2 = {
.name = "bxtwc_irq_chip_level2",
-   .status_base = BXTWC_THRM0IRQ,
-   .mask_base = BXTWC_MTHRM0IRQ,
+   .status_base = BXTWC_BCUIRQ,
+   .mask_base = BXTWC_MBCUIRQ,
.irqs = bxtwc_regmap_irqs_level2,
.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_level2),
.num_regs = 10,
@@ -177,9 +171,7 @@ static struct resource charger_resources[] = {
 };
 
 static struct resource thermal_resources[] = {
-   DEFINE_RES_IRQ(BXTWC_THRM0_IRQ),
-   DEFINE_RES_IRQ(BXTWC_THRM1_IRQ),
-   DEFINE_RES_IRQ(BXTWC_THRM2_IRQ),
+   DEFINE_RES_IRQ(BXTWC_THRM_LVL1_IRQ),
 };
 
 static struct resource bcu_resources[] = {
diff --git a/drivers/thermal/intel_bxt_pmic_thermal.c 
b/drivers/thermal/intel_bxt_pmic_thermal.c
index 0f19a39..ef6b322 100644
--- a/drivers/thermal/intel_bxt_pmic_thermal.c
+++ b/drivers/thermal/intel_bxt_pmic_thermal.c
@@ -241,7 +241,7 @@ static int pmic_thermal_probe(struct platform_device *pdev)
}
 
regmap = pmic->regmap;
-   regmap_irq_chip = pmic->irq_chip_data_level2;
+   regmap_irq_chip = pmic->irq_chip_data;
 
pmic_irq_count = 0;
while ((irq = platform_get_irq(pdev, pmic_irq_count)) != -ENXIO) {
-- 
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 v6 4/6] mfd: intel_soc_pmic_bxtwc: Utilize devm_* functions in driver probe

2017-06-05 Thread sathyanarayanan . kuppuswamy
From: Kuppuswamy Sathyanarayanan 

Cleanup the resource allocation/free code in probe function by using
devm_* calls.

Signed-off-by: Kuppuswamy Sathyanarayanan 

Acked-for-MFD-by: Lee Jones 
Reviewed-by: Andy Shevchenko 
---
 drivers/mfd/intel_soc_pmic_bxtwc.c | 54 +-
 1 file changed, 18 insertions(+), 36 deletions(-)

Changes since v1:
 * None

Changes since v2:
 * Rebased on top of latest release.

Changes since v3:
 * None

Changes since v5:
 * None

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c 
b/drivers/mfd/intel_soc_pmic_bxtwc.c
index af11c43..feeda6e 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -399,45 +399,44 @@ static int bxtwc_probe(struct platform_device *pdev)
return ret;
}
 
-   ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
- IRQF_ONESHOT | IRQF_SHARED,
- 0, _regmap_irq_chip,
- >irq_chip_data);
+   ret = devm_regmap_add_irq_chip(>dev, pmic->regmap, pmic->irq,
+  IRQF_ONESHOT | IRQF_SHARED,
+  0, _regmap_irq_chip,
+  >irq_chip_data);
if (ret) {
dev_err(>dev, "Failed to add IRQ chip\n");
return ret;
}
 
-   ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
- IRQF_ONESHOT | IRQF_SHARED,
- 0, _regmap_irq_chip_level2,
- >irq_chip_data_level2);
+   ret = devm_regmap_add_irq_chip(>dev, pmic->regmap, pmic->irq,
+  IRQF_ONESHOT | IRQF_SHARED,
+  0, _regmap_irq_chip_level2,
+  >irq_chip_data_level2);
if (ret) {
dev_err(>dev, "Failed to add secondary IRQ chip\n");
-   goto err_irq_chip_level2;
+   return ret;
}
 
-   ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
- IRQF_ONESHOT | IRQF_SHARED,
- 0, _regmap_irq_chip_tmu,
- >irq_chip_data_tmu);
+   ret = devm_regmap_add_irq_chip(>dev, pmic->regmap, pmic->irq,
+  IRQF_ONESHOT | IRQF_SHARED,
+  0, _regmap_irq_chip_tmu,
+  >irq_chip_data_tmu);
if (ret) {
dev_err(>dev, "Failed to add TMU IRQ chip\n");
-   goto err_irq_chip_tmu;
+   return ret;
}
 
-   ret = mfd_add_devices(>dev, PLATFORM_DEVID_NONE, bxt_wc_dev,
- ARRAY_SIZE(bxt_wc_dev), NULL, 0,
- NULL);
+   ret = devm_mfd_add_devices(>dev, PLATFORM_DEVID_NONE, bxt_wc_dev,
+  ARRAY_SIZE(bxt_wc_dev), NULL, 0, NULL);
if (ret) {
dev_err(>dev, "Failed to add devices\n");
-   goto err_mfd;
+   return ret;
}
 
ret = sysfs_create_group(>dev.kobj, _group);
if (ret) {
dev_err(>dev, "Failed to create sysfs group %d\n", ret);
-   goto err_sysfs;
+   return ret;
}
 
/*
@@ -451,28 +450,11 @@ static int bxtwc_probe(struct platform_device *pdev)
BXTWC_MIRQLVL1_MCHGR, 0);
 
return 0;
-
-err_sysfs:
-   mfd_remove_devices(>dev);
-err_mfd:
-   regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_tmu);
-err_irq_chip_tmu:
-   regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2);
-err_irq_chip_level2:
-   regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
-
-   return ret;
 }
 
 static int bxtwc_remove(struct platform_device *pdev)
 {
-   struct intel_soc_pmic *pmic = dev_get_drvdata(>dev);
-
sysfs_remove_group(>dev.kobj, _group);
-   mfd_remove_devices(>dev);
-   regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
-   regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2);
-   regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_tmu);
 
return 0;
 }
-- 
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 v6 0/6] mfd: intel_soc_pmic_bxtwc: Add chained IRQ support

2017-06-05 Thread sathyanarayanan . kuppuswamy
From: Kuppuswamy Sathyanarayanan 

Following patch set adds chained IRQ support to WCOVE PMIC driver.

Changes since v3:
 * Added fix for typec wcove driver.

Changes since v4:
 * Squashed following two commits, to keep the patch set bisectable.
   usb: typec: typec_wcove: Use charger irq chip to get usbc virq
   mfd: intel_soc_pmic_bxtwc: use chained irqs for second level irq chips

Changes since v5:
* Changed irq->IRQ, pmic->PMIC in all commit messages.
* Squashed following two thermal driver related commits.
  mfd: intel_soc_pmic_bxtwc: Remove thermal second level irqs
  thermal: intel_bxt_pmic_thermal: Use first level PMIC thermal irq
* Squashed following two GPIO driver related commits.
  mfd: intel_soc_pmic_bxtwc: Remove second level irq for gpio device
  gpio: gpio-wcove: Use first level PMIC GPIO irq

Kuppuswamy Sathyanarayanan (6):
  mfd: intel_soc_pmic_bxtwc: Fix TMU interrupt index
  mfd: intel_soc_pmic_bxtwc: Remove thermal second level IRQs
  mfd: intel_soc_pmic_bxtwc: Remove second level IRQ for gpio device
  mfd: intel_soc_pmic_bxtwc: Utilize devm_* functions in driver probe
  mfd: intel_soc_pmic_bxtwc: Use chained IRQs for second level IRQ chips
  platform/x86: intel_bxtwc_tmu: Remove first level IRQ unmask

 drivers/gpio/gpio-wcove.c|  14 +-
 drivers/mfd/intel_soc_pmic_bxtwc.c   | 232 +--
 drivers/platform/x86/intel_bxtwc_tmu.c   |   4 -
 drivers/thermal/intel_bxt_pmic_thermal.c |   2 +-
 drivers/usb/typec/typec_wcove.c  |   2 +-
 include/linux/mfd/intel_soc_pmic.h   |   5 +-
 6 files changed, 175 insertions(+), 84 deletions(-)

-- 
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 v6 5/6] mfd: intel_soc_pmic_bxtwc: Use chained IRQs for second level IRQ chips

2017-06-05 Thread sathyanarayanan . kuppuswamy
From: Kuppuswamy Sathyanarayanan 

Whishkey cove PMIC has support to mask/unmask interrupts at two levels.
At first level we can mask/unmask interrupt domains like TMU, GPIO, ADC,
CHGR, BCU THERMAL and PWRBTN and at second level, it provides facility
to mask/unmask individual interrupts belong each of this domain. For
example, in case of TMU, at first level we have TMU interrupt domain,
and at second level we have two interrupts, wake alarm, system alarm that
belong to the TMU interrupt domain.

Currently, in this driver all first level IRQs are registered as part of
IRQ chip(bxtwc_regmap_irq_chip). By default, after you register the IRQ
chip from your driver, all IRQs in that chip will masked and can only be
enabled if that IRQ is requested using request_irq() call. This is the
default Linux IRQ behavior model. And whenever a dependent device that
belongs to PMIC requests only the second level IRQ and not explicitly
unmask the first level IRQ, then in essence the second level IRQ will
still be disabled. For example, if TMU device driver request wake_alarm
IRQ and not explicitly unmask TMU level 1 IRQ then according to the default
Linux IRQ model,  wake_alarm IRQ will still be disabled. So the proper
solution to fix this issue is to use the chained IRQ chip concept. We
should chain all the second level chip IRQs to the corresponding first
level IRQ. To do this, we need to create separate IRQ chips for every
group of second level IRQs.

In case of TMU, when adding second level IRQ chip, instead of using PMIC
IRQ we should use the corresponding first level IRQ. So the following
code will change from

ret = regmap_add_irq_chip(pmic->regmap, pmic->irq, ...)

to,

virq = regmap_irq_get_virq(>irq_chip_data, BXTWC_TMU_LVL1_IRQ);

ret = regmap_add_irq_chip(pmic->regmap, virq, ...)

In case of Whiskey Cove Type-C driver, Since USBC IRQ is moved under
charger level2 IRQ chip. We should use charger IRQ chip(irq_chip_data_chgr)
to get the USBC virtual IRQ number.

Signed-off-by: Kuppuswamy Sathyanarayanan 

Acked-for-MFD-by: Lee Jones 
---
 drivers/mfd/intel_soc_pmic_bxtwc.c | 168 ++---
 drivers/usb/typec/typec_wcove.c|   2 +-
 include/linux/mfd/intel_soc_pmic.h |   5 +-
 3 files changed, 143 insertions(+), 32 deletions(-)

Changes since v1:
 * Rebased on top of dev_* cleanup patch.
 * Fixed style & grammer issues reported by Lee Jones

Changes since v2:
 * Rebased on top of latest release.

Changes since v3:
 * None

Changes since v4:
 * Merged typec driver fix to this patch to prevent branch bisect issue.

Changes since v5:
 * Changed irq->IRQ and pmic -> PMIC.
 * Rearranged tmu related changes to make the patch more readable.
 * Modfied the virq request failure message to include chip name and pirq.

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c 
b/drivers/mfd/intel_soc_pmic_bxtwc.c
index feeda6e..15bc052 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -82,20 +82,28 @@ enum bxtwc_irqs {
BXTWC_PWRBTN_IRQ,
 };
 
-enum bxtwc_irqs_level2 {
-   /* Level 2 */
+enum bxtwc_irqs_bcu {
BXTWC_BCU_IRQ = 0,
-   BXTWC_ADC_IRQ,
-   BXTWC_USBC_IRQ,
+};
+
+enum bxtwc_irqs_adc {
+   BXTWC_ADC_IRQ = 0,
+};
+
+enum bxtwc_irqs_chgr {
+   BXTWC_USBC_IRQ = 0,
BXTWC_CHGR0_IRQ,
BXTWC_CHGR1_IRQ,
-   BXTWC_CRIT_IRQ,
 };
 
 enum bxtwc_irqs_tmu {
BXTWC_TMU_IRQ = 0,
 };
 
+enum bxtwc_irqs_crit {
+   BXTWC_CRIT_IRQ = 0,
+};
+
 static const struct regmap_irq bxtwc_regmap_irqs[] = {
REGMAP_IRQ_REG(BXTWC_PWRBTN_LVL1_IRQ, 0, BIT(0)),
REGMAP_IRQ_REG(BXTWC_TMU_LVL1_IRQ, 0, BIT(1)),
@@ -108,19 +116,28 @@ static const struct regmap_irq bxtwc_regmap_irqs[] = {
REGMAP_IRQ_REG(BXTWC_PWRBTN_IRQ, 1, 0x03),
 };
 
-static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
+static const struct regmap_irq bxtwc_regmap_irqs_bcu[] = {
REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 0, 0x1f),
-   REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 1, 0xff),
-   REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
-   REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
-   REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
-   REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
+};
+
+static const struct regmap_irq bxtwc_regmap_irqs_adc[] = {
+   REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 0, 0xff),
+};
+
+static const struct regmap_irq bxtwc_regmap_irqs_chgr[] = {
+   REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 0, BIT(5)),
+   REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 0, 0x1f),
+   REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 1, 0x1f),
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
 };
 
+static const struct regmap_irq bxtwc_regmap_irqs_crit[] = {
+   REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 0, 0x03),
+};
+
 static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
.name = "bxtwc_irq_chip",

Re: XHCI is slow during boot (bios/efi) and leaves many dmesg messages

2017-06-05 Thread Alex Henrie
2017-06-05 2:08 GMT-06:00 Maël Lavault :
> Anything I can do to help ? It is a very annoying issue for me, to
> boot, I have to select an old 4.10 kernel (which takes ages since usb
> is extremely slow on first boot), then reboot into a 4.11 kernel (usb
> works again as it should after a reboot) with a special drm fix
> (otherwise my computer freeze).

Run `dmesg | grep 'DMI:'` and if the BIOS version is older than
February 2017, reinstall Mac OS X and install the Sierra 10.12.5
update. That finally fixed the problem for me.

-Alex
--
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] net/{mii,smsc}: Make mii_ethtool_get_link_ksettings and smc_netdev_get_ecmd return void

2017-06-05 Thread David Miller
From: Yuval Shaia 
Date: Sun,  4 Jun 2017 20:22:00 +0300

> Make return value void since functions never returns meaningfull value.
> 
> Signed-off-by: Yuval Shaia 

Applied to net-next.
--
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: [v2 1/1] usb:host:xhci support option to disable xHCI 1.0 USB2 HW LPM

2017-06-05 Thread Mathias Nyman

On 05.06.2017 15:57, Thang Q. Nguyen wrote:

On Mon, Jun 5, 2017 at 6:14 PM, Mathias Nyman
 wrote:

On 20.05.2017 10:24, Thang Q. Nguyen wrote:


XHCI specification 1.1 does not require xHCI 1.0 compliant controllers
to always enable hardware USB2 LPM.
However, the current xHCI driver always enable it by setting HLE=1 when
seeing HLC=1. This makes certain xHCI controllers that have broken USB2
HW LPM fail to work as there is no way to disable this feature.
This patch adds support to control disabling USB2 Hardware LPM via
DT/ACPI attribute.



  Wouldn't it be better to just keep  xhci->hw_lpm_support = 0 if the host
doesn't support Hardware LPM Capability, (HLC)?

This should prevent all those extra steps getting here just to do nothing.

No, HLC = 0 means the host doesn't support Hardware LPM.
The problem here is the host support Hardware LPM but there is a bug
in host controller that make the LPM fail to work.



So the host support hw LPM, and has its HLC capability bit set,
but in the end it just doesn't work at all, and should be prevented.


When debugging the host controller, we detect there are some holes in
the current usb specifications which can can result in inter-operating
problems between USB Host Controller and USB PHY. To be more specific,
the specs have not clarified the resume recovery timing after the port
has just waken up from L1. This can lead to different interpretations
of the specs by Host Controller and PHY. What happened in our case is
that a Host controller cannot work with a PHY right after resuming
from L1 because these two Vendors have different views of the specs
regarding LPM timing after L1. These views are contradictory and
cannot work together.

If Host Controller and PHY are from the same vendor, they might have
some "internal handshake mechanisms" to avoid these holes of the USB
specs. However, these mechanisms are not standardized in USB specs;
and not all vendors follow these mechanisms. In fact, we have observed
this kind of "internal handshake mechanism" in HOST Controller and PHY
from SYNOPSYS DWC. So, we can say that if users use Host Controller
and PHY from different Vendors, the inteopering problems after waking
up from L1 are more likely to occur.


Can you explain the reason why you prefer preventing hw lpm inside the
xhci_set_usb2_hardware_lpm() function instead of preventing hw lpm usage
all together for this platform -i.e. by not setting xhci->hw_lpm_support


Again, something like:
if (temp & XHCI_HLC && !(xhci->quirks & XHCI_HW_LPM_BROKEN))
xhci->hw_lpm_support = 1;


The HW LPM can also be disabled (per device) in sysfs if needed.

This does not work. When the issue happens, the USB device is fail to
probe so no /sys interface created. Messages displayed when issue
happen is similar to:

[ 2846.677903] usb 1-1: reset high-speed USB device number 2 using xhci-hcd
[ 2882.037125] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
or
usb usb3-port1: disabled by hub (EMI?), re-enabling...
[   57.840237] usb 3-1: USB disconnect, device number 5


Ok, the sysfs entry is not useful in this case.

-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


[PATCH v2 2/2] usb: typec: ucsi: Add ACPI driver

2017-06-05 Thread Heikki Krogerus
Driver for ACPI UCSI interface method. This driver replaces
the previous UCSI driver drivers/usb/misc/ucsi.c.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/misc/Kconfig   |  26 --
 drivers/usb/misc/Makefile  |   1 -
 drivers/usb/misc/ucsi.c| 478 -
 drivers/usb/misc/ucsi.h| 215 -
 drivers/usb/typec/ucsi/Kconfig |  16 ++
 drivers/usb/typec/ucsi/Makefile|   2 +
 drivers/usb/typec/ucsi/ucsi_acpi.c | 158 
 7 files changed, 176 insertions(+), 720 deletions(-)
 delete mode 100644 drivers/usb/misc/ucsi.c
 delete mode 100644 drivers/usb/misc/ucsi.h
 create mode 100644 drivers/usb/typec/ucsi/ucsi_acpi.c

diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index 1d1d70d62a19..0f9f25db9163 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -275,29 +275,3 @@ config USB_CHAOSKEY
 
  To compile this driver as a module, choose M here: the
  module will be called chaoskey.
-
-config UCSI
-   tristate "USB Type-C Connector System Software Interface driver"
-   depends on ACPI
-   help
- UCSI driver is meant to be used as a convenience tool for desktop and
- server systems that are not equipped to handle USB in device mode. It
- will always select USB host role for the USB Type-C ports on systems
- that provide UCSI interface.
-
- USB Type-C Connector System Software Interface (UCSI) is a
- specification for an interface that allows the Operating System to
- control the USB Type-C ports on a system. Things the need controlling
- include the USB Data Role (host or device), and when USB Power
- Delivery is supported, the Power Role (source or sink). With USB
- Type-C connectors, when two dual role capable devices are attached
- together, the data role is selected randomly. Therefore it is
- important to give the OS a way to select the role. Otherwise the user
- would have to unplug and replug in order in order to attempt to swap
- the data and power roles.
-
- The UCSI specification can be downloaded from:
- 
http://www.intel.com/content/www/us/en/io/universal-serial-bus/usb-type-c-ucsi-spec.html
-
- To compile the driver as a module, choose M here: the module will be
- called ucsi.
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index f6ac6c99a6e6..7fdb45fc976f 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -27,7 +27,6 @@ obj-$(CONFIG_USB_HUB_USB251XB)+= usb251xb.o
 obj-$(CONFIG_USB_HSIC_USB3503) += usb3503.o
 obj-$(CONFIG_USB_HSIC_USB4604) += usb4604.o
 obj-$(CONFIG_USB_CHAOSKEY) += chaoskey.o
-obj-$(CONFIG_UCSI) += ucsi.o
 
 obj-$(CONFIG_USB_SISUSBVGA)+= sisusbvga/
 obj-$(CONFIG_USB_LINK_LAYER_TEST)  += lvstest.o
diff --git a/drivers/usb/misc/ucsi.c b/drivers/usb/misc/ucsi.c
deleted file mode 100644
index 07397bddefa3..
--- a/drivers/usb/misc/ucsi.c
+++ /dev/null
@@ -1,478 +0,0 @@
-/*
- * USB Type-C Connector System Software Interface driver
- *
- * Copyright (C) 2016, Intel Corporation
- * Author: Heikki Krogerus 
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include 
-#include 
-#include 
-#include 
-
-#include "ucsi.h"
-
-/* Double the time defined by MIN_TIME_TO_RESPOND_WITH_BUSY */
-#define UCSI_TIMEOUT_MS 20
-
-enum ucsi_status {
-   UCSI_IDLE = 0,
-   UCSI_BUSY,
-   UCSI_ERROR,
-};
-
-struct ucsi_connector {
-   int num;
-   struct ucsi *ucsi;
-   struct work_struct work;
-   struct ucsi_connector_capability cap;
-};
-
-struct ucsi {
-   struct device *dev;
-   struct ucsi_data __iomem *data;
-
-   enum ucsi_status status;
-   struct completion complete;
-   struct ucsi_capability cap;
-   struct ucsi_connector *connector;
-
-   /* device lock */
-   spinlock_t dev_lock;
-
-   /* PPM Communication lock */
-   struct mutex ppm_lock;
-
-   /* PPM communication flags */
-   unsigned long flags;
-#define EVENT_PENDING  0
-#define COMMAND_PENDING1
-};
-
-static int ucsi_acpi_cmd(struct ucsi *ucsi, struct ucsi_control *ctrl)
-{
-   uuid_le uuid = UUID_LE(0x6f8398c2, 0x7ca4, 0x11e4,
-  0xad, 0x36, 0x63, 0x10, 0x42, 0xb5, 0x00, 0x8f);
-   union acpi_object *obj;
-
-   ucsi->data->ctrl.raw_cmd = ctrl->raw_cmd;
-
-   obj = acpi_evaluate_dsm(ACPI_HANDLE(ucsi->dev), uuid.b, 1, 1, NULL);
-   if (!obj) {
-   dev_err(ucsi->dev, "%s: failed to evaluate _DSM\n", __func__);
-   return -EIO;
-   }
-
-   

[PATCH v2 0/3] New driver for UCSI (USB Type-C)

2017-06-05 Thread Heikki Krogerus
Hi,

This moves the current ucsi driver from drivers/usb/misc/ucsi.c to the
new USB Type-C class (drivers/usb/typec/). That allows us to finally do
role swapping.

The driver is now split into core library part, and ACPI driver. That
should make it easy to add support for other interface methods (first
most likely being I2C) later if needed.

Changes since v1:
- Added separate flag from pending ACK. Some new platforms generate "command
  complete" event on top of the normal "ACK complete" event with ACK commands.
  In such cases the driver has to be able to basically ignore the command
  completion in case of ACK and only finish acknowledge routine when the actual
  ACK complete event is received. Otherwise a new command may be queued to the
  PPM before the previous has fully completed.
- Added an explanation why we are handling the PPM initialization in a work as
  suggested by Guenter.
- Fixed ucsi_reset_ppm() by removing possibility of returning -ETIMEDOUT in case
  of success right before the time expires. Suggested by Guenter.
- Replaced useless "goto err;" with "break;" in ucsi_run_command() as suggested
  by Guenter.
- Removed traceback in case of failure from ucsi_run_command() which is not
  necessary as suggested by Guenter.
- Highlighting the fact that the timeouts are in milliseconds by using _MS
  ending with the definition (UCSI_TIMEOUT_MS and UCSI_SWAP_TIMEOUT_MS) as
  suggested by Guenter.
- Including also  in ucsi.h as suggested by Guenter.
- In ucsi_acpi.c, explicitly pointing out in the comment that we can not use
  devm_ioremap_resource() as suggested by Guenter.


Heikki Krogerus (2):
  usb: typec: Add support for UCSI interface
  usb: typec: ucsi: Add ACPI driver

 drivers/usb/misc/Kconfig|  26 --
 drivers/usb/misc/Makefile   |   1 -
 drivers/usb/misc/ucsi.c | 478 ---
 drivers/usb/typec/Kconfig   |   2 +
 drivers/usb/typec/Makefile  |   1 +
 drivers/usb/typec/ucsi/Kconfig  |  38 ++
 drivers/usb/typec/ucsi/Makefile |   9 +
 drivers/usb/typec/ucsi/debug.h  |  64 +++
 drivers/usb/typec/ucsi/trace.c  |   2 +
 drivers/usb/typec/ucsi/trace.h  | 143 ++
 drivers/usb/typec/ucsi/ucsi.c   | 790 
 drivers/usb/{misc => typec/ucsi}/ucsi.h | 187 ++--
 drivers/usb/typec/ucsi/ucsi_acpi.c  | 158 +++
 13 files changed, 1358 insertions(+), 541 deletions(-)
 delete mode 100644 drivers/usb/misc/ucsi.c
 create mode 100644 drivers/usb/typec/ucsi/Kconfig
 create mode 100644 drivers/usb/typec/ucsi/Makefile
 create mode 100644 drivers/usb/typec/ucsi/debug.h
 create mode 100644 drivers/usb/typec/ucsi/trace.c
 create mode 100644 drivers/usb/typec/ucsi/trace.h
 create mode 100644 drivers/usb/typec/ucsi/ucsi.c
 rename drivers/usb/{misc => typec/ucsi}/ucsi.h (60%)
 create mode 100644 drivers/usb/typec/ucsi/ucsi_acpi.c

-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/2] usb: typec: Add support for UCSI interface

2017-06-05 Thread Heikki Krogerus
UCSI - USB Type-C Connector System Software Interface - is a
specification that defines set of registers and data
structures for controlling the USB Type-C ports. It's
designed for systems where an embedded controller (EC) is in
charge of the USB Type-C PHY or USB Power Delivery
controller. It is designed for systems with EC, but it is
not limited to them, and for example some USB Power Delivery
controllers will use it as their direct control interface.

With UCSI the EC (or USB PD controller) acts as the port
manager, implementing all USB Type-C and Power Delivery state
machines. The OS can use the interfaces for reading the
status of the ports and controlling basic operations like
role swapping.

The UCSI specification highlights the fact that it does not
define the interface method (PCI/I2C/ACPI/etc.).
Therefore the driver is implemented as library and every
supported interface method needs its own driver. Driver for
ACPI is provided in separate patch following this one.

The initial driver includes support for all required
features from UCSI specification version 1.0 (getting
connector capabilities and status, and support for power and
data role swapping), but none of the optional UCSI features
(alternate modes, power source capabilities, and cable
capabilities).

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/Kconfig   |   2 +
 drivers/usb/typec/Makefile  |   1 +
 drivers/usb/typec/ucsi/Kconfig  |  22 ++
 drivers/usb/typec/ucsi/Makefile |   7 +
 drivers/usb/typec/ucsi/debug.h  |  64 
 drivers/usb/typec/ucsi/trace.c  |   2 +
 drivers/usb/typec/ucsi/trace.h  | 143 
 drivers/usb/typec/ucsi/ucsi.c   | 790 
 drivers/usb/typec/ucsi/ucsi.h   | 330 +
 9 files changed, 1361 insertions(+)
 create mode 100644 drivers/usb/typec/ucsi/Kconfig
 create mode 100644 drivers/usb/typec/ucsi/Makefile
 create mode 100644 drivers/usb/typec/ucsi/debug.h
 create mode 100644 drivers/usb/typec/ucsi/trace.c
 create mode 100644 drivers/usb/typec/ucsi/trace.h
 create mode 100644 drivers/usb/typec/ucsi/ucsi.c
 create mode 100644 drivers/usb/typec/ucsi/ucsi.h

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index dfcfe459b7cf..bc1b7745f1d4 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -19,4 +19,6 @@ config TYPEC_WCOVE
  To compile this driver as module, choose M here: the module will be
  called typec_wcove
 
+source "drivers/usb/typec/ucsi/Kconfig"
+
 endmenu
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index b9cb862221af..bc214f15f1b5 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_TYPEC)+= typec.o
 obj-$(CONFIG_TYPEC_WCOVE)  += typec_wcove.o
+obj-$(CONFIG_TYPEC_UCSI)   += ucsi/
diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
new file mode 100644
index ..679ba6648396
--- /dev/null
+++ b/drivers/usb/typec/ucsi/Kconfig
@@ -0,0 +1,22 @@
+config TYPEC_UCSI
+   tristate "USB Type-C Connector System Software Interface driver"
+   select TYPEC
+   help
+ USB Type-C Connector System Software Interface (UCSI) is a
+ specification for an interface that allows the operating system to
+ control the USB Type-C ports. On UCSI system the USB Type-C ports
+ function autonomously by default, but in order to get the status of
+ the ports and support basic operations like role swapping, the driver
+ is required. UCSI is available on most of the new Intel based systems
+ that are equipped with Embedded Controller and USB Type-C ports.
+
+ UCSI specification does not define the interface method, so depending
+ on the platform, ACPI, PCI, I2C, etc. may be used. Therefore this
+ driver only provides the core part, and separate drivers are needed
+ for every supported interface method.
+
+ The UCSI specification can be downloaded from:
+ 
http://www.intel.com/content/www/us/en/io/universal-serial-bus/usb-type-c-ucsi-spec.html
+
+ To compile the driver as a module, choose M here: the module will be
+ called typec_ucsi.
diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
new file mode 100644
index ..87dd6ee6c9f3
--- /dev/null
+++ b/drivers/usb/typec/ucsi/Makefile
@@ -0,0 +1,7 @@
+CFLAGS_trace.o := -I$(src)
+
+obj-$(CONFIG_TYPEC_UCSI)   += typec_ucsi.o
+
+typec_ucsi-y   := ucsi.o
+
+typec_ucsi-$(CONFIG_FTRACE)+= trace.o
diff --git a/drivers/usb/typec/ucsi/debug.h b/drivers/usb/typec/ucsi/debug.h
new file mode 100644
index ..87d0cd20597a
--- /dev/null
+++ b/drivers/usb/typec/ucsi/debug.h
@@ -0,0 +1,64 @@
+#ifndef __UCSI_DEBUG_H
+#define __UCSI_DEBUG_H
+
+#include "ucsi.h"
+
+static const char * const ucsi_cmd_strs[] = {
+

[PATCH v2] USB: add usbfs ioctl to retrieve the connection speed

2017-06-05 Thread Alan Stern
The usbfs interface does not provide any way for the user to learn the
speed at which a device is connected.  The current API includes a
USBDEVFS_CONNECTINFO ioctl, but all it provides is the device's
address and a one-bit value indicating whether the connection is low
speed.  That may have sufficed in the era of USB-1.1, but it isn't
good enough today.

This patch introduces a new ioctl, USBDEVFS_GET_SPEED, which returns a
numeric value indicating the speed of the connection: unknown, low,
full, high, wireless, super, or super-plus.

Similar information (not exactly the same) is available through sysfs,
but it seems reasonable to provide the actual value in usbfs.

Signed-off-by: Alan Stern 
Reported-by: Reinhard Huck 

---

v2: Use the USB_SPEED_* values already defined in
include/uapi/linux/usb/ch9.h instead of inventing new constants.


[as1832b]


 drivers/usb/core/devio.c  |3 +++
 include/uapi/linux/usbdevice_fs.h |6 ++
 2 files changed, 9 insertions(+)

Index: usb-4.x/drivers/usb/core/devio.c
===
--- usb-4.x.orig/drivers/usb/core/devio.c
+++ usb-4.x/drivers/usb/core/devio.c
@@ -2537,6 +2537,9 @@ static long usbdev_do_ioctl(struct file
case USBDEVFS_DROP_PRIVILEGES:
ret = proc_drop_privileges(ps, p);
break;
+   case USBDEVFS_GET_SPEED:
+   ret = ps->dev->speed;
+   break;
}
 
  done:
Index: usb-4.x/include/uapi/linux/usbdevice_fs.h
===
--- usb-4.x.orig/include/uapi/linux/usbdevice_fs.h
+++ usb-4.x/include/uapi/linux/usbdevice_fs.h
@@ -156,6 +156,11 @@ struct usbdevfs_streams {
unsigned char eps[0];
 };
 
+/*
+ * USB_SPEED_* values returned by USBDEVFS_GET_SPEED are defined in
+ * linux/usb/ch9.h
+ */
+
 #define USBDEVFS_CONTROL   _IOWR('U', 0, struct usbdevfs_ctrltransfer)
 #define USBDEVFS_CONTROL32   _IOWR('U', 0, struct 
usbdevfs_ctrltransfer32)
 #define USBDEVFS_BULK  _IOWR('U', 2, struct usbdevfs_bulktransfer)
@@ -190,5 +195,6 @@ struct usbdevfs_streams {
 #define USBDEVFS_ALLOC_STREAMS _IOR('U', 28, struct usbdevfs_streams)
 #define USBDEVFS_FREE_STREAMS  _IOR('U', 29, struct usbdevfs_streams)
 #define USBDEVFS_DROP_PRIVILEGES   _IOW('U', 30, __u32)
+#define USBDEVFS_GET_SPEED _IO('U', 31)
 
 #endif /* _UAPI_LINUX_USBDEVICE_FS_H */

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v14 4/7] usb: core: add power sequence handling for USB devices

2017-06-05 Thread Alan Stern
On Mon, 5 Jun 2017, Peter Chen wrote:

> On Thu, May 18, 2017 at 08:49:00AM +0800, Peter Chen wrote:
> > Some hard-wired USB devices need to do power sequence to let the
> > device work normally, the typical power sequence like: enable USB
> > PHY clock, toggle reset pin, etc. But current Linux USB driver
> > lacks of such code to do it, it may cause some hard-wired USB devices
> > works abnormal or can't be recognized by controller at all.
> > 
> > In this patch, it calls power sequence library APIs to finish
> > the power sequence events. It will do power on sequence at hub's
> > probe for all devices under this hub (includes root hub).
> > At hub_disconnect, it will do power off sequence which is at powered
> > on list.
> > 
> 
> Greg, Alan, would you please help on reviewing it? It seems Rafael
> is waiting for USB MAINTAINERS's comments or ack. It resolves some
> USB HUB issues for several persons.
> 
> Peter
> 
> > Signed-off-by: Peter Chen 
> > Tested-by Joshua Clayton 
> > Tested-by: Maciej S. Szmigiero 
> > Reviewed-by: Vaibhav Hiremath 

Acked-by: Alan Stern 

> > ---
> >  drivers/usb/Kconfig|  1 +
> >  drivers/usb/core/hub.c | 49 
> > +
> >  drivers/usb/core/hub.h |  1 +
> >  3 files changed, 47 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> > index 939a63b..b6f626e 100644
> > --- a/drivers/usb/Kconfig
> > +++ b/drivers/usb/Kconfig
> > @@ -39,6 +39,7 @@ config USB
> > tristate "Support for Host-side USB"
> > depends on USB_ARCH_HAS_HCD
> > select USB_COMMON
> > +   select POWER_SEQUENCE
> > select NLS  # for UTF-8 strings
> > ---help---
> >   Universal Serial Bus (USB) is a specification for a serial bus
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 9dca59e..7a67296 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -28,6 +28,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  
> >  #include 
> >  #include 
> > @@ -1619,6 +1620,7 @@ static void hub_disconnect(struct usb_interface *intf)
> > hub->error = 0;
> > hub_quiesce(hub, HUB_DISCONNECT);
> >  
> > +   of_pwrseq_off_list(>pwrseq_on_list);

Things like this look a little peculiar -- you are passing an "on_list" 
to a function named "...off_list".

How about naming the new field hub->pwrseq_list instead?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: g_mass_storage emulation of flash drive - difficulties with passing vendor/product ID

2017-06-05 Thread Alan Stern
On Sun, 4 Jun 2017, Alan Robertson wrote:

> Hi
> 
> Apologies for my first post here being a request for help, but after
> extensive searching online I've not yet been able to find and answer
> and am hopeful the expertise here may be able to help!
> 
> I have a Raspberry Pi Zero W that I'm using in OTG/gadget mode. I've
> activated dwc2 USB, partitioned and formatted the file that is to be
> used as filesystem and loaded g_mass_storage.
> 
> It is working fine in Windows - I can connect and see the contents,
> add/delete files, eject and reconnect seeing changes fine. I can mount
> it in Raspbian to check contents and can also attach it to other
> devices and interact with it fine.
> 
> However, I've got some devices that are very picky about what USB
> devices they will connect to (solely flash drives). The problem is the
> OTG mass storage gadget is being identified as "File-Stor Gadget (Rev:
> 0404)" for device type, which is causing it to be rejected.
> 
> I can successfully use a USB flash drive in the target devices and
> have scraped the vendor/product ID, etc from this flash drive to see
> if I can mimic them via g_mass_storage.
> 
> I have adjusted modprobe to the following:
> 
> $ sudo modprobe g_mass_storage file=/home/pi/piusb.bin stall=0
> removable=1 idVendor=0x0781 idProduct=0x5572 bcdDevice=0x011a
> iManufacturer="SanDisk" iProduct="Cruzer Switch"
> iSerialNumber="123456789012"
> 
> Unfortunately this doesn't seem to have made any difference and it is
> still rejected by the target devices.
> 
> As far as I can tell the above parameters are getting passed through correctly
> $ ls /sys/module/g_mass_storage/parameters/*
> /sys/module/g_mass_storage/parameters/bcdDevice
> /sys/module/g_mass_storage/parameters/cdrom
> /sys/module/g_mass_storage/parameters/file
> /sys/module/g_mass_storage/parameters/idProduct
> /sys/module/g_mass_storage/parameters/idVendor
> /sys/module/g_mass_storage/parameters/iManufacturer
> /sys/module/g_mass_storage/parameters/iProduct
> /sys/module/g_mass_storage/parameters/iSerialNumber
> /sys/module/g_mass_storage/parameters/luns
> /sys/module/g_mass_storage/parameters/nofua
> /sys/module/g_mass_storage/parameters/removable
> /sys/module/g_mass_storage/parameters/ro
> /sys/module/g_mass_storage/parameters/stall
> 
> $ cat /sys/module/g_mass_storage/parameters/*
> 282
> /home/pi/piusb.bin
> 21874
> 1921
> SanDisk
> Cruzer_Switch
> 123456789012
> 0
> Y
> N
> 
> However when connecting to Windows, here's what the USB mass storage
> displays - it appears that the iSerialNumber is making it through, but
> nothing else is being altered.  My router shows similar issues,
> identifying it as "File-Stor Gadget (Rev: 0404)"
> 
> USBSTOR\DISK_LINUX_FILE-STOR_GADGET_0404\123456789012&0
> USBSTOR\DiskLinux___File-Stor_Gadget0404
> USBSTOR\DiskLinux___File-Stor_Gadget
> USBSTOR\DiskLinux___
> USBSTOR\Linux___File-Stor_Gadget0
> Linux___File-Stor_Gadget0
> USBSTOR\GenDisk
> GenDisk
> 
> By comparison, here's what SanDisk shows:
> 
> USBSTOR\DISK_SANDISK_CRUZER_SWITCH_1.26\4C532015741508522393&0
> USBSTOR\DiskSanDisk_Cruzer_Switch___1.26
> USBSTOR\DiskSanDisk_Cruzer_Switch___
> USBSTOR\DiskSanDisk_
> USBSTOR\SanDisk_Cruzer_Switch___1
> SanDisk_Cruzer_Switch___1
> USBSTOR\GenDisk
> GenDisk

Never mind Windows -- if you connect your gadget to a computer running 
Linux, what does "lsusb -v" show?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [v2 1/1] usb:host:xhci support option to disable xHCI 1.0 USB2 HW LPM

2017-06-05 Thread Thang Q. Nguyen
On Mon, Jun 5, 2017 at 6:14 PM, Mathias Nyman
 wrote:
> On 20.05.2017 10:24, Thang Q. Nguyen wrote:
>>
>> XHCI specification 1.1 does not require xHCI 1.0 compliant controllers
>> to always enable hardware USB2 LPM.
>> However, the current xHCI driver always enable it by setting HLE=1 when
>> seeing HLC=1. This makes certain xHCI controllers that have broken USB2
>> HW LPM fail to work as there is no way to disable this feature.
>> This patch adds support to control disabling USB2 Hardware LPM via
>> DT/ACPI attribute.
>>
>> Signed-off-by: Tung Nguyen 
>> Signed-off-by: Thang Q. Nguyen 
>> ---
>> Changes since v1:
>>   - Update DT/ACPI attribute and corresponding codes from HLE to LPM to
>>be consistent with other attribute names.
>> ---
>>   Documentation/devicetree/bindings/usb/usb-xhci.txt |1 +
>>   drivers/usb/host/xhci-plat.c   |3 +++
>>   drivers/usb/host/xhci.c|7 ++-
>>   drivers/usb/host/xhci.h|1 +
>>   4 files changed, 11 insertions(+), 1 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> index 2d80b60..96f1ac0 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> @@ -26,6 +26,7 @@ Required properties:
>>
>>   Optional properties:
>> - clocks: reference to a clock
>> +  - usb2-lpm-disable: disable USB2 LPM for hardware does not support it
>> - usb3-lpm-capable: determines if platform is USB3 LPM capable
>> - quirk-broken-port-ped: set if the controller has broken port disable
>> mechanism
>>
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 7c2a9e7..950eaf0 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -267,6 +267,9 @@ static int xhci_plat_probe(struct platform_device
>> *pdev)
>> goto disable_clk;
>> }
>>
>> +   if (device_property_read_bool(>dev, "usb2-lpm-disable"))
>> +   xhci->quirks |= XHCI_USB2_LPM_DISABLE;
>> +
>> if (device_property_read_bool(sysdev, "usb3-lpm-capable"))
>> xhci->quirks |= XHCI_LPM_SUPPORT;
>>
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index 2d13102..47d51d4 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -4055,6 +4055,7 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd
>> *hcd,
>> unsigned long   flags;
>> int hird, exit_latency;
>> int ret;
>> +   int usb2_lpm_disable = 0;
>>
>> if (hcd->speed >= HCD_USB3 || !xhci->hw_lpm_support ||
>> !udev->lpm_capable)
>> @@ -4079,7 +4080,11 @@ static int xhci_set_usb2_hardware_lpm(struct
>> usb_hcd *hcd,
>> xhci_dbg(xhci, "%s port %d USB2 hardware LPM\n",
>> enable ? "enable" : "disable", port_num + 1);
>>
>> -   if (enable) {
>> +   /* Check for optional disable USB2 LPM if XHCI 1.0 */
>> +   if ((xhci->quirks & XHCI_USB2_LPM_DISABLE) && (xhci->hci_version
>> == 0x100))
>> +   usb2_lpm_disable = 1;
>> +
>> +   if (enable && !usb2_lpm_disable) {
>
>  Wouldn't it be better to just keep  xhci->hw_lpm_support = 0 if the host
> doesn't support Hardware LPM Capability, (HLC)?
>
> This should prevent all those extra steps getting here just to do nothing.
No, HLC = 0 means the host doesn't support Hardware LPM.
The problem here is the host support Hardware LPM but there is a bug
in host controller that make the LPM fail to work.

When debugging the host controller, we detect there are some holes in
the current usb specifications which can can result in inter-operating
problems between USB Host Controller and USB PHY. To be more specific,
the specs have not clarified the resume recovery timing after the port
has just waken up from L1. This can lead to different interpretations
of the specs by Host Controller and PHY. What happened in our case is
that a Host controller cannot work with a PHY right after resuming
from L1 because these two Vendors have different views of the specs
regarding LPM timing after L1. These views are contradictory and
cannot work together.

If Host Controller and PHY are from the same vendor, they might have
some "internal handshake mechanisms" to avoid these holes of the USB
specs. However, these mechanisms are not standardized in USB specs;
and not all vendors follow these mechanisms. In fact, we have observed
this kind of "internal handshake mechanism" in HOST Controller and PHY
from SYNOPSYS DWC. So, we can say that if users use Host Controller
and PHY from different Vendors, the inteopering problems after waking
up from L1 are more likely to occur.
>
> the quirk check could be done in xhci-mem.c in xhci_add_in_port()

Re: [PATCH] xhci: remove endpoint ring cache

2017-06-05 Thread Mathias Nyman

On 05.06.2017 11:46, Anurag Kumar Vulisha wrote:

Hi Mathias,

I have tested this patch with my platform and it works fine

Thanks,
Anurag Kumar Vulisha


Thanks, I'll add your tested-by tag to it as well, and add it to my tree

-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: [v2 1/1] usb:host:xhci support option to disable xHCI 1.0 USB2 HW LPM

2017-06-05 Thread Mathias Nyman

On 20.05.2017 10:24, Thang Q. Nguyen wrote:

XHCI specification 1.1 does not require xHCI 1.0 compliant controllers
to always enable hardware USB2 LPM.
However, the current xHCI driver always enable it by setting HLE=1 when
seeing HLC=1. This makes certain xHCI controllers that have broken USB2
HW LPM fail to work as there is no way to disable this feature.
This patch adds support to control disabling USB2 Hardware LPM via
DT/ACPI attribute.

Signed-off-by: Tung Nguyen 
Signed-off-by: Thang Q. Nguyen 
---
Changes since v1:
  - Update DT/ACPI attribute and corresponding codes from HLE to LPM to
   be consistent with other attribute names.
---
  Documentation/devicetree/bindings/usb/usb-xhci.txt |1 +
  drivers/usb/host/xhci-plat.c   |3 +++
  drivers/usb/host/xhci.c|7 ++-
  drivers/usb/host/xhci.h|1 +
  4 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt 
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index 2d80b60..96f1ac0 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -26,6 +26,7 @@ Required properties:

  Optional properties:
- clocks: reference to a clock
+  - usb2-lpm-disable: disable USB2 LPM for hardware does not support it
- usb3-lpm-capable: determines if platform is USB3 LPM capable
- quirk-broken-port-ped: set if the controller has broken port disable 
mechanism

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 7c2a9e7..950eaf0 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -267,6 +267,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
goto disable_clk;
}

+   if (device_property_read_bool(>dev, "usb2-lpm-disable"))
+   xhci->quirks |= XHCI_USB2_LPM_DISABLE;
+
if (device_property_read_bool(sysdev, "usb3-lpm-capable"))
xhci->quirks |= XHCI_LPM_SUPPORT;

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2d13102..47d51d4 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4055,6 +4055,7 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
unsigned long   flags;
int hird, exit_latency;
int ret;
+   int usb2_lpm_disable = 0;

if (hcd->speed >= HCD_USB3 || !xhci->hw_lpm_support ||
!udev->lpm_capable)
@@ -4079,7 +4080,11 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd 
*hcd,
xhci_dbg(xhci, "%s port %d USB2 hardware LPM\n",
enable ? "enable" : "disable", port_num + 1);

-   if (enable) {
+   /* Check for optional disable USB2 LPM if XHCI 1.0 */
+   if ((xhci->quirks & XHCI_USB2_LPM_DISABLE) && (xhci->hci_version == 
0x100))
+   usb2_lpm_disable = 1;
+
+   if (enable && !usb2_lpm_disable) {
 
Wouldn't it be better to just keep  xhci->hw_lpm_support = 0 if the host

doesn't support Hardware LPM Capability, (HLC)?

This should prevent all those extra steps getting here just to do nothing.

the quirk check could be done in xhci-mem.c in xhci_add_in_port()

if (temp & XHCI_HLC &&) {
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"xHCI 1.0: support USB2 hardware lpm");
xhci->hw_lpm_support = 1;
}

Sidenote, the only purpose of the HLC bit is to inform the driver that the host 
port
is HLC (Hardware LPM Capable). No wonder driver sets HW LPM if HLC is set.

The HW LPM can also be disabled (per device) in sysfs if needed.

-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 v6] usb: typec: Add a sysfs node to manage port type

2017-06-05 Thread Heikki Krogerus
Hi,

Sorry for the late reply. I was unable to follow my email last week.

On Tue, May 30, 2017 at 12:39:53PM -0700, Badhri Jagan Sridharan wrote:
> User space applications in some cases have the need to enforce a
> specific port type(DFP/UFP/DRP). This change allows userspace to
> attempt setting the desired port type. Low level drivers can
> however reject the request if the specific port type is not supported.
> 
> Signed-off-by: Badhri Jagan Sridharan 
> Reviewed-by: Guenter Roeck 

It looks good to me. In case Greg has not yet picked this:

Acked-by: Heikki Krogerus 


Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] usb-phy-generic: Add support to SMSC USB3315

2017-06-05 Thread Fabien Lahoudere
On Mon, 2017-06-05 at 17:43 +0800, Peter Chen wrote:
> On Mon, Jun 05, 2017 at 10:57:00AM +0200, Fabien Lahoudere wrote:
> > On Fri, 2017-06-02 at 15:00 -0700, Stephen Boyd wrote:
> > > On 05/26, Fabien Lahoudere wrote:
> > > > Hello
> > > > 
> > > > I modify ci_hrdc_imx_probe to bypass "data->phy = 
> > > > devm_usb_get_phy_by_phandle(>dev,
> > > > "fsl,usbphy", 0);". Everything works as expected and call ci_ulpi_init.
> > > > 
> > > > The problem is that in ci_ulpi_init, before calling "ci->ulpi = 
> > > > ulpi_register_interface(ci-
> > > > >dev,
> > > > >ulpi_ops);" (to initialize our phy), "hw_phymode_configure(ci);" 
> > > > is called which is the
> > > > original function that make our system to hang.
> > > > 
> > > > Our phy is not initialised before calling ulpi_register_interface so I 
> > > > don't understand how
> > > > the
> > > > phy
> > > > can reply if it is not out of reset state.
> > > 
> > > I haven't see any problem in hw_phymode_configure(). What's the
> > > value of ci->platdata->phy_mode? USBPHY_INTERFACE_MODE_ULPI? If
> > > you phy needs to be taken out of reset to reply to the ulpi reads
> > > of the vendor/product ids, then it sounds like you have a similar
> > > situation to what I had. I needed to turn on some regulators to
> > > get those reads to work, otherwise they would fail, but knowing
> > > what needed to be turned on basically meant I needed to probe the
> > > ulpi driver so probing the ids wasn't going to be useful. So on
> > > my device the reads for the ids go through, but they get all
> > > zeroes back, which is actually ok because there aren't any bits
> > > set on my devices anyway. After the reads see 0, we fallback to
> > > DT matching, which avoids the "bring it out of reset/power it on"
> > > sorts of problems entirely.
> > > 
> > 
> > Yes the phy mode is configured to USBPHY_INTERFACE_MODE_ULPI.
> > Indeed, this phy need to be out of reset to work. For example everything 
> > works fine if I call 
> > "_ci_usb_phy_init(ci);" before calling "hw_phymode_configure(ci);"
> > This function only init reset GPIO and clock.
> > 
> > For information, the original patch I have to fix the issue:
> > 
> > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > index 79ad8e9..21aaff1 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -391,6 +391,7 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
> >     case USBPHY_INTERFACE_MODE_UTMI:
> >     case USBPHY_INTERFACE_MODE_UTMIW:
> >     case USBPHY_INTERFACE_MODE_HSIC:
> > +   case USBPHY_INTERFACE_MODE_ULPI:
> >     ret = _ci_usb_phy_init(ci);
> >     if (!ret)
> >     hw_wait_phy_stable();
> > @@ -398,7 +399,6 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
> >     return ret;
> >     hw_phymode_configure(ci);
> >     break;
> > -   case USBPHY_INTERFACE_MODE_ULPI:
> >     case USBPHY_INTERFACE_MODE_SERIAL:
> >     hw_phymode_configure(ci);
> >     ret = _ci_usb_phy_init(ci);
> > -- 
> 
> Currently, the hw_phymode_configure is called twice for ULPI PHY, the
> two execution are between _ci_usb_phy_init, would you test which one
> causes hang? If the second causes hang, you can make a patch for
> hw_phymode_configure that if the required PORTSC_PTS is the same
> the value in register, do noop.

The first one hangs, _ci_usb_phy_init is not called due to hang.

--
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] usb-phy-generic: Add support to SMSC USB3315

2017-06-05 Thread Peter Chen
On Mon, Jun 05, 2017 at 10:57:00AM +0200, Fabien Lahoudere wrote:
> On Fri, 2017-06-02 at 15:00 -0700, Stephen Boyd wrote:
> > On 05/26, Fabien Lahoudere wrote:
> > > Hello
> > > 
> > > I modify ci_hrdc_imx_probe to bypass "data->phy = 
> > > devm_usb_get_phy_by_phandle(>dev,
> > > "fsl,usbphy", 0);". Everything works as expected and call ci_ulpi_init.
> > > 
> > > The problem is that in ci_ulpi_init, before calling "ci->ulpi = 
> > > ulpi_register_interface(ci->dev,
> > > >ulpi_ops);" (to initialize our phy), "hw_phymode_configure(ci);" is 
> > > called which is the
> > > original function that make our system to hang.
> > > 
> > > Our phy is not initialised before calling ulpi_register_interface so I 
> > > don't understand how the
> > > phy
> > > can reply if it is not out of reset state.
> > 
> > I haven't see any problem in hw_phymode_configure(). What's the
> > value of ci->platdata->phy_mode? USBPHY_INTERFACE_MODE_ULPI? If
> > you phy needs to be taken out of reset to reply to the ulpi reads
> > of the vendor/product ids, then it sounds like you have a similar
> > situation to what I had. I needed to turn on some regulators to
> > get those reads to work, otherwise they would fail, but knowing
> > what needed to be turned on basically meant I needed to probe the
> > ulpi driver so probing the ids wasn't going to be useful. So on
> > my device the reads for the ids go through, but they get all
> > zeroes back, which is actually ok because there aren't any bits
> > set on my devices anyway. After the reads see 0, we fallback to
> > DT matching, which avoids the "bring it out of reset/power it on"
> > sorts of problems entirely.
> > 
> 
> Yes the phy mode is configured to USBPHY_INTERFACE_MODE_ULPI.
> Indeed, this phy need to be out of reset to work. For example everything 
> works fine if I call 
> "_ci_usb_phy_init(ci);" before calling "hw_phymode_configure(ci);"
> This function only init reset GPIO and clock.
> 
> For information, the original patch I have to fix the issue:
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 79ad8e9..21aaff1 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -391,6 +391,7 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
>   case USBPHY_INTERFACE_MODE_UTMI:
>   case USBPHY_INTERFACE_MODE_UTMIW:
>   case USBPHY_INTERFACE_MODE_HSIC:
> + case USBPHY_INTERFACE_MODE_ULPI:
>   ret = _ci_usb_phy_init(ci);
>   if (!ret)
>   hw_wait_phy_stable();
> @@ -398,7 +399,6 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
>   return ret;
>   hw_phymode_configure(ci);
>   break;
> - case USBPHY_INTERFACE_MODE_ULPI:
>   case USBPHY_INTERFACE_MODE_SERIAL:
>   hw_phymode_configure(ci);
>   ret = _ci_usb_phy_init(ci);
> -- 

Currently, the hw_phymode_configure is called twice for ULPI PHY, the
two execution are between _ci_usb_phy_init, would you test which one
causes hang? If the second causes hang, you can make a patch for
hw_phymode_configure that if the required PORTSC_PTS is the same
the value in register, do noop.

-- 

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 v3] xhci: AMD Promontory USB disable port support

2017-06-05 Thread Mathias Nyman

On 31.05.2017 10:41, Jiahau Chang wrote:

v3: Fix some checkpatch.pl warnings

For AMD Promontory xHCI host, although you can disable USB 2.0 ports in BIOS
settings, those ports will be enabled anyway after you remove a device on
that port and re-plug it in again. It's a known limitation of the chip.
As a workaround we can clear the PORT_WAKE_BITS.

Signed-off-by: Jiahau Chang 
---
  drivers/usb/host/xhci-hub.c | 19 ++-
  drivers/usb/host/xhci-pci.c | 13 +
  drivers/usb/host/xhci.h |  2 ++
  3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0dde49c..aad32c6 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1218,12 +1218,19 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, 
u16 wValue,
xhci_dbg(xhci, "set port reset, actual port %d status  = 
0x%x\n", wIndex, temp);
break;
case USB_PORT_FEAT_REMOTE_WAKE_MASK:
-   xhci_set_remote_wake_mask(xhci, port_array,
+   if ((xhci->quirks & XHCI_U2_DISABLE_WAKE) && (hcd->speed 
< HCD_USB3)) {
+   temp = readl(port_array[wIndex]);
+   xhci_dbg(xhci, "skip set port remote wake mask, 
"
+   "actual port %d status  = 
0x%x\n",
+   wIndex, temp);


Does this work?

If I remember correctly USB_PORT_FEAT_REMOTE_WAKE_MASK is a USB3 only feature.
The (hcd->speed < HCD_USB3) condition should never be true here.

-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 v4 0/3] USB Audio Gadget refactoring

2017-06-05 Thread Felipe Balbi

Hi,

Ruslan Bilovol  writes:
>> Ruslan Bilovol  writes:
>>> I came to this patch series when wanted to do two things:
>>>  - use UAC1 as virtual ALSA sound card on gadget side,
>>>just like UAC2 is used so it's possible to do rate
>>>resampling
>>>  - have both playback/capture support in UAC1
>>>
>>> Since I wanted to have same behavior for both UAC1/UAC2,
>>> obviously I've got an utility part (u_audio.c) for
>>> virtual ALSA sound card handling like we have
>>> for ethernet(u_ether) or serial(u_serial) functions.
>>> Function-specific parts (f_uac1/f_uac2) became almost
>>> as storage for class-specific USB descriptors, some
>>> boilerplate for configfs, binding and few USB
>>> config request handling.
>>>
>>> Originally in RFC [1] I've posted before, there was
>>> major change to f_uac1 after that it couldn't do
>>> direct play to existing ALSA sound card anymore,
>>> representing audio on gadget side as virtual
>>> ALSA sound card where audio streams are simply
>>> sinked to and sourced from it, so it may break
>>> current usecase for some people (and that's why
>>> it was RFC).
>>>
>>> During RFC discussion, it was agreed to not touch
>>> existing f_uac1 implementation and create new one
>>> instead. This patchset (v4) introduced new function
>>> named f_uac1_acard and doesn't touch current f_uac1
>>> implementation, so people still can use old behavior
>>
>> Do you have a pointer to the original RFC discussion where this was
>> discussed? If we really *must* keep the old implementation, I would
>> rather rename that to f_uac1_legacy. Still, I find it unlikely that
>> anybody will care about the old implementation.
>
> It is on LKML (which is down for me) [1] or alternative archive [2]
>
>>
>>> Now, it's possible to use existing user-space
>>> applications for audio routing between Audio Gadget
>>> and real sound card. I personally use alsaloop tool
>>> from alsautils and have ability to create PCM
>>> loopback between two different ALSA cards using
>>> rate resampling, which was not possible with previous
>>> "direct play to ALSA card" approach in f_uac1.
>>
>> this is really good result and will actually make it a lot easier for
>> testing things out.
>>
>>> While here, also dropped redundant platform
>>> driver/device creation in f_uac2 driver (as well as
>>> didn't add "never implemented" volume/mute functionality
>>> in f_uac1 to f_uac1_acard) that made this work even
>>> easier to do.
>>>
>>> This series is tested with both legacy g_audio.ko and
>>> modern configfs approaches under Ubuntu 14.04 (UAC1 and
>>> UAC2) and under Windows7 x64 (UAC1 only) having
>>> perfect results in all cases.
>>>
>>> Comments, testing are welcome.
>>>
>>> v4 changes:
>>>  - renamed f_uac1_newapi to f_uac1_acard that is
>>>more meaningful
>>
>> I really don't get why you wanna keep both f_uac1 and f_uac1_acard. Why
>> do we need to maintain the old uac1 implementation? Why two separate
>> files?
>
> In first RFC ([1],[2]) I did exactly what you wrote here (removed
> old uac1 implementation and replaced it by new one) but got feedback
> that it will break things for existing f_uac1 legacy users and it's better to
> have separate implementation.
>
> I'm OK with dropping legacy f_uac1 implementation.
>
> Another idea I was thinking about is to implement simple in-kernel
> driver which will do the same as existing alsaloop tool userspace
> tool does (so legacy users will need to load two kernel modules
> and get same functionality). But this seems to be a wrong way,
> since It known that Linux kernel community doesn't like to take drivers
> with same functionality as existing userspace tools already have.
>
> So bottom line: since I'm not a legacy f_uac1 user, there is no
> difference for me how to handle it - remove legacy f_uac1 completely,
> rename it to f_uac1_legacy or add separate f_uac1_acard function.
>
> So if dropping of legacy f_uac1 implementation is OK for you,
> I can do it quickly in next patchset.

Personally, I don't want duplicated functionality and I think the
virtual sound card approach is much better. Then again, removing
functionality we already support is kind of odd.

Greg, Alan, what do you guys think? Do we keep a duplicated function
around or do we just tell people to rely on alsaloop? Personally, I
think we're better off with the flexibility of the virtual sound card,
what's your take?

-- 
balbi


signature.asc
Description: PGP signature


Re: [RFC] usb-phy-generic: Add support to SMSC USB3315

2017-06-05 Thread Fabien Lahoudere
On Fri, 2017-06-02 at 15:00 -0700, Stephen Boyd wrote:
> On 05/26, Fabien Lahoudere wrote:
> > Hello
> > 
> > I modify ci_hrdc_imx_probe to bypass "data->phy = 
> > devm_usb_get_phy_by_phandle(>dev,
> > "fsl,usbphy", 0);". Everything works as expected and call ci_ulpi_init.
> > 
> > The problem is that in ci_ulpi_init, before calling "ci->ulpi = 
> > ulpi_register_interface(ci->dev,
> > >ulpi_ops);" (to initialize our phy), "hw_phymode_configure(ci);" is 
> > called which is the
> > original function that make our system to hang.
> > 
> > Our phy is not initialised before calling ulpi_register_interface so I 
> > don't understand how the
> > phy
> > can reply if it is not out of reset state.
> 
> I haven't see any problem in hw_phymode_configure(). What's the
> value of ci->platdata->phy_mode? USBPHY_INTERFACE_MODE_ULPI? If
> you phy needs to be taken out of reset to reply to the ulpi reads
> of the vendor/product ids, then it sounds like you have a similar
> situation to what I had. I needed to turn on some regulators to
> get those reads to work, otherwise they would fail, but knowing
> what needed to be turned on basically meant I needed to probe the
> ulpi driver so probing the ids wasn't going to be useful. So on
> my device the reads for the ids go through, but they get all
> zeroes back, which is actually ok because there aren't any bits
> set on my devices anyway. After the reads see 0, we fallback to
> DT matching, which avoids the "bring it out of reset/power it on"
> sorts of problems entirely.
> 

Yes the phy mode is configured to USBPHY_INTERFACE_MODE_ULPI.
Indeed, this phy need to be out of reset to work. For example everything works 
fine if I call 
"_ci_usb_phy_init(ci);" before calling "hw_phymode_configure(ci);"
This function only init reset GPIO and clock.

For information, the original patch I have to fix the issue:

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 79ad8e9..21aaff1 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -391,6 +391,7 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
    case USBPHY_INTERFACE_MODE_UTMI:
    case USBPHY_INTERFACE_MODE_UTMIW:
    case USBPHY_INTERFACE_MODE_HSIC:
+   case USBPHY_INTERFACE_MODE_ULPI:
    ret = _ci_usb_phy_init(ci);
    if (!ret)
    hw_wait_phy_stable();
@@ -398,7 +399,6 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
    return ret;
    hw_phymode_configure(ci);
    break;
-   case USBPHY_INTERFACE_MODE_ULPI:
    case USBPHY_INTERFACE_MODE_SERIAL:
    hw_phymode_configure(ci);
    ret = _ci_usb_phy_init(ci);
-- 

So if some ULPI phys need to be initialised before calling 
"hw_phymode_configure", is it acceptable
if I separate "case USBPHY_INTERFACE_MODE_ULPI:" and add a DT binding 
("init_phy_first") to define
the order to call both functions?

Something like:

case USBPHY_INTERFACE_MODE_ULPI:
if (ci->platdata->init_phy_first) {
ret = _ci_usb_phy_init(ci);
if (!ret)
    hw_wait_phy_stable();
    else
    return ret;
}
hw_phymode_configure(ci);
if (!ci->platdata->init_phy_first) {
ret = _ci_usb_phy_init(ci);
if (ret)
    return ret;
}
break;

This approach will not modify current behaviour but allow to initialize phy 
first on demand.

--
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] xhci: remove endpoint ring cache

2017-06-05 Thread Anurag Kumar Vulisha
Hi Mathias,

I have tested this patch with my platform and it works fine

Thanks,
Anurag Kumar Vulisha

>-Original Message-
>From: Mathias Nyman [mailto:mathias.ny...@linux.intel.com]
>Sent: Friday, June 02, 2017 4:51 PM
>To: Anurag Kumar Vulisha 
>Cc: gre...@linuxfoundation.org; Anirudha Sarangi ;
>Anurag Kumar Vulisha ; linux-usb@vger.kernel.org;
>Mathias Nyman 
>Subject: [PATCH] xhci: remove endpoint ring cache
>
>Having a properly working ring cache could ease a bit the memory
>reallocation, but this current implemetation isn't the correct way.
>It's faulty and hogs a lot of memory.
>
>A pool of cached rings that any device could use would be more useful,
>but xhci driver isn't there yet, just keeping the basic functionality
>working is already a handful.
>
>How about your case, does removing the cache work instead with your setup?
>
>8<---
>
>Anurag Kumar Vulisha reported several issues with xhci endpoint
>ring caching.
>
>31 Rings are cached per device before a ring is freed.
>These cached rings are not used as default if a new ring is needed.
>They are only used if the drive fails to allocate memory for a ring.
>
>The current ring cache is more a reason to why we run out memry than a
>help when we actually do so.
>
>Anurag Kumar Vulisha tried to use cached rings as a first option and
>found new issues with cached ring initialization.
>Cached rings were first zeroed and then manually reinitialized with link
>rbs etc, but forgetting to set some important bits like cycle toggle bit.
>
>Remove the ring cache completely as it's a faulty premature optimization
>eating memory
>
>Reported-by: Anurag Kumar Vulisha 
>Signed-off-by: Mathias Nyman 
>---
> drivers/usb/host/xhci-mem.c | 81 -
> drivers/usb/host/xhci.c | 17 +-
> drivers/usb/host/xhci.h |  6 +---
> 3 files changed, 15 insertions(+), 89 deletions(-)
>
>diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>index 1f1687e..7c2501a 100644
>--- a/drivers/usb/host/xhci-mem.c
>+++ b/drivers/usb/host/xhci-mem.c
>@@ -407,64 +407,17 @@ static struct xhci_ring *xhci_ring_alloc(struct xhci_hcd
>*xhci,
>   return NULL;
> }
>
>-void xhci_free_or_cache_endpoint_ring(struct xhci_hcd *xhci,
>+void xhci_free_endpoint_ring(struct xhci_hcd *xhci,
>   struct xhci_virt_device *virt_dev,
>   unsigned int ep_index)
> {
>-  int rings_cached;
>-
>-  rings_cached = virt_dev->num_rings_cached;
>-  if (rings_cached < XHCI_MAX_RINGS_CACHED) {
>-  virt_dev->ring_cache[rings_cached] =
>-  virt_dev->eps[ep_index].ring;
>-  virt_dev->num_rings_cached++;
>-  xhci_dbg(xhci, "Cached old ring, "
>-  "%d ring%s cached\n",
>-  virt_dev->num_rings_cached,
>-  (virt_dev->num_rings_cached > 1) ? "s" : "");
>-  } else {
>-  xhci_ring_free(xhci, virt_dev->eps[ep_index].ring);
>-  xhci_dbg(xhci, "Ring cache full (%d rings), "
>-  "freeing ring\n",
>-  virt_dev->num_rings_cached);
>-  }
>+  xhci_ring_free(xhci, virt_dev->eps[ep_index].ring);
>   virt_dev->eps[ep_index].ring = NULL;
> }
>
>-/* Zero an endpoint ring (except for link TRBs) and move the enqueue and
>dequeue
>- * pointers to the beginning of the ring.
>- */
>-static void xhci_reinit_cached_ring(struct xhci_hcd *xhci,
>-  struct xhci_ring *ring, unsigned int cycle_state,
>-  enum xhci_ring_type type)
>-{
>-  struct xhci_segment *seg = ring->first_seg;
>-  int i;
>-
>-  do {
>-  memset(seg->trbs, 0,
>-  sizeof(union xhci_trb)*TRBS_PER_SEGMENT);
>-  if (cycle_state == 0) {
>-  for (i = 0; i < TRBS_PER_SEGMENT; i++)
>-  seg->trbs[i].link.control |=
>-  cpu_to_le32(TRB_CYCLE);
>-  }
>-  /* All endpoint rings have link TRBs */
>-  xhci_link_segments(xhci, seg, seg->next, type);
>-  seg = seg->next;
>-  } while (seg != ring->first_seg);
>-  ring->type = type;
>-  xhci_initialize_ring_info(ring, cycle_state);
>-  /* td list should be empty since all URBs have been cancelled,
>-   * but just in case...
>-   */
>-  INIT_LIST_HEAD(>td_list);
>-}
>-
> /*
>  * Expand an existing ring.
>- * Look for a cached ring or allocate a new ring which has same segment
>numbers
>- * and link the two rings.
>+ * Allocate a new ring which has same segment numbers and link the two rings.
>  */
> int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
>   

Re: XHCI is slow during boot (bios/efi) and leaves many dmesg messages

2017-06-05 Thread Maël Lavault
On Tue, 2017-05-16 at 13:25 +0300, Mathias Nyman wrote:
> On 16.05.2017 13:00, Maël Lavault wrote:
> > On Wed, 2017-05-10 at 16:04 +0300, Mathias Nyman wrote:
> > > On 09.05.2017 16:50, Maël Lavault wrote:
> > > > On Tue, 2017-05-09 at 11:34 +0300, Felipe Balbi wrote:
> > > > > Hi,
> > > > > 
> > > > > Maël Lavault  writes:
> > > > > > Le 28 avr. 2017 15:57, "Maël Lavault"  > > > > > alav
> > > > > > enue
> > > > > > .com
> > > > > > > a écrit :
> > > > > > 
> > > > > > On Fri, 2017-04-28 at 16:58 +0300, Mathias Nyman wrote:
> > > > > > > On 21.04.2017 11:08, Maël Lavault wrote:
> > > > > > > > On Tue, 2017-04-18 at 16:58 +0200, Maël Lavault wrote:
> > > > > > > > > Hi,
> > > > > > > > > 
> > > > > > > > > I can't find how to reply to an old thread with
> > > > > > > > > majordomo,
> > > > > > > > > sorry
> > > > > > > > > for
> > > > > > > > > the inconvenience it might cause.
> > > > > > > > > 
> > > > > > > > > I'm reposting an issue [0] that has been inactive for
> > > > > > > > > a
> > > > > > > > > few
> > > > > > > > > month
> > > > > > > > > but
> > > > > > > > > still present in kernel 4.10.10 on a Macbook pro
> > > > > > > > > 12,1.
> > > > > > > > > 
> > > > > > > > > I can provide more informations if needed but the
> > > > > > > > > issue
> > > > > > > > > is
> > > > > > > > > explained
> > > > > > > > > in
> > > > > > > > > details in the bugzilla issue.
> > > > > > > > > 
> > > > > > > > > Thanks.
> > > > > > > > > 
> > > > > > > > > ---
> > > > > > > > > Hi list,
> > > > > > > > > 
> > > > > > > > > I have a Apple Inc. MacBookPro11,1 (with the most
> > > > > > > > > recent
> > > > > > > > > 'bios':
> > > > > > > > > BIOS
> > > > > > > > > MBP111.88Z.0138.B16.1509081438 09/08/2015).
> > > > > > > > > 
> > > > > > > > > At the beginning, USB worked normally. After a while
> > > > > > > > > (and
> > > > > > > > > after
> > > > > > > > > newer
> > > > > > > > > kernel versions released by debian?) things started
> > > > > > > > > to
> > > > > > > > > act
> > > > > > > > > strangely.
> > > > > > > > > For
> > > > > > > > > one, the bios/efi boot takes a very long time
> > > > > > > > > (probably
> > > > > > > > > due
> > > > > > > > > to
> > > > > > > > > the
> > > > > > > > > same
> > > > > > > > > reason I describe later) just to get to the
> > > > > > > > > bootloader/grub.
> > > > > > > > > Likley
> > > > > > > > > resetting and probing for USB ports/mass storage.
> > > > > > > > > When
> > > > > > > > > grub
> > > > > > > > > finally
> > > > > > > > > pops
> > > > > > > > > up, I can use the (internal USB based keyboard)
> > > > > > > > > normally
> > > > > > > > > to
> > > > > > > > > select a
> > > > > > > > > grub
> > > > > > > > > entry etc.
> > > > > > > > > 
> > > > > > > > > Booting the kernel then works reasonably fine, until
> > > > > > > > > it
> > > > > > > > > loads
> > > > > > > > > the
> > > > > > > > > xhci
> > > > > > > > > module.
> > > > > > > > > It spews some messages in dmesg (taking some 15
> > > > > > > > > seconds)
> > > > > > > > > and
> > > > > > > > > only
> > > > > > > > > then,
> > > > > > > > > the
> > > > > > > > > keyboard starts to work again.
> > > > > > > > > 
> > > > > > > > > The log is filled with messages like:
> > > > > > > > > [7.248479] xhci_hcd :00:14.0: Command
> > > > > > > > > completion
> > > > > > > > > event
> > > > > > > > > does
> > > > > > > > > not
> > > > > > > > > match command
> > > > > > > > > [7.248495] xhci_hcd :00:14.0: Timeout while
> > > > > > > > > waiting
> > > > > > > > > for
> > > > > > > > > setup
> > > > > > > > > device command
> > > > > > > > > [   12.256347] xhci_hcd :00:14.0: Timeout while
> > > > > > > > > waiting
> > > > > > > > > for
> > > > > > > > > setup
> > > > > > > > > device command
> > > > > > > > > [   12.256363] usb 1-2: hub failed to enable device,
> > > > > > > > > error
> > > > > > > > > -62
> > > > > > > > > [   17.264166] xhci_hcd :00:14.0: Timeout while
> > > > > > > > > waiting
> > > > > > > > > for
> > > > > > > > > setup
> > > > > > > > > device command
> > > > > > > > > (followed by USB hub/device enumeration)
> > > > > > > > > 
> > > > > > > > > I've tried several combinations and quirks, updating
> > > > > > > > > to
> > > > > > > > > the
> > > > > > > > > latest rc
> > > > > > > > > kernels since 3.16 (am on 4.5.0 right now) and it
> > > > > > > > > only
> > > > > > > > > seems
> > > > > > > > > to
> > > > > > > > > get
> > > > > > > > > worse.
> > > > > > > > > 
> > > > > > > > > Last year, on the 3.x series of kernels occasionally
> > > > > > > > > after a
> > > > > > > > > reboot
> > > > > > > > > the
> > > > > > > > > 'bios' would go through quickly and fine and also no
> > > > > > > > > problems
> > > > > > > > > loading
> > > > > > > > > the
> > > > > > > > > module and logging in. But now it always fails.
> > > > > > > > > 
> > > > > > > > > Additionally it (may or may not) seems to cause the
> 

Re: Gadget driver & virtual hub

2017-06-05 Thread Felipe Balbi

Hi,

Benjamin Herrenschmidt  writes:
>> Benjamin Herrenschmidt  writes:
>> > So another question :-)
>> > 
>> > Is it legal to break down a bulk (or ISO) IN request into packets that
>> > are *smaller* than the Max EP size ?
>> 
>> it depends if they are *supposed* to be two smaller packets or should be
>> two parts of a single one.
>> 
>> > For example, I have a max EP size of 512 bytes and the gadget gives me
>> > an SG list where that's broken up into two segments (for example 2x256
>> > bytes).
>> > 
>> > Now, I'm still trying to confirm with the vendor what happens here, but
>> > I *suspect* that if I create 2 descriptors, the HW will break it down
>> > into 2 packets smaller than the max EP size on the wire.
>> 
>> If that's the case, then your HW can't handle SG lists. I suspect there
>> will be some "chain" bit somewhere in the DMA descriptor which you can
>> use to tell DMA that these two segments are part of a single transfer.
>
> No, there isn't, sadly. I've confirmed with the HW vendor. I might
> still enable SG with a fallback to bounce buffering to a pre-allocated
> packet buffer if I hit such a situation... It shouldn't be common for
> mass storage (though do we use sglists for the mass storage gadget ?),

UASP does use it ;-)

> it might be more with networking if our network function does
> fragmented sends, I haven't checked.

Well, even networking gadget is using a linear buffer. Nobody
implemented support for sg lists yet.

>> > I do have vague memories of seeing "client" drivers in the past with
>> > make assumptions that some "replies" are fully contained in a single
>> > packet, so I'm tempted to say that this won't work, and thus should
>> > bounce such packets through a linear buffer but I'd like confirmation.
>> 
>> yeah, USB makes that assumption. If host side requests 512 bytes and you
>> end up breaking that into two packets, host will receive only the first
>> 256 bytes, as short packets terminate transfers.
>
> Yup, that was indeed my assumption. Thanks for confirming.

np

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH] usb: Make use of ktime_* comparison functions

2017-06-05 Thread Peter Chen
On Fri, May 26, 2017 at 12:15:59PM +0200, Mariusz Skamra wrote:
> Start using ktime_* compare functions to make the code backportable.
> Now that may be a bit tricky due to recent change of ktime_t.
> 
> Signed-off-by: Mariusz Skamra 
> Acked-by: Kuppuswamy Sathyanarayanan 
> ---
>  drivers/usb/chipidea/otg_fsm.c | 8 
>  drivers/usb/host/ehci-timer.c  | 2 +-
>  drivers/usb/host/fotg210-hcd.c | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> index 93e24ce..949183e 100644
> --- a/drivers/usb/chipidea/otg_fsm.c
> +++ b/drivers/usb/chipidea/otg_fsm.c
> @@ -234,7 +234,7 @@ static void ci_otg_add_timer(struct ci_hdrc *ci, enum 
> otg_fsm_timer t)
>   ktime_set(timer_sec, timer_nsec));
>   ci->enabled_otg_timer_bits |= (1 << t);
>   if ((ci->next_otg_timer == NUM_OTG_FSM_TIMERS) ||
> - (ci->hr_timeouts[ci->next_otg_timer] >
> + ktime_after(ci->hr_timeouts[ci->next_otg_timer],
>   ci->hr_timeouts[t])) {
>   ci->next_otg_timer = t;
>   hrtimer_start_range_ns(>otg_fsm_hrtimer,
> @@ -269,7 +269,7 @@ static void ci_otg_del_timer(struct ci_hdrc *ci, enum 
> otg_fsm_timer t)
>   for_each_set_bit(cur_timer, _timer_bits,
>   NUM_OTG_FSM_TIMERS) {
>   if ((next_timer == NUM_OTG_FSM_TIMERS) ||
> - (ci->hr_timeouts[next_timer] <
> + 
> ktime_before(ci->hr_timeouts[next_timer],
>ci->hr_timeouts[cur_timer]))
>   next_timer = cur_timer;
>   }
> @@ -397,13 +397,13 @@ static enum hrtimer_restart ci_otg_hrtimer_func(struct 
> hrtimer *t)
>  
>   now = ktime_get();
>   for_each_set_bit(cur_timer, _timer_bits, NUM_OTG_FSM_TIMERS) {
> - if (now >= ci->hr_timeouts[cur_timer]) {
> + if (ktime_compare(now, ci->hr_timeouts[cur_timer]) >= 0) {
>   ci->enabled_otg_timer_bits &= ~(1 << cur_timer);
>   if (otg_timer_handlers[cur_timer])
>   ret = otg_timer_handlers[cur_timer](ci);
>   } else {
>   if ((next_timer == NUM_OTG_FSM_TIMERS) ||
> - (ci->hr_timeouts[cur_timer] <
> + ktime_before(ci->hr_timeouts[cur_timer],
>   ci->hr_timeouts[next_timer]))
>   next_timer = cur_timer;
>   }

For chipidea part:

Acked-by: Peter Chen 

-- 

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: Gadget driver & virtual hub

2017-06-05 Thread Benjamin Herrenschmidt
On Mon, 2017-06-05 at 09:02 +0300, Felipe Balbi wrote:
> Hi,
> 
> Benjamin Herrenschmidt  writes:
> > So another question :-)
> > 
> > Is it legal to break down a bulk (or ISO) IN request into packets that
> > are *smaller* than the Max EP size ?
> 
> it depends if they are *supposed* to be two smaller packets or should be
> two parts of a single one.
> 
> > For example, I have a max EP size of 512 bytes and the gadget gives me
> > an SG list where that's broken up into two segments (for example 2x256
> > bytes).
> > 
> > Now, I'm still trying to confirm with the vendor what happens here, but
> > I *suspect* that if I create 2 descriptors, the HW will break it down
> > into 2 packets smaller than the max EP size on the wire.
> 
> If that's the case, then your HW can't handle SG lists. I suspect there
> will be some "chain" bit somewhere in the DMA descriptor which you can
> use to tell DMA that these two segments are part of a single transfer.

No, there isn't, sadly. I've confirmed with the HW vendor. I might
still enable SG with a fallback to bounce buffering to a pre-allocated
packet buffer if I hit such a situation... It shouldn't be common for
mass storage (though do we use sglists for the mass storage gadget ?),
it might be more with networking if our network function does
fragmented sends, I haven't checked.

> > I do have vague memories of seeing "client" drivers in the past with
> > make assumptions that some "replies" are fully contained in a single
> > packet, so I'm tempted to say that this won't work, and thus should
> > bounce such packets through a linear buffer but I'd like confirmation.
> 
> yeah, USB makes that assumption. If host side requests 512 bytes and you
> end up breaking that into two packets, host will receive only the first
> 256 bytes, as short packets terminate transfers.

Yup, that was indeed my assumption. Thanks for confirming.

Cheers,
Ben.

--
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 v14 4/7] usb: core: add power sequence handling for USB devices

2017-06-05 Thread Peter Chen
On Thu, May 18, 2017 at 08:49:00AM +0800, Peter Chen wrote:
> Some hard-wired USB devices need to do power sequence to let the
> device work normally, the typical power sequence like: enable USB
> PHY clock, toggle reset pin, etc. But current Linux USB driver
> lacks of such code to do it, it may cause some hard-wired USB devices
> works abnormal or can't be recognized by controller at all.
> 
> In this patch, it calls power sequence library APIs to finish
> the power sequence events. It will do power on sequence at hub's
> probe for all devices under this hub (includes root hub).
> At hub_disconnect, it will do power off sequence which is at powered
> on list.
> 

Greg, Alan, would you please help on reviewing it? It seems Rafael
is waiting for USB MAINTAINERS's comments or ack. It resolves some
USB HUB issues for several persons.

Peter

> Signed-off-by: Peter Chen 
> Tested-by Joshua Clayton 
> Tested-by: Maciej S. Szmigiero 
> Reviewed-by: Vaibhav Hiremath 
> ---
>  drivers/usb/Kconfig|  1 +
>  drivers/usb/core/hub.c | 49 +
>  drivers/usb/core/hub.h |  1 +
>  3 files changed, 47 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 939a63b..b6f626e 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -39,6 +39,7 @@ config USB
>   tristate "Support for Host-side USB"
>   depends on USB_ARCH_HAS_HCD
>   select USB_COMMON
> + select POWER_SEQUENCE
>   select NLS  # for UTF-8 strings
>   ---help---
> Universal Serial Bus (USB) is a specification for a serial bus
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 9dca59e..7a67296 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -1619,6 +1620,7 @@ static void hub_disconnect(struct usb_interface *intf)
>   hub->error = 0;
>   hub_quiesce(hub, HUB_DISCONNECT);
>  
> + of_pwrseq_off_list(>pwrseq_on_list);
>   mutex_lock(_port_peer_mutex);
>  
>   /* Avoid races with recursively_mark_NOTATTACHED() */
> @@ -1646,12 +1648,42 @@ static void hub_disconnect(struct usb_interface *intf)
>   kref_put(>kref, hub_release);
>  }
>  
> +#ifdef CONFIG_OF
> +static int hub_of_pwrseq_on(struct usb_hub *hub)
> +{
> + struct device *parent;
> + struct usb_device *hdev = hub->hdev;
> + struct device_node *np;
> + int ret;
> +
> + if (hdev->parent)
> + parent = >dev;
> + else
> + parent = bus_to_hcd(hdev->bus)->self.sysdev;
> +
> + for_each_child_of_node(parent->of_node, np) {
> + ret = of_pwrseq_on_list(np, >pwrseq_on_list);
> + /* Maybe no power sequence library is chosen */
> + if (ret && ret != -ENOENT)
> + return ret;
> + }
> +
> + return 0;
> +}
> +#else
> +static int hub_of_pwrseq_on(struct usb_hub *hub)
> +{
> + return 0;
> +}
> +#endif
> +
>  static int hub_probe(struct usb_interface *intf, const struct usb_device_id 
> *id)
>  {
>   struct usb_host_interface *desc;
>   struct usb_endpoint_descriptor *endpoint;
>   struct usb_device *hdev;
>   struct usb_hub *hub;
> + int ret = -ENODEV;
>  
>   desc = intf->cur_altsetting;
>   hdev = interface_to_usbdev(intf);
> @@ -1756,6 +1788,7 @@ static int hub_probe(struct usb_interface *intf, const 
> struct usb_device_id *id)
>   INIT_DELAYED_WORK(>leds, led_work);
>   INIT_DELAYED_WORK(>init_work, NULL);
>   INIT_WORK(>events, hub_event);
> + INIT_LIST_HEAD(>pwrseq_on_list);
>   usb_get_intf(intf);
>   usb_get_dev(hdev);
>  
> @@ -1769,11 +1802,14 @@ static int hub_probe(struct usb_interface *intf, 
> const struct usb_device_id *id)
>   if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
>   hub->quirk_check_port_auto_suspend = 1;
>  
> - if (hub_configure(hub, endpoint) >= 0)
> - return 0;
> + if (hub_configure(hub, endpoint) >= 0) {
> + ret = hub_of_pwrseq_on(hub);
> + if (!ret)
> + return 0;
> + }
>  
>   hub_disconnect(intf);
> - return -ENODEV;
> + return ret;
>  }
>  
>  static int
> @@ -3593,14 +3629,19 @@ static int hub_suspend(struct usb_interface *intf, 
> pm_message_t msg)
>  
>   /* stop hub_wq and related activity */
>   hub_quiesce(hub, HUB_SUSPEND);
> - return 0;
> + return pwrseq_suspend_list(>pwrseq_on_list);
>  }
>  
>  static int hub_resume(struct usb_interface *intf)
>  {
>   struct usb_hub *hub = usb_get_intfdata(intf);
> + int ret;
>  
>   dev_dbg(>dev, "%s\n", __func__);
> + ret = pwrseq_resume_list(>pwrseq_on_list);
> + if (ret)
> + return ret;
> +
>   hub_activate(hub, HUB_RESUME);

Re: [PATCH v14 2/7] power: add power sequence library

2017-06-05 Thread Peter Chen
On Thu, May 18, 2017 at 08:48:58AM +0800, Peter Chen wrote:
> We have an well-known problem that the device needs to do some power
> sequence before it can be recognized by related host, the typical
> example like hard-wired mmc devices and usb devices.
> 
> This power sequence is hard to be described at device tree and handled by
> related host driver, so we have created a common power sequence
> library to cover this requirement. The core code has supplied
> some common helpers for host driver, and individual power sequence
> libraries handle kinds of power sequence for devices. The pwrseq
> librares always need to allocate extra instance for compatible
> string match.
> 
> pwrseq_generic is intended for general purpose of power sequence, which
> handles gpios and clocks currently, and can cover other controls in
> future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> if only one power sequence is needed, else call of_pwrseq_on_list
> /of_pwrseq_off_list instead (eg, USB hub driver).
> 
> For new power sequence library, it can add its compatible string
> to pwrseq_of_match_table, then the pwrseq core will match it with
> DT's, and choose this library at runtime.

Ping

> 
> Signed-off-by: Peter Chen 
> Tested-by: Maciej S. Szmigiero 
> Tested-by Joshua Clayton 
> Reviewed-by: Matthias Kaehlcke 
> Tested-by: Matthias Kaehlcke 
> ---
>  Documentation/power/power-sequence/design.rst |  54 +
>  MAINTAINERS   |   9 +
>  drivers/power/Kconfig |   1 +
>  drivers/power/Makefile|   1 +
>  drivers/power/pwrseq/Kconfig  |  20 ++
>  drivers/power/pwrseq/Makefile |   2 +
>  drivers/power/pwrseq/core.c   | 335 
> ++
>  drivers/power/pwrseq/pwrseq_generic.c | 234 ++
>  include/linux/power/pwrseq.h  |  81 +++
>  9 files changed, 737 insertions(+)
>  create mode 100644 Documentation/power/power-sequence/design.rst
>  create mode 100644 drivers/power/pwrseq/Kconfig
>  create mode 100644 drivers/power/pwrseq/Makefile
>  create mode 100644 drivers/power/pwrseq/core.c
>  create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
>  create mode 100644 include/linux/power/pwrseq.h
> 
> diff --git a/Documentation/power/power-sequence/design.rst 
> b/Documentation/power/power-sequence/design.rst
> new file mode 100644
> index 000..554608e
> --- /dev/null
> +++ b/Documentation/power/power-sequence/design.rst
> @@ -0,0 +1,54 @@
> +
> +Power Sequence Library
> +
> +
> +:Date: Feb, 2017
> +:Author: Peter Chen 
> +
> +
> +Introduction
> +
> +
> +We have an well-known problem that the device needs to do a power
> +sequence before it can be recognized by related host, the typical
> +examples are hard-wired mmc devices and usb devices. The host controller
> +can't know what kinds of this device is in its bus if the power
> +sequence has not done, since the related devices driver's probe calling
> +is determined by runtime according to eunumeration results. Besides,
> +the devices may have custom power sequence, so the power sequence library
> +which is independent with the devices is needed.
> +
> +Design
> +
> +
> +The power sequence library includes the core file and customer power
> +sequence library. The core file exports interfaces are called by
> +host controller driver for power sequence and customer power sequence
> +library files to register its power sequence instance to global
> +power sequence list. The custom power sequence library creates power
> +sequence instance and implement custom power sequence.
> +
> +Since the power sequence describes hardware design, the description is
> +located at board description file, eg, device tree dts file. And
> +a specific power sequence belongs to device, so its description
> +is under the device node, please refer to:
> +Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> +
> +Custom power sequence library allocates one power sequence instance at
> +bootup periods using postcore_initcall, this static allocated instance is
> +used to compare with device-tree (DT) node to see if this library can be
> +used for the node or not. When the result is matched, the core API will
> +try to get resourses (->get, implemented at each library) for power
> +sequence, if all resources are got, it will try to allocate another
> +instance for next possible request from host driver.
> +
> +Then, the host controller driver can carry out power sequence on for this
> +DT node, the library will do corresponding operations, like open clocks,
> +toggle gpio, etc. The power sequence off routine will close and free the
> +resources, and is called when 

Re: Device tree nodes for USB devices

2017-06-05 Thread Peter Chen
On Mon, May 29, 2017 at 04:15:36PM +0200, Johan Hovold wrote:
> On Mon, May 29, 2017 at 11:01:52AM +0200, Martin Fuzzey wrote:
> > Hi Peter,
> > 
> > I'm trying to create device tree nodes for USB devices (use case a USB 
> > device providing GPIO and I2C controllers)
> > 
> > Your patch (now merged)
> >  69bec725  "USB: core: let USB device know device node"
> > 
> > looks just like what I need but I have a couple of problems.
> > 
> > First it doesn't work at all on an i.MX53 because the actual struct 
> > device for the USB host controller is a "ci_hdrc" which is a child of 
> > the device having the DT node (ci_hdrc_imx)
> 
> I believe you should set the new sysdev pointer to the parent controller
> device, which has the device tree node. Take a look at a8c06e407ef9
> ("usb: separate out sysdev pointer from usb_bus").
> 
> > However my larger question is that I don't see how to associate a DT 
> > node with a USB *interface* rather than a USB *device*.
> 
> I started looking into this a while back but got interrupted. I have
> some preliminary code, mostly lacking associated documentation.
> 

Johan has already supplied the correct answer and related code, if you
would like see how USB device work based on DT, please check my power
sequence patch set:

http://www.spinics.net/lists/linux-usb/msg157134.html

-- 

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 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt

2017-06-05 Thread Felipe Balbi

Hi,

Paul Zimmerman  writes:
> Forgot to CC linux-usb, doing that now
>
> On Fri, 2 Jun 2017 16:27:56 -0700, Paul Zimmerman  wrote:
>
>> Felipe Balbi  writes:
>> > Thinh Nguyen  writes:
>> >> this could be, I don't remember if I checked this or not :-)
>> >>
>> >> Really, the best way here, IMHO, would be to re-verify what's going on
>> >> with macOS and revert my orignal patch since it's, rather clearly,
>> >> wrong.
>> >>
>> >
>> > Sure. Are you going to make a revert patch or I am?
>> 
>>  Well, after we really know what's going on with macOS and have a better
>>  fix, then who makes the revert is less important as long as problems get
>>  sorted out :-) Either way is fine for me.
>> 
>> >>> 
>> >>> Do you have any update on this issue?
>> >>> 
>> >>
>> >> The patch ffb80fc672c3 ("usb: dwc3: gadget: skip Set/Clear Halt when 
>> >> invalid") still causes a regression for us. As there hasn't any update 
>> >> for the macOS issue, can I submit a revert patch for this?
>> >
>> > I just came back from vacations ;-) I'll get back to this. Reverting
>> > that commit won't do any good as we'd be exchanging one regression for
>> > another. We really need to understand what's going on.
>> 
>> Hi Felipe,
>> 
>> I think we worked around this same issue in the Synopsys vendor driver
>> after a customer reported a problem with CLEAR_FEATURE(ENDPOINT_HALT).
>> I no longer have access to either the databook or the codebase, so I
>> can't be sure about what the workaround was, but if either John or Thinh
>> can have a look at the Clear Stall code in the vendor driver they should
>> be able to figure it out.

Thanks a lot Paul :-) Good to see you still have a look here every once
in a while :-)

John, Thinh could either of you check what Paul mentions here?

cheers

-- 
balbi


signature.asc
Description: PGP signature


Re: [GIT PULL] Fixes for v4.12-rc4

2017-06-05 Thread Felipe Balbi

Hi,

Greg KH  writes:
> On Fri, Jun 02, 2017 at 12:57:57PM +0300, Felipe Balbi wrote:
>> 
>> Hi Greg,
>> 
>> here's another set of fixes for current -rc cycle. Let me know if you
>> want anything to be changed. Patches have been on the mailing list for
>> quite some time.
>> 
>> At least the mass storage patch (the only one I could test) has been
>> tested on some intel boards I have around for a few days.
>> 
>> cheers
>> 
>> The following changes since commit 5ed02dbb497422bf225783f46e6eadd237d23d6b:
>> 
>>   Linux 4.12-rc3 (2017-05-28 17:20:53 -0700)
>> 
>> are available in the git repository at:
>> 
>>   git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
>> fixes-for-v4.12-rc4
>
> Pulled and pushed out, you will not get the automated emails as my tree
> was based on 4.12-rc2, and I don't want to spam everyone who did work
> since them with those emails :)

Thanks :-)

-- 
balbi


signature.asc
Description: PGP signature


Re: Gadget driver & virtual hub

2017-06-05 Thread Felipe Balbi

Hi,

Benjamin Herrenschmidt  writes:
> So another question :-)
>
> Is it legal to break down a bulk (or ISO) IN request into packets that
> are *smaller* than the Max EP size ?

it depends if they are *supposed* to be two smaller packets or should be
two parts of a single one.

> For example, I have a max EP size of 512 bytes and the gadget gives me
> an SG list where that's broken up into two segments (for example 2x256
> bytes).
>
> Now, I'm still trying to confirm with the vendor what happens here, but
> I *suspect* that if I create 2 descriptors, the HW will break it down
> into 2 packets smaller than the max EP size on the wire.

If that's the case, then your HW can't handle SG lists. I suspect there
will be some "chain" bit somewhere in the DMA descriptor which you can
use to tell DMA that these two segments are part of a single transfer.

> I do have vague memories of seeing "client" drivers in the past with
> make assumptions that some "replies" are fully contained in a single
> packet, so I'm tempted to say that this won't work, and thus should
> bounce such packets through a linear buffer but I'd like confirmation.

yeah, USB makes that assumption. If host side requests 512 bytes and you
end up breaking that into two packets, host will receive only the first
256 bytes, as short packets terminate transfers.

-- 
balbi


signature.asc
Description: PGP signature