Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx

2014-09-26 Thread Arnd Bergmann
On Friday 26 September 2014 08:23:40 Peter Chen wrote:
 In current chipidea structure, the parent (glue layer) driver will not be
 used for dma, udc/host driver uses dma mask from child (core layer), at core
 layer we will do:
 
 
 pdev-dev.dma_mask = dev-dma_mask; /* this device is parent */
 dma_set_coherent_mask(pdev-dev, dev-coherent_dma_mask);
 
 Would you suggestion us a suitable way? Or it is ok we use just Antoine's way 
 that
 call dma_coerce_mask_and_coherent(pdev-dev, DMA_BIT_MASK(32)) at parent
 driver no matter dt or non-dt? Thanks.

dma_coerce_mask_and_coherent is not ok, it will force a dma mask that is
unrelated to the actual requirements of the hardware.

I think the best way would be to never use the child device pointer for
DMA operations, just use a pointer to the parent device and make the
child dev-dma_mask pointer NULL to ensure all DMA operations fail.

Arnd
--
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 07/12] usb: chipidea: add a usb2 driver for ci13xxx

2014-09-26 Thread Arnd Bergmann
On Thursday 25 September 2014 19:39:34 Felipe Balbi wrote:
  
  why would a glue layer need to access registers from the core ? That
  sounds very odd. I haven't seen that and will, definitely, NACK such a
  patch 
  
  can you further describe why you think a glue layer might need to access
  core IP's registers ?
 
 I just realised we're talking about chipidea here... in any case, it's
 still valid to ask why would glue need to fiddle with core IP's
 registers.

Generally, the glue driver wouldn't access the registers, but I don't
think it's important to prevent it from doing that. In some cases, 
a glue driver needs to override a function of the core driver, e.g.
to work around an errata. We have a lot of those quirks in ATA drivers,
one example from ahci_mvebu.c is

static void ahci_mvebu_regret_option(struct ahci_host_priv *hpriv)
{
/*
 * Enable the regret bit to allow the SATA unit to regret a
 * request that didn't receive an acknowlegde and avoid a
 * deadlock
 */
writel(0x4, hpriv-mmio + AHCI_VENDOR_SPECIFIC_0_ADDR);
writel(0x80, hpriv-mmio + AHCI_VENDOR_SPECIFIC_0_DATA);
}

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


Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL

2014-09-26 Thread Huang Rui
On Thu, Sep 25, 2014 at 09:50:32AM -0500, Felipe Balbi wrote:
 Hi,
 
 On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
  diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
  index b0f4d52..6138c5d 100644
  --- a/drivers/usb/dwc3/core.c
  +++ b/drivers/usb/dwc3/core.c
  @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
   }
   
   /**
  + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
  + * @dwc: Pointer to our controller context structure
  + */
  +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
  +{
  +   u32 reg = 0;
  +
  +   reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK | DWC3_GUSB3PIPECTL_UX_EXITINPX
  +   | DWC3_GUSB3PIPECTL_UX_EXITINPX | DWC3_GUSB3PIPECTL_U1U2EXITFAIL
  +   | DWC3_GUSB3PIPECTL_DEPOCHANGE | DWC3_GUSB3PIPECTL_RX_DETOPOLL;
 
 UX_EXITINPX is supposed to be used with a faulty PHY, I need to see an
 erratum before I can accept it. You have also duplicated the bit in this
 initialization.
 
 U1U2EXITFAIL - also a workaround bit and I need to see an erratum.
 
 RX_DETOPOLL - it seems like it's safe to leave this one out as it can
 only be proven to work with this bit after going through full USB
 certification.
 

What do you mean of the faulty PHY?
We would use AMD PHY to talk with DWC3 controller.

  +   reg |= DWC3_GUSB3PIPECTL_DEP1P2P3(1) | DWC3_GUSB3PIPECTL_TX_DEEPH(1);
 
 DEP1P2P3 and DEP0CHANGE seem to be required only for faulty PHYs too, I
 need to see an erratum.
 
  +   dwc3_writel(dwc-regs, DWC3_GUSB3PIPECTL(0), reg);
  +
  +   mdelay(100);
  +}
  +
  +/**
* dwc3_free_one_event_buffer - Frees one event buffer
* @dwc: Pointer to our controller context structure
* @evt: Pointer to event buffer to be freed
  @@ -412,9 +431,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
  if (ret)
  goto err0;
   
  +   if (dwc-quirks  DWC3_AMD_NL_PLAT)
  +   dwc3_core_nl_set_pipe3(dwc);
  +
  reg = dwc3_readl(dwc-regs, DWC3_GCTL);
  +
  reg = ~DWC3_GCTL_SCALEDOWN_MASK;
  -   reg = ~DWC3_GCTL_DISSCRAMBLE;
  +
  +   if (dwc-quirks  DWC3_AMD_NL_PLAT) {
  +   reg |= DWC3_GCTL_DISSCRAMBLE;
 
 you're disabling scrambling ? What about EMI ? Why doesn't your device
 work with scrambling ? I need to see an erratum before I can accept
 this.
 

Sorry, disabling scrambling is workaround for debugging the temporary
simulation board, needn't be set for the SoC chip. Will update in v2.

  +   reg |= DWC3_GCTL_U2EXIT_LFPS;
 
 hmm, seems like this bit was added due to a faulty host which was found.
 I need to see an erratum before I can accept this.
 
  +   reg |= DWC3_GCTL_GBLHIBERNATIONEN;
 
 looks like this should always be set for all versions of the core
 configured with hibernation.
 

yes, amd nl chip will support hibernation.

  diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
  index cebbd01..7f471ff 100644
  --- a/drivers/usb/dwc3/dwc3-pci.c
  +++ b/drivers/usb/dwc3/dwc3-pci.c
  @@ -25,6 +25,9 @@
   #include linux/usb/otg.h
   #include linux/usb/usb_phy_generic.h
   
  +#include platform_data.h
  +#include core.h
 
 you should never need to include core.h, why are you including
 core.h ???
 

Because I defined DWC3_AMD_NL_PLAT in dwc3 struture at core.h. I think
this quirk might be included on most of the source files like core.c,
gadget.c. If I added into platform_data.h, it would make these source
files include platform_data.h either. So I add DWC3_AMD_NL_PLAT into
core.h.

So should I define DWC3_AMD_NL_PLAT and other QUIRKS at
platform_data.h or create a quirks.h file?

  @@ -102,6 +105,9 @@ static int dwc3_pci_probe(struct pci_dev *pci,
  struct dwc3_pci *glue;
  int ret;
  struct device   *dev = pci-dev;
  +   struct dwc3_platform_data dwc3_pdata;
  +
  +   memset(dwc3_pdata, 0x00, sizeof(dwc3_pdata));
   
  glue = devm_kzalloc(dev, sizeof(*glue), GFP_KERNEL);
  if (!glue)
  @@ -148,6 +154,14 @@ static int dwc3_pci_probe(struct pci_dev *pci,
   
  pci_set_drvdata(pci, glue);
   
  +   if (pci-vendor == PCI_VENDOR_ID_AMD 
  +   pci-device == PCI_DEVICE_ID_AMD_NL)
  +   dwc3_pdata.quirks |= DWC3_AMD_NL_PLAT;
 
 this looks wrong. It looks like you need several quirk bits for each of
 the workarounds you need. Having a single bit for my device is wrong
 and if your next device fixes one or two of these errata, you'd still
 have to introduce a new my new device bit, instead of just clearing
 the ones which were fixed.
 

I got it, so do you mean:
if I set disabling scrambling workaround, I should use a macro as
DWC3_DISSCRAMBING_QUIRK to present this specific setting. Then use the
Deivce ID to enable these kinds of the quirks I need, is that right?

  diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
  index 0fcc0a3..8277065 100644
  --- a/drivers/usb/dwc3/gadget.c
  +++ b/drivers/usb/dwc3/gadget.c
  @@ -2635,6 +2635,7 @@ static irqreturn_t 

Re: [RFC PATCH 4/4] usb: dwc3: introduce dual role switch function with debugfs

2014-09-26 Thread Huang Rui
On Thu, Sep 25, 2014 at 09:53:20AM -0500, Felipe Balbi wrote:
 Hi,
 
 On Thu, Sep 25, 2014 at 03:21:47PM +0800, Huang Rui wrote:
  This patch implemented a feature to dynamic switch to host or device
  role under debugfs for some physical limitation that unable to
  leverage connector A/B cables (ID pin) to change roles.
  
  The default role should be set as OTG mode. Then use below commands:
  
  [1] switch to host:
  echo host  /sys/kernel/debug/dwc3.0.auto/mode
  
  [2] switch to device:
  echo device  /sys/kernel/debug/dwc3.0.auto/mode
  
  [3] switch to otg (default mode):
  echo otg  /sys/kernel/debug/dwc3.0.auto/mode
 
 thanks, but not thanks. This is not how things should be done.
 
  diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
  index 6138c5d..7b50584 100644
  --- a/drivers/usb/dwc3/core.c
  +++ b/drivers/usb/dwc3/core.c
  @@ -61,7 +61,7 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
* dwc3_core_soft_reset - Issues core soft reset and PHY reset
* @dwc: pointer to our context structure
*/
  -static int dwc3_core_soft_reset(struct dwc3 *dwc)
  +int dwc3_core_soft_reset(struct dwc3 *dwc)
 
 this should not be exposed.
 
  @@ -228,7 +228,7 @@ static int dwc3_alloc_event_buffers(struct dwc3 *dwc, 
  unsigned length)
*
* Returns 0 on success otherwise negative errno.
*/
  -static int dwc3_event_buffers_setup(struct dwc3 *dwc)
  +int dwc3_event_buffers_setup(struct dwc3 *dwc)
 
 this should not be exposed.
 
  diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
  index a1c7dc5..3a30f33 100644
  --- a/drivers/usb/dwc3/core.h
  +++ b/drivers/usb/dwc3/core.h
  @@ -643,6 +643,8 @@ struct dwc3_scratchpad_array {
* @maximum_speed: maximum speed requested (mainly for testing purposes)
* @revision: revision register contents
* @quirks: represents different SOCs hardware work-arounds and quirks
  + * @has_gadget: true when gadget is initialized
  + * @has_xhci: true when xhci is initialized
 
 you shouldn't need these.
 
  diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
  index 9ac37fe..76384df 100644
  --- a/drivers/usb/dwc3/debugfs.c
  +++ b/drivers/usb/dwc3/debugfs.c
  @@ -32,6 +32,7 @@
   #include gadget.h
   #include io.h
   #include debug.h
  +#include drd.h
 
 and debugfs is definitely *not* the way to get this going.
 
 What you need here is something to talk to usbcore and udc-core and
 orchestrate the mode change through usb_add_hcd()/usb_add_gadget_udc()
 and their counterparts.
 
 George Cherian is already working on a version of that.
 

I saw your tree have a dwc3-role-switch branch, is that you mentioned
here for the working.

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


Re: [RFC PATCH 1/4] usb: dwc3: add AMD NL support

2014-09-26 Thread Huang Rui
On Fri, Sep 26, 2014 at 12:26:05AM -0500, Felipe Balbi wrote:
 Hi,
 
 On Fri, Sep 26, 2014 at 12:53:21PM +0800, Huang Rui wrote:
  On Thu, Sep 25, 2014 at 09:53:55AM -0500, Felipe Balbi wrote:
   On Thu, Sep 25, 2014 at 03:21:44PM +0800, Huang Rui wrote:
Add PCI device ID of AMD NL.

Signed-off-by: Huang Rui ray.hu...@amd.com
---
 drivers/usb/dwc3/dwc3-pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 436fb08..cebbd01 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -30,6 +30,7 @@
 #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB30xabcd
 #define PCI_DEVICE_ID_INTEL_BYT0x0f37
 #define PCI_DEVICE_ID_INTEL_MRFLD  0x119e
+#define PCI_DEVICE_ID_AMD_NL   0x7912
 
 struct dwc3_pci {
struct device   *dev;
@@ -183,6 +184,7 @@ static const struct pci_device_id 
dwc3_pci_id_table[] = {
},
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT), },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRFLD), },
+   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_NL), },
   
   this patch causes a build breakage.
   
  
  That's strange, I tested it once again in my side based on your next
  branch and didn't find any build error. If I misunderstood your
  meaning, please correct me. :)
 
 no, I just misread your patch. I'm sorry. I thought the PCI Device ID
 was set to the quirk which was not defined here. I apologize, this patch
 is 100% correct.
 

It doesn't matter, thanks a lot to your kindly support. :)

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


[PATCH 0/6] mfd: fix platform-device id collisions

2014-09-26 Thread Johan Hovold
Hot-pluggable multi-function devices should always be registered with
PLATFORM_DEVID_AUTO to avoid name collisions on the platform bus.

This series fix the two mfd drivers that currently fail to get this
right, and also adds a new helper function to assist any future driver
authors.

Included is also a fix of how mfd core generates the platform ids for
subdevices. Currently, the mfd cell-id is simply added to the id base
that mfd_add_devices is called with. This effectively prevents
mfd-devices from using PLATFORM_DEVID_AUTO (-2) while still having
non-zero cell ids.

In a different thread I mentioned that using for example

bus_num  8 | dev_num

as an id-base for USB multi-function devices would also avoid any
collisions, but encoding the bus topology in the id base like this
is not good idea. [1] Not only would it force any new transports to come
up with unique id bases, it would also be very ad-hoc differ from driver
to driver (consider multi-interface USB devices or non-zero cell ids).

Note that if userspace needs to find sibling interfaces it should
never rely on device naming anyway, but rather use the topology already
encoded in sysfs.

The only thing that is currently not possible trough sysfs is to figure
out which sibling interface is which should they have the same name but
unique cell ids (consider an MFD with multiple leds or gpio chips).
Again, parsing device ids is not an option, but if needed we could
simply let driver core export any mfd cell-id for platform devices. Note
that the final patch is a pre-requisite for this.

Johan

[1] http://marc.info/?l=linux-kernelm=141094514827834w=2


Johan Hovold (6):
  mfd: viperboard: fix platform-device id collision
  mfd: rtsx_usb: fix platform device-id collision
  mfd: core: add helper function to register hotplug devices
  mfd: use mfd_add_hotplug_devices helper
  HID: hid-sensor-hub: use mfd_add_hotplug_devices helper
  mfd: core: fix platform-device id generation

 drivers/hid/hid-sensor-hub.c | 8 +++-
 drivers/mfd/mfd-core.c   | 8 +++-
 drivers/mfd/rtsx_usb.c   | 4 ++--
 drivers/mfd/viperboard.c | 4 ++--
 include/linux/mfd/core.h | 7 +++
 5 files changed, 21 insertions(+), 10 deletions(-)

-- 
1.8.5.5

--
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 6/6] mfd: core: fix platform-device id generation

2014-09-26 Thread Johan Hovold
Make sure to always honour multi-function devices registered with
PLATFORM_DEVID_NONE (-1) or PLATFORM_DEVID_AUTO (-2) as id base. In this
case it does not make sense to append the cell id to the mfd-id base and
potentially change the requested behaviour.

Specifically this will allow multi-function devices to be registered
with PLATFORM_DEVID_AUTO while still having non-zero cell ids.

Signed-off-by: Johan Hovold jo...@kernel.org
---
 drivers/mfd/mfd-core.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 892d343193ad..79f25633d7db 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -87,9 +87,15 @@ static int mfd_add_device(struct device *parent, int id,
struct platform_device *pdev;
struct device_node *np = NULL;
int ret = -ENOMEM;
+   int platform_id;
int r;
 
-   pdev = platform_device_alloc(cell-name, id + cell-id);
+   if (id  0)
+   platform_id = id;
+   else
+   platform_id = id + cell-id;
+
+   pdev = platform_device_alloc(cell-name, platform_id);
if (!pdev)
goto fail_alloc;
 
-- 
1.8.5.5

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


[PATCH 2/6] mfd: rtsx_usb: fix platform device-id collision

2014-09-26 Thread Johan Hovold
Hot-pluggable multi-function devices should use PLATFORM_DEVID_AUTO to
avoid name collisions on the platform bus.

This driver currently uses the USB-device address as an id. This makes
name collisions unlikely, but it could still happen if two devices are
connected to separate buses and gets assigned the same address.

Signed-off-by: Johan Hovold jo...@kernel.org
---
 drivers/mfd/rtsx_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
index 71f387ce8cbd..78073e4b87e4 100644
--- a/drivers/mfd/rtsx_usb.c
+++ b/drivers/mfd/rtsx_usb.c
@@ -647,7 +647,7 @@ static int rtsx_usb_probe(struct usb_interface *intf,
/* initialize USB SG transfer timer */
setup_timer(ucr-sg_timer, rtsx_usb_sg_timed_out, (unsigned long) ucr);
 
-   ret = mfd_add_devices(intf-dev, usb_dev-devnum, rtsx_usb_cells,
+   ret = mfd_add_devices(intf-dev, PLATFORM_DEVID_AUTO, rtsx_usb_cells,
ARRAY_SIZE(rtsx_usb_cells), NULL, 0, NULL);
if (ret)
goto out_init_fail;
-- 
1.8.5.5

--
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 4/6] mfd: use mfd_add_hotplug_devices helper

2014-09-26 Thread Johan Hovold
Use mfd_add_hotplug_devices helper to register the subdevices.

Signed-off-by: Johan Hovold jo...@kernel.org
---
 drivers/mfd/rtsx_usb.c   | 4 ++--
 drivers/mfd/viperboard.c | 5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
index 78073e4b87e4..5b17339fce7c 100644
--- a/drivers/mfd/rtsx_usb.c
+++ b/drivers/mfd/rtsx_usb.c
@@ -647,8 +647,8 @@ static int rtsx_usb_probe(struct usb_interface *intf,
/* initialize USB SG transfer timer */
setup_timer(ucr-sg_timer, rtsx_usb_sg_timed_out, (unsigned long) ucr);
 
-   ret = mfd_add_devices(intf-dev, PLATFORM_DEVID_AUTO, rtsx_usb_cells,
-   ARRAY_SIZE(rtsx_usb_cells), NULL, 0, NULL);
+   ret = mfd_add_hotplug_devices(intf-dev, rtsx_usb_cells,
+   ARRAY_SIZE(rtsx_usb_cells));
if (ret)
goto out_init_fail;
 
diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
index 3c2b8f9e3c84..be450ebcb52d 100644
--- a/drivers/mfd/viperboard.c
+++ b/drivers/mfd/viperboard.c
@@ -93,9 +93,8 @@ static int vprbrd_probe(struct usb_interface *interface,
 version  8, version  0xff,
 vb-usb_dev-bus-busnum, vb-usb_dev-devnum);
 
-   ret = mfd_add_devices(interface-dev, PLATFORM_DEVID_AUTO,
-   vprbrd_devs, ARRAY_SIZE(vprbrd_devs), NULL, 0,
-   NULL);
+   ret = mfd_add_hotplug_devices(interface-dev, vprbrd_devs,
+   ARRAY_SIZE(vprbrd_devs));
if (ret != 0) {
dev_err(interface-dev, Failed to add mfd devices to core.);
goto error;
-- 
1.8.5.5

--
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 3/6] mfd: core: add helper function to register hotplug devices

2014-09-26 Thread Johan Hovold
Hot-pluggable multi-function devices should always be registered with
PLATFORM_DEVID_AUTO to avoid name collisions on the platform bus. This
helper also hides the memory map and irq parameters, which aren't used
by hot-pluggable (e.g. USB-based) devices.

Signed-off-by: Johan Hovold jo...@kernel.org
---
 include/linux/mfd/core.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
index f543de91ce19..1e47262a1c63 100644
--- a/include/linux/mfd/core.h
+++ b/include/linux/mfd/core.h
@@ -108,6 +108,13 @@ extern int mfd_add_devices(struct device *parent, int id,
   struct resource *mem_base,
   int irq_base, struct irq_domain *irq_domain);
 
+static inline int mfd_add_hotplug_devices(struct device *parent,
+   const struct mfd_cell *cells, int n_devs)
+{
+   return mfd_add_devices(parent, PLATFORM_DEVID_AUTO, cells, n_devs,
+   NULL, 0, NULL);
+}
+
 extern void mfd_remove_devices(struct device *parent);
 
 #endif
-- 
1.8.5.5

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


[PATCH 1/6] mfd: viperboard: fix platform-device id collision

2014-09-26 Thread Johan Hovold
Allow more than one viperboard to be connected by registering with
PLATFORM_DEVID_AUTO instead of PLATFORM_DEVID_NONE.

The subdevices are currently registered with PLATFORM_DEVID_NONE, which
will cause a name collision on the platform bus when a second viperboard
is plugged in:

viperboard 1-2.4:1.0: version 0.00 found at bus 001 address 004
[ cut here ]
WARNING: CPU: 0 PID: 181 at 
/home/johan/work/omicron/src/linux/fs/sysfs/dir.c:31 sysfs_warn_dup+0x74/0x84()
sysfs: cannot create duplicate filename '/bus/platform/devices/viperboard-gpio'
Modules linked in: i2c_viperboard viperboard netconsole [last unloaded: 
viperboard]
CPU: 0 PID: 181 Comm: bash Tainted: GW  3.17.0-rc6 #1
[c0016bf4] (unwind_backtrace) from [c0013860] (show_stack+0x20/0x24)
[c0013860] (show_stack) from [c04305f8] (dump_stack+0x24/0x28)
[c04305f8] (dump_stack) from [c0040fb4] (warn_slowpath_common+0x80/0x98)
[c0040fb4] (warn_slowpath_common) from [c004100c] 
(warn_slowpath_fmt+0x40/0x48)
[c004100c] (warn_slowpath_fmt) from [c016f1bc] (sysfs_warn_dup+0x74/0x84)
[c016f1bc] (sysfs_warn_dup) from [c016f548] 
(sysfs_do_create_link_sd.isra.2+0xcc/0xd0)
[c016f548] (sysfs_do_create_link_sd.isra.2) from [c016f588] 
(sysfs_create_link+0x3c/0x48)
[c016f588] (sysfs_create_link) from [c02867ec] (bus_add_device+0x12c/0x1e0)
[c02867ec] (bus_add_device) from [c0284820] (device_add+0x410/0x584)
[c0284820] (device_add) from [c0289440] (platform_device_add+0xd8/0x26c)
[c0289440] (platform_device_add) from [c02a5ae4] 
(mfd_add_device+0x240/0x344)
[c02a5ae4] (mfd_add_device) from [c02a5ce0] (mfd_add_devices+0xb8/0x110)
[c02a5ce0] (mfd_add_devices) from [bf00d1c8] (vprbrd_probe+0x160/0x1b0 
[viperboard])
[bf00d1c8] (vprbrd_probe [viperboard]) from [c030c000] 
(usb_probe_interface+0x1bc/0x2a8)
[c030c000] (usb_probe_interface) from [c028768c] 
(driver_probe_device+0x14c/0x3ac)
[c028768c] (driver_probe_device) from [c02879e4] (__driver_attach+0xa4/0xa8)
[c02879e4] (__driver_attach) from [c0285698] (bus_for_each_dev+0x70/0xa4)
[c0285698] (bus_for_each_dev) from [c0287030] (driver_attach+0x2c/0x30)
[c0287030] (driver_attach) from [c030a288] (usb_store_new_id+0x170/0x1ac)
[c030a288] (usb_store_new_id) from [c030a2f8] (new_id_store+0x34/0x3c)
[c030a2f8] (new_id_store) from [c02853ec] (drv_attr_store+0x30/0x3c)
[c02853ec] (drv_attr_store) from [c016eaa8] (sysfs_kf_write+0x5c/0x60)
[c016eaa8] (sysfs_kf_write) from [c016dc68] (kernfs_fop_write+0xd4/0x194)
[c016dc68] (kernfs_fop_write) from [c010fe40] (vfs_write+0xb4/0x1c0)
[c010fe40] (vfs_write) from [c01104a8] (SyS_write+0x4c/0xa0)
[c01104a8] (SyS_write) from [c000f900] (ret_fast_syscall+0x0/0x48)
---[ end trace 98e8603c22d65817 ]---
viperboard 1-2.4:1.0: Failed to add mfd devices to core.
viperboard: probe of 1-2.4:1.0 failed with error -17

Signed-off-by: Johan Hovold jo...@kernel.org
---
 drivers/mfd/viperboard.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
index e00f5340ed87..3c2b8f9e3c84 100644
--- a/drivers/mfd/viperboard.c
+++ b/drivers/mfd/viperboard.c
@@ -93,8 +93,9 @@ static int vprbrd_probe(struct usb_interface *interface,
 version  8, version  0xff,
 vb-usb_dev-bus-busnum, vb-usb_dev-devnum);
 
-   ret = mfd_add_devices(interface-dev, -1, vprbrd_devs,
-   ARRAY_SIZE(vprbrd_devs), NULL, 0, NULL);
+   ret = mfd_add_devices(interface-dev, PLATFORM_DEVID_AUTO,
+   vprbrd_devs, ARRAY_SIZE(vprbrd_devs), NULL, 0,
+   NULL);
if (ret != 0) {
dev_err(interface-dev, Failed to add mfd devices to core.);
goto error;
-- 
1.8.5.5

--
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 5/6] HID: hid-sensor-hub: use mfd_add_hotplug_devices helper

2014-09-26 Thread Johan Hovold
Use mfd_add_hotplug_devices helper to register the subdevices.

Compile-only tested.

Signed-off-by: Johan Hovold jo...@kernel.org
---
 drivers/hid/hid-sensor-hub.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 2ac25760a9a9..fbe1a6d2aa53 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -641,9 +641,6 @@ static int sensor_hub_probe(struct hid_device *hdev,
goto err_stop_hw;
}
sd-hid_sensor_hub_client_devs[
-   sd-hid_sensor_client_cnt].id =
-   PLATFORM_DEVID_AUTO;
-   sd-hid_sensor_hub_client_devs[
sd-hid_sensor_client_cnt].name = name;
sd-hid_sensor_hub_client_devs[
sd-hid_sensor_client_cnt].platform_data =
@@ -659,8 +656,9 @@ static int sensor_hub_probe(struct hid_device *hdev,
if (last_hsdev)
last_hsdev-end_collection_index = i;
 
-   ret = mfd_add_devices(hdev-dev, 0, sd-hid_sensor_hub_client_devs,
-   sd-hid_sensor_client_cnt, NULL, 0, NULL);
+   ret = mfd_add_hotplug_devices(hdev-dev,
+   sd-hid_sensor_hub_client_devs,
+   sd-hid_sensor_client_cnt);
if (ret  0)
goto err_stop_hw;
 
-- 
1.8.5.5

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


[PATCH 1/2] USB: EHCI: add portpower callback and override

2014-09-26 Thread Michael Grzeschik
The current EHCI implementation is prepared to toggle the
PORT_POWER bit to enable or disable a USB-Port. In some
cases this port power can not be toggled by the PORT_POWER
bit but instead i.e. by an external GPIO.
This patch adds an callback to be triggered as well.
The host needs to assign a capable portpower callback.

Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
---
 drivers/usb/host/ehci-hcd.c |  2 ++
 drivers/usb/host/ehci-hub.c | 25 +++--
 drivers/usb/host/ehci.h |  1 +
 include/linux/usb/hcd.h |  1 +
 4 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 488a308..063128c 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1233,6 +1233,8 @@ void ehci_init_driver(struct hc_driver *drv,
drv-hcd_priv_size += over-extra_priv_size;
if (over-reset)
drv-reset = over-reset;
+   if (over-portpower)
+   drv-portpower = over-portpower;
}
 }
 EXPORT_SYMBOL_GPL(ehci_init_driver);
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 6130b75..2bfe660 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -952,6 +952,11 @@ int ehci_hub_control(
clear_bit(wIndex, ehci-port_c_suspend);
break;
case USB_PORT_FEAT_POWER:
+
+   /* disable port power by callback */
+   if (hcd-driver-portpower)
+   hcd-driver-portpower(hcd, 0);
+
if (HCS_PPC (ehci-hcs_params))
ehci_writel(ehci, temp  ~PORT_POWER,
status_reg);
@@ -1002,12 +1007,16 @@ int ehci_hub_control(
 * power switching; they're allowed to just limit the
 * current.  khubd will turn the power back on.
 */
-   if (((temp  PORT_OC) || (ehci-need_oc_pp_cycle))
-HCS_PPC(ehci-hcs_params)) {
-   ehci_writel(ehci,
-   temp  ~(PORT_RWC_BITS | PORT_POWER),
-   status_reg);
-   temp = ehci_readl(ehci, status_reg);
+   if ((temp  PORT_OC) || (ehci-need_oc_pp_cycle)) {
+   /* disable port power by callback */
+   if (hcd-driver-portpower)
+   hcd-driver-portpower(hcd, 0);
+
+   if (HCS_PPC(ehci-hcs_params)) {
+   temp = ~(PORT_RWC_BITS | PORT_POWER);
+   ehci_writel(ehci, temp, status_reg);
+   temp = ehci_readl(ehci, status_reg);
+   }
}
}
 
@@ -1187,6 +1196,10 @@ int ehci_hub_control(
set_bit(wIndex, ehci-suspended_ports);
break;
case USB_PORT_FEAT_POWER:
+   /* enable port power by callback */
+   if (hcd-driver-portpower)
+   hcd-driver-portpower(hcd, 1);
+
if (HCS_PPC (ehci-hcs_params))
ehci_writel(ehci, temp | PORT_POWER,
status_reg);
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index eee228a..7f48048 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -859,6 +859,7 @@ static inline u32 hc32_to_cpup (const struct ehci_hcd 
*ehci, const __hc32 *x)
 struct ehci_driver_overrides {
size_t  extra_priv_size;
int (*reset)(struct usb_hcd *hcd);
+   int (*portpower)(struct usb_hcd *hcd, int enable);
 };
 
 extern voidehci_init_driver(struct hc_driver *drv,
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 485cd5e..3f60acb 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -241,6 +241,7 @@ struct hc_driver {
/* called to init HCD and root hub */
int (*reset) (struct usb_hcd *hcd);
int (*start) (struct usb_hcd *hcd);
+   int (*portpower)(struct usb_hcd *hcd, int enable);
 
/* NOTE:  these suspend/resume calls relate to the HC as
 * a whole, not just the root hub; they're for PCI bus glue.
-- 
2.1.0

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


[PATCH 0/2] Add portpower callback to ehci-hub

2014-09-26 Thread Michael Grzeschik
This series adds an portpower callback that can be triggered additionaly
to the PORT_POWER ehci bit. With that we are able to toggle an external
registered regulator i.e. fixed GPIO.

The first user is the chipidea-host to toggle the regulator.

This series is based on v3.17-rc6.

Thanks,
Michael

Michael Grzeschik (2):
  USB: EHCI: add portpower callback and override
  usb: chipidea: add portpower override for host and use it

 drivers/usb/chipidea/host.c | 69 +++--
 drivers/usb/host/ehci-hcd.c |  2 ++
 drivers/usb/host/ehci-hub.c | 25 
 drivers/usb/host/ehci.h |  1 +
 include/linux/usb/hcd.h |  1 +
 5 files changed, 71 insertions(+), 27 deletions(-)

-- 
2.1.0

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


[PATCH 2/2] usb: chipidea: add portpower override for host and use it

2014-09-26 Thread Michael Grzeschik
This patch adds an external portpower override callback.
It can be used by the ehci-hub to toggle the PORT_POWER
by the registered regulator. That is needed when an external
GPIO is used instead of the default PORT_POWER bit.

Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
---
 drivers/usb/chipidea/host.c | 69 +++--
 1 file changed, 48 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index a93d950..17537b1 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -34,6 +34,44 @@
 
 static struct hc_driver __read_mostly ci_ehci_hc_driver;
 
+struct ehci_ci_priv {
+   struct regulator *reg_vbus;
+};
+
+static int ehci_ci_portpower(struct usb_hcd *hcd, int enable)
+{
+   struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+   struct ehci_ci_priv *priv = (struct ehci_ci_priv *)ehci-priv;
+   struct device *dev = hcd-self.controller;
+   struct ci_hdrc *ci = dev_get_drvdata(dev);
+   int ret = 0;
+
+   if (enable  hw_read(ci, OP_PORTSC, PORT_POWER))
+   return -EINVAL;
+
+   if (!enable  !hw_read(ci, OP_PORTSC, PORT_POWER))
+   return -EINVAL;
+
+   if (priv-reg_vbus  !ci_otg_is_fsm_mode(ci)) {
+   if (enable)
+   ret = regulator_enable(priv-reg_vbus);
+   else
+   ret = regulator_disable(priv-reg_vbus);
+   if (ret) {
+   dev_err(dev,
+   Failed to %s vbus regulator, ret=%d\n,
+   enable ? enable : disable, ret);
+   return ret;
+   }
+   }
+   return 0;
+};
+
+static const struct ehci_driver_overrides ehci_ci_overrides = {
+   .extra_priv_size = sizeof(struct ehci_ci_priv),
+   .portpower   = ehci_ci_portpower,
+};
+
 static irqreturn_t host_irq(struct ci_hdrc *ci)
 {
return usb_hcd_irq(ci-irq, ci-hcd);
@@ -43,6 +81,7 @@ static int host_start(struct ci_hdrc *ci)
 {
struct usb_hcd *hcd;
struct ehci_hcd *ehci;
+   struct ehci_ci_priv *priv;
int ret;
 
if (usb_disabled())
@@ -67,23 +106,15 @@ static int host_start(struct ci_hdrc *ci)
ehci-has_tdi_phy_lpm = ci-hw_bank.lpm;
ehci-imx28_write_fix = ci-imx28_write_fix;
 
-   /*
-* vbus is always on if host is not in OTG FSM mode,
-* otherwise should be controlled by OTG FSM
-*/
-   if (ci-platdata-reg_vbus  !ci_otg_is_fsm_mode(ci)) {
-   ret = regulator_enable(ci-platdata-reg_vbus);
-   if (ret) {
-   dev_err(ci-dev,
-   Failed to enable vbus regulator, ret=%d\n,
-   ret);
-   goto put_hcd;
-   }
-   }
+   priv = (struct ehci_ci_priv *)ehci-priv;
+   priv-reg_vbus = NULL;
+
+   if (ci-platdata-reg_vbus)
+   priv-reg_vbus = ci-platdata-reg_vbus;
 
ret = usb_add_hcd(hcd, 0, 0);
if (ret) {
-   goto disable_reg;
+   goto put_hcd;
} else {
struct usb_otg *otg = ci-transceiver-otg;
 
@@ -99,10 +130,6 @@ static int host_start(struct ci_hdrc *ci)
 
return ret;
 
-disable_reg:
-   if (ci-platdata-reg_vbus  !ci_otg_is_fsm_mode(ci))
-   regulator_disable(ci-platdata-reg_vbus);
-
 put_hcd:
usb_put_hcd(hcd);
 
@@ -114,10 +141,10 @@ static void host_stop(struct ci_hdrc *ci)
struct usb_hcd *hcd = ci-hcd;
 
if (hcd) {
+   if (!ci_otg_is_fsm_mode(ci))
+   ehci_ci_portpower(hcd, 0);
usb_remove_hcd(hcd);
usb_put_hcd(hcd);
-   if (ci-platdata-reg_vbus  !ci_otg_is_fsm_mode(ci))
-   regulator_disable(ci-platdata-reg_vbus);
}
 }
 
@@ -145,7 +172,7 @@ int ci_hdrc_host_init(struct ci_hdrc *ci)
rdrv-name  = host;
ci-roles[CI_ROLE_HOST] = rdrv;
 
-   ehci_init_driver(ci_ehci_hc_driver, NULL);
+   ehci_init_driver(ci_ehci_hc_driver, ehci_ci_overrides);
 
return 0;
 }
-- 
2.1.0

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


Re: [RFC PATCH 4/4] usb: dwc3: introduce dual role switch function with debugfs

2014-09-26 Thread Felipe Balbi
Hi,

On Fri, Sep 26, 2014 at 05:22:09PM +0800, Huang Rui wrote:
  What you need here is something to talk to usbcore and udc-core and
  orchestrate the mode change through usb_add_hcd()/usb_add_gadget_udc()
  and their counterparts.
  
  George Cherian is already working on a version of that.
  
 
 I saw your tree have a dwc3-role-switch branch, is that you mentioned
 here for the working.

unfortunately not, what George is working is not there yet :-s

-- 
balbi


signature.asc
Description: Digital signature


Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL

2014-09-26 Thread Felipe Balbi
On Fri, Sep 26, 2014 at 04:50:26PM +0800, Huang Rui wrote:
 On Thu, Sep 25, 2014 at 09:50:32AM -0500, Felipe Balbi wrote:
  Hi,
  
  On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
   diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
   index b0f4d52..6138c5d 100644
   --- a/drivers/usb/dwc3/core.c
   +++ b/drivers/usb/dwc3/core.c
   @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
}

/**
   + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
   + * @dwc: Pointer to our controller context structure
   + */
   +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
   +{
   + u32 reg = 0;
   +
   + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK | DWC3_GUSB3PIPECTL_UX_EXITINPX
   + | DWC3_GUSB3PIPECTL_UX_EXITINPX | DWC3_GUSB3PIPECTL_U1U2EXITFAIL
   + | DWC3_GUSB3PIPECTL_DEPOCHANGE | DWC3_GUSB3PIPECTL_RX_DETOPOLL;
  
  UX_EXITINPX is supposed to be used with a faulty PHY, I need to see an
  erratum before I can accept it. You have also duplicated the bit in this
  initialization.
  
  U1U2EXITFAIL - also a workaround bit and I need to see an erratum.
  
  RX_DETOPOLL - it seems like it's safe to leave this one out as it can
  only be proven to work with this bit after going through full USB
  certification.
  
 
 What do you mean of the faulty PHY?
 We would use AMD PHY to talk with DWC3 controller.

Look at the description of each of those bits and you'll see it mentions
they're supposed to be used for workarounds. Here's UX_EXIT_IN_PX, as an
example:


This bit is added for SS PHY workaround where SS PHY ...


It's alright that AMD PHY needs this bit, but then, let's get
confirmation from IP/SoC/SilVal team and add a proper comment stating
why we need them. This is so we don't forget that $version of AMD's PHY
needs workarounds for A, B, and C silicon errata.

Also, I'd have to ask you to provide full boot logs of your platform
booting with my testing/next (where all the latest patches are) plus
your patches. Then, load g_mass_storage with a backing storage of your
choice and run my msc.c/msc.sh tools which you can get from [1] and [2];
post the logs for that too. Last, but not least, please USB30CV (chapter
9 and Link Layer test, at least) just so we know there isn't anything
new with your version of the core, which I suppose is 2.80a, based on
the LPM Errata bits.

This is just because I don't have access to the HW myself, so I can't
verify your patches. One thing I can tell you, with my testing/next,
dwc3 is really stable. I have every test passing except for Halt
Endpoint which I'm debugging right now.

   + reg |= DWC3_GUSB3PIPECTL_DEP1P2P3(1) | DWC3_GUSB3PIPECTL_TX_DEEPH(1);
  
  DEP1P2P3 and DEP0CHANGE seem to be required only for faulty PHYs too, I
  need to see an erratum.
  
   + dwc3_writel(dwc-regs, DWC3_GUSB3PIPECTL(0), reg);
   +
   + mdelay(100);
   +}
   +
   +/**
 * dwc3_free_one_event_buffer - Frees one event buffer
 * @dwc: Pointer to our controller context structure
 * @evt: Pointer to event buffer to be freed
   @@ -412,9 +431,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
 if (ret)
 goto err0;

   + if (dwc-quirks  DWC3_AMD_NL_PLAT)
   + dwc3_core_nl_set_pipe3(dwc);
   +
 reg = dwc3_readl(dwc-regs, DWC3_GCTL);
   +
 reg = ~DWC3_GCTL_SCALEDOWN_MASK;
   - reg = ~DWC3_GCTL_DISSCRAMBLE;
   +
   + if (dwc-quirks  DWC3_AMD_NL_PLAT) {
   + reg |= DWC3_GCTL_DISSCRAMBLE;
  
  you're disabling scrambling ? What about EMI ? Why doesn't your device
  work with scrambling ? I need to see an erratum before I can accept
  this.
  
 
 Sorry, disabling scrambling is workaround for debugging the temporary
 simulation board, needn't be set for the SoC chip. Will update in v2.

oh, alright. Then let's not merge this in mainline. I guess I have an
idea which simulation board you're using :-)

   + reg |= DWC3_GCTL_U2EXIT_LFPS;
  
  hmm, seems like this bit was added due to a faulty host which was found.
  I need to see an erratum before I can accept this.
  
   + reg |= DWC3_GCTL_GBLHIBERNATIONEN;
  
  looks like this should always be set for all versions of the core
  configured with hibernation.
  
 
 yes, amd nl chip will support hibernation.

in that case, we do this:

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 4d4e854..584dcde 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -449,6 +449,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
case DWC3_GHWPARAMS1_EN_PWROPT_HIB:
/* enable hibernation here */
dwc-nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4);
+   reg |= DWC3_GCTL_GBLHIBERNATIONEN;
break;
default:
dev_dbg(dwc-dev, No power optimization available\n);

that'll work for everybody with hibernation enabled in coreConsultant.

   diff --git a/drivers/usb/dwc3/dwc3-pci.c 

Re: [PATCH 1/2] USB: EHCI: add portpower callback and override

2014-09-26 Thread Alan Stern
On Fri, 26 Sep 2014, Michael Grzeschik wrote:

 The current EHCI implementation is prepared to toggle the
 PORT_POWER bit to enable or disable a USB-Port. In some
 cases this port power can not be toggled by the PORT_POWER
 bit but instead i.e. by an external GPIO.
 This patch adds an callback to be triggered as well.
 The host needs to assign a capable portpower callback.

The idea is reasonable enough, I suppose (but where does this stop?  
Before you know it, every function normally carried out by the host 
controller will actually be handled by a platform-specific piece of 
hardware!)

The implementation leaves something to be desired.  Instead of keeping
the current port-power code in ehci-hcd inline, as it is, you should
define a default callback routine to do the work.  Then the override
can replace the default callback.

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: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL

2014-09-26 Thread Felipe Balbi
Hi,

On Fri, Sep 26, 2014 at 09:35:21AM -0500, Felipe Balbi wrote:
 This is just because I don't have access to the HW myself, so I can't
 verify your patches. One thing I can tell you, with my testing/next,
 dwc3 is really stable. I have every test passing except for Halt
 Endpoint which I'm debugging right now.

alright, found the problem. I'll push to my testing/next in a bit.

-- 
balbi


signature.asc
Description: Digital signature


g_mass_storage READ_CAPACITY(16)

2014-09-26 Thread Felipe Balbi
Hi Alan,

it seems like newer versions of USB20CV will fail us on the MSC tests
unless we support READ_CAPACITY(16).

There is one detail though, looking at the sniffer, CBW comes as:

55 53 42 43 43 59 45 4C 00 00 00 00 00 00 0A 2F 00 00
00 00 00 00 00 80 00 00 00 00 00 00 00

so SCSI cmd opcode is set to 0x2f. Looking at scsi.h that's defined to
VERIFY:

89 #define WRITE_VERIFY  0x2e
90 #define VERIFY0x2f
91 #define SEARCH_HIGH   0x30

I suppose this would be a Mass Storage quirk ? Any idea how to implement
READ CAPACITY 16 ? (yeah, haven't read SCSI spec yet, doing it now).

Any hints will be appreciated.

cheers

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx

2014-09-26 Thread Felipe Balbi
Hi,

On Fri, Sep 26, 2014 at 09:20:54AM +0200, Arnd Bergmann wrote:
 On Thursday 25 September 2014 19:39:34 Felipe Balbi wrote:
   
   why would a glue layer need to access registers from the core ? That
   sounds very odd. I haven't seen that and will, definitely, NACK such a
   patch 
   
   can you further describe why you think a glue layer might need to access
   core IP's registers ?
  
  I just realised we're talking about chipidea here... in any case, it's
  still valid to ask why would glue need to fiddle with core IP's
  registers.
 
 Generally, the glue driver wouldn't access the registers, but I don't
 think it's important to prevent it from doing that. In some cases, 

sure it is. Have already gone through debugging sessions just because
someone fiddled with registers they shouldn't. Also RMK's L2 rework
patchset is another example of why it's important to prevent other
layers from messing with registers they don't really own.

 a glue driver needs to override a function of the core driver, e.g.
 to work around an errata. We have a lot of those quirks in ATA drivers,

pass a quirk flag and let core driver handle it.

 one example from ahci_mvebu.c is
 
 static void ahci_mvebu_regret_option(struct ahci_host_priv *hpriv)
 {
 /*
  * Enable the regret bit to allow the SATA unit to regret a
  * request that didn't receive an acknowlegde and avoid a
  * deadlock
  */
 writel(0x4, hpriv-mmio + AHCI_VENDOR_SPECIFIC_0_ADDR);
 writel(0x80, hpriv-mmio + AHCI_VENDOR_SPECIFIC_0_DATA);

I would rather see:

if (this_is_one_of_the_broken_mvebu_versions(hpriv))
quirks |= AHCI_NEEDS_REGRET_BIT;

then let core handle the rest. If other glue has the same bug and needs
the workaround, we don't duplicate code.

-- 
balbi


signature.asc
Description: Digital signature


Re: g_mass_storage bug ?

2014-09-26 Thread Felipe Balbi
On Thu, Sep 25, 2014 at 06:12:51PM -0500, Felipe Balbi wrote:
 Hi,
 
 On Thu, Sep 25, 2014 at 09:57:59PM +, Paul Zimmerman wrote:
   From: Felipe Balbi [mailto:ba...@ti.com]
   
   On Thu, Sep 25, 2014 at 07:08:12PM +, Paul Zimmerman wrote:
 From: Alan Stern [mailto:st...@rowland.harvard.edu]
 Sent: Thursday, September 25, 2014 11:08 AM

 On Thu, 25 Sep 2014, Paul Zimmerman wrote:

  That's why I don't understand how this can happen for IN. AFAIK, a 
  STALL
  is only sent in response to something the host sent in the CBW. At 
  that
  point, there shouldn't be any IN transfers active.

 The gadget may send a partial response to the CBW.  The end of the
 response is marked with a STALL.  The mass-storage gadget driver
 submits the partial response and then calls usb_ep_set_halt() without
 waiting for the IN data to be delivered.  It relies on the UDC driver
 returning -EAGAIN if any data is still pending.
   
I guess you're referring to the code under the DATA_DIR_TO_HOST case
in finish_reply().
   
Felipe, in our vendor driver, the ep_set_halt() and ep_set_wedge()
functions check the request queue for the endpoint, and if it is not
empty, they return -EAGAIN.
   
   is that really enough ? The request is deleted (rather, moved to another
   list) as soon as StartTransfer is issued. I can certainly check both
   lists (and already do) but I fear that we might still race with the HW
   because there's no documentation to guarantee that I won't :-)
   
I see your patch for the dwc3 driver does add that, in addition to the
FIFO empty check. Does it still work OK if you remove the FIFO empty
check portion?
   
   It probably does, but I can't be sure there won't be a corner case where
   the list is empty (Xfercomplete was issued and request given back) but
   host hasn't moved all the data. Synopsys databook doesn't guarantee that
   will never be the case.
   
   Can you confirm, without a shadow of a doubt, that XferComplete will
   only be issued after host has moved every single bit of data ? If that
   can be updated to the databook, then sure, I can remove the FIFO space
   check; otherwise we might leave a corner case that will take us a few
   days to debug again because it will be very difficult to trigger :-)
  
  From the 2.70a databook, section 8.2.2:
  
  While processing TRBs, two conditions may cause the core to write out an
   event and raise an interrupt line:
  - TRB Complete:
  - For OUT endpoints, a packet is received which reduces the
remaining byte count in the TRB buffer to zero.
  - For IN endpoints, an acknowledgement is received for a
transmitted packet which reduces the remaining byte count
in the TRB buffer to zero.
  - Short Packet Received:
  - For OUT endpoints only. While writing to a TRB buffer,
the endpoint receives a packet that is smaller than the
endpoint's MaxPacketSize.
  
  
  So for an IN, the completion event doesn't come until the TRB has been
  ACKed, which means all the data has been sent.
 
 a very good point indeed. I completely missed that. Well, the patch can
 become a lot smaller, which makes my life easier when backporting :-)
 
 I'll test that out tomorrow, for sure :-)
 
  But, I'm not saying you *have* to remove the FIFO space check. I think
  we now know the problem was caused by the missing -EAGAIN. So having
 
 sure, that's certainly what was missing.
 
  the FIFO check there shouldn't hurt anything, it's not masking some
 
 it's also completely useless. A few register accesses for no reason :-)
 
  other problem. And you may need it in the future for one of those
  other cases the databook talks about.
 
 I'll keep that in the back of my head. Perhaps I can repurpose these
 FIFO space accessors to implement usb_ep_fifo_status(). I'll consider
 that.

alright, removed all that and it still works fine. Even passes Halt
Endpoint test on usb20cv.

-- 
balbi


signature.asc
Description: Digital signature


Re: g_mass_storage READ_CAPACITY(16)

2014-09-26 Thread Alan Stern
On Fri, 26 Sep 2014, Felipe Balbi wrote:

 Hi Alan,
 
 it seems like newer versions of USB20CV will fail us on the MSC tests
 unless we support READ_CAPACITY(16).

How is that possible?  The USB Mass-Storage Class spec doesn't require 
the device to support any particular set of SCSI commands.  READ 
CAPACITY(16) isn't even part of the SCSI-2 spec, which is what 
g-mass-storage supports.

 There is one detail though, looking at the sniffer, CBW comes as:
 
 55 53 42 43 43 59 45 4C 00 00 00 00 00 00 0A 2F 00 00
 00 00 00 00 00 80 00 00 00 00 00 00 00
 
 so SCSI cmd opcode is set to 0x2f. Looking at scsi.h that's defined to
 VERIFY:
 
 89 #define WRITE_VERIFY  0x2e
 90 #define VERIFY0x2f
 91 #define SEARCH_HIGH   0x30

Yes, that's right.  g-mass-storage does implement VERIFY.

 I suppose this would be a Mass Storage quirk ? Any idea how to implement
 READ CAPACITY 16 ? (yeah, haven't read SCSI spec yet, doing it now).

According to scsi.h, READ CAPACITY(16) is service action 0x10 of
SERVICE ACTION IN, which is 0x9e.  The draft SBC-3 standard from
t10.org agrees.  I don't see how anyone could get 0x2f out of that.

 Any hints will be appreciated.

We could implement READ CAPACITY(16) if necessary.  But since we don't 
implement READ(16) or WRITE(16), it seems rather pointless.

Are you certain you are interpreting the USBCV output correctly?

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 READ_CAPACITY(16)

2014-09-26 Thread Felipe Balbi
On Fri, Sep 26, 2014 at 12:06:00PM -0400, Alan Stern wrote:
  Any hints will be appreciated.
 
 We could implement READ CAPACITY(16) if necessary.  But since we don't 
 implement READ(16) or WRITE(16), it seems rather pointless.
 
 Are you certain you are interpreting the USBCV output correctly?

heh, yes I am. That READ CAPACITY(16) is just a note, not the error
itself. It seems like one of the CSWs (it sends a lot of commands on
this test) was sent with zero bytes. I'll go fire up the sniffer and
debug this further.

-- 
balbi


signature.asc
Description: Digital signature


Re: g_mass_storage READ_CAPACITY(16)

2014-09-26 Thread Felipe Balbi
Hi,

On Fri, Sep 26, 2014 at 11:15:03AM -0500, Felipe Balbi wrote:
 On Fri, Sep 26, 2014 at 12:06:00PM -0400, Alan Stern wrote:
   Any hints will be appreciated.
  
  We could implement READ CAPACITY(16) if necessary.  But since we don't 
  implement READ(16) or WRITE(16), it seems rather pointless.
  
  Are you certain you are interpreting the USBCV output correctly?
 
 heh, yes I am. That READ CAPACITY(16) is just a note, not the error
 itself. It seems like one of the CSWs (it sends a lot of commands on
 this test) was sent with zero bytes. I'll go fire up the sniffer and
 debug this further.

alright, found it. It was caused by another bug on my set_stall fix.
Now, hopefully, it's all fixed. Passing USB20CV chapter 9 and MSC tests,
also working against Linux and Mac OS X.

Here's the final patch, I'll send it officialy once the merge window has
closed:

8

From 4c6282e877ebef6db6bde6f661acf4ea37b3920a Mon Sep 17 00:00:00 2001
From: Felipe Balbi ba...@ti.com
Date: Wed, 24 Sep 2014 14:19:52 -0500
Subject: [PATCH] usb: dwc3: gadget: fix set_halt() bug with pending transfers

According to our Gadget Framework API documentation,
-set_halt() *must* return -EAGAIN if we have pending
transfers (on either direction) or FIFO isn't empty (on
TX endpoints).

Fix this bug so that the mass storage gadget can be used
without stall=0 parameter.

This patch should be backported to all kernels since v3.2.

Cc: sta...@vger.kernel.org # v3.2+
Suggested-by: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/dwc3/ep0.c|  4 ++--
 drivers/usb/dwc3/gadget.c | 16 
 drivers/usb/dwc3/gadget.h |  2 +-
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 36f6158..ae6b575 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -256,7 +256,7 @@ static void dwc3_ep0_stall_and_restart(struct dwc3 *dwc)
 
/* stall is always issued on EP0 */
dep = dwc-eps[0];
-   __dwc3_gadget_ep_set_halt(dep, 1);
+   __dwc3_gadget_ep_set_halt(dep, 1, false);
dep-flags = DWC3_EP_ENABLED;
dwc-delayed_status = false;
 
@@ -480,7 +480,7 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
return -EINVAL;
if (set == 0  (dep-flags  DWC3_EP_WEDGE))
break;
-   ret = __dwc3_gadget_ep_set_halt(dep, set);
+   ret = __dwc3_gadget_ep_set_halt(dep, set, true);
if (ret)
return -EINVAL;
break;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index de53361..68497b3 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -581,7 +581,7 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
 
/* make sure HW endpoint isn't stalled */
if (dep-flags  DWC3_EP_STALL)
-   __dwc3_gadget_ep_set_halt(dep, 0);
+   __dwc3_gadget_ep_set_halt(dep, 0, false);
 
reg = dwc3_readl(dwc-regs, DWC3_DALEPENA);
reg = ~DWC3_DALEPENA_EP(dep-number);
@@ -1202,7 +1202,7 @@ out0:
return ret;
 }
 
-int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value)
+int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol)
 {
struct dwc3_gadget_ep_cmd_paramsparams;
struct dwc3 *dwc = dep-dwc;
@@ -1216,6 +1216,14 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int 
value)
memset(params, 0x00, sizeof(params));
 
if (value) {
+   if (!protocol  ((dep-direction  dep-flags  DWC3_EP_BUSY) 
||
+   (!list_empty(dep-req_queued) ||
+!list_empty(dep-request_list {
+   dev_dbg(dwc-dev, %s: pending request, cannot halt\n,
+   dep-name);
+   return -EAGAIN;
+   }
+
ret = dwc3_send_gadget_ep_cmd(dwc, dep-number,
DWC3_DEPCMD_SETSTALL, params);
if (ret)
@@ -1246,7 +1254,7 @@ static int dwc3_gadget_ep_set_halt(struct usb_ep *ep, int 
value)
int ret;
 
spin_lock_irqsave(dwc-lock, flags);
-   ret = __dwc3_gadget_ep_set_halt(dep, value);
+   ret = __dwc3_gadget_ep_set_halt(dep, value, false);
spin_unlock_irqrestore(dwc-lock, flags);
 
return ret;
@@ -1265,7 +1273,7 @@ static int dwc3_gadget_ep_set_wedge(struct usb_ep *ep)
if (dep-number == 0 || dep-number == 1)
ret = __dwc3_gadget_ep0_set_halt(ep, 1);
else
-   ret = __dwc3_gadget_ep_set_halt(dep, 1);
+   ret = __dwc3_gadget_ep_set_halt(dep, 1, false);
spin_unlock_irqrestore(dwc-lock, 

[PATCH 02/12] libusbg: fix: reverse comparsion to fix error

2014-09-26 Thread Krzysztof Opasiak
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 src/usbg.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/usbg.c b/src/usbg.c
index bbba689..d2ec246 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -2058,7 +2058,7 @@ int usbg_create_config(usbg_gadget *g, int id, const char 
*label,
free_space = sizeof(cpath) - n;
/* Append string at the end of previous one */
n = snprintf((cpath[n]), free_space, /%s, (*c)-name);
-   if (n  free_space) {
+   if (n = free_space) {
ret = USBG_ERROR_PATH_TOO_LONG;
usbg_free_config(conf);
goto out;
-- 
1.7.9.5

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


[PATCH 04/12] libusbg: Fix off-by-one error.

2014-09-26 Thread Krzysztof Opasiak
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 src/usbg.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/usbg.c b/src/usbg.c
index d2ec246..f12a4b3 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -319,7 +319,7 @@ static int usbg_lookup_function_type(const char *name)
 
 const const char *usbg_get_function_type_str(usbg_function_type type)
 {
-   return type  0  type  sizeof(function_names)/sizeof(char *) ?
+   return type = 0  type  sizeof(function_names)/sizeof(char *) ?
function_names[type] : NULL;
 }
 
-- 
1.7.9.5

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


[PATCH 11/12] libusbg: Add own name to AUTHORS file.

2014-09-26 Thread Krzysztof Opasiak
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 AUTHORS |2 ++
 1 file changed, 2 insertions(+)

diff --git a/AUTHORS b/AUTHORS
index f9b069c..6fea705 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -1 +1,3 @@
 Matt Porter mpor...@linaro.org
+Krzysztof Opasiak k.opas...@samsung.com
+
-- 
1.7.9.5

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


[PATCH 07/12] libusbg: Simplify getting names form library structures

2014-09-26 Thread Krzysztof Opasiak
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com

Changes since v1:
Fix show-gadgets example: get attributes before dereferencing them

Reported-by: Philippe De Swert philippe.desw...@jollamobile.com
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 examples/show-gadgets.c |   42 
 include/usbg/usbg.h |   70 ---
 src/usbg.c  |   35 
 3 files changed, 121 insertions(+), 26 deletions(-)

diff --git a/examples/show-gadgets.c b/examples/show-gadgets.c
index 672eac3..823188f 100644
--- a/examples/show-gadgets.c
+++ b/examples/show-gadgets.c
@@ -30,20 +30,26 @@
 void show_gadget(usbg_gadget *g)
 {
char buf[USBG_MAX_STR_LENGTH];
+   const char *name;
int usbg_ret;
usbg_gadget_attrs g_attrs;
usbg_gadget_strs g_strs;
 
-   usbg_get_gadget_name(g, buf, USBG_MAX_STR_LENGTH);
+   name = usbg_get_gadget_name(g);
+   if (!name) {
+   fprintf(stderr, Unable to get gadget name\n);
+   return;
+   }
+
usbg_ret = usbg_get_gadget_attrs(g, g_attrs);
if (usbg_ret != USBG_SUCCESS) {
fprintf(stderr, Error: %s : %s\n, usbg_error_name(usbg_ret),
-   usbg_strerror(usbg_ret));
+   usbg_strerror(usbg_ret));
return;
}
 
fprintf(stdout, ID %04x:%04x '%s'\n,
-   g_attrs.idVendor, g_attrs.idProduct, buf);
+   g_attrs.idVendor, g_attrs.idProduct, name);
 
usbg_get_gadget_udc(g, buf, USBG_MAX_STR_LENGTH);
fprintf(stdout,   UDC\t\t\t%s\n, buf);
@@ -70,12 +76,17 @@ void show_gadget(usbg_gadget *g)
 
 void show_function(usbg_function *f)
 {
-   char instance[USBG_MAX_STR_LENGTH];
+   const char *instance;
usbg_function_type type;
int usbg_ret;
usbg_function_attrs f_attrs;
 
-   usbg_get_function_instance(f, instance, USBG_MAX_STR_LENGTH);
+   instance = usbg_get_function_instance(f);
+   if (!instance) {
+   fprintf(stderr, Unable to get function instance name\n);
+   return;
+   }
+
type = usbg_get_function_type(f);
usbg_ret = usbg_get_function_attrs(f, f_attrs);
if (usbg_ret != USBG_SUCCESS) {
@@ -120,15 +131,20 @@ void show_config(usbg_config *c)
 {
usbg_binding *b;
usbg_function *f;
-   char buf[USBG_MAX_STR_LENGTH], instance[USBG_MAX_STR_LENGTH];
+   const char *label, *instance, *bname;
usbg_function_type type;
usbg_config_attrs c_attrs;
usbg_config_strs c_strs;
int usbg_ret, id;
 
-   usbg_get_config_label(c, buf, USBG_MAX_STR_LENGTH);
+   label = usbg_get_config_label(c);
+   if (!label) {
+   fprintf(stderr, Unable to get config label\n);
+   return;
+   }
+
id = usbg_get_config_id(c);
-   fprintf(stdout,   Configuration: '%s' ID: %d\n, buf, id);
+   fprintf(stdout,   Configuration: '%s' ID: %d\n, label, id);
 
usbg_ret = usbg_get_config_attrs(c, c_attrs);
if (usbg_ret != USBG_SUCCESS) {
@@ -150,11 +166,15 @@ void show_config(usbg_config *c)
fprintf(stdout, configuration\t%s\n, c_strs.configuration);
 
usbg_for_each_binding(b, c) {
-   usbg_get_binding_name(b, buf, USBG_MAX_STR_LENGTH);
+   bname = usbg_get_binding_name(b);
f = usbg_get_binding_target(b);
-   usbg_get_function_instance(f, instance, USBG_MAX_STR_LENGTH);
+   instance = usbg_get_function_instance(f);
type = usbg_get_function_type(f);
-   fprintf(stdout, %s - %s %s\n, buf,
+   if (!bname || !instance) {
+   fprintf(stderr, Unable to get binding details\n);
+   return;
+   }
+   fprintf(stdout, %s - %s %s\n, bname,
usbg_get_function_type_str(type), instance);
}
 }
diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index fe0e019..3f5d561 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -255,6 +255,16 @@ extern int usbg_init(const char *configfs_path, usbg_state 
**state);
 extern void usbg_cleanup(usbg_state *s);
 
 /**
+ * @brief Get ConfigFS path
+ * @param s Pointer to state
+ * @return Path to configfs or NULL if error occurred
+ * @warning Returned buffer should not be edited!
+ * Returned string is valid as long as passed usbg_state is valid.
+ * For example path is valid unitill usbg_cleanup() call.
+ */
+extern const char *usbg_get_configfs_path(usbg_state *s);
+
+/**
  * @brief Get ConfigFS path length
  * @param s Pointer to state
  * @return Length of path or usbg_error if error occurred.
@@ -262,13 +272,13 @@ extern void usbg_cleanup(usbg_state *s);
 extern size_t usbg_get_configfs_path_len(usbg_state *s);
 
 /**
- * @brief 

[PATCH 00/12] libusbg: Fixes and small API improvements

2014-09-26 Thread Krzysztof Opasiak
Dear Matt,

This series fixes a lot of bugs and makes API usage easier.

First two commits contains contains a difference between
libubsg/libusbg and kopasiak/libusbg repository.

List of fixed issues:
- doxygen typos
- Off-by-one error in usbg_get_function_type_str()
- remove unused variables
- Always append '\0' at the end of string

Also some small API changes has been done. Now user can choose
if he would like to get name as a constant string or copy the
name to his buffer. Of course name is valid only as long as 
related entity exist.

I have also added usbg_get/set_gadget_attr() to allow set
selected gadget attribute instead of using dedicated function
for each of them. It makes iteration over attributes much simpler.

I have created also a pull request:

https://github.com/libusbg/libusbg/pull/11

Moreover, I would like to suggest you to not use github merge
button. This button makes allways non-fast-forrward merge
so it generates always a merge commit. We have only 6 pull requests
merged till now and the history looks a litlle bit weird.
When we get more and more commist the history will get unnecessarily
complicated. In additions a lot of merge commits in upstream branch
causes some problems in gbs (used to build rpms in tizen).
Of course this is not a must but only a suggestion.

--
BR's
Krzysztof Opasiak
Samsung RD Institute Poland
Samsung Electronics

Krzysztof Opasiak (11):
  libusbg: fix: reverse comparsion to fix error
  libusbg: doc: Fix typos in doxygen documentation
  libusbg: Fix off-by-one error.
  libusbg: fix: Remove unused variables
  libusbg: Always add '\0' at end of string
  libusbg: Simplify getting names form library structures
  libusbg: Allow to set all gadget attrs using one function.
  libusbg: Return suitable error codes instead of -1
  libusbg: Make usbg_lookup_function_type() publicly available
  libusbg: Add own name to AUTHORS file.
  libusbg: Don't print errno when it has not been directly set

Philippe De Swert (1):
  libusbg: Return USBG_ERROR_PATH_TOO_LONG error when it happens

 AUTHORS |2 +
 examples/show-gadgets.c |   42 ++---
 include/usbg/usbg.h |  164 +--
 src/usbg.c  |  218 +--
 4 files changed, 322 insertions(+), 104 deletions(-)

-- 
1.7.9.5

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


[PATCH 01/12] libusbg: Return USBG_ERROR_PATH_TOO_LONG error when it happens

2014-09-26 Thread Krzysztof Opasiak
From: Philippe De Swert philippe.desw...@jollamobile.com

Signed-off-by: Philippe De Swert philippe.desw...@jollamobile.com
Acked-by: Krzysztof Opasiak k.opas...@samsung.com
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com

Update commit message.

Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 src/usbg.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/usbg.c b/src/usbg.c
index 1703ff1..bbba689 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -2023,7 +2023,7 @@ int usbg_create_config(usbg_gadget *g, int id, const char 
*label,
usbg_config_attrs *c_attrs, usbg_config_strs *c_strs, 
usbg_config **c)
 {
char cpath[USBG_MAX_PATH_LENGTH];
-   usbg_config *conf;
+   usbg_config *conf = NULL;
int ret = USBG_ERROR_INVALID_PARAM;
int n, free_space;
 
@@ -2060,6 +2060,8 @@ int usbg_create_config(usbg_gadget *g, int id, const char 
*label,
n = snprintf((cpath[n]), free_space, /%s, (*c)-name);
if (n  free_space) {
ret = USBG_ERROR_PATH_TOO_LONG;
+   usbg_free_config(conf);
+   goto out;
}
 
ret = mkdir(cpath, S_IRWXU | S_IRWXG | S_IRWXO);
-- 
1.7.9.5

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


[PATCH 12/12] libusbg: Don't print errno when it has not been directly set

2014-09-26 Thread Krzysztof Opasiak
Replace ERRORNO() macro with ERROR(). Difference betwen
them is that ERRORNO() prints also standard error string
based on errno value. This variable should not be accessed
when called function doesn't directly set it.

Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 src/usbg.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/usbg.c b/src/usbg.c
index 6e0ce6f..7640dda 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -1291,7 +1291,7 @@ static int usbg_init_state(char *path, usbg_state *s)
 
ret = usbg_parse_gadgets(path, s);
if (ret != USBG_SUCCESS)
-   ERRORNO(unable to parse %s\n, path);
+   ERROR(unable to parse %s\n, path);
 
return ret;
 }
@@ -1330,7 +1330,7 @@ int usbg_init(const char *configfs_path, usbg_state 
**state)
 
ret = usbg_init_state(path, s);
if (ret != USBG_SUCCESS) {
-   ERRORNO(couldn't init gadget state\n);
+   ERROR(couldn't init gadget state\n);
usbg_free_state(s);
goto out;
}
@@ -2070,7 +2070,7 @@ int usbg_create_function(usbg_gadget *g, 
usbg_function_type type,
*f = usbg_allocate_function(fpath, type, instance, g);
func = *f;
if (!func) {
-   ERRORNO(allocating function\n);
+   ERROR(allocating function\n);
ret = USBG_ERROR_NO_MEM;
goto out;
}
@@ -2130,7 +2130,7 @@ int usbg_create_config(usbg_gadget *g, int id, const char 
*label,
*c = usbg_allocate_config(cpath, label, id, g);
conf = *c;
if (!conf) {
-   ERRORNO(allocating configuration\n);
+   ERROR(allocating configuration\n);
ret = USBG_ERROR_NO_MEM;
goto out;
}
-- 
1.7.9.5

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


[PATCH 05/12] libusbg: fix: Remove unused variables

2014-09-26 Thread Krzysztof Opasiak
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 src/usbg.c |   13 ++---
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/src/usbg.c b/src/usbg.c
index f12a4b3..9c9b51f 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -3125,7 +3125,6 @@ static int usbg_export_gadget_prep(usbg_gadget *g, 
config_setting_t *root)
config_setting_t *node;
int ret = USBG_ERROR_NO_MEM;
int usbg_ret;
-   int cfg_ret;
 
/* We don't export name tag because name should be given during
 * loading of gadget */
@@ -3559,7 +3558,6 @@ out:
 static int usbg_import_config_bindings(config_setting_t *root, usbg_config *c)
 {
config_setting_t *node;
-   int usbg_ret, cfg_ret;
int ret = USBG_SUCCESS;
int count, i;
 
@@ -3588,7 +3586,6 @@ static int usbg_import_config_strs_lang(config_setting_t 
*root, usbg_config *c)
int lang;
const char *str;
usbg_config_strs c_strs = {0};
-   int usbg_ret, cfg_ret;
int ret = USBG_ERROR_INVALID_TYPE;
 
node = config_setting_get_member(root, USBG_LANG_TAG);
@@ -3624,7 +3621,6 @@ out:
 static int usbg_import_config_strings(config_setting_t *root, usbg_config *c)
 {
config_setting_t *node;
-   int usbg_ret, cfg_ret;
int ret = USBG_SUCCESS;
int count, i;
 
@@ -3648,9 +3644,8 @@ static int usbg_import_config_strings(config_setting_t 
*root, usbg_config *c)
 static int usbg_import_config_attrs(config_setting_t *root, usbg_config *c)
 {
config_setting_t *node;
-   int usbg_ret, cfg_ret;
+   int usbg_ret;
int bmAttributes, bMaxPower;
-   short format;
int ret = USBG_ERROR_INVALID_TYPE;
 
node = config_setting_get_member(root, bmAttributes);
@@ -3773,7 +3768,6 @@ error2:
 static int usbg_import_gadget_configs(config_setting_t *root, usbg_gadget *g)
 {
config_setting_t *node, *id_node;
-   int usbg_ret, cfg_ret;
int id;
usbg_config *c;
int ret = USBG_SUCCESS;
@@ -3818,7 +3812,6 @@ static int usbg_import_gadget_configs(config_setting_t 
*root, usbg_gadget *g)
 static int usbg_import_gadget_functions(config_setting_t *root, usbg_gadget *g)
 {
config_setting_t *node, *inst_node;
-   int usbg_ret, cfg_ret;
const char *instance;
const char *label;
usbg_function *f;
@@ -3884,7 +3877,6 @@ static int usbg_import_gadget_strs_lang(config_setting_t 
*root, usbg_gadget *g)
int lang;
const char *str;
usbg_gadget_strs g_strs = {0};
-   int usbg_ret, cfg_ret;
int ret = USBG_ERROR_INVALID_TYPE;
 
node = config_setting_get_member(root, USBG_LANG_TAG);
@@ -3926,7 +3918,6 @@ out:
 static int usbg_import_gadget_strings(config_setting_t *root, usbg_gadget *g)
 {
config_setting_t *node;
-   int usbg_ret, cfg_ret;
int ret = USBG_SUCCESS;
int count, i;
 
@@ -3951,7 +3942,7 @@ static int usbg_import_gadget_strings(config_setting_t 
*root, usbg_gadget *g)
 static int usbg_import_gadget_attrs(config_setting_t *root, usbg_gadget *g)
 {
config_setting_t *node;
-   int usbg_ret, cfg_ret;
+   int usbg_ret;
int val;
int ret = USBG_ERROR_INVALID_TYPE;
 
-- 
1.7.9.5

--
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 10/12] libusbg: Make usbg_lookup_function_type() publicly available

2014-09-26 Thread Krzysztof Opasiak
Users of library may often receive function type as a string.
Instead of implementing parsing of such string in each program
it would be more convenient to provide such support in library.

Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 include/usbg/usbg.h |7 +++
 src/usbg.c  |2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 2c191eb..eea49c8 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -658,6 +658,13 @@ extern int usbg_cpy_function_instance(usbg_function *f, 
char *buf, size_t len);
  */
 extern const char *usbg_get_function_type_str(usbg_function_type type);
 
+/**
+ * @brief Lookup function type sutable for given name
+ * @param name Name of function
+ * @return Function type enum or negative error code
+ */
+extern int usbg_lookup_function_type(const char *name);
+
 /* USB configurations allocation and configuration */
 
 /**
diff --git a/src/usbg.c b/src/usbg.c
index 4d74a0d..6e0ce6f 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -310,7 +310,7 @@ const char *usbg_strerror(usbg_error e)
return ret;
 }
 
-static int usbg_lookup_function_type(const char *name)
+int usbg_lookup_function_type(const char *name)
 {
int i = 0;
int max = sizeof(function_names)/sizeof(char *);
-- 
1.7.9.5

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


[PATCH 03/12] libusbg: doc: Fix typos in doxygen documentation

2014-09-26 Thread Krzysztof Opasiak
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 include/usbg/usbg.h |   40 +++-
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index a934a7b..fe0e019 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -224,14 +224,14 @@ typedef enum  {
USBG_ERROR_OTHER_ERROR = -99
 } usbg_error;
 
-/*
+/**
  * @brief Get the error name as a constant string
  * @param e error code
  * @return Constant string with error name
  */
 extern const char *usbg_error_name(usbg_error e);
 
-/*
+/**
  * @brief Get the short description of error
  * @param e error code
  * @return Constant string with error description
@@ -243,7 +243,7 @@ extern const char *usbg_strerror(usbg_error e);
 /**
  * @brief Initialize the libusbg library state
  * @param configfs_path Path to the mounted configfs filesystem
- * @param Pointer to be filled with pointer to usbg_state
+ * @param state Pointer to be filled with pointer to usbg_state
  * @return 0 on success, usbg_error on error
  */
 extern int usbg_init(const char *configfs_path, usbg_state **state);
@@ -262,7 +262,7 @@ extern void usbg_cleanup(usbg_state *s);
 extern size_t usbg_get_configfs_path_len(usbg_state *s);
 
 /**
- * @brieg Get ConfigFS path
+ * @brief Get ConfigFS path
  * @param s Pointer to state
  * @param buf Buffer where path should be copied
  * @param len Length of given buffer
@@ -409,8 +409,8 @@ extern int usbg_get_gadget_attrs(usbg_gadget *g, 
usbg_gadget_attrs *g_attrs);
 extern size_t usbg_get_gadget_name_len(usbg_gadget *g);
 
 /**
- * @brieg Get gadget name
- * @param b Pointer to gadget
+ * @brief Get gadget name
+ * @param g Pointer to gadget
  * @param buf Buffer where name should be copied
  * @param len Length of given buffer
  * @return 0 on success or usbg_error if error occurred.
@@ -490,7 +490,7 @@ extern int usbg_set_gadget_device_bcd_usb(usbg_gadget *g, 
uint16_t bcdUSB);
  * @brief Get the USB gadget strings
  * @param g Pointer to gadget
  * @param lang Language of strings
- * @param g_sttrs Structure to be filled
+ * @param g_strs Structure to be filled
  * @return 0 on success usbg_error if error occurred
  */
 extern int usbg_get_gadget_strs(usbg_gadget *g, int lang,
@@ -500,7 +500,7 @@ extern int usbg_get_gadget_strs(usbg_gadget *g, int lang,
  * @brief Set the USB gadget strings
  * @param g Pointer to gadget
  * @param lang USB language ID
- * @param g_sttrs Gadget attributes
+ * @param g_strs Gadget attributes
  * @return 0 on success usbg_error if error occurred
  */
 extern int usbg_set_gadget_strs(usbg_gadget *g, int lang,
@@ -599,7 +599,7 @@ extern int usbg_create_config(usbg_gadget *g, int id, const 
char *label,
 extern size_t usbg_get_config_label_len(usbg_config *c);
 
 /**
- * @brieg Get config label
+ * @brief Get config label
  * @param c Pointer to config
  * @param buf Buffer where label should be copied
  * @param len Length of given buffer
@@ -608,7 +608,7 @@ extern size_t usbg_get_config_label_len(usbg_config *c);
 extern int usbg_get_config_label(usbg_config *c, char *buf, size_t len);
 
 /**
- * @brieg Get config id
+ * @brief Get config id
  * @param c Pointer to config
  * @return Configuration id or usbg_error if error occurred.
  */
@@ -651,7 +651,7 @@ extern int usbg_set_config_bm_attrs(usbg_config *c, int 
bmAttributes);
  * @brief Get the USB configuration strings
  * @param c Pointer to configuration
  * @param lang Language of strings
- * @param c_sttrs Structure to be filled
+ * @param c_strs Structure to be filled
  * @return 0 on success or usbg_error if error occurred.
  */
 extern int usbg_get_config_strs(usbg_config *c, int lang,
@@ -661,7 +661,7 @@ extern int usbg_get_config_strs(usbg_config *c, int lang,
  * @brief Set the USB configuration strings
  * @param c Pointer to configuration
  * @param lang USB language ID
- * @param c_sttrs Configuration strings
+ * @param c_strs Configuration strings
  * @return 0 on success, usbg_error on failure.
  */
 extern int usbg_set_config_strs(usbg_config *c, int lang,
@@ -742,8 +742,8 @@ extern int usbg_disable_gadget(usbg_gadget *g);
 extern size_t usbg_get_gadget_udc_len(usbg_gadget *g);
 
 /**
- * @brieg Get name of udc to which gadget is binded
- * @param b Pointer to gadget
+ * @brief Get name of udc to which gadget is binded
+ * @param g Pointer to gadget
  * @param buf Buffer where udc name should be copied
  * @param len Length of given buffer
  * @return 0 on success or usbg_error if error occurred.
@@ -866,7 +866,7 @@ extern usbg_config *usbg_get_first_config(usbg_gadget *g);
 
 /**
  * @brief Get first binding in binding list
- * @param C Pointer to configuration
+ * @param c Pointer to configuration
  * @return Pointer to binding or NULL if list is empty.
  * @note Bindings are sorted in strings (name) order
  */
@@ -874,28 +874,28 @@ extern usbg_binding *usbg_get_first_binding(usbg_config 
*c);
 
 /**
  * @brief Get the next gadget on a 

[PATCH 08/12] libusbg: Allow to set all gadget attrs using one function.

2014-09-26 Thread Krzysztof Opasiak
Having specialized functions for each attribute may be
sometime inconvenient to use. Provide also function
whihc allows to set attribute selected by parameter.

Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 include/usbg/usbg.h |   53 +
 src/usbg.c  |   73 +++
 2 files changed, 126 insertions(+)

diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 3f5d561..2c191eb 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -88,6 +88,22 @@ typedef struct usbg_function usbg_function;
 typedef struct usbg_binding usbg_binding;
 
 /**
+ * @typedef usbg_gadget_attr
+ * @brief Gadget attributes which can be set using
+ * usbg_set_gadget_attr() function.
+ */
+typedef enum {
+   BCD_USB = 0,
+   B_DEVICE_CLASS,
+   B_DEVICE_SUB_CLASS,
+   B_DEVICE_PROTOCOL,
+   B_MAX_PACKET_SIZE_0,
+   ID_VENDOR,
+   ID_PRODUCT,
+   BCD_DEVICE,
+} usbg_gadget_attr;
+
+/**
  * @typedef usbg_gadget_attrs
  * @brief USB gadget device attributes
  */
@@ -395,6 +411,43 @@ extern int usbg_create_gadget(usbg_state *s, const char 
*name,
  usbg_gadget **g);
 
 /**
+ * @brief Get string representing selected gadget attribute
+ * @param attr code of selected attrobute
+ * @return String suitable for given attribute or NULL if such
+ * string has not been found
+ */
+extern const char *usbg_get_gadget_attr_str(usbg_gadget_attr attr);
+
+/**
+ * @brief Lookup attr code based on its name
+ * @param name of attribute
+ * @return code of suitable attribute or usbg_error
+ */
+int usbg_lookup_gadget_attr(const char *name);
+
+/**
+ * @brief Set selected attribute to value
+ * @param g Pointer to gadget
+ * @param attr Code of selected attribute
+ * @param val value to be set
+ * @return 0 on success, usbg_error otherwise
+ * @note val is of type int but value provided to this function should
+ * be suitable to place it in type dedicated for selected attr (uint16 or 
uint8)
+ */
+extern int usbg_set_gadget_attr(usbg_gadget *g, usbg_gadget_attr attr, int 
val);
+
+/**
+ * @brief Get value of selected attribute
+ * @param g Pointer to gadget
+ * @param attr Code of selected attribute
+ * @return Value of selected attribute (always above zero) or
+ * usbg_error if error occurred.
+ * @note User should use only lowest one or two bytes as attribute value
+ * depending on attribute size (see usbg_gadget_attrs for details).
+ */
+extern int usbg_get_gadget_attr(usbg_gadget *g, usbg_gadget_attr attr);
+
+/**
  * @brief Set the USB gadget attributes
  * @param g Pointer to gadget
  * @param g_attrs Gadget attributes
diff --git a/src/usbg.c b/src/usbg.c
index bd63790..cd2179e 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -111,6 +111,18 @@ const char *function_names[] =
ffs,
 };
 
+const char *gadget_attr_names[] =
+{
+   bcdUSB,
+   bDeviceClass,
+   bDeviceSubClass,
+   bDeviceProtocol,
+   bMaxPacketSize0,
+   idVendor,
+   idProduct,
+   bcdDevice
+};
+
 #define ERROR(msg, ...) do {\
 fprintf(stderr, %s()  msg \n, \
 __func__, ##__VA_ARGS__);\
@@ -158,6 +170,7 @@ static int usbg_translate_error(int error)
case ENOTDIR:
ret = USBG_ERROR_NOT_FOUND;
break;
+   case ERANGE:
case EINVAL:
case USBG_ERROR_INVALID_PARAM:
ret = USBG_ERROR_INVALID_PARAM;
@@ -323,6 +336,29 @@ const const char 
*usbg_get_function_type_str(usbg_function_type type)
function_names[type] : NULL;
 }
 
+int usbg_lookup_gadget_attr(const char *name)
+{
+   int i = 0;
+   int max = sizeof(gadget_attr_names)/sizeof(char *);
+
+   if (!name)
+   return USBG_ERROR_INVALID_PARAM;
+
+   do {
+   if (!strcmp(name, gadget_attr_names[i]))
+   return i;
+   i++;
+   } while (i != max);
+
+   return USBG_ERROR_NOT_FOUND;
+}
+
+const const char *usbg_get_gadget_attr_str(usbg_gadget_attr attr)
+{
+   return attr = 0  attr  sizeof(gadget_attr_names)/sizeof(char *) ?
+   gadget_attr_names[attr] : NULL;
+}
+
 static usbg_error usbg_split_function_instance_type(const char *full_name,
usbg_function_type *f_type, const char **instance)
 {
@@ -519,6 +555,7 @@ static int usbg_write_int(const char *path, const char 
*name, const char *file,
 }
 
 #define usbg_write_dec(p, n, f, v) usbg_write_int(p, n, f, v, %d\n)
+#define usbg_write_hex(p, n, f, v) usbg_write_int(p, n, f, v, 0x%x\n)
 #define usbg_write_hex16(p, n, f, v)   usbg_write_int(p, n, f, v, 0x%04x\n)
 #define usbg_write_hex8(p, n, f, v)usbg_write_int(p, n, f, v, 0x%02x\n)
 
@@ -1746,6 +1783,42 @@ int usbg_get_gadget_udc(usbg_gadget *g, char *buf, 
size_t len)
return USBG_SUCCESS;
 }
 
+int usbg_set_gadget_attr(usbg_gadget *g, usbg_gadget_attr attr, 

[PATCH 06/12] libusbg: Always add '\0' at end of string

2014-09-26 Thread Krzysztof Opasiak
strncpy() may not append trailing '\0' so let's append
it always at end of string to avoid getting into troubles.

Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 src/usbg.c |   72 ++--
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/src/usbg.c b/src/usbg.c
index 9c9b51f..4f476eb 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -1323,13 +1323,13 @@ size_t usbg_get_configfs_path_len(usbg_state *s)
 
 int usbg_get_configfs_path(usbg_state *s, char *buf, size_t len)
 {
-   int ret = USBG_SUCCESS;
-   if (s  buf)
-   strncpy(buf, s-path, len);
-   else
-   ret = USBG_ERROR_INVALID_PARAM;
+   if (!s || !buf || len == 0)
+   return USBG_ERROR_INVALID_PARAM;
 
-   return ret;
+   buf[--len] = '\0';
+   strncpy(buf, s-path, len);
+
+   return USBG_SUCCESS;
 }
 
 usbg_gadget *usbg_get_gadget(usbg_state *s, const char *name)
@@ -1711,13 +1711,13 @@ size_t usbg_get_gadget_name_len(usbg_gadget *g)
 
 int usbg_get_gadget_name(usbg_gadget *g, char *buf, size_t len)
 {
-   int ret = USBG_SUCCESS;
-   if (g  buf)
-   strncpy(buf, g-name, len);
-   else
-   ret = USBG_ERROR_INVALID_PARAM;
+   if (!g || !buf || len == 0)
+   return USBG_ERROR_INVALID_PARAM;
 
-   return ret;
+   buf[--len] = '\0';
+   strncpy(buf, g-name, len);
+
+   return USBG_SUCCESS;
 }
 
 size_t usbg_get_gadget_udc_len(usbg_gadget *g)
@@ -1727,13 +1727,13 @@ size_t usbg_get_gadget_udc_len(usbg_gadget *g)
 
 int usbg_get_gadget_udc(usbg_gadget *g, char *buf, size_t len)
 {
-   int ret = USBG_SUCCESS;
-   if (g  buf)
-   strncpy(buf, g-udc, len);
-   else
-   ret = USBG_ERROR_INVALID_PARAM;
+   if (!g || !buf || len == 0)
+   return USBG_ERROR_INVALID_PARAM;
 
-   return ret;
+   buf[--len] = '\0';
+   strncpy(buf, g-udc, len);
+
+   return USBG_SUCCESS;
 }
 
 int usbg_set_gadget_attrs(usbg_gadget *g, usbg_gadget_attrs *g_attrs)
@@ -2094,13 +2094,13 @@ size_t usbg_get_config_label_len(usbg_config *c)
 
 int usbg_get_config_label(usbg_config *c, char *buf, size_t len)
 {
-   int ret = USBG_SUCCESS;
-   if (c  buf)
-   strncpy(buf, c-label, len);
-   else
-   ret = USBG_ERROR_INVALID_PARAM;
+   if (!c || !buf || len == 0)
+   return USBG_ERROR_INVALID_PARAM;
 
-   return ret;
+   buf[--len] = '\0';
+   strncpy(buf, c-label, len);
+
+   return USBG_SUCCESS;
 }
 
 int usbg_get_config_id(usbg_config *c)
@@ -2115,13 +2115,13 @@ size_t usbg_get_function_instance_len(usbg_function *f)
 
 int usbg_get_function_instance(usbg_function *f, char *buf, size_t len)
 {
-   int ret = USBG_SUCCESS;
-   if (f  buf)
-   strncpy(buf, f-instance, len);
-   else
-   ret = USBG_ERROR_INVALID_PARAM;
+   if (!f || !buf || len == 0)
+   return USBG_ERROR_INVALID_PARAM;
 
-   return ret;
+   buf[--len] = '\0';
+   strncpy(buf, f-instance, len);
+
+   return USBG_SUCCESS;
 }
 
 int usbg_set_config_attrs(usbg_config *c, usbg_config_attrs *c_attrs)
@@ -2274,13 +2274,13 @@ size_t usbg_get_binding_name_len(usbg_binding *b)
 
 int usbg_get_binding_name(usbg_binding *b, char *buf, size_t len)
 {
-   int ret = USBG_SUCCESS;
-   if (b  buf)
-   strncpy(buf, b-name, len);
-   else
-   ret = USBG_ERROR_INVALID_PARAM;
+   if (!b || !buf || len == 0)
+   return USBG_ERROR_INVALID_PARAM;
 
-   return ret;
+   buf[--len] = '\0';
+   strncpy(buf, b-name, len);
+
+   return USBG_SUCCESS;
 }
 
 int usbg_get_udcs(struct dirent ***udc_list)
-- 
1.7.9.5

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


[PATCH 09/12] libusbg: Return suitable error codes instead of -1

2014-09-26 Thread Krzysztof Opasiak
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 src/usbg.c |9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/usbg.c b/src/usbg.c
index cd2179e..4d74a0d 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -316,18 +316,15 @@ static int usbg_lookup_function_type(const char *name)
int max = sizeof(function_names)/sizeof(char *);
 
if (!name)
-   return -1;
+   return USBG_ERROR_INVALID_PARAM;
 
do {
if (!strcmp(name, function_names[i]))
-   break;
+   return i;
i++;
} while (i != max);
 
-   if (i == max)
-   i = -1;
-
-   return i;
+   return USBG_ERROR_NOT_FOUND;
 }
 
 const const char *usbg_get_function_type_str(usbg_function_type type)
-- 
1.7.9.5

--
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 0/6] libusbg: Add usbg_udc structure

2014-09-26 Thread Krzysztof Opasiak
Dear Matt,

This series adds usbg_udc structure to remove direct udc name usage.

Before this series UDC didn't have its own structure. Gadget enable
took const char * as parameter. Such approach is inconsisten with rest
of API.

This series introduce structure usbg_udc as abstraction of UDC.  This
makes API more consistent and some functions (like enable gadget) are
now shorter and easier to read. Moreover usage of such structure
allows to implement usbg_get_udc_gadget() which provides possibility
to check which gadget is enabled on selected UDC.

I'm looking froward for your review and merge if you find it
suitable. I have also created a github pull request:

https://github.com/libusbg/libusbg/pull/10

Please notice that this series depends on previous one with
fixes:

https://github.com/libusbg/libusbg/pull/11

--
BR's
Krzysztof Opasiak
Samsung RD Institute Poland
Samsung Electronics
k.opas...@samsung.com

---
Chages since v1:
- Move fixes to separate series
- Fix error reported by Philippe De Swert in github pull request

@David Laight
libusbg: Always add '\0' at end of string
has been fixed as you suggested but it has been moved
to separate series along with other fixes

Krzysztof Opasiak (6):
  libusbg: Add udc structure
  libusbg: Replace usbg_get_udcs() with loop
  libusbg: Rework API to use usbg_udc structure
  libusbg: Refresh gadget UDC while each usbg_get_gadget_udc()
  libusbg: Allow to get gadget enabled on given UDC
  libusbg: Add example how to use usbg_udc structure

 examples/Makefile.am |3 +-
 examples/gadget-vid-pid-remove.c |   12 +-
 examples/show-gadgets.c  |   15 +-
 examples/show-udcs.c |   61 
 include/usbg/usbg.h  |   81 +--
 src/usbg.c   |  288 ++
 6 files changed, 374 insertions(+), 86 deletions(-)
 create mode 100644 examples/show-udcs.c

-- 
1.7.9.5

--
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 6/6] libusbg: Add example how to use usbg_udc structure

2014-09-26 Thread Krzysztof Opasiak
This example shows how to learn what udcs are available
in system and also how to find out what gadgets are
enabled on them.

Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 examples/Makefile.am |3 ++-
 examples/show-udcs.c |   61 ++
 2 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 examples/show-udcs.c

diff --git a/examples/Makefile.am b/examples/Makefile.am
index a77d890..d7a3b48 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -1,9 +1,10 @@
-bin_PROGRAMS = show-gadgets gadget-acm-ecm gadget-vid-pid-remove gadget-ffs 
gadget-export gadget-import
+bin_PROGRAMS = show-gadgets gadget-acm-ecm gadget-vid-pid-remove gadget-ffs 
gadget-export gadget-import show-udcs
 gadget_acm_ecm_SOURCES = gadget-acm-ecm.c
 show_gadgets_SOURCES = show-gadgets.c
 gadget_vid_pid_remove_SOURCES = gadget-vid-pid-remove.c
 gadget_ffs_SOURCES = gadget-ffs.c
 gadget_export_SOURCE = gadget-export.c
 gadget_import_SOURCE = gadget-import.c
+show_udcs_SOURCE = show-udcs.c
 AM_CPPFLAGS=-I$(top_srcdir)/include/
 AM_LDFLAGS=-L../src/ -lusbg
diff --git a/examples/show-udcs.c b/examples/show-udcs.c
new file mode 100644
index 000..66e950f
--- /dev/null
+++ b/examples/show-udcs.c
@@ -0,0 +1,61 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ *
+ * Krzysztof Opasiak k.opas...@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+/**
+ * @file show-udcs.c
+ * @example show-udcs.c
+ * This is an example of how to learn about UDCs available in system
+ * and find out what gadget are enabled on them.
+ */
+
+#include errno.h
+#include stdio.h
+#include usbg/usbg.h
+
+int main(void)
+{
+   int usbg_ret;
+   int ret = -EINVAL;
+   usbg_state *s;
+   usbg_gadget *g;
+   usbg_udc *u;
+   const char *udc_name, *gadget_name;
+
+   usbg_ret = usbg_init(/sys/kernel/config, s);
+   if (usbg_ret != USBG_SUCCESS) {
+   fprintf(stderr, Error on USB state init\n);
+   fprintf(stderr, Error: %s : %s\n, usbg_error_name(usbg_ret),
+   usbg_strerror(usbg_ret));
+   goto out;
+   }
+
+   usbg_for_each_udc(u, s) {
+   udc_name = usbg_get_udc_name(u);
+   g = usbg_get_udc_gadget(u);
+   if (g)
+   /* some gadget is enabled */
+   gadget_name = usbg_get_gadget_name(g);
+   else
+   gadget_name = ;
+
+   fprintf(stdout, %s - %s\n, udc_name, gadget_name);
+   }
+
+   ret = 0;
+   usbg_cleanup(s);
+out:
+   return ret;
+}
-- 
1.7.9.5

--
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/6] libusbg: Replace usbg_get_udcs() with loop

2014-09-26 Thread Krzysztof Opasiak
Library allows to iterate over each gadget using loop.
To be consistent with this convention add similar loop
for UDCs.

Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 include/usbg/usbg.h |   31 ---
 src/usbg.c  |   49 +++--
 2 files changed, 43 insertions(+), 37 deletions(-)

diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 3e11dc5..86a49e8 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -836,13 +836,6 @@ extern int usbg_cpy_binding_name(usbg_binding *b, char 
*buf, size_t len);
 /* USB gadget setup and teardown */
 
 /**
- * @brief Get a list of UDC devices on the system
- * @param udc_list Pointer to pointer to dirent pointer
- * @return Number of UDC devices on success, usbg_error on failure
- */
-extern int usbg_get_udcs(struct dirent ***udc_list);
-
-/**
  * @brief Enable a USB gadget device
  * @param g Pointer to gadget
  * @param udc Name of UDC to enable gadget
@@ -965,6 +958,15 @@ extern int usbg_set_net_qmult(usbg_function *f, int qmult);
b = usbg_get_next_binding(b))
 
 /**
+ * @def usbg_for_each_udc(b, c)
+ * Iterates over each udc
+ */
+#define usbg_for_each_udc(u, s)\
+   for (u = usbg_get_first_udc(s); \
+   u != NULL; \
+   u = usbg_get_next_udc(u))
+
+/**
  * @brief Get first gadget in gadget list
  * @param s State of library
  * @return Pointer to gadget or NULL if list is empty.
@@ -997,6 +999,14 @@ extern usbg_config *usbg_get_first_config(usbg_gadget *g);
 extern usbg_binding *usbg_get_first_binding(usbg_config *c);
 
 /**
+ * @brief Get first udc in udc list
+ * @param s State of library
+ * @return Pointer to udc or NULL if list is empty.
+ * @note UDCs are sorted in strings (name) order
+ */
+extern usbg_udc *usbg_get_first_udc(usbg_state *s);
+
+/**
  * @brief Get the next gadget on a list.
  * @param g Pointer to current gadget
  * @return Next gadget or NULL if end of list.
@@ -1024,6 +1034,13 @@ extern usbg_config *usbg_get_next_config(usbg_config *c);
  */
 extern usbg_binding *usbg_get_next_binding(usbg_binding *b);
 
+/**
+ * @brief Get the next udc on a list.
+ * @param u Pointer to current udc
+ * @return Next udc or NULL if end of list.
+ */
+extern usbg_udc *usbg_get_next_udc(usbg_udc *u);
+
 /* Import / Export API */
 
 /**
diff --git a/src/usbg.c b/src/usbg.c
index 1849de4..8c3e2fe 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -2472,49 +2472,28 @@ int usbg_cpy_binding_name(usbg_binding *b, char *buf, 
size_t len)
return USBG_SUCCESS;
 }
 
-int usbg_get_udcs(struct dirent ***udc_list)
-{
-   int ret = USBG_ERROR_INVALID_PARAM;
-
-   if (udc_list) {
-   ret = scandir(/sys/class/udc, udc_list, file_select, 
alphasort);
-   if (ret  0)
-   ret = usbg_translate_error(errno);
-   }
-
-   return ret;
-}
-
 int usbg_enable_gadget(usbg_gadget *g, const char *udc)
 {
-   char gudc[USBG_MAX_STR_LENGTH];
-   struct dirent **udc_list;
-   int i;
+   usbg_udc *sudc;
int ret = USBG_ERROR_INVALID_PARAM;
 
if (!g)
return ret;
 
if (!udc) {
-   ret = usbg_get_udcs(udc_list);
-   if (ret = 0) {
-   /* Look for default one - first in string order */
-   strcpy(gudc, udc_list[0]-d_name);
-   udc = gudc;
-
-   /** Free the memory */
-   for (i = 0; i  ret; ++i)
-   free(udc_list[i]);
-   free(udc_list);
-   } else {
+   sudc = usbg_get_first_udc(g-parent);
+   if (sudc)
+   udc = sudc-name;
+   else
return ret;
-   }
}
 
ret = usbg_write_string(g-path, g-name, UDC, udc);
 
-   if (ret == USBG_SUCCESS)
-   strcpy(g-udc, udc);
+   if (ret == USBG_SUCCESS) {
+   strncpy(g-udc, udc, USBG_MAX_STR_LENGTH);
+   g-udc[USBG_MAX_STR_LENGTH - 1] = '\0';
+   }
 
return ret;
 }
@@ -2673,6 +2652,11 @@ usbg_binding *usbg_get_first_binding(usbg_config *c)
return c ? TAILQ_FIRST(c-bindings) : NULL;
 }
 
+usbg_udc *usbg_get_first_udc(usbg_state *s)
+{
+   return s ? TAILQ_FIRST(s-udcs) : NULL;
+}
+
 usbg_gadget *usbg_get_next_gadget(usbg_gadget *g)
 {
return g ? TAILQ_NEXT(g, gnode) : NULL;
@@ -2693,6 +2677,11 @@ usbg_binding *usbg_get_next_binding(usbg_binding *b)
return b ? TAILQ_NEXT(b, bnode) : NULL;
 }
 
+usbg_udc *usbg_get_next_udc(usbg_udc *u)
+{
+   return u ? TAILQ_NEXT(u, unode) : NULL;
+}
+
 #define USBG_NAME_TAG name
 #define USBG_ATTRS_TAG attrs
 #define USBG_STRINGS_TAG strings
-- 
1.7.9.5

--
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  

[PATCH v2 4/6] libusbg: Refresh gadget UDC while each usbg_get_gadget_udc()

2014-09-26 Thread Krzysztof Opasiak
Kernel may decide to detach our gadget in some cases.
A good example is when FFS daemon received a SIGSEGV
and all descriptors has been closed.

To avoid returning false informations to user we have
to refresh UDC attribute before returning cached pointer
to usbg_udc.

Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 src/usbg.c |   37 +++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/usbg.c b/src/usbg.c
index 3a8e656..998bce8 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -1931,7 +1931,36 @@ out:
 
 usbg_udc *usbg_get_gadget_udc(usbg_gadget *g)
 {
-   return g ? g-udc : NULL;
+   usbg_udc *u = NULL;
+
+   if (!g)
+   goto out;
+   /*
+* if gadget was enabled we have to check if kernel
+* didn't modify the UDC file due to some errors.
+* For example some FFS daemon could just get
+* a segmentation fault or sth
+*/
+   if (g-udc) {
+   char buf[USBG_MAX_STR_LENGTH];
+   int ret;
+
+   ret = usbg_read_string(g-path, g-name, UDC, buf);
+   if (ret != USBG_SUCCESS)
+   goto out;
+
+   if (!strcmp(g-udc-name, buf)) {
+   /* Gadget is still assigned to this UDC */
+   u = g-udc;
+   } else {
+   /* Kernel decided to detach this gadget */
+   g-udc-gadget = NULL;
+   g-udc = NULL;
+   }
+   }
+
+out:
+   return u;
 }
 
 int usbg_set_gadget_attrs(usbg_gadget *g, usbg_gadget_attrs *g_attrs)
@@ -2510,8 +2539,12 @@ int usbg_enable_gadget(usbg_gadget *g, usbg_udc *udc)
}
 
ret = usbg_write_string(g-path, g-name, UDC, udc-name);
-
if (ret == USBG_SUCCESS) {
+   /* If gadget has been detached and we didn't noticed
+* it we have to clean up now.
+*/
+   if (g-udc)
+   g-udc-gadget = NULL;
g-udc = udc;
udc-gadget = g;
}
-- 
1.7.9.5

--
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 3/6] libusbg: Rework API to use usbg_udc structure

2014-09-26 Thread Krzysztof Opasiak
Using string as udc identifier provides a lot
of troubles. To be more consistent with rest of
API rework it to start using usbg_udc structure
instead of using char *.

Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 examples/gadget-vid-pid-remove.c |   12 ++---
 examples/show-gadgets.c  |   15 --
 include/usbg/usbg.h  |   29 +---
 src/usbg.c   |   96 --
 4 files changed, 98 insertions(+), 54 deletions(-)

diff --git a/examples/gadget-vid-pid-remove.c b/examples/gadget-vid-pid-remove.c
index 25e763f..c3f9c9b 100644
--- a/examples/gadget-vid-pid-remove.c
+++ b/examples/gadget-vid-pid-remove.c
@@ -31,19 +31,13 @@
 int remove_gadget(usbg_gadget *g)
 {
int usbg_ret;
-   char udc[USBG_MAX_STR_LENGTH];
+   usbg_udc *u;
 
/* Check if gadget is enabled */
-   usbg_ret = usbg_get_gadget_udc(g, udc, USBG_MAX_STR_LENGTH);
-   if (usbg_ret != USBG_SUCCESS) {
-   fprintf(stderr, Error on USB get gadget udc\n);
-   fprintf(stderr, Error: %s : %s\n, usbg_error_name(usbg_ret),
-   usbg_strerror(usbg_ret));
-   goto out;
-   }
+   u = usbg_get_gadget_udc(g);
 
/* If gadget is enable we have to disable it first */
-   if (udc[0] != '\0') {
+   if (u) {
usbg_ret = usbg_disable_gadget(g);
if (usbg_ret != USBG_SUCCESS) {
fprintf(stderr, Error on USB disable gadget udc\n);
diff --git a/examples/show-gadgets.c b/examples/show-gadgets.c
index 823188f..e5da2ce 100644
--- a/examples/show-gadgets.c
+++ b/examples/show-gadgets.c
@@ -29,8 +29,8 @@
 
 void show_gadget(usbg_gadget *g)
 {
-   char buf[USBG_MAX_STR_LENGTH];
-   const char *name;
+   const char *name, *udc;
+   usbg_udc *u;
int usbg_ret;
usbg_gadget_attrs g_attrs;
usbg_gadget_strs g_strs;
@@ -51,8 +51,15 @@ void show_gadget(usbg_gadget *g)
fprintf(stdout, ID %04x:%04x '%s'\n,
g_attrs.idVendor, g_attrs.idProduct, name);
 
-   usbg_get_gadget_udc(g, buf, USBG_MAX_STR_LENGTH);
-   fprintf(stdout,   UDC\t\t\t%s\n, buf);
+   u = usbg_get_gadget_udc(g);
+   if (u)
+   /* gadget is enabled */
+   udc = usbg_get_udc_name(u);
+   else
+   /* gadget is disabled */
+   udc = \0;
+
+   fprintf(stdout,   UDC\t\t\t%s\n, udc);
 
fprintf(stdout,   bDeviceClass\t\t0x%02x\n, g_attrs.bDeviceClass);
fprintf(stdout,   bDeviceSubClass\t0x%02x\n, g_attrs.bDeviceSubClass);
diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 86a49e8..f5d0098 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -838,10 +838,11 @@ extern int usbg_cpy_binding_name(usbg_binding *b, char 
*buf, size_t len);
 /**
  * @brief Enable a USB gadget device
  * @param g Pointer to gadget
- * @param udc Name of UDC to enable gadget
+ * @param udc where gadget should be assigned.
+ *  If NULL, default one (first) is used.
  * @return 0 on success or usbg_error if error occurred.
  */
-extern int usbg_enable_gadget(usbg_gadget *g, const char *udc);
+extern int usbg_enable_gadget(usbg_gadget *g, usbg_udc *udc);
 
 /**
  * @brief Disable a USB gadget device
@@ -851,6 +852,16 @@ extern int usbg_enable_gadget(usbg_gadget *g, const char 
*udc);
 extern int usbg_disable_gadget(usbg_gadget *g);
 
 /**
+ * @brief Get name of udc
+ * @param u Pointer to udc
+ * @return UDC name or NULL if error occurred.
+ * @warning Returned buffer should not be edited!
+ * Returned string is valid as long as passed usbg_state is valid.
+ * For example UDC name is valid until usbg_cleanup().
+ */
+extern const char *usbg_get_udc_name(usbg_udc *u);
+
+/**
  * @brief Get gadget name length
  * @param g Gadget which name length should be returned
  * @return Length of name string or usbg_error if error occurred.
@@ -859,14 +870,20 @@ extern int usbg_disable_gadget(usbg_gadget *g);
 extern size_t usbg_get_gadget_udc_len(usbg_gadget *g);
 
 /**
- * @brief Get name of udc to which gadget is binded
- * @param g Pointer to gadget
+ * @brief Copy name of udc
+ * @param u Pointer to udc
  * @param buf Buffer where udc name should be copied
  * @param len Length of given buffer
  * @return 0 on success or usbg_error if error occurred.
- * @note If gadget isn't enabled on any udc returned string is empty.
  */
-extern int usbg_get_gadget_udc(usbg_gadget *g, char *buf, size_t len);
+extern int usbg_cpy_udc_name(usbg_udc *u, char *buf, size_t len);
+
+/**
+ * @brief Get udc to which gadget is binded
+ * @param g Pointer to gadget
+ * @return Pointer to UDC or NULL if gadget is not enabled
+ */
+extern usbg_udc *usbg_get_gadget_udc(usbg_gadget *g);
 
 /*
  * USB function-specific attribute configuration
diff --git a/src/usbg.c b/src/usbg.c
index 8c3e2fe..3a8e656 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -50,13 +50,13 @@ 

[PATCH v2 1/6] libusbg: Add udc structure

2014-09-26 Thread Krzysztof Opasiak
Add structure to store informations about available udcs
instead of using their names as a string.

Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 include/usbg/usbg.h |   14 
 src/usbg.c  |   94 +++
 2 files changed, 108 insertions(+)

diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index eea49c8..3e11dc5 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -61,6 +61,7 @@ struct usbg_gadget;
 struct usbg_config;
 struct usbg_function;
 struct usbg_binding;
+struct usbg_udc;
 
 /**
  * @brief State of the gadget devices in the system
@@ -88,6 +89,11 @@ typedef struct usbg_function usbg_function;
 typedef struct usbg_binding usbg_binding;
 
 /**
+ * @brief USB device controler
+ */
+typedef struct usbg_udc usbg_udc;
+
+/**
  * @typedef usbg_gadget_attr
  * @brief Gadget attributes which can be set using
  * usbg_set_gadget_attr() function.
@@ -328,6 +334,14 @@ extern usbg_function *usbg_get_function(usbg_gadget *g,
  */
 extern usbg_config *usbg_get_config(usbg_gadget *g, int id, const char *label);
 
+/**
+ * @brief Get a udc by name
+ * @param s Pointer to state
+ * @param name Name of the udc
+ * @return Pointer to udc or NULL if a matching udc isn't found
+ */
+extern usbg_udc *usbg_get_udc(usbg_state *s, const char *name);
+
 /* USB gadget/config/function/binding removal */
 
 /**
diff --git a/src/usbg.c b/src/usbg.c
index 7640dda..1849de4 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -42,6 +42,7 @@ struct usbg_state
char *path;
 
TAILQ_HEAD(ghead, usbg_gadget) gadgets;
+   TAILQ_HEAD(uhead, usbg_udc) udcs;
config_t *last_failed_import;
 };
 
@@ -93,6 +94,15 @@ struct usbg_binding
char *path;
 };
 
+struct usbg_udc
+{
+   TAILQ_ENTRY(usbg_udc) unode;
+   usbg_state *parent;
+   usbg_gadget *gadget;
+
+   char *name;
+};
+
 /**
  * @var function_names
  * @brief Name strings for supported USB function types
@@ -616,15 +626,30 @@ static void usbg_free_gadget(usbg_gadget *g)
free(g);
 }
 
+static void usbg_free_udc(usbg_udc *u)
+{
+   free(u-name);
+   free(u);
+}
+
 static void usbg_free_state(usbg_state *s)
 {
usbg_gadget *g;
+   usbg_udc *u;
+
while (!TAILQ_EMPTY(s-gadgets)) {
g = TAILQ_FIRST(s-gadgets);
TAILQ_REMOVE(s-gadgets, g, gnode);
usbg_free_gadget(g);
}
 
+
+   while (!TAILQ_EMPTY(s-udcs)) {
+   u = TAILQ_FIRST(s-udcs);
+   TAILQ_REMOVE(s-udcs, u, unode);
+   usbg_free_udc(u);
+   }
+
if (s-last_failed_import) {
config_destroy(s-last_failed_import);
free(s-last_failed_import);
@@ -758,6 +783,26 @@ static usbg_binding *usbg_allocate_binding(const char 
*path, const char *name,
return b;
 }
 
+static usbg_udc *usbg_allocate_udc(usbg_state *parent, const char *name)
+{
+   usbg_udc *u;
+
+   u = malloc(sizeof(*u));
+   if (!u)
+   goto out;
+
+   u-gadget = NULL;
+   u-parent = parent;
+   u-name = strdup(name);
+   if (!u-name) {
+   free(u);
+   u = NULL;
+   }
+
+ out:
+   return u;
+}
+
 static int ubsg_rm_file(const char *path, const char *name)
 {
int ret = USBG_SUCCESS;
@@ -1280,6 +1325,36 @@ static int usbg_parse_gadgets(const char *path, 
usbg_state *s)
return ret;
 }
 
+static int usbg_parse_udcs(usbg_state *s)
+{
+   usbg_udc *u;
+   int n, i;
+   int ret = USBG_SUCCESS;
+   struct dirent **dent;
+
+   n = scandir(/sys/class/udc, dent, file_select, alphasort);
+   if (n  0) {
+   ret = usbg_translate_error(errno);
+   goto out;
+   }
+
+   for (i = 0; i  n; ++i) {
+   if (ret == USBG_SUCCESS) {
+   u = usbg_allocate_udc(s, dent[i]-d_name);
+   if (u)
+   TAILQ_INSERT_TAIL(s-udcs, u, unode);
+   else
+   ret = USBG_ERROR_NO_MEM;
+   }
+
+   free(dent[i]);
+   }
+   free(dent);
+
+out:
+   return ret;
+}
+
 static int usbg_init_state(char *path, usbg_state *s)
 {
int ret = USBG_SUCCESS;
@@ -1288,11 +1363,19 @@ static int usbg_init_state(char *path, usbg_state *s)
s-path = path;
s-last_failed_import = NULL;
TAILQ_INIT(s-gadgets);
+   TAILQ_INIT(s-udcs);
+
+   ret = usbg_parse_udcs(s);
+   if (ret != USBG_SUCCESS) {
+   ERROR(Unable to parse udcs);
+   goto out;
+   }
 
ret = usbg_parse_gadgets(path, s);
if (ret != USBG_SUCCESS)
ERROR(unable to parse %s\n, path);
 
+out:
return ret;
 }
 
@@ -1405,6 +1488,17 @@ usbg_config *usbg_get_config(usbg_gadget *g, int id, 
const char *label)
return c;
 }
 
+usbg_udc *usbg_get_udc(usbg_state *s, const char 

[PATCH v2 5/6] libusbg: Allow to get gadget enabled on given UDC

2014-09-26 Thread Krzysztof Opasiak
Add usbg_get_udc_gadget() which allows user to learn
which gadget is attached to selected UDC.

Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 include/usbg/usbg.h |7 +++
 src/usbg.c  |   28 
 2 files changed, 35 insertions(+)

diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index f5d0098..0a1fdf5 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -885,6 +885,13 @@ extern int usbg_cpy_udc_name(usbg_udc *u, char *buf, 
size_t len);
  */
 extern usbg_udc *usbg_get_gadget_udc(usbg_gadget *g);
 
+/**
+ * @brief Get gadget which is attached to this UDC
+ * @param u Pointer to udc
+ * @return Pointer to gadget or NULL if UDC is free
+ */
+extern usbg_gadget *usbg_get_udc_gadget(usbg_udc *u);
+
 /*
  * USB function-specific attribute configuration
  */
diff --git a/src/usbg.c b/src/usbg.c
index 998bce8..488aa97 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -1963,6 +1963,34 @@ out:
return u;
 }
 
+usbg_gadget *usbg_get_udc_gadget(usbg_udc *u)
+{
+   usbg_gadget *g = NULL;
+
+   if (!u)
+   goto out;
+   /*
+* if gadget was enabled on this UDC we have to check if kernel
+* didn't modify this due to some errors.
+* For example some FFS daemon could just get a segmentation fault
+* what causes detach of gadget
+*/
+   if (u-gadget) {
+   usbg_udc *u_checked;
+
+   u_checked = usbg_get_gadget_udc(u-gadget);
+   if (u_checked) {
+   g = u-gadget;
+   } else {
+   u-gadget-udc = NULL;
+   u-gadget = NULL;
+   }
+   }
+
+out:
+   return g;
+}
+
 int usbg_set_gadget_attrs(usbg_gadget *g, usbg_gadget_attrs *g_attrs)
 {
int ret;
-- 
1.7.9.5

--
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] r8152: fix the carrier off when autoresuming

2014-09-26 Thread David Miller
From: Hayes Wang hayesw...@realtek.com
Date: Tue, 23 Sep 2014 16:31:47 +0800

 netif_carrier_off would be called when autoresuming, even though
 the cable is plugged. This causes some applications do relative
 actions when detecting the carrier off. Keep the status of the
 carrier, and let it be modified when the linking change occurs.
 
 Signed-off-by: Hayes Wang hayesw...@realtek.com

Applied, thanks.
--
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 v4 0/2] Add USB PHY device tree support for R8A7790/Lager board

2014-09-26 Thread Sergei Shtylyov
Hello.

   Here's the set of 2 patches against Simon Horman's 'renesas.git' repo,
'renesas-devel-20140924-v3.17-rc6' tag. Here we add the USB PHY device tree
support on the R8A7790/Lager reference board. The patchset requires the USB PHY
driver already merged by Kishon and Greg in order to work, so can be applied at
last...

[1/2] ARM: shmobile: r8a7790: add USB PHY DT support
[2/2] ARM: shmobile: lager: enable USB PHY

WBR, Sergei

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


RE: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL

2014-09-26 Thread Paul Zimmerman
From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-ow...@vger.kernel.org] 
On Behalf Of Felipe Balbi
Sent: Thursday, September 25, 2014 7:51 AM

 On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
  diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
  index 0fcc0a3..8277065 100644
  --- a/drivers/usb/dwc3/gadget.c
  +++ b/drivers/usb/dwc3/gadget.c
  @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, void *_dwc)
*/
   int dwc3_gadget_init(struct dwc3 *dwc)
   {
  +   u32 reg;
  int ret;
 
  dwc-ctrl_req = dma_alloc_coherent(dwc-dev, sizeof(*dwc-ctrl_req),
  @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
  if (ret)
  goto err4;
 
  +   if (dwc-quirks  DWC3_AMD_NL_PLAT) {
  +   reg = dwc3_readl(dwc-regs, DWC3_DCTL);
  +   reg |= DWC3_DCTL_LPM_ERRATA(0xf);
 
 weird, why would Synopsys put this here ? It seems like this is only
 useful when LPM Errata is enabled and that's, apparently, a host-side
 thing.
 
 Paul, can you comment ?

These bits contribute to how the device responds to an LPM transaction
from the host. If DCFG.LPMCap=1, they set the BESL value above which
the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So
it's definitely a device-side thing, but only if the core is configured
with LPM Errata support enabled.

-- 
Paul

--
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 v4 1/2] ARM: shmobile: r8a7790: add USB PHY DT support

2014-09-26 Thread Sergei Shtylyov
Define the R8A7790 generic part of the USB PHY device node. It is up to the
board file to enable the device.

Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com

---
Changes in version 4:
- refreshed the patch.

Changes in version 3:
- changed subnodes of the USB PHY node to be per-channel;
- changed renesas,phy-select property to reg in the subnodes, and thus added
  #address-cells and #size-cells properties to the parent node;
- moved #phy-cells property from the parent node to the subnodes, changing its
  value to 1;
- refreshed the patch.

Changes in version 2:
- added subnodes to the USB PHY node;
- refreshed the patch.

 arch/arm/boot/dts/r8a7790.dtsi |   19 +++
 1 file changed, 19 insertions(+)

Index: renesas/arch/arm/boot/dts/r8a7790.dtsi
===
--- renesas.orig/arch/arm/boot/dts/r8a7790.dtsi
+++ renesas/arch/arm/boot/dts/r8a7790.dtsi
@@ -600,6 +600,25 @@
status = disabled;
};
 
+   usbphy: usb-phy@e6590100 {
+   compatible = renesas,usb-phy-r8a7790;
+   reg = 0 0xe6590100 0 0x100;
+   #address-cells = 1;
+   #size-cells = 0;
+   clocks = mstp7_clks R8A7790_CLK_HSUSB;
+   clock-names = usbhs;
+   status = disabled;
+
+   usb0: usb-channel@0 {
+   reg = 0;
+   #phy-cells = 1;
+   };
+   usb2: usb-channel@2 {
+   reg = 2;
+   #phy-cells = 1;
+   };
+   };
+
clocks {
#address-cells = 2;
#size-cells = 2;

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


[PATCH v4 2/2] ARM: shmobile: lager: enable USB PHY

2014-09-26 Thread Sergei Shtylyov
Enable USB PHY device for the Lager board. 

Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com

---
Changes in version 4:
- refreshed the patch.

Changes in version 3:
- refreshed the patch.

Changes in version 2:
- refreshed the patch.

 arch/arm/boot/dts/r8a7790-lager.dts |4 
 1 file changed, 4 insertions(+)

Index: renesas/arch/arm/boot/dts/r8a7790-lager.dts
===
--- renesas.orig/arch/arm/boot/dts/r8a7790-lager.dts
+++ renesas/arch/arm/boot/dts/r8a7790-lager.dts
@@ -424,6 +424,10 @@
pinctrl-names = default;
 };
 
+usbphy {
+   status = okay;
+};
+
 /* composite video input */
 vin1 {
pinctrl-0 = vin1_pins;

--
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 v4 0/3] Add USB PHY device tree support for R8A7791/Koelsch/Henninger board

2014-09-26 Thread Sergei Shtylyov
Hello.

   Here's the set of 3 patches against Simon Horman's 'renesas.git' repo,
'renesas-devel-20140924-v3.17-rc6' tag. Here we add the USB PHY device tree
support on the R8A7791/Koelsch/Henninger boards. The patchset requires the USB
PHY driver already merged by Kishon and Greg in order to work, so can be
applied at last...

[1/3] ARM: shmobile: r8a7791: add USB PHY DT support
[2/3] ARM: shmobile: koelsch: enable USB PHY
[3/3] ARM: shmobile: henninger: enable USB PHY

WBR, Sergei

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


[PATCH v4 1/3] ARM: shmobile: r8a7791: add USB PHY DT support

2014-09-26 Thread Sergei Shtylyov
Define the R8A7791 generic part of the USB PHY device node. It is up to the
board file to enable the device.

Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com

---
Changes in version 4:
- refreshed the patch.

Changes in version 3:
- changed subnodes of the USB PHY node to be per-channel;
- changed renesas,phy-select property to reg in the subnodes, and thus added
  #address-cells and #size-cells properties to the parent node;
- moved #phy-cells property from the parent node to the subnodes, changing its
  value to 1;
- refreshed the patch.

Changes in version 2:
- added subnodes to the USB PHY node;
- refreshed the patch.

 arch/arm/boot/dts/r8a7791.dtsi |   19 +++
 1 file changed, 19 insertions(+)

Index: renesas/arch/arm/boot/dts/r8a7791.dtsi
===
--- renesas.orig/arch/arm/boot/dts/r8a7791.dtsi
+++ renesas/arch/arm/boot/dts/r8a7791.dtsi
@@ -637,6 +637,25 @@
status = disabled;
};
 
+   usbphy: usb-phy@e6590100 {
+   compatible = renesas,usb-phy-r8a7791;
+   reg = 0 0xe6590100 0 0x100;
+   #address-cells = 1;
+   #size-cells = 0;
+   clocks = mstp7_clks R8A7791_CLK_HSUSB;
+   clock-names = usbhs;
+   status = disabled;
+
+   usb0: usb-channel@0 {
+   reg = 0;
+   #phy-cells = 1;
+   };
+   usb2: usb-channel@2 {
+   reg = 2;
+   #phy-cells = 1;
+   };
+   };
+
clocks {
#address-cells = 2;
#size-cells = 2;

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


[PATCH v4 2/3] ARM: shmobile: koelsch: enable USB PHY

2014-09-26 Thread Sergei Shtylyov
Enable USB PHY device for the Koelsch board.

Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com

---
Changes in version 4:
- refreshed the patch.

Changes in version 3:
- refreshed the patch.

Changes in version 2:
- fixed the wrong board name in the changelog;
- refreshed the patch.

 arch/arm/boot/dts/r8a7791-koelsch.dts |4 
 1 file changed, 4 insertions(+)

Index: renesas/arch/arm/boot/dts/r8a7791-koelsch.dts
===
--- renesas.orig/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ renesas/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -463,6 +463,10 @@
pinctrl-names = default;
 };
 
+usbphy {
+   status = okay;
+};
+
 pcie_bus_clk {
status = okay;
 };

--
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 v4 3/3] ARM: shmobile: henninger: enable USB PHY

2014-09-26 Thread Sergei Shtylyov
Enable USB PHY device for the Henninger board.

Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com

---
Changes in version 4:
- refreshed the patch.

Changes in version 3:
- refreshed the patch.

 arch/arm/boot/dts/r8a7791-henninger.dts |4 
 1 file changed, 4 insertions(+)

Index: renesas/arch/arm/boot/dts/r8a7791-henninger.dts
===
--- renesas.orig/arch/arm/boot/dts/r8a7791-henninger.dts
+++ renesas/arch/arm/boot/dts/r8a7791-henninger.dts
@@ -271,6 +271,10 @@
pinctrl-names = default;
 };
 
+usbphy {
+   status = okay;
+};
+
 pcie_bus_clk {
status = okay;
 };

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


Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL

2014-09-26 Thread Felipe Balbi
Hi,

On Fri, Sep 26, 2014 at 08:57:19PM +, Paul Zimmerman wrote:
   diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
   index 0fcc0a3..8277065 100644
   --- a/drivers/usb/dwc3/gadget.c
   +++ b/drivers/usb/dwc3/gadget.c
   @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, void 
   *_dwc)
 */
int dwc3_gadget_init(struct dwc3 *dwc)
{
   + u32 reg;
 int ret;
  
 dwc-ctrl_req = dma_alloc_coherent(dwc-dev, sizeof(*dwc-ctrl_req),
   @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 if (ret)
 goto err4;
  
   + if (dwc-quirks  DWC3_AMD_NL_PLAT) {
   + reg = dwc3_readl(dwc-regs, DWC3_DCTL);
   + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
  
  weird, why would Synopsys put this here ? It seems like this is only
  useful when LPM Errata is enabled and that's, apparently, a host-side
  thing.
  
  Paul, can you comment ?
 
 These bits contribute to how the device responds to an LPM transaction
 from the host. If DCFG.LPMCap=1, they set the BESL value above which
 the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So
 it's definitely a device-side thing, but only if the core is configured
 with LPM Errata support enabled.

right, and how can SW detect if LPM Errata was enabled ? From the host
point of view, we can check bit 20 of xHCI capability register. What
about device ? I can't seem to find anything :-s

cheers

-- 
balbi


signature.asc
Description: Digital signature


RE: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL

2014-09-26 Thread Paul Zimmerman
 From: Felipe Balbi [mailto:ba...@ti.com]
 Sent: Friday, September 26, 2014 2:40 PM
 
 On Fri, Sep 26, 2014 at 08:57:19PM +, Paul Zimmerman wrote:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 0fcc0a3..8277065 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, void 
*_dwc)
  */
 int dwc3_gadget_init(struct dwc3 *dwc)
 {
+   u32 reg;
int ret;
   
dwc-ctrl_req = dma_alloc_coherent(dwc-dev, 
sizeof(*dwc-ctrl_req),
@@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
if (ret)
goto err4;
   
+   if (dwc-quirks  DWC3_AMD_NL_PLAT) {
+   reg = dwc3_readl(dwc-regs, DWC3_DCTL);
+   reg |= DWC3_DCTL_LPM_ERRATA(0xf);
  
   weird, why would Synopsys put this here ? It seems like this is only
   useful when LPM Errata is enabled and that's, apparently, a host-side
   thing.
  
   Paul, can you comment ?
 
  These bits contribute to how the device responds to an LPM transaction
  from the host. If DCFG.LPMCap=1, they set the BESL value above which
  the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So
  it's definitely a device-side thing, but only if the core is configured
  with LPM Errata support enabled.
 
 right, and how can SW detect if LPM Errata was enabled ? From the host
 point of view, we can check bit 20 of xHCI capability register. What
 about device ? I can't seem to find anything :-s

I just talked to one of our RTL designers. You're right, there is no
way to tell from the Device registers alone whether the controller is
configured with LPM Errata support or not.

You can tell for sure if it is *not* enabled, by checking GSNPSID, and
if the version is earlier than 2.40a, the feature wasn't available.

So for Device-mode only controllers, I guess you will need a DT property
for this.

-- 
Paul

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


Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL

2014-09-26 Thread Felipe Balbi
Hi,

On Fri, Sep 26, 2014 at 11:18:48PM +, Paul Zimmerman wrote:
  From: Felipe Balbi [mailto:ba...@ti.com]
  Sent: Friday, September 26, 2014 2:40 PM
  
  On Fri, Sep 26, 2014 at 08:57:19PM +, Paul Zimmerman wrote:
 diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
 index 0fcc0a3..8277065 100644
 --- a/drivers/usb/dwc3/gadget.c
 +++ b/drivers/usb/dwc3/gadget.c
 @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, void 
 *_dwc)
   */
  int dwc3_gadget_init(struct dwc3 *dwc)
  {
 + u32 reg;
   int ret;

   dwc-ctrl_req = dma_alloc_coherent(dwc-dev, 
 sizeof(*dwc-ctrl_req),
 @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
   if (ret)
   goto err4;

 + if (dwc-quirks  DWC3_AMD_NL_PLAT) {
 + reg = dwc3_readl(dwc-regs, DWC3_DCTL);
 + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
   
weird, why would Synopsys put this here ? It seems like this is only
useful when LPM Errata is enabled and that's, apparently, a host-side
thing.
   
Paul, can you comment ?
  
   These bits contribute to how the device responds to an LPM transaction
   from the host. If DCFG.LPMCap=1, they set the BESL value above which
   the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So
   it's definitely a device-side thing, but only if the core is configured
   with LPM Errata support enabled.
  
  right, and how can SW detect if LPM Errata was enabled ? From the host
  point of view, we can check bit 20 of xHCI capability register. What
  about device ? I can't seem to find anything :-s
 
 I just talked to one of our RTL designers. You're right, there is no
 way to tell from the Device registers alone whether the controller is
 configured with LPM Errata support or not.
 
 You can tell for sure if it is *not* enabled, by checking GSNPSID, and
 if the version is earlier than 2.40a, the feature wasn't available.

alright, we can use this for something like:

WARN(rev  2.40a  (flags  LPM_ERRATA || of_property_read_bool(lpm-errata)),
LPM Errata not available on dwc3 revisions = 2.40a\n);

 So for Device-mode only controllers, I guess you will need a DT property
 for this.

right, DT property and platform_data feature flag, or something. I don't
wanna call it a quirk though, although it _is_ an erratum WRT LPM
support. Dunno. Any strong feelings ?

-- 
balbi


signature.asc
Description: Digital signature


RE: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL

2014-09-26 Thread Paul Zimmerman
 From: Felipe Balbi [mailto:ba...@ti.com]
 Sent: Friday, September 26, 2014 5:54 PM
 
 On Fri, Sep 26, 2014 at 11:18:48PM +, Paul Zimmerman wrote:
   From: Felipe Balbi [mailto:ba...@ti.com]
   Sent: Friday, September 26, 2014 2:40 PM
  
   On Fri, Sep 26, 2014 at 08:57:19PM +, Paul Zimmerman wrote:
  diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
  index 0fcc0a3..8277065 100644
  --- a/drivers/usb/dwc3/gadget.c
  +++ b/drivers/usb/dwc3/gadget.c
  @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, 
  void *_dwc)
*/
   int dwc3_gadget_init(struct dwc3 *dwc)
   {
  +   u32 reg;
  int ret;
 
  dwc-ctrl_req = dma_alloc_coherent(dwc-dev, 
  sizeof(*dwc-ctrl_req),
  @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
  if (ret)
  goto err4;
 
  +   if (dwc-quirks  DWC3_AMD_NL_PLAT) {
  +   reg = dwc3_readl(dwc-regs, DWC3_DCTL);
  +   reg |= DWC3_DCTL_LPM_ERRATA(0xf);

 weird, why would Synopsys put this here ? It seems like this is only
 useful when LPM Errata is enabled and that's, apparently, a host-side
 thing.

 Paul, can you comment ?
   
These bits contribute to how the device responds to an LPM transaction
from the host. If DCFG.LPMCap=1, they set the BESL value above which
the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So
it's definitely a device-side thing, but only if the core is configured
with LPM Errata support enabled.
  
   right, and how can SW detect if LPM Errata was enabled ? From the host
   point of view, we can check bit 20 of xHCI capability register. What
   about device ? I can't seem to find anything :-s
 
  I just talked to one of our RTL designers. You're right, there is no
  way to tell from the Device registers alone whether the controller is
  configured with LPM Errata support or not.
 
  You can tell for sure if it is *not* enabled, by checking GSNPSID, and
  if the version is earlier than 2.40a, the feature wasn't available.
 
 alright, we can use this for something like:
 
 WARN(rev  2.40a  (flags  LPM_ERRATA || 
 of_property_read_bool(lpm-errata)),
   LPM Errata not available on dwc3 revisions = 2.40a\n);
 
  So for Device-mode only controllers, I guess you will need a DT property
  for this.
 
 right, DT property and platform_data feature flag, or something. I don't
 wanna call it a quirk though, although it _is_ an erratum WRT LPM
 support. Dunno. Any strong feelings ?

Well, it's called LPM Errata because the feature was added to the USB
spec as an erratum. It's not an erratum to our controller. But I don't
have any strong feelings about how the driver support is implemented.

-- 
Paul

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


Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL

2014-09-26 Thread Felipe Balbi
Hi,

On Sat, Sep 27, 2014 at 01:05:46AM +, Paul Zimmerman wrote:
  From: Felipe Balbi [mailto:ba...@ti.com]
  Sent: Friday, September 26, 2014 5:54 PM
  
  On Fri, Sep 26, 2014 at 11:18:48PM +, Paul Zimmerman wrote:
From: Felipe Balbi [mailto:ba...@ti.com]
Sent: Friday, September 26, 2014 2:40 PM
   
On Fri, Sep 26, 2014 at 08:57:19PM +, Paul Zimmerman wrote:
   diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
   index 0fcc0a3..8277065 100644
   --- a/drivers/usb/dwc3/gadget.c
   +++ b/drivers/usb/dwc3/gadget.c
   @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, 
   void *_dwc)
 */
int dwc3_gadget_init(struct dwc3 *dwc)
{
   + u32 reg;
 int ret;
  
 dwc-ctrl_req = dma_alloc_coherent(dwc-dev, 
   sizeof(*dwc-ctrl_req),
   @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 if (ret)
 goto err4;
  
   + if (dwc-quirks  DWC3_AMD_NL_PLAT) {
   + reg = dwc3_readl(dwc-regs, DWC3_DCTL);
   + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
 
  weird, why would Synopsys put this here ? It seems like this is only
  useful when LPM Errata is enabled and that's, apparently, a 
  host-side
  thing.
 
  Paul, can you comment ?

 These bits contribute to how the device responds to an LPM transaction
 from the host. If DCFG.LPMCap=1, they set the BESL value above which
 the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So
 it's definitely a device-side thing, but only if the core is 
 configured
 with LPM Errata support enabled.
   
right, and how can SW detect if LPM Errata was enabled ? From the host
point of view, we can check bit 20 of xHCI capability register. What
about device ? I can't seem to find anything :-s
  
   I just talked to one of our RTL designers. You're right, there is no
   way to tell from the Device registers alone whether the controller is
   configured with LPM Errata support or not.
  
   You can tell for sure if it is *not* enabled, by checking GSNPSID, and
   if the version is earlier than 2.40a, the feature wasn't available.
  
  alright, we can use this for something like:
  
  WARN(rev  2.40a  (flags  LPM_ERRATA || 
  of_property_read_bool(lpm-errata)),
  LPM Errata not available on dwc3 revisions = 2.40a\n);
  
   So for Device-mode only controllers, I guess you will need a DT property
   for this.
  
  right, DT property and platform_data feature flag, or something. I don't
  wanna call it a quirk though, although it _is_ an erratum WRT LPM
  support. Dunno. Any strong feelings ?
 
 Well, it's called LPM Errata because the feature was added to the USB
 spec as an erratum. It's not an erratum to our controller. But I don't
 have any strong feelings about how the driver support is implemented.

how about adding a has_lpm_erratum to struct dwc3 which gets
initialized either through pdata or DT ? Then above WARN() would become:

WARN(dwc-revision  DWC3_REVISION_240A  dwc-has_lpm_erratum,
LPM Erratum not available on dwc3 revisisions  2.40a\n);

Then we're not really calling it a quirk. In fact Huang, when respining
your series, let's convert your quirk bits into single-bit fields inside
struct dwc3 and struct platform_data. Like so:

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 4d4e854..e233ce1 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -695,11 +695,13 @@ static int dwc3_probe(struct platform_device *pdev)
 
if (node) {
dwc-maximum_speed = of_usb_get_maximum_speed(node);
+   dwc-has_lpm_erratum = of_property_read_bool(node, 
snps,has-lpm-erratum);
 
dwc-needs_fifo_resize = of_property_read_bool(node, 
tx-fifo-resize);
dwc-dr_mode = of_usb_get_dr_mode(node);
} else if (pdata) {
dwc-maximum_speed = pdata-maximum_speed;
+   dwc-has_lpm_erratum = pdata-has_lpm_erratum;
 
dwc-needs_fifo_resize = pdata-tx_fifo_resize;
dwc-dr_mode = pdata-dr_mode;
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 66f6256..23e1504 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -661,6 +661,8 @@ struct dwc3_scratchpad_array {
  * @ep0_bounced: true when we used bounce buffer
  * @ep0_expect_in: true when we expect a DATA IN transfer
  * @has_hibernation: true when dwc3 was configured with Hibernation
+ * @has_lpm_erratum: true when core was configured with LPM Erratum. Note that
+ * there's now way for software to detect this in runtime.
  * @is_selfpowered: true when we are selfpowered
  * @needs_fifo_resize: not all users might want fifo resizing, flag it
  * @pullups_connected: true when Run/Stop bit is set
@@ -764,6