Hello Marek, On 10/23/24 07:44, Marek Vasut wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > On 10/23/24 9:17 AM, Ajay.Kathat@xxxxxxxxxxxxx wrote: > > Hello Ajay, > >>> What I am trying to say is this: >>> >>> With current code, this can happen, which is not good, because transfers >>> from multiple threads can be interleaved and interfere with each other: >> >> Did you observe any bus failure in your test setup with SDIO. Is there any >> configure to recreate it. > > I am observing sporadic command and data CRC errors on STM32MP157F > system with SDIO WILC3000. Does this patch help to resolve the CRC errors? Do you observe the CRC errors during the bulk data transfer(iPerf) or does it even happen during the basic WiFi operation like(scan/connect or basic ping operation). Is power-save enabled during the test. With PS enabled, The SDIO commands may fail momentarily but it should recover. > >> The SDIO transfer may appear to be split into into multiple transaction but >> these calls should not impact each other since most of them are updating the >> different registers except WILC_SDIO_FBR_CSA_REG register, which is used for >> CSA read/write. If needed, wilc_sdio_set_func0_csa_address() can be modified >> to club the 3x CMD52 and 1x CMD53 into a single transfer API. >> >> In my opinion, If sdio_claim_host() is moved to acquire_bus() API then the >> SDIO bus will be claimed longer than required especially in >> wilc_wlan_handle_txq() API. Ideally, calling sdio_claim_host() call just >> before the transfer is enough but now the SDIO I/O bus would be continuously >> blocked for multiple independent SDIO transactions that is not necessary. > > Why would that pose a problem ? wilc_wlan_handle_txq() performs many operations on different registers which are not related. It will block the SDIO bux for longer. Otherwise those registers are allowed to update separately from the WILC SD side. > > I am more concerned that ksdioirqd can insert a command transfer right > in the middle of CMD52/CMD53 register read composite transfer, because > while ksdioirqd does use proper sdio_claim/release_host, this driver > does it per-SDIO-command instead of per the whole e.g. register read > "transaction". > I think, using sdio_claim/release for each-SDIO command should suffice because the CMD52/CMD53 modify the specific registers that are unrelated. Each CMD52/53 should work properly and independently provided they are protected with sdio_claim/release. Additionally, there is no WILC SD module limitation requiring a strict order for CMD52/CMD53, except for Card Storage Area (CSA) read/write operations. For CSA read/write operations, which are necessary to read/write any specific address from the card, multiple CMD52 commands are used to pass the desired address to be read/written using registers (0x10c, 0x10d, 0x10e). Then, CMD53 is used to read/write to address 0x10f (CSA data window register). This complete command sequence is required for a single CSA address read/write. Based on this requirement, CSA read/write operations can cause issues if they run in parallel with another CSA read/write operation, but not with other CMD52 and CMD53 commands. Therefore, one approach to resolve this issue is to add sdio_claim/release around wilc_sdio_set_func0_csa_address(). This way, this function will be treated as a single operation and it will only modify the required command flow. Regards, Ajay
