On Fri, 2021-12-24 at 11:10 +0100, AngeloGioacchino Del Regno wrote: > Il 18/12/21 09:28, Chunfeng Yun ha scritto: > > Add three helpers mtk_phy_clear/set/update_bits() for registers > > operation > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@xxxxxxxxxxxx> > > --- > > v2: new patch, add register access helpers, > > Add updatel() macro suggested by Vinod, here add more ones > > instead. > > --- > > drivers/phy/mediatek/phy-mtk-io.h | 38 > > +++++++++++++++++++++++++++++++ > > 1 file changed, 38 insertions(+) > > create mode 100644 drivers/phy/mediatek/phy-mtk-io.h > > > > diff --git a/drivers/phy/mediatek/phy-mtk-io.h > > b/drivers/phy/mediatek/phy-mtk-io.h > > new file mode 100644 > > index 000000000000..500fcdab165d > > --- /dev/null > > +++ b/drivers/phy/mediatek/phy-mtk-io.h > > @@ -0,0 +1,38 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Copyright (C) 2021 MediaTek Inc. > > + * > > + * Author: Chunfeng Yun <chunfeng.yun@xxxxxxxxxxxx> > > + */ > > + > > +#ifndef __PHY_MTK_H__ > > +#define __PHY_MTK_H__ > > + > > +#include <linux/io.h> > > + > > +static inline void mtk_phy_clear_bits(void __iomem *reg, u32 bits) > > +{ > > + u32 tmp = readl(reg); > > + > > + tmp &= ~bits; > > + writel(tmp, reg); > > +} > > + > > +static inline void mtk_phy_set_bits(void __iomem *reg, u32 bits) > > +{ > > + u32 tmp = readl(reg); > > + > > + tmp |= bits; > > + writel(tmp, reg); > > +} > > + > > +static inline void mtk_phy_update_bits(void __iomem *reg, u32 > > mask, u32 val) > > +{ > > + u32 tmp = readl(reg); > > + > > + tmp &= ~mask; > > + tmp |= val & mask; > > + writel(tmp, reg); > > +} > > + > > +#endif > > > > These helpers are almost exactly duplicating what > regmap_update_bits() is doing. > I appreciate the effort to stop open-coding the same sequences over > and over by > adding such helper functions, I agree with you. > but I think that the proper way of doing what you > are proposing is not to add custom functions but rather reuse what > the Linux APIs > give you. I also like to use common APIs ASAP, but not found suitable ones. This may be a problem, I found that some similar custom helps already added under phy fold. > > What about doing a conversion to use regmap on this driver? No, we don't use regmap here, these registers are monopolized by t-phy, it's not syscon.