Re: [PATCH v3 3/7] mfd: cros_ec: Move protocol helpers out of the MFD driver

2015-05-27 Thread Lee Jones
On Fri, 22 May 2015, Javier Martinez Canillas wrote:

 The MFD driver should only have the logic to instantiate its child devices
 and setup any shared resources that will be used by the subdevices drivers.
 
 The cros_ec MFD is more complex than expected since it also has helpers to
 communicate with the EC. So the driver will only get more bigger as other
 protocols are supported in the future. So move the communication protocol
 helpers to its own driver as drivers/platform/chrome/cros_ec_proto.c.
 
 Suggested-by: Lee Jones lee.jo...@linaro.org
 Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
 ---
 
 Changes since v2: None, new patch.
 ---
  drivers/i2c/busses/Kconfig  |   2 +-
  drivers/input/keyboard/Kconfig  |   2 +-
  drivers/mfd/Kconfig |   5 +-
  drivers/mfd/cros_ec.c   |  96 --

:)

Acked-by: Lee Jones lee.jo...@linaro.org

  drivers/platform/chrome/Kconfig |   9 ++-
  drivers/platform/chrome/Makefile|   1 +
  drivers/platform/chrome/cros_ec_proto.c | 115 
 
  7 files changed, 128 insertions(+), 102 deletions(-)
  create mode 100644 drivers/platform/chrome/cros_ec_proto.c
 
 diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
 index 2255af23b9c7..5f1c1c4f5d87 100644
 --- a/drivers/i2c/busses/Kconfig
 +++ b/drivers/i2c/busses/Kconfig
 @@ -1103,7 +1103,7 @@ config I2C_SIBYTE
  
  config I2C_CROS_EC_TUNNEL
   tristate ChromeOS EC tunnel I2C bus
 - depends on MFD_CROS_EC
 + depends on CROS_EC_PROTO
   help
 If you say yes here you get an I2C bus that will tunnel i2c commands
 through to the other side of the ChromeOS EC to the i2c bus
 diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
 index 106fbac7f8c5..e8eb60c6d83e 100644
 --- a/drivers/input/keyboard/Kconfig
 +++ b/drivers/input/keyboard/Kconfig
 @@ -677,7 +677,7 @@ config KEYBOARD_W90P910
  config KEYBOARD_CROS_EC
   tristate ChromeOS EC keyboard
   select INPUT_MATRIXKMAP
 - depends on MFD_CROS_EC
 + depends on CROS_EC_PROTO
   help
 Say Y here to enable the matrix keyboard used by ChromeOS devices
 and implemented on the ChromeOS EC. You must enable one bus option
 diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
 index d5ad04dad081..927ba61e5bf9 100644
 --- a/drivers/mfd/Kconfig
 +++ b/drivers/mfd/Kconfig
 @@ -94,6 +94,7 @@ config MFD_AXP20X
  config MFD_CROS_EC
   tristate ChromeOS Embedded Controller
   select MFD_CORE
 + select CROS_EC_PROTO
   help
 If you say Y here you get support for the ChromeOS Embedded
 Controller (EC) providing keyboard, battery and power services.
 @@ -102,7 +103,7 @@ config MFD_CROS_EC
  
  config MFD_CROS_EC_I2C
   tristate ChromeOS Embedded Controller (I2C)
 - depends on MFD_CROS_EC  I2C
 + depends on MFD_CROS_EC  CROS_EC_PROTO  I2C
  
   help
 If you say Y here, you get support for talking to the ChromeOS
 @@ -112,7 +113,7 @@ config MFD_CROS_EC_I2C
  
  config MFD_CROS_EC_SPI
   tristate ChromeOS Embedded Controller (SPI)
 - depends on MFD_CROS_EC  SPI  OF
 + depends on MFD_CROS_EC  CROS_EC_PROTO  SPI  OF
  
   ---help---
 If you say Y here, you get support for talking to the ChromeOS EC
 diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
 index 4a0f6dfcd376..d857f6a2b57b 100644
 --- a/drivers/mfd/cros_ec.c
 +++ b/drivers/mfd/cros_ec.c
 @@ -23,102 +23,6 @@
  #include linux/module.h
  #include linux/mfd/core.h
  #include linux/mfd/cros_ec.h
 -#include linux/mfd/cros_ec_commands.h
 -#include linux/delay.h
 -
 -#define EC_COMMAND_RETRIES   50
 -
 -int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
 -struct cros_ec_command *msg)
 -{
 - uint8_t *out;
 - int csum, i;
 -
 - BUG_ON(msg-outsize  EC_PROTO2_MAX_PARAM_SIZE);
 - out = ec_dev-dout;
 - out[0] = EC_CMD_VERSION0 + msg-version;
 - out[1] = msg-command;
 - out[2] = msg-outsize;
 - csum = out[0] + out[1] + out[2];
 - for (i = 0; i  msg-outsize; i++)
 - csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg-data[i];
 - out[EC_MSG_TX_HEADER_BYTES + msg-outsize] = (uint8_t)(csum  0xff);
 -
 - return EC_MSG_TX_PROTO_BYTES + msg-outsize;
 -}
 -EXPORT_SYMBOL(cros_ec_prepare_tx);
 -
 -int cros_ec_check_result(struct cros_ec_device *ec_dev,
 -  struct cros_ec_command *msg)
 -{
 - switch (msg-result) {
 - case EC_RES_SUCCESS:
 - return 0;
 - case EC_RES_IN_PROGRESS:
 - dev_dbg(ec_dev-dev, command 0x%02x in progress\n,
 - msg-command);
 - return -EAGAIN;
 - default:
 - dev_dbg(ec_dev-dev, command 0x%02x returned %d\n,
 - msg-command, msg-result);
 - return 0;
 - }
 -}
 -EXPORT_SYMBOL(cros_ec_check_result);
 -
 -int 

[PATCH v3 3/7] mfd: cros_ec: Move protocol helpers out of the MFD driver

2015-05-22 Thread Javier Martinez Canillas
The MFD driver should only have the logic to instantiate its child devices
and setup any shared resources that will be used by the subdevices drivers.

The cros_ec MFD is more complex than expected since it also has helpers to
communicate with the EC. So the driver will only get more bigger as other
protocols are supported in the future. So move the communication protocol
helpers to its own driver as drivers/platform/chrome/cros_ec_proto.c.

Suggested-by: Lee Jones lee.jo...@linaro.org
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
---

Changes since v2: None, new patch.
---
 drivers/i2c/busses/Kconfig  |   2 +-
 drivers/input/keyboard/Kconfig  |   2 +-
 drivers/mfd/Kconfig |   5 +-
 drivers/mfd/cros_ec.c   |  96 --
 drivers/platform/chrome/Kconfig |   9 ++-
 drivers/platform/chrome/Makefile|   1 +
 drivers/platform/chrome/cros_ec_proto.c | 115 
 7 files changed, 128 insertions(+), 102 deletions(-)
 create mode 100644 drivers/platform/chrome/cros_ec_proto.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 2255af23b9c7..5f1c1c4f5d87 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1103,7 +1103,7 @@ config I2C_SIBYTE
 
 config I2C_CROS_EC_TUNNEL
tristate ChromeOS EC tunnel I2C bus
-   depends on MFD_CROS_EC
+   depends on CROS_EC_PROTO
help
  If you say yes here you get an I2C bus that will tunnel i2c commands
  through to the other side of the ChromeOS EC to the i2c bus
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 106fbac7f8c5..e8eb60c6d83e 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -677,7 +677,7 @@ config KEYBOARD_W90P910
 config KEYBOARD_CROS_EC
tristate ChromeOS EC keyboard
select INPUT_MATRIXKMAP
-   depends on MFD_CROS_EC
+   depends on CROS_EC_PROTO
help
  Say Y here to enable the matrix keyboard used by ChromeOS devices
  and implemented on the ChromeOS EC. You must enable one bus option
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index d5ad04dad081..927ba61e5bf9 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -94,6 +94,7 @@ config MFD_AXP20X
 config MFD_CROS_EC
tristate ChromeOS Embedded Controller
select MFD_CORE
+   select CROS_EC_PROTO
help
  If you say Y here you get support for the ChromeOS Embedded
  Controller (EC) providing keyboard, battery and power services.
@@ -102,7 +103,7 @@ config MFD_CROS_EC
 
 config MFD_CROS_EC_I2C
tristate ChromeOS Embedded Controller (I2C)
-   depends on MFD_CROS_EC  I2C
+   depends on MFD_CROS_EC  CROS_EC_PROTO  I2C
 
help
  If you say Y here, you get support for talking to the ChromeOS
@@ -112,7 +113,7 @@ config MFD_CROS_EC_I2C
 
 config MFD_CROS_EC_SPI
tristate ChromeOS Embedded Controller (SPI)
-   depends on MFD_CROS_EC  SPI  OF
+   depends on MFD_CROS_EC  CROS_EC_PROTO  SPI  OF
 
---help---
  If you say Y here, you get support for talking to the ChromeOS EC
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 4a0f6dfcd376..d857f6a2b57b 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -23,102 +23,6 @@
 #include linux/module.h
 #include linux/mfd/core.h
 #include linux/mfd/cros_ec.h
-#include linux/mfd/cros_ec_commands.h
-#include linux/delay.h
-
-#define EC_COMMAND_RETRIES 50
-
-int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
-  struct cros_ec_command *msg)
-{
-   uint8_t *out;
-   int csum, i;
-
-   BUG_ON(msg-outsize  EC_PROTO2_MAX_PARAM_SIZE);
-   out = ec_dev-dout;
-   out[0] = EC_CMD_VERSION0 + msg-version;
-   out[1] = msg-command;
-   out[2] = msg-outsize;
-   csum = out[0] + out[1] + out[2];
-   for (i = 0; i  msg-outsize; i++)
-   csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg-data[i];
-   out[EC_MSG_TX_HEADER_BYTES + msg-outsize] = (uint8_t)(csum  0xff);
-
-   return EC_MSG_TX_PROTO_BYTES + msg-outsize;
-}
-EXPORT_SYMBOL(cros_ec_prepare_tx);
-
-int cros_ec_check_result(struct cros_ec_device *ec_dev,
-struct cros_ec_command *msg)
-{
-   switch (msg-result) {
-   case EC_RES_SUCCESS:
-   return 0;
-   case EC_RES_IN_PROGRESS:
-   dev_dbg(ec_dev-dev, command 0x%02x in progress\n,
-   msg-command);
-   return -EAGAIN;
-   default:
-   dev_dbg(ec_dev-dev, command 0x%02x returned %d\n,
-   msg-command, msg-result);
-   return 0;
-   }
-}
-EXPORT_SYMBOL(cros_ec_check_result);
-
-int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
-struct cros_ec_command *msg)
-{
-   int ret;
-
-