linux-kernel - Re: [PATCH staging] staging: rtl8723bs: replace magic numbers in rtl8723b

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9f4ae2d7-5a8b-4785-9df8-3d22925b8f3a@suswa.mountain>
Date: Tue, 1 Jul 2025 17:10:58 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Marcos Garcia <magazo2005@...il.com>
Cc: gregkh@...uxfoundation.org, philipp.g.hortmann@...il.com,
	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
 rtl8723b_InitBeaconParameters()

On Tue, Jul 01, 2025 at 09:40:49AM +0200, Marcos Garcia wrote:
> 
> 
> Replace hardcoded values in rtl8723b_InitBeaconParameters() with defined constants
> TBTT_PROHIBIT_VENDOR_DEFAULT (0x6404) and BCNTCFG_AIFS_MAX (0x660F) for clarity and
> maintainability, addressing the TODO comment in the code.
> 
> The values were sourced from the following documentation:
> - REG_TBTT_PROHIBIT (Offset 0x0540): Bits [15:8] = 0x64 (100ms prohibit time, 1ms units),
>   Bits [7:0] = 0x04 (2ms margin, 0.5ms units), as per RTL8723BS Datasheet v1.5,
>   Section 7.3.1.5 and RTL8723BS Programming Guide, p. 112.
> - REG_BCNTCFG (Offset 0x0510): 0x660F sets max AIFS (0x0F) to prioritize beacon
>   transmission, as per RTL8723BS Datasheet v1.5, Section 7.3.1.3.
> 
> Hi Dan,
> 
> Thank you for your detailed feedback — I truly appreciate it. I tried to contact you
> earlier, but it seems my email didn't reach you. This is my first kernel contribution,
> and I started by addressing TODO comments, thinking they were straightforward. I now
> realize even these changes require deep hardware understanding. I used AI to assist with
> parts of the commit message, but I didn't review it thoroughly enough, and I take full
> responsibility for the vague comments. I could only find limited references to these
> values, and the documentation seems restricted. I apologize for any oversight and
> promise to research more carefully in the future. Thank you for your guidance.
> 

I feel like there needs to be some kind of notice on patches which are
generated with AI.  I was so puzzled when it said that the units were
in half miliseconds, but AI sounds so confident and professional and
people do crazy things all the time.

I have sent my fair share of guesswork patch where it's like "I think
the code is correct, but it's just indented incorrectly" or whatever.
And I try to put a note to say that it's guesswork so, "Please, review
this one extra carefully."  With AI it's all 100% guesswork, but it's
formatted as if it's quoting a spec.  There is a lot of potential for
confusion.

regards,
dan carpenter