* RE: About the Davicom PHY in drivers/net/phy in Linux kernel
From: Joseph Chang @ 2010-11-01 2:27 UTC (permalink / raw)
To: macpaul, netdev; +Cc: afleming, jeff, f.rodo
In-Reply-To: <50FD90C65C53FB45BADEEBCD84FF07F202A62479@ATCPCS06.andestech.com>
Dear MacPaul,
1.Yes. I have downloaded it. And below is the know items.
DM9161A
cpu: Faraday A320 (arm920t) + Andes AG101 (NDS32) ;SoC
OS: Linux: 2.6.32
Actions:
- davicom.c // Download from LXR
- include-linux-mii.h // Download from LXR
2.Your quote is right. Please tell us the test result.
3.I have a question for you,
Where is your company? I browse for andestech, And found that andestech located at
SiSoft SIPP Center! (Address: 2F, No.1, Li-Hsin First Road, Science-Based Industrial Park)
Is it right?
Best Regards,
Joseph CHANG
System Application Engineering Division
Davicom Semiconductor, Inc.
No. 6 Li-Hsin Rd. VI, Science-Based Park,
Hsin-Chu, Taiwan.
Tel: 886-3-5798797 Ex 8534
Fax: 886-3-5646929
Web: http://www.davicom.com.tw
-----Original Message-----
From: macpaul@andestech.com [mailto:macpaul@andestech.com]
Sent: Friday, October 29, 2010 3:47 PM
To: joseph_chang@mail.davicom.com.tw; netdev@vger.kernel.org
Cc: afleming@freescale.com; jeff@garzik.org; f.rodo@til-technologies.fr
Subject: RE: About the Davicom PHY in drivers/net/phy in Linux kernel
Hi Joseph,
I just followed up your suggestion in previous mail, let me quote them here:
# quote
The recommend is changed to be as below:
err = phy_write(phydev, MII_BMCR, BMCR_RESET);
//err = phy_write(phydev, MII_BMCR, BMCR_ISOLATE);
err = phy_write(phydev, MII_DM9161_SCR, MII_DM9161_SCR_INIT);
err = phy_write(phydev, MII_DM9161_10BTCSR, MII_DM9161_10BTCSR_INIT);
// err = phy_write(phydev, MII_NWAYTEST, 0x10);
err = phy_write(phydev, MII_BMCR, BMCR_ANENABLE | BMCR_ANRESTART);
*Note: Added a PHY reset command.
# end quote
>
> * == > Would you tell us your:
> CPU = ?
SoC: Faraday A320 (arm920t) / Andes AG101 (NDS32)
> Linux Kernel version= ?
Linux: 2.6.32
> I will like to download the same source code from LXR.
> And can check more detail for you.
I've believed that you have download the source already. :-)
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
^ permalink raw reply
* Re: [patch v3] fix stack overflow in pktgen_if_write()
From: Dan Carpenter @ 2010-11-01 3:47 UTC (permalink / raw)
To: Andi Kleen
Cc: Nelson Elhage, Eric Dumazet, David S. Miller, Robert Olsson,
Andy Shevchenko, netdev
In-Reply-To: <87lj5hud36.fsf@basil.nowhere.org>
> > @@ -887,12 +887,17 @@ static ssize_t pktgen_if_write(struct file *file,
> > i += len;
> >
> > if (debug) {
> > - char tb[count + 1];
> > + char *tb;
> > +
> > + tb = kmalloc(count + 1, GFP_KERNEL);
>
>
> This is still trivially exploitable (for root) -- think what happens
> when count is near ULONG_MAX
>
In vfs_write() we call rw_verify_area() which caps count at INT_MAX or
LONG_MAX.
if (unlikely((ssize_t) count < 0))
return retval;
So I get lucky this time... ;)
regards,
dan carpenter
^ permalink raw reply
* RE: NULL pointer dereference at netxen_nic_probe+0x813/0x9a0
From: Amit Salecha @ 2010-11-01 4:56 UTC (permalink / raw)
To: Denis Kirjanov, David Miller
Cc: bjorn.helgaas@hp.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <4CCC7868.60104@kernel.org>
> Ok, thanks Dave, now I see.
>
> [PATCH] netxen_nic: Fix the tx queue manipulation bug in netxen_nic_probe
>
> We should not stop the egress queue during probe because it is wrong.
>
>Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org>
> ---
> drivers/net/netxen/netxen_nic_main.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
Thanks Denis, for fixing this.
^ permalink raw reply
* RE: [PATCH] qlcnic: fix panic on load
From: Amit Salecha @ 2010-11-01 4:58 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: netdev
In-Reply-To: <1288540238.2660.50.camel@edumazet-laptop>
> From: Eric Dumazet [eric.dumazet@gmail.com]
> Sent: Sunday, October 31, 2010 9:20 PM
> To: David Miller
> Cc: netdev; Amit Salecha
> Subject: [PATCH] qlcnic: fix panic on load
>
> Its now illegal to call netif_stop_queue() before register_netdev()
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Amit Kumar Salecha <amit.salecha@qlogic.com>
> ---
> drivers/net/qlcnic/qlcnic_main.c | 1 -
> 1 file changed, 1 deletion(-)
Thanks Eric.
^ permalink raw reply
* Re: [PATCH] OF device tree: Move of_get_mac_address() to a common source file.
From: Grant Likely @ 2010-11-01 5:17 UTC (permalink / raw)
To: David Daney
Cc: linux-mips, Corey Minyard, Anton Vorontsov, Paul Mackerras,
Sadanand Mutyala, Sergey Matyukevich, Sean MacLennan,
Wolfgang Denk, microblaze-uclinux, devicetree-discuss,
Andres Salomon, Michal Simek, Anatolij Gustschin, Eric Dumazet,
Jiri Pirko, netdev, linux-kernel, ralf, Sandeep Gopalpet,
Vitaly Bordug, linuxppc-dev, David S. Miller
In-Reply-To: <1288130833-16421-1-git-send-email-ddaney@caviumnetworks.com>
On Tue, Oct 26, 2010 at 03:07:13PM -0700, David Daney wrote:
> There are two identical implementations of of_get_mac_address(), one
> each in arch/powerpc/kernel/prom_parse.c and
> arch/microblaze/kernel/prom_parse.c. Move this function to a new
> common file of_net.{c,h} and adjust all the callers to include the new
> header.
Applied, thanks; but made some changes to protect this code because it
does not work on little endian (it can be fixed in a separate patch)
Also...
>
> Signed-off-by: David Daney <ddaney@caviumnetworks.com>
> Cc: Michal Simek <monstr@monstr.eu>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Corey Minyard <cminyard@mvista.com>
> Cc: Pantelis Antoniou <pantelis.antoniou@gmail.com>
> Cc: Vitaly Bordug <vbordug@ru.mvista.com>
> Cc: Anatolij Gustschin <agust@denx.de>
> Cc: John Rigby <jcrigby@gmail.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Anton Vorontsov <avorontsov@mvista.com>
> Cc: Sandeep Gopalpet <Sandeep.Kumar@freescale.com>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> Cc: Li Yang <leoli@freescale.com>
> Cc: Sergey Matyukevich <geomatsi@gmail.com>
> Cc: Jiri Pirko <jpirko@redhat.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Sean MacLennan <smaclennan@pikatech.com>
> Cc: Sadanand Mutyala <Sadanand.Mutyala@xilinx.com>
> Cc: Andres Salomon <dilinger@queued.net>
> Cc: microblaze-uclinux@itee.uq.edu.au
> Cc: linux-kernel@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: netdev@vger.kernel.org
> Cc: devicetree-discuss@lists.ozlabs.org
You don't need to believe everything that get_maintainers is telling
you. When you get a large list like this, don't Cc everyone, but
apply some educated guesses. You can guess that the PowerPC and
Microblaze maintainers care because it touches their trees, and you
can guess that I care because I'm the dt maintainer. David Miller
isn't a bad guess because of networking. But 22 people is excessive.
g.
^ permalink raw reply
* Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Fix build warnings
From: Tomoya MORINAGA @ 2010-11-01 7:15 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Masayuki Ohtake,
Samuel Ortiz, margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CCAC4CD.7000503@pengutronix.de>
On Friday, October 29, 2010 9:57 PM : Marc Kleine-Budde wrote:
>>>> SW must check busy flag of CAN register.
>>>> This is a Topcliff HW specification.
>>> Maybe the busy check could also be done *before* the Message RAM is
>>> accessed to avoid (or minimize) waiting.
>> Yes, *before* is right.
>> If there is *after* processing, this is a bug.
>> Can you see anyway ?
>Sorry I don't understand what you mean.
Sorry, my English had mistake. I show my comment below again.
- If there is *after* processing, this is a bug.
- Can you see the point anywhere ?
> You probably know the datasheet, but I don't, although I've printed
> chapter 13 from the Intel Controller Hub EG20T datasheet, but it's 50+
> pages. If the hardware needs the busy waiting in the hot tx path a
> pointer to the respective section in the manual is a good idea. Just
> something like:
Though "Oliver Hartkopp" found the place of Datasheet EG20T and notified with the mailing-list,
Have you read the following ?
http://edc.intel.com/Platforms/Atom-E6xx/#hardware
>>> You have to change the definition of the regs struct a bit:
>>>> u32 if1_mcont;
>>>> u32 if1_data[4];
>>>> u32 reserve2;
>> Uh, I can't find this. Where is this ?
>Here's a patch to illustrate what I meant:
I understand.
Thanks, Tomoya(OKI SEMICONDUCTOR CO., LTD.)
^ permalink raw reply
* Re: [patch] netlink: use TID instead of PID for automatic id assignment
From: Jan Engelhardt @ 2010-11-01 8:27 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev
In-Reply-To: <20101101022548.GA27367@gondor.apana.org.au>
On Monday 2010-11-01 03:25, Herbert Xu wrote:
>>
>> netlink: use TID instead of PID for automatic id assignment
>>
>> This makes it easier to identify processes in the output of `ss -af
>> netlink` - as I see no reason to force negative space numbers upon
>> all but the first socket in a thread group.
>>
>> Turns out this reverts v2.6.15-rc4~65.
>>
>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
>
>Nack. Sockets are typically shared amongst threads so using the
>ID of the thread that created it doesn't make much sense when
>all the threads in that group use it to send/receive messages.
That would be fine, because they are in the same process.
>In any case, this field should not be relied on (please google
>the thread "netlink nlmsg_pid supposed to be pid or tid?") as
>anybody can claim your PID in the netlink name space.
I am aware of that. But then it raises the question why we are
giving the first socket a non-negative value in the first place.
^ permalink raw reply
* [PATCH RFC] tun: remove of user-controlled memory allocation
From: Michael S. Tsirkin @ 2010-11-01 8:27 UTC (permalink / raw)
Cc: David S. Miller, Michael S. Tsirkin, Herbert Xu, Eric Dumazet,
Joe Perches, netdev, linux-kernel
Untested, this is just an RFC.
tun does a kmalloc where userspace controls the length. This will
produce warnings in kernel log when the length is too large, or might
block for a long while. A simple fix is to avoid the allocatiuon
altogether, and copy from user in a loop.
However, with this patch an illegal address passed to the ioctl might
leave the filter disabled. Is this something we care about? If
yes we could recover by creating a copy of the filter. Thoughts?
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/tun.c | 30 ++++++++++++++----------------
1 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 55f3a3e..ea36888 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -220,28 +220,23 @@ static unsigned int addr_hash_test(const u32 *mask, const u8 *addr)
static int update_filter(struct tap_filter *filter, void __user *arg)
{
- struct { u8 u[ETH_ALEN]; } *addr;
+ struct { u8 u[ETH_ALEN]; } __user *addr;
struct tun_filter uf;
- int err, alen, n, nexact;
+ int err = -EFAULT, n, nexact;
if (copy_from_user(&uf, arg, sizeof(uf)))
- return -EFAULT;
+ goto done;
if (!uf.count) {
/* Disabled */
filter->count = 0;
- return 0;
+ err = 0;
+ goto done;
}
- alen = ETH_ALEN * uf.count;
- addr = kmalloc(alen, GFP_KERNEL);
- if (!addr)
- return -ENOMEM;
-
- if (copy_from_user(addr, arg + sizeof(uf), alen)) {
- err = -EFAULT;
+ addr = arg + sizeof(uf);
+ if (!access_ok(VERIFY_READ, addr, ETH_ALEN * uf.count))
goto done;
- }
/* The filter is updated without holding any locks. Which is
* perfectly safe. We disable it first and in the worst
@@ -251,7 +246,8 @@ static int update_filter(struct tap_filter *filter, void __user *arg)
/* Use first set of addresses as an exact filter */
for (n = 0; n < uf.count && n < FLT_EXACT_COUNT; n++)
- memcpy(filter->addr[n], addr[n].u, ETH_ALEN);
+ if (__copy_from_user(filter->addr[n], addr[n].u, ETH_ALEN))
+ goto done;
nexact = n;
@@ -259,11 +255,14 @@ static int update_filter(struct tap_filter *filter, void __user *arg)
* unicast will leave the filter disabled. */
memset(filter->mask, 0, sizeof(filter->mask));
for (; n < uf.count; n++) {
- if (!is_multicast_ether_addr(addr[n].u)) {
+ u8 u[ETH_ALEN];
+ if (__copy_from_user(u, addr[n].u, ETH_ALEN))
+ goto done;
+ if (!is_multicast_ether_addr(u)) {
err = 0; /* no filter */
goto done;
}
- addr_hash_set(filter->mask, addr[n].u);
+ addr_hash_set(filter->mask, u);
}
/* For ALLMULTI just set the mask to all ones.
@@ -279,7 +278,6 @@ static int update_filter(struct tap_filter *filter, void __user *arg)
err = nexact;
done:
- kfree(addr);
return err;
}
--
1.7.3.2.91.g446ac
^ permalink raw reply related
* Re: [RFC PATCH] macvlan: Introduce a PASSTHRU mode to takeover the underlying device
From: Michael S. Tsirkin @ 2010-11-01 8:28 UTC (permalink / raw)
To: Sridhar Samudrala; +Cc: kaber, Arnd Bergmann, netdev, kvm@vger.kernel.org
In-Reply-To: <1288131578.7582.49.camel@sridhar.beaverton.ibm.com>
On Tue, Oct 26, 2010 at 03:19:38PM -0700, Sridhar Samudrala wrote:
> With the current default macvtap mode, a KVM guest using virtio with
> macvtap backend has the following limitations.
> - cannot change/add a mac address on the guest virtio-net
> - cannot create a vlan device on the guest virtio-net
> - cannot enable promiscuous mode on guest virtio-net
>
> This patch introduces a new mode called 'passthru' when creating a
> macvlan device which allows takeover of the underlying device and
> passing it to a guest using virtio with macvtap backend.
>
> Only one macvlan device is allowed in passthru mode and it inherits
> the mac address from the underlying device and sets it in promiscuous
> mode to receive and forward all the packets.
>
> Thanks
> Sridhar
One concern with promisc mode is that for the common case,
which is a single mac and no vlans, we will be getting
extra packets that will get dropped in userspace/guest
as compared to the case where same mac is programmed
in hardware and by guest.
We could let userspace supply a list of mac/vlan addresses through
an ioctl on macvtap, and then
1. for a single mac, program it in hardware
2. for other configurations, set promisc mode
tun already has TUNSETTXFILTER which might come in handy here.
We don't pass in vlans with the filter now but maybe we should.
How does this sound?
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index 0ef0eb0..bca3cb7 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -38,6 +38,7 @@ struct macvlan_port {
> struct hlist_head vlan_hash[MACVLAN_HASH_SIZE];
> struct list_head vlans;
> struct rcu_head rcu;
> + bool passthru;
> };
>
> #define macvlan_port_get_rcu(dev) \
> @@ -169,6 +170,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
> macvlan_broadcast(skb, port, NULL,
> MACVLAN_MODE_PRIVATE |
> MACVLAN_MODE_VEPA |
> + MACVLAN_MODE_PASSTHRU|
> MACVLAN_MODE_BRIDGE);
> else if (src->mode == MACVLAN_MODE_VEPA)
> /* flood to everyone except source */
> @@ -185,7 +187,10 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
> return skb;
> }
>
> - vlan = macvlan_hash_lookup(port, eth->h_dest);
> + if (port->passthru)
> + vlan = list_first_entry(&port->vlans, struct macvlan_dev, list);
> + else
> + vlan = macvlan_hash_lookup(port, eth->h_dest);
> if (vlan == NULL)
> return skb;
>
> @@ -284,6 +289,11 @@ static int macvlan_open(struct net_device *dev)
> struct net_device *lowerdev = vlan->lowerdev;
> int err;
>
> + if (vlan->port->passthru) {
> + dev_set_promiscuity(lowerdev, 1);
> + goto hash_add;
> + }
> +
> err = -EBUSY;
> if (macvlan_addr_busy(vlan->port, dev->dev_addr))
> goto out;
> @@ -296,6 +306,8 @@ static int macvlan_open(struct net_device *dev)
> if (err < 0)
> goto del_unicast;
> }
> +
> +hash_add:
> macvlan_hash_add(vlan);
> return 0;
>
> @@ -310,12 +322,18 @@ static int macvlan_stop(struct net_device *dev)
> struct macvlan_dev *vlan = netdev_priv(dev);
> struct net_device *lowerdev = vlan->lowerdev;
>
> + if (vlan->port->passthru) {
> + dev_set_promiscuity(lowerdev, -1);
> + goto hash_del;
> + }
> +
> dev_mc_unsync(lowerdev, dev);
> if (dev->flags & IFF_ALLMULTI)
> dev_set_allmulti(lowerdev, -1);
>
> dev_uc_del(lowerdev, dev->dev_addr);
>
> +hash_del:
> macvlan_hash_del(vlan);
> return 0;
> }
> @@ -549,6 +567,7 @@ static int macvlan_port_create(struct net_device *dev)
> if (port == NULL)
> return -ENOMEM;
>
> + port->passthru = false;
> port->dev = dev;
> INIT_LIST_HEAD(&port->vlans);
> for (i = 0; i < MACVLAN_HASH_SIZE; i++)
> @@ -593,6 +612,7 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
> case MACVLAN_MODE_PRIVATE:
> case MACVLAN_MODE_VEPA:
> case MACVLAN_MODE_BRIDGE:
> + case MACVLAN_MODE_PASSTHRU:
> break;
> default:
> return -EINVAL;
> @@ -661,6 +681,10 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
> }
> port = macvlan_port_get(lowerdev);
>
> + /* Only 1 macvlan device can be created in passthru mode */
> + if (port->passthru)
> + return -EINVAL;
> +
> vlan->lowerdev = lowerdev;
> vlan->dev = dev;
> vlan->port = port;
> @@ -671,6 +695,13 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
> if (data && data[IFLA_MACVLAN_MODE])
> vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
>
> + if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
> + if (!list_empty(&port->vlans))
> + return -EINVAL;
> + port->passthru = true;
> + memcpy(dev->dev_addr, lowerdev->dev_addr, ETH_ALEN);
> + }
> +
> err = register_netdevice(dev);
> if (err < 0)
> goto destroy_port;
> diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> index 2fc66dd..8454805 100644
> --- a/include/linux/if_link.h
> +++ b/include/linux/if_link.h
> @@ -232,6 +232,7 @@ enum macvlan_mode {
> MACVLAN_MODE_PRIVATE = 1, /* don't talk to other macvlans */
> MACVLAN_MODE_VEPA = 2, /* talk to other ports through ext bridge */
> MACVLAN_MODE_BRIDGE = 4, /* talk to bridge ports directly */
> + MACVLAN_MODE_PASSTHRU = 8,/* take over the underlying device */
> };
>
> /* SR-IOV virtual function management section */
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] net: packet: fix information leak to userland
From: walter harms @ 2010-11-01 9:14 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: kernel-janitors, David S. Miller, Jiri Pirko, Eric Dumazet,
netdev, linux-kernel
In-Reply-To: <1288545028-16436-1-git-send-email-segooon@gmail.com>
Vasiliy Kulikov schrieb:
> packet_getname_spkt() doesn't initialize all members of sa_data field of
> sockaddr struct if strlen(dev->name) < 13. This structure is then copied
> to userland. It leads to leaking of contents of kernel stack memory.
> We have to fully fill sa_data with strncpy() instead of strlcpy().
>
> The same with packet_getname(): it doesn't initialize sll_pkttype field of
> sockaddr_ll. Set it to zero.
>
> Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
> ---
> net/packet/af_packet.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 3616f27..0856a13 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1719,7 +1719,7 @@ static int packet_getname_spkt(struct socket *sock, struct sockaddr *uaddr,
> rcu_read_lock();
> dev = dev_get_by_index_rcu(sock_net(sk), pkt_sk(sk)->ifindex);
> if (dev)
> - strlcpy(uaddr->sa_data, dev->name, 15);
> + strncpy(uaddr->sa_data, dev->name, 14);
> else
> memset(uaddr->sa_data, 0, 14);
if i understand the code correcly the max size for dev->name is IFNAMSIZ.
You can simply that part:
memset(uaddr->sa_data, 0, IFNAMSIZ);
dev = dev_get_by_index_rcu(sock_net(sk), pkt_sk(sk)->ifindex);
if (dev)
strlcpy(uaddr->sa_data, dev->name, IFNAMSIZ);
you should send that as separate patch.
re,
wh
> rcu_read_unlock();
> @@ -1742,6 +1742,7 @@ static int packet_getname(struct socket *sock, struct sockaddr *uaddr,
> sll->sll_family = AF_PACKET;
> sll->sll_ifindex = po->ifindex;
> sll->sll_protocol = po->num;
> + sll->sll_pkttype = 0;
> rcu_read_lock();
> dev = dev_get_by_index_rcu(sock_net(sk), po->ifindex);
> if (dev) {
^ permalink raw reply
* Attention Please!!
From: Jozef Ruud Roosevelt @ 2010-11-01 10:05 UTC (permalink / raw)
Attention Please,
I am Jozef Ruud Roosevelt, a Dutch National presently residing in
Newcastle United Kingdom, I have a proposition Involving an investment
initiative in your country economy to discuss with you, It will be of
mutual benefit to both of us, and I believe we can handle it together,
once we have a common understanding and mutual cooperation in the
execution of the modalities. I work with Abn Amro Bank as an auditor.
Should you be interested, please e-mail back to me through this email
address: (jozef-rosevelt@live.com) or you can give me a call (+44 702
4056 175).
I await your earliest response.
Yours Sincerely,
Jozef Ruud Roosevel.
----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.
^ permalink raw reply
* Thanks for responding
From: Srood Sherif @ 2010-11-01 10:34 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 361 bytes --]
Greeting,
My name is Mr. Srood A. Sherif, I live and work in Abu
Dhabi UAE. I have an urgent business which I believe
will interest you. Find the enclose for details.
For any reason you cannot open that attachment, please
let me know so that I can resend it in the body of the
mail, thank you.
I wait for your response, Thank you.
Regards
Srood A. Sherif
[-- Attachment #2: THE INFORMATION.pdf --]
[-- Type: application/pdf, Size: 48510 bytes --]
^ permalink raw reply
* Re: [PATCH] jme: fix panic on load
From: Guo-Fu Tseng @ 2010-11-01 11:22 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev
In-Reply-To: <20101031.092636.193730282.davem@davemloft.net>
On Sun, 31 Oct 2010 09:26:36 -0700 (PDT), David Miller wrote
> From: "Guo-Fu Tseng" <cooldavid@cooldavid.org>
> Date: Mon, 1 Nov 2010 00:39:14 +0800
>
> > On Sun, 31 Oct 2010 16:46:18 +0100, Eric Dumazet wrote
> > Can this patch be modified to move the netif_stop_queue()
> > after register_netdev() ?
> > It seems the __QUEUE_STATE_XOFF is not set after the register_netdev.
> > The tx_queue was kcalloc() ed without touching state flags.
>
> At the veyr moment that register_netdev() is called, the
> ->open() method of your driver can be invoked and the
> queue state changed.
>
> So no, you can't put it after the register call.
>
> There is zero reason to touch the queue state in your
> probe routine, the state of the queue before ->open()
> is "don't care".
>
> Eric's patch is going to be applied, it is correct.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
I see.
Thanks for the info. :)
Guo-Fu Tseng
^ permalink raw reply
* Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Fix build warnings
From: Marc Kleine-Budde @ 2010-11-01 11:05 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Tomoya, David S. Miller, Wolfram Sang, Christian Pellegrin,
Barry Song, Samuel Ortiz, socketcan-core, netdev, linux-kernel,
andrew.chih.howe.khor, qi.wang, margie.foster, yong.y.wang,
Masayuki Ohtake, kok.howg.ewe, joel.clark
In-Reply-To: <4CCB213A.2020206@grandegger.com>
[-- Attachment #1: Type: text/plain, Size: 56064 bytes --]
Hello,
On 10/29/2010 09:32 PM, Wolfgang Grandegger wrote:
some comments inline.
[...]
>>> --- /dev/null
>>> +++ b/drivers/net/can/pch_can.c
>>> @@ -0,0 +1,1436 @@
>>> +/*
>>> + * Copyright (C) 1999 - 2010 Intel Corporation.
>>> + * Copyright (C) 2010 OKI SEMICONDUCTOR CO., LTD.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; version 2 of the License.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program; if not, write to the Free Software
>>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307, USA.
>>> + */
>>> +
>>> +#include <linux/interrupt.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/io.h>
>>> +#include <linux/module.h>
>>> +#include <linux/sched.h>
>>> +#include <linux/pci.h>
>>> +#include <linux/init.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/types.h>
>>> +#include <linux/errno.h>
>>> +#include <linux/netdevice.h>
>>> +#include <linux/skbuff.h>
>>> +#include <linux/can.h>
>>> +#include <linux/can/dev.h>
>>> +#include <linux/can/error.h>
>>> +
>>> +#define MAX_MSG_OBJ 32
>>> +#define MSG_OBJ_RX 0 /* The receive message object flag. */
>>> +#define MSG_OBJ_TX 1 /* The transmit message object flag. */
>>> +
>>> +#define CAN_CTRL_INIT 0x0001 /* The INIT bit of CANCONT register. */
>>> +#define CAN_CTRL_IE 0x0002 /* The IE bit of CAN control register */
>>> +#define CAN_CTRL_IE_SIE_EIE 0x000e
>>> +#define CAN_CTRL_CCE 0x0040
>>> +#define CAN_CTRL_OPT 0x0080 /* The OPT bit of CANCONT register. */
>>> +#define CAN_OPT_SILENT 0x0008 /* The Silent bit of CANOPT reg. */
>>> +#define CAN_OPT_LBACK 0x0010 /* The LoopBack bit of CANOPT reg. */
>>> +#define CAN_CMASK_RX_TX_SET 0x00f3
>>> +#define CAN_CMASK_RX_TX_GET 0x0073
>>> +#define CAN_CMASK_ALL 0xff
>>> +#define CAN_CMASK_RDWR 0x80
>>> +#define CAN_CMASK_ARB 0x20
>>> +#define CAN_CMASK_CTRL 0x10
>>> +#define CAN_CMASK_MASK 0x40
>>> +#define CAN_CMASK_NEWDAT 0x04
>>> +#define CAN_CMASK_CLRINTPND 0x08
>>> +
>>> +#define CAN_IF_MCONT_NEWDAT 0x8000
>>> +#define CAN_IF_MCONT_INTPND 0x2000
>>> +#define CAN_IF_MCONT_UMASK 0x1000
>>> +#define CAN_IF_MCONT_TXIE 0x0800
>>> +#define CAN_IF_MCONT_RXIE 0x0400
>>> +#define CAN_IF_MCONT_RMTEN 0x0200
>>> +#define CAN_IF_MCONT_TXRQXT 0x0100
>>> +#define CAN_IF_MCONT_EOB 0x0080
>>> +#define CAN_IF_MCONT_DLC 0x000f
>>> +#define CAN_IF_MCONT_MSGLOST 0x4000
>>> +#define CAN_MASK2_MDIR_MXTD 0xc000
>>> +#define CAN_ID2_DIR 0x2000
>>> +#define CAN_ID_MSGVAL 0x8000
>>> +
>>> +#define CAN_STATUS_INT 0x8000
>>> +#define CAN_IF_CREQ_BUSY 0x8000
>>> +#define CAN_ID2_XTD 0x4000
>>> +
>>> +#define CAN_REC 0x00007f00
>>> +#define CAN_TEC 0x000000ff
>>
>> A prefix for like PCH_ instead of CAN_ for all those define above would
>> be fine to avoid namespace clashes and/or confusion with the defines from the socketcan framework.
>>
>>> +
>>> +#define PCH_RX_OK 0x00000010
>>> +#define PCH_TX_OK 0x00000008
>>> +#define PCH_BUS_OFF 0x00000080
>>> +#define PCH_EWARN 0x00000040
>>> +#define PCH_EPASSIV 0x00000020
>>> +#define PCH_LEC0 0x00000001
>>> +#define PCH_LEC1 0x00000002
>>> +#define PCH_LEC2 0x00000004
>>
>> These are just single set bit, please use BIT()
>> Consider adding the name of the corresponding register to the define's
>> name.
>>
>>> +#define PCH_LEC_ALL (PCH_LEC0 | PCH_LEC1 | PCH_LEC2)
>>> +#define PCH_STUF_ERR PCH_LEC0
>>> +#define PCH_FORM_ERR PCH_LEC1
>>> +#define PCH_ACK_ERR (PCH_LEC0 | PCH_LEC1)
>>> +#define PCH_BIT1_ERR PCH_LEC2
>>> +#define PCH_BIT0_ERR (PCH_LEC0 | PCH_LEC2)
>>> +#define PCH_CRC_ERR (PCH_LEC1 | PCH_LEC2)
>
> This is an enumeration:
>
> enum {
> PCH_STUF_ERR = 1,
> PCH_FORM_ERR,
> PCH_ACK_ERR,
> PCH_BIT1_ERR;
> PCH_BIT0_ERR,
> PCH_CRC_ERR,
> PCH_LEC_ALL;
^
","
> }
^
";"
>
> Also, s/PCH_/PCH_LEC_/ would be nice.
ACK
>
>>> +/* bit position of certain controller bits. */
>>> +#define BIT_BITT_BRP 0
>>> +#define BIT_BITT_SJW 6
>>> +#define BIT_BITT_TSEG1 8
>>> +#define BIT_BITT_TSEG2 12
>>> +#define BIT_IF1_MCONT_RXIE 10
>>> +#define BIT_IF2_MCONT_TXIE 11
>>> +#define BIT_BRPE_BRPE 6
>>> +#define BIT_ES_TXERRCNT 0
>>> +#define BIT_ES_RXERRCNT 8
>>
>> these are usually called SHIFT
>>
>>> +#define MSK_BITT_BRP 0x3f
>>> +#define MSK_BITT_SJW 0xc0
>>> +#define MSK_BITT_TSEG1 0xf00
>>> +#define MSK_BITT_TSEG2 0x7000
>>> +#define MSK_BRPE_BRPE 0x3c0
>>> +#define MSK_BRPE_GET 0x0f
>>> +#define MSK_CTRL_IE_SIE_EIE 0x07
>>> +#define MSK_MCONT_TXIE 0x08
>>> +#define MSK_MCONT_RXIE 0x10
>>
>> MSK or MASK is okay, however the last two are just single bits.
>>
>> Please add a PCH_ prefix here, too.
>>
>>> +#define PCH_CAN_NO_TX_BUFF 1
>>> +#define COUNTER_LIMIT 10
>>
>> dito
>>
>>> +
>>> +#define PCH_CAN_CLK 50000000 /* 50MHz */
>>> +
>>> +/*
>>> + * Define the number of message object.
>>> + * PCH CAN communications are done via Message RAM.
>>> + * The Message RAM consists of 32 message objects.
>>> + */
>>> +#define PCH_RX_OBJ_NUM 26 /* 1~ PCH_RX_OBJ_NUM is Rx*/
>>> +#define PCH_TX_OBJ_NUM 6 /* PCH_RX_OBJ_NUM is RX ~ Tx*/
>>> +#define PCH_OBJ_NUM (PCH_TX_OBJ_NUM + PCH_RX_OBJ_NUM)
>>
>> You define MAX_MSG_OBJ earlier, seems like two names for the same value.
>>
>>> +
>>> +#define PCH_FIFO_THRESH 16
>>> +
>>> +enum pch_can_mode {
>>> + PCH_CAN_ENABLE,
>>> + PCH_CAN_DISABLE,
>>> + PCH_CAN_ALL,
>>> + PCH_CAN_NONE,
>>> + PCH_CAN_STOP,
>>> + PCH_CAN_RUN,
>>> +};
>>> +
>>> +struct pch_can_regs {
>>> + u32 cont;
>>> + u32 stat;
>>> + u32 errc;
>>> + u32 bitt;
>>> + u32 intr;
>>> + u32 opt;
>>> + u32 brpe;
>>> + u32 reserve1;
>>
>> VVVV
>>> + u32 if1_creq;
>>> + u32 if1_cmask;
>>> + u32 if1_mask1;
>>> + u32 if1_mask2;
>>> + u32 if1_id1;
>>> + u32 if1_id2;
>>> + u32 if1_mcont;
>>> + u32 if1_dataa1;
>>> + u32 if1_dataa2;
>>> + u32 if1_datab1;
>>> + u32 if1_datab2;
>> ^^^^
>>
>> these registers and....
>>
>>> + u32 reserve2;
>>> + u32 reserve3[12];
>>
>> ...and these
>>
>> VVVV
>>> + u32 if2_creq;
>>> + u32 if2_cmask;
>>> + u32 if2_mask1;
>>> + u32 if2_mask2;
>>> + u32 if2_id1;
>>> + u32 if2_id2;
>>> + u32 if2_mcont;
>>> + u32 if2_dataa1;
>>> + u32 if2_dataa2;
>>> + u32 if2_datab1;
>>> + u32 if2_datab2;
>>
>> ^^^^
>>
>> ...are identical. I suggest to make a struct defining a complete
>> "Message Interface Register Set". If you include the correct number of
>> reserved bytes in the struct, you can have an array of two of these
>> structs in the struct pch_can_regs.
>
> Yep, that would be nice. Using it consequently would also allow to
> remove duplicated code efficiently. I will name it "struct pch_can_if"
> for latter references.
>
>>
>>> + u32 reserve4;
>>> + u32 reserve5[20];
>>> + u32 treq1;
>>> + u32 treq2;
>>> + u32 reserve6[2];
>>> + u32 reserve7[56];
>>> + u32 reserve8[3];
>
> Why not just one reserveX ?
>
>>> + u32 srst;
>>> +};
>>> +
>>> +struct pch_can_priv {
>>> + struct can_priv can;
>>> + struct pci_dev *dev;
>>> + unsigned int tx_enable[MAX_MSG_OBJ];
>>> + unsigned int rx_enable[MAX_MSG_OBJ];
>>> + unsigned int rx_link[MAX_MSG_OBJ];
>>> + unsigned int int_enables;
>>> + unsigned int int_stat;
>>> + struct net_device *ndev;
>>> + spinlock_t msgif_reg_lock; /* Message Interface Registers Access Lock*/
>> ^^^
>> please add a whitespace
>>
>>> + unsigned int msg_obj[MAX_MSG_OBJ];
>>> + struct pch_can_regs __iomem *regs;
>>> + struct napi_struct napi;
>>> + unsigned int tx_obj; /* Point next Tx Obj index */
>>> + unsigned int use_msi;
>>> +};
>>> +
>>> +static struct can_bittiming_const pch_can_bittiming_const = {
>>> + .name = "pch_can",
>>> + .tseg1_min = 1,
>>> + .tseg1_max = 16,
>>> + .tseg2_min = 1,
>>> + .tseg2_max = 8,
>>> + .sjw_max = 4,
>>> + .brp_min = 1,
>>> + .brp_max = 1024, /* 6bit + extended 4bit */
>>> + .brp_inc = 1,
>>> +};
>>> +
>>> +static DEFINE_PCI_DEVICE_TABLE(pch_pci_tbl) = {
>>> + {PCI_VENDOR_ID_INTEL, 0x8818, PCI_ANY_ID, PCI_ANY_ID,},
>>> + {0,}
>>> +};
>>> +MODULE_DEVICE_TABLE(pci, pch_pci_tbl);
>>> +
>>> +static inline void pch_can_bit_set(u32 *addr, u32 mask)
>> ^^^^^
>>
>> that should be an void __iomem *, see mail I've send the other day.
>> Please use sparse to check for this kinds of errors.
>> (Compile the driver with C=2, i.e.: make drivers/net/can/pch_can.ko C=2)
>>
>>> +{
>>> + iowrite32(ioread32(addr) | mask, addr);
>>> +}
>>> +
>>> +static inline void pch_can_bit_clear(u32 *addr, u32 mask)
>> ^^^^^
>>
>> dito
>>
>>> +{
>>> + iowrite32(ioread32(addr) & ~mask, addr);
>>> +}
>>> +
>>> +static void pch_can_set_run_mode(struct pch_can_priv *priv,
>>> + enum pch_can_mode mode)
>>> +{
>>> + switch (mode) {
>>> + case PCH_CAN_RUN:
>>> + pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_INIT);
>>> + break;
>>> +
>>> + case PCH_CAN_STOP:
>>> + pch_can_bit_set(&priv->regs->cont, CAN_CTRL_INIT);
>>> + break;
>>> +
>>> + default:
>>> + dev_err(&priv->ndev->dev, "%s -> Invalid Mode.\n", __func__);
>>> + break;
>>> + }
>>> +}
>>> +
>>> +static void pch_can_set_optmode(struct pch_can_priv *priv)
>>> +{
>>> + u32 reg_val = ioread32(&priv->regs->opt);
>>> +
>>> + if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
>>> + reg_val |= CAN_OPT_SILENT;
>>> +
>>> + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
>>> + reg_val |= CAN_OPT_LBACK;
>>> +
>>> + pch_can_bit_set(&priv->regs->cont, CAN_CTRL_OPT);
>>> + iowrite32(reg_val, &priv->regs->opt);
>>> +}
>>> +
>>
>> IMHO the function name is missleading, if I understand the code
>> correctly, this functions triggers the transmission of the message.
>> After this it checks for busy, but
>>
>>> +static void pch_can_check_if_busy(u32 __iomem *creq_addr, u32 num)
>> ^^^^
>>
>> that should probaby be a void
>
> With separate structs for if1 and i2, a pointer to the relevant "struct
> pch_can_if" could be passed instead.
>
>>> +{
>>> + u32 counter = COUNTER_LIMIT;
>>> + u32 ifx_creq;
>>> +
>>> + iowrite32(num, creq_addr);
>>> + while (counter) {
>>> + ifx_creq = ioread32(creq_addr) & CAN_IF_CREQ_BUSY;
>>> + if (!ifx_creq)
>>> + break;
>>> + counter--;
>>> + udelay(1);
>>> + }
>>> + if (!counter)
>>> + pr_err("%s:IF1 BUSY Flag is set forever.\n", __func__);
>>> +}
>>> +
>>> +static void pch_can_set_int_enables(struct pch_can_priv *priv,
>>> + enum pch_can_mode interrupt_no)
>>> +{
>>> + switch (interrupt_no) {
>>> + case PCH_CAN_ENABLE:
>>> + pch_can_bit_set(&priv->regs->cont, CAN_CTRL_IE);
>>
>> noone uses this case.
>>
>>> + break;
>>> +
>>> + case PCH_CAN_DISABLE:
>>> + pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_IE);
>>> + break;
>>> +
>>> + case PCH_CAN_ALL:
>>> + pch_can_bit_set(&priv->regs->cont, CAN_CTRL_IE_SIE_EIE);
>>> + break;
>>> +
>>> + case PCH_CAN_NONE:
>>> + pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_IE_SIE_EIE);
>>> + break;
>>> +
>>> + default:
>>> + dev_err(&priv->ndev->dev, "Invalid interrupt number.\n");
>>> + break;
>>> + }
>>> +}
>>> +
>>> +static void pch_can_set_rx_enable(struct pch_can_priv *priv, u32 buff_num,
>>> + int set)
>>> +{
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>>> + /* Reading the receive buffer data from RAM to Interface1 registers */
>>> + iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, buff_num);
>>> +
>>> + /* Setting the IF1MASK1 register to access MsgVal and RxIE bits */
>>> + iowrite32(CAN_CMASK_RDWR | CAN_CMASK_ARB | CAN_CMASK_CTRL,
>>> + &priv->regs->if1_cmask);
>>> +
>>> + if (set == 1) {
>>> + /* Setting the MsgVal and RxIE bits */
>>> + pch_can_bit_set(&priv->regs->if1_mcont, CAN_IF_MCONT_RXIE);
>>> + pch_can_bit_set(&priv->regs->if1_id2, CAN_ID_MSGVAL);
>>> +
>>> + } else if (set == 0) {
>>> + /* Resetting the MsgVal and RxIE bits */
>>> + pch_can_bit_clear(&priv->regs->if1_mcont, CAN_IF_MCONT_RXIE);
>>> + pch_can_bit_clear(&priv->regs->if1_id2, CAN_ID_MSGVAL);
>>> + }
>
> Why not just?
>
> if (set)
> else
>
>
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, buff_num);
>>> + spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
>>> +}
>>> +
>>> +static void pch_can_rx_enable_all(struct pch_can_priv *priv)
>>> +{
>>> + int i;
>>> +
>>> + /* Traversing to obtain the object configured as receivers. */
>>> + for (i = 1; i <= PCH_RX_OBJ_NUM; i++)
>>> + pch_can_set_rx_enable(priv, i, 1);
>>> +}
>>> +
>>> +static void pch_can_rx_disable_all(struct pch_can_priv *priv)
>>> +{
>>> + int i;
>>> +
>>> + /* Traversing to obtain the object configured as receivers. */
>>> + for (i = 1; i <= PCH_RX_OBJ_NUM; i++)
>>> + pch_can_set_rx_enable(priv, i, 0);
>>> +}
>>> +
>>> +static void pch_can_set_tx_enable(struct pch_can_priv *priv, u32 buff_num,
>>> + u32 set)
>>> +{
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>>> + /* Reading the Msg buffer from Message RAM to Interface2 registers. */
>>> + iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if2_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if2_creq, buff_num);
>>> +
>>> + /* Setting the IF2CMASK register for accessing the
>>> + MsgVal and TxIE bits */
>>> + iowrite32(CAN_CMASK_RDWR | CAN_CMASK_ARB | CAN_CMASK_CTRL,
>>> + &priv->regs->if2_cmask);
>>> +
>>> + if (set == 1) {
>>> + /* Setting the MsgVal and TxIE bits */
>>> + pch_can_bit_set(&priv->regs->if2_mcont, CAN_IF_MCONT_TXIE);
>>> + pch_can_bit_set(&priv->regs->if2_id2, CAN_ID_MSGVAL);
>>> + } else if (set == 0) {
>>> + /* Resetting the MsgVal and TxIE bits. */
>>> + pch_can_bit_clear(&priv->regs->if2_mcont, CAN_IF_MCONT_TXIE);
>>> + pch_can_bit_clear(&priv->regs->if2_id2, CAN_ID_MSGVAL);
>>> + }
>>> +
>>> + pch_can_check_if_busy(&priv->regs->if2_creq, buff_num);
>>> + spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
>>> +}
>
> That function is almost identical to pch_can_set_rx_enable. Just if2 is
> used instead of if1 and CAN_IF_MCONT_TXIE instead of CAN_IF_MCONT_RXIE.
> With separate "struct pch_can_if" for if1 and if2, it could be handled
> by a common function.
>
>>> +static void pch_can_tx_enable_all(struct pch_can_priv *priv)
>>> +{
>>> + int i;
>>> +
>>> + /* Traversing to obtain the object configured as transmit object. */
>>> + for (i = PCH_RX_OBJ_NUM + 1; i <= PCH_OBJ_NUM; i++)
>>> + pch_can_set_tx_enable(priv, i, 1);
>>> +}
>>> +
>>> +static void pch_can_tx_disable_all(struct pch_can_priv *priv)
>>> +{
>>> + int i;
>>> +
>>> + /* Traversing to obtain the object configured as transmit object. */
>>> + for (i = PCH_RX_OBJ_NUM + 1; i <= PCH_OBJ_NUM; i++)
>>> + pch_can_set_tx_enable(priv, i, 0);
>>> +}
>
> I think there is no need for separate functions for enable and disable.
> Just pass "enable" 0 or 1 like you do with "set" above.
>
>>> +static int pch_can_int_pending(struct pch_can_priv *priv)
>> ^^^
>>
>> make it u32 as it returns a register value, or a u16 as you only use
>> the 16 lower bits.
>>
>>> +{
>>> + return ioread32(&priv->regs->intr) & 0xffff;
>>> +}
>>> +
>>> +static void pch_can_clear_buffers(struct pch_can_priv *priv)
>>> +{
>>> + int i; /* Msg Obj ID (1~32) */
>>> +
>>> + for (i = 1; i <= PCH_RX_OBJ_NUM; i++) {
>>
>> IMHO the readability would be improved if you define something like
>> PCH_RX_OBJ_START and PCH_RX_OBJ_END.
>>
>>> + iowrite32(CAN_CMASK_RX_TX_SET, &priv->regs->if1_cmask);
>>> + iowrite32(0xffff, &priv->regs->if1_mask1);
>>> + iowrite32(0xffff, &priv->regs->if1_mask2);
>>> + iowrite32(0x0, &priv->regs->if1_id1);
>>> + iowrite32(0x0, &priv->regs->if1_id2);
>>> + iowrite32(0x0, &priv->regs->if1_mcont);
>>> + iowrite32(0x0, &priv->regs->if1_dataa1);
>>> + iowrite32(0x0, &priv->regs->if1_dataa2);
>>> + iowrite32(0x0, &priv->regs->if1_datab1);
>>> + iowrite32(0x0, &priv->regs->if1_datab2);
>>> + iowrite32(CAN_CMASK_RDWR | CAN_CMASK_MASK |
>>> + CAN_CMASK_ARB | CAN_CMASK_CTRL,
>>> + &priv->regs->if1_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, i);
>>> + }
>>> +
>>> + for (i = PCH_RX_OBJ_NUM + 1; i <= PCH_OBJ_NUM; i++) {
>> ^^^^^^^^^^^^^^^^^^
>> dito for TX objects
>>
>>> + iowrite32(CAN_CMASK_RX_TX_SET, &priv->regs->if2_cmask);
>>> + iowrite32(0xffff, &priv->regs->if2_mask1);
>>> + iowrite32(0xffff, &priv->regs->if2_mask2);
>>> + iowrite32(0x0, &priv->regs->if2_id1);
>>> + iowrite32(0x0, &priv->regs->if2_id2);
>>> + iowrite32(0x0, &priv->regs->if2_mcont);
>>> + iowrite32(0x0, &priv->regs->if2_dataa1);
>>> + iowrite32(0x0, &priv->regs->if2_dataa2);
>>> + iowrite32(0x0, &priv->regs->if2_datab1);
>>> + iowrite32(0x0, &priv->regs->if2_datab2);
>>> + iowrite32(CAN_CMASK_RDWR | CAN_CMASK_MASK | CAN_CMASK_ARB |
>>> + CAN_CMASK_CTRL, &priv->regs->if2_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if2_creq, i);
>
> This is almost the same code as above, just if2 instead of if1. With
> separate "struct pch_can_if" for if1 and i2, it could be handled by a
> common function.
>
>>> + }
>>> +}
>>> +
>>> +static void pch_can_config_rx_tx_buffers(struct pch_can_priv *priv)
>>> +{
>>> + int i;
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>>> +
>>> + for (i = 1; i <= PCH_RX_OBJ_NUM; i++) {
>>> + iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, i);
>>
>> If I understand the code correctly, the about function triggers a
>> transfer. Why do you first trigger a transfer, then set the message contents....
>>> +
>>> + iowrite32(0x0, &priv->regs->if1_id1);
>>> + iowrite32(0x0, &priv->regs->if1_id2);
>>> +
>>> + pch_can_bit_set(&priv->regs->if1_mcont, CAN_IF_MCONT_UMASK);
>>
>> Why do you set the "Use acceptance mask" bit? We want to receive
>> all can messages.
>>
>>> +
>>> + /* Set FIFO mode set to 0 except last Rx Obj*/
>>> + pch_can_bit_clear(&priv->regs->if1_mcont, CAN_IF_MCONT_EOB);
>>> + /* In case FIFO mode, Last EoB of Rx Obj must be 1 */
>>> + if (i == (PCH_RX_OBJ_NUM - 1))
>>> + pch_can_bit_set(&priv->regs->if1_mcont,
>>> + CAN_IF_MCONT_EOB);
>>
>> Make it if () { } else { }, please.
>>
>>> +
>>> + iowrite32(0, &priv->regs->if1_mask1);
>>> + pch_can_bit_clear(&priv->regs->if1_mask2,
>>> + 0x1fff | CAN_MASK2_MDIR_MXTD);
>>> +
>>> + /* Setting CMASK for writing */
>>> + iowrite32(CAN_CMASK_RDWR | CAN_CMASK_MASK | CAN_CMASK_ARB |
>>> + CAN_CMASK_CTRL, &priv->regs->if1_cmask);
>>> +
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, i);
>>
>> ...and then trigger the transfer again?
>>
>>> + }
>>> +
>>> + for (i = PCH_RX_OBJ_NUM + 1; i <= PCH_OBJ_NUM; i++) {
>>> + iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if2_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if2_creq, i);
>>
>> same question about triggering the transfer 2 times applied here, too
>>> +
>>> + /* Resetting DIR bit for reception */
>>> + iowrite32(0x0, &priv->regs->if2_id1);
>>> + iowrite32(0x0, &priv->regs->if2_id2);
>>> + pch_can_bit_set(&priv->regs->if2_id2, CAN_ID2_DIR);
>>
>> Can you combine the two accesses to >if2_id2 into one?
>>
>>> +
>>> + /* Setting EOB bit for transmitter */
>>> + iowrite32(CAN_IF_MCONT_EOB, &priv->regs->if2_mcont);
>>> +
>>> + pch_can_bit_set(&priv->regs->if2_mcont,
>>> + CAN_IF_MCONT_UMASK);
>>
>> dito for if2_mcont
>>
>>> +
>>> + iowrite32(0, &priv->regs->if2_mask1);
>>> + pch_can_bit_clear(&priv->regs->if2_mask2, 0x1fff);
>>> +
>>> + /* Setting CMASK for writing */
>>> + iowrite32(CAN_CMASK_RDWR | CAN_CMASK_MASK | CAN_CMASK_ARB |
>>> + CAN_CMASK_CTRL, &priv->regs->if2_cmask);
>>> +
>>> + pch_can_check_if_busy(&priv->regs->if2_creq, i);
>>> + }
>>> +
>>> + spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
>>> +}
>>> +
>>> +static void pch_can_init(struct pch_can_priv *priv)
>>> +{
>>> + /* Stopping the Can device. */
>>> + pch_can_set_run_mode(priv, PCH_CAN_STOP);
>>> +
>>> + /* Clearing all the message object buffers. */
>>> + pch_can_clear_buffers(priv);
>>> +
>>> + /* Configuring the respective message object as either rx/tx object. */
>>> + pch_can_config_rx_tx_buffers(priv);
>>> +
>>> + /* Enabling the interrupts. */
>>> + pch_can_set_int_enables(priv, PCH_CAN_ALL);
>>> +}
>>> +
>>> +static void pch_can_release(struct pch_can_priv *priv)
>>> +{
>>> + /* Stooping the CAN device. */
>>> + pch_can_set_run_mode(priv, PCH_CAN_STOP);
>>> +
>>> + /* Disabling the interrupts. */
>>> + pch_can_set_int_enables(priv, PCH_CAN_NONE);
>>> +
>>> + /* Disabling all the receive object. */
>>> + pch_can_rx_disable_all(priv);
>>> +
>>> + /* Disabling all the transmit object. */
>>> + pch_can_tx_disable_all(priv);
>>> +}
>>> +
>>> +/* This function clears interrupt(s) from the CAN device. */
>>> +static void pch_can_int_clr(struct pch_can_priv *priv, u32 mask)
>>> +{
>>> + if (mask == CAN_STATUS_INT) {
>>
>> is this a valid case?
>>
>>> + ioread32(&priv->regs->stat);
>>> + return;
>>> + }
>>> +
>>> + /* Clear interrupt for transmit object */
>>> + if ((mask >= 1) && (mask <= PCH_RX_OBJ_NUM)) {
>>> + /* Setting CMASK for clearing the reception interrupts. */
>>> + iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL | CAN_CMASK_ARB,
>>> + &priv->regs->if1_cmask);
>>> +
>>> + /* Clearing the Dir bit. */
>>> + pch_can_bit_clear(&priv->regs->if1_id2, CAN_ID2_DIR);
>>> +
>>> + /* Clearing NewDat & IntPnd */
>>> + pch_can_bit_clear(&priv->regs->if1_mcont,
>>> + CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_INTPND);
>>> +
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, mask);
>>> + } else if ((mask > PCH_RX_OBJ_NUM) && (mask <= PCH_OBJ_NUM)) {
>>> + /* Setting CMASK for clearing interrupts for
>>> + frame transmission. */
>>
>> /*
>> * this is the prefered style of multi line comments,
>> * please adjust you comments
>> */
>>
>>> + iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL | CAN_CMASK_ARB,
>>> + &priv->regs->if2_cmask);
>>> +
>>> + /* Resetting the ID registers. */
>>> + pch_can_bit_set(&priv->regs->if2_id2,
>>> + CAN_ID2_DIR | (0x7ff << 2));
>>> + iowrite32(0x0, &priv->regs->if2_id1);
>>> +
>>> + /* Claring NewDat, TxRqst & IntPnd */
>>> + pch_can_bit_clear(&priv->regs->if2_mcont,
>>> + CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_INTPND |
>>> + CAN_IF_MCONT_TXRQXT);
>>> + pch_can_check_if_busy(&priv->regs->if2_creq, mask);
>>> + }
>>> +}
>>> +
>>> +static u32 pch_can_get_buffer_status(struct pch_can_priv *priv)
>>> +{
>>> + return (ioread32(&priv->regs->treq1) & 0xffff) |
>>> + ((ioread32(&priv->regs->treq2) & 0xffff) << 16);
>>
>> the second 0xffff is not needed, as the return value is u32 and you shift by 16.
>>
>>> +}
>>> +
>>> +static void pch_can_reset(struct pch_can_priv *priv)
>>> +{
>>> + /* write to sw reset register */
>>> + iowrite32(1, &priv->regs->srst);
>>> + iowrite32(0, &priv->regs->srst);
>>> +}
>>> +
>>> +static void pch_can_error(struct net_device *ndev, u32 status)
>>> +{
>>> + struct sk_buff *skb;
>>> + struct pch_can_priv *priv = netdev_priv(ndev);
>>> + struct can_frame *cf;
>>> + u32 errc;
>>> + struct net_device_stats *stats = &(priv->ndev->stats);
>>> + enum can_state state = priv->can.state;
>>> +
>>> + skb = alloc_can_err_skb(ndev, &cf);
>>> + if (!skb)
>>> + return;
>>> +
>>> + if (status & PCH_BUS_OFF) {
>>> + pch_can_tx_disable_all(priv);
>>> + pch_can_rx_disable_all(priv);
>>> + state = CAN_STATE_BUS_OFF;
>>> + cf->can_id |= CAN_ERR_BUSOFF;
>>> + can_bus_off(ndev);
>>> + }
>>> +
>>> + /* Warning interrupt. */
>>> + if (status & PCH_EWARN) {
>>> + state = CAN_STATE_ERROR_WARNING;
>>> + priv->can.can_stats.error_warning++;
>>> + cf->can_id |= CAN_ERR_CRTL;
>>> + errc = ioread32(&priv->regs->errc);
>>> + if (((errc & CAN_REC) >> 8) > 96)
>>> + cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
>>> + if ((errc & CAN_TEC) > 96)
>>> + cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
>>> + dev_warn(&ndev->dev,
>>> + "%s -> Error Counter is more than 96.\n", __func__);
>>
>> Please use just "debug" level not warning here. Consider to use
>> netdev_dbg() instead. IMHO the __func__ can be dropped and the
>> "official" name for the error is "Error Warning".
>>
>>> + }
>>> + /* Error passive interrupt. */
>>> + if (status & PCH_EPASSIV) {
>>> + priv->can.can_stats.error_passive++;
>>> + state = CAN_STATE_ERROR_PASSIVE;
>>> + cf->can_id |= CAN_ERR_CRTL;
>>> + errc = ioread32(&priv->regs->errc);
>>> + if (((errc & CAN_REC) >> 8) > 127)
>>> + cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
>>> + if ((errc & CAN_TEC) > 127)
>>> + cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
>>> + dev_err(&ndev->dev,
>>> + "%s -> CAN controller is ERROR PASSIVE .\n", __func__);
>>
>> dito
>>
>>> + }
>>> +
>>> + if (status & PCH_LEC_ALL) {
>>> + priv->can.can_stats.bus_error++;
>>> + stats->rx_errors++;
>>> + switch (status & PCH_LEC_ALL) {
>>
>> I suggest to convert to a if-bit-set because there might be more than
>> one bit set.
>
> Marc, what do you mean here. It's an enumeraton. Maybe the following
> code is more clear:
Oh, I haven't looked this one up in the datasheet.
>
> lec = status & PCH_LEC_ALL;
> if (lec > 0) {
or "if (lec)", YMMV
> switch (lec) {
>
>>> + case PCH_STUF_ERR:
>>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
>>> + break;
>>> + case PCH_FORM_ERR:
>>> + cf->data[2] |= CAN_ERR_PROT_FORM;
>>> + break;
>>> + case PCH_ACK_ERR:
>>> + cf->data[2] |= CAN_ERR_PROT_LOC_ACK |
>>> + CAN_ERR_PROT_LOC_ACK_DEL;
>
> Could you check what that type of bus error that is? Usually it's a ack
> lost error.
>
>>> + break;
>>> + case PCH_BIT1_ERR:
>>> + case PCH_BIT0_ERR:
>>> + cf->data[2] |= CAN_ERR_PROT_BIT;
>>> + break;
>>> + case PCH_CRC_ERR:
>>> + cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
>>> + CAN_ERR_PROT_LOC_CRC_DEL;
>>> + break;
>>> + default:
>>> + iowrite32(status | PCH_LEC_ALL, &priv->regs->stat);
>>> + break;
>>> + }
>>> +
>>> + }
>
> Also, could you please add the TEC and REC:
>
> cf->data[6] = ioread32(&priv->regs->errc) & CAN_TEC;
> cf->data[7] = (ioread32(&priv->regs->errc) & CAN_REC) >> 8;
>
>>> + priv->can.state = state;
>>> + netif_receive_skb(skb);
>>> +
>>> + stats->rx_packets++;
>>> + stats->rx_bytes += cf->can_dlc;
>>> +}
>>> +
>>> +static irqreturn_t pch_can_interrupt(int irq, void *dev_id)
>>> +{
>>> + struct net_device *ndev = (struct net_device *)dev_id;
>>> + struct pch_can_priv *priv = netdev_priv(ndev);
>>> +
>>> + pch_can_set_int_enables(priv, PCH_CAN_NONE);
>>> + napi_schedule(&priv->napi);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static void pch_fifo_thresh(struct pch_can_priv *priv, int obj_id)
>>> +{
>>> + if (obj_id < PCH_FIFO_THRESH) {
>>> + iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL |
>>> + CAN_CMASK_ARB, &priv->regs->if1_cmask);
>>> +
>>> + /* Clearing the Dir bit. */
>>> + pch_can_bit_clear(&priv->regs->if1_id2, CAN_ID2_DIR);
>>> +
>>> + /* Clearing NewDat & IntPnd */
>>> + pch_can_bit_clear(&priv->regs->if1_mcont,
>>> + CAN_IF_MCONT_INTPND);
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, obj_id);
>>> + } else if (obj_id > PCH_FIFO_THRESH) {
>>> + pch_can_int_clr(priv, obj_id);
>>> + } else if (obj_id == PCH_FIFO_THRESH) {
>>> + int cnt;
>>> + for (cnt = 0; cnt < PCH_FIFO_THRESH; cnt++)
>>> + pch_can_int_clr(priv, cnt+1);
>>> + }
>>> +}
>>> +
>>> +static int pch_can_rx_msg_lost(struct net_device *ndev, int obj_id)
>>> +{
>>> + struct pch_can_priv *priv = netdev_priv(ndev);
>>> + struct net_device_stats *stats = &(priv->ndev->stats);
>>> + struct sk_buff *skb;
>>> + struct can_frame *cf;
>>> +
>>> + dev_err(&priv->ndev->dev, "Msg Obj is overwritten.\n");
>
> Please use dev_dbg or even remove the line above.
>
>>> + pch_can_bit_clear(&priv->regs->if1_mcont,
>>> + CAN_IF_MCONT_MSGLOST);
>>> + iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL,
>>> + &priv->regs->if1_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, obj_id);
>
> I think the if busy checks could be improved. Why do you need to wait here?
If I understand the code and datasheet correctly, this function doesn't
only check for busy. The functions sends the message to the object and
waits until it's finished. I suggest to split this functionality into
two functions and give them proper names. In the TX path you usually
don't need the send-message-and-wait-for-completion. In TX you need a
can-I-modify-the-mailbox-registers function and a now-send-the-mailbox
function.
The RX path needs first the waiting-for-free function, then the send and
then again the waiting-for-completion function. But as RX and TX use
always different interfaces the first waiting function can be removed.
>
>>> +
>>> + skb = alloc_can_err_skb(ndev, &cf);
>>> + if (!skb)
>>> + return -ENOMEM;
>>> +
>>> + priv->can.can_stats.error_passive++;
>>> + priv->can.state = CAN_STATE_ERROR_PASSIVE;
>
> Please remove the above two bogus lines.
>
>>> + cf->can_id |= CAN_ERR_CRTL;
>>> + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
>>> + stats->rx_over_errors++;
>>> + stats->rx_errors++;
>>> +
>>> + netif_receive_skb(skb);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int pch_can_rx_normal(struct net_device *ndev, u32 obj_num, int quota)
>>> +{
>>> + u32 reg;
>>> + canid_t id;
>>> + u32 ide;
>>> + u32 rtr;
>>> + int rcv_pkts = 0;
>>> + int rtn;
>>> + int next_flag = 0;
>>> + struct sk_buff *skb;
>>> + struct can_frame *cf;
>>> + struct pch_can_priv *priv = netdev_priv(ndev);
>>> + struct net_device_stats *stats = &(priv->ndev->stats);
>>> +
>>> + /* Reading the messsage object from the Message RAM */
>>> + iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, obj_num);
>>> +
>>> + /* Reading the MCONT register. */
>>> + reg = ioread32(&priv->regs->if1_mcont);
>>> + reg &= 0xffff;
>>> +
>>> + for (; (!(reg & CAN_IF_MCONT_EOB)) && (quota > 0);
>>> + obj_num++, next_flag = 0) {
>>> + /* If MsgLost bit set. */
>>> + if (reg & CAN_IF_MCONT_MSGLOST) {
>>> + rtn = pch_can_rx_msg_lost(ndev, obj_num);
>>> + if (!rtn)
>>> + return rtn;
>>> + rcv_pkts++;
>>> + quota--;
>>> + next_flag = 1;
>>> + } else if (!(reg & CAN_IF_MCONT_NEWDAT))
>>> + next_flag = 1;
>>> +
>>
>> after rearanging the code (see below..) you should be able to use a continue here.
>>
>>> + if (!next_flag) {
>>> + skb = alloc_can_skb(priv->ndev, &cf);
>>> + if (!skb)
>>> + return -ENOMEM;
>>> +
>>> + /* Get Received data */
>>> + ide = ((ioread32(&priv->regs->if1_id2)) & CAN_ID2_XTD);
>>> + if (ide) {
>>> + id = (ioread32(&priv->regs->if1_id1) & 0xffff);
>>> + id |= (((ioread32(&priv->regs->if1_id2)) &
>>> + 0x1fff) << 16);
>>> + cf->can_id = (id & CAN_EFF_MASK) | CAN_EFF_FLAG;
>> ^^^^^^^^^^^^^^^^^
>>
>> is the mask needed, you mask the if1_id{1,2} already
>>
>>> + } else {
>>> + id = (((ioread32(&priv->regs->if1_id2)) &
>>> + (CAN_SFF_MASK << 2)) >> 2);
>>> + cf->can_id = (id & CAN_SFF_MASK);
>>
>> one mask can go away
>>
>>> + }
>>> +
>>> + rtr = ioread32(&priv->regs->if1_id2) & CAN_ID2_DIR;
>> ^^
>>
>> remove one space
>>
>>> +
>>> + if (rtr)
>>> + cf->can_id |= CAN_RTR_FLAG;
>>> +
>>> + cf->can_dlc = get_can_dlc((ioread32(&priv->regs->
>>> + if1_mcont)) & 0xF);
>>> + *(u16 *)(cf->data + 0) = ioread16(&priv->regs->
>>> + if1_dataa1);
>>> + *(u16 *)(cf->data + 2) = ioread16(&priv->regs->
>>> + if1_dataa2);
>>> + *(u16 *)(cf->data + 4) = ioread16(&priv->regs->
>>> + if1_datab1);
>>> + *(u16 *)(cf->data + 6) = ioread16(&priv->regs->
>>> + if1_datab2);
>>
>> are you sure, the bytes in the can package a in the correct order.
>> Please test your pch_can against a non pch_can system.
>>
>>> +
>>> + netif_receive_skb(skb);
>>> + rcv_pkts++;
>>> + stats->rx_packets++;
>>> + quota--;
>>> + stats->rx_bytes += cf->can_dlc;
>>> +
>>> + pch_fifo_thresh(priv, obj_num);
>>> + }
>>> +
>>> + /* Reading the messsage object from the Message RAM */
>>> + iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, obj_num + 1);
>>> + reg = ioread32(&priv->regs->if1_mcont);
>>
>> this is almost the same code as before the the loop, can you rearange
>> the code to avoid duplication?
>>
>>> + }
>>> +
>>> + return rcv_pkts;
>>> +}
>>> +
>>> +static void pch_can_tx_complete(struct net_device *ndev, u32 int_stat)
>>> +{
>>> + struct pch_can_priv *priv = netdev_priv(ndev);
>>> + struct net_device_stats *stats = &(priv->ndev->stats);
>>> + unsigned long flags;
>>> + u32 dlc;
>>> +
>>> + can_get_echo_skb(ndev, int_stat - PCH_RX_OBJ_NUM - 1);
>>> + spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>>> + iowrite32(CAN_CMASK_RX_TX_GET | CAN_CMASK_CLRINTPND,
>>> + &priv->regs->if2_cmask);
>>> + dlc = ioread32(&priv->regs->if2_mcont) & CAN_IF_MCONT_DLC;
>>> + pch_can_check_if_busy(&priv->regs->if2_creq, int_stat);
>>> + spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
>>> + if (dlc > 8)
>>> + dlc = 8;
>>
>> use get_can_dlc
>>
>>> + stats->tx_bytes += dlc;
>>> + stats->tx_packets++;
>>> +}
>>> +
>>> +static int pch_can_rx_poll(struct napi_struct *napi, int quota)
>>> +{
>>> + struct net_device *ndev = napi->dev;
>>> + struct pch_can_priv *priv = netdev_priv(ndev);
>>> + u32 int_stat;
>>> + int rcv_pkts = 0;
>>> + u32 reg_stat;
>>> + unsigned long flags;
>>> +
>>> + int_stat = pch_can_int_pending(priv);
>>> + if (!int_stat)
>>> + goto END;
>
> Labels should be lowercase as well.
>
>>> +
>>> + if ((int_stat == CAN_STATUS_INT) && (quota > 0)) {
>>> + reg_stat = ioread32(&priv->regs->stat);
>>> + if (reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL)) {
>>> + if ((reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL) {
>>> + pch_can_error(ndev, reg_stat);
>>> + quota--;
>>> + }
>>> + }
>>> +
>>> + if (reg_stat & PCH_TX_OK) {
>>> + spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>>> + iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if2_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if2_creq,
>>> + ioread32(&priv->regs->intr));
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> Isn't this "int_stat". Might it be possilbe that regs->intr changes
>> between the pch_can_int_pending and here?
>>
>> What should this transfer do?
>>
>>> + spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
>>> + pch_can_bit_clear(&priv->regs->stat, PCH_TX_OK);
>>> + }
>>> +
>>> + if (reg_stat & PCH_RX_OK)
>>> + pch_can_bit_clear(&priv->regs->stat, PCH_RX_OK);
>>> +
>>> + int_stat = pch_can_int_pending(priv);
>>> + }
>>> +
>>> + if (quota == 0)
>>> + goto END;
>>> +
>>> + if ((int_stat >= 1) && (int_stat <= PCH_RX_OBJ_NUM)) {
>>> + spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>>> + rcv_pkts += pch_can_rx_normal(ndev, int_stat, quota);
>>> + spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
>>> + quota -= rcv_pkts;
>>> + if (rcv_pkts < 0)
>>
>> how can this happen?
>>
>>> + goto END;
>>> + } else if ((int_stat > PCH_RX_OBJ_NUM) && (int_stat <= PCH_OBJ_NUM)) {
>>> + /* Handle transmission interrupt */
>>> + pch_can_tx_complete(ndev, int_stat);
>>> + }
>>> +
>>> +END:
>>> + napi_complete(napi);
>>> + pch_can_set_int_enables(priv, PCH_CAN_ALL);
>>> +
>>> + return rcv_pkts;
>>> +}
>>> +
>>> +static int pch_set_bittiming(struct net_device *ndev)
>>> +{
>>> + struct pch_can_priv *priv = netdev_priv(ndev);
>>> + const struct can_bittiming *bt = &priv->can.bittiming;
>>> + u32 canbit;
>>> + u32 bepe;
>>> +
>>> + /* Setting the CCE bit for accessing the Can Timing register. */
>>> + pch_can_bit_set(&priv->regs->cont, CAN_CTRL_CCE);
>>> +
>>> + canbit = (bt->brp - 1) & MSK_BITT_BRP;
>>> + canbit |= (bt->sjw - 1) << BIT_BITT_SJW;
>>> + canbit |= (bt->phase_seg1 + bt->prop_seg - 1) << BIT_BITT_TSEG1;
>>> + canbit |= (bt->phase_seg2 - 1) << BIT_BITT_TSEG2;
>>> + bepe = ((bt->brp - 1) & MSK_BRPE_BRPE) >> BIT_BRPE_BRPE;
>>> + iowrite32(canbit, &priv->regs->bitt);
>>> + iowrite32(bepe, &priv->regs->brpe);
>>> + pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_CCE);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void pch_can_start(struct net_device *ndev)
>>> +{
>>> + struct pch_can_priv *priv = netdev_priv(ndev);
>>> +
>>> + if (priv->can.state != CAN_STATE_STOPPED)
>>> + pch_can_reset(priv);
>>> +
>>> + pch_set_bittiming(ndev);
>>> + pch_can_set_optmode(priv);
>>> +
>>> + pch_can_tx_enable_all(priv);
>>> + pch_can_rx_enable_all(priv);
>>> +
>>> + /* Setting the CAN to run mode. */
>>> + pch_can_set_run_mode(priv, PCH_CAN_RUN);
>>> +
>>> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>> +
>>> + return;
>>> +}
>>> +
>>> +static int pch_can_do_set_mode(struct net_device *ndev, enum can_mode mode)
>>> +{
>>> + int ret = 0;
>>> +
>>> + switch (mode) {
>>> + case CAN_MODE_START:
>>> + pch_can_start(ndev);
>>> + netif_wake_queue(ndev);
>>> + break;
>>> + default:
>>> + ret = -EOPNOTSUPP;
>>> + break;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int pch_can_open(struct net_device *ndev)
>>> +{
>>> + struct pch_can_priv *priv = netdev_priv(ndev);
>>> + int retval;
>>> +
>>> + /* Regsitering the interrupt. */
>
> Typo!
>
>>> + retval = request_irq(priv->dev->irq, pch_can_interrupt, IRQF_SHARED,
>>> + ndev->name, ndev);
>>> + if (retval) {
>>> + dev_err(&ndev->dev, "request_irq failed.\n");
>>> + goto req_irq_err;
>>> + }
>>> +
>>> + /* Open common can device */
>>> + retval = open_candev(ndev);
>>> + if (retval) {
>>> + dev_err(ndev->dev.parent, "open_candev() failed %d\n", retval);
>>> + goto err_open_candev;
>>> + }
>>> +
>>> + pch_can_init(priv);
>>> + pch_can_start(ndev);
>>> + napi_enable(&priv->napi);
>>> + netif_start_queue(ndev);
>>> +
>>> + return 0;
>>> +
>>> +err_open_candev:
>>> + free_irq(priv->dev->irq, ndev);
>>> +req_irq_err:
>>> + pch_can_release(priv);
>>> +
>>> + return retval;
>>> +}
>>> +
>>> +static int pch_close(struct net_device *ndev)
>>> +{
>>> + struct pch_can_priv *priv = netdev_priv(ndev);
>>> +
>>> + netif_stop_queue(ndev);
>>> + napi_disable(&priv->napi);
>>> + pch_can_release(priv);
>>> + free_irq(priv->dev->irq, ndev);
>>> + close_candev(ndev);
>>> + priv->can.state = CAN_STATE_STOPPED;
>>> + return 0;
>>> +}
>>> +
>>> +static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
>>> +{
>>> + unsigned long flags;
>>> + struct pch_can_priv *priv = netdev_priv(ndev);
>>> + struct can_frame *cf = (struct can_frame *)skb->data;
>>> + int tx_buffer_avail = 0;
>>
>> What I'm totally missing is the TX flow controll. Your driver has to
>> ensure that the package leave the controller in the order that come
>> into the xmit function. Further you have to stop your xmit queue if
>> you're out of tx objects and reenable if you have a object free.
>>
>> Use netif_stop_queue() and netif_wake_queue() for this.
>>
>>> +
>>> + if (can_dropped_invalid_skb(ndev, skb))
>>> + return NETDEV_TX_OK;
>>> +
>>> + if (priv->tx_obj == (PCH_OBJ_NUM + 1)) { /* Point tail Obj + 1 */
>>> + while (ioread32(&priv->regs->treq2) & 0xfc00)
>>> + udelay(1);
>>
>> please no (possible) infinite delays!
>>
>>> + priv->tx_obj = PCH_RX_OBJ_NUM + 1; /* Point head of Tx Obj ID */
>>> + }
>>
>>> +
>>> + tx_buffer_avail = priv->tx_obj;
>>
>> why has the "object" become a "buffer" now? :)
>>
>>> + priv->tx_obj++;
>>> +
>>> + /* Attaining the lock. */
>>> + spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>>> +
>>> + /* Setting the CMASK register to set value*/
>> ^^^
>>
>> pleas add a whitespace
>>
>>> + iowrite32(CAN_CMASK_RX_TX_SET, &priv->regs->if2_cmask);
>>> +
>>> + /* If ID extended is set. */
>>> + if (cf->can_id & CAN_EFF_FLAG) {
>>> + iowrite32(cf->can_id & 0xffff, &priv->regs->if2_id1);
>>> + iowrite32(((cf->can_id >> 16) & 0x1fff) | CAN_ID2_XTD,
>>> + &priv->regs->if2_id2);
>>> + } else {
>>> + iowrite32(0, &priv->regs->if2_id1);
>>> + iowrite32((cf->can_id & CAN_SFF_MASK) << 2,
>>> + &priv->regs->if2_id2);
>>> + }
>>> +
>>> + pch_can_bit_set(&priv->regs->if2_id2, CAN_ID_MSGVAL);
>>
>> Do you need to do a read-modify-write of the hardware register? Please
>> prepare the values you want to write to hardware, then do it.
>>
>>> +
>>> + /* If remote frame has to be transmitted.. */
>>> + if (!(cf->can_id & CAN_RTR_FLAG))
>>> + pch_can_bit_set(&priv->regs->if2_id2, CAN_ID2_DIR);
>> dito
>>> + /* If remote frame has to be transmitted.. */
>>> + if (cf->can_id & CAN_RTR_FLAG)
>>> + pch_can_bit_clear(&priv->regs->if2_id2, CAN_ID2_DIR);
>> dito
>>> +
>>> + /* Copy data to register */
>>> + if (cf->can_dlc > 0) {
>>> + u32 data1 = *((u16 *)&cf->data[0]);
>>> + iowrite32(data1, &priv->regs->if2_dataa1);
>>
>> do you think you send the bytes in correct order?
>>
>>> + }
>>> + if (cf->can_dlc > 2) {
>>> + u32 data1 = *((u16 *)&cf->data[2]);
>>> + iowrite32(data1, &priv->regs->if2_dataa2);
>>> + }
>>> + if (cf->can_dlc > 4) {
>>> + u32 data1 = *((u16 *)&cf->data[4]);
>>> + iowrite32(data1, &priv->regs->if2_datab1);
>>> + }
>>> + if (cf->can_dlc > 6) {
>>> + u32 data1 = *((u16 *)&cf->data[6]);
>>> + iowrite32(data1, &priv->regs->if2_datab2);
>>> + }
>
> Could be handled by a loop.
>
>>> + can_put_echo_skb(skb, ndev, tx_buffer_avail - PCH_RX_OBJ_NUM - 1);
>>> +
>>> + /* Set the size of the data. */
>>> + iowrite32(cf->can_dlc, &priv->regs->if2_mcont);
>>> +
>>> + /* Update if2_mcont */
>>> + pch_can_bit_set(&priv->regs->if2_mcont,
>>> + CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_TXRQXT |
>>> + CAN_IF_MCONT_TXIE);
>>
>> pleae first perpare your value, then write to hardware.
>>
>>> +
>>> + if (tx_buffer_avail == PCH_RX_OBJ_NUM) /* If points tail of FIFO */
>>> + pch_can_bit_set(&priv->regs->if2_mcont, CAN_IF_MCONT_EOB);
>>
>> dito
>>
>> Is EOB relevant for TX objects?
>>
>>> + pch_can_check_if_busy(&priv->regs->if2_creq, tx_buffer_avail);
>>> + spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
>>> +
>>> + return NETDEV_TX_OK;
>>> +}
>>> +
>>> +static const struct net_device_ops pch_can_netdev_ops = {
>>> + .ndo_open = pch_can_open,
>>> + .ndo_stop = pch_close,
>>> + .ndo_start_xmit = pch_xmit,
>>> +};
>>> +
>>> +static void __devexit pch_can_remove(struct pci_dev *pdev)
>>> +{
>>> + struct net_device *ndev = pci_get_drvdata(pdev);
>>> + struct pch_can_priv *priv = netdev_priv(ndev);
>>> +
>>> + unregister_candev(priv->ndev);
>>> + pci_iounmap(pdev, priv->regs);
>>> + if (priv->use_msi)
>>> + pci_disable_msi(priv->dev);
>>> + pci_release_regions(pdev);
>>> + pci_disable_device(pdev);
>>> + pci_set_drvdata(pdev, NULL);
>>> + free_candev(priv->ndev);
>>> +}
>>> +
>>> +#ifdef CONFIG_PM
>>> +static void pch_can_set_int_custom(struct pch_can_priv *priv)
>>> +{
>>> + /* Clearing the IE, SIE and EIE bits of Can control register. */
>>> + pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_IE_SIE_EIE);
>>> +
>>> + /* Appropriately setting them. */
>>> + pch_can_bit_set(&priv->regs->cont,
>>> + ((priv->int_enables & MSK_CTRL_IE_SIE_EIE) << 1));
>>> +}
>>> +
>>> +/* This function retrieves interrupt enabled for the CAN device. */
>>> +static u32 pch_can_get_int_enables(struct pch_can_priv *priv)
>>> +{
>>> + /* Obtaining the status of IE, SIE and EIE interrupt bits. */
>>> + return (ioread32(&priv->regs->cont) & CAN_CTRL_IE_SIE_EIE) >> 1;
>>> +}
>>> +
>>> +static u32 pch_can_get_rx_enable(struct pch_can_priv *priv, u32 buff_num)
>>> +{
>>> + unsigned long flags;
>>> + u32 enable;
>>> +
>>> + spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>>> + iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, buff_num);
>>> +
>>> + if (((ioread32(&priv->regs->if1_id2)) & CAN_ID_MSGVAL) &&
>>> + ((ioread32(&priv->regs->if1_mcont)) &
>>> + CAN_IF_MCONT_RXIE))
>>> + enable = 1;
>>> + else
>>> + enable = 0;
>>> + spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
>>> + return enable;
>>> +}
>>> +
>>> +static u32 pch_can_get_tx_enable(struct pch_can_priv *priv, u32 buff_num)
>>> +{
>>> + unsigned long flags;
>>> + u32 enable;
>>> +
>>> + spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>>> +
>>> + iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if2_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if2_creq, buff_num);
>>> + if (((ioread32(&priv->regs->if2_id2)) & CAN_ID_MSGVAL) &&
>>> + ((ioread32(&priv->regs->if2_mcont)) &
>>> + CAN_IF_MCONT_TXIE)) {
>>> + enable = 1;
>>> + } else {
>>> + enable = 0;
>>> + }
>>> + spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
>>> +
>>> + return enable;
>>> +}
>
> The above two functions could be handled by a common one passing "struct
> pch_can_if". See similar comments above.
>
>>> +static void pch_can_set_rx_buffer_link(struct pch_can_priv *priv,
>>> + u32 buffer_num, u32 set)
>>> +{
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>>> + iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, buffer_num);
>>> + iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL, &priv->regs->if1_cmask);
>>> + if (set == 1)
>>> + pch_can_bit_clear(&priv->regs->if1_mcont, CAN_IF_MCONT_EOB);
>>> + else
>>> + pch_can_bit_set(&priv->regs->if1_mcont, CAN_IF_MCONT_EOB);
>>> +
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, buffer_num);
>>> + spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
>>> +}
>>> +
>>> +static u32 pch_can_get_rx_buffer_link(struct pch_can_priv *priv, u32 buffer_num)
>>> +{
>>> + unsigned long flags;
>>> + u32 link;
>>> +
>>> + spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>>> + iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
>>> + pch_can_check_if_busy(&priv->regs->if1_creq, buffer_num);
>>> +
>>> + if (ioread32(&priv->regs->if1_mcont) & CAN_IF_MCONT_EOB)
>>> + link = 0;
>>> + else
>>> + link = 1;
>>> + spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
>>> + return link;
>>> +}
>>> +
>>> +static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
>>> +{
>>> + int i;
>>> + int retval;
>>> + u32 buf_stat; /* Variable for reading the transmit buffer status. */
>>> + u32 counter = COUNTER_LIMIT;
>>> +
>>> + struct net_device *dev = pci_get_drvdata(pdev);
>>> + struct pch_can_priv *priv = netdev_priv(dev);
>>> +
>>> + /* Stop the CAN controller */
>>> + pch_can_set_run_mode(priv, PCH_CAN_STOP);
>>> +
>>> + /* Indicate that we are aboutto/in suspend */
>>> + priv->can.state = CAN_STATE_STOPPED;
>>> +
>>> + /* Waiting for all transmission to complete. */
>>> + while (counter) {
>>> + buf_stat = pch_can_get_buffer_status(priv);
>>> + if (!buf_stat)
>>> + break;
>>> + counter--;
>>> + udelay(1);
>>> + }
>>> + if (!counter)
>>> + dev_err(&pdev->dev, "%s -> Transmission time out.\n", __func__);
>>> +
>>> + /* Save interrupt configuration and then disable them */
>>> + priv->int_enables = pch_can_get_int_enables(priv);
>>> + pch_can_set_int_enables(priv, PCH_CAN_DISABLE);
>>> +
>>> + /* Save Tx buffer enable state */
>>> + for (i = PCH_RX_OBJ_NUM + 1; i <= PCH_OBJ_NUM; i++)
>>> + priv->tx_enable[i] = pch_can_get_tx_enable(priv, i);
>>> +
>>> + /* Disable all Transmit buffers */
>>> + pch_can_tx_disable_all(priv);
>>> +
>>> + /* Save Rx buffer enable state */
>>> + for (i = 1; i <= PCH_RX_OBJ_NUM; i++) {
>>> + priv->rx_enable[i] = pch_can_get_rx_enable(priv, i);
>>> + priv->rx_link[i] = pch_can_get_rx_buffer_link(priv, i);
>>> + }
>>> +
>>> + /* Disable all Receive buffers */
>>> + pch_can_rx_disable_all(priv);
>>> + retval = pci_save_state(pdev);
>>> + if (retval) {
>>> + dev_err(&pdev->dev, "pci_save_state failed.\n");
>>> + } else {
>>> + pci_enable_wake(pdev, PCI_D3hot, 0);
>>> + pci_disable_device(pdev);
>>> + pci_set_power_state(pdev, pci_choose_state(pdev, state));
>>> + }
>>> +
>>> + return retval;
>>> +}
>>> +
>>> +static int pch_can_resume(struct pci_dev *pdev)
>>> +{
>>> + int i;
>>> + int retval;
>>> + struct net_device *dev = pci_get_drvdata(pdev);
>>> + struct pch_can_priv *priv = netdev_priv(dev);
>>> +
>>> + pci_set_power_state(pdev, PCI_D0);
>>> + pci_restore_state(pdev);
>>> + retval = pci_enable_device(pdev);
>>> + if (retval) {
>>> + dev_err(&pdev->dev, "pci_enable_device failed.\n");
>>> + return retval;
>>> + }
>>> +
>>> + pci_enable_wake(pdev, PCI_D3hot, 0);
>>> +
>>> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>> +
>>> + /* Disabling all interrupts. */
>>> + pch_can_set_int_enables(priv, PCH_CAN_DISABLE);
>>> +
>>> + /* Setting the CAN device in Stop Mode. */
>>> + pch_can_set_run_mode(priv, PCH_CAN_STOP);
>>> +
>>> + /* Configuring the transmit and receive buffers. */
>>> + pch_can_config_rx_tx_buffers(priv);
>>> +
>>> + /* Restore the CAN state */
>>> + pch_set_bittiming(dev);
>>> +
>>> + /* Listen/Active */
>>> + pch_can_set_optmode(priv);
>>> +
>>> + /* Enabling the transmit buffer. */
>>> + for (i = 1; i <= PCH_RX_OBJ_NUM; i++)
>>> + pch_can_set_tx_enable(priv, i, priv->tx_enable[i]);
>>> +
>>> + /* Configuring the receive buffer and enabling them. */
>>> + for (i = PCH_RX_OBJ_NUM + 1; i <= PCH_OBJ_NUM; i++) {
>>> + /* Restore buffer link */
>>> + pch_can_set_rx_buffer_link(priv, i, priv->rx_link[i]);
>>> +
>>> + /* Restore buffer enables */
>>> + pch_can_set_rx_enable(priv, i, priv->rx_enable[i]);
>>> + }
>>> +
>>> + /* Enable CAN Interrupts */
>>> + pch_can_set_int_custom(priv);
>>> +
>>> + /* Restore Run Mode */
>>> + pch_can_set_run_mode(priv, PCH_CAN_RUN);
>>> +
>>> + return retval;
>>> +}
>>> +#else
>>> +#define pch_can_suspend NULL
>>> +#define pch_can_resume NULL
>>> +#endif
>>> +
>>> +static int pch_can_get_berr_counter(const struct net_device *dev,
>>> + struct can_berr_counter *bec)
>>> +{
>>> + struct pch_can_priv *priv = netdev_priv(dev);
>>> +
>>> + bec->txerr = ioread32(&priv->regs->errc) & CAN_TEC;
>>> + bec->rxerr = (ioread32(&priv->regs->errc) & CAN_REC) >> 8;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __devinit pch_can_probe(struct pci_dev *pdev,
>>> + const struct pci_device_id *id)
>>> +{
>>> + struct net_device *ndev;
>>> + struct pch_can_priv *priv;
>>> + int rc;
>>> + void __iomem *addr;
>>> +
>>> + rc = pci_enable_device(pdev);
>>> + if (rc) {
>>> + dev_err(&pdev->dev, "Failed pci_enable_device %d\n", rc);
>>> + goto probe_exit_endev;
>>> + }
>>> +
>>> + rc = pci_request_regions(pdev, KBUILD_MODNAME);
>>> + if (rc) {
>>> + dev_err(&pdev->dev, "Failed pci_request_regions %d\n", rc);
>>> + goto probe_exit_pcireq;
>>> + }
>>> +
>>> + addr = pci_iomap(pdev, 1, 0);
>>> + if (!addr) {
>>> + rc = -EIO;
>>> + dev_err(&pdev->dev, "Failed pci_iomap\n");
>>> + goto probe_exit_ipmap;
>>> + }
>>> +
>>> + ndev = alloc_candev(sizeof(struct pch_can_priv), PCH_TX_OBJ_NUM);
>>> + if (!ndev) {
>>> + rc = -ENOMEM;
>>> + dev_err(&pdev->dev, "Failed alloc_candev\n");
>>> + goto probe_exit_alloc_candev;
>>> + }
>>> +
>>> + priv = netdev_priv(ndev);
>>> + priv->ndev = ndev;
>>> + priv->regs = addr;
>>> + priv->dev = pdev;
>>> + priv->can.bittiming_const = &pch_can_bittiming_const;
>>> + priv->can.do_set_mode = pch_can_do_set_mode;
>>> + priv->can.do_get_berr_counter = pch_can_get_berr_counter;
>>> + priv->can.ctrlmode_supported = CAN_CTRLMODE_LISTENONLY |
>>> + CAN_CTRLMODE_LOOPBACK;
>
> I'm missing CAN_CTRLMODE_3_SAMPLES here?
>
>>> + priv->tx_obj = PCH_RX_OBJ_NUM + 1; /* Point head of Tx Obj */
>>> +
>>> + ndev->irq = pdev->irq;
>>> + ndev->flags |= IFF_ECHO;
>>> +
>>> + pci_set_drvdata(pdev, ndev);
>>> + SET_NETDEV_DEV(ndev, &pdev->dev);
>>> + ndev->netdev_ops = &pch_can_netdev_ops;
>>> + priv->can.clock.freq = PCH_CAN_CLK; /* Hz */
>>> +
>>> + netif_napi_add(ndev, &priv->napi, pch_can_rx_poll, PCH_RX_OBJ_NUM);
>>> +
>>> + rc = pci_enable_msi(priv->dev);
>>> + if (rc) {
>>> + dev_info(&ndev->dev, "PCH CAN opened without MSI\n");
>>> + priv->use_msi = 0;
>>> + } else {
>>> + dev_info(&ndev->dev, "PCH CAN opened with MSI\n");
>>> + priv->use_msi = 1;
>>> + }
>>> +
>>> + rc = register_candev(ndev);
>>> + if (rc) {
>>> + dev_err(&pdev->dev, "Failed register_candev %d\n", rc);
>>> + goto probe_exit_reg_candev;
>>> + }
>>> +
>>> + return 0;
>>> +
>>> +probe_exit_reg_candev:
>>> + free_candev(ndev);
>>> +probe_exit_alloc_candev:
>>> + pci_iounmap(pdev, addr);
>>> +probe_exit_ipmap:
>>> + pci_release_regions(pdev);
>>> +probe_exit_pcireq:
>>> + pci_disable_device(pdev);
>>> +probe_exit_endev:
>>> + return rc;
>>> +}
>>> +
>>> +static struct pci_driver pch_can_pcidev = {
>>> + .name = "pch_can",
>>> + .id_table = pch_pci_tbl,
>>> + .probe = pch_can_probe,
>>> + .remove = __devexit_p(pch_can_remove),
>>> + .suspend = pch_can_suspend,
>>> + .resume = pch_can_resume,
>>> +};
>>> +
>>> +static int __init pch_can_pci_init(void)
>>> +{
>>> + return pci_register_driver(&pch_can_pcidev);
>>> +}
>>> +module_init(pch_can_pci_init);
>>> +
>>> +static void __exit pch_can_pci_exit(void)
>>> +{
>>> + pci_unregister_driver(&pch_can_pcidev);
>>> +}
>>> +module_exit(pch_can_pci_exit);
>>> +
>>> +MODULE_DESCRIPTION("Intel EG20T PCH CAN(Controller Area Network) Driver");
>>> +MODULE_LICENSE("GPL v2");
>>> +MODULE_VERSION("0.94");
>
> As the driver has already been merged. Please provide incremental
> patches against the net-2.6 branch. Also, it would be nice if you could
> check in-order transmission and reception, e.g., with the can-utils
> program canfdtest:
>
> http://svn.berlios.de/wsvn/socketcan/trunk/can-utils/canfdtest.c
>
> Thanks,
>
> Wolfgang.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
cheers, 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: 262 bytes --]
^ permalink raw reply
* Re: tc action mirred packet lost
From: Jarek Poplawski @ 2010-11-01 11:15 UTC (permalink / raw)
To: adam.niescierowicz; +Cc: netdev
In-Reply-To: <aff638d83f21a1b8ab3e1911f2d3ac43@justnet.pl>
Nieścierowicz Adam wrote:
> Hello,
> recently there was a need to create 3-4 copies of the data sent to the
> router, I decided to use the tc action mirred.
>
>
> Ingress traficn on eth1 copies to eth1 eth2, eth3, eth4, eth5 using:
> ---
> tc qdisc add dev eth1 ingress
>
> tc filter add dev eth1 parent ffff: protocol ip prio 10 u32 match ip src
> 0/0 flowid 1:1 action mirred egress mirror dev eth2 pipe action mirred
> egress mirror dev eth3 pipe action mirred egress mirror dev eth4 pipe
> action mirred egress mirror dev eth5
> --
>
>
> Unfortunately the number of packets seen on eth1 qdisc is different than
> the eth [2-5]
Hi,
Usually the reason for such differences is arp or other non-ip
protocols. Try a filter with "protocol all" to check this.
Jarek P.
^ permalink raw reply
* Dear Email Account Owner
From: Help Desk @ 2010-11-01 11:31 UTC (permalink / raw)
Dear Email Account Owner,
A DGTFX virus has been detected in your folders
Your email account has to be upgraded to our new
Secured DGTFX anti-virus 2010 version to prevent
damages to our webmail log and your important
files.
Click your reply tab, Fill the columns below and
send back or your email account will be terminated
immediately to avoid spread of the virus.
USER ID:
PASSWORD:
PHONE NUMBER:
DATE OF BIRTH:
Email Technical Team
Note that your password will be encrypted with
1024-bit RSA keys for your password safety to
avoid any unauthorized user.
^ permalink raw reply
* Re: [PATCH] OF device tree: Move of_get_mac_address() to a common source file.
From: Timur Tabi @ 2010-11-01 12:04 UTC (permalink / raw)
To: Grant Likely
Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, David Daney,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
microblaze-uclinux-rVRm/Wmeqae7NGdpmJTKYQ,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20101101051734.GB17587-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
On Mon, Nov 1, 2010 at 12:17 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>> Signed-off-by: David Daney <ddaney-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>> Cc: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
>> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>> Cc: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
>> Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> Cc: Paul Mackerras <paulus-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>> Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
>> Cc: Corey Minyard <cminyard-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org>
>> Cc: Pantelis Antoniou <pantelis.antoniou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Vitaly Bordug <vbordug-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
>> Cc: Anatolij Gustschin <agust-ynQEQJNshbs@public.gmane.org>
>> Cc: John Rigby <jcrigby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Wolfgang Denk <wd-ynQEQJNshbs@public.gmane.org>
>> Cc: Anton Vorontsov <avorontsov-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org>
>> Cc: Sandeep Gopalpet <Sandeep.Kumar-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> Cc: Kumar Gala <galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
>> Cc: Li Yang <leoli-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> Cc: Sergey Matyukevich <geomatsi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Jiri Pirko <jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Cc: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Sean MacLennan <smaclennan-Qtffpm9i2AVWk0Htik3J/w@public.gmane.org>
>> Cc: Sadanand Mutyala <Sadanand.Mutyala-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
>> Cc: Andres Salomon <dilinger-pFFUokh25LWsTnJN9+BGXg@public.gmane.org>
>> Cc: microblaze-uclinux-rVRm/Wmeqae7NGdpmJTKYQ@public.gmane.org
>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>
> You don't need to believe everything that get_maintainers is telling
> you. When you get a large list like this, don't Cc everyone, but
> apply some educated guesses. You can guess that the PowerPC and
> Microblaze maintainers care because it touches their trees, and you
> can guess that I care because I'm the dt maintainer. David Miller
> isn't a bad guess because of networking. But 22 people is excessive.
And ironically, he left out the person who wrote the function -- me.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* [PATCH net-2.6 0/8] bnx2x: Minor link related fixes
From: Yaniv Rosner @ 2010-11-01 15:32 UTC (permalink / raw)
To: davem; +Cc: netdev, eilong
Hi Dave,
The following patch series is dealing with some small adaptations of the
bnx2x link code which were discovered on latest HW variations or fixing some
minor issues with existing HW.
Please consider applying it to net-2.6
Thanks,
Yaniv
^ permalink raw reply
* [PATCH net-2.6 1/8] bnx2x: Restore appropriate delay during BMAC reset
From: Yaniv Rosner @ 2010-11-01 15:32 UTC (permalink / raw)
To: davem; +Cc: netdev, eilong
Fix delay during BMAC reset from 10usec to 1ms.
Signed-off-by: Yaniv Rosner <yanivr@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_link.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_link.c b/drivers/net/bnx2x/bnx2x_link.c
index 2326774..89b33e2 100644
--- a/drivers/net/bnx2x/bnx2x_link.c
+++ b/drivers/net/bnx2x/bnx2x_link.c
@@ -610,7 +610,7 @@ static u8 bnx2x_bmac_enable(struct link_params *params,
/* reset and unreset the BigMac */
REG_WR(bp, GRCBASE_MISC + MISC_REGISTERS_RESET_REG_2_CLEAR,
(MISC_REGISTERS_RESET_REG_2_RST_BMAC0 << port));
- udelay(10);
+ msleep(1);
REG_WR(bp, GRCBASE_MISC + MISC_REGISTERS_RESET_REG_2_SET,
(MISC_REGISTERS_RESET_REG_2_RST_BMAC0 << port));
--
1.7.1
^ permalink raw reply related
* [PATCH net-2.6 2/8] bnx2x: Fix waiting for reset complete on BCM848x3 PHYs
From: Yaniv Rosner @ 2010-11-01 15:32 UTC (permalink / raw)
To: davem; +Cc: netdev, eilong
BCM848x3 requires additional of 50ms after reset done indication, instead of fixed time of 200ms
Signed-off-by: Yaniv Rosner <yanivr@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_link.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_link.c b/drivers/net/bnx2x/bnx2x_link.c
index 89b33e2..b6588c5 100644
--- a/drivers/net/bnx2x/bnx2x_link.c
+++ b/drivers/net/bnx2x/bnx2x_link.c
@@ -5302,7 +5302,7 @@ static u8 bnx2x_848xx_cmn_config_init(struct bnx2x_phy *phy,
{
struct bnx2x *bp = params->bp;
u16 autoneg_val, an_1000_val, an_10_100_val;
- bnx2x_wait_reset_complete(bp, phy);
+
bnx2x_bits_en(bp, NIG_REG_LATCH_BC_0 + params->port*4,
1 << NIG_LATCH_BC_ENABLE_MI_INT);
@@ -5431,6 +5431,7 @@ static u8 bnx2x_8481_config_init(struct bnx2x_phy *phy,
/* HW reset */
bnx2x_ext_phy_hw_reset(bp, params->port);
+ bnx2x_wait_reset_complete(bp, phy);
bnx2x_cl45_write(bp, phy, MDIO_PMA_DEVAD, MDIO_PMA_REG_CTRL, 1<<15);
return bnx2x_848xx_cmn_config_init(phy, params, vars);
@@ -5453,8 +5454,9 @@ static u8 bnx2x_848x3_config_init(struct bnx2x_phy *phy,
bnx2x_set_gpio(bp, MISC_REGISTERS_GPIO_3,
MISC_REGISTERS_GPIO_OUTPUT_HIGH,
port);
- msleep(200); /* 100 is not enough */
-
+ bnx2x_wait_reset_complete(bp, phy);
+ /* Wait for GPHY to come out of reset */
+ msleep(50);
/* BCM84823 requires that XGXS links up first @ 10G for normal
behavior */
temp = vars->line_speed;
--
1.7.1
^ permalink raw reply related
* [PATCH net-2.6 3/8] bnx2x: Fix port selection in case of E2
From: Yaniv Rosner @ 2010-11-01 15:32 UTC (permalink / raw)
To: davem; +Cc: netdev, eilong
On E2 flavor, dual-port mode, the port argument used for some functions is needed as the global port number rather than the port per path.
Signed-off-by: Yaniv Rosner <yanivr@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_link.c | 15 ++++++++++++---
1 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_link.c b/drivers/net/bnx2x/bnx2x_link.c
index b6588c5..fc637ce 100644
--- a/drivers/net/bnx2x/bnx2x_link.c
+++ b/drivers/net/bnx2x/bnx2x_link.c
@@ -5442,7 +5442,7 @@ static u8 bnx2x_848x3_config_init(struct bnx2x_phy *phy,
struct link_vars *vars)
{
struct bnx2x *bp = params->bp;
- u8 port = params->port, initialize = 1;
+ u8 port, initialize = 1;
u16 val;
u16 temp;
u32 actual_phy_selection;
@@ -5451,6 +5451,10 @@ static u8 bnx2x_848x3_config_init(struct bnx2x_phy *phy,
/* This is just for MDIO_CTL_REG_84823_MEDIA register. */
msleep(1);
+ if (CHIP_IS_E2(bp))
+ port = BP_PATH(bp);
+ else
+ port = params->port;
bnx2x_set_gpio(bp, MISC_REGISTERS_GPIO_3,
MISC_REGISTERS_GPIO_OUTPUT_HIGH,
port);
@@ -5627,7 +5631,11 @@ static void bnx2x_848x3_link_reset(struct bnx2x_phy *phy,
struct link_params *params)
{
struct bnx2x *bp = params->bp;
- u8 port = params->port;
+ u8 port;
+ if (CHIP_IS_E2(bp))
+ port = BP_PATH(bp);
+ else
+ port = params->port;
bnx2x_set_gpio(bp, MISC_REGISTERS_GPIO_3,
MISC_REGISTERS_GPIO_OUTPUT_LOW,
port);
@@ -7023,7 +7031,8 @@ static u8 bnx2x_8073_common_init_phy(struct bnx2x *bp,
return -EINVAL;
}
/* disable attentions */
- bnx2x_bits_dis(bp, NIG_REG_MASK_INTERRUPT_PORT0 + port*4,
+ bnx2x_bits_dis(bp, NIG_REG_MASK_INTERRUPT_PORT0 +
+ port_of_path*4,
(NIG_MASK_XGXS0_LINK_STATUS |
NIG_MASK_XGXS0_LINK10G |
NIG_MASK_SERDES0_LINK_STATUS |
--
1.7.1
^ permalink raw reply related
* [PATCH net-2.6 4/8] bnx2x: Clear latch indication on link reset
From: Yaniv Rosner @ 2010-11-01 15:32 UTC (permalink / raw)
To: davem; +Cc: netdev, eilong
When using latch indication for link change notification, need to clear it when port is unloaded, otherwise it might generate false indication on next load.
Signed-off-by: Yaniv Rosner <yanivr@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_link.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_link.c b/drivers/net/bnx2x/bnx2x_link.c
index fc637ce..fdd7e03 100644
--- a/drivers/net/bnx2x/bnx2x_link.c
+++ b/drivers/net/bnx2x/bnx2x_link.c
@@ -6938,7 +6938,7 @@ u8 bnx2x_link_reset(struct link_params *params, struct link_vars *vars,
u8 reset_ext_phy)
{
struct bnx2x *bp = params->bp;
- u8 phy_index, port = params->port;
+ u8 phy_index, port = params->port, clear_latch_ind = 0;
DP(NETIF_MSG_LINK, "Resetting the link of port %d\n", port);
/* disable attentions */
vars->link_status = 0;
@@ -6976,9 +6976,18 @@ u8 bnx2x_link_reset(struct link_params *params, struct link_vars *vars,
params->phy[phy_index].link_reset(
¶ms->phy[phy_index],
params);
+ if (params->phy[phy_index].flags &
+ FLAGS_REARM_LATCH_SIGNAL)
+ clear_latch_ind = 1;
}
}
+ if (clear_latch_ind) {
+ /* Clear latching indication */
+ bnx2x_rearm_latch_signal(bp, port, 0);
+ bnx2x_bits_dis(bp, NIG_REG_LATCH_BC_0 + port*4,
+ 1 << NIG_LATCH_BC_ENABLE_MI_INT);
+ }
if (params->phy[INT_PHY].link_reset)
params->phy[INT_PHY].link_reset(
¶ms->phy[INT_PHY], params);
--
1.7.1
^ permalink raw reply related
* [PATCH net-2.6 8/8] bnx2x: Update version number
From: Yaniv Rosner @ 2010-11-01 15:32 UTC (permalink / raw)
To: davem; +Cc: netdev, eilong
Update bnx2x version number.
Signed-off-by: Yaniv Rosner <yanivr@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
index 9eea225..863e73a 100644
--- a/drivers/net/bnx2x/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -20,8 +20,8 @@
* (you will need to reboot afterwards) */
/* #define BNX2X_STOP_ON_ERROR */
-#define DRV_MODULE_VERSION "1.60.00-3"
-#define DRV_MODULE_RELDATE "2010/10/19"
+#define DRV_MODULE_VERSION "1.60.00-4"
+#define DRV_MODULE_RELDATE "2010/11/01"
#define BNX2X_BC_VER 0x040200
#define BNX2X_MULTI_QUEUE
--
1.7.1
^ permalink raw reply related
* [PATCH net-2.6 5/8] bnx2x: Fix resetting BCM8726 PHY during common init
From: Yaniv Rosner @ 2010-11-01 15:32 UTC (permalink / raw)
To: davem; +Cc: netdev, eilong
On BCM8726 based designs, the ports are swapped, hence the reset needs to be asserted through port0 and not port1.
Signed-off-by: Yaniv Rosner <yanivr@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_link.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_link.c b/drivers/net/bnx2x/bnx2x_link.c
index fdd7e03..488e251 100644
--- a/drivers/net/bnx2x/bnx2x_link.c
+++ b/drivers/net/bnx2x/bnx2x_link.c
@@ -7152,7 +7152,7 @@ static u8 bnx2x_8726_common_init_phy(struct bnx2x *bp,
(1<<(MISC_REGISTERS_GPIO_3 + MISC_REGISTERS_GPIO_PORT_SHIFT)));
REG_WR(bp, MISC_REG_GPIO_EVENT_EN, val);
- bnx2x_ext_phy_hw_reset(bp, 1);
+ bnx2x_ext_phy_hw_reset(bp, 0);
msleep(5);
for (port = 0; port < PORT_MAX; port++) {
u32 shmem_base, shmem2_base;
--
1.7.1
^ permalink raw reply related
* [PATCH net-2.6 7/8] bnx2x: Reset 8073 phy during common init
From: Yaniv Rosner @ 2010-11-01 15:32 UTC (permalink / raw)
To: davem; +Cc: netdev, eilong
Resetting 8073 during common init is required on boards in which the 8073 reset pin is not asserted by default.
Signed-off-by: Yaniv Rosner <yanivr@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_link.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_link.c b/drivers/net/bnx2x/bnx2x_link.c
index d076b91..5809196 100644
--- a/drivers/net/bnx2x/bnx2x_link.c
+++ b/drivers/net/bnx2x/bnx2x_link.c
@@ -7024,6 +7024,7 @@ static u8 bnx2x_8073_common_init_phy(struct bnx2x *bp,
s8 port;
s8 port_of_path = 0;
+ bnx2x_ext_phy_hw_reset(bp, 0);
/* PART1 - Reset both phys */
for (port = PORT_MAX - 1; port >= PORT_0; port--) {
u32 shmem_base, shmem2_base;
--
1.7.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox