On Mon, 4 Jul 2022 21:37:30 +0200 Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > On Mon, Jul 4, 2022 at 12:04 AM Jakob Hauser <jahau@xxxxxxxxxxxxxx> wrote: > > > > This commit introduces the "chip_info" structure approach for better variant > > handling. > > > > The variant to be used is now chosen by the devicetree (enum "chip_ids"), > > Device Tree > > > not by the chip ID in the register. However, there is a check to make sure > > they match (using integer "id_check"). > > ... > > Thanks for a new version, it's getting better. My comments below. > > But first of all, can you split this to at least two patches, i.e. > 1) split out functions without introducing chip->info yet; > 2) adding chip_info. > > Possible alternative would be more steps in 2), i.e. introducing > chip_info for the callback only, then add field (or semantically > unified fields) by field with corresponding changes in the code. In > this case it would be easier to review. > > I leave this exercise to you if Jonathan thinks it worth it. You are of course correct that it would be nicer to have it split, but I'm not going to be fussy about it this time ;) Other than addressing Andy's eagle eyed review comments, this series looks good to me. Thanks, Jonathan
