Re: [PATCH 9/9] staging: rtl8723bs: wrap long lines and fix argument alignment
From: Andy Shevchenko
Date: Tue Apr 14 2026 - 08:31:53 EST
On Tue, Apr 14, 2026 at 12:58:33PM +0300, Marc Finkelbaum wrote:
> Break lines that exceed 80 columns and align continuation arguments to
> the opening parenthesis. Affected sites: scnprintf calls in
> dump_chip_info, hw_chnlPlan assignment, Queue2Pipe[7] assignments in
> the pipe-mapping functions, rtw_hal_set_hwreg in hal_init_macaddr,
> c2h_evt payload read loop, rtw_write16/rtw_write8 in SetHwReg and
> rtw_hal_check_rxfifo_full, rtw_get_stainfo and the UndecoratedSmoothedPWDB
> assignment in GetHalDefVar, ODM_CmnInfoPtrArrayHook calls in SetHalODMVar,
> and rtw_hal_read_rfreg/PHY_SetRFReg in rtw_bb_rf_gain_offset.
>
> No functional change.
"No" to most of the changes here. Use the common sense.
...
> char buf[128];
> size_t cnt = 0;
>
> - cnt += scnprintf(buf + cnt, sizeof(buf) - cnt, "Chip Version Info: CHIP_8723B_%s_",
> - IS_NORMAL_CHIP(ChipVersion) ? "Normal_Chip" : "Test_Chip");
> + cnt += scnprintf(buf + cnt, sizeof(buf) - cnt,
> + "Chip Version Info: CHIP_8723B_%s_",
> + IS_NORMAL_CHIP(ChipVersion) ? "Normal_Chip" :
> + "Test_Chip");
Rather check if this needs to be converted to use sysfs_emit_at().
...
> - hw_chnlPlan = hw_channel_plan & (~EEPROM_CHANNEL_PLAN_BY_HW_MASK);
> + hw_chnlPlan = hw_channel_plan &
> + (~EEPROM_CHANNEL_PLAN_BY_HW_MASK);
Definitely "no". And there are too many parentheses.
hw_chnlPlan = hw_channel_plan & ~EEPROM_CHANNEL_PLAN_BY_HW_MASK;
is exactly 80 characters, btw.
...
> pdvobjpriv->Queue2Pipe[4] = pdvobjpriv->RtOutPipe[0]; /* BCN */
> pdvobjpriv->Queue2Pipe[5] = pdvobjpriv->RtOutPipe[0]; /* MGT */
> pdvobjpriv->Queue2Pipe[6] = pdvobjpriv->RtOutPipe[0]; /* HIGH */
> - pdvobjpriv->Queue2Pipe[7] = pdvobjpriv->RtOutPipe[0]; /* TXCMD */
> + pdvobjpriv->Queue2Pipe[7] =
> + pdvobjpriv->RtOutPipe[0]; /* TXCMD */
"No".
These are well adjusted right now and also you already touched this code
once, do not do ping-pong series (order the changes the way that you don't
change the same line(s) more than once).
...
> } else { /* typical setting */
> +
Huh?!
You have a patch that removed this and other blank lines, what's going on?
...
> if (padapter->eeprompriv.EEPROMRFGainVal != 0xff) {
> - rtw_hal_read_rfreg(padapter, RF_PATH_A, 0x7f, 0xffffffff);
> + rtw_hal_read_rfreg(padapter, RF_PATH_A, 0x7f,
> + 0xffffffff);
83 is okay when it gives better readability (and here is the case).
> for (i = 0; i < ARRAY_SIZE(Array_kfreemap); i += 2) {
> v1 = Array[i];
> v2 = Array[i + 1];
> - if (v1 == padapter->eeprompriv.EEPROMRFGainVal) {
> + if (v1 ==
> + padapter->eeprompriv.EEPROMRFGainVal) {
> target = v2;
> break;
> }
> }
--
With Best Regards,
Andy Shevchenko