* [PATCH v2 0/3] media: i2c: ak7375: Add regulator management
@ 2022-07-11 14:40 Yassine Oudjana
2022-07-11 14:40 ` [PATCH v2 1/3] media: dt-bindings: ak7375: Convert to DT schema Yassine Oudjana
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Yassine Oudjana @ 2022-07-11 14:40 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Tianshu Qiu, Bingbu Cao
Cc: Yassine Oudjana, Yassine Oudjana, Jacopo Mondi, linux-media,
devicetree, linux-kernel
From: Yassine Oudjana <y.oudjana@protonmail.com>
This series adds needed regulator management for the AK7375 VCM.
A DT schema conversion is made before adding new properties.
Changes since v1:
- Add vendor prefix to DT binding doc file name
- Reorganize variable declaration
- Change the power-on delay range to 3000-3500 microseconds.
Yassine Oudjana (3):
media: dt-bindings: ak7375: Convert to DT schema
media: dt-bindings: ak7375: Add supplies
media: i2c: ak7375: Add regulator management
.../devicetree/bindings/media/i2c/ak7375.txt | 8 ---
.../media/i2c/asahi-kasei,ak7375.yaml | 52 +++++++++++++++++++
MAINTAINERS | 2 +-
drivers/media/i2c/ak7375.c | 39 ++++++++++++++
4 files changed, 92 insertions(+), 9 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/media/i2c/ak7375.txt
create mode 100644 Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
--
2.37.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/3] media: dt-bindings: ak7375: Convert to DT schema
2022-07-11 14:40 [PATCH v2 0/3] media: i2c: ak7375: Add regulator management Yassine Oudjana
@ 2022-07-11 14:40 ` Yassine Oudjana
2022-07-12 8:17 ` Krzysztof Kozlowski
2022-11-28 6:44 ` Umang Jain
2022-07-11 14:40 ` [PATCH v2 2/3] media: dt-bindings: ak7375: Add supplies Yassine Oudjana
2022-07-11 14:40 ` [PATCH v2 3/3] media: i2c: ak7375: Add regulator management Yassine Oudjana
2 siblings, 2 replies; 17+ messages in thread
From: Yassine Oudjana @ 2022-07-11 14:40 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Tianshu Qiu, Bingbu Cao
Cc: Yassine Oudjana, Yassine Oudjana, Jacopo Mondi, linux-media,
devicetree, linux-kernel
From: Yassine Oudjana <y.oudjana@protonmail.com>
Convert DT bindings document for AKM AK7375 VCM to DT schema
format and add an example.
Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Changes since v1:
- Add vendor prefix to DT binding doc file name
.../devicetree/bindings/media/i2c/ak7375.txt | 8 ----
.../media/i2c/asahi-kasei,ak7375.yaml | 41 +++++++++++++++++++
MAINTAINERS | 2 +-
3 files changed, 42 insertions(+), 9 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/media/i2c/ak7375.txt
create mode 100644 Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
diff --git a/Documentation/devicetree/bindings/media/i2c/ak7375.txt b/Documentation/devicetree/bindings/media/i2c/ak7375.txt
deleted file mode 100644
index aa3e24b41241..000000000000
--- a/Documentation/devicetree/bindings/media/i2c/ak7375.txt
+++ /dev/null
@@ -1,8 +0,0 @@
-Asahi Kasei Microdevices AK7375 voice coil lens driver
-
-AK7375 is a camera voice coil lens.
-
-Mandatory properties:
-
-- compatible: "asahi-kasei,ak7375"
-- reg: I2C slave address
diff --git a/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml b/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
new file mode 100644
index 000000000000..22b1251b16ee
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/asahi-kasei,ak7375.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Asahi Kasei Microdevices AK7375 voice coil lens actuator
+
+maintainers:
+ - Tianshu Qiu <tian.shu.qiu@intel.com>
+
+description:
+ AK7375 is a voice coil motor (VCM) camera lens actuator that
+ is controlled over I2C.
+
+properties:
+ compatible:
+ const: asahi-kasei,ak7375
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ak7375: camera-lens@c {
+ compatible = "asahi-kasei,ak7375";
+ reg = <0x0c>;
+ };
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index a97fef8c131d..e6accf8bc12b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3063,7 +3063,7 @@ M: Tianshu Qiu <tian.shu.qiu@intel.com>
L: linux-media@vger.kernel.org
S: Maintained
T: git git://linuxtv.org/media_tree.git
-F: Documentation/devicetree/bindings/media/i2c/ak7375.txt
+F: Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
F: drivers/media/i2c/ak7375.c
ASAHI KASEI AK8974 DRIVER
--
2.37.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/3] media: dt-bindings: ak7375: Add supplies
2022-07-11 14:40 [PATCH v2 0/3] media: i2c: ak7375: Add regulator management Yassine Oudjana
2022-07-11 14:40 ` [PATCH v2 1/3] media: dt-bindings: ak7375: Convert to DT schema Yassine Oudjana
@ 2022-07-11 14:40 ` Yassine Oudjana
2022-11-28 6:45 ` Umang Jain
2022-07-11 14:40 ` [PATCH v2 3/3] media: i2c: ak7375: Add regulator management Yassine Oudjana
2 siblings, 1 reply; 17+ messages in thread
From: Yassine Oudjana @ 2022-07-11 14:40 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Tianshu Qiu, Bingbu Cao
Cc: Yassine Oudjana, Yassine Oudjana, Jacopo Mondi, linux-media,
devicetree, linux-kernel, Krzysztof Kozlowski
From: Yassine Oudjana <y.oudjana@protonmail.com>
Add supply properties to describe regulators needed to power
the AK7375 VCM.
Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
.../bindings/media/i2c/asahi-kasei,ak7375.yaml | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml b/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
index 22b1251b16ee..22a810fc7222 100644
--- a/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
@@ -20,9 +20,17 @@ properties:
reg:
maxItems: 1
+ vdd-supply:
+ description: VDD supply
+
+ vio-supply:
+ description: I/O pull-up supply
+
required:
- compatible
- reg
+ - vdd-supply
+ - vio-supply
additionalProperties: false
@@ -35,6 +43,9 @@ examples:
ak7375: camera-lens@c {
compatible = "asahi-kasei,ak7375";
reg = <0x0c>;
+
+ vdd-supply = <&vreg_l23a_2p8>;
+ vio-supply = <&vreg_lvs1a_1p8>;
};
};
--
2.37.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 3/3] media: i2c: ak7375: Add regulator management
2022-07-11 14:40 [PATCH v2 0/3] media: i2c: ak7375: Add regulator management Yassine Oudjana
2022-07-11 14:40 ` [PATCH v2 1/3] media: dt-bindings: ak7375: Convert to DT schema Yassine Oudjana
2022-07-11 14:40 ` [PATCH v2 2/3] media: dt-bindings: ak7375: Add supplies Yassine Oudjana
@ 2022-07-11 14:40 ` Yassine Oudjana
2022-07-11 17:31 ` Jacopo Mondi
` (3 more replies)
2 siblings, 4 replies; 17+ messages in thread
From: Yassine Oudjana @ 2022-07-11 14:40 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Tianshu Qiu, Bingbu Cao
Cc: Yassine Oudjana, Yassine Oudjana, Jacopo Mondi, linux-media,
devicetree, linux-kernel
From: Yassine Oudjana <y.oudjana@protonmail.com>
Make the driver get needed regulators on probe and enable/disable
them on runtime PM callbacks.
Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Changes since v1:
- Reorganize variable declaration
- Change the power-on delay range to 3000-3500 microseconds.
drivers/media/i2c/ak7375.c | 39 ++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
index 40b1a4aa846c..c2b2542a0056 100644
--- a/drivers/media/i2c/ak7375.c
+++ b/drivers/media/i2c/ak7375.c
@@ -6,6 +6,7 @@
#include <linux/i2c.h>
#include <linux/module.h>
#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
@@ -23,17 +24,32 @@
*/
#define AK7375_CTRL_STEPS 64
#define AK7375_CTRL_DELAY_US 1000
+/*
+ * The vcm takes around 3 ms to power on and start taking
+ * I2C messages. This value was found experimentally due to
+ * lack of documentation.
+ */
+#define AK7375_POWER_DELAY_US 3000
#define AK7375_REG_POSITION 0x0
#define AK7375_REG_CONT 0x2
#define AK7375_MODE_ACTIVE 0x0
#define AK7375_MODE_STANDBY 0x40
+static const char * const ak7375_supply_names[] = {
+ "vdd",
+ "vio",
+};
+
+#define AK7375_NUM_SUPPLIES ARRAY_SIZE(ak7375_supply_names)
+
/* ak7375 device structure */
struct ak7375_device {
struct v4l2_ctrl_handler ctrls_vcm;
struct v4l2_subdev sd;
struct v4l2_ctrl *focus;
+ struct regulator_bulk_data supplies[AK7375_NUM_SUPPLIES];
+
/* active or standby mode */
bool active;
};
@@ -133,12 +149,24 @@ static int ak7375_probe(struct i2c_client *client)
{
struct ak7375_device *ak7375_dev;
int ret;
+ int i;
ak7375_dev = devm_kzalloc(&client->dev, sizeof(*ak7375_dev),
GFP_KERNEL);
if (!ak7375_dev)
return -ENOMEM;
+ for (i = 0; i < AK7375_NUM_SUPPLIES; i++)
+ ak7375_dev->supplies[i].supply = ak7375_supply_names[i];
+
+ ret = devm_regulator_bulk_get(&client->dev, AK7375_NUM_SUPPLIES,
+ ak7375_dev->supplies);
+ if (ret) {
+ dev_err(&client->dev, "Failed to get regulators: %pe",
+ ERR_PTR(ret));
+ return ret;
+ }
+
v4l2_i2c_subdev_init(&ak7375_dev->sd, client, &ak7375_ops);
ak7375_dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
ak7375_dev->sd.internal_ops = &ak7375_int_ops;
@@ -210,6 +238,10 @@ static int __maybe_unused ak7375_vcm_suspend(struct device *dev)
if (ret)
dev_err(dev, "%s I2C failure: %d\n", __func__, ret);
+ ret = regulator_bulk_disable(AK7375_NUM_SUPPLIES, ak7375_dev->supplies);
+ if (ret)
+ return ret;
+
ak7375_dev->active = false;
return 0;
@@ -230,6 +262,13 @@ static int __maybe_unused ak7375_vcm_resume(struct device *dev)
if (ak7375_dev->active)
return 0;
+ ret = regulator_bulk_enable(AK7375_NUM_SUPPLIES, ak7375_dev->supplies);
+ if (ret)
+ return ret;
+
+ /* Wait for vcm to become ready */
+ usleep_range(AK7375_POWER_DELAY_US, AK7375_POWER_DELAY_US + 500);
+
ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_CONT,
AK7375_MODE_ACTIVE, 1);
if (ret) {
--
2.37.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: ak7375: Add regulator management
2022-07-11 14:40 ` [PATCH v2 3/3] media: i2c: ak7375: Add regulator management Yassine Oudjana
@ 2022-07-11 17:31 ` Jacopo Mondi
2022-07-14 13:56 ` Yassine Oudjana
2022-07-20 7:36 ` Christophe JAILLET
2022-07-13 7:39 ` Jacopo Mondi
` (2 subsequent siblings)
3 siblings, 2 replies; 17+ messages in thread
From: Jacopo Mondi @ 2022-07-11 17:31 UTC (permalink / raw)
To: Yassine Oudjana
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Tianshu Qiu, Bingbu Cao, Yassine Oudjana, linux-media,
devicetree, linux-kernel
Hi Yassine,
On Mon, Jul 11, 2022 at 06:40:39PM +0400, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Make the driver get needed regulators on probe and enable/disable
> them on runtime PM callbacks.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
> Changes since v1:
> - Reorganize variable declaration
> - Change the power-on delay range to 3000-3500 microseconds.
>
> drivers/media/i2c/ak7375.c | 39 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
> index 40b1a4aa846c..c2b2542a0056 100644
> --- a/drivers/media/i2c/ak7375.c
> +++ b/drivers/media/i2c/ak7375.c
> @@ -6,6 +6,7 @@
> #include <linux/i2c.h>
> #include <linux/module.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-device.h>
>
> @@ -23,17 +24,32 @@
> */
> #define AK7375_CTRL_STEPS 64
> #define AK7375_CTRL_DELAY_US 1000
> +/*
> + * The vcm takes around 3 ms to power on and start taking
> + * I2C messages. This value was found experimentally due to
> + * lack of documentation.
> + */
> +#define AK7375_POWER_DELAY_US 3000
>
> #define AK7375_REG_POSITION 0x0
> #define AK7375_REG_CONT 0x2
> #define AK7375_MODE_ACTIVE 0x0
> #define AK7375_MODE_STANDBY 0x40
>
> +static const char * const ak7375_supply_names[] = {
> + "vdd",
> + "vio",
> +};
> +
> +#define AK7375_NUM_SUPPLIES ARRAY_SIZE(ak7375_supply_names)
> +
> /* ak7375 device structure */
> struct ak7375_device {
> struct v4l2_ctrl_handler ctrls_vcm;
> struct v4l2_subdev sd;
> struct v4l2_ctrl *focus;
> + struct regulator_bulk_data supplies[AK7375_NUM_SUPPLIES];
> +
> /* active or standby mode */
> bool active;
> };
> @@ -133,12 +149,24 @@ static int ak7375_probe(struct i2c_client *client)
> {
> struct ak7375_device *ak7375_dev;
> int ret;
> + int i;
>
> ak7375_dev = devm_kzalloc(&client->dev, sizeof(*ak7375_dev),
> GFP_KERNEL);
> if (!ak7375_dev)
> return -ENOMEM;
>
> + for (i = 0; i < AK7375_NUM_SUPPLIES; i++)
> + ak7375_dev->supplies[i].supply = ak7375_supply_names[i];
> +
> + ret = devm_regulator_bulk_get(&client->dev, AK7375_NUM_SUPPLIES,
> + ak7375_dev->supplies);
> + if (ret) {
> + dev_err(&client->dev, "Failed to get regulators: %pe",
> + ERR_PTR(ret));
Why are you using %pe here ? Your return value is not a pointer
(Also, missing \n at the end of the string)
From Documentation/core-api/printk-formats.rst:
%pe -ENOSPC
For printing error pointers (i.e. a pointer for which IS_ERR() is true)
as a symbolic error name. Error values for which no symbolic name is
known are printed in decimal, while a non-ERR_PTR passed as the
argument to %pe gets treated as ordinary %p.
> + return ret;
> + }
> +
> v4l2_i2c_subdev_init(&ak7375_dev->sd, client, &ak7375_ops);
> ak7375_dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> ak7375_dev->sd.internal_ops = &ak7375_int_ops;
> @@ -210,6 +238,10 @@ static int __maybe_unused ak7375_vcm_suspend(struct device *dev)
> if (ret)
> dev_err(dev, "%s I2C failure: %d\n", __func__, ret);
>
> + ret = regulator_bulk_disable(AK7375_NUM_SUPPLIES, ak7375_dev->supplies);
> + if (ret)
> + return ret;
> +
> ak7375_dev->active = false;
>
> return 0;
> @@ -230,6 +262,13 @@ static int __maybe_unused ak7375_vcm_resume(struct device *dev)
> if (ak7375_dev->active)
> return 0;
>
> + ret = regulator_bulk_enable(AK7375_NUM_SUPPLIES, ak7375_dev->supplies);
> + if (ret)
> + return ret;
> +
> + /* Wait for vcm to become ready */
> + usleep_range(AK7375_POWER_DELAY_US, AK7375_POWER_DELAY_US + 500);
> +
> ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_CONT,
> AK7375_MODE_ACTIVE, 1);
> if (ret) {
> --
> 2.37.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/3] media: dt-bindings: ak7375: Convert to DT schema
2022-07-11 14:40 ` [PATCH v2 1/3] media: dt-bindings: ak7375: Convert to DT schema Yassine Oudjana
@ 2022-07-12 8:17 ` Krzysztof Kozlowski
2022-11-28 6:44 ` Umang Jain
1 sibling, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-12 8:17 UTC (permalink / raw)
To: Yassine Oudjana, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Tianshu Qiu, Bingbu Cao
Cc: Yassine Oudjana, Jacopo Mondi, linux-media, devicetree, linux-kernel
On 11/07/2022 16:40, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Convert DT bindings document for AKM AK7375 VCM to DT schema
> format and add an example.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: ak7375: Add regulator management
2022-07-11 14:40 ` [PATCH v2 3/3] media: i2c: ak7375: Add regulator management Yassine Oudjana
2022-07-11 17:31 ` Jacopo Mondi
@ 2022-07-13 7:39 ` Jacopo Mondi
2022-07-14 14:06 ` Yassine Oudjana
2022-07-23 11:33 ` Sakari Ailus
2022-11-28 6:47 ` Umang Jain
3 siblings, 1 reply; 17+ messages in thread
From: Jacopo Mondi @ 2022-07-13 7:39 UTC (permalink / raw)
To: Yassine Oudjana
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Tianshu Qiu, Bingbu Cao, Yassine Oudjana, linux-media,
devicetree, linux-kernel
Hi Yassine
On Mon, Jul 11, 2022 at 06:40:39PM +0400, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Make the driver get needed regulators on probe and enable/disable
> them on runtime PM callbacks.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
Have you seen this ?
https://github.com/ArduCAM/IMX519_AK7375/blob/main/AK7375/0002-media-i2c-ak7375-driver-add-optional-regulator-suppo.patch#L172
It claims
+ * Initialisation delay between VDD low->high and the moment
+ * when the i2c command is available.
+ * From the datasheet, it should be 10ms + 2ms (max power
+ * up sequence duration)
10ms seems like a long time, it would be nice to have the datasheet to
cross-check.
Thanks
j
> ---
> Changes since v1:
> - Reorganize variable declaration
> - Change the power-on delay range to 3000-3500 microseconds.
>
> drivers/media/i2c/ak7375.c | 39 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
> index 40b1a4aa846c..c2b2542a0056 100644
> --- a/drivers/media/i2c/ak7375.c
> +++ b/drivers/media/i2c/ak7375.c
> @@ -6,6 +6,7 @@
> #include <linux/i2c.h>
> #include <linux/module.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-device.h>
>
> @@ -23,17 +24,32 @@
> */
> #define AK7375_CTRL_STEPS 64
> #define AK7375_CTRL_DELAY_US 1000
> +/*
> + * The vcm takes around 3 ms to power on and start taking
> + * I2C messages. This value was found experimentally due to
> + * lack of documentation.
> + */
> +#define AK7375_POWER_DELAY_US 3000
>
> #define AK7375_REG_POSITION 0x0
> #define AK7375_REG_CONT 0x2
> #define AK7375_MODE_ACTIVE 0x0
> #define AK7375_MODE_STANDBY 0x40
>
> +static const char * const ak7375_supply_names[] = {
> + "vdd",
> + "vio",
> +};
> +
> +#define AK7375_NUM_SUPPLIES ARRAY_SIZE(ak7375_supply_names)
> +
> /* ak7375 device structure */
> struct ak7375_device {
> struct v4l2_ctrl_handler ctrls_vcm;
> struct v4l2_subdev sd;
> struct v4l2_ctrl *focus;
> + struct regulator_bulk_data supplies[AK7375_NUM_SUPPLIES];
> +
> /* active or standby mode */
> bool active;
> };
> @@ -133,12 +149,24 @@ static int ak7375_probe(struct i2c_client *client)
> {
> struct ak7375_device *ak7375_dev;
> int ret;
> + int i;
>
> ak7375_dev = devm_kzalloc(&client->dev, sizeof(*ak7375_dev),
> GFP_KERNEL);
> if (!ak7375_dev)
> return -ENOMEM;
>
> + for (i = 0; i < AK7375_NUM_SUPPLIES; i++)
> + ak7375_dev->supplies[i].supply = ak7375_supply_names[i];
> +
> + ret = devm_regulator_bulk_get(&client->dev, AK7375_NUM_SUPPLIES,
> + ak7375_dev->supplies);
> + if (ret) {
> + dev_err(&client->dev, "Failed to get regulators: %pe",
> + ERR_PTR(ret));
> + return ret;
> + }
> +
> v4l2_i2c_subdev_init(&ak7375_dev->sd, client, &ak7375_ops);
> ak7375_dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> ak7375_dev->sd.internal_ops = &ak7375_int_ops;
> @@ -210,6 +238,10 @@ static int __maybe_unused ak7375_vcm_suspend(struct device *dev)
> if (ret)
> dev_err(dev, "%s I2C failure: %d\n", __func__, ret);
>
> + ret = regulator_bulk_disable(AK7375_NUM_SUPPLIES, ak7375_dev->supplies);
> + if (ret)
> + return ret;
> +
> ak7375_dev->active = false;
>
> return 0;
> @@ -230,6 +262,13 @@ static int __maybe_unused ak7375_vcm_resume(struct device *dev)
> if (ak7375_dev->active)
> return 0;
>
> + ret = regulator_bulk_enable(AK7375_NUM_SUPPLIES, ak7375_dev->supplies);
> + if (ret)
> + return ret;
> +
> + /* Wait for vcm to become ready */
> + usleep_range(AK7375_POWER_DELAY_US, AK7375_POWER_DELAY_US + 500);
> +
> ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_CONT,
> AK7375_MODE_ACTIVE, 1);
> if (ret) {
> --
> 2.37.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: ak7375: Add regulator management
2022-07-11 17:31 ` Jacopo Mondi
@ 2022-07-14 13:56 ` Yassine Oudjana
2022-07-14 15:56 ` Jacopo Mondi
2022-07-20 7:36 ` Christophe JAILLET
1 sibling, 1 reply; 17+ messages in thread
From: Yassine Oudjana @ 2022-07-14 13:56 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Tianshu Qiu, Bingbu Cao, Yassine Oudjana, linux-media,
devicetree, linux-kernel
On Mon, Jul 11 2022 at 19:31:23 +0200, Jacopo Mondi <jacopo@jmondi.org>
wrote:
> Hi Yassine,
>
> On Mon, Jul 11, 2022 at 06:40:39PM +0400, Yassine Oudjana wrote:
>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>>
>> Make the driver get needed regulators on probe and enable/disable
>> them on runtime PM callbacks.
>>
>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>> ---
>> Changes since v1:
>> - Reorganize variable declaration
>> - Change the power-on delay range to 3000-3500 microseconds.
>>
>> drivers/media/i2c/ak7375.c | 39
>> ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
>> index 40b1a4aa846c..c2b2542a0056 100644
>> --- a/drivers/media/i2c/ak7375.c
>> +++ b/drivers/media/i2c/ak7375.c
>> @@ -6,6 +6,7 @@
>> #include <linux/i2c.h>
>> #include <linux/module.h>
>> #include <linux/pm_runtime.h>
>> +#include <linux/regulator/consumer.h>
>> #include <media/v4l2-ctrls.h>
>> #include <media/v4l2-device.h>
>>
>> @@ -23,17 +24,32 @@
>> */
>> #define AK7375_CTRL_STEPS 64
>> #define AK7375_CTRL_DELAY_US 1000
>> +/*
>> + * The vcm takes around 3 ms to power on and start taking
>> + * I2C messages. This value was found experimentally due to
>> + * lack of documentation.
>> + */
>> +#define AK7375_POWER_DELAY_US 3000
>>
>> #define AK7375_REG_POSITION 0x0
>> #define AK7375_REG_CONT 0x2
>> #define AK7375_MODE_ACTIVE 0x0
>> #define AK7375_MODE_STANDBY 0x40
>>
>> +static const char * const ak7375_supply_names[] = {
>> + "vdd",
>> + "vio",
>> +};
>> +
>> +#define AK7375_NUM_SUPPLIES ARRAY_SIZE(ak7375_supply_names)
>> +
>> /* ak7375 device structure */
>> struct ak7375_device {
>> struct v4l2_ctrl_handler ctrls_vcm;
>> struct v4l2_subdev sd;
>> struct v4l2_ctrl *focus;
>> + struct regulator_bulk_data supplies[AK7375_NUM_SUPPLIES];
>> +
>> /* active or standby mode */
>> bool active;
>> };
>> @@ -133,12 +149,24 @@ static int ak7375_probe(struct i2c_client
>> *client)
>> {
>> struct ak7375_device *ak7375_dev;
>> int ret;
>> + int i;
>>
>> ak7375_dev = devm_kzalloc(&client->dev, sizeof(*ak7375_dev),
>> GFP_KERNEL);
>> if (!ak7375_dev)
>> return -ENOMEM;
>>
>> + for (i = 0; i < AK7375_NUM_SUPPLIES; i++)
>> + ak7375_dev->supplies[i].supply = ak7375_supply_names[i];
>> +
>> + ret = devm_regulator_bulk_get(&client->dev, AK7375_NUM_SUPPLIES,
>> + ak7375_dev->supplies);
>> + if (ret) {
>> + dev_err(&client->dev, "Failed to get regulators: %pe",
>> + ERR_PTR(ret));
>
> Why are you using %pe here ? Your return value is not a pointer
In order to have it print a symbolic error name instead of a value
with CONFIG_SYMBOLIC_ERRNAME=y. There is no format code for an
error integer (or at least I couldn't find one mentioned anywhere
in the docs), so instead I use %pe then wrap `ret` in ERR_PTR().
> (Also, missing \n at the end of the string)
That wasn't intentional. I'll fix it.
>
> From Documentation/core-api/printk-formats.rst:
> %pe -ENOSPC
>
> For printing error pointers (i.e. a pointer for which IS_ERR() is
> true)
> as a symbolic error name. Error values for which no symbolic name is
> known are printed in decimal, while a non-ERR_PTR passed as the
> argument to %pe gets treated as ordinary %p.
>
>> + return ret;
>> + }
>> +
>> v4l2_i2c_subdev_init(&ak7375_dev->sd, client, &ak7375_ops);
>> ak7375_dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> ak7375_dev->sd.internal_ops = &ak7375_int_ops;
>> @@ -210,6 +238,10 @@ static int __maybe_unused
>> ak7375_vcm_suspend(struct device *dev)
>> if (ret)
>> dev_err(dev, "%s I2C failure: %d\n", __func__, ret);
>>
>> + ret = regulator_bulk_disable(AK7375_NUM_SUPPLIES,
>> ak7375_dev->supplies);
>> + if (ret)
>> + return ret;
>> +
>> ak7375_dev->active = false;
>>
>> return 0;
>> @@ -230,6 +262,13 @@ static int __maybe_unused
>> ak7375_vcm_resume(struct device *dev)
>> if (ak7375_dev->active)
>> return 0;
>>
>> + ret = regulator_bulk_enable(AK7375_NUM_SUPPLIES,
>> ak7375_dev->supplies);
>> + if (ret)
>> + return ret;
>> +
>> + /* Wait for vcm to become ready */
>> + usleep_range(AK7375_POWER_DELAY_US, AK7375_POWER_DELAY_US + 500);
>> +
>> ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_CONT,
>> AK7375_MODE_ACTIVE, 1);
>> if (ret) {
>> --
>> 2.37.0
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: ak7375: Add regulator management
2022-07-13 7:39 ` Jacopo Mondi
@ 2022-07-14 14:06 ` Yassine Oudjana
2022-07-14 15:58 ` Jacopo Mondi
0 siblings, 1 reply; 17+ messages in thread
From: Yassine Oudjana @ 2022-07-14 14:06 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Lee Jackson, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Tianshu Qiu, Bingbu Cao, Yassine Oudjana,
linux-media, devicetree, linux-kernel
On Wed, Jul 13 2022 at 09:39:51 +0200, Jacopo Mondi <jacopo@jmondi.org>
wrote:
> Hi Yassine
>
> On Mon, Jul 11, 2022 at 06:40:39PM +0400, Yassine Oudjana wrote:
>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>>
>> Make the driver get needed regulators on probe and enable/disable
>> them on runtime PM callbacks.
>>
>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Have you seen this ?
> https://github.com/ArduCAM/IMX519_AK7375/blob/main/AK7375/0002-media-i2c-ak7375-driver-add-optional-regulator-suppo.patch#L172
>
> It claims
> + * Initialisation delay between VDD low->high and the moment
> + * when the i2c command is available.
> + * From the datasheet, it should be 10ms + 2ms (max power
> + * up sequence duration)
>
> 10ms seems like a long time, it would be nice to have the datasheet to
> cross-check.
It does seem quite long. I couldn't find a datasheet anywhere
so the value I discovered is the best I have. I've added the
author of that patch to CC; maybe they have some info to
contribute.
>
> Thanks
> j
>
>> ---
>> Changes since v1:
>> - Reorganize variable declaration
>> - Change the power-on delay range to 3000-3500 microseconds.
>>
>> drivers/media/i2c/ak7375.c | 39
>> ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
>> index 40b1a4aa846c..c2b2542a0056 100644
>> --- a/drivers/media/i2c/ak7375.c
>> +++ b/drivers/media/i2c/ak7375.c
>> @@ -6,6 +6,7 @@
>> #include <linux/i2c.h>
>> #include <linux/module.h>
>> #include <linux/pm_runtime.h>
>> +#include <linux/regulator/consumer.h>
>> #include <media/v4l2-ctrls.h>
>> #include <media/v4l2-device.h>
>>
>> @@ -23,17 +24,32 @@
>> */
>> #define AK7375_CTRL_STEPS 64
>> #define AK7375_CTRL_DELAY_US 1000
>> +/*
>> + * The vcm takes around 3 ms to power on and start taking
>> + * I2C messages. This value was found experimentally due to
>> + * lack of documentation.
>> + */
>> +#define AK7375_POWER_DELAY_US 3000
>>
>> #define AK7375_REG_POSITION 0x0
>> #define AK7375_REG_CONT 0x2
>> #define AK7375_MODE_ACTIVE 0x0
>> #define AK7375_MODE_STANDBY 0x40
>>
>> +static const char * const ak7375_supply_names[] = {
>> + "vdd",
>> + "vio",
>> +};
>> +
>> +#define AK7375_NUM_SUPPLIES ARRAY_SIZE(ak7375_supply_names)
>> +
>> /* ak7375 device structure */
>> struct ak7375_device {
>> struct v4l2_ctrl_handler ctrls_vcm;
>> struct v4l2_subdev sd;
>> struct v4l2_ctrl *focus;
>> + struct regulator_bulk_data supplies[AK7375_NUM_SUPPLIES];
>> +
>> /* active or standby mode */
>> bool active;
>> };
>> @@ -133,12 +149,24 @@ static int ak7375_probe(struct i2c_client
>> *client)
>> {
>> struct ak7375_device *ak7375_dev;
>> int ret;
>> + int i;
>>
>> ak7375_dev = devm_kzalloc(&client->dev, sizeof(*ak7375_dev),
>> GFP_KERNEL);
>> if (!ak7375_dev)
>> return -ENOMEM;
>>
>> + for (i = 0; i < AK7375_NUM_SUPPLIES; i++)
>> + ak7375_dev->supplies[i].supply = ak7375_supply_names[i];
>> +
>> + ret = devm_regulator_bulk_get(&client->dev, AK7375_NUM_SUPPLIES,
>> + ak7375_dev->supplies);
>> + if (ret) {
>> + dev_err(&client->dev, "Failed to get regulators: %pe",
>> + ERR_PTR(ret));
>> + return ret;
>> + }
>> +
>> v4l2_i2c_subdev_init(&ak7375_dev->sd, client, &ak7375_ops);
>> ak7375_dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> ak7375_dev->sd.internal_ops = &ak7375_int_ops;
>> @@ -210,6 +238,10 @@ static int __maybe_unused
>> ak7375_vcm_suspend(struct device *dev)
>> if (ret)
>> dev_err(dev, "%s I2C failure: %d\n", __func__, ret);
>>
>> + ret = regulator_bulk_disable(AK7375_NUM_SUPPLIES,
>> ak7375_dev->supplies);
>> + if (ret)
>> + return ret;
>> +
>> ak7375_dev->active = false;
>>
>> return 0;
>> @@ -230,6 +262,13 @@ static int __maybe_unused
>> ak7375_vcm_resume(struct device *dev)
>> if (ak7375_dev->active)
>> return 0;
>>
>> + ret = regulator_bulk_enable(AK7375_NUM_SUPPLIES,
>> ak7375_dev->supplies);
>> + if (ret)
>> + return ret;
>> +
>> + /* Wait for vcm to become ready */
>> + usleep_range(AK7375_POWER_DELAY_US, AK7375_POWER_DELAY_US + 500);
>> +
>> ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_CONT,
>> AK7375_MODE_ACTIVE, 1);
>> if (ret) {
>> --
>> 2.37.0
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: ak7375: Add regulator management
2022-07-14 13:56 ` Yassine Oudjana
@ 2022-07-14 15:56 ` Jacopo Mondi
0 siblings, 0 replies; 17+ messages in thread
From: Jacopo Mondi @ 2022-07-14 15:56 UTC (permalink / raw)
To: Yassine Oudjana
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Tianshu Qiu, Bingbu Cao, Yassine Oudjana, linux-media,
devicetree, linux-kernel
Hi Yassine,
On Thu, Jul 14, 2022 at 05:56:29PM +0400, Yassine Oudjana wrote:
>
> On Mon, Jul 11 2022 at 19:31:23 +0200, Jacopo Mondi <jacopo@jmondi.org>
> wrote:
> > Hi Yassine,
> >
> > On Mon, Jul 11, 2022 at 06:40:39PM +0400, Yassine Oudjana wrote:
> > > From: Yassine Oudjana <y.oudjana@protonmail.com>
> > >
> > > Make the driver get needed regulators on probe and enable/disable
> > > them on runtime PM callbacks.
> > >
> > > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> > > ---
> > > Changes since v1:
> > > - Reorganize variable declaration
> > > - Change the power-on delay range to 3000-3500 microseconds.
> > >
> > > drivers/media/i2c/ak7375.c | 39
> > > ++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 39 insertions(+)
> > >
> > > diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
> > > index 40b1a4aa846c..c2b2542a0056 100644
> > > --- a/drivers/media/i2c/ak7375.c
> > > +++ b/drivers/media/i2c/ak7375.c
> > > @@ -6,6 +6,7 @@
> > > #include <linux/i2c.h>
> > > #include <linux/module.h>
> > > #include <linux/pm_runtime.h>
> > > +#include <linux/regulator/consumer.h>
> > > #include <media/v4l2-ctrls.h>
> > > #include <media/v4l2-device.h>
> > >
> > > @@ -23,17 +24,32 @@
> > > */
> > > #define AK7375_CTRL_STEPS 64
> > > #define AK7375_CTRL_DELAY_US 1000
> > > +/*
> > > + * The vcm takes around 3 ms to power on and start taking
> > > + * I2C messages. This value was found experimentally due to
> > > + * lack of documentation.
> > > + */
> > > +#define AK7375_POWER_DELAY_US 3000
> > >
> > > #define AK7375_REG_POSITION 0x0
> > > #define AK7375_REG_CONT 0x2
> > > #define AK7375_MODE_ACTIVE 0x0
> > > #define AK7375_MODE_STANDBY 0x40
> > >
> > > +static const char * const ak7375_supply_names[] = {
> > > + "vdd",
> > > + "vio",
> > > +};
> > > +
> > > +#define AK7375_NUM_SUPPLIES ARRAY_SIZE(ak7375_supply_names)
> > > +
> > > /* ak7375 device structure */
> > > struct ak7375_device {
> > > struct v4l2_ctrl_handler ctrls_vcm;
> > > struct v4l2_subdev sd;
> > > struct v4l2_ctrl *focus;
> > > + struct regulator_bulk_data supplies[AK7375_NUM_SUPPLIES];
> > > +
> > > /* active or standby mode */
> > > bool active;
> > > };
> > > @@ -133,12 +149,24 @@ static int ak7375_probe(struct i2c_client
> > > *client)
> > > {
> > > struct ak7375_device *ak7375_dev;
> > > int ret;
> > > + int i;
> > >
> > > ak7375_dev = devm_kzalloc(&client->dev, sizeof(*ak7375_dev),
> > > GFP_KERNEL);
> > > if (!ak7375_dev)
> > > return -ENOMEM;
> > >
> > > + for (i = 0; i < AK7375_NUM_SUPPLIES; i++)
> > > + ak7375_dev->supplies[i].supply = ak7375_supply_names[i];
> > > +
> > > + ret = devm_regulator_bulk_get(&client->dev, AK7375_NUM_SUPPLIES,
> > > + ak7375_dev->supplies);
> > > + if (ret) {
> > > + dev_err(&client->dev, "Failed to get regulators: %pe",
> > > + ERR_PTR(ret));
> >
> > Why are you using %pe here ? Your return value is not a pointer
>
> In order to have it print a symbolic error name instead of a value
> with CONFIG_SYMBOLIC_ERRNAME=y. There is no format code for an
> error integer (or at least I couldn't find one mentioned anywhere
> in the docs), so instead I use %pe then wrap `ret` in ERR_PTR().
>
Ah nice, sorry I didn't realize. I grepped around in drivers/media and
saw it only used with pointer values, but I actually missed two
drivers that wrap ret in ERR_PTR() like you're doing here.
> > (Also, missing \n at the end of the string)
>
> That wasn't intentional. I'll fix it.
>
That's indeed minor.
Let me reply to the other email about delays..
> >
> > From Documentation/core-api/printk-formats.rst:
> > %pe -ENOSPC
> >
> > For printing error pointers (i.e. a pointer for which IS_ERR() is true)
> > as a symbolic error name. Error values for which no symbolic name is
> > known are printed in decimal, while a non-ERR_PTR passed as the
> > argument to %pe gets treated as ordinary %p.
> >
> > > + return ret;
> > > + }
> > > +
> > > v4l2_i2c_subdev_init(&ak7375_dev->sd, client, &ak7375_ops);
> > > ak7375_dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > ak7375_dev->sd.internal_ops = &ak7375_int_ops;
> > > @@ -210,6 +238,10 @@ static int __maybe_unused
> > > ak7375_vcm_suspend(struct device *dev)
> > > if (ret)
> > > dev_err(dev, "%s I2C failure: %d\n", __func__, ret);
> > >
> > > + ret = regulator_bulk_disable(AK7375_NUM_SUPPLIES,
> > > ak7375_dev->supplies);
> > > + if (ret)
> > > + return ret;
> > > +
> > > ak7375_dev->active = false;
> > >
> > > return 0;
> > > @@ -230,6 +262,13 @@ static int __maybe_unused
> > > ak7375_vcm_resume(struct device *dev)
> > > if (ak7375_dev->active)
> > > return 0;
> > >
> > > + ret = regulator_bulk_enable(AK7375_NUM_SUPPLIES,
> > > ak7375_dev->supplies);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + /* Wait for vcm to become ready */
> > > + usleep_range(AK7375_POWER_DELAY_US, AK7375_POWER_DELAY_US + 500);
> > > +
> > > ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_CONT,
> > > AK7375_MODE_ACTIVE, 1);
> > > if (ret) {
> > > --
> > > 2.37.0
> > >
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: ak7375: Add regulator management
2022-07-14 14:06 ` Yassine Oudjana
@ 2022-07-14 15:58 ` Jacopo Mondi
2022-07-20 2:58 ` Lee Jackson
0 siblings, 1 reply; 17+ messages in thread
From: Jacopo Mondi @ 2022-07-14 15:58 UTC (permalink / raw)
To: Yassine Oudjana
Cc: Lee Jackson, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Tianshu Qiu, Bingbu Cao, Yassine Oudjana,
linux-media, devicetree, linux-kernel
Hello Yassine
On Thu, Jul 14, 2022 at 06:06:32PM +0400, Yassine Oudjana wrote:
>
> On Wed, Jul 13 2022 at 09:39:51 +0200, Jacopo Mondi <jacopo@jmondi.org>
> wrote:
> > Hi Yassine
> >
> > On Mon, Jul 11, 2022 at 06:40:39PM +0400, Yassine Oudjana wrote:
> > > From: Yassine Oudjana <y.oudjana@protonmail.com>
> > >
> > > Make the driver get needed regulators on probe and enable/disable
> > > them on runtime PM callbacks.
> > >
> > > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> >
> > Have you seen this ?
> > https://github.com/ArduCAM/IMX519_AK7375/blob/main/AK7375/0002-media-i2c-ak7375-driver-add-optional-regulator-suppo.patch#L172
> >
> > It claims
> > + * Initialisation delay between VDD low->high and the moment
> > + * when the i2c command is available.
> > + * From the datasheet, it should be 10ms + 2ms (max power
> > + * up sequence duration)
> >
> > 10ms seems like a long time, it would be nice to have the datasheet to
> > cross-check.
>
> It does seem quite long. I couldn't find a datasheet anywhere
> so the value I discovered is the best I have. I've added the
> author of that patch to CC; maybe they have some info to
> contribute.
>
I have now tested these patches with an Arducam IMX519 camera.
Using a 3msec delay I get failures in the establishing i2c
communications (I only tested 2 times though).
With 10milliseconds (which I concur is a lot) I get stable results.
Let's see if we can get more info from who has the manual.
Thanks
j
> >
> > Thanks
> > j
> >
> > > ---
> > > Changes since v1:
> > > - Reorganize variable declaration
> > > - Change the power-on delay range to 3000-3500 microseconds.
> > >
> > > drivers/media/i2c/ak7375.c | 39
> > > ++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 39 insertions(+)
> > >
> > > diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
> > > index 40b1a4aa846c..c2b2542a0056 100644
> > > --- a/drivers/media/i2c/ak7375.c
> > > +++ b/drivers/media/i2c/ak7375.c
> > > @@ -6,6 +6,7 @@
> > > #include <linux/i2c.h>
> > > #include <linux/module.h>
> > > #include <linux/pm_runtime.h>
> > > +#include <linux/regulator/consumer.h>
> > > #include <media/v4l2-ctrls.h>
> > > #include <media/v4l2-device.h>
> > >
> > > @@ -23,17 +24,32 @@
> > > */
> > > #define AK7375_CTRL_STEPS 64
> > > #define AK7375_CTRL_DELAY_US 1000
> > > +/*
> > > + * The vcm takes around 3 ms to power on and start taking
> > > + * I2C messages. This value was found experimentally due to
> > > + * lack of documentation.
> > > + */
> > > +#define AK7375_POWER_DELAY_US 3000
> > >
> > > #define AK7375_REG_POSITION 0x0
> > > #define AK7375_REG_CONT 0x2
> > > #define AK7375_MODE_ACTIVE 0x0
> > > #define AK7375_MODE_STANDBY 0x40
> > >
> > > +static const char * const ak7375_supply_names[] = {
> > > + "vdd",
> > > + "vio",
> > > +};
> > > +
> > > +#define AK7375_NUM_SUPPLIES ARRAY_SIZE(ak7375_supply_names)
> > > +
> > > /* ak7375 device structure */
> > > struct ak7375_device {
> > > struct v4l2_ctrl_handler ctrls_vcm;
> > > struct v4l2_subdev sd;
> > > struct v4l2_ctrl *focus;
> > > + struct regulator_bulk_data supplies[AK7375_NUM_SUPPLIES];
> > > +
> > > /* active or standby mode */
> > > bool active;
> > > };
> > > @@ -133,12 +149,24 @@ static int ak7375_probe(struct i2c_client
> > > *client)
> > > {
> > > struct ak7375_device *ak7375_dev;
> > > int ret;
> > > + int i;
> > >
> > > ak7375_dev = devm_kzalloc(&client->dev, sizeof(*ak7375_dev),
> > > GFP_KERNEL);
> > > if (!ak7375_dev)
> > > return -ENOMEM;
> > >
> > > + for (i = 0; i < AK7375_NUM_SUPPLIES; i++)
> > > + ak7375_dev->supplies[i].supply = ak7375_supply_names[i];
> > > +
> > > + ret = devm_regulator_bulk_get(&client->dev, AK7375_NUM_SUPPLIES,
> > > + ak7375_dev->supplies);
> > > + if (ret) {
> > > + dev_err(&client->dev, "Failed to get regulators: %pe",
> > > + ERR_PTR(ret));
> > > + return ret;
> > > + }
> > > +
> > > v4l2_i2c_subdev_init(&ak7375_dev->sd, client, &ak7375_ops);
> > > ak7375_dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > ak7375_dev->sd.internal_ops = &ak7375_int_ops;
> > > @@ -210,6 +238,10 @@ static int __maybe_unused
> > > ak7375_vcm_suspend(struct device *dev)
> > > if (ret)
> > > dev_err(dev, "%s I2C failure: %d\n", __func__, ret);
> > >
> > > + ret = regulator_bulk_disable(AK7375_NUM_SUPPLIES,
> > > ak7375_dev->supplies);
> > > + if (ret)
> > > + return ret;
> > > +
> > > ak7375_dev->active = false;
> > >
> > > return 0;
> > > @@ -230,6 +262,13 @@ static int __maybe_unused
> > > ak7375_vcm_resume(struct device *dev)
> > > if (ak7375_dev->active)
> > > return 0;
> > >
> > > + ret = regulator_bulk_enable(AK7375_NUM_SUPPLIES,
> > > ak7375_dev->supplies);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + /* Wait for vcm to become ready */
> > > + usleep_range(AK7375_POWER_DELAY_US, AK7375_POWER_DELAY_US + 500);
> > > +
> > > ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_CONT,
> > > AK7375_MODE_ACTIVE, 1);
> > > if (ret) {
> > > --
> > > 2.37.0
> > >
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: ak7375: Add regulator management
2022-07-14 15:58 ` Jacopo Mondi
@ 2022-07-20 2:58 ` Lee Jackson
0 siblings, 0 replies; 17+ messages in thread
From: Lee Jackson @ 2022-07-20 2:58 UTC (permalink / raw)
To: jacopo
Cc: bingbu.cao, devicetree, info, krzysztof.kozlowski+dt,
linux-kernel, linux-media, mchehab, robh+dt, tian.shu.qiu,
y.oudjana, yassine.oudjana
Hello All,
This is Lee Jackson from Arducam, please check the reply following.
On Thu, 14 Jul 2022 17:58:31 +0200
Jacopo Mondi <jacopo@jmondi.org> wrote:
> Hello Yassine
>
> On Thu, Jul 14, 2022 at 06:06:32PM +0400, Yassine Oudjana wrote:
> >
> > On Wed, Jul 13 2022 at 09:39:51 +0200, Jacopo Mondi
> > <jacopo@jmondi.org> wrote:
> > > Hi Yassine
> > >
> > > On Mon, Jul 11, 2022 at 06:40:39PM +0400, Yassine Oudjana wrote:
> > > > From: Yassine Oudjana <y.oudjana@protonmail.com>
> > > >
> > > > Make the driver get needed regulators on probe and
> > > > enable/disable them on runtime PM callbacks.
> > > >
> > > > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> > >
> > > Have you seen this ?
> > > https://github.com/ArduCAM/IMX519_AK7375/blob/main/AK7375/0002-media-i2c-ak7375-driver-add-optional-regulator-suppo.patch#L172
> > >
> > > It claims
> > > + * Initialisation delay between VDD low->high and the
> > > moment
> > > + * when the i2c command is available.
> > > + * From the datasheet, it should be 10ms + 2ms (max power
> > > + * up sequence duration)
> > >
> > > 10ms seems like a long time, it would be nice to have the
> > > datasheet to cross-check.
> >
> > It does seem quite long. I couldn't find a datasheet anywhere
> > so the value I discovered is the best I have. I've added the
> > author of that patch to CC; maybe they have some info to
> > contribute.
> >
>
> I have now tested these patches with an Arducam IMX519 camera.
> Using a 3msec delay I get failures in the establishing i2c
> communications (I only tested 2 times though).
>
> With 10milliseconds (which I concur is a lot) I get stable results.
> Let's see if we can get more info from who has the manual.
>
> Thanks
> j
>
We have the datasheet for the ak7371 and as far as we know they are not
much different, here is the power up sequence for the ak7371:
https://www.arducam.com/wp-content/uploads/2022/07/ak7371-power-sequence.png
(We're not very familiar with working on the linux kernel mailing list
yet, that's one reason we didn't submit ak7375patch, please let me know
if I'm doing something wrong.)
> > >
> > > Thanks
> > > j
> > >
> > > > ---
> > > > Changes since v1:
> > > > - Reorganize variable declaration
> > > > - Change the power-on delay range to 3000-3500 microseconds.
> > > >
> > > > drivers/media/i2c/ak7375.c | 39
> > > > ++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 39 insertions(+)
> > > >
> > > > diff --git a/drivers/media/i2c/ak7375.c
> > > > b/drivers/media/i2c/ak7375.c index 40b1a4aa846c..c2b2542a0056
> > > > 100644 --- a/drivers/media/i2c/ak7375.c
> > > > +++ b/drivers/media/i2c/ak7375.c
> > > > @@ -6,6 +6,7 @@
> > > > #include <linux/i2c.h>
> > > > #include <linux/module.h>
> > > > #include <linux/pm_runtime.h>
> > > > +#include <linux/regulator/consumer.h>
> > > > #include <media/v4l2-ctrls.h>
> > > > #include <media/v4l2-device.h>
> > > >
> > > > @@ -23,17 +24,32 @@
> > > > */
> > > > #define AK7375_CTRL_STEPS 64
> > > > #define AK7375_CTRL_DELAY_US 1000
> > > > +/*
> > > > + * The vcm takes around 3 ms to power on and start taking
> > > > + * I2C messages. This value was found experimentally due to
> > > > + * lack of documentation.
> > > > + */
> > > > +#define AK7375_POWER_DELAY_US 3000
> > > >
> > > > #define AK7375_REG_POSITION 0x0
> > > > #define AK7375_REG_CONT 0x2
> > > > #define AK7375_MODE_ACTIVE 0x0
> > > > #define AK7375_MODE_STANDBY 0x40
> > > >
> > > > +static const char * const ak7375_supply_names[] = {
> > > > + "vdd",
> > > > + "vio",
> > > > +};
> > > > +
> > > > +#define AK7375_NUM_SUPPLIES ARRAY_SIZE(ak7375_supply_names)
> > > > +
> > > > /* ak7375 device structure */
> > > > struct ak7375_device {
> > > > struct v4l2_ctrl_handler ctrls_vcm;
> > > > struct v4l2_subdev sd;
> > > > struct v4l2_ctrl *focus;
> > > > + struct regulator_bulk_data
> > > > supplies[AK7375_NUM_SUPPLIES]; +
> > > > /* active or standby mode */
> > > > bool active;
> > > > };
> > > > @@ -133,12 +149,24 @@ static int ak7375_probe(struct i2c_client
> > > > *client)
> > > > {
> > > > struct ak7375_device *ak7375_dev;
> > > > int ret;
> > > > + int i;
> > > >
> > > > ak7375_dev = devm_kzalloc(&client->dev,
> > > > sizeof(*ak7375_dev), GFP_KERNEL);
> > > > if (!ak7375_dev)
> > > > return -ENOMEM;
> > > >
> > > > + for (i = 0; i < AK7375_NUM_SUPPLIES; i++)
> > > > + ak7375_dev->supplies[i].supply =
> > > > ak7375_supply_names[i]; +
> > > > + ret = devm_regulator_bulk_get(&client->dev,
> > > > AK7375_NUM_SUPPLIES,
> > > > + ak7375_dev->supplies);
> > > > + if (ret) {
> > > > + dev_err(&client->dev, "Failed to get
> > > > regulators: %pe",
> > > > + ERR_PTR(ret));
> > > > + return ret;
> > > > + }
> > > > +
> > > > v4l2_i2c_subdev_init(&ak7375_dev->sd, client,
> > > > &ak7375_ops); ak7375_dev->sd.flags |=
> > > > V4L2_SUBDEV_FL_HAS_DEVNODE; ak7375_dev->sd.internal_ops =
> > > > &ak7375_int_ops; @@ -210,6 +238,10 @@ static int __maybe_unused
> > > > ak7375_vcm_suspend(struct device *dev)
> > > > if (ret)
> > > > dev_err(dev, "%s I2C failure: %d\n",
> > > > __func__, ret);
> > > >
> > > > + ret = regulator_bulk_disable(AK7375_NUM_SUPPLIES,
> > > > ak7375_dev->supplies);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > ak7375_dev->active = false;
> > > >
> > > > return 0;
> > > > @@ -230,6 +262,13 @@ static int __maybe_unused
> > > > ak7375_vcm_resume(struct device *dev)
> > > > if (ak7375_dev->active)
> > > > return 0;
> > > >
> > > > + ret = regulator_bulk_enable(AK7375_NUM_SUPPLIES,
> > > > ak7375_dev->supplies);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + /* Wait for vcm to become ready */
> > > > + usleep_range(AK7375_POWER_DELAY_US,
> > > > AK7375_POWER_DELAY_US + 500); +
> > > > ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_CONT,
> > > > AK7375_MODE_ACTIVE, 1);
> > > > if (ret) {
> > > > --
> > > > 2.37.0
> > > >
> >
> >
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: ak7375: Add regulator management
2022-07-11 17:31 ` Jacopo Mondi
2022-07-14 13:56 ` Yassine Oudjana
@ 2022-07-20 7:36 ` Christophe JAILLET
1 sibling, 0 replies; 17+ messages in thread
From: Christophe JAILLET @ 2022-07-20 7:36 UTC (permalink / raw)
To: Jacopo Mondi, Yassine Oudjana
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Tianshu Qiu, Bingbu Cao, Yassine Oudjana, linux-media,
devicetree, linux-kernel
Le 11/07/2022 à 19:31, Jacopo Mondi a écrit :
> Hi Yassine,
>
> On Mon, Jul 11, 2022 at 06:40:39PM +0400, Yassine Oudjana wrote:
>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>>
>> Make the driver get needed regulators on probe and enable/disable
>> them on runtime PM callbacks.
>>
>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>> ---
>> Changes since v1:
>> - Reorganize variable declaration
>> - Change the power-on delay range to 3000-3500 microseconds.
>>
>> drivers/media/i2c/ak7375.c | 39 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
>> index 40b1a4aa846c..c2b2542a0056 100644
>> --- a/drivers/media/i2c/ak7375.c
>> +++ b/drivers/media/i2c/ak7375.c
>> @@ -6,6 +6,7 @@
>> #include <linux/i2c.h>
>> #include <linux/module.h>
>> #include <linux/pm_runtime.h>
>> +#include <linux/regulator/consumer.h>
>> #include <media/v4l2-ctrls.h>
>> #include <media/v4l2-device.h>
>>
>> @@ -23,17 +24,32 @@
>> */
>> #define AK7375_CTRL_STEPS 64
>> #define AK7375_CTRL_DELAY_US 1000
>> +/*
>> + * The vcm takes around 3 ms to power on and start taking
>> + * I2C messages. This value was found experimentally due to
>> + * lack of documentation.
>> + */
>> +#define AK7375_POWER_DELAY_US 3000
>>
>> #define AK7375_REG_POSITION 0x0
>> #define AK7375_REG_CONT 0x2
>> #define AK7375_MODE_ACTIVE 0x0
>> #define AK7375_MODE_STANDBY 0x40
>>
>> +static const char * const ak7375_supply_names[] = {
>> + "vdd",
>> + "vio",
>> +};
>> +
>> +#define AK7375_NUM_SUPPLIES ARRAY_SIZE(ak7375_supply_names)
>> +
>> /* ak7375 device structure */
>> struct ak7375_device {
>> struct v4l2_ctrl_handler ctrls_vcm;
>> struct v4l2_subdev sd;
>> struct v4l2_ctrl *focus;
>> + struct regulator_bulk_data supplies[AK7375_NUM_SUPPLIES];
>> +
>> /* active or standby mode */
>> bool active;
>> };
>> @@ -133,12 +149,24 @@ static int ak7375_probe(struct i2c_client *client)
>> {
>> struct ak7375_device *ak7375_dev;
>> int ret;
>> + int i;
>>
>> ak7375_dev = devm_kzalloc(&client->dev, sizeof(*ak7375_dev),
>> GFP_KERNEL);
>> if (!ak7375_dev)
>> return -ENOMEM;
>>
>> + for (i = 0; i < AK7375_NUM_SUPPLIES; i++)
>> + ak7375_dev->supplies[i].supply = ak7375_supply_names[i];
>> +
>> + ret = devm_regulator_bulk_get(&client->dev, AK7375_NUM_SUPPLIES,
>> + ak7375_dev->supplies);
>> + if (ret) {
>> + dev_err(&client->dev, "Failed to get regulators: %pe",
>> + ERR_PTR(ret));
>
> Why are you using %pe here ? Your return value is not a pointer
> (Also, missing \n at the end of the string)
>
> From Documentation/core-api/printk-formats.rst:
> %pe -ENOSPC
>
> For printing error pointers (i.e. a pointer for which IS_ERR() is true)
> as a symbolic error name. Error values for which no symbolic name is
> known are printed in decimal, while a non-ERR_PTR passed as the
> argument to %pe gets treated as ordinary %p.
>
Nit: using:
+ return dev_err_probe(&client->dev, ret, "Failed to get regulators);
would be even simpler.
CJ
>> + return ret;
>> + }
>> +
>> v4l2_i2c_subdev_init(&ak7375_dev->sd, client, &ak7375_ops);
>> ak7375_dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> ak7375_dev->sd.internal_ops = &ak7375_int_ops;
>> @@ -210,6 +238,10 @@ static int __maybe_unused ak7375_vcm_suspend(struct device *dev)
>> if (ret)
>> dev_err(dev, "%s I2C failure: %d\n", __func__, ret);
>>
>> + ret = regulator_bulk_disable(AK7375_NUM_SUPPLIES, ak7375_dev->supplies);
>> + if (ret)
>> + return ret;
>> +
>> ak7375_dev->active = false;
>>
>> return 0;
>> @@ -230,6 +262,13 @@ static int __maybe_unused ak7375_vcm_resume(struct device *dev)
>> if (ak7375_dev->active)
>> return 0;
>>
>> + ret = regulator_bulk_enable(AK7375_NUM_SUPPLIES, ak7375_dev->supplies);
>> + if (ret)
>> + return ret;
>> +
>> + /* Wait for vcm to become ready */
>> + usleep_range(AK7375_POWER_DELAY_US, AK7375_POWER_DELAY_US + 500);
>> +
>> ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_CONT,
>> AK7375_MODE_ACTIVE, 1);
>> if (ret) {
>> --
>> 2.37.0
>>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: ak7375: Add regulator management
2022-07-11 14:40 ` [PATCH v2 3/3] media: i2c: ak7375: Add regulator management Yassine Oudjana
2022-07-11 17:31 ` Jacopo Mondi
2022-07-13 7:39 ` Jacopo Mondi
@ 2022-07-23 11:33 ` Sakari Ailus
2022-11-28 6:47 ` Umang Jain
3 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2022-07-23 11:33 UTC (permalink / raw)
To: Yassine Oudjana
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Tianshu Qiu, Bingbu Cao, Yassine Oudjana, Jacopo Mondi,
linux-media, devicetree, linux-kernel
Hi Yassine,
Thanks for the patch.
On Mon, Jul 11, 2022 at 06:40:39PM +0400, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Make the driver get needed regulators on probe and enable/disable
> them on runtime PM callbacks.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
> Changes since v1:
> - Reorganize variable declaration
> - Change the power-on delay range to 3000-3500 microseconds.
>
> drivers/media/i2c/ak7375.c | 39 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
> index 40b1a4aa846c..c2b2542a0056 100644
> --- a/drivers/media/i2c/ak7375.c
> +++ b/drivers/media/i2c/ak7375.c
> @@ -6,6 +6,7 @@
> #include <linux/i2c.h>
> #include <linux/module.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-device.h>
>
> @@ -23,17 +24,32 @@
> */
> #define AK7375_CTRL_STEPS 64
> #define AK7375_CTRL_DELAY_US 1000
> +/*
> + * The vcm takes around 3 ms to power on and start taking
> + * I2C messages. This value was found experimentally due to
> + * lack of documentation.
> + */
> +#define AK7375_POWER_DELAY_US 3000
>
> #define AK7375_REG_POSITION 0x0
> #define AK7375_REG_CONT 0x2
> #define AK7375_MODE_ACTIVE 0x0
> #define AK7375_MODE_STANDBY 0x40
>
> +static const char * const ak7375_supply_names[] = {
> + "vdd",
> + "vio",
> +};
> +
> +#define AK7375_NUM_SUPPLIES ARRAY_SIZE(ak7375_supply_names)
Please drop this, and use ARRAY_SIZE() directly.
> +
> /* ak7375 device structure */
> struct ak7375_device {
> struct v4l2_ctrl_handler ctrls_vcm;
> struct v4l2_subdev sd;
> struct v4l2_ctrl *focus;
> + struct regulator_bulk_data supplies[AK7375_NUM_SUPPLIES];
> +
> /* active or standby mode */
> bool active;
> };
> @@ -133,12 +149,24 @@ static int ak7375_probe(struct i2c_client *client)
> {
> struct ak7375_device *ak7375_dev;
> int ret;
> + int i;
unsigned int, please.
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/3] media: dt-bindings: ak7375: Convert to DT schema
2022-07-11 14:40 ` [PATCH v2 1/3] media: dt-bindings: ak7375: Convert to DT schema Yassine Oudjana
2022-07-12 8:17 ` Krzysztof Kozlowski
@ 2022-11-28 6:44 ` Umang Jain
1 sibling, 0 replies; 17+ messages in thread
From: Umang Jain @ 2022-11-28 6:44 UTC (permalink / raw)
To: Yassine Oudjana, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Tianshu Qiu, Bingbu Cao
Cc: Yassine Oudjana, Jacopo Mondi, linux-media, devicetree, linux-kernel
Hello,
I have been testing this VCM in conjunction with Arducam's IMX519 sensor
with libcamera and an autofocus algorithm plumbed in libcamera [1]
On 7/11/22 8:10 PM, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Convert DT bindings document for AKM AK7375 VCM to DT schema
> format and add an example.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
So,
Tested-by: Umang Jain <umang.jain@ideasonboard.com>
[1] https://patchwork.libcamera.org/project/libcamera/list/?series=3174
> ---
> Changes since v1:
> - Add vendor prefix to DT binding doc file name
>
> .../devicetree/bindings/media/i2c/ak7375.txt | 8 ----
> .../media/i2c/asahi-kasei,ak7375.yaml | 41 +++++++++++++++++++
> MAINTAINERS | 2 +-
> 3 files changed, 42 insertions(+), 9 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/media/i2c/ak7375.txt
> create mode 100644 Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ak7375.txt b/Documentation/devicetree/bindings/media/i2c/ak7375.txt
> deleted file mode 100644
> index aa3e24b41241..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ak7375.txt
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -Asahi Kasei Microdevices AK7375 voice coil lens driver
> -
> -AK7375 is a camera voice coil lens.
> -
> -Mandatory properties:
> -
> -- compatible: "asahi-kasei,ak7375"
> -- reg: I2C slave address
> diff --git a/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml b/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
> new file mode 100644
> index 000000000000..22b1251b16ee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/asahi-kasei,ak7375.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Asahi Kasei Microdevices AK7375 voice coil lens actuator
> +
> +maintainers:
> + - Tianshu Qiu <tian.shu.qiu@intel.com>
> +
> +description:
> + AK7375 is a voice coil motor (VCM) camera lens actuator that
> + is controlled over I2C.
> +
> +properties:
> + compatible:
> + const: asahi-kasei,ak7375
> +
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ak7375: camera-lens@c {
> + compatible = "asahi-kasei,ak7375";
> + reg = <0x0c>;
> + };
> + };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a97fef8c131d..e6accf8bc12b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3063,7 +3063,7 @@ M: Tianshu Qiu <tian.shu.qiu@intel.com>
> L: linux-media@vger.kernel.org
> S: Maintained
> T: git git://linuxtv.org/media_tree.git
> -F: Documentation/devicetree/bindings/media/i2c/ak7375.txt
> +F: Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
> F: drivers/media/i2c/ak7375.c
>
> ASAHI KASEI AK8974 DRIVER
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/3] media: dt-bindings: ak7375: Add supplies
2022-07-11 14:40 ` [PATCH v2 2/3] media: dt-bindings: ak7375: Add supplies Yassine Oudjana
@ 2022-11-28 6:45 ` Umang Jain
0 siblings, 0 replies; 17+ messages in thread
From: Umang Jain @ 2022-11-28 6:45 UTC (permalink / raw)
To: Yassine Oudjana, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Tianshu Qiu, Bingbu Cao
Cc: Yassine Oudjana, Jacopo Mondi, linux-media, devicetree,
linux-kernel, Krzysztof Kozlowski
Hi Yassine,
I have tested this VCM with IMX519 arducam sensor with a auto-focus
algorithm plumbed with libcamera [1]
On 7/11/22 8:10 PM, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Add supply properties to describe regulators needed to power
> the AK7375 VCM.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
So,
Tested-by: Umang Jain <umang.jain@ideasonboard.com>
[1] https://patchwork.libcamera.org/project/libcamera/list/?series=3174
> ---
> .../bindings/media/i2c/asahi-kasei,ak7375.yaml | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml b/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
> index 22b1251b16ee..22a810fc7222 100644
> --- a/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
> @@ -20,9 +20,17 @@ properties:
> reg:
> maxItems: 1
>
> + vdd-supply:
> + description: VDD supply
> +
> + vio-supply:
> + description: I/O pull-up supply
> +
> required:
> - compatible
> - reg
> + - vdd-supply
> + - vio-supply
>
> additionalProperties: false
>
> @@ -35,6 +43,9 @@ examples:
> ak7375: camera-lens@c {
> compatible = "asahi-kasei,ak7375";
> reg = <0x0c>;
> +
> + vdd-supply = <&vreg_l23a_2p8>;
> + vio-supply = <&vreg_lvs1a_1p8>;
> };
> };
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: ak7375: Add regulator management
2022-07-11 14:40 ` [PATCH v2 3/3] media: i2c: ak7375: Add regulator management Yassine Oudjana
` (2 preceding siblings ...)
2022-07-23 11:33 ` Sakari Ailus
@ 2022-11-28 6:47 ` Umang Jain
3 siblings, 0 replies; 17+ messages in thread
From: Umang Jain @ 2022-11-28 6:47 UTC (permalink / raw)
To: Yassine Oudjana, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Tianshu Qiu, Bingbu Cao
Cc: Yassine Oudjana, Jacopo Mondi, linux-media, devicetree, linux-kernel
Hi Yassine,
I have tested this VCM with autofocus algorithm plumbed in libcamera [1]
On 7/11/22 8:10 PM, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Make the driver get needed regulators on probe and enable/disable
> them on runtime PM callbacks.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
Tested-by: Umang Jain <umang.jain@ideasonboard.com>
[1] https://patchwork.libcamera.org/project/libcamera/list/?series=3174
> ---
> Changes since v1:
> - Reorganize variable declaration
> - Change the power-on delay range to 3000-3500 microseconds.
>
> drivers/media/i2c/ak7375.c | 39 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
> index 40b1a4aa846c..c2b2542a0056 100644
> --- a/drivers/media/i2c/ak7375.c
> +++ b/drivers/media/i2c/ak7375.c
> @@ -6,6 +6,7 @@
> #include <linux/i2c.h>
> #include <linux/module.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-device.h>
>
> @@ -23,17 +24,32 @@
> */
> #define AK7375_CTRL_STEPS 64
> #define AK7375_CTRL_DELAY_US 1000
> +/*
> + * The vcm takes around 3 ms to power on and start taking
> + * I2C messages. This value was found experimentally due to
> + * lack of documentation.
> + */
> +#define AK7375_POWER_DELAY_US 3000
>
> #define AK7375_REG_POSITION 0x0
> #define AK7375_REG_CONT 0x2
> #define AK7375_MODE_ACTIVE 0x0
> #define AK7375_MODE_STANDBY 0x40
>
> +static const char * const ak7375_supply_names[] = {
> + "vdd",
> + "vio",
> +};
> +
> +#define AK7375_NUM_SUPPLIES ARRAY_SIZE(ak7375_supply_names)
> +
> /* ak7375 device structure */
> struct ak7375_device {
> struct v4l2_ctrl_handler ctrls_vcm;
> struct v4l2_subdev sd;
> struct v4l2_ctrl *focus;
> + struct regulator_bulk_data supplies[AK7375_NUM_SUPPLIES];
> +
> /* active or standby mode */
> bool active;
> };
> @@ -133,12 +149,24 @@ static int ak7375_probe(struct i2c_client *client)
> {
> struct ak7375_device *ak7375_dev;
> int ret;
> + int i;
>
> ak7375_dev = devm_kzalloc(&client->dev, sizeof(*ak7375_dev),
> GFP_KERNEL);
> if (!ak7375_dev)
> return -ENOMEM;
>
> + for (i = 0; i < AK7375_NUM_SUPPLIES; i++)
> + ak7375_dev->supplies[i].supply = ak7375_supply_names[i];
> +
> + ret = devm_regulator_bulk_get(&client->dev, AK7375_NUM_SUPPLIES,
> + ak7375_dev->supplies);
> + if (ret) {
> + dev_err(&client->dev, "Failed to get regulators: %pe",
> + ERR_PTR(ret));
> + return ret;
> + }
> +
> v4l2_i2c_subdev_init(&ak7375_dev->sd, client, &ak7375_ops);
> ak7375_dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> ak7375_dev->sd.internal_ops = &ak7375_int_ops;
> @@ -210,6 +238,10 @@ static int __maybe_unused ak7375_vcm_suspend(struct device *dev)
> if (ret)
> dev_err(dev, "%s I2C failure: %d\n", __func__, ret);
>
> + ret = regulator_bulk_disable(AK7375_NUM_SUPPLIES, ak7375_dev->supplies);
> + if (ret)
> + return ret;
> +
> ak7375_dev->active = false;
>
> return 0;
> @@ -230,6 +262,13 @@ static int __maybe_unused ak7375_vcm_resume(struct device *dev)
> if (ak7375_dev->active)
> return 0;
>
> + ret = regulator_bulk_enable(AK7375_NUM_SUPPLIES, ak7375_dev->supplies);
> + if (ret)
> + return ret;
> +
> + /* Wait for vcm to become ready */
> + usleep_range(AK7375_POWER_DELAY_US, AK7375_POWER_DELAY_US + 500);
> +
> ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_CONT,
> AK7375_MODE_ACTIVE, 1);
> if (ret) {
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-11-28 6:48 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-11 14:40 [PATCH v2 0/3] media: i2c: ak7375: Add regulator management Yassine Oudjana
2022-07-11 14:40 ` [PATCH v2 1/3] media: dt-bindings: ak7375: Convert to DT schema Yassine Oudjana
2022-07-12 8:17 ` Krzysztof Kozlowski
2022-11-28 6:44 ` Umang Jain
2022-07-11 14:40 ` [PATCH v2 2/3] media: dt-bindings: ak7375: Add supplies Yassine Oudjana
2022-11-28 6:45 ` Umang Jain
2022-07-11 14:40 ` [PATCH v2 3/3] media: i2c: ak7375: Add regulator management Yassine Oudjana
2022-07-11 17:31 ` Jacopo Mondi
2022-07-14 13:56 ` Yassine Oudjana
2022-07-14 15:56 ` Jacopo Mondi
2022-07-20 7:36 ` Christophe JAILLET
2022-07-13 7:39 ` Jacopo Mondi
2022-07-14 14:06 ` Yassine Oudjana
2022-07-14 15:58 ` Jacopo Mondi
2022-07-20 2:58 ` Lee Jackson
2022-07-23 11:33 ` Sakari Ailus
2022-11-28 6:47 ` Umang Jain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).