You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nuttx.apache.org by TimH <ti...@jti.uk.com.INVALID> on 2022/09/06 11:24:56 UTC
Driver for combined battery charger and regulator
I'm writing a driver for the Quorvo ACT8945A device. This is a combined
7-output programmable regulator AND Li-ion battery charger, all in one.
I see there are existing "regulator.h" and "battery_charger.h" driver
templates.
Would my best approach to be to create a driver using a new (e.g.)
"battery_charger_regulator.h" template combining that existing functionality
or is there another, preferred, approach?
Would the preferred device name be "/dev/bat0"? Or "/dev/act8945a"? Or
something else?
Then, once that is decided, I seem to have a mental block when it comes to
device initialisation vs registering.
In my mind, registering should just create the /dev/xxxx entry and
initialise the relevant structs, but not touch the device itself? But some
drivers do go on to actually set up the device, whereas I think that should
be a separate "init" function called independently of the "register"
function, or a specific ioctl perhaps? What is the best practice here?
Thanks,
Tim.
Re: Driver for combined battery charger and regulator
Posted by Xiang Xiao <xi...@gmail.com>.
Battery driver is a special one because the framework doesn't define any
ioctl to enable/disable the device, which means you have to enable it in
register.
On Tue, Sep 6, 2022 at 10:18 PM TimH <ti...@jti.uk.com.invalid> wrote:
> Thanks!
>
> I think I will take the following approach:
>
> - Use Kconfig to determine the default setup (which regulators are
> enabled, and their voltage). If another user wants to do this manually it
> can be achieved by disabling them in Kconfig and using ioctl to set them
> manually
> - have first initialisation of the device via the device register function
> - since the ACT8945A has no chip ID register to read, I can readback
> another register and check it matches what it should be (based on Kconfig
> and initial setup) and abort the device registration if there's a problem
> (most likely caused by the device not being there or otherwise failed).
>
> Think that should do it.
>
> Thank you as ever, Alan, for taking the time to help me.
>
> FYI I am awaiting 11.0 release and will then rebase to that, and submit my
> first PR's for my new drivers π
>
> >-----Original Message-----
> >From: Alan Carvalho de Assis <ac...@gmail.com>
> >Sent: 06 September 2022 15:08
> >To: dev@nuttx.apache.org
> >Subject: Re: Driver for combined battery charger and regulator
> >
> >Hi Tim,
> >
> >I think you can implement a register and a initialization as separated
> functions,
> >if you search you will find some drivers implemented that way.
> >
> >I remember a developer that complained about the fact the some sensors are
> >initialized automatically during the register phase and it was bad for
> him
> >because he was developing a low power device, so he needs to go through
> >ioctl to fix that issue.
> >
> >Currently I think the approach is make it works from scratch, but of
> course it
> >could be an issue for some usage.
> >
> >This is something we need to revisit in the future.
> >
> >Other thing is an issue is that some drivers don't have a probe (some
> kind of
> >prove of existence), it just assumes the device is there in the SPI/I2C
> bus and
> >don't try to talk with it and goes on creating the device file. It makes
> the file of
> >user hard because he see the device at /dev and think that everything is
> fine.
> >
> >BR,
> >
> >Alan
> >
> >On 9/6/22, TimH <ti...@jti.uk.com.invalid> wrote:
> >> Oh - OK! Thanks Alan. Makes my life easier (for now) as I'm not using
> >> the battery charger element on this board iteration so I can leave it
> >> for later.
> >>
> >> Any thoughts on my mental tussles regarding registering vs.
> initialising?
> >>
> >>>-----Original Message-----
> >>>From: Alan Carvalho de Assis <ac...@gmail.com>
> >>>Sent: 06 September 2022 13:51
> >>>To: dev@nuttx.apache.org
> >>>Subject: Re: Driver for combined battery charger and regulator
> >>>
> >>>Hi Tim,
> >>>
> >>>AFAIK we don't have a PMIC with battery regulator in the mainline yet.
> >>>So you don't have a reference to base on it.
> >>>
> >>>You don't need to create a single file with all functions on it, you
> >>>can create separated file for each specific function. This is how
> >>>MC13892 is implemented on Linux (please don't look the source code, it
> >>>is GPL), this chip is a PMIC with regulator, battery charger and LEDs
> >>>control.
> >>>
> >>>I think for ACT8945A should be included a regulator at
> >>>drivers/power/supply/ and will implement the functions from
> >>><nuttx/power/regulator.h> and will register itself with
> >>>regulator_register().
> >>>
> >>>Also you will include the battery charger logic at
> >>>driver/power/battery/ as a separated act8945_batchr.c to avoid mixing
> >>>things and will export itself as /dev/bat0.
> >>>
> >>>But you will need to use spinlock when accessing the I2C bus inside
> >>>these drivers to avoid both drivers trying to use it while a transfer
> >>>is in progress.
> >>>
> >>>BR,
> >>>
> >>>Alan
> >>>
> >>>On 9/6/22, TimH <ti...@jti.uk.com.invalid> wrote:
> >>>> I'm writing a driver for the Quorvo ACT8945A device. This is a
> >>>> combined 7-output programmable regulator AND Li-ion battery charger,
> >>>> all
> >>>in one.
> >>>>
> >>>>
> >>>>
> >>>> I see there are existing "regulator.h" and "battery_charger.h"
> >>>> driver templates.
> >>>>
> >>>>
> >>>>
> >>>> Would my best approach to be to create a driver using a new (e.g.)
> >>>> "battery_charger_regulator.h" template combining that existing
> >>>> functionality or is there another, preferred, approach?
> >>>>
> >>>>
> >>>>
> >>>> Would the preferred device name be "/dev/bat0"? Or "/dev/act8945a"?
> >>>> Or something else?
> >>>>
> >>>>
> >>>>
> >>>> Then, once that is decided, I seem to have a mental block when it
> >>>> comes to device initialisation vs registering.
> >>>>
> >>>>
> >>>>
> >>>> In my mind, registering should just create the /dev/xxxx entry and
> >>>> initialise the relevant structs, but not touch the device itself?
> >>>> But some drivers do go on to actually set up the device, whereas I
> >>>> think that should be a separate "init" function called independently
> >>>> of the
> >>>"register"
> >>>> function, or a specific ioctl perhaps? What is the best practice here?
> >>>>
> >>>>
> >>>>
> >>>> Thanks,
> >>>>
> >>>>
> >>>>
> >>>> Tim.
> >>>>
> >>>>
> >>>>
> >>>>
> >>
> >>
>
>
Re: Driver for combined battery charger and regulator
Posted by Alan Carvalho de Assis <ac...@gmail.com>.
Hi Tim,
You are welcome!
That is a good idea, defining the initial voltage for each channel on
Kconfig will make user life easier.
BR,
Alan
On 9/6/22, TimH <ti...@jti.uk.com.invalid> wrote:
> Thanks!
>
> I think I will take the following approach:
>
> - Use Kconfig to determine the default setup (which regulators are enabled,
> and their voltage). If another user wants to do this manually it can be
> achieved by disabling them in Kconfig and using ioctl to set them manually
> - have first initialisation of the device via the device register function
> - since the ACT8945A has no chip ID register to read, I can readback another
> register and check it matches what it should be (based on Kconfig and
> initial setup) and abort the device registration if there's a problem (most
> likely caused by the device not being there or otherwise failed).
>
> Think that should do it.
>
> Thank you as ever, Alan, for taking the time to help me.
>
> FYI I am awaiting 11.0 release and will then rebase to that, and submit my
> first PR's for my new drivers π
>
>>-----Original Message-----
>>From: Alan Carvalho de Assis <ac...@gmail.com>
>>Sent: 06 September 2022 15:08
>>To: dev@nuttx.apache.org
>>Subject: Re: Driver for combined battery charger and regulator
>>
>>Hi Tim,
>>
>>I think you can implement a register and a initialization as separated
>> functions,
>>if you search you will find some drivers implemented that way.
>>
>>I remember a developer that complained about the fact the some sensors are
>>initialized automatically during the register phase and it was bad for
>> him
>>because he was developing a low power device, so he needs to go through
>>ioctl to fix that issue.
>>
>>Currently I think the approach is make it works from scratch, but of course
>> it
>>could be an issue for some usage.
>>
>>This is something we need to revisit in the future.
>>
>>Other thing is an issue is that some drivers don't have a probe (some kind
>> of
>>prove of existence), it just assumes the device is there in the SPI/I2C bus
>> and
>>don't try to talk with it and goes on creating the device file. It makes
>> the file of
>>user hard because he see the device at /dev and think that everything is
>> fine.
>>
>>BR,
>>
>>Alan
>>
>>On 9/6/22, TimH <ti...@jti.uk.com.invalid> wrote:
>>> Oh - OK! Thanks Alan. Makes my life easier (for now) as I'm not using
>>> the battery charger element on this board iteration so I can leave it
>>> for later.
>>>
>>> Any thoughts on my mental tussles regarding registering vs.
>>> initialising?
>>>
>>>>-----Original Message-----
>>>>From: Alan Carvalho de Assis <ac...@gmail.com>
>>>>Sent: 06 September 2022 13:51
>>>>To: dev@nuttx.apache.org
>>>>Subject: Re: Driver for combined battery charger and regulator
>>>>
>>>>Hi Tim,
>>>>
>>>>AFAIK we don't have a PMIC with battery regulator in the mainline yet.
>>>>So you don't have a reference to base on it.
>>>>
>>>>You don't need to create a single file with all functions on it, you
>>>>can create separated file for each specific function. This is how
>>>>MC13892 is implemented on Linux (please don't look the source code, it
>>>>is GPL), this chip is a PMIC with regulator, battery charger and LEDs
>>>>control.
>>>>
>>>>I think for ACT8945A should be included a regulator at
>>>>drivers/power/supply/ and will implement the functions from
>>>><nuttx/power/regulator.h> and will register itself with
>>>>regulator_register().
>>>>
>>>>Also you will include the battery charger logic at
>>>>driver/power/battery/ as a separated act8945_batchr.c to avoid mixing
>>>>things and will export itself as /dev/bat0.
>>>>
>>>>But you will need to use spinlock when accessing the I2C bus inside
>>>>these drivers to avoid both drivers trying to use it while a transfer
>>>>is in progress.
>>>>
>>>>BR,
>>>>
>>>>Alan
>>>>
>>>>On 9/6/22, TimH <ti...@jti.uk.com.invalid> wrote:
>>>>> I'm writing a driver for the Quorvo ACT8945A device. This is a
>>>>> combined 7-output programmable regulator AND Li-ion battery charger,
>>>>> all
>>>>in one.
>>>>>
>>>>>
>>>>>
>>>>> I see there are existing "regulator.h" and "battery_charger.h"
>>>>> driver templates.
>>>>>
>>>>>
>>>>>
>>>>> Would my best approach to be to create a driver using a new (e.g.)
>>>>> "battery_charger_regulator.h" template combining that existing
>>>>> functionality or is there another, preferred, approach?
>>>>>
>>>>>
>>>>>
>>>>> Would the preferred device name be "/dev/bat0"? Or "/dev/act8945a"?
>>>>> Or something else?
>>>>>
>>>>>
>>>>>
>>>>> Then, once that is decided, I seem to have a mental block when it
>>>>> comes to device initialisation vs registering.
>>>>>
>>>>>
>>>>>
>>>>> In my mind, registering should just create the /dev/xxxx entry and
>>>>> initialise the relevant structs, but not touch the device itself?
>>>>> But some drivers do go on to actually set up the device, whereas I
>>>>> think that should be a separate "init" function called independently
>>>>> of the
>>>>"register"
>>>>> function, or a specific ioctl perhaps? What is the best practice here?
>>>>>
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>>
>>>>>
>>>>> Tim.
>>>>>
>>>>>
>>>>>
>>>>>
>>>
>>>
>
>
RE: Driver for combined battery charger and regulator
Posted by TimH <ti...@jti.uk.com.INVALID>.
Thanks!
I think I will take the following approach:
- Use Kconfig to determine the default setup (which regulators are enabled, and their voltage). If another user wants to do this manually it can be achieved by disabling them in Kconfig and using ioctl to set them manually
- have first initialisation of the device via the device register function
- since the ACT8945A has no chip ID register to read, I can readback another register and check it matches what it should be (based on Kconfig and initial setup) and abort the device registration if there's a problem (most likely caused by the device not being there or otherwise failed).
Think that should do it.
Thank you as ever, Alan, for taking the time to help me.
FYI I am awaiting 11.0 release and will then rebase to that, and submit my first PR's for my new drivers π
>-----Original Message-----
>From: Alan Carvalho de Assis <ac...@gmail.com>
>Sent: 06 September 2022 15:08
>To: dev@nuttx.apache.org
>Subject: Re: Driver for combined battery charger and regulator
>
>Hi Tim,
>
>I think you can implement a register and a initialization as separated functions,
>if you search you will find some drivers implemented that way.
>
>I remember a developer that complained about the fact the some sensors are
>initialized automatically during the register phase and it was bad for him
>because he was developing a low power device, so he needs to go through
>ioctl to fix that issue.
>
>Currently I think the approach is make it works from scratch, but of course it
>could be an issue for some usage.
>
>This is something we need to revisit in the future.
>
>Other thing is an issue is that some drivers don't have a probe (some kind of
>prove of existence), it just assumes the device is there in the SPI/I2C bus and
>don't try to talk with it and goes on creating the device file. It makes the file of
>user hard because he see the device at /dev and think that everything is fine.
>
>BR,
>
>Alan
>
>On 9/6/22, TimH <ti...@jti.uk.com.invalid> wrote:
>> Oh - OK! Thanks Alan. Makes my life easier (for now) as I'm not using
>> the battery charger element on this board iteration so I can leave it
>> for later.
>>
>> Any thoughts on my mental tussles regarding registering vs. initialising?
>>
>>>-----Original Message-----
>>>From: Alan Carvalho de Assis <ac...@gmail.com>
>>>Sent: 06 September 2022 13:51
>>>To: dev@nuttx.apache.org
>>>Subject: Re: Driver for combined battery charger and regulator
>>>
>>>Hi Tim,
>>>
>>>AFAIK we don't have a PMIC with battery regulator in the mainline yet.
>>>So you don't have a reference to base on it.
>>>
>>>You don't need to create a single file with all functions on it, you
>>>can create separated file for each specific function. This is how
>>>MC13892 is implemented on Linux (please don't look the source code, it
>>>is GPL), this chip is a PMIC with regulator, battery charger and LEDs
>>>control.
>>>
>>>I think for ACT8945A should be included a regulator at
>>>drivers/power/supply/ and will implement the functions from
>>><nuttx/power/regulator.h> and will register itself with
>>>regulator_register().
>>>
>>>Also you will include the battery charger logic at
>>>driver/power/battery/ as a separated act8945_batchr.c to avoid mixing
>>>things and will export itself as /dev/bat0.
>>>
>>>But you will need to use spinlock when accessing the I2C bus inside
>>>these drivers to avoid both drivers trying to use it while a transfer
>>>is in progress.
>>>
>>>BR,
>>>
>>>Alan
>>>
>>>On 9/6/22, TimH <ti...@jti.uk.com.invalid> wrote:
>>>> I'm writing a driver for the Quorvo ACT8945A device. This is a
>>>> combined 7-output programmable regulator AND Li-ion battery charger,
>>>> all
>>>in one.
>>>>
>>>>
>>>>
>>>> I see there are existing "regulator.h" and "battery_charger.h"
>>>> driver templates.
>>>>
>>>>
>>>>
>>>> Would my best approach to be to create a driver using a new (e.g.)
>>>> "battery_charger_regulator.h" template combining that existing
>>>> functionality or is there another, preferred, approach?
>>>>
>>>>
>>>>
>>>> Would the preferred device name be "/dev/bat0"? Or "/dev/act8945a"?
>>>> Or something else?
>>>>
>>>>
>>>>
>>>> Then, once that is decided, I seem to have a mental block when it
>>>> comes to device initialisation vs registering.
>>>>
>>>>
>>>>
>>>> In my mind, registering should just create the /dev/xxxx entry and
>>>> initialise the relevant structs, but not touch the device itself?
>>>> But some drivers do go on to actually set up the device, whereas I
>>>> think that should be a separate "init" function called independently
>>>> of the
>>>"register"
>>>> function, or a specific ioctl perhaps? What is the best practice here?
>>>>
>>>>
>>>>
>>>> Thanks,
>>>>
>>>>
>>>>
>>>> Tim.
>>>>
>>>>
>>>>
>>>>
>>
>>
Re: Driver for combined battery charger and regulator
Posted by Alan Carvalho de Assis <ac...@gmail.com>.
Hi Tim,
I think you can implement a register and a initialization as separated
functions, if you search you will find some drivers implemented that
way.
I remember a developer that complained about the fact the some sensors
are initialized automatically during the register phase and it was
bad for him because he was developing a low power device, so he needs
to go through ioctl to fix that issue.
Currently I think the approach is make it works from scratch, but of
course it could be an issue for some usage.
This is something we need to revisit in the future.
Other thing is an issue is that some drivers don't have a probe (some
kind of prove of existence), it just assumes the device is there in
the SPI/I2C bus and don't try to talk with it and goes on creating the
device file. It makes the file of user hard because he see the device
at /dev and think that everything is fine.
BR,
Alan
On 9/6/22, TimH <ti...@jti.uk.com.invalid> wrote:
> Oh - OK! Thanks Alan. Makes my life easier (for now) as I'm not using the
> battery charger element on this board iteration so I can leave it for
> later.
>
> Any thoughts on my mental tussles regarding registering vs. initialising?
>
>>-----Original Message-----
>>From: Alan Carvalho de Assis <ac...@gmail.com>
>>Sent: 06 September 2022 13:51
>>To: dev@nuttx.apache.org
>>Subject: Re: Driver for combined battery charger and regulator
>>
>>Hi Tim,
>>
>>AFAIK we don't have a PMIC with battery regulator in the mainline yet.
>>So you don't have a reference to base on it.
>>
>>You don't need to create a single file with all functions on it, you can
>> create
>>separated file for each specific function. This is how
>>MC13892 is implemented on Linux (please don't look the source code, it is
>>GPL), this chip is a PMIC with regulator, battery charger and LEDs
>> control.
>>
>>I think for ACT8945A should be included a regulator at
>> drivers/power/supply/
>>and will implement the functions from <nuttx/power/regulator.h> and will
>>register itself with regulator_register().
>>
>>Also you will include the battery charger logic at driver/power/battery/ as
>> a
>>separated act8945_batchr.c to avoid mixing things and will export itself
>> as
>>/dev/bat0.
>>
>>But you will need to use spinlock when accessing the I2C bus inside these
>>drivers to avoid both drivers trying to use it while a transfer is in
>> progress.
>>
>>BR,
>>
>>Alan
>>
>>On 9/6/22, TimH <ti...@jti.uk.com.invalid> wrote:
>>> I'm writing a driver for the Quorvo ACT8945A device. This is a
>>> combined 7-output programmable regulator AND Li-ion battery charger, all
>>in one.
>>>
>>>
>>>
>>> I see there are existing "regulator.h" and "battery_charger.h" driver
>>> templates.
>>>
>>>
>>>
>>> Would my best approach to be to create a driver using a new (e.g.)
>>> "battery_charger_regulator.h" template combining that existing
>>> functionality or is there another, preferred, approach?
>>>
>>>
>>>
>>> Would the preferred device name be "/dev/bat0"? Or "/dev/act8945a"? Or
>>> something else?
>>>
>>>
>>>
>>> Then, once that is decided, I seem to have a mental block when it
>>> comes to device initialisation vs registering.
>>>
>>>
>>>
>>> In my mind, registering should just create the /dev/xxxx entry and
>>> initialise the relevant structs, but not touch the device itself? But
>>> some drivers do go on to actually set up the device, whereas I think
>>> that should be a separate "init" function called independently of the
>>"register"
>>> function, or a specific ioctl perhaps? What is the best practice here?
>>>
>>>
>>>
>>> Thanks,
>>>
>>>
>>>
>>> Tim.
>>>
>>>
>>>
>>>
>
>
RE: Driver for combined battery charger and regulator
Posted by TimH <ti...@jti.uk.com.INVALID>.
Oh - OK! Thanks Alan. Makes my life easier (for now) as I'm not using the battery charger element on this board iteration so I can leave it for later.
Any thoughts on my mental tussles regarding registering vs. initialising?
>-----Original Message-----
>From: Alan Carvalho de Assis <ac...@gmail.com>
>Sent: 06 September 2022 13:51
>To: dev@nuttx.apache.org
>Subject: Re: Driver for combined battery charger and regulator
>
>Hi Tim,
>
>AFAIK we don't have a PMIC with battery regulator in the mainline yet.
>So you don't have a reference to base on it.
>
>You don't need to create a single file with all functions on it, you can create
>separated file for each specific function. This is how
>MC13892 is implemented on Linux (please don't look the source code, it is
>GPL), this chip is a PMIC with regulator, battery charger and LEDs control.
>
>I think for ACT8945A should be included a regulator at drivers/power/supply/
>and will implement the functions from <nuttx/power/regulator.h> and will
>register itself with regulator_register().
>
>Also you will include the battery charger logic at driver/power/battery/ as a
>separated act8945_batchr.c to avoid mixing things and will export itself as
>/dev/bat0.
>
>But you will need to use spinlock when accessing the I2C bus inside these
>drivers to avoid both drivers trying to use it while a transfer is in progress.
>
>BR,
>
>Alan
>
>On 9/6/22, TimH <ti...@jti.uk.com.invalid> wrote:
>> I'm writing a driver for the Quorvo ACT8945A device. This is a
>> combined 7-output programmable regulator AND Li-ion battery charger, all
>in one.
>>
>>
>>
>> I see there are existing "regulator.h" and "battery_charger.h" driver
>> templates.
>>
>>
>>
>> Would my best approach to be to create a driver using a new (e.g.)
>> "battery_charger_regulator.h" template combining that existing
>> functionality or is there another, preferred, approach?
>>
>>
>>
>> Would the preferred device name be "/dev/bat0"? Or "/dev/act8945a"? Or
>> something else?
>>
>>
>>
>> Then, once that is decided, I seem to have a mental block when it
>> comes to device initialisation vs registering.
>>
>>
>>
>> In my mind, registering should just create the /dev/xxxx entry and
>> initialise the relevant structs, but not touch the device itself? But
>> some drivers do go on to actually set up the device, whereas I think
>> that should be a separate "init" function called independently of the
>"register"
>> function, or a specific ioctl perhaps? What is the best practice here?
>>
>>
>>
>> Thanks,
>>
>>
>>
>> Tim.
>>
>>
>>
>>
Re: Driver for combined battery charger and regulator
Posted by Xiang Xiao <xi...@gmail.com>.
On Wed, Sep 7, 2022 at 9:20 PM TimH <ti...@jti.uk.com.invalid> wrote:
> >From: Xiang Xiao <xi...@gmail.com>
> >Sent: 07 September 2022 13:40
> >
> >On Wed, Sep 7, 2022 at 8:13 PM TimH <ti...@jti.uk.com.invalid> wrote:
> >
> >> >From: Alan Carvalho de Assis <ac...@gmail.com>
> >> >Sent: 06 September 2022 13:51
> >> >I think for ACT8945A should be included a regulator at
> >> drivers/power/supply/
> >> >and will implement the functions from <nuttx/power/regulator.h> and
> >> >will register itself with regulator_register().
> >>
> >> Sorry for the dumb question, but do I use regulator.c directly, or is
> >> it just a template for my own act8945a.c driver? I can find no other
> >> files that include regulator.h!
> >>
> >>
> >You need implement regulator_ops_s and call regulator_register, other
> >drivers which want to enable the power need call regulator_get and
> >regulator_enable
> >
> >
> >> And if this is very clear from some documentation somewhere, I
> apologise!
> >>
> >>
> >NuttX regulator framework is similar Linux kernel, so you can reference
> >this:
> >https://docs.kernel.org/power/regulator/overview.html
>
> From looking at the Linux documentation then back at Nuttx register.c it
> leads me to think I have to register 6 different devices for this 6-output
> PMIC. Is that right?
>
>
Yes.
RE: Driver for combined battery charger and regulator
Posted by TimH <ti...@jti.uk.com.INVALID>.
>>>> Please correct me if I'm still missing something, as there are zero
>> clues I can find as to what each ops function (for example) is
>> actually supposed to do.
>>
>>
>>
>Here is two examples:
>https://github.com/apache/incubator-
>nuttx/blob/master/drivers/power/supply/regulator_gpio.c
>https://github.com/apache/incubator-
>nuttx/blob/master/drivers/power/supply/regulator_rpmsg.c
10.3 release that I'm using has no files with reference to (for example) get_voltage_sel. It is impractical to keep a development project up to date with master so I only merge/rebase when there's a formal release.
But thank you for these, I will take a look and hopefully they will point me in the right direction.
Re: Driver for combined battery charger and regulator
Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Sep 7, 2022 at 1:34 PM Tim Hardisty <ti...@jti.uk.com.invalid> wrote:
> On Wed, Sep 7, 2022 at 1:03 PM Tim Hardisty <ti...@jti.uk.com.invalid>
> wrote:
> >>
> >>
> >> >>> Please correct me if I'm still missing something, as there
> are zero
> >> >>> clues I can find as to what each ops function (for example)
> is
> >> >>> actually supposed to do.
> >> >>
> >> >>
> >> >>
> >> >>Here is two examples:
> >> >>https://github.com/apache/incubator-
> >> >>nuttx/blob/master/drivers/power/supply/regulator_gpio.c
> >> >>https://github.com/apache/incubator-
> >> >>nuttx/blob/master/drivers/power/supply/regulator_rpmsg.c
> >>
> >> >thank you for these, I will take a look and hopefully they
> will point me
> >> >in the right direction.
> >>
> > Please: what are the purpose of:
> >>
> >> set_voltage_sel
> >> get_voltage_sel
>
>
> >This really should be documented in the definition of struct
> >regulator_ops_s in include/nuttx/power/regulator.h, particularly what
> >is the difference between set_voltage and set_voltage_sel.
>
> I agree - this should be there as comments. I will eventually submit a PR
> for my driver and, if it's not done by then, I will add these in the same
> PR
Thank you! That will be much appreciated!
>That said, based on reading the code at _regulator_do_set_voltage() in
> >drivers/power/supply/regulator.c, it appears that:
>
> >1) set_voltage() is given a minimum and maximum desired voltage in
> >microvolts and sets (if possible) the regulator's voltage selector to
> >generate something between those bounds. It is up to the lower-half
> >driver to look at the desired min and max voltage and figure out what
> >the selector should be, whereas:
>
> >2) set_voltage_sel() is given the selector value itself and sets that.
> >The lower-half driver doesn't make decisions in this case; the
> >upper-half driver iterates through all available voltages to decide
> >which one is the "best" and then calls the lower-half driver with that
> >selector value.
>
> >The logic in _regulator_do_set_voltage() is such that if the
> >lower-half driver provides set_voltage(), it prefers to call that; if
> >not, then I assume it's a more "primitive" driver and therefore the
> >upper-half decides the selector value.
>
> This is a great help, Nathan: thank you. The ACT8945A only allows discrete
> voltages to be set (64 values) so the set_voltage_sel may one way to sort
> this, although I already have code to select the right register value by
> some mathematical shenigans so could put that in the lower half and make
> use of the min and max values.
I suppose you could do both, though I don't know whether that would just be
a waste of code space. Perhaps internally one would call the other.
It is all making sense now: lack of comments, and no examples in 10.3, are
> what done me in.
Glad to be of help! Comments--or rather GOOD comments--are invaluable!
Cheers
Nathan
Re: Driver for combined battery charger and regulator
Posted by Tim Hardisty <ti...@jti.uk.com.INVALID>.
On Wed, Sep 7, 2022 at 1:03 PM Tim Hardisty <ti...@jti.uk.com.invalid> wrote:
>>
>>
>> >>> Please correct me if I'm still missing something, as there are zero
>> >>> clues I can find as to what each ops function (for example) is
>> >>> actually supposed to do.
>> >>
>> >>
>> >>
>> >>Here is two examples:
>> >>https://github.com/apache/incubator-
>> >>nuttx/blob/master/drivers/power/supply/regulator_gpio.c
>> >>https://github.com/apache/incubator-
>> >>nuttx/blob/master/drivers/power/supply/regulator_rpmsg.c
>>
>> >thank you for these, I will take a look and hopefully they will point me
>> >in the right direction.
>>
> Please: what are the purpose of:
>>
>> set_voltage_sel
>> get_voltage_sel
>This really should be documented in the definition of struct
>regulator_ops_s in include/nuttx/power/regulator.h, particularly what
>is the difference between set_voltage and set_voltage_sel.
I agree - this should be there as comments. I will eventually submit a PR for my driver and, if it's not done by then, I will add these in the same PR
>That said, based on reading the code at _regulator_do_set_voltage() in
>drivers/power/supply/regulator.c, it appears that:
>1) set_voltage() is given a minimum and maximum desired voltage in
>microvolts and sets (if possible) the regulator's voltage selector to
>generate something between those bounds. It is up to the lower-half
>driver to look at the desired min and max voltage and figure out what
>the selector should be, whereas:
>2) set_voltage_sel() is given the selector value itself and sets that.
>The lower-half driver doesn't make decisions in this case; the
>upper-half driver iterates through all available voltages to decide
>which one is the "best" and then calls the lower-half driver with that
>selector value.
>The logic in _regulator_do_set_voltage() is such that if the
>lower-half driver provides set_voltage(), it prefers to call that; if
>not, then I assume it's a more "primitive" driver and therefore the
>upper-half decides the selector value.
This is a great help, Nathan: thank you. The ACT8945A only allows discrete voltages to be set (64 values) so the set_voltage_sel may one way to sort this, although I already have code to select the right register value by some mathematical shenigans so could put that in the lower half and make use of the min and max values.
It is all making sense now: lack of comments, and no examples in 10.3, are what done me in.
Re: Driver for combined battery charger and regulator
Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Sep 7, 2022 at 1:03 PM Tim Hardisty <ti...@jti.uk.com.invalid> wrote:
>
>
> >>> Please correct me if I'm still missing something, as there are zero
> >>> clues I can find as to what each ops function (for example) is
> >>> actually supposed to do.
> >>
> >>
> >>
> >>Here is two examples:
> >>https://github.com/apache/incubator-
> >>nuttx/blob/master/drivers/power/supply/regulator_gpio.c
> >>https://github.com/apache/incubator-
> >>nuttx/blob/master/drivers/power/supply/regulator_rpmsg.c
>
> >thank you for these, I will take a look and hopefully they will point me
> >in the right direction.
>
> Please: what are the purpose of:
>
> set_voltage_sel
> get_voltage_sel
This really should be documented in the definition of struct
regulator_ops_s in include/nuttx/power/regulator.h, particularly what
is the difference between set_voltage and set_voltage_sel.
That said, based on reading the code at _regulator_do_set_voltage() in
drivers/power/supply/regulator.c, it appears that:
1) set_voltage() is given a minimum and maximum desired voltage in
microvolts and sets (if possible) the regulator's voltage selector to
generate something between those bounds. It is up to the lower-half
driver to look at the desired min and max voltage and figure out what
the selector should be, whereas:
2) set_voltage_sel() is given the selector value itself and sets that.
The lower-half driver doesn't make decisions in this case; the
upper-half driver iterates through all available voltages to decide
which one is the "best" and then calls the lower-half driver with that
selector value.
The logic in _regulator_do_set_voltage() is such that if the
lower-half driver provides set_voltage(), it prefers to call that; if
not, then I assume it's a more "primitive" driver and therefore the
upper-half decides the selector value.
If any of this is incorrect, then hopefully someone will correct me.
I'm reading this code for the first time.
Cheers,
Nathan
Re: Driver for combined battery charger and regulator
Posted by Tim Hardisty <ti...@jti.uk.com.INVALID>.
>> Please: what are the purpose of:
>>
>> set_voltage_sel
>> get_voltage_sel
>>
>FYI: https://www.kernel.org/doc/html/v4.12/driver-api/regulator.html
Alan to the rescue again :-)
Perhaps Nuttx needs a warning to newcomers: "enter without Linux knowledge at your peril" LOL
Re: Driver for combined battery charger and regulator
Posted by Alan Carvalho de Assis <ac...@gmail.com>.
On 9/7/22, Tim Hardisty <ti...@jti.uk.com.invalid> wrote:
>
> >>> Please correct me if I'm still missing something, as there are zero
> >>> clues I can find as to what each ops function (for example) is
> >>> actually supposed to do.
> >>
> >>
> >>
> >>Here is two examples:
> >>https://github.com/apache/incubator-
> >>nuttx/blob/master/drivers/power/supply/regulator_gpio.c
> >>https://github.com/apache/incubator-
> >>nuttx/blob/master/drivers/power/supply/regulator_rpmsg.c
>
> >thank you for these, I will take a look and hopefully they will point
> me
> >in the right direction.
>
> Please: what are the purpose of:
>
> set_voltage_sel
> get_voltage_sel
>
FYI: https://www.kernel.org/doc/html/v4.12/driver-api/regulator.html
Re: Driver for combined battery charger and regulator
Posted by Tim Hardisty <ti...@jti.uk.com.INVALID>.
>>> Please correct me if I'm still missing something, as there are zero
>>> clues I can find as to what each ops function (for example) is
>>> actually supposed to do.
>>
>>
>>
>>Here is two examples:
>>https://github.com/apache/incubator-
>>nuttx/blob/master/drivers/power/supply/regulator_gpio.c
>>https://github.com/apache/incubator-
>>nuttx/blob/master/drivers/power/supply/regulator_rpmsg.c
>thank you for these, I will take a look and hopefully they will point me
>in the right direction.
Please: what are the purpose of:
set_voltage_sel
get_voltage_sel
Re: Driver for combined battery charger and regulator
Posted by Xiang Xiao <xi...@gmail.com>.
On Wed, Sep 7, 2022 at 10:26 PM TimH <ti...@jti.uk.com.invalid> wrote:
> Looking further, I'm thinking that this driver template has never been
> used, is undocumented, and is therefore bordering on unusable.
>
> Please correct me if I'm still missing something, as there are zero clues
> I can find as to what each ops function (for example) is actually supposed
> to do.
>
>
>
Here is two examples:
https://github.com/apache/incubator-nuttx/blob/master/drivers/power/supply/regulator_gpio.c
https://github.com/apache/incubator-nuttx/blob/master/drivers/power/supply/regulator_rpmsg.c
RE: Driver for combined battery charger and regulator
Posted by TimH <ti...@jti.uk.com.INVALID>.
Looking further, I'm thinking that this driver template has never been used, is undocumented, and is therefore bordering on unusable.
Please correct me if I'm still missing something, as there are zero clues I can find as to what each ops function (for example) is actually supposed to do.
RE: Driver for combined battery charger and regulator
Posted by TimH <ti...@jti.uk.com.INVALID>.
>From: Xiang Xiao <xi...@gmail.com>
>Sent: 07 September 2022 13:40
>
>On Wed, Sep 7, 2022 at 8:13 PM TimH <ti...@jti.uk.com.invalid> wrote:
>
>> >From: Alan Carvalho de Assis <ac...@gmail.com>
>> >Sent: 06 September 2022 13:51
>> >I think for ACT8945A should be included a regulator at
>> drivers/power/supply/
>> >and will implement the functions from <nuttx/power/regulator.h> and
>> >will register itself with regulator_register().
>>
>> Sorry for the dumb question, but do I use regulator.c directly, or is
>> it just a template for my own act8945a.c driver? I can find no other
>> files that include regulator.h!
>>
>>
>You need implement regulator_ops_s and call regulator_register, other
>drivers which want to enable the power need call regulator_get and
>regulator_enable
>
>
>> And if this is very clear from some documentation somewhere, I apologise!
>>
>>
>NuttX regulator framework is similar Linux kernel, so you can reference
>this:
>https://docs.kernel.org/power/regulator/overview.html
From looking at the Linux documentation then back at Nuttx register.c it leads me to think I have to register 6 different devices for this 6-output PMIC. Is that right?
Re: Driver for combined battery charger and regulator
Posted by Tim Hardisty <ti...@jti.uk.com.INVALID>.
ο»ΏOn 08/09/2022, 18:07, "Xiang Xiao" <xi...@gmail.com> wrote:
On Fri, Sep 9, 2022 at 12:19 AM TimH <ti...@jti.uk.com.invalid> wrote:
>> unsigned int apply_uv; /* ?? SEE BELOW [1] */
>Yes, bool is more suitable than int for apply_uv.
Thanks for confirming/agreeing - once 11.0 is released and I rebase my own work I'll do a PR to change this (and add comments to the driver files).
>>[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.
OK, makes sense, thanks.
> If this is all (nearly) correct, then I think the call to register a
> regulator
<snip>
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.
<snip>
> 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.
The two I suggested are in the Linux spec and are common things for processor PMICs. These would be good candidates to add to the common code I think.
>> Final observation - Linux documentation quite clearly states microvolts
>> for all voltage parameters.
<snip>
>
>
> 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.
OK I will use uv. Think it might mean the AXP202 driver needs fixing...it is using battery charger framework for regulator functions so someone probably needs to revisit it at some point.
Re: Driver for combined battery charger and regulator
Posted by Xiang Xiao <xi...@gmail.com>.
On Fri, Sep 9, 2022 at 12:19 AM TimH <ti...@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.
RE: Driver for combined battery charger and regulator
Posted by TimH <ti...@jti.uk.com.INVALID>.
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.
[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?
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?
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.
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?
Re: Driver for combined battery charger and regulator
Posted by Xiang Xiao <xi...@gmail.com>.
On Wed, Sep 7, 2022 at 8:13 PM TimH <ti...@jti.uk.com.invalid> wrote:
> >From: Alan Carvalho de Assis <ac...@gmail.com>
> >Sent: 06 September 2022 13:51
> >I think for ACT8945A should be included a regulator at
> drivers/power/supply/
> >and will implement the functions from <nuttx/power/regulator.h> and will
> >register itself with regulator_register().
>
> Sorry for the dumb question, but do I use regulator.c directly, or is it
> just a template for my own act8945a.c driver? I can find no other files
> that include regulator.h!
>
>
You need implement regulator_ops_s and call regulator_register, other
drivers which want to enable the power need call regulator_get
and regulator_enable
> And if this is very clear from some documentation somewhere, I apologise!
>
>
NuttX regulator framework is similar Linux kernel, so you can reference
this:
https://docs.kernel.org/power/regulator/overview.html
RE: Driver for combined battery charger and regulator
Posted by TimH <ti...@jti.uk.com.INVALID>.
>From: Alan Carvalho de Assis <ac...@gmail.com>
>Sent: 06 September 2022 13:51
>I think for ACT8945A should be included a regulator at drivers/power/supply/
>and will implement the functions from <nuttx/power/regulator.h> and will
>register itself with regulator_register().
Sorry for the dumb question, but do I use regulator.c directly, or is it just a template for my own act8945a.c driver? I can find no other files that include regulator.h!
And if this is very clear from some documentation somewhere, I apologise!
Re: Driver for combined battery charger and regulator
Posted by Alan Carvalho de Assis <ac...@gmail.com>.
Hi Tim,
AFAIK we don't have a PMIC with battery regulator in the mainline yet.
So you don't have a reference to base on it.
You don't need to create a single file with all functions on it, you
can create separated file for each specific function. This is how
MC13892 is implemented on Linux (please don't look the source code, it
is GPL), this chip is a PMIC with regulator, battery charger and LEDs
control.
I think for ACT8945A should be included a regulator at
drivers/power/supply/ and will implement the functions from
<nuttx/power/regulator.h> and will register itself with
regulator_register().
Also you will include the battery charger logic at
driver/power/battery/ as a separated act8945_batchr.c to avoid mixing
things and will export itself as /dev/bat0.
But you will need to use spinlock when accessing the I2C bus inside
these drivers to avoid both drivers trying to use it while a transfer
is in progress.
BR,
Alan
On 9/6/22, TimH <ti...@jti.uk.com.invalid> wrote:
> I'm writing a driver for the Quorvo ACT8945A device. This is a combined
> 7-output programmable regulator AND Li-ion battery charger, all in one.
>
>
>
> I see there are existing "regulator.h" and "battery_charger.h" driver
> templates.
>
>
>
> Would my best approach to be to create a driver using a new (e.g.)
> "battery_charger_regulator.h" template combining that existing
> functionality
> or is there another, preferred, approach?
>
>
>
> Would the preferred device name be "/dev/bat0"? Or "/dev/act8945a"? Or
> something else?
>
>
>
> Then, once that is decided, I seem to have a mental block when it comes to
> device initialisation vs registering.
>
>
>
> In my mind, registering should just create the /dev/xxxx entry and
> initialise the relevant structs, but not touch the device itself? But some
> drivers do go on to actually set up the device, whereas I think that should
> be a separate "init" function called independently of the "register"
> function, or a specific ioctl perhaps? What is the best practice here?
>
>
>
> Thanks,
>
>
>
> Tim.
>
>
>
>