Messages in this thread | | | Date | Tue, 8 Nov 2022 18:37:19 +0100 | Subject | Re: [PATCH v28 05/11] soc: mediatek: refine code to use mtk_mmsys_update_bits API | From | Matthias Brugger <> |
| |
On 07/11/2022 08:22, Nancy.Lin wrote: > Simplify code for update mmsys reg. > > Signed-off-by: Nancy.Lin <nancy.lin@mediatek.com> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > Reviewed-by: CK Hu <ck.hu@mediatek.com> > Tested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > Tested-by: Bo-Chen Chen <rex-bc.chen@mediatek.com> > Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > --- > drivers/soc/mediatek/mtk-mmsys.c | 45 ++++++++++++-------------------- > 1 file changed, 16 insertions(+), 29 deletions(-) > > diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c > index 9a327eb5d9d7..73c8bd27e6ae 100644 > --- a/drivers/soc/mediatek/mtk-mmsys.c > +++ b/drivers/soc/mediatek/mtk-mmsys.c > @@ -99,22 +99,27 @@ struct mtk_mmsys { > struct reset_controller_dev rcdev; > }; > > +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 & mask);
I'm not sure about the change in the implementation of mtk_mmsys_update_bits(). Nicolas tried to explain it to me on IRC but I wasn't totally convincing. As we have to go for at least another round of this patches, I'd like to get a clear understanding while it is needed that val bits are set to 1 in the mask.
Regards, Matthias
> + writel_relaxed(tmp, mmsys->regs + offset); > +} > + > void mtk_mmsys_ddp_connect(struct device *dev, > enum mtk_ddp_comp_id cur, > enum mtk_ddp_comp_id next) > { > struct mtk_mmsys *mmsys = dev_get_drvdata(dev); > const struct mtk_mmsys_routes *routes = mmsys->data->routes; > - u32 reg; > int i; > > for (i = 0; i < mmsys->data->num_routes; i++) > - if (cur == routes[i].from_comp && next == routes[i].to_comp) { > - reg = readl_relaxed(mmsys->regs + routes[i].addr); > - reg &= ~routes[i].mask; > - reg |= routes[i].val; > - writel_relaxed(reg, mmsys->regs + routes[i].addr); > - } > + if (cur == routes[i].from_comp && next == routes[i].to_comp) > + mtk_mmsys_update_bits(mmsys, routes[i].addr, routes[i].mask, > + routes[i].val); > } > EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_connect); > > @@ -124,27 +129,14 @@ void mtk_mmsys_ddp_disconnect(struct device *dev, > { > struct mtk_mmsys *mmsys = dev_get_drvdata(dev); > const struct mtk_mmsys_routes *routes = mmsys->data->routes; > - u32 reg; > int i; > > for (i = 0; i < mmsys->data->num_routes; i++) > - if (cur == routes[i].from_comp && next == routes[i].to_comp) { > - reg = readl_relaxed(mmsys->regs + routes[i].addr); > - reg &= ~routes[i].mask; > - writel_relaxed(reg, mmsys->regs + routes[i].addr); > - } > + if (cur == routes[i].from_comp && next == routes[i].to_comp) > + mtk_mmsys_update_bits(mmsys, routes[i].addr, routes[i].mask, 0); > } > EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_disconnect); > > -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; > - writel_relaxed(tmp, mmsys->regs + offset); > -} > - > void mtk_mmsys_ddp_dpi_fmt_config(struct device *dev, u32 val) > { > if (val) > @@ -161,18 +153,13 @@ static int mtk_mmsys_reset_update(struct reset_controller_dev *rcdev, unsigned l > { > struct mtk_mmsys *mmsys = container_of(rcdev, struct mtk_mmsys, rcdev); > unsigned long flags; > - u32 reg; > > spin_lock_irqsave(&mmsys->lock, flags); > > - reg = readl_relaxed(mmsys->regs + mmsys->data->sw0_rst_offset); > - > if (assert) > - reg &= ~BIT(id); > + mtk_mmsys_update_bits(mmsys, mmsys->data->sw0_rst_offset, BIT(id), 0); > else > - reg |= BIT(id); > - > - writel_relaxed(reg, mmsys->regs + mmsys->data->sw0_rst_offset); > + mtk_mmsys_update_bits(mmsys, mmsys->data->sw0_rst_offset, BIT(id), BIT(id)); > > spin_unlock_irqrestore(&mmsys->lock, flags); >
| |