lkml.org 
[lkml]   [2022]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH net-next v8 05/16] net: pcs: add Renesas MII converter driver
    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@bootlin.com>
    > ---

    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@gmail.com>

    > +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);
    > +}

    \
     
     \ /
      Last update: 2022-06-21 13:45    [W:5.642 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site