linux-kernel - Re: [PATCH v2 5/9] staging: iio: tsl2x7x: cleaned up i2c calls in tsl2x7x_als

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 9 Jul 2017 18:28:58 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Brian Masney <masneyb@...tation.org>
Cc:     linux-iio@...r.kernel.org, gregkh@...uxfoundation.org,
        devel@...verdev.osuosl.org, knaack.h@....de, lars@...afoo.de,
        pmeerw@...erw.net, linux-kernel@...r.kernel.org,
        Jon.Brenner@....com
Subject: Re: [PATCH v2 5/9] staging: iio: tsl2x7x: cleaned up i2c calls in
 tsl2x7x_als_calibrate()

On Thu,  6 Jul 2017 18:56:22 -0400
Brian Masney <masneyb@...tation.org> wrote:

> The calibration function calls i2c_smbus_write_byte() and
> i2c_smbus_read_byte(). These two function calls are replaced with a
> single call to i2c_smbus_read_byte_data() by this patch. This patch
> also removes an unnecessary call that reads the CNTRL register
> a second time. One of the error paths returned -1 if the ADC was not
> enabled and this patch changes that return value to -EINVAL.
> 
> Signed-off-by: Brian Masney <masneyb@...tation.org>
Applied.
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 34 ++++++++++++----------------------
>  1 file changed, 12 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 6c9f06b..5734628 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -542,39 +542,29 @@ static void tsl2x7x_defaults(struct tsl2X7X_chip *chip)
>  static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
>  {
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> -	u8 reg_val;
>  	int gain_trim_val;
>  	int ret;
>  	int lux_val;
>  
> -	ret = i2c_smbus_write_byte(chip->client,
> -				   (TSL2X7X_CMD_REG | TSL2X7X_CNTRL));
> +	ret = i2c_smbus_read_byte_data(chip->client,
> +				       TSL2X7X_CMD_REG | TSL2X7X_CNTRL);
>  	if (ret < 0) {
>  		dev_err(&chip->client->dev,
> -			"failed to write CNTRL register, ret=%d\n", ret);
> +			"%s: failed to read from the CNTRL register\n",
> +			__func__);
>  		return ret;
>  	}
>  
> -	reg_val = i2c_smbus_read_byte(chip->client);
> -	if ((reg_val & (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON))
> -		!= (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON)) {
> -		dev_err(&chip->client->dev,
> -			"%s: failed: ADC not enabled\n", __func__);
> -		return -1;
> -	}
> -
> -	ret = i2c_smbus_write_byte(chip->client,
> -				   (TSL2X7X_CMD_REG | TSL2X7X_CNTRL));
> -	if (ret < 0) {
> +	if ((ret & (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON))
> +			!= (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON)) {
>  		dev_err(&chip->client->dev,
> -			"failed to write ctrl reg: ret=%d\n", ret);
> -		return ret;
> -	}
> -
> -	reg_val = i2c_smbus_read_byte(chip->client);
> -	if ((reg_val & TSL2X7X_STA_ADC_VALID) != TSL2X7X_STA_ADC_VALID) {
> +			"%s: Device is not powered on and/or ADC is not enabled\n",
> +			__func__);
> +		return -EINVAL;
> +	} else if ((ret & TSL2X7X_STA_ADC_VALID) != TSL2X7X_STA_ADC_VALID) {
>  		dev_err(&chip->client->dev,
> -			"%s: failed: STATUS - ADC not valid.\n", __func__);
> +			"%s: The two ADC channels have not completed an integration cycle\n",
> +			__func__);
>  		return -ENODATA;
>  	}
>