RE: [PATCH v3 04/12] dt-bindings: timer: arm,arch_timer: Add optional clock and reset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Marc and Mark,

On 03 May 2022 16:57 Marc Zyngier wrote:
> On Tue, 03 May 2022 15:22:35 +0100, Geert Uytterhoeven wrote:
> > On Tue, May 3, 2022 at 3:12 PM Marc Zyngier wrote:
> > > On 2022-05-03 12:55, Phil Edworthy wrote:
> > > > Some SoCs use a gated clock for the timer and the means to reset the
> > > > timer.
> > > > Hence add these as optional.
> > >
> > > The architecture is crystal clear on the subject: the counter
> > > is in an always-on domain. Why should this be visible to SW?
> > > Also, reseting the counter breaks the guaranteed monotonicity
> > > we rely on.
> >
> > The DT bindings do state:
> >
> >   always-on:
> >     type: boolean
> >     description: If present, the timer is powered through an always-on
> power
> >       domain, therefore it never loses context.
> >
> > and (surprisingly?) the absence of this property seems to be the
> > norm...
> 
> *timer* is the key word. And counter != timer. What your HW has is a
> gate on the *counter* which is illegal if observable from NS SW.
Ok, thanks for your feedback. We'll pretend this clock gate and reset
doesn't exist and drop this patch.
 
> >
> > And:
> >
> >   arm,no-tick-in-suspend:
> >     type: boolean
> >     description: The main counter does not tick when the system is in
> >       low-power system suspend on some SoCs. This behavior does not
> match the
> >       Architecture Reference Manual's specification that the system
> > counter "must
> >       be implemented in an always-on power domain."
> >
> > So there's already precedent for clocks that can be disabled.
> 
> No, this is only the case in *suspend*, as the name of the property
> vaguely hints at. And that's a property for a bug. In your case, the
> clock can be controlled arbitrarily, which is even worse.
> 
> >
> > > Worse case, this belongs to the boot firmware, not the kernel,
> > > and I don't think this should be described in the DT.
> >
> > "DT describes hardware, not software policy"?
> 
> I'm happy to spread "always-on" properties all over the shop, but
> that's not helping. The HW spec says it in bold letters: the counter
> is always running, and doesn't jump backward. I can't imagine how
> secure SW will behave when you reset its counter... :-/

Thanks
Phil




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux