| Message ID | 20230822161620.1915110-9-bryan.odonoghue@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: qcom: camss: Bugfix series | expand |
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); >
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 ? --- bod
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 --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);
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(+)