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@jannau.net>
Signed-off-by: Janne Grunau <j@jannau.net>
Co-developed-by: Hector Martin <redacted>
Signed-off-by: Hector Martin <redacted>
Signed-off-by: Sven Peter <sven@kernel.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(+)

[...]

[...]

+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@pengutronix.de>

regards
Philipp