Message-ID: <1797126.QkHrqEjB74@senjougahara>
Date: Thu, 28 Aug 2025 17:13:22 +0900
From: Mikko Perttunen <mperttunen@...dia.com>
To: Svyatoslav Ryhel <clamor95@...il.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

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.

> 
> > > >Cheers,
> > > >Mikko

Powered by blists - more mailing lists