* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-08 20:56 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-08 20:56 UTC (permalink / raw)
To: linux-arm-kernel
Hi Paul,
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
> 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?
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.
>
>
> - 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(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index e9f65fec55c0..ab66dd988709 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -2077,7 +2077,7 @@ static int _enable_preprogram(struct omap_hwmod *oh)
> */
> static int _enable(struct omap_hwmod *oh)
> {
> - int r;
> + int r, i;
> int hwsup = 0;
>
> pr_debug("omap_hwmod: %s: enabling\n", oh->name);
> @@ -2109,17 +2109,23 @@ static int _enable(struct omap_hwmod *oh)
> }
>
> /*
> - * If an IP block contains HW reset lines and all of them are
> - * asserted, we let integration code associated with that
> - * block handle the enable. We've received very little
> + * If an IP block contains HW reset lines, all of them are
> + * asserted, and the IP block is marked as requiring a custom
> + * hardreset handler, we let integration code associated with
> + * that block handle the enable. We've received very little
> * information on what those driver authors need, and until
> * detailed information is provided and the driver code is
> * posted to the public lists, this is probably the best we
> * can do.
> */
> - if (_are_all_hardreset_lines_asserted(oh))
> + if ((oh->flags & HWMOD_CUSTOM_HARDRESET) &&
> + _are_all_hardreset_lines_asserted(oh))
> return 0;
>
> + /* 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
> +
> /* Mux pins for device runtime if populated */
> if (oh->mux && (!oh->mux->enabled ||
> ((oh->_state == _HWMOD_STATE_IDLE) &&
> diff --git a/arch/arm/mach-omap2/omap_hwmod.h b/arch/arm/mach-omap2/omap_hwmod.h
> index 76bce11c85a4..4198829551a4 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.h
> +++ b/arch/arm/mach-omap2/omap_hwmod.h
> @@ -525,6 +525,17 @@ struct omap_hwmod_omap4_prcm {
> * or idled.
> * HWMOD_OPT_CLKS_NEEDED: The optional clocks are needed for the module to
> * operate and they need to be handled at the same time as the main_clk.
> + * HWMOD_CUSTOM_HARDRESET: By default, if a hwmod has PRCM hardreset
> + * lines associated with it (i.e., a populated .rst_lines field in
> + * the hwmod), the hwmod code will assert the hardreset lines when
> + * the IP block is initially reset, deassert the hardreset lines
> + * in _enable(), and reassert them in _shutdown(). If this flag
> + * is set, the hwmod code will not deassert the hardreset lines in
> + * _enable(), leaving this responsibility to the driver code. This flag may
> + * be needed for processor IP blocks that must be put into a WFI/HLT
> + * state after reset is deasserted, lest the processor leave its MSTANDBY
> + * signal deasserted, thus blocking the chip from entering a system-wide
> + * low power state.
> */
> #define HWMOD_SWSUP_SIDLE (1 << 0)
> #define HWMOD_SWSUP_MSTANDBY (1 << 1)
> @@ -541,6 +552,7 @@ struct omap_hwmod_omap4_prcm {
> #define HWMOD_SWSUP_SIDLE_ACT (1 << 12)
> #define HWMOD_RECONFIG_IO_CHAIN (1 << 13)
> #define HWMOD_OPT_CLKS_NEEDED (1 << 14)
> +#define HWMOD_CUSTOM_HARDRESET (1 << 15)
>
> /*
> * omap_hwmod._int_flags definitions
> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> index dad871a4cd96..20501f0e3c6c 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> @@ -553,6 +553,7 @@ static struct omap_hwmod omap44xx_dsp_hwmod = {
> .modulemode = MODULEMODE_HWCTRL,
> },
> },
> + .flags = HWMOD_CUSTOM_HARDRESET,
> };
>
> /*
> @@ -1433,6 +1434,7 @@ static struct omap_hwmod omap44xx_ipu_hwmod = {
> .modulemode = MODULEMODE_HWCTRL,
> },
> },
> + .flags = HWMOD_CUSTOM_HARDRESET,
> };
>
> /*
> @@ -1517,6 +1519,7 @@ static struct omap_hwmod omap44xx_iva_hwmod = {
> .modulemode = MODULEMODE_HWCTRL,
> },
> },
> + .flags = HWMOD_CUSTOM_HARDRESET,
> };
>
> /*
> @@ -2115,6 +2118,7 @@ static struct omap_hwmod omap44xx_mmu_ipu_hwmod = {
> .modulemode = MODULEMODE_HWCTRL,
> },
> },
> + .flags = HWMOD_CUSTOM_HARDRESET, /* XXX doublecheck */
> };
>
> /* mmu dsp */
> @@ -2147,6 +2151,7 @@ static struct omap_hwmod omap44xx_mmu_dsp_hwmod = {
> .modulemode = MODULEMODE_HWCTRL,
> },
> },
> + .flags = HWMOD_CUSTOM_HARDRESET, /* XXX doublecheck */
> };
>
> /*
> @@ -2299,6 +2304,7 @@ static struct omap_hwmod omap44xx_prm_hwmod = {
> .class = &omap44xx_prcm_hwmod_class,
> .rst_lines = omap44xx_prm_resets,
> .rst_lines_cnt = ARRAY_SIZE(omap44xx_prm_resets),
> + .flags = HWMOD_CUSTOM_HARDRESET,
> };
>
> /*
>
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-08 20:56 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-08 20:56 UTC (permalink / raw)
To: Paul Walmsley, Kishon Vijay Abraham I
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
Hi Paul,
On 02/07/2016 08:48 PM, Paul Walmsley wrote:
> 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?
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.
>
>
> - 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(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index e9f65fec55c0..ab66dd988709 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -2077,7 +2077,7 @@ static int _enable_preprogram(struct omap_hwmod *oh)
> */
> static int _enable(struct omap_hwmod *oh)
> {
> - int r;
> + int r, i;
> int hwsup = 0;
>
> pr_debug("omap_hwmod: %s: enabling\n", oh->name);
> @@ -2109,17 +2109,23 @@ static int _enable(struct omap_hwmod *oh)
> }
>
> /*
> - * If an IP block contains HW reset lines and all of them are
> - * asserted, we let integration code associated with that
> - * block handle the enable. We've received very little
> + * If an IP block contains HW reset lines, all of them are
> + * asserted, and the IP block is marked as requiring a custom
> + * hardreset handler, we let integration code associated with
> + * that block handle the enable. We've received very little
> * information on what those driver authors need, and until
> * detailed information is provided and the driver code is
> * posted to the public lists, this is probably the best we
> * can do.
> */
> - if (_are_all_hardreset_lines_asserted(oh))
> + if ((oh->flags & HWMOD_CUSTOM_HARDRESET) &&
> + _are_all_hardreset_lines_asserted(oh))
> return 0;
>
> + /* 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
> +
> /* Mux pins for device runtime if populated */
> if (oh->mux && (!oh->mux->enabled ||
> ((oh->_state == _HWMOD_STATE_IDLE) &&
> diff --git a/arch/arm/mach-omap2/omap_hwmod.h b/arch/arm/mach-omap2/omap_hwmod.h
> index 76bce11c85a4..4198829551a4 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.h
> +++ b/arch/arm/mach-omap2/omap_hwmod.h
> @@ -525,6 +525,17 @@ struct omap_hwmod_omap4_prcm {
> * or idled.
> * HWMOD_OPT_CLKS_NEEDED: The optional clocks are needed for the module to
> * operate and they need to be handled at the same time as the main_clk.
> + * HWMOD_CUSTOM_HARDRESET: By default, if a hwmod has PRCM hardreset
> + * lines associated with it (i.e., a populated .rst_lines field in
> + * the hwmod), the hwmod code will assert the hardreset lines when
> + * the IP block is initially reset, deassert the hardreset lines
> + * in _enable(), and reassert them in _shutdown(). If this flag
> + * is set, the hwmod code will not deassert the hardreset lines in
> + * _enable(), leaving this responsibility to the driver code. This flag may
> + * be needed for processor IP blocks that must be put into a WFI/HLT
> + * state after reset is deasserted, lest the processor leave its MSTANDBY
> + * signal deasserted, thus blocking the chip from entering a system-wide
> + * low power state.
> */
> #define HWMOD_SWSUP_SIDLE (1 << 0)
> #define HWMOD_SWSUP_MSTANDBY (1 << 1)
> @@ -541,6 +552,7 @@ struct omap_hwmod_omap4_prcm {
> #define HWMOD_SWSUP_SIDLE_ACT (1 << 12)
> #define HWMOD_RECONFIG_IO_CHAIN (1 << 13)
> #define HWMOD_OPT_CLKS_NEEDED (1 << 14)
> +#define HWMOD_CUSTOM_HARDRESET (1 << 15)
>
> /*
> * omap_hwmod._int_flags definitions
> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> index dad871a4cd96..20501f0e3c6c 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> @@ -553,6 +553,7 @@ static struct omap_hwmod omap44xx_dsp_hwmod = {
> .modulemode = MODULEMODE_HWCTRL,
> },
> },
> + .flags = HWMOD_CUSTOM_HARDRESET,
> };
>
> /*
> @@ -1433,6 +1434,7 @@ static struct omap_hwmod omap44xx_ipu_hwmod = {
> .modulemode = MODULEMODE_HWCTRL,
> },
> },
> + .flags = HWMOD_CUSTOM_HARDRESET,
> };
>
> /*
> @@ -1517,6 +1519,7 @@ static struct omap_hwmod omap44xx_iva_hwmod = {
> .modulemode = MODULEMODE_HWCTRL,
> },
> },
> + .flags = HWMOD_CUSTOM_HARDRESET,
> };
>
> /*
> @@ -2115,6 +2118,7 @@ static struct omap_hwmod omap44xx_mmu_ipu_hwmod = {
> .modulemode = MODULEMODE_HWCTRL,
> },
> },
> + .flags = HWMOD_CUSTOM_HARDRESET, /* XXX doublecheck */
> };
>
> /* mmu dsp */
> @@ -2147,6 +2151,7 @@ static struct omap_hwmod omap44xx_mmu_dsp_hwmod = {
> .modulemode = MODULEMODE_HWCTRL,
> },
> },
> + .flags = HWMOD_CUSTOM_HARDRESET, /* XXX doublecheck */
> };
>
> /*
> @@ -2299,6 +2304,7 @@ static struct omap_hwmod omap44xx_prm_hwmod = {
> .class = &omap44xx_prcm_hwmod_class,
> .rst_lines = omap44xx_prm_resets,
> .rst_lines_cnt = ARRAY_SIZE(omap44xx_prm_resets),
> + .flags = HWMOD_CUSTOM_HARDRESET,
> };
>
> /*
>
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-08 20:56 ` Suman Anna
@ 2016-02-09 8:49 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-02-09 8:49 UTC (permalink / raw)
To: Suman Anna
Cc: Kishon Vijay Abraham I, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel, nsekhar
On Mon, 8 Feb 2016, Suman Anna wrote:
> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
> > 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?
>
> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-09 8:49 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-02-09 8:49 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 8 Feb 2016, Suman Anna wrote:
> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
> > 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?
>
> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-09 8:49 ` Paul Walmsley
(?)
@ 2016-02-09 17:40 ` Suman Anna
-1 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-09 17:40 UTC (permalink / raw)
To: Paul Walmsley
Cc: Kishon Vijay Abraham I, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel, nsekhar
Hi Paul,
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
> On Mon, 8 Feb 2016, Suman Anna wrote:
>
>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>> 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?
>>
>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-09 17:40 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-09 17:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi Paul,
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
> On Mon, 8 Feb 2016, Suman Anna wrote:
>
>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>> 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?
>>
>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-09 17:40 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-09 17:40 UTC (permalink / raw)
To: Paul Walmsley
Cc: Kishon Vijay Abraham I, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel, nsekhar
Hi Paul,
On 02/09/2016 02:49 AM, Paul Walmsley wrote:
> On Mon, 8 Feb 2016, Suman Anna wrote:
>
>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>> 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?
>>
>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-09 17:40 ` Suman Anna
@ 2016-02-09 19:36 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-02-09 19:36 UTC (permalink / raw)
To: Suman Anna
Cc: Kishon Vijay Abraham I, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel, nsekhar
Hi Suman
On Tue, 9 Feb 2016, Suman Anna wrote:
> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
> > On Mon, 8 Feb 2016, Suman Anna wrote:
> >> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
> >>> 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?
> >>
> >> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-09 19:36 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-02-09 19:36 UTC (permalink / raw)
To: linux-arm-kernel
Hi Suman
On Tue, 9 Feb 2016, Suman Anna wrote:
> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
> > On Mon, 8 Feb 2016, Suman Anna wrote:
> >> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
> >>> 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?
> >>
> >> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-09 19:36 ` Paul Walmsley
(?)
@ 2016-02-10 1:42 ` Suman Anna
-1 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-10 1:42 UTC (permalink / raw)
To: Paul Walmsley
Cc: Kishon Vijay Abraham I, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel, nsekhar
Hi Paul,
On 02/09/2016 01:36 PM, Paul Walmsley wrote:
> Hi Suman
>
> On Tue, 9 Feb 2016, Suman Anna wrote:
>
>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>> 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?
>>>>
>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-10 1:42 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-10 1:42 UTC (permalink / raw)
To: linux-arm-kernel
Hi Paul,
On 02/09/2016 01:36 PM, Paul Walmsley wrote:
> Hi Suman
>
> On Tue, 9 Feb 2016, Suman Anna wrote:
>
>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>> 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?
>>>>
>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-10 1:42 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-10 1:42 UTC (permalink / raw)
To: Paul Walmsley
Cc: Kishon Vijay Abraham I, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel, nsekhar
Hi Paul,
On 02/09/2016 01:36 PM, Paul Walmsley wrote:
> Hi Suman
>
> On Tue, 9 Feb 2016, Suman Anna wrote:
>
>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>> 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?
>>>>
>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-10 1:42 ` Suman Anna
(?)
@ 2016-02-10 5:38 ` Kishon Vijay Abraham I
-1 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-10 5:38 UTC (permalink / raw)
To: Suman Anna, Paul Walmsley
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
Hi,
On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
> Hi Paul,
>
> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>> Hi Suman
>>
>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>
>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>> 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?
>>>>>
>>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-10 5:38 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-10 5:38 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
> Hi Paul,
>
> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>> Hi Suman
>>
>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>
>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>> 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?
>>>>>
>>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-10 5:38 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-10 5:38 UTC (permalink / raw)
To: Suman Anna, Paul Walmsley
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
Hi,
On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
> Hi Paul,
>
> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>> Hi Suman
>>
>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>
>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>> 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?
>>>>>
>>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-10 5:38 ` Kishon Vijay Abraham I
@ 2016-02-11 19:27 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-02-11 19:27 UTC (permalink / raw)
To: Suman Anna, Kishon Vijay Abraham I, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
Hi Kishon, Suman,
On Wed, 10 Feb 2016, Kishon Vijay Abraham I wrote:
> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
> > On 02/09/2016 01:36 PM, Paul Walmsley wrote:
> >> On Tue, 9 Feb 2016, Suman Anna wrote:
> >>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
> >>>> On Mon, 8 Feb 2016, Suman Anna wrote:
> >>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
> >>>>>> 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?
> >>>>>
> >>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-11 19:27 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-02-11 19:27 UTC (permalink / raw)
To: linux-arm-kernel
Hi Kishon, Suman,
On Wed, 10 Feb 2016, Kishon Vijay Abraham I wrote:
> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
> > On 02/09/2016 01:36 PM, Paul Walmsley wrote:
> >> On Tue, 9 Feb 2016, Suman Anna wrote:
> >>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
> >>>> On Mon, 8 Feb 2016, Suman Anna wrote:
> >>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
> >>>>>> 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?
> >>>>>
> >>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-11 19:27 ` Paul Walmsley
(?)
@ 2016-02-11 22:04 ` Suman Anna
-1 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-11 22:04 UTC (permalink / raw)
To: Paul Walmsley, Kishon Vijay Abraham I, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
On 02/11/2016 01:27 PM, Paul Walmsley wrote:
> Hi Kishon, Suman,
>
> On Wed, 10 Feb 2016, Kishon Vijay Abraham I wrote:
>
>> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>>> 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?
>>>>>>>
>>>>>>> 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
>
> Dave, any further comments here?
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-11 22:04 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-11 22:04 UTC (permalink / raw)
To: linux-arm-kernel
On 02/11/2016 01:27 PM, Paul Walmsley wrote:
> Hi Kishon, Suman,
>
> On Wed, 10 Feb 2016, Kishon Vijay Abraham I wrote:
>
>> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>>> 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?
>>>>>>>
>>>>>>> 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
>
> Dave, any further comments here?
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-11 22:04 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-11 22:04 UTC (permalink / raw)
To: Paul Walmsley, Kishon Vijay Abraham I, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
On 02/11/2016 01:27 PM, Paul Walmsley wrote:
> Hi Kishon, Suman,
>
> On Wed, 10 Feb 2016, Kishon Vijay Abraham I wrote:
>
>> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>>> 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?
>>>>>>>
>>>>>>> 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
>
> Dave, any further comments here?
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-11 19:27 ` Paul Walmsley
(?)
@ 2016-02-12 6:49 ` Kishon Vijay Abraham I
-1 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-12 6:49 UTC (permalink / raw)
To: Paul Walmsley, Suman Anna, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
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:
>
>> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>>> 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?
>>>>>>>
>>>>>>> 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.
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index e9f65fe..24cafd9 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -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);
+ }
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.
>
> Dave, any further comments here?
>
>
> - Paul
>
^ permalink raw reply related [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-12 6:49 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-12 6:49 UTC (permalink / raw)
To: linux-arm-kernel
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:
>
>> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>>> 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?
>>>>>>>
>>>>>>> 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.
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index e9f65fe..24cafd9 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -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);
+ }
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.
>
> Dave, any further comments here?
>
>
> - Paul
>
^ permalink raw reply related [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-12 6:49 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-12 6:49 UTC (permalink / raw)
To: Paul Walmsley, Suman Anna, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
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:
>
>> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>>> 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?
>>>>>>>
>>>>>>> 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.
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index e9f65fe..24cafd9 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -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);
+ }
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.
>
> Dave, any further comments here?
>
>
> - Paul
>
^ permalink raw reply related [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-12 6:49 ` Kishon Vijay Abraham I
(?)
@ 2016-02-12 17:20 ` Suman Anna
-1 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-12 17:20 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Paul Walmsley, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
Kishon,
On 02/12/2016 12:49 AM, Kishon Vijay Abraham I wrote:
> 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:
>>
>>> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>>>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>>>> 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?
>>>>>>>>
>>>>>>>> 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.
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index e9f65fe..24cafd9 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -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
>>
>> Dave, any further comments here?
>>
>>
>> - Paul
>>
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-12 17:20 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-12 17:20 UTC (permalink / raw)
To: linux-arm-kernel
Kishon,
On 02/12/2016 12:49 AM, Kishon Vijay Abraham I wrote:
> 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:
>>
>>> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>>>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>>>> 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?
>>>>>>>>
>>>>>>>> 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.
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index e9f65fe..24cafd9 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -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
>>
>> Dave, any further comments here?
>>
>>
>> - Paul
>>
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-12 17:20 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-12 17:20 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Paul Walmsley, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
Kishon,
On 02/12/2016 12:49 AM, Kishon Vijay Abraham I wrote:
> 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:
>>
>>> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>>>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>>>> 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?
>>>>>>>>
>>>>>>>> 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.
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index e9f65fe..24cafd9 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -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
>>
>> Dave, any further comments here?
>>
>>
>> - Paul
>>
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-12 17:20 ` Suman Anna
(?)
@ 2016-02-18 14:21 ` Sekhar Nori
-1 siblings, 0 replies; 120+ messages in thread
From: Sekhar Nori @ 2016-02-18 14:21 UTC (permalink / raw)
To: Suman Anna, Kishon Vijay Abraham I, Paul Walmsley, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel
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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
From: Sekhar Nori <nsekhar@ti.com>
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 <nsekhar@ti.com>
---
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(+)
diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index b61355e2a771..252b74633e31 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
*
*/
+/*
+ * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
+ * functionality of OMAP HWMOD layer does not deassert the hardreset lines
+ * associated with an IP automatically leaving the driver to handle that
+ * by itself. This does not work for PCIeSS which needs the reset lines
+ * deasserted for the driver to start accessing registers.
+ *
+ * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
+ * lines after asserting them.
+ */
+static int dra7xx_pciess_reset(struct omap_hwmod *oh)
+{
+ int i;
+
+ for (i = 0; i < oh->rst_lines_cnt; i++) {
+ omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
+ omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
+ }
+
+ return 0;
+}
+
static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
.name = "pcie",
+ .reset = dra7xx_pciess_reset,
};
/* pcie1 */
--
2.6.3
^ permalink raw reply related [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-18 14:21 ` Sekhar Nori
0 siblings, 0 replies; 120+ messages in thread
From: Sekhar Nori @ 2016-02-18 14:21 UTC (permalink / raw)
To: linux-arm-kernel
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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
From: Sekhar Nori <nsekhar@ti.com>
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 <nsekhar@ti.com>
---
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(+)
diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index b61355e2a771..252b74633e31 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
*
*/
+/*
+ * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
+ * functionality of OMAP HWMOD layer does not deassert the hardreset lines
+ * associated with an IP automatically leaving the driver to handle that
+ * by itself. This does not work for PCIeSS which needs the reset lines
+ * deasserted for the driver to start accessing registers.
+ *
+ * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
+ * lines after asserting them.
+ */
+static int dra7xx_pciess_reset(struct omap_hwmod *oh)
+{
+ int i;
+
+ for (i = 0; i < oh->rst_lines_cnt; i++) {
+ omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
+ omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
+ }
+
+ return 0;
+}
+
static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
.name = "pcie",
+ .reset = dra7xx_pciess_reset,
};
/* pcie1 */
--
2.6.3
^ permalink raw reply related [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-18 14:21 ` Sekhar Nori
0 siblings, 0 replies; 120+ messages in thread
From: Sekhar Nori @ 2016-02-18 14:21 UTC (permalink / raw)
To: Suman Anna, Kishon Vijay Abraham I, Paul Walmsley, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel
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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
From: Sekhar Nori <nsekhar@ti.com>
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 <nsekhar@ti.com>
---
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(+)
diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index b61355e2a771..252b74633e31 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
*
*/
+/*
+ * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
+ * functionality of OMAP HWMOD layer does not deassert the hardreset lines
+ * associated with an IP automatically leaving the driver to handle that
+ * by itself. This does not work for PCIeSS which needs the reset lines
+ * deasserted for the driver to start accessing registers.
+ *
+ * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
+ * lines after asserting them.
+ */
+static int dra7xx_pciess_reset(struct omap_hwmod *oh)
+{
+ int i;
+
+ for (i = 0; i < oh->rst_lines_cnt; i++) {
+ omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
+ omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
+ }
+
+ return 0;
+}
+
static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
.name = "pcie",
+ .reset = dra7xx_pciess_reset,
};
/* pcie1 */
--
2.6.3
^ permalink raw reply related [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-18 14:21 ` Sekhar Nori
@ 2016-02-18 17:23 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-02-18 17:23 UTC (permalink / raw)
To: Sekhar Nori, Suman Anna, Kishon Vijay Abraham I, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel
On Thu, 18 Feb 2016, Sekhar Nori wrote:
> 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. 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
>
> Thanks,
> Sekhar
>
> ---8<---
> >From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
> Message-Id: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
> From: Sekhar Nori <nsekhar@ti.com>
> 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 <nsekhar@ti.com>
> ---
> 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(+)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index b61355e2a771..252b74633e31 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> *
> */
>
> +/*
> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> + * associated with an IP automatically leaving the driver to handle that
> + * by itself. This does not work for PCIeSS which needs the reset lines
> + * deasserted for the driver to start accessing registers.
> + *
> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> + * lines after asserting them.
> + */
> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> +{
> + int i;
> +
> + for (i = 0; i < oh->rst_lines_cnt; i++) {
> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> + }
> +
> + return 0;
> +}
> +
> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> .name = "pcie",
> + .reset = dra7xx_pciess_reset,
> };
>
> /* pcie1 */
> --
> 2.6.3
>
>
- Paul
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-18 17:23 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-02-18 17:23 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 18 Feb 2016, Sekhar Nori wrote:
> 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. 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
>
> Thanks,
> Sekhar
>
> ---8<---
> >From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
> Message-Id: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
> From: Sekhar Nori <nsekhar@ti.com>
> 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 <nsekhar@ti.com>
> ---
> 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(+)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index b61355e2a771..252b74633e31 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> *
> */
>
> +/*
> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> + * associated with an IP automatically leaving the driver to handle that
> + * by itself. This does not work for PCIeSS which needs the reset lines
> + * deasserted for the driver to start accessing registers.
> + *
> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> + * lines after asserting them.
> + */
> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> +{
> + int i;
> +
> + for (i = 0; i < oh->rst_lines_cnt; i++) {
> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> + }
> +
> + return 0;
> +}
> +
> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> .name = "pcie",
> + .reset = dra7xx_pciess_reset,
> };
>
> /* pcie1 */
> --
> 2.6.3
>
>
- Paul
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-18 17:23 ` Paul Walmsley
(?)
@ 2016-02-18 18:27 ` Suman Anna
-1 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-18 18:27 UTC (permalink / raw)
To: Paul Walmsley, Sekhar Nori, Kishon Vijay Abraham I, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel
On 02/18/2016 11:23 AM, Paul Walmsley wrote:
>
> On Thu, 18 Feb 2016, Sekhar Nori wrote:
>
>> 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. 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
>
>
>>
>> Thanks,
>> Sekhar
>>
>> ---8<---
>> >From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
>> Message-Id: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
>> From: Sekhar Nori <nsekhar@ti.com>
>> 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.
>>
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> ---
>> 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(+)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> index b61355e2a771..252b74633e31 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>> *
>> */
>>
>> +/*
>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
>> + * associated with an IP automatically leaving the driver to handle that
>> + * by itself. This does not work for PCIeSS which needs the reset lines
>> + * deasserted for the driver to start accessing registers.
>> + *
>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
>> + * lines after asserting them.
>> + */
>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
Hmm, ignoring return values??
regards
Suman
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>> .name = "pcie",
>> + .reset = dra7xx_pciess_reset,
>> };
>>
>> /* pcie1 */
>> --
>> 2.6.3
>>
>>
>
>
> - Paul
>
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-18 18:27 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-18 18:27 UTC (permalink / raw)
To: linux-arm-kernel
On 02/18/2016 11:23 AM, Paul Walmsley wrote:
>
> On Thu, 18 Feb 2016, Sekhar Nori wrote:
>
>> 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. 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
>
>
>>
>> Thanks,
>> Sekhar
>>
>> ---8<---
>> >From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
>> Message-Id: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
>> From: Sekhar Nori <nsekhar@ti.com>
>> 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.
>>
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> ---
>> 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(+)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> index b61355e2a771..252b74633e31 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>> *
>> */
>>
>> +/*
>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
>> + * associated with an IP automatically leaving the driver to handle that
>> + * by itself. This does not work for PCIeSS which needs the reset lines
>> + * deasserted for the driver to start accessing registers.
>> + *
>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
>> + * lines after asserting them.
>> + */
>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
Hmm, ignoring return values??
regards
Suman
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>> .name = "pcie",
>> + .reset = dra7xx_pciess_reset,
>> };
>>
>> /* pcie1 */
>> --
>> 2.6.3
>>
>>
>
>
> - Paul
>
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-18 18:27 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-18 18:27 UTC (permalink / raw)
To: Paul Walmsley, Sekhar Nori, Kishon Vijay Abraham I, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel
On 02/18/2016 11:23 AM, Paul Walmsley wrote:
>
> On Thu, 18 Feb 2016, Sekhar Nori wrote:
>
>> 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. 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
>
>
>>
>> Thanks,
>> Sekhar
>>
>> ---8<---
>> >From e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19 Mon Sep 17 00:00:00 2001
>> Message-Id: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
>> From: Sekhar Nori <nsekhar@ti.com>
>> 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.
>>
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> ---
>> 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(+)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> index b61355e2a771..252b74633e31 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>> *
>> */
>>
>> +/*
>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
>> + * associated with an IP automatically leaving the driver to handle that
>> + * by itself. This does not work for PCIeSS which needs the reset lines
>> + * deasserted for the driver to start accessing registers.
>> + *
>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
>> + * lines after asserting them.
>> + */
>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
Hmm, ignoring return values??
regards
Suman
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>> .name = "pcie",
>> + .reset = dra7xx_pciess_reset,
>> };
>>
>> /* pcie1 */
>> --
>> 2.6.3
>>
>>
>
>
> - Paul
>
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-18 14:21 ` Sekhar Nori
(?)
@ 2016-02-22 6:18 ` Kishon Vijay Abraham I
-1 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-22 6:18 UTC (permalink / raw)
To: Sekhar Nori, Suman Anna, Paul Walmsley, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel
Sekhar,
On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
> From: Sekhar Nori <nsekhar@ti.com>
> 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 <nsekhar@ti.com>
> ---
> 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(+)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index b61355e2a771..252b74633e31 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> *
> */
>
> +/*
> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> + * associated with an IP automatically leaving the driver to handle that
> + * by itself. This does not work for PCIeSS which needs the reset lines
> + * deasserted for the driver to start accessing registers.
> + *
> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> + * lines after asserting them.
> + */
> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> +{
> + int i;
> +
> + for (i = 0; i < oh->rst_lines_cnt; i++) {
> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> + }
> +
> + return 0;
> +}
> +
> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> .name = "pcie",
> + .reset = dra7xx_pciess_reset,
> };
Thanks for the patch.
-Kishon
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-22 6:18 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-22 6:18 UTC (permalink / raw)
To: linux-arm-kernel
Sekhar,
On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
> From: Sekhar Nori <nsekhar@ti.com>
> 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 <nsekhar@ti.com>
> ---
> 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(+)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index b61355e2a771..252b74633e31 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> *
> */
>
> +/*
> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> + * associated with an IP automatically leaving the driver to handle that
> + * by itself. This does not work for PCIeSS which needs the reset lines
> + * deasserted for the driver to start accessing registers.
> + *
> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> + * lines after asserting them.
> + */
> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> +{
> + int i;
> +
> + for (i = 0; i < oh->rst_lines_cnt; i++) {
> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> + }
> +
> + return 0;
> +}
> +
> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> .name = "pcie",
> + .reset = dra7xx_pciess_reset,
> };
Thanks for the patch.
-Kishon
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-22 6:18 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-22 6:18 UTC (permalink / raw)
To: Sekhar Nori, Suman Anna, Paul Walmsley, d-gerlach
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel
Sekhar,
On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
> From: Sekhar Nori <nsekhar@ti.com>
> 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 <nsekhar@ti.com>
> ---
> 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(+)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index b61355e2a771..252b74633e31 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> *
> */
>
> +/*
> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> + * associated with an IP automatically leaving the driver to handle that
> + * by itself. This does not work for PCIeSS which needs the reset lines
> + * deasserted for the driver to start accessing registers.
> + *
> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> + * lines after asserting them.
> + */
> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> +{
> + int i;
> +
> + for (i = 0; i < oh->rst_lines_cnt; i++) {
> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> + }
> +
> + return 0;
> +}
> +
> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> .name = "pcie",
> + .reset = dra7xx_pciess_reset,
> };
Thanks for the patch.
-Kishon
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-22 6:18 ` Kishon Vijay Abraham I
@ 2016-02-22 6:31 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-02-22 6:31 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Sekhar Nori, Suman Anna, d-gerlach, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel
Kishon,
On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
> Sekhar,
>
> On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
> > 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
> > From: Sekhar Nori <nsekhar@ti.com>
> > 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 <nsekhar@ti.com>
> > ---
> > 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(+)
> >
> > diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > index b61355e2a771..252b74633e31 100644
> > --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> > *
> > */
> >
> > +/*
> > + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> > + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> > + * associated with an IP automatically leaving the driver to handle that
> > + * by itself. This does not work for PCIeSS which needs the reset lines
> > + * deasserted for the driver to start accessing registers.
> > + *
> > + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> > + * lines after asserting them.
> > + */
> > +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < oh->rst_lines_cnt; i++) {
> > + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> > + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> > .name = "pcie",
> > + .reset = dra7xx_pciess_reset,
> > };
>
> Thanks for the patch.
Could you please test the bind/unbind functionality just to make sure it
works?
thanks
- Paul
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-22 6:31 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-02-22 6:31 UTC (permalink / raw)
To: linux-arm-kernel
Kishon,
On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
> Sekhar,
>
> On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
> > 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
> > From: Sekhar Nori <nsekhar@ti.com>
> > 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 <nsekhar@ti.com>
> > ---
> > 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(+)
> >
> > diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > index b61355e2a771..252b74633e31 100644
> > --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> > *
> > */
> >
> > +/*
> > + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> > + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> > + * associated with an IP automatically leaving the driver to handle that
> > + * by itself. This does not work for PCIeSS which needs the reset lines
> > + * deasserted for the driver to start accessing registers.
> > + *
> > + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> > + * lines after asserting them.
> > + */
> > +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < oh->rst_lines_cnt; i++) {
> > + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> > + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> > .name = "pcie",
> > + .reset = dra7xx_pciess_reset,
> > };
>
> Thanks for the patch.
Could you please test the bind/unbind functionality just to make sure it
works?
thanks
- Paul
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-22 6:31 ` Paul Walmsley
(?)
@ 2016-02-22 9:55 ` Kishon Vijay Abraham I
-1 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-22 9:55 UTC (permalink / raw)
To: Paul Walmsley
Cc: Sekhar Nori, Suman Anna, d-gerlach, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel
Hi Paul,
On Monday 22 February 2016 12:01 PM, Paul Walmsley wrote:
> Kishon,
>
> On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
>
>> Sekhar,
>>
>> On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
>>> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
>>> From: Sekhar Nori <nsekhar@ti.com>
>>> 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 <nsekhar@ti.com>
>>> ---
>>> 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(+)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> index b61355e2a771..252b74633e31 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>>> *
>>> */
>>>
>>> +/*
>>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
>>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
>>> + * associated with an IP automatically leaving the driver to handle that
>>> + * by itself. This does not work for PCIeSS which needs the reset lines
>>> + * deasserted for the driver to start accessing registers.
>>> + *
>>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
>>> + * lines after asserting them.
>>> + */
>>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
>>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
>>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>>> .name = "pcie",
>>> + .reset = dra7xx_pciess_reset,
>>> };
>>
>> Thanks for the patch.
>
> 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/
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-22 9:55 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-22 9:55 UTC (permalink / raw)
To: linux-arm-kernel
Hi Paul,
On Monday 22 February 2016 12:01 PM, Paul Walmsley wrote:
> Kishon,
>
> On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
>
>> Sekhar,
>>
>> On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
>>> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
>>> From: Sekhar Nori <nsekhar@ti.com>
>>> 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 <nsekhar@ti.com>
>>> ---
>>> 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(+)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> index b61355e2a771..252b74633e31 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>>> *
>>> */
>>>
>>> +/*
>>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
>>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
>>> + * associated with an IP automatically leaving the driver to handle that
>>> + * by itself. This does not work for PCIeSS which needs the reset lines
>>> + * deasserted for the driver to start accessing registers.
>>> + *
>>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
>>> + * lines after asserting them.
>>> + */
>>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
>>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
>>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>>> .name = "pcie",
>>> + .reset = dra7xx_pciess_reset,
>>> };
>>
>> Thanks for the patch.
>
> 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/
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-22 9:55 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-22 9:55 UTC (permalink / raw)
To: Paul Walmsley
Cc: Sekhar Nori, Suman Anna, d-gerlach, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel
Hi Paul,
On Monday 22 February 2016 12:01 PM, Paul Walmsley wrote:
> Kishon,
>
> On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
>
>> Sekhar,
>>
>> On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
>>> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
>>> From: Sekhar Nori <nsekhar@ti.com>
>>> 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 <nsekhar@ti.com>
>>> ---
>>> 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(+)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> index b61355e2a771..252b74633e31 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>>> *
>>> */
>>>
>>> +/*
>>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
>>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
>>> + * associated with an IP automatically leaving the driver to handle that
>>> + * by itself. This does not work for PCIeSS which needs the reset lines
>>> + * deasserted for the driver to start accessing registers.
>>> + *
>>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
>>> + * lines after asserting them.
>>> + */
>>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
>>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
>>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>>> .name = "pcie",
>>> + .reset = dra7xx_pciess_reset,
>>> };
>>
>> Thanks for the patch.
>
> 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/
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-22 9:55 ` Kishon Vijay Abraham I
(?)
@ 2016-02-23 11:57 ` Kishon Vijay Abraham I
-1 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-23 11:57 UTC (permalink / raw)
To: Paul Walmsley
Cc: Sekhar Nori, Suman Anna, d-gerlach, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel
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:
>> Kishon,
>>
>> On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
>>
>>> Sekhar,
>>>
>>> On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
>>>> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
>>>> From: Sekhar Nori <nsekhar@ti.com>
>>>> 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 <nsekhar@ti.com>
>>>> ---
>>>> 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(+)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>> index b61355e2a771..252b74633e31 100644
>>>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>>>> *
>>>> */
>>>>
>>>> +/*
>>>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
>>>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
>>>> + * associated with an IP automatically leaving the driver to handle that
>>>> + * by itself. This does not work for PCIeSS which needs the reset lines
>>>> + * deasserted for the driver to start accessing registers.
>>>> + *
>>>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
>>>> + * lines after asserting them.
>>>> + */
>>>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
>>>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
>>>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>>>> .name = "pcie",
>>>> + .reset = dra7xx_pciess_reset,
>>>> };
>>>
>>> Thanks for the patch.
>>
>> 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?
-Kishon
>
> Thanks
> Kishon
>
> [1] -> http://pastebin.ubuntu.com/15169387/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-23 11:57 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-23 11:57 UTC (permalink / raw)
To: linux-arm-kernel
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:
>> Kishon,
>>
>> On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
>>
>>> Sekhar,
>>>
>>> On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
>>>> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
>>>> From: Sekhar Nori <nsekhar@ti.com>
>>>> 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 <nsekhar@ti.com>
>>>> ---
>>>> 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(+)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>> index b61355e2a771..252b74633e31 100644
>>>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>>>> *
>>>> */
>>>>
>>>> +/*
>>>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
>>>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
>>>> + * associated with an IP automatically leaving the driver to handle that
>>>> + * by itself. This does not work for PCIeSS which needs the reset lines
>>>> + * deasserted for the driver to start accessing registers.
>>>> + *
>>>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
>>>> + * lines after asserting them.
>>>> + */
>>>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
>>>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
>>>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>>>> .name = "pcie",
>>>> + .reset = dra7xx_pciess_reset,
>>>> };
>>>
>>> Thanks for the patch.
>>
>> 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?
-Kishon
>
> Thanks
> Kishon
>
> [1] -> http://pastebin.ubuntu.com/15169387/
> --
> 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
>
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-23 11:57 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-23 11:57 UTC (permalink / raw)
To: Paul Walmsley
Cc: Sekhar Nori, Suman Anna, d-gerlach, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel
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:
>> Kishon,
>>
>> On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
>>
>>> Sekhar,
>>>
>>> On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
>>>> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
>>>> From: Sekhar Nori <nsekhar@ti.com>
>>>> 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 <nsekhar@ti.com>
>>>> ---
>>>> 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(+)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>> index b61355e2a771..252b74633e31 100644
>>>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>>>> *
>>>> */
>>>>
>>>> +/*
>>>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
>>>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
>>>> + * associated with an IP automatically leaving the driver to handle that
>>>> + * by itself. This does not work for PCIeSS which needs the reset lines
>>>> + * deasserted for the driver to start accessing registers.
>>>> + *
>>>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
>>>> + * lines after asserting them.
>>>> + */
>>>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
>>>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
>>>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>>>> .name = "pcie",
>>>> + .reset = dra7xx_pciess_reset,
>>>> };
>>>
>>> Thanks for the patch.
>>
>> 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?
-Kishon
>
> Thanks
> Kishon
>
> [1] -> http://pastebin.ubuntu.com/15169387/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-23 11:57 ` Kishon Vijay Abraham I
@ 2016-02-23 18:28 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-02-23 18:28 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Sekhar Nori, Suman Anna, d-gerlach, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel
Kishon
On Tue, 23 Feb 2016, Kishon Vijay Abraham I wrote:
> On Monday 22 February 2016 03:25 PM, Kishon Vijay Abraham I wrote:
> > On Monday 22 February 2016 12:01 PM, Paul Walmsley wrote:
> >> On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
> >>> On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
> >>>> On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
> >>>>> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
> >>>> From: Sekhar Nori <nsekhar@ti.com>
> >>>> 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 <nsekhar@ti.com>
> >>>> ---
> >>>> 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(+)
> >>>>
> >>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> >>>> index b61355e2a771..252b74633e31 100644
> >>>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> >>>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> >>>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> >>>> *
> >>>> */
> >>>>
> >>>> +/*
> >>>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> >>>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> >>>> + * associated with an IP automatically leaving the driver to handle that
> >>>> + * by itself. This does not work for PCIeSS which needs the reset lines
> >>>> + * deasserted for the driver to start accessing registers.
> >>>> + *
> >>>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> >>>> + * lines after asserting them.
> >>>> + */
> >>>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> >>>> +{
> >>>> + int i;
> >>>> +
> >>>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
> >>>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> >>>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> >>>> + }
> >>>> +
> >>>> + return 0;
> >>>> +}
> >>>> +
> >>>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> >>>> .name = "pcie",
> >>>> + .reset = dra7xx_pciess_reset,
> >>>> };
> >>>
> >>> Thanks for the patch.
> >>
> >> 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
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-23 18:28 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-02-23 18:28 UTC (permalink / raw)
To: linux-arm-kernel
Kishon
On Tue, 23 Feb 2016, Kishon Vijay Abraham I wrote:
> On Monday 22 February 2016 03:25 PM, Kishon Vijay Abraham I wrote:
> > On Monday 22 February 2016 12:01 PM, Paul Walmsley wrote:
> >> On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
> >>> On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
> >>>> On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
> >>>>> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
> >>>> From: Sekhar Nori <nsekhar@ti.com>
> >>>> 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 <nsekhar@ti.com>
> >>>> ---
> >>>> 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(+)
> >>>>
> >>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> >>>> index b61355e2a771..252b74633e31 100644
> >>>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> >>>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> >>>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> >>>> *
> >>>> */
> >>>>
> >>>> +/*
> >>>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> >>>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> >>>> + * associated with an IP automatically leaving the driver to handle that
> >>>> + * by itself. This does not work for PCIeSS which needs the reset lines
> >>>> + * deasserted for the driver to start accessing registers.
> >>>> + *
> >>>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> >>>> + * lines after asserting them.
> >>>> + */
> >>>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> >>>> +{
> >>>> + int i;
> >>>> +
> >>>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
> >>>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> >>>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> >>>> + }
> >>>> +
> >>>> + return 0;
> >>>> +}
> >>>> +
> >>>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> >>>> .name = "pcie",
> >>>> + .reset = dra7xx_pciess_reset,
> >>>> };
> >>>
> >>> Thanks for the patch.
> >>
> >> 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
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-23 18:28 ` Paul Walmsley
(?)
@ 2016-02-24 6:21 ` Kishon Vijay Abraham I
-1 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-24 6:21 UTC (permalink / raw)
To: Paul Walmsley
Cc: Sekhar Nori, Suman Anna, d-gerlach, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel
Hi Paul,
On Tuesday 23 February 2016 11:58 PM, Paul Walmsley wrote:
> Kishon
>
> On Tue, 23 Feb 2016, Kishon Vijay Abraham I wrote:
>
>> On Monday 22 February 2016 03:25 PM, Kishon Vijay Abraham I wrote:
>>> On Monday 22 February 2016 12:01 PM, Paul Walmsley wrote:
>>>> On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
>>>>> On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
>>>>>> On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
>>>>>>> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
>>>>>> From: Sekhar Nori <nsekhar@ti.com>
>>>>>> 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 <nsekhar@ti.com>
>>>>>> ---
>>>>>> 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(+)
>>>>>>
>>>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>>>> index b61355e2a771..252b74633e31 100644
>>>>>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>>>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>>>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>>>>>> *
>>>>>> */
>>>>>>
>>>>>> +/*
>>>>>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
>>>>>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
>>>>>> + * associated with an IP automatically leaving the driver to handle that
>>>>>> + * by itself. This does not work for PCIeSS which needs the reset lines
>>>>>> + * deasserted for the driver to start accessing registers.
>>>>>> + *
>>>>>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
>>>>>> + * lines after asserting them.
>>>>>> + */
>>>>>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
>>>>>> +{
>>>>>> + int i;
>>>>>> +
>>>>>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
>>>>>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
>>>>>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
>>>>>> + }
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>>>>>> .name = "pcie",
>>>>>> + .reset = dra7xx_pciess_reset,
>>>>>> };
>>>>>
>>>>> Thanks for the patch.
>>>>
>>>> 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 <kishon@ti.com>
>
> 2. Are you going to follow up with these PCIe core issues with the PCIe
> core developers?
yeah, I see efforts have already started to convert the PCI drivers from
built-in to module [1] and issues arising out of that will be debugged.
Thanks
Kishon
[1] -> http://lkml.iu.edu/hypermail/linux/kernel/1602.0/06011.html
>
>
> - Paul
>
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-24 6:21 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-24 6:21 UTC (permalink / raw)
To: linux-arm-kernel
Hi Paul,
On Tuesday 23 February 2016 11:58 PM, Paul Walmsley wrote:
> Kishon
>
> On Tue, 23 Feb 2016, Kishon Vijay Abraham I wrote:
>
>> On Monday 22 February 2016 03:25 PM, Kishon Vijay Abraham I wrote:
>>> On Monday 22 February 2016 12:01 PM, Paul Walmsley wrote:
>>>> On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
>>>>> On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
>>>>>> On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
>>>>>>> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
>>>>>> From: Sekhar Nori <nsekhar@ti.com>
>>>>>> 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 <nsekhar@ti.com>
>>>>>> ---
>>>>>> 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(+)
>>>>>>
>>>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>>>> index b61355e2a771..252b74633e31 100644
>>>>>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>>>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>>>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>>>>>> *
>>>>>> */
>>>>>>
>>>>>> +/*
>>>>>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
>>>>>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
>>>>>> + * associated with an IP automatically leaving the driver to handle that
>>>>>> + * by itself. This does not work for PCIeSS which needs the reset lines
>>>>>> + * deasserted for the driver to start accessing registers.
>>>>>> + *
>>>>>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
>>>>>> + * lines after asserting them.
>>>>>> + */
>>>>>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
>>>>>> +{
>>>>>> + int i;
>>>>>> +
>>>>>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
>>>>>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
>>>>>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
>>>>>> + }
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>>>>>> .name = "pcie",
>>>>>> + .reset = dra7xx_pciess_reset,
>>>>>> };
>>>>>
>>>>> Thanks for the patch.
>>>>
>>>> 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 <kishon@ti.com>
>
> 2. Are you going to follow up with these PCIe core issues with the PCIe
> core developers?
yeah, I see efforts have already started to convert the PCI drivers from
built-in to module [1] and issues arising out of that will be debugged.
Thanks
Kishon
[1] -> http://lkml.iu.edu/hypermail/linux/kernel/1602.0/06011.html
>
>
> - Paul
>
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-24 6:21 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-24 6:21 UTC (permalink / raw)
To: Paul Walmsley
Cc: Sekhar Nori, Suman Anna, d-gerlach, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel
Hi Paul,
On Tuesday 23 February 2016 11:58 PM, Paul Walmsley wrote:
> Kishon
>
> On Tue, 23 Feb 2016, Kishon Vijay Abraham I wrote:
>
>> On Monday 22 February 2016 03:25 PM, Kishon Vijay Abraham I wrote:
>>> On Monday 22 February 2016 12:01 PM, Paul Walmsley wrote:
>>>> On Mon, 22 Feb 2016, Kishon Vijay Abraham I wrote:
>>>>> On Thursday 18 February 2016 07:51 PM, Sekhar Nori wrote:
>>>>>> On Friday 12 February 2016 10:50 PM, Suman Anna wrote:
>>>>>>> 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: <e3ba368f2235e1bf38a22ba8ea4e5c12aaafda19.1455803758.git.nsekhar@ti.com>
>>>>>> From: Sekhar Nori <nsekhar@ti.com>
>>>>>> 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 <nsekhar@ti.com>
>>>>>> ---
>>>>>> 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(+)
>>>>>>
>>>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>>>> index b61355e2a771..252b74633e31 100644
>>>>>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>>>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>>>>> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>>>>>> *
>>>>>> */
>>>>>>
>>>>>> +/*
>>>>>> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
>>>>>> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
>>>>>> + * associated with an IP automatically leaving the driver to handle that
>>>>>> + * by itself. This does not work for PCIeSS which needs the reset lines
>>>>>> + * deasserted for the driver to start accessing registers.
>>>>>> + *
>>>>>> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
>>>>>> + * lines after asserting them.
>>>>>> + */
>>>>>> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
>>>>>> +{
>>>>>> + int i;
>>>>>> +
>>>>>> + for (i = 0; i < oh->rst_lines_cnt; i++) {
>>>>>> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
>>>>>> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
>>>>>> + }
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>>>>>> .name = "pcie",
>>>>>> + .reset = dra7xx_pciess_reset,
>>>>>> };
>>>>>
>>>>> Thanks for the patch.
>>>>
>>>> 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 <kishon@ti.com>
>
> 2. Are you going to follow up with these PCIe core issues with the PCIe
> core developers?
yeah, I see efforts have already started to convert the PCI drivers from
built-in to module [1] and issues arising out of that will be debugged.
Thanks
Kishon
[1] -> http://lkml.iu.edu/hypermail/linux/kernel/1602.0/06011.html
>
>
> - Paul
>
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-24 6:21 ` Kishon Vijay Abraham I
@ 2016-03-01 8:25 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-03-01 8:25 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Sekhar Nori, Suman Anna, d-gerlach, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel
Folks, the following is what I've queued for this.
- Paul
From: Sekhar Nori <nsekhar@ti.com>
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 <nsekhar@ti.com>
Reported-by: Richard Cochran <richardcochran@gmail.com>
Tested-by: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Suman Anna <s-anna@ti.com>
Cc: Dave Gerlach <d-gerlach@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index b61355e2a771..252b74633e31 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
*
*/
+/*
+ * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
+ * functionality of OMAP HWMOD layer does not deassert the hardreset lines
+ * associated with an IP automatically leaving the driver to handle that
+ * by itself. This does not work for PCIeSS which needs the reset lines
+ * deasserted for the driver to start accessing registers.
+ *
+ * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
+ * lines after asserting them.
+ */
+static int dra7xx_pciess_reset(struct omap_hwmod *oh)
+{
+ int i;
+
+ for (i = 0; i < oh->rst_lines_cnt; i++) {
+ omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
+ omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
+ }
+
+ return 0;
+}
+
static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
.name = "pcie",
+ .reset = dra7xx_pciess_reset,
};
/* pcie1 */
--
2.7.0
^ permalink raw reply related [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-03-01 8:25 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2016-03-01 8:25 UTC (permalink / raw)
To: linux-arm-kernel
Folks, the following is what I've queued for this.
- Paul
From: Sekhar Nori <nsekhar@ti.com>
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 <nsekhar@ti.com>
Reported-by: Richard Cochran <richardcochran@gmail.com>
Tested-by: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Suman Anna <s-anna@ti.com>
Cc: Dave Gerlach <d-gerlach@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index b61355e2a771..252b74633e31 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
*
*/
+/*
+ * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
+ * functionality of OMAP HWMOD layer does not deassert the hardreset lines
+ * associated with an IP automatically leaving the driver to handle that
+ * by itself. This does not work for PCIeSS which needs the reset lines
+ * deasserted for the driver to start accessing registers.
+ *
+ * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
+ * lines after asserting them.
+ */
+static int dra7xx_pciess_reset(struct omap_hwmod *oh)
+{
+ int i;
+
+ for (i = 0; i < oh->rst_lines_cnt; i++) {
+ omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
+ omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
+ }
+
+ return 0;
+}
+
static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
.name = "pcie",
+ .reset = dra7xx_pciess_reset,
};
/* pcie1 */
--
2.7.0
^ permalink raw reply related [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-03-01 8:25 ` Paul Walmsley
(?)
@ 2016-03-01 11:55 ` Kishon Vijay Abraham I
-1 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-03-01 11:55 UTC (permalink / raw)
To: Paul Walmsley
Cc: Sekhar Nori, Suman Anna, d-gerlach, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel
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
>
>
> - Paul
>
>
> From: Sekhar Nori <nsekhar@ti.com>
> 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 <nsekhar@ti.com>
> Reported-by: Richard Cochran <richardcochran@gmail.com>
> Tested-by: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Suman Anna <s-anna@ti.com>
> Cc: Dave Gerlach <d-gerlach@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> ---
> arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index b61355e2a771..252b74633e31 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> *
> */
>
> +/*
> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> + * associated with an IP automatically leaving the driver to handle that
> + * by itself. This does not work for PCIeSS which needs the reset lines
> + * deasserted for the driver to start accessing registers.
> + *
> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> + * lines after asserting them.
> + */
> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> +{
> + int i;
> +
> + for (i = 0; i < oh->rst_lines_cnt; i++) {
> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> + }
> +
> + return 0;
> +}
> +
> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> .name = "pcie",
> + .reset = dra7xx_pciess_reset,
> };
>
> /* pcie1 */
>
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-03-01 11:55 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-03-01 11:55 UTC (permalink / raw)
To: linux-arm-kernel
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
>
>
> - Paul
>
>
> From: Sekhar Nori <nsekhar@ti.com>
> 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 <nsekhar@ti.com>
> Reported-by: Richard Cochran <richardcochran@gmail.com>
> Tested-by: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Suman Anna <s-anna@ti.com>
> Cc: Dave Gerlach <d-gerlach@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> ---
> arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index b61355e2a771..252b74633e31 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> *
> */
>
> +/*
> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> + * associated with an IP automatically leaving the driver to handle that
> + * by itself. This does not work for PCIeSS which needs the reset lines
> + * deasserted for the driver to start accessing registers.
> + *
> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> + * lines after asserting them.
> + */
> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> +{
> + int i;
> +
> + for (i = 0; i < oh->rst_lines_cnt; i++) {
> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> + }
> +
> + return 0;
> +}
> +
> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> .name = "pcie",
> + .reset = dra7xx_pciess_reset,
> };
>
> /* pcie1 */
>
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-03-01 11:55 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-03-01 11:55 UTC (permalink / raw)
To: Paul Walmsley
Cc: Sekhar Nori, Suman Anna, d-gerlach, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel
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
>
>
> - Paul
>
>
> From: Sekhar Nori <nsekhar@ti.com>
> 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 <nsekhar@ti.com>
> Reported-by: Richard Cochran <richardcochran@gmail.com>
> Tested-by: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Suman Anna <s-anna@ti.com>
> Cc: Dave Gerlach <d-gerlach@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> ---
> arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index b61355e2a771..252b74633e31 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> *
> */
>
> +/*
> + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> + * associated with an IP automatically leaving the driver to handle that
> + * by itself. This does not work for PCIeSS which needs the reset lines
> + * deasserted for the driver to start accessing registers.
> + *
> + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> + * lines after asserting them.
> + */
> +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> +{
> + int i;
> +
> + for (i = 0; i < oh->rst_lines_cnt; i++) {
> + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> + }
> +
> + return 0;
> +}
> +
> static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> .name = "pcie",
> + .reset = dra7xx_pciess_reset,
> };
>
> /* pcie1 */
>
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-03-01 11:55 ` Kishon Vijay Abraham I
@ 2016-03-01 14:43 ` Bjorn Helgaas
-1 siblings, 0 replies; 120+ messages in thread
From: Bjorn Helgaas @ 2016-03-01 14:43 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Paul Walmsley, Sekhar Nori, Suman Anna, d-gerlach, Tony Lindgren,
Bjorn Helgaas, richardcochran, Russell King, linux-omap,
linux-arm-kernel, linux-kernel
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:
> >
> > 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.
> > From: Sekhar Nori <nsekhar@ti.com>
> > 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 <nsekhar@ti.com>
> > Reported-by: Richard Cochran <richardcochran@gmail.com>
> > Tested-by: Kishon Vijay Abraham I <kishon@ti.com>
> > Cc: Suman Anna <s-anna@ti.com>
> > Cc: Dave Gerlach <d-gerlach@ti.com>
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Russell King <linux@arm.linux.org.uk>
> > Signed-off-by: Paul Walmsley <paul@pwsan.com>
> > ---
> > arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > index b61355e2a771..252b74633e31 100644
> > --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> > *
> > */
> >
> > +/*
> > + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> > + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> > + * associated with an IP automatically leaving the driver to handle that
> > + * by itself. This does not work for PCIeSS which needs the reset lines
> > + * deasserted for the driver to start accessing registers.
> > + *
> > + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> > + * lines after asserting them.
> > + */
> > +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < oh->rst_lines_cnt; i++) {
> > + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> > + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> > .name = "pcie",
> > + .reset = dra7xx_pciess_reset,
> > };
> >
> > /* pcie1 */
> >
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-03-01 14:43 ` Bjorn Helgaas
0 siblings, 0 replies; 120+ messages in thread
From: Bjorn Helgaas @ 2016-03-01 14:43 UTC (permalink / raw)
To: linux-arm-kernel
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:
> >
> > 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.
> > From: Sekhar Nori <nsekhar@ti.com>
> > 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 <nsekhar@ti.com>
> > Reported-by: Richard Cochran <richardcochran@gmail.com>
> > Tested-by: Kishon Vijay Abraham I <kishon@ti.com>
> > Cc: Suman Anna <s-anna@ti.com>
> > Cc: Dave Gerlach <d-gerlach@ti.com>
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Russell King <linux@arm.linux.org.uk>
> > Signed-off-by: Paul Walmsley <paul@pwsan.com>
> > ---
> > arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > index b61355e2a771..252b74633e31 100644
> > --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > @@ -1526,8 +1526,31 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
> > *
> > */
> >
> > +/*
> > + * As noted in documentation for _reset() in omap_hwmod.c, the stock reset
> > + * functionality of OMAP HWMOD layer does not deassert the hardreset lines
> > + * associated with an IP automatically leaving the driver to handle that
> > + * by itself. This does not work for PCIeSS which needs the reset lines
> > + * deasserted for the driver to start accessing registers.
> > + *
> > + * We use a PCIeSS HWMOD class specific reset handler to deassert the hardreset
> > + * lines after asserting them.
> > + */
> > +static int dra7xx_pciess_reset(struct omap_hwmod *oh)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < oh->rst_lines_cnt; i++) {
> > + omap_hwmod_assert_hardreset(oh, oh->rst_lines[i].name);
> > + omap_hwmod_deassert_hardreset(oh, oh->rst_lines[i].name);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
> > .name = "pcie",
> > + .reset = dra7xx_pciess_reset,
> > };
> >
> > /* pcie1 */
> >
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-03-01 11:55 ` Kishon Vijay Abraham I
(?)
@ 2016-03-01 16:55 ` Suman Anna
-1 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-03-01 16:55 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Paul Walmsley
Cc: Sekhar Nori, d-gerlach, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel
On 03/01/2016 05:55 AM, Kishon Vijay Abraham I wrote:
> 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.
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]
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-03-01 16:55 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-03-01 16:55 UTC (permalink / raw)
To: linux-arm-kernel
On 03/01/2016 05:55 AM, Kishon Vijay Abraham I wrote:
> 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.
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]
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-03-01 16:55 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-03-01 16:55 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Paul Walmsley
Cc: Sekhar Nori, d-gerlach, Tony Lindgren, Bjorn Helgaas,
richardcochran, Russell King, linux-omap, linux-arm-kernel,
linux-kernel
On 03/01/2016 05:55 AM, Kishon Vijay Abraham I wrote:
> 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.
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]
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-10 5:38 ` Kishon Vijay Abraham I
(?)
@ 2016-02-11 20:43 ` Suman Anna
-1 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-11 20:43 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Paul Walmsley
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
On 02/09/2016 11:38 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>> Hi Paul,
>>
>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>> Hi Suman
>>>
>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>
>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>> 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?
>>>>>>
>>>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-11 20:43 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-11 20:43 UTC (permalink / raw)
To: linux-arm-kernel
On 02/09/2016 11:38 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>> Hi Paul,
>>
>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>> Hi Suman
>>>
>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>
>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>> 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?
>>>>>>
>>>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-11 20:43 ` Suman Anna
0 siblings, 0 replies; 120+ messages in thread
From: Suman Anna @ 2016-02-11 20:43 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Paul Walmsley
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
On 02/09/2016 11:38 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>> Hi Paul,
>>
>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>> Hi Suman
>>>
>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>
>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>> 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?
>>>>>>
>>>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-11 20:43 ` Suman Anna
(?)
@ 2016-02-12 6:55 ` Kishon Vijay Abraham I
-1 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-12 6:55 UTC (permalink / raw)
To: Suman Anna, Paul Walmsley
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
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:
>> Hi,
>>
>> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>>> Hi Paul,
>>>
>>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>>> Hi Suman
>>>>
>>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>>
>>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>>> 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?
>>>>>>>
>>>>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-12 6:55 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-12 6:55 UTC (permalink / raw)
To: linux-arm-kernel
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:
>> Hi,
>>
>> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>>> Hi Paul,
>>>
>>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>>> Hi Suman
>>>>
>>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>>
>>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>>> 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?
>>>>>>>
>>>>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-12 6:55 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-12 6:55 UTC (permalink / raw)
To: Suman Anna, Paul Walmsley
Cc: Russell King, Tony Lindgren, richardcochran, nsekhar,
linux-kernel, Bjorn Helgaas, linux-omap, linux-arm-kernel
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:
>> Hi,
>>
>> On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:
>>> Hi Paul,
>>>
>>> On 02/09/2016 01:36 PM, Paul Walmsley wrote:
>>>> Hi Suman
>>>>
>>>> On Tue, 9 Feb 2016, Suman Anna wrote:
>>>>
>>>>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>>>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>>>>> 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?
>>>>>>>
>>>>>>> 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
2016-02-09 19:36 ` Paul Walmsley
(?)
@ 2016-02-10 5:36 ` Kishon Vijay Abraham I
-1 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-10 5:36 UTC (permalink / raw)
To: Paul Walmsley, Suman Anna
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
On Wednesday 10 February 2016 01:06 AM, Paul Walmsley wrote:
> Hi Suman
>
> On Tue, 9 Feb 2016, Suman Anna wrote:
>
>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>> 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?
>>>>
>>>> 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
>
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-10 5:36 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-10 5:36 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 10 February 2016 01:06 AM, Paul Walmsley wrote:
> Hi Suman
>
> On Tue, 9 Feb 2016, Suman Anna wrote:
>
>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>> 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?
>>>>
>>>> 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
>
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
@ 2016-02-10 5:36 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 120+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-10 5:36 UTC (permalink / raw)
To: Paul Walmsley, Suman Anna
Cc: Tony Lindgren, Bjorn Helgaas, richardcochran, Russell King,
linux-omap, linux-arm-kernel, linux-kernel, nsekhar
On Wednesday 10 February 2016 01:06 AM, Paul Walmsley wrote:
> Hi Suman
>
> On Tue, 9 Feb 2016, Suman Anna wrote:
>
>> On 02/09/2016 02:49 AM, Paul Walmsley wrote:
>>> On Mon, 8 Feb 2016, Suman Anna wrote:
>>>> On 02/07/2016 08:48 PM, Paul Walmsley wrote:
>>>>> 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?
>>>>
>>>> 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
>
^ permalink raw reply [flat|nested] 120+ messages in thread