On 11/04/2024 04:04, Zhi Mao (毛智) wrote: > Hi Conor, > > Thanks for your review. > > On Wed, 2024-04-10 at 12:27 +0100, Conor Dooley wrote: >>> >>> >> Hey, >> >> On Wed, Apr 10, 2024 at 06:40:01PM +0800, Zhi Mao wrote: >>> b/Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml >>> @@ -0,0 +1,91 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +# Copyright (c) 2020 MediaTek Inc. >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/media/i2c/giantec,gt97xx.yaml# >> >> Filename patching compatible please. >> >> > Sorry, I don't catch this point. > Can you explain more details? s/patching/matching/ Use compatible as filename. >>> >>> >>> + >>> + giantec,aac-mode: >>> + description: >>> + Indication of AAC mode select. >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + enum: >>> + - 1 # AAC2 mode(operation time# 0.48 x Tvib) >>> + - 2 # AAC3 mode(operation time# 0.70 x Tvib) >>> + - 3 # AAC4 mode(operation time# 0.75 x Tvib) >>> + - 5 # AAC8 mode(operation time# 1.13 x Tvib) >> >> I dislike these enum based properties and I would rather this either >> be >> the values themselves (0.48, 0.70 etc). >> >>> + >>> + giantec,aac-timing: >>> + description: >>> + Number of AAC Timing count that controlled by one 6-bit >>> period of >>> + vibration register AACT[5:0], the unit of which is 100 us. >> >> Then the property should be in a standard unit of time, not "random" >> hex >> numbers that correspond to register values. >> >>> >>> + giantec,clock-presc: >>> + description: >>> + Indication of VCM internal clock dividing rate select, as >>> one multiple >>> + factor to calculate VCM ring periodic time Tvib. >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + enum: >>> + - 0 # Dividing Rate - 2 >>> + - 1 # Dividing Rate - 1 >>> + - 2 # Dividing Rate - 1/2 >>> + - 3 # Dividing Rate - 1/4 >>> + - 4 # Dividing Rate - 8 >>> + - 5 # Dividing Rate - 4 >> >> Same here, you should not need these comments explaining the values, >> use >> an enum with meaningful values please. >> > About "aac-mode/aac-timing/clock-presc", we test this driver with > default settings accroding to SPEC and VCM works well, so I will not > export these property in YMAL and let driver use default settings. > How do you think about it? You must remove them from the driver code in such case. Best regards, Krzysztof
