Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets

[Felix Fietkau]

On 2018-03-08 15:05, Pavel Machek wrote:
> On Thu 2018-03-08 13:33:29, Sebastian Gottschall wrote:
>> Am 08.03.2018 um 10:02 schrieb Pavel Machek:
>> >On Wed 2018-03-07 18:54:41, Sebastian Gottschall wrote:
>> >>Am 07.03.2018 um 17:22 schrieb Rafał Miłecki:
>> >>>On 2 March 2018 at 10:22, Sebastian Gottschall  
>> >>>wrote:
>> >>leds-gpio is crap and limited. you can just register one platform data 
>> >>at
>> >>kernel runtime since its identified by its object name "led-gpio" but 
>> >>the
>> >>kernel forbidds to register 2 platform datas with the same name
>> >>consider the ar71xx devices with qca988x wifi chipsets. they all have
>> >>already a led platform data registered
>> >>at boottime. a second can't be registered anymore so gpio_led is 
>> >>useless
>> >>at
>> >>all for most developers on such platforms. its mainly used for early
>> >>kernel
>> >>platform data initialisation for system leds.
>> >If leds-gpio has limitations, please fix those, rather then
>> >introducing duplicated code.
>> there is no duplicated code introduced and there is no solution for it.
>> consider that all wifi drivers with softled support
>> are going that way with registering a own led driver. see ath9k for
>> instance. gpio-led cannot be used for it and there is no way to
>> support multiple platform datas with the same name. its a kernel 
>> limitation
>> >>>I just reviewed some mips arch patch adding support for more LEDs for
>> >>>selected devices:
>> >>>[PATCH] MIPS: BCM47XX: Add Luxul XAP1500/XWR1750 WiFi LEDs
>> >>>https://patchwork.linux-mips.org/patch/18681/
>> >>>
>> >>>It seems to be simply adding another call to the
>> >>>gpio_led_register_device(). It seems to me you can call that function
>> >>>multiple times and register multiple structs with LEDs.
>> >>>
>> >>>Isn't all you need something like this?
>> >>>
>> >>>static const struct gpio_led ath10k_leds[] = {
>> >>> {
>> >>> .name = "ath10k:color:function",
>> >>> .active_low = 1,
>> >>> .default_state = LEDS_GPIO_DEFSTATE_KEEP,
>> >>> }
>> >>>};
>> >>>
>> >>>static struct gpio_led_platform_data bcm47xx_leds_pdata_extra = {
>> >>> leds = ath10k_leds;
>> >>> num_leds = ARRAY_SIZE(ath10k_leds);
>> >>>};
>> >>>
>> >>>ath10k_leds.gpio = ar->hw_params.led_pin;
>> >>>gpio_led_register_device(0, _leds);
>> >>the problem are other architectures which have already registered gpio_led
>> >>at system start like ar71xx
>> >>you cannot register a second one. so a independend led driver is a
>> >>requirement for direct control
>> >If the limitation indeed exists, please fix the limitation rather than
>> >working around it in each and every driver.
>> see ath9k. its exact the same implementation.
> 
> Ok, so one more driver to fix.
> 
>> in addition my variant does also work without gpiolib support. so it can be
>> used even if the kernel is configured
>> without gpio support.
>> and not to forget, using a own led driver is more ligthweight from the call
>> path for each led on / off event which is important for
>> low performance embedded devices
> 
> We are not going to copy code because such code works without
> libraries, and we are not going to copy code because that uses
> less cache during calls. Sorry.
> 
> NAK. Please fix your patch. Since this discussion seems to have taken a 
> rather weird turn, I've
taken a look at the specific code, and from my point of view the code
that sets up the LED (including callback) is so trivial that it's simply
not worth dealing with adding the leds-gpio driver to the mix.
It adds extra complexity and an extra dependency for no reason at all.
There's no extra functionality to be gained by using it.

- Felix

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets

[Sebastian Gottschall]

Am 08.03.2018 um 16:04 schrieb Pavel Machek:

Hi!


show me a proof that its copy & paste. because its not

I don't have to prove you anything. Sorry.

then i will deny your argument because its false.


But you said:


see ath9k. its exact the same implementation.

We don't want to have exact same code multiple times in the tree.
it isnt the exact same code, its just the way its done. i mean 
registering a led driver function happens multiple times in the kernel
you cannot say that calling led_classdev_register with the required 
parameters and function implementation is a case of code duplication.

then i would just say using of "printk" is a case of code duplication.
registering a led driver is nothing unusual, the implementation of the 
led driver is different each time. the implementation for led_brightness 
is very different

there are many led drivers in the kernel. all are going the same way.

i checked the kernel drivers just right now. almost all major wireless 
drivers are comming with a led driver without using gpiolib
your way is a case of codebloating since a registering gpio-leds 
requires a gpio driver for each wireless driver, even if its sometimes 
just a single register write for a led  and no real gpio.
a gpio driver is more complex and bigger than just a led driver. i just 
wrote a optional gpio driver to get access to all gpios available on the 
card. some vendors are using these in a unusual way
i have seen that vendors used them for reset button input etc. this is 
why i made it. so dont take this as a argument for going a impossible 
way (again. the kernel does not support multiple platform datas with the 
same name
THE KERNEL not leds-gpio. so once a leds-gpio platform data is 
registered, no other driver can register a new one.
in addition the kernel must have gpiolib support which increases the 
kernel size. the best way is always the most simple way and the smallest 
performant way.


and again. i did not duplicate the code of ath9k, i just used it as 
documentation to write a own led driver in a simple way


now a list of wireless drivers with a led driver
intersil
carl9170
ath5k
rt2x00
b43legacy
b43
iwlegacy
rtl8187
brcmsmac
iwlwifi

Sebastian

  
	Pavel



--
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottsch...@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets

[Pavel Machek]

Hi!

> show me a proof that its copy & paste. because its not

I don't have to prove you anything. Sorry.

But you said:

> >>see ath9k. its exact the same implementation.

We don't want to have exact same code multiple times in the tree.
 
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets

[Sebastian Gottschall]

Am 08.03.2018 um 15:29 schrieb Kalle Valo:

Pavel Machek  writes:


ath10k_leds.gpio = ar->hw_params.led_pin;
gpio_led_register_device(0, _leds);

the problem are other architectures which have already registered gpio_led
at system start like ar71xx
you cannot register a second one. so a independend led driver is a
requirement for direct control

If the limitation indeed exists, please fix the limitation rather than
working around it in each and every driver.

see ath9k. its exact the same implementation.

Ok, so one more driver to fix.


in addition my variant does also work without gpiolib support. so it
can be used even if the kernel is configured without gpio support.
and not to forget, using a own led driver is more ligthweight from
the call path for each led on / off event which is important for low
performance embedded devices

We are not going to copy code because such code works without
libraries, and we are not going to copy code because that uses
less cache during calls. Sorry.

NAK. Please fix your patch.

To me it's not that black and white, sometimes copying code is simpler
than trying to bring up complicated or restricting frameworks (talking
in general here).

I haven't been able to review this patch in detail yet but from a quick
look most of the code is about standard ath10k infrastructure code. How
many lines of code would using leds-gpio save?
nothing. the led driver code is already very small. (see gpio.c in the 
patch) i had a leds-gpio implementation but it will not work since a 
platform data with the name "leds-gpio" does already exist on various 
platforms (ath79 for instance)
so a second leds-gpio can't be registered. so this solution does simply 
not work and asking me for fixing the kernel generic platform data 
handling like pavel did, is really out of mind
and a own led driver has also better performance than using the 
leds-gpio->gpiolib->ath10k gpio driver callpath.
if someone is still complaining that i added a gpio feature driver as 
well to my patch, i can simply remove that usefull feature and all would 
be happy. but i think having access to all gpios of the card
instead of just the led gpio makes sense to. thats why i implemented a 
gpiolib driver as well


Sebastian


--
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottsch...@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets

[Sebastian Gottschall]

Am 08.03.2018 um 15:05 schrieb Pavel Machek:

On Thu 2018-03-08 13:33:29, Sebastian Gottschall wrote:

Am 08.03.2018 um 10:02 schrieb Pavel Machek:

On Wed 2018-03-07 18:54:41, Sebastian Gottschall wrote:

Am 07.03.2018 um 17:22 schrieb Rafał Miłecki:

On 2 March 2018 at 10:22, Sebastian Gottschall  wrote:

leds-gpio is crap and limited. you can just register one platform data at
kernel runtime since its identified by its object name "led-gpio" but the
kernel forbidds to register 2 platform datas with the same name
consider the ar71xx devices with qca988x wifi chipsets. they all have
already a led platform data registered
at boottime. a second can't be registered anymore so gpio_led is useless
at
all for most developers on such platforms. its mainly used for early
kernel
platform data initialisation for system leds.

If leds-gpio has limitations, please fix those, rather then
introducing duplicated code.

there is no duplicated code introduced and there is no solution for it.
consider that all wifi drivers with softled support
are going that way with registering a own led driver. see ath9k for
instance. gpio-led cannot be used for it and there is no way to
support multiple platform datas with the same name. its a kernel limitation

I just reviewed some mips arch patch adding support for more LEDs for
selected devices:
[PATCH] MIPS: BCM47XX: Add Luxul XAP1500/XWR1750 WiFi LEDs
https://patchwork.linux-mips.org/patch/18681/

It seems to be simply adding another call to the
gpio_led_register_device(). It seems to me you can call that function
multiple times and register multiple structs with LEDs.

Isn't all you need something like this?

static const struct gpio_led ath10k_leds[] = {
 {
 .name = "ath10k:color:function",
 .active_low = 1,
 .default_state = LEDS_GPIO_DEFSTATE_KEEP,
 }
};

static struct gpio_led_platform_data bcm47xx_leds_pdata_extra = {
 leds = ath10k_leds;
 num_leds = ARRAY_SIZE(ath10k_leds);
};

ath10k_leds.gpio = ar->hw_params.led_pin;
gpio_led_register_device(0, _leds);

the problem are other architectures which have already registered gpio_led
at system start like ar71xx
you cannot register a second one. so a independend led driver is a
requirement for direct control

If the limitation indeed exists, please fix the limitation rather than
working around it in each and every driver.

see ath9k. its exact the same implementation.

Ok, so one more driver to fix.


in addition my variant does also work without gpiolib support. so it can be
used even if the kernel is configured
without gpio support.
and not to forget, using a own led driver is more ligthweight from the call
path for each led on / off event which is important for
low performance embedded devices

We are not going to copy code because such code works without
libraries, and we are not going to copy code because that uses
less cache during calls. Sorry.

NAK. Please fix your patch.

show me a proof that its copy & paste. because its not


Pavel



--
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottsch...@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets

[Kalle Valo]

Pavel Machek  writes:

>> >>>ath10k_leds.gpio = ar->hw_params.led_pin;
>> >>>gpio_led_register_device(0, _leds);
>> >>
>> >>the problem are other architectures which have already registered gpio_led
>> >>at system start like ar71xx
>> >>you cannot register a second one. so a independend led driver is a
>> >>requirement for direct control
>> >
>> >If the limitation indeed exists, please fix the limitation rather than
>> >working around it in each and every driver.
>> see ath9k. its exact the same implementation.
>
> Ok, so one more driver to fix.
>
>> in addition my variant does also work without gpiolib support. so it
>> can be used even if the kernel is configured without gpio support.
>> and not to forget, using a own led driver is more ligthweight from
>> the call path for each led on / off event which is important for low
>> performance embedded devices
>
> We are not going to copy code because such code works without
> libraries, and we are not going to copy code because that uses
> less cache during calls. Sorry.
>
> NAK. Please fix your patch. 

To me it's not that black and white, sometimes copying code is simpler
than trying to bring up complicated or restricting frameworks (talking
in general here).

I haven't been able to review this patch in detail yet but from a quick
look most of the code is about standard ath10k infrastructure code. How
many lines of code would using leds-gpio save?

-- 
Kalle Valo

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets

[Pavel Machek]

On Thu 2018-03-08 13:33:29, Sebastian Gottschall wrote:
> Am 08.03.2018 um 10:02 schrieb Pavel Machek:
> >On Wed 2018-03-07 18:54:41, Sebastian Gottschall wrote:
> >>Am 07.03.2018 um 17:22 schrieb Rafał Miłecki:
> >>>On 2 March 2018 at 10:22, Sebastian Gottschall  
> >>>wrote:
> >>leds-gpio is crap and limited. you can just register one platform data 
> >>at
> >>kernel runtime since its identified by its object name "led-gpio" but 
> >>the
> >>kernel forbidds to register 2 platform datas with the same name
> >>consider the ar71xx devices with qca988x wifi chipsets. they all have
> >>already a led platform data registered
> >>at boottime. a second can't be registered anymore so gpio_led is useless
> >>at
> >>all for most developers on such platforms. its mainly used for early
> >>kernel
> >>platform data initialisation for system leds.
> >If leds-gpio has limitations, please fix those, rather then
> >introducing duplicated code.
> there is no duplicated code introduced and there is no solution for it.
> consider that all wifi drivers with softled support
> are going that way with registering a own led driver. see ath9k for
> instance. gpio-led cannot be used for it and there is no way to
> support multiple platform datas with the same name. its a kernel 
> limitation
> >>>I just reviewed some mips arch patch adding support for more LEDs for
> >>>selected devices:
> >>>[PATCH] MIPS: BCM47XX: Add Luxul XAP1500/XWR1750 WiFi LEDs
> >>>https://patchwork.linux-mips.org/patch/18681/
> >>>
> >>>It seems to be simply adding another call to the
> >>>gpio_led_register_device(). It seems to me you can call that function
> >>>multiple times and register multiple structs with LEDs.
> >>>
> >>>Isn't all you need something like this?
> >>>
> >>>static const struct gpio_led ath10k_leds[] = {
> >>> {
> >>> .name = "ath10k:color:function",
> >>> .active_low = 1,
> >>> .default_state = LEDS_GPIO_DEFSTATE_KEEP,
> >>> }
> >>>};
> >>>
> >>>static struct gpio_led_platform_data bcm47xx_leds_pdata_extra = {
> >>> leds = ath10k_leds;
> >>> num_leds = ARRAY_SIZE(ath10k_leds);
> >>>};
> >>>
> >>>ath10k_leds.gpio = ar->hw_params.led_pin;
> >>>gpio_led_register_device(0, _leds);
> >>the problem are other architectures which have already registered gpio_led
> >>at system start like ar71xx
> >>you cannot register a second one. so a independend led driver is a
> >>requirement for direct control
> >If the limitation indeed exists, please fix the limitation rather than
> >working around it in each and every driver.
> see ath9k. its exact the same implementation.

Ok, so one more driver to fix.

> in addition my variant does also work without gpiolib support. so it can be
> used even if the kernel is configured
> without gpio support.
> and not to forget, using a own led driver is more ligthweight from the call
> path for each led on / off event which is important for
> low performance embedded devices

We are not going to copy code because such code works without
libraries, and we are not going to copy code because that uses
less cache during calls. Sorry.

NAK. Please fix your patch. 

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Enable IOMMU support for WCN3990 target

[govinds]

On 2018-03-08 19:20, Kalle Valo wrote:

Arnd Bergmann  writes:

On Thu, Mar 1, 2018 at 2:18 PM, Govind Singh 
 wrote:
The asm/dma-iommu.h header file exsists only on arm32, no other 
architecture.

I'm not sure about the purpose of the patch to start with:
it's normally up to the platform code to allocate IOMMU domains,
device drivers should only need to manually interact with the
IOMMU layer if they need more than one domain, but this ath10k
patch appears to be using the default domain and should have no
effect as long as the platform code works correctly.

Thanks Arnd, I have fixed this and migrated to 64bit
API's(iommu_attach_device/iommu_detach_device/
iommu_get_domain_for_dev), will share the next revision.
I tried using the default domain by adding the stream ID and mask in
dt and no manual interaction, but it is resulting in TZ error and
unhandled context fault.
Seems I need to provide explicit mapping range(aperture_start/
aperture_end) as this is only working combination for me..


I don't see why you need to do that at all, can you clarify?

The IOMMU should be set up implicitly for you here based on the iommus
property in DT, with no driver changes at all. This should work on all
architectures/


Maybe Govind is using some out-of-tree tree which is buggy in this
regard?


Actually there is limitations in using the iova address range for wlan 
IP.
It can allow certain iova range and i was attaching the iommu to specify 
the iova range.
I am exploring if i can use "dma-ranges" in dt to avoid the manual 
interaction apart from

stream ID and mask.

BR,
Govind

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Enable IOMMU support for WCN3990 target

[Kalle Valo]

Arnd Bergmann  writes:

> On Thu, Mar 1, 2018 at 2:18 PM, Govind Singh  wrote:
 The asm/dma-iommu.h header file exsists only on arm32, no other 
 architecture.
 I'm not sure about the purpose of the patch to start with:
 it's normally up to the platform code to allocate IOMMU domains,
 device drivers should only need to manually interact with the
 IOMMU layer if they need more than one domain, but this ath10k
 patch appears to be using the default domain and should have no
 effect as long as the platform code works correctly.
>> Thanks Arnd, I have fixed this and migrated to 64bit
>> API's(iommu_attach_device/iommu_detach_device/
>> iommu_get_domain_for_dev), will share the next revision.
>> I tried using the default domain by adding the stream ID and mask in
>> dt and no manual interaction, but it is resulting in TZ error and
>> unhandled context fault.
>> Seems I need to provide explicit mapping range(aperture_start/
>> aperture_end) as this is only working combination for me..
>
> I don't see why you need to do that at all, can you clarify?
>
> The IOMMU should be set up implicitly for you here based on the iommus
> property in DT, with no driver changes at all. This should work on all
> architectures/

Maybe Govind is using some out-of-tree tree which is buggy in this
regard?

-- 
Kalle Valo

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets

[Sebastian Gottschall]

Am 08.03.2018 um 10:02 schrieb Pavel Machek:

On Wed 2018-03-07 18:54:41, Sebastian Gottschall wrote:

Am 07.03.2018 um 17:22 schrieb Rafał Miłecki:

On 2 March 2018 at 10:22, Sebastian Gottschall  wrote:

leds-gpio is crap and limited. you can just register one platform data at
kernel runtime since its identified by its object name "led-gpio" but the
kernel forbidds to register 2 platform datas with the same name
consider the ar71xx devices with qca988x wifi chipsets. they all have
already a led platform data registered
at boottime. a second can't be registered anymore so gpio_led is useless
at
all for most developers on such platforms. its mainly used for early
kernel
platform data initialisation for system leds.

If leds-gpio has limitations, please fix those, rather then
introducing duplicated code.

there is no duplicated code introduced and there is no solution for it.
consider that all wifi drivers with softled support
are going that way with registering a own led driver. see ath9k for
instance. gpio-led cannot be used for it and there is no way to
support multiple platform datas with the same name. its a kernel limitation

I just reviewed some mips arch patch adding support for more LEDs for
selected devices:
[PATCH] MIPS: BCM47XX: Add Luxul XAP1500/XWR1750 WiFi LEDs
https://patchwork.linux-mips.org/patch/18681/

It seems to be simply adding another call to the
gpio_led_register_device(). It seems to me you can call that function
multiple times and register multiple structs with LEDs.

Isn't all you need something like this?

static const struct gpio_led ath10k_leds[] = {
 {
 .name = "ath10k:color:function",
 .active_low = 1,
 .default_state = LEDS_GPIO_DEFSTATE_KEEP,
 }
};

static struct gpio_led_platform_data bcm47xx_leds_pdata_extra = {
 leds = ath10k_leds;
 num_leds = ARRAY_SIZE(ath10k_leds);
};

ath10k_leds.gpio = ar->hw_params.led_pin;
gpio_led_register_device(0, _leds);

the problem are other architectures which have already registered gpio_led
at system start like ar71xx
you cannot register a second one. so a independend led driver is a
requirement for direct control

If the limitation indeed exists, please fix the limitation rather than
working around it in each and every driver.

see ath9k. its exact the same implementation.
in addition my variant does also work without gpiolib support. so it can 
be used even if the kernel is configured

without gpio support.
and not to forget, using a own led driver is more ligthweight from the 
call path for each led on / off event which is important for

low performance embedded devices


Sebastian

Pavel



--
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottsch...@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[RFC] ath10k: report tx rate using ieee80211_tx_status()

[Anilkumar Kolli]

Mesh path metric needs txrate information from ieee80211_tx_status()
call but in ath10k there is no mechanism to report tx rate information
via ieee80211_tx_status(), the rate is only accessible via
sta_statiscs() op.

Per peer stats has tx rate info available, this patch stores per peer
last tx rate and updates the tx rate info structures in tx completition.
The rate updated in ieee80211_tx_status() is not exactly for the last
transmitted frame since the actual rate is available in
HTT_T2H_MSG_TYPE_PEER_STATS.

Per peer txrate information is updated through per peer statistics
and is available for QCA9888/QCA9984/QCA4019/QCA998X only

Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036

Signed-off-by: Anilkumar Kolli 
---
 drivers/net/wireless/ath/ath10k/core.h   |1 +
 drivers/net/wireless/ath/ath10k/htt_rx.c |   97 +-
 drivers/net/wireless/ath/ath10k/mac.c|   13 
 drivers/net/wireless/ath/ath10k/txrx.c   |   25 
 drivers/net/wireless/ath/ath10k/wmi.h|1 +
 5 files changed, 94 insertions(+), 43 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index fe6b30356d3b..d69f0340b487 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -346,6 +346,7 @@ struct ath10k_peer {
 
/* protected by ar->data_lock */
struct ieee80211_key_conf *keys[WMI_MAX_KEY_INDEX + 1];
+   struct ieee80211_tx_info tx_info;
 };
 
 struct ath10k_txq {
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 6d96f9560950..abfea326e1ad 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2427,7 +2427,7 @@ void ath10k_htt_htc_t2h_msg_handler(struct ath10k *ar, 
struct sk_buff *skb)
dev_kfree_skb_any(skb);
 }
 
-static inline bool is_valid_legacy_rate(u8 rate)
+static inline int get_ath10k_rate_idx(u8 rate)
 {
static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
  18, 24, 36, 48, 54};
@@ -2435,20 +2435,23 @@ static inline bool is_valid_legacy_rate(u8 rate)
 
for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
if (rate == legacy_rates[i])
-   return true;
+   return i;
}
 
-   return false;
+   return -EINVAL;
 }
 
 static void
 ath10k_update_per_peer_tx_stats(struct ath10k *ar,
struct ieee80211_sta *sta,
-   struct ath10k_per_peer_tx_stats *peer_stats)
+   struct ath10k_per_peer_tx_stats *peer_stats,
+   struct ath10k_peer *peer)
 {
struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
+   struct ieee80211_chanctx_conf *conf = NULL;
u8 rate = 0, sgi;
struct rate_info txrate;
+   bool skip_auto_rate;
 
lockdep_assert_held(>data_lock);
 
@@ -2457,6 +2460,13 @@ static inline bool is_valid_legacy_rate(u8 rate)
txrate.nss = ATH10K_HW_NSS(peer_stats->ratecode);
txrate.mcs = ATH10K_HW_MCS_RATE(peer_stats->ratecode);
sgi = ATH10K_HW_GI(peer_stats->flags);
+   skip_auto_rate = ATH10K_FW_SKIPPED_RATE_CTRL(peer_stats->flags);
+
+   /* Firmware's rate control skips broadcast/managment frames, if host has
+* configure fixed rates and in some other special cases.
+*/
+   if (skip_auto_rate)
+   return;
 
if (txrate.flags == WMI_RATE_PREAMBLE_VHT && txrate.mcs > 9) {
ath10k_warn(ar, "Invalid VHT mcs %hhd peer stats",  txrate.mcs);
@@ -2470,36 +2480,63 @@ static inline bool is_valid_legacy_rate(u8 rate)
return;
}
 
-   memset(>txrate, 0, sizeof(arsta->txrate));
+   memset(>tx_info.status, 0, sizeof(peer->tx_info.status));
 
if (txrate.flags == WMI_RATE_PREAMBLE_CCK ||
txrate.flags == WMI_RATE_PREAMBLE_OFDM) {
rate = ATH10K_HW_LEGACY_RATE(peer_stats->ratecode);
-
-   if (!is_valid_legacy_rate(rate)) {
-   ath10k_warn(ar, "Invalid legacy rate %hhd peer stats",
-   rate);
-   return;
-   }
-
-   /* This is hacky, FW sends CCK rate 5.5Mbps as 6 */
-   rate *= 10;
-   if (rate == 60 && txrate.flags == WMI_RATE_PREAMBLE_CCK)
-   rate = rate - 5;
-   arsta->txrate.legacy = rate;
-   } else if (txrate.flags == WMI_RATE_PREAMBLE_HT) {
-   arsta->txrate.flags = RATE_INFO_FLAGS_MCS;
-   arsta->txrate.mcs = txrate.mcs + 8 * (txrate.nss - 1);
-   } else {
-   arsta->txrate.flags = RATE_INFO_FLAGS_VHT_MCS;
-   arsta->txrate.mcs = txrate.mcs;
+   

Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets

[Pavel Machek]

On Wed 2018-03-07 18:54:41, Sebastian Gottschall wrote:
> Am 07.03.2018 um 17:22 schrieb Rafał Miłecki:
> >On 2 March 2018 at 10:22, Sebastian Gottschall  
> >wrote:
> leds-gpio is crap and limited. you can just register one platform data at
> kernel runtime since its identified by its object name "led-gpio" but the
> kernel forbidds to register 2 platform datas with the same name
> consider the ar71xx devices with qca988x wifi chipsets. they all have
> already a led platform data registered
> at boottime. a second can't be registered anymore so gpio_led is useless
> at
> all for most developers on such platforms. its mainly used for early
> kernel
> platform data initialisation for system leds.
> >>>If leds-gpio has limitations, please fix those, rather then
> >>>introducing duplicated code.
> >>there is no duplicated code introduced and there is no solution for it.
> >>consider that all wifi drivers with softled support
> >>are going that way with registering a own led driver. see ath9k for
> >>instance. gpio-led cannot be used for it and there is no way to
> >>support multiple platform datas with the same name. its a kernel limitation
> >I just reviewed some mips arch patch adding support for more LEDs for
> >selected devices:
> >[PATCH] MIPS: BCM47XX: Add Luxul XAP1500/XWR1750 WiFi LEDs
> >https://patchwork.linux-mips.org/patch/18681/
> >
> >It seems to be simply adding another call to the
> >gpio_led_register_device(). It seems to me you can call that function
> >multiple times and register multiple structs with LEDs.
> >
> >Isn't all you need something like this?
> >
> >static const struct gpio_led ath10k_leds[] = {
> > {
> > .name = "ath10k:color:function",
> > .active_low = 1,
> > .default_state = LEDS_GPIO_DEFSTATE_KEEP,
> > }
> >};
> >
> >static struct gpio_led_platform_data bcm47xx_leds_pdata_extra = {
> > leds = ath10k_leds;
> > num_leds = ARRAY_SIZE(ath10k_leds);
> >};
> >
> >ath10k_leds.gpio = ar->hw_params.led_pin;
> >gpio_led_register_device(0, _leds);
> the problem are other architectures which have already registered gpio_led
> at system start like ar71xx
> you cannot register a second one. so a independend led driver is a
> requirement for direct control

If the limitation indeed exists, please fix the limitation rather than
working around it in each and every driver.
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k