RE: [PATCH v8] ALSA: hda/tas2781: Add tas2781 hda SPI driver

From: Simon Trimmer
Date: Tue Jun 18 2024 - 07:46:44 EST


> From: Baojun Xu <baojun.xu@xxxxxx>
> Sent: Friday, June 14, 2024 5:06 AM
> +static int tas2781_hda_bind(struct device *dev, struct device *master,
> + void *master_data)
> +{
> + struct tas2781_hda *tas_hda = dev_get_drvdata(dev);
> + struct hda_component *comps = master_data;
> + struct hda_codec *codec;
> + int ret;
> +
> + if (tas_hda->priv->index < 0 ||
> + tas_hda->priv->index >= HDA_MAX_COMPONENTS ||
> + (!comps))
> + return -EINVAL;
> +
> + comps = &comps[tas_hda->priv->index];
> + if (comps->dev)
> + return -EBUSY;
> +
> + codec = comps->codec;
> +
> + pm_runtime_get_sync(dev);
> +
> + comps->dev = dev;
> +
> + strscpy(comps->name, dev_name(dev), sizeof(comps->name));
> +
> + ret = tascodec_spi_init(tas_hda->priv, codec, THIS_MODULE,
> + tasdev_fw_ready);
> + if (!ret)
> + comps->playback_hook = tas2781_hda_playback_hook;
> +
...
> +
> +static void tas2781_hda_unbind(struct device *dev, struct device *master,
> + void *master_data)
> +{
> + struct tas2781_hda *tas_hda = dev_get_drvdata(dev);
> + struct hda_component *comps = master_data;
> +
> + comps = &comps[tas_hda->priv->index];
> +
> + if (comps->dev == dev) {
> + comps->dev = NULL;
> + memset(comps->name, 0, sizeof(comps->name));
> + comps->playback_hook = NULL;
> + }
> +

I'd been making sure you're on CC but in addition to the other review
comments I wanted to highlight that this in for-next will affect this patch
and require similar changes to these in the i2c driver:

https://lore.kernel.org/alsa-devel/87le327f29.wl-tiwai@xxxxxxx/T/#Z2e.:..:20
240617154105.108635-3-simont::40opensource.cirrus.com:1sound:pci:hda:tas2781
_hda_i2c.c

-Simon