Re: [PATCH 2/2] dt-bindings: mfd: syscon: Add support for regmap fast-io

[AngeloGioacchino Del Regno]

Il 02/04/22 13:38, Krzysztof Kozlowski ha scritto:
> On 01/04/2022 15:50, AngeloGioacchino Del Regno wrote:
> > The syscon driver now enables the .fast_io regmap configuration when
> > the 'fast-io' property is found in a syscon node.
> >
> > Keeping in mind that, in regmap, fast_io is checked only if we are
> > not using hardware spinlocks, allow the fast-io property only if
> > there is no hwlocks reference (and vice-versa).
>
> I have doubts you need a property for this. "fast" is subjective in
> terms of hardware, so this looks more like a software property, not
> hardware.
>
> I think most of MMIOs inside a SoC are considered fast. Usually also the
> syscon/regmap consumer knows which regmap it gets, so knows that it is
> fast or not.

Hello Krzysztof,

well yes, this property is changing how software behaves - specifically,
as you've correctly understood, what regmap does.

It's true that most of MMIOs inside a SoC are considered fast.. the word "most" is
the exact reason why I haven't proposed simply hardcoding '.fast_io = true' in
syscon, or in regmap-mmio...
There are too many different SoCs around, and I didn't want to end up breaking
anything (even if it should be unlikely, since MMIO is fast by principle).


> > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
> > ---
> >   Documentation/devicetree/bindings/mfd/syscon.yaml | 15 +++++++++++++++
> >   1 file changed, 15 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
> > index 13baa452cc9d..85a2e83b5861 100644
> > --- a/Documentation/devicetree/bindings/mfd/syscon.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
> > @@ -83,11 +83,26 @@ properties:
> >       $ref: /schemas/types.yaml#/definitions/uint32
> >       enum: [1, 2, 4, 8]
> >   
> > +  fast-io:
> > +    description:
> > +      Indicates that this bus has a very fast IO, for which
> > +      acquiring a mutex would be significant overhead.
> > +      When present, regmap will use a spinlock instead.
>
> Regmap is current implementation behind this, but it's not related to
> hardware, so how about removing it from the description? Something like:
> "..., for which different locking methods should be used to reduce
> overhead (e.g. spinlock instead of mutex)."

That's a very good point. I didn't think about any future in which the
implementation would be changed from regmap to *new-name-here*... but
anyway it makes a lot more sense to "speak generic".

I'll change the description to match your proposal, thank you!

Regards,
Angelo