| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <53ae7338-e5db-40dd-9fac-f06a97584cef@oss.qualcomm.com>
Date: Wed, 15 Oct 2025 11:41:02 +0800
From: Hangxiang Ma <hangxiang.ma@....qualcomm.com>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
Jingyi Wang <jingyi.wang@....qualcomm.com>,
Loic Poulain <loic.poulain@....qualcomm.com>,
Robert Foss
<rfoss@...nel.org>, Andi Shyti <andi.shyti@...nel.org>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Todor Tomov <todor.too@...il.com>,
Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: linux-i2c@...r.kernel.org, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, aiqun.yu@....qualcomm.com,
tingwei.zhang@....qualcomm.com, trilok.soni@....qualcomm.com,
yijie.yang@....qualcomm.com
Subject: Re: [PATCH 4/6] media: qcom: camss: csiphy: Add support for v2.4.0
two-phase CSIPHY
On 9/25/2025 8:57 PM, Bryan O'Donoghue wrote:
> On 25/09/2025 01:02, Jingyi Wang wrote:
>> From: Hangxiang Ma <hangxiang.ma@....qualcomm.com>
>>
>> Add more detailed resource information for CSIPHY devices in the camss
>> driver along with the support for v2.4.0 in the 2 phase CSIPHY driver
>> that is responsible for the PHY lane register configuration, module
>> reset and interrupt handling.
>>
>> This change adds 'cmn_status_offset' variable in 'csidphy_device_regs'
>> structure. It helps adapt the offset to the common status registers that
>> is different in v2.4.0 from others.
>>
>> Signed-off-by: Hangxiang Ma <hangxiang.ma@....qualcomm.com>
>> Signed-off-by: Jingyi Wang <jingyi.wang@....qualcomm.com>
>> ---
>> .../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 138
>> ++++++++++++++++++++-
>> drivers/media/platform/qcom/camss/camss-csiphy.h | 1 +
>> drivers/media/platform/qcom/camss/camss.c | 107
>> ++++++++++++++++
>> 3 files changed, 240 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>> b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>> index a229ba04b158..ecb91d3688ca 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>> @@ -46,7 +46,7 @@
>> #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_STATUSn(offset, n) ((offset) +
>> 0xb0 + 0x4 * (n))
>> +#define CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(offset, bias, n) ((offset)
>> + (bias) + 0x4 * (n))
>
> You need to explain this bias parameter in the commit log.
Ack. Now rename the 'bias' parameter the same as 'common_status_offset'
to remove ambiguity.
>>
>> #define CSIPHY_DEFAULT_PARAMS 0
>> #define CSIPHY_LANE_ENABLE 1
>> @@ -587,6 +587,123 @@ csiphy_lane_regs lane_regs_sm8550[] = {
>> {0x0C64, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> };
>>
>> +/* GEN2 2.4.0 2PH DPHY mode */
>
> You need to call out the process node in this comment, per the other
> recent additions.
Ack
>> +static const struct
>> +csiphy_lane_regs lane_regs_kaanapali[] = {
>> + /* LN 0 */
>> + {0x0094, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x00A0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0090, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0098, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0094, 0x07, 0xd1, CSIPHY_DEFAULT_PARAMS},
>> + {0x0030, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0000, 0x8C, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0038, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x002C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0034, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x001C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0014, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x003C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0004, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0020, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0008, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
>> + {0x0010, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0094, 0xD7, 0x00, CSIPHY_SKEW_CAL},
>> + {0x005C, 0x54, 0x00, CSIPHY_SKEW_CAL},
>> + {0x0060, 0xFD, 0x00, CSIPHY_SKEW_CAL},
>> + {0x0064, 0x7F, 0x00, CSIPHY_SKEW_CAL},
>> +
>> + /* LN 2 */
>> + {0x0494, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x04A0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0490, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0498, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0494, 0x07, 0xd1, CSIPHY_DEFAULT_PARAMS},
>> + {0x0430, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0400, 0x8C, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0438, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x042C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0434, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x041C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0414, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x043C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0404, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0420, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0408, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
>> + {0x0410, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0494, 0xD7, 0x00, CSIPHY_SKEW_CAL},
>> + {0x045C, 0x54, 0x00, CSIPHY_SKEW_CAL},
>> + {0x0460, 0xFD, 0x00, CSIPHY_SKEW_CAL},
>> + {0x0464, 0x7F, 0x00, CSIPHY_SKEW_CAL},
>> +
>> + /* LN 4 */
>> + {0x0894, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x08A0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0890, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0898, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0894, 0x07, 0xd1, CSIPHY_DEFAULT_PARAMS},
>> + {0x0830, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0800, 0x8C, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0838, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x082C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0834, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x081C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0814, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x083C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0804, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0820, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0808, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
>> + {0x0810, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0894, 0xD7, 0x00, CSIPHY_SKEW_CAL},
>> + {0x085C, 0x54, 0x00, CSIPHY_SKEW_CAL},
>> + {0x0860, 0xFD, 0x00, CSIPHY_SKEW_CAL},
>> + {0x0864, 0x7F, 0x00, CSIPHY_SKEW_CAL},
>> +
>> + /* LN 6 */
>> + {0x0C94, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0CA0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C90, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C98, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C94, 0x07, 0xd1, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C30, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C00, 0x8C, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C38, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C2C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C34, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C1C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C14, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C3C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C04, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C20, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C08, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
>> + {0x0C10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0C94, 0xD7, 0x00, CSIPHY_SKEW_CAL},
>> + {0x0C5C, 0x54, 0x00, CSIPHY_SKEW_CAL},
>> + {0x0C60, 0xFD, 0x00, CSIPHY_SKEW_CAL},
>> + {0x0C64, 0x7F, 0x00, CSIPHY_SKEW_CAL},
>> +
>> + /* LN CLK */
>> + {0x0E94, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0EA0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E90, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E98, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E94, 0x07, 0xd1, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E30, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E28, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E00, 0x80, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E0C, 0xFF, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E38, 0x1F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E2C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E34, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E1C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E14, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E3C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E04, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E20, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> + {0x0E08, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
>> + {0x0E10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +};
>> +
>> /* 4nm 2PH v 2.1.2 2p5Gbps 4 lane DPHY mode */
>> static const struct
>> csiphy_lane_regs lane_regs_x1e80100[] = {
>> @@ -714,13 +831,13 @@ static void csiphy_hw_version_read(struct
>> csiphy_device *csiphy,
>> CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 6));
>>
>> hw_version = readl_relaxed(csiphy->base +
>> - CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, 12));
>> + CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset,
>> regs->cmn_status_offset, 12));
>> hw_version |= readl_relaxed(csiphy->base +
>> - CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, 13)) << 8;
>> + CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset,
>> regs->cmn_status_offset, 13)) << 8;
>> hw_version |= readl_relaxed(csiphy->base +
>> - CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, 14)) << 16;
>> + CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset,
>> regs->cmn_status_offset, 14)) << 16;
>> hw_version |= readl_relaxed(csiphy->base +
>> - CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, 15)) << 24;
>> + CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset,
>> regs->cmn_status_offset, 15)) << 24;
>>
>> dev_dbg(dev, "CSIPHY 3PH HW Version = 0x%08x\n", hw_version);
>> }
>> @@ -749,7 +866,8 @@ static irqreturn_t csiphy_isr(int irq, void *dev)
>> for (i = 0; i < 11; i++) {
>> int c = i + 22;
>> u8 val = readl_relaxed(csiphy->base +
>> - CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, i));
>> + CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset,
>> + regs->cmn_status_offset, i));
>>
>> writel_relaxed(val, csiphy->base +
>> CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, c));
>> @@ -915,6 +1033,7 @@ static bool csiphy_is_gen2(u32 version)
>> case CAMSS_845:
>> case CAMSS_8550:
>> case CAMSS_8775P:
>> + case CAMSS_KAANAPALI:
>> case CAMSS_X1E80100:
>> ret = true;
>> break;
>> @@ -989,6 +1108,7 @@ static int csiphy_init(struct csiphy_device
>> *csiphy)
>>
>> csiphy->regs = regs;
>> regs->offset = 0x800;
>> + regs->cmn_status_offset = 0xb0;
>>
>> switch (csiphy->camss->res->version) {
>> case CAMSS_845:
>> @@ -1023,6 +1143,12 @@ static int csiphy_init(struct csiphy_device
>> *csiphy)
>> regs->lane_regs = &lane_regs_sa8775p[0];
>> regs->lane_array_size = ARRAY_SIZE(lane_regs_sa8775p);
>> break;
>> + case CAMSS_KAANAPALI:
>> + regs->lane_regs = &lane_regs_kaanapali[0];
>> + regs->lane_array_size = ARRAY_SIZE(lane_regs_kaanapali);
>> + regs->offset = 0x1000;
>> + regs->cmn_status_offset = 0x138;
>
> I don't think a second offset is warranted
>
> You could acheive the required offset with offset = 0x1138; and a
> comment.
>
> Perhaps I'm not seeing it but seems like an additional - fixed - fluff
> variable.
Necessary to add another variable here. The 'offset' parameter denotes
the address offset between base and the common register. But the
'common_status_offset' denotes the offset between common registers and
status registers.
Powered by blists - more mailing lists