[PATCH] ASoc: hdmi-codec: remove HDMI device unregister

[PATCH] ASoc: hdmi-codec: remove HDMI device unregister

[Vincent Abriou]

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) {
+		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

Re: [PATCH] ASoc: hdmi-codec: remove HDMI device unregister

[Philipp Zabel]

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

Re: [PATCH] ASoc: hdmi-codec: remove HDMI device unregister

[Lars-Peter Clausen]

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;
>  }
> 

Re: [PATCH] ASoc: hdmi-codec: remove HDMI device unregister

[Kuninori Morimoto]

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

Re: [PATCH] ASoc: hdmi-codec: remove HDMI device unregister

[Vincent ABRIOU]

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

Re: [PATCH] ASoc: hdmi-codec: remove HDMI device unregister

[Kuninori Morimoto]

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

Re: [PATCH] ASoc: hdmi-codec: remove HDMI device unregister

[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

Re: [PATCH] ASoc: hdmi-codec: remove HDMI device unregister

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 131 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]