| Message ID | 1371849949-12649-2-git-send-email-nm@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 21, 2013 at 04:25:42PM -0500, Nishanth Menon wrote: > +static const struct omap_pmic_info omap_twl4030_vdd1 = { > + .slave_addr = 0x12, > + .voltage_reg_addr = 0x00, > + .cmd_reg_addr = 0x00, > + .i2c_timeout_us = 200, > + .slew_rate_uV = 4000, > + .step_size_uV = 12500, > + .min_uV = 600000, > + .max_uV = 1450000, > + .voltage_selector_offset = 0, > + .voltage_selector_mask = 0x7F, > + .voltage_selector_setbits = 0x0, > + .voltage_selector_zero = false, > +}; So, this still has the thing where all the data about the PMIC is replicated (but now in this driver). It should be possible to pull all the above information except possibly the I2C timeout and perhaps the I2C address (if there's a separate control interface) from the standard regulator core data structures for the PMIC. This would allow the driver to Just Work with any PMIC without needing to add it in two places. Other than that this looks good, the only issue I see is where the driver is getting the data from.
On 16:41-20130704, Mark Brown wrote: > On Fri, Jun 21, 2013 at 04:25:42PM -0500, Nishanth Menon wrote: > > > +static const struct omap_pmic_info omap_twl4030_vdd1 = { > > + .slave_addr = 0x12, > > + .voltage_reg_addr = 0x00, > > + .cmd_reg_addr = 0x00, > > + .i2c_timeout_us = 200, > > + .slew_rate_uV = 4000, > > + .step_size_uV = 12500, > > + .min_uV = 600000, > > + .max_uV = 1450000, > > + .voltage_selector_offset = 0, > > + .voltage_selector_mask = 0x7F, > > + .voltage_selector_setbits = 0x0, > > + .voltage_selector_zero = false, > > +}; > > So, this still has the thing where all the data about the PMIC is > replicated (but now in this driver). It should be possible to pull all > the above information except possibly the I2C timeout and perhaps the > I2C address (if there's a separate control interface) from the standard > regulator core data structures for the PMIC. This would allow the > driver to Just Work with any PMIC without needing to add it in two > places. > > Other than that this looks good, the only issue I see is where the > driver is getting the data from. I like the idea and had tried to implement it as well..lets get down to the details: If I understand what you are stating, regulators such as twl-regulator has the same/similar data that is represented here. twl-regulator uses standard i2c path, it cannot send voltage over so called "SR path". Alrite, so, lets say we reuse definition of VDD1, option 1) we just bypass get_voltage/set_voltage to point through to this function. result will be something similar to what we got here[1] Again, based on this discussion, it is wrong - and we already did implement the *wrong* way in OMAP and the new code is supposed to throw away the old code once all relevant platforms are converted to DT. - now, we could improve this by passing rdev and slurp out required data from regulator structures, but obviously, as you pointed out, it wont be sufficient. - In this solution, we will have existing regulator drivers supporting additional data path. *but* that also means that existing regulator drivers will need to be modified to handle multiple data path and "Just Work" wont happen - so not really good other than screw up existing regulator drivers with handling OMAP specific APIs in them. option 2) make omap_pmic regulator use twl_regulator - regulator chaining. - we already agreed that this is conceptually wrong[2](regulator talking to another regulator). So wont debate more here. - but here, you'd have omap_pmic regulator "using twl/existing regulators" to pick up data about slew, conversion information etc.. This series attempts to keep omap pmic delta simplified to just the additional PMIC data, no replication of code or "OMAP specific infection" of existing generic regulators. Is there any other suggestions how to pick up the data from an existing regulator and avoid few bytes of data replication? [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-omap2/omap_twl.c#n142 [2] http://marc.info/?t=136513865200005&r=1&w=2
On Fri, Jul 05, 2013 at 08:55:07AM -0500, Nishanth Menon wrote: Please write in paragraphs, an enormous wall of unbroken text isn't helpful for legibility. > On 16:41-20130704, Mark Brown wrote: > > So, this still has the thing where all the data about the PMIC is > > replicated (but now in this driver). It should be possible to pull all > > the above information except possibly the I2C timeout and perhaps the > > I2C address (if there's a separate control interface) from the standard > > regulator core data structures for the PMIC. This would allow the > > driver to Just Work with any PMIC without needing to add it in two > > places. > option 1) we just bypass get_voltage/set_voltage to point through to > this function. result will be something similar to what we got here[1] I don't really know what you mean when you say "bypass get_voltage/set_voltage so it's kind of hard to comment... the link you posted appears to be a link to the code I'm reviewing so it's not terribly enlightening. > Again, based on this discussion, it is wrong - and we already did implement > the *wrong* way in OMAP and the new code is supposed to throw away the > old code once all relevant platforms are converted to DT. > - now, we could improve this by passing rdev and slurp out required > data from regulator structures, but obviously, as you pointed out, it wont > be sufficient. > - In this solution, we will have existing regulator drivers supporting > additional data path. *but* that also means that existing regulator > drivers will need to be modified to handle multiple data path and "Just > Work" wont happen - so not really good other than screw up existing > regulator drivers with handling OMAP specific APIs in them. What makes you think that the existing regulator drivers need to be modified? They already have all the data exported to the core (or should do)... the only thing that's missing is the timeouts and it's not at all obvious why those need to be tuned per device. > option 2) make omap_pmic regulator use twl_regulator - regulator > chaining. Again I don't know what this is.
On 07/05/2013 09:08 AM, Mark Brown wrote: > On Fri, Jul 05, 2013 at 08:55:07AM -0500, Nishanth Menon wrote: > > Please write in paragraphs, an enormous wall of unbroken text isn't > helpful for legibility. Apologies on the same. Will try to do better. > >> On 16:41-20130704, Mark Brown wrote: > >>> So, this still has the thing where all the data about the PMIC is >>> replicated (but now in this driver). It should be possible to pull all >>> the above information except possibly the I2C timeout and perhaps the >>> I2C address (if there's a separate control interface) from the standard >>> regulator core data structures for the PMIC. This would allow the >>> driver to Just Work with any PMIC without needing to add it in two >>> places. > >> option 1) we just bypass get_voltage/set_voltage to point through to >> this function. result will be something similar to what we got here[1] > > I don't really know what you mean when you say "bypass get_voltage/set_voltage > so it's kind of hard to comment... the link you posted appears to be a > link to the code I'm reviewing so it's not terribly enlightening. :) it is similar, yes. by bypass get/set_voltage, I mean something like [1] > >> Again, based on this discussion, it is wrong - and we already did implement >> the *wrong* way in OMAP and the new code is supposed to throw away the >> old code once all relevant platforms are converted to DT. >> - now, we could improve this by passing rdev and slurp out required >> data from regulator structures, but obviously, as you pointed out, it wont >> be sufficient. >> - In this solution, we will have existing regulator drivers supporting >> additional data path. *but* that also means that existing regulator >> drivers will need to be modified to handle multiple data path and "Just >> Work" wont happen - so not really good other than screw up existing >> regulator drivers with handling OMAP specific APIs in them. > > What makes you think that the existing regulator drivers need to be > modified? data path difference - Almost all standard regulators use i2c (standard i2c APIs) for every other SMPS/LDO except for the ones controlled by OMAP custom i2c path(vc/vp), the set_voltage/get_voltage needs a different implementation when it comes to using the vc/vp path. > They already have all the data exported to the core (or > should do)... I see that twl-regulator does not indeed do it, but, assuming the regulators have all the data exported to the core, the data is hidden in struct regulator_desc which is private to the device and driver. lets go through these: > +static const struct omap_pmic_info omap_twl4030_vdd1 = { > + .slave_addr = 0x12, Not readily exposed, but we can solve that using reg=<0x12>; in dts - this might not be duplication as the register address will be unique for the bus. > + .voltage_reg_addr = 0x00, desc->vsel_reg maps fine > + .cmd_reg_addr = 0x00, command register is used for sending low power state commands - which is not the same as voltage register or vsel_reg as used in depicted in regulator_desc. > + .i2c_timeout_us = 200, yep, does not match up. > + .slew_rate_uV = 4000, desc->ramp_delay maps fine > + .step_size_uV = 12500, desc->uV_step maps fine. > + .min_uV = 600000, desc->min_uV maps fine. > + .max_uV = 1450000, can be used with constraints, but most regulator drivers seem to store this internally. > + .voltage_selector_offset = 0, > + .voltage_selector_mask = 0x7F, > + .voltage_selector_setbits = 0x0, > + .voltage_selector_zero = false, to an extent we can map these to vsel_mask, linear_min_sel etc. (again assuming the regulator driver has populated it - most that implement custom set_voltage, get_voltage do not do that. Now, How do you suggest I pick up this data in omap_pmic regulator? linux/regulator/consumer.h does not seem appropriate omap_pmic is not really a consumer of the regulator and further all the detailed data is not exposed either. Other option is to make rdev available to omap_pmic regulator - which again implies change in existing regulator driver? > the only thing that's missing is the timeouts and it's > not at all obvious why those need to be tuned per device. OMAP VC hardware has no idea about how long to wait before giving up on an ongoing i2c transaction. This may depend on PMIC and what it does before acking on i2c. > >> option 2) make omap_pmic regulator use twl_regulator - regulator >> chaining. > > Again I don't know what this is. Lets ignore this for the moment. [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=63bfff4e20211b464cbea6e79e5fd36df227c154
On Fri, Jul 05, 2013 at 09:50:34AM -0500, Nishanth Menon wrote: > On 07/05/2013 09:08 AM, Mark Brown wrote: > >>option 1) we just bypass get_voltage/set_voltage to point through to > >>this function. result will be something similar to what we got here[1] > >I don't really know what you mean when you say "bypass get_voltage/set_voltage > >so it's kind of hard to comment... the link you posted appears to be a > >link to the code I'm reviewing so it's not terribly enlightening. > :) it is similar, yes. by bypass get/set_voltage, I mean something like [1] No, that's not a good idea. > >What makes you think that the existing regulator drivers need to be > >modified? > data path difference - Almost all standard regulators use i2c > (standard i2c APIs) for every other SMPS/LDO except for the ones > controlled by OMAP custom i2c path(vc/vp), the > set_voltage/get_voltage needs a different implementation when it > comes to using the vc/vp path. > > They already have all the data exported to the core (or > > should do)... > I see that twl-regulator does not indeed do it, but, assuming the > regulators have all the data exported to the core, the data is > hidden in struct regulator_desc which is private to the device and > driver. lets go through these: That's just a simple matter of programming to fix, and any regulator which can work with this sort of table based stuff you're looking at here must also be able to be converted to work with the equivalent code in the regulator core so open coding is a deficiency in the driver. > > + .cmd_reg_addr = 0x00, > command register is used for sending low power state commands - > which is not the same as voltage register or vsel_reg as used in > depicted in regulator_desc. There's no information about how to use this register in your bindings... but anyway, can't be too hard to add this if it's actually used. > > + .i2c_timeout_us = 200, > yep, does not match up. Like I say I just don't see why this is even specified per device. > > + .max_uV = 1450000, > can be used with constraints, but most regulator drivers seem to > store this internally. Or just trivially calculate it as we do currently. > > + .voltage_selector_offset = 0, > > + .voltage_selector_mask = 0x7F, > > + .voltage_selector_setbits = 0x0, > > + .voltage_selector_zero = false, > to an extent we can map these to vsel_mask, linear_min_sel etc. > (again assuming the regulator driver has populated it - most that > implement custom set_voltage, get_voltage do not do that. Anything that implements a custom set_voltage() won't work with your data structure either... > Other option is to make rdev available to omap_pmic regulator - > which again implies change in existing regulator driver? Why would any of the drivers need to change to do this? They're already exporting the information. > >the only thing that's missing is the timeouts and it's > >not at all obvious why those need to be tuned per device. > OMAP VC hardware has no idea about how long to wait before giving up > on an ongoing i2c transaction. This may depend on PMIC and what it > does before acking on i2c. So pick a high number (it's only for error cases...)?
On 07/05/2013 11:52 AM, Mark Brown wrote: > On Fri, Jul 05, 2013 at 09:50:34AM -0500, Nishanth Menon wrote: >> On 07/05/2013 09:08 AM, Mark Brown wrote: > >>>> option 1) we just bypass get_voltage/set_voltage to point through to >>>> this function. result will be something similar to what we got here[1] > >>> I don't really know what you mean when you say "bypass get_voltage/set_voltage >>> so it's kind of hard to comment... the link you posted appears to be a >>> link to the code I'm reviewing so it's not terribly enlightening. > >> :) it is similar, yes. by bypass get/set_voltage, I mean something like [1] > > No, that's not a good idea. I agree. > >>> What makes you think that the existing regulator drivers need to be >>> modified? > >> data path difference - Almost all standard regulators use i2c >> (standard i2c APIs) for every other SMPS/LDO except for the ones >> controlled by OMAP custom i2c path(vc/vp), the >> set_voltage/get_voltage needs a different implementation when it >> comes to using the vc/vp path. > >>> They already have all the data exported to the core (or >>> should do)... > >> I see that twl-regulator does not indeed do it, but, assuming the >> regulators have all the data exported to the core, the data is >> hidden in struct regulator_desc which is private to the device and >> driver. lets go through these: > > That's just a simple matter of programming to fix, and any regulator > which can work with this sort of table based stuff you're looking at > here must also be able to be converted to work with the equivalent code > in the regulator core so open coding is a deficiency in the driver. OK, conceptually, I am a bit lost here (may be I thinking about "how the heck am I supposed to implement this?") - Hoping for your patience in trying to get through to my thick skull :) Taking an example of twl-regulator and omap_pmic, are you suggesting omap_pmic to be a user twl-regulator using include/linux/regulator/consumer.h? or are you suggesting that omap_pmic should not be a regulator at all? Could you suggest what you have in your mind here, I can see how we can make that happen. I am not averse to writing new code ofcourse. > >>> + .cmd_reg_addr = 0x00, > >> command register is used for sending low power state commands - >> which is not the same as voltage register or vsel_reg as used in >> depicted in regulator_desc. > > There's no information about how to use this register in your > bindings... but anyway, can't be too hard to add this if it's actually > used. Yes it is, and also happens to be how OMAPs achieve maximum power savings - when low power modes are achieved in OMAP(automatic hardware assisted commands are send to the specific command registers in PMIC and viceversa on wakeup) - but this also happens to be very specific to OMAP way of handling things. I can refer to the Reference Manual as to how it actually works, but that'd be an overkill, I will try to expand on the bindings a little more, I guess. > >>> + .i2c_timeout_us = 200, > >> yep, does not match up. > > Like I say I just don't see why this is even specified per device. > >>> + .max_uV = 1450000, > >> can be used with constraints, but most regulator drivers seem to >> store this internally. > > Or just trivially calculate it as we do currently. > >>> + .voltage_selector_offset = 0, >>> + .voltage_selector_mask = 0x7F, >>> + .voltage_selector_setbits = 0x0, >>> + .voltage_selector_zero = false, > >> to an extent we can map these to vsel_mask, linear_min_sel etc. >> (again assuming the regulator driver has populated it - most that >> implement custom set_voltage, get_voltage do not do that. > > Anything that implements a custom set_voltage() won't work with your > data structure either... It would not work with OMAP either ;). But that said, drivers do freely implement custom set_voltage/get_voltage primarily because there are ranges in supported voltages that are non-linear and try to be generic to work on non-OMAP platforms as well. However, within the supported range, only the linear ranges are used with OMAP. > >> Other option is to make rdev available to omap_pmic regulator - >> which again implies change in existing regulator driver? > > Why would any of the drivers need to change to do this? They're already > exporting the information. I am thinking here: two regulator drivers, using same rdev/desc for getting the data. probably makes no sense, I agree. > >>> the only thing that's missing is the timeouts and it's >>> not at all obvious why those need to be tuned per device. > >> OMAP VC hardware has no idea about how long to wait before giving up >> on an ongoing i2c transaction. This may depend on PMIC and what it >> does before acking on i2c. > > So pick a high number (it's only for error cases...)? > from hardware perspective yeah, if it does not lockup (based on erratas on specific devices ;) ). it also controls in part, the latency of response to Voltage processor from Voltage controller also needed for computing SmartReflex latencies (as it should consider worst case conditions)
On Fri, Jul 05, 2013 at 12:33:10PM -0500, Nishanth Menon wrote: > Taking an example of twl-regulator and omap_pmic, are you suggesting > omap_pmic to be a user twl-regulator using > include/linux/regulator/consumer.h? or are you suggesting that > omap_pmic should not be a regulator at all? No, I'm suggesting that omap_pmic find the TWL driver data at runtime (eg, using the device tree to locate the relevant regulator) and get the information out of the regulator driver that way. It can then tell the hardware about the data that way without having to explictly add every single regulator both standalone and to the OMAP driver. > >There's no information about how to use this register in your > >bindings... but anyway, can't be too hard to add this if it's actually > >used. > Yes it is, and also happens to be how OMAPs achieve maximum power > savings - when low power modes are achieved in OMAP(automatic > hardware assisted commands are send to the specific command > registers in PMIC and viceversa on wakeup) - but this also happens > to be very specific to OMAP way of handling things. I can refer to > the Reference Manual as to how it actually works, but that'd be an > overkill, I will try to expand on the bindings a little more, I > guess. OK, so this is a register defined by the OMAP architecture? I think it's reasonable to add something to allow this to be obtained to the core, using a DT property seems yucky since every board usingt this is going to have to cut'n'paste the value. Some sort of custom parameter readback thing perhaps, it doesn't have to be too generic. > >Anything that implements a custom set_voltage() won't work with your > >data structure either... > It would not work with OMAP either ;). But that said, drivers do Yes, that's kind of my point - as with the code Paul was implementing it doesn't matter if you can't support every single regualtor since the hardware design constrains what the regulator can do. The regualtor framework already has helpers which factor out the code for anything which has the limiations the OMAP hardware has (or where it doesn't we could add them) so there shouldn't be any need for a driver to provide custom callbacks. > freely implement custom set_voltage/get_voltage primarily because > there are ranges in supported voltages that are non-linear and try > to be generic to work on non-OMAP platforms as well. However, within > the supported range, only the linear ranges are used with OMAP. OK, that's a bit more interesting but I expect such regulators will actually work with the linear ranges helpers I added the other day (Marvell had a PMIC using them and I realised that the same pattern can be applied to a bunch of other devices). Do you think that'd cover the cases you're aware of? Another option is for the drivers to provide the data and use the helpers for their linear ranges as part of a more complex implementation. > >>OMAP VC hardware has no idea about how long to wait before giving up > >>on an ongoing i2c transaction. This may depend on PMIC and what it > >>does before acking on i2c. > >So pick a high number (it's only for error cases...)? > from hardware perspective yeah, if it does not lockup (based on > erratas on specific devices ;) ). it also controls in part, the > latency of response to Voltage processor from Voltage controller > also needed for computing SmartReflex latencies (as it should > consider worst case conditions) OK, that's a bit more fun but I think the kernel wants that information in general anyway since a software cpufreq driver or something might want to make the same latency decisions. This is what set_voltage_time() is for in part. But to a first approximation is there really much variation in the numbers?
On 07/05/2013 12:47 PM, Mark Brown wrote: > On Fri, Jul 05, 2013 at 12:33:10PM -0500, Nishanth Menon wrote: > >> Taking an example of twl-regulator and omap_pmic, are you suggesting >> omap_pmic to be a user twl-regulator using >> include/linux/regulator/consumer.h? or are you suggesting that >> omap_pmic should not be a regulator at all? > > No, I'm suggesting that omap_pmic find the TWL driver data at runtime > (eg, using the device tree to locate the relevant regulator) and get the > information out of the regulator driver that way. It can then tell the > hardware about the data that way without having to explictly add every > single regulator both standalone and to the OMAP driver. Apologies on delayed response, I had spend sometime thinking about this(and try some prototype code), here it goes: case #1 - TPS62361 has a single SMPS and a single generic i2c bus, one can talk on. In this case, you'd associate the regulator device in one place - i2cX or on custom SoC hardware interface. When used with custom SoC hardware interface, generic tps62361 regulator which talks regular i2c wont even probe for omap_pmic to get the regulator data from tps62361 regulator driver. Even if we were to define the generic TPS62361 in dts nodes, it will fail probe as it cant access any of it's registers using generic i2c. case #2 - TWL6030/2 has a multiple SMPS and LDOs and a two i2c bus one can talk on. however, SMPS meant for SoC can only be controlled by custom SoC hardware. In this case, we'd not even have a regulator_register taking place for SMPS controlled by custom hardware. I am lost as to how we can even do this? All the dts node tells for an SMPS today is: vaux3: regulator-vaux3 { compatible = "ti,twl6030-vaux3"; } as an example. the slew data is embedded inside the twl-regulator data structures - if it never gets probed, it wont make that data available in regulator core information. so doing something like: omap_pmic_vdd_mpu { pmic-supply = <&vcore1>; }; wont work, unless we define a phandle vcore1. vcore1: regulator-vcore1 { compatible="ti,twl6030-vcore1"; }; wont help us either, because this wont be an SMPS that twl-regulator can even try to control. So, how would omap_pmic find vcore1 twl driver data when it does not even twl6030 does not even probe vcore1? and making twl-regulator.c probe vcore1 (aka making it probe for a device that it cannot manage) is conceptually wrong, no (not to mention wrong definition of two device nodes in dt blob for the same device)? > >>> There's no information about how to use this register in your >>> bindings... but anyway, can't be too hard to add this if it's actually >>> used. > >> Yes it is, and also happens to be how OMAPs achieve maximum power >> savings - when low power modes are achieved in OMAP(automatic >> hardware assisted commands are send to the specific command >> registers in PMIC and viceversa on wakeup) - but this also happens >> to be very specific to OMAP way of handling things. I can refer to >> the Reference Manual as to how it actually works, but that'd be an >> overkill, I will try to expand on the bindings a little more, I >> guess. > > OK, so this is a register defined by the OMAP architecture? I think I might say yes, but that will only be because I just know OMAP architecture alone, Have'nt had enough indepth knowledge of other architectures to make a broader statement :( > it's reasonable to add something to allow this to be obtained to the > core, using a DT property seems yucky since every board usingt this is > going to have to cut'n'paste the value. Some sort of custom parameter > readback thing perhaps, it doesn't have to be too generic. custom of->match data parameter is how it is done today in this patch > >>> Anything that implements a custom set_voltage() won't work with your >>> data structure either... > >> It would not work with OMAP either ;). But that said, drivers do > > Yes, that's kind of my point - as with the code Paul was implementing it > doesn't matter if you can't support every single regualtor since the > hardware design constrains what the regulator can do. The regualtor > framework already has helpers which factor out the code for anything > which has the limiations the OMAP hardware has (or where it doesn't we > could add them) so there shouldn't be any need for a driver to provide > custom callbacks. custom callback context was as follows: if we force the SoC generic regulators to use OMAP custom interfaces, then we have a need for custom callbacks, else w.r.t PMIC data, covered above, we do not need any custom callbacks, agreed. >> freely implement custom set_voltage/get_voltage primarily because >> there are ranges in supported voltages that are non-linear and try >> to be generic to work on non-OMAP platforms as well. However, within >> the supported range, only the linear ranges are used with OMAP. > > OK, that's a bit more interesting but I expect such regulators will > actually work with the linear ranges helpers I added the other day > (Marvell had a PMIC using them and I realised that the same pattern can > be applied to a bunch of other devices). Do you think that'd cover the > cases you're aware of? > > Another option is for the drivers to provide the data and use the > helpers for their linear ranges as part of a more complex > implementation. Having looked at a few regulator driver implementations, there seems to be the following combinations: a) drivers which have n ranges of linear voltages for incremental selector b) drivers with 1 range of linear voltages only for all ranges of selectors c) drivers with 1 range of linear voltages and nonlinear voltage values for other vsels. > >>>> OMAP VC hardware has no idea about how long to wait before giving up >>>> on an ongoing i2c transaction. This may depend on PMIC and what it >>>> does before acking on i2c. > >>> So pick a high number (it's only for error cases...)? > >> from hardware perspective yeah, if it does not lockup (based on >> erratas on specific devices ;) ). it also controls in part, the >> latency of response to Voltage processor from Voltage controller >> also needed for computing SmartReflex latencies (as it should >> consider worst case conditions) > > OK, that's a bit more fun but I think the kernel wants that information > in general anyway since a software cpufreq driver or something might > want to make the same latency decisions. This is what set_voltage_time() > is for in part. But to a first approximation is there really much > variation in the numbers? between PMICs? yep, twl4030 does 4mV/uSec, 6030 can do 6mV/uSec, TPS62361 can do 32mV/uSec, TWL6035/37 does 0.220mV/uSec For a given PMIC, voltage X to Y time will vary ofcourse. SmartReflex AVS is something cpufreq wont know about -> completely different topic of its own :) What we are representing is data for PMIC supported on specific SoC hardware controller interface meant just for PMIC - IMHO, it is similar to data that twl-regulator stores for the SMPS/LDOs that it can control - given these are starting to become legacy platforms (OMAP3,4,5) and the fact that it is a one-off(at this point in time) knwon custom hardware doing stuff like this, we would'nt expect too much churn either there(new PMICs added etc). On the flip side, I do understand your concern with scalability and ease of re-use here. but given that we have'nt seen much of a churn in this data for years on the go now, I'd expect lesser in the future. This dedicated interface has been dropped in our future chips precisely due to the lack of scalability to PMIC choices such as those using SPI. just my 2 cents.
On Mon, Jul 08, 2013 at 12:22:36PM -0500, Nishanth Menon wrote: > case #1 - TPS62361 has a single SMPS and a single generic i2c bus, > one can talk on. In this case, you'd associate the regulator device > in one place - i2cX or on custom SoC hardware interface. > When used with custom SoC hardware interface, generic tps62361 > regulator which talks regular i2c wont even probe for omap_pmic to > get the regulator data from tps62361 regulator driver. Even if we > were to define the generic TPS62361 in dts nodes, it will fail probe > as it cant access any of it's registers using generic i2c. This seems like something we should be able to cope with by for example adding a bus for the custom PMIC interface or otherwise finding a way to get to the data at runtime based on the compatible string. This would need some custom code in the regulators but would have the advantage of keeping the data the same at least. Hrm. > >Another option is for the drivers to provide the data and use the > >helpers for their linear ranges as part of a more complex > >implementation. > Having looked at a few regulator driver implementations, there seems > to be the following combinations: > a) drivers which have n ranges of linear voltages for incremental selector > b) drivers with 1 range of linear voltages only for all ranges of selectors > c) drivers with 1 range of linear voltages and nonlinear voltage > values for other vsels. Everything else is just a special case of option a - either there's just a single range or there's a bunch of ranges each with a single value. I suspect that ranges will be the most useful thing for any hardware which can practically be used by these regulators anyway. > >OK, that's a bit more fun but I think the kernel wants that information > >in general anyway since a software cpufreq driver or something might > >want to make the same latency decisions. This is what set_voltage_time() > >is for in part. But to a first approximation is there really much > >variation in the numbers? > between PMICs? yep, twl4030 does 4mV/uSec, 6030 can do 6mV/uSec, > TPS62361 can do 32mV/uSec, TWL6035/37 does 0.220mV/uSec Those are ramp rates, they're not I2C I/O limits. Ramp rates we already know about. I think what you're saying here is that this latency value is actually about worst case ramp times?
On 07/09/2013 10:29 AM, Mark Brown wrote: > On Mon, Jul 08, 2013 at 12:22:36PM -0500, Nishanth Menon wrote: > >> case #1 - TPS62361 has a single SMPS and a single generic i2c bus, >> one can talk on. In this case, you'd associate the regulator device >> in one place - i2cX or on custom SoC hardware interface. > >> When used with custom SoC hardware interface, generic tps62361 >> regulator which talks regular i2c wont even probe for omap_pmic to >> get the regulator data from tps62361 regulator driver. Even if we >> were to define the generic TPS62361 in dts nodes, it will fail probe >> as it cant access any of it's registers using generic i2c. > > This seems like something we should be able to cope with by for example > adding a bus for the custom PMIC interface or otherwise finding a way to I had considered introducing a custom bus for custom PMIC interface, but as you stated below, standard regulators will probably need some custom monkeying around. > get to the data at runtime based on the compatible string. This would > need some custom code in the regulators but would have the advantage of > keeping the data the same at least. Hrm. Ofcourse,this will be to add custom set/get_voltage implementation using this "custom API" which we discussed was'nt that good an idea. I am at a stalemate as to where we should go from here. > >>> Another option is for the drivers to provide the data and use the >>> helpers for their linear ranges as part of a more complex >>> implementation. > >> Having looked at a few regulator driver implementations, there seems >> to be the following combinations: >> a) drivers which have n ranges of linear voltages for incremental selector >> b) drivers with 1 range of linear voltages only for all ranges of selectors >> c) drivers with 1 range of linear voltages and nonlinear voltage >> values for other vsels. > > Everything else is just a special case of option a - either there's just > a single range or there's a bunch of ranges each with a single value. I > suspect that ranges will be the most useful thing for any hardware which > can practically be used by these regulators anyway. True, but slightly different topic though. > >>> OK, that's a bit more fun but I think the kernel wants that information >>> in general anyway since a software cpufreq driver or something might >>> want to make the same latency decisions. This is what set_voltage_time() >>> is for in part. But to a first approximation is there really much >>> variation in the numbers? > >> between PMICs? yep, twl4030 does 4mV/uSec, 6030 can do 6mV/uSec, >> TPS62361 can do 32mV/uSec, TWL6035/37 does 0.220mV/uSec > > Those are ramp rates, they're not I2C I/O limits. Ramp rates we already > know about. I think what you're saying here is that this latency value > is actually about worst case ramp times? Arrgh.. my bad. I confused ramp time with I2C transfer timeout parameter. I know that I2C bus can be held[1] by PMIC as long as it is busy. Some custom ASIC can do some weird stuff I suppose. I dont seem to have clear data points in the sketchy TRMs for 6030/2 , 6035, 5030, for these other than to state it is i2c specification compliant (/me grumbles). So, I just have emperical value which is a bit conservative and seem to work on the devices. [1] http://www.i2c-bus.org/clock-generation-stretching-arbitration/
On Tue, Jul 09, 2013 at 11:04:23AM -0500, Nishanth Menon wrote: > On 07/09/2013 10:29 AM, Mark Brown wrote: > >This seems like something we should be able to cope with by for example > >adding a bus for the custom PMIC interface or otherwise finding a way to > I had considered introducing a custom bus for custom PMIC interface, > but as you stated below, standard regulators will probably need some > custom monkeying around. We should just be able to use the platform bus here. > >get to the data at runtime based on the compatible string. This would > >need some custom code in the regulators but would have the advantage of > >keeping the data the same at least. Hrm. > Ofcourse,this will be to add custom set/get_voltage implementation > using this "custom API" which we discussed was'nt that good an idea. No, if the regulator isn't being interacted with directly then it doesn't need to export any operations - just data. The operations would come from the magic SoC hardware that controls the regulator. The code in the drivers should be very small, if it isn't there's no point in doing this. > >>between PMICs? yep, twl4030 does 4mV/uSec, 6030 can do 6mV/uSec, > >>TPS62361 can do 32mV/uSec, TWL6035/37 does 0.220mV/uSec > >Those are ramp rates, they're not I2C I/O limits. Ramp rates we already > >know about. I think what you're saying here is that this latency value > >is actually about worst case ramp times? > Arrgh.. my bad. I confused ramp time with I2C transfer timeout > parameter. I know that I2C bus can be held[1] by PMIC as long as it > is busy. Some custom ASIC can do some weird stuff I suppose. I dont > seem to have clear data points in the sketchy TRMs for 6030/2 , > 6035, 5030, for these other than to state it is i2c specification > compliant (/me grumbles). So, I just have emperical value which is a > bit conservative and seem to work on the devices. OK, no problem - like we said further up the thread I think adding something to get the data
diff --git a/Documentation/devicetree/bindings/regulator/omap-pmic-regulator.txt b/Documentation/devicetree/bindings/regulator/omap-pmic-regulator.txt new file mode 100644 index 0000000..6991e58 --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/omap-pmic-regulator.txt @@ -0,0 +1,41 @@ +Generic Power Management IC(PMIC) Regulator for Texas Instruments OMAP SoCs + +Required Properties: +- compatible: Should be one of: + - "ti,omap-twl4030-vdd1" - TWL4030/5030/TPS65950 VDD1 + - "ti,omap-twl4030-vdd2" - TWL4030/5030/TPS65950 VDD2 + - "ti,omap-twl6030-vcore1" - TWL6030 VCORE1 + - "ti,omap-twl6030-vcore2" - TWL6030 VCORE2 + - "ti,omap-twl6030-vcore3" - TWL6030 VCORE3 + - "ti,omap-tps62361" - TPS62361 VSEL1 + - "ti,omap-twl6032-smps1" - TWL6032 SMPS1 + - "ti,omap-twl6032-smps2" - TWL6032 SMPS2 + - "ti,omap-twl6032-smps5" - TWL6032 SMPS5 + - "ti,omap-twl6035-smps1" - TWL6035/6037 in SMPS1, 12, 123 configuration + - "ti,omap-twl6035-smps4" - TWL6035/6037 in SMPS4, 45 configuration + - "ti,omap-twl6035-smps8" - TWL6035/6037 in SMPS8 configuration + +Optional Properties: +- gpios: OF device-tree gpio specification - can be an array, will be setup + in the order of definition and set to the flags. +- pinctrl: OF device-tree pinctrl definitions as needed (usually for the GPIOs) +- ti,boot-voltage-micro-volts - voltage in microvolts that bootloader is leaving + over the PMIC at. This may be 'PMIC data manual configuration' if + bootloader does not set an value, or appropritate value. + + +Example: Sample PMIC description +omap_tps62361: tps62361 { + compatible = "ti,omap-tps62361"; +}; + +/* Board Specific configuration */ +&omap_tps62361 { + pinctrl-names = "default"; + pinctrl-0 = < + &tps62361_wkgpio_pins + >; + gpios = <&gpio1 7 1>; /* gpio_wk7 */ + + ti,boot-voltage-micro-volts=<1200000>; +}; diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig index 9296425..e14b534 100644 --- a/drivers/regulator/Kconfig +++ b/drivers/regulator/Kconfig @@ -482,6 +482,18 @@ config REGULATOR_TI_ABB on TI SoCs may be unstable without enabling this as it provides device specific optimized bias to allow/optimize functionality. +config REGULATOR_TI_OMAP_PMIC + tristate "TI Generic regulator for Voltage Processor / Controller" + depends on ARCH_OMAP + help + Select this option to support PMICs over Texas Instruments' + custom Voltage Processor + Voltage Controller data interface + used in OMAP SoCs. This is a specific "write-only" interface + designed to interface with I2C based PMICs. + + This option enables the regulator driver representing the PMIC + on the OMAP VC/VP hardware. + config REGULATOR_VEXPRESS tristate "Versatile Express regulators" depends on VEXPRESS_CONFIG diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile index 26e6c4a..8874e74 100644 --- a/drivers/regulator/Makefile +++ b/drivers/regulator/Makefile @@ -64,6 +64,7 @@ obj-$(CONFIG_REGULATOR_TPS65912) += tps65912-regulator.o obj-$(CONFIG_REGULATOR_TPS80031) += tps80031-regulator.o obj-$(CONFIG_REGULATOR_TWL4030) += twl-regulator.o obj-$(CONFIG_REGULATOR_TI_ABB) += ti-abb-regulator.o +obj-$(CONFIG_REGULATOR_TI_OMAP_PMIC) += omap-pmic-regulator.o obj-$(CONFIG_REGULATOR_VEXPRESS) += vexpress.o obj-$(CONFIG_REGULATOR_WM831X) += wm831x-dcdc.o obj-$(CONFIG_REGULATOR_WM831X) += wm831x-isink.o diff --git a/drivers/regulator/omap-pmic-regulator.c b/drivers/regulator/omap-pmic-regulator.c new file mode 100644 index 0000000..40c725f --- /dev/null +++ b/drivers/regulator/omap-pmic-regulator.c @@ -0,0 +1,638 @@ +/* + * OMAP Generic PMIC Regulator + * + * Idea based on arch/arm/mach-omap2/omap_twl.c + * Copyright (C) 2010 Texas Instruments Incorporated. + * Thara Gopinath + * Copyright (C) 2009 Texas Instruments Incorporated. + * Nishanth Menon + * Copyright (C) 2009 Nokia Corporation + * Paul Walmsley + * + * Copyright (C) 2013 Texas Instruments Incorporated + * Taras Kondratiuk + * Grygorii Strashko + * Nishanth Menon + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed "as is" WITHOUT ANY WARRANTY of any + * kind, whether express or implied; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__ + +#include <linux/err.h> +#include <linux/gpio.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/of_gpio.h> +#include <linux/regulator/driver.h> +#include <linux/regulator/machine.h> +#include <linux/regulator/of_regulator.h> +#include <linux/regulator/omap-pmic-regulator.h> + +#define DRIVER_NAME "omap-pmic" + +static DEFINE_MUTEX(omap_pmic_cops_mutex); +static struct omap_pmic_controller_ops *pmic_cops; + +/** + * omap_pmic_register_controller_ops() - Register voltage operations + * @cops: voltage operations + * + * It is expected that appropriate controller register it's functions + * with this driver using this interface, If this is not done, the probe + * for the corresponding device will defer till it fails. + * + * Return: -EBUSY if already registered, else returns 0 + */ +int omap_pmic_register_controller_ops(struct omap_pmic_controller_ops *cops) +{ + int ret = 0; + + mutex_lock(&omap_pmic_cops_mutex); + if (pmic_cops) { + pr_err("Controller operations already registered\n"); + ret = -EBUSY; + goto out; + } + if (!cops->devm_pmic_register || !cops->voltage_set || + !cops->voltage_get || !cops->voltage_get_range) { + pr_err("Missing operations!\n"); + ret = -EINVAL; + goto out; + } + + pmic_cops = cops; +out: + mutex_unlock(&omap_pmic_cops_mutex); + + return ret; +} +EXPORT_SYMBOL_GPL(omap_pmic_register_controller_ops); + +/** + * omap_pmic_vsel_to_uv() - Convert voltage selector(vsel) to microvolts + * @pmic: pointer to pmic struct + * @vsel: voltage selector(vsel) + * @uv: If conversion is successful, returns the voltage in micro volts + * + * Return: 0 if conversion is successful and *uv has proper value, else + * appropriate error value for failure. + */ +static int omap_pmic_vsel_to_uv(struct omap_pmic *pmic, u8 vsel, u32 *uv) +{ + u32 tmp = vsel; + const struct omap_pmic_info *info; + + if (!pmic || !uv) { + pr_err("Bad parameters pmic=%p uv=%p!\n", pmic, uv); + return -EINVAL; + } + info = pmic->info; + + if (info->voltage_selector_mask) { + tmp &= info->voltage_selector_mask; + tmp >>= __ffs(info->voltage_selector_mask); + } + + if (!tmp && info->voltage_selector_zero) + goto out; + + tmp -= info->voltage_selector_offset; + tmp *= info->step_size_uV; + tmp += info->min_uV; + + if (tmp < info->min_uV || tmp > info->max_uV) { + dev_dbg(pmic->dev, "%s: Out of range 0x%02x[%d] (%d <-> %d)\n", + __func__, vsel, tmp, info->min_uV, info->max_uV); + return -ERANGE; + } + +out: + *uv = tmp; + dev_dbg(pmic->dev, "%s: uv=%d vsel=0x%02x\n", __func__, *uv, vsel); + + return 0; +} + +/** + * omap_pmic_uv_to_vsel() - Convert microvolts to voltage selector(vsel) + * @pmic: pointer to pmic struct + * @uv: voltage in micro volts + * @vsel: If conversion is successful, voltage selector(vsel) + * + * Return: 0 if conversion is successful and *vsel has proper value, else + * appropriate error value for failure. + */ +static int omap_pmic_uv_to_vsel(struct omap_pmic *pmic, u32 uv, u8 *vsel) +{ + u32 tmp = uv; + const struct omap_pmic_info *info; + + if (!pmic || !vsel) { + pr_err("Bad parameters pmic=%p vsel=%p!\n", pmic, vsel); + return -EINVAL; + } + info = pmic->info; + + if (!tmp && info->voltage_selector_zero) + goto skip_convert; + + if (tmp > info->max_uV) + goto skip_convert; + + tmp -= info->min_uV; + tmp = DIV_ROUND_UP(tmp, info->step_size_uV); + + tmp += info->voltage_selector_offset; + +skip_convert: + if (tmp > 0xFF) { + dev_dbg(pmic->dev, "%s: Out of range 0x%04x[%d] (%d - %d)\n", + __func__, tmp, uv, info->min_uV, info->max_uV); + return -ERANGE; + } + if (info->voltage_selector_mask) { + tmp <<= __ffs(info->voltage_selector_mask); + if (tmp > 0xFF) { + dev_warn(pmic->dev, "%s: Out of range 0x%04x[%d]\n", + __func__, tmp, uv); + return -ERANGE; + } + tmp &= info->voltage_selector_mask; + } + + tmp |= info->voltage_selector_setbits; + + *vsel = tmp; + dev_dbg(pmic->dev, "%s: uv=%d vsel=0x%02x\n", __func__, uv, *vsel); + + return 0; +} + +/** + * omap_pmic_set_voltage() - regulator interface to set voltage + * @rdev: regulator device + * @min_uV: min voltage in micro-volts + * @max_uV: max voltage in micro-volts + * @unused: unused.. we dont use sel + * + * Return: -ERANGE for out of range values, appropriate error code if conversion + * fails, else returns 0. + */ +static int omap_pmic_set_voltage(struct regulator_dev *rdev, int min_uV, + int max_uV, unsigned *unused) +{ + struct omap_pmic *pmic = rdev_get_drvdata(rdev); + + return pmic_cops->voltage_set(pmic->v_dev, min_uV); +} + +/** + * omap_pmic_get_voltage() - regulator interface to get voltage + * @rdev: regulator device + * + * Return: current voltage set on PMIC OR appropriate error value + */ +static int omap_pmic_get_voltage(struct regulator_dev *rdev) +{ + struct omap_pmic *pmic = rdev_get_drvdata(rdev); + int ret; + u32 uv; + + ret = pmic_cops->voltage_get(pmic->v_dev, &uv); + if (ret) + return ret; + + return uv; +} + +static struct omap_pmic_ops omap_generic_pmic_ops = { + .vsel_to_uv = omap_pmic_vsel_to_uv, + .uv_to_vsel = omap_pmic_uv_to_vsel, +}; + +static struct regulator_ops omap_pmic_reg_ops = { + .list_voltage = regulator_list_voltage_linear, + + .set_voltage = omap_pmic_set_voltage, + .get_voltage = omap_pmic_get_voltage, +}; + +/** + * omap_pmic_of_setup_gpios() - Setup GPIO array if needed. + * @dev: device to pick up the gpios from + */ +static int omap_pmic_of_setup_gpios(struct device *dev) +{ + struct device_node *node = dev->of_node; + int num_gpios, i, ret; + + num_gpios = of_gpio_count(node); + if (num_gpios < 0) + return 0; + + for (i = 0; i < num_gpios; i++) { + int gpio, level; + enum of_gpio_flags flags; + + gpio = of_get_gpio_flags(node, i, &flags); + if (!gpio_is_valid(gpio)) { + dev_err(dev, "Invalid GPIO[%d]: %d\n", i, gpio); + return -EINVAL; + } + + ret = devm_gpio_request(dev, gpio, dev_name(dev)); + if (ret) { + dev_err(dev, "Unable to get GPIO %d (%d)\n", gpio, ret); + return ret; + } + level = (flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1; + ret = gpio_direction_output(gpio, level); + if (ret) { + dev_err(dev, "Failed to set GPIO %d to %d (%d)\n", + gpio, level, ret); + return ret; + } + dev_dbg(dev, "GPIO=%d set_to=%d flags=0x%08x\n", gpio, + level, flags); + } + + return 0; +} + +/** + * omap_pmic_parse_of() - Do DT OF node parsing + * @pmic: pointer to PMIC + */ +static int omap_pmic_parse_of(struct omap_pmic *pmic) +{ + struct device *dev = pmic->dev; + struct device_node *node = dev->of_node; + u32 val = 0; + char *pname; + int ret; + + pname = "ti,boot-voltage-micro-volts"; + ret = of_property_read_u32(node, pname, &val); + if (!ret) { + if (!val) + goto invalid_of_property; + pmic->boot_voltage_uV = val; + } + + return ret; + +invalid_of_property: + if (!ret) { + dev_err(dev, "Invalid value 0x%x[%d] in '%s' property.\n", + val, val, pname); + ret = -EINVAL; + } else { + dev_err(dev, "Missing/Invalid '%s' property - error(%d)\n", + pname, ret); + } + return ret; +} + +static const struct omap_pmic_info omap_twl4030_vdd1 = { + .slave_addr = 0x12, + .voltage_reg_addr = 0x00, + .cmd_reg_addr = 0x00, + .i2c_timeout_us = 200, + .slew_rate_uV = 4000, + .step_size_uV = 12500, + .min_uV = 600000, + .max_uV = 1450000, + .voltage_selector_offset = 0, + .voltage_selector_mask = 0x7F, + .voltage_selector_setbits = 0x0, + .voltage_selector_zero = false, +}; + +static const struct omap_pmic_info omap_twl4030_vdd2 = { + .slave_addr = 0x12, + .voltage_reg_addr = 0x01, + .cmd_reg_addr = 0x01, + .i2c_timeout_us = 200, + .slew_rate_uV = 4000, + .step_size_uV = 12500, + .min_uV = 600000, + .max_uV = 1450000, + .voltage_selector_offset = 0, + .voltage_selector_mask = 0x7F, + .voltage_selector_setbits = 0x0, + .voltage_selector_zero = false, +}; + +static const struct omap_pmic_info omap_twl6030_vcore1 = { + .slave_addr = 0x12, + .voltage_reg_addr = 0x55, + .cmd_reg_addr = 0x56, + .i2c_timeout_us = 200, + .slew_rate_uV = 9000, + .step_size_uV = 12660, + .min_uV = 709000, + .max_uV = 1418000, + .voltage_selector_offset = 0x1, + .voltage_selector_mask = 0x7F, + .voltage_selector_setbits = 0x0, + .voltage_selector_zero = true, +}; + +static const struct omap_pmic_info omap_twl6030_vcore2 = { + .slave_addr = 0x12, + .voltage_reg_addr = 0x5b, + .cmd_reg_addr = 0x5c, + .i2c_timeout_us = 200, + .slew_rate_uV = 9000, + .step_size_uV = 12660, + .min_uV = 709000, + .max_uV = 1418000, + .voltage_selector_offset = 0x1, + .voltage_selector_mask = 0x7F, + .voltage_selector_setbits = 0x0, + .voltage_selector_zero = true, +}; + +static const struct omap_pmic_info omap_twl6030_vcore3 = { + .slave_addr = 0x12, + .voltage_reg_addr = 0x61, + .cmd_reg_addr = 0x62, + .i2c_timeout_us = 200, + .slew_rate_uV = 9000, + .step_size_uV = 12660, + .min_uV = 709000, + .max_uV = 1418000, + .voltage_selector_offset = 0x1, + .voltage_selector_mask = 0x7F, + .voltage_selector_setbits = 0x0, + .voltage_selector_zero = true, +}; + +static const struct omap_pmic_setup_commands omap_tps62361_cmds[] = { + {.reg = 0x06, .cmd_val = 0x06}, /* TPS6236X_RAMP_CTRL 32mV/uS */ + {.reg = 0x04, .cmd_val = 0xc0}, /* TPS6236X_CTRL VSEL0 pull down */ + {.reg = 0x05, .cmd_val = 0x00}, /* REG_TPS6236X_TEMP enable tshut */ +}; + +static const struct omap_pmic_info omap_tps62361 = { + .slave_addr = 0x60, + .voltage_reg_addr = 0x01, + .cmd_reg_addr = 0x01, + .i2c_timeout_us = 200, + .slew_rate_uV = 32000, + .step_size_uV = 10000, + .min_uV = 500000, + .max_uV = 1770000, + .voltage_selector_offset = 0x0, + .voltage_selector_mask = 0x7F, + .voltage_selector_setbits = 0x80, /* PFM mode */ + .voltage_selector_zero = false, + .setup_command_list = omap_tps62361_cmds, + .setup_num_commands = ARRAY_SIZE(omap_tps62361_cmds), +}; + +static const struct omap_pmic_info omap_twl6032_smps1 = { + .slave_addr = 0x12, + .voltage_reg_addr = 0x55, + .cmd_reg_addr = 0x56, + .i2c_timeout_us = 200, + .slew_rate_uV = 9000, + .step_size_uV = 12660, + .min_uV = 709000, + .max_uV = 1418000, + .voltage_selector_offset = 0x1, + .voltage_selector_mask = 0x7F, + .voltage_selector_setbits = 0x0, + .voltage_selector_zero = true, +}; + +static const struct omap_pmic_info omap_twl6032_smps2 = { + .slave_addr = 0x12, + .voltage_reg_addr = 0x5b, + .cmd_reg_addr = 0x5c, + .i2c_timeout_us = 200, + .slew_rate_uV = 9000, + .step_size_uV = 12660, + .min_uV = 709000, + .max_uV = 1418000, + .voltage_selector_offset = 0x1, + .voltage_selector_mask = 0x7F, + .voltage_selector_setbits = 0x0, + .voltage_selector_zero = true, +}; + +static const struct omap_pmic_info omap_twl6032_smps5 = { + .slave_addr = 0x12, + .voltage_reg_addr = 0x49, + .cmd_reg_addr = 0x4a, + .i2c_timeout_us = 200, + .slew_rate_uV = 9000, + .step_size_uV = 12660, + .min_uV = 709000, + .max_uV = 1418000, + .voltage_selector_offset = 0x1, + .voltage_selector_mask = 0x7F, + .voltage_selector_setbits = 0x0, + .voltage_selector_zero = true, +}; + +static const struct omap_pmic_info omap_twl6035_smps1 = { + .slave_addr = 0x12, + .voltage_reg_addr = 0x23, + .cmd_reg_addr = 0x22, + .i2c_timeout_us = 200, + .slew_rate_uV = 220, + .step_size_uV = 10000, + .min_uV = 500000, + .max_uV = 1650000, + .voltage_selector_offset = 0x6, + .voltage_selector_mask = 0x7F, + .voltage_selector_setbits = 0x0, + .voltage_selector_zero = true, +}; + +static const struct omap_pmic_info omap_twl6035_smps4 = { + .slave_addr = 0x12, + .voltage_reg_addr = 0x2b, + .cmd_reg_addr = 0x2a, + .i2c_timeout_us = 200, + .slew_rate_uV = 220, + .step_size_uV = 10000, + .min_uV = 500000, + .max_uV = 1650000, + .voltage_selector_offset = 0x6, + .voltage_selector_mask = 0x7F, + .voltage_selector_setbits = 0x0, + .voltage_selector_zero = true, +}; + +static const struct omap_pmic_info omap_twl6035_smps8 = { + .slave_addr = 0x12, + .voltage_reg_addr = 0x37, + .cmd_reg_addr = 0x36, + .i2c_timeout_us = 200, + .slew_rate_uV = 220, + .step_size_uV = 10000, + .min_uV = 500000, + .max_uV = 1650000, + .voltage_selector_offset = 0x6, + .voltage_selector_mask = 0x7F, + .voltage_selector_setbits = 0x0, + .voltage_selector_zero = true, +}; + +static const struct of_device_id omap_pmic_of_match_tbl[] = { + {.compatible = "ti,omap-twl4030-vdd1", .data = &omap_twl4030_vdd1,}, + {.compatible = "ti,omap-twl4030-vdd2", .data = &omap_twl4030_vdd2,}, + {.compatible = "ti,omap-twl6030-vcore1", .data = &omap_twl6030_vcore1,}, + {.compatible = "ti,omap-twl6030-vcore2", .data = &omap_twl6030_vcore2,}, + {.compatible = "ti,omap-twl6030-vcore3", .data = &omap_twl6030_vcore3,}, + {.compatible = "ti,omap-tps62361", .data = &omap_tps62361,}, + {.compatible = "ti,omap-twl6032-smps1", .data = &omap_twl6032_smps1,}, + {.compatible = "ti,omap-twl6032-smps2", .data = &omap_twl6032_smps2,}, + {.compatible = "ti,omap-twl6032-smps5", .data = &omap_twl6032_smps5,}, + {.compatible = "ti,omap-twl6035-smps1", .data = &omap_twl6035_smps1,}, + {.compatible = "ti,omap-twl6035-smps4", .data = &omap_twl6035_smps4,}, + {.compatible = "ti,omap-twl6035-smps8", .data = &omap_twl6035_smps8,}, + {}, +}; +MODULE_DEVICE_TABLE(of, omap_pmic_of_match_tbl); + +static int omap_pmic_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *node = dev->of_node; + const struct of_device_id *match; + struct omap_pmic *pmic; + struct regulator_desc *desc; + struct regulation_constraints *c; + struct regulator_config config = { }; + struct regulator_init_data *initdata = NULL; + struct regulator_dev *rdev = NULL; + int ret = 0; + bool ops_ready; + + if (!node) { + dev_err(dev, "%s: missing device tree nodes?\n", __func__); + return -EINVAL; + } + + mutex_lock(&omap_pmic_cops_mutex); + ops_ready = pmic_cops ? true : false; + mutex_unlock(&omap_pmic_cops_mutex); + if (!ops_ready) { + dev_dbg(dev, "Voltage Operations not ready yet..\n"); + return -EPROBE_DEFER; + } + + match = of_match_device(omap_pmic_of_match_tbl, dev); + if (!match) { + /* We do not expect this to happen */ + dev_err(dev, "%s: Unable to match device\n", __func__); + return -ENODEV; + } + if (!match->data) { + dev_err(dev, "%s: Bad data in match\n", __func__); + return -EINVAL; + } + + desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL); + if (!desc) { + dev_err(dev, "%s: unable to allocate desc\n", __func__); + return -ENOMEM; + } + + pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL); + if (!pmic) { + dev_err(dev, "%s: unable to allocate pmic\n", __func__); + return -ENOMEM; + } + + /* Read mandatory OF parameters */ + pmic->dev = dev; + pmic->ops = &omap_generic_pmic_ops; + pmic->info = match->data; + + initdata = of_get_regulator_init_data(dev, node); + if (!initdata) { + dev_err(dev, "%s: Unable to alloc regulator init data\n", + __func__); + return -ENOMEM; + } + c = &initdata->constraints; + + /* Constraint to PMIC limits */ + if (pmic->info->min_uV > c->min_uV) + c->min_uV = pmic->info->min_uV; + if (pmic->info->max_uV < c->max_uV) + c->max_uV = pmic->info->max_uV; + + ret = omap_pmic_parse_of(pmic); + if (ret) + return ret; + + ret = omap_pmic_of_setup_gpios(dev); + if (ret) + return ret; + + pmic->v_dev = pmic_cops->devm_pmic_register(dev, pmic); + if (IS_ERR(pmic->v_dev)) { + dev_dbg(dev, "Registration of pmic failed (%d)\n", ret); + ret = PTR_ERR(pmic->v_dev); + return ret; + } + desc->name = dev_name(dev); + desc->owner = THIS_MODULE; + desc->type = REGULATOR_VOLTAGE; + desc->ops = &omap_pmic_reg_ops; + desc->uV_step = pmic->info->step_size_uV; + desc->ramp_delay = pmic->info->slew_rate_uV; + + c->valid_ops_mask |= REGULATOR_CHANGE_VOLTAGE; + c->always_on = true; + ret = pmic_cops->voltage_get_range(pmic->v_dev, &c->min_uV, &c->max_uV); + if (ret) { + dev_err(dev, "Voltage Range get failed (%d)\n", ret); + return ret; + } + + config.dev = dev; + config.init_data = initdata; + config.driver_data = pmic; + config.of_node = node; + + rdev = regulator_register(desc, &config); + if (IS_ERR(rdev)) { + ret = PTR_ERR(rdev); + dev_err(dev, "%s: failed to register regulator(%d)\n", + __func__, ret); + return ret; + } + + platform_set_drvdata(pdev, rdev); + + return ret; +} + +static struct platform_driver omap_pmic_driver = { + .driver = { + .name = DRIVER_NAME, + .owner = THIS_MODULE, + .of_match_table = of_match_ptr(omap_pmic_of_match_tbl), + }, + .probe = omap_pmic_probe, +}; +module_platform_driver(omap_pmic_driver); + +MODULE_DESCRIPTION("OMAP Generic PMIC Regulator"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:" DRIVER_NAME); +MODULE_AUTHOR("Texas Instruments Inc."); diff --git a/include/linux/regulator/omap-pmic-regulator.h b/include/linux/regulator/omap-pmic-regulator.h new file mode 100644 index 0000000..db0bab4 --- /dev/null +++ b/include/linux/regulator/omap-pmic-regulator.h @@ -0,0 +1,162 @@ +/* + * OMAP Generic PMIC Regulator interfaces + * + * Idea based on arch/arm/mach-omap2/omap_twl.c and + * arch/arm/mach-omap2/voltage.h + * Copyright (C) 2010 Texas Instruments Incorporated. + * Thara Gopinath + * Copyright (C) 2009 Texas Instruments Incorporated. + * Nishanth Menon + * Copyright (C) 2009 Nokia Corporation + * Paul Walmsley + * + * Copyright (C) 2013 Texas Instruments Incorporated. + * Grygorii Strashko + * Nishanth Menon + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed "as is" WITHOUT ANY WARRANTY of any + * kind, whether express or implied; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __POWER_OMAP_PMIC_H +#define __POWER_OMAP_PMIC_H + +struct omap_pmic; + +/** + * struct omap_pmic_setup_commands - setup commands over voltage controller + * @reg: device's i2c register address + * @cmd_val: command to send. + */ +struct omap_pmic_setup_commands { + u8 reg; + u8 cmd_val; +}; + +/** + * struct omap_pmic_ops - Conversion routines for voltage controller/processor + * @vsel_to_uv: convert voltage selector to micro-volts + * @uv_to_vsel: convert micro-volts to voltage selector + * + * voltage controller/processor drivers SHOULD NOT do modifications on vsel or + * make any assumptions about vsel. Instead, they may operate on micro-volts + * and request vsel conversion once they are ready to do hardware operations. + * + * This is provided over the omap_pmic structure. + */ +struct omap_pmic_ops { + int (*vsel_to_uv) (struct omap_pmic *pmic, u8 vsel, u32 *uv); + int (*uv_to_vsel) (struct omap_pmic *pmic, u32 uv, u8 *vsel); +}; + +/** + * struct omap_pmic_controller_ops - regulator operations implemented + * @devm_pmic_register: managed registration of an PMIC device with a specific + * voltage processor. Voltage processor provides an device + * handle which is remaining operations. + * NOTE: + * - This will be first interface to be invoked by + * omap_pmic regulator. + * - if the underlying layers are not ready, this is + * expected to return -EPROBE_DEFER + * - if failure, appropriate error code is expected. + * - This is expected to be a managed device to avoid + * explicit cleanup operations + * - Once this succeeds, this returns the pointer to + * the controller device and all other operations are + * expected to be ready for functionality. + * @voltage_set: set the voltage - expected to be synchronous. + * @voltage_get: get the current voltage in micro-volts set on PMIC. + * @voltage_get_range: Get minimum and maxium permissible operational voltage + * range for the device - used to set initial regulator + * constraints. + * + * These voltage processor interfaces are registered by voltage processor driver + * using omap_pmic_register_controller_ops. This allows the omap_pmic driver to + * operate with a specific voltage processor driver. + */ +struct omap_pmic_controller_ops { + struct device *(*devm_pmic_register) (struct device *dev, + struct omap_pmic *pmic); + int (*voltage_set) (struct device *control_dev, u32 uv); + int (*voltage_get) (struct device *control_dev, u32 *uv); + int (*voltage_get_range) (struct device *control_dev, u32 *min_uv, + u32 *max_uv); +}; + +/** + * struct omap_pmic_info - PMIC information + * + * @slave_addr: 7 bit address representing I2C slave address. + * @voltage_reg_addr: I2C register address for setting voltage + * @cmd_reg_addr: I2C register address for low power transition commands + * @i2c_timeout_us: worst case latency for I2C operations for the device + * @slew_rate_uV: Slew rate in uV/uSeconds for voltage transitions + * @step_size_uV: Step size in uV for one vsel increment. + * @min_uV: represents the minimum step_sized incremental voltage + * @max_uV: represents the maximum step_sized incremental voltage + * @voltage_selector_setbits: what bits to set permenantly for the PMIC + * voltage selector - this may have PMIC specific meaning. + * @voltage_selector_mask: what mask to use for the vsel value - this is useful + * for PMICs where the vsel has to be applied at an offset. + * @voltage_selector_offset: what offset to apply to conversion routine when + * operating on vsel. + * @voltage_selector_zero: Special case handling if 0 value does NOT indicates + * power-off for PMIC. + * @setup_command_list: array of setup commands for PMIC to operate + * @setup_num_commands: number of setup commands for PMIC to operate + */ +struct omap_pmic_info { + u8 slave_addr; + u8 voltage_reg_addr; + u8 cmd_reg_addr; + u32 i2c_timeout_us; + + u32 slew_rate_uV; + u32 step_size_uV; + u32 min_uV; + u32 max_uV; + + u8 voltage_selector_offset; + u8 voltage_selector_mask; + u8 voltage_selector_setbits; + bool voltage_selector_zero; + + const struct omap_pmic_setup_commands *setup_command_list; + u8 setup_num_commands; +}; + +/** + * struct omap_pmic - represents the OMAP PMIC device. + * @ops: PMIC conversion operations. + * + * NOTE: the fields marked Private are meant for PMIC driver. + */ +struct omap_pmic { + const struct omap_pmic_info *info; + u32 boot_voltage_uV; + + struct omap_pmic_ops *ops; + + struct device *dev; + struct device *v_dev; +}; + +#if IS_ENABLED(CONFIG_REGULATOR_TI_OMAP_PMIC) +int omap_pmic_register_controller_ops(struct omap_pmic_controller_ops *cops); +#else +static inline int omap_pmic_register_controller_ops(struct + omap_pmic_controller_ops + *cops) +{ + return -EINVAL; +} +#endif + +#endif /* __POWER_OMAP_PMIC_H */