All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: yuantian.tang@nxp.com
Cc: tj@kernel.org, linux-ide@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/2] ahci: qoriq: report warning when ecc register is missing
Date: Wed, 16 Nov 2016 08:56:13 -0700	[thread overview]
Message-ID: <20161116155613.GA1228@linaro.org> (raw)
In-Reply-To: <1479265879-48840-2-git-send-email-yuantian.tang@nxp.com>

On Wed, Nov 16, 2016 at 11:11:19AM +0800, yuantian.tang@nxp.com wrote:
> From: Tang Yuantian <Yuantian.Tang@nxp.com>
> 
> For ls1021a and ls1046a socs, sata ecc must be disabled.
> If ecc register is not found in sata node in dts, report
> a warning.

Hi Yuantian,

What happens if sata ecc is _not_ disaled on those socs?  Can the driver still
work?  If not then it is probably a better idea to return an error code that can
prevent the driver from initialising.

Thanks,
Mathieu

> 
> Signed-off-by: Tang Yuantian <yuantian.tang@nxp.com>
> ---
>  drivers/ata/ahci_qoriq.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/ata/ahci_qoriq.c b/drivers/ata/ahci_qoriq.c
> index 45c88de..66eb4b5 100644
> --- a/drivers/ata/ahci_qoriq.c
> +++ b/drivers/ata/ahci_qoriq.c
> @@ -158,6 +158,7 @@ static int ahci_qoriq_phy_init(struct ahci_host_priv *hpriv)
>  
>  	switch (qpriv->type) {
>  	case AHCI_LS1021A:
> +		WARN_ON(!qpriv->ecc_addr);
>  		writel(SATA_ECC_DISABLE, qpriv->ecc_addr);
>  		writel(AHCI_PORT_PHY_1_CFG, reg_base + PORT_PHY1);
>  		writel(LS1021A_PORT_PHY2, reg_base + PORT_PHY2);
> @@ -185,6 +186,7 @@ static int ahci_qoriq_phy_init(struct ahci_host_priv *hpriv)
>  		break;
>  
>  	case AHCI_LS1046A:
> +		WARN_ON(!qpriv->ecc_addr);
>  		writel(LS1046A_SATA_ECC_DIS, qpriv->ecc_addr);
>  		writel(AHCI_PORT_PHY_1_CFG, reg_base + PORT_PHY1);
>  		writel(AHCI_PORT_TRANS_CFG, reg_base + PORT_TRANS);
> -- 
> 2.1.0.27.g96db324
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: mathieu.poirier@linaro.org (Mathieu Poirier)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] ahci: qoriq: report warning when ecc register is missing
Date: Wed, 16 Nov 2016 08:56:13 -0700	[thread overview]
Message-ID: <20161116155613.GA1228@linaro.org> (raw)
In-Reply-To: <1479265879-48840-2-git-send-email-yuantian.tang@nxp.com>

On Wed, Nov 16, 2016 at 11:11:19AM +0800, yuantian.tang at nxp.com wrote:
> From: Tang Yuantian <Yuantian.Tang@nxp.com>
> 
> For ls1021a and ls1046a socs, sata ecc must be disabled.
> If ecc register is not found in sata node in dts, report
> a warning.

Hi Yuantian,

What happens if sata ecc is _not_ disaled on those socs?  Can the driver still
work?  If not then it is probably a better idea to return an error code that can
prevent the driver from initialising.

Thanks,
Mathieu

> 
> Signed-off-by: Tang Yuantian <yuantian.tang@nxp.com>
> ---
>  drivers/ata/ahci_qoriq.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/ata/ahci_qoriq.c b/drivers/ata/ahci_qoriq.c
> index 45c88de..66eb4b5 100644
> --- a/drivers/ata/ahci_qoriq.c
> +++ b/drivers/ata/ahci_qoriq.c
> @@ -158,6 +158,7 @@ static int ahci_qoriq_phy_init(struct ahci_host_priv *hpriv)
>  
>  	switch (qpriv->type) {
>  	case AHCI_LS1021A:
> +		WARN_ON(!qpriv->ecc_addr);
>  		writel(SATA_ECC_DISABLE, qpriv->ecc_addr);
>  		writel(AHCI_PORT_PHY_1_CFG, reg_base + PORT_PHY1);
>  		writel(LS1021A_PORT_PHY2, reg_base + PORT_PHY2);
> @@ -185,6 +186,7 @@ static int ahci_qoriq_phy_init(struct ahci_host_priv *hpriv)
>  		break;
>  
>  	case AHCI_LS1046A:
> +		WARN_ON(!qpriv->ecc_addr);
>  		writel(LS1046A_SATA_ECC_DIS, qpriv->ecc_addr);
>  		writel(AHCI_PORT_PHY_1_CFG, reg_base + PORT_PHY1);
>  		writel(AHCI_PORT_TRANS_CFG, reg_base + PORT_TRANS);
> -- 
> 2.1.0.27.g96db324
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2016-11-16 15:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-16  3:11 [PATCH 1/2] ahci: qoriq: added a condition to enable dma coherence yuantian.tang
2016-11-16  3:11 ` yuantian.tang
2016-11-16  3:11 ` yuantian.tang at nxp.com
2016-11-16  3:11 ` [PATCH 2/2] ahci: qoriq: report warning when ecc register is missing yuantian.tang
2016-11-16  3:11   ` yuantian.tang
2016-11-16  3:11   ` yuantian.tang at nxp.com
2016-11-16 15:56   ` Mathieu Poirier [this message]
2016-11-16 15:56     ` Mathieu Poirier
2016-11-16 16:02 ` [PATCH 1/2] ahci: qoriq: added a condition to enable dma coherence Robin Murphy
2016-11-16 16:02   ` Robin Murphy

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=20161116155613.GA1228@linaro.org \
    --to=mathieu.poirier@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=yuantian.tang@nxp.com \
    /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.