From: John Crispin <hidden> Date: 2012-04-30 17:46:59
On MIPS we want to call of_irq_map_pci from inside
arch/mips/include/asm/pci.h:extern int pcibios_map_irq(
const struct pci_dev *dev, u8 slot, u8 pin);
For this to work we need to change several functions to const usage.
Signed-off-by: John Crispin <redacted>
Cc: linux-pci@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org
Cc: linux-mips@linux-mips.org
---
I am not sure which tree this should go via. Grant, can you take it ?
drivers/of/of_pci_irq.c | 2 +-
drivers/pci/pci.c | 2 +-
include/linux/of_pci.h | 2 +-
include/linux/pci.h | 5 +++--
4 files changed, 6 insertions(+), 5 deletions(-)
From: David Daney <hidden> Date: 2012-04-30 17:54:36
On 04/30/2012 10:46 AM, John Crispin wrote:
On MIPS we want to call of_irq_map_pci from inside
arch/mips/include/asm/pci.h:extern int pcibios_map_irq(
const struct pci_dev *dev, u8 slot, u8 pin);
For this to work we need to change several functions to const usage.
I think there is a mismatch on this throughout the kernel.
Properly fixing it requires touching many more places than these. Although I haven't tried it, I wouldn't be surprised if doing this caused warnings to appear in non-MIPS code.
Ralf had a patch at one point that tried to make this consistent tree-wide, but it is not yet applied.
David Daney
quoted hunk
Signed-off-by: John Crispin<redacted>
Cc: linux-pci@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org
Cc: linux-mips@linux-mips.org
---
I am not sure which tree this should go via. Grant, can you take it ?
drivers/of/of_pci_irq.c | 2 +-
drivers/pci/pci.c | 2 +-
include/linux/of_pci.h | 2 +-
include/linux/pci.h | 5 +++--
4 files changed, 6 insertions(+), 5 deletions(-)
From: John Crispin <hidden> Date: 2012-05-01 13:28:22
On 30/04/12 19:54, David Daney wrote:
On 04/30/2012 10:46 AM, John Crispin wrote:
quoted
On MIPS we want to call of_irq_map_pci from inside
arch/mips/include/asm/pci.h:extern int pcibios_map_irq(
const struct pci_dev *dev, u8 slot, u8 pin);
For this to work we need to change several functions to const usage.
I think there is a mismatch on this throughout the kernel.
Properly fixing it requires touching many more places than these.
Although I haven't tried it, I wouldn't be surprised if doing this
caused warnings to appear in non-MIPS code.
Ralf had a patch at one point that tried to make this consistent
tree-wide, but it is not yet applied.
David Daney
Hi,
Ok, lets see what Ralf has to say.
I just tested the patch on x86 with OF enabled and drivers turned on
that use the API. I did not see any errors appear.
Thanks,
John
On Tue, May 1, 2012 at 7:28 AM, John Crispin [off-list ref] wrote:
On 30/04/12 19:54, David Daney wrote:
quoted
On 04/30/2012 10:46 AM, John Crispin wrote:
quoted
On MIPS we want to call of_irq_map_pci from inside
arch/mips/include/asm/pci.h:extern int pcibios_map_irq(
const struct pci_dev *dev, u8 slot, u8 pin);
For this to work we need to change several functions to const usage.
I think there is a mismatch on this throughout the kernel.
Properly fixing it requires touching many more places than these.
Although I haven't tried it, I wouldn't be surprised if doing this
caused warnings to appear in non-MIPS code.
Ralf had a patch at one point that tried to make this consistent
tree-wide, but it is not yet applied.
David Daney
Hi,
Ok, lets see what Ralf has to say.
I just tested the patch on x86 with OF enabled and drivers turned on
that use the API. I did not see any errors appear.
I'm far from a const expert, but I think this should be safe. Here's
my reasoning:
We're changing pci_swizzle_interrupt_pin() to take a pointer to a
constant struct pci_dev. pci_swizzle_interrupt_pin() only reads the
struct pci_dev; it doesn't modify it. It is legal to pass either
"struct pci_dev *" or "const struct pci_dev *" to a function expecting
"const struct pci_dev *"; the callee just won't be able to modify the
struct, even if the caller can.
Similar reasoning applies to of_irq_map_pci().
So I'm fine with this. You sent it to Grant, so I'll assume he'll
merge it unless I hear otherwise.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
From: David Daney <hidden> Date: 2012-05-04 01:17:59
On 05/03/2012 05:30 PM, Bjorn Helgaas wrote:
On Tue, May 1, 2012 at 7:28 AM, John Crispin[off-list ref] wrote:
quoted
On 30/04/12 19:54, David Daney wrote:
quoted
On 04/30/2012 10:46 AM, John Crispin wrote:
quoted
On MIPS we want to call of_irq_map_pci from inside
arch/mips/include/asm/pci.h:extern int pcibios_map_irq(
const struct pci_dev *dev, u8 slot, u8 pin);
For this to work we need to change several functions to const usage.
I think there is a mismatch on this throughout the kernel.
Properly fixing it requires touching many more places than these.
Although I haven't tried it, I wouldn't be surprised if doing this
caused warnings to appear in non-MIPS code.
Ralf had a patch at one point that tried to make this consistent
tree-wide, but it is not yet applied.
David Daney
Hi,
Ok, lets see what Ralf has to say.
I just tested the patch on x86 with OF enabled and drivers turned on
that use the API. I did not see any errors appear.
I'm far from a const expert, but I think this should be safe.
Here's my reasoning:
We're changing pci_swizzle_interrupt_pin() to take a pointer to a
constant struct pci_dev. pci_swizzle_interrupt_pin() only reads the
struct pci_dev; it doesn't modify it. It is legal to pass either
"struct pci_dev *" or "const struct pci_dev *" to a function expecting
"const struct pci_dev *"; the callee just won't be able to modify the
struct, even if the caller can.
The problem is when you start declaring function pointers in various ops vectors.
Consider:
void (*foo)(const struct pci_dev *)
void (*bar)(struct pci_dev *)
foo and bar are not type compatible, and you will get compiler warnings if you use one where the other is expected.
So the question is: Are we ever going to the address of any of the functions that are being modified? If so, we have created a problem.
Similar reasoning applies to of_irq_map_pci().
So I'm fine with this. You sent it to Grant, so I'll assume he'll
merge it unless I hear otherwise.
Acked-by: Bjorn Helgaas<bhelgaas@google.com>
From: John Crispin <hidden> Date: 2012-05-04 10:55:18
Hi David,
The problem is when you start declaring function pointers in various
ops vectors.
Consider:
void (*foo)(const struct pci_dev *)
void (*bar)(struct pci_dev *)
foo and bar are not type compatible, and you will get compiler
warnings if you use one where the other is expected.
So the question is: Are we ever going to the address of any of the
functions that are being modified? If so, we have created a problem.
i could not find any place in the code where this happens, which does
not mean that there are none.
quoted
Similar reasoning applies to of_irq_map_pci().
So I'm fine with this. You sent it to Grant, so I'll assume he'll
merge it unless I hear otherwise.
Acked-by: Bjorn Helgaas<bhelgaas@google.com>
Thanks for the Ack, i hope this patch gets accepted as is. I am simply
missing the overview of the pci subsystem to evaluate if this can cause
regressions.
John
On Fri, May 4, 2012 at 3:55 AM, John Crispin [off-list ref] wrote:
Hi David,
quoted
The problem is when you start declaring function pointers in various
ops vectors.
Consider:
void (*foo)(const struct pci_dev *)
void (*bar)(struct pci_dev *)
foo and bar are not type compatible, and you will get compiler
warnings if you use one where the other is expected.
Oh, right. I vaguely remember tripping over this a few years ago when
I refactored pci_swizzle_interrupt_pin(). Thanks for enhancing my
simple understanding.
quoted
So the question is: Are we ever going to the address of any of the
functions that are being modified? If so, we have created a problem.
i could not find any place in the code where this happens, which does
not mean that there are none.
I compiled alpha, ia64, mips, parisc, powerpc, sh, sparc, and x86 and
didn't see any issues related to this patch. There might still be
something, but I'm willing to help work through them or revert this
if it turns out to be a problem. I'm still assuming that Grant will
handle this.
Bjorn
From: John Crispin <hidden> Date: 2012-05-08 10:35:37
Hi Bjorn,
I compiled alpha, ia64, mips, parisc, powerpc, sh, sparc, and x86 and
didn't see any issues related to this patch. There might still be
something, but I'm willing to help work through them or revert this
if it turns out to be a problem. I'm still assuming that Grant will
handle this.
Bjorn
From: John Crispin <hidden> Date: 2012-05-12 05:55:05
I compiled alpha, ia64, mips, parisc, powerpc, sh, sparc, and x86 and
didn't see any issues related to this patch. There might still be
something, but I'm willing to help work through them or revert this
if it turns out to be a problem. I'm still assuming that Grant will
handle this.
Bjorn
Hi Grant,
Is this patch ok with you ? If so would you mind if Ralf takes this via
his tree ? (this would avoid merge order problems)
Thanks,
John
From: Rob Herring <hidden> Date: 2012-05-17 22:14:58
On 05/12/2012 12:55 AM, John Crispin wrote:
quoted
I compiled alpha, ia64, mips, parisc, powerpc, sh, sparc, and x86 and
didn't see any issues related to this patch. There might still be
something, but I'm willing to help work through them or revert this
if it turns out to be a problem. I'm still assuming that Grant will
handle this.
Bjorn
Hi Grant,
Is this patch ok with you ? If so would you mind if Ralf takes this via
his tree ? (this would avoid merge order problems)
This looks fine to me and merging thru Ralf's tree is fine.
Acked-by: Rob Herring <redacted>
Rob