Re: [PATCH v1 1/3] regulator: act8865: Add support to turn off all outputs
Well, I will think about it and I will propose a separated patch with a generic property and helper functions. 2014-09-28 15:25 GMT+02:00 PERIER Romain : > 2014-09-28 14:25 GMT+02:00 Mark Brown : >> On Sun, Sep 28, 2014 at 02:00:54PM +0200, PERIER Romain wrote: >>> 2014-09-28 12:33 GMT+02:00 Mark Brown : >> >>> > We really need to come up with a standard property for this and document >>> > it rather than continuing to add individual device specific properties >>> > all doing the same thing, and probably also some helper code and/or a >>> > standard operation for this - there's a lot of drivers implementing the >>> > same pattern here. >> >>> I completly agree about adding a unified and generic property for that >>> purpose. >>> I already proposed something for that on devicetree ML, see the thread >>> "Proposal: generic property for system-power-controller". >>> Unfortunately I did not get replies :) . >> >> Did you CC relevant maintainers and send a patch? > > No I did not propose patches as I was an open discussion. Perhaps I > might propose something as part of my contribution for act8865... (at > least for the property) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 1/3] regulator: act8865: Add support to turn off all outputs
2014-09-28 14:25 GMT+02:00 Mark Brown : > On Sun, Sep 28, 2014 at 02:00:54PM +0200, PERIER Romain wrote: >> 2014-09-28 12:33 GMT+02:00 Mark Brown : > >> > We really need to come up with a standard property for this and document >> > it rather than continuing to add individual device specific properties >> > all doing the same thing, and probably also some helper code and/or a >> > standard operation for this - there's a lot of drivers implementing the >> > same pattern here. > >> I completly agree about adding a unified and generic property for that >> purpose. >> I already proposed something for that on devicetree ML, see the thread >> "Proposal: generic property for system-power-controller". >> Unfortunately I did not get replies :) . > > Did you CC relevant maintainers and send a patch? No I did not propose patches as I was an open discussion. Perhaps I might propose something as part of my contribution for act8865... (at least for the property) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 1/3] regulator: act8865: Add support to turn off all outputs
On Sun, Sep 28, 2014 at 02:00:54PM +0200, PERIER Romain wrote: > 2014-09-28 12:33 GMT+02:00 Mark Brown : > > We really need to come up with a standard property for this and document > > it rather than continuing to add individual device specific properties > > all doing the same thing, and probably also some helper code and/or a > > standard operation for this - there's a lot of drivers implementing the > > same pattern here. > I completly agree about adding a unified and generic property for that > purpose. > I already proposed something for that on devicetree ML, see the thread > "Proposal: generic property for system-power-controller". > Unfortunately I did not get replies :) . Did you CC relevant maintainers and send a patch? signature.asc Description: Digital signature
Re: [PATCH v1 1/3] regulator: act8865: Add support to turn off all outputs
Hi Mark,
2014-09-28 12:33 GMT+02:00 Mark Brown :
> On Sat, Sep 27, 2014 at 04:21:44PM +, Romain Perier wrote:
>> When the property "active-semi,system-power-controller" 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.
>
> We really need to come up with a standard property for this and document
> it rather than continuing to add individual device specific properties
> all doing the same thing, and probably also some helper code and/or a
> standard operation for this - there's a lot of drivers implementing the
> same pattern here.
I completly agree about adding a unified and generic property for that purpose.
I already proposed something for that on devicetree ML, see the thread
"Proposal: generic property for system-power-controller".
Unfortunately I did not get replies :) .
>
>> + if (dev->of_node &&
>> + of_property_read_bool(dev->of_node,
>> + "active-semi,system-power-controller")) {
>> + act8865_i2c_client = client;
>
> Indentation seems messed up here - tabs vs spaces?
Yes, I really don't understand where the problem is. I use good emacs
settings to be compatible with kernel coding style and git diff does
not show me indent/spaces problems :/ .
Sorry because this is a very boring issue when reviewing patches... I
will investigate ^^
Romain
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 1/3] regulator: act8865: Add support to turn off all outputs
On Sat, Sep 27, 2014 at 04:21:44PM +, Romain Perier wrote:
> When the property "active-semi,system-power-controller" 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.
We really need to come up with a standard property for this and document
it rather than continuing to add individual device specific properties
all doing the same thing, and probably also some helper code and/or a
standard operation for this - there's a lot of drivers implementing the
same pattern here.
> + if (dev->of_node &&
> + of_property_read_bool(dev->of_node,
> + "active-semi,system-power-controller")) {
> + act8865_i2c_client = client;
Indentation seems messed up here - tabs vs spaces?
signature.asc
Description: Digital signature
[PATCH v1 1/3] regulator: act8865: Add support to turn off all outputs
When the property "active-semi,system-power-controller" 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.
Signed-off-by: Romain Perier
---
drivers/regulator/act8865-regulator.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/regulator/act8865-regulator.c
b/drivers/regulator/act8865-regulator.c
index afd06f9..6cf202d 100644
--- a/drivers/regulator/act8865-regulator.c
+++ b/drivers/regulator/act8865-regulator.c
@@ -61,6 +61,8 @@
#defineACT8846_REG12_VSET 0xa0
#defineACT8846_REG12_CTRL 0xa1
#defineACT8846_REG13_CTRL 0xb1
+#defineACT8846_GLB_OFF_CTRL0xc3
+#defineACT8846_OFF_SYSMASK 0x18
/*
* ACT8865 Global Register Map.
@@ -84,6 +86,7 @@
#defineACT8865_LDO3_CTRL 0x61
#defineACT8865_LDO4_VSET 0x64
#defineACT8865_LDO4_CTRL 0x65
+#defineACT8865_MSTROFF 0x20
/*
* Field Definitions.
@@ -98,6 +101,8 @@
struct act8865 {
struct regmap *regmap;
+ int off_reg;
+ int off_mask;
};
static const struct regmap_config act8865_regmap_config = {
@@ -275,6 +280,16 @@ static struct regulator_init_data
return NULL;
}
+static struct i2c_client *act8865_i2c_client;
+static void act8865_power_off(void)
+{
+ struct act8865 *act8865;
+
+ act8865 = i2c_get_clientdata(act8865_i2c_client);
+ regmap_write(act8865->regmap, act8865->off_reg, act8865->off_mask);
+ while (1);
+}
+
static int act8865_pmic_probe(struct i2c_client *client,
const struct i2c_device_id *i2c_id)
{
@@ -285,6 +300,7 @@ static int act8865_pmic_probe(struct i2c_client *client,
int i, ret, num_regulators;
struct act8865 *act8865;
unsigned long type;
+ int off_reg, off_mask;
pdata = dev_get_platdata(dev);
@@ -304,10 +320,14 @@ static int act8865_pmic_probe(struct i2c_client *client,
case ACT8846:
regulators = act8846_regulators;
num_regulators = ARRAY_SIZE(act8846_regulators);
+ off_reg = ACT8846_GLB_OFF_CTRL;
+ off_mask = ACT8846_OFF_SYSMASK;
break;
case ACT8865:
regulators = act8865_regulators;
num_regulators = ARRAY_SIZE(act8865_regulators);
+ off_reg = ACT8865_SYS_CTRL;
+ off_mask = ACT8865_MSTROFF;
break;
default:
dev_err(dev, "invalid device id %lu\n", type);
@@ -345,6 +365,15 @@ static int act8865_pmic_probe(struct i2c_client *client,
return ret;
}
+ if (dev->of_node &&
+ of_property_read_bool(dev->of_node,
+ "active-semi,system-power-controller")) {
+ act8865_i2c_client = client;
+ act8865->off_reg = off_reg;
+ act8865->off_mask = off_mask;
+ pm_power_off = act8865_power_off;
+ }
+
/* Finally register devices */
for (i = 0; i < num_regulators; i++) {
const struct regulator_desc *desc = ®ulators[i];
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
