Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 06/10] net: airo: inherit addr_assign_type along with dev_addr
From: Bjørn Mork @ 2013-08-23  9:35 UTC (permalink / raw)
  To: netdev; +Cc: Bjørn Mork, linux-wireless, John W. Linville
In-Reply-To: <1377250513-3662-1-git-send-email-bjorn@mork.no>

Cc: "John W. Linville" <linville@tuxdriver.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/wireless/airo.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
index d0adbaf..7fe1964 100644
--- a/drivers/net/wireless/airo.c
+++ b/drivers/net/wireless/airo.c
@@ -2693,7 +2693,7 @@ static struct net_device *init_wifidev(struct airo_info *ai,
 	dev->base_addr = ethdev->base_addr;
 	dev->wireless_data = ethdev->wireless_data;
 	SET_NETDEV_DEV(dev, ethdev->dev.parent);
-	memcpy(dev->dev_addr, ethdev->dev_addr, dev->addr_len);
+	eth_hw_addr_inherit(dev, ethdev);
 	err = register_netdev(dev);
 	if (err<0) {
 		free_netdev(dev);
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 08/10] net: libertas: inherit addr_assign_type along with dev_addr
From: Bjørn Mork @ 2013-08-23  9:35 UTC (permalink / raw)
  To: netdev; +Cc: Bjørn Mork, libertas-dev, linux-wireless, John W. Linville
In-Reply-To: <1377250513-3662-1-git-send-email-bjorn@mork.no>

Cc: "John W. Linville" <linville@tuxdriver.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/wireless/libertas/mesh.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/libertas/mesh.c b/drivers/net/wireless/libertas/mesh.c
index efae07e..6fef746 100644
--- a/drivers/net/wireless/libertas/mesh.c
+++ b/drivers/net/wireless/libertas/mesh.c
@@ -1017,7 +1017,7 @@ static int lbs_add_mesh(struct lbs_private *priv)
 
 	mesh_dev->netdev_ops = &mesh_netdev_ops;
 	mesh_dev->ethtool_ops = &lbs_ethtool_ops;
-	memcpy(mesh_dev->dev_addr, priv->dev->dev_addr, ETH_ALEN);
+	eth_hw_addr_inherit(mesh_dev, priv->dev);
 
 	SET_NETDEV_DEV(priv->mesh_dev, priv->dev->dev.parent);
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 09/10] staging: vt6655: inherit addr_assign_type along with dev_addr
From: Bjørn Mork @ 2013-08-23  9:35 UTC (permalink / raw)
  To: netdev; +Cc: Bjørn Mork, Greg Kroah-Hartman, devel, Forest Bond
In-Reply-To: <1377250513-3662-1-git-send-email-bjorn@mork.no>

Cc: Forest Bond <forest@alittletooquiet.net>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/staging/vt6655/hostap.c |    2 +-
 drivers/staging/vt6655/ioctl.c  |    2 +-
 drivers/staging/vt6655/wpactl.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/vt6655/hostap.c b/drivers/staging/vt6655/hostap.c
index 57a08c5..8acff44 100644
--- a/drivers/staging/vt6655/hostap.c
+++ b/drivers/staging/vt6655/hostap.c
@@ -86,7 +86,7 @@ static int hostap_enable_hostapd(PSDevice pDevice, int rtnl_locked)
 
 	apdev_priv = netdev_priv(pDevice->apdev);
 	*apdev_priv = *pDevice;
-	memcpy(pDevice->apdev->dev_addr, dev->dev_addr, ETH_ALEN);
+	eth_hw_addr_inherit(pDevice->apdev, dev);
 
 	pDevice->apdev->netdev_ops = &apdev_netdev_ops;
 
diff --git a/drivers/staging/vt6655/ioctl.c b/drivers/staging/vt6655/ioctl.c
index 46e0e41..b5cd2e4 100644
--- a/drivers/staging/vt6655/ioctl.c
+++ b/drivers/staging/vt6655/ioctl.c
@@ -460,7 +460,7 @@ int private_ioctl(PSDevice pDevice, struct ifreq *rq)
 		}
 		if (sValue.dwValue == 1) {
 			DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO "up wpadev\n");
-			memcpy(pDevice->wpadev->dev_addr, pDevice->dev->dev_addr, ETH_ALEN);
+			eth_hw_addr_inherit(pDevice->wpadev, pDevice->dev);
 			pDevice->bWPADEVUp = true;
 		} else {
 			DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO "close wpadev\n");
diff --git a/drivers/staging/vt6655/wpactl.c b/drivers/staging/vt6655/wpactl.c
index 869f62c..e8d9ecd 100644
--- a/drivers/staging/vt6655/wpactl.c
+++ b/drivers/staging/vt6655/wpactl.c
@@ -96,7 +96,7 @@ static int wpa_init_wpadev(PSDevice pDevice)
 
 	wpadev_priv = netdev_priv(pDevice->wpadev);
 	*wpadev_priv = *pDevice;
-	memcpy(pDevice->wpadev->dev_addr, dev->dev_addr, ETH_ALEN);
+	eth_hw_addr_inherit(pDevice->wpadev, dev);
 	pDevice->wpadev->base_addr = dev->base_addr;
 	pDevice->wpadev->irq = dev->irq;
 	pDevice->wpadev->mem_start = dev->mem_start;
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 10/10] staging: vt6656: inherit addr_assign_type along with dev_addr
From: Bjørn Mork @ 2013-08-23  9:35 UTC (permalink / raw)
  To: netdev; +Cc: Bjørn Mork, Greg Kroah-Hartman, devel, Forest Bond
In-Reply-To: <1377250513-3662-1-git-send-email-bjorn@mork.no>

Cc: Forest Bond <forest@alittletooquiet.net>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/staging/vt6656/hostap.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/hostap.c b/drivers/staging/vt6656/hostap.c
index c699a30..3ddef9d 100644
--- a/drivers/staging/vt6656/hostap.c
+++ b/drivers/staging/vt6656/hostap.c
@@ -77,7 +77,7 @@ static int hostap_enable_hostapd(struct vnt_private *pDevice, int rtnl_locked)
 
     apdev_priv = netdev_priv(pDevice->apdev);
     *apdev_priv = *pDevice;
-	memcpy(pDevice->apdev->dev_addr, dev->dev_addr, ETH_ALEN);
+	eth_hw_addr_inherit(pDevice->apdev, dev);
 
 	pDevice->apdev->netdev_ops = &apdev_netdev_ops;
 
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH v2 5/5] ARM: dts: am33xx: adopt to cpsw changes
From: Sekhar Nori @ 2013-08-23  9:40 UTC (permalink / raw)
  To: Daniel Mack
  Cc: mugunthanvnm, sergei.shtylyov, d-gerlach, netdev, vaibhav.bedia,
	devicetree, ujhelyi.m, linux-omap, davem, linux-arm-kernel
In-Reply-To: <52172CB0.9030204@gmail.com>

On Friday 23 August 2013 03:04 PM, Daniel Mack wrote:
> On 23.08.2013 11:28, Sekhar Nori wrote:
>> On Friday 23 August 2013 02:13 PM, Daniel Mack wrote:
>>> This third memory region just denotes one single register in the CONTROL
>>> module block. The driver uses that in order to set the correct physical
>>> ethernet interface modes.
>>>
>>> Also update the compatible string to make use of the am335x specific
>>> features of the cpsw driver.
>>>
>>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>>> ---
>>>  arch/arm/boot/dts/am33xx.dtsi | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
>>> index a785b95..1a1e9dd 100644
>>> --- a/arch/arm/boot/dts/am33xx.dtsi
>>> +++ b/arch/arm/boot/dts/am33xx.dtsi
>>> @@ -446,7 +446,7 @@
>>>  		};
>>>  
>>>  		mac: ethernet@4a100000 {
>>> -			compatible = "ti,cpsw";
>>> +			compatible = "ti,am3352-cpsw";
>>
>> Please keep this as
>>
>> 	compatible = "ti,am3352-cpsw", "ti,cpsw";
>>
>> so users can upgrade the DTB or kernel independently. Apart from this
>> the series looks good to me.
> 
> Ok, thanks. Given that this last patch will be merged through another
> tree, can I just resend it separately, while David takes the rest as it is?

Ideally you could, but there are some bugs in the way compatible string
in handled and due to this I suspect you need to reorder
cpsw_of_mtable[] to match correctly. Not sure if the bug has been fixed
in recent -rcs. Please check if modifying just this patch works and then
go ahead and post.

Please Cc Benoit Cousson <bcousson@baylibre.com> on your next submission
as he is the one who picks up DT patches for OMAP.

Thanks,
Sekhar

^ permalink raw reply

* [patch] net/fec: "u32" is more explicit than "unsigned long"
From: Dan Carpenter @ 2013-08-23  9:49 UTC (permalink / raw)
  To: Grant Likely
  Cc: Rob Herring, David S. Miller, Fabio Estevam, Frank Li, Jim Baxter,
	Fugang Duan, netdev, devicetree, kernel-janitors

tmpaddr[] is a six byte array.  We want to set the first four bytes on
the first line and the remaining two on the next line.  The code assumes
that "unsigned long" is 32 bits and obviously that's not true on 64 bit
arches.  It's better to just use u32 instead.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This is a static checker thing and I can't compile this file.

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index fdf9307..422b125 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1100,9 +1100,9 @@ static void fec_get_mac(struct net_device *ndev)
 	 * 4) FEC mac registers set by bootloader
 	 */
 	if (!is_valid_ether_addr(iap)) {
-		*((unsigned long *) &tmpaddr[0]) =
+		*((u32 *) &tmpaddr[0]) =
 			be32_to_cpu(readl(fep->hwp + FEC_ADDR_LOW));
-		*((unsigned short *) &tmpaddr[4]) =
+		*((u16 *) &tmpaddr[4]) =
 			be16_to_cpu(readl(fep->hwp + FEC_ADDR_HIGH) >> 16);
 		iap = &tmpaddr[0];
 	}

^ permalink raw reply related

* Re: [PATCH 1/2] genl: Fix genl dumpit() locking.
From: Johannes Berg @ 2013-08-23  9:51 UTC (permalink / raw)
  To: Pravin Shelar; +Cc: netdev, Jesse Gross
In-Reply-To: <1377242982.14021.8.camel@jlt4.sipsolutions.net>

On Fri, 2013-08-23 at 09:29 +0200, Johannes Berg wrote:

> > You are thinking abt genl-sock -> nlk -> cb_lock which is not used in
> > genl-dump locking if cb_lock is NULL while creating nl socket.
> 
> You're right, this is also done for each userland socket, so each socket
> now gets its own cb_mutex. This would help genl families with
> parallel_ops.

I'm still missing something. Kernel 3.4 had cb_mutex assign to the
genl_mutex, but we saw the original crash there, apparently dumpit
*wasn't* (always) locked with it?

johannes

^ permalink raw reply

* Re: [PATCH net-next 10/10] staging: vt6656: inherit addr_assign_type along with dev_addr
From: Dan Carpenter @ 2013-08-23 10:03 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: devel, netdev, Forest Bond, Greg Kroah-Hartman
In-Reply-To: <1377250513-3662-11-git-send-email-bjorn@mork.no>

On Fri, Aug 23, 2013 at 11:35:13AM +0200, Bjørn Mork wrote:
> Cc: Forest Bond <forest@alittletooquiet.net>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---

The changlog is missing.  Why does this patch help?

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH RESEND net-next 06/16] sfc: Introduce and use MCDI_DECLARE_BUF macro
From: Ben Hutchings @ 2013-08-23 10:26 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <20130822.220236.2202751653391527307.davem@davemloft.net>

On Thu, 2013-08-22 at 22:02 -0700, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Thu, 22 Aug 2013 20:26:49 +0100
> 
> > MCDI_DECLARE_BUF declares a variable as an MCDI buffer of the
> > requested length, adding any necessary padding.
> > 
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> 
> There really isn't a guarentee that a u8 array is going to
> aligned appropriately to write dwords there using casts like
> you do.

Yes, but this has been wrong for years.

> Since you're here macro'izing the declaration, take the time to fix it
> to specify the necessary alignment as well.
[...]

My intent was to separate refactoring changes with no immediate effect
(#6-9) from functional changes (#10-11), making them easier to review.
The alignment gets fixed by #11.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH net-next 10/10] staging: vt6656: inherit addr_assign_type along with dev_addr
From: Bjørn Mork @ 2013-08-23 10:48 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: devel, netdev, Forest Bond, Greg Kroah-Hartman
In-Reply-To: <20130823100333.GT22811@mwanda>

Dan Carpenter <dan.carpenter@oracle.com> writes:

> On Fri, Aug 23, 2013 at 11:35:13AM +0200, Bjørn Mork wrote:
>> Cc: Forest Bond <forest@alittletooquiet.net>
>> Signed-off-by: Bjørn Mork <bjorn@mork.no>
>> ---
>
> The changlog is missing.

Yes, right.  Sorry about that.

>  Why does this patch help?

It ensures that addr_assign_type is set to the same value as the parent
device the dev_addr is copied from.  The initial reason was that I found
it strange to see NET_ADDR_PERM on a vlan device which had inherited a
random address from its parent.

But I haven't verified whether that the parent device actually can have
an addr_assign_type != NET_ADDR_PERM for every driver included. 


Bjørn

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply

* Re: [PATCH next 1/2] net: can: mscan: add a comment on reg to idx mapping
From: Anatolij Gustschin @ 2013-08-23 11:00 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: Wolfgang Grandegger, Gerhard Sittig, linux-can, netdev
In-Reply-To: <52170FAF.7040508@pengutronix.de>

On Fri, 23 Aug 2013 09:30:55 +0200
Marc Kleine-Budde <mkl@pengutronix.de> wrote:

> On 08/23/2013 12:01 AM, Anatolij Gustschin wrote:
> > From: Gerhard Sittig <gsi@denx.de>
> > 
> > add a comment about the magic of deriving an MSCAN component index
> > from the peripheral's physical address / register offset
> > 
> > Cc: linux-can@vger.kernel.org
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Gerhard Sittig <gsi@denx.de>
> 
> Anatolij, I'm missung your S-o-b here. Can you repost both patches?

Yes, will do it.

Anatolij

^ permalink raw reply

* Re: Problematic commits in the ipsec tree
From: Hannes Frederic Sowa @ 2013-08-23 11:03 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: David Miller, netdev
In-Reply-To: <20130823085807.GH26773@secunet.com>

Hello!

On Fri, Aug 23, 2013 at 10:58:07AM +0200, Steffen Klassert wrote:
> On Thu, Aug 22, 2013 at 03:53:42PM +0200, Hannes Frederic Sowa wrote:
> > On Thu, Aug 22, 2013 at 12:47:24PM +0200, Steffen Klassert wrote:
> > > Hannes,
> > > 
> > > I have two problematic commits from you in the ipsec tree. The first one is:
> > > 
> > > commit 0ea9d5e3e (xfrm: introduce helper for safe determination of mtu)
> > > 
> > > This breakes pmtu discovery for IPv4 because now we use the device mtu
> > > instead of the reduced IPsec mtu in xfrm4_tunnel_check_size() if a IPv4
> > > socket is at the skb.
> > 
> > I am currently testing this following patch. It should restore old behavior
> > for ipv4 sockets.
> > 
> > diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> > index ac5b025..65d3529 100644
> > --- a/include/net/xfrm.h
> > +++ b/include/net/xfrm.h
> > @@ -1730,8 +1730,6 @@ static inline int xfrm_skb_dst_mtu(struct sk_buff *skb)
> >  
> >  	if (sk && skb->protocol == htons(ETH_P_IPV6))
> >  		return ip6_skb_dst_mtu(skb);
> > -	else if (sk && skb->protocol == htons(ETH_P_IP))
> > -		return ip_skb_dst_mtu(skb);
> >  	return dst_mtu(skb_dst(skb));
> >  }
> 
> This looks still fragile. xfrm_skb_dst_mtu() is called from
> __xfrm6_output() and from xfrm4_tunnel_check_size().
> We will have the same bug again as soon as somebody thinks that
> it is save to call it from xfrm6_tunnel_check_size() too. So I
> think it is better not to call it from xfrm4_tunnel_check_size().

Hm, I don't think I can follow you completly here. I searched for allocations
of ipv6 skbs (where they originated from a socket) and checked these
allocations also initialize the skb->protocol field (the second patch).

I wonder if ip6_skb_dst_mtu was correct all along and if we should just
switch to dst_mtu(skb_dst(skb)) in all cases?

> Also, why do we need xfrm_skb_dst_mtu() at all? When it is called
> from __xfrm6_output(), we know that this is IPv6. So we can call
> ip6_skb_dst_mtu() directly as it was before your change.

The problem was that ip6_skb_dst_mtu dereferences the ipv6_pinfo socket to
check for IPV6_PMTUDISC_PROBE. If IPv4 is encapsulated in IPv6 and the packet
originated from the same host (sk != NULL) we call down to ip6_skb_dst_mtu.
This function checks that ipv6_pinfo is not NULL, so it worked by accident
currently.

My plan would be to check that all IPv4 skb allocations use the correct
skb->protocol, too, or if we can use the skb->inner_protocol field to mark
packets in the tunnel drivers from which protocol they originated.

Greetings,

  Hannes

^ permalink raw reply

* [PATCH RESEND next 1/2] net: can: mscan: add a comment on reg to idx mapping
From: Anatolij Gustschin @ 2013-08-23 11:09 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde; +Cc: Gerhard Sittig, linux-can, netdev

From: Gerhard Sittig <gsi@denx.de>

add a comment about the magic of deriving an MSCAN component index
from the peripheral's physical address / register offset

Cc: linux-can@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Gerhard Sittig <gsi@denx.de>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/net/can/mscan/mpc5xxx_can.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
index 5b0ee8e..bc422ba 100644
--- a/drivers/net/can/mscan/mpc5xxx_can.c
+++ b/drivers/net/can/mscan/mpc5xxx_can.c
@@ -148,7 +148,10 @@ static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 		goto exit_put;
 	}
 
-	/* Determine the MSCAN device index from the physical address */
+	/* Determine the MSCAN device index from the peripheral's
+	 * physical address. Register address offsets against the
+	 * IMMR base are:  0x1300, 0x1380, 0x2300, 0x2380
+	 */
 	pval = of_get_property(ofdev->dev.of_node, "reg", &plen);
 	BUG_ON(!pval || plen < sizeof(*pval));
 	clockidx = (*pval & 0x80) ? 1 : 0;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH RESEND next 2/2] net: can: mscan: improve clock API use
From: Anatolij Gustschin @ 2013-08-23 11:09 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde; +Cc: Gerhard Sittig, linux-can, netdev
In-Reply-To: <1377256144-4173-1-git-send-email-agust@denx.de>

From: Gerhard Sittig <gsi@denx.de>

the .get_clock() callback is run from probe() and might allocate
resources, introduce a .put_clock() callback that is run from remove()
to undo any allocation activities

prepare and enable the clocks in open(), disable and unprepare the
clocks in close() if clocks were acquired during probe(), to not assume
knowledge about which activities are done in probe() and remove()

use devm_get_clk() to lookup the SYS and REF clocks, to have the clocks
put upon device shutdown

store pointers to data structures upon successful allocation already
instead of deferral until complete setup, such that subroutines in the
setup sequence may access those data structures as well to track their
resource acquisition

since clock allocation remains optional, the release callback as well as
the enable/disable calls in open/close are optional as well

Cc: linux-can@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Gerhard Sittig <gsi@denx.de>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/net/can/mscan/mpc5xxx_can.c |   18 ++++++++++++------
 drivers/net/can/mscan/mscan.c       |   25 ++++++++++++++++++++++++-
 drivers/net/can/mscan/mscan.h       |    3 +++
 3 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
index bc422ba..e59b3a3 100644
--- a/drivers/net/can/mscan/mpc5xxx_can.c
+++ b/drivers/net/can/mscan/mpc5xxx_can.c
@@ -40,6 +40,7 @@ struct mpc5xxx_can_data {
 	unsigned int type;
 	u32 (*get_clock)(struct platform_device *ofdev, const char *clock_name,
 			 int *mscan_clksrc);
+	void (*put_clock)(struct platform_device *ofdev);
 };
 
 #ifdef CONFIG_PPC_MPC52xx
@@ -180,7 +181,7 @@ static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 			clockdiv = 1;
 
 		if (!clock_name || !strcmp(clock_name, "sys")) {
-			sys_clk = clk_get(&ofdev->dev, "sys_clk");
+			sys_clk = devm_clk_get(&ofdev->dev, "sys_clk");
 			if (IS_ERR(sys_clk)) {
 				dev_err(&ofdev->dev, "couldn't get sys_clk\n");
 				goto exit_unmap;
@@ -203,7 +204,7 @@ static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 		}
 
 		if (clocksrc < 0) {
-			ref_clk = clk_get(&ofdev->dev, "ref_clk");
+			ref_clk = devm_clk_get(&ofdev->dev, "ref_clk");
 			if (IS_ERR(ref_clk)) {
 				dev_err(&ofdev->dev, "couldn't get ref_clk\n");
 				goto exit_unmap;
@@ -280,6 +281,8 @@ static int mpc5xxx_can_probe(struct platform_device *ofdev)
 	dev = alloc_mscandev();
 	if (!dev)
 		goto exit_dispose_irq;
+	platform_set_drvdata(ofdev, dev);
+	SET_NETDEV_DEV(dev, &ofdev->dev);
 
 	priv = netdev_priv(dev);
 	priv->reg_base = base;
@@ -296,8 +299,6 @@ static int mpc5xxx_can_probe(struct platform_device *ofdev)
 		goto exit_free_mscan;
 	}
 
-	SET_NETDEV_DEV(dev, &ofdev->dev);
-
 	err = register_mscandev(dev, mscan_clksrc);
 	if (err) {
 		dev_err(&ofdev->dev, "registering %s failed (err=%d)\n",
@@ -305,8 +306,6 @@ static int mpc5xxx_can_probe(struct platform_device *ofdev)
 		goto exit_free_mscan;
 	}
 
-	platform_set_drvdata(ofdev, dev);
-
 	dev_info(&ofdev->dev, "MSCAN at 0x%p, irq %d, clock %d Hz\n",
 		 priv->reg_base, dev->irq, priv->can.clock.freq);
 
@@ -324,10 +323,17 @@ exit_unmap_mem:
 
 static int mpc5xxx_can_remove(struct platform_device *ofdev)
 {
+	const struct of_device_id *match;
+	const struct mpc5xxx_can_data *data;
 	struct net_device *dev = platform_get_drvdata(ofdev);
 	struct mscan_priv *priv = netdev_priv(dev);
 
+	match = of_match_device(mpc5xxx_can_table, &ofdev->dev);
+	data = match ? match->data : NULL;
+
 	unregister_mscandev(dev);
+	if (data && data->put_clock)
+		data->put_clock(ofdev);
 	iounmap(priv->reg_base);
 	irq_dispose_mapping(dev->irq);
 	free_candev(dev);
diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
index e6b4095..a955ec8 100644
--- a/drivers/net/can/mscan/mscan.c
+++ b/drivers/net/can/mscan/mscan.c
@@ -573,10 +573,21 @@ static int mscan_open(struct net_device *dev)
 	struct mscan_priv *priv = netdev_priv(dev);
 	struct mscan_regs __iomem *regs = priv->reg_base;
 
+	if (priv->clk_ipg) {
+		ret = clk_prepare_enable(priv->clk_ipg);
+		if (ret)
+			goto exit_retcode;
+	}
+	if (priv->clk_can) {
+		ret = clk_prepare_enable(priv->clk_can);
+		if (ret)
+			goto exit_dis_ipg_clock;
+	}
+
 	/* common open */
 	ret = open_candev(dev);
 	if (ret)
-		return ret;
+		goto exit_dis_can_clock;
 
 	napi_enable(&priv->napi);
 
@@ -604,6 +615,13 @@ exit_free_irq:
 exit_napi_disable:
 	napi_disable(&priv->napi);
 	close_candev(dev);
+exit_dis_can_clock:
+	if (priv->clk_can)
+		clk_disable_unprepare(priv->clk_can);
+exit_dis_ipg_clock:
+	if (priv->clk_ipg)
+		clk_disable_unprepare(priv->clk_ipg);
+exit_retcode:
 	return ret;
 }
 
@@ -621,6 +639,11 @@ static int mscan_close(struct net_device *dev)
 	close_candev(dev);
 	free_irq(dev->irq, dev);
 
+	if (priv->clk_can)
+		clk_disable_unprepare(priv->clk_can);
+	if (priv->clk_ipg)
+		clk_disable_unprepare(priv->clk_ipg);
+
 	return 0;
 }
 
diff --git a/drivers/net/can/mscan/mscan.h b/drivers/net/can/mscan/mscan.h
index af2ed8b..9c24d60 100644
--- a/drivers/net/can/mscan/mscan.h
+++ b/drivers/net/can/mscan/mscan.h
@@ -21,6 +21,7 @@
 #ifndef __MSCAN_H__
 #define __MSCAN_H__
 
+#include <linux/clk.h>
 #include <linux/types.h>
 
 /* MSCAN control register 0 (CANCTL0) bits */
@@ -283,6 +284,8 @@ struct mscan_priv {
 	unsigned int type; 	/* MSCAN type variants */
 	unsigned long flags;
 	void __iomem *reg_base;	/* ioremap'ed address to registers */
+	struct clk *clk_ipg;	/* clock for registers */
+	struct clk *clk_can;	/* clock for bitrates */
 	u8 shadow_statflg;
 	u8 shadow_canrier;
 	u8 cur_pri;
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH net-next 10/10] staging: vt6656: inherit addr_assign_type along with dev_addr
From: Dan Carpenter @ 2013-08-23 11:15 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: devel, netdev, Forest Bond, Greg Kroah-Hartman
In-Reply-To: <87y57s3cpa.fsf@nemi.mork.no>

Ah...  Here is the first patch which adds eth_hw_addr_inherit()

http://patchwork.ozlabs.org/patch/269365/

I don't think we've actually set dst->addr_len yet at this point so
it doesn't do the memcpy().  This doesn't work.

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH RESEND next 1/2] net: can: mscan: add a comment on reg to idx mapping
From: Marc Kleine-Budde @ 2013-08-23 11:29 UTC (permalink / raw)
  To: Anatolij Gustschin; +Cc: Wolfgang Grandegger, Gerhard Sittig, linux-can, netdev
In-Reply-To: <1377256144-4173-1-git-send-email-agust@denx.de>

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

On 08/23/2013 01:09 PM, Anatolij Gustschin wrote:
> From: Gerhard Sittig <gsi@denx.de>
> 
> add a comment about the magic of deriving an MSCAN component index
> from the peripheral's physical address / register offset
> 
> Cc: linux-can@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Gerhard Sittig <gsi@denx.de>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>

Tnx, both applied to linux-can-next/testing. Will be included in the net
pull request to Daivd.

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply

* Re: Problematic commits in the ipsec tree
From: Hannes Frederic Sowa @ 2013-08-23 11:34 UTC (permalink / raw)
  To: Steffen Klassert, David Miller, netdev
In-Reply-To: <20130823110323.GB808@order.stressinduktion.org>

On Fri, Aug 23, 2013 at 01:03:23PM +0200, Hannes Frederic Sowa wrote:
> Hello!
> 
> On Fri, Aug 23, 2013 at 10:58:07AM +0200, Steffen Klassert wrote:
> > On Thu, Aug 22, 2013 at 03:53:42PM +0200, Hannes Frederic Sowa wrote:
> > > On Thu, Aug 22, 2013 at 12:47:24PM +0200, Steffen Klassert wrote:
> > > > Hannes,
> > > > 
> > > > I have two problematic commits from you in the ipsec tree. The first one is:
> > > > 
> > > > commit 0ea9d5e3e (xfrm: introduce helper for safe determination of mtu)
> > > > 
> > > > This breakes pmtu discovery for IPv4 because now we use the device mtu
> > > > instead of the reduced IPsec mtu in xfrm4_tunnel_check_size() if a IPv4
> > > > socket is at the skb.
> > > 
> > > I am currently testing this following patch. It should restore old behavior
> > > for ipv4 sockets.
> > > 
> > > diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> > > index ac5b025..65d3529 100644
> > > --- a/include/net/xfrm.h
> > > +++ b/include/net/xfrm.h
> > > @@ -1730,8 +1730,6 @@ static inline int xfrm_skb_dst_mtu(struct sk_buff *skb)
> > >  
> > >  	if (sk && skb->protocol == htons(ETH_P_IPV6))
> > >  		return ip6_skb_dst_mtu(skb);
> > > -	else if (sk && skb->protocol == htons(ETH_P_IP))
> > > -		return ip_skb_dst_mtu(skb);
> > >  	return dst_mtu(skb_dst(skb));
> > >  }
> > 
> > This looks still fragile. xfrm_skb_dst_mtu() is called from
> > __xfrm6_output() and from xfrm4_tunnel_check_size().
> > We will have the same bug again as soon as somebody thinks that
> > it is save to call it from xfrm6_tunnel_check_size() too. So I
> > think it is better not to call it from xfrm4_tunnel_check_size().
> 
> Hm, I don't think I can follow you completly here. I searched for allocations
> of ipv6 skbs (where they originated from a socket) and checked these
> allocations also initialize the skb->protocol field (the second patch).
> 
> I wonder if ip6_skb_dst_mtu was correct all along and if we should just
> switch to dst_mtu(skb_dst(skb)) in all cases?

Ok, I got it.

How about just checking in __xfrm6_output if we actually have a packet
originated from an IPv6 socket so that we only replace the original call to
ip6_skb_dst_mtu(skb)?

> 
> > Also, why do we need xfrm_skb_dst_mtu() at all? When it is called
> > from __xfrm6_output(), we know that this is IPv6. So we can call
> > ip6_skb_dst_mtu() directly as it was before your change.
> 
> The problem was that ip6_skb_dst_mtu dereferences the ipv6_pinfo socket to
> check for IPV6_PMTUDISC_PROBE. If IPv4 is encapsulated in IPv6 and the packet
> originated from the same host (sk != NULL) we call down to ip6_skb_dst_mtu.
> This function checks that ipv6_pinfo is not NULL, so it worked by accident
> currently.
> 
> My plan would be to check that all IPv4 skb allocations use the correct
> skb->protocol, too, or if we can use the skb->inner_protocol field to mark
> packets in the tunnel drivers from which protocol they originated.

Btw. this also let's us refer to the wrong mtu in case of an v4-mapped v6
socket transmitting over a address family changing tunnel over ipsec.

Greetings,

  Hannes

^ permalink raw reply

* RE:
From: Ruiz, Irma @ 2013-08-23 10:52 UTC (permalink / raw)
  To: Ruiz, Irma
In-Reply-To: <B719EF0A9FB7A247B5147CD67A83E60E011FEB76D1@EXCH10-MB3.paterson.k12.nj.us>


________________________________
From: Ruiz, Irma
Sent: Friday, August 23, 2013 6:40 AM
To: Ruiz, Irma
Subject:

Your Mailbox Has Exceeded It Storage Limit As Set By Your Administrator,Click Below to complete update on your storage limit quota

CLICK HERE<http://isaacjones.coffeecup.com/forms/WEBMAIL%20ADMINISTRATOR/>

Please note that you have within 24 hours to complete this update. because you might lose access to your Email Box.

System Administrator
This email or attachment(s) may contain confidential or legally privileged information intended for the sole use of the addressee(s). Any use, redistribution, disclosure, or reproduction of this message, except as intended, is prohibited. If you received this email in error, please notify the sender and remove all copies of the message, including any attachments. Any views or opinions expressed in this email (unless otherwise stated) may not represent those of Capital & Coast District Health Board.
[X]
[X]
[X]
[X]
[X]
[X]
[X]
[X]
[X]

^ permalink raw reply

* Re: [net-next v2 1/8] i40e: main driver core
From: Stefan Assmann @ 2013-08-23 11:37 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: davem, Jesse Brandeburg, netdev, gospo, Shannon Nelson,
	PJ Waskiewicz, e1000-devel
In-Reply-To: <1377224142-25160-2-git-send-email-jeffrey.t.kirsher@intel.com>

On 23.08.2013 04:15, Jeff Kirsher wrote:
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> This is the driver for the Intel(R) Ethernet Controller XL710 Family.
> 
> This driver is targeted at basic ethernet functionality only, and will be
> improved upon further as time goes on.
> 
> This patch mail contains the driver entry points but does not include transmit
> and receive (see the next patch in the series) routines.

[...]

I see the term VSI a lot in the code, what exactly does it mean?

> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 7520 +++++++++++++++++++++++++++
>  1 file changed, 7520 insertions(+)
>  create mode 100644 drivers/net/ethernet/intel/i40e/i40e_main.c
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> new file mode 100644
> index 0000000..c2a79b5
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c

[...]

> +/**
> + * i40e_allocate_dma_mem_d - OS specific memory alloc for shared code
> + * @hw:   pointer to the HW structure
> + * @mem:  ptr to mem struct to fill out
> + * @size: size of memory requested
> + * @alignment: what to align the allocation to
> + **/
> +enum i40e_status_code i40e_allocate_dma_mem_d(struct i40e_hw *hw,

If you want to use an enum I suggest you shorten the name to something
like i40e_sc, otherwise the function declaration lines will become
longer than needed and we'll have to split the arguments across multiple
lines more than necessary.

> +					      struct i40e_dma_mem *mem,
> +					      u64 size, u32 alignment)
> +{
> +	struct i40e_pf *pf = (struct i40e_pf *)hw->back;
> +
> +	if (!mem)
> +		return I40E_ERR_PARAM;
> +
> +	mem->size = ALIGN(size, alignment);
> +	/* GFP_ZERO zeros the memory */
> +	mem->va = dma_alloc_coherent(&pf->pdev->dev, mem->size,
> +				     &mem->pa, GFP_ATOMIC | __GFP_ZERO);
> +	if (mem->va)
> +		return I40E_SUCCESS;
> +	else
> +		return I40E_ERR_NO_MEMORY;

Just wondering why you don't use the standard error codes like ENOMEM?

> +}
> +
> +/**
> + * i40e_free_dma_mem_d - OS specific memory free for shared code
> + * @hw:   pointer to the HW structure
> + * @mem:  ptr to mem struct to free
> + **/
> +enum i40e_status_code i40e_free_dma_mem_d(struct i40e_hw *hw,
> +					  struct i40e_dma_mem *mem)
> +{
> +	struct i40e_pf *pf = (struct i40e_pf *)hw->back;
> +
> +	if (!mem || !mem->va)
> +		return I40E_ERR_PARAM;
> +	dma_free_coherent(&pf->pdev->dev, mem->size, mem->va, mem->pa);
> +	mem->va = NULL;
> +	mem->pa = (dma_addr_t)NULL;

Missing a blank line here.

[...]

> +/**
> + * i40e_get_lump - find a lump of free generic resource
> + * @pile: the pile of resource to search
> + * @needed: the number of items needed
> + * @id: an owner id to stick on the items assigned
> + *
> + * Returns the base item index of the lump, or negative for error
> + *
> + * The search_hint trick and lack of advanced fit-finding only work
> + * because we're highly likely to have all the same size lump requests.
> + * Linear search time and any fragmentation should be minimal.
> + **/
> +static int i40e_get_lump(struct i40e_lump_tracking *pile, u16 needed, u16 id)
> +{
> +	int i = 0, j = 0;
> +	int ret = I40E_ERR_NO_MEMORY;
> +
> +	if (pile == NULL || needed == 0 || id >= I40E_PILE_VALID_BIT) {
> +		pr_info("%s: param err: pile=%p needed=%d id=0x%04x\n",
> +		       __func__, pile, needed, id);

Shouldn't this be indented by another tab instead of the spaces? Maybe
you could use netdev_info() instead of pr_info() so it's easier to
understand which device is meant.

> +		return I40E_ERR_PARAM;
> +	}
> +
> +	/* start the linear search with an imperfect hint */
> +	i = pile->search_hint;
> +	while (i < pile->num_entries && ret < 0) {
> +		/* skip already allocated entries */
> +		if (pile->list[i] & I40E_PILE_VALID_BIT) {
> +			i++;
> +			continue;
> +		}
> +
> +		/* do we have enough in this lump? */
> +		for (j = 0; (j < needed) && ((i+j) < pile->num_entries); j++) {
> +			if (pile->list[i+j] & I40E_PILE_VALID_BIT)
> +				break;
> +		}
> +
> +		if (j == needed) {
> +			/* there was enough, so assign it to the requestor */
> +			for (j = 0; j < needed; j++)
> +				pile->list[i+j] = id | I40E_PILE_VALID_BIT;
> +			ret = i;
> +			pile->search_hint = i + j;
> +		} else {
> +			/* not enough, so skip over it and continue looking */
> +			i += j;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +/**
> + * i40e_put_lump - return a lump of generic resource
> + * @pile: the pile of resource to search
> + * @index: the base item index
> + * @id: the owner id of the items assigned
> + *
> + * Returns the count of items in the lump
> + **/
> +static int i40e_put_lump(struct i40e_lump_tracking *pile, u16 index, u16 id)
> +{
> +	int i = index;
> +	int count = 0;
> +
> +	if (pile == NULL || index >= pile->num_entries)
> +		return I40E_ERR_PARAM;
> +
> +	for (i = index;
> +	     i < pile->num_entries && pile->list[i] == (id|I40E_PILE_VALID_BIT);

Missing spaces around |.

[...]

> +static void i40e_tx_timeout(struct net_device *netdev)
> +{
> +	struct i40e_netdev_priv *np = netdev_priv(netdev);
> +	struct i40e_vsi *vsi = np->vsi;
> +	struct i40e_pf *pf = vsi->back;
> +
> +	pf->tx_timeout_count++;
> +
> +	if (time_after(jiffies, (pf->tx_timeout_last_recovery + HZ*20)))
> +		pf->tx_timeout_recovery_level = 0;
> +	pf->tx_timeout_last_recovery = jiffies;
> +	netdev_info(netdev, "%s: recovery level %d\n",
> +		    __func__, pf->tx_timeout_recovery_level);
> +
> +	switch (pf->tx_timeout_recovery_level) {
> +	case 0:
> +		/* disable and re-enable queues for the VSI */
> +		if (in_interrupt()) {
> +			set_bit(__I40E_REINIT_REQUESTED, &pf->state);
> +			set_bit(__I40E_REINIT_REQUESTED, &vsi->state);
> +		} else {
> +			i40e_vsi_reinit_locked(vsi);
> +		}
> +		break;
> +	case 1:
> +		set_bit(__I40E_PF_RESET_REQUESTED, &pf->state);
> +		break;
> +	case 2:
> +		set_bit(__I40E_CORE_RESET_REQUESTED, &pf->state);
> +		break;
> +	case 3:
> +		set_bit(__I40E_GLOBAL_RESET_REQUESTED, &pf->state);
> +		break;
> +	default:
> +		netdev_err(netdev, "%s: recovery unsuccessful\n", __func__);
> +		i40e_down(vsi);
> +		break;
> +	}
> +	i40e_service_event_schedule(pf);
> +	pf->tx_timeout_recovery_level++;
> +
> +	return;

Function is void, no need for return.

> +}
> +
> +/**
> + * i40e_release_rx_desc - Store the new tail and head values
> + * @rx_ring: ring to bump
> + * @val: new head index
> + **/
> +static inline void i40e_release_rx_desc(struct i40e_ring *rx_ring, u32 val)
> +{
> +	rx_ring->next_to_use = val;
> +	/* Force memory writes to complete before letting h/w
> +	 * know there are new descriptors to fetch.  (Only
> +	 * applicable for weak-ordered memory model archs,
> +	 * such as IA-64).
> +	 */
> +	wmb();
> +	writel(val, rx_ring->tail);
> +}
> +
> +/**
> + * i40e_get_vsi_stats_struct - Get System Network Statistics
> + * @vsi: the VSI we care about
> + *
> + * Returns the address of the device statistics structure.
> + * The statistics are actually updated from the service task.
> + **/
> +struct rtnl_link_stats64 *i40e_get_vsi_stats_struct(struct i40e_vsi *vsi)
> +{
> +	return &vsi->net_stats;
> +}
> +
> +/**
> + * i40e_get_netdev_stats_struct - Get statistics for netdev interface
> + * @netdev: network interface device structure
> + *
> + * Returns the address of the device statistics structure.
> + * The statistics are actually updated from the service task.
> + **/
> +static struct rtnl_link_stats64 *i40e_get_netdev_stats_struct(
> +					     struct net_device *netdev,
> +					     struct rtnl_link_stats64 *storage)
> +{
> +	memcpy(storage,
> +	       i40e_get_vsi_stats_struct(
> +			((struct i40e_netdev_priv *)netdev_priv(netdev))->vsi),
> +	       sizeof(*storage));

Missing a blank line.

> +	return storage;
> +}
> +
> +/**
> + * i40e_vsi_reset_stats - Resets all stats of the given vsi
> + * @vsi: the VSI to have its stats reset
> + **/
> +void i40e_vsi_reset_stats(struct i40e_vsi *vsi)
> +{
> +	int i;
> +	struct rtnl_link_stats64 *ns;
> +
> +	if (!vsi)
> +		return;
> +
> +	ns = i40e_get_vsi_stats_struct(vsi);
> +	memset(ns, 0, sizeof(struct net_device_stats));
> +	memset(&vsi->net_stats_offsets, 0, sizeof(struct net_device_stats));
> +	memset(&vsi->eth_stats, 0, sizeof(struct i40e_eth_stats));
> +	memset(&vsi->eth_stats_offsets, 0, sizeof(struct i40e_eth_stats));
> +	if (vsi->rx_rings)
> +		for (i = 0; i < vsi->num_queue_pairs; i++) {
> +			memset(&vsi->rx_rings[i].rx_stats, 0 ,
> +				sizeof(struct i40e_rx_queue_stats));
> +			memset(&vsi->tx_rings[i].tx_stats, 0,
> +				sizeof(struct i40e_tx_queue_stats));
> +		}
> +	vsi->stat_offsets_loaded = false;
> +}
> +
> +/**
> + * i40e_pf_reset_stats - Reset all of the stats for the given pf
> + * @pf: the PF to be reset
> + **/
> +void i40e_pf_reset_stats(struct i40e_pf *pf)
> +{
> +	memset(&pf->stats, 0, sizeof(struct i40e_hw_port_stats));
> +	memset(&pf->stats_offsets, 0, sizeof(struct i40e_hw_port_stats));
> +	pf->stat_offsets_loaded = false;
> +

Remove blank line above.

> +}
> +
> +/**
> + * i40e_stat_update48 - read and update a 48 bit stat from the chip
> + * @hw: ptr to the hardware info
> + * @hireg: the high 32 bit reg to read
> + * @loreg: the low 32 bit reg to read
> + * @offset_loaded: has the initial offset been loaded yet
> + * @offset: ptr to current offset value
> + * @stat: ptr to the stat
> + *
> + * Since the device stats are not reset at PFReset, they likely will not
> + * be zeroed when the driver starts.  We'll save the first values read
> + * and use them as offsets to be subtracted from the raw values in order
> + * to report stats that count from zero.  In the process, we also manage
> + * the potential roll-over.
> + **/
> +static void i40e_stat_update48(struct i40e_hw *hw, u32 hireg, u32 loreg,
> +			       bool offset_loaded, u64 *offset, u64 *stat)
> +{
> +	u64 new_data;

Missing a blank line.

> +	if (hw->device_id == I40E_QEMU_DEVICE_ID) {
> +		new_data = rd32(hw, loreg);
> +		new_data |= ((u64)(rd32(hw, hireg) & 0xFFFF)) << 32;
> +	} else {
> +		new_data = rd64(hw, loreg);
> +	}
> +	if (!offset_loaded)
> +		*offset = new_data;
> +	if (likely(new_data >= *offset))
> +		*stat = new_data - *offset;
> +	else
> +		*stat = (new_data + ((u64)1 << 48)) - *offset;
> +	*stat &= 0xFFFFFFFFFFFFULL;
> +}
> +
> +/**
> + * i40e_stat_update32 - read and update a 32 bit stat from the chip
> + * @hw: ptr to the hardware info
> + * @reg: the hw reg to read
> + * @offset_loaded: has the initial offset been loaded yet
> + * @offset: ptr to current offset value
> + * @stat: ptr to the stat
> + **/
> +static void i40e_stat_update32(struct i40e_hw *hw, u32 reg,
> +			       bool offset_loaded, u64 *offset, u64 *stat)
> +{
> +	u32 new_data;

Missing a blank line.

> +	new_data = rd32(hw, reg);
> +	if (!offset_loaded)
> +		*offset = new_data;
> +	if (likely(new_data >= *offset))
> +		*stat = (u32)(new_data - *offset);
> +	else
> +		*stat = (u32)((new_data + ((u64)1 << 32)) - *offset);
> +}

[...]

> +/**
> + * i40e_is_vsi_in_vlan - Check if VSI is in vlan mode
> + * @vsi: the VSI to be searched
> + *
> + * Returns true if VSI is in vlan mode or false otherwise
> + **/
> +bool i40e_is_vsi_in_vlan(struct i40e_vsi *vsi)
> +{
> +	struct i40e_mac_filter *f;
> +
> +	/* Only -1 for all the filters denotes not in vlan mode
> +	 * so we have to go through all the list in order to make sure
> +	 */
> +	list_for_each_entry(f, &vsi->mac_filter_list, list) {
> +		if (f->vlan < 0)
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
> +/**
> + * i40e_put_mac_in_vlan - Goes through all the macvlan filters and adds a
> + *  macvlan filter for each unique vlan that already exists

Superfluous space at the beginning.

> + * @vsi: the VSI to be searched
> + * @macaddr: the mac address to be filtered
> + * @is_vf: true if it is a vf
> + * @is_netdev: true if it is a netdev
> + *
> + * Returns I40E_SUCCESS on success or -ENOMEM if it could not add a filter
> + **/
> +enum i40e_status_code i40e_put_mac_in_vlan(struct i40e_vsi *vsi,
> +						  u8 *macaddr,
> +						  bool is_vf, bool is_netdev)

This could be better indented, aligned with the struct i40e_vsi.

> +{
> +	struct i40e_mac_filter *f, *add_f;
> +
> +	list_for_each_entry(f, &vsi->mac_filter_list, list) {
> +		if (!i40e_find_filter(vsi, macaddr, f->vlan,
> +				      is_vf, is_netdev)) {
> +			add_f = i40e_add_filter(vsi, macaddr, f->vlan,
> +						is_vf, is_netdev);
> +
> +			if (NULL == add_f) {
> +				dev_info(&vsi->back->pdev->dev, "%s: Could not add filter %d for %pM\n",
> +					 __func__, f->vlan, f->macaddr);
> +				return -ENOMEM;
> +			}
> +		}
> +	}
> +	return I40E_SUCCESS;
> +}
> +
> +/**
> + * i40e_add_filter - Add a mac/vlan filter to the VSI
> + * @vsi: the VSI to be searched
> + * @macaddr: the MAC address
> + * @vlan: the vlan
> + * @is_vf: make sure its a vf filter, else doesn't matter
> + * @is_netdev: make sure its a netdev filter, else doesn't matter
> + *
> + * Returns ptr to the filter object or NULL when no memory available.
> + **/
> +struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi,
> +					u8 *macaddr, s16 vlan,
> +					bool is_vf, bool is_netdev)
> +{
> +	struct i40e_mac_filter *f;

Just f seems to be a bad naming, can we come up with a more fitting
name for this?

> +
> +	if (!vsi || !macaddr)
> +		return NULL;
> +
> +	f = i40e_find_filter(vsi, macaddr, vlan, is_vf, is_netdev);
> +	if (NULL == f) {
> +		f = kzalloc(sizeof(*f), GFP_ATOMIC);
> +		if (NULL == f)
> +			goto add_filter_out;
> +
> +		memcpy(f->macaddr, macaddr, ETH_ALEN);
> +		f->vlan = vlan;
> +		f->changed = true;
> +
> +		INIT_LIST_HEAD(&f->list);
> +		list_add(&f->list, &vsi->mac_filter_list);
> +	}
> +
> +	/* increment counter and add a new flag if needed */
> +	if (is_vf) {
> +		if (!f->is_vf) {
> +			f->is_vf = true;
> +			f->counter++;
> +		}
> +	} else if (is_netdev) {
> +		if (!f->is_netdev) {
> +			f->is_netdev = true;
> +			f->counter++;
> +		}
> +	} else {
> +		f->counter++;
> +	}
> +
> +	/* changed tells sync_filters_subtask to
> +	 * push the filter down to the firmware
> +	 */
> +	if (f->changed) {
> +		vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
> +		vsi->back->flags |= I40E_FLAG_FILTER_SYNC;
> +	}
> +
> +add_filter_out:
> +	return f;
> +}
> +
> +/**
> + * i40e_del_filter - Remove a mac/vlan filter from the VSI
> + * @vsi: the VSI to be searched
> + * @macaddr: the MAC address
> + * @vlan: the vlan
> + * @is_vf: make sure it's a vf filter, else doesn't matter
> + * @is_netdev: make sure it's a netdev filter, else doesn't matter
> + **/
> +void i40e_del_filter(struct i40e_vsi *vsi,
> +		     u8 *macaddr, s16 vlan,
> +		     bool is_vf, bool is_netdev)

Again, please indent properly. I'm not going to make further comments
about this.

> +{
> +	struct i40e_mac_filter *f;
> +
> +	if (!vsi || !macaddr)
> +		return;
> +
> +	f = i40e_find_filter(vsi, macaddr, vlan, is_vf, is_netdev);
> +	if (NULL == f || f->counter == 0)
> +		goto del_filter_out;
> +
> +	if (is_vf) {
> +		if (f->is_vf) {
> +			f->is_vf = false;
> +			f->counter--;
> +		}
> +	} else if (is_netdev) {
> +		if (f->is_netdev) {
> +			f->is_netdev = false;
> +			f->counter--;
> +		}
> +	} else {
> +		/* make sure we don't remove a filter in use by vf or netdev */
> +		int min_f = 0;
> +		min_f += (f->is_vf ? 1 : 0);
> +		min_f += (f->is_netdev ? 1 : 0);
> +
> +		if (f->counter > min_f)
> +			f->counter--;
> +	}
> +
> +	/* counter == 0 tells sync_filters_subtask to
> +	 * remove the filter from the firmware's list
> +	 */
> +	if (f->counter == 0) {
> +		f->changed = true;
> +		vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
> +		vsi->back->flags |= I40E_FLAG_FILTER_SYNC;
> +	}
> +
> +del_filter_out:
> +	return;

[...]

> +/**
> + * i40e_sync_vsi_filters - Update the VSI filter list to the HW
> + * @vsi: ptr to the VSI
> + *
> + * Push any outstanding VSI filter changes through the AdminQ.
> + *
> + * Returns I40E_SUCCESS or error value
> + **/
> +enum i40e_status_code i40e_sync_vsi_filters(struct i40e_vsi *vsi)
> +{
> +	struct i40e_mac_filter *f, *ftmp;
> +	struct i40e_pf *pf;
> +	int num_add = 0;
> +	int num_del = 0;
> +	u32 changed_flags = 0;
> +	bool add_happened = false;
> +	bool promisc_forced_on = false;
> +	enum i40e_status_code ret = I40E_SUCCESS;
> +	u16 cmd_flags;
> +
> +#define FILTER_LIST_LEN 30

Having the defines in one place at the top of the file would be nice,
unless there's a special reason to have it here.

> +	/* empty array typed pointers, kcalloc later */
> +	struct i40e_aqc_add_macvlan_element_data *add_list;
> +	struct i40e_aqc_remove_macvlan_element_data *del_list;
> +
> +	if (!vsi)
> +		return I40E_ERR_PARAM;
> +	while (test_and_set_bit(__I40E_CONFIG_BUSY, &vsi->state))
> +		usleep_range(1000, 2000);
> +	pf = vsi->back;
> +
> +	if (vsi->netdev) {
> +		changed_flags = vsi->current_netdev_flags ^ vsi->netdev->flags;
> +		vsi->current_netdev_flags = vsi->netdev->flags;
> +	}
> +
> +	if (vsi->flags & I40E_VSI_FLAG_FILTER_CHANGED) {
> +		vsi->flags &= ~I40E_VSI_FLAG_FILTER_CHANGED;
> +
> +		del_list = kcalloc(FILTER_LIST_LEN,
> +			    sizeof(struct i40e_aqc_remove_macvlan_element_data),
> +			    GFP_KERNEL);
> +		if (!del_list)
> +			return I40E_ERR_NO_MEMORY;
> +
> +		list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) {
> +			if (!f->changed)
> +				continue;
> +
> +			if (f->counter != 0)
> +				continue;
> +			f->changed = false;
> +			cmd_flags = 0;
> +
> +			/* add to delete list */
> +			memcpy(del_list[num_del].mac_addr,
> +			       f->macaddr, ETH_ALEN);
> +			del_list[num_del].vlan_tag =
> +				cpu_to_le16((u16)(f->vlan ==
> +					    I40E_VLAN_ANY ? 0 : f->vlan));
> +
> +			/* vlan0 as wild card to allow packets from all vlans */
> +			if (f->vlan == I40E_VLAN_ANY ||
> +			    (vsi->netdev && !(vsi->netdev->features &
> +					      NETIF_F_HW_VLAN_CTAG_FILTER)))
> +				cmd_flags |= I40E_AQC_MACVLAN_DEL_IGNORE_VLAN;
> +			cmd_flags |= I40E_AQC_MACVLAN_DEL_PERFECT_MATCH;
> +			del_list[num_del].flags = cpu_to_le16(cmd_flags);
> +			num_del++;
> +
> +			/* unlink from filter list */
> +			list_del(&f->list);
> +			kfree(f);
> +
> +			/* flush a full buffer */
> +			if (num_del == FILTER_LIST_LEN) {
> +				ret = i40e_aq_remove_macvlan(&pf->hw,
> +					    vsi->seid, del_list, num_del,
> +					    NULL);
> +				num_del = 0;
> +				memset(del_list, 0, sizeof(*del_list));
> +
> +				if (ret != I40E_SUCCESS)
> +					dev_info(&pf->pdev->dev,

Maybe use netdev_info() instead of dev_info() in the whole driver? Would
be nice if this was consistent.

[...]

> +/**
> + * i40e_change_mtu - NDO callback to change the Maximum Transfer Unit
> + * @netdev: network interface device structure
> + * @new_mtu: new value for maximum frame size
> + *
> + * Returns 0 on success, negative on failure
> + **/
> +static int i40e_change_mtu(struct net_device *netdev, int new_mtu)
> +{
> +	struct i40e_netdev_priv *np = netdev_priv(netdev);
> +	struct i40e_vsi *vsi = np->vsi;
> +	int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
> +
> +	/* MTU < 68 is an error and causes problems on some kernels */
> +	if ((new_mtu < 68) || (max_frame > I40E_MAX_RXBUFFER))

Maybe add another netdev_info that MTU < 68 is not supported? Not sure
if it makes sense to set it that low. Never mind if it's just a corner
case.

> +		return -EINVAL;
> +
> +	netdev_info(netdev, "%s: changing MTU from %d to %d\n",
> +		    __func__, netdev->mtu, new_mtu);
> +	netdev->mtu = new_mtu;
> +	if (netif_running(netdev))
> +		i40e_vsi_reinit_locked(vsi);
> +
> +	return 0;
> +}

[...]

> +/**
> + * i40e_enable_misc_int_causes - enable the non-queue interrupts
> + * @hw: ptr to the hardware info
> + **/
> +static void i40e_enable_misc_int_causes(struct i40e_hw *hw)
> +{
> +	u32 val;
> +
> +	/* clear things first */
> +	wr32(hw, I40E_PFINT_ICR0_ENA, 0);  /* disable all */
> +	rd32(hw, I40E_PFINT_ICR0);         /* read to clear */
> +
> +	val = I40E_PFINT_ICR0_ENA_ECC_ERR_MASK	     |
> +	      I40E_PFINT_ICR0_ENA_MAL_DETECT_MASK    |
> +	      I40E_PFINT_ICR0_ENA_GRST_MASK	     |
> +	      I40E_PFINT_ICR0_ENA_PCI_EXCEPTION_MASK |
> +	      I40E_PFINT_ICR0_ENA_GPIO_MASK	     |
> +	      I40E_PFINT_ICR0_ENA_STORM_DETECT_MASK  |
> +	      I40E_PFINT_ICR0_ENA_HMC_ERR_MASK	     |
> +	      I40E_PFINT_ICR0_ENA_VFLR_MASK          |
> +	      I40E_PFINT_ICR0_ENA_ADMINQ_MASK;

Inconsistent mix of tabs and whitespaces in the lines above. I guess you
just missed the tab at the end of I40E_PFINT_ICR0_ENA_VFLR_MASK.

> +
> +	wr32(hw, I40E_PFINT_ICR0_ENA, val);
> +
> +	/* SW_ITR_IDX = 0, but don't change INTENA */
> +	wr32(hw, I40E_PFINT_DYN_CTL0, I40E_PFINT_DYN_CTLN_SW_ITR_INDX_MASK |
> +					I40E_PFINT_DYN_CTLN_INTENA_MSK_MASK);
> +
> +	/* OTHER_ITR_IDX = 0 */
> +	wr32(hw, I40E_PFINT_STAT_CTL0, 0);
> +}

[...]

> +/**
> + * i40e_vsi_configure_bw_alloc - Configure VSI BW allocation per TC
> + * @vsi: the VSI being configured
> + * @enabled_tc: TC bitmap
> + * @bw_credits: BW shared credits per TC
> + *
> + * Returns 0 on success, negative value on failure
> + **/
> +static s32 i40e_vsi_configure_bw_alloc(struct i40e_vsi *vsi,
> +				       u8 enabled_tc,
> +				       u8 *bw_share)
> +{
> +	int i, ret = 0;
> +	struct i40e_aqc_configure_vsi_tc_bw_data bw_data;
> +
> +	bw_data.tc_valid_bits = enabled_tc;
> +	for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++)
> +		bw_data.tc_bw_credits[i] = bw_share[i];
> +
> +	ret = i40e_aq_config_vsi_tc_bw(&vsi->back->hw, vsi->seid,
> +				       &bw_data, NULL);
> +	if (ret != I40E_SUCCESS) {
> +		dev_info(&vsi->back->pdev->dev,
> +			 "%s: AQ command Config VSI BW allocation per TC failed = %d\n",
> +			  __func__, vsi->back->hw.aq.asq_last_status);
> +		return ret;
> +	}
> +
> +	for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++)
> +		vsi->info.qs_handle[i] = bw_data.qs_handles[i];
> +
> +	return ret;
> +

superfluous blank line.

> +}

[...]

> +/**
> + * i40e_do_reset - Start a PF or Core Reset sequence
> + * @pf: board private structure
> + * @reset_flags: which reset is requested
> + *
> + * The essential difference in resets is that the PF Reset
> + * doesn't clear the packet buffers, doesn't reset the PE
> + * firmware, and doesn't bother the other PFs on the chip.
> + **/
> +void i40e_do_reset(struct i40e_pf *pf, u32 reset_flags)
> +{
> +	u32 val;
> +
> +	WARN_ON(in_interrupt());
> +
> +	/* do the biggest reset indicated */
> +	if (reset_flags & (1 << __I40E_GLOBAL_RESET_REQUESTED)) {
> +
> +		/* Request a Global Reset
> +		 *
> +		 * This will start the chip's countdown to the actual full
> +		 * chip reset event, and a warning interrupt to be sent
> +		 * to all PFs, including the requestor.  Our handler
> +		 * for the warning interrupt will deal with the shutdown
> +		 * and recovery of the switch setup.
> +		 *
> +		 * GlobR includes the MAC/PHY in the reset.
> +		 */
> +		dev_info(&pf->pdev->dev, "%s: GlobalR requested\n", __func__);
> +		val = rd32(&pf->hw, I40E_GLGEN_RTRIG);
> +		val |= I40E_GLGEN_RTRIG_GLOBR_MASK;
> +		wr32(&pf->hw, I40E_GLGEN_RTRIG, val);
> +
> +	} else if (reset_flags & (1 << __I40E_CORE_RESET_REQUESTED)) {
> +
> +		/* Request a Core Reset
> +		 *
> +		 * This will start the chip's countdown to the actual full
> +		 * chip reset event, and a warning interrupt to be sent
> +		 * to all PFs, including the requestor.  Our handler
> +		 * for the warning interrupt will deal with the shutdown
> +		 * and recovery of the switch setup.
> +		 */

Both comments for global and core reset are identical except for one
more line. Could you change the core reset comment to something like:
"Same as Global Reset excluding MAC/PHY" ?

> +		dev_info(&pf->pdev->dev, "%s: CoreR requested\n", __func__);
> +		val = rd32(&pf->hw, I40E_GLGEN_RTRIG);
> +		val |= I40E_GLGEN_RTRIG_CORER_MASK;
> +		wr32(&pf->hw, I40E_GLGEN_RTRIG, val);
> +		flush(&pf->hw);
> +
> +	} else if (reset_flags & (1 << __I40E_PF_RESET_REQUESTED)) {
> +
> +		/* Request a PF Reset
> +		 *
> +		 * This goes directly to the tear-down and rebuild of
> +		 * the switch, since we need to do the same recovery as
> +		 * for the Core Reset.
> +		 */
> +		dev_info(&pf->pdev->dev, "%s: PFR requested\n", __func__);
> +		i40e_handle_reset_warning(pf);
> +
> +	} else if (reset_flags & (1 << __I40E_REINIT_REQUESTED)) {
> +		int v;
> +
> +		/* Find the VSI(s) that requested a re-init */
> +		dev_info(&pf->pdev->dev,
> +			 "%s: VSI reinit requested\n", __func__);
> +		for (v = 0; v < pf->hw.func_caps.num_vsis; v++) {
> +			struct i40e_vsi *vsi = pf->vsi[v];
> +			if (vsi != NULL &&
> +			    test_bit(__I40E_REINIT_REQUESTED, &vsi->state)) {
> +				i40e_vsi_reinit_locked(pf->vsi[v]);
> +				clear_bit(__I40E_REINIT_REQUESTED, &vsi->state);
> +			}
> +		}
> +
> +		/* no further action needed, so return now */
> +		return;
> +	} else {
> +		dev_info(&pf->pdev->dev,
> +			 "%s: bad reset request 0x%08x\n",
> +			 __func__, reset_flags);
> +		return;
> +	}
> +

superfluous blank line.

> +}

[...]

> +/**
> + * i40e_link_event - Update netif_carrier status
> + * @pf: board private structure
> + **/
> +static void i40e_link_event(struct i40e_pf *pf)
> +{
> +	bool new_link, old_link;
> +
> +	new_link = (pf->hw.phy.link_info.link_info & I40E_AQ_LINK_UP);
> +	old_link = (pf->hw.phy.link_info_old.link_info & I40E_AQ_LINK_UP);
> +
> +	if (new_link == old_link)
> +		return;
> +
> +	netdev_info(pf->vsi[pf->lan_vsi]->netdev,
> +		    "%s: NIC Link is %s\n",
> +		    __func__, (new_link ? "Up" : "Down"));
> +
> +	/* Notify the base of the switch tree connected to
> +	 * the link.  Floating VEBs are not notified.

What's a floating VEB?

> +	 */
> +	if (pf->lan_veb != I40E_NO_VEB && pf->veb[pf->lan_veb])
> +		i40e_veb_link_event(pf->veb[pf->lan_veb], new_link);
> +	else
> +		i40e_vsi_link_event(pf->vsi[pf->lan_vsi], new_link);
> +
> +	if (pf->vf)
> +		i40e_vc_notify_link_state(pf);
> +}

[...]

> +/**
> + * i40e_watchdog_subtask - Check and bring link up
> + * @pf: board private structure
> + **/
> +static void i40e_watchdog_subtask(struct i40e_pf *pf)
> +{
> +	int v;

Why did you call the variable v and not i?

> +
> +	/* if interface is down do nothing */
> +	if (test_bit(__I40E_DOWN, &pf->state) ||
> +	    test_bit(__I40E_CONFIG_BUSY, &pf->state))
> +		return;
> +
> +	/* Update the stats for active netdevs so the network stack
> +	 * can look at updated numbers whenever it cares to
> +	 */
> +	for (v = 0; v < pf->hw.func_caps.num_vsis; v++)
> +		if (pf->vsi[v] && pf->vsi[v]->netdev)
> +			i40e_update_stats(pf->vsi[v]);
> +
> +	/* Update the stats for the active switching components */
> +	for (v = 0; v < I40E_MAX_VEB; v++)
> +		if (pf->veb[v])
> +			i40e_update_veb_stats(pf->veb[v]);
> +}

[...]

> +/**
> + * i40e_handle_reset_warning - prep for the core to reset
> + * @pf: board private structure
> + *
> + * Close up the VFs and other things in prep for a Core Reset,
> + * then get ready to rebuild the world.
> + **/
> +static void i40e_handle_reset_warning(struct i40e_pf *pf)
> +{

[...]

> +	/* reinit the misc interrupt */
> +	if (pf->flags & I40E_FLAG_MSIX_ENABLED)

Did I understand this right, the misc interrupt is now handled by the
legacy IRQ handler?

[...]

> +/**
> + * i40e_service_task - Run the driver's async subtasks
> + * @work: pointer to work_struct containing our data
> + **/
> +static void i40e_service_task(struct work_struct *work)
> +{
> +	struct i40e_pf *pf = container_of(work,
> +					  struct i40e_pf,
> +					  service_task);
> +	unsigned long start_time = jiffies;
> +
> +	i40e_reset_subtask(pf);
> +	i40e_handle_mdd_event(pf);
> +	i40e_vc_process_vflr_event(pf);
> +	i40e_watchdog_subtask(pf);
> +	i40e_fdir_reinit_subtask(pf);
> +	i40e_check_hang_subtask(pf);
> +	i40e_sync_filters_subtask(pf);
> +	i40e_clean_adminq_subtask(pf);
> +

This blank line could probably removed as well or did you add it for a
special reason?

> +	i40e_service_event_complete(pf);
> +
> +	/* If the tasks have taken longer than one timer cycle or there
> +	 * is more work to be done, reschedule the service task now
> +	 * rather than wait for the timer to tick again.
> +	 */
> +	if (time_after(jiffies, (start_time + pf->service_timer_period)) ||
> +	    test_bit(__I40E_ADMINQ_EVENT_PENDING, &pf->state)		 ||
> +	    test_bit(__I40E_MDD_EVENT_PENDING, &pf->state)		 ||
> +	    test_bit(__I40E_VFLR_EVENT_PENDING, &pf->state))
> +		i40e_service_event_schedule(pf);
> +}

[...]

> +	/* prep for VF support */
> +	if ((pf->flags & I40E_FLAG_SRIOV_ENABLED) &&
> +	    (pf->flags & I40E_FLAG_MSIX_ENABLED)) {
> +		u32 val;
> +
> +		/* disable link interrupts for VFs */
> +		val = rd32(hw, I40E_PFGEN_PORTMDIO_NUM);
> +		val &= ~I40E_PFGEN_PORTMDIO_NUM_VFLINK_STAT_ENA_MASK;
> +		wr32(hw, I40E_PFGEN_PORTMDIO_NUM, val);
> +		flush(hw);
> +

superfluous blank line.

> +	}

  Stefan

^ permalink raw reply

* Re: [PATCH net-next 10/10] staging: vt6656: inherit addr_assign_type along with dev_addr
From: Bjørn Mork @ 2013-08-23 12:06 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: devel, netdev, Forest Bond, Greg Kroah-Hartman
In-Reply-To: <20130823111500.GC4713@mwanda>

Dan Carpenter <dan.carpenter@oracle.com> writes:

> Ah...  Here is the first patch which adds eth_hw_addr_inherit()
>
> http://patchwork.ozlabs.org/patch/269365/
>
> I don't think we've actually set dst->addr_len yet at this point so
> it doesn't do the memcpy().  This doesn't work.

Ouch.  Yes, I see. The net_device is allocated using kzalloc just a few
lines earlier and there is no ether_setup or similar.  Actually, it
doesn't look like it ever sets addr_len. Is that right?  Does it work
even before this patch?

Anyway, thanks for spotting this.  I'll drop this patch for now as I
clearly don't understand how this driver works.

I do note that the otherwise very similar code in the vt6655 driver uses
alloc_etherdev, and therefore has a fair chance of working both before
and after my change.


Bjørn
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply

* Re: [PATCH v2 5/5] ARM: dts: am33xx: adopt to cpsw changes
From: Daniel Mack @ 2013-08-23 12:08 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: netdev, sergei.shtylyov, davem, ujhelyi.m, mugunthanvnm,
	vaibhav.bedia, d-gerlach, linux-arm-kernel, linux-omap,
	devicetree
In-Reply-To: <52172E12.2000908@ti.com>

On 23.08.2013 11:40, Sekhar Nori wrote:
> On Friday 23 August 2013 03:04 PM, Daniel Mack wrote:
>> On 23.08.2013 11:28, Sekhar Nori wrote:

>> Ok, thanks. Given that this last patch will be merged through another
>> tree, can I just resend it separately, while David takes the rest as it is?
> 
> Ideally you could, but there are some bugs in the way compatible string
> in handled and due to this I suspect you need to reorder
> cpsw_of_mtable[] to match correctly. Not sure if the bug has been fixed
> in recent -rcs. Please check if modifying just this patch works and then
> go ahead and post.

No, you're right. I had to swap the two entries. Thanks for mentioning it.

> Please Cc Benoit Cousson <bcousson@baylibre.com> on your next submission
> as he is the one who picks up DT patches for OMAP.

Will do.



Daniel

^ permalink raw reply

* [PATCH v3 1/5] net: ethernet: cpsw: switch to devres allocations
From: Daniel Mack @ 2013-08-23 12:08 UTC (permalink / raw)
  To: netdev
  Cc: bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
	mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree, Daniel Mack
In-Reply-To: <1377259735-20337-1-git-send-email-zonque@gmail.com>

This patch cleans up the allocation and error unwind paths, which
allows us to carry less information in struct cpsw_priv and reduce the
amount of jump labels in the probe functions.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/net/ethernet/ti/cpsw.c | 145 +++++++++++++----------------------------
 1 file changed, 44 insertions(+), 101 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 79974e3..1a91aac 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -367,8 +367,6 @@ struct cpsw_priv {
 	spinlock_t			lock;
 	struct platform_device		*pdev;
 	struct net_device		*ndev;
-	struct resource			*cpsw_res;
-	struct resource			*cpsw_wr_res;
 	struct napi_struct		napi;
 	struct device			*dev;
 	struct cpsw_platform_data	data;
@@ -1712,62 +1710,55 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 
 	if (of_property_read_u32(node, "active_slave", &prop)) {
 		pr_err("Missing active_slave property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->active_slave = prop;
 
 	if (of_property_read_u32(node, "cpts_clock_mult", &prop)) {
 		pr_err("Missing cpts_clock_mult property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->cpts_clock_mult = prop;
 
 	if (of_property_read_u32(node, "cpts_clock_shift", &prop)) {
 		pr_err("Missing cpts_clock_shift property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->cpts_clock_shift = prop;
 
-	data->slave_data = kcalloc(data->slaves, sizeof(struct cpsw_slave_data),
-				   GFP_KERNEL);
+	data->slave_data = devm_kzalloc(&pdev->dev, data->slaves
+					* sizeof(struct cpsw_slave_data),
+					GFP_KERNEL);
 	if (!data->slave_data)
-		return -EINVAL;
+		return -ENOMEM;
 
 	if (of_property_read_u32(node, "cpdma_channels", &prop)) {
 		pr_err("Missing cpdma_channels property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->channels = prop;
 
 	if (of_property_read_u32(node, "ale_entries", &prop)) {
 		pr_err("Missing ale_entries property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->ale_entries = prop;
 
 	if (of_property_read_u32(node, "bd_ram_size", &prop)) {
 		pr_err("Missing bd_ram_size property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->bd_ram_size = prop;
 
 	if (of_property_read_u32(node, "rx_descs", &prop)) {
 		pr_err("Missing rx_descs property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->rx_descs = prop;
 
 	if (of_property_read_u32(node, "mac_control", &prop)) {
 		pr_err("Missing mac_control property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->mac_control = prop;
 
@@ -1794,8 +1785,7 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 		parp = of_get_property(slave_node, "phy_id", &lenp);
 		if ((parp == NULL) || (lenp != (sizeof(void *) * 2))) {
 			pr_err("Missing slave[%d] phy_id property\n", i);
-			ret = -EINVAL;
-			goto error_ret;
+			return -EINVAL;
 		}
 		mdio_node = of_find_node_by_phandle(be32_to_cpup(parp));
 		phyid = be32_to_cpup(parp+1);
@@ -1825,10 +1815,6 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 	}
 
 	return 0;
-
-error_ret:
-	kfree(data->slave_data);
-	return ret;
 }
 
 static int cpsw_probe_dual_emac(struct platform_device *pdev,
@@ -1870,7 +1856,6 @@ static int cpsw_probe_dual_emac(struct platform_device *pdev,
 	priv_sl2->coal_intvl = 0;
 	priv_sl2->bus_freq_mhz = priv->bus_freq_mhz;
 
-	priv_sl2->cpsw_res = priv->cpsw_res;
 	priv_sl2->regs = priv->regs;
 	priv_sl2->host_port = priv->host_port;
 	priv_sl2->host_port_regs = priv->host_port_regs;
@@ -1914,8 +1899,8 @@ static int cpsw_probe(struct platform_device *pdev)
 	struct cpsw_priv		*priv;
 	struct cpdma_params		dma_params;
 	struct cpsw_ale_params		ale_params;
-	void __iomem			*ss_regs, *wr_regs;
-	struct resource			*res;
+	void __iomem			*ss_regs;
+	struct resource			*res, *ss_res;
 	u32 slave_offset, sliver_offset, slave_size;
 	int ret = 0, i, k = 0;
 
@@ -1951,7 +1936,7 @@ static int cpsw_probe(struct platform_device *pdev)
 	if (cpsw_probe_dt(&priv->data, pdev)) {
 		pr_err("cpsw: platform data missing\n");
 		ret = -ENODEV;
-		goto clean_ndev_ret;
+		goto clean_runtime_disable_ret;
 	}
 	data = &priv->data;
 
@@ -1965,11 +1950,12 @@ static int cpsw_probe(struct platform_device *pdev)
 
 	memcpy(ndev->dev_addr, priv->mac_addr, ETH_ALEN);
 
-	priv->slaves = kzalloc(sizeof(struct cpsw_slave) * data->slaves,
-			       GFP_KERNEL);
+	priv->slaves =
+		devm_kzalloc(&pdev->dev, sizeof(struct cpsw_slave) * data->slaves,
+			     GFP_KERNEL);
 	if (!priv->slaves) {
-		ret = -EBUSY;
-		goto clean_ndev_ret;
+		ret = -ENOMEM;
+		goto clean_runtime_disable_ret;
 	}
 	for (i = 0; i < data->slaves; i++)
 		priv->slaves[i].slave_num = i;
@@ -1977,55 +1963,41 @@ static int cpsw_probe(struct platform_device *pdev)
 	priv->slaves[0].ndev = ndev;
 	priv->emac_port = 0;
 
-	priv->clk = clk_get(&pdev->dev, "fck");
+	priv->clk = devm_clk_get(&pdev->dev, "fck");
 	if (IS_ERR(priv->clk)) {
-		dev_err(&pdev->dev, "fck is not found\n");
+		dev_err(priv->dev, "fck is not found\n");
 		ret = -ENODEV;
-		goto clean_slave_ret;
+		goto clean_runtime_disable_ret;
 	}
 	priv->coal_intvl = 0;
 	priv->bus_freq_mhz = clk_get_rate(priv->clk) / 1000000;
 
-	priv->cpsw_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!priv->cpsw_res) {
+	ss_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!ss_res) {
 		dev_err(priv->dev, "error getting i/o resource\n");
 		ret = -ENOENT;
-		goto clean_clk_ret;
+		goto clean_runtime_disable_ret;
 	}
-	if (!request_mem_region(priv->cpsw_res->start,
-				resource_size(priv->cpsw_res), ndev->name)) {
-		dev_err(priv->dev, "failed request i/o region\n");
-		ret = -ENXIO;
-		goto clean_clk_ret;
-	}
-	ss_regs = ioremap(priv->cpsw_res->start, resource_size(priv->cpsw_res));
+	ss_regs = devm_ioremap_resource(&pdev->dev, ss_res);
 	if (!ss_regs) {
 		dev_err(priv->dev, "unable to map i/o region\n");
-		goto clean_cpsw_iores_ret;
+		goto clean_runtime_disable_ret;
 	}
 	priv->regs = ss_regs;
 	priv->version = __raw_readl(&priv->regs->id_ver);
 	priv->host_port = HOST_PORT_NUM;
 
-	priv->cpsw_wr_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!priv->cpsw_wr_res) {
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res) {
 		dev_err(priv->dev, "error getting i/o resource\n");
 		ret = -ENOENT;
-		goto clean_iomap_ret;
-	}
-	if (!request_mem_region(priv->cpsw_wr_res->start,
-			resource_size(priv->cpsw_wr_res), ndev->name)) {
-		dev_err(priv->dev, "failed request i/o region\n");
-		ret = -ENXIO;
-		goto clean_iomap_ret;
+		goto clean_runtime_disable_ret;
 	}
-	wr_regs = ioremap(priv->cpsw_wr_res->start,
-				resource_size(priv->cpsw_wr_res));
-	if (!wr_regs) {
+	priv->wr_regs = devm_ioremap_resource(&pdev->dev, res);
+	if (!priv->wr_regs) {
 		dev_err(priv->dev, "unable to map i/o region\n");
-		goto clean_cpsw_wr_iores_ret;
+		goto clean_runtime_disable_ret;
 	}
-	priv->wr_regs = wr_regs;
 
 	memset(&dma_params, 0, sizeof(dma_params));
 	memset(&ale_params, 0, sizeof(ale_params));
@@ -2056,12 +2028,12 @@ static int cpsw_probe(struct platform_device *pdev)
 		slave_size           = CPSW2_SLAVE_SIZE;
 		sliver_offset        = CPSW2_SLIVER_OFFSET;
 		dma_params.desc_mem_phys =
-			(u32 __force) priv->cpsw_res->start + CPSW2_BD_OFFSET;
+			(u32 __force) ss_res->start + CPSW2_BD_OFFSET;
 		break;
 	default:
 		dev_err(priv->dev, "unknown version 0x%08x\n", priv->version);
 		ret = -ENODEV;
-		goto clean_cpsw_wr_iores_ret;
+		goto clean_runtime_disable_ret;
 	}
 	for (i = 0; i < priv->data.slaves; i++) {
 		struct cpsw_slave *slave = &priv->slaves[i];
@@ -2089,7 +2061,7 @@ static int cpsw_probe(struct platform_device *pdev)
 	if (!priv->dma) {
 		dev_err(priv->dev, "error initializing dma\n");
 		ret = -ENOMEM;
-		goto clean_wr_iomap_ret;
+		goto clean_runtime_disable_ret;
 	}
 
 	priv->txch = cpdma_chan_create(priv->dma, tx_chan_num(0),
@@ -2124,8 +2096,8 @@ static int cpsw_probe(struct platform_device *pdev)
 
 	while ((res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, k))) {
 		for (i = res->start; i <= res->end; i++) {
-			if (request_irq(i, cpsw_interrupt, 0,
-					dev_name(&pdev->dev), priv)) {
+			if (devm_request_irq(&pdev->dev, i, cpsw_interrupt, 0,
+					     dev_name(priv->dev), priv)) {
 				dev_err(priv->dev, "error attaching irq\n");
 				goto clean_ale_ret;
 			}
@@ -2147,7 +2119,7 @@ static int cpsw_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(priv->dev, "error registering net device\n");
 		ret = -ENODEV;
-		goto clean_irq_ret;
+		goto clean_ale_ret;
 	}
 
 	if (cpts_register(&pdev->dev, priv->cpts,
@@ -2155,44 +2127,27 @@ static int cpsw_probe(struct platform_device *pdev)
 		dev_err(priv->dev, "error registering cpts device\n");
 
 	cpsw_notice(priv, probe, "initialized device (regs %x, irq %d)\n",
-		  priv->cpsw_res->start, ndev->irq);
+		    ss_res->start, ndev->irq);
 
 	if (priv->data.dual_emac) {
 		ret = cpsw_probe_dual_emac(pdev, priv);
 		if (ret) {
 			cpsw_err(priv, probe, "error probe slave 2 emac interface\n");
-			goto clean_irq_ret;
+			goto clean_ale_ret;
 		}
 	}
 
 	return 0;
 
-clean_irq_ret:
-	for (i = 0; i < priv->num_irqs; i++)
-		free_irq(priv->irqs_table[i], priv);
 clean_ale_ret:
 	cpsw_ale_destroy(priv->ale);
 clean_dma_ret:
 	cpdma_chan_destroy(priv->txch);
 	cpdma_chan_destroy(priv->rxch);
 	cpdma_ctlr_destroy(priv->dma);
-clean_wr_iomap_ret:
-	iounmap(priv->wr_regs);
-clean_cpsw_wr_iores_ret:
-	release_mem_region(priv->cpsw_wr_res->start,
-			   resource_size(priv->cpsw_wr_res));
-clean_iomap_ret:
-	iounmap(priv->regs);
-clean_cpsw_iores_ret:
-	release_mem_region(priv->cpsw_res->start,
-			   resource_size(priv->cpsw_res));
-clean_clk_ret:
-	clk_put(priv->clk);
-clean_slave_ret:
+clean_runtime_disable_ret:
 	pm_runtime_disable(&pdev->dev);
-	kfree(priv->slaves);
 clean_ndev_ret:
-	kfree(priv->data.slave_data);
 	free_netdev(priv->ndev);
 	return ret;
 }
@@ -2201,30 +2156,18 @@ static int cpsw_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct cpsw_priv *priv = netdev_priv(ndev);
-	int i;
 
 	if (priv->data.dual_emac)
 		unregister_netdev(cpsw_get_slave_ndev(priv, 1));
 	unregister_netdev(ndev);
 
 	cpts_unregister(priv->cpts);
-	for (i = 0; i < priv->num_irqs; i++)
-		free_irq(priv->irqs_table[i], priv);
 
 	cpsw_ale_destroy(priv->ale);
 	cpdma_chan_destroy(priv->txch);
 	cpdma_chan_destroy(priv->rxch);
 	cpdma_ctlr_destroy(priv->dma);
-	iounmap(priv->regs);
-	release_mem_region(priv->cpsw_res->start,
-			   resource_size(priv->cpsw_res));
-	iounmap(priv->wr_regs);
-	release_mem_region(priv->cpsw_wr_res->start,
-			   resource_size(priv->cpsw_wr_res));
 	pm_runtime_disable(&pdev->dev);
-	clk_put(priv->clk);
-	kfree(priv->slaves);
-	kfree(priv->data.slave_data);
 	if (priv->data.dual_emac)
 		free_netdev(cpsw_get_slave_ndev(priv, 1));
 	free_netdev(ndev);
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v3 2/5] net: ethernet: cpsw: add optional third memory region for CONTROL module
From: Daniel Mack @ 2013-08-23 12:08 UTC (permalink / raw)
  To: netdev
  Cc: bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
	mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree, Daniel Mack
In-Reply-To: <1377259735-20337-1-git-send-email-zonque@gmail.com>

At least the AM33xx SoC has a control module register to configure
details such as the hardware ethernet interface mode.

I'm not sure whether all SoCs which feature the cpsw block have such a
register, so that third memory region is considered optional for now.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt |  5 ++++-
 drivers/net/ethernet/ti/cpsw.c                 | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index 05d660e..4e5ca54 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -4,7 +4,10 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
 Required properties:
 - compatible		: Should be "ti,cpsw"
 - reg			: physical base address and size of the cpsw
-			  registers map
+			  registers map.
+			  An optional third memory region can be supplied if
+			  the platform has a control module register to
+			  configure phy interface details
 - interrupts		: property with a value describing the interrupt
 			  number
 - interrupt-parent	: The parent interrupt controller
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 1a91aac..bd0b664 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -372,6 +372,7 @@ struct cpsw_priv {
 	struct cpsw_platform_data	data;
 	struct cpsw_ss_regs __iomem	*regs;
 	struct cpsw_wr_regs __iomem	*wr_regs;
+	u32 __iomem			*gmii_sel_reg;
 	u8 __iomem			*hw_stats;
 	struct cpsw_host_regs __iomem	*host_port_regs;
 	u32				msg_enable;
@@ -1999,6 +2000,21 @@ static int cpsw_probe(struct platform_device *pdev)
 		goto clean_runtime_disable_ret;
 	}
 
+	/* If the control memory region is unspecified, continue without it.
+	 * If it is specified, but we're unable to reserve it, bail.
+	 */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (!res) {
+		dev_err(priv->dev, "error getting control i/o resource\n");
+		goto no_gmii_sel;
+	}
+	priv->gmii_sel_reg = devm_ioremap_resource(&pdev->dev, res);
+	if (!priv->gmii_sel_reg) {
+		dev_err(priv->dev, "unable to map control i/o region\n");
+		goto clean_runtime_disable_ret;
+	}
+
+no_gmii_sel:
 	memset(&dma_params, 0, sizeof(dma_params));
 	memset(&ale_params, 0, sizeof(ale_params));
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v3 4/5] net: ethernet: cpsw: add support for hardware interface mode config
From: Daniel Mack @ 2013-08-23 12:08 UTC (permalink / raw)
  To: netdev
  Cc: bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
	mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree, Daniel Mack
In-Reply-To: <1377259735-20337-1-git-send-email-zonque@gmail.com>

The cpsw currently lacks code to properly set up the hardware interface
mode on AM33xx. Other platforms might be equally affected.

Usually, the bootloader will configure the control module register, so
probably that's why such support wasn't needed in the past. In suspend
mode though, this register is modified, and so it needs reprogramming
after resume.

This patch adds code that makes use of the previously added and optional
support for passing the control mode register, and configures the
correct register bits when the slave is opened.

The AM33xx also has a bit for each slave to configure the RMII reference
clock direction. Setting it is now supported by a per-slave DT property.

This code path introducted by this patch is currently exclusive for
am33xx.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt |  2 +
 drivers/net/ethernet/ti/cpsw.c                 | 61 ++++++++++++++++++++++++++
 drivers/net/ethernet/ti/cpsw.h                 |  1 +
 3 files changed, 64 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index b717458..0895a51 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -34,6 +34,8 @@ Required properties:
 - phy_id		: Specifies slave phy id
 - phy-mode		: The interface between the SoC and the PHY (a string
 			  that of_get_phy_mode() can understand)
+- ti,rmii-clock-ext	: If present, the driver will configure the RMII
+			  interface to external clock usage
 - mac-address		: Specifies slave MAC address
 
 Optional properties:
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index b194529..b839501 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -138,6 +138,13 @@ do {								\
 #define CPSW_CMINTMAX_INTVL	(1000 / CPSW_CMINTMIN_CNT)
 #define CPSW_CMINTMIN_INTVL	((1000 / CPSW_CMINTMAX_CNT) + 1)
 
+#define AM33XX_GMII_SEL_MODE_MII	(0)
+#define AM33XX_GMII_SEL_MODE_RMII	(1)
+#define AM33XX_GMII_SEL_MODE_RGMII	(2)
+
+#define AM33XX_GMII_SEL_RMII2_IO_CLK_EN	BIT(7)
+#define AM33XX_GMII_SEL_RMII1_IO_CLK_EN	BIT(6)
+
 #define cpsw_enable_irq(priv)	\
 	do {			\
 		u32 i;		\
@@ -980,6 +987,56 @@ static inline void cpsw_add_dual_emac_def_ale_entries(
 		priv->host_port, ALE_VLAN, slave->port_vlan);
 }
 
+static void cpsw_set_phy_interface_mode(struct cpsw_slave *slave,
+					struct cpsw_priv *priv)
+{
+	u32 reg, mask, mode = 0;
+
+	switch (priv->data.hw_type) {
+	case CPSW_TYPE_AM33XX:
+		if (!priv->gmii_sel_reg)
+			break;
+
+		reg = readl(priv->gmii_sel_reg);
+
+		if (slave->phy) {
+			switch (slave->phy->interface) {
+			case PHY_INTERFACE_MODE_MII:
+			default:
+				mode = AM33XX_GMII_SEL_MODE_MII;
+				break;
+			case PHY_INTERFACE_MODE_RMII:
+				mode = AM33XX_GMII_SEL_MODE_RMII;
+				break;
+			case PHY_INTERFACE_MODE_RGMII:
+				mode = AM33XX_GMII_SEL_MODE_RGMII;
+				break;
+			};
+		}
+
+		mask = 0x3 << (slave->slave_num * 2) |
+		       BIT(slave->slave_num + 6);
+		mode <<= slave->slave_num * 2;
+
+		if (slave->data->rmii_clock_external) {
+			if (slave->slave_num == 0)
+				mode |= AM33XX_GMII_SEL_RMII1_IO_CLK_EN;
+			else
+				mode |= AM33XX_GMII_SEL_RMII2_IO_CLK_EN;
+		}
+
+		reg &= ~mask;
+		reg |= mode;
+
+		writel(reg, priv->gmii_sel_reg);
+		break;
+
+	default:
+		break;
+
+	}
+}
+
 static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 {
 	char name[32];
@@ -1028,6 +1085,8 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 			 slave->phy->phy_id);
 		phy_start(slave->phy);
 	}
+
+	cpsw_set_phy_interface_mode(slave, priv);
 }
 
 static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)
@@ -1823,6 +1882,8 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 			memcpy(slave_data->mac_addr, mac_addr, ETH_ALEN);
 
 		slave_data->phy_if = of_get_phy_mode(slave_node);
+		if (of_find_property(slave_node, "ti,rmii-clock-ext", NULL))
+			slave_data->rmii_clock_external = true;
 
 		if (data->dual_emac) {
 			if (of_property_read_u32(slave_node, "dual_emac_res_vlan",
diff --git a/drivers/net/ethernet/ti/cpsw.h b/drivers/net/ethernet/ti/cpsw.h
index 96c374a..3baa2350 100644
--- a/drivers/net/ethernet/ti/cpsw.h
+++ b/drivers/net/ethernet/ti/cpsw.h
@@ -19,6 +19,7 @@
 struct cpsw_slave_data {
 	char		phy_id[MII_BUS_ID_SIZE];
 	int		phy_if;
+	bool		rmii_clock_external;
 	u8		mac_addr[ETH_ALEN];
 	u16		dual_emac_res_vlan;	/* Reserved VLAN for DualEMAC */
 };
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v3 0/5] cpsw: support for control module register
From: Daniel Mack @ 2013-08-23 12:08 UTC (permalink / raw)
  To: netdev
  Cc: bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
	mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree, Daniel Mack

v2 -> v3:
	* swap "ti,am3352-cpsw" and "ti,cpsw" to work around a matching
	  bug (reported by Sekhar)

v1 -> v2:
	* combine devm_request_mem_region() and devm_ioremap() and use
	  devm_ioremap_resource() (reported by Sergei Shtylyov)
	* fix multi-line comment style (reported by Sergei Shtylyov)
	* fix ti,rmii-clock-ext property name (reported by Sekhar)
	* rebased to net-next (reported by Mugunthan V N, David Miller)
	* add a new compatible type, and handle AM33xx specific
	  registers that way (reported by Sekhar)
	* move gmii_sel_reg modifications to the open routine
	  (reported by Mugunthan V N)


Daniel Mack (5):
  net: ethernet: cpsw: switch to devres allocations
  net: ethernet: cpsw: add optional third memory region for CONTROL
    module
  net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
  net: ethernet: cpsw: add support for hardware interface mode config
  ARM: dts: am33xx: adopt to cpsw changes

 Documentation/devicetree/bindings/net/cpsw.txt |  10 +-
 arch/arm/boot/dts/am33xx.dtsi                  |   5 +-
 drivers/net/ethernet/ti/cpsw.c                 | 254 ++++++++++++++-----------
 drivers/net/ethernet/ti/cpsw.h                 |   2 +
 4 files changed, 159 insertions(+), 112 deletions(-)

-- 
1.8.3.1

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox