On 01/06/2026 13:48, David Heidelberg wrote: > On 01/06/2026 10:49, Sakari Ailus wrote: >> Hi David, >> >> Thanks for the patch. >> >> On Sun, May 31, 2026 at 03:08:10PM +0200, David Heidelberg via B4 Relay >> wrote: >>> From: David Heidelberg <[email protected]> >>> >>> So far, only D-PHY mode was supported, which uses even bits when enabling >>> or masking lanes. For C-PHY configuration, the hardware instead requires >>> using the odd bits. >>> >>> Since there can be unrecognized configuration allow returning failure. >>> >>> Reviewed-by: Bryan O'Donoghue <[email protected]> >>> Acked-by: Cory Keitz <[email protected]> >>> Signed-off-by: David Heidelberg <[email protected]> >>> --- >>> .../platform/qcom/camss/camss-csiphy-2ph-1-0.c | 8 ++-- >>> .../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 49 >>> +++++++++++++++++----- >>> drivers/media/platform/qcom/camss/camss-csiphy.c | 5 +-- >>> drivers/media/platform/qcom/camss/camss-csiphy.h | 6 +-- >>> 4 files changed, 48 insertions(+), 20 deletions(-) >>> > > [...] > >>> -static void csiphy_lanes_enable(struct csiphy_device *csiphy, >>> - struct csiphy_config *cfg, >>> - s64 link_freq, u8 lane_mask) >>> +static int csiphy_lanes_enable(struct csiphy_device *csiphy, >>> + struct csiphy_config *cfg, >>> + s64 link_freq, u8 lane_mask) >>> { >>> + struct device *dev = csiphy->camss->dev; >>> struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg; >>> struct csiphy_device_regs *regs = csiphy->regs; >>> u8 settle_cnt; >>> u8 val; >>> int i; >>> >>> settle_cnt = csiphy_settle_cnt_calc(link_freq, csiphy->timer_clk_rate); >>> >>> - val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE; >>> - for (i = 0; i < c->num_data; i++) >>> - val |= BIT(c->data[i].pos * 2); >>> + val = 0; >>> + >>> + switch (c->phy_cfg) { >>> + case V4L2_MBUS_CSI2_CPHY: >>> + for (i = 0; i < c->num_data; i++) >>> + val |= BIT((c->data[i].pos * 2) + 1); >>> + break; >>> + case V4L2_MBUS_CSI2_DPHY: >>> + val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE; >>> + >>> + for (i = 0; i < c->num_data; i++) >>> + val |= BIT(c->data[i].pos * 2); >>> + break; >>> + default: >>> + dev_err(dev, "Unsupported bus type %d\n", c->phy_cfg); >>> + return -EINVAL; >> >> If this is the only reason why you're changing the return type to int, I'd >> suggest using WARN_ON() in this case. I presume it'd take a driver bug for >> this to trigger? > > Thank you for the review. > > If others agree, I would also prefer WARN_ON instead of changing return > values. > > David > > [...] >
In the standalone PHY we validate the PHY type in probe :) WARN_ON() seems fine to me though. Reviewed-by: Bryan O'Donoghue <[email protected]> --- bod
