Re: [PATCHv2 2/2] ath10k: add spectral scan feature

[Kalle Valo <kvalo@qca.qualcomm.com>]

Simon Wunderlich <sw@simonwunderlich.de> writes:

> Hey Kalle,
>
> thanks for all the comments!
>
> I'll make the changes as you suggested, although I'd like to point out that 
> the "empty line before comment" and the "no indentation for WMI command 
> parameters" things are not really consistent - there are also empty lines 
> missing (e.g. struct wmi_start_scan_cmd) and indentation present (e.g. 
> ath10k_wmi_peer_assoc and many others). Maybe that should be cleaned up at 
> some point if you think that's important. :)

Yeah, I have been sloppy in reviewing those part before but not anymore
:) Also struct ath10k is quite a mess now. Sorry for the confusion.

I'm planning to fix these at some point, but if someone else wants to do
it I'm very happy to take patches.

>> > +struct fft_sample_ath10k {
>> > +	struct fft_sample_tlv tlv;
>> > +	u8 chan_width_mhz;
>> > +	__be16 freq1;
>> > +	__be16 freq2;
>> > +	__be16 noise;
>> > +	__be16 max_magnitude;
>> > +	__be16 total_gain_db;
>> > +	__be16 base_pwr_db;
>> > +	__be64 tsf;
>> > +	s8 max_index;
>> > +	u8 rssi;
>> > +	u8 relpwr_db;
>> > +	u8 avgpwr_db;
>> > +	u8 max_exp;
>> > +
>> > +	u8 data[0];
>> > +} __packed;
>> 
>> __be16, that's a first. Just making sure that this really is big endian?
>
> As the __le32 you use in other ath10k structs, this is just for checking - at 
> least sparse should check that, maybe other tools as well.

Sorry, I didn't understand your comment here. But basically I was asking
is the fft sample really in big endian? I assume it would be little
endian as everything else coming from the firmware.

-- 
Kalle Valo