* [PATCH v3 0/1] platform/chrome: Update cros_ec sysfs attribute after sensors are found @ 2021-08-04 21:31 Gwendal Grignou 2021-08-04 21:31 ` [PATCH v3 1/1] platform/chrome: Poke kb_wake_angle attribute visibility when needed Gwendal Grignou 0 siblings, 1 reply; 7+ messages in thread From: Gwendal Grignou @ 2021-08-04 21:31 UTC (permalink / raw) To: dtor, jwerner, bleung, enric.balletbo, groeck Cc: linux-kernel, Gwendal Grignou Attribute "kb_wake_angle" in /sys/<cros_ec hw path>/chromeos/cros_ec, used to set at which angle the embedded controller must wake up the host, is only set when there is at least 2 accelerometers in the chromebook. The detection of these sensors is done in cros_ec_sensorhub, driver that can be probed after the cros_ec_sysfs driver that sets the attribute on behalf of the chromeos EC class driver. Therefore, we need to upgrade the sysfs group in the sensorhub probe routine. Gwendal Grignou (1): platform/chrome: Poke kb_wake_angle attribute visibility when needed drivers/platform/chrome/cros_ec_sensorhub.c | 6 +++++- drivers/platform/chrome/cros_ec_sysfs.c | 5 +++-- include/linux/platform_data/cros_ec_proto.h | 2 ++ 3 files changed, 10 insertions(+), 3 deletions(-) -- 2.32.0.554.ge1b32706d8-goog ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/1] platform/chrome: Poke kb_wake_angle attribute visibility when needed 2021-08-04 21:31 [PATCH v3 0/1] platform/chrome: Update cros_ec sysfs attribute after sensors are found Gwendal Grignou @ 2021-08-04 21:31 ` Gwendal Grignou 2022-11-15 4:10 ` Gwendal Grignou 0 siblings, 1 reply; 7+ messages in thread From: Gwendal Grignou @ 2021-08-04 21:31 UTC (permalink / raw) To: dtor, jwerner, bleung, enric.balletbo, groeck Cc: linux-kernel, Gwendal Grignou When the sensorhub detects 2 accelerometers, kb_wake_angle attribute in the chromeos EC class device needs to be visible. When detected, the sensorhub requests the class device to revisit the visibility of the kb_wake_angle attribute. Expose the attribute group to alter to close a potiential race between cros-ec-sensorhub and cros-ec-sysfs (that creates the attribute group on behalf of the class driver). Signed-off-by: Gwendal Grignou <gwendal@chromium.org> --- drivers/platform/chrome/cros_ec_sensorhub.c | 6 +++++- drivers/platform/chrome/cros_ec_sysfs.c | 5 +++-- include/linux/platform_data/cros_ec_proto.h | 2 ++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c b/drivers/platform/chrome/cros_ec_sensorhub.c index 9c4af76a9956e..8f24ed90b229c 100644 --- a/drivers/platform/chrome/cros_ec_sensorhub.c +++ b/drivers/platform/chrome/cros_ec_sensorhub.c @@ -106,8 +106,12 @@ static int cros_ec_sensorhub_register(struct device *dev, sensor_type[sensorhub->resp->info.type]++; } - if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) + if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) { ec->has_kb_wake_angle = true; + if (ec->group && sysfs_update_group(&ec->class_dev.kobj, + ec->group)) + dev_warn(dev, "Unable to update cros-ec-sysfs"); + } if (cros_ec_check_features(ec, EC_FEATURE_REFINED_TABLET_MODE_HYSTERESIS)) { diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c index f07eabcf9494c..17fe657f3ffd5 100644 --- a/drivers/platform/chrome/cros_ec_sysfs.c +++ b/drivers/platform/chrome/cros_ec_sysfs.c @@ -341,7 +341,8 @@ static int cros_ec_sysfs_probe(struct platform_device *pd) struct device *dev = &pd->dev; int ret; - ret = sysfs_create_group(&ec_dev->class_dev.kobj, &cros_ec_attr_group); + ec_dev->group = &cros_ec_attr_group; + ret = sysfs_create_group(&ec_dev->class_dev.kobj, ec_dev->group); if (ret < 0) dev_err(dev, "failed to create attributes. err=%d\n", ret); @@ -352,7 +353,7 @@ 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); + sysfs_remove_group(&ec_dev->class_dev.kobj, ec_dev->group); return 0; } diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h index 02599687770c5..c6dca260bbd5d 100644 --- a/include/linux/platform_data/cros_ec_proto.h +++ b/include/linux/platform_data/cros_ec_proto.h @@ -191,6 +191,7 @@ struct cros_ec_platform { /** * struct cros_ec_dev - ChromeOS EC device entry point. * @class_dev: Device structure used in sysfs. + * @groups: sysfs attributes groups for this EC. * @ec_dev: cros_ec_device structure to talk to the physical device. * @dev: Pointer to the platform device. * @debug_info: cros_ec_debugfs structure for debugging information. @@ -200,6 +201,7 @@ struct cros_ec_platform { */ struct cros_ec_dev { struct device class_dev; + const struct attribute_group *group; struct cros_ec_device *ec_dev; struct device *dev; struct cros_ec_debugfs *debug_info; -- 2.32.0.554.ge1b32706d8-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/1] platform/chrome: Poke kb_wake_angle attribute visibility when needed 2021-08-04 21:31 ` [PATCH v3 1/1] platform/chrome: Poke kb_wake_angle attribute visibility when needed Gwendal Grignou @ 2022-11-15 4:10 ` Gwendal Grignou 2022-11-16 18:23 ` Gwendal Grignou 0 siblings, 1 reply; 7+ messages in thread From: Gwendal Grignou @ 2022-11-15 4:10 UTC (permalink / raw) To: dtor, jwerner, bleung, enric.balletbo, groeck; +Cc: linux-kernel This patch is still needed to get the attribute to be present on most machines. Putting the attribute in the sensor hub device is more complicated, as that device is not as easily findable as `cros_ec`, present in `/sys/class`. Gwendal. On Wed, Aug 4, 2021 at 2:31 PM Gwendal Grignou <gwendal@chromium.org> wrote: > > When the sensorhub detects 2 accelerometers, kb_wake_angle attribute > in the chromeos EC class device needs to be visible. > When detected, the sensorhub requests the class device to > revisit the visibility of the kb_wake_angle attribute. > Expose the attribute group to alter to close a potiential race between > cros-ec-sensorhub and cros-ec-sysfs (that creates the attribute group > on behalf of the class driver). > > Signed-off-by: Gwendal Grignou <gwendal@chromium.org> > --- > drivers/platform/chrome/cros_ec_sensorhub.c | 6 +++++- > drivers/platform/chrome/cros_ec_sysfs.c | 5 +++-- > include/linux/platform_data/cros_ec_proto.h | 2 ++ > 3 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c b/drivers/platform/chrome/cros_ec_sensorhub.c > index 9c4af76a9956e..8f24ed90b229c 100644 > --- a/drivers/platform/chrome/cros_ec_sensorhub.c > +++ b/drivers/platform/chrome/cros_ec_sensorhub.c > @@ -106,8 +106,12 @@ static int cros_ec_sensorhub_register(struct device *dev, > sensor_type[sensorhub->resp->info.type]++; > } > > - if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) > + if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) { > ec->has_kb_wake_angle = true; > + if (ec->group && sysfs_update_group(&ec->class_dev.kobj, > + ec->group)) > + dev_warn(dev, "Unable to update cros-ec-sysfs"); > + } > > if (cros_ec_check_features(ec, > EC_FEATURE_REFINED_TABLET_MODE_HYSTERESIS)) { > diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c > index f07eabcf9494c..17fe657f3ffd5 100644 > --- a/drivers/platform/chrome/cros_ec_sysfs.c > +++ b/drivers/platform/chrome/cros_ec_sysfs.c > @@ -341,7 +341,8 @@ static int cros_ec_sysfs_probe(struct platform_device *pd) > struct device *dev = &pd->dev; > int ret; > > - ret = sysfs_create_group(&ec_dev->class_dev.kobj, &cros_ec_attr_group); > + ec_dev->group = &cros_ec_attr_group; > + ret = sysfs_create_group(&ec_dev->class_dev.kobj, ec_dev->group); > if (ret < 0) > dev_err(dev, "failed to create attributes. err=%d\n", ret); > > @@ -352,7 +353,7 @@ 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); > + sysfs_remove_group(&ec_dev->class_dev.kobj, ec_dev->group); > > return 0; > } > diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h > index 02599687770c5..c6dca260bbd5d 100644 > --- a/include/linux/platform_data/cros_ec_proto.h > +++ b/include/linux/platform_data/cros_ec_proto.h > @@ -191,6 +191,7 @@ struct cros_ec_platform { > /** > * struct cros_ec_dev - ChromeOS EC device entry point. > * @class_dev: Device structure used in sysfs. > + * @groups: sysfs attributes groups for this EC. > * @ec_dev: cros_ec_device structure to talk to the physical device. > * @dev: Pointer to the platform device. > * @debug_info: cros_ec_debugfs structure for debugging information. > @@ -200,6 +201,7 @@ struct cros_ec_platform { > */ > struct cros_ec_dev { > struct device class_dev; > + const struct attribute_group *group; > struct cros_ec_device *ec_dev; > struct device *dev; > struct cros_ec_debugfs *debug_info; > -- > 2.32.0.554.ge1b32706d8-goog > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/1] platform/chrome: Poke kb_wake_angle attribute visibility when needed 2022-11-15 4:10 ` Gwendal Grignou @ 2022-11-16 18:23 ` Gwendal Grignou 2022-11-18 2:01 ` Tzung-Bi Shih 0 siblings, 1 reply; 7+ messages in thread From: Gwendal Grignou @ 2022-11-16 18:23 UTC (permalink / raw) To: dtor, jwerner, bleung, enric.balletbo, groeck Cc: linux-kernel, chrome-platform [+chrome-platform@lists.linux.dev] On Mon, Nov 14, 2022 at 8:10 PM Gwendal Grignou <gwendal@chromium.org> wrote: > > This patch is still needed to get the attribute to be present on most machines. > Putting the attribute in the sensor hub device is more complicated, as > that device is not as easily findable as `cros_ec`, present in > `/sys/class`. > > Gwendal. > > On Wed, Aug 4, 2021 at 2:31 PM Gwendal Grignou <gwendal@chromium.org> wrote: > > > > When the sensorhub detects 2 accelerometers, kb_wake_angle attribute > > in the chromeos EC class device needs to be visible. > > When detected, the sensorhub requests the class device to > > revisit the visibility of the kb_wake_angle attribute. > > Expose the attribute group to alter to close a potiential race between > > cros-ec-sensorhub and cros-ec-sysfs (that creates the attribute group > > on behalf of the class driver). > > > > Signed-off-by: Gwendal Grignou <gwendal@chromium.org> > > --- > > drivers/platform/chrome/cros_ec_sensorhub.c | 6 +++++- > > drivers/platform/chrome/cros_ec_sysfs.c | 5 +++-- > > include/linux/platform_data/cros_ec_proto.h | 2 ++ > > 3 files changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c b/drivers/platform/chrome/cros_ec_sensorhub.c > > index 9c4af76a9956e..8f24ed90b229c 100644 > > --- a/drivers/platform/chrome/cros_ec_sensorhub.c > > +++ b/drivers/platform/chrome/cros_ec_sensorhub.c > > @@ -106,8 +106,12 @@ static int cros_ec_sensorhub_register(struct device *dev, > > sensor_type[sensorhub->resp->info.type]++; > > } > > > > - if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) > > + if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) { > > ec->has_kb_wake_angle = true; > > + if (ec->group && sysfs_update_group(&ec->class_dev.kobj, > > + ec->group)) > > + dev_warn(dev, "Unable to update cros-ec-sysfs"); > > + } > > > > if (cros_ec_check_features(ec, > > EC_FEATURE_REFINED_TABLET_MODE_HYSTERESIS)) { > > diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c > > index f07eabcf9494c..17fe657f3ffd5 100644 > > --- a/drivers/platform/chrome/cros_ec_sysfs.c > > +++ b/drivers/platform/chrome/cros_ec_sysfs.c > > @@ -341,7 +341,8 @@ static int cros_ec_sysfs_probe(struct platform_device *pd) > > struct device *dev = &pd->dev; > > int ret; > > > > - ret = sysfs_create_group(&ec_dev->class_dev.kobj, &cros_ec_attr_group); > > + ec_dev->group = &cros_ec_attr_group; > > + ret = sysfs_create_group(&ec_dev->class_dev.kobj, ec_dev->group); > > if (ret < 0) > > dev_err(dev, "failed to create attributes. err=%d\n", ret); > > > > @@ -352,7 +353,7 @@ 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); > > + sysfs_remove_group(&ec_dev->class_dev.kobj, ec_dev->group); > > > > return 0; > > } > > diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h > > index 02599687770c5..c6dca260bbd5d 100644 > > --- a/include/linux/platform_data/cros_ec_proto.h > > +++ b/include/linux/platform_data/cros_ec_proto.h > > @@ -191,6 +191,7 @@ struct cros_ec_platform { > > /** > > * struct cros_ec_dev - ChromeOS EC device entry point. > > * @class_dev: Device structure used in sysfs. > > + * @groups: sysfs attributes groups for this EC. > > * @ec_dev: cros_ec_device structure to talk to the physical device. > > * @dev: Pointer to the platform device. > > * @debug_info: cros_ec_debugfs structure for debugging information. > > @@ -200,6 +201,7 @@ struct cros_ec_platform { > > */ > > struct cros_ec_dev { > > struct device class_dev; > > + const struct attribute_group *group; > > struct cros_ec_device *ec_dev; > > struct device *dev; > > struct cros_ec_debugfs *debug_info; > > -- > > 2.32.0.554.ge1b32706d8-goog > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/1] platform/chrome: Poke kb_wake_angle attribute visibility when needed 2022-11-16 18:23 ` Gwendal Grignou @ 2022-11-18 2:01 ` Tzung-Bi Shih 2022-11-18 6:41 ` Gwendal Grignou 0 siblings, 1 reply; 7+ messages in thread From: Tzung-Bi Shih @ 2022-11-18 2:01 UTC (permalink / raw) To: Gwendal Grignou Cc: dtor, jwerner, bleung, enric.balletbo, groeck, linux-kernel, chrome-platform On Wed, Nov 16, 2022 at 10:23:38AM -0800, Gwendal Grignou wrote: > [+chrome-platform@lists.linux.dev] Please also Cc to the mailing list if the patch gets chance to have next version. > On Mon, Nov 14, 2022 at 8:10 PM Gwendal Grignou <gwendal@chromium.org> wrote: [...] > > > Expose the attribute group to alter to close a potiential race between > > > cros-ec-sensorhub and cros-ec-sysfs (that creates the attribute group > > > on behalf of the class driver). I failed to realize the potential race. Could you explain it a bit? > > > diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h > > > index 02599687770c5..c6dca260bbd5d 100644 > > > --- a/include/linux/platform_data/cros_ec_proto.h > > > +++ b/include/linux/platform_data/cros_ec_proto.h > > > @@ -191,6 +191,7 @@ struct cros_ec_platform { > > > /** > > > * struct cros_ec_dev - ChromeOS EC device entry point. > > > * @class_dev: Device structure used in sysfs. > > > + * @groups: sysfs attributes groups for this EC. The field name has extra "s". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/1] platform/chrome: Poke kb_wake_angle attribute visibility when needed 2022-11-18 2:01 ` Tzung-Bi Shih @ 2022-11-18 6:41 ` Gwendal Grignou 2022-11-18 6:48 ` Tzung-Bi Shih 0 siblings, 1 reply; 7+ messages in thread From: Gwendal Grignou @ 2022-11-18 6:41 UTC (permalink / raw) To: Tzung-Bi Shih Cc: dtor, jwerner, bleung, enric.balletbo, groeck, linux-kernel, chrome-platform On Thu, Nov 17, 2022 at 6:01 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote: > > On Wed, Nov 16, 2022 at 10:23:38AM -0800, Gwendal Grignou wrote: > > [+chrome-platform@lists.linux.dev] > > Please also Cc to the mailing list if the patch gets chance to have > next version. > > > On Mon, Nov 14, 2022 at 8:10 PM Gwendal Grignou <gwendal@chromium.org> wrote: > [...] > > > > Expose the attribute group to alter to close a potiential race between > > > > cros-ec-sensorhub and cros-ec-sysfs (that creates the attribute group > > > > on behalf of the class driver). > > I failed to realize the potential race. Could you explain it a bit? The decision to show or not an attribute is done at the attribute file creation time, once. If the module cros_ec_sysfs is loaded before cros_ec_sensorhub, the attribute kb_wake_angle will never be shown. If it is loaded after, and there are 2 accelerometers, the is_visible() function will return true and the attribute is shown. This patch ensures the attribute is_visible() is run again after the sensorhub driver is loaded. Gwendal. > > > > > diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h > > > > index 02599687770c5..c6dca260bbd5d 100644 > > > > --- a/include/linux/platform_data/cros_ec_proto.h > > > > +++ b/include/linux/platform_data/cros_ec_proto.h > > > > @@ -191,6 +191,7 @@ struct cros_ec_platform { > > > > /** > > > > * struct cros_ec_dev - ChromeOS EC device entry point. > > > > * @class_dev: Device structure used in sysfs. > > > > + * @groups: sysfs attributes groups for this EC. > > The field name has extra "s". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/1] platform/chrome: Poke kb_wake_angle attribute visibility when needed 2022-11-18 6:41 ` Gwendal Grignou @ 2022-11-18 6:48 ` Tzung-Bi Shih 0 siblings, 0 replies; 7+ messages in thread From: Tzung-Bi Shih @ 2022-11-18 6:48 UTC (permalink / raw) To: Gwendal Grignou Cc: dtor, jwerner, bleung, enric.balletbo, groeck, linux-kernel, chrome-platform On Thu, Nov 17, 2022 at 10:41:06PM -0800, Gwendal Grignou wrote: > On Thu, Nov 17, 2022 at 6:01 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote: > > > > On Wed, Nov 16, 2022 at 10:23:38AM -0800, Gwendal Grignou wrote: > > > [+chrome-platform@lists.linux.dev] > > > > Please also Cc to the mailing list if the patch gets chance to have > > next version. > > > > > On Mon, Nov 14, 2022 at 8:10 PM Gwendal Grignou <gwendal@chromium.org> wrote: > > [...] > > > > > Expose the attribute group to alter to close a potiential race between > > > > > cros-ec-sensorhub and cros-ec-sysfs (that creates the attribute group > > > > > on behalf of the class driver). > > > > I failed to realize the potential race. Could you explain it a bit? > The decision to show or not an attribute is done at the attribute file > creation time, once. If the module cros_ec_sysfs is loaded before > cros_ec_sensorhub, the attribute kb_wake_angle will never be shown. If > it is loaded after, and there are 2 accelerometers, the is_visible() > function will return true and the attribute is shown. > > This patch ensures the attribute is_visible() is run again after the > sensorhub driver is loaded. I see. Thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-18 6:48 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-04 21:31 [PATCH v3 0/1] platform/chrome: Update cros_ec sysfs attribute after sensors are found Gwendal Grignou 2021-08-04 21:31 ` [PATCH v3 1/1] platform/chrome: Poke kb_wake_angle attribute visibility when needed Gwendal Grignou 2022-11-15 4:10 ` Gwendal Grignou 2022-11-16 18:23 ` Gwendal Grignou 2022-11-18 2:01 ` Tzung-Bi Shih 2022-11-18 6:41 ` Gwendal Grignou 2022-11-18 6:48 ` Tzung-Bi Shih
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.