[PATCH 10/13] [ARM] pxa/balloon3: PCMCIA Support

[Eric Miao]

>> 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.

>>
>> 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?

> - #define BALLOON3_PCMCIA0_REG               (BALLOON3_FPGA_VIRT + 0x00e00008)

And

> +#define      BALLOON3_CF_STATUS_REG          (0x10e00008)

Even the name has been changed?

>>
>> 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
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>