[PATCH RESEND] ARM: l2x0: add three special L210 aux control flags

Brad Mouring brad.mouring at ni.com
Tue Apr 5 07:05:15 PDT 2016


On Tue, Apr 05, 2016 at 08:12:54AM -0500, Josh Cartwright wrote:
> On Tue, Apr 05, 2016 at 09:08:27AM +0100, Russell King - ARM Linux wrote:
> > On Tue, Apr 05, 2016 at 12:36:40AM +0100, Russell King - ARM Linux wrote:
> > > On Mon, Apr 04, 2016 at 06:29:26PM -0500, Josh Cartwright wrote:
> > > > Similarly, we're waiting on feedback for:
> > > > 
> > > >   http://lkml.kernel.org/r/1456761716-10174-1-git-send-email-brad.mouring@ni.com
> > > > 
> > > > Also in the queue; also w/ Rob's Ack.  There was a change awhile back to
> > > > unconditionally enable the power-management features of the PL310 which
> > > > caused a noticeable performance degradation on our boards; placing the
> > > > PM options behind a togglable DT property provides us an out.
> > > 
> > > For that one, I've been wondering why it's seemingly acceptable to
> > > start throwing *errors* for *new* DT properties which weren't required
> > > before.  Also, there's no DT documentation for the new properties,
> > > which is a fundamental requirement - and Rob should not have given
> > > his ack without there being a DT documentation patch.

2/2 Is the documentation. The patch that sits in the queue is the combination
of those two, including the documentation since the help of the patch queue
recommends submitting a single feature as a single patch

Queued patch: http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8549/1

> > > Obviously, the kernel review process has broken on this one... or
> > > I must remember this sneaky trick for getting new DT properties in
> > > without the required documentation...
> > 
> > There's another issue here - we go from enabling these options by
> > default to leaving them as the firmware specified them.  While that's
> > something I approve of, I'm not happy because it means all platforms
> > that have these PM bits enabled today could end up losing them - so
> > it's a PM regression waiting to happen.  Who's going to add them to
> > all the dt blobs where platforms _can_ cope with having these PM bits
> > enabled?
> 
> Viewing the pending patch as causing a PM regression is one way to look
> at it, however another way to look at is is that the commit which
> unconditionally enabled the PM features caused a performance regression;
> in particular commit 3a43b581dac1 ("ARM: l2c: always enable low power
> modes") which landed in v3.16.  (Perhaps, though, the distinction isn't
> important, as it doesn't necessarily help dictate a course of action.)

I initially wrote the patch maintaining the "on-by-default" behavior. So
many of the other DT-controlled features, however, take the approach of
defaulting to the firmware's configuration, it seems odd and out-of-place
to suddenly impose a default setting.

Of course, I am open to feedback, and thanks for giving this some attention
Russell.

Brad



More information about the linux-arm-kernel mailing list