On 24/06/2022 18:21, Thierry Reding wrote: > On Fri, Jun 24, 2022 at 06:02:58PM +0200, Krzysztof Kozlowski wrote: >> On 23/06/2022 09:46, Bhadram Varka wrote: >>> From: Thierry Reding <treding@xxxxxxxxxx> >>> >>> Add the clocks and resets used by the MGBE Ethernet hardware found on >>> Tegra234 SoCs. >>> >>> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx> >>> Signed-off-by: Bhadram Varka <vbhadram@xxxxxxxxxx> >>> --- >>> include/dt-bindings/clock/tegra234-clock.h | 101 +++++++++++++++++++++ >>> include/dt-bindings/reset/tegra234-reset.h | 8 ++ >>> 2 files changed, 109 insertions(+) >>> >>> diff --git a/include/dt-bindings/clock/tegra234-clock.h b/include/dt-bindings/clock/tegra234-clock.h >>> index bd4c3086a2da..bab85d9ba8cd 100644 >>> --- a/include/dt-bindings/clock/tegra234-clock.h >>> +++ b/include/dt-bindings/clock/tegra234-clock.h >>> @@ -164,10 +164,111 @@ >>> #define TEGRA234_CLK_PEX1_C5_CORE 225U >>> /** @brief PLL controlled by CLK_RST_CONTROLLER_PLLC4_BASE */ >>> #define TEGRA234_CLK_PLLC4 237U >>> +/** @brief RX clock recovered from MGBE0 lane input */ >> >> The IDs should be abstract integer incremented by one, without any >> holes. I guess the issue was here before, so it's fine but I'll start >> complaining at some point :) > > These IDs originate from firmware and therefore are more like hardware > IDs rather than an arbitrary enumeration. These will be used directly in > IPC calls with the firmware to reference individual clocks and resets. If they are actually shared with firmware, it's fine. Thanks for explanation. > We've adopted these 1:1 in order to avoid adding an extra level of > indirection (via some lookup table) in the kernel. This if fine, but some folks (including myself once...) define in bindings register values and offsets without any actual need. I was afraid that's the case here. Best regards, Krzysztof