2018-12-05 15:49:54

by Paweł Chmiel

[permalink] [raw]
Subject: [PATCH 0/5] media: radio-si470x-i2c: Add device tree and reset gpio support

This patchset adds support for device tree and reset-gpios
to si470x i2c radio driver.

First two patches adds and documents device tree support.
Third patch simplifies code by using managed resource helpers.
Last two patches adds and documents new optional reset gpios support.

It was tested on Samsung Galaxy S (i9000) phone.

Paweł Chmiel (5):
si470x-i2c: Add device tree support
media: dt-bindings: Add binding for si470x radio
si470x-i2c: Use managed resource helpers
si470x-i2c: Add optional reset-gpio support
media: dt-bindings: si470x: Document new reset-gpios property

.../devicetree/bindings/media/si470x.txt | 26 ++++++++++
drivers/media/radio/si470x/radio-si470x-i2c.c | 52 ++++++++++++-------
drivers/media/radio/si470x/radio-si470x.h | 1 +
3 files changed, 61 insertions(+), 18 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/si470x.txt

--
2.17.1



2018-12-05 15:49:58

by Paweł Chmiel

[permalink] [raw]
Subject: [PATCH 1/5] si470x-i2c: Add device tree support

This commit enables device tree support adding simple of_match table.

Signed-off-by: Paweł Chmiel <[email protected]>
---
drivers/media/radio/si470x/radio-si470x-i2c.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c b/drivers/media/radio/si470x/radio-si470x-i2c.c
index 9751ea1d80be..250828ddb5fa 100644
--- a/drivers/media/radio/si470x/radio-si470x-i2c.c
+++ b/drivers/media/radio/si470x/radio-si470x-i2c.c
@@ -527,6 +527,13 @@ static int si470x_i2c_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(si470x_i2c_pm, si470x_i2c_suspend, si470x_i2c_resume);
#endif

+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id si470x_of_match[] = {
+ { .compatible = "silabs,si470x" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, si470x_of_match);
+#endif

/*
* si470x_i2c_driver - i2c driver interface
@@ -534,6 +541,7 @@ static SIMPLE_DEV_PM_OPS(si470x_i2c_pm, si470x_i2c_suspend, si470x_i2c_resume);
static struct i2c_driver si470x_i2c_driver = {
.driver = {
.name = "si470x",
+ .of_match_table = of_match_ptr(si470x_of_match),
#ifdef CONFIG_PM_SLEEP
.pm = &si470x_i2c_pm,
#endif
--
2.17.1


2018-12-05 15:50:03

by Paweł Chmiel

[permalink] [raw]
Subject: [PATCH 4/5] si470x-i2c: Add optional reset-gpio support

If reset-gpio is defined, use it to bring device out of reset.
Without this, it's not possible to access si470x registers.

Signed-off-by: Paweł Chmiel <[email protected]>
---
drivers/media/radio/si470x/radio-si470x-i2c.c | 15 +++++++++++++++
drivers/media/radio/si470x/radio-si470x.h | 1 +
2 files changed, 16 insertions(+)

diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c b/drivers/media/radio/si470x/radio-si470x-i2c.c
index a7ac09c55188..15eea2b2c90f 100644
--- a/drivers/media/radio/si470x/radio-si470x-i2c.c
+++ b/drivers/media/radio/si470x/radio-si470x-i2c.c
@@ -28,6 +28,7 @@
#include <linux/i2c.h>
#include <linux/slab.h>
#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
#include <linux/interrupt.h>

#include "radio-si470x.h"
@@ -392,6 +393,17 @@ static int si470x_i2c_probe(struct i2c_client *client,
radio->videodev.release = video_device_release_empty;
video_set_drvdata(&radio->videodev, radio);

+ radio->gpio_reset = devm_gpiod_get_optional(&client->dev, "reset",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(radio->gpio_reset)) {
+ retval = PTR_ERR(radio->gpio_reset);
+ dev_err(&client->dev, "Failed to request gpio: %d\n", retval);
+ goto err_all;
+ }
+
+ if (radio->gpio_reset)
+ gpiod_set_value(radio->gpio_reset, 1);
+
/* power up : need 110ms */
radio->registers[POWERCFG] = POWERCFG_ENABLE;
if (si470x_set_register(radio, POWERCFG) < 0) {
@@ -478,6 +490,9 @@ static int si470x_i2c_remove(struct i2c_client *client)

video_unregister_device(&radio->videodev);

+ if (radio->gpio_reset)
+ gpiod_set_value(radio->gpio_reset, 0);
+
return 0;
}

diff --git a/drivers/media/radio/si470x/radio-si470x.h b/drivers/media/radio/si470x/radio-si470x.h
index 35fa0f3bbdd2..6fd6a399cb77 100644
--- a/drivers/media/radio/si470x/radio-si470x.h
+++ b/drivers/media/radio/si470x/radio-si470x.h
@@ -189,6 +189,7 @@ struct si470x_device {

#if IS_ENABLED(CONFIG_I2C_SI470X)
struct i2c_client *client;
+ struct gpio_desc *gpio_reset;
#endif
};

--
2.17.1


2018-12-05 15:50:10

by Paweł Chmiel

[permalink] [raw]
Subject: [PATCH 3/5] si470x-i2c: Use managed resource helpers

Simplify cleanup of failures by using managed resource helpers

Signed-off-by: Paweł Chmiel <[email protected]>
---
drivers/media/radio/si470x/radio-si470x-i2c.c | 29 +++++++------------
1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c b/drivers/media/radio/si470x/radio-si470x-i2c.c
index 250828ddb5fa..a7ac09c55188 100644
--- a/drivers/media/radio/si470x/radio-si470x-i2c.c
+++ b/drivers/media/radio/si470x/radio-si470x-i2c.c
@@ -350,7 +350,7 @@ static int si470x_i2c_probe(struct i2c_client *client,
unsigned char version_warning = 0;

/* private data allocation and initialization */
- radio = kzalloc(sizeof(struct si470x_device), GFP_KERNEL);
+ radio = devm_kzalloc(&client->dev, sizeof(*radio), GFP_KERNEL);
if (!radio) {
retval = -ENOMEM;
goto err_initial;
@@ -370,7 +370,7 @@ static int si470x_i2c_probe(struct i2c_client *client,
retval = v4l2_device_register(&client->dev, &radio->v4l2_dev);
if (retval < 0) {
dev_err(&client->dev, "couldn't register v4l2_device\n");
- goto err_radio;
+ goto err_initial;
}

v4l2_ctrl_handler_init(&radio->hdl, 2);
@@ -396,14 +396,14 @@ static int si470x_i2c_probe(struct i2c_client *client,
radio->registers[POWERCFG] = POWERCFG_ENABLE;
if (si470x_set_register(radio, POWERCFG) < 0) {
retval = -EIO;
- goto err_ctrl;
+ goto err_all;
}
msleep(110);

/* get device and chip versions */
if (si470x_get_all_registers(radio) < 0) {
retval = -EIO;
- goto err_ctrl;
+ goto err_all;
}
dev_info(&client->dev, "DeviceID=0x%4.4hx ChipID=0x%4.4hx\n",
radio->registers[DEVICEID], radio->registers[SI_CHIPID]);
@@ -430,10 +430,10 @@ static int si470x_i2c_probe(struct i2c_client *client,

/* rds buffer allocation */
radio->buf_size = rds_buf * 3;
- radio->buffer = kmalloc(radio->buf_size, GFP_KERNEL);
+ radio->buffer = devm_kmalloc(&client->dev, radio->buf_size, GFP_KERNEL);
if (!radio->buffer) {
retval = -EIO;
- goto err_ctrl;
+ goto err_all;
}

/* rds buffer configuration */
@@ -441,12 +441,13 @@ static int si470x_i2c_probe(struct i2c_client *client,
radio->rd_index = 0;
init_waitqueue_head(&radio->read_queue);

- retval = request_threaded_irq(client->irq, NULL, si470x_i2c_interrupt,
- IRQF_TRIGGER_FALLING | IRQF_ONESHOT, DRIVER_NAME,
- radio);
+ retval = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+ si470x_i2c_interrupt,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ DRIVER_NAME, radio);
if (retval) {
dev_err(&client->dev, "Failed to register interrupt\n");
- goto err_rds;
+ goto err_all;
}

/* register video device */
@@ -460,15 +461,9 @@ static int si470x_i2c_probe(struct i2c_client *client,

return 0;
err_all:
- free_irq(client->irq, radio);
-err_rds:
- kfree(radio->buffer);
-err_ctrl:
v4l2_ctrl_handler_free(&radio->hdl);
err_dev:
v4l2_device_unregister(&radio->v4l2_dev);
-err_radio:
- kfree(radio);
err_initial:
return retval;
}
@@ -481,9 +476,7 @@ static int si470x_i2c_remove(struct i2c_client *client)
{
struct si470x_device *radio = i2c_get_clientdata(client);

- free_irq(client->irq, radio);
video_unregister_device(&radio->videodev);
- kfree(radio);

return 0;
}
--
2.17.1


2018-12-05 15:51:00

by Paweł Chmiel

[permalink] [raw]
Subject: [PATCH 2/5] media: dt-bindings: Add binding for si470x radio

Add device tree bindings for si470x family radio receiver driver.

Signed-off-by: Paweł Chmiel <[email protected]>
---
.../devicetree/bindings/media/si470x.txt | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/si470x.txt

diff --git a/Documentation/devicetree/bindings/media/si470x.txt b/Documentation/devicetree/bindings/media/si470x.txt
new file mode 100644
index 000000000000..9294fdfd3aae
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/si470x.txt
@@ -0,0 +1,24 @@
+* Silicon Labs FM Radio receiver
+
+The Silicon Labs Si470x is family of FM radio receivers with receive power scan
+supporting 76-108 MHz, programmable through an I2C interface.
+Some of them includes an RDS encoder.
+
+Required Properties:
+- compatible: Should contain "silabs,si470x"
+- reg: the I2C address of the device
+
+Optional Properties:
+- interrupts : The interrupt number
+
+Example:
+
+&i2c2 {
+ si470x@63 {
+ compatible = "silabs,si470x";
+ reg = <0x63>;
+
+ interrupt-parent = <&gpj2>;
+ interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+ };
+};
--
2.17.1


2018-12-05 15:51:22

by Paweł Chmiel

[permalink] [raw]
Subject: [PATCH 5/5] media: dt-bindings: si470x: Document new reset-gpios property

Add information about new reset-gpios property to driver documentation

Signed-off-by: Paweł Chmiel <[email protected]>
---
Documentation/devicetree/bindings/media/si470x.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/si470x.txt b/Documentation/devicetree/bindings/media/si470x.txt
index 9294fdfd3aae..a9403558362e 100644
--- a/Documentation/devicetree/bindings/media/si470x.txt
+++ b/Documentation/devicetree/bindings/media/si470x.txt
@@ -10,6 +10,7 @@ Required Properties:

Optional Properties:
- interrupts : The interrupt number
+- reset-gpios: GPIO specifier for the chips reset line

Example:

@@ -20,5 +21,6 @@ Example:

interrupt-parent = <&gpj2>;
interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+ reset-gpios = <&gpj2 5 GPIO_ACTIVE_HIGH>;
};
};
--
2.17.1


2018-12-07 11:34:10

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH 2/5] media: dt-bindings: Add binding for si470x radio

Please combine 2/5 with 5/5. No need to have two patches for these bindings.

Regards,

Hans

On 12/05/2018 04:47 PM, Paweł Chmiel wrote:
> Add device tree bindings for si470x family radio receiver driver.
>
> Signed-off-by: Paweł Chmiel <[email protected]>
> ---
> .../devicetree/bindings/media/si470x.txt | 24 +++++++++++++++++++
> 1 file changed, 24 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/si470x.txt
>
> diff --git a/Documentation/devicetree/bindings/media/si470x.txt b/Documentation/devicetree/bindings/media/si470x.txt
> new file mode 100644
> index 000000000000..9294fdfd3aae
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/si470x.txt
> @@ -0,0 +1,24 @@
> +* Silicon Labs FM Radio receiver
> +
> +The Silicon Labs Si470x is family of FM radio receivers with receive power scan
> +supporting 76-108 MHz, programmable through an I2C interface.
> +Some of them includes an RDS encoder.
> +
> +Required Properties:
> +- compatible: Should contain "silabs,si470x"
> +- reg: the I2C address of the device
> +
> +Optional Properties:
> +- interrupts : The interrupt number
> +
> +Example:
> +
> +&i2c2 {
> + si470x@63 {
> + compatible = "silabs,si470x";
> + reg = <0x63>;
> +
> + interrupt-parent = <&gpj2>;
> + interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
> + };
> +};
>


2018-12-07 11:44:10

by Paweł Chmiel

[permalink] [raw]
Subject: Re: [PATCH 2/5] media: dt-bindings: Add binding for si470x radio

Dnia piątek, 7 grudnia 2018 12:33:10 CET Hans Verkuil pisze:
> Please combine 2/5 with 5/5. No need to have two patches for these bindings.
I though that it will be better to separate patches which just adds devicetree support
and those adding new functionality (reset), so for example if there is more work needed on one of them,
the second one can still be picked (devicetree one).

Ok will do this in next version of patchset.

>
> Regards,
>
> Hans
>
> On 12/05/2018 04:47 PM, Paweł Chmiel wrote:
> > Add device tree bindings for si470x family radio receiver driver.
> >
> > Signed-off-by: Paweł Chmiel <[email protected]>
> > ---
> > .../devicetree/bindings/media/si470x.txt | 24 +++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/si470x.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/si470x.txt b/Documentation/devicetree/bindings/media/si470x.txt
> > new file mode 100644
> > index 000000000000..9294fdfd3aae
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/si470x.txt
> > @@ -0,0 +1,24 @@
> > +* Silicon Labs FM Radio receiver
> > +
> > +The Silicon Labs Si470x is family of FM radio receivers with receive power scan
> > +supporting 76-108 MHz, programmable through an I2C interface.
> > +Some of them includes an RDS encoder.
> > +
> > +Required Properties:
> > +- compatible: Should contain "silabs,si470x"
> > +- reg: the I2C address of the device
> > +
> > +Optional Properties:
> > +- interrupts : The interrupt number
> > +
> > +Example:
> > +
> > +&i2c2 {
> > + si470x@63 {
> > + compatible = "silabs,si470x";
> > + reg = <0x63>;
> > +
> > + interrupt-parent = <&gpj2>;
> > + interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
> > + };
> > +};
> >
>
>





2018-12-19 22:12:59

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 5/5] media: dt-bindings: si470x: Document new reset-gpios property

On Wed, 5 Dec 2018 16:47:50 +0100, =?UTF-8?q?Pawe=C5=82=20Chmiel?= wrote:
> Add information about new reset-gpios property to driver documentation
>
> Signed-off-by: Paweł Chmiel <[email protected]>
> ---
> Documentation/devicetree/bindings/media/si470x.txt | 2 ++
> 1 file changed, 2 insertions(+)
>

Reviewed-by: Rob Herring <[email protected]>