| Message ID | 20190216100927.10017-1-daniel.baluta@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 461d854c0dba3cdf63cce37ffb9423eca0793a47 |
| Headers | show |
| Series | ASoC: simple-card: Fix refcount underflow | expand |
Hi Daniel, Mark, again > > of_get_child_by_name() takes a reference we'll need to drop > > later so when we substitute in top we need to take a reference > > as well as just assigning. > > > > Without this patch we hit the following error: > > > > [ 1.246852] OF: ERROR: Bad of_node_put() on /sound-wm8524 > > [ 1.262261] Hardware name: NXP i.MX8MQ EVK (DT) > > [ 1.266807] Workqueue: events deferred_probe_work_func > > [ 1.271950] Call trace: > > [ 1.274406] dump_backtrace+0x0/0x158 > > [ 1.278074] show_stack+0x14/0x20 > > [ 1.281396] dump_stack+0xa8/0xcc > > [ 1.284717] of_node_release+0xb0/0xc8 > > [ 1.288474] kobject_put+0x74/0xf0 > > [ 1.291879] of_node_put+0x14/0x28 > > [ 1.295286] __of_get_next_child+0x44/0x70 > > [ 1.299387] of_get_next_child+0x3c/0x60 > > [ 1.303315] simple_for_each_link+0x1dc/0x230 > > [ 1.307676] simple_probe+0x80/0x540 > > [ 1.311256] platform_drv_probe+0x50/0xa0 > > > > This patch is based on an earlier version posted by Kuninori Morimoto > > and commit message includes explanations from Mark Brown. > > > > https://patchwork.kernel.org/patch/10814255/ > > > > Reported-by: Vicente Bergas <vicencb@gmail.com> > > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> > > --- > > I'm not sure which one is correct in this case > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > or > > Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> And we want to add Fixes: commit da215354eb55c ("ASoC: simple-card: merge simple-scu-card")
On Mon, 2019-02-18 at 10:37 +0900, Kuninori Morimoto wrote: > Hi Daniel, Mark, again > > > > of_get_child_by_name() takes a reference we'll need to drop > > > later so when we substitute in top we need to take a reference > > > as well as just assigning. > > > > > > Without this patch we hit the following error: > > > > > > [ 1.246852] OF: ERROR: Bad of_node_put() on /sound-wm8524 > > > [ 1.262261] Hardware name: NXP i.MX8MQ EVK (DT) > > > [ 1.266807] Workqueue: events deferred_probe_work_func > > > [ 1.271950] Call trace: > > > [ 1.274406] dump_backtrace+0x0/0x158 > > > [ 1.278074] show_stack+0x14/0x20 > > > [ 1.281396] dump_stack+0xa8/0xcc > > > [ 1.284717] of_node_release+0xb0/0xc8 > > > [ 1.288474] kobject_put+0x74/0xf0 > > > [ 1.291879] of_node_put+0x14/0x28 > > > [ 1.295286] __of_get_next_child+0x44/0x70 > > > [ 1.299387] of_get_next_child+0x3c/0x60 > > > [ 1.303315] simple_for_each_link+0x1dc/0x230 > > > [ 1.307676] simple_probe+0x80/0x540 > > > [ 1.311256] platform_drv_probe+0x50/0xa0 > > > > > > This patch is based on an earlier version posted by Kuninori > > > Morimoto > > > and commit message includes explanations from Mark Brown. > > > > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fpatch%2F10814255%2F&data=02%7C01%7Cdaniel.baluta%40nxp.com%7Ced1b7adc66a546c1bb4608d69541aaf8%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636860506617983174&sdata=hTsurDLdtLAtTw0a5v%2FrSsleSawAP2yiVPl87%2BHCzTI%3D&reserved=0 > > > > > > Reported-by: Vicente Bergas <vicencb@gmail.com> > > > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> > > > --- > > > > I'm not sure which one is correct in this case > > > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > or > > > > Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > And we want to add > > Fixes: commit da215354eb55c ("ASoC: simple-card: merge simple-scu- > card") > Thanks! I thought about adding this but the current patch will not cleanly apply because of: 2d01a84605a55c ("ASoC: simple-card: reduce naming prefix")
On Mon, Feb 18, 2019 at 08:52:26AM +0100, Takashi Iwai wrote: > Daniel Baluta wrote: > > > Fixes: commit da215354eb55c ("ASoC: simple-card: merge simple-scu- > > > card") > > Thanks! I thought about adding this but the current patch will not > > cleanly apply because of: > > 2d01a84605a55c ("ASoC: simple-card: reduce naming prefix") That doesn't matter for fixes tags. However... > Yeah, it's a bit tricky because of the code change at this time. > At best, send two patches, once for 5.0 and once for 5.1, and let Mark > apply both of them and do the empty merge from 5.0 to 5.1, or let him > resolve the merge conflict by applying manually the 5.1 patch. ...since this is something that needs to be fixed for v5.0 we need a patch that can be applied on v5.0 (and has been tested by someone who sees the issue there). Takashi's plan seems like a good way to do that, if you could send a patch against my for-5.0 branch (or just Linus' tree) that'd be great.
On Mon, Feb 18, 2019 at 7:50 PM Mark Brown <broonie@kernel.org> wrote: > > On Mon, Feb 18, 2019 at 08:52:26AM +0100, Takashi Iwai wrote: > > Daniel Baluta wrote: > > > > > Fixes: commit da215354eb55c ("ASoC: simple-card: merge simple-scu- > > > > card") > > > > Thanks! I thought about adding this but the current patch will not > > > cleanly apply because of: > > > > 2d01a84605a55c ("ASoC: simple-card: reduce naming prefix") > > That doesn't matter for fixes tags. However... > > > Yeah, it's a bit tricky because of the code change at this time. > > > At best, send two patches, once for 5.0 and once for 5.1, and let Mark > > apply both of them and do the empty merge from 5.0 to 5.1, or let him > > resolve the merge conflict by applying manually the 5.1 patch. > > ...since this is something that needs to be fixed for v5.0 we need a > patch that can be applied on v5.0 (and has been tested by someone who > sees the issue there). Takashi's plan seems like a good way to do that, > if you could send a patch against my for-5.0 branch (or just Linus' > tree) that'd be great. for-5.0 branch Kuninori's patch works fine: https://patchwork.kernel.org/patch/10814255/ for-5.1 branch and later my patch works fine: https://patchwork.kernel.org/patch/10817449/ Would you want me to change the patch for 5.0 to have an identical commit message as the one for 5.1?
On Mon, Feb 18, 2019 at 08:00:48PM +0200, Daniel Baluta wrote: > Would you want me to change the patch for 5.0 to have an identical commit > message as the one for 5.1? No, that's fine - since Morimoto-san's patch has been tested I'll just use that.
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 08df261024cf..dc18c4492955 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -445,7 +445,7 @@ static int simple_for_each_link(struct simple_priv *priv, /* Check if it has dai-link */ node = of_get_child_by_name(top, PREFIX "dai-link"); if (!node) { - node = top; + node = of_node_get(top); is_top = 1; }
of_get_child_by_name() takes a reference we'll need to drop later so when we substitute in top we need to take a reference as well as just assigning. Without this patch we hit the following error: [ 1.246852] OF: ERROR: Bad of_node_put() on /sound-wm8524 [ 1.262261] Hardware name: NXP i.MX8MQ EVK (DT) [ 1.266807] Workqueue: events deferred_probe_work_func [ 1.271950] Call trace: [ 1.274406] dump_backtrace+0x0/0x158 [ 1.278074] show_stack+0x14/0x20 [ 1.281396] dump_stack+0xa8/0xcc [ 1.284717] of_node_release+0xb0/0xc8 [ 1.288474] kobject_put+0x74/0xf0 [ 1.291879] of_node_put+0x14/0x28 [ 1.295286] __of_get_next_child+0x44/0x70 [ 1.299387] of_get_next_child+0x3c/0x60 [ 1.303315] simple_for_each_link+0x1dc/0x230 [ 1.307676] simple_probe+0x80/0x540 [ 1.311256] platform_drv_probe+0x50/0xa0 This patch is based on an earlier version posted by Kuninori Morimoto and commit message includes explanations from Mark Brown. https://patchwork.kernel.org/patch/10814255/ Reported-by: Vicente Bergas <vicencb@gmail.com> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> --- sound/soc/generic/simple-card.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)