Re: [PATCH 04/10] mfd: cros_ec: Tweak struct cros_ec_device for clarity

[Lee Jones]

On Mon, 16 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson 
> 
> The members of struct cros_ec_device were improperly commented, and
> intermixed the private and public sections. This is just cleanup to make it
> more obvious what goes with what.
> 
> [dianders: left lock in the structure but gave it the name that will
> eventually be used.]
> 
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 
> ---
>  drivers/mfd/cros_ec.c   |  2 +-
>  drivers/mfd/cros_ec_i2c.c   |  4 +--
>  drivers/mfd/cros_ec_spi.c   | 10 +++
>  include/linux/mfd/cros_ec.h | 65 
> -
>  4 files changed, 43 insertions(+), 38 deletions(-)

For the re-spin:
  Acked-by: Lee Jones 

> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index bd6f936..a9eede5 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -57,7 +57,7 @@ static int cros_ec_command_sendrecv(struct cros_ec_device 
> *ec_dev,
>   msg.in_buf = in_buf;
>   msg.in_len = in_len;
>  
> - return ec_dev->command_xfer(ec_dev, &msg);
> + return ec_dev->cmd_xfer(ec_dev, &msg);
>  }
>  
>  static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
> diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
> index 4f71be9..777e529 100644
> --- a/drivers/mfd/cros_ec_i2c.c
> +++ b/drivers/mfd/cros_ec_i2c.c
> @@ -29,7 +29,7 @@ static inline struct cros_ec_device *to_ec_dev(struct 
> device *dev)
>   return i2c_get_clientdata(client);
>  }
>  
> -static int cros_ec_command_xfer(struct cros_ec_device *ec_dev,
> +static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
>   struct cros_ec_msg *msg)
>  {
>   struct i2c_client *client = ec_dev->priv;
> @@ -136,7 +136,7 @@ static int cros_ec_i2c_probe(struct i2c_client *client,
>   ec_dev->dev = dev;
>   ec_dev->priv = client;
>   ec_dev->irq = client->irq;
> - ec_dev->command_xfer = cros_ec_command_xfer;
> + ec_dev->cmd_xfer = cros_ec_cmd_xfer_i2c;
>   ec_dev->ec_name = client->name;
>   ec_dev->phys_name = client->adapter->name;
>   ec_dev->parent = &client->dev;
> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index 0b8d328..52d4d7b 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -73,7 +73,7 @@
>   *   if no record
>   * @end_of_msg_delay: used to set the delay_usecs on the spi_transfer that
>   *  is sent when we want to turn off CS at the end of a transaction.
> - * @lock: mutex to ensure only one user of cros_ec_command_spi_xfer at a time
> + * @lock: mutex to ensure only one user of cros_ec_cmd_xfer_spi at a time
>   */
>  struct cros_ec_spi {
>   struct spi_device *spi;
> @@ -210,13 +210,13 @@ static int cros_ec_spi_receive_response(struct 
> cros_ec_device *ec_dev,
>  }
>  
>  /**
> - * cros_ec_command_spi_xfer - Transfer a message over SPI and receive the 
> reply
> + * cros_ec_cmd_xfer_spi - Transfer a message over SPI and receive the reply
>   *
>   * @ec_dev: ChromeOS EC device
>   * @ec_msg: Message to transfer
>   */
> -static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
> - struct cros_ec_msg *ec_msg)
> +static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
> + struct cros_ec_msg *ec_msg)
>  {
>   struct cros_ec_spi *ec_spi = ec_dev->priv;
>   struct spi_transfer trans;
> @@ -372,7 +372,7 @@ static int cros_ec_spi_probe(struct spi_device *spi)
>   ec_dev->dev = dev;
>   ec_dev->priv = ec_spi;
>   ec_dev->irq = spi->irq;
> - ec_dev->command_xfer = cros_ec_command_spi_xfer;
> + ec_dev->cmd_xfer = cros_ec_cmd_xfer_spi;
>   ec_dev->ec_name = ec_spi->spi->modalias;
>   ec_dev->phys_name = dev_name(&ec_spi->spi->dev);
>   ec_dev->parent = &ec_spi->spi->dev;
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 2ee3190..79a3585 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -16,7 +16,9 @@
>  #ifndef __LINUX_MFD_CROS_EC_H
>  #define __LINUX_MFD_CROS_EC_H
>  
> +#include 
>  #include 
> +#include 
>  
>  /*
>   * Command interface between EC and AP, for LPC, I2C and SPI interfaces.
> @@ -55,34 +57,53 @@ struct cros_ec_msg {
>  /**
>   * struct cros_ec_device - Information about a ChromeOS EC device
>   *
> + * @ec_name: name of EC device (e.g. 'chromeos-ec')
> + * @phys_name: name of physical comms layer (e.g. 'i2c-4')
> + * @dev: Device pointer
> + * @was_wake_device: true if this device was set to wake the system from
> + * sleep at the last suspend
> + * @event_notifier: interrupt event notifier for transport devices
> + * @command_send: send a command
> + * @command_recv: receive a response
> + * @command_sendrecv: send a command and receive a response
> + *
>   * @name: Name of this EC interface
>   * @priv: Private data
>   * @irq: Interrupt to use
> - * @din: input buffer (from EC)
>

Re: [PATCH 04/10] mfd: cros_ec: Tweak struct cros_ec_device for clarity

[Doug Anderson]

Simon,

On Tue, Jun 17, 2014 at 8:35 PM, Simon Glass  wrote:
> Hi Doug,
>
> On 16 June 2014 14:39, Doug Anderson  wrote:
>> From: Bill Richardson 
>>
>> The members of struct cros_ec_device were improperly commented, and
>> intermixed the private and public sections. This is just cleanup to make it
>> more obvious what goes with what.
>>
>> [dianders: left lock in the structure but gave it the name that will
>> eventually be used.]
>>
>> Signed-off-by: Bill Richardson 
>> Signed-off-by: Doug Anderson 
>> ---
>>  drivers/mfd/cros_ec.c   |  2 +-
>>  drivers/mfd/cros_ec_i2c.c   |  4 +--
>>  drivers/mfd/cros_ec_spi.c   | 10 +++
>>  include/linux/mfd/cros_ec.h | 65 
>> -
>>  4 files changed, 43 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
>> index bd6f936..a9eede5 100644
>> --- a/drivers/mfd/cros_ec.c
>> +++ b/drivers/mfd/cros_ec.c
>> @@ -57,7 +57,7 @@ static int cros_ec_command_sendrecv(struct cros_ec_device 
>> *ec_dev,
>> msg.in_buf = in_buf;
>> msg.in_len = in_len;
>>
>> -   return ec_dev->command_xfer(ec_dev, &msg);
>> +   return ec_dev->cmd_xfer(ec_dev, &msg);
>
> Why do this rename? It makes it different from the other members.

All I know of the history of this change is at
.  My best guess is that Bill was trying
to differentiate public vs. private function pointers.  Perhaps he
will chime in.

If it helps the other command_xxx() function pointers are removed in
the later "mfd: cros_ec: cleanup: Remove EC wrapper functions"

If you wish I can skip this rename, just let me know and it won't be
too much trouble.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/10] mfd: cros_ec: Tweak struct cros_ec_device for clarity

[Simon Glass]

Hi Doug,

On 16 June 2014 14:39, Doug Anderson  wrote:
> From: Bill Richardson 
>
> The members of struct cros_ec_device were improperly commented, and
> intermixed the private and public sections. This is just cleanup to make it
> more obvious what goes with what.
>
> [dianders: left lock in the structure but gave it the name that will
> eventually be used.]
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 
> ---
>  drivers/mfd/cros_ec.c   |  2 +-
>  drivers/mfd/cros_ec_i2c.c   |  4 +--
>  drivers/mfd/cros_ec_spi.c   | 10 +++
>  include/linux/mfd/cros_ec.h | 65 
> -
>  4 files changed, 43 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index bd6f936..a9eede5 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -57,7 +57,7 @@ static int cros_ec_command_sendrecv(struct cros_ec_device 
> *ec_dev,
> msg.in_buf = in_buf;
> msg.in_len = in_len;
>
> -   return ec_dev->command_xfer(ec_dev, &msg);
> +   return ec_dev->cmd_xfer(ec_dev, &msg);

Why do this rename? It makes it different from the other members.

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 04/10] mfd: cros_ec: Tweak struct cros_ec_device for clarity

[Doug Anderson]

From: Bill Richardson 

The members of struct cros_ec_device were improperly commented, and
intermixed the private and public sections. This is just cleanup to make it
more obvious what goes with what.

[dianders: left lock in the structure but gave it the name that will
eventually be used.]

Signed-off-by: Bill Richardson 
Signed-off-by: Doug Anderson 
---
 drivers/mfd/cros_ec.c   |  2 +-
 drivers/mfd/cros_ec_i2c.c   |  4 +--
 drivers/mfd/cros_ec_spi.c   | 10 +++
 include/linux/mfd/cros_ec.h | 65 -
 4 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index bd6f936..a9eede5 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -57,7 +57,7 @@ static int cros_ec_command_sendrecv(struct cros_ec_device 
*ec_dev,
msg.in_buf = in_buf;
msg.in_len = in_len;
 
-   return ec_dev->command_xfer(ec_dev, &msg);
+   return ec_dev->cmd_xfer(ec_dev, &msg);
 }
 
 static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index 4f71be9..777e529 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -29,7 +29,7 @@ static inline struct cros_ec_device *to_ec_dev(struct device 
*dev)
return i2c_get_clientdata(client);
 }
 
-static int cros_ec_command_xfer(struct cros_ec_device *ec_dev,
+static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
struct cros_ec_msg *msg)
 {
struct i2c_client *client = ec_dev->priv;
@@ -136,7 +136,7 @@ static int cros_ec_i2c_probe(struct i2c_client *client,
ec_dev->dev = dev;
ec_dev->priv = client;
ec_dev->irq = client->irq;
-   ec_dev->command_xfer = cros_ec_command_xfer;
+   ec_dev->cmd_xfer = cros_ec_cmd_xfer_i2c;
ec_dev->ec_name = client->name;
ec_dev->phys_name = client->adapter->name;
ec_dev->parent = &client->dev;
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 0b8d328..52d4d7b 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -73,7 +73,7 @@
  * if no record
  * @end_of_msg_delay: used to set the delay_usecs on the spi_transfer that
  *  is sent when we want to turn off CS at the end of a transaction.
- * @lock: mutex to ensure only one user of cros_ec_command_spi_xfer at a time
+ * @lock: mutex to ensure only one user of cros_ec_cmd_xfer_spi at a time
  */
 struct cros_ec_spi {
struct spi_device *spi;
@@ -210,13 +210,13 @@ static int cros_ec_spi_receive_response(struct 
cros_ec_device *ec_dev,
 }
 
 /**
- * cros_ec_command_spi_xfer - Transfer a message over SPI and receive the reply
+ * cros_ec_cmd_xfer_spi - Transfer a message over SPI and receive the reply
  *
  * @ec_dev: ChromeOS EC device
  * @ec_msg: Message to transfer
  */
-static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
-   struct cros_ec_msg *ec_msg)
+static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
+   struct cros_ec_msg *ec_msg)
 {
struct cros_ec_spi *ec_spi = ec_dev->priv;
struct spi_transfer trans;
@@ -372,7 +372,7 @@ static int cros_ec_spi_probe(struct spi_device *spi)
ec_dev->dev = dev;
ec_dev->priv = ec_spi;
ec_dev->irq = spi->irq;
-   ec_dev->command_xfer = cros_ec_command_spi_xfer;
+   ec_dev->cmd_xfer = cros_ec_cmd_xfer_spi;
ec_dev->ec_name = ec_spi->spi->modalias;
ec_dev->phys_name = dev_name(&ec_spi->spi->dev);
ec_dev->parent = &ec_spi->spi->dev;
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 2ee3190..79a3585 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -16,7 +16,9 @@
 #ifndef __LINUX_MFD_CROS_EC_H
 #define __LINUX_MFD_CROS_EC_H
 
+#include 
 #include 
+#include 
 
 /*
  * Command interface between EC and AP, for LPC, I2C and SPI interfaces.
@@ -55,34 +57,53 @@ struct cros_ec_msg {
 /**
  * struct cros_ec_device - Information about a ChromeOS EC device
  *
+ * @ec_name: name of EC device (e.g. 'chromeos-ec')
+ * @phys_name: name of physical comms layer (e.g. 'i2c-4')
+ * @dev: Device pointer
+ * @was_wake_device: true if this device was set to wake the system from
+ * sleep at the last suspend
+ * @event_notifier: interrupt event notifier for transport devices
+ * @command_send: send a command
+ * @command_recv: receive a response
+ * @command_sendrecv: send a command and receive a response
+ *
  * @name: Name of this EC interface
  * @priv: Private data
  * @irq: Interrupt to use
- * @din: input buffer (from EC)
- * @dout: output buffer (to EC)
+ * @din: input buffer (for data from EC)
+ * @dout: output buffer (for data to EC)
  * \note
  * These two buffers will always be dword-aligned and include enough
  * space for up to 7 word-alignment bytes also, so we can ensure that
  * the body of t