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? > > > > > > + > > + 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? > Thanks, > Conor. > > > > >
