Re: [PATCH] ASoC: hdmi-codec: hdmi_codec_priv includes snd_kcontrol_new

2017-07-05 Thread Takashi Iwai
On Wed, 05 Jul 2017 11:29:43 +0200,
Arnaud Pouliquen wrote:
> 
> 
> 
> On 07/05/2017 10:40 AM, Takashi Iwai wrote:
> > On Wed, 05 Jul 2017 10:15:35 +0200,
> > Kuninori Morimoto wrote:
> >>
> >>
> >> From: Kuninori Morimoto <kuninori.morimoto...@renesas.com>
> >>
> >> Current hdmi-codec driver is using hdmi_controls for "ELD" control.
> >> But, hdmi-codec driver might be used from many HDMIs. In such case,
> >> they will use same "ELD" name and kernel will indicate below error.
> >>
> >>xxx: control x:x:x:ELD:x is already present
> >>
> >> hdmi_controls will be registered in soc_probe_component(), and we can
> >> replace it by component driver probe function.
> >>
> >> This patch registers current hdmi_controls in new hdmi_probe().
> >> If hdmi-codec is used from only 1 device, it will use "ELD" as control name
> >> (We can keep compatibility).
> >> If hdmi-codec is used from many devices, it will use "ELD.x" as control 
> >> name.
> > 
> > The de facto standard way as HD-audio does is to pass the device
> > number of each ELD control as same as the corresponding PCM stream
> > number.
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> In theory, hdmi_codec_pcm_new already does the job.
> snd_pcm_add_chmap_ctls instantiates the controls using PCM device ID. So
> you should observe (with amixer) 2 controls with same name but not same
> device ID.

Indeed, we can add each ELD control there like a (totally untested)
patch below.


thanks,

Takashi

---
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 22ed0dc88f0a..13d9cf5b1831 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -399,16 +399,13 @@ static int hdmi_codec_chmap_ctl_get(struct snd_kcontrol 
*kcontrol,
return 0;
 }
 
-
-static const struct snd_kcontrol_new hdmi_controls[] = {
-   {
-   .access = SNDRV_CTL_ELEM_ACCESS_READ |
- SNDRV_CTL_ELEM_ACCESS_VOLATILE,
-   .iface = SNDRV_CTL_ELEM_IFACE_PCM,
-   .name = "ELD",
-   .info = hdmi_eld_ctl_info,
-   .get = hdmi_eld_ctl_get,
-   },
+static const struct snd_kcontrol_new hdmi_eld_ctl = {
+   .access = SNDRV_CTL_ELEM_ACCESS_READ |
+ SNDRV_CTL_ELEM_ACCESS_VOLATILE,
+   .iface = SNDRV_CTL_ELEM_IFACE_PCM,
+   .name = "ELD",
+   .info = hdmi_eld_ctl_info,
+   .get = hdmi_eld_ctl_get,
 };
 
 static int hdmi_codec_new_stream(struct snd_pcm_substream *substream,
@@ -668,6 +665,7 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime 
*rtd,
 {
struct snd_soc_dai_driver *drv = dai->driver;
struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
+   struct snd_kcontrol *kctl;
int ret;
 
dev_dbg(dai->dev, "%s()\n", __func__);
@@ -686,7 +684,12 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime 
*rtd,
hcp->chmap_info->chmap = hdmi_codec_stereo_chmaps;
hcp->chmap_idx = HDMI_CODEC_CHMAP_IDX_UNKNOWN;
 
-   return 0;
+   /* add ELD ctl with the device number corresponding to the PCM stream */
+   kctl = snd_ctl_new1(_eld_ctl, dai->component);
+   if (!kctl)
+   return -ENOMEM;
+   kctl->id.device = rtd->pcm->device;
+   return snd_ctl_add(rtd->card->snd_card, kctl);
 }
 
 static struct snd_soc_dai_driver hdmi_i2s_dai = {
@@ -732,8 +735,6 @@ static int hdmi_of_xlate_dai_id(struct snd_soc_component 
*component,
 
 static struct snd_soc_codec_driver hdmi_codec = {
.component_driver = {
-   .controls   = hdmi_controls,
-   .num_controls   = ARRAY_SIZE(hdmi_controls),
.dapm_widgets   = hdmi_widgets,
.num_dapm_widgets   = ARRAY_SIZE(hdmi_widgets),
.dapm_routes= hdmi_routes,


Re: [PATCH] ASoC: hdmi-codec: hdmi_codec_priv includes snd_kcontrol_new

2017-07-05 Thread Takashi Iwai
On Wed, 05 Jul 2017 10:15:35 +0200,
Kuninori Morimoto wrote:
> 
> 
> From: Kuninori Morimoto 
> 
> Current hdmi-codec driver is using hdmi_controls for "ELD" control.
> But, hdmi-codec driver might be used from many HDMIs. In such case,
> they will use same "ELD" name and kernel will indicate below error.
> 
>   xxx: control x:x:x:ELD:x is already present
> 
> hdmi_controls will be registered in soc_probe_component(), and we can
> replace it by component driver probe function.
> 
> This patch registers current hdmi_controls in new hdmi_probe().
> If hdmi-codec is used from only 1 device, it will use "ELD" as control name
> (We can keep compatibility).
> If hdmi-codec is used from many devices, it will use "ELD.x" as control name.

The de facto standard way as HD-audio does is to pass the device
number of each ELD control as same as the corresponding PCM stream
number.


thanks,

Takashi

> Signed-off-by: Kuninori Morimoto 
> ---
>  sound/soc/codecs/hdmi-codec.c | 60 
> +--
>  1 file changed, 46 insertions(+), 14 deletions(-)
> 
> diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
> index 22ed0dc..5835a90 100644
> --- a/sound/soc/codecs/hdmi-codec.c
> +++ b/sound/soc/codecs/hdmi-codec.c
> @@ -276,6 +276,7 @@ struct hdmi_codec_cea_spk_alloc {
> .mask = FL | FR | LFE | FC | RL | RR | FLC | FRC },
>  };
>  
> +#define ELD_NAME_SIZE8
>  struct hdmi_codec_priv {
>   struct hdmi_codec_pdata hcd;
>   struct snd_soc_dai_driver *daidrv;
> @@ -284,7 +285,10 @@ struct hdmi_codec_priv {
>   struct snd_pcm_substream *current_stream;
>   uint8_t eld[MAX_ELD_BYTES];
>   struct snd_pcm_chmap *chmap_info;
> + struct snd_kcontrol_new control;
>   unsigned int chmap_idx;
> + int id;
> + char eld_name[ELD_NAME_SIZE];
>  };
>  
>  static const struct snd_soc_dapm_widget hdmi_widgets[] = {
> @@ -399,18 +403,6 @@ static int hdmi_codec_chmap_ctl_get(struct snd_kcontrol 
> *kcontrol,
>   return 0;
>  }
>  
> -
> -static const struct snd_kcontrol_new hdmi_controls[] = {
> - {
> - .access = SNDRV_CTL_ELEM_ACCESS_READ |
> -   SNDRV_CTL_ELEM_ACCESS_VOLATILE,
> - .iface = SNDRV_CTL_ELEM_IFACE_PCM,
> - .name = "ELD",
> - .info = hdmi_eld_ctl_info,
> - .get = hdmi_eld_ctl_get,
> - },
> -};
> -
>  static int hdmi_codec_new_stream(struct snd_pcm_substream *substream,
>struct snd_soc_dai *dai)
>  {
> @@ -718,6 +710,45 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime 
> *rtd,
>   .pcm_new = hdmi_codec_pcm_new,
>  };
>  
> +static int hdmi_last_id = 0;
> +static int hdmi_probe(struct snd_soc_component *component)
> +{
> + struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
> + static struct snd_soc_component *component_1st = NULL;
> + char *name = "ELD";
> +
> + /*
> +  * It will use "ELD"   if hdmi_codec was used from only 1 device.
> +  * It will use "ELD.x" if hdmi_codec was used from many devices.
> +  * Then, first "ELD" will be replaced to "ELD.0"
> +  */
> + snprintf(hcp->eld_name, ELD_NAME_SIZE, "ELD.%d", hcp->id);
> +
> + if (hcp->id == 0)
> + component_1st = component;
> +
> + if (hdmi_last_id > 1) {
> + name = hcp->eld_name;
> +
> + if (component_1st) {
> + struct hdmi_codec_priv *hcp_1st;
> +
> + /* replaced 1st "ELD" to "ELD.0" */
> + hcp_1st = snd_soc_component_get_drvdata(component_1st);
> + hcp_1st->control.name = hcp_1st->eld_name;
> + }
> + }
> +
> + hcp->control.access = SNDRV_CTL_ELEM_ACCESS_READ |
> +   SNDRV_CTL_ELEM_ACCESS_VOLATILE;
> + hcp->control.iface  = SNDRV_CTL_ELEM_IFACE_PCM;
> + hcp->control.name   = name;
> + hcp->control.info   = hdmi_eld_ctl_info;
> + hcp->control.get= hdmi_eld_ctl_get;
> +
> + return snd_soc_add_component_controls(component, >control, 1);
> +}
> +
>  static int hdmi_of_xlate_dai_id(struct snd_soc_component *component,
>struct device_node *endpoint)
>  {
> @@ -732,8 +763,7 @@ static int hdmi_of_xlate_dai_id(struct snd_soc_component 
> *component,
>  
>  static struct snd_soc_codec_driver hdmi_codec = {
>   .component_driver = {
> - .controls   = hdmi_controls,
> - .num_controls   = ARRAY_SIZE(hdmi_controls),
> + .probe  = hdmi_probe,
>   .dapm_widgets   = hdmi_widgets,
>   .num_dapm_widgets   = ARRAY_SIZE(hdmi_widgets),
>   .dapm_routes= hdmi_routes,
> @@ -768,6 +798,7 @@ static int hdmi_codec_probe(struct platform_device *pdev)
>   if 

Re: [alsa-devel] [RFC][PATCH] ASoC: soc-core: verify Sound Card normality

2017-04-03 Thread Takashi Iwai
On Mon, 03 Apr 2017 10:26:05 +0200,
Kuninori Morimoto wrote:
> 
> 
> Hi Takashi-san
> 
> > > So, we can call snd_soc_unregister_card() whenever CPU/Codec/Platform
> > > were unregsiterd.
> > 
> > In theory yes, but you should be careful to do so, e.g. make sure that
> > it won't be called again by the removal/unbind of other components /
> > drivers.
> > 
> > I suggested snd_card_disconnect() because it doesn't release resources
> > by itself, but it just disconnects from the further accesses.  So,
> > double-free won't happen in this case.  It makes the hotunplug safer
> > as long as the drivers manage the resource releases properly.
> 
> I had checked many unbind/bind pattern/order on 2nd [RFC] patch which I 
> posted.
> At first, I believe Oops on unbind/bind issue was solved on it.
> 2nd, if my understanding was correct, it doesn't have double-free issue,
> or something like that.
> But, I'm not 100% sure about 2nd, thus it has [RFC] on patch.

Ah, I see that snd_soc_unregister_card() has the check of
card->instantiated, so it should be fine to call multiple times.


thanks,

Takashi


Re: [alsa-devel] [RFC][PATCH] ASoC: soc-core: verify Sound Card normality

2017-04-03 Thread Takashi Iwai
On Mon, 03 Apr 2017 08:29:34 +0200,
Kuninori Morimoto wrote:
> 
> 
> Hi Takashi-san
> 
> > > I think this is a good direction to at least start to mitigate these
> > > problems (which we really should be doing) and hopefully make it easier
> > > to do further improvements in future.  There's obviously more places
> > > where we should be checking the flag (controls for example) but they can
> > > be added later.  One thing I would like to see is instead of setting the
> > > flag directly when we see a problem call a function to do it.  That way
> > > if we want to improve things in the future we can do that without having
> > > to update the callers again.
> > 
> > BTW, ALSA core has snd_card_disconnect() that does this kind of
> > shut-up from user-space.  It was introduced for hot-unplug, but
> > basically unbinding is the software hot-unplug.  So, if ASoC won't
> > rebind a once-unbound component, you can simply call
> > snd_card_disconnect() at the component unbinding time to assure that
> > no further user actions can be done.
> 
> Thanks. I checked about snd_card_disconnect(), and it will be called
> from snd_card_free(). And it will be called from snd_soc_unregister_card()

Yes, snd_card_free() assures the disconnection at first, syncs the all
settled down, then releases the resources.

> So, we can call snd_soc_unregister_card() whenever CPU/Codec/Platform
> were unregsiterd.

In theory yes, but you should be careful to do so, e.g. make sure that
it won't be called again by the removal/unbind of other components /
drivers.

I suggested snd_card_disconnect() because it doesn't release resources
by itself, but it just disconnects from the further accesses.  So,
double-free won't happen in this case.  It makes the hotunplug safer
as long as the drivers manage the resource releases properly.


Takashi

> This method also solve random unbind/bind Oops too.
> Here, random unbind/bind example is that
> expected correct operation is unbind all CPU/Codec/Platfrom/Card,
> and then, bind all CPU/Codec/Platfrom/Card again.
> (here unbind order can be random)
> But this case, we will get Oops if unbind Codec -> bind Codec -> unbind Card.
> Using snd_soc_unregister_card() can solve this issue too.
> 
> Best regards
> ---
> Kuninori Morimoto
> 


Re: [alsa-devel] [RFC][PATCH] ASoC: soc-core: verify Sound Card normality

2017-03-31 Thread Takashi Iwai
On Thu, 30 Mar 2017 23:53:34 +0200,
Mark Brown wrote:
> 
> On Wed, Mar 29, 2017 at 02:45:37AM +, Kuninori Morimoto wrote:
> 
> > To avoid Sound Card crash in (2) case, what we can do now is, add dirty
> > flag on Sound Card, and avoid to open Sound Card.
> > This patch solved this issue.
> 
> I think this is a good direction to at least start to mitigate these
> problems (which we really should be doing) and hopefully make it easier
> to do further improvements in future.  There's obviously more places
> where we should be checking the flag (controls for example) but they can
> be added later.  One thing I would like to see is instead of setting the
> flag directly when we see a problem call a function to do it.  That way
> if we want to improve things in the future we can do that without having
> to update the callers again.

BTW, ALSA core has snd_card_disconnect() that does this kind of
shut-up from user-space.  It was introduced for hot-unplug, but
basically unbinding is the software hot-unplug.  So, if ASoC won't
rebind a once-unbound component, you can simply call
snd_card_disconnect() at the component unbinding time to assure that
no further user actions can be done.


Takashi


Re: Question about struct snd_soc_dai() :: cpu_dai->codec

2016-08-04 Thread Takashi Iwai
On Thu, 04 Aug 2016 15:39:40 +0200,
Takashi Sakamoto wrote:
> 
> On Aug 4 2016 21:27, Takashi Iwai wrote:
> > On Thu, 04 Aug 2016 14:12:09 +0200,
> > Takashi Sakamoto wrote:
> >>
> >> On Aug 4 2016 19:28, Mark Brown wrote:
> >>> On Thu, Aug 04, 2016 at 12:17:57PM +0900, Takashi Sakamoto wrote:
> >>>> On Jul 30 2016 07:08, Mark Brown wrote:
> >>>
> >>>>> The card should be deinstantiated and reinstantiated whenever a
> >>>>> component driver unbinds and rebinds (respectively).  You'd need to
> >>>>> completely deregister the card to change the list of things it's
> >>>>> expecting currently.
> >>>
> >>>> In a point of application interfaces, I guess that current 
> >>>> implementation of
> >>>> ALSA soc part includes a bug that it's possible to unload codec or 
> >>>> component
> >>>> modules when any ALSA character devices are opened. The framework has no
> >>>> codes to manage reference counting of character devices or loaded codecs,
> >>>> components.
> >>>
> >>> Yes, exactly - we don't cope very well with that situation and we really
> >>> ought to but since it's hard to trigger without trying in practice it's
> >>> never been a priority.
> >>
> >> Ugly... completely ugly idea for user space applications and operating
> >> system... It's better for developers for ALSA soc part to pay enough
> >> attention not only to their hardwares but also to application interfaces.
> >>
> >> Please assume that a loaded module for SoC's sound interface which
> >> supports Jack detection, and pulseaudio runs on the system. Then,
> >> typically, the process listen to ALSA ctrl character device for Jack
> >> detection.
> >>
> >> In this case, when modules for codec or component are unloaded, what
> >> happends?
> > 
> > You can't unload.  The module unload is already protected by the
> > proper module refcounting.
> 
> Hm. For my information, could you please show call graph to increment
> the reference counter of codec/component modules when modules for SoC's
> sound interfaces refer to them?

Just grep try_module_get() and module_put() calls.


Takashi


Re: Question about struct snd_soc_dai() :: cpu_dai->codec

2016-08-04 Thread Takashi Iwai
On Thu, 04 Aug 2016 14:12:09 +0200,
Takashi Sakamoto wrote:
> 
> On Aug 4 2016 19:28, Mark Brown wrote:
> > On Thu, Aug 04, 2016 at 12:17:57PM +0900, Takashi Sakamoto wrote:
> >> On Jul 30 2016 07:08, Mark Brown wrote:
> > 
> >>> The card should be deinstantiated and reinstantiated whenever a
> >>> component driver unbinds and rebinds (respectively).  You'd need to
> >>> completely deregister the card to change the list of things it's
> >>> expecting currently.
> > 
> >> In a point of application interfaces, I guess that current implementation 
> >> of
> >> ALSA soc part includes a bug that it's possible to unload codec or 
> >> component
> >> modules when any ALSA character devices are opened. The framework has no
> >> codes to manage reference counting of character devices or loaded codecs,
> >> components.
> > 
> > Yes, exactly - we don't cope very well with that situation and we really
> > ought to but since it's hard to trigger without trying in practice it's
> > never been a priority.
> 
> Ugly... completely ugly idea for user space applications and operating
> system... It's better for developers for ALSA soc part to pay enough
> attention not only to their hardwares but also to application interfaces.
> 
> Please assume that a loaded module for SoC's sound interface which
> supports Jack detection, and pulseaudio runs on the system. Then,
> typically, the process listen to ALSA ctrl character device for Jack
> detection.
> 
> In this case, when modules for codec or component are unloaded, what
> happends?

You can't unload.  The module unload is already protected by the
proper module refcounting.

> In worst case, pulseaudio process can kill the system in a
> system call path or something else, because the modules for SoC's sound
> interface is still loaded and userspace applications can execute system
> calls via ALSA character devices.
> 
> It should be forbidden to build ALSA soc part as loadable kernel
> modules. It's really danger...

The only danger is about the dynamic unbinding, and it has nothing to
do with the module.  And, protecting the sysfs unbind itself is
trivial, too: as I already suggested, we can put the flags in
appropriate places.


Takashi


Re: Question about struct snd_soc_dai() :: cpu_dai->codec

2016-07-29 Thread Takashi Iwai
On Fri, 29 Jul 2016 18:07:02 +0200,
Vinod Koul wrote:
> 
> On Fri, Jul 29, 2016 at 11:07:49AM +0200, Lars-Peter Clausen wrote:
> > On 07/29/2016 02:30 AM, Takashi Sakamoto wrote:
> > > Lars,
> > > 
> > > On Jul 29 2016 05:33, Lars-Peter Clausen wrote:
> > >> Hotplug is something that always pops up sooner or later. E.g. if someone
> > >> puts a ASoC supported CODEC on a hot-pluggable device (maybe USB) we
> > >> don't want to duplicate the code, but be able to reuse.
> > > 
> > > (A bit to sidetrack)
> > > 
> > > To me, it's unclear for devices on USB. When ALSA SoC part supports these
> > > devices, what is the scenario you assumed? In short, assuming we put codes
> > > to ALSA SoC part, what is the shape of the corresponding devices and links
> > > of pairs of endpoints? Additionally, in this case, what codes are able to 
> > > be
> > > reused?
> > 
> > Lets say you have USB stick with a small micro controller or FPGA which has
> > a USB interface on one side and a I2S and I2C interface on the other side.
> > The I2S and I2C are connected to a CODEC. I2S for data, I2C for control. If
> > the interface is implemented in a way so that it is just a simple USB to I2C
> > bridge, this means the raw I2C commands are send over the USB interface you
> > can implement a I2C adapter driver for this bridge. If you have that you can
> > instantiate the existing ASoC CODEC driver, which is a I2C device driver, on
> > the bus registered by the adapter.
> 
> That still seems a bit fancy hardware :)
> 
> If we can reasonably support this, I am for it. But not making stuff
> overtly complex...

Yes, we don't have to overreact to a dream immediately now.
We should consider whether it can be shifted to the card-level instead
before worrying too much about hot-unplug of an ASoC component.  Then
the whole story becomes far easier.

And, the suppress_bind_attrs flag I suggested is only to suppress the
sysfs bind/unbind, and doesn't mean to prohibit hotplug itself via the
bus driver.


Takashi


Re: [alsa-devel] Question about struct snd_soc_dai() :: cpu_dai->codec

2016-07-28 Thread Takashi Iwai
On Thu, 28 Jul 2016 22:33:31 +0200,
Lars-Peter Clausen wrote:
> 
> On 07/28/2016 05:46 AM, Vinod Koul wrote:
> > On Wed, Jul 27, 2016 at 10:22:41PM +0200, Takashi Iwai wrote:
> >> On Wed, 27 Jul 2016 20:22:33 +0200,
> >> Mark Brown wrote:
> >>>
> >>> On Wed, Jul 27, 2016 at 08:11:49PM +0200, Takashi Iwai wrote:
> >>>
> >>>> I'm wondering whether it's a better option to block the unbind
> >>>> behavior, either in driver base (allowing to return an error) or in
> >>>> the sound side (waiting in remove() until the sane point). 
> >>>
> >>> That's certainly going to be a lot easier and part of the reason it's
> >>> never been looked at much is that (unlike USB) there's very little
> >>> reason why an ASoC sound card would ever be hotplugged - even in
> >>> development these days the normal development flow involves rebooting.
> > 
> > I agree, it makese no sense for devices to be hotplugged. And for
> > developement flows people can do rmmond and insmod. That works fine!
> 
> I don't agree. In my opinion hot-plug is an essential feature of a
> modern device driver framework and if ASoC wants to claim to fall in
> this category we ought to support it. Hotplug is something that always
> pops up sooner or later. E.g. if someone puts a ASoC supported CODEC on
> a hot-pluggable device (maybe USB) we don't want to duplicate the code,
> but be able to reuse.
> 
> One area that e.g. requires hot-plug/-unplug and were ASoC supported
> devices are used is embedded development boards that support shields and
> devicetree overlays. Like e.g. RPI and similar.
> 
> The reason why we don't support hot-unplug in ASoC at the moment is
> because it is not trivial to implement and nobody has cared enough yet.
> 
> But if somebody wants to and has the resources to implement this we
> should not discourage this.
> 
> I'd very much prefer to have proper hot-plug support instead of
> prohibiting unbinding even on systems that do not require hot-plug
> support normally. It's a much cleaner solution.

Well, but hot-unplugging only a component like codec would be needed
in a real scenario?  It's a different story from the hotplug in the
card level.


Takashi


Re: [alsa-devel] Question about struct snd_soc_dai() :: cpu_dai->codec

2016-07-27 Thread Takashi Iwai
On Wed, 27 Jul 2016 20:22:33 +0200,
Mark Brown wrote:
> 
> On Wed, Jul 27, 2016 at 08:11:49PM +0200, Takashi Iwai wrote:
> 
> > I'm wondering whether it's a better option to block the unbind
> > behavior, either in driver base (allowing to return an error) or in
> > the sound side (waiting in remove() until the sane point). 
> 
> That's certainly going to be a lot easier and part of the reason it's
> never been looked at much is that (unlike USB) there's very little
> reason why an ASoC sound card would ever be hotplugged - even in
> development these days the normal development flow involves rebooting.

Actually there is already the suppress_bind_attr flag in struct
device_driver.  For a simple platform driver like snd-soc-rcar, it's
easy like:

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 3351a701c60e..d019824927de 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -1251,6 +1251,7 @@ static struct platform_driver rsnd_driver = {
.driver = {
.name   = "rcar_sound",
.of_match_table = rsnd_of_match,
+   .suppress_bind_attrs = true,
},
.probe  = rsnd_probe,
.remove = rsnd_remove,

Then there will be no sysfs bind/unbind for this driver.
(Note: totally untested: let me know if it really works.)

The same technique is likely available for i2c and spi codec drivers.
But it's another open question whether we should suppress the sysfs
bind/unbind of these devices at all.  My gut feeling is that sysfs
bind/unbind are mostly useless for drivers like ASoC codecs.  At
least, it would be much safer to disable for now.


Takashi


Re: [alsa-devel] Question about struct snd_soc_dai() :: cpu_dai->codec

2016-07-26 Thread Takashi Iwai
On Wed, 27 Jul 2016 05:21:11 +0200,
Vinod Koul wrote:
> 
> On Tue, Jul 26, 2016 at 05:41:56AM +, Kuninori Morimoto wrote:
> > 
> > Hi ALSA SoC
> > 
> > My current headache is ALSA SoC's each modules (= Card/Codec/CPU/Platform)
> > doesn't care about "unbind/rmmod".
> > For example, if someone unbinded/rmmoded "Codec", Card or other modules
> > doesn't know about it. Thus, user can continue to use this sound card,
> > and kernel will be Oops.
> 
> Are you sure about this? Have you tried removing a module?
> 
> During card probe, asoc will hold a reference to the component. See the
> calls to try_module_get(). This will prevent from unloading under normal
> cases.

For unloading the module, yes, it should have been prevented by
managing the module refcount.  However, unbinding can't be stopped by
that.  It's a known problem.

Morimoto-san, do you see the issue really via module unloading, or is
it only via unbinding?


Takashi


Re: [PATCH] media: add GFP flag to media_*() that could get called in atomic context

2016-03-14 Thread Takashi Iwai
On Mon, 14 Mar 2016 11:13:58 +0100,
Mauro Carvalho Chehab wrote:
> 
> Em Mon, 14 Mar 2016 09:22:37 +0200
> Sakari Ailus  escreveu:
> 
> > Hi Shuah,
> > 
> > On Sat, Mar 12, 2016 at 06:48:09PM -0700, Shuah Khan wrote:
> > > Add GFP flags to media_create_pad_link(), media_create_intf_link(),
> > > media_devnode_create(), and media_add_link() that could get called
> > > in atomic context to allow callers to pass in the right flags for
> > > memory allocation.
> > > 
> > > tree-wide driver changes for media_*() GFP flags change:
> > > Change drivers to add gfpflags to interffaces, media_create_pad_link(),
> > > media_create_intf_link() and media_devnode_create().
> > > 
> > > Signed-off-by: Shuah Khan 
> > > Suggested-by: Mauro Carvalho Chehab   
> > 
> > What's the use case for calling the above functions in an atomic context?
> > 
> 
> ALSA code seems to do a lot of stuff at atomic context. That's what
> happens on my test machine when au0828 gets probed before
> snd-usb-audio:
>   http://pastebin.com/LEX5LD5K
> 
> It seems that ALSA USB probe routine (usb_audio_probe) happens in
> atomic context.

No, usb_audio_probe() doesn't take a lock.  But
media_device_register_entity() seems taking a spinlock, instead.


Takashi