[v2,3/3] pci: dra7xx: use pdata callbacks to perform reset
diff mbox

Message ID 1452667666-17533-4-git-send-email-kishon@ti.com
State New
Headers show

Commit Message

Kishon Vijay Abraham I Jan. 13, 2016, 6:47 a.m. UTC
Use platform populated reset assert and deassert
callbacks to perform reset of PCIe.

Use these callbacks until a reset interface using drivers/reset
is available for the purpose.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
 drivers/pci/host/pci-dra7xx.c |   32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Comments

Tony Lindgren Jan. 13, 2016, 5:19 p.m. UTC | #1
* Kishon Vijay Abraham I <kishon@ti.com> [160112 22:48]:
> Use platform populated reset assert and deassert
> callbacks to perform reset of PCIe.
> 
> Use these callbacks until a reset interface using drivers/reset
> is available for the purpose.

This one has a dependency to the second patch for the platform
data.

Bjorn, how do you prefer to merge this once there are no more
comments?

How about I set up an immutable branch against v4.5-rc1 with
just these three patches that we can both then merge in?

My preference is to add this to linux next after the merge
window for v4.6. Bjorn, if you want it merged as fixes, I'm
fine with that too naturally.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suman Anna Jan. 13, 2016, 5:51 p.m. UTC | #2
On 01/13/2016 12:47 AM, Kishon Vijay Abraham I wrote:
> Use platform populated reset assert and deassert
> callbacks to perform reset of PCIe.
> 
> Use these callbacks until a reset interface using drivers/reset
> is available for the purpose.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---
>  drivers/pci/host/pci-dra7xx.c |   32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
> index 8c36880..049083d 100644
> --- a/drivers/pci/host/pci-dra7xx.c
> +++ b/drivers/pci/host/pci-dra7xx.c
> @@ -25,6 +25,8 @@
>  #include <linux/resource.h>
>  #include <linux/types.h>
>  
> +#include <linux/platform_data/pci-dra7xx.h>
> +
>  #include "pcie-designware.h"
>  
>  /* PCIe controller wrapper DRA7XX configuration registers */
> @@ -329,6 +331,32 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>  	return 0;
>  }
>  
> +static int dra7xx_pcie_reset(struct platform_device *pdev)
> +{
> +	int ret;
> +	struct device *dev = &pdev->dev;
> +	struct pci_dra7xx_platform_data *pdata = pdev->dev.platform_data;
> +
> +	if (!(pdata && pdata->deassert_reset && pdata->assert_reset)) {
> +		dev_err(dev, "platform data for reset not found!\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = pdata->assert_reset(pdev, pdata->reset_name);
> +	if (ret) {
> +		dev_err(dev, "assert_reset failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = pdata->deassert_reset(pdev, pdata->reset_name);
> +	if (ret) {
> +		dev_err(dev, "deassert_reset failed: %d\n", ret);
> +		return ret;
> +	}

The only comment I have on this is the symmetry (assert_reset invocation
in driver remove). If you install and remove the module once, then the
reset stays deasserted. On Power-On-Reset, the resets by default will be
in asserted state.

regards
SUman

> +
> +	return 0;
> +}
> +
>  static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  {
>  	u32 reg;
> @@ -347,6 +375,10 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  	enum of_gpio_flags flags;
>  	unsigned long gpio_flags;
>  
> +	ret = dra7xx_pcie_reset(pdev);
> +	if (ret)
> +		return ret;
> +
>  	dra7xx = devm_kzalloc(dev, sizeof(*dra7xx), GFP_KERNEL);
>  	if (!dra7xx)
>  		return -ENOMEM;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kishon Vijay Abraham I Jan. 14, 2016, 8:37 a.m. UTC | #3
Hi Suman,

On Wednesday 13 January 2016 11:21 PM, Suman Anna wrote:
> On 01/13/2016 12:47 AM, Kishon Vijay Abraham I wrote:
>> Use platform populated reset assert and deassert
>> callbacks to perform reset of PCIe.
>>
>> Use these callbacks until a reset interface using drivers/reset
>> is available for the purpose.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> ---
>>  drivers/pci/host/pci-dra7xx.c |   32 ++++++++++++++++++++++++++++++++
>>  1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
>> index 8c36880..049083d 100644
>> --- a/drivers/pci/host/pci-dra7xx.c
>> +++ b/drivers/pci/host/pci-dra7xx.c
>> @@ -25,6 +25,8 @@
>>  #include <linux/resource.h>
>>  #include <linux/types.h>
>>  
>> +#include <linux/platform_data/pci-dra7xx.h>
>> +
>>  #include "pcie-designware.h"
>>  
>>  /* PCIe controller wrapper DRA7XX configuration registers */
>> @@ -329,6 +331,32 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>>  	return 0;
>>  }
>>  
>> +static int dra7xx_pcie_reset(struct platform_device *pdev)
>> +{
>> +	int ret;
>> +	struct device *dev = &pdev->dev;
>> +	struct pci_dra7xx_platform_data *pdata = pdev->dev.platform_data;
>> +
>> +	if (!(pdata && pdata->deassert_reset && pdata->assert_reset)) {
>> +		dev_err(dev, "platform data for reset not found!\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = pdata->assert_reset(pdev, pdata->reset_name);
>> +	if (ret) {
>> +		dev_err(dev, "assert_reset failed: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = pdata->deassert_reset(pdev, pdata->reset_name);
>> +	if (ret) {
>> +		dev_err(dev, "deassert_reset failed: %d\n", ret);
>> +		return ret;
>> +	}
> 
> The only comment I have on this is the symmetry (assert_reset invocation
> in driver remove). If you install and remove the module once, then the
> reset stays deasserted. On Power-On-Reset, the resets by default will be
> in asserted state.

hmm.. not sure of the benefits of leaving the reset lines de-asserted during
remove. The idea is irrespective of the initial sate or power-on state, during
probe the driver should assert and de-assert the reset lines.

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kishon Vijay Abraham I Jan. 14, 2016, 1:28 p.m. UTC | #4
Hi,

On Thursday 14 January 2016 02:07 PM, Kishon Vijay Abraham I wrote:
> Hi Suman,
> 
> On Wednesday 13 January 2016 11:21 PM, Suman Anna wrote:
>> On 01/13/2016 12:47 AM, Kishon Vijay Abraham I wrote:
>>> Use platform populated reset assert and deassert
>>> callbacks to perform reset of PCIe.
>>>
>>> Use these callbacks until a reset interface using drivers/reset
>>> is available for the purpose.
>>>
>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>>> ---
>>>  drivers/pci/host/pci-dra7xx.c |   32 ++++++++++++++++++++++++++++++++
>>>  1 file changed, 32 insertions(+)
>>>
>>> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
>>> index 8c36880..049083d 100644
>>> --- a/drivers/pci/host/pci-dra7xx.c
>>> +++ b/drivers/pci/host/pci-dra7xx.c
>>> @@ -25,6 +25,8 @@
>>>  #include <linux/resource.h>
>>>  #include <linux/types.h>
>>>  
>>> +#include <linux/platform_data/pci-dra7xx.h>
>>> +
>>>  #include "pcie-designware.h"
>>>  
>>>  /* PCIe controller wrapper DRA7XX configuration registers */
>>> @@ -329,6 +331,32 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>>>  	return 0;
>>>  }
>>>  
>>> +static int dra7xx_pcie_reset(struct platform_device *pdev)
>>> +{
>>> +	int ret;
>>> +	struct device *dev = &pdev->dev;
>>> +	struct pci_dra7xx_platform_data *pdata = pdev->dev.platform_data;
>>> +
>>> +	if (!(pdata && pdata->deassert_reset && pdata->assert_reset)) {
>>> +		dev_err(dev, "platform data for reset not found!\n");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	ret = pdata->assert_reset(pdev, pdata->reset_name);
>>> +	if (ret) {
>>> +		dev_err(dev, "assert_reset failed: %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = pdata->deassert_reset(pdev, pdata->reset_name);
>>> +	if (ret) {
>>> +		dev_err(dev, "deassert_reset failed: %d\n", ret);
>>> +		return ret;
>>> +	}
>>
>> The only comment I have on this is the symmetry (assert_reset invocation
>> in driver remove). If you install and remove the module once, then the
>> reset stays deasserted. On Power-On-Reset, the resets by default will be
>> in asserted state.
> 
> hmm.. not sure of the benefits of leaving the reset lines de-asserted during
> remove. The idea is irrespective of the initial sate or power-on state, during
> probe the driver should assert and de-assert the reset lines.

Also right now the pci-dra7xx can't be inserted as a module. However since that
might be added in the future, I'll add assert_reset in the remove path of this
driver.

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index 8c36880..049083d 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -25,6 +25,8 @@ 
 #include <linux/resource.h>
 #include <linux/types.h>
 
+#include <linux/platform_data/pci-dra7xx.h>
+
 #include "pcie-designware.h"
 
 /* PCIe controller wrapper DRA7XX configuration registers */
@@ -329,6 +331,32 @@  static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
 	return 0;
 }
 
+static int dra7xx_pcie_reset(struct platform_device *pdev)
+{
+	int ret;
+	struct device *dev = &pdev->dev;
+	struct pci_dra7xx_platform_data *pdata = pdev->dev.platform_data;
+
+	if (!(pdata && pdata->deassert_reset && pdata->assert_reset)) {
+		dev_err(dev, "platform data for reset not found!\n");
+		return -EINVAL;
+	}
+
+	ret = pdata->assert_reset(pdev, pdata->reset_name);
+	if (ret) {
+		dev_err(dev, "assert_reset failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = pdata->deassert_reset(pdev, pdata->reset_name);
+	if (ret) {
+		dev_err(dev, "deassert_reset failed: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int __init dra7xx_pcie_probe(struct platform_device *pdev)
 {
 	u32 reg;
@@ -347,6 +375,10 @@  static int __init dra7xx_pcie_probe(struct platform_device *pdev)
 	enum of_gpio_flags flags;
 	unsigned long gpio_flags;
 
+	ret = dra7xx_pcie_reset(pdev);
+	if (ret)
+		return ret;
+
 	dra7xx = devm_kzalloc(dev, sizeof(*dra7xx), GFP_KERNEL);
 	if (!dra7xx)
 		return -ENOMEM;