From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH v6 1/3] ALSA: hdac_ext: add extended HDA bus Date: Mon, 08 Jun 2015 11:00:28 +0200 Message-ID: References: <1433411602-5444-1-git-send-email-vinod.koul@intel.com> <1433411602-5444-2-git-send-email-vinod.koul@intel.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 48B822605B2 for ; Mon, 8 Jun 2015 11:00:31 +0200 (CEST) In-Reply-To: <1433411602-5444-2-git-send-email-vinod.koul@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Vinod Koul Cc: liam.r.girdwood@linux.intel.com, patches.audio@intel.com, alsa-devel@alsa-project.org, broonie@kernel.org, Jeeja KP List-Id: alsa-devel@alsa-project.org At Thu, 4 Jun 2015 15:23:20 +0530, Vinod Koul wrote: > > The new HDA controllers from Intel support new capabilities like multilink, > pipe processing, SPIB, GTS etc In order to use them we create an extended HDA > bus which embed the hdac bus and contains the fields for extended > configurations > > Signed-off-by: Jeeja KP > Signed-off-by: Vinod Koul > --- > include/sound/hdaudio_ext.h | 93 ++++++++++++++++++++++++++++++++++ > sound/hda/Kconfig | 4 ++ > sound/hda/Makefile | 3 ++ > sound/hda/ext/Makefile | 3 ++ > sound/hda/ext/hdac_ext_bus.c | 115 +++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 218 insertions(+) > create mode 100644 include/sound/hdaudio_ext.h > create mode 100644 sound/hda/ext/Makefile > create mode 100644 sound/hda/ext/hdac_ext_bus.c > > diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h > new file mode 100644 > index 000000000000..df1f8ae64693 > --- /dev/null > +++ b/include/sound/hdaudio_ext.h > @@ -0,0 +1,93 @@ > +#ifndef __SOUND_HDAUDIO_EXT_H > +#define __SOUND_HDAUDIO_EXT_H > + > +#include > +#include Do we need this header? > +#include > +#include > + > +/** > + * hdac_ext_bus: HDAC extended bus for extended HDA caps > + * > + * @bus: hdac bus > + * @num_streams: streams supported > + * @ppcap: HDA ext post processing caps supported > + * @spbcap: HDA ext Software Position in Buffer cap support > + * @mlcap: HDA ext MultiLink cap support > + * @gtscap: HDA ext Global Time Sync cap support > + * @ppcap_addr: pp capabilities pointer > + * @spbcap_addr: SPIB capabilities pointer > + * @mlcap_addr: MultiLink capabilities pointer > + * @gtscap_addr: gts capabilities pointer > + * @hlink_list: link list of HDA links > + */ > +struct hdac_ext_bus { > + struct hdac_bus bus; > + int num_streams; > + > + bool ppcap:1; > + bool spbcap:1; > + bool mlcap:1; > + bool gtscap:1; > + > + void __iomem *ppcap_addr; > + void __iomem *spbcap_addr; > + void __iomem *mlcap_addr; > + void __iomem *gtscap_addr; > + > + struct list_head hlink_list; > +}; > + > +int snd_hdac_ext_bus_init(struct hdac_ext_bus *sbus, struct device *dev, > + const struct hdac_bus_ops *ops, > + const struct hdac_io_ops *io_ops); > + > +void snd_hdac_ext_bus_exit(struct hdac_ext_bus *sbus); > +int snd_hdac_ext_bus_device_init(struct hdac_ext_bus *sbus, int addr); > +void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev); > + > +#define hdac_bus(sbus) (&(sbus)->bus) This macro name is a bit too ambiguous. If it were really a generic cast, I wouldn't mind. But it's specific to this type. > +#define bus_to_hdac_ext_bus(_bus) \ > + container_of(_bus, struct hdac_ext_bus, bus) > + > +int snd_hdac_ext_bus_parse_capabilities(struct hdac_ext_bus *sbus); > +void snd_hdac_ext_bus_ppcap_enable(struct hdac_ext_bus *chip, bool enable); > +void snd_hdac_ext_bus_ppcap_int_enable(struct hdac_ext_bus *chip, bool enable); > + > +/* > + * macros for ppcap register read/write > + */ > +#define _snd_hdac_ext_bus_ppcap_write(type, dev, reg, value) \ > + ((dev)->bus.io_ops->reg_write ## type(value, (dev)->ppcap_addr + (reg))) > +#define _snd_hdac_ext_bus_ppcap_read(type, dev, reg) \ > + ((dev)->bus.io_ops->reg_read ## type((dev)->ppcap_addr + (reg))) > + > +/* read/write a register, pass without AZX_REG_ prefix */ > +#define snd_hdac_ext_bus_ppcap_writel(dev, reg, value) \ > + _snd_hdac_ext_bus_ppcap_write(l, dev, AZX_REG_ ## reg, value) > +#define snd_hdac_ext_bus_ppcap_writew(dev, reg, value) \ > + _snd_hdac_ext_bus_ppcap_write(w, dev, AZX_REG_ ## reg, value) > +#define snd_hdac_ext_bus_ppcap_writeb(dev, reg, value) \ > + _snd_hdac_ext_bus_ppcap_write(b, dev, AZX_REG_ ## reg, value) > +#define snd_hdac_ext_bus_ppcap_readl(dev, reg) \ > + _snd_hdac_ext_bus_ppcap_read(l, dev, AZX_REG_ ## reg) > +#define snd_hdac_ext_bus_ppcap_readw(dev, reg) \ > + _snd_hdac_ext_bus_ppcap_read(w, dev, AZX_REG_ ## reg) > +#define snd_hdac_ext_bus_ppcap_readb(dev, reg) \ > + _snd_hdac_ext_bus_ppcap_read(b, dev, AZX_REG_ ## reg) > + > +/* update a register, pass without AZX_REG_ prefix */ > +#define snd_hdac_ext_bus_ppcap_updatel(dev, reg, mask, val) \ > + snd_hdac_ext_bus_ppcap_writel(dev, reg, \ > + (snd_hdac_ext_bus_ppcap_readl(dev, reg) & \ > + ~(mask)) | (val)) > +#define snd_hdac_ext_bus_ppcap_updatew(dev, reg, mask, val) \ > + snd_hdac_ext_bus_ppcap_writew(dev, reg, \ > + (snd_hdac_ext_bus_ppcap_readw(dev, reg) & \ > + ~(mask)) | (val)) > +#define snd_hdac_ext_bus_ppcap_updateb(dev, reg, mask, val) \ > + snd_hdac_ext_bus_ppcap_writeb(dev, reg, \ > + (snd_hdac_ext_bus_ppcap_readb(dev, reg) & \ > + ~(mask)) | (val)) It's not necessarily good to wrap all with such macros. For azx_write*(), I kept them as is for reducing the amount of useless code rewrites. But for new codes, I don't think it's always worth... thanks, Takashi > +#endif /* __SOUND_HDAUDIO_EXT_H */ > diff --git a/sound/hda/Kconfig b/sound/hda/Kconfig > index ac5ffac2a272..6dc3914fd28b 100644 > --- a/sound/hda/Kconfig > +++ b/sound/hda/Kconfig > @@ -10,3 +10,7 @@ config SND_HDA_I915 > default y > depends on DRM_I915 > depends on SND_HDA_CORE > + > +config SND_HDA_EXT_CORE > + tristate > + select SND_HDA_CORE > diff --git a/sound/hda/Makefile b/sound/hda/Makefile > index 55dd465c7042..7e999c995cdc 100644 > --- a/sound/hda/Makefile > +++ b/sound/hda/Makefile > @@ -8,3 +8,6 @@ CFLAGS_trace.o := -I$(src) > snd-hda-core-$(CONFIG_SND_HDA_I915) += hdac_i915.o > > obj-$(CONFIG_SND_HDA_CORE) += snd-hda-core.o > + > +#extended hda > +obj-$(CONFIG_SND_HDA_EXT_CORE) += ext/ > diff --git a/sound/hda/ext/Makefile b/sound/hda/ext/Makefile > new file mode 100644 > index 000000000000..2e583e664916 > --- /dev/null > +++ b/sound/hda/ext/Makefile > @@ -0,0 +1,3 @@ > +snd-hda-ext-core-objs := hdac_ext_bus.o > + > +obj-$(CONFIG_SND_HDA_EXT_CORE) += snd-hda-ext-core.o > diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c > new file mode 100644 > index 000000000000..b5dbf33be72d > --- /dev/null > +++ b/sound/hda/ext/hdac_ext_bus.c > @@ -0,0 +1,115 @@ > +/* > + * hdac-ext-bus.c - HD-audio extended core bus functions. > + * > + * Copyright (C) 2014-2015 Intel Corp > + * Author: Jeeja KP > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; version 2 of the License. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/** > + * snd_hdac_ext_bus_init - initialize a HD-audio extended bus > + * @sbus: the pointer to extended bus object > + * @dev: device pointer > + * @ops: bus verb operators > + * @io_ops: lowlevel I/O operators > + * > + * Returns 0 if successful, or a negative error code. > + */ > +int snd_hdac_ext_bus_init(struct hdac_ext_bus *sbus, struct device *dev, > + const struct hdac_bus_ops *ops, > + const struct hdac_io_ops *io_ops) > +{ > + int ret; > + > + ret = snd_hdac_bus_init(&sbus->bus, dev, ops, io_ops); > + if (ret < 0) > + return ret; > + > + INIT_LIST_HEAD(&sbus->hlink_list); > + return 0; > +} > +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_init); > + > +/** > + * snd_hdac_ext_bus_exit - clean up a HD-audio extended bus > + * @sbus: the pointer to extended bus object > + */ > +void snd_hdac_ext_bus_exit(struct hdac_ext_bus *sbus) > +{ > + snd_hdac_bus_exit(&sbus->bus); > + WARN_ON(!list_empty(&sbus->hlink_list)); > +} > +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_exit); > + > +static void default_release(struct device *dev) > +{ > + snd_hdac_ext_bus_device_exit(container_of(dev, struct hdac_device, dev)); > +} > + > +/** > + * snd_hdac_ext_device_init - initialize the HDA extended codec base device > + * @sbus: hdac extended bus to attach to > + * @addr: codec address > + * > + * Returns zero for success or a negative error code. > + */ > +int snd_hdac_ext_bus_device_init(struct hdac_ext_bus *sbus, int addr) > +{ > + struct hdac_device *hdev = NULL; > + struct hdac_bus *bus = hdac_bus(sbus); > + char name[15]; > + int ret; > + > + hdev = kzalloc(sizeof(*hdev), GFP_KERNEL); > + if (!hdev) > + return -ENOMEM; > + > + snprintf(name, sizeof(name), "hda-codec#%03x", addr); > + > + ret = snd_hdac_device_init(hdev, bus, name, addr); > + if (ret < 0) { > + dev_err(bus->dev, "device init failed for hdac device\n"); > + return ret; > + } > + hdev->type = HDA_DEV_ASOC; > + hdev->dev.release = default_release; > + > + ret = snd_hdac_device_register(hdev); > + if (ret) { > + dev_err(bus->dev, "failed to register hdac device\n"); > + snd_hdac_ext_bus_device_exit(hdev); > + return ret; > + } > + return 0; > +} > +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_init); > + > +/** > + * snd_hdac_ext_bus_device_exit - clean up a HD-audio extended codec base device > + * @hdev: hdac device to clean up > + */ > +void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev) > +{ > + snd_hdac_device_exit(hdev); > + kfree(hdev); > +} > +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_exit); > -- > 1.9.1 >