Re: [PATCH WIP v5 4/9] media: qcom: camss: Initialize lanes after lane configuration is available

[David Heidelberg <david@ixit.cz>]

On 01/06/2026 11:00, Sakari Ailus wrote:
> Hi David,
> 
> On Sun, May 31, 2026 at 03:08:12PM +0200, David Heidelberg via B4 Relay wrote:
>> From: David Heidelberg <david@ixit.cz>
>>
>> The lanes must not be initialized before the driver has access to
>> the lane configuration, as it depends on whether D-PHY or C-PHY mode
>> is in use. Move the lane initialization to csiphy_lanes_enable which is
>> called when the configuration structures are available.
>>
>> Co-developed-by: Petr Hodina <phodina@protonmail.com>
>> Signed-off-by: Petr Hodina <phodina@protonmail.com>
>> Signed-off-by: David Heidelberg <david@ixit.cz>
>> ---
>>   .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 133 +++++++++++++++------
>>   1 file changed, 95 insertions(+), 38 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 d4624417a7424..8bcba6107471f 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
>> @@ -1164,16 +1164,110 @@ static int csiphy_lanes_enable(struct csiphy_device *csiphy,
>>   {
>>   	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;
>>   
>> +	switch (csiphy->camss->res->version) {
>> +	case CAMSS_845:
>> +		if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
>> +			regs->lane_regs = NULL;
>> +			regs->lane_array_size = 0;
> 
> Aren't these fields already initialised to 0?

I assume yes, but I wanted to be more explicit here so it's more obvious that 
C-PHY sequences are missing (until these get filled in by users).

> 
> Using the compatible string to assign these using device matching would be
> nice if possible.

Sure, thou if not explicitly requested, I would prefer to do minimal changes 
above the C-PHY itself introduction (it's pretty much breaking with every CAMSS 
version addition). I can do it as follow up, if wanted.

David

> 
>> +		} else {
>> +			regs->lane_regs = &lane_regs_sdm845[0];
>> +			regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
>> +		}
>> +		break;
>> +	case CAMSS_2290:
>> +	case CAMSS_6150:
>> +		if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
>> +			regs->lane_regs = NULL;
>> +			regs->lane_array_size = 0;
>> +		} else {
>> +			regs->lane_regs = &lane_regs_qcm2290[0];
>> +			regs->lane_array_size = ARRAY_SIZE(lane_regs_qcm2290);
>> +		}
>> +		break;
>> +	case CAMSS_6350:
>> +		if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
>> +			regs->lane_regs = NULL;
>> +			regs->lane_array_size = 0;
>> +		} else {
>> +			regs->lane_regs = &lane_regs_sm6350[0];
>> +			regs->lane_array_size = ARRAY_SIZE(lane_regs_sm6350);
>> +		}
>> +		break;
>> +	case CAMSS_7280:
>> +	case CAMSS_8250:
>> +		if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
>> +			regs->lane_regs = NULL;
>> +			regs->lane_array_size = 0;
>> +		} else {
>> +			regs->lane_regs = &lane_regs_sm8250[0];
>> +			regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8250);
>> +		}
>> +		break;
>> +	case CAMSS_8280XP:
>> +		if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
>> +			regs->lane_regs = NULL;
>> +			regs->lane_array_size = 0;
>> +		} else {
>> +			regs->lane_regs = &lane_regs_sc8280xp[0];
>> +			regs->lane_array_size = ARRAY_SIZE(lane_regs_sc8280xp);
>> +		}
>> +		break;
>> +	case CAMSS_X1E80100:
>> +		if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
>> +			regs->lane_regs = NULL;
>> +			regs->lane_array_size = 0;
>> +		} else {
>> +			regs->lane_regs = &lane_regs_x1e80100[0];
>> +			regs->lane_array_size = ARRAY_SIZE(lane_regs_x1e80100);
>> +		}
>> +		break;
>> +	case CAMSS_8550:
>> +		if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
>> +			regs->lane_regs = NULL;
>> +			regs->lane_array_size = 0;
>> +		} else {
>> +			regs->lane_regs = &lane_regs_sm8550[0];
>> +			regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8550);
>> +		}
>> +		break;
>> +	case CAMSS_8650:
>> +		if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
>> +			regs->lane_regs = NULL;
>> +			regs->lane_array_size = 0;
>> +		} else {
>> +			regs->lane_regs = &lane_regs_sm8650[0];
>> +			regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8650);
>> +		}
>> +		break;
>> +	case CAMSS_8300:
>> +	case CAMSS_8775P:
>> +		if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
>> +			regs->lane_regs = NULL;
>> +			regs->lane_array_size = 0;
>> +		} else {
>> +			regs->lane_regs = &lane_regs_sa8775p[0];
>> +			regs->lane_array_size = ARRAY_SIZE(lane_regs_sa8775p);
>> +		}
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	if (!regs->lane_regs && c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
>> +		dev_err(dev, "Missing lane_regs definition for C-PHY\n");
>> +		return -EINVAL;
>> +	}
>> +
>>   	settle_cnt = csiphy_settle_cnt_calc(link_freq, csiphy->timer_clk_rate);
>>   
>>   	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);
>> @@ -1235,63 +1329,26 @@ static int csiphy_init(struct csiphy_device *csiphy)
>>   	struct device *dev = csiphy->camss->dev;
>>   	struct csiphy_device_regs *regs;
>>   
>>   	regs = devm_kmalloc(dev, sizeof(*regs), GFP_KERNEL);
>>   	if (!regs)
>>   		return -ENOMEM;
>>   
>>   	csiphy->regs = regs;
>> -	regs->offset = 0x800;
>>   	regs->common_status_offset = 0xb0;
>>   
>>   	switch (csiphy->camss->res->version) {
>> -	case CAMSS_845:
>> -		regs->lane_regs = &lane_regs_sdm845[0];
>> -		regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
>> -		break;
>> -	case CAMSS_2290:
>> -	case CAMSS_6150:
>> -		regs->lane_regs = &lane_regs_qcm2290[0];
>> -		regs->lane_array_size = ARRAY_SIZE(lane_regs_qcm2290);
>> -		break;
>> -	case CAMSS_6350:
>> -		regs->lane_regs = &lane_regs_sm6350[0];
>> -		regs->lane_array_size = ARRAY_SIZE(lane_regs_sm6350);
>> -		break;
>> -	case CAMSS_7280:
>> -	case CAMSS_8250:
>> -		regs->lane_regs = &lane_regs_sm8250[0];
>> -		regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8250);
>> -		break;
>> -	case CAMSS_8280XP:
>> -		regs->lane_regs = &lane_regs_sc8280xp[0];
>> -		regs->lane_array_size = ARRAY_SIZE(lane_regs_sc8280xp);
>> -		break;
>>   	case CAMSS_X1E80100:
>> -		regs->lane_regs = &lane_regs_x1e80100[0];
>> -		regs->lane_array_size = ARRAY_SIZE(lane_regs_x1e80100);
>> -		regs->offset = 0x1000;
>> -		break;
>>   	case CAMSS_8550:
>> -		regs->lane_regs = &lane_regs_sm8550[0];
>> -		regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8550);
>> -		regs->offset = 0x1000;
>> -		break;
>>   	case CAMSS_8650:
>> -		regs->lane_regs = &lane_regs_sm8650[0];
>> -		regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8650);
>>   		regs->offset = 0x1000;
>>   		break;
>> -	case CAMSS_8300:
>> -	case CAMSS_8775P:
>> -		regs->lane_regs = &lane_regs_sa8775p[0];
>> -		regs->lane_array_size = ARRAY_SIZE(lane_regs_sa8775p);
>> -		break;
>>   	default:
>> +		regs->offset = 0x800;
>>   		break;
>>   	}
>>   
>>   	return 0;
>>   }
>>   
>>   const struct csiphy_hw_ops csiphy_ops_3ph_1_0 = {
>>   	.get_lane_mask = csiphy_get_lane_mask,
>>
>> -- 
>> 2.53.0
>>
>>
> 

-- 
David Heidelberg