linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] soc: aspeed: fix probe error handling
@ 2019-06-19 12:56 Arnd Bergmann
  2019-06-20  8:29 ` Joel Stanley
  0 siblings, 1 reply; 3+ messages in thread
From: Arnd Bergmann @ 2019-06-19 12:56 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Arnd Bergmann, linux-aspeed, Andrew Jeffery, Patrick Venture,
	linux-kernel, Vijay Khemka, linux-arm-kernel

gcc warns that a mising "flash" phandle node leads to undefined
behavior later:

drivers/soc/aspeed/aspeed-lpc-ctrl.c: In function 'aspeed_lpc_ctrl_probe':
drivers/soc/aspeed/aspeed-lpc-ctrl.c:201:18: error: '*((void *)&resm+8)' may be used uninitialized in this function [-Werror=maybe-uninitialized]

The device cannot work without this node, so just error out here.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/soc/aspeed/aspeed-lpc-ctrl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/soc/aspeed/aspeed-lpc-ctrl.c b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
index 239520bb207e..81109d22af6a 100644
--- a/drivers/soc/aspeed/aspeed-lpc-ctrl.c
+++ b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
@@ -212,6 +212,7 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
 	node = of_parse_phandle(dev->of_node, "flash", 0);
 	if (!node) {
 		dev_dbg(dev, "Didn't find host pnor flash node\n");
+		return -ENXIO;
 	} else {
 		rc = of_address_to_resource(node, 1, &resm);
 		of_node_put(node);
-- 
2.20.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] soc: aspeed: fix probe error handling
  2019-06-19 12:56 [PATCH] soc: aspeed: fix probe error handling Arnd Bergmann
@ 2019-06-20  8:29 ` Joel Stanley
  2019-06-21 16:53   ` Vijay Khemka
  0 siblings, 1 reply; 3+ messages in thread
From: Joel Stanley @ 2019-06-20  8:29 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-aspeed, Andrew Jeffery, Patrick Venture,
	Linux Kernel Mailing List, Vijay Khemka, Linux ARM

On Wed, 19 Jun 2019 at 12:56, Arnd Bergmann <arnd@arndb.de> wrote:
>
> gcc warns that a mising "flash" phandle node leads to undefined
> behavior later:
>
> drivers/soc/aspeed/aspeed-lpc-ctrl.c: In function 'aspeed_lpc_ctrl_probe':
> drivers/soc/aspeed/aspeed-lpc-ctrl.c:201:18: error: '*((void *)&resm+8)' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> The device cannot work without this node, so just error out here.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks Arnd. This looks like it applies on top of Vijay's recent patch?

The intent of that change was to make the driver usable for systems
that do not want to depend on the flash phandle. I think the fix we
want looks like this:

--- a/drivers/soc/aspeed/aspeed-lpc-ctrl.c
+++ b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
@@ -224,10 +224,11 @@ static int aspeed_lpc_ctrl_probe(struct
platform_device *pdev)
                        dev_err(dev, "Couldn't address to resource for
flash\n");
                        return rc;
                }
+
+               lpc_ctrl->pnor_size = resource_size(&resm);
+               lpc_ctrl->pnor_base = resm.start;
        }

-       lpc_ctrl->pnor_size = resource_size(&resm);
-       lpc_ctrl->pnor_base = resm.start;


Vijay, do you agree?

Cheers,

Joel

> ---
>  drivers/soc/aspeed/aspeed-lpc-ctrl.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/soc/aspeed/aspeed-lpc-ctrl.c b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
> index 239520bb207e..81109d22af6a 100644
> --- a/drivers/soc/aspeed/aspeed-lpc-ctrl.c
> +++ b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
> @@ -212,6 +212,7 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
>         node = of_parse_phandle(dev->of_node, "flash", 0);
>         if (!node) {
>                 dev_dbg(dev, "Didn't find host pnor flash node\n");
> +               return -ENXIO;
>         } else {
>                 rc = of_address_to_resource(node, 1, &resm);
>                 of_node_put(node);
> --
> 2.20.0
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] soc: aspeed: fix probe error handling
  2019-06-20  8:29 ` Joel Stanley
@ 2019-06-21 16:53   ` Vijay Khemka
  0 siblings, 0 replies; 3+ messages in thread
From: Vijay Khemka @ 2019-06-21 16:53 UTC (permalink / raw)
  To: Joel Stanley, Arnd Bergmann
  Cc: Andrew Jeffery, Patrick Venture, linux-aspeed, Linux ARM,
	Linux Kernel Mailing List



On 6/20/19, 1:29 AM, "Joel Stanley" <joel@jms.id.au> wrote:

    On Wed, 19 Jun 2019 at 12:56, Arnd Bergmann <arnd@arndb.de> wrote:
    >
    > gcc warns that a mising "flash" phandle node leads to undefined
    > behavior later:
    >
    > drivers/soc/aspeed/aspeed-lpc-ctrl.c: In function 'aspeed_lpc_ctrl_probe':
    > drivers/soc/aspeed/aspeed-lpc-ctrl.c:201:18: error: '*((void *)&resm+8)' may be used uninitialized in this function [-Werror=maybe-uninitialized]
    >
    > The device cannot work without this node, so just error out here.
    >
    > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
    
    Thanks Arnd. This looks like it applies on top of Vijay's recent patch?
    
    The intent of that change was to make the driver usable for systems
    that do not want to depend on the flash phandle. I think the fix we
    want looks like this:
    
    --- a/drivers/soc/aspeed/aspeed-lpc-ctrl.c
    +++ b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
    @@ -224,10 +224,11 @@ static int aspeed_lpc_ctrl_probe(struct
    platform_device *pdev)
                            dev_err(dev, "Couldn't address to resource for
    flash\n");
                            return rc;
                    }
    +
    +               lpc_ctrl->pnor_size = resource_size(&resm);
    +               lpc_ctrl->pnor_base = resm.start;
            }
    
    -       lpc_ctrl->pnor_size = resource_size(&resm);
    -       lpc_ctrl->pnor_base = resm.start;
    
    
    Vijay, do you agree?
Yes Joel. Thanks for this.
    
    Cheers,
    
    Joel
    
    > ---
    >  drivers/soc/aspeed/aspeed-lpc-ctrl.c | 1 +
    >  1 file changed, 1 insertion(+)
    >
    > diff --git a/drivers/soc/aspeed/aspeed-lpc-ctrl.c b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
    > index 239520bb207e..81109d22af6a 100644
    > --- a/drivers/soc/aspeed/aspeed-lpc-ctrl.c
    > +++ b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
    > @@ -212,6 +212,7 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
    >         node = of_parse_phandle(dev->of_node, "flash", 0);
    >         if (!node) {
    >                 dev_dbg(dev, "Didn't find host pnor flash node\n");
    > +               return -ENXIO;
    >         } else {
    >                 rc = of_address_to_resource(node, 1, &resm);
    >                 of_node_put(node);
    > --
    > 2.20.0
    >
    

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-06-21 16:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-19 12:56 [PATCH] soc: aspeed: fix probe error handling Arnd Bergmann
2019-06-20  8:29 ` Joel Stanley
2019-06-21 16:53   ` Vijay Khemka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).