A bit flag, a bitfield is something else and should be avoided. Ideally the values being written into the registers in sxiahci.c would have their own defines instead of being a bunch of magic numbers as well. But that would be a seperate diff.
On Sat, Nov 22, 2014 at 10:04:06PM -0600, Edwin Amsler wrote: > Okay, preferred method is the bitfield, not #ifdef. > > I?ll get a patch done soon and submit. > > On Nov 22, 2014, at 8:24 PM, Jonathan Gray <[email protected]> wrote: > > > On Sat, Nov 22, 2014 at 02:06:48PM -0600, Edwin Amsler wrote: > >> Hey there, > >> > >> This e-mail is about design and custom arch-specific code. > >> > >> I?m trying to author a patch from Bitrig to enable proper support for AHCI > >> in the SUNXI ARMv7 port. There?s a bit of a quirk in how SUNXI handles DMA > >> from what I understand of the patch. I think it may only affect the A10, > >> but that?s outside the scope of this e-mail. > >> > >> The fix is a series of patches, but the first one is going to be the tough > >> since the change nestled inside ahci_port_start() in sys/dev/ic/ahci.c. > >> Here?s the web-view of the patch: (let me know if you need me to provide a > >> diff for readability) > >> https://github.com/bitrig/bitrig/commit/6342a27bfe4dc590f8e266e370f54846a766a737 > >> > >> Now, I don?t care about the original author?s implementation choice, > >> everyone is justified in their own actions. What I want to know is what > >> needs to be done to increase the chances this will be committed by you > >> folks. > >> > >> To be honest, I?d prefer to #ifdef the custom initialization based on > >> target platform and clean up the ahci_softc.sc_flags bit field. Is > >> everyone on board for that? Is there a better way? > > > > The quirk bit should just be set in arch/armv7/sunxi/sxiahci.c > > Change > > sc->sc_flags |= AHCI_F_NO_PMP; /* XXX enough? */ > > to > > sc->sc_flags |= AHCI_F_NO_PMP | AHCI_F_SUNXI_QUIRK; > > > > That is how bitrig did it as well. > > > > Some of the other register access changes in the sxiahci.c part of > > https://github.com/bitrig/bitrig/commit/9859ab07da261ebb4f561ff2b5bc5802b63ac57c > > may be of interest as well.
