[PATCH] mmc: tegra: fix SDR50 tuning override

[PATCH] mmc: tegra: fix SDR50 tuning override

[Michał Mirosław]

Commit 7ad2ed1dfcbe inadvertently mixed up a quirk flag's name and
broke SDR50 tuning override. Use correct NVQUIRK_ name.

Fixes: 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes")
Depends-on: 4f6aa3264af4 ("mmc: tegra: Only advertise UHS modes if IO regulator is present")
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/mmc/host/sdhci-tegra.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 7bc950520fd9..403ac44a7378 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -386,7 +386,7 @@ static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
 			misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_DDR50;
 		if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR104)
 			misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDR104;
-		if (soc_data->nvquirks & SDHCI_MISC_CTRL_ENABLE_SDR50)
+		if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR50)
 			clk_ctrl |= SDHCI_CLOCK_CTRL_SDR50_TUNING_OVERRIDE;
 	}
 
-- 
2.20.1

Re: [PATCH] mmc: tegra: fix SDR50 tuning override

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1796 bytes --]

On Thu, Jan 02, 2020 at 11:30:50AM +0100, Michał Mirosław wrote:
> Commit 7ad2ed1dfcbe inadvertently mixed up a quirk flag's name and
> broke SDR50 tuning override. Use correct NVQUIRK_ name.
> 
> Fixes: 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes")
> Depends-on: 4f6aa3264af4 ("mmc: tegra: Only advertise UHS modes if IO regulator is present")
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
>  drivers/mmc/host/sdhci-tegra.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Oh my... good catch!

Reviewed-by: Thierry Reding <treding@nvidia.com>

I also ran this through our internal test system and all tests pass, so
also:

Tested-by: Thierry Reding <treding@nvidia.com>

I'm not sure if that "Depends-on:" tag is anything that's recognized
anywhere. It might be better to turn that into an additional "Fixes:"
line. Adding Greg to see if he has a standard way of dealing with this
kind of dependency.

Greg, what's your preferred way to handle these situations? I think the
intention here was to describe that the original error was introduced by
commit 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes"), but then commit
4f6aa3264af4 ("mmc: tegra: Only advertise UHS modes if IO regulator is
present") moved that code around, so this patch here will only be back-
portable until the latter commit, but should be backported until the
former.

I'm not sure it matters in this case because the latter went into v4.5
and the former went into v4.8, so we don't need this in v4.4 and v4.9
should already have all the prerequisites for this. But in general, do
you need anything special to model such dependencies, or is this simply
solved by requiring a manual backport if the fix stops applying cleanly
at some point?

Thierry

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

Re: [PATCH] mmc: tegra: fix SDR50 tuning override

[Greg Kroah-Hartman]

On Mon, Jan 06, 2020 at 01:07:18PM +0100, Thierry Reding wrote:
> On Thu, Jan 02, 2020 at 11:30:50AM +0100, Michał Mirosław wrote:
> > Commit 7ad2ed1dfcbe inadvertently mixed up a quirk flag's name and
> > broke SDR50 tuning override. Use correct NVQUIRK_ name.
> > 
> > Fixes: 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes")
> > Depends-on: 4f6aa3264af4 ("mmc: tegra: Only advertise UHS modes if IO regulator is present")
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > ---
> >  drivers/mmc/host/sdhci-tegra.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Oh my... good catch!
> 
> Reviewed-by: Thierry Reding <treding@nvidia.com>
> 
> I also ran this through our internal test system and all tests pass, so
> also:
> 
> Tested-by: Thierry Reding <treding@nvidia.com>
> 
> I'm not sure if that "Depends-on:" tag is anything that's recognized
> anywhere. It might be better to turn that into an additional "Fixes:"
> line. Adding Greg to see if he has a standard way of dealing with this
> kind of dependency.
> 
> Greg, what's your preferred way to handle these situations? I think the
> intention here was to describe that the original error was introduced by
> commit 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes"), but then commit
> 4f6aa3264af4 ("mmc: tegra: Only advertise UHS modes if IO regulator is
> present") moved that code around, so this patch here will only be back-
> portable until the latter commit, but should be backported until the
> former.

The stable kernel rules document says how to handle this, but the
"depends on" commit id in the comment to the right of the stable@k.o cc:
line in the changelog area.

thanks,

greg k-h

Re: [PATCH] mmc: tegra: fix SDR50 tuning override

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2617 bytes --]

On Mon, Jan 06, 2020 at 01:27:45PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Jan 06, 2020 at 01:07:18PM +0100, Thierry Reding wrote:
> > On Thu, Jan 02, 2020 at 11:30:50AM +0100, Michał Mirosław wrote:
> > > Commit 7ad2ed1dfcbe inadvertently mixed up a quirk flag's name and
> > > broke SDR50 tuning override. Use correct NVQUIRK_ name.
> > > 
> > > Fixes: 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes")
> > > Depends-on: 4f6aa3264af4 ("mmc: tegra: Only advertise UHS modes if IO regulator is present")
> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > ---
> > >  drivers/mmc/host/sdhci-tegra.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Oh my... good catch!
> > 
> > Reviewed-by: Thierry Reding <treding@nvidia.com>
> > 
> > I also ran this through our internal test system and all tests pass, so
> > also:
> > 
> > Tested-by: Thierry Reding <treding@nvidia.com>
> > 
> > I'm not sure if that "Depends-on:" tag is anything that's recognized
> > anywhere. It might be better to turn that into an additional "Fixes:"
> > line. Adding Greg to see if he has a standard way of dealing with this
> > kind of dependency.
> > 
> > Greg, what's your preferred way to handle these situations? I think the
> > intention here was to describe that the original error was introduced by
> > commit 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes"), but then commit
> > 4f6aa3264af4 ("mmc: tegra: Only advertise UHS modes if IO regulator is
> > present") moved that code around, so this patch here will only be back-
> > portable until the latter commit, but should be backported until the
> > former.
> 
> The stable kernel rules document says how to handle this, but the
> "depends on" commit id in the comment to the right of the stable@k.o cc:
> line in the changelog area.

That only mentions "static" prerequisites needed by the patch, but what
if the prerequisites change depending on version?

Could I do something like this:

	Cc: <stable@vger.kernel.org> # 4.4.x: abcdef: ...
	Cc: <stable@vger.kernel.org> # 4.9.x: bcdefa: ...
	Cc: <stable@vger.kernel.org>

Would that mean that the patch is selected for all stable releases
(because of the last line with no version prerequisite) but when applied
for stable-4.4 the abcdef patch gets pulled in and for stable-4.9 the
bcdefa dependency is applied before the patch?

I suppose this is perhaps a bit of an exotic case, but it might be good
to document it specifically because it might be fairly rare. I can draft
a change if you think this is useful to add.

Thierry

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

Re: [PATCH] mmc: tegra: fix SDR50 tuning override

[Greg Kroah-Hartman]

On Mon, Jan 06, 2020 at 02:37:03PM +0100, Thierry Reding wrote:
> On Mon, Jan 06, 2020 at 01:27:45PM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Jan 06, 2020 at 01:07:18PM +0100, Thierry Reding wrote:
> > > On Thu, Jan 02, 2020 at 11:30:50AM +0100, Michał Mirosław wrote:
> > > > Commit 7ad2ed1dfcbe inadvertently mixed up a quirk flag's name and
> > > > broke SDR50 tuning override. Use correct NVQUIRK_ name.
> > > > 
> > > > Fixes: 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes")
> > > > Depends-on: 4f6aa3264af4 ("mmc: tegra: Only advertise UHS modes if IO regulator is present")
> > > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > > ---
> > > >  drivers/mmc/host/sdhci-tegra.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > Oh my... good catch!
> > > 
> > > Reviewed-by: Thierry Reding <treding@nvidia.com>
> > > 
> > > I also ran this through our internal test system and all tests pass, so
> > > also:
> > > 
> > > Tested-by: Thierry Reding <treding@nvidia.com>
> > > 
> > > I'm not sure if that "Depends-on:" tag is anything that's recognized
> > > anywhere. It might be better to turn that into an additional "Fixes:"
> > > line. Adding Greg to see if he has a standard way of dealing with this
> > > kind of dependency.
> > > 
> > > Greg, what's your preferred way to handle these situations? I think the
> > > intention here was to describe that the original error was introduced by
> > > commit 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes"), but then commit
> > > 4f6aa3264af4 ("mmc: tegra: Only advertise UHS modes if IO regulator is
> > > present") moved that code around, so this patch here will only be back-
> > > portable until the latter commit, but should be backported until the
> > > former.
> > 
> > The stable kernel rules document says how to handle this, but the
> > "depends on" commit id in the comment to the right of the stable@k.o cc:
> > line in the changelog area.
> 
> That only mentions "static" prerequisites needed by the patch, but what
> if the prerequisites change depending on version?
> 
> Could I do something like this:
> 
> 	Cc: <stable@vger.kernel.org> # 4.4.x: abcdef: ...
> 	Cc: <stable@vger.kernel.org> # 4.9.x: bcdefa: ...
> 	Cc: <stable@vger.kernel.org>

Yes.

> Would that mean that the patch is selected for all stable releases
> (because of the last line with no version prerequisite) but when applied
> for stable-4.4 the abcdef patch gets pulled in and for stable-4.9 the
> bcdefa dependency is applied before the patch?

Yes.

> I suppose this is perhaps a bit of an exotic case, but it might be good
> to document it specifically because it might be fairly rare. I can draft
> a change if you think this is useful to add.

I thought this was already in there, as others have done it in the past.

It's a _very_ exotic case, I wouldn't worry about it, just document it
like this, and if I have problems applying the patches to stable I'll be
sure to let you know and you can always tell me then.  That's usually
the easiest thing to do anyway :)

thanks,

greg k-h

Re: [PATCH] mmc: tegra: fix SDR50 tuning override

[Michał Mirosław]

On Tue, Jan 07, 2020 at 10:37:15AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Jan 06, 2020 at 02:37:03PM +0100, Thierry Reding wrote:
> > On Mon, Jan 06, 2020 at 01:27:45PM +0100, Greg Kroah-Hartman wrote:
> > > On Mon, Jan 06, 2020 at 01:07:18PM +0100, Thierry Reding wrote:
> > > > On Thu, Jan 02, 2020 at 11:30:50AM +0100, Michał Mirosław wrote:
> > > > > Commit 7ad2ed1dfcbe inadvertently mixed up a quirk flag's name and
> > > > > broke SDR50 tuning override. Use correct NVQUIRK_ name.
> > > > > 
> > > > > Fixes: 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes")
> > > > > Depends-on: 4f6aa3264af4 ("mmc: tegra: Only advertise UHS modes if IO regulator is present")
> > > > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > > > ---
> > > > >  drivers/mmc/host/sdhci-tegra.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > Oh my... good catch!
> > > > 
> > > > Reviewed-by: Thierry Reding <treding@nvidia.com>
> > > > 
> > > > I also ran this through our internal test system and all tests pass, so
> > > > also:
> > > > 
> > > > Tested-by: Thierry Reding <treding@nvidia.com>
> > > > 
> > > > I'm not sure if that "Depends-on:" tag is anything that's recognized
> > > > anywhere. It might be better to turn that into an additional "Fixes:"
> > > > line. Adding Greg to see if he has a standard way of dealing with this
> > > > kind of dependency.
> > > > 
> > > > Greg, what's your preferred way to handle these situations? I think the
> > > > intention here was to describe that the original error was introduced by
> > > > commit 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes"), but then commit
> > > > 4f6aa3264af4 ("mmc: tegra: Only advertise UHS modes if IO regulator is
> > > > present") moved that code around, so this patch here will only be back-
> > > > portable until the latter commit, but should be backported until the
> > > > former.
> > > 
> > > The stable kernel rules document says how to handle this, but the
> > > "depends on" commit id in the comment to the right of the stable@k.o cc:
> > > line in the changelog area.
> > 
> > That only mentions "static" prerequisites needed by the patch, but what
> > if the prerequisites change depending on version?
> > 
> > Could I do something like this:
> > 
> > 	Cc: <stable@vger.kernel.org> # 4.4.x: abcdef: ...
> > 	Cc: <stable@vger.kernel.org> # 4.9.x: bcdefa: ...
> > 	Cc: <stable@vger.kernel.org>
> 
> Yes.
> 
> > Would that mean that the patch is selected for all stable releases
> > (because of the last line with no version prerequisite) but when applied
> > for stable-4.4 the abcdef patch gets pulled in and for stable-4.9 the
> > bcdefa dependency is applied before the patch?
> 
> Yes.
> 
> > I suppose this is perhaps a bit of an exotic case, but it might be good
> > to document it specifically because it might be fairly rare. I can draft
> > a change if you think this is useful to add.
> 
> I thought this was already in there, as others have done it in the past.
> 
> It's a _very_ exotic case, I wouldn't worry about it, just document it
> like this, and if I have problems applying the patches to stable I'll be
> sure to let you know and you can always tell me then.  That's usually
> the easiest thing to do anyway :)

I understood the wording in stable-kernel-rules.rst as meaning that
comments on Cc: lines make mentioned commit pulled in (cherry-picked).
In this case I think this is ok, but in case the pulled-in patch changes
something else (the dependency is only because of touching nearby code),
how would I specify this and avoid the hint to include the other patch?

Best Regards,
Michał Mirosław

Re: [PATCH] mmc: tegra: fix SDR50 tuning override

[Greg Kroah-Hartman]

On Tue, Jan 07, 2020 at 10:53:59AM +0100, Michał Mirosław wrote:
> On Tue, Jan 07, 2020 at 10:37:15AM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Jan 06, 2020 at 02:37:03PM +0100, Thierry Reding wrote:
> > > On Mon, Jan 06, 2020 at 01:27:45PM +0100, Greg Kroah-Hartman wrote:
> > > > On Mon, Jan 06, 2020 at 01:07:18PM +0100, Thierry Reding wrote:
> > > > > On Thu, Jan 02, 2020 at 11:30:50AM +0100, Michał Mirosław wrote:
> > > > > > Commit 7ad2ed1dfcbe inadvertently mixed up a quirk flag's name and
> > > > > > broke SDR50 tuning override. Use correct NVQUIRK_ name.
> > > > > > 
> > > > > > Fixes: 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes")
> > > > > > Depends-on: 4f6aa3264af4 ("mmc: tegra: Only advertise UHS modes if IO regulator is present")
> > > > > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > > > > ---
> > > > > >  drivers/mmc/host/sdhci-tegra.c | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > Oh my... good catch!
> > > > > 
> > > > > Reviewed-by: Thierry Reding <treding@nvidia.com>
> > > > > 
> > > > > I also ran this through our internal test system and all tests pass, so
> > > > > also:
> > > > > 
> > > > > Tested-by: Thierry Reding <treding@nvidia.com>
> > > > > 
> > > > > I'm not sure if that "Depends-on:" tag is anything that's recognized
> > > > > anywhere. It might be better to turn that into an additional "Fixes:"
> > > > > line. Adding Greg to see if he has a standard way of dealing with this
> > > > > kind of dependency.
> > > > > 
> > > > > Greg, what's your preferred way to handle these situations? I think the
> > > > > intention here was to describe that the original error was introduced by
> > > > > commit 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes"), but then commit
> > > > > 4f6aa3264af4 ("mmc: tegra: Only advertise UHS modes if IO regulator is
> > > > > present") moved that code around, so this patch here will only be back-
> > > > > portable until the latter commit, but should be backported until the
> > > > > former.
> > > > 
> > > > The stable kernel rules document says how to handle this, but the
> > > > "depends on" commit id in the comment to the right of the stable@k.o cc:
> > > > line in the changelog area.
> > > 
> > > That only mentions "static" prerequisites needed by the patch, but what
> > > if the prerequisites change depending on version?
> > > 
> > > Could I do something like this:
> > > 
> > > 	Cc: <stable@vger.kernel.org> # 4.4.x: abcdef: ...
> > > 	Cc: <stable@vger.kernel.org> # 4.9.x: bcdefa: ...
> > > 	Cc: <stable@vger.kernel.org>
> > 
> > Yes.
> > 
> > > Would that mean that the patch is selected for all stable releases
> > > (because of the last line with no version prerequisite) but when applied
> > > for stable-4.4 the abcdef patch gets pulled in and for stable-4.9 the
> > > bcdefa dependency is applied before the patch?
> > 
> > Yes.
> > 
> > > I suppose this is perhaps a bit of an exotic case, but it might be good
> > > to document it specifically because it might be fairly rare. I can draft
> > > a change if you think this is useful to add.
> > 
> > I thought this was already in there, as others have done it in the past.
> > 
> > It's a _very_ exotic case, I wouldn't worry about it, just document it
> > like this, and if I have problems applying the patches to stable I'll be
> > sure to let you know and you can always tell me then.  That's usually
> > the easiest thing to do anyway :)
> 
> I understood the wording in stable-kernel-rules.rst as meaning that
> comments on Cc: lines make mentioned commit pulled in (cherry-picked).
> In this case I think this is ok, but in case the pulled-in patch changes
> something else (the dependency is only because of touching nearby code),
> how would I specify this and avoid the hint to include the other patch?

I really do not understand what you are asking for here.

Worst case, just say:
	cc: stable... # 4.4.x

and if you know that fails to apply or build, then just wait for my
email saying something failed and then respond with the needed commit
ids or backported patches.

It's not rocket science, and this isn't all that automated, I _can_
handle free-form text :)

thanks,

greg k-h