Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d2fbfe0f-b2bf-58e4-1ec9-02dff2d08420@axis.com>
Date:   Mon, 15 May 2023 17:00:21 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH v2 5/7] iio: light: vcnl4000: Add debounce count for
 vcnl4040/4200
Content-Language: en-US
To:     Jonathan Cameron <jic23@kernel.org>,
        Astrid Rost <astrid.rost@axis.com>
CC:     Lars-Peter Clausen <lars@metafoo.de>, <linux-iio@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <kernel@axis.com>
References: <20230509140153.3279288-1-astrid.rost@axis.com>
 <20230509140153.3279288-6-astrid.rost@axis.com>
 <20230514183748.256a210c@jic23-huawei>
From:   Astrid Rost <astridr@axis.com>
In-Reply-To: <20230514183748.256a210c@jic23-huawei>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WFgrRStGQWNDOFBqN3pXaERybW1KSkRqMFVHY0swSDJLSFRER1hmZmgxcmZh?=
 =?utf-8?B?ZXNURHBuUlZGVnZlc01QRlJ4Qk1KaisyWWRsL3J6OFpFRmpXbjY5ZTNWQkpJ?=
 =?utf-8?B?dENRNXpPSTZOaERybW9hM0JSeWNmYmlSYnJONXpnWmhCZnFrSkVRdjlzV082?=
 =?utf-8?B?ZXdjcXE1WjRDQ1hNY1BOQ1MveE5nR2RJRUpGQXNyYmZxQm5yVDd5TXZIYkh0?=
 =?utf-8?B?emUzajFVOThkM2YvQ2V3ZXZFVHJoTkRWVlVPL0REUHRhTmxtQ2g4eVEzNFF3?=
 =?utf-8?B?eVliSE9wSUxVdnhWdG5sbjRpQmdaYzNhanFpMnZBWWVFczNDUlgyMWU2OXVR?=
 =?utf-8?B?eEw3S1NGdnYvcGI2Y2FxZXQ3M25QQ1F5akNKOVNnV3VRSkNqUHAzVXAzbWM0?=
 =?utf-8?B?bDEvbE1DZ0hqZDEyMjMzekZlUGo3WlBVMWprdkd2aGtQei9KUnIrU00xd3JG?=
 =?utf-8?B?dGk3MmM0UW9KdmhLS0ZnLzRmK1owWi9EUVVmbElUbHZEeVY2YllhQUNPcUVX?=
 =?utf-8?B?VEovR2dLQ2w3Q05oWjlzQ2ZKTTNWNGJCVk5oeHphdnNrNjBKejZaaUhvRjQr?=
 =?utf-8?B?TVBlQjhHQzEzcHdaUnpiZEduMDNzcUlMQ2FGTnpmYjBQQ2FjNUdpMUpEdk5E?=
 =?utf-8?B?c09qUVNTcm42cEhqMVpoSjBHSko5N29rNktYL0h6NW9zTmtMaE92NGpBVlJx?=
 =?utf-8?B?dHFYWXFKbXZHNVkwUXZveVl2R05KbU9lSENLSDc3OVFrMTJsdlprSjhVQjRy?=
 =?utf-8?B?UVhEeGhTVXlNMlJJUXk1cnhPUHo2Q3ZGZUF1empidjJ2U044S2ZRNUE3NlBi?=
 =?utf-8?B?cU55aVhvYjBDNzBDcGlNTzlHRWJSMEt3YlFyQzQ1K1hibm1TKzIwbG5vY0JM?=
 =?utf-8?B?WURlQ3pwbkVPMmdWeGlZaG1lWEh4V2ZNVkZDSjlQNG5UMWk2d3ZQNXZpTGpn?=
 =?utf-8?B?UzBRck1pM3VHenBQVFlidWpPSWpUWWYyZ1oxREhpRFhWcVZKemxSdG14TW9X?=
 =?utf-8?B?bEFsL2Z5RlBKNzE5cURSWXFLeGdMTFZWNXhYWW4zSFpKd1JFcE14OHgwd1JB?=
 =?utf-8?B?RW1yNE1ubWt4VFZOQVZhaUJ1VnlVRzQ3Y1NvT2RtRTlOM3hucGRWL2RsSDZT?=
 =?utf-8?B?Ylo2b2piM2dKcmJGSlVLcFFyeWFUMkJ6enhuOVdUVlNlZzh1VDBldTBUOW1L?=
 =?utf-8?B?RzkyQlFCSTk4RGVMUzBFZDYwYlUwSGdnN1BuTmFiVVJVVjhzQndHL3JjUlpS?=
 =?utf-8?B?TnkxV1RuUmJNV2lBMHhUZEIrUDlxWnhxRXNKM0xVaUlFK090QWRWU1lNM3dF?=
 =?utf-8?B?cndYNXVuWlVENml5RlJXUVdRZTRFMXV2MWxWN2ZhWnFPdlllUG5VMGtNVmlt?=
 =?utf-8?B?dmFvMGtpOUk1OThMMUZ3ZVZydllEaVRybmRCVXV1M2tKT3lwUUpwL3lickFl?=
 =?utf-8?B?Y1R5dHB6OUROblNhR0dPMFZVd3o1REdXblU5bDZ4WUhMM1Z1bGxnZ0szODM4?=
 =?utf-8?B?SDNLWXlLZjBmY0sxUDMvM3lKOUNzUWpPaUFUU1lIU3JEb042V09MOVliR0J4?=
 =?utf-8?B?ekJYQXdRR2NaY09Ycm52djFRWk92ZjRYZGEzeDYwaUorS25XNk9tNDA1VkQz?=
 =?utf-8?B?bWszQy9IUEc4OFNKUis0UENRMHFYYXlLaHNSVTBVVXhobGtIZW9oODQ2MWVN?=
 =?utf-8?B?elFieUVVTmlwOHR4bUx4aWs0MTM3UUlnV2JtbVY0Q1YyWkliVzRDdm4xanNK?=
 =?utf-8?B?ei9lOURpS00yT3BIUUdTVlpiaVZGSlJLcno2L1lNdWZ6STVsSzJsemoxMjAy?=
 =?utf-8?B?Z083STNVY2x5QTFTenpnbzdiUHZCd1JYVHF6a1dGMEtVZUFLdzg1NFJrL2ZQ?=
 =?utf-8?B?eEN3czlEMFNQcWltWDJFR3NDazJmcjBvUDZyZTl6RjBmdGx5ejc0UEFtN0hF?=
 =?utf-8?B?czg5L2J1S1RML1pxK1VLTkViVkJleWpzVkRyL1dQQVhjc0pZbDZmVFdmSTIr?=
 =?utf-8?B?VmFrQWwvd1lSWDVNL0dQbkZ5OUV4cmZRZTZ4UFRUaEhWS0RHWFkvVWZEUlRa?=
 =?utf-8?B?TFFpYUgrWUg5MEZBNU1uUjFVSVovS1BGa3Nwamx6WFEvRjEzMjZlZnVGWWhu?=
 =?utf-8?Q?swqk=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: da7fe36b-c81b-4dcc-3746-08db55551bfd
X-MS-Exchange-CrossTenant-AuthSource: DU0PR02MB9467.eurprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 May 2023 15:00:23.4705
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 78703d3c-b907-432f-b066-88f7af9ca3af
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: F4/z4lXRMdskW2ejag2TvMib/2aY8x+hmagLDl9HJAK6XdWmlUqvV4XtA29qGgd9PwyWr3p/cSdyukMgFUEzqw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DU0PR02MB8190
X-Spam-Status: No, score=-8.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_MED,
        SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hello Jonathan,

Thank you!
I will use period instead, but this means it is dependent on integration 
time as well.

Maybe I should try to deliver the first four matching patches and make 
another delivery of the last three.

Astrid


On 5/14/23 19:37, Jonathan Cameron wrote:
> On Tue, 9 May 2023 16:01:51 +0200
> Astrid Rost <astrid.rost@axis.com> wrote:
> 
>> Add read/write attribute for proximity and illumination debounce-count
>> and read attribute for available debounce-counts.
>> The debounce-count is set in the interrupt persistence flags
>> (PS_PERS and ALS_PERS). An interrupt will not be asserted if the raw
>> value is not over (or lower) than the threshold for the set
>> continued refresh times.
>> This is supported for vcnl4040 vcnl4200.
> 
> Why debounce count? That has previously just been used for step detection
> on accelerometers and has a pretty specific definition in
> See Documentation/ABI/testing/sysfs-bus-iio
> 
> "
> What:		/sys/.../iio:deviceX/in_steps_debounce_count
> ...
> 		Specifies the number of steps that must occur within
> 		in_steps_filter_debounce_time for the pedometer to decide the
> 		consumer is making steps.
> "
> 
> What you probably want is period, though that is defined in seconds so
> you may need some unit conversions.  That is defined for events, not for
> a channel so IIO_EV_INFO_PERIOD.
> "
> 		Period of time (in seconds) for which the condition must be
> 		met before an event is generated. If direction is not
> 		specified then this period applies to both directions.
> "
> 
>>
>> Signed-off-by: Astrid Rost <astrid.rost@axis.com>
>> ---
>>   drivers/iio/light/vcnl4000.c | 156 ++++++++++++++++++++++++++++++++++-
>>   1 file changed, 152 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
>> index 68eea686b2dc..b8ce4ed6b0bb 100644
>> --- a/drivers/iio/light/vcnl4000.c
>> +++ b/drivers/iio/light/vcnl4000.c
>> @@ -84,8 +84,10 @@
>>   #define VCNL4040_ALS_CONF_ALS_SHUTDOWN	BIT(0)
>>   #define VCNL4040_ALS_CONF_IT		GENMASK(7, 6) /* Ambient integration time */
>>   #define VCNL4040_ALS_CONF_INT_EN	BIT(1) /* Ambient light Interrupt enable */
>> +#define VCNL4040_ALS_CONF_PERS	GENMASK(3, 2) /* Ambient interrupt persistence setting */
>>   #define VCNL4040_PS_CONF1_PS_SHUTDOWN	BIT(0)
>>   #define VCNL4040_PS_CONF2_PS_IT	GENMASK(3, 1) /* Proximity integration time */
>> +#define VCNL4040_CONF1_PS_PERS	GENMASK(5, 4) /* Proximity interrupt persistence setting */
>>   #define VCNL4040_PS_CONF2_PS_INT	GENMASK(9, 8) /* Proximity interrupt mode */
>>   #define VCNL4040_PS_IF_AWAY		BIT(8) /* Proximity event cross low threshold */
>>   #define VCNL4040_PS_IF_CLOSE		BIT(9) /* Proximity event cross high threshold */
>> @@ -153,6 +155,9 @@ static const int vcnl4200_als_it_times[][2] = {
>>   	{0, 400000},
>>   };
>>   
>> +static const int vcnl4040_als_debounce_count[] = {1, 2, 4, 8};
>> +static const int vcnl4040_ps_debounce_count[] = {1, 2, 3, 4};
>> +
>>   #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
>>   
>>   enum vcnl4000_device_ids {
>> @@ -680,6 +685,106 @@ static ssize_t vcnl4040_write_ps_it(struct vcnl4000_data *data, int val)
>>   	return ret;
>>   }
>>   
>> +static ssize_t vcnl4040_read_als_debounce_count(struct vcnl4000_data *data, int *val)
>> +{
>> +	int ret;
>> +
>> +	ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = FIELD_GET(VCNL4040_ALS_CONF_PERS, ret);
>> +
>> +	if (ret >= ARRAY_SIZE(vcnl4200_als_it_times))
>> +		return -EINVAL;
>> +
>> +	*val = vcnl4040_als_debounce_count[ret];
>> +
>> +	return ret;
>> +}
>> +
>> +static ssize_t vcnl4040_write_als_debounce_count(struct vcnl4000_data *data, int val)
>> +{
>> +	unsigned int i;
>> +	int ret, index = -1;
>> +	u16 regval;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(vcnl4040_als_debounce_count); i++) {
>> +		if (val == vcnl4040_als_debounce_count[i]) {
>> +			index = i;
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (index < 0)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&data->vcnl4000_lock);
>> +
>> +	ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
>> +	if (ret < 0)
>> +		goto out;
>> +
>> +	regval = (ret & ~VCNL4040_ALS_CONF_PERS) |
>> +	    FIELD_PREP(VCNL4040_ALS_CONF_PERS, index);
>> +	ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF,
>> +					regval);
>> +
>> +out:
>> +	mutex_unlock(&data->vcnl4000_lock);
>> +	return ret;
>> +}
>> +
>> +static ssize_t vcnl4040_read_ps_debounce_count(struct vcnl4000_data *data, int *val)
>> +{
>> +	int ret;
>> +
>> +	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = FIELD_GET(VCNL4040_CONF1_PS_PERS, ret);
>> +
>> +	if (ret >= ARRAY_SIZE(vcnl4200_ps_it_times))
>> +		return -EINVAL;
>> +
>> +	*val = vcnl4040_ps_debounce_count[ret];
>> +
>> +	return ret;
>> +}
>> +
>> +static ssize_t vcnl4040_write_ps_debounce_count(struct vcnl4000_data *data, int val)
>> +{
>> +	unsigned int i;
>> +	int ret, index = -1;
>> +	u16 regval;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(vcnl4040_ps_debounce_count); i++) {
>> +		if (val == vcnl4040_ps_debounce_count[i]) {
>> +			index = i;
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (index < 0)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&data->vcnl4000_lock);
>> +
>> +	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
>> +	if (ret < 0)
>> +		goto out;
>> +
>> +	regval = (ret & ~VCNL4040_CONF1_PS_PERS) |
>> +	    FIELD_PREP(VCNL4040_CONF1_PS_PERS, index);
>> +	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1,
>> +					regval);
>> +
>> +out:
>> +	mutex_unlock(&data->vcnl4000_lock);
>> +	return ret;
>> +}
>> +
>>   static int vcnl4000_read_raw(struct iio_dev *indio_dev,
>>   				struct iio_chan_spec const *chan,
>>   				int *val, int *val2, long mask)
>> @@ -730,6 +835,21 @@ static int vcnl4000_read_raw(struct iio_dev *indio_dev,
>>   		if (ret < 0)
>>   			return ret;
>>   		return IIO_VAL_INT_PLUS_MICRO;
>> +	case IIO_CHAN_INFO_DEBOUNCE_COUNT:
>> +		switch (chan->type) {
>> +		case IIO_LIGHT:
>> +			ret = vcnl4040_read_als_debounce_count(data, val);
>> +			break;
>> +		case IIO_PROXIMITY:
>> +			ret = vcnl4040_read_ps_debounce_count(data, val);
>> +			break;
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +		if (ret < 0)
>> +			return ret;
>> +		return IIO_VAL_INT;
>> +
>>   	default:
>>   		return -EINVAL;
>>   	}
>> @@ -753,6 +873,15 @@ static int vcnl4040_write_raw(struct iio_dev *indio_dev,
>>   		default:
>>   			return -EINVAL;
>>   		}
>> +	case IIO_CHAN_INFO_DEBOUNCE_COUNT:
>> +		switch (chan->type) {
>> +		case IIO_LIGHT:
>> +			return vcnl4040_write_als_debounce_count(data, val);
>> +		case IIO_PROXIMITY:
>> +			return vcnl4040_write_ps_debounce_count(data, val);
>> +		default:
>> +			return -EINVAL;
>> +		}
>>   	default:
>>   		return -EINVAL;
>>   	}
>> @@ -791,6 +920,21 @@ static int vcnl4040_read_avail(struct iio_dev *indio_dev,
>>   		}
>>   		*type = IIO_VAL_INT_PLUS_MICRO;
>>   		return IIO_AVAIL_LIST;
>> +	case IIO_CHAN_INFO_DEBOUNCE_COUNT:
>> +		switch (chan->type) {
>> +		case IIO_LIGHT:
>> +			*vals = (int *)vcnl4040_als_debounce_count;
>> +			*length = ARRAY_SIZE(vcnl4040_als_debounce_count);
>> +			break;
>> +		case IIO_PROXIMITY:
>> +			*vals = (int *)vcnl4040_ps_debounce_count;
>> +			*length = ARRAY_SIZE(vcnl4040_ps_debounce_count);
>> +			break;
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +		*type = IIO_VAL_INT;
>> +		return IIO_AVAIL_LIST;
>>   	default:
>>   		return -EINVAL;
>>   	}
>> @@ -1504,15 +1648,19 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
>>   		.type = IIO_LIGHT,
>>   		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>>   			BIT(IIO_CHAN_INFO_SCALE) |
>> -			BIT(IIO_CHAN_INFO_INT_TIME),
>> -		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME),
>> +			BIT(IIO_CHAN_INFO_INT_TIME) |
>> +			BIT(IIO_CHAN_INFO_DEBOUNCE_COUNT),
>> +		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME) |
>> +			BIT(IIO_CHAN_INFO_DEBOUNCE_COUNT),
>>   		.event_spec = vcnl4000_event_spec,
>>   		.num_event_specs = ARRAY_SIZE(vcnl4000_event_spec),
>>   	}, {
>>   		.type = IIO_PROXIMITY,
>>   		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> -			BIT(IIO_CHAN_INFO_INT_TIME),
>> -		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME),
>> +			BIT(IIO_CHAN_INFO_INT_TIME) |
>> +			BIT(IIO_CHAN_INFO_DEBOUNCE_COUNT),
>> +		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME) |
>> +			BIT(IIO_CHAN_INFO_DEBOUNCE_COUNT),
>>   		.ext_info = vcnl4000_ext_info,
>>   		.event_spec = vcnl4040_event_spec,
>>   		.num_event_specs = ARRAY_SIZE(vcnl4040_event_spec),
>