Re: [PATCH 2/2] staging: iio: tsl2x7x: remove header file

From: Jonathan Cameron
Date: Sun May 07 2017 - 17:47:21 EST

On 05/05/17 01:38, Brian Masney wrote:
> There is a tsl2x7x.h header that is only used by tsl2x7x.c. This patch
> moves the contents of the header file into the C code with the driver.
>
> Signed-off-by: Brian Masney <masneyb@xxxxxxxxxxxxx>
> ---
> drivers/staging/iio/light/tsl2x7x.c | 76 ++++++++++++++++++++++++++-
> drivers/staging/iio/light/tsl2x7x.h | 100 ------------------------------------
> 2 files changed, 75 insertions(+), 101 deletions(-)
> delete mode 100644 drivers/staging/iio/light/tsl2x7x.h
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 8121a51..cd66c9f 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -30,7 +30,7 @@
> #include <linux/iio/events.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> -#include "tsl2x7x.h"
> +#include <linux/pm.h>
>
> /* Cal defs*/
> #define PROX_STAT_CAL 0
> @@ -126,6 +126,80 @@
>
> #define TSL2X7X_MIN_ITIME 3
>
> +/* Max number of segments allowable in LUX table */
> +#define TSL2X7X_MAX_LUX_TABLE_SIZE 9
> +#define MAX_DEFAULT_TABLE_BYTES (sizeof(int) * TSL2X7X_MAX_LUX_TABLE_SIZE)
> +
> +struct iio_dev;
> +
> +struct tsl2x7x_lux {
> + unsigned int ratio;
> + unsigned int ch0;
> + unsigned int ch1;
> +};
> +
> +/**
> + * struct tsl2x7x_default_settings - power on defaults unless
> + * overridden by platform data.
> + * @als_time: ALS Integration time - multiple of 50mS
> + * @als_gain: Index into the ALS gain table.
> + * @als_gain_trim: default gain trim to account for
> + * aperture effects.
> + * @wait_time: Time between PRX and ALS cycles
> + * in 2.7 periods
> + * @prx_time: 5.2ms prox integration time -
> + * decrease in 2.7ms periods
> + * @prx_gain: Proximity gain index
> + * @prox_config: Prox configuration filters.
> + * @als_cal_target: Known external ALS reading for
> + * calibration.
> + * @interrupts_en: Enable/Disable - 0x00 = none, 0x10 = als,
> + * 0x20 = prx, 0x30 = bth
> + * @persistence: H/W Filters, Number of 'out of limits'
> + * ADC readings PRX/ALS.
> + * @als_thresh_low: CH0 'low' count to trigger interrupt.
> + * @als_thresh_high: CH0 'high' count to trigger interrupt.
> + * @prox_thres_low: Low threshold proximity detection.
> + * @prox_thres_high: High threshold proximity detection
> + * @prox_pulse_count: Number if proximity emitter pulses
> + * @prox_max_samples_cal: Used for prox cal.
> + */
> +struct tsl2x7x_settings {
> + int als_time;
> + int als_gain;
> + int als_gain_trim;
> + int wait_time;
> + int prx_time;
> + int prox_gain;
> + int prox_config;
> + int als_cal_target;
> + u8 interrupts_en;
> + u8 persistence;
> + int als_thresh_low;
> + int als_thresh_high;
> + int prox_thres_low;
> + int prox_thres_high;
> + int prox_pulse_count;
> + int prox_max_samples_cal;
> +};
> +
> +/**
> + * struct tsl2X7X_platform_data - Platform callback, glass and defaults
> + * @platform_power: Suspend/resume platform callback
> + * @power_on: Power on callback
> + * @power_off: Power off callback
> + * @platform_lux_table: Device specific glass coefficents
> + * @platform_default_settings: Device specific power on defaults
> + *
> + */
> +struct tsl2X7X_platform_data {
> + int (*platform_power)(struct device *dev, pm_message_t);
> + int (*power_on)(struct iio_dev *indio_dev);
> + int (*power_off)(struct i2c_client *dev);
> + struct tsl2x7x_lux platform_lux_table[TSL2X7X_MAX_LUX_TABLE_SIZE];
> + struct tsl2x7x_settings *platform_default_settings;
> +};
> +
> /* TAOS txx2x7x Device family members */
> enum {
> tsl2571,
> diff --git a/drivers/staging/iio/light/tsl2x7x.h b/drivers/staging/iio/light/tsl2x7x.h
> deleted file mode 100644
> index ecae922..0000000
> --- a/drivers/staging/iio/light/tsl2x7x.h
> +++ /dev/null
> @@ -1,100 +0,0 @@
> -/*
> - * Device driver for monitoring ambient light intensity (lux)
> - * and proximity (prox) within the TAOS TSL2X7X family of devices.
> - *
> - * Copyright (c) 2012, TAOS Corporation.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful, but WITHOUT
> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> - * more details.
> - *
> - * You should have received a copy of the GNU General Public License along
> - * with this program; if not, write to the Free Software Foundation, Inc.,
> - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
> - */
> -
> -#ifndef __TSL2X7X_H
> -#define __TSL2X7X_H
> -#include <linux/pm.h>
Should have ever been here.
> -
> -/* Max number of segments allowable in LUX table */
> -#define TSL2X7X_MAX_LUX_TABLE_SIZE 9
> -#define MAX_DEFAULT_TABLE_BYTES (sizeof(int) * TSL2X7X_MAX_LUX_TABLE_SIZE)
> -
> -struct iio_dev;
Not sure why this was ever here...
> -
> -struct tsl2x7x_lux {
> - unsigned int ratio;
> - unsigned int ch0;
> - unsigned int ch1;
> -};
> -
> -/**
> - * struct tsl2x7x_default_settings - power on defaults unless
> - * overridden by platform data.
> - * @als_time: ALS Integration time - multiple of 50mS
> - * @als_gain: Index into the ALS gain table.
> - * @als_gain_trim: default gain trim to account for
> - * aperture effects.
> - * @wait_time: Time between PRX and ALS cycles
> - * in 2.7 periods
> - * @prx_time: 5.2ms prox integration time -
> - * decrease in 2.7ms periods
> - * @prx_gain: Proximity gain index
> - * @prox_config: Prox configuration filters.
> - * @als_cal_target: Known external ALS reading for
> - * calibration.
> - * @interrupts_en: Enable/Disable - 0x00 = none, 0x10 = als,
> - * 0x20 = prx, 0x30 = bth
> - * @persistence: H/W Filters, Number of 'out of limits'
> - * ADC readings PRX/ALS.
> - * @als_thresh_low: CH0 'low' count to trigger interrupt.
> - * @als_thresh_high: CH0 'high' count to trigger interrupt.
> - * @prox_thres_low: Low threshold proximity detection.
> - * @prox_thres_high: High threshold proximity detection
> - * @prox_pulse_count: Number if proximity emitter pulses
> - * @prox_max_samples_cal: Used for prox cal.
> - */
> -struct tsl2x7x_settings {
> - int als_time;
> - int als_gain;
> - int als_gain_trim;
> - int wait_time;
> - int prx_time;
> - int prox_gain;
> - int prox_config;
> - int als_cal_target;
> - u8 interrupts_en;
> - u8 persistence;
> - int als_thresh_low;
> - int als_thresh_high;
> - int prox_thres_low;
> - int prox_thres_high;
> - int prox_pulse_count;
> - int prox_max_samples_cal;
> -};
> -
> -/**
> - * struct tsl2X7X_platform_data - Platform callback, glass and defaults
> - * @platform_power: Suspend/resume platform callback
> - * @power_on: Power on callback
> - * @power_off: Power off callback
> - * @platform_lux_table: Device specific glass coefficents
> - * @platform_default_settings: Device specific power on defaults
> - *
> - */
> -struct tsl2X7X_platform_data {
> - int (*platform_power)(struct device *dev, pm_message_t);
> - int (*power_on)(struct iio_dev *indio_dev);
Yikes. Can't do that. the iio_dev structures shouldn't be exposed to
board files. Nor for that matter should the i2c_client really.
not sure why they would need them.

Chances are these predate the regulator framework being widely used.
Perhaps Jon can remember how these were used?

I suspect we can kill all of these callbacks off in favour of some
modern PM.
> - int (*power_off)(struct i2c_client *dev);
> - struct tsl2x7x_lux platform_lux_table[TSL2X7X_MAX_LUX_TABLE_SIZE];
> - struct tsl2x7x_settings *platform_default_settings;
> -};
This is an issue. Platform data is provided by board files which need to
be able to see this structure.

Whilst a driver is in staging, this header has to stay there as well.
When you move it out then move it to include/linux/platform_data/*.h


> -
> -#endif /* __TSL2X7X_H */
>