| Message ID | 20230202070345.191514-1-neal_liu@aspeedtech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [v2,-next] crypto: aspeed: fix type warnings | expand |
On 2/1/2023 11:03 PM, Neal Liu wrote: > This patch fixes following warnings: > > 1. sparse: incorrect type in assignment (different base types) > Fix: change to __le32 type. > 2. sparse: cast removes address space '__iomem' of expression > Fix: use readb to avoid dereferencing the memory. > > Signed-off-by: Neal Liu <neal_liu@aspeedtech.com> > --- > Change since v1: keep iomem marker to remain its purpose. > > drivers/crypto/aspeed/aspeed-acry.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/crypto/aspeed/aspeed-acry.c b/drivers/crypto/aspeed/aspeed-acry.c > index 164c524015f0..f2429e699d14 100644 > --- a/drivers/crypto/aspeed/aspeed-acry.c > +++ b/drivers/crypto/aspeed/aspeed-acry.c > @@ -252,7 +252,7 @@ static int aspeed_acry_rsa_ctx_copy(struct aspeed_acry_dev *acry_dev, void *buf, > enum aspeed_rsa_key_mode mode) > { > const u8 *src = xbuf; > - u32 *dw_buf = (u32 *)buf; > + __le32 *dw_buf = (__le32 *)buf; All callers are passing acry_dev->buf_addr as buf, can just change that type to __le32 * and deref here directly? > int nbits, ndw; > int i, j, idx; > u32 data = 0; > @@ -302,7 +302,7 @@ static int aspeed_acry_rsa_ctx_copy(struct aspeed_acry_dev *acry_dev, void *buf, > static int aspeed_acry_rsa_transfer(struct aspeed_acry_dev *acry_dev) > { > struct akcipher_request *req = acry_dev->req; > - u8 *sram_buffer = (u8 *)acry_dev->acry_sram; > + u8 __iomem *sram_buffer = (u8 __iomem *)acry_dev->acry_sram; u8 cast seems unnecessary, readb takes (void *) and using "+ data_idx" offset below anyway. > struct scatterlist *out_sg = req->dst; > static u8 dram_buffer[ASPEED_ACRY_SRAM_MAX_LEN]; > int leading_zero = 1; > @@ -321,11 +321,11 @@ static int aspeed_acry_rsa_transfer(struct aspeed_acry_dev *acry_dev) > > for (j = ASPEED_ACRY_SRAM_MAX_LEN - 1; j >= 0; j--) { > data_idx = acry_dev->data_byte_mapping[j]; > - if (sram_buffer[data_idx] == 0 && leading_zero) { > + if (readb(sram_buffer + data_idx) == 0 && leading_zero) { > result_nbytes--; > } else { > leading_zero = 0; > - dram_buffer[i] = sram_buffer[data_idx]; > + dram_buffer[i] = readb(sram_buffer + data_idx); > i++; > } > }
On Thu, Feb 02, 2023 at 05:36:05PM -0800, Dhananjay Phadke wrote: > > > diff --git a/drivers/crypto/aspeed/aspeed-acry.c b/drivers/crypto/aspeed/aspeed-acry.c > > index 164c524015f0..f2429e699d14 100644 > > --- a/drivers/crypto/aspeed/aspeed-acry.c > > +++ b/drivers/crypto/aspeed/aspeed-acry.c > > @@ -252,7 +252,7 @@ static int aspeed_acry_rsa_ctx_copy(struct aspeed_acry_dev *acry_dev, void *buf, > > enum aspeed_rsa_key_mode mode) > > { > > const u8 *src = xbuf; > > - u32 *dw_buf = (u32 *)buf; > > + __le32 *dw_buf = (__le32 *)buf; > > All callers are passing acry_dev->buf_addr as buf, can just change that type > to __le32 * and deref here directly? As buf is already void *, the cast can simply be removed. > > > int nbits, ndw; > > int i, j, idx; > > u32 data = 0; > > @@ -302,7 +302,7 @@ static int aspeed_acry_rsa_ctx_copy(struct aspeed_acry_dev *acry_dev, void *buf, > > static int aspeed_acry_rsa_transfer(struct aspeed_acry_dev *acry_dev) > > { > > struct akcipher_request *req = acry_dev->req; > > - u8 *sram_buffer = (u8 *)acry_dev->acry_sram; > > + u8 __iomem *sram_buffer = (u8 __iomem *)acry_dev->acry_sram; > > u8 cast seems unnecessary, readb takes (void *) and using "+ data_idx" > offset below anyway. Either way a cast is unnecessary as (void __iomem *) converts to (u8 __iomem *) just fine. Thanks,
> > > diff --git a/drivers/crypto/aspeed/aspeed-acry.c > > > b/drivers/crypto/aspeed/aspeed-acry.c > > > index 164c524015f0..f2429e699d14 100644 > > > --- a/drivers/crypto/aspeed/aspeed-acry.c > > > +++ b/drivers/crypto/aspeed/aspeed-acry.c > > > @@ -252,7 +252,7 @@ static int aspeed_acry_rsa_ctx_copy(struct > aspeed_acry_dev *acry_dev, void *buf, > > > enum aspeed_rsa_key_mode mode) > > > { > > > const u8 *src = xbuf; > > > - u32 *dw_buf = (u32 *)buf; > > > + __le32 *dw_buf = (__le32 *)buf; > > > > All callers are passing acry_dev->buf_addr as buf, can just change > > that type to __le32 * and deref here directly? > > As buf is already void *, the cast can simply be removed. I'll just remove the cast. Thanks for your suggestion. > > > > > int nbits, ndw; > > > int i, j, idx; > > > u32 data = 0; > > > @@ -302,7 +302,7 @@ static int aspeed_acry_rsa_ctx_copy(struct > aspeed_acry_dev *acry_dev, void *buf, > > > static int aspeed_acry_rsa_transfer(struct aspeed_acry_dev *acry_dev) > > > { > > > struct akcipher_request *req = acry_dev->req; > > > - u8 *sram_buffer = (u8 *)acry_dev->acry_sram; > > > + u8 __iomem *sram_buffer = (u8 __iomem *)acry_dev->acry_sram; > > > > u8 cast seems unnecessary, readb takes (void *) and using "+ data_idx" > > offset below anyway. > > Either way a cast is unnecessary as (void __iomem *) converts to > (u8 __iomem *) just fine. I'll just remove the cast too. -Neal
diff --git a/drivers/crypto/aspeed/aspeed-acry.c b/drivers/crypto/aspeed/aspeed-acry.c index 164c524015f0..f2429e699d14 100644 --- a/drivers/crypto/aspeed/aspeed-acry.c +++ b/drivers/crypto/aspeed/aspeed-acry.c @@ -252,7 +252,7 @@ static int aspeed_acry_rsa_ctx_copy(struct aspeed_acry_dev *acry_dev, void *buf, enum aspeed_rsa_key_mode mode) { const u8 *src = xbuf; - u32 *dw_buf = (u32 *)buf; + __le32 *dw_buf = (__le32 *)buf; int nbits, ndw; int i, j, idx; u32 data = 0; @@ -302,7 +302,7 @@ static int aspeed_acry_rsa_ctx_copy(struct aspeed_acry_dev *acry_dev, void *buf, static int aspeed_acry_rsa_transfer(struct aspeed_acry_dev *acry_dev) { struct akcipher_request *req = acry_dev->req; - u8 *sram_buffer = (u8 *)acry_dev->acry_sram; + u8 __iomem *sram_buffer = (u8 __iomem *)acry_dev->acry_sram; struct scatterlist *out_sg = req->dst; static u8 dram_buffer[ASPEED_ACRY_SRAM_MAX_LEN]; int leading_zero = 1; @@ -321,11 +321,11 @@ static int aspeed_acry_rsa_transfer(struct aspeed_acry_dev *acry_dev) for (j = ASPEED_ACRY_SRAM_MAX_LEN - 1; j >= 0; j--) { data_idx = acry_dev->data_byte_mapping[j]; - if (sram_buffer[data_idx] == 0 && leading_zero) { + if (readb(sram_buffer + data_idx) == 0 && leading_zero) { result_nbytes--; } else { leading_zero = 0; - dram_buffer[i] = sram_buffer[data_idx]; + dram_buffer[i] = readb(sram_buffer + data_idx); i++; } }
This patch fixes following warnings: 1. sparse: incorrect type in assignment (different base types) Fix: change to __le32 type. 2. sparse: cast removes address space '__iomem' of expression Fix: use readb to avoid dereferencing the memory. Signed-off-by: Neal Liu <neal_liu@aspeedtech.com> --- Change since v1: keep iomem marker to remain its purpose. drivers/crypto/aspeed/aspeed-acry.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)