Send users mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
http://rt2x00.serialmonkey.com/mailman/listinfo/users_rt2x00.serialmonkey.com
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of users digest..."
Today's Topics:
1. Re: [PATCH 2/2] rt2x00: rt2800pci: allow to load EEPROM data
via firmware API (Stanislaw Gruszka)
2. Re: [PATCH 2/2] rt2x00: rt2800pci: allow to load EEPROM data
via firmware API (Gabor Juhos)
3. Re: [PATCH 1/2] rt2x00: rt2800pci: verify ioremap return
value (Jones Desougi)
4. Re: [PATCH 2/2] rt2x00: rt2800pci: allow to load EEPROM data
via firmware API (Kalle Valo)
----------------------------------------------------------------------
Message: 1
Date: Thu, 20 Dec 2012 12:06:02 +0100
From: Stanislaw Gruszka <[email protected]>
To: Kalle Valo <[email protected]>
Cc: Daniel Golle <[email protected]>, [email protected],
[email protected], Gabor Juhos <[email protected]>
Subject: Re: [rt2x00-users] [PATCH 2/2] rt2x00: rt2800pci: allow to
load EEPROM data via firmware API
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
On Thu, Dec 20, 2012 at 10:58:31AM +0200, Kalle Valo wrote:
> Gabor Juhos <[email protected]> writes:
>
> >> Since we use completion here, why we can not just use normal synchronous
> >> version of request_firmware? I heard of request_firmware drawbacks, so
> >> this approach can be correct. Just want to know if we do not complicate
> >> things not necessarily here.
> >
> > If the driver is built into the kernel, then the synchronous version
> > would fail because user-space is not up during probe time.
>
> Didn't udev/systemd guys finally fix their software? At least I recall
> seeing such claims on g+.
Udev broke and later fixed request_firmware when driver is compiled as
module. Here we avoid problem when driver is compiled into kernel
(problem which was there from very beginning) - no userspace available
so request_firmware fail. But since everyone are using modules, nobody
cares.
Openwrt must use modules too, since it use compat-wireless, right? So
I'm not sure if it worth to use request_firmware_nowait. Especially
like that with completion, which block ->probe. As long kernel does
not probes devices in parallel, we block whole boot, and deadlock since
probe wait for userspace.
Gabor, did you test your patches ?
Stanislaw
------------------------------
Message: 2
Date: Thu, 20 Dec 2012 15:34:48 +0100
From: Gabor Juhos <[email protected]>
To: Stanislaw Gruszka <[email protected]>
Cc: Daniel Golle <[email protected]>, [email protected],
[email protected], Kalle Valo <[email protected]>
Subject: Re: [rt2x00-users] [PATCH 2/2] rt2x00: rt2800pci: allow to
load EEPROM data via firmware API
Message-ID: <[email protected]>
Content-Type: text/plain; charset=ISO-8859-1
2012.12.20. 12:06 keltez?ssel, Stanislaw Gruszka ?rta:
> On Thu, Dec 20, 2012 at 10:58:31AM +0200, Kalle Valo wrote:
>> Gabor Juhos <[email protected]> writes:
>>
>>>> Since we use completion here, why we can not just use normal synchronous
>>>> version of request_firmware? I heard of request_firmware drawbacks, so
>>>> this approach can be correct. Just want to know if we do not complicate
>>>> things not necessarily here.
>>>
>>> If the driver is built into the kernel, then the synchronous version
>>> would fail because user-space is not up during probe time.
>>
>> Didn't udev/systemd guys finally fix their software? At least I recall
>> seeing such claims on g+.
>
> Udev broke and later fixed request_firmware when driver is compiled as
> module. Here we avoid problem when driver is compiled into kernel
> (problem which was there from very beginning) - no userspace available
> so request_firmware fail. But since everyone are using modules, nobody
> cares.
>
> Openwrt must use modules too, since it use compat-wireless, right?
Yes, we are using modules.
> So I'm not sure if it worth to use request_firmware_nowait. Especially like
> that with completion, which block ->probe. As long kernel does not probes
> devices in parallel, we block whole boot, and deadlock since probe wait for
> userspace.
>
> Gabor, did you test your patches ?
I have tested the patches, but only agains compat-wireless. I did not try the
rt2x00 driver built into the kernel. Although we are using 3.6.11 currently, but
the deadlock should happen on that as well. I will try it.
-Gabor
------------------------------
Message: 3
Date: Wed, 19 Dec 2012 12:05:59 +0100
From: Jones Desougi <[email protected]>
To: Gabor Juhos <[email protected]>
Cc: [email protected], Daniel Golle <[email protected]>,
[email protected]
Subject: Re: [rt2x00-users] [PATCH 1/2] rt2x00: rt2800pci: verify
ioremap return value
Message-ID: <[email protected]>
Content-Type: text/plain; charset=ISO-8859-1
Hi Gabor,
One thing:
On 12/18/2012 05:22 PM, Gabor Juhos wrote:
> An ioremap call is allowed to fail, however
> the return value of that is not checked in
> the 'rt2800pci_read_eeprom_soc' function.
>
> The patch adds the missing check, and makes
> the function return an int value. The patch
> also converts the 'rt2800_read_eeprom' and
> 'rt2800_ops.read_eeprom' functions to return
> an int value, so the error value can be
> propagated up to the 'rt2800_validate_eeprom'
> function.
>
> Signed-off-by: Gabor Juhos <[email protected]>
> ---
> drivers/net/wireless/rt2x00/rt2800lib.c | 5 ++++-
> drivers/net/wireless/rt2x00/rt2800lib.h | 6 +++---
> drivers/net/wireless/rt2x00/rt2800pci.c | 17 ++++++++++++-----
> drivers/net/wireless/rt2x00/rt2800usb.c | 4 +++-
> 4 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c
> b/drivers/net/wireless/rt2x00/rt2800lib.c
> index 197b446..52b2978 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -4635,11 +4635,14 @@ static int rt2800_validate_eeprom(struct rt2x00_dev
> *rt2x00dev)
> u16 word;
> u8 *mac;
> u8 default_lna_gain;
> + int retval;
>
> /*
> * Read the EEPROM.
> */
> - rt2800_read_eeprom(rt2x00dev);
> + retval = rt2800_read_eeprom(rt2x00dev);
> + if (retval)
> + return retval;
>
> /*
> * Start validation of the data that has been read.
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.h
> b/drivers/net/wireless/rt2x00/rt2800lib.h
> index a128cea..8252c67 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.h
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.h
> @@ -43,7 +43,7 @@ struct rt2800_ops {
> const unsigned int offset,
> const struct rt2x00_field32 field, u32 *reg);
>
> - void (*read_eeprom)(struct rt2x00_dev *rt2x00dev);
> + int (*read_eeprom)(struct rt2x00_dev *rt2x00dev);
> bool (*hwcrypt_disabled)(struct rt2x00_dev *rt2x00dev);
>
> int (*drv_write_firmware)(struct rt2x00_dev *rt2x00dev,
> @@ -117,11 +117,11 @@ static inline int rt2800_regbusy_read(struct rt2x00_dev
> *rt2x00dev,
> return rt2800ops->regbusy_read(rt2x00dev, offset, field, reg);
> }
>
> -static inline void rt2800_read_eeprom(struct rt2x00_dev *rt2x00dev)
> +static inline int rt2800_read_eeprom(struct rt2x00_dev *rt2x00dev)
> {
> const struct rt2800_ops *rt2800ops = rt2x00dev->ops->drv;
>
> - rt2800ops->read_eeprom(rt2x00dev);
> + return rt2800ops->read_eeprom(rt2x00dev);
> }
>
> static inline bool rt2800_hwcrypt_disabled(struct rt2x00_dev *rt2x00dev)
> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c
> b/drivers/net/wireless/rt2x00/rt2800pci.c
> index 9224d87..5fc16dd 100644
> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> @@ -90,17 +90,21 @@ static void rt2800pci_mcu_status(struct rt2x00_dev
> *rt2x00dev, const u8 token)
> }
>
> #if defined(CONFIG_RALINK_RT288X) || defined(CONFIG_RALINK_RT305X)
> -static void rt2800pci_read_eeprom_soc(struct rt2x00_dev *rt2x00dev)
> +static int rt2800pci_read_eeprom_soc(struct rt2x00_dev *rt2x00dev)
> {
> void __iomem *base_addr = ioremap(0x1F040000, EEPROM_SIZE);
>
> + if (!base_addr)
> + return -ENOMEM;
> +
> memcpy_fromio(rt2x00dev->eeprom, base_addr, EEPROM_SIZE);
>
> iounmap(base_addr);
> }
Missing return at the end, since it should now return an int.
Even if this is gone with the second patch.
> #else
> -static inline void rt2800pci_read_eeprom_soc(struct rt2x00_dev *rt2x00dev)
> +static inline int rt2800pci_read_eeprom_soc(struct rt2x00_dev *rt2x00dev)
> {
> + return -ENOMEM;
> }
> #endif /* CONFIG_RALINK_RT288X || CONFIG_RALINK_RT305X */
>
> @@ -970,14 +974,17 @@ static irqreturn_t rt2800pci_interrupt(int irq, void
> *dev_instance)
> /*
> * Device probe functions.
> */
> -static void rt2800pci_read_eeprom(struct rt2x00_dev *rt2x00dev)
> +static int rt2800pci_read_eeprom(struct rt2x00_dev *rt2x00dev)
> {
> if (rt2x00_is_soc(rt2x00dev))
> - rt2800pci_read_eeprom_soc(rt2x00dev);
> - else if (rt2800pci_efuse_detect(rt2x00dev))
> + return rt2800pci_read_eeprom_soc(rt2x00dev);
> +
> + if (rt2800pci_efuse_detect(rt2x00dev))
> rt2800pci_read_eeprom_efuse(rt2x00dev);
> else
> rt2800pci_read_eeprom_pci(rt2x00dev);
> +
> + return 0;
> }
>
> static const struct ieee80211_ops rt2800pci_mac80211_ops = {
> diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c
> b/drivers/net/wireless/rt2x00/rt2800usb.c
> index 5c149b5..48de5c9 100644
> --- a/drivers/net/wireless/rt2x00/rt2800usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2800usb.c
> @@ -735,13 +735,15 @@ static void rt2800usb_fill_rxdone(struct queue_entry
> *entry,
> /*
> * Device probe functions.
> */
> -static void rt2800usb_read_eeprom(struct rt2x00_dev *rt2x00dev)
> +static int rt2800usb_read_eeprom(struct rt2x00_dev *rt2x00dev)
> {
> if (rt2800_efuse_detect(rt2x00dev))
> rt2800_read_eeprom_efuse(rt2x00dev);
> else
> rt2x00usb_eeprom_read(rt2x00dev, rt2x00dev->eeprom,
> EEPROM_SIZE);
> +
> + return 0;
> }
>
> static int rt2800usb_probe_hw(struct rt2x00_dev *rt2x00dev)
>
Regards,
/Jones
------------------------------
Message: 4
Date: Thu, 20 Dec 2012 10:58:31 +0200
From: Kalle Valo <[email protected]>
To: Gabor Juhos <[email protected]>
Cc: Daniel Golle <[email protected]>, [email protected],
[email protected]
Subject: Re: [rt2x00-users] [PATCH 2/2] rt2x00: rt2800pci: allow to
load EEPROM data via firmware API
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
Gabor Juhos <[email protected]> writes:
>> Since we use completion here, why we can not just use normal synchronous
>> version of request_firmware? I heard of request_firmware drawbacks, so
>> this approach can be correct. Just want to know if we do not complicate
>> things not necessarily here.
>
> If the driver is built into the kernel, then the synchronous version
> would fail because user-space is not up during probe time.
Didn't udev/systemd guys finally fix their software? At least I recall
seeing such claims on g+.
--
Kalle Valo
------------------------------
Subject: Digest Footer
_______________________________________________
users mailing list
[email protected]
http://rt2x00.serialmonkey.com/mailman/listinfo/users_rt2x00.serialmonkey.com
------------------------------
End of users Digest, Vol 46, Issue 23
*************************************
