rt2x00: add RT6352 specific calibration routines #626
Conversation
This helps devices to sync at better TX/RX rates and improves overall performance. Signed-off-by: Tomislav Požega <pozega.tomislav@gmail.com>
This helps devices to sync at better TX/RX rates and improves overall performance. Signed-off-by: Tomislav Požega <pozega.tomislav@gmail.com>
Add TX self calibration based on mtk driver. Signed-off-by: Tomislav Požega <pozega.tomislav@gmail.com>
Add r calibration code as found in mtk driver. Signed-off-by: Tomislav Požega <pozega.tomislav@gmail.com>
Add RXDCOC calibration code from mtk driver. Please try if this makes any difference among various board/RF layouts. Signed-off-by: Tomislav Požega <pozega.tomislav@gmail.com>
Add RXIQ calibration found in mtk driver. With old openwrt builds this gets us ~8Mbps more of RX bandwidth (test with iPA/eLNA layout). Please try if this makes any difference among various board/RF layouts. Signed-off-by: Tomislav Požega <pozega.tomislav@gmail.com>
pepe2k
added
kernel
Add TX LOFT calibration from mtk driver. Signed-off-by: Tomislav Požega <pozega.tomislav@gmail.com>
There was a problem hiding this comment.
[daniel@box patches]$ /usr/src/linux/scripts/checkpatch.pl 984-rt2x00-add-rxdcoc-calibration.patch
CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt
#30: FILE: drivers/net/wireless/ralink/rt2x00/rt2800lib.c:7884:
+ udelay(50);
ERROR: spaces required around that '==' (ctx:VxV)
#52: FILE: drivers/net/wireless/ralink/rt2x00/rt2800lib.c:7906:
+ if ((bbpreg & 0x40)==0)
^
CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt
#54: FILE: drivers/net/wireless/ralink/rt2x00/rt2800lib.c:7908:
+ udelay(50);
WARNING: Unnecessary space before function pointer arguments
#99: FILE: drivers/net/wireless/ralink/rt2x00/rt2x00.h:576:
+ void (*rxdcoc_calibration) (struct rt2x00_dev *rt2x00dev);
ERROR: Missing Signed-off-by: line(s)
total: 2 errors, 1 warnings, 2 checks, 92 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
984-rt2x00-add-rxdcoc-calibration.patch has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
|
|
||
| /* AGC init */ | ||
| - if (rt2x00_rt(rt2x00dev, RT6352)) | ||
| - reg = 0x04; |
There was a problem hiding this comment.
All vendor drivers I've seen got
R66 = 0x04 + 2 * GET_LNA_GAIN(pAd);
hence I doubt reverting to 0x1c instead of 0x04 is actually correct. However, if it actually measurably improves anything, I'm fine with giving it a spin.
There was a problem hiding this comment.
take a look at RT6352_VGA_TABLE in cmm_rf_cal.c ! there is no power entry for 0x04 value causing chip to stuck at very low throughput. mtk driver maybe writes this reg on-the-fly in STA mode
There was a problem hiding this comment.
RT6352_VGA_TABLE in cmm_rf_cal.c isn't actually used anywhere, apart from some traces in debug messages inside DPD calibration which isn't implemented in rt2x00...
And while it's true that RT6352_ChipAGCAdjust got an #ifdef CONFIG_STA_SUPPORT around it which hints that it's only being used in STA mode, there is also RT6352_RTMPSetAGCInitValue which sets register 66 in the way described above independently of the mode of operation.
My guess is that 0x04 is fine for ePA and works for iPA as well if we do DPD calibration.
There was a problem hiding this comment.
in rtmp_init_inf DPD is done before RT6352_Init_ExtPA_ExtLNA. with rt2x00 LNA is always on, can't even turn it off passing 0 to TX_PIN_CFG_LNA_PE_G0_EN or TX_PIN_CFG_LNA_PE_G1_EN. calibration as such does not help, but i'm also missing a piece of code from RtmpKickOutHwNullFrame. do you know is there an equivalent in rt2x00 that would transmit a packet?
There was a problem hiding this comment.
managed to run it with R66 0x04 without regression.and without improvement too so best to revert this reg and focus on other things
| @@ -0,0 +1,89 @@ | |||
| --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c | |||
There was a problem hiding this comment.
Please create kernel patches like this 982-rt2x00-add-rf-self-txdc-calibration.patch file using git format-patch --to linux-wireless@vger.kernel.org --cc "Stanislaw Gruszka <sgruszka@redhat.com>" --cc "Kalle Valo <kvalo@codeaurora.org>" with commits on top of the wireless-drivers-next tree so they end up in the right patchwork. (your current patches should apply on that tree just fine)
Each commit should include a meaningful patch description starting with the rt2x00: prefix and your Signed-of-by: tag so they are fit for upstream submission. A basic description like the ones you wrote for the commits adding the patches to OpenWrt is fine, just also make sure to mention that this is for MT7620/RT6352 WiSoC in each patch description.
| + u8 r_cal_code = 0; | ||
| + char d1 = 0, d2 = 0; | ||
| + u8 rfvalue; | ||
| + u32 MAC_RF_BYPASS0, MAC_RF_CONTROL0, MAC_PWR_PIN_CFG; |
There was a problem hiding this comment.
kernel style prohibits all-capital variable names. please change this and ideally use scripts/check-patch.pl to fix style issues prior to submission.
|
Thanks for the great work! Just a few remaining style issues and this is ready to be included and submitted upstream.
Which build did you test it with? |
besides code-style issues that i haven't checked for could you take a closer look at code that differentiates between chains, variables like chain or ch_idx? i'd like to use rt2x00 code for this but not sure which of the two would be correct to use ant->tx_chain_num or default_ant.tx_chain_num
CC, OpenWrt from archived branch with kernel 4.4 |
| + | ||
| + return root; | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_do_sqrt_accumulation); |
There was a problem hiding this comment.
You declare rt2800_do_sqrt_accumulation as static above which contradicts the export. Besides that, what would be the external user of that function?
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_do_sqrt_accumulation); | ||
| + | ||
| +void rt2800_rxiq_calibration(struct rt2x00_dev *rt2x00dev) { |
There was a problem hiding this comment.
Shouldn't this be static void ... as well?
| + rt2800_register_write(rt2x00dev, RF_BYPASS3, orig_RF_BYPASS3); | ||
| + rt2800_register_write(rt2x00dev, MAC_SYS_CTRL, savemacsysctrl); | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_rxiq_calibration); |
There was a problem hiding this comment.
There are no users of this function outside of this very file. No need to export it.
| int rt2800_calcrcalibrationcode(struct rt2x00_dev *rt2x00dev, int d1, int d2); | ||
| void rt2800_r_calibration(struct rt2x00_dev *rt2x00dev); | ||
| void rt2800_rxdcoc_calibration(struct rt2x00_dev *rt2x00dev); | ||
| +void rt2800_rxiq_calibration(struct rt2x00_dev *rt2x00dev); |
There was a problem hiding this comment.
No need to define it here. If you are calling the function before defining it in rt2800lib.c, just include the protoype in rt2800lib.c or move the function above the call within the file.
There was a problem hiding this comment.
tried to keep functions in the same order as with mtk driver, so i kept this one.
| int (*calcrcalibrationcode) (struct rt2x00_dev *rt2x00dev, int d1, int d2); | ||
| void (*r_calibration) (struct rt2x00_dev *rt2x00dev); | ||
| void (*rxdcoc_calibration) (struct rt2x00_dev *rt2x00dev); | ||
| + void (*rxiq_calibration) (struct rt2x00_dev *rt2x00dev); |
There was a problem hiding this comment.
No need to define it here (there is no call to the function using that function pointer)
| } | ||
| EXPORT_SYMBOL_GPL(rt2800_r_calibration); | ||
|
|
||
| +void rt2800_rxdcoc_calibration(struct rt2x00_dev *rt2x00dev) |
| + | ||
| + rt2800_rfcsr_write_bank(rt2x00dev, 0, 2, saverfb0r2); | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_rxdcoc_calibration); |
There was a problem hiding this comment.
No need to export it if it's only being called below in rt2800_init_rfcsr_6352
| void rt2800_rf_self_txdc_cal(struct rt2x00_dev *rt2x00dev); | ||
| int rt2800_calcrcalibrationcode(struct rt2x00_dev *rt2x00dev, int d1, int d2); | ||
| void rt2800_r_calibration(struct rt2x00_dev *rt2x00dev); | ||
| +void rt2800_rxdcoc_calibration(struct rt2x00_dev *rt2x00dev); |
| void (*rf_self_txdc_cal) (struct rt2x00_dev *rt2x00dev); | ||
| int (*calcrcalibrationcode) (struct rt2x00_dev *rt2x00dev, int d1, int d2); | ||
| void (*r_calibration) (struct rt2x00_dev *rt2x00dev); | ||
| + void (*rxdcoc_calibration) (struct rt2x00_dev *rt2x00dev); |
| } | ||
| EXPORT_SYMBOL_GPL(rt2800_rf_self_txdc_cal); | ||
|
|
||
| +int rt2800_calcrcalibrationcode(struct rt2x00_dev *rt2x00dev, int d1, int d2) |
| + | ||
| + return calcode; | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_calcrcalibrationcode); |
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_calcrcalibrationcode); | ||
| + | ||
| +void rt2800_r_calibration(struct rt2x00_dev *rt2x00dev) |
| + rt2800_register_write(rt2x00dev, MAC_SYS_CTRL, savemacsysctrl); | ||
| + rt2800_register_write(rt2x00dev, PWR_PIN_CFG, MAC_PWR_PIN_CFG); | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_r_calibration); |
| void rt2800_gain_calibration(struct rt2x00_dev *rt2x00dev); | ||
| void rt2800_vco_calibration(struct rt2x00_dev *rt2x00dev); | ||
| void rt2800_rf_self_txdc_cal(struct rt2x00_dev *rt2x00dev); | ||
| +int rt2800_calcrcalibrationcode(struct rt2x00_dev *rt2x00dev, int d1, int d2); |
| void rt2800_vco_calibration(struct rt2x00_dev *rt2x00dev); | ||
| void rt2800_rf_self_txdc_cal(struct rt2x00_dev *rt2x00dev); | ||
| +int rt2800_calcrcalibrationcode(struct rt2x00_dev *rt2x00dev, int d1, int d2); | ||
| +void rt2800_r_calibration(struct rt2x00_dev *rt2x00dev); |
| void (*vco_calibration) (struct rt2x00_dev *rt2x00dev); | ||
| void (*rf_self_txdc_cal) (struct rt2x00_dev *rt2x00dev); | ||
| + int (*calcrcalibrationcode) (struct rt2x00_dev *rt2x00dev, int d1, int d2); | ||
| + void (*r_calibration) (struct rt2x00_dev *rt2x00dev); |
There was a problem hiding this comment.
both above lines are unneeded (both functions are only being called within rt2800lib.c and only needed for that specific chip)
There was a problem hiding this comment.
ok. so linking this code to rt2x00lib_ops in rt2800soc wouldn't help any? i ask beacuse i see all functions listed in the ops sections of soc/usb/pci
There was a problem hiding this comment.
this is only needed for functions which have different implementations for different ralink chips and/or bus-access schemes. different driver code can then use the function pointers to call out to a chip-specific routine. calcrcalibration is only used inside of rt2800lib.c and doesn't any callers outside of it (should it?), the same applies to r_calibration. In case those functions should have any other callers (like reoccurently being called by a mac80211 tasklet or such) the situation may be different...
| rt2800_led_open_drain_enable(rt2x00dev); | ||
| } | ||
|
|
||
| +void rt2800_rf_self_txdc_cal(struct rt2x00_dev *rt2x00dev) |
| + rt2x00_info(rt2x00dev, "RF Tx self calibration end\n"); | ||
| + | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_rf_self_txdc_cal); |
| const u32 count); | ||
| void rt2800_gain_calibration(struct rt2x00_dev *rt2x00dev); | ||
| void rt2800_vco_calibration(struct rt2x00_dev *rt2x00dev); | ||
| +void rt2800_rf_self_txdc_cal(struct rt2x00_dev *rt2x00dev); |
| struct link_qual *qual, const u32 count); | ||
| void (*gain_calibration) (struct rt2x00_dev *rt2x00dev); | ||
| void (*vco_calibration) (struct rt2x00_dev *rt2x00dev); | ||
| + void (*rf_self_txdc_cal) (struct rt2x00_dev *rt2x00dev); |
| + | ||
| + return; | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_rf_configstore); |
| + | ||
| + return; | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_rf_configrecover); |
| + | ||
| + return; | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_setbbptonegenerator); |
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_setbbptonegenerator); | ||
| + | ||
| +u32 rt2800_do_fft_accumulation(struct rt2x00_dev *rt2x00dev, u8 tidx, u8 read_neg) |
| + | ||
| + return pint; | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_do_fft_accumulation); |
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_do_fft_accumulation); | ||
| + | ||
| +u32 rt2800_read_fft_accumulation(struct rt2x00_dev *rt2x00dev, u8 tidx) { |
| + | ||
| + return pint; | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_read_fft_accumulation); |
| + | ||
| + return; | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_write_dc); |
| + | ||
| + return; | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_loft_search); |
| + rt2800_rfcsr_write_bank(rt2x00dev, 5, 19, 0xa2); | ||
| + rt2800_rfcsr_write_bank(rt2x00dev, 5, 20, 0x20); | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_rf_aux_tx0_loopback); |
| + rt2800_rfcsr_write_bank(rt2x00dev, 7, 19, 0xa2); | ||
| + rt2800_rfcsr_write_bank(rt2x00dev, 7, 20, 0x20); | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_rf_aux_tx1_loopback); |
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_rf_aux_tx1_loopback); | ||
| + | ||
| +void rt2800_loft_iq_calibration(struct rt2x00_dev *rt2x00dev) |
There was a problem hiding this comment.
makes no difference, builds just the same as without static declaration..
| + | ||
| + return; | ||
| +} | ||
| +EXPORT_SYMBOL_GPL(rt2800_loft_iq_calibration); |
| +#define CHAIN_0 0x0 | ||
| +#define CHAIN_1 0x1 | ||
| +#define RF_ALC_NUM 6 | ||
| +#define CHAIN_NUM 2 |
There was a problem hiding this comment.
This is specific for RT6252, we should get the number of (active) chains from driver runtime in rt2x00.
There was a problem hiding this comment.
i don't think it matters as long as there is no way to properly disable unused chain in b/g mode
| void rt2800_r_calibration(struct rt2x00_dev *rt2x00dev); | ||
| void rt2800_rxdcoc_calibration(struct rt2x00_dev *rt2x00dev); | ||
| void rt2800_rxiq_calibration(struct rt2x00_dev *rt2x00dev); | ||
| +void rt2800_loft_iq_calibration(struct rt2x00_dev *rt2x00dev); |
There was a problem hiding this comment.
needed? (ie. called form anywhere outside of rt2800lib.c?)
| void (*r_calibration) (struct rt2x00_dev *rt2x00dev); | ||
| void (*rxdcoc_calibration) (struct rt2x00_dev *rt2x00dev); | ||
| void (*rxiq_calibration) (struct rt2x00_dev *rt2x00dev); | ||
| + void (*loft_iq_calibration) (struct rt2x00_dev *rt2x00dev); |
There was a problem hiding this comment.
Needed? (ie. needs per-chip-type abstraction? imho this is only relevant for MT7620/RT6352 which is always rt2800mmio)
There was a problem hiding this comment.
no. calibration will run without this. with doubts in it's efficiency.
|
Are you still going to update this PR? |
|
Sure, take your time, I just wanted to avoid redundant work on the same issue. About the binary: I don't own that hardware (miwifi mini), unfortunately. Can you make a build for either ZBT-WE1026 (iLNA/iPA) or Phicomm PSG1218B (eLNA,ePA )and share that instead? |
Add several calibration routines as found in mtk driver. Reduce power usage and revert AGC register value.
Signed-off-by: Tomislav Požega pozega.tomislav@gmail.com