The left shift operation followed by a mask with 0xf will
always result in 0. To correctly evaluate the expression for
the bitwise OR operation, use a right shift instead.

Reported by Coverity Scan CID: 1511468

Fixes: 1c66496b1391 ("drm/sprd: add Unisoc's drm mipi dsi&dphy driver")

Reviewed-by: Chunyan Zhang <zhang.lyra@gmail.com>
Signed-off-by: Karan Sanghavi <karansanghvi98@gmail.com>
---
Coverity Scan Message:
CID 1511468: (#1 of 1): Wrong operator used (CONSTANT_EXPRESSION_RESULT)
operator_confusion: (pll->kint << 4) & 15 is always 0 regardless of the 
values of its operands. This occurs as the bitwise second operand of "|"
---
Changes in v2:
- Added the fixes tag
- Link to v1: https://lore.kernel.org/r/20241105-coverity1511468wrongoperator-v1-1-06c7513c3efc@gmail.com
---
 drivers/gpu/drm/sprd/megacores_pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sprd/megacores_pll.c b/drivers/gpu/drm/sprd/megacores_pll.c
index 3091dfdc11e3..43c10a5fc441 100644
--- a/drivers/gpu/drm/sprd/megacores_pll.c
+++ b/drivers/gpu/drm/sprd/megacores_pll.c
@@ -94,7 +94,7 @@ static void dphy_set_pll_reg(struct dphy_pll *pll, struct regmap *regmap)
 	reg_val[3] = pll->vco_band | (pll->sdm_en << 1) | (pll->refin << 2);
 	reg_val[4] = pll->kint >> 12;
 	reg_val[5] = pll->kint >> 4;
-	reg_val[6] = pll->out_sel | ((pll->kint << 4) & 0xf);
+	reg_val[6] = pll->out_sel | ((pll->kint >> 4) & 0xf);
 	reg_val[7] = 1 << 4;
 	reg_val[8] = pll->det_delay;
 

---
base-commit: 81983758430957d9a5cb3333fe324fd70cf63e7e
change-id: 20241105-coverity1511468wrongoperator-20130bcd4240

Best regards,
-- 
Karan Sanghavi <karansanghvi98@gmail.com>

On Fri, Nov 08, 2024 at 05:11:25PM +0000, Karan Sanghavi wrote:
> The left shift operation followed by a mask with 0xf will
> always result in 0. To correctly evaluate the expression for
> the bitwise OR operation, use a right shift instead.
> 
> Reported by Coverity Scan CID: 1511468
> 
> Fixes: 1c66496b1391 ("drm/sprd: add Unisoc's drm mipi dsi&dphy driver")
> 
> Reviewed-by: Chunyan Zhang <zhang.lyra@gmail.com>
> Signed-off-by: Karan Sanghavi <karansanghvi98@gmail.com>

Please drop the empty line between tags.

Also see Documentation/process/stable-kernel-rules.rst

> ---
> Coverity Scan Message:
> CID 1511468: (#1 of 1): Wrong operator used (CONSTANT_EXPRESSION_RESULT)
> operator_confusion: (pll->kint << 4) & 15 is always 0 regardless of the 
> values of its operands. This occurs as the bitwise second operand of "|"

Is there any kind of a public link for the report? Should there be a Closes: tag?

> ---
> Changes in v2:
> - Added the fixes tag
> - Link to v1: https://lore.kernel.org/r/20241105-coverity1511468wrongoperator-v1-1-06c7513c3efc@gmail.com
> ---
>  drivers/gpu/drm/sprd/megacores_pll.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/sprd/megacores_pll.c b/drivers/gpu/drm/sprd/megacores_pll.c
> index 3091dfdc11e3..43c10a5fc441 100644
> --- a/drivers/gpu/drm/sprd/megacores_pll.c
> +++ b/drivers/gpu/drm/sprd/megacores_pll.c
> @@ -94,7 +94,7 @@ static void dphy_set_pll_reg(struct dphy_pll *pll, struct regmap *regmap)
>  	reg_val[3] = pll->vco_band | (pll->sdm_en << 1) | (pll->refin << 2);
>  	reg_val[4] = pll->kint >> 12;
>  	reg_val[5] = pll->kint >> 4;
> -	reg_val[6] = pll->out_sel | ((pll->kint << 4) & 0xf);
> +	reg_val[6] = pll->out_sel | ((pll->kint >> 4) & 0xf);
>  	reg_val[7] = 1 << 4;
>  	reg_val[8] = pll->det_delay;
>  
> 
> ---
> base-commit: 81983758430957d9a5cb3333fe324fd70cf63e7e
> change-id: 20241105-coverity1511468wrongoperator-20130bcd4240
> 
> Best regards,
> -- 
> Karan Sanghavi <karansanghvi98@gmail.com>
> 

-- 
With best wishes
Dmitry

On Sat, Nov 09, 2024 at 09:27:36AM +0200, Dmitry Baryshkov wrote:
> On Fri, Nov 08, 2024 at 05:11:25PM +0000, Karan Sanghavi wrote:
> > The left shift operation followed by a mask with 0xf will
> > always result in 0. To correctly evaluate the expression for
> > the bitwise OR operation, use a right shift instead.
> > 
> > Reported by Coverity Scan CID: 1511468
> > 
> > Fixes: 1c66496b1391 ("drm/sprd: add Unisoc's drm mipi dsi&dphy driver")
> > 
> > Reviewed-by: Chunyan Zhang <zhang.lyra@gmail.com>
> > Signed-off-by: Karan Sanghavi <karansanghvi98@gmail.com>
> 
> Please drop the empty line between tags.
> 
> Also see Documentation/process/stable-kernel-rules.rst
> 
Sure will go through that.
> > ---
> > Coverity Scan Message:
> > CID 1511468: (#1 of 1): Wrong operator used (CONSTANT_EXPRESSION_RESULT)
> > operator_confusion: (pll->kint << 4) & 15 is always 0 regardless of the 
> > values of its operands. This occurs as the bitwise second operand of "|"
> 
> Is there any kind of a public link for the report? Should there be a Closes: tag?
>
https://scan7.scan.coverity.com/#/project-view/51975/11354?selectedIssue=1511468

This is the link for coverity scan report. 

Should I add this link in the Closes tag? 

> > ---
> > Changes in v2:
> > - Added the fixes tag
> > - Link to v1: https://lore.kernel.org/r/20241105-coverity1511468wrongoperator-v1-1-06c7513c3efc@gmail.com
> > ---
> >  drivers/gpu/drm/sprd/megacores_pll.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/sprd/megacores_pll.c b/drivers/gpu/drm/sprd/megacores_pll.c
> > index 3091dfdc11e3..43c10a5fc441 100644
> > --- a/drivers/gpu/drm/sprd/megacores_pll.c
> > +++ b/drivers/gpu/drm/sprd/megacores_pll.c
> > @@ -94,7 +94,7 @@ static void dphy_set_pll_reg(struct dphy_pll *pll, struct regmap *regmap)
> >  	reg_val[3] = pll->vco_band | (pll->sdm_en << 1) | (pll->refin << 2);
> >  	reg_val[4] = pll->kint >> 12;
> >  	reg_val[5] = pll->kint >> 4;
> > -	reg_val[6] = pll->out_sel | ((pll->kint << 4) & 0xf);
> > +	reg_val[6] = pll->out_sel | ((pll->kint >> 4) & 0xf);
> >  	reg_val[7] = 1 << 4;
> >  	reg_val[8] = pll->det_delay;
> >  
> > 
> > ---
> > base-commit: 81983758430957d9a5cb3333fe324fd70cf63e7e
> > change-id: 20241105-coverity1511468wrongoperator-20130bcd4240
> > 
> > Best regards,
> > -- 
> > Karan Sanghavi <karansanghvi98@gmail.com>
> > 
> 
> -- 
> With best wishes
> Dmitry
Thank you,
Karan.