On Thu, May 15, 2025 at 10:49:13AM -0500, Daniel Dadap wrote:
> On Thu, May 15, 2025 at 04:54:25PM +0200, Takashi Iwai wrote:
> > On Thu, 15 May 2025 16:45:52 +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@xxxxxxxxxx>
> > >
> > > 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)
> >
> > Also, at the next time, please put the maintainer (me) to Cc.
> >
>
> Sure, already sent it with you on To: as a reply to the last message. I was
> wrong about hda_tegra having __maybe_unused, you already cleaned that up.
Oops, sorry for the churn, I did not notice the difference between the
SYSTEM_SLEEP_PM_OPS() and SET_SYSTEM_SLEEP_PM_OPS() macros. I think I got
it right, now.
>
> >
> > thanks,
> >
> > Takashi
