On Mon, 27 Sept 2021 at 15:59, Eddie James <eajames@xxxxxxxxxxxxx> wrote:
>
> Allocate a large buffer for each OCC to handle response data. This
> removes memory allocation during an operation, and also allows for
> the maximum amount of SBE FFDC.
Why do we need this change? (is it fixing a bug, did the host change,
is it an unimplemented feature, etc)
>
> Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx>
> ---
> drivers/fsi/fsi-occ.c | 109 ++++++++++++++++------------------------
> include/linux/fsi-occ.h | 2 +
> 2 files changed, 45 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c
> index b0c9322078a1..ace3ec7767e5 100644
> --- a/drivers/fsi/fsi-occ.c
> +++ b/drivers/fsi/fsi-occ.c
> @@ -10,6 +10,7 @@
> #include <linux/kernel.h>
> #include <linux/list.h>
> #include <linux/miscdevice.h>
> +#include <linux/mm.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/fsi-occ.h>
> @@ -42,13 +43,6 @@
>
> #define OCC_P10_SRAM_MODE 0x58 /* Normal mode, OCB channel 2 */
>
> -/*
> - * Assume we don't have much FFDC, if we do we'll overflow and
> - * fail the command. This needs to be big enough for simple
> - * commands as well.
> - */
> -#define OCC_SBE_STATUS_WORDS 32
> -
> #define OCC_TIMEOUT_MS 1000
> #define OCC_CMD_IN_PRG_WAIT_MS 50
>
> @@ -60,6 +54,7 @@ struct occ {
> char name[32];
> int idx;
> u8 sequence_number;
> + void *buffer;
> enum versions version;
> struct miscdevice mdev;
> struct mutex occ_lock;
> @@ -250,8 +245,10 @@ static int occ_verify_checksum(struct occ *occ, struct occ_response *resp,
> static int occ_getsram(struct occ *occ, u32 offset, void *data, ssize_t len)
> {
> u32 data_len = ((len + 7) / 8) * 8; /* must be multiples of 8 B */
> - size_t cmd_len, resp_len, resp_data_len;
> - __be32 *resp, cmd[6];
> + size_t cmd_len, resp_data_len;
> + size_t resp_len = OCC_MAX_RESP_WORDS;
> + __be32 *resp = occ->buffer;
> + __be32 cmd[6];
> int idx = 0, rc;
>
> /*
> @@ -278,19 +275,19 @@ static int occ_getsram(struct occ *occ, u32 offset, void *data, ssize_t len)
> cmd[1] = cpu_to_be32(SBEFIFO_CMD_GET_OCC_SRAM);
> cmd[4 + idx] = cpu_to_be32(data_len);
>
> - resp_len = (data_len >> 2) + OCC_SBE_STATUS_WORDS;
> - resp = kzalloc(resp_len << 2, GFP_KERNEL);
Previously the driver would zero the buffer before using it. Should
you add a memset here?
> @@ -635,6 +605,10 @@ static int occ_probe(struct platform_device *pdev)
> if (!occ)
> return -ENOMEM;
>
> + occ->buffer = kvmalloc(OCC_MAX_RESP_WORDS * 4, GFP_KERNEL);
Why do you allocate WORDS * 4?
> diff --git a/include/linux/fsi-occ.h b/include/linux/fsi-occ.h
> index d4cdc2aa6e33..7ee3dbd7f4b3 100644
> --- a/include/linux/fsi-occ.h
> +++ b/include/linux/fsi-occ.h
> @@ -19,6 +19,8 @@ struct device;
> #define OCC_RESP_CRIT_OCB 0xE3
> #define OCC_RESP_CRIT_HW 0xE4
>
> +#define OCC_MAX_RESP_WORDS 2048
Does this need to go in the header?
