linux-kernel - Re: [resend PATCH v6 12/12] drm/mediatek: Add support for mmsys through a pdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1575869682.7013.14.camel@mtksdaap41>
Date:   Mon, 9 Dec 2019 13:34:42 +0800
From:   CK Hu <ck.hu@...iatek.com>
To:     <matthias.bgg@...nel.org>
CC:     <robh+dt@...nel.org>, <mark.rutland@....com>,
        <p.zabel@...gutronix.de>, <airlied@...ux.ie>,
        <mturquette@...libre.com>, <sboyd@...nel.org>,
        <ulrich.hecht+renesas@...il.com>,
        <laurent.pinchart@...asonboard.com>,
        <enric.balletbo@...labora.com>, <sean.wang@...iatek.com>,
        <sean.wang@...nel.org>, <rdunlap@...radead.org>, <wens@...e.org>,
        <hsinyi@...omium.org>, <frank-w@...lic-files.de>,
        <drinkcat@...omium.org>, <linux-media@...r.kernel.org>,
        <dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-mediatek@...ts.infradead.org>, <linux-clk@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <mbrugger@...e.com>
Subject: Re: [resend PATCH v6 12/12] drm/mediatek: Add support for mmsys
 through a pdev

Hi, Matthias:

On Sat, 2019-12-07 at 23:47 +0100, matthias.bgg@...nel.org wrote:
> From: Matthias Brugger <mbrugger@...e.com>
> 
> The MMSYS subsystem includes clocks and drm components.
> This patch adds an initailization path through a platform device
> for the clock part, so that both drivers get probed from the same
> device tree compatible.

You've switched mt6779 and mt6797 clock driver to platform device
probing, but you does not probe then in drm driver, so your design is to
let mmsys clock not work if drm driver is not enabled? It's fine for me
because it seems that drm driver is the only user of mmsys clock now.

Regards,
CK

> 
> Signed-off-by: Matthias Brugger <mbrugger@...e.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 24 ++++++++++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h |  2 ++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 210455e9f46c..5ada74d8d0c9 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -186,6 +186,7 @@ static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
>  	.ext_path = mt2701_mtk_ddp_ext,
>  	.ext_len = ARRAY_SIZE(mt2701_mtk_ddp_ext),
>  	.shadow_register = true,
> +	.clk_drv_name = "clk-mt2701-mm",
>  };
>  
>  static const struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = {
> @@ -195,6 +196,7 @@ static const struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = {
>  	.ext_len = ARRAY_SIZE(mt2712_mtk_ddp_ext),
>  	.third_path = mt2712_mtk_ddp_third,
>  	.third_len = ARRAY_SIZE(mt2712_mtk_ddp_third),
> +	.clk_drv_name = "clk-mt2712-mm",
>  };
>  
>  static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
> @@ -202,6 +204,7 @@ static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
>  	.main_len = ARRAY_SIZE(mt8173_mtk_ddp_main),
>  	.ext_path = mt8173_mtk_ddp_ext,
>  	.ext_len = ARRAY_SIZE(mt8173_mtk_ddp_ext),
> +	.clk_drv_name = "clk-mt8173-mm",
>  };
>  
>  static int mtk_drm_kms_init(struct drm_device *drm)
> @@ -499,6 +502,24 @@ static int mtk_drm_probe(struct platform_device *pdev)
>  	INIT_WORK(&private->commit.work, mtk_atomic_work);
>  	private->data = of_device_get_match_data(dev);
>  
> +	/*
> +	 * MMSYS includes apart from components management a block providing
> +	 * clocks for the subsystem. We probe this clock driver via a platform
> +	 * device.
> +	 */
> +	if (private->data->clk_drv_name) {
> +		private->clk_dev = platform_device_register_data(dev,
> +						private->data->clk_drv_name, -1,
> +						NULL, 0);
> +
> +		if (IS_ERR(private->clk_dev)) {
> +			dev_err(dev, "failed to register %s platform device\n",
> +				private->data->clk_drv_name);
> +
> +			return PTR_ERR(private->clk_dev);
> +		}
> +	}
> +
>  	private->config_regs = syscon_node_to_regmap(dev->of_node);
>  	if (IS_ERR(private->config_regs))
>  		return PTR_ERR(private->config_regs);
> @@ -605,6 +626,9 @@ static int mtk_drm_remove(struct platform_device *pdev)
>  	for (i = 0; i < DDP_COMPONENT_ID_MAX; i++)
>  		of_node_put(private->comp_node[i]);
>  
> +	if (private->clk_dev)
> +		platform_device_unregister(private->clk_dev);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index 63a121577dcb..8fe9136adc38 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -29,11 +29,13 @@ struct mtk_mmsys_driver_data {
>  	unsigned int third_len;
>  
>  	bool shadow_register;
> +	const char *clk_drv_name;
>  };
>  
>  struct mtk_drm_private {
>  	struct drm_device *drm;
>  	struct device *dma_dev;
> +	struct platform_device *clk_dev;
>  
>  	unsigned int num_pipes;
>