| Message ID | 20231120084606.4083194-2-claudiu.beznea.uj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ravb: Add suspend to RAM and runtime PM support for RZ/G3S | expand |
On 11/20/23 11:45 AM, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > reset_control_deassert() could return an error. Some devices cannot work > if reset signal de-assert operation fails. To avoid this check the return > code of reset_control_deassert() in ravb_probe() and take proper action. > > Fixes: 0d13a1a464a0 ("ravb: Add reset support") > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- > drivers/net/ethernet/renesas/ravb_main.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index c70cff80cc99..342978bdbd7e 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c > @@ -2645,7 +2645,12 @@ static int ravb_probe(struct platform_device *pdev) > ndev->features = info->net_features; > ndev->hw_features = info->net_hw_features; > > - reset_control_deassert(rstc); > + error = reset_control_deassert(rstc); > + if (error) { > + free_netdev(ndev); > + return error; No, please use *goto* here. And please fix up the order of statements under the out_release label before doing that. [...] MBR, Sergey
On 20.11.2023 21:03, Sergey Shtylyov wrote: > On 11/20/23 11:45 AM, Claudiu wrote: > >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> reset_control_deassert() could return an error. Some devices cannot work >> if reset signal de-assert operation fails. To avoid this check the return >> code of reset_control_deassert() in ravb_probe() and take proper action. >> >> Fixes: 0d13a1a464a0 ("ravb: Add reset support") >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> --- >> drivers/net/ethernet/renesas/ravb_main.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c >> index c70cff80cc99..342978bdbd7e 100644 >> --- a/drivers/net/ethernet/renesas/ravb_main.c >> +++ b/drivers/net/ethernet/renesas/ravb_main.c >> @@ -2645,7 +2645,12 @@ static int ravb_probe(struct platform_device *pdev) >> ndev->features = info->net_features; >> ndev->hw_features = info->net_hw_features; >> >> - reset_control_deassert(rstc); >> + error = reset_control_deassert(rstc); >> + if (error) { >> + free_netdev(ndev); >> + return error; > > No, please use *goto* here. And please fix up the order of statements under > the out_release label before doing that. This will lead to a bit more complicated patch, AFAICT. I tried to keep it simple for a fix. Same thing for patch 2. > > [...] > > MBR, Sergey
On 11/21/23 8:59 AM, claudiu beznea wrote: [...] >>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >>> >>> reset_control_deassert() could return an error. Some devices cannot work >>> if reset signal de-assert operation fails. To avoid this check the return >>> code of reset_control_deassert() in ravb_probe() and take proper action. >>> >>> Fixes: 0d13a1a464a0 ("ravb: Add reset support") >>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >>> --- >>> drivers/net/ethernet/renesas/ravb_main.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c >>> index c70cff80cc99..342978bdbd7e 100644 >>> --- a/drivers/net/ethernet/renesas/ravb_main.c >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c >>> @@ -2645,7 +2645,12 @@ static int ravb_probe(struct platform_device *pdev) >>> ndev->features = info->net_features; >>> ndev->hw_features = info->net_hw_features; >>> >>> - reset_control_deassert(rstc); >>> + error = reset_control_deassert(rstc); >>> + if (error) { >>> + free_netdev(ndev); >>> + return error; >> >> No, please use *goto* here. And please fix up the order of statements under >> the out_release label before doing that. > > This will lead to a bit more complicated patch, AFAICT. I tried to keep it It's OK! :-) > simple for a fix. Same thing for patch 2. Patch 2 is not as simple as it could have been... [...] MBR, Sergey
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index c70cff80cc99..342978bdbd7e 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -2645,7 +2645,12 @@ static int ravb_probe(struct platform_device *pdev) ndev->features = info->net_features; ndev->hw_features = info->net_hw_features; - reset_control_deassert(rstc); + error = reset_control_deassert(rstc); + if (error) { + free_netdev(ndev); + return error; + } + pm_runtime_enable(&pdev->dev); pm_runtime_get_sync(&pdev->dev);