mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/5] mci: stm32_sdmmc2: don't ignore reset_control_get errors
@ 2020-02-10 18:08 Ahmad Fatoum
  2020-02-10 18:08 ` [PATCH 2/5] net: designware: socfpga: fix possible invalid pointer deref Ahmad Fatoum
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2020-02-10 18:08 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

reset_control_get returns a NULL pointer when no resets were specified
via device tree properties. If there's a malformed "resets"-property, we
get an error pointer. This error should be propagated, only the NULL
returns are the ones we can safely ignore.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/stm32_sdmmc2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mci/stm32_sdmmc2.c b/drivers/mci/stm32_sdmmc2.c
index 44f7e6239949..695e61d1ccd7 100644
--- a/drivers/mci/stm32_sdmmc2.c
+++ b/drivers/mci/stm32_sdmmc2.c
@@ -622,7 +622,7 @@ static int stm32_sdmmc2_probe(struct amba_device *adev,
 
 	priv->reset_ctl = reset_control_get(dev, NULL);
 	if (IS_ERR(priv->reset_ctl))
-		priv->reset_ctl = NULL;
+		return PTR_ERR(priv->reset_ctl);
 
 	mci->f_min = 400000;
 	/* f_max is taken from kernel v5.3 variant_stm32_sdmmc */
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 2/5] net: designware: socfpga: fix possible invalid pointer deref
  2020-02-10 18:08 [PATCH 1/5] mci: stm32_sdmmc2: don't ignore reset_control_get errors Ahmad Fatoum
@ 2020-02-10 18:08 ` Ahmad Fatoum
  2020-02-10 18:08 ` [PATCH 3/5] watchdog: dw_wdt: inform user on missing reset control line Ahmad Fatoum
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2020-02-10 18:08 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

In cases where the reset controller specification in the device tree is
malformed, we get an error pointer back from reset_control_get. This
compares unequal to NULL and would cause an access violation when passed
to reset_control_(de)?assert.

Fix this by propagating the error. When the reset controller is missing,
reset_control_(de)?assert will be passed NULL pointers, rendering them
no-ops.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/net/designware_socfpga.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/net/designware_socfpga.c b/drivers/net/designware_socfpga.c
index ce3ac38ebe87..d6c28af45e65 100644
--- a/drivers/net/designware_socfpga.c
+++ b/drivers/net/designware_socfpga.c
@@ -77,8 +77,7 @@ static int socfpga_gen5_set_phy_mode(struct socfpga_dwc_dev *dwc_dev)
 	}
 
 	/* Assert reset to the enet controller before changing the phy mode */
-	if (eth_dev->rst)
-		reset_control_assert(eth_dev->rst);
+	reset_control_assert(eth_dev->rst);
 
 	ctrl = readl(dwc_dev->sys_mgr_base + reg_offset);
 	ctrl &= ~(SYSMGR_EMACGRP_CTRL_PHYSEL_MASK << reg_shift);
@@ -104,8 +103,7 @@ static int socfpga_gen5_set_phy_mode(struct socfpga_dwc_dev *dwc_dev)
 	/* Deassert reset for the phy configuration to be sampled by
 	 * the enet controller, and operation to start in requested mode
 	 */
-	if (eth_dev->rst)
-		reset_control_deassert(eth_dev->rst);
+	reset_control_deassert(eth_dev->rst);
 
 	return 0;
 }
@@ -124,8 +122,7 @@ static int socfpga_gen10_set_phy_mode(struct socfpga_dwc_dev *dwc_dev)
 	}
 
 	/* Assert reset to the enet controller before changing the phy mode */
-	if (eth_dev->rst)
-		reset_control_assert(eth_dev->rst);
+	reset_control_assert(eth_dev->rst);
 
 	ctrl = readl(dwc_dev->sys_mgr_base + reg_offset);
 	ctrl &= ~(SYSMGR_EMACGRP_CTRL_PHYSEL_MASK << reg_shift);
@@ -151,8 +148,7 @@ static int socfpga_gen10_set_phy_mode(struct socfpga_dwc_dev *dwc_dev)
 	/* Deassert reset for the phy configuration to be sampled by
 	 * the enet controller, and operation to start in requested mode
 	 */
-	if (eth_dev->rst)
-		reset_control_deassert(eth_dev->rst);
+	reset_control_deassert(eth_dev->rst);
 
 	return 0;
 }
@@ -212,8 +208,10 @@ static int socfpga_dwc_ether_probe(struct device_d *dev)
 		return PTR_ERR(priv);
 
 	priv->rst = reset_control_get(dev, NULL);
-	if (IS_ERR(priv->rst))
-		dev_warn(dev, "No reset lines.\n");
+	if (IS_ERR(priv->rst)) {
+		dev_err(dev, "Invalid reset lines.\n");
+		return PTR_ERR(priv->rst);
+	}
 
 	dwc_dev->priv = priv;
 
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 3/5] watchdog: dw_wdt: inform user on missing reset control line
  2020-02-10 18:08 [PATCH 1/5] mci: stm32_sdmmc2: don't ignore reset_control_get errors Ahmad Fatoum
  2020-02-10 18:08 ` [PATCH 2/5] net: designware: socfpga: fix possible invalid pointer deref Ahmad Fatoum
@ 2020-02-10 18:08 ` Ahmad Fatoum
  2020-02-10 18:08 ` [PATCH 4/5] watchdog: dw_wdt: remove duplicated error message Ahmad Fatoum
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2020-02-10 18:08 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The driver has an error message on probe and one on each watchdog stop
attempt that warns the user if the reset line is missing.

Missing reset line (because the "reset"-property is missing) is
indicated by a NULL pointer though, so these warnings were only
triggered when the reset controller specification is malformed.

Fix this by propagating malformed reset pointer specification and
continuing only if it's either valid or completely missing.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/dw_wdt.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index bedb2ddfe81f..2d1aa5e587ec 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -72,9 +72,9 @@ static int dw_wdt_stop(struct watchdog *wdd)
 {
 	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
 
-	if (IS_ERR(dw_wdt->rst)) {
+	if (!dw_wdt->rst) {
 		dev_warn(dw_wdt->wdd.hwdev, "No reset line. Will not stop.\n");
-		return PTR_ERR(dw_wdt->rst);
+		return -ENOSYS;
 	}
 
 	reset_control_assert(dw_wdt->rst);
@@ -153,7 +153,7 @@ static int dw_wdt_drv_probe(struct device_d *dev)
 
 	dw_wdt->rst = reset_control_get(dev, NULL);
 	if (IS_ERR(dw_wdt->rst))
-		dev_warn(dev, "No reset lines. Will not be able to stop once started.\n");
+		return PTR_ERR(dw_wdt->rst);
 
 	wdd = &dw_wdt->wdd;
 	wdd->name = "dw_wdt";
@@ -171,8 +171,10 @@ static int dw_wdt_drv_probe(struct device_d *dev)
 	if (ret)
 		dev_warn(dev, "cannot register restart handler\n");
 
-	if (!IS_ERR(dw_wdt->rst))
+	if (dw_wdt->rst)
 		reset_control_deassert(dw_wdt->rst);
+	else
+		dev_warn(dev, "No reset lines. Will not be able to stop once started.\n");
 
 	return 0;
 
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 4/5] watchdog: dw_wdt: remove duplicated error message
  2020-02-10 18:08 [PATCH 1/5] mci: stm32_sdmmc2: don't ignore reset_control_get errors Ahmad Fatoum
  2020-02-10 18:08 ` [PATCH 2/5] net: designware: socfpga: fix possible invalid pointer deref Ahmad Fatoum
  2020-02-10 18:08 ` [PATCH 3/5] watchdog: dw_wdt: inform user on missing reset control line Ahmad Fatoum
@ 2020-02-10 18:08 ` Ahmad Fatoum
  2020-02-10 18:08 ` [PATCH 5/5] i2c: tegra: correct " Ahmad Fatoum
  2020-02-12  7:36 ` [PATCH 1/5] mci: stm32_sdmmc2: don't ignore reset_control_get errors Sascha Hauer
  4 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2020-02-10 18:08 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The wd command already warns that "Watchdog cannot be disabled",
when the stop operation returns -ENOSYS. We do that now, so telling the
user that it will not stop is superfluous. Remove it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/dw_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 2d1aa5e587ec..cb0d17e3610b 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -73,7 +73,7 @@ static int dw_wdt_stop(struct watchdog *wdd)
 	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
 
 	if (!dw_wdt->rst) {
-		dev_warn(dw_wdt->wdd.hwdev, "No reset line. Will not stop.\n");
+		dev_warn(dw_wdt->wdd.hwdev, "No reset line\n");
 		return -ENOSYS;
 	}
 
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 5/5] i2c: tegra: correct error message
  2020-02-10 18:08 [PATCH 1/5] mci: stm32_sdmmc2: don't ignore reset_control_get errors Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2020-02-10 18:08 ` [PATCH 4/5] watchdog: dw_wdt: remove duplicated error message Ahmad Fatoum
@ 2020-02-10 18:08 ` Ahmad Fatoum
  2020-02-12  7:36 ` [PATCH 1/5] mci: stm32_sdmmc2: don't ignore reset_control_get errors Sascha Hauer
  4 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2020-02-10 18:08 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

reset_control_get returns NULL when the controller reset is missing.
The error pointer is used for malformed reset controller specification.

Correct the message accordingly.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/i2c/busses/i2c-tegra.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index d56c0def6528..94c982d5c2b7 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -632,7 +632,7 @@ static int tegra_i2c_probe(struct device_d *dev)
 
 	i2c_dev->rst = reset_control_get(dev, "i2c");
 	if (IS_ERR(i2c_dev->rst)) {
-		dev_err(dev, "missing controller reset");
+		dev_err(dev, "invalid controller reset");
 		return PTR_ERR(i2c_dev->rst);
 	}
 
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/5] mci: stm32_sdmmc2: don't ignore reset_control_get errors
  2020-02-10 18:08 [PATCH 1/5] mci: stm32_sdmmc2: don't ignore reset_control_get errors Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2020-02-10 18:08 ` [PATCH 5/5] i2c: tegra: correct " Ahmad Fatoum
@ 2020-02-12  7:36 ` Sascha Hauer
  4 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2020-02-12  7:36 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Mon, Feb 10, 2020 at 07:08:30PM +0100, Ahmad Fatoum wrote:
> reset_control_get returns a NULL pointer when no resets were specified
> via device tree properties. If there's a malformed "resets"-property, we
> get an error pointer. This error should be propagated, only the NULL
> returns are the ones we can safely ignore.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/mci/stm32_sdmmc2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks

Sascha

> 
> diff --git a/drivers/mci/stm32_sdmmc2.c b/drivers/mci/stm32_sdmmc2.c
> index 44f7e6239949..695e61d1ccd7 100644
> --- a/drivers/mci/stm32_sdmmc2.c
> +++ b/drivers/mci/stm32_sdmmc2.c
> @@ -622,7 +622,7 @@ static int stm32_sdmmc2_probe(struct amba_device *adev,
>  
>  	priv->reset_ctl = reset_control_get(dev, NULL);
>  	if (IS_ERR(priv->reset_ctl))
> -		priv->reset_ctl = NULL;
> +		return PTR_ERR(priv->reset_ctl);
>  
>  	mci->f_min = 400000;
>  	/* f_max is taken from kernel v5.3 variant_stm32_sdmmc */
> -- 
> 2.25.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-12  7:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-10 18:08 [PATCH 1/5] mci: stm32_sdmmc2: don't ignore reset_control_get errors Ahmad Fatoum
2020-02-10 18:08 ` [PATCH 2/5] net: designware: socfpga: fix possible invalid pointer deref Ahmad Fatoum
2020-02-10 18:08 ` [PATCH 3/5] watchdog: dw_wdt: inform user on missing reset control line Ahmad Fatoum
2020-02-10 18:08 ` [PATCH 4/5] watchdog: dw_wdt: remove duplicated error message Ahmad Fatoum
2020-02-10 18:08 ` [PATCH 5/5] i2c: tegra: correct " Ahmad Fatoum
2020-02-12  7:36 ` [PATCH 1/5] mci: stm32_sdmmc2: don't ignore reset_control_get errors Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox