| Message ID | 1470351518-22404-2-git-send-email-york.sun@nxp.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
On 08/04/2016 04:36 PM, Andrew Donnellan wrote: > On 05/08/16 08:58, York Sun wrote: >> Two symbols are missing if mpc85xx_edac driver is compiled as module. >> >> Signed-off-by: York Sun <york.sun@nxp.com> > > Good catch! One comment below. > > Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> > >> /* >> * Reads the interrupt pin to determine if interrupt is use by card. >> @@ -1585,6 +1586,7 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn, >> { >> return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap); >> } >> +EXPORT_SYMBOL(early_find_capability); > > It'd be nicer for this to be renamed as "pci_early_find_capability" or > something like that with a "namespace", I think. > I will rename it if I respin this patch for any reason. Otherwise, I will send out another patch to rename it after merging. York
York Sun <york.sun@nxp.com> writes: > Two symbols are missing if mpc85xx_edac driver is compiled as module. > > Signed-off-by: York Sun <york.sun@nxp.com> > > --- > Change log > v3: Change subject tag > v2: no change > > arch/powerpc/kernel/pci-common.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c > index 0f7a60f..86bc484 100644 > --- a/arch/powerpc/kernel/pci-common.c > +++ b/arch/powerpc/kernel/pci-common.c > @@ -226,6 +226,7 @@ struct pci_controller* pci_find_hose_for_OF_device(struct device_node* node) > } > return NULL; > } > +EXPORT_SYMBOL(pci_find_hose_for_OF_device); > > /* > * Reads the interrupt pin to determine if interrupt is use by card. > @@ -1585,6 +1586,7 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn, > { > return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap); > } > +EXPORT_SYMBOL(early_find_capability); Does the driver really need to use these routines? They're meant for use early in boot, before PCI is setup. AFAICS this is just a regular driver, so when it's probed the PCI devices should have already been scanned. In which case pci_get_device() could work couldn't it? (I see other edac drivers doing that). cheers
On 08/04/2016 08:43 PM, Michael Ellerman wrote: > York Sun <york.sun@nxp.com> writes: > >> Two symbols are missing if mpc85xx_edac driver is compiled as module. >> >> Signed-off-by: York Sun <york.sun@nxp.com> >> >> --- >> Change log >> v3: Change subject tag >> v2: no change >> >> arch/powerpc/kernel/pci-common.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c >> index 0f7a60f..86bc484 100644 >> --- a/arch/powerpc/kernel/pci-common.c >> +++ b/arch/powerpc/kernel/pci-common.c >> @@ -226,6 +226,7 @@ struct pci_controller* pci_find_hose_for_OF_device(struct device_node* node) >> } >> return NULL; >> } >> +EXPORT_SYMBOL(pci_find_hose_for_OF_device); >> >> /* >> * Reads the interrupt pin to determine if interrupt is use by card. >> @@ -1585,6 +1586,7 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn, >> { >> return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap); >> } >> +EXPORT_SYMBOL(early_find_capability); > > Does the driver really need to use these routines? They're meant for use > early in boot, before PCI is setup. > > AFAICS this is just a regular driver, so when it's probed the PCI > devices should have already been scanned. In which case pci_get_device() > could work couldn't it? (I see other edac drivers doing that). > I don't have deep knowledge of this driver. What I am trying is to separate the common DDR part and share it with ARM platforms. Along the way, I found the compiling error if build a module. If exposing these functions becomes a concern, I can live without it. York
On Thu, Aug 04, 2016 at 11:39:14PM +0000, york sun wrote: > I will rename it if I respin this patch for any reason. Otherwise, I > will send out another patch to rename it after merging. Feel free to send an updated one as a reply to this thread.
On Fri, Aug 05, 2016 at 04:26:26AM +0000, york sun wrote: > I don't have deep knowledge of this driver. What I am trying is to > separate the common DDR part and share it with ARM platforms. Along the > way, I found the compiling error if build a module. If exposing these > functions becomes a concern, I can live without it. Perhaps you or Johannes could fix this properly to use pci_get_device() as the rest of the EDAC drivers do, instead of exporting core PCI functions...
On Fri, Aug 05, 2016 at 09:01:26AM +0200, Borislav Petkov wrote: > On Fri, Aug 05, 2016 at 04:26:26AM +0000, york sun wrote: > > I don't have deep knowledge of this driver. What I am trying is to > > separate the common DDR part and share it with ARM platforms. Along the > > way, I found the compiling error if build a module. If exposing these > > functions becomes a concern, I can live without it. > > Perhaps you or Johannes could fix this properly to use pci_get_device() > as the rest of the EDAC drivers do, instead of exporting core PCI > functions... I can give it a shot, but I don't have too much spare time atm and no hardware to test, so it'll have a strong RFC smell attached to it. Byte, Johannes
On 08/04/2016 08:43 PM, Michael Ellerman wrote: > York Sun <york.sun@nxp.com> writes: > >> Two symbols are missing if mpc85xx_edac driver is compiled as module. >> >> Signed-off-by: York Sun <york.sun@nxp.com> >> >> --- >> Change log >> v3: Change subject tag >> v2: no change >> >> arch/powerpc/kernel/pci-common.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c >> index 0f7a60f..86bc484 100644 >> --- a/arch/powerpc/kernel/pci-common.c >> +++ b/arch/powerpc/kernel/pci-common.c >> @@ -226,6 +226,7 @@ struct pci_controller* pci_find_hose_for_OF_device(struct device_node* node) >> } >> return NULL; >> } >> +EXPORT_SYMBOL(pci_find_hose_for_OF_device); >> >> /* >> * Reads the interrupt pin to determine if interrupt is use by card. >> @@ -1585,6 +1586,7 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn, >> { >> return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap); >> } >> +EXPORT_SYMBOL(early_find_capability); > > Does the driver really need to use these routines? They're meant for use > early in boot, before PCI is setup. > > AFAICS this is just a regular driver, so when it's probed the PCI > devices should have already been scanned. In which case pci_get_device() > could work couldn't it? (I see other edac drivers doing that). I am trying to fix this but need some help. We are dealing with PCIe controller here. Does it have a bus number assigned at this point? If yes, how can I find it? I seem not able to find out where the platform_data is filled as well. Can someone kindly point it out to me? York
On Fri, 2016-08-05 at 20:29 +0000, york sun wrote: > On 08/04/2016 08:43 PM, Michael Ellerman wrote: > > > > Does the driver really need to use these routines? They're meant for use > > early in boot, before PCI is setup. > > > > AFAICS this is just a regular driver, so when it's probed the PCI > > devices should have already been scanned. In which case pci_get_device() > > could work couldn't it? (I see other edac drivers doing that). > I am trying to fix this but need some help. We are dealing with PCIe > controller here. Does it have a bus number assigned at this point? If > yes, how can I find it? I seem not able to find out where the > platform_data is filled as well. Can someone kindly point it out to me? The platform data comes from add_err_dev() in arch/powerpc/sysdev/fsl_pci.c. -Scott
On 08/05/2016 02:09 PM, Scott Wood wrote: > On Fri, 2016-08-05 at 20:29 +0000, york sun wrote: >> On 08/04/2016 08:43 PM, Michael Ellerman wrote: >>> >>> Does the driver really need to use these routines? They're meant for use >>> early in boot, before PCI is setup. >>> >>> AFAICS this is just a regular driver, so when it's probed the PCI >>> devices should have already been scanned. In which case pci_get_device() >>> could work couldn't it? (I see other edac drivers doing that). >> I am trying to fix this but need some help. We are dealing with PCIe >> controller here. Does it have a bus number assigned at this point? If >> yes, how can I find it? I seem not able to find out where the >> platform_data is filled as well. Can someone kindly point it out to me? > > > The platform data comes from add_err_dev() in arch/powerpc/sysdev/fsl_pci.c. > Thanks, Scott. When add_err_dev() is called, pci is not scanned, is using early_find_capability() justified? York
On Fri, 2016-08-05 at 21:20 +0000, york sun wrote: > On 08/05/2016 02:09 PM, Scott Wood wrote: > > > > On Fri, 2016-08-05 at 20:29 +0000, york sun wrote: > > > > > > On 08/04/2016 08:43 PM, Michael Ellerman wrote: > > > > > > > > > > > > Does the driver really need to use these routines? They're meant for > > > > use > > > > early in boot, before PCI is setup. > > > > > > > > AFAICS this is just a regular driver, so when it's probed the PCI > > > > devices should have already been scanned. In which case > > > > pci_get_device() > > > > could work couldn't it? (I see other edac drivers doing that). > > > I am trying to fix this but need some help. We are dealing with PCIe > > > controller here. Does it have a bus number assigned at this point? If > > > yes, how can I find it? I seem not able to find out where the > > > platform_data is filled as well. Can someone kindly point it out to me? > > > > The platform data comes from add_err_dev() in > > arch/powerpc/sysdev/fsl_pci.c. > > > Thanks, Scott. > > When add_err_dev() is called, pci is not scanned, is using > early_find_capability() justified? The edac driver is registered with a normal device-level initcall. The PCI scanning appears to happen at the subsys initcall level. -Scott
On 08/05/2016 12:01 AM, Borislav Petkov wrote: > On Fri, Aug 05, 2016 at 04:26:26AM +0000, york sun wrote: >> I don't have deep knowledge of this driver. What I am trying is to >> separate the common DDR part and share it with ARM platforms. Along the >> way, I found the compiling error if build a module. If exposing these >> functions becomes a concern, I can live without it. > > Perhaps you or Johannes could fix this properly to use pci_get_device() > as the rest of the EDAC drivers do, instead of exporting core PCI > functions... > Boris, I'd like to separate the first two patches from this set. They are not really related to the DDR part I am working on. It will take me a while to sort out the correct fix. York
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index 0f7a60f..86bc484 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -226,6 +226,7 @@ struct pci_controller* pci_find_hose_for_OF_device(struct device_node* node) } return NULL; } +EXPORT_SYMBOL(pci_find_hose_for_OF_device); /* * Reads the interrupt pin to determine if interrupt is use by card. @@ -1585,6 +1586,7 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn, { return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap); } +EXPORT_SYMBOL(early_find_capability); struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) {
Two symbols are missing if mpc85xx_edac driver is compiled as module. Signed-off-by: York Sun <york.sun@nxp.com> --- Change log v3: Change subject tag v2: no change arch/powerpc/kernel/pci-common.c | 2 ++ 1 file changed, 2 insertions(+)