Re: [PATCH 03/11] Input: synaptics-rmi4 - do not update configuration in rmi_f01_probe()

[Christopher Heiny <cheiny@synaptics.com>]

On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> Do not write configuration data in probe(), we have config() for that.

Then we should call config() in rmi_function_probe() to ensure that
any platform data or device tree configuration settings get written
to the device.

Thinking about that, it looks like it's not fatal if the config
write fails in that situation.  The device might not function as
intended, but you can hopefully get some use out of it (for
instance, a phone's touchscreen sensitivity might be wacky, but
the user will still be able to dial tech support).

For example:

static int rmi_function_probe(struct device *dev)
{
	struct rmi_function *fn = to_rmi_function(dev);
	struct rmi_function_handler *handler = 			 
to_rmi_function_handler(dev->driver);
	int error;

	if (handler->probe) {
		error = handler->probe(fn);
		if (error)
			return error;
	}
	if (handler->config) {
		error = handler->config(fn);
		if (error)
			dev_warn(dev, "Driver config failed.\n");
	}

	return 0;
}

>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>   drivers/input/rmi4/rmi_f01.c | 18 ------------------
>   1 file changed, 18 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 897d8ac..976aba3 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -303,12 +303,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
>   		if (pdata->power_management.doze_interval) {
>   			data->device_control.doze_interval =
>   				pdata->power_management.doze_interval;
> -			error = rmi_write(rmi_dev, data->doze_interval_addr,
> -					data->device_control.doze_interval);
> -			if (error < 0) {
> -				dev_err(&fn->dev, "Failed to configure F01 doze interval register.\n");
> -				return error;
> -			}
>   		} else {
>   			error = rmi_read(rmi_dev, data->doze_interval_addr,
>   					&data->device_control.doze_interval);
> @@ -324,12 +318,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
>   		if (pdata->power_management.wakeup_threshold) {
>   			data->device_control.wakeup_threshold =
>   				pdata->power_management.wakeup_threshold;
> -			error = rmi_write(rmi_dev, data->wakeup_threshold_addr,
> -					data->device_control.wakeup_threshold);
> -			if (error < 0) {
> -				dev_err(&fn->dev, "Failed to configure F01 wakeup threshold register.\n");
> -				return error;
> -			}
>   		} else {
>   			error = rmi_read(rmi_dev, data->wakeup_threshold_addr,
>   					&data->device_control.wakeup_threshold);
> @@ -347,12 +335,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
>   		if (pdata->power_management.doze_holdoff) {
>   			data->device_control.doze_holdoff =
>   				pdata->power_management.doze_holdoff;
> -			error = rmi_write(rmi_dev, data->doze_holdoff_addr,
> -					data->device_control.doze_holdoff);
> -			if (error < 0) {
> -				dev_err(&fn->dev, "Failed to configure F01 doze holdoff register.\n");
> -				return error;
> -			}
>   		} else {
>   			error = rmi_read(rmi_dev, data->doze_holdoff_addr,
>   					&data->device_control.doze_holdoff);
>


-- 

Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated