* [PATCH 07/14] I2C: ali15x3: Do PCI error checks on own line
[not found] <20230824132832.78705-1-ilpo.jarvinen@linux.intel.com>
@ 2023-08-24 13:28 ` Ilpo Järvinen
2023-08-24 16:00 ` Andi Shyti
2023-08-24 13:28 ` [PATCH 08/14] I2C: nforce2: Do PCI error check " Ilpo Järvinen
2023-08-24 13:28 ` [PATCH 09/14] I2C: sis5595: Do PCI error checks " Ilpo Järvinen
2 siblings, 1 reply; 9+ messages in thread
From: Ilpo Järvinen @ 2023-08-24 13:28 UTC (permalink / raw)
To: linux-pci, Bjorn Helgaas, Jean Delvare, Andi Shyti, linux-i2c,
linux-kernel
Cc: Ilpo Järvinen
Instead of if conditions with line splits, use the usual error handling
pattern with a separate variable to improve readability.
No functional changes intended.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/i2c/busses/i2c-ali15x3.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-ali15x3.c b/drivers/i2c/busses/i2c-ali15x3.c
index cc58feacd082..6fedecef9df3 100644
--- a/drivers/i2c/busses/i2c-ali15x3.c
+++ b/drivers/i2c/busses/i2c-ali15x3.c
@@ -122,6 +122,7 @@ static int ali15x3_setup(struct pci_dev *ALI15X3_dev)
{
u16 a;
unsigned char temp;
+ int ret;
/* Check the following things:
- SMB I/O address is initialized
@@ -167,12 +168,11 @@ static int ali15x3_setup(struct pci_dev *ALI15X3_dev)
if(force_addr) {
dev_info(&ALI15X3_dev->dev, "forcing ISA address 0x%04X\n",
ali15x3_smba);
- if (PCIBIOS_SUCCESSFUL != pci_write_config_word(ALI15X3_dev,
- SMBBA,
- ali15x3_smba))
+ ret = pci_write_config_word(ALI15X3_dev, SMBBA, ali15x3_smba);
+ if (ret != PCIBIOS_SUCCESSFUL)
goto error;
- if (PCIBIOS_SUCCESSFUL != pci_read_config_word(ALI15X3_dev,
- SMBBA, &a))
+ ret = pci_read_config_word(ALI15X3_dev, SMBBA, &a);
+ if (ret != PCIBIOS_SUCCESSFUL)
goto error;
if ((a & ~(ALI15X3_SMB_IOSIZE - 1)) != ali15x3_smba) {
/* make sure it works */
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 07/14] I2C: ali15x3: Do PCI error checks on own line
2023-08-24 13:28 ` [PATCH 07/14] I2C: ali15x3: Do PCI error checks on own line Ilpo Järvinen
@ 2023-08-24 16:00 ` Andi Shyti
2023-08-25 8:34 ` Ilpo Järvinen
0 siblings, 1 reply; 9+ messages in thread
From: Andi Shyti @ 2023-08-24 16:00 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: linux-pci, Bjorn Helgaas, Jean Delvare, linux-i2c, linux-kernel
Hi Ilpo,
On Thu, Aug 24, 2023 at 04:28:25PM +0300, Ilpo Järvinen wrote:
> Instead of if conditions with line splits, use the usual error handling
> pattern with a separate variable to improve readability.
>
> No functional changes intended.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> drivers/i2c/busses/i2c-ali15x3.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-ali15x3.c b/drivers/i2c/busses/i2c-ali15x3.c
> index cc58feacd082..6fedecef9df3 100644
> --- a/drivers/i2c/busses/i2c-ali15x3.c
> +++ b/drivers/i2c/busses/i2c-ali15x3.c
> @@ -122,6 +122,7 @@ static int ali15x3_setup(struct pci_dev *ALI15X3_dev)
> {
> u16 a;
> unsigned char temp;
> + int ret;
can you please add this ret declaration inside the
"if (force_addr)"?
Andi
>
> /* Check the following things:
> - SMB I/O address is initialized
> @@ -167,12 +168,11 @@ static int ali15x3_setup(struct pci_dev *ALI15X3_dev)
> if(force_addr) {
> dev_info(&ALI15X3_dev->dev, "forcing ISA address 0x%04X\n",
> ali15x3_smba);
> - if (PCIBIOS_SUCCESSFUL != pci_write_config_word(ALI15X3_dev,
> - SMBBA,
> - ali15x3_smba))
> + ret = pci_write_config_word(ALI15X3_dev, SMBBA, ali15x3_smba);
> + if (ret != PCIBIOS_SUCCESSFUL)
> goto error;
> - if (PCIBIOS_SUCCESSFUL != pci_read_config_word(ALI15X3_dev,
> - SMBBA, &a))
> + ret = pci_read_config_word(ALI15X3_dev, SMBBA, &a);
> + if (ret != PCIBIOS_SUCCESSFUL)
> goto error;
> if ((a & ~(ALI15X3_SMB_IOSIZE - 1)) != ali15x3_smba) {
> /* make sure it works */
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 07/14] I2C: ali15x3: Do PCI error checks on own line
2023-08-24 16:00 ` Andi Shyti
@ 2023-08-25 8:34 ` Ilpo Järvinen
0 siblings, 0 replies; 9+ messages in thread
From: Ilpo Järvinen @ 2023-08-25 8:34 UTC (permalink / raw)
To: Andi Shyti; +Cc: linux-pci, Bjorn Helgaas, Jean Delvare, linux-i2c, LKML
[-- Attachment #1: Type: text/plain, Size: 1766 bytes --]
On Thu, 24 Aug 2023, Andi Shyti wrote:
> On Thu, Aug 24, 2023 at 04:28:25PM +0300, Ilpo Järvinen wrote:
> > Instead of if conditions with line splits, use the usual error handling
> > pattern with a separate variable to improve readability.
> >
> > No functional changes intended.
> >
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> > drivers/i2c/busses/i2c-ali15x3.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-ali15x3.c b/drivers/i2c/busses/i2c-ali15x3.c
> > index cc58feacd082..6fedecef9df3 100644
> > --- a/drivers/i2c/busses/i2c-ali15x3.c
> > +++ b/drivers/i2c/busses/i2c-ali15x3.c
> > @@ -122,6 +122,7 @@ static int ali15x3_setup(struct pci_dev *ALI15X3_dev)
> > {
> > u16 a;
> > unsigned char temp;
> > + int ret;
>
> can you please add this ret declaration inside the
> "if (force_addr)"?
Sure.
Thanks for taking a look.
--
i.
> > /* Check the following things:
> > - SMB I/O address is initialized
> > @@ -167,12 +168,11 @@ static int ali15x3_setup(struct pci_dev *ALI15X3_dev)
> > if(force_addr) {
> > dev_info(&ALI15X3_dev->dev, "forcing ISA address 0x%04X\n",
> > ali15x3_smba);
> > - if (PCIBIOS_SUCCESSFUL != pci_write_config_word(ALI15X3_dev,
> > - SMBBA,
> > - ali15x3_smba))
> > + ret = pci_write_config_word(ALI15X3_dev, SMBBA, ali15x3_smba);
> > + if (ret != PCIBIOS_SUCCESSFUL)
> > goto error;
> > - if (PCIBIOS_SUCCESSFUL != pci_read_config_word(ALI15X3_dev,
> > - SMBBA, &a))
> > + ret = pci_read_config_word(ALI15X3_dev, SMBBA, &a);
> > + if (ret != PCIBIOS_SUCCESSFUL)
> > goto error;
> > if ((a & ~(ALI15X3_SMB_IOSIZE - 1)) != ali15x3_smba) {
> > /* make sure it works */
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 08/14] I2C: nforce2: Do PCI error check on own line
[not found] <20230824132832.78705-1-ilpo.jarvinen@linux.intel.com>
2023-08-24 13:28 ` [PATCH 07/14] I2C: ali15x3: Do PCI error checks on own line Ilpo Järvinen
@ 2023-08-24 13:28 ` Ilpo Järvinen
2023-08-24 16:02 ` Andi Shyti
2023-09-01 11:12 ` Jean Delvare
2023-08-24 13:28 ` [PATCH 09/14] I2C: sis5595: Do PCI error checks " Ilpo Järvinen
2 siblings, 2 replies; 9+ messages in thread
From: Ilpo Järvinen @ 2023-08-24 13:28 UTC (permalink / raw)
To: linux-pci, Bjorn Helgaas, Jean Delvare, Andi Shyti, linux-i2c,
linux-kernel
Cc: Ilpo Järvinen
Instead of a if condition with a line split, use the usual error
handling pattern with a separate variable to improve readability.
No functional changes intended.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/i2c/busses/i2c-nforce2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-nforce2.c b/drivers/i2c/busses/i2c-nforce2.c
index 777278386f58..38d203d93eee 100644
--- a/drivers/i2c/busses/i2c-nforce2.c
+++ b/drivers/i2c/busses/i2c-nforce2.c
@@ -327,8 +327,8 @@ static int nforce2_probe_smb(struct pci_dev *dev, int bar, int alt_reg,
/* Older incarnations of the device used non-standard BARs */
u16 iobase;
- if (pci_read_config_word(dev, alt_reg, &iobase)
- != PCIBIOS_SUCCESSFUL) {
+ error = pci_read_config_word(dev, alt_reg, &iobase);
+ if (error != PCIBIOS_SUCCESSFUL) {
dev_err(&dev->dev, "Error reading PCI config for %s\n",
name);
return -EIO;
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 08/14] I2C: nforce2: Do PCI error check on own line
2023-08-24 13:28 ` [PATCH 08/14] I2C: nforce2: Do PCI error check " Ilpo Järvinen
@ 2023-08-24 16:02 ` Andi Shyti
2023-09-01 11:12 ` Jean Delvare
1 sibling, 0 replies; 9+ messages in thread
From: Andi Shyti @ 2023-08-24 16:02 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: linux-pci, Bjorn Helgaas, Jean Delvare, linux-i2c, linux-kernel
Hi Ilpo,
On Thu, Aug 24, 2023 at 04:28:26PM +0300, Ilpo Järvinen wrote:
> Instead of a if condition with a line split, use the usual error
> handling pattern with a separate variable to improve readability.
>
> No functional changes intended.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
Andi
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 08/14] I2C: nforce2: Do PCI error check on own line
2023-08-24 13:28 ` [PATCH 08/14] I2C: nforce2: Do PCI error check " Ilpo Järvinen
2023-08-24 16:02 ` Andi Shyti
@ 2023-09-01 11:12 ` Jean Delvare
1 sibling, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2023-09-01 11:12 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: linux-pci, Bjorn Helgaas, Andi Shyti, linux-i2c, linux-kernel
On Thu, 24 Aug 2023 16:28:26 +0300, Ilpo Järvinen wrote:
> Instead of a if condition with a line split, use the usual error
> handling pattern with a separate variable to improve readability.
>
> No functional changes intended.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> drivers/i2c/busses/i2c-nforce2.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-nforce2.c b/drivers/i2c/busses/i2c-nforce2.c
> index 777278386f58..38d203d93eee 100644
> --- a/drivers/i2c/busses/i2c-nforce2.c
> +++ b/drivers/i2c/busses/i2c-nforce2.c
> @@ -327,8 +327,8 @@ static int nforce2_probe_smb(struct pci_dev *dev, int bar, int alt_reg,
> /* Older incarnations of the device used non-standard BARs */
> u16 iobase;
>
> - if (pci_read_config_word(dev, alt_reg, &iobase)
> - != PCIBIOS_SUCCESSFUL) {
> + error = pci_read_config_word(dev, alt_reg, &iobase);
> + if (error != PCIBIOS_SUCCESSFUL) {
> dev_err(&dev->dev, "Error reading PCI config for %s\n",
> name);
> return -EIO;
Reviewed-by: Jean Delvare <jdelvare@suse.de>
Thanks,
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 09/14] I2C: sis5595: Do PCI error checks on own line
[not found] <20230824132832.78705-1-ilpo.jarvinen@linux.intel.com>
2023-08-24 13:28 ` [PATCH 07/14] I2C: ali15x3: Do PCI error checks on own line Ilpo Järvinen
2023-08-24 13:28 ` [PATCH 08/14] I2C: nforce2: Do PCI error check " Ilpo Järvinen
@ 2023-08-24 13:28 ` Ilpo Järvinen
2023-08-24 16:04 ` Andi Shyti
2023-09-01 11:16 ` Jean Delvare
2 siblings, 2 replies; 9+ messages in thread
From: Ilpo Järvinen @ 2023-08-24 13:28 UTC (permalink / raw)
To: linux-pci, Bjorn Helgaas, Jean Delvare, Andi Shyti, linux-i2c,
linux-kernel
Cc: Ilpo Järvinen
Instead of if conditions with line splits, use the usual error handling
pattern with a separate variable to improve readability.
No functional changes intended.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/i2c/busses/i2c-sis5595.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/i2c-sis5595.c b/drivers/i2c/busses/i2c-sis5595.c
index c793a5c14cda..486f1e9dfb74 100644
--- a/drivers/i2c/busses/i2c-sis5595.c
+++ b/drivers/i2c/busses/i2c-sis5595.c
@@ -175,11 +175,11 @@ static int sis5595_setup(struct pci_dev *SIS5595_dev)
if (force_addr) {
dev_info(&SIS5595_dev->dev, "forcing ISA address 0x%04X\n", sis5595_base);
- if (pci_write_config_word(SIS5595_dev, ACPI_BASE, sis5595_base)
- != PCIBIOS_SUCCESSFUL)
+ retval = pci_write_config_word(SIS5595_dev, ACPI_BASE, sis5595_base);
+ if (retval != PCIBIOS_SUCCESSFUL)
goto error;
- if (pci_read_config_word(SIS5595_dev, ACPI_BASE, &a)
- != PCIBIOS_SUCCESSFUL)
+ retval = pci_read_config_word(SIS5595_dev, ACPI_BASE, &a);
+ if (retval != PCIBIOS_SUCCESSFUL)
goto error;
if ((a & ~(SIS5595_EXTENT - 1)) != sis5595_base) {
/* doesn't work for some chips! */
@@ -188,16 +188,16 @@ static int sis5595_setup(struct pci_dev *SIS5595_dev)
}
}
- if (pci_read_config_byte(SIS5595_dev, SIS5595_ENABLE_REG, &val)
- != PCIBIOS_SUCCESSFUL)
+ retval = pci_read_config_byte(SIS5595_dev, SIS5595_ENABLE_REG, &val);
+ if (retval != PCIBIOS_SUCCESSFUL)
goto error;
if ((val & 0x80) == 0) {
dev_info(&SIS5595_dev->dev, "enabling ACPI\n");
- if (pci_write_config_byte(SIS5595_dev, SIS5595_ENABLE_REG, val | 0x80)
- != PCIBIOS_SUCCESSFUL)
+ retval = pci_write_config_byte(SIS5595_dev, SIS5595_ENABLE_REG, val | 0x80);
+ if (retval != PCIBIOS_SUCCESSFUL)
goto error;
- if (pci_read_config_byte(SIS5595_dev, SIS5595_ENABLE_REG, &val)
- != PCIBIOS_SUCCESSFUL)
+ retval = pci_read_config_byte(SIS5595_dev, SIS5595_ENABLE_REG, &val);
+ if (retval != PCIBIOS_SUCCESSFUL)
goto error;
if ((val & 0x80) == 0) {
/* doesn't work for some chips? */
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 09/14] I2C: sis5595: Do PCI error checks on own line
2023-08-24 13:28 ` [PATCH 09/14] I2C: sis5595: Do PCI error checks " Ilpo Järvinen
@ 2023-08-24 16:04 ` Andi Shyti
2023-09-01 11:16 ` Jean Delvare
1 sibling, 0 replies; 9+ messages in thread
From: Andi Shyti @ 2023-08-24 16:04 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: linux-pci, Bjorn Helgaas, Jean Delvare, linux-i2c, linux-kernel
Hi Ilpo,
On Thu, Aug 24, 2023 at 04:28:27PM +0300, Ilpo Järvinen wrote:
> Instead of if conditions with line splits, use the usual error handling
> pattern with a separate variable to improve readability.
>
> No functional changes intended.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
Thanks,
Andi
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 09/14] I2C: sis5595: Do PCI error checks on own line
2023-08-24 13:28 ` [PATCH 09/14] I2C: sis5595: Do PCI error checks " Ilpo Järvinen
2023-08-24 16:04 ` Andi Shyti
@ 2023-09-01 11:16 ` Jean Delvare
1 sibling, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2023-09-01 11:16 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: linux-pci, Bjorn Helgaas, Andi Shyti, linux-i2c, linux-kernel
On Thu, 24 Aug 2023 16:28:27 +0300, Ilpo Järvinen wrote:
> Instead of if conditions with line splits, use the usual error handling
> pattern with a separate variable to improve readability.
>
> No functional changes intended.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> drivers/i2c/busses/i2c-sis5595.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-sis5595.c b/drivers/i2c/busses/i2c-sis5595.c
> index c793a5c14cda..486f1e9dfb74 100644
> --- a/drivers/i2c/busses/i2c-sis5595.c
> +++ b/drivers/i2c/busses/i2c-sis5595.c
> @@ -175,11 +175,11 @@ static int sis5595_setup(struct pci_dev *SIS5595_dev)
>
> if (force_addr) {
> dev_info(&SIS5595_dev->dev, "forcing ISA address 0x%04X\n", sis5595_base);
> - if (pci_write_config_word(SIS5595_dev, ACPI_BASE, sis5595_base)
> - != PCIBIOS_SUCCESSFUL)
> + retval = pci_write_config_word(SIS5595_dev, ACPI_BASE, sis5595_base);
> + if (retval != PCIBIOS_SUCCESSFUL)
> goto error;
> - if (pci_read_config_word(SIS5595_dev, ACPI_BASE, &a)
> - != PCIBIOS_SUCCESSFUL)
> + retval = pci_read_config_word(SIS5595_dev, ACPI_BASE, &a);
> + if (retval != PCIBIOS_SUCCESSFUL)
> goto error;
> if ((a & ~(SIS5595_EXTENT - 1)) != sis5595_base) {
> /* doesn't work for some chips! */
> @@ -188,16 +188,16 @@ static int sis5595_setup(struct pci_dev *SIS5595_dev)
> }
> }
>
> - if (pci_read_config_byte(SIS5595_dev, SIS5595_ENABLE_REG, &val)
> - != PCIBIOS_SUCCESSFUL)
> + retval = pci_read_config_byte(SIS5595_dev, SIS5595_ENABLE_REG, &val);
> + if (retval != PCIBIOS_SUCCESSFUL)
> goto error;
> if ((val & 0x80) == 0) {
> dev_info(&SIS5595_dev->dev, "enabling ACPI\n");
> - if (pci_write_config_byte(SIS5595_dev, SIS5595_ENABLE_REG, val | 0x80)
> - != PCIBIOS_SUCCESSFUL)
> + retval = pci_write_config_byte(SIS5595_dev, SIS5595_ENABLE_REG, val | 0x80);
> + if (retval != PCIBIOS_SUCCESSFUL)
> goto error;
> - if (pci_read_config_byte(SIS5595_dev, SIS5595_ENABLE_REG, &val)
> - != PCIBIOS_SUCCESSFUL)
> + retval = pci_read_config_byte(SIS5595_dev, SIS5595_ENABLE_REG, &val);
> + if (retval != PCIBIOS_SUCCESSFUL)
> goto error;
> if ((val & 0x80) == 0) {
> /* doesn't work for some chips? */
Reviewed-by: Jean Delvare <jdelvare@suse.de>
Thanks,
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 9+ messages in thread