Received-SPF: pass (google.com: domain of linux-kernel+bounces-66352-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 15 Feb 2024 15:16:13 +0800
From: Peter Chen <peter.chen@kernel.org>
To: Pawel Laszczak <pawell@cadence.com>
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org, peter.chen@cixtech.com,
	stable@vger.kernel.org
Subject: Re: [PATCH] usb: cdnsp: fixed issue with incorrect detecting CDNSP
 family controllers
Message-ID: <20240215071613.GA1256251@nchen-desktop>
References: <20240206104325.55456-1-pawell@cadence.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240206104325.55456-1-pawell@cadence.com>

On 24-02-06 11:43:25, Pawel Laszczak wrote:
> Cadence have several controllers from 0x000403xx family but current
> driver suuport detecting only one with DID equal 0x0004034E.
> It causes that if someone use different CDNSP controller then driver

%s/use/uses

> will use incorrect version and register space.
> Patch fix this issue.
> 
> cc: <stable@vger.kernel.org>
> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
>  drivers/usb/cdns3/core.c |  1 -
>  drivers/usb/cdns3/drd.c  | 13 +++++++++----
>  drivers/usb/cdns3/drd.h  |  6 +++++-
>  3 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index 33548771a0d3..465e9267b49c 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -395,7 +395,6 @@ static int cdns_role_set(struct usb_role_switch *sw, enum usb_role role)
>  	return ret;
>  }
>  
> -
>  /**
>   * cdns_wakeup_irq - interrupt handler for wakeup events
>   * @irq: irq number for cdns3/cdnsp core device
> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
> index 04b6d12f2b9a..ee917f1b091c 100644
> --- a/drivers/usb/cdns3/drd.c
> +++ b/drivers/usb/cdns3/drd.c
> @@ -156,7 +156,8 @@ bool cdns_is_device(struct cdns *cdns)
>   */
>  static void cdns_otg_disable_irq(struct cdns *cdns)
>  {
> -	writel(0, &cdns->otg_irq_regs->ien);
> +	if (cdns->version)
> +		writel(0, &cdns->otg_irq_regs->ien);
>  }
>  
>  /**
> @@ -422,15 +423,20 @@ int cdns_drd_init(struct cdns *cdns)
>  
>  		cdns->otg_regs = (void __iomem *)&cdns->otg_v1_regs->cmd;
>  
> -		if (readl(&cdns->otg_cdnsp_regs->did) == OTG_CDNSP_DID) {
> +		state = readl(&cdns->otg_cdnsp_regs->did);

Use a meaningful variable.

> +
> +		if (OTG_CDNSP_CHECK_DID(state)) {
>  			cdns->otg_irq_regs = (struct cdns_otg_irq_regs __iomem *)
>  					      &cdns->otg_cdnsp_regs->ien;
>  			cdns->version  = CDNSP_CONTROLLER_V2;
> -		} else {
> +		} else if (OTG_CDNS3_CHECK_DID(state)) {
>  			cdns->otg_irq_regs = (struct cdns_otg_irq_regs __iomem *)
>  					      &cdns->otg_v1_regs->ien;
>  			writel(1, &cdns->otg_v1_regs->simulate);
>  			cdns->version  = CDNS3_CONTROLLER_V1;
> +		} else {
> +			dev_err(cdns->dev, "not supporte DID=0x%08x\n", state);
> +			return -EINVAL;
>  		}
>  
>  		dev_dbg(cdns->dev, "DRD version v1 (ID: %08x, rev: %08x)\n",
> @@ -483,7 +489,6 @@ int cdns_drd_exit(struct cdns *cdns)
>  	return 0;
>  }
>  
> -
>  /* Indicate the cdns3 core was power lost before */
>  bool cdns_power_is_lost(struct cdns *cdns)
>  {
> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
> index cbdf94f73ed9..d72370c321d3 100644
> --- a/drivers/usb/cdns3/drd.h
> +++ b/drivers/usb/cdns3/drd.h
> @@ -79,7 +79,11 @@ struct cdnsp_otg_regs {
>  	__le32 susp_timing_ctrl;
>  };
>  
> -#define OTG_CDNSP_DID	0x0004034E
> +/* CDNSP driver supports 0x000403xx Cadence USB controller family. */
> +#define OTG_CDNSP_CHECK_DID(did) (((did) & GENMASK(31, 8)) == 0x00040300)

GENMASK(19, 8)?

> +
> +/* CDNS3 driver supports 0x000402xx Cadence USB controller family. */
> +#define OTG_CDNS3_CHECK_DID(did) (((did) & GENMASK(31, 8)) == 0x00040200)
>  
>  /*
>   * Common registers interface for both CDNS3 and CDNSP version of DRD.
> -- 
> 2.37.2
> 

-- 

Thanks,
Peter Chen