Re: [PATCH 3/4] phy: apple: atc: Add initial M3 series support

[Sven Peter <sven@kernel.org>]

Hi,


On 11/8/26 16:43, Sasha Finkelstein wrote:
> Adds initial support for the new t8122 phy. USB/TB only for now, DP
> will fail to negotiate
>
> Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
> ---

So I wonder if it's time to split this into multiple files now.
Maybe something like atc.c that does the core stuff (probing, 
phy/reset/etc. interfacing, mode transitions, tunables, etc.) and then 
atc-t8103.c and atc-t8112.c for the hardware specific parts since 
especially DP seems to be quite different between the two generations now.


>   drivers/phy/apple/atc.c | 382 +++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 297 insertions(+), 85 deletions(-)

[...]

> diff --git a/drivers/phy/apple/atc.c b/drivers/phy/apple/atc.c
> index 45f1280a6faa..256a08499646 100644
> --- a/drivers/phy/apple/atc.c
> +++ b/drivers/phy/apple/atc.c
> @@ -100,9 +100,15 @@
>   #define AUSPLL_FREQ_CFG 0x2224
>   #define AUSPLL_FREQ_REFCLK GENMASK(1, 0)
>   
> +#define AUS_COMMON_SHIM_BLK_BIAS_REG 0x0a00
> +#define AUS_COMMON_SHIM_BLK_BIAS_REG_BGBIAS_OV BIT(1)
> +
>   #define AUS_COMMON_SHIM_BLK_VREG 0x0a04
>   #define AUS_VREG_TRIM GENMASK(6, 2)
>   
[...]
>   	atcphy->mode = mode;
>   
> @@ -2201,6 +2387,7 @@ static int atcphy_probe_mux(struct apple_atcphy *atcphy)
>   
>   static int atcphy_load_tunables(struct apple_atcphy *atcphy)
>   {
> +	size_t tunable_count;
>   	struct {
>   		const char *dt_name;
>   		struct apple_tunable **tunable;
> @@ -2216,8 +2403,14 @@ static int atcphy_load_tunables(struct apple_atcphy *atcphy)
>   		{ "apple,tunable-lane0-dp", &atcphy->tunables.lane_dp[0], atcphy->res.core },
>   		{ "apple,tunable-lane1-dp", &atcphy->tunables.lane_dp[1], atcphy->res.core },
>   	};
> -
> -	for (int i = 0; i < ARRAY_SIZE(tunables); i++) {
> +	if (atcphy->hw->gen == ATCPHY_GENERATION_T8122) {
> +		tunable_count = ARRAY_SIZE(tunables) - 2;
> +		atcphy->tunables.lane_dp[0] = NULL;
> +		atcphy->tunables.lane_dp[1] = NULL;
> +	} else
> +		tunable_count = ARRAY_SIZE(tunables);

nit: I think else { .. } looks nicer when the if already has braces even 
though it's just a single line.



Sven