From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754222AbcFPPho (ORCPT ); Thu, 16 Jun 2016 11:37:44 -0400 Received: from mail-wm0-f42.google.com ([74.125.82.42]:37261 "EHLO mail-wm0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753893AbcFPPhm (ORCPT ); Thu, 16 Jun 2016 11:37:42 -0400 Date: Thu, 16 Jun 2016 16:38:17 +0100 From: Lee Jones To: Brian Norris Cc: Thierry Reding , Olof Johansson , linux-kernel@vger.kernel.org, Doug Anderson , Brian Norris , linux-pwm@vger.kernel.org, devicetree@vger.kernel.org, Boris Brezillon , Stephen Barber , Javier Martinez Canillas , Benson Leung , Enric Balletbo , Randall Spangler , Shawn Nematbakhsh , Dmitry Torokhov , Todd Broch , Gwendal Grignou , Tomeu Vizoso Subject: Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Message-ID: <20160616153817.GD21702@dell> References: <1464916904-37554-1-git-send-email-briannorris@chromium.org> <1464916904-37554-3-git-send-email-briannorris@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1464916904-37554-3-git-send-email-briannorris@chromium.org> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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