Message-ID: <CAPVz0n3AvQaFrpeyUODpqOwkxxinjWgMQTgqvD4hAZvdqprVdA@mail.gmail.com>
Date: Thu, 28 Aug 2025 13:23:04 +0300
From: Svyatoslav Ryhel <clamor95@...il.com>
To: Mikko Perttunen <mperttunen@...dia.com>
Cc: Thierry Reding <thierry.reding@...il.com>, Thierry Reding <treding@...dia.com>, 
	Jonathan Hunter <jonathanh@...dia.com>, Sowjanya Komatineni <skomatineni@...dia.com>, 
	Luca Ceresoli <luca.ceresoli@...tlin.com>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Peter De Schrijver <pdeschrijver@...dia.com>, Prashant Gaikwad <pgaikwad@...dia.com>, 
	Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, 
	Mauro Carvalho Chehab <mchehab@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Dmitry Osipenko <digetx@...il.com>, Charan Pedumuru <charan.pedumuru@...il.com>, 
	linux-media@...r.kernel.org, linux-tegra@...r.kernel.org, 
	dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org, 
	linux-staging@...ts.linux.dev
Subject: Re: [PATCH v1 01/19] clk: tegra: init CSUS clock for Tegra20 and Tegra30

чт, 28 серп. 2025 р. о 13:15 Mikko Perttunen <mperttunen@...dia.com> пише:
>
> On Thursday, August 28, 2025 5:28 PM Svyatoslav Ryhel wrote:
> > чт, 28 серп. 2025 р. о 11:13 Mikko Perttunen <mperttunen@...dia.com> пише:
> > > On Wednesday, August 27, 2025 7:45 PM Svyatoslav Ryhel wrote:
> > > > ср, 27 серп. 2025 р. о 13:36 Mikko Perttunen <mperttunen@...dia.com>
> пише:
> > > > > On Wednesday, August 27, 2025 1:32 PM Svyatoslav wrote:
> > > > > > 27 серпня 2025 р. 07:09:45 GMT+03:00, Mikko Perttunen
> > > > >
> > > > > <mperttunen@...dia.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@...il.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?

> >
> > 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
>
>
>
>

Powered by blists - more mailing lists