[PATCH 01/11] rockchip: otp: Refactor to use driver data and ops

[Kever Yang]

Hi Jonas,

     Thanks for your patches.

On 2023/2/16 07:48, Jonas Karlman wrote:
> Refactor the driver to use driver data and ops to simplify handling
> of SoCs that require a unique read op.
>
> Use readl_poll_sleep_timeout instead of a custom poll loop, and add
> validation of input parameter to main read op.
>
> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
Reviewed-by: Kever Yang <kever.yang at rock-chips.com>

Thanks,
- Kever
> ---
>   drivers/misc/rockchip-otp.c | 83 +++++++++++++++++++------------------
>   1 file changed, 43 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/misc/rockchip-otp.c b/drivers/misc/rockchip-otp.c
> index cc9a5450e0ce..a6df0834ebbc 100644
> --- a/drivers/misc/rockchip-otp.c
> +++ b/drivers/misc/rockchip-otp.c
> @@ -5,10 +5,10 @@
>   
>   #include <common.h>
>   #include <asm/io.h>
> -#include <command.h>
>   #include <dm.h>
>   #include <linux/bitops.h>
>   #include <linux/delay.h>
> +#include <linux/iopoll.h>
>   #include <misc.h>
>   
>   /* OTP Register Offsets */
> @@ -49,35 +49,31 @@
>   
>   struct rockchip_otp_plat {
>   	void __iomem *base;
> -	unsigned long secure_conf_base;
> -	unsigned long otp_mask_base;
>   };
>   
> -static int rockchip_otp_wait_status(struct rockchip_otp_plat *otp,
> -				    u32 flag)
> +struct rockchip_otp_data {
> +	int (*read)(struct udevice *dev, int offset, void *buf, int size);
> +	int size;
> +};
> +
> +static int rockchip_otp_poll_timeout(struct rockchip_otp_plat *otp, u32 flag)
>   {
> -	int delay = OTPC_TIMEOUT;
> -
> -	while (!(readl(otp->base + OTPC_INT_STATUS) & flag)) {
> -		udelay(1);
> -		delay--;
> -		if (delay <= 0) {
> -			printf("%s: wait init status timeout\n", __func__);
> -			return -ETIMEDOUT;
> -		}
> -	}
> +	u32 status;
> +	int ret;
> +
> +	ret = readl_poll_sleep_timeout(otp->base + OTPC_INT_STATUS, status,
> +				       (status & flag), 1, OTPC_TIMEOUT);
> +	if (ret)
> +		return ret;
>   
> -	/* clean int status */
> +	/* Clear int flag */
>   	writel(flag, otp->base + OTPC_INT_STATUS);
>   
>   	return 0;
>   }
>   
> -static int rockchip_otp_ecc_enable(struct rockchip_otp_plat *otp,
> -				   bool enable)
> +static int rockchip_otp_ecc_enable(struct rockchip_otp_plat *otp, bool enable)
>   {
> -	int ret = 0;
> -
>   	writel(SBPI_DAP_ADDR_MASK | (SBPI_DAP_ADDR << SBPI_DAP_ADDR_SHIFT),
>   	       otp->base + OTPC_SBPI_CTRL);
>   
> @@ -92,11 +88,7 @@ static int rockchip_otp_ecc_enable(struct rockchip_otp_plat *otp,
>   
>   	writel(SBPI_ENABLE_MASK | SBPI_ENABLE, otp->base + OTPC_SBPI_CTRL);
>   
> -	ret = rockchip_otp_wait_status(otp, OTPC_SBPI_DONE);
> -	if (ret < 0)
> -		printf("%s timeout during ecc_enable\n", __func__);
> -
> -	return ret;
> +	return rockchip_otp_poll_timeout(otp, OTPC_SBPI_DONE);
>   }
>   
>   static int rockchip_px30_otp_read(struct udevice *dev, int offset,
> @@ -104,27 +96,24 @@ static int rockchip_px30_otp_read(struct udevice *dev, int offset,
>   {
>   	struct rockchip_otp_plat *otp = dev_get_plat(dev);
>   	u8 *buffer = buf;
> -	int ret = 0;
> +	int ret;
>   
>   	ret = rockchip_otp_ecc_enable(otp, false);
> -	if (ret < 0) {
> -		printf("%s rockchip_otp_ecc_enable err\n", __func__);
> +	if (ret)
>   		return ret;
> -	}
>   
>   	writel(OTPC_USE_USER | OTPC_USE_USER_MASK, otp->base + OTPC_USER_CTRL);
>   	udelay(5);
> +
>   	while (size--) {
>   		writel(offset++ | OTPC_USER_ADDR_MASK,
>   		       otp->base + OTPC_USER_ADDR);
>   		writel(OTPC_USER_FSM_ENABLE | OTPC_USER_FSM_ENABLE_MASK,
>   		       otp->base + OTPC_USER_ENABLE);
>   
> -		ret = rockchip_otp_wait_status(otp, OTPC_USER_DONE);
> -		if (ret < 0) {
> -			printf("%s timeout during read setup\n", __func__);
> +		ret = rockchip_otp_poll_timeout(otp, OTPC_USER_DONE);
> +		if (ret)
>   			goto read_end;
> -		}
>   
>   		*buffer++ = readb(otp->base + OTPC_USER_Q);
>   	}
> @@ -138,7 +127,16 @@ read_end:
>   static int rockchip_otp_read(struct udevice *dev, int offset,
>   			     void *buf, int size)
>   {
> -	return rockchip_px30_otp_read(dev, offset, buf, size);
> +	const struct rockchip_otp_data *data =
> +		(void *)dev_get_driver_data(dev);
> +
> +	if (offset < 0 || !buf || size <= 0 || offset + size > data->size)
> +		return -EINVAL;
> +
> +	if (!data->read)
> +		return -ENOSYS;
> +
> +	return data->read(dev, offset, buf, size);
>   }
>   
>   static const struct misc_ops rockchip_otp_ops = {
> @@ -147,21 +145,26 @@ static const struct misc_ops rockchip_otp_ops = {
>   
>   static int rockchip_otp_of_to_plat(struct udevice *dev)
>   {
> -	struct rockchip_otp_plat *otp = dev_get_plat(dev);
> +	struct rockchip_otp_plat *plat = dev_get_plat(dev);
>   
> -	otp->base = dev_read_addr_ptr(dev);
> +	plat->base = dev_read_addr_ptr(dev);
>   
>   	return 0;
>   }
>   
> +static const struct rockchip_otp_data px30_data = {
> +	.read = rockchip_px30_otp_read,
> +	.size = 0x40,
> +};
> +
>   static const struct udevice_id rockchip_otp_ids[] = {
>   	{
>   		.compatible = "rockchip,px30-otp",
> -		.data = (ulong)&rockchip_px30_otp_read,
> +		.data = (ulong)&px30_data,
>   	},
>   	{
>   		.compatible = "rockchip,rk3308-otp",
> -		.data = (ulong)&rockchip_px30_otp_read,
> +		.data = (ulong)&px30_data,
>   	},
>   	{}
>   };
> @@ -170,7 +173,7 @@ U_BOOT_DRIVER(rockchip_otp) = {
>   	.name = "rockchip_otp",
>   	.id = UCLASS_MISC,
>   	.of_match = rockchip_otp_ids,
> -	.ops = &rockchip_otp_ops,
>   	.of_to_plat = rockchip_otp_of_to_plat,
> -	.plat_auto	= sizeof(struct rockchip_otp_plat),
> +	.plat_auto = sizeof(struct rockchip_otp_plat),
> +	.ops = &rockchip_otp_ops,
>   };