Messages in this thread Patch in this message | | | From | Enric Balletbo i Serra <> | Subject | [PATCH 7/7] platform/chrome: cros_ec_lightbar: instantiate only if the EC has a lightbar. | Date | Thu, 22 Nov 2018 12:33:56 +0100 |
| |
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> ---
drivers/platform/chrome/cros_ec_lightbar.c | 29 +++++++++------------- 1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c index 31d22f594fac..d255264eb082 100644 --- a/drivers/platform/chrome/cros_ec_lightbar.c +++ b/drivers/platform/chrome/cros_ec_lightbar.c @@ -567,37 +567,28 @@ 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; + if (!!get_lightbar_version(ec_dev, NULL, NULL)) { + ec_with_lightbar = ec_dev; + return true; } - return 0; + + 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) @@ -611,6 +602,10 @@ static int cros_ec_lightbar_probe(struct platform_device *pd) 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); -- 2.19.1
| |