Skip to contentSkip to conversation

mfd: cros-ec: Add cros_ec_readmem() helpers for I2C/SPI based ECs

PATCH In review Moritz Fischer 3 participants 4 messages May 8, 2017 – May 9, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mfd: cros-ec: Add cros_ec_readmem() helpers for I2C/SPI based ECs

Add cros_ec_readmem() based helpers for I2C/SPI based ECs. Unlike the LPC based ECs the I2C/SPI based ones cannot just directly read the mapped region.

---

Hi Lee,

I'm currently reworking my patchset for the cros-ec hwmon,
and as part of the feedback there Guenter had pointed out that I should use the cmd_readmem() callbacks. Since they don't exist for the I2C / SPI ECs I'll send this patch ahead, while working on my hwmon patchset.

Thanks,

Moritz

---
 drivers/mfd/cros_ec.c     | 39 +++++++++++++++++++++++++++++++++++++++
 drivers/mfd/cros_ec.h     | 29 +++++++++++++++++++++++++++++
 drivers/mfd/cros_ec_i2c.c |  3 +++
 drivers/mfd/cros_ec_spi.c |  2 ++
 4 files changed, 73 insertions(+)
 create mode 100644 drivers/mfd/cros_ec.h
drivers/mfd/cros_ec.c +39 -0
index d4a407e..2121cd5 100644
@@ -2,6 +2,7 @@
  * ChromeOS EC multi-function device
  *
  * Copyright (C) 2012 Google, Inc
+ * Copyright (C) 2017 National Instruments Corp
  *
  * This software is licensed under the terms of the GNU General Public
  * License version 2, as published by the Free Software Foundation, and
@@ -265,5 +266,43 @@ EXPORT_SYMBOL(cros_ec_resume);
 
 #endif
 
+int cros_ec_readmem(struct cros_ec_device *ec, unsigned int offset,
+			   unsigned int bytes, void *dest)
+{
+	int ret;
+	struct ec_params_read_memmap *params;
+	struct cros_ec_command *msg;
+
+	if (offset >= EC_MEMMAP_SIZE - bytes)
+		return -EINVAL;
+
+	msg = kzalloc(sizeof(*msg) + max(sizeof(*params), bytes), GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	msg->version = 0;
+	msg->command = EC_CMD_READ_MEMMAP;
+	msg->insize = bytes;
+	msg->outsize = sizeof(*params);
+
+	params = (struct ec_params_read_memmap *)msg->data;
+	params->offset = offset;
+	params->size = bytes;
+
+	ret = cros_ec_cmd_xfer_status(ec, msg);
+	if (ret < 0) {
+		dev_warn(ec->dev, "cannot read mapped reg: %d/%d\n",
+			 ret, msg->result);
+		goto out_free;
+	}
+
+	memcpy(dest, msg->data, bytes);
+
+out_free:
+	kfree(msg);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cros_ec_readmem);
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("ChromeOS EC core driver");
drivers/mfd/cros_ec.h new file +29 -0
new file mode 100644
index 0000000..09e83e6
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2017 National Instruments Corp
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef MFD_CROS_EC_H
+#define MFD_CROS_EC_H
+
+/**
+ * cros_ec_readmem - Read mapped memory in the ChromeOS EC
+ *
+ * @ec: Device to read from
+ * @offset: Offset to read within the mapped region
+ * @bytes: number of bytes to read
+ * @data: Return data
+ * @return: 0 if Ok, -ve on error
+ */
+int cros_ec_readmem(struct cros_ec_device *ec, unsigned int offset,
+		    unsigned int bytes, void *dest);
+
+#endif /* MFD_CROS_EC_H */
drivers/mfd/cros_ec_i2c.c +3 -0
index 9f70de1..012f949 100644
@@ -23,6 +23,8 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
+#include "cros_ec.h"
+
 /**
  * Request format for protocol v3
  * byte 0	0xda (EC_COMMAND_PROTOCOL_3)
@@ -302,6 +304,7 @@ static int cros_ec_i2c_probe(struct i2c_client *client,
 	ec_dev->irq = client->irq;
 	ec_dev->cmd_xfer = cros_ec_cmd_xfer_i2c;
 	ec_dev->pkt_xfer = cros_ec_pkt_xfer_i2c;
+	ec_dev->cmd_readmem = cros_ec_readmem;
 	ec_dev->phys_name = client->adapter->name;
 	ec_dev->din_size = sizeof(struct ec_host_response_i2c) +
 			   sizeof(struct ec_response_get_protocol_info);
drivers/mfd/cros_ec_spi.c +2 -0
index c971407..8f90bd6 100644
@@ -23,6 +23,7 @@
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
 
+#include "cros_ec.h"
 
 /* The header byte, which follows the preamble */
 #define EC_MSG_HEADER			0xec
@@ -661,6 +662,7 @@ static int cros_ec_spi_probe(struct spi_device *spi)
 	ec_dev->irq = spi->irq;
 	ec_dev->cmd_xfer = cros_ec_cmd_xfer_spi;
 	ec_dev->pkt_xfer = cros_ec_pkt_xfer_spi;
+	ec_dev->cmd_readmem = cros_ec_readmem;
 	ec_dev->phys_name = dev_name(&ec_spi->spi->dev);
 	ec_dev->din_size = EC_MSG_PREAMBLE_COUNT +
 			   sizeof(struct ec_host_response) +
Signature · 1 line
2.7.4
#1 lore raw
reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross make.cross ARCH=tile allmodconfig make.cross ARCH=tile

Hi Moritz,

[auto build test WARNING on ljones-mfd/for-mfd-next]
[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/Moritz-Fischer/mfd-cros-ec-Add-cros_ec_readmem-helpers-for-I2C-SPI-based-ECs/20170509-040606
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: tile-allmodconfig
compiler: tilegx-linux-gcc (GCC) 4.6.2

reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
make.cross ARCH=tile allmodconfig
make.cross ARCH=tile

All warnings (new ones prefixed by >>):

vim +279 drivers/mfd/cros_ec.c

263 return 0;
264 }
265 EXPORT_SYMBOL(cros_ec_resume);
266
267 #endif
268
269 int cros_ec_readmem(struct cros_ec_device *ec, unsigned int offset,
270 unsigned int bytes, void *dest)
271 {
272 int ret;
273 struct ec_params_read_memmap *params;
274 struct cros_ec_command *msg;
275
276 if (offset >= EC_MEMMAP_SIZE - bytes)
277 return -EINVAL;
278

 > 279		msg = kzalloc(sizeof(*msg) + max(sizeof(*params), bytes), GFP_KERNEL);

280 if (!msg)
281 return -ENOMEM;
282
283 msg->version = 0;
284 msg->command = EC_CMD_READ_MEMMAP;
285 msg->insize = bytes;
286 msg->outsize = sizeof(*params);
287

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

#2 lore raw
Quoted 9 lines · kbuild test robot

Hi Moritz,

[auto build test WARNING on ljones-mfd/for-mfd-next]
[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/Moritz-Fischer/mfd-cros-ec-Add-cros_ec_readmem-helpers-for-I2C-SPI-based-ECs/20170509-040606
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: tile-allmodconfig
compiler: tilegx-linux-gcc (GCC) 4.6.2
Quoted 5 lines

reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
make.cross ARCH=tile allmodconfig
make.cross ARCH=tile

I just locally ran that and it wouldn't print a warning. False positive?

Quoted 6 lines

All warnings (new ones prefixed by >>):

vim +279 drivers/mfd/cros_ec.c

Quoted 17 lines
   263		return 0;
   264	}
   265	EXPORT_SYMBOL(cros_ec_resume);
   266	
   267	#endif
   268	
   269	int cros_ec_readmem(struct cros_ec_device *ec, unsigned int offset,
   270				   unsigned int bytes, void *dest)
   271	{
   272		int ret;
   273		struct ec_params_read_memmap *params;
   274		struct cros_ec_command *msg;
   275	
   276		if (offset >= EC_MEMMAP_SIZE - bytes)
   277			return -EINVAL;
   278	
Quoted 1 line

279 msg = kzalloc(sizeof(*msg) + max(sizeof(*params), bytes), GFP_KERNEL);

What am I not seeing here?

Quoted 9 lines
   280		if (!msg)
   281			return -ENOMEM;
   282	
   283		msg->version = 0;
   284		msg->command = EC_CMD_READ_MEMMAP;
   285		msg->insize = bytes;
   286		msg->outsize = sizeof(*params);
   287	
Quoted 3 lines

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

Thanks,

Moritz

#3 lore raw
Quoted 16 lines · Moritz Fischer
Quoted 9 lines · kbuild test robot

Hi Moritz,

[auto build test WARNING on ljones-mfd/for-mfd-next]
[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/Moritz-Fischer/mfd-cros-ec-Add-cros_ec_readmem-helpers-for-I2C-SPI-based-ECs/20170509-040606
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: tile-allmodconfig
compiler: tilegx-linux-gcc (GCC) 4.6.2
Quoted 5 lines

reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
make.cross ARCH=tile allmodconfig
make.cross ARCH=tile

Quoted 2 lines

I just locally ran that and it wouldn't print a warning. False positive?

Good question. This is an unusual report - normally the actual warning ...

Quoted 3 lines
Quoted 3 lines

All warnings (new ones prefixed by >>):

... would be listed here, but is missing.

Guenter

Quoted 38 lines
Quoted 20 lines

vim +279 drivers/mfd/cros_ec.c

   263		return 0;
   264	}
   265	EXPORT_SYMBOL(cros_ec_resume);
   266	
   267	#endif
   268	
   269	int cros_ec_readmem(struct cros_ec_device *ec, unsigned int offset,
   270				   unsigned int bytes, void *dest)
   271	{
   272		int ret;
   273		struct ec_params_read_memmap *params;
   274		struct cros_ec_command *msg;
   275	
   276		if (offset >= EC_MEMMAP_SIZE - bytes)
   277			return -EINVAL;
   278	
Quoted 1 line

279 msg = kzalloc(sizeof(*msg) + max(sizeof(*params), bytes), GFP_KERNEL);

What am I not seeing here?

Quoted 9 lines
   280		if (!msg)
   281			return -ENOMEM;
   282	
   283		msg->version = 0;
   284		msg->command = EC_CMD_READ_MEMMAP;
   285		msg->insize = bytes;
   286		msg->outsize = sizeof(*params);
   287	
Quoted 3 lines

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

Thanks,

Moritz