This is a port of my NetBSD Allwinner A10/A20 DMA controller driver.
Details
- Reviewers
- None
- Group Reviewers
ARM - Commits
- rS295635: Add support for the Allwinner DMA controller. This will be used by the at
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Why are you adding both drivers in one review? I would expect the audio driver could depend on DMA, but not the reverse.
sys/arm/allwinner/a10_clk.c | ||
---|---|---|
274–275 ↗ | (On Diff #12642) | What are these magic numbers? A comment explaining them would be useful. |
279 ↗ | (On Diff #12642) | More magic |
sys/arm/allwinner/a10_codec.c | ||
62 ↗ | (On Diff #12642) | #define<tab>FOO |
177–178 ↗ | (On Diff #12642) | Why do you need these? You can use bus_read_* and bus_write_* with the res. |
sys/arm/allwinner/a10_dmac.c | ||
81–82 ↗ | (On Diff #12642) | Are these needed when you could use bus_read_* and bus_write_* |
I can split the changes into separate reviews, not a problem.
sys/arm/allwinner/a10_clk.c | ||
---|---|---|
274–275 ↗ | (On Diff #12642) | Sure. PLL2 output is ref * n / prediv / postdiv. With ref clock of 24MHz (always the case), 24MHZ * n / 21 / 4 gives us an output close to the desired frequency. For 24.576MHz target, n=86 gives us 24.571MHz and for 22.5792MHz target, n=79 gives us 22.571MHz. I'll add some comments to explain better. |
sys/arm/allwinner/a10_codec.c | ||
260 ↗ | (On Diff #12642) | Correct, the hardware doesn't offer per-channel volume controls. |
267–268 ↗ | (On Diff #12642) | I couldn't find documentation on this interface but existing drivers in dev/sound do this as well. |
sys/arm/allwinner/a10_dmac.c | ||
203 ↗ | (On Diff #12642) | This converts a bit in a status register into an index in either the ndma or ddma channels array. The register is encoded as two bits per channel (lowest bit is half transfer pending, highest bit is end transfer pending). The 8 normal DMA channel status are in bits 15-0 and the 8 dedicated DMA channel status are in bits 31-16. I'll add a comment to clarify what's going on here. |
sys/arm/allwinner/a10_codec.c | ||
---|---|---|
267–268 ↗ | (On Diff #12642) | you can grep MIXER_SET in kernel and see how it is used |
Reducing scope of this diff to the DMA controller driver only. Changes based on previous feedback.
sys/arm/allwinner/a10_dmac.c | ||
---|---|---|
434–439 ↗ | (On Diff #12658) | Called from the audio driver, and possibly any other device driver that needs to use the DMA controller. |
sys/boot/fdt/dts/arm/sun7i-a20.dtsi | ||
160 ↗ | (On Diff #12660) | The compatible string "allwinner,sun4i-a10-dma" matches the one used in the vendor DTS. |
sys/arm/allwinner/a10_dmac.c | ||
---|---|---|
397–400 ↗ | (On Diff #12660) | What are these magic numbers? |
sys/arm/allwinner/a10_dmac.c | ||
---|---|---|
397–400 ↗ | (On Diff #12660) | Oops, these should actually be set by a10dmac_set_config (this path is not exercised by the A10 codec driver as it uses an NDMA channel). Thanks, I'll fix that and update the patch. |