Hi Chris,
> -----Original Message-----
> From: Chris Brandt <Chris.Brandt@xxxxxxxxxxx>
> Sent: 10 July 2025 17:53
> Subject: RE: [PATCH 1/2] clk: renesas: rzg2l: Remove DSI clock rate restrictions
>
> Hi Biju,
>
> Thank you for your review!
>
>
> > > +/* Required division ratio for the MIPI clock */ int dsi_div_ab;
> >
> > static int dsi_div_ab;
>
> Good catch.
>
>
> > for the DPI, DIV_DSI_B = 1 and DIV_DSI_A ={2, 4, 8}
> >
> > So, you need to adjust the below calculation for DPI as well??
>
> You bring up a good point.
>
> And looking at the hardware manual again, there are other restrictions when using FOUTPOSTDIV (straight
> PLL) compared to FOUT1PH0 (PLL/2).
>
> From a chip design standpoint, they just expect to have 'one big driver that configures everything at
> once'.
>
> > Not sure do we need DSI driver registering a callback with CPG driver and CPG driver uses the
> callback to get DSI divider value and this callback can be used to distinguish DPI from DSI??
>
> Ya, you can't just tell the CPG driver to 'give me this rate'. There is so much other information that
> it needs to have before it can set up the registers.
> Hmm....
>
>
> > > +found_clk:
> > > + if (!found) {
> >
> > Can we add a dev_dbg statement here for !found clock?
>
> Yes, good idea.
> That was in the original driver before I started pulling out all the printk statements.
>
>
> > > + /* If foutvco is above 1.5GHz, change parent and recalculate */
> > > + if (priv->mux_dsi_div_params.clksrc && (foutvco_rate >
> > > +1500000000)) {
> >
> > Check patch is complaining:
> >
> > CHECK: Unnecessary parentheses around 'foutvco_rate > 1500000000'
> > #146: FILE: drivers/clk/renesas/rzg2l-cpg.c:648:
> > + if (priv->mux_dsi_div_params.clksrc && (foutvco_rate > 1500000000))
> > +{
>
> I saw that...but I thought the ( ) makes it a little easier to read.
>
> But, what's the general rule here? Make checkpatch come out perfect?
> What's your thoughts?
I just ran check patch and it complained this.
I am leaving Geert to comment on this.
Cheers,
Biju
