netdev - Re: [PATCH net-next v2 1/3] net: dsa: microchip: add KSZ9477 I2C 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]

Date:   Tue, 10 Sep 2019 09:40:32 -0500
From:   George McCollister <george.mccollister@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     netdev@...r.kernel.org, Woojung Huh <woojung.huh@...rochip.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Tristram Ha <Tristram.Ha@...rochip.com>,
        "David S. Miller" <davem@...emloft.net>,
        Marek Vasut <marex@...x.de>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v2 1/3] net: dsa: microchip: add KSZ9477 I2C driver

Andrew,

On Tue, Sep 10, 2019 at 9:03 AM Andrew Lunn <andrew@...n.ch> wrote:
>
> Hi George
>
> > +KSZ_REGMAP_TABLE(ksz9477, not_used, 16, 0, 0);
> > +
> > @@ -294,6 +294,8 @@ static inline void ksz_pwrite32(struct ksz_device *dev, int port, int offset,
> >  #define KSZ_SPI_OP_RD                3
> >  #define KSZ_SPI_OP_WR                2
> >
> > +#define swabnot_used(x)              0
>
> > +
> >  #define KSZ_SPI_OP_FLAG_MASK(opcode, swp, regbits, regpad)           \
> >       swab##swp((opcode) << ((regbits) + (regpad)))
>
> There seems to be quite a lot of macro magic here which is not
> obvious. Can this be simplified or made more obvious?

I thought about this for quite some time. To reduce the "macro magic"
the SPI specific parts will need to be removed from the common macro
and arguments for read_flag_mask and write_flag_mask would need to be
added to both KSZ_REGMAP_TABLE and KSZ_REGMAP_TABLE. That would leave
us with two macros that have 7 arguments. Not really an improvement
IMHO. Alternatively we could have different macros for SPI and I2C (or
not use the macros at all and define the i2c regmaps in ksz9477_i2c.c)
at the cost of ~20 lines of duplication. I prefer the "macro magic"
approach, however if you won't let the patch through the way it is
I'll respect your decision, just let me know which of the three
proposed approaches you want to go with.

>
>          Andrew

Cheers,
George