linux-kernel - Re: [PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver

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 for Android: free password hash cracker in your pocket

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

Message-ID: <830d8e26-dbb9-4b9c-bbab-a5c4c49a7ffd@linux.intel.com>
Date:   Tue, 5 Dec 2023 11:22:32 -0600
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Gergo Koteles <soyer@....hu>, Shenghao Ding <shenghao-ding@...com>,
        Kevin Lu <kevin-lu@...com>, Baojun Xu <baojun.xu@...com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>
Cc:     linux-kernel@...r.kernel.org, alsa-devel@...a-project.org
Subject: Re: [PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver



>>>>> +static void tas2563_fixup_i2c(struct hda_codec *cdc,
>>>>> +	const struct hda_fixup *fix, int action)
>>>>> +{
>>>>> +	 tas2xxx_generic_fixup(cdc, action, "i2c", "INT8866");
>>>>
>>>> Any specific reason to use an Intel ACPI identifier? Why not use
>>>> "TIAS2563" ?
>>>>
>>> INT8866 is in the ACPI.
>>> I don't know why Lenovo uses this name.
>>> I think it's more internal than intel.
>>>
>>>    Scope (_SB.I2CD)
>>>     {
>>>         Device (TAS)
>>>         {
>>>             Name (_HID, "INT8866")  // _HID: Hardware ID
>>
>> Ouch, I hope they checked with Intel that this isn't an HID already in
>> use...
>>
> It looks the INT prefix is not reserved. (yet)
> https://uefi.org/ACPI_ID_List?acpi_search=INT

It's been de-facto reclaimed by Intel over the years, apparently using
INTC or INTL was too hard for some of my colleagues...

There are lots of INT devices in the kernel today, here's a small list
for sound/soc/codecs only

rt274.c:        { "INT34C2", 0 },
rt286.c:        { "INT343A", 0 },
rt298.c:        { "INT343A", 0 },
ssm4567.c:      { "INT343B", 0 },

Those INT values were added by Intel teams though, it's really odd to
see Lenovo use an INT-based HID. Should really use 104C2563 or something.


>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static const struct dev_pm_ops tas2563_hda_pm_ops = {
>>>>> +	SYSTEM_SLEEP_PM_OPS(tas2563_system_suspend, tas2563_system_resume)
>>>>
>>>> where's the pm_runtime stuff?
>>>>
>>>
>>> The amp stores its state in software shutdown mode.
>>> The tas2563_hda_playback_hook wakes/shutdowns the amp, not the
>>> pm_runtime.
>>
>> My point was that you have all these pm_runtime_ calls in the code, but
>> nothing that provides pm_runtime suspend-resume functions so not sure
>> what exactly the result is?
>>
>>
> I think nothing. I haven't experienced anything unusual recently.

you can probably see from the /sys directory what the pm_runtime power
state is, most likely the status is 'unknown'.