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=-4.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 7E6C8C43461 for ; Mon, 7 Sep 2020 13:30:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2D2D121481 for ; Mon, 7 Sep 2020 13:30:03 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="Tr/UEPlB" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729543AbgIGNXm (ORCPT ); Mon, 7 Sep 2020 09:23:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:32916 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729407AbgIGNXM (ORCPT ); Mon, 7 Sep 2020 09:23:12 -0400 Received: from mail-ej1-x641.google.com (mail-ej1-x641.google.com [IPv6:2a00:1450:4864:20::641]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CD08CC061573 for ; Mon, 7 Sep 2020 06:23:11 -0700 (PDT) Received: by mail-ej1-x641.google.com with SMTP id lo4so18203253ejb.8 for ; Mon, 07 Sep 2020 06:23:11 -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=gw9lL2TvHDntCUhAnmDZSsRD6P570jc9dLDcigHjLqo=; b=Tr/UEPlBz2fsDMLmFd0VCTGHcFQoc3bi3jh3lDW53ENl1TrQGuLEKeEciByE6ZA2WD XSmZEXdGl7UtzQnOv0mi030b/ZpXsd3jp6njzQGgvSUCvjgszzE07oO7iR4juvN86L83 BBDqz5v7eDZQXHBAJ4NVL4rmU5c4uIBCozDxg= 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=gw9lL2TvHDntCUhAnmDZSsRD6P570jc9dLDcigHjLqo=; b=tj2ZrmkDgQu93m3Fp6Zc1vPNw7Uk1QZVTBfHMdKg1I1+PqaeZulweipOmeu38MG0V8 htmd5juA5TdUp3MO7k1loHUSAIT/nBL0NvxjuCcHFQEmbWr6BgLjFSHmiGf+F4VJ3AFC V+mX1onQpGO/5rpNk3ZthH7I4OqX8QamzWOlpPPoCDViga0Ehv3Te/5CaA+00UkWB93N rZ4d8syrulSTNNrcMCKdlMlEWNpXBrOYiTYYoo7rmnECSCmmAWJ3LO0Z0IHiYVTdMCq8 95sWzM3ss9zdA0jtYmYIBeMjMK/FmazduxLoheYpk0pVkEbu9/4091lDlS9qGnEX59il +t/g== X-Gm-Message-State: AOAM531u1+NlwDBuKDHPDptVUORkCZSAae/0H0slzlFTWbzNcnzxNsJ7 wYrijn36enTOfw3col50Ro55QsoDVD5ohA== X-Google-Smtp-Source: ABdhPJxR9e5a9GEwu0OoWmLhOIaAU2nsSBMcomlJC43VswmBEAm44GcWAObDjl1M2u/KaE/QO6hbjQ== X-Received: by 2002:a17:907:b0b:: with SMTP id h11mr21448992ejl.330.1599484990278; Mon, 07 Sep 2020 06:23:10 -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 w11sm14800893edx.81.2020.09.07.06.23.09 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 07 Sep 2020 06:23:10 -0700 (PDT) Received: by mail-wm1-f54.google.com with SMTP id c19so12319946wmd.1 for ; Mon, 07 Sep 2020 06:23:09 -0700 (PDT) X-Received: by 2002:a1c:a5c8:: with SMTP id o191mr21337268wme.127.1599484528763; Mon, 07 Sep 2020 06:15:28 -0700 (PDT) MIME-Version: 1.0 References: <20200902120122.24456-1-dongchun.zhu@mediatek.com> <20200902120122.24456-3-dongchun.zhu@mediatek.com> <20200902134421.GN1891694@smile.fi.intel.com> <1599225767.4733.64.camel@mhfsdcap03> In-Reply-To: From: Tomasz Figa Date: Mon, 7 Sep 2020 15:15:17 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v14 2/2] media: i2c: Add OV02A10 image sensor driver To: Andy Shevchenko Cc: Dongchun Zhu , Andy Shevchenko , Mauro Carvalho Chehab , Rob Herring , Mark Rutland , Sakari Ailus , Nicolas Boichat , Matthias Brugger , Bingbu Cao , srv_heupstream , "moderated list:ARM/Mediatek SoC support" , linux-arm Mailing List , Sj Huang , Linux Media Mailing List , devicetree , Louis Kuo , =?UTF-8?B?U2hlbmduYW4gV2FuZyAo546L5Zyj55S3KQ==?= , matrix.zhu@aliyun.com Content-Type: text/plain; charset="UTF-8" Sender: devicetree-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On Fri, Sep 4, 2020 at 4:06 PM Andy Shevchenko wrote: > > On Fri, Sep 4, 2020 at 4:48 PM Dongchun Zhu wrote: > > On Wed, 2020-09-02 at 16:44 +0300, Andy Shevchenko wrote: > > > On Wed, Sep 02, 2020 at 08:01:22PM +0800, Dongchun Zhu wrote: > > ... > > > > > + struct i2c_client *client = to_i2c_client(dev); > > > > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > > > > > > struct v4l2_subdev *sd = dev_get_drvdata(dev); > > > > > > Same for the rest similar cases. > > > > We've discussed the issue in DW9768 V2. > > > > For V4L2 sub-device drivers, dev_get_drvdata() shouldn't be used > > directly. > > > > More details please check the Google Issue: > > https://partnerissuetracker.corp.google.com/issues/147957975 > > This is not a public link. Can you remind me what was the issue? > v4l2-subdev framework uses dev drvdata for its own purposes. However, that problem was about the driver setting its own drvdata and having it overridden by the framework. There is nothing wrong in using dev_get_drvdata(), assuming the correct type is known and here it's guaranteed to be v4l2_subdev for the v4l2-subdev framework. In fact i2c_get_clientdata() [1] is just a wrapper that calls dev_get_drvdata(&client->dev). [1] https://elixir.bootlin.com/linux/v5.9-rc3/source/include/linux/i2c.h#L351 > ... > > > > > + if (!bus_cfg.nr_of_link_frequencies) { > > > > + dev_err(dev, "no link frequencies defined\n"); > > > > + ret = -EINVAL; > > > > + goto check_hwcfg_error; > > > > + } > > > > > > If it's 0, the below will break on 'if (j == 0)' with slightly different but > > > informative enough message. What do you keep above check for? > > > > I still prefer to the original version. > > If 'bus_cfg.nr_of_link_frequencies' is 0, shouldn't we directly return > > error? > > But that will happen anyway. I will leave this to Sakari and > maintainers to decide. > I agree with Andy on this. The check is redundant. In fact, the later error message is more meaningful, because it at least suggests a frequency that must be supported, while the earlier one only states the fact. Best regards, Tomasz 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=-3.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 8AF58C433E2 for ; Mon, 7 Sep 2020 13:15:45 +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 2DFE0207C3 for ; Mon, 7 Sep 2020 13:15:45 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="UlMDRTTi"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="Tr/UEPlB" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2DFE0207C3 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-mediatek-bounces+linux-mediatek=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=ZdTIkMpVu91qDLz2zdX3waZC4QzMUD+pSuYHPF8kdYY=; b=UlMDRTTiYPMs6Yy0GkOucjO4I kuv54YFtgRG2C5Cr99zzwvGD9GeO5SOAvD6PZgoGJ/mdmyd6sYXMBJPzZwwIbsg6/gVgPl4MK8YH3 9xBuzGdSe/k8lC/LRqLkXyocNXITP/u0geGZ1KghBrLiTwWNDpLReewVqsBLmZBO5p4P2G182xbBo jyUmFgcRacSxVehlJ2U64ftAF0Z8jWQHif8+MFWQuJLs+QcRY3ykbYPfqZH3OqwdPSuZymqgDxJfz IFYPFJfNZwnVABK+ka3ohdFef1KNLy5BVBvoi5wHXx2d9KtlnM9Q5kxd1Xt5u8gp/y4z4MeDLhr3N QZ8C03uBQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFGzg-0007Jl-Q7; Mon, 07 Sep 2020 13:15:36 +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 1kFGzc-0007IC-E6 for linux-mediatek@lists.infradead.org; Mon, 07 Sep 2020 13:15:33 +0000 Received: by mail-ej1-x641.google.com with SMTP id o8so4982781ejb.10 for ; Mon, 07 Sep 2020 06:15:31 -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=gw9lL2TvHDntCUhAnmDZSsRD6P570jc9dLDcigHjLqo=; b=Tr/UEPlBz2fsDMLmFd0VCTGHcFQoc3bi3jh3lDW53ENl1TrQGuLEKeEciByE6ZA2WD XSmZEXdGl7UtzQnOv0mi030b/ZpXsd3jp6njzQGgvSUCvjgszzE07oO7iR4juvN86L83 BBDqz5v7eDZQXHBAJ4NVL4rmU5c4uIBCozDxg= 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=gw9lL2TvHDntCUhAnmDZSsRD6P570jc9dLDcigHjLqo=; b=CLd9e1dX+ZIoEyhcBw5k3iVqehXSb6rLO3/NDc2pfVaDe1q+vrAEbUa8O4zBcfRjpH ahZG3KXQDzgnp+FSFa2atIsQQwa34BX/IwAjXffbVUHEK0oA5eo+WCJqVNmFQy00sAtA R3ZeTyZa9fDxl2xvPnM3zX5Ywqi1dcluhfjh41uB06lyl3QXONDau8ltY9vC70nYuLH/ L1IqfvoVZYn48XV6+JVt6FybTJli7tX8KUKwPc5bqCen0CsZxxDSEEtCbBRCw7Klj9Cj k83S9kFg6icpZk/L6y6cxRdG4f246zHug2jJqkyAT2oWtlJmhYdMAf+e0MKPqXWDltXD AEXw== X-Gm-Message-State: AOAM533xNnV8IWBIqNn3nQs71u9ATW12cgJ3carA6rqPG1ef8zS1d9Sa 2k/yHUHkJOtJ9HuL+LHOKpRxTfLjmEUIbw== X-Google-Smtp-Source: ABdhPJyUwKHshM3Yt6hlFGYyTMztoUG7AoIOK3hvJlyhA+8ja/jWL14KilNRNweaEC8UYEBRxoN6tQ== X-Received: by 2002:a17:906:8690:: with SMTP id g16mr20552336ejx.187.1599484530669; Mon, 07 Sep 2020 06:15:30 -0700 (PDT) Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com. [209.85.128.52]) by smtp.gmail.com with ESMTPSA id w14sm15302255ejn.36.2020.09.07.06.15.29 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 07 Sep 2020 06:15:29 -0700 (PDT) Received: by mail-wm1-f52.google.com with SMTP id e11so12312773wme.0 for ; Mon, 07 Sep 2020 06:15:29 -0700 (PDT) X-Received: by 2002:a1c:a5c8:: with SMTP id o191mr21337268wme.127.1599484528763; Mon, 07 Sep 2020 06:15:28 -0700 (PDT) MIME-Version: 1.0 References: <20200902120122.24456-1-dongchun.zhu@mediatek.com> <20200902120122.24456-3-dongchun.zhu@mediatek.com> <20200902134421.GN1891694@smile.fi.intel.com> <1599225767.4733.64.camel@mhfsdcap03> In-Reply-To: From: Tomasz Figa Date: Mon, 7 Sep 2020 15:15:17 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v14 2/2] media: i2c: Add OV02A10 image sensor driver To: Andy Shevchenko X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200907_091532_485198_27A91323 X-CRM114-Status: GOOD ( 25.51 ) X-BeenThere: linux-mediatek@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 , devicetree , =?UTF-8?B?U2hlbmduYW4gV2FuZyAo546L5Zyj55S3KQ==?= , Louis Kuo , Sj Huang , Rob Herring , "moderated list:ARM/Mediatek SoC support" , Dongchun Zhu , Sakari Ailus , Matthias Brugger , Bingbu Cao , matrix.zhu@aliyun.com, Mauro Carvalho Chehab , linux-arm Mailing List , Linux Media Mailing List Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On Fri, Sep 4, 2020 at 4:06 PM Andy Shevchenko wrote: > > On Fri, Sep 4, 2020 at 4:48 PM Dongchun Zhu wrote: > > On Wed, 2020-09-02 at 16:44 +0300, Andy Shevchenko wrote: > > > On Wed, Sep 02, 2020 at 08:01:22PM +0800, Dongchun Zhu wrote: > > ... > > > > > + struct i2c_client *client = to_i2c_client(dev); > > > > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > > > > > > struct v4l2_subdev *sd = dev_get_drvdata(dev); > > > > > > Same for the rest similar cases. > > > > We've discussed the issue in DW9768 V2. > > > > For V4L2 sub-device drivers, dev_get_drvdata() shouldn't be used > > directly. > > > > More details please check the Google Issue: > > https://partnerissuetracker.corp.google.com/issues/147957975 > > This is not a public link. Can you remind me what was the issue? > v4l2-subdev framework uses dev drvdata for its own purposes. However, that problem was about the driver setting its own drvdata and having it overridden by the framework. There is nothing wrong in using dev_get_drvdata(), assuming the correct type is known and here it's guaranteed to be v4l2_subdev for the v4l2-subdev framework. In fact i2c_get_clientdata() [1] is just a wrapper that calls dev_get_drvdata(&client->dev). [1] https://elixir.bootlin.com/linux/v5.9-rc3/source/include/linux/i2c.h#L351 > ... > > > > > + if (!bus_cfg.nr_of_link_frequencies) { > > > > + dev_err(dev, "no link frequencies defined\n"); > > > > + ret = -EINVAL; > > > > + goto check_hwcfg_error; > > > > + } > > > > > > If it's 0, the below will break on 'if (j == 0)' with slightly different but > > > informative enough message. What do you keep above check for? > > > > I still prefer to the original version. > > If 'bus_cfg.nr_of_link_frequencies' is 0, shouldn't we directly return > > error? > > But that will happen anyway. I will leave this to Sakari and > maintainers to decide. > I agree with Andy on this. The check is redundant. In fact, the later error message is more meaningful, because it at least suggests a frequency that must be supported, while the earlier one only states the fact. Best regards, Tomasz _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek 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=-3.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 56723C43461 for ; Mon, 7 Sep 2020 13:16:54 +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 D2A28207C3 for ; Mon, 7 Sep 2020 13:16:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="gJUs8pI4"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="Tr/UEPlB" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D2A28207C3 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=dzDmk+Ld9Uoge0NOK5N9OP62fyK52b8TT8gEpXBx+BE=; b=gJUs8pI4IILIrt1Ndv39E/NlP soAd93mBEvzy0Q0KoXy4fnX47XRHKpL9rYyQsXdEW90HFbI1X/daZynXcUjhyWBw207q3C9Vj5cap Su9/HjRrGzBui/cH7i//utHLDLdSLiWdWU6exxXrx6GSXbrycrtfQWBog5fPUvqoGT8flFWaozxmn ttYHIx4NnhPyRnDiXvxGyEeUW8GPOYVa8zFpOF4+s2BEwPbj9PU9IcADS58tjvtQGQhpBLM4EzjZH +1wHHS3qPdpVnBaCGKFvlDV98j21FiZnfLDCi5oyLkfqiuxQ/4PuZlUKS4kJUe0oFfGItkFbBsL2n 1K7eS2swg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFGzf-0007JF-4x; Mon, 07 Sep 2020 13:15:35 +0000 Received: from mail-ej1-x643.google.com ([2a00:1450:4864:20::643]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFGzb-0007IB-RY for linux-arm-kernel@lists.infradead.org; Mon, 07 Sep 2020 13:15:32 +0000 Received: by mail-ej1-x643.google.com with SMTP id o8so4982758ejb.10 for ; Mon, 07 Sep 2020 06:15:31 -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=gw9lL2TvHDntCUhAnmDZSsRD6P570jc9dLDcigHjLqo=; b=Tr/UEPlBz2fsDMLmFd0VCTGHcFQoc3bi3jh3lDW53ENl1TrQGuLEKeEciByE6ZA2WD XSmZEXdGl7UtzQnOv0mi030b/ZpXsd3jp6njzQGgvSUCvjgszzE07oO7iR4juvN86L83 BBDqz5v7eDZQXHBAJ4NVL4rmU5c4uIBCozDxg= 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=gw9lL2TvHDntCUhAnmDZSsRD6P570jc9dLDcigHjLqo=; b=h/8qWmhsNbGvmeusARcx94blP5GY/Bfz2I0kmjGr6qMonGqTWWnaqobhrRHlE2vSEt XTYsnVbS1asa71FktIZR1cjXJgEp2jIzVNktF1ACP3PIg9/50Pbfgyy7IrxgfhB8byFK bjOXB2+My82BflziGRu/lxRcZfCwXu7o7JIADBgZatWTh/dCxUswIoseO5VyZsUGlUQ8 71lIa+75XDf/yGyPESXtx0Ztx+gD88J6KNQ2BE94u6tHAehzrRZWSrHFHEGXu3QN9YCP FKTGEhIXn9YJx5mP1QPsldnmM4dDVw912eLo2AjaR+COZcL+MhLfINBF50Z+pPSaYX02 XRow== X-Gm-Message-State: AOAM532oZupsjnca0S0NiOrt37p+7jfkwTrwGKxZwcJ23SVGdjF1nswL jWvGeKNU0Uhns1RBiteEdclztVuoLZh/oA== X-Google-Smtp-Source: ABdhPJy3n25d+PtmDmrU1bcL3j5es09Q5CVM5P4kgNmOIx6fWsXHBw07vVnDJwAmTmyVGNnCgzcigQ== X-Received: by 2002:a17:906:375a:: with SMTP id e26mr20522264ejc.552.1599484530437; Mon, 07 Sep 2020 06:15:30 -0700 (PDT) Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com. [209.85.128.53]) by smtp.gmail.com with ESMTPSA id bx24sm15482546ejb.51.2020.09.07.06.15.29 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 07 Sep 2020 06:15:29 -0700 (PDT) Received: by mail-wm1-f53.google.com with SMTP id e11so12312774wme.0 for ; Mon, 07 Sep 2020 06:15:29 -0700 (PDT) X-Received: by 2002:a1c:a5c8:: with SMTP id o191mr21337268wme.127.1599484528763; Mon, 07 Sep 2020 06:15:28 -0700 (PDT) MIME-Version: 1.0 References: <20200902120122.24456-1-dongchun.zhu@mediatek.com> <20200902120122.24456-3-dongchun.zhu@mediatek.com> <20200902134421.GN1891694@smile.fi.intel.com> <1599225767.4733.64.camel@mhfsdcap03> In-Reply-To: From: Tomasz Figa Date: Mon, 7 Sep 2020 15:15:17 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v14 2/2] media: i2c: Add OV02A10 image sensor driver To: Andy Shevchenko X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200907_091531_918739_6339A0AD X-CRM114-Status: GOOD ( 26.87 ) 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 , devicetree , =?UTF-8?B?U2hlbmduYW4gV2FuZyAo546L5Zyj55S3KQ==?= , Louis Kuo , Sj Huang , Rob Herring , "moderated list:ARM/Mediatek SoC support" , Dongchun Zhu , Sakari Ailus , Matthias Brugger , Bingbu Cao , matrix.zhu@aliyun.com, Mauro Carvalho Chehab , linux-arm Mailing List , 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 On Fri, Sep 4, 2020 at 4:06 PM Andy Shevchenko wrote: > > On Fri, Sep 4, 2020 at 4:48 PM Dongchun Zhu wrote: > > On Wed, 2020-09-02 at 16:44 +0300, Andy Shevchenko wrote: > > > On Wed, Sep 02, 2020 at 08:01:22PM +0800, Dongchun Zhu wrote: > > ... > > > > > + struct i2c_client *client = to_i2c_client(dev); > > > > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > > > > > > struct v4l2_subdev *sd = dev_get_drvdata(dev); > > > > > > Same for the rest similar cases. > > > > We've discussed the issue in DW9768 V2. > > > > For V4L2 sub-device drivers, dev_get_drvdata() shouldn't be used > > directly. > > > > More details please check the Google Issue: > > https://partnerissuetracker.corp.google.com/issues/147957975 > > This is not a public link. Can you remind me what was the issue? > v4l2-subdev framework uses dev drvdata for its own purposes. However, that problem was about the driver setting its own drvdata and having it overridden by the framework. There is nothing wrong in using dev_get_drvdata(), assuming the correct type is known and here it's guaranteed to be v4l2_subdev for the v4l2-subdev framework. In fact i2c_get_clientdata() [1] is just a wrapper that calls dev_get_drvdata(&client->dev). [1] https://elixir.bootlin.com/linux/v5.9-rc3/source/include/linux/i2c.h#L351 > ... > > > > > + if (!bus_cfg.nr_of_link_frequencies) { > > > > + dev_err(dev, "no link frequencies defined\n"); > > > > + ret = -EINVAL; > > > > + goto check_hwcfg_error; > > > > + } > > > > > > If it's 0, the below will break on 'if (j == 0)' with slightly different but > > > informative enough message. What do you keep above check for? > > > > I still prefer to the original version. > > If 'bus_cfg.nr_of_link_frequencies' is 0, shouldn't we directly return > > error? > > But that will happen anyway. I will leave this to Sakari and > maintainers to decide. > I agree with Andy on this. The check is redundant. In fact, the later error message is more meaningful, because it at least suggests a frequency that must be supported, while the earlier one only states the fact. Best regards, Tomasz _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel