From: Kishon Vijay Abraham I <hidden> Date: 2016-01-14 14:11:55
This series adds pdata-quirk mechanism to reset PCIe as a temporary
fix till reset controller driver is added in mainline.
Without this series, a stall is observed if pci dra7xx driver
is enabled.
Changes from v2:
*) Now assert reset line during driver removal (will be useful
when module insert/removal functionality is added)
*) Added kernel-doc comments for pci_dra7xx_platform_data
*) Better commit logs
*) Misc cleanup
Changes from v1:
*) Removed 'HACK' from $subject
*) removed reviewed-by Suman
Kishon Vijay Abraham I (3):
ARM: DRA7: hwmod: Add reset data for PCIe
ARM: DRA7: add pdata-quirks to do reset of PCIe
pci: dra7xx: use pdata callbacks to perform reset
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 15 +++++++
arch/arm/mach-omap2/pdata-quirks.c | 11 +++++
arch/arm/mach-omap2/prm7xx.h | 1 +
drivers/pci/host/pci-dra7xx.c | 66 +++++++++++++++++++++++++++++
include/linux/platform_data/pci-dra7xx.h | 29 +++++++++++++
5 files changed, 122 insertions(+)
create mode 100644 include/linux/platform_data/pci-dra7xx.h
--
1.7.9.5
From: Kishon Vijay Abraham I <hidden> Date: 2016-01-14 14:11:56
Create platform data for PCIe and populate it with function
pointers to perform assert and deassert of PCIe reset lines.
The PCIe driver can use the callbacks provided here to
reset the PCIe.
This will be removed once the reset contoller driver is
available to reset PCIe.
Signed-off-by: Kishon Vijay Abraham I <redacted>
Signed-off-by: Sekhar Nori <redacted>
---
arch/arm/mach-omap2/pdata-quirks.c | 11 +++++++++++
include/linux/platform_data/pci-dra7xx.h | 29 +++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
create mode 100644 include/linux/platform_data/pci-dra7xx.h
From: Kishon Vijay Abraham I <hidden> Date: 2016-01-14 14:12:04
Use assert/deassert callbacks populated in the platform data to
to perform reset of PCIe.
Use these callbacks until a reset controller driver is
is available in the kernel to reset PCIe.
Signed-off-by: Kishon Vijay Abraham I <redacted>
Signed-off-by: Sekhar Nori <redacted>
---
drivers/pci/host/pci-dra7xx.c | 66 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
@@ -329,6 +331,61 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,return0;}+staticintdra7xx_pcie_assert_reset(structplatform_device*pdev)+{+intret;+structdevice*dev=&pdev->dev;+structpci_dra7xx_platform_data*pdata=pdev->dev.platform_data;++if(!(pdata&&pdata->assert_reset)){+dev_err(dev,"platform data for assert 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);+returnret;+}++return0;+}++staticintdra7xx_pcie_deassert_reset(structplatform_device*pdev)+{+intret;+structdevice*dev=&pdev->dev;+structpci_dra7xx_platform_data*pdata=pdev->dev.platform_data;++if(!(pdata&&pdata->deassert_reset)){+dev_err(dev,"platform data for deassert reset not found!\n");+return-EINVAL;+}++ret=pdata->deassert_reset(pdev,pdata->reset_name);+if(ret){+dev_err(dev,"deassert_reset failed: %d\n",ret);+returnret;+}++return0;+}++staticintdra7xx_pcie_reset(structplatform_device*pdev)+{+intret;++ret=dra7xx_pcie_assert_reset(pdev);+if(ret<0)+returnret;++ret=dra7xx_pcie_deassert_reset(pdev);+if(ret<0)+returnret;++return0;+}+staticint__initdra7xx_pcie_probe(structplatform_device*pdev){u32reg;
@@ -347,6 +404,10 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)enumof_gpio_flagsflags;unsignedlonggpio_flags;+ret=dra7xx_pcie_reset(pdev);+if(ret)+returnret;+dra7xx=devm_kzalloc(dev,sizeof(*dra7xx),GFP_KERNEL);if(!dra7xx)return-ENOMEM;
@@ -457,6 +518,7 @@ static int __exit dra7xx_pcie_remove(struct platform_device *pdev)structpcie_port*pp=&dra7xx->pp;structdevice*dev=&pdev->dev;intcount=dra7xx->phy_count;+intret;if(pp->irq_domain)irq_domain_remove(pp->irq_domain);
@@ -467,6 +529,10 @@ static int __exit dra7xx_pcie_remove(struct platform_device *pdev)phy_exit(dra7xx->phy[count]);}+ret=dra7xx_pcie_assert_reset(pdev);+if(ret<0)+returnret;+return0;}
From: Suman Anna <hidden> Date: 2016-01-15 19:20:32
On 01/14/2016 08:11 AM, Kishon Vijay Abraham I wrote:
Create platform data for PCIe and populate it with function
pointers to perform assert and deassert of PCIe reset lines.
The PCIe driver can use the callbacks provided here to
reset the PCIe.
This will be removed once the reset contoller driver is
available to reset PCIe.
Signed-off-by: Kishon Vijay Abraham I <redacted>
Signed-off-by: Sekhar Nori <redacted>
Thanks for the revised version, looks much better.
Reviewed-by: Suman Anna <redacted>
From: Tony Lindgren <tony@atomide.com> Date: 2016-01-15 19:22:29
* Suman Anna [off-list ref] [160115 11:20]:
On 01/14/2016 08:11 AM, Kishon Vijay Abraham I wrote:
quoted
Create platform data for PCIe and populate it with function
pointers to perform assert and deassert of PCIe reset lines.
The PCIe driver can use the callbacks provided here to
reset the PCIe.
This will be removed once the reset contoller driver is
available to reset PCIe.
...
quoted
+/**
+ * struct pci_dra7xx_platform_data - platform data specific to pci in dra7xx
+ * @reset_name: name of the reset line
+ * @assert_reset: callback for performing assert reset operation
+ * @deassert_reset: callback for performing deassert reset operation
+ */
+struct pci_dra7xx_platform_data {
+ const char *reset_name;
+
+ int (*assert_reset)(struct platform_device *pdev, const char *name);
+ int (*deassert_reset)(struct platform_device *pdev, const char *name);
+};
I doubt this platform_data is dra7 specific. I believe it's
the same PCI controller that has been in the omap variants for
years?
Regards,
Tony
From: Suman Anna <hidden> Date: 2016-01-15 19:42:16
On 01/15/2016 01:22 PM, Tony Lindgren wrote:
* Suman Anna [off-list ref] [160115 11:20]:
quoted
On 01/14/2016 08:11 AM, Kishon Vijay Abraham I wrote:
quoted
Create platform data for PCIe and populate it with function
pointers to perform assert and deassert of PCIe reset lines.
The PCIe driver can use the callbacks provided here to
reset the PCIe.
This will be removed once the reset contoller driver is
available to reset PCIe.
...
quoted
quoted
+/**
+ * struct pci_dra7xx_platform_data - platform data specific to pci in dra7xx
+ * @reset_name: name of the reset line
+ * @assert_reset: callback for performing assert reset operation
+ * @deassert_reset: callback for performing deassert reset operation
+ */
+struct pci_dra7xx_platform_data {
+ const char *reset_name;
+
+ int (*assert_reset)(struct platform_device *pdev, const char *name);
+ int (*deassert_reset)(struct platform_device *pdev, const char *name);
+};
I doubt this platform_data is dra7 specific. I believe it's
the same PCI controller that has been in the omap variants for
years?
AFAIK, this only applies to DRA7. Sekhar/Kishon can confirm. I did take
a quick look at OMAP3/4/5 TRMs, and didn't find any. Neither did a grep
on current hwmod files other than DRA7. There's a DM81xx related PCI
clock domain, but don't see any corresponding driver/device for the same.
regards
Suman
From: Sekhar Nori <hidden> Date: 2016-01-18 09:13:41
On Saturday 16 January 2016 01:11 AM, Suman Anna wrote:
On 01/15/2016 01:22 PM, Tony Lindgren wrote:
quoted
* Suman Anna [off-list ref] [160115 11:20]:
quoted
On 01/14/2016 08:11 AM, Kishon Vijay Abraham I wrote:
quoted
Create platform data for PCIe and populate it with function
pointers to perform assert and deassert of PCIe reset lines.
The PCIe driver can use the callbacks provided here to
reset the PCIe.
This will be removed once the reset contoller driver is
available to reset PCIe.
...
quoted
quoted
+/**
+ * struct pci_dra7xx_platform_data - platform data specific to pci in dra7xx
+ * @reset_name: name of the reset line
+ * @assert_reset: callback for performing assert reset operation
+ * @deassert_reset: callback for performing deassert reset operation
+ */
+struct pci_dra7xx_platform_data {
+ const char *reset_name;
+
+ int (*assert_reset)(struct platform_device *pdev, const char *name);
+ int (*deassert_reset)(struct platform_device *pdev, const char *name);
+};
I doubt this platform_data is dra7 specific. I believe it's
the same PCI controller that has been in the omap variants for
years?
AFAIK, this only applies to DRA7. Sekhar/Kishon can confirm. I did take
a quick look at OMAP3/4/5 TRMs, and didn't find any. Neither did a grep
on current hwmod files other than DRA7. There's a DM81xx related PCI
clock domain, but don't see any corresponding driver/device for the same.
Like Suman, I do not know of any TI SoC that came off the OMAP mobile
business that has PCIe.
DM81xx has a PCIe (but no mainline driver). Both DM81xx and DRA7x use a
designware core. But, the glue layer (which is the subject of interest
here), is completely different. I looked at the DM81xx driver in TI
kernel[1] to confirm this.
I remember talking to Kishon about similarities between the DM81xx and
DRA7x PCIe subsystem and remember that he too mentioned that they are
quite different.
For an IP like PCIeSS, its quite difficult to come-up with unique names
without using the name of the platform they first appeared in. Anyway,
the driver is already called "pci-dra7xx", so I guess there is no harm
in having that name in platform data as well. That in itself should not
preclude its use on other platforms later (although I agree having a
generic name would be ideal).
Thanks,
Sekhar
PS: Kishon is out-of-office till the end of the month
[1]
http://arago-project.org/git/projects/?p=linux-omap3.git;a=blob;f=arch/arm/mach-omap2/pcie-ti81xx.c;h=05ae4f1df85a35e91a770435c50777a31de4f1ca;hb=4f1fb3bea4cc381c76e8e439f8af393c1a698dfc#l70
From: Tony Lindgren <tony@atomide.com> Date: 2016-01-27 17:23:29
* Sekhar Nori [off-list ref] [160118 01:13]:
On Saturday 16 January 2016 01:11 AM, Suman Anna wrote:
quoted
On 01/15/2016 01:22 PM, Tony Lindgren wrote:
quoted
I doubt this platform_data is dra7 specific. I believe it's
the same PCI controller that has been in the omap variants for
years?
AFAIK, this only applies to DRA7. Sekhar/Kishon can confirm. I did take
a quick look at OMAP3/4/5 TRMs, and didn't find any. Neither did a grep
on current hwmod files other than DRA7. There's a DM81xx related PCI
clock domain, but don't see any corresponding driver/device for the same.
Like Suman, I do not know of any TI SoC that came off the OMAP mobile
business that has PCIe.
DM81xx has a PCIe (but no mainline driver). Both DM81xx and DRA7x use a
designware core. But, the glue layer (which is the subject of interest
here), is completely different. I looked at the DM81xx driver in TI
kernel[1] to confirm this.
OK thanks for checking.
I remember talking to Kishon about similarities between the DM81xx and
DRA7x PCIe subsystem and remember that he too mentioned that they are
quite different.
OK
For an IP like PCIeSS, its quite difficult to come-up with unique names
without using the name of the platform they first appeared in. Anyway,
the driver is already called "pci-dra7xx", so I guess there is no harm
in having that name in platform data as well. That in itself should not
preclude its use on other platforms later (although I agree having a
generic name would be ideal).
Yeah seems OK to me. I still have some PM runtime related questions
though.. Will reply to the related patch chunk though.
Regards,
Tony
From: Tony Lindgren <tony@atomide.com> Date: 2016-01-27 17:31:09
* Kishon Vijay Abraham I [off-list ref] [160114 06:12]:
Use assert/deassert callbacks populated in the platform data to
to perform reset of PCIe.
Use these callbacks until a reset controller driver is
is available in the kernel to reset PCIe.
@@ -347,6 +404,10 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)enumof_gpio_flagsflags;unsignedlonggpio_flags;+ret=dra7xx_pcie_reset(pdev);+if(ret)+returnret;+dra7xx=devm_kzalloc(dev,sizeof(*dra7xx),GFP_KERNEL);if(!dra7xx)return-ENOMEM;
With the hwmod data properly configured the reset already happens
for the device by the bus driver, the hwmod code in this case?
quoted hunk
@@ -457,6 +518,7 @@ static int __exit dra7xx_pcie_remove(struct platform_device *pdev) struct pcie_port *pp = &dra7xx->pp; struct device *dev = &pdev->dev; int count = dra7xx->phy_count;+ int ret; if (pp->irq_domain) irq_domain_remove(pp->irq_domain);
@@ -467,6 +529,10 @@ static int __exit dra7xx_pcie_remove(struct platform_device *pdev) phy_exit(dra7xx->phy[count]); }+ ret = dra7xx_pcie_assert_reset(pdev);+ if (ret < 0)+ return ret;+ return 0; }
Why do you need another reset here? Can't you just implement PM runtime
in the driver and do the usual pm_runtime_put_sync followed by
pm_runtime_disable?
Basically I'm wondering how come we need these platform data callbacks
at all.
Regards,
Tony
From: Suman Anna <hidden> Date: 2016-01-27 18:17:18
Hi Tony,
On 01/27/2016 11:31 AM, Tony Lindgren wrote:
* Kishon Vijay Abraham I [off-list ref] [160114 06:12]:
quoted
Use assert/deassert callbacks populated in the platform data to
to perform reset of PCIe.
Use these callbacks until a reset controller driver is
is available in the kernel to reset PCIe.
@@ -347,6 +404,10 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)enumof_gpio_flagsflags;unsignedlonggpio_flags;+ret=dra7xx_pcie_reset(pdev);+if(ret)+returnret;+dra7xx=devm_kzalloc(dev,sizeof(*dra7xx),GFP_KERNEL);if(!dra7xx)return-ENOMEM;
With the hwmod data properly configured the reset already happens
for the device by the bus driver, the hwmod code in this case?
quoted
@@ -457,6 +518,7 @@ static int __exit dra7xx_pcie_remove(struct platform_device *pdev) struct pcie_port *pp = &dra7xx->pp; struct device *dev = &pdev->dev; int count = dra7xx->phy_count;+ int ret; if (pp->irq_domain) irq_domain_remove(pp->irq_domain);
@@ -467,6 +529,10 @@ static int __exit dra7xx_pcie_remove(struct platform_device *pdev) phy_exit(dra7xx->phy[count]); }+ ret = dra7xx_pcie_assert_reset(pdev);+ if (ret < 0)+ return ret;+ return 0; }
Why do you need another reset here? Can't you just implement PM runtime
in the driver and do the usual pm_runtime_put_sync followed by
pm_runtime_disable?
The omap_hwmod_enable/disable code does not deal with hardresets (PRCM
reset lines) and so the pm_runtime_get_sync/put_sync only end up dealing
with clocks, and we need to invoke the reset functions separately.
Modules with softresets in SYSCONFIG are ok, as they are dealt with
properly.
Basically I'm wondering how come we need these platform data callbacks
at all.
The hardresets are controlled through the
omap_device_assert(deassert)_hardreset functions, and since these are
limited to mach-omap2, we are invoking them through platform data callbacks.
regards
Suman
From: Tony Lindgren <tony@atomide.com> Date: 2016-01-27 18:56:55
* Suman Anna [off-list ref] [160127 10:17]:
On 01/27/2016 11:31 AM, Tony Lindgren wrote:
quoted
Why do you need another reset here? Can't you just implement PM runtime
in the driver and do the usual pm_runtime_put_sync followed by
pm_runtime_disable?
The omap_hwmod_enable/disable code does not deal with hardresets (PRCM
reset lines) and so the pm_runtime_get_sync/put_sync only end up dealing
with clocks, and we need to invoke the reset functions separately.
Modules with softresets in SYSCONFIG are ok, as they are dealt with
properly.
Hmm _reset() in omap_hwmod.c has this to call _assert_hardreset:
if (oh->class->reset) {
r = oh->class->reset(oh);
} else {
if (oh->rst_lines_cnt > 0) {
for (i = 0; i < oh->rst_lines_cnt; i++)
_assert_hardreset(oh, oh->rst_lines[i].name);
return 0;
} else {
r = _ocp_softreset(oh);
if (r == -ENOENT)
r = 0;
}
}
Care to explain what exactly the problem with the hwmod code not doing
the reset on init?
And why do you need to do another reset in dra7xx_pcie_remove()?
quoted
Basically I'm wondering how come we need these platform data callbacks
at all.
The hardresets are controlled through the
omap_device_assert(deassert)_hardreset functions, and since these are
limited to mach-omap2, we are invoking them through platform data callbacks.
Right.. But I'm wondering about the why you need to do this in the
driver at all part :)
Regards,
Tony
From: Suman Anna <hidden> Date: 2016-01-27 23:16:46
On 01/27/2016 12:56 PM, Tony Lindgren wrote:
* Suman Anna [off-list ref] [160127 10:17]:
quoted
On 01/27/2016 11:31 AM, Tony Lindgren wrote:
quoted
Why do you need another reset here? Can't you just implement PM runtime
in the driver and do the usual pm_runtime_put_sync followed by
pm_runtime_disable?
The omap_hwmod_enable/disable code does not deal with hardresets (PRCM
reset lines) and so the pm_runtime_get_sync/put_sync only end up dealing
with clocks, and we need to invoke the reset functions separately.
Modules with softresets in SYSCONFIG are ok, as they are dealt with
properly.
Hmm _reset() in omap_hwmod.c has this to call _assert_hardreset:
if (oh->class->reset) {
r = oh->class->reset(oh);
} else {
if (oh->rst_lines_cnt > 0) {
for (i = 0; i < oh->rst_lines_cnt; i++)
_assert_hardreset(oh, oh->rst_lines[i].name);
return 0;
} else {
r = _ocp_softreset(oh);
if (r == -ENOENT)
r = 0;
}
}
Right, hwmod code does the initial reset.
Care to explain what exactly the problem with the hwmod code not doing
the reset on init?
And we only need to deassert the reset in probe. Technically, we don't
need to assert first and deassert in probe, and that was a design choice
made by Kishon.
And why do you need to do another reset in dra7xx_pcie_remove()?
Primarily to restore the reset state back to what it was after the
driver remove gets called. We cannot call deassert twice without calling
a assert in between. Kishon had originally added the assert and deassert
only in probe, but nothing in remove, they ought to be deassert in probe
and assert in remove to match initial hardware state, and to also make
it work across multiple probe/remove.
quoted
quoted
Basically I'm wondering how come we need these platform data callbacks
at all.
The hardresets are controlled through the
omap_device_assert(deassert)_hardreset functions, and since these are
limited to mach-omap2, we are invoking them through platform data callbacks.
Right.. But I'm wondering about the why you need to do this in the
driver at all part :)
The initial reset at init time is okay, but hwmod _enable() bails out if
the resets lines are asserted. This was a change made long time back, I
believe to deal with the problems around the DSP enabling sequences. As
such, pm_runtime_get_sync() and put_sync() do not deassert and assert
the resets.
regards
Suman
From: Tony Lindgren <tony@atomide.com> Date: 2016-01-28 18:32:03
* Suman Anna [off-list ref] [160127 15:17]:
On 01/27/2016 12:56 PM, Tony Lindgren wrote:
quoted
* Suman Anna [off-list ref] [160127 10:17]:
quoted
On 01/27/2016 11:31 AM, Tony Lindgren wrote:
quoted
Why do you need another reset here? Can't you just implement PM runtime
in the driver and do the usual pm_runtime_put_sync followed by
pm_runtime_disable?
The omap_hwmod_enable/disable code does not deal with hardresets (PRCM
reset lines) and so the pm_runtime_get_sync/put_sync only end up dealing
with clocks, and we need to invoke the reset functions separately.
Modules with softresets in SYSCONFIG are ok, as they are dealt with
properly.
Hmm _reset() in omap_hwmod.c has this to call _assert_hardreset:
if (oh->class->reset) {
r = oh->class->reset(oh);
} else {
if (oh->rst_lines_cnt > 0) {
for (i = 0; i < oh->rst_lines_cnt; i++)
_assert_hardreset(oh, oh->rst_lines[i].name);
return 0;
} else {
r = _ocp_softreset(oh);
if (r == -ENOENT)
r = 0;
}
}
Right, hwmod code does the initial reset.
quoted
Care to explain what exactly the problem with the hwmod code not doing
the reset on init?
And we only need to deassert the reset in probe. Technically, we don't
need to assert first and deassert in probe, and that was a design choice
made by Kishon.
OK so if hwmod code has already done the reset, then why would you need
to deassert reset in the device driver probe?
quoted
And why do you need to do another reset in dra7xx_pcie_remove()?
Primarily to restore the reset state back to what it was after the
driver remove gets called. We cannot call deassert twice without calling
a assert in between. Kishon had originally added the assert and deassert
only in probe, but nothing in remove, they ought to be deassert in probe
and assert in remove to match initial hardware state, and to also make
it work across multiple probe/remove.
I don't understand this part either.. Usually you just power up and init
the registers to a sane state in a device driver probe and on exit just
power down the device.
quoted
quoted
quoted
Basically I'm wondering how come we need these platform data callbacks
at all.
The hardresets are controlled through the
omap_device_assert(deassert)_hardreset functions, and since these are
limited to mach-omap2, we are invoking them through platform data callbacks.
Right.. But I'm wondering about the why you need to do this in the
driver at all part :)
The initial reset at init time is okay, but hwmod _enable() bails out if
the resets lines are asserted. This was a change made long time back, I
believe to deal with the problems around the DSP enabling sequences. As
such, pm_runtime_get_sync() and put_sync() do not deassert and assert
the resets.
OK if the hwmod code does not deassert reset lines properly on enable,
then that sounds like a bug that should be fixed instead of adding
device specific work arounds.
Sorry to keep dragging this on a bit longer, but I think we need to
hear Paul's comments on this one.
Regards,
Tony
From: Suman Anna <hidden> Date: 2016-01-28 21:16:34
On 01/28/2016 12:31 PM, Tony Lindgren wrote:
* Suman Anna [off-list ref] [160127 15:17]:
quoted
On 01/27/2016 12:56 PM, Tony Lindgren wrote:
quoted
* Suman Anna [off-list ref] [160127 10:17]:
quoted
On 01/27/2016 11:31 AM, Tony Lindgren wrote:
quoted
Why do you need another reset here? Can't you just implement PM runtime
in the driver and do the usual pm_runtime_put_sync followed by
pm_runtime_disable?
The omap_hwmod_enable/disable code does not deal with hardresets (PRCM
reset lines) and so the pm_runtime_get_sync/put_sync only end up dealing
with clocks, and we need to invoke the reset functions separately.
Modules with softresets in SYSCONFIG are ok, as they are dealt with
properly.
Hmm _reset() in omap_hwmod.c has this to call _assert_hardreset:
if (oh->class->reset) {
r = oh->class->reset(oh);
} else {
if (oh->rst_lines_cnt > 0) {
for (i = 0; i < oh->rst_lines_cnt; i++)
_assert_hardreset(oh, oh->rst_lines[i].name);
return 0;
} else {
r = _ocp_softreset(oh);
if (r == -ENOENT)
r = 0;
}
}
Right, hwmod code does the initial reset.
quoted
Care to explain what exactly the problem with the hwmod code not doing
the reset on init?
And we only need to deassert the reset in probe. Technically, we don't
need to assert first and deassert in probe, and that was a design choice
made by Kishon.
OK so if hwmod code has already done the reset, then why would you need
to deassert reset in the device driver probe?
So the _reset() above asserts the reset for IPs with PRCM reset lines,
but module is not enabled (no register accesses even if clocks enabled).
The _enable() code bails out if there are PRCM reset lines (there are
varied IPs with resets including processors, and we really cannot enable
and idle it without loading some code that would have executed WFI).
quoted
quoted
And why do you need to do another reset in dra7xx_pcie_remove()?
Primarily to restore the reset state back to what it was after the
driver remove gets called. We cannot call deassert twice without calling
a assert in between. Kishon had originally added the assert and deassert
only in probe, but nothing in remove, they ought to be deassert in probe
and assert in remove to match initial hardware state, and to also make
it work across multiple probe/remove.
I don't understand this part either.. Usually you just power up and init
the registers to a sane state in a device driver probe and on exit just
power down the device.
Yes, in the case of IPs with hard-reset lines, that init is left to the
drivers.
quoted
quoted
quoted
quoted
Basically I'm wondering how come we need these platform data callbacks
at all.
The hardresets are controlled through the
omap_device_assert(deassert)_hardreset functions, and since these are
limited to mach-omap2, we are invoking them through platform data callbacks.
Right.. But I'm wondering about the why you need to do this in the
driver at all part :)
The initial reset at init time is okay, but hwmod _enable() bails out if
the resets lines are asserted. This was a change made long time back, I
believe to deal with the problems around the DSP enabling sequences. As
such, pm_runtime_get_sync() and put_sync() do not deassert and assert
the resets.
OK if the hwmod code does not deassert reset lines properly on enable,
then that sounds like a bug that should be fixed instead of adding
device specific work arounds.
As I said above, not all IPs with hard-reset lines have the same power
on/power off sequences.. IPs with only SYSCONFIG based soft-reset all
pretty much follow the PRCM HW_Auto idling, so dealing with them is
rather straightforward in the common hwmod code. I have had to do rather
funky stuff in our product kernels when doing suspend/resume on IOMMUs,
remoteprocs.
Sorry to keep dragging this on a bit longer, but I think we need to
hear Paul's comments on this one.
Yeah, it would be good to restart this discussion, as I will be adding
the DT support for the remoteproc devices a bit later.
regards
Suman
From: Kishon Vijay Abraham I <hidden> Date: 2016-02-02 10:41:30
Hi,
On Friday 29 January 2016 12:01 AM, Tony Lindgren wrote:
* Suman Anna [off-list ref] [160127 15:17]:
quoted
On 01/27/2016 12:56 PM, Tony Lindgren wrote:
quoted
* Suman Anna [off-list ref] [160127 10:17]:
quoted
On 01/27/2016 11:31 AM, Tony Lindgren wrote:
quoted
Why do you need another reset here? Can't you just implement PM runtime
in the driver and do the usual pm_runtime_put_sync followed by
pm_runtime_disable?
The omap_hwmod_enable/disable code does not deal with hardresets (PRCM
reset lines) and so the pm_runtime_get_sync/put_sync only end up dealing
with clocks, and we need to invoke the reset functions separately.
Modules with softresets in SYSCONFIG are ok, as they are dealt with
properly.
Hmm _reset() in omap_hwmod.c has this to call _assert_hardreset:
if (oh->class->reset) {
r = oh->class->reset(oh);
} else {
if (oh->rst_lines_cnt > 0) {
for (i = 0; i < oh->rst_lines_cnt; i++)
_assert_hardreset(oh, oh->rst_lines[i].name);
return 0;
} else {
r = _ocp_softreset(oh);
if (r == -ENOENT)
r = 0;
}
}
Right, hwmod code does the initial reset.
quoted
Care to explain what exactly the problem with the hwmod code not doing
the reset on init?
And we only need to deassert the reset in probe. Technically, we don't
need to assert first and deassert in probe, and that was a design choice
made by Kishon.
OK so if hwmod code has already done the reset, then why would you need
to deassert reset in the device driver probe?
The hwmod code only asserts the reset lines and that is not enough to access
the PCI registers. The reset lines must be de-asserted before accessing the
PCIe registers.
quoted
quoted
And why do you need to do another reset in dra7xx_pcie_remove()?
Primarily to restore the reset state back to what it was after the
driver remove gets called. We cannot call deassert twice without calling
a assert in between. Kishon had originally added the assert and deassert
only in probe, but nothing in remove, they ought to be deassert in probe
and assert in remove to match initial hardware state, and to also make
it work across multiple probe/remove.
right. I thought if some program like the bootloader requires the reset lines
to be in initial hw state, then it might break on 'reboot'. So restored it back
to the initial hw state.
I don't understand this part either.. Usually you just power up and init
the registers to a sane state in a device driver probe and on exit just
power down the device.
quoted
quoted
quoted
quoted
Basically I'm wondering how come we need these platform data callbacks
at all.
The hardresets are controlled through the
omap_device_assert(deassert)_hardreset functions, and since these are
limited to mach-omap2, we are invoking them through platform data callbacks.
Right.. But I'm wondering about the why you need to do this in the
driver at all part :)
The initial reset at init time is okay, but hwmod _enable() bails out if
the resets lines are asserted. This was a change made long time back, I
believe to deal with the problems around the DSP enabling sequences. As
such, pm_runtime_get_sync() and put_sync() do not deassert and assert
the resets.
OK if the hwmod code does not deassert reset lines properly on enable,
then that sounds like a bug that should be fixed instead of adding
device specific work arounds.
I think some devices require the reset lines to be asserted and some devices
require it to be de-asserted and hwmod was designed when there was only the
first type of devices. I'm not sure though.
Sorry to keep dragging this on a bit longer, but I think we need to
hear Paul's comments on this one.
I agree.
Paul, what do you think is the best way forward to perform reset?
Thanks
Kishon
From: Kishon Vijay Abraham I <hidden> Date: 2016-02-05 04:20:37
Hi Paul,
On Tuesday 02 February 2016 04:10 PM, Kishon Vijay Abraham I wrote:
Hi,
On Friday 29 January 2016 12:01 AM, Tony Lindgren wrote:
quoted
* Suman Anna [off-list ref] [160127 15:17]:
quoted
On 01/27/2016 12:56 PM, Tony Lindgren wrote:
quoted
* Suman Anna [off-list ref] [160127 10:17]:
quoted
On 01/27/2016 11:31 AM, Tony Lindgren wrote:
quoted
Why do you need another reset here? Can't you just implement PM runtime
in the driver and do the usual pm_runtime_put_sync followed by
pm_runtime_disable?
The omap_hwmod_enable/disable code does not deal with hardresets (PRCM
reset lines) and so the pm_runtime_get_sync/put_sync only end up dealing
with clocks, and we need to invoke the reset functions separately.
Modules with softresets in SYSCONFIG are ok, as they are dealt with
properly.
Hmm _reset() in omap_hwmod.c has this to call _assert_hardreset:
if (oh->class->reset) {
r = oh->class->reset(oh);
} else {
if (oh->rst_lines_cnt > 0) {
for (i = 0; i < oh->rst_lines_cnt; i++)
_assert_hardreset(oh, oh->rst_lines[i].name);
return 0;
} else {
r = _ocp_softreset(oh);
if (r == -ENOENT)
r = 0;
}
}
Right, hwmod code does the initial reset.
quoted
Care to explain what exactly the problem with the hwmod code not doing
the reset on init?
And we only need to deassert the reset in probe. Technically, we don't
need to assert first and deassert in probe, and that was a design choice
made by Kishon.
OK so if hwmod code has already done the reset, then why would you need
to deassert reset in the device driver probe?
The hwmod code only asserts the reset lines and that is not enough to access
the PCI registers. The reset lines must be de-asserted before accessing the
PCIe registers.
quoted
quoted
quoted
And why do you need to do another reset in dra7xx_pcie_remove()?
Primarily to restore the reset state back to what it was after the
driver remove gets called. We cannot call deassert twice without calling
a assert in between. Kishon had originally added the assert and deassert
only in probe, but nothing in remove, they ought to be deassert in probe
and assert in remove to match initial hardware state, and to also make
it work across multiple probe/remove.
right. I thought if some program like the bootloader requires the reset lines
to be in initial hw state, then it might break on 'reboot'. So restored it back
to the initial hw state.
quoted
I don't understand this part either.. Usually you just power up and init
the registers to a sane state in a device driver probe and on exit just
power down the device.
quoted
quoted
quoted
quoted
Basically I'm wondering how come we need these platform data callbacks
at all.
The hardresets are controlled through the
omap_device_assert(deassert)_hardreset functions, and since these are
limited to mach-omap2, we are invoking them through platform data callbacks.
Right.. But I'm wondering about the why you need to do this in the
driver at all part :)
The initial reset at init time is okay, but hwmod _enable() bails out if
the resets lines are asserted. This was a change made long time back, I
believe to deal with the problems around the DSP enabling sequences. As
such, pm_runtime_get_sync() and put_sync() do not deassert and assert
the resets.
OK if the hwmod code does not deassert reset lines properly on enable,
then that sounds like a bug that should be fixed instead of adding
device specific work arounds.
I think some devices require the reset lines to be asserted and some devices
require it to be de-asserted and hwmod was designed when there was only the
first type of devices. I'm not sure though.
quoted
Sorry to keep dragging this on a bit longer, but I think we need to
hear Paul's comments on this one.
I agree.
Paul, what do you think is the best way forward to perform reset?
Can you give your feedback as we are at the risk of PCIe driver being removed?
Thanks
Kishon
Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Paul Walmsley <paul@pwsan.com> Date: 2016-02-08 01:50:58
On Thu, 14 Jan 2016, Kishon Vijay Abraham I wrote:
Add PCIe reset data to PCIe hwmods on DRA7x.
Signed-off-by: Kishon Vijay Abraham I <redacted>
Signed-off-by: Sekhar Nori <redacted>
Reviewed-by: Suman Anna <redacted>
From: Paul Walmsley <paul@pwsan.com> Date: 2016-02-08 02:49:00
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs? So how about just creating a new hwmod flag to
mark all of the initiator IP blocks that require driver-based special
handling during _enable() - i.e., most of the processor IP blocks. Then
for the rest, like PCIe, implement a default behavior in the hwmod code to
automatically release the IP block's hardreset lines in
omap_hwmod.c:_enable()? Something similar to what's enclosed at the
bottom of this message. I've annotated what will be needed in the
OMAP44xx file; similar flags will be needed in any other hwmod data file
that contains IP blocks with hard reset lines defined.
Either that - or you could write custom reset handlers for all of the
processor IP blocks that put them into WFI/HLT.
I leave it to you TI folks to write and test the actual patches, since as
you probably know, I don't have any DRA7xx/AM57xx boards in the testbed.
As far as reasserting hardreset in *remove(), there's already hwmod code
to do that in omap_hwmod.c:_shutdown(). I don't recall any more if we
currently have code in the stack that calls it. Ideally the device model
code should call that during or after a .remove() call.
- Paul
---
arch/arm/mach-omap2/omap_hwmod.c | 16 +++++++++++-----
arch/arm/mach-omap2/omap_hwmod.h | 12 ++++++++++++
arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 6 ++++++
3 files changed, 29 insertions(+), 5 deletions(-)
@@ -2109,17 +2109,23 @@ static int _enable(struct omap_hwmod *oh)}/*-*IfanIPblockcontainsHWresetlinesandallofthemare-*asserted,weletintegrationcodeassociatedwiththat-*blockhandletheenable.We'vereceivedverylittle+*IfanIPblockcontainsHWresetlines,allofthemare+*asserted,andtheIPblockismarkedasrequiringacustom+*hardresethandler,weletintegrationcodeassociatedwith+*thatblockhandletheenable.We'vereceivedverylittle*informationonwhatthosedriverauthorsneed,anduntil*detailedinformationisprovidedandthedrivercodeis*postedtothepubliclists,thisisprobablythebestwe*cando.*/-if(_are_all_hardreset_lines_asserted(oh))+if((oh->flags&HWMOD_CUSTOM_HARDRESET)&&+_are_all_hardreset_lines_asserted(oh))return0;+/* If the IP block is an initiator, release it from hardreset */+for(i=0;i<oh->rst_lines_cnt;i++)+_deassert_hardreset(oh,oh->rst_lines[i].name);+/* Mux pins for device runtime if populated */if(oh->mux&&(!oh->mux->enabled||((oh->_state==_HWMOD_STATE_IDLE)&&
From: Suman Anna <hidden> Date: 2016-02-08 20:56:51
Hi Paul,
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs?
Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.
So how about just creating a new hwmod flag to
mark all of the initiator IP blocks that require driver-based special
handling during _enable() - i.e., most of the processor IP blocks. Then
for the rest, like PCIe, implement a default behavior in the hwmod code to
automatically release the IP block's hardreset lines in
omap_hwmod.c:_enable()? Something similar to what's enclosed at the
bottom of this message. I've annotated what will be needed in the
OMAP44xx file; similar flags will be needed in any other hwmod data file
that contains IP blocks with hard reset lines defined.
Either that - or you could write custom reset handlers for all of the
processor IP blocks that put them into WFI/HLT.
I leave it to you TI folks to write and test the actual patches, since as
you probably know, I don't have any DRA7xx/AM57xx boards in the testbed.
As far as reasserting hardreset in *remove(), there's already hwmod code
to do that in omap_hwmod.c:_shutdown(). I don't recall any more if we
currently have code in the stack that calls it. Ideally the device model
code should call that during or after a .remove() call.
Yeah, don't think there is any code that exercises
omap_hwmod_shutdown(). We used to have an omap_device_shutdown() but
that function has been removed in commit c1d1cd597fc7 ("ARM: OMAP2+:
omap_device: remove obsolete pm_lats and early_device code"). We used to
exercise this using custom pm_lats replacing idle with shutdown in the
out-of-tree processor drivers.
@@ -2109,17 +2109,23 @@ static int _enable(struct omap_hwmod *oh)}/*-*IfanIPblockcontainsHWresetlinesandallofthemare-*asserted,weletintegrationcodeassociatedwiththat-*blockhandletheenable.We'vereceivedverylittle+*IfanIPblockcontainsHWresetlines,allofthemare+*asserted,andtheIPblockismarkedasrequiringacustom+*hardresethandler,weletintegrationcodeassociatedwith+*thatblockhandletheenable.We'vereceivedverylittle*informationonwhatthosedriverauthorsneed,anduntil*detailedinformationisprovidedandthedrivercodeis*postedtothepubliclists,thisisprobablythebestwe*cando.*/-if(_are_all_hardreset_lines_asserted(oh))+if((oh->flags&HWMOD_CUSTOM_HARDRESET)&&+_are_all_hardreset_lines_asserted(oh))return0;+/* If the IP block is an initiator, release it from hardreset */+for(i=0;i<oh->rst_lines_cnt;i++)+_deassert_hardreset(oh,oh->rst_lines[i].name);
I believe this will cause a problem as typically we release the reset
and then call pm_runtime_get_sync() to enable the clock. We are not
checking error code, but if were, I do think _deassert_hardreset would
return a failure.
regards
Suman
quoted hunk
+
/* Mux pins for device runtime if populated */
if (oh->mux && (!oh->mux->enabled ||
((oh->_state == _HWMOD_STATE_IDLE) &&
From: Paul Walmsley <paul@pwsan.com> Date: 2016-02-09 08:49:16
On Mon, 8 Feb 2016, Suman Anna wrote:
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
quoted
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs?
Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.
I'm curious as to whether HWMOD_CUSTOM_HARDRESET is needed for the MMUs.
We've stated that the main point of the custom hardreset code is to handle
processors that need to be placed into WFI/HLT, but it doesn't seem like
there would be an equivalent for MMUs. Thoughts?
- Paul
From: Suman Anna <hidden> Date: 2016-02-09 17:41:27
Hi Paul,
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
On Mon, 8 Feb 2016, Suman Anna wrote:
quoted
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
quoted
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs?
Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.
I'm curious as to whether HWMOD_CUSTOM_HARDRESET is needed for the MMUs.
We've stated that the main point of the custom hardreset code is to handle
processors that need to be placed into WFI/HLT, but it doesn't seem like
there would be an equivalent for MMUs. Thoughts?
The current OMAP IOMMU code already leverages the pdata ops for
performing the resets, so not adding the flags would also require
additional changes in the driver.
Also, the reset lines controlling the MMUs actually also manage the
reset for all the other sub-modules other than the processor cores
within the sub-systems. We have currently different issues (see [1] for
eg. around the IPU sub-system entering RET in between), so from a PM
point of view, we do prefer to place the MMUs also in reset when we are
runtime suspended.
regards
Suman
[1]
http://git.ti.com/gitweb/?p=rpmsg/rpmsg.git;a=commit;h=a7db749a8a0fdfe7baa185db9f5071789a889061
From: Paul Walmsley <paul@pwsan.com> Date: 2016-02-09 19:36:43
Hi Suman
On Tue, 9 Feb 2016, Suman Anna wrote:
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
quoted
On Mon, 8 Feb 2016, Suman Anna wrote:
quoted
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
quoted
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs?
Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.
I'm curious as to whether HWMOD_CUSTOM_HARDRESET is needed for the MMUs.
We've stated that the main point of the custom hardreset code is to handle
processors that need to be placed into WFI/HLT, but it doesn't seem like
there would be an equivalent for MMUs. Thoughts?
The current OMAP IOMMU code already leverages the pdata ops for
performing the resets, so not adding the flags would also require
additional changes in the driver.
Also, the reset lines controlling the MMUs actually also manage the
reset for all the other sub-modules other than the processor cores
within the sub-systems. We have currently different issues (see [1] for
eg. around the IPU sub-system entering RET in between), so from a PM
point of view, we do prefer to place the MMUs also in reset when we are
runtime suspended.
Should we reassert hardreset in _idle() for IP blocks that don't have
HWMOD_CUSTOM_HARDRESET set on them? Would that allow us to use this
mechanism for the uncore hardreset lines, or are there other quirks?
Also - would that address the potential issue that you mentioned with the
PCIe block, or is that a different issue?
- Paul
From: Suman Anna <hidden> Date: 2016-02-10 01:43:16
Hi Paul,
On 02/09/2016 01:36 PM, Paul Walmsley wrote:
Hi Suman
On Tue, 9 Feb 2016, Suman Anna wrote:
quoted
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
quoted
On Mon, 8 Feb 2016, Suman Anna wrote:
quoted
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
quoted
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs?
Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.
I'm curious as to whether HWMOD_CUSTOM_HARDRESET is needed for the MMUs.
We've stated that the main point of the custom hardreset code is to handle
processors that need to be placed into WFI/HLT, but it doesn't seem like
there would be an equivalent for MMUs. Thoughts?
The current OMAP IOMMU code already leverages the pdata ops for
performing the resets, so not adding the flags would also require
additional changes in the driver.
Also, the reset lines controlling the MMUs actually also manage the
reset for all the other sub-modules other than the processor cores
within the sub-systems. We have currently different issues (see [1] for
eg. around the IPU sub-system entering RET in between), so from a PM
point of view, we do prefer to place the MMUs also in reset when we are
runtime suspended.
Should we reassert hardreset in _idle() for IP blocks that don't have
HWMOD_CUSTOM_HARDRESET set on them? Would that allow us to use this
mechanism for the uncore hardreset lines, or are there other quirks?
Also - would that address the potential issue that you mentioned with the
PCIe block, or is that a different issue?
Yeah, I think that would address the PCIe block issue in terms of reset
state balancing between pm_runtime_get_sync() and pm_runtime_put()
calls. Right now, they are unbalanced. The PCIe block is using these
only in probe and remove, so it should work for that IP.
The IPUs and DSPs in general would also place the reset lines asserted
when suspended, as the power up sequence almost always involves
releasing a reset line with the boot-up code on the processor detecting
that it is a power restore boot.
regards
Suman
From: Kishon Vijay Abraham I <hidden> Date: 2016-02-10 05:37:36
On Wednesday 10 February 2016 01:06 AM, Paul Walmsley wrote:
Hi Suman
On Tue, 9 Feb 2016, Suman Anna wrote:
quoted
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
quoted
On Mon, 8 Feb 2016, Suman Anna wrote:
quoted
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
quoted
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs?
Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.
I'm curious as to whether HWMOD_CUSTOM_HARDRESET is needed for the MMUs.
We've stated that the main point of the custom hardreset code is to handle
processors that need to be placed into WFI/HLT, but it doesn't seem like
there would be an equivalent for MMUs. Thoughts?
The current OMAP IOMMU code already leverages the pdata ops for
performing the resets, so not adding the flags would also require
additional changes in the driver.
Also, the reset lines controlling the MMUs actually also manage the
reset for all the other sub-modules other than the processor cores
within the sub-systems. We have currently different issues (see [1] for
eg. around the IPU sub-system entering RET in between), so from a PM
point of view, we do prefer to place the MMUs also in reset when we are
runtime suspended.
Should we reassert hardreset in _idle() for IP blocks that don't have
HWMOD_CUSTOM_HARDRESET set on them? Would that allow us to use this
mechanism for the uncore hardreset lines, or are there other quirks?
IIUC _idle() will be called on pm_runtime_put. That would mean we perform reset
on every suspend/resume cycle which is not desired. (suspend/resume support for
PCIe is not yet in mainline but once we have it runtime_put and runtime_get
will be invoked during suspend/resume cycle). Let me know if my understanding
is wrong.
Thanks
Kishon
Also - would that address the potential issue that you mentioned with the
PCIe block, or is that a different issue?
- Paul
From: Kishon Vijay Abraham I <hidden> Date: 2016-02-10 05:39:11
Hi,
On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
Hi Paul,
On 02/09/2016 01:36 PM, Paul Walmsley wrote:
quoted
Hi Suman
On Tue, 9 Feb 2016, Suman Anna wrote:
quoted
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
quoted
On Mon, 8 Feb 2016, Suman Anna wrote:
quoted
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
quoted
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs?
Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.
I'm curious as to whether HWMOD_CUSTOM_HARDRESET is needed for the MMUs.
We've stated that the main point of the custom hardreset code is to handle
processors that need to be placed into WFI/HLT, but it doesn't seem like
there would be an equivalent for MMUs. Thoughts?
The current OMAP IOMMU code already leverages the pdata ops for
performing the resets, so not adding the flags would also require
additional changes in the driver.
Also, the reset lines controlling the MMUs actually also manage the
reset for all the other sub-modules other than the processor cores
within the sub-systems. We have currently different issues (see [1] for
eg. around the IPU sub-system entering RET in between), so from a PM
point of view, we do prefer to place the MMUs also in reset when we are
runtime suspended.
Should we reassert hardreset in _idle() for IP blocks that don't have
HWMOD_CUSTOM_HARDRESET set on them? Would that allow us to use this
mechanism for the uncore hardreset lines, or are there other quirks?
Also - would that address the potential issue that you mentioned with the
PCIe block, or is that a different issue?
Yeah, I think that would address the PCIe block issue in terms of reset
state balancing between pm_runtime_get_sync() and pm_runtime_put()
calls. Right now, they are unbalanced. The PCIe block is using these
only in probe and remove, so it should work for that IP.
As I mentioned before this would result in undesired behavior during
suspend/resume cycle in PCIe. (This should be okay for the current mainline
code but would break once we add suspend/resume support for PCIe).
Thanks
Kishon
From: Paul Walmsley <paul@pwsan.com> Date: 2016-02-11 19:27:10
Hi Kishon, Suman,
On Wed, 10 Feb 2016, Kishon Vijay Abraham I wrote:
On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
quoted
On 02/09/2016 01:36 PM, Paul Walmsley wrote:
quoted
On Tue, 9 Feb 2016, Suman Anna wrote:
quoted
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
quoted
On Mon, 8 Feb 2016, Suman Anna wrote:
quoted
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
quoted
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs?
Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.
I'm curious as to whether HWMOD_CUSTOM_HARDRESET is needed for the MMUs.
We've stated that the main point of the custom hardreset code is to handle
processors that need to be placed into WFI/HLT, but it doesn't seem like
there would be an equivalent for MMUs. Thoughts?
The current OMAP IOMMU code already leverages the pdata ops for
performing the resets, so not adding the flags would also require
additional changes in the driver.
Also, the reset lines controlling the MMUs actually also manage the
reset for all the other sub-modules other than the processor cores
within the sub-systems. We have currently different issues (see [1] for
eg. around the IPU sub-system entering RET in between), so from a PM
point of view, we do prefer to place the MMUs also in reset when we are
runtime suspended.
Should we reassert hardreset in _idle() for IP blocks that don't have
HWMOD_CUSTOM_HARDRESET set on them? Would that allow us to use this
mechanism for the uncore hardreset lines, or are there other quirks?
Also - would that address the potential issue that you mentioned with the
PCIe block, or is that a different issue?
Yeah, I think that would address the PCIe block issue in terms of reset
state balancing between pm_runtime_get_sync() and pm_runtime_put()
calls. Right now, they are unbalanced. The PCIe block is using these
only in probe and remove, so it should work for that IP.
As I mentioned before this would result in undesired behavior during
suspend/resume cycle in PCIe. (This should be okay for the current mainline
code but would break once we add suspend/resume support for PCIe).
I'd like to understand where we're currently at here. It looks like we're
waiting for testing from Suman, and we're waiting for Kishon to try using
the bind/unbind driver model hook to see if that wedges PCIe? Does this
match your collective understanding of the status here?
Thinking about the question of what to do about hardreset assertion in
idle, if we need it, we could add a hwmod flag to control that mode. I
would consider it a temporary workaround until we have the hwmod code
moved into a bus driver and the bus driver/hwmod code can hook into the
LDM .remove operation (and connect it to .shutdown, etc.) Suman/Kishon:
is it your understanding that we could remove the existing hardreset
control in the IOMMU drivers and the PCIe driver if we had these options
in the hwmod code?
Dave, any further comments here?
- Paul
From: Suman Anna <hidden> Date: 2016-02-11 20:43:41
On 02/09/2016 11:38 PM, Kishon Vijay Abraham I wrote:
Hi,
On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
quoted
Hi Paul,
On 02/09/2016 01:36 PM, Paul Walmsley wrote:
quoted
Hi Suman
On Tue, 9 Feb 2016, Suman Anna wrote:
quoted
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
quoted
On Mon, 8 Feb 2016, Suman Anna wrote:
quoted
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
quoted
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs?
Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.
I'm curious as to whether HWMOD_CUSTOM_HARDRESET is needed for the MMUs.
We've stated that the main point of the custom hardreset code is to handle
processors that need to be placed into WFI/HLT, but it doesn't seem like
there would be an equivalent for MMUs. Thoughts?
The current OMAP IOMMU code already leverages the pdata ops for
performing the resets, so not adding the flags would also require
additional changes in the driver.
Also, the reset lines controlling the MMUs actually also manage the
reset for all the other sub-modules other than the processor cores
within the sub-systems. We have currently different issues (see [1] for
eg. around the IPU sub-system entering RET in between), so from a PM
point of view, we do prefer to place the MMUs also in reset when we are
runtime suspended.
Should we reassert hardreset in _idle() for IP blocks that don't have
HWMOD_CUSTOM_HARDRESET set on them? Would that allow us to use this
mechanism for the uncore hardreset lines, or are there other quirks?
Also - would that address the potential issue that you mentioned with the
PCIe block, or is that a different issue?
Yeah, I think that would address the PCIe block issue in terms of reset
state balancing between pm_runtime_get_sync() and pm_runtime_put()
calls. Right now, they are unbalanced. The PCIe block is using these
only in probe and remove, so it should work for that IP.
As I mentioned before this would result in undesired behavior during
suspend/resume cycle in PCIe. (This should be okay for the current mainline
code but would break once we add suspend/resume support for PCIe).
Yeah, I was wondering if some peripheral would want only the clock to be
controlled during _idle() and not reset. Even then for the PCIe case
that you are talking about, going through a pm_runtime_get_sync(),
pm_runtime_put_sync()/pm_runtime_put() deasserts the resets everytime
_enable() is called. Right now, the code block has ignored the return
value from the _hardreset_deassert(), but if you check it and bail out,
then your get_sync() would start failing from the second invocation.
Can you elaborate more on what kind of issues you will see on
suspend/resume cycle with PCIe? Do note that _idle() gets called through
_od_suspend_no_irq() in omap_device.c if your runtime status is not
suspended. I had to manage the runtime status in the IPU/DSP
suspend/resume code to deal with the reset
(omap_device_assert_hardreset) and clock sequences in
_idle()/omap_device_idle()
regards
Suman
From: Suman Anna <hidden> Date: 2016-02-11 22:04:59
On 02/11/2016 01:27 PM, Paul Walmsley wrote:
Hi Kishon, Suman,
On Wed, 10 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
quoted
On 02/09/2016 01:36 PM, Paul Walmsley wrote:
quoted
On Tue, 9 Feb 2016, Suman Anna wrote:
quoted
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
quoted
On Mon, 8 Feb 2016, Suman Anna wrote:
quoted
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
quoted
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs?
Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.
I'm curious as to whether HWMOD_CUSTOM_HARDRESET is needed for the MMUs.
We've stated that the main point of the custom hardreset code is to handle
processors that need to be placed into WFI/HLT, but it doesn't seem like
there would be an equivalent for MMUs. Thoughts?
The current OMAP IOMMU code already leverages the pdata ops for
performing the resets, so not adding the flags would also require
additional changes in the driver.
Also, the reset lines controlling the MMUs actually also manage the
reset for all the other sub-modules other than the processor cores
within the sub-systems. We have currently different issues (see [1] for
eg. around the IPU sub-system entering RET in between), so from a PM
point of view, we do prefer to place the MMUs also in reset when we are
runtime suspended.
Should we reassert hardreset in _idle() for IP blocks that don't have
HWMOD_CUSTOM_HARDRESET set on them? Would that allow us to use this
mechanism for the uncore hardreset lines, or are there other quirks?
Also - would that address the potential issue that you mentioned with the
PCIe block, or is that a different issue?
Yeah, I think that would address the PCIe block issue in terms of reset
state balancing between pm_runtime_get_sync() and pm_runtime_put()
calls. Right now, they are unbalanced. The PCIe block is using these
only in probe and remove, so it should work for that IP.
As I mentioned before this would result in undesired behavior during
suspend/resume cycle in PCIe. (This should be okay for the current mainline
code but would break once we add suspend/resume support for PCIe).
I'd like to understand where we're currently at here. It looks like we're
waiting for testing from Suman, and we're waiting for Kishon to try using
the bind/unbind driver model hook to see if that wedges PCIe? Does this
match your collective understanding of the status here?
Matches mine :)
For MMUs and (out of tree) OMAP remoteprocs, my current sequence is
omap_device_deassert_hardreset() followed by pm_runtime_get_sync() or
omap_device_enable() during booting, and pm_runtime_put_sync() or
omap_device_idle() followed by omap_device_assert_hardreset(). Atleast
they are bunched together.
So, the current code does _deassert_hardreset twice when invoking the
pm_runtime_get_sync() in my driver since the check for
_are_all_hardreset_lines_asserted(oh) would fail.
Thinking about the question of what to do about hardreset assertion in
idle, if we need it, we could add a hwmod flag to control that mode. I
would consider it a temporary workaround until we have the hwmod code
moved into a bus driver and the bus driver/hwmod code can hook into the
LDM .remove operation (and connect it to .shutdown, etc.) Suman/Kishon:
is it your understanding that we could remove the existing hardreset
control in the IOMMU drivers and the PCIe driver if we had these options
in the hwmod code?
For MMUs/processors, the position where we deassert the reset becomes
important. It has to be after the clocks are enabled (which is why half
of the _deassert_hardreset code looks like the code sequence in _enable()).
regards
Suman
From: Kishon Vijay Abraham I <hidden> Date: 2016-02-12 06:50:42
Hi,
On Friday 12 February 2016 12:57 AM, Paul Walmsley wrote:
Hi Kishon, Suman,
On Wed, 10 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
quoted
On 02/09/2016 01:36 PM, Paul Walmsley wrote:
quoted
On Tue, 9 Feb 2016, Suman Anna wrote:
quoted
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
quoted
On Mon, 8 Feb 2016, Suman Anna wrote:
quoted
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
quoted
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs?
Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.
I'm curious as to whether HWMOD_CUSTOM_HARDRESET is needed for the MMUs.
We've stated that the main point of the custom hardreset code is to handle
processors that need to be placed into WFI/HLT, but it doesn't seem like
there would be an equivalent for MMUs. Thoughts?
The current OMAP IOMMU code already leverages the pdata ops for
performing the resets, so not adding the flags would also require
additional changes in the driver.
Also, the reset lines controlling the MMUs actually also manage the
reset for all the other sub-modules other than the processor cores
within the sub-systems. We have currently different issues (see [1] for
eg. around the IPU sub-system entering RET in between), so from a PM
point of view, we do prefer to place the MMUs also in reset when we are
runtime suspended.
Should we reassert hardreset in _idle() for IP blocks that don't have
HWMOD_CUSTOM_HARDRESET set on them? Would that allow us to use this
mechanism for the uncore hardreset lines, or are there other quirks?
Also - would that address the potential issue that you mentioned with the
PCIe block, or is that a different issue?
Yeah, I think that would address the PCIe block issue in terms of reset
state balancing between pm_runtime_get_sync() and pm_runtime_put()
calls. Right now, they are unbalanced. The PCIe block is using these
only in probe and remove, so it should work for that IP.
As I mentioned before this would result in undesired behavior during
suspend/resume cycle in PCIe. (This should be okay for the current mainline
code but would break once we add suspend/resume support for PCIe).
I'd like to understand where we're currently at here. It looks like we're
waiting for testing from Suman, and we're waiting for Kishon to try using
the bind/unbind driver model hook to see if that wedges PCIe? Does this
match your collective understanding of the status here?
I got to try this and looks like even without this series there are other PM
issues possible introduced by Commit 5de85b9d57ab ("PM / runtime: Re-init
runtime PM states at probe error and driver unbind").
Now I get this error if I tried to modprobe after rmmod pci-dra7xx.
[ 54.352860] dra7-pcie 51000000.pcie: omap_device: omap_device_enable()
called from invalid state 1
[ 54.362318] dra7-pcie 51000000.pcie: pm_runtime_get_sync failed
[ 54.368624] dra7-pcie: probe of 51000000.pcie failed with error -22
From the thread that fixes this issue [1], looks like drivers that use
*_autosuspend() get this issue. However I don't use *_autosuspend() in
pci-dra7xx. Maybe pci core has this? This has to be debugged further. But I
feel this is not related to the problem that we are trying to solve right now
(dra7 hangs if PCI driver is enabled) and given the fact that pci-dra7xx driver
is now modeled as built-in driver, this can be deferred.
[1] -> http://www.spinics.net/lists/arm-kernel/msg481845.html
Thinking about the question of what to do about hardreset assertion in
idle, if we need it, we could add a hwmod flag to control that mode. I
would consider it a temporary workaround until we have the hwmod code
moved into a bus driver and the bus driver/hwmod code can hook into the
LDM .remove operation (and connect it to .shutdown, etc.) Suman/Kishon:
is it your understanding that we could remove the existing hardreset
control in the IOMMU drivers and the PCIe driver if we had these options
in the hwmod code?
Yeah, that's my understanding. And since this series solves the PCIe problem,
it's proven that hardreset control can be moved to hwmod code.
For PCIe, it's even okay to do deassert in _reset, but I'm not sure if it'll
have side effects with other modules.
From: Kishon Vijay Abraham I <hidden> Date: 2016-02-12 06:56:30
Hi Suman,
On Friday 12 February 2016 02:13 AM, Suman Anna wrote:
On 02/09/2016 11:38 PM, Kishon Vijay Abraham I wrote:
quoted
Hi,
On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
quoted
Hi Paul,
On 02/09/2016 01:36 PM, Paul Walmsley wrote:
quoted
Hi Suman
On Tue, 9 Feb 2016, Suman Anna wrote:
quoted
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
quoted
On Mon, 8 Feb 2016, Suman Anna wrote:
quoted
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
quoted
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs?
Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.
I'm curious as to whether HWMOD_CUSTOM_HARDRESET is needed for the MMUs.
We've stated that the main point of the custom hardreset code is to handle
processors that need to be placed into WFI/HLT, but it doesn't seem like
there would be an equivalent for MMUs. Thoughts?
The current OMAP IOMMU code already leverages the pdata ops for
performing the resets, so not adding the flags would also require
additional changes in the driver.
Also, the reset lines controlling the MMUs actually also manage the
reset for all the other sub-modules other than the processor cores
within the sub-systems. We have currently different issues (see [1] for
eg. around the IPU sub-system entering RET in between), so from a PM
point of view, we do prefer to place the MMUs also in reset when we are
runtime suspended.
Should we reassert hardreset in _idle() for IP blocks that don't have
HWMOD_CUSTOM_HARDRESET set on them? Would that allow us to use this
mechanism for the uncore hardreset lines, or are there other quirks?
Also - would that address the potential issue that you mentioned with the
PCIe block, or is that a different issue?
Yeah, I think that would address the PCIe block issue in terms of reset
state balancing between pm_runtime_get_sync() and pm_runtime_put()
calls. Right now, they are unbalanced. The PCIe block is using these
only in probe and remove, so it should work for that IP.
As I mentioned before this would result in undesired behavior during
suspend/resume cycle in PCIe. (This should be okay for the current mainline
code but would break once we add suspend/resume support for PCIe).
Yeah, I was wondering if some peripheral would want only the clock to be
controlled during _idle() and not reset. Even then for the PCIe case
that you are talking about, going through a pm_runtime_get_sync(),
pm_runtime_put_sync()/pm_runtime_put() deasserts the resets everytime
right. But it'll deassert a line which is already deasserted. So it actually
doesn't do a reset again.
_enable() is called. Right now, the code block has ignored the return
value from the _hardreset_deassert(), but if you check it and bail out,
then your get_sync() would start failing from the second invocation.
hmm.. yeah.
Can you elaborate more on what kind of issues you will see on
suspend/resume cycle with PCIe? Do note that _idle() gets called through
At this point there are other issues w.r.t suspend/resume in PCI-dra7xx but as
such reset of the controller is not desired during suspend/resume cycle and
it'll result in the register contents being reset (haven't tested it though).
Thanks
Kishon
From: Suman Anna <hidden> Date: 2016-02-12 17:22:33
Kishon,
On 02/12/2016 12:49 AM, Kishon Vijay Abraham I wrote:
quoted hunk
Hi,
On Friday 12 February 2016 12:57 AM, Paul Walmsley wrote:
quoted
Hi Kishon, Suman,
On Wed, 10 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
quoted
On 02/09/2016 01:36 PM, Paul Walmsley wrote:
quoted
On Tue, 9 Feb 2016, Suman Anna wrote:
quoted
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
quoted
On Mon, 8 Feb 2016, Suman Anna wrote:
quoted
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
quoted
On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Paul, what do you think is the best way forward to perform reset?
Many of the IP blocks with PRM hardreset lines are processor IP blocks.
Those often need special reset handling to ensure that WFI/HLT-like
instructions are executed after reset. This special handling ensures that
the IP blocks' bus initiator interfaces indicate that they are in standby
to the PRCM - thus allowing power management for the rest of the chip to
work correctly.
But that doesn't seem to be the case with PCIe - and maybe others -
possibly some of the MMUs?
Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.
I'm curious as to whether HWMOD_CUSTOM_HARDRESET is needed for the MMUs.
We've stated that the main point of the custom hardreset code is to handle
processors that need to be placed into WFI/HLT, but it doesn't seem like
there would be an equivalent for MMUs. Thoughts?
The current OMAP IOMMU code already leverages the pdata ops for
performing the resets, so not adding the flags would also require
additional changes in the driver.
Also, the reset lines controlling the MMUs actually also manage the
reset for all the other sub-modules other than the processor cores
within the sub-systems. We have currently different issues (see [1] for
eg. around the IPU sub-system entering RET in between), so from a PM
point of view, we do prefer to place the MMUs also in reset when we are
runtime suspended.
Should we reassert hardreset in _idle() for IP blocks that don't have
HWMOD_CUSTOM_HARDRESET set on them? Would that allow us to use this
mechanism for the uncore hardreset lines, or are there other quirks?
Also - would that address the potential issue that you mentioned with the
PCIe block, or is that a different issue?
Yeah, I think that would address the PCIe block issue in terms of reset
state balancing between pm_runtime_get_sync() and pm_runtime_put()
calls. Right now, they are unbalanced. The PCIe block is using these
only in probe and remove, so it should work for that IP.
As I mentioned before this would result in undesired behavior during
suspend/resume cycle in PCIe. (This should be okay for the current mainline
code but would break once we add suspend/resume support for PCIe).
I'd like to understand where we're currently at here. It looks like we're
waiting for testing from Suman, and we're waiting for Kishon to try using
the bind/unbind driver model hook to see if that wedges PCIe? Does this
match your collective understanding of the status here?
I got to try this and looks like even without this series there are other PM
issues possible introduced by Commit 5de85b9d57ab ("PM / runtime: Re-init
runtime PM states at probe error and driver unbind").
Now I get this error if I tried to modprobe after rmmod pci-dra7xx.
[ 54.352860] dra7-pcie 51000000.pcie: omap_device: omap_device_enable()
called from invalid state 1
[ 54.362318] dra7-pcie 51000000.pcie: pm_runtime_get_sync failed
[ 54.368624] dra7-pcie: probe of 51000000.pcie failed with error -22
From the thread that fixes this issue [1], looks like drivers that use
*_autosuspend() get this issue. However I don't use *_autosuspend() in
pci-dra7xx. Maybe pci core has this? This has to be debugged further. But I
feel this is not related to the problem that we are trying to solve right now
(dra7 hangs if PCI driver is enabled) and given the fact that pci-dra7xx driver
is now modeled as built-in driver, this can be deferred.
[1] -> http://www.spinics.net/lists/arm-kernel/msg481845.html
quoted
Thinking about the question of what to do about hardreset assertion in
idle, if we need it, we could add a hwmod flag to control that mode. I
would consider it a temporary workaround until we have the hwmod code
moved into a bus driver and the bus driver/hwmod code can hook into the
LDM .remove operation (and connect it to .shutdown, etc.) Suman/Kishon:
is it your understanding that we could remove the existing hardreset
control in the IOMMU drivers and the PCIe driver if we had these options
in the hwmod code?
Yeah, that's my understanding. And since this series solves the PCIe problem,
it's proven that hardreset control can be moved to hwmod code.
For PCIe, it's even okay to do deassert in _reset, but I'm not sure if it'll
have side effects with other modules.
@@ -1966,8 +1966,11 @@ static int _reset(struct omap_hwmod *oh)r=oh->class->reset(oh);}else{if(oh->rst_lines_cnt>0){-for(i=0;i<oh->rst_lines_cnt;i++)+for(i=0;i<oh->rst_lines_cnt;i++){_assert_hardreset(oh,oh->rst_lines[i].name);+if(!(oh->flags&HWMOD_CUSTOM_HARDRESET))+_deassert_hardreset(oh,oh->rst_lines[i].name);+}
Better yet, just add this specific _deassert_hardreset logic to a DRA7
PCIe-specific class->reset function. You won't need adding the
HWMOD_CUSTOM_HARDRESET flags either and will satisfy your suspend/resume
dilemma, and it won't affect other paths. If that can work for you, that
would be simplest patch for this -rc cycle.
return 0;
} else {
r = _ocp_softreset(oh);
Thanks
Kishon
P.S. I'll be on vacation till end of next week with no email access till then.
So email response will be delayed. Sorry about that.
Sekhar,
Will you be following up with above suggestion since Kishon is gonna be out?
regards
Suman
From: Sekhar Nori <hidden> Date: 2016-02-18 14:22:29
On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
Sekhar,
Will you be following up with above suggestion since Kishon is gonna be out?
Alright, noticed this action for me :) Went through the thread, and
looks like this is what we want to see?
Thanks,
Sekhar
---8<---
From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
Message-Id: [off-list ref]
From: Sekhar Nori <redacted>
Date: Thu, 18 Feb 2016 16:49:56 +0530
Subject: [PATCH 1/1] ARM: DRA7: hwmod: Add custom reset handler for PCIeSS
Add a custom reset handler for DRA7x PCIeSS. This
handler is required to deassert PCIe hardreset lines
after they have been asserted.
This enables the PCIe driver to access registers after
PCIeSS has been runtime enabled without having to
deassert hardreset lines itself.
With this patch applied, used lspci to make sure
connected PCIe device enumerates on DRA74x and DRA72x
EVMs.
Signed-off-by: Sekhar Nori <redacted>
---
Applies to tag for-v4.6/omap-hwmod-a of Paul W's tree.
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
From: Paul Walmsley <paul@pwsan.com> Date: 2016-02-18 17:23:56
On Thu, 18 Feb 2016, Sekhar Nori wrote:
On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
quoted
Sekhar,
Will you be following up with above suggestion since Kishon is gonna be out?
Alright, noticed this action for me :) Went through the thread, and
looks like this is what we want to see?
Thanks Sekhar. Did you try the driver unbind/bind sequence a few times to
ensure that works, per Suman's earlier E-mail?
Suman, is there any further testing that you are planning to do on this
patch?
- Paul
quoted hunk
Thanks,
Sekhar
---8<---
quoted
From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
Message-Id: [off-list ref]
From: Sekhar Nori <redacted>
Date: Thu, 18 Feb 2016 16:49:56 +0530
Subject: [PATCH 1/1] ARM: DRA7: hwmod: Add custom reset handler for PCIeSS
Add a custom reset handler for DRA7x PCIeSS. This
handler is required to deassert PCIe hardreset lines
after they have been asserted.
This enables the PCIe driver to access registers after
PCIeSS has been runtime enabled without having to
deassert hardreset lines itself.
With this patch applied, used lspci to make sure
connected PCIe device enumerates on DRA74x and DRA72x
EVMs.
Signed-off-by: Sekhar Nori <redacted>
---
Applies to tag for-v4.6/omap-hwmod-a of Paul W's tree.
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
From: Suman Anna <hidden> Date: 2016-02-18 18:28:40
On 02/18/2016 11:23 AM, Paul Walmsley wrote:
On Thu, 18 Feb 2016, Sekhar Nori wrote:
quoted
On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
quoted
Sekhar,
Will you be following up with above suggestion since Kishon is gonna be out?
Alright, noticed this action for me :) Went through the thread, and
looks like this is what we want to see?
Thanks Sekhar. Did you try the driver unbind/bind sequence a few times to
ensure that works, per Suman's earlier E-mail?
Should work since the assert/deassert is now out of the driver
probe/remove path and is done only at init time, but will let Sekhar
confirm this.
Suman, is there any further testing that you are planning to do on this
patch?
No, nothing on my side, since this is now localized to PCIe and only on
DRA7xx. I will relook at your custom flags solution when I consolidate
the reset for OMAP IOMMUs and remoteprocs, that looks promising to
remove the pdata quirks for resets or dependencies in drivers against a
reset API.
- Paul
quoted
Thanks,
Sekhar
---8<---
quoted
From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
Message-Id: [off-list ref]
From: Sekhar Nori <redacted>
Date: Thu, 18 Feb 2016 16:49:56 +0530
Subject: [PATCH 1/1] ARM: DRA7: hwmod: Add custom reset handler for PCIeSS
Add a custom reset handler for DRA7x PCIeSS. This
handler is required to deassert PCIe hardreset lines
after they have been asserted.
This enables the PCIe driver to access registers after
PCIeSS has been runtime enabled without having to
deassert hardreset lines itself.
With this patch applied, used lspci to make sure
connected PCIe device enumerates on DRA74x and DRA72x
EVMs.
Yep, this is what I had in mind. Glad that it resolves the issue.
quoted
Signed-off-by: Sekhar Nori <redacted>
---
Applies to tag for-v4.6/omap-hwmod-a of Paul W's tree.
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
From: Kishon Vijay Abraham I <hidden> Date: 2016-02-22 06:19:08
Sekhar,
On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
quoted hunk
On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
quoted
Sekhar,
Will you be following up with above suggestion since Kishon is gonna be out?
Alright, noticed this action for me :) Went through the thread, and
looks like this is what we want to see?
Thanks,
Sekhar
---8<---
From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
Message-Id: [off-list ref]
From: Sekhar Nori <redacted>
Date: Thu, 18 Feb 2016 16:49:56 +0530
Subject: [PATCH 1/1] ARM: DRA7: hwmod: Add custom reset handler for PCIeSS
Add a custom reset handler for DRA7x PCIeSS. This
handler is required to deassert PCIe hardreset lines
after they have been asserted.
This enables the PCIe driver to access registers after
PCIeSS has been runtime enabled without having to
deassert hardreset lines itself.
With this patch applied, used lspci to make sure
connected PCIe device enumerates on DRA74x and DRA72x
EVMs.
Signed-off-by: Sekhar Nori <redacted>
---
Applies to tag for-v4.6/omap-hwmod-a of Paul W's tree.
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
From: Paul Walmsley <paul@pwsan.com> Date: 2016-02-22 06:31:59
Kishon,
On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
Sekhar,
On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
quoted
On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
quoted
Sekhar,
Will you be following up with above suggestion since Kishon is gonna be out?
Alright, noticed this action for me :) Went through the thread, and
looks like this is what we want to see?
Thanks,
Sekhar
---8<---
From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
Message-Id: [off-list ref]
From: Sekhar Nori <redacted>
Date: Thu, 18 Feb 2016 16:49:56 +0530
Subject: [PATCH 1/1] ARM: DRA7: hwmod: Add custom reset handler for PCIeSS
Add a custom reset handler for DRA7x PCIeSS. This
handler is required to deassert PCIe hardreset lines
after they have been asserted.
This enables the PCIe driver to access registers after
PCIeSS has been runtime enabled without having to
deassert hardreset lines itself.
With this patch applied, used lspci to make sure
connected PCIe device enumerates on DRA74x and DRA72x
EVMs.
Signed-off-by: Sekhar Nori <redacted>
---
Applies to tag for-v4.6/omap-hwmod-a of Paul W's tree.
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
From: Kishon Vijay Abraham I <hidden> Date: 2016-02-22 09:56:40
Hi Paul,
On Monday 22 February 2016 12:01 PM, Paul Walmsley wrote:
Kishon,
On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Sekhar,
On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
quoted
On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
quoted
Sekhar,
Will you be following up with above suggestion since Kishon is gonna be out?
Alright, noticed this action for me :) Went through the thread, and
looks like this is what we want to see?
Thanks,
Sekhar
---8<---
From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
Message-Id: [off-list ref]
From: Sekhar Nori <redacted>
Date: Thu, 18 Feb 2016 16:49:56 +0530
Subject: [PATCH 1/1] ARM: DRA7: hwmod: Add custom reset handler for PCIeSS
Add a custom reset handler for DRA7x PCIeSS. This
handler is required to deassert PCIe hardreset lines
after they have been asserted.
This enables the PCIe driver to access registers after
PCIeSS has been runtime enabled without having to
deassert hardreset lines itself.
With this patch applied, used lspci to make sure
connected PCIe device enumerates on DRA74x and DRA72x
EVMs.
Signed-off-by: Sekhar Nori <redacted>
---
Applies to tag for-v4.6/omap-hwmod-a of Paul W's tree.
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
Could you please test the bind/unbind functionality just to make sure it
works?
The pci-dra7xx driver neither expose the bind/unbind sysfs entry nor can be
built as module.
However I hacked the pci-dra7xx driver to be built as module [and reverted
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe error
and driver unbind")] and I don't see an abort when the PCI registers are
accessed after rmmod/modprobe cycle (that was the issue that Sekhar's patch
tried to solve). However PCI as such doesn't work after rmmod/modprobe cycle
[1], but this is a different issue most likely in PCIe core and has to debugged.
In summary, there are other issues in PCI across rmmod/modprobe cycle but the
reset of pci-dra7xx happens fine with this patch.
Thanks
Kishon
[1] -> http://pastebin.ubuntu.com/15169387/
From: Kishon Vijay Abraham I <hidden> Date: 2016-02-23 11:57:52
Hi Paul,
On Monday 22 February 2016 03:25 PM, Kishon Vijay Abraham I wrote:
Hi Paul,
On Monday 22 February 2016 12:01 PM, Paul Walmsley wrote:
quoted
Kishon,
On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
Sekhar,
On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
quoted
On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
quoted
Sekhar,
Will you be following up with above suggestion since Kishon is gonna be out?
Alright, noticed this action for me :) Went through the thread, and
looks like this is what we want to see?
Thanks,
Sekhar
---8<---
From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
Message-Id: [off-list ref]
From: Sekhar Nori <redacted>
Date: Thu, 18 Feb 2016 16:49:56 +0530
Subject: [PATCH 1/1] ARM: DRA7: hwmod: Add custom reset handler for PCIeSS
Add a custom reset handler for DRA7x PCIeSS. This
handler is required to deassert PCIe hardreset lines
after they have been asserted.
This enables the PCIe driver to access registers after
PCIeSS has been runtime enabled without having to
deassert hardreset lines itself.
With this patch applied, used lspci to make sure
connected PCIe device enumerates on DRA74x and DRA72x
EVMs.
Signed-off-by: Sekhar Nori <redacted>
---
Applies to tag for-v4.6/omap-hwmod-a of Paul W's tree.
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
Could you please test the bind/unbind functionality just to make sure it
works?
The pci-dra7xx driver neither expose the bind/unbind sysfs entry nor can be
built as module.
However I hacked the pci-dra7xx driver to be built as module [and reverted
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe error
and driver unbind")] and I don't see an abort when the PCI registers are
accessed after rmmod/modprobe cycle (that was the issue that Sekhar's patch
tried to solve). However PCI as such doesn't work after rmmod/modprobe cycle
[1], but this is a different issue most likely in PCIe core and has to debugged.
In summary, there are other issues in PCI across rmmod/modprobe cycle but the
reset of pci-dra7xx happens fine with this patch.
From: Paul Walmsley <paul@pwsan.com> Date: 2016-02-23 18:28:38
Kishon
On Tue, 23 Feb 2016, Kishon Vijay Abraham I wrote:
On Monday 22 February 2016 03:25 PM, Kishon Vijay Abraham I wrote:
quoted
On Monday 22 February 2016 12:01 PM, Paul Walmsley wrote:
quoted
On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
quoted
On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
quoted
Will you be following up with above suggestion since Kishon is gonna be out?
Alright, noticed this action for me :) Went through the thread, and
looks like this is what we want to see?
Thanks,
Sekhar
---8<---
From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
Message-Id: [off-list ref]
From: Sekhar Nori <redacted>
Date: Thu, 18 Feb 2016 16:49:56 +0530
Subject: [PATCH 1/1] ARM: DRA7: hwmod: Add custom reset handler for PCIeSS
Add a custom reset handler for DRA7x PCIeSS. This
handler is required to deassert PCIe hardreset lines
after they have been asserted.
This enables the PCIe driver to access registers after
PCIeSS has been runtime enabled without having to
deassert hardreset lines itself.
With this patch applied, used lspci to make sure
connected PCIe device enumerates on DRA74x and DRA72x
EVMs.
Signed-off-by: Sekhar Nori <redacted>
---
Applies to tag for-v4.6/omap-hwmod-a of Paul W's tree.
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
Could you please test the bind/unbind functionality just to make sure it
works?
The pci-dra7xx driver neither expose the bind/unbind sysfs entry nor can be
built as module.
However I hacked the pci-dra7xx driver to be built as module [and reverted
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe error
and driver unbind")] and I don't see an abort when the PCI registers are
accessed after rmmod/modprobe cycle (that was the issue that Sekhar's patch
tried to solve). However PCI as such doesn't work after rmmod/modprobe cycle
[1], but this is a different issue most likely in PCIe core and has to debugged.
In summary, there are other issues in PCI across rmmod/modprobe cycle but the
reset of pci-dra7xx happens fine with this patch.
Do you expect any other testing from me?
No I think that's sufficient for the time being, thanks - but, two
questions:
1. Can I add your Tested-by: ?
2. Are you going to follow up with these PCIe core issues with the PCIe
core developers?
- Paul
From: Kishon Vijay Abraham I <hidden> Date: 2016-02-24 06:22:00
Hi Paul,
On Tuesday 23 February 2016 11:58 PM, Paul Walmsley wrote:
Kishon
On Tue, 23 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
On Monday 22 February 2016 03:25 PM, Kishon Vijay Abraham I wrote:
quoted
On Monday 22 February 2016 12:01 PM, Paul Walmsley wrote:
quoted
On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
quoted
On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
quoted
On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
quoted
Will you be following up with above suggestion since Kishon is gonna be out?
Alright, noticed this action for me :) Went through the thread, and
looks like this is what we want to see?
Thanks,
Sekhar
---8<---
From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
Message-Id: [off-list ref]
From: Sekhar Nori <redacted>
Date: Thu, 18 Feb 2016 16:49:56 +0530
Subject: [PATCH 1/1] ARM: DRA7: hwmod: Add custom reset handler for PCIeSS
Add a custom reset handler for DRA7x PCIeSS. This
handler is required to deassert PCIe hardreset lines
after they have been asserted.
This enables the PCIe driver to access registers after
PCIeSS has been runtime enabled without having to
deassert hardreset lines itself.
With this patch applied, used lspci to make sure
connected PCIe device enumerates on DRA74x and DRA72x
EVMs.
Signed-off-by: Sekhar Nori <redacted>
---
Applies to tag for-v4.6/omap-hwmod-a of Paul W's tree.
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
Could you please test the bind/unbind functionality just to make sure it
works?
The pci-dra7xx driver neither expose the bind/unbind sysfs entry nor can be
built as module.
However I hacked the pci-dra7xx driver to be built as module [and reverted
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe error
and driver unbind")] and I don't see an abort when the PCI registers are
accessed after rmmod/modprobe cycle (that was the issue that Sekhar's patch
tried to solve). However PCI as such doesn't work after rmmod/modprobe cycle
[1], but this is a different issue most likely in PCIe core and has to debugged.
In summary, there are other issues in PCI across rmmod/modprobe cycle but the
reset of pci-dra7xx happens fine with this patch.
Do you expect any other testing from me?
No I think that's sufficient for the time being, thanks - but, two
questions:
1. Can I add your Tested-by: ?
sure..
Tested-by: Kishon Vijay Abraham I <redacted>
2. Are you going to follow up with these PCIe core issues with the PCIe
core developers?
From: Paul Walmsley <paul@pwsan.com> Date: 2016-03-01 08:26:00
Folks, the following is what I've queued for this.
- Paul
From: Sekhar Nori <redacted>
Date: Thu, 18 Feb 2016 16:49:56 +0530
Subject: [PATCH] ARM: DRA7: hwmod: Add custom reset handler for PCIeSS
Add a custom reset handler for DRA7x PCIeSS. This
handler is required to deassert PCIe hardreset lines
after they have been asserted.
This enables the PCIe driver to access registers after
PCIeSS has been runtime enabled without having to
deassert hardreset lines itself.
With this patch applied, used lspci to make sure
connected PCIe device enumerates on DRA74x and DRA72x
EVMs.
Signed-off-by: Sekhar Nori <redacted>
Reported-by: Richard Cochran <richardcochran@gmail.com>
Tested-by: Kishon Vijay Abraham I <redacted>
Cc: Suman Anna <redacted>
Cc: Dave Gerlach <redacted>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <redacted>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
From: Kishon Vijay Abraham I <hidden> Date: 2016-03-01 11:56:45
Hi,
On Tuesday 01 March 2016 01:55 PM, Paul Walmsley wrote:
Folks, the following is what I've queued for this.
Thanks Paul.
Bjorn,
With this patch merged, enabling pci-dra7xx won't result in system freeze
anymore. I can send a patch to revert depends on BROKEN.
Thanks
Kishon
quoted hunk
- Paul
From: Sekhar Nori <redacted>
Date: Thu, 18 Feb 2016 16:49:56 +0530
Subject: [PATCH] ARM: DRA7: hwmod: Add custom reset handler for PCIeSS
Add a custom reset handler for DRA7x PCIeSS. This
handler is required to deassert PCIe hardreset lines
after they have been asserted.
This enables the PCIe driver to access registers after
PCIeSS has been runtime enabled without having to
deassert hardreset lines itself.
With this patch applied, used lspci to make sure
connected PCIe device enumerates on DRA74x and DRA72x
EVMs.
Signed-off-by: Sekhar Nori <redacted>
Reported-by: Richard Cochran <richardcochran@gmail.com>
Tested-by: Kishon Vijay Abraham I <redacted>
Cc: Suman Anna <redacted>
Cc: Dave Gerlach <redacted>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <redacted>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
On Tue, Mar 01, 2016 at 05:25:56PM +0530, Kishon Vijay Abraham I wrote:
Hi,
On Tuesday 01 March 2016 01:55 PM, Paul Walmsley wrote:
quoted
Folks, the following is what I've queued for this.
Thanks Paul.
Bjorn,
With this patch merged, enabling pci-dra7xx won't result in system freeze
anymore. I can send a patch to revert depends on BROKEN.
Great! Please send me that patch, and I'll merge it for v4.6.
quoted
From: Sekhar Nori <redacted>
Date: Thu, 18 Feb 2016 16:49:56 +0530
Subject: [PATCH] ARM: DRA7: hwmod: Add custom reset handler for PCIeSS
Add a custom reset handler for DRA7x PCIeSS. This
handler is required to deassert PCIe hardreset lines
after they have been asserted.
This enables the PCIe driver to access registers after
PCIeSS has been runtime enabled without having to
deassert hardreset lines itself.
With this patch applied, used lspci to make sure
connected PCIe device enumerates on DRA74x and DRA72x
EVMs.
Signed-off-by: Sekhar Nori <redacted>
Reported-by: Richard Cochran <richardcochran@gmail.com>
Tested-by: Kishon Vijay Abraham I <redacted>
Cc: Suman Anna <redacted>
Cc: Dave Gerlach <redacted>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <redacted>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
From: Suman Anna <hidden> Date: 2016-03-01 16:56:37
On 03/01/2016 05:55 AM, Kishon Vijay Abraham I wrote:
Hi,
On Tuesday 01 March 2016 01:55 PM, Paul Walmsley wrote:
quoted
Folks, the following is what I've queued for this.
Thanks Paul.
Bjorn,
With this patch merged, enabling pci-dra7xx won't result in system freeze
anymore. I can send a patch to revert depends on BROKEN.
Kishon,
Make sure you send only that after both this patch and the pcie reset
data are in. I see the pcie reset data in Paul's for-4.6 branch.
regards
Suman
[snip]