Re: [PATCH 01/11] rockchip: otp: Refactor to use driver data and ops
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
Reviewed-by: Kever Yang
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
#include
-#include
#include
#include
#include
+#include
#include
/* 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
[PATCH 01/11] rockchip: otp: Refactor to use driver data and ops
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
---
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
#include
-#include
#include
#include
#include
+#include
#include
/* 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
