Re: [PATCH v2 04/11] soc: airoha: add support for configuring SCU SSR Serdes port

On Thu, Mar 20, 2025 at 03:49:08PM +0100, Arnd Bergmann wrote:

On Thu, Mar 20, 2025, at 14:00, Christian Marangi wrote:

quoted

Add support for configuring SCU SSR Serdes port. Airoha AN7581 SoC can
configure the different Serdes port by toggling bits in the SCU register
space.

Port Serdes mode are mutually exclusive, force example the USB2 Serdes port
can either used for USB 3.0 or PCIe 2 port. Enabling USB 3.0 makes the
PCIe 2 to not work.

The current supported Serdes port are:
- WiFi 1 and defaults to PCIe0 1 line mode
- Wifi 2 and defaults to PCIe1 1 line mode
- USB 1 and defaults to USB 3.0 mode
- USB 2 and defaults to USB 3.0 mode

WiFi 1, WiFi 2 and USB 1 also support a particular Ethernet mode that
can toggle between USXGMII or HSGMII mode (USB 1 only to HSGMII)
Such mode doesn't configure bits as specific Ethernet PCS driver will
take care of configuring the Serdes mode based on what is required.

This driver is to correctly setup these bits.
Single driver can't independently set the Serdes port mode as that
would cause a conflict if someone declare, for example, in DT
(and enable) PCIe 2 port and USB2 3.0 port.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

I think serdes drivers are usually implement in the drivers/phy
layer, and I see there is already a drivers/phy/phy-airoha-pcie.c,
which may or may not overlap with this one (I have not looked at
the details).

Have you tried to use the phy subsystem interface here instead
of creating a custom in-kernel interface?

These really set 1-2 bit and I think PHY can't describe PCIe in x2 mode
or in x1. Also I think PHY is used for more advanced stuff and usually
have dedicated register/maps. This is really to configure 1-2 bit and
provide the mode, nothing else... no enable, no power up.

Do you think a it's possible to implement a ""read-only"" PHY driver?

The PCIe x2 mode maybe can be modelled with
phy-cells = <2> and adding a extra entry to enforce x2 line mode?

But I feel it would be wrong to say that the SCU expose PHY as it won't
be true.

Honestly we should really consider starting to implement a generic
provider for these stuff... it's not the first time we have bit that
configure part of the entire system.

For example this is very common for QCOM with TCSR and also for Mediatek
with the TPHY. (but TPHY is at least more realistic as it can enable and
disable serdes port... here it's just 1 bit)

-- 
	Ansuel