All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Kefeng Wang <kefeng.wang@linaro.org>, Tejun Heo <tj@kernel.org>,
	Maxime Ripard <maxime.ripard@free-electrons.com>,
	linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: wangkefeng.wang@huawei.com
Subject: Re: [PATCH v4 2/3] libahci_platform: add host_flags parameter in ahci_platform_init_host()
Date: Wed, 23 Apr 2014 13:47:56 +0200	[thread overview]
Message-ID: <5357A86C.1070709@redhat.com> (raw)
In-Reply-To: <1398253008-17059-3-git-send-email-kefeng.wang@linaro.org>

Hi,

On 04/23/2014 01:36 PM, Kefeng Wang wrote:
> Add a dynamic host flags argument to make ahci_platform_init_host more flexible,
> then remove the AHCI_HFLAGS(...) argument from some driver's ata_port_info,
> and pass that in as the new arguments.
> 
> Cc: Hans de Geode <hdegoede@redhat.com>
> Signed-off-by: Kefeng Wang <kefeng.wang@linaro.org>

Thanks for working on this, review comments inline.

> ---
>  drivers/ata/ahci_da850.c       |    3 ++-
>  drivers/ata/ahci_imx.c         |    3 ++-
>  drivers/ata/ahci_platform.c    |    2 +-
>  drivers/ata/ahci_st.c          |    2 +-
>  drivers/ata/ahci_sunxi.c       |    9 ++++++---
>  drivers/ata/ahci_xgene.c       |    6 +++++-
>  drivers/ata/libahci_platform.c |    2 ++
>  include/linux/ahci_platform.h  |    1 +
>  8 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
> index 2c83613..2b77d53 100644
> --- a/drivers/ata/ahci_da850.c
> +++ b/drivers/ata/ahci_da850.c
> @@ -85,7 +85,8 @@ static int ahci_da850_probe(struct platform_device *pdev)
>  
>  	da850_sata_init(dev, pwrdn_reg, hpriv->mmio);
>  
> -	rc = ahci_platform_init_host(pdev, hpriv, &ahci_da850_port_info, 0, 0);
> +	rc = ahci_platform_init_host(pdev, hpriv, &ahci_da850_port_info,
> +				     0, 0, 0);
>  	if (rc)
>  		goto disable_resources;
>  
> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> index 497c7ab..e7e44a7 100644
> --- a/drivers/ata/ahci_imx.c
> +++ b/drivers/ata/ahci_imx.c
> @@ -267,7 +267,8 @@ static int imx_ahci_probe(struct platform_device *pdev)
>  	reg_val = clk_get_rate(imxpriv->ahb_clk) / 1000;
>  	writel(reg_val, hpriv->mmio + HOST_TIMER1MS);
>  
> -	ret = ahci_platform_init_host(pdev, hpriv, &ahci_imx_port_info, 0, 0);
> +	ret = ahci_platform_init_host(pdev, hpriv, &ahci_imx_port_info,
> +				      0, 0, 0);
>  	if (ret)
>  		imx_sata_disable(hpriv);
>  
> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> index ef67e79..a476a1f 100644
> --- a/drivers/ata/ahci_platform.c
> +++ b/drivers/ata/ahci_platform.c
> @@ -55,7 +55,7 @@ static int ahci_probe(struct platform_device *pdev)
>  			goto disable_resources;
>  	}
>  
> -	rc = ahci_platform_init_host(pdev, hpriv, &ahci_port_info, 0, 0);
> +	rc = ahci_platform_init_host(pdev, hpriv, &ahci_port_info, 0, 0, 0);
>  	if (rc)
>  		goto pdata_exit;
>  
> diff --git a/drivers/ata/ahci_st.c b/drivers/ata/ahci_st.c
> index 6332222..2595598 100644
> --- a/drivers/ata/ahci_st.c
> +++ b/drivers/ata/ahci_st.c
> @@ -166,7 +166,7 @@ static int st_ahci_probe(struct platform_device *pdev)
>  	if (err)
>  		return err;
>  
> -	err = ahci_platform_init_host(pdev, hpriv, &st_ahci_port_info, 0, 0);
> +	err = ahci_platform_init_host(pdev, hpriv, &st_ahci_port_info, 0, 0, 0);
>  	if (err) {
>  		ahci_platform_disable_resources(hpriv);
>  		return err;
> diff --git a/drivers/ata/ahci_sunxi.c b/drivers/ata/ahci_sunxi.c
> index 42d3f64..543132a 100644
> --- a/drivers/ata/ahci_sunxi.c
> +++ b/drivers/ata/ahci_sunxi.c
> @@ -157,8 +157,6 @@ static void ahci_sunxi_start_engine(struct ata_port *ap)
>  }
>  
>  static const struct ata_port_info ahci_sunxi_port_info = {
> -	AHCI_HFLAGS(AHCI_HFLAG_32BIT_ONLY | AHCI_HFLAG_NO_MSI |
> -			  AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ),
>  	.flags		= AHCI_FLAG_COMMON | ATA_FLAG_NCQ,
>  	.pio_mask	= ATA_PIO4,
>  	.udma_mask	= ATA_UDMA6,
> @@ -169,6 +167,7 @@ static int ahci_sunxi_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct ahci_host_priv *hpriv;
> +	unsigned int hflags;
>  	int rc;
>  
>  	hpriv = ahci_platform_get_resources(pdev);
> @@ -185,7 +184,11 @@ static int ahci_sunxi_probe(struct platform_device *pdev)
>  	if (rc)
>  		goto disable_resources;
>  
> -	rc = ahci_platform_init_host(pdev, hpriv, &ahci_sunxi_port_info, 0, 0);
> +	hflags = AHCI_HFLAG_32BIT_ONLY | AHCI_HFLAG_NO_MSI |
> +		 AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ,
> +
> +	rc = ahci_platform_init_host(pdev, hpriv, &ahci_sunxi_port_info,
> +				     hflags, 0, 0);
>  	if (rc)
>  		goto disable_resources;
>  
> diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
> index 77c89bf..8022b5a 100644
> --- a/drivers/ata/ahci_xgene.c
> +++ b/drivers/ata/ahci_xgene.c
> @@ -382,6 +382,7 @@ static int xgene_ahci_probe(struct platform_device *pdev)
>  	struct ahci_host_priv *hpriv;
>  	struct xgene_ahci_context *ctx;
>  	struct resource *res;
> +	unsigned int hflags;
>  	int rc;
>  
>  	hpriv = ahci_platform_get_resources(pdev);
> @@ -450,7 +451,10 @@ static int xgene_ahci_probe(struct platform_device *pdev)
>  		goto disable_resources;
>  	}
>  
> -	rc = ahci_platform_init_host(pdev, hpriv, &xgene_ahci_port_info, 0, 0);
> +	hflags = AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ,
> +
> +	rc = ahci_platform_init_host(pdev, hpriv, &xgene_ahci_port_info,
> +				     hflags, 0, 0);
>  	if (rc)
>  		goto disable_resources;
>  

You should remove the AHCI_HFLAGS(AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ),
part from xgene_ahci_port_info here, like you do for ahci_sunxi.c

> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index 7cb3a85..ebd880e 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -283,6 +283,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_get_resources);
>   * @pdev: platform device pointer for the host
>   * @hpriv: ahci-host private data for the host
>   * @pi_template: template for the ata_port_info to use
> + * @host_flags: ahci host flags used in ahci_host_priv
>   * @force_port_map: param passed to ahci_save_initial_config
>   * @mask_port_map: param passed to ahci_save_initial_config
>   *
> @@ -296,6 +297,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_get_resources);
>  int ahci_platform_init_host(struct platform_device *pdev,
>  			    struct ahci_host_priv *hpriv,
>  			    const struct ata_port_info *pi_template,
> +			    unsigned int host_flags,
>  			    unsigned int force_port_map,
>  			    unsigned int mask_port_map)
>  {

And here you should actually do something with hflags, not just throw
them away. Something like:

pi.private_data = (void *)host_flags;

> diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
> index 1f16d50..45897f6 100644
> --- a/include/linux/ahci_platform.h
> +++ b/include/linux/ahci_platform.h
> @@ -44,6 +44,7 @@ struct ahci_host_priv *ahci_platform_get_resources(
>  int ahci_platform_init_host(struct platform_device *pdev,
>  			    struct ahci_host_priv *hpriv,
>  			    const struct ata_port_info *pi_template,
> +			    unsigned int host_flags,
>  			    unsigned int force_port_map,
>  			    unsigned int mask_port_map);
>  
> 

Regards,

Hans

  reply	other threads:[~2014-04-23 11:48 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-23 11:36 [PATCH v4 0/3] ahci: add support for hisilicon sata Kefeng Wang
2014-04-23 11:36 ` [PATCH v4 1/3] ata: ahci: append new hflag AHCI_HFLAG_NO_FBS Kefeng Wang
2014-04-23 11:36 ` [PATCH v4 2/3] libahci_platform: add host_flags parameter in ahci_platform_init_host() Kefeng Wang
2014-04-23 11:47   ` Hans de Goede [this message]
2014-04-23 12:12     ` kefeng.wang
2014-04-23 11:36 ` [PATCH v4 3/3] ahci: add support for Hisilicon sata Kefeng Wang

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=5357A86C.1070709@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=kefeng.wang@linaro.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.ripard@free-electrons.com \
    --cc=tj@kernel.org \
    --cc=wangkefeng.wang@huawei.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.