Hello Jakub, >> workaround when stopping the device), register bit should actually >> be toggled. > > Does the bit get set by the driver or HW? > > If it gets set by HW there is still a tiny race from reading to > writing.. Perhaps doing two writes -> to 0 and to 1 would be a better > option?
No, set is done by the driver, not HW. HW just tracks for the toggle. >> It was previosly always just set. Due to the way driver stops HW this >> never actually caused any issues, but it still may, so cleaning this up. > > Hm. So is it a cleanup of fix? Does the way code is written guarantee > it will never cause issues? Yes, thats a cleanup. We just had other products where this cache reset had to be done multiple times. Obviously doing that second time was just no-op for hardware ;) On linux this always gets called on deinit only once - thus it was safe. We just aligning here the linux driver with actual HW specification. >> + if (err) >> + goto err_exit; >> + >> + readx_poll_timeout_atomic(hw_atl_rdm_rx_dma_desc_cache_init_done_get, >> + self, val, val == 1, 1000U, 10000U); > > It's a little strange to toggle, yet wait for it to be of a specific > value.. Notice thats a different value - 'cache_init_done' bit. This is used by HW to indicate completion of cache reset operation. >> +err_exit: >> + return err; > > Just return err instead of doing this pointless goto. It make the code > harder to follow. Sure >> +#define RDM_RX_DMA_DESC_CACHE_INIT_DONE_DEFAULT 0x0 >> + >> + > > two empty lines here? Will fix. Regards, Igor
