2022-11-03 21:24:23

by Nícolas F. R. A. Prado

Subject: Re: [PATCH v27 05/11] soc: mediatek: refine code to use mtk_mmsys_update_bits API

On Thu, Nov 03, 2022 at 11:25:06AM +0800, Nancy.Lin wrote:
[..]
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
[..]
> +static void mtk_mmsys_update_bits(struct mtk_mmsys *mmsys, u32 offset, u32 mask, u32 val)
> +{
> + u32 tmp;
> +
> + tmp = readl_relaxed(mmsys->regs + offset);
> + tmp = (tmp & ~mask) | val;

You should mask the value before writing to prevent bad values from overwriting
bits outside the mask.

tmp = (tmp & ~mask) | (val & mask);

With that,

Reviewed-by: N?colas F. R. A. Prado <[email protected]>

Thanks,
N?colas

2022-11-04 09:43:48

by Nancy Lin (林欣螢)

Subject: Re: [PATCH v27 05/11] soc: mediatek: refine code to use mtk_mmsys_update_bits API

Dear Nicolas,

Thanks for the review.

On Thu, 2022-11-03 at 16:32 -0400, Nícolas F. R. A. Prado wrote:
> On Thu, Nov 03, 2022 at 11:25:06AM +0800, Nancy.Lin wrote:
> [..]
> > --- a/drivers/soc/mediatek/mtk-mmsys.c
> > +++ b/drivers/soc/mediatek/mtk-mmsys.c
>
> [..]
> > +static void mtk_mmsys_update_bits(struct mtk_mmsys *mmsys, u32
> > offset, u32 mask, u32 val)
> > +{
> > + u32 tmp;
> > +
> > + tmp = readl_relaxed(mmsys->regs + offset);
> > + tmp = (tmp & ~mask) | val;
>
> You should mask the value before writing to prevent bad values from
> overwriting
> bits outside the mask.
>
> tmp = (tmp & ~mask) | (val & mask);
>
> With that,
>
> Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
>
> Thanks,
> Nícolas

OK, I will fix it in the next revision.

Thanks,
Nancy