netdev - Re: [RFC PATCH 2/3] net: sparx5: Add Sparx5 switchdev driver

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: <20201130144206.g7hnvwrtbblgblpm@mchp-dev-shegelun>
Date:   Mon, 30 Nov 2020 15:42:06 +0100
From:   Steen Hegelund <steen.hegelund@...rochip.com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Lars Povlsen <lars.povlsen@...rochip.com>,
        Bjarni Jonasson <bjarni.jonasson@...rochip.com>,
        Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Microsemi List <microsemi@...ts.bootlin.com>,
        <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 2/3] net: sparx5: Add Sparx5 switchdev driver

On 29.11.2020 18:35, Andrew Lunn wrote:
>EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
>> +#define SPX5_RD_(sparx5, id, tinst, tcnt,                    \
>> +              gbase, ginst, gcnt, gwidth,                    \
>> +              raddr, rinst, rcnt, rwidth)                    \
>> +     readl(spx5_addr((sparx5)->regs, id, tinst, tcnt,        \
>> +                     gbase, ginst, gcnt, gwidth,             \
>> +                     raddr, rinst, rcnt, rwidth))
>> +
>> +#define SPX5_INST_RD_(iomem, id, tinst, tcnt,                        \
>> +                   gbase, ginst, gcnt, gwidth,               \
>> +                   raddr, rinst, rcnt, rwidth)               \
>> +     readl(spx5_inst_addr(iomem,                             \
>> +                          gbase, ginst, gcnt, gwidth,        \
>> +                          raddr, rinst, rcnt, rwidth))
>> +
>> +#define SPX5_WR_(val, sparx5, id, tinst, tcnt,                       \
>> +              gbase, ginst, gcnt, gwidth,                    \
>> +              raddr, rinst, rcnt, rwidth)                    \
>> +     writel(val, spx5_addr((sparx5)->regs, id, tinst, tcnt,  \
>> +                           gbase, ginst, gcnt, gwidth,       \
>> +                           raddr, rinst, rcnt, rwidth))
>> +
>> +#define SPX5_INST_WR_(val, iomem, id, tinst, tcnt,           \
>> +                   gbase, ginst, gcnt, gwidth,               \
>> +                   raddr, rinst, rcnt, rwidth)               \
>> +     writel(val, spx5_inst_addr(iomem,                       \
>> +                                gbase, ginst, gcnt, gwidth,  \
>> +                                raddr, rinst, rcnt, rwidth))
>> +
>> +#define SPX5_RMW_(val, mask, sparx5, id, tinst, tcnt,                        \
>> +               gbase, ginst, gcnt, gwidth,                           \
>> +               raddr, rinst, rcnt, rwidth)                           \
>> +     do {                                                            \
>> +             u32 _v_;                                                \
>> +             u32 _m_ = mask;                                         \
>> +             void __iomem *addr =                                    \
>> +                     spx5_addr((sparx5)->regs, id, tinst, tcnt,      \
>> +                               gbase, ginst, gcnt, gwidth,           \
>> +                               raddr, rinst, rcnt, rwidth);          \
>> +             _v_ = readl(addr);                                      \
>> +             _v_ = ((_v_ & ~(_m_)) | ((val) & (_m_)));               \
>> +             writel(_v_, addr);                                      \
>> +     } while (0)
>> +
>> +#define SPX5_INST_RMW_(val, mask, iomem, id, tinst, tcnt,            \
>> +                    gbase, ginst, gcnt, gwidth,                      \
>> +                    raddr, rinst, rcnt, rwidth)                      \
>> +     do {                                                            \
>> +             u32 _v_;                                                \
>> +             u32 _m_ = mask;                                         \
>> +             void __iomem *addr =                                    \
>> +                     spx5_inst_addr(iomem,                           \
>> +                                    gbase, ginst, gcnt, gwidth,      \
>> +                                    raddr, rinst, rcnt, rwidth);     \
>> +             _v_ = readl(addr);                                      \
>> +             _v_ = ((_v_ & ~(_m_)) | ((val) & (_m_)));               \
>> +             writel(_v_, addr);                                      \
>> +     } while (0)
>> +
>> +#define SPX5_REG_RD_(regaddr)                        \
>> +     readl(regaddr)
>> +
>> +#define SPX5_REG_WR_(val, regaddr)           \
>> +     writel(val, regaddr)
>> +
>> +#define SPX5_REG_RMW_(val, mask, regaddr)                \
>> +     do {                                                \
>> +             u32 _v_;                                    \
>> +             u32 _m_ = mask;                             \
>> +             void __iomem *_r_ = regaddr;                \
>> +             _v_ = readl(_r_);                           \
>> +             _v_ = ((_v_ & ~(_m_)) | ((val) & (_m_)));   \
>> +             writel(_v_, _r_);                           \
>> +     } while (0)
>> +
>> +#define SPX5_REG_GET_(sparx5, id, tinst, tcnt,                       \
>> +                   gbase, ginst, gcnt, gwidth,               \
>> +                   raddr, rinst, rcnt, rwidth)               \
>> +     spx5_addr((sparx5)->regs, id, tinst, tcnt,              \
>> +               gbase, ginst, gcnt, gwidth,                   \
>> +               raddr, rinst, rcnt, rwidth)
>> +
>> +#define SPX5_RD(...)  SPX5_RD_(__VA_ARGS__)
>> +#define SPX5_WR(...)  SPX5_WR_(__VA_ARGS__)
>> +#define SPX5_RMW(...) SPX5_RMW_(__VA_ARGS__)
>> +#define SPX5_INST_RD(...) SPX5_INST_RD_(__VA_ARGS__)
>> +#define SPX5_INST_WR(...) SPX5_INST_WR_(__VA_ARGS__)
>> +#define SPX5_INST_RMW(...) SPX5_INST_RMW_(__VA_ARGS__)
>> +#define SPX5_INST_GET(sparx5, id, tinst) ((sparx5)->regs[(id) + (tinst)])
>> +#define SPX5_REG_RMW(...) SPX5_REG_RMW_(__VA_ARGS__)
>> +#define SPX5_REG_WR(...) SPX5_REG_WR_(__VA_ARGS__)
>> +#define SPX5_REG_RD(...) SPX5_REG_RD_(__VA_ARGS__)
>> +#define SPX5_REG_GET(...) SPX5_REG_GET_(__VA_ARGS__)
>
>I don't see any reason for macro magic here. If this just left over
>from HAL code? Please turn this all into functions.
>
>     Andrew
>

Thanks for the comment.  I will transform this into functions.

BR
Steen

---------------------------------------
Steen Hegelund
steen.hegelund@...rochip.com