linux-kernel - Re: [PATCH v5 6/8] platform/chrome: cros_ec

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: <CAE-0n52Y-t0TEuGS25GQsctCLC8FTm6A5tfQb3XLbwh2G5W_MA@mail.gmail.com>
Date: Tue, 17 Dec 2024 20:16:52 -0500
From: Stephen Boyd <swboyd@...omium.org>
To: Abhishek Pandit-Subedi <abhishekpandit@...omium.org>, chrome-platform@...ts.linux.dev, 
	heikki.krogerus@...ux.intel.com, linux-usb@...r.kernel.org, 
	tzungbi@...nel.org
Cc: akuchynski@...gle.com, pmalani@...omium.org, badhri@...gle.com, 
	rdbabiera@...gle.com, dmitry.baryshkov@...aro.org, jthies@...gle.com, 
	Benson Leung <bleung@...omium.org>, Guenter Roeck <groeck@...omium.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 6/8] platform/chrome: cros_ec_typec: Displayport support

Quoting Abhishek Pandit-Subedi (2024-12-13 15:35:47)
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index fb8335458a22..1a5a484563cc 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -19,7 +19,11 @@ obj-$(CONFIG_CROS_EC_SPI)            += cros_ec_spi.o
>  obj-$(CONFIG_CROS_EC_UART)             += cros_ec_uart.o
>  cros_ec_lpcs-objs                      := cros_ec_lpc.o cros_ec_lpc_mec.o
>  cros-ec-typec-objs                     := cros_ec_typec.o cros_typec_vdm.o
> +ifneq ($(CONFIG_CROS_EC_TYPEC_ALTMODES),)
> +       cros-ec-typec-objs              += cros_typec_altmode.o
> +endif

Can this be simplified by collapsing the config in because it is a bool?

	cros-ec-typec-y					:= cros_ec_typec.o cros_typec_vdm.o
	cros-ec-typec-$(CONFIG_CROS_EC_TYPEC_ALTMODES)	+= cros_typec_altmode.o

>  obj-$(CONFIG_CROS_EC_TYPEC)            += cros-ec-typec.o
> +
>  obj-$(CONFIG_CROS_EC_LPC)              += cros_ec_lpcs.o
>  obj-$(CONFIG_CROS_EC_PROTO)            += cros_ec_proto.o cros_ec_trace.o
>  obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT)   += cros_kbd_led_backlight.o
> diff --git a/drivers/platform/chrome/cros_typec_altmode.c b/drivers/platform/chrome/cros_typec_altmode.c
> new file mode 100644
> index 000000000000..6e736168ccc3
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_typec_altmode.c
> @@ -0,0 +1,285 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Alt-mode implementation on ChromeOS EC.
> + *
> + * Copyright 2024 Google LLC
> + * Author: Abhishek Pandit-Subedi <abhishekpandit@...omium.org>
> + */
> +#include "cros_ec_typec.h"

Please include this after linux headers.

> +
> +#include <linux/mutex.h>
> +#include <linux/workqueue.h>
> +#include <linux/usb/typec_dp.h>
> +#include <linux/usb/pd_vdo.h>
> +
> +#include "cros_typec_altmode.h"
> +

[...]
> +
> +static int cros_typec_displayport_vdm(struct typec_altmode *alt, u32 header,
> +                                     const u32 *data, int count)
> +{
> +       struct cros_typec_dp_data *dp_data = typec_altmode_get_drvdata(alt);
> +       struct cros_typec_altmode_data *adata = &dp_data->adata;
> +
> +

Nitpick: Remove extra newlines.

> +       int cmd_type = PD_VDO_CMDT(header);
> +       int cmd = PD_VDO_CMD(header);
> +       int svdm_version;
> +
> +       svdm_version = typec_altmode_get_svdm_version(alt);
> +       if (svdm_version < 0)
> +               return svdm_version;
> +