* [PATCH 17/18] pcmcia: do not request windows if you don't need to
From: Dominik Brodowski @ 2010-08-05 22:06 UTC (permalink / raw)
To: linux-pcmcia-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Dominik Brodowski, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20100805220511.GA7040-S7uyTPAaJ/sb6pqDj42GsMgv3T4z79SOrE5yTffgRl4@public.gmane.org>
Several drivers contained dummy code to request for memory windows,
even though they never made use of it. Remove all such code
snippets.
CC: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Dominik Brodowski <linux-X3ehHDuj6sIIGcDfoQAp7OTW4wlIGRCZ@public.gmane.org>
---
drivers/isdn/hisax/sedlbauer_cs.c | 37 +------------------------
drivers/net/wireless/airo_cs.c | 37 +------------------------
drivers/staging/comedi/drivers/ni_daq_700.c | 23 +---------------
drivers/staging/comedi/drivers/ni_daq_dio24.c | 24 +---------------
drivers/staging/comedi/drivers/ni_labpc_cs.c | 24 +---------------
5 files changed, 5 insertions(+), 140 deletions(-)
diff --git a/drivers/isdn/hisax/sedlbauer_cs.c b/drivers/isdn/hisax/sedlbauer_cs.c
index 0b06dbb..a024192 100644
--- a/drivers/isdn/hisax/sedlbauer_cs.c
+++ b/drivers/isdn/hisax/sedlbauer_cs.c
@@ -171,8 +171,6 @@ static int sedlbauer_config_check(struct pcmcia_device *p_dev,
unsigned int vcc,
void *priv_data)
{
- win_req_t *req = priv_data;
-
if (cfg->index == 0)
return -ENODEV;
@@ -219,31 +217,6 @@ static int sedlbauer_config_check(struct pcmcia_device *p_dev,
return -ENODEV;
}
- /*
- Now set up a common memory window, if needed. There is room
- in the struct pcmcia_device structure for one memory window handle,
- but if the base addresses need to be saved, or if multiple
- windows are needed, the info should go in the private data
- structure for this device.
-
- Note that the memory window base is a physical address, and
- needs to be mapped to virtual space with ioremap() before it
- is used.
- */
- if ((cfg->mem.nwin > 0) || (dflt->mem.nwin > 0)) {
- cistpl_mem_t *mem = (cfg->mem.nwin) ? &cfg->mem : &dflt->mem;
- req->Attributes = WIN_DATA_WIDTH_16|WIN_MEMORY_TYPE_CM;
- req->Attributes |= WIN_ENABLE;
- req->Base = mem->win[0].host_addr;
- req->Size = mem->win[0].len;
- req->AccessSpeed = 0;
- if (pcmcia_request_window(p_dev, req, &p_dev->win) != 0)
- return -ENODEV;
-
- if (pcmcia_map_mem_page(p_dev, p_dev->win,
- mem->win[0].card_addr) != 0)
- return -ENODEV;
- }
return 0;
}
@@ -251,16 +224,11 @@ static int sedlbauer_config_check(struct pcmcia_device *p_dev,
static int __devinit sedlbauer_config(struct pcmcia_device *link)
{
- win_req_t *req;
int ret;
IsdnCard_t icard;
dev_dbg(&link->dev, "sedlbauer_config(0x%p)\n", link);
- req = kzalloc(sizeof(win_req_t), GFP_KERNEL);
- if (!req)
- return -ENOMEM;
-
/*
In this loop, we scan the CIS for configuration table entries,
each of which describes a valid card configuration, including
@@ -273,7 +241,7 @@ static int __devinit sedlbauer_config(struct pcmcia_device *link)
these things without consulting the CIS, and most client drivers
will only use the CIS to fill in implementation-defined details.
*/
- ret = pcmcia_loop_config(link, sedlbauer_config_check, req);
+ ret = pcmcia_loop_config(link, sedlbauer_config_check, NULL);
if (ret)
goto failed;
@@ -297,9 +265,6 @@ static int __devinit sedlbauer_config(struct pcmcia_device *link)
printk(" & %pR", link->resource[0]);
if (link->resource[1])
printk(" & %pR", link->resource[1]);
- if (link->win)
- printk(", mem 0x%06lx-0x%06lx", req->Base,
- req->Base+req->Size-1);
printk("\n");
icard.para[0] = link->irq;
diff --git a/drivers/net/wireless/airo_cs.c b/drivers/net/wireless/airo_cs.c
index d47672c..9a121a5 100644
--- a/drivers/net/wireless/airo_cs.c
+++ b/drivers/net/wireless/airo_cs.c
@@ -154,8 +154,6 @@ static int airo_cs_config_check(struct pcmcia_device *p_dev,
unsigned int vcc,
void *priv_data)
{
- win_req_t *req = priv_data;
-
if (cfg->index == 0)
return -ENODEV;
@@ -194,29 +192,6 @@ static int airo_cs_config_check(struct pcmcia_device *p_dev,
if (pcmcia_request_io(p_dev) != 0)
return -ENODEV;
- /*
- Now set up a common memory window, if needed. There is room
- in the struct pcmcia_device structure for one memory window handle,
- but if the base addresses need to be saved, or if multiple
- windows are needed, the info should go in the private data
- structure for this device.
-
- Note that the memory window base is a physical address, and
- needs to be mapped to virtual space with ioremap() before it
- is used.
- */
- if ((cfg->mem.nwin > 0) || (dflt->mem.nwin > 0)) {
- cistpl_mem_t *mem = (cfg->mem.nwin) ? &cfg->mem : &dflt->mem;
- req->Attributes = WIN_DATA_WIDTH_16|WIN_MEMORY_TYPE_CM;
- req->Base = mem->win[0].host_addr;
- req->Size = mem->win[0].len;
- req->AccessSpeed = 0;
- if (pcmcia_request_window(p_dev, req, &p_dev->win) != 0)
- return -ENODEV;
- if (pcmcia_map_mem_page(p_dev, p_dev->win,
- mem->win[0].card_addr) != 0)
- return -ENODEV;
- }
/* If we got this far, we're cool! */
return 0;
}
@@ -225,17 +200,12 @@ static int airo_cs_config_check(struct pcmcia_device *p_dev,
static int airo_config(struct pcmcia_device *link)
{
local_info_t *dev;
- win_req_t *req;
int ret;
dev = link->priv;
dev_dbg(&link->dev, "airo_config\n");
- req = kzalloc(sizeof(win_req_t), GFP_KERNEL);
- if (!req)
- return -ENOMEM;
-
/*
* In this loop, we scan the CIS for configuration table
* entries, each of which describes a valid card
@@ -250,7 +220,7 @@ static int airo_config(struct pcmcia_device *link)
* and most client drivers will only use the CIS to fill in
* implementation-defined details.
*/
- ret = pcmcia_loop_config(link, airo_cs_config_check, req);
+ ret = pcmcia_loop_config(link, airo_cs_config_check, NULL);
if (ret)
goto failed;
@@ -281,16 +251,11 @@ static int airo_config(struct pcmcia_device *link)
printk(" & %pR", link->resource[0]);
if (link->resource[1])
printk(" & %pR", link->resource[1]);
- if (link->win)
- printk(", mem 0x%06lx-0x%06lx", req->Base,
- req->Base+req->Size-1);
printk("\n");
- kfree(req);
return 0;
failed:
airo_release(link);
- kfree(req);
return -ENODEV;
} /* airo_config */
diff --git a/drivers/staging/comedi/drivers/ni_daq_700.c b/drivers/staging/comedi/drivers/ni_daq_700.c
index abaa40b..bf2e84a 100644
--- a/drivers/staging/comedi/drivers/ni_daq_700.c
+++ b/drivers/staging/comedi/drivers/ni_daq_700.c
@@ -555,8 +555,6 @@ static int dio700_pcmcia_config_loop(struct pcmcia_device *p_dev,
unsigned int vcc,
void *priv_data)
{
- win_req_t *req = priv_data;
-
if (cfg->index == 0)
return -ENODEV;
@@ -589,22 +587,6 @@ static int dio700_pcmcia_config_loop(struct pcmcia_device *p_dev,
return -ENODEV;
}
- if ((cfg->mem.nwin > 0) || (dflt->mem.nwin > 0)) {
- cistpl_mem_t *mem =
- (cfg->mem.nwin) ? &cfg->mem : &dflt->mem;
- req->Attributes = WIN_DATA_WIDTH_16 | WIN_MEMORY_TYPE_CM;
- req->Attributes |= WIN_ENABLE;
- req->Base = mem->win[0].host_addr;
- req->Size = mem->win[0].len;
- if (req->Size < 0x1000)
- req->Size = 0x1000;
- req->AccessSpeed = 0;
- if (pcmcia_request_window(p_dev, req, &p_dev->win))
- return -ENODEV;
- if (pcmcia_map_mem_page(p_dev, p_dev->win,
- mem->win[0].card_addr))
- return -ENODEV;
- }
/* If we got this far, we're cool! */
return 0;
}
@@ -618,7 +600,7 @@ static void dio700_config(struct pcmcia_device *link)
dev_dbg(&link->dev, "dio700_config\n");
- ret = pcmcia_loop_config(link, dio700_pcmcia_config_loop, &req);
+ ret = pcmcia_loop_config(link, dio700_pcmcia_config_loop, NULL);
if (ret) {
dev_warn(&link->dev, "no configuration found\n");
goto failed;
@@ -644,9 +626,6 @@ static void dio700_config(struct pcmcia_device *link)
printk(", io %pR", link->resource[0]);
if (link->resource[1])
printk(" & %pR", link->resource[1]);
- if (link->win)
- printk(", mem 0x%06lx-0x%06lx", req.Base,
- req.Base + req.Size - 1);
printk("\n");
return;
diff --git a/drivers/staging/comedi/drivers/ni_daq_dio24.c b/drivers/staging/comedi/drivers/ni_daq_dio24.c
index caccece..9112e70 100644
--- a/drivers/staging/comedi/drivers/ni_daq_dio24.c
+++ b/drivers/staging/comedi/drivers/ni_daq_dio24.c
@@ -307,8 +307,6 @@ static int dio24_pcmcia_config_loop(struct pcmcia_device *p_dev,
unsigned int vcc,
void *priv_data)
{
- win_req_t *req = priv_data;
-
if (cfg->index == 0)
return -ENODEV;
@@ -341,22 +339,6 @@ static int dio24_pcmcia_config_loop(struct pcmcia_device *p_dev,
return -ENODEV;
}
- if ((cfg->mem.nwin > 0) || (dflt->mem.nwin > 0)) {
- cistpl_mem_t *mem =
- (cfg->mem.nwin) ? &cfg->mem : &dflt->mem;
- req->Attributes = WIN_DATA_WIDTH_16 | WIN_MEMORY_TYPE_CM;
- req->Attributes |= WIN_ENABLE;
- req->Base = mem->win[0].host_addr;
- req->Size = mem->win[0].len;
- if (req->Size < 0x1000)
- req->Size = 0x1000;
- req->AccessSpeed = 0;
- if (pcmcia_request_window(p_dev, req, &p_dev->win))
- return -ENODEV;
- if (pcmcia_map_mem_page(p_dev, p_dev->win,
- mem->win[0].card_addr))
- return -ENODEV;
- }
/* If we got this far, we're cool! */
return 0;
}
@@ -364,13 +346,12 @@ static int dio24_pcmcia_config_loop(struct pcmcia_device *p_dev,
static void dio24_config(struct pcmcia_device *link)
{
int ret;
- win_req_t req;
printk(KERN_INFO "ni_daq_dio24: HOLA SOY YO! - config\n");
dev_dbg(&link->dev, "dio24_config\n");
- ret = pcmcia_loop_config(link, dio24_pcmcia_config_loop, &req);
+ ret = pcmcia_loop_config(link, dio24_pcmcia_config_loop, NULL);
if (ret) {
dev_warn(&link->dev, "no configuration found\n");
goto failed;
@@ -396,9 +377,6 @@ static void dio24_config(struct pcmcia_device *link)
printk(" & %pR", link->resource[0]);
if (link->resource[1])
printk(" & %pR", link->resource[1]);
- if (link->win)
- printk(", mem 0x%06lx-0x%06lx", req.Base,
- req.Base + req.Size - 1);
printk("\n");
return;
diff --git a/drivers/staging/comedi/drivers/ni_labpc_cs.c b/drivers/staging/comedi/drivers/ni_labpc_cs.c
index 94d9f7f..f204961 100644
--- a/drivers/staging/comedi/drivers/ni_labpc_cs.c
+++ b/drivers/staging/comedi/drivers/ni_labpc_cs.c
@@ -285,8 +285,6 @@ static int labpc_pcmcia_config_loop(struct pcmcia_device *p_dev,
unsigned int vcc,
void *priv_data)
{
- win_req_t *req = priv_data;
-
if (cfg->index == 0)
return -ENODEV;
@@ -319,22 +317,6 @@ static int labpc_pcmcia_config_loop(struct pcmcia_device *p_dev,
return -ENODEV;
}
- if ((cfg->mem.nwin > 0) || (dflt->mem.nwin > 0)) {
- cistpl_mem_t *mem =
- (cfg->mem.nwin) ? &cfg->mem : &dflt->mem;
- req->Attributes = WIN_DATA_WIDTH_16 | WIN_MEMORY_TYPE_CM;
- req->Attributes |= WIN_ENABLE;
- req->Base = mem->win[0].host_addr;
- req->Size = mem->win[0].len;
- if (req->Size < 0x1000)
- req->Size = 0x1000;
- req->AccessSpeed = 0;
- if (pcmcia_request_window(p_dev, req, &p_dev->win))
- return -ENODEV;
- if (pcmcia_map_mem_page(p_dev, p_dev->win,
- mem->win[0].card_addr))
- return -ENODEV;
- }
/* If we got this far, we're cool! */
return 0;
}
@@ -343,11 +325,10 @@ static int labpc_pcmcia_config_loop(struct pcmcia_device *p_dev,
static void labpc_config(struct pcmcia_device *link)
{
int ret;
- win_req_t req;
dev_dbg(&link->dev, "labpc_config\n");
- ret = pcmcia_loop_config(link, labpc_pcmcia_config_loop, &req);
+ ret = pcmcia_loop_config(link, labpc_pcmcia_config_loop, NULL);
if (ret) {
dev_warn(&link->dev, "no configuration found\n");
goto failed;
@@ -373,9 +354,6 @@ static void labpc_config(struct pcmcia_device *link)
printk(" & %pR", link->resource[0]);
if (link->resource[1])
printk(" & %pR", link->resource[1]);
- if (link->win)
- printk(", mem 0x%06lx-0x%06lx", req.Base,
- req.Base + req.Size - 1);
printk("\n");
return;
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
From: Kevin Hilman @ 2010-08-05 22:24 UTC (permalink / raw)
To: Patrick Pannuto
Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-omap@vger.kernel.org, damm@opensource.se,
lethal@linux-sh.org, rjw@sisk.pl, eric.y.miao@gmail.com,
netdev@vger.kernel.org, Greg Kroah-Hartman, alan, zt.tmzt,
grant.likely, magnus.damm
In-Reply-To: <4C5AE75F.5020507@codeaurora.org>
Patrick Pannuto <ppannuto@codeaurora.org> writes:
>>
>>> You had indicated that you were going on vacation for a month and I
>>> had not seen any more follow-up on this issue, so I forged ahead.
>>
>> Great, I'm glad you forged ahead. There is definitely a broader need
>> for something like this, and I have no personal attachment to the code.
>>
>> I have no problems with you continuing the work (in fact, I'd prefer it.
>> I have lots of other things to catch up on after my vacation.)
>>
>> In the future though, it's common (and kind) to note the original author
>> in the changelog when basing a patch on previous work. Something like
>> "originally written by..." or "based on the work of..." etc.
>
> Ok, I can do that; that was the intention of the "original inspiration from"
> line at the beginning.
Maybe we need an 'Inspired-by:' tag. :)
> Is there a more formal way of indicating this in the
> next version of the patch? Should I add you as a "From:" or an "Author:"?
I don't know if there is an official way of doing this.
I typically add something like "based on the idea/code originally
proposed/written by John Doe" in the changelog, and then add Cc: John
Doe <...> in the changelog too, but AFAIK, none of this is formalized.
Kevin
^ permalink raw reply
* Re: [RFC PATCH] platform: Faciliatate the creation of pseduo-platform busses
From: Grant Likely @ 2010-08-05 22:59 UTC (permalink / raw)
To: Patrick Pannuto
Cc: Greg KH, Patrick Pannuto, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-omap@vger.kernel.org,
damm@opensource.se, lethal@linux-sh.org, rjw@sisk.pl,
dtor@mail.ru, eric.y.miao@gmail.com, netdev@vger.kernel.org,
Kevin Hilman
In-Reply-To: <4C58B1B6.9050005@quicinc.com>
On Tue, Aug 3, 2010 at 6:17 PM, Patrick Pannuto <ppannuto@quicinc.com> wrote:
>>>>>
>>>>> struct platform_device sub_bus1 = {
>>>>> .name = "sub_bus1",
>>>>> .id = -1,
>>>>> .dev.bus = &my_bus_type,
>>>>> }
>>>>> EXPORT_SYMBOL_GPL(sub_bus1);
>>>>
>>>> You really want a bus hanging off of a bus? Normally you need a device
>>>> to do that, which is what I think you have here, but the naming is a bit
>>>> odd to me.
>>>>
>>>> What would you do with this "sub bus"? It's just a device, but you are
>>>> wanting it to be around for something.
>>>>
>>>
>>> It's for power management stuff, basically, there are actual physical buses
>>> involved that can be completely powered off IFF all of their devices are
>>> not in use. Plus it actually matches bus topology this way.
>>
>> Then create a real bus hanging off of a device, not another device that
>> "acts" like a bus here, right? Or am I missing the point?
>>
>
> The motivation for doing it this was is that one driver could drive
> devices on two different subbusses. In the example, "my-driver" could
> drive a device on sub_bus1 AND sub_bus2 (if there were 2+ devices, one
> or more on each bus).
>
> From my understanding, this is not possible if they are actually
> different busses.
It sounds an awful lot like 2 separate concepts are being conflated
here. In Linux terminology, we have two things:
/bus types/ describe the common behavour of similarly attached things.
/physical or logical busses/ are where devices are actually attached
registered to the system.
These are not the same thing.
The primary thing that a bus-type provides is the set of device driver
that can be bound to the device plus some common functionality.
Almost every struct device has the 'struct bus_type bus' field set so
that it can be bound against a device driver when it is registered.
Every struct device also has a 'struct device *parent' field. The
parent field denotes where the device lives in the Linux device model
(see /sys/devices/*), but it says *nothing* about what the bus type of
device it is. Remember I mentioned physical/logical busses earlier?
Well the core Linux device model doesn't really have a concept of bus
instances, it only knows about devices and bus types. The .parent
field is the closest thing the device model has to a bus instance, but
the model places no constraints on the bus type of each child 'struct
device'.
(caveat: many bus_types do have a concept of bus instances; but that
is specific to the bus_type. The core device model doesn't care.
Also, in practical terms, yes most sibling devices will also be of the
same bus_type, but it is not a given, and it is not enforced.)
For example, a system can have many physical pci busses, but all the
devices hanging off those busses are still just registered on
pci_bus_type. The parent of those devices, however, reflects the
physical bus the device is attached to.
In fact, the bus type has absolutely no bearing on where a device
actually lives in the Linux driver model. A common misunderstanding
is that all platform devices live under /sys/devices/platform.
This also is not true.
By default the platform bus will put devices without a parent into
/sys/devices/platform, but if parent is set, then the device will be
registered where the parent pointer tells it to be registered. It is
totally okay to have the following structure (off the top of my head):
system_bus (platform_bus_type, registers a bunch of child devices)
|--pci_bus (platform_bus_type; implements a pci host bridge)
| |--usb host controller 1 (pci_bus_type, implements a usb bus instance)
| |--usb host controller 2 (pci_bus_type, implements a usb bus instance)
| |--fpga (pci_bus_type, implements a bunch of devices)
| |--spi controller (platform_bus_type)
| |--custom peripheral (platform_bus_type)
|--uart (platform_bus_type)
|--uart (platform_bus_type)
|--ethernet (platform_bus_type)
|--external bus (platform_bus_type; represents and external local bus)
| |--i2c controller (platform_bus_type)
| | |--eeprom (i2c_bus_type)
| | |--temperature sensor (i2c_bus_type)
|--gpio controller (platform_bus_type)
Platform devices can show up just about anywhere in the tree.
(On that point Greg, what is the reason for even having the
/sys/devices/platform/ parent? Why not just let the platform devices
sit at the root of the device tree? In the OF case (granted, I'm
biased) all of the platform_device registrations reflect the actual
device hierarchy expressed in the device tree data.)
So, the *only* reason to have a different bus_type is if the actual
behaviour of the bus is different. A different bus type wouldn't even
cause a different topology if doing so wouldn't make sense. For
example in the OMAP case, the topology might look like:
system_bus (platform_bus_type, registers a bunch of child devices)
| ...
|--uart (omap_bus_type)
|--uart (omap_bus_type)
|--ethernet (platform_bus_type)
See? Devices of different types can often be siblings at the same
device layer. (note: not *always* true. PCI busses for instance
always expect to only host pci_devices and visa-versa, but for simple
platform-like busses it may be true).
Now, having gone on this whole long tirade, it looks like having
separate platform bus types may not be the best approach after all.
Magnus and Kevin are exploring using devres to handle implementing the
system-specific behaviour. It is certainly an area for more research
and experimental implementations.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: Using virtio as a physical (wire-level) transport
From: Ira W. Snyder @ 2010-08-05 23:01 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Rusty Russell, virtualization, Zang Roy, netdev
In-Reply-To: <20100805213050.GA24984@redhat.com>
On Fri, Aug 06, 2010 at 12:30:50AM +0300, Michael S. Tsirkin wrote:
> Hi Ira,
>
> > Making my life harder since the last time I tried this, mainline commit
> > 7c5e9ed0c (virtio_ring: remove a level of indirection) has removed the
> > possibility of using an alternative virtqueue implementation. The commit
> > message suggests that you might be willing to add this capability back.
> > Would this be an option?
>
> Sorry about that.
>
> With respect to this commit, we only had one implementation upstream
> and extra levels of indirection made extending the API
> much harder for no apparent benefit.
>
> When there's more than one ring implementation with very small amount of
> common code, I think that it might make sense to readd the indirection
> back, to separate the code cleanly.
>
> OTOH if the two implementations share a lot of code, I think that it
> might be better to just add a couple of if statements here and there.
> This way compiler even might have a chance to compile the code out if
> the feature is disabled in kernel config.
>
The virtqueue implementation I envision will be almost identical to the
current virtio_ring virtqueue implementation, with the following
exceptions:
* the "shared memory" will actually be remote, on the PCI BAR of a device
* iowrite32(), ioread32() and friends will be needed to access the memory
* there will only be a fixed number of virtqueues available, due to PCI
BAR size
* cross-endian virtqueues must work
* kick needs to be cross-machine (using PCI IRQ's)
I don't think it is feasible to add this to the existing implementation.
I think the requirement of being cross-endian will be the hardest to
overcome. Rusty did not envision the cross-endian use case when he
designed this, and it shows, in virtio_ring, virtio_net and vhost. I
have no idea what to do about this. Do you have any ideas?
I plan to create a custom socket similar to tun/macvtap which will use
DMA to transfer around data. This, along with a few other tricks, will
allow me to use vhost_net to operate the device. Along with a custom
virtqueue implementation meeting the requirements above, this seems like
a good plan.
Thanks for responding,
Ira
^ permalink raw reply
* Re: [RFC PATCH] platform: Faciliatate the creation of pseduo-platform busses
From: Grant Likely @ 2010-08-05 23:00 UTC (permalink / raw)
To: Greg KH
Cc: Patrick Pannuto, linux-kernel, linux-arm-msm, linux-omap, damm,
lethal, rjw, dtor, eric.y.miao, netdev
In-Reply-To: <20100803235631.GA17759@suse.de>
On Tue, Aug 3, 2010 at 5:56 PM, Greg KH <gregkh@suse.de> wrote:
> On Tue, Aug 03, 2010 at 04:35:06PM -0700, Patrick Pannuto wrote:
>> @@ -539,12 +541,12 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv,
>> * if the probe was successful, and make sure any forced probes of
>> * new devices fail.
>> */
>> - spin_lock(&platform_bus_type.p->klist_drivers.k_lock);
>> + spin_lock(&drv->driver.bus->p->klist_drivers.k_lock);
>> drv->probe = NULL;
>> if (code == 0 && list_empty(&drv->driver.p->klist_devices.k_list))
>> retval = -ENODEV;
>> drv->driver.probe = platform_drv_probe_fail;
>> - spin_unlock(&platform_bus_type.p->klist_drivers.k_lock);
>> + spin_unlock(&drv->driver.bus->p->klist_drivers.k_lock);
>>
>> if (code != retval)
>> platform_driver_unregister(drv);
>
> I'm guessing that this chunk can be applied now, right?
Yes, I agree. This chunk makes sense independently of the rest of the patch.
g.
^ permalink raw reply
* Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
From: Grant Likely @ 2010-08-05 23:16 UTC (permalink / raw)
To: Kevin Hilman
Cc: Patrick Pannuto, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-omap@vger.kernel.org,
damm@opensource.se, lethal@linux-sh.org, rjw@sisk.pl,
eric.y.miao@gmail.com, netdev@vger.kernel.org, Greg Kroah-Hartman,
alan, zt.tmzt, magnus.damm
In-Reply-To: <87fwytxdba.fsf@deeprootsystems.com>
On Thu, Aug 5, 2010 at 9:57 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Patrick Pannuto <ppannuto@codeaurora.org> writes:
>
>> On 08/04/2010 05:16 PM, Kevin Hilman wrote:
>>> Patrick Pannuto <ppannuto@codeaurora.org> writes:
>>>
>>>> Inspiration for this comes from:
>>>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
>>>
>>> Also, later in that thread I also wrote[1] what seems to be the core of
>>> what you've done here: namely, allow platform_devices and
>>> platform_drivers to to be used on custom busses. Patch is at the end of
>>> this mail with a more focused changelog. As Greg suggested in his reply
>>> to your first version, this part could be merged today, and the
>>> platform_bus_init stuff could be added later, after some more review.
>>> Some comments below...
>>>
>>
>> I can split this into 2 patches.
>
> Yes, I think that would be better.
>
>> Was your patch sent to linux-kernel or just linux-omap? I'm not on linux-omap...
>
> That thread was on linux-arm-kernel and linux-omap
>
>>
>>>> [snip]
>>>>
>>>> Which will allow the same driver to easily to used on either
>>>> the platform bus or the newly defined bus type.
>>>
>>> Except it requires a re-compile.
>>>
>>> Rather than doing this at compile time, it would be better to support
>>> legacy devices at runtime. You could handle this by simply registering
>>> the driver on the custom bus and the platform_bus and let the bus
>>> matching code handle it. Then, the same binary would work on both
>>> legacy and updated SoCs.
>>>
>>
>> Can you safely register a driver on more than one bus? I didn't think
>> that was safe -- normally it's impossible since you're calling
>>
>> struct BUS_TYPE_driver mydriver;
>> BUS_TYPE_driver_register(&mydriver)
>>
>> but now we have multiple "bus types" that are all actually platform type; still,
>> at a minimum you would need:
>> struct platform_driver mydrvier1 = {
>> .driver.bus = &sub_bus1,
>> };
>> struct platform_driver mydrvier2 = {
>> .driver.bus = &sub_bus2,
>> };
>> which would all point to the same driver functions, yet the respective devices
>> attached for the "same" driver would be on different buses. I fear this might
>> confuse some drivers. I don't think dynamic bus assignment is this easy
>>
>> In short: I do not believe the same driver can be registered on multiple
>> different buses -- if this is wrong, please correct me.
>
> It is possible, and currently done in powerpc land where some
> drivers handle devices on the platform_bus and the custom OF bus.
As of now, the of_platform_bus_type has been removed. It was a bad
idea because it tried to encode non-bus-specific information into
something that was just a clone of the platform_bus. Drivers that
worked on both had to be bound to both busses. I do actually have
code that automatically registers a driver on more than one bus, but
it is rather a hack and was only a temporary measure.
The relevant question before going down this path is, "Is the
omap/sh/other-soc behaviour something fundamentally different from the
platform bus? Or is it something complementary that would be better
handled with a notifier or some orthogonal method of adding new
behaviour?"
I don't have a problem with multiple platform_bus instances using the
same code (I did suggest it after all), but I do worry about muddying
the Linux device model or making it overly complex. Binding single
drivers to multiple device types could be messy.
Cheers,
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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: Using virtio as a physical (wire-level) transport
From: Michael S. Tsirkin @ 2010-08-05 23:20 UTC (permalink / raw)
To: Ira W. Snyder; +Cc: Rusty Russell, virtualization, Zang Roy, netdev
In-Reply-To: <20100805230102.GD4757@ovro.caltech.edu>
On Thu, Aug 05, 2010 at 04:01:03PM -0700, Ira W. Snyder wrote:
> On Fri, Aug 06, 2010 at 12:30:50AM +0300, Michael S. Tsirkin wrote:
> > Hi Ira,
> >
> > > Making my life harder since the last time I tried this, mainline commit
> > > 7c5e9ed0c (virtio_ring: remove a level of indirection) has removed the
> > > possibility of using an alternative virtqueue implementation. The commit
> > > message suggests that you might be willing to add this capability back.
> > > Would this be an option?
> >
> > Sorry about that.
> >
> > With respect to this commit, we only had one implementation upstream
> > and extra levels of indirection made extending the API
> > much harder for no apparent benefit.
> >
> > When there's more than one ring implementation with very small amount of
> > common code, I think that it might make sense to readd the indirection
> > back, to separate the code cleanly.
> >
> > OTOH if the two implementations share a lot of code, I think that it
> > might be better to just add a couple of if statements here and there.
> > This way compiler even might have a chance to compile the code out if
> > the feature is disabled in kernel config.
> >
>
> The virtqueue implementation I envision will be almost identical to the
> current virtio_ring virtqueue implementation, with the following
> exceptions:
>
> * the "shared memory" will actually be remote, on the PCI BAR of a device
> * iowrite32(), ioread32() and friends will be needed to access the memory
> * there will only be a fixed number of virtqueues available, due to PCI
> BAR size
> * cross-endian virtqueues must work
> * kick needs to be cross-machine (using PCI IRQ's)
>
> I don't think it is feasible to add this to the existing implementation.
> I think the requirement of being cross-endian will be the hardest to
> overcome. Rusty did not envision the cross-endian use case when he
> designed this, and it shows, in virtio_ring, virtio_net and vhost. I
> have no idea what to do about this. Do you have any ideas?
My guess is sticking an if around each access in virtio would hurt,
if this is what you are asking about.
Just a crazy idea: vhost already uses wrappers like get_user etc,
maybe when building kernel for your board you could
redefine these to also byteswap?
--
MST
^ permalink raw reply
* Re: NET_NS: unregister_netdevice: waiting for lo to become free (adding ipv6 address to interface)
From: lkml20100708 @ 2010-08-05 23:45 UTC (permalink / raw)
To: David Miller; +Cc: ebiederm, greg, netdev, linux-kernel, adobriyan, kaber
In-Reply-To: <20100805.131128.42817872.davem@davemloft.net>
On Thu, 05 Aug 2010 13:11:28 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Thu, 05 Aug 2010 12:57:59 -0700
>
> > I wonder what has changed with ipv6 recently.
>
> There was a recent fix to the IGMP snooping code we have in
> the bridging layer, if parsing of an ipv6 IGMP packet failed
> we'd leak the packet (and thus references to whatever device
> it referenced).
>
> commit 6d1d1d398cb7db7a12c5d652d50f85355345234f
[...]
But this patch is not in 2.6.35 and therefore cannot make the
difference Eric sees (belives to see) between his modified 2.6.32 and
2.6.35.
Also, this patch, if I understand that correctly, only changes bridging
and in my scenario bridge.ko (have it as module) was not even loaded,
so applying this patch should not make any difference for the bug I
see, or do I overlook something?
So, I guess, your answer was general information to Erics question what
changed with ipv6, not related to that bug we seek in particular?
--
MfG,
Michael Leun
^ permalink raw reply
* Re: NET_NS: unregister_netdevice: waiting for lo to become free (adding ipv6 address to interface)
From: Michael Leun @ 2010-08-06 0:09 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Greg KH, netdev, davem, linux-kernel, Alexey Dobriyan,
Patrick McHardy
In-Reply-To: <m1bp9g7ry0.fsf@fess.ebiederm.org>
On Thu, 05 Aug 2010 12:57:59 -0700
ebiederm@xmission.com (Eric W. Biederman) wrote:
> What puzzles me is that on a slightly patched 2.6.32 (so sysfs works)
> and I am doing very similar things (openvpn tunnels, ipv6 to the
> network as a whole etc), and I am not seeing the infinite
> unregister_netdevice: messages you are talking about.
Hmmm, I think there are 2 possibilities:
- You send me a patch against plain 2.6.32, so I can check my
scenarios against that kernel
or
- You could try yourself, its really just that few lines against a
fresh booted system in a clean, easy to reproduce state
(Only, if you think that would yield useful information, of course).
> When a network device is removed most references to it are redirected
> to the loopback device so a normal network device should not see the
> worst of the problems. That is why lo showed up.
>
> In that context I'm a bit surprised you managed trigger a problem on
> veth1.
Difference was, when that message showed up with veth1, lo in that
namespace was down while testing. When lo was up it showed up on lo.
--
MfG,
Michael Leun
^ permalink raw reply
* Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
From: Patrick Pannuto @ 2010-08-06 1:25 UTC (permalink / raw)
To: Grant Likely
Cc: Kevin Hilman, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-omap@vger.kernel.org,
damm@opensource.se, lethal@linux-sh.org, rjw@sisk.pl,
eric.y.miao@gmail.com, netdev@vger.kernel.org, Greg Kroah-Hartman,
alan, zt.tmzt, magnus.damm
In-Reply-To: <AANLkTimCS4RV+8E_ZzX97=r5rmQR3nmizp0JYK+syLRa@mail.gmail.com>
On 08/05/2010 04:16 PM, Grant Likely wrote:
> On Thu, Aug 5, 2010 at 9:57 AM, Kevin Hilman
> <khilman@deeprootsystems.com> wrote:
>> Patrick Pannuto <ppannuto@codeaurora.org> writes:
>>
>>> On 08/04/2010 05:16 PM, Kevin Hilman wrote:
>>>> Patrick Pannuto <ppannuto@codeaurora.org> writes:
>>>>
>>>>> Inspiration for this comes from:
>>>>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
>>>>
>>>> Also, later in that thread I also wrote[1] what seems to be the core of
>>>> what you've done here: namely, allow platform_devices and
>>>> platform_drivers to to be used on custom busses. Patch is at the end of
>>>> this mail with a more focused changelog. As Greg suggested in his reply
>>>> to your first version, this part could be merged today, and the
>>>> platform_bus_init stuff could be added later, after some more review.
>>>> Some comments below...
>>>>
>>>
>>> I can split this into 2 patches.
>>
>> Yes, I think that would be better.
>>
>>> Was your patch sent to linux-kernel or just linux-omap? I'm not on linux-omap...
>>
>> That thread was on linux-arm-kernel and linux-omap
>>
>>>
>>>>> [snip]
>>>>>
>>>>> Which will allow the same driver to easily to used on either
>>>>> the platform bus or the newly defined bus type.
>>>>
>>>> Except it requires a re-compile.
>>>>
>>>> Rather than doing this at compile time, it would be better to support
>>>> legacy devices at runtime. You could handle this by simply registering
>>>> the driver on the custom bus and the platform_bus and let the bus
>>>> matching code handle it. Then, the same binary would work on both
>>>> legacy and updated SoCs.
>>>>
>>>
>>> Can you safely register a driver on more than one bus? I didn't think
>>> that was safe -- normally it's impossible since you're calling
>>>
>>> struct BUS_TYPE_driver mydriver;
>>> BUS_TYPE_driver_register(&mydriver)
>>>
>>> but now we have multiple "bus types" that are all actually platform type; still,
>>> at a minimum you would need:
>>> struct platform_driver mydrvier1 = {
>>> .driver.bus = &sub_bus1,
>>> };
>>> struct platform_driver mydrvier2 = {
>>> .driver.bus = &sub_bus2,
>>> };
>>> which would all point to the same driver functions, yet the respective devices
>>> attached for the "same" driver would be on different buses. I fear this might
>>> confuse some drivers. I don't think dynamic bus assignment is this easy
>>>
>>> In short: I do not believe the same driver can be registered on multiple
>>> different buses -- if this is wrong, please correct me.
>>
>> It is possible, and currently done in powerpc land where some
>> drivers handle devices on the platform_bus and the custom OF bus.
>
> As of now, the of_platform_bus_type has been removed. It was a bad
> idea because it tried to encode non-bus-specific information into
> something that was just a clone of the platform_bus. Drivers that
> worked on both had to be bound to both busses. I do actually have
> code that automatically registers a driver on more than one bus, but
> it is rather a hack and was only a temporary measure.
>
> The relevant question before going down this path is, "Is the
> omap/sh/other-soc behaviour something fundamentally different from the
> platform bus? Or is it something complementary that would be better
> handled with a notifier or some orthogonal method of adding new
> behaviour?"
>
> I don't have a problem with multiple platform_bus instances using the
> same code (I did suggest it after all), but I do worry about muddying
> the Linux device model or making it overly complex. Binding single
> drivers to multiple device types could be messy.
>
> Cheers,
> g.
>From your other email, the distinction of /bus_types/ versus /physical
or logical buses/ was very valuable (thanks). I am becoming less
convinced that I need this infrastructure or the ability to create
pseudo-platform buses. Let me outline some thoughts below, which I
think at least solves my problems ( ;) ), and if some of the OMAP folks
and Alan could chime in as to whether they can apply something similar,
or if they still have need of creating actual new bus types?
As we are exploring all of this, let me put a concrete (ish) scenario
out there to discuss:
SUB_BUS1---DEVICE1
/
CPU ---
\
SUB_BUS2---DEVICE2
DEVICE1 and DEVICE2 are the same device (say, usb host controller, or
whatever), and they should be driven by the same driver. However,
physically they are attached to different buses.
SUB_BUS1 and SUB_BUS2 are *different* and let's say require a different
suspend method. If I understand correctly, the right way to build the
topology would be:
struct device_type SUB_BUS1_type = {
.name = "sub-bus1-type",
.pm = &sub_bus1_pm_ops;
};
struct platform_device SUB_BUS1 = {
.init_name = "sub-bus1",
.dev.type = &sub_bus1_type,
};
struct platform_device DEVICE1 = {
.name = "device1",
.dev.parent = &SUB_BUS1.dev,
};
platform_device_register(&SUB_BUS1);
platform_device_register(&DEVICE1);
[analogous for *2]
which would yield:
/sys/bus/platform/devices/
|
|-SUB_BUS1
| |
| |-DEVICE1
|
|-SUB_BUS2
| |
| |-DEVICE2
Which is the correct topology, and logically provides all the correct
interfaces. The only thing that becomes confusing now, is that SUB_BUS*
is *actually* a physically different bus, yet it's not in /sys/bus;
although I suppose this is actually how the world works presently (you
don't see an individual entry in /sys/bus for each usb host controller,
which is technically defining a sub-bus...)
Thoughts? Am I understanding everything correctly?
Is there anything more not covered by co-opting the device-type of SUB_BUS
that people would still need the pseduo-platform-bus extensions for?
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: [rfc 09/13] [RFC 09/13] IPVS: Add persistence engine data to /proc/net/ip_vs_conn
From: Simon Horman @ 2010-08-06 1:45 UTC (permalink / raw)
To: Jan Engelhardt
Cc: lvs-devel, netdev, netfilter, netfilter-devel, Wensong Zhang,
Julian Anastasov, Patrick McHardy
In-Reply-To: <alpine.LSU.2.01.1008051446060.13201@obet.zrqbmnf.qr>
On Thu, Aug 05, 2010 at 02:46:22PM +0200, Jan Engelhardt wrote:
> On Thursday 2010-08-05 13:48, Simon Horman wrote:
>
> >This shouldn't break compatibility with userspace as the new data
> >is at the end of the line.
> >
> >I have confirmed that this doesn't break ipvsadm, the main (only?)
> >user-space user of this data.
>
> Wouldn't it be best if this be done over Netlink?
I'm happy to add code to do that over netlink,
though it does seem to be overkill to me.
^ permalink raw reply
* Re: [rfc 13/13] [RFC 13/13] IPVS: sip persistence engine
From: Simon Horman @ 2010-08-06 1:46 UTC (permalink / raw)
To: Jan Engelhardt
Cc: lvs-devel, netdev, netfilter, netfilter-devel, Wensong Zhang,
Julian Anastasov, Patrick McHardy
In-Reply-To: <alpine.LSU.2.01.1008051447100.13201@obet.zrqbmnf.qr>
On Thu, Aug 05, 2010 at 02:50:11PM +0200, Jan Engelhardt wrote:
> On Thursday 2010-08-05 13:48, Simon Horman wrote:
> >+
> >+static const char *ip_vs_dbg_callid(char *buf, size_t buf_len,
> >+ const char *callid, size_t callid_len,
> >+ int *idx)
> >+{
> >+ size_t len = min(min(callid_len, (size_t)64), buf_len - *idx - 1);
> >+ memcpy(buf + *idx, callid, len);
> >+ *(buf + *idx + len) = '\0';
>
> Nothing particular wrong, though I just noticed that I would have
> personally, instinctively written
>
> buf[*idx+len] = '\0';
>
> instead. Sure you have your reasons ;-)
Actually, I had a bit of trouble writing that code clearly
(though I'm not sure why, perhaps it was late in the day).
I think your suggestion looks good.
> >+ /* RFC 2543 allows lines to be terminated with CR, LF or CRLF,
> >+ * RFC 3261 allows only CRLF, we support both. */
> >+ if (*(dptr + *matchoff + *matchlen) != '\r' &&
> >+ *(dptr + *matchoff + *matchlen) != '\n')
> >+ return -EINVAL;
^ permalink raw reply
* Re: [rfc 10/13] [RFC 10/13] IPVS: management of persistence engine modules
From: Simon Horman @ 2010-08-06 1:48 UTC (permalink / raw)
To: Stephen Hemminger
Cc: lvs-devel, netdev, netfilter, netfilter-devel, Wensong Zhang,
Julian Anastasov, Patrick McHardy
In-Reply-To: <20100805092920.7573c96b@nehalam>
On Thu, Aug 05, 2010 at 09:29:20AM -0700, Stephen Hemminger wrote:
> On Thu, 05 Aug 2010 20:48:05 +0900
> Simon Horman <horms@verge.net.au> wrote:
>
> > +/* lock for service table */
> > +static DEFINE_RWLOCK(__ip_vs_pe_lock);
>
> It is already static so why the __?
> Reader/writer locks are slower than spinlocks. Either use
> a spinlock, or RCU (if possible)
No good reason, other than I copied the code from elsewhere.
I'll fix both this and the code that I copied.
> > +/* Bind a service with a pe */
> > +void ip_vs_bind_pe(struct ip_vs_service *svc, struct ip_vs_pe *pe)
> > +{
> > + svc->pe = pe;
> > +}
> > +
> > +/* Unbind a service from its pe */
> > +void ip_vs_unbind_pe(struct ip_vs_service *svc)
> > +{
> > + svc->pe = NULL;
> > +}
>
> What does having these wrappers buy?
Again, I copied the code from elsewhere, where the wrappers did more. As
it happens, I think that these do make some sense as they are called along
side other bind and unbind calls. But if you have a strong aversion to them
then I am happy to remove these wrappers.
^ permalink raw reply
* Re: [GIT] Networking
From: Yinghai Lu @ 2010-08-06 2:04 UTC (permalink / raw)
To: Linus Torvalds; +Cc: David Miller, akpm, netdev, linux-kernel
In-Reply-To: <AANLkTimxv1y3QJ7s+ghi1FFCcwnUVWT1n6dvT-NNN=Oo@mail.gmail.com>
On Wed, Aug 4, 2010 at 12:06 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Aug 3, 2010 at 8:38 PM, David Miller <davem@davemloft.net> wrote:
>>
>> Another release, another merge window, another set of networking
>> changes to merge :-)
>
> Ok, merged. But you should double-check my merge resolution fixes,
CC drivers/net/ixgbe/ixgbe_main.o
drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_select_queue’:
drivers/net/ixgbe/ixgbe_main.c:6159: error: ‘struct ixgbe_fcoe’ has no
member named ‘up’
drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_xmit_frame’:
drivers/net/ixgbe/ixgbe_main.c:6221: error: ‘struct ixgbe_fcoe’ has no
member named ‘up’
make[1]: *** [drivers/net/ixgbe/ixgbe_main.o] Error 1
make: *** [drivers/net/ixgbe/] Error 2
YH
^ permalink raw reply
* Re: [GIT] Networking
From: Linus Torvalds @ 2010-08-06 2:15 UTC (permalink / raw)
To: Yinghai Lu; +Cc: David Miller, akpm, netdev, linux-kernel
In-Reply-To: <AANLkTinU4+tcm2O_BVBFkHSdnx7JEPtM4sw_s--qdUKC@mail.gmail.com>
On Thu, Aug 5, 2010 at 7:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>
> CC drivers/net/ixgbe/ixgbe_main.o
> drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_select_queue’:
> drivers/net/ixgbe/ixgbe_main.c:6159: error: ‘struct ixgbe_fcoe’ has no member named ‘up’
Hmm. You must have some other version of that file than I do. Line
6159 is in ixgbe_init_module() in my source tree.
Linus
^ permalink raw reply
* Re: [GIT] Networking
From: Yinghai Lu @ 2010-08-06 4:49 UTC (permalink / raw)
To: Linus Torvalds; +Cc: David Miller, akpm, netdev, linux-kernel
In-Reply-To: <AANLkTi=3dVWKtgL2A=4xm_1VSk64--3Zk_fQBYUdAAn+@mail.gmail.com>
On 08/05/2010 07:15 PM, Linus Torvalds wrote:
> On Thu, Aug 5, 2010 at 7:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>
>> CC drivers/net/ixgbe/ixgbe_main.o
>> drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_select_queue’:
>> drivers/net/ixgbe/ixgbe_main.c:6159: error: ‘struct ixgbe_fcoe’ has no member named ‘up’
>
> Hmm. You must have some other version of that file than I do. Line
> 6159 is in ixgbe_init_module() in my source tree.
>
> Linus
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/net/ixgbe/ixgbe_main.c;h=7d6a415bcf885633999e10ed795df49876d6c47e;hb=HEAD
6146 static u16 ixgbe_select_queue(struct net_device *dev, struct sk_buff *skb)
6147 {
6148 struct ixgbe_adapter *adapter = netdev_priv(dev);
6149 int txq = smp_processor_id();
6150
6151 #ifdef IXGBE_FCOE
6152 if ((skb->protocol == htons(ETH_P_FCOE)) ||
6153 (skb->protocol == htons(ETH_P_FIP))) {
6154 if (adapter->flags & IXGBE_FLAG_FCOE_ENABLED) {
6155 txq &= (adapter->ring_feature[RING_F_FCOE].indices - 1);
6156 txq += adapter->ring_feature[RING_F_FCOE].mask;
6157 return txq;
6158 } else if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
6159 txq = adapter->fcoe.up;
6160 return txq;
6161 }
6162 }
6163 #endif
^ permalink raw reply
* Re: [GIT] Networking
From: Yinghai Lu @ 2010-08-06 4:58 UTC (permalink / raw)
To: Linus Torvalds; +Cc: David Miller, akpm, netdev, linux-kernel
In-Reply-To: <AANLkTi=3dVWKtgL2A=4xm_1VSk64--3Zk_fQBYUdAAn+@mail.gmail.com>
On 08/05/2010 07:15 PM, Linus Torvalds wrote:
> On Thu, Aug 5, 2010 at 7:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>
>> CC drivers/net/ixgbe/ixgbe_main.o
>> drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_select_queue’:
>> drivers/net/ixgbe/ixgbe_main.c:6159: error: ‘struct ixgbe_fcoe’ has no member named ‘up’
>
> Hmm. You must have some other version of that file than I do. Line
> 6159 is in ixgbe_init_module() in my source tree.
>
http://patchwork.ozlabs.org/patch/60777/
Yinghai
^ permalink raw reply
* Re: [GIT] Networking
From: David Miller @ 2010-08-06 5:19 UTC (permalink / raw)
To: yinghai; +Cc: torvalds, akpm, netdev, linux-kernel
In-Reply-To: <4C5B965F.4020109@kernel.org>
We know about this FCOE build issue, it's config specific, and the
Intel folks have a patch coming to me tonight that will fix this up.
Thanks.
^ permalink raw reply
* Re: e1000e crashes with 2.6.34.x and ThinkPad T60
From: Chuck Ebbert @ 2010-08-06 5:46 UTC (permalink / raw)
To: Marc Haber
Cc: e1000-devel@lists.sourceforge.net,
Linux Kernel Network Developers, Allan, Bruce W,
Linux Kernel Developers
In-Reply-To: <20100730125614.GB21444@torres.zugschlus.de>
On Fri, 30 Jul 2010 14:56:14 +0200
Marc Haber <mh+linux-kernel@zugschlus.de> wrote:
> Capabilities: <access denied>
That's not very useful.
The pcie capabilities are completely missing.
------------------------------------------------------------------------------
This SF.net email is sponsored by
Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* [PATCH] phy: SMSC: fix unused warnings when built into kernel
From: Mike Frysinger @ 2010-08-06 6:27 UTC (permalink / raw)
To: netdev, David S. Miller
If the SMSC phy is not built as a module, we get the warning:
drivers/net/phy/smsc.c:257: warning: 'smsc_tbl' defined but not used
So mark the structure as maybe unused to avoid this.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
drivers/net/phy/smsc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 78fa988..11944ef 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -254,7 +254,7 @@ MODULE_LICENSE("GPL");
module_init(smsc_init);
module_exit(smsc_exit);
-static struct mdio_device_id smsc_tbl[] = {
+static __maybe_unused struct mdio_device_id smsc_tbl[] = {
{ 0x0007c0a0, 0xfffffff0 },
{ 0x0007c0b0, 0xfffffff0 },
{ 0x0007c0c0, 0xfffffff0 },
--
1.7.2
^ permalink raw reply related
* Re: [PATCH] phy: SMSC: fix unused warnings when built into kernel
From: Changli Gao @ 2010-08-06 6:43 UTC (permalink / raw)
To: Mike Frysinger; +Cc: netdev, David S. Miller
In-Reply-To: <1281076034-17390-1-git-send-email-vapier@gentoo.org>
On Fri, Aug 6, 2010 at 2:27 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> If the SMSC phy is not built as a module, we get the warning:
> drivers/net/phy/smsc.c:257: warning: 'smsc_tbl' defined but not used
>
> So mark the structure as maybe unused to avoid this.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> drivers/net/phy/smsc.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
> index 78fa988..11944ef 100644
> --- a/drivers/net/phy/smsc.c
> +++ b/drivers/net/phy/smsc.c
> @@ -254,7 +254,7 @@ MODULE_LICENSE("GPL");
> module_init(smsc_init);
> module_exit(smsc_exit);
>
> -static struct mdio_device_id smsc_tbl[] = {
> +static __maybe_unused struct mdio_device_id smsc_tbl[] = {
> { 0x0007c0a0, 0xfffffff0 },
> { 0x0007c0b0, 0xfffffff0 },
> { 0x0007c0c0, 0xfffffff0 },
How about adding a #ifdef CONFIG_SMSC_PHY_MODULE clause around these lines?
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH 32/42] drivers/net/bnx2x: Adjust confusing if indentation
From: Dan Carpenter @ 2010-08-06 7:07 UTC (permalink / raw)
To: Julia Lawall; +Cc: Eilon Greenstein, netdev, linux-kernel, kernel-janitors
In-Reply-To: <Pine.LNX.4.64.1008052226210.31692@ask.diku.dk>
On Thu, Aug 05, 2010 at 10:26:38PM +0200, Julia Lawall wrote:
> ---
> This patch doesn't change the semantics of the code. But it might not be
> what is intended.
>
I think there may have been some if statements which were removed before
the code was merged into the kernel? Here is another one from that same
function. Someone can roll this in with your patch.
regards,
dan carpenter
diff --git a/drivers/net/bnx2x/bnx2x_link.c b/drivers/net/bnx2x/bnx2x_link.c
index 0fa0991..ca0e435 100644
--- a/drivers/net/bnx2x/bnx2x_link.c
+++ b/drivers/net/bnx2x/bnx2x_link.c
@@ -1242,11 +1242,11 @@ static void bnx2x_set_autoneg(struct link_params *params,
MDIO_CL73_USERB0_CL73_BAM_CTRL1_BAM_NP_AFTER_BP_EN);
/* Advertise CL73 link speeds */
- CL45_RD_OVER_CL22(bp, params->port,
- params->phy_addr,
- MDIO_REG_BANK_CL73_IEEEB1,
- MDIO_CL73_IEEEB1_AN_ADV2,
- ®_val);
+ CL45_RD_OVER_CL22(bp, params->port,
+ params->phy_addr,
+ MDIO_REG_BANK_CL73_IEEEB1,
+ MDIO_CL73_IEEEB1_AN_ADV2,
+ ®_val);
if (params->speed_cap_mask &
PORT_HW_CFG_SPEED_CAPABILITY_D0_10G)
reg_val |= MDIO_CL73_IEEEB1_AN_ADV2_ADVR_10G_KX4;
^ permalink raw reply related
* Re: [PATCH] phy: SMSC: fix unused warnings when built into kernel
From: Mike Frysinger @ 2010-08-06 7:37 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev, David S. Miller
In-Reply-To: <AANLkTi=_-ZZ8nyFLiBKSWNTcM_SkwLe2-ZL17zBQb0i4@mail.gmail.com>
[-- Attachment #1: Type: Text/Plain, Size: 1125 bytes --]
On Friday, August 06, 2010 02:43:46 Changli Gao wrote:
> On Fri, Aug 6, 2010 at 2:27 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > If the SMSC phy is not built as a module, we get the warning:
> > drivers/net/phy/smsc.c:257: warning: 'smsc_tbl' defined but not
> > used
> >
> > So mark the structure as maybe unused to avoid this.
> >
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > ---
> > drivers/net/phy/smsc.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
> > index 78fa988..11944ef 100644
> > --- a/drivers/net/phy/smsc.c
> > +++ b/drivers/net/phy/smsc.c
> > @@ -254,7 +254,7 @@ MODULE_LICENSE("GPL");
> > module_init(smsc_init);
> > module_exit(smsc_exit);
> >
> > -static struct mdio_device_id smsc_tbl[] = {
> > +static __maybe_unused struct mdio_device_id smsc_tbl[] = {
> > { 0x0007c0a0, 0xfffffff0 },
> > { 0x0007c0b0, 0xfffffff0 },
> > { 0x0007c0c0, 0xfffffff0 },
>
> How about adding a #ifdef CONFIG_SMSC_PHY_MODULE clause around these
> lines?
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] phy: SMSC: fix unused warnings when built into kernel
From: Mike Frysinger @ 2010-08-06 7:37 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev, David S. Miller
In-Reply-To: <AANLkTi=_-ZZ8nyFLiBKSWNTcM_SkwLe2-ZL17zBQb0i4@mail.gmail.com>
[-- Attachment #1: Type: Text/Plain, Size: 882 bytes --]
On Friday, August 06, 2010 02:43:46 Changli Gao wrote:
> On Fri, Aug 6, 2010 at 2:27 PM, Mike Frysinger wrote:
> > If the SMSC phy is not built as a module, we get the warning:
> > drivers/net/phy/smsc.c:257: warning: 'smsc_tbl' defined but not
> > used
> >
> > So mark the structure as maybe unused to avoid this.
> >
> > --- a/drivers/net/phy/smsc.c
> > +++ b/drivers/net/phy/smsc.c
> > @@ -254,7 +254,7 @@ MODULE_LICENSE("GPL");
> > module_init(smsc_init);
> > module_exit(smsc_exit);
> >
> > -static struct mdio_device_id smsc_tbl[] = {
> > +static __maybe_unused struct mdio_device_id smsc_tbl[] = {
> > { 0x0007c0a0, 0xfffffff0 },
> > { 0x0007c0b0, 0xfffffff0 },
> > { 0x0007c0c0, 0xfffffff0 },
>
> How about adding a #ifdef CONFIG_SMSC_PHY_MODULE clause around these
> lines?
imo, the less ifdefs the better
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] netfilter: xt_condition: change the value from boolean to u32
From: Luciano Coelho @ 2010-08-06 8:00 UTC (permalink / raw)
To: ext Jan Engelhardt
Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
kaber@trash.net, sameo@linux.intel.com
In-Reply-To: <alpine.LSU.2.01.1008051705320.1104@obet.zrqbmnf.qr>
On Thu, 2010-08-05 at 17:12 +0200, ext Jan Engelhardt wrote:
> On Thursday 2010-08-05 16:41, luciano.coelho@nokia.com wrote:
>
> > struct xt_condition_mtinfo {
> >- char name[31];
> >+ char name[27];
> > __u8 invert;
> >+ __u32 value;
>
> Please also bump the .revision field to 2 with this patch so that
> testing can always proceed without an ABI clash.
> (rev 2 would then remain over the course of the remaining patches
> you submit.)
> (rev 0 = ipt_condition/pom-ng; rev 1 = xt_condition from Xt-a)
Oh, did I forget that? I thought I had bumped it up in my previous patch
after your comment, but apparently I didn't. I'll fix it.
> >+ char buf[14];
>
> char buf[sizeof("4294967296")];
>
> seems more intuitive :-)
I'm using base 0 in the strtoull call, so I need at least 14 chars to
accommodate octals. In octal MAX_UINT takes 11 digits, plus the leading
0 that marks it as an octal, plus the optional + sign (not sure if this
one makes sense, though), plus the null-terminator (or actually the \n
when writing to procfs) leads to 14. But I'm not sure whether:
char buf[sizeof("+037777777777")];
Would be very intuitive? Surely I don't like the magic number, so I
should probably change it anyway.
> >+ unsigned long long value;
> >+
> >+ if (length == 0)
> >+ return 0;
> >+
> >+ if (length > sizeof(buf))
> >+ return -EINVAL;
> >+
> >+ if (copy_from_user(buf, input, length) != 0)
> >+ return -EFAULT;
> >+
> >+ buf[length - 1] = '\0';
> >+
> >+ if (strict_strtoull(buf, 0, &value) != 0)
> >+ return -EINVAL;
> >+
> >+ if (value > (u32) value)
> >+ return -EINVAL;
>
> Is it possible to use just strict_strtoul?
Not easily. I found that there is a bug in strtoul (and strtoull for
that matter) that causes the long to overflow if there are valid digits
after the maximum possible digits for the base. For example if you try
to strtoul 0xfffffffff (with 9 f's) the strtoul will overflow and come
up with a bogus result. I can't easily truncate the string to avoid
this problem, because with decimal or octal, the same valid value would
take more spaces. I could do some magic here, checking whether it's a
hex, dec or oct and truncate appropriately, but that would be very ugly.
So the simplest way I came up with was to use strtoull and return
-EINVAL if the value exceeds 32 bits. ;)
> >- return var->enabled ^ info->invert;
> >+ return (var->value == info->value) ^ info->invert;
>
> Since the condition value (cdmark) was thought of an nfmark-style thing,
> would it perhaps make sense to model it after it
>
> return (var->value & ~info->mask) ^ info->value;
>
> Other opinions?
I think it's nicer to have it as a normal equals here for now and then
extend the match with more operations. We can later add, for example,
an --and option to the condition match in order to do other kinds of
binary operations. It would be more flexible this way because we could
use several different types of comparisons, wouldn't it? And in the
target we could have several different types of operations.
--
Cheers,
Luca.
^ 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