Skip to content

bcm270x: add dtparam to enable onboard sound device #981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 27, 2015

Conversation

HiassofT
Copy link
Contributor

PR977 has the sideeffect of snd-bcm2835 being enabled by default.

This PR changes this back to the previous default and adds a device tree parameter
so snd-bcm2835 can be enabled like the other devices with "dtparam=audio=on".

@popcornmix
Copy link
Collaborator

ping @notro @pelwell

@notro
Copy link
Contributor

notro commented May 24, 2015

The sound driver loads by default on Raspian, so the default behaviour has not changed.
But I see one difference: Removing 'snd-bcm2835' from /etc/modules (probably) does not stop the sound driver from loading anymore.
Previously the device aliases were not coded in the driver, but now the DT compatible is, so Linux is able to find the matching module/driver even if the module is not loaded.
Try blacklisting 'snd-bcm2835' to verify this.

The problem with this PR is that it changes the default behaviour by having onboard audio disabled by default.
I suggest having it enabled, but use the audio parameter you added to disable it: audio=off.
But lets hear what @pelwell has to say.

@HiassofT
Copy link
Contributor Author

HiassofT commented May 24, 2015 via email

@pelwell
Copy link
Contributor

pelwell commented May 24, 2015

Switching to blacklisting (opt-out) from /etc/modules (opt-in) runs counter to the "blacklisting bad, device tree good" message. But I also want to preserve the existing enabled-by-default behaviour.

I vote for:

  1. adding the dtparam
  2. leaving the audio node enabled by default
  3. adding "audio=on" to future default Raspbian config.txts, and either telling users to hand edit their existing configs or patch it as part of a future update.
  4. (Optional) Changing the status of the audio node to "disabled" in the DTB at a later date, if it still bothers us.

@popcornmix
Copy link
Collaborator

Report of HifiBerry breakage in OpenELEC after #977
http://forum.kodi.tv/showthread.php?tid=224025&pid=2012283#pid2012283

I think the issue is that snd-bcm2835 wasn't enabled by firmware/kernel in the past, but raspbian enabled it (in /etc/modules). OpenELEC/OSMC deliberately didn't enable it in /etc/modules, so their default behaviour has changed.

@notro
Copy link
Contributor

notro commented May 25, 2015

This will give back the old behaviour (no module autoloading):

diff --git a/sound/arm/bcm2835.c b/sound/arm/bcm2835.c
index 6b545e7..d78cb38 100644
--- a/sound/arm/bcm2835.c
+++ b/sound/arm/bcm2835.c
@@ -312,7 +312,6 @@ static const struct of_device_id snd_bcm2835_of_match_table[] = {
    { .compatible = "brcm,bcm2835-audio", },
    {},
 };
-MODULE_DEVICE_TABLE(of, snd_bcm2835_of_match_table);

 static struct platform_driver bcm2835_alsa0_driver = {
    .probe = snd_bcm2835_alsa_probe,

But I don't recommend it as the driver will not behave as one would expect.

Ideally it's the Device Tree that should decide which devices are present/wanted.

The audio node is enabled by default and can be disabled
with dtparam=audio=off in config.txt
@HiassofT
Copy link
Contributor Author

HiassofT commented May 26, 2015 via email

pelwell added a commit that referenced this pull request May 27, 2015
bcm270x: add dtparam to enable/disable onboard sound device
@pelwell pelwell merged commit cc96dc4 into raspberrypi:rpi-4.0.y May 27, 2015
@popcornmix
Copy link
Collaborator

Further discussions with @pelwell and we've taken a compromise step:
1e1ac98

This should means that either snd-bcm2835 in /etc/modules or dtparam=audio=on in config.txt results in working audio. If both are missing then snd-bcm2835 will not be loaded.

The concern is that OpenELEC/OSMC/other platforms don't expect snd-bcm2835 to be loaded by default and don't behave well. Expecting all users to add dtparam=audio=off to resolve this will create a lot of support issues.

This will be temporary. We'll ensure future raspbian images ship with dtparam=audio=on in config.txt and will look into adding this as part of the apt-get upgrade procedure.

When we are happy dtparam=audio=on is commonplace, we'll revert the platform part of this commit (leaving audio disabled in default DT).

@HiassofT HiassofT deleted the dt-audio branch May 28, 2015 16:04
@HiassofT
Copy link
Contributor Author

HiassofT commented May 28, 2015 via email

pfpacket pushed a commit to pfpacket/linux-rpi-rust that referenced this pull request Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants