From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Thu, 15 Oct 2020 09:05:40 -0600
Subject: [PATCH 2/3] rockchip: efuse: add support for RK3328 non-secure
 efuse
In-Reply-To: <20201013202116.24850-3-jonas@kwiboo.se>
References: <20201013202116.24850-1-jonas@kwiboo.se>
 <20201013202116.24850-3-jonas@kwiboo.se>
Message-ID: <CAPnjgZ2GD16oqUP2K3CWvkormNXMs7q0idEr2dkt5KJ2OFgYSg@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Jonas,

On Tue, 13 Oct 2020 at 14:21, Jonas Karlman <jonas@kwiboo.se> wrote:
>
> From: Joseph Chen <chenjh@rock-chips.com>
>
> Extend rockchip efuse driver with support for RK3328 non-secure efuse.
>
> Signed-off-by: Joseph Chen <chenjh@rock-chips.com>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>  drivers/misc/rockchip-efuse.c | 67 +++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
>
> diff --git a/drivers/misc/rockchip-efuse.c b/drivers/misc/rockchip-efuse.c
> index 20423544d9..9b5a525322 100644
> --- a/drivers/misc/rockchip-efuse.c
> +++ b/drivers/misc/rockchip-efuse.c
> @@ -13,6 +13,7 @@
>  #include <dm.h>
>  #include <linux/bitops.h>
>  #include <linux/delay.h>
> +#include <malloc.h>
>  #include <misc.h>
>
>  #define RK3399_A_SHIFT          16
> @@ -36,6 +37,13 @@
>  #define RK3288_STROBE           BIT(1)
>  #define RK3288_CSB              BIT(0)
>
> +#define RK3328_INT_STATUS      0x0018
> +#define RK3328_DOUT            0x0020
> +#define RK3328_AUTO_CTRL       0x0024
> +#define RK3328_INT_FINISH      BIT(0)
> +#define RK3328_AUTO_ENB                BIT(0)
> +#define RK3328_AUTO_RD         BIT(1)
> +
>  typedef int (*EFUSE_READ)(struct udevice *dev, int offset, void *buf, int size);
>
>  struct rockchip_efuse_regs {
> @@ -46,6 +54,10 @@ struct rockchip_efuse_regs {
>         u32 jtag_pass; /* 0x10  JTAG password */
>         u32 strobe_finish_ctrl;
>                        /* 0x14  efuse strobe finish control register */
> +       u32 int_status;/* 0x18 */
> +       u32 reserved;  /* 0x1c */
> +       u32 dout2;     /* 0x20 */
> +       u32 auto_ctrl; /* 0x24 */
>  };
>
>  struct rockchip_efuse_platdata {
> @@ -181,6 +193,57 @@ static int rockchip_rk3288_efuse_read(struct udevice *dev, int offset,
>         return 0;
>  }
>
> +static int rockchip_rk3328_efuse_read(struct udevice *dev, int offset,
> +                                     void *buf, int size)

Is this a completely different algorithm from rk3399, or just
different parameters?

> +{
> +       struct rockchip_efuse_platdata *plat = dev_get_platdata(dev);
> +       struct rockchip_efuse_regs *efuse =
> +               (struct rockchip_efuse_regs *)plat->base;
> +       unsigned int addr_start, addr_end, addr_offset, addr_len;
> +       u32 out_value, status;
> +       u8 *buffer;
> +       int ret = 0, i = 0, j = 0;

Don't init unless needed.

> +
> +       /* Max non-secure Byte */
> +       if (size > 32)
> +               size = 32;
> +
> +       /* 128 Byte efuse, 96 Byte for secure, 32 Byte for non-secure */
> +       offset += 96;
> +       addr_start = rounddown(offset, RK3399_BYTES_PER_FUSE) /
> +                                               RK3399_BYTES_PER_FUSE;
> +       addr_end = roundup(offset + size, RK3399_BYTES_PER_FUSE) /
> +                                               RK3399_BYTES_PER_FUSE;
> +       addr_offset = offset % RK3399_BYTES_PER_FUSE;
> +       addr_len = addr_end - addr_start;
> +
> +       buffer = calloc(1, sizeof(*buffer) * addr_len * RK3399_BYTES_PER_FUSE);
> +       if (!buffer)
> +               return -ENOMEM;
> +
> +       for (j = 0; j < addr_len; j++) {
> +               writel(RK3328_AUTO_RD | RK3328_AUTO_ENB |
> +                      ((addr_start++ & RK3399_A_MASK) << RK3399_A_SHIFT),

Move addr_start to end of loop

> +                      &efuse->auto_ctrl);
> +               udelay(5);
> +               status = readl(&efuse->int_status);
> +               if (!(status & RK3328_INT_FINISH)) {
> +                       ret = -EIO;
> +                       goto err;
> +               }
> +               out_value = readl(&efuse->dout2);
> +               writel(RK3328_INT_FINISH, &efuse->int_status);
> +
> +               memcpy(&buffer[i], &out_value, RK3399_BYTES_PER_FUSE);
> +               i += RK3399_BYTES_PER_FUSE;

Please use a proper descriptive variable name instead of i.

> +       }
> +       memcpy(buf, buffer + addr_offset, size);
> +err:
> +       free(buffer);
> +
> +       return ret;
> +}
> +
>  static int rockchip_efuse_read(struct udevice *dev, int offset,
>                                void *buf, int size)
>  {
> @@ -226,6 +289,10 @@ static const struct udevice_id rockchip_efuse_ids[] = {
>                 .compatible = "rockchip,rk3368-efuse",
>                 .data = (ulong)&rockchip_rk3288_efuse_read,
>         },
> +       {
> +               .compatible = "rockchip,rk3328-efuse",
> +               .data = (ulong)&rockchip_rk3328_efuse_read,
> +       },
>         {
>                 .compatible = "rockchip,rk3399-efuse",
>                 .data = (ulong)&rockchip_rk3399_efuse_read,
> --
> 2.17.1
>

Regards,
Simon