Page MenuHomeFreeBSD

patch for sdhci_fdt.c so emmc works on rk356x boards
Needs RevisionPublic

Authored by titus_edc.ro on Oct 27 2023, 10:29 AM.
Tags
None
Referenced Files
F106860129: D42377.id130092.diff
Mon, Jan 6, 1:30 PM
Unknown Object (File)
Mon, Dec 9, 11:03 AM
Unknown Object (File)
Dec 3 2024, 7:37 PM
Unknown Object (File)
Dec 2 2024, 4:57 PM
Unknown Object (File)
Sep 22 2024, 11:33 PM
Unknown Object (File)
Sep 17 2024, 8:08 AM
Unknown Object (File)
Aug 26 2024, 4:57 AM
Unknown Object (File)
Aug 18 2024, 11:17 PM
Subscribers

Details

Reviewers
manu
Group Reviewers
arm64
Summary

mostly changed some constants and added uhs timing function (from linux)

Test Plan

tested on orange pi 3b
dts needs disable-wp otherwise the emmc is reported readonly

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

manu added inline comments.
sys/dev/sdhci/sdhci_fdt.c
539

Extra blank line.

687

This will need to go in a separate function like we did for rk3399

728

Extra blank line.

744

Extra blank line.

Please upload later diff with full context (git diff -U999 or git-format-patch -U999)

fixed extra lines
moved rk35688 clock initialization to separate function

manu requested changes to this revision.Oct 28 2023, 7:51 AM

Mhm, I'm starting to think that we might want to subclass sdhci_fdt.
Like all IP which are re-used a lot in different SoC there is always some difference between the implementation (mostly clocks/resets for sometimes more).
UHS are likely to be different for each variant of this driver.
I don't want to have a mess in this driver (it's already a bit of a mess).

sys/dev/sdhci/sdhci_fdt.c
582

Extra blank line.

583

What are those magic values ?

587

Extra blank line.

669

Extra blank line.

737–742

Those register likely don't exists on other variant (or at least might not exist), that should be gated based on the compatible.

This revision now requires changes to proceed.Oct 28 2023, 7:51 AM

fixed some extra blank lines, added compat guard for vendor specific regs
I cant find the vendor specific regs bits doc again (0x05xx) but im sure ive seen then in some pdf

in linux the vendor register offset is read from yet another register, ours is hardwired to 0x500 so i wanted to check the doc and found them somewhere
it was a bit painful to make it work everything seemed ok but write ops still failed
in the end i dumped all the registers with devmem2 in linux and freebsd and compared them and then i found the diffs in defines etc

fixed stupid bug in sdhci_init_rk3568

if(err) {
        device_printf(dev, "cannot enable core clock\n");
        return (err);
}

was missing {}
result was clocks not being enabled

manu requested changes to this revision.Nov 27 2023, 7:08 AM

Thanks for the update but I still thinks that we should split the driver into multiple file (or per vendor).

This revision now requires changes to proceed.Nov 27 2023, 7:08 AM