Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752523AbdHKF3x (ORCPT ); Fri, 11 Aug 2017 01:29:53 -0400 Received: from mail-pf0-f181.google.com ([209.85.192.181]:36405 "EHLO mail-pf0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751871AbdHKF3w (ORCPT ); Fri, 11 Aug 2017 01:29:52 -0400 Date: Thu, 10 Aug 2017 22:29:47 -0700 From: Benson Leung To: Thierry Escande Cc: Benson Leung , Lee Jones , linux-kernel@vger.kernel.org, bleung@google.com, gwendal@chromium.org Subject: Re: [PATCH 7/8] platform/chrome: cros_ec: Add sysfs entry to set keyboard wake lid angle Message-ID: <20170811052947.GA155834@decatoncale.mtv.corp.google.com> References: <1502403410-5233-1-git-send-email-thierry.escande@collabora.com> <1502403410-5233-8-git-send-email-thierry.escande@collabora.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="3MwIy2ne0vdjdPXF" Content-Disposition: inline In-Reply-To: <1502403410-5233-8-git-send-email-thierry.escande@collabora.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9011 Lines: 269 --3MwIy2ne0vdjdPXF Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Thierry, On Fri, Aug 11, 2017 at 12:16:49AM +0200, Thierry Escande wrote: > From: Gwendal Grignou >=20 > This adds a sysfs attribute (/sys/class/chromeos/cros_ec/kb_wake_angle) > used to set and get the keyboard wake lid angle. This attribute is > present only if 2 accelerometers are controlled by the EC. >=20 > This patch also moves the cros_ec features check before the device is > added so the features map obtained from the EC is ready on time. >=20 > Signed-off-by: Gwendal Grignou > Signed-off-by: Thierry Escande > --- > drivers/platform/chrome/cros_ec_dev.c | 32 ++++++------- > drivers/platform/chrome/cros_ec_sysfs.c | 80 +++++++++++++++++++++++++++= ++++++ > include/linux/mfd/cros_ec.h | 1 + > 3 files changed, 94 insertions(+), 19 deletions(-) >=20 > diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chr= ome/cros_ec_dev.c > index 225d8e9..a7d711c 100644 > --- a/drivers/platform/chrome/cros_ec_dev.c > +++ b/drivers/platform/chrome/cros_ec_dev.c > @@ -304,8 +304,8 @@ static void cros_ec_sensors_register(struct cros_ec_d= ev *ec) > =20 > resp =3D (struct ec_response_motion_sense *)msg->data; > sensor_num =3D resp->dump.sensor_count; > - /* Allocate 2 extra sensors in case lid angle or FIFO are needed */ > - sensor_cells =3D kzalloc(sizeof(struct mfd_cell) * (sensor_num + 2), > + /* Allocate one extra sensor for MOTION_SENSE_FIFO if needed */ > + sensor_cells =3D kzalloc(sizeof(struct mfd_cell) * (sensor_num + 1), > GFP_KERNEL); > if (sensor_cells =3D=3D NULL) > goto error; > @@ -361,16 +361,8 @@ static void cros_ec_sensors_register(struct cros_ec_= dev *ec) > sensor_type[resp->info.type]++; > id++; > } > - if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >=3D 2) { > - sensor_platforms[id].sensor_num =3D sensor_num; > - > - sensor_cells[id].name =3D "cros-ec-angle"; > - sensor_cells[id].id =3D 0; > - sensor_cells[id].platform_data =3D &sensor_platforms[id]; > - sensor_cells[id].pdata_size =3D > - sizeof(struct cros_ec_sensor_platform); > - id++; > - } > + if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >=3D 2) > + ec->has_kb_wake_angle =3D true; > if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) { > sensor_cells[id].name =3D "cros-ec-ring"; > id++; > @@ -423,6 +415,15 @@ static int ec_device_probe(struct platform_device *p= dev) > goto failed; > } > =20 > + /* check whether this EC is a sensor hub. */ > + if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE)) { > + pr_err("has EC_FEATURE_MOTION_SENSE\n"); > + cros_ec_sensors_register(ec); > + } > + > + /* Take control of the lightbar from the EC. */ > + lb_manual_suspend_ctrl(ec, 1); > + > retval =3D cdev_device_add(&ec->cdev, &ec->class_dev); > if (retval) { > dev_err(dev, "cdev_device_add failed =3D> %d\n", retval); > @@ -432,13 +433,6 @@ static int ec_device_probe(struct platform_device *p= dev) > if (cros_ec_debugfs_init(ec)) > dev_warn(dev, "failed to create debugfs directory\n"); > =20 > - /* check whether this EC is a sensor hub. */ > - if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE)) > - cros_ec_sensors_register(ec); > - > - /* Take control of the lightbar from the EC. */ > - lb_manual_suspend_ctrl(ec, 1); > - > return 0; > =20 > failed: > diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/c= hrome/cros_ec_sysfs.c > index f3baf99..60ff122 100644 > --- a/drivers/platform/chrome/cros_ec_sysfs.c > +++ b/drivers/platform/chrome/cros_ec_sysfs.c > @@ -278,20 +278,100 @@ static ssize_t show_ec_flashinfo(struct device *de= v, > return ret; > } > =20 > +/* Keyboard wake angle control */ > + > +static ssize_t show_kb_wake_angle(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct ec_response_motion_sense *resp; > + struct ec_params_motion_sense *param; > + struct cros_ec_command *msg; > + int ret; > + struct cros_ec_dev *ec =3D container_of( > + dev, struct cros_ec_dev, class_dev); > + > + msg =3D kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL); > + if (!msg) > + return -ENOMEM; > + > + param =3D (struct ec_params_motion_sense *)msg->data; > + msg->command =3D EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset; > + msg->version =3D 2; > + param->cmd =3D MOTIONSENSE_CMD_KB_WAKE_ANGLE; > + param->kb_wake_angle.data =3D EC_MOTION_SENSE_NO_VALUE; > + msg->outsize =3D sizeof(*param); > + msg->insize =3D sizeof(*resp); > + ret =3D cros_ec_cmd_xfer_status(ec->ec_dev, msg); Original CHROMIUM commit uses cros_ec_cmd_xfer here. https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/chan= ges/15/456415/5/drivers/platform/chrome/cros_ec_sysfs.c#303 > + if (ret < 0) > + return ret; Original commit also has this check: if (msg->result !=3D EC_RES_SUCCESS) return scnprintf(buf, PAGE_SIZE, "ERROR: EC returned %d\n", msg->result); Now, in the CHROMIUM commits, Gwendal removes this in the next patch, which is "[PATCH 8/8] platform/chrome: cros_ec: sysfs: Modify error handling" in = your series, but can we try to keep these as close to the originals as possible? > + resp =3D (struct ec_response_motion_sense *)msg->data; > + return scnprintf(buf, PAGE_SIZE, "%d\n", > + resp->kb_wake_angle.ret); > +} > + > +static ssize_t store_kb_wake_angle(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct ec_params_motion_sense *param; > + struct cros_ec_command *msg; > + int ret; > + struct cros_ec_dev *ec =3D container_of( > + dev, struct cros_ec_dev, class_dev); > + uint16_t angle; > + > + ret =3D kstrtou16(buf, 0, &angle); > + if (ret) > + return ret; > + > + msg =3D kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL); > + if (!msg) > + return -ENOMEM; > + > + param =3D (struct ec_params_motion_sense *)msg->data; > + msg->command =3D EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset; > + msg->version =3D 2; > + param->cmd =3D MOTIONSENSE_CMD_KB_WAKE_ANGLE; > + param->kb_wake_angle.data =3D angle; > + msg->outsize =3D sizeof(*param); > + msg->insize =3D sizeof(struct ec_response_motion_sense); > + ret =3D cros_ec_cmd_xfer_status(ec->ec_dev, msg); > + if (ret < 0) > + return ret; > + return count; > +} > + > /* Module initialization */ > =20 > static DEVICE_ATTR(reboot, S_IWUSR | S_IRUGO, show_ec_reboot, store_ec_r= eboot); > static DEVICE_ATTR(version, S_IRUGO, show_ec_version, NULL); > static DEVICE_ATTR(flashinfo, S_IRUGO, show_ec_flashinfo, NULL); > +static DEVICE_ATTR(kb_wake_angle, S_IWUSR | S_IRUGO, show_kb_wake_angle, > + store_kb_wake_angle); > =20 > static struct attribute *__ec_attrs[] =3D { > + &dev_attr_kb_wake_angle.attr, > &dev_attr_reboot.attr, > &dev_attr_version.attr, > &dev_attr_flashinfo.attr, > NULL, > }; > =20 > +static umode_t cros_ec_ctrl_visible(struct kobject *kobj, > + struct attribute *a, int n) > +{ > + struct device *dev =3D container_of(kobj, struct device, kobj); > + struct cros_ec_dev *ec =3D container_of(dev, struct cros_ec_dev, > + class_dev); > + > + if (a =3D=3D &dev_attr_kb_wake_angle.attr && !ec->has_kb_wake_angle) > + return 0; > + > + return a->mode; > +} > + > struct attribute_group cros_ec_attr_group =3D { > .attrs =3D __ec_attrs, > + .is_visible =3D cros_ec_ctrl_visible, > }; > =20 > diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h > index 4e887ba..350b8a4 100644 > --- a/include/linux/mfd/cros_ec.h > +++ b/include/linux/mfd/cros_ec.h > @@ -191,6 +191,7 @@ struct cros_ec_dev { > struct cros_ec_device *ec_dev; > struct device *dev; > struct cros_ec_debugfs *debug_info; > + bool has_kb_wake_angle; > u16 cmd_offset; > u32 features[2]; > }; > --=20 > 2.7.4 >=20 --=20 Benson Leung Staff Software Engineer Chrome OS Kernel Google Inc. bleung@google.com Chromium OS Project bleung@chromium.org --3MwIy2ne0vdjdPXF Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJZjUDLAAoJEB8J9XsKL+ZYnKsQAJEZkXINvMv1i2nyKt4LXYZ7 fzmO3kFlnR9KUzfJKw3FbNlOKJJ1mmebQEjtRcR9M2hc5hkqD52XfFUk9nT6pFu7 4IzdkTu0WG8lKqpfYxrTxG9Vx/Lej3e4ILTwaYCDRXzYIkJvdCie7wi0CR8XcZkf 6w48ktY/MXqa1VV8mYwhIjIxyVyVraxLftiHX/0nV6V0VIfTz2epDkOeDL/u/Oa7 ufiLJH4NOsWRW3+UoolaQzZgQoZq2li1x6IOtzWYBuCCzBctloPfil3fRAx/qZMN dhWE/QoaUM+ixVOOT0gDYAeYliXtoav+558VphyCp1jlD51PhJghs8xjXDcBYE4e eLgNs+H6xDNagDlu93/tUu5nCSb1BcyVo+KOqB5kpe8Ri1tGSzVfFGFMks5mFgEP vT9MWqh9e6ELdJY1zLLMwntbbc0riBcmVAnJ3P7ANUo+/a7kO34zqnb/O96HUpvK 9tyjsVjgkr1bs3iceVOikrDRMXUreSyCyM9scJ+aTzYLwFtxQBFA/v0BAxlcM1xk htpbrMx/ahb30cIZm4MuzwpRIIbrSb6lDVUvlpB+4rUGOO/1A3KpgcFzmWtZVx6/ J41gKSBMSofFxV1geNqLYs0r08mgpc9vqSpn0T7sw5r7/QawPu4xcQBp6Mi3pr7C QLvlD/jjLZqE/9Pq3yQo =2l/g -----END PGP SIGNATURE----- --3MwIy2ne0vdjdPXF--