Rex-BC Chen <rex-bc.chen@xxxxxxxxxxxx> writes: [...] > I can summary what I got now: > > 1. Why we need cci for cpufreq in MT8183 and MT8186: > a. CCI is a mediatek hw module. > b. For mediatek SoCs before MT8183, like MT8173, the CCI hw > is not introduced. > c. The frequency for cci and cpufreq are determined could > be configed at bootloader stage, so the frequency when > entering kernel is unknown. > d. Cpu little core and cci are using the same regulator. > e. If we do not control CCI and just adjust the voltage in > cpufreq driver. > When we adjust the voltage smaller because we need to reduce > the frequency, the CCI could run in high frequency which is > set in bootloader. > This will cause some problem, the cci could crash. > > Use MT8186 for a example, the bootloader set cci freq as > 1.385GHz and cpufreq as 2GHz. > If entering kernel and we only adjust the cpufreq voltage, if > the cpufreq is under 1.618GHz, the cci will be out of spec. > > f. If cpufreq driver wait cci ready, regulator framework will take > the highest voltage requests from cci and cpufreq as output > so that it prevents from high freqeuncy low voltage crash. > > d. Therefore, I think it's not a good idea to bypass cci device if > the ccifreq_supported is true in MT8183 and MT8186. I do not propose to bypass CCI device. What both Angelo and I are saying is just that you need a better way to handle the cases when CCI is not (yet) enabled. The current way in the propsed patch is not good enough. I fully understand the potential problems with high frequency & low voltage when using a shared regulator. But, I think the problem we're trying to solve here is specific to the initial boot of the platform, while we are waiting for the CCI driver to be loaded. The root of the problem is that the CCI bus has constraints on the voltage regulator that are not defined anywhere until the CCI driver is loaded. So to fix that, you need to either: 1) not allow any voltage changes 2) register the CCI device constraints In the current patch, you attempt to do (1). There's nothing wrong with the idea, we just pointed out problems in your implementation, especially the fact that it does nothing, but it "succeeds" so the CPUfreq framework will think the OPPs are different from what they actually are. Just an idea, but another option could be (2). While waiting for the CCI device to be ready, the CPUfreq driver could check the current CCI freq/voltage and set min/max constraints on the regulator that prevent CCI from breaking. These constraints would stay in place until the CCI driver is ready. Once the real CCI driver is ready/registerd these contraints would be removed. Another version of this same idea would be to check the CCI freq/voltage and then limit the OPPs available to CPUfreq to only the ones that would not break CCI. Then, when CCI is ready/registered, you remove the limits. > 2. Check the device link status is DL_DEV_DRIVER_BOUND is used for > promising the cci is probed done. > > 3. About the cpufreq_driver->target_index > a. When I trace the common drivers, I found if the return value is > not zero, it will be BUG_ON. > ref: > https://elixir.bootlin.com/linux/latest/source/drivers/cpufreq/cpufreq.c#L1471 Right, you should not try to do deferred probe in the ->set_target() callback. Deferred probe is meant for init/probe time. > b. I also try to move is_ccifreq_ready() to other place, like > cpufreq_driver->init and cpufreq probe function. > There will be new issue. Do you have any suggetion that we can > retern value of DEFER_PROBE? The only appropriate place is in the probe function. Kevin
