Re: [PATCH] ALSA: hda - Add new driver for HDA controllers listed via ACPI

[Takashi Iwai <tiwai@suse.de>]

On Thu, 15 May 2025 17:45:08 +0200,
Daniel Dadap wrote:
> 
> On Thu, May 15, 2025 at 04:45:52PM +0200, Takashi Iwai wrote:
> > On Thu, 15 May 2025 15:31:46 +0200,
> > Daniel Dadap wrote:
> > > 
> > > Some systems expose HD-Audio controllers via objects in the ACPI tables
> > > which encapsulate the controller's interrupt and the base address for the
> > > HDA registers in an ACPI _CRS object, for example, as listed in this ACPI
> > > table dump excerpt:
> > > 
> > >         Device (HDA0)
> > >         {
> > >             Name (_HID, "NVDA2014")  // _HID: Hardware ID
> > >             ...
> > >             Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
> > >             {
> > >                 Memory32Fixed (ReadWrite,
> > >                     0x36078000,         // Address Base
> > >                     0x00008000,         // Address Length
> > >                     )
> > >                 Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, )
> > >                 {
> > >                     0x0000021E,
> > >                 }
> > >             })
> > >         }
> > > 
> > > Add support for HDA controllers discovered through ACPI, including support
> > > for some platforms which expose such HDA controllers on NVIDIA SoCs. This
> > > is done with a new driver which uses existing infrastructure for extracting
> > > resource information from _CRS objects and plumbs the parsed resource
> > > information through to the existing HDA infrastructure to enable HD-Audio
> > > functionality on such devices.
> > > 
> > > Although this driver is in the sound/pci/hda/ directory, it targets devices
> > > which are not actually enumerated on the PCI bus. This is because it depends
> > > upon the Intel "Azalia" infrastructure which has traditionally been used for
> > > PCI-based devices.
> > > 
> > > Signed-off-by: Daniel Dadap <ddadap@nvidia.com>
> > 
> > Thanks for the patch.
> > The code looks fine.  Just a nitpicking:
> > 
> > > +static int __maybe_unused hda_acpi_suspend(struct device *dev)
> > ....
> > > +static int __maybe_unused hda_acpi_resume(struct device *dev)
> > 
> > The __maybe_unused is superfluous when you set up
> > SYSTEM_SLEEP_PM_OPS() macro instead in the below:
> > 
> > > +static const struct dev_pm_ops hda_acpi_pm = {
> > > +	SET_SYSTEM_SLEEP_PM_OPS(hda_acpi_suspend, hda_acpi_resume)
> >
> 
> Thanks, I'll send an updated patch. The hda_tegra driver has these as well
> so I presume it can be cleaned up from that driver as well?

It's been already cleaned up on 6.15-rc1 :)


Takashi