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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A3BA4C19F2C for ; Mon, 1 Aug 2022 18:07:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233080AbiHASHZ (ORCPT ); Mon, 1 Aug 2022 14:07:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47276 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232300AbiHASHY (ORCPT ); Mon, 1 Aug 2022 14:07:24 -0400 Received: from mail-lf1-x132.google.com (mail-lf1-x132.google.com [IPv6:2a00:1450:4864:20::132]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 62786E39 for ; Mon, 1 Aug 2022 11:07:23 -0700 (PDT) Received: by mail-lf1-x132.google.com with SMTP id a9so5744733lfm.12 for ; Mon, 01 Aug 2022 11:07:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=370jJersisG8fPy4Ka8zqcPhpPqZLT8jJAiRbixcWys=; b=Gg5B/B4sDvtXYmQYppiwAZbHVpuHZxBp+P6rIAK+DrkCM2DhFipqmRYazNbyDiL963 ecLhsKeEd5eV///nUpvUNd+vF9QOBoPl2uAO4hgPa48elzV4QAnhSqJwyWuxZFX4jW2W fpKspmtZptX5VFPeA1NYcDL2W1CkHW31zCVFiF8Ml8gKa2HgXYt3dQhnA1pSWEZkXou3 IDSnmLpKcvOkh4TNMiJTdyCAxk5Uv7fyz8Q4uyV06KwNbvNEhCNHJP3QuwYO9Z5J2J6P UqXcKXRErG+LLVAtiOqJjluBIBuZH+OUfMvyGU+6nCnjeaER+6+Gc+W/tWuvyG0Fi5Vq 8iwQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=370jJersisG8fPy4Ka8zqcPhpPqZLT8jJAiRbixcWys=; b=VWr6W6OsKe4JVMi+aUrs2pL7DhJUNqahoGdb5fBwBK/9ZtZ8ynESn5CpmAY/vmX4XZ e+JApKry6QJaLQRgV2t2Swpur68QjQKZIrKMFuYGrFFM1j32zeD8oPcsVik3RY73+JR+ lZ1DlIBS1iKPXRdrfkRlYCn0qiYiWF8lxdw3qn3nidwPDV3fOg7ABtkhs/Pt0X7VqNld CS1iioyBjGM47lthEKFTW+7w727qzXRjYx6wTSxGG/kXKVh7RE9KmRH8D8HQErBUnF3v 8ezq/a9j2bWD390OtJgtixSnSiVMx1ODZJtmSIN9lxm/PGYX14dAO4CFPk7Xe4TZZwec nxrg== X-Gm-Message-State: AJIora8mTSChRyqzyrIilmoSocfGF/sFSiaXDXo5oIRaYqcNoIOUMNiW jhq7RO7HrQjSTzjEHgZ7Jxccug== X-Google-Smtp-Source: AGRyM1suLNdfhpm/00Q+GZnN4Z3MI9qLna7mJW2e3US6fJa/WDzCKc6pPbvVcAz/8p31Rd+frlNkbw== X-Received: by 2002:a05:6512:16a4:b0:48a:aebb:42fb with SMTP id bu36-20020a05651216a400b0048aaebb42fbmr6428767lfb.355.1659377241169; Mon, 01 Aug 2022 11:07:21 -0700 (PDT) Received: from [192.168.1.6] ([213.161.169.44]) by smtp.gmail.com with ESMTPSA id g41-20020a0565123ba900b0048aec70f7e6sm987459lfv.194.2022.08.01.11.07.19 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 01 Aug 2022 11:07:20 -0700 (PDT) Message-ID: Date: Mon, 1 Aug 2022 20:07:18 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [PATCH v4 3/7] media: i2c: ov9282: Add ov9281 compatible Content-Language: en-US To: Laurent Pinchart , Sakari Ailus Cc: Alexander Stein , "Paul J . Murphy" , Daniele Alessandrelli , Mauro Carvalho Chehab , Rob Herring , Krzysztof Kozlowski , linux-media@vger.kernel.org, devicetree@vger.kernel.org, Dave Stevenson , Naushir Patuck References: <20220728130237.3396663-1-alexander.stein@ew.tq-group.com> <20220728130237.3396663-4-alexander.stein@ew.tq-group.com> <4e89e55b-9312-5525-974b-0a1dbe0b3dd1@linaro.org> From: Krzysztof Kozlowski In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On 29/07/2022 10:18, Laurent Pinchart wrote: > Hi Sakari, > > (Adding Dave and Naush to the CC list) > > On Fri, Jul 29, 2022 at 10:07:36AM +0300, Sakari Ailus wrote: >> On Thu, Jul 28, 2022 at 03:13:11PM +0200, Krzysztof Kozlowski wrote: >>> On 28/07/2022 15:02, Alexander Stein wrote: >>>> According to product brief they are identical from software point of view. >>>> Differences are a different chief ray angle (CRA) and the package. >>>> >>>> Signed-off-by: Alexander Stein >>>> Acked-by: Daniele Alessandrelli >>>> --- >>>> drivers/media/i2c/ov9282.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c >>>> index 8a252bf3b59f..c8d83a29f9bb 100644 >>>> --- a/drivers/media/i2c/ov9282.c >>>> +++ b/drivers/media/i2c/ov9282.c >>>> @@ -1113,6 +1113,7 @@ static const struct dev_pm_ops ov9282_pm_ops = { >>>> }; >>>> >>>> static const struct of_device_id ov9282_of_match[] = { >>>> + { .compatible = "ovti,ov9281" }, >>> >>> The devices seem entirely compatible, so why you add a new compatible >>> and not re-use existing? >>> >>> The difference in lens does not explain this. >> >> It is typically necessary to know what kind of related hardware can be >> found in the system, beyond just the device's register interface. Apart >> from USB cameras, less integrated cameras require low-level software >> control in which specific device properties are important. In this case it >> could be the lens shading table, among other things. >> >> https://www.ovt.com/sensor/ov9282/ >> >> Therefore I think adding a specific compatible string for this one is >> justified. Specific compatible in binding is a requirement. No one discussed this. However not in the driver. None of the arguments above justify adding such binding, unless user-space depends on matching compatible, but not real compatible? >> >> Also cc Laurent. > > Interesting coincidence, we've talked about this topic (as part of a > broader discussion) no later than yesterday. > > I agree with Sakari in that userspace needs to know the exact model of > the camera sensor. I don't see a good alternative to providing that > information through the platform firmware, so the device tree in this > case. The question is how it should be provided (the question of how it > should then be exposed to userspace is also important, but out of scope > in this discussion). > > The compatible string is meant to indicate a device's compatibility with > "something", and that something is often considered from the point of > view of software support, and in particular to pick an appropriate > kernel driver and tune its behaviour for the device. Here, one could > argue that the exact model is also needed to ensure proper software > support, but in userspace this time, not in the kernel. I think using a > dedicated compatible string would be reasonable. An alternative would be > to use another DT property, which should then be standardized. I'm not > sure it's worth it. > > Broadening the discussion, we also need to know detailed information > about the camera lens (I'm talking about the lens itself here, not the > lens controller IC that controls the motor that moves the focus lens). > The lens isn't described in the device tree with a dedicated device tree > node today, and I don't think it should (I'd have a hard time coming up > with a naming scheme for lenses that we could use in compatible strings, > and the lens-related data that a system requires can possibly vary based > not only on the lens itself but on the ISP that the camera sensor is > used with). Typical useful data are the lens movement range, the > hyperfocal distance, but also the lens shading tables. (Part of) that > information is sometimes stored in non-volatile memory in the camera > module (OTP in the camera sensor itself, or a separate EEPROM), but > that's not always the case. We have considered the possibility of > storing the information in the device tree, but I doubt that would be > accepted. We can store the information in userspace in configuration > files, but we will still need to device tree to provide lens > identification information to select the correct configuration file. I > don't know how that should be done. It seems both you and Sakari suggested not to have specific compatible. Such idea (not to have specific compatible) was not proposed by me. Quite contrary - specific compatible is a requirement. However device driver does no need it. Just use fallback for the driver. Best regards, Krzysztof