Mark On 5/30/19 10:26 AM, Mark Brown wrote:
That code is rather far away from the code you're changing and it's really not clear that this will do the right thing for new devices, it already appears that we're handling two devices without obvious code for that (the regulator IDs get reused AFAICT). If there were a switch statement right there it'd be clearer. Part of this is a reviewability thing - I initially stopped on the patch because it looked like it was using the enable field for something other than the intended purpose and now there's all this chasing around to see if the code is OK.
I am going to introduce this update in patch 4 of this series when the LM36274 is introduced to the regulator driver.
It makes more sense to add it there as in patch 1 there is not really a need to extend the value or mask as it only supports the LM3632 device. It makes sense to add the condition when adding another device to support
Thoughts? Dan
