Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190910131836.114058-1-george.mccollister@gmail.com>
 <20190910131836.114058-2-george.mccollister@gmail.com> <20190910140304.GA4683@lunn.ch>
In-Reply-To: <20190910140304.GA4683@lunn.ch>
From:   George McCollister <george.mccollister@gmail.com>
Date:   Tue, 10 Sep 2019 09:40:32 -0500
Message-ID: <CAFSKS=O9nDMk-ytxkFhTuZNT-QDmJ_twDdk2o2u0R4Y_YZ0z8A@mail.gmail.com>
Subject: Re: [PATCH net-next v2 1/3] net: dsa: microchip: add KSZ9477 I2C driver
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     netdev@vger.kernel.org, Woojung Huh <woojung.huh@microchip.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Tristram Ha <Tristram.Ha@microchip.com>,
        "David S. Miller" <davem@davemloft.net>,
        Marek Vasut <marex@denx.de>,
        open list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Andrew,

On Tue, Sep 10, 2019 at 9:03 AM Andrew Lunn <andrew@lunn.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