2020-07-23 23:04:16

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 0/3] Some sx9310 iio driver updates

These three patches are only related because I'm looking at this driver.
The first one resend the DT binding for the driver that was merged in
v5.8-rc1 with a small change to update for proper regulators. The second
patch fixes a few printks that are missing newlines and should
be totally non-trivial to apply. The final patch adds support to enable
the svdd supply so that this driver can work on a board I have where the
svdd supply isn't enabled at boot and needs to be turned on before this
driver starts to communicate with the chip.

Daniel Campello (1):
dt-bindings: iio: Add bindings for sx9310 sensor

Stephen Boyd (2):
iio: sx9310: Add newlines to printks
iio: sx9310: Enable regulator for svdd supply

.../iio/proximity/semtech,sx9310.yaml | 60 +++++++++++++++++++
drivers/iio/proximity/sx9310.c | 59 ++++++++++++++----
2 files changed, 106 insertions(+), 13 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml

Cc: Hartmut Knaack <[email protected]>
Cc: Lars-Peter Clausen <[email protected]>
Cc: Peter Meerwald-Stadler <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Daniel Campello <[email protected]>

base-commit: b3a9e3b9622ae10064826dccb4f7a52bd88c7407
--
Sent by a computer, using git, on the internet


2020-07-23 23:04:16

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 2/3] iio: sx9310: Add newlines to printks

Printks in the kernel have newlines at the end. Add them to the few
printks in this driver.

Cc: Gwendal Grignou <[email protected]>
Cc: Daniel Campello <[email protected]>
Cc: Hartmut Knaack <[email protected]>
Cc: Lars-Peter Clausen <[email protected]>
Cc: Peter Meerwald-Stadler <[email protected]>
Cc: Douglas Anderson <[email protected]>
Fixes: 72ad02b15d63 ("iio: Add SEMTECH SX9310/9311 sensor driver")
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/iio/proximity/sx9310.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
index d161f3061e35..84c3c9ae80dc 100644
--- a/drivers/iio/proximity/sx9310.c
+++ b/drivers/iio/proximity/sx9310.c
@@ -824,7 +824,7 @@ static int sx9310_init_compensation(struct iio_dev *indio_dev)

if (i < 0) {
dev_err(&data->client->dev,
- "initial compensation timed out: 0x%02x", val);
+ "initial compensation timed out: 0x%02x\n", val);
ret = -ETIMEDOUT;
}

@@ -871,14 +871,14 @@ static int sx9310_set_indio_dev_name(struct device *dev,
/* id will be NULL when enumerated via ACPI */
if (id) {
if (id->driver_data != whoami)
- dev_err(dev, "WHOAMI does not match i2c_device_id: %s",
+ dev_err(dev, "WHOAMI does not match i2c_device_id: %s\n",
id->name);
} else if (ACPI_HANDLE(dev)) {
acpi_id = acpi_match_device(dev->driver->acpi_match_table, dev);
if (!acpi_id)
return -ENODEV;
if (acpi_id->driver_data != whoami)
- dev_err(dev, "WHOAMI does not match acpi_device_id: %s",
+ dev_err(dev, "WHOAMI does not match acpi_device_id: %s\n",
acpi_id->id);
} else
return -ENODEV;
@@ -891,7 +891,7 @@ static int sx9310_set_indio_dev_name(struct device *dev,
indio_dev->name = "sx9311";
break;
default:
- dev_err(dev, "unexpected WHOAMI response: %u", whoami);
+ dev_err(dev, "unexpected WHOAMI response: %u\n", whoami);
return -ENODEV;
}

@@ -920,7 +920,7 @@ static int sx9310_probe(struct i2c_client *client,

ret = regmap_read(data->regmap, SX9310_REG_WHOAMI, &data->whoami);
if (ret < 0) {
- dev_err(&client->dev, "error in reading WHOAMI register: %d",
+ dev_err(&client->dev, "error in reading WHOAMI register: %d\n",
ret);
return ret;
}
--
Sent by a computer, using git, on the internet

2020-07-23 23:04:30

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 1/3] dt-bindings: iio: Add bindings for sx9310 sensor

From: Daniel Campello <[email protected]>

Adds device tree bandings for sx9310 sensor.

Signed-off-by: Daniel Campello <[email protected]>
Cc: Hartmut Knaack <[email protected]>
Cc: Lars-Peter Clausen <[email protected]>
Cc: Peter Meerwald-Stadler <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Douglas Anderson <[email protected]>
[[email protected]: Add both regulators and make them optional]
Signed-off-by: Stephen Boyd <[email protected]>
---
.../iio/proximity/semtech,sx9310.yaml | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml

diff --git a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml
new file mode 100644
index 000000000000..ba734ee868c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/proximity/semtech,sx9310.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Semtech's SX9310 capacitive proximity sensor
+
+maintainers:
+ - Daniel Campello <[email protected]>
+
+description: |
+ Semtech's SX9310/SX9311 capacitive proximity/button solution.
+
+ Specifications about the devices can be found at:
+ https://www.semtech.com/products/smart-sensing/sar-sensors/sx9310
+
+properties:
+ compatible:
+ enum:
+ - semtech,sx9310
+ - semtech,sx9311
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ description:
+ The sole interrupt generated by the device used to announce the
+ preceding reading request has finished and that data is
+ available or that a close/far proximity event has happened.
+ maxItems: 1
+
+ vdd-supply:
+ description: Main power supply
+
+ svdd-supply:
+ description: Host interface power supply
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ proximity@28 {
+ compatible = "semtech,sx9310";
+ reg = <0x28>;
+ interrupt-parent = <&pio>;
+ interrupts = <5 IRQ_TYPE_LEVEL_LOW 5>;
+ vdd-supply = <&pp3300_a>;
+ svdd-supply = <&pp1800_prox>;
+ };
+ };
--
Sent by a computer, using git, on the internet

2020-07-23 23:05:11

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 3/3] iio: sx9310: Enable regulator for svdd supply

Enable the digital IO power supply (svdd) during probe so that the i2c
communication works properly on boards that aggressively power gate this
supply.

Cc: Gwendal Grignou <[email protected]>
Cc: Daniel Campello <[email protected]>
Cc: Hartmut Knaack <[email protected]>
Cc: Lars-Peter Clausen <[email protected]>
Cc: Peter Meerwald-Stadler <[email protected]>
Cc: Douglas Anderson <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/iio/proximity/sx9310.c | 49 ++++++++++++++++++++++++++++------
1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
index 84c3c9ae80dc..d21c17a4d541 100644
--- a/drivers/iio/proximity/sx9310.c
+++ b/drivers/iio/proximity/sx9310.c
@@ -19,6 +19,7 @@
#include <linux/of.h>
#include <linux/pm.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <linux/slab.h>

#include <linux/iio/buffer.h>
@@ -131,6 +132,7 @@ struct sx9310_data {
struct i2c_client *client;
struct iio_trigger *trig;
struct regmap *regmap;
+ struct regulator *supply;
/*
* Last reading of the proximity status for each channel.
* We only send an event to user space when this changes.
@@ -914,21 +916,31 @@ static int sx9310_probe(struct i2c_client *client,
mutex_init(&data->mutex);
init_completion(&data->completion);

+ data->supply = devm_regulator_get(&client->dev, "svdd");
+ if (IS_ERR(data->supply))
+ return PTR_ERR(data->supply);
+
data->regmap = devm_regmap_init_i2c(client, &sx9310_regmap_config);
if (IS_ERR(data->regmap))
return PTR_ERR(data->regmap);

+ ret = regulator_enable(data->supply);
+ if (ret)
+ return ret;
+ /* Must wait for Tpor time after initial power up */
+ usleep_range(1000, 1100);
+
ret = regmap_read(data->regmap, SX9310_REG_WHOAMI, &data->whoami);
if (ret < 0) {
dev_err(&client->dev, "error in reading WHOAMI register: %d\n",
ret);
- return ret;
+ goto err;
}

ret = sx9310_set_indio_dev_name(&client->dev, indio_dev, id,
data->whoami);
if (ret < 0)
- return ret;
+ goto err;

ACPI_COMPANION_SET(&indio_dev->dev, ACPI_COMPANION(&client->dev));
indio_dev->dev.parent = &client->dev;
@@ -940,7 +952,7 @@ static int sx9310_probe(struct i2c_client *client,

ret = sx9310_init_device(indio_dev);
if (ret < 0)
- return ret;
+ goto err;

if (client->irq) {
ret = devm_request_threaded_irq(&client->dev, client->irq,
@@ -949,13 +961,15 @@ static int sx9310_probe(struct i2c_client *client,
IRQF_TRIGGER_LOW | IRQF_ONESHOT,
"sx9310_event", indio_dev);
if (ret < 0)
- return ret;
+ goto err;

data->trig =
devm_iio_trigger_alloc(&client->dev, "%s-dev%d",
indio_dev->name, indio_dev->id);
- if (!data->trig)
- return -ENOMEM;
+ if (!data->trig) {
+ ret = -ENOMEM;
+ goto err;
+ }

data->trig->dev.parent = &client->dev;
data->trig->ops = &sx9310_trigger_ops;
@@ -963,7 +977,7 @@ static int sx9310_probe(struct i2c_client *client,

ret = devm_iio_trigger_register(&client->dev, data->trig);
if (ret)
- return ret;
+ goto err;
}

ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev,
@@ -971,9 +985,27 @@ static int sx9310_probe(struct i2c_client *client,
sx9310_trigger_handler,
&sx9310_buffer_setup_ops);
if (ret < 0)
- return ret;
+ goto err;

return devm_iio_device_register(&client->dev, indio_dev);
+err:
+ regulator_disable(data->supply);
+ return ret;
+}
+
+static int sx9310_remove(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev;
+ struct sx9310_data *data;
+
+ indio_dev = i2c_get_clientdata(client);
+ if (!indio_dev)
+ return -ENODEV;
+
+ data = iio_priv(indio_dev);
+ regulator_disable(data->supply);
+
+ return 0;
}

static int __maybe_unused sx9310_suspend(struct device *dev)
@@ -1059,6 +1091,7 @@ static struct i2c_driver sx9310_driver = {
.pm = &sx9310_pm_ops,
},
.probe = sx9310_probe,
+ .remove = sx9310_remove,
.id_table = sx9310_id,
};
module_i2c_driver(sx9310_driver);
--
Sent by a computer, using git, on the internet

2020-07-24 20:26:02

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH 3/3] iio: sx9310: Enable regulator for svdd supply

Hi,

On Thu, Jul 23, 2020 at 4:03 PM Stephen Boyd <[email protected]> wrote:
>
> Enable the digital IO power supply (svdd) during probe so that the i2c
> communication works properly on boards that aggressively power gate this
> supply.
>
> Cc: Gwendal Grignou <[email protected]>
> Cc: Daniel Campello <[email protected]>
> Cc: Hartmut Knaack <[email protected]>
> Cc: Lars-Peter Clausen <[email protected]>
> Cc: Peter Meerwald-Stadler <[email protected]>
> Cc: Douglas Anderson <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> drivers/iio/proximity/sx9310.c | 49 ++++++++++++++++++++++++++++------
> 1 file changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
> index 84c3c9ae80dc..d21c17a4d541 100644
> --- a/drivers/iio/proximity/sx9310.c
> +++ b/drivers/iio/proximity/sx9310.c
> @@ -19,6 +19,7 @@
> #include <linux/of.h>
> #include <linux/pm.h>
> #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> #include <linux/slab.h>
>
> #include <linux/iio/buffer.h>
> @@ -131,6 +132,7 @@ struct sx9310_data {
> struct i2c_client *client;
> struct iio_trigger *trig;
> struct regmap *regmap;
> + struct regulator *supply;

Done need to store if you use devm. See below.


> /*
> * Last reading of the proximity status for each channel.
> * We only send an event to user space when this changes.
> @@ -914,21 +916,31 @@ static int sx9310_probe(struct i2c_client *client,
> mutex_init(&data->mutex);
> init_completion(&data->completion);
>
> + data->supply = devm_regulator_get(&client->dev, "svdd");
> + if (IS_ERR(data->supply))
> + return PTR_ERR(data->supply);
> +
> data->regmap = devm_regmap_init_i2c(client, &sx9310_regmap_config);
> if (IS_ERR(data->regmap))
> return PTR_ERR(data->regmap);
>
> + ret = regulator_enable(data->supply);
> + if (ret)
> + return ret;
> + /* Must wait for Tpor time after initial power up */
> + usleep_range(1000, 1100);

ret = devm_add_action_or_reset(&client->dev, sx9310_regulator_disable, supply);
if (ret)
return ret;

static void sx9310_regulator_disable(void *regulator) {
regulator_disable(regulator);
}

Then drop all changes below this line.


Seems like you could add support for the other regulator listed in the
bindings, too?


-Doug

2020-07-24 20:29:32

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH 2/3] iio: sx9310: Add newlines to printks

Hi,

On Thu, Jul 23, 2020 at 4:03 PM Stephen Boyd <[email protected]> wrote:
>
> Printks in the kernel have newlines at the end. Add them to the few
> printks in this driver.
>
> Cc: Gwendal Grignou <[email protected]>
> Cc: Daniel Campello <[email protected]>
> Cc: Hartmut Knaack <[email protected]>
> Cc: Lars-Peter Clausen <[email protected]>
> Cc: Peter Meerwald-Stadler <[email protected]>
> Cc: Douglas Anderson <[email protected]>
> Fixes: 72ad02b15d63 ("iio: Add SEMTECH SX9310/9311 sensor driver")
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> drivers/iio/proximity/sx9310.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Douglas Anderson <[email protected]>

2020-07-24 20:39:22

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH 3/3] iio: sx9310: Enable regulator for svdd supply

Quoting Doug Anderson (2020-07-24 13:25:01)
> On Thu, Jul 23, 2020 at 4:03 PM Stephen Boyd <[email protected]> wrote:
> > /*
> > * Last reading of the proximity status for each channel.
> > * We only send an event to user space when this changes.
> > @@ -914,21 +916,31 @@ static int sx9310_probe(struct i2c_client *client,
> > mutex_init(&data->mutex);
> > init_completion(&data->completion);
> >
> > + data->supply = devm_regulator_get(&client->dev, "svdd");
> > + if (IS_ERR(data->supply))
> > + return PTR_ERR(data->supply);
> > +
> > data->regmap = devm_regmap_init_i2c(client, &sx9310_regmap_config);
> > if (IS_ERR(data->regmap))
> > return PTR_ERR(data->regmap);
> >
> > + ret = regulator_enable(data->supply);
> > + if (ret)
> > + return ret;
> > + /* Must wait for Tpor time after initial power up */
> > + usleep_range(1000, 1100);
>
> ret = devm_add_action_or_reset(&client->dev, sx9310_regulator_disable, supply);
> if (ret)
> return ret;
>
> static void sx9310_regulator_disable(void *regulator) {
> regulator_disable(regulator);
> }
>
> Then drop all changes below this line.

Ok. I originally had code to turn off the supply during suspend/resume
but I removed it because I wasn't sure that the device would be OK with
the IO interface dropping after the device went to sleep.

>
>
> Seems like you could add support for the other regulator listed in the
> bindings, too?
>

Sure. I will use the bulk APIs.

2020-07-24 20:42:40

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: iio: Add bindings for sx9310 sensor

Hi,

On Thu, Jul 23, 2020 at 4:03 PM Stephen Boyd <[email protected]> wrote:
>
> From: Daniel Campello <[email protected]>
>
> Adds device tree bandings for sx9310 sensor.
>
> Signed-off-by: Daniel Campello <[email protected]>
> Cc: Hartmut Knaack <[email protected]>
> Cc: Lars-Peter Clausen <[email protected]>
> Cc: Peter Meerwald-Stadler <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Douglas Anderson <[email protected]>
> [[email protected]: Add both regulators and make them optional]
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> .../iio/proximity/semtech,sx9310.yaml | 60 +++++++++++++++++++
> 1 file changed, 60 insertions(+)

Reviewed-by: Douglas Anderson <[email protected]>

2020-07-24 20:44:14

by Daniel Campello

[permalink] [raw]
Subject: Re: [PATCH 2/3] iio: sx9310: Add newlines to printks

Hi,

On Thu, Jul 23, 2020 at 5:03 PM Stephen Boyd <[email protected]> wrote:
>
> Printks in the kernel have newlines at the end. Add them to the few
> printks in this driver.
>
> Cc: Gwendal Grignou <[email protected]>
> Cc: Daniel Campello <[email protected]>
> Cc: Hartmut Knaack <[email protected]>
> Cc: Lars-Peter Clausen <[email protected]>
> Cc: Peter Meerwald-Stadler <[email protected]>
> Cc: Douglas Anderson <[email protected]>
> Fixes: 72ad02b15d63 ("iio: Add SEMTECH SX9310/9311 sensor driver")
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> drivers/iio/proximity/sx9310.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Daniel Campello <[email protected]>