On Mon, Feb 17, 2020 at 05:44:36PM +0800, Chen-Yu Tsai wrote:
On Mon, Feb 17, 2020 at 5:14 PM Maxime Ripard maxime@cerno.tech wrote:
Hi,
On Mon, Feb 17, 2020 at 12:42:16AM -0600, Samuel Holland wrote:
The sun8i-codec driver, as used in the Allwinner A33 and A64, currently only exposes a small subset of the available hardware features. In order to use the A64 in a smartphone (the PinePhone), I've added the necessary functionality to the driver:
- The full set of supported DAI format options
- Support for AIF2 and AIF3
- Additional routing knobs
- Additional volume controls
Unfortunately, due to preexisting issues with the driver, there are some breaking changes, as explained further in the commit messages:
- The LRCK inversion issue means we need a new compatible for the A64.
- Some controls are named inaccurately, so they are renamed.
- Likewise, the DAPM widgets used in device trees were either named wrong, or the device trees were using the wrong widgets in the first place. (Specifically, the links between the analog codec and digital codec happen at the ADC and DAC, not AIF1.)
I tended to take the philosophy of "while I'm breaking things, I might as well do them right", so I've probably made a few more changes than absolutely necessary. I'm not sure about where all of the policy boundaries are, about how far I should go to maintain compatibility. For example, for the DT widget usage, I could:
- Rename everything and update the DTS files (which is what I did)
- Keep the old (misleading/wrong) name for the widgets, but repurpose them to work correctly (i.e. "ADC Left" would be named "AIF1 Slot 0 Left ADC", but it would work just like "ADC Left" does in this patchset)
- Keep the old widgets around as a compatibility layer, but add new widgets and update the in-tree DTS files to use them (i.e. "ADC Left" would have a path from "AIF1 Slot 0 Left ADC", but "AIF1 Slot 0 Left ADC" would be a no-op widget)
- Something else entirely
I'm not sure this is really a concern here. We need to maintain the compatibility with old DT's, but those will have an A33 compatible too, and as far as I can see, you're not changing anything for that compatible, so we're in the clear?
If not, then the third option would probably be the best, especially since it's only a couple of them.
Unfortunately the description for both chips are shared, and they're wrong. So we probably need a new compatible (or a new driver)... or like options 2 or 3, keep the DT visible endpoints (but deprecate them), and route them to a new set of proper widgets.
And hmm, it might be a bit wild, but since it's basically just a sed on a string in DT, can't we leverage the dynamic DT stuff to rewrite the property if we find the old one at probe? That would keep the driver clean.
Maxime