Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mmc_spi: has anyone been successfully using it with one of the newest kernels? #911

Closed
msperl opened this issue Mar 26, 2015 · 14 comments
Closed
Labels
Closing due to inaction Issue likely to be closed due to lack of recent comments Waiting for external input Waiting for a comment from the originator of the issue, or a collaborator.

Comments

@msperl
Copy link
Contributor

msperl commented Mar 26, 2015

Hi!

has anyone been able to run mmc_spi with one of the latest kernels?
The last time I have tried I was still on 3.12 or earlier - now I only get error messages like this:
mmc1: error -110 whilst initialising SD card
-110 would mean -ETIMEDOUT.

And that with 8 different SD-Cards.

Might be related to kernel versions...

Any Ideas?

Martin

@malgrati-ferdinando
Copy link

are you using spi-gpio.ko?

2015-03-26 17:10 GMT+01:00 msperl [email protected]:

Hi!

has anyone been able to run mmc_spi with one of the latest kernels?
The last time I have tried I was still on 3.12 or earlier - now I only get
error messages like this:
mmc1: error -110 whilst initialising SD card
-110 would mean -ETIMEDOUT.

And that with 8 different SD-Cards.

Might be related to kernel versions...

Any Ideas?

Martin


Reply to this email directly or view it on GitHub
#911.

@msperl
Copy link
Contributor Author

msperl commented Mar 26, 2015

no - the native spi-bcm2835 driver.

@msperl
Copy link
Contributor Author

msperl commented Mar 30, 2015

note that I have bisected as far as:
Working: 3.9.11-d5572370289f698b101f3d0198b1c99f17f0d278
Failing: 3.10.38-1b49b450222df26e4abf7abb6d9302f72b2ed386

Has anyone a hint what the earliest commit of 3.10 is that works on the rpi?

All commits in the 3.10 I have tried so far miss arch/arm/mach-bcm2708/bcm2708.c, so they have not been merged that early during the 3.10 cycle...

@msperl
Copy link
Contributor Author

msperl commented Mar 31, 2015

note that on a 4.0-rc5 the following seems to solve the issue:
(after bisecting the issue with an upstream kernel)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index b189733..28e7286 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -179,9 +179,10 @@ static void of_dma_configure(struct device *dev)
         * Set it to coherent_dma_mask by default if the architecture
         * code has not set it.
         */
+/*
        if (!dev->dma_mask)
                dev->dma_mask = &dev->coherent_dma_mask;
-
+*/
        ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
        if (ret < 0) {
                dma_addr = offset = 0;

I have not tried it with a foundation kernel (yet), but it seems likely to work as well.
Why this has such a strange impact is an open question...

@Ruffio
Copy link

Ruffio commented Aug 15, 2016

@msperl has your issue been resolved? If so, please close this issue. Thanks.

@msperl
Copy link
Contributor Author

msperl commented Aug 15, 2016

Well - afaik this has not been fixed upstream, so I assume it is not fixed downstream either (unless you have applied the above patch here: msperl@aef50a1).

So many in reality it does not make any sense compiling spi_mmc unless the patch is applied.

So from my perspective it is your call if you want to apply a variation of the patch or if you want to ship a broken driver.

You can also close the issue.

@popcornmix
Copy link
Collaborator

Has this been reported upstream?
We'd prefer not to apply the patch without some sort of upstream acknowledgement that it is the correct thing to do. I can't judge if the patch will create any new issues.
Of course if upstream accepts a fix for this we'll be happy to cherry-pick it.

@msperl
Copy link
Contributor Author

msperl commented Aug 15, 2016

The point is that it is still unclear why it fails with the dma code - or even just this section - enabled, so I guess upstream will not accept a bandaid patch. And as I am not using it (unless I work on spi-bcm2835, where I use it for test purposes)I am not pushing it.

Fact is that it does not work with a set dma_mask and you are shipping it broken right now.

@JamesH65
Copy link
Contributor

@msperl Any movement on this on latest kernel?

@JamesH65 JamesH65 added the Waiting for external input Waiting for a comment from the originator of the issue, or a collaborator. label May 18, 2017
@JamesH65 JamesH65 added the Closing due to inaction Issue likely to be closed due to lack of recent comments label Sep 13, 2017
@JamesH65
Copy link
Contributor

Will be closing soon if no furthur comments.

@JamesH65
Copy link
Contributor

JamesH65 commented Dec 4, 2017

Closing due to lack of activity. Reopen if you feel this issue is still relevant.

@JamesH65 JamesH65 closed this as completed Dec 4, 2017
popcornmix pushed a commit that referenced this issue Jan 14, 2020
[ Upstream commit ce5c31d ]

At the moment, UBSAN report will be serialized using a spin_lock().  On
RT-systems, spinlocks are turned to rt_spin_lock and may sleep.  This
will result to the following splat if the undefined behavior is in a
context that can sleep:

  BUG: sleeping function called from invalid context at /src/linux/kernel/locking/rtmutex.c:968
  in_atomic(): 1, irqs_disabled(): 128, pid: 3447, name: make
  1 lock held by make/3447:
   #0: 000000009a966332 (&mm->mmap_sem){++++}, at: do_page_fault+0x140/0x4f8
  irq event stamp: 6284
  hardirqs last  enabled at (6283): [<ffff000011326520>] _raw_spin_unlock_irqrestore+0x90/0xa0
  hardirqs last disabled at (6284): [<ffff0000113262b0>] _raw_spin_lock_irqsave+0x30/0x78
  softirqs last  enabled at (2430): [<ffff000010088ef8>] fpsimd_restore_current_state+0x60/0xe8
  softirqs last disabled at (2427): [<ffff000010088ec0>] fpsimd_restore_current_state+0x28/0xe8
  Preemption disabled at:
  [<ffff000011324a4c>] rt_mutex_futex_unlock+0x4c/0xb0
  CPU: 3 PID: 3447 Comm: make Tainted: G        W         5.2.14-rt7-01890-ge6e057589653 #911
  Call trace:
    dump_backtrace+0x0/0x148
    show_stack+0x14/0x20
    dump_stack+0xbc/0x104
    ___might_sleep+0x154/0x210
    rt_spin_lock+0x68/0xa0
    ubsan_prologue+0x30/0x68
    handle_overflow+0x64/0xe0
    __ubsan_handle_add_overflow+0x10/0x18
    __lock_acquire+0x1c28/0x2a28
    lock_acquire+0xf0/0x370
    _raw_spin_lock_irqsave+0x58/0x78
    rt_mutex_futex_unlock+0x4c/0xb0
    rt_spin_unlock+0x28/0x70
    get_page_from_freelist+0x428/0x2b60
    __alloc_pages_nodemask+0x174/0x1708
    alloc_pages_vma+0x1ac/0x238
    __handle_mm_fault+0x4ac/0x10b0
    handle_mm_fault+0x1d8/0x3b0
    do_page_fault+0x1c8/0x4f8
    do_translation_fault+0xb8/0xe0
    do_mem_abort+0x3c/0x98
    el0_da+0x20/0x24

The spin_lock() will protect against multiple CPUs to output a report
together, I guess to prevent them from being interleaved.  However, they
can still interleave with other messages (and even splat from
__might_sleep).

So the lock usefulness seems pretty limited.  Rather than trying to
accomodate RT-system by switching to a raw_spin_lock(), the lock is now
completely dropped.

Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Julien Grall <[email protected]>
Reported-by: Andre Przywara <[email protected]>
Acked-by: Andrey Ryabinin <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Sebastian Andrzej Siewior <[email protected]>
Cc: Steven Rostedt <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
@TheCrazyT
Copy link

TheCrazyT commented Apr 27, 2023

I honestly don't know why nobody complained about it.
I only got it to work after patching the mmc_spi.c at the following locations:

https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/mmc/host/mmc_spi.c#L1513
https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/mmc/host/mmc_spi.c#L1519

I patched it because spi_probe was never actually invoking mmc_spi_probe .
Somehow I then managed to figure out the problem was the check in spi_match_device.

Changed both "mmc-spi-slot" to "mmc_spi"

and used the following overlay "mmc_spi.dts":

/dts-v1/;
/plugin/;

/ {
   compatible = "brcm,bcm2835", "brcm,bcm2836", "brcm,bcm2708", "brcm,bcm2709";

   fragment@0 {
                target = <&spidev0>;
                __overlay__ {
                        status = "disabled";
                };
   };

   fragment@1 {
      target = <&spi0>;
      __overlay__ {
         // needed to avoid dtc warning
         #address-cells = <1>;
         #size-cells = <0>;
         status = "okay";
         sd1: sd1@0 {
                reg = <0>;
                status = "okay";
                compatible = "spi,mmc_spi";
                voltage-ranges = <3000 3500>;
                spi-max-frequency = <8000000>;
         };
      };
   };
};

compiled it with: dtc -@ -I dts -O dtb -o mmc_spi.dtbo mmc_spi.dts
and loaded all with sudo dtoverlay -d . mmc_spi

output from dmesg is:

[ 2546.892076] mmc_spi spi0.0: SD/MMC host mmc2, no WP, no poweroff, cd polling
[ 2547.005168] mmc2: host does not support reading read-only switch, assuming write-enable
[ 2547.005215] mmc2: new SDXC card on SPI
[ 2547.005898] mmcblk2: mmc2:0000 SD 116 GiB (ro)
[ 2547.020020]  mmcblk2: p1 p2
[ 2547.020717] mmcblk2: mmc2:0000 SD 116 GiB (ro)
[ 2554.122622] EXT4-fs (mmcblk2p2): INFO: recovery required on readonly filesystem

using this basic sd-card-reader:
tech1276-1-550x550

@6by9
Copy link
Contributor

6by9 commented Apr 27, 2023

The module is called mmc_spi. The compatible string required in an overlay is "mmc-spi-slot", exactly as stated in the binding.

I don't see an overlay being shipped as part of this kernel tree using mmc_spi / mmc-spi-slot, so I don't believe there is an issue outstanding.
The OP's post appears to have been down to changes in dma ranges which are presumably solved in the 8 years since they opened the issue.

@TheCrazyT
Copy link

@6by9 thx for clarification, will try to test the version with the "mmc-spi-slot" in the overlay instead. Guess I got confused by some examples(like here and here ) on the internet that used "mmc_spi" instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closing due to inaction Issue likely to be closed due to lack of recent comments Waiting for external input Waiting for a comment from the originator of the issue, or a collaborator.
Projects
None yet
Development

No branches or pull requests

7 participants