Re: [PATCH net-next v1 2/9] dt-bindings: Add Tegra234 MGBE clocks and resets

On 24/06/2022 18:21, Thierry Reding wrote:

On Fri, Jun 24, 2022 at 06:02:58PM +0200, Krzysztof Kozlowski wrote:

quoted

On 23/06/2022 09:46, Bhadram Varka wrote:

quoted

From: Thierry Reding <redacted>

Add the clocks and resets used by the MGBE Ethernet hardware found on
Tegra234 SoCs.

Signed-off-by: Thierry Reding <redacted>
Signed-off-by: Bhadram Varka <redacted>
---
 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