All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: Takashi Iwai <tiwai@suse.de>, linux-sound@vger.kernel.org
Subject: Re: [RFC] Reorganizing HD-audio driver code?
Date: Sun, 29 Jun 2025 11:22:25 +0200	[thread overview]
Message-ID: <87h5zygawe.wl-tiwai@suse.de> (raw)
In-Reply-To: <e6451c29-2ade-4b73-bfd7-f48bafccfa98@opensource.cirrus.com>

On Fri, 27 Jun 2025 15:22:20 +0200,
Richard Fitzgerald wrote:
> 
> On 27/06/2025 1:04 pm, Takashi Iwai wrote:
> > Hi,
> > 
> > HD-audio driver is known to be quite messy in both file structures and
> > its design, but until now I haven't touched its files paths so much
> > because I set a higher priority for the easiness of backport to stable
> > kernels.  But, you can't leave garbages forever, it's been already
> > high time for a large clean up.
> > 
> > So I tried a quick code reorganization, and put the result in
> > test/hda-reorg branch of sound.git tree.
> >    https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/log/?h=test/hda-reorg
> > 
> > The basic idea is to move the code from sound/pci/hda/* into different
> > subdirectories in sound/hda/ per functionality, as most of the stuff
> > are independent from PCI, but rather HD-audio bus specific.
> > 
> 
> This all seems reasonable to me. I always thought it strange that there
> is a sound/hda directory but most of the HDA support isn't in that
> directory.

Thanks for your review!

It's a long story: at the time of ASoC Intel HD-audio support, we
thought to implement more ASoC-friendly way of HD-audio controller and
codec drivers, e.g. adapting DAPM and others.  So we began with the
factoring out the HD-audio basic core stuff to the common directory
sound/hda/*, while keeping the rest legacy stuff almost as is.  But
ASoC implementation didn't fly in the end from various reasons, and
the legacy HD-audio stuff was good enough for the actual use cases;
it's too bit to fail, after all.

> > The Realtek codec is split further to smaller pieces (which was really
> > huge).
> 
> Yes, that file was very confusing having support and quirks for so many
> Realtek parts all in one file.
> 
> > The Cirrus and TI sub-codec drivers are moved to codecs/side-codecs
> > subdirectory:
> 
> That's ok
> 
> > They can be put to each own directory and drop the file name prefix,
> > if we want, too.  Let me know if Cirrus and TI people would like to
> > split to more subdirectories.
> 
> I don't mind either way.
> The hda_component* files are common to the amps and the realtek driver
> so I wonder whether they belong in helpers. They are only utility
> wrappers around the kernel component-binding APIs.

Right.  So far, just because it's basically only binding with
side-codecs, I put into side-codecs subdirectory.

> > *HOWEVER* the biggest question is: whether it's worth?
> > 
> > Essentially, this makes almost impossible to make a patch for stable
> > trees from the original commit as is; one has to translate the file
> > paths and adjust manually in each patch.
> 
> Depends which file you are patching and how much it has changed.
> Git can figure out file renames (and changing directory is a rename).

I'm afraid that the Realtek codec changes might be hard to track
automatically.  Maybe the Cirrus side-codecs stuff would work.

> > Also, of course, if anyone is working on HD-audio stuff right now, the
> > work had to be adjusted to the new file path.  It'd be one-off action,
> > though.
> 
> Speaking only for Cirrus, I don't think this makes much extra effort
> for us. Our amp drivers have only changed location, so that should be
> trivial. The other file we change is patch_realtek.c but that typically
> is only 1-line quirk entries.

OK, thanks for confirmation!


Takashi

  reply	other threads:[~2025-06-29  9:22 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-27 12:04 [RFC] Reorganizing HD-audio driver code? Takashi Iwai
2025-06-27 13:22 ` Richard Fitzgerald
2025-06-29  9:22   ` Takashi Iwai [this message]
2025-07-02 14:53     ` Takashi Iwai
2025-07-03 13:29       ` Amadeusz Sławiński
2025-07-03 13:38         ` Takashi Iwai
2025-07-03 13:57           ` Amadeusz Sławiński
2025-07-03 14:32             ` Takashi Iwai
2025-07-03 14:43               ` Takashi Iwai
2025-07-03 14:52                 ` Amadeusz Sławiński
2025-07-03 15:12                   ` Takashi Iwai
2025-07-08 10:56                     ` Amadeusz Sławiński
2025-07-08 11:15                       ` Takashi Iwai
2025-07-03 14:04     ` Richard Fitzgerald
2025-07-03 14:34       ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h5zygawe.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=linux-sound@vger.kernel.org \
    --cc=rf@opensource.cirrus.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.