On Fri, Aug 25, 2023 at 12:01:35AM +0300, Marian Postevca wrote:
+static int acp_asoc_suspend_pre(struct snd_soc_card *card) +{
- int ret;
- ret = acp_ops_suspend_pre(card);
- if (ret == 1)
return 0;- else
return ret;+}
+static int acp_asoc_resume_post(struct snd_soc_card *card) +{
- int ret;
- ret = acp_ops_resume_post(card);
- if (ret == 1)
return 0;- else
return ret;+}
This feels like it should've been part of the prior commit adding support for more complex cards?
- card->drvdata = (struct acp_card_drvdata *)pdev->id_entry->driver_data;
- acp_card_drvdata = card->drvdata;
- acp_card_drvdata->acpi_mach = (struct snd_soc_acpi_mach *)pdev->dev.platform_data;
Similarly these changes in probe() -
- {
.name = "acp3x-es83xx",.driver_data = (kernel_ulong_t)&es83xx_rn_data,- },
The main thing I'd expect to see in the generic code in a patch adding a specific driver is table entries like this and the ones you had earlier.
if (drv_data->hs_codec_id == ES83XX) {rc = acp_ops_configure_link(card, &links[i]);if (rc != 0) {dev_err(dev, "Failed to configure link for ES83XX: %d\n", rc);return rc;}}
This function should ideally have been using switch statemnts but that's not an issue your patch introduced.
+#define ES83XX_ENABLE_DMIC BIT(4) +#define ES83XX_48_MHZ_MCLK BIT(5)
+static void acp3x_es83xx_set_gpios_values(struct acp3x_es83xx_private *priv,
bool speaker, bool headphone)+{
- gpiod_set_value_cansleep(priv->gpio_speakers, speaker);
- gpiod_set_value_cansleep(priv->gpio_headphone, headphone);
+}
I'm not clear what this function buys us other than just calling gpiolib directly in the DAPM events?
Otherewise the actual machine driver looks good.