[OpenWrt-Devel] [PATCH v3 1/2] [mac80211] Add "channel set helper" callback to ath9k

[Felix Fietkau]

On 2015-06-13 10:57, Stefan Rompf wrote:
> Hi Felix,
> 
> thanks for your feedback!
> 
>> NACK from me for the ath9k_set_channel_helper part. This helper function
>> needs to be passed in from the platform data, similar to how external
>> reset is handled.
>> From previous emails you pointed out that you guys chose this
>> design/structure simply because you want to have an easy way of
>> debugging, but I think that's not a strong enough reason to make the API
>> between ath9k and the HSR code this quirky.
>> 
>> Here's what I want:
>> The hsr filter code function is passed in by the mach-* function for the
>> UniFi Outdoor, and the code is in the kernel itself.
> 
> however this approach looks even more ugly to me.
> 
> The HSR is connected to the GPIO lines of the AR928x. As long as ath9k does 
> not register with the kernels' GPIO subsystem (which is definitely Designated 
> Driver stuff if I had to implement it), we need to callback into the ath9k 
> driver when tuning it. We'd end up with something like this in 
> ath9k_platform.h:
Okay, I missed that. I thought it was on system GPIO lines.

> typedef void (cfg_gpio_input_fkt)(struct ath_hw *ah, u32 gpio);
> typedef void (cfg_gpio_output_fkt)struct ath_hw *ah, u32 gpio,                        
> u32 ah_signal_type);
> typedef void (set_gpio_fkt)(struct ath_hw *ah, u32 gpio, u32 val);
> typedef u32 (gpio_get_fkt)(struct ath_hw *ah, u32 gpio, u32 val);
> 
> struct ath9k_platform_data {
> ...
>  void (*set_channel_helper)(struct ath_hw*, int bw, int fq, cfg_gpio_input_fkt 
> *,  cfg_gpio_output_fkt *, set_gpio_fkt*, gpio_get_fkt*);
> ...
> };
> 
> and the callback inside channel.c:
> 
> if (sc->dev->platform_data->set_channel_helper) {
>  sc->dev->platform_data->set_channel_helper(ah, chandef->width, chan-
>>center_freq, ath9k_hw_cfg_gpio_input, ath9k_hw_cfg_output, ath9k_hw_set_gpio, 
> ath9k_hw_gpio_get);
> }
> 
>> And here's how you make debugging easy:
>> You make a package (which can live somewhere in a separate git tree),
>> which contains a copy of the code that is also in the kernel, and when
>> this package gets loaded, it looks up the ath9k pci device and the
>> platform data, and it overwrites the channel helper function callback.
> 
> May be we can use a similiar approach to avoid this function pointer passing 
> horror. Let's have a 
> 
> void (*set_channel_helper)(struct ath_hw*, int bw, int fq)
> 
> callback in ath9k_platform_data that is enabled by the channel tuning module. 
> The module could then call into ath9k without indirections.
> 
> What do you think?
That won't work, because the kernel ath9k header files are not in sync
with the ones from the mac80211 package. Maybe in this case it would be
better to put the code in ath9k.ko and enable it via a Kconfig option in
the mac80211 package. Then add a bool flag to the ath9k_platform_data
struct to enable it only for devices that need it.

- Felix
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel