netdev - Re: [PATCH 1/3] can: rcar_canfd: Add support for r8a779a0 SoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1393112852.2793965.1641918086821@webmail.strato.com>
Date:   Tue, 11 Jan 2022 17:21:26 +0100 (CET)
From:   Ulrich Hecht <uli@...nd.eu>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        Ulrich Hecht <uli+renesas@...nd.eu>
Cc:     Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>, linux-can@...r.kernel.org,
        "Lad, Prabhakar" <prabhakar.mahadev-lad.rj@...renesas.com>,
        Biju Das <biju.das.jz@...renesas.com>,
        Wolfram Sang <wsa@...nel.org>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        Wolfgang Grandegger <wg@...ndegger.com>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        Jakub Kicinski <kuba@...nel.org>, mailhol.vincent@...adoo.fr,
        socketcan@...tkopp.net
Subject: Re: [PATCH 1/3] can: rcar_canfd: Add support for r8a779a0 SoC

Thank you for your review.

> On 10/05/2021 3:06 PM Geert Uytterhoeven <geert@...ux-m68k.org> 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