Re: [Balloon] [PATCH 10/13] [ARM] pxa/balloon3: PCMCIA Suppo…

Top Page
Attachments:
Message as email
+ (text/plain)
Delete this message
Reply to this message
Author: Marek Vasut
Date:  
To: Eric Miao
CC: Wookey, Balloon, linux-arm-kernel
Subject: Re: [Balloon] [PATCH 10/13] [ARM] pxa/balloon3: PCMCIA Support
Dne Pá 30. července 2010 07:02:46 Eric Miao napsal(a):
> >> erm. Why move the FPGA/CPLD register definitions out of balloon3.h
> >> (along with the others) and put them in balloon3.c?
> >> Is that really good kernel form?
> >
> > They are not used anywhere else so they don't need to be in that file
> > publically available.
>
> If the FPGA/CPLD is _not_ solely for the purpose of CF/PCMCIA,
> nor it is like an MFD device where the functionality can be clearly
> separated from different ranges of registers, I'd agree with Wookey
> better to keep them in balloon3.h.


Well the registers used are solely for PCMCIA (or rather CF socket on the board,
which clears your question below).
>
> >> And surely keeping the FPGA_VIRT offset makes it clearer where the
> >> magic addresses come from?
> >
> > You mean PHYS offset I believe. That is a good idea, but I'd firstly like
> > to distribute all the addresses in balloon3.h into drivers (well all
> > that can be distributed) and then flip it over so all registers will be
> > defined by offset from some FPGA base address. That's not possible yet
> > so for consistency's sake, I do it this way now. The patch flipping it
> > to offsets will be very easy, but this cleanup needs a two-stage plan.
>
> And the offsets were a bit different from their originals here?


I ioremap-ed them.
>
> > - #define BALLOON3_PCMCIA0_REG               (BALLOON3_FPGA_VIRT +
> > 0x00e00008)

>
> And
>
> > +#define      BALLOON3_CF_STATUS_REG          (0x10e00008)

>
> Even the name has been changed?


This is more appropriate. The thing on the board is a CF socket.
>
> >> And finally as this code is quite dependent on VHDL version numbers
> >> and we know we have a number of incompatible versions around
> >> should we add a version check to ensure that we have the same VHDL
> >> version as the driver is expecting? I guess it should be of the 'at
> >> least n' form, otherwise later VHDL updates would break the driver,
> >> probbaly for no good reason.
> >
> > Yes, the check is in place.
> >
> > +       if (ver > 0x0201)
> > +               pr_warn("The FPGA code, version 0x%04x, is newer than
> > rel-0.3. " +                       "PCMCIA/CF support might be broken in
> > this version!", +                       ver);

> >
> > Actually I'm planning to fix the VHDL if I'll have some time for that.
> >
> > Cheers
> >
> >> Wookey
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> >
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel