linux-kernel - Re: [PATCH AUTOSEL 4.18 28/48] sound: don't call skl_init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <s5hy3b84w46.wl-tiwai@suse.de>
Date:   Mon, 08 Oct 2018 11:37:45 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     Mark Brown <broonie@...nel.org>
Cc:     Sasha Levin <sashal@...nel.org>, stable@...r.kernel.org,
        linux-kernel@...r.kernel.org, Yu Zhao <yuzhao@...gle.com>,
        Sasha Levin <alexander.levin@...rosoft.com>, tiwai@...e.de
Subject: Re: [PATCH AUTOSEL 4.18 28/48] sound: don't call skl_init_chip() to reset intel skl soc

On Mon, 08 Oct 2018 11:34:25 +0200,
Mark Brown wrote:
> 
> On Fri, Oct 05, 2018 at 12:14:04PM -0400, Sasha Levin wrote:
> > From: Yu Zhao <yuzhao@...gle.com>
> > 
> > [ Upstream commit 75383f8d39d4c0fb96083dd460b7b139fbdac492 ]
> > 
> > Internally, skl_init_chip() calls snd_hdac_bus_init_chip() which
> > 1) sets bus->chip_init to prevent multiple entrances before device
> > is stopped; 2) enables interrupt.
> 
> And here.

This one should be OK as a stable fix, too.


thanks,

Takashi

> > 
> > We shouldn't use it for the purpose of resetting device only because
> > 1) when we really want to initialize device, we won't be able to do
> > so; 2) we are ready to handle interrupt yet, and kernel crashes when
> > interrupt comes in.
> > 
> > Rename azx_reset() to snd_hdac_bus_reset_link(), and use it to reset
> > device properly.
> > 
> > Fixes: 60767abcea3d ("ASoC: Intel: Skylake: Reset the controller in probe")
> > Reviewed-by: Takashi Iwai <tiwai@...e.de>
> > Signed-off-by: Yu Zhao <yuzhao@...gle.com>
> > Signed-off-by: Mark Brown <broonie@...nel.org>
> > Signed-off-by: Sasha Levin <alexander.levin@...rosoft.com>
> > ---
> >  include/sound/hdaudio.h       | 1 +
> >  sound/hda/hdac_controller.c   | 7 ++++---
> >  sound/soc/intel/skylake/skl.c | 2 +-
> >  3 files changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
> > index c052afc27547..138e976a2ba2 100644
> > --- a/include/sound/hdaudio.h
> > +++ b/include/sound/hdaudio.h
> > @@ -355,6 +355,7 @@ void snd_hdac_bus_init_cmd_io(struct hdac_bus *bus);
> >  void snd_hdac_bus_stop_cmd_io(struct hdac_bus *bus);
> >  void snd_hdac_bus_enter_link_reset(struct hdac_bus *bus);
> >  void snd_hdac_bus_exit_link_reset(struct hdac_bus *bus);
> > +int snd_hdac_bus_reset_link(struct hdac_bus *bus, bool full_reset);
> >  
> >  void snd_hdac_bus_update_rirb(struct hdac_bus *bus);
> >  int snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
> > diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
> > index 11057d9f84ec..74244d8e2909 100644
> > --- a/sound/hda/hdac_controller.c
> > +++ b/sound/hda/hdac_controller.c
> > @@ -385,7 +385,7 @@ void snd_hdac_bus_exit_link_reset(struct hdac_bus *bus)
> >  EXPORT_SYMBOL_GPL(snd_hdac_bus_exit_link_reset);
> >  
> >  /* reset codec link */
> > -static int azx_reset(struct hdac_bus *bus, bool full_reset)
> > +int snd_hdac_bus_reset_link(struct hdac_bus *bus, bool full_reset)
> >  {
> >  	if (!full_reset)
> >  		goto skip_reset;
> > @@ -410,7 +410,7 @@ static int azx_reset(struct hdac_bus *bus, bool full_reset)
> >   skip_reset:
> >  	/* check to see if controller is ready */
> >  	if (!snd_hdac_chip_readb(bus, GCTL)) {
> > -		dev_dbg(bus->dev, "azx_reset: controller not ready!\n");
> > +		dev_dbg(bus->dev, "controller not ready!\n");
> >  		return -EBUSY;
> >  	}
> >  
> > @@ -425,6 +425,7 @@ static int azx_reset(struct hdac_bus *bus, bool full_reset)
> >  
> >  	return 0;
> >  }
> > +EXPORT_SYMBOL_GPL(snd_hdac_bus_reset_link);
> >  
> >  /* enable interrupts */
> >  static void azx_int_enable(struct hdac_bus *bus)
> > @@ -479,7 +480,7 @@ bool snd_hdac_bus_init_chip(struct hdac_bus *bus, bool full_reset)
> >  		return false;
> >  
> >  	/* reset controller */
> > -	azx_reset(bus, full_reset);
> > +	snd_hdac_bus_reset_link(bus, full_reset);
> >  
> >  	/* clear interrupts */
> >  	azx_int_clear(bus);
> > diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
> > index f0d9793f872a..c7cdfa4a7076 100644
> > --- a/sound/soc/intel/skylake/skl.c
> > +++ b/sound/soc/intel/skylake/skl.c
> > @@ -844,7 +844,7 @@ static int skl_first_init(struct hdac_ext_bus *ebus)
> >  		return -ENXIO;
> >  	}
> >  
> > -	skl_init_chip(bus, true);
> > +	snd_hdac_bus_reset_link(bus, true);
> >  
> >  	snd_hdac_bus_parse_capabilities(bus);
> >  
> > -- 
> > 2.17.1
> > 
> [2 signature.asc <application/pgp-signature (7bit)>]
>