linux-kernel - Re: [PATCH v2 2/5] platform: arm64: add Huawei Matebook E Go EC driver

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: <Z30dIRA4MdtCp63q@kuha.fi.intel.com>
Date: Tue, 7 Jan 2025 14:25:05 +0200
From: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To: Pengyu Luo <mitltlatltl@...il.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Bjorn Andersson <andersson@...nel.org>,
	Konrad Dybcio <konradybcio@...nel.org>,
	Hans de Goede <hdegoede@...hat.com>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
	Sebastian Reichel <sre@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jean Delvare <jdelvare@...e.com>,
	Guenter Roeck <linux@...ck-us.net>, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	platform-driver-x86@...r.kernel.org, linux-pm@...r.kernel.org,
	linux-usb@...r.kernel.org, linux-hwmon@...r.kernel.org,
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Subject: Re: [PATCH v2 2/5] platform: arm64: add Huawei Matebook E Go EC
 driver

Hi,

> +/* -------------------------------------------------------------------------- */
> +/* API For UCSI */
> +
> +int gaokun_ec_ucsi_read(struct gaokun_ec *ec,
> +			u8 resp[GAOKUN_UCSI_READ_SIZE])
> +{
> +	u8 req[] = MKREQ(0x03, 0xD5, 0);
> +	u8 _resp[] = MKRESP(GAOKUN_UCSI_READ_SIZE);
> +	int ret;
> +
> +	ret = gaokun_ec_read(ec, req, sizeof(_resp), _resp);
> +	if (ret)
> +		return ret;
> +
> +	extr_resp(resp, _resp, GAOKUN_UCSI_READ_SIZE);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(gaokun_ec_ucsi_read);
> +
> +int gaokun_ec_ucsi_write(struct gaokun_ec *ec,
> +			 const u8 req[GAOKUN_UCSI_WRITE_SIZE])
> +{
> +	u8 _req[] = MKREQ(0x03, 0xD4, GAOKUN_UCSI_WRITE_SIZE);
> +
> +
> +	refill_req(_req, req, GAOKUN_UCSI_WRITE_SIZE);
> +
> +	return gaokun_ec_write(ec, _req);
> +}
> +EXPORT_SYMBOL_GPL(gaokun_ec_ucsi_write);
> +
> +int gaokun_ec_ucsi_get_reg(struct gaokun_ec *ec, u8 *ureg)
> +{
> +	u8 req[] = MKREQ(0x03, 0xD3, 0);
> +	u8 _resp[] = MKRESP(UCSI_REG_SIZE);
> +	int ret;
> +
> +	ret = gaokun_ec_read(ec, req, sizeof(_resp), _resp);
> +	if (ret)
> +		return ret;
> +
> +	extr_resp(ureg, _resp, UCSI_REG_SIZE);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(gaokun_ec_ucsi_get_reg);

Why not just take struct gaokun_ucsi_reg as parameter? I did not see
this (or any of these) being used anywhere else except in your UCSI
glue driver. So the prototype would be:

        int gaokun_ec_ucsi_get_reg(struct gaokun_ec *ec,
                                   struct gaokun_ucsi_reg *reg);

> +int gaokun_ec_ucsi_pan_ack(struct gaokun_ec *ec, int port_id)
> +{
> +	u8 req[] = MKREQ(0x03, 0xD2, 1);
> +	u8 data = 1 << port_id;
> +
> +	if (port_id == GAOKUN_UCSI_NO_PORT_UPDATE)
> +		data = 0;
> +
> +	refill_req(req, &data, 1);
> +
> +	return gaokun_ec_write(ec, req);
> +}
> +EXPORT_SYMBOL_GPL(gaokun_ec_ucsi_pan_ack);

I think you should add proper kernel doc comments to these exported
functions.

thanks,

-- 
heikki