Re: [PATCH 2/2] media: hi846: Add 6MP and 8MP modes support

[Sakari Ailus]

On Fri, May 01, 2026 at 12:18:09AM +0800, Pengyu Luo wrote:
> > > @@ -1042,9 +1142,11 @@ static const char * const hi846_test_pattern_menu[] = {
> > >
> > >  #define FREQ_INDEX_640       0
> > >  #define FREQ_INDEX_1280      1
> > > +#define FREQ_INDEX_3264      2
> > >  static const s64 hi846_link_freqs[] = {
> > >       [FREQ_INDEX_640] = 80000000,
> > >       [FREQ_INDEX_1280] = 200000000,
> > > +     [FREQ_INDEX_3264] = 288000000,
> >
> > Looking at the driver, the PLL configuration is present in the lane number
> > specific register list so the link frequency is in fact the same for all
> > modes. This problem isn't introduced by this patch but I think this needs
> > to be fixed before adding further modes to the driver.
> >
> 
> TBH, I didn't investigate the calculations. I just searched the
> datasheet for it. They are exactly wrong. Thans for pointing out this.
> 
> I can fix it, but the question is this driver was writing against
> 25Mhz mclk, but the typical value from datasheet is 24Mhz, and one
> device is using 25Mhz as the clock rate, my device is using 24Mhz,
> 25Mhz is unsupported on my platform. It seems that making the list not
> be runtime is not allowed as nobody did like this.

What do you mean?

PLL configuration calculated from platform and runtime configuration is
definitely preferred over hard-coded configuration, albeit much harder to
implement. It also helps to prevent problems such as the one above.

-- 
Sakari Ailus