On Wed, May 27, 2020 at 10:57 PM Mark Brown broonie@kernel.org wrote:
On Wed, May 27, 2020 at 09:34:13PM +0200, Arnd Bergmann wrote:
index 000000000000..c891aa80757f --- /dev/null +++ b/sound/soc/codecs/rt5682-i2c.c @@ -0,0 +1,341 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*
- rt5682.c -- RT5682 ALSA SoC audio component driver
Please keep the entire comment C++ so things look more intentional.
That's the kind of change I was trying to avoid by just moving things around, but I've done that now in both files.
+static const char *rt5682_supply_names[RT5682_NUM_SUPPLIES] = {
"AVDD","MICVDD","VBAT",+};
I'm *fairly* sure the device needs power even with Soundwire?
I have no idea how this is done with ACPI. I'm moving it back as an exported symbol. There should probably be some abstraction that handles this in the common part of the driver along with some of the other bits of rt5682_i2c_probe, but really don't want to shake things up too much and would let this be done by whoever needs to add DT support to the soundwire driver in the future and is able to test the changes.
+static void rt5682_jd_check_handler(struct work_struct *work) +{
struct rt5682_priv *rt5682 = container_of(work, struct rt5682_priv,jd_check_work.work);+static int rt5682_parse_dt(struct rt5682_priv *rt5682, struct device *dev) +{
device_property_read_u32(dev, "realtek,dmic1-data-pin",&rt5682->pdata.dmic1_data_pin);device_property_read_u32(dev, "realtek,dmic1-clk-pin",&rt5682->pdata.dmic1_clk_pin);This doesn't look very I2C specific either, nor do chunks of the rest of the code. The usual pattern with this stuff is to have the bus specific code do bus specific stuff like setting up the regmap and then call into a common init function for the shared parts of the chip. I'd expect a bit more unshared code here but not this much.
Right, I was surprised the soundwire portion does not tie into device tree at all, and none of the other soundwire codecs seem to either and no dts files reference it, though there is some code for the subsystem and a binding.
Arnd