* Re: PATCH: Network Device Naming mechanism and policy
From: Bryan Kadzban @ 2009-10-10 17:34 UTC (permalink / raw)
To: Greg KH
Cc: Matt Domsch, Stephen Hemminger, netdev, linux-hotplug, Narendra_K,
jordan_hargrave
In-Reply-To: <20091010162518.GA30354@kroah.com>
[-- Attachment #1: Type: text/plain, Size: 3218 bytes --]
Greg KH wrote:
> On Sat, Oct 10, 2009 at 07:47:32AM -0500, Matt Domsch wrote:
>> On Fri, Oct 09, 2009 at 10:23:08PM -0700, Greg KH wrote:
>>> On Fri, Oct 09, 2009 at 11:40:57PM -0500, Matt Domsch wrote:
>>>> The fundamental roadblock to this is that enumeration !=
>>>> naming, except that it is for network devices, and we keep
>>>> changing the enumeration order.
>>> No, the hardware changes the enumeration order, it places _no_
>>> guarantees on what order stuff will be found in. So this is not
>>> the kernel changing, just to be clear.
>> Over time the kernel has changed its enumeration mechanisms, and
>> introduced parallelism into the process (which is a good thing),
>> which, from a user perspective, makes names nondeterministic. Yes,
>> fixing this up by hard-coding MAC addresses after install has been
>> the traditional mechanism to address this. I think there's a
>> better way.
>
> Ok, but that way can be done in userspace, without the need for this
> char device, right?
For the record -- when I tried to send a patch that did exactly this
(provided an option to use by-path persistence for network drivers), it
was rejected because "that doesn't work for USB".
True, it doesn't. But by-mac (what we have today) doesn't work for
replacing motherboards in a random home system (that can't override the
MAC address in the BIOS), either.
So why not provide both alternatives?
As you say below, it's up to the network devs whether this should be
allowed...
>> biosdevname can be used in udev rules to create multiple names for
>> a given device. Rules such as:
>
> Yes, if you want multiple ways to name a network device, then you
> need the char nodes. But without that, you can just pick "always use
> the biosdevname" type option from your distro setup screen and go
> with that. Then you have everything always working properly from the
> very beginning.
*If* biosdevname works on your system. It doesn't on mine: this SMBIOS
extension doesn't exist. :-)
> So you really want this for multiple ways to name the same network
> device. That's a choice the network developers are going to have to
> make, as to if that is going to be a legal thing to have happen or
> not.
Yes. So do I, actually (for what little that's worth)...
> But this code is not a requirement to "solve" the fact that network
> devices can show up in different order, that problem can be solved as
> long as the user picks a single way to name the devices, using tools
> that are already present today in distros.
This code is not a requirement, no. But -- as you say -- it does
provide a halfway-decent way to assign multiple names to a NIC. And
that provides admins the choice to use a couple different persistence
schemes, depending on how they expect their hardware to work.
(It *may* even be possible to use some kind of layer-2 traffic to see
what else is on the connected network and provide symlinks based on
that. IPv6 autoconfig type of thing, maybe. That's probably a *lot*
more complicated, and may be impossible, but would be even closer to
what I think Dell customers are asking for based on Matt's posts.)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] iwmc3200top: Add Intel Wireless MultiCom 3200 top driver.
From: Tomas Winkler @ 2009-10-10 18:05 UTC (permalink / raw)
To: Marcel Holtmann
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, linville-2XuSBdqkA4R54TAoqtyWWQ,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-mmc-u79uwXL29TY76Z2rM5mHXA, yi.zhu-ral2JQCrhuEAvxtiuMwx3w,
inaky.perez-gonzalez-ral2JQCrhuEAvxtiuMwx3w,
cindy.h.kao-ral2JQCrhuEAvxtiuMwx3w,
guy.cohen-ral2JQCrhuEAvxtiuMwx3w,
ron.rindjunsky-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <1255172508.19127.22.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
On Sat, Oct 10, 2009 at 1:01 PM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> Hi Tomas,
>
>> This patch adds Intel Wireless MultiCom 3200 top driver.
>> IWMC3200 is 4Wireless Com CHIP (GPS/BT/WiFi/WiMAX).
>> Top driver is responsible for device initialization and firmware download.
>> Firmware handled by top is responsible for top itself and
>> as well as bluetooth and GPS coms. (Wifi and WiMax provide their own
>> firmware)
>> In addition top driver is used to retrieve firmware logs
>> and supports other debugging features
>>
>> Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> ---
>> drivers/misc/Kconfig | 1 +
>> drivers/misc/Makefile | 1 +
>> drivers/misc/iwmc3200top/Kconfig | 20 +
>> drivers/misc/iwmc3200top/Makefile | 29 ++
>> drivers/misc/iwmc3200top/debugfs.c | 145 +++++++
>> drivers/misc/iwmc3200top/debugfs.h | 61 +++
>> drivers/misc/iwmc3200top/fw-download.c | 359 ++++++++++++++++
>> drivers/misc/iwmc3200top/fw-msg.h | 113 +++++
>> drivers/misc/iwmc3200top/iwmc3200top.h | 202 +++++++++
>> drivers/misc/iwmc3200top/log.c | 339 +++++++++++++++
>> drivers/misc/iwmc3200top/log.h | 158 +++++++
>> drivers/misc/iwmc3200top/main.c | 702 ++++++++++++++++++++++++++++++++
>> 12 files changed, 2130 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/misc/iwmc3200top/Kconfig
>> create mode 100644 drivers/misc/iwmc3200top/Makefile
>> create mode 100644 drivers/misc/iwmc3200top/debugfs.c
>> create mode 100644 drivers/misc/iwmc3200top/debugfs.h
>> create mode 100644 drivers/misc/iwmc3200top/fw-download.c
>> create mode 100644 drivers/misc/iwmc3200top/fw-msg.h
>> create mode 100644 drivers/misc/iwmc3200top/iwmc3200top.h
>> create mode 100644 drivers/misc/iwmc3200top/log.c
>> create mode 100644 drivers/misc/iwmc3200top/log.h
>> create mode 100644 drivers/misc/iwmc3200top/main.c
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 68ab39d..6f85b43 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -236,5 +236,6 @@ config ISL29003
>> source "drivers/misc/c2port/Kconfig"
>> source "drivers/misc/eeprom/Kconfig"
>> source "drivers/misc/cb710/Kconfig"
>> +source "drivers/misc/iwmc3200top/Kconfig"
>>
>> endif # MISC_DEVICES
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 36f733c..97f5303 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -20,5 +20,6 @@ obj-$(CONFIG_SGI_GRU) += sgi-gru/
>> obj-$(CONFIG_HP_ILO) += hpilo.o
>> obj-$(CONFIG_ISL29003) += isl29003.o
>> obj-$(CONFIG_C2PORT) += c2port/
>> +obj-$(CONFIG_IWMC3200TOP) += iwmc3200top/
>> obj-y += eeprom/
>> obj-y += cb710/
>> diff --git a/drivers/misc/iwmc3200top/Kconfig b/drivers/misc/iwmc3200top/Kconfig
>> new file mode 100644
>> index 0000000..9e4b88f
>> --- /dev/null
>> +++ b/drivers/misc/iwmc3200top/Kconfig
>> @@ -0,0 +1,20 @@
>> +config IWMC3200TOP
>> + tristate "Intel Wireless MultiCom Top Driver"
>> + depends on MMC && EXPERIMENTAL
>> + select FW_LOADER
>> + ---help---
>> + Intel Wireless MultiCom 3200 Top driver is responsible for
>> + for firmware load and enabled coms enumeration
>> +
>> +config IWMC3200TOP_DEBUG
>> + bool "Enable full debug output of iwmc3200top Driver"
>> + depends on IWMC3200TOP
>> + ---help---
>> + Enable full debug output of iwmc3200top Driver
>> +
>> +config IWMC3200TOP_DEBUGFS
>> + bool "Enable Debugfs debugging interface for iwmc3200top"
>> + depends on IWMC3200TOP
>> + ---help---
>> + Enable creation of debugfs files for iwmc3200top
>> +
>> diff --git a/drivers/misc/iwmc3200top/Makefile b/drivers/misc/iwmc3200top/Makefile
>> new file mode 100644
>> index 0000000..fbf53fb
>> --- /dev/null
>> +++ b/drivers/misc/iwmc3200top/Makefile
>> @@ -0,0 +1,29 @@
>> +# iwmc3200top - Intel Wireless MultiCom 3200 Top Driver
>> +# drivers/misc/iwmc3200top/Makefile
>> +#
>> +# Copyright (C) 2009 Intel Corporation. All rights reserved.
>> +#
>> +# This program is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU General Public License version
>> +# 2 as published by the Free Software Foundation.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, write to the Free Software
>> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> +# 02110-1301, USA.
>> +#
>> +#
>> +# Author Name: Maxim Grabarnik <maxim.grabarnink-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> +# -
>> +#
>> +#
>> +
>> +obj-$(CONFIG_IWMC3200TOP) += iwmc3200top.o
>> +iwmc3200top-objs := main.o fw-download.o
>> +iwmc3200top-$(CONFIG_IWMC3200TOP_DEBUG) += log.o
>> +iwmc3200top-$(CONFIG_IWMC3200TOP_DEBUGFS) += debugfs.o
>> diff --git a/drivers/misc/iwmc3200top/debugfs.c b/drivers/misc/iwmc3200top/debugfs.c
>> new file mode 100644
>> index 0000000..a531f6c
>> --- /dev/null
>> +++ b/drivers/misc/iwmc3200top/debugfs.c
>> @@ -0,0 +1,145 @@
>> +/*
>> + * iwmc3200top - Intel Wireless MultiCom 3200 Top Driver
>> + * drivers/misc/iwmc3200top/debufs.c
>> + *
>> + * Copyright (C) 2009 Intel Corporation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version
>> + * 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> + * 02110-1301, USA.
>> + *
>> + *
>> + * Author Name: Maxim Grabarnik <maxim.grabarnink-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> + * -
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/string.h>
>> +#include <linux/ctype.h>
>> +#include <linux/mmc/sdio_func.h>
>> +#include <linux/mmc/sdio.h>
>> +
>> +#include "iwmc3200top.h"
>> +#include "log.h"
>> +#include "fw-msg.h"
>> +#include "debugfs.h"
>> +
>> +/* Constants definition */
>> +#define HEXADECIMAL_RADIX 16
>> +
>> +/* Functions definition */
>> +
>> +
>> +/* Debugfs macros */
>> +#define DEBUGFS_ADD_DIR(name, parent) do { \
>> + dbgfs->dir_##name = debugfs_create_dir(#name, parent); \
>> + if (!(dbgfs->dir_##name)) \
>> + goto err; \
>> +} while (0)
>> +
>> +#define DEBUGFS_ADD_FILE(name, parent) do { \
>> + dbgfs->dbgfs_##parent##_files.file_##name = \
>> + debugfs_create_file(#name, 0644, dbgfs->dir_##parent, priv, \
>> + &iwmct_dbgfs_##name##_ops); \
>> + if (!(dbgfs->dbgfs_##parent##_files.file_##name)) \
>> + goto err; \
>> +} while (0)
>> +
>> +#define DEBUGFS_REMOVE(name) do { \
>> + debugfs_remove(name); \
>> + name = NULL; \
>> +} while (0)
>> +
>> +#define DEBUGFS_READ_FUNC(name) \
>> +ssize_t iwmct_dbgfs_##name##_read(struct file *file, \
>> + char __user *user_buf, \
>> + size_t count, loff_t *ppos);
>> +
>> +#define DEBUGFS_WRITE_FUNC(name) \
>> +ssize_t iwmct_dbgfs_##name##_write(struct file *file, \
>> + const char __user *user_buf, \
>> + size_t count, loff_t *ppos);
>> +
>> +#define DEBUGFS_READ_FILE_OPS(name) \
>> + DEBUGFS_READ_FUNC(name) \
>> + static const struct file_operations iwmct_dbgfs_##name##_ops = { \
>> + .read = iwmct_dbgfs_##name##_read, \
>> + .open = iwmct_dbgfs_open_file_generic, \
>> + };
>> +
>> +#define DEBUGFS_WRITE_FILE_OPS(name) \
>> + DEBUGFS_WRITE_FUNC(name) \
>> + static const struct file_operations iwmct_dbgfs_##name##_ops = { \
>> + .write = iwmct_dbgfs_##name##_write, \
>> + .open = iwmct_dbgfs_open_file_generic, \
>> + };
>> +
>> +#define DEBUGFS_READ_WRITE_FILE_OPS(name) \
>> + DEBUGFS_READ_FUNC(name) \
>> + DEBUGFS_WRITE_FUNC(name) \
>> + static const struct file_operations iwmct_dbgfs_##name##_ops = {\
>> + .write = iwmct_dbgfs_##name##_write, \
>> + .read = iwmct_dbgfs_##name##_read, \
>> + .open = iwmct_dbgfs_open_file_generic, \
>> + };
>> +
>> +
>> +/* Debugfs file ops definitions */
>> +
>> +/* Debugfs registration functions */
>> +
>> +/*
>> + * Create the debugfs files and directories
>> + *
>> + */
>
> so for me the whole debug infrastructure is way too much overhead for
> what this driver has to achieve. What is the benefit of having all these
> debugfs support. What is it trying to achieve?
There is some testing infrastracture built erround debugfs, it
exericeses the firmware. There is no use for
it outside our development so we ripped it off but removing also
debugfs will nightmare to maintain.
>
> Also all these macros really obfuscate the code for no real benefit. If
> you wanna keep debugfs support, then just use the functions directly.
> They are not that complicated that an extra abstraction layer is needed.
>
I believe the macros were borrowed from wireless code. They shalll
reduce codelines
but I have no strong feelings about them
Thanks for your comments I will address them in consequitve patch.
Thanks
Tomas
>> +void iwmct_dbgfs_register(struct iwmct_priv *priv, const char *name)
>> +{
>> + struct iwmct_debugfs *dbgfs;
>> +
>> + dbgfs = kzalloc(sizeof(struct iwmct_debugfs), GFP_KERNEL);
>> + if (!dbgfs) {
>> + LOG_ERROR(priv, DEBUGFS, "failed to allocate %zd bytes\n",
>> + sizeof(struct iwmct_debugfs));
>> + goto err;
>> + }
>> +
>> + priv->dbgfs = dbgfs;
>> + dbgfs->name = name;
>> + dbgfs->dir_drv = debugfs_create_dir(name, NULL);
>> + if (!dbgfs->dir_drv) {
>> + LOG_ERROR(priv, DEBUGFS, "failed to create debugfs dir\n");
>> + goto err;
>> + }
>> +
>> +
>> +err:
>> + return;
>> +}
>
> goto err. Seriously. Just call return in the error case. And also there
> is a memory leak if the debugfs_create_dir fails.
>
>> +/**
>> + * Remove the debugfs files and directories
>> + *
>> + */
>> +void iwmct_dbgfs_unregister(struct iwmct_debugfs *dbgfs)
>> +{
>> + if (!dbgfs)
>> + return;
>> +
>> +
>> + DEBUGFS_REMOVE(dbgfs->dir_drv);
>> + kfree(dbgfs);
>> +
>> + dbgfs = NULL;
>> +}
>> +
>> diff --git a/drivers/misc/iwmc3200top/debugfs.h b/drivers/misc/iwmc3200top/debugfs.h
>> new file mode 100644
>> index 0000000..24b1369
>> --- /dev/null
>> +++ b/drivers/misc/iwmc3200top/debugfs.h
>> @@ -0,0 +1,61 @@
>> +/*
>> + * iwmc3200top - Intel Wireless MultiCom 3200 Top Driver
>> + * drivers/misc/iwmc3200top/debufs.h
>> + *
>> + * Copyright (C) 2009 Intel Corporation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version
>> + * 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> + * 02110-1301, USA.
>> + *
>> + *
>> + * Author Name: Maxim Grabarnik <maxim.grabarnink-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> + * -
>> + *
>> + */
>> +
>> +#ifndef __DEBUGFS_H__
>> +#define __DEBUGFS_H__
>> +
>> +
>> +#ifdef CONFIG_IWMC3200TOP_DEBUGFS
>> +
>> +#include <linux/debugfs.h>
>> +
>> +struct iwmct_debugfs {
>> + const char *name;
>> + struct dentry *dir_drv;
>> + struct dir_drv_files {
>> + } dbgfs_drv_files;
>> +};
>> +
>> +void iwmct_dbgfs_register(struct iwmct_priv *priv, const char *name);
>> +void iwmct_dbgfs_unregister(struct iwmct_debugfs *dbgfs);
>> +
>> +#else /* CONFIG_IWMC3200TOP_DEBUGFS */
>> +
>> +/* struct iwmct_debugfs is empty if CONFIG_IWMC3200TOP_DEBUGFS is not defined */
>> +struct iwmct_debugfs {
>> +};
>
> If struct iwmct_debugfs is never referenced directly then a simple
> forward struct imwct_debugfs; declaration is good enough.
>
>> +
>> +static inline void
>> +iwmct_dbgfs_register(struct iwmct_priv *priv, const char *name)
>> +{}
>> +
>> +static inline void iwmct_dbgfs_unregister(struct iwmct_debugfs *dbgfs)
>> +{}
>> +
>> +#endif /* CONFIG_IWMC3200TOP_DEBUGFS */
>> +
>> +#endif /* __DEBUGFS_H__ */
>> +
>> diff --git a/drivers/misc/iwmc3200top/fw-download.c b/drivers/misc/iwmc3200top/fw-download.c
>> new file mode 100644
>> index 0000000..33cb693
>> --- /dev/null
>> +++ b/drivers/misc/iwmc3200top/fw-download.c
>> @@ -0,0 +1,359 @@
>> +/*
>> + * iwmc3200top - Intel Wireless MultiCom 3200 Top Driver
>> + * drivers/misc/iwmc3200top/fw-download.c
>> + *
>> + * Copyright (C) 2009 Intel Corporation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version
>> + * 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> + * 02110-1301, USA.
>> + *
>> + *
>> + * Author Name: Maxim Grabarnik <maxim.grabarnink-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> + * -
>> + *
>> + */
>> +
>> +#include <linux/firmware.h>
>> +#include <linux/mmc/sdio_func.h>
>> +#include <asm/unaligned.h>
>> +
>> +#include "iwmc3200top.h"
>> +#include "log.h"
>> +#include "fw-msg.h"
>> +
>> +#define CHECKSUM_BYTES_NUM sizeof(u32)
>> +
>> +/**
>> + init parser struct with file
>> + */
>> +static int iwmct_fw_parser_init(struct iwmct_priv *priv, const u8 *file,
>> + size_t file_size, size_t block_size)
>> +{
>> + struct iwmct_parser *parser = &priv->parser;
>> + struct iwmct_fw_hdr *fw_hdr = &parser->versions;
>> +
>> + LOG_INFOEX(priv, INIT, "-->\n");
>> +
>> + LOG_INFO(priv, FW_DOWNLOAD, "file_size=%zd\n", file_size);
>
> This logging is overkill. Can we just hide this via pr_debug and then
> use just simply dynamic_printk support.
>
>> +
>> + parser->file = file;
>> + parser->file_size = file_size;
>> + parser->cur_pos = 0;
>> + parser->buf = NULL;
>> +
>> + parser->buf = kzalloc(block_size, GFP_KERNEL);
>> + if (!parser->buf) {
>> + LOG_ERROR(priv, FW_DOWNLOAD, "kzalloc error\n");
>> + return -ENOMEM;
>> + }
>> + parser->buf_size = block_size;
>> +
>> + /* extract fw versions */
>> + memcpy(fw_hdr, parser->file, sizeof(struct iwmct_fw_hdr));
>> + LOG_INFO(priv, FW_DOWNLOAD, "fw versions are:\n"
>> + "top %u.%u.%u gps %u.%u.%u bt %u.%u.%u tic %s\n",
>> + fw_hdr->top_major, fw_hdr->top_minor, fw_hdr->top_revision,
>> + fw_hdr->gps_major, fw_hdr->gps_minor, fw_hdr->gps_revision,
>> + fw_hdr->bt_major, fw_hdr->bt_minor, fw_hdr->bt_revision,
>> + fw_hdr->tic_name);
>> +
>> + parser->cur_pos += sizeof(struct iwmct_fw_hdr);
>> +
>> + LOG_INFOEX(priv, INIT, "<--\n");
>> + return 0;
>> +}
>> +
>> +static bool iwmct_checksum(struct iwmct_priv *priv)
>> +{
>> + struct iwmct_parser *parser = &priv->parser;
>> + __le32 *file = (__le32 *)parser->file;
>> + int i, pad, steps;
>> + u32 accum = 0;
>> + u32 checksum;
>> + u32 mask = 0xffffffff;
>> +
>> + pad = (parser->file_size - CHECKSUM_BYTES_NUM) % 4;
>> + steps = (parser->file_size - CHECKSUM_BYTES_NUM) / 4;
>> +
>> + LOG_INFO(priv, FW_DOWNLOAD, "pad=%d steps=%d\n", pad, steps);
>> +
>> + for (i = 0; i < steps; i++)
>> + accum += le32_to_cpu(file[i]);
>> +
>> + if (pad) {
>> + mask <<= 8 * (4 - pad);
>> + accum += le32_to_cpu(file[steps]) & mask;
>> + }
>> +
>> + checksum = get_unaligned_le32((__le32 *)(parser->file +
>> + parser->file_size - CHECKSUM_BYTES_NUM));
>
> The access to file and parser->file. One with unaligned access and the
> other with, looks wrong. And it looks way to complicated.
>
> Can we just not have a proper __le32 type of parser->file to begin with.
>
>> +
>> + LOG_INFO(priv, FW_DOWNLOAD,
>> + "compare checksum accum=0x%x to checksum=0x%x\n",
>> + accum, checksum);
>> +
>> + return checksum == accum;
>> +}
>> +
>> +static int iwmct_parse_next_section(struct iwmct_priv *priv, const u8 **p_sec,
>> + size_t *sec_size, __le32 *sec_addr)
>> +{
>> + struct iwmct_parser *parser = &priv->parser;
>> + struct iwmct_dbg *dbg = &priv->dbg;
>> + struct iwmct_fw_sec_hdr *sec_hdr;
>> +
>> + LOG_INFOEX(priv, INIT, "-->\n");
>> +
>> + while (parser->cur_pos + sizeof(struct iwmct_fw_sec_hdr)
>> + <= parser->file_size) {
>> +
>> + sec_hdr = (struct iwmct_fw_sec_hdr *)
>> + (parser->file + parser->cur_pos);
>> + parser->cur_pos += sizeof(struct iwmct_fw_sec_hdr);
>> +
>> + LOG_INFO(priv, FW_DOWNLOAD,
>> + "sec hdr: type=%s addr=0x%x size=%d\n",
>> + sec_hdr->type, sec_hdr->target_addr,
>> + sec_hdr->data_size);
>> +
>> + if (strcmp(sec_hdr->type, "ENT") == 0)
>> + parser->entry_point = le32_to_cpu(sec_hdr->target_addr);
>> + else if (strcmp(sec_hdr->type, "LBL") == 0)
>> + strcpy(dbg->label_fw, parser->file + parser->cur_pos);
>> + else if (((strcmp(sec_hdr->type, "TOP") == 0) &&
>> + (priv->barker & BARKER_DNLOAD_TOP_MSK)) ||
>> + ((strcmp(sec_hdr->type, "GPS") == 0) &&
>> + (priv->barker & BARKER_DNLOAD_GPS_MSK)) ||
>> + ((strcmp(sec_hdr->type, "BTH") == 0) &&
>> + (priv->barker & BARKER_DNLOAD_BT_MSK))) {
>> + *sec_addr = sec_hdr->target_addr;
>> + *sec_size = le32_to_cpu(sec_hdr->data_size);
>> + *p_sec = parser->file + parser->cur_pos;
>> + parser->cur_pos += le32_to_cpu(sec_hdr->data_size);
>> + return 1;
>
> This if statement is unreadable. This indentation gives me a headache.
>
> Also this function better return bool and not int.
>
>> + } else if (strcmp(sec_hdr->type, "LOG") != 0)
>> + LOG_WARNING(priv, FW_DOWNLOAD,
>> + "skipping section type %s\n",
>> + sec_hdr->type);
>> +
>> + parser->cur_pos += le32_to_cpu(sec_hdr->data_size);
>> + LOG_INFO(priv, FW_DOWNLOAD,
>> + "finished with section cur_pos=%zd\n", parser->cur_pos);
>> + }
>> +
>> + LOG_INFOEX(priv, INIT, "<--\n");
>> + return 0;
>> +}
>> +
>> +static int iwmct_download_section(struct iwmct_priv *priv, const u8 *p_sec,
>> + size_t sec_size, __le32 addr)
>> +{
>> + struct iwmct_parser *parser = &priv->parser;
>> + struct iwmct_fw_load_hdr *hdr = (struct iwmct_fw_load_hdr *)parser->buf;
>> + const u8 *cur_block = p_sec;
>> + size_t sent = 0;
>> + int cnt = 0;
>> + int ret = 0;
>> + u32 cmd = 0;
>> +
>> + LOG_INFOEX(priv, INIT, "-->\n");
>> + LOG_INFO(priv, FW_DOWNLOAD, "Download address 0x%x size 0x%zx\n",
>> + addr, sec_size);
>> +
>> + while (sent < sec_size) {
>> + int i;
>> + u32 chksm = 0;
>> + u32 reset = atomic_read(&priv->reset);
>> + /* actual FW data */
>> + u32 data_size = min(parser->buf_size - sizeof(*hdr),
>> + sec_size - sent);
>> + /* Pad to block size */
>> + u32 trans_size = (data_size + sizeof(*hdr) +
>> + IWMC_SDIO_BLK_SIZE - 1) &
>> + ~(IWMC_SDIO_BLK_SIZE - 1);
>> + ++cnt;
>> +
>> + /* in case of reset, interrupt FW DOWNLAOD */
>> + if (reset) {
>> + LOG_INFO(priv, FW_DOWNLOAD,
>> + "Reset detected. Abort FW download!!!");
>> + ret = -ECANCELED;
>> + goto exit;
>> + }
>> +
>> + memset(parser->buf, 0, parser->buf_size);
>> + cmd |= IWMC_OPCODE_WRITE << CMD_HDR_OPCODE_POS;
>> + cmd |= IWMC_CMD_SIGNATURE << CMD_HDR_SIGNATURE_POS;
>> + cmd |= (priv->dbg.direct ? 1 : 0) << CMD_HDR_DIRECT_ACCESS_POS;
>> + cmd |= (priv->dbg.checksum ? 1 : 0) << CMD_HDR_USE_CHECKSUM_POS;
>> + hdr->data_size = cpu_to_le32(data_size);
>> + hdr->target_addr = addr;
>> +
>> + /* checksum is allowed for sizes divisible by 4 */
>> + if (data_size & 0x3)
>> + cmd &= ~CMD_HDR_USE_CHECKSUM_MSK;
>> +
>> + memcpy(hdr->data, cur_block, data_size);
>> +
>> +
>> + if (cmd & CMD_HDR_USE_CHECKSUM_MSK) {
>> +
>> + chksm = data_size + le32_to_cpu(addr) + cmd;
>> + for (i = 0; i < data_size >> 2; i++)
>> + chksm += ((u32 *)cur_block)[i];
>> +
>> + hdr->block_chksm = cpu_to_le32(chksm);
>> + LOG_INFO(priv, FW_DOWNLOAD, "Checksum = 0x%X\n",
>> + hdr->block_chksm);
>> + }
>> +
>> + LOG_INFO(priv, FW_DOWNLOAD, "trans#%d, len=%d, sent=%zd, "
>> + "sec_size=%zd, startAddress 0x%X\n",
>> + cnt, trans_size, sent, sec_size, addr);
>> +
>> + if (priv->dbg.dump)
>> + LOG_HEXDUMP(FW_DOWNLOAD, parser->buf, trans_size);
>> +
>> +
>> + hdr->cmd = cpu_to_le32(cmd);
>> + /* send it down */
>> + /* TODO: add more proper sending and error checking */
>> + ret = iwmct_tx(priv, 0, parser->buf, trans_size);
>> + if (ret != 0) {
>> + LOG_INFO(priv, FW_DOWNLOAD,
>> + "iwmct_tx returned %d\n", ret);
>> + goto exit;
>> + }
>> +
>> + addr = cpu_to_le32(le32_to_cpu(addr) + data_size);
>> + sent += data_size;
>> + cur_block = p_sec + sent;
>> +
>> + if (priv->dbg.blocks && (cnt + 1) >= priv->dbg.blocks) {
>> + LOG_INFO(priv, FW_DOWNLOAD,
>> + "Block number limit is reached [%d]\n",
>> + priv->dbg.blocks);
>> + break;
>> + }
>> + }
>> +
>> + if (sent < sec_size)
>> + ret = -EINVAL;
>> +exit:
>> + LOG_INFOEX(priv, INIT, "<--\n");
>> + return ret;
>> +}
>> +
>> +static int iwmct_kick_fw(struct iwmct_priv *priv, bool jump)
>> +{
>> + struct iwmct_parser *parser = &priv->parser;
>> + struct iwmct_fw_load_hdr *hdr = (struct iwmct_fw_load_hdr *)parser->buf;
>> + int ret;
>> + u32 cmd;
>> +
>> + LOG_INFOEX(priv, INIT, "-->\n");
>> +
>> + memset(parser->buf, 0, parser->buf_size);
>> + cmd = IWMC_CMD_SIGNATURE << CMD_HDR_SIGNATURE_POS;
>> + if (jump) {
>> + cmd |= IWMC_OPCODE_JUMP << CMD_HDR_OPCODE_POS;
>> + hdr->target_addr = cpu_to_le32(parser->entry_point);
>> + LOG_INFO(priv, FW_DOWNLOAD, "jump address 0x%x\n",
>> + parser->entry_point);
>> + } else {
>> + cmd |= IWMC_OPCODE_LAST_COMMAND << CMD_HDR_OPCODE_POS;
>> + LOG_INFO(priv, FW_DOWNLOAD, "last command\n");
>> + }
>> +
>> + hdr->cmd = cpu_to_le32(cmd);
>> +
>> + LOG_HEXDUMP(FW_DOWNLOAD, parser->buf, sizeof(*hdr));
>> + /* send it down */
>> + /* TODO: add more proper sending and error checking */
>> + ret = iwmct_tx(priv, 0, parser->buf, IWMC_SDIO_BLK_SIZE);
>> + if (ret)
>> + LOG_INFO(priv, FW_DOWNLOAD, "iwmct_tx returned %d", ret);
>> +
>> + LOG_INFOEX(priv, INIT, "<--\n");
>> + return 0;
>> +}
>> +
>> +int iwmct_fw_load(struct iwmct_priv *priv)
>> +{
>> + const struct firmware *raw = NULL;
>> + __le32 addr;
>> + size_t len;
>> + const u8 *pdata;
>> + const u8 *name = "iwmc3200top.1.fw";
>> + int ret = 0;
>> +
>> + /* clear parser struct */
>> + memset(&priv->parser, 0, sizeof(struct iwmct_parser));
>> + if (!name) {
>> + ret = -EINVAL;
>> + goto exit;
>> + }
>> +
>> + /* get the firmware */
>> + ret = request_firmware(&raw, name, &priv->func->dev);
>> + if (ret < 0) {
>> + LOG_ERROR(priv, FW_DOWNLOAD, "%s request_firmware failed %d\n",
>> + name, ret);
>> + goto exit;
>> + }
>> +
>> + if (raw->size < sizeof(struct iwmct_fw_sec_hdr)) {
>> + LOG_ERROR(priv, FW_DOWNLOAD, "%s smaller then (%zd) (%zd)\n",
>> + name, sizeof(struct iwmct_fw_sec_hdr), raw->size);
>> + goto exit;
>> + }
>> +
>> + LOG_INFO(priv, FW_DOWNLOAD, "Read firmware '%s'\n", name);
>> +
>> + ret = iwmct_fw_parser_init(priv, raw->data, raw->size, priv->trans_len);
>> + if (ret < 0) {
>> + LOG_ERROR(priv, FW_DOWNLOAD,
>> + "iwmct_parser_init failed: Reason %d\n", ret);
>> + goto exit;
>> + }
>> +
>> + /* checksum */
>> + if (!iwmct_checksum(priv)) {
>> + LOG_ERROR(priv, FW_DOWNLOAD, "checksum error\n");
>> + ret = -EINVAL;
>> + goto exit;
>> + }
>> +
>> + /* download firmware to device */
>> + while (iwmct_parse_next_section(priv, &pdata, &len, &addr)) {
>> + if (iwmct_download_section(priv, pdata, len, addr)) {
>> + LOG_ERROR(priv, FW_DOWNLOAD,
>> + "%s download section failed\n", name);
>> + ret = -EIO;
>> + goto exit;
>> + }
>> + }
>> +
>> + iwmct_kick_fw(priv, !!(priv->barker & BARKER_DNLOAD_JUMP_MSK));
>> +
>> +exit:
>> + kfree(priv->parser.buf);
>> +
>> + if (raw)
>> + release_firmware(raw);
>> +
>> + raw = NULL;
>> +
>> + return ret;
>> +}
>> diff --git a/drivers/misc/iwmc3200top/fw-msg.h b/drivers/misc/iwmc3200top/fw-msg.h
>> new file mode 100644
>> index 0000000..9e26b75
>> --- /dev/null
>> +++ b/drivers/misc/iwmc3200top/fw-msg.h
>> @@ -0,0 +1,113 @@
>> +/*
>> + * iwmc3200top - Intel Wireless MultiCom 3200 Top Driver
>> + * drivers/misc/iwmc3200top/fw-msg.h
>> + *
>> + * Copyright (C) 2009 Intel Corporation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version
>> + * 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> + * 02110-1301, USA.
>> + *
>> + *
>> + * Author Name: Maxim Grabarnik <maxim.grabarnink-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> + * -
>> + *
>> + */
>> +
>> +#ifndef __FWMSG_H__
>> +#define __FWMSG_H__
>> +
>> +#define COMM_TYPE_D2H 0xFF
>> +#define COMM_TYPE_H2D 0xEE
>> +
>> +#define COMM_CATEGORY_OPERATIONAL 0x00
>> +#define COMM_CATEGORY_DEBUG 0x01
>> +#define COMM_CATEGORY_TESTABILITY 0x02
>> +#define COMM_CATEGORY_DIAGNOSTICS 0x03
>> +
>> +#define OP_DBG_ZSTR_MSG cpu_to_le16(0x1A)
>> +
>> +#define FW_LOG_SRC_MAX 32
>> +#define FW_LOG_SRC_ALL 255
>> +
>> +#define FW_STRING_TABLE_ADDR cpu_to_le32(0x0C000000)
>> +
>> +#define CMD_DBG_LOG_LEVEL cpu_to_le16(0x0001)
>> +#define CMD_TST_DEV_RESET cpu_to_le16(0x0060)
>> +#define CMD_TST_FUNC_RESET cpu_to_le16(0x0062)
>> +#define CMD_TST_IFACE_RESET cpu_to_le16(0x0064)
>> +#define CMD_TST_CPU_UTILIZATION cpu_to_le16(0x0065)
>> +#define CMD_TST_TOP_DEEP_SLEEP cpu_to_le16(0x0080)
>> +#define CMD_TST_WAKEUP cpu_to_le16(0x0081)
>> +#define CMD_TST_FUNC_WAKEUP cpu_to_le16(0x0082)
>> +#define CMD_TST_FUNC_DEEP_SLEEP_REQUEST cpu_to_le16(0x0083)
>> +#define CMD_TST_GET_MEM_DUMP cpu_to_le16(0x0096)
>> +
>> +#define OP_OPR_ALIVE cpu_to_le16(0x0010)
>> +#define OP_OPR_CMD_ACK cpu_to_le16(0x001F)
>> +#define OP_OPR_CMD_NACK cpu_to_le16(0x0020)
>> +#define OP_TST_MEM_DUMP cpu_to_le16(0x0043)
>> +
>> +#define CMD_FLAG_PADDING_256 0x80
>> +
>> +#define FW_HCMD_BLOCK_SIZE 256
>> +
>> +struct msg_hdr {
>> + u8 type;
>> + u8 category;
>> + __le16 opcode;
>> + u8 seqnum;
>> + u8 flags;
>> + __le16 length;
>> +} __attribute__((__packed__));
>> +
>> +struct log_hdr {
>> + __le32 timestamp;
>> + u8 severity;
>> + u8 logsource;
>> + __le16 reserved;
>> +} __attribute__((__packed__));
>> +
>> +struct mdump_hdr {
>> + u8 dmpid;
>> + u8 frag;
>> + __le16 size;
>> + __le32 addr;
>> +} __attribute__((__packed__));
>> +
>> +struct top_msg {
>> + struct msg_hdr hdr;
>> + union {
>> + /* D2H messages */
>> + struct {
>> + struct log_hdr log_hdr;
>> + u8 data[1];
>> + } __attribute__((__packed__)) log;
>> +
>> + struct {
>> + struct log_hdr log_hdr;
>> + struct mdump_hdr md_hdr;
>> + u8 data[1];
>> + } __attribute__((__packed__)) mdump;
>> +
>> + /* H2D messages */
>> + struct {
>> + u8 logsource;
>> + u8 sevmask;
>> + } __attribute__((__packed__)) logdefs[FW_LOG_SRC_MAX];
>> + struct mdump_hdr mdump_req;
>> + } u;
>> +} __attribute__((__packed__));
>> +
>> +
>> +#endif /* __FWMSG_H__ */
>> diff --git a/drivers/misc/iwmc3200top/iwmc3200top.h b/drivers/misc/iwmc3200top/iwmc3200top.h
>> new file mode 100644
>> index 0000000..59e4b7a
>> --- /dev/null
>> +++ b/drivers/misc/iwmc3200top/iwmc3200top.h
>> @@ -0,0 +1,202 @@
>> +/*
>> + * iwmc3200top - Intel Wireless MultiCom 3200 Top Driver
>> + * drivers/misc/iwmc3200top/iwmc3200top.h
>> + *
>> + * Copyright (C) 2009 Intel Corporation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version
>> + * 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> + * 02110-1301, USA.
>> + *
>> + *
>> + * Author Name: Maxim Grabarnik <maxim.grabarnink-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> + * -
>> + *
>> + */
>> +
>> +#ifndef __IWMC3200TOP_H__
>> +#define __IWMC3200TOP_H__
>> +
>> +#define DRV_NAME "iwmc3200top"
>> +
>> +#define IWMC_SDIO_BLK_SIZE 256
>> +#define IWMC_DEFAULT_TR_BLK 64
>> +#define IWMC_SDIO_DATA_ADDR 0x0
>> +#define IWMC_SDIO_INTR_ENABLE_ADDR 0x14
>> +#define IWMC_SDIO_INTR_STATUS_ADDR 0x13
>> +#define IWMC_SDIO_INTR_CLEAR_ADDR 0x13
>> +#define IWMC_SDIO_INTR_GET_SIZE_ADDR 0x2C
>> +
>> +#define COMM_HUB_HEADER_LENGTH 16
>> +#define LOGGER_HEADER_LENGTH 10
>> +
>> +
>> +#define BARKER_DNLOAD_BT_POS 0
>> +#define BARKER_DNLOAD_BT_MSK BIT(BARKER_DNLOAD_BT_POS)
>> +#define BARKER_DNLOAD_GPS_POS 1
>> +#define BARKER_DNLOAD_GPS_MSK BIT(BARKER_DNLOAD_GPS_POS)
>> +#define BARKER_DNLOAD_TOP_POS 2
>> +#define BARKER_DNLOAD_TOP_MSK BIT(BARKER_DNLOAD_TOP_POS)
>> +#define BARKER_DNLOAD_RESERVED1_POS 3
>> +#define BARKER_DNLOAD_RESERVED1_MSK BIT(BARKER_DNLOAD_RESERVED1_POS)
>> +#define BARKER_DNLOAD_JUMP_POS 4
>> +#define BARKER_DNLOAD_JUMP_MSK BIT(BARKER_DNLOAD_JUMP_POS)
>> +#define BARKER_DNLOAD_SYNC_POS 5
>> +#define BARKER_DNLOAD_SYNC_MSK BIT(BARKER_DNLOAD_SYNC_POS)
>> +#define BARKER_DNLOAD_RESERVED2_POS 6
>> +#define BARKER_DNLOAD_RESERVED2_MSK (0x3 << BARKER_DNLOAD_RESERVED2_POS)
>> +#define BARKER_DNLOAD_BARKER_POS 8
>> +#define BARKER_DNLOAD_BARKER_MSK (0xffffff << BARKER_DNLOAD_BARKER_POS)
>> +
>> +#define IWMC_BARKER_REBOOT (0xdeadbe << BARKER_DNLOAD_BARKER_POS)
>> +/* whole field barker */
>> +#define IWMC_BARKER_ACK 0xfeedbabe
>> +
>> +#define IWMC_CMD_SIGNATURE 0xcbbc
>> +
>> +#define CMD_HDR_OPCODE_POS 0
>> +#define CMD_HDR_OPCODE_MSK_MSK (0xf << CMD_HDR_OPCODE_MSK_POS)
>> +#define CMD_HDR_RESPONSE_CODE_POS 4
>> +#define CMD_HDR_RESPONSE_CODE_MSK (0xf << CMD_HDR_RESPONSE_CODE_POS)
>> +#define CMD_HDR_USE_CHECKSUM_POS 8
>> +#define CMD_HDR_USE_CHECKSUM_MSK BIT(CMD_HDR_USE_CHECKSUM_POS)
>> +#define CMD_HDR_RESPONSE_REQUIRED_POS 9
>> +#define CMD_HDR_RESPONSE_REQUIRED_MSK BIT(CMD_HDR_RESPONSE_REQUIRED_POS)
>> +#define CMD_HDR_DIRECT_ACCESS_POS 10
>> +#define CMD_HDR_DIRECT_ACCESS_MSK BIT(CMD_HDR_DIRECT_ACCESS_POS)
>> +#define CMD_HDR_RESERVED_POS 11
>> +#define CMD_HDR_RESERVED_MSK BIT(0x1f << CMD_HDR_RESERVED_POS)
>> +#define CMD_HDR_SIGNATURE_POS 16
>> +#define CMD_HDR_SIGNATURE_MSK BIT(0xffff << CMD_HDR_SIGNATURE_POS)
>> +
>> +enum {
>> + IWMC_OPCODE_PING = 0,
>> + IWMC_OPCODE_READ = 1,
>> + IWMC_OPCODE_WRITE = 2,
>> + IWMC_OPCODE_JUMP = 3,
>> + IWMC_OPCODE_REBOOT = 4,
>> + IWMC_OPCODE_PERSISTENT_WRITE = 5,
>> + IWMC_OPCODE_PERSISTENT_READ = 6,
>> + IWMC_OPCODE_READ_MODIFY_WRITE = 7,
>> + IWMC_OPCODE_LAST_COMMAND = 15
>> +};
>> +
>> +struct iwmct_fw_load_hdr {
>> + __le32 cmd;
>> + __le32 target_addr;
>> + __le32 data_size;
>> + __le32 block_chksm;
>> + u8 data[0];
>> +};
>> +
>> +/**
>> + * struct iwmct_fw_hdr
>> + * holds all sw components versions
>> + */
>> +struct iwmct_fw_hdr {
>> + u8 top_major;
>> + u8 top_minor;
>> + u8 top_revision;
>> + u8 gps_major;
>> + u8 gps_minor;
>> + u8 gps_revision;
>> + u8 bt_major;
>> + u8 bt_minor;
>> + u8 bt_revision;
>> + u8 tic_name[31];
>> +};
>> +
>> +/**
>> + * struct iwmct_fw_sec_hdr
>> + * @type: function type
>> + * @data_size: section's data size
>> + * @target_addr: download address
>> + */
>> +struct iwmct_fw_sec_hdr {
>> + u8 type[4];
>> + __le32 data_size;
>> + __le32 target_addr;
>> +};
>> +
>> +/**
>> + * struct iwmct_parser
>> + * @file: fw image
>> + * @file_size: fw size
>> + * @cur_pos: position in file
>> + * @buf: temp buf for download
>> + * @buf_size: size of buf
>> + * @entry_point: address to jump in fw kick-off
>> + */
>> +struct iwmct_parser {
>> + const u8 *file;
>> + size_t file_size;
>> + size_t cur_pos;
>> + u8 *buf;
>> + size_t buf_size;
>> + u32 entry_point;
>> + struct iwmct_fw_hdr versions;
>> +};
>> +
>> +
>> +struct iwmct_work_struct {
>> + struct list_head list;
>> + ssize_t iosize;
>> +};
>> +
>> +struct iwmct_dbg {
>> + int blocks;
>> + bool dump;
>> + bool jump;
>> + bool direct;
>> + bool checksum;
>> + bool fw_download;
>> + int block_size;
>> + int download_trans_blks;
>> +
>> + char label_fw[256];
>> +};
>> +
>> +struct iwmct_priv {
>> + struct sdio_func *func;
>> + struct iwmct_debugfs *dbgfs;
>> + struct iwmct_parser parser;
>> + atomic_t reset;
>> + atomic_t dev_sync;
>> + u32 trans_len;
>> + u32 barker;
>> + struct iwmct_dbg dbg;
>> +
>> + /* drivers work queue */
>> + struct workqueue_struct *wq;
>> + struct workqueue_struct *bus_rescan_wq;
>> + struct work_struct bus_rescan_worker;
>> + struct work_struct isr_worker;
>> +
>> + /* drivers wait queue */
>> + wait_queue_head_t wait_q;
>> +
>> + /* rx request list */
>> + struct list_head read_req_list;
>> +};
>> +
>> +extern int iwmct_tx(struct iwmct_priv *priv, unsigned int addr,
>> + void *src, int count);
>> +
>> +extern int iwmct_fw_load(struct iwmct_priv *priv);
>> +
>> +extern void iwmct_dbg_init_params(struct iwmct_priv *drv);
>> +extern void iwmct_dbg_init_drv_attrs(struct device_driver *drv);
>> +extern void iwmct_dbg_remove_drv_attrs(struct device_driver *drv);
>> +extern int iwmct_send_hcmd(struct iwmct_priv *priv, u8 *cmd, u16 len);
>> +
>> +#endif /* __IWMC3200TOP_H__ */
>> diff --git a/drivers/misc/iwmc3200top/log.c b/drivers/misc/iwmc3200top/log.c
>> new file mode 100644
>> index 0000000..96b5e4a
>> --- /dev/null
>> +++ b/drivers/misc/iwmc3200top/log.c
>> @@ -0,0 +1,339 @@
>> +/*
>> + * iwmc3200top - Intel Wireless MultiCom 3200 Top Driver
>> + * drivers/misc/iwmc3200top/log.c
>> + *
>> + * Copyright (C) 2009 Intel Corporation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version
>> + * 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> + * 02110-1301, USA.
>> + *
>> + *
>> + * Author Name: Maxim Grabarnik <maxim.grabarnink-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> + * -
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/mmc/sdio_func.h>
>> +#include <linux/ctype.h>
>> +#include "fw-msg.h"
>> +#include "iwmc3200top.h"
>> +#include "log.h"
>> +
>> +/* Maximal hexadecimal string size of the FW memdump message */
>> +#define LOG_MSG_SIZE_MAX 12400
>> +
>> +/* iwmct_logdefs is a global used by log macros */
>> +u8 iwmct_logdefs[LOG_SRC_MAX];
>> +static u8 iwmct_fw_logdefs[FW_LOG_SRC_MAX];
>> +
>> +
>> +static int _log_set_log_filter(u8 *logdefs, int size, u8 src, u8 logmask)
>> +{
>> + int i;
>> +
>> + if (src < size)
>> + logdefs[src] = logmask;
>> + else if (src == LOG_SRC_ALL)
>> + for (i = 0; i < size; i++)
>> + logdefs[i] = logmask;
>> + else
>> + return -1;
>> +
>> + return 0;
>> +}
>> +
>> +
>> +int iwmct_log_set_filter(u8 src, u8 logmask)
>> +{
>> + return _log_set_log_filter(iwmct_logdefs, LOG_SRC_MAX, src, logmask);
>> +}
>> +
>> +
>> +int iwmct_log_set_fw_filter(u8 src, u8 logmask)
>> +{
>> + return _log_set_log_filter(iwmct_fw_logdefs,
>> + FW_LOG_SRC_MAX, src, logmask);
>> +}
>> +
>> +
>> +static int log_msg_format_hex(char *str, int slen, u8 *ibuf,
>> + int ilen, char *pref)
>> +{
>> + int pos = 0;
>> + int i;
>> + int len;
>> +
>> + for (pos = 0, i = 0; pos < slen - 2 && pref[i] != '\0'; i++, pos++)
>> + str[pos] = pref[i];
>> +
>> + for (i = 0; pos < slen - 2 && i < ilen; pos += len, i++)
>> + len = snprintf(&str[pos], slen - pos - 1, " %2.2X", ibuf[i]);
>> +
>> + if (i < ilen)
>> + return -1;
>> +
>> + return 0;
>> +}
>> +
>> +/* NOTE: This function is not thread safe.
>> + Currently it's called only from sdio rx worker - no race there
>> +*/
>> +void iwmct_log_top_message(struct iwmct_priv *priv, u8 *buf, int len)
>> +{
>> + struct top_msg *msg;
>> + static char logbuf[LOG_MSG_SIZE_MAX];
>> +
>> + msg = (struct top_msg *)buf;
>> +
>> + if (len < sizeof(msg->hdr) + sizeof(msg->u.log.log_hdr)) {
>> + LOG_ERROR(priv, FW_MSG, "Log message from TOP "
>> + "is too short %d (expected %zd)\n",
>> + len, sizeof(msg->hdr) + sizeof(msg->u.log.log_hdr));
>> + return;
>> + }
>> +
>> + if (!(iwmct_fw_logdefs[msg->u.log.log_hdr.logsource] &
>> + BIT(msg->u.log.log_hdr.severity)) ||
>> + !(iwmct_logdefs[LOG_SRC_FW_MSG] & BIT(msg->u.log.log_hdr.severity)))
>> + return;
>> +
>> + switch (msg->hdr.category) {
>> + case COMM_CATEGORY_TESTABILITY:
>> + if (!(iwmct_logdefs[LOG_SRC_TST] &
>> + BIT(msg->u.log.log_hdr.severity)))
>> + return;
>> + if (log_msg_format_hex(logbuf, LOG_MSG_SIZE_MAX, buf,
>> + le16_to_cpu(msg->hdr.length) +
>> + sizeof(msg->hdr), "<TST>"))
>> + LOG_WARNING(priv, TST,
>> + "TOP TST message is too long, truncating...");
>> + LOG_WARNING(priv, TST, "%s\n", logbuf);
>> + break;
>> + case COMM_CATEGORY_DEBUG:
>> + if (msg->hdr.opcode == OP_DBG_ZSTR_MSG)
>> + LOG_INFO(priv, FW_MSG, "%s %s", "<DBG>",
>> + ((u8 *)msg) + sizeof(msg->hdr)
>> + + sizeof(msg->u.log.log_hdr));
>> + else {
>> + if (log_msg_format_hex(logbuf, LOG_MSG_SIZE_MAX, buf,
>> + le16_to_cpu(msg->hdr.length)
>> + + sizeof(msg->hdr),
>> + "<DBG>"))
>> + LOG_WARNING(priv, FW_MSG,
>> + "TOP DBG message is too long,"
>> + "truncating...");
>> + LOG_WARNING(priv, FW_MSG, "%s\n", logbuf);
>> + }
>> + break;
>> + default:
>> + break;
>> + }
>> +}
>> +
>> +static int _log_get_filter_str(u8 *logdefs, int logdefsz, char *buf, int size)
>> +{
>> + int i, pos, len;
>> + for (i = 0, pos = 0; (pos < size-1) && (i < logdefsz); i++) {
>> + len = snprintf(&buf[pos], size - pos - 1, "0x%02X%02X,",
>> + i, logdefs[i]);
>> + pos += len;
>> + }
>> + buf[pos-1] = '\n';
>> + buf[pos] = '\0';
>> +
>> + if (i < logdefsz)
>> + return -1;
>> + return 0;
>> +}
>> +
>> +int log_get_filter_str(char *buf, int size)
>> +{
>> + return _log_get_filter_str(iwmct_logdefs, LOG_SRC_MAX, buf, size);
>> +}
>> +
>> +int log_get_fw_filter_str(char *buf, int size)
>> +{
>> + return _log_get_filter_str(iwmct_fw_logdefs, FW_LOG_SRC_MAX, buf, size);
>> +}
>> +
>> +#define HEXADECIMAL_RADIX 16
>> +#define LOG_SRC_FORMAT 7 /* log level is in format of "0xXXXX," */
>> +
>> +ssize_t show_iwmct_log_level(struct device *d,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct iwmct_priv *priv = d->driver_data;
>> + char *str_buf;
>> + int buf_size;
>> + ssize_t ret = -EAGAIN;
>> +
>> + buf_size = (LOG_SRC_FORMAT * LOG_SRC_MAX) + 1;
>> + str_buf = kzalloc(buf_size, GFP_KERNEL);
>> + if (!str_buf) {
>> + LOG_ERROR(priv, DEBUGFS,
>> + "failed to allocate %d bytes\n", buf_size);
>> + goto exit;
>> + }
>> +
>> + if (log_get_filter_str(str_buf, buf_size) < 0)
>> + goto exit;
>> +
>> + ret = sprintf(buf, "%s", str_buf);
>> +
>> +exit:
>> + kfree(str_buf);
>> + return ret;
>> +}
>> +
>> +ssize_t store_iwmct_log_level(struct device *d,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct iwmct_priv *priv = d->driver_data;
>> + char *token, *str_buf = NULL;
>> + long val;
>> + u8 src, mask;
>> +
>> + if (!count)
>> + goto err;
>> +
>> + str_buf = kzalloc(count, GFP_KERNEL);
>> + if (!str_buf) {
>> + LOG_ERROR(priv, DEBUGFS,
>> + "failed to allocate %zd bytes\n", count);
>> + goto err;
>> + }
>> +
>> + memcpy(str_buf, buf, count);
>> +
>> + while ((token = strsep(&str_buf, ",")) != NULL) {
>> + while (isspace(*token))
>> + ++token;
>> + if (strict_strtol(token, HEXADECIMAL_RADIX, &val)) {
>> + LOG_ERROR(priv, DEBUGFS,
>> + "failed to convert string to long %s\n",
>> + token);
>> + goto err;
>> + }
>> +
>> + mask = val & 0xFF;
>> + src = (val & 0XFF00) >> 8;
>> + iwmct_log_set_filter(src, mask);
>> + }
>> +
>> + kfree(str_buf);
>> + return count;
>> +
>> +err:
>> + kfree(str_buf);
>> + return -EAGAIN;
>> +}
>> +
>> +ssize_t show_iwmct_log_level_fw(struct device *d,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct iwmct_priv *priv = d->driver_data;
>> + char *str_buf;
>> + int buf_size;
>> + ssize_t ret = -EAGAIN;
>> +
>> + buf_size = (LOG_SRC_FORMAT * FW_LOG_SRC_MAX) + 2;
>> +
>> + str_buf = kzalloc(buf_size, GFP_KERNEL);
>> + if (!str_buf) {
>> + LOG_ERROR(priv, DEBUGFS,
>> + "failed to allocate %d bytes\n", buf_size);
>> + goto exit;
>> + }
>> +
>> + if (log_get_fw_filter_str(str_buf, buf_size) < 0)
>> + goto exit;
>> +
>> + ret = sprintf(buf, "%s", str_buf);
>> +
>> +exit:
>> + kfree(str_buf);
>> + return ret;
>> +}
>> +
>> +ssize_t store_iwmct_log_level_fw(struct device *d,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct iwmct_priv *priv = d->driver_data;
>> + char *token, *str_buf = NULL;
>> + long val;
>> + u8 src, mask;
>> + struct top_msg cmd;
>> + u16 cmdlen = 0;
>> + int i, rc;
>> +
>> + if (!count)
>> + goto err;
>> +
>> + str_buf = kzalloc(count, GFP_KERNEL);
>> + if (!str_buf) {
>> + LOG_ERROR(priv, DEBUGFS,
>> + "failed to allocate %zd bytes\n", count);
>> + goto err;
>> + }
>> +
>> + memcpy(str_buf, buf, count);
>> +
>> + cmd.hdr.type = COMM_TYPE_H2D;
>> + cmd.hdr.category = COMM_CATEGORY_DEBUG;
>> + cmd.hdr.opcode = CMD_DBG_LOG_LEVEL;
>> +
>> + for (i = 0; ((token = strsep(&str_buf, ",")) != NULL) &&
>> + (i < FW_LOG_SRC_MAX); i++) {
>> +
>> + while (isspace(*token))
>> + ++token;
>> +
>> + if (strict_strtol(token, HEXADECIMAL_RADIX, &val)) {
>> + LOG_ERROR(priv, DEBUGFS,
>> + "failed to convert string to long %s\n",
>> + token);
>> + goto err;
>> + }
>> +
>> + mask = val & 0xFF; /* LSB */
>> + src = (val & 0XFF00) >> 8; /* 2nd least significant byte. */
>> + iwmct_log_set_fw_filter(src, mask);
>> +
>> + cmd.u.logdefs[i].logsource = src;
>> + cmd.u.logdefs[i].sevmask = mask;
>> + }
>> +
>> + cmd.hdr.length = cpu_to_le16(i * sizeof(cmd.u.logdefs[0]));
>> + cmdlen = (i * sizeof(cmd.u.logdefs[0]) + sizeof(cmd.hdr));
>> +
>> + rc = iwmct_send_hcmd(priv, (u8 *) &cmd, cmdlen);
>> + if (rc) {
>> + LOG_ERROR(priv, DEBUGFS,
>> + "Failed to send %d bytes of fwcmd, rc=%d\n",
>> + cmdlen, rc);
>> + goto err;
>> + } else
>> + LOG_INFO(priv, DEBUGFS, "fwcmd sent (%d bytes)\n", cmdlen);
>> +
>> + kfree(str_buf);
>> + return count;
>> +
>> +err:
>> + kfree(str_buf);
>> + return -EAGAIN;
>> +}
>> +
>> diff --git a/drivers/misc/iwmc3200top/log.h b/drivers/misc/iwmc3200top/log.h
>> new file mode 100644
>> index 0000000..aba8121
>> --- /dev/null
>> +++ b/drivers/misc/iwmc3200top/log.h
>> @@ -0,0 +1,158 @@
>> +/*
>> + * iwmc3200top - Intel Wireless MultiCom 3200 Top Driver
>> + * drivers/misc/iwmc3200top/log.h
>> + *
>> + * Copyright (C) 2009 Intel Corporation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version
>> + * 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> + * 02110-1301, USA.
>> + *
>> + *
>> + * Author Name: Maxim Grabarnik <maxim.grabarnink-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> + * -
>> + *
>> + */
>> +
>> +#ifndef __LOG_H__
>> +#define __LOG_H__
>> +
>> +
>> +/* log severity:
>> + * The log levels here match FW log levels
>> + * so values need to stay as is */
>> +#define LOG_SEV_CRITICAL 0
>> +#define LOG_SEV_ERROR 1
>> +#define LOG_SEV_WARNING 2
>> +#define LOG_SEV_INFO 3
>> +#define LOG_SEV_INFOEX 4
>> +
>> +#define LOG_SEV_FILTER_ALL \
>> + (BIT(LOG_SEV_CRITICAL) | \
>> + BIT(LOG_SEV_ERROR) | \
>> + BIT(LOG_SEV_WARNING) | \
>> + BIT(LOG_SEV_INFO) | \
>> + BIT(LOG_SEV_INFOEX))
>> +
>> +/* log source */
>> +#define LOG_SRC_INIT 0
>> +#define LOG_SRC_DEBUGFS 1
>> +#define LOG_SRC_FW_DOWNLOAD 2
>> +#define LOG_SRC_FW_MSG 3
>> +#define LOG_SRC_TST 4
>> +#define LOG_SRC_IRQ 5
>> +
>> +#define LOG_SRC_MAX 6
>> +#define LOG_SRC_ALL 0xFF
>> +
>> +/**
>> + * Default intitialization runtime log level
>> + */
>> +#ifndef LOG_SEV_FILTER_RUNTIME
>> +#define LOG_SEV_FILTER_RUNTIME \
>> + (BIT(LOG_SEV_CRITICAL) | \
>> + BIT(LOG_SEV_ERROR) | \
>> + BIT(LOG_SEV_WARNING))
>> +#endif
>> +
>> +#ifndef FW_LOG_SEV_FILTER_RUNTIME
>> +#define FW_LOG_SEV_FILTER_RUNTIME LOG_SEV_FILTER_ALL
>> +#endif
>> +
>> +#ifdef CONFIG_IWMC3200TOP_DEBUG
>> +/**
>> + * Log macros
>> + */
>> +
>> +#define priv2dev(priv) (&(priv->func)->dev)
>> +
>> +#define LOG_CRITICAL(priv, src, fmt, args...) \
>> +do { \
>> + if (iwmct_logdefs[LOG_SRC_ ## src] & BIT(LOG_SEV_CRITICAL)) \
>> + dev_crit(priv2dev(priv), "%s %d: " fmt, \
>> + __func__, __LINE__, ##args); \
>> +} while (0)
>> +
>> +#define LOG_ERROR(priv, src, fmt, args...) \
>> +do { \
>> + if (iwmct_logdefs[LOG_SRC_ ## src] & BIT(LOG_SEV_ERROR)) \
>> + dev_err(priv2dev(priv), "%s %d: " fmt, \
>> + __func__, __LINE__, ##args); \
>> +} while (0)
>> +
>> +#define LOG_WARNING(priv, src, fmt, args...) \
>> +do { \
>> + if (iwmct_logdefs[LOG_SRC_ ## src] & BIT(LOG_SEV_WARNING)) \
>> + dev_warn(priv2dev(priv), "%s %d: " fmt, \
>> + __func__, __LINE__, ##args); \
>> +} while (0)
>> +
>> +#define LOG_INFO(priv, src, fmt, args...) \
>> +do { \
>> + if (iwmct_logdefs[LOG_SRC_ ## src] & BIT(LOG_SEV_INFO)) \
>> + dev_info(priv2dev(priv), "%s %d: " fmt, \
>> + __func__, __LINE__, ##args); \
>> +} while (0)
>> +
>> +#define LOG_INFOEX(priv, src, fmt, args...) \
>> +do { \
>> + if (iwmct_logdefs[LOG_SRC_ ## src] & BIT(LOG_SEV_INFOEX)) \
>> + dev_dbg(priv2dev(priv), "%s %d: " fmt, \
>> + __func__, __LINE__, ##args); \
>> +} while (0)
>> +
>> +#define LOG_HEXDUMP(src, ptr, len) \
>> +do { \
>> + if (iwmct_logdefs[LOG_SRC_ ## src] & BIT(LOG_SEV_INFOEX)) \
>> + print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, \
>> + 16, 1, ptr, len, false); \
>> +} while (0)
>> +
>> +void iwmct_log_top_message(struct iwmct_priv *priv, u8 *buf, int len);
>> +
>> +extern u8 iwmct_logdefs[];
>> +
>> +int iwmct_log_set_filter(u8 src, u8 logmask);
>> +int iwmct_log_set_fw_filter(u8 src, u8 logmask);
>> +
>> +ssize_t show_iwmct_log_level(struct device *d,
>> + struct device_attribute *attr, char *buf);
>> +ssize_t store_iwmct_log_level(struct device *d,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count);
>> +ssize_t show_iwmct_log_level_fw(struct device *d,
>> + struct device_attribute *attr, char *buf);
>> +ssize_t store_iwmct_log_level_fw(struct device *d,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count);
>> +
>> +#else
>> +
>> +#define LOG_CRITICAL(priv, src, fmt, args...)
>> +#define LOG_ERROR(priv, src, fmt, args...)
>> +#define LOG_WARNING(priv, src, fmt, args...)
>> +#define LOG_INFO(priv, src, fmt, args...)
>> +#define LOG_INFOEX(priv, src, fmt, args...)
>> +#define LOG_HEXDUMP(src, ptr, len)
>> +
>> +static inline void iwmct_log_top_message(struct iwmct_priv *priv,
>> + u8 *buf, int len) {}
>> +static inline int iwmct_log_set_filter(u8 src, u8 logmask) { return 0; }
>> +static inline int iwmct_log_set_fw_filter(u8 src, u8 logmask) { return 0; }
>> +
>> +#endif /* CONFIG_IWMC3200TOP_DEBUG */
>> +
>> +int log_get_filter_str(char *buf, int size);
>> +int log_get_fw_filter_str(char *buf, int size);
>> +
>> +#endif /* __LOG_H__ */
>
> Seriously. Please just use dynamic_printk and lets remove all this log
> infrastructure. It is way too much overhead for too little benefit.
>
>> diff --git a/drivers/misc/iwmc3200top/main.c b/drivers/misc/iwmc3200top/main.c
>> new file mode 100644
>> index 0000000..09f8d06
>> --- /dev/null
>> +++ b/drivers/misc/iwmc3200top/main.c
>> @@ -0,0 +1,702 @@
>> +/*
>> + * iwmc3200top - Intel Wireless MultiCom 3200 Top Driver
>> + * drivers/misc/iwmc3200top/main.c
>> + *
>> + * Copyright (C) 2009 Intel Corporation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version
>> + * 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> + * 02110-1301, USA.
>> + *
>> + *
>> + * Author Name: Maxim Grabarnik <maxim.grabarnink-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> + * -
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mmc/sdio_ids.h>
>> +#include <linux/mmc/sdio_func.h>
>> +#include <linux/mmc/sdio.h>
>> +
>> +#include "iwmc3200top.h"
>> +#include "log.h"
>> +#include "fw-msg.h"
>> +#include "debugfs.h"
>> +
>> +
>> +#define DRIVER_DESCRIPTION "Intel(R) IWMC 3200 Top Driver"
>> +#define DRIVER_COPYRIGHT "Copyright (c) 2008 Intel Corporation."
>> +
>> +#define IWMCT_VERSION "0.1.62"
>> +
>> +#ifdef REPOSITORY_LABEL
>> +#define RL REPOSITORY_LABEL
>> +#else
>> +#define RL ""
>> +#endif
>> +
>> +#ifdef CONFIG_IWMC3200TOP_DEBUG
>> +#define VD "-d"
>> +#else
>> +#define VD
>> +#endif
>> +
>> +#define DRIVER_VERSION IWMCT_VERSION "-" __stringify(RL) VD
>> +
>> +MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
>> +MODULE_VERSION(DRIVER_VERSION);
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR(DRIVER_COPYRIGHT);
>> +
>> +
>> +#ifndef SDIO_DEVICE_ID_INTEL_IWMC3200TOP
>> +#define SDIO_DEVICE_ID_INTEL_IWMC3200TOP 0x1404
>> +#endif
>> +
>> +/*
>> + * This workers main task is to wait for OP_OPR_ALIVE
>> + * from TOP FW until ALIVE_MSG_TIMOUT timeout is elapsed.
>> + * When OP_OPR_ALIVE received it will issue
>> + * a call to "bus_rescan_devices".
>> + */
>> +static void iwmct_rescan_worker(struct work_struct *ws)
>> +{
>> + struct iwmct_priv *priv;
>> + int ret;
>> +
>> + priv = container_of(ws, struct iwmct_priv, bus_rescan_worker);
>> +
>> + LOG_INFO(priv, FW_MSG, "Calling bus_rescan\n");
>> +
>> + ret = bus_rescan_devices(priv->func->dev.bus);
>> + if (ret < 0)
>> + LOG_INFO(priv, FW_DOWNLOAD, "bus_rescan_devices FAILED!!!\n");
>> +}
>> +
>> +static void op_top_message(struct iwmct_priv *priv, struct top_msg *msg)
>> +{
>> + switch (msg->hdr.opcode) {
>> + case OP_OPR_ALIVE:
>> + LOG_INFO(priv, FW_MSG, "Got ALIVE from device, wake rescan\n");
>> + queue_work(priv->bus_rescan_wq, &priv->bus_rescan_worker);
>> + break;
>> + default:
>> + LOG_INFO(priv, FW_MSG, "Received msg opcode 0x%X\n",
>> + msg->hdr.opcode);
>> + break;
>> + }
>> +}
>> +
>> +
>> +static void handle_top_message(struct iwmct_priv *priv, u8 *buf, int len)
>> +{
>> + struct top_msg *msg;
>> +
>> + msg = (struct top_msg *)buf;
>> +
>> + if (msg->hdr.type != COMM_TYPE_D2H) {
>> + LOG_ERROR(priv, FW_MSG,
>> + "Message from TOP with invalid message type 0x%X\n",
>> + msg->hdr.type);
>> + return;
>> + }
>> +
>> + if (len < sizeof(msg->hdr)) {
>> + LOG_ERROR(priv, FW_MSG,
>> + "Message from TOP is too short for message header "
>> + "received %d bytes, expected at least %zd bytes\n",
>> + len, sizeof(msg->hdr));
>> + return;
>> + }
>> +
>> + if (len < le16_to_cpu(msg->hdr.length) + sizeof(msg->hdr)) {
>> + LOG_ERROR(priv, FW_MSG,
>> + "Message length (%d bytes) is shorter than "
>> + "in header (%d bytes)\n",
>> + len, le16_to_cpu(msg->hdr.length));
>> + return;
>> + }
>> +
>> + switch (msg->hdr.category) {
>> + case COMM_CATEGORY_OPERATIONAL:
>> + op_top_message(priv, (struct top_msg *)buf);
>> + break;
>> +
>> + case COMM_CATEGORY_DEBUG:
>> + case COMM_CATEGORY_TESTABILITY:
>> + case COMM_CATEGORY_DIAGNOSTICS:
>> + iwmct_log_top_message(priv, buf, len);
>> + break;
>> +
>> + default:
>> + LOG_ERROR(priv, FW_MSG,
>> + "Message from TOP with unknown category 0x%X\n",
>> + msg->hdr.category);
>> + break;
>> + }
>> +}
>> +
>> +int iwmct_send_hcmd(struct iwmct_priv *priv, u8 *cmd, u16 len)
>> +{
>> + int ret;
>> + u8 *buf;
>> +
>> + LOG_INFOEX(priv, FW_MSG, "Sending hcmd:\n");
>> +
>> + /* add padding to 256 for IWMC */
>> + ((struct top_msg *)cmd)->hdr.flags |= CMD_FLAG_PADDING_256;
>> +
>> + LOG_HEXDUMP(FW_MSG, cmd, len);
>> +
>> + if (len > FW_HCMD_BLOCK_SIZE) {
>> + LOG_ERROR(priv, FW_MSG, "size %d exceeded hcmd max size %d\n",
>> + len, FW_HCMD_BLOCK_SIZE);
>> + return -1;
>> + }
>> +
>> + buf = kzalloc(FW_HCMD_BLOCK_SIZE, GFP_KERNEL);
>> + if (!buf) {
>> + LOG_ERROR(priv, FW_MSG, "kzalloc error, buf size %d\n",
>> + FW_HCMD_BLOCK_SIZE);
>> + return -1;
>> + }
>> +
>> + memcpy(buf, cmd, len);
>> +
>> + sdio_claim_host(priv->func);
>> + ret = sdio_memcpy_toio(priv->func, IWMC_SDIO_DATA_ADDR, buf,
>> + FW_HCMD_BLOCK_SIZE);
>> + sdio_release_host(priv->func);
>> +
>> + kfree(buf);
>> + return ret;
>> +}
>> +
>> +int iwmct_tx(struct iwmct_priv *priv, unsigned int addr,
>> + void *src, int count)
>> +{
>> + int ret;
>> +
>> + sdio_claim_host(priv->func);
>> + ret = sdio_memcpy_toio(priv->func, addr, src, count);
>> + sdio_release_host(priv->func);
>> +
>> + return ret;
>> +}
>> +
>> +static void iwmct_irq_read_worker(struct work_struct *ws)
>> +{
>> + struct iwmct_priv *priv;
>> + struct iwmct_work_struct *read_req;
>> + __le32 *buf = NULL;
>> + int ret, val, i;
>> + int iosize;
>> + u32 barker;
>> +
>> + priv = container_of(ws, struct iwmct_priv, isr_worker);
>> +
>> + LOG_INFO(priv, IRQ, "enter iwmct_irq_read_worker %p\n", ws);
>> +
>> + /* --------------------- Handshake with device -------------------- */
>> + sdio_claim_host(priv->func);
>> +
>> + /* all list manipulations have to be protected by
>> + * sdio_claim_host/sdio_release_host */
>> + if (list_empty(&priv->read_req_list)) {
>> + LOG_ERROR(priv, IRQ, "read_req_list empty in read worker\n");
>> + goto exit_release;
>> + }
>> +
>> + read_req = list_entry(priv->read_req_list.next,
>> + struct iwmct_work_struct, list);
>> +
>> + list_del(&read_req->list);
>> + iosize = read_req->iosize;
>> + kfree(read_req);
>> +
>> + buf = kzalloc(iosize, GFP_KERNEL);
>> + if (!buf) {
>> + LOG_ERROR(priv, IRQ, "kzalloc error, buf size %d\n", iosize);
>> + goto exit_release;
>> + }
>> +
>> + LOG_INFO(priv, IRQ, "iosize=%d, buf=%p, func=%d\n",
>> + iosize, buf, priv->func->num);
>> +
>> + /* read from device */
>> + ret = sdio_memcpy_fromio(priv->func, buf, IWMC_SDIO_DATA_ADDR, iosize);
>> + if (ret) {
>> + LOG_ERROR(priv, IRQ, "error %d reading buffer\n", ret);
>> + goto exit_release;
>> + }
>> +
>> + LOG_HEXDUMP(IRQ, (u8 *)buf, iosize);
>> +
>> + barker = le32_to_cpu(buf[0]);
>> + if (barker != IWMC_BARKER_ACK &&
>> + (barker & BARKER_DNLOAD_BARKER_MSK) != IWMC_BARKER_REBOOT) {
>> + /* Handle Top Commhub message.
>> + * Current handling - treat all message as
>> + * logger messages and print the context as a string. */
>> + sdio_release_host(priv->func);
>> + handle_top_message(priv, (u8 *)buf, iosize);
>> + goto exit;
>> + }
>> +
>> + /* verify basic barker sanity */
>> + for (i = 1; i < 4; i++)
>> + if (buf[i] != buf[0]) {
>> + LOG_ERROR(priv, IRQ, "Corrupted barker,"
>> + "buf[%d]=%x buf[0]=%x\n",
>> + i, buf[i], buf[0]);
>> + goto exit_release;
>> + }
>> +
>> + val = atomic_read(&priv->dev_sync);
>> +
>> + switch (val) {
>> + case 0:
>> + /* we expect to recieve REBOOT BARKER */
>> + if ((barker & BARKER_DNLOAD_BARKER_MSK) != IWMC_BARKER_REBOOT) {
>> + LOG_ERROR(priv, IRQ, "Expecting for reboot barker %x,"
>> + "but got %x\n",
>> + IWMC_BARKER_REBOOT,
>> + (barker & BARKER_DNLOAD_BARKER_MSK));
>> + goto exit_release;
>> + }
>> +
>> + LOG_INFO(priv, IRQ, "Recieved reboot barker: %x\n", barker);
>> + priv->barker = barker;
>> +
>> + if (barker & BARKER_DNLOAD_SYNC_MSK) {
>> + /* Send the same barker back */
>> + ret = sdio_memcpy_toio(priv->func, IWMC_SDIO_DATA_ADDR,
>> + buf, iosize);
>> + if (ret) {
>> + LOG_ERROR(priv, IRQ,
>> + "error %d echoing barker\n", ret);
>> + goto exit_release;
>> + }
>> + LOG_INFO(priv, IRQ, "Echoing barker to device\n");
>> + atomic_inc(&priv->dev_sync);
>> + goto exit_release;
>> + }
>> + break;
>> +
>> + case 1:
>> + /* we expect to recieve ACK BARKER */
>> + if (barker != IWMC_BARKER_ACK) {
>> + LOG_INFO(priv, IRQ, "Expecting for ACK barker[%x],"
>> + "got this instead %x\n",
>> + IWMC_BARKER_ACK, barker);
>> + goto exit_release;
>> + }
>> + kfree(buf);
>> + atomic_set(&priv->dev_sync, 0);
>> + atomic_set(&priv->reset, 0);
>> + break;
>> +
>> + default:
>> + LOG_INFO(priv, IRQ, "Wrong dev_sync %d\n", val);
>> + break;
>> + }
>> +
>> + sdio_release_host(priv->func);
>> +
>> +
>> + LOG_INFO(priv, IRQ, "barker download request 0x%x is:\n", priv->barker);
>> + LOG_INFO(priv, IRQ, "******* Top FW %s requested ********\n",
>> + (priv->barker & BARKER_DNLOAD_TOP_MSK) ? "was" : "not");
>> + LOG_INFO(priv, IRQ, "******* GPS FW %s requested ********\n",
>> + (priv->barker & BARKER_DNLOAD_GPS_MSK) ? "was" : "not");
>> + LOG_INFO(priv, IRQ, "******* BT FW %s requested ********\n",
>> + (priv->barker & BARKER_DNLOAD_BT_MSK) ? "was" : "not");
>> +
>> + if (priv->dbg.fw_download)
>> + iwmct_fw_load(priv);
>> + else
>> + LOG_ERROR(priv, IRQ, "FW download not allowed\n");
>> +
>> + return;
>> +
>> +exit_release:
>> + sdio_release_host(priv->func);
>> +exit:
>> + kfree(buf);
>> + LOG_INFO(priv, IRQ, "exit iwmct_irq_read_worker\n");
>> +}
>> +
>> +static void iwmct_irq(struct sdio_func *func)
>> +{
>> + struct iwmct_priv *priv;
>> + int val, ret;
>> + int iosize;
>> + int addr = IWMC_SDIO_INTR_GET_SIZE_ADDR;
>> + struct iwmct_work_struct *read_req;
>> +
>> + priv = sdio_get_drvdata(func);
>> +
>> + LOG_INFO(priv, IRQ, "enter iwmct_irq\n");
>> +
>> + /* read the function's status register */
>> + val = sdio_readb(func, IWMC_SDIO_INTR_STATUS_ADDR, &ret);
>> +
>> + LOG_INFO(priv, IRQ, "iir value = %d, ret=%d\n", val, ret);
>> +
>> + if (!val) {
>> + LOG_ERROR(priv, IRQ, "iir = 0, exiting ISR\n");
>> + goto exit_clear_intr;
>> + }
>> +
>> +
>> + /*
>> + * read 2 bytes of the transaction size
>> + * IMPORTANT: sdio transaction size has to be read before clearing
>> + * sdio interrupt!!!
>> + */
>> + val = sdio_readb(priv->func, addr++, &ret);
>> + iosize = val;
>> + val = sdio_readb(priv->func, addr++, &ret);
>> + iosize += val << 8;
>> +
>> + LOG_INFO(priv, IRQ, "READ size %d\n", iosize);
>> +
>> + if (iosize == 0) {
>> + LOG_ERROR(priv, IRQ, "READ size %d, exiting ISR\n", iosize);
>> + goto exit_clear_intr;
>> + }
>> +
>> + /* allocate a work structure to pass iosize to the worker */
>> + read_req = kzalloc(sizeof(struct iwmct_work_struct), GFP_KERNEL);
>> + if (!read_req) {
>> + LOG_ERROR(priv, IRQ, "failed to allocate read_req, exit ISR\n");
>> + goto exit_clear_intr;
>> + }
>> +
>> + INIT_LIST_HEAD(&read_req->list);
>> + read_req->iosize = iosize;
>> +
>> + list_add_tail(&priv->read_req_list, &read_req->list);
>> +
>> + /* clear the function's interrupt request bit (write 1 to clear) */
>> + sdio_writeb(func, 1, IWMC_SDIO_INTR_CLEAR_ADDR, &ret);
>> +
>> + queue_work(priv->wq, &priv->isr_worker);
>> +
>> + LOG_INFO(priv, IRQ, "exit iwmct_irq\n");
>> +
>> + return;
>> +
>> +exit_clear_intr:
>> + /* clear the function's interrupt request bit (write 1 to clear) */
>> + sdio_writeb(func, 1, IWMC_SDIO_INTR_CLEAR_ADDR, &ret);
>> +}
>> +
>> +
>> +static int blocks;
>> +module_param(blocks, int, 0604);
>> +MODULE_PARM_DESC(blocks, "max_blocks_to_send");
>> +
>> +static int dump;
>> +module_param(dump, bool, 0604);
>> +MODULE_PARM_DESC(dump, "dump_hex_content");
>> +
>> +static int jump = 1;
>> +module_param(jump, bool, 0604);
>> +
>> +static int direct = 1;
>> +module_param(direct, bool, 0604);
>> +
>> +static int checksum = 1;
>> +module_param(checksum, bool, 0604);
>> +
>> +static int fw_download = 1;
>> +module_param(fw_download, bool, 0604);
>> +
>> +static int block_size = IWMC_SDIO_BLK_SIZE;
>> +module_param(block_size, int, 0404);
>> +
>> +static int download_trans_blks = IWMC_DEFAULT_TR_BLK;
>> +module_param(download_trans_blks, int, 0604);
>> +
>> +static int rubbish_barker;
>> +module_param(rubbish_barker, bool, 0604);
>
> Most of them are missing descriptions. And do we really need them? What
> cases are they trying to work around?
>
>> +
>> +#ifdef CONFIG_IWMC3200TOP_DEBUG
>> +static int log_level[LOG_SRC_MAX];
>> +static unsigned int log_level_argc;
>> +module_param_array(log_level, int, &log_level_argc, 0604);
>> +MODULE_PARM_DESC(log_level, "log_level");
>> +
>> +static int log_level_fw[FW_LOG_SRC_MAX];
>> +static unsigned int log_level_fw_argc;
>> +module_param_array(log_level_fw, int, &log_level_fw_argc, 0604);
>> +MODULE_PARM_DESC(log_level_fw, "log_level_fw");
>> +#endif
>> +
>> +void iwmct_dbg_init_params(struct iwmct_priv *priv)
>> +{
>> +#ifdef CONFIG_IWMC3200TOP_DEBUG
>> + int i;
>> +
>> + for (i = 0; i < log_level_argc; i++) {
>> + dev_notice(&priv->func->dev, "log_level[%d]=0x%X\n",
>> + i, log_level[i]);
>> + iwmct_log_set_filter((log_level[i] >> 8) & 0xFF,
>> + log_level[i] & 0xFF);
>> + }
>> + for (i = 0; i < log_level_fw_argc; i++) {
>> + dev_notice(&priv->func->dev, "log_level_fw[%d]=0x%X\n",
>> + i, log_level_fw[i]);
>> + iwmct_log_set_fw_filter((log_level_fw[i] >> 8) & 0xFF,
>> + log_level_fw[i] & 0xFF);
>> + }
>> +#endif
>> +
>> + priv->dbg.blocks = blocks;
>> + LOG_INFO(priv, INIT, "blocks=%d\n", blocks);
>> + priv->dbg.dump = (bool)dump;
>> + LOG_INFO(priv, INIT, "dump=%d\n", dump);
>> + priv->dbg.jump = (bool)jump;
>> + LOG_INFO(priv, INIT, "jump=%d\n", jump);
>> + priv->dbg.direct = (bool)direct;
>> + LOG_INFO(priv, INIT, "direct=%d\n", direct);
>> + priv->dbg.checksum = (bool)checksum;
>> + LOG_INFO(priv, INIT, "checksum=%d\n", checksum);
>> + priv->dbg.fw_download = (bool)fw_download;
>> + LOG_INFO(priv, INIT, "fw_download=%d\n", fw_download);
>> + priv->dbg.block_size = block_size;
>> + LOG_INFO(priv, INIT, "block_size=%d\n", block_size);
>> + priv->dbg.download_trans_blks = download_trans_blks;
>> + LOG_INFO(priv, INIT, "download_trans_blks=%d\n", download_trans_blks);
>> +}
>> +
>> +/*****************************************************************************
>> + *
>> + * sysfs attributes
>> + *
>> + *****************************************************************************/
>> +static ssize_t show_iwmct_fw_version(struct device *d,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + return sprintf(buf, "%s\n",
>> + ((struct iwmct_priv *)d->driver_data)->dbg.label_fw);
>> +}
>> +static DEVICE_ATTR(cc_label_fw, S_IRUGO, show_iwmct_fw_version, NULL);
>> +
>> +#ifdef CONFIG_IWMC3200TOP_DEBUG
>> +static DEVICE_ATTR(log_level, S_IWUSR | S_IRUGO,
>> + show_iwmct_log_level, store_iwmct_log_level);
>> +static DEVICE_ATTR(log_level_fw, S_IWUSR | S_IRUGO,
>> + show_iwmct_log_level_fw, store_iwmct_log_level_fw);
>> +#endif
>
> First module parameters and now device attributes. Why?
>
>> +
>> +static struct attribute *iwmct_sysfs_entries[] = {
>> + &dev_attr_cc_label_fw.attr,
>> +#ifdef CONFIG_IWMC3200TOP_DEBUG
>> + &dev_attr_log_level.attr,
>> + &dev_attr_log_level_fw.attr,
>> +#endif
>> + NULL
>> +};
>> +
>> +static struct attribute_group iwmct_attribute_group = {
>> + .name = NULL, /* put in device directory */
>> + .attrs = iwmct_sysfs_entries,
>> +};
>> +
>> +
>> +static int iwmct_probe(struct sdio_func *func,
>> + const struct sdio_device_id *id)
>> +{
>> + struct iwmct_priv *priv;
>> + int ret;
>> + int val = 1;
>> + int addr = IWMC_SDIO_INTR_ENABLE_ADDR;
>> +
>> + dev_info(&func->dev, "enter iwmct_probe\n");
>> +
>> + dev_info(&func->dev, "IRQ polling period id %u msecs, HZ is %d\n",
>> + jiffies_to_msecs(2147483647), HZ);
>
> This is debug code. Don't use dev_info().
>
>> +
>> + priv = kzalloc(sizeof(struct iwmct_priv), GFP_KERNEL);
>> + if (!priv) {
>> + dev_err(&func->dev, "kzalloc error\n");
>> + return -ENOMEM;
>> + }
>> + priv->func = func;
>> + sdio_set_drvdata(func, priv);
>> +
>> + LOG_INFO(priv, INIT, "HIDR supported\n");
>> +
>> + /* create drivers work queue */
>> + priv->wq = create_workqueue(DRV_NAME "_wq");
>> + priv->bus_rescan_wq = create_workqueue(DRV_NAME "_rescan_wq");
>> + INIT_WORK(&priv->bus_rescan_worker, iwmct_rescan_worker);
>> + INIT_WORK(&priv->isr_worker, iwmct_irq_read_worker);
>> +
>> + init_waitqueue_head(&priv->wait_q);
>> +
>> + sdio_claim_host(func);
>> + /* FIXME: Remove after it is fixed in the Boot ROM upgrade */
>> + func->enable_timeout = 10;
>> +
>> + /* In our HW, setting the block size also wakes up the boot rom. */
>> + ret = sdio_set_block_size(func, priv->dbg.block_size);
>> + if (ret) {
>> + LOG_ERROR(priv, INIT,
>> + "sdio_set_block_size() failure: %d\n", ret);
>> + goto error_sdio_enable;
>> + }
>> +
>> + ret = sdio_enable_func(func);
>> + if (ret) {
>> + LOG_ERROR(priv, INIT, "sdio_enable_func() failure: %d\n", ret);
>> + goto error_sdio_enable;
>> + }
>> +
>> + /* init reset and dev_sync states */
>> + atomic_set(&priv->reset, 0);
>> + atomic_set(&priv->dev_sync, 0);
>> +
>> + /* init read req queue */
>> + INIT_LIST_HEAD(&priv->read_req_list);
>> +
>> + /* process configurable parameters */
>> + iwmct_dbg_init_params(priv);
>> + ret = sysfs_create_group(&func->dev.kobj, &iwmct_attribute_group);
>> + if (ret) {
>> + LOG_ERROR(priv, INIT, "Failed to register attributes and "
>> + "initialize module_params\n");
>> + goto error_dev_attrs;
>> + }
>> +
>> + iwmct_dbgfs_register(priv, DRV_NAME);
>> +
>> + if (!priv->dbg.direct && priv->dbg.download_trans_blks > 8) {
>> + LOG_INFO(priv, INIT,
>> + "Reducing transaction to 8 blocks = 2K (from %d)\n",
>> + priv->dbg.download_trans_blks);
>> + priv->dbg.download_trans_blks = 8;
>> + }
>> + priv->trans_len = priv->dbg.download_trans_blks * priv->dbg.block_size;
>> + LOG_INFO(priv, INIT, "Transaction length = %d\n", priv->trans_len);
>> +
>> + ret = sdio_claim_irq(func, iwmct_irq);
>> + if (ret) {
>> + LOG_ERROR(priv, INIT, "sdio_claim_irq() failure: %d\n", ret);
>> + goto error_claim_irq;
>> + }
>> +
>> +
>> + /* Enable function's interrupt */
>> + sdio_writeb(priv->func, val, addr, &ret);
>> + if (ret) {
>> + LOG_ERROR(priv, INIT, "Failure writing to "
>> + "Interrupt Enable Register (%d): %d\n", addr, ret);
>> + goto error_enable_int;
>> + }
>> +
>> + sdio_release_host(func);
>> +
>> + LOG_INFO(priv, INIT, "exit iwmct_probe\n");
>> +
>> + return ret;
>> +
>> +error_enable_int:
>> + sdio_release_irq(func);
>> +error_claim_irq:
>> + sdio_disable_func(func);
>> +error_dev_attrs:
>> + iwmct_dbgfs_unregister(priv->dbgfs);
>> + sysfs_remove_group(&func->dev.kobj, &iwmct_attribute_group);
>> +error_sdio_enable:
>> + sdio_release_host(func);
>> + return ret;
>> +}
>> +
>> +static void iwmct_remove(struct sdio_func *func)
>> +{
>> + struct iwmct_work_struct *read_req;
>> + struct iwmct_priv *priv = sdio_get_drvdata(func);
>> +
>> + priv = sdio_get_drvdata(func);
>> +
>> + LOG_INFO(priv, INIT, "enter\n");
>> +
>> + sdio_claim_host(func);
>> + sdio_release_irq(func);
>> + sdio_release_host(func);
>> +
>> + /* Safely destroy osc workqueue */
>> + destroy_workqueue(priv->bus_rescan_wq);
>> + destroy_workqueue(priv->wq);
>> +
>> + sdio_claim_host(func);
>> + sdio_disable_func(func);
>> + sysfs_remove_group(&func->dev.kobj, &iwmct_attribute_group);
>> + iwmct_dbgfs_unregister(priv->dbgfs);
>> + sdio_release_host(func);
>> +
>> + /* free read requests */
>> + while (!list_empty(&priv->read_req_list)) {
>> + read_req = list_entry(priv->read_req_list.next,
>> + struct iwmct_work_struct, list);
>> +
>> + list_del(&read_req->list);
>> + kfree(read_req);
>> + }
>> +
>> + kfree(priv);
>> +}
>> +
>> +
>> +static const struct sdio_device_id iwmct_ids[] = {
>> + { SDIO_DEVICE(SDIO_VENDOR_ID_INTEL, SDIO_DEVICE_ID_INTEL_IWMC3200TOP)},
>> + { /* end: all zeroes */ },
>> +};
>> +
>> +MODULE_DEVICE_TABLE(sdio, iwmct_ids);
>> +
>> +static struct sdio_driver iwmct_driver = {
>> + .probe = iwmct_probe,
>> + .remove = iwmct_remove,
>> + .name = DRV_NAME,
>> + .id_table = iwmct_ids,
>> +};
>> +
>> +static int __init iwmct_init(void)
>> +{
>> + int rc;
>> +
>> + /* Default log filter settings */
>> + iwmct_log_set_filter(LOG_SRC_ALL, LOG_SEV_FILTER_RUNTIME);
>> + iwmct_log_set_filter(LOG_SRC_FW_MSG, LOG_SEV_FILTER_ALL);
>> + iwmct_log_set_fw_filter(LOG_SRC_ALL, FW_LOG_SEV_FILTER_RUNTIME);
>> +
>> + rc = sdio_register_driver(&iwmct_driver);
>> +
>> + return rc;
>> +}
>
> Actually return sdio_register_driver() would do the same trick.
>
>> +
>> +static void __exit iwmct_exit(void)
>> +{
>> + sdio_unregister_driver(&iwmct_driver);
>> +}
>> +
>> +module_init(iwmct_init);
>> +module_exit(iwmct_exit);
>> +
>
> At some point I got lost in this code. It contains more debug statements
> and infrastructure than actual code as it seems. Can you please clean
> this up. I mentioned it above. Just use dynamic_printk and you should be
> have something clean and simple.
>
> Regards
>
> Marcel
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
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
* Re: PATCH: Network Device Naming mechanism and policy
From: Bill Fink @ 2009-10-10 18:11 UTC (permalink / raw)
To: Greg KH
Cc: Matt Domsch, Stephen Hemminger, netdev, linux-hotplug, Narendra_K,
jordan_hargrave
In-Reply-To: <20091010052308.GA12458@kroah.com>
On Fri, 9 Oct 2009, Greg KH wrote:
> On Fri, Oct 09, 2009 at 11:40:57PM -0500, Matt Domsch wrote:
> > The fundamental roadblock to this is that enumeration != naming,
> > except that it is for network devices, and we keep changing the
> > enumeration order.
>
> No, the hardware changes the enumeration order, it places _no_
> guarantees on what order stuff will be found in. So this is not the
> kernel changing, just to be clear.
>
> Again, I have a machine here that likes to reorder PCI devices every 4th
> or so boot times, and that's fine according to the PCI spec. Yeah, it's
> a crappy BIOS, but the manufacturer rightly pointed out that it is not
> in violation of anything.
>
> > Today, port naming is completely nondeterministic. If you have but
> > one NIC, there are few chances to get the name wrong (it'll be eth0).
> > If you have >1 NIC, chances increase to get it wrong.
>
> That is why all distros name network devices based on the only
> deterministic thing they have today, the MAC address. I still fail to
> see why you do not like this solution, it is honestly the only way to
> properly name network devices in a sane manner.
>
> All distros also provide a way to easily rename the network devices, to
> place a specific name on a specific MAC address, so again, this should
> all be solved already.
>
> No matter how badly your BIOS teams mess up the PCI enumeration order :)
No comment on the specific implementation decision, but I am in the
process of setting up a large number of test systems with identical
hardware configurations, and using a master disk image to clone all the
test systems. The biggest pain in this process is identiying the MAC
addresses for each of the six or more network interfaces in each test
system (we want eth0...ethN to always reference the same physical port
on the test systems), and then having to modify the 70-persistent-net.rules
udev file and the HWADDR entry for all the ifcfg-ethX files to reflect
the correct MAC addresses. It would be fantastic if there were some
mechanism for making this part of the process unnecessary.
-Bill
^ permalink raw reply
* Re: Real networking namespace
From: Casey Schaufler @ 2009-10-10 18:14 UTC (permalink / raw)
To: Paul Moore
Cc: Stephen Smalley, Stephen Hemminger, linux-security-module,
Al Viro, netdev, James Morris, Casey Schaufler
In-Reply-To: <200910091812.16046.paul.moore@hp.com>
Paul Moore wrote:
> On Friday 09 October 2009 12:44:52 pm Stephen Smalley wrote:
>
>> On Fri, 2009-10-09 at 12:37 -0400, Stephen Smalley wrote:
>>
>>> On Fri, 2009-10-09 at 08:38 -0700, Stephen Hemminger wrote:
>>>
>>>> The existing networking namespace model is unattractive for what I
>>>> want, has anyone investigated better alternatives?
>>>>
>>>> I would like to be able to allow access to a network interface and
>>>> associated objects (routing tables etc), to be controlled by Mandatory
>>>> Access Control API's.
As I'll mention later, getting agreement on what qualifies as an
object in the networking stack ain't going to happen anytime soon.
Sure, routing tables are important components of the system's
state, but they don't qualify as objects under any definition of
objects with which I'm familiar. Similarly, a network device is
more like a disk controller than a directory, and no one I know
of wants to start doing access checks based on the disk controller
(file system, yes, controller, no) that a file resides on.
The ad hoc security mechanisms for networking include firewalls,
netfilter, and routing schemes. These are very interesting and
useful things, but they don't have anything to do with the
"subject accesses object" mindset. Trying to shoehorn them in
always results in tears.
>>>> I.e grant access to eth0 and to only certain
>>>> processes. Some the issues with the existing models are:
>>>> * eth0 and associated objects don't really exist in filesystem so
>>>> not subject to LSM style control (SeLinux/SMACK/TOMOYO)
>>>>
>
> As Stephen points out, SELinux does have the ability to assign security labels
> to network interfaces, check out the 'semanage' command. A while back I wrote
> up something about the SELinux network "ingress/egress" access controls:
>
> * http://paulmoore.livejournal.com/2128.html
>
> Smack doesn't support controlling network access at the interface level, but
> that is due to a Smack design decision and not an inherent functionality gap
> in the LSM.
Paul is correct. A security model that includes network interface
devices as policy components has all the tools it needs at its
disposal. The Smack model does not consider network interface devices
as policy components. Certainly there are data import/export issues
that get raised with the Smack model, but they center around the
question of whether sending a packet on the network is in fact an
export and whether receiving a packet is an import. You can argue
it either way, and the implications are kind of painful whichever
way you choose. You really only want network interface devices as
policy components if you consider network traffic as import/export,
in which case you have serious work to do explaining why it is
acceptable to do multi-label import/export over that media.
Smack treats incoming packets as IPC messages from subjects that
may be elsewhere. The label on the packet, which may be based on
the host the packet came from, is the only information that Smack
cares about.
> TOMOYO is currently working on improved network access controls
> (see patches posted earlier this week), I haven't had a chance to review them
> yet so I don't know the state of TOMOYO's network access controls.
>
>
>>>> * network namespaces do not allow object to exist in multiple
>>>> namespaces. The current model is more restrictive than chroot jails. At
>>>> least with chroot, put filesystem objects in multiple jails.
>>>>
>
> Perhaps I don't fully understand what you are getting at here, but I don't
> think this should be an issue with a flexible LSM.
>
>
>>> Is there something that prevents you from using the existing SELinux
>>> network access controls? netif is a security class governed by SELinux
>>> policy, and routing table operations would be covered by the SELinux
>>> checks on netlink_route_socket. SELinux uses a combination of LSM hooks
>>> and netfilter hooks to mediate network operations.
>>>
>> Also, depending on what you want to do, SECMARK may be useful to you.
>> That allows you to mark packets with security contexts via iptables, and
>> then use SELinux policy to control their flow.
>> http://paulmoore.livejournal.com/4281.html
>> http://james-morris.livejournal.com/11010.html
>>
>
> While we're at it, a few more links ... here is a presentation from last year
> on Linux's labeled networking capabilities (which hits at a lot of your
> questions):
>
> * http://paulmoore.livejournal.com/964.html
>
> ... and there is a video too:
>
> * http://paulmoore.livejournal.com/1329.html
>
>
^ permalink raw reply
* Re: PATCH: Network Device Naming mechanism and policy
From: Stephen Hemminger @ 2009-10-10 18:32 UTC (permalink / raw)
To: Matt Domsch; +Cc: netdev, linux-hotplug, Narendra_K, jordan_hargrave
In-Reply-To: <20091010044056.GA5350@mock.linuxdev.us.dell.com>
On Fri, 9 Oct 2009 23:40:57 -0500
Matt Domsch <Matt_Domsch@dell.com> wrote:
> On Fri, Oct 09, 2009 at 07:44:01PM -0700, Stephen Hemminger wrote:
> > Maybe I'm dense but can't see why having a useless /dev/net/ symlinks
> > is a good interface choice. Perhaps you should explain the race between
> > PCI scan and udev in more detail, and why solving it in either of those
> > places won't work. As it stands you are proposing yet another wart to
> > the already complex set of network interface API's which has implications
> > for security as well as increasing the number of possible bugs.
>
> The fundamental challenge is that system administrators, particularly
> those of server-class hardware with multiple network ports present
> (some on the motherboard, some on add-in cards), have the
> not-so-unreasonable expectation that there is a deterministic mapping
> between those ports and the name one uses to address those ports.
>
> The fundamental roadblock to this is that enumeration != naming,
> except that it is for network devices, and we keep changing the
> enumeration order.
>
> Today, port naming is completely nondeterministic. If you have but
> one NIC, there are few chances to get the name wrong (it'll be eth0).
> If you have >1 NIC, chances increase to get it wrong.
>
> The complexity arises at multiple levels.
>
> First, device driver load order. In the 2.4 kernel days, and even
> mostly early 2.6 kernel days, the order in which network drivers
> loaded played a role in determining the name of the device. Drivers
> loaded first would get their devices named first. If I have two types
> of devices, say an e100-driven NIC and a tg3-driven NIC, I could
> figure out that the names would be eth0=e100 and eth1=tg3 by setting
> the load order in /etc/modules.conf (now modprobe.conf). If I wanted
> the other order, fine, just switch it around in modules.conf and
> reboot. OS installers, being the first running instance of Linux,
> before modprobe.conf existed to set that ordering, had to have other
> mechanisms to load drivers (often manually, or if programmatically
> such as in a kickstart or autoyast file, was still somewhat fixed).
>
> With the advent of modaliases + udev, now modprobe.conf doesn't
> contain this ordering anymore, and udev loads the drivers. So while
> it wasn't perfect, it was better than nothing, and that's gone now.
>
> It gets even worse as, to speed up boot time, modprobes can be run in
> parallel, and even within individual drivers, the NICs get initialized
> (and named) in parallel. Further confusing things, some devices need
> firmware loaded into them before getting names assigned, which is done
> from userspace, and they race.
>
> Second, PCI device list order. In the 2.4 kernel days, the PCI device
> list was scanned "breadth-first" (for each bus; for each device; for
> each function; do load...). FWIW, Windows still does this. It gives
> BIOS, which assigns PCI bus numbers, a chance to put LOMs at a lower
> bus number than add-in cards. Module load order still mattered, but
> at least if you had say 2 e1000 ports as LOMs, and 2 e1000 ports on
> add-in cards, you pretty much knew the ordering would be eth0 as
> lowest bdf on the motherboard, eth1 as next bdf on the motherboard,
> and eth2 and 3 as the add-in cards in ascending slot order.
>
> With the advent of PCI hot plug in the 2.5 kernel series, the
> breadth-first ordering became depth-first. (for each bus; for each
> device; if the device is a bridge, scan the busses behind it.). This
> caused NICs on bus 0 device 5, and bus 1 device 3, (eth0 and 1
> respectively) to be enumerated differently due to the a bridge from
> bus 0 to bus 1 at 0:4. My crude hack of pci=bfsort, with some dmi
> strings to match and auto-enable, at least reverted this back to the
> ordering the 2.4 kernel and Windows used. Now we have to keep adding
> systems to this DMI list (Dell has a number of systems on this list
> today; HP has even more). And it doesn't completely solve the
> problem, just masks it.
>
> So, to address the ordering problem, I placed a constraint on our
> server hardware teams, forcing them to lay out their boards and assign
> PCIe lanes and bus numbers, such that at least the designed "first"
> LOM would get found first in either depth-first or breadth-first
> order. Our 10G and 11G servers have this restriction in place, though
> it wasn't easy. And it's gotten even harder, as the PCIe switches
> expand the number of lanes available. We no longer have the
> traditional tiered buses architecture, but the PCI layer for this
> purpose thinks we do. I need to remove this constraint on the
> hardware teams - it's gotten to be impossible for the chipset lanes to
> be laid out efficiently with this constraint.
>
> All of the above just papered over the enumeration != naming problem.
>
> Third, stateless computing is becoming more and more commonplace. The
> Field Replaceable Unit is the server itself. Got a bad server? Pull
> it out, move the disks to an identical unit, insert the new server,
> and go. Fix the bad server offline and bring it back. In this model,
> having MAC addresses as the mechanism that is providing the
> determinism (/etc/mactab or udev persistent naming rules) breaks,
> because the MAC addresses of the ports on the new server won't be the
> same as on the old server. HP even has a technology to solve _this_
> problem (in their blade chassis) - Virtual Connect. The MACs get
> assigned by the chassis to the blades at POST, and are fixed to the
> slot. Slick, and Dell has an even more flexible similar feature
> FlexAddress. This doesn't solve the OS installer problem of "which of
> these NICs should I use to do an install?" but it does recognize the
> problem space and tries to overcome it.
>
> Fourth, for OS installers, choosing which NIC to use at installtime,
> when all the NICs are plugged in, can be difficult. PXE environments,
> using pxelinux and its IPAPPEND 2 option, will append
> "BOOTIF=xx:xx:xx:xx:xx:xx" to the kernel command line, that
> containing the MAC address of the NIC used for PXE. Neat trick. Yes,
> we then had to teach the OS installers to recognize and use this. But
> it only works if you PXE boot, and only for that one NIC.
>
> Fifth, network devices can have only a single name. eth0. If we look
> at disks, we see udev manages a tree of symlinks for
> /dev/disk/by-label, /dev/disk/by-path, /dev/disk/by-uuid. And as a
> system admin, if I wanted to also create a udev rule for
> /dev/disk/by-function (boot, swap, mattsstorage), it's trivial to do
> so. Why can't we have this flexibility for network devices too?
>
> So, how do we get deterministic naming for all the NICs in a system?
> That's what I'm going for. Picture a network switch, with several
> blades, and several ports on each blade. The network admin addresses
> each port as say 1/16 (the 16th port on blade 1, clearly labeled).
> The parallel on servers is the chassis label printed on the outside
> (say, "Gb1"). But due the above, there is no guarantee, and in fact
> little chance, that Gb1 will be consistently named eth0 - it may vary
> from boot to boot. That's full of fail.
>
> For a concrete example, the 4 bnx2 chips in my PowerEdge R610 with a
> current 2.6 kernel, loading only one driver, the ports get assigned
> names in nondeterministic order on each boot. Given that the
> ifcfg-eth* rules, netfilter rules, and the rest all expect
> deterministic naming, massive failure ensues unless some form of
> determinism is brought back in.
>
> The idea to use a character device node to expose the ifindex value,
> and udev to manage a tree of symlinks to it, really follows the model
> used today for disks. It allows us to get deterministic names for
> devices (albeit, the names are symlinks), and multiple names for
> devices (through multiple symlink rules). That some people want to
> use the char device to call ioctl() and read/write, as is possible on
> the BSDs, would just be gravy IMHO.
>
> It does require a change in behavior for a system administrator.
> Instead of hard-coding 'eth0' into her scripts, she uses
> '/dev/net/by-function/boot' or somesuch. But then that name is
> guaranteed to always refer to the "right" NIC. Every admin I've
> spoken to is willing to make this kind of change, as long as they get
> the consistent, deterministic naming they expect but don't have
> today. And it does require patching userspace apps to take both a
> kernel device name, or a path, and to resolve the path to device name
> or ifindex. We wrote libnetdevname (really, one function), and have
> patches for several userspace apps to use it, to prove it can be done.
>
> One alternative would be to do something using the sysfs ifindex value
> already exported. e.g.
> /sys/devices/pci0000:00/0000:00:05.0/0000:05:00.0/0000:06:07.0/net/eth0/ifindex
>
> but we have never had symlinks from /dev into /sys before (doesn't
> mean we couldn't though). In that case, udev would grow to manage
> /dev/net/by-chassis-label/Embedded_NIC_1 -> /sys/devices/.../net/eth0,
> and libnetdevname would be used to follow the symlink in applications.
> This approach could solve my problem without (many or any?) kernel
> changes needed, but wouldn't help those who want to do
> ioctl/read/write to a devnode.
>
> Given the problem, I really do need a solution. I've proposed one
> method, and an alternative, but I can't afford to let the problem stay
> unaddressed any longer, and need a clear direction to be chosen. The
> char device gives me what I need, and others what they want also.
>
> Thanks for listening to the diatribe. For more examples and
> workarounds that we've been telling our customers for several years,
> check out http://linux.dell.com/papers.shtml for the Network Interface
> Card Naming whitepaper.
>
>
Why isn't the available through sysfs enough, if not why not
add the necessary attributes there.
BTW, for our distro, we are looking into device renaming based on PCI slot
because that is what router OS's do. Customers expect if they replace the card
in slot 0, it will come back with the same name. This is not what server
customers expect.
^ permalink raw reply
* Re: PATCH: Network Device Naming mechanism and policy
From: Kay Sievers @ 2009-10-10 18:35 UTC (permalink / raw)
To: Bill Fink
Cc: Greg KH, Matt Domsch, Stephen Hemminger, netdev, linux-hotplug,
Narendra_K, jordan_hargrave
In-Reply-To: <20091010141124.82d226b8.billfink@mindspring.com>
On Sat, Oct 10, 2009 at 20:11, Bill Fink <billfink@mindspring.com> wrote:
> No comment on the specific implementation decision, but I am in the
> process of setting up a large number of test systems with identical
> hardware configurations, and using a master disk image to clone all the
> test systems. The biggest pain in this process is identiying the MAC
> addresses for each of the six or more network interfaces in each test
> system (we want eth0...ethN to always reference the same physical port
> on the test systems), and then having to modify the 70-persistent-net.rules
> udev file and the HWADDR entry for all the ifcfg-ethX files to reflect
> the correct MAC addresses. It would be fantastic if there were some
> mechanism for making this part of the process unnecessary.
Udev creates the persistent rules only if no other rule set a name.
Adding something like:
SUBSYSTEM=="net", KERNEL==""eth*", NAME="eth%n"
in any earlier rules file before the udev generated one will skip all
off the automatic udev rule creation.
Kay
^ permalink raw reply
* Re: PATCH: Network Device Naming mechanism and policy
From: Ben Hutchings @ 2009-10-10 19:00 UTC (permalink / raw)
To: Greg KH
Cc: Sujit K M, Matt Domsch, Stephen Hemminger, netdev, linux-hotplug,
Narendra_K, jordan_hargrave
In-Reply-To: <20091010162703.GB30354@kroah.com>
On Sat, 2009-10-10 at 09:27 -0700, Greg KH wrote:
> On Sat, Oct 10, 2009 at 01:47:39PM +0530, Sujit K M wrote:
> > Greg,
> >
> >
> > > No, the hardware changes the enumeration order, it places _no_
> > > guarantees on what order stuff will be found in. ?So this is not the
> > > kernel changing, just to be clear.
> > > Again, I have a machine here that likes to reorder PCI devices every 4th
> > > or so boot times, and that's fine according to the PCI spec. ?Yeah, it's
> > > a crappy BIOS, but the manufacturer rightly pointed out that it is not
> > > in violation of anything.
> > >
> >
> > I think the open call should be implemented then. By the patch very little
> > knowledge is being shared on type of network implementation it is trying to
> > do.
>
> What would open() accomplish? What good would the file descriptor be?
> What could you use it for?
Currently all net device ioctls are carried out through arbitrary
sockets and identify the device by name (aside from one to look up the
name by ifindex). Ever since it became possible to rename net devices,
it has been possible for a sequence of ioctls intended for one device to
race with renaming of that device. Adding open() and ioctl() to the
character device (which seems reasonably easy) would provide a way to
avoid this.
On the other hand, the netlink configuration APIs already use ifindex so
it may be better just to say that the device ioctls are deprecated and
applications should use netlink.
> > Also it is messing with core datastructure and procedures. This seems
> > to be simplified by changing implementing the other operations like poll().
>
> I don't understand.
>
> > > That is why all distros name network devices based on the only
> > > deterministic thing they have today, the MAC address. ?I still fail to
> > > see why you do not like this solution, it is honestly the only way to
> > > properly name network devices in a sane manner.
> >
> > This is feature that needs to be implemented. As per the rules followed.
>
> This feature is already implemented today, all distros have it.
No, see below.
> > > All distros also provide a way to easily rename the network devices, to
> > > place a specific name on a specific MAC address, so again, this should
> > > all be solved already.
> > >
> > > No matter how badly your BIOS teams mess up the PCI enumeration order :)
> >
> > This is an problem, But I think this can be solved by implementing some of the
> > routines in the network device.
>
> I don't, see the rules that your distro ships today for persistant
> network devices, it's already there, no need to change the kernel at
> all.
The udev persistent net rules work tolerably well for a single system
with a stable set of net devices.
They do not solve the problem Matt's talking about, which is lack of
consistency between multiple systems, because the initial enumeration
order is not predictable.
They also result in name changes when a NIC (or motherboard) is swapped.
For some users, that's fine; for others, it's not.
The ability to specify NICs by port name or PCI address should solve
these problems.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [Bonding-devel] [PATCH] net, bonding: Add return statement in bond_create_proc_entry.
From: Nicolas de Pesloüan @ 2009-10-10 19:19 UTC (permalink / raw)
To: Rakib Mullick
Cc: Jay Vosburgh, netdev, linux-kernel, Andrew Morton, bonding-devel
In-Reply-To: <b9df5fa10910091910u1b6bf14bg2781cb12c58b21f9@mail.gmail.com>
Rakib Mullick wrote:
> The function bond_create_proc_entry supposed to return int instead of void.
> And fixes the following compilation warning.
>
> drivers/net/bonding/bond_main.c: In function `bond_create_proc_entry':
> drivers/net/bonding/bond_main.c:3393: warning: control reaches end of
> non-void function
>
> ---
> Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
>
> --- linus/drivers/net/bonding/bond_main.c 2009-10-09 17:38:35.000000000 +0600
> +++ rakib/drivers/net/bonding/bond_main.c 2009-10-09 17:47:46.000000000 +0600
> @@ -3391,6 +3391,7 @@ static void bond_destroy_proc_dir(void)
>
> static int bond_create_proc_entry(struct bonding *bond)
> {
> + return 0;
> }
This empty function is defined inside the else branch of an ifdef. The corresponding non-empty
function always return 0 and no caller of this function use the returned value.
So I suggest to change the return type of this function from int to void, instead of adding a return
0 into the empty one.
Nicolas.
^ permalink raw reply
* [net-next PATCH 0/8] qlge: Cleanup and additions for qlge.
From: Ron Mercer @ 2009-10-10 19:35 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
Cleanup and a couple of small performance tweeks for qlge.
^ permalink raw reply
* [net-next PATCH 1/8] qlge: Remove explicit setting of PCI Dev CTL reg.
From: Ron Mercer @ 2009-10-10 19:35 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer, root
In-Reply-To: <1255203310-18114-1-git-send-email-ron.mercer@qlogic.com>
From: root <root@localhost.localdomain>
Remove explicit setting of error reporting bits.
Signed-off-by: root <root@localhost.localdomain>
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge_main.c | 16 +---------------
1 files changed, 1 insertions(+), 15 deletions(-)
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 48b45df..7a9dca8 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -3868,8 +3868,7 @@ static int __devinit ql_init_device(struct pci_dev *pdev,
struct net_device *ndev, int cards_found)
{
struct ql_adapter *qdev = netdev_priv(ndev);
- int pos, err = 0;
- u16 val16;
+ int err = 0;
memset((void *)qdev, 0, sizeof(*qdev));
err = pci_enable_device(pdev);
@@ -3881,19 +3880,6 @@ static int __devinit ql_init_device(struct pci_dev *pdev,
qdev->ndev = ndev;
qdev->pdev = pdev;
pci_set_drvdata(pdev, ndev);
- pos = pci_find_capability(pdev, PCI_CAP_ID_EXP);
- if (pos <= 0) {
- dev_err(&pdev->dev, PFX "Cannot find PCI Express capability, "
- "aborting.\n");
- return pos;
- } else {
- pci_read_config_word(pdev, pos + PCI_EXP_DEVCTL, &val16);
- val16 &= ~PCI_EXP_DEVCTL_NOSNOOP_EN;
- val16 |= (PCI_EXP_DEVCTL_CERE |
- PCI_EXP_DEVCTL_NFERE |
- PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE);
- pci_write_config_word(pdev, pos + PCI_EXP_DEVCTL, val16);
- }
err = pci_request_regions(pdev, DRV_NAME);
if (err) {
--
1.6.0.2
^ permalink raw reply related
* [net-next PATCH 2/8] qlge: Set PCIE max read request size.
From: Ron Mercer @ 2009-10-10 19:35 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer, root
In-Reply-To: <1255203310-18114-1-git-send-email-ron.mercer@qlogic.com>
From: root <root@localhost.localdomain>
Signed-off-by: root <root@localhost.localdomain>
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge_main.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 7a9dca8..aeb0104 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -3881,6 +3881,13 @@ static int __devinit ql_init_device(struct pci_dev *pdev,
qdev->pdev = pdev;
pci_set_drvdata(pdev, ndev);
+ /* Set PCIe read request size */
+ err = pcie_set_readrq(pdev, 4096);
+ if (err) {
+ dev_err(&pdev->dev, "Set readrq failed.\n");
+ goto err_out;
+ }
+
err = pci_request_regions(pdev, DRV_NAME);
if (err) {
dev_err(&pdev->dev, "PCI region request failed.\n");
--
1.6.0.2
^ permalink raw reply related
* [net-next PATCH 3/8] qlge: Add handler for DCBX firmware event.
From: Ron Mercer @ 2009-10-10 19:35 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1255203310-18114-1-git-send-email-ron.mercer@qlogic.com>
The driver has nothing to do, but this marker prevents the event from
showing up 'not handled'.
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge_mpi.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/qlge/qlge_mpi.c b/drivers/net/qlge/qlge_mpi.c
index 99e58e3..2e83c4b 100644
--- a/drivers/net/qlge/qlge_mpi.c
+++ b/drivers/net/qlge/qlge_mpi.c
@@ -446,6 +446,9 @@ static int ql_mpi_handler(struct ql_adapter *qdev, struct mbox_params *mbcp)
ql_aen_lost(qdev, mbcp);
break;
+ case AEN_DCBX_CHG:
+ /* Need to support AEN 8110 */
+ break;
default:
QPRINTK(qdev, DRV, ERR,
"Unsupported AE %.08x.\n", mbcp->mbox_out[0]);
--
1.6.0.2
^ permalink raw reply related
* [net-next PATCH 4/8] qlge: Store firmware revision as early as possible.
From: Ron Mercer @ 2009-10-10 19:35 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1255203310-18114-1-git-send-email-ron.mercer@qlogic.com>
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge_mpi.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/qlge/qlge_mpi.c b/drivers/net/qlge/qlge_mpi.c
index 2e83c4b..9c0dfe0 100644
--- a/drivers/net/qlge/qlge_mpi.c
+++ b/drivers/net/qlge/qlge_mpi.c
@@ -317,6 +317,7 @@ static void ql_init_fw_done(struct ql_adapter *qdev, struct mbox_params *mbcp)
} else {
QPRINTK(qdev, DRV, ERR, "Firmware Revision = 0x%.08x.\n",
mbcp->mbox_out[1]);
+ qdev->fw_rev_id = mbcp->mbox_out[1];
status = ql_cam_route_initialize(qdev);
if (status)
QPRINTK(qdev, IFUP, ERR,
--
1.6.0.2
^ permalink raw reply related
* [net-next PATCH 5/8] qlge: Remove inline math for small rx buf mapping.
From: Ron Mercer @ 2009-10-10 19:35 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1255203310-18114-1-git-send-email-ron.mercer@qlogic.com>
rx_ring->sbq_buf_len now holds the length of the mapped portion of the
buffer rather than the overall length.
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge.h | 3 ++-
drivers/net/qlge/qlge_main.c | 14 +++++++-------
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index e7285f0..633fcd1 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -54,7 +54,8 @@
#define RX_RING_SHADOW_SPACE (sizeof(u64) + \
MAX_DB_PAGES_PER_BQ(NUM_SMALL_BUFFERS) * sizeof(u64) + \
MAX_DB_PAGES_PER_BQ(NUM_LARGE_BUFFERS) * sizeof(u64))
-#define SMALL_BUFFER_SIZE 256
+#define SMALL_BUFFER_SIZE 512
+#define SMALL_BUF_MAP_SIZE (SMALL_BUFFER_SIZE / 2)
#define LARGE_BUFFER_SIZE PAGE_SIZE
#define MAX_SPLIT_SIZE 1023
#define QLGE_SB_PAD 32
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index aeb0104..09247ab 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -1147,7 +1147,7 @@ static void ql_update_sbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
sbq_desc->index);
sbq_desc->p.skb =
netdev_alloc_skb(qdev->ndev,
- rx_ring->sbq_buf_size);
+ SMALL_BUFFER_SIZE);
if (sbq_desc->p.skb == NULL) {
QPRINTK(qdev, PROBE, ERR,
"Couldn't get an skb.\n");
@@ -1157,8 +1157,8 @@ static void ql_update_sbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
skb_reserve(sbq_desc->p.skb, QLGE_SB_PAD);
map = pci_map_single(qdev->pdev,
sbq_desc->p.skb->data,
- rx_ring->sbq_buf_size /
- 2, PCI_DMA_FROMDEVICE);
+ rx_ring->sbq_buf_size,
+ PCI_DMA_FROMDEVICE);
if (pci_dma_mapping_error(qdev->pdev, map)) {
QPRINTK(qdev, IFUP, ERR, "PCI mapping failed.\n");
rx_ring->sbq_clean_idx = clean_idx;
@@ -1168,7 +1168,7 @@ static void ql_update_sbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
}
pci_unmap_addr_set(sbq_desc, mapaddr, map);
pci_unmap_len_set(sbq_desc, maplen,
- rx_ring->sbq_buf_size / 2);
+ rx_ring->sbq_buf_size);
*sbq_desc->addr = cpu_to_le64(map);
}
@@ -2692,7 +2692,7 @@ static int ql_start_rx_ring(struct ql_adapter *qdev, struct rx_ring *rx_ring)
cqicb->sbq_addr =
cpu_to_le64(rx_ring->sbq_base_indirect_dma);
cqicb->sbq_buf_size =
- cpu_to_le16((u16)(rx_ring->sbq_buf_size/2));
+ cpu_to_le16((u16)(rx_ring->sbq_buf_size));
bq_len = (rx_ring->sbq_len == 65536) ? 0 :
(u16) rx_ring->sbq_len;
cqicb->sbq_len = cpu_to_le16(bq_len);
@@ -3268,7 +3268,7 @@ static int ql_adapter_initialize(struct ql_adapter *qdev)
ql_write32(qdev, FSC, mask | value);
ql_write32(qdev, SPLT_HDR, SPLT_HDR_EP |
- min(SMALL_BUFFER_SIZE, MAX_SPLIT_SIZE));
+ min(SMALL_BUF_MAP_SIZE, MAX_SPLIT_SIZE));
/* Set RX packet routing to use port/pci function on which the
* packet arrived on in addition to usual frame routing.
@@ -3548,7 +3548,7 @@ static int ql_configure_rings(struct ql_adapter *qdev)
rx_ring->sbq_len = NUM_SMALL_BUFFERS;
rx_ring->sbq_size =
rx_ring->sbq_len * sizeof(__le64);
- rx_ring->sbq_buf_size = SMALL_BUFFER_SIZE * 2;
+ rx_ring->sbq_buf_size = SMALL_BUF_MAP_SIZE;
rx_ring->type = RX_Q;
} else {
/*
--
1.6.0.2
^ permalink raw reply related
* [net-next PATCH 6/8] qlge: Get rid of firmware handler debug code.
From: Ron Mercer @ 2009-10-10 19:35 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1255203310-18114-1-git-send-email-ron.mercer@qlogic.com>
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge_mpi.c | 21 ---------------------
1 files changed, 0 insertions(+), 21 deletions(-)
diff --git a/drivers/net/qlge/qlge_mpi.c b/drivers/net/qlge/qlge_mpi.c
index 9c0dfe0..e497eac 100644
--- a/drivers/net/qlge/qlge_mpi.c
+++ b/drivers/net/qlge/qlge_mpi.c
@@ -1,25 +1,5 @@
#include "qlge.h"
-static void ql_display_mb_sts(struct ql_adapter *qdev,
- struct mbox_params *mbcp)
-{
- int i;
- static char *err_sts[] = {
- "Command Complete",
- "Command Not Supported",
- "Host Interface Error",
- "Checksum Error",
- "Unused Completion Status",
- "Test Failed",
- "Command Parameter Error"};
-
- QPRINTK(qdev, DRV, DEBUG, "%s.\n",
- err_sts[mbcp->mbox_out[0] & 0x0000000f]);
- for (i = 0; i < mbcp->out_count; i++)
- QPRINTK(qdev, DRV, DEBUG, "mbox_out[%d] = 0x%.08x.\n",
- i, mbcp->mbox_out[i]);
-}
-
int ql_read_mpi_reg(struct ql_adapter *qdev, u32 reg, u32 *data)
{
int status;
@@ -540,7 +520,6 @@ static int ql_mailbox_command(struct ql_adapter *qdev, struct mbox_params *mbcp)
MB_CMD_STS_GOOD) &&
((mbcp->mbox_out[0] & 0x0000f000) !=
MB_CMD_STS_INTRMDT)) {
- ql_display_mb_sts(qdev, mbcp);
status = -EIO;
}
end:
--
1.6.0.2
^ permalink raw reply related
* [net-next PATCH 7/8] qlge: Don't fail open when port is not initialized.
From: Ron Mercer @ 2009-10-10 19:35 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1255203310-18114-1-git-send-email-ron.mercer@qlogic.com>
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge_main.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 09247ab..9eefb11 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -3310,10 +3310,8 @@ static int ql_adapter_initialize(struct ql_adapter *qdev)
/* Initialize the port and set the max framesize. */
status = qdev->nic_ops->port_initialize(qdev);
- if (status) {
- QPRINTK(qdev, IFUP, ERR, "Failed to start port.\n");
- return status;
- }
+ if (status)
+ QPRINTK(qdev, IFUP, ERR, "Failed to start port.\n");
/* Set up the MAC address and frame routing filter. */
status = ql_cam_route_initialize(qdev);
--
1.6.0.2
^ permalink raw reply related
* [net-next PATCH 8/8] qlge: Add CBFC pause frame counters to ethtool stats.
From: Ron Mercer @ 2009-10-10 19:35 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1255203310-18114-1-git-send-email-ron.mercer@qlogic.com>
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge.h | 21 ++++++++++++
drivers/net/qlge/qlge_ethtool.c | 69 +++++++++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+), 0 deletions(-)
diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 633fcd1..fd47691 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -1363,6 +1363,27 @@ struct nic_stats {
u64 rx_1024_to_1518_pkts;
u64 rx_1519_to_max_pkts;
u64 rx_len_err_pkts;
+ /*
+ * These stats come from offset 500h to 5C8h
+ * in the XGMAC register.
+ */
+ u64 tx_cbfc_pause_frames0;
+ u64 tx_cbfc_pause_frames1;
+ u64 tx_cbfc_pause_frames2;
+ u64 tx_cbfc_pause_frames3;
+ u64 tx_cbfc_pause_frames4;
+ u64 tx_cbfc_pause_frames5;
+ u64 tx_cbfc_pause_frames6;
+ u64 tx_cbfc_pause_frames7;
+ u64 rx_cbfc_pause_frames0;
+ u64 rx_cbfc_pause_frames1;
+ u64 rx_cbfc_pause_frames2;
+ u64 rx_cbfc_pause_frames3;
+ u64 rx_cbfc_pause_frames4;
+ u64 rx_cbfc_pause_frames5;
+ u64 rx_cbfc_pause_frames6;
+ u64 rx_cbfc_pause_frames7;
+ u64 rx_nic_fifo_drop;
};
/*
diff --git a/drivers/net/qlge/qlge_ethtool.c b/drivers/net/qlge/qlge_ethtool.c
index 5207394..aac6c6f 100644
--- a/drivers/net/qlge/qlge_ethtool.c
+++ b/drivers/net/qlge/qlge_ethtool.c
@@ -132,6 +132,41 @@ static void ql_update_stats(struct ql_adapter *qdev)
iter++;
}
+ /*
+ * Get Per-priority TX pause frame counter statistics.
+ */
+ for (i = 0x500; i < 0x540; i += 8) {
+ if (ql_read_xgmac_reg64(qdev, i, &data)) {
+ QPRINTK(qdev, DRV, ERR,
+ "Error reading status register 0x%.04x.\n", i);
+ goto end;
+ } else
+ *iter = data;
+ iter++;
+ }
+
+ /*
+ * Get Per-priority RX pause frame counter statistics.
+ */
+ for (i = 0x568; i < 0x5a8; i += 8) {
+ if (ql_read_xgmac_reg64(qdev, i, &data)) {
+ QPRINTK(qdev, DRV, ERR,
+ "Error reading status register 0x%.04x.\n", i);
+ goto end;
+ } else
+ *iter = data;
+ iter++;
+ }
+
+ /*
+ * Get RX NIC FIFO DROP statistics.
+ */
+ if (ql_read_xgmac_reg64(qdev, 0x5b8, &data)) {
+ QPRINTK(qdev, DRV, ERR,
+ "Error reading status register 0x%.04x.\n", i);
+ goto end;
+ } else
+ *iter = data;
end:
ql_sem_unlock(qdev, qdev->xg_sem_mask);
quit:
@@ -185,6 +220,23 @@ static char ql_stats_str_arr[][ETH_GSTRING_LEN] = {
{"rx_1024_to_1518_pkts"},
{"rx_1519_to_max_pkts"},
{"rx_len_err_pkts"},
+ {"tx_cbfc_pause_frames0"},
+ {"tx_cbfc_pause_frames1"},
+ {"tx_cbfc_pause_frames2"},
+ {"tx_cbfc_pause_frames3"},
+ {"tx_cbfc_pause_frames4"},
+ {"tx_cbfc_pause_frames5"},
+ {"tx_cbfc_pause_frames6"},
+ {"tx_cbfc_pause_frames7"},
+ {"rx_cbfc_pause_frames0"},
+ {"rx_cbfc_pause_frames1"},
+ {"rx_cbfc_pause_frames2"},
+ {"rx_cbfc_pause_frames3"},
+ {"rx_cbfc_pause_frames4"},
+ {"rx_cbfc_pause_frames5"},
+ {"rx_cbfc_pause_frames6"},
+ {"rx_cbfc_pause_frames7"},
+ {"rx_nic_fifo_drop"},
};
static void ql_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
@@ -257,6 +309,23 @@ ql_get_ethtool_stats(struct net_device *ndev,
*data++ = s->rx_1024_to_1518_pkts;
*data++ = s->rx_1519_to_max_pkts;
*data++ = s->rx_len_err_pkts;
+ *data++ = s->tx_cbfc_pause_frames0;
+ *data++ = s->tx_cbfc_pause_frames1;
+ *data++ = s->tx_cbfc_pause_frames2;
+ *data++ = s->tx_cbfc_pause_frames3;
+ *data++ = s->tx_cbfc_pause_frames4;
+ *data++ = s->tx_cbfc_pause_frames5;
+ *data++ = s->tx_cbfc_pause_frames6;
+ *data++ = s->tx_cbfc_pause_frames7;
+ *data++ = s->rx_cbfc_pause_frames0;
+ *data++ = s->rx_cbfc_pause_frames1;
+ *data++ = s->rx_cbfc_pause_frames2;
+ *data++ = s->rx_cbfc_pause_frames3;
+ *data++ = s->rx_cbfc_pause_frames4;
+ *data++ = s->rx_cbfc_pause_frames5;
+ *data++ = s->rx_cbfc_pause_frames6;
+ *data++ = s->rx_cbfc_pause_frames7;
+ *data++ = s->rx_nic_fifo_drop;
}
static int ql_get_settings(struct net_device *ndev,
--
1.6.0.2
^ permalink raw reply related
* [PATCH netnext-2.6] bonding: change bond_create_proc_entry() to return void
From: Nicolas de Pesloüan @ 2009-10-10 20:41 UTC (permalink / raw)
To: fubar, davem, netdev, bonding-devel, rakib.mullick
Cc: Nicolas de Pesloüan
In-Reply-To: <b9df5fa10910091910u1b6bf14bg2781cb12c58b21f9@mail.gmail.com>
The function bond_create_proc_entry is currently of type int.
Two versions of this function exist:
The one in the ifdef CONFIG_PROC_FS branch always return 0.
The one in the else branch (which is empty) return nothing.
When CONFIG_PROC_FS is undef, this cause the following warning:
drivers/net/bonding/bond_main.c: In function `bond_create_proc_entry':
drivers/net/bonding/bond_main.c:3393: warning: control reaches end of
non-void function
No caller of this function use the returned value.
So change the returned type from int to void and remove the
useless return 0; .
Signed-off-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
Reported-by: Rakib Mullick <rakib.mullick@gmail.com>
---
drivers/net/bonding/bond_main.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ef6af1c..feb03ad 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3375,7 +3375,7 @@ static const struct file_operations bond_info_fops = {
.release = seq_release,
};
-static int bond_create_proc_entry(struct bonding *bond)
+static void bond_create_proc_entry(struct bonding *bond)
{
struct net_device *bond_dev = bond->dev;
@@ -3390,8 +3390,6 @@ static int bond_create_proc_entry(struct bonding *bond)
else
memcpy(bond->proc_file_name, bond_dev->name, IFNAMSIZ);
}
-
- return 0;
}
static void bond_remove_proc_entry(struct bonding *bond)
@@ -3430,7 +3428,7 @@ static void bond_destroy_proc_dir(void)
#else /* !CONFIG_PROC_FS */
-static int bond_create_proc_entry(struct bonding *bond)
+static void bond_create_proc_entry(struct bonding *bond)
{
}
--
1.6.3.3
^ permalink raw reply related
* Re: PATCH: Network Device Naming mechanism and policy
From: Greg KH @ 2009-10-10 21:06 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Matt Domsch, netdev, linux-hotplug, Narendra_K, jordan_hargrave
In-Reply-To: <20091010113219.3136fb8b@s6510>
On Sat, Oct 10, 2009 at 11:32:19AM -0700, Stephen Hemminger wrote:
>
> BTW, for our distro, we are looking into device renaming based on PCI slot
> because that is what router OS's do. Customers expect if they replace the card
> in slot 0, it will come back with the same name. This is not what server
> customers expect.
If your bios exposes the PCI slots to userspace (through the proper ACPI
namespace), doing this type of naming should be trivial with some simple
udev rules, no additional kernel infrastructure is needed.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 00/31] Swap over NFS -v20
From: Pavel Machek @ 2009-10-10 21:10 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Christoph Hellwig, Suresh Jayaraman, Linus Torvalds,
Andrew Morton, linux-kernel, linux-mm, netdev, Neil Brown,
Miklos Szeredi, Wouter Verhelst, trond.myklebust
In-Reply-To: <1255177421.11081.0.camel@twins>
On Sat 2009-10-10 14:23:41, Peter Zijlstra wrote:
> On Sat, 2009-10-10 at 14:06 +0200, Pavel Machek wrote:
> > Hi!
> >
> > > > One of them
> > > > would be the whole VM/net work to just make swap over nbd/iscsi safe.
> > >
> > > Getting those two 'fixed' is going to be tons of interesting work
> > > because they involve interaction with userspace daemons.
> > >
> > > NBD has fairly simple userspace, but iSCSI has a rather large userspace
> > > footprint and a rather complicated user/kernel interaction which will be
> > > mighty interesting to get allocation safe.
> > >
> > > Ideally the swap-over-$foo bits have no userspace component.
> > >
> > > That said, Wouter is the NBD userspace maintainer and has expressed
> > > interest into looking at making that work, but its sure going to be
> > > non-trivial, esp. since exposing PF_MEMALLOC to userspace is a, not over
> > > my dead-bodym like thing.
> >
> > Well, as long as nbd-server is on separate machine (with real swap),
> > safe swapping over network should be ok, without PF_MEMALLOC for
> > userspace or similar nightmares, right?
>
> Nope, as soon as the nbd-client looses its connection you're up shit
> creek.
Oops, right. Putting reconnect logic into the kernel would make sense.
I misunderstood your proposal. I thought you'd want to put
nbd-_server_ into the kernel too. I guess we violently agree that
that's unneccessary.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: PATCH: Network Device Naming mechanism and policy
From: Greg KH @ 2009-10-10 21:10 UTC (permalink / raw)
To: Ben Hutchings
Cc: Sujit K M, Matt Domsch, Stephen Hemminger, netdev, linux-hotplug,
Narendra_K, jordan_hargrave
In-Reply-To: <1255201230.25061.60.camel@localhost>
On Sat, Oct 10, 2009 at 08:00:30PM +0100, Ben Hutchings wrote:
> On the other hand, the netlink configuration APIs already use ifindex so
> it may be better just to say that the device ioctls are deprecated and
> applications should use netlink.
I thought that is what was already encouraged to happen.
> > > > That is why all distros name network devices based on the only
> > > > deterministic thing they have today, the MAC address. ?I still fail to
> > > > see why you do not like this solution, it is honestly the only way to
> > > > properly name network devices in a sane manner.
> > >
> > > This is feature that needs to be implemented. As per the rules followed.
> >
> > This feature is already implemented today, all distros have it.
>
> No, see below.
Yes, if not, file a bug in your distro, all of the infrastructure is
already in place, and the udev rules and scripts are already written.
> > > > All distros also provide a way to easily rename the network devices, to
> > > > place a specific name on a specific MAC address, so again, this should
> > > > all be solved already.
> > > >
> > > > No matter how badly your BIOS teams mess up the PCI enumeration order :)
> > >
> > > This is an problem, But I think this can be solved by implementing some of the
> > > routines in the network device.
> >
> > I don't, see the rules that your distro ships today for persistant
> > network devices, it's already there, no need to change the kernel at
> > all.
>
> The udev persistent net rules work tolerably well for a single system
> with a stable set of net devices.
>
> They do not solve the problem Matt's talking about, which is lack of
> consistency between multiple systems, because the initial enumeration
> order is not predictable.
Again, you name the device as a MAC address. Or something else that the
BIOS exports in a unique manner (PCI slot name, etc.). That is
consistant. If not, then fix the BIOS.
> They also result in name changes when a NIC (or motherboard) is swapped.
> For some users, that's fine; for others, it's not.
>
> The ability to specify NICs by port name or PCI address should solve
> these problems.
That can be done today quite easily. But note that PCI addresses are
not guaranteed to be stable. As lots of machines are known to have
happen.
Again, none of this requires any kernel changes today at all, let alone
adding dummy char devices for network devices.
thanks,
greg k-h
^ permalink raw reply
* Re: PATCH: Network Device Naming mechanism and policy
From: Greg KH @ 2009-10-10 21:13 UTC (permalink / raw)
To: Bryan Kadzban
Cc: Matt Domsch, Stephen Hemminger, netdev, linux-hotplug, Narendra_K,
jordan_hargrave
In-Reply-To: <4AD0C598.4070403@kadzban.is-a-geek.net>
On Sat, Oct 10, 2009 at 10:34:16AM -0700, Bryan Kadzban wrote:
> Greg KH wrote:
> > On Sat, Oct 10, 2009 at 07:47:32AM -0500, Matt Domsch wrote:
> >> On Fri, Oct 09, 2009 at 10:23:08PM -0700, Greg KH wrote:
> >>> On Fri, Oct 09, 2009 at 11:40:57PM -0500, Matt Domsch wrote:
> >>>> The fundamental roadblock to this is that enumeration !=
> >>>> naming, except that it is for network devices, and we keep
> >>>> changing the enumeration order.
> >>> No, the hardware changes the enumeration order, it places _no_
> >>> guarantees on what order stuff will be found in. So this is not
> >>> the kernel changing, just to be clear.
> >> Over time the kernel has changed its enumeration mechanisms, and
> >> introduced parallelism into the process (which is a good thing),
> >> which, from a user perspective, makes names nondeterministic. Yes,
> >> fixing this up by hard-coding MAC addresses after install has been
> >> the traditional mechanism to address this. I think there's a
> >> better way.
> >
> > Ok, but that way can be done in userspace, without the need for this
> > char device, right?
>
> For the record -- when I tried to send a patch that did exactly this
> (provided an option to use by-path persistence for network drivers), it
> was rejected because "that doesn't work for USB".
>
> True, it doesn't. But by-mac (what we have today) doesn't work for
> replacing motherboards in a random home system (that can't override the
> MAC address in the BIOS), either.
If you replace a motherboard, you honestly expect no configuration to be
needed to be changed? If so, then don't use the MAC naming scheme for
your systems.
> > But this code is not a requirement to "solve" the fact that network
> > devices can show up in different order, that problem can be solved as
> > long as the user picks a single way to name the devices, using tools
> > that are already present today in distros.
>
> This code is not a requirement, no. But -- as you say -- it does
> provide a halfway-decent way to assign multiple names to a NIC. And
> that provides admins the choice to use a couple different persistence
> schemes, depending on how they expect their hardware to work.
But the names need to then be resolved back to a "real" kernel name in
order to do anything with that network connection, as the char devices
are not real ones. So that adds an additional layer of complexity on
all of the system configuration tools.
thanks,
greg k-h
^ permalink raw reply
* [PATCH] ax25: unsigned cannot be less than 0 in ax25_ctl_ioctl()
From: Roel Kluin @ 2009-10-10 21:22 UTC (permalink / raw)
To: Joerg Reuter, linux-hams, Andrew Morton, netdev
struct ax25_ctl_struct member `arg' is unsigned and cannot be less
than 0.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
If the ax25_ctl.arg limit is known to be lower, please suggest
other values.
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index f454607..0d99704 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -398,14 +398,14 @@ static int ax25_ctl_ioctl(const unsigned int cmd, void __user *arg)
break;
case AX25_T1:
- if (ax25_ctl.arg < 1)
+ if (ax25_ctl.arg < 1 || ax25_ctl.arg * HZ > ULONG_MAX)
goto einval_put;
ax25->rtt = (ax25_ctl.arg * HZ) / 2;
ax25->t1 = ax25_ctl.arg * HZ;
break;
case AX25_T2:
- if (ax25_ctl.arg < 1)
+ if (ax25_ctl.arg < 1 || ax25_ctl.arg * HZ > ULONG_MAX)
goto einval_put;
ax25->t2 = ax25_ctl.arg * HZ;
break;
@@ -418,13 +418,13 @@ static int ax25_ctl_ioctl(const unsigned int cmd, void __user *arg)
break;
case AX25_T3:
- if (ax25_ctl.arg < 0)
+ if (ax25_ctl.arg * HZ > ULONG_MAX)
goto einval_put;
ax25->t3 = ax25_ctl.arg * HZ;
break;
case AX25_IDLE:
- if (ax25_ctl.arg < 0)
+ if (ax25_ctl.arg * 60 * HZ > ULONG_MAX)
goto einval_put;
ax25->idle = ax25_ctl.arg * 60 * HZ;
break;
^ permalink raw reply related
* Re: Very strange issues with ethernet wake on lan
From: Maxim Levitsky @ 2009-10-10 21:37 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: netdev@vger.kernel.org, linux-pm@lists.linux-foundation.org
In-Reply-To: <200909292228.18200.rjw@sisk.pl>
On Tue, 2009-09-29 at 22:28 +0200, Rafael J. Wysocki wrote:
> On Sunday 16 August 2009, Maxim Levitsky wrote:
> > Hi,
> >
> > I have recently put back the davicom dm9009 ethernet card into my
> > computer.
> >
> > Some long time ago, I have written its suspend/resume routines.
> > Now I see that few things have changed, like I need to enable wake in
> > sysfs or better patch the code to do so, some nice helpers like
> > pci_prepare_to_sleep have arrived, etc.
> >
> >
> > I narrowed the strange issue down to following situation:
> >
> > I reload dmfe.ko (and networkmanager is disabled)
> > I don't ifup the device, thus pretty much no hardware initialization
> > takes place (but this appears not to matter anyway)
> >
> > I then suspend the system, and WOL doesn't work (I have patched the
> > driver to enable WOL automaticly)
> >
> > I then, suspend again. WOL works, and continues to work as long as I
> > don't reload the driver. If I do, same situation repeats.
> >
> > Also, after a boot, WOL works, so a reload cycle triggers that issue.
> >
> > And most importantly, if I don't do a
> >
> > pci_set_power_state(pci_dev, pci_choose_state (pci_dev, state));
> >
> > in .suspend, then WOL always works.
> >
> > and I have even tried to set state manually to PCI_D3hot or PCI_D3cold,
> >
> > I also tried to use pci_save_state
> >
> >
> > I also have 2 copies of this card, and both have this issue.
> > I also tried 2 pci slots.
> >
> > Kernel is vanilla 2.6.31-rc5
>
> Please check if this still happens with 2.6.32-rc1.
It doesn't! (-git as of today tested)
Thanks,
Best regards,
Maxim Levitsky
>
> Best,
> Rafael
> --
> 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: Real networking namespace
From: Paul Moore @ 2009-10-10 21:40 UTC (permalink / raw)
To: shemminger; +Cc: sds, linux-security-module, viro, netdev, jmorris
In-Reply-To: <20091009190820.0a0f09c2@nehalam>
------- Original message -------
> From: Stephen Hemminger <shemminger@linux-foundation.org>
> Cc: sds@tycho.nsa.gov, linux-security-module@vger.kernel.org,
> viro@zeniv.linux.org.uk, netdev@vger.kernel.org, jmorris@namei.org
> Sent: 10/9, 22:08
>
> On Fri, 9 Oct 2009 18:12:15 -0400
> Paul Moore <paul.moore@hp.com> wrote:
>
>> On Friday 09 October 2009 12:44:52 pm Stephen Smalley wrote:
>> > On Fri, 2009-10-09 at 12:37 -0400, Stephen Smalley wrote:
>> > > On Fri, 2009-10-09 at 08:38 -0700, Stephen Hemminger wrote:
>> > > > The existing networking namespace model is unattractive for what I
>> > > > want, has anyone investigated better alternatives?
>> > > >
>> > > > I would like to be able to allow access to a network interface and
>> > > > associated objects (routing tables etc), to be controlled by
>> Mandatory
>> > > > Access Control API's. I.e grant access to eth0 and to only certain
>> > > > processes. Some the issues with the existing models are:
>> > > > * eth0 and associated objects don't really exist in filesystem
>> so
>> > > > not subject to LSM style control (SeLinux/SMACK/TOMOYO)
>>
>> As Stephen points out, SELinux does have the ability to assign security
>> labels
>> to network interfaces, check out the 'semanage' command. A while back I
>> wrote
>> up something about the SELinux network "ingress/egress" access controls:
>>
>> * http://paulmoore.livejournal.com/2128.html
>
> I was hoping to be able to not have inaccessible interfaces visible,
> is it possible to not have interfaces show up in commands like:
> ip link show
> or sysfs?
I haven't looked at the code for 'ip' but I'm pretty sure it uses netlink
to configure the kernel, yes? If that is the case, no I don't believe any
of the current LSMs provide that level of granularity (netlink, generic
netlink in particular, is a bit of a problem spot at the moment). As for
sysfs, I don't believe we label the interface related files based on their
semanage labels but I could be wrong - we've got plenty of good people
already working on fs labeling so I spend most of my time worrying about
network labeling.
--
paul moore
linux @ hp
^ 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