* Re: [PATCH v5 2/2] hw_random: timeriomem-rng: add configurable read width and data mask
From: sashiko-bot @ 2026-06-18 12:13 UTC (permalink / raw)
To: Jad Keskes; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260618120110.36439-2-inasj268@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Race condition and broken teardown sequence between module unbind and active hwrng readers causes Deadlocks or Use-After-Free of the private data.
--
commit d12f91b70dc751fd43c31e87b02671ebcad42270
Author: Jad Keskes <inasj268@gmail.com>
hw_random: timeriomem-rng: add configurable read width and data mask
This commit implements configurable read sizes and data masking for the
timeriomem-rng driver. It adds support for 1, 2, or 4-byte reads and an
optional bitmask to filter output entropy, configurable via platform data
or device tree.
> diff --git a/drivers/char/hw_random/timeriomem-rng.c b/drivers/char/hw_random/timeriomem-rng.c
> index e61f063932090..42393409f22ad 100644
> --- a/drivers/char/hw_random/timeriomem-rng.c
> +++ b/drivers/char/hw_random/timeriomem-rng.c
[ ... ]
> @@ -71,11 +76,28 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
> usleep_range(period_us,
> period_us + max(1, period_us / 100));
>
> - *(u32 *)data = readl(priv->io_base);
> - retval += sizeof(u32);
> - data += sizeof(u32);
> - max -= sizeof(u32);
> - } while (wait && max > sizeof(u32));
> + switch (priv->reg_io_width) {
> + case 1: {
> + u8 val = readb(priv->io_base) & priv->mask;
> + *(u8 *)data = val;
> + break;
> + }
> + case 2: {
> + u16 val = readw(priv->io_base) & priv->mask;
> + *(u16 *)data = val;
> + break;
> + }
> + case 4: {
> + u32 val = readl(priv->io_base) & priv->mask;
> + *(u32 *)data = val;
> + break;
> + }
> + }
> +
> + retval += chunk;
> + data += chunk;
> + max -= chunk;
> + } while (wait && max > chunk);
>
> /*
> * Block any new callers until the RNG has had time to generate new
[Severity: High]
This is a pre-existing issue, but can this sequence lead to deadlocks or a
use-after-free during module teardown?
Because the driver uses devm_hwrng_register(), devres unregisters the RNG
after the driver's remove function returns. The timeriomem_rng_remove function
explicitly calls hrtimer_cancel(&priv->timer), but concurrent readers might
still be active in timeriomem_rng_read.
If a reader is waiting earlier in this function at
wait_for_completion(&priv->completion) when the timer is cancelled, won't it
hang forever since the timer will never fire to wake it up?
Alternatively, if the reader is already past the wait, it will unconditionally
call hrtimer_restart(&priv->timer) slightly further down at the end of this
function. Doesn't this re-arm the timer after it was cancelled by the removal
thread?
If the timer is re-armed and devres subsequently frees the
timeriomem_rng_private data, won't the timer eventually fire and access freed
memory?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618120110.36439-1-inasj268@gmail.com?part=2
^ permalink raw reply
* [PATCH v2 3/3] arm64: dts: qcom: sm8750: Add camera MCLK pinctrl
From: Hangxiang Ma @ 2026-06-18 12:03 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel,
jeyaprakash.soundrapandian, Vijay Kumar Tumati, Hangxiang Ma,
Konrad Dybcio
In-Reply-To: <20260618-pakala-camera-v2-0-9fbb729fd242@oss.qualcomm.com>
Define pinctrl definitions to enable camera master clocks on SM8750.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/sm8750.dtsi | 56 ++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
index 7dda8408c6d1..57d1b05be8ac 100644
--- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
@@ -3899,6 +3899,62 @@ tlmm: pinctrl@f100000 {
gpio-ranges = <&tlmm 0 0 216>;
wakeup-parent = <&pdc>;
+ cam0_default: cam0-default-state {
+ pins = "gpio89";
+ function = "cam_mclk";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ cam1_default: cam1-default-state {
+ pins = "gpio90";
+ function = "cam_mclk";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ cam2_default: cam2-default-state {
+ pins = "gpio91";
+ function = "cam_aon_mclk2";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ cam3_default: cam3-default-state {
+ pins = "gpio92";
+ function = "cam_mclk";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ cam4_default: cam4-default-state {
+ pins = "gpio93";
+ function = "cam_aon_mclk4";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ cam5_default: cam5-default-state {
+ pins = "gpio94";
+ function = "cam_mclk";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ cam6_default: cam6-default-state {
+ pins = "gpio95";
+ function = "cam_mclk";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ cam7_default: cam7-default-state {
+ pins = "gpio96";
+ function = "cam_mclk";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
cci0_0_default: cci0-0-default-state {
sda-pins {
pins = "gpio113";
--
2.34.1
^ permalink raw reply related
* [PATCH v2 2/3] arm64: dts: qcom: sm8750: Add CCI definitions
From: Hangxiang Ma @ 2026-06-18 12:03 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel,
jeyaprakash.soundrapandian, Vijay Kumar Tumati, Hangxiang Ma
In-Reply-To: <20260618-pakala-camera-v2-0-9fbb729fd242@oss.qualcomm.com>
Qualcomm SM8750 SoC has three Camera Control Interface (CCI). Each
controller contains two I2C hosts.
Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/sm8750.dtsi | 282 +++++++++++++++++++++++++++++++++++
1 file changed, 282 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
index 15eb588acafb..7dda8408c6d1 100644
--- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
@@ -3037,6 +3037,96 @@ videocc: clock-controller@aaf0000 {
#power-domain-cells = <1>;
};
+ cci0: cci@ac7b000 {
+ compatible = "qcom,sm8750-cci", "qcom,msm8996-cci";
+ reg = <0x0 0x0ac7b000 0x0 0x1000>;
+ interrupts = <GIC_SPI 426 IRQ_TYPE_EDGE_RISING>;
+ power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>;
+ clocks = <&camcc CAM_CC_CAM_TOP_AHB_CLK>,
+ <&camcc CAM_CC_CCI_0_CLK>;
+ clock-names = "ahb", "cci";
+ pinctrl-0 = <&cci0_0_default &cci0_1_default>;
+ pinctrl-1 = <&cci0_0_sleep &cci0_1_sleep>;
+ pinctrl-names = "default", "sleep";
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cci0_i2c0: i2c-bus@0 {
+ reg = <0>;
+ clock-frequency = <1000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ cci0_i2c1: i2c-bus@1 {
+ reg = <1>;
+ clock-frequency = <1000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
+ cci1: cci@ac7c000 {
+ compatible = "qcom,sm8750-cci", "qcom,msm8996-cci";
+ reg = <0x0 0x0ac7c000 0x0 0x1000>;
+ interrupts = <GIC_SPI 427 IRQ_TYPE_EDGE_RISING>;
+ power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>;
+ clocks = <&camcc CAM_CC_CAM_TOP_AHB_CLK>,
+ <&camcc CAM_CC_CCI_1_CLK>;
+ clock-names = "ahb", "cci";
+ pinctrl-0 = <&cci1_0_default &cci1_1_default>;
+ pinctrl-1 = <&cci1_0_sleep &cci1_1_sleep>;
+ pinctrl-names = "default", "sleep";
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cci1_i2c0: i2c-bus@0 {
+ reg = <0>;
+ clock-frequency = <1000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ cci1_i2c1: i2c-bus@1 {
+ reg = <1>;
+ clock-frequency = <1000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
+ cci2: cci@ac7d000 {
+ compatible = "qcom,sm8750-cci", "qcom,msm8996-cci";
+ reg = <0x0 0x0ac7d000 0x0 0x1000>;
+ interrupts = <GIC_SPI 428 IRQ_TYPE_EDGE_RISING>;
+ power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>;
+ clocks = <&camcc CAM_CC_CAM_TOP_AHB_CLK>,
+ <&camcc CAM_CC_CCI_2_CLK>;
+ clock-names = "ahb", "cci";
+ pinctrl-0 = <&cci2_0_default &cci2_1_default>;
+ pinctrl-1 = <&cci2_0_sleep &cci2_1_sleep>;
+ pinctrl-names = "default", "sleep";
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cci2_i2c0: i2c-bus@0 {
+ reg = <0>;
+ clock-frequency = <1000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ cci2_i2c1: i2c-bus@1 {
+ reg = <1>;
+ clock-frequency = <1000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
camss: isp@ad27000 {
compatible = "qcom,sm8750-camss";
@@ -3809,6 +3899,198 @@ tlmm: pinctrl@f100000 {
gpio-ranges = <&tlmm 0 0 216>;
wakeup-parent = <&pdc>;
+ cci0_0_default: cci0-0-default-state {
+ sda-pins {
+ pins = "gpio113";
+ function = "cci_i2c_sda";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ scl-pins {
+ pins = "gpio114";
+ function = "cci_i2c_scl";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+ };
+
+ cci0_0_sleep: cci0-0-sleep-state {
+ sda-pins {
+ pins = "gpio113";
+ function = "cci_i2c_sda";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+
+ scl-pins {
+ pins = "gpio114";
+ function = "cci_i2c_scl";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+ };
+
+ cci0_1_default: cci0-1-default-state {
+ sda-pins {
+ pins = "gpio115";
+ function = "cci_i2c_sda";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ scl-pins {
+ pins = "gpio116";
+ function = "cci_i2c_scl";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+ };
+
+ cci0_1_sleep: cci0-1-sleep-state {
+ sda-pins {
+ pins = "gpio115";
+ function = "cci_i2c_sda";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+
+ scl-pins {
+ pins = "gpio116";
+ function = "cci_i2c_scl";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+ };
+
+ cci1_0_default: cci1-0-default-state {
+ sda-pins {
+ pins = "gpio117";
+ function = "cci_i2c_sda";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ scl-pins {
+ pins = "gpio118";
+ function = "cci_i2c_scl";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+ };
+
+ cci1_0_sleep: cci1-0-sleep-state {
+ sda-pins {
+ pins = "gpio117";
+ function = "cci_i2c_sda";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+
+ scl-pins {
+ pins = "gpio118";
+ function = "cci_i2c_scl";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+ };
+
+ cci1_1_default: cci1-1-default-state {
+ sda-pins {
+ pins = "gpio111";
+ function = "cci_i2c_sda";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ scl-pins {
+ pins = "gpio164";
+ function = "cci_i2c_scl";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+ };
+
+ cci1_1_sleep: cci1-1-sleep-state {
+ sda-pins {
+ pins = "gpio111";
+ function = "cci_i2c_sda";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+
+ scl-pins {
+ pins = "gpio164";
+ function = "cci_i2c_scl";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+ };
+
+ cci2_0_default: cci2-0-default-state {
+ sda-pins {
+ pins = "gpio112";
+ function = "cci_i2c_sda";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ scl-pins {
+ pins = "gpio153";
+ function = "cci_i2c_scl";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+ };
+
+ cci2_0_sleep: cci2-0-sleep-state {
+ sda-pins {
+ pins = "gpio112";
+ function = "cci_i2c_sda";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+
+ scl-pins {
+ pins = "gpio153";
+ function = "cci_i2c_scl";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+ };
+
+ cci2_1_default: cci2-1-default-state {
+ sda-pins {
+ pins = "gpio119";
+ function = "cci_i2c_sda";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ scl-pins {
+ pins = "gpio120";
+ function = "cci_i2c_scl";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+ };
+
+ cci2_1_sleep: cci2-1-sleep-state {
+ sda-pins {
+ pins = "gpio119";
+ function = "cci_i2c_sda";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+
+ scl-pins {
+ pins = "gpio120";
+ function = "cci_i2c_scl";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+ };
+
hub_i2c0_data_clk: hub-i2c0-data-clk-state {
/* SDA, SCL */
pins = "gpio64", "gpio65";
--
2.34.1
^ permalink raw reply related
* [PATCH v2 1/3] arm64: dts: qcom: sm8750: Add camss node
From: Hangxiang Ma @ 2026-06-18 12:03 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel,
jeyaprakash.soundrapandian, Vijay Kumar Tumati, Hangxiang Ma
In-Reply-To: <20260618-pakala-camera-v2-0-9fbb729fd242@oss.qualcomm.com>
Add node for the SM8750 camera subsystem.
Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/sm8750.dtsi | 203 +++++++++++++++++++++++++++++++++++
1 file changed, 203 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
index fafed417c66f..15eb588acafb 100644
--- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
@@ -3037,6 +3037,209 @@ videocc: clock-controller@aaf0000 {
#power-domain-cells = <1>;
};
+ camss: isp@ad27000 {
+ compatible = "qcom,sm8750-camss";
+
+ reg = <0x0 0x0ad27000 0x0 0x2b00>,
+ <0x0 0x0ad2a000 0x0 0x2b00>,
+ <0x0 0x0ad2d000 0x0 0x2b00>,
+ <0x0 0x0ad6d000 0x0 0xa00>,
+ <0x0 0x0ad72000 0x0 0xa00>,
+ <0x0 0x0ada9000 0x0 0x2000>,
+ <0x0 0x0adab000 0x0 0x2000>,
+ <0x0 0x0adad000 0x0 0x2000>,
+ <0x0 0x0adaf000 0x0 0x2000>,
+ <0x0 0x0adb1000 0x0 0x2000>,
+ <0x0 0x0adb3000 0x0 0x2000>,
+ <0x0 0x0ad8b000 0x0 0x400>,
+ <0x0 0x0ad8c000 0x0 0x400>,
+ <0x0 0x0ad8d000 0x0 0x400>,
+ <0x0 0x0ac86000 0x0 0x10000>,
+ <0x0 0x0ac96000 0x0 0x10000>,
+ <0x0 0x0aca6000 0x0 0x10000>,
+ <0x0 0x0ad6e000 0x0 0x3000>,
+ <0x0 0x0ad73000 0x0 0x3000>;
+ reg-names = "csid0",
+ "csid1",
+ "csid2",
+ "csid_lite0",
+ "csid_lite1",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy3",
+ "csiphy4",
+ "csiphy5",
+ "csitpg0",
+ "csitpg1",
+ "csitpg2",
+ "vfe0",
+ "vfe1",
+ "vfe2",
+ "vfe_lite0",
+ "vfe_lite1";
+
+ clocks = <&camcc CAM_CC_CAMNOC_NRT_AXI_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_AXI_CLK>,
+ <&camcc CAM_CC_CAM_TOP_AHB_CLK>,
+ <&camcc CAM_CC_CAM_TOP_FAST_AHB_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_TFE_0_MAIN_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_TFE_1_MAIN_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_TFE_2_MAIN_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_IFE_LITE_CLK>,
+ <&camcc CAM_CC_CSID_CLK>,
+ <&camcc CAM_CC_CSID_CSIPHY_RX_CLK>,
+ <&camcc CAM_CC_CSIPHY0_CLK>,
+ <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY1_CLK>,
+ <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY2_CLK>,
+ <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY3_CLK>,
+ <&camcc CAM_CC_CSI3PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY4_CLK>,
+ <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY5_CLK>,
+ <&camcc CAM_CC_CSI5PHYTIMER_CLK>,
+ <&gcc GCC_CAMERA_HF_AXI_CLK>,
+ <&gcc GCC_CAMERA_SF_AXI_CLK>,
+ <&camcc CAM_CC_TFE_0_MAIN_CLK>,
+ <&camcc CAM_CC_TFE_0_MAIN_FAST_AHB_CLK>,
+ <&camcc CAM_CC_TFE_1_MAIN_CLK>,
+ <&camcc CAM_CC_TFE_1_MAIN_FAST_AHB_CLK>,
+ <&camcc CAM_CC_TFE_2_MAIN_CLK>,
+ <&camcc CAM_CC_TFE_2_MAIN_FAST_AHB_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CLK>,
+ <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CSID_CLK>,
+ <&camcc CAM_CC_QDSS_DEBUG_XO_CLK>;
+ clock-names = "camnoc_nrt_axi",
+ "camnoc_rt_axi",
+ "cpas_ahb",
+ "cpas_fast_ahb",
+ "cpas_vfe0",
+ "cpas_vfe1",
+ "cpas_vfe2",
+ "cpas_vfe_lite",
+ "csid",
+ "csid_csiphy_rx",
+ "csiphy0",
+ "csiphy0_timer",
+ "csiphy1",
+ "csiphy1_timer",
+ "csiphy2",
+ "csiphy2_timer",
+ "csiphy3",
+ "csiphy3_timer",
+ "csiphy4",
+ "csiphy4_timer",
+ "csiphy5",
+ "csiphy5_timer",
+ "gcc_axi_hf",
+ "gcc_axi_sf",
+ "vfe0",
+ "vfe0_fast_ahb",
+ "vfe1",
+ "vfe1_fast_ahb",
+ "vfe2",
+ "vfe2_fast_ahb",
+ "vfe_lite",
+ "vfe_lite_ahb",
+ "vfe_lite_cphy_rx",
+ "vfe_lite_csid",
+ "qdss_debug_xo";
+
+ interrupts = <GIC_SPI 601 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 603 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 431 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 605 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 376 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 448 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 122 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 89 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 433 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 436 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 457 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 606 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 377 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "csid0",
+ "csid1",
+ "csid2",
+ "csid_lite0",
+ "csid_lite1",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy3",
+ "csiphy4",
+ "csiphy5",
+ "vfe0",
+ "vfe1",
+ "vfe2",
+ "vfe_lite0",
+ "vfe_lite1";
+
+ interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+ &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
+ <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+ <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+ <&mmss_noc MASTER_CAMNOC_NRT_ICP_SF QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
+ interconnect-names = "ahb",
+ "hf_mnoc",
+ "sf_mnoc",
+ "sf_icp_mnoc";
+
+ iommus = <&apps_smmu 0x1c00 0x00>;
+
+ power-domains = <&camcc CAM_CC_TFE_0_GDSC>,
+ <&camcc CAM_CC_TFE_1_GDSC>,
+ <&camcc CAM_CC_TFE_2_GDSC>,
+ <&camcc CAM_CC_TITAN_TOP_GDSC>;
+ power-domain-names = "ife0",
+ "ife1",
+ "ife2",
+ "top";
+
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ };
+
+ port@1 {
+ reg = <1>;
+ };
+
+ port@2 {
+ reg = <2>;
+ };
+
+ port@3 {
+ reg = <3>;
+ };
+
+ port@4 {
+ reg = <4>;
+ };
+
+ port@5 {
+ reg = <5>;
+ };
+ };
+
+ };
+
mdss: display-subsystem@ae00000 {
compatible = "qcom,sm8750-mdss";
reg = <0x0 0x0ae00000 0x0 0x1000>;
--
2.34.1
^ permalink raw reply related
* [PATCH v2 0/3] Add CCI and CAMSS support for SM8750
From: Hangxiang Ma @ 2026-06-18 12:03 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel,
jeyaprakash.soundrapandian, Vijay Kumar Tumati, Hangxiang Ma,
Konrad Dybcio
This series adds CCI and CAMSS support for Qualcomm SM8750 SoC.
This series has been tested using the following commands with a downstream
driver for S5KJN5 sensor.
- media-ctl --reset
- media-ctl -V '"msm_csiphy2":0[fmt:SGBRG10/4096x3072]'
- media-ctl -V '"msm_csid0":0[fmt:SGBRG10/4096x3072]'
- media-ctl -V '"msm_vfe0_rdi0":0[fmt:SGBRG10/4096x3072]'
- media-ctl -l '"msm_csiphy2":1->"msm_csid0":0[1]'
- media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
- yavta --capture=20 -I -n 5 -f SGBRG10P -s 4096x3072 -F /dev/video0
Driver and dt-binding are waiting to be merged:
https://lore.kernel.org/all/20260601-add-support-for-camss-on-sm8750-v5-0-dac36a190de8@oss.qualcomm.com/
Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
Changes in v2:
- Fix typo for mclk2 and mclk4 tlmm function
- Link to v1: https://lore.kernel.org/r/20260519-pakala-camera-v1-0-b6d897864916@oss.qualcomm.com
---
Hangxiang Ma (3):
arm64: dts: qcom: sm8750: Add camss node
arm64: dts: qcom: sm8750: Add CCI definitions
arm64: dts: qcom: sm8750: Add camera MCLK pinctrl
arch/arm64/boot/dts/qcom/sm8750.dtsi | 541 +++++++++++++++++++++++++++++++++++
1 file changed, 541 insertions(+)
---
base-commit: fb80987f81756cb697a01362654a8e55fb505600
change-id: 20260518-pakala-camera-bf03e3df1db8
Best regards,
--
Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
^ permalink raw reply
* [PATCH v5 2/2] hw_random: timeriomem-rng: add configurable read width and data mask
From: Jad Keskes @ 2026-06-18 12:01 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivia Mackall,
Herbert Xu
Cc: Alexander Clouter, devicetree, linux-crypto, linux-kernel,
Jad Keskes
In-Reply-To: <20260618120110.36439-1-inasj268@gmail.com>
The TODO for supporting read sizes other than 32 bits and masking has
been sitting in this driver since 2009. Implement it.
Add reg-io-width (1, 2, or 4 bytes) and mask support. The read loop
dispatches on width using readb/readw/readl so a configured 1-byte
access does not trigger a bus error on hardware that rejects 32-bit
reads to that address. The mask is ANDed with the value before storing.
These are platform properties, not runtime policy -- width depends on
SoC integration, mask reflects which output bits carry entropy.
The alignment check in probe is updated to verify the resource is
aligned to the configured width instead of hardcoding 4-byte alignment.
Signed-off-by: Jad Keskes <inasj268@gmail.com>
---
v5: No changes since v4
v4: Initial version with reg-io-width (bytes) and readb/readw/readl
v3: Added configurable width (bits) with mask
v2: Rebased
v1: Initial submission
---
drivers/char/hw_random/timeriomem-rng.c | 77 ++++++++++++++++++++-----
include/linux/timeriomem-rng.h | 12 ++++
2 files changed, 76 insertions(+), 13 deletions(-)
diff --git a/drivers/char/hw_random/timeriomem-rng.c b/drivers/char/hw_random/timeriomem-rng.c
index e61f06393209..42393409f22a 100644
--- a/drivers/char/hw_random/timeriomem-rng.c
+++ b/drivers/char/hw_random/timeriomem-rng.c
@@ -14,7 +14,9 @@
* has to do is provide the address and 'wait time' that new data becomes
* available.
*
- * TODO: add support for reading sizes other than 32bits and masking
+ * The read width (8, 16, or 32 bits) and an optional data mask can be
+ * configured through platform data or device tree properties. Default is
+ * 32-bit reads with no mask.
*/
#include <linux/completion.h>
@@ -34,6 +36,8 @@ struct timeriomem_rng_private {
void __iomem *io_base;
ktime_t period;
unsigned int present:1;
+ unsigned int reg_io_width;
+ u32 mask;
struct hrtimer timer;
struct completion completion;
@@ -48,6 +52,7 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
container_of(hwrng, struct timeriomem_rng_private, rng_ops);
int retval = 0;
int period_us = ktime_to_us(priv->period);
+ int chunk = priv->reg_io_width;
/*
* There may not have been enough time for new data to be generated
@@ -71,11 +76,28 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
usleep_range(period_us,
period_us + max(1, period_us / 100));
- *(u32 *)data = readl(priv->io_base);
- retval += sizeof(u32);
- data += sizeof(u32);
- max -= sizeof(u32);
- } while (wait && max > sizeof(u32));
+ switch (priv->reg_io_width) {
+ case 1: {
+ u8 val = readb(priv->io_base) & priv->mask;
+ *(u8 *)data = val;
+ break;
+ }
+ case 2: {
+ u16 val = readw(priv->io_base) & priv->mask;
+ *(u16 *)data = val;
+ break;
+ }
+ case 4: {
+ u32 val = readl(priv->io_base) & priv->mask;
+ *(u32 *)data = val;
+ break;
+ }
+ }
+
+ retval += chunk;
+ data += chunk;
+ max -= chunk;
+ } while (wait && max > chunk);
/*
* Block any new callers until the RNG has had time to generate new
@@ -125,11 +147,8 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
if (IS_ERR(priv->io_base))
return PTR_ERR(priv->io_base);
- if (res->start % 4 != 0 || resource_size(res) < 4) {
- dev_err(&pdev->dev,
- "address must be at least four bytes wide and 32-bit aligned\n");
- return -EINVAL;
- }
+ priv->reg_io_width = 4;
+ priv->mask = 0xFFFFFFFF;
if (pdev->dev.of_node) {
int i;
@@ -145,9 +164,41 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
if (!of_property_read_u32(pdev->dev.of_node,
"quality", &i))
priv->rng_ops.quality = i;
+
+ of_property_read_u32(pdev->dev.of_node,
+ "reg-io-width", &priv->reg_io_width);
+ of_property_read_u32(pdev->dev.of_node,
+ "mask", &priv->mask);
} else {
period = pdata->period;
priv->rng_ops.quality = pdata->quality;
+
+ if (pdata->reg_io_width_set)
+ priv->reg_io_width = pdata->reg_io_width;
+ if (pdata->mask_set)
+ priv->mask = pdata->mask;
+ }
+
+ if (priv->reg_io_width == 0)
+ priv->reg_io_width = 4;
+
+ switch (priv->reg_io_width) {
+ case 1:
+ case 2:
+ case 4:
+ break;
+ default:
+ dev_err(&pdev->dev, "invalid reg-io-width %u, must be 1, 2, or 4\n",
+ priv->reg_io_width);
+ return -EINVAL;
+ }
+
+ if (!IS_ALIGNED(res->start, priv->reg_io_width) ||
+ resource_size(res) < priv->reg_io_width) {
+ dev_err(&pdev->dev,
+ "address must be %u-byte aligned\n",
+ priv->reg_io_width);
+ return -EINVAL;
}
priv->period = us_to_ktime(period);
@@ -167,8 +218,8 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
return err;
}
- dev_info(&pdev->dev, "32bits from 0x%p @ %dus\n",
- priv->io_base, period);
+ dev_info(&pdev->dev, "%u-byte from %p @ %dus\n",
+ priv->reg_io_width, priv->io_base, period);
return 0;
}
diff --git a/include/linux/timeriomem-rng.h b/include/linux/timeriomem-rng.h
index 672df7fbf6c1..5732489a17a1 100644
--- a/include/linux/timeriomem-rng.h
+++ b/include/linux/timeriomem-rng.h
@@ -16,6 +16,18 @@ struct timeriomem_rng_data {
/* bits of entropy per 1024 bits read */
unsigned int quality;
+
+ /* read width (1, 2, or 4 bytes), 0 means 4 */
+ unsigned int reg_io_width;
+
+ /* set to true if reg-io-width is explicitly provided */
+ bool reg_io_width_set;
+
+ /* mask applied to raw read value */
+ u32 mask;
+
+ /* set to true if mask is explicitly provided */
+ bool mask_set;
};
#endif /* _LINUX_TIMERIOMEM_RNG_H */
--
2.54.0
^ permalink raw reply related
* [PATCH v5 1/2] dt-bindings: rng: timeriomem_rng: add reg-io-width and mask properties
From: Jad Keskes @ 2026-06-18 12:01 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivia Mackall,
Herbert Xu
Cc: Alexander Clouter, devicetree, linux-crypto, linux-kernel,
Jad Keskes
Add optional reg-io-width (1, 2, or 4 bytes) and mask properties.
reg-io-width selects the bus access size. mask is ANDed with the raw
register value to allow only the entropy-bearing bits through.
Update the example to show a 1-byte configuration.
Signed-off-by: Jad Keskes <inasj268@gmail.com>
---
v5:
- Fix description to describe hardware, not the binding
- Drop SPDX dual-license change
- Merge examples into one
v4: Initial version with reg-io-width (replaced custom width from v3)
v3: Changed from custom width to reg-io-width per dt-bindings convention
v2: Split DT binding and driver into separate patches
v1: Initial submission
---
.../bindings/rng/timeriomem_rng.yaml | 50 ++++++++++++++-----
1 file changed, 37 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml b/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
index 4754174e9849..7f0068f785b7 100644
--- a/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
+++ b/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
@@ -4,7 +4,11 @@
$id: http://devicetree.org/schemas/rng/timeriomem_rng.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: TimerIO Random Number Generator
+title: Timer IOMEM hardware random number generator
+
+description: |
+ A device that provides random data via a single memory-mapped IO register.
+ A new value becomes available at a fixed interval.
maintainers:
- Krzysztof Kozlowski <krzk@kernel.org>
@@ -13,29 +17,47 @@ properties:
compatible:
const: timeriomem_rng
+ reg:
+ maxItems: 1
+ description:
+ Base address to sample from. Must be aligned to the configured access
+ width (1, 2, or 4 bytes) and at least that wide.
+
period:
$ref: /schemas/types.yaml#/definitions/uint32
- description: wait time in microseconds to use between samples
+ description:
+ Interval in microseconds between random value updates.
quality:
$ref: /schemas/types.yaml#/definitions/uint32
default: 0
description:
- Estimated number of bits of true entropy per 1024 bits read from the rng.
- Defaults to zero which causes the kernel's default quality to be used
- instead. Note that the default quality is usually zero which disables
- using this rng to automatically fill the kernel's entropy pool.
+ Estimated number of bits of true entropy per 1024 bits read from the
+ device. Defaults to zero which causes the kernel's default quality to
+ be used instead. Note that the default quality is usually zero which
+ disables using this RNG to automatically fill the kernel's entropy
+ pool.
- reg:
- maxItems: 1
+ reg-io-width:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 4
+ enum: [1, 2, 4]
description:
- Base address to sample from. Currently 'reg' must be at least four bytes
- wide and 32-bit aligned.
+ Access width in bytes. Determines whether the read is performed as
+ an 8-bit, 16-bit, or 32-bit bus access.
+
+ mask:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 0xFFFFFFFF
+ description:
+ Mask applied to the value read from the register. Bits set to 0 in
+ the mask are cleared in the output data. Default (no mask) passes
+ all bits through.
required:
- compatible
- - period
- reg
+ - period
additionalProperties: false
@@ -43,6 +65,8 @@ examples:
- |
rng@44 {
compatible = "timeriomem_rng";
- reg = <0x44 0x04>;
- period = <1000000>;
+ reg = <0x44 0x01>;
+ period = <50000>;
+ reg-io-width = <1>;
+ mask = <0xFF>;
};
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] arm64: dts: qcom: glymur: add TRNG node
From: Harshal Dev @ 2026-06-18 11:58 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Kuldeep Singh, linux-arm-msm, linux-crypto, devicetree,
linux-kernel, Konrad Dybcio, Herbert Xu, David S. Miller,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vinod Koul,
Konrad Dybcio, Dmitry Baryshkov
In-Reply-To: <0debc1fb-f6ae-44c6-aa87-d5ef3e39b47d@oss.qualcomm.com>
Hi Bjorn,
On 6/9/2026 12:06 PM, Harshal Dev wrote:
> Hello Bjorn,
>
> On 5/18/2026 2:06 PM, Harshal Dev wrote:
>> Hi Bjorn,
>>
>> On 4/24/2026 2:05 PM, Harshal Dev wrote:
>>> Glymur has a True Random Number Generator, add the node with the correct
>>> compatible set.
>>>
>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>> Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
>>> ---
>>> arch/arm64/boot/dts/qcom/glymur.dtsi | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
>>> index f23cf81ddb77..64bbd5691229 100644
>>> --- a/arch/arm64/boot/dts/qcom/glymur.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
>>> @@ -3675,6 +3675,11 @@ pcie3b_phy: phy@f10000 {
>>> status = "disabled";
>>> };
>>>
>>> + rng: rng@10c3000 {
>>> + compatible = "qcom,glymur-trng", "qcom,trng";
>>> + reg = <0x0 0x010c3000 0x0 0x1000>;
>>> + };
>>> +
>>> tcsr_mutex: hwlock@1f40000 {
>>> compatible = "qcom,tcsr-mutex";
>>> reg = <0x0 0x01f40000 0x0 0x20000>;
>>>
>>
>> A gentle reminder to pick this patch for the 7.2 merge window.
>>
>
> Another reminder to pick this patch up in-case you've missed it.
>
Gentle reminder.
Thanks,
Harshal
> Thanks,
> Harshal
>
>> Regards,
>> Harshal
>
^ permalink raw reply
* Re: [PATCH] dma-iommu: Introduce API to reserve IOVA regions for dynamically created devices
From: Krzysztof Kozlowski @ 2026-06-18 11:57 UTC (permalink / raw)
To: Jason Gunthorpe, Vishnu Reddy, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: Vikash Garodia, Robin Murphy, joro, will, m.szyprowski, iommu,
linux-kernel, dikshita.agarwal
In-Reply-To: <20260617194038.GC231643@ziepe.ca>
On 17/06/2026 21:40, Jason Gunthorpe wrote:
> On Mon, Jun 15, 2026 at 11:21:45PM +0530, Vishnu Reddy wrote:
>> Hi Jason,
>>
>> Here, the parent node does not have an iommus property — it only has iommu-map,
>> like example below:
>>
>> iommu-map = <0x0 &apps_smmu 0x1940 0x0 0x1>, /* function_id 0 → SID 0x1940 */
>> <0x1 &apps_smmu 0x1943 0x0 0x1>, /* function_id 1 → SID 0x1943 */
>> <0x2 &apps_smmu 0x1944 0x0 0x1>; /* function_id 2 → SID 0x1944 */
>>
>> When the parent device is probed, of_dma_configure() is called, which
>> internally invokes of_dma_configure_id() with NULL as the function ID. Since
>> there is no iommus entry, no stream ID gets mapped to the parent device.
>
> Sure, but it still did of_dma_configure on the parent..
>
>> The child devices are created at runtime and have no of_node of their own. The
>> only place the iommu-map property exists is on the parent's of_node. So when
>> configuring a child device, we pass the parent's of_node along with the child's
>> specific function_id — this is how of_dma_configure_id() finds and maps the
>> correct stream ID to the child device only.
>
> The whole thing seems like the wrong way to use DT to me. Having an
> iommu-map in the dt that no iommus property ever uses strikes me as
> abusive.
Same with interrupt-map.
There are PCIe controller nodes which have interrupt-map and no
interrupts property ever uses them.
https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/imx8mm.dtsi?h=v7.1#n1340
There might be DT children but they still won't have interrupts at all.
>
> Then hard coding the ID table and manually creating the missing
> struct devices in C code is a throw back to board files :(
There are no distinctive, specific child devices here. Devices in terms
of hardware. There are no resources of children, they are not real
hardware devices.
You mentioned earlier that DT description is incomplete or not proper. I
disagree. Adding any child here just to configure IOMMU differently is
abuse of DT - creating fake device nodes to overcome somehow Linux
limitations.
>
> Of course it doesn't fully work, it was never intended to be used like
> this.
>
> Why the resistance to doing DT properly with actual iommus and dma
> ranges for each and every stream your device needs?
Because DT person - me - told that creating child device nodes just to
configure iommus is abuse of DT. There are no child devices in terms of
hardware or firmware. The iommu ranges here are no real hardware.
However, said all this, since I pushed folks to come with the iommu-map
approach, I will revoke my disagreement to child device nodes in DT, if
you really believe that is the approach. IOW, I will agree to device
nodes in DT representing fake hardware-children, just for the sake of
Linux driver model limitations.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/9] drm/rockchip: vop2: Reset AXI and DCLK to improve robustness
From: Philipp Zabel @ 2026-06-18 11:52 UTC (permalink / raw)
To: Cristian Ciocaltea, Sandy Huang, Heiko Stübner, Andy Yan,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli
Cc: kernel, Andy Yan, dri-devel, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel
In-Reply-To: <302d42b0-108e-4936-bc34-49b9194985f1@collabora.com>
On Do, 2026-06-18 at 14:46 +0300, Cristian Ciocaltea wrote:
> On 6/18/26 12:39 PM, Philipp Zabel wrote:
> > On Mi, 2026-06-17 at 21:51 +0300, Cristian Ciocaltea wrote:
> > > Assert the AXI reset in the CRTC disable path, and the VP DCLK reset in
> > > the enable path.
> > >
> > > These resets are intended to leave the hardware in a clean state for the
> > > next use, helping recover from exceptions such as IOMMU page faults, as
> > > well as to prevent random display output glitches, such as a blank
> > > image, observed when switching modes that also change the color format,
> > > e.g. from RGB to YUV420 and vice versa.
> > >
> > > For now this seems to affect only the RK3588, hence the resets are
> > > optional and will be provided in the device tree for this SoC only.
> > >
> > > Co-developed-by: Andy Yan <andy.yan@rock-chips.com>
> > > Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > > ---
> > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 35 ++++++++++++++++++++++++++++
> > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 4 ++++
> > > 2 files changed, 39 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > index 4cce3e336f5b..2833fb49ad81 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > @@ -17,6 +17,7 @@
> > > #include <linux/platform_device.h>
> > > #include <linux/pm_runtime.h>
> > > #include <linux/regmap.h>
> > > +#include <linux/reset.h>
> > > #include <linux/swab.h>
> > >
> > > #include <drm/drm.h>
> > > @@ -860,6 +861,26 @@ static int vop2_core_clks_prepare_enable(struct vop2 *vop2)
> > > return ret;
> > > }
> > >
> > > +static void vop2_clk_reset(struct vop2 *vop2, struct reset_control *rstc)
> >
> > The _clk part of the function name is misleading ...
>
> Ack.
>
> We need to make this clearly distinct from another similarly named function,
> vop2_crtc_reset(), hence I'd propose:
>
> - vop2_reset_assert_deassert()
> - vop2_reset_cycle()
> - vop2_do_reset()
>
> Any preference / alternative suggestions?
Or vop2_reset_control_reset(). The three above are equally fine.
regards
Philipp
^ permalink raw reply
* Re: [PATCH v3 3/3] media: i2c: os02g10: implement crop handling with set_selection
From: Sakari Ailus @ 2026-06-18 11:47 UTC (permalink / raw)
To: Elgin Perumbilly
Cc: laurent.pinchart, Tarang Raval, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hans Verkuil,
Hans de Goede, Vladimir Zapolskiy, Mehdi Djait, Sylvain Petinot,
Benjamin Mugnier, Bryan O'Donoghue, Heimir Thor Sverrisson,
Hardevsinh Palaniya, linux-media, devicetree, linux-kernel
In-Reply-To: <20260424092554.26130-4-elgin.perumbilly@siliconsignals.io>
Hi Elgin,
On Fri, Apr 24, 2026 at 02:55:47PM +0530, Elgin Perumbilly wrote:
> From: Tarang Raval <tarang.raval@siliconsignals.io>
>
> Add crop support to os02g10 by implementing .set_selection() and
> storing the crop rectangle in subdev state.
>
> Initialize the default crop to the active area, make set_fmt() use the
> current crop, and update the output format when the crop size changes.
> Also program the sensor window from the active crop/format state instead
> of using the fixed supported_modes entry.
>
> This allows userspace to configure the sensor crop window explicitly.
Please wait for the Common Raw Sensor Model patches to be merged before
adding this -- we don't have an established way to configure cropping
before that. Some drivers might do something but it's all a bit haphazard.
--
Kind regards,
Sakari Ailus
^ permalink raw reply
* Re: [PATCH 2/9] drm/rockchip: vop2: Reset AXI and DCLK to improve robustness
From: Cristian Ciocaltea @ 2026-06-18 11:46 UTC (permalink / raw)
To: Philipp Zabel, Sandy Huang, Heiko Stübner, Andy Yan,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli
Cc: kernel, Andy Yan, dri-devel, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel
In-Reply-To: <7c79f233c863654b0266de4b5ec5a8c72bb17715.camel@pengutronix.de>
On 6/18/26 12:39 PM, Philipp Zabel wrote:
> On Mi, 2026-06-17 at 21:51 +0300, Cristian Ciocaltea wrote:
>> Assert the AXI reset in the CRTC disable path, and the VP DCLK reset in
>> the enable path.
>>
>> These resets are intended to leave the hardware in a clean state for the
>> next use, helping recover from exceptions such as IOMMU page faults, as
>> well as to prevent random display output glitches, such as a blank
>> image, observed when switching modes that also change the color format,
>> e.g. from RGB to YUV420 and vice versa.
>>
>> For now this seems to affect only the RK3588, hence the resets are
>> optional and will be provided in the device tree for this SoC only.
>>
>> Co-developed-by: Andy Yan <andy.yan@rock-chips.com>
>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 35 ++++++++++++++++++++++++++++
>> drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 4 ++++
>> 2 files changed, 39 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> index 4cce3e336f5b..2833fb49ad81 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> @@ -17,6 +17,7 @@
>> #include <linux/platform_device.h>
>> #include <linux/pm_runtime.h>
>> #include <linux/regmap.h>
>> +#include <linux/reset.h>
>> #include <linux/swab.h>
>>
>> #include <drm/drm.h>
>> @@ -860,6 +861,26 @@ static int vop2_core_clks_prepare_enable(struct vop2 *vop2)
>> return ret;
>> }
>>
>> +static void vop2_clk_reset(struct vop2 *vop2, struct reset_control *rstc)
>
> The _clk part of the function name is misleading ...
Ack.
We need to make this clearly distinct from another similarly named function,
vop2_crtc_reset(), hence I'd propose:
- vop2_reset_assert_deassert()
- vop2_reset_cycle()
- vop2_do_reset()
Any preference / alternative suggestions?
>
> [...]
>> @@ -938,6 +959,8 @@ static void vop2_disable(struct vop2 *vop2)
>> {
>> rockchip_drm_dma_detach_device(vop2->drm, vop2->dev);
>>
>> + vop2_clk_reset(vop2, vop2->axi_rst);
>
> ... because this function is also called with the AXI reset control.
>
>> +
>> pm_runtime_put_sync(vop2->dev);
>>
>> regcache_drop_region(vop2->map, 0, vop2_regmap_config.max_register);
>> @@ -1948,6 +1971,8 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
>>
>> vop2_crtc_atomic_try_set_gamma(vop2, vp, crtc, crtc_state);
>>
>> + vop2_clk_reset(vop2, vp->dclk_rst);
>> +
>> drm_crtc_vblank_on(crtc);
>>
>> vop2_unlock(vop2);
>> @@ -2531,6 +2556,11 @@ static int vop2_create_crtcs(struct vop2 *vop2)
>> return dev_err_probe(drm->dev, PTR_ERR(vp->dclk),
>> "failed to get %s\n", dclk_name);
>>
>> + vp->dclk_rst = devm_reset_control_get_optional(vop2->dev, dclk_name);
>
> Please use devm_reset_control_get_optional_exclusive() directly.
Thanks for pointing this out, I missed the comment mentioning the explicit API
transition.
>> + if (IS_ERR(vp->dclk_rst))
>> + return dev_err_probe(drm->dev, PTR_ERR(vp->dclk_rst),
>> + "failed to get %s reset\n", dclk_name);
>> +
>> np = of_graph_get_remote_node(dev->of_node, i, -1);
>> if (!np) {
>> drm_dbg(vop2->drm, "%s: No remote for vp%d\n", __func__, i);
>> @@ -2890,6 +2920,11 @@ static int vop2_bind(struct device *dev, struct device *master, void *data)
>> return dev_err_probe(drm->dev, PTR_ERR(vop2->pll_hdmiphy1),
>> "failed to get pll_hdmiphy1\n");
>>
>> + vop2->axi_rst = devm_reset_control_get_optional(vop2->dev, "axi");
>
> Same as above, devm_reset_control_get_optional_exclusive().
Ack.
Thanks for reviewing,
Cristian
^ permalink raw reply
* Re: [PATCH v3 2/3] media: i2c: add os02g10 image sensor driver
From: Laurent Pinchart @ 2026-06-18 11:46 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Elgin Perumbilly, sakari.ailus@linux.intel.com, Tarang Raval,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hans Verkuil, Hans de Goede, Mehdi Djait,
Sylvain Petinot, Benjamin Mugnier, Bryan O'Donoghue,
Himanshu Bhavani, Heimir Thor Sverrisson, Jingjing Xiong,
Svyatoslav Ryhel, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <fa5eb21d-ea67-47c9-b00e-6b9060e0c5f0@linaro.org>
On Thu, Jun 18, 2026 at 02:06:20PM +0300, Vladimir Zapolskiy wrote:
> On 6/18/26 09:22, Elgin Perumbilly wrote:
> > Hi Vladimir,
> >
> > Thank you for the review.
> >
> > I have addressed all of the comments except for two, where I am not entirely
> > sure about the requested changes. Could you please take a look at the points
> > below and let me know your opinion?
> >
> >> On 4/24/26 12:25, Elgin Perumbilly wrote:
> >>> Add a v4l2 subdevice driver for the Omnivision os02g10 sensor.
> >>>
> >>> The Omnivision os02g10 is a CMOS image sensor with an active array size of
> >>> 1920 x 1080.
> >>>
> >>> The following features are supported:
> >>> - Manual exposure an gain control support
> >>> - vblank/hblank control support
> >>> - vflip/hflip control support
> >>> - Test pattern control support
> >>> - Supported resolution: 1920 x 1080 @ 30fps (SBGGR10)
> >>>
> >>> Signed-off-by: Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>
> >>> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
> >
> > ...
> >
> >>> +#include <linux/array_size.h>
> >>> +#include <linux/bitops.h>
> >>> +#include <linux/cleanup.h>
> >>> +#include <linux/clk.h>
> >>> +#include <linux/container_of.h>
> >>> +#include <linux/delay.h>
> >>> +#include <linux/err.h>
> >>> +#include <linux/gpio/consumer.h>
> >>> +#include <linux/i2c.h>
> >>> +#include <linux/module.h>
> >>> +#include <linux/mutex.h>
> >>> +#include <linux/pm_runtime.h>
> >>> +#include <linux/property.h>
> >>> +#include <linux/regulator/consumer.h>
> >>> +#include <linux/units.h>
> >>> +#include <linux/types.h>
> >>> +#include <linux/time.h>
> >>> +#include <linux/regmap.h>
> >>
> >> Please sort the list of includes in alphabetical order, also you
> >> may consider to shrink the list by removing quite many inherited
> >> includes.
> >
> > Some maintainers prefer the "include what you use" approach, like Andy,
> > so I added all the headers that are directly used. Should I now remove
> > any inherited includes?
>
> Yes, here opinions may vary, that's why I asked for sorting and to
Sorting is a good idea.
> consider to remove some of the redundant headers. In my personal opinion
> this type of excessive information is not needed, especially if it is
> justified only by probable and far future trivial clean-up work.
I typically ask for a "include what you use" approach too, to avoid
build breakages. It's not only a matter of future work, but indirect
includes can also vary based on the kernel configuration (and the
architecture).
> >>> +#include <media/v4l2-cci.h>
> >>> +#include <media/v4l2-ctrls.h>
> >>> +#include <media/v4l2-device.h>
> >>> +#include <media/v4l2-fwnode.h>
> >>> +#include <media/v4l2-mediabus.h>
> >
> > ...
> >
> >>> +static int os02g10_set_framefmt(struct os02g10 *os02g10,
> >>> + struct v4l2_subdev_state *state)
> >>> +{
> >>> + const struct v4l2_mbus_framefmt *format;
> >>> + const struct os02g10_mode *mode;
> >>> + int ret = 0;
> >>> +
> >>> + format = v4l2_subdev_state_get_format(state, 0);
> >>> + mode = v4l2_find_nearest_size(supported_modes,
> >>> + ARRAY_SIZE(supported_modes), width,
> >>> + height, format->width, format->height);
> >>> +
> >>> + cci_write(os02g10->cci, OS02G10_REG_V_START, mode->y_start, &ret);
> >>> + cci_write(os02g10->cci, OS02G10_REG_V_SIZE, mode->height, &ret);
> >>> + cci_write(os02g10->cci, OS02G10_REG_V_SIZE_MIPI, mode->height, &ret);
> >>> + cci_write(os02g10->cci, OS02G10_REG_H_START, mode->x_start, &ret);
> >>> + cci_write(os02g10->cci, OS02G10_REG_H_SIZE, mode->width, &ret);
> >>> + cci_write(os02g10->cci, OS02G10_REG_H_SIZE_MIPI, mode->width, &ret);
> >>> +
> >>> + return ret;
> >>
> >> Just "return 0" here, and remove the local variable.
> >
> > Could you clarify why this should return 0? The local ret is passed to all
> > cci_write() calls so that any write error is propagated. Returning 0 here
> > would appear to suppress those errors and always report success.
>
> My bad, yes, here please leave 'return ret' as is, I was confused and
> misleaded by initialization of the local variable to zero, which is
> redundant, and I'd suggest to remove this initialization.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v3 2/3] media: i2c: add os02g10 image sensor driver
From: Sakari Ailus @ 2026-06-18 11:43 UTC (permalink / raw)
To: Elgin Perumbilly
Cc: laurent.pinchart, Tarang Raval, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hans Verkuil,
Hans de Goede, Vladimir Zapolskiy, Mehdi Djait, Sylvain Petinot,
Benjamin Mugnier, Bryan O'Donoghue, Himanshu Bhavani,
Heimir Thor Sverrisson, Jingjing Xiong, Svyatoslav Ryhel,
linux-media, devicetree, linux-kernel
In-Reply-To: <20260424092554.26130-3-elgin.perumbilly@siliconsignals.io>
Hi Elgin,
Thanks for the update. A few minor comments below...
On Fri, Apr 24, 2026 at 02:55:46PM +0530, Elgin Perumbilly wrote:
> Add a v4l2 subdevice driver for the Omnivision os02g10 sensor.
>
> The Omnivision os02g10 is a CMOS image sensor with an active array size of
> 1920 x 1080.
>
> The following features are supported:
> - Manual exposure an gain control support
> - vblank/hblank control support
> - vflip/hflip control support
> - Test pattern control support
> - Supported resolution: 1920 x 1080 @ 30fps (SBGGR10)
>
> Signed-off-by: Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>
> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
> ---
> MAINTAINERS | 1 +
> drivers/media/i2c/Kconfig | 10 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/os02g10.c | 949 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 961 insertions(+)
> create mode 100644 drivers/media/i2c/os02g10.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8a0a55073c30..693e71b51926 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19449,6 +19449,7 @@ M: Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>
> L: linux-media@vger.kernel.org
> S: Maintained
> F: Documentation/devicetree/bindings/media/i2c/ovti,os02g10.yaml
> +F: drivers/media/i2c/os02g10.c
>
> OMNIVISION OS05B10 SENSOR DRIVER
> M: Himanshu Bhavani <himanshu.bhavani@siliconsignals.io>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 5eb1e0e0a87a..dd6e9562acf6 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -372,6 +372,16 @@ config VIDEO_OG0VE1B
> To compile this driver as a module, choose M here: the
> module will be called og0ve1b.
>
> +config VIDEO_OS02G10
> + tristate "OmniVision OS02G10 sensor support"
> + select V4L2_CCI_I2C
> + help
> + This is a Video4Linux2 sensor driver for Omnivision
> + OS02G10 camera sensor.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called os02g10.
> +
> config VIDEO_OS05B10
> tristate "OmniVision OS05B10 sensor support"
> select V4L2_CCI_I2C
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index a3a6396df3c4..a7554d2eb140 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -84,6 +84,7 @@ obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o
> obj-$(CONFIG_VIDEO_MT9V111) += mt9v111.o
> obj-$(CONFIG_VIDEO_OG01A1B) += og01a1b.o
> obj-$(CONFIG_VIDEO_OG0VE1B) += og0ve1b.o
> +obj-$(CONFIG_VIDEO_OS02G10) += os02g10.o
> obj-$(CONFIG_VIDEO_OS05B10) += os05b10.o
> obj-$(CONFIG_VIDEO_OV01A10) += ov01a10.o
> obj-$(CONFIG_VIDEO_OV02A10) += ov02a10.o
> diff --git a/drivers/media/i2c/os02g10.c b/drivers/media/i2c/os02g10.c
> new file mode 100644
> index 000000000000..fad2dd0ad7aa
> --- /dev/null
> +++ b/drivers/media/i2c/os02g10.c
> @@ -0,0 +1,949 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * V4L2 Support for the OS02G10
> + *
> + * Copyright (C) 2026 Silicon Signals Pvt. Ltd.
> + *
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bitops.h>
> +#include <linux/cleanup.h>
> +#include <linux/clk.h>
> +#include <linux/container_of.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/units.h>
> +#include <linux/types.h>
> +#include <linux/time.h>
> +#include <linux/regmap.h>
> +
> +#include <media/v4l2-cci.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mediabus.h>
> +
> +#define OS02G10_XCLK_FREQ (24 * HZ_PER_MHZ)
> +
> +/* Page 0 */
> +#define OS02G10_REG_CHIPID CCI_REG24(0x002)
> +#define OS02G10_CHIPID 0x560247
> +
> +#define OS02G10_REG_PLL_DIV_CTRL CCI_REG8(0x030)
> +#define OS02G10_REG_PLL_DCTL_BIAS_CTRL CCI_REG8(0x035)
> +#define OS02G10_REG_GATE_EN_CTRL CCI_REG8(0x038)
> +#define OS02G10_REG_DPLL_NC CCI_REG8(0x041)
> +#define OS02G10_REG_MP_PHASE_CTRL CCI_REG8(0x044)
> +
> +/* Page 1 */
> +#define OS02G10_REG_STREAM_CTRL CCI_REG8(0x1b1)
> +#define OS02G10_STREAM_CTRL_ON 0x03
> +#define OS02G10_STREAM_CTRL_OFF 0x00
> +
> +#define OS02G10_REG_FRAME_SYNC CCI_REG8(0x101)
> +
> +#define OS02G10_REG_FRAME_LENGTH CCI_REG16(0x10e)
> +#define OS02G10_FRAME_LENGTH_MAX 0xffff
> +#define OS02G10_REG_HBLANK CCI_REG16(0x109)
> +
> +#define OS02G10_REG_FRAME_TEST_CTRL CCI_REG8(0x10d)
> +#define OS02G10_FRAME_EXP_SEPERATE_EN BIT(4)
> +#define OS02G10_TEST_PATTERN_ENABLE BIT(0)
> +
> +#define OS02G10_REG_ULP_PWD_DUMMY_CTRL CCI_REG8(0x13c)
> +#define OS02G10_REG_DC_LEVEL_LIMIT_EN CCI_REG8(0x146)
> +#define OS02G10_REG_DC_LEVEL_LIMIT_L CCI_REG8(0x147)
> +#define OS02G10_REG_BLC_DATA_LIMIT_L CCI_REG8(0x148)
> +#define OS02G10_REG_DC_BLC_LIMIT_H CCI_REG8(0x149)
> +
> +#define OS02G10_REG_HS_LP_CTRL CCI_REG8(0x192)
> +#define OS02G10_REG_HS_LEVEL CCI_REG8(0x19d)
> +#define OS02G10_REG_HS_DRV CCI_REG8(0x19e)
> +
> +#define OS02G10_REG_GB_SUBOFFSET CCI_REG8(0x1f0)
> +#define OS02G10_REG_BLUE_SUBOFFSET CCI_REG8(0x1f1)
> +#define OS02G10_REG_RED_SUBOFFSET CCI_REG8(0x1f2)
> +#define OS02G10_REG_GR_SUBOFFSET CCI_REG8(0x1f3)
> +
> +#define OS02G10_REG_ABL_TRIGGER CCI_REG8(0x1fa)
> +#define OS02G10_REG_ABL CCI_REG8(0x1fb)
> +
> +#define OS02G10_REG_H_SIZE_MIPI CCI_REG16(0x18e)
> +#define OS02G10_REG_V_SIZE_MIPI CCI_REG16(0x190)
> +#define OS02G10_REG_MIPI_TX_SPEED_CTRL CCI_REG8(0x1a1)
> +
> +#define OS02G10_REG_LONG_EXPOSURE CCI_REG16(0x103)
> +#define OS02G10_EXPOSURE_MIN 4
> +#define OS02G10_EXPOSURE_STEP 1
> +#define OS02G10_EXPOSURE_MARGIN 9
> +
> +#define OS02G10_REG_ANALOG_GAIN CCI_REG8(0x124)
> +#define OS02G10_ANALOG_GAIN_MIN 0x10
> +#define OS02G10_ANALOG_GAIN_MAX 0xf8
> +#define OS02G10_ANALOG_GAIN_STEP 1
> +#define OS02G10_ANALOG_GAIN_DEFAULT 0x10
> +
> +#define OS02G10_REG_DIGITAL_GAIN_H CCI_REG8(0x137)
> +#define OS02G10_REG_DIGITAL_GAIN_L CCI_REG8(0x139)
> +#define OS02G10_DIGITAL_GAIN_MIN 0x40
> +#define OS02G10_DIGITAL_GAIN_MAX 0x800
> +#define OS02G10_DIGITAL_GAIN_STEP 64
> +#define OS02G10_DIGITAL_GAIN_DEFAULT 0x40
> +
> +#define OS02G10_REG_FLIP_MIRROR CCI_REG8(0x13f)
> +#define OS02G10_FLIP BIT(1)
> +#define OS02G10_MIRROR BIT(0)
> +
> +/* Page 2 */
> +#define OS02G10_REG_V_START CCI_REG16(0x2a0)
> +#define OS02G10_REG_V_SIZE CCI_REG16(0x2a2)
> +#define OS02G10_REG_H_START CCI_REG16(0x2a4)
> +#define OS02G10_REG_H_SIZE CCI_REG16(0x2a6)
> +
> +#define OS02G10_REG_SIF_CTRL CCI_REG8(0x25e)
> +#define OS02G10_ORIENTATION_BAYER_FIX 0x32
> +
> +#define OS02G10_LINK_FREQ_720MHZ (720 * HZ_PER_MHZ)
> +
> +/* OS02G10 native and active pixel array size */
> +static const struct v4l2_rect os02g10_native_area = {
> + .top = 0,
> + .left = 0,
> + .width = 1928,
> + .height = 1088,
> +};
> +
> +static const struct v4l2_rect os02g10_active_area = {
> + .top = 4,
> + .left = 4,
> + .width = 1920,
> + .height = 1080,
> +};
> +
> +static const char * const os02g10_supply_name[] = {
> + "avdd", /* Analog power */
> + "dovdd", /* Digital I/O power */
> + "dvdd", /* Digital core power */
> +};
> +
> +struct os02g10 {
> + struct device *dev;
> + struct regmap *cci;
> + struct v4l2_subdev sd;
> + struct media_pad pad;
> + struct clk *xclk;
> + struct gpio_desc *reset_gpio;
> + struct regulator_bulk_data supplies[ARRAY_SIZE(os02g10_supply_name)];
> +
> + /* V4L2 Controls */
> + struct v4l2_ctrl_handler handler;
> + struct v4l2_ctrl *link_freq;
> + struct v4l2_ctrl *vblank;
> + struct v4l2_ctrl *exposure;
> + struct v4l2_ctrl *vflip;
> + struct v4l2_ctrl *hflip;
> +};
> +
> +struct os02g10_mode {
> + u32 width;
> + u32 height;
> + u32 vts_def;
> + u32 exp_def;
> + u32 x_start;
> + u32 y_start;
> +};
> +
> +static const struct cci_reg_sequence os02g10_common_regs[] = {
> + { OS02G10_REG_PLL_DIV_CTRL, 0x0a},
> + { OS02G10_REG_PLL_DCTL_BIAS_CTRL, 0x04},
> + { OS02G10_REG_GATE_EN_CTRL, 0x11},
> + { OS02G10_REG_DPLL_NC, 0x06},
> + { OS02G10_REG_MP_PHASE_CTRL, 0x20},
> + { CCI_REG8(0x119), 0x50},
> + { CCI_REG8(0x11a), 0x0c},
> + { CCI_REG8(0x11b), 0x0d},
> + { CCI_REG8(0x11c), 0x00},
> + { CCI_REG8(0x11d), 0x75},
> + { CCI_REG8(0x11e), 0x52},
> + { CCI_REG8(0x122), 0x14},
> + { CCI_REG8(0x125), 0x44},
> + { CCI_REG8(0x126), 0x0f},
> + { OS02G10_REG_ULP_PWD_DUMMY_CTRL, 0xca},
> + { CCI_REG8(0x13d), 0x4a},
> + { CCI_REG8(0x140), 0x0f},
> + { CCI_REG8(0x143), 0x38},
> + { OS02G10_REG_DC_LEVEL_LIMIT_EN, 0x01},
> + { OS02G10_REG_DC_LEVEL_LIMIT_L, 0x00},
> + { OS02G10_REG_DC_BLC_LIMIT_H, 0x32},
> + { CCI_REG8(0x150), 0x01},
> + { CCI_REG8(0x151), 0x28},
> + { CCI_REG8(0x152), 0x20},
> + { CCI_REG8(0x153), 0x03},
> + { CCI_REG8(0x157), 0x16},
> + { CCI_REG8(0x159), 0x01},
> + { CCI_REG8(0x15a), 0x01},
> + { CCI_REG8(0x15d), 0x04},
> + { CCI_REG8(0x16a), 0x04},
> + { CCI_REG8(0x16b), 0x03},
> + { CCI_REG8(0x16e), 0x28},
> + { CCI_REG8(0x171), 0xc2},
> + { CCI_REG8(0x172), 0x04},
> + { CCI_REG8(0x173), 0x38},
> + { CCI_REG8(0x174), 0x04},
> + { CCI_REG8(0x179), 0x00},
> + { CCI_REG8(0x17a), 0xb2},
> + { CCI_REG8(0x17b), 0x10},
> + { OS02G10_REG_HS_LP_CTRL, 0x02},
> + { OS02G10_REG_HS_LEVEL, 0x03},
> + { OS02G10_REG_HS_DRV, 0x55},
> + { CCI_REG8(0x1b8), 0x70},
> + { CCI_REG8(0x1b9), 0x70},
> + { CCI_REG8(0x1ba), 0x70},
> + { CCI_REG8(0x1bb), 0x70},
> + { CCI_REG8(0x1bc), 0x00},
> + { CCI_REG8(0x1c4), 0x6d},
> + { CCI_REG8(0x1c5), 0x6d},
> + { CCI_REG8(0x1c6), 0x6d},
> + { CCI_REG8(0x1c7), 0x6d},
> + { CCI_REG8(0x1cc), 0x11},
> + { CCI_REG8(0x1cd), 0xe0},
> + { CCI_REG8(0x1d0), 0x1b},
> + { CCI_REG8(0x1d2), 0x76},
> + { CCI_REG8(0x1d3), 0x68},
> + { CCI_REG8(0x1d4), 0x68},
> + { CCI_REG8(0x1d5), 0x73},
> + { CCI_REG8(0x1d6), 0x73},
> + { CCI_REG8(0x1e8), 0x55},
> + { OS02G10_REG_GB_SUBOFFSET, 0x40},
> + { OS02G10_REG_BLUE_SUBOFFSET, 0x40},
> + { OS02G10_REG_RED_SUBOFFSET, 0x40},
> + { OS02G10_REG_GR_SUBOFFSET, 0x40},
> + { OS02G10_REG_ABL_TRIGGER, 0x1c},
> + { OS02G10_REG_ABL, 0x33},
> + { CCI_REG8(0x1fc), 0x80},
> + { CCI_REG8(0x1fe), 0x80},
> + { CCI_REG8(0x303), 0x67},
> + { CCI_REG8(0x300), 0x59},
> + { CCI_REG8(0x304), 0x11},
> + { CCI_REG8(0x305), 0x04},
> + { CCI_REG8(0x306), 0x0c},
> + { CCI_REG8(0x307), 0x08},
> + { CCI_REG8(0x308), 0x08},
> + { CCI_REG8(0x309), 0x4f},
> + { CCI_REG8(0x30b), 0x08},
> + { CCI_REG8(0x30d), 0x26},
> + { CCI_REG8(0x30f), 0x00},
> + { CCI_REG8(0x234), 0xfe},
> + { OS02G10_REG_MIPI_TX_SPEED_CTRL, 0x05},
> +};
> +
> +static const struct os02g10_mode supported_modes[] = {
> + {
> + .width = 1920,
> + .height = 1080,
> + .vts_def = 1246,
> + .exp_def = 1100,
> + .x_start = 2,
> + .y_start = 6,
> + },
> +};
> +
> +static const s64 link_freq_menu_items[] = {
> + OS02G10_LINK_FREQ_720MHZ,
> +};
> +
> +static const char * const os02g10_test_pattern_menu[] = {
> + "Disabled",
> + "Colorbar",
> +};
> +
> +static inline struct os02g10 *to_os02g10(struct v4l2_subdev *sd)
> +{
> + return container_of_const(sd, struct os02g10, sd);
> +}
> +
> +static u32 os02g10_get_format_code(struct os02g10 *os02g10)
> +{
> + static const u32 codes[2][2] = {
> + { MEDIA_BUS_FMT_SBGGR10_1X10, MEDIA_BUS_FMT_SGBRG10_1X10, },
> + { MEDIA_BUS_FMT_SGRBG10_1X10, MEDIA_BUS_FMT_SRGGB10_1X10, },
> + };
> + u32 code = codes[os02g10->vflip->val][os02g10->hflip->val];
No need for a local variable for this.
> +
> + return code;
> +}
> +
> +static int os02g10_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct os02g10 *os02g10 = container_of_const(ctrl->handler,
> + struct os02g10, handler);
> + struct v4l2_subdev_state *state;
> + struct v4l2_mbus_framefmt *fmt;
> + int ret = 0;
> +
> + state = v4l2_subdev_get_locked_active_state(&os02g10->sd);
> + fmt = v4l2_subdev_state_get_format(state, 0);
> +
> + if (ctrl->id == V4L2_CID_VBLANK) {
> + /* Honour the VBLANK limits when setting exposure */
> + s64 max = fmt->height + ctrl->val - OS02G10_EXPOSURE_MARGIN;
> +
> + ret = __v4l2_ctrl_modify_range(os02g10->exposure,
> + os02g10->exposure->minimum, max,
> + os02g10->exposure->step,
> + os02g10->exposure->default_value);
> + if (ret)
> + return ret;
> + }
> +
> + if (pm_runtime_get_if_active(os02g10->dev) == 0)
> + return 0;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_EXPOSURE:
> + cci_write(os02g10->cci, OS02G10_REG_LONG_EXPOSURE,
> + ctrl->val, &ret);
> + break;
> + case V4L2_CID_ANALOGUE_GAIN:
> + cci_write(os02g10->cci, OS02G10_REG_ANALOG_GAIN,
> + ctrl->val, &ret);
> + break;
> + case V4L2_CID_DIGITAL_GAIN:
> + cci_write(os02g10->cci, OS02G10_REG_DIGITAL_GAIN_L,
> + (ctrl->val & 0xff), &ret);
> + cci_write(os02g10->cci, OS02G10_REG_DIGITAL_GAIN_H,
> + ((ctrl->val >> 8) & 0x7), &ret);
> + break;
> + case V4L2_CID_VBLANK:
> + u64 vts = ctrl->val + fmt->height;
> +
> + cci_update_bits(os02g10->cci, OS02G10_REG_FRAME_TEST_CTRL,
> + OS02G10_FRAME_EXP_SEPERATE_EN,
> + OS02G10_FRAME_EXP_SEPERATE_EN, &ret);
> + cci_write(os02g10->cci, OS02G10_REG_FRAME_LENGTH, vts, &ret);
> + break;
> + case V4L2_CID_HFLIP:
> + case V4L2_CID_VFLIP:
> + cci_write(os02g10->cci, OS02G10_REG_FLIP_MIRROR,
> + os02g10->hflip->val | os02g10->vflip->val << 1,
> + &ret);
Fits on previous line.
> + cci_write(os02g10->cci, OS02G10_REG_SIF_CTRL,
> + OS02G10_ORIENTATION_BAYER_FIX, &ret);
> + break;
> + case V4L2_CID_TEST_PATTERN:
> + cci_update_bits(os02g10->cci,
> + OS02G10_REG_FRAME_TEST_CTRL,
> + OS02G10_TEST_PATTERN_ENABLE,
> + ctrl->val ? OS02G10_TEST_PATTERN_ENABLE : 0,
> + &ret);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + cci_write(os02g10->cci, OS02G10_REG_FRAME_SYNC, 0x01, &ret);
> +
> + pm_runtime_put(os02g10->dev);
> +
> + return ret;
> +}
> +
> +static const struct v4l2_ctrl_ops os02g10_ctrl_ops = {
> + .s_ctrl = os02g10_set_ctrl,
> +};
> +
> +static int os02g10_init_controls(struct os02g10 *os02g10)
> +{
> + const struct os02g10_mode *mode = &supported_modes[0];
> + struct v4l2_fwnode_device_properties props;
> + u64 vblank_def, exp_max, pixel_rate;
> + struct v4l2_ctrl_handler *ctrl_hdlr;
> + int ret;
> +
> + ctrl_hdlr = &os02g10->handler;
> + v4l2_ctrl_handler_init(ctrl_hdlr, 11);
> +
> + /* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
> + pixel_rate = div_u64(OS02G10_LINK_FREQ_720MHZ * 2 * 2, 10);
> + v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops, V4L2_CID_PIXEL_RATE, 0,
> + pixel_rate, 1, pixel_rate);
> +
> + os02g10->link_freq =
> + v4l2_ctrl_new_int_menu(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_LINK_FREQ,
> + ARRAY_SIZE(link_freq_menu_items) - 1,
> + 0, link_freq_menu_items);
> + if (os02g10->link_freq)
> + os02g10->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + vblank_def = mode->vts_def - mode->height;
> + os02g10->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_VBLANK, vblank_def,
> + OS02G10_FRAME_LENGTH_MAX - mode->height,
> + 1, vblank_def);
> +
> + exp_max = mode->vts_def - OS02G10_EXPOSURE_MARGIN;
> + os02g10->exposure =
> + v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_EXPOSURE,
> + OS02G10_EXPOSURE_MIN, exp_max,
> + OS02G10_EXPOSURE_STEP, mode->exp_def);
> +
> + v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_ANALOGUE_GAIN, OS02G10_ANALOG_GAIN_MIN,
> + OS02G10_ANALOG_GAIN_MAX, OS02G10_ANALOG_GAIN_STEP,
> + OS02G10_ANALOG_GAIN_DEFAULT);
> +
> + v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_DIGITAL_GAIN, OS02G10_DIGITAL_GAIN_MIN,
> + OS02G10_DIGITAL_GAIN_MAX, OS02G10_DIGITAL_GAIN_STEP,
> + OS02G10_DIGITAL_GAIN_DEFAULT);
> +
> + os02g10->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_HFLIP, 0, 1, 1, 0);
> + if (os02g10->hflip)
> + os02g10->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
> +
> + os02g10->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_VFLIP, 0, 1, 1, 0);
> + if (os02g10->vflip)
> + os02g10->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
There are quite a few flags being modified in the function.
You can drop the checks if you do this after the control handler's error
check.
> +
> + v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_TEST_PATTERN,
> + ARRAY_SIZE(os02g10_test_pattern_menu) - 1,
> + 0, 0, os02g10_test_pattern_menu);
> +
> + ret = v4l2_fwnode_device_parse(os02g10->dev, &props);
> + if (ret)
> + goto err_handler_free;
Do this first in the function and error handling is simplified. You can
rely on the handler's error value only, for instance (apart from the above
call).
> +
> + ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr,
> + &os02g10_ctrl_ops, &props);
> + if (ret)
> + goto err_handler_free;
> +
> + os02g10->sd.ctrl_handler = ctrl_hdlr;
> +
> + return 0;
> +
> +err_handler_free:
> + v4l2_ctrl_handler_free(ctrl_hdlr);
> +
> + return ret;
> +}
> +
> +static int os02g10_set_framefmt(struct os02g10 *os02g10,
> + struct v4l2_subdev_state *state)
> +{
> + const struct v4l2_mbus_framefmt *format;
> + const struct os02g10_mode *mode;
> + int ret = 0;
> +
> + format = v4l2_subdev_state_get_format(state, 0);
> + mode = v4l2_find_nearest_size(supported_modes,
> + ARRAY_SIZE(supported_modes), width,
> + height, format->width, format->height);
> +
> + cci_write(os02g10->cci, OS02G10_REG_V_START, mode->y_start, &ret);
> + cci_write(os02g10->cci, OS02G10_REG_V_SIZE, mode->height, &ret);
> + cci_write(os02g10->cci, OS02G10_REG_V_SIZE_MIPI, mode->height, &ret);
> + cci_write(os02g10->cci, OS02G10_REG_H_START, mode->x_start, &ret);
> + cci_write(os02g10->cci, OS02G10_REG_H_SIZE, mode->width, &ret);
> + cci_write(os02g10->cci, OS02G10_REG_H_SIZE_MIPI, mode->width, &ret);
> +
> + return ret;
> +}
> +
> +static int os02g10_enable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> +{
> + struct os02g10 *os02g10 = to_os02g10(sd);
> + int ret;
> +
> + ret = pm_runtime_resume_and_get(os02g10->dev);
> + if (ret < 0)
> + return ret;
> +
> + ret = cci_multi_reg_write(os02g10->cci, os02g10_common_regs,
> + ARRAY_SIZE(os02g10_common_regs), NULL);
> + if (ret) {
> + dev_err(os02g10->dev, "failed to write common registers\n");
> + goto err_rpm_put;
> + }
> +
> + ret = os02g10_set_framefmt(os02g10, state);
> + if (ret) {
> + dev_err(os02g10->dev, "failed to set frame foramt\n");
> + goto err_rpm_put;
> + }
> +
> + /* Apply customized values from user */
> + ret = __v4l2_ctrl_handler_setup(os02g10->sd.ctrl_handler);
> + if (ret)
> + goto err_rpm_put;
> +
> + ret = cci_write(os02g10->cci, OS02G10_REG_STREAM_CTRL,
> + OS02G10_STREAM_CTRL_ON, NULL);
> + if (ret)
> + goto err_rpm_put;
> +
> + /* vflip and hflip cannot change during streaming */
> + __v4l2_ctrl_grab(os02g10->vflip, true);
> + __v4l2_ctrl_grab(os02g10->hflip, true);
> +
> + return 0;
> +
> +err_rpm_put:
> + pm_runtime_put(os02g10->dev);
> + return ret;
> +}
> +
> +static int os02g10_disable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> +{
> + struct os02g10 *os02g10 = to_os02g10(sd);
> + int ret;
> +
> + ret = cci_write(os02g10->cci, OS02G10_REG_STREAM_CTRL,
> + OS02G10_STREAM_CTRL_OFF, NULL);
> + if (ret)
> + dev_err(os02g10->dev, "Failed to stop stream\n");
> +
> + __v4l2_ctrl_grab(os02g10->vflip, false);
> + __v4l2_ctrl_grab(os02g10->hflip, false);
> +
> + pm_runtime_put(os02g10->dev);
> +
> + return ret;
> +}
> +
> +static int os02g10_get_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_selection *sel)
> +{
> + switch (sel->target) {
> + case V4L2_SEL_TGT_CROP_BOUNDS:
> + case V4L2_SEL_TGT_NATIVE_SIZE:
> + sel->r = os02g10_native_area;
> + return 0;
> + case V4L2_SEL_TGT_CROP:
> + case V4L2_SEL_TGT_CROP_DEFAULT:
> + sel->r = os02g10_active_area;
> + return 0;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int os02g10_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + struct os02g10 *os02g10 = to_os02g10(sd);
> +
> + if (code->index)
> + return -EINVAL;
> +
> + code->code = os02g10_get_format_code(os02g10);
> +
> + return 0;
> +}
> +
> +static int os02g10_enum_frame_size(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_frame_size_enum *fse)
> +{
> + struct os02g10 *os02g10 = to_os02g10(sd);
> +
> + if (fse->index >= ARRAY_SIZE(supported_modes))
> + return -EINVAL;
> +
> + if (fse->code != os02g10_get_format_code(os02g10))
> + return -EINVAL;
> +
> + fse->min_width = supported_modes[fse->index].width;
> + fse->max_width = fse->min_width;
> + fse->min_height = supported_modes[fse->index].height;
> + fse->max_height = fse->min_height;
> +
> + return 0;
> +}
> +
> +static int os02g10_set_pad_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct os02g10 *os02g10 = to_os02g10(sd);
> + struct v4l2_mbus_framefmt *format;
> + const struct os02g10_mode *mode;
> +
> + format = v4l2_subdev_state_get_format(sd_state, 0);
> +
> + mode = v4l2_find_nearest_size(supported_modes,
> + ARRAY_SIZE(supported_modes),
> + width, height,
> + fmt->format.width, fmt->format.height);
> +
> + fmt->format.code = os02g10_get_format_code(os02g10);
> + fmt->format.width = mode->width;
> + fmt->format.height = mode->height;
> + fmt->format.field = V4L2_FIELD_NONE;
> + fmt->format.colorspace = V4L2_COLORSPACE_RAW;
> + fmt->format.quantization = V4L2_QUANTIZATION_FULL_RANGE;
> + fmt->format.xfer_func = V4L2_XFER_FUNC_NONE;
> +
> + *format = fmt->format;
> +
> + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> + u32 vblank_def = mode->vts_def - mode->height;
> +
> + int ret = __v4l2_ctrl_modify_range(os02g10->vblank, vblank_def,
> + OS02G10_FRAME_LENGTH_MAX -
> + mode->height, 1, vblank_def);
return ...;
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int os02g10_init_state(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state)
> +{
> + struct os02g10 *os02g10 = to_os02g10(sd);
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_TRY,
> + .format = {
> + .code = os02g10_get_format_code(os02g10),
> + .width = supported_modes[0].width,
> + .height = supported_modes[0].height,
> + },
> + };
> +
> + os02g10_set_pad_format(sd, state, &fmt);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_subdev_video_ops os02g10_video_ops = {
> + .s_stream = v4l2_subdev_s_stream_helper,
> +};
> +
> +static const struct v4l2_subdev_pad_ops os02g10_pad_ops = {
> + .enum_mbus_code = os02g10_enum_mbus_code,
> + .get_fmt = v4l2_subdev_get_fmt,
> + .set_fmt = os02g10_set_pad_format,
> + .get_selection = os02g10_get_selection,
> + .enum_frame_size = os02g10_enum_frame_size,
> + .enable_streams = os02g10_enable_streams,
> + .disable_streams = os02g10_disable_streams,
> +};
> +
> +static const struct v4l2_subdev_ops os02g10_subdev_ops = {
> + .video = &os02g10_video_ops,
> + .pad = &os02g10_pad_ops,
> +};
> +
> +static const struct v4l2_subdev_internal_ops os02g10_internal_ops = {
> + .init_state = os02g10_init_state,
> +};
> +
> +static int os02g10_power_on(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct os02g10 *os02g10 = to_os02g10(sd);
> + int ret;
> +
> + ret = regulator_bulk_enable(ARRAY_SIZE(os02g10_supply_name),
> + os02g10->supplies);
> + if (ret) {
> + dev_err(os02g10->dev, "failed to enable regulators\n");
> + return ret;
> + }
> +
> + /* T4: delay from DOVDD stable to MCLK on */
> + fsleep(5 * USEC_PER_MSEC);
Does the sensor really require this? Typically no delays are required
before lifting xshutdown.
> +
> + ret = clk_prepare_enable(os02g10->xclk);
> + if (ret) {
> + dev_err(os02g10->dev, "failed to enable clock\n");
> + goto err_regulator_off;
> + }
> +
> + /* T3: delay from DVDD stable to sensor power up stable */
> + fsleep(5 * USEC_PER_MSEC);
The supplies were enabled before the clock so right now there's already 5
ms delay here. Consider with the above comment.
> +
> + gpiod_set_value_cansleep(os02g10->reset_gpio, 0);
> +
> + /* T5: delay from sensor power up stable to SCCB initialization */
> + fsleep(5 * USEC_PER_MSEC);
> +
> + return 0;
> +
> +err_regulator_off:
> + regulator_bulk_disable(ARRAY_SIZE(os02g10_supply_name), os02g10->supplies);
A newline here would be nice.
> + return ret;
> +}
> +
> +static int os02g10_power_off(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct os02g10 *os02g10 = to_os02g10(sd);
> +
> + clk_disable_unprepare(os02g10->xclk);
> + gpiod_set_value_cansleep(os02g10->reset_gpio, 1);
> + regulator_bulk_disable(ARRAY_SIZE(os02g10_supply_name), os02g10->supplies);
> +
> + return 0;
> +}
> +
> +static int os02g10_identify_module(struct os02g10 *os02g10)
> +{
> + u64 chip_id;
> + int ret;
> +
> + ret = cci_read(os02g10->cci, OS02G10_REG_CHIPID, &chip_id, NULL);
> + if (ret)
> + return dev_err_probe(os02g10->dev, ret,
> + "failed to read chip id %x\n",
> + OS02G10_CHIPID);
> +
> + if (chip_id != OS02G10_CHIPID)
> + return dev_err_probe(os02g10->dev, -EIO,
> + "chip id mismatch: %x!=%llx\n",
> + OS02G10_CHIPID, chip_id);
> +
> + return 0;
> +}
> +
> +static int os02g10_parse_endpoint(struct os02g10 *os02g10)
> +{
> + struct v4l2_fwnode_endpoint bus_cfg = {
> + .bus_type = V4L2_MBUS_CSI2_DPHY,
> + };
> + unsigned long link_freq_bitmap;
> + struct fwnode_handle *ep;
> + int ret;
> +
> + ep = fwnode_graph_get_next_endpoint(dev_fwnode(os02g10->dev), NULL);
> + ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
> + fwnode_handle_put(ep);
> + if (ret)
> + return ret;
> +
> + if (bus_cfg.bus.mipi_csi2.num_data_lanes != 2) {
> + ret = dev_err_probe(os02g10->dev, -EINVAL,
> + "only 2 data lanes are supported\n");
> + goto error_out;
> + }
> +
> + ret = v4l2_link_freq_to_bitmap(os02g10->dev, bus_cfg.link_frequencies,
> + bus_cfg.nr_of_link_frequencies,
> + link_freq_menu_items,
> + ARRAY_SIZE(link_freq_menu_items),
> + &link_freq_bitmap);
> + if (ret) {
> + ret = dev_err_probe(os02g10->dev, -EINVAL,
> + "only 720MHz frequency is available\n");
> + goto error_out;
> + }
> +
> +error_out:
> + v4l2_fwnode_endpoint_free(&bus_cfg);
> +
> + return ret;
> +};
> +
> +static const struct regmap_range_cfg os02g10_ranges[] = {
> + {
> + .range_min = 0x0000,
> + .range_max = 0x03ff,
> + .selector_reg = 0xfd,
> + .selector_mask = 0x03,
> + .selector_shift = 0,
> + .window_start = 0x00,
> + .window_len = 0x100,
> + },
> +};
> +
> +static const struct regmap_config os02g10_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .reg_format_endian = REGMAP_ENDIAN_BIG,
> + .max_register = 0x3ff,
> + .ranges = os02g10_ranges,
> + .num_ranges = ARRAY_SIZE(os02g10_ranges),
> + .disable_locking = true,
> +};
Nice!
> +
> +static int os02g10_probe(struct i2c_client *client)
> +{
> + struct os02g10 *os02g10;
> + unsigned int xclk_freq;
> + int ret;
> +
> + os02g10 = devm_kzalloc(&client->dev, sizeof(*os02g10), GFP_KERNEL);
> + if (!os02g10)
> + return -ENOMEM;
> +
> + os02g10->dev = &client->dev;
> +
> + v4l2_i2c_subdev_init(&os02g10->sd, client, &os02g10_subdev_ops);
> + os02g10->sd.internal_ops = &os02g10_internal_ops;
> +
> + /*
> + * This is not using devm_cci_regmap_init_i2c(), because the driver
> + * makes use of regmap's pagination feature. The chosen settings are
> + * compatible with the CCI helpers.
> + */
> + os02g10->cci = devm_regmap_init_i2c(client, &os02g10_regmap_config);
> + if (IS_ERR(os02g10->cci))
> + return dev_err_probe(os02g10->dev, PTR_ERR(os02g10->cci),
> + "failed to initialize CCI\n");
> +
> + ret = os02g10_parse_endpoint(os02g10);
> + if (ret)
> + return dev_err_probe(os02g10->dev, ret,
> + "failed to parse endpoint configuration\n");
> +
> + /* Get system clock (xvclk) */
> + os02g10->xclk = devm_v4l2_sensor_clk_get(os02g10->dev, NULL);
> + if (IS_ERR(os02g10->xclk))
> + return dev_err_probe(os02g10->dev, PTR_ERR(os02g10->xclk),
> + "failed to get xclk\n");
> +
> + xclk_freq = clk_get_rate(os02g10->xclk);
> + if (xclk_freq != OS02G10_XCLK_FREQ)
> + return dev_err_probe(os02g10->dev, -EINVAL,
> + "xclk frequency not supported: %u Hz\n",
> + xclk_freq);
> +
> + for (unsigned int i = 0; i < ARRAY_SIZE(os02g10_supply_name); i++)
> + os02g10->supplies[i].supply = os02g10_supply_name[i];
> +
> + ret = devm_regulator_bulk_get(os02g10->dev,
> + ARRAY_SIZE(os02g10_supply_name),
> + os02g10->supplies);
> + if (ret)
> + return dev_err_probe(os02g10->dev, ret,
> + "failed to get regulators\n");
> +
> + os02g10->reset_gpio = devm_gpiod_get_optional(os02g10->dev,
> + "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(os02g10->reset_gpio))
> + return dev_err_probe(os02g10->dev, PTR_ERR(os02g10->reset_gpio),
> + "failed to get reset GPIO\n");
> +
> + ret = os02g10_power_on(os02g10->dev);
> + if (ret)
> + return ret;
> +
> + ret = os02g10_identify_module(os02g10);
> + if (ret)
> + goto error_power_off;
> +
> + ret = os02g10_init_controls(os02g10);
> + if (ret)
> + goto error_power_off;
> +
> + /* Initialize subdev */
> + os02g10->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + os02g10->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> + os02g10->pad.flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(&os02g10->sd.entity, 1, &os02g10->pad);
> + if (ret) {
> + dev_err_probe(os02g10->dev, ret, "failed to init entity pads\n");
> + goto error_handler_free;
> + }
> +
> + os02g10->sd.state_lock = os02g10->handler.lock;
> + ret = v4l2_subdev_init_finalize(&os02g10->sd);
> + if (ret) {
> + dev_err_probe(os02g10->dev, ret, "subdev init error\n");
> + goto error_media_entity;
> + }
> +
> + pm_runtime_set_active(os02g10->dev);
> + pm_runtime_enable(os02g10->dev);
> +
> + ret = v4l2_async_register_subdev_sensor(&os02g10->sd);
> + if (ret) {
> + dev_err_probe(os02g10->dev, ret,
> + "failed to register os02g10 sub-device\n");
> + goto error_subdev_cleanup;
> + }
> +
> + pm_runtime_idle(os02g10->dev);
> +
> + return 0;
> +
> +error_subdev_cleanup:
> + v4l2_subdev_cleanup(&os02g10->sd);
> + pm_runtime_disable(os02g10->dev);
> + pm_runtime_set_suspended(os02g10->dev);
> +
> +error_media_entity:
> + media_entity_cleanup(&os02g10->sd.entity);
> +
> +error_handler_free:
> + v4l2_ctrl_handler_free(os02g10->sd.ctrl_handler);
> +
> +error_power_off:
> + os02g10_power_off(os02g10->dev);
> +
> + return ret;
> +}
> +
> +static void os02g10_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct os02g10 *os02g10 = to_os02g10(sd);
> +
> + v4l2_async_unregister_subdev(sd);
> + v4l2_subdev_cleanup(&os02g10->sd);
> + media_entity_cleanup(&sd->entity);
> + v4l2_ctrl_handler_free(os02g10->sd.ctrl_handler);
> +
> + pm_runtime_disable(&client->dev);
> + if (!pm_runtime_status_suspended(&client->dev)) {
> + os02g10_power_off(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> + }
> +}
> +
> +static DEFINE_RUNTIME_DEV_PM_OPS(os02g10_pm_ops,
> + os02g10_power_off, os02g10_power_on, NULL);
> +
> +static const struct of_device_id os02g10_id[] = {
> + { .compatible = "ovti,os02g10" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, os02g10_id);
> +
> +static struct i2c_driver os02g10_driver = {
> + .driver = {
> + .name = "os02g10",
> + .pm = pm_ptr(&os02g10_pm_ops),
> + .of_match_table = os02g10_id,
> + },
> + .probe = os02g10_probe,
> + .remove = os02g10_remove,
> +};
> +module_i2c_driver(os02g10_driver);
> +
> +MODULE_DESCRIPTION("OS02G10 Camera Sensor Driver");
> +MODULE_AUTHOR("Tarang Raval <tarang.raval@siliconsignals.io>");
> +MODULE_AUTHOR("Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>");
> +MODULE_LICENSE("GPL");
--
Kind regards,
Sakari Ailus
^ permalink raw reply
* [PATCH 2/2] arm64: dts: qcom: shikra: Add MDSP carveout memory and update APM DAIs memory regions
From: Ajay Kumar Nandam @ 2026-06-18 11:35 UTC (permalink / raw)
To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel, ajay.nandam
In-Reply-To: <20260618113509.2025881-1-ajay.nandam@oss.qualcomm.com>
Add a dedicated MDSP carveout memory region for audio usecases on
Shikra and mark both existing audio heap and MDSP carveout regions
as shared DMA pools.
Update the Q6 APM DAI node to reference multiple memory regions,
where index 0 is used for control path buffers and index 1 is used
for MDSP data path buffers. This separation ensures proper memory
allocation and access for APM communication between APSS and MDSP.
Also add shared-dma-pool compatibility to the existing audio heap
region to align with upstream DMA pool usage.
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/shikra.dtsi | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
index 88954ee943ef..d744f7e38ca6 100644
--- a/arch/arm64/boot/dts/qcom/shikra.dtsi
+++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
@@ -365,7 +365,14 @@ smem_mem: smem@86000000 {
};
audio_heap_mem: audio-heap@86200000 {
- reg = <0x0 0x86200000 0x0 0x100000>;
+ compatible = "shared-dma-pool";
+ reg = <0x0 0x86200000 0x0 0x40000>;
+ no-map;
+ };
+
+ audio_mdsp_carveout_mem: audio-mdsp-carveout@86240000 {
+ compatible = "shared-dma-pool";
+ reg = <0x0 0x86240000 0x0 0x100000>;
no-map;
};
@@ -1935,7 +1942,10 @@ q6apmbedai: bedais {
q6apmdai: dais {
compatible = "qcom,q6apm-dais";
- qcom,vmid = <QCOM_SCM_VMID_MSS_MSA>;
+ memory-region = <&audio_heap_mem
+ &audio_mdsp_carveout_mem>;
+ qcom,vmid = <QCOM_SCM_VMID_LPASS
+ QCOM_SCM_VMID_MSS_MSA>;
};
};
--
2.34.1
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: sound: qcom,q6apm-dai: add memory-region property
From: Ajay Kumar Nandam @ 2026-06-18 11:35 UTC (permalink / raw)
To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel, ajay.nandam
In-Reply-To: <20260618113509.2025881-1-ajay.nandam@oss.qualcomm.com>
Document the new memory-region property that allows one or more
reserved-memory carveout regions to be SCM-assigned to the VMIDs
listed in qcom,vmid.
- Add memory-region as an optional phandle-array (1-8 entries).
Each entry must point to a shared-dma-pool, no-map reserved-memory
node. Index 0 is the control-path buffer; subsequent entries are
data-path buffers.
- Enforce via dependentRequired that memory-region is only valid
when qcom,vmid is also present.
- Expand qcom,vmid description to mention carveout regions and the
qcom_scm_is_available() probe-defer requirement.
- Add a second example showing both properties in use with two
carveout regions and two destination VMIDs.
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
.../bindings/sound/qcom,q6apm-dai.yaml | 38 +++++++++++++++++--
1 file changed, 35 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
index b767625985a7..601055182da6 100644
--- a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
@@ -10,7 +10,11 @@ maintainers:
- Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
description: |
- This binding describes the Qualcomm APM DAIs in DSP
+ This binding describes the Qualcomm APM DAIs in DSP.
+ When qcom,vmid is present, the driver performs SCM memory
+ assignment for PCM DMA buffers and any reserved-memory regions
+ listed in memory-region, granting the specified VMIDs RW access
+ while retaining HLOS as a co-owner.
properties:
compatible:
@@ -20,9 +24,24 @@ properties:
minItems: 1
maxItems: 2
+ memory-region:
+ description:
+ List of phandles to reserved-memory regions (shared-dma-pool,
+ no-map) that must be SCM-assigned to the VMIDs in qcom,vmid.
+ The first entry is the control-path buffer; subsequent entries
+ are data-path buffers. Only meaningful when qcom,vmid is present.
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ items:
+ maxItems: 1
+ minItems: 1
+ maxItems: 8
+
qcom,vmid:
- description: Optional list of destination VMIDs to share PCM DMA buffers with.
- HLOS retains RW access as source owner and must not be listed.
+ description:
+ Optional list of destination VMIDs to grant RW access to PCM DMA
+ buffers and any memory-region carveouts. HLOS retains RW access
+ as source owner and must not be listed. When present,
+ qcom_scm_is_available() must return true at probe time.
$ref: /schemas/types.yaml#/definitions/uint32-array
items:
minimum: 1
@@ -34,6 +53,10 @@ required:
- compatible
- iommus
+dependentRequired:
+ memory-region:
+ - qcom,vmid
+
additionalProperties: false
examples:
@@ -42,3 +65,12 @@ examples:
compatible = "qcom,q6apm-dais";
iommus = <&apps_smmu 0x1801 0x0>;
};
+ - |
+ #include <dt-bindings/firmware/qcom,scm.h>
+ dais {
+ compatible = "qcom,q6apm-dais";
+ iommus = <&apps_smmu 0x0c01 0x0>;
+ /* index 0: control path, index 1: data path */
+ memory-region = <&audio_heap_mem &audio_mdsp_carveout_mem>;
+ qcom,vmid = <QCOM_SCM_VMID_MSS_MSA QCOM_SCM_VMID_LPASS>;
+ };
--
2.34.1
^ permalink raw reply related
* [PATCH 0/2] Add MDSP carveout memory-region support for Q6 APM on Shikra
From: Ajay Kumar Nandam @ 2026-06-18 11:35 UTC (permalink / raw)
To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel, ajay.nandam
Unlike most Qualcomm targets where audio runs on the ADSP, the Shikra
target runs its audio use cases on the MDSP. The MDSP operates in an S2
(stage-2) protected/secured memory context, so any buffer shared with
it must reside in a dedicated carveout that is SCM-assigned to the
MDSP's VMID before use. Without this assignment the MDSP cannot access
the buffers and APM communication between APSS and MDSP fails.
This series wires up that flow for the Q6 APM DAI:
Patch 1 documents a new optional "memory-region" property in the
qcom,q6apm-dai binding. It accepts one or more reserved-memory carveout
phandles that get SCM-assigned to the VMIDs listed in "qcom,vmid".
Index 0 is the control-path buffer; subsequent entries are data-path
buffers. The property is only valid together with "qcom,vmid"
(enforced via dependentRequired).
Patch 2 adds a dedicated MDSP carveout reserved-memory region on Shikra,
marks both the existing audio heap and the new MDSP carveout as
shared-dma-pool, and points the Q6 APM DAI node at both regions: index 0
for control-path buffers and index 1 for MDSP data-path buffers. This
provides the secured carveout the MDSP needs once it is SCM-assigned to
its VMID.
Depends-on: https://lore.kernel.org/all/20260609064038.492641-1-ajay.nandam@oss.qualcomm.com/
Depends-on: https://lore.kernel.org/all/20260618112810.2009847-1-ajay.nandam@oss.qualcomm.com/
Ajay Kumar Nandam (2):
dt-bindings: sound: qcom,q6apm-dai: add memory-region property
arm64: dts: qcom: shikra: Add MDSP carveout memory and update APM DAIs
memory regions
.../bindings/sound/qcom,q6apm-dai.yaml | 38 +++++++++++++++++--
arch/arm64/boot/dts/qcom/shikra.dtsi | 14 ++++++-
2 files changed, 47 insertions(+), 5 deletions(-)
base-commit: ec039126b7fac4e3af35ebccaa7c6f9b6875ba81
prerequisite-patch-id: 59bb0a7828e41f546f734f127d81da83c0adcda9
prerequisite-patch-id: 197da6bcb15cadc47869dba88c8020987b25c335
prerequisite-patch-id: 8ec9c1eb03f052ae232ed54117abed38672c23f6
prerequisite-patch-id: 350db4f4bcdfc0fad9ed57cd5b1723f85ad44f5d
prerequisite-patch-id: e80ea7940b9817449cec21afa6e9e443e007166f
prerequisite-patch-id: 80d8ab865b7b0663c5b2878b45b55e2e4fde9c19
prerequisite-patch-id: 8e645e1c6ad6182de4813a726c293654324de1df
prerequisite-patch-id: f6781d2cf0829ccb32f1400623c95739972f2ee2
prerequisite-patch-id: fb821179cbe1fb1a97b987d04acb5a656aee9c14
prerequisite-patch-id: 42248b68d3392d122eab7441ae451981851c87fb
prerequisite-patch-id: 2acc300a68ed8c5364fb5f2f7d28fc0d56ab07bf
prerequisite-patch-id: 2357cac636e019eaf14d6a493a1c72bca56fe405
prerequisite-patch-id: 2885f299e711582da312ca9d13983d296a3dd5dc
prerequisite-patch-id: 91af5f3c01e766a53ce8de69aa21847a2d6bbbf8
prerequisite-patch-id: b5d7f75df02fde56181f576a936baf09d0a72276
prerequisite-patch-id: 3ce52e07ae57139c2e2b71a29ed7d7250f6fcc87
prerequisite-patch-id: 48ecd66c06c4fad81f91283c26ec57d95bbde29d
prerequisite-patch-id: 7d92e5a301d09616840e54dc9e4a81f30a64383c
prerequisite-patch-id: be1a16f53e23dd5ab90210a056c9da3559c6186d
prerequisite-patch-id: 960421b3afdc01064a337eaadd6a53ce1afb8005
prerequisite-patch-id: ab22c5fdb2fa65b78ffef76bbe4459c8d5ffe7b2
prerequisite-patch-id: cb24efedb648271ef5c60ace96e2366d89254e8f
prerequisite-patch-id: 8be7df0395c5847a988c7e814c7db878b5932b11
prerequisite-patch-id: 0510992d022cef7317b6efee6765ac78c0225356
prerequisite-patch-id: dbf7ba8aca718414ced7604061ac60879a4e92b6
prerequisite-patch-id: 67fa5f31ee5109470da23db3b513721580f4c86f
prerequisite-patch-id: 0e79e46bc5a88849a2f0a410b39c08f3244dfed3
prerequisite-patch-id: 0396ac157aba73a5afd7ba4a8a744847f5a7b433
prerequisite-patch-id: 2b1aecd97b9c073a1b323138cd7a98cb34e3715f
prerequisite-patch-id: 823bc7bc713f6fce1b9de47a266307f1829636b9
prerequisite-patch-id: e409bf71b615ed23828696cc9c1c6085b631c479
prerequisite-patch-id: 5b89b41d7c729c23b3b1fff9b5f572f4baa915ca
prerequisite-patch-id: acd08e91e5e2c6f4799879e48481b07167c0a400
prerequisite-patch-id: c9f2942207341ad4f450b20f049199f35188c02a
prerequisite-patch-id: dd62ebff6be6a2e2d32743812d35ec54daf91d00
prerequisite-patch-id: 3a6e9752793f2d7b084008b47daed10ea572064a
prerequisite-patch-id: 3338cdc5915c1e6b991067d3a7afb734c182663e
prerequisite-patch-id: a3026c858ffdfd3bfafc837e72c67fffe46021eb
prerequisite-patch-id: d16278d739341083377fb2a98862ba1932ad6ced
prerequisite-patch-id: 0a3f6abb3fe8634d297cf87f9cbec00664608ba4
prerequisite-patch-id: a54681edd36e35393b7b1906fca59e2106c6e313
prerequisite-patch-id: f45e9442cfb4307f9c4180c90da9e997a253da63
prerequisite-patch-id: 09258ed9eead7a956528f2d790be5cacb4e5fc03
prerequisite-patch-id: a5227c41a691f66e5b1c7e1259a101ec7345724d
prerequisite-patch-id: 0ff2d43b5d614f9fd8b4e6fb871bef2fda4ef3bf
prerequisite-patch-id: e4cb4b9831d278facc73913d3283038fdbe59ee0
prerequisite-patch-id: 003035cc99f02794043818256ba0ef657872d511
prerequisite-patch-id: c0e0e2054f26cdec216778d6e1aa8b51aa3210a6
prerequisite-patch-id: 5d23c938843176de2a02987d2832f31fe5df7fcf
prerequisite-patch-id: 539c31d705de3729782b86aeb7ca9dfd00d04208
prerequisite-patch-id: 430488f50f36039338965ab1fc28d83f02dbc9fd
prerequisite-patch-id: 359ddad8a3fb36f171c96da5ce5ffacd7dd63e8f
prerequisite-patch-id: a98885b9d0e0655bb3161dc2c31fd92a844a5e4e
prerequisite-patch-id: 63e6b911ad6700c1039802235c0a6d5870957f8c
prerequisite-patch-id: 6d481dd14afe58a17230318f418ec3fe0d327faf
prerequisite-patch-id: 7675b5ea6f01d7dadb8df43c8532daa4167fc92f
prerequisite-patch-id: 3fcc510f8f38ce63b24f02d48257ce8d7079b61e
prerequisite-patch-id: 037f677639a12a986e024f9a66df2def301925d4
prerequisite-patch-id: 888f7d13f882fdd0a01bc3fcbe008e9e56394bd3
prerequisite-patch-id: 7db9bb6a1d3de3667a0880f8a75c24ce62e52ecd
prerequisite-patch-id: b898d117a21bddc176ae19937b03e733df72f821
prerequisite-patch-id: 3ca81fae4cb388c4970e908fb63cc99bc1cdd008
prerequisite-patch-id: 80aead6484e36f52cb6cc7fd7d9e0326d8296860
prerequisite-patch-id: 2f1bd3efac328030dd8efe28fb95f84603868043
prerequisite-patch-id: 047b4fb1894b92109aa7afcebd7d5c7988ec5379
prerequisite-patch-id: 222630a15afc952683d954a3c66617a223546de0
prerequisite-patch-id: 3c55edb41f1e25920a350ce1c6f31fde67fee45a
prerequisite-patch-id: 99977ae9253a961b85331b9808c1feff0c2cc38e
prerequisite-patch-id: ab1dc085eeaaca2d88d4ffa3bb7a9eb70d479473
prerequisite-patch-id: 9e3edab83e9fc008b2dc254fb3b548ddd8f8b5f8
prerequisite-patch-id: 53c415b2ee6aae7cc5f446903fd6fb68b22d5692
prerequisite-patch-id: 0de75678d071f174c865afa2915be4df1aef8c8d
prerequisite-patch-id: 14840d7e2441e2110c1e147941744be637c9595f
prerequisite-patch-id: a7706e25f5951ec41e6b662c1704df8d20662d77
prerequisite-patch-id: 1e1be31d7ddc47eec9193164defa3e5c473b6ef1
prerequisite-patch-id: d13035abab3ff342753f5bf87b53bbf06a02c6a1
prerequisite-patch-id: 79fcbe1428667a6e0059cea9cbfd62d38d114930
prerequisite-patch-id: 591dd358a559fa83d46149f74f80ff0f2a98da7f
prerequisite-patch-id: 4d40e704139dc4b0ec2529c49f096d86a4e4dc5f
prerequisite-patch-id: c02813140f0c1c3d783f5643e34ef8e175cb20ea
prerequisite-patch-id: af52b9126b3ecbb6a39d855c2396a92fce275385
prerequisite-patch-id: f03075f14ae8d9a70c222abd747e07cca7f4ec09
prerequisite-patch-id: fab5ed6c3e94e533cca876424d076c8fd375e97b
prerequisite-patch-id: 97e013b8da4107d9634013a64189b101c0386301
prerequisite-patch-id: 68714bb4df41ab5d509440bf271e87e53d6aeb12
prerequisite-patch-id: 6df2cd98d019fffab2dc6d5a9ace72818107e0c5
prerequisite-patch-id: 5b9d4eeccd5ade672fbe3c40ef8e6f57f454fec0
prerequisite-patch-id: d4299778bdefe71a07621fdd7a3bd6c9bb6e241f
prerequisite-patch-id: f6872077ea6857f304dc5b1918827e84675c4c21
--
2.34.1
^ permalink raw reply
* Re: [PATCH v7 0/6] Enable I2C on SA8255p Qualcomm platforms
From: Andi Shyti @ 2026-06-18 11:25 UTC (permalink / raw)
To: Praveen Talari
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio,
Bjorn Andersson, Mukesh Kumar Savaliya, Viken Dadhaniya,
Mattijs Korpershoek, linux-arm-msm, linux-i2c, devicetree,
linux-kernel, bjorn.andersson, konrad.dybcio, aniket.randive,
chandana.chiluveru, prasad.sodagudi, Krzysztof Kozlowski,
Nikunj Kela
In-Reply-To: <ajNLmO9UBH_Xs3W2@zenone.zhora.eu>
Hi Praveen,
> > Praveen Talari (6):
> > dt-bindings: i2c: Describe SA8255p
> > i2c: qcom-geni: Isolate serial engine setup
> > i2c: qcom-geni: Move resource initialization to separate function
> > i2c: qcom-geni: Use resources helper APIs in runtime PM functions
> > i2c: qcom-geni: Store of_device_id data in driver private struct
> > i2c: qcom-geni: Enable I2C on SA8255p Qualcomm platforms
>
> merged to i2c/i2c-for-7.2.
sorry, 7.3. It's too late to send it for this cycle.
Andi
^ permalink raw reply
* Re: [PATCH 08/12] rtc: rzn1: Dynamically calculate synchronization delay based on clock rate
From: Lad, Prabhakar @ 2026-06-18 11:23 UTC (permalink / raw)
To: Wolfram Sang
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <ajJ95P-jxChrTY9w@shikoro>
Hi Wolfram,
Thank you for the review.
On Wed, Jun 17, 2026 at 11:58 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
>
> As mentioned in another thread:
>
> > drivers/rtc/rtc-rzn1.c | 19 ++++++++++++++-----
> > 1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> > index 06339adae71f..bc6af59744e4 100644
> > --- a/drivers/rtc/rtc-rzn1.c
> > +++ b/drivers/rtc/rtc-rzn1.c
> > @@ -71,6 +71,7 @@ struct rzn1_rtc {
> > */
> > spinlock_t ctl1_access_lock;
> > struct rtc_time tm_alarm;
> > + unsigned long sync_time;
> > int alarm_irq;
> > int sec_irq;
> > bool alarm_enabled;
>
> rate = 32768 here...
>
Agreed (in the rzn1_rtc_probe, to be precise).
> > + rtc->sync_time = DIV_ROUND_UP(2 * NSEC_PER_MSEC, rate);
> > +
> > }
>
> ... and move this to the main body of the function.
>
>
> Then, we should have all values always initialized.
>
Agreed, I will fix it in v2.
Cheers,
Prabhakar
^ permalink raw reply
* Re: [PATCH v3 2/3] media: i2c: add os02g10 image sensor driver
From: Vladimir Zapolskiy @ 2026-06-18 11:06 UTC (permalink / raw)
To: Elgin Perumbilly, sakari.ailus@linux.intel.com,
laurent.pinchart@ideasonboard.com
Cc: Tarang Raval, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hans Verkuil, Hans de Goede,
Mehdi Djait, Sylvain Petinot, Benjamin Mugnier,
Bryan O'Donoghue, Himanshu Bhavani, Heimir Thor Sverrisson,
Jingjing Xiong, Svyatoslav Ryhel, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <MA0P287MB2178300B0541EC81B91312F588E32@MA0P287MB2178.INDP287.PROD.OUTLOOK.COM>
On 6/18/26 09:22, Elgin Perumbilly wrote:
> Hi Vladimir,
>
> Thank you for the review.
>
> I have addressed all of the comments except for two, where I am not entirely
> sure about the requested changes. Could you please take a look at the points
> below and let me know your opinion?
>
>> On 4/24/26 12:25, Elgin Perumbilly wrote:
>>> Add a v4l2 subdevice driver for the Omnivision os02g10 sensor.
>>>
>>> The Omnivision os02g10 is a CMOS image sensor with an active array size of
>>> 1920 x 1080.
>>>
>>> The following features are supported:
>>> - Manual exposure an gain control support
>>> - vblank/hblank control support
>>> - vflip/hflip control support
>>> - Test pattern control support
>>> - Supported resolution: 1920 x 1080 @ 30fps (SBGGR10)
>>>
>>> Signed-off-by: Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>
>>> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
>
> ...
>
>>> +#include <linux/array_size.h>
>>> +#include <linux/bitops.h>
>>> +#include <linux/cleanup.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/container_of.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/err.h>
>>> +#include <linux/gpio/consumer.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/module.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/property.h>
>>> +#include <linux/regulator/consumer.h>
>>> +#include <linux/units.h>
>>> +#include <linux/types.h>
>>> +#include <linux/time.h>
>>> +#include <linux/regmap.h>
>>
>> Please sort the list of includes in alphabetical order, also you
>> may consider to shrink the list by removing quite many inherited
>> includes.
>
> Some maintainers prefer the "include what you use" approach, like Andy,
> so I added all the headers that are directly used. Should I now remove
> any inherited includes?
Yes, here opinions may vary, that's why I asked for sorting and to
consider to remove some of the redundant headers. In my personal opinion
this type of excessive information is not needed, especially if it is
justified only by probable and far future trivial clean-up work.
>>> +#include <media/v4l2-cci.h>
>>> +#include <media/v4l2-ctrls.h>
>>> +#include <media/v4l2-device.h>
>>> +#include <media/v4l2-fwnode.h>
>>> +#include <media/v4l2-mediabus.h>
>
> ...
>
>>> +static int os02g10_set_framefmt(struct os02g10 *os02g10,
>>> + struct v4l2_subdev_state *state)
>>> +{
>>> + const struct v4l2_mbus_framefmt *format;
>>> + const struct os02g10_mode *mode;
>>> + int ret = 0;
>>> +
>>> + format = v4l2_subdev_state_get_format(state, 0);
>>> + mode = v4l2_find_nearest_size(supported_modes,
>>> + ARRAY_SIZE(supported_modes), width,
>>> + height, format->width, format->height);
>>> +
>>> + cci_write(os02g10->cci, OS02G10_REG_V_START, mode->y_start, &ret);
>>> + cci_write(os02g10->cci, OS02G10_REG_V_SIZE, mode->height, &ret);
>>> + cci_write(os02g10->cci, OS02G10_REG_V_SIZE_MIPI, mode->height, &ret);
>>> + cci_write(os02g10->cci, OS02G10_REG_H_START, mode->x_start, &ret);
>>> + cci_write(os02g10->cci, OS02G10_REG_H_SIZE, mode->width, &ret);
>>> + cci_write(os02g10->cci, OS02G10_REG_H_SIZE_MIPI, mode->width, &ret);
>>> +
>>> + return ret;
>>
>> Just "return 0" here, and remove the local variable.
>
> Could you clarify why this should return 0? The local ret is passed to all
> cci_write() calls so that any write error is propagated. Returning 0 here
> would appear to suppress those errors and always report success.
>
My bad, yes, here please leave 'return ret' as is, I was confused and
misleaded by initialization of the local variable to zero, which is
redundant, and I'd suggest to remove this initialization.
--
Best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH v3 2/3] media: i2c: add os02g10 image sensor driver
From: sakari.ailus @ 2026-06-18 10:56 UTC (permalink / raw)
To: Elgin Perumbilly
Cc: Vladimir Zapolskiy, laurent.pinchart@ideasonboard.com,
Tarang Raval, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hans Verkuil, Hans de Goede,
Mehdi Djait, Sylvain Petinot, Benjamin Mugnier,
Bryan O'Donoghue, Himanshu Bhavani, Heimir Thor Sverrisson,
Jingjing Xiong, Svyatoslav Ryhel, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <MA0P287MB2178300B0541EC81B91312F588E32@MA0P287MB2178.INDP287.PROD.OUTLOOK.COM>
Hi Elgin, Vladimir,
On Thu, Jun 18, 2026 at 06:22:14AM +0000, Elgin Perumbilly wrote:
> Hi Vladimir,
>
> Thank you for the review.
>
> I have addressed all of the comments except for two, where I am not entirely
> sure about the requested changes. Could you please take a look at the points
> below and let me know your opinion?
>
> > On 4/24/26 12:25, Elgin Perumbilly wrote:
> > > Add a v4l2 subdevice driver for the Omnivision os02g10 sensor.
> > >
> > > The Omnivision os02g10 is a CMOS image sensor with an active array size of
> > > 1920 x 1080.
> > >
> > > The following features are supported:
> > > - Manual exposure an gain control support
> > > - vblank/hblank control support
> > > - vflip/hflip control support
> > > - Test pattern control support
> > > - Supported resolution: 1920 x 1080 @ 30fps (SBGGR10)
> > >
> > > Signed-off-by: Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>
> > > Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
>
> ...
>
> > > +#include <linux/array_size.h>
> > > +#include <linux/bitops.h>
> > > +#include <linux/cleanup.h>
> > > +#include <linux/clk.h>
> > > +#include <linux/container_of.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/err.h>
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/module.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/property.h>
> > > +#include <linux/regulator/consumer.h>
> > > +#include <linux/units.h>
> > > +#include <linux/types.h>
> > > +#include <linux/time.h>
> > > +#include <linux/regmap.h>
> >
> > Please sort the list of includes in alphabetical order, also you
> > may consider to shrink the list by removing quite many inherited
> > includes.
>
> Some maintainers prefer the "include what you use" approach, like Andy,
> so I added all the headers that are directly used. Should I now remove
> any inherited includes?
Andy has been indeed asking to include what you use; this way cleaning up
the headers in the future becomes possible.
>
> > > +#include <media/v4l2-cci.h>
> > > +#include <media/v4l2-ctrls.h>
> > > +#include <media/v4l2-device.h>
> > > +#include <media/v4l2-fwnode.h>
> > > +#include <media/v4l2-mediabus.h>
>
> ...
>
> > > +static int os02g10_set_framefmt(struct os02g10 *os02g10,
> > > + struct v4l2_subdev_state *state)
> > > +{
> > > + const struct v4l2_mbus_framefmt *format;
> > > + const struct os02g10_mode *mode;
> > > + int ret = 0;
> > > +
> > > + format = v4l2_subdev_state_get_format(state, 0);
> > > + mode = v4l2_find_nearest_size(supported_modes,
> > > + ARRAY_SIZE(supported_modes), width,
> > > + height, format->width, format->height);
> > > +
> > > + cci_write(os02g10->cci, OS02G10_REG_V_START, mode->y_start, &ret);
> > > + cci_write(os02g10->cci, OS02G10_REG_V_SIZE, mode->height, &ret);
> > > + cci_write(os02g10->cci, OS02G10_REG_V_SIZE_MIPI, mode->height, &ret);
> > > + cci_write(os02g10->cci, OS02G10_REG_H_START, mode->x_start, &ret);
> > > + cci_write(os02g10->cci, OS02G10_REG_H_SIZE, mode->width, &ret);
> > > + cci_write(os02g10->cci, OS02G10_REG_H_SIZE_MIPI, mode->width, &ret);
> > > +
> > > + return ret;
> >
> > Just "return 0" here, and remove the local variable.
>
> Could you clarify why this should return 0? The local ret is passed to all
> cci_write() calls so that any write error is propagated. Returning 0 here
> would appear to suppress those errors and always report success.
The code seems fine to me.
--
Regards,
Sakari Ailus
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: media: i2c: Add OmniVision OG0VA1B
From: Vladimir Zapolskiy @ 2026-06-18 10:55 UTC (permalink / raw)
To: Wenmeng Liu, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sakari Ailus
Cc: linux-media, devicetree, linux-kernel
In-Reply-To: <20260618-og0va1b-v1-1-dda71bb83009@oss.qualcomm.com>
Hello Wenmeng.
On 6/18/26 13:37, Wenmeng Liu wrote:
> Add devicetree binding for OmniVision OG0VA1B image sensor.
> OmniVision OG0VA1B is an image sensor, which produces frames in 10-bit
> raw output format (Y10) over a 1-lane MIPI CSI-2 interface and supports
> the 640x480 (VGA) resolution.
>
> Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
> ---
> .../bindings/media/i2c/ovti,og0va1b.yaml | 104 +++++++++++++++++++++
> MAINTAINERS | 6 ++
> 2 files changed, 110 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,og0va1b.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,og0va1b.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..302afc41bb776f75c08b26ac2f04014f8cbea4fa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,og0va1b.yaml
> @@ -0,0 +1,104 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ovti,og0va1b.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: OmniVision OG0VA1B Image Sensor
> +
> +maintainers:
> + - Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
> +
> +description:
> + The OmniVision OG0VA1B is a 1/10-inch black and white CMOS VGA (640x480)
> + image sensor. It is controlled over an I2C-compatible SCCB bus and transmits
> + images on a 1-lane MIPI CSI-2 output interface.
> +
> +allOf:
> + - $ref: /schemas/media/video-interface-devices.yaml#
> +
> +properties:
> + compatible:
> + const: ovti,og0va1b
So far I don't see a difference from the neighbouring ovti,og0ve1b.yaml,
most likely it is just the same device, and even if it is not, it'd make
sense to consider and add OG0VE1B support into the existing dt binding
documentation.
--
Best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH 2/2] media: i2c: og0va1b: Add OmniVision OG0VA1B camera sensor
From: Vladimir Zapolskiy @ 2026-06-18 10:50 UTC (permalink / raw)
To: Wenmeng Liu, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sakari Ailus
Cc: linux-media, devicetree, linux-kernel
In-Reply-To: <20260618-og0va1b-v1-2-dda71bb83009@oss.qualcomm.com>
Hello Wenmeng.
On 6/18/26 13:37, Wenmeng Liu wrote:
> Add V4L2 sub device driver for OmniVision OG0VA1B image sensor.
> OmniVision OG0VA1B is an image sensor, which produces frames in 10-bit
> raw output format (Y10) over a 1-lane MIPI CSI-2 interface and supports
> the 640x480 (VGA) resolution.
>
> Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
> ---
> MAINTAINERS | 1 +
> drivers/media/i2c/Kconfig | 10 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/og0va1b.c | 867 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 879 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5aa846c8479b20651291d5bd2e316308310f826c..85a06eb9eacc410a565b80d56979eaa565515d0e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19891,6 +19891,7 @@ M: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
> L: linux-media@vger.kernel.org
> S: Maintained
> F: Documentation/devicetree/bindings/media/i2c/ovti,og0va1b.yaml
> +F: drivers/media/i2c/og0va1b.c
>
> OMNIVISION OG0VE1B SENSOR DRIVER
> M: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 5d173e0ecf424f2f204f8d426be818e44357f8e4..56680772f5f47b4629c4e17f5a5feba08b1d94fc 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -363,6 +363,16 @@ config VIDEO_OG01A1B
> To compile this driver as a module, choose M here: the
> module will be called og01a1b.
>
> +config VIDEO_OG0VA1B
> + tristate "OmniVision OG0VA1B sensor support"
> + select V4L2_CCI_I2C
> + help
> + This is a Video4Linux2 sensor driver for the OmniVision
> + OG0VA1B camera.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called og0va1b.
> +
> config VIDEO_OG0VE1B
> tristate "OmniVision OG0VE1B sensor support"
> select V4L2_CCI_I2C
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index e45359efe0e41e13e3c0869e5ead7d6cf4aca3a7..c60851c7fe07e3bdc511c5f482525ba7a044f48e 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -83,6 +83,7 @@ obj-$(CONFIG_VIDEO_MT9V011) += mt9v011.o
> obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o
> obj-$(CONFIG_VIDEO_MT9V111) += mt9v111.o
> obj-$(CONFIG_VIDEO_OG01A1B) += og01a1b.o
> +obj-$(CONFIG_VIDEO_OG0VA1B) += og0va1b.o
> obj-$(CONFIG_VIDEO_OG0VE1B) += og0ve1b.o
> obj-$(CONFIG_VIDEO_OS05B10) += os05b10.o
> obj-$(CONFIG_VIDEO_OV01A10) += ov01a10.o
> diff --git a/drivers/media/i2c/og0va1b.c b/drivers/media/i2c/og0va1b.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..f0505b7ba7f329ad57ffafa8f90a24204f002d3c
> --- /dev/null
> +++ b/drivers/media/i2c/og0va1b.c
> @@ -0,0 +1,867 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * OmniVision OG0VA1B Camera Sensor Driver
> + *
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <media/v4l2-cci.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define OG0VA1B_REG_CHIP_ID CCI_REG16(0x300a)
> +#define OG0VA1B_CHIP_ID 0xC756
This is the same chip id as of the OG0VE1B sensor device.
What's the difference between these two sensors, and do you find it possible
to add support of OG0VA1B sensor/modes into OG0VE1B sensor driver? Or is
it just the same device?
Hardware specifics described in dt changes also does not show a difference
in comparison to ovti,og0ve1b.yaml.
--
Best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH 07/12] rtc: rzn1: fix alarm range check truncation on 32-bit systems
From: Lad, Prabhakar @ 2026-06-18 10:49 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Miquel Raynal, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Wolfram Sang, linux-rtc,
linux-renesas-soc, devicetree, linux-kernel, Biju Das,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260617165538dad7e36b@mail.local>
Hi Alexandre,
On Wed, Jun 17, 2026 at 5:55 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 15/06/2026 16:48:00+0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > alarm and farest were declared as unsigned long, but
> > rtc_tm_to_time64() returns time64_t (s64). On 32-bit systems where
> > unsigned long is 32 bits, the assignment silently truncates the upper
> > 32 bits of the timestamp.
> >
> > Fix by declaring alarm and farest as time64_t and replacing
> > time_after() with a direct signed comparison, which is correct for
> > time64_t values that will never realistically overflow.
> >
>
> I'd argue that this is never going to overflow ever as unsigned long
> gets you to 2106 which is way past the usable range of the RTC so there
> is a trade off between the size you are going to take on the stack and
> the actual usefulness of the fix.
>
While it's true that unsigned long lasts until 2106 (well past this
RTC's practical lifetime), rtc_tm_to_time64() explicitly returns
time64_t. Using unsigned long causes silent truncation and types
mismatch with the API, which modern static analyzers flag. Given that
this function is not deeply nested, the 8-byte stack trade-off seems
worth it for type cleanliness and consistency. What do you think?
Cheers,
Prabhakar
^ permalink raw reply
* Re: [PATCH v1 1/9] dt-bindings: reset: altr: add COMBOPHY_RESET for Agilex5
From: Philipp Zabel @ 2026-06-18 10:48 UTC (permalink / raw)
To: Tanmay Kathpalia, linux-mmc
Cc: ulf.hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
devicetree, linux-kernel
In-Reply-To: <20260511202132.5597-2-tanmay.kathpalia@altera.com>
On Mo, 2026-05-11 at 13:21 -0700, Tanmay Kathpalia wrote:
> Add COMBOPHY_RESET definition at index 38 for the combo PHY reset
> control on Altera Agilex5 SoCs. This reset is used by peripherals
> such as the SD/eMMC controller that share the combo PHY.
>
> Signed-off-by: Tanmay Kathpalia <tanmay.kathpalia@altera.com>
> ---
> include/dt-bindings/reset/altr,rst-mgr-s10.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/dt-bindings/reset/altr,rst-mgr-s10.h b/include/dt-bindings/reset/altr,rst-mgr-s10.h
> index 04c4d0c6fd34..c2505b9eb63e 100644
> --- a/include/dt-bindings/reset/altr,rst-mgr-s10.h
> +++ b/include/dt-bindings/reset/altr,rst-mgr-s10.h
> @@ -22,7 +22,7 @@
> #define USB0_RESET 35
> #define USB1_RESET 36
> #define NAND_RESET 37
> -/* 38 is empty */
> +#define COMBOPHY_RESET 38
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
I'll pick this up into reset/fixes after v7.2-rc1.
regards
Philipp
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox