| Message ID | 20210526135535.2515123-1-vladimir.oltean@nxp.com |
|---|---|
| Headers | show |
| Series | Add NXP SJA1110 support to the sja1105 DSA driver | expand |
On Wed, May 26, 2021 at 8:56 AM Vladimir Oltean <olteanv@gmail.com> wrote: > > There are 4 variations of the SJA1110 switch which have a different set > of MII protocols supported per port. Document the compatible strings. > > Also, the SJA1110 optionally supports 2 internal MDIO buses for 2 > different types of Ethernet PHYs. Document a container node called > "mdios" which has 2 subnodes "mdio@0" and "mdio@1", identifiable via > compatible string, under which the driver finds the internal PHYs. > > Cc: Rob Herring <robh@kernel.org> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > Changes in v2: > Patch is new. > > .../bindings/net/dsa/nxp,sja1105.yaml | 43 +++++++++++++++++++ > 1 file changed, 43 insertions(+) Please use get_maintainers.pl and resend to the right lists. Specifically, the DT list in this case. > diff --git a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml > index c1f18849a54a..640da65b0f59 100644 > --- a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml > @@ -28,10 +28,53 @@ properties: > - nxp,sja1105q > - nxp,sja1105r > - nxp,sja1105s > + - nxp,sja1110a > + - nxp,sja1110b > + - nxp,sja1110c > + - nxp,sja1110d > > reg: > maxItems: 1 > > + # Optional container node for the 2 internal MDIO buses of the SJA1110 > + # (one for the internal 100base-T1 PHYs and the other for the single > + # 100base-TX PHY). The "reg" property does not have physical significance. > + # The PHY addresses to port correspondence is as follows: for 100base-T1, > + # port 5 has PHY 1, port 6 has PHY 2 etc, while for 100base-TX, port 1 has > + # PHY 1. > + mdios: > + type: object > + > + properties: > + '#address-cells': > + const: 1 > + '#size-cells': > + const: 0 > + > + patternProperties: > + "^mdio@[0-1]$": > + type: object > + > + allOf: > + - $ref: "http://devicetree.org/schemas/net/mdio.yaml#" > + > + properties: > + compatible: > + oneOf: > + - enum: Don't need oneOf when there is only 1 entry. > + - nxp,sja1110-base-t1-mdio > + - nxp,sja1110-base-tx-mdio > + > + reg: > + oneOf: > + - enum: > + - 0 > + - 1 > + > + required: > + - compatible > + - reg > + > patternProperties: > "^(ethernet-)?ports$": > type: object > -- > 2.25.1 >
On Wed, May 26, 2021 at 04:55:24PM +0300, Vladimir Oltean wrote: > - const struct sja1105_regs *regs = priv->info->regs; > + u64 addr = (mmd << 16) | pcs_reg; What is the reason for using "u64" here. pcs_reg is 16-bits, and mmd is five bits, which is well below 32 bits. So, why not u32?
On Wed, May 26, 2021 at 06:34:47PM +0300, Vladimir Oltean wrote: > Hi Russell, > > On Wed, May 26, 2021 at 04:24:54PM +0100, Russell King (Oracle) wrote: > > On Wed, May 26, 2021 at 04:55:24PM +0300, Vladimir Oltean wrote: > > > - const struct sja1105_regs *regs = priv->info->regs; > > > + u64 addr = (mmd << 16) | pcs_reg; > > > > What is the reason for using "u64" here. pcs_reg is 16-bits, and mmd is > > five bits, which is well below 32 bits. So, why not u32? > > The "addr" variable holds a SPI address, and in the sja1105 driver, the > SPI addresses are universally held in u64 variables, mainly because of > the packing() API (Documentation/core-api/packing.rst). As you are passing it into a function, the argument of which is a u64, the compiler will promote the u32 to a u64 by itself. I guess it doesn't actually matter, but the current code just looks really weird.
This patch series is sent as RFC and based on linux-next because it depends on some changes which are in "net" but not in "net-next". Changes in v2: - converted nxp,sja1105 DT bindings to YAML - registered the PCS MDIO bus and forced auto-probing off for all PHY addresses for this bus - changed the container node name for the 2 MDIO buses from "mdio" to "mdios" to avoid matching on the mdio.yaml schema (it's just a container node, not an MDIO bus) - fixed an uninitialized "offset" variable usage in sja1110_pcs_mdio_{read,write} - using the mdiobus_c45_addr macro instead of open-coding that operation Reason for reposting so early: Would like some feedback on the DT bindings for the internal MDIO buses. Feedback from v1 not addressed: (Q) Can the Synopsys PCS initialization code be moved into drivers/net/pcs/xpcs.c? (A) Yes and no. Initializing the PCS is not sufficient for proper SGMII/2500base-x operation, one also needs to initialize the non-Synopsys PMA/PMD, which is accessible through the same struct mdio_device as the PCS itself. (Q) No interrupts for the internal PHYs? (A) In a later patch series (this one is already large), and only in a reduced functionality mode, where the switch driver registers an irqchip but it busy polls the interrupt status register. The board I am working on does not have the switch interrupt pin wired. Previous cover letter: The NXP SJA1110 is an automotive Ethernet switch with an embedded Arm Cortex-M7 microcontroller. The switch has 11 ports (10 external + one for the DSA-style connection to the microcontroller). The microcontroller can be disabled and the switch can be controlled over SPI, a la SJA1105 - this is how this driver handles things. There are some integrated NXP PHYs (100base-T1 and 100base-TX). Their initialization is handled by their own PHY drivers, the switch is only concerned with enabling register accesses to them, by registering two MDIO buses. Cc: Russell King <linux@armlinux.org.uk> Cc: Rob Herring <robh@kernel.org> Vladimir Oltean (14): net: dsa: sja1105: be compatible with "ethernet-ports" OF node name net: dsa: sja1105: allow SGMII PCS configuration to be per port net: dsa: sja1105: the 0x1F0000 SGMII "base address" is actually MDIO_MMD_VEND2 net: dsa: sja1105: cache the phy-mode port property net: dsa: sja1105: add a PHY interface type compatibility matrix net: dsa: sja1105: add a translation table for port speeds net: dsa: sja1105: always keep RGMII ports in the MAC role net: dsa: sja1105: some table entries are always present when read dynamically dt-bindings: net: dsa: sja1105: convert to YAML schema dt-bindings: net: dsa: sja1105: add SJA1110 bindings net: dsa: sja1105: add support for the SJA1110 switch family net: dsa: sja1105: register the MDIO buses for 100base-T1 and 100base-TX net: dsa: sja1105: expose the SGMII PCS as an mdio_device net: dsa: sja1105: add support for the SJA1110 SGMII/2500base-x PCS .../bindings/net/dsa/nxp,sja1105.yaml | 172 ++++++ .../devicetree/bindings/net/dsa/sja1105.txt | 156 ----- drivers/net/dsa/sja1105/Makefile | 1 + drivers/net/dsa/sja1105/sja1105.h | 88 ++- drivers/net/dsa/sja1105/sja1105_clocking.c | 120 +++- .../net/dsa/sja1105/sja1105_dynamic_config.c | 336 ++++++++++- .../net/dsa/sja1105/sja1105_dynamic_config.h | 1 + drivers/net/dsa/sja1105/sja1105_main.c | 518 +++++++++++++---- drivers/net/dsa/sja1105/sja1105_mdio.c | 539 ++++++++++++++++++ drivers/net/dsa/sja1105/sja1105_sgmii.h | 63 +- drivers/net/dsa/sja1105/sja1105_spi.c | 368 +++++++++++- .../net/dsa/sja1105/sja1105_static_config.c | 483 ++++++++++++++++ .../net/dsa/sja1105/sja1105_static_config.h | 98 +++- 13 files changed, 2619 insertions(+), 324 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml delete mode 100644 Documentation/devicetree/bindings/net/dsa/sja1105.txt create mode 100644 drivers/net/dsa/sja1105/sja1105_mdio.c