| 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
| ||
|
Message-ID: <4d46f4997e1c73aa5b10bdb61dec6760c30fa2f1.camel@pengutronix.de>
Date: Mon, 08 Sep 2025 17:04:53 +0200
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Sven Peter <sven@...nel.org>, 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>, Janne Grunau <j@...nau.net>, 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>, Frank Li <Frank.Li@....com>, Ran Wang
<ran.wang_1@....com>, Peter Chen <peter.chen@....com>
Cc: 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
Hi Sven,
On Sa, 2025-09-06 at 15:43 +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 @@
[...]
> +struct apple_atcphy {
[...]
> + struct reset_controller_dev rcdev;
> + struct typec_switch *sw;
> + struct typec_mux *mux;
> +
> + struct mutex lock;
Consider documenting the purpose of this lock to make 'checkpatch.pl --
strict' happy.
[...]
> +static int atcphy_dp_configure(struct apple_atcphy *atcphy, enum atcphy_dp_link_rate lr)
> +{
> + const struct atcphy_dp_link_rate_configuration *cfg = &dp_lr_config[lr];
> + const struct atcphy_mode_configuration *mode_cfg;
> + int ret;
> + u32 reg;
This function does a lot of register read-modify-writes.
lockdep_assert_held(&atcphy->lock);
maybe? Or you could move the 'guard(mutex)(&atcphy->lock);' from
atcphy_dpphy_configure() in here.
[...]
> +static int atcphy_dpphy_configure(struct phy *phy, union phy_configure_opts *opts_)
> +{
> + struct phy_configure_opts_dp *opts = &opts_->dp;
> + struct apple_atcphy *atcphy = phy_get_drvdata(phy);
> + enum atcphy_dp_link_rate link_rate;
> +
> + if (opts->set_voltages)
> + return -EINVAL;
> + if (opts->set_lanes)
> + return -EINVAL;
> +
> + if (opts->set_rate) {
> + guard(mutex)(&atcphy->lock);
> +
> + switch (opts->link_rate) {
> + case 1620:
> + link_rate = ATCPHY_DP_LINK_RATE_RBR;
> + break;
> + case 2700:
> + link_rate = ATCPHY_DP_LINK_RATE_HBR;
> + break;
> + case 5400:
> + link_rate = ATCPHY_DP_LINK_RATE_HBR2;
> + break;
> + case 8100:
> + link_rate = ATCPHY_DP_LINK_RATE_HBR3;
> + break;
> + case 0:
> + return 0;
> + default:
> + dev_err(atcphy->dev, "Unsupported link rate: %d\n", opts->link_rate);
> + return -EINVAL;
> + }
Seems to me like this switch(){} doesn't need to be under guard.
> +
> + return atcphy_dp_configure(atcphy, link_rate);
> + }
> +
> + return 0;
> +}
[...]
> +static void _atcphy_dwc3_reset_assert(struct apple_atcphy *atcphy)
> +{
> + lockdep_assert_held(&atcphy->lock);
> +
> + clear32(atcphy->regs.pipehandler + PIPEHANDLER_AON_GEN, PIPEHANDLER_AON_GEN_DWC3_RESET_N);
> + set32(atcphy->regs.pipehandler + PIPEHANDLER_AON_GEN,
> + PIPEHANDLER_AON_GEN_DWC3_FORCE_CLAMP_EN);
> +}
> +
> +static int atcphy_dwc3_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct apple_atcphy *atcphy = container_of(rcdev, struct apple_atcphy, rcdev);
> + int ret;
> +
> + guard(mutex)(&atcphy->lock);
> +
> + _atcphy_dwc3_reset_assert(atcphy);
> +
> + if (atcphy->pipehandler_up) {
> + ret = atcphy_configure_pipehandler_dummy(atcphy);
> + if (ret)
> + dev_warn(atcphy->dev, "Failed to switch PIPE to dummy: %d\n", ret);
> + else
> + atcphy->pipehandler_up = false;
> + }
> +
> + atcphy_usb2_power_off(atcphy);
> +
> + atcphy->dwc3_running = false;
> +
> + return 0;
> +}
> +
> +static int atcphy_dwc3_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct apple_atcphy *atcphy = container_of(rcdev, struct apple_atcphy, rcdev);
> +
> + guard(mutex)(&atcphy->lock);
> +
> + clear32(atcphy->regs.pipehandler + PIPEHANDLER_AON_GEN,
> + PIPEHANDLER_AON_GEN_DWC3_FORCE_CLAMP_EN);
> + set32(atcphy->regs.pipehandler + PIPEHANDLER_AON_GEN, PIPEHANDLER_AON_GEN_DWC3_RESET_N);
> +
> + atcphy->dwc3_running = true;
> +
> + return 0;
> +}
> +
> +const struct reset_control_ops atcphy_dwc3_reset_ops = {
> + .assert = atcphy_dwc3_reset_assert,
> + .deassert = atcphy_dwc3_reset_deassert,
> +};
> +
> +static int atcphy_reset_xlate(struct reset_controller_dev *rcdev,
> + const struct of_phandle_args *reset_spec)
> +{
> + return 0;
> +}
> +
> +static int atcphy_probe_rcdev(struct apple_atcphy *atcphy)
> +{
> + atcphy->rcdev.owner = THIS_MODULE;
> + atcphy->rcdev.nr_resets = 1;
> + atcphy->rcdev.ops = &atcphy_dwc3_reset_ops;
> + atcphy->rcdev.of_node = atcphy->dev->of_node;
> + atcphy->rcdev.of_reset_n_cells = 0;
> + atcphy->rcdev.of_xlate = atcphy_reset_xlate;
> +
> + return devm_reset_controller_register(atcphy->dev, &atcphy->rcdev);
> +}
For the reset controller part,
Reviewed-by: Philipp Zabel <p.zabel@...gutronix.de>
regards
Philipp
Powered by blists - more mailing lists