Commit 67b18dfb authored by Kai-Heng Feng's avatar Kai-Heng Feng Committed by Benjamin Tissoires
Browse files

HID: i2c-hid: Remove runtime power management 



Runtime power management in i2c-hid brings lots of issues, such as:
- When transitioning from display manager to desktop session, i2c-hid
was closed and opened, so the device was set to SLEEP and ON in a short
period. Vendors confirmed that their devices can't handle fast ON/SLEEP
command because Windows doesn't have this behavior.

- When rebooting, i2c-hid was closed, and the driver core put the device
back to full power before shutdown. This behavior also triggers a quick
SLEEP and ON commands that some devices can't handle, renders an
unusable touchpad after reboot.

- Most importantly, my power meter reports little to none energy saving
when i2c-hid is runtime suspended.

So let's remove runtime power management since there is no actual
benefit.

Signed-off-by: default avatarKai-Heng Feng <kai.heng.feng@canonical.com>
Acked-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarBenjamin Tissoires <benjamin.tissoires@redhat.com>
parent 16ff7bf6

drivers/hid/i2c-hid/i2c-hid-core.c

+7 −111
Original line number Original line Diff line number Diff line
@@ -26,7 +26,6 @@ 
#include <linux/delay.h>

#include <linux/delay.h>

#include <linux/slab.h>

#include <linux/slab.h>

#include <linux/pm.h>

#include <linux/pm.h>

#include <linux/pm_runtime.h>

#include <linux/device.h>

#include <linux/device.h>

#include <linux/wait.h>

#include <linux/wait.h>

#include <linux/err.h>

#include <linux/err.h>
@@ -48,8 +47,6 @@ 
/* quirks to control the device */

/* quirks to control the device */

#define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV	BIT(0)

#define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV	BIT(0)

#define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET	BIT(1)

#define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET	BIT(1)

#define I2C_HID_QUIRK_NO_RUNTIME_PM		BIT(2)

#define I2C_HID_QUIRK_DELAY_AFTER_SLEEP		BIT(3)

#define I2C_HID_QUIRK_BOGUS_IRQ			BIT(4)

#define I2C_HID_QUIRK_BOGUS_IRQ			BIT(4)





/* flags */

/* flags */
@@ -172,14 +169,7 @@ static const struct i2c_hid_quirks { 
	{ USB_VENDOR_ID_WEIDA, HID_ANY_ID,

	{ USB_VENDOR_ID_WEIDA, HID_ANY_ID,

		I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },

		I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },

	{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,

	{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,

		I2C_HID_QUIRK_NO_IRQ_AFTER_RESET |

		I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },

		I2C_HID_QUIRK_NO_RUNTIME_PM },

	{ I2C_VENDOR_ID_RAYDIUM, I2C_PRODUCT_ID_RAYDIUM_4B33,

		I2C_HID_QUIRK_DELAY_AFTER_SLEEP },

	{ USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,

		I2C_HID_QUIRK_NO_RUNTIME_PM },

	{ I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_01F0,

		I2C_HID_QUIRK_NO_RUNTIME_PM },

	{ USB_VENDOR_ID_ELAN, HID_ANY_ID,

	{ USB_VENDOR_ID_ELAN, HID_ANY_ID,

		 I2C_HID_QUIRK_BOGUS_IRQ },

		 I2C_HID_QUIRK_BOGUS_IRQ },

	{ 0, 0 }

	{ 0, 0 }
@@ -397,7 +387,6 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state) 
{

{

	struct i2c_hid *ihid = i2c_get_clientdata(client);

	struct i2c_hid *ihid = i2c_get_clientdata(client);

	int ret;

	int ret;

	unsigned long now, delay;





	i2c_hid_dbg(ihid, "%s\n", __func__);

	i2c_hid_dbg(ihid, "%s\n", __func__);
@@ -415,22 +404,9 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state) 
			goto set_pwr_exit;

			goto set_pwr_exit;

	}

	}





	if (ihid->quirks & I2C_HID_QUIRK_DELAY_AFTER_SLEEP &&

	    power_state == I2C_HID_PWR_ON) {

		now = jiffies;

		if (time_after(ihid->sleep_delay, now)) {

			delay = jiffies_to_usecs(ihid->sleep_delay - now);

			usleep_range(delay, delay + 1);

		}

	}



	ret = __i2c_hid_command(client, &hid_set_power_cmd, power_state,

	ret = __i2c_hid_command(client, &hid_set_power_cmd, power_state,

		0, NULL, 0, NULL, 0);

		0, NULL, 0, NULL, 0);





	if (ihid->quirks & I2C_HID_QUIRK_DELAY_AFTER_SLEEP &&

	    power_state == I2C_HID_PWR_SLEEP)

		ihid->sleep_delay = jiffies + msecs_to_jiffies(20);



	if (ret)

	if (ret)

		dev_err(&client->dev, "failed to change power setting.\n");

		dev_err(&client->dev, "failed to change power setting.\n");
@@ -791,11 +767,6 @@ static int i2c_hid_open(struct hid_device *hid) 
{

{

	struct i2c_client *client = hid->driver_data;

	struct i2c_client *client = hid->driver_data;

	struct i2c_hid *ihid = i2c_get_clientdata(client);

	struct i2c_hid *ihid = i2c_get_clientdata(client);

	int ret = 0;



	ret = pm_runtime_get_sync(&client->dev);

	if (ret < 0)

		return ret;





	set_bit(I2C_HID_STARTED, &ihid->flags);

	set_bit(I2C_HID_STARTED, &ihid->flags);

	return 0;

	return 0;
@@ -807,27 +778,6 @@ static void i2c_hid_close(struct hid_device *hid) 
	struct i2c_hid *ihid = i2c_get_clientdata(client);

	struct i2c_hid *ihid = i2c_get_clientdata(client);





	clear_bit(I2C_HID_STARTED, &ihid->flags);

	clear_bit(I2C_HID_STARTED, &ihid->flags);



	/* Save some power */

	pm_runtime_put(&client->dev);

}



static int i2c_hid_power(struct hid_device *hid, int lvl)

{

	struct i2c_client *client = hid->driver_data;

	struct i2c_hid *ihid = i2c_get_clientdata(client);



	i2c_hid_dbg(ihid, "%s lvl:%d\n", __func__, lvl);



	switch (lvl) {

	case PM_HINT_FULLON:

		pm_runtime_get_sync(&client->dev);

		break;

	case PM_HINT_NORMAL:

		pm_runtime_put(&client->dev);

		break;

	}

	return 0;

}

}





struct hid_ll_driver i2c_hid_ll_driver = {

struct hid_ll_driver i2c_hid_ll_driver = {
@@ -836,7 +786,6 @@ struct hid_ll_driver i2c_hid_ll_driver = { 
	.stop = i2c_hid_stop,

	.stop = i2c_hid_stop,

	.open = i2c_hid_open,

	.open = i2c_hid_open,

	.close = i2c_hid_close,

	.close = i2c_hid_close,

	.power = i2c_hid_power,

	.output_report = i2c_hid_output_report,

	.output_report = i2c_hid_output_report,

	.raw_request = i2c_hid_raw_request,

	.raw_request = i2c_hid_raw_request,

};

};
@@ -1104,9 +1053,6 @@ static int i2c_hid_probe(struct i2c_client *client, 




	i2c_hid_acpi_fix_up_power(&client->dev);

	i2c_hid_acpi_fix_up_power(&client->dev);





	pm_runtime_get_noresume(&client->dev);

	pm_runtime_set_active(&client->dev);

	pm_runtime_enable(&client->dev);

	device_enable_async_suspend(&client->dev);

	device_enable_async_suspend(&client->dev);





	/* Make sure there is something at this address */

	/* Make sure there is something at this address */
@@ -1114,16 +1060,16 @@ static int i2c_hid_probe(struct i2c_client *client, 
	if (ret < 0) {

	if (ret < 0) {

		dev_dbg(&client->dev, "nothing at this address: %d\n", ret);

		dev_dbg(&client->dev, "nothing at this address: %d\n", ret);

		ret = -ENXIO;

		ret = -ENXIO;

		goto err_pm;

		goto err_regulator;

	}

	}





	ret = i2c_hid_fetch_hid_descriptor(ihid);

	ret = i2c_hid_fetch_hid_descriptor(ihid);

	if (ret < 0)

	if (ret < 0)

		goto err_pm;

		goto err_regulator;





	ret = i2c_hid_init_irq(client);

	ret = i2c_hid_init_irq(client);

	if (ret < 0)

	if (ret < 0)

		goto err_pm;

		goto err_regulator;





	hid = hid_allocate_device();

	hid = hid_allocate_device();

	if (IS_ERR(hid)) {

	if (IS_ERR(hid)) {
@@ -1154,9 +1100,6 @@ static int i2c_hid_probe(struct i2c_client *client, 
		goto err_mem_free;

		goto err_mem_free;

	}

	}





	if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))

		pm_runtime_put(&client->dev);



	return 0;

	return 0;





err_mem_free:

err_mem_free:
@@ -1165,10 +1108,6 @@ err_mem_free: 
err_irq:

err_irq:

	free_irq(client->irq, ihid);

	free_irq(client->irq, ihid);





err_pm:

	pm_runtime_put_noidle(&client->dev);

	pm_runtime_disable(&client->dev);



err_regulator:

err_regulator:

	regulator_bulk_disable(ARRAY_SIZE(ihid->pdata.supplies),

	regulator_bulk_disable(ARRAY_SIZE(ihid->pdata.supplies),

			       ihid->pdata.supplies);

			       ihid->pdata.supplies);
@@ -1181,12 +1120,6 @@ static int i2c_hid_remove(struct i2c_client *client) 
	struct i2c_hid *ihid = i2c_get_clientdata(client);

	struct i2c_hid *ihid = i2c_get_clientdata(client);

	struct hid_device *hid;

	struct hid_device *hid;





	if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))

		pm_runtime_get_sync(&client->dev);

	pm_runtime_disable(&client->dev);

	pm_runtime_set_suspended(&client->dev);

	pm_runtime_put_noidle(&client->dev);



	hid = ihid->hid;

	hid = ihid->hid;

	hid_destroy_device(hid);

	hid_destroy_device(hid);
@@ -1219,25 +1152,15 @@ static int i2c_hid_suspend(struct device *dev) 
	int wake_status;

	int wake_status;





	if (hid->driver && hid->driver->suspend) {

	if (hid->driver && hid->driver->suspend) {

		/*

		 * Wake up the device so that IO issues in

		 * HID driver's suspend code can succeed.

		 */

		ret = pm_runtime_resume(dev);

		if (ret < 0)

			return ret;



		ret = hid->driver->suspend(hid, PMSG_SUSPEND);

		ret = hid->driver->suspend(hid, PMSG_SUSPEND);

		if (ret < 0)

		if (ret < 0)

			return ret;

			return ret;

	}

	}





	if (!pm_runtime_suspended(dev)) {

	/* Save some power */

	/* Save some power */

	i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);

	i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);





	disable_irq(client->irq);

	disable_irq(client->irq);

	}





	if (device_may_wakeup(&client->dev)) {

	if (device_may_wakeup(&client->dev)) {

		wake_status = enable_irq_wake(client->irq);

		wake_status = enable_irq_wake(client->irq);
@@ -1279,11 +1202,6 @@ static int i2c_hid_resume(struct device *dev) 
				wake_status);

				wake_status);

	}

	}





	/* We'll resume to full power */

	pm_runtime_disable(dev);

	pm_runtime_set_active(dev);

	pm_runtime_enable(dev);



	enable_irq(client->irq);

	enable_irq(client->irq);





	/* Instead of resetting device, simply powers the device on. This

	/* Instead of resetting device, simply powers the device on. This
@@ -1304,30 +1222,8 @@ static int i2c_hid_resume(struct device *dev) 
}

}

#endif

#endif





#ifdef CONFIG_PM

static int i2c_hid_runtime_suspend(struct device *dev)

{

	struct i2c_client *client = to_i2c_client(dev);



	i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);

	disable_irq(client->irq);

	return 0;

}



static int i2c_hid_runtime_resume(struct device *dev)

{

	struct i2c_client *client = to_i2c_client(dev);



	enable_irq(client->irq);

	i2c_hid_set_power(client, I2C_HID_PWR_ON);

	return 0;

}

#endif



static const struct dev_pm_ops i2c_hid_pm = {

static const struct dev_pm_ops i2c_hid_pm = {

	SET_SYSTEM_SLEEP_PM_OPS(i2c_hid_suspend, i2c_hid_resume)

	SET_SYSTEM_SLEEP_PM_OPS(i2c_hid_suspend, i2c_hid_resume)

	SET_RUNTIME_PM_OPS(i2c_hid_runtime_suspend, i2c_hid_runtime_resume,

			   NULL)

};

};





static const struct i2c_device_id i2c_hid_id_table[] = {

static const struct i2c_device_id i2c_hid_id_table[] = {

CRA Git | Maintained and supported by SUSTech CRA and CCSE