linux-kernel - Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <44468cbc-6dc1-4b73-a2f5-eca7742241b3@collabora.com>
Date: Tue, 22 Jul 2025 10:24:44 +0300
From: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
To: Roderick Colenbrander <thunderbird2k@...il.com>
Cc: Roderick Colenbrander <roderick.colenbrander@...y.com>,
 Jiri Kosina <jikos@...nel.org>, Benjamin Tissoires <bentiss@...nel.org>,
 Henrik Rydberg <rydberg@...math.org>, kernel@...labora.com,
 linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack
 handling on DualSense

Hi Roderick,

On 7/22/25 8:47 AM, Roderick Colenbrander wrote:
> Hi Christian,
> 
> I just got back from Japan (trip was a bit extended). In the meantime
> I had some of employees had a look as well.
> 
> The audio patches towards the end seem to be okay. We tried to dig for
> the official volume numbers, but they were too hard to find (too many
> layers, too many repositories). When we use a PS5, the default volume
> for the headset and speaker are both close to 70% (just eyeballing).
> At the hardware level the volume is quite non-linear and internally we
> use a mapping table (not sure what the curve is based on). For the
> speaker this starts at 0x3d as you found out already. The 70% volume
> for the speaker seems to correspond to a value of 93 and headphones
> 83.
> The set pre-amp gain of 0x2 is a common value we seem to set and means
> +6dB, so change comment around to mean that I guess.

Thanks for the additional clarifications.  I added a fixup below, if Jiri is
fine applying that before merging, just to avoid respinning the whole
series:

Subject: [PATCH] fixup! HID: playstation: Support DualSense audio jack
 hotplug detection

---
 drivers/hid/hid-playstation.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 4285260c7e22..641c6337ff63 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -1352,7 +1352,7 @@ static void dualsense_output_worker(struct work_struct *work)
 				 */
 				common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_SPEAKER_VOLUME_ENABLE;
 				common->speaker_volume = 0x64;
-				/* Set SP preamp gain to ~30% */
+				/* Set SP preamp gain to +6dB */
 				common->valid_flag1 = DS_OUTPUT_VALID_FLAG1_AUDIO_CONTROL2_ENABLE;
 				common->audio_control2 =
 					FIELD_PREP(DS_OUTPUT_AUDIO_FLAGS2_SP_PREAMP_GAIN, 0x2);


> As for the other patches I'm not entirely sure yet. I know they were
> well intended, but let me just say, they rubbed some of my team
> members quite the wrong way resulting in some heavy discussion. I have
> somewhat similar feelings about the ultra strict checkpatch toggle as
> well.
> 
> We had to move mountains to be allowed to even upstream controller
> code among our limited time (it is closer to a hobby thing, even
> though many products nowadays use it as well). So that's a factor
> which adds up a bit as well.
> 
> I think some of the patches we could live with if it came to it. There
> is no real agreed up full kernel standard (as it is contentious). So
> for example we tend to prefer more uint8_t family, where older kernel
> style was more u8 and the kernel allows for both. I think we would
> probably lean towards keeping it at the modern form.
> 
> Some of the macros also felt a little too magical. Our feeling tends
> to be if you have to go many layers deep to understand what a macro or
> line of code does (and it is easier to then printk the value),
> something feels off...

I'm sorry for all the troubles introduced with the additional patches! My
intention was not to highlight deficiencies with the current implementation,
but to bring the driver as close as possible to the coding standard agreed
by the kernel community, to avoid dealing with the kind of problems that I
tried to explain a while ago.

Thanks again for your support,
Cristian