2013/4/15 Bengt Jönsson <bengt.g.jons...@stericsson.com>: > On 04/08/2013 02:31 PM, Axel Lin wrote: >> >> The special handling code for getting shared mode status is wrong >> because it needs to check info->shared_mode->lp_mode_req for >> both regulators that shared the same mode register. >> >> In set_mode(), current code ensures we won't set mode to >> REGULATOR_MODE_IDLE >> if only one of the regulator requests to set idle. >> >> In get_mode(), we can just remove the special handling code for shared >> mode. >> Read the register value always returns correct status no matter the >> regulator >> has shared mode register or not. > > I am not convinced about this patch. The purpose of the original code is > that the regulator framework should be unaware that the mode register is > shared with another regulator. If we apply this patch, get_mode may return > different results depending on the other regulators mode settings.
No. Original code is just wrong. First, take a look at ab8500_regulator_set_mode(). When setting REGULATOR_MODE_IDLE mode, current code will only write the register to idle mode only when *both* shared regulator have set idle mode. Which means if only one of the shared mode has set regulator to idle, both should be still in NORMAL mode. In ab8500_regulator_get_mode(), the special handling code only check it's own lp_mode_req flag which is wrong. it needs to check both it's own lp_mode_req and the other one shared mode regulator. However, this patch makes things simpler by just remove these check. Because set_mode ensures the register is set to IDLE only when *both* shared regulator are in idle. > > Let me take an example: The other shared regulator is already set to LP mode > and the current regulator is requested to low power mode. Then the framework > first checks current mode and compares to requested mode. If equal, it > returns. With this patch applied, it will see that the regulator is already > set to LP mode and return without calling set_mode in the driver. However, > the state information in the driver will be wrong so when the other > regulator is requested to HP mode and back to LP mode it will not actually > set LP mode again to HW. I'm not sure if I understand this part. My understanding is for shared mode regulators: It can be in LP mode only when *BOTH* are in LP mode. If only one of the regulator in HP mode, then *BOTH* should be in HP mode. Did I misunderstand something? Axel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
