RE: [EXT] Re: [PATCH 1/8] ethosu: Add Arm Ethos-U driver

[Alison Wang]

On Fri, Jun 16, 2023 at 01:59:06PM +0800, Alison Wang wrote:
> Ethos-U Linux driver is to provide an example of how a rich operating
> system like Linux can dispatch inferences to an Arm Cortex-M
> subsystem, consisting of an Arm Cortex-M and an Arm Ethos-U NPU.
>
> Link:
> https://git/.
> mlplatform.org%2Fml%2Fethos-u%2Fethos-u-linux-driver-stack.git&data=05
> %7C01%7Calison.wang%40nxp.com%7C4b1681cd19b9450e832208db6e32709b%7C686
> ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638224935139578328%7CUnknown%7
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXV
> CI6Mn0%3D%7C3000%7C%7C%7C&sdata=jwhvw5i1Q2M%2FOLID8TsOOnaI9LqD6y%2BMnn
> jzdvdCXcw%3D&reserved=0
> Tag: 22.02

What does "Tag:" mean?

>
> Signed-off-by: Kristofer Jonsson <kristofer.jonsson@xxxxxxx>
> Signed-off-by: Per Astrand <per.astrand@xxxxxxx>
> Signed-off-by: Jonny Svärd <jonny.svaerd@xxxxxxx>
> Signed-off-by: Lior Dekel <Lior.dekel@xxxxxxx>
> Signed-off-by: Henrik Hoglind <henrik.hoglind@xxxxxxx>
> Signed-off-by: Davide Grohmann <davide.grohmann@xxxxxxx>
> Signed-off-by: Alison Wang <alison.wang@xxxxxxx>

If this many people signed off on this, it better be correct :)
[Alison Wang] The source codes of this patch come from the kernel folder of https://git.mlplatform.org/ml/ethos-u/ethos-u-linux-driver-stack.git, the detailed tag is 22.02.
So the source codes of this patch are written by Arm guys. The above people are all the authors. I list them all to avoid any missing.

> --- /dev/null
> +++ b/drivers/firmware/ethosu/Kconfig
> @@ -0,0 +1,24 @@
> +#
> +# (C) COPYRIGHT 2020 ARM Limited. All rights reserved.

It's not 2020 anymore.

> +#
> +# This program is free software and is provided to you under the
> +terms of the # GNU General Public License version 2 as published by
> +the Free Software # Foundation, and any use by you of this program is
> +subject to the terms # of such GNU licence.
> +#
> +# This program is distributed in the hope that it will be useful, #
> +but WITHOUT ANY WARRANTY; without even the implied warranty of #
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU
> +General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License #
> +along with this program; if not, you can access it online at #
> +http://www.gnu.org/licenses/gpl-2.0.html.
> +#
> +# SPDX-License-Identifier: GPL-2.0-only #
> +
> +config ETHOSU
> +    tristate "Arm Ethos-U NPU support"
> +    help
> +      Arm Ethos-U NPU driver.

{sigh}

With all of those people, NO ONE ran checkpatch.pl on the thing?

I'm stopping here, please go do that, fix up the obvious issues it tells you about (hint, no SPDX line and drop the huge license "boiler plate"
texts)

And also provide a REAL configuration help text, 4 words for this is just not acceptable, what would you do if you were asked to review this?
[Alison Wang] So sorry, I didn't do any change for the issues you mentioned above for this patch.
My previous idea is the first patch comes from Arm guys and the other patches are written by NXP guys. It seems this is not a good idea.
I will change this way in the next version.

Thanks a lot for your review.

thanks,

greg k-h