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 <enric.balletbo@collabora.com>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index c22318b..c3e4e6e 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -43,7 +43,6 @@
* 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 @@
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 @@
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 @@
{
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 @@
{
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);
