Re: [PATCH 1/2] dt-bindings: Documentation for qcom,llcc
From: Channa
Date: Tue Feb 13 2018 - 12:38:44 EST
On 2018-02-13 06:37, Mark Rutland wrote:
On Tue, Feb 06, 2018 at 11:56:50AM -0800, Channa wrote:
On 2018-02-02 03:05, Mark Rutland wrote:
> On Thu, Feb 01, 2018 at 12:39:09PM -0800, Channa wrote:
> > On 2018-02-01 02:44, Mark Rutland wrote:
> > > On Thu, Jan 25, 2018 at 03:55:12PM -0800, Channagoud Kadabi wrote:
> > > > +- llcc-bank-off:
> > > > + Usage: required
> > > > + Value Type: <u32 array>
> > > > + Definition: Offsets of llcc banks from llcc base address starting
> > > > from
> > > > + LLCC bank0.
> > > > +
> > > > +- llcc-broadcast-off:
> > > > + Usage: required
> > > > + Value Type: <u32>
> > > > + Definition: Offset of broadcast register from LLCC bank0 address.
> > >
> > > Please could we use "offset" rather than "off" for both of these? That
> > > way it's obvious these aren't properties for disabling some feature.
> > >
> > > How variable are these offsets in practice? Is the memory map not fixed?
> >
> > The offsets depends on the number of LLCC HW blocks. These number of
> > HW
> > blocks vary from
> > chipset to chipset and new registers could be added that changes the
> > offset.
>
> Surely if new registers are added, we need a new compatible string?
>
> Can't we encode the number of LLCC HW blocks, instead? Presumably that
> would give enough information to cover both llcc-bank-off and
> llcc-broadcast-off.
>
> [...]
Are you suggesting to move these offset handing out of DTS files and
manage
in the driver?
Something like that, though it depends on how exactly the offsets can
be
derived.
Using reg entries, as Matt suggested, sounds better though.
> > > > +compatible devices:
> > > > + qcom,sdm845-llcc
> > >
> > > Huh? The "qcom,sdm845-llcc" bindings wasn't described above, and it's
> > > not clear what this means.
> > >
> > > > +
> > > > +Example:
> > > > +
> > > > + qcom,system-cache@1300000 {
> > > > + compatible = "qcom,llcc-core", "syscon", "simple-mfd";
> > >
> > > This looks very wrong. Why do you need syscon and simple-mfd?
> >
> > LLCC HW block has 3 functionalities:
> > System cache core, ECC & AMON drivers for debugging.
> > All three drivers use the same register space for configuration,
> > status etc.
> > In order to avoid remapping the same address region across multiple
> > drivers,
> > I have implemented this driver as a syncon and simple-mfd.
>
> Please don't do that; that's completely dependent on Linux
> implementation details.
Why do you think simple-mfd is not good here? The LLCC HW clock is
outside
of CPUSS and has
multiple functional blocks.
For one thing, there's no need for this to be a syscon *and* a
simple-mfd.
W.R.T. simple-mfd, I think it would bet better to decompose the device
in a top-level driver, as I described in my prior reply, rather than
describing a set of drivers (which are not themselves HW).
> > > > +- cache-slices:
> > > > + Usage: required
> > > > + Value type: <prop-encoded-array>
> > > > + Definition: The tuple has phandle to llcc device as the first
> > > > argument and the
> > > > + second argument is the usecase id of the client.
> > >
> > > What is a "usecase id" ?
> >
> > Usecase id for use case that wants to use system cache for eg:
> > video-encode
> > and video-decode
>
> Sure, but how is the value used? Is it the index of a slice? Or
> something more abstract?
This is used as an index to the SCT (System cache Table) configuration
data that controls the behavior of each cache slice.
Ok. Where does that SCT live? Is that in HW? Is it programmed by SW, or
statically configured for a given platform?
SCT is in the HW. Kernel driver programs these settings for a given
platform. The table is also used to lookup size, cache slice id details
requested by client drivers.
Thanks,
Mark.
--
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project