On 10/11/16 09:25, Brian Masney wrote:
> taos_chip_on() reads an eight member array called taos_config
> that contains the desired state of the chip's registers. Only four
> of the registers actually need to be written to. The four that do
> not need to be written to are for the {low,high} byte of the lower
> interrupt threshold and the {low,high} byte of the upper interrupt
> threshold. Interrupts are currently not supported by this driver
> so there is no need to write to these registers.
>
> This patch removes the taos_config array and separates out the
> i2c calls that write to the CONTROL, TIMING, INTERRUPT and ANALOG
> registers. This is part of a larger refactor that was split up to
> make the code review easier.
>
> Signed-off-by: Brian Masney <masn...@onstation.org>
Looks good. Applied to the togreg branch of iio.git.
Thanks
Jonathan
> ---
> drivers/staging/iio/light/tsl2583.c | 122
> +++++++++++++++++-------------------
> 1 file changed, 58 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/staging/iio/light/tsl2583.c
> b/drivers/staging/iio/light/tsl2583.c
> index d74e33b..c196a42 100644
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ b/drivers/staging/iio/light/tsl2583.c
> @@ -31,9 +31,6 @@
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
>
> -/* Triton register offsets */
> -#define TSL258X_REG_MAX 8
> -
> /* Device Registers and Masks */
> #define TSL258X_CNTRL 0x00
> #define TSL258X_ALS_TIME 0X01
> @@ -55,6 +52,7 @@
>
> /* tsl2583 cntrl reg masks */
> #define TSL258X_CNTL_ADC_ENBL 0x02
> +#define TSL258X_CNTL_PWR_OFF 0x00
> #define TSL258X_CNTL_PWR_ON 0x01
>
> /* tsl2583 status reg masks */
> @@ -64,6 +62,8 @@
> /* Lux calculation constants */
> #define TSL258X_LUX_CALC_OVER_FLOW 65535
>
> +#define TSL2583_INTERRUPT_DISABLED 0x00
> +
> #define TSL2583_CHIP_ID 0x90
> #define TSL2583_CHIP_ID_MASK 0xf0
>
> @@ -95,18 +95,8 @@ struct tsl2583_chip {
> int als_time_scale;
> int als_saturation;
> int taos_chip_status;
> - u8 taos_config[8];
> };
>
> -/*
> - * Initial values for device - this values can/will be changed by driver.
> - * and applications as needed.
> - * These values are dynamic.
> - */
> -static const u8 taos_config[8] = {
> - 0x00, 0xee, 0x00, 0x03, 0x00, 0xFF, 0xFF, 0x00
> -}; /* cntrl atime intC Athl0 Athl1 Athh0 Athh1 gain */
> -
> struct taos_lux {
> unsigned int ratio;
> unsigned int ch0;
> @@ -362,16 +352,26 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
> return (int)gain_trim_val;
> }
>
> +static int tsl2583_set_power_state(struct tsl2583_chip *chip, u8 state)
> +{
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(chip->client,
> + TSL258X_CMD_REG | TSL258X_CNTRL, state);
> + if (ret < 0)
> + dev_err(&chip->client->dev, "%s failed to set the power state
> to %d\n",
> + __func__, state);
> +
> + return ret;
> +}
> +
> /*
> * Turn the device on.
> * Configuration must be set before calling this function.
> */
> -static int taos_chip_on(struct iio_dev *indio_dev)
> +static int tsl2583_chip_init_and_power_on(struct iio_dev *indio_dev)
> {
> - int i;
> - int ret;
> - u8 *uP;
> - u8 utmp;
> + int ret, val;
> int als_count;
> int als_time;
> struct tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -383,6 +383,20 @@ static int taos_chip_on(struct iio_dev *indio_dev)
> return -EINVAL;
> }
>
> + /* Power on the device; ADC off. */
> + ret = tsl2583_set_power_state(chip, TSL258X_CNTL_PWR_ON);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(chip->client,
> + TSL258X_CMD_REG | TSL258X_INTERRUPT,
> + TSL2583_INTERRUPT_DISABLED);
> + if (ret < 0) {
> + dev_err(&chip->client->dev, "%s failed to disable interrupts\n",
> + __func__);
> + return ret;
> + }
> +
> /* determine als integration register */
> als_count = (chip->taos_settings.als_time * 100 + 135) / 270;
> if (!als_count)
> @@ -390,62 +404,43 @@ static int taos_chip_on(struct iio_dev *indio_dev)
>
> /* convert back to time (encompasses overrides) */
> als_time = (als_count * 27 + 5) / 10;
> - chip->taos_config[TSL258X_ALS_TIME] = 256 - als_count;
> +
> + val = 256 - als_count;
> + ret = i2c_smbus_write_byte_data(chip->client,
> + TSL258X_CMD_REG | TSL258X_ALS_TIME,
> + val);
> + if (ret < 0) {
> + dev_err(&chip->client->dev, "%s failed to set the als time to
> %d\n",
> + __func__, val);
> + return ret;
> + }
>
> /* Set the gain based on taos_settings struct */
> - chip->taos_config[TSL258X_GAIN] = chip->taos_settings.als_gain;
> + ret = i2c_smbus_write_byte_data(chip->client,
> + TSL258X_CMD_REG | TSL258X_GAIN,
> + chip->taos_settings.als_gain);
> + if (ret < 0) {
> + dev_err(&chip->client->dev, "%s failed to set the gain to %d\n",
> + __func__, chip->taos_settings.als_gain);
> + return ret;
> + }
>
> /* set chip struct re scaling and saturation */
> chip->als_saturation = als_count * 922; /* 90% of full scale */
> chip->als_time_scale = (als_time + 25) / 50;
>
> - /* Power on the device; ADC off. */
> - chip->taos_config[TSL258X_CNTRL] = TSL258X_CNTL_PWR_ON;
> -
> - /*
> - * Use the following shadow copy for our delay before enabling ADC.
> - * Write all the registers.
> - */
> - for (i = 0, uP = chip->taos_config; i < TSL258X_REG_MAX; i++) {
> - ret = i2c_smbus_write_byte_data(chip->client,
> - TSL258X_CMD_REG + i,
> - *uP++);
> - if (ret < 0) {
> - dev_err(&chip->client->dev,
> - "taos_chip_on failed on reg %d.\n", i);
> - return ret;
> - }
> - }
> -
> usleep_range(3000, 3500);
> - /*
> - * NOW enable the ADC
> - * initialize the desired mode of operation
> - */
> - utmp = TSL258X_CNTL_PWR_ON | TSL258X_CNTL_ADC_ENBL;
> - ret = i2c_smbus_write_byte_data(chip->client,
> - TSL258X_CMD_REG | TSL258X_CNTRL,
> - utmp);
> - if (ret < 0) {
> - dev_err(&chip->client->dev, "taos_chip_on failed on 2nd CTRL
> reg.\n");
> +
> + ret = tsl2583_set_power_state(chip, TSL258X_CNTL_PWR_ON |
> + TSL258X_CNTL_ADC_ENBL);
> + if (ret < 0)
> return ret;
> - }
> +
> chip->taos_chip_status = TSL258X_CHIP_WORKING;
>
> return ret;
> }
>
> -static int taos_chip_off(struct iio_dev *indio_dev)
> -{
> - struct tsl2583_chip *chip = iio_priv(indio_dev);
> -
> - /* turn device off */
> - chip->taos_chip_status = TSL258X_CHIP_SUSPENDED;
> - return i2c_smbus_write_byte_data(chip->client,
> - TSL258X_CMD_REG | TSL258X_CNTRL,
> - 0x00);
> -}
> -
> /* Sysfs Interface Functions */
>
> static ssize_t in_illuminance_input_target_show(struct device *dev,
> @@ -768,7 +763,6 @@ static int taos_probe(struct i2c_client *clientp,
>
> mutex_init(&chip->als_mutex);
> chip->taos_chip_status = TSL258X_CHIP_UNKNOWN;
> - memcpy(chip->taos_config, taos_config, sizeof(chip->taos_config));
>
> ret = i2c_smbus_read_byte_data(clientp,
> TSL258X_CMD_REG | TSL258X_CHIPID);
> @@ -808,7 +802,7 @@ static int taos_probe(struct i2c_client *clientp,
> taos_defaults(chip);
>
> /* Make sure the chip is on */
> - ret = taos_chip_on(indio_dev);
> + ret = tsl2583_chip_init_and_power_on(indio_dev);
> if (ret < 0)
> return ret;
>
> @@ -825,7 +819,7 @@ static int __maybe_unused taos_suspend(struct device *dev)
> mutex_lock(&chip->als_mutex);
>
> if (chip->taos_chip_status == TSL258X_CHIP_WORKING) {
> - ret = taos_chip_off(indio_dev);
> + ret = tsl2583_set_power_state(chip, TSL258X_CNTL_PWR_OFF);
> chip->taos_chip_status = TSL258X_CHIP_SUSPENDED;
> }
>
> @@ -842,7 +836,7 @@ static int __maybe_unused taos_resume(struct device *dev)
> mutex_lock(&chip->als_mutex);
>
> if (chip->taos_chip_status == TSL258X_CHIP_SUSPENDED)
> - ret = taos_chip_on(indio_dev);
> + ret = tsl2583_chip_init_and_power_on(indio_dev);
>
> mutex_unlock(&chip->als_mutex);
> return ret;
>
