| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2025070157-suing-uptake-356e@gregkh>
Date: Tue, 1 Jul 2025 07:14:13 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Marcos Garcia <magazo2005@...il.com>
Cc: philipp.g.hortmann@...il.com, dan.carpenter@...aro.org,
karanja99erick@...il.com, rodrigo.gobbi.7@...il.com,
linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH staging] staging: rtl8723bs: Replace magic numbers in
beacon initialization
On Mon, Jun 30, 2025 at 11:02:11PM +0200, Marcos Garcia wrote:
> Replace hardcoded register values in rtl8723b_InitBeaconParameters()
> with descriptive defines to improve code readability and maintainability:
> - TBTT_PROHIBIT_DEFAULT (0x6404): Sets TBTT prohibit time to 100ms
> (bits [15:8]) with a 2ms margin (bits [7:0]).
> - BCNTCFG_DEFAULT (0x660F): Configures maximum AIFS for beacon
> transmission to ensure high priority, as recommended by the designer.
>
> Preserve original comments where they provide useful context, such as
> firmware control in power-saving mode and designer notes about beacon
> contention. Fix typo "contension" to "contention" in the comment.
>
> Signed-off-by: Marcos Garcia <magazo2005@...il.com>
> ---
> .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 50 +++++++++++++++----
> kernel/sched/ext.c | 8 ++-
> 2 files changed, 46 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> index cc7886d75a0b..a3438280e1ab 100644
> --- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> +++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> @@ -1183,9 +1183,25 @@ void rtl8723b_read_chip_version(struct adapter *padapter)
> ReadChipVersion8723B(padapter);
> }
>
> -/* Beacon Configuration */
> -#define TBTT_PROHIBIT_DEFAULT_MS 0x6404 /* 100ms interval + 4ms margin */
> -#define BCNTCFG_AIFS_LARGEST 0x660F /* Max AIFS for beacon priority */
> +/*
> + * Beacon Configuration
> + *
> + * REG_TBTT_PROHIBIT: 16-bit register where:
> + * - Bits [15:8] = TBTT prohibit time in units of 1ms
> + * - Bits [7:0] = TBTT prohibit margin time in units of 0.5ms
> + * Default value of 0x6404 means:
> + * - 0x64 (100) ms prohibit time
> + * - 0x04 (4 * 0.5 = 2) ms margin time
> + */
> +#define TBTT_PROHIBIT_100MS_2MS_MARGIN 0x6404
> +
> +/*
> + * REG_BCNTCFG: Beacon configuration register
> + * 0x660F sets AIFS to maximum value (0xF) with other default parameters
> + * This ensures beacons get highest priority (no contention window),
> + * as suggested by the original designer for test chips.
> + */
> +#define BCNTCFG_MAX_AIFS 0x660F
>
> void rtl8723b_InitBeaconParameters(struct adapter *padapter)
> {
> @@ -1193,19 +1209,31 @@ void rtl8723b_InitBeaconParameters(struct adapter *padapter)
> u16 val16;
> u8 val8 = DIS_TSF_UDT;
>
> - val16 = val8 | (val8 << 8); /* port0 and port1 */
> - val16 |= EN_BCN_FUNCTION; /* Enable prot0 beacon function */
> + /* Configure beacon control for both port 0 and port 1 */
> + val16 = val8 | (val8 << 8);
> + val16 |= EN_BCN_FUNCTION; /* Enable beacon function for PSTDMA */
> rtw_write16(padapter, REG_BCN_CTRL, val16);
>
> - /* Fixed: Replaced magic numbers with defines */
> - rtw_write16(padapter, REG_TBTT_PROHIBIT, TBTT_PROHIBIT_DEFAULT_MS);
> + /* Configure TBTT prohibit timer with 100ms interval + 2ms margin */
> + rtw_write16(padapter, REG_TBTT_PROHIBIT, TBTT_PROHIBIT_100MS_2MS_MARGIN);
> +
> + /*
> + * Firmware controls early interrupt timing in power save mode,
> + * so only set REG_DRVERLYINT when not in station mode
> + */
> + if (!check_fwstate(&padapter->mlmepriv, WIFI_STATION_STATE))
> + rtw_write8(padapter, REG_DRVERLYINT, DRIVER_EARLY_INT_TIME_8723B); /* 5ms */
>
> - if (check_fwstate(&padapter->mlmepriv, WIFI_STATION_STATE) == false)
> - rtw_write8(padapter, REG_DRVERLYINT, DRIVER_EARLY_INT_TIME_8723B);
> + /* Set beacon DMA interrupt time to 2ms */
> + rtw_write8(padapter, REG_BCNDMATIM, BCN_DMA_ATIME_INT_TIME_8723B); /* 2ms */
>
> - rtw_write8(padapter, REG_BCNDMATIM, BCN_DMA_ATIME_INT_TIME_8723B);
> - rtw_write16(padapter, REG_BCNTCFG, BCNTCFG_AIFS_LARGEST);
> + /*
> + * Suggested by designer timchen: Set AIFS to maximum to avoid contention
> + * before sending beacons on test chips. By tynli, 2009.11.03
> + */
> + rtw_write16(padapter, REG_BCNTCFG, BCNTCFG_MAX_AIFS);
>
> + /* Save initial register values for reference */
> pHalData->RegBcnCtrlVal = rtw_read8(padapter, REG_BCN_CTRL);
> pHalData->RegTxPause = rtw_read8(padapter, REG_TXPAUSE);
> pHalData->RegFwHwTxQCtrl = rtw_read8(padapter, REG_FWHW_TXQ_CTRL+2);
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index b498d867ba21..7cecc0ca700d 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -4258,7 +4258,13 @@ void scx_group_set_weight(struct task_group *tg, unsigned long weight)
>
> void scx_group_set_idle(struct task_group *tg, bool idle)
> {
> - /* TODO: Implement ops->cgroup_set_idle() */
> + struct sched_ext_ops *ops;
> +
> + rcu_read_lock();
> + ops = rcu_dereference(ext_ops);
> + if (ops && ops->cgroup_set_idle)
> + ops->cgroup_set_idle(tg, idle);
> + rcu_read_unlock();
> }
>
> static void scx_cgroup_lock(void)
> --
> 2.50.0
>
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/process/submitting-patches.rst for what
needs to be done here to properly describe this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
Powered by blists - more mailing lists