Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1482078

Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family

Show all headers | View raw

> +static int
> +qca8k_setup(struct dsa_switch *ds)
> +{
> +	struct qca8k_priv *priv = qca8k_to_priv(ds);
> +	int ret, i, phy_mode = -1;
> +
> +	/* Keep track of which port is the connected to the cpu. This can be
> +	 * phy 11 / port 0 or phy 5 / port 6.
> +	 */
> +	switch (dsa_upstream_port(ds)) {
> +	case 11:
> +		priv->cpu_port = 0;
> +		break;
> +	case 5:
> +		priv->cpu_port = 6;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}

Hi John

Can you explain this funkiness with CPU port numbers. Why use 11
instead of 0? I ideally i would like to use 0 here, but if that it not
possible, it needs documenting in the device tree binding that 5 and
11 are special, representing ports 0 and 6.

> +
> +	/* Start by setting up the register mapping */
> +	priv->regmap = devm_regmap_init(ds->dev, NULL, priv,
> +					&qca8k_regmap_config);
> +
> +	if (IS_ERR(priv->regmap))
> +		pr_warn("regmap initialization failed");
> +
> +	/* Initialize CPU port pad mode (xMII type, delays...) */
> +	phy_mode = of_get_phy_mode(ds->ports[ds->dst->cpu_port].dn);
> +	if (phy_mode < 0) {
> +		pr_err("Can't find phy-mode for master device\n");
> +		return phy_mode;
> +	}
> +	ret = qca8k_set_pad_ctrl(priv, priv->cpu_port, phy_mode);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable CPU Port */
> +	qca8k_reg_set(priv, QCA8K_REG_GLOBAL_FW_CTRL0,
> +		      QCA8K_GLOBAL_FW_CTRL0_CPU_PORT_EN);

Does it matter which port is the CPU port here? 11 or 5

> +
> +	/* Enable MIB counters */
> +	qca8k_reg_set(priv, QCA8K_REG_MIB, QCA8K_MIB_CPU_KEEP);
> +	qca8k_write(priv, QCA8K_REG_MODULE_EN, QCA8K_MODULE_EN_MIB);
> +
> +	/* Enable QCA header mode on Port 0 */
> +	qca8k_write(priv, QCA8K_REG_PORT_HDR_CTRL(priv->cpu_port),
> +		    QCA8K_PORT_HDR_CTRL_ALL << QCA8K_PORT_HDR_CTRL_TX_S |
> +		    QCA8K_PORT_HDR_CTRL_ALL << QCA8K_PORT_HDR_CTRL_RX_S);

Does the header mode only work on port 0?

> +static int
> +qca8k_phy_read(struct dsa_switch *ds, int phy, int regnum)
> +{
> +	struct qca8k_priv *priv = qca8k_to_priv(ds);
> +
> +	return mdiobus_read(priv->bus, phy, regnum);
> +}
> +
> +static int
> +qca8k_phy_write(struct dsa_switch *ds, int phy, int regnum, u16 val)
> +{
> +	struct qca8k_priv *priv = qca8k_to_priv(ds);
> +
> +	return mdiobus_write(priv->bus, phy, regnum, val);
> +}

snip

> +static int
> +qca8k_sw_probe(struct mdio_device *mdiodev)
> +{
> +	struct qca8k_priv *priv;
> +	u32 phy_id;
> +
> +	/* sw_addr is irrelevant as the switch occupies the MDIO bus from
> +	 * addresses 0 to 4 (PHYs) and 16-23 (for MDIO 32bits protocol). So
> +	 * we'll probe address 0 to see if we see the right switch family.
> +	 */

So lets see if i have this right.

Port 0 has no internal phy.
Port 1 has an internal PHY at MDIO address 0.
Port 2 has an internal PHY at MDIO address 1.
...
Port 5 has an internal PHY ad MDIO address 4.
Port 6 has no internal PHY.

This is why you have funky port numbers, and phy_to_port.

I think it would be a lot cleaner to handle this in qca8k_phy_read()
and qca8k_phy_write(). 

Also, the comment it a bit misleading. You are probing the PHY ID, not
the switch ID. At least for the Marvell switches, different switches
can have the same embedded PHY. It would be annoying to find there is
another incompatible switch with the same PHY ID.

Is the embedded PHY compatible with the at803x driver?

   Thanks
	Andrew

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/3] net-next: dsa: add QCA8K support John Crispin <john@phrozen.org> - 2016-09-12 10:40 +0200
  [PATCH 2/3] net-next: dsa: add Qualcomm tag RX/TX handler John Crispin <john@phrozen.org> - 2016-09-12 10:40 +0200
    Re: [PATCH 2/3] net-next: dsa: add Qualcomm tag RX/TX handler Andrew Lunn <andrew@lunn.ch> - 2016-09-12 14:30 +0200
  [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family John Crispin <john@phrozen.org> - 2016-09-12 10:40 +0200
    Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family Andrew Lunn <andrew@lunn.ch> - 2016-09-12 15:20 +0200
    Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family Andrew Lunn <andrew@lunn.ch> - 2016-09-13 01:00 +0200
    Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family Andrew Lunn <andrew@lunn.ch> - 2016-09-13 02:50 +0200
      Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family John Crispin <john@phrozen.org> - 2016-09-13 10:10 +0200
        Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family Andrew Lunn <andrew@lunn.ch> - 2016-09-13 18:00 +0200
          Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family Florian Fainelli <f.fainelli@gmail.com> - 2016-09-13 19:20 +0200
            Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family John Crispin <john@phrozen.org> - 2016-09-13 20:10 +0200
              Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family Florian Fainelli <f.fainelli@gmail.com> - 2016-09-13 20:20 +0200
    Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family Andrew Lunn <andrew@lunn.ch> - 2016-09-13 03:30 +0200
      Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family John Crispin <john@phrozen.org> - 2016-09-13 11:50 +0200
        Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family Andrew Lunn <andrew@lunn.ch> - 2016-09-13 15:20 +0200
          Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-09-13 19:20 +0200
            Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family Andrew Lunn <andrew@lunn.ch> - 2016-09-13 21:10 +0200
              Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family John Crispin <john@phrozen.org> - 2016-09-13 21:20 +0200

csiph-web