| Message ID | 20220916191349.1659269-2-colin.foster@in-advantage.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | clean up ocelot_reset() routine | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next, async |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 9 of 9 maintainers |
| netdev/build_clang | fail | Errors and warnings before: 0 this patch: 2 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | fail | Errors and warnings before: 0 this patch: 1 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 57 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Fri, Sep 16, 2022 at 12:13:48PM -0700, Colin Foster wrote: > Clean up the reset code by utilizing readx_poll_timeout instead of a custom > loop. > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > --- Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
On Fri, Sep 16, 2022 at 12:13:48PM -0700, Colin Foster wrote: > Clean up the reset code by utilizing readx_poll_timeout instead of a custom > loop. > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > --- > drivers/net/ethernet/mscc/ocelot_vsc7514.c | 32 ++++++++++++++++------ > 1 file changed, 23 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c > index ae42bbba5747..79b7af36b4f4 100644 > --- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c > +++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c > @@ -6,6 +6,7 @@ > */ > #include <linux/dsa/ocelot.h> > #include <linux/interrupt.h> > +#include <linux/iopoll.h> > #include <linux/module.h> > #include <linux/of_net.h> > #include <linux/netdevice.h> > @@ -25,6 +26,9 @@ > #define VSC7514_VCAP_POLICER_BASE 128 > #define VSC7514_VCAP_POLICER_MAX 191 > > +#define MEM_INIT_SLEEP_US 1000 > +#define MEM_INIT_TIMEOUT_US 100000 > + > static const u32 *ocelot_regmap[TARGET_MAX] = { > [ANA] = vsc7514_ana_regmap, > [QS] = vsc7514_qs_regmap, > @@ -191,22 +195,32 @@ static const struct of_device_id mscc_ocelot_match[] = { > }; > MODULE_DEVICE_TABLE(of, mscc_ocelot_match); > > +static int ocelot_mem_init_status(struct ocelot *ocelot) > +{ > + unsigned int val; > + int err; > + > + err = regmap_field_read(ocelot->regfields[SYS_RESET_CFG_MEM_INIT], > + &val); > + > + return err ?: val; > +} > + > static int ocelot_reset(struct ocelot *ocelot) > { > - int retries = 100; > + int err; > u32 val; > > regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_INIT], 1); > regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1); > > - do { > - msleep(1); > - regmap_field_read(ocelot->regfields[SYS_RESET_CFG_MEM_INIT], > - &val); > - } while (val && --retries); > - > - if (!retries) > - return -ETIMEDOUT; > + /* MEM_INIT is a self-clearing bit. Wait for it to be cleared (should be > + * 100us) before enabling the switch core. > + */ > + err = readx_poll_timeout(ocelot_mem_init_status, ocelot, val, !val, > + MEM_INIT_SLEEP_US, MEM_INIT_TIMEOUT_US); > + if (IS_ERR_VALUE(err)) I see patchwork is complaining about IS_ERR_VALUE. I think it should just be if (err). I'll test before sending v2 after the weekend. > + return err; > > regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1); > regmap_field_write(ocelot->regfields[SYS_RESET_CFG_CORE_ENA], 1); > -- > 2.25.1 >
Hi Colin,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Colin-Foster/clean-up-ocelot_reset-routine/20220917-031554
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 862deb68c1bc19783ab7a98ba17a441aa76eba52
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20220917/202209171105.95JOLHu6-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/11df0e8e7af298721b4bb1af286571272dd0d56e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Colin-Foster/clean-up-ocelot_reset-routine/20220917-031554
git checkout 11df0e8e7af298721b4bb1af286571272dd0d56e
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/net/ethernet/mscc/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/net/ethernet/mscc/ocelot_vsc7514.c:222:6: warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast]
if (IS_ERR_VALUE(err))
^~~~~~~~~~~~~~~~~
include/linux/err.h:22:49: note: expanded from macro 'IS_ERR_VALUE'
#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:78:42: note: expanded from macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
^
1 warning generated.
vim +222 drivers/net/ethernet/mscc/ocelot_vsc7514.c
208
209 static int ocelot_reset(struct ocelot *ocelot)
210 {
211 int err;
212 u32 val;
213
214 regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_INIT], 1);
215 regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1);
216
217 /* MEM_INIT is a self-clearing bit. Wait for it to be cleared (should be
218 * 100us) before enabling the switch core.
219 */
220 err = readx_poll_timeout(ocelot_mem_init_status, ocelot, val, !val,
221 MEM_INIT_SLEEP_US, MEM_INIT_TIMEOUT_US);
> 222 if (IS_ERR_VALUE(err))
223 return err;
224
225 regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1);
226 regmap_field_write(ocelot->regfields[SYS_RESET_CFG_CORE_ENA], 1);
227
228 return 0;
229 }
230
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c index ae42bbba5747..79b7af36b4f4 100644 --- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c +++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c @@ -6,6 +6,7 @@ */ #include <linux/dsa/ocelot.h> #include <linux/interrupt.h> +#include <linux/iopoll.h> #include <linux/module.h> #include <linux/of_net.h> #include <linux/netdevice.h> @@ -25,6 +26,9 @@ #define VSC7514_VCAP_POLICER_BASE 128 #define VSC7514_VCAP_POLICER_MAX 191 +#define MEM_INIT_SLEEP_US 1000 +#define MEM_INIT_TIMEOUT_US 100000 + static const u32 *ocelot_regmap[TARGET_MAX] = { [ANA] = vsc7514_ana_regmap, [QS] = vsc7514_qs_regmap, @@ -191,22 +195,32 @@ static const struct of_device_id mscc_ocelot_match[] = { }; MODULE_DEVICE_TABLE(of, mscc_ocelot_match); +static int ocelot_mem_init_status(struct ocelot *ocelot) +{ + unsigned int val; + int err; + + err = regmap_field_read(ocelot->regfields[SYS_RESET_CFG_MEM_INIT], + &val); + + return err ?: val; +} + static int ocelot_reset(struct ocelot *ocelot) { - int retries = 100; + int err; u32 val; regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_INIT], 1); regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1); - do { - msleep(1); - regmap_field_read(ocelot->regfields[SYS_RESET_CFG_MEM_INIT], - &val); - } while (val && --retries); - - if (!retries) - return -ETIMEDOUT; + /* MEM_INIT is a self-clearing bit. Wait for it to be cleared (should be + * 100us) before enabling the switch core. + */ + err = readx_poll_timeout(ocelot_mem_init_status, ocelot, val, !val, + MEM_INIT_SLEEP_US, MEM_INIT_TIMEOUT_US); + if (IS_ERR_VALUE(err)) + return err; regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1); regmap_field_write(ocelot->regfields[SYS_RESET_CFG_CORE_ENA], 1);
Clean up the reset code by utilizing readx_poll_timeout instead of a custom loop. Signed-off-by: Colin Foster <colin.foster@in-advantage.com> --- drivers/net/ethernet/mscc/ocelot_vsc7514.c | 32 ++++++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-)