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=-11.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=ham 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 F04BBC00A89 for ; Thu, 5 Nov 2020 07:59:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 79D212071A for ; Thu, 5 Nov 2020 07:59:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604563145; bh=UXfWX6hHeHhg5Ax1bmRo9i70QcWzhpzD1F9qWVpCrEU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=TH86QGV5+wIv8K+JbAR3OZ0yW4KjBg3ZaNzGx3z5Lf3+50trR1UsFj2tLFK4jr7AQ bk3gFLUPvLy/+DF+1TkltOOwYEoAwmKlG+ZXurSiTmIWv8G3J0u86tZzty/m5r6A+9 kY1H83JUKyyK7DfVlFeEDIp08anRoet4X4vj0S2w= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726428AbgKEH7E (ORCPT ); Thu, 5 Nov 2020 02:59:04 -0500 Received: from mail.kernel.org ([198.145.29.99]:53684 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725287AbgKEH7E (ORCPT ); Thu, 5 Nov 2020 02:59:04 -0500 Received: from coco.lan (ip5f5ad5d8.dynamic.kabel-deutschland.de [95.90.213.216]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 7F22A2071A; Thu, 5 Nov 2020 07:59:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604563143; bh=UXfWX6hHeHhg5Ax1bmRo9i70QcWzhpzD1F9qWVpCrEU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=oUgRIqlDADuK20G9Dcpxd5t/a7X+D+/CBGIM/UVZCSfLV4lphrh3KP+W9pIXSsq2E kfC73i39XGvMF5cAnwq545pJxY2MAHgpOAceWyfN/HwjnK0pXn3J9fixyj/Wx73PPI pJhA9BzCfVBoSPe6fGsBKpNpV9nh53ZFCLpyp3wc= Date: Thu, 5 Nov 2020 08:58:59 +0100 From: Mauro Carvalho Chehab To: Sakari Ailus Cc: linux-media@vger.kernel.org Subject: Re: [PATCH v2 003/106] smiapp: Calculate CCS limit offsets and limit buffer size Message-ID: <20201105085859.250ff135@coco.lan> In-Reply-To: <20201105084338.23327eea@coco.lan> References: <20201007084505.25761-1-sakari.ailus@linux.intel.com> <20201007084557.25843-1-sakari.ailus@linux.intel.com> <20201007084557.25843-3-sakari.ailus@linux.intel.com> <20201105084338.23327eea@coco.lan> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.32; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Em Thu, 5 Nov 2020 08:43:38 +0100 Mauro Carvalho Chehab escreveu: > Em Wed, 7 Oct 2020 11:44:23 +0300 > Sakari Ailus escreveu: > > > Calculate the limit offsets and the size of the limit buffer. CCS limits > > are read into this buffer, and the offsets are helpful in accessing the > > information in it. > > > > Signed-off-by: Sakari Ailus > > --- > > drivers/media/i2c/smiapp/smiapp-core.c | 40 +++++++++++++++++++++++++- > > 1 file changed, 39 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c > > index 105ef29152e8..47e983e9cd87 100644 > > --- a/drivers/media/i2c/smiapp/smiapp-core.c > > +++ b/drivers/media/i2c/smiapp/smiapp-core.c > > @@ -27,6 +27,7 @@ > > #include > > #include > > > > +#include "ccs-limits.h" > > #include "smiapp.h" > > > > #define SMIAPP_ALIGN_DIM(dim, flags) \ > > @@ -34,6 +35,11 @@ > > ? ALIGN((dim), 2) \ > > : (dim) & ~1) > > > > +struct ccs_limit_offset { > > + u16 lim; > > + u16 info; > > +} ccs_limit_offsets[CCS_L_LAST + 1]; > > + > > Hmm... that sounds weird. > > As you're declaring the struct inside smiapp-core.c, this > should be static... > > > /* > > * smiapp_module_idents - supported camera modules > > */ > > @@ -3166,7 +3172,39 @@ static struct i2c_driver smiapp_i2c_driver = { > > .id_table = smiapp_id_table, > > }; > > > > -module_i2c_driver(smiapp_i2c_driver); > > +static int smiapp_module_init(void) > > +{ > > + unsigned int i, l; > > + > > + for (i = 0, l = 0; ccs_limits[i].size && l < CCS_L_LAST; i++) { > > + if (!(ccs_limits[i].flags & CCS_L_FL_SAME_REG)) { > > + ccs_limit_offsets[l + 1].lim = > > + ALIGN(ccs_limit_offsets[l].lim + > > + ccs_limits[i].size, > > + ccs_reg_width(ccs_limits[i + 1].reg)); > > + ccs_limit_offsets[l].info = i; > > + l++; > > + } else { > > + ccs_limit_offsets[l].lim += ccs_limits[i].size; > > + } > > + } > > + > > + if (WARN_ON(ccs_limits[i].size)) > > + return -EINVAL; > > ... yet, this is the only place where this is used. > > It sounds to me that you should move the var to be inside this function, > > e. g. changing the above to: > > struct ccs_limit_offset { > u16 lim; > u16 info; > }; > > static int smiapp_module_init(void) > { > struct ccs_limit_offset ccs_limit_offsets[CCS_L_LAST + 1]; > unsigned int i, l; Ok, it sounds that you start using this on patch 007/106. So, it sounds that you should just need to add "static" to its definition. > > > > + > > + if (WARN_ON(l != CCS_L_LAST)) > > + return -EINVAL; > > + > > + return i2c_register_driver(THIS_MODULE, &smiapp_i2c_driver); > > +} > > + > > +static void smiapp_module_cleanup(void) > > +{ > > + i2c_del_driver(&smiapp_i2c_driver); > > +} > > + > > +module_init(smiapp_module_init); > > +module_exit(smiapp_module_cleanup); > > > > MODULE_AUTHOR("Sakari Ailus "); > > MODULE_DESCRIPTION("Generic SMIA/SMIA++ camera module driver"); > > > > Thanks, > Mauro Thanks, Mauro