snd_hdspe(4): Add sysctl settings for clock source.

[dev_submerge.ch]

Summary

Setups with digital audio connections like SPDIF and ADAT require a
designated master clock to stay in sync. Add a sysctl setting to control
the preferred clock source for each HDSPe sound card. Complement this by
sysctl values to list available clock sources, show the currently
effective clock source and display the sync status of all connections.
Clock sources are named according to RME user manuals.

[dev_submerge.ch]

This is part of the improvements developed on github as an alternative kernel module for HDSPe sound cards. I built that separately as a local port / package. As such these changes have been tested extensively with a HDSPe RayDAT card, and are in use for several months now.

Information on the hardware register values can be found in the well-structured linux snd-hdspe driver, with hdspe_raio.c being relevant here.

The following patch was built and briefly tested on 15.0-CURRENT amd64, again with a RayDAT card, I don't own an HDSPe AIO card.

Testing

• Set the clock_preference to internal (master) and playback to an AD/DA converter via ADAT.
• Set the clock_preference to a digital input (ADAT) and do a recording with the AD/DA converter as master clock.
• Check the values of clock_source and sync_status in both setups, and with ADAT connected to a different port.

[br]

great work, just minor things

sys/dev/sound/pci/hdspe.c

311

I'd do this for easier reading, but this is not enforced by style(9) -- it allows initialization in declaration I think

struct sc_info *sc;

sc = oidp->oid_arg1;

317

may be later to rename AIO into HDSPE_AIO, and have HDSPE_RAYDAT etc. (less confusion)

338

Per style(9) we need to agree on usage (or not usage) of braces in single line statements (other parts of driver is not using them)

415

Per style(9) "Values in return statements should be enclosed in parentheses."

[dev_submerge.ch]

Fix minor readability and style(9) issues.

Unless I missed something, this should resolve all issues:

• Initialization in declaration is common in snd_uaudio(4), but not here. Fixed.
• Renamed hardware type macros to HDPSE_AIO, HDSPE_RAYDAT.
• Lots of braces removed around if / else conditional single line statements.
• Return values in parentheses.

@br: If all is ok, I'd be glad if you could commit this. I don't have commit bits.
Thank you!

[br]

Looks good. but I noticed these things just before committing

sys/dev/sound/pci/hdspe.c

330

what is magic number 15? may be some macro is needed for that.

sys/dev/sound/pci/hdspe.h

129

does CLOCK_MASK include master bit here? a bit of confusion.

may be it should be like this

/* Clock sources */
#define	HDSPE_SETTING_MASTER		(1 << 0)
#define	HDSPE_SETTING_CLOCK_SHIFT	(1)
#define	HDSPE_SETTING_CLOCK_MASK	(0x0f << HDSPE_SETTING_CLOCK_SHIFT)
#define	HDSPE_SETTING_CLOCK(m, n)	((((n) << HDSPE_SETTING_CLOCK_SHIFT) &\
					HDSPE_SETTING_CLOCK_MASK) |\
					((m) & HDSPE_SETTING_MASTER))

#define	HDSPE_STATUS1_CLOCK_SHIFT	(28)
#define	HDSPE_STATUS1_CLOCK_MASK	(0x0f << HDSPE_STATUS1_CLOCK_SHIFT)
#define	HDSPE_STATUS1_CLOCK(n)		(((n) << HDSPE_STATUS1_CLOCK_SHIFT) &\
					HDSPE_STATUS1_CLOCK_MASK)

132

note there should be tab after "#define" not space

[dev_submerge.ch]

I'll fix up the patches tomorrow, see inline comments.

sys/dev/sound/pci/hdspe.c

330

what is magic number 15? may be some macro is needed for that.

It's just the status value of the internal entry in the clock source table. I don't want to create macros for arbitrary values from that table, but I could look up that value in the table instead?

sys/dev/sound/pci/hdspe.h

129

does CLOCK_MASK include master bit here? a bit of confusion.

CLOCK_MASK does include the master bit. It's one setting, the values are not independent. Have a look at the hdspe_clock_source tables in hdspe.c. The only reason to split it in two is to expose the enumeration logic in that table. If you prefer, we could compose the value directly in the table initializer with 0 << 1 | 1 up to 10 << 1 | 0?

Also style(9) talks about macro functions with side effects, but for simple cases like this upper or lower case seems to be arbitrary in the sources. Any reason for that?

132

note there should be tab after "#define" not space

Missed that. Frankly it's the least obvious use of tabs I've seen in my whole career ;-)

[dev_submerge.ch]

Changelog:

• Should be all tabs now between #define and macros.
• Table lookup instead of magic number 15 for clock_source sysctl.
• Moved clock source setting register value creation to tables in hdspe.c.
• Simplified and cleaned up clock source related macros.
• Removed unused and invalid HDSPM_CLOCK_MODE_MASTER macro.