Re: [PATCH v2 1/2] Staging: iio: Place driver in sleep mode on error

Re: [PATCH v2 1/2] Staging: iio: Place driver in sleep mode on error

[Jonathan Cameron]

On 22/03/15 00:26, Cristina Opriceana wrote:
> Put device in sleep mode if an error is encountered after
> initialization in order to avoid wasting power.
The patch title should definitely include the name of the driver in question!
Otherwise you'll not generally get reviews from the relevant people.
Staging:iio:hmc5843 Place driver in sleep mode on error during probe.

Also try to cc relevant people and the relevant list.
See MAINTAINERS and you'll notice we have both a list and a
set of designated reviewers who would like to be cc'd on IIO patches.
They may not respond, but if you don't send it to them then they
definitely won't!



Jonathan

> 
> Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com>
> ---
>  drivers/staging/iio/magnetometer/hmc5843_core.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/magnetometer/hmc5843_core.c b/drivers/staging/iio/magnetometer/hmc5843_core.c
> index 0074af2..7de19c2 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843_core.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843_core.c
> @@ -611,7 +611,7 @@ int hmc5843_common_probe(struct device *dev, struct regmap *regmap,
>  	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>  					 hmc5843_trigger_handler, NULL);
>  	if (ret < 0)
> -		return ret;
> +		goto buffer_setup_err;
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret < 0)
> @@ -619,8 +619,12 @@ int hmc5843_common_probe(struct device *dev, struct regmap *regmap,
>  
>  	return 0;
>  
> +buffer_setup_err:
> +	hmc5843_set_mode(iio_priv(indio_dev), HMC5843_MODE_SLEEP);
> +	return ret;
>  buffer_cleanup:
>  	iio_triggered_buffer_cleanup(indio_dev);
> +	hmc5843_set_mode(iio_priv(indio_dev), HMC5843_MODE_SLEEP);
>  	return ret;
Single exit path that reverses the ordering of creation.

e.g.
buffer_setup_err:
	hmc_set_mode(iio_priv(indio_dev), HMC5843_MODE_SLEEP);
buffer_cleanup:
	iio_triggered_buffer_cleanup(indio_dev);
	return ret;
>  }
>  EXPORT_SYMBOL(hmc5843_common_probe);
> 

Re: [PATCH v2 1/2] Staging: iio: Place driver in sleep mode on error

[Cristina Georgiana Opriceana]

On Sat, Mar 28, 2015 at 2:13 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 22/03/15 00:26, Cristina Opriceana wrote:
>> Put device in sleep mode if an error is encountered after
>> initialization in order to avoid wasting power.
> The patch title should definitely include the name of the driver in question!
> Otherwise you'll not generally get reviews from the relevant people.
> Staging:iio:hmc5843 Place driver in sleep mode on error during probe.
>
> Also try to cc relevant people and the relevant list.
> See MAINTAINERS and you'll notice we have both a list and a
> set of designated reviewers who would like to be cc'd on IIO patches.

Okay, thanks for your feedback, I'll keep that in mind for the future work
because Greg has already merged this on his staging-next branch.

Thanks,
Cristina

Re: [PATCH v2 1/2] Staging: iio: Place driver in sleep mode on error

[Jonathan Cameron]

On 28/03/15 13:10, Cristina Georgiana Opriceana wrote:
> On Sat, Mar 28, 2015 at 2:13 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 22/03/15 00:26, Cristina Opriceana wrote:
>>> Put device in sleep mode if an error is encountered after
>>> initialization in order to avoid wasting power.
>> The patch title should definitely include the name of the driver in question!
>> Otherwise you'll not generally get reviews from the relevant people.
>> Staging:iio:hmc5843 Place driver in sleep mode on error during probe.
>>
>> Also try to cc relevant people and the relevant list.
>> See MAINTAINERS and you'll notice we have both a list and a
>> set of designated reviewers who would like to be cc'd on IIO patches.
> 
> Okay, thanks for your feedback, I'll keep that in mind for the future work
> because Greg has already merged this on his staging-next branch.
Fair enough!

J
> 
> Thanks,
> Cristina
> --
> 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
>