[PATCH 0/3] platform/chrome: cros_ec debugfs and sysfs updates

[PATCH 0/3] platform/chrome: cros_ec debugfs and sysfs updates

[Enric Balletbo i Serra]

Hi,

This patchset is another recopilation of some patches that were send
before but never merged.

First patch was already sent [1] but did not receive feedback so I am
resending it. Needs to go throught platform/chrome tree and can be
picked without dependencies.

The second patch is new, like the first needs to go throught
platform/chrome tree and can be picked without dependecies.

Finally, the third patch was already sent [2] but now needed to be
reworked due the split of the cros_ec_devs in 2 parts [3]. This patch
touches platform/chrome and mfd so we will probably need an immutable
branch when is ok.

Best regards,
  Enric

[1] https://patchwork.kernel.org/patch/9894655/
[2] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1488891.html
[3] https://lkml.org/lkml/2017/11/20/408
 
Gwendal Grignou (2):
  platform/chrome: cros_ec_sysfs: Modify error handling
  platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake
    lid angle

Shawn Nematbakhsh (1):
  platform/chrome: cros_ec_debugfs: Add PD port info to debugfs

 drivers/mfd/cros_ec_dev.c                 |  19 +++---
 drivers/platform/chrome/cros_ec_debugfs.c |  86 ++++++++++++++++++++++--
 drivers/platform/chrome/cros_ec_sysfs.c   | 108 ++++++++++++++++++++++++------
 include/linux/mfd/cros_ec.h               |   1 +
 4 files changed, 180 insertions(+), 34 deletions(-)

-- 
2.16.1

[PATCH 1/3] platform/chrome: cros_ec_sysfs: Modify error handling

[Enric Balletbo i Serra]

From: Gwendal Grignou <gwendal@chromium.org>

When accessing a sysfs attribute, if the EC command fails, -EPROTO is
now returned instead of an error message as it is unlikely an app is
parsing the error message to do something meaningful.
Also, this patch makes use of cros_ec_cmd_xfer_status() instead of
cros_ec_cmd_xfer() so an error message is printed in the syslog.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/platform/chrome/cros_ec_sysfs.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index da0a719d32f7..c03621e523a3 100644
--- a/drivers/platform/chrome/cros_ec_sysfs.c
+++ b/drivers/platform/chrome/cros_ec_sysfs.c
@@ -114,15 +114,9 @@ static ssize_t store_ec_reboot(struct device *dev,
 	msg->command = EC_CMD_REBOOT_EC + ec->cmd_offset;
 	msg->outsize = sizeof(*param);
 	msg->insize = 0;
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
-	if (ret < 0) {
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+	if (ret < 0)
 		count = ret;
-		goto exit;
-	}
-	if (msg->result != EC_RES_SUCCESS) {
-		dev_dbg(ec->dev, "EC result %d\n", msg->result);
-		count = -EINVAL;
-	}
 exit:
 	kfree(msg);
 	return count;
@@ -150,17 +144,11 @@ static ssize_t show_ec_version(struct device *dev,
 	msg->command = EC_CMD_GET_VERSION + ec->cmd_offset;
 	msg->insize = sizeof(*r_ver);
 	msg->outsize = 0;
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
 	if (ret < 0) {
 		count = ret;
 		goto exit;
 	}
-	if (msg->result != EC_RES_SUCCESS) {
-		count = scnprintf(buf, PAGE_SIZE,
-				  "ERROR: EC returned %d\n", msg->result);
-		goto exit;
-	}
-
 	r_ver = (struct ec_response_get_version *)msg->data;
 	/* Strings should be null-terminated, but let's be sure. */
 	r_ver->version_string_ro[sizeof(r_ver->version_string_ro) - 1] = '\0';
@@ -255,14 +243,9 @@ static ssize_t show_ec_flashinfo(struct device *dev,
 	msg->command = EC_CMD_FLASH_INFO + ec->cmd_offset;
 	msg->insize = sizeof(*resp);
 	msg->outsize = 0;
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
 	if (ret < 0)
 		goto exit;
-	if (msg->result != EC_RES_SUCCESS) {
-		ret = scnprintf(buf, PAGE_SIZE,
-				"ERROR: EC returned %d\n", msg->result);
-		goto exit;
-	}
 
 	resp = (struct ec_response_flash_info *)msg->data;
 
-- 
2.16.1

[PATCH 2/3] platform/chrome: cros_ec_debugfs: Add PD port info to debugfs

[Enric Balletbo i Serra]

From: Shawn Nematbakhsh <shawnn@chromium.org>

Add info useful for debugging USB-PD port state.

Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/platform/chrome/cros_ec_debugfs.c | 86 +++++++++++++++++++++++++++++--
 1 file changed, 82 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 0328856ec3a2..1d7e28c633f2 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -35,7 +35,8 @@
 
 #define CIRC_ADD(idx, size, value)	(((idx) + (value)) & ((size) - 1))
 
-/* struct cros_ec_debugfs - ChromeOS EC debugging information
+/*
+ * struct cros_ec_debugfs - ChromeOS EC debugging information
  *
  * @ec: EC device this debugfs information belongs to
  * @dir: dentry for debugfs files
@@ -170,7 +171,8 @@ static ssize_t cros_ec_console_log_read(struct file *file, char __user *buf,
 		mutex_lock(&debug_info->log_mutex);
 	}
 
-	/* Only copy until the end of the circular buffer, and let userspace
+	/*
+	 * Only copy until the end of the circular buffer, and let userspace
 	 * retry to get the rest of the data.
 	 */
 	ret = min_t(size_t, CIRC_CNT_TO_END(cb->head, cb->tail, LOG_SIZE),
@@ -211,6 +213,61 @@ static int cros_ec_console_log_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
+static ssize_t cros_ec_pdinfo_read(struct file *file,
+				   char __user *user_buf,
+				   size_t count,
+				   loff_t *ppos)
+{
+	char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
+	struct cros_ec_debugfs *debug_info = file->private_data;
+	struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
+
+	struct {
+		struct cros_ec_command msg;
+		union {
+			struct ec_response_usb_pd_control_v1 resp;
+			struct ec_params_usb_pd_control params;
+		};
+	} __packed ec_buf;
+
+	struct cros_ec_command *msg;
+	struct ec_response_usb_pd_control_v1 *resp;
+	struct ec_params_usb_pd_control *params;
+
+	int i;
+
+	msg = &ec_buf.msg;
+	params = (struct ec_params_usb_pd_control *)msg->data;
+	resp = (struct ec_response_usb_pd_control_v1 *)msg->data;
+
+	msg->command = EC_CMD_USB_PD_CONTROL;
+	msg->version = 1;
+	msg->insize = sizeof(*resp);
+	msg->outsize = sizeof(*params);
+
+	/*
+	 * Read status from all PD ports until failure, typically caused
+	 * by attempting to read status on a port that doesn't exist.
+	 */
+	for (i = 0; i < EC_USB_PD_MAX_PORTS; ++i) {
+		params->port = i;
+		params->role = 0;
+		params->mux = 0;
+		params->swap = 0;
+
+		if (cros_ec_cmd_xfer_status(ec_dev, msg) < 0)
+			break;
+
+		p += scnprintf(p, sizeof(read_buf) + read_buf - p,
+			"p%d: %s en:%.2x role:%.2x pol:%.2x\n",
+			i, resp->state, resp->enabled, resp->role,
+			resp->polarity);
+	}
+
+	return simple_read_from_buffer(user_buf, count, ppos,
+				       read_buf, p - read_buf);
+}
+
 const struct file_operations cros_ec_console_log_fops = {
 	.owner = THIS_MODULE,
 	.open = cros_ec_console_log_open,
@@ -220,6 +277,13 @@ const struct file_operations cros_ec_console_log_fops = {
 	.release = cros_ec_console_log_release,
 };
 
+const struct file_operations cros_ec_pdinfo_fops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = cros_ec_pdinfo_read,
+	.llseek = default_llseek,
+};
+
 static int ec_read_version_supported(struct cros_ec_dev *ec)
 {
 	struct ec_params_get_cmd_versions_v1 *params;
@@ -288,7 +352,7 @@ static int cros_ec_create_console_log(struct cros_ec_debugfs *debug_info)
 	init_waitqueue_head(&debug_info->log_wq);
 
 	if (!debugfs_create_file("console_log",
-				 S_IFREG | S_IRUGO,
+				 S_IFREG | 0444,
 				 debug_info->dir,
 				 debug_info,
 				 &cros_ec_console_log_fops))
@@ -341,7 +405,7 @@ static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
 	debug_info->panicinfo_blob.size = ret;
 
 	if (!debugfs_create_blob("panicinfo",
-				 S_IFREG | S_IRUGO,
+				 S_IFREG | 0444,
 				 debug_info->dir,
 				 &debug_info->panicinfo_blob)) {
 		ret = -ENOMEM;
@@ -355,6 +419,16 @@ static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
 	return ret;
 }
 
+static int cros_ec_create_pdinfo(struct cros_ec_debugfs *debug_info)
+{
+	if (!debugfs_create_file("pdinfo", S_IFREG | 0444,
+				 debug_info->dir, debug_info,
+				 &cros_ec_pdinfo_fops))
+		return -ENOMEM;
+
+	return 0;
+}
+
 int cros_ec_debugfs_init(struct cros_ec_dev *ec)
 {
 	struct cros_ec_platform *ec_platform = dev_get_platdata(ec->dev);
@@ -379,6 +453,10 @@ int cros_ec_debugfs_init(struct cros_ec_dev *ec)
 	if (ret)
 		goto remove_debugfs;
 
+	ret = cros_ec_create_pdinfo(debug_info);
+	if (ret)
+		goto remove_debugfs;
+
 	ec->debug_info = debug_info;
 
 	return 0;
-- 
2.16.1

Re: [PATCH 2/3] platform/chrome: cros_ec_debugfs: Add PD port info to debugfs

[Andy Shevchenko]

On Wed, Feb 21, 2018 at 4:49 PM, Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
> From: Shawn Nematbakhsh <shawnn@chromium.org>
>
> Add info useful for debugging USB-PD port state.

This patch mixes up at least three things.
Split it accordingly or drop unrelated changes.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] platform/chrome: cros_ec debugfs and sysfs updates

[Andy Shevchenko]

On Wed, Feb 21, 2018 at 4:49 PM, Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
> Hi,
>
> This patchset is another recopilation of some patches that were send
> before but never merged.
>
> First patch was already sent [1] but did not receive feedback so I am
> resending it. Needs to go throught platform/chrome tree and can be
> picked without dependencies.
>
> The second patch is new, like the first needs to go throught
> platform/chrome tree and can be picked without dependecies.
>
> Finally, the third patch was already sent [2] but now needed to be
> reworked due the split of the cros_ec_devs in 2 parts [3]. This patch
> touches platform/chrome and mfd so we will probably need an immutable
> branch when is ok.
>

New patches should have a tag, but there is, I hope, no need to resend
and maintainers would add them as I posted.

The rest looks good to me, FWIW,

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Best regards,
>   Enric
>
> [1] https://patchwork.kernel.org/patch/9894655/
> [2] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1488891.html
> [3] https://lkml.org/lkml/2017/11/20/408
>
> Gwendal Grignou (2):
>   platform/chrome: cros_ec_sysfs: Modify error handling
>   platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake
>     lid angle
>
> Shawn Nematbakhsh (1):
>   platform/chrome: cros_ec_debugfs: Add PD port info to debugfs
>
>  drivers/mfd/cros_ec_dev.c                 |  19 +++---
>  drivers/platform/chrome/cros_ec_debugfs.c |  86 ++++++++++++++++++++++--
>  drivers/platform/chrome/cros_ec_sysfs.c   | 108 ++++++++++++++++++++++++------
>  include/linux/mfd/cros_ec.h               |   1 +
>  4 files changed, 180 insertions(+), 34 deletions(-)
>
> --
> 2.16.1
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3] platform/chrome: cros_ec_debugfs: Add PD port info to debugfs

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3542 bytes --]

Hi Shawn,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.16-rc2 next-20180221]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Enric-Balletbo-i-Serra/platform-chrome-cros_ec-debugfs-and-sysfs-updates/20180222-074242
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: i386-randconfig-s0-201807 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/platform/chrome/cros_ec_debugfs.c: In function 'cros_ec_pdinfo_read':
>> drivers/platform/chrome/cros_ec_debugfs.c:221:16: error: 'EC_USB_PD_MAX_PORTS' undeclared (first use in this function)
     char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
                   ^~~~~~~~~~~~~~~~~~~
   drivers/platform/chrome/cros_ec_debugfs.c:221:16: note: each undeclared identifier is reported only once for each function it appears in
   drivers/platform/chrome/cros_ec_debugfs.c:221:7: warning: unused variable 'read_buf' [-Wunused-variable]
     char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
          ^~~~~~~~
   drivers/platform/chrome/cros_ec_debugfs.c:269:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^

vim +/EC_USB_PD_MAX_PORTS +221 drivers/platform/chrome/cros_ec_debugfs.c

   215	
   216	static ssize_t cros_ec_pdinfo_read(struct file *file,
   217					   char __user *user_buf,
   218					   size_t count,
   219					   loff_t *ppos)
   220	{
 > 221		char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
   222		struct cros_ec_debugfs *debug_info = file->private_data;
   223		struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
   224	
   225		struct {
   226			struct cros_ec_command msg;
   227			union {
   228				struct ec_response_usb_pd_control_v1 resp;
   229				struct ec_params_usb_pd_control params;
   230			};
   231		} __packed ec_buf;
   232	
   233		struct cros_ec_command *msg;
   234		struct ec_response_usb_pd_control_v1 *resp;
   235		struct ec_params_usb_pd_control *params;
   236	
   237		int i;
   238	
   239		msg = &ec_buf.msg;
   240		params = (struct ec_params_usb_pd_control *)msg->data;
   241		resp = (struct ec_response_usb_pd_control_v1 *)msg->data;
   242	
   243		msg->command = EC_CMD_USB_PD_CONTROL;
   244		msg->version = 1;
   245		msg->insize = sizeof(*resp);
   246		msg->outsize = sizeof(*params);
   247	
   248		/*
   249		 * Read status from all PD ports until failure, typically caused
   250		 * by attempting to read status on a port that doesn't exist.
   251		 */
   252		for (i = 0; i < EC_USB_PD_MAX_PORTS; ++i) {
   253			params->port = i;
   254			params->role = 0;
   255			params->mux = 0;
   256			params->swap = 0;
   257	
   258			if (cros_ec_cmd_xfer_status(ec_dev, msg) < 0)
   259				break;
   260	
   261			p += scnprintf(p, sizeof(read_buf) + read_buf - p,
   262				"p%d: %s en:%.2x role:%.2x pol:%.2x\n",
   263				i, resp->state, resp->enabled, resp->role,
   264				resp->polarity);
   265		}
   266	
   267		return simple_read_from_buffer(user_buf, count, ppos,
   268					       read_buf, p - read_buf);
   269	}
   270	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36975 bytes --]

Re: [PATCH 2/3] platform/chrome: cros_ec_debugfs: Add PD port info to debugfs

[Enric Balletbo Serra]

Hi Lee,

2018-02-22 1:25 GMT+01:00 kbuild test robot <lkp@intel.com>:
> Hi Shawn,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on ljones-mfd/for-mfd-next]
> [also build test ERROR on v4.16-rc2 next-20180221]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Enric-Balletbo-i-Serra/platform-chrome-cros_ec-debugfs-and-sysfs-updates/20180222-074242
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
> config: i386-randconfig-s0-201807 (attached as .config)
> compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386
>
> All errors (new ones prefixed by >>):
>
>    drivers/platform/chrome/cros_ec_debugfs.c: In function 'cros_ec_pdinfo_read':
>>> drivers/platform/chrome/cros_ec_debugfs.c:221:16: error: 'EC_USB_PD_MAX_PORTS' undeclared (first use in this function)
>      char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
>                    ^~~~~~~~~~~~~~~~~~~
>    drivers/platform/chrome/cros_ec_debugfs.c:221:16: note: each undeclared identifier is reported only once for each function it appears in
>    drivers/platform/chrome/cros_ec_debugfs.c:221:7: warning: unused variable 'read_buf' [-Wunused-variable]
>      char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
>           ^~~~~~~~
>    drivers/platform/chrome/cros_ec_debugfs.c:269:1: warning: control reaches end of non-void function [-Wreturn-type]
>     }
>     ^
>
> vim +/EC_USB_PD_MAX_PORTS +221 drivers/platform/chrome/cros_ec_debugfs.c
>
>    215
>    216  static ssize_t cros_ec_pdinfo_read(struct file *file,
>    217                                     char __user *user_buf,
>    218                                     size_t count,
>    219                                     loff_t *ppos)
>    220  {
>  > 221          char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
>    222          struct cros_ec_debugfs *debug_info = file->private_data;
>    223          struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
>    224
>    225          struct {
>    226                  struct cros_ec_command msg;
>    227                  union {
>    228                          struct ec_response_usb_pd_control_v1 resp;
>    229                          struct ec_params_usb_pd_control params;
>    230                  };
>    231          } __packed ec_buf;
>    232
>    233          struct cros_ec_command *msg;
>    234          struct ec_response_usb_pd_control_v1 *resp;
>    235          struct ec_params_usb_pd_control *params;
>    236
>    237          int i;
>    238
>    239          msg = &ec_buf.msg;
>    240          params = (struct ec_params_usb_pd_control *)msg->data;
>    241          resp = (struct ec_response_usb_pd_control_v1 *)msg->data;
>    242
>    243          msg->command = EC_CMD_USB_PD_CONTROL;
>    244          msg->version = 1;
>    245          msg->insize = sizeof(*resp);
>    246          msg->outsize = sizeof(*params);
>    247
>    248          /*
>    249           * Read status from all PD ports until failure, typically caused
>    250           * by attempting to read status on a port that doesn't exist.
>    251           */
>    252          for (i = 0; i < EC_USB_PD_MAX_PORTS; ++i) {
>    253                  params->port = i;
>    254                  params->role = 0;
>    255                  params->mux = 0;
>    256                  params->swap = 0;
>    257
>    258                  if (cros_ec_cmd_xfer_status(ec_dev, msg) < 0)
>    259                          break;
>    260
>    261                  p += scnprintf(p, sizeof(read_buf) + read_buf - p,
>    262                          "p%d: %s en:%.2x role:%.2x pol:%.2x\n",
>    263                          i, resp->state, resp->enabled, resp->role,
>    264                          resp->polarity);
>    265          }
>    266
>    267          return simple_read_from_buffer(user_buf, count, ppos,
>    268                                         read_buf, p - read_buf);
>    269  }
>    270
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Looks like I missed to add a dependency on this patch [1] here. I
think that the series that contains the dependency is farther to be
accepted than these. So maybe what we could do is include the missing
dependency here. What would you prefer, I send a new version with the
missing patch included or you just pick the patch from the other
series? The patch was already acked by you.

Thanks,
  Enric

[1] https://lkml.org/lkml/2018/1/23/217