[V13,3/7] mtk-jpegenc: support jpegenc multi-hardware

[kyrie.wu]

On Fri, 2022-08-26 at 11:45 +0200, Christophe JAILLET wrote:
> Le 26/08/2022 à 11:29, Irui Wang a écrit :
> > From: kyrie wu <kyrie.wu-NuS5LvNUpcJWk0Htik3J/w at public.gmane.org>
> > 
> > support jpeg encode multi-hardware includes HW0 and HW1.
> > 
> > Signed-off-by: kyrie wu <
> > kyrie.wu-NuS5LvNUpcJWk0Htik3J/w at public.gmane.org>
> > Signed-off-by: irui wang <
> > irui.wang-NuS5LvNUpcJWk0Htik3J/w at public.gmane.org>
> > ---
> >   drivers/media/platform/mediatek/jpeg/Makefile |  11 +-
> >   .../platform/mediatek/jpeg/mtk_jpeg_core.c    |  67 +++----
> >   .../platform/mediatek/jpeg/mtk_jpeg_core.h    |  37 ++++
> >   .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  | 163
> > ++++++++++++++++++
> >   4 files changed, 245 insertions(+), 33 deletions(-)
> 
> [...]
> 
> > static int mtk_jpegenc_hw_init_irq(struct mtk_jpegenc_comp_dev
> > *dev)
> > +{
> > +	struct platform_device *pdev = dev->plat_dev;
> > +	int ret;
> > +
> > +	dev->jpegenc_irq = platform_get_irq(pdev, 0);
> > +	if (dev->jpegenc_irq < 0) {
> > +		dev_err(&pdev->dev, "Failed to get irq resource");
> 
> Hi,
> 
> Should there be a v14, this dev_err() is useless.
> platform_get_irq() already prints a more informative message.

Hi Christophe,

thanks for your introduction about platform_get_irq(),
I will remove it.
> 
> > +		return dev->jpegenc_irq;
> > +	}
> > +
> > +	ret = devm_request_irq(&pdev->dev,
> > +			       dev->jpegenc_irq,
> > +			       mtk_jpegenc_hw_irq_handler,
> > +			       0,
> > +			       pdev->name, dev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to devm_request_irq %d
> > (%d)",
> > +			dev->jpegenc_irq, ret);
> > +		return -ENOENT;
> 
> Why not propagate 'ret' here?

yeah, kindly suggestion, maybe ret contains enough messages for this
error.
> 
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
> > +{
> > +	struct mtk_jpegenc_clk *jpegenc_clk;
> > +	struct mtk_jpeg_dev *master_dev;
> > +	struct mtk_jpegenc_comp_dev *dev;
> > +	int ret, i;
> > +
> > +	struct device *decs = &pdev->dev;
> > +
> > +	if (!decs->parent)
> > +		return -EPROBE_DEFER;
> > +
> > +	master_dev = dev_get_drvdata(decs->parent);
> > +	if (!master_dev)
> > +		return -EPROBE_DEFER;
> > +
> > +	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
> > +	if (!dev)
> > +		return -ENOMEM;
> > +
> > +	dev->plat_dev = pdev;
> > +	dev->dev = &pdev->dev;
> > +
> > +	if (!master_dev->is_jpgenc_multihw) {
> > +		master_dev->is_jpgenc_multihw = true;
> > +		for (i = 0; i < MTK_JPEGENC_HW_MAX; i++)
> > +			master_dev->enc_hw_dev[i] = NULL;
> > +	}
> > +
> > +	jpegenc_clk = &dev->venc_clk;
> > +
> > +	jpegenc_clk->clk_num = devm_clk_bulk_get_all(&pdev->dev,
> > +						     &jpegenc_clk-
> > >clks);
> > +	if (jpegenc_clk->clk_num < 0)
> > +		return dev_err_probe(&pdev->dev, jpegenc_clk->clk_num,
> > +				     "Failed to get jpegenc clock
> > count\n");
> > +
> > +	dev->reg_base =
> > +		devm_platform_ioremap_resource(pdev, 0);
> 
> This can be written on 1 line only.
> 
> just my 2c,
> 
> CJ

OK, I will update this item.

Thanks.

Regards,
Kyire.
> 
> > +	if (IS_ERR(dev->reg_base))
> > +		return PTR_ERR(dev->reg_base);
> > +
> > +	ret = mtk_jpegenc_hw_init_irq(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	for (i = 0; i < MTK_JPEGENC_HW_MAX; i++) {
> > +		if (master_dev->enc_hw_dev[i])
> > +			continue;
> > +
> > +		master_dev->enc_hw_dev[i] = dev;
> > +		master_dev->reg_encbase[i] = dev->reg_base;
> > +		dev->master_dev = master_dev;
> > +	}
> > +
> > +	platform_set_drvdata(pdev, dev);
> > +	pm_runtime_enable(&pdev->dev);
> > +
> > +	return 0;
> > +}
> > +
> > +struct platform_driver mtk_jpegenc_hw_driver = {
> > +	.probe = mtk_jpegenc_hw_probe,
> > +	.driver = {
> > +		.name = "mtk-jpegenc-hw",
> > +		.of_match_table = of_match_ptr(mtk_jpegenc_drv_ids),
> > +	},
> > +};
> > +
> > +module_platform_driver(mtk_jpegenc_hw_driver);
> > +
> > +MODULE_DESCRIPTION("MediaTek JPEG encode HW driver");
> > +MODULE_LICENSE("GPL");
> 
>