Hi,

On Mon 13 Sep 21, 09:54, Maxime Ripard wrote:

On Fri, Sep 10, 2021 at 08:41:26PM +0200, Paul Kocialkowski wrote:

quoted

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?

quoted

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