2011-07-14 12:48:03

by Kahn, Gery

[permalink] [raw]
Subject: [PATCH 1/2] wl12xx: Fix for PG version and sysfs files

Fix the value of PG version for 128x at sysfs, remove write permissions
from PG version (hw_pg_ver) in sysfs and add remove files
(hw_pg_ver,bt_coex_state) from sysfs while freeing hardware.
New macro names for register Fuse_data_2_1 depend on architecture.

Signed-off-by: Gery Kahn <[email protected]>
---
drivers/net/wireless/wl12xx/boot.c | 13 ++++++++-----
drivers/net/wireless/wl12xx/boot.h | 3 ++-
drivers/net/wireless/wl12xx/main.c | 6 +++++-
3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/boot.c b/drivers/net/wireless/wl12xx/boot.c
index 4bcee1c..74ed1b3 100644
--- a/drivers/net/wireless/wl12xx/boot.c
+++ b/drivers/net/wireless/wl12xx/boot.c
@@ -524,13 +524,13 @@ static void wl1271_boot_hw_version(struct wl1271 *wl)
{
u32 fuse;

- fuse = wl1271_top_reg_read(wl, REG_FUSE_DATA_2_1);
+ if (wl->chip.id == CHIP_ID_1283_PG20)
+ fuse = wl1271_top_reg_read(wl, WL128X_REG_FUSE_DATA_2_1);
+ else
+ fuse = wl1271_top_reg_read(wl, WL127X_REG_FUSE_DATA_2_1);
fuse = (fuse & PG_VER_MASK) >> PG_VER_OFFSET;

wl->hw_pg_ver = (s8)fuse;
-
- if (((wl->hw_pg_ver & PG_MAJOR_VER_MASK) >> PG_MAJOR_VER_OFFSET) < 3)
- wl->quirks |= WL12XX_QUIRK_END_OF_TRANSACTION;
}

static int wl128x_switch_tcxo_to_fref(struct wl1271 *wl)
@@ -671,7 +671,8 @@ static int wl127x_boot_clk(struct wl1271 *wl)
u32 pause;
u32 clk;

- wl1271_boot_hw_version(wl);
+ if (((wl->hw_pg_ver & PG_MAJOR_VER_MASK) >> PG_MAJOR_VER_OFFSET) < 3)
+ wl->quirks |= WL12XX_QUIRK_END_OF_TRANSACTION;

if (wl->ref_clock == CONF_REF_CLK_19_2_E ||
wl->ref_clock == CONF_REF_CLK_38_4_E ||
@@ -725,6 +726,8 @@ int wl1271_load_firmware(struct wl1271 *wl)
u32 tmp, clk;
int selected_clock = -1;

+ wl1271_boot_hw_version(wl);
+
if (wl->chip.id == CHIP_ID_1283_PG20) {
ret = wl128x_boot_clk(wl, &selected_clock);
if (ret < 0)
diff --git a/drivers/net/wireless/wl12xx/boot.h b/drivers/net/wireless/wl12xx/boot.h
index e8f8255..06dad93 100644
--- a/drivers/net/wireless/wl12xx/boot.h
+++ b/drivers/net/wireless/wl12xx/boot.h
@@ -55,7 +55,8 @@ struct wl1271_static_data {
#define OCP_REG_CLK_POLARITY 0x0cb2
#define OCP_REG_CLK_PULL 0x0cb4

-#define REG_FUSE_DATA_2_1 0x050a
+#define WL127X_REG_FUSE_DATA_2_1 0x050a
+#define WL128X_REG_FUSE_DATA_2_1 0x2152
#define PG_VER_MASK 0x3c
#define PG_VER_OFFSET 2

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 85b3385..d65fef6 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -4434,7 +4434,7 @@ static ssize_t wl1271_sysfs_show_hw_pg_ver(struct device *dev,
return len;
}

-static DEVICE_ATTR(hw_pg_ver, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(hw_pg_ver, S_IRUGO,
wl1271_sysfs_show_hw_pg_ver, NULL);

static ssize_t wl1271_sysfs_read_fwlog(struct file *filp, struct kobject *kobj,
@@ -4848,6 +4848,10 @@ int wl1271_free_hw(struct wl1271 *wl)
mutex_unlock(&wl->mutex);

device_remove_bin_file(&wl->plat_dev->dev, &fwlog_attr);
+
+ device_remove_file(&wl->plat_dev->dev, &dev_attr_hw_pg_ver);
+
+ device_remove_file(&wl->plat_dev->dev, &dev_attr_bt_coex_state);
platform_device_unregister(wl->plat_dev);
free_page((unsigned long)wl->fwlog);
dev_kfree_skb(wl->dummy_packet);
--
1.7.0.4



2011-07-14 12:54:51

by Kahn, Gery

[permalink] [raw]
Subject: [PATCH 2/2] wl12xx Export chip id to sysfs

Export the chip id to userspace. This helps to change application behavior
according to architecture of the wl12xx chip.

Signed-off-by: Gery Kahn <[email protected]>
---
drivers/net/wireless/wl12xx/main.c | 30 ++++++++++++++++++++++++++++++
1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index d65fef6..13a6cfa 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -4437,6 +4437,25 @@ static ssize_t wl1271_sysfs_show_hw_pg_ver(struct device *dev,
static DEVICE_ATTR(hw_pg_ver, S_IRUGO,
wl1271_sysfs_show_hw_pg_ver, NULL);

+static ssize_t wl1271_sysfs_show_chip_id(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct wl1271 *wl = dev_get_drvdata(dev);
+ ssize_t len;
+
+ len = PAGE_SIZE;
+
+ mutex_lock(&wl->mutex);
+ len = snprintf(buf, len, "0x%x\n", wl->chip.id);
+ mutex_unlock(&wl->mutex);
+
+ return len;
+}
+
+static DEVICE_ATTR(chip_id, S_IRUGO,
+ wl1271_sysfs_show_chip_id, NULL);
+
static ssize_t wl1271_sysfs_read_fwlog(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr,
char *buffer, loff_t pos, size_t count)
@@ -4803,8 +4822,17 @@ struct ieee80211_hw *wl1271_alloc_hw(void)
goto err_hw_pg_ver;
}

+ ret = device_create_file(&wl->plat_dev->dev, &dev_attr_chip_id);
+ if (ret < 0) {
+ wl1271_error("failed to create sysfs file chip_id");
+ goto err_fwlog_attr;
+ }
+
return hw;

+err_fwlog_attr:
+ device_remove_bin_file(&wl->plat_dev->dev, &fwlog_attr);
+
err_hw_pg_ver:
device_remove_file(&wl->plat_dev->dev, &dev_attr_hw_pg_ver);

@@ -4849,6 +4877,8 @@ int wl1271_free_hw(struct wl1271 *wl)

device_remove_bin_file(&wl->plat_dev->dev, &fwlog_attr);

+ device_remove_file(&wl->plat_dev->dev, &dev_attr_chip_id);
+
device_remove_file(&wl->plat_dev->dev, &dev_attr_hw_pg_ver);

device_remove_file(&wl->plat_dev->dev, &dev_attr_bt_coex_state);
--
1.7.0.4


2011-07-18 09:37:00

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] wl12xx Export chip id to sysfs

"Kahn, Gery" <[email protected]> writes:

> On Mon, Jul 18, 2011 at 10:18, Kalle Valo <[email protected]> wrote:
>> Kalle Valo <[email protected]> writes:
>>
>>>> After some investigation, found that ethtool's ioctl interface
>>>> doesn't work at this case.
>>>
>>> This reminds me: I have patches for these issues but I never sent them
>>> upstream. It's too late for me now, but I'll search them tomorrow (or
>>> today to be exact..) and send them for testing purposes.
>>
>> Here's the patch I used:

[...]

> Tested-by: Gery Kahn <[email protected]>

Thanks for testing. I'm busy with somethign else today, but I'll send
a proper patch tomorrow with your tested-by.

--
Kalle Valo

2011-07-17 08:16:38

by Kahn, Gery

[permalink] [raw]
Subject: Re: [PATCH 2/2] wl12xx Export chip id to sysfs

On Fri, Jul 15, 2011 at 13:56, Kalle Valo <[email protected]> wrote:
> Gery Kahn <[email protected]> writes:
>
>> Export the chip id to userspace. This helps to change application behavior
>> according to architecture of the wl12xx chip.
>
> We already support exporting this type of information through
> ethtool's ioctl interface using the wiphy.hw_version, which was
> specifially added for this purpose. That's much better than yet
> another sysfs file.
>
Right.
In PLT mode it didn't show any information, but
that might have been a mistake on our part.

I'll send another patch.

Gery

2011-07-18 07:21:56

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] wl12xx Export chip id to sysfs

Kalle Valo <[email protected]> writes:

> Kalle Valo <[email protected]> writes:
>
>>> After some investigation, found that ethtool's ioctl interface doesn't work
>>> at this case.
>>
>> This reminds me: I have patches for these issues but I never sent them
>> upstream. It's too late for me now, but I'll search them tomorrow (or
>> today to be exact..) and send them for testing purposes.
>
> Here's the patch I used:

And patch to ethtool:

diff --git a/ethtool.c b/ethtool.c
index 4226a67..6e80926 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1363,12 +1363,15 @@ static int dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
fclose(f);
}

- if (!gregs_dump_hex)
+ if (!gregs_dump_hex) {
for (i = 0; i < ARRAY_SIZE(driver_list); i++)
if (!strncmp(driver_list[i].name, info->driver,
ETHTOOL_BUSINFO_LEN))
return driver_list[i].func(info, regs);

+ fprintf(stdout, "version: 0x%x\n", regs->version);
+ }
+
fprintf(stdout, "Offset\tValues\n");
fprintf(stdout, "--------\t-----");
for (i = 0; i < regs->len; i++) {

2011-07-18 08:39:20

by Kahn, Gery

[permalink] [raw]
Subject: Re: [PATCH 2/2] wl12xx Export chip id to sysfs

On Mon, Jul 18, 2011 at 10:18, Kalle Valo <[email protected]> wrote:
> Kalle Valo <[email protected]> writes:
>
>>> After some investigation, found that ethtool's ioctl interface doesn't work
>>> at this case.
>>
>> This reminds me: I have patches for these issues but I never sent them
>> upstream. It's too late for me now, but I'll search them tomorrow (or
>> today to be exact..) and send them for testing purposes.
>
> Here's the patch I used:
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 74ead9e..0836aa3 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -1185,7 +1185,7 @@ static int ethtool_get_regs(struct net_device *dev, char __user *useraddr)
> ?{
> ? ? ? ?struct ethtool_regs regs;
> ? ? ? ?const struct ethtool_ops *ops = dev->ethtool_ops;
> - ? ? ? void *regbuf;
> + ? ? ? void *regbuf = NULL;
> ? ? ? ?int reglen, ret;
>
> ? ? ? ?if (!ops->get_regs || !ops->get_regs_len)
> @@ -1198,18 +1198,24 @@ static int ethtool_get_regs(struct net_device *dev, char __user *useraddr)
> ? ? ? ?if (regs.len > reglen)
> ? ? ? ? ? ? ? ?regs.len = reglen;
>
> - ? ? ? regbuf = vzalloc(reglen);
> - ? ? ? if (!regbuf)
> - ? ? ? ? ? ? ? return -ENOMEM;
> + ? ? ? if (reglen > 0) {
> + ? ? ? ? ? ? ? regbuf = vzalloc(reglen);
> + ? ? ? ? ? ? ? if (!regbuf)
> + ? ? ? ? ? ? ? ? ? ? ? return -ENOMEM;
> + ? ? ? }
>
> ? ? ? ?ops->get_regs(dev, &regs, regbuf);
>
> ? ? ? ?ret = -EFAULT;
> ? ? ? ?if (copy_to_user(useraddr, &regs, sizeof(regs)))
> ? ? ? ? ? ? ? ?goto out;
> - ? ? ? useraddr += offsetof(struct ethtool_regs, data);
> - ? ? ? if (copy_to_user(useraddr, regbuf, regs.len))
> - ? ? ? ? ? ? ? goto out;
> +
> + ? ? ? if (regs.len > 0) {
> + ? ? ? ? ? ? ? useraddr += offsetof(struct ethtool_regs, data);
> + ? ? ? ? ? ? ? if (copy_to_user(useraddr, regbuf, regs.len))
> + ? ? ? ? ? ? ? ? ? ? ? goto out;
> + ? ? ? }
> +
> ? ? ? ?ret = 0;
>
> ?out:
>

Tested-by: Gery Kahn <[email protected]>

2011-07-15 10:56:14

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] wl12xx Export chip id to sysfs

Gery Kahn <[email protected]> writes:

> Export the chip id to userspace. This helps to change application behavior
> according to architecture of the wl12xx chip.

We already support exporting this type of information through
ethtool's ioctl interface using the wiphy.hw_version, which was
specifially added for this purpose. That's much better than yet
another sysfs file.

--
Kalle Valo

2011-07-18 07:18:39

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] wl12xx Export chip id to sysfs

Kalle Valo <[email protected]> writes:

>> After some investigation, found that ethtool's ioctl interface doesn't work
>> at this case.
>
> This reminds me: I have patches for these issues but I never sent them
> upstream. It's too late for me now, but I'll search them tomorrow (or
> today to be exact..) and send them for testing purposes.

Here's the patch I used:

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 74ead9e..0836aa3 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1185,7 +1185,7 @@ static int ethtool_get_regs(struct net_device *dev, char __user *useraddr)
{
struct ethtool_regs regs;
const struct ethtool_ops *ops = dev->ethtool_ops;
- void *regbuf;
+ void *regbuf = NULL;
int reglen, ret;

if (!ops->get_regs || !ops->get_regs_len)
@@ -1198,18 +1198,24 @@ static int ethtool_get_regs(struct net_device *dev, char __user *useraddr)
if (regs.len > reglen)
regs.len = reglen;

- regbuf = vzalloc(reglen);
- if (!regbuf)
- return -ENOMEM;
+ if (reglen > 0) {
+ regbuf = vzalloc(reglen);
+ if (!regbuf)
+ return -ENOMEM;
+ }

ops->get_regs(dev, &regs, regbuf);

ret = -EFAULT;
if (copy_to_user(useraddr, &regs, sizeof(regs)))
goto out;
- useraddr += offsetof(struct ethtool_regs, data);
- if (copy_to_user(useraddr, regbuf, regs.len))
- goto out;
+
+ if (regs.len > 0) {
+ useraddr += offsetof(struct ethtool_regs, data);
+ if (copy_to_user(useraddr, regbuf, regs.len))
+ goto out;
+ }
+
ret = 0;

out:

2011-07-17 21:45:07

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] wl12xx Export chip id to sysfs

"Kahn, Gery" <[email protected]> writes:

Hi Gery,

> On Sun, Jul 17, 2011 at 11:16, Kahn, Gery <[email protected]> wrote:
>> On Fri, Jul 15, 2011 at 13:56, Kalle Valo <[email protected]> wrote:
>>> Gery Kahn <[email protected]> writes:
>>>
>>>> Export the chip id to userspace. This helps to change application behavior
>>>> according to architecture of the wl12xx chip.
>>>
>>> We already support exporting this type of information through
>>> ethtool's ioctl interface using the wiphy.hw_version, which was
>>> specifially added for this purpose. That's much better than yet
>>> another sysfs file.
>>>
>> Right.
>> In PLT mode it didn't show any information, but
>> that might have been a mistake on our part.
>>
>> I'll send another patch.
>>
> After some investigation, found that ethtool's ioctl interface doesn't work
> at this case.

This reminds me: I have patches for these issues but I never sent them
upstream. It's too late for me now, but I'll search them tomorrow (or
today to be exact..) and send them for testing purposes.

If I forget, please ping me :)

--
Kalle Valo

2011-07-18 08:47:07

by Kahn, Gery

[permalink] [raw]
Subject: Re: [PATCH 2/2] wl12xx Export chip id to sysfs

On Mon, Jul 18, 2011 at 11:39, Kahn, Gery <[email protected]> wrote:
> On Mon, Jul 18, 2011 at 10:18, Kalle Valo <[email protected]> wrote:
>> Kalle Valo <[email protected]> writes:
>>
>>>> After some investigation, found that ethtool's ioctl interface doesn't work
>>>> at this case.
>>>
>>> This reminds me: I have patches for these issues but I never sent them
>>> upstream. It's too late for me now, but I'll search them tomorrow (or
>>> today to be exact..) and send them for testing purposes.
>>
>> Here's the patch I used:
>>
>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index 74ead9e..0836aa3 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -1185,7 +1185,7 @@ static int ethtool_get_regs(struct net_device *dev, char __user *useraddr)
>> ?{
>> ? ? ? ?struct ethtool_regs regs;
>> ? ? ? ?const struct ethtool_ops *ops = dev->ethtool_ops;
>> - ? ? ? void *regbuf;
>> + ? ? ? void *regbuf = NULL;
>> ? ? ? ?int reglen, ret;
>>
>> ? ? ? ?if (!ops->get_regs || !ops->get_regs_len)
>> @@ -1198,18 +1198,24 @@ static int ethtool_get_regs(struct net_device *dev, char __user *useraddr)
>> ? ? ? ?if (regs.len > reglen)
>> ? ? ? ? ? ? ? ?regs.len = reglen;
>>
>> - ? ? ? regbuf = vzalloc(reglen);
>> - ? ? ? if (!regbuf)
>> - ? ? ? ? ? ? ? return -ENOMEM;
>> + ? ? ? if (reglen > 0) {
>> + ? ? ? ? ? ? ? regbuf = vzalloc(reglen);
>> + ? ? ? ? ? ? ? if (!regbuf)
>> + ? ? ? ? ? ? ? ? ? ? ? return -ENOMEM;
>> + ? ? ? }
>>
>> ? ? ? ?ops->get_regs(dev, &regs, regbuf);
>>
>> ? ? ? ?ret = -EFAULT;
>> ? ? ? ?if (copy_to_user(useraddr, &regs, sizeof(regs)))
>> ? ? ? ? ? ? ? ?goto out;
>> - ? ? ? useraddr += offsetof(struct ethtool_regs, data);
>> - ? ? ? if (copy_to_user(useraddr, regbuf, regs.len))
>> - ? ? ? ? ? ? ? goto out;
>> +
>> + ? ? ? if (regs.len > 0) {
>> + ? ? ? ? ? ? ? useraddr += offsetof(struct ethtool_regs, data);
>> + ? ? ? ? ? ? ? if (copy_to_user(useraddr, regbuf, regs.len))
>> + ? ? ? ? ? ? ? ? ? ? ? goto out;
>> + ? ? ? }
>> +
>> ? ? ? ?ret = 0;
>>
>> ?out:
>>
>
> Tested-by: Gery Kahn <[email protected]>
>
I wanted to add thank you for sending your and reviewing my patch.

It is good to have you around.

Gery

2011-07-17 12:51:51

by Kahn, Gery

[permalink] [raw]
Subject: Re: [PATCH 2/2] wl12xx Export chip id to sysfs

Hi Kalle,

On Sun, Jul 17, 2011 at 11:16, Kahn, Gery <[email protected]> wrote:
> On Fri, Jul 15, 2011 at 13:56, Kalle Valo <[email protected]> wrote:
>> Gery Kahn <[email protected]> writes:
>>
>>> Export the chip id to userspace. This helps to change application behavior
>>> according to architecture of the wl12xx chip.
>>
>> We already support exporting this type of information through
>> ethtool's ioctl interface using the wiphy.hw_version, which was
>> specifially added for this purpose. That's much better than yet
>> another sysfs file.
>>
> Right.
> In PLT mode it didn't show any information, but
> that might have been a mistake on our part.
>
> I'll send another patch.
>
After some investigation, found that ethtool's ioctl interface doesn't work
at this case.

1.
It demands data for registers while the purpose of the change was to have
only HW version at sysfs.
BTW, if the registers data exists, the ethtool doesn't display
HW version anyway.

2.
The datapath of ioctl SIOCETHTOOL also has problem that can be solved
by changing the cfg80211_get_regs_len() from net/wireless/ethtool.c

static int cfg80211_get_regs_len(struct net_device *dev)
struct wireless_dev *wdev = dev->ieee80211_ptr;
return sizeof(wdev->wiphy->hw_version);
}
But i need to check more with other drivers may be.

I am back to suggest my patch for the case.

Gery