On Tue, Dec 07, 2010 at 02:04:02PM -0800, Brandon Leong wrote: [Please don't top post, and please quote properly - I've added a layer of quotation to my text.] > So is it decided that we should use BUG_ON() now? I'd prefer it, other people's mileage may vary. But please do also engage with the big picture stuff I was talking about. > Also, regarding this issue: > > > + if (val) > > > + err_info = regulator_enable(data); > > > + else > > > + err_info = regulator_disable(data); > > This isn't going to do what people expect - the refcounting really is > > going to surprise people, especially as you read back the physical > > enable/disable state through the same file. Abuse of this file is > > likely to confuse any actual consumers we have too. > ---- > Could you clarify the issue with this? All I am doing here is if the user > enters a "1", then enable, if the user enters a "0" then disable. Right, but remember that the regulator API does refcounting. Writing to the file won't always have an immediate effect, it'll update the refcount which may or may not do what's expected to the actual hardware. One other thing: your code checks for operations before it creates files but the operations may not be permitted by the constraints. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
