Re: [PATCH v4 1/4] dt-bindings: interconnect: qcom,sdm845-cpu-bwmon: add BWMON device

[Rajendra Nayak]

On 6/26/2022 8:49 AM, Bjorn Andersson wrote:
> On Wed 22 Jun 06:58 CDT 2022, Rajendra Nayak wrote:
>
> > On 6/7/2022 12:20 PM, Krzysztof Kozlowski wrote:
> > > On 06/06/2022 23:11, Bjorn Andersson wrote:
> > > > On Wed 01 Jun 03:11 PDT 2022, Krzysztof Kozlowski wrote:
> > > >
> > > > > Add bindings for the Qualcomm Bandwidth Monitor device providing
> > > > > performance data on interconnects.  The bindings describe only BWMON
> > > > > version 4, e.g. the instance on SDM845 between CPU and Last Level Cache
> > > > > Controller.
> > > > >
> > > > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> > > > > Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
> > > > > Acked-by: Georgi Djakov <djakov@xxxxxxxxxx>
> > > > > ---
> > > > >    .../interconnect/qcom,sdm845-cpu-bwmon.yaml   | 97 +++++++++++++++++++
> > > > >    1 file changed, 97 insertions(+)
> > > > >    create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml b/Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..8c82e06ee432
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
> > > > > @@ -0,0 +1,97 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/interconnect/qcom,sdm845-cpu-bwmon.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Qualcomm Interconnect Bandwidth Monitor
> > > > > +
> > > > > +maintainers:
> > > > > +  - Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> > > > > +
> > > > > +description:
> > > > > +  Bandwidth Monitor measures current throughput on buses between various NoC
> > > > > +  fabrics and provides information when it crosses configured thresholds.
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    enum:
> > > > > +      - qcom,sdm845-cpu-bwmon       # BWMON v4
> > > >
> > > > It seems the thing that's called bwmon v4 is compatible with a number of
> > > > different platforms, should we add a generic compatible to the binding
> > > > as well, to avoid having to update the implementation for each SoC?
> > > >
> > > > (I.e. "qcom,sdm845-cpu-bwmon", "qcom,bwmon-v4")
> >
> > it seems pretty useful to have the "qcom,bwmon-v4" and "qcom,bwmon-v5"
> > compatibles, I tried these patches on a sc7280 device which has a bwmon4
> > between the cpu and caches (and also has a bwmon5 between the caches and DDR)
> > and the driver works with zero changes.
>
> But does the '4' and '5' has a relation to the hardware? Or is just the
> 4th and 5th register layout supported by the downstream driver?

Right, it was just based on the downstream driver register layouts, i could not
find these numbers in HW specs anywhere, but that said I do see 2 instances of
these, one of them called the LAGG bwmon which is the one between the LLCC and DDR
and is documented as part of the LLCC specs. I'll try and dig somemore into the
documentation to see how we could define compatibles to match hw revs.

> Regards,
> Bjorn