* [PATCH v7 4/7] phy: can-transceiver: drop temporary helper getting optional mux-state
From: Josua Mayer @ 2026-01-28 14:46 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Vinod Koul, Neil Armstrong,
Peter Rosin, Aaro Koskinen, Andreas Kemnade, Kevin Hilman,
Roger Quadros, Tony Lindgren, Janusz Krzysztofik, Vignesh R,
Andi Shyti, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Magnus Damm, Wolfram Sang
Cc: Yazan Shhady, Jon Nettleton, Mikhail Anikin, linux-can, linux-phy,
linux-kernel, linux-omap, linux-i2c, linux-mmc, devicetree,
linux-renesas-soc, Josua Mayer
In-Reply-To: <20260128-rz-sdio-mux-v7-0-92ebb6da0df8@solid-run.com>
Multiplexer subsystem has now added helpers for getting managed optional
mux-state.
Switch to the new devm_mux_state_get_optional helper.
This change is only compile-tested.
Acked-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
drivers/phy/phy-can-transceiver.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c
index 81591d247128..2b52e47f247a 100644
--- a/drivers/phy/phy-can-transceiver.c
+++ b/drivers/phy/phy-can-transceiver.c
@@ -126,16 +126,6 @@ static const struct of_device_id can_transceiver_phy_ids[] = {
};
MODULE_DEVICE_TABLE(of, can_transceiver_phy_ids);
-/* Temporary wrapper until the multiplexer subsystem supports optional muxes */
-static inline struct mux_state *
-temp_devm_mux_state_get_optional(struct device *dev, const char *mux_name)
-{
- if (!of_property_present(dev->of_node, "mux-states"))
- return NULL;
-
- return devm_mux_state_get(dev, mux_name);
-}
-
static struct phy *can_transceiver_phy_xlate(struct device *dev,
const struct of_phandle_args *args)
{
@@ -183,7 +173,7 @@ static int can_transceiver_phy_probe(struct platform_device *pdev)
priv->num_ch = num_ch;
platform_set_drvdata(pdev, priv);
- mux_state = temp_devm_mux_state_get_optional(dev, NULL);
+ mux_state = devm_mux_state_get_optional(dev, NULL);
if (IS_ERR(mux_state))
return PTR_ERR(mux_state);
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v7 5/7] i2c: omap: switch to new generic helper for getting selected mux-state
From: Josua Mayer @ 2026-01-28 14:46 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Vinod Koul, Neil Armstrong,
Peter Rosin, Aaro Koskinen, Andreas Kemnade, Kevin Hilman,
Roger Quadros, Tony Lindgren, Janusz Krzysztofik, Vignesh R,
Andi Shyti, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Magnus Damm, Wolfram Sang
Cc: Yazan Shhady, Jon Nettleton, Mikhail Anikin, linux-can, linux-phy,
linux-kernel, linux-omap, linux-i2c, linux-mmc, devicetree,
linux-renesas-soc, Josua Mayer
In-Reply-To: <20260128-rz-sdio-mux-v7-0-92ebb6da0df8@solid-run.com>
Multiplexer subsystem has added generic helper functions for getting an
already selected mux-state object.
Replace existing logic in probe with the equivalent helper function.
There is a functional difference in that the mux is now automatically
deselected on release, replacing the explicit mux_state_deselect call.
This change is only compile-tested.
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Andreas Kemnade <andreas@kemnade.info>
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
drivers/i2c/busses/i2c-omap.c | 24 +++++-------------------
1 file changed, 5 insertions(+), 19 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index d9f590f0c384..f02d294db42a 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1453,27 +1453,16 @@ omap_i2c_probe(struct platform_device *pdev)
(1000 * omap->speed / 8);
}
- if (of_property_present(node, "mux-states")) {
- struct mux_state *mux_state;
-
- mux_state = devm_mux_state_get(&pdev->dev, NULL);
- if (IS_ERR(mux_state)) {
- r = PTR_ERR(mux_state);
- dev_dbg(&pdev->dev, "failed to get I2C mux: %d\n", r);
- goto err_put_pm;
- }
- omap->mux_state = mux_state;
- r = mux_state_select(omap->mux_state);
- if (r) {
- dev_err(&pdev->dev, "failed to select I2C mux: %d\n", r);
- goto err_put_pm;
- }
+ omap->mux_state = devm_mux_state_get_optional_selected(&pdev->dev, NULL);
+ if (IS_ERR(omap->mux_state)) {
+ r = PTR_ERR(omap->mux_state);
+ goto err_put_pm;
}
/* reset ASAP, clearing any IRQs */
r = omap_i2c_init(omap);
if (r)
- goto err_mux_state_deselect;
+ goto err_put_pm;
if (omap->rev < OMAP_I2C_OMAP1_REV_2)
r = devm_request_irq(&pdev->dev, omap->irq, omap_i2c_omap1_isr,
@@ -1515,9 +1504,6 @@ omap_i2c_probe(struct platform_device *pdev)
err_unuse_clocks:
omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, 0);
-err_mux_state_deselect:
- if (omap->mux_state)
- mux_state_deselect(omap->mux_state);
err_put_pm:
pm_runtime_put_sync(omap->dev);
err_disable_pm:
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v7 6/7] dt-bindings: mmc: renesas,sdhi: Add mux-states property
From: Josua Mayer @ 2026-01-28 14:46 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Vinod Koul, Neil Armstrong,
Peter Rosin, Aaro Koskinen, Andreas Kemnade, Kevin Hilman,
Roger Quadros, Tony Lindgren, Janusz Krzysztofik, Vignesh R,
Andi Shyti, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Magnus Damm, Wolfram Sang
Cc: Yazan Shhady, Jon Nettleton, Mikhail Anikin, linux-can, linux-phy,
linux-kernel, linux-omap, linux-i2c, linux-mmc, devicetree,
linux-renesas-soc, Josua Mayer
In-Reply-To: <20260128-rz-sdio-mux-v7-0-92ebb6da0df8@solid-run.com>
Add mux controller support for data or control lines that are muxed
between a host and multiple cards.
There are several devices supporting a choice of eMMC or SD on a single
board by both dip switch and gpio, e.g. Renesas RZ/G2L SMARC SoM and
SolidRun RZ/G2L SoM.
In-tree dts for the Renesas boards currently rely on preprocessor macros
and gpio hogs to describe the respective cards.
By adding mux-states property to sdhi controller description, boards can
correctly describe the mux that already exists in hardware - and drivers
can coordinate between mux selection and probing for cards.
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
index c754ea71f51f..64fac0d11329 100644
--- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
+++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
@@ -106,6 +106,11 @@ properties:
iommus:
maxItems: 1
+ mux-states:
+ description:
+ mux controller node to route the SD/SDIO/eMMC signals from SoC to cards.
+ maxItems: 1
+
power-domains:
maxItems: 1
@@ -275,6 +280,7 @@ examples:
max-frequency = <195000000>;
power-domains = <&sysc R8A7790_PD_ALWAYS_ON>;
resets = <&cpg 314>;
+ mux-states = <&mux 0>;
};
sdhi1: mmc@ee120000 {
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v7 7/7] mmc: host: renesas_sdhi_core: support selecting an optional mux
From: Josua Mayer @ 2026-01-28 14:46 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Vinod Koul, Neil Armstrong,
Peter Rosin, Aaro Koskinen, Andreas Kemnade, Kevin Hilman,
Roger Quadros, Tony Lindgren, Janusz Krzysztofik, Vignesh R,
Andi Shyti, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Magnus Damm, Wolfram Sang
Cc: Yazan Shhady, Jon Nettleton, Mikhail Anikin, linux-can, linux-phy,
linux-kernel, linux-omap, linux-i2c, linux-mmc, devicetree,
linux-renesas-soc, Josua Mayer
In-Reply-To: <20260128-rz-sdio-mux-v7-0-92ebb6da0df8@solid-run.com>
Some hardware designs route data or control signals through a mux to
support multiple devices on a single sdhi controller.
In particular SolidRun RZ/G2L/G2LC/V2L System on Module use a mux for
switching between soldered eMMC and an optional microSD on a carrier
board, e.g. for development or provisioning.
SD/SDIO/eMMC are not well suited for runtime switching between different
cards, however boot-time selection is possible and useful - in
particular considering dt overlays.
Add support for an optional SD/SDIO/eMMC mux defined in dt, and select
it during probe.
Similar functionality already exists in other places, e.g. i2c-omap.
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
drivers/mmc/host/renesas_sdhi_core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 2a310a145785..f9ec78d699f4 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -26,6 +26,7 @@
#include <linux/mmc/mmc.h>
#include <linux/mmc/slot-gpio.h>
#include <linux/module.h>
+#include <linux/mux/consumer.h>
#include <linux/pinctrl/consumer.h>
#include <linux/pinctrl/pinctrl-state.h>
#include <linux/platform_data/tmio.h>
@@ -1062,6 +1063,7 @@ int renesas_sdhi_probe(struct platform_device *pdev,
struct regulator_dev *rdev;
struct renesas_sdhi_dma *dma_priv;
struct device *dev = &pdev->dev;
+ struct mux_state *mux_state;
struct tmio_mmc_host *host;
struct renesas_sdhi *priv;
int num_irqs, irq, ret, i;
@@ -1116,6 +1118,10 @@ int renesas_sdhi_probe(struct platform_device *pdev,
"state_uhs");
}
+ mux_state = devm_mux_state_get_optional_selected(&pdev->dev, NULL);
+ if (IS_ERR(mux_state))
+ return PTR_ERR(mux_state);
+
host = tmio_mmc_host_alloc(pdev, mmc_data);
if (IS_ERR(host))
return PTR_ERR(host);
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH v6 3/7] mux: add help text for MULTIPLEXER config option
From: Josua Mayer @ 2026-01-28 14:47 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Marc Kleine-Budde, Vincent Mailhol, Vinod Koul, Neil Armstrong,
Peter Rosin, Aaro Koskinen, Andreas Kemnade, Kevin Hilman,
Roger Quadros, Tony Lindgren, Janusz Krzysztofik, Vignesh R,
Andi Shyti, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Magnus Damm, Wolfram Sang,
Yazan Shhady, Jon Nettleton, Mikhail Anikin,
linux-can@vger.kernel.org, linux-phy@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
linux-i2c@vger.kernel.org, linux-mmc@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <CAMuHMdX_rKgfusHP5qVny8OZufU6VAiA6sqg1LP3T2jikSz7yQ@mail.gmail.com>
On 21/01/2026 12:59, Geert Uytterhoeven wrote:
> Hi Josua,
>
> On Wed, 21 Jan 2026 at 11:02, Josua Mayer <josua@solid-run.com> wrote:
>> Add help text for CONFIG_MULTIPLEXER to allow enabling this option
>> through the kernel configuration without explicit "select" driver
>> dependencies.
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
> Thanks for your patch!
>
>> --- a/drivers/mux/Kconfig
>> +++ b/drivers/mux/Kconfig
>> @@ -5,6 +5,14 @@
>>
>> config MULTIPLEXER
>> tristate
>> + help
>> + Generic Multiplexer Support.
>> +
>> + This framework is designed to abstract multiplexer handling for
>> + devices via various GPIO-, MMIO/Regmap or specific multiplexer
>> + controller chips.
>> +
>> + If unsure, say no.
>>
>> menu "Multiplexer drivers"
>> depends on MULTIPLEXER
>>
> Unfortunately it doesn't work like that. As the tristate has no prompt
> specified, the user will never be asked about this.
> You should use something like below:
>
> --- a/drivers/mux/Kconfig
> +++ b/drivers/mux/Kconfig
> @@ -4,10 +4,8 @@
> #
>
> config MULTIPLEXER
> - tristate
> + tristate "Generic Multiplexer Support" if COMPILE_TEST
> help
> - Generic Multiplexer Support.
> -
> This framework is designed to abstract multiplexer handling for
> devices via various GPIO-, MMIO/Regmap or specific multiplexer
> controller chips.
Thank you, changed with v7
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH next] phy: google: fix build dependency for Google Tensor USB PHY
From: André Draszik @ 2026-01-28 19:57 UTC (permalink / raw)
To: Roy Luo
Cc: Vinod Koul, Neil Armstrong, Peter Griffin, Tudor Ambarus,
Joy Chakraborty, Naveen Kumar, linux-phy, linux-kernel,
linux-arm-kernel, linux-samsung-soc, kernel test robot
In-Reply-To: <CA+zupgztBHTJwNPEOfXjdw9TpcpiVGO=hdFBYk3PUo-0Vf95mg@mail.gmail.com>
Hi Roy,
On Mon, 2026-01-26 at 18:34 -0800, Roy Luo wrote:
> the wrong example. Let's look at this build error scenario with
> "depends on TYPEC || COMPILE_TEST":
> Given COMPILE_TEST=y and CONFIG_TYPEC=m, the
> "depends on" would be evaluated as max(y,m) = y.
> Hence, CONFIG_PHY_GOOGLE_USB is allowed to be
> set to y, m or n. When it's set to y, the exact build error
> would be introduced. I also tested this locally and saw
> the build error.
Thanks for trying Roy. In that case, it should be written as
depends on TYPEC || (TYPEC=n && COMPILE_TEST)
Cheers,
Andre'
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH] phy: PHY_GOOGLE_USB should depend on ARCH_GOOGLE
From: André Draszik @ 2026-01-28 20:03 UTC (permalink / raw)
To: Geert Uytterhoeven, Vinod Koul, Neil Armstrong, Peter Griffin,
Tudor Ambarus, Naveen Kumar, Roy Luo, Joy Chakraborty,
Arnd Bergmann
Cc: linux-phy, linux-arm-kernel, linux-samsung-soc, linux-kernel
In-Reply-To: <0caa3c449b0c3d64944da3f1003d9389bdc13f98.1769541083.git.geert+renesas@glider.be>
Hi Geert,
On Tue, 2026-01-27 at 20:12 +0100, Geert Uytterhoeven wrote:
> The Google Tensor SoC USB PHY is only present on Google Tensor G5
> (Laguna) SoCs. Hence add a dependency on ARCH_GOOGLE, to prevent asking
> the user about this driver when configuring a kernel without Google
> Tensor SoC support.
>
> Fixes: cbce66669c82ee9a ("phy: Add Google Tensor SoC USB PHY driver")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> ARCH_GOOGLE is not yet upstream, but that doesn't hurt.
I can see your point, and have no objection, but for context, Greg
had asked to not add dependencies on a config option that might or
might not get added in the future - ARCH_GOOGLE:
https://lore.kernel.org/all/2025121728-reliably-crabgrass-2601@gregkh/
Maybe an alternative would be to make it default=n, and change that
default once ARCH_GOOGLE gets added?
Cheers,
Andre'
> ---
> drivers/phy/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 142e7b0ef2efb920..3ceda9a20d571038 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -49,6 +49,7 @@ config GENERIC_PHY_MIPI_DPHY
>
> config PHY_GOOGLE_USB
> tristate "Google Tensor SoC USB PHY driver"
> + depends on ARCH_GOOGLE || COMPILE_TEST
> select GENERIC_PHY
> help
> Enable support for the USB PHY on Google Tensor SoCs, starting with
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH next] phy: google: fix build dependency for Google Tensor USB PHY
From: Roy Luo @ 2026-01-28 21:22 UTC (permalink / raw)
To: André Draszik
Cc: Vinod Koul, Neil Armstrong, Peter Griffin, Tudor Ambarus,
Joy Chakraborty, Naveen Kumar, linux-phy, linux-kernel,
linux-arm-kernel, linux-samsung-soc, kernel test robot
In-Reply-To: <f13cf27da2d1d7d8a54448391c16a8eae349e8a5.camel@linaro.org>
On Wed, Jan 28, 2026 at 11:57 AM André Draszik <andre.draszik@linaro.org> wrote:
>
> Hi Roy,
>
> On Mon, 2026-01-26 at 18:34 -0800, Roy Luo wrote:
> > the wrong example. Let's look at this build error scenario with
> > "depends on TYPEC || COMPILE_TEST":
> > Given COMPILE_TEST=y and CONFIG_TYPEC=m, the
> > "depends on" would be evaluated as max(y,m) = y.
> > Hence, CONFIG_PHY_GOOGLE_USB is allowed to be
> > set to y, m or n. When it's set to y, the exact build error
> > would be introduced. I also tested this locally and saw
> > the build error.
>
> Thanks for trying Roy. In that case, it should be written as
>
> depends on TYPEC || (TYPEC=n && COMPILE_TEST)
>
>
> Cheers,
> Andre'
Hi Andre',
Thank you for the clarification. I noticed similar expressions
like "(CONFIG_X=n && COMPILE_TEST)" in other drivers,
though it seems less common, which is why I was initially hesitant.
I will send out v2 now with this change included.
Thanks,
Roy
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH next v2] phy: google: fix build dependency for Google Tensor USB PHY
From: Roy Luo @ 2026-01-28 21:22 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Peter Griffin, André Draszik,
Tudor Ambarus, Joy Chakraborty, Naveen Kumar
Cc: linux-phy, linux-kernel, linux-arm-kernel, linux-samsung-soc,
kernel test robot, Roy Luo
The Google Tensor USB PHY driver uses the Type-C switch framework to
handle orientation changes. However, the Kconfig did not specify a
dependency on the TYPEC framework, leading to undefined reference
errors when building for architectures or configurations where
CONFIG_TYPEC is configured as a module while CONFIG_PHY_GOOGLE_USB
is configured as built-in.
Add 'depends on TYPEC' to the PHY_GOOGLE_USB entry to ensure all
required symbols are available during linking, and 'COMPILE_TEST'
to expand test coverage.
Fixes: cbce66669c82 ("phy: Add Google Tensor SoC USB PHY driver")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202601210825.ELrpQeED-lkp@intel.com/
Reviewed-by: Peter Griffin <peter.griffin@linaro.org>
Signed-off-by: Roy Luo <royluo@google.com>
---
Changes in v2:
- Add '(TYPEC=n && COMPILE_TEST)' to build dependency to expand test
coverage as suggested by André Draszik <andre.draszik@linaro.org>
- Keep Peter's Reviewed-by tag because the change suggested by André
is a trivial improvement.
- Link to v1: https://lore.kernel.org/r/20260121-next-v1-1-c18068b091b9@google.com
---
drivers/phy/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 142e7b0ef2efb9209781800ee47b820a91b115ae..29953c911deda5fd3c43aef6da28d96954a22f8d 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -49,6 +49,7 @@ config GENERIC_PHY_MIPI_DPHY
config PHY_GOOGLE_USB
tristate "Google Tensor SoC USB PHY driver"
+ depends on TYPEC || (TYPEC=n && COMPILE_TEST)
select GENERIC_PHY
help
Enable support for the USB PHY on Google Tensor SoCs, starting with
---
base-commit: 8bb92fd7a04077925c8330f46a6ab44c80ca59f4
change-id: 20260121-next-b949189cacf4
Best regards,
--
Roy Luo <royluo@google.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH next v2] phy: google: fix build dependency for Google Tensor USB PHY
From: André Draszik @ 2026-01-28 22:34 UTC (permalink / raw)
To: Roy Luo, Vinod Koul, Neil Armstrong, Peter Griffin, Tudor Ambarus,
Joy Chakraborty, Naveen Kumar
Cc: linux-phy, linux-kernel, linux-arm-kernel, linux-samsung-soc,
kernel test robot
In-Reply-To: <20260128-next-v2-1-624bdae8e6d0@google.com>
On Wed, 2026-01-28 at 21:22 +0000, Roy Luo wrote:
> The Google Tensor USB PHY driver uses the Type-C switch framework to
> handle orientation changes. However, the Kconfig did not specify a
> dependency on the TYPEC framework, leading to undefined reference
> errors when building for architectures or configurations where
> CONFIG_TYPEC is configured as a module while CONFIG_PHY_GOOGLE_USB
> is configured as built-in.
>
> Add 'depends on TYPEC' to the PHY_GOOGLE_USB entry to ensure all
> required symbols are available during linking, and 'COMPILE_TEST'
> to expand test coverage.
>
> Fixes: cbce66669c82 ("phy: Add Google Tensor SoC USB PHY driver")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202601210825.ELrpQeED-lkp@intel.com/
> Reviewed-by: Peter Griffin <peter.griffin@linaro.org>
> Signed-off-by: Roy Luo <royluo@google.com>
> ---
> Changes in v2:
> - Add '(TYPEC=n && COMPILE_TEST)' to build dependency to expand test
> coverage as suggested by André Draszik <andre.draszik@linaro.org>
> - Keep Peter's Reviewed-by tag because the change suggested by André
> is a trivial improvement.
> - Link to v1: https://lore.kernel.org/r/20260121-next-v1-1-c18068b091b9@google.com
> ---
> drivers/phy/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: André Draszik <andre.draszik@linaro.org>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH net-next v2 00/14] net: stmmac: SerDes, PCS, BASE-X, and inband goodies
From: Mohd Ayaan Anwar @ 2026-01-29 7:27 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Andrew Lunn, Heiner Kallweit, Alexandre Torgue, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Konrad Dybcio,
linux-arm-kernel, linux-arm-msm, linux-phy, linux-stm32,
Maxime Coquelin, Neil Armstrong, netdev, Paolo Abeni, Vinod Koul
In-Reply-To: <aXjdAZeUl2Dsu4mE@shell.armlinux.org.uk>
On Tue, Jan 27, 2026 at 03:42:57PM +0000, Russell King (Oracle) wrote:
> On Tue, Jan 27, 2026 at 08:27:50PM +0530, Mohd Ayaan Anwar wrote:
> > During pcs_init, BIT(8) of GMAC_AN_STATUS is 0:
> > [ 7.985913] [DBG] GMAC_AN_STATUS = 8
>
> Hmm. This means that your hardware doesn't support TBI or RTBI modes
> (which is what the dwmac core uses for BASE-X) and what it's actually
> offering is an up-clocked Cisco SGMII implementation.
>
> With AN disabled, this is compatible with 2500BASE-X implementations
> that do not require AN.
Yes, this hardware implements what some vendors call OCSGMII (i.e.
2500BASE-X without in-band signalling).
> > I also tried enabling comma detect during dwmac_integrated_pcs_config,
> > but I am still seeing the Tx timeouts. I remember that when I had
> > tested the patches in October (without the SerDes driver changes),
> > the link state used to flap, but the data path became functional
> > after the link stabilized.
>
> I wonder whether the SerDes needs to be calibrated after the link has
> come up and the clocks configured. phy_calibdate() will re-invoke the
> programming of the SerDes, so you could try adding that at the bottom
> of ethqos_configure_sgmii():
>
> return phy_calibrate(priv->plat->serdes);
>
> which will do the calibration after the clocks have been set, and see
> whether that stabilises the link.
Somehow booting up with a 2.5G link is more unstable after adding this.
Behaviour at 1G is pretty similar. I kept comma detection disabled
during these tests.
I was thinking about the "good" sequence (i.e., the current net-next
tree):
-> phylink_up
-> mac_link_up
-> fix_mac_speed
-> SerDes configured via phy_set_speed
-> stmmac_pcs_ctrl_ane(priv, enable/disable, 0)
Please let me know if you want me to try any other experiments.
Maybe I need to do some more testing after playing around with the
sequence followed by this series?
Ayaan
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH] phy: PHY_GOOGLE_USB should depend on ARCH_GOOGLE
From: Geert Uytterhoeven @ 2026-01-29 8:32 UTC (permalink / raw)
To: André Draszik
Cc: Geert Uytterhoeven, Vinod Koul, Neil Armstrong, Peter Griffin,
Tudor Ambarus, Naveen Kumar, Roy Luo, Joy Chakraborty,
Arnd Bergmann, linux-phy, linux-arm-kernel, linux-samsung-soc,
linux-kernel
In-Reply-To: <e955868c06547ac62d34d594d45dd07aa19e361b.camel@linaro.org>
Hi André,
On Wed, 28 Jan 2026 at 21:03, André Draszik <andre.draszik@linaro.org> wrote:
> On Tue, 2026-01-27 at 20:12 +0100, Geert Uytterhoeven wrote:
> > The Google Tensor SoC USB PHY is only present on Google Tensor G5
> > (Laguna) SoCs. Hence add a dependency on ARCH_GOOGLE, to prevent asking
> > the user about this driver when configuring a kernel without Google
> > Tensor SoC support.
> >
> > Fixes: cbce66669c82ee9a ("phy: Add Google Tensor SoC USB PHY driver")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > ARCH_GOOGLE is not yet upstream, but that doesn't hurt.
>
> I can see your point, and have no objection, but for context, Greg
> had asked to not add dependencies on a config option that might or
> might not get added in the future - ARCH_GOOGLE:
The exact name may indeed still be under discussion. But we all know
there _will_ be a config option (unless the platform is abandoned before
upstreaming, and this driver will be removed again anyway ;-).
> Maybe an alternative would be to make it default=n, and change that
> default once ARCH_GOOGLE gets added?
It already defaults to disabled, as "default=n" is the default.
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -49,6 +49,7 @@ config GENERIC_PHY_MIPI_DPHY
> >
> > config PHY_GOOGLE_USB
> > tristate "Google Tensor SoC USB PHY driver"
> > + depends on ARCH_GOOGLE || COMPILE_TEST
> > select GENERIC_PHY
> > help
> > Enable support for the USB PHY on Google Tensor SoCs, starting with
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/4] phy: s32g: Add serdes subsystem phy
From: Simon Horman @ 2026-01-29 9:54 UTC (permalink / raw)
To: Vincent Guittot
Cc: vkoul, neil.armstrong, krzk+dt, conor+dt, ciprianmarian.costea,
s32, p.zabel, linux, ghennadi.procopciuc, bogdan-gabriel.roman,
Ionut.Vicovan, alexandru-catalin.ionita, linux-phy, devicetree,
linux-kernel, linux-arm-kernel, netdev, Frank.li
In-Reply-To: <20260126092159.815968-3-vincent.guittot@linaro.org>
On Mon, Jan 26, 2026 at 10:21:57AM +0100, Vincent Guittot wrote:
...
> diff --git a/drivers/phy/freescale/phy-nxp-s32g-serdes.c b/drivers/phy/freescale/phy-nxp-s32g-serdes.c
> new file mode 100644
> index 000000000000..8336c868c8dc
> --- /dev/null
> +++ b/drivers/phy/freescale/phy-nxp-s32g-serdes.c
> @@ -0,0 +1,569 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * SerDes driver for S32G SoCs
> + *
> + * Copyright 2021-2026 NXP
> + */
> +
> +#include <dt-bindings/phy/phy.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
Hi Vincent, all,
I think that you also need:
#include <linux/iopoll.h>
So that read_poll_timeout() is declared.
Else this patch causes a transient build failure
(for x86_64 allmodconfig)
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/processor.h>
> +#include <linux/reset.h>
> +#include <linux/units.h>
...
> +static int s32g_serdes_phy_set_mode_ext(struct phy *p,
> + enum phy_mode mode, int submode)
> +{
> + struct s32g_serdes *serdes = phy_get_drvdata(p);
> +
> + if (mode == PHY_MODE_PCIE)
> + return -EINVAL;
This is part of an AI Generated review.
I have looked over it and I think it warrants investigation.
For information on how to reproduce locally, as I did, please see [1].
[1] https://netdev-ai.bots.linux.dev/ai-local.html
This returns error if mode IS PHY_MODE_PCIE, but this is a PCIe PHY!
This looks like a typo. Should be !=.
> +
> + if (!is_pcie_phy_mode_valid(submode))
> + return -EINVAL;
> +
> + /*
> + * Do not configure SRIS or CRSS PHY MODE in conjunction
> + * with any SGMII mode on the same SerDes subsystem
> + */
> + if ((submode == CRSS || submode == SRIS) &&
> + serdes->ctrl.ss_mode != 0)
> + return -EINVAL;
> +
> + /*
> + * Internal reference clock cannot be used with either Common clock
> + * or Spread spectrum, leaving only SRNSS
> + */
> + if (submode != SRNS && !serdes->ctrl.ext_clk)
> + return -EINVAL;
> +
> + serdes->pcie.phy_mode = submode;
The AI review also suggested that it may be unsafe
to set the submode after s32g_serdes_phy_power_on()
has been called. And that there is nothing preventing that.
TBH, I am unsure if either of those statements are true.
But it seems worth validating with you.
> +
> + return 0;
> +}
...
> +static int s32g_serdes_get_ctrl_resources(struct platform_device *pdev, struct s32g_serdes *serdes)
> +{
> + struct s32g_serdes_ctrl *ctrl = &serdes->ctrl;
> + struct device *dev = &pdev->dev;
> + int ret, idx;
> +
> + ret = of_property_read_u32(dev->of_node, "nxp,sys-mode",
> + &ctrl->ss_mode);
> + if (ret) {
> + dev_err(dev, "Failed to get SerDes subsystem mode\n");
> + return -EINVAL;
> + }
> +
> + if (ctrl->ss_mode > S32G_SERDES_MODE_MAX) {
> + dev_err(dev, "Invalid SerDes subsystem mode %u\n",
> + ctrl->ss_mode);
> + return -EINVAL;
> + }
> +
> + ctrl->ss_base = devm_platform_ioremap_resource_byname(pdev, "ss_pcie");
> + if (IS_ERR(ctrl->ss_base)) {
> + dev_err(dev, "Failed to map 'ss_pcie'\n");
> + return PTR_ERR(ctrl->ss_base);
> + }
> +
> + ctrl->rst = devm_reset_control_get(dev, "serdes");
> + if (IS_ERR(ctrl->rst))
> + return dev_err_probe(dev, PTR_ERR(ctrl->rst),
> + "Failed to get 'serdes' reset control\n");
> +
> + ctrl->nclks = devm_clk_bulk_get_all(dev, &ctrl->clks);
> + if (ctrl->nclks < 1)
> + return dev_err_probe(dev, ctrl->nclks,
> + "Failed to get SerDes clocks\n");
If devm_clk_bulk_get_all returns 0 then this value will
be passed to dev_err_probe(). And 0 will, in turn be returned by
dev_err_probe() and this function. However, that will be treated
as success by the caller, even though this is an error condition.
Perhaps something like this is more appropriate if ctrl->nclks
must be greater than 0. (Completely untested!)
if (ctrl->nclks < 1) {
ret = ctrl->nclks ? : -EINVAL;
return dev_err_probe(dev, ret,
"Failed to get SerDes clocks\n");
}
Flagged by Smatch.
...
> +static int s32g_serdes_parse_lanes(struct device *dev, struct s32g_serdes *serdes)
> +{
> + int ret;
> +
> + for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> + ret = s32g2_serdes_create_phy(serdes, of_port);
> + if (ret)
> + break;
> + }
> +
> + return ret;
Perhaps it cannot occur.
But if the loop above iterates zero times,
then ret will be used uninitialised here.
Also flagged by Smatch.
> +}
> +
> +static int s32g_serdes_probe(struct platform_device *pdev)
> +{
> + struct s32g_serdes *serdes;
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + serdes = devm_kzalloc(dev, sizeof(*serdes), GFP_KERNEL);
> + if (!serdes)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, serdes);
> + serdes->dev = dev;
> +
> + ret = s32g_serdes_get_ctrl_resources(pdev, serdes);
> + if (ret)
> + return ret;
> +
> + ret = s32g_serdes_get_pcie_resources(pdev, serdes);
> + if (ret)
> + return ret;
> +
> + ret = s32g_serdes_parse_lanes(dev, serdes);
> + if (ret)
> + return ret;
The I review also says:
The probe function calls s32g_serdes_init() which enables clocks,
configures hardware, and deasserts reset. However,
s32g_serdes_parse_lanes() creates PHY providers via
devm_of_phy_provider_register().
Problem: PHY consumers can start calling PHY ops (like power_on) as soon
as the provider is registered, but the hardware isn't initialized until
s32g_serdes_init() runs afterward. This creates a race window.
Recommendation: Move s32g_serdes_init() before s32g_serdes_parse_lanes().
> +
> + ret = s32g_serdes_init(serdes);
> +
> + return ret;
nit: This could be more succinctly written as:
return s32g_serdes_init(serdes);
> +}
...
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/4] phy: s32g: Add serdes subsystem phy
From: Russell King (Oracle) @ 2026-01-29 11:17 UTC (permalink / raw)
To: Vincent Guittot
Cc: vkoul, neil.armstrong, krzk+dt, conor+dt, ciprianmarian.costea,
s32, p.zabel, ghennadi.procopciuc, bogdan-gabriel.roman,
Ionut.Vicovan, alexandru-catalin.ionita, linux-phy, devicetree,
linux-kernel, linux-arm-kernel, netdev, Frank.li
In-Reply-To: <20260126092159.815968-3-vincent.guittot@linaro.org>
On Mon, Jan 26, 2026 at 10:21:57AM +0100, Vincent Guittot wrote:
> +/*
> + * Until now, there is no generic way to describe and set PCIe clock mode.
> + * PCIe controller uses the default CRNS = 0 mode.
> + */
> +enum pcie_phy_mode {
> + CRNS = 0, /* Common Reference Clock, No Spread Spectrum */
> + CRSS = 1, /* Common Reference Clock, Spread Spectrum */
> + SRNS = 2, /* Separate Reference Clock, No Spread Spectrum */
> + SRIS = 3 /* Separate Reference Clock, Spread Spectrum */
> +};
So this is a PCIe thing. If it's part of the driver's API, then it
should be common and not driver-private.
> +static inline bool is_pcie_phy_mode_valid(int mode)
> +{
> + switch (mode) {
> + case CRNS:
> + case CRSS:
> + case SRNS:
> + case SRIS:
> + return true;
> + default:
> + return false;
> + }
> +}
This checks that the submode is one of the PCIe private modes that this
driver wants to see.
> +
> +static int s32g_serdes_phy_set_mode_ext(struct phy *p,
> + enum phy_mode mode, int submode)
> +{
> + struct s32g_serdes *serdes = phy_get_drvdata(p);
> +
> + if (mode == PHY_MODE_PCIE)
> + return -EINVAL;
> +
> + if (!is_pcie_phy_mode_valid(submode))
> + return -EINVAL;
This checks for the PCIe submode, but notice the test immediately
above. PCIE mode is being rejected. So, this driver supports
everything else but PCIe.
That doesn't seem right.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/4] phy: s32g: Add serdes xpcs subsystem
From: Simon Horman @ 2026-01-29 11:59 UTC (permalink / raw)
To: Vincent Guittot
Cc: vkoul, neil.armstrong, krzk+dt, conor+dt, ciprianmarian.costea,
s32, p.zabel, linux, ghennadi.procopciuc, bogdan-gabriel.roman,
Ionut.Vicovan, alexandru-catalin.ionita, linux-phy, devicetree,
linux-kernel, linux-arm-kernel, netdev, Frank.li
In-Reply-To: <20260126092159.815968-4-vincent.guittot@linaro.org>
On Mon, Jan 26, 2026 at 10:21:58AM +0100, Vincent Guittot wrote:
...
> diff --git a/drivers/phy/freescale/phy-nxp-s32g-serdes.c b/drivers/phy/freescale/phy-nxp-s32g-serdes.c
...
> +static void s32g_serdes_prepare_pma_mode5(struct s32g_serdes *serdes)
> +{
> + u32 val;
> + /* Configure TX_VBOOST_LVL and TX_TERM_CTRL */
> + val = readl(serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MISC_CTRL_2);
> + val &= ~(EXT_TX_VBOOST_LVL_MASK | EXT_TX_TERM_CTRL_MASK);
> + val |= FIELD_PREP(EXT_TX_VBOOST_LVL_MASK, 0x3) |
> + FIELD_PREP(EXT_TX_TERM_CTRL_MASK, 0x4);
> + writel(val, serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MISC_CTRL_2);
This is part of an AI Generated review.
I have looked over it and I think it warrants investigation.
For information on how to reproduce locally, as I did, please see [1].
The entire s32g_serdes_prepare_pma_mode5() function is ~70 lines of
magic numbers with zero explanation. These appear to be
hardware-specific PLL/PHY tuning parameters for 2.5G mode.
Please consider using #defines, to give values names.
...
> +static int s32g_serdes_init_clks(struct s32g_serdes *serdes)
> +{
> + struct s32g_serdes_ctrl *ctrl = &serdes->ctrl;
> + struct s32g_xpcs_ctrl *xpcs = &serdes->xpcs;
> + int ret, order[2], xpcs_id;
> + size_t i;
> +
> + switch (ctrl->ss_mode) {
> + case 0:
> + return 0;
> + case 1:
> + order[0] = 0;
> + order[1] = XPCS_DISABLED;
> + break;
> + case 2:
> + case 5:
> + order[0] = 1;
> + order[1] = XPCS_DISABLED;
> + break;
> + case 3:
> + order[0] = 1;
> + order[1] = 0;
> + break;
> + case 4:
> + order[0] = 0;
> + order[1] = 1;
> + break;
> + default:
> + return -EINVAL;
AI review also flags that s32g_serdes_get_ctrl_resources() ensures that
ss_mode is <= 5. So this check is unnecessary.
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(order); i++) {
> + xpcs_id = order[i];
> +
> + if (xpcs_id == XPCS_DISABLED)
> + continue;
> +
> + ret = s32g_xpcs_init_plls(xpcs->phys[xpcs_id]);
> + if (ret)
> + return ret;
> + }
> +
> + if (ctrl->ss_mode == 5) {
> + s32g_serdes_prepare_pma_mode5(serdes);
> +
> + ret = s32g_xpcs_pre_pcie_2g5(xpcs->phys[1]);
Also from the AI review:
In mode 5, code directly accesses xpcs->phys[1] without checking if
it was created. If XPCS1 wasn't created for some reason (DT
misconfiguration, probe failure), this NULL dereferences.
...
> @@ -460,6 +741,41 @@ static int s32g2_serdes_create_phy(struct s32g_serdes *serdes, struct device_nod
...
> + xpcs = devm_kmalloc(dev, sizeof(*xpcs), GFP_KERNEL);
> + if (IS_ERR(xpcs)) {
> + dev_err(dev, "Failed to allocate xpcs\n");
> + return -ENOMEM;
> + }
AI review also flags:
devm_kmalloc() returns NULL on failure, not an error pointer.
This check will never trigger.
[1] https://netdev-ai.bots.linux.dev/ai-local.html
> +
> + xpcs_ctrl->phys[port] = xpcs;
> +
> + xpcs->an = of_property_read_bool(dev->of_node, "nxp,xpcs_an");
AI review flags that nxp,xpcs_an is not part of the binding.
...
> +struct phylink_pcs *s32g_serdes_pcs_create(struct device *dev, struct device_node *np)
> +{
> + struct platform_device *pdev;
> + struct device_node *pcs_np;
> + struct s32g_serdes *serdes;
> + u32 port;
> +
> + if (of_property_read_u32(np, "reg", &port))
> + return ERR_PTR(-EINVAL);
> +
> + if (port > S32G_SERDES_XPCS_MAX)
> + return ERR_PTR(-EINVAL);
> +
> + /* The PCS pdev is attached to the parent node */
> + pcs_np = of_get_parent(np);
> + if (!pcs_np)
> + return ERR_PTR(-ENODEV);
> +
> + if (!of_device_is_available(pcs_np)) {
> + of_node_put(pcs_np);
> + return ERR_PTR(-ENODEV);
> + }
> +
> + pdev = of_find_device_by_node(pcs_np);
> + of_node_put(pcs_np);
> + if (!pdev || !platform_get_drvdata(pdev)) {
> + if (pdev)
> + put_device(&pdev->dev);
> + return ERR_PTR(-EPROBE_DEFER);
> + }
> +
> + serdes = platform_get_drvdata(pdev);
Also from the AI review:
On success, the function gets a reference to pdev but never
releases it. This leaks a device reference every time a MAC driver
calls s32g_serdes_pcs_create().
> +
> + return &serdes->xpcs.phys[port]->pcs;
Also from the AI review:
The check port > S32G_SERDES_XPCS_MAX allows port=2,
but array only has indices 0 and 1.
Also I'm not seeing bounds checking on port in s32g2_serdes_create_phy
which uses port as an index for the same array both directly and indirectly
via s32g_serdes_xpcs_init().
...
> diff --git a/drivers/phy/freescale/phy-nxp-s32g-xpcs.c b/drivers/phy/freescale/phy-nxp-s32g-xpcs.c
...
> +static int s32g_xpcs_regmap_reg_read(void *context, unsigned int reg,
> + unsigned int *result)
> +{
> + struct s32g_xpcs *xpcs = context;
> + u16 ofsleft = (reg >> 8) & 0xffffU;
> + u16 ofsright = (reg & 0xffU);
> +
> + writew(ofsleft, xpcs->base + ADDR1_OFS);
> + *result = readw(xpcs->base + (ofsright * 4));
> +
> + return 0;
> +}
> +
> +static int s32g_xpcs_regmap_reg_write(void *context, unsigned int reg,
> + unsigned int val)
> +{
> + struct s32g_xpcs *xpcs = context;
> + u16 ofsleft = (reg >> 8) & 0xffffU;
> + u16 ofsright = (reg & 0xffU);
> +
> + writew(ofsleft, xpcs->base + ADDR1_OFS);
> + writew(val, xpcs->base + (ofsright * 4));
> +
> + return 0;
> +}
> +
> +static const struct regmap_config s32g_xpcs_regmap_config = {
> + .reg_bits = 16,
> + .val_bits = 16,
> + .reg_read = s32g_xpcs_regmap_reg_read,
> + .reg_write = s32g_xpcs_regmap_reg_write,
> + .wr_table = &s32g_xpcs_wr_table,
> + .rd_table = &s32g_xpcs_rd_table,
> + .max_register = 0x1F80E1,
> +};
AI review also flags that s32g_xpcs_regmap_reg_read and
s32g_xpcs_regmap_reg_write do not protect against concurrent access.
...
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 4/4] MAINTAINERS: Add MAINTAINER for NXP S32G Serdes driver
From: Simon Horman @ 2026-01-29 12:07 UTC (permalink / raw)
To: Vincent Guittot
Cc: vkoul, neil.armstrong, krzk+dt, conor+dt, ciprianmarian.costea,
s32, p.zabel, linux, ghennadi.procopciuc, bogdan-gabriel.roman,
Ionut.Vicovan, alexandru-catalin.ionita, linux-phy, devicetree,
linux-kernel, linux-arm-kernel, netdev, Frank.li
In-Reply-To: <20260126092159.815968-5-vincent.guittot@linaro.org>
On Mon, Jan 26, 2026 at 10:21:59AM +0100, Vincent Guittot wrote:
> Add a new entry for S32G Serdes driver.
>
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
> MAINTAINERS | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 765ad2daa218..888674a308a5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3202,6 +3202,15 @@ S: Maintained
> F: Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
> F: drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
>
> +ARM/NXP S32G SERDES DRIVER
> +M: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> +R: NXP S32 Linux Team <s32@nxp.com>
> +L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> +S: Maintained
> +F: Documentation/devicetree/bindings/pci/nxp,s32g-serdes.yaml
This patchset adds the following file, not the one above:
Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
^^^
> +F: drivers/phy/freescale/phy-nxp-s32g-*
> +F: include/linux/pcs/pcs-nxp-xpcs.h
> +
> ARM/Orion SoC/Technologic Systems TS-78xx platform support
> M: Alexander Clouter <alex@digriz.org.uk>
> L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> --
> 2.43.0
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/4] phy: s32g: Add serdes xpcs subsystem
From: Russell King (Oracle) @ 2026-01-29 12:30 UTC (permalink / raw)
To: Vincent Guittot
Cc: vkoul, neil.armstrong, krzk+dt, conor+dt, ciprianmarian.costea,
s32, p.zabel, ghennadi.procopciuc, bogdan-gabriel.roman,
Ionut.Vicovan, alexandru-catalin.ionita, linux-phy, devicetree,
linux-kernel, linux-arm-kernel, netdev, Frank.li
In-Reply-To: <20260126092159.815968-4-vincent.guittot@linaro.org>
On Mon, Jan 26, 2026 at 10:21:58AM +0100, Vincent Guittot wrote:
> s32g SoC family includes 2 serdes subsystems which are made of one PCIe
> controller, 2 XPCS and one Phy. The Phy got 2 lanes that can be configure
> to output PCIe lanes and/or SGMII.
>
> Add XPCS and SGMII support.
>
> Co-developed-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> Co-developed-by: Alexandru-Catalin Ionita <alexandru-catalin.ionita@nxp.com>
> Signed-off-by: Alexandru-Catalin Ionita <alexandru-catalin.ionita@nxp.com>
> Co-developed-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
> Signed-off-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
> Co-developed-by: Ionut Vicovan <Ionut.Vicovan@nxp.com>
> Signed-off-by: Ionut Vicovan <Ionut.Vicovan@nxp.com>
> Co-developed-by: Bogdan Roman <bogdan-gabriel.roman@nxp.com>
> Signed-off-by: Bogdan Roman <bogdan-gabriel.roman@nxp.com>
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
I'm not doing a full review for this patch yet.
> +/*
> + * Note: This function should be compatible with phylink.
> + * That means it should only modify link, duplex, speed
> + * an_complete, pause.
> + */
> +static int s32g_xpcs_get_state(struct s32g_xpcs *xpcs,
> + struct phylink_link_state *state)
> +{
> + struct device *dev = xpcs->dev;
> + unsigned int mii_ctrl, val, ss;
> + bool ss6, ss13, an_enabled, intr_en;
> +
> + mii_ctrl = s32g_xpcs_read(xpcs, SR_MII_CTRL);
> + an_enabled = !!(mii_ctrl & AN_ENABLE);
> + intr_en = !!(s32g_xpcs_read(xpcs, VR_MII_AN_CTRL) & MII_AN_INTR_EN);
> +
> + /* Check this important condition */
> + if (an_enabled && !intr_en) {
> + dev_err(dev, "Invalid SGMII AN config interrupt is disabled\n");
> + return -EINVAL;
> + }
This isn't an interrupt handler. Phylink calls it from the state
resolver, which _may_ be triggered by an interrupt handler, but will
also be called at other times.
> +
> + if (an_enabled) {
> + /* MLO_AN_INBAND */
> + state->speed = SPEED_UNKNOWN;
> + state->link = 0;
> + state->duplex = DUPLEX_UNKNOWN;
> + state->an_complete = 0;
> + state->pause = MLO_PAUSE_NONE;
Have you looked at the initial state that phylink sets up before
calling the .pcs_get_state() method? See phylink_mac_pcs_get_state().
speed/duplex/pause/an_complete are already setup like that for you if
autoneg is enabled. link is the only member you'd need to touch.
> + val = s32g_xpcs_read(xpcs, VR_MII_AN_INTR_STS);
> +
> + /* Interrupt is raised with each SGMII AN that is in cases
> + * Link down - Every SGMII link timer expire
> + * Link up - Once before link goes up
> + * So either linkup or raised interrupt mean AN was completed
> + */
> + if ((val & CL37_ANCMPLT_INTR) || (val & CL37_ANSGM_STS_LINK)) {
> + state->an_complete = 1;
> + if (val & CL37_ANSGM_STS_LINK)
> + state->link = 1;
> + else
> + return 0;
> + if (val & CL37_ANSGM_STS_DUPLEX)
> + state->duplex = DUPLEX_FULL;
> + else
> + state->duplex = DUPLEX_HALF;
> + ss = FIELD_GET(CL37_ANSGM_STS_SPEED_MASK, val);
> + } else {
> + return 0;
> + }
> +
> + } else {
> + /* MLO_AN_FIXED, MLO_AN_PHY */
This function won't be called in those modes, so this is a misleading
comment. It can be called in MLO_AN_INBAND but when autoneg is disabled.
> + val = s32g_xpcs_read(xpcs, SR_MII_STS);
> + state->link = !!(val & LINK_STS);
> + state->an_complete = 0;
> + state->pause = MLO_PAUSE_NONE;
> +
> + if (mii_ctrl & DUPLEX_MODE)
> + state->duplex = DUPLEX_FULL;
> + else
> + state->duplex = DUPLEX_HALF;
> +
> + /*
> + * Build similar value as CL37_ANSGM_STS_SPEED with
> + * SS6 and SS13 of SR_MII_CTRL:
> + * - 0 for 10 Mbps
> + * - 1 for 100 Mbps
> + * - 2 for 1000 Mbps
> + */
> + ss6 = !!(mii_ctrl & SS6);
> + ss13 = !!(mii_ctrl & SS13);
> + ss = ss6 << 1 | ss13;
> + }
> +
> + switch (ss) {
> + case CL37_ANSGM_10MBPS:
> + state->speed = SPEED_10;
> + break;
> + case CL37_ANSGM_100MBPS:
> + state->speed = SPEED_100;
> + break;
> + case CL37_ANSGM_1000MBPS:
> + state->speed = SPEED_1000;
> + break;
> + default:
> + dev_err(dev, "Failed to interpret the value of SR_MII_CTRL\n");
> + break;
> + }
> +
> + val = s32g_xpcs_read(xpcs, VR_MII_DIG_CTRL1);
> + if ((val & EN_2_5G_MODE) && state->speed == SPEED_1000)
> + state->speed = SPEED_2500;
> +
> + /* Cover SGMII AN inability to distigunish between 1G and 2.5G */
> + if ((val & EN_2_5G_MODE) &&
> + state->speed != SPEED_2500 && an_enabled) {
> + dev_err(dev, "Speed not supported in SGMII AN mode\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int s32g_xpcs_config_an(struct s32g_xpcs *xpcs,
> + const struct phylink_link_state state)
> +{
> + bool an_enabled = false;
> +
> + an_enabled = linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
> + state.advertising);
> + if (!an_enabled)
> + return 0;
Don't check the autoneg bit. This is the media-side autoneg, not
the PCS autoneg.
> +
> + s32g_xpcs_write_bits(xpcs, VR_MII_DIG_CTRL1,
> + CL37_TMR_OVRRIDE, CL37_TMR_OVRRIDE);
> +
> + s32g_xpcs_write_bits(xpcs, VR_MII_AN_CTRL,
> + PCS_MODE_MASK | MII_AN_INTR_EN,
> + FIELD_PREP(PCS_MODE_MASK, PCS_MODE_SGMII) | MII_AN_INTR_EN);
> + /* Enable SGMII AN */
> + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, AN_ENABLE, AN_ENABLE);
> + /* Enable SGMII AUTO SW */
> + s32g_xpcs_write_bits(xpcs, VR_MII_DIG_CTRL1,
> + MAC_AUTO_SW, MAC_AUTO_SW);
> +
> + return 0;
> +}
> +
> +static int s32g_xpcs_config(struct s32g_xpcs *xpcs,
> + const struct phylink_link_state state)
> +{
> + struct device *dev = xpcs->dev;
> + unsigned int val = 0, duplex = 0;
> + int ret = 0;
> + int speed = state.speed;
> + bool an_enabled;
> +
> + /* Configure adaptive MII width */
> + s32g_xpcs_write_bits(xpcs, VR_MII_AN_CTRL, MII_CTRL, 0);
> +
> + an_enabled = !!(s32g_xpcs_read(xpcs, SR_MII_CTRL) & AN_ENABLE);
> +
> + dev_dbg(dev, "xpcs_%d: speed=%u duplex=%d an=%d\n", xpcs->id,
> + speed, state.duplex, an_enabled);
> +
> + if (an_enabled) {
> + switch (speed) {
> + case SPEED_10:
> + case SPEED_100:
> + case SPEED_1000:
> + s32g_xpcs_write(xpcs, VR_MII_LINK_TIMER_CTRL, 0x2faf);
> + break;
> + case SPEED_2500:
> + s32g_xpcs_write(xpcs, VR_MII_LINK_TIMER_CTRL, 0x7a1);
> + s32g_xpcs_write_bits(xpcs, VR_MII_DIG_CTRL1, MAC_AUTO_SW, 0);
Configuring the link timer _after_ the link has already come up looks
completely wrong to me... this should be done when .pcs_config() detects
that the PHY interface mode has changed.
> + break;
> + default:
> + dev_err(dev, "Speed not recognized. Can't setup xpcs\n");
> + return -EINVAL;
> + }
> +
> + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, RESTART_AN, RESTART_AN);
As this is called from the .pcs_link_up() method, expect the link to
go bouncey bouncy bouncy if you restart AN _after_ the link has
come up.
> +
> + ret = s32g_xpcs_wait_an_done(xpcs);
> + if (ret)
> + dev_warn(dev, "AN did not finish for XPCS%d", xpcs->id);
> +
> + /* Clear the AN CMPL intr */
> + s32g_xpcs_write_bits(xpcs, VR_MII_AN_INTR_STS, CL37_ANCMPLT_INTR, 0);
> + } else {
> + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, AN_ENABLE, 0);
> + s32g_xpcs_write_bits(xpcs, VR_MII_AN_CTRL, MII_AN_INTR_EN, 0);
> +
> + switch (speed) {
> + case SPEED_10:
> + break;
> + case SPEED_100:
> + val = SS13;
> + break;
> + case SPEED_1000:
> + val = SS6;
> + break;
> + case SPEED_2500:
> + val = SS6;
> + break;
> + default:
> + dev_err(dev, "Speed not supported\n");
> + break;
> + }
> +
> + if (state.duplex == DUPLEX_FULL)
> + duplex = DUPLEX_MODE;
> +
> + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, DUPLEX_MODE, duplex);
> +
> + if (speed == SPEED_2500) {
> + ret = s32g_serdes_bifurcation_pll_transit(xpcs, XPCS_PLLB);
> + if (ret)
> + dev_err(dev, "Switch to PLLB failed\n");
> + } else {
> + ret = s32g_serdes_bifurcation_pll_transit(xpcs, XPCS_PLLA);
> + if (ret)
> + dev_err(dev, "Switch to PLLA failed\n");
> + }
> +
> + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, SS6 | SS13, val);
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * phylink_pcs_ops fops
They are not "fops" which commonly refers to struct file_operations
> + */
> +
> +static void s32cc_phylink_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
> + struct phylink_link_state *state)
> +{
> + struct s32g_xpcs *xpcs = phylink_pcs_to_s32g_xpcs(pcs);
> +
> + s32g_xpcs_get_state(xpcs, state);
> +}
Seems to me a pointless wrapper.
> +
> +static int s32cc_phylink_pcs_config(struct phylink_pcs *pcs,
> + unsigned int neg_mode,
> + phy_interface_t interface,
> + const unsigned long *advertising,
> + bool permit_pause_to_mac)
> +{
> + struct s32g_xpcs *xpcs = phylink_pcs_to_s32g_xpcs(pcs);
> + struct phylink_link_state state = { 0 };
> +
> + if (!(neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED))
> + return 0;
> +
> + linkmode_copy(state.advertising, advertising);
> +
> + return s32g_xpcs_config_an(xpcs, state);
Given this is the only callsite for this function, and the only thing
you pass is the advertising mask, why pass a struct phylink_link_state
rather than the advertising mask?
> +}
> +
> +static void s32cc_phylink_pcs_restart_an(struct phylink_pcs *pcs)
> +{
> + /* Not yet */
> +}
> +
> +static void s32cc_phylink_pcs_link_up(struct phylink_pcs *pcs,
> + unsigned int neg_mode,
> + phy_interface_t interface, int speed,
> + int duplex)
> +{
> + struct s32g_xpcs *xpcs = phylink_pcs_to_s32g_xpcs(pcs);
> + struct phylink_link_state state = { 0 };
> +
> + state.speed = speed;
> + state.duplex = duplex;
> + state.an_complete = false;
an_complete is never an "input" to drivers. It's a state from PCS
drivers back to phylink. Also, s32g_xpcs_config never looks at this.
> +
> + s32g_xpcs_config(xpcs, state);
Again, the only things that this function uses are the speed and
duplex, so why wrap them up into a struct?
> +}
> +
> +static const struct phylink_pcs_ops s32cc_phylink_pcs_ops = {
> + .pcs_get_state = s32cc_phylink_pcs_get_state,
> + .pcs_config = s32cc_phylink_pcs_config,
> + .pcs_an_restart = s32cc_phylink_pcs_restart_an,
> + .pcs_link_up = s32cc_phylink_pcs_link_up,
> +};
Please implement .pcs_inband_caps. As you don't support disabling
inband for SGMII, that means you can't support MLO_AN_PHY mode
reliably.
Also note that there are PHYs out there that do _not_ provide SGMII
inband, which means if you have it enabled, and you're relying on it
to complete, you won't be able to interface with those PHYs. There's
such a PHY on a SFP module.
If this driver is purely for a network PCS, then please locate it in
drivers/net/pcs.
I'm pretty sure there's other stuff I've missed as far as the phylink
API goes, so please expect further review once you've addressed the
comments above.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 0/4] Serdes: s32g: Add support for serdes subsystem
From: Russell King (Oracle) @ 2026-01-29 12:36 UTC (permalink / raw)
To: Vincent Guittot
Cc: vkoul, neil.armstrong, krzk+dt, conor+dt, ciprianmarian.costea,
s32, p.zabel, ghennadi.procopciuc, Ionut.Vicovan, linux-phy,
devicetree, linux-kernel, linux-arm-kernel, netdev, Frank.li
In-Reply-To: <20260126092159.815968-1-vincent.guittot@linaro.org>
Please drop these addresses from future patch series:
alexandru-catalin.ionita@nxp.com
host nxp-com.mail.protection.outlook.com [2a01:111:f403:ca09::7]
SMTP error from remote mail server after RCPT TO:<alexandru-catalin.ionita@n
xp.com>:
550 5.4.1 Recipient address rejected: Access denied. For more information se
e https://aka.ms/EXOSmtpErrors [AM4PEPF00027A62.eurprd04.prod.outlook.com 2026-0
1-29T12:31:01.197Z 08DE5971FB66165F]
bogdan-gabriel.roman@nxp.com
host nxp-com.mail.protection.outlook.com [2a01:111:f403:ca09::6]
SMTP error from remote mail server after RCPT TO:<bogdan-gabriel.roman@nxp.c
om>:
550 5.4.1 Recipient address rejected: Access denied. For more information se
e https://aka.ms/EXOSmtpErrors [AM3PEPF0000A798.eurprd04.prod.outlook.com 2026-0
1-29T12:31:00.781Z 08DE596525364766]
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: serdes: s32g: Add NXP serdes subsystem
From: Russell King (Oracle) @ 2026-01-29 12:50 UTC (permalink / raw)
To: Vincent Guittot
Cc: vkoul, neil.armstrong, krzk+dt, conor+dt, ciprianmarian.costea,
s32, p.zabel, ghennadi.procopciuc, bogdan-gabriel.roman,
Ionut.Vicovan, alexandru-catalin.ionita, linux-phy, devicetree,
linux-kernel, linux-arm-kernel, netdev, Frank.li
In-Reply-To: <20260126092159.815968-2-vincent.guittot@linaro.org>
On Mon, Jan 26, 2026 at 10:21:56AM +0100, Vincent Guittot wrote:
> Describe the serdes subsystem available on the S32G platforms.
>
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
> .../bindings/phy/nxp,s32g-serdes.yaml | 154 ++++++++++++++++++
> 1 file changed, 154 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
>
> diff --git a/Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml b/Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
> new file mode 100644
> index 000000000000..fad34bee2a4f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
> @@ -0,0 +1,154 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/nxp,s32g-serdes.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP S32G2xxx/S32G3xxx SerDes PHY subsystem
> +
> +maintainers:
> + - Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
> +
> +description: |
> + The SerDes subsystem on S32G SoC Family includes two types of PHYs:
> + - One PCIe PHY: Supports various PCIe operation modes
> + - Two Ethernet Physical Coding Sublayer (XPCS) controllers
> +
> + SerDes operation mode selects the enabled PHYs and speeds. Clock frequency
> + must be adapted accordingly. Below table describes all possible operation
> + modes.
> +
> + Mode PCIe XPCS0 XPCS1 PHY clock Description
> + SGMII SGMII (MHz)
> + -------------------------------------------------------------------------
> + 0 Gen3 N/A N/A 100 Single PCIe
> + 1 Gen2 1.25Gbps N/A 100 PCIe/SGMII
> + 2 Gen2 N/A 1.25Gbps 100 PCIe/SGMII
> + 3 N/A 1.25Gbps 1.25Gbps 100,125 SGMII
> + 4 N/A 3.125/1.25Gbps 3.125/1.25Gbps 125 SGMII
> + 5 Gen2 N/A 3.125Gbps 100 PCIe/SGMII
Shouldn't the mode be configured via phy_set_mode_ext()?
This identifies whether it is operating as PCIe or for networking and
in the case of networking, the PHY interface mode should be passed as
the submode.
Have a look at include/linux/phy/pcie.h to see the submodes that may
be appropriate to pass to phy_set_mode_ext() - but talk to the PHY
subsystem maintainers.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/4] phy: s32g: Add serdes subsystem phy
From: Vincent Guittot @ 2026-01-29 13:01 UTC (permalink / raw)
To: Simon Horman
Cc: vkoul, neil.armstrong, krzk+dt, conor+dt, ciprianmarian.costea,
s32, p.zabel, linux, ghennadi.procopciuc, bogdan-gabriel.roman,
Ionut.Vicovan, alexandru-catalin.ionita, linux-phy, devicetree,
linux-kernel, linux-arm-kernel, netdev, Frank.li
In-Reply-To: <aXsuRTZUUnw0kdzV@horms.kernel.org>
On Thu, 29 Jan 2026 at 10:54, Simon Horman <horms@kernel.org> wrote:
>
> On Mon, Jan 26, 2026 at 10:21:57AM +0100, Vincent Guittot wrote:
>
> ...
>
> > diff --git a/drivers/phy/freescale/phy-nxp-s32g-serdes.c b/drivers/phy/freescale/phy-nxp-s32g-serdes.c
> > new file mode 100644
> > index 000000000000..8336c868c8dc
> > --- /dev/null
> > +++ b/drivers/phy/freescale/phy-nxp-s32g-serdes.c
> > @@ -0,0 +1,569 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/**
> > + * SerDes driver for S32G SoCs
> > + *
> > + * Copyright 2021-2026 NXP
> > + */
> > +
> > +#include <dt-bindings/phy/phy.h>
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
>
> Hi Vincent, all,
>
> I think that you also need:
>
> #include <linux/iopoll.h>
>
> So that read_poll_timeout() is declared.
> Else this patch causes a transient build failure
> (for x86_64 allmodconfig)
ok, i will add it
>
> > +#include <linux/module.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/of_address.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/processor.h>
> > +#include <linux/reset.h>
> > +#include <linux/units.h>
>
> ...
>
> > +static int s32g_serdes_phy_set_mode_ext(struct phy *p,
> > + enum phy_mode mode, int submode)
> > +{
> > + struct s32g_serdes *serdes = phy_get_drvdata(p);
> > +
> > + if (mode == PHY_MODE_PCIE)
> > + return -EINVAL;
>
> This is part of an AI Generated review.
> I have looked over it and I think it warrants investigation.
> For information on how to reproduce locally, as I did, please see [1].
>
> [1] https://netdev-ai.bots.linux.dev/ai-local.html
>
> This returns error if mode IS PHY_MODE_PCIE, but this is a PCIe PHY!
> This looks like a typo. Should be !=.
yes don't know what happened here but it should be !=
>
> > +
> > + if (!is_pcie_phy_mode_valid(submode))
> > + return -EINVAL;
> > +
> > + /*
> > + * Do not configure SRIS or CRSS PHY MODE in conjunction
> > + * with any SGMII mode on the same SerDes subsystem
> > + */
> > + if ((submode == CRSS || submode == SRIS) &&
> > + serdes->ctrl.ss_mode != 0)
> > + return -EINVAL;
> > +
> > + /*
> > + * Internal reference clock cannot be used with either Common clock
> > + * or Spread spectrum, leaving only SRNSS
> > + */
> > + if (submode != SRNS && !serdes->ctrl.ext_clk)
> > + return -EINVAL;
> > +
> > + serdes->pcie.phy_mode = submode;
>
> The AI review also suggested that it may be unsafe
> to set the submode after s32g_serdes_phy_power_on()
> has been called. And that there is nothing preventing that.
>
> TBH, I am unsure if either of those statements are true.
> But it seems worth validating with you.
yes, the usual pattern is :
- phy_set_mode_ext()
- then phy_power_on()
but I can add an additional check
>
> > +
> > + return 0;
> > +}
>
> ...
>
> > +static int s32g_serdes_get_ctrl_resources(struct platform_device *pdev, struct s32g_serdes *serdes)
> > +{
> > + struct s32g_serdes_ctrl *ctrl = &serdes->ctrl;
> > + struct device *dev = &pdev->dev;
> > + int ret, idx;
> > +
> > + ret = of_property_read_u32(dev->of_node, "nxp,sys-mode",
> > + &ctrl->ss_mode);
> > + if (ret) {
> > + dev_err(dev, "Failed to get SerDes subsystem mode\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (ctrl->ss_mode > S32G_SERDES_MODE_MAX) {
> > + dev_err(dev, "Invalid SerDes subsystem mode %u\n",
> > + ctrl->ss_mode);
> > + return -EINVAL;
> > + }
> > +
> > + ctrl->ss_base = devm_platform_ioremap_resource_byname(pdev, "ss_pcie");
> > + if (IS_ERR(ctrl->ss_base)) {
> > + dev_err(dev, "Failed to map 'ss_pcie'\n");
> > + return PTR_ERR(ctrl->ss_base);
> > + }
> > +
> > + ctrl->rst = devm_reset_control_get(dev, "serdes");
> > + if (IS_ERR(ctrl->rst))
> > + return dev_err_probe(dev, PTR_ERR(ctrl->rst),
> > + "Failed to get 'serdes' reset control\n");
> > +
> > + ctrl->nclks = devm_clk_bulk_get_all(dev, &ctrl->clks);
> > + if (ctrl->nclks < 1)
> > + return dev_err_probe(dev, ctrl->nclks,
> > + "Failed to get SerDes clocks\n");
>
> If devm_clk_bulk_get_all returns 0 then this value will
> be passed to dev_err_probe(). And 0 will, in turn be returned by
> dev_err_probe() and this function. However, that will be treated
> as success by the caller, even though this is an error condition.
>
> Perhaps something like this is more appropriate if ctrl->nclks
> must be greater than 0. (Completely untested!)
>
> if (ctrl->nclks < 1) {
> ret = ctrl->nclks ? : -EINVAL;
> return dev_err_probe(dev, ret,
> "Failed to get SerDes clocks\n");
> }
>
> Flagged by Smatch.
okay
>
> ...
>
> > +static int s32g_serdes_parse_lanes(struct device *dev, struct s32g_serdes *serdes)
> > +{
> > + int ret;
> > +
> > + for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> > + ret = s32g2_serdes_create_phy(serdes, of_port);
> > + if (ret)
> > + break;
> > + }
> > +
> > + return ret;
>
> Perhaps it cannot occur.
> But if the loop above iterates zero times,
> then ret will be used uninitialised here.
should not but will fix it
>
> Also flagged by Smatch.
>
> > +}
> > +
> > +static int s32g_serdes_probe(struct platform_device *pdev)
> > +{
> > + struct s32g_serdes *serdes;
> > + struct device *dev = &pdev->dev;
> > + int ret;
> > +
> > + serdes = devm_kzalloc(dev, sizeof(*serdes), GFP_KERNEL);
> > + if (!serdes)
> > + return -ENOMEM;
> > +
> > + platform_set_drvdata(pdev, serdes);
> > + serdes->dev = dev;
> > +
> > + ret = s32g_serdes_get_ctrl_resources(pdev, serdes);
> > + if (ret)
> > + return ret;
> > +
> > + ret = s32g_serdes_get_pcie_resources(pdev, serdes);
> > + if (ret)
> > + return ret;
> > +
> > + ret = s32g_serdes_parse_lanes(dev, serdes);
> > + if (ret)
> > + return ret;
>
> The I review also says:
>
> The probe function calls s32g_serdes_init() which enables clocks,
> configures hardware, and deasserts reset. However,
> s32g_serdes_parse_lanes() creates PHY providers via
> devm_of_phy_provider_register().
>
> Problem: PHY consumers can start calling PHY ops (like power_on) as soon
> as the provider is registered, but the hardware isn't initialized until
> s32g_serdes_init() runs afterward. This creates a race window.
>
> Recommendation: Move s32g_serdes_init() before s32g_serdes_parse_lanes().
I will look at this more deeply but part of s32g_serdes_init() needs
lanes to be parsed for configuring clock
>
> > +
> > + ret = s32g_serdes_init(serdes);
> > +
> > + return ret;
>
> nit: This could be more succinctly written as:
fair enough
>
> return s32g_serdes_init(serdes);
>
> > +}
>
> ...
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/4] phy: s32g: Add serdes subsystem phy
From: Vincent Guittot @ 2026-01-29 13:02 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: vkoul, neil.armstrong, krzk+dt, conor+dt, ciprianmarian.costea,
s32, p.zabel, ghennadi.procopciuc, Ionut.Vicovan, linux-phy,
devicetree, linux-kernel, linux-arm-kernel, netdev, Frank.li
In-Reply-To: <aXtB0QcVOkE_hhny@shell.armlinux.org.uk>
On Thu, 29 Jan 2026 at 12:17, Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Mon, Jan 26, 2026 at 10:21:57AM +0100, Vincent Guittot wrote:
> > +/*
> > + * Until now, there is no generic way to describe and set PCIe clock mode.
> > + * PCIe controller uses the default CRNS = 0 mode.
> > + */
> > +enum pcie_phy_mode {
> > + CRNS = 0, /* Common Reference Clock, No Spread Spectrum */
> > + CRSS = 1, /* Common Reference Clock, Spread Spectrum */
> > + SRNS = 2, /* Separate Reference Clock, No Spread Spectrum */
> > + SRIS = 3 /* Separate Reference Clock, Spread Spectrum */
> > +};
>
> So this is a PCIe thing. If it's part of the driver's API, then it
> should be common and not driver-private.
>
> > +static inline bool is_pcie_phy_mode_valid(int mode)
> > +{
> > + switch (mode) {
> > + case CRNS:
> > + case CRSS:
> > + case SRNS:
> > + case SRIS:
> > + return true;
> > + default:
> > + return false;
> > + }
> > +}
>
> This checks that the submode is one of the PCIe private modes that this
> driver wants to see.
>
> > +
> > +static int s32g_serdes_phy_set_mode_ext(struct phy *p,
> > + enum phy_mode mode, int submode)
> > +{
> > + struct s32g_serdes *serdes = phy_get_drvdata(p);
> > +
> > + if (mode == PHY_MODE_PCIE)
> > + return -EINVAL;
> > +
> > + if (!is_pcie_phy_mode_valid(submode))
> > + return -EINVAL;
>
> This checks for the PCIe submode, but notice the test immediately
> above. PCIE mode is being rejected. So, this driver supports
> everything else but PCIe.
>
> That doesn't seem right.
It's a mistake
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: serdes: s32g: Add NXP serdes subsystem
From: Vincent Guittot @ 2026-01-29 13:05 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: vkoul, neil.armstrong, krzk+dt, conor+dt, ciprianmarian.costea,
s32, p.zabel, ghennadi.procopciuc, bogdan-gabriel.roman,
Ionut.Vicovan, alexandru-catalin.ionita, linux-phy, devicetree,
linux-kernel, linux-arm-kernel, netdev, Frank.li
In-Reply-To: <aXtXmN1V8wg57yuE@shell.armlinux.org.uk>
On Thu, 29 Jan 2026 at 13:50, Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Mon, Jan 26, 2026 at 10:21:56AM +0100, Vincent Guittot wrote:
> > Describe the serdes subsystem available on the S32G platforms.
> >
> > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> > ---
> > .../bindings/phy/nxp,s32g-serdes.yaml | 154 ++++++++++++++++++
> > 1 file changed, 154 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml b/Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
> > new file mode 100644
> > index 000000000000..fad34bee2a4f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
> > @@ -0,0 +1,154 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/nxp,s32g-serdes.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP S32G2xxx/S32G3xxx SerDes PHY subsystem
> > +
> > +maintainers:
> > + - Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
> > +
> > +description: |
> > + The SerDes subsystem on S32G SoC Family includes two types of PHYs:
> > + - One PCIe PHY: Supports various PCIe operation modes
> > + - Two Ethernet Physical Coding Sublayer (XPCS) controllers
> > +
> > + SerDes operation mode selects the enabled PHYs and speeds. Clock frequency
> > + must be adapted accordingly. Below table describes all possible operation
> > + modes.
> > +
> > + Mode PCIe XPCS0 XPCS1 PHY clock Description
> > + SGMII SGMII (MHz)
> > + -------------------------------------------------------------------------
> > + 0 Gen3 N/A N/A 100 Single PCIe
> > + 1 Gen2 1.25Gbps N/A 100 PCIe/SGMII
> > + 2 Gen2 N/A 1.25Gbps 100 PCIe/SGMII
> > + 3 N/A 1.25Gbps 1.25Gbps 100,125 SGMII
> > + 4 N/A 3.125/1.25Gbps 3.125/1.25Gbps 125 SGMII
> > + 5 Gen2 N/A 3.125Gbps 100 PCIe/SGMII
>
> Shouldn't the mode be configured via phy_set_mode_ext()?
There is a phy for only pcie only. In mode 3 to 5 there is no generic
phy created
>
> This identifies whether it is operating as PCIe or for networking and
> in the case of networking, the PHY interface mode should be passed as
> the submode.
>
> Have a look at include/linux/phy/pcie.h to see the submodes that may
> be appropriate to pass to phy_set_mode_ext() - but talk to the PHY
> subsystem maintainers.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/4] phy: s32g: Add serdes subsystem phy
From: Russell King (Oracle) @ 2026-01-29 13:23 UTC (permalink / raw)
To: Vincent Guittot
Cc: Simon Horman, vkoul, neil.armstrong, krzk+dt, conor+dt,
ciprianmarian.costea, s32, p.zabel, ghennadi.procopciuc,
bogdan-gabriel.roman, Ionut.Vicovan, alexandru-catalin.ionita,
linux-phy, devicetree, linux-kernel, linux-arm-kernel, netdev,
Frank.li
In-Reply-To: <CAKfTPtDfnpzq2CB-isVzvh1ZCWo7kit9KRJvVGoU1C3zZTgdXw@mail.gmail.com>
On Thu, Jan 29, 2026 at 02:01:13PM +0100, Vincent Guittot wrote:
> yes, the usual pattern is :
> - phy_set_mode_ext()
> - then phy_power_on()
> but I can add an additional check
Please read Documentation/driver-api/phy/phy.rst section "Order of API
calls" which suggests phy_set_mode_ext() after phy_power_on().
Having different requirements for different SerDes PHYs quickly becomes
annoying for users of the SerDes PHYs.
E.g. I'm trying to add SerDes PHY support to stmmac, which is used
across different platforms. Having all SerDes PHY drivers behave the
same as far as their PHY API calls are concerned means that the whole
point of having an abstracted interface is maintained. Otherwise, it's
completely pointless.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/4] phy: s32g: Add serdes xpcs subsystem
From: Vincent Guittot @ 2026-01-29 13:24 UTC (permalink / raw)
To: Simon Horman
Cc: vkoul, neil.armstrong, krzk+dt, conor+dt, ciprianmarian.costea,
s32, p.zabel, linux, ghennadi.procopciuc, bogdan-gabriel.roman,
Ionut.Vicovan, alexandru-catalin.ionita, linux-phy, devicetree,
linux-kernel, linux-arm-kernel, netdev, Frank.li
In-Reply-To: <aXtLsuOkMMryrUs4@horms.kernel.org>
On Thu, 29 Jan 2026 at 12:59, Simon Horman <horms@kernel.org> wrote:
>
> On Mon, Jan 26, 2026 at 10:21:58AM +0100, Vincent Guittot wrote:
>
> ...
>
> > diff --git a/drivers/phy/freescale/phy-nxp-s32g-serdes.c b/drivers/phy/freescale/phy-nxp-s32g-serdes.c
>
> ...
>
> > +static void s32g_serdes_prepare_pma_mode5(struct s32g_serdes *serdes)
> > +{
> > + u32 val;
> > + /* Configure TX_VBOOST_LVL and TX_TERM_CTRL */
> > + val = readl(serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MISC_CTRL_2);
> > + val &= ~(EXT_TX_VBOOST_LVL_MASK | EXT_TX_TERM_CTRL_MASK);
> > + val |= FIELD_PREP(EXT_TX_VBOOST_LVL_MASK, 0x3) |
> > + FIELD_PREP(EXT_TX_TERM_CTRL_MASK, 0x4);
> > + writel(val, serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MISC_CTRL_2);
>
> This is part of an AI Generated review.
> I have looked over it and I think it warrants investigation.
> For information on how to reproduce locally, as I did, please see [1].
>
> The entire s32g_serdes_prepare_pma_mode5() function is ~70 lines of
> magic numbers with zero explanation. These appear to be
> hardware-specific PLL/PHY tuning parameters for 2.5G mode.
Unfortunately there is no additional information in the reference
manual other than
*step 4:
- Write 3h to EXT_TX_VBOOST_LVL.
- Write 4h to EXT_TX_TERM_CTRL
>
> Please consider using #defines, to give values names.
>
> ...
>
> > +static int s32g_serdes_init_clks(struct s32g_serdes *serdes)
> > +{
> > + struct s32g_serdes_ctrl *ctrl = &serdes->ctrl;
> > + struct s32g_xpcs_ctrl *xpcs = &serdes->xpcs;
> > + int ret, order[2], xpcs_id;
> > + size_t i;
> > +
> > + switch (ctrl->ss_mode) {
> > + case 0:
> > + return 0;
> > + case 1:
> > + order[0] = 0;
> > + order[1] = XPCS_DISABLED;
> > + break;
> > + case 2:
> > + case 5:
> > + order[0] = 1;
> > + order[1] = XPCS_DISABLED;
> > + break;
> > + case 3:
> > + order[0] = 1;
> > + order[1] = 0;
> > + break;
> > + case 4:
> > + order[0] = 0;
> > + order[1] = 1;
> > + break;
> > + default:
> > + return -EINVAL;
>
> AI review also flags that s32g_serdes_get_ctrl_resources() ensures that
> ss_mode is <= 5. So this check is unnecessary.
okay but providing a default seems a good practice
>
> > + }
> > +
> > + for (i = 0; i < ARRAY_SIZE(order); i++) {
> > + xpcs_id = order[i];
> > +
> > + if (xpcs_id == XPCS_DISABLED)
> > + continue;
> > +
> > + ret = s32g_xpcs_init_plls(xpcs->phys[xpcs_id]);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + if (ctrl->ss_mode == 5) {
> > + s32g_serdes_prepare_pma_mode5(serdes);
> > +
> > + ret = s32g_xpcs_pre_pcie_2g5(xpcs->phys[1]);
>
> Also from the AI review:
>
> In mode 5, code directly accesses xpcs->phys[1] without checking if
> it was created. If XPCS1 wasn't created for some reason (DT
> misconfiguration, probe failure), this NULL dereferences.
I will add a check
>
> ...
>
> > @@ -460,6 +741,41 @@ static int s32g2_serdes_create_phy(struct s32g_serdes *serdes, struct device_nod
>
> ...
>
> > + xpcs = devm_kmalloc(dev, sizeof(*xpcs), GFP_KERNEL);
> > + if (IS_ERR(xpcs)) {
> > + dev_err(dev, "Failed to allocate xpcs\n");
> > + return -ENOMEM;
> > + }
>
> AI review also flags:
>
> devm_kmalloc() returns NULL on failure, not an error pointer.
> This check will never trigger.
okay
>
> [1] https://netdev-ai.bots.linux.dev/ai-local.html
>
> > +
> > + xpcs_ctrl->phys[port] = xpcs;
> > +
> > + xpcs->an = of_property_read_bool(dev->of_node, "nxp,xpcs_an");
>
> AI review flags that nxp,xpcs_an is not part of the binding.
That's a leftover that should be removed
>
> ...
>
> > +struct phylink_pcs *s32g_serdes_pcs_create(struct device *dev, struct device_node *np)
> > +{
> > + struct platform_device *pdev;
> > + struct device_node *pcs_np;
> > + struct s32g_serdes *serdes;
> > + u32 port;
> > +
> > + if (of_property_read_u32(np, "reg", &port))
> > + return ERR_PTR(-EINVAL);
> > +
> > + if (port > S32G_SERDES_XPCS_MAX)
> > + return ERR_PTR(-EINVAL);
> > +
> > + /* The PCS pdev is attached to the parent node */
> > + pcs_np = of_get_parent(np);
> > + if (!pcs_np)
> > + return ERR_PTR(-ENODEV);
> > +
> > + if (!of_device_is_available(pcs_np)) {
> > + of_node_put(pcs_np);
> > + return ERR_PTR(-ENODEV);
> > + }
> > +
> > + pdev = of_find_device_by_node(pcs_np);
> > + of_node_put(pcs_np);
> > + if (!pdev || !platform_get_drvdata(pdev)) {
> > + if (pdev)
> > + put_device(&pdev->dev);
> > + return ERR_PTR(-EPROBE_DEFER);
> > + }
> > +
> > + serdes = platform_get_drvdata(pdev);
>
> Also from the AI review:
>
> On success, the function gets a reference to pdev but never
> releases it. This leaks a device reference every time a MAC driver
> calls s32g_serdes_pcs_create().
will fix it
>
> > +
> > + return &serdes->xpcs.phys[port]->pcs;
>
> Also from the AI review:
>
> The check port > S32G_SERDES_XPCS_MAX allows port=2,
> but array only has indices 0 and 1.
>
> Also I'm not seeing bounds checking on port in s32g2_serdes_create_phy
> which uses port as an index for the same array both directly and indirectly
> via s32g_serdes_xpcs_init().
will add it
>
> ...
>
> > diff --git a/drivers/phy/freescale/phy-nxp-s32g-xpcs.c b/drivers/phy/freescale/phy-nxp-s32g-xpcs.c
>
> ...
>
> > +static int s32g_xpcs_regmap_reg_read(void *context, unsigned int reg,
> > + unsigned int *result)
> > +{
> > + struct s32g_xpcs *xpcs = context;
> > + u16 ofsleft = (reg >> 8) & 0xffffU;
> > + u16 ofsright = (reg & 0xffU);
> > +
> > + writew(ofsleft, xpcs->base + ADDR1_OFS);
> > + *result = readw(xpcs->base + (ofsright * 4));
> > +
> > + return 0;
> > +}
> > +
> > +static int s32g_xpcs_regmap_reg_write(void *context, unsigned int reg,
> > + unsigned int val)
> > +{
> > + struct s32g_xpcs *xpcs = context;
> > + u16 ofsleft = (reg >> 8) & 0xffffU;
> > + u16 ofsright = (reg & 0xffU);
> > +
> > + writew(ofsleft, xpcs->base + ADDR1_OFS);
> > + writew(val, xpcs->base + (ofsright * 4));
> > +
> > + return 0;
> > +}
> > +
> > +static const struct regmap_config s32g_xpcs_regmap_config = {
> > + .reg_bits = 16,
> > + .val_bits = 16,
> > + .reg_read = s32g_xpcs_regmap_reg_read,
> > + .reg_write = s32g_xpcs_regmap_reg_write,
> > + .wr_table = &s32g_xpcs_wr_table,
> > + .rd_table = &s32g_xpcs_rd_table,
> > + .max_register = 0x1F80E1,
> > +};
>
> AI review also flags that s32g_xpcs_regmap_reg_read and
> s32g_xpcs_regmap_reg_write do not protect against concurrent access.
but regmap framework should
>
> ...
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 4/4] MAINTAINERS: Add MAINTAINER for NXP S32G Serdes driver
From: Vincent Guittot @ 2026-01-29 13:25 UTC (permalink / raw)
To: Simon Horman
Cc: vkoul, neil.armstrong, krzk+dt, conor+dt, ciprianmarian.costea,
s32, p.zabel, linux, ghennadi.procopciuc, Ionut.Vicovan,
linux-phy, devicetree, linux-kernel, linux-arm-kernel, netdev,
Frank.li
In-Reply-To: <20260129120757.GA358439@kernel.org>
On Thu, 29 Jan 2026 at 13:08, Simon Horman <horms@kernel.org> wrote:
>
> On Mon, Jan 26, 2026 at 10:21:59AM +0100, Vincent Guittot wrote:
> > Add a new entry for S32G Serdes driver.
> >
> > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> > ---
> > MAINTAINERS | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 765ad2daa218..888674a308a5 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -3202,6 +3202,15 @@ S: Maintained
> > F: Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
> > F: drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
> >
> > +ARM/NXP S32G SERDES DRIVER
> > +M: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> > +R: NXP S32 Linux Team <s32@nxp.com>
> > +L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/pci/nxp,s32g-serdes.yaml
>
> This patchset adds the following file, not the one above:
>
> Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
yes, that's a typo.
> ^^^
>
> > +F: drivers/phy/freescale/phy-nxp-s32g-*
> > +F: include/linux/pcs/pcs-nxp-xpcs.h
> > +
> > ARM/Orion SoC/Technologic Systems TS-78xx platform support
> > M: Alexander Clouter <alex@digriz.org.uk>
> > L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> > --
> > 2.43.0
> >
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox