Re: [PATCH v3 2/4] mfd: add Gateworks System Controller core driver

[Mark Brown]

On Wed, Apr 04, 2018 at 03:12:39PM +0200, Andrew Lunn wrote:
> > What about the 'reset' functionality? Is there something in the power
> > supply API for hooking in a GPIO based power switch (in my case it
> > would be i2c) as I would think that would be common for ATX supplies?
> > I didn't see anything in Documentation/power.

> > This is what led me to the restart handler idea. Ultimately when
> > someone issues a 'reboot' I would like it to use the GSC to
> > power-cycle the board.

> I think you end up with the same problem. By the time you need to turn
> the power supply off, too much of the kernel is shut down to be able
> to use I2C. And if you are in the middle of an Oops, you have no idea
> of the current state. Another I2C transaction could be under way etc.
> All the current reset drivers are pretty much self contained, atomic
> and use KISS hardware like a GPIO.

> Maybe you best bet is to see if you can find any other I2C PMICs which
> the kernel supports.

Most systems have a handshake for final power down via asserting signals
rather than using register writes, the final power down sequence usually
runs way after software.  There's a few things that don't which just
unceremoniously cut the power earlier on without completing the full
power down sequence which for all practical purposes mostly works.


signature.asc
Description: PGP signature

Re: [PATCH v3 2/4] mfd: add Gateworks System Controller core driver

[Mark Brown]

On Wed, Apr 04, 2018 at 03:12:39PM +0200, Andrew Lunn wrote:
> > What about the 'reset' functionality? Is there something in the power
> > supply API for hooking in a GPIO based power switch (in my case it
> > would be i2c) as I would think that would be common for ATX supplies?
> > I didn't see anything in Documentation/power.

> > This is what led me to the restart handler idea. Ultimately when
> > someone issues a 'reboot' I would like it to use the GSC to
> > power-cycle the board.

> I think you end up with the same problem. By the time you need to turn
> the power supply off, too much of the kernel is shut down to be able
> to use I2C. And if you are in the middle of an Oops, you have no idea
> of the current state. Another I2C transaction could be under way etc.
> All the current reset drivers are pretty much self contained, atomic
> and use KISS hardware like a GPIO.

> Maybe you best bet is to see if you can find any other I2C PMICs which
> the kernel supports.

Most systems have a handshake for final power down via asserting signals
rather than using register writes, the final power down sequence usually
runs way after software.  There's a few things that don't which just
unceremoniously cut the power earlier on without completing the full
power down sequence which for all practical purposes mostly works.


signature.asc
Description: PGP signature

Re: [PATCH v3 2/4] mfd: add Gateworks System Controller core driver

[Andrew Lunn]

> What about the 'reset' functionality? Is there something in the power
> supply API for hooking in a GPIO based power switch (in my case it
> would be i2c) as I would think that would be common for ATX supplies?
> I didn't see anything in Documentation/power.
> 
> This is what led me to the restart handler idea. Ultimately when
> someone issues a 'reboot' I would like it to use the GSC to
> power-cycle the board.

Hi Tim

I think you end up with the same problem. By the time you need to turn
the power supply off, too much of the kernel is shut down to be able
to use I2C. And if you are in the middle of an Oops, you have no idea
of the current state. Another I2C transaction could be under way etc.
All the current reset drivers are pretty much self contained, atomic
and use KISS hardware like a GPIO.

Maybe you best bet is to see if you can find any other I2C PMICs which
the kernel supports.

Andrew

Re: [PATCH v3 2/4] mfd: add Gateworks System Controller core driver

[Andrew Lunn]

> What about the 'reset' functionality? Is there something in the power
> supply API for hooking in a GPIO based power switch (in my case it
> would be i2c) as I would think that would be common for ATX supplies?
> I didn't see anything in Documentation/power.
> 
> This is what led me to the restart handler idea. Ultimately when
> someone issues a 'reboot' I would like it to use the GSC to
> power-cycle the board.

Hi Tim

I think you end up with the same problem. By the time you need to turn
the power supply off, too much of the kernel is shut down to be able
to use I2C. And if you are in the middle of an Oops, you have no idea
of the current state. Another I2C transaction could be under way etc.
All the current reset drivers are pretty much self contained, atomic
and use KISS hardware like a GPIO.

Maybe you best bet is to see if you can find any other I2C PMICs which
the kernel supports.

Andrew

Re: [PATCH v3 2/4] mfd: add Gateworks System Controller core driver

[Tim Harvey]

On Tue, Apr 3, 2018 at 9:47 AM, Andrew Lunn  wrote:
> On Tue, Apr 03, 2018 at 08:48:27AM -0700, Tim Harvey wrote:
>> On Wed, Mar 28, 2018 at 8:14 AM, Tim Harvey  wrote:
>> > The Gateworks System Controller (GSC) is an I2C slave controller
>> > implemented with an MSP430 micro-controller whose firmware embeds the
>> > following features:
>> >  - I/O expander (16 GPIO's) using PCA955x protocol
>> >  - Real Time Clock using DS1672 protocol
>> >  - User EEPROM using AT24 protocol
>> >  - HWMON using custom protocol
>> >  - Interrupt controller with tamper detect, user pushbotton
>> >  - Watchdog controller capable of full board power-cycle
>> >  - Power Control capable of full board power-cycle
>> >
>> > see http://trac.gateworks.com/wiki/gsc for more details
>> >
>> 
>> > +
>> > +/*
>> > + * gsc_powerdown - API to use GSC to power down board for a specific time
>> > + *
>> > + * secs - number of seconds to remain powered off
>> > + */
>> > +static int gsc_powerdown(struct gsc_dev *gsc, unsigned long secs)
>> > +{
>> > +   int ret;
>> > +   unsigned char regs[4];
>> > +
>> > +   dev_info(>i2c->dev, "GSC powerdown for %ld seconds\n",
>> > +secs);
>> > +   regs[0] = secs & 0xff;
>> > +   regs[1] = (secs >> 8) & 0xff;
>> > +   regs[2] = (secs >> 16) & 0xff;
>> > +   regs[3] = (secs >> 24) & 0xff;
>> > +   ret = regmap_bulk_write(gsc->regmap, GSC_TIME_ADD, regs, 4);
>> > +
>> > +   return ret;
>> > +}
>>
>> Any feedback on the 'powerdown' sysfs attribute that hooks to this
>> function? This allows the GSC to disable the board primary power
>> supply for 2^32 seconds and is often used to 'reset' the board
>> although it could also be used to put the board in a power down state
>> longer. I'm wondering if there is a more appropriate API for this in
>> the kernel that I don't know about.
>
> Hi Tim
>
> RTC can cause wakeup when an alarm is set. It looks like the DS1672
> does not have this. But you are emulating the DS1672 right? You could
> add a second emulated RTC which does support an alarm? DS3232?

Andrew,

Thanks for the response!

An RTC alarm may indeed be a good route for the overall sleep
functionality I will look into that.

What about the 'reset' functionality? Is there something in the power
supply API for hooking in a GPIO based power switch (in my case it
would be i2c) as I would think that would be common for ATX supplies?
I didn't see anything in Documentation/power.

This is what led me to the restart handler idea. Ultimately when
someone issues a 'reboot' I would like it to use the GSC to
power-cycle the board.

>
>> I would also like to register a restart handler using this but I
>> believe that ARM restart handlers currently can not use I2C - is that
>> correct?
>
> There are plenty which use GPIOs, or UARTs. Not seen any which use
> i2c. What do you think does not work at this point?

I'll have to dig around for the e-mail thread. I recall someone else
trying to implement a restart handler for something hanging off i2c
and the issue was that by the time the (ARM) restart handler got
called interrupts were disabled making i2c unreliable. I have hooked
the ARM restart handler to my GSC powerdown in some kernels and have
had mixed results. When the handler gets called from a clean 'reboot'
things seem fine but if its called from some error condition that
halts the kernel it seems that i2c may not be reliable anymore.

Regards,

Tim

Re: [PATCH v3 2/4] mfd: add Gateworks System Controller core driver

[Tim Harvey]

On Tue, Apr 3, 2018 at 9:47 AM, Andrew Lunn  wrote:
> On Tue, Apr 03, 2018 at 08:48:27AM -0700, Tim Harvey wrote:
>> On Wed, Mar 28, 2018 at 8:14 AM, Tim Harvey  wrote:
>> > The Gateworks System Controller (GSC) is an I2C slave controller
>> > implemented with an MSP430 micro-controller whose firmware embeds the
>> > following features:
>> >  - I/O expander (16 GPIO's) using PCA955x protocol
>> >  - Real Time Clock using DS1672 protocol
>> >  - User EEPROM using AT24 protocol
>> >  - HWMON using custom protocol
>> >  - Interrupt controller with tamper detect, user pushbotton
>> >  - Watchdog controller capable of full board power-cycle
>> >  - Power Control capable of full board power-cycle
>> >
>> > see http://trac.gateworks.com/wiki/gsc for more details
>> >
>> 
>> > +
>> > +/*
>> > + * gsc_powerdown - API to use GSC to power down board for a specific time
>> > + *
>> > + * secs - number of seconds to remain powered off
>> > + */
>> > +static int gsc_powerdown(struct gsc_dev *gsc, unsigned long secs)
>> > +{
>> > +   int ret;
>> > +   unsigned char regs[4];
>> > +
>> > +   dev_info(>i2c->dev, "GSC powerdown for %ld seconds\n",
>> > +secs);
>> > +   regs[0] = secs & 0xff;
>> > +   regs[1] = (secs >> 8) & 0xff;
>> > +   regs[2] = (secs >> 16) & 0xff;
>> > +   regs[3] = (secs >> 24) & 0xff;
>> > +   ret = regmap_bulk_write(gsc->regmap, GSC_TIME_ADD, regs, 4);
>> > +
>> > +   return ret;
>> > +}
>>
>> Any feedback on the 'powerdown' sysfs attribute that hooks to this
>> function? This allows the GSC to disable the board primary power
>> supply for 2^32 seconds and is often used to 'reset' the board
>> although it could also be used to put the board in a power down state
>> longer. I'm wondering if there is a more appropriate API for this in
>> the kernel that I don't know about.
>
> Hi Tim
>
> RTC can cause wakeup when an alarm is set. It looks like the DS1672
> does not have this. But you are emulating the DS1672 right? You could
> add a second emulated RTC which does support an alarm? DS3232?

Andrew,

Thanks for the response!

An RTC alarm may indeed be a good route for the overall sleep
functionality I will look into that.

What about the 'reset' functionality? Is there something in the power
supply API for hooking in a GPIO based power switch (in my case it
would be i2c) as I would think that would be common for ATX supplies?
I didn't see anything in Documentation/power.

This is what led me to the restart handler idea. Ultimately when
someone issues a 'reboot' I would like it to use the GSC to
power-cycle the board.

>
>> I would also like to register a restart handler using this but I
>> believe that ARM restart handlers currently can not use I2C - is that
>> correct?
>
> There are plenty which use GPIOs, or UARTs. Not seen any which use
> i2c. What do you think does not work at this point?

I'll have to dig around for the e-mail thread. I recall someone else
trying to implement a restart handler for something hanging off i2c
and the issue was that by the time the (ARM) restart handler got
called interrupts were disabled making i2c unreliable. I have hooked
the ARM restart handler to my GSC powerdown in some kernels and have
had mixed results. When the handler gets called from a clean 'reboot'
things seem fine but if its called from some error condition that
halts the kernel it seems that i2c may not be reliable anymore.

Regards,

Tim

Re: [PATCH v3 2/4] mfd: add Gateworks System Controller core driver

[Andrew Lunn]

On Tue, Apr 03, 2018 at 08:48:27AM -0700, Tim Harvey wrote:
> On Wed, Mar 28, 2018 at 8:14 AM, Tim Harvey  wrote:
> > The Gateworks System Controller (GSC) is an I2C slave controller
> > implemented with an MSP430 micro-controller whose firmware embeds the
> > following features:
> >  - I/O expander (16 GPIO's) using PCA955x protocol
> >  - Real Time Clock using DS1672 protocol
> >  - User EEPROM using AT24 protocol
> >  - HWMON using custom protocol
> >  - Interrupt controller with tamper detect, user pushbotton
> >  - Watchdog controller capable of full board power-cycle
> >  - Power Control capable of full board power-cycle
> >
> > see http://trac.gateworks.com/wiki/gsc for more details
> >
> 
> > +
> > +/*
> > + * gsc_powerdown - API to use GSC to power down board for a specific time
> > + *
> > + * secs - number of seconds to remain powered off
> > + */
> > +static int gsc_powerdown(struct gsc_dev *gsc, unsigned long secs)
> > +{
> > +   int ret;
> > +   unsigned char regs[4];
> > +
> > +   dev_info(>i2c->dev, "GSC powerdown for %ld seconds\n",
> > +secs);
> > +   regs[0] = secs & 0xff;
> > +   regs[1] = (secs >> 8) & 0xff;
> > +   regs[2] = (secs >> 16) & 0xff;
> > +   regs[3] = (secs >> 24) & 0xff;
> > +   ret = regmap_bulk_write(gsc->regmap, GSC_TIME_ADD, regs, 4);
> > +
> > +   return ret;
> > +}
> 
> Any feedback on the 'powerdown' sysfs attribute that hooks to this
> function? This allows the GSC to disable the board primary power
> supply for 2^32 seconds and is often used to 'reset' the board
> although it could also be used to put the board in a power down state
> longer. I'm wondering if there is a more appropriate API for this in
> the kernel that I don't know about.

Hi Tim

RTC can cause wakeup when an alarm is set. It looks like the DS1672
does not have this. But you are emulating the DS1672 right? You could
add a second emulated RTC which does support an alarm? DS3232?

> I would also like to register a restart handler using this but I
> believe that ARM restart handlers currently can not use I2C - is that
> correct?

There are plenty which use GPIOs, or UARTs. Not seen any which use
i2c. What do you think does not work at this point?

 Andrew

Re: [PATCH v3 2/4] mfd: add Gateworks System Controller core driver

[Andrew Lunn]

On Tue, Apr 03, 2018 at 08:48:27AM -0700, Tim Harvey wrote:
> On Wed, Mar 28, 2018 at 8:14 AM, Tim Harvey  wrote:
> > The Gateworks System Controller (GSC) is an I2C slave controller
> > implemented with an MSP430 micro-controller whose firmware embeds the
> > following features:
> >  - I/O expander (16 GPIO's) using PCA955x protocol
> >  - Real Time Clock using DS1672 protocol
> >  - User EEPROM using AT24 protocol
> >  - HWMON using custom protocol
> >  - Interrupt controller with tamper detect, user pushbotton
> >  - Watchdog controller capable of full board power-cycle
> >  - Power Control capable of full board power-cycle
> >
> > see http://trac.gateworks.com/wiki/gsc for more details
> >
> 
> > +
> > +/*
> > + * gsc_powerdown - API to use GSC to power down board for a specific time
> > + *
> > + * secs - number of seconds to remain powered off
> > + */
> > +static int gsc_powerdown(struct gsc_dev *gsc, unsigned long secs)
> > +{
> > +   int ret;
> > +   unsigned char regs[4];
> > +
> > +   dev_info(>i2c->dev, "GSC powerdown for %ld seconds\n",
> > +secs);
> > +   regs[0] = secs & 0xff;
> > +   regs[1] = (secs >> 8) & 0xff;
> > +   regs[2] = (secs >> 16) & 0xff;
> > +   regs[3] = (secs >> 24) & 0xff;
> > +   ret = regmap_bulk_write(gsc->regmap, GSC_TIME_ADD, regs, 4);
> > +
> > +   return ret;
> > +}
> 
> Any feedback on the 'powerdown' sysfs attribute that hooks to this
> function? This allows the GSC to disable the board primary power
> supply for 2^32 seconds and is often used to 'reset' the board
> although it could also be used to put the board in a power down state
> longer. I'm wondering if there is a more appropriate API for this in
> the kernel that I don't know about.

Hi Tim

RTC can cause wakeup when an alarm is set. It looks like the DS1672
does not have this. But you are emulating the DS1672 right? You could
add a second emulated RTC which does support an alarm? DS3232?

> I would also like to register a restart handler using this but I
> believe that ARM restart handlers currently can not use I2C - is that
> correct?

There are plenty which use GPIOs, or UARTs. Not seen any which use
i2c. What do you think does not work at this point?

 Andrew

Re: [PATCH v3 2/4] mfd: add Gateworks System Controller core driver

[Tim Harvey]

On Wed, Mar 28, 2018 at 8:14 AM, Tim Harvey  wrote:
> The Gateworks System Controller (GSC) is an I2C slave controller
> implemented with an MSP430 micro-controller whose firmware embeds the
> following features:
>  - I/O expander (16 GPIO's) using PCA955x protocol
>  - Real Time Clock using DS1672 protocol
>  - User EEPROM using AT24 protocol
>  - HWMON using custom protocol
>  - Interrupt controller with tamper detect, user pushbotton
>  - Watchdog controller capable of full board power-cycle
>  - Power Control capable of full board power-cycle
>
> see http://trac.gateworks.com/wiki/gsc for more details
>

> +
> +/*
> + * gsc_powerdown - API to use GSC to power down board for a specific time
> + *
> + * secs - number of seconds to remain powered off
> + */
> +static int gsc_powerdown(struct gsc_dev *gsc, unsigned long secs)
> +{
> +   int ret;
> +   unsigned char regs[4];
> +
> +   dev_info(>i2c->dev, "GSC powerdown for %ld seconds\n",
> +secs);
> +   regs[0] = secs & 0xff;
> +   regs[1] = (secs >> 8) & 0xff;
> +   regs[2] = (secs >> 16) & 0xff;
> +   regs[3] = (secs >> 24) & 0xff;
> +   ret = regmap_bulk_write(gsc->regmap, GSC_TIME_ADD, regs, 4);
> +
> +   return ret;
> +}

Any feedback on the 'powerdown' sysfs attribute that hooks to this
function? This allows the GSC to disable the board primary power
supply for 2^32 seconds and is often used to 'reset' the board
although it could also be used to put the board in a power down state
longer. I'm wondering if there is a more appropriate API for this in
the kernel that I don't know about.

I would also like to register a restart handler using this but I
believe that ARM restart handlers currently can not use I2C - is that
correct?

Regards,

Tim

Re: [PATCH v3 2/4] mfd: add Gateworks System Controller core driver

[Tim Harvey]

On Wed, Mar 28, 2018 at 8:14 AM, Tim Harvey  wrote:
> The Gateworks System Controller (GSC) is an I2C slave controller
> implemented with an MSP430 micro-controller whose firmware embeds the
> following features:
>  - I/O expander (16 GPIO's) using PCA955x protocol
>  - Real Time Clock using DS1672 protocol
>  - User EEPROM using AT24 protocol
>  - HWMON using custom protocol
>  - Interrupt controller with tamper detect, user pushbotton
>  - Watchdog controller capable of full board power-cycle
>  - Power Control capable of full board power-cycle
>
> see http://trac.gateworks.com/wiki/gsc for more details
>

> +
> +/*
> + * gsc_powerdown - API to use GSC to power down board for a specific time
> + *
> + * secs - number of seconds to remain powered off
> + */
> +static int gsc_powerdown(struct gsc_dev *gsc, unsigned long secs)
> +{
> +   int ret;
> +   unsigned char regs[4];
> +
> +   dev_info(>i2c->dev, "GSC powerdown for %ld seconds\n",
> +secs);
> +   regs[0] = secs & 0xff;
> +   regs[1] = (secs >> 8) & 0xff;
> +   regs[2] = (secs >> 16) & 0xff;
> +   regs[3] = (secs >> 24) & 0xff;
> +   ret = regmap_bulk_write(gsc->regmap, GSC_TIME_ADD, regs, 4);
> +
> +   return ret;
> +}

Any feedback on the 'powerdown' sysfs attribute that hooks to this
function? This allows the GSC to disable the board primary power
supply for 2^32 seconds and is often used to 'reset' the board
although it could also be used to put the board in a power down state
longer. I'm wondering if there is a more appropriate API for this in
the kernel that I don't know about.

I would also like to register a restart handler using this but I
believe that ARM restart handlers currently can not use I2C - is that
correct?

Regards,

Tim