lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ