On 2020-01-08 2:23 p.m., Bjorn Helgaas wrote: > On Mon, Jan 06, 2020 at 12:03:34PM -0700, Logan Gunthorpe wrote: >> From: Kelvin Cao <kelvin.cao@xxxxxxxxxxxxx> >> >> Add a union with gen3 and gen4 flash_info structs. > > This does a lot more than add a union :) > > I think this looks reasonable, but I would like it even better if this > and related patches could be split up a little bit differently: > > - Rename SWITCHTEC_CFG0_RUNNING to SWITCHTEC_GEN3_CFG0_RUNNING, etc > (purely mechanical change, so trivial and obvious). > > - Add switchtec_gen and the tests where it's needed, but with only > SWITCHTEC_GEN3 cases for now. > > - Refactor ioctl_flash_part_info() (still only supports GEN3). > Maybe adds struct flash_info_regs and union, but only with gen3. > > - Add GEN4 support (patch basically contains only GEN4-related > things and doesn't touch GEN3 things at all). Maybe it would > still make sense to split the GEN4 support into multiple patches > (as in this series), or maybe they could be squashed into a single > GEN4 patch? > > - It seems like at least the aliasing quirk and the driver device ID > update could/should be squashed since they contain the same > constants. Thanks for the review. Yes, I should be able to clean this up and submit a v2 in the next week or two. Logan
