* [PATCH v2 1/8] mfd / platform: cros_ec: use devm_mfd_add_devices.
2018-11-26 17:51 [PATCH v2 0/8] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers Enric Balletbo i Serra
@ 2018-11-26 17:51 ` Enric Balletbo i Serra
2018-11-26 17:58 ` Guenter Roeck
2018-11-26 17:51 ` [PATCH v2 2/8] mfd / platform: cros_ec: move lightbar attributes to its own driver Enric Balletbo i Serra
` (6 subsequent siblings)
7 siblings, 1 reply; 11+ messages in thread
From: Enric Balletbo i Serra @ 2018-11-26 17:51 UTC (permalink / raw)
To: lee.jones
Cc: gwendal, drinkcat, linux-kernel, groeck, kernel, bleung, Olof Johansson
Use devm_mfd_add_devices() for adding cros-ec core MFD child devices. This
reduces the need of remove callback from platform/chrome for removing the
MFD child devices.
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes in v2:
- Use devm only for the cros-ec core.
drivers/mfd/cros_ec.c | 14 +++-----------
drivers/mfd/cros_ec_dev.c | 1 +
drivers/platform/chrome/cros_ec_i2c.c | 10 ----------
drivers/platform/chrome/cros_ec_lpc.c | 4 ----
drivers/platform/chrome/cros_ec_spi.c | 11 -----------
5 files changed, 4 insertions(+), 36 deletions(-)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index fe6f83766144..6acfe036d522 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -129,8 +129,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
}
}
- err = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, &ec_cell, 1,
- NULL, ec_dev->irq, NULL);
+ err = devm_mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, &ec_cell,
+ 1, NULL, ec_dev->irq, NULL);
if (err) {
dev_err(dev,
"Failed to register Embedded Controller subdevice %d\n",
@@ -147,7 +147,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
* - the EC is responsive at init time (it is not true for a
* sensor hub.
*/
- err = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO,
+ err = devm_mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO,
&ec_pd_cell, 1, NULL, ec_dev->irq, NULL);
if (err) {
dev_err(dev,
@@ -181,14 +181,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
}
EXPORT_SYMBOL(cros_ec_register);
-int cros_ec_remove(struct cros_ec_device *ec_dev)
-{
- mfd_remove_devices(ec_dev->dev);
-
- return 0;
-}
-EXPORT_SYMBOL(cros_ec_remove);
-
#ifdef CONFIG_PM_SLEEP
int cros_ec_suspend(struct cros_ec_device *ec_dev)
{
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 8f9d6964173e..ff788d3e6d5f 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -493,6 +493,7 @@ static int ec_device_remove(struct platform_device *pdev)
cros_ec_debugfs_remove(ec);
+ mfd_remove_devices(ec->dev);
cdev_del(&ec->cdev);
device_unregister(&ec->class_dev);
return 0;
diff --git a/drivers/platform/chrome/cros_ec_i2c.c b/drivers/platform/chrome/cros_ec_i2c.c
index ef9b4763356f..9a009eaa4ada 100644
--- a/drivers/platform/chrome/cros_ec_i2c.c
+++ b/drivers/platform/chrome/cros_ec_i2c.c
@@ -317,15 +317,6 @@ static int cros_ec_i2c_probe(struct i2c_client *client,
return 0;
}
-static int cros_ec_i2c_remove(struct i2c_client *client)
-{
- struct cros_ec_device *ec_dev = i2c_get_clientdata(client);
-
- cros_ec_remove(ec_dev);
-
- return 0;
-}
-
#ifdef CONFIG_PM_SLEEP
static int cros_ec_i2c_suspend(struct device *dev)
{
@@ -376,7 +367,6 @@ static struct i2c_driver cros_ec_driver = {
.pm = &cros_ec_i2c_pm_ops,
},
.probe = cros_ec_i2c_probe,
- .remove = cros_ec_i2c_remove,
.id_table = cros_ec_i2c_id,
};
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index e1b75775cd4a..14684a56e40f 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -327,7 +327,6 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
static int cros_ec_lpc_remove(struct platform_device *pdev)
{
- struct cros_ec_device *ec_dev;
struct acpi_device *adev;
adev = ACPI_COMPANION(&pdev->dev);
@@ -335,9 +334,6 @@ static int cros_ec_lpc_remove(struct platform_device *pdev)
acpi_remove_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
cros_ec_lpc_acpi_notify);
- ec_dev = platform_get_drvdata(pdev);
- cros_ec_remove(ec_dev);
-
return 0;
}
diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
index 2060d1483043..6cfbc2835beb 100644
--- a/drivers/platform/chrome/cros_ec_spi.c
+++ b/drivers/platform/chrome/cros_ec_spi.c
@@ -685,16 +685,6 @@ static int cros_ec_spi_probe(struct spi_device *spi)
return 0;
}
-static int cros_ec_spi_remove(struct spi_device *spi)
-{
- struct cros_ec_device *ec_dev;
-
- ec_dev = spi_get_drvdata(spi);
- cros_ec_remove(ec_dev);
-
- return 0;
-}
-
#ifdef CONFIG_PM_SLEEP
static int cros_ec_spi_suspend(struct device *dev)
{
@@ -733,7 +723,6 @@ static struct spi_driver cros_ec_driver_spi = {
.pm = &cros_ec_spi_pm_ops,
},
.probe = cros_ec_spi_probe,
- .remove = cros_ec_spi_remove,
.id_table = cros_ec_spi_id,
};
--
2.19.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/8] mfd / platform: cros_ec: use devm_mfd_add_devices.
2018-11-26 17:51 ` [PATCH v2 1/8] mfd / platform: cros_ec: use devm_mfd_add_devices Enric Balletbo i Serra
@ 2018-11-26 17:58 ` Guenter Roeck
0 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2018-11-26 17:58 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Lee Jones, Gwendal Grignou, Nicolas Boichat, linux-kernel,
Guenter Roeck, kernel, Benson Leung, Olof Johansson
Hi Enric,
On Mon, Nov 26, 2018 at 9:52 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Use devm_mfd_add_devices() for adding cros-ec core MFD child devices. This
> reduces the need of remove callback from platform/chrome for removing the
> MFD child devices.
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>
> Changes in v2:
> - Use devm only for the cros-ec core.
>
...
> -int cros_ec_remove(struct cros_ec_device *ec_dev)
> -{
> - mfd_remove_devices(ec_dev->dev);
> -
> - return 0;
> -}
> -EXPORT_SYMBOL(cros_ec_remove);
> -
You should also remove the function declaration from
include/linux/mfd/cros_ec.h.
Guenter
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/8] mfd / platform: cros_ec: move lightbar attributes to its own driver.
2018-11-26 17:51 [PATCH v2 0/8] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers Enric Balletbo i Serra
2018-11-26 17:51 ` [PATCH v2 1/8] mfd / platform: cros_ec: use devm_mfd_add_devices Enric Balletbo i Serra
@ 2018-11-26 17:51 ` Enric Balletbo i Serra
2018-11-26 17:51 ` [PATCH v2 3/8] mfd / platform: cros_ec: move vbc " Enric Balletbo i Serra
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Enric Balletbo i Serra @ 2018-11-26 17:51 UTC (permalink / raw)
To: lee.jones
Cc: gwendal, drinkcat, linux-kernel, groeck, kernel, bleung, Olof Johansson
The entire way how cros sysfs attibutes are created is broken.
cros_ec_lightbar should be its own driver and its attributes should be
associated with a lightbar driver not the mfd driver. In order to retain
the path, the lightbar attributes are attached to the cros_class.
The patch also adds the sysfs documentation.
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes in v2:
- Removed the two exported functions to attach/detach to the cros_class.
- Use dev_warn instead of dev_err when adding the lightbar.
...sfs-class-chromeos-driver-cros-ec-lightbar | 74 +++++++++++++
drivers/mfd/cros_ec_dev.c | 24 +++--
drivers/mfd/cros_ec_dev.h | 6 --
drivers/platform/chrome/Kconfig | 10 ++
drivers/platform/chrome/Makefile | 3 +-
drivers/platform/chrome/cros_ec_lightbar.c | 100 ++++++++++++++----
include/linux/mfd/cros_ec.h | 1 -
7 files changed, 177 insertions(+), 41 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-lightbar
diff --git a/Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-lightbar b/Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-lightbar
new file mode 100644
index 000000000000..4893fd2e956f
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-lightbar
@@ -0,0 +1,74 @@
+What: /sys/class/chromeos/<cros-ec>/lightbar/brightness
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ Writing to this file adjusts the overall brightness of
+ the lightbar, separate from any color intensity. The
+ valid range is 0 (off) to 255 (maximum brightness).
+
+What: /sys/class/chromeos/<cros-ec>/lightbar/interval_msec
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ The lightbar is controlled by an embedded controller (EC),
+ which also manages the keyboard, battery charging, fans,
+ and other system hardware. To prevent unprivileged users
+ from interfering with the other EC functions, the rate at
+ which the lightbar control files can be read or written is
+ limited.
+
+ Reading this file will return the number of milliseconds
+ that must elapse between accessing any of the lightbar
+ functions through this interface. Going faster will simply
+ block until the necessary interval has lapsed. The interval
+ applies uniformly to all accesses of any kind by any user.
+
+What: /sys/class/chromeos/<cros-ec>/lightbar/led_rgb
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ This allows you to control each LED segment. If the
+ lightbar is already running one of the automatic
+ sequences, you probably won’t see anything change because
+ your color setting will be almost immediately replaced.
+ To get useful results, you should stop the lightbar
+ sequence first.
+
+ The values written to this file are sets of four integers,
+ indicating LED, RED, GREEN, BLUE. The LED number is 0 to 3
+ to select a single segment, or 4 to set all four segments
+ to the same value at once. The RED, GREEN, and BLUE
+ numbers should be in the range 0 (off) to 255 (maximum).
+ You can update more than one segment at a time by writing
+ more than one set of four integers.
+
+What: /sys/class/chromeos/<cros-ec>/lightbar/program
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ This allows you to upload and run custom lightbar sequences.
+
+What: /sys/class/chromeos/<cros-ec>/lightbar/sequence
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ The Pixel lightbar has a number of built-in sequences
+ that it displays under various conditions, such as at
+ power on, shut down, or while running. Reading from this
+ file displays the current sequence that the lightbar is
+ displaying. Writing to this file allows you to change the
+ sequence.
+
+What: /sys/class/chromeos/<cros-ec>/lightbar/userspace_control
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ This allows you to take the control of the lightbar. This
+ prevents the kernel from going through its normal
+ sequences.
+
+What: /sys/class/chromeos/<cros-ec>/lightbar/version
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ Show the information about the lightbar version.
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index ff788d3e6d5f..614f888c9d6d 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -36,7 +36,6 @@ static int ec_major;
static const struct attribute_group *cros_ec_groups[] = {
&cros_ec_attr_group,
- &cros_ec_lightbar_attr_group,
&cros_ec_vbc_attr_group,
NULL,
};
@@ -390,6 +389,10 @@ static const struct mfd_cell cros_usbpd_charger_cells[] = {
{ .name = "cros-usbpd-charger" }
};
+static const struct mfd_cell cros_ec_platform_cells[] = {
+ { .name = "cros-ec-lightbar" },
+};
+
static int ec_device_probe(struct platform_device *pdev)
{
int retval = -ENOMEM;
@@ -464,9 +467,6 @@ static int ec_device_probe(struct platform_device *pdev)
retval);
}
- /* Take control of the lightbar from the EC. */
- lb_manual_suspend_ctrl(ec, 1);
-
/* We can now add the sysfs class, we know which parameter to show */
retval = cdev_device_add(&ec->cdev, &ec->class_dev);
if (retval) {
@@ -474,6 +474,15 @@ static int ec_device_probe(struct platform_device *pdev)
goto failed;
}
+ retval = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
+ cros_ec_platform_cells,
+ ARRAY_SIZE(cros_ec_platform_cells),
+ NULL, 0, NULL);
+ if (retval)
+ dev_warn(ec->dev,
+ "failed to add cros-ec platform devices: %d\n",
+ retval);
+
if (cros_ec_debugfs_init(ec))
dev_warn(dev, "failed to create debugfs directory\n");
@@ -488,9 +497,6 @@ static int ec_device_remove(struct platform_device *pdev)
{
struct cros_ec_dev *ec = dev_get_drvdata(&pdev->dev);
- /* Let the EC take over the lightbar again. */
- lb_manual_suspend_ctrl(ec, 0);
-
cros_ec_debugfs_remove(ec);
mfd_remove_devices(ec->dev);
@@ -519,8 +525,6 @@ static __maybe_unused int ec_device_suspend(struct device *dev)
cros_ec_debugfs_suspend(ec);
- lb_suspend(ec);
-
return 0;
}
@@ -530,8 +534,6 @@ static __maybe_unused int ec_device_resume(struct device *dev)
cros_ec_debugfs_resume(ec);
- lb_resume(ec);
-
return 0;
}
diff --git a/drivers/mfd/cros_ec_dev.h b/drivers/mfd/cros_ec_dev.h
index 978d836a0248..ec750433455a 100644
--- a/drivers/mfd/cros_ec_dev.h
+++ b/drivers/mfd/cros_ec_dev.h
@@ -44,10 +44,4 @@ struct cros_ec_readmem {
#define CROS_EC_DEV_IOCXCMD _IOWR(CROS_EC_DEV_IOC, 0, struct cros_ec_command)
#define CROS_EC_DEV_IOCRDMEM _IOWR(CROS_EC_DEV_IOC, 1, struct cros_ec_readmem)
-/* Lightbar utilities */
-extern bool ec_has_lightbar(struct cros_ec_dev *ec);
-extern int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable);
-extern int lb_suspend(struct cros_ec_dev *ec);
-extern int lb_resume(struct cros_ec_dev *ec);
-
#endif /* _CROS_EC_DEV_H_ */
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 16b1615958aa..05bcefbe1926 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -111,4 +111,14 @@ config CROS_KBD_LED_BACKLIGHT
To compile this driver as a module, choose M here: the
module will be called cros_kbd_led_backlight.
+config CROS_EC_LIGHTBAR
+ tristate "Chromebook Pixel's lightbar support"
+ depends on MFD_CROS_EC_CHARDEV
+ help
+ This option exposes the Chromebook Pixel's lightbar to
+ userspace.
+
+ To compile this driver as a module, choose M here: the
+ module will be called cros_ec_lightbar.
+
endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index cd591bf872bb..3c29a4b405d5 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -3,7 +3,7 @@
obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o
obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o
obj-$(CONFIG_CHROMEOS_TBMC) += chromeos_tbmc.o
-cros_ec_ctl-objs := cros_ec_sysfs.o cros_ec_lightbar.o \
+cros_ec_ctl-objs := cros_ec_sysfs.o \
cros_ec_vbc.o cros_ec_debugfs.o
obj-$(CONFIG_CROS_EC_CTL) += cros_ec_ctl.o
obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o
@@ -13,3 +13,4 @@ cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o
obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpcs.o
obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o
obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT) += cros_kbd_led_backlight.o
+obj-$(CONFIG_CROS_EC_LIGHTBAR) += cros_ec_lightbar.o
diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index 68193bb53383..874d0b03a95d 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -33,6 +33,8 @@
#include <linux/uaccess.h>
#include <linux/slab.h>
+#define DRV_NAME "cros-ec-lightbar"
+
/* Rate-limit the lightbar interface to prevent DoS. */
static unsigned long lb_interval_jiffies = 50 * HZ / 1000;
@@ -373,7 +375,7 @@ static int lb_send_empty_cmd(struct cros_ec_dev *ec, uint8_t cmd)
return ret;
}
-int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
+static int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
{
struct ec_params_lightbar *param;
struct cros_ec_command *msg;
@@ -408,25 +410,6 @@ int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
return ret;
}
-EXPORT_SYMBOL(lb_manual_suspend_ctrl);
-
-int lb_suspend(struct cros_ec_dev *ec)
-{
- if (userspace_control || ec != ec_with_lightbar)
- return 0;
-
- return lb_send_empty_cmd(ec, LIGHTBAR_CMD_SUSPEND);
-}
-EXPORT_SYMBOL(lb_suspend);
-
-int lb_resume(struct cros_ec_dev *ec)
-{
- if (userspace_control || ec != ec_with_lightbar)
- return 0;
-
- return lb_send_empty_cmd(ec, LIGHTBAR_CMD_RESUME);
-}
-EXPORT_SYMBOL(lb_resume);
static ssize_t sequence_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
@@ -584,7 +567,7 @@ static struct attribute *__lb_cmds_attrs[] = {
NULL,
};
-bool ec_has_lightbar(struct cros_ec_dev *ec)
+static bool ec_has_lightbar(struct cros_ec_dev *ec)
{
return !!get_lightbar_version(ec, NULL, NULL);
}
@@ -616,4 +599,77 @@ struct attribute_group cros_ec_lightbar_attr_group = {
.attrs = __lb_cmds_attrs,
.is_visible = cros_ec_lightbar_attrs_are_visible,
};
-EXPORT_SYMBOL(cros_ec_lightbar_attr_group);
+
+static int cros_ec_lightbar_probe(struct platform_device *pd)
+{
+ struct cros_ec_dev *ec_dev = dev_get_drvdata(pd->dev.parent);
+ struct device *dev = &pd->dev;
+ int ret;
+
+ if (!ec_dev) {
+ dev_err(dev, "No EC dev found\n");
+ return -EINVAL;
+ }
+
+ /* Take control of the lightbar from the EC. */
+ lb_manual_suspend_ctrl(ec_dev, 1);
+
+ ret = sysfs_create_group(&ec_dev->class_dev.kobj,
+ &cros_ec_lightbar_attr_group);
+ if (ret < 0)
+ dev_err(dev, "failed to create %s attributes. err=%d\n",
+ cros_ec_lightbar_attr_group.name, ret);
+
+ return ret;
+}
+
+static int cros_ec_lightbar_remove(struct platform_device *pd)
+{
+ struct cros_ec_dev *ec_dev = dev_get_drvdata(pd->dev.parent);
+
+ sysfs_remove_group(&ec_dev->class_dev.kobj,
+ &cros_ec_lightbar_attr_group);
+
+ /* Let the EC take over the lightbar again. */
+ lb_manual_suspend_ctrl(ec_dev, 0);
+
+ return 0;
+}
+
+static int __maybe_unused cros_ec_lightbar_resume(struct device *dev)
+{
+ struct cros_ec_dev *ec_dev = dev_get_drvdata(dev);
+
+ if (userspace_control || ec_dev != ec_with_lightbar)
+ return 0;
+
+ return lb_send_empty_cmd(ec_dev, LIGHTBAR_CMD_RESUME);
+}
+
+static int __maybe_unused cros_ec_lightbar_suspend(struct device *dev)
+{
+ struct cros_ec_dev *ec_dev = dev_get_drvdata(dev);
+
+ if (userspace_control || ec_dev != ec_with_lightbar)
+ return 0;
+
+ return lb_send_empty_cmd(ec_dev, LIGHTBAR_CMD_SUSPEND);
+}
+
+static SIMPLE_DEV_PM_OPS(cros_ec_lightbar_pm_ops,
+ cros_ec_lightbar_suspend, cros_ec_lightbar_resume);
+
+static struct platform_driver cros_ec_lightbar_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .pm = &cros_ec_lightbar_pm_ops,
+ },
+ .probe = cros_ec_lightbar_probe,
+ .remove = cros_ec_lightbar_remove,
+};
+
+module_platform_driver(cros_ec_lightbar_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Expose the Chromebook Pixel's lightbar to userspace");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index e44e3ec8a9c7..c69d322dd4f9 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -335,7 +335,6 @@ u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
/* sysfs stuff */
extern struct attribute_group cros_ec_attr_group;
-extern struct attribute_group cros_ec_lightbar_attr_group;
extern struct attribute_group cros_ec_vbc_attr_group;
/* debugfs stuff */
--
2.19.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v2 3/8] mfd / platform: cros_ec: move vbc attributes to its own driver.
2018-11-26 17:51 [PATCH v2 0/8] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers Enric Balletbo i Serra
2018-11-26 17:51 ` [PATCH v2 1/8] mfd / platform: cros_ec: use devm_mfd_add_devices Enric Balletbo i Serra
2018-11-26 17:51 ` [PATCH v2 2/8] mfd / platform: cros_ec: move lightbar attributes to its own driver Enric Balletbo i Serra
@ 2018-11-26 17:51 ` Enric Balletbo i Serra
2018-11-26 20:06 ` kbuild test robot
2018-11-26 17:51 ` [PATCH v2 4/8] mfd / platform: cros_ec: move debugfs " Enric Balletbo i Serra
` (4 subsequent siblings)
7 siblings, 1 reply; 11+ messages in thread
From: Enric Balletbo i Serra @ 2018-11-26 17:51 UTC (permalink / raw)
To: lee.jones
Cc: gwendal, drinkcat, linux-kernel, groeck, kernel, bleung, Olof Johansson
The entire way how cros sysfs attibutes are created is broken.
cros_ec_vbc should be its own driver and its attributes should be
associated with a vbc driver not the mfd driver. In order to retain
the path, the vbc attributes are attached to the cros_class.
The patch also adds the sysfs documentation.
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes in v2:
- Remove unneeded check of ec_dev.
.../sysfs-class-chromeos-driver-cros-ec-vbc | 6 +++
drivers/mfd/cros_ec_dev.c | 14 +++++-
drivers/platform/chrome/Kconfig | 10 +++++
drivers/platform/chrome/Makefile | 3 +-
drivers/platform/chrome/cros_ec_vbc.c | 43 ++++++++++++++++++-
include/linux/mfd/cros_ec.h | 1 -
6 files changed, 73 insertions(+), 4 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-vbc
diff --git a/Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-vbc b/Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-vbc
new file mode 100644
index 000000000000..c256a2febf37
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-vbc
@@ -0,0 +1,6 @@
+What: /sys/class/chromeos/<cros-ec>/vbc/vboot_context
+Date: October 2015
+KernelVersion: 4.4
+Description:
+ Read/write the verified boot context data included on a
+ small nvram space on some EC implementations.
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 614f888c9d6d..3d7edc9ff604 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -36,7 +36,6 @@ static int ec_major;
static const struct attribute_group *cros_ec_groups[] = {
&cros_ec_attr_group,
- &cros_ec_vbc_attr_group,
NULL,
};
@@ -391,6 +390,7 @@ static const struct mfd_cell cros_usbpd_charger_cells[] = {
static const struct mfd_cell cros_ec_platform_cells[] = {
{ .name = "cros-ec-lightbar" },
+ { .name = "cros-ec-vbc" },
};
static int ec_device_probe(struct platform_device *pdev)
@@ -483,6 +483,18 @@ static int ec_device_probe(struct platform_device *pdev)
"failed to add cros-ec platform devices: %d\n",
retval);
+ /* Check whether this EC instance has a VBC NVRAM */
+ node = ec->ec_dev->dev->of_node;
+ if (of_property_read_bool(node, "google,has-vbc-nvram")) {
+ retval = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
+ cros_ec_vbc_cells,
+ ARRAY_SIZE(cros_ec_vbc_cells),
+ NULL, 0, NULL);
+ if (retval)
+ dev_warn(ec->dev, "failed to add VBC devices: %d\n",
+ retval);
+ }
+
if (cros_ec_debugfs_init(ec))
dev_warn(dev, "failed to create debugfs directory\n");
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 05bcefbe1926..29bd9837d520 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -121,4 +121,14 @@ config CROS_EC_LIGHTBAR
To compile this driver as a module, choose M here: the
module will be called cros_ec_lightbar.
+config CROS_EC_VBC
+ tristate "ChromeOS EC vboot context support"
+ depends on MFD_CROS_EC_CHARDEV && OF
+ help
+ This option exposes the ChromeOS EC vboot context nvram to
+ userspace.
+
+ To compile this driver as a module, choose M here: the
+ module will be called cros_ec_vbc.
+
endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 3c29a4b405d5..4081b7179df7 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -4,7 +4,7 @@ obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o
obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o
obj-$(CONFIG_CHROMEOS_TBMC) += chromeos_tbmc.o
cros_ec_ctl-objs := cros_ec_sysfs.o \
- cros_ec_vbc.o cros_ec_debugfs.o
+ cros_ec_debugfs.o
obj-$(CONFIG_CROS_EC_CTL) += cros_ec_ctl.o
obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o
obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o
@@ -14,3 +14,4 @@ obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpcs.o
obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o
obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT) += cros_kbd_led_backlight.o
obj-$(CONFIG_CROS_EC_LIGHTBAR) += cros_ec_lightbar.o
+obj-$(CONFIG_CROS_EC_VBC) += cros_ec_vbc.o
diff --git a/drivers/platform/chrome/cros_ec_vbc.c b/drivers/platform/chrome/cros_ec_vbc.c
index 5356f26bc022..da3bbf05e86f 100644
--- a/drivers/platform/chrome/cros_ec_vbc.c
+++ b/drivers/platform/chrome/cros_ec_vbc.c
@@ -22,8 +22,11 @@
#include <linux/platform_device.h>
#include <linux/mfd/cros_ec.h>
#include <linux/mfd/cros_ec_commands.h>
+#include <linux/module.h>
#include <linux/slab.h>
+#define DRV_NAME "cros-ec-vbc"
+
static ssize_t vboot_context_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *att, char *buf,
loff_t pos, size_t count)
@@ -132,4 +135,42 @@ struct attribute_group cros_ec_vbc_attr_group = {
.bin_attrs = cros_ec_vbc_bin_attrs,
.is_bin_visible = cros_ec_vbc_is_visible,
};
-EXPORT_SYMBOL(cros_ec_vbc_attr_group);
+
+static int cros_ec_vbc_probe(struct platform_device *pd)
+{
+ struct cros_ec_dev *ec_dev = dev_get_drvdata(pd->dev.parent);
+ struct device *dev = &pd->dev;
+ int ret;
+
+ ret = sysfs_create_group(&ec_dev->class_dev.kobj,
+ &cros_ec_vbc_attr_group);
+ if (ret < 0)
+ dev_err(dev, "failed to create %s attributes. err=%d\n",
+ cros_ec_vbc_attr_group.name, ret);
+
+ return ret;
+}
+
+static int cros_ec_vbc_remove(struct platform_device *pd)
+{
+ struct cros_ec_dev *ec_dev = dev_get_drvdata(pd->dev.parent);
+
+ sysfs_remove_group(&ec_dev->class_dev.kobj,
+ &cros_ec_vbc_attr_group);
+
+ return 0;
+}
+
+static struct platform_driver cros_ec_vbc_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ },
+ .probe = cros_ec_vbc_probe,
+ .remove = cros_ec_vbc_remove,
+};
+
+module_platform_driver(cros_ec_vbc_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Expose the vboot context nvram to userspace");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index c69d322dd4f9..a8b9f097eeb0 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -335,7 +335,6 @@ u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
/* sysfs stuff */
extern struct attribute_group cros_ec_attr_group;
-extern struct attribute_group cros_ec_vbc_attr_group;
/* debugfs stuff */
int cros_ec_debugfs_init(struct cros_ec_dev *ec);
--
2.19.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 3/8] mfd / platform: cros_ec: move vbc attributes to its own driver.
2018-11-26 17:51 ` [PATCH v2 3/8] mfd / platform: cros_ec: move vbc " Enric Balletbo i Serra
@ 2018-11-26 20:06 ` kbuild test robot
0 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2018-11-26 20:06 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: kbuild-all, lee.jones, gwendal, drinkcat, linux-kernel, groeck,
kernel, bleung, Olof Johansson
[-- Attachment #1: Type: text/plain, Size: 9012 bytes --]
Hi Enric,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.20-rc4 next-20181126]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Enric-Balletbo-i-Serra/mfd-platform-cros_ec-move-cros_ec-sysfs-attributes-to-its-own-drivers/20181127-032421
config: x86_64-randconfig-x017-201847 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
Note: the linux-review/Enric-Balletbo-i-Serra/mfd-platform-cros_ec-move-cros_ec-sysfs-attributes-to-its-own-drivers/20181127-032421 HEAD 08af4d071e30b3c486bdef3b3834f0f106f98187 builds fine.
It only hurts bisectibility.
All error/warnings (new ones prefixed by >>):
drivers//mfd/cros_ec_dev.c: In function 'ec_device_probe':
>> drivers//mfd/cros_ec_dev.c:487:2: error: 'node' undeclared (first use in this function); did you mean 'inode'?
node = ec->ec_dev->dev->of_node;
^~~~
inode
drivers//mfd/cros_ec_dev.c:487:2: note: each undeclared identifier is reported only once for each function it appears in
>> drivers//mfd/cros_ec_dev.c:488:6: error: implicit declaration of function 'of_property_read_bool' [-Werror=implicit-function-declaration]
if (of_property_read_bool(node, "google,has-vbc-nvram")) {
^~~~~~~~~~~~~~~~~~~~~
>> drivers//mfd/cros_ec_dev.c:490:7: error: 'cros_ec_vbc_cells' undeclared (first use in this function); did you mean 'cros_ec_rtc_cells'?
cros_ec_vbc_cells,
^~~~~~~~~~~~~~~~~
cros_ec_rtc_cells
In file included from include/linux/kernel.h:15:0,
from include/linux/list.h:9,
from include/linux/wait.h:7,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from drivers//mfd/cros_ec_dev.c:20:
>> include/linux/build_bug.h:29:45: error: bit-field '<anonymous>' width not an integer constant
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
^
include/linux/compiler.h:379:28: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
^~~~~~~~~~~~~~~~~
include/linux/kernel.h:72:59: note: in expansion of macro '__must_be_array'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^~~~~~~~~~~~~~~
>> drivers//mfd/cros_ec_dev.c:491:7: note: in expansion of macro 'ARRAY_SIZE'
ARRAY_SIZE(cros_ec_vbc_cells),
^~~~~~~~~~
cc1: some warnings being treated as errors
--
drivers/mfd/cros_ec_dev.c: In function 'ec_device_probe':
drivers/mfd/cros_ec_dev.c:487:2: error: 'node' undeclared (first use in this function); did you mean 'inode'?
node = ec->ec_dev->dev->of_node;
^~~~
inode
drivers/mfd/cros_ec_dev.c:487:2: note: each undeclared identifier is reported only once for each function it appears in
drivers/mfd/cros_ec_dev.c:488:6: error: implicit declaration of function 'of_property_read_bool' [-Werror=implicit-function-declaration]
if (of_property_read_bool(node, "google,has-vbc-nvram")) {
^~~~~~~~~~~~~~~~~~~~~
drivers/mfd/cros_ec_dev.c:490:7: error: 'cros_ec_vbc_cells' undeclared (first use in this function); did you mean 'cros_ec_rtc_cells'?
cros_ec_vbc_cells,
^~~~~~~~~~~~~~~~~
cros_ec_rtc_cells
In file included from include/linux/kernel.h:15:0,
from include/linux/list.h:9,
from include/linux/wait.h:7,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from drivers/mfd/cros_ec_dev.c:20:
>> include/linux/build_bug.h:29:45: error: bit-field '<anonymous>' width not an integer constant
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
^
include/linux/compiler.h:379:28: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
^~~~~~~~~~~~~~~~~
include/linux/kernel.h:72:59: note: in expansion of macro '__must_be_array'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^~~~~~~~~~~~~~~
drivers/mfd/cros_ec_dev.c:491:7: note: in expansion of macro 'ARRAY_SIZE'
ARRAY_SIZE(cros_ec_vbc_cells),
^~~~~~~~~~
cc1: some warnings being treated as errors
vim +/of_property_read_bool +488 drivers//mfd/cros_ec_dev.c
395
396 static int ec_device_probe(struct platform_device *pdev)
397 {
398 int retval = -ENOMEM;
399 struct device *dev = &pdev->dev;
400 struct cros_ec_platform *ec_platform = dev_get_platdata(dev);
401 struct cros_ec_dev *ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
402
403 if (!ec)
404 return retval;
405
406 dev_set_drvdata(dev, ec);
407 ec->ec_dev = dev_get_drvdata(dev->parent);
408 ec->dev = dev;
409 ec->cmd_offset = ec_platform->cmd_offset;
410 ec->features[0] = -1U; /* Not cached yet */
411 ec->features[1] = -1U; /* Not cached yet */
412 device_initialize(&ec->class_dev);
413 cdev_init(&ec->cdev, &fops);
414
415 /*
416 * Add the class device
417 * Link to the character device for creating the /dev entry
418 * in devtmpfs.
419 */
420 ec->class_dev.devt = MKDEV(ec_major, pdev->id);
421 ec->class_dev.class = &cros_class;
422 ec->class_dev.parent = dev;
423
424 retval = dev_set_name(&ec->class_dev, "%s", ec_platform->ec_name);
425 if (retval) {
426 dev_err(dev, "dev_set_name failed => %d\n", retval);
427 goto failed;
428 }
429
430 /* check whether this EC is a sensor hub. */
431 if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
432 cros_ec_sensors_register(ec);
433
434 /* Check whether this EC instance has CEC host command support */
435 if (cros_ec_check_features(ec, EC_FEATURE_CEC)) {
436 retval = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
437 cros_ec_cec_cells,
438 ARRAY_SIZE(cros_ec_cec_cells),
439 NULL, 0, NULL);
440 if (retval)
441 dev_err(ec->dev,
442 "failed to add cros-ec-cec device: %d\n",
443 retval);
444 }
445
446 /* Check whether this EC instance has RTC host command support */
447 if (cros_ec_check_features(ec, EC_FEATURE_RTC)) {
448 retval = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
449 cros_ec_rtc_cells,
450 ARRAY_SIZE(cros_ec_rtc_cells),
451 NULL, 0, NULL);
452 if (retval)
453 dev_err(ec->dev,
454 "failed to add cros-ec-rtc device: %d\n",
455 retval);
456 }
457
458 /* Check whether this EC instance has the PD charge manager */
459 if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) {
460 retval = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
461 cros_usbpd_charger_cells,
462 ARRAY_SIZE(cros_usbpd_charger_cells),
463 NULL, 0, NULL);
464 if (retval)
465 dev_err(ec->dev,
466 "failed to add cros-usbpd-charger device: %d\n",
467 retval);
468 }
469
470 /* We can now add the sysfs class, we know which parameter to show */
471 retval = cdev_device_add(&ec->cdev, &ec->class_dev);
472 if (retval) {
473 dev_err(dev, "cdev_device_add failed => %d\n", retval);
474 goto failed;
475 }
476
477 retval = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
478 cros_ec_platform_cells,
479 ARRAY_SIZE(cros_ec_platform_cells),
480 NULL, 0, NULL);
481 if (retval)
482 dev_warn(ec->dev,
483 "failed to add cros-ec platform devices: %d\n",
484 retval);
485
486 /* Check whether this EC instance has a VBC NVRAM */
> 487 node = ec->ec_dev->dev->of_node;
> 488 if (of_property_read_bool(node, "google,has-vbc-nvram")) {
489 retval = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
> 490 cros_ec_vbc_cells,
> 491 ARRAY_SIZE(cros_ec_vbc_cells),
492 NULL, 0, NULL);
493 if (retval)
494 dev_warn(ec->dev, "failed to add VBC devices: %d\n",
495 retval);
496 }
497
498 if (cros_ec_debugfs_init(ec))
499 dev_warn(dev, "failed to create debugfs directory\n");
500
501 return 0;
502
503 failed:
504 put_device(&ec->class_dev);
505 return retval;
506 }
507
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27619 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 4/8] mfd / platform: cros_ec: move debugfs attributes to its own driver.
2018-11-26 17:51 [PATCH v2 0/8] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers Enric Balletbo i Serra
` (2 preceding siblings ...)
2018-11-26 17:51 ` [PATCH v2 3/8] mfd / platform: cros_ec: move vbc " Enric Balletbo i Serra
@ 2018-11-26 17:51 ` Enric Balletbo i Serra
2018-11-26 17:51 ` [PATCH v2 5/8] mfd / platform: cros_ec: move device sysfs " Enric Balletbo i Serra
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Enric Balletbo i Serra @ 2018-11-26 17:51 UTC (permalink / raw)
To: lee.jones
Cc: gwendal, drinkcat, linux-kernel, groeck, kernel, bleung, Olof Johansson
The entire way how cros debugfs attibutes are created is broken.
cros_ec_debugfs should be its own driver and its attributes should be
associated with a debugfs driver not the mfd driver.
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes in v2:
- Add a "default MFD_CROS_EC_CHARDEV" in Kconfig for this.
- Remove the checks for missing debug_info, are not needed now.
- Remove a comment that no longer applies.
drivers/mfd/cros_ec_dev.c | 41 +--------------
drivers/platform/chrome/Kconfig | 11 ++++
drivers/platform/chrome/Makefile | 4 +-
drivers/platform/chrome/cros_ec_debugfs.c | 62 +++++++++++++++--------
include/linux/mfd/cros_ec.h | 6 ---
5 files changed, 56 insertions(+), 68 deletions(-)
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 3d7edc9ff604..308ec8dc92ba 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -389,6 +389,7 @@ static const struct mfd_cell cros_usbpd_charger_cells[] = {
};
static const struct mfd_cell cros_ec_platform_cells[] = {
+ { .name = "cros-ec-debugfs" },
{ .name = "cros-ec-lightbar" },
{ .name = "cros-ec-vbc" },
};
@@ -495,9 +496,6 @@ static int ec_device_probe(struct platform_device *pdev)
retval);
}
- if (cros_ec_debugfs_init(ec))
- dev_warn(dev, "failed to create debugfs directory\n");
-
return 0;
failed:
@@ -509,62 +507,25 @@ static int ec_device_remove(struct platform_device *pdev)
{
struct cros_ec_dev *ec = dev_get_drvdata(&pdev->dev);
- cros_ec_debugfs_remove(ec);
-
mfd_remove_devices(ec->dev);
cdev_del(&ec->cdev);
device_unregister(&ec->class_dev);
return 0;
}
-static void ec_device_shutdown(struct platform_device *pdev)
-{
- struct cros_ec_dev *ec = dev_get_drvdata(&pdev->dev);
-
- /* Be sure to clear up debugfs delayed works */
- cros_ec_debugfs_remove(ec);
-}
-
static const struct platform_device_id cros_ec_id[] = {
{ DRV_NAME, 0 },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(platform, cros_ec_id);
-static __maybe_unused int ec_device_suspend(struct device *dev)
-{
- struct cros_ec_dev *ec = dev_get_drvdata(dev);
-
- cros_ec_debugfs_suspend(ec);
-
- return 0;
-}
-
-static __maybe_unused int ec_device_resume(struct device *dev)
-{
- struct cros_ec_dev *ec = dev_get_drvdata(dev);
-
- cros_ec_debugfs_resume(ec);
-
- return 0;
-}
-
-static const struct dev_pm_ops cros_ec_dev_pm_ops = {
-#ifdef CONFIG_PM_SLEEP
- .suspend = ec_device_suspend,
- .resume = ec_device_resume,
-#endif
-};
-
static struct platform_driver cros_ec_dev_driver = {
.driver = {
.name = DRV_NAME,
- .pm = &cros_ec_dev_pm_ops,
},
.id_table = cros_ec_id,
.probe = ec_device_probe,
.remove = ec_device_remove,
- .shutdown = ec_device_shutdown,
};
static int __init cros_ec_dev_init(void)
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 29bd9837d520..4e4232b47764 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -131,4 +131,15 @@ config CROS_EC_VBC
To compile this driver as a module, choose M here: the
module will be called cros_ec_vbc.
+config CROS_EC_DEBUGFS
+ tristate "Export ChromeOS EC internals in DebugFS"
+ depends on MFD_CROS_EC_CHARDEV && DEBUG_FS
+ default MFD_CROS_EC_CHARDEV
+ help
+ This option exposes the ChromeOS EC device internals to
+ userspace.
+
+ To compile this driver as a module, choose M here: the
+ module will be called cros_ec_debugfs.
+
endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 4081b7179df7..12a5c4d18c17 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -3,8 +3,7 @@
obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o
obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o
obj-$(CONFIG_CHROMEOS_TBMC) += chromeos_tbmc.o
-cros_ec_ctl-objs := cros_ec_sysfs.o \
- cros_ec_debugfs.o
+cros_ec_ctl-objs := cros_ec_sysfs.o
obj-$(CONFIG_CROS_EC_CTL) += cros_ec_ctl.o
obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o
obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o
@@ -15,3 +14,4 @@ obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o
obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT) += cros_kbd_led_backlight.o
obj-$(CONFIG_CROS_EC_LIGHTBAR) += cros_ec_lightbar.o
obj-$(CONFIG_CROS_EC_VBC) += cros_ec_vbc.o
+obj-$(CONFIG_CROS_EC_DEBUGFS) += cros_ec_debugfs.o
diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index c62ee8e610a0..6cacac53dfce 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -23,12 +23,16 @@
#include <linux/fs.h>
#include <linux/mfd/cros_ec.h>
#include <linux/mfd/cros_ec_commands.h>
+#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/platform_device.h>
#include <linux/poll.h>
#include <linux/sched.h>
#include <linux/slab.h>
#include <linux/wait.h>
+#define DRV_NAME "cros-ec-debugfs"
+
#define LOG_SHIFT 14
#define LOG_SIZE (1 << LOG_SHIFT)
#define LOG_POLL_SEC 10
@@ -423,8 +427,9 @@ static int cros_ec_create_pdinfo(struct cros_ec_debugfs *debug_info)
return 0;
}
-int cros_ec_debugfs_init(struct cros_ec_dev *ec)
+static int cros_ec_debugfs_probe(struct platform_device *pd)
{
+ struct cros_ec_dev *ec = dev_get_drvdata(pd->dev.parent);
struct cros_ec_platform *ec_platform = dev_get_platdata(ec->dev);
const char *name = ec_platform->ec_name;
struct cros_ec_debugfs *debug_info;
@@ -453,40 +458,57 @@ int cros_ec_debugfs_init(struct cros_ec_dev *ec)
ec->debug_info = debug_info;
+ dev_set_drvdata(&pd->dev, ec);
+
return 0;
remove_debugfs:
debugfs_remove_recursive(debug_info->dir);
return ret;
}
-EXPORT_SYMBOL(cros_ec_debugfs_init);
-void cros_ec_debugfs_remove(struct cros_ec_dev *ec)
+static int cros_ec_debugfs_remove(struct platform_device *pd)
{
- if (!ec->debug_info)
- return;
+ struct cros_ec_dev *ec = dev_get_drvdata(pd->dev.parent);
debugfs_remove_recursive(ec->debug_info->dir);
cros_ec_cleanup_console_log(ec->debug_info);
+
+ return 0;
}
-EXPORT_SYMBOL(cros_ec_debugfs_remove);
-void cros_ec_debugfs_suspend(struct cros_ec_dev *ec)
+static int __maybe_unused cros_ec_debugfs_suspend(struct device *dev)
{
- /*
- * cros_ec_debugfs_init() failures are non-fatal; it's also possible
- * that we initted things but decided that console log wasn't supported.
- * We'll use the same set of checks that cros_ec_debugfs_remove() +
- * cros_ec_cleanup_console_log() end up using to handle those cases.
- */
- if (ec->debug_info && ec->debug_info->log_buffer.buf)
- cancel_delayed_work_sync(&ec->debug_info->log_poll_work);
+ struct cros_ec_dev *ec = dev_get_drvdata(dev);
+
+ cancel_delayed_work_sync(&ec->debug_info->log_poll_work);
+
+ return 0;
}
-EXPORT_SYMBOL(cros_ec_debugfs_suspend);
-void cros_ec_debugfs_resume(struct cros_ec_dev *ec)
+static int __maybe_unused cros_ec_debugfs_resume(struct device *dev)
{
- if (ec->debug_info && ec->debug_info->log_buffer.buf)
- schedule_delayed_work(&ec->debug_info->log_poll_work, 0);
+ struct cros_ec_dev *ec = dev_get_drvdata(dev);
+
+ schedule_delayed_work(&ec->debug_info->log_poll_work, 0);
+
+ return 0;
}
-EXPORT_SYMBOL(cros_ec_debugfs_resume);
+
+static SIMPLE_DEV_PM_OPS(cros_ec_debugfs_pm_ops,
+ cros_ec_debugfs_suspend, cros_ec_debugfs_resume);
+
+static struct platform_driver cros_ec_debugfs_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .pm = &cros_ec_debugfs_pm_ops,
+ },
+ .probe = cros_ec_debugfs_probe,
+ .remove = cros_ec_debugfs_remove,
+};
+
+module_platform_driver(cros_ec_debugfs_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Debug logs for ChromeOS EC");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index a8b9f097eeb0..69e90376043e 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -336,10 +336,4 @@ u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
/* sysfs stuff */
extern struct attribute_group cros_ec_attr_group;
-/* debugfs stuff */
-int cros_ec_debugfs_init(struct cros_ec_dev *ec);
-void cros_ec_debugfs_remove(struct cros_ec_dev *ec);
-void cros_ec_debugfs_suspend(struct cros_ec_dev *ec);
-void cros_ec_debugfs_resume(struct cros_ec_dev *ec);
-
#endif /* __LINUX_MFD_CROS_EC_H */
--
2.19.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v2 5/8] mfd / platform: cros_ec: move device sysfs attributes to its own driver.
2018-11-26 17:51 [PATCH v2 0/8] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers Enric Balletbo i Serra
` (3 preceding siblings ...)
2018-11-26 17:51 ` [PATCH v2 4/8] mfd / platform: cros_ec: move debugfs " Enric Balletbo i Serra
@ 2018-11-26 17:51 ` Enric Balletbo i Serra
2018-11-26 17:51 ` [PATCH v2 6/8] mfd / platform: cros_ec: instantiate only if th EC has a VBC NVRAM Enric Balletbo i Serra
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Enric Balletbo i Serra @ 2018-11-26 17:51 UTC (permalink / raw)
To: lee.jones
Cc: gwendal, drinkcat, linux-kernel, groeck, kernel, bleung, Olof Johansson
The entire way how cros debugfs attibutes are created is broken.
cros_ec_sysfs should be its own driver and its attributes should be
associated with the sysfs driver not the mfd driver.
The patch also adds the sysfs documentation.
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes in v2:
- Remove unnecessary check for ec_dev.
- Create the attributes directly instead of use the attach/detach callbacks.
.../ABI/testing/sysfs-class-chromeos | 32 ++++++++++++++++
drivers/mfd/Kconfig | 1 -
drivers/mfd/cros_ec_dev.c | 7 +---
drivers/platform/chrome/Kconfig | 14 +++++--
drivers/platform/chrome/Makefile | 3 +-
drivers/platform/chrome/cros_ec_sysfs.c | 38 ++++++++++++++++++-
include/linux/mfd/cros_ec.h | 3 --
7 files changed, 82 insertions(+), 16 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-class-chromeos
diff --git a/Documentation/ABI/testing/sysfs-class-chromeos b/Documentation/ABI/testing/sysfs-class-chromeos
new file mode 100644
index 000000000000..a5399c4ca82d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-chromeos
@@ -0,0 +1,32 @@
+What: /sys/class/chromeos/<cros-ec>/flashinfo
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ Show the EC flash information.
+
+What: /sys/class/chromeos/<cros-ec>/kb_wake_angle
+Date: March 2018
+KernelVersion: 4.17
+Description:
+ Control the keyboard wake lid angle. Values are between
+ 0 and 360. This file will also show the keyboard wake lid
+ angle by querying the hardware.
+
+What: /sys/class/chromeos/<cros-ec>/reboot
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ Tell the EC to reboot in various ways. Options are:
+ "cancel": Cancel a pending reboot.
+ "ro": Jump to RO without rebooting.
+ "rw": Jump to RW without rebooting.
+ "cold": Cold reboot.
+ "disable-jump": Disable jump until next reboot.
+ "hibernate": Hibernate the EC.
+ "at-shutdown": Reboot after an AP shutdown.
+
+What: /sys/class/chromeos/<cros-ec>/version
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ Show the information about the EC software and hardware.
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8c5dfdce4326..089ffb408918 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -214,7 +214,6 @@ config MFD_CROS_EC
config MFD_CROS_EC_CHARDEV
tristate "Chrome OS Embedded Controller userspace device interface"
depends on MFD_CROS_EC
- select CROS_EC_CTL
---help---
This driver adds support to talk with the ChromeOS EC from userspace.
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 308ec8dc92ba..590e705f253f 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -34,15 +34,9 @@
#define CROS_MAX_DEV 128
static int ec_major;
-static const struct attribute_group *cros_ec_groups[] = {
- &cros_ec_attr_group,
- NULL,
-};
-
static struct class cros_class = {
.owner = THIS_MODULE,
.name = "chromeos",
- .dev_groups = cros_ec_groups,
};
/* Basic communication */
@@ -391,6 +385,7 @@ static const struct mfd_cell cros_usbpd_charger_cells[] = {
static const struct mfd_cell cros_ec_platform_cells[] = {
{ .name = "cros-ec-debugfs" },
{ .name = "cros-ec-lightbar" },
+ { .name = "cros-ec-sysfs" },
{ .name = "cros-ec-vbc" },
};
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 4e4232b47764..b709cc5173a2 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -49,9 +49,6 @@ config CHROMEOS_TBMC
To compile this driver as a module, choose M here: the
module will be called chromeos_tbmc.
-config CROS_EC_CTL
- tristate
-
config CROS_EC_I2C
tristate "ChromeOS Embedded Controller (I2C)"
depends on MFD_CROS_EC && I2C
@@ -142,4 +139,15 @@ config CROS_EC_DEBUGFS
To compile this driver as a module, choose M here: the
module will be called cros_ec_debugfs.
+config CROS_EC_SYSFS
+ tristate "ChromeOS EC control and information through sysfs"
+ depends on MFD_CROS_EC_CHARDEV && SYSFS
+ default y
+ help
+ This option exposes some sysfs attributes to control and get
+ information from ChromeOS EC.
+
+ To compile this driver as a module, choose M here: the
+ module will be called cros_ec_sysfs.
+
endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 12a5c4d18c17..fdbee501931b 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -3,8 +3,6 @@
obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o
obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o
obj-$(CONFIG_CHROMEOS_TBMC) += chromeos_tbmc.o
-cros_ec_ctl-objs := cros_ec_sysfs.o
-obj-$(CONFIG_CROS_EC_CTL) += cros_ec_ctl.o
obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o
obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o
cros_ec_lpcs-objs := cros_ec_lpc.o cros_ec_lpc_reg.o
@@ -15,3 +13,4 @@ obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT) += cros_kbd_led_backlight.o
obj-$(CONFIG_CROS_EC_LIGHTBAR) += cros_ec_lightbar.o
obj-$(CONFIG_CROS_EC_VBC) += cros_ec_vbc.o
obj-$(CONFIG_CROS_EC_DEBUGFS) += cros_ec_debugfs.o
+obj-$(CONFIG_CROS_EC_SYSFS) += cros_ec_sysfs.o
diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index f34a50121064..bff24c19953d 100644
--- a/drivers/platform/chrome/cros_ec_sysfs.c
+++ b/drivers/platform/chrome/cros_ec_sysfs.c
@@ -34,6 +34,8 @@
#include <linux/types.h>
#include <linux/uaccess.h>
+#define DRV_NAME "cros-ec-sysfs"
+
/* Accessor functions */
static ssize_t reboot_show(struct device *dev,
@@ -353,7 +355,41 @@ struct attribute_group cros_ec_attr_group = {
.attrs = __ec_attrs,
.is_visible = cros_ec_ctrl_visible,
};
-EXPORT_SYMBOL(cros_ec_attr_group);
+
+static int cros_ec_sysfs_probe(struct platform_device *pd)
+{
+ struct cros_ec_dev *ec_dev = dev_get_drvdata(pd->dev.parent);
+ struct cros_ec_platform *ec_platform = dev_get_platdata(ec_dev->dev);
+ struct device *dev = &pd->dev;
+ int ret;
+
+ ret = sysfs_create_group(&ec_dev->class_dev.kobj, &cros_ec_attr_group);
+ if (ret < 0)
+ dev_err(dev, "failed to create %s attributes. err=%d\n",
+ ec_platform->ec_name, ret);
+
+ return ret;
+}
+
+static int cros_ec_sysfs_remove(struct platform_device *pd)
+{
+ struct cros_ec_dev *ec_dev = dev_get_drvdata(pd->dev.parent);
+
+ sysfs_remove_group(&ec_dev->class_dev.kobj, &cros_ec_attr_group);
+
+ return 0;
+}
+
+static struct platform_driver cros_ec_sysfs_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ },
+ .probe = cros_ec_sysfs_probe,
+ .remove = cros_ec_sysfs_remove,
+};
+
+module_platform_driver(cros_ec_sysfs_driver);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("ChromeOS EC control driver");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 69e90376043e..517f1320e101 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -333,7 +333,4 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev, bool *wake_event);
*/
u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
-/* sysfs stuff */
-extern struct attribute_group cros_ec_attr_group;
-
#endif /* __LINUX_MFD_CROS_EC_H */
--
2.19.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v2 6/8] mfd / platform: cros_ec: instantiate only if th EC has a VBC NVRAM.
2018-11-26 17:51 [PATCH v2 0/8] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers Enric Balletbo i Serra
` (4 preceding siblings ...)
2018-11-26 17:51 ` [PATCH v2 5/8] mfd / platform: cros_ec: move device sysfs " Enric Balletbo i Serra
@ 2018-11-26 17:51 ` Enric Balletbo i Serra
2018-11-26 17:51 ` [PATCH v2 7/8] platform/chrome: cros_ec_lightbar: instantiate only if the EC has a lightbar Enric Balletbo i Serra
2018-11-26 17:51 ` [PATCH v2 8/8] mfd: cros_ec: add a dev_release empty method Enric Balletbo i Serra
7 siblings, 0 replies; 11+ messages in thread
From: Enric Balletbo i Serra @ 2018-11-26 17:51 UTC (permalink / raw)
To: lee.jones
Cc: gwendal, drinkcat, linux-kernel, groeck, kernel, bleung, Olof Johansson
The cros-ec-vbc driver is DT-only and there is a DT property that
indicates if the EC has the VCB NVRAM, in such case instantiate the
driver but don't instantiate on the other cases.
To do this move the check code to its parent instead of play with the
attribute group visibility. This changes a bit the actual behaviour.
Before the patch if an EC doesn't have a VBC NVRAM an empty vbc folder
is created in /sys/class/chromeos/<ec device>, after the patch the empty
folder is not created, so, the folder is only created if the vbc is set.
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes in v2:
- Remove unnecessary IS_ENABLED.
- Remove dev_err message telling that VBC is found.
- Use dev_warn instead of dev_err as the error is ignored.
drivers/mfd/cros_ec_dev.c | 7 ++++++-
drivers/platform/chrome/cros_ec_vbc.c | 16 ----------------
2 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 590e705f253f..13b6dc26dc9a 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -21,6 +21,7 @@
#include <linux/mfd/core.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
+#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/pm.h>
#include <linux/slab.h>
@@ -386,7 +387,10 @@ static const struct mfd_cell cros_ec_platform_cells[] = {
{ .name = "cros-ec-debugfs" },
{ .name = "cros-ec-lightbar" },
{ .name = "cros-ec-sysfs" },
- { .name = "cros-ec-vbc" },
+};
+
+static const struct mfd_cell cros_ec_vbc_cells[] = {
+ { .name = "cros-ec-vbc" }
};
static int ec_device_probe(struct platform_device *pdev)
@@ -395,6 +399,7 @@ static int ec_device_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct cros_ec_platform *ec_platform = dev_get_platdata(dev);
struct cros_ec_dev *ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
+ struct device_node *node;
if (!ec)
return retval;
diff --git a/drivers/platform/chrome/cros_ec_vbc.c b/drivers/platform/chrome/cros_ec_vbc.c
index da3bbf05e86f..af9bfe6f385c 100644
--- a/drivers/platform/chrome/cros_ec_vbc.c
+++ b/drivers/platform/chrome/cros_ec_vbc.c
@@ -108,21 +108,6 @@ static ssize_t vboot_context_write(struct file *filp, struct kobject *kobj,
return data_sz;
}
-static umode_t cros_ec_vbc_is_visible(struct kobject *kobj,
- struct bin_attribute *a, int n)
-{
- struct device *dev = container_of(kobj, struct device, kobj);
- struct cros_ec_dev *ec = to_cros_ec_dev(dev);
- struct device_node *np = ec->ec_dev->dev->of_node;
-
- if (IS_ENABLED(CONFIG_OF) && np) {
- if (of_property_read_bool(np, "google,has-vbc-nvram"))
- return a->attr.mode;
- }
-
- return 0;
-}
-
static BIN_ATTR_RW(vboot_context, 16);
static struct bin_attribute *cros_ec_vbc_bin_attrs[] = {
@@ -133,7 +118,6 @@ static struct bin_attribute *cros_ec_vbc_bin_attrs[] = {
struct attribute_group cros_ec_vbc_attr_group = {
.name = "vbc",
.bin_attrs = cros_ec_vbc_bin_attrs,
- .is_bin_visible = cros_ec_vbc_is_visible,
};
static int cros_ec_vbc_probe(struct platform_device *pd)
--
2.19.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v2 7/8] platform/chrome: cros_ec_lightbar: instantiate only if the EC has a lightbar.
2018-11-26 17:51 [PATCH v2 0/8] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers Enric Balletbo i Serra
` (5 preceding siblings ...)
2018-11-26 17:51 ` [PATCH v2 6/8] mfd / platform: cros_ec: instantiate only if th EC has a VBC NVRAM Enric Balletbo i Serra
@ 2018-11-26 17:51 ` Enric Balletbo i Serra
2018-11-26 17:51 ` [PATCH v2 8/8] mfd: cros_ec: add a dev_release empty method Enric Balletbo i Serra
7 siblings, 0 replies; 11+ messages in thread
From: Enric Balletbo i Serra @ 2018-11-26 17:51 UTC (permalink / raw)
To: lee.jones
Cc: gwendal, drinkcat, linux-kernel, groeck, kernel, bleung, Olof Johansson
Due to the way attribute groups visibility work, the function
cros_ec_lightbar_attrs_are_visible is called multiple times, once per
attribute, and each of these calls makes an EC transaction. For what is
worth the EC log reports multiple errors on boot when the lightbar is
not available. Instead, check if the EC has a lightbar in the probe
function and only instantiate the device.
Ideally we should have instantiate the driver only if the
EC_FEATURE_LIGHTBAR is defined, but that's not possible because that flag
is not in the very first Pixel Chromebook (Link), only on Samus. So, the
driver is instantiated by his parent always.
This patch changes a bit the actual behaviour. Before the patch if an EC
doesn't have a lightbar an empty lightbar folder is created in
/sys/class/chromeos/<ec device>, after the patch the empty folder is not
created, so, the folder is only created if the lightbar exists.
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes in v2:
- Removed ec_with_lightbar variable.
drivers/platform/chrome/cros_ec_lightbar.c | 40 +++++++---------------
1 file changed, 12 insertions(+), 28 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index 874d0b03a95d..6f6a4cbfc3fc 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -43,7 +43,6 @@ static unsigned long lb_interval_jiffies = 50 * HZ / 1000;
* If this is true, we won't do anything during suspend/resume.
*/
static bool userspace_control;
-static struct cros_ec_dev *ec_with_lightbar;
static ssize_t interval_msec_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -381,9 +380,6 @@ static int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
struct cros_ec_command *msg;
int ret;
- if (ec != ec_with_lightbar)
- return 0;
-
msg = alloc_lightbar_cmd_msg(ec);
if (!msg)
return -ENOMEM;
@@ -567,37 +563,26 @@ static struct attribute *__lb_cmds_attrs[] = {
NULL,
};
-static bool ec_has_lightbar(struct cros_ec_dev *ec)
+static bool cros_ec_has_lightbar(struct cros_ec_dev *ec_dev)
{
- return !!get_lightbar_version(ec, NULL, NULL);
-}
-
-static umode_t cros_ec_lightbar_attrs_are_visible(struct kobject *kobj,
- struct attribute *a, int n)
-{
- struct device *dev = container_of(kobj, struct device, kobj);
- struct cros_ec_dev *ec = to_cros_ec_dev(dev);
- struct platform_device *pdev = to_platform_device(ec->dev);
+ struct platform_device *pdev = to_platform_device(ec_dev->dev);
struct cros_ec_platform *pdata = pdev->dev.platform_data;
int is_cros_ec;
is_cros_ec = strcmp(pdata->ec_name, CROS_EC_DEV_NAME);
if (is_cros_ec != 0)
- return 0;
+ return false;
- /* Only instantiate this stuff if the EC has a lightbar */
- if (ec_has_lightbar(ec)) {
- ec_with_lightbar = ec;
- return a->mode;
- }
- return 0;
+ if (!!get_lightbar_version(ec_dev, NULL, NULL))
+ return true;
+
+ return false;
}
struct attribute_group cros_ec_lightbar_attr_group = {
.name = "lightbar",
.attrs = __lb_cmds_attrs,
- .is_visible = cros_ec_lightbar_attrs_are_visible,
};
static int cros_ec_lightbar_probe(struct platform_device *pd)
@@ -606,10 +591,9 @@ static int cros_ec_lightbar_probe(struct platform_device *pd)
struct device *dev = &pd->dev;
int ret;
- if (!ec_dev) {
- dev_err(dev, "No EC dev found\n");
- return -EINVAL;
- }
+ /* Only instantiate this stuff if the EC has a lightbar */
+ if (!cros_ec_has_lightbar(ec_dev))
+ return -ENODEV;
/* Take control of the lightbar from the EC. */
lb_manual_suspend_ctrl(ec_dev, 1);
@@ -640,7 +624,7 @@ static int __maybe_unused cros_ec_lightbar_resume(struct device *dev)
{
struct cros_ec_dev *ec_dev = dev_get_drvdata(dev);
- if (userspace_control || ec_dev != ec_with_lightbar)
+ if (userspace_control)
return 0;
return lb_send_empty_cmd(ec_dev, LIGHTBAR_CMD_RESUME);
@@ -650,7 +634,7 @@ static int __maybe_unused cros_ec_lightbar_suspend(struct device *dev)
{
struct cros_ec_dev *ec_dev = dev_get_drvdata(dev);
- if (userspace_control || ec_dev != ec_with_lightbar)
+ if (userspace_control)
return 0;
return lb_send_empty_cmd(ec_dev, LIGHTBAR_CMD_SUSPEND);
--
2.19.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v2 8/8] mfd: cros_ec: add a dev_release empty method.
2018-11-26 17:51 [PATCH v2 0/8] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers Enric Balletbo i Serra
` (6 preceding siblings ...)
2018-11-26 17:51 ` [PATCH v2 7/8] platform/chrome: cros_ec_lightbar: instantiate only if the EC has a lightbar Enric Balletbo i Serra
@ 2018-11-26 17:51 ` Enric Balletbo i Serra
7 siblings, 0 replies; 11+ messages in thread
From: Enric Balletbo i Serra @ 2018-11-26 17:51 UTC (permalink / raw)
To: lee.jones; +Cc: gwendal, drinkcat, linux-kernel, groeck, kernel, bleung
Devices are required to provide a release method. This patch fixes the
following WARN():
[ 47.218707] ------------[ cut here ]------------
[ 47.223901] Device 'cros_ec' does not have a release() function, it is broken and must be fixed.
[ 47.234430] WARNING: CPU: 0 PID: 3585 at drivers/base/core.c:895 device_release+0x80/0x90
[ 47.243560] Modules linked in: btusb btrtl btintel btbcm bluetooth ecdh_generic [...]
[ 47.323851] CPU: 0 PID: 3585 Comm: rmmod Not tainted 4.20.0-rc2+ #29
[ 47.330947] Hardware name: Google Kevin (DT)
[ 47.335714] pstate: 40000005 (nZcv daif -PAN -UAO)
[ 47.341063] pc : device_release+0x80/0x90
[ 47.345537] lr : device_release+0x80/0x90
[ 47.350001] sp : ffff00000b17bc70
[ 47.353698] x29: ffff00000b17bc70 x28: ffff8000e48e9a80
[ 47.359629] x27: 0000000000000000 x26: 0000000000000000
[ 47.365561] x25: 0000000056000000 x24: 0000000000000015
[ 47.371492] x23: ffff8000f0248060 x22: ffff000000b700a0
[ 47.377414] x21: ffff8000edf56100 x20: ffff8000edd13028
[ 47.383346] x19: ffff8000edd13018 x18: 0000000000000095
[ 47.389278] x17: 0000000000000000 x16: 0000000000000000
[ 47.395209] x15: 0000000000000400 x14: 0000000000000400
[ 47.401131] x13: 00000000000001a7 x12: 0000000000000000
[ 47.407053] x11: 0000000000000001 x10: 0000000000000960
[ 47.412976] x9 : ffff00000b17b9b0 x8 : ffff8000e48ea440
[ 47.418898] x7 : ffff8000ee9090c0 x6 : ffff8000f7d0b0b8
[ 47.424830] x5 : ffff8000f7d0b0b8 x4 : 0000000000000000
[ 47.430752] x3 : ffff8000f7d11e68 x2 : ffff8000e48e9a80
[ 47.436674] x1 : 37d859939c964800 x0 : 0000000000000000
[ 47.442597] Call trace:
[ 47.445324] device_release+0x80/0x90
[ 47.449414] kobject_put+0x74/0xe8
[ 47.453210] device_unregister+0x20/0x30
[ 47.457592] ec_device_remove+0x34/0x48 [cros_ec_dev]
[ 47.463233] platform_drv_remove+0x28/0x48
[ 47.467805] device_release_driver_internal+0x1a8/0x240
[ 47.473630] driver_detach+0x40/0x80
[ 47.477609] bus_remove_driver+0x54/0xa8
[ 47.481986] driver_unregister+0x2c/0x58
[ 47.486355] platform_driver_unregister+0x10/0x18
[ 47.491599] cros_ec_dev_exit+0x1c/0x258 [cros_ec_dev]
[ 47.497338] __arm64_sys_delete_module+0x16c/0x1f8
[ 47.502689] el0_svc_common+0x84/0xd8
[ 47.506776] el0_svc_handler+0x2c/0x80
[ 47.510960] el0_svc+0x8/0xc
[ 47.514171] ---[ end trace 9087279fc8c03450 ]---
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes in v2:
- Fix WARN when unloading. This is new in these series.
drivers/mfd/cros_ec_dev.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 13b6dc26dc9a..bd561e91153a 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -35,9 +35,14 @@
#define CROS_MAX_DEV 128
static int ec_major;
+static void cros_ec_dev_release(struct device *dev)
+{
+}
+
static struct class cros_class = {
.owner = THIS_MODULE,
.name = "chromeos",
+ .dev_release = cros_ec_dev_release,
};
/* Basic communication */
--
2.19.1
^ permalink raw reply related [flat|nested] 11+ messages in thread