Patchwork [RFC,v3,05/13] ahci-platform: Pass ahci_host_priv ptr to ahci_platform_data init method

login
register
mail settings
Submitter Hans de Goede
Date Jan. 18, 2014, 11:48 p.m.
Message ID <1390088935-7193-6-git-send-email-hdegoede@redhat.com>
Download mbox | patch
Permalink /patch/312345/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Hans de Goede - Jan. 18, 2014, 11:48 p.m.
Some ahci_platform_data->init methods need access to the ahci_host_priv data.

Note:

When calling ahci_platform_data->init the ata_host has not been allocated yet,
so access to ahci_host_priv through the dev argument is not possible.

The hpriv->mmio argument may seem redundant, but it is useful for drivers
which live outside of drivers/ata and thus don't know the contents of the
ahci_platform_data data type.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/mach-davinci/devices-da8xx.c | 3 ++-
 arch/arm/mach-spear/spear1340.c       | 3 ++-
 drivers/ata/ahci_imx.c                | 3 ++-
 drivers/ata/ahci_platform.c           | 2 +-
 include/linux/ahci_platform.h         | 4 +++-
 5 files changed, 10 insertions(+), 5 deletions(-)
Tejun Heo - Jan. 19, 2014, 11:30 a.m.
On Sun, Jan 19, 2014 at 12:48:47AM +0100, Hans de Goede wrote:
> Some ahci_platform_data->init methods need access to the ahci_host_priv data.
> 
> Note:
> 
> When calling ahci_platform_data->init the ata_host has not been allocated yet,
> so access to ahci_host_priv through the dev argument is not possible.
> 
> The hpriv->mmio argument may seem redundant, but it is useful for drivers
> which live outside of drivers/ata and thus don't know the contents of the
> ahci_platform_data data type.

Wouldn't the right thing to do be moving them under drivers/ata?

Thanks.
Hans de Goede - Jan. 19, 2014, 6:51 p.m.
Hi,

On 01/19/2014 12:30 PM, Tejun Heo wrote:
> On Sun, Jan 19, 2014 at 12:48:47AM +0100, Hans de Goede wrote:
>> Some ahci_platform_data->init methods need access to the ahci_host_priv data.
>>
>> Note:
>>
>> When calling ahci_platform_data->init the ata_host has not been allocated yet,
>> so access to ahci_host_priv through the dev argument is not possible.
>>
>> The hpriv->mmio argument may seem redundant, but it is useful for drivers
>> which live outside of drivers/ata and thus don't know the contents of the
>> ahci_platform_data data type.
>
> Wouldn't the right thing to do be moving them under drivers/ata?

Yes, but they are part of arch/arm/mach-foo, so moving them is non trivial, and
I don't want to go make these kinda changes without hardware to test. Please keep
in mind that I'm partially cleaning up other peoples mess here (the imx bits
specifically). I'm willing to do that to some extend (*). But buying an imx6q board
and fixing that is about as far as I'm willing to go.

Regards,

Hans


*) Keep in mind that this is a hobby project done in my spare time, with all
hardware bought from personal budget.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo - Jan. 19, 2014, 7:17 p.m.
On Sun, Jan 19, 2014 at 07:51:48PM +0100, Hans de Goede wrote:
> Yes, but they are part of arch/arm/mach-foo, so moving them is non trivial, and
> I don't want to go make these kinda changes without hardware to test. Please keep
> in mind that I'm partially cleaning up other peoples mess here (the imx bits
> specifically). I'm willing to do that to some extend (*). But buying an imx6q board
> and fixing that is about as far as I'm willing to go.

If they can't be moved, the right thing to do is moving the header out
to include/linux so that they can be included from those directories.
Let's please not contort the API to fit the organizational issues.

Thanks.
Hans de Goede - Jan. 19, 2014, 7:56 p.m.
Hi,

On 01/19/2014 08:17 PM, Tejun Heo wrote:
> On Sun, Jan 19, 2014 at 07:51:48PM +0100, Hans de Goede wrote:
>> Yes, but they are part of arch/arm/mach-foo, so moving them is non trivial, and
>> I don't want to go make these kinda changes without hardware to test. Please keep
>> in mind that I'm partially cleaning up other peoples mess here (the imx bits
>> specifically). I'm willing to do that to some extend (*). But buying an imx6q board
>> and fixing that is about as far as I'm willing to go.
>
> If they can't be moved, the right thing to do is moving the header out
> to include/linux so that they can be included from those directories.
> Let's please not contort the API to fit the organizational issues.

Ok, asking the obvious here I guess, but you would accept a patch moving drivers/ata/ahci.h
to include/linux as part of this patch-set ?

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo - Jan. 20, 2014, 12:28 p.m.
On Sun, Jan 19, 2014 at 08:56:09PM +0100, Hans de Goede wrote:
> Ok, asking the obvious here I guess, but you would accept a patch moving drivers/ata/ahci.h
> to include/linux as part of this patch-set ?

Wouldn't creating a minimal file which only contains the necessary
bits make more sense?

Thanks.

Patch

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index 78829c5..81d0110 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -1061,7 +1061,8 @@  static unsigned long da850_sata_xtal[] = {
 	KHZ_TO_HZ(60000),
 };
 
-static int da850_sata_init(struct device *dev, void __iomem *addr)
+static int da850_sata_init(struct device *dev, struct ahci_host_priv *hpriv,
+			   void __iomem *addr)
 {
 	int i, ret;
 	unsigned int val;
diff --git a/arch/arm/mach-spear/spear1340.c b/arch/arm/mach-spear/spear1340.c
index c4cbbd2..9e4f70b 100644
--- a/arch/arm/mach-spear/spear1340.c
+++ b/arch/arm/mach-spear/spear1340.c
@@ -77,7 +77,8 @@ 
 			SPEAR1340_MIPHY_PLL_RATIO_TOP(25))
 
 /* SATA device registration */
-static int sata_miphy_init(struct device *dev, void __iomem *addr)
+static int sata_miphy_init(struct device *dev, struct ahci_host_priv *hpriv,
+			   void __iomem *addr)
 {
 	writel(SPEAR1340_SATA_CFG_VAL, SPEAR1340_PCIE_SATA_CFG);
 	writel(SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK,
diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index 30568d3..0051f29 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -91,7 +91,8 @@  static const struct ata_port_info ahci_imx_port_info = {
 	.port_ops	= &ahci_imx_ops,
 };
 
-static int imx6q_sata_init(struct device *dev, void __iomem *mmio)
+static int imx6q_sata_init(struct device *dev, struct ahci_host_priv *hpriv,
+			   void __iomem *mmio)
 {
 	int ret = 0;
 	unsigned int reg_val;
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 41720cb..33ac7a4 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -149,7 +149,7 @@  static int ahci_probe(struct platform_device *pdev)
 	 * returned successfully.
 	 */
 	if (pdata && pdata->init) {
-		rc = pdata->init(dev, hpriv->mmio);
+		rc = pdata->init(dev, hpriv, hpriv->mmio);
 		if (rc)
 			goto disable_unprepare_clk;
 	}
diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
index a641cb6..796dfb4 100644
--- a/include/linux/ahci_platform.h
+++ b/include/linux/ahci_platform.h
@@ -19,9 +19,11 @@ 
 
 struct device;
 struct ata_port_info;
+struct ahci_host_priv;
 
 struct ahci_platform_data {
-	int (*init)(struct device *dev, void __iomem *addr);
+	int (*init)(struct device *dev, struct ahci_host_priv *hpriv,
+		    void __iomem *addr);
 	void (*exit)(struct device *dev);
 	void (*suspend)(struct device *dev);
 	int (*resume)(struct device *dev);