Re: [PATCH 1/3] mfd: add Cypress FM33256B Processor Companion driver

[Alexandre Belloni]

On 26/04/2016 at 16:31:46 +0200, Jeppe Ledet-Pedersen wrote :
> > Good catch, I didn't look at this patch but it includes a lot of code
> > that should be going to the RTC driver.
> > If trickle charging is not enabled, I guess the RTC will not charge its
> > backup battery.
> 
> Thank you for the comments.
> 
> Alexandre is correct. If the "cypress,charge-enabled" property is
> present, the internal 80 uA trickle charger is enabled to charge a
> backup capacitor on the VBAK pin.
> 
> Do you want more code than the trickle charger setup moved to the RTC
> driver?
> 

I guess the 32k oscillator could go in the rtc driver too.

> >>> +- cypress,charge-fast: enable fast (1 mA) charging
> >>
> >> What does fast mean?
> >>
> 
> Fast charging means charging with 1 mA instead of 80 uA. The wording is
> from the datasheet, but I agree it should be renamed to something that
> better explains what the difference is.
> 
> >> I think it is time for a common binding here. There's all sorts of 
> >> variations on setting the charge current in bindings. Add something like 
> >> "charge-current-microamp" in power_supply.txt and use it here. Then 
> >> 1000uA implies "fast charge".
> > 
> > Well, this is not a power supply, it is an RTC.
> > 
> > I think both properties should got to the RTC subdevice and be parsed in
> > the RTC driver.
> 
> The backup capacitor is primarily used to supply current to the RTC, but
> a couple of other registers are also battery-backed (watchdog reset
> cause, event counter), so I put the charger setup in the main MFD file.
> 
> If you prefer, I can move them both to the RTC driver?

I would prefer unless you think that many people are not using the RTC
while still using the watchdog (I'd say that is not the case).

> 
> > Note that for trickle charging, we currently have the following
> > properties:
> >  - trickle-resistor-ohms
> >  - trickle-diode-disable
> >  - abracon,tc-diode
> >  - abracon,tc-resistor
> > 
> > abracon,tc-resistor can be replaced by trickle-resistor-ohms, I'll make
> > a patch but abracon,tc-diode allows to select a diode type (and in
> > particular a voltage drop).
> > 
> > I think we could add a trickle-current-microamp like you suggested and
> > trickle-diode-disable can be reused.
> 
> So replace "cypress,charge-enabled/charge-fast" with
> "trickle-current-microamp" and a check for 0/80/1000 uA?
> 

Yeah, seems fine.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/3] mfd: add Cypress FM33256B Processor Companion driver

[Alexandre Belloni]

On 26/04/2016 at 16:31:46 +0200, Jeppe Ledet-Pedersen wrote :
> > Good catch, I didn't look at this patch but it includes a lot of code
> > that should be going to the RTC driver.
> > If trickle charging is not enabled, I guess the RTC will not charge its
> > backup battery.
> 
> Thank you for the comments.
> 
> Alexandre is correct. If the "cypress,charge-enabled" property is
> present, the internal 80 uA trickle charger is enabled to charge a
> backup capacitor on the VBAK pin.
> 
> Do you want more code than the trickle charger setup moved to the RTC
> driver?
> 

I guess the 32k oscillator could go in the rtc driver too.

> >>> +- cypress,charge-fast: enable fast (1 mA) charging
> >>
> >> What does fast mean?
> >>
> 
> Fast charging means charging with 1 mA instead of 80 uA. The wording is
> from the datasheet, but I agree it should be renamed to something that
> better explains what the difference is.
> 
> >> I think it is time for a common binding here. There's all sorts of 
> >> variations on setting the charge current in bindings. Add something like 
> >> "charge-current-microamp" in power_supply.txt and use it here. Then 
> >> 1000uA implies "fast charge".
> > 
> > Well, this is not a power supply, it is an RTC.
> > 
> > I think both properties should got to the RTC subdevice and be parsed in
> > the RTC driver.
> 
> The backup capacitor is primarily used to supply current to the RTC, but
> a couple of other registers are also battery-backed (watchdog reset
> cause, event counter), so I put the charger setup in the main MFD file.
> 
> If you prefer, I can move them both to the RTC driver?

I would prefer unless you think that many people are not using the RTC
while still using the watchdog (I'd say that is not the case).

> 
> > Note that for trickle charging, we currently have the following
> > properties:
> >  - trickle-resistor-ohms
> >  - trickle-diode-disable
> >  - abracon,tc-diode
> >  - abracon,tc-resistor
> > 
> > abracon,tc-resistor can be replaced by trickle-resistor-ohms, I'll make
> > a patch but abracon,tc-diode allows to select a diode type (and in
> > particular a voltage drop).
> > 
> > I think we could add a trickle-current-microamp like you suggested and
> > trickle-diode-disable can be reused.
> 
> So replace "cypress,charge-enabled/charge-fast" with
> "trickle-current-microamp" and a check for 0/80/1000 uA?
> 

Yeah, seems fine.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/3] mfd: add Cypress FM33256B Processor Companion driver

[Jeppe Ledet-Pedersen]

On 22/04/16 21:32, Rob Herring wrote:
> On Wed, Apr 20, 2016 at 01:07:49PM +0200, Jeppe Ledet-Pedersen wrote:
[snip]
>> +
>> +The MFD exposes two subdevices:
>> +- The FRAM: "cypress,fm33256b-fram"
>> +- The RTC: "cypress,fm33256b-rtc"
>> +
>> +Example:
>> +
>> +spi1: spi@f800800 {
>> +status = "okay";
>> +cs-gpios = < 25 0>;
>> +
>> +fm33256b@0 {
>> +compatible = "cypress,fm33256b";
>> +spi-max-frequency = <1000>;
>> +cypress,charge-enabled;
>> +cypress,charge-fast;
>> +reg = <0>;
>> +};
> 
> Where's the 2nd sub device?

Hi Rob,

Right now I just add the two sub-devices using mfd_add_devices in the
fm33256b_probe function in my MFD driver. Would it be better to check
for compatible child nodes before adding each sub-device, like it's done
in drivers/mfd/tc3589x.c?

Thanks,

-Jeppe

Re: [PATCH 1/3] mfd: add Cypress FM33256B Processor Companion driver

[Jeppe Ledet-Pedersen]

On 22/04/16 21:32, Rob Herring wrote:
> On Wed, Apr 20, 2016 at 01:07:49PM +0200, Jeppe Ledet-Pedersen wrote:
[snip]
>> +
>> +The MFD exposes two subdevices:
>> +- The FRAM: "cypress,fm33256b-fram"
>> +- The RTC: "cypress,fm33256b-rtc"
>> +
>> +Example:
>> +
>> +spi1: spi@f800800 {
>> +status = "okay";
>> +cs-gpios = < 25 0>;
>> +
>> +fm33256b@0 {
>> +compatible = "cypress,fm33256b";
>> +spi-max-frequency = <1000>;
>> +cypress,charge-enabled;
>> +cypress,charge-fast;
>> +reg = <0>;
>> +};
> 
> Where's the 2nd sub device?

Hi Rob,

Right now I just add the two sub-devices using mfd_add_devices in the
fm33256b_probe function in my MFD driver. Would it be better to check
for compatible child nodes before adding each sub-device, like it's done
in drivers/mfd/tc3589x.c?

Thanks,

-Jeppe

Re: [PATCH 1/3] mfd: add Cypress FM33256B Processor Companion driver

[Jeppe Ledet-Pedersen]

On 22/04/16 22:11, Alexandre Belloni wrote:
> On 22/04/2016 at 14:32:32 -0500, Rob Herring wrote :
>> On Wed, Apr 20, 2016 at 01:07:49PM +0200, Jeppe Ledet-Pedersen wrote:
>>> This patch adds support for the Cypress Semiconductor FM33256B processor
>>> companion. The device contains a 256 kbit FRAM, an RTC, a supply voltage
>>> monitor, and a watchdog timer.
>>>
>>> Signed-off-by: Jeppe Ledet-Pedersen 
>>> ---
>>>  Documentation/devicetree/bindings/mfd/fm33256b.txt |  30 ++
>>>  MAINTAINERS|   6 +
>>>  drivers/mfd/Kconfig|  16 +
>>>  drivers/mfd/Makefile   |   1 +
>>>  drivers/mfd/fm33256b.c | 488 
>>> +
>>>  include/linux/mfd/fm33256b.h   |  76 
>>>  6 files changed, 617 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mfd/fm33256b.txt
>>>  create mode 100644 drivers/mfd/fm33256b.c
>>>  create mode 100644 include/linux/mfd/fm33256b.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/fm33256b.txt 
>>> b/Documentation/devicetree/bindings/mfd/fm33256b.txt
>>> new file mode 100644
>>> index 000..6591c94
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/fm33256b.txt
>>> @@ -0,0 +1,30 @@
>>> +Device-tree bindings for Cypress Semiconductor FM33256B Processor Companion
>>> +---
>>> +
>>> +Required properties:
>>> +- compatible: must be "cypress,fm33256b".
>>> +- reg: SPI chip select
>>> +- spi-max-frequency: Max SPI frequency to use (< 1600)
>>> +
>>> +Optional properties:
>>> +- cypress,charge-enabled: enable trickle charger
>>
>> What does the driver do if charging is disabled? Would using 'status = 
>> "disabled"' be any different?
>>  
> 
> Good catch, I didn't look at this patch but it includes a lot of code
> that should be going to the RTC driver.
> If trickle charging is not enabled, I guess the RTC will not charge its
> backup battery.

Thank you for the comments.

Alexandre is correct. If the "cypress,charge-enabled" property is
present, the internal 80 uA trickle charger is enabled to charge a
backup capacitor on the VBAK pin.

Do you want more code than the trickle charger setup moved to the RTC
driver?

>>> +- cypress,charge-fast: enable fast (1 mA) charging
>>
>> What does fast mean?
>>

Fast charging means charging with 1 mA instead of 80 uA. The wording is
from the datasheet, but I agree it should be renamed to something that
better explains what the difference is.

>> I think it is time for a common binding here. There's all sorts of 
>> variations on setting the charge current in bindings. Add something like 
>> "charge-current-microamp" in power_supply.txt and use it here. Then 
>> 1000uA implies "fast charge".
> 
> Well, this is not a power supply, it is an RTC.
> 
> I think both properties should got to the RTC subdevice and be parsed in
> the RTC driver.

The backup capacitor is primarily used to supply current to the RTC, but
a couple of other registers are also battery-backed (watchdog reset
cause, event counter), so I put the charger setup in the main MFD file.

If you prefer, I can move them both to the RTC driver?

> Note that for trickle charging, we currently have the following
> properties:
>  - trickle-resistor-ohms
>  - trickle-diode-disable
>  - abracon,tc-diode
>  - abracon,tc-resistor
> 
> abracon,tc-resistor can be replaced by trickle-resistor-ohms, I'll make
> a patch but abracon,tc-diode allows to select a diode type (and in
> particular a voltage drop).
> 
> I think we could add a trickle-current-microamp like you suggested and
> trickle-diode-disable can be reused.

So replace "cypress,charge-enabled/charge-fast" with
"trickle-current-microamp" and a check for 0/80/1000 uA?

Thanks,

-Jeppe

Re: [PATCH 1/3] mfd: add Cypress FM33256B Processor Companion driver

[Jeppe Ledet-Pedersen]

On 22/04/16 22:11, Alexandre Belloni wrote:
> On 22/04/2016 at 14:32:32 -0500, Rob Herring wrote :
>> On Wed, Apr 20, 2016 at 01:07:49PM +0200, Jeppe Ledet-Pedersen wrote:
>>> This patch adds support for the Cypress Semiconductor FM33256B processor
>>> companion. The device contains a 256 kbit FRAM, an RTC, a supply voltage
>>> monitor, and a watchdog timer.
>>>
>>> Signed-off-by: Jeppe Ledet-Pedersen 
>>> ---
>>>  Documentation/devicetree/bindings/mfd/fm33256b.txt |  30 ++
>>>  MAINTAINERS|   6 +
>>>  drivers/mfd/Kconfig|  16 +
>>>  drivers/mfd/Makefile   |   1 +
>>>  drivers/mfd/fm33256b.c | 488 
>>> +
>>>  include/linux/mfd/fm33256b.h   |  76 
>>>  6 files changed, 617 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mfd/fm33256b.txt
>>>  create mode 100644 drivers/mfd/fm33256b.c
>>>  create mode 100644 include/linux/mfd/fm33256b.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/fm33256b.txt 
>>> b/Documentation/devicetree/bindings/mfd/fm33256b.txt
>>> new file mode 100644
>>> index 000..6591c94
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/fm33256b.txt
>>> @@ -0,0 +1,30 @@
>>> +Device-tree bindings for Cypress Semiconductor FM33256B Processor Companion
>>> +---
>>> +
>>> +Required properties:
>>> +- compatible: must be "cypress,fm33256b".
>>> +- reg: SPI chip select
>>> +- spi-max-frequency: Max SPI frequency to use (< 1600)
>>> +
>>> +Optional properties:
>>> +- cypress,charge-enabled: enable trickle charger
>>
>> What does the driver do if charging is disabled? Would using 'status = 
>> "disabled"' be any different?
>>  
> 
> Good catch, I didn't look at this patch but it includes a lot of code
> that should be going to the RTC driver.
> If trickle charging is not enabled, I guess the RTC will not charge its
> backup battery.

Thank you for the comments.

Alexandre is correct. If the "cypress,charge-enabled" property is
present, the internal 80 uA trickle charger is enabled to charge a
backup capacitor on the VBAK pin.

Do you want more code than the trickle charger setup moved to the RTC
driver?

>>> +- cypress,charge-fast: enable fast (1 mA) charging
>>
>> What does fast mean?
>>

Fast charging means charging with 1 mA instead of 80 uA. The wording is
from the datasheet, but I agree it should be renamed to something that
better explains what the difference is.

>> I think it is time for a common binding here. There's all sorts of 
>> variations on setting the charge current in bindings. Add something like 
>> "charge-current-microamp" in power_supply.txt and use it here. Then 
>> 1000uA implies "fast charge".
> 
> Well, this is not a power supply, it is an RTC.
> 
> I think both properties should got to the RTC subdevice and be parsed in
> the RTC driver.

The backup capacitor is primarily used to supply current to the RTC, but
a couple of other registers are also battery-backed (watchdog reset
cause, event counter), so I put the charger setup in the main MFD file.

If you prefer, I can move them both to the RTC driver?

> Note that for trickle charging, we currently have the following
> properties:
>  - trickle-resistor-ohms
>  - trickle-diode-disable
>  - abracon,tc-diode
>  - abracon,tc-resistor
> 
> abracon,tc-resistor can be replaced by trickle-resistor-ohms, I'll make
> a patch but abracon,tc-diode allows to select a diode type (and in
> particular a voltage drop).
> 
> I think we could add a trickle-current-microamp like you suggested and
> trickle-diode-disable can be reused.

So replace "cypress,charge-enabled/charge-fast" with
"trickle-current-microamp" and a check for 0/80/1000 uA?

Thanks,

-Jeppe

Re: [PATCH 1/3] mfd: add Cypress FM33256B Processor Companion driver

[Alexandre Belloni]

On 22/04/2016 at 14:32:32 -0500, Rob Herring wrote :
> On Wed, Apr 20, 2016 at 01:07:49PM +0200, Jeppe Ledet-Pedersen wrote:
> > This patch adds support for the Cypress Semiconductor FM33256B processor
> > companion. The device contains a 256 kbit FRAM, an RTC, a supply voltage
> > monitor, and a watchdog timer.
> > 
> > Signed-off-by: Jeppe Ledet-Pedersen 
> > ---
> >  Documentation/devicetree/bindings/mfd/fm33256b.txt |  30 ++
> >  MAINTAINERS|   6 +
> >  drivers/mfd/Kconfig|  16 +
> >  drivers/mfd/Makefile   |   1 +
> >  drivers/mfd/fm33256b.c | 488 
> > +
> >  include/linux/mfd/fm33256b.h   |  76 
> >  6 files changed, 617 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/fm33256b.txt
> >  create mode 100644 drivers/mfd/fm33256b.c
> >  create mode 100644 include/linux/mfd/fm33256b.h
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/fm33256b.txt 
> > b/Documentation/devicetree/bindings/mfd/fm33256b.txt
> > new file mode 100644
> > index 000..6591c94
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/fm33256b.txt
> > @@ -0,0 +1,30 @@
> > +Device-tree bindings for Cypress Semiconductor FM33256B Processor Companion
> > +---
> > +
> > +Required properties:
> > +- compatible: must be "cypress,fm33256b".
> > +- reg: SPI chip select
> > +- spi-max-frequency: Max SPI frequency to use (< 1600)
> > +
> > +Optional properties:
> > +- cypress,charge-enabled: enable trickle charger
> 
> What does the driver do if charging is disabled? Would using 'status = 
> "disabled"' be any different?
>  

Good catch, I didn't look at this patch but it includes a lot of code
that should be going to the RTC driver.
If trickle charging is not enabled, I guess the RTC will not charge its
backup battery.

> > +- cypress,charge-fast: enable fast (1 mA) charging
> 
> What does fast mean?
> 
> I think it is time for a common binding here. There's all sorts of 
> variations on setting the charge current in bindings. Add something like 
> "charge-current-microamp" in power_supply.txt and use it here. Then 
> 1000uA implies "fast charge".

Well, this is not a power supply, it is an RTC.

I think both properties should got to the RTC subdevice and be parsed in
the RTC driver.

Note that for trickle charging, we currently have the following
properties:
 - trickle-resistor-ohms
 - trickle-diode-disable
 - abracon,tc-diode
 - abracon,tc-resistor

abracon,tc-resistor can be replaced by trickle-resistor-ohms, I'll make
a patch but abracon,tc-diode allows to select a diode type (and in
particular a voltage drop).

I think we could add a trickle-current-microamp like you suggested and
trickle-diode-disable can be reused.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/3] mfd: add Cypress FM33256B Processor Companion driver

[Alexandre Belloni]

On 22/04/2016 at 14:32:32 -0500, Rob Herring wrote :
> On Wed, Apr 20, 2016 at 01:07:49PM +0200, Jeppe Ledet-Pedersen wrote:
> > This patch adds support for the Cypress Semiconductor FM33256B processor
> > companion. The device contains a 256 kbit FRAM, an RTC, a supply voltage
> > monitor, and a watchdog timer.
> > 
> > Signed-off-by: Jeppe Ledet-Pedersen 
> > ---
> >  Documentation/devicetree/bindings/mfd/fm33256b.txt |  30 ++
> >  MAINTAINERS|   6 +
> >  drivers/mfd/Kconfig|  16 +
> >  drivers/mfd/Makefile   |   1 +
> >  drivers/mfd/fm33256b.c | 488 
> > +
> >  include/linux/mfd/fm33256b.h   |  76 
> >  6 files changed, 617 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/fm33256b.txt
> >  create mode 100644 drivers/mfd/fm33256b.c
> >  create mode 100644 include/linux/mfd/fm33256b.h
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/fm33256b.txt 
> > b/Documentation/devicetree/bindings/mfd/fm33256b.txt
> > new file mode 100644
> > index 000..6591c94
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/fm33256b.txt
> > @@ -0,0 +1,30 @@
> > +Device-tree bindings for Cypress Semiconductor FM33256B Processor Companion
> > +---
> > +
> > +Required properties:
> > +- compatible: must be "cypress,fm33256b".
> > +- reg: SPI chip select
> > +- spi-max-frequency: Max SPI frequency to use (< 1600)
> > +
> > +Optional properties:
> > +- cypress,charge-enabled: enable trickle charger
> 
> What does the driver do if charging is disabled? Would using 'status = 
> "disabled"' be any different?
>  

Good catch, I didn't look at this patch but it includes a lot of code
that should be going to the RTC driver.
If trickle charging is not enabled, I guess the RTC will not charge its
backup battery.

> > +- cypress,charge-fast: enable fast (1 mA) charging
> 
> What does fast mean?
> 
> I think it is time for a common binding here. There's all sorts of 
> variations on setting the charge current in bindings. Add something like 
> "charge-current-microamp" in power_supply.txt and use it here. Then 
> 1000uA implies "fast charge".

Well, this is not a power supply, it is an RTC.

I think both properties should got to the RTC subdevice and be parsed in
the RTC driver.

Note that for trickle charging, we currently have the following
properties:
 - trickle-resistor-ohms
 - trickle-diode-disable
 - abracon,tc-diode
 - abracon,tc-resistor

abracon,tc-resistor can be replaced by trickle-resistor-ohms, I'll make
a patch but abracon,tc-diode allows to select a diode type (and in
particular a voltage drop).

I think we could add a trickle-current-microamp like you suggested and
trickle-diode-disable can be reused.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/3] mfd: add Cypress FM33256B Processor Companion driver

[Rob Herring]

On Wed, Apr 20, 2016 at 01:07:49PM +0200, Jeppe Ledet-Pedersen wrote:
> This patch adds support for the Cypress Semiconductor FM33256B processor
> companion. The device contains a 256 kbit FRAM, an RTC, a supply voltage
> monitor, and a watchdog timer.
> 
> Signed-off-by: Jeppe Ledet-Pedersen 
> ---
>  Documentation/devicetree/bindings/mfd/fm33256b.txt |  30 ++
>  MAINTAINERS|   6 +
>  drivers/mfd/Kconfig|  16 +
>  drivers/mfd/Makefile   |   1 +
>  drivers/mfd/fm33256b.c | 488 
> +
>  include/linux/mfd/fm33256b.h   |  76 
>  6 files changed, 617 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/fm33256b.txt
>  create mode 100644 drivers/mfd/fm33256b.c
>  create mode 100644 include/linux/mfd/fm33256b.h
> 
> diff --git a/Documentation/devicetree/bindings/mfd/fm33256b.txt 
> b/Documentation/devicetree/bindings/mfd/fm33256b.txt
> new file mode 100644
> index 000..6591c94
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/fm33256b.txt
> @@ -0,0 +1,30 @@
> +Device-tree bindings for Cypress Semiconductor FM33256B Processor Companion
> +---
> +
> +Required properties:
> +- compatible: must be "cypress,fm33256b".
> +- reg: SPI chip select
> +- spi-max-frequency: Max SPI frequency to use (< 1600)
> +
> +Optional properties:
> +- cypress,charge-enabled: enable trickle charger

What does the driver do if charging is disabled? Would using 'status = 
"disabled"' be any different?
 
> +- cypress,charge-fast: enable fast (1 mA) charging

What does fast mean?

I think it is time for a common binding here. There's all sorts of 
variations on setting the charge current in bindings. Add something like 
"charge-current-microamp" in power_supply.txt and use it here. Then 
1000uA implies "fast charge".

> +
> +The MFD exposes two subdevices:
> +- The FRAM: "cypress,fm33256b-fram"
> +- The RTC: "cypress,fm33256b-rtc"
> +
> +Example:
> +
> +spi1: spi@f800800 {
> + status = "okay";
> + cs-gpios = < 25 0>;
> +
> + fm33256b@0 {
> + compatible = "cypress,fm33256b";
> + spi-max-frequency = <1000>;
> + cypress,charge-enabled;
> + cypress,charge-fast;
> + reg = <0>;
> + };

Where's the 2nd sub device?

> +};

Re: [PATCH 1/3] mfd: add Cypress FM33256B Processor Companion driver

[Rob Herring]

On Wed, Apr 20, 2016 at 01:07:49PM +0200, Jeppe Ledet-Pedersen wrote:
> This patch adds support for the Cypress Semiconductor FM33256B processor
> companion. The device contains a 256 kbit FRAM, an RTC, a supply voltage
> monitor, and a watchdog timer.
> 
> Signed-off-by: Jeppe Ledet-Pedersen 
> ---
>  Documentation/devicetree/bindings/mfd/fm33256b.txt |  30 ++
>  MAINTAINERS|   6 +
>  drivers/mfd/Kconfig|  16 +
>  drivers/mfd/Makefile   |   1 +
>  drivers/mfd/fm33256b.c | 488 
> +
>  include/linux/mfd/fm33256b.h   |  76 
>  6 files changed, 617 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/fm33256b.txt
>  create mode 100644 drivers/mfd/fm33256b.c
>  create mode 100644 include/linux/mfd/fm33256b.h
> 
> diff --git a/Documentation/devicetree/bindings/mfd/fm33256b.txt 
> b/Documentation/devicetree/bindings/mfd/fm33256b.txt
> new file mode 100644
> index 000..6591c94
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/fm33256b.txt
> @@ -0,0 +1,30 @@
> +Device-tree bindings for Cypress Semiconductor FM33256B Processor Companion
> +---
> +
> +Required properties:
> +- compatible: must be "cypress,fm33256b".
> +- reg: SPI chip select
> +- spi-max-frequency: Max SPI frequency to use (< 1600)
> +
> +Optional properties:
> +- cypress,charge-enabled: enable trickle charger

What does the driver do if charging is disabled? Would using 'status = 
"disabled"' be any different?
 
> +- cypress,charge-fast: enable fast (1 mA) charging

What does fast mean?

I think it is time for a common binding here. There's all sorts of 
variations on setting the charge current in bindings. Add something like 
"charge-current-microamp" in power_supply.txt and use it here. Then 
1000uA implies "fast charge".

> +
> +The MFD exposes two subdevices:
> +- The FRAM: "cypress,fm33256b-fram"
> +- The RTC: "cypress,fm33256b-rtc"
> +
> +Example:
> +
> +spi1: spi@f800800 {
> + status = "okay";
> + cs-gpios = < 25 0>;
> +
> + fm33256b@0 {
> + compatible = "cypress,fm33256b";
> + spi-max-frequency = <1000>;
> + cypress,charge-enabled;
> + cypress,charge-fast;
> + reg = <0>;
> + };

Where's the 2nd sub device?

> +};

Re: [PATCH 1/3] mfd: add Cypress FM33256B Processor Companion driver

[kbuild test robot]

Hi,

[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.6-rc4 next-20160421]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Jeppe-Ledet-Pedersen/Cypress-FM33256B-processor-companion-support/20160420-193004
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: x86_64-randconfig-n0-04220648 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

warning: (BMP085_SPI && TOUCHSCREEN_TSC2005 && SERIAL_MAX310X && 
SERIAL_SC16IS7XX_SPI && MFD_FM33256B && MFD_DA9052_SPI && MFD_MC13XXX_SPI && 
MFD_TPS65912_SPI && MFD_ARIZONA_SPI && MFD_WM831X_SPI && SND_SOC && 
SND_SOC_ADAU1761_SPI && SND_SOC_ADAU1781_SPI && SND_SOC_ADAU1977_SPI && 
SND_SOC_CS4271_SPI && SND_SOC_PCM3168A_SPI && SND_SOC_PCM512x_SPI && 
SND_SOC_SSM2602_SPI && SND_SOC_WM8804_SPI && RTC_I2C_AND_SPI && RTC_DRV_DS1343 
&& RTC_DRV_RX6110 && BMC150_ACCEL_SPI && MMA7455_SPI && AD5380 && BMG160_SPI && 
AFE4403 && INV_MPU6050_SPI && SENSORS_HMC5843_SPI) selects REGMAP_SPI which has 
unmet direct dependencies (SPI)
   drivers/built-in.o: In function `regmap_spi_read':
>> regmap-spi.c:(.text+0x29e4c9): undefined reference to `spi_write_then_read'
   drivers/built-in.o: In function `regmap_spi_gather_write':
>> regmap-spi.c:(.text+0x29e5e9): undefined reference to `spi_sync'
   drivers/built-in.o: In function `regmap_spi_write':
   regmap-spi.c:(.text+0x29e6d9): undefined reference to `spi_sync'
   drivers/built-in.o: In function `regmap_spi_async_write':
>> regmap-spi.c:(.text+0x29e9eb): undefined reference to `spi_async'
   drivers/built-in.o: In function `fm33256b_probe':
>> fm33256b.c:(.text+0x35c975): undefined reference to `spi_setup'
   drivers/built-in.o: In function `fm33256b_io':
>> fm33256b.c:(.text+0x35cec6): undefined reference to `spi_sync'
   fm33256b.c:(.text+0x35cf53): undefined reference to `spi_sync'
   drivers/built-in.o: In function `fm33256b_spi_driver_init':
>> fm33256b.c:(.init.text+0x131e9): undefined reference to 
>> `__spi_register_driver'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

Re: [PATCH 1/3] mfd: add Cypress FM33256B Processor Companion driver

[kbuild test robot]

Hi,

[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.6-rc4 next-20160421]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Jeppe-Ledet-Pedersen/Cypress-FM33256B-processor-companion-support/20160420-193004
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: x86_64-randconfig-n0-04220648 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

warning: (BMP085_SPI && TOUCHSCREEN_TSC2005 && SERIAL_MAX310X && 
SERIAL_SC16IS7XX_SPI && MFD_FM33256B && MFD_DA9052_SPI && MFD_MC13XXX_SPI && 
MFD_TPS65912_SPI && MFD_ARIZONA_SPI && MFD_WM831X_SPI && SND_SOC && 
SND_SOC_ADAU1761_SPI && SND_SOC_ADAU1781_SPI && SND_SOC_ADAU1977_SPI && 
SND_SOC_CS4271_SPI && SND_SOC_PCM3168A_SPI && SND_SOC_PCM512x_SPI && 
SND_SOC_SSM2602_SPI && SND_SOC_WM8804_SPI && RTC_I2C_AND_SPI && RTC_DRV_DS1343 
&& RTC_DRV_RX6110 && BMC150_ACCEL_SPI && MMA7455_SPI && AD5380 && BMG160_SPI && 
AFE4403 && INV_MPU6050_SPI && SENSORS_HMC5843_SPI) selects REGMAP_SPI which has 
unmet direct dependencies (SPI)
   drivers/built-in.o: In function `regmap_spi_read':
>> regmap-spi.c:(.text+0x29e4c9): undefined reference to `spi_write_then_read'
   drivers/built-in.o: In function `regmap_spi_gather_write':
>> regmap-spi.c:(.text+0x29e5e9): undefined reference to `spi_sync'
   drivers/built-in.o: In function `regmap_spi_write':
   regmap-spi.c:(.text+0x29e6d9): undefined reference to `spi_sync'
   drivers/built-in.o: In function `regmap_spi_async_write':
>> regmap-spi.c:(.text+0x29e9eb): undefined reference to `spi_async'
   drivers/built-in.o: In function `fm33256b_probe':
>> fm33256b.c:(.text+0x35c975): undefined reference to `spi_setup'
   drivers/built-in.o: In function `fm33256b_io':
>> fm33256b.c:(.text+0x35cec6): undefined reference to `spi_sync'
   fm33256b.c:(.text+0x35cf53): undefined reference to `spi_sync'
   drivers/built-in.o: In function `fm33256b_spi_driver_init':
>> fm33256b.c:(.init.text+0x131e9): undefined reference to 
>> `__spi_register_driver'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data