linux-kernel - Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20220915113801.hexlaer3sp725co5@penduick>
Date:   Thu, 15 Sep 2022 12:38:01 +0100
From:   Maxime Ripard <maxime@...no.tech>
To:     Stefan Wahren <stefan.wahren@...e.com>
Cc:     Stephen Boyd <sboyd@...nel.org>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@...adcom.com>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>, Emma Anholt <emma@...olt.net>,
        Florian Fainelli <f.fainelli@...il.com>,
        Michael Turquette <mturquette@...libre.com>,
        Ray Jui <rjui@...adcom.com>,
        Scott Branden <sbranden@...adcom.com>,
        linux-arm-kernel@...ts.infradead.org,
        linux-rpi-kernel@...ts.infradead.org,
        dri-devel@...ts.freedesktop.org, Dom Cobley <popcornmix@...il.com>,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the
 maximum

Hi Stefan,

On Thu, Sep 15, 2022 at 01:30:02PM +0200, Stefan Wahren wrote:
> Am 15.09.22 um 09:54 schrieb Maxime Ripard:
> > On Wed, Sep 14, 2022 at 08:26:55PM +0200, Stefan Wahren wrote:
> > > Am 14.09.22 um 20:14 schrieb Stephen Boyd:
> > > > Quoting Stefan Wahren (2022-09-14 11:09:04)
> > > > > Am 14.09.22 um 20:05 schrieb Stephen Boyd:
> > > > > > Quoting Stefan Wahren (2022-09-14 10:45:48)
> > > > > > > Am 14.09.22 um 17:50 schrieb Stephen Boyd:
> > > > > > > > Furthermore, I wonder if even that part needs to be implemented.  Why
> > > > > > > > not make a direct call to rpi_firmware_property() and get the max rate?
> > > > > > > > All of that can live in the drm driver. Making it a generic API that
> > > > > > > > takes a 'struct clk' means that it looks like any clk can be passed,
> > > > > > > > when that isn't true. It would be better to restrict it to the one use
> > > > > > > > case so that the scope of the problem doesn't grow. I understand that it
> > > > > > > > duplicates a few lines of code, but that looks like a fair tradeoff vs.
> > > > > > > > exposing an API that can be used for other clks in the future.
> > > > > > > it would be nice to keep all the Rpi specific stuff out of the DRM
> > > > > > > driver, since there more users of it.
> > > > > > Instead of 'all' did you mean 'any'?
> > > > > yes
> > > > Why?
> > > This firmware is written specific for the Raspberry Pi and not stable from
> > > interface point of view. So i'm afraid that the DRM driver is only usable
> > > for the Raspberry Pi at the end with all these board specific dependencies.
> > I'm open for suggestions there, but is there any other bcm2711 device
> > that we support upstream?
>
> I meant the driver as a whole. According to the vc4 binding there are three
> compatibles bcm2835-vc4, cygnus-vc4 and bcm2711-vc5. Unfortunately i don't
> have access to any of these Cygnus boards, so i cannot do any regression
> tests or provide more information to your question.

I don't have access to these boards either, and none of them are
upstream, so I'm not sure what we can do to improve their support by then.

> > If not, I'm not sure what the big deal is at this point. Chances are the
> > DRM driver won't work as is on a different board.
> > 
> > Plus, such a board wouldn't be using config.txt at all, so this whole
> > dance to find what was enabled or not wouldn't be used at all.
>
> My concern is that we reach some point that we need to say this kernel
> version requires this firmware version. In the Raspberry Pi OS world this is
> not a problem, but not all distributions has this specific knowledge.

The recent mess with the Intel GPU firmware
(https://lore.kernel.org/dri-devel/CAPM=9txdca1VnRpp-oNLXpBc2UWq3=ceeim5+Gw4N9tAriRY6A@mail.gmail.com/)
makes it fairly clear that such a situation should be considered a
regression and fixed. So it might be a situation that the downstream
tree will end up in, but it's not something we will allow to happen
upstream.

> > > Emma invested a lot of time to make this open source and now it looks that
> > > like that more and more functionality moves back to firmware.
> > What functionality has been moved back to firmware?
>
> This wasn't a offense against your great work. Just a slight warning that
> some functions of clock or power management moved back into firmware. We
> should watch out, but maybe i emote here.

Yeah, I guess we'll want to consider it on a case per case basis but
it's not like we merged fkms either :)

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)