linux-kernel - Re: [PATCH V23 00/16] Add support UHS-II for GL9755 and GL9767

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPDyKFrgwmPaAnv3CgH=rL9yV4JfbF9mX6nt+Wy=2OUphadZJQ@mail.gmail.com>
Date: Thu, 24 Oct 2024 14:57:04 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Victor Shih <victorshihgli@...il.com>
Cc: adrian.hunter@...el.com, linux-mmc@...r.kernel.org, 
	linux-kernel@...r.kernel.org, benchuanggli@...il.com, 
	Lucas.Lai@...esyslogic.com.tw, HL.Liu@...esyslogic.com.tw, 
	Greg.tu@...esyslogic.com.tw, dlunev@...omium.org, 
	Victor Shih <victor.shih@...esyslogic.com.tw>
Subject: Re: [PATCH V23 00/16] Add support UHS-II for GL9755 and GL9767

On Fri, 18 Oct 2024 at 12:53, Victor Shih <victorshihgli@...il.com> wrote:
>
> From: Victor Shih <victor.shih@...esyslogic.com.tw>
>
> Summary
> =======
> These patches[1] support UHS-II and fix GL9755 and GL9767
> UHS-II compatibility.
>
> About UHS-II, roughly deal with the following three parts:
> 1) A UHS-II detection and initialization:
> - Host setup to support UHS-II (Section 3.13.1 Host Controller Setup
>   Sequence[2]).
> - Detect a UHS-II I/F (Section 3.13.2 Card Interface Detection Sequence
>   [2]).
> - In step(9) of Section 3.13.2 in [2], UHS-II initialization is include
>   Section 3.13.3 UHS-II Card Initialization and Section 3.13.4 UHS-II
>   Setting Register Setup Sequence.
>
> 2) Send Legacy SD command through SD-TRAN
> - Encapsulated SD packets are defined in SD-TRAN in order to ensure Legacy
>   SD compatibility and preserve Legacy SD infrastructures (Section 7.1.1
>   Packet Types and Format Overview[3]).
> - Host issue a UHS-II CCMD packet or a UHS-II DCMD (Section 3.13.5 UHS-II
>   CCMD Packet issuing and Section 3.13.6 UHS-II DCMD Packet issuing[2]).
>
> 3) UHS-II Interrupt
> - Except for UHS-II error interrupts, most interrupts share the original
>   interrupt registers.
>
> Patch structure
> ===============
> patch#1:     for core
> patch#2-#14: for sdhci
> patch#15:    for GL9755
> patch#16:    for GL9767
>
> Tests
> =====
> Ran 'dd' command to evaluate the performance 3 times:
> (SanDisk UHS-II card on GL9755 controller)
>                              Read    Write
> UHS-II disabled (UHS-I): 81.9MB/s 51.4MB/s
> UHS-II enabled         :  206MB/s 80.5MB/s
>                              Read    Write
> UHS-II disabled (UHS-I): 82.3MB/s 49.7MB/s
> UHS-II enabled         :  208MB/s 80.8MB/s
>                              Read    Write
> UHS-II disabled (UHS-I): 82.9MB/s 50.8MB/s
> UHS-II enabled         :  205MB/s 90.0MB/s
> (SanDisk UHS-II card on GL9767 controller)
>                              Read    Write
> UHS-II disabled (UHS-I): 83.5MB/s 50.5MB/s
> UHS-II enabled         :  200MB/s 75.3MB/s
>                              Read    Write
> UHS-II disabled (UHS-I): 85.2MB/s 56.3MB/s
> UHS-II enabled         :  203MB/s 75.8MB/s
>                              Read    Write
> UHS-II disabled (UHS-I): 82.9MB/s 51.1MB/s
> UHS-II enabled         :  196MB/s 77.8MB/s
>
> Test command
> =====
> Read: dd if=/dev/mmcxxx of=/dev/null bs=4096k count=2000 iflag=direct
> Write:dd if=/dev/zero of=/dev/mmcxxx bs=4096k count=2000 oflag=direct
>
> Changes in v23 (October. 18, 2024)
> * Rebase on latest mmc/next.
> * Version 22 patch#1-patch#6 have already been applied to the mmc/next
>   branch, so the patch order for version 23 has been shifted forward.
> * Patch#1: Remove mmc_uhs2_card_prepare_cmd() function.
>            Remove mmc_sd_can_poweroff_notify() function.
>            Modify ios.timing setting in the sd_uhs2_power_off() function.
>            Restore the position of assign the host->card to original
>            position in the sd_uhs2_init_card() function.
>            Remove unnecessary error handle in the sd_uhs2_init_card()
>            function.
>            Add oldcard judgment to skip some programs in the
>            sd_uhs2_legacy_init() function.
>            Remove unnecessary error handle in the sd_uhs2_legacy_init()
>            function.
>            Remove mmc_card_set_present() function in the
>            sd_uhs2_reinit()function.

I have queued this up for next, with a few amendments and added
Adrian's acks to the SDHCI patches.

When applying, I made a couple of cleanups and fixed
sd_uhs2_power_off() to patch 1. I also took care of the checkpatch
warnings for the SDHCI patches.

That said, there are still some issues that remain to be fixed, but
let's do that on top of the $subject series. See below:

*) The error/re-init path of the card is broken, as also pointed out
by Adrian. I will work on this shortly and send patches to fix this
up, please help to review and test.
*) We should read the card's ext registers, to for example enable the
card's internal cache to be used.
*) We should avoid unnecessary commands during re-init of the card, to
decrease latency.
*) During suspend, we should probably send the go-dormant command with
the hibernate bit, rather than just doing a plain power-off.

[...]

Thanks and kind regards
Uffe