[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <fd921f50-075d-c39e-b22a-7de296e59038@rempel-privat.de>
Date: Mon, 2 Mar 2020 12:07:59 +0100
From: Oleksij Rempel <linux@...pel-privat.de>
To: Hauke Mehrtens <hauke@...ke-m.de>, davem@...emloft.net
Cc: netdev@...r.kernel.org, chris.snook@...il.com, jcliburn@...il.com
Subject: Re: [PATCH 2/2] ag71xx: Configure Ethernet interface
Am 02.03.20 um 01:18 schrieb Hauke Mehrtens:
> Configure the Ethernet interface when the register range is provided.
>
> The GMAC0 of the AR9330
I was not able to find any ar9330 documentation or chip. Are there any the wild? The AR9331 do not
have support for external PHYs. RGMII on GMAC0 seems to be not supported or not working. At least,
currently I was not able to get it in working state.
> and AR9340 can be operated in different modes,
> like MII, RMII, GMII and RGMII. This allows to configure this mode in
> the interface register block.
>
> Signed-off-by: Hauke Mehrtens <hauke@...ke-m.de>
> ---
> drivers/net/ethernet/atheros/ag71xx.c | 76 +++++++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
>
> diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
> index 69125f870363..7ef16fdd6617 100644
> --- a/drivers/net/ethernet/atheros/ag71xx.c
> +++ b/drivers/net/ethernet/atheros/ag71xx.c
> @@ -64,6 +64,14 @@
> #define AG71XX_MDIO_DELAY 5
> #define AG71XX_MDIO_MAX_CLK 5000000
>
> +/* GMAC Interface Registers */
> +#define AG71XX_GMAC_REG_ETH_CFG 0x00
> +#define ETH_CFG_RGMII_GE0 BIT(0)
> +#define ETH_CFG_MII_GE0 BIT(1)
> +#define ETH_CFG_GMII_GE0 BIT(2)
> +#define ETH_CFG_RMII_GE0_AR933X BIT(9)
> +#define ETH_CFG_RMII_GE0_AR934X BIT(10)
According to the ar9344 doc this bit is MII_CNTL_SPEED, not RMII.
> +
> /* Register offsets */
> #define AG71XX_REG_MAC_CFG1 0x0000
> #define MAC_CFG1_TXE BIT(0) /* Tx Enable */
> @@ -311,6 +319,8 @@ struct ag71xx {
> /* From this point onwards we're not looking at per-packet fields. */
> void __iomem *mac_base;
>
> + void __iomem *inf_base;
please use regmap instead.
> +
> struct ag71xx_desc *stop_desc;
> dma_addr_t stop_desc_dma;
>
> @@ -364,6 +374,18 @@ static u32 ag71xx_rr(struct ag71xx *ag, unsigned int reg)
> return ioread32(ag->mac_base + reg);
> }
>
> +static void ag71xx_inf_wr(struct ag71xx *ag, unsigned int reg, u32 value)
> +{
> + iowrite32(value, ag->inf_base + reg);
> + /* flush write */
> + (void)ioread32(ag->inf_base + reg);
> +}
> +
> +static u32 ag71xx_inf_rr(struct ag71xx *ag, unsigned int reg)
> +{
> + return ioread32(ag->inf_base + reg);
> +}
> +
> static void ag71xx_sb(struct ag71xx *ag, unsigned int reg, u32 mask)
> {
> void __iomem *r;
> @@ -848,6 +870,52 @@ static void ag71xx_hw_start(struct ag71xx *ag)
> netif_wake_queue(ag->ndev);
> }
>
> +static void ag71xx_mac_config_inf(struct ag71xx *ag,
> + const struct phylink_link_state *state)
> +{
> + u32 val;
> +
> + if (!ag71xx_is(ag, AR9330) && !ag71xx_is(ag, AR9340))
> + return;
We should do complete validate (port, chip and so on) here again, or do proper validation in
ag71xx_mac_validate() and trust provided results here.
I would prefer to do proper validation only one time.
> +
> + val = ag71xx_inf_rr(ag, AG71XX_GMAC_REG_ETH_CFG);
> +
> + val &= ~ETH_CFG_MII_GE0;
> + val &= ~ETH_CFG_GMII_GE0;
> + val &= ~ETH_CFG_RGMII_GE0;
Better val &= ~(ETH_CFG_MII_GE0 | ETH_CFG_GMII_GE0 ...)
> + if (ag71xx_is(ag, AR9330))
> + val &= ~ETH_CFG_RMII_GE0_AR933X;
> + else
> + val &= ~ETH_CFG_RMII_GE0_AR934X;
> +
> + switch (state->interface) {
> + case PHY_INTERFACE_MODE_MII:
> + val |= ETH_CFG_MII_GE0;
> + break;
> + case PHY_INTERFACE_MODE_RMII:
> + if (ag71xx_is(ag, AR9330))
> + val |= ETH_CFG_RMII_GE0_AR933X;
> + else
> + val |= ETH_CFG_RMII_GE0_AR934X;
> + break;
> + case PHY_INTERFACE_MODE_GMII:
> + val |= ETH_CFG_GMII_GE0;
> + break;
> + case PHY_INTERFACE_MODE_RGMII:
> + case PHY_INTERFACE_MODE_RGMII_ID:
> + case PHY_INTERFACE_MODE_RGMII_RXID:
> + case PHY_INTERFACE_MODE_RGMII_TXID:
> + val |= ETH_CFG_RGMII_GE0;
> + break;
> + default:
> + netif_err(ag, link, ag->ndev,
> + "Unsupported interface: %d\n",
> + state->interface);
> + return;
> + }
> + ag71xx_inf_wr(ag, AG71XX_GMAC_REG_ETH_CFG, val);
> +}
> +
> static void ag71xx_mac_config(struct phylink_config *config, unsigned int mode,
> const struct phylink_link_state *state)
> {
> @@ -859,6 +927,9 @@ static void ag71xx_mac_config(struct phylink_config *config, unsigned int mode,
> if (!ag71xx_is(ag, AR7100) && !ag71xx_is(ag, AR9130))
> ag71xx_fast_reset(ag);
>
> + if (ag->inf_base)
> + ag71xx_mac_config_inf(ag, state);
> +
> if (ag->tx_ring.desc_split) {
> ag->fifodata[2] &= 0xffff;
> ag->fifodata[2] |= ((2048 - ag->tx_ring.desc_split) / 4) << 16;
> @@ -1703,6 +1774,11 @@ static int ag71xx_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> + /* The interface resource is optional */
> + ag->inf_base = devm_platform_ioremap_resource_byname(pdev, "interface");
> + if (IS_ERR(ag->inf_base) && PTR_ERR(ag->inf_base) != -ENOENT)
> + return PTR_ERR(ag->inf_base);
Please use regmap, this register should be shared with switch driver at least on ar9331. You will
need to define separate devicetree node for ETH_CFG register and link it to the eth0 node.
> ag->clk_eth = devm_clk_get(&pdev->dev, "eth");
> if (IS_ERR(ag->clk_eth)) {
> netif_err(ag, probe, ndev, "Failed to get eth clk.\n");
>
--
Regards,
Oleksij
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists