* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-06-04 0:30 UTC (permalink / raw)
To: Vladimir Zapolskiy, Bryan O'Donoghue, Vijay Kumar Tumati,
Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <5ca611b1-0663-4975-bd56-b1343851e5fd@linaro.org>
On 04/06/2026 01:07, Vladimir Zapolskiy wrote:
> On 6/4/26 00:18, Bryan O'Donoghue wrote:
>> On 03/06/2026 21:51, Vladimir Zapolskiy wrote:
>>>> Actually, one more thing, Why isn't TITAN TOP GDSC here?>>>> +
>>> If CSIPHYs are true subdevices under the umbrella CAMSS device and well
>>> described as subnodes, then likely none of power domains are needed
>>> to be
>>> repeatedly described in the children device nodes, since this
>>> information
>>> can be obtained from the parent device by the driver.
>>>
>>> Technically 'power-domains' property can be safely removed, I believe.
>>
>> The policy is to describe the power-domain dependency fully since DT
>> describes hardware not software architecture.
>
> It brings no contardiction to the statement I've given above, the needed
> power domans will be properly described in the parent device, and there
> is no
> sense to repeat the properties it again and again in every child subdevice.
>
>> Also for a very practical reason a sub-devices can probe/run
>> asynchronously of the parent device being active so in fact we do need
>> to describe the PDs fully.
>
> In opposite to the above this one is precisely a software centric argument,
> which should be excluded from the consideration, as well it's not a big
> deal to make a proper async initialization, removing excessive dt
> properties
> is worth it.
>
Right look forget about that.
- DT requires you to describe your hardware. You're not entitled to have
some other device vote for a clock or a PD you rely on.
That's exactly the type of downstream short cut we are trying to zap.
- In our case we also need to vote on PDs individually when the PHY
is active.
In extremis say we are only running the TPG then we have no reason to
vote for CSIPHY specific rails or operating points in the parent device.
We could make the parent power-domain argument for CAMSS and CCI but we
have TITAN_TOP_GDSC in CCI specifically because we have to model the
hardware - including the PDs for that device.
If tomorrow we put CCI as a sub-device of top-level CAMSS, that won't
negate the need to include that GDSC.
---
bod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vladimir Zapolskiy @ 2026-06-04 0:07 UTC (permalink / raw)
To: Bryan O'Donoghue, Vijay Kumar Tumati, Bryan O'Donoghue,
Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <29e8491f-20e8-4082-8943-66bee7e3af1d@kernel.org>
On 6/4/26 00:18, Bryan O'Donoghue wrote:
> On 03/06/2026 21:51, Vladimir Zapolskiy wrote:
>>> Actually, one more thing, Why isn't TITAN TOP GDSC here?>>>> +
>> If CSIPHYs are true subdevices under the umbrella CAMSS device and well
>> described as subnodes, then likely none of power domains are needed to be
>> repeatedly described in the children device nodes, since this information
>> can be obtained from the parent device by the driver.
>>
>> Technically 'power-domains' property can be safely removed, I believe.
>
> The policy is to describe the power-domain dependency fully since DT
> describes hardware not software architecture.
It brings no contardiction to the statement I've given above, the needed
power domans will be properly described in the parent device, and there is no
sense to repeat the properties it again and again in every child subdevice.
> Also for a very practical reason a sub-devices can probe/run
> asynchronously of the parent device being active so in fact we do need
> to describe the PDs fully.
In opposite to the above this one is precisely a software centric argument,
which should be excluded from the consideration, as well it's not a big
deal to make a proper async initialization, removing excessive dt properties
is worth it.
--
Best wishes,
Vladimir
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Vladimir Zapolskiy @ 2026-06-03 23:58 UTC (permalink / raw)
To: Bryan O'Donoghue, Bryan O'Donoghue, Dmitry Baryshkov
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <41ab1685-3d0a-4c8b-b7a9-6e9299c48400@kernel.org>
On 6/4/26 00:12, Bryan O'Donoghue wrote:
> On 03/06/2026 21:42, Vladimir Zapolskiy wrote:
>>>> Split configurations explicitly use other lanes for clocks. E.g. check
>>>> the RB5 Navigation schematics, CAM0B connector.
>>> Can you please check:
>>>
>>> CSI_3PHASE_COMMON.CSI_COMMON_CTRL5
>>>
>>> 0 LN0_PWRDN_B Lane 0
>>> ...
>>> 7 LNCK_PWRDN_B Clock Lane
>> Please note that media devices have a numeration scheme of lanes starting
>> from 1 (it'd be easy to check/confirm it), for instance today CAMSS has
>> lane numeration starting from 0 is out of the accepted scheme, and here
>> it'd be better to correct it and not enter the same pit.
>
> Yes fair point CAMSS has done this wrong since forever. data-lanes = <1
> 2 3 4> => LN0, LN1, LN2, LN3>
>> I don't have access to the IP spec, anyway I do not grasp it, where are
>> 8 lanes on the CSIPHY found? Each CSIPHY IP has 4+1 D-PHY lanes, not 8.
>
> Max CSI2 data-lanes is 8
>
> #define V4L2_MBUS_CSI2_MAX_DATA_LANES 8
>
Judging by the name that's something V4L2 or in other words software
specific, while I do reference to the actual pads of Qualcomm SoCs,
and I do not see 8 differential pairs per a CSIPHY, but only 5 lanes.
If that's correct, the data/clock lane numbers should lay in 1 to 5 range.
> That doesn't really explain why this register has seven data-lanes and
> one-clock lane.
>
> It just is what it is.
>
--
Best wishes,
Vladimir
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: phy: qcom,usb-hs-phy: add qcom,vendor-init-seq
From: Dmitry Baryshkov @ 2026-06-03 23:02 UTC (permalink / raw)
To: github.com, me
Cc: linux-phy, devicetree, linux-arm-msm, Vinod Koul, Neil Armstrong,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson
In-Reply-To: <66905f4914dc022629158ac02df1c7df@herrie.org>
On Wed, Jun 03, 2026 at 06:09:18PM +0200, me@herrie.org wrote:
> On 2026-06-03 15:57, Dmitry Baryshkov wrote:
> > On Wed, Jun 03, 2026 at 07:48:08AM +0200, Herman van Hazendonk wrote:
> > > Add an optional "qcom,vendor-init-seq" property carrying raw ULPI
> > > (address, value) pairs that are written after PHY reset.
> > >
> > > Unlike the existing "qcom,init-seq" property, the address field is
> > > NOT offset by ULPI_EXT_VENDOR_SPECIFIC, so the new property can
> > > reach the standard ULPI vendor register range (0x30-0x3f). MSM8x60-
> > > class hardware needs this range to programme pre-emphasis, HS driver
> > > slope and CDR auto-reset bits the legacy msm_otg driver used to set
> > > via platform data.
> >
> > Are those register writes specific to the device or to the whole
> > platform? In the latter case please extend the driver to write them.
>
> Looking at every MSM8x60 reference kernel I could find (Qualcomm's own
> msm8x60 board, HP TouchPad / APQ8060, and some HTC/Saumsung MSM8660
> devices), the writes split into two groups:
>
> Platform-level (same across all MSM8x60 hardware):
> - reg 0x36 bits 1+2: CDR auto-reset disabled, SE1 gating disabled
> - reg 0x32 bits [5:4]: pre-emphasis at 20%
>
> Board-specific:
> - reg 0x32 bits [3:0]: HS driver slope — HP TouchPad uses 5, HTC
> devices use 1. This clearly depends on board layout (trace length,
> connector loading, etc.).
>
> So the platform-level writes should move unconditionally into the driver
> behind a match-data flag for the MSM8x60-class compatible, and only the
> HS driver slope value belongs in DT.
Looks like it. Please hardcode the value for your platform in the driver
(with the comment), meanwhile we can try looking up the actual values.
>
> > If you have questions regarding any of the bitfields, please ask here,
> > we can try finding the data for those.
>
> One thing I haven't been able to nail down: every HTC MSM8660 board also
> writes 0x0C (bits 2 and 3) to ULPI register 0x31 via phy_init_seq, but
> the HP TouchPad webOS kernel never sets that register through its named
> platform fields — and USB works fine on the TouchPad without it. Do you
> know what those two bits in 0x31 control? If they are needed on CI 45nm
> PHY silicon but not on the APQ8060 PHY variant, I should probably leave
> them out of the driver and note why.
>
> I don't have any technical documentation for the S3 SoC family, except for
> the legacy kernels (HP, Samsung, HTC) and a bunch of various HP TouchPad
> variants.
Again, please use the value for your current platform, add a note to the
commit message and to the code.
--
With best wishes
Dmitry
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Vijay Kumar Tumati @ 2026-06-03 22:39 UTC (permalink / raw)
To: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <20260523-x1e-csi2-phy-v8-2-a85668459521@linaro.org>
On 5/22/2026 7:48 PM, Bryan O'Donoghue wrote:
> Add a new MIPI CSI2 driver in DPHY mode initially. The entire set of
> existing CAMSS CSI PHY init sequences are imported in order to save time
> and effort in later patches.
>
> The following devices are supported in this drop:
> "qcom,x1e80100-csi2-phy"
>
> In-line with other PHY drivers the process node is included in the name.
> Data-lane and clock lane positioning and polarity selection via newly
> amended struct phy_configure_opts_mipi_dphy{} is supported.
>
> The Qualcomm 3PH class of PHYs can do both DPHY and CPHY mode. For now only
> DPHY is supported.
>
> In porting some of the logic over from camss-csiphy*.c to here its also
> possible to rationalise some of the code.
>
> In particular use of regulator_bulk and clk_bulk as well as dropping the
> seemingly useless and unused interrupt handler.
>
> The PHY sequences and a lot of the logic that goes with them are well
> proven in CAMSS and mature so the main thing to watch out for here is how
> to get the right sequencing of regulators, clocks and register-writes.
>
> The register init sequence table is imported verbatim from the existing
> CAMSS csiphy driver. A follow-up series will rework the table to extract
> the repetitive per-lane pattern into a loop.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> MAINTAINERS | 10 +
> drivers/phy/qualcomm/Kconfig | 14 +
> drivers/phy/qualcomm/Makefile | 5 +
> drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 376 +++++++++++++++++++
> drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c | 402 +++++++++++++++++++++
> drivers/phy/qualcomm/phy-qcom-mipi-csi2.h | 95 +++++
> 6 files changed, 902 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 63389fea5d150..3b5da8a40383f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22018,6 +22018,16 @@ S: Maintained
> F: Documentation/devicetree/bindings/media/qcom,*-iris.yaml
> F: drivers/media/platform/qcom/iris/
>
> +QUALCOMM MIPI CSI2 PHY DRIVER
> +M: Bryan O'Donoghue <bod@kernel.org>
> +L: linux-phy@lists.infradead.org
> +L: linux-media@vger.kernel.org
> +L: linux-arm-msm@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/phy/qcom,*-csi2-phy.yaml
> +F: drivers/phy/qualcomm/phy-qcom-mipi-csi2*.c
> +F: drivers/phy/qualcomm/phy-qcom-mipi-csi2*.h
> +
> QUALCOMM NAND CONTROLLER DRIVER
> M: Manivannan Sadhasivam <mani@kernel.org>
> L: linux-mtd@lists.infradead.org
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index 60a0ead127fa9..779a3511ba852 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -28,6 +28,20 @@ config PHY_QCOM_EDP
> Enable this driver to support the Qualcomm eDP PHY found in various
> Qualcomm chipsets.
>
> +config PHY_QCOM_MIPI_CSI2
> + tristate "Qualcomm MIPI CSI2 PHY driver"
> + depends on ARCH_QCOM || COMPILE_TEST
> + depends on OF
> + depends on PM
> + depends on COMMON_CLK
> + select GENERIC_PHY
> + select GENERIC_PHY_MIPI_DPHY
> + help
> + Enable this to support the MIPI CSI2 PHY driver found in various
> + Qualcomm chipsets. This PHY is used to connect MIPI CSI2
> + camera sensors to the CSI Decoder in the Qualcomm Camera Subsystem
> + CAMSS.
> +
> config PHY_QCOM_IPQ4019_USB
> tristate "Qualcomm IPQ4019 USB PHY driver"
> depends on OF && (ARCH_QCOM || COMPILE_TEST)
> diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
> index b71a6a0bed3f1..382cb594b06b6 100644
> --- a/drivers/phy/qualcomm/Makefile
> +++ b/drivers/phy/qualcomm/Makefile
> @@ -6,6 +6,11 @@ obj-$(CONFIG_PHY_QCOM_IPQ4019_USB) += phy-qcom-ipq4019-usb.o
> obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA) += phy-qcom-ipq806x-sata.o
> obj-$(CONFIG_PHY_QCOM_M31_USB) += phy-qcom-m31.o
> obj-$(CONFIG_PHY_QCOM_M31_EUSB) += phy-qcom-m31-eusb2.o
> +
> +phy-qcom-mipi-csi2-objs += phy-qcom-mipi-csi2-core.o \
> + phy-qcom-mipi-csi2-3ph-dphy.o
> +obj-$(CONFIG_PHY_QCOM_MIPI_CSI2) += phy-qcom-mipi-csi2.o
> +
> obj-$(CONFIG_PHY_QCOM_PCIE2) += phy-qcom-pcie2.o
>
> obj-$(CONFIG_PHY_QCOM_QMP_COMBO) += phy-qcom-qmp-combo.o phy-qcom-qmp-usbc.o
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> new file mode 100644
> index 0000000000000..86ec405820e62
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> @@ -0,0 +1,376 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Qualcomm MSM Camera Subsystem - CSIPHY Module 3phase v1.0
> + *
> + * Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2016-2025 Linaro Ltd.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/time64.h>
> +
> +#include "phy-qcom-mipi-csi2.h"
> +
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(offset, n) ((offset) + 0x4 * (n))
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET BIT(0)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE BIT(7)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B BIT(0)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID BIT(1)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL10_IRQ_CLEAR_CMD BIT(0)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(offset, n) ((offset) + 0xb0 + 0x4 * (n))
> +
> +#define CSIPHY_2PH_LN_CSI_2PHASE_CTRL9n(n) ((0x200 * (n)) + 0x24)
> +
> +/*
> + * 3 phase CSI has 19 common status regs with only 0-10 being used
> + * and 11-18 being reserved.
> + */
> +#define CSI_COMMON_STATUS_NUM 11
> +/*
> + * There are a number of common control registers
> + * The offset to clear the CSIPHY IRQ status starts @ 22
> + * So to clear CSI_COMMON_STATUS0 this is CSI_COMMON_CONTROL22, STATUS1 is
> + * CONTROL23 and so on
> + */
> +#define CSI_CTRL_STATUS_INDEX 22
> +
> +/*
> + * There are 43 COMMON_CTRL registers with regs after # 33 being reserved
> + */
> +#define CSI_CTRL_MAX 33
> +
> +#define CSIPHY_DEFAULT_PARAMS 0
> +#define CSIPHY_SETTLE_CNT_LOWER_BYTE 2
> +#define CSIPHY_SKEW_CAL 7
> +
> +/* 4nm 2PH v 2.1.2 2p5Gbps 4 lane DPHY mode */
> +static const struct
> +mipi_csi2phy_lane_regs lane_regs_x1e80100[] = {
> + /* Power up lanes 2ph mode */
> + {.reg_addr = 0x1014, .reg_data = 0xd5, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x101c, .reg_data = 0x7a, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x1018, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +
> + {.reg_addr = 0x0094, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x00a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0090, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0098, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0094, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0030, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0000, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0038, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x002c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0034, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x001c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0014, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x003c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0004, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0020, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0008, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {.reg_addr = 0x0010, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0094, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x005c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0060, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0064, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> +
> + {.reg_addr = 0x0e94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0ea0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e94, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e28, .reg_data = 0x04, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e00, .reg_data = 0x80, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e0c, .reg_data = 0xff, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e38, .reg_data = 0x1f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {.reg_addr = 0x0e10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> +
> + {.reg_addr = 0x0494, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x04a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0490, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0498, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0494, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0430, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0400, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0438, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x042c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0434, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x041c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0414, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x043c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0404, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0420, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0408, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {.reg_addr = 0x0410, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0494, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x045c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0460, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0464, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> +
> + {.reg_addr = 0x0894, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x08a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0890, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0898, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0894, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0830, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0800, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0838, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x082c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0834, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x081c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0814, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x083c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0804, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0820, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0808, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {.reg_addr = 0x0810, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0894, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x085c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0860, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0864, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> +
> + {.reg_addr = 0x0c94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0ca0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c94, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c00, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c38, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {.reg_addr = 0x0c10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c94, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0c5c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0c60, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0c64, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> +};
> +
> +static inline const struct mipi_csi2phy_device_regs *
> +csi2phy_dev_to_regs(struct mipi_csi2phy_device *csi2phy)
> +{
> + return &csi2phy->soc_cfg->reg_info;
> +}
> +
> +static void phy_qcom_mipi_csi2_hw_version_read(struct mipi_csi2phy_device *csi2phy)
> +{
> + const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> + u32 tmp;
> +
> + writel(CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
> +
> + tmp = readl_relaxed(csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 12));
> + csi2phy->hw_version = tmp;
> +
> + tmp = readl_relaxed(csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 13));
> + csi2phy->hw_version |= (tmp << 8) & 0xFF00;
> +
> + tmp = readl_relaxed(csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 14));
> + csi2phy->hw_version |= (tmp << 16) & 0xFF0000;
> +
> + tmp = readl_relaxed(csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 15));
> + csi2phy->hw_version |= (tmp << 24) & 0xFF000000;
> +
> + dev_dbg_once(csi2phy->dev, "CSIPHY 3PH HW Version = 0x%08x\n", csi2phy->hw_version);
> +}
> +
> +/*
> + * phy_qcom_mipi_csi2_reset - Perform software reset on CSIPHY module
> + * @phy_qcom_mipi_csi2: CSIPHY device
> + */
> +static void phy_qcom_mipi_csi2_reset(struct mipi_csi2phy_device *csi2phy)
> +{
> + const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> +
> + writel(CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET,
> + csi2phy->base + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
> + usleep_range(5000, 8000);
> + writel(0x0, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
> +}
> +
> +/*
> + * phy_qcom_mipi_csi2_settle_cnt_calc - Calculate settle count value
> + *
> + * Helper function to calculate settle count value. This is
> + * based on the CSI2 T_hs_settle parameter which in turn
> + * is calculated based on the CSI2 transmitter link frequency.
> + *
> + * Return settle count value or 0 if the CSI2 link frequency
> + * is not available
> + */
> +static u8 phy_qcom_mipi_csi2_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
> +{
> + u32 t_hs_prepare_max_ps;
> + u32 timer_period_ps;
> + u32 t_hs_settle_ps;
> + u8 settle_cnt;
> + u32 ui_ps;
> +
> + if (link_freq <= 0)
> + return 0;
> +
> + ui_ps = div_u64(PSEC_PER_SEC, link_freq);
> + ui_ps /= 2;
> + t_hs_prepare_max_ps = 85000 + 6 * ui_ps;
> + t_hs_settle_ps = t_hs_prepare_max_ps;
> +
> + timer_period_ps = div_u64(PSEC_PER_SEC, timer_clk_rate);
> + settle_cnt = t_hs_settle_ps / timer_period_ps - 6;
> +
> + return settle_cnt;
> +}
> +
> +static void
> +phy_qcom_mipi_csi2_gen2_config_lanes(struct mipi_csi2phy_device *csi2phy,
> + u8 settle_cnt)
> +{
> + const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> + const struct mipi_csi2phy_lane_regs *r = regs->init_seq;
> + int i, array_size = regs->lane_array_size;
> + u32 val;
> +
> + for (i = 0; i < array_size; i++, r++) {
> + switch (r->param_type) {
> + case CSIPHY_SETTLE_CNT_LOWER_BYTE:
> + val = settle_cnt & 0xff;
> + break;
> + case CSIPHY_SKEW_CAL:
> + /* TODO: support application of skew from dt flag */
> + continue;
> + default:
> + val = r->reg_data;
> + break;
> + }
> + writel(val, csi2phy->base + r->reg_addr);
> + if (r->delay_us)
> + udelay(r->delay_us);
> + }
> +}
> +
> +static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
> + struct mipi_csi2phy_stream_cfg *cfg)
> +{
> + const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> + struct mipi_csi2phy_lanes_cfg *lane_cfg = &cfg->lane_cfg;
> + u8 settle_cnt;
> + u8 val;
> + int i;
> +
> + settle_cnt = phy_qcom_mipi_csi2_settle_cnt_calc(cfg->link_freq, csi2phy->timer_clk_rate);
> +
> + /* Lane position enable in common reg offset */
> + val = BIT(lane_cfg->clk.pos);
> + for (i = 0; i < cfg->num_data_lanes; i++)
> + val |= BIT(lane_cfg->data[i].pos);
> +
> + writel(val, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
> +
> + /* Lane configuration for polarity @ CSIPHY-base + CTRL9 */
> + for (i = 0; i < cfg->num_data_lanes; i++) {
> + if (lane_cfg->data[i].pol) {
> + u8 pos = lane_cfg->data[i].pos;
> +
> + writel(BIT(2), csi2phy->base + CSIPHY_2PH_LN_CSI_2PHASE_CTRL9n(pos));
> + }
> + }
> +
> + if (lane_cfg->clk.pol)
> + writel(BIT(2), csi2phy->base + CSIPHY_2PH_LN_CSI_2PHASE_CTRL9n(lane_cfg->clk.pos));
> +
> + val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B;
> + writel(val, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
> +
> + val = 0x02;
> + writel(val, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 7));
> +
> + val = 0x00;
> + writel(val, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
> +
> + phy_qcom_mipi_csi2_gen2_config_lanes(csi2phy, settle_cnt);
> +
> + /* IRQ_MASK registers - disable all interrupts */
> + for (i = CSI_COMMON_STATUS_NUM; i < CSI_CTRL_STATUS_INDEX; i++) {
> + writel(0, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, i));
> + }
> +
> + return 0;
> +}
> +
> +static void
> +phy_qcom_mipi_csi2_lanes_disable(struct mipi_csi2phy_device *csi2phy,
> + struct mipi_csi2phy_stream_cfg *cfg)
> +{
> + const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> +
> + writel(0, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
> +
> + writel(0, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
> +}
> +
> +static const struct mipi_csi2phy_hw_ops phy_qcom_mipi_csi2_ops_3ph_1_0 = {
> + .hw_version_read = phy_qcom_mipi_csi2_hw_version_read,
> + .reset = phy_qcom_mipi_csi2_reset,
> + .lanes_enable = phy_qcom_mipi_csi2_lanes_enable,
> + .lanes_disable = phy_qcom_mipi_csi2_lanes_disable,
> +};
> +
> +static const char * const x1e_clks[] = {
> + "core",
> + "timer"
> +};
> +
> +static const char * const x1e_supplies[] = {
> + "vdda-0p9",
> + "vdda-1p2"
> +};
> +
> +static const char * const x1e_genpd_names[] = {
> + "mmcx",
> + "mx",
> +};
> +
> +const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e = {
> + .ops = &phy_qcom_mipi_csi2_ops_3ph_1_0,
> + .reg_info = {
> + .init_seq = lane_regs_x1e80100,
> + .lane_array_size = ARRAY_SIZE(lane_regs_x1e80100),
> + .common_regs_offset = 0x1000,
> + },
> + .supply_names = (const char **)x1e_supplies,
> + .num_supplies = ARRAY_SIZE(x1e_supplies),
> + .clk_names = (const char **)x1e_clks,
> + .num_clk = ARRAY_SIZE(x1e_clks),
> + .opp_clk = x1e_clks[0],
> + .timer_clk = x1e_clks[1],
> + .genpd_names = (const char **)x1e_genpd_names,
> + .num_genpd_names = ARRAY_SIZE(x1e_genpd_names),
> +};
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> new file mode 100644
> index 0000000000000..dfeff863a406f
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> @@ -0,0 +1,402 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025, Linaro Ltd.
> + */
> +#include <dt-bindings/phy/phy.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pm_opp.h>
> +#include <linux/phy/phy.h>
> +#include <linux/phy/phy-mipi-dphy.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#include "phy-qcom-mipi-csi2.h"
> +
> +static int
> +phy_qcom_mipi_csi2_set_clock_rates(struct mipi_csi2phy_device *csi2phy,
> + s64 link_freq)
> +{
> + struct device *dev = csi2phy->dev;
> + unsigned long opp_rate = link_freq / 4;
Btw, when this is running in the combo mode, is this 'link_freq'
particular to a sensor? If so, how do we ensure that we do not scale
down the clocks based on the 1+1 session, if it's started after a 2+1
mode that may have higher data rate? Meaning, where do we consolidate
the clock and vote?> + struct dev_pm_opp *opp;
> + long timer_rate;
> + int i, ret;
> +
> + opp = dev_pm_opp_find_freq_ceil(dev, &opp_rate);
> + if (IS_ERR(opp)) {
> + dev_err(csi2phy->dev, "Couldn't find ceiling for %lld Hz\n",
> + link_freq);
> + return PTR_ERR(opp);
> + }
> +
> + for (i = 0; i < csi2phy->pd_list->num_pds; i++) {
> + unsigned int perf = dev_pm_opp_get_required_pstate(opp, i);
> +
> + ret = dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], perf);
> + if (ret) {
> + dev_err(csi2phy->dev, "Couldn't set perf state %u\n",
> + perf);
> + dev_pm_opp_put(opp);
> + goto unset_pstate;
> + }
> + }
> + dev_pm_opp_put(opp);
> +
> + ret = dev_pm_opp_set_rate(dev, opp_rate);
> + if (ret) {
> + dev_err(csi2phy->dev, "dev_pm_opp_set_rate() fail\n");
> + goto unset_opp_rate;
> + }
> +
> + timer_rate = clk_round_rate(csi2phy->timer_clk, link_freq / 4);
> + if (timer_rate <= 0) {
> + ret = -ENODEV;
> + goto unset_opp_rate;
> + }
> +
> + ret = clk_set_rate(csi2phy->timer_clk, timer_rate);
> + if (ret)
> + goto unset_opp_rate;
> +
> + csi2phy->timer_clk_rate = timer_rate;
> +
> + return 0;
> +
> +unset_opp_rate:
> + dev_pm_opp_set_rate(dev, 0);
> +
> +unset_pstate:
> + while (i--)
> + dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
> +
> + return ret;
> +}
> +
> +static int phy_qcom_mipi_csi2_configure(struct phy *phy,
> + union phy_configure_opts *opts)
> +{
> + struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
> + struct phy_configure_opts_mipi_dphy *dphy_cfg = &opts->mipi_dphy;
> + struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
> + int ret;
> +
> + ret = phy_mipi_dphy_config_validate(dphy_cfg);
> + if (ret)
> + return ret;
> +
> + if (dphy_cfg->lanes < 1 || dphy_cfg->lanes > CSI2_MAX_DATA_LANES)
> + return -EINVAL;
> +
> + stream_cfg->link_freq = dphy_cfg->hs_clk_rate;
> +
> + return 0;
> +}
> +
> +static int phy_qcom_mipi_csi2_power_on(struct phy *phy)
> +{
> + struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
> + const struct mipi_csi2phy_hw_ops *ops = csi2phy->soc_cfg->ops;
> + struct device *dev = &phy->dev;
> + int i, ret;
> +
> + ret = regulator_bulk_enable(csi2phy->soc_cfg->num_supplies,
> + csi2phy->supplies);
> + if (ret)
> + return ret;
> +
> + ret = pm_runtime_resume_and_get(csi2phy->dev);
> + if (ret < 0)
> + goto disable_regulators;
> +
> + ret = phy_qcom_mipi_csi2_set_clock_rates(csi2phy, csi2phy->stream_cfg.link_freq);
> + if (ret)
> + goto poweroff_phy;
> +
> + ret = clk_bulk_prepare_enable(csi2phy->soc_cfg->num_clk,
> + csi2phy->clks);
> + if (ret) {
> + dev_err(dev, "failed to enable clocks, %d\n", ret);
> + goto unset_rate;
> + }
> +
> + ops->reset(csi2phy);
> +
> + ops->hw_version_read(csi2phy);
> +
> + return ops->lanes_enable(csi2phy, &csi2phy->stream_cfg);
> +
> +unset_rate:
> + for (i = 0; i < csi2phy->pd_list->num_pds; i++)
> + dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
> +
> + dev_pm_opp_set_rate(csi2phy->dev, 0);
> +
> +poweroff_phy:
> + pm_runtime_put_sync(csi2phy->dev);
> +
> +disable_regulators:
> + regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
> + csi2phy->supplies);
> +
> + return ret;
> +}
> +
> +static int phy_qcom_mipi_csi2_power_off(struct phy *phy)
> +{
> + struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
> + const struct mipi_csi2phy_hw_ops *ops = csi2phy->soc_cfg->ops;
> + int i;
> +
> + ops->lanes_disable(csi2phy, &csi2phy->stream_cfg);
> +
> + clk_bulk_disable_unprepare(csi2phy->soc_cfg->num_clk,
> + csi2phy->clks);
> +
> + for (i = 0; i < csi2phy->pd_list->num_pds; i++)
> + dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
> +
> + dev_pm_opp_set_rate(csi2phy->dev, 0);
> +
> + pm_runtime_put_sync(csi2phy->dev);
> +
> + regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
> + csi2phy->supplies);
> +
> + return 0;
> +}
> +
> +static const struct phy_ops phy_qcom_mipi_csi2_ops = {
> + .configure = phy_qcom_mipi_csi2_configure,
> + .power_on = phy_qcom_mipi_csi2_power_on,
> + .power_off = phy_qcom_mipi_csi2_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static struct phy *qcom_csi2_phy_xlate(struct device *dev,
> + const struct of_phandle_args *args)
> +{
> + struct mipi_csi2phy_device *csi2phy = dev_get_drvdata(dev);
> +
> + if (args->args[0] != PHY_TYPE_DPHY) {
> + dev_err(csi2phy->dev, "mode %d -EOPNOTSUPP\n", args->args[0]);
> + return ERR_PTR(-EOPNOTSUPP);
> + }
> +
> + csi2phy->phy_mode = args->args[0];
> +
> + return csi2phy->phy;
> +}
> +
> +static int phy_qcom_mipi_csi2_attach_pm_domains(struct mipi_csi2phy_device *csi2phy)
> +{
> + const struct dev_pm_domain_attach_data pd_data = {
> + .pd_names = csi2phy->soc_cfg->genpd_names,
> + .num_pd_names = csi2phy->soc_cfg->num_genpd_names,
> + };
> +
> + return devm_pm_domain_attach_list(csi2phy->dev, &pd_data, &csi2phy->pd_list);
> +}
> +
> +static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
> +{
> + struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
> + u32 lane_polarities[CSI2_MAX_DATA_LANES + 1];
> + u32 data_lanes[CSI2_MAX_DATA_LANES];
> + struct device *dev = csi2phy->dev;
> + struct fwnode_handle *ep;
> + int num_polarities;
> + int num_data_lanes;
> + u32 clock_lane;
> + int i, ret;
> +
> + ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 1, 0,
> + FWNODE_GRAPH_ENDPOINT_NEXT);
> + if (ep) {
> + fwnode_handle_put(ep);
> + dev_err(dev, "DPHY split mode is not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0, 0);
> + if (!ep) {
> + dev_err(dev, "Missing port@0\n");
> + return -ENODEV;
> + }
> +
> + num_data_lanes = fwnode_property_count_u32(ep, "data-lanes");
> + if (num_data_lanes < 1 || num_data_lanes > CSI2_MAX_DATA_LANES) {
> + ret = -EINVAL;
> + dev_err(dev, "Invalid data-lanes count: %d\n", num_data_lanes);
> + goto out_put;
> + }
> + stream_cfg->num_data_lanes = num_data_lanes;
> +
> + ret = fwnode_property_read_u32_array(ep, "data-lanes", data_lanes,
> + stream_cfg->num_data_lanes);
> + if (ret) {
> + dev_err(dev, "Failed to read data-lanes: %d\n", ret);
> + goto out_put;
> + }
> +
> + ret = fwnode_property_read_u32(ep, "clock-lanes", &clock_lane);
> + if (ret) {
> + clock_lane = CSI2_DEFAULT_CLK_LN;
> + dev_info(dev, "Using default clock-lane %d\n",
> + CSI2_DEFAULT_CLK_LN);
> + }
> +
> + /* lane-polarities: optional, up to num_data_lanes + 1 entries */
> + memset(lane_polarities, 0x00, sizeof(lane_polarities));
> + num_polarities = fwnode_property_count_u32(ep, "lane-polarities");
> + if (num_polarities > 0) {
> + if (num_polarities != stream_cfg->num_data_lanes + 1) {
> + ret = -EINVAL;
> + dev_err(dev, "clock+data-lane %d/polarities %d mismatch\n",
> + stream_cfg->num_data_lanes + 1, num_polarities);
> + goto out_put;
> + }
> +
> + ret = fwnode_property_read_u32_array(ep, "lane-polarities", lane_polarities,
> + num_polarities);
> + if (ret) {
> + dev_err(dev, "Failed to read lane-polarities: %d\n", ret);
> + goto out_put;
> + }
> + }
> +
> + for (i = 0; i < csi2phy->stream_cfg.num_data_lanes; i++) {
> + csi2phy->stream_cfg.lane_cfg.data[i].pos = data_lanes[i];
> + csi2phy->stream_cfg.lane_cfg.data[i].pol = lane_polarities[i + 1];
> + }
> + csi2phy->stream_cfg.lane_cfg.clk.pos = clock_lane;
> + csi2phy->stream_cfg.lane_cfg.clk.pol = lane_polarities[0];
> +
> + ret = 0;
> +
> +out_put:
> + fwnode_handle_put(ep);
> +
> + return ret;
> +}
> +
> +static int phy_qcom_mipi_csi2_probe(struct platform_device *pdev)
> +{
> + unsigned int i, num_clk, num_supplies;
> + struct mipi_csi2phy_device *csi2phy;
> + struct phy_provider *phy_provider;
> + struct device *dev = &pdev->dev;
> + struct phy *generic_phy;
> + int ret;
> +
> + csi2phy = devm_kzalloc(dev, sizeof(*csi2phy), GFP_KERNEL);
> + if (!csi2phy)
> + return -ENOMEM;
> +
> + csi2phy->dev = dev;
> + dev_set_drvdata(dev, csi2phy);
> +
> + csi2phy->soc_cfg = device_get_match_data(&pdev->dev);
> +
> + if (!csi2phy->soc_cfg)
> + return -EINVAL;
> +
> + num_clk = csi2phy->soc_cfg->num_clk;
> + csi2phy->clks = devm_kzalloc(dev, sizeof(*csi2phy->clks) * num_clk, GFP_KERNEL);
> + if (!csi2phy->clks)
> + return -ENOMEM;
> +
> + ret = phy_qcom_mipi_csi2_parse_routing(csi2phy);
> + if (ret)
> + return ret;
> +
> + ret = phy_qcom_mipi_csi2_attach_pm_domains(csi2phy);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Failed to attach power-domain list\n");
> +
> + devm_pm_runtime_enable(dev);
> +
> + for (i = 0; i < num_clk; i++)
> + csi2phy->clks[i].id = csi2phy->soc_cfg->clk_names[i];
> +
> + ret = devm_clk_bulk_get(dev, num_clk, csi2phy->clks);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to get clocks\n");
> +
> + csi2phy->timer_clk = devm_clk_get(dev, csi2phy->soc_cfg->timer_clk);
> + if (IS_ERR(csi2phy->timer_clk)) {
> + return dev_err_probe(dev, PTR_ERR(csi2phy->timer_clk),
> + "Failed to get timer clock\n");
> + }
> +
> + ret = devm_pm_opp_set_clkname(dev, csi2phy->soc_cfg->opp_clk);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to set opp clkname\n");
> +
> + ret = devm_pm_opp_of_add_table(dev);
> + if (ret && ret != -ENODEV)
> + return dev_err_probe(dev, ret, "invalid OPP table in device tree\n");
> +
> + num_supplies = csi2phy->soc_cfg->num_supplies;
> + csi2phy->supplies = devm_kzalloc(dev, sizeof(*csi2phy->supplies) * num_supplies,
> + GFP_KERNEL);
> + if (!csi2phy->supplies)
> + return -ENOMEM;
> +
> + for (i = 0; i < num_supplies; i++)
> + csi2phy->supplies[i].supply = csi2phy->soc_cfg->supply_names[i];
> +
> + ret = devm_regulator_bulk_get(dev, num_supplies, csi2phy->supplies);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to get regulator supplies\n");
> +
> + csi2phy->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(csi2phy->base))
> + return PTR_ERR(csi2phy->base);
> +
> + generic_phy = devm_phy_create(dev, NULL, &phy_qcom_mipi_csi2_ops);
> + if (IS_ERR(generic_phy)) {
> + ret = PTR_ERR(generic_phy);
> + return dev_err_probe(dev, ret, "failed to create phy\n");
> + }
> + csi2phy->phy = generic_phy;
> +
> + phy_set_drvdata(generic_phy, csi2phy);
> +
> + phy_provider = devm_of_phy_provider_register(dev, qcom_csi2_phy_xlate);
> + if (!IS_ERR(phy_provider))
> + dev_dbg(dev, "Registered MIPI CSI2 PHY device\n");
> +
> + return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct of_device_id phy_qcom_mipi_csi2_of_match_table[] = {
> + { .compatible = "qcom,x1e80100-csi2-phy", .data = &mipi_csi2_dphy_4nm_x1e },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, phy_qcom_mipi_csi2_of_match_table);
> +
> +static struct platform_driver phy_qcom_mipi_csi2_driver = {
> + .probe = phy_qcom_mipi_csi2_probe,
> + .driver = {
> + .name = "qcom-mipi-csi2-phy",
> + .of_match_table = phy_qcom_mipi_csi2_of_match_table,
> + },
> +};
> +
> +module_platform_driver(phy_qcom_mipi_csi2_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm MIPI CSI2 PHY driver");
> +MODULE_AUTHOR("Bryan O'Donoghue <bryan.odonoghue@linaro.org>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
> new file mode 100644
> index 0000000000000..e7c1ce00916e3
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
> @@ -0,0 +1,95 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + *
> + * Qualcomm MIPI CSI2 CPHY/DPHY driver
> + *
> + * Copyright (C) 2025 Linaro Ltd.
> + */
> +#ifndef __PHY_QCOM_MIPI_CSI2_H__
> +#define __PHY_QCOM_MIPI_CSI2_H__
> +
> +#include <linux/phy/phy.h>
> +
> +#define CSI2_MAX_DATA_LANES 4
> +#define CSI2_DEFAULT_CLK_LN 7
> +
> +struct mipi_csi2phy_lane {
> + u8 pos;
> + u8 pol;
> +};
> +
> +struct mipi_csi2phy_lanes_cfg {
> + struct mipi_csi2phy_lane data[CSI2_MAX_DATA_LANES];
> + struct mipi_csi2phy_lane clk;
> +};
> +
> +struct mipi_csi2phy_stream_cfg {
> + s64 link_freq;
> + u8 num_data_lanes;
> + struct mipi_csi2phy_lanes_cfg lane_cfg;
> +};
> +
> +struct mipi_csi2phy_device;
> +
> +struct mipi_csi2phy_hw_ops {
> + void (*hw_version_read)(struct mipi_csi2phy_device *csi2phy_dev);
> + void (*reset)(struct mipi_csi2phy_device *csi2phy_dev);
> + int (*lanes_enable)(struct mipi_csi2phy_device *csi2phy_dev,
> + struct mipi_csi2phy_stream_cfg *cfg);
> + void (*lanes_disable)(struct mipi_csi2phy_device *csi2phy_dev,
> + struct mipi_csi2phy_stream_cfg *cfg);
> +};
> +
> +struct mipi_csi2phy_lane_regs {
> + const s32 reg_addr;
> + const s32 reg_data;
> + const u32 delay_us;
> + const u32 param_type;
> +};
> +
> +struct mipi_csi2phy_device_regs {
> + const struct mipi_csi2phy_lane_regs *init_seq;
> + const int lane_array_size;
> + const u32 common_regs_offset;
> +};
> +
> +struct mipi_csi2phy_soc_cfg {
> + const struct mipi_csi2phy_hw_ops *ops;
> + const struct mipi_csi2phy_device_regs reg_info;
> +
> + const char ** const supply_names;
> + const unsigned int num_supplies;
> +
> + const char ** const clk_names;
> + const unsigned int num_clk;
> +
> + const char * const opp_clk;
> + const char * const timer_clk;
> +
> + const char ** const genpd_names;
> + const unsigned int num_genpd_names;
> +};
> +
> +struct mipi_csi2phy_device {
> + struct device *dev;
> + u8 phy_mode;
> +
> + struct phy *phy;
> + void __iomem *base;
> +
> + struct clk_bulk_data *clks;
> + struct clk *timer_clk;
> + u32 timer_clk_rate;
> +
> + struct regulator_bulk_data *supplies;
> + struct dev_pm_domain_list *pd_list;
> +
> + const struct mipi_csi2phy_soc_cfg *soc_cfg;
> + struct mipi_csi2phy_stream_cfg stream_cfg;
> +
> + u32 hw_version;
> +};
> +
> +extern const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e;
> +
> +#endif /* __PHY_QCOM_MIPI_CSI2_H__ */
>
Thanks,
Vijay.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Dmitry Baryshkov @ 2026-06-03 22:14 UTC (permalink / raw)
To: Vijay Kumar Tumati
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Bryan O'Donoghue, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <b6a8c860-8b47-4250-a428-5225d0e64a1c@oss.qualcomm.com>
On Wed, Jun 03, 2026 at 02:37:48PM -0700, Vijay Kumar Tumati wrote:
>
>
> On 6/3/2026 5:57 AM, Bryan O'Donoghue wrote:
> > On 03/06/2026 13:40, Dmitry Baryshkov wrote:
> > > > Are you sure about that ?
> > > Yes.
> > >
> > > > ipcat I thought designated lane 7 specifically as clk-lane i.e. named it
> > > > CLK_LN of some description.
> > > Split configurations explicitly use other lanes for clocks. E.g. check
> > > the RB5 Navigation schematics, CAM0B connector.
> >
> > Can you please check:
> >
> > CSI_3PHASE_COMMON.CSI_COMMON_CTRL5
> >
> > 0 LN0_PWRDN_B Lane 0
> > ...
> > 7 LNCK_PWRDN_B Clock Lane
> >
> > ... just a badly name field
> >
> > CSI_2PHASE_CTRL10
> >
> > Bit[2] = IS_CLKLANE
> >
> > Right so CSI_2PHASE_CTRL10 controls lane mode, indeed. Thanks for checking.
> I can check this with the HW team. Although the SWI has this knob, there may
> be some limitations to use any lane as the clk lane. AFAIK, only two
> specific lanes are clk capable in DPHY mode.>
Having clock-names property doesn't mean that all values are valid.
--
With best wishes
Dmitry
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vijay Kumar Tumati @ 2026-06-03 21:46 UTC (permalink / raw)
To: Bryan O'Donoghue, Vladimir Zapolskiy, Bryan O'Donoghue,
Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <29e8491f-20e8-4082-8943-66bee7e3af1d@kernel.org>
On 6/3/2026 2:18 PM, Bryan O'Donoghue wrote:
> On 03/06/2026 21:51, Vladimir Zapolskiy wrote:
>>> Actually, one more thing, Why isn't TITAN TOP GDSC here?>>>> +
>> If CSIPHYs are true subdevices under the umbrella CAMSS device and well
>> described as subnodes, then likely none of power domains are needed to be
>> repeatedly described in the children device nodes, since this information
>> can be obtained from the parent device by the driver.
>>
>> Technically 'power-domains' property can be safely removed, I believe.
>
> The policy is to describe the power-domain dependency fully since DT
> describes hardware not software architecture.
>
> Also for a very practical reason a sub-devices can probe/run
> asynchronously of the parent device being active so in fact we do need
> to describe the PDs fully.
This makes me think. When the PHY devices are made the sub nodes of the
top camss node eventually, we can discuss for somethings to be avoided
in each child, which we can come to later but for now, you need to add
the top AHB clock as well in the PHY node, Bryan.>
> ---
> bod
Thanks,
Vijay.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Vijay Kumar Tumati @ 2026-06-03 21:37 UTC (permalink / raw)
To: Bryan O'Donoghue, Dmitry Baryshkov
Cc: Vladimir Zapolskiy, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
In-Reply-To: <8eb709ce-3469-4303-9c3d-8d1a221cb8de@linaro.org>
On 6/3/2026 5:57 AM, Bryan O'Donoghue wrote:
> On 03/06/2026 13:40, Dmitry Baryshkov wrote:
>>> Are you sure about that ?
>> Yes.
>>
>>> ipcat I thought designated lane 7 specifically as clk-lane i.e. named it
>>> CLK_LN of some description.
>> Split configurations explicitly use other lanes for clocks. E.g. check
>> the RB5 Navigation schematics, CAM0B connector.
>
> Can you please check:
>
> CSI_3PHASE_COMMON.CSI_COMMON_CTRL5
>
> 0 LN0_PWRDN_B Lane 0
> ...
> 7 LNCK_PWRDN_B Clock Lane
>
> ... just a badly name field
>
> CSI_2PHASE_CTRL10
>
> Bit[2] = IS_CLKLANE
>
> Right so CSI_2PHASE_CTRL10 controls lane mode, indeed. Thanks for checking.
I can check this with the HW team. Although the SWI has this knob, there
may be some limitations to use any lane as the clk lane. AFAIK, only two
specific lanes are clk capable in DPHY mode.>
> ---
> bod
>
Thanks,
Vijay.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vijay Kumar Tumati @ 2026-06-03 21:35 UTC (permalink / raw)
To: Bryan O'Donoghue, Vladimir Zapolskiy, Bryan O'Donoghue,
Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <dbc4b9b9-5c7a-4426-85bb-e99ad6be6ac6@kernel.org>
On 6/3/2026 1:52 PM, Bryan O'Donoghue wrote:
> On 03/06/2026 21:24, Vijay Kumar Tumati wrote:
>>
>>
>> On 6/3/2026 1:16 PM, Vijay Kumar Tumati wrote:
>>> Hi,
>>>
>>> On 6/2/2026 3:51 PM, Bryan O'Donoghue wrote:
>>>> On 02/06/2026 22:59, Vladimir Zapolskiy wrote:
>>>>> On 5/23/26 05:48, Bryan O'Donoghue wrote:
>>>>>> Add a base schema initially compatible with x1e80100 to describe
>>>>>> MIPI CSI2
>>>>>> PHY devices.
>>>>>>
>>>>>> The hardware can support both CPHY, DPHY and a special split-mode
>>>>>> DPHY.
>>>>>>
>>>>>> The schema here defines three ports:
>>>>>>
>>>>>> port@0:
>>>>>> The first input port where a sensor is always required.
>>>>>>
>>>>>> port@1:
>>>>>> A second optional input port which if present implies DPHY
>>>>>> split-mode.
>>>>>>
>>>>>> port@2:
>>>>>> A third always required output port which connects to the
>>>>>> controller.
>>>>>>
>>>>>
>>>>> This port numeration is imperfect, because port@0 and port@2 are
>>>>> required,
>>>>> while middle port@1 is optional.
>>>>>
>>>>> Like it was stated before a number of times, it seems natural to
>>>>> operate
>>>>> with two ports, where input port may have two endpoints rather than 3
>>>>> ports,
>>>>> also that approach solves the problem of a hole in the port
>>>>> numeration.
>>>>
>>>> Can you confirm this is what you are after ?
>>>>
>>>> port@0 {
>>>> #address-cells = <1>;
>>>> #size-cells = <0>;
>>>>
>>>> endpoint@0 { /* primary sensor */
>>>> reg = <0>;
>>>> data-lanes = <0 1 2 3>;
>>>> remote-endpoint = <&sensor0_out>;
>>>> };
>>>>
>>>> endpoint@1 { /* split-mode second sensor,
>>>> optional */
>>>> reg = <1>;
>>>> data-lanes = <0>;
>>>> remote-endpoint = <&sensor1_out>;
>>>> };
>>>> };
>>>>
>>>> port@1 { /* output to CAMSS, was port@2 */
>>>> endpoint { remote-endpoint = <&controller_in>; };
>>>> };
>>>>
>>>> This works for me BTW.
>>> Either way, do we need to document the constraint of using port@0 or
>>> endpoint@0 'only' for the 4+1 or 2+1 mode and the other one is for the
>>> 1+1 mode? Or is it implicit from this bindings for a developer?
>
> The binding mandates it with an if / else structure
>
>>
>>>>>> The CSIPHY devices have their own pinouts on the SoC as well as
>>>>>> their own
>>>>>> individual voltage rails.
>>>>>>
>>>>>> The need to model voltage rails on a per-PHY basis leads us to define
>>>>>> CSIPHY devices as individual nodes.
>>>>>>
>>>>>> Two nice outcomes in terms of schema and DT arise from this change.
>>>>>>
>>>>>> 1. The ability to define on a per-PHY basis voltage rails.
>>>>>> 2. The ability to require those voltage.
>>>>>>
>>>>>> We have had a complete bodge upstream for this where a single set of
>>>>>> voltage rail for all CSIPHYs has been buried inside of CAMSS.
>>>>>>
>>>>>> Much like the I2C bus which is dedicated to Camera sensors - the CCI
>>>>>> bus in
>>>>>> CAMSS parlance, the CSIPHY devices should be individually modelled.
>>>>>>
>>>>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>>>> ---
>>>>>> .../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 209 ++++++++
>>>>>> + ++++++++++++
>>>>>> 1 file changed, 209 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-
>>>>>> csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-
>>>>>> csi2-phy.yaml
>>>>>> new file mode 100644
>>>>>> index 0000000000000..270375f949880
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-
>>>>>> phy.yaml
>>>>>> @@ -0,0 +1,209 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Qualcomm CSI2 PHY
>>>>>> +
>>>>>> +maintainers:
>>>>>> + - Bryan O'Donoghue <bod@kernel.org>
>>>>>> +
>>>>>> +description:
>>>>>> + Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI
>>>>>> CSI2 sensors
>>>>>> + to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and
>>>>>> D-PHY
>>>>>> + modes.
>>>>>> +
>>>>>> +properties:
>>>>>> + compatible:
>>>>>> + const: qcom,x1e80100-csi2-phy
>>>>>> +
>>>>>> + reg:
>>>>>> + maxItems: 1
>>>>>> +
>>>>>> + "#phy-cells":
>>>>>> + const: 1
>>>>>> + description:
>>>>>> + The single cell specifies the PHY operating mode.
>>>>>
>>>>> #phy-cells should be 0, because the PHY operating mode is well defined
>>>>> by 'bus-type' property of an endpoint on the sensor side, the opposite
>>>>> side of CAMSS/CSID as a CSIPHY "consumer" should not dictate the PHY
>>>>> type.
>>>>
>>>> Rob said consumer but, I'm also not very bothered about that. bus-type
>>>> is perfectly acceptable to me.
>>>>
>>>>>> +
>>>>>> + clocks:
>>>>>> + maxItems: 2
>>>>>> +
>>>>>> + clock-names:
>>>>>> + items:
>>>>>> + - const: core
>>>>>> + - const: timer
>>>>>> +
>>>>>> + interrupts:
>>>>>> + maxItems: 1
>>>>>> +
>>>>>> + operating-points-v2:
>>>>>> + maxItems: 1
>>>>>> +
>>>>>> + power-domains:
>>>>>> + items:
>>>>>> + - description: MMCX voltage rail
>>>>>> + - description: MXC or MXA voltage rail
>>>>>
>>>>> Only "qcom,x1e80100-csi2-phy" device is supported so far, unlikely
>>>>> it's
>>>>> the case that "MXC or MXA voltage rail" should be specified, it'd be
>>>>> just one of two or both.
>>>>
>>>> Hmm. I'm not being clear here if this is your take, I will reword it
>>>> to make it clearer this generation of PHY _must_ have either
>>>>
>>>> - MMCX and MXC
>>>> or
>>>> - MMCX and MXA
>>> I am not sure of this, Bryan. If you look at the PHY core clock
>>> separately, sure, that is correct. But all of them, on this platform as
>>> well, share the RCG, which requires all 3 power domains. So
>>> fundamentally, you need to enable all of those from each PHY. You can
>>> make it constant 3 power domains.>
>
> Hmm do you mean the GDSC which I omitted form the example and shouldn't
> have TITAN_TOP_GDSC or do you mean MMCX, MXC and MXA are required ?
>
> I don't believe the clock definitions say that. Also not what you said
> in the previous cycle.
>
> I'd be obliged if you could be precise and clear since as you know the
> PHY as a separate thing is important to release new SoC additions.
I said the same thing in the past as well, Bryan.
https://lore.kernel.org/all/5705b48a-fc24-4c5f-aa6d-40952f0070d9@oss.qualcomm.com/
I would expect 3 voltage rails for each PHY and a TOP GDSC, so a total
of 4 power domains. Perhaps, you don't have the access to IPCAT but I
can confirm that, on X1E80100 platform, the parent of cam_cc_csiphy0_clk
(and other PHYs) is cam_cc_cphy_rx_clk_src and that depends on mxc, mxa
and mm for core voltage. And cam_cc_csiphy0_clk (and other PHYs) itself
is controlled by the GDSC TITAN_TOP.>>>>>> +
>>>>>> + power-domain-names:
>>>>>> + items:
>>>>>> + - const: mmcx
>>>>>> + - const: mx
>>>>>> +
>>>>>> + vdda-0p9-supply:
>>>>>> + description: Phandle to a 0.9V regulator supply to a PHY.
>>>>>> +
>>>>>> + vdda-1p2-supply:
>>>>>> + description: Phandle to 1.2V regulator supply to a PHY.
>>>>>> +
>>>>>> + ports:
>>>>>> + $ref: /schemas/graph.yaml#/properties/ports
>>>>>> +
>>>>>> + properties:
>>>>>> + port@0:
>>>>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>>>>> + description: Sensor input. Always present.
>>>>>> + unevaluatedProperties: false
>>>>>> +
>>>>>> + properties:
>>>>>> + endpoint:
>>>>>> + $ref: /schemas/media/video-interfaces.yaml#
>>>>>> + unevaluatedProperties: false
>>>>>> + properties:
>>>>>> + data-lanes:
>>>>>> + minItems: 1
>>>>>> + maxItems: 4
>>>>>> + clock-lanes:
>>>>>> + maxItems: 1
>>>>>> + remote-endpoint: true
>>>>>> + required:
>>>>>> + - data-lanes
>>>>>> + - remote-endpoint
>>>>>> +
>>>>>> + port@1:
>>>>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>>>>> + description:
>>>>>> + Second sensor input. When present, indicates DPHY split
>>>>>> mode.
>>>>>> + unevaluatedProperties: false
>>>>>> +
>>>>>> + properties:
>>>>>> + endpoint:
>>>>>> + $ref: /schemas/media/video-interfaces.yaml#
>>>>>> + unevaluatedProperties: false
>>>>>> + properties:
>>>>>> + data-lanes:
>>>>>> + maxItems: 1
>>>>>> + clock-lanes:
>>>>>> + maxItems: 1
>>>>>> + remote-endpoint: true
>>>>>> + required:
>>>>>> + - data-lanes
>>>>>> + - clock-lanes
>>>>>> + - remote-endpoint
>>>>>
>>>>> As it's stated above, it should be converted to a single port with two
>>>>> endpoints, it'd be done in accordance to video-interfaces.yaml.
>>>>>
>>>>>> +
>>>>>> + port@2:
>>>>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>>>>> + description: Output to CAMSS controller.
>>>>>> + unevaluatedProperties: false
>>>>>> +
>>>>>> + properties:
>>>>>> + endpoint:
>>>>>> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
>>>>>> + unevaluatedProperties: false
>>>>>> + properties:
>>>>>> + remote-endpoint: true
>>>>>> + required:
>>>>>> + - remote-endpoint
>>>>>> +
>>>>>> + required:
>>>>>> + - port@0
>>>>>> + - port@2
>>>>>> +
>>>>>> +required:
>>>>>> + - compatible
>>>>>> + - reg
>>>>>> + - "#phy-cells"
>>>>>> + - clocks
>>>>>> + - clock-names
>>>>>> + - interrupts
>>>>>> + - operating-points-v2
>>>>>> + - power-domains
>>>>>> + - power-domain-names
>>>>>> + - vdda-0p9-supply
>>>>>> + - vdda-1p2-supply
>>>>>> + - ports
>>>>>> +
>>>>>> +additionalProperties: false
>>>>>> +
>>>>>> +examples:
>>>>>> + - |
>>>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>>> + #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
>>>>>> + #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
>>>>>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>>>>>> +
>>>>>> + csiphy4: csiphy@ace4000 {
>>>>>> + compatible = "qcom,x1e80100-csi2-phy";
>>>>>> + reg = <0x0ace4000 0x2000>;
>>>>>> + #phy-cells = <1>;
>>>>>> +
>>>>>> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>>>>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
>>>>>> + clock-names = "core",
>>>>>> + "timer";
>>>>>> +
>>>>>> + operating-points-v2 = <&csiphy_opp_table>;
>>>>>> +
>>>>>> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>>>>>> +
>>>>>> + power-domains = <&rpmhpd RPMHPD_MMCX>,
>>>>>> + <&rpmhpd RPMHPD_MX>;
>>>>>> + power-domain-names = "mmcx",
>>>>>> + "mx";
>> Actually, one more thing, Why isn't TITAN TOP GDSC here?>>>> +
>
> Yes the DTSI has TITAN_TOP_GDSC I haven't updated the YAML to capture that.
>
> So it should be
>
> top
> mmcx
> mx
Ack.>
> With obviously on mmcx and mx scalable. We established that CSIPHY4 had
> MXA whereas the other CSIPHYs had MXC in v5 or v4 - can you be clear if
> you agreeing with that still or saying something different. Per my
> memory of reading the docs, there was nothing in the clock tree to
> indicate both MXA and MXC were required for all PHYs.
>
>>>>>> + vdda-0p9-supply = <&vreg_l2c_0p8>;
>>>>>> + vdda-1p2-supply = <&vreg_l1c_1p2>;
>>>>>> +
>>>>>> + ports {
>>>>>> + #address-cells = <1>;
>>>>>> + #size-cells = <0>;
>>>>>> +
>>>>>> + port@0 {
>>>>>> + reg = <0>;
>>>>>> + csiphy0_in_ep: endpoint {
>>>>>> + data-lanes = <0 1>;
>>>>>> + clock-lanes = <2>;
>>>>>> + remote-endpoint = <&sensor_out>;
>>>>>> + };
>>>>>> + };
>>>>>> +
>>>>>> + port@2 {
>>>>>> + reg = <2>;
>>>>>> + csiphy0_out_ep: endpoint {
>>>>>> + remote-endpoint = <&controller_in>;
>>>>>> + };
>>>>>> + };
>>>>>> + };
>>>>>> + };
>>>>>> +
>>>>>> + csiphy_opp_table: opp-table {
>>>>>> + compatible = "operating-points-v2";
>>>>>> +
>>>>>> + opp-300000000 {
>>>>>> + opp-hz = /bits/ 64 <300000000>;
>>> I wonder why you would have only one clock here. You should be setting
>>> the rate for both the core and timer, isn't it?
>
> Yes the dtsi has it, the example does not. I had pushback from others
> about the example being too complex - you can't please all of the people
> all of the time.
>
> I will drop the full table @ v9
Ok. I noticed that you are setting the rate for the timer clk explicitly
using 'clk_set_rate'. May be the best way is to control the rate of both
the clocks with the OPP table itself, with core clk as your anchor that
is calculated based on the data rate.>
>>>> + required-opps =
>>> <&rpmhpd_opp_low_svs_d1>,
>>>>>> + <&rpmhpd_opp_low_svs_d1>;
>>> Same here, it should 3 power domains set.>>> + };
>
> Two power domains scaled, one set at least I hope that's what you mean
> i.e. add the GDSC, already in my code I just didn't add it here as I
> should have.
>
> - GDSC enabled
> - MMCX scaled
> - MX scaled
>
> When MX points to MXA the scaling is a NOP @ rpmhpd_opp_low_svs_d1.
>
> Agreed ?
Like I clarified in the first comment, I would expect all of mcx, mxa
and mm to be set by OPP table.>>>>>> +
>>>>>> + opp-400000000 {
>>>>>> + opp-hz = /bits/ 64 <400000000>;
>>>>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>>>>> + <&rpmhpd_opp_low_svs_d1>;
>>> Why is one at svs and the other at svs_d1? Shouldn't both be svs?>>>
>>> + };
>>>>>> +
>>>>>> + opp-480000000 {
>>>>>> + opp-hz = /bits/ 64 <480000000>;
>>>>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>>>>> + <&rpmhpd_opp_low_svs_d1>;
>>> And here, both should be svs_l1?>>> + };
>>>>>> + };
>>>>>>
>>>>>
>>>>> --
>>>>> Best wishes,
>>>>> Vladimir
>>>>
>>>>
>>> Thanks,
>>> Vijay.
>>>
>>
>
Thanks,
Vijay.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-06-03 21:18 UTC (permalink / raw)
To: Vladimir Zapolskiy, Vijay Kumar Tumati, Bryan O'Donoghue,
Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <f01c0e22-4e5c-44e7-9ea4-4bc8d53aea2e@linaro.org>
On 03/06/2026 21:51, Vladimir Zapolskiy wrote:
>> Actually, one more thing, Why isn't TITAN TOP GDSC here?>>>> +
> If CSIPHYs are true subdevices under the umbrella CAMSS device and well
> described as subnodes, then likely none of power domains are needed to be
> repeatedly described in the children device nodes, since this information
> can be obtained from the parent device by the driver.
>
> Technically 'power-domains' property can be safely removed, I believe.
The policy is to describe the power-domain dependency fully since DT
describes hardware not software architecture.
Also for a very practical reason a sub-devices can probe/run
asynchronously of the parent device being active so in fact we do need
to describe the PDs fully.
---
bod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Bryan O'Donoghue @ 2026-06-03 21:12 UTC (permalink / raw)
To: Vladimir Zapolskiy, Bryan O'Donoghue, Dmitry Baryshkov
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <6e962010-8097-4b52-9369-0727c0d5a31a@linaro.org>
On 03/06/2026 21:42, Vladimir Zapolskiy wrote:
>>> Split configurations explicitly use other lanes for clocks. E.g. check
>>> the RB5 Navigation schematics, CAM0B connector.
>> Can you please check:
>>
>> CSI_3PHASE_COMMON.CSI_COMMON_CTRL5
>>
>> 0 LN0_PWRDN_B Lane 0
>> ...
>> 7 LNCK_PWRDN_B Clock Lane
> Please note that media devices have a numeration scheme of lanes starting
> from 1 (it'd be easy to check/confirm it), for instance today CAMSS has
> lane numeration starting from 0 is out of the accepted scheme, and here
> it'd be better to correct it and not enter the same pit.
Yes fair point CAMSS has done this wrong since forever. data-lanes = <1
2 3 4> => LN0, LN1, LN2, LN3>
> I don't have access to the IP spec, anyway I do not grasp it, where are
> 8 lanes on the CSIPHY found? Each CSIPHY IP has 4+1 D-PHY lanes, not 8.
Max CSI2 data-lanes is 8
#define V4L2_MBUS_CSI2_MAX_DATA_LANES 8
That doesn't really explain why this register has seven data-lanes and
one-clock lane.
It just is what it is.
---
bod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Vijay Kumar Tumati @ 2026-06-03 21:11 UTC (permalink / raw)
To: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <20260523-x1e-csi2-phy-v8-2-a85668459521@linaro.org>
On 5/22/2026 7:48 PM, Bryan O'Donoghue wrote:
> Add a new MIPI CSI2 driver in DPHY mode initially. The entire set of
> existing CAMSS CSI PHY init sequences are imported in order to save time
> and effort in later patches.
>
> The following devices are supported in this drop:
> "qcom,x1e80100-csi2-phy"
>
> In-line with other PHY drivers the process node is included in the name.
> Data-lane and clock lane positioning and polarity selection via newly
> amended struct phy_configure_opts_mipi_dphy{} is supported.
>
> The Qualcomm 3PH class of PHYs can do both DPHY and CPHY mode. For now only
> DPHY is supported.
>
> In porting some of the logic over from camss-csiphy*.c to here its also
> possible to rationalise some of the code.
>
> In particular use of regulator_bulk and clk_bulk as well as dropping the
> seemingly useless and unused interrupt handler.
>
> The PHY sequences and a lot of the logic that goes with them are well
> proven in CAMSS and mature so the main thing to watch out for here is how
> to get the right sequencing of regulators, clocks and register-writes.
>
> The register init sequence table is imported verbatim from the existing
> CAMSS csiphy driver. A follow-up series will rework the table to extract
> the repetitive per-lane pattern into a loop.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> MAINTAINERS | 10 +
> drivers/phy/qualcomm/Kconfig | 14 +
> drivers/phy/qualcomm/Makefile | 5 +
> drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 376 +++++++++++++++++++
> drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c | 402 +++++++++++++++++++++
> drivers/phy/qualcomm/phy-qcom-mipi-csi2.h | 95 +++++
> 6 files changed, 902 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 63389fea5d150..3b5da8a40383f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22018,6 +22018,16 @@ S: Maintained
> F: Documentation/devicetree/bindings/media/qcom,*-iris.yaml
> F: drivers/media/platform/qcom/iris/
>
> +QUALCOMM MIPI CSI2 PHY DRIVER
> +M: Bryan O'Donoghue <bod@kernel.org>
> +L: linux-phy@lists.infradead.org
> +L: linux-media@vger.kernel.org
> +L: linux-arm-msm@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/phy/qcom,*-csi2-phy.yaml
> +F: drivers/phy/qualcomm/phy-qcom-mipi-csi2*.c
> +F: drivers/phy/qualcomm/phy-qcom-mipi-csi2*.h
> +
> QUALCOMM NAND CONTROLLER DRIVER
> M: Manivannan Sadhasivam <mani@kernel.org>
> L: linux-mtd@lists.infradead.org
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index 60a0ead127fa9..779a3511ba852 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -28,6 +28,20 @@ config PHY_QCOM_EDP
> Enable this driver to support the Qualcomm eDP PHY found in various
> Qualcomm chipsets.
>
> +config PHY_QCOM_MIPI_CSI2
> + tristate "Qualcomm MIPI CSI2 PHY driver"
> + depends on ARCH_QCOM || COMPILE_TEST
> + depends on OF
> + depends on PM
> + depends on COMMON_CLK
> + select GENERIC_PHY
> + select GENERIC_PHY_MIPI_DPHY
> + help
> + Enable this to support the MIPI CSI2 PHY driver found in various
> + Qualcomm chipsets. This PHY is used to connect MIPI CSI2
> + camera sensors to the CSI Decoder in the Qualcomm Camera Subsystem
> + CAMSS.
> +
> config PHY_QCOM_IPQ4019_USB
> tristate "Qualcomm IPQ4019 USB PHY driver"
> depends on OF && (ARCH_QCOM || COMPILE_TEST)
> diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
> index b71a6a0bed3f1..382cb594b06b6 100644
> --- a/drivers/phy/qualcomm/Makefile
> +++ b/drivers/phy/qualcomm/Makefile
> @@ -6,6 +6,11 @@ obj-$(CONFIG_PHY_QCOM_IPQ4019_USB) += phy-qcom-ipq4019-usb.o
> obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA) += phy-qcom-ipq806x-sata.o
> obj-$(CONFIG_PHY_QCOM_M31_USB) += phy-qcom-m31.o
> obj-$(CONFIG_PHY_QCOM_M31_EUSB) += phy-qcom-m31-eusb2.o
> +
> +phy-qcom-mipi-csi2-objs += phy-qcom-mipi-csi2-core.o \
> + phy-qcom-mipi-csi2-3ph-dphy.o
> +obj-$(CONFIG_PHY_QCOM_MIPI_CSI2) += phy-qcom-mipi-csi2.o
> +
> obj-$(CONFIG_PHY_QCOM_PCIE2) += phy-qcom-pcie2.o
>
> obj-$(CONFIG_PHY_QCOM_QMP_COMBO) += phy-qcom-qmp-combo.o phy-qcom-qmp-usbc.o
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> new file mode 100644
> index 0000000000000..86ec405820e62
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
This file name has always been confusing. I know it is saying that the
HW supports 3phase (cphy) but the driver is for dphy (2 phase). But is
there a way we can just make it phy-qcom-mipi-csi2-dphy.c so it is clear
for the reader that it is for 2ph only.> @@ -0,0 +1,376 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Qualcomm MSM Camera Subsystem - CSIPHY Module 3phase v1.0
> + *
> + * Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2016-2025 Linaro Ltd.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/time64.h>
> +
> +#include "phy-qcom-mipi-csi2.h"
> +
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(offset, n) ((offset) + 0x4 * (n))
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET BIT(0)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE BIT(7)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B BIT(0)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID BIT(1)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL10_IRQ_CLEAR_CMD BIT(0)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(offset, n) ((offset) + 0xb0 + 0x4 * (n))
> +
> +#define CSIPHY_2PH_LN_CSI_2PHASE_CTRL9n(n) ((0x200 * (n)) + 0x24)
> +
> +/*
> + * 3 phase CSI has 19 common status regs with only 0-10 being used
> + * and 11-18 being reserved.
> + */
> +#define CSI_COMMON_STATUS_NUM 11
> +/*
> + * There are a number of common control registers
> + * The offset to clear the CSIPHY IRQ status starts @ 22
> + * So to clear CSI_COMMON_STATUS0 this is CSI_COMMON_CONTROL22, STATUS1 is
> + * CONTROL23 and so on
> + */
> +#define CSI_CTRL_STATUS_INDEX 22
Does a name like CSI_COMMON_STATUS_CTRL_OFFSET be clearer?> +
> +/*
> + * There are 43 COMMON_CTRL registers with regs after # 33 being reserved
> + */
> +#define CSI_CTRL_MAX 33
> +
> +#define CSIPHY_DEFAULT_PARAMS 0
> +#define CSIPHY_SETTLE_CNT_LOWER_BYTE 2
> +#define CSIPHY_SKEW_CAL 7
> +
> +/* 4nm 2PH v 2.1.2 2p5Gbps 4 lane DPHY mode */
> +static const struct
> +mipi_csi2phy_lane_regs lane_regs_x1e80100[] = {
> + /* Power up lanes 2ph mode */
I think there were some discussions to not duplicate the settings for
each lane. Are we planning to take that up separately?> + {.reg_addr =
0x1014, .reg_data = 0xd5, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x101c, .reg_data = 0x7a, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x1018, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +
> + {.reg_addr = 0x0094, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x00a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0090, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0098, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0094, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0030, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0000, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0038, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x002c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0034, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x001c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0014, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x003c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0004, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0020, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0008, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {.reg_addr = 0x0010, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0094, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x005c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0060, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0064, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> +
> + {.reg_addr = 0x0e94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0ea0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e94, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e28, .reg_data = 0x04, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e00, .reg_data = 0x80, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e0c, .reg_data = 0xff, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e38, .reg_data = 0x1f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0e08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {.reg_addr = 0x0e10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> +
> + {.reg_addr = 0x0494, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x04a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0490, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0498, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0494, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0430, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0400, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0438, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x042c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0434, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x041c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0414, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x043c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0404, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0420, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0408, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {.reg_addr = 0x0410, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0494, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x045c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0460, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0464, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> +
> + {.reg_addr = 0x0894, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x08a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0890, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0898, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0894, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0830, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0800, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0838, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x082c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0834, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x081c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0814, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x083c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0804, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0820, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0808, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {.reg_addr = 0x0810, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0894, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x085c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0860, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0864, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> +
> + {.reg_addr = 0x0c94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0ca0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c94, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c00, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c38, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {.reg_addr = 0x0c10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> + {.reg_addr = 0x0c94, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0c5c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0c60, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> + {.reg_addr = 0x0c64, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> +};
> +
> +static inline const struct mipi_csi2phy_device_regs *
> +csi2phy_dev_to_regs(struct mipi_csi2phy_device *csi2phy)
> +{
> + return &csi2phy->soc_cfg->reg_info;
> +}
> +
> +static void phy_qcom_mipi_csi2_hw_version_read(struct mipi_csi2phy_device *csi2phy)
> +{
> + const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> + u32 tmp;
> +
> + writel(CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
> +
> + tmp = readl_relaxed(csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 12));
> + csi2phy->hw_version = tmp;
> +
> + tmp = readl_relaxed(csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 13));
> + csi2phy->hw_version |= (tmp << 8) & 0xFF00;
> +
> + tmp = readl_relaxed(csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 14));
> + csi2phy->hw_version |= (tmp << 16) & 0xFF0000;
> +
> + tmp = readl_relaxed(csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 15));
> + csi2phy->hw_version |= (tmp << 24) & 0xFF000000;
> +
> + dev_dbg_once(csi2phy->dev, "CSIPHY 3PH HW Version = 0x%08x\n", csi2phy->hw_version);
> +}
> +
> +/*
> + * phy_qcom_mipi_csi2_reset - Perform software reset on CSIPHY module
> + * @phy_qcom_mipi_csi2: CSIPHY device
> + */
> +static void phy_qcom_mipi_csi2_reset(struct mipi_csi2phy_device *csi2phy)
> +{
> + const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> +
> + writel(CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET,
> + csi2phy->base + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
> + usleep_range(5000, 8000);
I know this delay is coming from the existing driver but it may be
necessary as much. Anyway, you can leave it for now I think.> +
writel(0x0, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
> +}
> +
> +/*
> + * phy_qcom_mipi_csi2_settle_cnt_calc - Calculate settle count value
> + *
> + * Helper function to calculate settle count value. This is
> + * based on the CSI2 T_hs_settle parameter which in turn
> + * is calculated based on the CSI2 transmitter link frequency.
> + *
> + * Return settle count value or 0 if the CSI2 link frequency
> + * is not available
> + */
> +static u8 phy_qcom_mipi_csi2_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
> +{
> + u32 t_hs_prepare_max_ps;
> + u32 timer_period_ps;
> + u32 t_hs_settle_ps;
> + u8 settle_cnt;
> + u32 ui_ps;
> +
> + if (link_freq <= 0)
> + return 0;
> +
> + ui_ps = div_u64(PSEC_PER_SEC, link_freq);
> + ui_ps /= 2;
> + t_hs_prepare_max_ps = 85000 + 6 * ui_ps;
> + t_hs_settle_ps = t_hs_prepare_max_ps;
> +
> + timer_period_ps = div_u64(PSEC_PER_SEC, timer_clk_rate);
> + settle_cnt = t_hs_settle_ps / timer_period_ps - 6;
> +
> + return settle_cnt;
> +}
> +
> +static void
> +phy_qcom_mipi_csi2_gen2_config_lanes(struct mipi_csi2phy_device *csi2phy,
> + u8 settle_cnt)
> +{
> + const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> + const struct mipi_csi2phy_lane_regs *r = regs->init_seq;
> + int i, array_size = regs->lane_array_size;
> + u32 val;
> +
> + for (i = 0; i < array_size; i++, r++) {
> + switch (r->param_type) {
> + case CSIPHY_SETTLE_CNT_LOWER_BYTE:
> + val = settle_cnt & 0xff;
> + break;
> + case CSIPHY_SKEW_CAL:
> + /* TODO: support application of skew from dt flag */
> + continue;
> + default:
> + val = r->reg_data;
> + break;
> + }
> + writel(val, csi2phy->base + r->reg_addr);
> + if (r->delay_us)
> + udelay(r->delay_us);
> + }
> +}
> +
> +static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
> + struct mipi_csi2phy_stream_cfg *cfg)
> +{
> + const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> + struct mipi_csi2phy_lanes_cfg *lane_cfg = &cfg->lane_cfg;
> + u8 settle_cnt;
> + u8 val;
> + int i;
> +
> + settle_cnt = phy_qcom_mipi_csi2_settle_cnt_calc(cfg->link_freq, csi2phy->timer_clk_rate);
> +
> + /* Lane position enable in common reg offset */
> + val = BIT(lane_cfg->clk.pos);
> + for (i = 0; i < cfg->num_data_lanes; i++)
> + val |= BIT(lane_cfg->data[i].pos);
> +
> + writel(val, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
> +
> + /* Lane configuration for polarity @ CSIPHY-base + CTRL9 */
> + for (i = 0; i < cfg->num_data_lanes; i++) {
> + if (lane_cfg->data[i].pol) {
> + u8 pos = lane_cfg->data[i].pos;
> +
> + writel(BIT(2), csi2phy->base + CSIPHY_2PH_LN_CSI_2PHASE_CTRL9n(pos));
> + }
> + }
> +
> + if (lane_cfg->clk.pol)
> + writel(BIT(2), csi2phy->base + CSIPHY_2PH_LN_CSI_2PHASE_CTRL9n(lane_cfg->clk.pos));
> +
> + val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B;
> + writel(val, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
> +
> + val = 0x02;
> + writel(val, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 7));
> +
> + val = 0x00;
> + writel(val, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
> +
> + phy_qcom_mipi_csi2_gen2_config_lanes(csi2phy, settle_cnt);
> +
> + /* IRQ_MASK registers - disable all interrupts */
> + for (i = CSI_COMMON_STATUS_NUM; i < CSI_CTRL_STATUS_INDEX; i++) {
> + writel(0, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, i));
> + }
> +
> + return 0;
> +}
> +
> +static void
> +phy_qcom_mipi_csi2_lanes_disable(struct mipi_csi2phy_device *csi2phy,
> + struct mipi_csi2phy_stream_cfg *cfg)
> +{
> + const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> +
> + writel(0, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
> +
> + writel(0, csi2phy->base +
> + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
> +}
> +
> +static const struct mipi_csi2phy_hw_ops phy_qcom_mipi_csi2_ops_3ph_1_0 = {
> + .hw_version_read = phy_qcom_mipi_csi2_hw_version_read,
> + .reset = phy_qcom_mipi_csi2_reset,
> + .lanes_enable = phy_qcom_mipi_csi2_lanes_enable,
> + .lanes_disable = phy_qcom_mipi_csi2_lanes_disable,
> +};
> +
> +static const char * const x1e_clks[] = {
> + "core",
> + "timer"
> +};
> +
> +static const char * const x1e_supplies[] = {
> + "vdda-0p9",
> + "vdda-1p2"
> +};
> +
> +static const char * const x1e_genpd_names[] = {
> + "mmcx",
> + "mx",
> +};
> +
> +const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e = {
> + .ops = &phy_qcom_mipi_csi2_ops_3ph_1_0,
> + .reg_info = {
> + .init_seq = lane_regs_x1e80100,
> + .lane_array_size = ARRAY_SIZE(lane_regs_x1e80100),
> + .common_regs_offset = 0x1000,
> + },
> + .supply_names = (const char **)x1e_supplies,
> + .num_supplies = ARRAY_SIZE(x1e_supplies),
> + .clk_names = (const char **)x1e_clks,
> + .num_clk = ARRAY_SIZE(x1e_clks),
> + .opp_clk = x1e_clks[0],
> + .timer_clk = x1e_clks[1],
> + .genpd_names = (const char **)x1e_genpd_names,
> + .num_genpd_names = ARRAY_SIZE(x1e_genpd_names),
> +};
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> new file mode 100644
> index 0000000000000..dfeff863a406f
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> @@ -0,0 +1,402 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025, Linaro Ltd.
> + */
> +#include <dt-bindings/phy/phy.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pm_opp.h>
> +#include <linux/phy/phy.h>
> +#include <linux/phy/phy-mipi-dphy.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#include "phy-qcom-mipi-csi2.h"
> +
> +static int
> +phy_qcom_mipi_csi2_set_clock_rates(struct mipi_csi2phy_device *csi2phy,
> + s64 link_freq)
> +{
> + struct device *dev = csi2phy->dev;
> + unsigned long opp_rate = link_freq / 4;
> + struct dev_pm_opp *opp;
> + long timer_rate;
> + int i, ret;
> +
> + opp = dev_pm_opp_find_freq_ceil(dev, &opp_rate);
> + if (IS_ERR(opp)) {
> + dev_err(csi2phy->dev, "Couldn't find ceiling for %lld Hz\n",
> + link_freq);
> + return PTR_ERR(opp);
> + }
> +
> + for (i = 0; i < csi2phy->pd_list->num_pds; i++) {
> + unsigned int perf = dev_pm_opp_get_required_pstate(opp, i);
> +
> + ret = dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], perf);
Same comment as
https://lore.kernel.org/all/1f5d28ff-5661-4912-952d-d6114d1b1cb3@oss.qualcomm.com/
This is already handled in dev_pm_opp_set_rate -> _set_opp ->
_set_required_opps -> _set_opp_level ->
dev_pm_domain_set_performance_state. Should we call it explicitly again?
> + if (ret) {
> + dev_err(csi2phy->dev, "Couldn't set perf state %u\n",
> + perf);
> + dev_pm_opp_put(opp);
> + goto unset_pstate;
> + }
> + }
> + dev_pm_opp_put(opp);
> +
> + ret = dev_pm_opp_set_rate(dev, opp_rate);
> + if (ret) {
> + dev_err(csi2phy->dev, "dev_pm_opp_set_rate() fail\n");
> + goto unset_opp_rate;
> + }
> +
> + timer_rate = clk_round_rate(csi2phy->timer_clk, link_freq / 4);
> + if (timer_rate <= 0) {
> + ret = -ENODEV;
> + goto unset_opp_rate;
> + }
> +
> + ret = clk_set_rate(csi2phy->timer_clk, timer_rate);
> + if (ret)
> + goto unset_opp_rate;
> +
> + csi2phy->timer_clk_rate = timer_rate;
> +
> + return 0;
> +
> +unset_opp_rate:
> + dev_pm_opp_set_rate(dev, 0);
> +
> +unset_pstate:
> + while (i--)
> + dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
> +
> + return ret;
> +}
> +
> +static int phy_qcom_mipi_csi2_configure(struct phy *phy,
> + union phy_configure_opts *opts)
> +{
> + struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
> + struct phy_configure_opts_mipi_dphy *dphy_cfg = &opts->mipi_dphy;
> + struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
> + int ret;
> +
> + ret = phy_mipi_dphy_config_validate(dphy_cfg);
> + if (ret)
> + return ret;
> +
> + if (dphy_cfg->lanes < 1 || dphy_cfg->lanes > CSI2_MAX_DATA_LANES)
Can have a macro for the min # lanes as well if you would like.> +
return -EINVAL;
> +
> + stream_cfg->link_freq = dphy_cfg->hs_clk_rate;
> +
> + return 0;
> +}
> +
> +static int phy_qcom_mipi_csi2_power_on(struct phy *phy)
> +{
> + struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
> + const struct mipi_csi2phy_hw_ops *ops = csi2phy->soc_cfg->ops;
> + struct device *dev = &phy->dev;
> + int i, ret;
> +
> + ret = regulator_bulk_enable(csi2phy->soc_cfg->num_supplies,
> + csi2phy->supplies);
> + if (ret)
> + return ret;
> +
> + ret = pm_runtime_resume_and_get(csi2phy->dev);
> + if (ret < 0)
> + goto disable_regulators;
> +
> + ret = phy_qcom_mipi_csi2_set_clock_rates(csi2phy, csi2phy->stream_cfg.link_freq);
> + if (ret)
> + goto poweroff_phy;
> +
> + ret = clk_bulk_prepare_enable(csi2phy->soc_cfg->num_clk,
> + csi2phy->clks);
> + if (ret) {
> + dev_err(dev, "failed to enable clocks, %d\n", ret);
> + goto unset_rate;
> + }
> +
> + ops->reset(csi2phy);
> +
> + ops->hw_version_read(csi2phy);
> +
> + return ops->lanes_enable(csi2phy, &csi2phy->stream_cfg);
> +
> +unset_rate:
> + for (i = 0; i < csi2phy->pd_list->num_pds; i++)
> + dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
> +
> + dev_pm_opp_set_rate(csi2phy->dev, 0);
> +
> +poweroff_phy:
> + pm_runtime_put_sync(csi2phy->dev);
> +
> +disable_regulators:
> + regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
> + csi2phy->supplies);
> +
> + return ret;
> +}
> +
> +static int phy_qcom_mipi_csi2_power_off(struct phy *phy)
> +{
> + struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
> + const struct mipi_csi2phy_hw_ops *ops = csi2phy->soc_cfg->ops;
> + int i;
> +
> + ops->lanes_disable(csi2phy, &csi2phy->stream_cfg);
> +
> + clk_bulk_disable_unprepare(csi2phy->soc_cfg->num_clk,
> + csi2phy->clks);
> +
> + for (i = 0; i < csi2phy->pd_list->num_pds; i++)
> + dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
> +
> + dev_pm_opp_set_rate(csi2phy->dev, 0);
> +
> + pm_runtime_put_sync(csi2phy->dev);
> +
> + regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
> + csi2phy->supplies);
> +
> + return 0;
> +}
> +
> +static const struct phy_ops phy_qcom_mipi_csi2_ops = {
> + .configure = phy_qcom_mipi_csi2_configure,
> + .power_on = phy_qcom_mipi_csi2_power_on,
> + .power_off = phy_qcom_mipi_csi2_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static struct phy *qcom_csi2_phy_xlate(struct device *dev,
> + const struct of_phandle_args *args)
> +{
> + struct mipi_csi2phy_device *csi2phy = dev_get_drvdata(dev);
> +
> + if (args->args[0] != PHY_TYPE_DPHY) {
> + dev_err(csi2phy->dev, "mode %d -EOPNOTSUPP\n", args->args[0]);
> + return ERR_PTR(-EOPNOTSUPP);
> + }
> +
> + csi2phy->phy_mode = args->args[0];
> +
> + return csi2phy->phy;
> +}
> +
> +static int phy_qcom_mipi_csi2_attach_pm_domains(struct mipi_csi2phy_device *csi2phy)
> +{
> + const struct dev_pm_domain_attach_data pd_data = {
> + .pd_names = csi2phy->soc_cfg->genpd_names,
> + .num_pd_names = csi2phy->soc_cfg->num_genpd_names,
> + };
> +
> + return devm_pm_domain_attach_list(csi2phy->dev, &pd_data, &csi2phy->pd_list);
> +}
> +
> +static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
> +{
> + struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
> + u32 lane_polarities[CSI2_MAX_DATA_LANES + 1];
> + u32 data_lanes[CSI2_MAX_DATA_LANES];
> + struct device *dev = csi2phy->dev;
> + struct fwnode_handle *ep;
> + int num_polarities;
> + int num_data_lanes;
> + u32 clock_lane;
> + int i, ret;
> +
> + ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 1, 0,
> + FWNODE_GRAPH_ENDPOINT_NEXT);
> + if (ep) {
> + fwnode_handle_put(ep);
> + dev_err(dev, "DPHY split mode is not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0, 0);
> + if (!ep) {
> + dev_err(dev, "Missing port@0\n");
> + return -ENODEV;
> + }
> +
> + num_data_lanes = fwnode_property_count_u32(ep, "data-lanes");
> + if (num_data_lanes < 1 || num_data_lanes > CSI2_MAX_DATA_LANES) {
> + ret = -EINVAL;
> + dev_err(dev, "Invalid data-lanes count: %d\n", num_data_lanes);
> + goto out_put;
> + }
> + stream_cfg->num_data_lanes = num_data_lanes;
> +
> + ret = fwnode_property_read_u32_array(ep, "data-lanes", data_lanes,
> + stream_cfg->num_data_lanes);
> + if (ret) {
> + dev_err(dev, "Failed to read data-lanes: %d\n", ret);
> + goto out_put;
> + }
> +
> + ret = fwnode_property_read_u32(ep, "clock-lanes", &clock_lane);
> + if (ret) {
> + clock_lane = CSI2_DEFAULT_CLK_LN;
> + dev_info(dev, "Using default clock-lane %d\n",
> + CSI2_DEFAULT_CLK_LN);
> + }
> +
> + /* lane-polarities: optional, up to num_data_lanes + 1 entries */
> + memset(lane_polarities, 0x00, sizeof(lane_polarities));
> + num_polarities = fwnode_property_count_u32(ep, "lane-polarities");
> + if (num_polarities > 0) {
> + if (num_polarities != stream_cfg->num_data_lanes + 1) {
> + ret = -EINVAL;
> + dev_err(dev, "clock+data-lane %d/polarities %d mismatch\n",
> + stream_cfg->num_data_lanes + 1, num_polarities);
> + goto out_put;
> + }
> +
> + ret = fwnode_property_read_u32_array(ep, "lane-polarities", lane_polarities,
> + num_polarities);
> + if (ret) {
> + dev_err(dev, "Failed to read lane-polarities: %d\n", ret);
> + goto out_put;
> + }
> + }
> +
> + for (i = 0; i < csi2phy->stream_cfg.num_data_lanes; i++) {
> + csi2phy->stream_cfg.lane_cfg.data[i].pos = data_lanes[i];
> + csi2phy->stream_cfg.lane_cfg.data[i].pol = lane_polarities[i + 1];
> + }
> + csi2phy->stream_cfg.lane_cfg.clk.pos = clock_lane;
> + csi2phy->stream_cfg.lane_cfg.clk.pol = lane_polarities[0];
> +
> + ret = 0;
> +
> +out_put:
> + fwnode_handle_put(ep);
> +
> + return ret;
> +}
> +
> +static int phy_qcom_mipi_csi2_probe(struct platform_device *pdev)
> +{
> + unsigned int i, num_clk, num_supplies;
> + struct mipi_csi2phy_device *csi2phy;
> + struct phy_provider *phy_provider;
> + struct device *dev = &pdev->dev;
> + struct phy *generic_phy;
> + int ret;
> +
> + csi2phy = devm_kzalloc(dev, sizeof(*csi2phy), GFP_KERNEL);
> + if (!csi2phy)
> + return -ENOMEM;
> +
> + csi2phy->dev = dev;
> + dev_set_drvdata(dev, csi2phy);
> +
> + csi2phy->soc_cfg = device_get_match_data(&pdev->dev);
> +
> + if (!csi2phy->soc_cfg)
> + return -EINVAL;
> +
> + num_clk = csi2phy->soc_cfg->num_clk;
> + csi2phy->clks = devm_kzalloc(dev, sizeof(*csi2phy->clks) * num_clk, GFP_KERNEL);
> + if (!csi2phy->clks)
> + return -ENOMEM;
> +
> + ret = phy_qcom_mipi_csi2_parse_routing(csi2phy);
> + if (ret)
> + return ret;
> +
> + ret = phy_qcom_mipi_csi2_attach_pm_domains(csi2phy);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Failed to attach power-domain list\n");
> +
> + devm_pm_runtime_enable(dev);
> +
> + for (i = 0; i < num_clk; i++)
> + csi2phy->clks[i].id = csi2phy->soc_cfg->clk_names[i];
> +
> + ret = devm_clk_bulk_get(dev, num_clk, csi2phy->clks);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to get clocks\n");
> +
> + csi2phy->timer_clk = devm_clk_get(dev, csi2phy->soc_cfg->timer_clk);
> + if (IS_ERR(csi2phy->timer_clk)) {
> + return dev_err_probe(dev, PTR_ERR(csi2phy->timer_clk),
> + "Failed to get timer clock\n");
> + }
> +
> + ret = devm_pm_opp_set_clkname(dev, csi2phy->soc_cfg->opp_clk);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to set opp clkname\n");
> +
> + ret = devm_pm_opp_of_add_table(dev);
> + if (ret && ret != -ENODEV)
> + return dev_err_probe(dev, ret, "invalid OPP table in device tree\n");
> +
> + num_supplies = csi2phy->soc_cfg->num_supplies;
> + csi2phy->supplies = devm_kzalloc(dev, sizeof(*csi2phy->supplies) * num_supplies,
> + GFP_KERNEL);
> + if (!csi2phy->supplies)
> + return -ENOMEM;
> +
> + for (i = 0; i < num_supplies; i++)
> + csi2phy->supplies[i].supply = csi2phy->soc_cfg->supply_names[i];
> +
> + ret = devm_regulator_bulk_get(dev, num_supplies, csi2phy->supplies);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to get regulator supplies\n");
> +
> + csi2phy->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(csi2phy->base))
> + return PTR_ERR(csi2phy->base);
> +
> + generic_phy = devm_phy_create(dev, NULL, &phy_qcom_mipi_csi2_ops);
> + if (IS_ERR(generic_phy)) {
> + ret = PTR_ERR(generic_phy);
> + return dev_err_probe(dev, ret, "failed to create phy\n");
> + }
> + csi2phy->phy = generic_phy;
> +
> + phy_set_drvdata(generic_phy, csi2phy);
> +
> + phy_provider = devm_of_phy_provider_register(dev, qcom_csi2_phy_xlate);
> + if (!IS_ERR(phy_provider))
> + dev_dbg(dev, "Registered MIPI CSI2 PHY device\n");
> +
> + return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct of_device_id phy_qcom_mipi_csi2_of_match_table[] = {
> + { .compatible = "qcom,x1e80100-csi2-phy", .data = &mipi_csi2_dphy_4nm_x1e },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, phy_qcom_mipi_csi2_of_match_table);
> +
> +static struct platform_driver phy_qcom_mipi_csi2_driver = {
> + .probe = phy_qcom_mipi_csi2_probe,
> + .driver = {
> + .name = "qcom-mipi-csi2-phy",
> + .of_match_table = phy_qcom_mipi_csi2_of_match_table,
> + },
> +};
> +
> +module_platform_driver(phy_qcom_mipi_csi2_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm MIPI CSI2 PHY driver");
> +MODULE_AUTHOR("Bryan O'Donoghue <bryan.odonoghue@linaro.org>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
> new file mode 100644
> index 0000000000000..e7c1ce00916e3
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
> @@ -0,0 +1,95 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + *
> + * Qualcomm MIPI CSI2 CPHY/DPHY driver
> + *
> + * Copyright (C) 2025 Linaro Ltd.
> + */
> +#ifndef __PHY_QCOM_MIPI_CSI2_H__
> +#define __PHY_QCOM_MIPI_CSI2_H__
> +
> +#include <linux/phy/phy.h>
> +
> +#define CSI2_MAX_DATA_LANES 4
> +#define CSI2_DEFAULT_CLK_LN 7
> +
> +struct mipi_csi2phy_lane {
> + u8 pos;
> + u8 pol;
> +};
> +
> +struct mipi_csi2phy_lanes_cfg {
> + struct mipi_csi2phy_lane data[CSI2_MAX_DATA_LANES];
> + struct mipi_csi2phy_lane clk;
> +};
> +
> +struct mipi_csi2phy_stream_cfg {
> + s64 link_freq;
> + u8 num_data_lanes;
> + struct mipi_csi2phy_lanes_cfg lane_cfg;
> +};
> +
> +struct mipi_csi2phy_device;
> +
> +struct mipi_csi2phy_hw_ops {
> + void (*hw_version_read)(struct mipi_csi2phy_device *csi2phy_dev);
> + void (*reset)(struct mipi_csi2phy_device *csi2phy_dev);
> + int (*lanes_enable)(struct mipi_csi2phy_device *csi2phy_dev,
> + struct mipi_csi2phy_stream_cfg *cfg);
> + void (*lanes_disable)(struct mipi_csi2phy_device *csi2phy_dev,
> + struct mipi_csi2phy_stream_cfg *cfg);
> +};
> +
> +struct mipi_csi2phy_lane_regs {
> + const s32 reg_addr;
> + const s32 reg_data;
> + const u32 delay_us;
> + const u32 param_type;
> +};
> +
> +struct mipi_csi2phy_device_regs {
> + const struct mipi_csi2phy_lane_regs *init_seq;
> + const int lane_array_size;
> + const u32 common_regs_offset;
> +};
> +
> +struct mipi_csi2phy_soc_cfg {
> + const struct mipi_csi2phy_hw_ops *ops;
> + const struct mipi_csi2phy_device_regs reg_info;
> +
> + const char ** const supply_names;
> + const unsigned int num_supplies;
> +
> + const char ** const clk_names;
> + const unsigned int num_clk;
> +
> + const char * const opp_clk;
> + const char * const timer_clk;
> +
> + const char ** const genpd_names;
> + const unsigned int num_genpd_names;
> +};
> +
> +struct mipi_csi2phy_device {
> + struct device *dev;
> + u8 phy_mode;
> +
> + struct phy *phy;
> + void __iomem *base;
> +
> + struct clk_bulk_data *clks;
> + struct clk *timer_clk;
> + u32 timer_clk_rate;
> +
> + struct regulator_bulk_data *supplies;
> + struct dev_pm_domain_list *pd_list;
> +
> + const struct mipi_csi2phy_soc_cfg *soc_cfg;
> + struct mipi_csi2phy_stream_cfg stream_cfg;
> +
> + u32 hw_version;
> +};
> +
> +extern const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e;
> +
> +#endif /* __PHY_QCOM_MIPI_CSI2_H__ */
>
Thanks,
Vijay.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-06-03 20:52 UTC (permalink / raw)
To: Vijay Kumar Tumati, Vladimir Zapolskiy, Bryan O'Donoghue,
Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <514cf213-5778-45e1-8d70-d3fe27991fcc@oss.qualcomm.com>
On 03/06/2026 21:24, Vijay Kumar Tumati wrote:
>
>
> On 6/3/2026 1:16 PM, Vijay Kumar Tumati wrote:
>> Hi,
>>
>> On 6/2/2026 3:51 PM, Bryan O'Donoghue wrote:
>>> On 02/06/2026 22:59, Vladimir Zapolskiy wrote:
>>>> On 5/23/26 05:48, Bryan O'Donoghue wrote:
>>>>> Add a base schema initially compatible with x1e80100 to describe
>>>>> MIPI CSI2
>>>>> PHY devices.
>>>>>
>>>>> The hardware can support both CPHY, DPHY and a special split-mode DPHY.
>>>>>
>>>>> The schema here defines three ports:
>>>>>
>>>>> port@0:
>>>>> The first input port where a sensor is always required.
>>>>>
>>>>> port@1:
>>>>> A second optional input port which if present implies DPHY
>>>>> split-mode.
>>>>>
>>>>> port@2:
>>>>> A third always required output port which connects to the
>>>>> controller.
>>>>>
>>>>
>>>> This port numeration is imperfect, because port@0 and port@2 are
>>>> required,
>>>> while middle port@1 is optional.
>>>>
>>>> Like it was stated before a number of times, it seems natural to operate
>>>> with two ports, where input port may have two endpoints rather than 3
>>>> ports,
>>>> also that approach solves the problem of a hole in the port numeration.
>>>
>>> Can you confirm this is what you are after ?
>>>
>>> port@0 {
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>>
>>> endpoint@0 { /* primary sensor */
>>> reg = <0>;
>>> data-lanes = <0 1 2 3>;
>>> remote-endpoint = <&sensor0_out>;
>>> };
>>>
>>> endpoint@1 { /* split-mode second sensor, optional */
>>> reg = <1>;
>>> data-lanes = <0>;
>>> remote-endpoint = <&sensor1_out>;
>>> };
>>> };
>>>
>>> port@1 { /* output to CAMSS, was port@2 */
>>> endpoint { remote-endpoint = <&controller_in>; };
>>> };
>>>
>>> This works for me BTW.
>> Either way, do we need to document the constraint of using port@0 or
>> endpoint@0 'only' for the 4+1 or 2+1 mode and the other one is for the
>> 1+1 mode? Or is it implicit from this bindings for a developer?
The binding mandates it with an if / else structure
>
>>>>> The CSIPHY devices have their own pinouts on the SoC as well as
>>>>> their own
>>>>> individual voltage rails.
>>>>>
>>>>> The need to model voltage rails on a per-PHY basis leads us to define
>>>>> CSIPHY devices as individual nodes.
>>>>>
>>>>> Two nice outcomes in terms of schema and DT arise from this change.
>>>>>
>>>>> 1. The ability to define on a per-PHY basis voltage rails.
>>>>> 2. The ability to require those voltage.
>>>>>
>>>>> We have had a complete bodge upstream for this where a single set of
>>>>> voltage rail for all CSIPHYs has been buried inside of CAMSS.
>>>>>
>>>>> Much like the I2C bus which is dedicated to Camera sensors - the CCI
>>>>> bus in
>>>>> CAMSS parlance, the CSIPHY devices should be individually modelled.
>>>>>
>>>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>>> ---
>>>>> .../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 209 ++++++++
>>>>> + ++++++++++++
>>>>> 1 file changed, 209 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-
>>>>> csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-
>>>>> csi2-phy.yaml
>>>>> new file mode 100644
>>>>> index 0000000000000..270375f949880
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>>>> @@ -0,0 +1,209 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Qualcomm CSI2 PHY
>>>>> +
>>>>> +maintainers:
>>>>> + - Bryan O'Donoghue <bod@kernel.org>
>>>>> +
>>>>> +description:
>>>>> + Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI
>>>>> CSI2 sensors
>>>>> + to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and
>>>>> D-PHY
>>>>> + modes.
>>>>> +
>>>>> +properties:
>>>>> + compatible:
>>>>> + const: qcom,x1e80100-csi2-phy
>>>>> +
>>>>> + reg:
>>>>> + maxItems: 1
>>>>> +
>>>>> + "#phy-cells":
>>>>> + const: 1
>>>>> + description:
>>>>> + The single cell specifies the PHY operating mode.
>>>>
>>>> #phy-cells should be 0, because the PHY operating mode is well defined
>>>> by 'bus-type' property of an endpoint on the sensor side, the opposite
>>>> side of CAMSS/CSID as a CSIPHY "consumer" should not dictate the PHY
>>>> type.
>>>
>>> Rob said consumer but, I'm also not very bothered about that. bus-type
>>> is perfectly acceptable to me.
>>>
>>>>> +
>>>>> + clocks:
>>>>> + maxItems: 2
>>>>> +
>>>>> + clock-names:
>>>>> + items:
>>>>> + - const: core
>>>>> + - const: timer
>>>>> +
>>>>> + interrupts:
>>>>> + maxItems: 1
>>>>> +
>>>>> + operating-points-v2:
>>>>> + maxItems: 1
>>>>> +
>>>>> + power-domains:
>>>>> + items:
>>>>> + - description: MMCX voltage rail
>>>>> + - description: MXC or MXA voltage rail
>>>>
>>>> Only "qcom,x1e80100-csi2-phy" device is supported so far, unlikely it's
>>>> the case that "MXC or MXA voltage rail" should be specified, it'd be
>>>> just one of two or both.
>>>
>>> Hmm. I'm not being clear here if this is your take, I will reword it
>>> to make it clearer this generation of PHY _must_ have either
>>>
>>> - MMCX and MXC
>>> or
>>> - MMCX and MXA
>> I am not sure of this, Bryan. If you look at the PHY core clock
>> separately, sure, that is correct. But all of them, on this platform as
>> well, share the RCG, which requires all 3 power domains. So
>> fundamentally, you need to enable all of those from each PHY. You can
>> make it constant 3 power domains.>
Hmm do you mean the GDSC which I omitted form the example and shouldn't
have TITAN_TOP_GDSC or do you mean MMCX, MXC and MXA are required ?
I don't believe the clock definitions say that. Also not what you said
in the previous cycle.
I'd be obliged if you could be precise and clear since as you know the
PHY as a separate thing is important to release new SoC additions.
>>>>> +
>>>>> + power-domain-names:
>>>>> + items:
>>>>> + - const: mmcx
>>>>> + - const: mx
>>>>> +
>>>>> + vdda-0p9-supply:
>>>>> + description: Phandle to a 0.9V regulator supply to a PHY.
>>>>> +
>>>>> + vdda-1p2-supply:
>>>>> + description: Phandle to 1.2V regulator supply to a PHY.
>>>>> +
>>>>> + ports:
>>>>> + $ref: /schemas/graph.yaml#/properties/ports
>>>>> +
>>>>> + properties:
>>>>> + port@0:
>>>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>>>> + description: Sensor input. Always present.
>>>>> + unevaluatedProperties: false
>>>>> +
>>>>> + properties:
>>>>> + endpoint:
>>>>> + $ref: /schemas/media/video-interfaces.yaml#
>>>>> + unevaluatedProperties: false
>>>>> + properties:
>>>>> + data-lanes:
>>>>> + minItems: 1
>>>>> + maxItems: 4
>>>>> + clock-lanes:
>>>>> + maxItems: 1
>>>>> + remote-endpoint: true
>>>>> + required:
>>>>> + - data-lanes
>>>>> + - remote-endpoint
>>>>> +
>>>>> + port@1:
>>>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>>>> + description:
>>>>> + Second sensor input. When present, indicates DPHY split
>>>>> mode.
>>>>> + unevaluatedProperties: false
>>>>> +
>>>>> + properties:
>>>>> + endpoint:
>>>>> + $ref: /schemas/media/video-interfaces.yaml#
>>>>> + unevaluatedProperties: false
>>>>> + properties:
>>>>> + data-lanes:
>>>>> + maxItems: 1
>>>>> + clock-lanes:
>>>>> + maxItems: 1
>>>>> + remote-endpoint: true
>>>>> + required:
>>>>> + - data-lanes
>>>>> + - clock-lanes
>>>>> + - remote-endpoint
>>>>
>>>> As it's stated above, it should be converted to a single port with two
>>>> endpoints, it'd be done in accordance to video-interfaces.yaml.
>>>>
>>>>> +
>>>>> + port@2:
>>>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>>>> + description: Output to CAMSS controller.
>>>>> + unevaluatedProperties: false
>>>>> +
>>>>> + properties:
>>>>> + endpoint:
>>>>> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
>>>>> + unevaluatedProperties: false
>>>>> + properties:
>>>>> + remote-endpoint: true
>>>>> + required:
>>>>> + - remote-endpoint
>>>>> +
>>>>> + required:
>>>>> + - port@0
>>>>> + - port@2
>>>>> +
>>>>> +required:
>>>>> + - compatible
>>>>> + - reg
>>>>> + - "#phy-cells"
>>>>> + - clocks
>>>>> + - clock-names
>>>>> + - interrupts
>>>>> + - operating-points-v2
>>>>> + - power-domains
>>>>> + - power-domain-names
>>>>> + - vdda-0p9-supply
>>>>> + - vdda-1p2-supply
>>>>> + - ports
>>>>> +
>>>>> +additionalProperties: false
>>>>> +
>>>>> +examples:
>>>>> + - |
>>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>> + #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
>>>>> + #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
>>>>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>>>>> +
>>>>> + csiphy4: csiphy@ace4000 {
>>>>> + compatible = "qcom,x1e80100-csi2-phy";
>>>>> + reg = <0x0ace4000 0x2000>;
>>>>> + #phy-cells = <1>;
>>>>> +
>>>>> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>>>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
>>>>> + clock-names = "core",
>>>>> + "timer";
>>>>> +
>>>>> + operating-points-v2 = <&csiphy_opp_table>;
>>>>> +
>>>>> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>>>>> +
>>>>> + power-domains = <&rpmhpd RPMHPD_MMCX>,
>>>>> + <&rpmhpd RPMHPD_MX>;
>>>>> + power-domain-names = "mmcx",
>>>>> + "mx";
> Actually, one more thing, Why isn't TITAN TOP GDSC here?>>>> +
Yes the DTSI has TITAN_TOP_GDSC I haven't updated the YAML to capture that.
So it should be
top
mmcx
mx
With obviously on mmcx and mx scalable. We established that CSIPHY4 had
MXA whereas the other CSIPHYs had MXC in v5 or v4 - can you be clear if
you agreeing with that still or saying something different. Per my
memory of reading the docs, there was nothing in the clock tree to
indicate both MXA and MXC were required for all PHYs.
>>>>> + vdda-0p9-supply = <&vreg_l2c_0p8>;
>>>>> + vdda-1p2-supply = <&vreg_l1c_1p2>;
>>>>> +
>>>>> + ports {
>>>>> + #address-cells = <1>;
>>>>> + #size-cells = <0>;
>>>>> +
>>>>> + port@0 {
>>>>> + reg = <0>;
>>>>> + csiphy0_in_ep: endpoint {
>>>>> + data-lanes = <0 1>;
>>>>> + clock-lanes = <2>;
>>>>> + remote-endpoint = <&sensor_out>;
>>>>> + };
>>>>> + };
>>>>> +
>>>>> + port@2 {
>>>>> + reg = <2>;
>>>>> + csiphy0_out_ep: endpoint {
>>>>> + remote-endpoint = <&controller_in>;
>>>>> + };
>>>>> + };
>>>>> + };
>>>>> + };
>>>>> +
>>>>> + csiphy_opp_table: opp-table {
>>>>> + compatible = "operating-points-v2";
>>>>> +
>>>>> + opp-300000000 {
>>>>> + opp-hz = /bits/ 64 <300000000>;
>> I wonder why you would have only one clock here. You should be setting
>> the rate for both the core and timer, isn't it?
Yes the dtsi has it, the example does not. I had pushback from others
about the example being too complex - you can't please all of the people
all of the time.
I will drop the full table @ v9
>>> + required-opps =
>> <&rpmhpd_opp_low_svs_d1>,
>>>>> + <&rpmhpd_opp_low_svs_d1>;
>> Same here, it should 3 power domains set.>>> + };
Two power domains scaled, one set at least I hope that's what you mean
i.e. add the GDSC, already in my code I just didn't add it here as I
should have.
- GDSC enabled
- MMCX scaled
- MX scaled
When MX points to MXA the scaling is a NOP @ rpmhpd_opp_low_svs_d1.
Agreed ?
>>>>> +
>>>>> + opp-400000000 {
>>>>> + opp-hz = /bits/ 64 <400000000>;
>>>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>>>> + <&rpmhpd_opp_low_svs_d1>;
>> Why is one at svs and the other at svs_d1? Shouldn't both be svs?>>>
>> + };
>>>>> +
>>>>> + opp-480000000 {
>>>>> + opp-hz = /bits/ 64 <480000000>;
>>>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>>>> + <&rpmhpd_opp_low_svs_d1>;
>> And here, both should be svs_l1?>>> + };
>>>>> + };
>>>>>
>>>>
>>>> --
>>>> Best wishes,
>>>> Vladimir
>>>
>>>
>> Thanks,
>> Vijay.
>>
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vladimir Zapolskiy @ 2026-06-03 20:51 UTC (permalink / raw)
To: Vijay Kumar Tumati, Bryan O'Donoghue, Bryan O'Donoghue,
Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <514cf213-5778-45e1-8d70-d3fe27991fcc@oss.qualcomm.com>
On 6/3/26 23:24, Vijay Kumar Tumati wrote:
>
>
> On 6/3/2026 1:16 PM, Vijay Kumar Tumati wrote:
>> Hi,
>>
>> On 6/2/2026 3:51 PM, Bryan O'Donoghue wrote:
>>> On 02/06/2026 22:59, Vladimir Zapolskiy wrote:
>>>> On 5/23/26 05:48, Bryan O'Donoghue wrote:
>>>>> Add a base schema initially compatible with x1e80100 to describe
>>>>> MIPI CSI2
>>>>> PHY devices.
>>>>>
>>>>> The hardware can support both CPHY, DPHY and a special split-mode DPHY.
>>>>>
>>>>> The schema here defines three ports:
>>>>>
>>>>> port@0:
>>>>> The first input port where a sensor is always required.
>>>>>
>>>>> port@1:
>>>>> A second optional input port which if present implies DPHY
>>>>> split-mode.
>>>>>
>>>>> port@2:
>>>>> A third always required output port which connects to the
>>>>> controller.
>>>>>
>>>>
>>>> This port numeration is imperfect, because port@0 and port@2 are
>>>> required,
>>>> while middle port@1 is optional.
>>>>
>>>> Like it was stated before a number of times, it seems natural to operate
>>>> with two ports, where input port may have two endpoints rather than 3
>>>> ports,
>>>> also that approach solves the problem of a hole in the port numeration.
>>>
>>> Can you confirm this is what you are after ?
>>>
>>> port@0 {
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>>
>>> endpoint@0 { /* primary sensor */
>>> reg = <0>;
>>> data-lanes = <0 1 2 3>;
>>> remote-endpoint = <&sensor0_out>;
>>> };
>>>
>>> endpoint@1 { /* split-mode second sensor, optional */
>>> reg = <1>;
>>> data-lanes = <0>;
>>> remote-endpoint = <&sensor1_out>;
>>> };
>>> };
>>>
>>> port@1 { /* output to CAMSS, was port@2 */
>>> endpoint { remote-endpoint = <&controller_in>; };
>>> };
>>>
>>> This works for me BTW.
>> Either way, do we need to document the constraint of using port@0 or
>> endpoint@0 'only' for the 4+1 or 2+1 mode and the other one is for the
>> 1+1 mode? Or is it implicit from this bindings for a developer?>
>>>>> The CSIPHY devices have their own pinouts on the SoC as well as
>>>>> their own
>>>>> individual voltage rails.
>>>>>
>>>>> The need to model voltage rails on a per-PHY basis leads us to define
>>>>> CSIPHY devices as individual nodes.
>>>>>
>>>>> Two nice outcomes in terms of schema and DT arise from this change.
>>>>>
>>>>> 1. The ability to define on a per-PHY basis voltage rails.
>>>>> 2. The ability to require those voltage.
>>>>>
>>>>> We have had a complete bodge upstream for this where a single set of
>>>>> voltage rail for all CSIPHYs has been buried inside of CAMSS.
>>>>>
>>>>> Much like the I2C bus which is dedicated to Camera sensors - the CCI
>>>>> bus in
>>>>> CAMSS parlance, the CSIPHY devices should be individually modelled.
>>>>>
>>>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>>> ---
>>>>> .../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 209 ++++++++
>>>>> + ++++++++++++
>>>>> 1 file changed, 209 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-
>>>>> csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-
>>>>> csi2-phy.yaml
>>>>> new file mode 100644
>>>>> index 0000000000000..270375f949880
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>>>> @@ -0,0 +1,209 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Qualcomm CSI2 PHY
>>>>> +
>>>>> +maintainers:
>>>>> + - Bryan O'Donoghue <bod@kernel.org>
>>>>> +
>>>>> +description:
>>>>> + Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI
>>>>> CSI2 sensors
>>>>> + to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and
>>>>> D-PHY
>>>>> + modes.
>>>>> +
>>>>> +properties:
>>>>> + compatible:
>>>>> + const: qcom,x1e80100-csi2-phy
>>>>> +
>>>>> + reg:
>>>>> + maxItems: 1
>>>>> +
>>>>> + "#phy-cells":
>>>>> + const: 1
>>>>> + description:
>>>>> + The single cell specifies the PHY operating mode.
>>>>
>>>> #phy-cells should be 0, because the PHY operating mode is well defined
>>>> by 'bus-type' property of an endpoint on the sensor side, the opposite
>>>> side of CAMSS/CSID as a CSIPHY "consumer" should not dictate the PHY
>>>> type.
>>>
>>> Rob said consumer but, I'm also not very bothered about that. bus-type
>>> is perfectly acceptable to me.
>>>
>>>>> +
>>>>> + clocks:
>>>>> + maxItems: 2
>>>>> +
>>>>> + clock-names:
>>>>> + items:
>>>>> + - const: core
>>>>> + - const: timer
>>>>> +
>>>>> + interrupts:
>>>>> + maxItems: 1
>>>>> +
>>>>> + operating-points-v2:
>>>>> + maxItems: 1
>>>>> +
>>>>> + power-domains:
>>>>> + items:
>>>>> + - description: MMCX voltage rail
>>>>> + - description: MXC or MXA voltage rail
>>>>
>>>> Only "qcom,x1e80100-csi2-phy" device is supported so far, unlikely it's
>>>> the case that "MXC or MXA voltage rail" should be specified, it'd be
>>>> just one of two or both.
>>>
>>> Hmm. I'm not being clear here if this is your take, I will reword it
>>> to make it clearer this generation of PHY _must_ have either
>>>
>>> - MMCX and MXC
>>> or
>>> - MMCX and MXA
>> I am not sure of this, Bryan. If you look at the PHY core clock
>> separately, sure, that is correct. But all of them, on this platform as
>> well, share the RCG, which requires all 3 power domains. So
>> fundamentally, you need to enable all of those from each PHY. You can
>> make it constant 3 power domains.>
>>>>> +
>>>>> + power-domain-names:
>>>>> + items:
>>>>> + - const: mmcx
>>>>> + - const: mx
>>>>> +
>>>>> + vdda-0p9-supply:
>>>>> + description: Phandle to a 0.9V regulator supply to a PHY.
>>>>> +
>>>>> + vdda-1p2-supply:
>>>>> + description: Phandle to 1.2V regulator supply to a PHY.
>>>>> +
>>>>> + ports:
>>>>> + $ref: /schemas/graph.yaml#/properties/ports
>>>>> +
>>>>> + properties:
>>>>> + port@0:
>>>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>>>> + description: Sensor input. Always present.
>>>>> + unevaluatedProperties: false
>>>>> +
>>>>> + properties:
>>>>> + endpoint:
>>>>> + $ref: /schemas/media/video-interfaces.yaml#
>>>>> + unevaluatedProperties: false
>>>>> + properties:
>>>>> + data-lanes:
>>>>> + minItems: 1
>>>>> + maxItems: 4
>>>>> + clock-lanes:
>>>>> + maxItems: 1
>>>>> + remote-endpoint: true
>>>>> + required:
>>>>> + - data-lanes
>>>>> + - remote-endpoint
>>>>> +
>>>>> + port@1:
>>>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>>>> + description:
>>>>> + Second sensor input. When present, indicates DPHY split
>>>>> mode.
>>>>> + unevaluatedProperties: false
>>>>> +
>>>>> + properties:
>>>>> + endpoint:
>>>>> + $ref: /schemas/media/video-interfaces.yaml#
>>>>> + unevaluatedProperties: false
>>>>> + properties:
>>>>> + data-lanes:
>>>>> + maxItems: 1
>>>>> + clock-lanes:
>>>>> + maxItems: 1
>>>>> + remote-endpoint: true
>>>>> + required:
>>>>> + - data-lanes
>>>>> + - clock-lanes
>>>>> + - remote-endpoint
>>>>
>>>> As it's stated above, it should be converted to a single port with two
>>>> endpoints, it'd be done in accordance to video-interfaces.yaml.
>>>>
>>>>> +
>>>>> + port@2:
>>>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>>>> + description: Output to CAMSS controller.
>>>>> + unevaluatedProperties: false
>>>>> +
>>>>> + properties:
>>>>> + endpoint:
>>>>> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
>>>>> + unevaluatedProperties: false
>>>>> + properties:
>>>>> + remote-endpoint: true
>>>>> + required:
>>>>> + - remote-endpoint
>>>>> +
>>>>> + required:
>>>>> + - port@0
>>>>> + - port@2
>>>>> +
>>>>> +required:
>>>>> + - compatible
>>>>> + - reg
>>>>> + - "#phy-cells"
>>>>> + - clocks
>>>>> + - clock-names
>>>>> + - interrupts
>>>>> + - operating-points-v2
>>>>> + - power-domains
>>>>> + - power-domain-names
>>>>> + - vdda-0p9-supply
>>>>> + - vdda-1p2-supply
>>>>> + - ports
>>>>> +
>>>>> +additionalProperties: false
>>>>> +
>>>>> +examples:
>>>>> + - |
>>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>> + #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
>>>>> + #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
>>>>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>>>>> +
>>>>> + csiphy4: csiphy@ace4000 {
>>>>> + compatible = "qcom,x1e80100-csi2-phy";
>>>>> + reg = <0x0ace4000 0x2000>;
>>>>> + #phy-cells = <1>;
>>>>> +
>>>>> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>>>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
>>>>> + clock-names = "core",
>>>>> + "timer";
>>>>> +
>>>>> + operating-points-v2 = <&csiphy_opp_table>;
>>>>> +
>>>>> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>>>>> +
>>>>> + power-domains = <&rpmhpd RPMHPD_MMCX>,
>>>>> + <&rpmhpd RPMHPD_MX>;
>>>>> + power-domain-names = "mmcx",
>>>>> + "mx";
> Actually, one more thing, Why isn't TITAN TOP GDSC here?>>>> +
If CSIPHYs are true subdevices under the umbrella CAMSS device and well
described as subnodes, then likely none of power domains are needed to be
repeatedly described in the children device nodes, since this information
can be obtained from the parent device by the driver.
Technically 'power-domains' property can be safely removed, I believe.
>>>>> + vdda-0p9-supply = <&vreg_l2c_0p8>;
>>>>> + vdda-1p2-supply = <&vreg_l1c_1p2>;
>>>>> +
>>>>> + ports {
>>>>> + #address-cells = <1>;
>>>>> + #size-cells = <0>;
>>>>> +
>>>>> + port@0 {
>>>>> + reg = <0>;
>>>>> + csiphy0_in_ep: endpoint {
>>>>> + data-lanes = <0 1>;
>>>>> + clock-lanes = <2>;
>>>>> + remote-endpoint = <&sensor_out>;
>>>>> + };
>>>>> + };
>>>>> +
>>>>> + port@2 {
>>>>> + reg = <2>;
>>>>> + csiphy0_out_ep: endpoint {
>>>>> + remote-endpoint = <&controller_in>;
>>>>> + };
>>>>> + };
>>>>> + };
>>>>> + };
>>>>> +
>>>>> + csiphy_opp_table: opp-table {
>>>>> + compatible = "operating-points-v2";
>>>>> +
>>>>> + opp-300000000 {
>>>>> + opp-hz = /bits/ 64 <300000000>;
>> I wonder why you would have only one clock here. You should be setting
>> the rate for both the core and timer, isn't it?>>> + required-opps =
>> <&rpmhpd_opp_low_svs_d1>,
>>>>> + <&rpmhpd_opp_low_svs_d1>;
>> Same here, it should 3 power domains set.>>> + };
>>>>> +
>>>>> + opp-400000000 {
>>>>> + opp-hz = /bits/ 64 <400000000>;
>>>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>>>> + <&rpmhpd_opp_low_svs_d1>;
>> Why is one at svs and the other at svs_d1? Shouldn't both be svs?>>>
>> + };
>>>>> +
>>>>> + opp-480000000 {
>>>>> + opp-hz = /bits/ 64 <480000000>;
>>>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>>>> + <&rpmhpd_opp_low_svs_d1>;
>> And here, both should be svs_l1?>>> + };
>>>>> + };
>>>>>
>>>>
--
Best wishes,
Vladimir
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Vladimir Zapolskiy @ 2026-06-03 20:42 UTC (permalink / raw)
To: Bryan O'Donoghue, Dmitry Baryshkov
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
In-Reply-To: <8eb709ce-3469-4303-9c3d-8d1a221cb8de@linaro.org>
On 6/3/26 15:57, Bryan O'Donoghue wrote:
> On 03/06/2026 13:40, Dmitry Baryshkov wrote:
>>> Are you sure about that ?
>> Yes.
>>
>>> ipcat I thought designated lane 7 specifically as clk-lane i.e. named it
>>> CLK_LN of some description.
>> Split configurations explicitly use other lanes for clocks. E.g. check
>> the RB5 Navigation schematics, CAM0B connector.
>
> Can you please check:
>
> CSI_3PHASE_COMMON.CSI_COMMON_CTRL5
>
> 0 LN0_PWRDN_B Lane 0
> ...
> 7 LNCK_PWRDN_B Clock Lane
Please note that media devices have a numeration scheme of lanes starting
from 1 (it'd be easy to check/confirm it), for instance today CAMSS has
lane numeration starting from 0 is out of the accepted scheme, and here
it'd be better to correct it and not enter the same pit.
I don't have access to the IP spec, anyway I do not grasp it, where are
8 lanes on the CSIPHY found? Each CSIPHY IP has 4+1 D-PHY lanes, not 8.
>
> ... just a badly name field
>
> CSI_2PHASE_CTRL10
>
> Bit[2] = IS_CLKLANE
>
> Right so CSI_2PHASE_CTRL10 controls lane mode, indeed. Thanks for checking.
>
--
Best wishes,
Vladimir
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vijay Kumar Tumati @ 2026-06-03 20:24 UTC (permalink / raw)
To: Bryan O'Donoghue, Vladimir Zapolskiy, Bryan O'Donoghue,
Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <ec98ef2f-02b4-4086-8b4b-07b6953dbd20@oss.qualcomm.com>
On 6/3/2026 1:16 PM, Vijay Kumar Tumati wrote:
> Hi,
>
> On 6/2/2026 3:51 PM, Bryan O'Donoghue wrote:
>> On 02/06/2026 22:59, Vladimir Zapolskiy wrote:
>>> On 5/23/26 05:48, Bryan O'Donoghue wrote:
>>>> Add a base schema initially compatible with x1e80100 to describe
>>>> MIPI CSI2
>>>> PHY devices.
>>>>
>>>> The hardware can support both CPHY, DPHY and a special split-mode DPHY.
>>>>
>>>> The schema here defines three ports:
>>>>
>>>> port@0:
>>>> The first input port where a sensor is always required.
>>>>
>>>> port@1:
>>>> A second optional input port which if present implies DPHY
>>>> split-mode.
>>>>
>>>> port@2:
>>>> A third always required output port which connects to the
>>>> controller.
>>>>
>>>
>>> This port numeration is imperfect, because port@0 and port@2 are
>>> required,
>>> while middle port@1 is optional.
>>>
>>> Like it was stated before a number of times, it seems natural to operate
>>> with two ports, where input port may have two endpoints rather than 3
>>> ports,
>>> also that approach solves the problem of a hole in the port numeration.
>>
>> Can you confirm this is what you are after ?
>>
>> port@0 {
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> endpoint@0 { /* primary sensor */
>> reg = <0>;
>> data-lanes = <0 1 2 3>;
>> remote-endpoint = <&sensor0_out>;
>> };
>>
>> endpoint@1 { /* split-mode second sensor, optional */
>> reg = <1>;
>> data-lanes = <0>;
>> remote-endpoint = <&sensor1_out>;
>> };
>> };
>>
>> port@1 { /* output to CAMSS, was port@2 */
>> endpoint { remote-endpoint = <&controller_in>; };
>> };
>>
>> This works for me BTW.
> Either way, do we need to document the constraint of using port@0 or
> endpoint@0 'only' for the 4+1 or 2+1 mode and the other one is for the
> 1+1 mode? Or is it implicit from this bindings for a developer?>
>>>> The CSIPHY devices have their own pinouts on the SoC as well as
>>>> their own
>>>> individual voltage rails.
>>>>
>>>> The need to model voltage rails on a per-PHY basis leads us to define
>>>> CSIPHY devices as individual nodes.
>>>>
>>>> Two nice outcomes in terms of schema and DT arise from this change.
>>>>
>>>> 1. The ability to define on a per-PHY basis voltage rails.
>>>> 2. The ability to require those voltage.
>>>>
>>>> We have had a complete bodge upstream for this where a single set of
>>>> voltage rail for all CSIPHYs has been buried inside of CAMSS.
>>>>
>>>> Much like the I2C bus which is dedicated to Camera sensors - the CCI
>>>> bus in
>>>> CAMSS parlance, the CSIPHY devices should be individually modelled.
>>>>
>>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>> ---
>>>> .../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 209 ++++++++
>>>> + ++++++++++++
>>>> 1 file changed, 209 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-
>>>> csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-
>>>> csi2-phy.yaml
>>>> new file mode 100644
>>>> index 0000000000000..270375f949880
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>>> @@ -0,0 +1,209 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Qualcomm CSI2 PHY
>>>> +
>>>> +maintainers:
>>>> + - Bryan O'Donoghue <bod@kernel.org>
>>>> +
>>>> +description:
>>>> + Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI
>>>> CSI2 sensors
>>>> + to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and
>>>> D-PHY
>>>> + modes.
>>>> +
>>>> +properties:
>>>> + compatible:
>>>> + const: qcom,x1e80100-csi2-phy
>>>> +
>>>> + reg:
>>>> + maxItems: 1
>>>> +
>>>> + "#phy-cells":
>>>> + const: 1
>>>> + description:
>>>> + The single cell specifies the PHY operating mode.
>>>
>>> #phy-cells should be 0, because the PHY operating mode is well defined
>>> by 'bus-type' property of an endpoint on the sensor side, the opposite
>>> side of CAMSS/CSID as a CSIPHY "consumer" should not dictate the PHY
>>> type.
>>
>> Rob said consumer but, I'm also not very bothered about that. bus-type
>> is perfectly acceptable to me.
>>
>>>> +
>>>> + clocks:
>>>> + maxItems: 2
>>>> +
>>>> + clock-names:
>>>> + items:
>>>> + - const: core
>>>> + - const: timer
>>>> +
>>>> + interrupts:
>>>> + maxItems: 1
>>>> +
>>>> + operating-points-v2:
>>>> + maxItems: 1
>>>> +
>>>> + power-domains:
>>>> + items:
>>>> + - description: MMCX voltage rail
>>>> + - description: MXC or MXA voltage rail
>>>
>>> Only "qcom,x1e80100-csi2-phy" device is supported so far, unlikely it's
>>> the case that "MXC or MXA voltage rail" should be specified, it'd be
>>> just one of two or both.
>>
>> Hmm. I'm not being clear here if this is your take, I will reword it
>> to make it clearer this generation of PHY _must_ have either
>>
>> - MMCX and MXC
>> or
>> - MMCX and MXA
> I am not sure of this, Bryan. If you look at the PHY core clock
> separately, sure, that is correct. But all of them, on this platform as
> well, share the RCG, which requires all 3 power domains. So
> fundamentally, you need to enable all of those from each PHY. You can
> make it constant 3 power domains.>
>>>> +
>>>> + power-domain-names:
>>>> + items:
>>>> + - const: mmcx
>>>> + - const: mx
>>>> +
>>>> + vdda-0p9-supply:
>>>> + description: Phandle to a 0.9V regulator supply to a PHY.
>>>> +
>>>> + vdda-1p2-supply:
>>>> + description: Phandle to 1.2V regulator supply to a PHY.
>>>> +
>>>> + ports:
>>>> + $ref: /schemas/graph.yaml#/properties/ports
>>>> +
>>>> + properties:
>>>> + port@0:
>>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>>> + description: Sensor input. Always present.
>>>> + unevaluatedProperties: false
>>>> +
>>>> + properties:
>>>> + endpoint:
>>>> + $ref: /schemas/media/video-interfaces.yaml#
>>>> + unevaluatedProperties: false
>>>> + properties:
>>>> + data-lanes:
>>>> + minItems: 1
>>>> + maxItems: 4
>>>> + clock-lanes:
>>>> + maxItems: 1
>>>> + remote-endpoint: true
>>>> + required:
>>>> + - data-lanes
>>>> + - remote-endpoint
>>>> +
>>>> + port@1:
>>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>>> + description:
>>>> + Second sensor input. When present, indicates DPHY split
>>>> mode.
>>>> + unevaluatedProperties: false
>>>> +
>>>> + properties:
>>>> + endpoint:
>>>> + $ref: /schemas/media/video-interfaces.yaml#
>>>> + unevaluatedProperties: false
>>>> + properties:
>>>> + data-lanes:
>>>> + maxItems: 1
>>>> + clock-lanes:
>>>> + maxItems: 1
>>>> + remote-endpoint: true
>>>> + required:
>>>> + - data-lanes
>>>> + - clock-lanes
>>>> + - remote-endpoint
>>>
>>> As it's stated above, it should be converted to a single port with two
>>> endpoints, it'd be done in accordance to video-interfaces.yaml.
>>>
>>>> +
>>>> + port@2:
>>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>>> + description: Output to CAMSS controller.
>>>> + unevaluatedProperties: false
>>>> +
>>>> + properties:
>>>> + endpoint:
>>>> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
>>>> + unevaluatedProperties: false
>>>> + properties:
>>>> + remote-endpoint: true
>>>> + required:
>>>> + - remote-endpoint
>>>> +
>>>> + required:
>>>> + - port@0
>>>> + - port@2
>>>> +
>>>> +required:
>>>> + - compatible
>>>> + - reg
>>>> + - "#phy-cells"
>>>> + - clocks
>>>> + - clock-names
>>>> + - interrupts
>>>> + - operating-points-v2
>>>> + - power-domains
>>>> + - power-domain-names
>>>> + - vdda-0p9-supply
>>>> + - vdda-1p2-supply
>>>> + - ports
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> + - |
>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>> + #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
>>>> + #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
>>>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>>>> +
>>>> + csiphy4: csiphy@ace4000 {
>>>> + compatible = "qcom,x1e80100-csi2-phy";
>>>> + reg = <0x0ace4000 0x2000>;
>>>> + #phy-cells = <1>;
>>>> +
>>>> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
>>>> + clock-names = "core",
>>>> + "timer";
>>>> +
>>>> + operating-points-v2 = <&csiphy_opp_table>;
>>>> +
>>>> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>>>> +
>>>> + power-domains = <&rpmhpd RPMHPD_MMCX>,
>>>> + <&rpmhpd RPMHPD_MX>;
>>>> + power-domain-names = "mmcx",
>>>> + "mx";
Actually, one more thing, Why isn't TITAN TOP GDSC here?>>>> +
>>>> + vdda-0p9-supply = <&vreg_l2c_0p8>;
>>>> + vdda-1p2-supply = <&vreg_l1c_1p2>;
>>>> +
>>>> + ports {
>>>> + #address-cells = <1>;
>>>> + #size-cells = <0>;
>>>> +
>>>> + port@0 {
>>>> + reg = <0>;
>>>> + csiphy0_in_ep: endpoint {
>>>> + data-lanes = <0 1>;
>>>> + clock-lanes = <2>;
>>>> + remote-endpoint = <&sensor_out>;
>>>> + };
>>>> + };
>>>> +
>>>> + port@2 {
>>>> + reg = <2>;
>>>> + csiphy0_out_ep: endpoint {
>>>> + remote-endpoint = <&controller_in>;
>>>> + };
>>>> + };
>>>> + };
>>>> + };
>>>> +
>>>> + csiphy_opp_table: opp-table {
>>>> + compatible = "operating-points-v2";
>>>> +
>>>> + opp-300000000 {
>>>> + opp-hz = /bits/ 64 <300000000>;
> I wonder why you would have only one clock here. You should be setting
> the rate for both the core and timer, isn't it?>>> + required-opps =
> <&rpmhpd_opp_low_svs_d1>,
>>>> + <&rpmhpd_opp_low_svs_d1>;
> Same here, it should 3 power domains set.>>> + };
>>>> +
>>>> + opp-400000000 {
>>>> + opp-hz = /bits/ 64 <400000000>;
>>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>>> + <&rpmhpd_opp_low_svs_d1>;
> Why is one at svs and the other at svs_d1? Shouldn't both be svs?>>>
> + };
>>>> +
>>>> + opp-480000000 {
>>>> + opp-hz = /bits/ 64 <480000000>;
>>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>>> + <&rpmhpd_opp_low_svs_d1>;
> And here, both should be svs_l1?>>> + };
>>>> + };
>>>>
>>>
>>> --
>>> Best wishes,
>>> Vladimir
>>
>>
> Thanks,
> Vijay.
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vijay Kumar Tumati @ 2026-06-03 20:16 UTC (permalink / raw)
To: Bryan O'Donoghue, Vladimir Zapolskiy, Bryan O'Donoghue,
Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <478df3ed-d4ef-43aa-bb84-e2075798542b@kernel.org>
Hi,
On 6/2/2026 3:51 PM, Bryan O'Donoghue wrote:
> On 02/06/2026 22:59, Vladimir Zapolskiy wrote:
>> On 5/23/26 05:48, Bryan O'Donoghue wrote:
>>> Add a base schema initially compatible with x1e80100 to describe MIPI
>>> CSI2
>>> PHY devices.
>>>
>>> The hardware can support both CPHY, DPHY and a special split-mode DPHY.
>>>
>>> The schema here defines three ports:
>>>
>>> port@0:
>>> The first input port where a sensor is always required.
>>>
>>> port@1:
>>> A second optional input port which if present implies DPHY
>>> split-mode.
>>>
>>> port@2:
>>> A third always required output port which connects to the
>>> controller.
>>>
>>
>> This port numeration is imperfect, because port@0 and port@2 are
>> required,
>> while middle port@1 is optional.
>>
>> Like it was stated before a number of times, it seems natural to operate
>> with two ports, where input port may have two endpoints rather than 3
>> ports,
>> also that approach solves the problem of a hole in the port numeration.
>
> Can you confirm this is what you are after ?
>
> port@0 {
> #address-cells = <1>;
> #size-cells = <0>;
>
> endpoint@0 { /* primary sensor */
> reg = <0>;
> data-lanes = <0 1 2 3>;
> remote-endpoint = <&sensor0_out>;
> };
>
> endpoint@1 { /* split-mode second sensor, optional */
> reg = <1>;
> data-lanes = <0>;
> remote-endpoint = <&sensor1_out>;
> };
> };
>
> port@1 { /* output to CAMSS, was port@2 */
> endpoint { remote-endpoint = <&controller_in>; };
> };
>
> This works for me BTW.
Either way, do we need to document the constraint of using port@0 or
endpoint@0 'only' for the 4+1 or 2+1 mode and the other one is for the
1+1 mode? Or is it implicit from this bindings for a developer?>
>>> The CSIPHY devices have their own pinouts on the SoC as well as their
>>> own
>>> individual voltage rails.
>>>
>>> The need to model voltage rails on a per-PHY basis leads us to define
>>> CSIPHY devices as individual nodes.
>>>
>>> Two nice outcomes in terms of schema and DT arise from this change.
>>>
>>> 1. The ability to define on a per-PHY basis voltage rails.
>>> 2. The ability to require those voltage.
>>>
>>> We have had a complete bodge upstream for this where a single set of
>>> voltage rail for all CSIPHYs has been buried inside of CAMSS.
>>>
>>> Much like the I2C bus which is dedicated to Camera sensors - the CCI
>>> bus in
>>> CAMSS parlance, the CSIPHY devices should be individually modelled.
>>>
>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> ---
>>> .../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 209 +++++++++
>>> ++++++++++++
>>> 1 file changed, 209 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-
>>> csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-
>>> csi2-phy.yaml
>>> new file mode 100644
>>> index 0000000000000..270375f949880
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>> @@ -0,0 +1,209 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm CSI2 PHY
>>> +
>>> +maintainers:
>>> + - Bryan O'Donoghue <bod@kernel.org>
>>> +
>>> +description:
>>> + Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2
>>> sensors
>>> + to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and
>>> D-PHY
>>> + modes.
>>> +
>>> +properties:
>>> + compatible:
>>> + const: qcom,x1e80100-csi2-phy
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + "#phy-cells":
>>> + const: 1
>>> + description:
>>> + The single cell specifies the PHY operating mode.
>>
>> #phy-cells should be 0, because the PHY operating mode is well defined
>> by 'bus-type' property of an endpoint on the sensor side, the opposite
>> side of CAMSS/CSID as a CSIPHY "consumer" should not dictate the PHY
>> type.
>
> Rob said consumer but, I'm also not very bothered about that. bus-type
> is perfectly acceptable to me.
>
>>> +
>>> + clocks:
>>> + maxItems: 2
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: core
>>> + - const: timer
>>> +
>>> + interrupts:
>>> + maxItems: 1
>>> +
>>> + operating-points-v2:
>>> + maxItems: 1
>>> +
>>> + power-domains:
>>> + items:
>>> + - description: MMCX voltage rail
>>> + - description: MXC or MXA voltage rail
>>
>> Only "qcom,x1e80100-csi2-phy" device is supported so far, unlikely it's
>> the case that "MXC or MXA voltage rail" should be specified, it'd be
>> just one of two or both.
>
> Hmm. I'm not being clear here if this is your take, I will reword it to
> make it clearer this generation of PHY _must_ have either
>
> - MMCX and MXC
> or
> - MMCX and MXA
I am not sure of this, Bryan. If you look at the PHY core clock
separately, sure, that is correct. But all of them, on this platform as
well, share the RCG, which requires all 3 power domains. So
fundamentally, you need to enable all of those from each PHY. You can
make it constant 3 power domains.>
>>> +
>>> + power-domain-names:
>>> + items:
>>> + - const: mmcx
>>> + - const: mx
>>> +
>>> + vdda-0p9-supply:
>>> + description: Phandle to a 0.9V regulator supply to a PHY.
>>> +
>>> + vdda-1p2-supply:
>>> + description: Phandle to 1.2V regulator supply to a PHY.
>>> +
>>> + ports:
>>> + $ref: /schemas/graph.yaml#/properties/ports
>>> +
>>> + properties:
>>> + port@0:
>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>> + description: Sensor input. Always present.
>>> + unevaluatedProperties: false
>>> +
>>> + properties:
>>> + endpoint:
>>> + $ref: /schemas/media/video-interfaces.yaml#
>>> + unevaluatedProperties: false
>>> + properties:
>>> + data-lanes:
>>> + minItems: 1
>>> + maxItems: 4
>>> + clock-lanes:
>>> + maxItems: 1
>>> + remote-endpoint: true
>>> + required:
>>> + - data-lanes
>>> + - remote-endpoint
>>> +
>>> + port@1:
>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>> + description:
>>> + Second sensor input. When present, indicates DPHY split mode.
>>> + unevaluatedProperties: false
>>> +
>>> + properties:
>>> + endpoint:
>>> + $ref: /schemas/media/video-interfaces.yaml#
>>> + unevaluatedProperties: false
>>> + properties:
>>> + data-lanes:
>>> + maxItems: 1
>>> + clock-lanes:
>>> + maxItems: 1
>>> + remote-endpoint: true
>>> + required:
>>> + - data-lanes
>>> + - clock-lanes
>>> + - remote-endpoint
>>
>> As it's stated above, it should be converted to a single port with two
>> endpoints, it'd be done in accordance to video-interfaces.yaml.
>>
>>> +
>>> + port@2:
>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>> + description: Output to CAMSS controller.
>>> + unevaluatedProperties: false
>>> +
>>> + properties:
>>> + endpoint:
>>> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
>>> + unevaluatedProperties: false
>>> + properties:
>>> + remote-endpoint: true
>>> + required:
>>> + - remote-endpoint
>>> +
>>> + required:
>>> + - port@0
>>> + - port@2
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - "#phy-cells"
>>> + - clocks
>>> + - clock-names
>>> + - interrupts
>>> + - operating-points-v2
>>> + - power-domains
>>> + - power-domain-names
>>> + - vdda-0p9-supply
>>> + - vdda-1p2-supply
>>> + - ports
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> + #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
>>> + #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
>>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>>> +
>>> + csiphy4: csiphy@ace4000 {
>>> + compatible = "qcom,x1e80100-csi2-phy";
>>> + reg = <0x0ace4000 0x2000>;
>>> + #phy-cells = <1>;
>>> +
>>> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
>>> + clock-names = "core",
>>> + "timer";
>>> +
>>> + operating-points-v2 = <&csiphy_opp_table>;
>>> +
>>> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>>> +
>>> + power-domains = <&rpmhpd RPMHPD_MMCX>,
>>> + <&rpmhpd RPMHPD_MX>;
>>> + power-domain-names = "mmcx",
>>> + "mx";
>>> +
>>> + vdda-0p9-supply = <&vreg_l2c_0p8>;
>>> + vdda-1p2-supply = <&vreg_l1c_1p2>;
>>> +
>>> + ports {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + port@0 {
>>> + reg = <0>;
>>> + csiphy0_in_ep: endpoint {
>>> + data-lanes = <0 1>;
>>> + clock-lanes = <2>;
>>> + remote-endpoint = <&sensor_out>;
>>> + };
>>> + };
>>> +
>>> + port@2 {
>>> + reg = <2>;
>>> + csiphy0_out_ep: endpoint {
>>> + remote-endpoint = <&controller_in>;
>>> + };
>>> + };
>>> + };
>>> + };
>>> +
>>> + csiphy_opp_table: opp-table {
>>> + compatible = "operating-points-v2";
>>> +
>>> + opp-300000000 {
>>> + opp-hz = /bits/ 64 <300000000>;
I wonder why you would have only one clock here. You should be setting
the rate for both the core and timer, isn't it?>>> +
required-opps = <&rpmhpd_opp_low_svs_d1>,
>>> + <&rpmhpd_opp_low_svs_d1>;
Same here, it should 3 power domains set.>>> + };
>>> +
>>> + opp-400000000 {
>>> + opp-hz = /bits/ 64 <400000000>;
>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>> + <&rpmhpd_opp_low_svs_d1>;
Why is one at svs and the other at svs_d1? Shouldn't both be svs?>>>
+ };
>>> +
>>> + opp-480000000 {
>>> + opp-hz = /bits/ 64 <480000000>;
>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>> + <&rpmhpd_opp_low_svs_d1>;
And here, both should be svs_l1?>>> + };
>>> + };
>>>
>>
>> --
>> Best wishes,
>> Vladimir
>
>
Thanks,
Vijay.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 3/5] phy: fsl-imx8mq-usb: add runtime PM support
From: Frank Li @ 2026-06-03 18:36 UTC (permalink / raw)
To: Xu Yang
Cc: Vinod Koul, Neil Armstrong, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Jun Li, linux-phy, imx, linux-arm-kernel,
linux-kernel, Xu Yang
In-Reply-To: <20260603-imx8mp-usb-phy-improvement-v3-3-7afb8f89abc6@nxp.com>
On Wed, Jun 03, 2026 at 01:37:16PM +0800, Xu Yang wrote:
> From: Xu Yang <xu.yang_2@nxp.com>
>
> Add runtime PM to ensure the PHY is properly powered and clocked during
> register access, preventing potential system hangs.
>
> It guards register access in the following scenarios:
> - PHY operations: init() and power_on/off() callbacks are guarded by
> phy core
> - Type-C orientation switching when PHY/Controller are suspended which
> needs explicitly care
> - Future PHY control port register regmap debugfs access
>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>
> ---
> Changes in v3:
> - new patch
> ---
> drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 60 ++++++++++++++++++++----------
> 1 file changed, 41 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index 591ddf346061..b0092c34416e 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -9,6 +9,7 @@
> #include <linux/of.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> #include <linux/regulator/consumer.h>
> #include <linux/usb/typec_mux.h>
>
> @@ -136,17 +137,13 @@ static int tca_blk_typec_switch_set(struct typec_switch_dev *sw,
> {
> struct imx8mq_usb_phy *imx_phy = typec_switch_get_drvdata(sw);
> struct tca_blk *tca = imx_phy->tca;
> - int ret;
>
> if (tca->orientation == orientation)
> return 0;
>
> - ret = clk_prepare_enable(imx_phy->clk);
> - if (ret)
> - return ret;
> + guard(pm_runtime_active)(&imx_phy->phy->dev);
use PM_RUNTIME_ACQUIRE macro
>
> tca_blk_orientation_set(tca, orientation);
> - clk_disable_unprepare(imx_phy->clk);
>
> return 0;
> }
> @@ -620,16 +617,6 @@ static int imx8mq_phy_power_on(struct phy *phy)
> if (ret)
> return ret;
>
> - ret = clk_prepare_enable(imx_phy->clk);
> - if (ret)
> - return ret;
> -
> - ret = clk_prepare_enable(imx_phy->alt_clk);
> - if (ret) {
> - clk_disable_unprepare(imx_phy->clk);
> - return ret;
> - }
> -
> /* Disable rx term override */
> value = readl(imx_phy->base + PHY_CTRL6);
> value &= ~PHY_CTRL6_RXTERM_OVERRIDE_SEL;
> @@ -648,8 +635,6 @@ static int imx8mq_phy_power_off(struct phy *phy)
> value |= PHY_CTRL6_RXTERM_OVERRIDE_SEL;
> writel(value, imx_phy->base + PHY_CTRL6);
>
> - clk_disable_unprepare(imx_phy->alt_clk);
> - clk_disable_unprepare(imx_phy->clk);
> regulator_disable(imx_phy->vbus);
>
> return 0;
> @@ -686,6 +671,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct imx8mq_usb_phy *imx_phy;
> const struct phy_ops *phy_ops;
> + int ret;
>
> imx_phy = devm_kzalloc(dev, sizeof(*imx_phy), GFP_KERNEL);
> if (!imx_phy)
> @@ -693,13 +679,13 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, imx_phy);
>
> - imx_phy->clk = devm_clk_get(dev, "phy");
> + imx_phy->clk = devm_clk_get_enabled(dev, "phy");
> if (IS_ERR(imx_phy->clk)) {
> dev_err(dev, "failed to get imx8mq usb phy clock\n");
> return PTR_ERR(imx_phy->clk);
> }
>
> - imx_phy->alt_clk = devm_clk_get_optional(dev, "alt");
> + imx_phy->alt_clk = devm_clk_get_optional_enabled(dev, "alt");
> if (IS_ERR(imx_phy->alt_clk))
> return dev_err_probe(dev, PTR_ERR(imx_phy->alt_clk),
> "Failed to get alt clk\n");
> @@ -708,6 +694,10 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> if (IS_ERR(imx_phy->base))
> return PTR_ERR(imx_phy->base);
>
> + ret = devm_pm_runtime_set_active_enabled(dev);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
> +
you enable runtime pm here, when runtimes suspend
Frank
> phy_ops = of_device_get_match_data(dev);
> if (!phy_ops)
> return -EINVAL;
> @@ -735,11 +725,43 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> return PTR_ERR_OR_ZERO(phy_provider);
> }
>
> +static int imx8mq_usb_phy_runtime_suspend(struct device *dev)
> +{
> + struct imx8mq_usb_phy *imx_phy = dev_get_drvdata(dev);
> +
> + clk_disable_unprepare(imx_phy->alt_clk);
> + clk_disable_unprepare(imx_phy->clk);
> +
> + return 0;
> +}
> +
> +static int imx8mq_usb_phy_runtime_resume(struct device *dev)
> +{
> + struct imx8mq_usb_phy *imx_phy = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = clk_prepare_enable(imx_phy->clk);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(imx_phy->alt_clk);
> + if (ret) {
> + clk_disable_unprepare(imx_phy->clk);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static DEFINE_RUNTIME_DEV_PM_OPS(imx8mq_usb_phy_pm_ops, imx8mq_usb_phy_runtime_suspend,
> + imx8mq_usb_phy_runtime_resume, NULL);
> +
> static struct platform_driver imx8mq_usb_phy_driver = {
> .probe = imx8mq_usb_phy_probe,
> .driver = {
> .name = "imx8mq-usb-phy",
> .of_match_table = imx8mq_usb_phy_of_match,
> + .pm = pm_ptr(&imx8mq_usb_phy_pm_ops),
> .suppress_bind_attrs = true,
> }
> };
>
> --
> 2.34.1
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 2/5] phy: fsl-imx8mq-usb: set usb phy to be wakeup capable
From: Frank Li @ 2026-06-03 18:26 UTC (permalink / raw)
To: Xu Yang
Cc: Vinod Koul, Neil Armstrong, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Jun Li, linux-phy, imx, linux-arm-kernel,
linux-kernel, Xu Yang
In-Reply-To: <20260603-imx8mp-usb-phy-improvement-v3-2-7afb8f89abc6@nxp.com>
On Wed, Jun 03, 2026 at 01:37:15PM +0800, Xu Yang wrote:
> From: Xu Yang <xu.yang_2@nxp.com>
>
> Set PHY wakeup capable because this PHY supports remote wakeup function.
>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Changes in v3:
> - no changes
> Changes in v2:
> - no changes
> ---
> drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index 88b804b2c982..591ddf346061 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -728,6 +728,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> "failed to get tca\n");
>
> imx8m_get_phy_tuning_data(imx_phy);
> + device_set_wakeup_capable(dev, true);
>
> phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>
>
> --
> 2.34.1
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 1/5] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path
From: Frank Li @ 2026-06-03 18:25 UTC (permalink / raw)
To: Xu Yang
Cc: Vinod Koul, Neil Armstrong, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Jun Li, linux-phy, imx, linux-arm-kernel,
linux-kernel, Felix Gu, stable, Xu Yang
In-Reply-To: <20260603-imx8mp-usb-phy-improvement-v3-1-7afb8f89abc6@nxp.com>
On Wed, Jun 03, 2026 at 01:37:14PM +0800, Xu Yang wrote:
> From: Felix Gu <ustc.gu@gmail.com>
>
> If probe fails after imx95_usb_phy_get_tca() succeeds, the typec
> switch leaks because the only cleanup path was in .remove, which
> never runs on probe failure.
>
> Use devm_add_action_or_reset() so the switch is cleaned up on both
> probe failure and driver removal. The .remove callback and
> imx95_usb_phy_put_tca() are no longer needed.
>
> Fixes: b58f0f86fd61 ("phy: fsl-imx8mq-usb: add tca function driver for imx95")
> Cc: stable@vger.kernel.org
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Reviewed-by: Xu Yang <xu.yang_2@nxp.com>
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
Xu yang, if you send out patch, need your s-o-b tag
Frank
>
> ---
> Changes in v3:
> - add R-b tag
> - cc statble
> - drop "sw = data" conversion
> ---
> drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 27 +++++++--------------------
> 1 file changed, 7 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index b05d80e849a1..88b804b2c982 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -173,9 +173,9 @@ static struct typec_switch_dev *tca_blk_get_typec_switch(struct platform_device
> return sw;
> }
>
> -static void tca_blk_put_typec_switch(struct typec_switch_dev *sw)
> +static void tca_blk_put_typec_switch(void *data)
> {
> - typec_switch_unregister(sw);
> + typec_switch_unregister(data);
> }
>
> static void tca_blk_orientation_set(struct tca_blk *tca,
> @@ -248,6 +248,7 @@ static struct tca_blk *imx95_usb_phy_get_tca(struct platform_device *pdev,
> struct device *dev = &pdev->dev;
> struct resource *res;
> struct tca_blk *tca;
> + int ret;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> if (!res)
> @@ -266,17 +267,11 @@ static struct tca_blk *imx95_usb_phy_get_tca(struct platform_device *pdev,
> tca->orientation = TYPEC_ORIENTATION_NORMAL;
> tca->sw = tca_blk_get_typec_switch(pdev, imx_phy);
>
> - return tca;
> -}
> -
> -static void imx95_usb_phy_put_tca(struct imx8mq_usb_phy *imx_phy)
> -{
> - struct tca_blk *tca = imx_phy->tca;
> -
> - if (!tca)
> - return;
> + ret = devm_add_action_or_reset(&pdev->dev, tca_blk_put_typec_switch, tca->sw);
> + if (ret)
> + return ERR_PTR(ret);
>
> - tca_blk_put_typec_switch(tca->sw);
> + return tca;
> }
>
> static u32 phy_tx_vref_tune_from_property(u32 percent)
> @@ -739,16 +734,8 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> return PTR_ERR_OR_ZERO(phy_provider);
> }
>
> -static void imx8mq_usb_phy_remove(struct platform_device *pdev)
> -{
> - struct imx8mq_usb_phy *imx_phy = platform_get_drvdata(pdev);
> -
> - imx95_usb_phy_put_tca(imx_phy);
> -}
> -
> static struct platform_driver imx8mq_usb_phy_driver = {
> .probe = imx8mq_usb_phy_probe,
> - .remove = imx8mq_usb_phy_remove,
> .driver = {
> .name = "imx8mq-usb-phy",
> .of_match_table = imx8mq_usb_phy_of_match,
>
> --
> 2.34.1
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2 2/2] iio: inkern: Use namespaced exports
From: Jonathan Cameron @ 2026-06-03 17:20 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Romain Gantois, MyungJoo Ham, Chanwoo Choi, Guenter Roeck,
Peter Rosin, David Lechner, Nuno Sá, Andy Shevchenko,
Lars-Peter Clausen, Michael Hennerich, Mariel Tinaco, Kevin Tsai,
Linus Walleij, Eugen Hristev, Vinod Koul, Kishon Vijay Abraham I,
Sebastian Reichel, Chen-Yu Tsai, Hans de Goede,
Support Opensource, Paul Cercueil, Iskren Chernev,
Krzysztof Kozlowski, Marek Szyprowski, Matheus Castello,
Saravanan Sekar, Matthias Brugger, AngeloGioacchino Del Regno,
Casey Connolly, Pali Rohár, Orson Zhai, Baolin Wang,
Chunyan Zhang, Amit Kucheria, Thara Gopinath, Rafael J. Wysocki,
Daniel Lezcano, Zhang Rui, Lukasz Luba, Claudiu Beznea,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
Sylwester Nawrocki, Olivier Moysan, Arnaud Pouliquen,
Maxime Coquelin, Alexandre Torgue, Thomas Petazzoni, linux-kernel,
linux-hwmon, linux-iio, linux-input, linux-phy, linux-pm,
linux-mips, linux-mediatek, linux-arm-msm, linux-sound,
linux-stm32, Sebastian Reichel, Andy Shevchenko
In-Reply-To: <acBr-W2ILu9tnMyd@google.com>
On Sun, 22 Mar 2026 15:24:21 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Tue, Dec 09, 2025 at 09:25:56AM +0100, Romain Gantois wrote:
> > Use namespaced exports for IIO consumer API functions.
> >
> > This will make it easier to manage the IIO export surface. Consumer drivers
> > will only be provided access to a specific set of functions, thereby
> > restricting usage of internal IIO functions by other parts of the kernel.
> >
> > This change cannot be split into several parts without breaking
> > bisectability, thus all of the affected drivers are modified at once.
> >
> > Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com> # for power-supply
> > Acked-by: Guenter Roeck <linux@roeck-us.net>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
>
> For input:
>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Thanks.
>
For anyone wondering what happened to this... I forgot to apply this at the
beginning of the cycle and by the time I remembered we had too much queued up
so it would have been messy to do an immutable branch. Anyhow, I plan to
sort this at start of next cycle.
Jonathan
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: phy: qcom,usb-hs-phy: add qcom,vendor-init-seq
From: me @ 2026-06-03 16:09 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Herman van Hazendonk, linux-phy, devicetree, linux-arm-msm,
Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson
In-Reply-To: <ti5p3oreibkae4pl5sn52g3ctv3ufcs6aklwgbterksrxvxrbj@b6scawz2cqmq>
On 2026-06-03 15:57, Dmitry Baryshkov wrote:
> On Wed, Jun 03, 2026 at 07:48:08AM +0200, Herman van Hazendonk wrote:
>> Add an optional "qcom,vendor-init-seq" property carrying raw ULPI
>> (address, value) pairs that are written after PHY reset.
>>
>> Unlike the existing "qcom,init-seq" property, the address field is
>> NOT offset by ULPI_EXT_VENDOR_SPECIFIC, so the new property can
>> reach the standard ULPI vendor register range (0x30-0x3f). MSM8x60-
>> class hardware needs this range to programme pre-emphasis, HS driver
>> slope and CDR auto-reset bits the legacy msm_otg driver used to set
>> via platform data.
>
> Are those register writes specific to the device or to the whole
> platform? In the latter case please extend the driver to write them.
Looking at every MSM8x60 reference kernel I could find (Qualcomm's own
msm8x60 board, HP TouchPad / APQ8060, and some HTC/Saumsung MSM8660
devices), the writes split into two groups:
Platform-level (same across all MSM8x60 hardware):
- reg 0x36 bits 1+2: CDR auto-reset disabled, SE1 gating disabled
- reg 0x32 bits [5:4]: pre-emphasis at 20%
Board-specific:
- reg 0x32 bits [3:0]: HS driver slope — HP TouchPad uses 5, HTC
devices use 1. This clearly depends on board layout (trace length,
connector loading, etc.).
So the platform-level writes should move unconditionally into the driver
behind a match-data flag for the MSM8x60-class compatible, and only the
HS driver slope value belongs in DT.
> Also please note that writing raw register values is generally frowned
> upon in the upstream. It's not easy to follow those and not nice to
> modify. Instead, if some the values are required for the particular
> device, please add corresponding values to the DT using the
> human-oriented values rather than the register values.
Agreed. The old platform-data msm_otg driver already did exactly this
with named fields (hsdrvslope, pemp_level, cdr_autoreset, se1_gating).
I can drop qcom,vendor-init-seq entirely and replace it with a named
property — something like qcom,hs-driver-slope — for the one value that
genuinely varies per board.
> If you have questions regarding any of the bitfields, please ask here,
> we can try finding the data for those.
One thing I haven't been able to nail down: every HTC MSM8660 board also
writes 0x0C (bits 2 and 3) to ULPI register 0x31 via phy_init_seq, but
the HP TouchPad webOS kernel never sets that register through its named
platform fields — and USB works fine on the TouchPad without it. Do you
know what those two bits in 0x31 control? If they are needed on CI 45nm
PHY silicon but not on the APQ8060 PHY variant, I should probably leave
them out of the driver and note why.
I don't have any technical documentation for the S3 SoC family, except
for the legacy kernels (HP, Samsung, HTC) and a bunch of various HP
TouchPad variants.
Let me know what you expect and I'll write up a v2.
Thanks,
Herman
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 phy-next 14/16] dt-bindings: phy: lynx-10g: initial document
From: Conor Dooley @ 2026-06-03 15:50 UTC (permalink / raw)
To: Vladimir Oltean
Cc: sashiko-reviews, robh, vkoul, neil.armstrong, conor+dt, linux-phy,
devicetree, olteanv
In-Reply-To: <20260603133825.l544tnmtn2pqfrzh@skbuf>
[-- Attachment #1.1: Type: text/plain, Size: 2916 bytes --]
On Wed, Jun 03, 2026 at 04:38:25PM +0300, Vladimir Oltean wrote:
> On Wed, Jun 03, 2026 at 01:33:50PM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Medium] The `if` condition for enforcing the `big-endian` property incorrectly matches the `compatible` string array directly against an `enum`, which will always evaluate to false and cause the requirement to silently fail.
> > --
> >
> > commit 8bd3982a89722b20a3686e42d4ab3b9f0711ae11
> > Author: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > dt-bindings: phy: lynx-10g: initial document
> >
> > This commit adds the schema for the 10G Lynx SerDes used on most QorIQ and
> > Layerscape SoCs. It defines the valid compatible strings, PHY cell requirements,
> > and enforces SoC-specific constraints such as lane counts and endianness.
> >
> > > diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > new file mode 100644
> > > index 0000000000000..8cf6027a75b91
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + enum:
> > > + - fsl,ls1046a-serdes1
> > > + - fsl,ls1046a-serdes2
> >
> > [Severity: Medium]
> > Does this conditional correctly enforce the big-endian property?
> >
> > Since the compatible property is a string array in device trees, evaluating it
> > directly against an enum always returns false. Should this use "contains:"
> > with the enum, similar to the preceding if block, so dt_binding_check
> > correctly enforces the requirement?
>
> The tests I did suggest that it does enforce the requirement.
Yeah, sashiko just doesn't understand how this works without "contains".
"contains" is only actually required for validation purposes if there's
fallback compatibles and you're trying to get a partial match.
>
> - "make dt_binding_check" passes with the submitted example
>
> - If I replace "fsl,ls1028a-serdes" from the example with
> "fsl,ls1046a-serdes1" but leave everything else as is (implying:
> little endian), I get:
>
> $ make -j 8 DT_CHECKER_FLAGS=-m dt_binding_check DT_SCHEMA_FILES=phy/fsl,lynx-10g.yaml
> SCHEMA Documentation/devicetree/bindings/processed-schema.json
> CHKDT ../Documentation/devicetree/bindings
> LINT ../Documentation/devicetree/bindings
> DTEX Documentation/devicetree/bindings/phy/fsl,lynx-10g.example.dts
> DTC [C] Documentation/devicetree/bindings/phy/fsl,lynx-10g.example.dtb
> Documentation/devicetree/bindings/phy/fsl,lynx-10g.example.dtb: serdes@1ea0000 (fsl,ls1046a-serdes1): 'big-endian' is a required property
> from schema $id: http://devicetree.org/schemas/phy/fsl,lynx-10g.yaml
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 112 bytes --]
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2 phy-next 13/15] dt-bindings: phy: lynx-10g: initial document
From: Vladimir Oltean @ 2026-06-03 15:39 UTC (permalink / raw)
To: Conor Dooley
Cc: linux-phy, Ioana Ciornei, Vinod Koul, Neil Armstrong,
Tanjeff Moos, linux-kernel, devicetree, Conor Dooley,
Krzysztof Kozlowski, Rob Herring
In-Reply-To: <20260603-bleach-arming-247f1b0fb87b@spud>
On Wed, Jun 03, 2026 at 04:09:32PM +0100, Conor Dooley wrote:
> On Wed, Jun 03, 2026 at 02:56:54PM +0300, Vladimir Oltean wrote:
> > On Tue, Jun 02, 2026 at 06:10:30PM +0100, Conor Dooley wrote:
> > > On Fri, May 29, 2026 at 08:15:07PM +0300, Vladimir Oltean wrote:
> > > > Add a schema for the 10G Lynx SerDes. This is very similar to the modern
> > > > form of the 28G Lynx SerDes, which is very much the intention.
> > > >
> > > > We allow both forms of #phy-cells = <1> in the top-level provider
> > > > and #phy-cells = <0> in the per-lane provider for more flexibility to
> > > > consumers, and because the kernel code is shared with the 28G Lynx which
> > > > already has that support for compatibility reasons.
> > > >
> > > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > > ---
> > > > Cc: devicetree@vger.kernel.org
> > > > Cc: Conor Dooley <conor+dt@kernel.org>
> > > > Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> > > > Cc: Rob Herring <robh@kernel.org>
> > > >
> > > > v1->v2:
> > > > - move patch later in series, right before driver
> > > > - deliberately ignoring this Sashiko feedback:
> > > > https://lore.kernel.org/linux-phy/20260529125017.ifqunh52gdzhthdg@skbuf/
> > > > ---
> > > > .../devicetree/bindings/phy/fsl,lynx-10g.yaml | 131 ++++++++++++++++++
> > > > 1 file changed, 131 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > > new file mode 100644
> > > > index 000000000000..993f076bba4e
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > > @@ -0,0 +1,131 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/phy/fsl,lynx-10g.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Freescale Lynx 10G SerDes PHY
> > > > +
> > > > +maintainers:
> > > > + - Vladimir Oltean <vladimir.oltean@nxp.com>
> > > > +
> > > > +description:
> > > > + The 10G Lynx is a multi-protocol SerDes block which handles networking, PCIe,
> > > > + SATA and other high-speed interfaces. It is present on most QorIQ and
> > > > + Layerscape SoCs. The register map is common, but the integration is
> > > > + SoC-specific, with the differences consisting in register endianness, the
> > > > + number of lanes, protocol converters available per lane and their location in
> > > > + the PCCR registers. Some SoCs have multiple SerDes blocks and those differ in
> > > > + their protocol capabilities per lane.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + description:
> > > > + There is intentionally no generic fsl,lynx-10g compatible string due to
> > > > + the hardware inability to report its capabilities, despite having a
> > > > + common register map.
> > >
> > > I think you can probably drop this from the diff, and put it in the
> > > commit message.
> >
> > Ok.
> >
> > > > + enum:
> > > > + - fsl,ls1028a-serdes
> > > > + - fsl,ls1046a-serdes1
> > > > + - fsl,ls1046a-serdes2
> > > > + - fsl,ls1088a-serdes1
> > > > + - fsl,ls1088a-serdes2
> > > > + - fsl,ls2088a-serdes1
> > > > + - fsl,ls2088a-serdes2
> > > > +
> > > > + reg:
> > > > + maxItems: 1
> > > > +
> > > > + big-endian: true
> > >
> > > This isn't a required property, but should it be made required for the
> > > specific compatibles that are big endian? Or are we not that lucky, and
> > > devices can be either?
> >
> > I can make big-endian a required property for fsl,ls1046a-serdes1 and
> > fsl,ls1046a-serdes2.
>
>
> Please do. In that case,
> pw-bot: changes-requested
>
> Cheers,
> Conor.
I've already sent v3:
https://lore.kernel.org/linux-phy/20260603132059.503527-1-vladimir.oltean@nxp.com/
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2 phy-next 13/15] dt-bindings: phy: lynx-10g: initial document
From: Conor Dooley @ 2026-06-03 15:09 UTC (permalink / raw)
To: Vladimir Oltean
Cc: linux-phy, Ioana Ciornei, Vinod Koul, Neil Armstrong,
Tanjeff Moos, linux-kernel, devicetree, Conor Dooley,
Krzysztof Kozlowski, Rob Herring
In-Reply-To: <20260603115654.k4v7cayysvy7yte4@skbuf>
[-- Attachment #1.1: Type: text/plain, Size: 3722 bytes --]
On Wed, Jun 03, 2026 at 02:56:54PM +0300, Vladimir Oltean wrote:
> On Tue, Jun 02, 2026 at 06:10:30PM +0100, Conor Dooley wrote:
> > On Fri, May 29, 2026 at 08:15:07PM +0300, Vladimir Oltean wrote:
> > > Add a schema for the 10G Lynx SerDes. This is very similar to the modern
> > > form of the 28G Lynx SerDes, which is very much the intention.
> > >
> > > We allow both forms of #phy-cells = <1> in the top-level provider
> > > and #phy-cells = <0> in the per-lane provider for more flexibility to
> > > consumers, and because the kernel code is shared with the 28G Lynx which
> > > already has that support for compatibility reasons.
> > >
> > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > ---
> > > Cc: devicetree@vger.kernel.org
> > > Cc: Conor Dooley <conor+dt@kernel.org>
> > > Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> > > Cc: Rob Herring <robh@kernel.org>
> > >
> > > v1->v2:
> > > - move patch later in series, right before driver
> > > - deliberately ignoring this Sashiko feedback:
> > > https://lore.kernel.org/linux-phy/20260529125017.ifqunh52gdzhthdg@skbuf/
> > > ---
> > > .../devicetree/bindings/phy/fsl,lynx-10g.yaml | 131 ++++++++++++++++++
> > > 1 file changed, 131 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > new file mode 100644
> > > index 000000000000..993f076bba4e
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > @@ -0,0 +1,131 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/phy/fsl,lynx-10g.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Freescale Lynx 10G SerDes PHY
> > > +
> > > +maintainers:
> > > + - Vladimir Oltean <vladimir.oltean@nxp.com>
> > > +
> > > +description:
> > > + The 10G Lynx is a multi-protocol SerDes block which handles networking, PCIe,
> > > + SATA and other high-speed interfaces. It is present on most QorIQ and
> > > + Layerscape SoCs. The register map is common, but the integration is
> > > + SoC-specific, with the differences consisting in register endianness, the
> > > + number of lanes, protocol converters available per lane and their location in
> > > + the PCCR registers. Some SoCs have multiple SerDes blocks and those differ in
> > > + their protocol capabilities per lane.
> > > +
> > > +properties:
> > > + compatible:
> > > + description:
> > > + There is intentionally no generic fsl,lynx-10g compatible string due to
> > > + the hardware inability to report its capabilities, despite having a
> > > + common register map.
> >
> > I think you can probably drop this from the diff, and put it in the
> > commit message.
>
> Ok.
>
> > > + enum:
> > > + - fsl,ls1028a-serdes
> > > + - fsl,ls1046a-serdes1
> > > + - fsl,ls1046a-serdes2
> > > + - fsl,ls1088a-serdes1
> > > + - fsl,ls1088a-serdes2
> > > + - fsl,ls2088a-serdes1
> > > + - fsl,ls2088a-serdes2
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + big-endian: true
> >
> > This isn't a required property, but should it be made required for the
> > specific compatibles that are big endian? Or are we not that lucky, and
> > devices can be either?
>
> I can make big-endian a required property for fsl,ls1046a-serdes1 and
> fsl,ls1046a-serdes2.
Please do. In that case,
pw-bot: changes-requested
Cheers,
Conor.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 112 bytes --]
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ 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