[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <hmyn24xd7ov752c5gyrdsocdp22feyole4cnerotnknygv7ujj@oowsdx3z4tmt>
Date: Wed, 9 Jul 2025 17:10:00 +0200
From: Thierry Reding <thierry.reding@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Svyatoslav Ryhel <clamor95@...il.com>,
Jonathan Hunter <jonathanh@...dia.com>, Peter De Schrijver <pdeschrijver@...dia.com>,
Prashant Gaikwad <pgaikwad@...dia.com>, Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
Viresh Kumar <viresh.kumar@...aro.org>, Philipp Zabel <p.zabel@...gutronix.de>,
linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
linux-pm@...r.kernel.org
Subject: Re: [PATCH v1 2/3] drivers: clk: tegra: add DFLL support for Tegra 4
On Wed, Jun 18, 2025 at 11:13:37AM +0200, Krzysztof Kozlowski wrote:
> On 10/06/2025 13:07, Thierry Reding wrote:
> >>
> >>> @@ -0,0 +1,13 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> >>> +/*
> >>> + * This header provides Tegra114-specific constants for binding
> >>> + * nvidia,tegra114-car.
> >>> + */
> >>> +
> >>> +#ifndef _DT_BINDINGS_RESET_TEGRA114_CAR_H
> >>> +#define _DT_BINDINGS_RESET_TEGRA114_CAR_H
> >>> +
> >>> +#define TEGRA114_RESET(x) (5 * 32 + (x))
> >>
> >>
> >> Does not look like a binding, but some sort of register. Binding IDs
> >> start from 0 (or 1) and are incremented by 1.
> >
> > I'll try and clear up some of the confusion around this. The way that
> > resets are handled on these Tegra devices is that there is a set of
> > peripheral clocks & resets which are paired up. This is because they
> > are laid out in banks within the CAR (clock and reset) controller. In
> > most cases we're referring to those resets, so you'll often see a clock
> > ID used in conjection with the same reset ID for a given IP block.
> >
> > In addition to those peripheral resets, there are a number of extra
> > resets that don't have a corresponding clock and which are exposed in
> > registers outside of the peripheral banks, but still part of the CAR.
> > To support those "special" registers, the TEGRA*_RESET() is used to
> > denote resets outside of the regular peripheral resets. Essentially it
> > defines the offset within the CAR at which special resets start. In the
> > above case, Tegra114 has 5 banks with 32 peripheral resets each. The
> > first special reset, TEGRA114_RESET(0), therefore gets ID 5 * 32 + 0.
> >
> > So to summarize: We cannot start enumerating these at 0 because that
> > would fall into the range of peripheral reset IDs.
>
> So these are hardware values, not bindings. Drop the header or move it
> outside of bindings like other headers for hardware constants.
5 banks and 32 peripheral resets per bank are properties of the
hardware, yes. However, the notion of starting the enumeration of the
extra resets after those 160 resets is a binding. There's no concept
in the chip that would tie the DFLL reset to index 160.
Dropping the header altogether would mean that we need to hardcode
the value, which makes its meaning completely opaque. Besides, there are
a bunch of header files in include/dt-bindings that define symbolic
names for hardware values, and I'm not sure why you think these here
would be different.
Thierry
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists