2012-11-06 00:27:44

by Stephen Rothwell

[permalink] [raw]
Subject: linux-next: build failure after merge of the pci tree

Hi Bjorn,

After merging the pci tree, today's linux-next build (x86_64 allmodconfig)
failed like this:

drivers/staging/telephony/ixj.c: In function 'ixj_probe_pci':
drivers/staging/telephony/ixj.c:7732:13: warning: assignment makes integer from pointer without a cast [enabled by default]
drivers/staging/telephony/ixj.c:7732:38: error: expected ';' before 'pci_resource_start'

Exposed by commit 545974a28f78 ("PCI: Convert pci_resource_<foo> macros to
static inlines"). The macro version of pci_resource_start() made this
RHS look like a function call and now it isn't.

Maybe it is time this driver just went away.

drivers/scsi/gdth.c: In function 'gdth_init_pci':
drivers/scsi/gdth.c:1111:34: error: lvalue required as left operand of assignment

This was also exposed by the above commit, but is caused by the driver
expecting to be able to assign to the result of pci_resource_start().

I have applied the following patch for today (the scsi one could probably
be done more correctly):

From: Stephen Rothwell <[email protected]>
Date: Tue, 6 Nov 2012 11:23:45 +1100
Subject: [PATCH] PCI: fixups for pci_resource_start conversion

Signed-off-by: Stephen Rothwell <[email protected]>
---
drivers/scsi/gdth.c | 2 +-
drivers/staging/telephony/Kconfig | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 5d72274..5209e81 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -1108,7 +1108,7 @@ static int __devinit gdth_init_pci(struct pci_dev *pdev, gdth_pci_str *pcistr,
command |= 6;
pci_write_config_word(pdev, PCI_COMMAND, command);
if (pci_resource_start(pdev, 8) == 1UL)
- pci_resource_start(pdev, 8) = 0UL;
+ pdev->resource[8].start = 0UL;
i = 0xFEFF0001UL;
pci_write_config_dword(pdev, PCI_ROM_ADDRESS, i);
gdth_delay(1);
diff --git a/drivers/staging/telephony/Kconfig b/drivers/staging/telephony/Kconfig
index b5f78b6..c5893e2 100644
--- a/drivers/staging/telephony/Kconfig
+++ b/drivers/staging/telephony/Kconfig
@@ -20,6 +20,7 @@ if PHONE
config PHONE_IXJ
tristate "QuickNet Internet LineJack/PhoneJack support"
depends on ISA || PCI
+ depends on BROKEN
---help---
Say M if you have a telephony card manufactured by Quicknet
Technologies, Inc. These include the Internet PhoneJACK and
--
1.7.10.280.gaa39

--
Cheers,
Stephen Rothwell [email protected]


Attachments:
(No filename) (2.39 kB)
(No filename) (836.00 B)
Download all attachments

2012-11-06 01:55:51

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: linux-next: build failure after merge of the pci tree

On Tue, Nov 06, 2012 at 11:27:29AM +1100, Stephen Rothwell wrote:
> Hi Bjorn,
>
> After merging the pci tree, today's linux-next build (x86_64 allmodconfig)
> failed like this:
>
> drivers/staging/telephony/ixj.c: In function 'ixj_probe_pci':
> drivers/staging/telephony/ixj.c:7732:13: warning: assignment makes integer from pointer without a cast [enabled by default]
> drivers/staging/telephony/ixj.c:7732:38: error: expected ';' before 'pci_resource_start'
>
> Exposed by commit 545974a28f78 ("PCI: Convert pci_resource_<foo> macros to
> static inlines"). The macro version of pci_resource_start() made this
> RHS look like a function call and now it isn't.
>
> Maybe it is time this driver just went away.

It is gone in my tree, and it should be deleted in yours, do you not see
that?

thanks,

greg k-h

2012-11-06 02:09:52

by Stephen Rothwell

[permalink] [raw]
Subject: Re: linux-next: build failure after merge of the pci tree

Hi Greg,

On Tue, 6 Nov 2012 02:55:42 +0100 Greg Kroah-Hartman <[email protected]> wrote:
>
> It is gone in my tree, and it should be deleted in yours, do you not see
> that?

Unfortunately, your tree gets merged late in my set of trees ... so I'll
just keep the disabling patch and the problem will go away eventually.

Thanks.

--
Cheers,
Stephen Rothwell [email protected]


Attachments:
(No filename) (408.00 B)
(No filename) (836.00 B)
Download all attachments

2012-11-06 22:01:19

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: linux-next: build failure after merge of the pci tree

On Tue, Nov 06, 2012 at 11:27:29AM +1100, Stephen Rothwell wrote:
> Hi Bjorn,
>
> After merging the pci tree, today's linux-next build (x86_64 allmodconfig)
> failed like this:
> ...

> drivers/scsi/gdth.c: In function 'gdth_init_pci':
> drivers/scsi/gdth.c:1111:34: error: lvalue required as left operand of assignment

Here's what I think we should do about the GDT issue. I'll send this
via the usual channels.

commit c6156dd31228e608e0a820d2eed7403fd1fd620b
Author: Bjorn Helgaas <[email protected]>
Date: Tue Nov 6 14:19:03 2012 -0700

[SCSI] gdth: Remove buggy ROM handling

The ROM address handling in gdth_init_pci() is useless and possibly
dangerous. This patch removes it.

"pci_resource_start(pdev, 8)" is not well-defined. PCI resources 0-5 are
standard PCI BARs and 6 is the expansion ROM. Resource 8 is either an
SR-IOV BAR (if CONFIG_PCI_IOV=y, resources 7-12 are SR-IOV BARs) or a
bridge window (resources 7-10).

The GDT device is neither an SR-IOV device nor a bridge, so in either case
resource 8 should be zero since struct pci_dev is allocated with kzalloc().

It is illegal for a driver to write an arbitrary address to the ROM BAR
because it has no way of knowing whether the ROM will conflict with another
device.

I think the only effect of the code being removed was to:

1) Enable the ROM at 0xFEFF0000 (possibly causing a conflict with
another device)
2) Delay one millisecond
3) Write zero to the ROM BAR, disabling it

I doubt the delay is needed, but I left it since it seems innocuous.

Signed-off-by: Bjorn Helgaas <[email protected]>

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 5d72274..3efe4ef 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -1107,14 +1107,8 @@ static int __devinit gdth_init_pci(struct pci_dev *pdev, gdth_pci_str *pcistr,
pci_read_config_word(pdev, PCI_COMMAND, &command);
command |= 6;
pci_write_config_word(pdev, PCI_COMMAND, command);
- if (pci_resource_start(pdev, 8) == 1UL)
- pci_resource_start(pdev, 8) = 0UL;
- i = 0xFEFF0001UL;
- pci_write_config_dword(pdev, PCI_ROM_ADDRESS, i);
- gdth_delay(1);
- pci_write_config_dword(pdev, PCI_ROM_ADDRESS,
- pci_resource_start(pdev, 8));
-
+ gdth_delay(1);
+
dp6m_ptr = ha->brd;

/* Ensure that it is safe to access the non HW portions of DPMEM.