On 2/16/23 6:17 AM, Krzysztof Kozlowski wrote: > On 14/02/2023 12:55, Dipen Patel wrote: >> Added timestamp provider support for the Tegra234 in devicetree >> bindings. > > 1. Your commit does much more. You need to explain it why you drop some > property. ACK, will address it next patch > > 2. Bindings go before its usage (in the patchset). Ack... > > 3. Please use scripts/get_maintainers.pl to get a list of necessary > people and lists to CC. It might happen, that command when run on an > older kernel, gives you outdated entries. Therefore please be sure you > base your patches on recent Linux kernel. It is based on recent linux at the time patch series was sent... > > >> >> Signed-off-by: Dipen Patel <dipenp@xxxxxxxxxx> >> --- >> v2: >> - Removed nvidia,slices property >> - Added nvidia,gpio-controller based on review comments from Thierry, >> this will help simplify the hte provider driver. >> >> .../timestamp/nvidia,tegra194-hte.yaml | 30 ++++++++++++------- >> 1 file changed, 19 insertions(+), 11 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml b/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml >> index c31e207d1652..d0f4ed75baee 100644 >> --- a/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml >> +++ b/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml >> @@ -4,7 +4,7 @@ >> $id: http://devicetree.org/schemas/timestamp/nvidia,tegra194-hte.yaml# >> $schema: http://devicetree.org/meta-schemas/core.yaml# >> >> -title: Tegra194 on chip generic hardware timestamping engine (HTE) >> +title: Tegra on chip generic hardware timestamping engine (HTE) provider >> >> maintainers: >> - Dipen Patel <dipenp@xxxxxxxxxx> >> @@ -23,6 +23,8 @@ properties: >> enum: >> - nvidia,tegra194-gte-aon >> - nvidia,tegra194-gte-lic >> + - nvidia,tegra234-gte-aon >> + - nvidia,tegra234-gte-lic >> >> reg: >> maxItems: 1 >> @@ -38,14 +40,11 @@ properties: >> minimum: 1 >> maximum: 256 >> >> - nvidia,slices: >> - $ref: /schemas/types.yaml#/definitions/uint32 >> + nvidia,gpio-controller: >> + $ref: /schemas/types.yaml#/definitions/phandle >> description: >> - HTE lines are arranged in 32 bit slice where each bit represents different >> - line/signal that it can enable/configure for the timestamp. It is u32 >> - property and depends on the HTE instance in the chip. The value 3 is for >> - GPIO GTE and 11 for IRQ GTE. >> - enum: [3, 11] >> + The phandle to AON gpio controller instance. This is required to handle >> + namespace conversion between GPIO and GTE. >> >> '#timestamp-cells': >> description: >> @@ -55,11 +54,21 @@ properties: >> mentioned in the nvidia GPIO device tree binding document. >> const: 1 >> >> +if: > > Keep it under allOf (so you no need to re-indent it on next if statement > in the future) and put entire allOf after "required:". > Ack... >> + properties: >> + compatible: >> + contains: >> + enum: >> + - nvidia,tegra194-gte-aon > > This is an ABI break. Does your driver handle it? yes, handling patch is part of this patch series. > >> + - nvidia,tegra234-gte-aon >> +then: >> + required: >> + - nvidia,gpio-controller >> + >> required: >> - compatible >> - reg >> - interrupts >> - - nvidia,slices >> - "#timestamp-cells" > > > Best regards, > Krzysztof >