Hi Bin, On 24 June 2015 at 09:15, Simon Glass <[email protected]> wrote: > Hi Bin, > > On 23 June 2015 at 21:59, Bin Meng <[email protected]> wrote: >> Hi Simon, >> >> On Wed, Jun 24, 2015 at 11:54 AM, Simon Glass <[email protected]> wrote: >>> Hi Bin, >>> >>> On 23 June 2015 at 21:46, Bin Meng <[email protected]> wrote: >>>> Hi Simon, >>>> >>>> On Wed, Jun 24, 2015 at 11:18 AM, Simon Glass <[email protected]> wrote: >>>>> Hi Bin, >>>>> >>>>> On 7 June 2015 at 20:15, Bin Meng <[email protected]> wrote: >>>>>> Hi Simon, >>>>>> >>>>>> On Sun, Jun 7, 2015 at 10:50 PM, Simon Glass <[email protected]> wrote: >>>>>>> This driver should use the x86 PCI configuration functions. Also adjust >>>>>>> its >>>>>>> compatible string to something generic (i.e. without a vendor name). >>>>>>> >>>>>>> Signed-off-by: Simon Glass <[email protected]> >>>>>>> --- >>>>>>> >>>>>>> drivers/pci/pci_x86.c | 5 ++++- >>>>>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/drivers/pci/pci_x86.c b/drivers/pci/pci_x86.c >>>>>>> index 901bdca..9f842c3 100644 >>>>>>> --- a/drivers/pci/pci_x86.c >>>>>>> +++ b/drivers/pci/pci_x86.c >>>>>>> @@ -7,12 +7,15 @@ >>>>>>> #include <common.h> >>>>>>> #include <dm.h> >>>>>>> #include <pci.h> >>>>>>> +#include <asm/pci.h> >>>>>>> >>>>>>> static const struct dm_pci_ops x86_pci_ops = { >>>>>> >>>>>> To keep the consistent naming to match the driver name, can we rename >>>>>> this to pci_x86_ops? >>>>> >>>>> OK >>>>> >>>>>> >>>>>>> + .read_config = pci_x86_read_config, >>>>>>> + .write_config = pci_x86_write_config, >>>>>> >>>>>> Can we move pci_x86_read_config() and pci_x86_write_config() from >>>>>> arch/x86/cpu/pci.c to this file to make it a complete driver file? >>>>>> Also create a new header file pci_x86.h to declare these two so that >>>>>> it can be used by ivybridge. >>>>> >>>>> I can certainly drop the ivybridge duplication. But I don't think it >>>>> is right to call directly into a driver in drivers/... >>>>> >>>>> We should use driver model for this if we want to do it properly. I >>>>> would like to continue the work to move x86 fully to driver model. >>>>> >>>>> In the meantime I think that directly called functions should be in >>>>> arch/x86. >>>>> >>>> >>>> Sorry I don't get it. I mean moving the implementation of >>>> pci_x86_read_config() and pci_x86_write_config() to >>>> drivers/pci/pci_x86.c. Do you have some concern about this? >>>> >>>> [snip] >>> >>> Yes it is still used by arch/x86/cpu/coreboot/pci.c - and as I say I >>> don't like the 'call directly into driver' idea. If we could remove >>> the coreboot case then it would be fine. >> >> Why not we drop this coreboot pci driver (arch/x86/cpu/coreboot/pci.c) >> and use the new one (drivers/pci/pci_x86.c) directly? > > Yes let's do that. I can't see any reason not to.
I think the problem is that cpu/ivybridge/pci.c has its own driver and wants to call the pci_x86_read/write_config() functions. So they really need to be in a generic area. I was thinking about coreboot yesterday, but the problem is ivybridge. Regards, Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
