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

risc-v/mpfs: emmcsd: enforce HS SDR mode properly #101

Open
wants to merge 61 commits into
base: master
Choose a base branch
from
Open

Conversation

jpaali
Copy link

@jpaali jpaali commented Apr 14, 2023

Previously, address 0x03b70000u was written with shift bits that only changed the bit width, not the mode. Enforce HS SDR mode (50 MHz) for now.

haitomatic and others added 30 commits January 18, 2023 14:46
Cast substraction arguments to FAR char *, which gives the same result as the
gcc extension on the original void * arithmetic.

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
- boots from eNVM
- uses lim memory for RAM
- has console on uart 0
- has procfs enabled
- has most of nsh commands enabled

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
I2C status register reset value (0xf8) was not handled properly causing unnecessary bus resets.
Added critical section to mpfs_i2c_reset() and removed unnecessary interrupt disabling elsewhere.
- Change git repository urls to point to our tiiuae repos for nuttx & nuttx apps
- Remove most of the the build steps, leave just arm-12 and riscv;
	arm-12 has a build for stm32f7, and riscv for mpfs

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
…onfirm/defconfig

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
…ptimized version in tiiuae repo

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
…iver to re-initialize on rx timeout

If the interface is UP, and no packets are received in 30s, re-initialize the interface by calling the
already implemented mpfs_txtimeout_expiry.

This is a temporary workaround for a bug where IF might be UP and working but packets can only
be transmitted. Receive side just doesn't work at all.

The original bug can be re-produced easily by disconnecting and reconnecting the ethernet cable while
the IF is up.

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
SD-card clock speed is just forced to 50MHz. Note that to be correct, one should first set the SD-card
into high-speed mode, but currently NuttX doesn't support this.

With our cards, just setting the interface to 50MHz seems to work fine, and it removes the issue with
25MHZ clock causing disturbance on GPS bands. Typically cards which support high-speed mode just work with
50MHz interface clock.

This patch should be reverted when the NuttX supports high-speed mode, and we can properly set it.

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
…ootloader

This removes the need to have all the DDR/clock configuration related
"LIBERODEFS" flags defined, when not building a standalone/coldboot
configuration

All of this code is unused when not building with CONFIG_MPFS_BOOTLOADER

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
This is not the correct way to do this, but it gives a nice perf. boost
Disable macOS builds for now.
All other commands are disabled in send_recv().

Signed-off-by: Jani Paalijarvi <jani.paalijarvi@unikie.com>
Signed-off-by: Jani Paalijarvi <jani.paalijarvi@unikie.com>
Remove unnecessary heap allocation by relocating ops inside priv data
This reverts commit c0735f0.

rpsg_release_tx_buffer only exists in the newer openamp. Revert this when
taking the new openamp into use
…ef and forward declare devif_loopback

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
pussuw and others added 16 commits February 17, 2023 13:02
Just a temporary patch, need to implement some kind of scalable solution
for this. It might be a good idea to map something else for the user
to avoid using ecall to enter the kernel for simple reads ?

Also, increase the L3 table size
The path was wrong, also enable the "if MPFS_CRYPTO" flag

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
There is no make step executed for this directory before the Kconfigure,
so all Kconfig's just need to be in-tree

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
In spi_irq handler the data is written into txfifo and transfer
is started before the TXDONE interrupt is cleared. If the bus/memory
access is in some cases delayed, the spi transfer may have been
finished already before the interrupt register is cleaned for the
transfer. This leads the early arrived interrupt to be just removed
and never handled, which would cause a timeout error.
This patch moves the clearing of the interrupt to the place before
the tx is started, so the interrupt is not missed in above cases.
If transfer is restarted in irq handler the interrupts shall be
cleared before the start bit is set in control register. This is
to avoid ints being accidentally cleared before they are handled leading
to timeout error.
Workaround to avoid deadlock situation: The RX shall not try to wait for complete
frame in case there is RX errors detected.

In case mpfs_receive is called, it keeps waiting for complete frame and
also keeps the net_lock locked. In the mean while, the TX may run out of free
descriptors, but can not get net_lock mutex lock to be able to release used
descriptors. If there are no free TX descs it disables RX interrupts because
it may require to send response to the received frame.
So, TX side keeps RX interrupts disabled due to lack of free descriptors and
RX blocks TX to release those descs by stubbornly waiting for complete frame.
Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
…dress

Add a function to read PolarFire's serial number from system controller, and use the first five digits as device's mac address

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
Previously, address 0x03b70000u was written with shift bits
that only changed the bit width, not the mode. Enforce HS
SDR mode (50 MHz) for now.

Signed-off-by: Eero Nurkkala <eero.nurkkala@offcode.fi>
@jpaali jpaali requested a review from jlaitine April 14, 2023 06:13
@jnippula
Copy link

Is it really necessary to use SDR mode? Why would DDR not work?

Copy link

@jlaitine jlaitine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed my mind; I don't think that we should by default change the mode to SDR; this just changes the timings but can't be the source of the problem.

So this PR should be modified in the way that it fixes the issues related to the mode change, but keeps the mode in DDR

@eenurkka
Copy link

eenurkka commented Apr 14, 2023

Is it really necessary to use SDR mode? Why would DDR not work?

By default, it's at the Legacy mode, not DDR. Nothing changes it to DDR (nor previously did). One problem is that the legacy mode is for 0-26MHz, but clk is at 50 Mhz, which should be HS mode (or SDR / DDR).

"Variable clock frequencies of 0-20 MHz, 0-26 MHz (default), 0-52 MHz (high-speed), 0-200 MHz SDR (HS200), 0-200 MHz DDR (HS400)" (from Sandisk datasheet 2105241638_SANDISK-SDINBDA4-64G_C2830404.pdf)

@eenurkka
Copy link

Previously:

  1. Set DDR:
    if ((ret = mpfs_sendcmd(dev, MMCSD_CMD6, 0x03b70000u | (6 << 8))) == OK)

  2. Unset DDR and use 8-bit lanes:
    if ((ret = mpfs_sendcmd(dev, MMCSD_CMD6, 0x03b70000u | (2 << 8))) == OK)
    (see: Conversely, a card in the dual data mode may be switched back to single data rate mode by writing new values in the BUS_WIDTH byte.)

I think all reference codes also switch to HS-mode first; DDRs may be used while keeping in mind some modes require PHY tuning.

From Jedec specs:

HS_TIMING must be set to “0x1” before setting BUS_WIDTH for dual data rate operation (values 5 or 6)

After the host verifies that the card complies with version 4.0, or higher, of this standard, it has to enable the high speed mode timing in the card, before changing the clock frequency to a frequency higher than 20MHz. After power-on, or software reset, the interface timing of the card is set as specified in Table 114 on page 177. For the host to change to a higher clock frequency, it has to enable the high speed interface timing.
The host uses the SWITCH command to write 0x01 to the HS_TIMING byte, in the Modes segment of the EXT_CSD register.

For the host to change to dual data rate mode, HS_TIMING must be set to 0x1 (Section 7.6.2 on page 50) and the card shall supports this mode as defined in EXT_CSD register [196] (CARD_TYPE) specified in Table 84 on page 139. The host uses the SWITCH command to write 0x05 (4-bit data width) or 0x06 (8-bit data width) to the BUS_WIDTH byte, in the Modes segment of the EXT_CSD register byte [183]. The valid values for this
register are defined in "BUS_WIDTH" on page 141. If the host tries to write an invalid value, the BUS_WIDTH byte is not changed, the dual data rate mode is not enabled, and the SWITCH_ERROR bit is set. Conversely, a card in the dual data mode may be switched back to single data rate mode by writing new values in the BUS_WIDTH byte.

A.8.2 Switching to high-speed mode
The following steps are supported by cards implementing version 4.0 or higher. Do these steps after the bus is initialized according to section Annex A.8.1 on page 203.
22-Send CMD7 with the card’s RCA to place the card in tran state
23-Send CMD8, SEND_EXT_CSD. From the EXT_CSD the host can learn the power class of the card, and choose to work with a wider data bus (See steps 26-37)
24-Send CMD6, writing 0x1 to the HS_TIMING byte of the EXT_CSD. The argument 0x03B9_0100 will do it.

@jlaitine
Copy link

Thanks for explanation, let's come back to this :)

@jlaitine jlaitine force-pushed the master branch 2 times, most recently from 6124da4 to ccf5808 Compare April 21, 2023 11:54
@jlaitine
Copy link

jlaitine commented Apr 26, 2023

I guess then setting the SDR mode like in this is more proper, and also the HS mode bit should be set. I wonder how much work would it be to support DDR50 ? IMHO, it shouldn't require training any more than SDR50. This is good, I just wonder if the magic number 0x03b70100u could be macroed to be more verbose (SOMETHING | HS_MODE...).

If you can please re-base this, and let's merge then

@eenurkka
Copy link

I think it's very simple, IIRC, just
if ((ret = mpfs_sendcmd(dev, MMCSD_CMD6, 0x03b90100u)) == OK)
->
if ((ret = mpfs_sendcmd(dev, MMCSD_CMD6, 0x03b90600u)) == OK)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants