| Message ID | 1449816267-11910-6-git-send-email-vinod.koul@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 11 Dec 2015 07:44:18 +0100, Vinod Koul wrote: > > From: Jayachandran B <jayachandran.b@intel.com> > > MISCBDCGE is a new register for Misc Backbone clock gate control > which is useful to control while resetting the link and ensuring > controller is in required state so add API to control it > > Signed-off-by: Jayachandran B <jayachandran.b@intel.com> > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > --- > include/sound/hda_register.h | 3 +++ > include/sound/hdaudio_ext.h | 1 + > sound/hda/ext/hdac_ext_controller.c | 23 +++++++++++++++++++++++ > 3 files changed, 27 insertions(+) > > diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h > index 28ac1f9a18ac..fa33237f4bd1 100644 > --- a/include/sound/hda_register.h > +++ b/include/sound/hda_register.h > @@ -96,6 +96,9 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 }; > /* PCI space */ > #define AZX_PCIREG_TCSEL 0x44 > > +#define AZX_PCIREG_CGCTL 0x48 > +#define AZX_CGCTL_MISCBDCGE_MASK (1 << 6) > + > /* > * other constants > */ > diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h > index f3454950ee0b..65961bbb8ca3 100644 > --- a/include/sound/hdaudio_ext.h > +++ b/include/sound/hdaudio_ext.h > @@ -152,6 +152,7 @@ void snd_hdac_ext_link_set_stream_id(struct hdac_ext_link *link, > int stream); > void snd_hdac_ext_link_clear_stream_id(struct hdac_ext_link *link, > int stream); > +void snd_hdac_ext_bus_enable_miscbdcge(struct device *dev, bool enable); > > /* update register macro */ > #define snd_hdac_updatel(addr, reg, mask, val) \ > diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c > index 556267e75591..8f1d292a522c 100644 > --- a/sound/hda/ext/hdac_ext_controller.c > +++ b/sound/hda/ext/hdac_ext_controller.c > @@ -21,6 +21,7 @@ > #include <linux/slab.h> > #include <sound/hda_register.h> > #include <sound/hdaudio_ext.h> > +#include <linux/pci.h> > > /* > * maximum HDAC capablities we should parse to avoid endless looping: > @@ -306,3 +307,25 @@ int snd_hdac_ext_bus_link_power_down_all(struct hdac_ext_bus *ebus) > return 0; > } > EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_down_all); > + > +static void update_pci_dword(struct pci_dev *pci, > + unsigned int reg, u32 mask, u32 val) > +{ > + u32 data; > + > + pci_read_config_dword(pci, reg, &data); > + data &= ~mask; > + data |= (val & mask); > + pci_write_config_dword(pci, reg, data); > +} > + > +void snd_hdac_ext_bus_enable_miscbdcge(struct device *dev, bool enable) > +{ > + struct pci_dev *pci = to_pci_dev(dev); > + u32 val; > + > + val = enable ? AZX_CGCTL_MISCBDCGE_MASK : 0; > + > + update_pci_dword(pci, AZX_PCIREG_CGCTL, AZX_CGCTL_MISCBDCGE_MASK, val); > +} > +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_enable_miscbdcge); PCI access doesn't belong in the HDA-core in general. It's over the abstraction level of HD-audio controller. Takashi
On Fri, Dec 11, 2015 at 09:54:42AM +0100, Takashi Iwai wrote: > > +static void update_pci_dword(struct pci_dev *pci, > > + unsigned int reg, u32 mask, u32 val) > > +{ > > + u32 data; > > + > > + pci_read_config_dword(pci, reg, &data); > > + data &= ~mask; > > + data |= (val & mask); > > + pci_write_config_dword(pci, reg, data); > > +} > > + > > +void snd_hdac_ext_bus_enable_miscbdcge(struct device *dev, bool enable) > > +{ > > + struct pci_dev *pci = to_pci_dev(dev); > > + u32 val; > > + > > + val = enable ? AZX_CGCTL_MISCBDCGE_MASK : 0; > > + > > + update_pci_dword(pci, AZX_PCIREG_CGCTL, AZX_CGCTL_MISCBDCGE_MASK, val); > > +} > > +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_enable_miscbdcge); > > PCI access doesn't belong in the HDA-core in general. It's over the > abstraction level of HD-audio controller. Okay but this is part of HDA controller and I think we might be using this bit so move to core for common usage. I can move to driver if you feel that is better approach
On Fri, 11 Dec 2015 10:25:27 +0100, Vinod Koul wrote: > > On Fri, Dec 11, 2015 at 09:54:42AM +0100, Takashi Iwai wrote: > > > +static void update_pci_dword(struct pci_dev *pci, > > > + unsigned int reg, u32 mask, u32 val) > > > +{ > > > + u32 data; > > > + > > > + pci_read_config_dword(pci, reg, &data); > > > + data &= ~mask; > > > + data |= (val & mask); > > > + pci_write_config_dword(pci, reg, data); > > > +} > > > + > > > +void snd_hdac_ext_bus_enable_miscbdcge(struct device *dev, bool enable) > > > +{ > > > + struct pci_dev *pci = to_pci_dev(dev); > > > + u32 val; > > > + > > > + val = enable ? AZX_CGCTL_MISCBDCGE_MASK : 0; > > > + > > > + update_pci_dword(pci, AZX_PCIREG_CGCTL, AZX_CGCTL_MISCBDCGE_MASK, val); > > > +} > > > +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_enable_miscbdcge); > > > > PCI access doesn't belong in the HDA-core in general. It's over the > > abstraction level of HD-audio controller. > > Okay but this is part of HDA controller and I think we might be using this > bit so move to core for common usage. No, PCI isn't common. Look through sound/hda directory once. sound/hda/* is the place to handle the HD-audio bus itself. > I can move to driver if you feel that is better approach Yes, it's better. Takashi
On Sat, Dec 12, 2015 at 05:14:48AM +0800, kbuild test robot wrote: > Hi Jayachandran, > > [auto build test WARNING on asoc/for-next] > [also build test WARNING on next-20151211] > [cannot apply to sound/for-next v4.4-rc4] > > url: https://github.com/0day-ci/linux/commits/Vinod-Koul/ASoC-Intel-Skylake-Update-to-SKL-driver/20151211-145345 > base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next > config: mn10300-allmodconfig (attached as .config) > reproduce: > wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=mn10300 > > All warnings (new ones prefixed by >>): > > sound/hda/ext/hdac_ext_controller.c: In function 'update_pci_dword': > >> sound/hda/ext/hdac_ext_controller.c:317:7: warning: 'data' is used uninitialized in this function [-Wuninitialized] > data &= ~mask; Thanks, will fix it in v2
diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h index 28ac1f9a18ac..fa33237f4bd1 100644 --- a/include/sound/hda_register.h +++ b/include/sound/hda_register.h @@ -96,6 +96,9 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 }; /* PCI space */ #define AZX_PCIREG_TCSEL 0x44 +#define AZX_PCIREG_CGCTL 0x48 +#define AZX_CGCTL_MISCBDCGE_MASK (1 << 6) + /* * other constants */ diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index f3454950ee0b..65961bbb8ca3 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -152,6 +152,7 @@ void snd_hdac_ext_link_set_stream_id(struct hdac_ext_link *link, int stream); void snd_hdac_ext_link_clear_stream_id(struct hdac_ext_link *link, int stream); +void snd_hdac_ext_bus_enable_miscbdcge(struct device *dev, bool enable); /* update register macro */ #define snd_hdac_updatel(addr, reg, mask, val) \ diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c index 556267e75591..8f1d292a522c 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -21,6 +21,7 @@ #include <linux/slab.h> #include <sound/hda_register.h> #include <sound/hdaudio_ext.h> +#include <linux/pci.h> /* * maximum HDAC capablities we should parse to avoid endless looping: @@ -306,3 +307,25 @@ int snd_hdac_ext_bus_link_power_down_all(struct hdac_ext_bus *ebus) return 0; } EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_down_all); + +static void update_pci_dword(struct pci_dev *pci, + unsigned int reg, u32 mask, u32 val) +{ + u32 data; + + pci_read_config_dword(pci, reg, &data); + data &= ~mask; + data |= (val & mask); + pci_write_config_dword(pci, reg, data); +} + +void snd_hdac_ext_bus_enable_miscbdcge(struct device *dev, bool enable) +{ + struct pci_dev *pci = to_pci_dev(dev); + u32 val; + + val = enable ? AZX_CGCTL_MISCBDCGE_MASK : 0; + + update_pci_dword(pci, AZX_PCIREG_CGCTL, AZX_CGCTL_MISCBDCGE_MASK, val); +} +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_enable_miscbdcge);