| Message ID | 1486116634-14930-1-git-send-email-vincent.abriou@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 2017-02-03 at 11:10 +0100, Vincent Abriou wrote: > While unregistering the hdmi-codec, the hdmi device list must be > cleaned up. It avoids kernel page fault when registering again the > hdmi-codec. > > Cc: Liam Girdwood <lgirdwood@gmail.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: Jaroslav Kysela <perex@perex.cz> > Cc: Takashi Iwai <tiwai@suse.com> > Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > Cc: Jyri Sarha <jsarha@ti.com> > Cc: Arnaud Pouliquen <arnaud.pouliquen@st.com> > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > Signed-off-by: Vincent Abriou <vincent.abriou@st.com> > --- > sound/soc/codecs/hdmi-codec.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c > index 90b5948..1ca405e 100644 > --- a/sound/soc/codecs/hdmi-codec.c > +++ b/sound/soc/codecs/hdmi-codec.c > @@ -479,6 +479,18 @@ static int hdmi_codec_probe(struct platform_device *pdev) > > static int hdmi_codec_remove(struct platform_device *pdev) > { > + struct device *dev = &pdev->dev; Seems unnecessary for a single use. > + struct list_head *pos; > + > + list_for_each(pos, &hdmi_device_list) { > + struct hdmi_device *tmp = pos_to_hdmi_device(pos); > + > + if (tmp->dev == dev->parent) { > + list_del(pos); > + break; > + } > + } > + > snd_soc_unregister_codec(&pdev->dev); Or, if dev was available, it should be used here, too. > return 0; > } regards Philipp
On 02/03/2017 11:10 AM, Vincent Abriou wrote: > While unregistering the hdmi-codec, the hdmi device list must be > cleaned up. It avoids kernel page fault when registering again the > hdmi-codec. > > Cc: Liam Girdwood <lgirdwood@gmail.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: Jaroslav Kysela <perex@perex.cz> > Cc: Takashi Iwai <tiwai@suse.com> > Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > Cc: Jyri Sarha <jsarha@ti.com> > Cc: Arnaud Pouliquen <arnaud.pouliquen@st.com> > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > Signed-off-by: Vincent Abriou <vincent.abriou@st.com> > --- > sound/soc/codecs/hdmi-codec.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c > index 90b5948..1ca405e 100644 > --- a/sound/soc/codecs/hdmi-codec.c > +++ b/sound/soc/codecs/hdmi-codec.c > @@ -479,6 +479,18 @@ static int hdmi_codec_probe(struct platform_device *pdev) > > static int hdmi_codec_remove(struct platform_device *pdev) > { > + struct device *dev = &pdev->dev; > + struct list_head *pos; > + > + list_for_each(pos, &hdmi_device_list) { This needs some kind of lock to protect against concurrent iteration and modification. > + struct hdmi_device *tmp = pos_to_hdmi_device(pos); > + > + if (tmp->dev == dev->parent) { > + list_del(pos); > + break; > + } > + } > + > snd_soc_unregister_codec(&pdev->dev); > return 0; > } >
Hi Vincent > While unregistering the hdmi-codec, the hdmi device list must be > cleaned up. It avoids kernel page fault when registering again the > hdmi-codec. > > Cc: Liam Girdwood <lgirdwood@gmail.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: Jaroslav Kysela <perex@perex.cz> > Cc: Takashi Iwai <tiwai@suse.com> > Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > Cc: Jyri Sarha <jsarha@ti.com> > Cc: Arnaud Pouliquen <arnaud.pouliquen@st.com> > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > Signed-off-by: Vincent Abriou <vincent.abriou@st.com> > --- Thank you about this patch. I added original "hdmi_device_list" to this driver for DW-HDMI device purpose. But, actually, 1) DW-HDMI binding method was exchanged, 2) I assumed it will not be exchanged, and 3) this patch was accepted under such (wrong) assumption. Thus, this "hdmi_device_list" is no longer needed. So, I'm planing to remove 9731f82d60166a19af6914f998092bbd1560f783 ("ASoC: hdmi-codec: enable multi probe for same device") and its related patch soon. Can you agree about it ? > sound/soc/codecs/hdmi-codec.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c > index 90b5948..1ca405e 100644 > --- a/sound/soc/codecs/hdmi-codec.c > +++ b/sound/soc/codecs/hdmi-codec.c > @@ -479,6 +479,18 @@ static int hdmi_codec_probe(struct platform_device *pdev) > > static int hdmi_codec_remove(struct platform_device *pdev) > { > + struct device *dev = &pdev->dev; > + struct list_head *pos; > + > + list_for_each(pos, &hdmi_device_list) { > + struct hdmi_device *tmp = pos_to_hdmi_device(pos); > + > + if (tmp->dev == dev->parent) { > + list_del(pos); > + break; > + } > + } > + > snd_soc_unregister_codec(&pdev->dev); > return 0; > } > -- > 2.7.4 > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Hi Kuninori, On 02/06/2017 01:09 AM, Kuninori Morimoto wrote: > > Hi Vincent > >> While unregistering the hdmi-codec, the hdmi device list must be >> cleaned up. It avoids kernel page fault when registering again the >> hdmi-codec. >> >> Cc: Liam Girdwood <lgirdwood@gmail.com> >> Cc: Mark Brown <broonie@kernel.org> >> Cc: Jaroslav Kysela <perex@perex.cz> >> Cc: Takashi Iwai <tiwai@suse.com> >> Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> >> Cc: Jyri Sarha <jsarha@ti.com> >> Cc: Arnaud Pouliquen <arnaud.pouliquen@st.com> >> Cc: Philipp Zabel <p.zabel@pengutronix.de> >> >> Signed-off-by: Vincent Abriou <vincent.abriou@st.com> >> --- > > Thank you about this patch. > I added original "hdmi_device_list" to this driver for DW-HDMI device purpose. > But, actually, 1) DW-HDMI binding method was exchanged, 2) I assumed it > will not be exchanged, and 3) this patch was accepted under such > (wrong) assumption. > > Thus, this "hdmi_device_list" is no longer needed. > So, I'm planing to remove 9731f82d60166a19af6914f998092bbd1560f783 > ("ASoC: hdmi-codec: enable multi probe for same device") > and its related patch soon. > > Can you agree about it ? > I am fine with you proposal. Can you keep Arnaud Pouliquen and myself in the loop of your new patche series so that we are able to test it on our side. Thanks. Vincent >> sound/soc/codecs/hdmi-codec.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c >> index 90b5948..1ca405e 100644 >> --- a/sound/soc/codecs/hdmi-codec.c >> +++ b/sound/soc/codecs/hdmi-codec.c >> @@ -479,6 +479,18 @@ static int hdmi_codec_probe(struct platform_device *pdev) >> >> static int hdmi_codec_remove(struct platform_device *pdev) >> { >> + struct device *dev = &pdev->dev; >> + struct list_head *pos; >> + >> + list_for_each(pos, &hdmi_device_list) { >> + struct hdmi_device *tmp = pos_to_hdmi_device(pos); >> + >> + if (tmp->dev == dev->parent) { >> + list_del(pos); >> + break; >> + } >> + } >> + >> snd_soc_unregister_codec(&pdev->dev); >> return 0; >> } >> -- >> 2.7.4 >> >> _______________________________________________ >> Alsa-devel mailing list >> Alsa-devel@alsa-project.org >> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Hi Vincent > >> While unregistering the hdmi-codec, the hdmi device list must be > >> cleaned up. It avoids kernel page fault when registering again the > >> hdmi-codec. > >> > >> Cc: Liam Girdwood <lgirdwood@gmail.com> > >> Cc: Mark Brown <broonie@kernel.org> > >> Cc: Jaroslav Kysela <perex@perex.cz> > >> Cc: Takashi Iwai <tiwai@suse.com> > >> Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > >> Cc: Jyri Sarha <jsarha@ti.com> > >> Cc: Arnaud Pouliquen <arnaud.pouliquen@st.com> > >> Cc: Philipp Zabel <p.zabel@pengutronix.de> > >> > >> Signed-off-by: Vincent Abriou <vincent.abriou@st.com> > >> --- > > > > Thank you about this patch. > > I added original "hdmi_device_list" to this driver for DW-HDMI device purpose. > > But, actually, 1) DW-HDMI binding method was exchanged, 2) I assumed it > > will not be exchanged, and 3) this patch was accepted under such > > (wrong) assumption. > > > > Thus, this "hdmi_device_list" is no longer needed. > > So, I'm planing to remove 9731f82d60166a19af6914f998092bbd1560f783 > > ("ASoC: hdmi-codec: enable multi probe for same device") > > and its related patch soon. > > > > Can you agree about it ? > > > > I am fine with you proposal. > Can you keep Arnaud Pouliquen and myself in the loop of your new patche > series so that we are able to test it on our side. Thanks. Above remove patch is one of my next patch-set which is based on my current OF-graph sound card patch-set. Thus, I'm waiting its result. But above removing patch itself is just single patch. So, I will post it if OF-graph patch-set progress was too late. Best regards --- Kuninori Morimoto
Hi Mark > > >> While unregistering the hdmi-codec, the hdmi device list must be > > >> cleaned up. It avoids kernel page fault when registering again the > > >> hdmi-codec. > > >> > > >> Cc: Liam Girdwood <lgirdwood@gmail.com> > > >> Cc: Mark Brown <broonie@kernel.org> > > >> Cc: Jaroslav Kysela <perex@perex.cz> > > >> Cc: Takashi Iwai <tiwai@suse.com> > > >> Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > >> Cc: Jyri Sarha <jsarha@ti.com> > > >> Cc: Arnaud Pouliquen <arnaud.pouliquen@st.com> > > >> Cc: Philipp Zabel <p.zabel@pengutronix.de> > > >> > > >> Signed-off-by: Vincent Abriou <vincent.abriou@st.com> > > >> --- (snip) > Thanks. > Above remove patch is one of my next patch-set which is > based on my current OF-graph sound card patch-set. > Thus, I'm waiting its result. > > But above removing patch itself is just single patch. > So, I will post it if OF-graph patch-set progress was too late. But... this patch is anyway bugfix patch. And my remove patch posting will be delayed (= based on OF-graph which will take long). Thus, I think applying this patch as bugfix so far, and remove/cleanup all after that seems reasonable. I guess this patch got 2 reviews, thus it needs v2 (?) For basic idea Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Best regards --- Kuninori Morimoto
On Wed, Feb 08, 2017 at 02:00:22AM +0000, Kuninori Morimoto wrote:
> I guess this patch got 2 reviews, thus it needs v2 (?)
Yes.
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c index 90b5948..1ca405e 100644 --- a/sound/soc/codecs/hdmi-codec.c +++ b/sound/soc/codecs/hdmi-codec.c @@ -479,6 +479,18 @@ static int hdmi_codec_probe(struct platform_device *pdev) static int hdmi_codec_remove(struct platform_device *pdev) { + struct device *dev = &pdev->dev; + struct list_head *pos; + + list_for_each(pos, &hdmi_device_list) { + struct hdmi_device *tmp = pos_to_hdmi_device(pos); + + if (tmp->dev == dev->parent) { + list_del(pos); + break; + } + } + snd_soc_unregister_codec(&pdev->dev); return 0; }
While unregistering the hdmi-codec, the hdmi device list must be cleaned up. It avoids kernel page fault when registering again the hdmi-codec. Cc: Liam Girdwood <lgirdwood@gmail.com> Cc: Mark Brown <broonie@kernel.org> Cc: Jaroslav Kysela <perex@perex.cz> Cc: Takashi Iwai <tiwai@suse.com> Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Cc: Jyri Sarha <jsarha@ti.com> Cc: Arnaud Pouliquen <arnaud.pouliquen@st.com> Cc: Philipp Zabel <p.zabel@pengutronix.de> Signed-off-by: Vincent Abriou <vincent.abriou@st.com> --- sound/soc/codecs/hdmi-codec.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)