Skip to contentSkip to conversation

ad5820: Multiple fixes

PATCH In review Ricardo Ribalda Delgado 2 participants 8 messages Oct 7, 2019 – Oct 13, 2019

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v8 cover ad5820: Multiple fixes

-Support for enable-pin, of-autoload, enable-gpios and ad5821 and ad5823

For some reason these patchset was lost in translation for a year ;)

v8: I screwed up sending v7, I sent it from a dirty directory and clicked on send-all without checking what was under v7*. Sorry :( This made patchwork very unhappy. I send v8 to make patchwork happy and hopefuly also the maintainer. Sorry again

v7: Rebase on current media/master

Ricardo Ribalda Delgado (6):
media: ad5820: Define entity function
media: ad5820: DT new optional field enable-gpios
media: ad5820: Add support for enable pin
media: ad5820: Add support for of-autoload
media: ad5820: DT new compatible devices
media: ad5820: Add support for ad5821 and ad5823

 .../devicetree/bindings/media/i2c/ad5820.txt  | 11 +++++-
 drivers/media/i2c/Kconfig                     |  2 +-
 drivers/media/i2c/ad5820.c                    | 35 ++++++++++++++++---
 3 files changed, 42 insertions(+), 6 deletions(-)
Signature · 1 line
2.23.0
#1 lore raw

PATCH v8 1/6 media: ad5820: Define entity function

Without this patch, media_device_register_entity throws a warning:

dev_warn(mdev->dev,

	 "Entity type for entity %s was not initialized!\n",
	 entity->name);
---
 drivers/media/i2c/ad5820.c | 1 +
 1 file changed, 1 insertion(+)
drivers/media/i2c/ad5820.c +1 -0
index 925c171e7797..7a49651f4d1f 100644
@@ -309,6 +309,7 @@ static int ad5820_probe(struct i2c_client *client,
 	v4l2_i2c_subdev_init(&coil->subdev, client, &ad5820_ops);
 	coil->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	coil->subdev.internal_ops = &ad5820_internal_ops;
+	coil->subdev.entity.function = MEDIA_ENT_F_LENS;
 	strscpy(coil->subdev.name, "ad5820 focus", sizeof(coil->subdev.name));
 
 	ret = media_entity_pads_init(&coil->subdev.entity, 0, NULL);
Signature · 1 line
2.23.0
#2 lore raw

PATCH v8 2/6 media: ad5820: DT new optional field enable-gpios

In reply to Ricardo Ribalda Delgado

Document new enable-gpio field. It can be used to disable the part without turning down its regulator.

---
 Documentation/devicetree/bindings/media/i2c/ad5820.txt | 6 ++++++
 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/media/i2c/ad5820.txt +6 -0
index 5940ca11c021..db596e8eb0ba 100644
@@ -8,6 +8,11 @@ Required Properties:
 
   - VANA-supply: supply of voltage for VANA pin
 
+Optional properties:
+
+   - enable-gpios : GPIO spec for the XSHUTDOWN pin. The XSHUTDOWN signal is
+active low, a high level on the pin enables the device.
+
 Example:
 
        ad5820: coil@c {
@@ -15,5 +20,6 @@ Example:
                reg = <0x0c>;
 
                VANA-supply = <&vaux4>;
+               enable-gpios = <&msmgpio 26 GPIO_ACTIVE_HIGH>;
        };
 
Signature · 1 line
2.23.0
#3 lore raw

PATCH v8 3/6 media: ad5820: Add support for enable pin

In reply to Ricardo Ribalda Delgado

This patch adds support for a programmable enable pin. It can be used in situations where the ANA-vcc is not configurable (dummy-regulator), or just to have a more fine control of the power saving.

The use of the enable pin is optional.

---
 drivers/media/i2c/Kconfig  |  2 +-
 drivers/media/i2c/ad5820.c | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)
drivers/media/i2c/Kconfig +1 -1
index 78dc64d7b0e7..30d460844dff 100644
@@ -975,7 +975,7 @@ if MEDIA_CAMERA_SUPPORT
 
 config VIDEO_AD5820
 	tristate "AD5820 lens voice coil support"
-	depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+	depends on GPIOLIB && I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
 	help
 	  This is a driver for the AD5820 camera lens voice coil.
 	  It is used for example in Nokia N900 (RX-51).
drivers/media/i2c/ad5820.c +17 -0
index 7a49651f4d1f..76aab651f217 100644
@@ -19,6 +19,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/regulator/consumer.h>
+#include <linux/gpio/consumer.h>
 
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
@@ -47,6 +48,8 @@ struct ad5820_device {
 	u32 focus_ramp_time;
 	u32 focus_ramp_mode;
 
+	struct gpio_desc *enable_gpio;
+
 	struct mutex power_lock;
 	int power_count;
 
@@ -114,6 +117,8 @@ static int ad5820_power_off(struct ad5820_device *coil, bool standby)
 		ret = ad5820_update_hw(coil);
 	}
 
+	gpiod_set_value_cansleep(coil->enable_gpio, 0);
+
 	ret2 = regulator_disable(coil->vana);
 	if (ret)
 		return ret;
@@ -128,6 +133,8 @@ static int ad5820_power_on(struct ad5820_device *coil, bool restore)
 	if (ret < 0)
 		return ret;
 
+	gpiod_set_value_cansleep(coil->enable_gpio, 1);
+
 	if (restore) {
 		/* Restore the hardware settings. */
 		coil->standby = false;
@@ -138,6 +145,7 @@ static int ad5820_power_on(struct ad5820_device *coil, bool restore)
 	return 0;
 
 fail:
+	gpiod_set_value_cansleep(coil->enable_gpio, 0);
 	coil->standby = true;
 	regulator_disable(coil->vana);
 
@@ -304,6 +312,15 @@ static int ad5820_probe(struct i2c_client *client,
 		return ret;
 	}
 
+	coil->enable_gpio = devm_gpiod_get_optional(&client->dev, "enable",
+						    GPIOD_OUT_LOW);
+	if (IS_ERR(coil->enable_gpio)) {
+		ret = PTR_ERR(coil->enable_gpio);
+		if (ret != -EPROBE_DEFER)
+			dev_err(&client->dev, "could not get enable gpio\n");
+		return ret;
+	}
+
 	mutex_init(&coil->power_lock);
 
 	v4l2_i2c_subdev_init(&coil->subdev, client, &ad5820_ops);
Signature · 1 line
2.23.0
#4 lore raw

PATCH v8 4/6 media: ad5820: Add support for of-autoload

In reply to Ricardo Ribalda Delgado

Since kernel 4.16, i2c devices with DT compatible tag are modprobed using their DT modalias.
Without this patch, if this driver is build as module it would never be autoprobed.

There is no need to mask it with CONFIG_OF to allow ACPI loading, this also builds find with CONFIG_OF=n.

---
 drivers/media/i2c/ad5820.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/media/i2c/ad5820.c +7 -0
index 76aab651f217..5651609e5095 100644
@@ -364,12 +364,19 @@ static const struct i2c_device_id ad5820_id_table[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ad5820_id_table);
 
+static const struct of_device_id ad5820_of_table[] = {
+	{ .compatible = "adi,ad5820" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad5820_of_table);
+
 static SIMPLE_DEV_PM_OPS(ad5820_pm, ad5820_suspend, ad5820_resume);
 
 static struct i2c_driver ad5820_i2c_driver = {
 	.driver		= {
 		.name	= AD5820_NAME,
 		.pm	= &ad5820_pm,
+		.of_match_table = ad5820_of_table,
 	},
 	.probe		= ad5820_probe,
 	.remove		= ad5820_remove,
Signature · 1 line
2.23.0
#5 lore raw

PATCH v8 5/6 media: ad5820: DT new compatible devices

In reply to Ricardo Ribalda Delgado

Document new compatible devices.

---
 Documentation/devicetree/bindings/media/i2c/ad5820.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/media/i2c/ad5820.txt +4 -1
index db596e8eb0ba..5764cbedf9b7 100644
@@ -2,7 +2,10 @@
 
 Required Properties:
 
-  - compatible: Must contain "adi,ad5820"
+  - compatible: Must contain one of:
+		- "adi,ad5820"
+		- "adi,ad5821"
+		- "adi,ad5823"
 
   - reg: I2C slave address
 
Signature · 1 line
2.23.0
#6 lore raw

PATCH v8 6/6 media: ad5820: Add support for ad5821 and ad5823

In reply to Ricardo Ribalda Delgado

According to the datasheet, both AD5821 and AD5820 share a compatible register-set:
http://www.analog.com/media/en/technical-documentation/data-sheets/AD5821.pdf

Some camera modules also refer that AD5823 is a replacement of AD5820:
https://download.kamami.com/p564094-OV8865_DS.pdf

---
 drivers/media/i2c/ad5820.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/media/i2c/ad5820.c +6 -4
index 5651609e5095..19c74db0649f 100644
@@ -25,8 +25,6 @@
 #include <media/v4l2-device.h>
 #include <media/v4l2-subdev.h>
 
-#define AD5820_NAME		"ad5820"
-
 /* Register definitions */
 #define AD5820_POWER_DOWN		(1 << 15)
 #define AD5820_DAC_SHIFT		4
@@ -359,13 +357,17 @@ static int ad5820_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id ad5820_id_table[] = {
-	{ AD5820_NAME, 0 },
+	{ "ad5820", 0 },
+	{ "ad5821", 0 },
+	{ "ad5823", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ad5820_id_table);
 
 static const struct of_device_id ad5820_of_table[] = {
 	{ .compatible = "adi,ad5820" },
+	{ .compatible = "adi,ad5821" },
+	{ .compatible = "adi,ad5823" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ad5820_of_table);
@@ -374,7 +376,7 @@ static SIMPLE_DEV_PM_OPS(ad5820_pm, ad5820_suspend, ad5820_resume);
 
 static struct i2c_driver ad5820_i2c_driver = {
 	.driver		= {
-		.name	= AD5820_NAME,
+		.name	= "ad5820",
 		.pm	= &ad5820_pm,
 		.of_match_table = ad5820_of_table,
 	},
Signature · 1 line
2.23.0
#7 lore raw
In reply to Ricardo Ribalda Delgado
Quoted 9 lines · Ricardo Ribalda Delgado

-Support for enable-pin, of-autoload, enable-gpios and ad5821 and ad5823

For some reason these patchset was lost in translation for a year ;)

v8: I screwed up sending v7, I sent it from a dirty directory and clicked on send-all without checking what was under v7*. Sorry :( This made patchwork very unhappy. I send v8 to make patchwork happy and hopefuly also the maintainer. Sorry again

Ok, this is a LED, but I assume it needs to go in using media tree. Good luck! :-).

Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html