| Submitter | Arne Georg Gleditsch |
|---|---|
| Date | 2010-04-12 06:52:21 |
| Message ID | <87tyrhduxm.fsf@taniquetil.gledits.ch> |
| Download | mbox | patch |
| Permalink | /patch/1224/ |
| State | New |
| Headers | show |
Comments
On Mon, Apr 12, 2010 at 12:52 AM, Arne Georg Gleditsch <arne.gleditsch@numascale.com> wrote: > Hi, > > This patch removes a non-32-bit aligned Get_NB32 from > src/northbridge/amd/amdmct/mct/mct_d.c. The actual behavior should be > unchanged for I/O-based config space reads, but previous behavior was > wrong for MMCFG-based config space reads. > > Perhaps we want pci_mmio_*_config* to enforce natural alignment as well? > > > Signed-off-by: Arne Georg Gleditsch <arne.gleditsch@numascale.com> > > -- > Arne. Hi Arne, That code is doing something a little ugly to make the two cs registers use the one mask register. It was ported from a routine that assumed that GET_NB32() fixed the alignment. Maybe add a comment about why that is happening. Acked-by: Marc Jones <marcj303@gmail.com> I would also ack a change to: u32 Get_NB32(u32 dev, u32 reg) u32 Get_NB32(u32 dev, u32 reg) or to the pci_mmio_*_config* to do it since the pci_io* versions do. Marc
Marc Jones <marcj303@gmail.com> writes: > That code is doing something a little ugly to make the two cs > registers use the one mask register. It was ported from a routine that > assumed that GET_NB32() fixed the alignment. Maybe add a comment about > why that is happening. I assumed as much. I'll see if I can't put a comment in. > Acked-by: Marc Jones <marcj303@gmail.com> > > I would also ack a change to: > u32 Get_NB32(u32 dev, u32 reg) > u32 Get_NB32(u32 dev, u32 reg) > > or to the pci_mmio_*_config* to do it since the pci_io* versions do. Thanks, I'll include this as well. The current situation with different semantics on unaligned accesses isn't really helping anyone, as far as I can see.
Patch
diff --git a/src/northbridge/amd/amdmct/mct/mct_d.c b/src/northbridge/amd/amdmct/mct/mct_d.c index 22e9902..1779938 100644 --- a/src/northbridge/amd/amdmct/mct/mct_d.c +++ b/src/northbridge/amd/amdmct/mct/mct_d.c @@ -1970,7 +1970,7 @@ static void StitchMemory_D(struct MCTStatStruc *pMCTstat, reg = 0x40 + (q << 2) + reg_off; /* Base[q] reg.*/ val = Get_NB32(dev, reg); if (!(val & 3)) { /* (CSEnable|Spare==1)bank is enabled already? */ - reg = 0x60 + (q << 1) + reg_off; /*Mask[q] reg.*/ + reg = 0x60 + ((q << 1) & 0xc) + reg_off; /*Mask[q] reg.*/ val = Get_NB32(dev, reg); val >>= 19; val++;
Hi, This patch removes a non-32-bit aligned Get_NB32 from src/northbridge/amd/amdmct/mct/mct_d.c. The actual behavior should be unchanged for I/O-based config space reads, but previous behavior was wrong for MMCFG-based config space reads. Perhaps we want pci_mmio_*_config* to enforce natural alignment as well? Signed-off-by: Arne Georg Gleditsch <arne.gleditsch@numascale.com>