| Message ID | 20220916191349.1659269-3-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 | success | Errors and warnings before: 2 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 | success | Errors and warnings before: 1 this patch: 1 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 31 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:49PM -0700, Colin Foster wrote: > - regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1); > - regmap_field_write(ocelot->regfields[SYS_RESET_CFG_CORE_ENA], 1); > + err = regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1); > + if (err) > + return err; > > - return 0; > + err = regmap_field_write(ocelot->regfields[SYS_RESET_CFG_CORE_ENA], 1); > + > + return err; > } A kernel janitor will come and patch this up to: return regmap_field_write(ocelot->regfields[SYS_RESET_CFG_CORE_ENA], 1); so it's better to do it yourself.
On Fri, Sep 16, 2022 at 10:40:10PM +0000, Vladimir Oltean wrote: > On Fri, Sep 16, 2022 at 12:13:49PM -0700, Colin Foster wrote: > > - regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1); > > - regmap_field_write(ocelot->regfields[SYS_RESET_CFG_CORE_ENA], 1); > > + err = regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1); > > + if (err) > > + return err; > > > > - return 0; > > + err = regmap_field_write(ocelot->regfields[SYS_RESET_CFG_CORE_ENA], 1); > > + > > + return err; > > } > > A kernel janitor will come and patch this up to: > > return regmap_field_write(ocelot->regfields[SYS_RESET_CFG_CORE_ENA], 1); > > so it's better to do it yourself. Good catch. That and removing the IS_ERR_VALUE macro from patch 1 and I'll resubmit after the weekend.
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c index 79b7af36b4f4..415b7f4c7277 100644 --- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c +++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c @@ -211,8 +211,13 @@ static int ocelot_reset(struct ocelot *ocelot) 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); + err = regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_INIT], 1); + if (err) + return err; + + err = regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1); + if (err) + return err; /* MEM_INIT is a self-clearing bit. Wait for it to be cleared (should be * 100us) before enabling the switch core. @@ -222,10 +227,13 @@ static int ocelot_reset(struct ocelot *ocelot) 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); + err = regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1); + if (err) + return err; - return 0; + err = regmap_field_write(ocelot->regfields[SYS_RESET_CFG_CORE_ENA], 1); + + return err; } /* Watermark encode
The ocelot_reset() function utilizes regmap_field_write() but wasn't checking return values. While this won't cause issues for the current MMIO regmaps, it could be an issue for externally controlled interfaces. Add checks for these return values. Signed-off-by: Colin Foster <colin.foster@in-advantage.com> --- drivers/net/ethernet/mscc/ocelot_vsc7514.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)