Re: [PATCH v7 08/13] regmap: add SLIMbus support

[Srinivas Kandagatla <srinivas.kandagatla@linaro.org>]

Thanks for the Review,

On 23/11/17 07:49, Charles Keepax wrote:
> On Wed, Nov 15, 2017 at 02:10:38PM +0000, srinivas.kandagatla@linaro.org wrote:
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patch adds support to read/write slimbus value elements.
>> Currently it only supports byte read/write. Adding this support in
>> regmap would give codec drivers more flexibility when there are more
>> than 2 control interfaces like slimbus, i2c.
>>
>> Without this patch each codec driver has to directly call slimbus value
>> element apis, and this could would get messy once we want to add i2c
>> interface to it.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>> +
>> +static int regmap_slimbus_byte_reg_read(void *context, unsigned int reg,
>> +					unsigned int *val)
>> +{
>> +	struct slim_device *sdev = context;
>> +	int v;
>> +
>> +	if (!sdev)
>> +		return 0;
> 
> Is there a specific reason we are checking the context here? The
> other regmap buses don't both and whilst I don't mind checking
> shouldn't we return an error if we don't have a context rather
> than pretending to succeed?
looks like over done here, I will fix it in next version.

> 
>> +
>> +	v = slim_readb(sdev, reg);
>> +
>> +	if (v < 0)
>> +		return v;
>> +
>> +	*val = v;
>> +
>> +	return 0;
>> +}
>> +
>> +static int regmap_slimbus_byte_reg_write(void *context, unsigned int reg,
>> +					 unsigned int val)
>> +{
>> +	struct slim_device *sdev = context;
>> +
>> +	if (!sdev)
>> +		return 0;
> 
> ditto.
> 
> Thanks,
> Charles
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>