On Fri, Sep 9, 2022 at 12:19 AM TimH <t...@jti.uk.com.invalid> wrote:
> Apologies for the long post, but given the absence of many/any voltage
> regulator drivers - especially PMICs - I feel I need to get this all sorted
> and agreed. So I have looked long and hard at the Nuttx files and Linux
> documentation and to see if I have got it right I would like to see if the
> following descriptions of the Nuttx struct fields make sense.
>
> struct regulator_desc_s
> {
> const char *name; /* Friendly name that will be used to
> get/set/etc a given regulator. For example "VCC" */
> unsigned int n_voltages; /* The number of discrete voltage selector
> steps this regulator allows. 0 if linear mapping is to be used instead */
> unsigned int vsel_reg; /* The device-specific regulator
> "channel", or GPIO output, or similar, for this regulator */
> unsigned int vsel_mask; /* Mask, if relevant, to use with
> vsel_reg*/
> unsigned int enable_reg; /* Device specific enable register, if
> relevant, for this regulator */
> unsigned int enable_mask; /* Mask, if relevant, to use with
> enable_reg*/
> unsigned int enable_time; /* Time taken for the regulator voltage
> output voltage to stabilise after being enabled, in microseconds */
> unsigned int ramp_delay; /* Set the ramp delay for the regulator.
> The driver should select ramp delay equal to or less than(closest)
> ramp_delay */
> unsigned int uv_step; /* Voltage increase with each selector if
> linear mapping. 0 if only selector steps are allowed */
> unsigned int min_uv; /* Smallest voltage consumers may set*/
> unsigned int max_uv; /* Largest voltage consumers may set */
> unsigned int apply_uv; /* ?? SEE BELOW [1] */
> bool boot_on; /* Set if the regulator is enabled when
> the system is initially started SEE BELOW [2].
> If the regulator is not enabled by the
> hardware or bootloader then it will be enabled when the constraints are
> applied */
> };
>
> [1] "apply_uv" is a bool in Linux but an int in Nuttx. In Linux it tells
> you whether the "constraint" voltage is applied during init. I think it
> should be a bool in Nuttx too? With the ACT8945A, for example, REG1-5 power
> on by default, whereas REG6 and 7 do not. The REG1-5 will report back "0"
> and REG6/6 "1" in boot_on.
>
>
Yes, bool is more suitable than int for apply_uv.
> [2] It is not clear, either in Linux or Nuttx, what the "constraint
> voltage" that is applied during init will be. Perhaps the NuttX intent is -
> or should be - to use an actual value here (hence the int?) which will be
> applied during init IF "boot_on" is set on (suggesting it is a parameter
> that should be passed TO the regulator register/init function not filled in
> BY it?
>
>
boot_on in NuttX is simple, the common code will enable the regulator if
boot_on is true otherwise disable it in regulaotr_register.
> If this is all (nearly) correct, then I think the call to register a
> regulator (from board_bringup, say) needs to initialise the following
> variables in the regulator_desc_s struct that's passed:
>
> - name For example, in my design, REG1 is "VIODDR"
> - vsel_reg In my example it is "REG1" and I need to
> tell the ACT8945A driver this.
> - vsel_mask (if relevant). Not relevant in my case
> - enable_reg, if relevant. Not relevant in my case.
> - enable_mask, if relevant. Not relevant in my case.
>
> So we would have (for this i2c device, at least):
>
> FAR struct regulator_desc_s *act8945a_desc_s;
> FAR struct i2c_master_s *act89945a_i2c ;
> int ret;
>
> act89945a_i2c = board_i2c_init(busno);
> if (!act8945a_i2c)
> {
> Blah blah;
> }
>
> act8945a_desc_s->name = "VIODDR";
> act8945a_desc_s->vsel_reg = 1;
>
> ret = act8945a_device_init(act8945a_desc_s, act8945a_i2c);
> if (ret < 0)
> {
> Blah blah;
> }
>
> The init/register routine will fill in the missing blanks in the structure
> and we can go from there to set it up. When needed in the user app, we can
> do a "regulator get" to see what we've got and change things as needed.
>
> Does that sound right or am I way off the mark?
>
>
Yes, I think so.
> If I'm nearly right I might add that I believe there are useful members of
> the regulator_desc_s structure missing, such as "active_discharge" that's
> useful for the ACT8945A. Maybe also "always on" which tells the driver
> NEVER to turn this one off - rather important for PMIC devices I would say,
> although NuttX apps aren't quite so likely to mess around with regulators
> on these custom boards. I can add these to the struct, of course, as I
> write my driver.
>
>
The fields in regulator_desc_s are designed for the common regulator
framework code, so it's better to put a new field to this struct if the
common does the different action based on it. If the new info is only used
by a specific driver, regulator_dev_s::priv is a better place.
> Final observation - Linux documentation quite clearly states microvolts
> for all voltage parameters. That means a simple 3V3 regulator will be set
> up with the value 3300000 passed to the functions, so it has to use
> uint32_t. Maybe that makes sense for Linux, but perhaps for Nuttx it should
> be in millivolts? I see there's a driver for an AXP202 device and that is
> clearly assuming millivolts?
>
>
The initial design follows Linux convention, that's why the field ends with
_uv not _mv.. uint32_t can hold 4293.967295V with uV which is enough in
most case.
