All of lore.kernel.org
 help / color / mirror / Atom feed
From: mkl@pengutronix.de (Marc Kleine-Budde)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 09/15] usb: phy-mxs: Enable IC fixes for related SoCs
Date: Mon, 09 Dec 2013 09:38:17 +0100	[thread overview]
Message-ID: <52A58179.2050702@pengutronix.de> (raw)
In-Reply-To: <1386570664-6713-10-git-send-email-peter.chen@freescale.com>

On 12/09/2013 07:30 AM, Peter Chen wrote:
> Some PHY bugs are fixed by IC logic, but these bits are not
> enabled by default, so we enable them at driver.

Which bugs are fixed by enabling this bit? Is it only suspend related?
Can you document them or better add a pointer to the documentation.

Further I don't like the idea of adding code, or enabling a feature on
certain hardware, that is broken in the first place and fixing it in a
later patch. Think about squashing it into the correct patch.

> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  drivers/usb/phy/phy-mxs-usb.c |   28 ++++++++++++++++++++++++++--
>  1 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index e3df53f..d1c319b 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -31,6 +31,10 @@
>  #define HW_USBPHY_CTRL_SET			0x34
>  #define HW_USBPHY_CTRL_CLR			0x38
>  
> +#define HW_USBPHY_IP				0x90
> +#define HW_USBPHY_IP_SET			0x94
> +#define HW_USBPHY_IP_CLR			0x98
> +
>  #define BM_USBPHY_CTRL_SFTRST			BIT(31)
>  #define BM_USBPHY_CTRL_CLKGATE			BIT(30)
>  #define BM_USBPHY_CTRL_ENAUTOSET_USBCLKS	BIT(26)
> @@ -42,6 +46,8 @@
>  #define BM_USBPHY_CTRL_ENUTMILEVEL2		BIT(14)
>  #define BM_USBPHY_CTRL_ENHOSTDISCONDETECT	BIT(1)
>  
> +#define BM_USBPHY_IP_FIX                       (BIT(17) | BIT(18))
> +
>  #define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
>  
>  /* Do disconnection between PHY and controller without vbus */
> @@ -63,6 +69,9 @@
>  /* The SoCs who have anatop module */
>  #define MXS_PHY_HAS_ANATOP			BIT(3)
>  
> +/* IC has bug fixes logic */
> +#define MXS_PHY_NEED_IP_FIX			BIT(4)
> +
>  struct mxs_phy_data {
>  	unsigned int flags;
>  };
> @@ -74,12 +83,14 @@ static const struct mxs_phy_data imx23_phy_data = {
>  static const struct mxs_phy_data imx6q_phy_data = {
>  	.flags = MXS_PHY_SENDING_SOF_TOO_FAST |
>  		MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
> -		MXS_PHY_HAS_ANATOP,
> +		MXS_PHY_HAS_ANATOP |
> +		MXS_PHY_NEED_IP_FIX,
>  };
>  
>  static const struct mxs_phy_data imx6sl_phy_data = {
>  	.flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
> -		MXS_PHY_HAS_ANATOP,
> +		MXS_PHY_HAS_ANATOP |
> +		MXS_PHY_NEED_IP_FIX,
>  };
>  
>  static const struct of_device_id mxs_phy_dt_ids[] = {
> @@ -97,6 +108,16 @@ struct mxs_phy {
>  	struct regmap *regmap_anatop;
>  };
>  
> +static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
> +{
> +	return mxs_phy->data == &imx6q_phy_data;
> +}
> +
> +static inline bool is_imx6sl_phy(struct mxs_phy *mxs_phy)
> +{
> +	return mxs_phy->data == &imx6sl_phy_data;
> +}

Are the two is_imx6* functions still needed?

> +
>  static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
>  {
>  	int ret;
> @@ -123,6 +144,9 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
>  		BM_USBPHY_CTRL_ENUTMILEVEL3,
>  	       base + HW_USBPHY_CTRL_SET);
>  
> +	if (mxs_phy->data->flags & MXS_PHY_NEED_IP_FIX)
> +		writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
> +
>  	return 0;
>  }

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 259 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131209/9242f466/attachment.sig>

WARNING: multiple messages have this Message-ID (diff)
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Peter Chen <peter.chen@freescale.com>
Cc: balbi@ti.com, shawn.guo@linaro.org, rob.herring@calxeda.com,
	grant.likely@linaro.org, alexander.shishkin@linux.intel.com,
	linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	festevam@gmail.com, marex@denx.de, kernel@pengutronix.de,
	m.grzeschik@pengutronix.de, frank.li@freescale.com,
	gregkh@linuxfoundation.org, devicetree@vger.kernel.org,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH v5 09/15] usb: phy-mxs: Enable IC fixes for related SoCs
Date: Mon, 09 Dec 2013 09:38:17 +0100	[thread overview]
Message-ID: <52A58179.2050702@pengutronix.de> (raw)
In-Reply-To: <1386570664-6713-10-git-send-email-peter.chen@freescale.com>

[-- Attachment #1: Type: text/plain, Size: 3445 bytes --]

On 12/09/2013 07:30 AM, Peter Chen wrote:
> Some PHY bugs are fixed by IC logic, but these bits are not
> enabled by default, so we enable them at driver.

Which bugs are fixed by enabling this bit? Is it only suspend related?
Can you document them or better add a pointer to the documentation.

Further I don't like the idea of adding code, or enabling a feature on
certain hardware, that is broken in the first place and fixing it in a
later patch. Think about squashing it into the correct patch.

> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  drivers/usb/phy/phy-mxs-usb.c |   28 ++++++++++++++++++++++++++--
>  1 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index e3df53f..d1c319b 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -31,6 +31,10 @@
>  #define HW_USBPHY_CTRL_SET			0x34
>  #define HW_USBPHY_CTRL_CLR			0x38
>  
> +#define HW_USBPHY_IP				0x90
> +#define HW_USBPHY_IP_SET			0x94
> +#define HW_USBPHY_IP_CLR			0x98
> +
>  #define BM_USBPHY_CTRL_SFTRST			BIT(31)
>  #define BM_USBPHY_CTRL_CLKGATE			BIT(30)
>  #define BM_USBPHY_CTRL_ENAUTOSET_USBCLKS	BIT(26)
> @@ -42,6 +46,8 @@
>  #define BM_USBPHY_CTRL_ENUTMILEVEL2		BIT(14)
>  #define BM_USBPHY_CTRL_ENHOSTDISCONDETECT	BIT(1)
>  
> +#define BM_USBPHY_IP_FIX                       (BIT(17) | BIT(18))
> +
>  #define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
>  
>  /* Do disconnection between PHY and controller without vbus */
> @@ -63,6 +69,9 @@
>  /* The SoCs who have anatop module */
>  #define MXS_PHY_HAS_ANATOP			BIT(3)
>  
> +/* IC has bug fixes logic */
> +#define MXS_PHY_NEED_IP_FIX			BIT(4)
> +
>  struct mxs_phy_data {
>  	unsigned int flags;
>  };
> @@ -74,12 +83,14 @@ static const struct mxs_phy_data imx23_phy_data = {
>  static const struct mxs_phy_data imx6q_phy_data = {
>  	.flags = MXS_PHY_SENDING_SOF_TOO_FAST |
>  		MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
> -		MXS_PHY_HAS_ANATOP,
> +		MXS_PHY_HAS_ANATOP |
> +		MXS_PHY_NEED_IP_FIX,
>  };
>  
>  static const struct mxs_phy_data imx6sl_phy_data = {
>  	.flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
> -		MXS_PHY_HAS_ANATOP,
> +		MXS_PHY_HAS_ANATOP |
> +		MXS_PHY_NEED_IP_FIX,
>  };
>  
>  static const struct of_device_id mxs_phy_dt_ids[] = {
> @@ -97,6 +108,16 @@ struct mxs_phy {
>  	struct regmap *regmap_anatop;
>  };
>  
> +static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
> +{
> +	return mxs_phy->data == &imx6q_phy_data;
> +}
> +
> +static inline bool is_imx6sl_phy(struct mxs_phy *mxs_phy)
> +{
> +	return mxs_phy->data == &imx6sl_phy_data;
> +}

Are the two is_imx6* functions still needed?

> +
>  static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
>  {
>  	int ret;
> @@ -123,6 +144,9 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
>  		BM_USBPHY_CTRL_ENUTMILEVEL3,
>  	       base + HW_USBPHY_CTRL_SET);
>  
> +	if (mxs_phy->data->flags & MXS_PHY_NEED_IP_FIX)
> +		writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
> +
>  	return 0;
>  }

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

  reply	other threads:[~2013-12-09  8:38 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-09  6:30 [PATCH v5 00/15] Add power management support for mxs phy Peter Chen
2013-12-09  6:30 ` Peter Chen
2013-12-09  6:30 ` [PATCH v5 01/15] usb: doc: phy-mxs: Add more compatible strings Peter Chen
2013-12-09  6:30   ` Peter Chen
2013-12-09  6:30 ` [PATCH v5 02/15] usb: phy-mxs: Add platform judgement code Peter Chen
2013-12-09  6:30   ` Peter Chen
2013-12-09  6:30 ` [PATCH v5 03/15] usb: phy-mxs: Add auto clock and power setting Peter Chen
2013-12-09  6:30   ` Peter Chen
2013-12-09  6:30 ` [PATCH v5 04/15] usb: doc: phy-mxs: update binding for adding anatop phandle Peter Chen
2013-12-09  6:30   ` Peter Chen
2013-12-09  6:30 ` [PATCH v5 05/15] ARM: dts: imx6: add anatop phandle for usbphy Peter Chen
2013-12-09  6:30   ` Peter Chen
2013-12-09  6:30 ` [PATCH v5 06/15] usb: phy-mxs: Add anatop regmap Peter Chen
2013-12-09  6:30   ` Peter Chen
2013-12-09  6:30 ` [PATCH v5 07/15] usb: phy: add notify suspend and resume callback Peter Chen
2013-12-09  6:30   ` Peter Chen
2013-12-09  6:30 ` [PATCH v5 08/15] usb: phy-mxs: Add implementation of nofity_suspend and notify_resume Peter Chen
2013-12-09  6:30   ` Peter Chen
2013-12-09  8:31   ` Marc Kleine-Budde
2013-12-09  8:31     ` Marc Kleine-Budde
2013-12-09  9:00     ` Peter Chen
2013-12-09  9:00       ` Peter Chen
2013-12-09  6:30 ` [PATCH v5 09/15] usb: phy-mxs: Enable IC fixes for related SoCs Peter Chen
2013-12-09  6:30   ` Peter Chen
2013-12-09  8:38   ` Marc Kleine-Budde [this message]
2013-12-09  8:38     ` Marc Kleine-Budde
2013-12-09  9:07     ` Peter Chen
2013-12-09  9:07       ` Peter Chen
2013-12-09  9:52       ` Marc Kleine-Budde
2013-12-09  9:52         ` Marc Kleine-Budde
2013-12-10  1:45         ` Peter Chen
2013-12-10  1:45           ` Peter Chen
2013-12-09  6:30 ` [PATCH v5 10/15] ARM: dts: imx: add mxs phy controller id Peter Chen
2013-12-09  6:30   ` Peter Chen
2013-12-09  6:31 ` [PATCH v5 11/15] usb: phy-mxs: add " Peter Chen
2013-12-09  6:31   ` Peter Chen
2013-12-09  6:31 ` [PATCH v5 12/15] usb: phy: Add set_wakeup API Peter Chen
2013-12-09  6:31   ` Peter Chen
2013-12-09  8:41   ` Marc Kleine-Budde
2013-12-09  8:41     ` Marc Kleine-Budde
2013-12-09  9:12     ` Peter Chen
2013-12-09  9:12       ` Peter Chen
2013-12-09  6:31 ` [PATCH v5 13/15] usb: phy-mxs: Add implementation of set_wakeup Peter Chen
2013-12-09  6:31   ` Peter Chen
2013-12-09  6:31 ` [PATCH v5 14/15] usb: phy-mxs: Add system suspend/resume API Peter Chen
2013-12-09  6:31   ` Peter Chen
2013-12-09  6:31 ` [PATCH v5 15/15] usb: phy-mxs: Add sync time after controller clear phcd Peter Chen
2013-12-09  6:31   ` Peter Chen
2013-12-09  9:05   ` Marc Kleine-Budde
2013-12-09  9:05     ` Marc Kleine-Budde
2013-12-09  9:19     ` Peter Chen
2013-12-09  9:19       ` Peter Chen
2013-12-09 10:00       ` Marc Kleine-Budde
2013-12-09 10:00         ` Marc Kleine-Budde
2013-12-09 18:20   ` Sergei Shtylyov
2013-12-09 18:20     ` Sergei Shtylyov
2013-12-10  1:47     ` Peter Chen
2013-12-10  1:47       ` Peter Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52A58179.2050702@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.