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

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

[Paweł Chmiel]

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

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

[Paweł Chmiel]

This commit enables device tree support adding simple of_match table.

Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
 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

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

[Paweł Chmiel]

Add device tree bindings for si470x family radio receiver driver.

Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
 .../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

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

[Hans Verkuil]

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 <pawel.mikolaj.chmiel@gmail.com>
> ---
>  .../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>;
> +        };
> +};
> 

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

[Paweł Chmiel]

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 <pawel.mikolaj.chmiel@gmail.com>
> > ---
> >  .../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>;
> > +        };
> > +};
> > 
> 
> 

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

[Paweł Chmiel]

Simplify cleanup of failures by using managed resource helpers

Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
 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

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

[Paweł Chmiel]

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 <pawel.mikolaj.chmiel@gmail.com>
---
 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

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

[Paweł Chmiel]

Add information about new reset-gpios property to driver documentation

Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
 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

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

[Rob Herring]

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 <pawel.mikolaj.chmiel@gmail.com>
> ---
>  Documentation/devicetree/bindings/media/si470x.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>