> From: Francesco Dolcini <francesco@xxxxxxxxxx> > Sent: Friday, December 1, 2023 7:49 PM > To: David Lin <yu-hao.lin@xxxxxxx> > Cc: linux-wireless@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > briannorris@xxxxxxxxxxxx; kvalo@xxxxxxxxxx; francesco@xxxxxxxxxx; Pete > Hsieh <tsung-hsien.hsieh@xxxxxxx> > Subject: [EXT] Re: [PATCH v7 00/12] wifi: mwifiex: added code to support > host mlme. > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > Hello Lin, > thanks for the patches here, I can clearly see that this code is going through > some real testing given the improvements you did lately. > > I have commented on the single patches, and honestly I did not look into the > code details at the moment. > > The major feedback from me is the following: > 1 - you should not add code with a bug and than fix a bug in the same > series, you should have a non buggy patch in the first place (e.g. > git --amend). (this applies till the patch is not merged into the > maintainer tree, of course). > 2 - point 1 applies also to reviewer comments > 3 - if you have fixes that are not connected to the feature addition > you are doing is beneficial to have those separated, this makes > reviewing easier, they can be "prioritized" to some extent (given > that they are fixes) and follow a slightly different patch flow > (they can get applied, depending on the maintainers decision, when > the > merge window is closed and should be backported). Not to mention > that smaller patch series are appreciated, "Maximum of 7-12 patches > per patchset " from [1] > > In general I would suggest you to have a look at [1], not sure how up to date > is that compared to the in-tree Documentation/process/. > > On Tue, Nov 28, 2023 at 04:31:03PM +0800, David Lin wrote: > > 5. Address reviewer comments. > You should list the changes you did, something that generic is forcing the > reviewer to compare v7 vs v6 to known what changed. Can I summary what should I do and hopefully I can make agreement with you: 1. Separate patch v7 6/12 as a single patch. 2. Merged all other patches as a single patch for host mlme. So there should be no patch v8 and only have two patches, one for host mlme and another one to fix hostap restart issue. > > > [1] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwireles > s.wiki.kernel.org%2Fen%2Fdevelopers%2Fdocumentation%2Fsubmittingpatch > es&data=05%7C01%7Cyu-hao.lin%40nxp.com%7Cde461eef26c44e28e80c08d > bf2638639%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63837028 > 1495779090%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj > oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=y > OW9SndPm%2FDuI%2BZU8fxBieerDxVZp5RzefQiSfFA%2BW0%3D&reserved=0 > > Francesco