Hi
A few comments
On Mon, 15 Sep 2014, Tony Lindgren wrote:
> In sprz318f.pdf "Usage Note 2.7" says that UARTs cannot acknowledge
> idle requests in smartidle mode when configured for DMA operations.
I don't have Rev. F here. But reading between the typos in Rev. E, all it
says that the UART should be placed into force-idle mode when there are no
DMA transfers scheduled, otherwise CORE won't go idle. However, this
patch does a lot more than that...
> This prevents L4 from going idle. So let's use force-idle mode
> instead and also force reconfiguring of the IO chain wake-up events.
>
> Otherwise systems using Sebastian's 8250 patches with DMA will
> never enter deeper idle states because of the errata above.
>
> Cc: Paul Walmsley <paul@xxxxxxxxx>
> Cc: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> Signed-off-by: Tony Lindgren <tony@xxxxxxxxxxx>
> ---
> arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> index 61cd49d..55f088c 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> @@ -490,7 +490,9 @@ static struct omap_hwmod omap3xxx_uart1_hwmod = {
> .mpu_irqs = omap2_uart1_mpu_irqs,
> .sdma_reqs = omap2_uart1_sdma_reqs,
> .main_clk = "uart1_fck",
> - .flags = DEBUG_TI81XXUART1_FLAGS | HWMOD_SWSUP_SIDLE_ACT,
> + .flags = DEBUG_TI81XXUART1_FLAGS |
> + HWMOD_NO_OCP_AUTOIDLE | HWMOD_SWSUP_SIDLE |
> + HWMOD_FORCE_MSTANDBY | HWMOD_RECONFIG_IO_CHAIN,
... this patch makes four significant changes to the hwmod flags:
1. It changes HWMOD_SWSUP_SIDLE_ACT to HWMOD_SWSUP_SIDLE
2. It adds HWMOD_NO_OCP_AUTOIDLE
3. It adds HWMOD_FORCE_MSTANDBY
4. It adds HWMOD_RECONFIG_IO_CHAIN
As far as I can tell, the only change that actually matches the erratum
usage note is #1. And even that should only be activated when the UART is
in DMA mode.. PIO mode shouldn't need it. (We used to have a DMA-specific
workaround in the driver code for this years ago, which was only enabled
when DMA was used, but it got ripped out in the rush to get rid of the
platform_data function pointers.)
So that leaves the other three flag changes. Of those three, adding
HWMOD_FORCE_MSTANDBY is definitely not correct according to the 34xx
public TRM Rev. ZV, since the UART IP blocks:
a) have no master port,
b) do not contain DMA controllers (they rely on the SDMA), and
c) according to the SYSC_REG register documentation in Table 17-112 of the
above TRM, have no MIDLEMODE bit field.
So adding HWMOD_FORCE_MSTANDBY should have zero effect and it most likely
should be dropped.
Regarding the rest:
I'd be pretty surprised if HWMOD_RECONFIG_IO_CHAIN is needed. If that was
needed, I'd think we would have seen wakeup failures in this path a long
time ago, since the console serial port is the primary debug tool we used
for waking the chip up from deep idle states on demand.
As far as HWMOD_NO_OCP_AUTOIDLE goes: does adding this flag have an effect
when HWMOD_SWSUP_SIDLE is used? If so then this should probably go in as
a separate patch, because it's probably a separate bug than the one
described in the commit message.
...
So I'd suggest restricting this patch to simply change
HWMOD_SWSUP_SIDLE_ACT to HWMOD_SWSUP_SIDLE. Then if the other flags are
needed, would suggest adding them in separate patches, describing what
problems are fixed by adding them.
- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
