Messages in this thread | | | Date | Wed, 29 Jun 2022 10:03:02 -0500 | Subject | Re: [PATCH 2/4] ASoC: codecs: add wsa883x amplifier support | From | Pierre-Louis Bossart <> |
| |
> +/* 4 ports */ > +static struct sdw_dpn_prop wsa_sink_dpn_prop[WSA883X_MAX_SWR_PORTS] = { > + { > + /* DAC */ > + .num = 1, > + .type = SDW_DPN_SIMPLE, > + .min_ch = 1, > + .max_ch = 1, > + .simple_ch_prep_sm = true, > + .read_only_wordlength = true, > + }, {
nit-pick: it's unusual to see such opening brackets for structure initialization, usually there are on a new line.
> + /* COMP */ > + .num = 2, > + .type = SDW_DPN_SIMPLE, > + .min_ch = 1, > + .max_ch = 1, > + .simple_ch_prep_sm = true, > + .read_only_wordlength = true, > + }, { > + /* BOOST */ > + .num = 3, > + .type = SDW_DPN_SIMPLE, > + .min_ch = 1, > + .max_ch = 1, > + .simple_ch_prep_sm = true, > + .read_only_wordlength = true, > + }, { > + /* VISENSE */ > + .num = 4, > + .type = SDW_DPN_SIMPLE, > + .min_ch = 1, > + .max_ch = 1, > + .simple_ch_prep_sm = true, > + .read_only_wordlength = true, > + } > +};
> +static int wsa883x_update_status(struct sdw_slave *slave, > + enum sdw_slave_status status) > +{ > + struct wsa883x_priv *wsa883x = dev_get_drvdata(&slave->dev); > + > + if (status == SDW_SLAVE_ATTACHED && slave->dev_num > 0)
do you actually need to test if slave->dev_num is > 0?
if I look at drivers/soundwire/bus.c, update_status cannot really be invoked with dev_num == 0.
> + wsa883x_init(wsa883x); > + > + return 0; > +} > +
> +static int __maybe_unused wsa883x_runtime_resume(struct device *dev) > +{ > + struct sdw_slave *slave = dev_to_sdw_dev(dev); > + struct regmap *regmap = dev_get_regmap(dev, NULL); > + struct wsa883x_priv *wsa883x = dev_get_drvdata(dev); > + int ret; > + > + ret = regulator_enable(wsa883x->vdd); > + if (ret) { > + dev_err(dev, "Failed to enable vdd regulator (%d)\n", ret); > + return ret; > + } > + > + gpiod_direction_output(wsa883x->sd_n, 1); > + > + wait_for_completion_timeout(&slave->initialization_complete, > + msecs_to_jiffies(WSA883X_PROBE_TIMEOUT));
check for success? You don't want to enable regmap sync below if the device never successfully attached and initialized.
> + > + usleep_range(20000, 20010); > + regcache_cache_only(regmap, false); > + regcache_sync(regmap); > + > + return 0; > +}
| |