On Wed, Feb 08, 2023 at 11:28:21AM -0700, Simon Glass wrote:
Hi Tom,
On Tue, 7 Feb 2023 at 17:10, Tom Rini trini@konsulko.com wrote:
On Tue, Feb 07, 2023 at 03:25:16PM -0700, Simon Glass wrote:
Hi Loic,
On Tue, 7 Feb 2023 at 14:47, Loic Poulain loic.poulain@linaro.org wrote:
Hi Simon,
On Tue, 7 Feb 2023 at 05:05, Simon Glass sjg@chromium.org wrote:
Hi Loic,
On Mon, 6 Feb 2023 at 15:12, Loic Poulain loic.poulain@linaro.org wrote:
Hi Simon,
Le lun. 6 févr. 2023 à 18:12, Simon Glass sjg@chromium.org a écrit : > > Hi Loic, > > On Wed, 1 Jun 2022 at 12:27, Loic Poulain loic.poulain@linaro.org wrote: > > > > Mark sha256_process as weak to allow hardware specific implementation. > > Add parameter for supporting multiple blocks processing. > > > > Signed-off-by: Loic Poulain loic.poulain@linaro.org > > --- > > lib/sha256.c | 26 +++++++++++++++++++------- > > 1 file changed, 19 insertions(+), 7 deletions(-) > >
[...]
> > +__weak void sha256_process(sha256_context *ctx, const unsigned char *data, > > + unsigned int blocks) > > +{ > > + if (!blocks) > > + return; > > + > > + while (blocks--) { > > + sha256_process_one(ctx, data); > > + data += 64; > > + } > > +} > > + > > void sha256_update(sha256_context *ctx, const uint8_t *input, uint32_t length) > > { > > uint32_t left, fill; > > @@ -204,17 +218,15 @@ void sha256_update(sha256_context *ctx, const uint8_t *input, uint32_t length) > > > > if (left && length >= fill) { > > memcpy((void *) (ctx->buffer + left), (void *) input, fill); > > - sha256_process(ctx, ctx->buffer); > > + sha256_process(ctx, ctx->buffer, 1); > > length -= fill; > > input += fill; > > left = 0; > > } > > > > - while (length >= 64) { > > - sha256_process(ctx, input); > > - length -= 64; > > - input += 64; > > - } > > + sha256_process(ctx, input, length / 64); > > + input += length / 64 * 64; > > + length = length % 64; > > > > if (length) > > memcpy((void *) (ctx->buffer + left), (void *) input, length); > > -- > > 2.7.4 > > > > I just came across this patch as it broke minnowmax.
Ok, is it a build time or runtime break?
Build, but you need the binary blobs to see it :-(
> > This should be using driver model, not weak functions. Please can you > take a look?
Just tested the minnowmax build (b69026c91f2e; minnowmax_defconfig; gcc-11.3.0), and I've not observed any issue (but I had to fake some of the binary blobs...). Could you share the build problem/error you
Unfortunately you need the blobs!
encountered? As you mentioned it, Is the error specifically related to _weak function linking? Would like to have a simple and quick fix before trying to move on to a more proper DM based solution.
It is just because of the code size increase, I believe. I am planning to dig into it a bit as Bin Meng asked for more info as to why I sent a revert for his patch moving U-Boot.
That honestly makes more sense, having stared at the commit in question. Perhaps Minnow needs LTO enabled.
Yes, that would likely help. But Bin's point is that it should be possible to move the text base.
I suspect that might just be more fall-out of the size problem and possibly some hard coded assumptions in the blobs?
Anyway, the point of this thread is that this needs to be done as drivers rather than weak functions. Unfortunately hash.c has grown a few appendages...this is yet another.
I don't know, it seems fine, especially since we aren't really talking about a "hash driver" but rather introducing some performance sensitive code.