On 1 March 2016 at 12:03, Ryan Harkin <[email protected]> wrote: > On 1 March 2016 at 09:52, Leif Lindholm <[email protected]> wrote: >> On Tue, Mar 01, 2016 at 10:40:53AM +0100, Ard Biesheuvel wrote: >>> On 1 March 2016 at 10:36, Leif Lindholm <[email protected]> wrote: >>> > On Tue, Mar 01, 2016 at 09:39:18AM +0100, Ard Biesheuvel wrote: >>> >> >> As far as the Primecell ID is concerned, let's just whitelist whatever >>> >> >> TC2 exposes, even if in error. >>> >> > >>> >> > You mean rather than mask the "stuck bit", specifically check if the >>> >> > register is 0 or 2? (Where 2 is added as a #define to the header). >>> >> >>> >> I don't care deeply either way, but supporting the documented and the >>> >> actual ID explicitly seems more correct, unless the difference is in >>> >> the revision field? >>> > >>> > So ... PeriphID3 holds the "Configuration" bits, so in any case, they >>> > would be irrelevant for identification. >>> >>> Well, the problem is not what they are called, it is what the >>> refmanual of the IP block explicitly states its value can be expected >>> to be. >> >> Yes, and the public documentation found in the obvious place is not >> for the part actually used on the platform, or at least not for the >> same revision. More recent ARM documents tend to be less confusing on >> which fields may be revision-sensitive or not. >> > > Is there a version available publicly for the revision found on > Versatile Express? A link would be handy for future reference. I'm > looking at this doc: > > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0172a/index.html > > ... which is assume is for pre r1p0, which is the version that is > claimed to be in the IOFPGA on the Versatile Express motherboard doc: > > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0172a/index.html > > >> The point is that the "configuration" bits have no bearing on which >> component it is, they merely indicate implementation-time decisions >> (things like configurable fifo depths, inclusion of randomly bolted >> onto the side GPIO block, ...). These are not even necessarily >> software-visible, and the lack of interest on behalf of the Linux >> driver suggests this is the case here. >> > > I'm happy to fix it "properly", but perhaps properly should be via > functions, not macros? > > Although, actually, isn't the PrimeCell identification mechanism > common across all Prime Cells and if so, perhaps a small library is in > order? > > So I propose a series of patches to get things working: > - depex fix to get TC2 working properly > - add debug to indicate PL180 probe failure > - simple fix to the probe, ignoring MCI_PERIPH_ID_REG3 >
Ack > Then another series: > - create a small PrimeCell library for common functions and only add > in the identification code > - a patch per PrimeCell (PL180, PL111, PL061, ...) to use the library > > For the library, we could have two functions: > > PrimeCellDesigner(baseaddr) > PrimeCellPartNumber(baseaddr) > > Do we want to return enums of known designers and part numbers or are > #defines preferable? > > Or perhaps just one TRUE/FALSE function IsPrimeCell(baseaddr, enum id) > is better? > Actually, it makes sense to defer the second part until we have some progress on the 'platform bus' discussion we had last week (between Charles, Leif, Eugene Cohen and myself, among others). The problem here is that many of these drivers should be UEFI drivers, not DXE drivers, and should be instantiated on demand. This means there should be some kind of Primecell bus, which is seeded statically with a list of base addresses, and then proceeds to instantiate driver instances for each primecell id that can be supported by any of the UEFI drivers that claims to support it. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
