Re: [RFC PATCH v2 2/4] regulator: act8865: Add support to turn off all outputs

From: Mark Brown
Date: Mon Oct 13 2014 - 09:17:28 EST


On Tue, Oct 07, 2014 at 07:45:02PM +0000, Romain Perier wrote:
> When the property "poweroff-source" is found in the
> devicetree, the function pm_power_off is defined. This function sends the
> rights bit fields to the global off control register. shutdown/poweroff
> commands are now supported for hardware components which use these PMU.

Reviwed-by: Mark Brown <broonie@xxxxxxxxxx>

but...

> + if (dev->of_node && is_system_poweroff_source(dev->of_node) &&
> + !pm_power_off) {
> + act8865_i2c_client = client;
> + act8865->off_reg = off_reg;
> + act8865->off_mask = off_mask;
> + pm_power_off = act8865_power_off;
> + }

Perhaps worth wrapping the dev->of_node check into the function? I
imagine the same pattern will be quite common.

Might also make sense to warn if we've got the property but we're not
setting pm_power_off - it's indicating that things aren't working as
expected, an error will help users figure out what's going wrong.

Attachment: signature.asc
Description: Digital signature