mbox series

[GIT,PULL] MMC fixes for v5.13-rc8

Message ID 20210624123251.191299-1-ulf.hansson@linaro.org (mailing list archive)
State New, archived
Headers show
Series [GIT,PULL] MMC fixes for v5.13-rc8 | expand

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git tags/mmc-v5.13-rc6

Message

Ulf Hansson June 24, 2021, 12:32 p.m. UTC
Hi Linus,

Here's a PR with an MMC fix intended for v5.13-rc8. Details about the
highlights are as usual found in the signed tag.

Please pull this in!

Kind regards
Ulf Hansson


The following changes since commit 009c9aa5be652675a06d5211e1640e02bbb1c33d:

  Linux 5.13-rc6 (2021-06-13 14:43:10 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git tags/mmc-v5.13-rc6

for you to fetch changes up to 103a5348c22c3fca8b96c735a9e353b8a0801842:

  mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk (2021-06-14 14:02:33 +0200)

----------------------------------------------------------------
MMC host:
 - meson-gx: Use memcpy_to/fromio for dram-access-quirk

----------------------------------------------------------------
Neil Armstrong (1):
      mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk

 drivers/mmc/host/meson-gx-mmc.c | 50 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 5 deletions(-)

Comments

Linus Torvalds June 24, 2021, 6 p.m. UTC | #1
On Thu, Jun 24, 2021 at 5:32 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> Neil Armstrong (1):
>       mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk

Ugh. How horribly ugly.

Why is that 'host->dram_access_quirk' test _inside_ the loop, rather
than be something like

        if (host->dram_access_quirk)
                return sg_copy_to_buffer(data->sg, data->sg_len,
                                host->bounce_buf, xfer_bytes);

at the top of the function, with meson_mmc_copy_buffer() then only
handling the mmio case?

No, I don't know this code, I'm just looking at the patch and going
"that looks really ugly".

Anyway, I've pulled it, but I thought I'd voice my reaction to it..

              Linus
pr-tracker-bot@kernel.org June 24, 2021, 6:01 p.m. UTC | #2
The pull request you sent on Thu, 24 Jun 2021 14:32:51 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git tags/mmc-v5.13-rc6

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/4a09d388f2ab382f217a764e6a152b3f614246f6

Thank you!
Ulf Hansson June 28, 2021, 7:49 a.m. UTC | #3
+ Neil, Robin

On Thu, 24 Jun 2021 at 20:00, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, Jun 24, 2021 at 5:32 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > Neil Armstrong (1):
> >       mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
>
> Ugh. How horribly ugly.
>
> Why is that 'host->dram_access_quirk' test _inside_ the loop, rather
> than be something like
>
>         if (host->dram_access_quirk)
>                 return sg_copy_to_buffer(data->sg, data->sg_len,
>                                 host->bounce_buf, xfer_bytes);
>
> at the top of the function, with meson_mmc_copy_buffer() then only
> handling the mmio case?
>
> No, I don't know this code, I'm just looking at the patch and going
> "that looks really ugly".
>
> Anyway, I've pulled it, but I thought I'd voice my reaction to it..

Thanks, good point!

Robin and Neil did discuss some cleanup that could be made on top [1],
but it looks like that was targeting a different part.

In any case, Neil, would you mind sending a cleanup according to the
comment from Linus?

Kind regards
Uffe

[1]
https://www.spinics.net/lists/arm-kernel/msg901045.html
Neil Armstrong June 28, 2021, 8:28 a.m. UTC | #4
Hi,

On 28/06/2021 09:49, Ulf Hansson wrote:
> + Neil, Robin
> 
> On Thu, 24 Jun 2021 at 20:00, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> On Thu, Jun 24, 2021 at 5:32 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>
>>> Neil Armstrong (1):
>>>       mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
>>
>> Ugh. How horribly ugly.
>>
>> Why is that 'host->dram_access_quirk' test _inside_ the loop, rather
>> than be something like
>>
>>         if (host->dram_access_quirk)
>>                 return sg_copy_to_buffer(data->sg, data->sg_len,
>>                                 host->bounce_buf, xfer_bytes);
>>
>> at the top of the function, with meson_mmc_copy_buffer() then only
>> handling the mmio case?
>>
>> No, I don't know this code, I'm just looking at the patch and going
>> "that looks really ugly".
>>
>> Anyway, I've pulled it, but I thought I'd voice my reaction to it..
> 
> Thanks, good point!
> 
> Robin and Neil did discuss some cleanup that could be made on top [1],
> but it looks like that was targeting a different part.

Yes this was a quick hack (and obviously not the optimal way to) to address the fatal system crash.

> 
> In any case, Neil, would you mind sending a cleanup according to the
> comment from Linus?

Sure, as discussed with Robin, the plan is to introduce sg_copy_to_buffer_to/fromio
and remove this crappy hack, but this requires multiple subsystems involved.

> 
> Kind regards
> Uffe

Thanks,
Neil

> 
> [1]
> https://www.spinics.net/lists/arm-kernel/msg901045.html
>