Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1424202

Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions

Show all headers | View raw

On Thu, 02 Jun 2016, Brian Norris wrote:

> The EC_CMD_PWM_{GET,SET}_DUTY commands allow us to control a PWM that is
> attached to the EC, rather than the main host SoC. The API provides
> functionality-based (e.g., keyboard light, backlight) or index-based
> addressing of the PWM(s). Duty cycles are represented by a 16-bit value,
> where 0 maps to 0% duty cycle and U16_MAX maps to 100%. The period
> cannot be controlled.
> 
> This command set is more generic than, e.g.,
> EC_CMD_PWM_{GET,SET}_KEYBOARD_BACKLIGHT and could possibly used to
> replace it on future products.
> 
> Let's update the command header to include the definitions.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> v2: no change
> 
>  include/linux/mfd/cros_ec_commands.h | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
> index 13b630c10d4c..d673575e0ada 100644
> --- a/include/linux/mfd/cros_ec_commands.h
> +++ b/include/linux/mfd/cros_ec_commands.h
> @@ -949,6 +949,37 @@ struct ec_params_pwm_set_fan_duty {
>  	uint32_t percent;
>  } __packed;
>  
> +#define EC_CMD_PWM_SET_DUTY 0x25
> +/* 16 bit duty cycle, 65535 = 100% */
> +#define EC_PWM_MAX_DUTY 65535

Any reason this isn't represented in hex, like we do normally?

> +enum ec_pwm_type {
> +	/* All types, indexed by board-specific enum pwm_channel */
> +	EC_PWM_TYPE_GENERIC = 0,
> +	/* Keyboard backlight */
> +	EC_PWM_TYPE_KB_LIGHT,
> +	/* Display backlight */
> +	EC_PWM_TYPE_DISPLAY_LIGHT,
> +	EC_PWM_TYPE_COUNT,
> +};

Are these comments really necessary?  I'd recommend that if your
defines require comments, then they are not adequately named.  In this
case however, I'd suggest that they are and the comments are
superfluous.

> +struct ec_params_pwm_set_duty {
> +	uint16_t duty;     /* Duty cycle, EC_PWM_MAX_DUTY = 100% */
> +	uint8_t pwm_type;  /* ec_pwm_type */
> +	uint8_t index;     /* Type-specific index, or 0 if unique */
> +} __packed;

Please use kerneldoc format.

> +#define EC_CMD_PWM_GET_DUTY 0x26
> +
> +struct ec_params_pwm_get_duty {
> +	uint8_t pwm_type;  /* ec_pwm_type */
> +	uint8_t index;     /* Type-specific index, or 0 if unique */
> +} __packed;

As above.

> +struct ec_response_pwm_get_duty {
> +	uint16_t duty;     /* Duty cycle, EC_PWM_MAX_DUTY = 100% */
> +} __packed;
> +
>  /*****************************************************************************/
>  /*
>   * Lightbar commands. This looks worse than it is. Since we only use one HOST

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Back to linux.kernel | Previous | Next — Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Lee Jones <lee.jones@linaro.org> - 2016-06-16 17:40 +0200
  Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Doug Anderson <dianders@chromium.org> - 2016-06-16 18:00 +0200
    Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Lee Jones <lee.jones@linaro.org> - 2016-06-17 10:10 +0200
      Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Doug Anderson <dianders@chromium.org> - 2016-06-17 17:30 +0200
        Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Lee Jones <lee.jones@linaro.org> - 2016-06-20 10:10 +0200
          Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Brian Norris <briannorris@chromium.org> - 2016-06-20 20:00 +0200
      Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Thierry Reding <thierry.reding@gmail.com> - 2016-06-17 18:00 +0200
        Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Brian Norris <briannorris@chromium.org> - 2016-06-17 21:20 +0200
  Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Brian Norris <briannorris@chromium.org> - 2016-06-17 21:30 +0200
    Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Lee Jones <lee.jones@linaro.org> - 2016-06-20 09:50 +0200

csiph-web