(unknown),

(unknown),

[Jonas Bonn]

Two patches for the ethoc driver and one to the Micrel PHY driver.

I'd like feedback on the patch 1/3 (write bus addresses to registers) as I'm 
not totally confident that this won't break for someone else.  The MAC registers
should definitely be getting bus/physical addresses and ->membase is thus
the wrong value to be using (it's virtual); however, since I presume that 
this driver was working for somebody else before (???), then I'd like to know
that whatever use case they have isn't broken by this -- what configuration
would have ->membase == ->mem_start anyway?  NO_MMU?

The other two patches are pretty trivial.

Thanks,
Jonas

[PATCH 1/3] ethoc: Write bus addresses to registers

[Jonas Bonn]

The ethoc driver should be writing bus addresses to the ethoc registers, not
virtual addresses.

Also, use bus_to_virt instead of phys_to_virt to make this explicit.
---
 drivers/net/ethoc.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index a8d9250..41bded6 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -283,7 +283,7 @@ static inline void ethoc_disable_rx_and_tx(struct ethoc *dev)
 	ethoc_write(dev, MODER, mode);
 }
 
-static int ethoc_init_ring(struct ethoc *dev)
+static int ethoc_init_ring(struct ethoc *dev, void* mem_start)
 {
 	struct ethoc_bd bd;
 	int i;
@@ -293,7 +293,7 @@ static int ethoc_init_ring(struct ethoc *dev)
 	dev->cur_rx = 0;
 
 	/* setup transmission buffers */
-	bd.addr = virt_to_phys(dev->membase);
+	bd.addr = mem_start;
 	bd.stat = TX_BD_IRQ | TX_BD_CRC;
 
 	for (i = 0; i < dev->num_tx; i++) {
@@ -413,7 +413,7 @@ static int ethoc_rx(struct net_device *dev, int limit)
 			skb = netdev_alloc_skb_ip_align(dev, size);
 
 			if (likely(skb)) {
-				void *src = phys_to_virt(bd.addr);
+				void *src = bus_to_virt(bd.addr);
 				memcpy_fromio(skb_put(skb, size), src, size);
 				skb->protocol = eth_type_trans(skb, dev);
 				priv->stats.rx_packets++;
@@ -672,7 +672,7 @@ static int ethoc_open(struct net_device *dev)
 	priv->num_rx = num_bd - priv->num_tx;
 	ethoc_write(priv, TX_BD_NUM, priv->num_tx);
 
-	ethoc_init_ring(priv);
+	ethoc_init_ring(priv, dev->mem_start);
 	ethoc_reset(priv);
 
 	if (netif_queue_stopped(dev)) {
@@ -836,7 +836,7 @@ static netdev_tx_t ethoc_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	else
 		bd.stat &= ~TX_BD_PAD;
 
-	dest = phys_to_virt(bd.addr);
+	dest = bus_to_virt(bd.addr);
 	memcpy_toio(dest, skb->data, skb->len);
 
 	bd.stat &= ~(TX_BD_STATS | TX_BD_LEN_MASK);
-- 
1.7.0.4

[PATCH 2/3] ethoc: Clean up PHY probing

[Jonas Bonn]

- No need to iterate over all possible addresses on bus
- Use helper function phy_find_first
- Use phy_connect_direct as we already have the relevant structure
---
 drivers/net/ethoc.c |   24 ++++++++----------------
 1 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 41bded6..4bccae2 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -620,21 +620,13 @@ static int ethoc_mdio_probe(struct net_device *dev)
 {
 	struct ethoc *priv = netdev_priv(dev);
 	struct phy_device *phy;
+	int err;
 	int i;
 
-	for (i = 0; i < PHY_MAX_ADDR; i++) {
-		phy = priv->mdio->phy_map[i];
-		if (phy) {
-			if (priv->phy_id != -1) {
-				/* attach to specified PHY */
-				if (priv->phy_id == phy->addr)
-					break;
-			} else {
-				/* autoselect PHY if none was specified */
-				if (phy->addr != 0)
-					break;
-			}
-		}
+	if (priv->phy_id != -1) {
+		phy = priv->mdio->phy_map[priv->phy_id];
+	} else {
+		phy = phy_find_first(priv->mdio);
 	}
 
 	if (!phy) {
@@ -642,11 +634,11 @@ static int ethoc_mdio_probe(struct net_device *dev)
 		return -ENXIO;
 	}
 
-	phy = phy_connect(dev, dev_name(&phy->dev), ethoc_mdio_poll, 0,
+	err = phy_connect_direct(dev, phy, ethoc_mdio_poll, 0,
 			PHY_INTERFACE_MODE_GMII);
-	if (IS_ERR(phy)) {
+	if (err) {
 		dev_err(&dev->dev, "could not attach to PHY\n");
-		return PTR_ERR(phy);
+		return err;
 	}
 
 	priv->phy = phy;
-- 
1.7.0.4

[PATCH 3/3] phy: Add suspend/resume functions to Micrel PHY

[Jonas Bonn]

Use the generic suspend and resume functions, as the genphy driver does.
---
 drivers/net/phy/micrel.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index e67691d..c38a0bb 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -39,6 +39,8 @@ static struct phy_driver ks8001_driver = {
 	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
+	.suspend	= genphy_suspend,
+	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE,},
 };
 
@@ -51,6 +53,8 @@ static struct phy_driver vsc8201_driver = {
 	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
+	.suspend	= genphy_suspend,
+	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE,},
 };
 
@@ -63,6 +67,8 @@ static struct phy_driver ksz9021_driver = {
 	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
+	.suspend	= genphy_suspend,
+	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE, },
 };
 
-- 
1.7.0.4

Re: [PATCH 1/3] ethoc: Write bus addresses to registers

[David Miller]

From: Jonas Bonn <jonas@southpole.se>
Date: Tue, 18 May 2010 07:39:24 +0200

> The ethoc driver should be writing bus addresses to the ethoc registers, not
> virtual addresses.
> 
> Also, use bus_to_virt instead of phys_to_virt to make this explicit.

Portable drivers must not use bus_to_virt().

You should keep track of the virtual addresses of the mapped RX
packets in your RX ring datastructure.

[PATCH] ethoc: Write bus addresses to registers

[Jonas Bonn]

The ethoc driver should be writing bus addresses to the ethoc registers, not
virtual addresses.  This patch adds an array to store the virtual addresses
in and references that array when manipulating the contents of the buffer
descriptors.
---

This patch mainly for review.  I've tested this on my architecture-specific
branch, but this patch specifically (rebased against 2.6.35 master) has not 
been tested.

 drivers/net/ethoc.c |   27 ++++++++++++++++++++++-----
 1 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 6ed2df1..c1951f6 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -180,6 +180,7 @@ MODULE_PARM_DESC(buffer_size, "DMA buffer allocation size");
  * @dty_tx:	last buffer actually sent
  * @num_rx:	number of receive buffers
  * @cur_rx:	current receive buffer
+ * @vma:        pointer to array of virtual memory addresses for buffers
  * @netdev:	pointer to network device structure
  * @napi:	NAPI structure
  * @stats:	network device statistics
@@ -203,6 +204,8 @@ struct ethoc {
 	unsigned int num_rx;
 	unsigned int cur_rx;
 
+	void** vma;
+
 	struct net_device *netdev;
 	struct napi_struct napi;
 	struct net_device_stats stats;
@@ -285,18 +288,20 @@ static inline void ethoc_disable_rx_and_tx(struct ethoc *dev)
 	ethoc_write(dev, MODER, mode);
 }
 
-static int ethoc_init_ring(struct ethoc *dev)
+static int ethoc_init_ring(struct ethoc *dev, void* mem_start)
 {
 	struct ethoc_bd bd;
 	int i;
+	void* vma;
 
 	dev->cur_tx = 0;
 	dev->dty_tx = 0;
 	dev->cur_rx = 0;
 
 	/* setup transmission buffers */
-	bd.addr = virt_to_phys(dev->membase);
+	bd.addr = mem_start;
 	bd.stat = TX_BD_IRQ | TX_BD_CRC;
+	vma = dev->membase;
 
 	for (i = 0; i < dev->num_tx; i++) {
 		if (i == dev->num_tx - 1)
@@ -304,6 +309,9 @@ static int ethoc_init_ring(struct ethoc *dev)
 
 		ethoc_write_bd(dev, i, &bd);
 		bd.addr += ETHOC_BUFSIZ;
+
+		dev->vma[i] = vma;
+		vma += ETHOC_BUFSIZ;
 	}
 
 	bd.stat = RX_BD_EMPTY | RX_BD_IRQ;
@@ -314,6 +322,9 @@ static int ethoc_init_ring(struct ethoc *dev)
 
 		ethoc_write_bd(dev, dev->num_tx + i, &bd);
 		bd.addr += ETHOC_BUFSIZ;
+
+		dev->vma[dev->num_tx + i] = vma;
+		vma += ETHOC_BUFSIZ;
 	}
 
 	return 0;
@@ -415,7 +426,7 @@ static int ethoc_rx(struct net_device *dev, int limit)
 			skb = netdev_alloc_skb_ip_align(dev, size);
 
 			if (likely(skb)) {
-				void *src = phys_to_virt(bd.addr);
+				void *src = priv->vma[entry];
 				memcpy_fromio(skb_put(skb, size), src, size);
 				skb->protocol = eth_type_trans(skb, dev);
 				priv->stats.rx_packets++;
@@ -674,7 +685,7 @@ static int ethoc_open(struct net_device *dev)
 	priv->num_rx = num_bd - priv->num_tx;
 	ethoc_write(priv, TX_BD_NUM, priv->num_tx);
 
-	ethoc_init_ring(priv);
+	ethoc_init_ring(priv, (void*)dev->mem_start);
 	ethoc_reset(priv);
 
 	if (netif_queue_stopped(dev)) {
@@ -838,7 +849,7 @@ static netdev_tx_t ethoc_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	else
 		bd.stat &= ~TX_BD_PAD;
 
-	dest = phys_to_virt(bd.addr);
+	dest = priv->vma[entry];
 	memcpy_toio(dest, skb->data, skb->len);
 
 	bd.stat &= ~(TX_BD_STATS | TX_BD_LEN_MASK);
@@ -978,6 +989,12 @@ static int ethoc_probe(struct platform_device *pdev)
 		priv->dma_alloc = buffer_size;
 	}
 
+	priv->vma = devm_kzalloc(&pdev->dev, priv->num_tx*sizeof(void*), GFP_KERNEL);
+	if (!priv->vma) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
 	/* Allow the platform setup code to pass in a MAC address. */
 	if (pdev->dev.platform_data) {
 		struct ethoc_platform_data *pdata =
-- 
1.7.0.4

Re: [PATCH] ethoc: Write bus addresses to registers

[Jonas Bonn]

[-- Attachment #1: Type: text/plain, Size: 947 bytes --]

Hmm... found a little problem here... this bit is dependent on another
patch that should be in there before it... I'd appreciate comment on the
approach of the whole thing and then I'll submit the missing bits in the
correct order.  Thanks and sorry for the confusion...
/Jonas

See my comment below...

> @@ -978,6 +989,12 @@ static int ethoc_probe(struct platform_device *pdev)
>  		priv->dma_alloc = buffer_size;
>  	}

---> Missing bits are here:  calculate number of transmission buffers...

>  
> +	priv->vma = devm_kzalloc(&pdev->dev, priv->num_tx*sizeof(void*), GFP_KERNEL);

---> And here: should be 
priv->vma = devm_kzalloc(&pdev->dev, (priv->num_tx
+priv->num_rx)*sizeof(void*), GFP_KERNEL);

> +	if (!priv->vma) {
> +		ret = -ENOMEM;
> +		goto error;
> +	}
> +
>  	/* Allow the platform setup code to pass in a MAC address. */
>  	if (pdev->dev.platform_data) {
>  		struct ethoc_platform_data *pdata =


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]