Re: [PATCH v3 03/15] soc: tegra: Add Tegra PMC clock registrations into PMC driver

From: Sowjanya Komatineni
Date: Tue Dec 10 2019 - 14:19:38 EST



On 12/10/19 10:30 AM, Dmitry Osipenko wrote:
10.12.2019 20:48, Sowjanya Komatineni ÐÐÑÐÑ:
On 12/10/19 9:41 AM, Dmitry Osipenko wrote:
09.12.2019 23:46, Sowjanya Komatineni ÐÐÑÐÑ:
On 12/9/19 12:12 PM, Dmitry Osipenko wrote:
08.12.2019 00:36, Sowjanya Komatineni ÐÐÑÐÑ:
On 12/7/19 11:59 AM, Sowjanya Komatineni wrote:
On 12/7/19 8:00 AM, Dmitry Osipenko wrote:
07.12.2019 18:53, Dmitry Osipenko ÐÐÑÐÑ:
07.12.2019 18:47, Dmitry Osipenko ÐÐÑÐÑ:
07.12.2019 17:28, Dmitry Osipenko ÐÐÑÐÑ:
06.12.2019 05:48, Sowjanya Komatineni ÐÐÑÐÑ:
Tegra210 and prior Tegra PMC has clk_out_1, clk_out_2, clk_out_3
with
mux and gate for each of these clocks.

Currently these PMC clocks are registered by Tegra clock driver
using
clk_register_mux and clk_register_gate by passing PMC base address
and register offsets and PMC programming for these clocks happens
through direct PMC access by the clock driver.

With this, when PMC is in secure mode any direct PMC access
from the
non-secure world does not go through and these clocks will not be
functional.

This patch adds these clocks registration with PMC as a clock
provider
for these clocks. clk_ops callback implementations for these
clocks
uses tegra_pmc_readl and tegra_pmc_writel which supports PMC
programming
in secure mode and non-secure mode.

Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
---
[snip]

+
+static const struct clk_ops pmc_clk_gate_ops = {
+ÂÂÂ .is_enabled = pmc_clk_is_enabled,
+ÂÂÂ .enable = pmc_clk_enable,
+ÂÂÂ .disable = pmc_clk_disable,
+};
What's the benefit of separating GATE from the MUX?

I think it could be a single clock.
According to TRM:

1. GATE and MUX are separate entities.

2. GATE is the parent of MUX (see PMC's CLK_OUT paths diagram in
TRM).

3. PMC doesn't gate EXTPERIPH clock but could "force-enable" it,
correct?
Was following existing clk-tegra-pmc as I am not sure of reason for
having these clocks registered as separate mux and gate clocks.

Yes, PMC clocks can be registered as single clock and can use clk_ops
for set/get parent and enable/disable.

enable/disable of PMC clocks is for force-enable to force the clock to
run regardless of ACCEPT_REQ or INVERT_REQ.

4. clk_m_div2/4 are internal PMC OSC dividers and thus these clocks
should belong to PMC.
Also, it should be "osc" and not "clk_m".
I followed the same parents as it were in existing clk-tegra-pmc
driver.

Yeah they are wrong and they should be from osc and not clk_m.

Will fix in next version.

Could you please describe the full EXTPERIPH clock topology and how the
pinmux configuration is related to it all?

What is internal to the Tegra chip and what are the external outputs?

Is it possible to bypass PMC on T30+ for the EXTPERIPH clocks?
PMC CLK1/2/3 possible sources are OSC_DIV1, OSC_DIV2, OSC_DIV4,
EXTPERIPH from CAR.

OSC_DIV1/2/4 are with internal dividers at the OSC Pads

EXTPERIPH is from CAR and it has reset and enable controls along with
clock source selections to choose one of the PLLA_OUT0, CLK_S,
PLLP_OUT0, CLK_M, PLLE_OUT0
Are you sure that EXTPERIPH has a reset? What will it reset? Why it's
not documented in TRM?
Yes, Extperiph1/2/3 has RST part of CAR RST_DEVICES_V bits 24/25/26
Are these bits not documented in a public TRMs? I checked
T30/114/124/210 TRMs and CLK_RST_CONTROLLER_RST_DEVICES_V_0 doesn't have
those bits in the docs.

Yeah these bits are missing in all Tegra TRM docs. Will request for having EXTPERIPH reset bits to be updated in TRM...
So, PMC CLK1/2/4 possible parents are OSC_DIV1, OSC_DIV2, OSC_DIV4, EXTERN.


CLK1/2/3 also has Pinmux to route EXTPERIPH output on to these pins.
Could you please clarify what are "these" pins? Perhaps you meant the
EXTERN pin of PMC?
By CLK1/2/3 pins, I am referring to CLK_OUT_1/2/3 pins from Tegra
I see now what you meant, thanks.

[snip}