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

[Stefan Rompf]

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:

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?

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