Re: [PATCH 4/5] staging:iio:adc:spear_adc use info_mask_shared_by_all for samp freq

[Jonathan Cameron <jic23@kernel.org>]

On 12/03/14 20:31, Hartmut Knaack wrote:
> Jonathan Cameron schrieb:
>> Using the core support makes this element available to in kernel users as
>> well as to userspace under exactly the same interface as before.  The
>> intent is to move all sampling frequency control to this approach
>> throughout IIO.
>>
>> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
>> ---
>>   drivers/staging/iio/adc/spear_adc.c | 95 ++++++++++++++-----------------------
>>   1 file changed, 36 insertions(+), 59 deletions(-)
>>
>> diff --git a/drivers/staging/iio/adc/spear_adc.c b/drivers/staging/iio/adc/spear_adc.c
>> index fbe69a7..d8232f1 100644
>> --- a/drivers/staging/iio/adc/spear_adc.c
>> +++ b/drivers/staging/iio/adc/spear_adc.c
>> @@ -185,16 +185,51 @@ static int spear_read_raw(struct iio_dev *indio_dev,
>>   		*val = info->vref_external;
>>   		*val2 = SPEAR_ADC_DATA_BITS;
>>   		return IIO_VAL_FRACTIONAL_LOG2;
>> +	case IIO_CHAN_INFO_SAMP_FREQ:
>> +		*val = info->current_clk;
>> +		return IIO_VAL_INT;
>>   	}
>>
>>   	return -EINVAL;
>>   }
>>
>> +static int spear_adc_write_raw(struct iio_dev *indio_dev,
>> +			       struct iio_chan_spec const *chan,
>> +			       int val,
>> +			       int val2,
>> +			       long mask)
>> +{
>> +	struct spear_adc_info *info = iio_priv(indio_dev);
>> +	u32 clk_high, clk_low, count;
>> +	u32 apb_clk = clk_get_rate(info->clk);
>> +	int ret = 0;
>> +
>> +	mutex_lock(&indio_dev->mlock);
>> +
>> +	if ((val < SPEAR_ADC_CLK_MIN) ||
>> +		(val > SPEAR_ADC_CLK_MAX) ||
>> +		(val2 != 0)) {
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	count = (apb_clk + val - 1) / val;
>> +	clk_low = count / 2;
>> +	clk_high = count - clk_low;
>> +	info->current_clk = apb_clk / count;
>> +	spear_adc_set_clk(info, val);
>> +
> What is actually the purpose of clk_low and clk_high? I don't see them used after doing that calculation.
Good spot.  Looks like this is the result of an earlier reworking.  These get recomputed in the set_clk
function.

> And is it really 100% safe to avoid checking mask?
Fair point. No it's not. Oops.  Will fix and repost in a sec.
>> +out:
>> +	mutex_unlock(&indio_dev->mlock);
>> +	return ret;
>> +}
>> +
>>   #define SPEAR_ADC_CHAN(idx) {				\
>>   	.type = IIO_VOLTAGE,				\
>>   	.indexed = 1,					\
>>   	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
>>   	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
>> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
>>   	.channel = idx,					\
>>   }
>>
>> @@ -236,67 +271,9 @@ static int spear_adc_configure(struct spear_adc_info *info)
>>   	return 0;
>>   }
>>
>> -static ssize_t spear_adc_read_frequency(struct device *dev,
>> -					struct device_attribute *attr,
>> -					char *buf)
>> -{
>> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> -	struct spear_adc_info *info = iio_priv(indio_dev);
>> -
>> -	return sprintf(buf, "%d\n", info->current_clk);
>> -}
>> -
>> -static ssize_t spear_adc_write_frequency(struct device *dev,
>> -					 struct device_attribute *attr,
>> -					 const char *buf,
>> -					 size_t len)
>> -{
>> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> -	struct spear_adc_info *info = iio_priv(indio_dev);
>> -	u32 clk_high, clk_low, count;
>> -	u32 apb_clk = clk_get_rate(info->clk);
>> -	unsigned long lval;
>> -	int ret;
>> -
>> -	ret = kstrtoul(buf, 10, &lval);
>> -	if (ret)
>> -		return ret;
>> -
>> -	mutex_lock(&indio_dev->mlock);
>> -
>> -	if ((lval < SPEAR_ADC_CLK_MIN) || (lval > SPEAR_ADC_CLK_MAX)) {
>> -		ret = -EINVAL;
>> -		goto out;
>> -	}
>> -
>> -	count = (apb_clk + lval - 1) / lval;
>> -	clk_low = count / 2;
>> -	clk_high = count - clk_low;
>> -	info->current_clk = apb_clk / count;
>> -	spear_adc_set_clk(info, lval);
>> -
>> -out:
>> -	mutex_unlock(&indio_dev->mlock);
>> -
>> -	return ret ? ret : len;
>> -}
>> -
>> -static IIO_DEV_ATTR_SAMP_FREQ(S_IWUSR | S_IRUGO,
>> -			      spear_adc_read_frequency,
>> -			      spear_adc_write_frequency);
>> -
>> -static struct attribute *spear_attributes[] = {
>> -	&iio_dev_attr_sampling_frequency.dev_attr.attr,
>> -	NULL
>> -};
>> -
>> -static const struct attribute_group spear_attribute_group = {
>> -	.attrs = spear_attributes,
>> -};
>> -
>>   static const struct iio_info spear_adc_iio_info = {
>>   	.read_raw = &spear_read_raw,
>> -	.attrs = &spear_attribute_group,
>> +	.write_raw = &spear_adc_write_raw,
>>   	.driver_module = THIS_MODULE,
>>   };
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>