Messages in this thread | | | From | George McCollister <> | Date | Tue, 10 Sep 2019 09:40:32 -0500 | 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@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
| |