| Message ID | 20170712155530.17765-2-johan@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 651e9268fb9b9944e063d731b09c0d2ad339bedb |
| Headers | show |
On Wed, Jul 12, 2017 at 5:55 PM, Johan Hovold <johan@kernel.org> wrote: > This reverts commit f1013cdeeeb9 ("ASoC: ux500: drop platform DAI > assignments"), which seems to have been based on a misunderstanding and > prevents the platform driver callbacks from being made (e.g. to > preallocate DMA memory). > > The real culprit for the warnings about attempts to create duplicate > procfs entries was commit 99b04f4c4051 ("ASoC: add Component level > pcm_new/pcm_free" that broke PCM creation on systems that use more than > one platform component. > > Fixes: f1013cdeeeb9 ("ASoC: ux500: drop platform DAI assignments") > Cc: stable <stable@vger.kernel.org> # 4.11 > Cc: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Johan Hovold <johan@kernel.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Tested-by: Linus Walleij <linus.walleij@linaro.org> These static assignments should go away, but not for the wrong reason. So this patch is fully in order given the source of the bug. Yours, Linus Walleij
On Fri, Jul 14, 2017 at 03:36:57PM +0200, Linus Walleij wrote: > On Wed, Jul 12, 2017 at 5:55 PM, Johan Hovold <johan@kernel.org> wrote: > > > This reverts commit f1013cdeeeb9 ("ASoC: ux500: drop platform DAI > > assignments"), which seems to have been based on a misunderstanding and > > prevents the platform driver callbacks from being made (e.g. to > > preallocate DMA memory). > > > > The real culprit for the warnings about attempts to create duplicate > > procfs entries was commit 99b04f4c4051 ("ASoC: add Component level > > pcm_new/pcm_free" that broke PCM creation on systems that use more than > > one platform component. > > > > Fixes: f1013cdeeeb9 ("ASoC: ux500: drop platform DAI assignments") > > Cc: stable <stable@vger.kernel.org> # 4.11 > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Johan Hovold <johan@kernel.org> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Tested-by: Linus Walleij <linus.walleij@linaro.org> > > These static assignments should go away, but not for the wrong reason. > So this patch is fully in order given the source of the bug. I assume you'll still need the of-related bits though (platform_of_node) even if you eventually make this driver use DT-instantiation only. Thanks again, Johan
On Wed, Jul 12, 2017 at 05:55:30PM +0200, Johan Hovold wrote: > This reverts commit f1013cdeeeb9 ("ASoC: ux500: drop platform DAI > assignments"), which seems to have been based on a misunderstanding and > prevents the platform driver callbacks from being made (e.g. to > preallocate DMA memory). Please submit patches using subject lines reflecting the style for the subsystem. This makes it easier for people to identify relevant patches. Look at what existing commits in the area you're changing are doing and make sure your subject lines visually resemble what they're doing.
On Mon, Jul 17, 2017 at 03:51:27PM +0100, Mark Brown wrote: > On Wed, Jul 12, 2017 at 05:55:30PM +0200, Johan Hovold wrote: > > This reverts commit f1013cdeeeb9 ("ASoC: ux500: drop platform DAI > > assignments"), which seems to have been based on a misunderstanding and > > prevents the platform driver callbacks from being made (e.g. to > > preallocate DMA memory). > > Please submit patches using subject lines reflecting the style for the > subsystem. This makes it easier for people to identify relevant > patches. Look at what existing commits in the area you're changing are > doing and make sure your subject lines visually resemble what they're > doing. I try to, but reverts are special as the default commit summary tend to already contain the subsystem prefix and some maintainers find that sufficient (or even preferred as this also makes reverts stand out more clearly). But now I know your preference, and thanks for fixing it up this time before applying. Johan
On Tue, Jul 18, 2017 at 10:21:18AM +0200, Johan Hovold wrote: > On Mon, Jul 17, 2017 at 03:51:27PM +0100, Mark Brown wrote: > > Please submit patches using subject lines reflecting the style for the > > subsystem. This makes it easier for people to identify relevant > > patches. Look at what existing commits in the area you're changing are > > doing and make sure your subject lines visually resemble what they're > > doing. > I try to, but reverts are special as the default commit summary tend to > already contain the subsystem prefix and some maintainers find that > sufficient (or even preferred as this also makes reverts stand out more > clearly). Reverts shouldn't be special - they're just regular patches and should have sensible changelogs like any others.
On Tue, Jul 18, 2017 at 11:06:55AM +0100, Mark Brown wrote: > On Tue, Jul 18, 2017 at 10:21:18AM +0200, Johan Hovold wrote: > > On Mon, Jul 17, 2017 at 03:51:27PM +0100, Mark Brown wrote: > > > > Please submit patches using subject lines reflecting the style for the > > > subsystem. This makes it easier for people to identify relevant > > > patches. Look at what existing commits in the area you're changing are > > > doing and make sure your subject lines visually resemble what they're > > > doing. > > > I try to, but reverts are special as the default commit summary tend to > > already contain the subsystem prefix and some maintainers find that > > sufficient (or even preferred as this also makes reverts stand out more > > clearly). > > Reverts shouldn't be special - they're just regular patches and should > have sensible changelogs like any others. Stating that you're reverting a commit and which commit that is is in the summary is arguable sensible (of course, you still also need further details in the commit message body itself describing why it was needed). Check the logs and you'll see that we have a ton of "Revert <reverted commit summary>" for various subsystems. In fact, it seems to be by far the most common summary for direct reverts. But again, now I know your preference. Johan
On Tue, Jul 18, 2017 at 12:36:29PM +0200, Johan Hovold wrote: > On Tue, Jul 18, 2017 at 11:06:55AM +0100, Mark Brown wrote: > > Reverts shouldn't be special - they're just regular patches and should > > have sensible changelogs like any others. > Stating that you're reverting a commit and which commit that is is in > the summary is arguable sensible (of course, you still also need further > details in the commit message body itself describing why it was needed). > Check the logs and you'll see that we have a ton of "Revert <reverted > commit summary>" for various subsystems. In fact, it seems to be by far > the most common summary for direct reverts. The easily findable ones are, and it doesn't mean it's good practice - reverts seem to attract particularly bad commit messages in general, not just the subject lines, and I happen to have a pre-canned response for this so...
diff --git a/sound/soc/ux500/mop500.c b/sound/soc/ux500/mop500.c index b50f68a439ce..ba9fc099cf67 100644 --- a/sound/soc/ux500/mop500.c +++ b/sound/soc/ux500/mop500.c @@ -33,6 +33,7 @@ static struct snd_soc_dai_link mop500_dai_links[] = { .stream_name = "ab8500_0", .cpu_dai_name = "ux500-msp-i2s.1", .codec_dai_name = "ab8500-codec-dai.0", + .platform_name = "ux500-msp-i2s.1", .codec_name = "ab8500-codec.0", .init = mop500_ab8500_machine_init, .ops = mop500_ab8500_ops, @@ -42,6 +43,7 @@ static struct snd_soc_dai_link mop500_dai_links[] = { .stream_name = "ab8500_1", .cpu_dai_name = "ux500-msp-i2s.3", .codec_dai_name = "ab8500-codec-dai.1", + .platform_name = "ux500-msp-i2s.3", .codec_name = "ab8500-codec.0", .init = NULL, .ops = mop500_ab8500_ops, @@ -85,6 +87,8 @@ static int mop500_of_probe(struct platform_device *pdev, for (i = 0; i < 2; i++) { mop500_dai_links[i].cpu_of_node = msp_np[i]; mop500_dai_links[i].cpu_dai_name = NULL; + mop500_dai_links[i].platform_of_node = msp_np[i]; + mop500_dai_links[i].platform_name = NULL; mop500_dai_links[i].codec_of_node = codec_np; mop500_dai_links[i].codec_name = NULL; }
This reverts commit f1013cdeeeb9 ("ASoC: ux500: drop platform DAI assignments"), which seems to have been based on a misunderstanding and prevents the platform driver callbacks from being made (e.g. to preallocate DMA memory). The real culprit for the warnings about attempts to create duplicate procfs entries was commit 99b04f4c4051 ("ASoC: add Component level pcm_new/pcm_free" that broke PCM creation on systems that use more than one platform component. Fixes: f1013cdeeeb9 ("ASoC: ux500: drop platform DAI assignments") Cc: stable <stable@vger.kernel.org> # 4.11 Cc: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Johan Hovold <johan@kernel.org> --- sound/soc/ux500/mop500.c | 4 ++++ 1 file changed, 4 insertions(+)