| Message ID | 20220920090038.15133-2-chunfeng.yun@mediatek.com |
|---|---|
| State | Accepted |
| Commit | 29c07477556eb68a64f0ff53235feb0bd1cf1f63 |
| Headers | show |
| Series | unify register access and macros | expand |
Il 20/09/22 11:00, Chunfeng Yun ha scritto: > Due to FIELD_PREP() macro can be used to prepare a bitfield value, > local ones can be remove; add the new helper to make bitfield update > easier. > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > --- > drivers/phy/mediatek/phy-mtk-io.h | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/phy/mediatek/phy-mtk-io.h b/drivers/phy/mediatek/phy-mtk-io.h > index 500fcdab165d..a723d4afc9b4 100644 > --- a/drivers/phy/mediatek/phy-mtk-io.h > +++ b/drivers/phy/mediatek/phy-mtk-io.h > @@ -8,6 +8,7 @@ > #ifndef __PHY_MTK_H__ > #define __PHY_MTK_H__ > > +#include <linux/bitfield.h> > #include <linux/io.h> > > static inline void mtk_phy_clear_bits(void __iomem *reg, u32 bits) > @@ -35,4 +36,10 @@ static inline void mtk_phy_update_bits(void __iomem *reg, u32 mask, u32 val) > writel(tmp, reg); > } > > +/* field @mask should be constant and continuous */ "Field @mask shall be [...]" ^^^^^ > +static inline void mtk_phy_update_field(void __iomem *reg, u32 mask, u32 val) ...so, (void __iomem *reg, const u32 mask, u32 val) > +{ > + mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); > +} > + > #endif
On Wed, 2022-09-21 at 10:15 +0200, AngeloGioacchino Del Regno wrote: > Il 20/09/22 11:00, Chunfeng Yun ha scritto: > > Due to FIELD_PREP() macro can be used to prepare a bitfield value, > > local ones can be remove; add the new helper to make bitfield > > update > > easier. > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > --- > > drivers/phy/mediatek/phy-mtk-io.h | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/phy/mediatek/phy-mtk-io.h > > b/drivers/phy/mediatek/phy-mtk-io.h > > index 500fcdab165d..a723d4afc9b4 100644 > > --- a/drivers/phy/mediatek/phy-mtk-io.h > > +++ b/drivers/phy/mediatek/phy-mtk-io.h > > @@ -8,6 +8,7 @@ > > #ifndef __PHY_MTK_H__ > > #define __PHY_MTK_H__ > > > > +#include <linux/bitfield.h> > > #include <linux/io.h> > > > > static inline void mtk_phy_clear_bits(void __iomem *reg, u32 > > bits) > > @@ -35,4 +36,10 @@ static inline void mtk_phy_update_bits(void > > __iomem *reg, u32 mask, u32 val) > > writel(tmp, reg); > > } > > > > +/* field @mask should be constant and continuous */ > > "Field @mask shall be [...]" > ^^^^^ Ok, will modify it > > > +static inline void mtk_phy_update_field(void __iomem *reg, u32 > > mask, u32 val) > > ...so, (void __iomem *reg, const u32 mask, u32 val) Maybe no need const, @mask will be checked it in compile time when use FIELD_PREP(), means @mask is a constant value, but not a variable. Thanks > > > +{ > > + mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); > > +} > > + > > #endif > >
Il 22/09/22 04:36, Chunfeng Yun ha scritto: > On Wed, 2022-09-21 at 10:15 +0200, AngeloGioacchino Del Regno wrote: >> Il 20/09/22 11:00, Chunfeng Yun ha scritto: >>> Due to FIELD_PREP() macro can be used to prepare a bitfield value, >>> local ones can be remove; add the new helper to make bitfield >>> update >>> easier. >>> >>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> >>> --- >>> drivers/phy/mediatek/phy-mtk-io.h | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/drivers/phy/mediatek/phy-mtk-io.h >>> b/drivers/phy/mediatek/phy-mtk-io.h >>> index 500fcdab165d..a723d4afc9b4 100644 >>> --- a/drivers/phy/mediatek/phy-mtk-io.h >>> +++ b/drivers/phy/mediatek/phy-mtk-io.h >>> @@ -8,6 +8,7 @@ >>> #ifndef __PHY_MTK_H__ >>> #define __PHY_MTK_H__ >>> >>> +#include <linux/bitfield.h> >>> #include <linux/io.h> >>> >>> static inline void mtk_phy_clear_bits(void __iomem *reg, u32 >>> bits) >>> @@ -35,4 +36,10 @@ static inline void mtk_phy_update_bits(void >>> __iomem *reg, u32 mask, u32 val) >>> writel(tmp, reg); >>> } >>> >>> +/* field @mask should be constant and continuous */ >> >> "Field @mask shall be [...]" >> ^^^^^ > Ok, will modify it > >> >>> +static inline void mtk_phy_update_field(void __iomem *reg, u32 >>> mask, u32 val) >> >> ...so, (void __iomem *reg, const u32 mask, u32 val) > Maybe no need const, @mask will be checked it in compile time when > use FIELD_PREP(), means @mask is a constant value, but not a variable. > Adding const is not *required*, but `mask` is, effectively, a constant, hence it fully makes sense to add const. > Thanks > >> >>> +{ >>> + mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); >>> +} >>> + >>> #endif >> >> >
On Thu, 2022-09-22 at 09:17 +0200, AngeloGioacchino Del Regno wrote: > Il 22/09/22 04:36, Chunfeng Yun ha scritto: > > On Wed, 2022-09-21 at 10:15 +0200, AngeloGioacchino Del Regno > > wrote: > > > Il 20/09/22 11:00, Chunfeng Yun ha scritto: > > > > Due to FIELD_PREP() macro can be used to prepare a bitfield > > > > value, > > > > local ones can be remove; add the new helper to make bitfield > > > > update > > > > easier. > > > > > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > > > --- > > > > drivers/phy/mediatek/phy-mtk-io.h | 7 +++++++ > > > > 1 file changed, 7 insertions(+) > > > > > > > > diff --git a/drivers/phy/mediatek/phy-mtk-io.h > > > > b/drivers/phy/mediatek/phy-mtk-io.h > > > > index 500fcdab165d..a723d4afc9b4 100644 > > > > --- a/drivers/phy/mediatek/phy-mtk-io.h > > > > +++ b/drivers/phy/mediatek/phy-mtk-io.h > > > > @@ -8,6 +8,7 @@ > > > > #ifndef __PHY_MTK_H__ > > > > #define __PHY_MTK_H__ > > > > > > > > +#include <linux/bitfield.h> > > > > #include <linux/io.h> > > > > > > > > static inline void mtk_phy_clear_bits(void __iomem *reg, u32 > > > > bits) > > > > @@ -35,4 +36,10 @@ static inline void mtk_phy_update_bits(void > > > > __iomem *reg, u32 mask, u32 val) > > > > writel(tmp, reg); > > > > } > > > > > > > > +/* field @mask should be constant and continuous */ > > > > > > "Field @mask shall be [...]" > > > ^^^^^ > > > > Ok, will modify it > > > > > > > > > +static inline void mtk_phy_update_field(void __iomem *reg, u32 > > > > mask, u32 val) > > > > > > ...so, (void __iomem *reg, const u32 mask, u32 val) > > > > Maybe no need const, @mask will be checked it in compile time when > > use FIELD_PREP(), means @mask is a constant value, but not a > > variable. > > > > Adding const is not *required*, but `mask` is, effectively, a > constant, hence > it fully makes sense to add const. Prefer to leave it unchanged, there is no 'const' in function mtk_phy_update_bits(), if add 'const', will cause build warning. and FIELD_PREP() already do many checks in compile time. Thanks > > > Thanks > > > > > > > > > +{ > > > > + mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); > > > > +} > > > > + > > > > #endif > > > > > > > > >
diff --git a/drivers/phy/mediatek/phy-mtk-io.h b/drivers/phy/mediatek/phy-mtk-io.h index 500fcdab165d..a723d4afc9b4 100644 --- a/drivers/phy/mediatek/phy-mtk-io.h +++ b/drivers/phy/mediatek/phy-mtk-io.h @@ -8,6 +8,7 @@ #ifndef __PHY_MTK_H__ #define __PHY_MTK_H__ +#include <linux/bitfield.h> #include <linux/io.h> static inline void mtk_phy_clear_bits(void __iomem *reg, u32 bits) @@ -35,4 +36,10 @@ static inline void mtk_phy_update_bits(void __iomem *reg, u32 mask, u32 val) writel(tmp, reg); } +/* field @mask should be constant and continuous */ +static inline void mtk_phy_update_field(void __iomem *reg, u32 mask, u32 val) +{ + mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); +} + #endif
Due to FIELD_PREP() macro can be used to prepare a bitfield value, local ones can be remove; add the new helper to make bitfield update easier. Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> --- drivers/phy/mediatek/phy-mtk-io.h | 7 +++++++ 1 file changed, 7 insertions(+)