Re: [PATCH 2/2] Revert "ASoC: ux500: drop platform DAI assignments"
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 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... signature.asc Description: PGP signature
Re: [PATCH 2/2] Revert "ASoC: ux500: drop platform DAI assignments"
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 " 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
Re: [PATCH 2/2] Revert "ASoC: ux500: drop platform DAI assignments"
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. signature.asc Description: PGP signature
Re: [PATCH 2/2] Revert "ASoC: ux500: drop platform DAI assignments"
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
Re: [PATCH 2/2] Revert "ASoC: ux500: drop platform DAI assignments"
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.
signature.asc
Description: PGP signature
Re: [PATCH 2/2] Revert "ASoC: ux500: drop platform DAI assignments"
On Fri, Jul 14, 2017 at 03:36:57PM +0200, Linus Walleij wrote:
> On Wed, Jul 12, 2017 at 5:55 PM, 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).
> >
> > 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 # 4.11
> > Cc: Linus Walleij
> > Signed-off-by: Johan Hovold
>
> Reviewed-by: Linus Walleij
> Tested-by: Linus Walleij
>
> 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
Re: [PATCH 2/2] Revert "ASoC: ux500: drop platform DAI assignments"
On Wed, Jul 12, 2017 at 5:55 PM, 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).
>
> 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 # 4.11
> Cc: Linus Walleij
> Signed-off-by: Johan Hovold
Reviewed-by: Linus Walleij
Tested-by: Linus Walleij
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
