Re: [PATCH 0/3] usb: remove duplicate & operation

2017-10-20 Thread Lukasz Majewski
Hi Jaejoong,

> usb_endpoint_maxp() has an inline keyword and searches for bits[10:0]
> by & operation with 0x7ff. So, we can remove the duplicate & operation
> with 0x7ff.
> 
> Jaejoong Kim (3):
>   usb: gadget: udc: remove duplicate & operation
>   usb: gadget: udc: gr: remove duplicate & operation
>   usb: misc: usbtest: remove duplicate & operation
> 
>  drivers/usb/gadget/udc/core.c   | 2 +-
>  drivers/usb/gadget/udc/gr_udc.c | 2 +-
>  drivers/usb/misc/usbtest.c  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 

I think that it would be better if you could squash those 3 patches
into one.



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
--
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: [iMX6Q][CI] EHCI Low-speed device problem

2017-10-19 Thread Lukasz Majewski
Hi Alan,

> On Thu, 19 Oct 2017, Lukasz Majewski wrote:
> 
> > > > Problem:
> > > >  
> > > > I do observe that the USB transfers are truncated - for example
> > > > "Get Configuration Descriptor", which length is 34B only gets
> > > > 16B from the device. 
> > > > 
> > > > I do get the "Invalid PID sequence" error and despite the
> > > 
> > > How do you know the error is "Invalid PID sequence" and not
> > > something else, like CRC error or timeout?
> > 
> > I'm using TotalPhase USB 480 Analyzer. It shows only "Invalid PID
> > sequence" as the error. When I "unroll" the
> > GetConfigurationDescriptor, then all data is the same as for
> > correct transmission.
> > 
> > > 
> > > > DATA1 Packet received by host - the EHCI controller is not
> > > > sending ACK. 
> > > 
> > > The DATA1 packets are the ones containing bytes 9-16, 25-32, and
> > > so on (low speed always uses maxpacket size = 8).  Since you
> > > received 16 bytes, this means the host controller accepted the
> > > DATA1 packet but didn't send an ACK -- that certainly would be a
> > > hardware bug.
> > 
> > I do receive 16B, but those are "comprised" of two separate IN
> > transactions (8 B each).
> > One such transaction is built with SETUP -> DATA1 -> ACK packets.
> > 
> > [Please see the attached screen]
> 
> According to the screen dump, the device sent a DATA1 packet which
> the host controller did not ACK.  That's a bug in the host
> controller.  In the next transaction, the device sent an empty DATA1
> packet instead of resending the previous data.  That's a bug in the
> device.
> 

Ok. So then we do have HW issues in both sides :-). Nice...

> > >  Anyway,
> > > this should have caused the device to retransmit the DATA1 packet
> > > in response to the next IN packet (if there was one).
> > 
> > There is IN packet sent afterwards, but it is empty. The whole
> > GetConfigurationDescriptor ends with OUT ZLP
> 
> That part is normal.

Ok.

> 
> > I'm wondering if it is feasible to manually check the XactErr bit
> > and then for example order the soft USB reset (from the root hub)?
> > 
> > Or is there any other acceptable in upstream solution?
> 
> Not in general.  For this one particular case (a short transfer for a
> Get-Configuration-Descriptor request), it would be easy enough to
> retry the transfer.  Just put a short retry loop around the second
> usb_get_descriptor() call in usb_get_configuration() in
> drivers/usb/core/config.c.  But similar failures at other times would
> be harder to recover from.

This only one case. Those errors happens also at other parts of the
enumeration phase -> very often also the "Get Report Descriptor" (178
bytes in size) is truncated.

This is more severe, since those errors are propagated up the USB stack
and exceed the "rule of thumb" of 4 retries set by Linus back at
2005 


I'm now trying to force the code from ./usb/host/ehci-q.c
qh_completions() to re-execute retry_xacterr.

Unfortunately, it would be hard since CI Host Controller do not set
"Halt" token status (bit 6) and some extra hacks are needed.



@ Fabio, Peter, is there anybody from NXP, who can I contact to get
some info regarding the way this IP block works (some IP block
designer/ RTL implementor)?
Maybe, I could get some hints on how to look for "reserved" registers?
Or alleviating the HW problem on the CI HCI side?

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

Anyway, many thanks for clarification.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
--
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: [iMX6Q][CI] EHCI Low-speed device problem

2017-10-19 Thread Lukasz Majewski
Hi Peter,

> On Thu, Oct 19, 2017 at 07:47:01AM +0200, Lukasz Majewski wrote:
> > > > I'm wondering if it is feasible to manually check the XactErr
> > > > bit and then for example order the soft USB reset (from the
> > > > root hub)?
> > > > 
> > > > Or is there any other acceptable in upstream solution?
> > > > 
> > > > > 
> > > > > > The problem has been described in detail (including screen
> > > > > > shots from USB analyzer + some further investigations) here:
> > > > > > https://community.nxp.com/thread/462409
> > > > > 
> > > 
> > > Lukasz, there is a known issue 
> > 
> > Could you point me to any "Errata" document describing this issue?
> > 
> > Could you elaborate on it a bit more?
> > 
> 
> No, it assumes PHY's power is prior to controller's initialization.

Is there any particular time necessary? 

> 
> > > that low speed device may have issue
> > > that i.mx6 PHY's power is not ready before controller goes to
> > > initialize,
> > 
> > Please correct my understanding if I'm wrong - the issue here is
> > with USB PHY. When low-speed device is connected, the USB PHY may
> > be not properly powered, but the USB controller is ready for
> > serving the transmission.
> 
> Yes.
> 
> > 
> > > so for i.mx6, we should not use PORT_PP for PHY's power.
> > 
> > In our design we do control (with some dedicated USB VBUS switch
> > IC) the VBUS power with USB_H1_PWR# pin PAD_EIM_D31, which indeed
> > is the output controlled by PORTSC1's bit 12 (PP).
> 
> Please configure PAD_EIM_D31 as GPIO instead of USB_PWR_EN.

pinctrl_usbh1_vbus: usbh1_vbus_grp {
fsl,pins = <
MX6QDL_PAD_EIM_D31__GPIO3_IO31 0x1b0b0
>;
};

It did not help.

I've also setup the regulator API to enable VBUS and then wait ~300ms
with stable power before we proceed with USB Host further
initialization.

reg_usbh1_vbus: usb-h1-vbus {
compatible = "regulator-fixed";
gpio = < 31 GPIO_ACTIVE_LOW>;
pinctrl-names = "default";
pinctrl-0 = <_usbh1_vbus>;
regulator-name = "usb_h1_vbus";
regulator-min-microvolt = <500>;
regulator-max-microvolt = <500>;
regulator-enable-ramp-delay = <30>;
};

 {
vbus-supply = <_usbh1_vbus>;
}

The ~300ms is from touchscreen datasheet - time needed for
initialization.

However, with EHCI from Samsung/Intel I don't need such time to have
touchscreen properly enumerated.

> > 
> > One more strange thing, which I've observed - the bit 4 (OCA) from
> > PORTSC1 goes high during the operation - no matter if we succeed or
> > no with the USB enumeration:
> > 
> > IMX6#0>mdahb 0x02184384 1 
> > AHB/AXI 00_02184384 : 14001415
> > 
> > It indicates that we do have an "over-current" condition, but the
> > touchscreen IC's MAX current consumption is 18 mA, and the port can
> > provide 100 mA. The touchscreen IC is the only device connected to
> > USB Host1.
> 
> Please check if pinctrl and polarity of OC pin are correct.

The OC on the PCB is active low (and it is externally pullup'ed with
10k resistor).

> 
> > 
> > > See flag: CI_HDRC_TURN_VBUS_EARLY_ON for detail.
> > 
> > This flag enables very early the VBUS power. I do use standard
> > settings here (the flag is set).
> > 
> 
> You need to enable VBUS power early really, that means using GPIO to
> control it instead of USB PIN. See imx6qdl-sabresd as an example.

OK. This is already done (I checked that the CI_HDRC_TURN_VBUS_EARLY_ON
is enabled and USB_PWR_EN is configured as GPIO).

> 
> > > At i.mx6, the VBUS
> > > 5v is the power for USB PHY.
> > 
> > So the VBUS is at iMX6q also the power for USB PHY? 
> > 
> > In our design we do use VBUS to power the sensor.
> > 
> > How can I workaround this issue? Can it be done in SW (to avoid
> > changing the PCB routing)?
> > 
> 
> One of USBOTG1_VBUS and USBH1_VBUS can be both USB PHY's power.
> Usually, it doesn't matter the VBUS powers USB peripheral unless
> the peripheral really consumes too much current, eg > 500mA, it
> may have problem, I am not sure, I am not hardware guy.
> 

Could you look into the picture attached to the earlier e-mail and
comment on Alan's reply (especially about the lack of error indication
fo XactErr)?


Thanks in advance,


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
--
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: [iMX6Q][CI] EHCI Low-speed device problem

2017-10-18 Thread Lukasz Majewski
Hi Peter,

> On Thu, Oct 19, 2017 at 01:38:45AM +0200, Lukasz Majewski wrote:
> > Hi Alan,
> > 
> > Thank you for you reply.
> > 
> > > On Wed, 18 Oct 2017, Lukasz Majewski wrote:
> > > 
> > > > Dear All,
> > > > 
> > > > I'm using iMX6q for my application. It uses the touchscreen IC
> > > > connected via USB port (Host port 1). The touchscreen operates
> > > > with low-speed. 
> > > 
> > > I don't know anything about the iMX6q specifically...
> > 
> > But you do know a lot about EHCI/USB :)
> > 
> > > 
> > > > Board: Wandboard rev1b / some custom board
> > > > SW: Mainline Linux 4.13
> > > >  
> > > > Problem:
> > > >  
> > > > I do observe that the USB transfers are truncated - for example
> > > > "Get Configuration Descriptor", which length is 34B only gets
> > > > 16B from the device. 
> > > > 
> > > > I do get the "Invalid PID sequence" error and despite the
> > > 
> > > How do you know the error is "Invalid PID sequence" and not
> > > something else, like CRC error or timeout?
> > 
> > I'm using TotalPhase USB 480 Analyzer. It shows only "Invalid PID
> > sequence" as the error. When I "unroll" the
> > GetConfigurationDescriptor, then all data is the same as for
> > correct transmission.
> > 
> > > 
> > > > DATA1 Packet received by host - the EHCI controller is not
> > > > sending ACK. 
> > > 
> > > The DATA1 packets are the ones containing bytes 9-16, 25-32, and
> > > so on (low speed always uses maxpacket size = 8).  Since you
> > > received 16 bytes, this means the host controller accepted the
> > > DATA1 packet but didn't send an ACK -- that certainly would be a
> > > hardware bug.
> > 
> > I do receive 16B, but those are "comprised" of two separate IN
> > transactions (8 B each).
> > One such transaction is built with SETUP -> DATA1 -> ACK packets.
> > 
> > [Please see the attached screen]
> > 
> > >  Anyway,
> > > this should have caused the device to retransmit the DATA1 packet
> > > in response to the next IN packet (if there was one).
> > 
> > There is IN packet sent afterwards, but it is empty. The whole
> > GetConfigurationDescriptor ends with OUT ZLP
> > 
> > > 
> > > > The problem is not observed when I plug the touchscreen device
> > > > via USB 2.0 HUB. (iMX6q -> USB 2.0 HUB -> touchscreen)
> > > > 
> > > > Also, the problem is not present on different EHCI
> > > > implementations - namely Intel or Synopsis (Exynos 5422). 
> > > > 
> > > > 
> > > > Please find below some data dump from EHCI's qTD (Queue Element
> > > >   Transfer
> > > > Descriptor): 
> > > > [ 1.987479] ci_hdrc ci_hdrc.0: submit_async 1 urb edfe6e80
> > > > ep0out len 178, qtd ef061180 [qh edfe6780]
> > > > [ 1.988781] ehci urb: IRQ status: 0x1 INTREN: 0x37 hrt: 0x80
> > > > [ 1.988795] scan_async: urb qh: 0xedfe6780 qtd_list: 0xef0610f8
> > > > [ 1.988802] qh_completions: urb token 0x8e00 0 178
> > > 
> > > This is the Setup transaction, which completed normally.
> > > 
> > > > [ 1.988809] qh_completions: urb token 0x920d08 0 178 
> > > >   ^--- here we do have
> > > > still active "status" 9x8
> > > 
> > > No, 0x08 in the low-order bits is XactErr.  Active would be 0x80.
> > 
> > Ach... right.
> > 
> > > Notice, however, that the CErr field was not decremented.
> > > According to the EHCI spec, that should not happen.  Another
> > > hardware bug?
> > 
> > Good point.
> > 
> > > 
> > > Notice that the Halted bit was not set.  This means the hardware
> > > thinks the transaction completed successfully.
> > 
> > Unfortunately, it does. It behaves like everything went correct and
> > pases truncated data up to the stack.
> > 
> > > 
> > > > [ 1.988815] qh_completions: urb token 0x8c00 32 178   
> > > >   ^--- here the IOC bit is 
> > > >set (0x8)
> > > > [ 1.988828] ci_hdrc ci_hdrc.0: ehci_urb_done 1 urb edfe6e80 
>

[iMX6Q][CI] EHCI Low-speed device problem

2017-10-18 Thread Lukasz Majewski
Dear All,

I'm using iMX6q for my application. It uses the touchscreen IC
connected via USB port (Host port 1). The touchscreen operates with
low-speed. 

Board: Wandboard rev1b / some custom board
SW: Mainline Linux 4.13
 
Problem:
 
I do observe that the USB transfers are truncated - for example
"Get Configuration Descriptor", which length is 34B only gets 16B from
the device. 

I do get the "Invalid PID sequence" error and despite the
DATA1 Packet received by host - the EHCI controller is not sending ACK. 
 
 
The problem is not observed when I plug the touchscreen device via USB
2.0 HUB. (iMX6q -> USB 2.0 HUB -> touchscreen)

Also, the problem is not present on different EHCI implementations -
namely Intel or Synopsis (Exynos 5422). 


Please find below some data dump from EHCI's qTD (Queue Element
  Transfer Descriptor):
 
[ 1.987479] ci_hdrc ci_hdrc.0: submit_async 1 urb edfe6e80 ep0out 
len 178, qtd ef061180 [qh edfe6780]
[ 1.988781] ehci urb: IRQ status: 0x1 INTREN: 0x37 hrt: 0x80
[ 1.988795] scan_async: urb qh: 0xedfe6780 qtd_list: 0xef0610f8
[ 1.988802] qh_completions: urb token 0x8e00 0 178
[ 1.988809] qh_completions: urb token 0x920d08 0 178 
  ^--- here we do have still 
active "status" 9x8
[ 1.988815] qh_completions: urb token 0x8c00 32 178   
  ^--- here the IOC bit is 
   set (0x8)
[ 1.988828] ci_hdrc ci_hdrc.0: ehci_urb_done 1 urb edfe6e80 
ep0in status 0 len 32/178
 
The above code is executed after receiving "USBINT" interrupt - 
according to EHCI controller everything went fine
 

[ 1.992192] ci_hdrc ci_hdrc.0: submit_async 1 urb edfe6e80 ep0out 
len 178, qtd ef061060 [qh edfe6780]
[ 1.995785] ehci urb: IRQ status: 0x1 INTREN: 0x37 hrt: 0x80
[ 1.995804] scan_async: urb qh: 0xedfe6780 qtd_list: 0xef0611b8
[ 1.995811] qh_completions: urb token 0x8e00 0 178  
  ^--- setup transaction
[ 1.995819] qh_completions: urb token 0xd00 0 178   
  ^--- here we do have 0x0 status
[ 1.995825] qh_completions: urb token 0x8c00 178 178   
  ^--- here we do ask for 
interrupt when done
[ 1.995840] ci_hdrc ci_hdrc.0: ehci_urb_done 1 urb edfe6e80 
ep0in status 0 len 178/178


The most _strange_ issue here is the lack of _any_ error indicated by
the HOST driver. All transmissions end with USBINT set. No error
interrupt present. This corrupted data (with smaller size than
expected) is then passed to upper layers, causing subtle errors.


Have anybody had similar problem?

Any hints on how to proceed?





The problem has been described in detail (including screen shots 
from USB analyzer + some further investigations) here:
https://community.nxp.com/thread/462409


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
--
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 v7 2/5] usb: s3c-hsotg: Adding phy driver support

2012-10-31 Thread Lukasz Majewski
Hi Felipe,

 Hi,
 
 On Tue, Oct 30, 2012 at 10:27:34AM +0530, Praveen Paneri wrote:
  @@ -3501,20 +3511,27 @@ static int __devinit s3c_hsotg_probe(struct
  platform_device *pdev) int ret;
  int i;
   
  -   plat = pdev-dev.platform_data;
  -   if (!plat) {
  -   dev_err(pdev-dev, no platform data defined\n);
  -   return -EINVAL;
  -   }
  -
  hsotg = devm_kzalloc(pdev-dev, sizeof(struct s3c_hsotg),
  GFP_KERNEL); if (!hsotg) {
  dev_err(dev, cannot get memory\n);
  return -ENOMEM;
  }
   
  +   plat = pdev-dev.platform_data;
  +   if (!plat) {
  +   /* Fallback for transceiver */
  +   phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
  +   if (IS_ERR_OR_NULL(phy)) {
  +   dev_err(pdev-dev, no platform data or
  transceiver defined\n);
  +   return -EPROBE_DEFER;
  +   } else {
  +   hsotg-phy = phy;
  +   }
  +   } else {
  +   hsotg-plat = plat;
  +   }
 
 I think this should be the other way around, meaning you try to grab
 the phy, if you can't, then you fallback to pdata.
 

I agree.

The new approach is to use new PHY driver. And only when failed we
shall use legacy approach.

-- 
Best regards,

Lukasz Majewski

Samsung Poland RD Center | Linux Platform Group
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] Kernel panic when try s3c-hsotg.c with kernel 3.5

2012-08-08 Thread Lukasz Majewski
Dear Peiyong Feng,

  Please enable the debug at s3c-hsotg.c driver and then paste the
  dmesg/debug output.  
 I have defined DEGUG in s3c-hsotg.c

Thank you for 2.6.36 log.
I'd also need the log from 3.6-rc1 kernel with DEBUG enabled.

-- 
Best regards,

Lukasz Majewski

Samsung Poland RD Center | Linux Platform Group
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] Kernel panic when try s3c-hsotg.c with kernel 3.5

2012-08-07 Thread Lukasz Majewski
On Tue, 07 Aug 2012 11:04:10 +0800
Peiyong Feng peiyong.feng.ker...@gmail.com wrote:

 2012/8/6 Lukasz Majewski l.majew...@samsung.com:
  Hi,
 
  Hi,
 
  On Mon, Aug 06, 2012 at 06:12:05PM +0800, Peiyong Feng wrote:
   I got a kernel panic when try hsotg of ok6410 which is based on
   s3c6410:
  As you said, you are using the ok6410. And it is based on the
  s3c6410 CPU. S3C6410 is a single core CPU. I assume that ok6410 is
  also single core?
 yes
 
  
  
   cdc_acm: USB Abstract Control Model driver for USB modems and
   ISDN adapters Unable to handle kernel NULL pointer dereference at
   virtual address 0100
 
   pgd = c0004000
   [0100] *pgd=
   Internal error: Oops: 5 [#1] ARM
   Modules linked in:
   CPU: 0Not tainted  (3.5.0 #9)
   PC is at s3c_hsotg_handle_outdone+0x44/0x158
   LR is at s3c_hsotg_irq+0x75c/0x804
   pc : [c023e7fc]lr : [c024061c]psr: 6193
   sp : c782fd20  ip : 0029  fp : c13a1460
   r10:   r9 : 0008  r8 : 00d0
   r7 : c13a1400  r6 : 0002  r5 :   r4 : 00060002
   r3 : 00d0  r2 :   r1 : 00080200  r0 : c13a1400
   Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment
   kernel Control: 00c5387d  Table: 50004008  DAC: 0017
   Process swapper (pid: 1, stack limit = 0xc782e268)
   Stack: (0xc782fd20 to 0xc783)
   fd20: c13a1460 c0200f64  00060002  0002
   c13a1400 0010 fd40:  c024061c 00060002 
   0002 0008 c782fda0 c139a5c0 fd60: c139a5c0 
    005a c04cc52c c04cc594 c04ea5fe c00565c0 fd80:
      c04cc52c c139a5c0 c04cc57c 
   c04eb328 fda0: c04cc55c c04eb324 c04c44c0 c0056768 c04cc52c
   c04cc57c  c0058d64 fdc0: 005a c04d7dd8 
   c005617c 005a c000efbc c04eb350 0001 fde0: c782fe08
   c000853c c036c540 6013  c782fe3c c04cc57c c04cc55c
   fe00: 6013 c000dd80 c04cc57c c782c000  0001
   6013 c04cc52c fe20: c139a5c0 005a c04cc57c c04cc55c
   6013 c04c44c0 f601 c782fe50 fe40: c036c53c c036c540
   6013  c023fec0 c00574c8  c008b2dc fe60:
   c023fec0   c13a1400 005a c139a5c0 c04cc52c
   c0057960 fe80: 005a c13a1400 c04c44b8  c051d238
   c049b0ec  c03688fc fea0: c7853e60 c13a1400 c7804f80
    c04c44f4 6013 c7855a80  fec0: c04e1bb4
   c04c44c0 c04c44c0 c04e1bb4 c04e1bb4 c051d238 c049b0ec 
   fee0: c04eb040 c020588c c04c44c0 c0204524 c04c44c0 c04c44f4
   c04e1bb4 c02046ac ff00: c13a01e0 c0204738  c782ff18
   c04e1bb4 c0202e30 c7803878 c7823700 ff20: c04dd1d0 c040b8d4
   c04e1bb4 c04e1bb4 c04dd1d0 c0203600 c040b8d4 c01b8568 ff40:
     c04e1bb4 0007 c04eb040 c782e000 c04a65e0
   c0204ce8 ff60:  c04a65d4 0007 c04eb040 c782e000
   c0008628 c04c7ea0  ff80: 009c  c0625cf9
   c0037178 0006 0006 c0461b84 c042cee8 ffa0: c04c7ea0
   c04abdd4 c04a65d4 0007 c04eb040 009c c04841b0 c04a65e0
   ffc0:  c048430c 0006 0006 c04841b0 
    c048421c ffe0: c000f08c 0013  
    c000f08c   [c023e7fc]
   (s3c_hsotg_handle_outdone+0x44/0x158) from [c024061c]
   (s3c_hsotg_irq+0x75c/0x804) [c024061c]
   (s3c_hsotg_irq+0x75c/0x804) from [c00565c0]
   (handle_irq_event_percpu+0x50/0x1bc) [c00565c0]
   (handle_irq_event_percpu+0x50/0x1bc) from [c0056768]
   (handle_irq_event+0x3c/0x5c) [c0056768]
   (handle_irq_event+0x3c/0x5c) from [c0058d64]
   (handle_level_irq+0x8c/0x118) [c0058d64]
   (handle_level_irq+0x8c/0x118) from [c005617c]
   (generic_handle_irq+0x38/0x44) [c005617c]
   (generic_handle_irq+0x38/0x44) from [c000efbc]
   (handle_IRQ+0x30/0x84) [c000efbc] (handle_IRQ+0x30/0x84) from
   [c000853c] (vic_handle_irq+0x68/0xa8) [c000853c]
   (vic_handle_irq+0x68/0xa8) from [c000dd80]
   (__irq_svc+0x40/0x60) Exception stack(0xc782fe08 to 0xc782fe50)
   fe00: c04cc57c c782c000  0001 6013 c04cc52c fe20:
   c139a5c0 005a c04cc57c c04cc55c 6013 c04c44c0 f601
   c782fe50 fe40: c036c53c c036c540 6013  [c000dd80]
   (__irq_svc+0x40/0x60) from [c036c540]
   (_raw_spin_unlock_irqrestore+0x10/0x14) [c036c540]
   (_raw_spin_unlock_irqrestore+0x10/0x14) from [c00574c8]
   (__setup_irq+0x178/0x3f8) [c00574c8] (__setup_irq+0x178/0x3f8)
   from [c0057960] (request_threaded_irq+0xc4/0x12c) [c0057960]
   (request_threaded_irq+0xc4/0x12c) from [c03688fc]
   (s3c_hsotg_probe+0x14c/0x700) [c03688fc]
   (s3c_hsotg_probe+0x14c/0x700) from [c020588c]
   (platform_drv_probe+0x18/0x1c) [c020588c]
   (platform_drv_probe+0x18/0x1c) from [c0204524]
   (driver_probe_device+0x78/0x200) [c0204524]
   (driver_probe_device+0x78/0x200) from [c0204738]
   (__driver_attach+0x8c/0x90) [c0204738]
   (__driver_attach+0x8c/0x90) from [c0202e30]
   (bus_for_each_dev+0x60/0x8c) [c0202e30]
   (bus_for_each_dev+0x60/0x8c) from

Re: [BUG] Kernel panic when try s3c-hsotg.c with kernel 3.5

2012-08-06 Thread Lukasz Majewski
]
  (do_one_initcall+0x34/0x17c) from [c048430c]
  (kernel_init+0xf0/0x1bc) [c048430c] (kernel_init+0xf0/0x1bc) from
  [c000f08c] (kernel_thread_exit+0x0/0x8) Code: e0433106 e0833006
  e1a03183 e0828003 (e5984030) ---[ end trace 2ea4e574318ecf99 ]---
  Kernel panic - not syncing: Fatal exception in interrupt
  ---
  
  
  When I try locate the source using arm-linux-gdb, I got this:
  -
  GNU gdb (Sourcery G++ Lite 2008q3-72) 6.8.50.20080821-cvs
  Copyright (C) 2008 Free Software Foundation, Inc.
  License GPLv3+: GNU GPL version 3 or later
  http://gnu.org/licenses/gpl.html This is free software: you are
  free to change and redistribute it. There is NO WARRANTY, to the
  extent permitted by law.  Type show copying and show warranty
  for details. This GDB was configured as --host=i686-pc-linux-gnu
  --target=arm-none-linux-gnueabi.
  For bug reporting instructions, please see:
  https://support.codesourcery.com/GNUToolchain/...
  (gdb) l *(s3c_hsotg_handle_outdone+0x44)
  0xc023e7fc is in s3c_hsotg_handle_outdone
  (drivers/usb/gadget/s3c-hsotg.c:1553). 1548 static void
  s3c_hsotg_handle_outdone(struct s3c_hsotg *hsotg,
  1549 int epnum, bool
  was_setup) 1550 { 1551  u32 epsize =
  readl(hsotg-regs + DOEPTSIZ(epnum)); 1552  struct
  s3c_hsotg_ep *hs_ep = hsotg-eps[epnum]; 1553
  struct s3c_hsotg_req *hs_req = hs_ep-req; 1554
  struct usb_request *req = hs_req-req; 1555
  unsigned size_left = DxEPTSIZ_XferSize_GET(epsize);
  1556int result = 0; 1557
  (gdb)
  
  
  
  I also try the old linux like 2.6.36, I works fine.
 
 Lukasz, any ideas ?
 

I see, that you are using 3.5.0 kernel.
During the 3.5 kernel development cycle, I've discovered very nasty SMP
related bug. It is fixed at 3.6-rc1 code.

Relevant commits are:
22258f4 usb: hsotg: samsung: Replace endpoint specific locks with a
global lock
2b19a52 usb: hsotg: samsung: Protect the udc_stop routine with spinlock
5ad1d31 usb: hsotg: samsung: smp Provide *_lock functions abstraction
layer for SMP SoCs

Above commits are already available at 3.6-rc1.

Would it be possible to cross compile 3.6-rc1 kernel for your platform
and test the USB?



I'd like to ask you for #define DEBUG above includes (to enable
dev_dbg()) calls at ./drivers/usb/gadget/s3c-hsotg.c file.

Afterwards please, paste the log (dmesg) from booting your target
device.

Thanks in advance.

-- 
Best regards,

Lukasz Majewski

Samsung Poland RD Center | Linux Platform Group
--
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 RESEND] [RFC] usb:gadget: Refcount for gadget pullup

2012-07-30 Thread Lukasz Majewski
This commit fixes the way gadget's pullup method (wrapped at
usb_gadget_connect/disconnect) is called in the udc-core.

The composite driver allows correct driver registration, even when it calls
the usb_gadget_disconnect method (composite driver configuration is defered 
for user space - please look into the description of usb_composite_probe at
composite.c - line: 1623)

One such example is the CCG (Configurable Composite Gadget) driver (at
drivers/staging/ccg), which after its registration has no usb descriptor
(i.e. idProduct, idVendor etc.) and functions registered. Those are configured
after writing to /sys/module/g_ccg/parameters/ or /sys/class/ccg_usb/ccg0/.

Unfortunately, the code at 'usb_gadget_probe_driver' method (some code omitted):

if (udc_is_newstyle(udc)) {
bind(udc-gadget);
usb_gadget_udc_start(udc-gadget, driver);
usb_gadget_connect(udc-gadget);
}

Explicitly calls the usb_gadget_connect method for this driver. It looks like
the udc-core enables pullup for a driver, which has no functions and no
descriptor filled (those values are feed from userspace).

The USB composite driver API allows correct driver registration with calling 
usb_gadget_disconnect method, but as it is now, _ALL_ newstyle usb gadgets are
connected by default. Therefore it violates the composite API.

The solution (at least until the udc-core is reworked) is to add atomic
variable, which helps in balancing the number of called usb_gadget_connect/
disconnect functions.

Signed-off-by: Lukasz Majewski l.majew...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/usb/gadget/udc-core.c |   17 -
 include/linux/usb/gadget.h|   13 +++--
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index e5e44f8..a26517e 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -349,7 +349,22 @@ found:
}
usb_gadget_connect(udc-gadget);
} else {
-
+   /*
+* This is a hack for old style gadgets:
+*
+* The udc_start for old style gadgets performs implicitly all
+* operations done by usb_gadget_connect(but not calling it).
+* Therefore non composite gadgets (like rndis) works even with
+* wrong connect_count value (old style gadgets don't call
+* usb_gadget_connect/disconnect).
+*
+* On the other hand the CCG (Configurable Composite Gadget)
+* requires this incrementation since it calls
+* usb_gadget_disconnect on its probe (it is allowed) and hence
+* the proper balance is needed when the usb_gadget_connect(i.e.
+* pullup) is called, when triggered from userspace event.
+*/
+   atomic_inc(udc-gadget-connect_count);
ret = usb_gadget_start(udc-gadget, driver, bind);
if (ret)
goto err1;
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 9517466..0801d83 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -534,6 +534,7 @@ struct usb_gadget {
unsignedb_hnp_enable:1;
unsigneda_hnp_support:1;
unsigneda_alt_hnp_support:1;
+   atomic_tconnect_count;
const char  *name;
struct device   dev;
 };
@@ -739,7 +740,11 @@ static inline int usb_gadget_connect(struct usb_gadget 
*gadget)
 {
if (!gadget-ops-pullup)
return -EOPNOTSUPP;
-   return gadget-ops-pullup(gadget, 1);
+
+   if (atomic_inc_return(gadget-connect_count) == 1)
+   return gadget-ops-pullup(gadget, 1);
+
+   return 0;
 }
 
 /**
@@ -761,7 +766,11 @@ static inline int usb_gadget_disconnect(struct usb_gadget 
*gadget)
 {
if (!gadget-ops-pullup)
return -EOPNOTSUPP;
-   return gadget-ops-pullup(gadget, 0);
+
+   if (atomic_dec_and_test(gadget-connect_count))
+   return gadget-ops-pullup(gadget, 0);
+
+   return 0;
 }
 
 
-- 
1.7.2.3

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