All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <computersforpeace@gmail.com>
To: Tejun Heo <tj@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-ide@vger.kernel.org,
	Brian Norris <computersforpeace@gmail.com>,
	kernel@stlinux.com,
	Srinivas Kandagatla <srinivas.kandagatla@gmail.com>,
	Maxime Coquelin <maxime.coquelin@st.com>,
	Patrice Chotard <patrice.chotard@st.com>
Subject: [PATCH] ata: ahci_st: fixup layering violations / drvdata errors
Date: Wed,  8 Apr 2015 11:59:59 -0700	[thread overview]
Message-ID: <1428519599-31885-1-git-send-email-computersforpeace@gmail.com> (raw)

When working on another SATA driver that uses libahci_platform, I
noticed an error in this driver; it tries to the the driver data for its
device, while libata also thinks it can set the driver data. See:

  ahci_platform_init_host()
  -> ata_host_alloc_pinfo()
     -> ata_host_alloc()
        -> dev_set_drvdata()

So instead of sticking the IP-specific platform data into drvdata, let's
use the plat_data variable that is reserved for this use.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@gmail.com>
Cc: Maxime Coquelin <maxime.coquelin@st.com>
Cc: Patrice Chotard <patrice.chotard@st.com>
---
This is ONLY compile tested; I don't have hardware to run. This looks like it
could have ramifications on suspend/resume support, and hot device removal
(e.g., sysfs unbind), so it might qualify as -stable, if someone can test it

 drivers/ata/ahci_st.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/ata/ahci_st.c b/drivers/ata/ahci_st.c
index bc971af262e7..2bd2375c2ab1 100644
--- a/drivers/ata/ahci_st.c
+++ b/drivers/ata/ahci_st.c
@@ -37,7 +37,6 @@ struct st_ahci_drv_data {
 	struct reset_control *pwr;
 	struct reset_control *sw_rst;
 	struct reset_control *pwr_rst;
-	struct ahci_host_priv *hpriv;
 };
 
 static void st_ahci_configure_oob(void __iomem *mmio)
@@ -57,7 +56,9 @@ static void st_ahci_configure_oob(void __iomem *mmio)
 
 static int st_ahci_deassert_resets(struct device *dev)
 {
-	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct st_ahci_drv_data *drv_data = hpriv->plat_data;
 	int err;
 
 	if (drv_data->pwr) {
@@ -68,7 +69,7 @@ static int st_ahci_deassert_resets(struct device *dev)
 		}
 	}
 
-	st_ahci_configure_oob(drv_data->hpriv->mmio);
+	st_ahci_configure_oob(hpriv->mmio);
 
 	if (drv_data->sw_rst) {
 		err = reset_control_deassert(drv_data->sw_rst);
@@ -92,8 +93,8 @@ static int st_ahci_deassert_resets(struct device *dev)
 static void st_ahci_host_stop(struct ata_host *host)
 {
 	struct ahci_host_priv *hpriv = host->private_data;
+	struct st_ahci_drv_data *drv_data = hpriv->plat_data;
 	struct device *dev = host->dev;
-	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
 	int err;
 
 	if (drv_data->pwr) {
@@ -107,7 +108,9 @@ static void st_ahci_host_stop(struct ata_host *host)
 
 static int st_ahci_probe_resets(struct platform_device *pdev)
 {
-	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct st_ahci_drv_data *drv_data = hpriv->plat_data;
 
 	drv_data->pwr = devm_reset_control_get(&pdev->dev, "pwr-dwn");
 	if (IS_ERR(drv_data->pwr)) {
@@ -161,8 +164,7 @@ static int st_ahci_probe(struct platform_device *pdev)
 	hpriv = ahci_platform_get_resources(pdev);
 	if (IS_ERR(hpriv))
 		return PTR_ERR(hpriv);
-
-	drv_data->hpriv = hpriv;
+	hpriv->plat_data = drv_data;
 
 	err = st_ahci_probe_resets(pdev);
 	if (err)
@@ -185,8 +187,9 @@ static int st_ahci_probe(struct platform_device *pdev)
 #ifdef CONFIG_PM_SLEEP
 static int st_ahci_suspend(struct device *dev)
 {
-	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
-	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct st_ahci_drv_data *drv_data = hpriv->plat_data;
 	int err;
 
 	err = ahci_platform_suspend_host(dev);
@@ -208,8 +211,8 @@ static int st_ahci_suspend(struct device *dev)
 
 static int st_ahci_resume(struct device *dev)
 {
-	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
-	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
 	int err;
 
 	err = ahci_platform_enable_resources(hpriv);
-- 
1.9.1


WARNING: multiple messages have this Message-ID (diff)
From: computersforpeace@gmail.com (Brian Norris)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ata: ahci_st: fixup layering violations / drvdata errors
Date: Wed,  8 Apr 2015 11:59:59 -0700	[thread overview]
Message-ID: <1428519599-31885-1-git-send-email-computersforpeace@gmail.com> (raw)

When working on another SATA driver that uses libahci_platform, I
noticed an error in this driver; it tries to the the driver data for its
device, while libata also thinks it can set the driver data. See:

  ahci_platform_init_host()
  -> ata_host_alloc_pinfo()
     -> ata_host_alloc()
        -> dev_set_drvdata()

So instead of sticking the IP-specific platform data into drvdata, let's
use the plat_data variable that is reserved for this use.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@gmail.com>
Cc: Maxime Coquelin <maxime.coquelin@st.com>
Cc: Patrice Chotard <patrice.chotard@st.com>
---
This is ONLY compile tested; I don't have hardware to run. This looks like it
could have ramifications on suspend/resume support, and hot device removal
(e.g., sysfs unbind), so it might qualify as -stable, if someone can test it

 drivers/ata/ahci_st.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/ata/ahci_st.c b/drivers/ata/ahci_st.c
index bc971af262e7..2bd2375c2ab1 100644
--- a/drivers/ata/ahci_st.c
+++ b/drivers/ata/ahci_st.c
@@ -37,7 +37,6 @@ struct st_ahci_drv_data {
 	struct reset_control *pwr;
 	struct reset_control *sw_rst;
 	struct reset_control *pwr_rst;
-	struct ahci_host_priv *hpriv;
 };
 
 static void st_ahci_configure_oob(void __iomem *mmio)
@@ -57,7 +56,9 @@ static void st_ahci_configure_oob(void __iomem *mmio)
 
 static int st_ahci_deassert_resets(struct device *dev)
 {
-	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct st_ahci_drv_data *drv_data = hpriv->plat_data;
 	int err;
 
 	if (drv_data->pwr) {
@@ -68,7 +69,7 @@ static int st_ahci_deassert_resets(struct device *dev)
 		}
 	}
 
-	st_ahci_configure_oob(drv_data->hpriv->mmio);
+	st_ahci_configure_oob(hpriv->mmio);
 
 	if (drv_data->sw_rst) {
 		err = reset_control_deassert(drv_data->sw_rst);
@@ -92,8 +93,8 @@ static int st_ahci_deassert_resets(struct device *dev)
 static void st_ahci_host_stop(struct ata_host *host)
 {
 	struct ahci_host_priv *hpriv = host->private_data;
+	struct st_ahci_drv_data *drv_data = hpriv->plat_data;
 	struct device *dev = host->dev;
-	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
 	int err;
 
 	if (drv_data->pwr) {
@@ -107,7 +108,9 @@ static void st_ahci_host_stop(struct ata_host *host)
 
 static int st_ahci_probe_resets(struct platform_device *pdev)
 {
-	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct st_ahci_drv_data *drv_data = hpriv->plat_data;
 
 	drv_data->pwr = devm_reset_control_get(&pdev->dev, "pwr-dwn");
 	if (IS_ERR(drv_data->pwr)) {
@@ -161,8 +164,7 @@ static int st_ahci_probe(struct platform_device *pdev)
 	hpriv = ahci_platform_get_resources(pdev);
 	if (IS_ERR(hpriv))
 		return PTR_ERR(hpriv);
-
-	drv_data->hpriv = hpriv;
+	hpriv->plat_data = drv_data;
 
 	err = st_ahci_probe_resets(pdev);
 	if (err)
@@ -185,8 +187,9 @@ static int st_ahci_probe(struct platform_device *pdev)
 #ifdef CONFIG_PM_SLEEP
 static int st_ahci_suspend(struct device *dev)
 {
-	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
-	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct st_ahci_drv_data *drv_data = hpriv->plat_data;
 	int err;
 
 	err = ahci_platform_suspend_host(dev);
@@ -208,8 +211,8 @@ static int st_ahci_suspend(struct device *dev)
 
 static int st_ahci_resume(struct device *dev)
 {
-	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
-	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
 	int err;
 
 	err = ahci_platform_enable_resources(hpriv);
-- 
1.9.1

WARNING: multiple messages have this Message-ID (diff)
From: Brian Norris <computersforpeace@gmail.com>
To: Tejun Heo <tj@kernel.org>
Cc: <linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>, <linux-ide@vger.kernel.org>,
	Brian Norris <computersforpeace@gmail.com>,
	kernel@stlinux.com,
	Srinivas Kandagatla <srinivas.kandagatla@gmail.com>,
	Maxime Coquelin <maxime.coquelin@st.com>,
	Patrice Chotard <patrice.chotard@st.com>
Subject: [PATCH] ata: ahci_st: fixup layering violations / drvdata errors
Date: Wed,  8 Apr 2015 11:59:59 -0700	[thread overview]
Message-ID: <1428519599-31885-1-git-send-email-computersforpeace@gmail.com> (raw)

When working on another SATA driver that uses libahci_platform, I
noticed an error in this driver; it tries to the the driver data for its
device, while libata also thinks it can set the driver data. See:

  ahci_platform_init_host()
  -> ata_host_alloc_pinfo()
     -> ata_host_alloc()
        -> dev_set_drvdata()

So instead of sticking the IP-specific platform data into drvdata, let's
use the plat_data variable that is reserved for this use.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@gmail.com>
Cc: Maxime Coquelin <maxime.coquelin@st.com>
Cc: Patrice Chotard <patrice.chotard@st.com>
---
This is ONLY compile tested; I don't have hardware to run. This looks like it
could have ramifications on suspend/resume support, and hot device removal
(e.g., sysfs unbind), so it might qualify as -stable, if someone can test it

 drivers/ata/ahci_st.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/ata/ahci_st.c b/drivers/ata/ahci_st.c
index bc971af262e7..2bd2375c2ab1 100644
--- a/drivers/ata/ahci_st.c
+++ b/drivers/ata/ahci_st.c
@@ -37,7 +37,6 @@ struct st_ahci_drv_data {
 	struct reset_control *pwr;
 	struct reset_control *sw_rst;
 	struct reset_control *pwr_rst;
-	struct ahci_host_priv *hpriv;
 };
 
 static void st_ahci_configure_oob(void __iomem *mmio)
@@ -57,7 +56,9 @@ static void st_ahci_configure_oob(void __iomem *mmio)
 
 static int st_ahci_deassert_resets(struct device *dev)
 {
-	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct st_ahci_drv_data *drv_data = hpriv->plat_data;
 	int err;
 
 	if (drv_data->pwr) {
@@ -68,7 +69,7 @@ static int st_ahci_deassert_resets(struct device *dev)
 		}
 	}
 
-	st_ahci_configure_oob(drv_data->hpriv->mmio);
+	st_ahci_configure_oob(hpriv->mmio);
 
 	if (drv_data->sw_rst) {
 		err = reset_control_deassert(drv_data->sw_rst);
@@ -92,8 +93,8 @@ static int st_ahci_deassert_resets(struct device *dev)
 static void st_ahci_host_stop(struct ata_host *host)
 {
 	struct ahci_host_priv *hpriv = host->private_data;
+	struct st_ahci_drv_data *drv_data = hpriv->plat_data;
 	struct device *dev = host->dev;
-	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
 	int err;
 
 	if (drv_data->pwr) {
@@ -107,7 +108,9 @@ static void st_ahci_host_stop(struct ata_host *host)
 
 static int st_ahci_probe_resets(struct platform_device *pdev)
 {
-	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct st_ahci_drv_data *drv_data = hpriv->plat_data;
 
 	drv_data->pwr = devm_reset_control_get(&pdev->dev, "pwr-dwn");
 	if (IS_ERR(drv_data->pwr)) {
@@ -161,8 +164,7 @@ static int st_ahci_probe(struct platform_device *pdev)
 	hpriv = ahci_platform_get_resources(pdev);
 	if (IS_ERR(hpriv))
 		return PTR_ERR(hpriv);
-
-	drv_data->hpriv = hpriv;
+	hpriv->plat_data = drv_data;
 
 	err = st_ahci_probe_resets(pdev);
 	if (err)
@@ -185,8 +187,9 @@ static int st_ahci_probe(struct platform_device *pdev)
 #ifdef CONFIG_PM_SLEEP
 static int st_ahci_suspend(struct device *dev)
 {
-	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
-	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct st_ahci_drv_data *drv_data = hpriv->plat_data;
 	int err;
 
 	err = ahci_platform_suspend_host(dev);
@@ -208,8 +211,8 @@ static int st_ahci_suspend(struct device *dev)
 
 static int st_ahci_resume(struct device *dev)
 {
-	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
-	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
 	int err;
 
 	err = ahci_platform_enable_resources(hpriv);
-- 
1.9.1


             reply	other threads:[~2015-04-08 19:00 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-08 18:59 Brian Norris [this message]
2015-04-08 18:59 ` [PATCH] ata: ahci_st: fixup layering violations / drvdata errors Brian Norris
2015-04-08 18:59 ` Brian Norris
2015-04-09  7:18 ` Maxime Coquelin
2015-04-09  7:18   ` Maxime Coquelin
2015-04-09  7:18   ` Maxime Coquelin
2015-04-09  9:43   ` Peter Griffin
2015-04-09  9:43     ` Peter Griffin
2015-04-17 14:55     ` Tejun Heo
2015-04-17 14:55       ` Tejun Heo
2015-04-20 13:04       ` Peter Griffin
2015-04-20 13:04         ` Peter Griffin
2015-04-09  9:34 ` Sergei Shtylyov
2015-04-09  9:34   ` Sergei Shtylyov
2015-04-09 17:27   ` Brian Norris
2015-04-09 17:27     ` Brian Norris

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=1428519599-31885-1-git-send-email-computersforpeace@gmail.com \
    --to=computersforpeace@gmail.com \
    --cc=kernel@stlinux.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.coquelin@st.com \
    --cc=patrice.chotard@st.com \
    --cc=srinivas.kandagatla@gmail.com \
    --cc=tj@kernel.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.