linux-kernel - Re: [PATCH v2 18/22] phy: apple: Add Apple Type-C PHY

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: <20250908181259.GC89417@robin.jannau.net>
Date: Mon, 8 Sep 2025 20:12:59 +0200
From: Janne Grunau <j@...nau.net>
To: Sven Peter <sven@...nel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Felipe Balbi <balbi@...nel.org>,
	Alyssa Rosenzweig <alyssa@...enzweig.io>,
	Neal Gompa <neal@...pa.dev>, Vinod Koul <vkoul@...nel.org>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Philipp Zabel <p.zabel@...gutronix.de>, Frank Li <Frank.Li@....com>,
	Ran Wang <ran.wang_1@....com>, Peter Chen <peter.chen@....com>,
	linux-usb@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, asahi@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org, linux-phy@...ts.infradead.org,
	Hector Martin <marcan@...can.st>
Subject: Re: [PATCH v2 18/22] phy: apple: Add Apple Type-C PHY

On Sat, Sep 06, 2025 at 03:43:31PM +0000, Sven Peter wrote:
> The Apple Type-C PHY (ATCPHY) is a PHY for USB 2.0, USB 3.x,
> USB4/Thunderbolt, and DisplayPort connectivity found in Apple Silicon SoCs.
> The PHY handles muxing between these different protocols and also provides
> the reset controller for the attached dwc3 USB controller.
> 
> There is no documentation available for this PHY and the entire sequence
> of MMIO pokes has been figured out by tracing all MMIO access of Apple's
> driver under a thin hypervisor and correlating the register reads/writes
> to their kernel's debug output to find their names. Deviations from this
> sequence generally results in the port not working or, especially when
> the mode is switched to USB4 or Thunderbolt, to some watchdog resetting
> the entire SoC.
> 
> This initial commit already introduces support for Display Port and
> USB4/Thunderbolt but the drivers for these are not ready. We cannot
> control the alternate mode negotiation and are stuck with whatever Apple's
> firmware decides such that any DisplayPort or USB4/Thunderbolt device will
> result in a correctly setup PHY but not be usable until the other drivers
> are upstreamed as well.
> 
> Co-developed-by: Janne Grunau <j@...nau.net>
> Signed-off-by: Janne Grunau <j@...nau.net>
> Co-developed-by: Hector Martin <marcan@...can.st>
> Signed-off-by: Hector Martin <marcan@...can.st>
> Signed-off-by: Sven Peter <sven@...nel.org>
> ---
>  MAINTAINERS                |    1 +
>  drivers/phy/Kconfig        |    1 +
>  drivers/phy/Makefile       |    1 +
>  drivers/phy/apple/Kconfig  |   14 +
>  drivers/phy/apple/Makefile |    4 +
>  drivers/phy/apple/atc.c    | 2214 ++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 2235 insertions(+)

[...]

> diff --git a/drivers/phy/apple/atc.c b/drivers/phy/apple/atc.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..9213485234873fcaafeb1d1d9de3ddf07767d552
> --- /dev/null
> +++ b/drivers/phy/apple/atc.c
> @@ -0,0 +1,2214 @@

[...]

> +static int atcphy_load_tunables(struct apple_atcphy *atcphy)
> +{
> +	int ret;
> +	struct {
> +		const char *dt_name;
> +		struct apple_tunable **tunable;
> +	} tunables[] = {
> +		{ "apple,tunable-fuses", &atcphy->tunables.fuses },
> +		{ "apple,tunable-axi2af", &atcphy->tunables.axi2af },
> +		{ "apple,tunable-common", &atcphy->tunables.common },
> +		{ "apple,tunable-lane0-usb", &atcphy->tunables.lane_usb3[0] },
> +		{ "apple,tunable-lane1-usb", &atcphy->tunables.lane_usb3[1] },
> +		{ "apple,tunable-lane0-cio", &atcphy->tunables.lane_usb4[0] },
> +		{ "apple,tunable-lane1-cio", &atcphy->tunables.lane_usb4[1] },
> +		{ "apple,tunable-lane0-dp", &atcphy->tunables.lane_displayport[0] },
> +		{ "apple,tunable-lane1-dp", &atcphy->tunables.lane_displayport[1] },
> +	};
> +
> +	for (int i = 0; i < ARRAY_SIZE(tunables); i++) {
> +		*tunables[i].tunable =
> +			devm_apple_tunable_parse(atcphy->dev, atcphy->np, tunables[i].dt_name);
> +		if (IS_ERR(tunables[i].tunable)) {
> +			dev_err(atcphy->dev, "Failed to read tunable %s: %ld\n",
> +				tunables[i].dt_name, PTR_ERR(tunables[i].tunable));
> +			return ret;

ret is unitialized here, could be `return PTR_ERR(tunables[i].tunable);`
instead

Janne