[PATCH] spi: spi-nxp-fspi: Fix a NULL vs IS_ERR() check in probe

[PATCH] spi: spi-nxp-fspi: Fix a NULL vs IS_ERR() check in probe

[Dan Carpenter]

The platform_get_resource_byname() function returns NULL on error, it
doesn't return error pointers.

Fixes: d166a73503ef ("spi: fspi: dynamically alloc AHB memory")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
The commit message for commit d166a73503ef ("spi: fspi: dynamically
alloc AHB memory") is not very good.  Why is it necessary to allocate
the AHB memory dynamically instead of during probe?  Also I suspect that
Adam should have recieved authorship credit for that patch.

 drivers/spi/spi-nxp-fspi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index 019f40e2917c..1ccda82da206 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -1019,8 +1019,8 @@ static int nxp_fspi_probe(struct platform_device *pdev)
 
 	/* find the resources - controller memory mapped space */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fspi_mmap");
-	if (IS_ERR(res)) {
-		ret = PTR_ERR(res);
+	if (!res) {
+		ret = -ENODEV;
 		goto err_put_ctrl;
 	}
 
-- 
2.20.1

Re: [PATCH] spi: spi-nxp-fspi: Fix a NULL vs IS_ERR() check in probe

[Adam Ford]

On Thu, Mar 12, 2020 at 6:32 AM Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
>
> The platform_get_resource_byname() function returns NULL on error, it
> doesn't return error pointers.
>
> Fixes: d166a73503ef ("spi: fspi: dynamically alloc AHB memory")
> Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
> The commit message for commit d166a73503ef ("spi: fspi: dynamically
> alloc AHB memory") is not very good.  Why is it necessary to allocate
> the AHB memory dynamically instead of during probe?  Also I suspect that
> Adam should have recieved authorship credit for that patch.

It wasn't my patch, I just pulled it in from NXP's repo.  The true
author is Han Xu.  When I pulled in the series from NXP, I found the
flexSPI on the i.MX8MM to become functional, and my company has a
board with a qspi flash on it.

adam

>
>  drivers/spi/spi-nxp-fspi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> index 019f40e2917c..1ccda82da206 100644
> --- a/drivers/spi/spi-nxp-fspi.c
> +++ b/drivers/spi/spi-nxp-fspi.c
> @@ -1019,8 +1019,8 @@ static int nxp_fspi_probe(struct platform_device *pdev)
>
>         /* find the resources - controller memory mapped space */
>         res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fspi_mmap");
> -       if (IS_ERR(res)) {
> -               ret = PTR_ERR(res);
> +       if (!res) {
> +               ret = -ENODEV;
>                 goto err_put_ctrl;
>         }
>
> --
> 2.20.1
>

Re: [PATCH] spi: spi-nxp-fspi: Fix a NULL vs IS_ERR() check in probe

[Fabio Estevam]

Hi Dan,

On Thu, Mar 12, 2020 at 8:33 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> The platform_get_resource_byname() function returns NULL on error, it
> doesn't return error pointers.
>
> Fixes: d166a73503ef ("spi: fspi: dynamically alloc AHB memory")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> The commit message for commit d166a73503ef ("spi: fspi: dynamically
> alloc AHB memory") is not very good.  Why is it necessary to allocate
> the AHB memory dynamically instead of during probe?  Also I suspect that

Agreed and I made the same comment during review:
https://patchwork.kernel.org/patch/11361581/

Thanks

Re: [PATCH] spi: spi-nxp-fspi: Fix a NULL vs IS_ERR() check in probe

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]

On Thu, Mar 12, 2020 at 09:11:44AM -0300, Fabio Estevam wrote:
> On Thu, Mar 12, 2020 at 8:33 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:

> > The commit message for commit d166a73503ef ("spi: fspi: dynamically
> > alloc AHB memory") is not very good.  Why is it necessary to allocate
> > the AHB memory dynamically instead of during probe?  Also I suspect that

> Agreed and I made the same comment during review:
> https://patchwork.kernel.org/patch/11361581/

Indeed.  There had been an earlier discussion of what it was doing IIRC
but it didn't make it's way into the changelog.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Applied "spi: spi-nxp-fspi: Fix a NULL vs IS_ERR() check in probe" to the spi tree

[Mark Brown]

The patch

   spi: spi-nxp-fspi: Fix a NULL vs IS_ERR() check in probe

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 1a421ebab6bb5bf65001743ba9fef48e94fb345a Mon Sep 17 00:00:00 2001
From: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Date: Thu, 12 Mar 2020 14:31:54 +0300
Subject: [PATCH] spi: spi-nxp-fspi: Fix a NULL vs IS_ERR() check in probe

The platform_get_resource_byname() function returns NULL on error, it
doesn't return error pointers.

Fixes: d166a73503ef ("spi: fspi: dynamically alloc AHB memory")
Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Link: https://lore.kernel.org/r/20200312113154.GC20562@mwanda
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-nxp-fspi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index 019f40e2917c..1ccda82da206 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -1019,8 +1019,8 @@ static int nxp_fspi_probe(struct platform_device *pdev)
 
 	/* find the resources - controller memory mapped space */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fspi_mmap");
-	if (IS_ERR(res)) {
-		ret = PTR_ERR(res);
+	if (!res) {
+		ret = -ENODEV;
 		goto err_put_ctrl;
 	}
 
-- 
2.20.1

Re: [PATCH] spi: spi-nxp-fspi: Fix a NULL vs IS_ERR() check in probe

[Dan Carpenter]

On Thu, Mar 12, 2020 at 06:58:31AM -0500, Adam Ford wrote:
> On Thu, Mar 12, 2020 at 6:32 AM Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> >
> > The platform_get_resource_byname() function returns NULL on error, it
> > doesn't return error pointers.
> >
> > Fixes: d166a73503ef ("spi: fspi: dynamically alloc AHB memory")
> > Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> > The commit message for commit d166a73503ef ("spi: fspi: dynamically
> > alloc AHB memory") is not very good.  Why is it necessary to allocate
> > the AHB memory dynamically instead of during probe?  Also I suspect that
> > Adam should have recieved authorship credit for that patch.
> 
> It wasn't my patch, I just pulled it in from NXP's repo.  The true
> author is Han Xu.  When I pulled in the series from NXP, I found the
> flexSPI on the i.MX8MM to become functional, and my company has a
> board with a qspi flash on it.

You should have put your Signed-off-by after Han Xu's.  They should be
in chronological order so whoever handles a patch adds their S-o-b to
the end.

regards,
dan carpenter

Re: [EXT] Re: [PATCH] spi: spi-nxp-fspi: Fix a NULL vs IS_ERR() check in probe

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

On Fri, Mar 13, 2020 at 04:16:44AM +0000, Ashish Kumar wrote:

> This patch needs to drop, until further debugged, since flexspi is
> shared ip it is not having desired result on few boards, but it is
> needed for i.mx series, This is one of the comments from han xu. I
> believe further modifications will be needed, if not commit msg and
> other things can be update.

Please send a patch doing the revert with a changelog explaining why
it's needed.

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]