<html dir="ltr"><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="text-align:left; direction:ltr;"><div>Hi Steve,</div><div><br></div><div>Thanks for your suggestion.</div><div>On Wed, 2021-12-01 at 15:36 -0800, Steve Cho wrote:</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div dir="ltr">LGTM with few nits. </div><div dir="ltr"><br clear="all"><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr">Thanks,<div>Steve</div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Nov 28, 2021 at 7:44 PM Yunfei Dong <<a href="mailto:yunfei.dong@mediatek.com">yunfei.dong@mediatek.com</a>> wrote:<br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">Different platform may has different numbers of register bases. Gets the<br>
numbers of register bases from DT (sizeof(u32) * 4 bytes for each)<br></blockquote><div>Few nits.</div><div>s/platform/platforms/</div><div>s/has/have/</div><div><br></div><div>Fix, DT is dts.</div><div>Btw, what is DT?</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"> <br>
Reviewed-by: Tzung-Bi Shih<<a href="mailto:tzungbi@google.com" target="_blank">tzungbi@google.com</a>><br>
Signed-off-by: Yunfei Dong <<a href="mailto:yunfei.dong@mediatek.com" target="_blank">yunfei.dong@mediatek.com</a>><br>
---<br>
.../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 37 ++++++++++++++-----<br>
1 file changed, 28 insertions(+), 9 deletions(-)<br>
<br>
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c<br>
index e6e6a8203eeb..59caf2163349 100644<br>
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c<br>
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c<br>
@@ -78,6 +78,30 @@ static irqreturn_t mtk_vcodec_dec_irq_handler(int irq, void *priv)<br>
return IRQ_HANDLED;<br>
}<br>
<br>
+static int mtk_vcodec_get_reg_bases(struct mtk_vcodec_dev *dev)<br>
+{<br></blockquote><div>I see that dev is already checked before entering into this function, but null check for dev would still be nice. </div><div> </div></div></div></blockquote><div>Dev is never null in this function, whether it looks not very reasonable?</div><div><br></div><div>Best Regards,</div><div>Yunfei Dong</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">
+ struct platform_device *pdev = dev->plat_dev;<br>
+ int reg_num, i;<br>
+<br>
+ /* Sizeof(u32) * 4 bytes for each register base. */<br>
+ reg_num = of_property_count_elems_of_size(pdev->dev.of_node, "reg",<br>
+ sizeof(u32) * 4);<br>
+ if (reg_num <= 0 || reg_num > NUM_MAX_VDEC_REG_BASE) {<br>
+ dev_err(&pdev->dev, "Invalid register property size: %d\n", reg_num);<br>
+ return -EINVAL;<br>
+ }<br>
+<br>
+ for (i = 0; i < reg_num; i++) {<br>
+ dev->reg_base[i] = devm_platform_ioremap_resource(pdev, i);<br>
+ if (IS_ERR(dev->reg_base[i]))<br>
+ return PTR_ERR(dev->reg_base[i]);<br>
+<br>
+ mtk_v4l2_debug(2, "reg[%d] base=%p", i, dev->reg_base[i]);<br>
+ }<br>
+<br>
+ return 0;<br>
+}<br>
+<br>
static int fops_vcodec_open(struct file *file)<br>
{<br>
struct mtk_vcodec_dev *dev = video_drvdata(file);<br>
@@ -206,7 +230,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)<br>
struct resource *res;<br>
phandle rproc_phandle;<br>
enum mtk_vcodec_fw_type fw_type;<br>
- int i, ret;<br>
+ int ret;<br>
<br>
dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);<br>
if (!dev)<br>
@@ -238,14 +262,9 @@ static int mtk_vcodec_probe(struct platform_device *pdev)<br>
goto err_dec_pm;<br>
}<br>
<br>
- for (i = 0; i < NUM_MAX_VDEC_REG_BASE; i++) {<br>
- dev->reg_base[i] = devm_platform_ioremap_resource(pdev, i);<br>
- if (IS_ERR((__force void *)dev->reg_base[i])) {<br>
- ret = PTR_ERR((__force void *)dev->reg_base[i]);<br>
- goto err_res;<br>
- }<br>
- mtk_v4l2_debug(2, "reg[%d] base=%p", i, dev->reg_base[i]);<br>
- }<br>
+ ret = mtk_vcodec_get_reg_bases(dev);<br>
+ if (ret)<br>
+ goto err_res;<br>
<br>
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);<br>
if (res == NULL) {<br>
</blockquote></div></div></blockquote></body></html>