[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <PA4PR04MB963866B6DCE66489F177B8F8D181A@PA4PR04MB9638.eurprd04.prod.outlook.com>
Date: Fri, 1 Dec 2023 23:12:52 +0000
From: David Lin <yu-hao.lin@....com>
To: Kalle Valo <kvalo@...nel.org>,
Francesco Dolcini <francesco@...cini.it>
CC: "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"briannorris@...omium.org" <briannorris@...omium.org>,
Pete Hsieh <tsung-hsien.hsieh@....com>
Subject: RE: [EXT] Re: [PATCH v7 00/12] wifi: mwifiex: added code to support
host mlme.
> From: Kalle Valo <kvalo@...nel.org>
> Sent: Friday, December 1, 2023 8:25 PM
> To: Francesco Dolcini <francesco@...cini.it>
> Cc: David Lin <yu-hao.lin@....com>; linux-wireless@...r.kernel.org;
> linux-kernel@...r.kernel.org; briannorris@...omium.org; Pete Hsieh
> <tsung-hsien.hsieh@....com>
> 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
>
>
> Francesco Dolcini <francesco@...cini.it> writes:
>
> > 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/.
>
> I haven't looked at the actual patches but a generic comment from me is that
> usually it's not a good idea for newcomers to submit a huge patchset like
> this. Start with something small, just with one patch first, learn the process
> and what we require from patches. After you have gained more knowledge
> you can start doing more complex stuff.
>
Can you help to check this patch?
https://patchwork.kernel.org/project/linux-wireless/patch/20231128082544.613179-1-yu-hao.lin@nxp.com/
This is a very small patch and it really fixed issue of mwifiex.
> --
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw
> ork.kernel.org%2Fproject%2Flinux-wireless%2Flist%2F&data=05%7C01%7Cyu-
> hao.lin%40nxp.com%7C47171ff92b204d48d7be08dbf26894c4%7C686ea1d3b
> c2b4c6fa92cd99c5c301635%7C0%7C0%7C638370303262144837%7CUnknown
> %7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> wiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2FwH3UEUxKckAwlAVJkh
> 5LpR2L76a1uyOmxXVWiyTUmQ%3D&reserved=0
>
> 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%7C47171ff92b204d48d7be08d
> bf26894c4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63837030
> 3262144837%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj
> oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=F
> I%2BwsMoSatv3woPhT2yWhHeFIi5pP1uzFQYUrynAZO0%3D&reserved=0
Powered by blists - more mailing lists