On Wed, Jun 30, 2021 at 3:32 PM kyrie.wu <kyrie.wu@xxxxxxxxxxxx> wrote: > Add MT8195 JPEG venc driver's compatible and device private data. > compatible = "mediatek,mt8195-jpgenc": this node would only register > jpgenc device node; > compatible = "mediatek,mt8195-jpgenc0": HW0 node, this node would not > register jpgenc device node, but register irq, init clk and power, > remmap register base and do other resource options; > compatible = "mediatek,mt8195-jpgenc1": HW1 node, just like HW0 node; The commit message is not easy to read. Please rephrase the sentences. What does "venc" stand for? I believe it is a copy-n-paste typo. The commit title "support MT8195 JPEG encoder" looks better to me. > -static const struct mtk_jpeg_variant mtk_jpeg_drvdata = { > +static const struct mtk_jpeg_variant mtk_jpegenc_drvdata = { Why remove mtk_jpeg_drvdata? > - .irq_handler = mtk_jpeg_enc_irq, Why remove the IRQ handler? > @@ -1548,10 +1551,6 @@ static const struct of_device_id mtk_jpeg_match[] = { > .compatible = "mediatek,mt2701-jpgdec", > .data = &mt8173_jpeg_drvdata, > }, > - { > - .compatible = "mediatek,mtk-jpgenc", > - .data = &mtk_jpeg_drvdata, > - }, Why remove "mediatek,mtk-jpgenc"? > +#if defined(CONFIG_OF) > +static const struct of_device_id mtk_jpegenc_hw_ids[] = { > + { > + .compatible = "mediatek,mt8195-jpgenc0", > + }, > + { .compatible = "mediatek,mt8195-jpgenc1", > + }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, mtk_jpegenc_hw_ids); > +#endif Would expect somewhere to reference mtk_jpegenc_hw_ids but failed to find it.