Re: [RFC PATCH 2/7] ALSA: ac97: add an ac97 bus

[Takashi Iwai <tiwai@suse.de>]

On Sat, 30 Apr 2016 23:15:34 +0200,
Robert Jarzmik wrote:
> 
> diff --git a/include/sound/ac97/codec.h b/include/sound/ac97/codec.h
> new file mode 100644
> index 000000000000..4b8b3e570892
> --- /dev/null
> +++ b/include/sound/ac97/codec.h
> @@ -0,0 +1,98 @@
> +/*
> + *  Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef AC97_CODEC_H
> +#define AC97_CODEC_H

Let's be careful about the choice of the guard.


> +
> +#include <linux/device.h>
> +
> +#define AC97_ID(vendor_id1, vendor_id2) \
> +	(((vendor_id1 & 0xffff) << 16) | (vendor_id2 & 0xffff))
> +#define AC97_DRIVER_ID(vendor_id1, vendor_id2, mask_id1, mask_id2, _data) \
> +	{ .id = ((vendor_id1 & 0xffff) << 16) | (vendor_id2 & 0xffff), \
> +	  .mask = ((mask_id1 & 0xffff) << 16) | (mask_id2 & 0xffff), \
> +	  .data = _data }

Give parentheses around the macro arguments.


> +
> +#define to_ac97_device(d) container_of(d, struct ac97_codec_device, dev)
> +#define to_ac97_driver(d) container_of(d, struct ac97_codec_driver, driver)
> +
> +struct ac97_controller;
> +
> +/**
> + * struct ac97_id - matches a codec device and driver on an ac97 bus
> + * @id: The significant bits if the codec vendor ID1 and ID2
> + * @mask: Bitmask specifying which bits of the id field are significant when
> + *	  matching. A driver binds to a device when :
> + *        ((vendorID1 << 8 | vendorID2) & (mask_id1 << 8 | mask_id2)) == id.
> + * @data: Private data used by the driver.
> + */
> +struct ac97_id {
> +	unsigned int		id;
> +	unsigned int		mask;
> +	void			*data;
> +};
> +
> +/**
> + * ac97_codec_device - a ac97 codec
> + * @dev: the code device
> + * @vendor_id: the vendor_id of the codec, as sensed on the AC-link
> + * @num: the codec number, 0 is primary, 1 is first slave, etc ...
> + * @ac97_ctrl: ac97 digital controller on the same AC-link
> + *
> + * This is the device instanciated for each codec living on a AC-link. There are
> + * normally 0 to 4 codec devices per AC-link, and all of them are controlled by
> + * an AC97 digital controller.
> + */
> +struct ac97_codec_device {
> +	struct device		dev;	/* Must stay first member */

This doesn't have to be the first element as long as you use container_of().


> +	unsigned int		vendor_id;
> +	unsigned int		num;
> +	struct list_head	list;
> +	struct ac97_controller	*ac97_ctrl;
> +};
> +
> +/**
> + * ac97_codec_driver - a ac97 codec driver
> + * @driver: the device driver structure
> + * @probe: the function called when a ac97_codec_device is matched
> + * @remove: the function called when the device is unbound/removed
> + * @suspend: suspend function (might be NULL)
> + * @resume: resume function (might be NULL)
> + * @shutdown: shutdown function (might be NULL)
> + * @id_table: ac97 vendor_id match table, { } member terminated
> + */
> +struct ac97_codec_driver {
> +	struct device_driver	driver;
> +	int			(*probe)(struct ac97_codec_device *);
> +	int			(*remove)(struct ac97_codec_device *);
> +	int			(*suspend)(struct ac97_codec_device *);
> +	int			(*resume)(struct ac97_codec_device *);
> +	void			(*shutdown)(struct ac97_codec_device *);
> +	struct ac97_id		*id_table;

Missing const?

> +};
> +
> +int ac97_codec_driver_register(struct ac97_codec_driver *drv);
> +void ac97_codec_driver_unregister(struct ac97_codec_driver *drv);
> +
> +static inline struct device *
> +ac97_codec_dev2dev(const struct ac97_codec_device *adev)
> +{
> +	return (struct device *)(adev);

What's wrong with the simple &adev->dev ?  Cast looks scary.

> +}
> +
> +static inline void *ac97_get_drvdata(const struct ac97_codec_device *adev)
> +{
> +	return dev_get_drvdata(ac97_codec_dev2dev(adev));
> +}
> +
> +static inline void ac97_set_drvdata(const struct ac97_codec_device *adev,
> +				    void *data)
> +{
> +	dev_set_drvdata(ac97_codec_dev2dev(adev), data);
> +}
> +
> +#endif
> diff --git a/include/sound/ac97/compat.h b/include/sound/ac97/compat.h
> new file mode 100644
> index 000000000000..bf611f572f2d
> --- /dev/null
> +++ b/include/sound/ac97/compat.h
> @@ -0,0 +1,21 @@
> +/*
> + *  Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This file is for backward compatibility with snd_ac97 structure and its
> + * multiple usages, such as the snd_ac97_bus and snd_ac97_build_ops.
> + *
> + */
> +#ifndef AC97_COMPAT_H
> +#define AC97_COMPAT_H
> +
> +#include <sound/ac97_codec.h>
> +#include <sound/soc.h>
> +
> +struct snd_ac97 *compat_alloc_snd_ac97_codec(struct snd_soc_codec *codec);
> +void compat_release_snd_ac97_codec(struct snd_ac97 *ac97);
> +
> +#endif
> diff --git a/include/sound/ac97/controller.h b/include/sound/ac97/controller.h
> new file mode 100644
> index 000000000000..f1e5e645f5ef
> --- /dev/null
> +++ b/include/sound/ac97/controller.h
> @@ -0,0 +1,39 @@
> +/*
> + *  Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef AC97_CONTROLLER_H
> +#define AC97_CONTROLLER_H
> +
> +#include <linux/list.h>
> +
> +struct device;
> +struct ac97_codec_device;
> +
> +struct ac97_controller_ops {
> +	void (*reset)(struct ac97_codec_device *ac97);
> +	void (*warm_reset)(struct ac97_codec_device *ac97);
> +	int (*write)(struct ac97_codec_device *ac97, unsigned short reg,
> +		     unsigned short val);
> +	int (*read)(struct ac97_codec_device *ac97, unsigned short reg);
> +	void (*wait)(struct ac97_codec_device *ac97);
> +	void (*init)(struct ac97_codec_device *ac97);
> +};
> +
> +struct ac97_controller {
> +	const struct ac97_controller_ops *ops;
> +	struct list_head controllers;
> +	struct device *dev;
> +	int bus_idx;

What is this bus_idx for?


> +	int bound_codecs;
> +	struct list_head codecs;
> +};
> +
> +int ac97_digital_controller_register(const struct ac97_controller_ops *ops,
> +				     struct device *dev);
> +int ac97_digital_controller_unregister(const struct device *dev);
> +
> +#endif
> diff --git a/sound/ac97/Kconfig b/sound/ac97/Kconfig
> new file mode 100644
> index 000000000000..fd2c2d031e62
> --- /dev/null
> +++ b/sound/ac97/Kconfig
> @@ -0,0 +1,9 @@
> +#
> +# PCI configuration
> +#
 
Still only for PCI? :)


> +
> +config AC97
> +	bool "AC97 bus"
> +	help
> +	   Say Y here if you want to have AC97 devices, which are sound oriented
> +	   devices around an AC-Link.
> diff --git a/sound/ac97/Makefile b/sound/ac97/Makefile
> new file mode 100644
> index 000000000000..5575909d46e2
> --- /dev/null
> +++ b/sound/ac97/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# make for AC97 bus drivers
> +#
> +
> +obj-y	+= bus.o codec.o snd_ac97_compat.o

No possibility for modules?


> diff --git a/sound/ac97/ac97_core.h b/sound/ac97/ac97_core.h
> new file mode 100644
> index 000000000000..db6e27288357
> --- /dev/null
> +++ b/sound/ac97/ac97_core.h
> @@ -0,0 +1,10 @@
> +/*
> + * Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +unsigned int ac97_bus_scan_one(struct ac97_controller *ac97,
> +			       int codec_num);
> diff --git a/sound/ac97/bus.c b/sound/ac97/bus.c
> new file mode 100644
> index 000000000000..f9bf5632d4aa
> --- /dev/null
> +++ b/sound/ac97/bus.c
> @@ -0,0 +1,330 @@
> +/*
> + * Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +#include <linux/pm.h>
> +#include <linux/slab.h>
> +#include <sound/ac97/codec.h>
> +#include <sound/ac97/controller.h>
> +#include <sound/ac97/regs.h>
> +
> +#define AC97_BUS_MAX_CODECS 4
> +
> +/*
> + * Protects ac97_controllers and each ac97_controller structure.
> + */
> +static DEFINE_MUTEX(ac97_controllers_mutex);
> +static LIST_HEAD(ac97_controllers);
> +static int ac97_bus_idx;
> +
> +struct bus_type ac97_bus_type;
> +
> +static struct ac97_codec_device *
> +ac97_codec_find(struct ac97_controller *ac97_ctrl, int codec_num)
> +{
> +	struct ac97_codec_device *codec;
> +
> +	list_for_each_entry(codec, &ac97_ctrl->codecs, list)
> +		if (codec->num == codec_num)
> +			return codec;
> +
> +	return NULL;
> +}

It's a question whether we need to manage the codecs in the linked
list.  There can be at most 4 codecs, so it fits in an array well,
too.  Then some codes like this would be simpler. (And it'll even
reduce the footprint, too.)


> +
> +static void ac97_codec_release(struct device *dev)
> +{
> +	struct ac97_codec_device *codec;
> +
> +	codec = container_of(dev, struct ac97_codec_device, dev);
> +	list_del(&codec->list);
> +	kfree(codec);
> +}
> +
> +static int ac97_codec_add(struct ac97_controller *ac97_ctrl, int idx,
> +		   unsigned int vendor_id)
> +{
> +	struct ac97_codec_device *codec;
> +	char *codec_name;
> +	int ret;
> +
> +	codec = kzalloc(sizeof(*codec), GFP_KERNEL);
> +	if (!codec)
> +		return -ENOMEM;
> +
> +	codec->vendor_id = vendor_id;
> +	codec->dev.release = ac97_codec_release;
> +	codec->dev.bus = &ac97_bus_type;
> +	codec->dev.parent = ac97_ctrl->dev;
> +	codec->num = idx;
> +	codec->ac97_ctrl = ac97_ctrl;
> +	INIT_LIST_HEAD(&codec->list);
> +	list_move_tail(&codec->list, &ac97_ctrl->codecs);
> +
> +	codec_name = kasprintf(GFP_KERNEL, "%s:%d", dev_name(ac97_ctrl->dev),
> +			       idx);
> +	codec->dev.init_name = codec_name;
> +
> +	ret = device_register(&codec->dev);
> +	kfree(codec_name);
> +
> +	return ret;
> +}
> +
> +unsigned int ac97_bus_scan_one(struct ac97_controller *ac97,
> +				      int codec_num)
> +{
> +	struct ac97_codec_device codec;
> +	unsigned short vid1, vid2;
> +	int ret;
> +
> +	codec.dev = *ac97->dev;
> +	codec.num = codec_num;
> +	ret = ac97->ops->read(&codec, AC97_VENDOR_ID1);
> +	vid1 = (ret & 0xffff);
> +	if (ret < 0)
> +		return 0;

Hmm.  This looks pretty hackish and dangerous.



> +	ret = ac97->ops->read(&codec, AC97_VENDOR_ID2);
> +	vid2 = (ret & 0xffff);
> +	if (ret < 0)
> +		return 0;
> +
> +	dev_dbg(&codec.dev, "%s(codec_num=%d): vendor_id=0x%08x\n",
> +		__func__, codec_num, AC97_ID(vid1, vid2));
> +	return AC97_ID(vid1, vid2);
> +}
> +
> +static int ac97_bus_scan(struct ac97_controller *ac97_ctrl)
> +{
> +	int ret, i;
> +	unsigned int vendor_id;
> +
> +	for (i = 0; i < AC97_BUS_MAX_CODECS; i++) {
> +		if (ac97_codec_find(ac97_ctrl, i))
> +			continue;
> +		vendor_id = ac97_bus_scan_one(ac97_ctrl, i);
> +		if (!vendor_id)
> +			continue;
> +
> +		ret = ac97_codec_add(ac97_ctrl, i, vendor_id);
> +		if (ret < 0)
> +			return ret;

This is one of concerns: we don't know whether the device really
reacts well if you access to a non-existing slot.  At least, it'd be
safer to have the masks for the devices we already know the slots.


> +	}
> +	return 0;
> +}
> +
> +void ac97_rescan_all_controllers(void)
> +{
> +	struct ac97_controller *ac97_ctrl;
> +	int ret;
> +
> +	mutex_lock(&ac97_controllers_mutex);
> +	list_for_each_entry(ac97_ctrl, &ac97_controllers, controllers) {
> +		ret = ac97_bus_scan(ac97_ctrl);
> +		if (ret)
> +			dev_warn(ac97_ctrl->dev, "scan failed: %d\n", ret);
> +	}
> +	mutex_unlock(&ac97_controllers_mutex);
> +}
> +EXPORT_SYMBOL(ac97_rescan_all_controllers);
> +
> +static int ac97_bus_reset(struct ac97_controller *ac97_ctrl)
> +{
> +	struct ac97_codec_device codec;
> +
> +	memset(&codec, 0, sizeof(codec));
> +	codec.dev = *ac97_ctrl->dev;
> +
> +	ac97_ctrl->ops->reset(&codec);

So, this assumes that reset ops is mandatory?  Then document it at
least.


thanks,

Takashi

> +	return 0;
> +}
> +
> +/**
> + * ac97_codec_driver_register - register an AC97 codec driver
> + * @dev: AC97 driver codec to register
> + *
> + * Register an AC97 codec driver to the ac97 bus driver, aka. the AC97 digital
> + * controller.
> + *
> + * Returns 0 on success or error code
> + */
> +int ac97_codec_driver_register(struct ac97_codec_driver *drv)
> +{
> +	int ret;
> +
> +	drv->driver.bus = &ac97_bus_type;
> +
> +	ret = driver_register(&drv->driver);
> +	if (!ret)
> +		ac97_rescan_all_controllers();
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(ac97_codec_driver_register);
> +
> +/**
> + * ac97_codec_driver_unregister - unregister an AC97 codec driver
> + * @dev: AC97 codec driver to unregister
> + *
> + * Unregister a previously registered ac97 codec driver.
> + */
> +void ac97_codec_driver_unregister(struct ac97_codec_driver *drv)
> +{
> +	driver_unregister(&drv->driver);
> +}
> +EXPORT_SYMBOL(ac97_codec_driver_unregister);
> +
> +static int ac97_dc_codecs_unregister(struct ac97_controller *ac97_ctrl)
> +{
> +	struct ac97_codec_device *codec, *tmp;
> +
> +	list_for_each_entry_safe(codec, tmp, &ac97_ctrl->codecs, list)
> +		put_device(&codec->dev);
> +
> +	return 0;
> +}
> +
> +/**
> + * ac97_digital_controller_register - register an ac97 controller
> + * @ops: the ac97 bus operations
> + * @dev: the device providing the ac97 DC function
> + *
> + * Register a digital controller which can control up to 4 ac97 codecs. This is
> + * the controller side of the AC97 AC-link, while the slave side are the codecs.
> + *
> + * Returns a positive bus index upon success, negative value upon error
> + */
> +int ac97_digital_controller_register(const struct ac97_controller_ops *ops,
> +				     struct device *dev)
> +{
> +	struct ac97_controller *ac97_ctrl;
> +
> +	ac97_ctrl = kzalloc(sizeof(*ac97_ctrl), GFP_KERNEL);
> +	if (!ac97_ctrl)
> +		return -ENOMEM;
> +
> +	mutex_lock(&ac97_controllers_mutex);
> +	ac97_ctrl->ops = ops;
> +	ac97_ctrl->bus_idx = ac97_bus_idx++;
> +	ac97_ctrl->dev = dev;
> +	INIT_LIST_HEAD(&ac97_ctrl->codecs);
> +	list_add(&ac97_ctrl->controllers, &ac97_controllers);
> +	mutex_unlock(&ac97_controllers_mutex);
> +
> +	ac97_bus_reset(ac97_ctrl);
> +	ac97_bus_scan(ac97_ctrl);
> +
> +	return ac97_ctrl->bus_idx;
> +}
> +EXPORT_SYMBOL(ac97_digital_controller_register);
> +
> +/**
> + * ac97_digital_controller_unregister - unregister an ac97 controller
> + * @dev: the device previously provided to ac97_digital_controller_register()
> + *
> + * Returns 0 on success, negative upon error
> + */
> +int ac97_digital_controller_unregister(const struct device *dev)
> +{
> +	struct ac97_controller *ac97_ctrl, *tmp;
> +	int ret = -ENODEV;
> +
> +	mutex_lock(&ac97_controllers_mutex);
> +	list_for_each_entry_safe(ac97_ctrl, tmp, &ac97_controllers,
> +				 controllers) {
> +		if (ac97_ctrl->dev != dev)
> +			continue;
> +		if (ac97_ctrl->bound_codecs)
> +			ret = -EBUSY;
> +		else
> +			ret = ac97_dc_codecs_unregister(ac97_ctrl);
> +		if (!ret)
> +			list_del(&ac97_ctrl->controllers);
> +	}
> +
> +	mutex_unlock(&ac97_controllers_mutex);
> +	return ret;
> +}
> +EXPORT_SYMBOL(ac97_digital_controller_unregister);
> +
> +static const struct dev_pm_ops ac97_pm = {
> +	.suspend	= pm_generic_suspend,
> +	.resume		= pm_generic_resume,
> +	.freeze		= pm_generic_freeze,
> +	.thaw		= pm_generic_thaw,
> +	.poweroff	= pm_generic_poweroff,
> +	.restore	= pm_generic_restore,
> +};
> +
> +static ssize_t vendor_id_show(struct device *dev,
> +			      struct device_attribute *attr, char *buf)
> +{
> +	struct ac97_codec_device *codec = to_ac97_device(dev);
> +
> +	return sprintf(buf, "%08x", codec->vendor_id);
> +}
> +
> +static struct device_attribute ac97_dev_attrs[] = {
> +	__ATTR_RO(vendor_id),
> +	__ATTR_NULL,
> +};
> +
> +int ac97_bus_match(struct device *dev, struct device_driver *drv)
> +{
> +	struct ac97_codec_device *adev = to_ac97_device(dev);
> +	struct ac97_codec_driver *adrv = to_ac97_driver(drv);
> +	struct ac97_id *id = adrv->id_table;
> +
> +	if (adev->vendor_id == 0x0 || adev->vendor_id == 0xffffffff)
> +		return false;
> +
> +	do {
> +		if ((id->id & id->mask) == (adev->vendor_id & id->mask))
> +			return true;
> +	} while (++id->id);
> +
> +	return false;
> +}
> +
> +static int ac97_bus_probe(struct device *dev)
> +{
> +	struct ac97_codec_device *adev = to_ac97_device(dev);
> +	struct ac97_codec_driver *adrv = to_ac97_driver(dev->driver);
> +
> +	return adrv->probe(adev);
> +}
> +
> +static int ac97_bus_remove(struct device *dev)
> +{
> +	struct ac97_codec_device *adev = to_ac97_device(dev);
> +	struct ac97_codec_driver *adrv = to_ac97_driver(dev->driver);
> +
> +	return adrv->remove(adev);
> +}
> +
> +struct bus_type ac97_bus_type = {
> +	.name		= "ac97",
> +	.dev_attrs	= ac97_dev_attrs,
> +	.match		= ac97_bus_match,
> +	.pm		= &ac97_pm,
> +	.probe		= ac97_bus_probe,
> +	.remove		= ac97_bus_remove,
> +};
> +EXPORT_SYMBOL(ac97_bus_type);
> +
> +static int __init ac97_bus_init(void)
> +{
> +	return bus_register(&ac97_bus_type);
> +}
> +subsys_initcall(ac97_bus_init);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Robert Jarzmik <robert.jarzmik@free.fr>");
> diff --git a/sound/ac97/codec.c b/sound/ac97/codec.c
> new file mode 100644
> index 000000000000..a835f03744bf
> --- /dev/null
> +++ b/sound/ac97/codec.c
> @@ -0,0 +1,15 @@
> +/*
> + *  Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <sound/ac97_codec.h>
> +#include <sound/ac97/codec.h>
> +#include <sound/ac97/controller.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <sound/soc.h>	/* For compat_ac97_* */
> +
> diff --git a/sound/ac97/snd_ac97_compat.c b/sound/ac97/snd_ac97_compat.c
> new file mode 100644
> index 000000000000..7e2f01c96fc9
> --- /dev/null
> +++ b/sound/ac97/snd_ac97_compat.c
> @@ -0,0 +1,104 @@
> +/*
> + *  Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +#include <sound/ac97/codec.h>
> +#include <sound/ac97/controller.h>
> +#include <sound/soc.h>
> +
> +#include "ac97_core.h"
> +
> +static void compat_ac97_reset(struct snd_ac97 *ac97)
> +{
> +	struct ac97_codec_device *adev = to_ac97_device(ac97->private_data);
> +	struct ac97_controller *actrl = adev->ac97_ctrl;
> +
> +	if (actrl->ops->reset)
> +		actrl->ops->reset(adev);
> +}
> +
> +static void compat_ac97_warm_reset(struct snd_ac97 *ac97)
> +{
> +	struct ac97_codec_device *adev = to_ac97_device(ac97->private_data);
> +	struct ac97_controller *actrl = adev->ac97_ctrl;
> +
> +	if (actrl->ops->warm_reset)
> +		actrl->ops->warm_reset(adev);
> +}
> +
> +static void compat_ac97_write(struct snd_ac97 *ac97, unsigned short reg,
> +			      unsigned short val)
> +{
> +	struct ac97_codec_device *adev = to_ac97_device(ac97->private_data);
> +	struct ac97_controller *actrl = adev->ac97_ctrl;
> +
> +	actrl->ops->write(adev, reg, val);
> +}
> +
> +static unsigned short compat_ac97_read(struct snd_ac97 *ac97,
> +				       unsigned short reg)
> +{
> +	struct ac97_codec_device *adev = to_ac97_device(ac97->private_data);
> +	struct ac97_controller *actrl = adev->ac97_ctrl;
> +
> +	return actrl->ops->read(adev, reg);
> +}
> +
> +static struct snd_ac97_bus_ops compat_snd_ac97_bus_ops = {
> +	.reset = compat_ac97_reset,
> +	.warm_reset = compat_ac97_warm_reset,
> +	.write = compat_ac97_write,
> +	.read = compat_ac97_read,
> +};
> +
> +static struct snd_ac97_bus compat_soc_ac97_bus = {
> +	.ops = &compat_snd_ac97_bus_ops,
> +};
> +
> +struct snd_ac97 *compat_alloc_snd_ac97_codec(struct snd_soc_codec *codec)
> +{
> +	struct snd_ac97 *ac97;
> +
> +	ac97 = kzalloc(sizeof(struct snd_ac97), GFP_KERNEL);
> +	if (ac97 == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	ac97->dev = *codec->dev;
> +	ac97->private_data = codec->dev;
> +	ac97->bus = &compat_soc_ac97_bus;
> +	return ac97;
> +}
> +EXPORT_SYMBOL_GPL(compat_alloc_snd_ac97_codec);
> +
> +void compat_release_snd_ac97_codec(struct snd_ac97 *ac97)
> +{
> +	kfree(ac97);
> +}
> +EXPORT_SYMBOL_GPL(compat_release_snd_ac97_codec);
> +
> +int snd_ac97_reset(struct snd_ac97 *ac97, bool try_warm, unsigned int id,
> +	unsigned int id_mask)
> +{
> +	struct ac97_codec_device *adev = to_ac97_device(ac97->private_data);
> +	struct ac97_controller *actrl = adev->ac97_ctrl;
> +
> +	if (try_warm) {
> +		compat_ac97_warm_reset(ac97);
> +		if (ac97_bus_scan_one(actrl, adev->num) == adev->vendor_id)
> +			return 1;
> +	}
> +
> +	compat_ac97_reset(ac97);
> +	compat_ac97_warm_reset(ac97);
> +	if (ac97_bus_scan_one(actrl, adev->num) == adev->vendor_id)
> +		return 0;
> +
> +	return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(snd_ac97_reset);
> -- 
> 2.1.4
> 
>