Re: [PATCH v3 1/9] dt-bindings: reset: Add a binding for the RPi Firmware reset controller

[Rob Herring]

On Tue, Jul 14, 2020 at 3:18 PM Florian Fainelli <f.fainelli@xxxxxxxxx> wrote:
>
>
>
> On 7/14/2020 2:07 PM, Rob Herring wrote:
> > On Tue, Jul 14, 2020 at 01:59:21PM +0200, Nicolas Saenz Julienne wrote:
> >> On Mon, 2020-07-13 at 12:23 -0600, Rob Herring wrote:
> >>> On Fri, Jun 12, 2020 at 07:13:25PM +0200, Nicolas Saenz Julienne wrote:
> >>>> The firmware running on the RPi VideoCore can be used to reset and
> >>>> initialize HW controlled by the firmware.
> >>>>
> >>>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx>
> >>>> Reviewed-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
> >>>>
> >>>> ---
> >>>> Changes since v2:
> >>>>  - Add include file for reset IDs
> >>>>
> >>>> Changes since v1:
> >>>>  - Correct cells binding as per Florian's comment
> >>>>  - Change compatible string to be more generic
> >>>>
> >>>>  .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 21 +++++++++++++++++++
> >>>>  .../reset/raspberrypi,firmware-reset.h        | 13 ++++++++++++
> >>>>  2 files changed, 34 insertions(+)
> >>>>  create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> >>>> firmware.yaml
> >>>> b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> >>>> firmware.yaml
> >>>> index b48ed875eb8e..23a885af3a28 100644
> >>>> --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> >>>> firmware.yaml
> >>>> +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> >>>> firmware.yaml
> >>>> @@ -39,6 +39,22 @@ properties:
> >>>>        - compatible
> >>>>        - "#clock-cells"
> >>>>
> >>>> +  reset:
> >>>
> >>> I'm not really thrilled how this is evolving with a node per provider.
> >>> There's no reason you can't just add #clock-cells and #reset-cells to
> >>> the parent firmware node.
> >>
> >> What are the downsides? The way I see it there is not much difference. And this
> >> way of handling things is feels more intuitive and flexible (overlays can
> >> control what to enable easily, we can take advantage of the platform device
> >> core).
> >
> > What the OS wants can evolve, so designing around the current needs of
> > the OS is not how bindings should be done.
> >
> > Using overlays to add clocks or resets wouldn't really work given they
> > are spread out over the tree. And with clocks in particular, you'd have
> > to replace dummy fixed clocks with actual firmware clocks. Sounds
> > fragile and messy...
> >
> >>> I probably should have complained with the clocks node, but that's only
> >>> pending for 5.9.
> >>
> >> Note that there are more users for this pattern: "raspberrypi,firmware-ts" and
> >> "raspberrypi,firmware-gpio". Actually you were the one to originally propose
> >> this it[1]. :P
> >
> > Sigh, this is why I dislike incomplete examples...
> >
> > Based on that,
> >
> > Acked-by: Rob Herring <robh@xxxxxxxxxx>
> >
> > And please get gpio and ts converted to schema and referenced here
> > before the next time I look at this.
> >
> >> There already is a fair amount of churn in these drivers because of all the DT
> >> changes we did in the past, and if we need to change how we integrate these
> >> again, I'd really like it to be for good.
> >>
> >>> The bigger issue is this stuff is just trickling in one bit at a time
> >>> which gives no context for review. What's next? Is it really a mystery
> >>> as to what functions the firmware provides?
> >>
> >> We have no control over it, RPi engineers integrate new designs and new
> >> firmware interfaces show up. This is a good example of it.
> >>
> >> I proposed them to use SCMI as it covers most of what they are already
> >> providing here. But no luck so far.
> >
> > Once we get tired of supporting all the different firmware interfaces
> > and the mess they become, we'll just have to start refusing custom ones.
> > Worked for PSCI.
>
> In this particular case, the Raspberry Pi Foundation VPU firmware should
> just implement SCMI and that would avoid having to write new client
> drivers for Linux, it is not clear to me why this has not been done yet.

Writing drivers is fun?

Perhaps we should start refusing new firmware interfaces now.

Rob