Hi Qianyu, [...]
>>>> @@ -308,6 +307,11 @@ int spi_get_bus_and_cs(int busnum, int cs, int >>> speed, int >>> >>>> mode, >>> >>>> slave->dev = dev; >>> >>>> } >>> >>>> >>> >>>> + plat = dev_get_parent_platdata(dev); >>> >>>> + if (!speed) { >>> >>>> + speed = plat->max_hz; >>> >>>> + mode = plat->mode; >>> >>>> + } >>> >>>> ret = spi_set_speed_mode(bus, speed, mode); >>> >>>> if (ret) >>> >>>> goto err; >>> >>>> -- >>> >>> >>> >>> I just doubt if spi_set_speed_mode() has really made a difference to >>> >>> the actual transfer. >>> >> >> It does (see below)... >> >>> Seems that if the device is inactive, calling device_probe() would >>> also call >>> >>> spi_set_speed_mode() and do the data transfer. Even if it's active, >>> setting >>> >>> speed and mode for it again would not be necessary. >> >> >> Yes, spi_set_speed_mode() is called from >> spi_flash_probe_slave()->spi_claim_bus() as part of device_probe(). >> spi_claim_bus() in spi-uclass.c speed & mode are appropriately passed based >> on DT >> data to spi_set_speed_mode(). But that's not the issue. >> >> But in spi_get_bus_and_cs() (called from sf probe) there is a call to >> spi_set_speed_mode() after device_probe() for inactive devices. This call is >> to > > Yes. Actually I'm thinking that the second spi_set_speed_mode()(called from > spi_get_bus_and_cs()) could just be removed from the end of the function. > If second call to spi_set_speed_mode() is removed then how would you override default speed/mode specified via DT with that of cmd line args passed to sf probe. (else we need to change the definition of sf probe to not accept speed/mode in case of DT) >> _override_ the speed set via DT with those passed as cmd line args of sf >> probe. But, >> if no args are passed to sf probe, speed and mode default to >> CONFIG_SF_DEFAULT_SPEED/MODE (see do_spi_flash_probe() in >> cmd/sf.c) instead of using DT inputs. >> > > Yes. But notice that the speed and mode will only be replaced by > CONFIG_SF_DEFAULT_SPEED/MODE at the end of the calling. Every time 'sf probe' > is > called, the device will be removed(if active) and thus it'll always call > device_probe() > to set the speed&mode from DT. Then the driver will use them in the actual > transfer. True, I see the first call and speed/mode is set to DT values accordingly. But, that's not the final spi_set_speed_mode() call to the SPI driver. > After the transfer is finished, the speed and mode are replaced by > CONFIG_SF_DEFAULT_SPEED/MODE(or anything else) again but they wouldn't be used > for the transfer at all. > Sorry... I don't understand. What do you mean by transfer? sf probe does not do any data transfer other than reading jedec id. The values set by the SPI driver during device_probe() will be overwritten by the last spi_set_speed_mode() call in spi_get_bus_and_cs() which happens to pass CONFIG_SF_DEFAULT_SPEED/MODE. Here is the call sequence: sf probe() ---> device_probe() ---> spi_set_speed_mode(values from DT) ---> driver's pi_set_speed_mode() /* set to DT values */ ---> spi_set_speed_mode(overridden values) ---> driver's spi_set_speed_mode() /* set to newer(not DT) values */ Now if sf read is called after sf probe. One would see CONFIG_SF_DEFAULT_SPEED/MODE values in driver settings and data transfers happen at DEFAULT_SPEED. > And there may be another related issue in this case that I have reported to > Simon. > If you would like to test on your board, please remove the device_unbind() in > do_spi_flash_probe(). The current SPI driver model will discard users' spi > slave in DT from > the second 'sf probe' and create a new one using > CONFIG_SF_DEFAULT_SPEED/MODE. > Yes, I am aware of the problem with second sf probe discarding values from DT. IIRC, Simon not just wanted to remove device_unbind() but also do some more refactoring of messages in uclass drivers. Maybe Simon is working on the fix. -- Regards Vignesh _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot