* [PATCH] usb: cdnsp: Fix issue with CV Bad Descriptor test
[not found] <20250617071045.128040-1-pawell@cadence.com>
@ 2025-06-17 7:15 ` Pawel Laszczak
2025-06-17 8:33 ` Peter Chen (CIX)
2025-06-17 23:58 ` kernel test robot
0 siblings, 2 replies; 3+ messages in thread
From: Pawel Laszczak @ 2025-06-17 7:15 UTC (permalink / raw)
To: peter.chen@kernel.org
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org,
Pawel Laszczak
The SSP2 controller has extra endpoint state preserve bit (ESP) which
setting causes that endpoint state will be preserved during
Halt Endpoint command. It is used only for EP0.
Without this bit the Command Verifier "TD 9.10 Bad Descriptor Test"
failed.
Setting this bit doesn't have any impact for SSP controller.
Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
cc: stable@vger.kernel.org
Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
drivers/usb/cdns3/cdnsp-debug.h | 5 +++--
drivers/usb/cdns3/cdnsp-ep0.c | 17 ++++++++++++++---
drivers/usb/cdns3/cdnsp-gadget.h | 3 +++
drivers/usb/cdns3/cdnsp-ring.c | 3 ++-
4 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/cdns3/cdnsp-debug.h b/drivers/usb/cdns3/cdnsp-debug.h
index cd138acdcce1..86860686d836 100644
--- a/drivers/usb/cdns3/cdnsp-debug.h
+++ b/drivers/usb/cdns3/cdnsp-debug.h
@@ -327,12 +327,13 @@ static inline const char *cdnsp_decode_trb(char *str, size_t size, u32 field0,
case TRB_RESET_EP:
case TRB_HALT_ENDPOINT:
ret = scnprintf(str, size,
- "%s: ep%d%s(%d) ctx %08x%08x slot %ld flags %c",
+ "%s: ep%d%s(%d) ctx %08x%08x slot %ld flags %c %c",
cdnsp_trb_type_string(type),
ep_num, ep_id % 2 ? "out" : "in",
TRB_TO_EP_INDEX(field3), field1, field0,
TRB_TO_SLOT_ID(field3),
- field3 & TRB_CYCLE ? 'C' : 'c');
+ field3 & TRB_CYCLE ? 'C' : 'c',
+ field3 & TRB_ESP ? 'P' : 'p');
break;
case TRB_STOP_RING:
ret = scnprintf(str, size,
diff --git a/drivers/usb/cdns3/cdnsp-ep0.c b/drivers/usb/cdns3/cdnsp-ep0.c
index f317d3c84781..567ccfdecded 100644
--- a/drivers/usb/cdns3/cdnsp-ep0.c
+++ b/drivers/usb/cdns3/cdnsp-ep0.c
@@ -414,6 +414,7 @@ static int cdnsp_ep0_std_request(struct cdnsp_device *pdev,
void cdnsp_setup_analyze(struct cdnsp_device *pdev)
{
struct usb_ctrlrequest *ctrl = &pdev->setup;
+ struct cdnsp_ep *pep;
int ret = -EINVAL;
u16 len;
@@ -428,9 +429,19 @@ void cdnsp_setup_analyze(struct cdnsp_device *pdev)
}
/* Restore the ep0 to Stopped/Running state. */
- if (pdev->eps[0].ep_state & EP_HALTED) {
- trace_cdnsp_ep0_halted("Restore to normal state");
- cdnsp_halt_endpoint(pdev, &pdev->eps[0], 0);
+ if (pep->ep_state & EP_HALTED) {
+ /*
+ * Halt Endpoint Command for SSP2 for ep0 preserve current
+ * endpoint state and driver has to synchronise the
+ * software endpointp state with endpoint output context
+ * state.
+ */
+ if (GET_EP_CTX_STATE(pep->out_ctx) == EP_STATE_HALTED) {
+ cdnsp_halt_endpoint(pdev, pep, 0);
+ } else {
+ pep->ep_state &= ~EP_HALTED;
+ pep->ep_state |= EP_STOPPED;
+ }
}
/*
diff --git a/drivers/usb/cdns3/cdnsp-gadget.h b/drivers/usb/cdns3/cdnsp-gadget.h
index 2afa3e558f85..c26abef6e1c9 100644
--- a/drivers/usb/cdns3/cdnsp-gadget.h
+++ b/drivers/usb/cdns3/cdnsp-gadget.h
@@ -987,6 +987,9 @@ enum cdnsp_setup_dev {
#define STREAM_ID_FOR_TRB(p) ((((p)) << 16) & GENMASK(31, 16))
#define SCT_FOR_TRB(p) (((p) << 1) & 0x7)
+/* Halt Endpoint Command TRB field. */
+#define TRB_ESP BIT(9)
+
/* Link TRB specific fields. */
#define TRB_TC BIT(1)
diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
index fd06cb85c4ea..d397d28efc6e 100644
--- a/drivers/usb/cdns3/cdnsp-ring.c
+++ b/drivers/usb/cdns3/cdnsp-ring.c
@@ -2483,7 +2483,8 @@ void cdnsp_queue_halt_endpoint(struct cdnsp_device *pdev, unsigned int ep_index)
{
cdnsp_queue_command(pdev, 0, 0, 0, TRB_TYPE(TRB_HALT_ENDPOINT) |
SLOT_ID_FOR_TRB(pdev->slot_id) |
- EP_ID_FOR_TRB(ep_index));
+ EP_ID_FOR_TRB(ep_index) |
+ (!ep_index ? TRB_ESP : 0));
}
void cdnsp_force_header_wakeup(struct cdnsp_device *pdev, int intf_num)
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: cdnsp: Fix issue with CV Bad Descriptor test
2025-06-17 7:15 ` [PATCH] usb: cdnsp: Fix issue with CV Bad Descriptor test Pawel Laszczak
@ 2025-06-17 8:33 ` Peter Chen (CIX)
2025-06-17 23:58 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: Peter Chen (CIX) @ 2025-06-17 8:33 UTC (permalink / raw)
To: Pawel Laszczak
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
On 25-06-17 07:15:06, Pawel Laszczak wrote:
> The SSP2 controller has extra endpoint state preserve bit (ESP) which
> setting causes that endpoint state will be preserved during
> Halt Endpoint command. It is used only for EP0.
> Without this bit the Command Verifier "TD 9.10 Bad Descriptor Test"
> failed.
> Setting this bit doesn't have any impact for SSP controller.
>
> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> cc: stable@vger.kernel.org
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
> drivers/usb/cdns3/cdnsp-debug.h | 5 +++--
> drivers/usb/cdns3/cdnsp-ep0.c | 17 ++++++++++++++---
> drivers/usb/cdns3/cdnsp-gadget.h | 3 +++
> drivers/usb/cdns3/cdnsp-ring.c | 3 ++-
> 4 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/cdns3/cdnsp-debug.h b/drivers/usb/cdns3/cdnsp-debug.h
> index cd138acdcce1..86860686d836 100644
> --- a/drivers/usb/cdns3/cdnsp-debug.h
> +++ b/drivers/usb/cdns3/cdnsp-debug.h
> @@ -327,12 +327,13 @@ static inline const char *cdnsp_decode_trb(char *str, size_t size, u32 field0,
> case TRB_RESET_EP:
> case TRB_HALT_ENDPOINT:
> ret = scnprintf(str, size,
> - "%s: ep%d%s(%d) ctx %08x%08x slot %ld flags %c",
> + "%s: ep%d%s(%d) ctx %08x%08x slot %ld flags %c %c",
> cdnsp_trb_type_string(type),
> ep_num, ep_id % 2 ? "out" : "in",
> TRB_TO_EP_INDEX(field3), field1, field0,
> TRB_TO_SLOT_ID(field3),
> - field3 & TRB_CYCLE ? 'C' : 'c');
> + field3 & TRB_CYCLE ? 'C' : 'c',
> + field3 & TRB_ESP ? 'P' : 'p');
> break;
> case TRB_STOP_RING:
> ret = scnprintf(str, size,
> diff --git a/drivers/usb/cdns3/cdnsp-ep0.c b/drivers/usb/cdns3/cdnsp-ep0.c
> index f317d3c84781..567ccfdecded 100644
> --- a/drivers/usb/cdns3/cdnsp-ep0.c
> +++ b/drivers/usb/cdns3/cdnsp-ep0.c
> @@ -414,6 +414,7 @@ static int cdnsp_ep0_std_request(struct cdnsp_device *pdev,
> void cdnsp_setup_analyze(struct cdnsp_device *pdev)
> {
> struct usb_ctrlrequest *ctrl = &pdev->setup;
> + struct cdnsp_ep *pep;
> int ret = -EINVAL;
> u16 len;
>
> @@ -428,9 +429,19 @@ void cdnsp_setup_analyze(struct cdnsp_device *pdev)
> }
>
> /* Restore the ep0 to Stopped/Running state. */
> - if (pdev->eps[0].ep_state & EP_HALTED) {
> - trace_cdnsp_ep0_halted("Restore to normal state");
> - cdnsp_halt_endpoint(pdev, &pdev->eps[0], 0);
> + if (pep->ep_state & EP_HALTED) {
> + /*
> + * Halt Endpoint Command for SSP2 for ep0 preserve current
> + * endpoint state and driver has to synchronise the
%s/synchronise/synchronize
> + * software endpointp state with endpoint output context
%s/endpointp/endpoint
> + * state.
> + */
> + if (GET_EP_CTX_STATE(pep->out_ctx) == EP_STATE_HALTED) {
> + cdnsp_halt_endpoint(pdev, pep, 0);
> + } else {
> + pep->ep_state &= ~EP_HALTED;
> + pep->ep_state |= EP_STOPPED;
> + }
> }
>
> /*
> diff --git a/drivers/usb/cdns3/cdnsp-gadget.h b/drivers/usb/cdns3/cdnsp-gadget.h
> index 2afa3e558f85..c26abef6e1c9 100644
> --- a/drivers/usb/cdns3/cdnsp-gadget.h
> +++ b/drivers/usb/cdns3/cdnsp-gadget.h
> @@ -987,6 +987,9 @@ enum cdnsp_setup_dev {
> #define STREAM_ID_FOR_TRB(p) ((((p)) << 16) & GENMASK(31, 16))
> #define SCT_FOR_TRB(p) (((p) << 1) & 0x7)
>
> +/* Halt Endpoint Command TRB field. */
> +#define TRB_ESP BIT(9)
> +
Please add comment it is specific for SSP2.
Peter
> /* Link TRB specific fields. */
> #define TRB_TC BIT(1)
>
> diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
> index fd06cb85c4ea..d397d28efc6e 100644
> --- a/drivers/usb/cdns3/cdnsp-ring.c
> +++ b/drivers/usb/cdns3/cdnsp-ring.c
> @@ -2483,7 +2483,8 @@ void cdnsp_queue_halt_endpoint(struct cdnsp_device *pdev, unsigned int ep_index)
> {
> cdnsp_queue_command(pdev, 0, 0, 0, TRB_TYPE(TRB_HALT_ENDPOINT) |
> SLOT_ID_FOR_TRB(pdev->slot_id) |
> - EP_ID_FOR_TRB(ep_index));
> + EP_ID_FOR_TRB(ep_index) |
> + (!ep_index ? TRB_ESP : 0));
> }
>
> void cdnsp_force_header_wakeup(struct cdnsp_device *pdev, int intf_num)
> --
> 2.43.0
>
--
Best regards,
Peter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: cdnsp: Fix issue with CV Bad Descriptor test
2025-06-17 7:15 ` [PATCH] usb: cdnsp: Fix issue with CV Bad Descriptor test Pawel Laszczak
2025-06-17 8:33 ` Peter Chen (CIX)
@ 2025-06-17 23:58 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2025-06-17 23:58 UTC (permalink / raw)
To: Pawel Laszczak, peter.chen@kernel.org
Cc: llvm, oe-kbuild-all, gregkh@linuxfoundation.org,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, Pawel Laszczak
Hi Pawel,
kernel test robot noticed the following build warnings:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus linus/master v6.16-rc2 next-20250617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Pawel-Laszczak/usb-cdnsp-Fix-issue-with-CV-Bad-Descriptor-test/20250617-155848
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/PH7PR07MB95388A30AE3E50D42E0592CADD73A%40PH7PR07MB9538.namprd07.prod.outlook.com
patch subject: [PATCH] usb: cdnsp: Fix issue with CV Bad Descriptor test
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20250618/202506180958.aX9DgCOG-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250618/202506180958.aX9DgCOG-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506180958.aX9DgCOG-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/usb/cdns3/cdnsp-ep0.c:432:6: warning: variable 'pep' is uninitialized when used here [-Wuninitialized]
432 | if (pep->ep_state & EP_HALTED) {
| ^~~
drivers/usb/cdns3/cdnsp-ep0.c:417:22: note: initialize the variable 'pep' to silence this warning
417 | struct cdnsp_ep *pep;
| ^
| = NULL
1 warning generated.
vim +/pep +432 drivers/usb/cdns3/cdnsp-ep0.c
413
414 void cdnsp_setup_analyze(struct cdnsp_device *pdev)
415 {
416 struct usb_ctrlrequest *ctrl = &pdev->setup;
417 struct cdnsp_ep *pep;
418 int ret = -EINVAL;
419 u16 len;
420
421 trace_cdnsp_ctrl_req(ctrl);
422
423 if (!pdev->gadget_driver)
424 goto out;
425
426 if (pdev->gadget.state == USB_STATE_NOTATTACHED) {
427 dev_err(pdev->dev, "ERR: Setup detected in unattached state\n");
428 goto out;
429 }
430
431 /* Restore the ep0 to Stopped/Running state. */
> 432 if (pep->ep_state & EP_HALTED) {
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-06-17 23:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250617071045.128040-1-pawell@cadence.com>
2025-06-17 7:15 ` [PATCH] usb: cdnsp: Fix issue with CV Bad Descriptor test Pawel Laszczak
2025-06-17 8:33 ` Peter Chen (CIX)
2025-06-17 23:58 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox