Re: [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions

[Takashi Iwai <tiwai@suse.de>]

On Mon, 18 Oct 2021 11:07:15 +0200,
Cezary Rojewski wrote:
> 
> 
> 
> On 2021-10-18 10:25 AM, Takashi Iwai wrote:
> > On Mon, 18 Oct 2021 10:08:30 +0200,
> > Cezary Rojewski wrote:
> >>
> >> On 2021-10-17 9:52 AM, Takashi Iwai wrote:
> >>> On Fri, 15 Oct 2021 18:42:33 +0200,
> >>> Pierre-Louis Bossart wrote:
> >>>> On 10/15/21 11:40 AM, Cezary Rojewski wrote:
> >>>>> From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> >>>>>
> >>>>> ACPI is device independent, so printing warnings using device functions
> >>>>> is misleading. Replace dev_xxx() with pr_xxx() and remove now
> >>>>> unnecessary argument.
> >>>>
> >>>> the routines in sound/hda/intel-nhtl.c are called from a specific PCI
> >>>> device, why would you remove that information?
> >>>>
> >>>> This makes no sense to me.
> >>>
> >>> Right, otherwise this change would confuse user, too; they'll be
> >>> clueless about who triggers it.
> >>>
> >>> It's OK to change to pr_*(), but then it should have more information
> >>> that can be easily identified and understood what user should do.
> >>
> >> Isn't the answer as to 'who' used it obvious, though? NHLT is used for
> >> I2S and DMIC endpoints only, so the question is 'limited' in the first
> >> place. And then, you cannot have several Intel ADSP drivers running
> >> simultaneously.
> >
> > Well, it's not about you or devs -- those must know which driver is
> > relevant very well, of course.  Instead, the problem is for *all*
> > others who read the message.
> >
> > IOW, which user would know and think "hey, it's a NHLT thingy that
> > must be ASoC xxx driver that spewed" only from the text snippet "NHLT
> > table not found"?  That's way too much expectation.  Some more
> > guidance is needed in the error message.  The dev_*() variant gave at
> > least the device names that can help guessing the relevant driver
> > easily.
> >
> >> Also, logs found ACPI-table interface are device-less so this patch
> >> makes NHLT interface look more ACPI-generic alike.
> >
> > The conversion itself is no problem, but the lost information is the
> > problem.
> 
> Well, most invocations that end in error will cause stack of
> error-logs (dev_xxx()) in the parent driver. NHLT reads:
> Non-HDAudio-Link-Table. The name itself points reader towards
> sound/hda already.

But then how dropping the device information would improve things at
all?  It rather cuts off the link between that message and the later
one.

> I understand that not everyone will google "NHLT"
> and such but then there are ton of kernel messages which won't guide
> reader by the hand anyway.

This can't be an excuse, we don't have to follow that anti pattern :)

> "NHLT table not found" is configuration problem, not a driver
> problem. Mixing BIOS/config problems with driver ones leads to
> incorrect conclusions what we want to avoid.

The configuration itself makes sense only if it's in actual use,
i.e. the problem is always tied with the driver.

So, if any, you can put more information around the message mentioning
that it's the configuration problem and what to do for users,
e.g. check your BIOS, etc.  *That* would be the improvement.


Takashi