Add a device attribute to control the enable regulator. Write 1 to
enable, 0 to disable (ref-count minus one), or -1 to force disable the
physical pin.
To disable a set of amplifiers wired to the same enable gpio, each
device must be disabled. For example:
echo 0 > /sys/devices/.../device:00/enable
echo 0 > /sys/devices/.../device:01/enable
In an emergency, we can force disable from any device:
echo -1 > /sys/devices/.../device:00/enable
Signed-off-by: Thomas Preston <[email protected]>
Cc: Patrick Glaser <[email protected]>
Cc: Rob Duncan <[email protected]>
Cc: Nate Case <[email protected]>
---
sound/soc/codecs/tda7802.c | 65 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 64 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/tda7802.c b/sound/soc/codecs/tda7802.c
index 38ca52de85f0..62aae011d9f1 100644
--- a/sound/soc/codecs/tda7802.c
+++ b/sound/soc/codecs/tda7802.c
@@ -458,6 +458,42 @@ static struct snd_soc_dai_driver tda7802_dai_driver = {
.ops = &tda7802_dai_ops,
};
+static ssize_t enable_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct tda7802_priv *tda7802 = dev_get_drvdata(dev);
+ int enabled = regulator_is_enabled(tda7802->enable_reg) ? 1 : 0;
+
+ return scnprintf(buf, PAGE_SIZE, "%d\n", enabled);
+}
+
+static ssize_t enable_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct tda7802_priv *tda7802 = dev_get_drvdata(dev);
+ int err;
+
+ if (sysfs_streq(buf, "1")) {
+ err = regulator_enable(tda7802->enable_reg);
+ if (err < 0)
+ dev_err(dev, "Could not enable (sysfs)\n");
+ } else if (sysfs_streq(buf, "0")) {
+ err = regulator_disable(tda7802->enable_reg);
+ if (err < 0)
+ dev_err(dev, "Could not disable (sysfs)\n");
+ } else if (sysfs_streq(buf, "-1")) {
+ err = regulator_force_disable(tda7802->enable_reg);
+ if (err < 0)
+ dev_err(dev, "Could not force disable (sysfs)\n");
+ } else {
+ return -EINVAL;
+ }
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(enable);
+
/* read device tree or ACPI properties from device */
static int tda7802_read_properties(struct tda7802_priv *tda7802)
{
@@ -493,7 +529,34 @@ static int tda7802_read_properties(struct tda7802_priv *tda7802)
return err;
}
-static const struct snd_soc_component_driver tda7802_component_driver;
+static int tda7802_probe(struct snd_soc_component *component)
+{
+ struct tda7802_priv *tda7802 = snd_soc_component_get_drvdata(component);
+ struct device *dev = &tda7802->i2c->dev;
+ int err;
+
+ dev_dbg(dev, "%s\n", __func__);
+
+ err = device_create_file(dev, &dev_attr_enable);
+ if (err < 0) {
+ dev_err(dev, "Could not create enable attr\n");
+ return err;
+ }
+
+ return err;
+}
+
+static void tda7802_remove(struct snd_soc_component *component)
+{
+ struct tda7802_priv *tda7802 = snd_soc_component_get_drvdata(component);
+
+ device_remove_file(&tda7802->i2c->dev, &dev_attr_enable);
+}
+
+static const struct snd_soc_component_driver tda7802_component_driver = {
+ .probe = tda7802_probe,
+ .remove = tda7802_remove,
+};
static int tda7802_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
--
2.11.0
On 2019-06-11 19:49, Thomas Preston wrote:
> Add a device attribute to control the enable regulator. Write 1 to
> enable, 0 to disable (ref-count minus one), or -1 to force disable the
> physical pin.
>
> To disable a set of amplifiers wired to the same enable gpio, each
> device must be disabled. For example:
>
> echo 0 > /sys/devices/.../device:00/enable
> echo 0 > /sys/devices/.../device:01/enable
>
> In an emergency, we can force disable from any device:
>
> echo -1 > /sys/devices/.../device:00/enable
>
> Signed-off-by: Thomas Preston <[email protected]>
> Cc: Patrick Glaser <[email protected]>
> Cc: Rob Duncan <[email protected]>
> Cc: Nate Case <[email protected]>
> ---
> sound/soc/codecs/tda7802.c | 65 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/codecs/tda7802.c b/sound/soc/codecs/tda7802.c
> index 38ca52de85f0..62aae011d9f1 100644
> --- a/sound/soc/codecs/tda7802.c
> +++ b/sound/soc/codecs/tda7802.c
> @@ -458,6 +458,42 @@ static struct snd_soc_dai_driver tda7802_dai_driver = {
> .ops = &tda7802_dai_ops,
> };
>
> +static ssize_t enable_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct tda7802_priv *tda7802 = dev_get_drvdata(dev);
> + int enabled = regulator_is_enabled(tda7802->enable_reg) ? 1 : 0;
> +
> + return scnprintf(buf, PAGE_SIZE, "%d\n", enabled);
> +}
> +
> +static ssize_t enable_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct tda7802_priv *tda7802 = dev_get_drvdata(dev);
> + int err;
> +
> + if (sysfs_streq(buf, "1")) {
> + err = regulator_enable(tda7802->enable_reg);
> + if (err < 0)
> + dev_err(dev, "Could not enable (sysfs)\n");
> + } else if (sysfs_streq(buf, "0")) {
> + err = regulator_disable(tda7802->enable_reg);
> + if (err < 0)
> + dev_err(dev, "Could not disable (sysfs)\n");
> + } else if (sysfs_streq(buf, "-1")) {
> + err = regulator_force_disable(tda7802->enable_reg);
> + if (err < 0)
> + dev_err(dev, "Could not force disable (sysfs)\n");
> + } else {
> + return -EINVAL;
> + }
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(enable);
> +
> /* read device tree or ACPI properties from device */
> static int tda7802_read_properties(struct tda7802_priv *tda7802)
> {
> @@ -493,7 +529,34 @@ static int tda7802_read_properties(struct tda7802_priv *tda7802)
> return err;
> }
>
> -static const struct snd_soc_component_driver tda7802_component_driver;
> +static int tda7802_probe(struct snd_soc_component *component)
> +{
> + struct tda7802_priv *tda7802 = snd_soc_component_get_drvdata(component);
> + struct device *dev = &tda7802->i2c->dev;
> + int err;
> +
> + dev_dbg(dev, "%s\n", __func__);
Function name alone ain't very informational. Is this intended?
> +
> + err = device_create_file(dev, &dev_attr_enable);
> + if (err < 0) {
> + dev_err(dev, "Could not create enable attr\n");
> + return err;
> + }
Regardless of outcome, you'll be returning err here. Consider leaving
error message alone within if-statement. Remove redundant brackets if
you decide to do so.
> +
> + return err;
> +}
> +
> +static void tda7802_remove(struct snd_soc_component *component)
> +{
> + struct tda7802_priv *tda7802 = snd_soc_component_get_drvdata(component);
> +
> + device_remove_file(&tda7802->i2c->dev, &dev_attr_enable);
> +}
> +
> +static const struct snd_soc_component_driver tda7802_component_driver = {
> + .probe = tda7802_probe,
> + .remove = tda7802_remove,
> +};
>
> static int tda7802_i2c_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id)
>
On Tue, Jun 11, 2019 at 06:49:08PM +0100, Thomas Preston wrote:
> Add a device attribute to control the enable regulator. Write 1 to
> enable, 0 to disable (ref-count minus one), or -1 to force disable the
> physical pin.
> To disable a set of amplifiers wired to the same enable gpio, each
> device must be disabled. For example:
> echo 0 > /sys/devices/.../device:00/enable
> echo 0 > /sys/devices/.../device:01/enable
This is adding a new ABI completely outstide the standard ALSA and power
management frameworks and ABIs with no explanation as to why or
integration with the rest of the driver. This is obviously not in the
slightest bit OK. If there's something missing from the frameworks
extend the frameworks, don't just ignore them.