[U-Boot] [PATCH 04/13] ppc: Remove pci config table pointer relocation fixups

Paul Gortmaker paul.gortmaker at windriver.com
Mon Sep 21 19:20:33 CEST 2009


Peter Tyser wrote:
> On Mon, 2009-09-21 at 12:49 -0400, Paul Gortmaker wrote:
>> On Mon, Sep 21, 2009 at 12:20 PM, Peter Tyser <ptyser at xes-inc.com> wrote:
>>> Signed-off-by: Peter Tyser <ptyser at xes-inc.com>
>> It looks like something happened during the send of your patches;
>> it seems the long log of what you are trying to fix and how the patch
>> fixes it is missing here and from several of the other patches
>> as well.
> 
> I didn't put long log messages in patches which I thought would be clear
> what was changing.  All the "Remove XYZ relocation fixups" are intended
> to only remove no longer needed fixups.  They shouldn't (hopefully) have
> any functional change.  I can add log messages, but they will all be the
> same "don't fixup xyz as relocation now works".

I guess even something as basic as that would be better than
nothing in my opinion.  Folks rummaging through history won't
have the benefit of your [0/13] description when they are
looking through the change history with git.

> 
>>> ---
>>>  board/freescale/mpc8548cds/mpc8548cds.c |    7 -------
>>>  board/mpl/common/pci.c                  |   18 ------------------
>>>  board/sbc8548/sbc8548.c                 |    6 ------
>>>  3 files changed, 0 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/board/freescale/mpc8548cds/mpc8548cds.c b/board/freescale/mpc8548cds/mpc8548cds.c
>>> index 80de6f8..73e7c21 100644
>>> --- a/board/freescale/mpc8548cds/mpc8548cds.c
>>> +++ b/board/freescale/mpc8548cds/mpc8548cds.c
>>> @@ -276,7 +276,6 @@ pci_init_board(void)
>>>  {
>>>        volatile ccsr_fsl_pci_t *pci = (ccsr_fsl_pci_t *) CONFIG_SYS_PCI1_ADDR;
>>>        struct pci_controller *hose = &pci1_hose;
>>> -       struct pci_config_table *table;
>>>        struct pci_region *r = hose->regions;
>>>
>>>        uint pci_32 = gur->pordevsr & MPC85xx_PORDEVSR_PCI1_PCI32;      /* PORDEVSR[15] */
>>> @@ -312,12 +311,6 @@ pci_init_board(void)
>>>                               PCI_REGION_IO);
>>>                hose->region_count = r - hose->regions;
>>>
>>> -               /* relocate config table pointers */
>>> -               hose->config_table = \
>>> -                       (struct pci_config_table *)((uint)hose->config_table + gd->reloc_off);
>>> -               for (table = hose->config_table; table && table->vendor; table++)
>>> -                       table->config_device += gd->reloc_off;
>> For the mpc8548cds, if this removal was somehow the right thing to do,
>> it would still be incomplete;  I am sure that there is a dummy function
>> related to a PCI bridge quirk associated with the above change that
>> would now be orphaned in the code.
> 
> I didn't intend to make any functional change as I know nothing about
> this board:)  I only intended to remove the references to gd->reloc_off.
> I looked over this code quickly and came to the conclusion I wasn't
> changing any functionality, let me know if I'm missing something.

OK, I just went and looked, and I think that you are correct.
The config_table with the dummy function is still hooked in via
the static pci1_hose initialization; sorry for the noise.

> 
>>> -
>>>                hose->first_busno=first_free_busno;
>>>
>>>                fsl_pci_init(hose, (u32)&pci->cfg_addr, (u32)&pci->cfg_data);
>> [...]
>>
>>> diff --git a/board/sbc8548/sbc8548.c b/board/sbc8548/sbc8548.c
>>> index e5b21b5..5cdfef4 100644
>>> --- a/board/sbc8548/sbc8548.c
>>> +++ b/board/sbc8548/sbc8548.c
>>> @@ -392,12 +392,6 @@ pci_init_board(void)
>>>                               PCI_REGION_IO);
>>>                hose->region_count = r - hose->regions;
>>>
>>> -               /* relocate config table pointers */
>>> -               hose->config_table = \
>>> -                       (struct pci_config_table *)((uint)hose->config_table + gd->reloc_off);
>>> -               for (table = hose->config_table; table && table->vendor; table++)
>>> -                       table->config_device += gd->reloc_off;
>> This code is already gone from the sbc8548 in the 85xx branch;
>> the sbc8548 didn't need the bridge quirk fixup.
> 
> Thanks for the heads up.  Maybe git will gracefully handle this change?

Depends on your definition of "gracefully", I guess.  :-)

> If not, I'd prefer to wait till Wolfgang attempts to merge this patch as
> other things may be merged between now and then and I'd rather just send
> 1 cleanup patch series.

Makes sense.
Paul.

> 
> Best,
> Peter
> 



More information about the U-Boot mailing list