motorola-nora: enable cameras and flashes #13

valpackett · 2025-02-03T04:29:51Z

s5k3l8 (back) + ov5675 (front), both work.. when camss itself isn't being stupid

apparently alternatives would be s5k4h7 front and s5k4h8 back (that one "for ashelby lite" according to commit messages)

xxx: bleh, adding more stuff to a very specific dts, before extracting a common nora+jeter one
todo: DW9718S VCM, should be easy
~~why does adding flash-leds break everything!? (subdevs silently don't get created)~~ right, leds v1 doesn't integrate v4l2 yet, that's a todo

barni2000 · 2025-02-05T23:18:16Z

I have converted to draft until todos are considered done.

barni2000 · 2025-02-05T23:19:19Z

If some dependency not available in time everything will be down so maybe that is why flash led breaks everything.

v5: - add test patterns - reduce init sequences - switch to regmap v6: - implement link_frequency

Fix NULL pointer check before device_link_del is called. Unable to handle kernel NULL pointer dereference at virtual address 000000000000032c Call trace: device_link_put_kref+0xc/0xb8 device_link_del+0x30/0x48 vfe_pm_domain_off+0x24/0x38 [qcom_camss] vfe_put+0x9c/0xd0 [qcom_camss] vfe_set_power+0x48/0x58 [qcom_camss] pipeline_pm_power_one+0x154/0x158 [videodev] pipeline_pm_power+0x74/0xfc [videodev] v4l2_pipeline_pm_use+0x54/0x90 [videodev] v4l2_pipeline_pm_put+0x14/0x34 [videodev] video_release+0x2c/0x44 [qcom_camss] v4l2_release+0xe4/0xec [videodev] Fixes: eb73fac ("media: qcom: camss: Use common VFE pm_domain_on/pm_domain_off where applicable") Tested-by: Yassine Oudjana <[email protected]> Reviewed-by: Bryan O'Donoghue <[email protected]> Signed-off-by: Barnabás Czémán <[email protected]>

Both rear and front flashes work. Signed-off-by: Val Packett <[email protected]>

These are common to all devices, make it easy to configure pinctrl for camera clocks (MCLK) like it is on msm8953. Signed-off-by: Val Packett <[email protected]>

Add DT bindings for the dw9719 voice coil motor driver, which is getting devicetree compatibles added along with DW9718S support. Also mention the binding file in the corresponding MAINTAINERS entry. Signed-off-by: Val Packett <[email protected]>

valpackett · 2025-02-10T08:34:25Z

Added DW9718S VCM, submitted it upstream

The spmi v1 flash driver does not integrate with v4l2 at all currently, we'll need to tackle that before upstreaming it

barni2000 · 2025-02-11T16:57:50Z

arch/arm64/boot/dts/qcom/msm8917-motorola-nora.dts

are you sure this is s3 and not l2?

oops. actually not sure where I got s3 from at all, lol. it should be l2

8953 have s3

Add support for the DW9761 VCM controller, which is very similar to the DW9719. The new support is based on drivers/external_drivers/camera/drivers/media/i2c/micam/dw9761.c from the Xiaomi kernel sources for the Mi Pad 2. The DW9761 support has been tested on a Xiaomi Mi Pad 2 tablet and DW9719 support has been tested (to avoid regressions) on a Microsoft Surface Go tablet. Link: https://github.com/MiCode/Xiaomi_Kernel_OpenSource/ Signed-off-by: Hans de Goede <[email protected]> Tested-by: André Apitzsch <[email protected]> Signed-off-by: Sakari Ailus <[email protected]> Signed-off-by: Val Packett <[email protected]>

In preparation for adding models with different register sets, start assigning the model based on the i2c match data. Signed-off-by: Val Packett <[email protected]>

The DW9718S is a similar part that uses a different register set but follows the same method of operation otherwise. Add support for it to the existing dw9719 driver. Tested on the Moto E5 (motorola-nora) smartphone. Signed-off-by: Val Packett <[email protected]>

Update the close callback to match other similar drivers like dw9768. Signed-off-by: Val Packett <[email protected]>

Allow the dw9719 driver to be attached via FDT. Signed-off-by: Val Packett <[email protected]>

The "jiggle" code was not actually expecting failure, which it should because that's what actually happens when the device wasn't already woken up by the regulator power-on (i.e. in the case of a shared regulator). Also, do actually enter the internal suspend mode on shutdown, to save power in the case of a shared regulator. Also, wait a bit longer (2x tOPR) on waking up, 1x is not enough at least on the DW9718S as found on the motorola-nora smartphone. Signed-off-by: Val Packett <[email protected]>

The Moto G5 includes front and back camera sensors. Signed-off-by: Val Packett <[email protected]>

a-andre · 2025-02-19T21:48:29Z

drivers/media/i2c/dw9719.c

+		if (val != dw9719->model)
+			dev_warn(dw9719->dev,
+			         "Model ID 0x%llx does not match declared model", val);


Is this necessary? Shouldn't the following switch handle this?

BTW val will probably never match dw9719->model, because val is one of {DW9719_ID, DW9761_ID} and dw9719->model is an enum entry mapped to an integer.

no, it's just "to be helpful"; and yes, I've realized that when you messaged me on matrix :D

a-andre · 2025-02-19T21:51:13Z

drivers/media/i2c/dw9719.c

+	 * Jiggle the SCL pin to wake up the device (even when the regulator
+	 * is shared) and wait double the time to be sure, then retry the write.
+	 */
+	cci_write(dw9719->regmap, reg_pwr, DW9719_STANDBY, &ret);


If you don't care about the return value, no need to set it.

Suggested change

cci_write(dw9719->regmap, reg_pwr, DW9719_STANDBY, &ret);

cci_write(dw9719->regmap, reg_pwr, DW9719_STANDBY, NULL);

The following comment can then be moved before this cci_write() line

NULL causes an error message to be printed in case of a failure. In this case we're fully expecting failure, so no need to bother/scare the users :)

(I thought the comments would make this obvious enough…)

NULL causes an error message to be printed in case of a failure.

Didn't know that.

a-andre · 2025-02-19T21:55:54Z

drivers/media/i2c/dw9719.c

+	{ .compatible = "dongwoon,dw9718s", .data = (const void*)DW9718S },
+	{ .compatible = "dongwoon,dw9719",  .data = (const void*)DW9719 },
+	{ .compatible = "dongwoon,dw9761",  .data = (const void*)DW9761 },
+	{ { 0 } }


Suggested change

{ { 0 } }

{ }

a-andre · 2025-02-20T08:34:55Z

Documentation/devicetree/bindings/media/i2c/dongwoon,dw9719.yaml

@@ -0,0 +1,110 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)


GPL-2.0 should be replaced by GPL-2.0-only (or GPL-2.0-or-later)

a-andre · 2025-02-20T08:35:46Z

drivers/media/i2c/s5k2xx.c

@@ -0,0 +1,1468 @@
+// SPDX-License-Identifier: GPL-2.0


GPL-2.0 should be replaced by GPL-2.0-only

This should be modified in the source repo (msm8953-mainline/linux)

a-andre · 2025-02-20T08:36:46Z

drivers/media/i2c/s5k2xx.c

+module_i2c_driver(s5k2xx_i2c_driver);
+
+MODULE_DESCRIPTION("Samsung S5K2xx sensor driver");
+MODULE_LICENSE("GPL v2");


Suggested change

MODULE_LICENSE("GPL v2");

MODULE_LICENSE("GPL");

Same it should be modified in the source repo.

a-andre · 2025-02-20T08:43:52Z

drivers/media/i2c/s5k2xx.c

@@ -0,0 +1,1468 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Intel Corporation.
+


For upstream the driver should probably be converted to CCI register access helpers.

Possibly it will be not upstreamed, there is a chance it will be replaced with ccs in the future.

barni2000 · 2025-03-01T22:01:50Z

arch/arm64/boot/dts/qcom/msm8917-motorola-nora.dts

+};
+
+&cci {
+	pinctrl-names = "default";


pinctrl-names comes after pinctrls-0

barni2000 · 2025-03-01T22:03:47Z

drivers/media/i2c/s5k2xx.c

@@ -0,0 +1,1468 @@
+// SPDX-License-Identifier: GPL-2.0


This should be modified in the source repo (msm8953-mainline/linux)

barni2000 · 2025-03-01T22:04:37Z

drivers/media/i2c/s5k2xx.c

@@ -0,0 +1,1468 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Intel Corporation.
+


Possibly it will be not upstreamed, there is a chance it will be replaced with ccs in the future.

barni2000 · 2025-03-01T22:05:05Z

drivers/media/i2c/s5k2xx.c

+module_i2c_driver(s5k2xx_i2c_driver);
+
+MODULE_DESCRIPTION("Samsung S5K2xx sensor driver");
+MODULE_LICENSE("GPL v2");


Same it should be modified in the source repo.

barni2000 · 2025-03-16T17:21:15Z

@valpackett Can you please rebase it on the 6.13.6 branch?

barni2000 marked this pull request as draft February 5, 2025 23:17

vldly and others added 5 commits February 9, 2025 19:48

media: i2c: add s5k2xx camera sensor driver (v6)

920c79d

v5: - add test patterns - reduce init sequences - switch to regmap v6: - implement link_frequency

media: i2c: s5k2xx:Add s5k3l8 support

aff6ed6

arm64: dts: qcom: motorola-nora: add flash

76f822d

Both rear and front flashes work. Signed-off-by: Val Packett <[email protected]>

arm64: dts: qcom: msm8917: add MCLK pinctrl configs

e9b4ecb

These are common to all devices, make it easy to configure pinctrl for camera clocks (MCLK) like it is on msm8953. Signed-off-by: Val Packett <[email protected]>

valpackett force-pushed the push-suytyzkqkprr branch from 804965e to 5669ea2 Compare February 9, 2025 23:03

valpackett force-pushed the push-suytyzkqkprr branch from 5669ea2 to 93b2a18 Compare February 10, 2025 08:21

barni2000 reviewed Feb 11, 2025

View reviewed changes

jwrdegoede and others added 5 commits February 17, 2025 16:43

media: i2c: dw9719: Add driver_data matching

8e66228

In preparation for adding models with different register sets, start assigning the model based on the i2c match data. Signed-off-by: Val Packett <[email protected]>

media: i2c: dw9719: Update PM last busy time upon close

8f7f3e0

Update the close callback to match other similar drivers like dw9768. Signed-off-by: Val Packett <[email protected]>

media: i2c: dw9719: Add an of_match_table

3250077

Allow the dw9719 driver to be attached via FDT. Signed-off-by: Val Packett <[email protected]>

valpackett force-pushed the push-suytyzkqkprr branch from 93b2a18 to 2ef13e9 Compare February 18, 2025 05:57

valpackett added 2 commits February 18, 2025 03:08

arm64: dts: qcom: motorola-nora: add cameras

6fb888e

The Moto G5 includes front and back camera sensors. Signed-off-by: Val Packett <[email protected]>

valpackett force-pushed the push-suytyzkqkprr branch from 2ef13e9 to 6fb888e Compare February 18, 2025 06:09

a-andre reviewed Feb 19, 2025

View reviewed changes

a-andre reviewed Feb 20, 2025

View reviewed changes

barni2000 requested changes Mar 1, 2025

View reviewed changes

	cci_write(dw9719->regmap, reg_pwr, DW9719_STANDBY, &ret);
	cci_write(dw9719->regmap, reg_pwr, DW9719_STANDBY, NULL);

		@@ -0,0 +1,110 @@
		# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)

		@@ -0,0 +1,1468 @@
		// SPDX-License-Identifier: GPL-2.0
		// Copyright (c) 2019 Intel Corporation.

motorola-nora: enable cameras and flashes #13

Are you sure you want to change the base?

motorola-nora: enable cameras and flashes #13

Uh oh!

Conversation

valpackett commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

barni2000 commented Feb 5, 2025

Uh oh!

barni2000 commented Feb 5, 2025

Uh oh!

valpackett commented Feb 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barni2000 commented Mar 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

valpackett commented Feb 3, 2025 •

edited

Loading