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 D866FC7EE23 for ; Mon, 29 May 2023 05:26:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231455AbjE2F05 (ORCPT ); Mon, 29 May 2023 01:26:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33602 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230212AbjE2F0y (ORCPT ); Mon, 29 May 2023 01:26:54 -0400 Received: from fgw22-7.mail.saunalahti.fi (fgw22-7.mail.saunalahti.fi [62.142.5.83]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0E7879B for ; Sun, 28 May 2023 22:26:51 -0700 (PDT) Received: from localhost (88-113-26-95.elisa-laajakaista.fi [88.113.26.95]) by fgw22.mail.saunalahti.fi (Halon) with ESMTP id 68d7f7da-fde1-11ed-a9de-005056bdf889; Mon, 29 May 2023 08:26:49 +0300 (EEST) From: andy.shevchenko@gmail.com Date: Mon, 29 May 2023 08:26:48 +0300 To: Astrid Rost Cc: Jonathan Cameron , Lars-Peter Clausen , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, kernel@axis.com, Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Mathieu Othacehe Subject: Re: [PATCH v4 7/7] iio: light: vcnl4000: Add calibration bias for 4040/4200 Message-ID: References: <20230522142621.1680563-1-astrid.rost@axis.com> <20230522142621.1680563-8-astrid.rost@axis.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230522142621.1680563-8-astrid.rost@axis.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Mon, May 22, 2023 at 04:26:21PM +0200, Astrid Rost kirjoitti: > The calibration bias is setting the LED current to change > the detection distance. > Add read/write attribute for proximity calibration bias and read > attribute for available values. > This is supported for vcnl4040 and vcnl4200. Indentation of the commit message is rather random. ... > + This means that one of the previous change missed the blank line here. > +static const int vcnl4040_ps_calibbias_ua[][2] = { > + {0, 50000}, > + {0, 75000}, > + {0, 100000}, > + {0, 120000}, > + {0, 140000}, > + {0, 160000}, > + {0, 180000}, > + {0, 200000}, > +}; > + > static const int vcnl4040_als_persistence[] = {1, 2, 4, 8}; > static const int vcnl4040_ps_persistence[] = {1, 2, 3, 4}; > static const int vcnl4040_ps_oversampling_ratio[] = {1, 2, 4, 8}; > return ret; > } ... > +static ssize_t vcnl4040_read_ps_calibbias(struct vcnl4000_data *data, int *val, int *val2) > +{ > + int ret; > + > + ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF3); > + if (ret < 0) > + return ret; > + > + ret = FIELD_GET(VCNL4040_PS_MS_LED_I, ret); > + > + if (ret >= ARRAY_SIZE(vcnl4040_ps_calibbias_ua)) > + return -EINVAL; > + > + *val = vcnl4040_ps_calibbias_ua[ret][0]; > + *val2 = vcnl4040_ps_calibbias_ua[ret][1]; > + return ret; Are you sure this is correct? > +} ... > +static ssize_t vcnl4040_write_ps_calibbias(struct vcnl4000_data *data, int val) > +{ > + unsigned int i; > + int ret, index = -1; i or index is redundant. > + u16 regval; > + > + for (i = 0; i < ARRAY_SIZE(vcnl4040_ps_calibbias_ua); i++) { > + if (val == vcnl4040_ps_calibbias_ua[i][1]) { > + index = i; > + break; > + } > + } > + > + if (index < 0) > + return -EINVAL; > + > + mutex_lock(&data->vcnl4000_lock); > + > + ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF3); > + if (ret < 0) > + goto out; > + > + regval = (ret & ~VCNL4040_PS_MS_LED_I) | > + FIELD_PREP(VCNL4040_PS_MS_LED_I, index); > + ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3, > + regval); > + > +out: out_unlock: > + mutex_unlock(&data->vcnl4000_lock); > + return ret; > +} ... > default: > return -EINVAL; > } > - Stray and ping-pong style of change at the same time. Avoid ping-pong style when you add something in the series which is being removed aftewards in the same series without need to be added in the first place. > + case IIO_CHAN_INFO_CALIBBIAS: > + switch (chan->type) { > + case IIO_PROXIMITY: > + ret = vcnl4040_read_ps_calibbias(data, val, val2); > + if (ret < 0) > + return ret; > + return IIO_VAL_INT_PLUS_MICRO; > + default: > + return -EINVAL; > + } -- With Best Regards, Andy Shevchenko