On Thu, Dec 12, 2019 at 03:09:32AM +0000, Peng Ma wrote: > Hello Russell, > > Thanks very much for your strict guidance and comments. > I realized it is hard to us that we want to i2c used edma when edma > probe after i2c probe. I have no problem with that aim. I'm just very concerned by the proposed implementation, especially when it has already been proven to cause regressions in the kernel. I seem to remember that the infinite loop caused other issues, such as the system being unable to complete booting. > I look forward to discussing with you as below, if you like. > Thanks. > > You say I could do this: > "So, if you want to do this (and yes, I'd also encourage it to be > conditional on EDMA being built-in, as I2C is commonly used as a way > to get at RTCs, which are read before kernel modules can be loaded) > then you MUST move > i2c_imx_dma_request() before > i2c_add_numbered_adapter() to avoid the infinite loop." > > Even if I do this, It's hard to avoid the infinite loop of i2c probe caused by EDMA(build-in) initialization failure. It isn't clear what you mean here. If EDMA fails to probe (because fsl_edma_probe() returns an error other than EPROBE_DEFER) then of_dma_find_controller() will return NULL. That will be propagated down through i2c_imx_dma_request(). This is no different from the case where EDMA is built as a module. It is also no different from the case where EDMA hasn't yet been probed. If i2c_imx_dma_request() is placed after i2c_add_numbered_adapter(), and EPROBE_DEFER is propagated out of i2c_imx_probe(), then _yes_, it will cause an infinite loop, because you are replicating the exact conditions that caused the attempt to propagate i2c_imx_dma_request()'s return value to be reverted last time - which brought the kernel to a grinding halt. If i2c_imx_dma_request() is placed before i2c_add_numbered_adapter(), then there is no infinite deferred probing loop - yes, i2c_imx_probe() will be called as a result of other drivers successfully probing, and each time it will return EPROBE_DEFER, but the _key_ point is that the action of i2c_imx_probe() will not _self trigger_ the deferred probing _and_ place itself onto the deferred probe list. Please, rather than continuing to send emails arguing over this point, investigate the stated issue with some practical tests: 1. Make i2c_imx_probe() propagate i2c_imx_dma_request()'s return value, as it did in the original patch. 2. Build i2c-imx into the kernel. 3. Build edma as a module. 4. Build and test boot the kernel and check what happens. 5. Move i2c_imx_dma_request() before i2c_add_numbered_adapter() 6. Build and test boot the resulting kernel and note any differences. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
