diff mbox series

[v1,8/9] media: qcom: camss: Fix set CSI2_RX_CFG1_VC_MODE when VC is greater than 3

Message ID 20230822161620.1915110-9-bryan.odonoghue@linaro.org
State Superseded
Headers show
Series media: qcom: camss: Bugfix series | expand

Commit Message

Bryan O'Donoghue Aug. 22, 2023, 4:16 p.m. UTC
VC_MODE = 0 implies a two bit VC address.
VC_MODE = 1 is required for VCs with a larger address than two bits.

Fixes: eebe6d00e9bf ("media: camss: Add support for CSID hardware version Titan 170")
Cc: stable@vger.kernel.org
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss-csid-gen2.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Konrad Dybcio Aug. 22, 2023, 4:32 p.m. UTC | #1
On 22.08.2023 18:16, Bryan O'Donoghue wrote:
> VC_MODE = 0 implies a two bit VC address.
> VC_MODE = 1 is required for VCs with a larger address than two bits.
> 
> Fixes: eebe6d00e9bf ("media: camss: Add support for CSID hardware version Titan 170")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/media/platform/qcom/camss/camss-csid-gen2.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> index 45c7986d4a8d0..140c584bfb8b1 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> @@ -449,6 +449,8 @@ static void __csid_configure_stream(struct csid_device *csid, u8 enable, u8 vc)
>  	writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG0);
>  
>  	val = 1 << CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
> +	if (vc > 3)
I hope you don't pull your hair out, but I think GENMASK(1,0) could be
in order here with a comment about the bitlength requirements

Konrad
> +		val |= 1 << CSI2_RX_CFG1_VC_MODE;
>  	val |= 1 << CSI2_RX_CFG1_MISR_EN;
>  	writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG1);
>
Konrad Dybcio Aug. 22, 2023, 7:03 p.m. UTC | #2
On 22.08.2023 21:03, Bryan O'Donoghue wrote:
> On 22/08/2023 17:32, Konrad Dybcio wrote:
>>>       val = 1 << CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
>>> +    if (vc > 3)
>> I hope you don't pull your hair out, but I think GENMASK(1,0) could be
> 
> 
> I generally dislike the pattern of 1 << value in this code but, it's not something I'm proposing to solve at this time.
> 
>> in order here with a comment about the bitlength requirements
> 
> Not parsing the bitlength requirements comment, whatdoyoumean ?
The thing you mentioned in the commit message, it could also be a
hint in the code above the if-statement.

Konrad
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
index 45c7986d4a8d0..140c584bfb8b1 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
@@ -449,6 +449,8 @@  static void __csid_configure_stream(struct csid_device *csid, u8 enable, u8 vc)
 	writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG0);
 
 	val = 1 << CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
+	if (vc > 3)
+		val |= 1 << CSI2_RX_CFG1_VC_MODE;
 	val |= 1 << CSI2_RX_CFG1_MISR_EN;
 	writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG1);