From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BB107C433E6 for ; Mon, 31 Aug 2020 11:53:23 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 876912098B for ; Mon, 31 Aug 2020 11:53:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="MHJLNj7f"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="FaiR145S" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 876912098B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=qRIxJS/x75n9AiydRty6A6udFtF3xfB44u7ZFi0p9U4=; b=MHJLNj7fyik8thaCs8W9J7Wok boGW7d5q4AoJ0rRZ3ru6v2lFoEq8hdZbDFB2QQOLrNF3UVQHsps62UJgaRMjkAv8Otfl6z6pDFhem 26QoQayiwMiBIQvCrTxR2z2SMSvwquF3jVO27VzT4jZJRKbCwRVg5cII+mR/IE5ESbq0LFutsJ1Ms 6Uzy3SXWmlQq4QspNcCAmnCqjCJxdmG+wrgsNElAvS+SFSPV+58W3EhR6AuHwdm5KOtbhxCd7E5Oh bHKYoBimm4zgd5WMm5z4myp/o5R/QWuLSfW7brQGscrMRTauqx9jinamIhz+M8ySm6ZOVYGKyPHxG 85WZzVftA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kCiLg-0001Sr-Vl; Mon, 31 Aug 2020 11:51:45 +0000 Received: from mail-ej1-x641.google.com ([2a00:1450:4864:20::641]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kCiLd-0001S8-Dq for linux-arm-kernel@lists.infradead.org; Mon, 31 Aug 2020 11:51:43 +0000 Received: by mail-ej1-x641.google.com with SMTP id i26so7730867ejb.12 for ; Mon, 31 Aug 2020 04:51:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=efGDe+zcHIa2VsRuM+m7GHMfouf/LYGtTzaBD57lCYU=; b=FaiR145STFKet4ABBn9muz0fH70Iib8OAiaAZFSQWpqeQHq3y/pZH07b/qf/KOfyOJ pl/wrRDqemy8wMDarwdpaqk8VWQka1xilVb4+b92LPoLNxq+dZ3ZWxV941MF8RQ+UCRL gyMOIAp1gr6MMoN3bCNDLeY1q5brBuXhDv/ag= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=efGDe+zcHIa2VsRuM+m7GHMfouf/LYGtTzaBD57lCYU=; b=FoyV+ZgrDOF1OhcleJ+WhYq+xQB5uoWA1TGg2uWclplK/nTNw1PrUahb6ec9ga4KCb G7n7CHIeOTikCvZQggRF+xrkHVlNVBi54CI6HGXEkpwf1XouHPmDWz5+uv+cxNn9A6T+ uTE3lTl5KSJ1olMrK230pyVplJl+wgpRnmgd3tbSUPnJefVuJOdkR33DMWYY53diGNRu 6S0Yfk1bIh5Ve+As6VShAPyWrAysxZJPvgHyjuv9t4cWsgp/WAZDYuQBG3GjSW0CKDYX bzWpuDd4FOmqCJd1chIJ2QWoSMPCb1zajLbZqfaKuXlTCKN+8+UZmX3Csb3UEYEHBPKf P/Nw== X-Gm-Message-State: AOAM533PrcHxvqmD9Oqbov5urwuOc1IKwSQda+8bMk+cZsmVhp8dGHcU Llz1SRC8sBxp6uMZHXpuop5NO+LSNniU2A== X-Google-Smtp-Source: ABdhPJy6h2GR3pSroI3sdaJ9/LkDo/Ed1gHekmi7Ac7JVs7od2dtVI6F/SrytJSR0Oh6BZ/+qcpVtQ== X-Received: by 2002:a17:906:3f82:: with SMTP id b2mr832209ejj.240.1598874699829; Mon, 31 Aug 2020 04:51:39 -0700 (PDT) Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com. [209.85.128.54]) by smtp.gmail.com with ESMTPSA id d7sm8035744ejk.99.2020.08.31.04.51.39 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 31 Aug 2020 04:51:39 -0700 (PDT) Received: by mail-wm1-f54.google.com with SMTP id a65so5082453wme.5 for ; Mon, 31 Aug 2020 04:51:39 -0700 (PDT) X-Received: by 2002:a7b:c925:: with SMTP id h5mr1009961wml.28.1598874287354; Mon, 31 Aug 2020 04:44:47 -0700 (PDT) MIME-Version: 1.0 References: <20200710101850.4604-1-dongchun.zhu@mediatek.com> <20200710101850.4604-2-dongchun.zhu@mediatek.com> In-Reply-To: <20200710101850.4604-2-dongchun.zhu@mediatek.com> From: Tomasz Figa Date: Mon, 31 Aug 2020 13:44:27 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v13 1/2] media: dt-bindings: media: i2c: Document OV02A10 bindings To: Dongchun Zhu , Sakari Ailus X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200831_075142_145444_530CA397 X-CRM114-Status: GOOD ( 28.73 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Nicolas Boichat , Andy Shevchenko , srv_heupstream , linux-devicetree , =?UTF-8?B?U2hlbmduYW4gV2FuZyAo546L5Zyj55S3KQ==?= , Louis Kuo , Sj Huang , Rob Herring , "moderated list:ARM/Mediatek SoC support" , Matthias Brugger , Cao Bing Bu , matrix.zhu@aliyun.com, Mauro Carvalho Chehab , 1095088256@qq.com, "list@263.net:IOMMU DRIVERS , Joerg Roedel , " , Linux Media Mailing List Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Dongchun, On Fri, Jul 10, 2020 at 12:19 PM Dongchun Zhu wrote: > > Add YAML device tree binding for OV02A10 CMOS image sensor, > and the relevant MAINTAINERS entries. > > Reviewed-by: Tomasz Figa > Reviewed-by: Rob Herring > Signed-off-by: Dongchun Zhu > --- > .../bindings/media/i2c/ovti,ov02a10.yaml | 172 +++++++++++++++++++++ > MAINTAINERS | 7 + > 2 files changed, 179 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml > new file mode 100644 > index 0000000..3a916cc > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml > @@ -0,0 +1,172 @@ > +# 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/ovti,ov02a10.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Omnivision OV02A10 CMOS Sensor Device Tree Bindings > + > +maintainers: > + - Dongchun Zhu > + > +description: |- > + The Omnivision OV02A10 is a low-cost, high performance, 1/5-inch, 2 megapixel > + image sensor, which is the latest production derived from Omnivision's CMOS > + image sensor technology. Ihis chip supports high frame rate speeds up to 30fps > + @ 1600x1200 (UXGA) resolution transferred over a 1-lane MIPI interface. The > + sensor output is available via CSI-2 serial data output. > + > +properties: > + compatible: > + const: ovti,ov02a10 > + > + reg: > + maxItems: 1 > + > + clocks: > + items: > + - description: top mux camtg clock > + - description: divider clock > + > + clock-names: > + items: > + - const: eclk > + - const: freq_mux Why do we have two clocks here? Looking at the example suggests that they may be the clocks of the SoC that the integration was done with. However, the binding must only define the aspects of the particular device, i.e. this sensor. I suppose we should only have "eclk" here and it should be described as "external clock for the sensor". > + > + clock-frequency: > + description: > + Frequency of the eclk clock in Hertz. nit: maybe Hz? > + > + dovdd-supply: > + description: > + Definition of the regulator used as Digital I/O voltage supply. > + > + avdd-supply: > + description: > + Definition of the regulator used as Analog voltage supply. > + > + dvdd-supply: > + description: > + Definition of the regulator used as Digital core voltage supply. > + > + powerdown-gpios: > + description: > + Must be the device tree identifier of the GPIO connected to the > + PD_PAD pin. This pin is used to place the OV02A10 into standby mode > + or shutdown mode. As the line needs to be high for the powerdown mode > + to be active, it should be marked GPIO_ACTIVE_HIGH. > + maxItems: 1 > + > + reset-gpios: > + description: > + Must be the device tree identifier of the GPIO connected to the > + RST_PD pin. If specified, it will be asserted during driver probe. > + As the line needs to be low for the reset to be active, it should be > + marked GPIO_ACTIVE_LOW. > + maxItems: 1 > + > + rotation: > + description: > + Definition of the sensor's placement. > + allOf: > + - $ref: "/schemas/types.yaml#/definitions/uint32" > + - enum: > + - 0 # Sensor Mounted Upright > + - 180 # Sensor Mounted Upside Down > + default: 0 > + > + ovti,mipi-tx-speed: > + description: > + Indication of MIPI transmission speed select, which is to control D-PHY > + timing setting by adjusting MIPI clock voltage to improve the clock > + driver capability. The description says that the value adjusts "MIPI clock voltage". Should the property be renamed to "ovti,mipi-clock-voltage"? > + allOf: > + - $ref: "/schemas/types.yaml#/definitions/uint32" > + - enum: > + - 0 # 20MHz - 30MHz > + - 1 # 30MHz - 50MHz > + - 2 # 50MHz - 75MHz > + - 3 # 75MHz - 100MHz > + - 4 # 100MHz - 130MHz > + default: 3 > + I've discussed this on IRC with Sakari. It sounds like this works as is for us because the driver currently only supports 1 mode, always running the link at 390 MHz. This won't scale if one intends to add more modes, because DT can't be expected to be updated when the driver changes. The two are expected to be separate and backwards compatible. I think we could model this in DT as an array of pairs. Similarly to the OPP bindings [1]. An example to have all link speeds up to 390 MHz use the value 4: ovti,mipi-clock-voltages = < /* KHz clock voltage unit */ 390000 4 >; if one wants to select different voltage for different link, they could do so as well. With the example below, the driver should configure "3" for link frequencies <= 150 MHz and "4" for > 150 MHz <= 390 MHz. Link frequencies > 390 MHz should be disallowed. ovti,mipi-clock-voltages = < /* KHz clock voltage unit */ 150000 3 390000 4 >; What do you think? [1] https://elixir.bootlin.com/linux/v5.9-rc3/source/Documentation/devicetree/bindings/opp/opp.txt Best regards, Tomasz _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel