Re: [net-next 3/3] net: ftgmac100: Support for AST2700

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>  	/* Setup RX ring buffer base */
> -	iowrite32(priv->rxdes_dma, priv->base + FTGMAC100_OFFSET_RXR_BADR);
> +	iowrite32(lower_32_bits(priv->rxdes_dma), priv->base + FTGMAC100_OFFSET_RXR_BADR);
> +	iowrite32(upper_32_bits(priv->rxdes_dma), priv->base + FTGMAC100_OFFSET_RXR_BADDR_HIGH);

This appears to write to registers which older generations do not
have. Is this safe? Is it defined in the datasheet what happens when
you write to reserved registers?

>  	/* Store DMA address into RX desc */
> -	rxdes->rxdes3 = cpu_to_le32(map);
> +	rxdes->rxdes2 = FIELD_PREP(FTGMAC100_RXDES2_RXBUF_BADR_HI, upper_32_bits(map));
> +	rxdes->rxdes3 = lower_32_bits(map);

Maybe update the comment:
        unsigned int    rxdes3; /* not used by HW */

Also, should its type be changed to __le32 ?

> -	map = le32_to_cpu(rxdes->rxdes3);
> +	map = le32_to_cpu(rxdes->rxdes3) | ((rxdes->rxdes2 & FTGMAC100_RXDES2_RXBUF_BADR_HI) << 16);

Is this safe? You have to assume older generation of devices will
return 42 in rxdes3, since it is not used by the hardware.

>  	/* Mark the end of the ring */
>  	rxdes->rxdes0 |= cpu_to_le32(priv->rxdes0_edorr_mask);
> @@ -1249,7 +1266,6 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget)
>  		more = ftgmac100_rx_packet(priv, &work_done);
>  	} while (more && work_done < budget);
>  
> -
>  	/* The interrupt is telling us to kick the MAC back to life
>  	 * after an RX overflow
>  	 */
> @@ -1339,7 +1355,6 @@ static void ftgmac100_reset(struct ftgmac100 *priv)
>  	if (priv->mii_bus)
>  		mutex_lock(&priv->mii_bus->mdio_lock);
>  
> -
>  	/* Check if the interface is still up */
>  	if (!netif_running(netdev))
>  		goto bail;
> @@ -1438,7 +1453,6 @@ static void ftgmac100_adjust_link(struct net_device *netdev)
>  
>  	if (netdev->phydev)
>  		mutex_lock(&netdev->phydev->lock);
> -
>  }

There are quite a few whitespace changes like this in this
patch. Please remove them. If they are important, put them into a
patch of there own.

> +	dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));

This can fail, you should check the return value.

	Andrew




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux