* [PATCH 19/22] Input: s6sy761 - use device core to create driver-specific device attributes
From: Dmitry Torokhov @ 2023-07-29 0:51 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Greg Kroah-Hartman, Andi Shyti
In-Reply-To: <20230729005133.1095051-1-dmitry.torokhov@gmail.com>
Instead of creating driver-specific device attributes with
devm_device_add_group() have device core do this by setting up dev_groups
pointer in the driver structure.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/s6sy761.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/input/touchscreen/s6sy761.c b/drivers/input/touchscreen/s6sy761.c
index 998d99d18911..149cc2c4925e 100644
--- a/drivers/input/touchscreen/s6sy761.c
+++ b/drivers/input/touchscreen/s6sy761.c
@@ -286,10 +286,7 @@ static struct attribute *s6sy761_sysfs_attrs[] = {
&dev_attr_devid.attr,
NULL
};
-
-static struct attribute_group s6sy761_attribute_group = {
- .attrs = s6sy761_sysfs_attrs
-};
+ATTRIBUTE_GROUPS(s6sy761_sysfs);
static int s6sy761_power_on(struct s6sy761_data *sdata)
{
@@ -465,10 +462,6 @@ static int s6sy761_probe(struct i2c_client *client)
if (err)
return err;
- err = devm_device_add_group(&client->dev, &s6sy761_attribute_group);
- if (err)
- return err;
-
pm_runtime_enable(&client->dev);
return 0;
@@ -535,6 +528,7 @@ MODULE_DEVICE_TABLE(i2c, s6sy761_id);
static struct i2c_driver s6sy761_driver = {
.driver = {
.name = S6SY761_DEV_NAME,
+ .dev_groups = s6sy761_sysfs_groups,
.of_match_table = of_match_ptr(s6sy761_of_match),
.pm = pm_ptr(&s6sy761_pm_ops),
},
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH 18/22] Input: rohm_bu21023 - use device core to create driver-specific device attributes
From: Dmitry Torokhov @ 2023-07-29 0:51 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Greg Kroah-Hartman
In-Reply-To: <20230729005133.1095051-1-dmitry.torokhov@gmail.com>
Instead of creating driver-specific device attributes with
devm_device_add_group() have device core do this by setting up dev_groups
pointer in the driver structure.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/rohm_bu21023.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/rohm_bu21023.c b/drivers/input/touchscreen/rohm_bu21023.c
index 240424f06b98..4493ad0c9322 100644
--- a/drivers/input/touchscreen/rohm_bu21023.c
+++ b/drivers/input/touchscreen/rohm_bu21023.c
@@ -854,10 +854,7 @@ static struct attribute *rohm_ts_attrs[] = {
&dev_attr_inv_y.attr,
NULL,
};
-
-static const struct attribute_group rohm_ts_attr_group = {
- .attrs = rohm_ts_attrs,
-};
+ATTRIBUTE_GROUPS(rohm_ts);
static int rohm_ts_device_init(struct i2c_client *client, u8 setup2)
{
@@ -1164,12 +1161,6 @@ static int rohm_bu21023_i2c_probe(struct i2c_client *client)
return error;
}
- error = devm_device_add_group(dev, &rohm_ts_attr_group);
- if (error) {
- dev_err(dev, "failed to create sysfs group: %d\n", error);
- return error;
- }
-
return error;
}
@@ -1182,6 +1173,7 @@ MODULE_DEVICE_TABLE(i2c, rohm_bu21023_i2c_id);
static struct i2c_driver rohm_bu21023_i2c_driver = {
.driver = {
.name = BU21023_NAME,
+ .dev_groups = rohm_ts_groups,
},
.probe = rohm_bu21023_i2c_probe,
.id_table = rohm_bu21023_i2c_id,
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH 17/22] Input: raydium_i2c_ts - use device core to create driver-specific device attributes
From: Dmitry Torokhov @ 2023-07-29 0:51 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Greg Kroah-Hartman, Raul E Rangel
In-Reply-To: <20230729005133.1095051-1-dmitry.torokhov@gmail.com>
Instead of creating driver-specific device attributes with
devm_device_add_group() have device core do this by setting up dev_groups
pointer in the driver structure.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/raydium_i2c_ts.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/drivers/input/touchscreen/raydium_i2c_ts.c b/drivers/input/touchscreen/raydium_i2c_ts.c
index 78dd3059d585..13c500e776f6 100644
--- a/drivers/input/touchscreen/raydium_i2c_ts.c
+++ b/drivers/input/touchscreen/raydium_i2c_ts.c
@@ -1004,7 +1004,7 @@ static DEVICE_ATTR(boot_mode, S_IRUGO, raydium_i2c_boot_mode_show, NULL);
static DEVICE_ATTR(update_fw, S_IWUSR, NULL, raydium_i2c_update_fw_store);
static DEVICE_ATTR(calibrate, S_IWUSR, NULL, raydium_i2c_calibrate_store);
-static struct attribute *raydium_i2c_attributes[] = {
+static struct attribute *raydium_i2c_attrs[] = {
&dev_attr_update_fw.attr,
&dev_attr_boot_mode.attr,
&dev_attr_fw_version.attr,
@@ -1012,10 +1012,7 @@ static struct attribute *raydium_i2c_attributes[] = {
&dev_attr_calibrate.attr,
NULL
};
-
-static const struct attribute_group raydium_i2c_attribute_group = {
- .attrs = raydium_i2c_attributes,
-};
+ATTRIBUTE_GROUPS(raydium_i2c);
static int raydium_i2c_power_on(struct raydium_data *ts)
{
@@ -1174,14 +1171,6 @@ static int raydium_i2c_probe(struct i2c_client *client)
return error;
}
- error = devm_device_add_group(&client->dev,
- &raydium_i2c_attribute_group);
- if (error) {
- dev_err(&client->dev, "failed to create sysfs attributes: %d\n",
- error);
- return error;
- }
-
return 0;
}
@@ -1265,6 +1254,7 @@ static struct i2c_driver raydium_i2c_driver = {
.id_table = raydium_i2c_id,
.driver = {
.name = "raydium_ts",
+ .dev_groups = raydium_i2c_groups,
.pm = pm_sleep_ptr(&raydium_i2c_pm_ops),
.acpi_match_table = ACPI_PTR(raydium_acpi_id),
.of_match_table = of_match_ptr(raydium_of_match),
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH 15/22] Input: iqs5xx - use device core to create driver-specific device attributes
From: Dmitry Torokhov @ 2023-07-29 0:51 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Greg Kroah-Hartman, Jeff LaBundy
In-Reply-To: <20230729005133.1095051-1-dmitry.torokhov@gmail.com>
Instead of creating driver-specific device attributes with
devm_device_add_group() have device core do this by setting up dev_groups
pointer in the driver structure.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/iqs5xx.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/input/touchscreen/iqs5xx.c b/drivers/input/touchscreen/iqs5xx.c
index b4768b66eb10..a3f4fb85bee5 100644
--- a/drivers/input/touchscreen/iqs5xx.c
+++ b/drivers/input/touchscreen/iqs5xx.c
@@ -974,10 +974,11 @@ static umode_t iqs5xx_attr_is_visible(struct kobject *kobj,
return attr->mode;
}
-static const struct attribute_group iqs5xx_attr_group = {
+static const struct attribute_group iqs5xx_group = {
.is_visible = iqs5xx_attr_is_visible,
.attrs = iqs5xx_attrs,
};
+__ATTRIBUTE_GROUPS(iqs5xx);
static int iqs5xx_suspend(struct device *dev)
{
@@ -1053,12 +1054,6 @@ static int iqs5xx_probe(struct i2c_client *client)
return error;
}
- error = devm_device_add_group(&client->dev, &iqs5xx_attr_group);
- if (error) {
- dev_err(&client->dev, "Failed to add attributes: %d\n", error);
- return error;
- }
-
if (iqs5xx->input) {
error = input_register_device(iqs5xx->input);
if (error)
@@ -1089,6 +1084,7 @@ MODULE_DEVICE_TABLE(of, iqs5xx_of_match);
static struct i2c_driver iqs5xx_i2c_driver = {
.driver = {
.name = "iqs5xx",
+ .dev_groups = iqs5xx_groups,
.of_match_table = iqs5xx_of_match,
.pm = pm_sleep_ptr(&iqs5xx_pm),
},
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH 16/22] Input: melfas-mip4 - use device core to create driver-specific device attributes
From: Dmitry Torokhov @ 2023-07-29 0:51 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Greg Kroah-Hartman, JungHoon Hyun
In-Reply-To: <20230729005133.1095051-1-dmitry.torokhov@gmail.com>
Instead of creating driver-specific device attributes with
devm_device_add_group() have device core do this by setting up dev_groups
pointer in the driver structure.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/melfas_mip4.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/input/touchscreen/melfas_mip4.c b/drivers/input/touchscreen/melfas_mip4.c
index 2ac4483fbc25..aa325486f618 100644
--- a/drivers/input/touchscreen/melfas_mip4.c
+++ b/drivers/input/touchscreen/melfas_mip4.c
@@ -1419,10 +1419,7 @@ static struct attribute *mip4_attrs[] = {
&dev_attr_update_fw.attr,
NULL,
};
-
-static const struct attribute_group mip4_attr_group = {
- .attrs = mip4_attrs,
-};
+ATTRIBUTE_GROUPS(mip4);
static int mip4_probe(struct i2c_client *client)
{
@@ -1514,13 +1511,6 @@ static int mip4_probe(struct i2c_client *client)
return error;
}
- error = devm_device_add_group(&client->dev, &mip4_attr_group);
- if (error) {
- dev_err(&client->dev,
- "Failed to create sysfs attribute group: %d\n", error);
- return error;
- }
-
return 0;
}
@@ -1589,6 +1579,7 @@ static struct i2c_driver mip4_driver = {
.probe = mip4_probe,
.driver = {
.name = MIP4_DEVICE_NAME,
+ .dev_groups = mip4_groups,
.of_match_table = of_match_ptr(mip4_of_match),
.acpi_match_table = ACPI_PTR(mip4_acpi_match),
.pm = pm_sleep_ptr(&mip4_pm_ops),
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH 14/22] Input: ilitek_ts_i2c - use device core to create driver-specific device attributes
From: Dmitry Torokhov @ 2023-07-29 0:51 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Greg Kroah-Hartman, Joe Hung
In-Reply-To: <20230729005133.1095051-1-dmitry.torokhov@gmail.com>
Instead of creating driver-specific device attributes with
devm_device_add_group() have device core do this by setting up dev_groups
pointer in the driver structure.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/ilitek_ts_i2c.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
index 2f872e95fbba..90c4934e750a 100644
--- a/drivers/input/touchscreen/ilitek_ts_i2c.c
+++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
@@ -537,10 +537,7 @@ static struct attribute *ilitek_sysfs_attrs[] = {
&dev_attr_product_id.attr,
NULL
};
-
-static struct attribute_group ilitek_attrs_group = {
- .attrs = ilitek_sysfs_attrs,
-};
+ATTRIBUTE_GROUPS(ilitek_sysfs);
static int ilitek_ts_i2c_probe(struct i2c_client *client)
{
@@ -595,12 +592,6 @@ static int ilitek_ts_i2c_probe(struct i2c_client *client)
return error;
}
- error = devm_device_add_group(dev, &ilitek_attrs_group);
- if (error) {
- dev_err(dev, "sysfs create group failed: %d\n", error);
- return error;
- }
-
return 0;
}
@@ -675,6 +666,7 @@ MODULE_DEVICE_TABLE(of, ilitek_ts_i2c_match);
static struct i2c_driver ilitek_ts_i2c_driver = {
.driver = {
.name = ILITEK_TS_NAME,
+ .dev_groups = ilitek_sysfs_groups,
.pm = pm_sleep_ptr(&ilitek_pm_ops),
.of_match_table = of_match_ptr(ilitek_ts_i2c_match),
.acpi_match_table = ACPI_PTR(ilitekts_acpi_id),
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH 05/22] Input: ad7877 - use device core to create driver-specific device attributes
From: Dmitry Torokhov @ 2023-07-29 0:51 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Greg Kroah-Hartman, Michael Hennerich
In-Reply-To: <20230729005133.1095051-1-dmitry.torokhov@gmail.com>
Instead of creating driver-specific device attributes with
devm_device_add_group() have device core do this by setting up dev_groups
pointer in the driver structure.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/ad7877.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index edb36d663f22..a0598e9c7aff 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -612,10 +612,11 @@ static umode_t ad7877_attr_is_visible(struct kobject *kobj,
return mode;
}
-static const struct attribute_group ad7877_attr_group = {
+static const struct attribute_group ad7877_group = {
.is_visible = ad7877_attr_is_visible,
.attrs = ad7877_attributes,
};
+__ATTRIBUTE_GROUPS(ad7877);
static void ad7877_setup_ts_def_msg(struct spi_device *spi, struct ad7877 *ts)
{
@@ -777,10 +778,6 @@ static int ad7877_probe(struct spi_device *spi)
return err;
}
- err = devm_device_add_group(&spi->dev, &ad7877_attr_group);
- if (err)
- return err;
-
err = input_register_device(input_dev);
if (err)
return err;
@@ -810,8 +807,9 @@ static DEFINE_SIMPLE_DEV_PM_OPS(ad7877_pm, ad7877_suspend, ad7877_resume);
static struct spi_driver ad7877_driver = {
.driver = {
- .name = "ad7877",
- .pm = pm_sleep_ptr(&ad7877_pm),
+ .name = "ad7877",
+ .dev_groups = ad7877_groups,
+ .pm = pm_sleep_ptr(&ad7877_pm),
},
.probe = ad7877_probe,
};
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH 04/22] Input: kxtj9 - use device core to create driver-specific device attributes
From: Dmitry Torokhov @ 2023-07-29 0:51 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Greg Kroah-Hartman
In-Reply-To: <20230729005133.1095051-1-dmitry.torokhov@gmail.com>
Instead of creating driver-specific device attributes with
devm_device_add_group() have device core do this by setting up dev_groups
pointer in the driver structure.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/kxtj9.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
index 912e614d039d..d47269b10e9a 100644
--- a/drivers/input/misc/kxtj9.c
+++ b/drivers/input/misc/kxtj9.c
@@ -334,14 +334,25 @@ static ssize_t kxtj9_set_poll(struct device *dev, struct device_attribute *attr,
static DEVICE_ATTR(poll, S_IRUGO|S_IWUSR, kxtj9_get_poll, kxtj9_set_poll);
-static struct attribute *kxtj9_attributes[] = {
+static struct attribute *kxtj9_attrs[] = {
&dev_attr_poll.attr,
NULL
};
-static struct attribute_group kxtj9_attribute_group = {
- .attrs = kxtj9_attributes
+static umode_t kxtj9_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int n)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct i2c_client *client = to_i2c_client(dev);
+
+ return client->irq ? attr->mode : 0;
+}
+
+static struct attribute_group kxtj9_group = {
+ .attrs = kxtj9_attrs,
+ .is_visible = kxtj9_attr_is_visible,
};
+__ATTRIBUTE_GROUPS(kxtj9);
static void kxtj9_poll(struct input_dev *input)
{
@@ -482,13 +493,6 @@ static int kxtj9_probe(struct i2c_client *client)
dev_err(&client->dev, "request irq failed: %d\n", err);
return err;
}
-
- err = devm_device_add_group(&client->dev,
- &kxtj9_attribute_group);
- if (err) {
- dev_err(&client->dev, "sysfs create failed: %d\n", err);
- return err;
- }
}
return 0;
@@ -535,8 +539,9 @@ MODULE_DEVICE_TABLE(i2c, kxtj9_id);
static struct i2c_driver kxtj9_driver = {
.driver = {
- .name = NAME,
- .pm = pm_sleep_ptr(&kxtj9_pm_ops),
+ .name = NAME,
+ .dev_groups = kxtj9_groups,
+ .pm = pm_sleep_ptr(&kxtj9_pm_ops),
},
.probe = kxtj9_probe,
.id_table = kxtj9_id,
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH 03/22] Input: iqs269a - use device core to create driver-specific device attributes
From: Dmitry Torokhov @ 2023-07-29 0:51 UTC (permalink / raw)
To: linux-input
Cc: linux-kernel, Greg Kroah-Hartman, Jeff LaBundy,
Mattijs Korpershoek
In-Reply-To: <20230729005133.1095051-1-dmitry.torokhov@gmail.com>
Instead of creating driver-specific device attributes with
devm_device_add_group() have device core do this by setting up dev_groups
pointer in the driver structure.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/iqs269a.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/input/misc/iqs269a.c b/drivers/input/misc/iqs269a.c
index c0a085639870..3c636c75e8a1 100644
--- a/drivers/input/misc/iqs269a.c
+++ b/drivers/input/misc/iqs269a.c
@@ -1586,10 +1586,7 @@ static struct attribute *iqs269_attrs[] = {
&dev_attr_ati_trigger.attr,
NULL,
};
-
-static const struct attribute_group iqs269_attr_group = {
- .attrs = iqs269_attrs,
-};
+ATTRIBUTE_GROUPS(iqs269);
static const struct regmap_config iqs269_regmap_config = {
.reg_bits = 8,
@@ -1671,10 +1668,6 @@ static int iqs269_probe(struct i2c_client *client)
return error;
}
- error = devm_device_add_group(&client->dev, &iqs269_attr_group);
- if (error)
- dev_err(&client->dev, "Failed to add attributes: %d\n", error);
-
return error;
}
@@ -1743,6 +1736,7 @@ MODULE_DEVICE_TABLE(of, iqs269_of_match);
static struct i2c_driver iqs269_i2c_driver = {
.driver = {
.name = "iqs269a",
+ .dev_groups = iqs269_groups,
.of_match_table = iqs269_of_match,
.pm = pm_sleep_ptr(&iqs269_pm),
},
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH 06/22] Input: ad7879 - use device core to create driver-specific device attributes
From: Dmitry Torokhov @ 2023-07-29 0:51 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Greg Kroah-Hartman, Michael Hennerich
In-Reply-To: <20230729005133.1095051-1-dmitry.torokhov@gmail.com>
Instead of creating driver-specific device attributes with
devm_device_add_group() have device core do this by setting up dev_groups
pointer in the driver structure.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/ad7879-i2c.c | 7 ++++---
drivers/input/touchscreen/ad7879-spi.c | 7 ++++---
drivers/input/touchscreen/ad7879.c | 10 ++++++----
drivers/input/touchscreen/ad7879.h | 3 +++
4 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/ad7879-i2c.c b/drivers/input/touchscreen/ad7879-i2c.c
index feaa6f8b01ed..5c094ab74698 100644
--- a/drivers/input/touchscreen/ad7879-i2c.c
+++ b/drivers/input/touchscreen/ad7879-i2c.c
@@ -58,9 +58,10 @@ MODULE_DEVICE_TABLE(of, ad7879_i2c_dt_ids);
static struct i2c_driver ad7879_i2c_driver = {
.driver = {
- .name = "ad7879",
- .pm = &ad7879_pm_ops,
- .of_match_table = of_match_ptr(ad7879_i2c_dt_ids),
+ .name = "ad7879",
+ .dev_groups = ad7879_groups,
+ .pm = &ad7879_pm_ops,
+ .of_match_table = of_match_ptr(ad7879_i2c_dt_ids),
},
.probe = ad7879_i2c_probe,
.id_table = ad7879_id,
diff --git a/drivers/input/touchscreen/ad7879-spi.c b/drivers/input/touchscreen/ad7879-spi.c
index 50e889846800..064968fe57cf 100644
--- a/drivers/input/touchscreen/ad7879-spi.c
+++ b/drivers/input/touchscreen/ad7879-spi.c
@@ -56,9 +56,10 @@ MODULE_DEVICE_TABLE(of, ad7879_spi_dt_ids);
static struct spi_driver ad7879_spi_driver = {
.driver = {
- .name = "ad7879",
- .pm = &ad7879_pm_ops,
- .of_match_table = of_match_ptr(ad7879_spi_dt_ids),
+ .name = "ad7879",
+ .dev_groups = ad7879_groups,
+ .pm = &ad7879_pm_ops,
+ .of_match_table = of_match_ptr(ad7879_spi_dt_ids),
},
.probe = ad7879_spi_probe,
};
diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
index e850853328f1..e5d69bf2276e 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -391,6 +391,12 @@ static const struct attribute_group ad7879_attr_group = {
.attrs = ad7879_attributes,
};
+const struct attribute_group *ad7879_groups[] = {
+ &ad7879_attr_group,
+ NULL
+};
+EXPORT_SYMBOL_GPL(ad7879_groups);
+
#ifdef CONFIG_GPIOLIB
static int ad7879_gpio_direction_input(struct gpio_chip *chip,
unsigned gpio)
@@ -612,10 +618,6 @@ int ad7879_probe(struct device *dev, struct regmap *regmap,
__ad7879_disable(ts);
- err = devm_device_add_group(dev, &ad7879_attr_group);
- if (err)
- return err;
-
err = ad7879_gpio_add(ts);
if (err)
return err;
diff --git a/drivers/input/touchscreen/ad7879.h b/drivers/input/touchscreen/ad7879.h
index ae8aa1428e56..d71a8e787290 100644
--- a/drivers/input/touchscreen/ad7879.h
+++ b/drivers/input/touchscreen/ad7879.h
@@ -8,11 +8,14 @@
#ifndef _AD7879_H_
#define _AD7879_H_
+#include <linux/pm.h>
#include <linux/types.h>
+struct attribute_group;
struct device;
struct regmap;
+extern const struct attribute_group *ad7879_groups[];
extern const struct dev_pm_ops ad7879_pm_ops;
int ad7879_probe(struct device *dev, struct regmap *regmap,
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* Re: [PATCH v4 00/11] drm/panel and i2c-hid: Allow panels and touchscreens to power sequence together
From: Doug Anderson @ 2023-07-28 17:24 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, linux-arm-msm, yangcong5, devicetree,
Daniel Vetter, hsinyi, Chris Morgan, linux-input,
cros-qcom-dts-watchers, Dmitry Torokhov, linux-kernel, dri-devel
In-Reply-To: <nn6cs4zvf27cxmtd3qcficyoyalcxi2iry6kgszb5oraplgaxy@sryeyseucdb3>
Hi,
On Fri, Jul 28, 2023 at 8:31 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> On Jul 27 2023, Douglas Anderson wrote:
> >
> > The big motivation for this patch series is mostly described in the patch
> > ("drm/panel: Add a way for other devices to follow panel state"), but to
> > quickly summarize here: for touchscreens that are connected to a panel we
> > need the ability to power sequence the two device together. This is not a
> > new need, but so far we've managed to get by through a combination of
> > inefficiency, added costs, or perhaps just a little bit of brokenness.
> > It's time to do better. This patch series allows us to do better.
> >
> > Assuming that people think this patch series looks OK, we'll have to
> > figure out the right way to land it. The panel patches and i2c-hid
> > patches will go through very different trees and so either we'll need
> > an Ack from one side or the other or someone to create a tag for the
> > other tree to pull in. This will _probably_ require the true drm-misc
> > maintainers to get involved, not a lowly committer. ;-)
> >
> > Version 4 of this series adds a new patch that suspends i2c-hid
> > devices at remove time even for non panel-followers to make things
> > consistent. It also attempts to isolate the panel follower code a bit
> > more as per Benjamin's feedback on v3 and adds an item to the DRM todo
> > list as per Maxime's request. As per Maxime's response to my v3 cover
> > letter, I added his Reviewed-by tag to all 10 patches that were part
> > of v3 (but left it off of the new i2c-hid patch in v4).
> >
> > Version 3 of this series was a long time coming after v2. Maxime and I
> > had a very long discussion trying to figure out if there was a beter
> > way and in the end we didn't find one so he was OK with the series in
> > general [1]. After that got resolved, I tried to resolve Benjamin's
> > feedback but got stuck [2]. Eventually I made my best guess. The end
> > result was a v3 that wasn't that different from v2 but that had a tiny
> > bit more code split out.
> >
> > Version 2 of this patch series didn't change too much. At a high level:
> > * I added all the forgotten "static" to functions.
> > * I've hopefully made the bindings better.
> > * I've integrated into fw_devlink.
> > * I cleaned up a few descriptions / comments.
> >
> > As far as I can tell, as of v4 everyone is on the same page that this
> > patch series looks like a reasonable solution to the problem and we
> > just need to get all the nits fixed and figure out how to land it.
>
> Thanks a lot for the new version. I like it much more on the HID side:
>
> for the HID part:
> Reviewed-by: Benjamin Tissoires <bentiss@kernel.org>
>
> I wouldn't mind having this series taken from the drm tree if that is
> easier. i2c-hid is a low patch rate driver, so having it updated through
> DRM should not be an issue.
>
> In that case:
> Acked-by: Benjamin Tissoires <bentiss@kernel.org>
Thanks for your reviews and your help getting this whipped into shape.
Lading through drm makes sense to me. I'm a drm committer, so with
your Ack I believe it should be fine for me to land the series (minus
the dts) in drm-misc-next. This series has been around for a while,
has been reviewed by relevant folks, and the last few changes haven't
fundamentally changed anything about the design, so I'm not going to
twiddle my thumbs too long. That being said, I'll still plan to wait
until early next week (Tuesday?) before landing to allow for any last
minute shouts.
Given how drm-misc works [1] and the fact that mainline is currently
at v6.5-rc3 (it will be -rc4 when I land it), I'd expect that these
commits will find their way into v6.6.
[1] https://drm.pages.freedesktop.org/maintainer-tools/drm-misc.html
^ permalink raw reply
* [PATCH v2] dt-bindings: input: convert syna,rmi4 to DT schema
From: Krzysztof Kozlowski @ 2023-07-28 16:50 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jason A. Donenfeld, Matthias Schiffer, Vincent Huang, linux-input,
devicetree, linux-kernel
Cc: Krzysztof Kozlowski
Convert the bindings for Synaptics RMI4 bus and devices to DT schema.
Changes during conversion:
1. Add reset-gpios already used in DTS and mentioned by RMI4
specification.
2. Do not require address/size cells, because without functions
(children) they aren't really needed.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. Correct [0-9a-z] to [0-9a-f] (Rob).
Jason, Matthias, Vincent,
I put your names as maintainers, because moderately recently you were
changing the driver. Let me know if this is okay or you prefer not to
maintain the hardware.
---
.../bindings/input/rmi4/rmi_2d_sensor.txt | 56 ----
.../bindings/input/rmi4/rmi_f01.txt | 39 ---
.../bindings/input/rmi4/rmi_i2c.txt | 61 ----
.../bindings/input/rmi4/rmi_spi.txt | 56 ----
.../devicetree/bindings/input/syna,rmi4.yaml | 271 ++++++++++++++++++
5 files changed, 271 insertions(+), 212 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
delete mode 100644 Documentation/devicetree/bindings/input/rmi4/rmi_f01.txt
delete mode 100644 Documentation/devicetree/bindings/input/rmi4/rmi_i2c.txt
delete mode 100644 Documentation/devicetree/bindings/input/rmi4/rmi_spi.txt
create mode 100644 Documentation/devicetree/bindings/input/syna,rmi4.yaml
diff --git a/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt b/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
deleted file mode 100644
index 9afffbdf6e28..000000000000
--- a/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
+++ /dev/null
@@ -1,56 +0,0 @@
-Synaptics RMI4 2D Sensor Device Binding
-
-The Synaptics RMI4 core is able to support RMI4 devices using different
-transports and different functions. This file describes the device tree
-bindings for devices which contain 2D sensors using Function 11 or
-Function 12. Complete documentation for transports and other functions
-can be found in:
-Documentation/devicetree/bindings/input/rmi4.
-
-RMI4 Function 11 and Function 12 are for 2D touch position sensing.
-Additional documentation for F11 can be found at:
-http://www.synaptics.com/sites/default/files/511-000136-01-Rev-E-RMI4-Interfacing-Guide.pdf
-
-Optional Touch Properties:
-Description in Documentation/devicetree/bindings/input/touchscreen
-- touchscreen-inverted-x
-- touchscreen-inverted-y
-- touchscreen-swapped-x-y
-- touchscreen-x-mm
-- touchscreen-y-mm
-
-Optional Properties:
-- syna,clip-x-low: Sets a minimum value for X.
-- syna,clip-y-low: Sets a minimum value for Y.
-- syna,clip-x-high: Sets a maximum value for X.
-- syna,clip-y-high: Sets a maximum value for Y.
-- syna,offset-x: Add an offset to X.
-- syna,offset-y: Add an offset to Y.
-- syna,delta-x-threshold: Set the minimum distance on the X axis required
- to generate an interrupt in reduced reporting
- mode.
-- syna,delta-y-threshold: Set the minimum distance on the Y axis required
- to generate an interrupt in reduced reporting
- mode.
-- syna,sensor-type: Set the sensor type. 1 for touchscreen 2 for touchpad.
-- syna,disable-report-mask: Mask for disabling posiiton reporting. Used to
- disable reporing absolute position data.
-- syna,rezero-wait-ms: Time in miliseconds to wait after issuing a rezero
- command.
-
-
-Example of a RMI4 I2C device with F11:
-Example:
- &i2c1 {
- rmi4-i2c-dev@2c {
- compatible = "syna,rmi4-i2c";
-
- ...
-
- rmi4-f11@11 {
- reg = <0x11>;
- touchscreen-inverted-y;
- syna,sensor-type = <2>;
- };
- };
- };
diff --git a/Documentation/devicetree/bindings/input/rmi4/rmi_f01.txt b/Documentation/devicetree/bindings/input/rmi4/rmi_f01.txt
deleted file mode 100644
index 079cad2b6843..000000000000
--- a/Documentation/devicetree/bindings/input/rmi4/rmi_f01.txt
+++ /dev/null
@@ -1,39 +0,0 @@
-Synaptics RMI4 F01 Device Binding
-
-The Synaptics RMI4 core is able to support RMI4 devices using different
-transports and different functions. This file describes the device tree
-bindings for devices which contain Function 1. Complete documentation
-for transports and other functions can be found in:
-Documentation/devicetree/bindings/input/rmi4.
-
-Additional documentation for F01 can be found at:
-http://www.synaptics.com/sites/default/files/511-000136-01-Rev-E-RMI4-Interfacing-Guide.pdf
-
-Optional Properties:
-- syna,nosleep-mode: If set the device will run at full power without sleeping.
- nosleep has 3 modes, 0 will not change the default
- setting, 1 will disable nosleep (allow sleeping),
- and 2 will enable nosleep (disabling sleep).
-- syna,wakeup-threshold: Defines the amplitude of the disturbance to the
- background capacitance that will cause the
- device to wake from dozing.
-- syna,doze-holdoff-ms: The delay to wait after the last finger lift and the
- first doze cycle.
-- syna,doze-interval-ms: The time period that the device sleeps between finger
- activity.
-
-
-Example of a RMI4 I2C device with F01:
- Example:
- &i2c1 {
- rmi4-i2c-dev@2c {
- compatible = "syna,rmi4-i2c";
-
- ...
-
- rmi4-f01@1 {
- reg = <0x1>;
- syna,nosleep-mode = <1>;
- };
- };
- };
diff --git a/Documentation/devicetree/bindings/input/rmi4/rmi_i2c.txt b/Documentation/devicetree/bindings/input/rmi4/rmi_i2c.txt
deleted file mode 100644
index dcb012f5b3ee..000000000000
--- a/Documentation/devicetree/bindings/input/rmi4/rmi_i2c.txt
+++ /dev/null
@@ -1,61 +0,0 @@
-Synaptics RMI4 I2C Device Binding
-
-The Synaptics RMI4 core is able to support RMI4 devices using different
-transports and different functions. This file describes the device tree
-bindings for devices using the I2C transport driver. Complete documentation
-for other transports and functions can be found in
-Documentation/devicetree/bindings/input/rmi4.
-
-Required Properties:
-- compatible: syna,rmi4-i2c
-- reg: I2C address
-- #address-cells: Set to 1 to indicate that the function child nodes
- consist of only on uint32 value.
-- #size-cells: Set to 0 to indicate that the function child nodes do not
- have a size property.
-
-Optional Properties:
-- interrupts: interrupt which the rmi device is connected to.
-See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
-
-- syna,reset-delay-ms: The number of milliseconds to wait after resetting the
- device.
-
-- syna,startup-delay-ms: The number of milliseconds to wait after powering on
- the device.
-
-- vdd-supply: VDD power supply.
-See ../regulator/regulator.txt
-
-- vio-supply: VIO power supply
-See ../regulator/regulator.txt
-
-Function Parameters:
-Parameters specific to RMI functions are contained in child nodes of the rmi device
- node. Documentation for the parameters of each function can be found in:
-Documentation/devicetree/bindings/input/rmi4/rmi_f*.txt.
-
-
-
-Example:
- &i2c1 {
- rmi4-i2c-dev@2c {
- compatible = "syna,rmi4-i2c";
- reg = <0x2c>;
- #address-cells = <1>;
- #size-cells = <0>;
- interrupt-parent = <&gpio>;
- interrupts = <4 2>;
-
- rmi4-f01@1 {
- reg = <0x1>;
- syna,nosleep-mode = <1>;
- };
-
- rmi4-f11@11 {
- reg = <0x11>;
- touchscreen-inverted-y;
- syna,sensor-type = <2>;
- };
- };
- };
diff --git a/Documentation/devicetree/bindings/input/rmi4/rmi_spi.txt b/Documentation/devicetree/bindings/input/rmi4/rmi_spi.txt
deleted file mode 100644
index 632f473db65b..000000000000
--- a/Documentation/devicetree/bindings/input/rmi4/rmi_spi.txt
+++ /dev/null
@@ -1,56 +0,0 @@
-Synaptics RMI4 SPI Device Binding
-
-The Synaptics RMI4 core is able to support RMI4 devices using different
-transports and different functions. This file describes the device tree
-bindings for devices using the SPI transport driver. Complete documentation
-for other transports and functions can be found in
-Documentation/devicetree/bindings/input/rmi4.
-
-Required Properties:
-- compatible: syna,rmi4-spi
-- reg: Chip select address for the device
-- #address-cells: Set to 1 to indicate that the function child nodes
- consist of only on uint32 value.
-- #size-cells: Set to 0 to indicate that the function child nodes do not
- have a size property.
-
-Optional Properties:
-- interrupts: interrupt which the rmi device is connected to.
-See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
-
-- spi-rx-delay-us: microsecond delay after a read transfer.
-- spi-tx-delay-us: microsecond delay after a write transfer.
-
-Function Parameters:
-Parameters specific to RMI functions are contained in child nodes of the rmi device
- node. Documentation for the parameters of each function can be found in:
-Documentation/devicetree/bindings/input/rmi4/rmi_f*.txt.
-
-
-
-Example:
- spi@7000d800 {
- rmi4-spi-dev@0 {
- compatible = "syna,rmi4-spi";
- reg = <0x0>;
- #address-cells = <1>;
- #size-cells = <0>;
- spi-max-frequency = <4000000>;
- spi-cpha;
- spi-cpol;
- interrupt-parent = <&gpio>;
- interrupts = <TEGRA_GPIO(K, 2) 0x2>;
- spi-rx-delay-us = <30>;
-
- rmi4-f01@1 {
- reg = <0x1>;
- syna,nosleep-mode = <1>;
- };
-
- rmi4-f11@11 {
- reg = <0x11>;
- touchscreen-inverted-y;
- syna,sensor-type = <2>;
- };
- };
- };
diff --git a/Documentation/devicetree/bindings/input/syna,rmi4.yaml b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
new file mode 100644
index 000000000000..4d4e1a8e36be
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
@@ -0,0 +1,271 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/syna,rmi4.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synaptics RMI4 compliant devices
+
+maintainers:
+ - Jason A. Donenfeld <Jason@zx2c4.com>
+ - Matthias Schiffer <matthias.schiffer@ew.tq-group.com
+ - Vincent Huang <vincent.huang@tw.synaptics.com>
+
+description: |
+ The Synaptics RMI4 (Register Mapped Interface 4) core is able to support RMI4
+ devices using different transports (I2C, SPI) and different functions (e.g.
+ Function 1, 2D sensors using Function 11 or 12).
+
+properties:
+ compatible:
+ enum:
+ - syna,rmi4-i2c
+ - syna,rmi4-spi
+
+ reg:
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+ description: Active low signal
+
+ spi-cpha: true
+ spi-cpol: true
+
+ syna,reset-delay-ms:
+ description:
+ Delay to wait after resetting the device.
+
+ syna,startup-delay-ms:
+ description:
+ Delay to wait after powering on the device.
+
+ vdd-supply: true
+ vio-supply: true
+
+ rmi4-f01@1:
+ type: object
+ additionalProperties: false
+ description:
+ Function 1
+
+ properties:
+ reg:
+ maxItems: 1
+
+ syna,nosleep-mode:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2]
+ description:
+ If set the device will run at full power without sleeping. nosleep
+ has 3 modes, 0 will not change the default setting, 1 will disable
+ nosleep (allow sleeping), and 2 will enable nosleep (disabling
+ sleep).
+
+ syna,wakeup-threshold:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Defines the amplitude of the disturbance to the background
+ capacitance that will cause the device to wake from dozing.
+
+ syna,doze-holdoff-ms:
+ description:
+ The delay to wait after the last finger lift and the first doze
+ cycle.
+
+ syna,doze-interval-ms:
+ description:
+ The time period that the device sleeps between finger activity.
+
+ required:
+ - reg
+
+patternProperties:
+ "^rmi4-f1[12]@1[12]$":
+ type: object
+ unevaluatedProperties: false
+ $ref: /schemas/input/touchscreen/touchscreen.yaml#
+ description:
+ RMI4 Function 11 and Function 12 are for 2D touch position sensing.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ syna,clip-x-low:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Minimum value for X.
+
+ syna,clip-y-low:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Minimum value for Y.
+
+ syna,clip-x-high:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Maximum value for X.
+
+ syna,clip-y-high:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Maximum value for Y.
+
+ syna,offset-x:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Add an offset to X.
+
+ syna,offset-y:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Add an offset to Y.
+
+ syna,delta-x-threshold:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Minimum distance on the X axis required to generate an interrupt in
+ reduced reporting mode.
+
+ syna,delta-y-threshold:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Minimum distance on the Y axis required to generate an interrupt in
+ reduced reporting mode.
+
+ syna,sensor-type:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [1, 2]
+ description: |
+ Sensor type: 1 for touchscreen 2 for touchpad.
+
+ syna,disable-report-mask:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Mask for disabling posiiton reporting. Used to disable reporing
+ absolute position data.
+
+ syna,rezero-wait-ms:
+ description:
+ Time to wait after issuing a rezero command.
+
+ required:
+ - reg
+
+ "^rmi4-f[0-9a-f]+@[0-9a-f]+$":
+ type: object
+ description:
+ Other functions, not documented yet.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: syna,rmi4-i2c
+ then:
+ properties:
+ spi-rx-delay-us: false
+ spi-tx-delay-us: false
+ else:
+ properties:
+ syna,reset-delay-ms: false
+ syna,startup-delay-ms: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen@20 {
+ compatible = "syna,rmi4-i2c";
+ reg = <0x20>;
+ interrupt-parent = <&gpx1>;
+ interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
+
+ syna,startup-delay-ms = <100>;
+ vdd-supply = <&tsp_vdd>;
+ vio-supply = <&ldo32_reg>;
+
+ pinctrl-0 = <&touch_irq>;
+ pinctrl-names = "default";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rmi4-f01@1 {
+ reg = <0x1>;
+ syna,nosleep-mode = <1>;
+ };
+
+ rmi4-f12@12 {
+ reg = <0x12>;
+ syna,sensor-type = <1>;
+ };
+
+ rmi4-f1a@1a {
+ reg = <0x1a>;
+ };
+ };
+ };
+
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen@0 {
+ compatible = "syna,rmi4-spi";
+ reg = <0x0>;
+ interrupt-parent = <&gpx1>;
+ interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
+
+ spi-max-frequency = <4000000>;
+ spi-rx-delay-us = <30>;
+ spi-cpha;
+ spi-cpol;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rmi4-f01@1 {
+ reg = <0x1>;
+ syna,nosleep-mode = <1>;
+ };
+
+ rmi4-f11@11 {
+ reg = <0x11>;
+ touchscreen-inverted-y;
+ syna,sensor-type = <2>;
+ };
+ };
+ };
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v4 00/11] drm/panel and i2c-hid: Allow panels and touchscreens to power sequence together
From: Benjamin Tissoires @ 2023-07-28 15:31 UTC (permalink / raw)
To: Douglas Anderson
Cc: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, linux-arm-msm, yangcong5, devicetree,
Daniel Vetter, hsinyi, Chris Morgan, linux-input,
cros-qcom-dts-watchers, Dmitry Torokhov, linux-kernel, dri-devel
In-Reply-To: <20230727171750.633410-1-dianders@chromium.org>
On Jul 27 2023, Douglas Anderson wrote:
>
> The big motivation for this patch series is mostly described in the patch
> ("drm/panel: Add a way for other devices to follow panel state"), but to
> quickly summarize here: for touchscreens that are connected to a panel we
> need the ability to power sequence the two device together. This is not a
> new need, but so far we've managed to get by through a combination of
> inefficiency, added costs, or perhaps just a little bit of brokenness.
> It's time to do better. This patch series allows us to do better.
>
> Assuming that people think this patch series looks OK, we'll have to
> figure out the right way to land it. The panel patches and i2c-hid
> patches will go through very different trees and so either we'll need
> an Ack from one side or the other or someone to create a tag for the
> other tree to pull in. This will _probably_ require the true drm-misc
> maintainers to get involved, not a lowly committer. ;-)
>
> Version 4 of this series adds a new patch that suspends i2c-hid
> devices at remove time even for non panel-followers to make things
> consistent. It also attempts to isolate the panel follower code a bit
> more as per Benjamin's feedback on v3 and adds an item to the DRM todo
> list as per Maxime's request. As per Maxime's response to my v3 cover
> letter, I added his Reviewed-by tag to all 10 patches that were part
> of v3 (but left it off of the new i2c-hid patch in v4).
>
> Version 3 of this series was a long time coming after v2. Maxime and I
> had a very long discussion trying to figure out if there was a beter
> way and in the end we didn't find one so he was OK with the series in
> general [1]. After that got resolved, I tried to resolve Benjamin's
> feedback but got stuck [2]. Eventually I made my best guess. The end
> result was a v3 that wasn't that different from v2 but that had a tiny
> bit more code split out.
>
> Version 2 of this patch series didn't change too much. At a high level:
> * I added all the forgotten "static" to functions.
> * I've hopefully made the bindings better.
> * I've integrated into fw_devlink.
> * I cleaned up a few descriptions / comments.
>
> As far as I can tell, as of v4 everyone is on the same page that this
> patch series looks like a reasonable solution to the problem and we
> just need to get all the nits fixed and figure out how to land it.
Thanks a lot for the new version. I like it much more on the HID side:
for the HID part:
Reviewed-by: Benjamin Tissoires <bentiss@kernel.org>
I wouldn't mind having this series taken from the drm tree if that is
easier. i2c-hid is a low patch rate driver, so having it updated through
DRM should not be an issue.
In that case:
Acked-by: Benjamin Tissoires <bentiss@kernel.org>
Cheers,
Benjamin
>
> [1] https://lore.kernel.org/r/gkwymmfkdy2p2evz22wmbwgw42ii4wnvmvu64m3bghmj2jhv7x@4mbstjxnagxd
> [2] https://lore.kernel.org/r/CAD=FV=VbdeomBGbWhppY+5TOSwt64GWBHga68OXFwsnO4gg4UA@mail.gmail.com
>
> Changes in v4:
> - Document further cleanup in the official DRM todo list.
> - ("Suspend i2c-hid devices in remove") new for v4.
> - Move panel follower alternative checks to wrapper functions.
> - Rebase atop ("Suspend i2c-hid devices in remove").
>
> Changes in v3:
> - Add is_panel_follower() as a convenience for clients.
> - Add "depends on DRM || !DRM" to Kconfig to avoid randconfig error.
> - Split more of the panel follower code out of the core.
>
> Changes in v2:
> - Move the description to the generic touchscreen.yaml.
> - Update the desc to make it clearer it's only for integrated devices.
> - Add even more text to the commit message.
> - A few comment cleanups.
> - ("Add a devlink for panel followers") new for v2.
> - i2c_hid_core_initial_power_up() is now static.
> - i2c_hid_core_panel_prepared() and ..._unpreparing() are now static.
> - ihid_core_panel_prepare_work() is now static.
> - Improve documentation for smp_wmb().
>
> Douglas Anderson (11):
> dt-bindings: HID: i2c-hid: Add "panel" property to i2c-hid backed
> touchscreens
> drm/panel: Check for already prepared/enabled in drm_panel
> drm/panel: Add a way for other devices to follow panel state
> of: property: fw_devlink: Add a devlink for panel followers
> HID: i2c-hid: Switch to SYSTEM_SLEEP_PM_OPS()
> HID: i2c-hid: Rearrange probe() to power things up later
> HID: i2c-hid: Make suspend and resume into helper functions
> HID: i2c-hid: Suspend i2c-hid devices in remove
> HID: i2c-hid: Support being a panel follower
> HID: i2c-hid: Do panel follower work on the system_wq
> arm64: dts: qcom: sc7180: Link trogdor touchscreens to the panels
>
> .../bindings/input/elan,ekth6915.yaml | 5 +
> .../bindings/input/goodix,gt7375p.yaml | 5 +
> .../bindings/input/hid-over-i2c.yaml | 2 +
> .../input/touchscreen/touchscreen.yaml | 7 +
> Documentation/gpu/todo.rst | 24 ++
> .../boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 1 +
> .../dts/qcom/sc7180-trogdor-homestar.dtsi | 1 +
> .../boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 1 +
> .../boot/dts/qcom/sc7180-trogdor-pompom.dtsi | 1 +
> .../qcom/sc7180-trogdor-quackingstick.dtsi | 1 +
> .../dts/qcom/sc7180-trogdor-wormdingler.dtsi | 1 +
> drivers/gpu/drm/drm_panel.c | 218 ++++++++++-
> drivers/hid/i2c-hid/Kconfig | 2 +
> drivers/hid/i2c-hid/i2c-hid-core.c | 349 +++++++++++++-----
> drivers/of/property.c | 2 +
> include/drm/drm_panel.h | 94 +++++
> 16 files changed, 617 insertions(+), 97 deletions(-)
>
> --
> 2.41.0.487.g6d72f3e995-goog
>
^ permalink raw reply
* [PATCH] Input: ads7846 - don't set ABS_PRESSURE when min == max
From: Benjamin Bara @ 2023-07-28 13:38 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: bbara93, benjamin.bara, linux-input, linux-kernel
In-Reply-To: <ZMLV9BVxHx3jlE5R@google.com>
Hi Dmitry,
thank you for the feedback!
On Thu, 27 Jul 2023 at 22:39, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Thu, Jul 27, 2023 at 05:19:57PM +0200, Benjamin Bara wrote:
> > From: Benjamin Bara <benjamin.bara@skidata.com>
> >
> > When the optional fields "pressure_min" and "pressure_max" are not set,
> > both fall back to 0, which results to the following libinput error:
> >
> > ADS7846 Touchscreen: kernel bug: device has min == max on ABS_PRESSURE
> >
> > Avoid it by only setting ABS_PRESSURE if the values differ.
>
> What is the benefit of silently continuing without reporting pressure on
> models that do support pressure readings, vs. using libinput's errors as
> signal to adjust DT?
With the current implementation, libinput rejects the device [1] when an
optional device-tree field is not set, which leads to a "non-functional" device.
But you are right, silently continuing might also not be the best option...
I could provide a v2 where the maximum is set to a fallback value (~0) and a
warning is printed that the pressure is not set and therefore a fallback value
is used, to signal the user to adjust the DT.
What do you think about that?
Best regards
Benjamin
[1] https://gitlab.freedesktop.org/libinput/libinput/-/blob/1.23.0/src/evdev.c?ref_type=tags#L1816
^ permalink raw reply
* Re: [PATCH v3 37/42] pwm: ep93xx: drop legacy pinctrl
From: Thierry Reding @ 2023-07-28 8:23 UTC (permalink / raw)
To: nikita.shubin
Cc: Hartley Sweeten, Lennert Buytenhek, Alexander Sverdlin,
Russell King, Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
Stephen Boyd, Daniel Lezcano, Thomas Gleixner, Alessandro Zummo,
Alexandre Belloni, Wim Van Sebroeck, Guenter Roeck,
Sebastian Reichel, Uwe Kleine-König, Mark Brown,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Vinod Koul, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Damien Le Moal, Sergey Shtylyov,
Dmitry Torokhov, Arnd Bergmann, Olof Johansson, soc,
Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Andy Shevchenko,
Michael Peters, Kris Bahnsen, linux-arm-kernel, linux-kernel,
linux-gpio, devicetree, linux-clk, linux-rtc, linux-watchdog,
linux-pm, linux-pwm, linux-spi, netdev, dmaengine, linux-mtd,
linux-ide, linux-input, alsa-devel
In-Reply-To: <20230605-ep93xx-v3-37-3d63a5f1103e@maquefel.me>
[-- Attachment #1: Type: text/plain, Size: 628 bytes --]
On Thu, Jul 20, 2023 at 02:29:37PM +0300, Nikita Shubin via B4 Relay wrote:
> From: Nikita Shubin <nikita.shubin@maquefel.me>
>
> Drop legacy gpio request/free since we are using
> pinctrl for this now.
>
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> arch/arm/mach-ep93xx/core.c | 42 ---------------------------------------
> drivers/pwm/pwm-ep93xx.c | 18 -----------------
> include/linux/soc/cirrus/ep93xx.h | 4 ----
> 3 files changed, 64 deletions(-)
Acked-by: Thierry Reding <thierry.reding@gmail.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] hid-mcp2200: added driver for GPIOs of MCP2200
From: Christophe JAILLET @ 2023-07-27 22:30 UTC (permalink / raw)
To: Johannes Roith, jikos, benjamin.tissoires
Cc: linux-kernel, linux-input, andi.shyti
In-Reply-To: <20230623110145.92566-1-johannes@gnu-linux.rocks>
Le 23/06/2023 à 13:01, Johannes Roith a écrit :
> Added a gpiochip compatible driver to control the 8 GPIOs of the MCP2200
> by using the HID interface.
>
> Using GPIOs with alternative functions (GP0<->SSPND, GP1<->USBCFG,
> GP6<->RXLED, GP7<->TXLED) will reset the functions, if set (unset by
> default).
>
> The driver was tested while also using the UART of the chip. Setting
> and reading the GPIOs has no effect on the UART communication. However,
> a reset is triggered after the CONFIGURE command. If the GPIO Direction
> is constantly changed, this will affect the communication at low baud
> rates. This is a hardware problem of the MCP2200 and is not caused by
> the driver.
>
> Feedback from reviewers Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> and Andi Shyti <andi.shyti@kernel.org> was added.
>
> Signed-off-by: Johannes Roith <johannes@gnu-linux.rocks>
> ---
[...]
> +static int mcp2200_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> + int ret;
> + struct mcp2200 *mcp;
> +
> + mcp = devm_kzalloc(&hdev->dev, sizeof(*mcp), GFP_KERNEL);
> + if (!mcp)
> + return -ENOMEM;
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "can't parse reports\n");
> + return ret;
> + }
> +
> + /*
> + * This driver uses the .raw_event callback and therefore does not need any
> + * HID_CONNECT_xxx flags.
> + */
> + ret = hid_hw_start(hdev, 0);
> + if (ret) {
> + hid_err(hdev, "can't start hardware\n");
> + return ret;
> + }
> +
> + hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n", hdev->version >> 8,
> + hdev->version & 0xff, hdev->name, hdev->phys);
> +
> + ret = hid_hw_open(hdev);
> + if (ret) {
> + hid_err(hdev, "can't open device\n");
> + hid_hw_stop(hdev);
> + return ret;
> + }
> +
> + mutex_init(&mcp->lock);
> + init_completion(&mcp->wait_in_report);
> + hid_set_drvdata(hdev, mcp);
> + mcp->hdev = hdev;
> +
> + ret = devm_add_action_or_reset(&hdev->dev, mcp2200_hid_unregister, hdev);
> + if (ret)
> + return ret;
> +
> + mcp->gc = template_chip;
> + mcp->gc.parent = &hdev->dev;
> +
> + ret = gpiochip_add_data(&mcp->gc, mcp);
devm_gpiochip_add_data()?
> + if (ret < 0) {
> + dev_err(&hdev->dev, "Unable to register gpiochip\n");
hid_err() to be consistent?
> + hid_hw_stop(hdev);
Isn't it already run by mcp2200_hid_unregister() registered a few lines
above?
CJ
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void mcp2200_remove(struct hid_device *hdev)
> +{
> + struct mcp2200 *mcp;
> +
> + mcp = hid_get_drvdata(hdev);
> + gpiochip_remove(&mcp->gc);
> +}
> +
[...]
^ permalink raw reply
* Re: [PATCH] hid-mcp2200: added driver for GPIOs of MCP2200
From: Andi Shyti @ 2023-07-27 22:03 UTC (permalink / raw)
To: Johannes Roith
Cc: jikos, benjamin.tissoires, linux-kernel, linux-input,
christophe.jaillet
In-Reply-To: <20230623110145.92566-1-johannes@gnu-linux.rocks>
Hi Johannes,
On Fri, Jun 23, 2023 at 01:01:45PM +0200, Johannes Roith wrote:
> Added a gpiochip compatible driver to control the 8 GPIOs of the MCP2200
> by using the HID interface.
>
> Using GPIOs with alternative functions (GP0<->SSPND, GP1<->USBCFG,
> GP6<->RXLED, GP7<->TXLED) will reset the functions, if set (unset by
> default).
>
> The driver was tested while also using the UART of the chip. Setting
> and reading the GPIOs has no effect on the UART communication. However,
> a reset is triggered after the CONFIGURE command. If the GPIO Direction
> is constantly changed, this will affect the communication at low baud
> rates. This is a hardware problem of the MCP2200 and is not caused by
> the driver.
>
> Feedback from reviewers Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> and Andi Shyti <andi.shyti@kernel.org> was added.
>
> Signed-off-by: Johannes Roith <johannes@gnu-linux.rocks>
Just one note... please add a versioning to your patches and a
changelog.
I saw that your previous version did not receive any reply
(sorry)... in that case you could add a prefix RESEND.
So that this patch would look like:
[PATCH RESEND v3] hid-mcp2200: Added driver for GPIOs of MCP2200
(note "Added" with capital letter)
[...]
> +/* Altternative pin assignments */
Alternative
> +#define TXLED 2
> +#define RXLED 3
> +#define USBCFG 6
> +#define SSPND 7
> +#define MCP_NGPIO 8
[...]
> +static int mcp_cmd_read_all(struct mcp2200 *mcp)
> +{
> + struct mcp_read_all *read_all;
> + int len, t;
> +
> + reinit_completion(&mcp->wait_in_report);
> + read_all = kzalloc(sizeof(struct mcp_read_all), GFP_KERNEL);
> + if (!read_all)
> + return -ENOMEM;
any reason for allocating read_all dynamically?
> +
> + read_all->cmd = READ_ALL;
> +
> + mutex_lock(&mcp->lock);
> + len = hid_hw_output_report(mcp->hdev, (u8 *) read_all,
> + sizeof(struct mcp_read_all));
Please align "sizeof" with "mcp"
> + mutex_unlock(&mcp->lock);
> + kfree(read_all);
> +
> + if (len != sizeof(struct mcp_read_all))
> + return -EINVAL;
> +
> + t = wait_for_completion_timeout(&mcp->wait_in_report, msecs_to_jiffies(4000));
not an error, just a suggestion... would be nice to keep lines
withing 80 characters whenever possible... and here it's possible :)
> + if (!t)
> + return -ETIMEDOUT;
> +
> + /* return status, negative value if wrong response was received */
> + return mcp->status;
> +}
> +
> +static void mcp_set_multiple(struct gpio_chip *gc, unsigned long *mask,
> + unsigned long *bits)
please align "struct" with "unsigned"
> +{
> + struct mcp2200 *mcp = gpiochip_get_data(gc);
> + u8 value;
> + int status;
> + struct mcp_set_clear_outputs *cmd;
> +
> + cmd = kzalloc(sizeof(struct mcp_set_clear_outputs), GFP_KERNEL);
> + if (!cmd)
> + return;
> +
> + mutex_lock(&mcp->lock);
> +
> + value = mcp->gpio_val & ~*mask;
> + value |= (*mask & *bits);
> +
> + cmd->cmd = SET_CLEAR_OUTPUTS;
> + cmd->set_bmap = value;
> + cmd->clear_bmap = ~(value);
> +
> + status = hid_hw_output_report(mcp->hdev, (u8 *) cmd,
> + sizeof(struct mcp_set_clear_outputs));
> +
> + mutex_unlock(&mcp->lock);
> + kfree(cmd);
again, any reason for allocating dynamically cmd?
> + if (status == sizeof(struct mcp_set_clear_outputs))
> + mcp->gpio_val = value;
should this be inside the lock?
> +}
[...]
> +static int mcp_get(struct gpio_chip *gc, unsigned int gpio_nr)
> +{
> + unsigned long mask = 0, bits = 0;
> +
> + mask = (1 << gpio_nr);
> + mcp_get_multiple(gc, &mask, &bits);
> + return (bits > 0) ? 1 : 0;
eventually you could just:
return bits > 0;
> +}
[...]
> +static int mcp2200_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> + int ret;
> + struct mcp2200 *mcp;
> +
> + mcp = devm_kzalloc(&hdev->dev, sizeof(*mcp), GFP_KERNEL);
> + if (!mcp)
> + return -ENOMEM;
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "can't parse reports\n");
> + return ret;
Is it dev_err_probe() preferred to hid_err() here?
> + }
> +
> + /*
> + * This driver uses the .raw_event callback and therefore does not need any
> + * HID_CONNECT_xxx flags.
> + */
> + ret = hid_hw_start(hdev, 0);
> + if (ret) {
> + hid_err(hdev, "can't start hardware\n");
> + return ret;
> + }
> +
> + hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n", hdev->version >> 8,
> + hdev->version & 0xff, hdev->name, hdev->phys);
> +
> + ret = hid_hw_open(hdev);
> + if (ret) {
> + hid_err(hdev, "can't open device\n");
> + hid_hw_stop(hdev);
> + return ret;
> + }
> +
> + mutex_init(&mcp->lock);
> + init_completion(&mcp->wait_in_report);
> + hid_set_drvdata(hdev, mcp);
> + mcp->hdev = hdev;
> +
> + ret = devm_add_action_or_reset(&hdev->dev, mcp2200_hid_unregister, hdev);
> + if (ret)
hit_hw_stop()?
Maybe a goto is better?
Andi
> + return ret;
> +
> + mcp->gc = template_chip;
> + mcp->gc.parent = &hdev->dev;
> +
> + ret = gpiochip_add_data(&mcp->gc, mcp);
> + if (ret < 0) {
> + dev_err(&hdev->dev, "Unable to register gpiochip\n");
> + hid_hw_stop(hdev);
> + return ret;
> + }
> +
> + return 0;
> +}
^ permalink raw reply
* Re: [PATCH] Input: ads7846 - don't set ABS_PRESSURE when min == max
From: Dmitry Torokhov @ 2023-07-27 20:39 UTC (permalink / raw)
To: Benjamin Bara; +Cc: linux-input, linux-kernel, Benjamin Bara
In-Reply-To: <20230727-ads7846-pressure-v1-1-fa74c7680191@skidata.com>
Hi Benjamin,
On Thu, Jul 27, 2023 at 05:19:57PM +0200, Benjamin Bara wrote:
> From: Benjamin Bara <benjamin.bara@skidata.com>
>
> When the optional fields "pressure_min" and "pressure_max" are not set,
> both fall back to 0, which results to the following libinput error:
>
> ADS7846 Touchscreen: kernel bug: device has min == max on ABS_PRESSURE
>
> Avoid it by only setting ABS_PRESSURE if the values differ.
What is the benefit of silently continuing without reporting pressure on
models that do support pressure readings, vs. using libinput's errors as
signal to adjust DT?
>
> Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
> ---
> drivers/input/touchscreen/ads7846.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index faea40dd66d0..2535424a5630 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -1281,7 +1281,7 @@ static int ads7846_probe(struct spi_device *spi)
> pdata->y_min ? : 0,
> pdata->y_max ? : MAX_12BIT,
> 0, 0);
> - if (ts->model != 7845)
> + if (ts->model != 7845 && pdata->pressure_min != pdata->pressure_max)
> input_set_abs_params(input_dev, ABS_PRESSURE,
> pdata->pressure_min, pdata->pressure_max, 0, 0);
>
>
> ---
> base-commit: 451cc82bd11eb6a374f4dbcfc1cf007eafea91ab
> change-id: 20230727-ads7846-pressure-05edfb01887f
>
> Best regards,
> --
> Benjamin Bara <benjamin.bara@skidata.com>
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v1 00/12] HID: cp2112: Cleanups and refactorings
From: Andy Shevchenko @ 2023-07-27 18:43 UTC (permalink / raw)
To: linux-input, linux-kernel; +Cc: Jiri Kosina, Benjamin Tissoires
In-Reply-To: <20230703185222.50554-1-andriy.shevchenko@linux.intel.com>
On Mon, Jul 03, 2023 at 09:52:10PM +0300, Andy Shevchenko wrote:
> After I updated GPIO library for the case Benjamin has with CP2112,
> I have a brief look into the CP2112 driver itself.
>
> From GPIO perspective it has two main (maitenance) issues:
> - usage of ->to_irq() with IRQ chip present;
> - having IRQ chip not immutable.
>
> Besides that there are plenty small cleanups here and there.
> Hence this series.
Any comments on this?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v4 11/11] arm64: dts: qcom: sc7180: Link trogdor touchscreens to the panels
From: Douglas Anderson @ 2023-07-27 17:16 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, yangcong5, devicetree, Daniel Vetter, hsinyi,
Chris Morgan, linux-input, cros-qcom-dts-watchers,
Dmitry Torokhov, linux-kernel, dri-devel, Douglas Anderson
In-Reply-To: <20230727171750.633410-1-dianders@chromium.org>
Let's provide the proper link from the touchscreen to the panel on
trogdor devices where the touchscreen support it. This allows the OS
to power sequence the touchscreen more properly.
For the most part, this is just expected to marginally improve power
consumption while the screen is off. However, in at least one trogdor
model (wormdingler) it's suspected that this will fix some behavorial
corner cases when the panel power cycles (like for a modeset) without
the touchscreen power cycling.
NOTE: some trogdor variants use touchscreens that don't (yet) support
linking the touchscreen and the panel. Those variants are left alone.
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
(no changes since v1)
arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 1 +
arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 1 +
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 1 +
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi | 1 +
arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi | 1 +
arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi | 1 +
6 files changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
index 8b8ea8af165d..b4f328d3e1f6 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
@@ -104,6 +104,7 @@ ap_ts: touchscreen@5d {
interrupt-parent = <&tlmm>;
interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+ panel = <&panel>;
reset-gpios = <&tlmm 8 GPIO_ACTIVE_LOW>;
vdd-supply = <&pp3300_ts>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
index b3ba23a88a0b..88aeb415bd5b 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
@@ -116,6 +116,7 @@ ap_ts: touchscreen@14 {
interrupt-parent = <&tlmm>;
interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+ panel = <&panel>;
reset-gpios = <&tlmm 8 GPIO_ACTIVE_LOW>;
vdd-supply = <&pp3300_touch>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
index 269007d73162..c65f18ea3e5c 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
@@ -43,6 +43,7 @@ ap_ts: touchscreen@10 {
interrupt-parent = <&tlmm>;
interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+ panel = <&panel>;
post-power-on-delay-ms = <20>;
hid-descr-addr = <0x0001>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
index 6c5287bd27d6..d2aafd1ea672 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
@@ -102,6 +102,7 @@ ap_ts: touchscreen@10 {
interrupt-parent = <&tlmm>;
interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+ panel = <&panel>;
post-power-on-delay-ms = <20>;
hid-descr-addr = <0x0001>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
index 62ab6427dd65..e5d6a7898f8c 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
@@ -69,6 +69,7 @@ ap_ts: touchscreen@10 {
interrupt-parent = <&tlmm>;
interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+ panel = <&panel>;
post-power-on-delay-ms = <20>;
hid-descr-addr = <0x0001>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
index 2efa8a4bcda6..0e2b4d06b490 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
@@ -123,6 +123,7 @@ ap_ts: touchscreen@1 {
interrupt-parent = <&tlmm>;
interrupts = <9 IRQ_TYPE_EDGE_FALLING>;
+ panel = <&panel>;
post-power-on-delay-ms = <70>;
hid-descr-addr = <0x0001>;
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH v4 10/11] HID: i2c-hid: Do panel follower work on the system_wq
From: Douglas Anderson @ 2023-07-27 17:16 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, yangcong5, devicetree, Daniel Vetter, hsinyi,
Chris Morgan, linux-input, cros-qcom-dts-watchers,
Dmitry Torokhov, linux-kernel, dri-devel, Douglas Anderson
In-Reply-To: <20230727171750.633410-1-dianders@chromium.org>
Turning on an i2c-hid device can be a slow process. This is why
i2c-hid devices use PROBE_PREFER_ASYNCHRONOUS. Unfortunately, when
we're a panel follower the i2c-hid power up sequence now blocks the
power on of the panel. Let's fix that by scheduling the work on the
system_wq.
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
(no changes since v2)
Changes in v2:
- ihid_core_panel_prepare_work() is now static.
- Improve documentation for smp_wmb().
drivers/hid/i2c-hid/i2c-hid-core.c | 50 +++++++++++++++++++++++++++---
1 file changed, 46 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index fc3087a983f5..9601c0605fd9 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -110,7 +110,9 @@ struct i2c_hid {
struct i2chid_ops *ops;
struct drm_panel_follower panel_follower;
+ struct work_struct panel_follower_prepare_work;
bool is_panel_follower;
+ bool prepare_work_finished;
};
static const struct i2c_hid_quirks {
@@ -1062,10 +1064,12 @@ static int __do_i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
return ret;
}
-static int i2c_hid_core_panel_prepared(struct drm_panel_follower *follower)
+static void ihid_core_panel_prepare_work(struct work_struct *work)
{
- struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
+ struct i2c_hid *ihid = container_of(work, struct i2c_hid,
+ panel_follower_prepare_work);
struct hid_device *hid = ihid->hid;
+ int ret;
/*
* hid->version is set on the first power up. If it's still zero then
@@ -1073,15 +1077,52 @@ static int i2c_hid_core_panel_prepared(struct drm_panel_follower *follower)
* steps.
*/
if (!hid->version)
- return __do_i2c_hid_core_initial_power_up(ihid);
+ ret = __do_i2c_hid_core_initial_power_up(ihid);
+ else
+ ret = i2c_hid_core_resume(ihid);
- return i2c_hid_core_resume(ihid);
+ if (ret)
+ dev_warn(&ihid->client->dev, "Power on failed: %d\n", ret);
+ else
+ WRITE_ONCE(ihid->prepare_work_finished, true);
+
+ /*
+ * The work APIs provide a number of memory ordering guarantees
+ * including one that says that memory writes before schedule_work()
+ * are always visible to the work function, but they don't appear to
+ * guarantee that a write that happened in the work is visible after
+ * cancel_work_sync(). We'll add a write memory barrier here to match
+ * with i2c_hid_core_panel_unpreparing() to ensure that our write to
+ * prepare_work_finished is visible there.
+ */
+ smp_wmb();
+}
+
+static int i2c_hid_core_panel_prepared(struct drm_panel_follower *follower)
+{
+ struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
+
+ /*
+ * Powering on a touchscreen can be a slow process. Queue the work to
+ * the system workqueue so we don't block the panel's power up.
+ */
+ WRITE_ONCE(ihid->prepare_work_finished, false);
+ schedule_work(&ihid->panel_follower_prepare_work);
+
+ return 0;
}
static int i2c_hid_core_panel_unpreparing(struct drm_panel_follower *follower)
{
struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
+ cancel_work_sync(&ihid->panel_follower_prepare_work);
+
+ /* Match with ihid_core_panel_prepare_work() */
+ smp_rmb();
+ if (!READ_ONCE(ihid->prepare_work_finished))
+ return 0;
+
return i2c_hid_core_suspend(ihid, true);
}
@@ -1173,6 +1214,7 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
init_waitqueue_head(&ihid->wait);
mutex_init(&ihid->reset_lock);
+ INIT_WORK(&ihid->panel_follower_prepare_work, ihid_core_panel_prepare_work);
/* we need to allocate the command buffer without knowing the maximum
* size of the reports. Let's use HID_MIN_BUFFER_SIZE, then we do the
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH v4 09/11] HID: i2c-hid: Support being a panel follower
From: Douglas Anderson @ 2023-07-27 17:16 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, yangcong5, devicetree, Daniel Vetter, hsinyi,
Chris Morgan, linux-input, cros-qcom-dts-watchers,
Dmitry Torokhov, linux-kernel, dri-devel, Douglas Anderson
In-Reply-To: <20230727171750.633410-1-dianders@chromium.org>
As talked about in the patch ("drm/panel: Add a way for other devices
to follow panel state"), we really want to keep the power states of a
touchscreen and the panel it's attached to in sync with each other. In
that spirit, add support to i2c-hid to be a panel follower. This will
let the i2c-hid driver get informed when the panel is powered on and
off. From there we can match the i2c-hid device's power state to that
of the panel.
NOTE: this patch specifically _doesn't_ use pm_runtime to keep track
of / manage the power state of the i2c-hid device, even though my
first instinct said that would be the way to go. Specific problems
with using pm_runtime():
* The initial power up couldn't happen in a runtime resume function
since it create sub-devices and, apparently, that's not good to do
in your resume function.
* Managing our power state with pm_runtime meant fighting to make the
right thing happen at system suspend to prevent the system from
trying to resume us only to suspend us again. While this might be
able to be solved, it added complexity.
Overall the code without pm_runtime() ended up being smaller and
easier to understand.
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v4:
- Move panel follower alternative checks to wrapper functions.
- Rebase atop ("Suspend i2c-hid devices in remove").
Changes in v3:
- Add "depends on DRM || !DRM" to Kconfig to avoid randconfig error.
- Split more of the panel follower code out of the core.
Changes in v2:
- i2c_hid_core_panel_prepared() and ..._unpreparing() are now static.
drivers/hid/i2c-hid/Kconfig | 2 +
drivers/hid/i2c-hid/i2c-hid-core.c | 93 +++++++++++++++++++++++++++++-
2 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/i2c-hid/Kconfig b/drivers/hid/i2c-hid/Kconfig
index 3be17109301a..2bdb55203104 100644
--- a/drivers/hid/i2c-hid/Kconfig
+++ b/drivers/hid/i2c-hid/Kconfig
@@ -70,5 +70,7 @@ config I2C_HID_OF_GOODIX
config I2C_HID_CORE
tristate
+ # We need to call into panel code so if DRM=m, this can't be 'y'
+ depends on DRM || !DRM
endif
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 46658ed6380f..fc3087a983f5 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -38,6 +38,8 @@
#include <linux/mutex.h>
#include <asm/unaligned.h>
+#include <drm/drm_panel.h>
+
#include "../hid-ids.h"
#include "i2c-hid.h"
@@ -107,6 +109,8 @@ struct i2c_hid {
struct mutex reset_lock;
struct i2chid_ops *ops;
+ struct drm_panel_follower panel_follower;
+ bool is_panel_follower;
};
static const struct i2c_hid_quirks {
@@ -993,7 +997,7 @@ static int i2c_hid_core_resume(struct i2c_hid *ihid)
}
/**
- * i2c_hid_core_initial_power_up() - First time power up of the i2c-hid device.
+ * __do_i2c_hid_core_initial_power_up() - First time power up of the i2c-hid device.
* @ihid: The ihid object created during probe.
*
* This function is called at probe time.
@@ -1004,7 +1008,7 @@ static int i2c_hid_core_resume(struct i2c_hid *ihid)
*
* Return: 0 or error code.
*/
-static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
+static int __do_i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
{
struct i2c_client *client = ihid->client;
struct hid_device *hid = ihid->hid;
@@ -1058,6 +1062,83 @@ static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
return ret;
}
+static int i2c_hid_core_panel_prepared(struct drm_panel_follower *follower)
+{
+ struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
+ struct hid_device *hid = ihid->hid;
+
+ /*
+ * hid->version is set on the first power up. If it's still zero then
+ * this is the first power on so we should perform initial power up
+ * steps.
+ */
+ if (!hid->version)
+ return __do_i2c_hid_core_initial_power_up(ihid);
+
+ return i2c_hid_core_resume(ihid);
+}
+
+static int i2c_hid_core_panel_unpreparing(struct drm_panel_follower *follower)
+{
+ struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
+
+ return i2c_hid_core_suspend(ihid, true);
+}
+
+static const struct drm_panel_follower_funcs i2c_hid_core_panel_follower_funcs = {
+ .panel_prepared = i2c_hid_core_panel_prepared,
+ .panel_unpreparing = i2c_hid_core_panel_unpreparing,
+};
+
+static int i2c_hid_core_register_panel_follower(struct i2c_hid *ihid)
+{
+ struct device *dev = &ihid->client->dev;
+ int ret;
+
+ ihid->is_panel_follower = true;
+ ihid->panel_follower.funcs = &i2c_hid_core_panel_follower_funcs;
+
+ /*
+ * If we're not in control of our own power up/power down then we can't
+ * do the logic to manage wakeups. Give a warning if a user thought
+ * that was possible then force the capability off.
+ */
+ if (device_can_wakeup(dev)) {
+ dev_warn(dev, "Can't wakeup if following panel\n");
+ device_set_wakeup_capable(dev, false);
+ }
+
+ ret = drm_panel_add_follower(dev, &ihid->panel_follower);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
+{
+ /*
+ * If we're a panel follower, we'll register and do our initial power
+ * up when the panel turns on; otherwise we do it right away.
+ */
+ if (drm_is_panel_follower(&ihid->client->dev))
+ return i2c_hid_core_register_panel_follower(ihid);
+ else
+ return __do_i2c_hid_core_initial_power_up(ihid);
+}
+
+static void i2c_hid_core_final_power_down(struct i2c_hid *ihid)
+{
+ /*
+ * If we're a follower, the act of unfollowing will cause us to be
+ * powered down. Otherwise we need to manually do it.
+ */
+ if (ihid->is_panel_follower)
+ drm_panel_remove_follower(&ihid->panel_follower);
+ else
+ i2c_hid_core_suspend(ihid, true);
+}
+
int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
u16 hid_descriptor_address, u32 quirks)
{
@@ -1143,7 +1224,7 @@ void i2c_hid_core_remove(struct i2c_client *client)
struct i2c_hid *ihid = i2c_get_clientdata(client);
struct hid_device *hid;
- i2c_hid_core_suspend(ihid, true);
+ i2c_hid_core_final_power_down(ihid);
hid = ihid->hid;
hid_destroy_device(hid);
@@ -1171,6 +1252,9 @@ static int i2c_hid_core_pm_suspend(struct device *dev)
struct i2c_client *client = to_i2c_client(dev);
struct i2c_hid *ihid = i2c_get_clientdata(client);
+ if (ihid->is_panel_follower)
+ return 0;
+
return i2c_hid_core_suspend(ihid, false);
}
@@ -1179,6 +1263,9 @@ static int i2c_hid_core_pm_resume(struct device *dev)
struct i2c_client *client = to_i2c_client(dev);
struct i2c_hid *ihid = i2c_get_clientdata(client);
+ if (ihid->is_panel_follower)
+ return 0;
+
return i2c_hid_core_resume(ihid);
}
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH v4 08/11] HID: i2c-hid: Suspend i2c-hid devices in remove
From: Douglas Anderson @ 2023-07-27 17:16 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, yangcong5, devicetree, Daniel Vetter, hsinyi,
Chris Morgan, linux-input, cros-qcom-dts-watchers,
Dmitry Torokhov, linux-kernel, dri-devel, Douglas Anderson
In-Reply-To: <20230727171750.633410-1-dianders@chromium.org>
In the i2c-hid remove() function we currently try to power off,
depopulate our child device, and free our resources. That's OK, but...
* If the i2c-hid device is on a power rail that can't turn off (either
an always-on or a shared power rail) we won't try to put the device
in a low power state during remove(). This probably doesn't matter
for very many devices but it could be nice in some instances.
* If the i2c-hid device somehow manages to generate an interrupt after
we tried to power off it is conceivable that the interrupt could
arrive during or after the call to hid_destroy_device() but before
the call to free_irq(). That could cause a crash since our IRQ
handler isn't expecting it. One could imagine this happening in
the case where we couldn't turn off (see the previous bullet) or,
possibly, if the interrupt line could glitch shortly after the
device powered off.
Let's call the suspend code during remove to avoid these issues. That
will put the device into a low power state and also disable
interrupts.
Technically, one could consider this a "fix" of commit 4a200c3b9a40
("HID: i2c-hid: introduce HID over i2c specification implementation").
However, since the above bullet points are more theoretical than
problems seen on real systems and since the remove() of an i2c-hid
touchscreen isn't terribly likely to be called in production, it's
probably not worth the bother of trying to backport it.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v4:
- ("Suspend i2c-hid devices in remove") new for v4.
drivers/hid/i2c-hid/i2c-hid-core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index fa8a1ca43d7f..46658ed6380f 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -941,7 +941,7 @@ static void i2c_hid_core_shutdown_tail(struct i2c_hid *ihid)
ihid->ops->shutdown_tail(ihid->ops);
}
-static int i2c_hid_core_suspend(struct i2c_hid *ihid)
+static int i2c_hid_core_suspend(struct i2c_hid *ihid, bool force_poweroff)
{
struct i2c_client *client = ihid->client;
struct hid_device *hid = ihid->hid;
@@ -956,7 +956,7 @@ static int i2c_hid_core_suspend(struct i2c_hid *ihid)
disable_irq(client->irq);
- if (!device_may_wakeup(&client->dev))
+ if (force_poweroff || !device_may_wakeup(&client->dev))
i2c_hid_core_power_down(ihid);
return 0;
@@ -1143,7 +1143,7 @@ void i2c_hid_core_remove(struct i2c_client *client)
struct i2c_hid *ihid = i2c_get_clientdata(client);
struct hid_device *hid;
- i2c_hid_core_power_down(ihid);
+ i2c_hid_core_suspend(ihid, true);
hid = ihid->hid;
hid_destroy_device(hid);
@@ -1171,7 +1171,7 @@ static int i2c_hid_core_pm_suspend(struct device *dev)
struct i2c_client *client = to_i2c_client(dev);
struct i2c_hid *ihid = i2c_get_clientdata(client);
- return i2c_hid_core_suspend(ihid);
+ return i2c_hid_core_suspend(ihid, false);
}
static int i2c_hid_core_pm_resume(struct device *dev)
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH v4 07/11] HID: i2c-hid: Make suspend and resume into helper functions
From: Douglas Anderson @ 2023-07-27 17:16 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, yangcong5, devicetree, Daniel Vetter, hsinyi,
Chris Morgan, linux-input, cros-qcom-dts-watchers,
Dmitry Torokhov, linux-kernel, dri-devel, Douglas Anderson
In-Reply-To: <20230727171750.633410-1-dianders@chromium.org>
In a future patch we'd like to be able to call the current i2c-hid
suspend and resume functions from times other than system
suspend. Move the functions higher up in the file and have them take a
"struct i2c_hid" to make this simpler. We'll then add tiny wrappers of
the functions for use with system suspend.
This change is expected to have no functional effect.
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
(no changes since v1)
drivers/hid/i2c-hid/i2c-hid-core.c | 98 +++++++++++++++++-------------
1 file changed, 56 insertions(+), 42 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index d29e6421ecba..fa8a1ca43d7f 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -941,6 +941,57 @@ static void i2c_hid_core_shutdown_tail(struct i2c_hid *ihid)
ihid->ops->shutdown_tail(ihid->ops);
}
+static int i2c_hid_core_suspend(struct i2c_hid *ihid)
+{
+ struct i2c_client *client = ihid->client;
+ struct hid_device *hid = ihid->hid;
+ int ret;
+
+ ret = hid_driver_suspend(hid, PMSG_SUSPEND);
+ if (ret < 0)
+ return ret;
+
+ /* Save some power */
+ i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);
+
+ disable_irq(client->irq);
+
+ if (!device_may_wakeup(&client->dev))
+ i2c_hid_core_power_down(ihid);
+
+ return 0;
+}
+
+static int i2c_hid_core_resume(struct i2c_hid *ihid)
+{
+ struct i2c_client *client = ihid->client;
+ struct hid_device *hid = ihid->hid;
+ int ret;
+
+ if (!device_may_wakeup(&client->dev))
+ i2c_hid_core_power_up(ihid);
+
+ enable_irq(client->irq);
+
+ /* Instead of resetting device, simply powers the device on. This
+ * solves "incomplete reports" on Raydium devices 2386:3118 and
+ * 2386:4B33 and fixes various SIS touchscreens no longer sending
+ * data after a suspend/resume.
+ *
+ * However some ALPS touchpads generate IRQ storm without reset, so
+ * let's still reset them here.
+ */
+ if (ihid->quirks & I2C_HID_QUIRK_RESET_ON_RESUME)
+ ret = i2c_hid_hwreset(ihid);
+ else
+ ret = i2c_hid_set_power(ihid, I2C_HID_PWR_ON);
+
+ if (ret)
+ return ret;
+
+ return hid_driver_reset_resume(hid);
+}
+
/**
* i2c_hid_core_initial_power_up() - First time power up of the i2c-hid device.
* @ihid: The ihid object created during probe.
@@ -1115,61 +1166,24 @@ void i2c_hid_core_shutdown(struct i2c_client *client)
}
EXPORT_SYMBOL_GPL(i2c_hid_core_shutdown);
-static int i2c_hid_core_suspend(struct device *dev)
+static int i2c_hid_core_pm_suspend(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct i2c_hid *ihid = i2c_get_clientdata(client);
- struct hid_device *hid = ihid->hid;
- int ret;
-
- ret = hid_driver_suspend(hid, PMSG_SUSPEND);
- if (ret < 0)
- return ret;
- /* Save some power */
- i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);
-
- disable_irq(client->irq);
-
- if (!device_may_wakeup(&client->dev))
- i2c_hid_core_power_down(ihid);
-
- return 0;
+ return i2c_hid_core_suspend(ihid);
}
-static int i2c_hid_core_resume(struct device *dev)
+static int i2c_hid_core_pm_resume(struct device *dev)
{
- int ret;
struct i2c_client *client = to_i2c_client(dev);
struct i2c_hid *ihid = i2c_get_clientdata(client);
- struct hid_device *hid = ihid->hid;
- if (!device_may_wakeup(&client->dev))
- i2c_hid_core_power_up(ihid);
-
- enable_irq(client->irq);
-
- /* Instead of resetting device, simply powers the device on. This
- * solves "incomplete reports" on Raydium devices 2386:3118 and
- * 2386:4B33 and fixes various SIS touchscreens no longer sending
- * data after a suspend/resume.
- *
- * However some ALPS touchpads generate IRQ storm without reset, so
- * let's still reset them here.
- */
- if (ihid->quirks & I2C_HID_QUIRK_RESET_ON_RESUME)
- ret = i2c_hid_hwreset(ihid);
- else
- ret = i2c_hid_set_power(ihid, I2C_HID_PWR_ON);
-
- if (ret)
- return ret;
-
- return hid_driver_reset_resume(hid);
+ return i2c_hid_core_resume(ihid);
}
const struct dev_pm_ops i2c_hid_core_pm = {
- SYSTEM_SLEEP_PM_OPS(i2c_hid_core_suspend, i2c_hid_core_resume)
+ SYSTEM_SLEEP_PM_OPS(i2c_hid_core_pm_suspend, i2c_hid_core_pm_resume)
};
EXPORT_SYMBOL_GPL(i2c_hid_core_pm);
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH v4 06/11] HID: i2c-hid: Rearrange probe() to power things up later
From: Douglas Anderson @ 2023-07-27 17:16 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, yangcong5, devicetree, Daniel Vetter, hsinyi,
Chris Morgan, linux-input, cros-qcom-dts-watchers,
Dmitry Torokhov, linux-kernel, dri-devel, Douglas Anderson
In-Reply-To: <20230727171750.633410-1-dianders@chromium.org>
In a future patch, we want to change i2c-hid not to necessarily power
up the touchscreen during probe. In preparation for that, rearrange
the probe function so that we put as much stuff _before_ powering up
the device as possible.
This change is expected to have no functional effect.
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
(no changes since v2)
Changes in v2:
- i2c_hid_core_initial_power_up() is now static.
drivers/hid/i2c-hid/i2c-hid-core.c | 124 ++++++++++++++++++-----------
1 file changed, 77 insertions(+), 47 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 19d985c20a5c..d29e6421ecba 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -855,7 +855,8 @@ static int i2c_hid_init_irq(struct i2c_client *client)
irqflags = IRQF_TRIGGER_LOW;
ret = request_threaded_irq(client->irq, NULL, i2c_hid_irq,
- irqflags | IRQF_ONESHOT, client->name, ihid);
+ irqflags | IRQF_ONESHOT | IRQF_NO_AUTOEN,
+ client->name, ihid);
if (ret < 0) {
dev_warn(&client->dev,
"Could not register for %s interrupt, irq = %d,"
@@ -940,6 +941,72 @@ static void i2c_hid_core_shutdown_tail(struct i2c_hid *ihid)
ihid->ops->shutdown_tail(ihid->ops);
}
+/**
+ * i2c_hid_core_initial_power_up() - First time power up of the i2c-hid device.
+ * @ihid: The ihid object created during probe.
+ *
+ * This function is called at probe time.
+ *
+ * The initial power on is where we do some basic validation that the device
+ * exists, where we fetch the HID descriptor, and where we create the actual
+ * HID devices.
+ *
+ * Return: 0 or error code.
+ */
+static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
+{
+ struct i2c_client *client = ihid->client;
+ struct hid_device *hid = ihid->hid;
+ int ret;
+
+ ret = i2c_hid_core_power_up(ihid);
+ if (ret)
+ return ret;
+
+ /* Make sure there is something at this address */
+ ret = i2c_smbus_read_byte(client);
+ if (ret < 0) {
+ i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret);
+ ret = -ENXIO;
+ goto err;
+ }
+
+ ret = i2c_hid_fetch_hid_descriptor(ihid);
+ if (ret < 0) {
+ dev_err(&client->dev,
+ "Failed to fetch the HID Descriptor\n");
+ goto err;
+ }
+
+ enable_irq(client->irq);
+
+ hid->version = le16_to_cpu(ihid->hdesc.bcdVersion);
+ hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
+ hid->product = le16_to_cpu(ihid->hdesc.wProductID);
+
+ hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor,
+ hid->product);
+
+ snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X",
+ client->name, (u16)hid->vendor, (u16)hid->product);
+ strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys));
+
+ ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
+
+ ret = hid_add_device(hid);
+ if (ret) {
+ if (ret != -ENODEV)
+ hid_err(client, "can't add hid device: %d\n", ret);
+ goto err;
+ }
+
+ return 0;
+
+err:
+ i2c_hid_core_power_down(ihid);
+ return ret;
+}
+
int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
u16 hid_descriptor_address, u32 quirks)
{
@@ -966,16 +1033,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
if (!ihid)
return -ENOMEM;
- ihid->ops = ops;
-
- ret = i2c_hid_core_power_up(ihid);
- if (ret)
- return ret;
-
i2c_set_clientdata(client, ihid);
+ ihid->ops = ops;
ihid->client = client;
-
ihid->wHIDDescRegister = cpu_to_le16(hid_descriptor_address);
init_waitqueue_head(&ihid->wait);
@@ -986,28 +1047,12 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
* real computation later. */
ret = i2c_hid_alloc_buffers(ihid, HID_MIN_BUFFER_SIZE);
if (ret < 0)
- goto err_powered;
-
+ return ret;
device_enable_async_suspend(&client->dev);
- /* Make sure there is something at this address */
- ret = i2c_smbus_read_byte(client);
- if (ret < 0) {
- i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret);
- ret = -ENXIO;
- goto err_powered;
- }
-
- ret = i2c_hid_fetch_hid_descriptor(ihid);
- if (ret < 0) {
- dev_err(&client->dev,
- "Failed to fetch the HID Descriptor\n");
- goto err_powered;
- }
-
ret = i2c_hid_init_irq(client);
if (ret < 0)
- goto err_powered;
+ goto err_buffers_allocated;
hid = hid_allocate_device();
if (IS_ERR(hid)) {
@@ -1021,26 +1066,11 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
hid->ll_driver = &i2c_hid_ll_driver;
hid->dev.parent = &client->dev;
hid->bus = BUS_I2C;
- hid->version = le16_to_cpu(ihid->hdesc.bcdVersion);
- hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
- hid->product = le16_to_cpu(ihid->hdesc.wProductID);
-
hid->initial_quirks = quirks;
- hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor,
- hid->product);
-
- snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X",
- client->name, (u16)hid->vendor, (u16)hid->product);
- strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys));
-
- ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
- ret = hid_add_device(hid);
- if (ret) {
- if (ret != -ENODEV)
- hid_err(client, "can't add hid device: %d\n", ret);
+ ret = i2c_hid_core_initial_power_up(ihid);
+ if (ret)
goto err_mem_free;
- }
return 0;
@@ -1050,9 +1080,9 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
err_irq:
free_irq(client->irq, ihid);
-err_powered:
- i2c_hid_core_power_down(ihid);
+err_buffers_allocated:
i2c_hid_free_buffers(ihid);
+
return ret;
}
EXPORT_SYMBOL_GPL(i2c_hid_core_probe);
@@ -1062,6 +1092,8 @@ void i2c_hid_core_remove(struct i2c_client *client)
struct i2c_hid *ihid = i2c_get_clientdata(client);
struct hid_device *hid;
+ i2c_hid_core_power_down(ihid);
+
hid = ihid->hid;
hid_destroy_device(hid);
@@ -1069,8 +1101,6 @@ void i2c_hid_core_remove(struct i2c_client *client)
if (ihid->bufsize)
i2c_hid_free_buffers(ihid);
-
- i2c_hid_core_power_down(ihid);
}
EXPORT_SYMBOL_GPL(i2c_hid_core_remove);
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox