Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20210804213139.4139492-1-gwendal@chromium.org> <20210804213139.4139492-2-gwendal@chromium.org>
In-Reply-To: <20210804213139.4139492-2-gwendal@chromium.org>
From:   Gwendal Grignou <gwendal@chromium.org>
Date:   Mon, 14 Nov 2022 20:10:29 -0800
Message-ID: <CAPUE2usQX_uQewpCYOs3SRUo7vDDMfg=n7idNvCgThDHMqTf7w@mail.gmail.com>
Subject: Re: [PATCH v3 1/1] platform/chrome: Poke kb_wake_angle attribute
 visibility when needed
To:     dtor@chromium.org, jwerner@chromium.org, bleung@chromium.org,
        enric.balletbo@collabora.com, groeck@chromium.org
Cc:     linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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
>