Thank you for your review. > On 10/05/2021 3:06 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > I'm wondering if some of these IS_V3U checks can be avoided, improving > legibility, by storing a feature struct instead of a chip_id in > rcar_canfd_of_table[].data? Not really. I have found perhaps three cases in which this is possible, compared to dozens where it isn't. In the end you would get virtually no change in legibility or verbosity, but an increase in complexity. > > /* RSCFDnCFDRFCCx / RSCFDnRFCCx */ > > -#define RCANFD_RFCC(x) (0x00b8 + (0x04 * (x))) > > +#define RCANFD_RFCC(x) ((IS_V3U ? 0x00c0 : 0x00b8) + \ > > + (0x04 * (x))) > > /* RSCFDnCFDRFSTSx / RSCFDnRFSTSx */ > > -#define RCANFD_RFSTS(x) (0x00d8 + (0x04 * (x))) > > +#define RCANFD_RFSTS(x) ((IS_V3U ? 0x00e0 : 0x00d8) + \ > > + (0x04 * (x))) > > /* RSCFDnCFDRFPCTRx / RSCFDnRFPCTRx */ > > -#define RCANFD_RFPCTR(x) (0x00f8 + (0x04 * (x))) > > +#define RCANFD_RFPCTR(x) ((IS_V3U ? 0x0100 : 0x00f8) + \ > > + (0x04 * (x))) > > There's some logic in the offsets: they're 32 bytes apart, regardless > of IS_V3U. Can we make use of that? We can here... > > /* Common FIFO Control registers */ > > > > /* RSCFDnCFDCFCCx / RSCFDnCFCCx */ > > -#define RCANFD_CFCC(ch, idx) (0x0118 + (0x0c * (ch)) + \ > > - (0x04 * (idx))) > > +#define RCANFD_CFCC(ch, idx) ((IS_V3U ? 0x0120 : 0x0118) + \ > > + (0x0c * (ch)) + (0x04 * (idx))) > > /* RSCFDnCFDCFSTSx / RSCFDnCFSTSx */ > > -#define RCANFD_CFSTS(ch, idx) (0x0178 + (0x0c * (ch)) + \ > > - (0x04 * (idx))) > > +#define RCANFD_CFSTS(ch, idx) ((IS_V3U ? 0x01e0 : 0x0178) + \ > > + (0x0c * (ch)) + (0x04 * (idx))) > > /* RSCFDnCFDCFPCTRx / RSCFDnCFPCTRx */ > > -#define RCANFD_CFPCTR(ch, idx) (0x01d8 + (0x0c * (ch)) + \ > > - (0x04 * (idx))) > > +#define RCANFD_CFPCTR(ch, idx) ((IS_V3U ? 0x0240 : 0x01d8) + \ > > + (0x0c * (ch)) + (0x04 * (idx))) > > Same here, 96 bytes spacing. ...but not here. (0x1e0 - 0x120 != 0x60) CU Uli
