* [PATCH] extcon: fix a missing check of regmap_read @ 2019-03-18 16:41 ` Kangjie Lu 2019-03-18 17:30 ` Hans de Goede 2019-03-19 0:39 ` Chanwoo Choi 0 siblings, 2 replies; 5+ messages in thread From: Kangjie Lu @ 2019-03-18 16:41 UTC (permalink / raw) To: kjlu Cc: pakki001, MyungJoo Ham, Chanwoo Choi, Hans de Goede, Chen-Yu Tsai, linux-kernel When regmap_read fails, it doesn't make sense to use the read value "val" because it can be uninitialized. The fix returns if regmap_read fails. Signed-off-by: Kangjie Lu <kjlu@umn.edu> --- drivers/extcon/extcon-axp288.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c index a983708b77a6..b2ba5f073aa7 100644 --- a/drivers/extcon/extcon-axp288.c +++ b/drivers/extcon/extcon-axp288.c @@ -143,6 +143,10 @@ static void axp288_extcon_log_rsi(struct axp288_extcon_info *info) int ret; ret = regmap_read(info->regmap, AXP288_PS_BOOT_REASON_REG, &val); + if (ret) { + dev_dbg(info->dev, "regmap_read error %d\n", ret); + return; + } for (i = 0, rsi = axp288_pwr_up_down_info; *rsi; rsi++, i++) { if (val & BIT(i)) { dev_dbg(info->dev, "%s\n", *rsi); -- 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] extcon: fix a missing check of regmap_read 2019-03-18 16:41 ` [PATCH] extcon: fix a missing check of regmap_read Kangjie Lu @ 2019-03-18 17:30 ` Hans de Goede 2019-03-19 0:39 ` Chanwoo Choi 1 sibling, 0 replies; 5+ messages in thread From: Hans de Goede @ 2019-03-18 17:30 UTC (permalink / raw) To: Kangjie Lu Cc: pakki001, MyungJoo Ham, Chanwoo Choi, Chen-Yu Tsai, linux-kernel Hi, On 18-03-19 17:41, Kangjie Lu wrote: > When regmap_read fails, it doesn't make sense to use the read > value "val" because it can be uninitialized. > > The fix returns if regmap_read fails. > > Signed-off-by: Kangjie Lu <kjlu@umn.edu> > --- > drivers/extcon/extcon-axp288.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c > index a983708b77a6..b2ba5f073aa7 100644 > --- a/drivers/extcon/extcon-axp288.c > +++ b/drivers/extcon/extcon-axp288.c > @@ -143,6 +143,10 @@ static void axp288_extcon_log_rsi(struct axp288_extcon_info *info) > int ret; > > ret = regmap_read(info->regmap, AXP288_PS_BOOT_REASON_REG, &val); > + if (ret) { > + dev_dbg(info->dev, "regmap_read error %d\n", ret); > + return; > + } > for (i = 0, rsi = axp288_pwr_up_down_info; *rsi; rsi++, i++) { > if (val & BIT(i)) { > dev_dbg(info->dev, "%s\n", *rsi); Patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] extcon: fix a missing check of regmap_read 2019-03-18 16:41 ` [PATCH] extcon: fix a missing check of regmap_read Kangjie Lu 2019-03-18 17:30 ` Hans de Goede @ 2019-03-19 0:39 ` Chanwoo Choi 2019-03-20 7:35 ` Kangjie Lu 1 sibling, 1 reply; 5+ messages in thread From: Chanwoo Choi @ 2019-03-19 0:39 UTC (permalink / raw) To: Kangjie Lu Cc: pakki001, MyungJoo Ham, Hans de Goede, Chen-Yu Tsai, linux-kernel Hi, On 19. 3. 19. 오전 1:41, Kangjie Lu wrote: > When regmap_read fails, it doesn't make sense to use the read > value "val" because it can be uninitialized. > > The fix returns if regmap_read fails. > > Signed-off-by: Kangjie Lu <kjlu@umn.edu> > --- > drivers/extcon/extcon-axp288.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c > index a983708b77a6..b2ba5f073aa7 100644 > --- a/drivers/extcon/extcon-axp288.c > +++ b/drivers/extcon/extcon-axp288.c > @@ -143,6 +143,10 @@ static void axp288_extcon_log_rsi(struct axp288_extcon_info *info) > int ret; > > ret = regmap_read(info->regmap, AXP288_PS_BOOT_REASON_REG, &val); > + if (ret) { > + dev_dbg(info->dev, "regmap_read error %d\n", ret); dev_err is correct for handling the error case instead of dev_dbg. And I recommend that you use following error log because extcon-axp288.c already used the 'failed to ...' log style. If there is no special reason, you better to keep the consistency for the readability. - dev_err(info->dev, "failed to read BOOT_REASON_REG: %d\n", ret); > + return; > + } > for (i = 0, rsi = axp288_pwr_up_down_info; *rsi; rsi++, i++) { > if (val & BIT(i)) { > dev_dbg(info->dev, "%s\n", *rsi); > -- Best Regards, Chanwoo Choi Samsung Electronics ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] extcon: fix a missing check of regmap_read 2019-03-19 0:39 ` Chanwoo Choi @ 2019-03-20 7:35 ` Kangjie Lu 2019-03-20 8:00 ` Chanwoo Choi 0 siblings, 1 reply; 5+ messages in thread From: Kangjie Lu @ 2019-03-20 7:35 UTC (permalink / raw) To: kjlu Cc: pakki001, MyungJoo Ham, Chanwoo Choi, Chen-Yu Tsai, Hans de Goede, linux-kernel When regmap_read fails, it doesn't make sense to use the read value "val" because it can be uninitialized. The fix returns if regmap_read fails. Signed-off-by: Kangjie Lu <kjlu@umn.edu> --- drivers/extcon/extcon-axp288.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c index a983708b77a6..b2ba5f073aa7 100644 --- a/drivers/extcon/extcon-axp288.c +++ b/drivers/extcon/extcon-axp288.c @@ -143,6 +143,10 @@ static void axp288_extcon_log_rsi(struct axp288_extcon_info *info) int ret; ret = regmap_read(info->regmap, AXP288_PS_BOOT_REASON_REG, &val); + if (ret) { + dev_err(info->dev, "failed to read BOOT_REASON_REG: %d\n", ret); + return; + } for (i = 0, rsi = axp288_pwr_up_down_info; *rsi; rsi++, i++) { if (val & BIT(i)) { dev_dbg(info->dev, "%s\n", *rsi); -- 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] extcon: fix a missing check of regmap_read 2019-03-20 7:35 ` Kangjie Lu @ 2019-03-20 8:00 ` Chanwoo Choi 0 siblings, 0 replies; 5+ messages in thread From: Chanwoo Choi @ 2019-03-20 8:00 UTC (permalink / raw) To: Kangjie Lu Cc: pakki001, MyungJoo Ham, Chen-Yu Tsai, Hans de Goede, linux-kernel Hi, You better to edit the patch title as following in order to sustain the title format for extcon: extcon: fix a missing check of regmap_read -> extcon: axp288: Fix a missing check of regmap_read On 19. 3. 20. 오후 4:35, Kangjie Lu wrote: > When regmap_read fails, it doesn't make sense to use the read > value "val" because it can be uninitialized. > > The fix returns if regmap_read fails. > > Signed-off-by: Kangjie Lu <kjlu@umn.edu> > --- > drivers/extcon/extcon-axp288.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c > index a983708b77a6..b2ba5f073aa7 100644 > --- a/drivers/extcon/extcon-axp288.c > +++ b/drivers/extcon/extcon-axp288.c > @@ -143,6 +143,10 @@ static void axp288_extcon_log_rsi(struct axp288_extcon_info *info) > int ret; > > ret = regmap_read(info->regmap, AXP288_PS_BOOT_REASON_REG, &val); > + if (ret) { > + dev_err(info->dev, "failed to read BOOT_REASON_REG: %d\n", ret); > + return; > + } Have to add the blank line. time. And I think that axp288_extcon_log_rsi() have to return the 'error value' instead of 'void' return type. And then should handle the return value of axp288_extcon_log_rsi() within the axp288_extcon_probe(). For example, ret = axp288_extcon_log_rsi(info) if (ret < 0) return ret; > for (i = 0, rsi = axp288_pwr_up_down_info; *rsi; rsi++, i++) { > if (val & BIT(i)) { > dev_dbg(info->dev, "%s\n", *rsi); > -- Best Regards, Chanwoo Choi Samsung Electronics ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-20 8:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20190318164133epcas5p1a539488d8614011bdf4e734d76da4cc3@epcas5p1.samsung.com>
2019-03-18 16:41 ` [PATCH] extcon: fix a missing check of regmap_read Kangjie Lu
2019-03-18 17:30 ` Hans de Goede
2019-03-19 0:39 ` Chanwoo Choi
2019-03-20 7:35 ` Kangjie Lu
2019-03-20 8:00 ` Chanwoo Choi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®