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: <CAFSKS=O9nDMk-ytxkFhTuZNT-QDmJ_twDdk2o2u0R4Y_YZ0z8A@mail.gmail.com>
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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ