| Message ID | 20230104132646.7100-1-chunfeng.yun@mediatek.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v6,1/3] phy: mediatek: fix build warning caused by clang | expand |
On 04-01-23, 21:26, Chunfeng Yun wrote: > Remove the temporary @mask_, this may cause build warning when use clang > compiler for powerpc, but can't reproduce it when compile for arm64. > the build warning is -Wtautological-constant-out-of-range-compare, and > caused by > "BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)" > > After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run > checkpatch.pl, due to @mask is constant, no reuse problem will happen. > > Fixes: 84513eccd678 ("phy: mediatek: fix build warning of FIELD_PREP()") > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > --- > v6: modify the title Title still does not tell what this patch is.. It tells me effect of the patch but not the changes, pls revise... "remove temp mask" can be better title > v5: no changes > v4 new patch, I'm not sure it can fix build warning, due to I don't cross compile > it for powerpc using clang in office. > --- > drivers/phy/mediatek/phy-mtk-io.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/mediatek/phy-mtk-io.h b/drivers/phy/mediatek/phy-mtk-io.h > index d20ad5e5be81..58f06db822cb 100644 > --- a/drivers/phy/mediatek/phy-mtk-io.h > +++ b/drivers/phy/mediatek/phy-mtk-io.h > @@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void __iomem *reg, u32 mask, u32 val) > /* field @mask shall be constant and continuous */ > #define mtk_phy_update_field(reg, mask, val) \ > ({ \ > - typeof(mask) mask_ = (mask); \ > - mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \ > + BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not constant"); \ > + mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \ > }) > > #endif > -- > 2.18.0
On Wed, Jan 4, 2023 at 5:26 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote: > > Remove the temporary @mask_, this may cause build warning when use clang > compiler for powerpc, but can't reproduce it when compile for arm64. > the build warning is -Wtautological-constant-out-of-range-compare, and > caused by > "BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)" Can you please include the text of the observed warning? > > After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run > checkpatch.pl, due to @mask is constant, no reuse problem will happen. > > Fixes: 84513eccd678 ("phy: mediatek: fix build warning of FIELD_PREP()") Uh, why was 84513eccd678 sent/reviewed/merged if it didn't fix the issue correctly in the first place? Is the issue perhaps that your mask isn't wide enough in the first place, and should be? See: commit cfd6fb45cfaf ("crypto: ccree - avoid out-of-range warnings from clang") for inspiration. > Reported-by: kernel test robot <lkp@intel.com> Can you please include the Link: tag to the lore URL of the report? > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > --- > v6: modify the title > v5: no changes > v4 new patch, I'm not sure it can fix build warning, due to I don't cross compile > it for powerpc using clang in office. > --- > drivers/phy/mediatek/phy-mtk-io.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/mediatek/phy-mtk-io.h b/drivers/phy/mediatek/phy-mtk-io.h > index d20ad5e5be81..58f06db822cb 100644 > --- a/drivers/phy/mediatek/phy-mtk-io.h > +++ b/drivers/phy/mediatek/phy-mtk-io.h > @@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void __iomem *reg, u32 mask, u32 val) > /* field @mask shall be constant and continuous */ > #define mtk_phy_update_field(reg, mask, val) \ > ({ \ > - typeof(mask) mask_ = (mask); \ > - mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \ > + BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not constant"); \ > + mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \ > }) > > #endif > -- > 2.18.0 >
On Fri, 2023-01-13 at 23:42 +0530, Vinod Koul wrote: > On 04-01-23, 21:26, Chunfeng Yun wrote: > > Remove the temporary @mask_, this may cause build warning when use > > clang > > compiler for powerpc, but can't reproduce it when compile for > > arm64. > > the build warning is -Wtautological-constant-out-of-range-compare, > > and > > caused by > > "BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)" > > > > After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run > > checkpatch.pl, due to @mask is constant, no reuse problem will > > happen. > > > > Fixes: 84513eccd678 ("phy: mediatek: fix build warning of > > FIELD_PREP()") > > Reported-by: kernel test robot <lkp@intel.com> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > --- > > v6: modify the title > > Title still does not tell what this patch is.. It tells me effect of > the > patch but not the changes, pls revise... > > "remove temp mask" can be better title Got it, will modify it in next version, thanks > > > v5: no changes > > v4 new patch, I'm not sure it can fix build warning, due to I don't > > cross compile > > it for powerpc using clang in office. > > --- > > drivers/phy/mediatek/phy-mtk-io.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/phy/mediatek/phy-mtk-io.h > > b/drivers/phy/mediatek/phy-mtk-io.h > > index d20ad5e5be81..58f06db822cb 100644 > > --- a/drivers/phy/mediatek/phy-mtk-io.h > > +++ b/drivers/phy/mediatek/phy-mtk-io.h > > @@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void > > __iomem *reg, u32 mask, u32 val) > > /* field @mask shall be constant and continuous */ > > #define mtk_phy_update_field(reg, mask, val) \ > > ({ \ > > - typeof(mask) mask_ = (mask); \ > > - mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \ > > + BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not > > constant"); \ > > + mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \ > > }) > > > > #endif > > -- > > 2.18.0 > >
On Fri, 2023-01-13 at 10:36 -0800, Nick Desaulniers wrote: > On Wed, Jan 4, 2023 at 5:26 AM Chunfeng Yun < > chunfeng.yun@mediatek.com> wrote: > > > > Remove the temporary @mask_, this may cause build warning when use > > clang > > compiler for powerpc, but can't reproduce it when compile for > > arm64. > > the build warning is -Wtautological-constant-out-of-range-compare, > > and > > caused by > > "BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)" > > Can you please include the text of the observed warning? Ok, will add more logs > > > > > After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run > > checkpatch.pl, due to @mask is constant, no reuse problem will > > happen. > > > > Fixes: 84513eccd678 ("phy: mediatek: fix build warning of > > FIELD_PREP()") > > Uh, why was 84513eccd678 sent/reviewed/merged if it didn't fix the > issue correctly in the first place? Sorry, I can't reproduce it, but make sure no such issue happens on arm arch using gcc/clang. MTK only uses arm/mips arch, it's difficult for me to set up clang cross compiler for other archs in office. > > Is the issue perhaps that your mask isn't wide enough in the first > place, and should be? the masks are usually created by GENMASK macro; There is no warning when build for arm64 using clang. > See: > commit cfd6fb45cfaf ("crypto: ccree - avoid out-of-range warnings > from clang") > for inspiration. Ok, I'll do it > > > Reported-by: kernel test robot <lkp@intel.com> > > Can you please include the Link: tag to the lore URL of the report? Ok, thanks a lot > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > --- > > v6: modify the title > > v5: no changes > > v4 new patch, I'm not sure it can fix build warning, due to I don't > > cross compile > > it for powerpc using clang in office. > > --- > > drivers/phy/mediatek/phy-mtk-io.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/phy/mediatek/phy-mtk-io.h > > b/drivers/phy/mediatek/phy-mtk-io.h > > index d20ad5e5be81..58f06db822cb 100644 > > --- a/drivers/phy/mediatek/phy-mtk-io.h > > +++ b/drivers/phy/mediatek/phy-mtk-io.h > > @@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void > > __iomem *reg, u32 mask, u32 val) > > /* field @mask shall be constant and continuous */ > > #define mtk_phy_update_field(reg, mask, val) \ > > ({ \ > > - typeof(mask) mask_ = (mask); \ > > - mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \ > > + BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not > > constant"); \ > > + mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \ > > }) > > > > #endif > > -- > > 2.18.0 > > > >
On Fri, 2023-01-13 at 10:36 -0800, Nick Desaulniers wrote: > On Wed, Jan 4, 2023 at 5:26 AM Chunfeng Yun < > chunfeng.yun@mediatek.com> wrote: > > > > Remove the temporary @mask_, this may cause build warning when use > > clang > > compiler for powerpc, but can't reproduce it when compile for > > arm64. > > the build warning is -Wtautological-constant-out-of-range-compare, > > and > > caused by > > "BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)" > > Can you please include the text of the observed warning? > > > > > After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run > > checkpatch.pl, due to @mask is constant, no reuse problem will > > happen. > > > > Fixes: 84513eccd678 ("phy: mediatek: fix build warning of > > FIELD_PREP()") > > Uh, why was 84513eccd678 sent/reviewed/merged if it didn't fix the > issue correctly in the first place? > > Is the issue perhaps that your mask isn't wide enough in the first > place, and should be? See: > commit cfd6fb45cfaf ("crypto: ccree - avoid out-of-range warnings > from clang") > for inspiration. I look at this patch, it said that " This does not happen in other places that just pass a constant here. " That's indeed true, no such warning before using FIELD_PREP() due to the masks are always constant. So seems that it can fix the waring if avoid using a temporary variable @mask_ in mtk_phy_update_field() macro. Thanks a lot > > > Reported-by: kernel test robot <lkp@intel.com> > > Can you please include the Link: tag to the lore URL of the report? > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > --- > > v6: modify the title > > v5: no changes > > v4 new patch, I'm not sure it can fix build warning, due to I don't > > cross compile > > it for powerpc using clang in office. > > --- > > drivers/phy/mediatek/phy-mtk-io.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/phy/mediatek/phy-mtk-io.h > > b/drivers/phy/mediatek/phy-mtk-io.h > > index d20ad5e5be81..58f06db822cb 100644 > > --- a/drivers/phy/mediatek/phy-mtk-io.h > > +++ b/drivers/phy/mediatek/phy-mtk-io.h > > @@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void > > __iomem *reg, u32 mask, u32 val) > > /* field @mask shall be constant and continuous */ > > #define mtk_phy_update_field(reg, mask, val) \ > > ({ \ > > - typeof(mask) mask_ = (mask); \ > > - mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \ > > + BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not > > constant"); \ > > + mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \ > > }) > > > > #endif > > -- > > 2.18.0 > > > >
On Fri, 2023-01-13 at 10:36 -0800, Nick Desaulniers wrote: > On Wed, Jan 4, 2023 at 5:26 AM Chunfeng Yun < > chunfeng.yun@mediatek.com> wrote: > > > > Remove the temporary @mask_, this may cause build warning when use > > clang > > compiler for powerpc, but can't reproduce it when compile for > > arm64. > > the build warning is -Wtautological-constant-out-of-range-compare, > > and > > caused by > > "BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)" > > Can you please include the text of the observed warning? > > > > > After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run > > checkpatch.pl, due to @mask is constant, no reuse problem will > > happen. > > > > Fixes: 84513eccd678 ("phy: mediatek: fix build warning of > > FIELD_PREP()") > > Uh, why was 84513eccd678 sent/reviewed/merged if it didn't fix the > issue correctly in the first place? > > Is the issue perhaps that your mask isn't wide enough in the first > place, and should be? See: > commit cfd6fb45cfaf ("crypto: ccree - avoid out-of-range warnings > from clang") > for inspiration. I look at this patch, it said that " This does not happen in other places that just pass a constant here. " That's indeed true, no such warning before using FIELD_PREP() due to the masks are always constant. So seems that it can fix the waring if avoid using a temporary variable @mask_. Thanks a lot > > > Reported-by: kernel test robot <lkp@intel.com> > > Can you please include the Link: tag to the lore URL of the report? > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > --- > > v6: modify the title > > v5: no changes > > v4 new patch, I'm not sure it can fix build warning, due to I don't > > cross compile > > it for powerpc using clang in office. > > --- > > drivers/phy/mediatek/phy-mtk-io.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/phy/mediatek/phy-mtk-io.h > > b/drivers/phy/mediatek/phy-mtk-io.h > > index d20ad5e5be81..58f06db822cb 100644 > > --- a/drivers/phy/mediatek/phy-mtk-io.h > > +++ b/drivers/phy/mediatek/phy-mtk-io.h > > @@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void > > __iomem *reg, u32 mask, u32 val) > > /* field @mask shall be constant and continuous */ > > #define mtk_phy_update_field(reg, mask, val) \ > > ({ \ > > - typeof(mask) mask_ = (mask); \ > > - mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \ > > + BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not > > constant"); \ > > + mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \ > > }) > > > > #endif > > -- > > 2.18.0 > > > >
diff --git a/drivers/phy/mediatek/phy-mtk-io.h b/drivers/phy/mediatek/phy-mtk-io.h index d20ad5e5be81..58f06db822cb 100644 --- a/drivers/phy/mediatek/phy-mtk-io.h +++ b/drivers/phy/mediatek/phy-mtk-io.h @@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void __iomem *reg, u32 mask, u32 val) /* field @mask shall be constant and continuous */ #define mtk_phy_update_field(reg, mask, val) \ ({ \ - typeof(mask) mask_ = (mask); \ - mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \ + BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not constant"); \ + mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \ }) #endif
Remove the temporary @mask_, this may cause build warning when use clang compiler for powerpc, but can't reproduce it when compile for arm64. the build warning is -Wtautological-constant-out-of-range-compare, and caused by "BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)" After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run checkpatch.pl, due to @mask is constant, no reuse problem will happen. Fixes: 84513eccd678 ("phy: mediatek: fix build warning of FIELD_PREP()") Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> --- v6: modify the title v5: no changes v4 new patch, I'm not sure it can fix build warning, due to I don't cross compile it for powerpc using clang in office. --- drivers/phy/mediatek/phy-mtk-io.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)