linux-kernel - Re: [PATCH v2 1/3] mfd: cros_ec: Add USBPD charger commands and struct definitions.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b8fd0545-4baa-ab82-462e-95179aa9b32d@collabora.com>
Date:   Wed, 2 May 2018 11:33:06 +0200
From:   Enric Balletbo i Serra <enric.balletbo@...labora.com>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     Sebastian Reichel <sre@...nel.org>,
        Gwendal Grignou <gwendal@...omium.org>,
        Sameer Nanda <snanda@...omium.org>, linux-pm@...r.kernel.org,
        Guenter Roeck <groeck@...omium.org>,
        linux-kernel@...r.kernel.org, Benson Leung <bleung@...omium.org>,
        miguel.ojeda.sandonis@...il.com
Subject: Re: [PATCH v2 1/3] mfd: cros_ec: Add USBPD charger commands and
 struct definitions.

Hi Lee,

Thanks for the reviews.

On 01/05/18 10:29, Lee Jones wrote:
> On Mon, 30 Apr 2018, Enric Balletbo i Serra wrote:
> 
>> From: Sameer Nanda <snanda@...omium.org>
>>
>> The USBPD charger driver gets information from the ChromeOS EC, this
>> patch adds the USBPD charger definitions needed by this driver.
>>
>> Signed-off-by: Sameer Nanda <snanda@...omium.org>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@...labora.com>
>> ---
>>
>> Changes in v2: None
>>
>>  include/linux/mfd/cros_ec_commands.h | 132 ++++++++++++++++++++++++++-
>>  1 file changed, 128 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
>> index f2edd9969b40..94dcf331796c 100644
>> --- a/include/linux/mfd/cros_ec_commands.h
>> +++ b/include/linux/mfd/cros_ec_commands.h
> 
> [...]
> 
>> +struct ec_response_pd_log {
>> +	uint32_t timestamp; /* relative timestamp in milliseconds */
>> +	uint8_t type;       /* event type : see PD_EVENT_xx below */
>> +	uint8_t size_port;  /* [7:5] port number [4:0] payload size in bytes */
>> +	uint16_t data;      /* type-defined data payload */
>> +	uint8_t payload[0]; /* optional additional data payload: 0..16 bytes */
>> +} __packed;
> 
> I think this whole file should be converted to Kerneldoc format.
> 

Ok, is something I can do, let me do this in a separate patchset though.

> [...]
> 
>> +/*
>> + * PD_EVENT_MCU_CHARGE event definition :
>> + * the payload is "struct usb_chg_measures"
>> + * the data field contains the port state flags as defined below :
>> + */
>> +/* Port partner is a dual role device */
>> +#define CHARGE_FLAGS_DUAL_ROLE         (1 << 15)
> 
> BIT()?
> 

Changed in next version, there are also other places (not in this patch) where
the BIT() macro is not used. I'll send separate patchset to change it and to
convert to kerneldoc format.

>> +/* Port is the pending override port */
>> +#define CHARGE_FLAGS_DELAYED_OVERRIDE  (1 << 14)
>> +/* Port is the override port */
>> +#define CHARGE_FLAGS_OVERRIDE          (1 << 13)
> 
> [...]
>