| Message ID | 20230321210521.2806486-1-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] usb: cdns3: fix super speed mass storage gadget device failure at imx8qm | expand |
> >The mass storage gadget has one IN and one OUT endpoint. The below (Fixes) >commit aimed to utilize all hardware FIFO to support composited gadget >devices. This resulted in an ep_buf_size of 15 when a single gadget was >enabled, such as the mass storage gadget. > >However, it was found that there are unknown limitations on the imx8qm and >imx8qxp B0 platforms. The device would fail to work if ep_buf_size exceeded >9. > >To resolve this issue, this patch reverts to the old settings used before the >below commit for the imx8qm and imx8qxp B0 platforms. > >Fixes: dce49449e04f ("usb: cdns3: allocate TX FIFO size according to >composite EP number") > >Signed-off-by: Frank Li <Frank.Li@nxp.com> >--- > >I hope cdns engineer, such as pawell@cadence.com help identfy the root >cause. >Look like old version ip use more memory then what ep_cfg show. > > drivers/usb/cdns3/cdns3-gadget.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > >diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3- >gadget.c >index 5adcb349718c..497c8e87dabf 100644 >--- a/drivers/usb/cdns3/cdns3-gadget.c >+++ b/drivers/usb/cdns3/cdns3-gadget.c >@@ -3005,6 +3005,17 @@ static int cdns3_gadget_check_config(struct >usb_gadget *gadget) > priv_dev->ep_buf_size = priv_dev->ep_iso_burst = > (priv_dev->onchip_buffers - 2) / (n_in + 1); > >+ /* >+ * There are unknown hardware limition: when work at super speed >mode, >+ * ep_buffer_size can't bigger than 9 for one IN and OUT case at >i.MX8QM >+ * and i.MX8QXP B0, which report there are 32k memory. >+ * Rollback to original settings for the these chipes. >+ */ >+ if (priv_dev->dev_ver < DEV_VER_V2) { >+ priv_dev->ep_buf_size = min_t(u8, priv_dev->ep_buf_size, 4); >+ priv_dev->ep_iso_burst = min_t(u8, priv_dev->ep_iso_burst, >3); >+ } >+ > return 0; > } I'm not sure whether you have 32KB. I remember that you had a Soc which have only 18KB on-chip memory and value in usb_cap2 was incorrect. It was the reason why the on-chip-buff-size property has been added to driver. Please confirm that you have 32KB, then I will recreate such test on my testing board. Regards, Pawel > >-- >2.34.1
>>The mass storage gadget has one IN and one OUT endpoint. The below >>(Fixes) commit aimed to utilize all hardware FIFO to support composited >>gadget devices. This resulted in an ep_buf_size of 15 when a single >>gadget was enabled, such as the mass storage gadget. >> >>However, it was found that there are unknown limitations on the imx8qm >>and imx8qxp B0 platforms. The device would fail to work if ep_buf_size >>exceeded 9. >> >>To resolve this issue, this patch reverts to the old settings used >>before the below commit for the imx8qm and imx8qxp B0 platforms. >> >>Fixes: dce49449e04f ("usb: cdns3: allocate TX FIFO size according to >>composite EP number") >> >>Signed-off-by: Frank Li <Frank.Li@nxp.com> >>--- >> >>I hope cdns engineer, such as pawell@cadence.com help identfy the root >>cause. >>Look like old version ip use more memory then what ep_cfg show. >> >> drivers/usb/cdns3/cdns3-gadget.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >>diff --git a/drivers/usb/cdns3/cdns3-gadget.c >>b/drivers/usb/cdns3/cdns3- gadget.c index 5adcb349718c..497c8e87dabf >>100644 >>--- a/drivers/usb/cdns3/cdns3-gadget.c >>+++ b/drivers/usb/cdns3/cdns3-gadget.c >>@@ -3005,6 +3005,17 @@ static int cdns3_gadget_check_config(struct >>usb_gadget *gadget) >> priv_dev->ep_buf_size = priv_dev->ep_iso_burst = >> (priv_dev->onchip_buffers - 2) / (n_in + 1); >> >>+ /* >>+ * There are unknown hardware limition: when work at super speed >>mode, >>+ * ep_buffer_size can't bigger than 9 for one IN and OUT case at >>i.MX8QM >>+ * and i.MX8QXP B0, which report there are 32k memory. >>+ * Rollback to original settings for the these chipes. >>+ */ >>+ if (priv_dev->dev_ver < DEV_VER_V2) { >>+ priv_dev->ep_buf_size = min_t(u8, priv_dev->ep_buf_size, 4); >>+ priv_dev->ep_iso_burst = min_t(u8, priv_dev->ep_iso_burst, >>3); >>+ } >>+ >> return 0; >> } > >I'm not sure whether you have 32KB. I remember that you had a Soc which >have only 18KB on-chip memory and value in usb_cap2 was incorrect. >It was the reason why the on-chip-buff-size property has been added to >driver. > >Please confirm that you have 32KB, then I will recreate such test on my testing >board. > I've made the MSC test with the following endpoint configuration; [ 6732.999537] cdns-usb3 cdns-usb3.1: Configure ep1in: with val 74000e05 [ 6732.999565] cdns-usb3 cdns-usb3.1: Configure ep1out: with val 74000e15 So, I used the 14 buffers per endpoint and run bonnie++ as tester. I didn't find any issue. Regards, Pawel >> >>-- >>2.34.1
> -----Original Message----- > From: Pawel Laszczak <pawell@cadence.com> > Sent: Wednesday, March 22, 2023 5:56 AM > To: Frank Li <frank.li@nxp.com>; Peter Chen <peter.chen@kernel.org>; > Roger Quadros <rogerq@kernel.org>; Aswath Govindraju <a- > govindraju@ti.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; > open list:CADENCE USB3 DRD IP DRIVER <linux-usb@vger.kernel.org>; open > list <linux-kernel@vger.kernel.org> > Cc: imx@lists.linux.dev > Subject: [EXT] RE: [PATCH 1/1] usb: cdns3: fix super speed mass storage > gadget device failure at imx8qm > > Caution: EXT Email > > >>The mass storage gadget has one IN and one OUT endpoint. The below > >>(Fixes) commit aimed to utilize all hardware FIFO to support composited > >>gadget devices. This resulted in an ep_buf_size of 15 when a single > >>gadget was enabled, such as the mass storage gadget. > >> > >>However, it was found that there are unknown limitations on the imx8qm > >>and imx8qxp B0 platforms. The device would fail to work if ep_buf_size > >>exceeded 9. > >> > >>To resolve this issue, this patch reverts to the old settings used > >>before the below commit for the imx8qm and imx8qxp B0 platforms. > >> > >>Fixes: dce49449e04f ("usb: cdns3: allocate TX FIFO size according to > >>composite EP number") > >> > >>Signed-off-by: Frank Li <Frank.Li@nxp.com> > >>--- > >> > >>I hope cdns engineer, such as pawell@cadence.com help identfy the root > >>cause. > >>Look like old version ip use more memory then what ep_cfg show. > >> > >> drivers/usb/cdns3/cdns3-gadget.c | 11 +++++++++++ > >> 1 file changed, 11 insertions(+) > >> > >>diff --git a/drivers/usb/cdns3/cdns3-gadget.c > >>b/drivers/usb/cdns3/cdns3- gadget.c index 5adcb349718c..497c8e87dabf > >>100644 > >>--- a/drivers/usb/cdns3/cdns3-gadget.c > >>+++ b/drivers/usb/cdns3/cdns3-gadget.c > >>@@ -3005,6 +3005,17 @@ static int cdns3_gadget_check_config(struct > >>usb_gadget *gadget) > >> priv_dev->ep_buf_size = priv_dev->ep_iso_burst = > >> (priv_dev->onchip_buffers - 2) / (n_in + 1); > >> > >>+ /* > >>+ * There are unknown hardware limition: when work at super speed > >>mode, > >>+ * ep_buffer_size can't bigger than 9 for one IN and OUT case at > >>i.MX8QM > >>+ * and i.MX8QXP B0, which report there are 32k memory. > >>+ * Rollback to original settings for the these chipes. > >>+ */ > >>+ if (priv_dev->dev_ver < DEV_VER_V2) { > >>+ priv_dev->ep_buf_size = min_t(u8, priv_dev->ep_buf_size, 4); > >>+ priv_dev->ep_iso_burst = min_t(u8, priv_dev->ep_iso_burst, > >>3); > >>+ } > >>+ > >> return 0; > >> } > > > >I'm not sure whether you have 32KB. I remember that you had a Soc which > >have only 18KB on-chip memory and value in usb_cap2 was incorrect. > >It was the reason why the on-chip-buff-size property has been added to > >driver. > > > >Please confirm that you have 32KB, then I will recreate such test on my > testing > >board. > > > > I've made the MSC test with the following endpoint configuration; > [ 6732.999537] cdns-usb3 cdns-usb3.1: Configure ep1in: with val 74000e05 > [ 6732.999565] cdns-usb3 cdns-usb3.1: Configure ep1out: with val 74000e15 > > So, I used the 14 buffers per endpoint and run bonnie++ as tester. > I didn't find any issue. Problem only happen at QM and QXP B0 and need work at *super speed* mode. Enumerate will be fail and continues reset. Highspeed mode work. I suspect 32KB problem because QXP C0 report is 18K. It doesn't make sense to reduce to 18K from 32k at QXP C0. But it work when IN/OUT is 9K. 9+9+2k(EP0) should bigger than 18K. Let me try Check IC design what's exact number in QM and QXPB0. > > Regards, > Pawel > >> > >>-- > >>2.34.1
diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c index 5adcb349718c..497c8e87dabf 100644 --- a/drivers/usb/cdns3/cdns3-gadget.c +++ b/drivers/usb/cdns3/cdns3-gadget.c @@ -3005,6 +3005,17 @@ static int cdns3_gadget_check_config(struct usb_gadget *gadget) priv_dev->ep_buf_size = priv_dev->ep_iso_burst = (priv_dev->onchip_buffers - 2) / (n_in + 1); + /* + * There are unknown hardware limition: when work at super speed mode, + * ep_buffer_size can't bigger than 9 for one IN and OUT case at i.MX8QM + * and i.MX8QXP B0, which report there are 32k memory. + * Rollback to original settings for the these chipes. + */ + if (priv_dev->dev_ver < DEV_VER_V2) { + priv_dev->ep_buf_size = min_t(u8, priv_dev->ep_buf_size, 4); + priv_dev->ep_iso_burst = min_t(u8, priv_dev->ep_iso_burst, 3); + } + return 0; }
The mass storage gadget has one IN and one OUT endpoint. The below (Fixes) commit aimed to utilize all hardware FIFO to support composited gadget devices. This resulted in an ep_buf_size of 15 when a single gadget was enabled, such as the mass storage gadget. However, it was found that there are unknown limitations on the imx8qm and imx8qxp B0 platforms. The device would fail to work if ep_buf_size exceeded 9. To resolve this issue, this patch reverts to the old settings used before the below commit for the imx8qm and imx8qxp B0 platforms. Fixes: dce49449e04f ("usb: cdns3: allocate TX FIFO size according to composite EP number") Signed-off-by: Frank Li <Frank.Li@nxp.com> --- I hope cdns engineer, such as pawell@cadence.com help identfy the root cause. Look like old version ip use more memory then what ep_cfg show. drivers/usb/cdns3/cdns3-gadget.c | 11 +++++++++++ 1 file changed, 11 insertions(+)