* Re: [RFC PATCH bridge 4/5] bridge: Add private ioctls to configure vlans on bridge ports
From: Paul E. McKenney @ 2012-08-24 17:56 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <1345750195-31598-5-git-send-email-vyasevic@redhat.com>
On Thu, Aug 23, 2012 at 03:29:54PM -0400, Vlad Yasevich wrote:
> Add a private ioctl to add and remove vlan configuration on bridge port.
>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
One question below...
> ---
> include/linux/if_bridge.h | 2 +
> net/bridge/br_if.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
> net/bridge/br_ioctl.c | 31 ++++++++++++++++++++
> net/bridge/br_private.h | 2 +
> 4 files changed, 104 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index 288ff10..ab750dd 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -42,6 +42,8 @@
> #define BRCTL_SET_PORT_PRIORITY 16
> #define BRCTL_SET_PATH_COST 17
> #define BRCTL_GET_FDB_ENTRIES 18
> +#define BRCTL_ADD_VLAN 19
> +#define BRCTL_DEL_VLAN 20
>
> #define BR_STATE_DISABLED 0
> #define BR_STATE_LISTENING 1
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index e1144e1..90c1038 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -23,6 +23,7 @@
> #include <linux/if_ether.h>
> #include <linux/slab.h>
> #include <net/sock.h>
> +#include <linux/if_vlan.h>
>
> #include "br_private.h"
>
> @@ -441,6 +442,74 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
> return 0;
> }
>
> +/* Called with RTNL */
> +int br_set_port_vlan(struct net_bridge_port *p, unsigned long vlan)
> +{
> + unsigned long table_size = (VLAN_N_VID/sizeof(unsigned long)) + 1;
> + unsigned long *vid_map = NULL;
> + __u16 vid = (__u16) vlan + 1;
> +
> + /* We are under lock so we can check this without rcu.
> + * The vlan map is indexed by vid+1. This way we can store
> + * vid 0 (untagged) into the map as well.
> + */
> + if (!p->vlan_map) {
> + vid_map = kzalloc(table_size, GFP_KERNEL);
> + if (!vid_map)
> + return -ENOMEM;
> +
> + set_bit(vid, vid_map);
> + rcu_assign_pointer(p->vlan_map, vid_map);
> +
> + } else {
> + /* Map is already allocated */
> + set_bit(vid, p->vlan_map);
> + }
> +
> + return 0;
> +}
> +
> +
> +/* Called with RTNL */
> +int br_del_port_vlan(struct net_bridge_port *p, unsigned long vlan)
> +{
> + unsigned long first_bit;
> + unsigned long next_bit;
> + __u16 vid = (__u16) vlan+1;
> + unsigned long tbl_len = VLAN_N_VID+1;
> +
> + if (!p->vlan_map)
> + return -EINVAL;
> +
> + if (!test_bit(vlan, p->vlan_map))
> + return -EINVAL;
> +
> + /* Check to see if any other vlans are in this table. If this
> + * is the last vlan, delete the whole table. If this is not the
> + * last vlan, just clear the bit.
> + */
> + first_bit = find_first_bit(p->vlan_map, tbl_len);
> + next_bit = find_next_bit(p->vlan_map, tbl_len, (tbl_len - vid));
> +
> + if ((__u16)first_bit != vid || (__u16)next_bit < tbl_len) {
> + /* There are other vlans still configured. We can simply
> + * clear our bit and be safe.
> + */
> + clear_bit(vid, p->vlan_map);
> + } else {
> + /* This is the last vlan we are removing. Replace the
> + * map with a NULL pointer and free the old map
> + */
> + unsigned long *map = rcu_dereference(p->vlan_map);
> +
> + rcu_assign_pointer(p->vlan_map, NULL);
> + synchronize_net();
> + kfree(map);
> + }
> +
> + return 0;
> +}
> +
> void __net_exit br_net_exit(struct net *net)
> {
> struct net_device *dev;
> diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
> index 7222fe1..3a5b1f9 100644
> --- a/net/bridge/br_ioctl.c
> +++ b/net/bridge/br_ioctl.c
> @@ -289,6 +289,37 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> case BRCTL_GET_FDB_ENTRIES:
> return get_fdb_entries(br, (void __user *)args[1],
> args[2], args[3]);
> + case BRCTL_ADD_VLAN:
> + {
> + struct net_bridge_port *p;
> +
> + if (!capable(CAP_NET_ADMIN))
> + return -EPERM;
> +
> + rcu_read_lock();
> + if ((p = br_get_port(br, args[1])) == NULL) {
> + rcu_read_unlock();
> + return -EINVAL;
> + }
> + rcu_read_unlock();
Why is it safe to pass "p" out of the RCU read-side critical section?
I don't see that br_get_port() does anything to make this safe, at least
not in v3.5.
> + return br_set_port_vlan(p, args[2]);
> + }
> +
> + case BRCTL_DEL_VLAN:
> + {
> + struct net_bridge_port *p;
> +
> + if (!capable(CAP_NET_ADMIN))
> + return -EPERM;
> +
> + rcu_read_lock();
> + if ((p = br_get_port(br, args[1])) == NULL) {
> + rcu_read_unlock();
> + return -EINVAL;
> + }
> + rcu_read_unlock();
> + br_set_port_vlan(p, args[2]);
> + }
> }
>
> return -EOPNOTSUPP;
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index b6c56ab..5639c1c 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -402,6 +402,8 @@ extern int br_del_if(struct net_bridge *br,
> extern int br_min_mtu(const struct net_bridge *br);
> extern netdev_features_t br_features_recompute(struct net_bridge *br,
> netdev_features_t features);
> +extern int br_set_port_vlan(struct net_bridge_port *p, unsigned long vid);
> +extern int br_del_port_vlan(struct net_bridge_port *p, unsigned long vid);
>
> /* br_input.c */
> extern int br_handle_frame_finish(struct sk_buff *skb);
> --
> 1.7.7.6
>
> --
> 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: [net-next 04/13] e1000e: cleanup checkpatch PREFER_PR_LEVEL warning
From: Joe Perches @ 2012-08-24 18:02 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, Bruce Allan, netdev, gospo, sassmann
In-Reply-To: <1345730504.16471.3.camel@joe2Laptop>
On Thu, 2012-08-23 at 07:01 -0700, Joe Perches wrote:
> On Thu, 2012-08-23 at 02:56 -0700, Jeff Kirsher wrote:
> > From: Bruce Allan <bruce.w.allan@intel.com>
> >
> > checkpatch warning: Prefer pr_info(... to printk(KERN_INFO, ...
> []
> > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> []
> > @@ -4330,9 +4330,8 @@ static void e1000_print_link_info(struct e1000_adapter *adapter)
> > u32 ctrl = er32(CTRL);
> >
> > /* Link status message must follow this format for user tools */
> > - printk(KERN_INFO "e1000e: %s NIC Link is Up %d Mbps %s Duplex, Flow Control: %s\n",
> > - adapter->netdev->name,
> > - adapter->link_speed,
> > + pr_info("e1000e: %s NIC Link is Up %d Mbps %s Duplex, Flow Control: %s\n",
> > + adapter->netdev->name, adapter->link_speed,
> > adapter->link_duplex == FULL_DUPLEX ? "Full" : "Half",
> > (ctrl & E1000_CTRL_TFCE) && (ctrl & E1000_CTRL_RFCE) ? "Rx/Tx" :
> > (ctrl & E1000_CTRL_RFCE) ? "Rx" :
>
> I think these conversions are not a good idea.
>
> When you have a specific message format that must be
> followed, use printk.
>
> pr_<level> may at some point in the near future use
> #define pr_fmt(fmt) KBUiLD_MODNAME ": " fmt
> as a global default equivalent.
Hey Jeff.
The comment above this change (and the other) reads
/* Link status message must follow this format for user tools */
This file already uses #define pr_fmt(fmt) KBUILD_MODNAME...
With this patch, the output form changes to use 2 prefixes.
Is that really desired? Probably not.
If the comments are old and don't apply any more, they
should be removed.
^ permalink raw reply
* Re: [RFC PATCH bridge 4/5] bridge: Add private ioctls to configure vlans on bridge ports
From: Stephen Hemminger @ 2012-08-24 18:11 UTC (permalink / raw)
To: paulmck; +Cc: Vlad Yasevich, netdev
In-Reply-To: <20120824175616.GL2472@linux.vnet.ibm.com>
On Fri, 24 Aug 2012 10:56:16 -0700
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> > + return -EPERM;
> > +
> > + rcu_read_lock();
> > + if ((p = br_get_port(br, args[1])) == NULL) {
> > + rcu_read_unlock();
> > + return -EINVAL;
> > + }
> > + rcu_read_unlock();
>
> Why is it safe to pass "p" out of the RCU read-side critical section?
> I don't see that br_get_port() does anything to make this safe, at least
> not in v3.5.
It would be accidentally protected by rtnl?
^ permalink raw reply
* Re: Gianfar ethernet drop package
From: Eric Dumazet @ 2012-08-24 18:17 UTC (permalink / raw)
To: Aníbal Almeida Pinto; +Cc: netdev
In-Reply-To: <5037A86E.2000004@efacec.com>
On Fri, 2012-08-24 at 17:14 +0100, Aníbal Almeida Pinto wrote:
> Hi,
>
> I have an board with a ppc with a 8379, kernel 3.0.39 and with a Gianfar
> Ethernet Controller Version 1.2.
>
> When connect the board to office network the number of dropped packets
> reported by ifconfig continues to increase.
>
> Following the test made at [1] I use tcpdump and when running it the
> number of drop packages don't have increased.
>
> When executing ethtool eth0 -S it return a rx-dropped-by-kernel != 0.
> And the rest of drop packages
>
> Is possible to get info why the packages are dropped?
You receive frames for unknown protocols. So they are dropped.
When running tcpdump, your tcpdump get them, they are not dropped.
So just parse your tcpdump output ?
^ permalink raw reply
* Re: [RFC PATCH bridge 4/5] bridge: Add private ioctls to configure vlans on bridge ports
From: Vlad Yasevich @ 2012-08-24 18:19 UTC (permalink / raw)
To: paulmck; +Cc: netdev
In-Reply-To: <20120824175616.GL2472@linux.vnet.ibm.com>
On 08/24/2012 01:56 PM, Paul E. McKenney wrote:
> On Thu, Aug 23, 2012 at 03:29:54PM -0400, Vlad Yasevich wrote:
>> Add a private ioctl to add and remove vlan configuration on bridge port.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>
> One question below...
>
> index 7222fe1..3a5b1f9 100644
>> --- a/net/bridge/br_ioctl.c
>> +++ b/net/bridge/br_ioctl.c
>> @@ -289,6 +289,37 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
>> case BRCTL_GET_FDB_ENTRIES:
>> return get_fdb_entries(br, (void __user *)args[1],
>> args[2], args[3]);
>> + case BRCTL_ADD_VLAN:
>> + {
>> + struct net_bridge_port *p;
>> +
>> + if (!capable(CAP_NET_ADMIN))
>> + return -EPERM;
>> +
>> + rcu_read_lock();
>> + if ((p = br_get_port(br, args[1])) == NULL) {
>> + rcu_read_unlock();
>> + return -EINVAL;
>> + }
>> + rcu_read_unlock();
>
> Why is it safe to pass "p" out of the RCU read-side critical section?
> I don't see that br_get_port() does anything to make this safe, at least
> not in v3.5.
>
You right. It not really safe. As Stephen pointed out, it is
accidentally protected by rtnl, so it will not go away. However, that's
not a good excuse and I've already fixed it in my tree.
thanks
-vlad
>> + return br_set_port_vlan(p, args[2]);
>> + }
>> +
>> + case BRCTL_DEL_VLAN:
>> + {
>> + struct net_bridge_port *p;
>> +
>> + if (!capable(CAP_NET_ADMIN))
>> + return -EPERM;
>> +
>> + rcu_read_lock();
>> + if ((p = br_get_port(br, args[1])) == NULL) {
>> + rcu_read_unlock();
>> + return -EINVAL;
>> + }
>> + rcu_read_unlock();
>> + br_set_port_vlan(p, args[2]);
>> + }
>> }
>>
>> return -EOPNOTSUPP;
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index b6c56ab..5639c1c 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -402,6 +402,8 @@ extern int br_del_if(struct net_bridge *br,
>> extern int br_min_mtu(const struct net_bridge *br);
>> extern netdev_features_t br_features_recompute(struct net_bridge *br,
>> netdev_features_t features);
>> +extern int br_set_port_vlan(struct net_bridge_port *p, unsigned long vid);
>> +extern int br_del_port_vlan(struct net_bridge_port *p, unsigned long vid);
>>
>> /* br_input.c */
>> extern int br_handle_frame_finish(struct sk_buff *skb);
>> --
>> 1.7.7.6
>>
>> --
>> 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
>>
>
> --
> 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 5/8] csiostor: Chelsio FCoE offload driver submission (sources part 5).
From: Joe Perches @ 2012-08-24 18:27 UTC (permalink / raw)
To: Naresh Kumar Inna
Cc: Nicholas A. Bellinger, JBottomley@parallels.com,
linux-scsi@vger.kernel.org, Dimitrios Michailidis,
netdev@vger.kernel.org, Chethan Seshadri
In-Reply-To: <5037BC94.1050201@chelsio.com>
On Fri, 2012-08-24 at 23:10 +0530, Naresh Kumar Inna wrote:
> Is there a TRUE/FALSE define in a standard header file?
include/linux/stddef.h:
enum {
false = 0,
true = 1
};
> I see a lot of
> kernel code defining their own TRUE/FALSE.
Those should be changed eventually.
^ permalink raw reply
* Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Stephen Warren @ 2012-08-24 18:29 UTC (permalink / raw)
To: Timur Tabi
Cc: Fleming Andy-AFLEMING, David Miller, ddaney.cavm@gmail.com,
netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
Scott Wood
In-Reply-To: <5037AB67.6030106@freescale.com>
On 08/24/2012 10:27 AM, Timur Tabi wrote:
> Stephen Warren wrote:
>>>>>> Then, that'd have to be <9 1>;
>>>>
>>>> Actually, I had #size-cells = <0>.
>
>> I think that if you have #size-cells=<0>, then you'll see the following
>> error message when attempting to translate the address into the parent's
>> address space:
>>
>> prom_parse: Bad cell count for /board-control@3,0/mdio-mux-emi2
>
> It doesn't appear to be working. Here's my tree:
>
> / {
> model = "fsl,P5020DS";
> compatible = "fsl,P5020DS";
> #address-cells = <2>;
> #size-cells = <2>;
> interrupt-parent = <&mpic>;
>
> lbc: localbus@ffe124000 {
> reg = <0xf 0xfe124000 0 0x1000>;
> ranges = <0 0 0xf 0xe8000000 0x08000000
> 2 0 0xf 0xffa00000 0x00040000
> 3 0 0xf 0xffdf0000 0x00008000>;
>
> fpga: board-control@3,0 {
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
> reg = <3 0 0x30>;
>
> mdio-mux-emi1 {
> compatible = "mdio-mux-mmioreg";
> mdio-parent-bus = <&mdio0>;
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <9 1>; // BRDCFG1
> mux-mask = <0x78>; // EMI1
>
> That means that the physical address that I need is fffdf0009. However,
> when I call of_address_to_resource(), the returned address I get is fe8000009.
>
> So it's not picking up the "3" in the 'reg' property of the
> board-control@3,0 node. What am I missing? Do I need a 'ranges' property
> in the board-control@3,0 node?
Yes.
When translating the child node's reg property into the parent's address
space, the parent's reg property shouldn't even be used at all; all the
mapping is done through the ranges property.
I thought the code error-checked for a missing ranges property, but I
guess not...
^ permalink raw reply
* Re: [PATCH 6/8] csiostor: Chelsio FCoE offload driver submission (headers part 1).
From: Naresh Kumar Inna @ 2012-08-24 18:36 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: JBottomley@parallels.com, linux-scsi@vger.kernel.org,
Dimitrios Michailidis, netdev@vger.kernel.org, Chethan Seshadri
In-Reply-To: <1345751929.10190.83.camel@haakon2.linux-iscsi.org>
On 8/24/2012 1:28 AM, Nicholas A. Bellinger wrote:
> On Fri, 2012-08-24 at 03:57 +0530, Naresh Kumar Inna wrote:
>> This patch contains the first set of the header files for csiostor driver.
>>
>> Signed-off-by: Naresh Kumar Inna <naresh@chelsio.com>
>> ---
>> drivers/scsi/csiostor/csio_defs.h | 143 ++++++
>> drivers/scsi/csiostor/csio_fcoe_proto.h | 843 +++++++++++++++++++++++++++++++
>> drivers/scsi/csiostor/csio_hw.h | 668 ++++++++++++++++++++++++
>> drivers/scsi/csiostor/csio_init.h | 158 ++++++
>> 4 files changed, 1812 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/scsi/csiostor/csio_defs.h
>> create mode 100644 drivers/scsi/csiostor/csio_fcoe_proto.h
>> create mode 100644 drivers/scsi/csiostor/csio_hw.h
>> create mode 100644 drivers/scsi/csiostor/csio_init.h
>>
>
> Hi Naresh,
>
> Just commenting on csio_defs.h bits here... As Robert mentioned, you'll
> need to convert the driver to use (or add to) upstream protocol
> definitions and drop the csio_fcoe_proto.h bits..
>
Hi Nicholas,
I would like take up the discussion of the protocol header file in that
email thread. Please find the rest of my replies inline.
Thanks for reviewing,
Naresh.
>> diff --git a/drivers/scsi/csiostor/csio_defs.h b/drivers/scsi/csiostor/csio_defs.h
>> new file mode 100644
>> index 0000000..4f1c713
>> --- /dev/null
>> +++ b/drivers/scsi/csiostor/csio_defs.h
>> @@ -0,0 +1,143 @@
>> +/*
>> + * This file is part of the Chelsio FCoE driver for Linux.
>> + *
>> + * Copyright (c) 2008-2012 Chelsio Communications, Inc. All rights reserved.
>> + *
>> + * This software is available to you under a choice of one of two
>> + * licenses. You may choose to be licensed under the terms of the GNU
>> + * General Public License (GPL) Version 2, available from the file
>> + * COPYING in the main directory of this source tree, or the
>> + * OpenIB.org BSD license below:
>> + *
>> + * Redistribution and use in source and binary forms, with or
>> + * without modification, are permitted provided that the following
>> + * conditions are met:
>> + *
>> + * - Redistributions of source code must retain the above
>> + * copyright notice, this list of conditions and the following
>> + * disclaimer.
>> + *
>> + * - Redistributions in binary form must reproduce the above
>> + * copyright notice, this list of conditions and the following
>> + * disclaimer in the documentation and/or other materials
>> + * provided with the distribution.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
>> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>> + * SOFTWARE.
>> + */
>> +
>> +#ifndef __CSIO_DEFS_H__
>> +#define __CSIO_DEFS_H__
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/timer.h>
>> +#include <linux/list.h>
>> +#include <linux/bug.h>
>> +#include <linux/pci.h>
>> +#include <linux/jiffies.h>
>> +
>> +/* Function returns */
>> +enum csio_retval {
>> + CSIO_SUCCESS = 0,
>> + CSIO_INVAL = 1,
>> + CSIO_BUSY = 2,
>> + CSIO_NOSUPP = 3,
>> + CSIO_TIMEOUT = 4,
>> + CSIO_NOMEM = 5,
>> + CSIO_NOPERM = 6,
>> + CSIO_RETRY = 7,
>> + CSIO_EPROTO = 8,
>> + CSIO_EIO = 9,
>> + CSIO_CANCELLED = 10,
>> +};
>> +
>
> Please don't assign macros for errno's, and give them positive values.
>
Although some of these return values appear to be mapped to errno
values, there are others that do not have an errno equivalent (example
CSIO_CANCELLED). We may have future needs to have more driver/protocol
specific return values as well. What do you suggest?
>> +#define csio_retval_t enum csio_retval
>
> Please get rid of this csio_retval_t nonsense.
I can get rid of the typedef and use enum csio_retval instead.
>
>> +
>> +enum {
>> + CSIO_FALSE = 0,
>> + CSIO_TRUE = 1,
>> +};
>> +
>
> Same here, please use normal Boolean macros
>
>> +#define CSIO_ROUNDUP(__v, __r) (((__v) + (__r) - 1) / (__r))
>> +#define CSIO_INVALID_IDX 0xFFFFFFFF
>> +#define csio_inc_stats(elem, val) ((elem)->stats.val++)
>> +#define csio_dec_stats(elem, val) ((elem)->stats.val--)
>
> No reason for either of this stats inc+dec macros. Please drop them.
I will get rid of them.
>
>> +#define csio_valid_wwn(__n) ((*__n >> 4) == 0x5 ? CSIO_TRUE : \
>> + CSIO_FALSE)
>> +#define CSIO_WORD_TO_BYTE 4
>> +
>> +static inline int
>> +csio_list_deleted(struct list_head *list)
>> +{
>> + return ((list->next == list) && (list->prev == list));
>> +}
>> +
>> +#define csio_list_next(elem) (((struct list_head *)(elem))->next)
>> +#define csio_list_prev(elem) (((struct list_head *)(elem))->prev)
>> +
>> +#define csio_deq_from_head(head, elem) \
>> +do { \
>> + if (!list_empty(head)) { \
>> + *((struct list_head **)(elem)) = csio_list_next((head)); \
>> + csio_list_next((head)) = \
>> + csio_list_next(csio_list_next((head))); \
>> + csio_list_prev(csio_list_next((head))) = (head); \
>> + INIT_LIST_HEAD(*((struct list_head **)(elem))); \
>> + } else \
>> + *((struct list_head **)(elem)) = (struct list_head *)NULL;\
>> +} while (0)
>> +
>
> This code is confusing as hell.. Why can't you just use normal list.h
> macros for this..?
I have not found an equivalent function in list.h that does the above
and the following macro. Could you please point me to it? I have seen a
couple of other drivers define their own macros to achieve what this
macro does, hence I assumed there isnt a list.h macro that does this.
>> +#define csio_deq_from_tail(head, elem) \
>> +do { \
>> + if (!list_empty(head)) { \
>> + *((struct list_head **)(elem)) = csio_list_prev((head)); \
>> + csio_list_prev((head)) = \
>> + csio_list_prev(csio_list_prev((head))); \
>> + csio_list_next(csio_list_prev((head))) = (head); \
>> + INIT_LIST_HEAD(*((struct list_head **)(elem))); \
>> + } else \
>> + *((struct list_head **)(elem)) = (struct list_head *)NULL;\
>> +} while (0)
>> +
>
> Same here.. Please don't use macros like this.
>
>> +/* State machine */
>> +typedef void (*csio_sm_state_t)(void *, uint32_t);
>> +
>> +struct csio_sm {
>> + struct list_head sm_list;
>> + csio_sm_state_t sm_state;
>> +};
>> +
>> +#define csio_init_state(__smp, __state) \
>> + (((struct csio_sm *)(__smp))->sm_state = (csio_sm_state_t)(__state))
>> +
>> +#define csio_set_state(__smp, __state) \
>> + (((struct csio_sm *)(__smp))->sm_state = (csio_sm_state_t)(__state))
>> +
>> +
>> +#define csio_post_event(__smp, __evt) \
>> + (((struct csio_sm *)(__smp))->sm_state((__smp), (uint32_t)(__evt)))
>> +
>> +#define csio_get_state(__smp) (((struct csio_sm *)(__smp))->sm_state)
>> +
>> +#define csio_match_state(__smp, __state) \
>> + (csio_get_state((__smp)) == (csio_sm_state_t)(__state))
>> +
>
> Why does any of the sm_state stuff need to be in macros..? Please
> inline all of this code.
I will change them to static inline.
>
>> +#define CSIO_ASSERT(cond) \
>> +do { \
>> + if (unlikely(!((cond)))) \
>> + BUG(); \
>> +} while (0)
>> +
>> +#ifdef __CSIO_DEBUG__
>> +#define CSIO_DB_ASSERT(__c) CSIO_ASSERT((__c))
>> +#else
>> +#define CSIO_DB_ASSERT(__c)
>> +#endif
>> +
>> +#endif /* ifndef __CSIO_DEFS_H__ */
>
>
^ permalink raw reply
* Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Timur Tabi @ 2012-08-24 18:36 UTC (permalink / raw)
To: Stephen Warren
Cc: Fleming Andy-AFLEMING, David Miller, ddaney.cavm@gmail.com,
netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
Scott Wood
In-Reply-To: <5037C815.4020906@wwwdotorg.org>
Stephen Warren wrote:
> When translating the child node's reg property into the parent's address
> space, the parent's reg property shouldn't even be used at all; all the
> mapping is done through the ranges property.
>
> I thought the code error-checked for a missing ranges property, but I
> guess not...
I don't think 'ranges' is always necessary, because sometimes the child
nodes have a different address space that's not mapped to the parent. For
instance, I2C devices have addresses that are not mapped to the I2C
controller itself.
Anyway, thanks to Scott for helping me figure this out. I was missing a
ranges property:
fpga: board-control@3,0 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
reg = <3 0 0x30>;
ranges = <0 3 0 0x30>;
This maps the child address of 0 to the parent address of 3 0. It seems
obvious now, but it was driving me crazy. We've never put child devices
under our FPGA nodes, so there was no prior use case of a 'ranges'
property in any of the localbus devices that I could learn from. Plus,
this is the first time we're probing directly on a child of a localbus device.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Scott Wood @ 2012-08-24 18:43 UTC (permalink / raw)
To: Timur Tabi
Cc: Stephen Warren, Fleming Andy-AFLEMING, David Miller,
ddaney.cavm@gmail.com, netdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org
In-Reply-To: <5037C9AF.3060903@freescale.com>
On 08/24/2012 01:36 PM, Timur Tabi wrote:
> Stephen Warren wrote:
>> When translating the child node's reg property into the parent's address
>> space, the parent's reg property shouldn't even be used at all; all the
>> mapping is done through the ranges property.
>>
>> I thought the code error-checked for a missing ranges property, but I
>> guess not...
>
> I don't think 'ranges' is always necessary, because sometimes the child
> nodes have a different address space that's not mapped to the parent. For
> instance, I2C devices have addresses that are not mapped to the I2C
> controller itself.
>
> Anyway, thanks to Scott for helping me figure this out. I was missing a
> ranges property:
>
> fpga: board-control@3,0 {
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
> reg = <3 0 0x30>;
> ranges = <0 3 0 0x30>;
>
> This maps the child address of 0 to the parent address of 3 0. It seems
> obvious now, but it was driving me crazy. We've never put child devices
> under our FPGA nodes, so there was no prior use case of a 'ranges'
> property in any of the localbus devices that I could learn from. Plus,
> this is the first time we're probing directly on a child of a localbus device.
There's ep8248e.dts, not that I'd have expected you to look there. :-)
-Scott
^ permalink raw reply
* Re: [PATCH 6/8] csiostor: Chelsio FCoE offload driver submission (headers part 1).
From: Naresh Kumar Inna @ 2012-08-24 18:45 UTC (permalink / raw)
To: Love, Robert W
Cc: JBottomley@parallels.com, linux-scsi@vger.kernel.org,
Dimitrios Michailidis, netdev@vger.kernel.org, Chethan Seshadri
In-Reply-To: <50367309.3080306@intel.com>
On 8/23/2012 11:45 PM, Love, Robert W wrote:
> On 8/23/2012 3:27 PM, Naresh Kumar Inna wrote:
>> This patch contains the first set of the header files for csiostor driver.
>>
>> Signed-off-by: Naresh Kumar Inna <naresh@chelsio.com>
>> ---
>> drivers/scsi/csiostor/csio_defs.h | 143 ++++++
>> drivers/scsi/csiostor/csio_fcoe_proto.h | 843 +++++++++++++++++++++++++++++++
>
> I think most of these protocol definitions can be found in
> include/scsi/fc/ and possibly incluse/scsi/scsi_transport_fc.h. I think
> any protocol stuff missing should be added to the common header files.
>
> Thanks, //Rob--
Hi Rob,
Thanks for reviewing this patch. I will look into how best I can re-use
the headers in include/scsi/fc and get back if I have questions.
Regards,
Naresh.
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" 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 net-next 0/3] team: couple of fixes
From: Jiri Pirko @ 2012-08-24 18:53 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, brouer@redhat.com, ogerlitz@mellanox.com
In-Reply-To: <20120824.134705.178022240623528140.davem@davemloft.net>
24. 8. 2012 v 19:47, David Miller <davem@davemloft.net>:
> From: Jiri Pirko <jiri@resnulli.us>
> Date: Thu, 23 Aug 2012 15:26:50 +0200
>
>> + vlan helper
>>
>> Jiri Pirko (3):
>> teamd: don't print warn message on -ESRCH during event send
>> vlan: add helper which can be called to se it device is used by vlan
>> team: do not allow to add VLAN challenged port when vlan is used
>
> Applied with typos corrected :-)
I owe you a beer :)
> --
> 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] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Stephen Warren @ 2012-08-24 18:56 UTC (permalink / raw)
To: Timur Tabi
Cc: Fleming Andy-AFLEMING, David Miller, ddaney.cavm@gmail.com,
netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
Scott Wood
In-Reply-To: <5037C9AF.3060903@freescale.com>
On 08/24/2012 12:36 PM, Timur Tabi wrote:
> Stephen Warren wrote:
>> When translating the child node's reg property into the parent's address
>> space, the parent's reg property shouldn't even be used at all; all the
>> mapping is done through the ranges property.
>>
>> I thought the code error-checked for a missing ranges property, but I
>> guess not...
>
> I don't think 'ranges' is always necessary, because sometimes the child
> nodes have a different address space that's not mapped to the parent. For
> instance, I2C devices have addresses that are not mapped to the I2C
> controller itself.
In the I2C case, the address spaces are disjoint, so there's never any
mapping between them, so there's no need for ranges.
Any time the child address space is intended to be part of the parent's
address space, I believe ranges is supposed to be specified, perhaps
even mandatory, even if the translation is 1:1.
> Anyway, thanks to Scott for helping me figure this out. I was missing a
> ranges property:
>
> fpga: board-control@3,0 {
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
> reg = <3 0 0x30>;
> ranges = <0 3 0 0x30>;
>
> This maps the child address of 0 to the parent address of 3 0.
Yes, that looks reasonable.
^ permalink raw reply
* Re: [PATCH 0/8] csiostor: Chelsio FCoE offload driver submission
From: Naresh Kumar Inna @ 2012-08-24 19:04 UTC (permalink / raw)
To: David Miller
Cc: JBottomley@parallels.com, linux-scsi@vger.kernel.org,
Dimitrios Michailidis, netdev@vger.kernel.org, Chethan Seshadri
In-Reply-To: <20120824.130406.1059160453285824849.davem@davemloft.net>
On 8/24/2012 10:34 PM, David Miller wrote:
> From: Naresh Kumar Inna <naresh@chelsio.com>
> Date: Fri, 24 Aug 2012 03:57:45 +0530
>
>> This is the initial submission of the Chelsio FCoE offload driver (csiostor)
>> to the upstream kernel. This driver currently supports FCoE offload
>> functionality over Chelsio T4-based 10Gb Converged Network Adapters.
>>
>> The following patches contain the driver sources for csiostor driver and
>> updates to firmware/hardware header files shared between csiostor and
>> cxgb4 (Chelsio T4-based NIC driver). The csiostor driver is dependent on these
>> header updates. These patches have been generated against scsi 'misc' branch.
>>
>> csiostor is a low level SCSI driver that interfaces with PCI, SCSI midlayer and
>> FC transport subsystems. This driver claims the FCoE PCIe function on the
>> Chelsio Converged Network Adapter. It relies on firmware events for slow path
>> operations like discovery, thereby offloading session management. The driver
>> programs firmware via Work Request interfaces for fast path I/O offload
>> features.
>
> You are going to have to get rid of these module parameters.
>
> That have to do with things that are in no way specific to your device,
> and therefore should be configured using generic kernel facilities.
>
> Using driver specific module parameters results in a poor user
> experience, because in order to make a configuration change the user
> has to know exactly what kind of device and driver is underneath,
> and then learn what the unique method is to make that configuration
> change.
>
> If you use a generic facility, the user only needs to learn one way to
> make a configuration change, regardless of device type and driver.
>
Hi Dave,
Thanks for reviewing. Is your comment with respect to any particular
module parameter[s] in this driver or all of them?
Regards,
Naresh.
^ permalink raw reply
* Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: David Miller @ 2012-08-24 19:07 UTC (permalink / raw)
To: swarren; +Cc: timur, afleming, ddaney.cavm, netdev, devicetree-discuss,
scottwood
In-Reply-To: <5037CE45.4010309@wwwdotorg.org>
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Fri, 24 Aug 2012 12:56:05 -0600
> In the I2C case, the address spaces are disjoint, so there's never any
> mapping between them, so there's no need for ranges.
>
> Any time the child address space is intended to be part of the parent's
> address space, I believe ranges is supposed to be specified, perhaps
> even mandatory, even if the translation is 1:1.
Regardless, you really can't just generically translate ranges
in some universal way and expect it to work in all cases.
You need bus specific drivers to deal with various special
cases.
See the *_map() methods implemented in:
arch/sparc/kernel/of_device_64.c
for example.
^ permalink raw reply
* Re: [PATCH 5/8] csiostor: Chelsio FCoE offload driver submission (sources part 5).
From: Naresh Kumar Inna @ 2012-08-24 19:07 UTC (permalink / raw)
To: Joe Perches
Cc: Nicholas A. Bellinger, JBottomley@parallels.com,
linux-scsi@vger.kernel.org, Dimitrios Michailidis,
netdev@vger.kernel.org, Chethan Seshadri
In-Reply-To: <1345832854.32359.11.camel@joe2Laptop>
On 8/24/2012 11:57 PM, Joe Perches wrote:
> On Fri, 2012-08-24 at 23:10 +0530, Naresh Kumar Inna wrote:
>> Is there a TRUE/FALSE define in a standard header file?
>
> include/linux/stddef.h:
> enum {
> false = 0,
> true = 1
> };
>
>> I see a lot of
>> kernel code defining their own TRUE/FALSE.
>
> Those should be changed eventually.
>
Thanks! I will switch over to these macros.
^ permalink raw reply
* Re: [PATCH 0/8] csiostor: Chelsio FCoE offload driver submission
From: David Miller @ 2012-08-24 19:10 UTC (permalink / raw)
To: naresh; +Cc: JBottomley, linux-scsi, dm, netdev, chethan
In-Reply-To: <5037D043.7000302@chelsio.com>
From: Naresh Kumar Inna <naresh@chelsio.com>
Date: Sat, 25 Aug 2012 00:34:35 +0530
> Thanks for reviewing. Is your comment with respect to any particular
> module parameter[s] in this driver or all of them?
All of them.
^ permalink raw reply
* [PATCH] [v3] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Timur Tabi @ 2012-08-24 19:10 UTC (permalink / raw)
To: Andy Fleming, David Miller, swarren-3lzwWm7+Weoh9ZMKESR00Q,
ddaney.cavm-Re5JQEeQqe8AvxtiuMwx3w,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
netdev-u79uwXL29TY76Z2rM5mHXA
Add support for an MDIO bus multiplexer controlled by a simple memory-mapped
device, like an FPGA. The device must be memory-mapped and contain only
8-bit registers (which keeps things simple).
Tested on a Freescale P5020DS board which uses the "PIXIS" FPGA attached
to the localbus.
Signed-off-by: Timur Tabi <timur-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
.../devicetree/bindings/net/mdio-mux-mmioreg.txt | 77 +++++++++
drivers/net/phy/Kconfig | 13 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/mdio-mux-mmioreg.c | 170 ++++++++++++++++++++
4 files changed, 261 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
create mode 100644 drivers/net/phy/mdio-mux-mmioreg.c
diff --git a/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt b/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
new file mode 100644
index 0000000..aff8fd9
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
@@ -0,0 +1,77 @@
+Properties for an MDIO bus multiplexer controlled by a memory-mapped device
+
+This is a special case of a MDIO bus multiplexer. A memory-mapped device,
+like an FPGA, is used to control which child bus is connected. The mdio-mux
+node must be a child of the memory-mapped device. The driver currently only
+supports devices with eight-bit registers.
+
+Required properties in addition to the generic multiplexer properties:
+
+- compatible : string, must contain "mdio-mux-mmioreg"
+
+- reg : integer, contains the offset of the register that controls the bus
+ multiplexer. The size field in the 'reg' property is the size of
+ register, and must therefore be 1.
+
+- mux-mask : integer, contains an eight-bit mask that specifies which
+ bits in the register control the actual bus multiplexer. The
+ 'reg' property of each child mdio-mux node must be constrained by
+ this mask.
+
+Example:
+
+The FPGA node defines a memory-mapped FPGA with a register space of 0x30 bytes.
+For the "EMI2" MDIO bus, register 9 (BRDCFG1) controls the mux on that bus.
+A bitmask of 0x6 means that bits 1 and 2 (bit 0 is lsb) are the bits on
+BRDCFG1 that control the actual mux.
+
+ /* The FPGA node */
+ fpga: board-control@3,0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
+ reg = <3 0 0x30>;
+ ranges = <0 3 0 0x30>;
+
+ mdio-mux-emi2 {
+ compatible = "mdio-mux-mmioreg", "mdio-mux";
+ mdio-parent-bus = <&xmdio0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <9 1>; // BRDCFG1
+ mux-mask = <0x6>; // EMI2
+
+ emi2_slot1: mdio@0 { // Slot 1 XAUI (FM2)
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy_xgmii_slot1: ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c45";
+ reg = <4>;
+ };
+ };
+
+ emi2_slot2: mdio@2 { // Slot 2 XAUI (FM1)
+ reg = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy_xgmii_slot2: ethernet-phy@4 {
+ compatible = "ethernet-phy-ieee802.3-c45";
+ reg = <0>;
+ };
+ };
+ };
+ };
+
+ /* The parent MDIO bus. */
+ xmdio0: mdio@f1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,fman-xmdio";
+ reg = <0xf1000 0x1000>;
+ interrupts = <100 1 0 0>;
+ };
+
+
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 3090dc6..0268edd 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -159,6 +159,19 @@ config MDIO_BUS_MUX_GPIO
several child MDIO busses to a parent bus. Child bus
selection is under the control of GPIO lines.
+config MDIO_BUS_MUX_MMIOREG
+ tristate "Support for MMIO device-controlled MDIO bus multiplexers"
+ depends on OF_MDIO
+ select MDIO_BUS_MUX
+ help
+ This module provides a driver for MDIO bus multiplexers that
+ are controlled via a simple memory-mapped device, like an FPGA.
+ The multiplexer connects one of several child MDIO busses to a
+ parent bus. Child bus selection is under the control of one of
+ the FPGA's registers.
+
+ Currently, only 8-bit registers are supported.
+
endif # PHYLIB
config MICREL_KS8995MA
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 6d2dc6c..426674d 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_MICREL_KS8995MA) += spi_ks8995.o
obj-$(CONFIG_AMD_PHY) += amd.o
obj-$(CONFIG_MDIO_BUS_MUX) += mdio-mux.o
obj-$(CONFIG_MDIO_BUS_MUX_GPIO) += mdio-mux-gpio.o
+obj-$(CONFIG_MDIO_BUS_MUX_MMIOREG) += mdio-mux-mmioreg.o
diff --git a/drivers/net/phy/mdio-mux-mmioreg.c b/drivers/net/phy/mdio-mux-mmioreg.c
new file mode 100644
index 0000000..098239a
--- /dev/null
+++ b/drivers/net/phy/mdio-mux-mmioreg.c
@@ -0,0 +1,170 @@
+/*
+ * Simple memory-mapped device MDIO MUX driver
+ *
+ * Author: Timur Tabi <timur-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
+ *
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/device.h>
+#include <linux/of_mdio.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/phy.h>
+#include <linux/mdio-mux.h>
+
+struct mdio_mux_mmioreg_state {
+ void *mux_handle;
+ phys_addr_t phys;
+ uint8_t mask;
+};
+
+/*
+ * MDIO multiplexing switch function
+ *
+ * This function is called by the mdio-mux layer when it thinks the mdio bus
+ * multiplexer needs to switch.
+ *
+ * 'current_child' is the current value of the mux register (masked via
+ * s->mask).
+ *
+ * 'desired_child' is the value of the 'reg' property of the target child MDIO
+ * node.
+ *
+ * The first time this function is called, current_child == -1.
+ *
+ * If current_child == desired_child, then the mux is already set to the
+ * correct bus.
+ */
+static int mdio_mux_mmioreg_switch_fn(int current_child, int desired_child,
+ void *data)
+{
+ struct mdio_mux_mmioreg_state *s = data;
+
+ if (current_child ^ desired_child) {
+ void *p = ioremap(s->phys, 1);
+ uint8_t x, y;
+
+ if (!p)
+ return -ENOMEM;
+
+ x = ioread8(p);
+ y = (x & ~s->mask) | desired_child;
+ if (x != y) {
+ iowrite8((x & ~s->mask) | desired_child, p);
+ pr_debug("%s: %02x -> %02x\n", __func__, x, y);
+ }
+
+ iounmap(p);
+ }
+
+ return 0;
+}
+
+static int __devinit mdio_mux_mmioreg_probe(struct platform_device *pdev)
+{
+ struct device_node *np2, *np = pdev->dev.of_node;
+ struct mdio_mux_mmioreg_state *s;
+ struct resource res;
+ const __be32 *iprop;
+ int len, ret;
+
+ dev_dbg(&pdev->dev, "probing node %s\n", np->full_name);
+
+ s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
+ if (!s)
+ return -ENOMEM;
+
+ ret = of_address_to_resource(np, 0, &res);
+ if (ret) {
+ dev_err(&pdev->dev, "could not obtain memory map for node %s\n",
+ np->full_name);
+ return ret;
+ }
+ s->phys = res.start;
+
+ if (resource_size(&res) != sizeof(uint8_t)) {
+ dev_err(&pdev->dev, "only 8-bit registers are supported\n");
+ return -EINVAL;
+ }
+
+ iprop = of_get_property(np, "mux-mask", &len);
+ if (!iprop || len != sizeof(uint32_t)) {
+ dev_err(&pdev->dev, "missing or invalid mux-mask property\n");
+ return -ENODEV;
+ }
+ if (be32_to_cpup(iprop) > 255) {
+ dev_err(&pdev->dev, "only 8-bit registers are supported\n");
+ return -EINVAL;
+ }
+ s->mask = be32_to_cpup(iprop);
+
+ /*
+ * Verify that the 'reg' property of each child MDIO bus does not
+ * set any bits outside of the 'mask'.
+ */
+ for_each_available_child_of_node(np, np2) {
+ iprop = of_get_property(np2, "reg", &len);
+ if (!iprop || len != sizeof(uint32_t)) {
+ dev_err(&pdev->dev, "mdio-mux child node %s is "
+ "missing a 'reg' property\n", np2->full_name);
+ return -ENODEV;
+ }
+ if (be32_to_cpup(iprop) & ~s->mask) {
+ dev_err(&pdev->dev, "mdio-mux child node %s has "
+ "a 'reg' value with unmasked bits\n",
+ np2->full_name);
+ return -ENODEV;
+ }
+ }
+
+ ret = mdio_mux_init(&pdev->dev, mdio_mux_mmioreg_switch_fn,
+ &s->mux_handle, s);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register mdio-mux bus %s\n",
+ np->full_name);
+ return ret;
+ }
+
+ pdev->dev.platform_data = s;
+
+ return 0;
+}
+
+static int __devexit mdio_mux_mmioreg_remove(struct platform_device *pdev)
+{
+ struct mdio_mux_mmioreg_state *s = dev_get_platdata(&pdev->dev);
+
+ mdio_mux_uninit(s->mux_handle);
+
+ return 0;
+}
+
+static struct of_device_id mdio_mux_mmioreg_match[] = {
+ {
+ .compatible = "mdio-mux-mmioreg",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mdio_mux_mmioreg_match);
+
+static struct platform_driver mdio_mux_mmioreg_driver = {
+ .driver = {
+ .name = "mdio-mux-mmioreg",
+ .owner = THIS_MODULE,
+ .of_match_table = mdio_mux_mmioreg_match,
+ },
+ .probe = mdio_mux_mmioreg_probe,
+ .remove = __devexit_p(mdio_mux_mmioreg_remove),
+};
+
+module_platform_driver(mdio_mux_mmioreg_driver);
+
+MODULE_AUTHOR("Timur Tabi <timur-KZfg59tc24xl57MIdRCFDg@public.gmane.org>");
+MODULE_DESCRIPTION("Memory-mapped device MDIO MUX driver");
+MODULE_LICENSE("GPL v2");
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Scott Wood @ 2012-08-24 19:18 UTC (permalink / raw)
To: David Miller
Cc: swarren, timur, afleming, ddaney.cavm, netdev, devicetree-discuss
In-Reply-To: <20120824.150740.9047511945034908.davem@davemloft.net>
On 08/24/2012 02:07 PM, David Miller wrote:
> From: Stephen Warren <swarren@wwwdotorg.org>
> Date: Fri, 24 Aug 2012 12:56:05 -0600
>
>> In the I2C case, the address spaces are disjoint, so there's never any
>> mapping between them, so there's no need for ranges.
>>
>> Any time the child address space is intended to be part of the parent's
>> address space, I believe ranges is supposed to be specified, perhaps
>> even mandatory, even if the translation is 1:1.
Yes, it's mandatory (even if the kernel lets you get away without it for
the sake of some broken Apple firmware, IIRC). If the translation is an
identity map you can use an empty "ranges;".
> Regardless, you really can't just generically translate ranges
> in some universal way and expect it to work in all cases.
>
> You need bus specific drivers to deal with various special
> cases.
>
> See the *_map() methods implemented in:
>
> arch/sparc/kernel/of_device_64.c
>
> for example.
We don't expect it to work in all cases. We expect it to work if the
bus node is on the whitelist for which we create devices on
platform_bus, there's a platform driver that binds against it, and that
driver calls of_iomap() or equivalent because the binding says that reg
refers to something that is memory mapped.
-Scott
^ permalink raw reply
* Re: pull request: wireless 2012-08-24
From: David Miller @ 2012-08-24 19:18 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20120824161330.GC27147@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Fri, 24 Aug 2012 12:13:31 -0400
> This batch of fixes is intended for 3.6...
>
> Johannes Berg gives us a pair of iwlwifi fixes. One corrects some
> improperly defined ifdefs that lead to crashes and BUG_ONs. The other
> prevents attempts to read SRAM for devices that aren't actually started.
>
> Julia Lawall provides an ipw2100 fix to properly set the return code
> from a function call before testing it! :-)
>
> Thomas Huehn corrects the improper use of a constant related to a power
> setting in ath5k.
>
> Thomas Pedersen offers a mac80211 fix to properly handle destination
> addresses of unicast frames passing though a mesh gate.
>
> Vladimir Zapolskiy provides a brcmsmac fix to properly mark the
> interface state when the device goes down.
Pulled, thanks a lot John.
^ permalink raw reply
* Re: pull request: wireless-next 2012-08-24
From: David Miller @ 2012-08-24 19:18 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev
In-Reply-To: <20120824173138.GD27147@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Fri, 24 Aug 2012 13:31:38 -0400
> This is a batch of updates intended for 3.7. The bulk of it is
> mac80211 changes, including some mesh work from Thomas Pederson and
> some multi-channel work from Johannes. A variety of driver updates
> and other bits are scattered in there as well.
Pulled, thanks!
^ permalink raw reply
* Re: pull-request: can 2012-08-24
From: David Miller @ 2012-08-24 19:21 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can
In-Reply-To: <1345800643-29456-1-git-send-email-mkl@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Fri, 24 Aug 2012 11:30:41 +0200
> here are two fixes for the v3.6 release cycle. Alexey Khoroshilov submitted a
> fix for a memory leak in the softing driver (in softing_load_fw()) in case a
> krealloc() fails. Sven Schmitt fixed the misuse of the IRQF_SHARED flag in the
> irq resouce of the sja1000 platform driver, now the correct flag is used. There
> are no mainline users of this feature which need to be converted.
Pulled, thanks Marc.
^ permalink raw reply
* RE: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB_PTP.
From: Keller, Jacob E @ 2012-08-24 19:38 UTC (permalink / raw)
To: Ben Hutchings, Richard Cochran
Cc: Vick, Matthew, Kirsher, Jeffrey T, davem@davemloft.net,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <1345827088.2694.11.camel@bwh-desktop.uk.solarflarecom.com>
> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Friday, August 24, 2012 9:51 AM
> To: Richard Cochran
> Cc: Vick, Matthew; Keller, Jacob E; Kirsher, Jeffrey T;
> davem@davemloft.net; netdev@vger.kernel.org; gospo@redhat.com;
> sassmann@redhat.com
> Subject: Re: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB_PTP.
>
> On Fri, 2012-08-24 at 12:10 +0200, Richard Cochran wrote:
> > On Thu, Aug 23, 2012 at 06:40:25PM +0000, Vick, Matthew wrote:
> > >
> > > I tend to agree with Jake here--I like having the information. I'm
> fine removing them, but I'd like to do it for all CONFIG_IGB_PTP wrapping
> if we're going to do it. What do you think, Richard?
> >
> > Come to think of it, I never liked the CONFIG_IGB_PTP very much in the
> > first place. These were added after the fact by Jeff Kirsher. He had
> > said off list that there was some issue with CONFIG_PTP_1588_CLOCK and
> > igb as a module, or something like that. At that time I said, just go
> > ahead and fix it up.
>
> If there is some feature of a driver that depends on an optional
> modularisable subsystem, and you want the driver to be built without that
> feature when the subsystem is missing, then the feature has to be disabled
> when the driver is built-in and the subsystem is a module. So in general
> you need:
>
> config DRIVER_FEATURE
> bool "This is a really neat feature for the driver"
> depends on DRIVER && SUBSYSTEM && !(DRIVER=y && SUBYSTEM=m)
>
> > I think it would be better if the "time stamp all Rx packets" of the
> > 82580 were always available, and that the PHC feature always be
> > compiled when CONFIG_PTP_1588_CLOCK is selected.
> >
> > Maybe you could ask Jeff what the issue was, and then see if there is
> > a way to remove CONFIG_IGB_PTP altogether.
>
> Even if they are to be enabled automatically, CONFIG_IGB_PTP and similar
> config symbols are important as shorthand. So I think what you want is:
>
> config IGB_PTP
> def_bool y
> depends on IGB && PTP_1588_CLOCK && !(IGB=y && PTP_1588_CLOCK=m)
>
> (But currently it's actually *selecting* PTP_1588_CLOCK. We should
> probably have drivers consistently select or depend on it, not a
> mixture.)
>
> 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.
The IGP_PTP is necessary otherwise you have to do something like #if defined(CONFIG_PTP_1588_CLOCK), or #ifdef CONFIG_PTP_1588_CLOCK \ #ifdef CONFIG_PTP_1588_CLOCK_MODULE
The main reason that I wouldn't want to un-wrap the timestamping is because the hwtstamp ioctl is somewhat problematic because it is almost all ptp only. Also, some of the parts for igb driver don't support timestamp all, they only support ptp only packets, and this would be a lot more confusing since I would say still only allow that if ptp is on.. (since those values are useless except with the PHC clock)
- Jake
^ permalink raw reply
* Re: [PATCH 14/15] netpoll: re-enable irq in poll_napi()
From: Andrew Morton @ 2012-08-24 19:43 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, David Miller
In-Reply-To: <1344597891-32242-15-git-send-email-amwang@redhat.com>
On Fri, 10 Aug 2012 19:24:50 +0800
Cong Wang <amwang@redhat.com> wrote:
> napi->poll() needs IRQ enabled, so we have to re-enable IRQ before
> calling it.
>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <amwang@redhat.com>
> ---
> net/core/netpoll.c | 10 +++++++++-
> 1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index e4ba3e7..346b1eb 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -168,16 +168,24 @@ static void poll_napi(struct net_device *dev)
> struct napi_struct *napi;
> int budget = 16;
>
> + WARN_ON_ONCE(!irqs_disabled());
> +
> list_for_each_entry(napi, &dev->napi_list, dev_list) {
> + local_irq_enable();
> if (napi->poll_owner != smp_processor_id() &&
> spin_trylock(&napi->poll_lock)) {
> + rcu_read_lock_bh();
> budget = poll_one_napi(rcu_dereference_bh(dev->npinfo),
> napi, budget);
> + rcu_read_unlock_bh();
> spin_unlock(&napi->poll_lock);
>
> - if (!budget)
> + if (!budget) {
> + local_irq_disable();
> break;
> + }
> }
> + local_irq_disable();
> }
> }
This commit (6bdb7fe3104 in mainline) makes my netconsole-using x86_64
box lock up during boot. Dunno why, but I do have a cellphone:
http://ozlabs.org/~akpm/stuff/IMG_20120824_122054.jpg
^ permalink raw reply
* pull request: sfc-next 2012-08-24
From: Ben Hutchings @ 2012-08-24 19:46 UTC (permalink / raw)
To: David Miller; +Cc: linux-net-drivers, netdev
The following changes since commit 8f4cccbbd92f2ad0ddbbc498ef7cee2a1c3defe9:
net: Set device operstate at registration time (2012-08-24 12:46:13 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next.git for-davem
(commit 8f8b3d518999fd1c342310910aa1e49112c86d05)
1. Change the TX path to stop queues earlier and avoid returning
NETDEV_TX_BUSY.
2. Remove some inefficiencies in soft-TSO.
3. Fix various bugs involving device state transitions and/or reset
scheduling by error handlers.
4. Take advantage of my previous change to operstate initialisation.
5. Miscellaneous cleanup.
Ben.
Ben Hutchings (16):
sfc: Refactor struct efx_tx_buffer to use a flags field
sfc: Stop TX queues before they fill up
sfc: Simplify TSO header buffer allocation
sfc: Replace tso_state::full_packet_space with ip_base_len
sfc: Stash header offsets for TSO in struct tso_state
sfc: Change state names to be clearer, and comment them
sfc: Hold the RTNL lock for more of the suspend/resume cycle
sfc: Keep disabled NICs quiescent during suspend/resume
sfc: Hold RTNL lock (only) when calling efx_stop_interrupts()
sfc: Never try to stop and start a NIC that is disabled
sfc: Improve log messages in case we abort probe due to a pending reset
sfc: Fix reset vs probe/remove/PM races involving efx_nic::state
sfc: Remove overly paranoid locking assertions from netdev operations
sfc: Remove bogus comment about MTU change and RX buffer overrun
sfc: Assign efx and efx->type as early as possible in efx_pci_probe()
sfc: Fix the initial device operstate
drivers/net/ethernet/sfc/efx.c | 235 +++++++-----
drivers/net/ethernet/sfc/ethtool.c | 4 +-
drivers/net/ethernet/sfc/falcon_boards.c | 2 +-
drivers/net/ethernet/sfc/net_driver.h | 49 ++--
drivers/net/ethernet/sfc/nic.c | 6 +-
drivers/net/ethernet/sfc/tx.c | 621 ++++++++++++------------------
6 files changed, 410 insertions(+), 507 deletions(-)
--
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
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