On Mon, Jun 20, 2022 at 01:08:35PM +0200, Clément Léger wrote:
> Add a PCS driver for the MII converter that is present on the Renesas
> RZ/N1 SoC. This MII converter is reponsible for converting MII to
> RMII/RGMII or act as a MII pass-trough. Exposing it as a PCS allows to
> reuse it in both the switch driver and the stmmac driver. Currently,
> this driver only allows the PCS to be used by the dual Cortex-A7
> subsystem since the register locking system is not used.
>
> Signed-off-by: Clément Léger <clement.leger@xxxxxxxxxxx>
> ---
Pretty cool driver. I understand this to be more or less the same thing
as drivers/net/phy/xilinx_gmii2rgmii.c in principle, but this appears
nicer done and I'm glad you didn't follow the same model (the
phylink_pcs seems to be a much better fit than a chained PHY).
If PHY library maintainers don't have any objections you can add my:
Reviewed-by: Vladimir Oltean <olteanv@xxxxxxxxx>
> +static void miic_reg_rmw(struct miic *miic, int offset, u32 mask, u32 val)
> +{
> + u32 reg;
> +
> + spin_lock(&miic->lock);
> +
> + reg = miic_reg_readl(miic, offset);
> + reg &= ~mask;
> + reg |= val;
> + miic_reg_writel(miic, offset, reg);
> +
> + spin_unlock(&miic->lock);
> +}
Just a small comment: I don't think pcs_config and pcs_link_up need
serialization with respect to each other, so this read-modify-write
spinlock doesn't do much. But it doesn't really hurt either.
> +
> +static void miic_converter_enable(struct miic *miic, int port, int enable)
> +{
> + u32 val = 0;
> +
> + if (enable)
> + val = MIIC_CONVRST_PHYIF_RST(port);
> +
> + miic_reg_rmw(miic, MIIC_CONVRST, MIIC_CONVRST_PHYIF_RST(port), val);
> +}
> +
> +static int miic_config(struct phylink_pcs *pcs, unsigned int mode,
> + phy_interface_t interface,
> + const unsigned long *advertising, bool permit)
> +{
> + struct miic_port *miic_port = phylink_pcs_to_miic_port(pcs);
> + struct miic *miic = miic_port->miic;
> + int port = miic_port->port;
> + u32 speed, conv_mode, val;
> +
> + switch (interface) {
> + case PHY_INTERFACE_MODE_RMII:
> + conv_mode = CONV_MODE_RMII;
> + speed = CONV_MODE_100MBPS;
> + break;
> + case PHY_INTERFACE_MODE_RGMII:
> + case PHY_INTERFACE_MODE_RGMII_ID:
> + case PHY_INTERFACE_MODE_RGMII_TXID:
> + case PHY_INTERFACE_MODE_RGMII_RXID:
> + conv_mode = CONV_MODE_RGMII;
> + speed = CONV_MODE_1000MBPS;
> + break;
> + case PHY_INTERFACE_MODE_MII:
> + conv_mode = CONV_MODE_MII;
> + /* When in MII mode, speed should be set to 0 (which is actually
> + * CONV_MODE_10MBPS)
> + */
> + speed = CONV_MODE_10MBPS;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + val = FIELD_PREP(MIIC_CONVCTRL_CONV_MODE, conv_mode) |
> + FIELD_PREP(MIIC_CONVCTRL_CONV_SPEED, speed);
> +
> + miic_reg_rmw(miic, MIIC_CONVCTRL(port),
> + MIIC_CONVCTRL_CONV_MODE | MIIC_CONVCTRL_CONV_SPEED, val);
> + miic_converter_enable(miic_port->miic, miic_port->port, 1);
> +
> + return 0;
> +}
> +
> +static void miic_link_up(struct phylink_pcs *pcs, unsigned int mode,
> + phy_interface_t interface, int speed, int duplex)
> +{
> + struct miic_port *miic_port = phylink_pcs_to_miic_port(pcs);
> + struct miic *miic = miic_port->miic;
> + u32 conv_speed = 0, val = 0;
> + int port = miic_port->port;
> +
> + if (duplex == DUPLEX_FULL)
> + val |= MIIC_CONVCTRL_FULLD;
> +
> + /* No speed in MII through-mode */
> + if (interface != PHY_INTERFACE_MODE_MII) {
> + switch (speed) {
> + case SPEED_1000:
> + conv_speed = CONV_MODE_1000MBPS;
> + break;
> + case SPEED_100:
> + conv_speed = CONV_MODE_100MBPS;
> + break;
> + case SPEED_10:
> + conv_speed = CONV_MODE_10MBPS;
> + break;
> + default:
> + return;
> + }
> + }
> +
> + val |= FIELD_PREP(MIIC_CONVCTRL_CONV_SPEED, conv_speed);
> +
> + miic_reg_rmw(miic, MIIC_CONVCTRL(port),
> + (MIIC_CONVCTRL_CONV_SPEED | MIIC_CONVCTRL_FULLD), val);
> +}
