[PATCH] cxl: Fix error handling

[PATCH] cxl: Fix error handling

[Christophe JAILLET]

'cxl_dev_context_init()' returns an error pointer in case of error, not
NULL. So test it with IS_ERR.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
un-compiled because I don't have the required  cross build environment.
---
 drivers/misc/cxl/pci.c | 2 +-
 drivers/misc/cxl/phb.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index e96be9ca4e60..80a87ab25b83 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -1921,7 +1921,7 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
 				goto err;
 
 			ctx = cxl_dev_context_init(afu_dev);
-			if (!ctx)
+			if (IS_ERR(ctx))
 				goto err;
 
 			afu_dev->dev.archdata.cxl_ctx = ctx;
diff --git a/drivers/misc/cxl/phb.c b/drivers/misc/cxl/phb.c
index 0935d44c1770..6ec69ada19f4 100644
--- a/drivers/misc/cxl/phb.c
+++ b/drivers/misc/cxl/phb.c
@@ -20,7 +20,7 @@ bool _cxl_pci_associate_default_context(struct pci_dev *dev, struct cxl_afu *afu
 	 * in the virtual phb, we'll need a default context to attach them to.
 	 */
 	ctx = cxl_dev_context_init(dev);
-	if (!ctx)
+	if (IS_ERR(ctx))
 		return false;
 	dev->dev.archdata.cxl_ctx = ctx;
 
-- 
2.9.3

Re: [PATCH] cxl: Fix error handling

[Michael Ellerman]

Christophe JAILLET <christophe.jaillet@wanadoo.fr> writes:

> 'cxl_dev_context_init()' returns an error pointer in case of error, not
> NULL. So test it with IS_ERR.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> un-compiled because I don't have the required  cross build environment.

Do you run Ubuntu or Fedora? If so it's just a dnf/apt-get away:

$ sudo dnf install gcc-c++-powerpc64-linux-gnu binutils-powerpc64-linux-gnu gcc-powerpc64-linux-gnu
or
$ sudo apt-get install gcc-powerpc64le-linux-gnu gcc-powerpc-linux-gnu libc-dev-powerpc-cross libc-dev-ppc64el-cross

More here:

https://github.com/linuxppc/linux/wiki/Building-powerpc-kernels


cheers

Re: [PATCH] cxl: Fix error handling

[Andrew Donnellan]

On 31/10/16 08:40, Christophe JAILLET wrote:
> 'cxl_dev_context_init()' returns an error pointer in case of error, not
> NULL. So test it with IS_ERR.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

> ---
> un-compiled because I don't have the required  cross build environment.

Builds for me.


-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

Re: [PATCH] cxl: Fix error handling

[Jim Davis]

On Sun, Oct 30, 2016 at 10:37 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Christophe JAILLET <christophe.jaillet@wanadoo.fr> writes:
>
>> 'cxl_dev_context_init()' returns an error pointer in case of error, not
>> NULL. So test it with IS_ERR.
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>> un-compiled because I don't have the required  cross build environment.
>
> Do you run Ubuntu or Fedora? If so it's just a dnf/apt-get away:
>
> $ sudo dnf install gcc-c++-powerpc64-linux-gnu binutils-powerpc64-linux-gnu gcc-powerpc64-linux-gnu
> or
> $ sudo apt-get install gcc-powerpc64le-linux-gnu gcc-powerpc-linux-gnu libc-dev-powerpc-cross libc-dev-ppc64el-cross
>
> More here:
>
> https://github.com/linuxppc/linux/wiki/Building-powerpc-kernels

Cool; the little-endian build worked fine, but

jim@krebstar:~/linux-rc$ make ARCH=powerpc
CROSS_COMPILE=powerpc64-linux-gnu- vmlinux
make: powerpc64-linux-gnu-gcc: Command not found
make: powerpc64-linux-gnu-gcc: Command not found
scripts/kconfig/conf  --silentoldconfig Kconfig
make: powerpc64-linux-gnu-gcc: Command not found

This is on Ubuntu 16.04; there's a /usr/bin/powerpc64le-linux-gnu-gcc
from installing gcc-powerpc64le-linux-gnu, and a
/usr/bin/powerpc-linux-gnu-gcc from installing gcc-powerpc-linux-gnu,
but no /usr/bin/powerpc64-linux-gnu-gcc.

-- 
Jim

Re: [PATCH] cxl: Fix error handling

[Michael Ellerman]

Jim Davis <jim.epost@gmail.com> writes:

> On Sun, Oct 30, 2016 at 10:37 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> More here:
>>
>> https://github.com/linuxppc/linux/wiki/Building-powerpc-kernels
>
> Cool; the little-endian build worked fine, but

Yay, thanks for trying.

> jim@krebstar:~/linux-rc$ make ARCH=powerpc
> CROSS_COMPILE=powerpc64-linux-gnu- vmlinux
> make: powerpc64-linux-gnu-gcc: Command not found
> make: powerpc64-linux-gnu-gcc: Command not found
> scripts/kconfig/conf  --silentoldconfig Kconfig
> make: powerpc64-linux-gnu-gcc: Command not found

Ah sorry.

> This is on Ubuntu 16.04; there's a /usr/bin/powerpc64le-linux-gnu-gcc
> from installing gcc-powerpc64le-linux-gnu, and a
> /usr/bin/powerpc-linux-gnu-gcc from installing gcc-powerpc-linux-gnu,
> but no /usr/bin/powerpc64-linux-gnu-gcc.

It's the powerpc-linux-gnu-gcc one.

That is a 32 and 64-bit compiler, it's 32-bit by default, but the kernel
Makefiles will pass -m64 appropriately.

You can actually build a single compiler that builds 32/64-bit BE, and
64-bit LE, but the distros don't do that for whatever reason.

cheers

Re: [PATCH] cxl: Fix error handling

[Frederic Barrat]

Le 30/10/2016 à 22:40, Christophe JAILLET a écrit :
> 'cxl_dev_context_init()' returns an error pointer in case of error, not
> NULL. So test it with IS_ERR.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---


Thanks for the 3 patches!

Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>

Re: [PATCH] cxl: Fix error handling

[Jim Davis]

On Wed, Nov 2, 2016 at 4:12 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Jim Davis <jim.epost@gmail.com> writes:
>
>> On Sun, Oct 30, 2016 at 10:37 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>>> More here:
>>>
>>> https://github.com/linuxppc/linux/wiki/Building-powerpc-kernels
>>
>> Cool; the little-endian build worked fine, but
>
> Yay, thanks for trying.
>
>> jim@krebstar:~/linux-rc$ make ARCH=powerpc
>> CROSS_COMPILE=powerpc64-linux-gnu- vmlinux
>> make: powerpc64-linux-gnu-gcc: Command not found
>> make: powerpc64-linux-gnu-gcc: Command not found
>> scripts/kconfig/conf  --silentoldconfig Kconfig
>> make: powerpc64-linux-gnu-gcc: Command not found

But I cleverly tried to run the Fedora command on Ubuntu... when I run
the right command for Ubuntu, the build worked just fine.  D'oh!

Nit: make distclean missed a few files

jim@krebstar:~/linux-rc$ make distclean; git clean -fdx
  CLEAN   .
  CLEAN   drivers/tty/vt
  CLEAN   drivers/video/logo
  CLEAN   firmware
  CLEAN   kernel
  CLEAN   lib
  CLEAN   usr
  CLEAN   .tmp_versions
  CLEAN   scripts/basic
  CLEAN   scripts/dtc
  CLEAN   scripts/genksyms
  CLEAN   scripts/kconfig
  CLEAN   scripts/mod
  CLEAN   scripts
  CLEAN   include/config include/generated arch/powerpc/include/generated
  CLEAN   .config .version Module.symvers
Removing arch/powerpc/kernel/systbl_chk.i
Removing arch/powerpc/kernel/vdso32/vdso32.lds
Removing arch/powerpc/kernel/vdso32/vdso32.so
Removing arch/powerpc/kernel/vdso32/vdso32.so.dbg
Removing arch/powerpc/kernel/vdso64/vdso64.lds
Removing arch/powerpc/kernel/vdso64/vdso64.so
Removing arch/powerpc/kernel/vdso64/vdso64.so.dbg
Removing arch/powerpc/kernel/vmlinux.lds

-- 
Jim

Re: [PATCH] cxl: Fix error handling

[Michael Ellerman]

Jim Davis <jim.epost@gmail.com> writes:

> On Wed, Nov 2, 2016 at 4:12 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Jim Davis <jim.epost@gmail.com> writes:
>>
>>> On Sun, Oct 30, 2016 at 10:37 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>>>> More here:
>>>>
>>>> https://github.com/linuxppc/linux/wiki/Building-powerpc-kernels
>>>
>>> Cool; the little-endian build worked fine, but
>>
>> Yay, thanks for trying.
>>
>>> jim@krebstar:~/linux-rc$ make ARCH=powerpc
>>> CROSS_COMPILE=powerpc64-linux-gnu- vmlinux
>>> make: powerpc64-linux-gnu-gcc: Command not found
>>> make: powerpc64-linux-gnu-gcc: Command not found
>>> scripts/kconfig/conf  --silentoldconfig Kconfig
>>> make: powerpc64-linux-gnu-gcc: Command not found
>
> But I cleverly tried to run the Fedora command on Ubuntu... when I run
> the right command for Ubuntu, the build worked just fine.  D'oh!
>
> Nit: make distclean missed a few files

Ah thanks. I literally never do that, but it is supposed work. Have
filed a bug for it.

cheers

Re: [PATCH] cxl: Fix error handling

[Ian Munsie]

Acked-by: Ian Munsie <imunsie@au1.ibm.com>

Re: cxl: Fix error handling

[Michael Ellerman]

On Sun, 2016-10-30 at 21:40:47 UTC, Christophe Jaillet wrote:
> 'cxl_dev_context_init()' returns an error pointer in case of error, not
> NULL. So test it with IS_ERR.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> Acked-by: Ian Munsie <imunsie@au1.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/bb81733de28c99e10b61dcaff15921

cheers

Re: [PATCH] cxl: Fix error handling

[Ian Munsie]

Acked-by: Ian Munsie <imunsie@au1.ibm.com>

Re: [PATCH] cxl: Fix error handling

[Michael Ellerman]

Andrew Donnellan <andrew.donnellan@au1.ibm.com> writes:

> On 31/10/16 08:34, Christophe JAILLET wrote:
>> 'cxl_dev_context_init()' returns an error pointer in case of error, not
>> NULL. So test it with IS_ERR.
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>
> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>
>> ---
>> un-compiled because I don't have the required  cross build environment.
>
> Builds fine here.
>
> In future, you might want to bundle all 3 of your patches either into 1 
> patch (while they are changes which can be done separately, it's all 
> fixes to one type of problem in relation to one function, so personally 
> I'm okay with that) or send it as a series, so that it's clear to 
> everyone that they're all related fixes.

> Additionally, having the same patch subject line for two patches sent in 
> short succession is a bit confusing - in future, try and avoid that.

Yeah. I actually assumed it was two versions of the same patch.

cheers

Re: [PATCH] cxl: Fix error handling

[Andrew Donnellan]

On 31/10/16 08:34, Christophe JAILLET wrote:
> 'cxl_dev_context_init()' returns an error pointer in case of error, not
> NULL. So test it with IS_ERR.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

> ---
> un-compiled because I don't have the required  cross build environment.

Builds fine here.

In future, you might want to bundle all 3 of your patches either into 1 
patch (while they are changes which can be done separately, it's all 
fixes to one type of problem in relation to one function, so personally 
I'm okay with that) or send it as a series, so that it's clear to 
everyone that they're all related fixes.

Additionally, having the same patch subject line for two patches sent in 
short succession is a bit confusing - in future, try and avoid that.


Andrew

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

[PATCH] cxl: Fix error handling

[Christophe JAILLET]

'cxl_dev_context_init()' returns an error pointer in case of error, not
NULL. So test it with IS_ERR.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
un-compiled because I don't have the required  cross build environment.
---
 drivers/misc/cxl/api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index 2b88ad8a2a89..e2efc6489c6e 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -538,7 +538,7 @@ int _cxl_cx4_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
 
 		if (remaining > 0) {
 			new_ctx = cxl_dev_context_init(pdev);
-			if (!new_ctx) {
+			if (IS_ERR(new_ctx)) {
 				pr_warn("%s: Failed to allocate enough contexts for MSIs\n", pci_name(pdev));
 				return -ENOSPC;
 			}
-- 
2.9.3