TAS5805M codec driver improvement

[anabolyc]

The goal of this PR is to provide a set of essential updates to the TAS5805M DAC (datasheet)

[X] Provide clearer register names and values
[X] Make DSP configuration binary optional (for those who do not have TI PPC3 application access)
[X] Fix digital volume control bug: It is currently possible to set volume value before DSP control sequence is sent, which is not allowed according to the datasheet (7.5.3.1 Startup Procedures)
[X] Add more ALSA hardware controls:
[X] DAC bridge mode (2 channel or 1 channel with double power),
[X] modulation mode (BD, 1SPW, Hybrid, affects power consumption and THD),
[X] analog gain (affects driver output voltage swing, should be selected based on the DAC VCC)
[X] read and dump fault details if any are detected
[X] reset faults if present
[X] Eq enable switch (for upcoming equilizer functionality)
[O] Mexer controls (coming in a different PR)
[O] EQ controls (coming in a different PR)

All changes are tested on Louder Raspberry Pi Hat by Sonocotta

A bit of background:

I am a hardware developer, a person behind Sonocotta audio boards, created for both Raspberry and ESP32. I created few Raspi Hats using TAS5805M DAC (and planning to move forward to TAS5825M model). I originally used TI-provided DAC driver code with my boards, pulled from their forums. I was happy to see that it was pulled into kernel source, however by that time I already started using mode advanced DAC features wih ESP32 and started to bake them into my custom driver code. At this point I know a lot about this DAC and have quite an extensive experience with this DAC. I see few bugs and logic errors were baked into TI version of the driver, leaving aside how poor subset of DAC functions were implemented. So my goal is to fix bugs and errors first, to unblock the road to implement the most cool features of the TAS5805M DSP, starting few a few simple but important settings affecting power consumption and performance, and moving forward to enable 15 band equilizer, mixer controls, and few other great features of this DAC.

I hope to drop my custom driver, as soon as all changes move to the raspi kernel source. I hope to see that as a part of the standard raspi image in the end, as I naturally want every personusing my Hats to be able to set them up without a need to build anything

I'm open to critique and ready to absorb every valuable piece of commentary.

[anabolyc]

Current state

[pelwell]

As far as I know, there is nothing Pi-specific about the TAS5805M or your changes to its driver. We are reluctant to accept random patches into our downstream branches unless it is Pi-specific, or is considered useful to Pi users but rejected upstream for some reason.

You could try sending the patches to the maintainers of the "sound" subsystem:

Liam Girdwood <lgirdwood@gmail.com> (supporter:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...)
Mark Brown <broonie@kernel.org> (supporter:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...)
Jaroslav Kysela <perex@perex.cz> (maintainer:SOUND)
Takashi Iwai <tiwai@suse.com> (maintainer:SOUND)
linux-sound@vger.kernel.org (open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...)

However, they are likely to reject you moving the #defines into a header (and the addition of the TAS5805M_ prefix which serves no obvious purpose here except to increase the code size. And that's just the first commit.

[6by9]

However, they are likely to reject you moving the #defines into a header (and the addition of the TAS5805M_ prefix which serves no obvious purpose here except to increase the code size. And that's just the first commit.

I'd looked at it and thought exactly the same.
The general upstream preference is that if the defines are only used by one file, then keep them in that file.

You've got a #define DEBUG 1 left at the start of the driver which isn't going to be accepted.

The patch commit texts are very vague as to what they're doing, and the subject lines don't follow the kernel contribution guidelines for prefixes (should be ASoC: tas5805m: <patch does this> in this case) or having a Signed-off-by: line.
If you'd run checkpatch.pl against the patches then it would have told you of the main places where the kernel coding style and contribution requirements hadn't been followed. If a patch sent to the mailing lists generates checkpatch warnings or errors, then expect a fairly terse response back.

[anabolyc]

Thanks for the feedback. I have a lot to learn. I will prepare a patch for the upstream code. Have no idea how the mail distributions work; I need to figure it out. I'm only familiar with GitHub workflows.

[anabolyc]

Would it make sense to keep multiple commits (fixing the commit messages), or squashed commit is a better approach?

[6by9]

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separate-your-changes

Separate each logical change into a separate patch.

For example, if your changes include both bug fixes and performance enhancements for a single driver, separate those changes into two or more patches. If your changes include an API update, and a new driver which uses that new API, separate those into two patches.

Contributing patches upstream is a bit of a learning curve.

If you want to update this PR with your patches once tidied up, then one of us should be able to do a cursory review of them for any obvious issues. None of us are experts in the sound subsystem though.

Also note that upstream patches need to be based on the latest kernel (https://github.com/torvalds/linux/), or even better is the relevant tree for the subsystem (https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for the sound one).
This driver appears to have little churn, so initially basing off our rpi-6.18.y branch would probably work (please be aware that there are minor issues we are aware of on that branch. Dropping back to rpi-6.17.y may be necessary).

I can highly recommend b4 for actually sending patches to the mailing lists. https://b4.docs.kernel.org/en/latest/contributor/prep.html
It sorts out all the relevant recipients, ensures you've run checkpatch, and just generally takes some of the legwork out of the process.