On 12.12.2024 1:47 AM, Alexey Klimov wrote:
> This is required in order to introduce wsa881x driver that works
> in analog mode and is configurable via i2c only.
> Functional changes, if any, are kept to be minimal and common
> parts or parts that can be shared are moved into wsa881x-common
> helper driver.
> The regmap config structure now contains 0x3000 offset as required
> by soundwire spec.
>
> While at this, also fix the typo in WSA881X_ADC_EN_SEL_IBIAS
> register name and rename wsa881x_set_sdw_stream() to
> wsa881x_set_stream().
>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> Signed-off-by: Alexey Klimov <alexey.klimov@xxxxxxxxxx>
> ---
[...]
> +
> +int wsa881x_digital_mute(struct snd_soc_dai *dai, int mute, int stream)
> +{
> + struct snd_soc_component *component = dai->component;
> +
> + if (mute)
> + snd_soc_component_update_bits(component,
> + WSA881X_SPKR_DRV_EN, 0x80, 0x00);
> + else
> + snd_soc_component_update_bits(component,
> + WSA881X_SPKR_DRV_EN, 0x80, 0x80);
mute ? 0x00 : BIT(7)
you can even return it for good measure
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(wsa881x_digital_mute);
> +
> +void wsa881x_init_common(struct wsa881x_priv *wsa881x)
> +{
> + struct regmap *rm = wsa881x->regmap;
> + unsigned int val = 0;
> +
> + /* Bring out of analog reset */
> + regmap_update_bits(rm, WSA881X_CDC_RST_CTL, 0x02, 0x02);
> +
> + /* Bring out of digital reset */
> + regmap_update_bits(rm, WSA881X_CDC_RST_CTL, 0x01, 0x01);
> + regmap_update_bits(rm, WSA881X_CLOCK_CONFIG, 0x10, 0x10);
> + regmap_update_bits(rm, WSA881X_SPKR_OCP_CTL, 0x02, 0x02);
> + regmap_update_bits(rm, WSA881X_SPKR_MISC_CTL1, 0xC0, 0x80);
> + regmap_update_bits(rm, WSA881X_SPKR_MISC_CTL1, 0x06, 0x06);
> + regmap_update_bits(rm, WSA881X_SPKR_BIAS_INT, 0xFF, 0x00);
> + regmap_update_bits(rm, WSA881X_SPKR_PA_INT, 0xF0, 0x40);
> + regmap_update_bits(rm, WSA881X_SPKR_PA_INT, 0x0E, 0x0E);
> + regmap_update_bits(rm, WSA881X_BOOST_LOOP_STABILITY, 0x03, 0x03);
> + regmap_update_bits(rm, WSA881X_BOOST_MISC2_CTL, 0xFF, 0x14);
> + regmap_update_bits(rm, WSA881X_BOOST_START_CTL, 0x80, 0x80);
> + regmap_update_bits(rm, WSA881X_BOOST_START_CTL, 0x03, 0x00);
> + regmap_update_bits(rm, WSA881X_BOOST_SLOPE_COMP_ISENSE_FB, 0x0C, 0x04);
> + regmap_update_bits(rm, WSA881X_BOOST_SLOPE_COMP_ISENSE_FB, 0x03, 0x00);
All these could use some #defines..
> +
> + regmap_read(rm, WSA881X_OTP_REG_0, &val);
> + if (val)
> + regmap_update_bits(rm, WSA881X_BOOST_PRESET_OUT1, 0xF0, 0x70);
And this, a comment..
> +
> + regmap_update_bits(rm, WSA881X_BOOST_PRESET_OUT2, 0xF0, 0x30);
> + regmap_update_bits(rm, WSA881X_SPKR_DRV_EN, 0x08, 0x08);
> + regmap_update_bits(rm, WSA881X_BOOST_CURRENT_LIMIT, 0x0F, 0x08);
> + regmap_update_bits(rm, WSA881X_SPKR_OCP_CTL, 0x30, 0x30);
> + regmap_update_bits(rm, WSA881X_SPKR_OCP_CTL, 0x0C, 0x00);
> + regmap_update_bits(rm, WSA881X_OTP_REG_28, 0x3F, 0x3A);
> + regmap_update_bits(rm, WSA881X_BONGO_RESRV_REG1, 0xFF, 0xB2);
> + regmap_update_bits(rm, WSA881X_BONGO_RESRV_REG2, 0xFF, 0x05);
> +}
> +EXPORT_SYMBOL_GPL(wsa881x_init_common);
> +
> +int wsa881x_probe_common(struct wsa881x_priv **wsa881x, struct device *dev)
> +{
> + struct wsa881x_priv *wsa;
> +
> + wsa = devm_kzalloc(dev, sizeof(*wsa), GFP_KERNEL);
> + if (!wsa)
> + return -ENOMEM;
> +
> + wsa->dev = dev;
> + wsa->sd_n = devm_gpiod_get_optional(dev, "powerdown",
> + GPIOD_FLAGS_BIT_NONEXCLUSIVE);
> + if (IS_ERR(wsa->sd_n))
> + return dev_err_probe(dev, PTR_ERR(wsa->sd_n),
> + "Shutdown Control GPIO not found\n");
> + /*
> + * Backwards compatibility work-around.
> + *
> + * The SD_N GPIO is active low, however upstream DTS used always active
> + * high. Changing the flag in driver and DTS will break backwards
> + * compatibility, so add a simple value inversion to work with both old
> + * and new DTS.
> + *
> + * This won't work properly with DTS using the flags properly in cases:
> + * 1. Old DTS with proper ACTIVE_LOW, however such case was broken
> + * before as the driver required the active high.
> + * 2. New DTS with proper ACTIVE_HIGH (intended), which is rare case
> + * (not existing upstream) but possible. This is the price of
> + * backwards compatibility, therefore this hack should be removed at
> + * some point.
> + */
> + wsa->sd_n_val = gpiod_is_active_low(wsa->sd_n);
> + if (!wsa->sd_n_val)
> + dev_warn(dev,
> + "Using ACTIVE_HIGH for shutdown GPIO. Your DTB might be outdated or you use unsupported configuration for the GPIO.");
> +
> + dev_set_drvdata(dev, wsa);
> + gpiod_direction_output(wsa->sd_n, !wsa->sd_n_val);
> +
> + *wsa881x = wsa;
> +
> + return 0;
There's no usage of wsa881x, so you can drop this dance
Konrad
