Re: [PATCH v1 01/19] clk: tegra: init CSUS clock for Tegra20 and Tegra30

[Mikko Perttunen <mperttunen@nvidia.com>]

On Thursday, August 28, 2025 7:23 PM Svyatoslav Ryhel wrote:
> чт, 28 серп. 2025 р. о 13:15 Mikko Perttunen <mperttunen@nvidia.com> пише:
> > On Thursday, August 28, 2025 5:28 PM Svyatoslav Ryhel wrote:
> > > чт, 28 серп. 2025 р. о 11:13 Mikko Perttunen <mperttunen@nvidia.com> 
пише:
> > > > On Wednesday, August 27, 2025 7:45 PM Svyatoslav Ryhel wrote:
> > > > > ср, 27 серп. 2025 р. о 13:36 Mikko Perttunen <mperttunen@nvidia.com>
> > 
> > пише:
> > > > > > On Wednesday, August 27, 2025 1:32 PM Svyatoslav wrote:
> > > > > > > 27 серпня 2025 р. 07:09:45 GMT+03:00, Mikko Perttunen
> > > > > > 
> > > > > > <mperttunen@nvidia.com> пише:
> > > > > > > >On Tuesday, August 19, 2025 9:16 PM Svyatoslav Ryhel wrote:
> > > > > > > >> CSUS clock is required to be enabled on camera device
> > > > > > > >> configuration
> > > > > > > >> or
> > > > > > > >> else camera module refuses to initiate properly.
> > > > > > > >> 
> > > > > > > >> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > > > > >> ---
> > > > > > > >> 
> > > > > > > >>  drivers/clk/tegra/clk-tegra20.c | 1 +
> > > > > > > >>  drivers/clk/tegra/clk-tegra30.c | 1 +
> > > > > > > >>  2 files changed, 2 insertions(+)
> > > > > > > >> 
> > > > > > > >> diff --git a/drivers/clk/tegra/clk-tegra20.c
> > > > > > > >> b/drivers/clk/tegra/clk-tegra20.c index
> > > > > > > >> 551ef0cf0c9a..42f8150c6110
> > > > > > > >> 100644
> > > > > > > >> --- a/drivers/clk/tegra/clk-tegra20.c
> > > > > > > >> +++ b/drivers/clk/tegra/clk-tegra20.c
> > > > > > > >> @@ -1043,6 +1043,7 @@ static struct tegra_clk_init_table
> > > > > > > >> init_table[]
> > > > > > > >> = {
> > > > > > > >> 
> > > > > > > >>    { TEGRA20_CLK_GR3D, TEGRA20_CLK_PLL_C, 300000000, 0 },
> > > > > > > >>    { TEGRA20_CLK_VDE, TEGRA20_CLK_PLL_C, 300000000, 0 },
> > > > > > > >>    { TEGRA20_CLK_PWM, TEGRA20_CLK_PLL_P, 48000000, 0 },
> > > > > > > >> 
> > > > > > > >> +  { TEGRA20_CLK_CSUS, TEGRA20_CLK_CLK_MAX, 6000000, 1 },
> > > > > > > >> 
> > > > > > > >>    /* must be the last entry */
> > > > > > > >>    { TEGRA20_CLK_CLK_MAX, TEGRA20_CLK_CLK_MAX, 0, 0 },
> > > > > > > >>  
> > > > > > > >>  };
> > > > > > > >> 
> > > > > > > >> diff --git a/drivers/clk/tegra/clk-tegra30.c
> > > > > > > >> b/drivers/clk/tegra/clk-tegra30.c index
> > > > > > > >> 82a8cb9545eb..70e85e2949e0
> > > > > > > >> 100644
> > > > > > > >> --- a/drivers/clk/tegra/clk-tegra30.c
> > > > > > > >> +++ b/drivers/clk/tegra/clk-tegra30.c
> > > > > > > >> @@ -1237,6 +1237,7 @@ static struct tegra_clk_init_table
> > > > > > > >> init_table[]
> > > > > > > >> = {
> > > > > > > >> 
> > > > > > > >>    { TEGRA30_CLK_HDA, TEGRA30_CLK_PLL_P, 102000000, 0 },
> > > > > > > >>    { TEGRA30_CLK_HDA2CODEC_2X, TEGRA30_CLK_PLL_P, 48000000, 0
> > > > > > > >>    },
> > > > > > > >>    { TEGRA30_CLK_PWM, TEGRA30_CLK_PLL_P, 48000000, 0 },
> > > > > > > >> 
> > > > > > > >> +  { TEGRA30_CLK_CSUS, TEGRA30_CLK_CLK_MAX, 6000000, 1 },
> > > > > > > >> 
> > > > > > > >>    /* must be the last entry */
> > > > > > > >>    { TEGRA30_CLK_CLK_MAX, TEGRA30_CLK_CLK_MAX, 0, 0 },
> > > > > > > >>  
> > > > > > > >>  };
> > > > > > > >
> > > > > > > >I looked into what this clock does and it seems to be a gate
> > > > > > > >for
> > > > > > > >the
> > > > > > > >CSUS
> > > > > > > >pin, which provides an output clock for camera sensors (VI
> > > > > > > >MCLK).
> > > > > > > >Default
> > > > > > > >source seems to be PLLC_OUT1. It would be good to note that on
> > > > > > > >the
> > > > > > > >commit
> > > > > > > >message, as I can't find any documentation about the CSUS clock
> > > > > > > >elsewhere.
> > > > > > > >
> > > > > > > >What is the 6MHz rate based on?
> > > > > > > 
> > > > > > > 6mhz is the statistic value which I was not able to alter while
> > > > > > > testing.
> > > > > > > I
> > > > > > > have tried 12mhz and 24mhz too but it remained 6mhz, so I left
> > > > > > > it
> > > > > > > 6mhz.
> > > > > > > 
> > > > > > > >Since this seems to be a clock consumed by the sensor, it seems
> > > > > > > >to
> > > > > > > >me
> > > > > > > >that
> > > > > > > >rather than making it always on, we could point to it in the
> > > > > > > >sensor's
> > > > > > > >device tree entry.
> > > > > > > 
> > > > > > > Sensor device tree uses vi_sensor as clocks source and sensor
> > > > > > > drivers
> > > > > > > don't
> > > > > > > support multiple linked clocks.
> > > > > > 
> > > > > > AIUI vi_sensor is an internal clock so the sensor cannot be
> > > > > > receiving
> > > > > > it
> > > > > > directly. Perhaps the sensor is actually connected to csus, and
> > > > > > the
> > > > > > reason
> > > > > > we need to enable it is to allow the vi_sensor clock to pass
> > > > > > through
> > > > > > the
> > > > > > csus gate?
> > > > > > 
> > > > > > That leaves the question of why the csus pad would be muxed to
> > > > > > vi_sensor
> > > > > > by
> > > > > > default, but perhaps there's an explanation for that.
> > > > > 
> > > > > From downstream T30 sources csus and vi_sensor are always called in
> > > > > pair (6MHz csus and 24MHz for vi_sensor), naturally I assumed that
> > > > > latter is used as camera reference clock since most sensors has
> > > > > reference clock around 24 MHz
> > > > 
> > > > It's possible that the csus pad is still outputting 24MHz. The pinmux
> > > > options for the csus pad are various clocks, so it would seem logical
> > > > that the clock source for the pad is one of those clocks. However, on
> > > > the
> > > > clock framework side, the csus clock is just a gate. What I'm confused
> > > > about is that since on the clock framework side the parent of csus is
> > > > currently set to clk_m, I don't know why setting the rate of csus
> > > > would
> > > > affect the output of the pad, given clk_m is not one of the options
> > > > for
> > > > the pinmux.
> > > > 
> > > > It's be good to verify the register value for the csus pinmux to see
> > > > where
> > > > it thinks the clock is coming from, and then check how that matches
> > > > with
> > > > what we are seeing.
> > > 
> > > TRM does not provide such data, it has only register address with
> > > layout for it as a plain pad control, that register has only DRVDN,
> > > DRVUP, SLWR and SLWF and I don't see a way to decode clock value or
> > > parent or anything similar. If you give me a method I will calculate
> > > those values.
> > 
> > I notice that on Tegra20, there is a mux pingroup called 'csus', which has
> > the mux options PLLC_OUT1, PLLP_OUT2, PLLP_OUT3, and VI_SENSOR_CLK (based
> > on upstream pinctrl-tegra20.c). The TRM also says 'Enable clock to SUS
> > pad.' about the CSUS (or SUS) clock.
> > 
> > On Tegra30, however, which I guess you refer to, I guess mux pingroups are
> > gone and each pin has its own mux (again looking at upstream pinctrl-
> > tegra30.c). vi_mclk_pt1 is now its own mux with the options VI, VI_ALT1,
> > VI_ALT2, VI_ALT3. The drive group for this pin is still called csus, so by
> > that name it only has the drive settings as you mention.
> > 
> > Are you testing on Tegra20, Tegra30, or both?
> 
> I am testing on Tegra30 since I did not have compatible Tegra20 device
> (with supported camera).
> 
> > I've looked at some Tegra30 schematics, and they show a signal called
> > VI_MCLK being routed to CSI cameras.
> > 
> > > Another theory is that maybe csus is used for VIP cameras only and
> > > vi_sensor is used for CSI cameras, but they both have to be on in
> > > order to work correctly. Csus was removed from Tegra114 along with
> > > VIP, might not be a coincidence. Moreover, T124 uses vi_sensor as
> > > camera mclk source.
> > 
> > I see the CSUS clock still on Tegra124 based on the upstream kernel. There
> > is also a CAM_MCLK pin. It seems Tegra30 has both VI_MCLK and CAM_MCLK
> > pins, which both can output the clock. After Tegra30 there is only
> > CAM_MCLK.
> > 
> > Looking at L4T r21, in tegra12_clocks.c, it defines the clocks mclk and
> > mclk2.> 
> > There is a comment on mclk saying:
> >                        .clk_num = 92, /* csus */
> > 
> > whereas mclk2 is vim2_clk. These clocks are indeed defined as gates, with
> > vi_sensor / vi_sensor2 as parent, set_rate being passed onto the parent.
> > 
> > All of that wasn't very coherently written, but to summarize my thoughts:
> > 
> > On Tegra30, we have
> > - Pins vi_mclk and cam_mclk. Both can only source from (vi_)mclk which
> > also
> > goes by name csus. The mclk/csus clock is a clock gate with vi_sensor as
> > parent.
> > On Tegra114 and later,
> > - Same situation, but vi_mclk is gone, so instead we have cam_mclk
> > (possibly multiple with associated mclkN and vi_sensorN clocks)
> > On Tegra20,
> > - The vi_mclk pin has a variety of mux options, one of which is
> > VI_SENSOR_CLK. I expect this to correspond to the same behavior as later
> > chips, i.e. sources from the csus(/mclk) clock, which sources from
> > vi_sensor.
> 
> While this is all quite interesting, how to configure this properly?

Fix the csus clock's parent to be vi_sensor. Point the sensor's device tree 
clock entry to csus. The sensor's clk_enable should then ungate csus and 
clk_set_rate should flow to vi_sensor to set the rate appropriately. In the 
board device tree pinctrl section, set the vi_mclk pin's function to VI 
(should be default on Tegra30, but best to be explicit).

I think that should do it, but it's all theoretical of course :)

> 
> > > Here is a fragment of Tegra124 clock tree (dumped from Mi pad 1)
> > > 
> > >          pll_p                                 on     13  x34     
> > >          408000000
> > > 
> > > vi_sensor2                       $ off    0   3.0      136000000 mclk2
> > > 
> > >                    $ off    0            136000000 vi_sensor
> > >         
> > >         $ off    0   3.0      136000000 mclk                          $
> > >         off
> > >    
> > >    0            136000000
> > >    
> > > > > > > >Cheers,
> > > > > > > >Mikko