[<prev] [next>] [day] [month] [year] [list]
Message-ID: <8bfc4350-6e92-e657-18f2-3624a2558521@collabora.com>
Date: Fri, 6 Mar 2020 17:30:16 +0100
From: Enric Balletbo i Serra <enric.balletbo@...labora.com>
To: Stephen Boyd <sboyd@...nel.org>, airlied@...ux.ie,
ck.hu@...iatek.com, laurent.pinchart@...asonboard.com,
mark.rutland@....com, mturquette@...libre.com,
p.zabel@...gutronix.de, robh+dt@...nel.org,
ulrich.hecht+renesas@...il.com
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>, rdunlap@...radead.org,
dri-devel@...ts.freedesktop.org, Weiyi Lu <weiyi.lu@...iatek.com>,
Seiya Wang <seiya.wang@...iatek.com>,
linux-clk@...r.kernel.org,
Collabora Kernel ML <kernel@...labora.com>,
mtk01761 <wendell.lin@...iatek.com>,
Allison Randal <allison@...utok.net>,
Thomas Gleixner <tglx@...utronix.de>, wens@...e.org,
Kate Stewart <kstewart@...uxfoundation.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Houlong Wei <houlong.wei@...iatek.com>,
Matthias Brugger <matthias.bgg@...il.com>,
linux-media@...r.kernel.org, devicetree@...r.kernel.org,
sean.wang@...iatek.com, frank-w@...lic-files.de,
Minghsiu Tsai <minghsiu.tsai@...iatek.com>,
Andrew-CT Chen <andrew-ct.chen@...iatek.com>,
linux-mediatek@...ts.infradead.org, hsinyi@...omium.org,
Matthias Brugger <mbrugger@...e.com>,
linux-arm-kernel@...ts.infradead.org,
Richard Fontana <rfontana@...hat.com>,
linux-kernel@...r.kernel.org, matthias.bgg@...nel.org,
Daniel Vetter <daniel@...ll.ch>
Subject: Re: [PATCH v11 3/5] soc: mediatek: Move mt8173 MMSYS to platform
driver
Hi Stephen,
On 5/3/20 22:01, Stephen Boyd wrote:
> Quoting Enric Balletbo i Serra (2020-03-02 03:01:26)
>> From: Matthias Brugger <mbrugger@...e.com>
>>
>> There is no strong reason for this to use CLK_OF_DECLARE instead of
>> being a platform driver.
>
> Cool.
>
>> Plus, this driver provides clocks but also
>> a shared register space for the mediatek-drm and the mediatek-mdp
>> driver. So move to drivers/soc/mediatek as a platform driver.
>>
>> Signed-off-by: Matthias Brugger <mbrugger@...e.com>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@...labora.com>
>> Reviewed-by: CK Hu <ck.hu@...iatek.com>
>> ---
>>
>> Changes in v11: None
>> Changes in v10:
>> - Renamed to be generic mtk-mmsys
>> - Add driver data support to be able to support diferent SoCs
>>
>> Changes in v9:
>> - Move mmsys to drivers/soc/mediatek (CK)
>>
>> Changes in v8:
>> - Be a builtin_platform_driver like other mediatek mmsys drivers.
>>
>> Changes in v7:
>> - Free clk_data->clks as well
>> - Get rid of private data structure
>>
>> drivers/clk/mediatek/clk-mt8173.c | 104 --------------------
>> drivers/soc/mediatek/Kconfig | 7 ++
>> drivers/soc/mediatek/Makefile | 1 +
>> drivers/soc/mediatek/mtk-mmsys.c | 154 ++++++++++++++++++++++++++++++
>
> Can you generate with -M so that we can see what has actually changed?
>
Sure, sorry about that.
>> 4 files changed, 162 insertions(+), 104 deletions(-)
>> create mode 100644 drivers/soc/mediatek/mtk-mmsys.c
>>
>> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
>> index 2114b563478c..7a156944d50e 100644
>> --- a/drivers/soc/mediatek/Kconfig
>> +++ b/drivers/soc/mediatek/Kconfig
>> @@ -44,4 +44,11 @@ config MTK_SCPSYS
>> Say yes here to add support for the MediaTek SCPSYS power domain
>> driver.
>>
>> +config MTK_MMSYS
>> + bool "MediaTek MMSYS Support"
>> + depends on COMMON_CLK_MT8173
>
> Does it need some default so that defconfig updates don't break things?
>
Right.
>> + help
>> + Say yes here to add support for the MediaTek Multimedia
>> + Subsystem (MMSYS).
>> +
>> endmenu
>> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
>> index b01733074ad6..01f9f873634a 100644
>> --- a/drivers/soc/mediatek/Makefile
>> +++ b/drivers/soc/mediatek/Makefile
>> @@ -3,3 +3,4 @@ obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o
>> obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
>> obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
>> obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
>> +obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
>> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
>> new file mode 100644
>> index 000000000000..473cdf732fb5
>> --- /dev/null
>> +++ b/drivers/soc/mediatek/mtk-mmsys.c
>> @@ -0,0 +1,154 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2014 MediaTek Inc.
>> + * Author: James Liao <jamesjj.liao@...iatek.com>
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "../../clk/mediatek/clk-gate.h"
>> +#include "../../clk/mediatek/clk-mtk.h"
>
> Why not use include/linux/clk/?
>
I can move these files to include, this will impact a lot more of drivers but,
yes, I think is the right way.
> But I also don't understand why the clk driver is moved outside of
> drivers/clk/ into drivers/soc/. Commit text saying that it has shared
> registers doesn't mean it can't still keep the clk driver part in the
> drivers/clk/ area.
>
Actually moving this to the soc directory has been requested by CK (mediatek) as
a change in v8. You can see the discussion in [1]
Thanks,
Enric
[1] https://patchwork.kernel.org/cover/11394709/
>> +
>> +#include <dt-bindings/clock/mt8173-clk.h>
>> +
>> +static const struct mtk_gate_regs mm0_cg_regs = {
>> + .set_ofs = 0x0104,
>> + .clr_ofs = 0x0108,
>> + .sta_ofs = 0x0100,
>> +};
>> +
>> +static const struct mtk_gate_regs mm1_cg_regs = {
>> + .set_ofs = 0x0114,
>> + .clr_ofs = 0x0118,
>> + .sta_ofs = 0x0110,
>> +};
Powered by blists - more mailing lists