diff mbox

[U-Boot,v1,2/2] sun8i_emac: remove unnecessary bit-masking for mdio_read/write

Message ID 42798afdd123e423817919a9d507894b463cc4d8.1487340707.git.philipp.tomsich@theobroma-systems.com
State Rejected
Delegated to: Jagannadha Sutradharudu Teki
Headers show

Commit Message

Philipp Tomsich Feb. 17, 2017, 5:46 p.m. UTC
The MDIO read/write builds up the MII_CMD register from scratch (starting
with a value of 0). No need to mask out any fields before writing the new
values.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---
 drivers/net/sun8i_emac.c | 8 --------
 1 file changed, 8 deletions(-)

Comments

Andre Przywara Feb. 17, 2017, 11:49 p.m. UTC | #1
On 17/02/17 17:46, Philipp Tomsich wrote:
> The MDIO read/write builds up the MII_CMD register from scratch (starting
> with a value of 0). No need to mask out any fields before writing the new
> values.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

As a general comment: Can you configure your setup to use the standard
three lines of context? Reading over those big gaps is rather confusing
and may lead to changes being missed.
Thanks!

Cheers,
Andre.

> ---
>  drivers/net/sun8i_emac.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> index 5ae17b7..5094dd8 100644
> --- a/drivers/net/sun8i_emac.c
> +++ b/drivers/net/sun8i_emac.c
> @@ -139,74 +139,66 @@ struct emac_eth_dev {
>  static int sun8i_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
>  {
>  	struct emac_eth_dev *priv = bus->priv;
>  	ulong start;
>  	u32 miiaddr = 0;
>  	int timeout = CONFIG_MDIO_TIMEOUT;
>  
> -	miiaddr &= ~MDIO_CMD_MII_WRITE;
> -	miiaddr &= ~MDIO_CMD_MII_PHY_REG_ADDR_MASK;
>  	miiaddr |= (reg << MDIO_CMD_MII_PHY_REG_ADDR_SHIFT) &
>  		MDIO_CMD_MII_PHY_REG_ADDR_MASK;
> -
> -	miiaddr &= ~MDIO_CMD_MII_PHY_ADDR_MASK;
> -
>  	miiaddr |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
>  		MDIO_CMD_MII_PHY_ADDR_MASK;
>  
>  	/* The MAC block is fed by a 300MHz clock, so we need to divide by 128
>  	   to bring the MDC into the range permissible by the IEEE standard. */
>  	miiaddr |= MDIO_CMD_MDC_DIV_128;
>  
>  	miiaddr |= MDIO_CMD_MII_BUSY;
>  
>  	writel(miiaddr, priv->mac_reg + EMAC_MII_CMD);
>  
>  	start = get_timer(0);
>  	while (get_timer(start) < timeout) {
>  		if (!(readl(priv->mac_reg + EMAC_MII_CMD) & MDIO_CMD_MII_BUSY))
>  			return readl(priv->mac_reg + EMAC_MII_DATA);
>  		udelay(10);
>  	};
>  
>  	return -1;
>  }
>  
>  static int sun8i_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
>  			    u16 val)
>  {
>  	struct emac_eth_dev *priv = bus->priv;
>  	ulong start;
>  	u32 miiaddr = 0;
>  	int ret = -1, timeout = CONFIG_MDIO_TIMEOUT;
>  
> -	miiaddr &= ~MDIO_CMD_MII_PHY_REG_ADDR_MASK;
>  	miiaddr |= (reg << MDIO_CMD_MII_PHY_REG_ADDR_SHIFT) &
>  		MDIO_CMD_MII_PHY_REG_ADDR_MASK;
> -
> -	miiaddr &= ~MDIO_CMD_MII_PHY_ADDR_MASK;
>  	miiaddr |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
>  		MDIO_CMD_MII_PHY_ADDR_MASK;
>  
>  	miiaddr |= MDIO_CMD_MII_WRITE;
>  	miiaddr |= MDIO_CMD_MII_BUSY;
>  
>  	/* The MAC block is fed by a 300MHz clock, so we need to divide by 128
>  	   to bring the MDC into the range permissible by the IEEE standard. */
>  	miiaddr |= MDIO_CMD_MDC_DIV_128;
>  
>  	writel(val, priv->mac_reg + EMAC_MII_DATA);
>  	writel(miiaddr, priv->mac_reg + EMAC_MII_CMD);
>  
>  	start = get_timer(0);
>  	while (get_timer(start) < timeout) {
>  		if (!(readl(priv->mac_reg + EMAC_MII_CMD) &
>  					MDIO_CMD_MII_BUSY)) {
>  			ret = 0;
>  			break;
>  		}
>  		udelay(10);
>  	};
>  
>  	return ret;
>  }
>  
>
diff mbox

Patch

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index 5ae17b7..5094dd8 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -139,74 +139,66 @@  struct emac_eth_dev {
 static int sun8i_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
 {
 	struct emac_eth_dev *priv = bus->priv;
 	ulong start;
 	u32 miiaddr = 0;
 	int timeout = CONFIG_MDIO_TIMEOUT;
 
-	miiaddr &= ~MDIO_CMD_MII_WRITE;
-	miiaddr &= ~MDIO_CMD_MII_PHY_REG_ADDR_MASK;
 	miiaddr |= (reg << MDIO_CMD_MII_PHY_REG_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_REG_ADDR_MASK;
-
-	miiaddr &= ~MDIO_CMD_MII_PHY_ADDR_MASK;
-
 	miiaddr |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_ADDR_MASK;
 
 	/* The MAC block is fed by a 300MHz clock, so we need to divide by 128
 	   to bring the MDC into the range permissible by the IEEE standard. */
 	miiaddr |= MDIO_CMD_MDC_DIV_128;
 
 	miiaddr |= MDIO_CMD_MII_BUSY;
 
 	writel(miiaddr, priv->mac_reg + EMAC_MII_CMD);
 
 	start = get_timer(0);
 	while (get_timer(start) < timeout) {
 		if (!(readl(priv->mac_reg + EMAC_MII_CMD) & MDIO_CMD_MII_BUSY))
 			return readl(priv->mac_reg + EMAC_MII_DATA);
 		udelay(10);
 	};
 
 	return -1;
 }
 
 static int sun8i_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
 			    u16 val)
 {
 	struct emac_eth_dev *priv = bus->priv;
 	ulong start;
 	u32 miiaddr = 0;
 	int ret = -1, timeout = CONFIG_MDIO_TIMEOUT;
 
-	miiaddr &= ~MDIO_CMD_MII_PHY_REG_ADDR_MASK;
 	miiaddr |= (reg << MDIO_CMD_MII_PHY_REG_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_REG_ADDR_MASK;
-
-	miiaddr &= ~MDIO_CMD_MII_PHY_ADDR_MASK;
 	miiaddr |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_ADDR_MASK;
 
 	miiaddr |= MDIO_CMD_MII_WRITE;
 	miiaddr |= MDIO_CMD_MII_BUSY;
 
 	/* The MAC block is fed by a 300MHz clock, so we need to divide by 128
 	   to bring the MDC into the range permissible by the IEEE standard. */
 	miiaddr |= MDIO_CMD_MDC_DIV_128;
 
 	writel(val, priv->mac_reg + EMAC_MII_DATA);
 	writel(miiaddr, priv->mac_reg + EMAC_MII_CMD);
 
 	start = get_timer(0);
 	while (get_timer(start) < timeout) {
 		if (!(readl(priv->mac_reg + EMAC_MII_CMD) &
 					MDIO_CMD_MII_BUSY)) {
 			ret = 0;
 			break;
 		}
 		udelay(10);
 	};
 
 	return ret;
 }