Hi Jianhua, > + ret = of_property_read_u32(pdev->dev.of_node, "mediatek,34bits", > + &jpeg->support_34bit); > + if (ret != 0) { > + dev_info(&pdev->dev, "default for 32bits"); > + jpeg->support_34bit = 0; > + } > + dev_info(&pdev->dev, "use 34bits: %d", jpeg->support_34bit); > + Please use of_property_read_bool instead. > enc_get_file_size(void __iomem *base, u32 support_34bit) > { > - return readl(base + JPEG_ENC_DMA_ADDR0) - > + u32 value = 1; > + > + if (support_34bit) > + value = 4; > + > + return readl(base + JPEG_ENC_DMA_ADDR0) * value - > readl(base + JPEG_ENC_DST_ADDR0); > } Please use the ternary operator instead, there’s no need for two assignments. Can you please add a comment inline to explain why this multiplier is needed? > EXPORT_SYMBOL_GPL(mtk_jpeg_enc_get_file_size); > @@ -75,6 +80,9 @@ void mtk_jpeg_enc_start(void __iomem *base) > > value = readl(base + JPEG_ENC_CTRL); > value |= JPEG_ENC_CTRL_INT_EN_BIT | JPEG_ENC_CTRL_ENABLE_BIT; > + value |= JPEG_ENC_CTRL_RDMA_PADDING_EN; > + value |= JPEG_ENC_CTRL_RDMA_RIGHT_PADDING_EN; > + value &= ~JPEG_ENC_CTRL_RDMA_PADDING_0_EN; > writel(value, base + JPEG_ENC_CTRL); > } These do not have to be gated by “support_34bit” ? — Daniel