Hi Benson,
> cyapa_check_is_operational and cyapa_create_input_dev are common to
> the probe and firmware update paths. Pull those out into
> cyapa_detect.
>
> Signed-off-by: Benson Leung <bleung@xxxxxxxxxxxx>
> ---
> drivers/input/mouse/cyapa.c | 57 +++++++++++++++++++++++++++++++--------------
> 1 file changed, 39 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
> index b409c3d..a631aca 100644
> --- a/drivers/input/mouse/cyapa.c
> +++ b/drivers/input/mouse/cyapa.c
> @@ -823,6 +823,40 @@ err_free_device:
> return ret;
> }
>
> +static void cyapa_detect(struct cyapa *cyapa)
No error return here?
> +{
> + struct device *dev = &cyapa->client->dev;
> + int ret;
> +
> + ret = cyapa_check_is_operational(cyapa);
> + if (ret == -ETIMEDOUT)
> + dev_err(dev, "no device detected, %d\n", ret);
> + else if (ret)
> + dev_err(dev, "device detected, but not operational, %d\n", ret);
> +
> + if (!cyapa->input) {
Return here saves you the ugly indentation.
> + ret = cyapa_create_input_dev(cyapa);
> + if (ret)
> + dev_err(dev, "create input_dev instance failed, %d\n",
> + ret);
> +
> + enable_irq(cyapa->irq);
> + /*
> + * On some systems, a system crash / warm boot does not reset
> + * the device's current power mode to FULL_ACTIVE.
> + * If such an event happens during suspend, after the device
> + * has been put in a low power mode, the device will still be
> + * in low power mode on a subsequent boot, since there was
> + * never a matching resume().
> + * Handle this by always forcing full power here, when a
> + * device is first detected to be in operational mode.
> + */
> + ret = cyapa_set_power_mode(cyapa, PWR_MODE_FULL_ACTIVE);
> + if (ret)
> + dev_warn(dev, "set active power failed, %d\n", ret);
> + }
> +}
> +
> static int cyapa_probe(struct i2c_client *client,
> const struct i2c_device_id *dev_id)
> {
> @@ -853,23 +887,8 @@ static int cyapa_probe(struct i2c_client *client,
> if (adapter_func == CYAPA_ADAPTER_FUNC_SMBUS)
> cyapa->smbus = true;
> cyapa->state = CYAPA_STATE_NO_DEVICE;
> - ret = cyapa_check_is_operational(cyapa);
> - if (ret) {
> - dev_err(dev, "device not operational, %d\n", ret);
> - goto err_mem_free;
> - }
>
> - ret = cyapa_create_input_dev(cyapa);
> - if (ret) {
> - dev_err(dev, "create input_dev instance failed, %d\n", ret);
> - goto err_mem_free;
> - }
> -
> - ret = cyapa_set_power_mode(cyapa, PWR_MODE_FULL_ACTIVE);
> - if (ret) {
> - dev_err(dev, "set active power failed, %d\n", ret);
> - goto err_unregister_device;
> - }
> + cyapa_detect(cyapa);
Replacing proper error handling with a silent function seems wrong.
>
> cyapa->irq = client->irq;
> ret = request_threaded_irq(cyapa->irq,
> @@ -886,8 +905,8 @@ static int cyapa_probe(struct i2c_client *client,
> return 0;
>
> err_unregister_device:
> - input_unregister_device(cyapa->input);
> -err_mem_free:
> + if (cyapa->input)
> + input_unregister_device(cyapa->input);
> kfree(cyapa);
>
> return ret;
> @@ -937,6 +956,8 @@ static int cyapa_resume(struct device *dev)
> if (device_may_wakeup(dev) && cyapa->irq_wake)
> disable_irq_wake(cyapa->irq);
>
> + cyapa_detect(cyapa);
> +
Ditto.
> ret = cyapa_set_power_mode(cyapa, PWR_MODE_FULL_ACTIVE);
> if (ret)
> dev_warn(dev, "resume active power failed, %d\n", ret);
> --
> 1.8.1.3
>
Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
