From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7C92231DD86 for ; Fri, 5 Dec 2025 10:38:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764931140; cv=none; b=Ir/tsmSdr08j9AjIToLaMGRthOGndNWzquMgIgUNYSULCeOfrvUkjOgK+WrGD1bjblIn+K8sAcFWCOKuXgf2aNaT2O6qnIbQS0eiSZWHt7nRxPIWV9O4S/UMZb8T8HAe3GUe00TeXIwBW1vcWWPtxiOS3xOvH+PcF20uXNTVPLY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764931140; c=relaxed/simple; bh=7Ti0/hGGkcg6vgg8ymr8rwSgVKCe99zuGiU54ocDWAY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=mrCIh66t8a/I28+xYXV3Xv79thzpoaa/woJbshDjXQtZEYtz9yvIGUd6T00YJ2t/H/b8WMkOlj9mkBG/TTk7LVL0M6PFESfm07mh35MkWu7EX9DCvi3h6CdF4KHWq5+/ErRvK+T46zJmPegCu74qUjTqRFKqYQDHQzDImphrnwI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=wIDmaRxL; arc=none smtp.client-ip=209.85.128.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="wIDmaRxL" Received: by mail-wm1-f49.google.com with SMTP id 5b1f17b1804b1-47118259fd8so18144685e9.3 for ; Fri, 05 Dec 2025 02:38:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1764931126; x=1765535926; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=fk2Qe6UttCBY6LSv0m45hYHnxidbxniuJXbEECMv7UY=; b=wIDmaRxLRmZmtfvpWC8v9C8oFKrswPcglDTp0Uj0bJbjhlYvIIAGm6yncpWdAS9txT 4/g1IVxSDaXapSvcKOHxPScqV4nGRmfFyOeZq6Ux7+SUPlyK3/i9q+vj/tMzDLxpG3kI Fn7yBexlS+sURVrzqwB/jfTBB2fomGqj5P+Kny+809oscQe8vlymxDjN2/tyXYzFMvYX u0Nkc16YDYdYL2OO3s7NlQrVzZ34DZT7UXJTOUDzLMxkEX2ToHmniNJFObvRgMSwFrrR y4oBS3NYRnlHEMjujPr7I4DoTyBN6iqYOy8rPGVGvpm/kT6W8qD+iW2dd1MEGTNV5yr8 TBxw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764931126; x=1765535926; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=fk2Qe6UttCBY6LSv0m45hYHnxidbxniuJXbEECMv7UY=; b=RhZ9839vLj2mSSSuwr1MBaRAvCFRR7sI8bLBMIzpxmQMRqEdNl/cPglKKf8/8cn8cj AIjTYzYzU+7eNNBmufV8qQHfhuSTfUYiFCzopHg9+UnSofV73io/vktMcwmDEtniC1kB dU1LWhzjofFfDcHOS4l97cp5Cezz21Qke/Bh8DEZ4mXB6/KCq2QFmTfPNw2/8JdAWel/ sdb20IVTrwYVlLN4Vr+iXNaFP9rOe+QtBWRQvdDS+LvTLIMiNm0jIemS4UVCYkNFKZfH BzMIn/F4ZO6FXwAPJfjrNRt3jHUtUzvn0hTchxTfJiSXIpl+3UTw+/ktSR1Kn2FPKgI0 Gc8g== X-Forwarded-Encrypted: i=1; AJvYcCVUqw3aWVujZQUK2cSz9FxKWyYtfRC6MUaRiQs/54+zZyFi4knnuP1nyvHOrP5oow3THFjTjAoBtPmgHhM=@vger.kernel.org X-Gm-Message-State: AOJu0Yznu5bPzxPVCQs92zuM7BbqqRpLoNNFBtfjJCd/KuYwXDd7Nrui 8M7rXE+OiVLFcJGpysaDO3KmB0YZD4Yu3Wt0q3+nl8XLOvSGkqrEHPCMPE+TZJZ5krU= X-Gm-Gg: ASbGncuiJ3+q77DgfacVqabBRON65DwZiy+uMIe/Y/wtE3bSQrxJkcqw2RnIDpKRUZm k2GsYt3lE7VoTb5ZB0d1s2vol3sHQA6u9r9So37JOiEupZ0GFV56hCtIvRb7g1e5ZXIUujEU2vv FCnJUoH9JSWkw6r+H3RWs8nacJx9S2iuWUGWgml3/TpF6bcAtyAAZC54Fab0Eqtp5Mr7ycRnJ97 dzSu5XHfCi2gcdTMfprjPHZMHjJZqt95UdplduomypVyAF/6oSFJvBovwg4Z+agn5K66TWBKT0Y ENYiHBE1K7c+QJfvDKgkfaKzSDYoMqAGmVHsbgcVny6absouZZxc54H7EduQIsvT/RtMEmYg1XU 15K38biBZJMCwuo4rgPpxQkg7s7ecxggWHAz5O8LYXkI/ch97Y7+adDiIkG/O1exQhW46Akeo6Q xy/jhWI/VGBPIZ60wo9hENKWpiefxFzIzEcGUE0wJ1MpO9rWwvcq2uhZOa9qI96rY= X-Google-Smtp-Source: AGHT+IEniZY9x6ScJP/72sFnDswrZrFy73p5tN4uqWyYm+lhn3NmJsuyIGi2aQ1n3YuJ/2WMhpQq1Q== X-Received: by 2002:a05:600c:35ce:b0:479:2651:3f9c with SMTP id 5b1f17b1804b1-4792f268a6bmr70857545e9.14.1764931126466; Fri, 05 Dec 2025 02:38:46 -0800 (PST) Received: from [192.168.0.40] (188-141-3-146.dynamic.upc.ie. [188.141.3.146]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-479311ed466sm72223685e9.13.2025.12.05.02.38.45 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 05 Dec 2025 02:38:45 -0800 (PST) Message-ID: <418e11df-1e17-46a7-813c-045a31d5c657@linaro.org> Date: Fri, 5 Dec 2025 10:38:44 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH WIP v2 7/8] media: qcom: camss: csiphy-3ph: C-PHY needs own lane configuration To: david@ixit.cz, Robert Foss , Todor Tomov , Vladimir Zapolskiy , Mauro Carvalho Chehab , Luca Weiss , Petr Hodina , Casey Connolly , "Dr. Git" Cc: Joel Selvaraj , Kieran Bingham , Sakari Ailus , linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, phone-devel@vger.kernel.org References: <20251204-qcom-cphy-v2-0-6b35ef8b071e@ixit.cz> <20251204-qcom-cphy-v2-7-6b35ef8b071e@ixit.cz> From: Bryan O'Donoghue Content-Language: en-US In-Reply-To: <20251204-qcom-cphy-v2-7-6b35ef8b071e@ixit.cz> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 04/12/2025 16:32, David Heidelberg via B4 Relay wrote: > From: David Heidelberg > > Make sure we have proper lane registers definition in-place for each > generation, otherwise C-PHY won't work. > > Signed-off-by: David Heidelberg > --- > .../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 48 ++++++++++++++++------ > 1 file changed, 36 insertions(+), 12 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 6d6dd54c5ac9c..c957f7dbfb243 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 > @@ -1222,8 +1222,12 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy, > } > break; > case CAMSS_2290: > - regs->lane_regs = &lane_regs_qcm2290[0]; > - regs->lane_array_size = ARRAY_SIZE(lane_regs_qcm2290); > + if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) { > + dev_err(dev, "Missing lane_regs definition for %d\n", c->phy_cfg); Is this error possible ? If so we want to trap it earlier and throw a meaningful error up the callstack to NAK the configuration. i.e. you could make this function return and int and throw the error upwards. But is there no opportunity to trap this error before this point ? Either way just printing out something like "critical error can't continue" without returning logical result codes to that effect is wrong. > + } else { /* V4L2_MBUS_CSI2_DPHY */ > + regs->lane_regs = &lane_regs_qcm2290[0]; > + regs->lane_array_size = ARRAY_SIZE(lane_regs_qcm2290); > + } > break; > case CAMSS_7280: > case CAMSS_8250: > @@ -1236,25 +1240,45 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy, > } > break; > case CAMSS_8280XP: > - regs->lane_regs = &lane_regs_sc8280xp[0]; > - regs->lane_array_size = ARRAY_SIZE(lane_regs_sc8280xp); > + if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) { > + dev_err(dev, "Missing lane_regs definition for %d\n", c->phy_cfg); > + } else { /* V4L2_MBUS_CSI2_DPHY */ > + 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); > + if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) { > + dev_err(dev, "Missing lane_regs definition for %d\n", c->phy_cfg); > + } else { /* V4L2_MBUS_CSI2_DPHY */ > + regs->lane_regs = &lane_regs_x1e80100[0]; > + regs->lane_array_size = ARRAY_SIZE(lane_regs_x1e80100); > + } > break; > case CAMSS_8550: > - regs->lane_regs = &lane_regs_sm8550[0]; > - regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8550); > + if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) { > + dev_err(dev, "Missing lane_regs definition for %d\n", c->phy_cfg); > + } else { /* V4L2_MBUS_CSI2_DPHY */ > + regs->lane_regs = &lane_regs_sm8550[0]; > + regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8550); > + } > break; > case CAMSS_8650: > - regs->lane_regs = &lane_regs_sm8650[0]; > - regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8650); > + if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) { > + dev_err(dev, "Missing lane_regs definition for %d\n", c->phy_cfg); > + } else { /* V4L2_MBUS_CSI2_DPHY */ > + regs->lane_regs = &lane_regs_sm8650[0]; > + regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8650); > + } > break; > case CAMSS_8300: > case CAMSS_8775P: > - regs->lane_regs = &lane_regs_sa8775p[0]; > - regs->lane_array_size = ARRAY_SIZE(lane_regs_sa8775p); > + if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) { > + dev_err(dev, "Missing lane_regs definition for %d\n", c->phy_cfg); You could trap this up the top of the function. Make this function return and int. Or better yet find this error earlier ? Anyway rather than replicate the if (cphy) { do stuff } in each case of the switch, just move the error handling to the top of the function once and return a real error. > + } else { /* V4L2_MBUS_CSI2_DPHY */ > + regs->lane_regs = &lane_regs_sa8775p[0]; > + regs->lane_array_size = ARRAY_SIZE(lane_regs_sa8775p); > + } > break; > default: > break; > --- bod