Message ID | 20240410104002.1197-2-zhi.mao@mediatek.com (mailing list archive) |
---|---|
State | Superseded |
Headers |
Received: from ny.mirrors.kernel.org ([147.75.199.223]) by linuxtv.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from <linux-media+bounces-8995-patchwork=linuxtv.org@vger.kernel.org>) id 1ruVNn-0004po-2f for patchwork@linuxtv.org; Wed, 10 Apr 2024 10:40:48 +0000 Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id A2C0E1C22727 for <patchwork@linuxtv.org>; Wed, 10 Apr 2024 10:40:46 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 03D74159560; Wed, 10 Apr 2024 10:40:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=mediatek.com header.i=@mediatek.com header.b="S+C085kn" X-Original-To: linux-media@vger.kernel.org Received: from mailgw02.mediatek.com (unknown [210.61.82.184]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 712DC1552EF; Wed, 10 Apr 2024 10:40:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=210.61.82.184 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712745632; cv=none; b=fbTVQc6dPuinO6d8jb0Pe9fLrgvmMfXdWYK3eI+7Ziz5+IRnx8N0FAFd4HsHmp8Vd8CJWzJgIMgXr+t8glY2X2jo4L7bM1vM6lAKwHJAtp2AREEEOXwitWw1zWUSn7+IXSdzrCe3vbe1zdD9ONmLmoL5TcCOIrEWFMjsBmOyF0Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712745632; c=relaxed/simple; bh=nmlI+1jg5aL2ruwfW6qmSVbZjwUzX5Rn62wwnZgu9RY=; h=From:To:CC:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=gxxNMpPaL9NQH67P9slk8IuddW+vfWBocXMR5+tinp4BD45FaJiozI0SSGViwGXKfXqHgpLp6vvYVXXYcEoVKIR2OcO8RssaVXIQ1KkVhnkYImHaQNHSql50/8LFOsUSVJP9JUdNDHXd6Y4nSch3cQwTDId5IVUfaT4Itta36Ww= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=mediatek.com; spf=pass smtp.mailfrom=mediatek.com; dkim=pass (1024-bit key) header.d=mediatek.com header.i=@mediatek.com header.b=S+C085kn; arc=none smtp.client-ip=210.61.82.184 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=mediatek.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=mediatek.com X-UUID: bdead744f72611ee935d6952f98a51a9-20240410 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=Content-Type:Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-ID:Date:Subject:CC:To:From; bh=IbvyZleKeBzCkSQwcR5/JyUse6gQVlPZjhR9xVr78M0=; b=S+C085kn1yqfCj88x8QZUlirtUkES7TSYul1woxoBAYbx5v3OB9X2ru6sP8ZkKHJ0DPS/rvaHOqwH85SNlpaPoM/GEQF/nrAMkJfVh9RlcUo/QjZvgD2Hd0E/3tzjWYDcghA8DAliDny+UkA1KpH3lzcTtRQiyVrH+Qt1cwfbZE=; X-CID-P-RULE: Release_Ham X-CID-O-INFO: VERSION:1.1.37,REQID:499b88e8-4a3e-41cb-87d6-81c5e86dd057,IP:0,U RL:25,TC:0,Content:0,EDM:0,RT:0,SF:0,FILE:0,BULK:0,RULE:Release_Ham,ACTION :release,TS:25 X-CID-META: VersionHash:6f543d0,CLOUDID:4f960386-8d4f-477b-89d2-1e3bdbef96d1,B ulkID:nil,BulkQuantity:0,Recheck:0,SF:102,TC:nil,Content:0,EDM:-3,IP:nil,U RL:11|1,File:nil,RT:nil,Bulk:nil,QS:nil,BEC:nil,COL:0,OSI:0,OSA:0,AV:0,LES :1,SPR:NO,DKR:0,DKP:0,BRR:0,BRE:0 X-CID-BVR: 0 X-CID-BAS: 0,_,0,_ X-CID-FACTOR: TF_CID_SPAM_SNR,TF_CID_SPAM_ULN X-UUID: bdead744f72611ee935d6952f98a51a9-20240410 Received: from mtkmbs13n1.mediatek.inc [(172.21.101.193)] by mailgw02.mediatek.com (envelope-from <zhi.mao@mediatek.com>) (Generic MTA with TLSv1.2 ECDHE-RSA-AES256-GCM-SHA384 256/256) with ESMTP id 1889807943; Wed, 10 Apr 2024 18:40:26 +0800 Received: from mtkmbs11n2.mediatek.inc (172.21.101.187) by mtkmbs11n1.mediatek.inc (172.21.101.185) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.26; Wed, 10 Apr 2024 18:40:25 +0800 Received: from mhfsdcap04.gcn.mediatek.inc (10.17.3.154) by mtkmbs11n2.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.2.1118.26 via Frontend Transport; Wed, 10 Apr 2024 18:40:23 +0800 From: Zhi Mao <zhi.mao@mediatek.com> To: Mauro Carvalho Chehab <mchehab@kernel.org>, Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>, Conor Dooley <conor+dt@kernel.org> CC: Matthias Brugger <matthias.bgg@gmail.com>, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, Zhi Mao <zhi.mao@mediatek.com>, Philipp Zabel <p.zabel@pengutronix.de>, Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>, Heiko Stuebner <heiko@sntech.de>, Sakari Ailus <sakari.ailus@linux.intel.com>, Hans Verkuil <hverkuil-cisco@xs4all.nl>, Hans de Goede <hdegoede@redhat.com>, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>, Alain Volmat <alain.volmat@foss.st.com>, Paul Elder <paul.elder@ideasonboard.com>, Mehdi Djait <mehdi.djait@bootlin.com>, Andy Shevchenko <andy.shevchenko@gmail.com>, Bingbu Cao <bingbu.cao@intel.com>, <linux-media@vger.kernel.org>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-mediatek@lists.infradead.org>, <shengnan.wang@mediatek.com>, <yaya.chang@mediatek.com>, <yunkec@chromium.org>, <10572168@qq.com> Subject: [PATCH 1/2] media: dt-bindings: i2c: add Giantec GT97xx VCM driver Date: Wed, 10 Apr 2024 18:40:01 +0800 Message-ID: <20240410104002.1197-2-zhi.mao@mediatek.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240410104002.1197-1-zhi.mao@mediatek.com> References: <20240410104002.1197-1-zhi.mao@mediatek.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: <linux-media.vger.kernel.org> List-Subscribe: <mailto:linux-media+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-media+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-MTK: N X-LSpam-Score: -3.3 (---) X-LSpam-Report: No, score=-3.3 required=5.0 tests=ARC_SIGNED=0.001,ARC_VALID=-0.1,BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,DMARC_PASS=-0.001,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1,RCVD_IN_DNSWL_LOW=-0.7,SPF_HELO_NONE=0.001,SPF_PASS=-0.001,UNPARSEABLE_RELAY=0.001 autolearn=ham autolearn_force=no |
Series |
media: i2c: Add support for GT97xx VCM
|
|
Commit Message
Zhi Mao (毛智)
April 10, 2024, 10:40 a.m. UTC
Add YAML device tree binding for GT97xx VCM driver,
and the relevant MAINTAINERS entries.
Signed-off-by: Zhi Mao <zhi.mao@mediatek.com>
---
.../bindings/media/i2c/giantec,gt97xx.yaml | 91 +++++++++++++++++++
1 file changed, 91 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml
Comments
Hey, On Wed, Apr 10, 2024 at 06:40:01PM +0800, Zhi Mao wrote: > Add YAML device tree binding for GT97xx VCM driver, Please don't mention drivers here, bindings are for hardware. > and the relevant MAINTAINERS entries. > > Signed-off-by: Zhi Mao <zhi.mao@mediatek.com> > --- > .../bindings/media/i2c/giantec,gt97xx.yaml | 91 +++++++++++++++++++ > 1 file changed, 91 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml > > diff --git a/Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml b/Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml > new file mode 100644 > index 000000000000..8c9f1eb4dac8 > --- /dev/null > +++ 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. > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Giantec Semiconductor, Crop. GT97xx Voice Coil Motor (VCM) > + > +maintainers: > + - Zhi Mao <zhi.mao@mediatek.com> > + > +description: |- > + The Giantec GT97xx is a 10-bit DAC with current sink capability. > + The DAC is controlled via I2C bus that operates at clock rates up to 1MHz. > + This chip integrates Advanced Actuator Control (AAC) technology > + and is intended for driving voice coil lens in camera modules. > + > +properties: > + compatible: > + enum: > + - giantec,gt9768 # for GT9768 VCM > + - giantec,gt9769 # for GT9769 VCM I don't think these comments are needed, they should be clear from the compatibles, no? > + > + reg: > + maxItems: 1 > + > + vin-supply: true > + > + vdd-supply: true > + > + 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). > + default: 2 > + > + 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. > + $ref: /schemas/types.yaml#/definitions/uint32 > + default: 0x20 > + minimum: 0x00 > + maximum: 0x3f > + > + 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. > + default: 1 > + > +required: > + - compatible > + - reg > + - vin-supply > + - vdd-supply > + > +additionalProperties: false > + > +examples: > + - | > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + vcm@c { "vcm" is not a generic node-name, can you use one please? Look around similar bindings or at the dt spec for generic node-names. Thanks, Conor. > + compatible = "giantec,gt9768"; > + reg = <0x0c>; > + > + vin-supply = <>97xx_vin>; > + vdd-supply = <>97xx_vdd>; > + giantec,aac-timing = <0x20>; > + }; > + }; > + > +... > -- > 2.25.1 >
Hi Conor, Zhi, On Wed, Apr 10, 2024 at 12:27:07PM +0100, Conor Dooley wrote: > Hey, > > On Wed, Apr 10, 2024 at 06:40:01PM +0800, Zhi Mao wrote: > > Add YAML device tree binding for GT97xx VCM driver, > > Please don't mention drivers here, bindings are for hardware. > > > and the relevant MAINTAINERS entries. > > > > Signed-off-by: Zhi Mao <zhi.mao@mediatek.com> > > --- > > .../bindings/media/i2c/giantec,gt97xx.yaml | 91 +++++++++++++++++++ > > 1 file changed, 91 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml b/Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml > > new file mode 100644 > > index 000000000000..8c9f1eb4dac8 > > --- /dev/null > > +++ 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. > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Giantec Semiconductor, Crop. GT97xx Voice Coil Motor (VCM) > > + > > +maintainers: > > + - Zhi Mao <zhi.mao@mediatek.com> > > + > > +description: |- > > + The Giantec GT97xx is a 10-bit DAC with current sink capability. > > + The DAC is controlled via I2C bus that operates at clock rates up to 1MHz. > > + This chip integrates Advanced Actuator Control (AAC) technology > > + and is intended for driving voice coil lens in camera modules. > > + > > +properties: > > + compatible: > > + enum: > > + - giantec,gt9768 # for GT9768 VCM > > + - giantec,gt9769 # for GT9769 VCM > > I don't think these comments are needed, they should be clear from the > compatibles, no? > > > + > > + reg: > > + maxItems: 1 > > + > > + vin-supply: true > > + > > + vdd-supply: true > > + > > + 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). > > > + default: 2 > > + > > + 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. > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + default: 0x20 > > + minimum: 0x00 > > + maximum: 0x3f > > + > > + 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. > > > + default: 1 > > + > > +required: > > + - compatible > > + - reg > > + - vin-supply > > + - vdd-supply > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + > > + i2c { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + vcm@c { > > "vcm" is not a generic node-name, can you use one please? > Look around similar bindings or at the dt spec for generic node-names. "camera-lens" would seem to be widely used. I can post patches to change some of the rest out there that aren't aligned. > > Thanks, > Conor. > > > + compatible = "giantec,gt9768"; > > + reg = <0x0c>; > > + > > + vin-supply = <>97xx_vin>; > > + vdd-supply = <>97xx_vdd>; > > + giantec,aac-timing = <0x20>; > > + }; > > + }; > > + > > +...
On Wed, Apr 10, 2024 at 12:27:07PM +0100, Conor Dooley wrote: > Hey, > > On Wed, Apr 10, 2024 at 06:40:01PM +0800, Zhi Mao wrote: > > Add YAML device tree binding for GT97xx VCM driver, > > Please don't mention drivers here, bindings are for hardware. > > > and the relevant MAINTAINERS entries. > > > > Signed-off-by: Zhi Mao <zhi.mao@mediatek.com> > > --- > > .../bindings/media/i2c/giantec,gt97xx.yaml | 91 +++++++++++++++++++ > > 1 file changed, 91 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml b/Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml > > new file mode 100644 > > index 000000000000..8c9f1eb4dac8 > > --- /dev/null > > +++ 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. > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Giantec Semiconductor, Crop. GT97xx Voice Coil Motor (VCM) > > + > > +maintainers: > > + - Zhi Mao <zhi.mao@mediatek.com> > > + > > +description: |- > > + The Giantec GT97xx is a 10-bit DAC with current sink capability. > > + The DAC is controlled via I2C bus that operates at clock rates up to 1MHz. > > + This chip integrates Advanced Actuator Control (AAC) technology > > + and is intended for driving voice coil lens in camera modules. > > + > > +properties: > > + compatible: > > + enum: > > + - giantec,gt9768 # for GT9768 VCM > > + - giantec,gt9769 # for GT9769 VCM > > I don't think these comments are needed, they should be clear from the > compatibles, no? > > > + > > + reg: > > + maxItems: 1 > > + > > + vin-supply: true > > + > > + vdd-supply: true > > + > > + 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). Except that those would have to be strings for floats or fractions. For properties which have little chance of being something common and aren't any form of standard unit, I think it is fine to just use the h/w specific values. The first question to ask whether these parameters are common to all/many voice coil motors? > > + default: 2 > > + > > + 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. Here, I agree. Rob
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. > > > > >
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
diff --git a/Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml b/Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml new file mode 100644 index 000000000000..8c9f1eb4dac8 --- /dev/null +++ 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# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Giantec Semiconductor, Crop. GT97xx Voice Coil Motor (VCM) + +maintainers: + - Zhi Mao <zhi.mao@mediatek.com> + +description: |- + The Giantec GT97xx is a 10-bit DAC with current sink capability. + The DAC is controlled via I2C bus that operates at clock rates up to 1MHz. + This chip integrates Advanced Actuator Control (AAC) technology + and is intended for driving voice coil lens in camera modules. + +properties: + compatible: + enum: + - giantec,gt9768 # for GT9768 VCM + - giantec,gt9769 # for GT9769 VCM + + reg: + maxItems: 1 + + vin-supply: true + + vdd-supply: true + + 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) + default: 2 + + 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. + $ref: /schemas/types.yaml#/definitions/uint32 + default: 0x20 + minimum: 0x00 + maximum: 0x3f + + 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 + default: 1 + +required: + - compatible + - reg + - vin-supply + - vdd-supply + +additionalProperties: false + +examples: + - | + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + vcm@c { + compatible = "giantec,gt9768"; + reg = <0x0c>; + + vin-supply = <>97xx_vin>; + vdd-supply = <>97xx_vdd>; + giantec,aac-timing = <0x20>; + }; + }; + +...