Re: [PATCH 01/22] clk: sunxi-ng: v3s: Make the ISP PLL clock public

From: Paul Kocialkowski
Date: Mon Sep 13 2021 - 04:54:00 EST


Hi,

On Mon 13 Sep 21, 09:54, Maxime Ripard wrote:
> On Fri, Sep 10, 2021 at 08:41:26PM +0200, Paul Kocialkowski wrote:
> > In order to reparent the CSI module clock to the ISP PLL via
> > device-tree, export the ISP PLL clock declaration in the public
> > device-tree header.
>
> You use clk_set_rate_exclusive in the ISP driver on the module clock so
> it should prevent what you're mentioning from happening.

It does, but then it breaks display support entirely (because the DRM
driver doesn't use clk_set_rate_exclusive).

The bottomline is that using the same PLL for both display and camera
easily results in conflicts.

> If it doesn't, then clk_set_rate_exclusive has a bug and should be
> fixed.
>
> Either way, using assigned-clock-parents is not a good solution here
> either, it only makes sure that this is the case when probe is run.

I'm not sure what could provide better guarantees. There is a clock
parenting API (in the clock framework) which may, but this implies
providing the parent clock to the driver which seems way out of line
since this is a platform-specific matter that should certainly not
be handled by the driver.

I also tried hardcoding the reparenting bit in the CCU driver, but
this felt less clean than doing it in device-tree.

What do you think?

> > Details regarding why the CSI module clock is best parented to the ISP
> > PLL are provided in the related commit.
>
> This is relevant to this commit too and "the related commit" is far too
> blurry when you consider the entire Linux git history.

Fair enough!

Cheers,

Paul

--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Attachment: signature.asc
Description: PGP signature