[PATCH] platform/chrome: cros_ec: Fix host command for regulator control.

[PATCH] platform/chrome: cros_ec: Fix host command for regulator control.

[Pi-Hsun Shih]

Since the host command number 0x012B conflicts with other EC host
command, add one to all regulator control related host command.

Also fix a wrong alignment on struct and sync the comment with the one
in ChromeOS EC codebase.

Fixes: dff08caf35ec ("platform/chrome: cros_ec: Add command for regulator control.")
Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
---
The original numbers were chosen before the 0x012B is used in ChromeOS
EC codebase. Since the original kernel patch got accepted before the
corresponding commit in ChromeOS EC codebase got merged, the host
command number was used by other commit first.

Since now the commit in ChromeOS EC codebase
(https://crrev.com/c/2247431) with updated host command numbers got
merged, need this patch to sync up the host command numbers with
ChromeOS EC codebase. Sorry for the confusion.
---
 include/linux/platform_data/cros_ec_commands.h | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index a417b51b5764..91e77f53414d 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -5438,7 +5438,7 @@ struct ec_response_rollback_info {
  *
  * Returns the regulator name and supported voltage list in mV.
  */
-#define EC_CMD_REGULATOR_GET_INFO 0x012B
+#define EC_CMD_REGULATOR_GET_INFO 0x012C
 
 /* Maximum length of regulator name */
 #define EC_REGULATOR_NAME_MAX_LEN 16
@@ -5454,12 +5454,12 @@ struct ec_response_regulator_get_info {
 	char name[EC_REGULATOR_NAME_MAX_LEN];
 	uint16_t num_voltages;
 	uint16_t voltages_mv[EC_REGULATOR_VOLTAGE_MAX_COUNT];
-} __ec_align1;
+} __ec_align2;
 
 /*
  * Configure the regulator as enabled / disabled.
  */
-#define EC_CMD_REGULATOR_ENABLE 0x012C
+#define EC_CMD_REGULATOR_ENABLE 0x012D
 
 struct ec_params_regulator_enable {
 	uint32_t index;
@@ -5471,7 +5471,7 @@ struct ec_params_regulator_enable {
  *
  * Returns 1 if the regulator is enabled, 0 if not.
  */
-#define EC_CMD_REGULATOR_IS_ENABLED 0x012D
+#define EC_CMD_REGULATOR_IS_ENABLED 0x012E
 
 struct ec_params_regulator_is_enabled {
 	uint32_t index;
@@ -5489,7 +5489,7 @@ struct ec_response_regulator_is_enabled {
  * Also note that this might be called before the regulator is enabled, and the
  * setting should be in effect after the regulator is enabled.
  */
-#define EC_CMD_REGULATOR_SET_VOLTAGE 0x012E
+#define EC_CMD_REGULATOR_SET_VOLTAGE 0x012F
 
 struct ec_params_regulator_set_voltage {
 	uint32_t index;
@@ -5500,9 +5500,10 @@ struct ec_params_regulator_set_voltage {
 /*
  * Get the currently configured voltage for the voltage regulator.
  *
- * Note that this might be called before the regulator is enabled.
+ * Note that this might be called before the regulator is enabled, and this
+ * should return the configured output voltage if the regulator is enabled.
  */
-#define EC_CMD_REGULATOR_GET_VOLTAGE 0x012F
+#define EC_CMD_REGULATOR_GET_VOLTAGE 0x0130
 
 struct ec_params_regulator_get_voltage {
 	uint32_t index;

base-commit: 8d9f8d57e023893bfa708d83e3a787e77766a378
-- 
2.28.0.rc0.142.g3c755180ce-goog

Re: [PATCH] platform/chrome: cros_ec: Fix host command for regulator control.

[Enric Balletbo i Serra]

Hi Pi-Hsun, Mark

Thank you for your patch.

On 24/7/20 10:03, Pi-Hsun Shih wrote:
> Since the host command number 0x012B conflicts with other EC host
> command, add one to all regulator control related host command.
> 
> Also fix a wrong alignment on struct and sync the comment with the one
> in ChromeOS EC codebase.
> 
> Fixes: dff08caf35ec ("platform/chrome: cros_ec: Add command for regulator control.")
> Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>

Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

This needs to go through Mark Brown's regulator tree as depends on
dff08caf35ec, if not, I can pick as a fix when the next merge window is closed.

> ---
> The original numbers were chosen before the 0x012B is used in ChromeOS
> EC codebase. Since the original kernel patch got accepted before the
> corresponding commit in ChromeOS EC codebase got merged, the host
> command number was used by other commit first.
> 

Ups, next time we need to make sure the EC code lands before.


> Since now the commit in ChromeOS EC codebase
> (https://crrev.com/c/2247431) with updated host command numbers got
> merged, need this patch to sync up the host command numbers with
> ChromeOS EC codebase. Sorry for the confusion.
> ---
>  include/linux/platform_data/cros_ec_commands.h | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
> index a417b51b5764..91e77f53414d 100644
> --- a/include/linux/platform_data/cros_ec_commands.h
> +++ b/include/linux/platform_data/cros_ec_commands.h
> @@ -5438,7 +5438,7 @@ struct ec_response_rollback_info {
>   *
>   * Returns the regulator name and supported voltage list in mV.
>   */
> -#define EC_CMD_REGULATOR_GET_INFO 0x012B
> +#define EC_CMD_REGULATOR_GET_INFO 0x012C
>  
>  /* Maximum length of regulator name */
>  #define EC_REGULATOR_NAME_MAX_LEN 16
> @@ -5454,12 +5454,12 @@ struct ec_response_regulator_get_info {
>  	char name[EC_REGULATOR_NAME_MAX_LEN];
>  	uint16_t num_voltages;
>  	uint16_t voltages_mv[EC_REGULATOR_VOLTAGE_MAX_COUNT];
> -} __ec_align1;
> +} __ec_align2;
>  
>  /*
>   * Configure the regulator as enabled / disabled.
>   */
> -#define EC_CMD_REGULATOR_ENABLE 0x012C
> +#define EC_CMD_REGULATOR_ENABLE 0x012D
>  
>  struct ec_params_regulator_enable {
>  	uint32_t index;
> @@ -5471,7 +5471,7 @@ struct ec_params_regulator_enable {
>   *
>   * Returns 1 if the regulator is enabled, 0 if not.
>   */
> -#define EC_CMD_REGULATOR_IS_ENABLED 0x012D
> +#define EC_CMD_REGULATOR_IS_ENABLED 0x012E
>  
>  struct ec_params_regulator_is_enabled {
>  	uint32_t index;
> @@ -5489,7 +5489,7 @@ struct ec_response_regulator_is_enabled {
>   * Also note that this might be called before the regulator is enabled, and the
>   * setting should be in effect after the regulator is enabled.
>   */
> -#define EC_CMD_REGULATOR_SET_VOLTAGE 0x012E
> +#define EC_CMD_REGULATOR_SET_VOLTAGE 0x012F
>  
>  struct ec_params_regulator_set_voltage {
>  	uint32_t index;
> @@ -5500,9 +5500,10 @@ struct ec_params_regulator_set_voltage {
>  /*
>   * Get the currently configured voltage for the voltage regulator.
>   *
> - * Note that this might be called before the regulator is enabled.
> + * Note that this might be called before the regulator is enabled, and this
> + * should return the configured output voltage if the regulator is enabled.
>   */
> -#define EC_CMD_REGULATOR_GET_VOLTAGE 0x012F
> +#define EC_CMD_REGULATOR_GET_VOLTAGE 0x0130
>  
>  struct ec_params_regulator_get_voltage {
>  	uint32_t index;
> 
> base-commit: 8d9f8d57e023893bfa708d83e3a787e77766a378
> 

Re: [PATCH] platform/chrome: cros_ec: Fix host command for regulator control.

[Mark Brown]

On Fri, 24 Jul 2020 16:03:55 +0800, Pi-Hsun Shih wrote:
> Since the host command number 0x012B conflicts with other EC host
> command, add one to all regulator control related host command.
> 
> Also fix a wrong alignment on struct and sync the comment with the one
> in ChromeOS EC codebase.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/1] platform/chrome: cros_ec: Fix host command for regulator control.
      commit: a233547660a3915973d41e2a9a0923d0cf317a62

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark