2018-12-12 17:36:18

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v5 0/7] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers.

Hi,

This is another patchset to try to cleanup a bit more the crossed
references for cros-ec driver between the MFD and the platform/chrome
subsystems.

The purpose of these patches is get rid of the different cros-ec attributes
from mfd/cros_ec_dev to its own sub-driver in platform/chrome. cros_ec_dev
continues instantiating the sub-devices but the sysfs attributes are owned
by the platform driver.E.g. The lightbar driver should own his sysfs
attributes and be instantiated only if the Embedded Controller has a
lightbar.

The patchset also adds the documentation of the sysfs attributes.

Most of the patches touches mfd subsystem and platform/chrome so I'd
suggest go all using and inmutable branch.

Best regards,
Enric

Changes in v5:
- Add static at cros_ec_attr_group.

Changes in v4:
- Added Reviewed-by tags.
- Moved mfd_remove_devices to another patch, it's already queued in Lee's tree.
- Removed patch 8 from the series, was a fix and it's already applied in current mainline.
- Use <ec-device-name> instead of <cros-ec> in documentation.
- Use default MFD_CROS_EC_CHARDEV in Kconfig.
- Changed the subject to specify the cros_ec_vbc driver.
- Fix typo s/th/the
- Get rid of the cros_ec_has_lightbar function.

Changes in v3:
- Removed cros_ec_remove from include file.
- Removed unneded check for ec_dev.
- Fixed build error as reported by the kbuild test robot.
- Do not print ec_platform name as is with dev_err is enough.

Changes in v2:
- Use devm only for the cros-ec core.
- Removed the two exported functions to attach/detach to the cros_class.
- Use dev_warn instead of dev_err when adding the lightbar.
- Removed unneeded check of ec_dev.
- 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.
- Create the attributes directly instead of use the attach/detach callbacks.
- 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.
- Removed ec_with_lightbar variable.

Enric Balletbo i Serra (7):
mfd / platform: cros_ec: use devm_mfd_add_devices
mfd / platform: cros_ec: move lightbar attributes to its own driver
mfd / platform: cros_ec: move vbc attributes to its own driver
mfd / platform: cros_ec: move debugfs attributes to its own driver
mfd / platform: cros_ec: move device sysfs attributes to its own
driver
mfd / platform: cros_ec_vbc: instantiate only if the EC has a VBC
NVRAM
platform/chrome: cros_ec_lightbar: instantiate only if the EC has a
lightbar

.../ABI/testing/sysfs-class-chromeos | 32 +++++
...sfs-class-chromeos-driver-cros-ec-lightbar | 74 ++++++++++
.../sysfs-class-chromeos-driver-cros-ec-vbc | 6 +
drivers/mfd/Kconfig | 1 -
drivers/mfd/cros_ec.c | 14 +-
drivers/mfd/cros_ec_dev.c | 89 +++++--------
drivers/mfd/cros_ec_dev.h | 6 -
drivers/platform/chrome/Kconfig | 47 ++++++-
drivers/platform/chrome/Makefile | 7 +-
drivers/platform/chrome/cros_ec_debugfs.c | 62 ++++++---
drivers/platform/chrome/cros_ec_i2c.c | 10 --
drivers/platform/chrome/cros_ec_lightbar.c | 126 +++++++++++-------
drivers/platform/chrome/cros_ec_lpc.c | 4 -
drivers/platform/chrome/cros_ec_spi.c | 11 --
drivers/platform/chrome/cros_ec_sysfs.c | 36 ++++-
drivers/platform/chrome/cros_ec_vbc.c | 59 +++++---
include/linux/mfd/cros_ec.h | 21 ---
17 files changed, 394 insertions(+), 211 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-class-chromeos
create mode 100644 Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-lightbar
create mode 100644 Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-vbc

--
2.19.2



2018-12-12 17:35:32

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v5 5/7] mfd / platform: cros_ec: move device sysfs attributes to its own driver

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 <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
Acked-for-MFD-by: Lee Jones <[email protected]>
---

Changes in v5:
- Add static at cros_ec_attr_group.

Changes in v4:
- Use <ec-device-name> instead of <cros-ec> in documentation.
- Use default MFD_CROS_EC_CHARDEV in Kconfig.

Changes in v3:
- Do not print ec_platform name as is with dev_err is enough.

Changes in v2:
- 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_lightbar.c | 2 +-
drivers/platform/chrome/cros_ec_sysfs.c | 36 ++++++++++++++++++-
include/linux/mfd/cros_ec.h | 3 --
8 files changed, 81 insertions(+), 17 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..5819699d66ec
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-chromeos
@@ -0,0 +1,32 @@
+What: /sys/class/chromeos/<ec-device-name>/flashinfo
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ Show the EC flash information.
+
+What: /sys/class/chromeos/<ec-device-name>/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/<ec-device-name>/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/<ec-device-name>/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 9955937b821d..b9ec2a798dbb 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 */
@@ -396,6 +390,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 6cbf5b69d156..5e2fde5ff63d 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
@@ -144,4 +141,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 MFD_CROS_EC_CHARDEV
+ 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_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index 80eed6317570..c22318ba93aa 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -594,7 +594,7 @@ static umode_t cros_ec_lightbar_attrs_are_visible(struct kobject *kobj,
return 0;
}

-struct attribute_group cros_ec_lightbar_attr_group = {
+static struct attribute_group cros_ec_lightbar_attr_group = {
.name = "lightbar",
.attrs = __lb_cmds_attrs,
.is_visible = cros_ec_lightbar_attrs_are_visible,
diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index f34a50121064..0ff5aa30c070 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,39 @@ 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 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 attributes. err=%d\n", 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 e50860d190db..8f2a8918bfa3 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -325,7 +325,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.2


2018-12-12 17:35:37

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v5 7/7] platform/chrome: cros_ec_lightbar: instantiate only if the EC has a lightbar

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-name>, 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 <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
---

Changes in v5: None
Changes in v4:
- Get rid of the cros_ec_has_lightbar function.

Changes in v3: None
Changes in v2:
- Removed ec_with_lightbar variable.

drivers/platform/chrome/cros_ec_lightbar.c | 53 ++++++++--------------
1 file changed, 18 insertions(+), 35 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index c22318ba93aa..c3e4e6e5211d 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,45 +563,32 @@ static struct attribute *__lb_cmds_attrs[] = {
NULL,
};

-static bool ec_has_lightbar(struct cros_ec_dev *ec)
-{
- 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 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;
-
- /* Only instantiate this stuff if the EC has a lightbar */
- if (ec_has_lightbar(ec)) {
- ec_with_lightbar = ec;
- return a->mode;
- }
- return 0;
-}
-
-static struct attribute_group cros_ec_lightbar_attr_group = {
+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)
{
struct cros_ec_dev *ec_dev = dev_get_drvdata(pd->dev.parent);
+ struct cros_ec_platform *pdata = dev_get_platdata(ec_dev->dev);
struct device *dev = &pd->dev;
int ret;

+ /*
+ * Only instantiate the lightbar if the EC name is 'cros_ec'. Other EC
+ * devices like 'cros_pd' doesn't have a lightbar.
+ */
+ if (strcmp(pdata->ec_name, CROS_EC_DEV_NAME) != 0)
+ return -ENODEV;
+
+ /*
+ * Ask then for the lightbar version, if it's 0 then the 'cros_ec'
+ * doesn't have a lightbar.
+ */
+ if (!get_lightbar_version(ec_dev, NULL, NULL))
+ return -ENODEV;
+
/* Take control of the lightbar from the EC. */
lb_manual_suspend_ctrl(ec_dev, 1);

@@ -635,7 +618,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);
@@ -645,7 +628,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.2


2018-12-12 17:35:46

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v5 6/7] mfd / platform: cros_ec_vbc: instantiate only if the EC has a VBC NVRAM

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-name>, 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 <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
Acked-for-MFD-by: Lee Jones <[email protected]>
---

Changes in v5: None
Changes in v4:
- Changed the subject to specify the cros_ec_vbc driver.
- Fix typo s/th/the

Changes in v3:
- Fixed build error as reported by kbuild test robot.

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 | 19 ++++++++++++++++++-
drivers/platform/chrome/cros_ec_vbc.c | 16 ----------------
2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index b9ec2a798dbb..ed809fc97df8 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>
@@ -391,12 +392,16 @@ 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)
{
int retval = -ENOMEM;
+ struct device_node *node;
struct device *dev = &pdev->dev;
struct cros_ec_platform *ec_platform = dev_get_platdata(dev);
struct cros_ec_dev *ec = kzalloc(sizeof(*ec), GFP_KERNEL);
@@ -485,6 +490,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);
+ }
+
return 0;

failed:
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.2


2018-12-12 17:35:57

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v5 2/7] mfd / platform: cros_ec: move lightbar attributes to its own driver

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 <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
---

Changes in v5: None
Changes in v4:
- Use <ec-device-name> instead <cros-ec> in documentation.

Changes in v3:
- Removed unneded check for ec_dev.

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 | 11 +++
drivers/platform/chrome/Makefile | 3 +-
drivers/platform/chrome/cros_ec_lightbar.c | 95 ++++++++++++++-----
include/linux/mfd/cros_ec.h | 1 -
7 files changed, 173 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..57a037791403
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-lightbar
@@ -0,0 +1,74 @@
+What: /sys/class/chromeos/<ec-device-name>/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/<ec-device-name>/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/<ec-device-name>/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/<ec-device-name>/lightbar/program
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ This allows you to upload and run custom lightbar sequences.
+
+What: /sys/class/chromeos/<ec-device-name>/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/<ec-device-name>/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/<ec-device-name>/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 2d0fee488c5a..b227718e0ec2 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,
};
@@ -395,6 +394,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;
@@ -470,9 +473,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) {
@@ -480,6 +480,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");

@@ -494,9 +503,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);
@@ -525,8 +531,6 @@ static __maybe_unused int ec_device_suspend(struct device *dev)

cros_ec_debugfs_suspend(ec);

- lb_suspend(ec);
-
return 0;
}

@@ -536,8 +540,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..6c05752a3334 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -111,4 +111,15 @@ 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
+ default 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..80eed6317570 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,72 @@ 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;
+
+ /* 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 977ebaa78e99..1e9b569564ea 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -327,7 +327,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.2


2018-12-12 17:36:23

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v5 1/7] mfd / platform: cros_ec: use devm_mfd_add_devices

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 <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
---

Changes in v5: None
Changes in v4:
- Added Reviewed-by tags.
- Moved mfd_remove_devices to another patch, it's already queued in Lee's tree.
- Removed patch 8 from the series, was a fix and it's already applied in current mainline.

Changes in v3:
- Removed cros_ec_remove from include file.

Changes in v2:
- Use devm only for the cros-ec core.

drivers/mfd/cros_ec.c | 14 +++-----------
drivers/platform/chrome/cros_ec_i2c.c | 10 ----------
drivers/platform/chrome/cros_ec_lpc.c | 4 ----
drivers/platform/chrome/cros_ec_spi.c | 11 -----------
include/linux/mfd/cros_ec.h | 10 ----------
5 files changed, 3 insertions(+), 46 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/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,
};

diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index de8b588c8776..977ebaa78e99 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -281,16 +281,6 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg);

-/**
- * cros_ec_remove() - Remove a ChromeOS EC.
- * @ec_dev: Device to register.
- *
- * Call this to deregister a ChromeOS EC, then clean up any private data.
- *
- * Return: 0 on success or negative error code.
- */
-int cros_ec_remove(struct cros_ec_device *ec_dev);
-
/**
* cros_ec_register() - Register a new ChromeOS EC, using the provided info.
* @ec_dev: Device to register.
--
2.19.2


2018-12-12 17:37:11

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v5 4/7] mfd / platform: cros_ec: move debugfs attributes to its own driver

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 <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
Acked-for-MFD-by: Lee Jones <[email protected]>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None
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 40c98808fa1c..9955937b821d 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -394,6 +394,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" },
};
@@ -489,9 +490,6 @@ static int ec_device_probe(struct platform_device *pdev)
"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");
-
return 0;

failed:
@@ -503,62 +501,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 1e9dc9626e84..6cbf5b69d156 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -133,4 +133,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 fdc3152cca1d..e50860d190db 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -328,10 +328,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.2


2018-12-12 17:37:27

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v5 3/7] mfd / platform: cros_ec: move vbc attributes to its own driver

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 <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
Acked-for-MFD-by: Lee Jones <[email protected]>
---

Changes in v5: None
Changes in v4:
- Use <ec-device-name> instead of <cros-ec> in documentation.

Changes in v3:
- Fixed build error as reported by the kbuild test robot.

Changes in v2:
- Removed unneeded check of ec_dev.

.../sysfs-class-chromeos-driver-cros-ec-vbc | 6 +++
drivers/mfd/cros_ec_dev.c | 2 +-
drivers/platform/chrome/Kconfig | 11 +++++
drivers/platform/chrome/Makefile | 3 +-
drivers/platform/chrome/cros_ec_vbc.c | 43 ++++++++++++++++++-
include/linux/mfd/cros_ec.h | 1 -
6 files changed, 62 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..38c5aaaaa89a
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-vbc
@@ -0,0 +1,6 @@
+What: /sys/class/chromeos/<ec-device-name>/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 b227718e0ec2..40c98808fa1c 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,
};

@@ -396,6 +395,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)
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 6c05752a3334..1e9dc9626e84 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -122,4 +122,15 @@ 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
+ default MFD_CROS_EC_CHARDEV
+ 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 1e9b569564ea..fdc3152cca1d 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -327,7 +327,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.2


2018-12-21 15:38:57

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v5 1/7] mfd / platform: cros_ec: use devm_mfd_add_devices

On Wed, 12 Dec 2018, Enric Balletbo i Serra 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 <[email protected]>
> Reviewed-by: Guenter Roeck <[email protected]>
> ---
>
> Changes in v5: None
> Changes in v4:
> - Added Reviewed-by tags.
> - Moved mfd_remove_devices to another patch, it's already queued in Lee's tree.
> - Removed patch 8 from the series, was a fix and it's already applied in current mainline.
>
> Changes in v3:
> - Removed cros_ec_remove from include file.
>
> Changes in v2:
> - Use devm only for the cros-ec core.
>
> drivers/mfd/cros_ec.c | 14 +++-----------
> drivers/platform/chrome/cros_ec_i2c.c | 10 ----------
> drivers/platform/chrome/cros_ec_lpc.c | 4 ----
> drivers/platform/chrome/cros_ec_spi.c | 11 -----------
> include/linux/mfd/cros_ec.h | 10 ----------
> 5 files changed, 3 insertions(+), 46 deletions(-)

For my own reference:
Acked-for-MFD-by: Lee Jones <[email protected]>

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2018-12-21 15:40:01

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers.

On Wed, 12 Dec 2018, Enric Balletbo i Serra wrote:

> Hi,
>
> This is another patchset to try to cleanup a bit more the crossed
> references for cros-ec driver between the MFD and the platform/chrome
> subsystems.
>
> The purpose of these patches is get rid of the different cros-ec attributes
> from mfd/cros_ec_dev to its own sub-driver in platform/chrome. cros_ec_dev
> continues instantiating the sub-devices but the sysfs attributes are owned
> by the platform driver.E.g. The lightbar driver should own his sysfs
> attributes and be instantiated only if the Embedded Controller has a
> lightbar.
>
> The patchset also adds the documentation of the sysfs attributes.
>
> Most of the patches touches mfd subsystem and platform/chrome so I'd
> suggest go all using and inmutable branch.

That's fine.

What else needs to happen with this set?

Any more Acks required?

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2018-12-21 18:25:16

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v5 2/7] mfd / platform: cros_ec: move lightbar attributes to its own driver

On Wed, 12 Dec 2018, Enric Balletbo i Serra wrote:

> 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 <[email protected]>
> Reviewed-by: Guenter Roeck <[email protected]>
> ---
>
> Changes in v5: None
> Changes in v4:
> - Use <ec-device-name> instead <cros-ec> in documentation.
>
> Changes in v3:
> - Removed unneded check for ec_dev.
>
> 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 | 11 +++
> drivers/platform/chrome/Makefile | 3 +-
> drivers/platform/chrome/cros_ec_lightbar.c | 95 ++++++++++++++-----
> include/linux/mfd/cros_ec.h | 1 -

For my own reference:
Acked-for-MFD-by: Lee Jones <[email protected]>

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2019-01-08 16:23:45

by Enric Balletbo Serra

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers.

Hi Lee,

Missatge de Lee Jones <[email protected]> del dia dv., 21 de des.
2018 a les 16:39:
>
> On Wed, 12 Dec 2018, Enric Balletbo i Serra wrote:
>
> > Hi,
> >
> > This is another patchset to try to cleanup a bit more the crossed
> > references for cros-ec driver between the MFD and the platform/chrome
> > subsystems.
> >
> > The purpose of these patches is get rid of the different cros-ec attributes
> > from mfd/cros_ec_dev to its own sub-driver in platform/chrome. cros_ec_dev
> > continues instantiating the sub-devices but the sysfs attributes are owned
> > by the platform driver.E.g. The lightbar driver should own his sysfs
> > attributes and be instantiated only if the Embedded Controller has a
> > lightbar.
> >
> > The patchset also adds the documentation of the sysfs attributes.
> >
> > Most of the patches touches mfd subsystem and platform/chrome so I'd
> > suggest go all using and inmutable branch.
>
> That's fine.
>
> What else needs to happen with this set?
>

I think the patchset is ready to be queued. Note that to apply cleanly
it depends on [1] which is already merged in your for-next branch.
What do you prefer, go through your repo or go through the
chrome-platform repo? Do you want Benson or I create an immutable
branch? I'm fine with whenever you decide.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=18e294ddafaeb80a1e2e10c9bd750a6cb8388d5b

> Any more Acks required?
>

I think that you, Guenter, Benson and I are fine with it, so not more
acks needed.

Thanks,
Enric

> --
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

2019-01-09 07:27:32

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers.

On Tue, 08 Jan 2019, Enric Balletbo Serra wrote:
> Missatge de Lee Jones <[email protected]> del dia dv., 21 de des.
> 2018 a les 16:39:
> >
> > On Wed, 12 Dec 2018, Enric Balletbo i Serra wrote:
> >
> > > Hi,
> > >
> > > This is another patchset to try to cleanup a bit more the crossed
> > > references for cros-ec driver between the MFD and the platform/chrome
> > > subsystems.
> > >
> > > The purpose of these patches is get rid of the different cros-ec attributes
> > > from mfd/cros_ec_dev to its own sub-driver in platform/chrome. cros_ec_dev
> > > continues instantiating the sub-devices but the sysfs attributes are owned
> > > by the platform driver.E.g. The lightbar driver should own his sysfs
> > > attributes and be instantiated only if the Embedded Controller has a
> > > lightbar.
> > >
> > > The patchset also adds the documentation of the sysfs attributes.
> > >
> > > Most of the patches touches mfd subsystem and platform/chrome so I'd
> > > suggest go all using and inmutable branch.
> >
> > That's fine.
> >
> > What else needs to happen with this set?
>
> I think the patchset is ready to be queued. Note that to apply cleanly
> it depends on [1] which is already merged in your for-next branch.
> What do you prefer, go through your repo or go through the
> chrome-platform repo? Do you want Benson or I create an immutable
> branch? I'm fine with whenever you decide.

Probably best if this goes through the MFD tree.

I have a PR pending upstream at the moment. Once I know what is
happening with that, I'll start taking patches/sets again.

> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=18e294ddafaeb80a1e2e10c9bd750a6cb8388d5b
>
> > Any more Acks required?
> >
>
> I think that you, Guenter, Benson and I are fine with it, so not more
> acks needed.

Thanks for the clarification.

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2019-01-09 09:19:01

by Enric Balletbo i Serra

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers.

Hi Lee,

On 9/1/19 8:25, Lee Jones wrote:
> On Tue, 08 Jan 2019, Enric Balletbo Serra wrote:
>> Missatge de Lee Jones <[email protected]> del dia dv., 21 de des.
>> 2018 a les 16:39:
>>>
>>> On Wed, 12 Dec 2018, Enric Balletbo i Serra wrote:
>>>
>>>> Hi,
>>>>
>>>> This is another patchset to try to cleanup a bit more the crossed
>>>> references for cros-ec driver between the MFD and the platform/chrome
>>>> subsystems.
>>>>
>>>> The purpose of these patches is get rid of the different cros-ec attributes
>>>> from mfd/cros_ec_dev to its own sub-driver in platform/chrome. cros_ec_dev
>>>> continues instantiating the sub-devices but the sysfs attributes are owned
>>>> by the platform driver.E.g. The lightbar driver should own his sysfs
>>>> attributes and be instantiated only if the Embedded Controller has a
>>>> lightbar.
>>>>
>>>> The patchset also adds the documentation of the sysfs attributes.
>>>>
>>>> Most of the patches touches mfd subsystem and platform/chrome so I'd
>>>> suggest go all using and inmutable branch.
>>>
>>> That's fine.
>>>
>>> What else needs to happen with this set?
>>
>> I think the patchset is ready to be queued. Note that to apply cleanly
>> it depends on [1] which is already merged in your for-next branch.
>> What do you prefer, go through your repo or go through the
>> chrome-platform repo? Do you want Benson or I create an immutable
>> branch? I'm fine with whenever you decide.
>
> Probably best if this goes through the MFD tree.
>

Sounds good to me, thanks
Enric

> I have a PR pending upstream at the moment. Once I know what is
> happening with that, I'll start taking patches/sets again.
>
>> [1]
>> https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=18e294ddafaeb80a1e2e10c9bd750a6cb8388d5b
>>
>>> Any more Acks required?
>>>
>>
>> I think that you, Guenter, Benson and I are fine with it, so not more
>> acks needed.
>
> Thanks for the clarification.
>

2019-01-16 18:31:38

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers.

On Wed, 12 Dec 2018, Enric Balletbo i Serra wrote:

> Hi,
>
> This is another patchset to try to cleanup a bit more the crossed
> references for cros-ec driver between the MFD and the platform/chrome
> subsystems.
>
> The purpose of these patches is get rid of the different cros-ec attributes
> from mfd/cros_ec_dev to its own sub-driver in platform/chrome. cros_ec_dev
> continues instantiating the sub-devices but the sysfs attributes are owned
> by the platform driver.E.g. The lightbar driver should own his sysfs
> attributes and be instantiated only if the Embedded Controller has a
> lightbar.
>
> The patchset also adds the documentation of the sysfs attributes.
>
> Most of the patches touches mfd subsystem and platform/chrome so I'd
> suggest go all using and inmutable branch.
>
> Best regards,
> Enric

[...]

> Enric Balletbo i Serra (7):
> mfd / platform: cros_ec: use devm_mfd_add_devices
> mfd / platform: cros_ec: move lightbar attributes to its own driver
> mfd / platform: cros_ec: move vbc attributes to its own driver
> mfd / platform: cros_ec: move debugfs attributes to its own driver
> mfd / platform: cros_ec: move device sysfs attributes to its own
> driver
> mfd / platform: cros_ec_vbc: instantiate only if the EC has a VBC
> NVRAM
> platform/chrome: cros_ec_lightbar: instantiate only if the EC has a
> lightbar

These patches have been applied to my local MFD tree. It will be
pushed when 5.0-rc3 has been released. The MFD tree needs rebasing
onto it since it contains all of the MFD patches due for v5.0.

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2019-01-31 09:30:27

by Enric Balletbo Serra

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers.

Hi Lee,

Missatge de Lee Jones <[email protected]> del dia dc., 16 de gen.
2019 a les 19:31:
>
> On Wed, 12 Dec 2018, Enric Balletbo i Serra wrote:
>
> > Hi,
> >
> > This is another patchset to try to cleanup a bit more the crossed
> > references for cros-ec driver between the MFD and the platform/chrome
> > subsystems.
> >
> > The purpose of these patches is get rid of the different cros-ec attributes
> > from mfd/cros_ec_dev to its own sub-driver in platform/chrome. cros_ec_dev
> > continues instantiating the sub-devices but the sysfs attributes are owned
> > by the platform driver.E.g. The lightbar driver should own his sysfs
> > attributes and be instantiated only if the Embedded Controller has a
> > lightbar.
> >
> > The patchset also adds the documentation of the sysfs attributes.
> >
> > Most of the patches touches mfd subsystem and platform/chrome so I'd
> > suggest go all using and inmutable branch.
> >
> > Best regards,
> > Enric
>
> [...]
>
> > Enric Balletbo i Serra (7):
> > mfd / platform: cros_ec: use devm_mfd_add_devices
> > mfd / platform: cros_ec: move lightbar attributes to its own driver
> > mfd / platform: cros_ec: move vbc attributes to its own driver
> > mfd / platform: cros_ec: move debugfs attributes to its own driver
> > mfd / platform: cros_ec: move device sysfs attributes to its own
> > driver
> > mfd / platform: cros_ec_vbc: instantiate only if the EC has a VBC
> > NVRAM
> > platform/chrome: cros_ec_lightbar: instantiate only if the EC has a
> > lightbar
>
> These patches have been applied to my local MFD tree. It will be
> pushed when 5.0-rc3 has been released. The MFD tree needs rebasing
> onto it since it contains all of the MFD patches due for v5.0.
>

Do you mind to push the patches in your tree, I've some patches that
depend on these to apply cleanly and I'd like to base my branch on top
of yours.

Thanks,
Enric

> --
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

2019-02-01 08:20:01

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] mfd / platform: cros_ec: move cros_ec sysfs attributes to its own drivers.

On Thu, 31 Jan 2019, Enric Balletbo Serra wrote:
> Missatge de Lee Jones <[email protected]> del dia dc., 16 de gen.
> 2019 a les 19:31:
> > On Wed, 12 Dec 2018, Enric Balletbo i Serra wrote:
> > > This is another patchset to try to cleanup a bit more the crossed
> > > references for cros-ec driver between the MFD and the platform/chrome
> > > subsystems.
> > >
> > > The purpose of these patches is get rid of the different cros-ec attributes
> > > from mfd/cros_ec_dev to its own sub-driver in platform/chrome. cros_ec_dev
> > > continues instantiating the sub-devices but the sysfs attributes are owned
> > > by the platform driver.E.g. The lightbar driver should own his sysfs
> > > attributes and be instantiated only if the Embedded Controller has a
> > > lightbar.
> > >
> > > The patchset also adds the documentation of the sysfs attributes.
> > >
> > > Most of the patches touches mfd subsystem and platform/chrome so I'd
> > > suggest go all using and inmutable branch.
> > >
> > > Best regards,
> > > Enric
> >
> > [...]
> >
> > > Enric Balletbo i Serra (7):
> > > mfd / platform: cros_ec: use devm_mfd_add_devices
> > > mfd / platform: cros_ec: move lightbar attributes to its own driver
> > > mfd / platform: cros_ec: move vbc attributes to its own driver
> > > mfd / platform: cros_ec: move debugfs attributes to its own driver
> > > mfd / platform: cros_ec: move device sysfs attributes to its own
> > > driver
> > > mfd / platform: cros_ec_vbc: instantiate only if the EC has a VBC
> > > NVRAM
> > > platform/chrome: cros_ec_lightbar: instantiate only if the EC has a
> > > lightbar
> >
> > These patches have been applied to my local MFD tree. It will be
> > pushed when 5.0-rc3 has been released. The MFD tree needs rebasing
> > onto it since it contains all of the MFD patches due for v5.0.
>
> Do you mind to push the patches in your tree, I've some patches that
> depend on these to apply cleanly and I'd like to base my branch on top
> of yours.

Thanks for the reminder. Will send it 1 min.

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2019-02-01 08:20:57

by Lee Jones

[permalink] [raw]
Subject: [GIT PULL] Immutable branch between MFD and Platform due for the v5.1 merge window

Enjoy!

The following changes since commit 49a57857aeea06ca831043acbb0fa5e0f50602fd:

Linux 5.0-rc3 (2019-01-21 13:14:44 +1300)

are available in the Git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-platform-v5.1

for you to fetch changes up to fd68bd0f5d4c2090c95f84e27b05d0836bcd6c0c:

platform/chrome: cros_ec_lightbar: Instantiate only if the EC has a lightbar (2019-02-01 08:09:27 +0000)

----------------------------------------------------------------
Immutable branch between MFD and Platform due for the v5.1 merge window

----------------------------------------------------------------
Enric Balletbo i Serra (7):
mfd / platform: cros_ec: Use devm_mfd_add_devices
mfd / platform: cros_ec: Move lightbar attributes to its own driver
mfd / platform: cros_ec: Move vbc attributes to its own driver
mfd / platform: cros_ec: Move debugfs attributes to its own driver
mfd / platform: cros_ec: Move device sysfs attributes to its own driver
mfd / platform: cros_ec_vbc: Instantiate only if the EC has a VBC NVRAM
platform/chrome: cros_ec_lightbar: Instantiate only if the EC has a lightbar

Documentation/ABI/testing/sysfs-class-chromeos | 32 ++++++
.../sysfs-class-chromeos-driver-cros-ec-lightbar | 74 ++++++++++++
.../sysfs-class-chromeos-driver-cros-ec-vbc | 6 +
drivers/mfd/Kconfig | 1 -
drivers/mfd/cros_ec.c | 14 +--
drivers/mfd/cros_ec_dev.c | 89 ++++++---------
drivers/mfd/cros_ec_dev.h | 6 -
drivers/platform/chrome/Kconfig | 47 +++++++-
drivers/platform/chrome/Makefile | 7 +-
drivers/platform/chrome/cros_ec_debugfs.c | 62 ++++++----
drivers/platform/chrome/cros_ec_i2c.c | 10 --
drivers/platform/chrome/cros_ec_lightbar.c | 126 +++++++++++++--------
drivers/platform/chrome/cros_ec_lpc.c | 4 -
drivers/platform/chrome/cros_ec_spi.c | 11 --
drivers/platform/chrome/cros_ec_sysfs.c | 36 +++++-
drivers/platform/chrome/cros_ec_vbc.c | 59 +++++++---
include/linux/mfd/cros_ec.h | 21 ----
17 files changed, 394 insertions(+), 211 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-class-chromeos
create mode 100644 Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-lightbar
create mode 100644 Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-vbc

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2019-02-06 20:08:00

by Benson Leung

[permalink] [raw]
Subject: Re: [GIT PULL] Immutable branch between MFD and Platform due for the v5.1 merge window

Hi Lee,

On Fri, Feb 01, 2019 at 08:19:38AM +0000, Lee Jones wrote:
> Enjoy!
>
> The following changes since commit 49a57857aeea06ca831043acbb0fa5e0f50602fd:
>
> Linux 5.0-rc3 (2019-01-21 13:14:44 +1300)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-platform-v5.1
>
> for you to fetch changes up to fd68bd0f5d4c2090c95f84e27b05d0836bcd6c0c:
>
> platform/chrome: cros_ec_lightbar: Instantiate only if the EC has a lightbar (2019-02-01 08:09:27 +0000)
>

Looks good. Applied to chrome-platform-5.1.

Thanks,
Benson

--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]


Attachments:
(No filename) (753.00 B)
signature.asc (849.00 B)
Download all attachments