From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH 1/7] ALSA: ac97: Add helper function to reset the AC97 device Date: Wed, 22 Jul 2015 10:44:30 +0200 Message-ID: References: <1437508386-13828-1-git-send-email-lars@metafoo.de> <1437508386-13828-2-git-send-email-lars@metafoo.de> 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 DD207260664 for ; Wed, 22 Jul 2015 10:44:31 +0200 (CEST) In-Reply-To: <1437508386-13828-2-git-send-email-lars@metafoo.de> 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: Lars-Peter Clausen Cc: alsa-devel@alsa-project.org, patches@opensource.wolfsonmicro.com, Liam Girdwood , Mark Brown , Manuel Lauss , Charles Keepax List-Id: alsa-devel@alsa-project.org On Tue, 21 Jul 2015 21:53:00 +0200, Lars-Peter Clausen wrote: > > There is currently a lot of code duplication in ASoC drivers regarding the > reset handling of devices. This patch introduces a new generic reset > function in the generic AC'97 framework that can be used to replace most > the custom reset functions. > > Signed-off-by: Lars-Peter Clausen Reviewed-by: Takashi Iwai One possible improvement would be to allow to pass id_mask=0 as the full bit, so that you don't have to define the mask 0xffffffff at each time. thanks, Takashi > --- > include/sound/ac97_codec.h | 2 ++ > sound/ac97_bus.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 64 insertions(+) > > diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h > index 0e9d75b..74bc8547 100644 > --- a/include/sound/ac97_codec.h > +++ b/include/sound/ac97_codec.h > @@ -584,6 +584,8 @@ static inline int snd_ac97_update_power(struct snd_ac97 *ac97, int reg, > void snd_ac97_suspend(struct snd_ac97 *ac97); > void snd_ac97_resume(struct snd_ac97 *ac97); > #endif > +int snd_ac97_reset(struct snd_ac97 *ac97, bool try_warm, unsigned int id, > + unsigned int id_mask); > > /* quirk types */ > enum { > diff --git a/sound/ac97_bus.c b/sound/ac97_bus.c > index 2b50cbe..55791a0 100644 > --- a/sound/ac97_bus.c > +++ b/sound/ac97_bus.c > @@ -18,6 +18,68 @@ > #include > > /* > + * snd_ac97_check_id() - Reads and checks the vendor ID of the device > + * @ac97: The AC97 device to check > + * @id: The ID to compare to > + * @id_mask: Mask that is applied to the device ID before comparing to @id > + * > + * If @id is 0 this function returns true if the read device vendor ID is > + * a valid ID. If @id is non 0 this functions returns true if @id > + * matches the read vendor ID. Otherwise the function returns false. > + */ > +static bool snd_ac97_check_id(struct snd_ac97 *ac97, unsigned int id, > + unsigned int id_mask) > +{ > + ac97->id = ac97->bus->ops->read(ac97, AC97_VENDOR_ID1) << 16; > + ac97->id |= ac97->bus->ops->read(ac97, AC97_VENDOR_ID2); > + > + if (ac97->id == 0x0 || ac97->id == 0xffffffff) > + return false; > + > + if (id != 0 && id != (ac97->id & id_mask)) > + return false; > + > + return true; > +} > + > +/** > + * snd_ac97_reset() - Reset AC'97 device > + * @ac97: The AC'97 device to reset > + * @try_warm: Try a warm reset first > + * @id: Expected device vendor ID > + * @id_mask: Mask that is applied to the device ID before comparing to @id > + * > + * This function resets the AC'97 device. If @try_warm is true the function > + * first performs a warm reset. If the warm reset is successful the function > + * returns 1. Otherwise or if @try_warm is false the function issues cold reset > + * followed by a warm reset. If this is successful the function returns 0, > + * otherwise a negative error code. If @id is 0 any valid device ID will be > + * accepted, otherwise only the ID that matches @id and @id_mask is accepted. > + */ > +int snd_ac97_reset(struct snd_ac97 *ac97, bool try_warm, unsigned int id, > + unsigned int id_mask) > +{ > + struct snd_ac97_bus_ops *ops = ac97->bus->ops; > + > + if (try_warm && ops->warm_reset) { > + ops->warm_reset(ac97); > + if (snd_ac97_check_id(ac97, id, id_mask)) > + return 1; > + } > + > + if (ops->reset) > + ops->reset(ac97); > + if (ops->warm_reset) > + ops->warm_reset(ac97); > + > + if (snd_ac97_check_id(ac97, id, id_mask)) > + return 0; > + > + return -ENODEV; > +} > +EXPORT_SYMBOL_GPL(snd_ac97_reset); > + > +/* > * Let drivers decide whether they want to support given codec from their > * probe method. Drivers have direct access to the struct snd_ac97 > * structure and may decide based on the id field amongst other things. > -- > 2.1.4 > >