RE: [PATCH 30/33] arm: aspeed: Remove and add needed includes

2024-05-01 Thread ChiaWei Wang
> From: Tom Rini 
> Sent: Tuesday, April 30, 2024 9:36 PM
> 
> Remove  from all mach-aspeed files and when needed add
> missing include files directly.
> 
> Signed-off-by: Tom Rini 

Reviewed-by: Chia-Wei Wang 

Thanks,
Chiawei


RE: [PATCH] Add support for more XMC series

2023-08-13 Thread ChiaWei Wang
> From: SSunk 
> Sent: Saturday, August 12, 2023 11:08 AM
> 
> Add XMC
> XM25QH128C/XM25QH256C/XM25QU256C/XM25QH512C/XM25QU512C
> site: https://www.xmcwh.com/site/product
> 
> Signed-off-by: Kankan Sun 

Reviewed-by: Chia-Wei Wang 


RE: [PATCH] configs: evb-ast2600: Enable configs to store env in SPI

2023-02-16 Thread ChiaWei Wang
Reviewed-by: Chia-Wei Wang 

Thanks.

> From: Ryan Chen 
> Sent: Friday, February 10, 2023 3:42 PM
> 
> Enable defconfigs relevant for storing env on SPI flash.
> 
> Signed-off-by: Ryan Chen 
> ---
>  configs/evb-ast2600_defconfig | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
> index 3440062156..7c09e846ac 100644
> --- a/configs/evb-ast2600_defconfig
> +++ b/configs/evb-ast2600_defconfig
> @@ -13,6 +13,8 @@ CONFIG_SPL_LIBGENERIC_SUPPORT=y
>  CONFIG_NR_DRAM_BANKS=1
>  CONFIG_SPL_LDSCRIPT="arch/arm/mach-aspeed/ast2600/u-boot-spl.lds"
>  CONFIG_ENV_SIZE=0x1
> +CONFIG_ENV_OFFSET=0xe
> +CONFIG_ENV_SECT_SIZE=0x1
>  CONFIG_DM_GPIO=y
>  CONFIG_DEFAULT_DEVICE_TREE="ast2600-evb"
>  CONFIG_SPL_SERIAL=y
> @@ -74,6 +76,8 @@ CONFIG_EFI_PARTITION=y  #
> CONFIG_SPL_EFI_PARTITION is not set  CONFIG_SPL_OF_CONTROL=y
> CONFIG_ENV_OVERWRITE=y
> +CONFIG_ENV_IS_IN_SPI_FLASH=y
> +CONFIG_ENV_SECT_SIZE_AUTO=y
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>  CONFIG_NET_RANDOM_ETHADDR=y
>  CONFIG_SPL_DM=y
> --
> 2.34.1



RE: [PATCH 100/171] Correct SPL uses of HW_WATCHDOG

2023-01-30 Thread ChiaWei Wang
Reviewed-by: Chia-Wei Wang 

Thanks.

> From: Simon Glass 
> Sent: Monday, January 30, 2023 10:42 PM
> 
> This converts 2 usages of this option to the non-SPL form, since there is no
> SPL_HW_WATCHDOG defined in Kconfig
> 
> Signed-off-by: Simon Glass 
> ---
> 
>  drivers/crypto/aspeed/aspeed_hace.c | 2 +-
>  drivers/crypto/hash/hash_sw.c   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/aspeed/aspeed_hace.c
> b/drivers/crypto/aspeed/aspeed_hace.c
> index a1b0b9f564b..6b6c8fa6588 100644
> --- a/drivers/crypto/aspeed/aspeed_hace.c
> +++ b/drivers/crypto/aspeed/aspeed_hace.c
> @@ -288,7 +288,7 @@ static int aspeed_hace_digest_wd(struct udevice *dev,
> enum HASH_ALGO algo,
>   if (rc)
>   return rc;
> 
> - if (CONFIG_IS_ENABLED(HW_WATCHDOG) ||
> CONFIG_IS_ENABLED(WATCHDOG)) {
> + if (IS_ENABLED(CONFIG_HW_WATCHDOG) ||
> CONFIG_IS_ENABLED(WATCHDOG)) {
>   cur = ibuf;
>   end = ibuf + ilen;
> 
> diff --git a/drivers/crypto/hash/hash_sw.c b/drivers/crypto/hash/hash_sw.c
> index 553c068010c..d8065d68ea4 100644
> --- a/drivers/crypto/hash/hash_sw.c
> +++ b/drivers/crypto/hash/hash_sw.c
> @@ -244,7 +244,7 @@ static int sw_hash_digest_wd(struct udevice *dev,
> enum HASH_ALGO algo,
>   if (rc)
>   return rc;
> 
> - if (CONFIG_IS_ENABLED(HW_WATCHDOG) ||
> CONFIG_IS_ENABLED(WATCHDOG)) {
> + if (IS_ENABLED(CONFIG_HW_WATCHDOG) ||
> CONFIG_IS_ENABLED(WATCHDOG)) {
>   cur = ibuf;
>   end = ibuf + ilen;
> 
> --
> 2.39.1.456.gfc5497dd1b-goog



RE: [PATCH] gpio: aspeed: port Linux dt-bindings header file

2022-08-02 Thread ChiaWei Wang
Acked-by: Chia-Wei Wang 

> From: Dhananjay Phadke 
> Sent: Wednesday, August 3, 2022 4:55 AM
> 
> Ported as is, makes it easier to add readable GPIO definitions in DTS files.
> 
> Signed-off-by: Dhananjay Phadke 
> ---
>  include/dt-bindings/gpio/aspeed-gpio.h | 49 ++
>  1 file changed, 49 insertions(+)
>  create mode 100644 include/dt-bindings/gpio/aspeed-gpio.h
> 
> diff --git a/include/dt-bindings/gpio/aspeed-gpio.h
> b/include/dt-bindings/gpio/aspeed-gpio.h
> new file mode 100644
> index 00..56fc4889b2
> --- /dev/null
> +++ b/include/dt-bindings/gpio/aspeed-gpio.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * This header provides constants for binding aspeed,*-gpio.
> + *
> + * The first cell in Aspeed's GPIO specifier is the GPIO ID. The macros
> +below
> + * provide names for this.
> + *
> + * The second cell contains standard flag values specified in gpio.h.
> + */
> +
> +#ifndef _DT_BINDINGS_GPIO_ASPEED_GPIO_H #define
> +_DT_BINDINGS_GPIO_ASPEED_GPIO_H
> +
> +#include 
> +
> +#define ASPEED_GPIO_PORT_A 0
> +#define ASPEED_GPIO_PORT_B 1
> +#define ASPEED_GPIO_PORT_C 2
> +#define ASPEED_GPIO_PORT_D 3
> +#define ASPEED_GPIO_PORT_E 4
> +#define ASPEED_GPIO_PORT_F 5
> +#define ASPEED_GPIO_PORT_G 6
> +#define ASPEED_GPIO_PORT_H 7
> +#define ASPEED_GPIO_PORT_I 8
> +#define ASPEED_GPIO_PORT_J 9
> +#define ASPEED_GPIO_PORT_K 10
> +#define ASPEED_GPIO_PORT_L 11
> +#define ASPEED_GPIO_PORT_M 12
> +#define ASPEED_GPIO_PORT_N 13
> +#define ASPEED_GPIO_PORT_O 14
> +#define ASPEED_GPIO_PORT_P 15
> +#define ASPEED_GPIO_PORT_Q 16
> +#define ASPEED_GPIO_PORT_R 17
> +#define ASPEED_GPIO_PORT_S 18
> +#define ASPEED_GPIO_PORT_T 19
> +#define ASPEED_GPIO_PORT_U 20
> +#define ASPEED_GPIO_PORT_V 21
> +#define ASPEED_GPIO_PORT_W 22
> +#define ASPEED_GPIO_PORT_X 23
> +#define ASPEED_GPIO_PORT_Y 24
> +#define ASPEED_GPIO_PORT_Z 25
> +#define ASPEED_GPIO_PORT_AA 26
> +#define ASPEED_GPIO_PORT_AB 27
> +#define ASPEED_GPIO_PORT_AC 28
> +
> +#define ASPEED_GPIO(port, offset) \
> + ((ASPEED_GPIO_PORT_##port * 8) + offset)
> +
> +#endif
> --
> 2.25.1



RE: [PATCH] configs: ast2600: Move SPL bss section to DRAM space

2022-06-28 Thread ChiaWei Wang
Hi Sean,

> From: Sean Anderson 
> Sent: Tuesday, June 28, 2022 12:57 PM
> 
> Hi Chai,
> 
> On 6/28/22 12:23 AM, Joel Stanley wrote:
> > Hi Chai Wei,
> >
> > On Wed, 1 Jun 2022 at 08:21, Chia-Wei Wang
>  wrote:
> >>
> >> The commit b583348ca8c8 ("image: fit: Align hash output buffers")
> >> places the hash output buffer at the .bss section. However, AST2600
> >> by default executes SPL in the NOR flash XIP way. This results in the
> >> hash output cannot be written to the buffer as it is located at the R/X 
> >> only
> region.
> >>
> >> We need to move the .bss section out of the SPL body to the DRAM
> >> space, where hash output can be written to. This patch includes:
> >>   - Define the .bss section base and size
> >>   - A new SPL linker script is added with a separate .bss region specified
> >>   - Enable CONFIG_SPL_SEPARATE_BSS kconfig option
> >>
> >> Signed-off-by: Chia-Wei Wang 
> >
> > This patch breaks booting for me.
> 
> Does the patch Joel posted [1] fix your issue? It seems like I used the wrong
> macro in the first place, so hopefully this patch shouldn't be necessary.
> 
> --Sean
> 
> [1] https://lore.kernel.org/u-boot/20220620070117.3443066-1-j...@jms.id.au/

Yes. Joel's patch also solved that issue.

Relocating .bss to DRAM space can avoid similar issues.
But it do create additional maintenance work.

Chiawei



RE: [PATCH] configs: ast2600: Move SPL bss section to DRAM space

2022-06-28 Thread ChiaWei Wang
Hi Joel,

> From: Joel Stanley 
> Sent: Tuesday, June 28, 2022 12:24 PM
> 
> Hi Chai Wei,
> 
> On Wed, 1 Jun 2022 at 08:21, Chia-Wei Wang
>  wrote:
> >
> > The commit b583348ca8c8 ("image: fit: Align hash output buffers")
> > places the hash output buffer at the .bss section. However, AST2600 by
> > default executes SPL in the NOR flash XIP way. This results in the
> > hash output cannot be written to the buffer as it is located at the R/X only
> region.
> >
> > We need to move the .bss section out of the SPL body to the DRAM
> > space, where hash output can be written to. This patch includes:
> >  - Define the .bss section base and size
> >  - A new SPL linker script is added with a separate .bss region
> > specified
> >  - Enable CONFIG_SPL_SEPARATE_BSS kconfig option
> >
> > Signed-off-by: Chia-Wei Wang 
> 
> This patch breaks booting for me.
> 
> My concern with the approach is it creates extra maintenance work.
> When changes are made to the main linker script they need to be mirrored
> here, or else the aspeed port will miss out. (Having the machine tested in CI
> will help this somewhat, but only for the code paths we can test under
> emulation).

The patch was trying to solve the hash buffer allocation change to common code 
and to avoid similar issues.
But I agree there is additional maintenance work on the customized linker 
script.

> 
> I know the patch has been merged, but I have a few questions:
> 
> I imagine the ast2600 is not the only board that runs XIP. How do other boards
> solve the problem?
> 
> What happens when a symbol that is used before DRAM training has
> completed is placed in bss?

Honestly, I am not sure how other platforms/boards dealing with this.
But like U-Boot REAME stated, the initial global data is read-only.
I guess we have to be careful about the global variable use before DRAM 
initialization or even variable relocation.

> 
> How do you plan to support systems that don't have NOR?

We would like to have another defconfig for eMMC booting.
If the NOR-based linker script is not applicable, the default one can be used.

Chiawei


RE: [PATCH RESEND 5/5] CI: Add Aspeed AST2600

2022-06-27 Thread ChiaWei Wang
Reviewed-by: Chia-Wei Wang 

> From: joel.s...@gmail.com  On Behalf Of Joel Stanley
> Sent: Monday, June 27, 2022 3:58 PM
> 
> The AST2600 has a Qemu model that allows testing. Create a SPI NOR image
> containing the combined SPL and u-boot FIT image.
> 
> Signed-off-by: Joel Stanley 
> ---
>  .azure-pipelines.yml | 3 +++
>  .gitlab-ci.yml   | 6 ++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index
> ad540ea63536..bdc515ebcdc1 100644
> --- a/.azure-pipelines.yml
> +++ b/.azure-pipelines.yml
> @@ -261,6 +261,9 @@ stages:
>  evb_ast2500:
>TEST_PY_BD: "evb-ast2500"
>TEST_PY_ID: "--id qemu"
> +evb_ast2600:
> +  TEST_PY_BD: "evb-ast2600"
> +  TEST_PY_ID: "--id qemu"
>  vexpress_ca9x4:
>TEST_PY_BD: "vexpress_ca9x4"
>TEST_PY_ID: "--id qemu"
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c6a608f7e2a7..f9cd41750791
> 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -272,6 +272,12 @@ evb-ast2500 test.py:
>  TEST_PY_ID: "--id qemu"
><<: *buildman_and_testpy_dfn
> 
> +evb-ast2600 test.py:
> +  variables:
> +TEST_PY_BD: "evb-ast2600"
> +TEST_PY_ID: "--id qemu"
> +  <<: *buildman_and_testpy_dfn
> +
>  sandbox_flattree test.py:
>variables:
>  TEST_PY_BD: "sandbox_flattree"
> --
> 2.35.1



RE: [PATCH RESEND 4/5] ast2600: Configure u-boot-with-spl.bin target

2022-06-27 Thread ChiaWei Wang
> From: joel.s...@gmail.com  On Behalf Of Joel Stanley
> Sent: Monday, June 27, 2022 3:58 PM
> 
> For the u-boot-with-spl.bin target to be useful for the AST2600, set the
> maximum SPL size which also sets the padding length.
> 
> The normal way of loading u-boot is as a FIT, so configure u-boot.img as the
> SPL playload.
> 
> With this the following simple steps can be used to build and boot a
> system:
> 
>   make u-boot-with-spl.bin
>   truncate -s 64M u-boot-with-spl.bin
>   qemu-system-arm -nographic -M ast2600-evb \
> -drive file=u-boot-with-spl.bin,if=mtd,format=raw
> 
> Signed-off-by: Joel Stanley 
> ---
>  include/configs/evb_ast2600.h | 3 +++
>  configs/evb-ast2600_defconfig | 2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/include/configs/evb_ast2600.h b/include/configs/evb_ast2600.h
> index 3c2155da46df..f5ac88447b52 100644
> --- a/include/configs/evb_ast2600.h
> +++ b/include/configs/evb_ast2600.h
> @@ -10,6 +10,9 @@
> 
>  #define CONFIG_SYS_UBOOT_BASECONFIG_SYS_TEXT_BASE
> 
> +/* The maximum size the AST2600 bootrom can load is 64KB */
> +#define CONFIG_SPL_MAX_SIZE  65536
> +

Please have this to be synced with CONFIG_SPL_SIZE_LIMIT. Thanks.

Reviewed-by: Chia-Wei Wang 


RE: [PATCH RESEND 1/5] config/ast2600: Enable CRC32

2022-06-27 Thread ChiaWei Wang
Reviewed-by: Chia-Wei Wang 

> From: joel.s...@gmail.com  On Behalf Of Joel Stanley
> Sent: Monday, June 27, 2022 3:58 PM
> 
> Useful for testing images with the default hash type.
> 
> Signed-off-by: Joel Stanley 
> ---
>  configs/evb-ast2600_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
> index f84b723bbba3..53ba36a28374 100644
> --- a/configs/evb-ast2600_defconfig
> +++ b/configs/evb-ast2600_defconfig
> @@ -35,6 +35,7 @@ CONFIG_SPL_SIZE_LIMIT_SUBTRACT_MALLOC=y
>  CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>  CONFIG_SPL_STACK_R=y
>  CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x200
> +CONFIG_SPL_CRC32=y
>  CONFIG_SPL_FIT_IMAGE_TINY=y
>  CONFIG_SPL_DM_RESET=y
>  CONFIG_SPL_RAM_SUPPORT=y
> --
> 2.35.1



RE: [PATCH RESEND 3/5] config/ast2600: Disable hash hardware accel

2022-06-27 Thread ChiaWei Wang
Reviewed-by: Chia-Wei Wang 

The QEMU emulation issue is under investigation by Steven.
The CRC32 and MD5 SW support will be added before we re-enabling HW crypto 
drivers.

Chiawei

> From: joel.s...@gmail.com  On Behalf Of Joel Stanley
> Sent: Monday, June 27, 2022 3:58 PM
> 
> The HACE driver lacks support for all the hash types, causing boot to fail 
> with
> the default FIT configuration which uses CRC32.
> 
> Additionally the Qemu model or the u-boot driver is unable to correctly
> compute the SHA256 hash used in a FIT.
> 
> Disable HACE by default while the above issues are worked out to enable boot
> testing in Qemu.
> 
> Signed-off-by: Joel Stanley 
> ---
>  configs/evb-ast2600_defconfig | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
> index f3a6cb222020..160bccff48e2 100644
> --- a/configs/evb-ast2600_defconfig
> +++ b/configs/evb-ast2600_defconfig
> @@ -59,9 +59,6 @@ CONFIG_REGMAP=y
>  CONFIG_SPL_OF_TRANSLATE=y
>  CONFIG_CLK=y
>  CONFIG_SPL_CLK=y
> -CONFIG_DM_HASH=y
> -CONFIG_HASH_ASPEED=y
> -CONFIG_ASPEED_ACRY=y
>  CONFIG_ASPEED_GPIO=y
>  CONFIG_DM_I2C=y
>  CONFIG_MISC=y
> --
> 2.35.1



RE: [PATCH RESEND 2/5] config/ast2600: Make position independent

2022-06-27 Thread ChiaWei Wang
Reviewed-by: Chia-Wei Wang 

> From: joel.s...@gmail.com  On Behalf Of Joel Stanley
> Sent: Monday, June 27, 2022 3:58 PM
> 
> Allows loading one u-boot from another. Useful for testing on hardware.
> 
> Signed-off-by: Joel Stanley 
> ---
>  configs/evb-ast2600_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
> index 53ba36a28374..f3a6cb222020 100644
> --- a/configs/evb-ast2600_defconfig
> +++ b/configs/evb-ast2600_defconfig
> @@ -1,5 +1,6 @@
>  CONFIG_ARM=y
>  CONFIG_SYS_DCACHE_OFF=y
> +CONFIG_POSITION_INDEPENDENT=y
>  CONFIG_SPL_SYS_THUMB_BUILD=y
>  CONFIG_ARCH_ASPEED=y
>  CONFIG_SYS_TEXT_BASE=0x8000
> --
> 2.35.1



RE: [PATCH 4/5] ast2600: Configure u-boot-with-spl.bin target

2022-06-26 Thread ChiaWei Wang
Reply again to leave record on mailing list.

> From: joel.s...@gmail.com  On Behalf Of Joel Stanley
> Sent: Friday, June 24, 2022 10:50 AM
> 
> For the u-boot-with-spl.bin target to be useful for the AST2600, set the
> maximum SPL size which also sets the padding length.
> 
> The normal way of loading u-boot is as a FIT, so configure u-boot.img as the
> SPL playload.
> 
> With this the following simple steps can be used to build and boot a
> system:
> 
>   make u-boot-with-spl.bin
>   truncate -s 64M u-boot-with-spl.bin
>   qemu-system-arm -nographic -M ast2600-evb \
> -drive file=u-boot-with-spl.bin,if=mtd,format=raw
> 
> Signed-off-by: Joel Stanley 
> ---
>  include/configs/evb_ast2600.h | 3 +++
>  configs/evb-ast2600_defconfig | 2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/include/configs/evb_ast2600.h b/include/configs/evb_ast2600.h
> index 3c2155da46df..f5ac88447b52 100644
> --- a/include/configs/evb_ast2600.h
> +++ b/include/configs/evb_ast2600.h
> @@ -10,6 +10,9 @@
> 
>  #define CONFIG_SYS_UBOOT_BASECONFIG_SYS_TEXT_BASE
> 
> +/* The maximum size the AST2600 bootrom can load is 64KB */
> +#define CONFIG_SPL_MAX_SIZE  65536

Please define this to the Kconfig option CONFIG_SPL_SIZE_LIMIT to avoid 
inconsistent definitions on SPL image size limitation.

Chiawei


RE: [PATCH 3/5] config/ast2600: Disable hash hardware accel

2022-06-26 Thread ChiaWei Wang
Reply again to leave record on mailing list.

> From: joel.s...@gmail.com  On Behalf Of Joel Stanley
> Sent: Friday, June 24, 2022 10:50 AM
> 
> The Qemu model or the u-boot driver is unable to correctly compute the
> SHA256 hash used in a FIT. Disable it by default while that issue is worked 
> out
> to enable boot testing in Qemu.
> 
> Signed-off-by: Joel Stanley 
> ---
>  configs/evb-ast2600_defconfig | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
> index f3a6cb222020..160bccff48e2 100644
> --- a/configs/evb-ast2600_defconfig
> +++ b/configs/evb-ast2600_defconfig
> @@ -59,9 +59,6 @@ CONFIG_REGMAP=y
>  CONFIG_SPL_OF_TRANSLATE=y
>  CONFIG_CLK=y
>  CONFIG_SPL_CLK=y
> -CONFIG_DM_HASH=y
> -CONFIG_HASH_ASPEED=y
> -CONFIG_ASPEED_ACRY=y

Per our previous discussion, SPL code size still exists if all of AST2600 
features are upstream-ed.
Therefore, HW-assisted crypto drivers are needed.

In addition, the current drivers works fine on real EVB to verify Hash + RSA 
signature (including the SHA256 in question).
This issue described in commit message should be attributed to incomplete QEMU 
emulation.

Therefore, fixing QEMU should be the right way to go instead of disabling these 
options for real HW.

Chiawei

>  CONFIG_ASPEED_GPIO=y
>  CONFIG_DM_I2C=y
>  CONFIG_MISC=y
> --
> 2.35.1



RE: [PATCH] image: fit: Use stack allocation macro

2022-06-20 Thread ChiaWei Wang
Tested-by: Chia-Wei Wang 

Thanks for the fix.

Driven by the same issue, We also sent another patch moving .BSS section into 
DRAM.
You may also check it out and any feedback is appreciated.
https://patchwork.ozlabs.org/project/uboot/patch/20220601082115.10799-1-chiawei_w...@aspeedtech.com/

Chiawei

> From: joel.s...@gmail.com  On Behalf Of Joel Stanley
> Sent: Monday, June 20, 2022 3:01 PM
> 
> The documentation above the DEFINE_ALIGN_BUFFER says it's for use outside
> functions, but we're inside one.
> 
> Instead use ALLOC_CACHE_ALIGN_BUFFER, the stack based macro, which also
> includes the cache alignment.
> 
> Fixes: b583348ca8c8 ("image: fit: Align hash output buffers")
> Signed-off-by: Joel Stanley 
> ---
> This fixes booting the ast2600-evb image in qemu, which was getting all zeroes
> for the output of the FIT hash check.
> 
> The 'static' buffer was in BSS but the output image didn't contain a BSS 
> section.
> The pointer was left pointing to the text, so the code was trying to write to 
> the
> (read only?) text area in SPI NOR memory space.
> 
>  tools/mkimage.h  | 3 +--
>  boot/image-fit.c | 3 +--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/mkimage.h b/tools/mkimage.h index
> 7652c8b001c3..f5ca65e2edfd 100644
> --- a/tools/mkimage.h
> +++ b/tools/mkimage.h
> @@ -41,8 +41,7 @@ static inline ulong map_to_sysmem(void *ptr)
>   return (ulong)(uintptr_t)ptr;
>  }
> 
> -#define ARCH_DMA_MINALIGN 1
> -#define DEFINE_ALIGN_BUFFER(type, name, size, alugn) type name[size]
> +#define ALLOC_CACHE_ALIGN_BUFFER(type, name, size) type name[size]
> 
>  #define MKIMAGE_TMPFILE_SUFFIX   ".tmp"
>  #define MKIMAGE_MAX_TMPFILE_LEN  256
> diff --git a/boot/image-fit.c b/boot/image-fit.c index
> f57d97f55229..df3e5df8836a 100644
> --- a/boot/image-fit.c
> +++ b/boot/image-fit.c
> @@ -1264,8 +1264,7 @@ int calculate_hash(const void *data, int data_len,
> const char *name,  static int fit_image_check_hash(const void *fit, int 
> noffset,
> const void *data,
>   size_t size, char **err_msgp)
>  {
> - DEFINE_ALIGN_BUFFER(uint8_t, value, FIT_MAX_HASH_LEN,
> - ARCH_DMA_MINALIGN);
> + ALLOC_CACHE_ALIGN_BUFFER(uint8_t, value, FIT_MAX_HASH_LEN);
>   int value_len;
>   const char *algo;
>   uint8_t *fit_value;
> --
> 2.35.1



RE: [PATCH] MAINTAINERS: aspeed: Add more files and myself as a reviewer

2022-05-18 Thread ChiaWei Wang
Reviewed-by: Chia-Wei Wang 

> -Original Message-
> From: joel.s...@gmail.com  On Behalf Of Joel Stanley
> Sent: Thursday, May 19, 2022 8:37 AM
> To: u-boot@lists.denx.de; Ryan Chen ; ChiaWei
> Wang 
> Subject: [PATCH] MAINTAINERS: aspeed: Add more files and myself as a
> reviewer
> 
> Add the rest of the ASPEED drivers that are in tree. Most are obvious, except
> for ftgmac100 which matches the register layout used in the ASPEED SoC.
> 
> I am the Linux maintainer for the ASPEED kernel port, and help maintain the
> fork of u-boot used for OpenBMC, so add myself as a reviewer so I can stay
> informed about u-boot changes.
> 
> Signed-off-by: Joel Stanley 
> ---
>  MAINTAINERS | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 56be0bfad00c..28e4d3823861 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -169,12 +169,23 @@ ARM ASPEED
>  M:   Ryan Chen 
>  M:   Chia-Wei Wang 
>  R:   Aspeed BMC SW team 
> +R:   Joel Stanley 
>  S:   Maintained
>  F:   arch/arm/mach-aspeed/
>  F:   arch/arm/include/asm/arch-aspeed/
>  F:   board/aspeed/
>  F:   drivers/clk/aspeed/
> +F:   drivers/crypto/aspeed/
> +F:   drivers/gpio/gpio-aspeed.c
> +F:   drivers/i2c/ast_i2c.[ch]
> +F:   drivers/mmc/aspeed_sdhci.c
> +F:   drivers/net/aspeed_mdio.c
> +F:   drivers/net/ftgmac100.[ch]
>  F:   drivers/pinctrl/aspeed/
> +F:   drivers/pwm/pwm-aspeed.c
> +F:   drivers/ram/aspeed/
> +F:   drivers/reset/reset-ast2500.c
> +F:   drivers/watchdog/ast_wdt.c
>  N:   aspeed
> 
>  ARM BROADCOM BCM283X / BCM27XX
> --
> 2.35.1



RE: [PATCH] pwm: aspeed: Select SYSCON to get parent detail.

2022-05-02 Thread ChiaWei Wang
> From: Billy Tsai 
> Sent: Friday, April 29, 2022 11:51 AM
> 
> To work correctly, this driver depends on SYSCON to get the base address from
> the parent dts node.
> 
> Signed-off-by: Billy Tsai 
> ---
>  drivers/pwm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index
> cb54e67fae..cf66293eeb 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -12,6 +12,7 @@ config DM_PWM
>  config PWM_ASPEED
>   bool "Enable support for the Aspeed PWM"
>   depends on DM_PWM
> + select SYSCON
>   help
> This PWM is found on Ast2600 SoCs. It supports a programmable period
> and duty cycle. It provides 16 channels which can be independently
> --
> 2.25.1

Reviewed-by: Chia-Wei Wang 


RE: [PATCH 3/3] ARM: dts: ast2600: Add PWM to device tree

2022-03-13 Thread ChiaWei Wang
> From: Simon Glass 
> Sent: Saturday, March 12, 2022 10:44 AM
> 
> On Mon, 7 Mar 2022 at 20:02, Billy Tsai  wrote:
> >
> > Add the PWM node and enable it for AST2600 EVB
> >
> > Signed-off-by: Billy Tsai 
> > ---
> >  arch/arm/dts/ast2600-evb.dts | 20 
> >  arch/arm/dts/ast2600.dtsi| 15 +++
> >  2 files changed, 35 insertions(+)
> >
> 
> Reviewed-by: Simon Glass 

Reviewed-by: Chia-Wei Wang 


RE: [PATCH 2/3] pinctrl: Add the pinctrl setting for PWM.

2022-03-13 Thread ChiaWei Wang
> From: Simon Glass 
> Sent: Saturday, March 12, 2022 10:44 AM
> 
> On Mon, 7 Mar 2022 at 20:02, Billy Tsai  wrote:
> >
> > This patchs add the signal description array for PWM pinctrl settings.
> >
> > Signed-off-by: Billy Tsai 
> > ---
> >  arch/arm/dts/ast2600.dtsi|  80 +++
> >  drivers/pinctrl/aspeed/pinctrl_ast2600.c | 120 +++
> >  2 files changed, 200 insertions(+)
> >
> 
> Reviewed-by: Simon Glass 

Reviewed-by: Chia-Wei Wang 


RE: [PATCH 1/3] pwm: Add Aspeed ast2600 PWM support

2022-03-13 Thread ChiaWei Wang
> From: Simon Glass 
> Sent: Saturday, March 12, 2022 10:44 AM
> 
> On Mon, 7 Mar 2022 at 20:02, Billy Tsai  wrote:
> >
> > This patch add the support of PWM controller which can be found at
> > aspeed
> > ast2600 soc. The pwm supoorts up to 16 channels and it's part function
> > of multi-function device "pwm-tach controller".
> >
> > Signed-off-by: Billy Tsai 
> > ---
> >  drivers/pwm/Kconfig  |   8 ++
> >  drivers/pwm/Makefile |   1 +
> >  drivers/pwm/pwm-aspeed.c | 251
> > +++
> >  3 files changed, 260 insertions(+)
> >  create mode 100644 drivers/pwm/pwm-aspeed.c
> 
> Reviewed-by: Simon Glass 

Reviewed-by: Chia-Wei Wang 


RE: [PATCH] crypto: aspeed: fix polling RSA status wrong issue

2022-02-15 Thread ChiaWei Wang
> From: Neal Liu 
> Sent: Tuesday, February 15, 2022 4:55 PM
> 
> Check interrupt status to see if RSA engine is completed. After completion of
> the task, write-clear the status to finish operation.
> Add missing register base for completion.
> 
> Signed-off-by: Neal Liu 

Please add a line with "Fixes:" tag in the commit message to indicate this is a 
fix.

Reviewed-by: Chia-Wei Wang 

Thanks,
Chiawei


RE: [PATCH] image: Control FIT signature verification at runtime

2022-02-06 Thread ChiaWei Wang
Hi Andrew,

I am curious about the usage scenario.
Is the runtime control required for production release?
As this control acts like a backdoor to bypass the chain-of-trust.
If it is for debugging/development purposes, should we encourage the use of 
unsigned images under RD environments?
Beyond this, I have no concern as the patch provides more flexibility.

> From: Andrew Jeffery 
> Sent: Monday, January 31, 2022 11:42 AM
> 
> Some platform designs include support for disabling secure-boot via a jumper
> on the board. Sometimes this control can be separate from the mechanism
> enabling the root-of-trust for the platform. Add support for this latter 
> scenario
> by allowing boards to implement board_fit_image_require_verfied(), which is
> then invoked in the usual FIT verification paths.
> 
> Signed-off-by: Andrew Jeffery 
> ---
> Hi,
> 
> This patch is extracted from and motivated by a series adding run-time control
> of FIT signature verification to u-boot in OpenBMC:
> 
> https://lore.kernel.org/openbmc/20220131012538.73021-1-and...@aj.id.au/
> 
> Unfortunately the OpenBMC u-boot tree is quite a way behind on tracking
> upstream and contains a bunch of out-of-tree work as well. As such I'm looking
> to upstream the couple of changes that make sense against master.
> 
> Please take a look!
> 
> Andrew
> 
>  boot/Kconfig |  8 
>  boot/image-fit.c | 21 +  include/image.h  |  9
> +
>  3 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/boot/Kconfig b/boot/Kconfig index c8d5906cd304..ec413151fd5a
> 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -78,6 +78,14 @@ config FIT_SIGNATURE
> format support in this case, enable it using
> CONFIG_LEGACY_IMAGE_FORMAT.
> 
> +if FIT_SIGNATURE
> +config FIT_RUNTIME_SIGNATURE
> + bool "Control verification of FIT uImages at runtime"
> + help
> +   This option allows board support to disable verification of
> +   signatures at runtime, for example through the state of a GPIO.
> +endif # FIT_SIGNATURE
> +

Using "depends on" might be preferred for Kconfig dependency.

Regards,
Chiawei


RE: [PATCH next v7 00/12] aspeed: Support secure boot chain with FIT image verification

2021-10-24 Thread ChiaWei Wang
Thank you all for the review comments and tags.
I will prepare the v8 patch with tag included and comments addressed.

In addition, as DM_HASH has been merged into the master branch.
The v8 patch will be rebased on the master branch.

Thanks,
Chiawei

> From: U-Boot  On Behalf Of Chia-Wei Wang
> Sent: Wednesday, October 20, 2021 10:49 AM
> 
> This patch series intends to provide a secure boot chain from SPL to Linux
> kernel based on the hash and signature verification of FIT image paradigm.
> 
> To improve the performance and save code size (SPL is limited to 64KB due to
> HW-RoT), the drviers of two HW crypto engine HACE and ACRY are also added
> for AST26xx SoCs.
> 
> As HACE and ACRY can only access to DRAM space, additional configuration
> and boot command are also updated according to move each FIT image before
> its booting.
> 
> In addition, the common code of FIT image hash algorithm lookup is also
> revised to leverage the HW accelerated calculation.
> 
> v7:
>  - fix missing interrupt status clear for ACRY RSA operation
> 
> v6:
>  - fix parameter comment for v5 update
> 
> v5:
>  - fix inconsistent parameter name due to parallel patch work
> 
> v4:
>  - add new DM_HASH based driver for Aspeed HACE
>  - remove SPL board init, which was originally used to probe non-DM HACE
> driver
>  - fix typo of ARCY to ACRY
>  - refactor defconfig based on the new Kconfig of U-Boot next branch
> 
> v3:
>  - add SW work around for HACE HW DMA issue by resetting HACE
>  - add reset control for HACE device tree node
>  - sync all of the HACE error message to use debug()
> 
> v2:
>  - update commit authors
> 
> Chia-Wei Wang (9):
>   image: fit: Fix parameter name for hash algorithm
>   aspeed: ast2600: Enlarge SRAM size
>   clk: ast2600: Add RSACLK control for ACRY
>   crypto: aspeed: Add AST2600 ACRY support
>   ARM: dts: ast2600: Add ACRY to device tree
>   ast2600: spl: Locate load buffer in DRAM space
>   configs: ast2600-evb: Enable SPL FIT support
>   configs: aspeed: Make EXTRA_ENV_SETTINGS board specific
>   configs: ast2600: Boot kernel FIT in DRAM
> 
> Joel Stanley (2):
>   clk: ast2600: Add YCLK control for HACE
>   ARM: dts: ast2600: Add HACE to device tree
> 
> Johnny Huang (1):
>   crypto: aspeed: Add AST2600 HACE support
> 
>  arch/arm/dts/ast2600-evb.dts  |  10 +
>  arch/arm/dts/ast2600.dtsi |  17 +
>  arch/arm/include/asm/arch-aspeed/platform.h   |   2 +-
>  .../arm/include/asm/arch-aspeed/scu_ast2600.h |   6 +-
>  arch/arm/mach-aspeed/ast2600/spl.c|   9 +-
>  common/image-fit.c|   4 +-
>  configs/evb-ast2600_defconfig |  22 +-
>  drivers/clk/aspeed/clk_ast2600.c  |  38 ++
>  drivers/crypto/Kconfig|   2 +
>  drivers/crypto/Makefile   |   1 +
>  drivers/crypto/aspeed/Kconfig |  20 +
>  drivers/crypto/aspeed/Makefile|   2 +
>  drivers/crypto/aspeed/aspeed_acry.c   | 190 +
>  drivers/crypto/aspeed/aspeed_hace.c   | 381
> ++
>  drivers/crypto/hash/Kconfig   |   8 +
>  include/configs/aspeed-common.h   |   9 -
>  include/configs/evb_ast2500.h |   9 +
>  include/configs/evb_ast2600.h |  16 +
>  lib/rsa/Kconfig   |  10 +-
>  19 files changed, 729 insertions(+), 27 deletions(-)  create mode 100644
> drivers/crypto/aspeed/Kconfig  create mode 100644
> drivers/crypto/aspeed/Makefile  create mode 100644
> drivers/crypto/aspeed/aspeed_acry.c
>  create mode 100644 drivers/crypto/aspeed/aspeed_hace.c
> 
> --
> 2.17.1



RE: [PATCH next v7 04/12] crypto: aspeed: Add AST2600 HACE support

2021-10-24 Thread ChiaWei Wang
> From: U-Boot  On Behalf Of ChiaWei Wang
> Sent: Thursday, October 21, 2021 9:57 AM
> 
> > From: Dhananjay Phadke 
> > Sent: Thursday, October 21, 2021 9:33 AM
> >
> > On Wed, 20 Oct 2021, Chia-Wei Wang wrote:
> >
> > > +static const struct hash_ops aspeed_hace_ops = {
> > > + .hash_init = aspeed_hace_init,
> > > + .hash_update = aspeed_hace_update,
> > > + .hash_finish = aspeed_hace_finish,
> > > + .hash_digest_wd = aspeed_hace_digest_wd,
> > > + .hash_digest = aspeed_hace_digest, };
> > > +
> > > +static const struct udevice_id aspeed_hace_ids[] = {
> > > + { .compatible = "aspeed,ast2600-hace" },
> > > + { }
> > > +};
> > > +
> >
> > Why only ast2600? Is ast2500 engine different or this driver can also
> > support it?
> 
> The HACE of AST2500 has certain HW issues.
> Therefore we don't enable support for it.

Sorry for the misleading information.

It is the driver compatibility issue instead of AST2500 HW issues.
The HACE design has been revised to support scatter-gather mode and more hash 
algorithms in AST2600.
And this driver is implemented based on the new design and does not fully fit 
AST2500.

This patch series currently focus on AST2600 only as secure boot is supported 
since then.

Chiawei


RE: [PATCH next v7 06/12] clk: ast2600: Add RSACLK control for ACRY

2021-10-24 Thread ChiaWei Wang
> From: Joel Stanley 
> Sent: Thursday, October 21, 2021 8:08 AM
> 
> On Wed, 20 Oct 2021 at 02:50, Chia-Wei Wang
>  wrote:
> >
> > Add RSACLK enable for ACRY, the HW RSA/ECC crypto engine of ASPEED
> > AST2600 SoCs.
> >
> > As ACRY and HACE share the same reset control bit, we do not perform
> > the reset-hold-n-release operation during their clock ungating
> > process. Instead, only reset release is conducted to prevent mutual
> > interference.
> 
> Is this okay to do? If so, can we do this for other clocks without having to 
> wait
> 100us?

No really.
But we thought that it should be fine as SPL/U-Boot should be the first one 
using HACE and ACRY.
And both of them, by default, will be reset by WDT if the system goes back to 
bootloader.
Except for this, resetting module upon clock-enabling could be much cleaner for 
other drivers.

Separating the reset control of HACE and ACRY has been taken into the 
consideration of next generation design.

> 
> It would make sense to merge this patch with the HACE patch to avoid adding
> and then removing code in the same series.

Agree.
We should have the reset assertion dropped in the HACE clock commit instead of 
removing it later.

> 
> >
> > Signed-off-by: Chia-Wei Wang 
> > ---
> >  .../arm/include/asm/arch-aspeed/scu_ast2600.h |  1 +
> >  drivers/clk/aspeed/clk_ast2600.c  | 22
> +--
> >  2 files changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch-aspeed/scu_ast2600.h
> > b/arch/arm/include/asm/arch-aspeed/scu_ast2600.h
> > index d7b500f656..7c5aab98b6 100644
> > --- a/arch/arm/include/asm/arch-aspeed/scu_ast2600.h
> > +++ b/arch/arm/include/asm/arch-aspeed/scu_ast2600.h
> > @@ -8,6 +8,7 @@
> >  #define SCU_UNLOCK_KEY 0x1688a8a8
> >
> >  #define SCU_CLKGATE1_EMMC  BIT(27)
> > +#define SCU_CLKGATE1_ACRY  BIT(24)
> >  #define SCU_CLKGATE1_MAC2  BIT(21)
> >  #define SCU_CLKGATE1_MAC1  BIT(20)
> >  #define SCU_CLKGATE1_USB_HUB   BIT(14)
> > diff --git a/drivers/clk/aspeed/clk_ast2600.c
> > b/drivers/clk/aspeed/clk_ast2600.c
> > index 69128fd3c4..f6ebf824aa 100644
> > --- a/drivers/clk/aspeed/clk_ast2600.c
> > +++ b/drivers/clk/aspeed/clk_ast2600.c
> > @@ -1018,11 +1018,26 @@ static ulong ast2600_enable_haceclk(struct
> ast2600_scu *scu)
> > uint32_t reset_bit;
> > uint32_t clkgate_bit;
> >
> > +   /* share the same reset control bit with ACRY */
> > reset_bit = BIT(ASPEED_RESET_HACE);
> > clkgate_bit = SCU_CLKGATE1_HACE;
> >
> > -   writel(reset_bit, >modrst_ctrl1);
> > -   udelay(100);
> > +   writel(clkgate_bit, >clkgate_clr1);
> > +   mdelay(20);
> > +   writel(reset_bit, >modrst_clr1);
> > +
> > +   return 0;
> > +}
> > +
> > +static ulong ast2600_enable_rsaclk(struct ast2600_scu *scu) {
> > +   uint32_t reset_bit;
> > +   uint32_t clkgate_bit;
> > +
> > +   /* share the same reset control bit with HACE */
> > +   reset_bit = BIT(ASPEED_RESET_HACE);
> > +   clkgate_bit = SCU_CLKGATE1_ACRY;
> > +
> > writel(clkgate_bit, >clkgate_clr1);
> > mdelay(20);
> > writel(reset_bit, >modrst_clr1); @@ -1071,6 +1086,9 @@
> > static int ast2600_clk_enable(struct clk *clk)
> > case ASPEED_CLK_GATE_YCLK:
> > ast2600_enable_haceclk(priv->scu);
> > break;
> > +   case ASPEED_CLK_GATE_RSACLK:
> > +   ast2600_enable_rsaclk(priv->scu);
> > +   break;
> > default:
> > pr_err("can't enable clk\n");
> > return -ENOENT;
> > --
> > 2.17.1
> >


RE: [PATCH next v7 01/12] image: fit: Fix parameter name for hash algorithm

2021-10-20 Thread ChiaWei Wang
> From: Joel Stanley 
> Sent: Thursday, October 21, 2021 8:08 AM
> 
> On Wed, 20 Oct 2021 at 02:49, Chia-Wei Wang
>  wrote:
> >
> > Fix inconsistent function parameter name of the hash algorithm.
> >
> > Signed-off-by: Chia-Wei Wang 
> > Fixes: 92055e138f2 ("image: Drop if/elseif hash selection in
> > calculate_hash()")
> 
> Reviewed-by: Joel Stanley 
> 
> This fix should go in independent of your series.

The fix is included to solve the compilation failure of this patch series.

Regards,
Chiawei


RE: [PATCH next v7 04/12] crypto: aspeed: Add AST2600 HACE support

2021-10-20 Thread ChiaWei Wang
> From: Joel Stanley 
> Sent: Thursday, October 21, 2021 8:08 AM
> 
> On Wed, 20 Oct 2021 at 02:50, Chia-Wei Wang
>  wrote:
> >
> > From: Johnny Huang 
> >
> > Hash and Crypto Engine (HACE) is designed to accelerate the throughput
> > of hash data digest, and symmetric-key encryption.
> >
> > Signed-off-by: Johnny Huang 
> > Signed-off-by: Chia-Wei Wang 
> 
> I see you've re-written the driver from the version I submitted. It's got a 
> bit
> more code now, but there's no explanation as to why you needed to add this
> code.
> 
> From what I can guess, you needed to pat the watchdog while waiting for hash
> operations to complete. Why not pat the watchdog in
> aspeed_hace_wait_completion instead of chunking up the operation?

The AST2600 HACE has HW issue using one shot mode.
The patch you previously provided is exactly the workaround for that. (Thanks!)
https://lore.kernel.org/openbmc/20a7316b-7a1b-41b3-b5d4-ef9cea98f...@linux.ibm.com/

To avoid upstreaming a workaround solution, Johnny confirmed that HACE has no 
issue using accumulate mode.
Thus the driver is re-written.

> 
> Secondly, your incremental API is not using the scatter gather mode any more.
> Is there a reason for this? We have been using it successfully in the system 
> for
> some time.

This is merely for the simplicity of driver implementation.
As MMU is not enabled in U-Boot. We thought that SG mode may not be required.

Johnny, please correct me if any misunderstanding.

Regards,
Chiawei

> 
> 
> > ---
> >  drivers/crypto/Kconfig  |   2 +
> >  drivers/crypto/Makefile |   1 +
> >  drivers/crypto/aspeed/Kconfig   |  10 +
> >  drivers/crypto/aspeed/Makefile  |   1 +
> >  drivers/crypto/aspeed/aspeed_hace.c | 381
> 
> >  drivers/crypto/hash/Kconfig |   8 +
> >  6 files changed, 403 insertions(+)
> >  create mode 100644 drivers/crypto/aspeed/Kconfig  create mode 100644
> > drivers/crypto/aspeed/Makefile  create mode 100644
> > drivers/crypto/aspeed/aspeed_hace.c
> >
> > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index
> > 0082177c21..675081ecd3 100644
> > --- a/drivers/crypto/Kconfig
> > +++ b/drivers/crypto/Kconfig
> > @@ -4,4 +4,6 @@ source drivers/crypto/hash/Kconfig
> >
> >  source drivers/crypto/fsl/Kconfig
> >
> > +source drivers/crypto/aspeed/Kconfig
> > +
> >  endmenu
> > diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile index
> > e8bae43e3f..6b762565a1 100644
> > --- a/drivers/crypto/Makefile
> > +++ b/drivers/crypto/Makefile
> > @@ -7,3 +7,4 @@ obj-$(CONFIG_EXYNOS_ACE_SHA)+= ace_sha.o
> >  obj-y += rsa_mod_exp/
> >  obj-y += fsl/
> >  obj-y += hash/
> > +obj-y += aspeed/
> > diff --git a/drivers/crypto/aspeed/Kconfig
> > b/drivers/crypto/aspeed/Kconfig new file mode 100644 index
> > 00..471c06f986
> > --- /dev/null
> > +++ b/drivers/crypto/aspeed/Kconfig
> > @@ -0,0 +1,10 @@
> > +config ASPEED_HACE
> > +   bool "ASPEED Hash and Crypto Engine"
> > +   depends on DM_HASH
> > +   help
> > + Select this option to enable a driver for using the SHA engine in
> > + the ASPEED BMC SoCs.
> > +
> > + Enabling this allows the use of SHA operations in hardware
> without
> > + requiring the SHA software implementations. It also improves
> performance
> > + and saves code size.
> > diff --git a/drivers/crypto/aspeed/Makefile
> > b/drivers/crypto/aspeed/Makefile new file mode 100644 index
> > 00..84e6bfe82a
> > --- /dev/null
> > +++ b/drivers/crypto/aspeed/Makefile
> > @@ -0,0 +1 @@
> > +obj-$(CONFIG_ASPEED_HACE) += aspeed_hace.o
> > diff --git a/drivers/crypto/aspeed/aspeed_hace.c
> > b/drivers/crypto/aspeed/aspeed_hace.c
> > new file mode 100644
> > index 00..1178cc6a76
> > --- /dev/null
> > +++ b/drivers/crypto/aspeed/aspeed_hace.c
> > @@ -0,0 +1,381 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright 2021 ASPEED Technology Inc.
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/* register offsets*/
> > +#define HACE_STS   0x1C
> > +#define   HACE_HASH_DATA_OVF   BIT(23)
> > +#define   HACE_HASH_INTBIT(9)
> > +#define   HACE_HASH_BUSY   BIT(0)
> > +#define HACE_HASH_DATA 0x20
> > +#define HACE_HASH_DIGEST   0x24
> > +#define HACE_HASH_HMAC_KEY 0x28
> > +#define HACE_HASH_DATA_LEN 0x2C
> > +#define HACE_HASH_CMD  0x30
> > +#define   HACE_HASH_MODE_ACCUM BIT(8)
> > +#define   HACE_HASH_ALGO_SHA1  BIT(5)
> > +#define   HACE_HASH_ALGO_SHA256(BIT(6) | BIT(4))
> > +#define   HACE_HASH_ALGO_SHA384(BIT(10) | BIT(6)
> | BIT(5))
> > +#define   HACE_HASH_ALGO_SHA512(BIT(6) | BIT(5))
> > +#define   HACE_HASH_SHA_BE_EN  BIT(3)
> > +
> > 

RE: [PATCH next v7 07/12] crypto: aspeed: Add AST2600 ACRY support

2021-10-20 Thread ChiaWei Wang
> From: Joel Stanley 
> Sent: Thursday, October 21, 2021 8:25 AM
> 
> On Wed, 20 Oct 2021 at 02:50, Chia-Wei Wang
>  wrote:
> >
> > ACRY is deisnged to accerlerate ECC/RSA digital signature
> 
> designed
> accelerate

Thanks for pointing out the typo. It will be fixed in the next revision

Regards,
Chiawei

> 
> > generation and verification.
> >
> > Signed-off-by: Chia-Wei Wang 
> > ---
> >  drivers/crypto/aspeed/Kconfig   |  10 ++
> >  drivers/crypto/aspeed/Makefile  |   1 +
> >  drivers/crypto/aspeed/aspeed_acry.c | 190
> 
> >  lib/rsa/Kconfig |  10 +-
> >  4 files changed, 210 insertions(+), 1 deletion(-)  create mode 100644
> > drivers/crypto/aspeed/aspeed_acry.c
> >
> > diff --git a/drivers/crypto/aspeed/Kconfig
> > b/drivers/crypto/aspeed/Kconfig index 471c06f986..9bf317177a 100644
> > --- a/drivers/crypto/aspeed/Kconfig
> > +++ b/drivers/crypto/aspeed/Kconfig
> > @@ -8,3 +8,13 @@ config ASPEED_HACE
> >   Enabling this allows the use of SHA operations in hardware
> without
> >   requiring the SHA software implementations. It also improves
> performance
> >   and saves code size.
> > +
> > +config ASPEED_ACRY
> > +   bool "ASPEED RSA and ECC Engine"
> > +   depends on ASPEED_AST2600
> > +   help
> > +Select this option to enable a driver for using the RSA/ECC engine
> in
> > +the ASPEED BMC SoCs.
> > +
> > +Enabling this allows the use of RSA/ECC operations in hardware
> without requiring the
> > +software implementations. It also improves performance and
> saves code size.
> > diff --git a/drivers/crypto/aspeed/Makefile
> > b/drivers/crypto/aspeed/Makefile index 84e6bfe82a..58b55fc46e 100644
> > --- a/drivers/crypto/aspeed/Makefile
> > +++ b/drivers/crypto/aspeed/Makefile
> > @@ -1 +1,2 @@
> >  obj-$(CONFIG_ASPEED_HACE) += aspeed_hace.o
> > +obj-$(CONFIG_ASPEED_ACRY) += aspeed_acry.o
> > diff --git a/drivers/crypto/aspeed/aspeed_acry.c
> > b/drivers/crypto/aspeed/aspeed_acry.c
> > new file mode 100644
> > index 00..c28cdf374b
> > --- /dev/null
> > +++ b/drivers/crypto/aspeed/aspeed_acry.c
> > @@ -0,0 +1,190 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright 2021 ASPEED Technology Inc.
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/* ACRY register offsets */
> > +#define ACRY_CTRL1 0x00
> > +#define   ACRY_CTRL1_RSA_DMA   BIT(1)
> > +#define   ACRY_CTRL1_RSA_START BIT(0)
> > +#define ACRY_CTRL2 0x44
> > +#define ACRY_CTRL3 0x48
> > +#define   ACRY_CTRL3_SRAM_AHB_ACCESS   BIT(8)
> > +#define   ACRY_CTRL3_ECC_RSA_MODE_MASK GENMASK(5, 4)
> > +#define   ACRY_CTRL3_ECC_RSA_MODE_SHIFT4
> > +#define ACRY_DMA_DRAM_SADDR0x4c
> > +#define ACRY_DMA_DMEM_TADDR0x50
> > +#define   ACRY_DMA_DMEM_TADDR_LEN_MASK GENMASK(15, 0)
> > +#define   ACRY_DMA_DMEM_TADDR_LEN_SHIFT0
> > +#define ACRY_RSA_PARAM 0x58
> > +#define   ACRY_RSA_PARAM_EXP_MASK  GENMASK(31, 16)
> > +#define   ACRY_RSA_PARAM_EXP_SHIFT 16
> > +#define   ACRY_RSA_PARAM_MOD_MASK  GENMASK(15, 0)
> > +#define   ACRY_RSA_PARAM_MOD_SHIFT 0
> > +#define ACRY_RSA_INT_EN0x3f8
> 
> The datasheet says this register is at address 0xf8.
> 
> > +#define   ACRY_RSA_INT_EN_RSA_READYBIT(2)
> > +#define   ACRY_RSA_INT_EN_RSA_CMPLTBIT(1)
> > +#define ACRY_RSA_INT_STS   0x3fc
> 
> The datasheet says this register is at address 0xfc.
> 
> > +#define   ACRY_RSA_INT_STS_RSA_READY   BIT(2)
> > +#define   ACRY_RSA_INT_STS_RSA_CMPLT   BIT(1)
> > +
> > +/* misc. constant */
> > +#define ACRY_ECC_MODE  2
> > +#define ACRY_RSA_MODE  3
> > +#define ACRY_CTX_BUFSZ 0x600
> > +
> > +struct aspeed_acry {
> > +   phys_addr_t base;
> > +   phys_addr_t sram_base; /* internal sram */
> > +   struct clk clk;
> > +};
> > +
> > +static int aspeed_acry_mod_exp(struct udevice *dev, const uint8_t *sig,
> uint32_t sig_len,
> > +  struct key_prop *prop, uint8_t *out) {
> > +   int i, j;
> > +   u8 *ctx;
> > +   u8 *ptr;
> > +   u32 reg;
> > +   struct aspeed_acry *acry = dev_get_priv(dev);
> > +
> > +   ctx = memalign(16, ACRY_CTX_BUFSZ);
> > +   if (!ctx)
> > +   return -ENOMEM;
> > +
> > +   memset(ctx, 0, ACRY_CTX_BUFSZ);
> > +
> > +   ptr = (u8 *)prop->public_exponent;
> > +   for (i = prop->exp_len - 1, j = 0; i >= 0; --i) {
> > +   ctx[j] = ptr[i];
> > +   j++;
> > +   j = (j % 16) ? j : j + 32;
> > +   }
> > +
> > +   ptr = (u8 *)prop->modulus;
> > +   for (i = (prop->num_bits >> 3) - 1, j = 0; i >= 0; --i) {
> > +   ctx[j + 16] = ptr[i];
> > +   j++;
> > +   j = (j % 16) ? j : j + 32;
> > +   }
> > +
> > + 

RE: [PATCH next v7 11/12] configs: aspeed: Make EXTRA_ENV_SETTINGS board specific

2021-10-20 Thread ChiaWei Wang
> From: Joel Stanley 
> Sent: Thursday, October 21, 2021 8:31 AM
> 
> On Wed, 20 Oct 2021 at 02:50, Chia-Wei Wang
>  wrote:
> >
> > Move CONFIG_EXTRA_ENV_SETTINGS to board-specific configuration
> > headers.
> >
> > Signed-off-by: Chia-Wei Wang 
> > ---
> >  include/configs/aspeed-common.h | 9 -
> >  include/configs/evb_ast2500.h   | 9 +
> >  include/configs/evb_ast2600.h   | 9 +
> >  3 files changed, 18 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/configs/aspeed-common.h
> > b/include/configs/aspeed-common.h index 5177bf20fa..96526e1a75 100644
> > --- a/include/configs/aspeed-common.h
> > +++ b/include/configs/aspeed-common.h
> > @@ -38,13 +38,4 @@
> >   */
> >  #define CONFIG_BOOTP_BOOTFILESIZE
> >
> > -/*
> > - * Miscellaneous configurable options
> > - */
> > -
> > -#define CONFIG_EXTRA_ENV_SETTINGS \
> > -   "verify=yes\0"  \
> 
> Does this impact any of the code we have enabled? It appears bootm sets
> verify unconditionally when CONFIG_FIT_SIGNATURE is enabled.
> 
> > -   "spi_dma=yes\0" \
> > -   ""
> 
> The spi_dma option isn't supported by any code in u-boot. Can we drop it?

These settings are left here in case any backward compatibility issues.
If no concern, we can drop them.

Chiawei

> 
> > -
> >  #endif /* __AST_COMMON_CONFIG_H */
> > diff --git a/include/configs/evb_ast2500.h
> > b/include/configs/evb_ast2500.h index dc032c1a41..a886fd941e 100644
> > --- a/include/configs/evb_ast2500.h
> > +++ b/include/configs/evb_ast2500.h
> > @@ -13,4 +13,13 @@
> >
> >  #define CONFIG_SYS_UBOOT_BASE  CONFIG_SYS_TEXT_BASE
> >
> > +/* Memory Info */
> > +#define CONFIG_SYS_LOAD_ADDR   0x8300
> > +
> > +/* Misc */
> > +#define CONFIG_EXTRA_ENV_SETTINGS \
> > +   "verify=yes\0"  \
> > +   "spi_dma=yes\0" \
> > +   ""
> > +
> >  #endif /* __CONFIG_H */
> > diff --git a/include/configs/evb_ast2600.h
> > b/include/configs/evb_ast2600.h index 177a52eb91..d2aceb6663 100644
> > --- a/include/configs/evb_ast2600.h
> > +++ b/include/configs/evb_ast2600.h
> > @@ -10,4 +10,13 @@
> >
> >  #define CONFIG_SYS_UBOOT_BASE  CONFIG_SYS_TEXT_BASE
> >
> > +/* Memory Info */
> > +#define CONFIG_SYS_LOAD_ADDR   0x8300
> > +
> > +/* Misc */
> > +#define CONFIG_EXTRA_ENV_SETTINGS \
> > +   "verify=yes\0"  \
> > +   "spi_dma=yes\0" \
> > +   ""
> > +
> >  #endif /* __CONFIG_H */
> > --
> > 2.17.1
> >


RE: [PATCH next v7 04/12] crypto: aspeed: Add AST2600 HACE support

2021-10-20 Thread ChiaWei Wang
> From: Dhananjay Phadke 
> Sent: Thursday, October 21, 2021 9:33 AM
> 
> On Wed, 20 Oct 2021, Chia-Wei Wang wrote:
> 
> > +static const struct hash_ops aspeed_hace_ops = {
> > +   .hash_init = aspeed_hace_init,
> > +   .hash_update = aspeed_hace_update,
> > +   .hash_finish = aspeed_hace_finish,
> > +   .hash_digest_wd = aspeed_hace_digest_wd,
> > +   .hash_digest = aspeed_hace_digest,
> > +};
> > +
> > +static const struct udevice_id aspeed_hace_ids[] = {
> > +   { .compatible = "aspeed,ast2600-hace" },
> > +   { }
> > +};
> > +
> 
> Why only ast2600? Is ast2500 engine different or this driver can also support
> it?

The HACE of AST2500 has certain HW issues.
Therefore we don't enable support for it.

Chiawei


RE: [PATCH next v6 07/12] crypto: aspeed: Add AST2600 ACRY support

2021-10-17 Thread ChiaWei Wang
> From: U-Boot  On Behalf Of Chia-Wei Wang
> Sent: Friday, October 15, 2021 10:04 AM
> 
> ACRY is deisnged to accerlerate ECC/RSA digital signature generation and
> verification.
> 
> Signed-off-by: Chia-Wei Wang 
> ---
>  drivers/crypto/aspeed/Kconfig   |  10 ++
>  drivers/crypto/aspeed/Makefile  |   1 +
>  drivers/crypto/aspeed/aspeed_acry.c | 182
> 
>  lib/rsa/Kconfig |  10 +-
>  4 files changed, 202 insertions(+), 1 deletion(-)  create mode 100644
> drivers/crypto/aspeed/aspeed_acry.c
> 
> diff --git a/drivers/crypto/aspeed/Kconfig b/drivers/crypto/aspeed/Kconfig
> index 471c06f986..9bf317177a 100644
> --- a/drivers/crypto/aspeed/Kconfig
> +++ b/drivers/crypto/aspeed/Kconfig
> @@ -8,3 +8,13 @@ config ASPEED_HACE
> Enabling this allows the use of SHA operations in hardware without
> requiring the SHA software implementations. It also improves
> performance
> and saves code size.
> +
> +config ASPEED_ACRY
> + bool "ASPEED RSA and ECC Engine"
> + depends on ASPEED_AST2600
> + help
> +  Select this option to enable a driver for using the RSA/ECC engine in
> +  the ASPEED BMC SoCs.
> +
> +  Enabling this allows the use of RSA/ECC operations in hardware without
> requiring the
> +  software implementations. It also improves performance and saves code
> size.
> diff --git a/drivers/crypto/aspeed/Makefile b/drivers/crypto/aspeed/Makefile
> index 84e6bfe82a..58b55fc46e 100644
> --- a/drivers/crypto/aspeed/Makefile
> +++ b/drivers/crypto/aspeed/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_ASPEED_HACE) += aspeed_hace.o
> +obj-$(CONFIG_ASPEED_ACRY) += aspeed_acry.o
> diff --git a/drivers/crypto/aspeed/aspeed_acry.c
> b/drivers/crypto/aspeed/aspeed_acry.c
> new file mode 100644
> index 00..0b948f828a
> --- /dev/null
> +++ b/drivers/crypto/aspeed/aspeed_acry.c
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2021 ASPEED Technology Inc.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* ACRY register offsets */
> +#define ACRY_CTRL1   0x00
> +#define   ACRY_CTRL1_RSA_DMA BIT(1)
> +#define   ACRY_CTRL1_RSA_START   BIT(0)
> +#define ACRY_CTRL2   0x44
> +#define ACRY_CTRL3   0x48
> +#define   ACRY_CTRL3_SRAM_AHB_ACCESS BIT(8)
> +#define   ACRY_CTRL3_ECC_RSA_MODE_MASK   GENMASK(5, 4)
> +#define   ACRY_CTRL3_ECC_RSA_MODE_SHIFT  4
> +#define ACRY_DMA_DRAM_SADDR  0x4c
> +#define ACRY_DMA_DMEM_TADDR  0x50
> +#define   ACRY_DMA_DMEM_TADDR_LEN_MASK   GENMASK(15, 0)
> +#define   ACRY_DMA_DMEM_TADDR_LEN_SHIFT  0
> +#define ACRY_RSA_PARAM   0x58
> +#define   ACRY_RSA_PARAM_EXP_MASKGENMASK(31, 16)
> +#define   ACRY_RSA_PARAM_EXP_SHIFT   16
> +#define   ACRY_RSA_PARAM_MOD_MASKGENMASK(15, 0)
> +#define   ACRY_RSA_PARAM_MOD_SHIFT   0
> +#define ACRY_RSA_INT_EN  0x3f8
> +#define   ACRY_RSA_INT_EN_RSA_READY  BIT(2)
> +#define   ACRY_RSA_INT_EN_RSA_CMPLT  BIT(1)
> +#define ACRY_RSA_INT_STS 0x3fc
> +#define   ACRY_RSA_INT_STS_RSA_READY BIT(2)
> +#define   ACRY_RSA_INT_STS_RSA_CMPLT BIT(1)
> +
> +/* misc. constant */
> +#define ACRY_ECC_MODE2
> +#define ACRY_RSA_MODE3
> +#define ACRY_CTX_BUFSZ   0x600
> +
> +struct aspeed_acry {
> + phys_addr_t base;
> + phys_addr_t sram_base; /* internal sram */
> + struct clk clk;
> +};
> +
> +static int aspeed_acry_mod_exp(struct udevice *dev, const uint8_t *sig,
> uint32_t sig_len,
> +struct key_prop *prop, uint8_t *out) {
> + int i, j;
> + u8 *ctx;
> + u8 *ptr;
> + u32 reg;
> + struct aspeed_acry *acry = dev_get_priv(dev);
> +
> + ctx = memalign(16, ACRY_CTX_BUFSZ);
> + if (!ctx)
> + return -ENOMEM;
> +
> + memset(ctx, 0, ACRY_CTX_BUFSZ);
> +
> + ptr = (u8 *)prop->public_exponent;
> + for (i = prop->exp_len - 1, j = 0; i >= 0; --i) {
> + ctx[j] = ptr[i];
> + j++;
> + j = (j % 16) ? j : j + 32;
> + }
> +
> + ptr = (u8 *)prop->modulus;
> + for (i = (prop->num_bits >> 3) - 1, j = 0; i >= 0; --i) {
> + ctx[j + 16] = ptr[i];
> + j++;
> + j = (j % 16) ? j : j + 32;
> + }
> +
> + ptr = (u8 *)sig;
> + for (i = sig_len - 1, j = 0; i >= 0; --i) {
> + ctx[j + 32] = ptr[i];
> + j++;
> + j = (j % 16) ? j : j + 32;
> + }
> +
> + writel((u32)ctx, acry->base + ACRY_DMA_DRAM_SADDR);
> +
> + reg = (((prop->exp_len << 3) << ACRY_RSA_PARAM_EXP_SHIFT) &
> ACRY_RSA_PARAM_EXP_MASK) |
> +   ((prop->num_bits << ACRY_RSA_PARAM_MOD_SHIFT) &
> ACRY_RSA_PARAM_MOD_MASK);
> + writel(reg, acry->base + ACRY_RSA_PARAM);
> +
> + reg = (ACRY_CTX_BUFSZ << ACRY_DMA_DMEM_TADDR_LEN_SHIFT) &
> ACRY_DMA_DMEM_TADDR_LEN_MASK;
> + 

RE: [PATCH next v5 01/12] image: fit: Fix parameter name for hash algorithm

2021-10-14 Thread ChiaWei Wang
Hi Simon,

> From: Simon Glass 
> Sent: Thursday, October 14, 2021 11:10 PM
> 
> Hi Chia-Wei,
> 
> On Sun, 3 Oct 2021 at 19:54, Chia-Wei Wang
>  wrote:
> >
> > Fix inconsistent function parameter name of the hash algorithm.
> >
> > Signed-off-by: Chia-Wei Wang 
> > Fixes: 92055e138f2 ("image: Drop if/elseif hash selection in
> > calculate_hash()")
> > ---
> >  common/image-fit.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/common/image-fit.c b/common/image-fit.c index
> > 5a0a0cc200..9e8a1f36c1 100644
> > --- a/common/image-fit.c
> > +++ b/common/image-fit.c
> > @@ -1229,7 +1229,7 @@ int calculate_hash(const void *data, int data_len,
> const char *name,
> > return -1;
> > }
> >
> > -   hash_algo = hash_algo_lookup_by_name(algo);
> > +   hash_algo = hash_algo_lookup_by_name(name);
> > if (hash_algo == HASH_ALGO_INVALID) {
> > debug("Unsupported hash algorithm\n");
> > return -1;
> > --
> 
> Can you please fix the comment for this function as well?
> 
>  * @algo: requested hash algorithm
> 
> (should be @name)
> 

Sure. I missed the comment part.
Will send v6 to make them consistent ASAP.

Regards,
Chiawei


RE: [PATCH next] lib: hash-checksum: Use DM_HASH if supported

2021-10-06 Thread ChiaWei Wang
Hi Simon,

> From: Simon Glass 
> Sent: Wednesday, October 6, 2021 10:10 PM
> 
> Hi Chia-Wei,
> 
> On Thu, 16 Sept 2021 at 00:39, Chia-Wei Wang
>  wrote:
> >
> > Use DM_HASH to perform hashing operations if supported.
> > Thus either SW or HW-assisted hashing could be leveraged.
> 
> This is missing a full motivation. Please can you explain why this code is
> needed on a board, rather than just the host?
> 
> As of recently, this has become host-only code.

The entry to non-DM hash function for U-Boot is kind of inconsistent.

When a FIT image is verified by a hash digest:
hash-1 {
algo = "sha256";
};

The hash is calculated by calculate_hash() in image-fit.c.
fit_image_verify_with_data() -> fit_image_check_hash() -> calculate_hash()

However, when a FIT image is verified by a checksum signature:
signature {
algo = "sha256,rsa2048";
key-name-hint = "dev";
};

The hash comes from hash_calculate() in hash-checksum.c.
fit_image_verify_with_data() -> fit_image_setup_verify() -> 
image_get_checksum_algo() -> hash_calculate()

I checked the master and next branches. It seems that the logic still exists. 
(correct me if I am wrong)
This patch is like a temporary solution to make the DM_HASH work smoothly.
I believe a patch to refactor hash calculation of U-boot itself and the host 
tools is needed in the future.

Regards,
Chiawei


RE: [PATCH next v4 09/11] configs: ast2600-evb: Enable SPL FIT support

2021-10-03 Thread ChiaWei Wang
Hi Tom,

> From: Tom Rini 
> Sent: Monday, October 4, 2021 2:41 AM
> 
> On Thu, Sep 16, 2021 at 04:52:19PM +0800, Chia-Wei Wang wrote:
> 
> > Enable SPL FIT image load and verification support.
> > The HW accelerated SHA is also available with the newly added support
> > of the HACE HW hash engine.
> >
> > The SPL thumb build is also enabled to keep the binary less than 64KB
> > to fit into the Aspeed secure boot design.
> >
> > Signed-off-by: Chia-Wei Wang 
> 
> This causes the board to fail to build:
>arm:  +   evb-ast2600
> +(evb-ast2600) common/image-fit.c: In function 'calculate_hash':
> +(evb-ast2600) common/image-fit.c:1232:46: error: 'algo' undeclared (first use
> in this function)
> +(evb-ast2600)  1232 | hash_algo =
> hash_algo_lookup_by_name(algo);
> +(evb-ast2600)   |
> ^~~~
> +(evb-ast2600) common/image-fit.c:1232:46: note: each undeclared
> +identifier is reported only once for each function it appears in
> +(evb-ast2600) make[2]: *** [common/image-fit.o] Error 1
> +(evb-ast2600) make[1]: *** [common] Error 2
> +(evb-ast2600) make: *** [sub-make] Error 2
> 
> Please rebase and repost the series, thanks!

Thanks! Will rebase to fix and send v5 ASAP.

Regards,
Chiawei


RE: [PATCH 2/4] dm: hash: Add new UCLASS_HASH support

2021-09-22 Thread ChiaWei Wang
Hi Simon,

> From: Simon Glass 
> Sent: Thursday, September 23, 2021 12:19 AM
> 
> Hi,
> 
> On Thu, 2 Sept 2021 at 07:28, Tom Rini  wrote:
> >
> > On Fri, Jul 30, 2021 at 09:08:03AM +0800, Chia-Wei Wang wrote:
> >
> > > Add UCLASS_HASH for hash driver development. Thus the hash drivers
> > > (SW or HW-accelerated) can be developed in the DM-based fashion.
> > >
> > > Signed-off-by: Chia-Wei Wang 
> >
> > Applied to u-boot/next, thanks!
> 
> Oddly enough I didn't see this patch but did see Tom's reply.

Truly odd. You and Tom are on the '--to' list. 
I also checked the content sent on U-Boot Patchwork as shown below.

---
To: , , 
Subject: [PATCH 2/4] dm: hash: Add new UCLASS_HASH support
---

Regards,
Chiawei


RE: [PATCH 4/4] fit: Use DM hash driver if supported

2021-09-21 Thread ChiaWei Wang
Hi Alex,

> From: Alex G. 
> Sent: Friday, September 17, 2021 12:00 AM
> 
> On 7/29/21 8:08 PM, Chia-Wei Wang wrote:
> > Calculate hash using DM driver if supported.
> > For backward compatibility, the call to legacy hash functions is
> > reserved.
> >
> > Signed-off-by: Chia-Wei Wang 
> > ---
> >   common/image-fit.c | 30 ++
> >   1 file changed, 30 insertions(+)
> >
> > diff --git a/common/image-fit.c b/common/image-fit.c index
> > d6b2c3c7ec..ec2e526356 100644
> > --- a/common/image-fit.c
> > +++ b/common/image-fit.c
> > @@ -25,6 +25,10 @@
> >   #include 
> >   #include 
> >   #include 
> > +#ifdef CONFIG_DM_HASH
> > +#include 
> > +#include 
> > +#endif
> >   DECLARE_GLOBAL_DATA_PTR;
> >   #endif /* !USE_HOSTCC*/
> >
> > @@ -1214,6 +1218,31 @@ int fit_set_timestamp(void *fit, int noffset,
> time_t timestamp)
> >   int calculate_hash(const void *data, int data_len, const char *algo,
> > uint8_t *value, int *value_len)
> >   {
> > +#if !defined(USE_HOSTCC) && defined(CONFIG_DM_HASH)
> 
> This file is shared in its entirety with host tools. There isn't a huge 
> opportunity
> for using a DM-type approach here without #ifndef USE_HOSTCC.
> 

There are still clean up missions to make U-boot bootloader to move on to the 
DM-based approach.

For instance, when a FIT image is verified by a hash digest, the hash is 
calculated by calculate_hash() in image-fit.c.
However, when a FIT image is verified by a signature, the hash comes from 
hash_calculate() in hash-checksum.c.
In my personal opinion, a consistent way to calculate hash for U-Boot 
bootloader would be better for maintenance.

To make this transition more smoothly, currently the USE_HOSTCC and 
CONFIG_DM_HASH are added in an ad-hoc way.

Chiawei

> > +   int rc;
> > +   enum HASH_ALGO hash_algo;
> > +   struct udevice *dev;
> > +
> > +   rc = uclass_get_device(UCLASS_HASH, 0, );
> > +   if (rc) {
> > +   debug("failed to get hash device, rc=%d\n", rc);
> > +   return -1;
> > +   }
> > +
> > +   hash_algo = hash_algo_lookup_by_name(algo);
> > +   if (hash_algo == HASH_ALGO_INVALID) {
> > +   debug("Unsupported hash algorithm\n");
> > +   return -1;
> > +   };
> > +
> > +   rc = hash_digest_wd(dev, hash_algo, data, data_len, value, CHUNKSZ);
> > +   if (rc) {
> > +   debug("failed to get hash value, rc=%d\n", rc);
> > +   return -1;
> > +   }
> > +
> > +   *value_len = hash_algo_digest_size(hash_algo); #else
> > if (IMAGE_ENABLE_CRC32 && strcmp(algo, "crc32") == 0) {
> > *((uint32_t *)value) = crc32_wd(0, data, data_len,
> > CHUNKSZ_CRC32);
> > @@ -1242,6 +1271,7 @@ int calculate_hash(const void *data, int data_len,
> const char *algo,
> > debug("Unsupported hash alogrithm\n");
> > return -1;
> > }
> > +#endif
> > return 0;
> >   }
> >
> >


RE: [PATCH 3/4] crypto: hash: Add software hash DM driver

2021-09-21 Thread ChiaWei Wang
Hi Alex,

> From: Alex G. 
> Sent: Thursday, September 16, 2021 11:49 PM
> 
> On 7/29/21 8:08 PM, Chia-Wei Wang wrote:
> > Add purely software-implmented drivers to support multiple hash
> > operations including CRC, MD5, and SHA family.
> >
> > This driver is based on the new hash uclass.
> >
> > Signed-off-by: Chia-Wei Wang 
> > ---
> >   drivers/crypto/hash/Kconfig   |  11 ++
> >   drivers/crypto/hash/Makefile  |   1 +
> >   drivers/crypto/hash/hash_sw.c | 301
> ++
> >   3 files changed, 313 insertions(+)
> >   create mode 100644 drivers/crypto/hash/hash_sw.c
> >
> > diff --git a/drivers/crypto/hash/Kconfig b/drivers/crypto/hash/Kconfig
> > index e226144b9b..cd29a5c6a4 100644
> > --- a/drivers/crypto/hash/Kconfig
> > +++ b/drivers/crypto/hash/Kconfig
> > @@ -3,3 +3,14 @@ config DM_HASH
> > depends on DM
> > help
> >   If you want to use driver model for Hash, say Y.
> > +
> > +config HASH_SOFTWARE
> > +   bool "Enable driver for Hash in software"
> > +   depends on DM_HASH
> > +   depends on MD5
> > +   depends on SHA1
> > +   depends on SHA256
> > +   depends on SHA512_ALGO
> 
> I would have expected a U_BOOT_DRIVER() for each hash algo, rather than a
> U_BOOT_DRIVER() wich encompassess all possible algos. If I'm trying to use
> SHA256 in SPL, I might not have the room too add SHA1 and MD5, so I'd have
> issues using HASH_SOFTWARE, as designed.

I agree with the SPL size issue.
A future patches to move those CONFIG_SHAxxx/CONFIG_MD5 options into the 
DM-based hash_sw.c could be an option.
Thus the balance between the hash algos support and the code size can be 
managed.

> 
> > diff --git a/drivers/crypto/hash/hash_sw.c
> > b/drivers/crypto/hash/hash_sw.c new file mode 100644 index
> > 00..fea9d12609
> > --- /dev/null
> > +++ b/drivers/crypto/hash/hash_sw.c
> > @@ -0,0 +1,301 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2021 ASPEED Technology Inc.
> > + * Author: ChiaWei Wang   */ #include
> > + #include  #include  #include 
> > +#include  #include  #include 
> > +#include  #include  #include
> > + #include  #include 
> > +
> > +/* CRC16-CCITT */
> > +static void hash_init_crc16_ccitt(void *ctx) {
> > +   *((uint16_t *)ctx) = 0;
> 
> Undefined behavior: Pointer aliased type-punning. I would suggest using
> memset(). Might not be necessarrym as expleined in the next comment.
> 
> > +static void hash_update_crc16_ccitt(void *ctx, const void *ibuf,
> > +uint32_t ilen) static void hash_finish_crc16_ccitt(void *ctx, void
> > +*obuf)
> > +/* CRC32 */
> > +static void hash_init_crc32(void *ctx) static void
> > +hash_update_crc32(void *ctx, const void *ibuf, uint32_t ilen) static
> > +void hash_finish_crc32(void *ctx, void *obuf)
> > +/* SHA1 */
> > +static void hash_init_sha1(void *ctx)
> > +/* SHA256 */
> > +/* SHA384 */
> > +/* SHA512 */
> 
> This logic already exists in common/hash.c for hash_Lookup_algo() and
> hash_progressive_algo().

Yes.
To prevent modifying the 'static' keyword in common/hash.c while reusing the 
hash lib, the same logic appears in the DM-based hash_sw.c.

Chiawei


RE: [PATCH 2/4] dm: hash: Add new UCLASS_HASH support

2021-09-21 Thread ChiaWei Wang
Hi Alex,

> From: Alex G. 
> Sent: Thursday, September 16, 2021 11:43 PM
> 
> Hi,
> 
> On 7/29/21 8:08 PM, Chia-Wei Wang wrote:
> > Add UCLASS_HASH for hash driver development. Thus the hash drivers (SW
> > or HW-accelerated) can be developed in the DM-based fashion.
> 
> Software hashing implementations are shared tightly with host tools.
> With DM, there's no opportunity for code sharing with host tools. The design
> question that I have is "do we want to DM hashes, or do we want to DM
> hardware accelerators for hashes?"
> 
> I did some parallel work expose remaining hash algos via
> hash_lookup_algo() and hash_progressive_lookup_algo().

DM-based approach is mainly for the U-Boot bootloader part.
A consistent, abstract interface is therefore available for vendor drivers 
regardless of the hashing are conducted in the SW or HW-assisted fashion.
And the CONFIG_SHAxxx_HW_ACCEL/CONFIG_SHA_PROG_HW_ACCEL options can be dropped.

Most of the current DM-based, SW hash driver implementation reuses the code of 
hash lib.
The code sharing benefit is still greatly leveraged.

> 
> > Signed-off-by: Chia-Wei Wang 
> > ---
> >   drivers/crypto/Kconfig|   2 +
> >   drivers/crypto/Makefile   |   1 +
> >   drivers/crypto/hash/Kconfig   |   5 ++
> >   drivers/crypto/hash/Makefile  |   5 ++
> >   drivers/crypto/hash/hash-uclass.c | 121
> ++
> >   include/dm/uclass-id.h|   1 +
> >   include/u-boot/hash.h |  61 +++
> >   7 files changed, 196 insertions(+)
> >   create mode 100644 drivers/crypto/hash/Kconfig
> >   create mode 100644 drivers/crypto/hash/Makefile
> >   create mode 100644 drivers/crypto/hash/hash-uclass.c
> >   create mode 100644 include/u-boot/hash.h
> >
> > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index
> > 1ea116be75..0082177c21 100644
> > --- a/drivers/crypto/Kconfig
> > +++ b/drivers/crypto/Kconfig
> > @@ -1,5 +1,7 @@
> >   menu "Hardware crypto devices"
> >
> > +source drivers/crypto/hash/Kconfig
> > +
> Hashes are useful outside of cryptographic functions, so it seems odd to merge
> them in crypto. For example, CRC32 is not a hash useful in crypto, but
> otherwise widely used in u-boot.

Certain systems have the hash functionality included in their crypto engine. 
(e.g. ARM, FSL, ASPEED, etc.)
Based on this observation, the DM hash driver is placed under crypto/.
However, it is OK for me to move the hash/ out of crypto/ if a more specific 
place is created and preferred.

> 
> [snip]
> > diff --git a/drivers/crypto/hash/hash-uclass.c
> > b/drivers/crypto/hash/hash-uclass.c
> > new file mode 100644
> > index 00..446eb9e56a
> > --- /dev/null
> > +++ b/drivers/crypto/hash/hash-uclass.c
> > @@ -0,0 +1,121 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2021 ASPEED Technology Inc.
> > + * Author: ChiaWei Wang   */
> > +
> > +#define LOG_CATEGORY UCLASS_HASH
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +struct hash_info {
> > +   char *name;
> > +   uint32_t digest_size;
> > +};
> > +
> > +static const struct hash_info hash_info[HASH_ALGO_NUM] = {
> > +   [HASH_ALGO_CRC16_CCITT] = { "crc16-ccitt", 2 },
> > +   [HASH_ALGO_CRC32] = { "crc32", 4 },
> > +   [HASH_ALGO_MD5] = { "md5", 16 },
> > +   [HASH_ALGO_SHA1] = { "sha1", 20 },
> > +   [HASH_ALGO_SHA256] = { "sha256", 32 },
> > +   [HASH_ALGO_SHA384] = { "sha384", 48 },
> > +   [HASH_ALGO_SHA512] = { "sha512", 64}, };
> 
> It seems a step backwards to have to enum {} our hash algos, since we already
> identify them by their strings (e.g. "sha256"). and then associated ops 
> structure.
> The
> 
> > +
> > +enum HASH_ALGO hash_algo_lookup_by_name(const char *name)
> 
>  string -> hash_lookup_algo() -> ops struct
> 
> Is the current way to do things. hash_algo_lookup_by_name() does the
> roundabout through an enum. That doesn't make sense to me.
> 

The common hash-uclass.c also provides the string_to_enum conversion.
Both the DM-based hash driver works on both the string-based and enum-based 
scenario.

Chiawei


RE: [PATCH 0/4] crypto: Add new UCLASS_HASH

2021-08-24 Thread ChiaWei Wang
Hi All,

Do you have update on this patch series?
We look forward to continuing the SPL FIT booting patch for Aspeed SoCs based 
on this one.
Any advice and suggestions are appreciated.

Chiawei

> From: U-Boot  On Behalf Of Chia-Wei Wang
> Sent: Friday, July 30, 2021 9:08 AM
> 
> This patch series proposes new UCLASS_HASH for hash devices.
> Thus the hash drivers (SW or HW-accelerated) can be developed in the
> DM-based fashion.
> 
> A purely software implemented hash driver is also added under the newly
> added UCLASS_HASH uclass. In addition, the FIT image hash verification is also
> updated to leverage the UCLASS_HASH driver if configured.
> 
> As there is widly spread use of non-DM hash functions (common/hash.c), this
> patch does not remove them. More patches are needed if UCLASS_HASH is
> established.
> 
> Chia-Wei Wang (4):
>   lib/md5: Export progressive APIs
>   dm: hash: Add new UCLASS_HASH support
>   crypto: hash: Add software hash DM driver
>   fit: Use DM hash driver if supported
> 
>  common/image-fit.c|  30 +++
>  drivers/crypto/Kconfig|   2 +
>  drivers/crypto/Makefile   |   1 +
>  drivers/crypto/hash/Kconfig   |  16 ++
>  drivers/crypto/hash/Makefile  |   6 +
>  drivers/crypto/hash/hash-uclass.c | 121 
>  drivers/crypto/hash/hash_sw.c | 301
> ++
>  include/dm/uclass-id.h|   1 +
>  include/u-boot/hash.h |  61 ++
>  include/u-boot/md5.h  |   4 +
>  lib/md5.c |   6 +-
>  11 files changed, 546 insertions(+), 3 deletions(-)  create mode 100644
> drivers/crypto/hash/Kconfig  create mode 100644
> drivers/crypto/hash/Makefile  create mode 100644
> drivers/crypto/hash/hash-uclass.c  create mode 100644
> drivers/crypto/hash/hash_sw.c  create mode 100644 include/u-boot/hash.h
> 
> --
> 2.17.1



RE: [PATCH] armv7: Add Position Independent Execution support

2021-08-03 Thread ChiaWei Wang
Hi Peng,

> -Original Message-
> From: Peng Fan (OSS) 
> Sent: Tuesday, August 3, 2021 2:02 PM
> 
> 
> On 2021/8/3 5:30, Tom Rini wrote:
> > On Mon, Aug 02, 2021 at 06:44:57PM +0800, Chia-Wei Wang wrote:
> >> A U-Boot image could be loaded and executed at a different location
> >> than it was linked at.
> >>
> >> For example, Aspeed takes a stable release version of U-Boot image as
> >> the golden one for recovery purposes. When the primary storage such
> >> as flash is corrupted, the golden image could be loaded to any
> >> SRAM/DRAM address on demands through ethernet/UART/etc.
> 
> How? When flash got corrupted, how do you manage to load the golden
> image?

AST2600 supports boot-from-UART through HW strap.
When boot-from-UART is enabled for recovery purposes, the ROM code receives 
image data bytes from UART and put them into SRAM for execution.
In addition, if secure boot is also enabled, the image sent has to be signed 
and be verified by the public key programmed in OTP.

As AST2600 is based on ARMv7 architecture, this patch is implemented for.

Regards,
Chiawei

> 
> >>
> >> To deal with this condition, the PIE is needed as there is only one
> >> signed, golden image, which could be however executed at different
> >> places.
> >>
> >> This patch adds the PIE support for ARMv7 platform.
> >>
> >> Signed-off-by: Chia-Wei Wang 
> >> ---
> >>   arch/arm/Kconfig   |  4 +++-
> >>   arch/arm/cpu/armv7/start.S | 43
> ++
> >>   arch/arm/lib/crt0.S| 11 ++
> >>   arch/arm/lib/relocate.S| 35 ++-
> >>   4 files changed, 82 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index
> >> 2b7b625705..45879c9f06 100644
> >> --- a/arch/arm/Kconfig
> >> +++ b/arch/arm/Kconfig
> >> @@ -9,7 +9,7 @@ config ARM64
> >>select PHYS_64BIT
> >>select SYS_CACHE_SHIFT_6
> >>
> >> -if ARM64
> >> +if ARM64 || CPU_V7A
> >>   config POSITION_INDEPENDENT
> >>bool "Generate position-independent pre-relocation code"
> >>help
> >
> > Thanks for doing this.  I think we need to fix the depends on lines
> > here rather than hide with if ARM64 || CPU_V7A, and then fix anything
> > else that follows to also have the correct dependencies.
> >


RE: [PATCH] armv7: Add Position Independent Execution support

2021-08-02 Thread ChiaWei Wang
Hi Tom,

> From: Tom Rini 
> Sent: Tuesday, August 3, 2021 5:31 AM
> On Mon, Aug 02, 2021 at 06:44:57PM +0800, Chia-Wei Wang wrote:
> > A U-Boot image could be loaded and executed at a different location
> > than it was linked at.
> >
> > For example, Aspeed takes a stable release version of U-Boot image as
> > the golden one for recovery purposes. When the primary storage such as
> > flash is corrupted, the golden image could be loaded to any SRAM/DRAM
> > address on demands through ethernet/UART/etc.
> >
> > To deal with this condition, the PIE is needed as there is only one
> > signed, golden image, which could be however executed at different
> > places.
> >
> > This patch adds the PIE support for ARMv7 platform.
> >
> > Signed-off-by: Chia-Wei Wang 
> > ---
> >  arch/arm/Kconfig   |  4 +++-
> >  arch/arm/cpu/armv7/start.S | 43
> ++
> >  arch/arm/lib/crt0.S| 11 ++
> >  arch/arm/lib/relocate.S| 35 ++-
> >  4 files changed, 82 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index
> > 2b7b625705..45879c9f06 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -9,7 +9,7 @@ config ARM64
> > select PHYS_64BIT
> > select SYS_CACHE_SHIFT_6
> >
> > -if ARM64
> > +if ARM64 || CPU_V7A
> >  config POSITION_INDEPENDENT
> > bool "Generate position-independent pre-relocation code"
> > help
> 
> Thanks for doing this.  I think we need to fix the depends on lines here 
> rather
> than hide with if ARM64 || CPU_V7A, and then fix anything else that follows to
> also have the correct dependencies.

Thanks for reviewing this. I will prepare a v2 patch to include the 'depends 
on' fix.

Regards,
Chiawei


RE: [PATCH v3 06/14] common: fit: Use hash.c to call CRC/SHA function

2021-07-25 Thread ChiaWei Wang
Hi Tom,

> From: Tom Rini 
> Sent: Saturday, July 24, 2021 8:57 PM
> 
> On Tue, Jul 20, 2021 at 02:38:31PM +0800, Chia-Wei Wang wrote:
> 
> > From: Joel Stanley 
> >
> > Currently the FIT verification calls directly into SW implemented
> > functions to get a CRC/SHA/MD5 hash.
> >
> > This patch removes duplcated algorithm lookup and use hash_lookup_algo
> > to get the hashing function with HW accelearation supported if
> > configured.
> >
> > The MD5 direct call remains as it is not included in the hash lookup
> > table of hash.c.
> >
> > Signed-off-by: Joel Stanley 
> > Signed-off-by: Chia-Wei Wang 
> 
> While this is a good idea, there's some required prep work.  At least the
> following platforms don't compile due to this patch:
> ls1046ardb_qspi imx8mm_beacon imx8mn_beacon imx8mn_beacon_2g
> imx8mm-icore-mx8mm-ctouch2 imx8mm-icore-mx8mm-edimm2.2
> imx8mm_evk imx8mn_ddr4_evk imx8mn_evk imx8mp_evk imx8mq_evk
> imx8mm_venice imx8mq_phanbell phycore-imx8mm phycore-imx8mp
> pico-imx8mq verdin-imx8mm mt8183_pumpkin mt8516_pumpkin mscc_jr2
> mscc_luton mscc_ocelot mscc_serval mscc_servalt mt7620_mt7530_rfb
> mt7620_rfb mt7628_rfb
> 
> Which is likely due to cases where HASH or SPL_HASH_SUPPORT are not being
> selected as it was not previously required.
> 

Thanks for the notification of this error. I will examine the code flow to 
figure out the root cause on these platforms.

Meanwhile, Simon also suggested the need to add a new UCLASS_HASH to refactor 
the hash structure.
http://patchwork.ozlabs.org/project/uboot/patch/20210720063839.1518-4-chiawei_w...@aspeedtech.com/

I was wondering if I can prepare another leading patch for UCLASS_HASH and also 
to make sure the current codebase works fine?
After that, we can restart this patch series for Aspeed FIT booting.

Regards,
Chiawei


RE: [PATCH v3 03/14] crypto: aspeed: Add AST2600 HACE support

2021-07-23 Thread ChiaWei Wang
Hi Simon,

> -Original Message-
> From: Simon Glass 
> Sent: Wednesday, July 21, 2021 2:33 AM
> 
> Hi Chia-Wei,
> 
> On Tue, 20 Jul 2021 at 00:38, Chia-Wei Wang
>  wrote:
> >
> > From: Joel Stanley 
> >
> > Hash and Crypto Engine (HACE) is designed to accelerate the throughput
> > of hash data digest, and symmetric-key encryption.
> >
> > Signed-off-by: Joel Stanley 
> > Signed-off-by: Chia-Wei Wang 
> > ---
> >  drivers/crypto/Kconfig  |   2 +
> >  drivers/crypto/Makefile |   1 +
> >  drivers/crypto/aspeed/Kconfig   |  12 ++
> >  drivers/crypto/aspeed/Makefile  |   1 +
> >  drivers/crypto/aspeed/aspeed_hace.c | 308
> > 
> >  5 files changed, 324 insertions(+)
> >  create mode 100644 drivers/crypto/aspeed/Kconfig  create mode 100644
> > drivers/crypto/aspeed/Makefile  create mode 100644
> > drivers/crypto/aspeed/aspeed_hace.c
> 
> This hash interface is wonky.
> 
> There should be a hash uclass and this driver should be in that uclass. The
> existing hw_... functions should be dropped.
> 
> Please let me know if you need further guidance, but basically, we need a
> proper uclass and driver.

I am thinking to take the UCLASS_MOD_EXP as an example to create UCLASS_HASH.
This UCLASS_HASH should provide .ops callback like UCLASS_MOD_EXP do to support 
both SW and HW implemented HSAH.
Is this a right direction to start with?

In addition, adding UCLASS_HASH may involve lots of common code modification.
Should we make it part of the current patch series? Or a new patch series after 
this one?

Thanks,
Chiawei


RE: [PATCH 07/13] arm: aspeed: Disable ATAGs support

2021-02-08 Thread ChiaWei Wang
Hi Tom,

I have tested this patch with Aspeed SDK as well as the U-Boot mainline 
codebase on AST2500/AST2600 EVBs.
Both AST2500/AST2600 can boot to kernel normally. Thanks.

Chiawei

Tested-by: Chia-Wei Wang 

> -Original Message-
> From: Tom Rini 
> Sent: Thursday, February 4, 2021 10:24 AM
> To: u-boot@lists.denx.de
> Cc: Ryan Chen ; ChiaWei Wang
> ; BMC-SW 
> Subject: [PATCH 07/13] arm: aspeed: Disable ATAGs support
> 
> This platform never had to support an ATAGs-based Linux kernel, so remove
> the support for it.
> 
> Cc: Ryan Chen 
> Cc: Chia-Wei Wang 
> Cc: Aspeed BMC SW team 
> Signed-off-by: Tom Rini 
> ---
> I'm assuming, please correct me if I'm wrong.
> ---
>  include/configs/aspeed-common.h | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/include/configs/aspeed-common.h
> b/include/configs/aspeed-common.h index df0f5d2e76f7..8d666c6ed415
> 100644
> --- a/include/configs/aspeed-common.h
> +++ b/include/configs/aspeed-common.h
> @@ -12,11 +12,6 @@
> 
>  #include 
> 
> -/* Misc CPU related */
> -#define CONFIG_CMDLINE_TAG
> -#define CONFIG_SETUP_MEMORY_TAGS
> -#define CONFIG_INITRD_TAG
> -
>  #define CONFIG_SYS_SDRAM_BASEASPEED_DRAM_BASE
> 
>  #ifdef CONFIG_PRE_CON_BUF_SZ
> --
> 2.17.1



RE: [v3 0/2] Refactor AST2500 reset control

2020-10-14 Thread ChiaWei Wang
Hi Tom,

The patches has been rebased onto the up-to-date U-Boot master.
A makefile error is also fixed in the revised version and verified with an 
AST2500 EVB.

Thanks,
Chiawei

> -Original Message-
> From: ChiaWei Wang 
> Sent: Thursday, October 15, 2020 10:25 AM
> To: tr...@konsulko.com; u-boot@lists.denx.de; max...@google.com
> Subject: [v3 0/2] Refactor AST2500 reset control
> 
> This patch series refactors the reset method to use the System Control Unit
> (SCU) reset control for simplicity.
> 
> In addition, the naming of reset driver and Kconfig option is also refined for
> future consistency.
> 
> v3:
>   - Fix Makefile to adapt to the renamed reset driver file
> 
> v2:
>   - Rebase patches to fix conflict
> 
> 
> Chia-Wei, Wang (2):
>   reset: ast2500: Use SCU for reset control
>   cosmetic: reset: ast2500: Rename driver and configs
> 
>  arch/arm/dts/ast2500-u-boot.dtsi  |   7 +-
>  drivers/reset/Kconfig |  11 +--
>  drivers/reset/Makefile|   2 +-
>  drivers/reset/ast2500-reset.c | 104 -
>  drivers/reset/reset-ast2500.c | 109
> ++
>  include/dt-bindings/reset/ast2500-reset.h |  73 ---
>  6 files changed, 157 insertions(+), 149 deletions(-)  delete mode 100644
> drivers/reset/ast2500-reset.c  create mode 100644
> drivers/reset/reset-ast2500.c
> 
> --
> 2.17.1



RE: [PATCH 0/2] Refactor AST2500 reset control

2020-10-11 Thread ChiaWei Wang
Hi Tom,

Thanks for the review.

> -Original Message-
> From: Tom Rini 
> Sent: Thursday, October 8, 2020 11:52 PM
> To: ChiaWei Wang 
> Cc: Ryan Chen ; max...@google.com;
> u-boot@lists.denx.de; BMC-SW 
> Subject: Re: [PATCH 0/2] Refactor AST2500 reset control
> 
> On Thu, Oct 08, 2020 at 02:21:50AM +, ChiaWei Wang wrote:
> 
> > Hi Tom,
> >
> > Could you help us on the review of the following patches for Aspeed platform
> code?
> > http://patchwork.ozlabs.org/project/uboot/cover/20200908072104.10067-1
> > -chiawei_w...@aspeedtech.com/
> > http://patchwork.ozlabs.org/project/uboot/cover/20200907082507.22290-1
> > -dylan_h...@aspeedtech.com/
> >
> > These patches are in fact refactoring to the existing Aspeed code structure 
> > in
> U-Boot.
> > When the refactoring is done, we would like to send a patch series for the
> support of Aspeed next generation SoC (AST2600).
> 
> These patches seem to break qemu, or my merging of them on to the current
> tree was incorrect (I had to fixup the dtsi part):
> https://gitlab.denx.de/u-boot/u-boot/-/jobs/162152

These patches conflict with the Aspeed clock one recently merged.
I have sent a v2 version to fix the conflicts with branch rebase for the 
review. Thanks.

Chiawei


RE: [PATCH 0/2] Refactor AST2500 reset control

2020-10-07 Thread ChiaWei Wang
Hi Tom,

Could you help us on the review of the following patches for Aspeed platform 
code?
http://patchwork.ozlabs.org/project/uboot/cover/20200908072104.10067-1-chiawei_w...@aspeedtech.com/
http://patchwork.ozlabs.org/project/uboot/cover/20200907082507.22290-1-dylan_h...@aspeedtech.com/

These patches are in fact refactoring to the existing Aspeed code structure in 
U-Boot.
When the refactoring is done, we would like to send a patch series for the 
support of Aspeed next generation SoC (AST2600).
Thanks.

Chiawei

> -Original Message-
> From: ChiaWei Wang 
> Sent: Tuesday, September 8, 2020 3:21 PM
> To: Ryan Chen ; max...@google.com;
> u-boot@lists.denx.de
> Cc: BMC-SW 
> Subject: [PATCH 0/2] Refactor AST2500 reset control
> 
> This patch series refactors the reset method to use the System Control Unit
> (SCU) reset control for simplicity.
> 
> In addition, the naming of reset driver and Kconfig option is also refined for
> future consistency.
> 
> Chia-Wei, Wang (2):
>   reset: ast2500: Use SCU for reset control
>   cosmetic: reset: ast2500: Rename driver and configs
> 
>  arch/arm/dts/ast2500-u-boot.dtsi  |   7 +-
>  drivers/reset/Kconfig |  11 +--
>  drivers/reset/ast2500-reset.c | 104 -
>  drivers/reset/reset-ast2500.c | 109
> ++
>  include/dt-bindings/reset/ast2500-reset.h |  73 ---
>  5 files changed, 156 insertions(+), 148 deletions(-)  delete mode 100644
> drivers/reset/ast2500-reset.c  create mode 100644
> drivers/reset/reset-ast2500.c
> 
> --
> 2.17.1



RE: [PATCH v2 3/3] cosmetic: aspeed: Modify for SPDX-License

2020-09-06 Thread ChiaWei Wang
> -Original Message-
> From: Ryan Chen 
> Sent: Monday, August 31, 2020 2:03 PM
> To: Ryan Chen ; ChiaWei Wang
> ; Lukasz Majewski ;
> c...@kaod.org; Eddie James ; Simon Glass
> ; u-boot@lists.denx.de
> Subject: [PATCH v2 3/3] cosmetic: aspeed: Modify for SPDX-License
> 
> Modify SPDX-License for furture patch warning
> 
> Signed-off-by: Ryan Chen 
> ---
>  arch/arm/dts/ast2500-u-boot.dtsi | 1 +
>  include/dt-bindings/clock/aspeed-clock.h | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Chia-Wei, Wang 


RE: [PATCH v2 1/3] cosmetic: aspeed: ast2500: Rename clock header

2020-09-06 Thread ChiaWei Wang
> -Original Message-
> From: Ryan Chen 
> Sent: Monday, August 31, 2020 2:03 PM
> To: Ryan Chen ; ChiaWei Wang
> ; Lukasz Majewski ;
> c...@kaod.org; Eddie James ; Simon Glass
> ; u-boot@lists.denx.de
> Subject: [PATCH v2 1/3] cosmetic: aspeed: ast2500: Rename clock header
> 
> Rename the ast2500-scu.h to aspeed-clock.h.
> 
> Signed-off-by: Ryan Chen 
> ---
>  arch/arm/dts/ast2500-u-boot.dtsi| 2 +-
>  arch/arm/mach-aspeed/ast2500/sdram_ast2500.c| 2 +-
>  drivers/clk/aspeed/clk_ast2500.c| 2 +-
>  include/dt-bindings/clock/{ast2500-scu.h => aspeed-clock.h} | 0
>  4 files changed, 3 insertions(+), 3 deletions(-)  rename
> include/dt-bindings/clock/{ast2500-scu.h => aspeed-clock.h} (100%)
> 
Reviewed-by: Chia-Wei, Wang 



RE: [PATCH v2 2/3] clock:aspeed: Sync with Linux kernel clock header define

2020-09-06 Thread ChiaWei Wang
> -Original Message-
> From: Ryan Chen 
> Sent: Monday, August 31, 2020 2:03 PM
> To: Ryan Chen ; ChiaWei Wang
> ; Lukasz Majewski ;
> c...@kaod.org; Eddie James ; Simon Glass
> ; u-boot@lists.denx.de
> Subject: [PATCH v2 2/3] clock:aspeed: Sync with Linux kernel clock header
> define
> 
> v2: modify title description aspeed:clock -> clock:aspeed
> 
> Use kernel include/dt-bindings/clock/aspeed-clock.h define for clock driver.
> 
> Signed-off-by: Ryan Chen 
> ---
>  arch/arm/dts/ast2500-u-boot.dtsi | 20 +++
>  drivers/clk/aspeed/clk_ast2500.c | 38 +++--
>  include/dt-bindings/clock/aspeed-clock.h | 68 ++--
>  3 files changed, 68 insertions(+), 58 deletions(-)

Reviewed-by: Chia-Wei, Wang 


RE: [U-Boot] requesting a custodian tree for Aspeed SoCs maintenance

2020-08-11 Thread ChiaWei Wang
Hi Tom,

Thank you for the feedback.

> -Original Message-
> From: Tom Rini [mailto:tr...@konsulko.com]
> Sent: Tuesday, August 11, 2020 11:26 PM
> To: ChiaWei Wang 
> Cc: u-boot@lists.denx.de
> Subject: Re: [U-Boot] requesting a custodian tree for Aspeed SoCs
> maintenance
> 
> On Mon, Aug 10, 2020 at 03:19:43AM +, ChiaWei Wang wrote:
> o
> > Hi,
> >
> > We are Aspeed SW team and would like to request the creation of a
> > u-boot-aspeed.git custodian tree for the u-boot support related to
> > Aspeed SoCs. Attached is the SSH key generated for this purpose.
> >
> > By the way, the Aspeed SoCs are based on ARM architecture. However, we
> > are not sure if the Aspeed custodian tree should be a downstream of
> > u-boot-arm.git tree or not. As there is no update to u-boot-arm since 4 
> > years
> ago.
> >
> > If anything is further needed, please let us know.
> 
> To start with, I would like to handle Aspeed development by pulling respective
> patches in to one of my testing branches and merge them after appropriate
> review time.  If the volume of changes makes it worthwhile we can add a
> new custodian tree later.  Thanks! 

So currently, should we simply submit individual Aspeed patches to the mailing 
list for the review process?
In fact, we have sent certain patches couple days ago. 
(http://patchwork.ozlabs.org/project/uboot/cover/20200803093610.3222-1-chiawei_w...@aspeedtech.com/)
If it is the case, we will proceed the patch submission for Aspeed SoCs after 
the review is done.

Regards,
Chiawei


[U-Boot] requesting a custodian tree for Aspeed SoCs maintenance

2020-08-09 Thread ChiaWei Wang
Hi,

We are Aspeed SW team and would like to request the creation of a 
u-boot-aspeed.git
custodian tree for the u-boot support related to Aspeed SoCs. Attached is the 
SSH key
generated for this purpose.

By the way, the Aspeed SoCs are based on ARM architecture. However, we are not 
sure
if the Aspeed custodian tree should be a downstream of u-boot-arm.git tree or 
not. As
there is no update to u-boot-arm since 4 years ago.

If anything is further needed, please let us know.

Thanks,
Chiawei

ASPEED Technology Inc.
Tel: 886-3-5751185 ext:8637
E-mail: chiawei_w...@aspeedtech.com

* Email Confidentiality Notice 
DISCLAIMER:
This message (and any attachments) may contain legally privileged and/or other 
confidential information. If you have received it in error, please notify the 
sender by reply e-mail and immediately delete the e-mail and any attachments 
without copying or disclosing the contents. Thank you.



id_rsa.pub
Description: id_rsa.pub