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

From: Stefan Wahren
Date: Thu Sep 15 2022 - 07:30:36 EST


Hi Maxime,

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.
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.

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.

Maxime