* [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 related [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 related [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.