Hi Simon, On Wed, Jun 24, 2015 at 11:54 AM, Simon Glass <s...@chromium.org> wrote: > Hi Bin, > > On 23 June 2015 at 21:46, Bin Meng <bmeng...@gmail.com> wrote: >> Hi Simon, >> >> On Wed, Jun 24, 2015 at 11:18 AM, Simon Glass <s...@chromium.org> wrote: >>> Hi Bin, >>> >>> On 7 June 2015 at 20:15, Bin Meng <bmeng...@gmail.com> wrote: >>>> Hi Simon, >>>> >>>> On Sun, Jun 7, 2015 at 10:50 PM, Simon Glass <s...@chromium.org> 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 <s...@chromium.org> >>>>> --- >>>>> >>>>> 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? Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot