* 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