[PATCH 0/4] Adding support to load secure firmware for HS devices

[PATCH 0/4] Adding support to load secure firmware for HS devices

[Udit Kumar]

Some use case needs rproc firmware to be loaded at u-boot stage,
using following commands at u-boot shell, firmware could be loaded

=> setenv dorprocboot 1
=> run boot_rprocs

For Secure devices, secure version of rproc firmware should be loaded,
which is appended by sec keyword[0].
but currently non-secure firmware is loaded even for secure devices.
So adding support for loading secure firmware on Secured devices.

[0]: https://gist.github.com/uditkumarti/cd8bf6a448079b59145d17a0e8bf13b7

Bootlogs:
GP : https://gist.github.com/uditkumarti/23a00c313e1c28b62537aab733a585df#file-gp_device line 65 onwards
HS : https://gist.github.com/uditkumarti/23a00c313e1c28b62537aab733a585df#file-hs-device line 60 onwards


Manorit Chawdhry (3):
  include: mach-k3: move k3 security functions to security.h
  drivers: remoteproc: ti_k3 : enable secure booting with firmware
    images
  mach-k3: common.c: add a flag for booting authenticated rproc binaries

Udit Kumar (1):
  include: env: ti: Add support for secure firmwares

 arch/arm/mach-k3/common.c                |  4 +++-
 arch/arm/mach-k3/common.h                |  4 ++--
 arch/arm/mach-k3/include/mach/security.h | 11 +++++++++++
 drivers/remoteproc/ti_k3_dsp_rproc.c     |  4 ++++
 drivers/remoteproc/ti_k3_r5f_rproc.c     |  4 ++++
 include/env/ti/k3_rproc.env              |  6 +++++-
 include/env/ti/ti_common.env             |  1 +
 7 files changed, 30 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/mach-k3/include/mach/security.h

-- 
2.34.1

[PATCH 1/4] include: mach-k3: move k3 security functions to security.h

[Udit Kumar]

From: Manorit Chawdhry <m-chawdhry@ti.com>

ti_secure_image_post_process and ti_secure_image_check_binary is used
for the authentication purposes in the current boot flow. Authentication
of remoteproc firmware images require ti_secure_image_post_process to be
available outside mach-k3.

Signed-off-by: Manorit Chawdhry <m-chawdhry@ti.com>
Signed-off-by: Udit Kumar <u-kumar1@ti.com>
---
 arch/arm/mach-k3/common.h                |  4 ++--
 arch/arm/mach-k3/include/mach/security.h | 11 +++++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/mach-k3/include/mach/security.h

diff --git a/arch/arm/mach-k3/common.h b/arch/arm/mach-k3/common.h
index 53aa186b31..7bd72da1de 100644
--- a/arch/arm/mach-k3/common.h
+++ b/arch/arm/mach-k3/common.h
@@ -2,12 +2,13 @@
 /*
  * K3: Architecture common definitions
  *
- * Copyright (C) 2018 Texas Instruments Incorporated - https://www.ti.com/
+ * Copyright (C) 2018-2024 Texas Instruments Incorporated - https://www.ti.com/
  *	Lokesh Vutla <lokeshvutla@ti.com>
  */
 
 #include <asm/armv7_mpu.h>
 #include <asm/hardware.h>
+#include <mach/security.h>
 
 #define K3_FIREWALL_BACKGROUND_BIT (8)
 
@@ -41,7 +42,6 @@ void spl_enable_cache(void);
 void mmr_unlock(uintptr_t base, u32 partition);
 bool is_rom_loaded_sysfw(struct rom_extended_boot_data *data);
 enum k3_device_type get_device_type(void);
-void ti_secure_image_post_process(void **p_image, size_t *p_size);
 struct ti_sci_handle *get_ti_sci_handle(void);
 void do_board_detect(void);
 void ti_secure_image_check_binary(void **p_image, size_t *p_size);
diff --git a/arch/arm/mach-k3/include/mach/security.h b/arch/arm/mach-k3/include/mach/security.h
new file mode 100644
index 0000000000..8502b57bd8
--- /dev/null
+++ b/arch/arm/mach-k3/include/mach/security.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * K3: Security related definitions
+ *
+ * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
+ *	Manorit Chawdhry <m-chawdhry@ti.com>
+ */
+
+#include <linux/types.h>
+
+void ti_secure_image_post_process(void **p_image, size_t *p_size);
-- 
2.34.1

[PATCH 2/4] drivers: remoteproc: ti_k3 : enable secure booting with firmware images

[Udit Kumar]

From: Manorit Chawdhry <m-chawdhry@ti.com>

Remoteproc firmware images aren't authenticated in the current boot flow.
Authenticates remoteproc firmware images to complete the root of trust
in secure booting.

Signed-off-by: Manorit Chawdhry <m-chawdhry@ti.com>
---
 drivers/remoteproc/ti_k3_dsp_rproc.c | 4 ++++
 drivers/remoteproc/ti_k3_r5f_rproc.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/remoteproc/ti_k3_dsp_rproc.c b/drivers/remoteproc/ti_k3_dsp_rproc.c
index 57fe1037da..7617bbb986 100644
--- a/drivers/remoteproc/ti_k3_dsp_rproc.c
+++ b/drivers/remoteproc/ti_k3_dsp_rproc.c
@@ -21,6 +21,7 @@
 #include <linux/sizes.h>
 #include <linux/soc/ti/ti_sci_protocol.h>
 #include "ti_sci_proc.h"
+#include <mach/security.h>
 
 #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK	(SZ_16M - 1)
 
@@ -127,6 +128,7 @@ static int k3_dsp_load(struct udevice *dev, ulong addr, ulong size)
 	struct k3_dsp_privdata *dsp = dev_get_priv(dev);
 	struct k3_dsp_boot_data *data = dsp->data;
 	u32 boot_vector;
+	void *image_addr = (void *)addr;
 	int ret;
 
 	if (dsp->in_use) {
@@ -148,6 +150,8 @@ static int k3_dsp_load(struct udevice *dev, ulong addr, ulong size)
 		goto proc_release;
 	}
 
+	ti_secure_image_post_process(&image_addr, &size);
+
 	ret = rproc_elf_load_image(dev, addr, size);
 	if (ret < 0) {
 		dev_err(dev, "Loading elf failed %d\n", ret);
diff --git a/drivers/remoteproc/ti_k3_r5f_rproc.c b/drivers/remoteproc/ti_k3_r5f_rproc.c
index b55b1dc10d..b9c6549e18 100644
--- a/drivers/remoteproc/ti_k3_r5f_rproc.c
+++ b/drivers/remoteproc/ti_k3_r5f_rproc.c
@@ -20,6 +20,7 @@
 #include <linux/kernel.h>
 #include <linux/soc/ti/ti_sci_protocol.h>
 #include "ti_sci_proc.h"
+#include <mach/security.h>
 
 /*
  * R5F's view of this address can either be for ATCM or BTCM with the other
@@ -301,6 +302,7 @@ static int k3_r5f_load(struct udevice *dev, ulong addr, ulong size)
 	u64 boot_vector;
 	u32 ctrl, sts, cfg = 0;
 	bool mem_auto_init;
+	void *image_addr = (void *)addr;
 	int ret;
 
 	dev_dbg(dev, "%s addr = 0x%lx, size = 0x%lx\n", __func__, addr, size);
@@ -328,6 +330,8 @@ static int k3_r5f_load(struct udevice *dev, ulong addr, ulong size)
 
 	k3_r5f_init_tcm_memories(core, mem_auto_init);
 
+	ti_secure_image_post_process(&image_addr, &size);
+
 	ret = rproc_elf_load_image(dev, addr, size);
 	if (ret < 0) {
 		dev_err(dev, "Loading elf failedi %d\n", ret);
-- 
2.34.1

[PATCH 3/4] include: env: ti: Add support for secure firmwares

[Udit Kumar]

Secure firmwares must be loaded if SOC is secure,
currently rproc framework chooses non-secure firmware always.

So adding support to load secure firmware, when SOC is secure

Signed-off-by: Manorit Chawdhry <m-chawdhry@ti.com>
Signed-off-by: Udit Kumar <u-kumar1@ti.com>
---
 include/env/ti/k3_rproc.env  | 6 +++++-
 include/env/ti/ti_common.env | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/env/ti/k3_rproc.env b/include/env/ti/k3_rproc.env
index 87d9d76eba..d4f0f1708c 100644
--- a/include/env/ti/k3_rproc.env
+++ b/include/env/ti/k3_rproc.env
@@ -13,11 +13,15 @@ rproc_load_and_boot_one=
 boot_rprocs_mmc=
 	env set rproc_id;
 	env set rproc_fw;
+	env set secure_suffix;
+	if test ${secure_rprocs} -eq 1; then
+		env set secure_suffix -sec;
+	fi;
 	for i in ${rproc_fw_binaries} ; do
 		if test -z "${rproc_id}" ; then
 			env set rproc_id $i;
 		else
-			env set rproc_fw $i;
+			env set rproc_fw $i${secure_suffix};
 			run rproc_load_and_boot_one;
 			env set rproc_id;
 			env set rproc_fw;
diff --git a/include/env/ti/ti_common.env b/include/env/ti/ti_common.env
index 02b410c3ad..c5c3642177 100644
--- a/include/env/ti/ti_common.env
+++ b/include/env/ti/ti_common.env
@@ -12,6 +12,7 @@ bootm_size=0x10000000
 boot_fdt=try
 
 boot_fit=0
+secure_rprocs=0
 addr_fit=0x90000000
 name_fit=fitImage
 update_to_fit=setenv loadaddr ${addr_fit}; setenv bootfile ${name_fit}
-- 
2.34.1

[PATCH 4/4] mach-k3: common.c: add a flag for booting authenticated rproc binaries

[Udit Kumar]

From: Manorit Chawdhry <m-chawdhry@ti.com>

The flag will be used for booting authenticated remote procs from hs-se
devices which can optionally be used in hs-fs devices also.

Signed-off-by: Manorit Chawdhry <m-chawdhry@ti.com>
Signed-off-by: Udit Kumar <u-kumar1@ti.com>
---
 arch/arm/mach-k3/common.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
index 1a269d6934..eaa7d36176 100644
--- a/arch/arm/mach-k3/common.c
+++ b/arch/arm/mach-k3/common.c
@@ -283,8 +283,10 @@ int misc_init_r(void)
 	}
 
 	/* Default FIT boot on HS-SE devices */
-	if (get_device_type() == K3_DEVICE_TYPE_HS_SE)
+	if (get_device_type() == K3_DEVICE_TYPE_HS_SE) {
 		env_set("boot_fit", "1");
+		env_set("secure_rprocs", "1");
+	}
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH 0/4] Adding support to load secure firmware for HS devices

[Andy Shevchenko]

On Tue, May 21, 2024 at 04:26:44PM +0530, Udit Kumar wrote:
> Some use case needs rproc firmware to be loaded at u-boot stage,
> using following commands at u-boot shell, firmware could be loaded
> 
> => setenv dorprocboot 1
> => run boot_rprocs
> 
> For Secure devices, secure version of rproc firmware should be loaded,
> which is appended by sec keyword[0].
> but currently non-secure firmware is loaded even for secure devices.
> So adding support for loading secure firmware on Secured devices.

Are you Cc'ing random people with this?
Please don't do it.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/4] Adding support to load secure firmware for HS devices

[Kumar, Udit]

On 5/21/2024 7:59 PM, Andy Shevchenko wrote:
> On Tue, May 21, 2024 at 04:26:44PM +0530, Udit Kumar wrote:
>> Some use case needs rproc firmware to be loaded at u-boot stage,
>> using following commands at u-boot shell, firmware could be loaded
>>
>> => setenv dorprocboot 1
>> => run boot_rprocs
>>
>> For Secure devices, secure version of rproc firmware should be loaded,
>> which is appended by sec keyword[0].
>> but currently non-secure firmware is loaded even for secure devices.
>> So adding support for loading secure firmware on Secured devices.
> Are you Cc'ing random people with this?
> Please don't do it.

I used names names popped up while running get_maintainer.pl script of 
u-boot

your email id was there for patch-3 of this series

./scripts/get_maintainer.pl 
0003-include-env-ti-Add-support-for-secure-firmwares.patch
Joe Hershberger <joe.hershberger@ni.com> (maintainer:ENVIRONMENT)
Tom Rini <trini@konsulko.com> (maintainer:THE REST)
Udit Kumar <u-kumar1@ti.com> 
(commit_signer:1/2=50%,authored:1/2=50%,commit_signer:1/7=14%,authored:1/7=14%)
Simon Glass <sjg@chromium.org> 
(commit_signer:1/2=50%,authored:1/2=50%,authored:1/7=14%)
Andy Shevchenko <andriy.shevchenko@linux.intel.com> (commit_signer:1/2=50%)
Bin Meng <bmeng.cn@gmail.com> (commit_signer:1/2=50%)
Manorit Chawdhry <m-chawdhry@ti.com> 
(commit_signer:1/2=50%,commit_signer:5/7=71%,authored:4/7=57%)
Mattijs Korpershoek <mkorpershoek@baylibre.com> (commit_signer:4/7=57%)
Nishanth Menon <nm@ti.com> (commit_signer:1/7=14%,authored:1/7=14%)
Siddharth Vadapalli <s-vadapalli@ti.com> (commit_signer:1/7=14%)
u-boot@lists.denx.de (open list)

Re: [PATCH 0/4] Adding support to load secure firmware for HS devices

[Andy Shevchenko]

On Tue, May 21, 2024 at 08:59:17PM +0530, Kumar, Udit wrote:
> On 5/21/2024 7:59 PM, Andy Shevchenko wrote:
> > On Tue, May 21, 2024 at 04:26:44PM +0530, Udit Kumar wrote:

...

> > Are you Cc'ing random people with this?
> > Please don't do it.
> 
> I used names names popped up while running get_maintainer.pl script of
> u-boot
> 
> your email id was there for patch-3 of this series

Nice, the tool is broken. Please, work with the respective people to fix a tool
or to tune it to avoid such mistakes in the future.

(Hint: I would first disable Git fallback completely)

> ./scripts/get_maintainer.pl
> 0003-include-env-ti-Add-support-for-secure-firmwares.patch
> Joe Hershberger <joe.hershberger@ni.com> (maintainer:ENVIRONMENT)
> Tom Rini <trini@konsulko.com> (maintainer:THE REST)
> Udit Kumar <u-kumar1@ti.com> (commit_signer:1/2=50%,authored:1/2=50%,commit_signer:1/7=14%,authored:1/7=14%)
> Simon Glass <sjg@chromium.org>
> (commit_signer:1/2=50%,authored:1/2=50%,authored:1/7=14%)
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> (commit_signer:1/2=50%)
> Bin Meng <bmeng.cn@gmail.com> (commit_signer:1/2=50%)
> Manorit Chawdhry <m-chawdhry@ti.com>
> (commit_signer:1/2=50%,commit_signer:5/7=71%,authored:4/7=57%)
> Mattijs Korpershoek <mkorpershoek@baylibre.com> (commit_signer:4/7=57%)
> Nishanth Menon <nm@ti.com> (commit_signer:1/7=14%,authored:1/7=14%)
> Siddharth Vadapalli <s-vadapalli@ti.com> (commit_signer:1/7=14%)
> u-boot@lists.denx.de (open list)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/4] Adding support to load secure firmware for HS devices

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1536 bytes --]

On Tue, May 21, 2024 at 07:12:09PM +0300, Andy Shevchenko wrote:
> On Tue, May 21, 2024 at 08:59:17PM +0530, Kumar, Udit wrote:
> > On 5/21/2024 7:59 PM, Andy Shevchenko wrote:
> > > On Tue, May 21, 2024 at 04:26:44PM +0530, Udit Kumar wrote:
> 
> ...
> 
> > > Are you Cc'ing random people with this?
> > > Please don't do it.
> > 
> > I used names names popped up while running get_maintainer.pl script of
> > u-boot
> > 
> > your email id was there for patch-3 of this series
> 
> Nice, the tool is broken. Please, work with the respective people to fix a tool
> or to tune it to avoid such mistakes in the future.
> 
> (Hint: I would first disable Git fallback completely)

So you were cc'd thanks to:
commit 5e541a05f70296dbad07fc1f4e8678ad207bb476
Author: Simon Glass <sjg@chromium.org>
Date:   Sun Jul 30 21:01:45 2023 -0600

    env: Use include/env for text-environment includes
    
    The 'environment' word is too long. We mostly use 'env' in U-Boot, so use
    that as the name of the include directory too.
    
    Signed-off-by: Simon Glass <sjg@chromium.org>
    Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # Intel Edison
    Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

And, I'm sorry because there are other contributors who get upset when git
fallback doesn't find them (because it also ages out after a year).
In this case there's not even a "tweak the threshold" option since
there's only a single commit to the file so you're in 100% of them.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 0/4] Adding support to load secure firmware for HS devices

[Andy Shevchenko]

On Tue, May 21, 2024 at 10:21:45AM -0600, Tom Rini wrote:
> On Tue, May 21, 2024 at 07:12:09PM +0300, Andy Shevchenko wrote:
> > On Tue, May 21, 2024 at 08:59:17PM +0530, Kumar, Udit wrote:
> > > On 5/21/2024 7:59 PM, Andy Shevchenko wrote:
> > > > On Tue, May 21, 2024 at 04:26:44PM +0530, Udit Kumar wrote:

...

> > > > Are you Cc'ing random people with this?
> > > > Please don't do it.
> > > 
> > > I used names names popped up while running get_maintainer.pl script of
> > > u-boot
> > > 
> > > your email id was there for patch-3 of this series
> > 
> > Nice, the tool is broken. Please, work with the respective people to fix a tool
> > or to tune it to avoid such mistakes in the future.
> > 
> > (Hint: I would first disable Git fallback completely)
> 
> So you were cc'd thanks to:
> commit 5e541a05f70296dbad07fc1f4e8678ad207bb476
> Author: Simon Glass <sjg@chromium.org>
> Date:   Sun Jul 30 21:01:45 2023 -0600
> 
>     env: Use include/env for text-environment includes
>     
>     The 'environment' word is too long. We mostly use 'env' in U-Boot, so use
>     that as the name of the include directory too.
>     
>     Signed-off-by: Simon Glass <sjg@chromium.org>
>     Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # Intel Edison

Yeah, and it even explicitly stated what I reviewed from that code...

>     Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> 
> And, I'm sorry because there are other contributors who get upset when git
> fallback doesn't find them (because it also ages out after a year).
> In this case there's not even a "tweak the threshold" option since
> there's only a single commit to the file so you're in 100% of them.

Just an example of false positive by get_maintainer.pl :-(
And a confirmation of what I said — tool is broken.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/4] Adding support to load secure firmware for HS devices

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2243 bytes --]

On Tue, May 21, 2024 at 07:35:55PM +0300, Andy Shevchenko wrote:
> On Tue, May 21, 2024 at 10:21:45AM -0600, Tom Rini wrote:
> > On Tue, May 21, 2024 at 07:12:09PM +0300, Andy Shevchenko wrote:
> > > On Tue, May 21, 2024 at 08:59:17PM +0530, Kumar, Udit wrote:
> > > > On 5/21/2024 7:59 PM, Andy Shevchenko wrote:
> > > > > On Tue, May 21, 2024 at 04:26:44PM +0530, Udit Kumar wrote:
> 
> ...
> 
> > > > > Are you Cc'ing random people with this?
> > > > > Please don't do it.
> > > > 
> > > > I used names names popped up while running get_maintainer.pl script of
> > > > u-boot
> > > > 
> > > > your email id was there for patch-3 of this series
> > > 
> > > Nice, the tool is broken. Please, work with the respective people to fix a tool
> > > or to tune it to avoid such mistakes in the future.
> > > 
> > > (Hint: I would first disable Git fallback completely)
> > 
> > So you were cc'd thanks to:
> > commit 5e541a05f70296dbad07fc1f4e8678ad207bb476
> > Author: Simon Glass <sjg@chromium.org>
> > Date:   Sun Jul 30 21:01:45 2023 -0600
> > 
> >     env: Use include/env for text-environment includes
> >     
> >     The 'environment' word is too long. We mostly use 'env' in U-Boot, so use
> >     that as the name of the include directory too.
> >     
> >     Signed-off-by: Simon Glass <sjg@chromium.org>
> >     Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # Intel Edison
> 
> Yeah, and it even explicitly stated what I reviewed from that code...
> 
> >     Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > 
> > And, I'm sorry because there are other contributors who get upset when git
> > fallback doesn't find them (because it also ages out after a year).
> > In this case there's not even a "tweak the threshold" option since
> > there's only a single commit to the file so you're in 100% of them.
> 
> Just an example of false positive by get_maintainer.pl :-(
> And a confirmation of what I said — tool is broken.

It's an imperfect tool, yes. A quick check shows that if you never want
to be cc'd from a git tag, you can add yourself to
.get_maintainer.ignore and you will still be matched for things you have
an explicit MAINTAINER entry for.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 0/4] Adding support to load secure firmware for HS devices

[Andy Shevchenko]

On Tue, May 21, 2024 at 10:45:41AM -0600, Tom Rini wrote:
> On Tue, May 21, 2024 at 07:35:55PM +0300, Andy Shevchenko wrote:
> > On Tue, May 21, 2024 at 10:21:45AM -0600, Tom Rini wrote:
> > > On Tue, May 21, 2024 at 07:12:09PM +0300, Andy Shevchenko wrote:
> > > > On Tue, May 21, 2024 at 08:59:17PM +0530, Kumar, Udit wrote:
> > > > > On 5/21/2024 7:59 PM, Andy Shevchenko wrote:
> > > > > > On Tue, May 21, 2024 at 04:26:44PM +0530, Udit Kumar wrote:

...

> > > > > > Are you Cc'ing random people with this?
> > > > > > Please don't do it.
> > > > > 
> > > > > I used names names popped up while running get_maintainer.pl script of
> > > > > u-boot
> > > > > 
> > > > > your email id was there for patch-3 of this series
> > > > 
> > > > Nice, the tool is broken. Please, work with the respective people to fix a tool
> > > > or to tune it to avoid such mistakes in the future.
> > > > 
> > > > (Hint: I would first disable Git fallback completely)
> > > 
> > > So you were cc'd thanks to:
> > > commit 5e541a05f70296dbad07fc1f4e8678ad207bb476
> > > Author: Simon Glass <sjg@chromium.org>
> > > Date:   Sun Jul 30 21:01:45 2023 -0600
> > > 
> > >     env: Use include/env for text-environment includes
> > >     
> > >     The 'environment' word is too long. We mostly use 'env' in U-Boot, so use
> > >     that as the name of the include directory too.
> > >     
> > >     Signed-off-by: Simon Glass <sjg@chromium.org>
> > >     Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # Intel Edison
> > 
> > Yeah, and it even explicitly stated what I reviewed from that code...
> > 
> > >     Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > 
> > > And, I'm sorry because there are other contributors who get upset when git
> > > fallback doesn't find them (because it also ages out after a year).
> > > In this case there's not even a "tweak the threshold" option since
> > > there's only a single commit to the file so you're in 100% of them.
> > 
> > Just an example of false positive by get_maintainer.pl :-(
> > And a confirmation of what I said — tool is broken.
> 
> It's an imperfect tool, yes. A quick check shows that if you never want
> to be cc'd from a git tag, you can add yourself to
> .get_maintainer.ignore and you will still be matched for things you have
> an explicit MAINTAINER entry for.

But this will kill me being a maintainer of U-Boot parts (regarding to
a certain platform) which is not what I want.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/4] Adding support to load secure firmware for HS devices

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2810 bytes --]

On Tue, May 21, 2024 at 08:48:54PM +0300, Andy Shevchenko wrote:
> On Tue, May 21, 2024 at 10:45:41AM -0600, Tom Rini wrote:
> > On Tue, May 21, 2024 at 07:35:55PM +0300, Andy Shevchenko wrote:
> > > On Tue, May 21, 2024 at 10:21:45AM -0600, Tom Rini wrote:
> > > > On Tue, May 21, 2024 at 07:12:09PM +0300, Andy Shevchenko wrote:
> > > > > On Tue, May 21, 2024 at 08:59:17PM +0530, Kumar, Udit wrote:
> > > > > > On 5/21/2024 7:59 PM, Andy Shevchenko wrote:
> > > > > > > On Tue, May 21, 2024 at 04:26:44PM +0530, Udit Kumar wrote:
> 
> ...
> 
> > > > > > > Are you Cc'ing random people with this?
> > > > > > > Please don't do it.
> > > > > > 
> > > > > > I used names names popped up while running get_maintainer.pl script of
> > > > > > u-boot
> > > > > > 
> > > > > > your email id was there for patch-3 of this series
> > > > > 
> > > > > Nice, the tool is broken. Please, work with the respective people to fix a tool
> > > > > or to tune it to avoid such mistakes in the future.
> > > > > 
> > > > > (Hint: I would first disable Git fallback completely)
> > > > 
> > > > So you were cc'd thanks to:
> > > > commit 5e541a05f70296dbad07fc1f4e8678ad207bb476
> > > > Author: Simon Glass <sjg@chromium.org>
> > > > Date:   Sun Jul 30 21:01:45 2023 -0600
> > > > 
> > > >     env: Use include/env for text-environment includes
> > > >     
> > > >     The 'environment' word is too long. We mostly use 'env' in U-Boot, so use
> > > >     that as the name of the include directory too.
> > > >     
> > > >     Signed-off-by: Simon Glass <sjg@chromium.org>
> > > >     Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # Intel Edison
> > > 
> > > Yeah, and it even explicitly stated what I reviewed from that code...
> > > 
> > > >     Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > 
> > > > And, I'm sorry because there are other contributors who get upset when git
> > > > fallback doesn't find them (because it also ages out after a year).
> > > > In this case there's not even a "tweak the threshold" option since
> > > > there's only a single commit to the file so you're in 100% of them.
> > > 
> > > Just an example of false positive by get_maintainer.pl :-(
> > > And a confirmation of what I said — tool is broken.
> > 
> > It's an imperfect tool, yes. A quick check shows that if you never want
> > to be cc'd from a git tag, you can add yourself to
> > .get_maintainer.ignore and you will still be matched for things you have
> > an explicit MAINTAINER entry for.
> 
> But this will kill me being a maintainer of U-Boot parts (regarding to
> a certain platform) which is not what I want.

With a quick local test, no, it still reported you for edison stuff
since that's not where the ignore list comes in.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 0/4] Adding support to load secure firmware for HS devices

[Andy Shevchenko]

On Tue, May 21, 2024 at 11:52:47AM -0600, Tom Rini wrote:
> On Tue, May 21, 2024 at 08:48:54PM +0300, Andy Shevchenko wrote:
> > On Tue, May 21, 2024 at 10:45:41AM -0600, Tom Rini wrote:
> > > On Tue, May 21, 2024 at 07:35:55PM +0300, Andy Shevchenko wrote:
> > > > On Tue, May 21, 2024 at 10:21:45AM -0600, Tom Rini wrote:
> > > > > On Tue, May 21, 2024 at 07:12:09PM +0300, Andy Shevchenko wrote:
> > > > > > On Tue, May 21, 2024 at 08:59:17PM +0530, Kumar, Udit wrote:
> > > > > > > On 5/21/2024 7:59 PM, Andy Shevchenko wrote:
> > > > > > > > On Tue, May 21, 2024 at 04:26:44PM +0530, Udit Kumar wrote:

...

> > > > > > > > Are you Cc'ing random people with this?
> > > > > > > > Please don't do it.
> > > > > > > 
> > > > > > > I used names names popped up while running get_maintainer.pl script of
> > > > > > > u-boot
> > > > > > > 
> > > > > > > your email id was there for patch-3 of this series
> > > > > > 
> > > > > > Nice, the tool is broken. Please, work with the respective people to fix a tool
> > > > > > or to tune it to avoid such mistakes in the future.
> > > > > > 
> > > > > > (Hint: I would first disable Git fallback completely)
> > > > > 
> > > > > So you were cc'd thanks to:
> > > > > commit 5e541a05f70296dbad07fc1f4e8678ad207bb476
> > > > > Author: Simon Glass <sjg@chromium.org>
> > > > > Date:   Sun Jul 30 21:01:45 2023 -0600
> > > > > 
> > > > >     env: Use include/env for text-environment includes
> > > > >     
> > > > >     The 'environment' word is too long. We mostly use 'env' in U-Boot, so use
> > > > >     that as the name of the include directory too.
> > > > >     
> > > > >     Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > >     Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # Intel Edison
> > > > 
> > > > Yeah, and it even explicitly stated what I reviewed from that code...
> > > > 
> > > > >     Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > 
> > > > > And, I'm sorry because there are other contributors who get upset when git
> > > > > fallback doesn't find them (because it also ages out after a year).
> > > > > In this case there's not even a "tweak the threshold" option since
> > > > > there's only a single commit to the file so you're in 100% of them.
> > > > 
> > > > Just an example of false positive by get_maintainer.pl :-(
> > > > And a confirmation of what I said — tool is broken.
> > > 
> > > It's an imperfect tool, yes. A quick check shows that if you never want
> > > to be cc'd from a git tag, you can add yourself to
> > > .get_maintainer.ignore and you will still be matched for things you have
> > > an explicit MAINTAINER entry for.
> > 
> > But this will kill me being a maintainer of U-Boot parts (regarding to
> > a certain platform) which is not what I want.
> 
> With a quick local test, no, it still reported you for edison stuff
> since that's not where the ignore list comes in.

I see. Still looking at the rate of false positives so far from U-Boot project
I don't see that a big hammer, like the proposed one, should be used. Better to
teach people on how to use/tune the tool, it will be helpful in a long-term.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/4] Adding support to load secure firmware for HS devices

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3896 bytes --]

On Tue, May 21, 2024 at 09:55:19PM +0300, Andy Shevchenko wrote:
> On Tue, May 21, 2024 at 11:52:47AM -0600, Tom Rini wrote:
> > On Tue, May 21, 2024 at 08:48:54PM +0300, Andy Shevchenko wrote:
> > > On Tue, May 21, 2024 at 10:45:41AM -0600, Tom Rini wrote:
> > > > On Tue, May 21, 2024 at 07:35:55PM +0300, Andy Shevchenko wrote:
> > > > > On Tue, May 21, 2024 at 10:21:45AM -0600, Tom Rini wrote:
> > > > > > On Tue, May 21, 2024 at 07:12:09PM +0300, Andy Shevchenko wrote:
> > > > > > > On Tue, May 21, 2024 at 08:59:17PM +0530, Kumar, Udit wrote:
> > > > > > > > On 5/21/2024 7:59 PM, Andy Shevchenko wrote:
> > > > > > > > > On Tue, May 21, 2024 at 04:26:44PM +0530, Udit Kumar wrote:
> 
> ...
> 
> > > > > > > > > Are you Cc'ing random people with this?
> > > > > > > > > Please don't do it.
> > > > > > > > 
> > > > > > > > I used names names popped up while running get_maintainer.pl script of
> > > > > > > > u-boot
> > > > > > > > 
> > > > > > > > your email id was there for patch-3 of this series
> > > > > > > 
> > > > > > > Nice, the tool is broken. Please, work with the respective people to fix a tool
> > > > > > > or to tune it to avoid such mistakes in the future.
> > > > > > > 
> > > > > > > (Hint: I would first disable Git fallback completely)
> > > > > > 
> > > > > > So you were cc'd thanks to:
> > > > > > commit 5e541a05f70296dbad07fc1f4e8678ad207bb476
> > > > > > Author: Simon Glass <sjg@chromium.org>
> > > > > > Date:   Sun Jul 30 21:01:45 2023 -0600
> > > > > > 
> > > > > >     env: Use include/env for text-environment includes
> > > > > >     
> > > > > >     The 'environment' word is too long. We mostly use 'env' in U-Boot, so use
> > > > > >     that as the name of the include directory too.
> > > > > >     
> > > > > >     Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > >     Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # Intel Edison
> > > > > 
> > > > > Yeah, and it even explicitly stated what I reviewed from that code...
> > > > > 
> > > > > >     Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > 
> > > > > > And, I'm sorry because there are other contributors who get upset when git
> > > > > > fallback doesn't find them (because it also ages out after a year).
> > > > > > In this case there's not even a "tweak the threshold" option since
> > > > > > there's only a single commit to the file so you're in 100% of them.
> > > > > 
> > > > > Just an example of false positive by get_maintainer.pl :-(
> > > > > And a confirmation of what I said — tool is broken.
> > > > 
> > > > It's an imperfect tool, yes. A quick check shows that if you never want
> > > > to be cc'd from a git tag, you can add yourself to
> > > > .get_maintainer.ignore and you will still be matched for things you have
> > > > an explicit MAINTAINER entry for.
> > > 
> > > But this will kill me being a maintainer of U-Boot parts (regarding to
> > > a certain platform) which is not what I want.
> > 
> > With a quick local test, no, it still reported you for edison stuff
> > since that's not where the ignore list comes in.
> 
> I see. Still looking at the rate of false positives so far from U-Boot project
> I don't see that a big hammer, like the proposed one, should be used. Better to
> teach people on how to use/tune the tool, it will be helpful in a long-term.

Long term, it would be best if nearly everything had a real MAINTAINERS
entry. Short term, I don't know how to reconcile "I don't want to be
found via git history" with "Why wasn't I cc'd on a patch? Oh, because I
touched it more than a year ago? I don't like that!" in a way other
than:
- Encouraging people to add themselves to / expand existing MAINTAINERS
  entries.
- Add themselves to the ignore list if they don't want to be found for
  other contributions they made.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 0/4] Adding support to load secure firmware for HS devices

[Tom Rini]

On Tue, 21 May 2024 16:26:44 +0530, Udit Kumar wrote:

> Some use case needs rproc firmware to be loaded at u-boot stage,
> using following commands at u-boot shell, firmware could be loaded
> 
> => setenv dorprocboot 1
> => run boot_rprocs
> 
> For Secure devices, secure version of rproc firmware should be loaded,
> which is appended by sec keyword[0].
> but currently non-secure firmware is loaded even for secure devices.
> So adding support for loading secure firmware on Secured devices.
> 
> [...]

Applied to u-boot/next, thanks!

-- 
Tom