bcm2708-i2s - bug with commit? #687
Comments
|
@ghollingworth any comments? |
|
It does look that way... I'm not sure that is going to be a huge leak, but could be fixed... (although in my defence I didn't write it!) Gordon From: plughplover <[email protected]mailto:[email protected]> I am FAR from being knowledgeable about kernel programming, but doesn't this commit (If this is a dumb question, please be kind) Reply to this email directly or view it on GitHubhttps://github.com//issues/687. |
|
When it was called from the probe routine, it only got executed once; moving it to hw_params makes it an issue. I interpret the text associated with the commit to mean that having the bcm2708-i2s module automagically loaded at system boot (and hence changing the pin config) is a problem for someone using those pins for other purposes (understandable). Perhaps a better solution is to leave the pin init in the probe (hence no ongoing leak) but require the addition of the i2s module to, uh, /etc/modules (like snd-bcm2835) for it to be loaded? And / Or alternatively add it to /etc/modprobe.d/raspi-blacklist.conf like the i2c-bcm2708 and spi-bcm2708 modules are now? (not sure how those config files all play together). Or simply unload the i2s module... oops - a different bug... It doesn't restore the pin config on unload! Bad driver! ;) Hey Koala, you obviously know all the Linux driver rules about using the regmap routines, is there a 'right' way for an Alsa driver to do that pin config (and unconfig)? |
|
The reason it was changed is because the alternate settings shouldn't be changed unless you are actually using the driver... if you move it to the probe function then it does... In future we'll be using the proper pinctrl driver so this is just a sticking plaster / bandaid solution |
|
Yeah, that's what I figured /said above. How about:
then the pins only get tweaked when someone intentionally loads the module, Note that as things stand, i2s users need to edit /etc/modules anyway to add BTW, the 'pinctrl driver' sounds interesting - where can I read up on it? Thanks! |
|
Thanks popcornmix, I'll give it a read. FWIW, I know I'm 'new kid on the block' and am not trying to step on any toes, I'm just thinking about xbmc and other media player apps that use i2s DACs or spdif and start/stop streams a lot... |
|
I don't think the ioremap is required. We already map gpio mem as IO in bcm2708.c (see bcm2708_gpio.c which is free to directly access these registers without ioremap). I'll come up with a patch that removes the ioremap. |
|
See: c256eb9 This should have the same effect and avoids the ioremap. |
See: raspberrypi/linux#687 firmware: hdmi: Add gencmd for controlling small adjustments to hdmi pixel clock
See: raspberrypi/linux#687 firmware: hdmi: Add gencmd for controlling small adjustments to hdmi pixel clock
|
popcornmix, your patch is interesting. I've definitely got to look into that "mach/gpio.h" include file (and the pinctrl driver, too). I implicitly verified that the patch does not seem to break anything whilst testing #681. As far as I'm concerned this issue can be closed. |
See: raspberrypi/linux#687 firmware: hdmi: Add gencmd for controlling small adjustments to hdmi pixel clock
This patch fixes a problem where the AR8035 PHY can't be detected on an Cisco Meraki MR24, if the ethernet cable is not connected on boot. Russell Senior provided steps to reproduce the issue: |Disconnect ethernet cable, apply power, wait until device has booted, |plug in ethernet, check for interfaces, no eth0 is listed. | |This appears to be a problem during probing of the AR8035 Phy chip. |When ethernet has no link, the phy detection fails, and eth0 is not |created. Plugging ethernet later has no effect, because there is no |interface as far as the kernel is concerned. The relevant part of |the boot log looks like this: |this is the failing case: | |[ 0.876611] /plb/opb/emac-rgmii@ef601500: input 0 in RGMII mode |[ 0.882532] /plb/opb/ethernet@ef600c00: reset timeout |[ 0.888546] /plb/opb/ethernet@ef600c00: can't find PHY! |and the succeeding case: | |[ 0.876672] /plb/opb/emac-rgmii@ef601500: input 0 in RGMII mode |[ 0.883952] eth0: EMAC-0 /plb/opb/ethernet@ef600c00, MAC 00:01:.. |[ 0.890822] eth0: found Atheros 8035 Gigabit Ethernet PHY (0x01) Based on the comment and the commit message of commit 23fbb5a ("emac: Fix EMAC soft reset on 460EX/GT"). This is because the AR8035 PHY doesn't provide the TX Clock, if the ethernet cable is not attached. This causes the reset to timeout and the PHY detection code in emac_init_phy() is unable to detect the AR8035 PHY. As a result, the emac driver bails out early and the user left with no ethernet. In order to stay compatible with existing configurations, the driver tries the current reset approach at first. Only if the first attempt timed out, it does perform one more retry with the clock temporarily switched to the internal source for just the duration of the reset. LEDE-Bug: #687 <https://bugs.lede-project.org/index.php?do=details&task_id=687> Cc: Chris Blake <[email protected]> Reported-by: Russell Senior <[email protected]> Fixes: 23fbb5a ("emac: Fix EMAC soft reset on 460EX/GT") Signed-off-by: Christian Lamparter <[email protected]> Reviewed-by: Andrew Lunn <[email protected]> Signed-off-by: David S. Miller <[email protected]>
[ Upstream commit a1c6ca3 ] It is possible to observe hung_task complaints when system goes to suspend-to-idle state: # echo freeze > /sys/power/state PM: Syncing filesystems ... done. Freezing user space processes ... (elapsed 0.001 seconds) done. OOM killer disabled. Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done. sd 0:0:0:0: [sda] Synchronizing SCSI cache INFO: task bash:1569 blocked for more than 120 seconds. Not tainted 4.19.0-rc3_+ #687 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. bash D 0 1569 604 0x00000000 Call Trace: ? __schedule+0x1fe/0x7e0 schedule+0x28/0x80 suspend_devices_and_enter+0x4ac/0x750 pm_suspend+0x2c0/0x310 Register a PM notifier to disable the detector on suspend and re-enable back on wakeup. Signed-off-by: Vitaly Kuznetsov <[email protected]> Signed-off-by: Rafael J. Wysocki <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
Documentation cleanups
I am FAR from being knowledgeable about kernel programming, but doesn't this commit
c7de283
result in a, uh, address space leak? That is, every time a stream is initiated, doesn't this line
335 gpio = ioremap(GPIO_BASE, SZ_16K);
allocate a new page of address space to (re)map the gpio registers?
(If this is a dumb question, please be kind)
The text was updated successfully, but these errors were encountered: