Re: [PATCH 2/2]: Be stric clocksource/drivers/fttmr010 on IRQs

From: Guenter Roeck
Date: Mon Aug 30 2021 - 11:24:18 EST


On 8/30/21 12:47 AM, Cédric Le Goater wrote:
On 8/30/21 6:58 AM, Guenter Roeck wrote:
On 8/29/21 9:16 PM, Andrew Jeffery wrote:
[ ... ]

I don't have the manuals, so I can't say what the correct behavior is,
but at least there is some evidence that TIMER_INTR_STATE may not exist
on ast2400 and ast2500 SOCs.

On Aspeed SoCs AST2400 and AST2500, the TMC[34] register is a
"control register #2" whereas on the AST2600 it is an "interruptarch/arm/boot/dts/ast2600-facebook-netbmc-common.dtsi:#include
status register" with bits [0-7] holding the timers status.

I would say that the patch simply should handle the "is_aspeed" case.

Well, is_aspeed is set true in the driver for all of the 2400, 2500 and
2600. 0x34 behaves the way this patch expects on the 2600. So I think
we need something less coarse than is_aspeed?


If I understand the code correctly, ast2400 and ast2500 execute
fttmr010_timer_interrupt(), while ast2600 has its own interrupt handler.
To make this work, it would probably be necessary to check for is_aspeed
in fttmr010_timer_interrupt(), and only execute the new code if the flag
is false. The existing flag in struct fttmr010 should be good enough
for that.

yes.

I wonder why we have ast2600 support in fttmr010. The AST2600 boards use
the arm_arch_timer AFAICT.


It was introduced and enabled, but later disabled with commit c998f40f2ae6a4
("ARM: dts: aspeed: ast2600: Set arch timer always-on"):

"According to ASPEED, FTTMR010 is not intended to be used in the AST2600.
The arch timer should be used, but Linux doesn't enable high-res timers
without being assured that the arch timer is always on, so set that
property in the devicetree."

That commit also disables the FTTMR010 timer, but doesn't remove the devicetree
node. Maybe it would make sense to remove the ast2600 code from the fttmr010
driver, including the devicetree node. After all, it looks like it is dead.

Guenter