| Message ID | 20210616224743.5109-1-chun-jie.chen@mediatek.com |
|---|---|
| Headers | show |
| Series | Mediatek MT8195 clock support | expand |
On Thu, Jun 17, 2021 at 06:47:22AM +0800, Chun-Jie Chen wrote: > This patch adds the new binding documentation for system clock > and functional clock on Mediatek MT8195. > > Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> > --- > .../arm/mediatek/mediatek,mt8195-clock.yaml | 287 ++++++++++++++++++ > .../mediatek/mediatek,mt8195-sys-clock.yaml | 66 ++++ > 2 files changed, 353 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-clock.yaml > create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-sys-clock.yaml > > diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-clock.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-clock.yaml > new file mode 100644 > index 000000000000..21554b3515cf > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-clock.yaml > @@ -0,0 +1,287 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/arm/mediatek/mediatek,mt8195-clock.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: MediaTek Functional Clock Controller for MT8195 > + > +maintainers: > + - Chun-Jie Chen <chun-jie.chen@mediatek.com> > + > +description: > + The Mediatek functional clock controller provides various clocks on MT8195. > + > +properties: > + compatible: > + oneOf: > + - items: > + - enum: Drop 'oneOf' and 'items'. > + - mediatek,mt8195-nnasys > + - mediatek,mt8195-scp_adsp > + - mediatek,mt8195-audsys > + - mediatek,mt8195-audsys_src > + - mediatek,mt8195-imp_iic_wrap_s > + - mediatek,mt8195-imp_iic_wrap_w > + - mediatek,mt8195-mfgcfg > + - mediatek,mt8195-vppsys0 > + - mediatek,mt8195-wpesys > + - mediatek,mt8195-wpesys_vpp0 > + - mediatek,mt8195-wpesys_vpp1 > + - mediatek,mt8195-vppsys1 > + - mediatek,mt8195-imgsys > + - mediatek,mt8195-imgsys1_dip_top > + - mediatek,mt8195-imgsys1_dip_nr > + - mediatek,mt8195-imgsys1_wpe > + - mediatek,mt8195-ipesys > + - mediatek,mt8195-camsys > + - mediatek,mt8195-camsys_rawa > + - mediatek,mt8195-camsys_yuva > + - mediatek,mt8195-camsys_rawb > + - mediatek,mt8195-camsys_yuvb > + - mediatek,mt8195-camsys_mraw > + - mediatek,mt8195-ccusys > + - mediatek,mt8195-vdecsys_soc > + - mediatek,mt8195-vdecsys > + - mediatek,mt8195-vdecsys_core1 > + - mediatek,mt8195-apusys_pll > + - mediatek,mt8195-vencsys > + - mediatek,mt8195-vencsys_core1 > + - mediatek,mt8195-vdosys0 > + - mediatek,mt8195-vdosys1 > + reg: > + maxItems: 1 > + > + '#clock-cells': > + const: 1 > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + nnasys: clock-controller@10211000 { > + compatible = "mediatek,mt8195-nnasys"; > + reg = <0x10211000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + scp_adsp: clock-controller@10720000 { > + compatible = "mediatek,mt8195-scp_adsp"; > + reg = <0x10720000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + audsys: clock-controller@10890000 { > + compatible = "mediatek,mt8195-audsys"; > + reg = <0x10890000 0x10000>; > + #clock-cells = <1>; > + }; > + > + - | > + audsys_src: clock-controller@108a0000 { > + compatible = "mediatek,mt8195-audsys_src"; > + reg = <0x108a0000 0x2000>; > + #clock-cells = <1>; > + }; > + > + - | > + imp_iic_wrap_s: clock-controller@11d03000 { > + compatible = "mediatek,mt8195-imp_iic_wrap_s"; > + reg = <0x11d03000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + imp_iic_wrap_w: clock-controller@11e05000 { > + compatible = "mediatek,mt8195-imp_iic_wrap_w"; > + reg = <0x11e05000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + mfgcfg: clock-controller@13fbf000 { > + compatible = "mediatek,mt8195-mfgcfg"; > + reg = <0x13fbf000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vppsys0: clock-controller@14000000 { > + compatible = "mediatek,mt8195-vppsys0"; > + reg = <0x14000000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + wpesys: clock-controller@14e00000 { > + compatible = "mediatek,mt8195-wpesys"; > + reg = <0x14e00000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + wpesys_vpp0: clock-controller@14e02000 { > + compatible = "mediatek,mt8195-wpesys_vpp0"; > + reg = <0x14e02000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + wpesys_vpp1: clock-controller@14e03000 { > + compatible = "mediatek,mt8195-wpesys_vpp1"; > + reg = <0x14e03000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vppsys1: clock-controller@14f00000 { > + compatible = "mediatek,mt8195-vppsys1"; > + reg = <0x14f00000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + imgsys: clock-controller@15000000 { > + compatible = "mediatek,mt8195-imgsys"; > + reg = <0x15000000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + imgsys1_dip_top: clock-controller@15110000 { > + compatible = "mediatek,mt8195-imgsys1_dip_top"; > + reg = <0x15110000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + imgsys1_dip_nr: clock-controller@15130000 { > + compatible = "mediatek,mt8195-imgsys1_dip_nr"; > + reg = <0x15130000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + imgsys1_wpe: clock-controller@15220000 { > + compatible = "mediatek,mt8195-imgsys1_wpe"; > + reg = <0x15220000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + ipesys: clock-controller@15330000 { > + compatible = "mediatek,mt8195-ipesys"; > + reg = <0x15330000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + camsys: clock-controller@16000000 { > + compatible = "mediatek,mt8195-camsys"; > + reg = <0x16000000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + camsys_rawa: clock-controller@1604f000 { > + compatible = "mediatek,mt8195-camsys_rawa"; > + reg = <0x1604f000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + camsys_yuva: clock-controller@1606f000 { > + compatible = "mediatek,mt8195-camsys_yuva"; > + reg = <0x1606f000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + camsys_rawb: clock-controller@1608f000 { > + compatible = "mediatek,mt8195-camsys_rawb"; > + reg = <0x1608f000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + camsys_yuvb: clock-controller@160af000 { > + compatible = "mediatek,mt8195-camsys_yuvb"; > + reg = <0x160af000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + camsys_mraw: clock-controller@16140000 { > + compatible = "mediatek,mt8195-camsys_mraw"; > + reg = <0x16140000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + ccusys: clock-controller@17200000 { > + compatible = "mediatek,mt8195-ccusys"; > + reg = <0x17200000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vdecsys_soc: clock-controller@1800f000 { > + compatible = "mediatek,mt8195-vdecsys_soc"; > + reg = <0x1800f000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vdecsys: clock-controller@1802f000 { > + compatible = "mediatek,mt8195-vdecsys"; > + reg = <0x1802f000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vdecsys_core1: clock-controller@1803f000 { > + compatible = "mediatek,mt8195-vdecsys_core1"; > + reg = <0x1803f000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + apusys_pll: clock-controller@190f3000 { > + compatible = "mediatek,mt8195-apusys_pll"; > + reg = <0x190f3000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vencsys: clock-controller@1a000000 { > + compatible = "mediatek,mt8195-vencsys"; > + reg = <0x1a000000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vencsys_core1: clock-controller@1b000000 { > + compatible = "mediatek,mt8195-vencsys_core1"; > + reg = <0x1b000000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vdosys0: clock-controller@1c01a000 { > + compatible = "mediatek,mt8195-vdosys0"; > + reg = <0x1c01a000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vdosys1: clock-controller@1c100000 { > + compatible = "mediatek,mt8195-vdosys1"; > + reg = <0x1c100000 0x1000>; > + #clock-cells = <1>; > + }; > diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-sys-clock.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-sys-clock.yaml > new file mode 100644 > index 000000000000..ea379452ba91 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-sys-clock.yaml > @@ -0,0 +1,66 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/arm/mediatek/mediatek,mt8195-sys-clock.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: MediaTek System Clock Controller for MT8195 > + > +maintainers: > + - Chun-Jie Chen <chun-jie.chen@mediatek.com> > + > +description: > + The Mediatek system clock controller provides various clocks and system configuration > + like reset and bus protection on MT8195. > + > +properties: > + compatible: > + oneOf: Drop oneOf. > + - items: > + - enum: > + - mediatek,mt8195-topckgen > + - mediatek,mt8195-infracfg_ao > + - mediatek,mt8195-apmixedsys > + - mediatek,mt8195-pericfg_ao > + - const: syscon > + > + reg: > + maxItems: 1 > + > + '#clock-cells': > + const: 1 > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + topckgen: syscon@10000000 { > + compatible = "mediatek,mt8195-topckgen", "syscon"; > + reg = <0x10000000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + infracfg_ao: syscon@10001000 { > + compatible = "mediatek,mt8195-infracfg_ao", "syscon"; > + reg = <0x10001000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + apmixedsys: syscon@1000c000 { > + compatible = "mediatek,mt8195-apmixedsys", "syscon"; > + reg = <0x1000c000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + pericfg_ao: syscon@11003000 { > + compatible = "mediatek,mt8195-pericfg_ao", "syscon"; > + reg = <0x11003000 0x1000>; > + #clock-cells = <1>; > + }; > -- > 2.18.0 > >
On Thu, Jun 17, 2021 at 7:01 AM Chun-Jie Chen <chun-jie.chen@mediatek.com> wrote: > > On MT8195, tuner_en_reg is moved to register offest 0x0. > If we only judge by tuner_en_reg, it may lead to wrong address. > Add tuner_en_bit to the check condition. And it has been confirmed, > on all the MediaTek SoCs, bit0 of offset 0x0 is always occupied by > clock square control. > > Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> Though you might want to consider converting these types of checks into feature flags.
On 30/06/2021 09:31, Chen-Yu Tsai wrote: > On Thu, Jun 17, 2021 at 7:01 AM Chun-Jie Chen > <chun-jie.chen@mediatek.com> wrote: >> >> On MT8195, tuner_en_reg is moved to register offest 0x0. >> If we only judge by tuner_en_reg, it may lead to wrong address. >> Add tuner_en_bit to the check condition. And it has been confirmed, >> on all the MediaTek SoCs, bit0 of offset 0x0 is always occupied by >> clock square control. >> >> Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> > > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> > > Though you might want to consider converting these types of checks into feature > flags. > Yes I think adding a feature flag is the way to go. Luckily there are only a few SoCs that will need updates at the same time. Regards, Matthias
On Wed, Jun 30, 2021 at 6:53 PM Matthias Brugger <matthias.bgg@gmail.com> wrote: > > > > On 30/06/2021 09:31, Chen-Yu Tsai wrote: > > On Thu, Jun 17, 2021 at 7:01 AM Chun-Jie Chen > > <chun-jie.chen@mediatek.com> wrote: > >> > >> On MT8195, tuner_en_reg is moved to register offest 0x0. > >> If we only judge by tuner_en_reg, it may lead to wrong address. > >> Add tuner_en_bit to the check condition. And it has been confirmed, > >> on all the MediaTek SoCs, bit0 of offset 0x0 is always occupied by > >> clock square control. > >> > >> Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> > > > > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> > > > > Though you might want to consider converting these types of checks into feature > > flags. > > > > Yes I think adding a feature flag is the way to go. Luckily there are only a few > SoCs that will need updates at the same time. I also see that the different clock modules are tied together using only clock names written in the drivers, instead of clock references in the device tree. Unfortunately reworking this would likely require a lot more work. I previously did a bit of internal reworking for the sunxi drivers. While not the same, I think the plumbing required is comparable. ChenYu
On 30/06/2021 13:09, Chen-Yu Tsai wrote: > On Wed, Jun 30, 2021 at 6:53 PM Matthias Brugger <matthias.bgg@gmail.com> wrote: >> >> >> >> On 30/06/2021 09:31, Chen-Yu Tsai wrote: >>> On Thu, Jun 17, 2021 at 7:01 AM Chun-Jie Chen >>> <chun-jie.chen@mediatek.com> wrote: >>>> >>>> On MT8195, tuner_en_reg is moved to register offest 0x0. >>>> If we only judge by tuner_en_reg, it may lead to wrong address. >>>> Add tuner_en_bit to the check condition. And it has been confirmed, >>>> on all the MediaTek SoCs, bit0 of offset 0x0 is always occupied by >>>> clock square control. >>>> >>>> Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> >>> >>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> >>> >>> Though you might want to consider converting these types of checks into feature >>> flags. >>> >> >> Yes I think adding a feature flag is the way to go. Luckily there are only a few >> SoCs that will need updates at the same time. > > I also see that the different clock modules are tied together using only clock > names written in the drivers, instead of clock references in the device tree. > Not sure I understand what you mean. Do you refer to something like [1]? That's because the clock is probed by the DRM driver, as they share the same compatible and IP block. Regards, Matthias [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/mediatek/clk-mt8173-mm.c?h=v5.13#n139 > Unfortunately reworking this would likely require a lot more work. I previously > did a bit of internal reworking for the sunxi drivers. While not the same, I > think the plumbing required is comparable. > > ChenYu >
"On Wed, Jun 30, 2021 at 7:43 PM Matthias Brugger <matthias.bgg@gmail.com> wrote: > On 30/06/2021 13:09, Chen-Yu Tsai wrote: > > On Wed, Jun 30, 2021 at 6:53 PM Matthias Brugger <matthias.bgg@gmail.com> wrote: > >> On 30/06/2021 09:31, Chen-Yu Tsai wrote: > >>> On Thu, Jun 17, 2021 at 7:01 AM Chun-Jie Chen > >>> <chun-jie.chen@mediatek.com> wrote: > >>>> > >>>> On MT8195, tuner_en_reg is moved to register offest 0x0. > >>>> If we only judge by tuner_en_reg, it may lead to wrong address. > >>>> Add tuner_en_bit to the check condition. And it has been confirmed, > >>>> on all the MediaTek SoCs, bit0 of offset 0x0 is always occupied by > >>>> clock square control. > >>>> > >>>> Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> > >>> > >>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> > >>> > >>> Though you might want to consider converting these types of checks into feature > >>> flags. > >>> > >> > >> Yes I think adding a feature flag is the way to go. Luckily there are only a few > >> SoCs that will need updates at the same time. > > > > I also see that the different clock modules are tied together using only clock > > names written in the drivers, instead of clock references in the device tree. > > > > Not sure I understand what you mean. Do you refer to something like [1]? That's > because the clock is probed by the DRM driver, as they share the same compatible > and IP block. In the example driver you mentioned, most of the registered clocks have the same parent clock, "mm_sel". This clock is from another hardware block, "topckgen" [1]. The two are linked together by looking up the clock name. The link should be explicitly described in the device tree, instead of implicitly by some name found in two drivers. The consuming driver can fetch the clock name via of_clk_get_parent_name(), or be migrated to use `struct clk_parent_data`, which allows specifying local (to the DT node) clock names or clk indices as parent clk references. What's more confusing is that the mmsys node actually has "assigned-clocks" properties [2] referencing the "mm_sel" clock, but not "clock" properties referencing the same clock. On the surface this looks like the hardware is trying to configure clocks that it doesn't use. Also, Maxime Ripard made the argument before that "assigned-clock-rates" doesn't give any real guarantees that the clock rate won't change. A better method is to request and "lock" the clock rate in the consuming driver. So overall I think there are many improvements that can be made to the Mediatek clk drivers. They aren't real blockers to new drivers though, and I think each would take some effort and coordination across all the SoCs. Regards ChenYu [1] https://elixir.bootlin.com/linux/latest/source/drivers/clk/mediatek/clk-mt8173.c#L545 [2] https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt8173.dtsi#L996 > Regards, > Matthias > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/mediatek/clk-mt8173-mm.c?h=v5.13#n139 > > > Unfortunately reworking this would likely require a lot more work. I previously > > did a bit of internal reworking for the sunxi drivers. While not the same, I > > think the plumbing required is comparable. > > > > ChenYu > >
Hi, On Thu, Jun 17, 2021 at 06:47:27AM +0800, Chun-Jie Chen wrote: > Add MT8195 audio src source clock provider > > Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> > --- > drivers/clk/mediatek/Kconfig | 6 +++ > drivers/clk/mediatek/Makefile | 1 + > drivers/clk/mediatek/clk-mt8195-aud_src.c | 60 +++++++++++++++++++++++ > 3 files changed, 67 insertions(+) > create mode 100644 drivers/clk/mediatek/clk-mt8195-aud_src.c > > diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig > index e2bae9d490a4..62dd02bc2755 100644 > --- a/drivers/clk/mediatek/Kconfig > +++ b/drivers/clk/mediatek/Kconfig > @@ -594,6 +594,12 @@ config COMMON_CLK_MT8195_AUDSYS > help > This driver supports MediaTek MT8195 audsys clocks. > > +config COMMON_CLK_MT8195_AUDSYS_SRC > + bool "Clock driver for MediaTek MT8195 audsys_src" > + depends on COMMON_CLK_MT8195 > + help > + This driver supports MediaTek MT8195 audsys_src clocks. > + Same comments regarding the Kconfig symbol as the previous patch. > config COMMON_CLK_MT8516 > bool "Clock driver for MediaTek MT8516" > depends on ARCH_MEDIATEK || COMPILE_TEST > diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile > index f27c04314186..52a5d3f49ff0 100644 > --- a/drivers/clk/mediatek/Makefile > +++ b/drivers/clk/mediatek/Makefile > @@ -82,5 +82,6 @@ obj-$(CONFIG_COMMON_CLK_MT8192_VDECSYS) += clk-mt8192-vdec.o > obj-$(CONFIG_COMMON_CLK_MT8192_VENCSYS) += clk-mt8192-venc.o > obj-$(CONFIG_COMMON_CLK_MT8195) += clk-mt8195.o > obj-$(CONFIG_COMMON_CLK_MT8195_AUDSYS) += clk-mt8195-aud.o > +obj-$(CONFIG_COMMON_CLK_MT8195_AUDSYS_SRC) += clk-mt8195-aud_src.o > obj-$(CONFIG_COMMON_CLK_MT8516) += clk-mt8516.o > obj-$(CONFIG_COMMON_CLK_MT8516_AUDSYS) += clk-mt8516-aud.o > diff --git a/drivers/clk/mediatek/clk-mt8195-aud_src.c b/drivers/clk/mediatek/clk-mt8195-aud_src.c > new file mode 100644 > index 000000000000..7cabe0d68825 > --- /dev/null > +++ b/drivers/clk/mediatek/clk-mt8195-aud_src.c > @@ -0,0 +1,60 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +// Copyright (c) 2021 MediaTek Inc. > +// Author: Chun-Jie Chen <chun-jie.chen@mediatek.com> > + > +#include <linux/clk-provider.h> > +#include <linux/platform_device.h> > + > +#include "clk-mtk.h" > +#include "clk-gate.h" > + > +#include <dt-bindings/clock/mt8195-clk.h> > + > +static const struct mtk_gate_regs aud_src_cg_regs = { > + .set_ofs = 0x1004, > + .clr_ofs = 0x1004, > + .sta_ofs = 0x1004, > +}; > + > +#define GATE_AUD_SRC(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &aud_src_cg_regs, _shift, &mtk_clk_gate_ops_no_setclr) > + > +static const struct mtk_gate aud_src_clks[] = { > + GATE_AUD_SRC(CLK_AUD_SRC_ASRC0, "aud_src_asrc0", "asm_h_sel", 0), > + GATE_AUD_SRC(CLK_AUD_SRC_ASRC1, "aud_src_asrc1", "asm_h_sel", 1), > + GATE_AUD_SRC(CLK_AUD_SRC_ASRC2, "aud_src_asrc2", "asm_h_sel", 2), > + GATE_AUD_SRC(CLK_AUD_SRC_ASRC3, "aud_src_asrc3", "asm_h_sel", 3), > + GATE_AUD_SRC(CLK_AUD_SRC_ASRC4, "aud_src_asrc4", "asm_h_sel", 4), > + GATE_AUD_SRC(CLK_AUD_SRC_ASRC5, "aud_src_asrc5", "asm_h_sel", 5), > + GATE_AUD_SRC(CLK_AUD_SRC_ASRC6, "aud_src_asrc6", "asm_h_sel", 6), > + GATE_AUD_SRC(CLK_AUD_SRC_ASRC7, "aud_src_asrc7", "asm_h_sel", 7), > + GATE_AUD_SRC(CLK_AUD_SRC_ASRC8, "aud_src_asrc8", "asm_h_sel", 8), > + GATE_AUD_SRC(CLK_AUD_SRC_ASRC9, "aud_src_asrc9", "asm_h_sel", 9), > + GATE_AUD_SRC(CLK_AUD_SRC_ASRC10, "aud_src_asrc10", "asm_h_sel", 10), > + GATE_AUD_SRC(CLK_AUD_SRC_ASRC11, "aud_src_asrc11", "asm_h_sel", 11), And same thing about moving this into the audio driver. AFAICT there is no audio driver supporting this hardware block, so this will never get used. Regards ChenYu
On Thu, Jun 17, 2021 at 7:04 AM Chun-Jie Chen <chun-jie.chen@mediatek.com> wrote: > > Add MT8195 camsys clock providers > > Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
On Thu, Jun 17, 2021 at 6:59 AM Chun-Jie Chen <chun-jie.chen@mediatek.com> wrote: > > Add MT8195 ccusys clock provider > > Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> Also, I noticed that Mediatek drivers don't support the reset controls found in these clock controllers. Are there plans to add support for them?
On Thu, Jun 17, 2021 at 7:08 AM Chun-Jie Chen <chun-jie.chen@mediatek.com> wrote: > > Add MT8195 mfgcfg clock provider Same thing about the commit log. More context please. > Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> > --- > drivers/clk/mediatek/Kconfig | 6 ++++ > drivers/clk/mediatek/Makefile | 1 + > drivers/clk/mediatek/clk-mt8195-mfg.c | 49 +++++++++++++++++++++++++++ > 3 files changed, 56 insertions(+) > create mode 100644 drivers/clk/mediatek/clk-mt8195-mfg.c > > diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig > index ef7d4b433eee..066c14a89cee 100644 > --- a/drivers/clk/mediatek/Kconfig > +++ b/drivers/clk/mediatek/Kconfig > @@ -624,6 +624,12 @@ config COMMON_CLK_MT8195_IPESYS > help > This driver supports MediaTek MT8195 ipesys clocks. > > +config COMMON_CLK_MT8195_MFGCFG > + bool "Clock driver for MediaTek MT8195 mfgcfg" > + depends on COMMON_CLK_MT8195 > + help > + This driver supports MediaTek MT8195 mfgcfg clocks. > + Same thing about the Kconfig option. I don't think it's necessary to have separate Kconfig options for clock controllers within the same SoC. > config COMMON_CLK_MT8516 > bool "Clock driver for MediaTek MT8516" > depends on ARCH_MEDIATEK || COMPILE_TEST > diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile > index 991a9be7ac46..9b09e7f640d1 100644 > --- a/drivers/clk/mediatek/Makefile > +++ b/drivers/clk/mediatek/Makefile > @@ -87,5 +87,6 @@ obj-$(CONFIG_COMMON_CLK_MT8195_CAMSYS) += clk-mt8195-cam.o > obj-$(CONFIG_COMMON_CLK_MT8195_CCUSYS) += clk-mt8195-ccu.o > obj-$(CONFIG_COMMON_CLK_MT8195_IMGSYS) += clk-mt8195-img.o > obj-$(CONFIG_COMMON_CLK_MT8195_IPESYS) += clk-mt8195-ipe.o > +obj-$(CONFIG_COMMON_CLK_MT8195_MFGCFG) += clk-mt8195-mfg.o > obj-$(CONFIG_COMMON_CLK_MT8516) += clk-mt8516.o > obj-$(CONFIG_COMMON_CLK_MT8516_AUDSYS) += clk-mt8516-aud.o > diff --git a/drivers/clk/mediatek/clk-mt8195-mfg.c b/drivers/clk/mediatek/clk-mt8195-mfg.c > new file mode 100644 > index 000000000000..a9b1d337cd01 > --- /dev/null > +++ b/drivers/clk/mediatek/clk-mt8195-mfg.c Code looks good. ChenYu
Hi, On Thu, Jun 17, 2021 at 7:00 AM Chun-Jie Chen <chun-jie.chen@mediatek.com> wrote: > > Add MT8195 nnasys clock provider > > Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> > --- > drivers/clk/mediatek/Kconfig | 6 ++ > drivers/clk/mediatek/Makefile | 1 + > drivers/clk/mediatek/clk-mt8195-nna.c | 128 ++++++++++++++++++++++++++ > 3 files changed, 135 insertions(+) > create mode 100644 drivers/clk/mediatek/clk-mt8195-nna.c > > diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig > index 7cb745d47770..d34517728f4a 100644 > --- a/drivers/clk/mediatek/Kconfig > +++ b/drivers/clk/mediatek/Kconfig > @@ -636,6 +636,12 @@ config COMMON_CLK_MT8195_SCP_ADSP > help > This driver supports MediaTek MT8195 scp_adsp clocks. > > +config COMMON_CLK_MT8195_NNASYS > + bool "Clock driver for MediaTek MT8195 nnasys" > + depends on COMMON_CLK_MT8195 > + help > + This driver supports MediaTek MT8195 nnasys clocks. > + > config COMMON_CLK_MT8516 > bool "Clock driver for MediaTek MT8516" > depends on ARCH_MEDIATEK || COMPILE_TEST Same comments about commit log and Kconfig option apply. > diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile > index 058ff55468a2..49e585a7ac8e 100644 > --- a/drivers/clk/mediatek/Makefile > +++ b/drivers/clk/mediatek/Makefile > @@ -89,5 +89,6 @@ obj-$(CONFIG_COMMON_CLK_MT8195_IMGSYS) += clk-mt8195-img.o > obj-$(CONFIG_COMMON_CLK_MT8195_IPESYS) += clk-mt8195-ipe.o > obj-$(CONFIG_COMMON_CLK_MT8195_MFGCFG) += clk-mt8195-mfg.o > obj-$(CONFIG_COMMON_CLK_MT8195_SCP_ADSP) += clk-mt8195-scp_adsp.o > +obj-$(CONFIG_COMMON_CLK_MT8195_NNASYS) += clk-mt8195-nna.o > obj-$(CONFIG_COMMON_CLK_MT8516) += clk-mt8516.o > obj-$(CONFIG_COMMON_CLK_MT8516_AUDSYS) += clk-mt8516-aud.o > diff --git a/drivers/clk/mediatek/clk-mt8195-nna.c b/drivers/clk/mediatek/clk-mt8195-nna.c > new file mode 100644 > index 000000000000..4210c6cf5ef4 > --- /dev/null > +++ b/drivers/clk/mediatek/clk-mt8195-nna.c > @@ -0,0 +1,128 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +// Copyright (c) 2021 MediaTek Inc. > +// Author: Chun-Jie Chen <chun-jie.chen@mediatek.com> > + > +#include <linux/clk-provider.h> > +#include <linux/platform_device.h> > + > +#include "clk-mtk.h" > +#include "clk-gate.h" > + > +#include <dt-bindings/clock/mt8195-clk.h> > + > +static const struct mtk_gate_regs nna0_cg_regs = { > + .set_ofs = 0x104, > + .clr_ofs = 0x104, > + .sta_ofs = 0x104, You are using the _no_setclr_ops variant. That means the .set_ofs and .clr_ofs fields don't have any meaning and are not used. Please just don't initialize them. I think this applies to some of the other patches as well. > +}; > + > +static const struct mtk_gate_regs nna1_cg_regs = { > + .set_ofs = 0x110, > + .clr_ofs = 0x110, > + .sta_ofs = 0x110, > +}; > + > +static const struct mtk_gate_regs nna2_cg_regs = { > + .set_ofs = 0x90, > + .clr_ofs = 0x90, > + .sta_ofs = 0x90, > +}; > + > +static const struct mtk_gate_regs nna3_cg_regs = { > + .set_ofs = 0x94, > + .clr_ofs = 0x94, > + .sta_ofs = 0x94, > +}; > + > +static const struct mtk_gate_regs nna4_cg_regs = { > + .set_ofs = 0x98, > + .clr_ofs = 0x98, > + .sta_ofs = 0x98, > +}; > + > +static const struct mtk_gate_regs nna5_cg_regs = { > + .set_ofs = 0x9c, > + .clr_ofs = 0x9c, > + .sta_ofs = 0x9c, > +}; > + > +static const struct mtk_gate_regs nna6_cg_regs = { > + .set_ofs = 0xa0, > + .clr_ofs = 0xa0, > + .sta_ofs = 0xa0, > +}; > + > +static const struct mtk_gate_regs nna7_cg_regs = { > + .set_ofs = 0xa4, > + .clr_ofs = 0xa4, > + .sta_ofs = 0xa4, > +}; Unfortunately this hardware block is not documented in the datasheets, so I can't verify these register offsets. > + > +#define GATE_NNA0(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &nna0_cg_regs, _shift, &mtk_clk_gate_ops_no_setclr_inv) > + > +#define GATE_NNA1(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &nna1_cg_regs, _shift, &mtk_clk_gate_ops_no_setclr_inv) > + > +#define GATE_NNA2(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &nna2_cg_regs, _shift, &mtk_clk_gate_ops_no_setclr_inv) > + > +#define GATE_NNA3(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &nna3_cg_regs, _shift, &mtk_clk_gate_ops_no_setclr_inv) > + > +#define GATE_NNA4(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &nna4_cg_regs, _shift, &mtk_clk_gate_ops_no_setclr_inv) > + > +#define GATE_NNA5(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &nna5_cg_regs, _shift, &mtk_clk_gate_ops_no_setclr_inv) > + > +#define GATE_NNA6(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &nna6_cg_regs, _shift, &mtk_clk_gate_ops_no_setclr_inv) > + > +#define GATE_NNA7(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &nna7_cg_regs, _shift, &mtk_clk_gate_ops_no_setclr_inv) Defining a bunch of macros, only to have each of them used once doesn't provide any gains with regards to reusing code. Please consider just using GATE_MTK directly below. In the general case I would probably make some derivatives of GATE_MTK, such that the ops field (which seems to be the longest) is hidden, but all the other fields are exposed. > +static const struct mtk_gate nna_clks[] = { > + /* NNA0 */ > + GATE_NNA0(CLK_NNA_F26M, "nna_f26m", "clk26m", 0), > + /* NNA1 */ > + GATE_NNA1(CLK_NNA_AXI, "nna_axi", "axi_sel", 0), > + /* NNA2 */ > + GATE_NNA2(CLK_NNA_NNA0, "nna_nna0", "nna0_sel", 0), > + /* NNA3 */ > + GATE_NNA3(CLK_NNA_NNA1, "nna_nna1", "nna0_sel", 0), Is this correct? I would expect the clock for nna1 to be fed from "nna1_sel". ChenYu > + /* NNA4 */ > + GATE_NNA4(CLK_NNA_NNA0_EMI, "nna_nna0_emi", "mem_466m", 0), > + GATE_NNA4(CLK_NNA_CKGEN_MEM, "nna_ckgen_mem", "mem_466m", 4), > + /* NNA5 */ > + GATE_NNA5(CLK_NNA_NNA1_EMI, "nna_nna1_emi", "mem_466m", 0), > + /* NNA6 */ > + GATE_NNA6(CLK_NNA_NNA0_AXI, "nna_nna0_axi", "axi_sel", 0), > + /* NNA7 */ > + GATE_NNA7(CLK_NNA_NNA1_AXI, "nna_nna1_axi", "axi_sel", 0), > +}; > + > +static const struct mtk_clk_desc nna_desc = { > + .clks = nna_clks, > + .num_clks = ARRAY_SIZE(nna_clks), > +}; > + > +static const struct of_device_id of_match_clk_mt8195_nna[] = { > + { > + .compatible = "mediatek,mt8195-nnasys", > + .data = &nna_desc, > + }, { > + /* sentinel */ > + } > +}; > + > +static struct platform_driver clk_mt8195_nna_drv = { > + .probe = mtk_clk_simple_probe, > + .driver = { > + .name = "clk-mt8195-nna", > + .of_match_table = of_match_clk_mt8195_nna, > + }, > +}; > + > +builtin_platform_driver(clk_mt8195_nna_drv); > -- > 2.18.0 > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
Hi, On Thu, Jun 17, 2021 at 7:03 AM Chun-Jie Chen <chun-jie.chen@mediatek.com> wrote: > > Add MT8195 vdosys0 clock provider > > Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> > --- > drivers/clk/mediatek/Kconfig | 6 ++ > drivers/clk/mediatek/Makefile | 1 + > drivers/clk/mediatek/clk-mt8195-vdo0.c | 114 +++++++++++++++++++++++++ > 3 files changed, 121 insertions(+) > create mode 100644 drivers/clk/mediatek/clk-mt8195-vdo0.c > > diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig > index b7881b8ebb23..6602f9ba13c7 100644 > --- a/drivers/clk/mediatek/Kconfig > +++ b/drivers/clk/mediatek/Kconfig > @@ -648,6 +648,12 @@ config COMMON_CLK_MT8195_VDECSYS > help > This driver supports MediaTek MT8195 vdecsys clocks. > > +config COMMON_CLK_MT8195_VDOSYS0 > + bool "Clock driver for MediaTek MT8195 vdosys0" > + depends on COMMON_CLK_MT8195 > + help > + This driver supports MediaTek MT8195 vdosys0 clocks. > + Same comments about commit log and Kconfig option. > config COMMON_CLK_MT8516 > bool "Clock driver for MediaTek MT8516" > depends on ARCH_MEDIATEK || COMPILE_TEST > diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile > index 9acfa705f1de..6aa1ba00342a 100644 > --- a/drivers/clk/mediatek/Makefile > +++ b/drivers/clk/mediatek/Makefile > @@ -91,5 +91,6 @@ obj-$(CONFIG_COMMON_CLK_MT8195_MFGCFG) += clk-mt8195-mfg.o > obj-$(CONFIG_COMMON_CLK_MT8195_SCP_ADSP) += clk-mt8195-scp_adsp.o > obj-$(CONFIG_COMMON_CLK_MT8195_NNASYS) += clk-mt8195-nna.o > obj-$(CONFIG_COMMON_CLK_MT8195_VDECSYS) += clk-mt8195-vdec.o > +obj-$(CONFIG_COMMON_CLK_MT8195_VDOSYS0) += clk-mt8195-vdo0.o > obj-$(CONFIG_COMMON_CLK_MT8516) += clk-mt8516.o > obj-$(CONFIG_COMMON_CLK_MT8516_AUDSYS) += clk-mt8516-aud.o > diff --git a/drivers/clk/mediatek/clk-mt8195-vdo0.c b/drivers/clk/mediatek/clk-mt8195-vdo0.c > new file mode 100644 > index 000000000000..4a34ccb0beed > --- /dev/null > +++ b/drivers/clk/mediatek/clk-mt8195-vdo0.c > @@ -0,0 +1,114 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +// Copyright (c) 2021 MediaTek Inc. > +// Author: Chun-Jie Chen <chun-jie.chen@mediatek.com> > + > +#include <linux/clk-provider.h> > +#include <linux/platform_device.h> > + > +#include "clk-mtk.h" > +#include "clk-gate.h" Alphabetical order within the same group please. > + > +#include <dt-bindings/clock/mt8195-clk.h> > + > +static const struct mtk_gate_regs vdo00_cg_regs = { > + .set_ofs = 0x104, > + .clr_ofs = 0x108, > + .sta_ofs = 0x100, > +}; > + > +static const struct mtk_gate_regs vdo01_cg_regs = { > + .set_ofs = 0x114, > + .clr_ofs = 0x118, > + .sta_ofs = 0x110, > +}; > + > +static const struct mtk_gate_regs vdo02_cg_regs = { > + .set_ofs = 0x124, > + .clr_ofs = 0x128, > + .sta_ofs = 0x120, > +}; > + > +#define GATE_VDO00(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &vdo00_cg_regs, _shift, &mtk_clk_gate_ops_setclr) > + > +#define GATE_VDO01(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &vdo01_cg_regs, _shift, &mtk_clk_gate_ops_setclr) > + > +#define GATE_VDO02(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &vdo02_cg_regs, _shift, &mtk_clk_gate_ops_setclr) > + > +static const struct mtk_gate vdo0_clks[] = { > + /* VDO00 */ > + GATE_VDO00(CLK_VDO0_DISP_OVL0, "vdo0_disp_ovl0", "vpp_sel", 0), > + GATE_VDO00(CLK_VDO0_DISP_COLOR0, "vdo0_disp_color0", "vpp_sel", 2), > + GATE_VDO00(CLK_VDO0_DISP_COLOR1, "vdo0_disp_color1", "vpp_sel", 3), > + GATE_VDO00(CLK_VDO0_DISP_CCORR0, "vdo0_disp_ccorr0", "vpp_sel", 4), > + GATE_VDO00(CLK_VDO0_DISP_CCORR1, "vdo0_disp_ccorr1", "vpp_sel", 5), > + GATE_VDO00(CLK_VDO0_DISP_AAL0, "vdo0_disp_aal0", "vpp_sel", 6), > + GATE_VDO00(CLK_VDO0_DISP_AAL1, "vdo0_disp_aal1", "vpp_sel", 7), > + GATE_VDO00(CLK_VDO0_DISP_GAMMA0, "vdo0_disp_gamma0", "vpp_sel", 8), > + GATE_VDO00(CLK_VDO0_DISP_GAMMA1, "vdo0_disp_gamma1", "vpp_sel", 9), > + GATE_VDO00(CLK_VDO0_DISP_DITHER0, "vdo0_disp_dither0", "vpp_sel", 10), > + GATE_VDO00(CLK_VDO0_DISP_DITHER1, "vdo0_disp_dither1", "vpp_sel", 11), > + GATE_VDO00(CLK_VDO0_DISP_OVL1, "vdo0_disp_ovl1", "vpp_sel", 16), > + GATE_VDO00(CLK_VDO0_DISP_WDMA0, "vdo0_disp_wdma0", "vpp_sel", 17), > + GATE_VDO00(CLK_VDO0_DISP_WDMA1, "vdo0_disp_wdma1", "vpp_sel", 18), > + GATE_VDO00(CLK_VDO0_DISP_RDMA0, "vdo0_disp_rdma0", "vpp_sel", 19), > + GATE_VDO00(CLK_VDO0_DISP_RDMA1, "vdo0_disp_rdma1", "vpp_sel", 20), > + GATE_VDO00(CLK_VDO0_DSI0, "vdo0_dsi0", "vpp_sel", 21), > + GATE_VDO00(CLK_VDO0_DSI1, "vdo0_dsi1", "vpp_sel", 22), > + GATE_VDO00(CLK_VDO0_DSC_WRAP0, "vdo0_dsc_wrap0", "vpp_sel", 23), > + GATE_VDO00(CLK_VDO0_VPP_MERGE0, "vdo0_vpp_merge0", "vpp_sel", 24), > + GATE_VDO00(CLK_VDO0_DP_INTF0, "vdo0_dp_intf0", "vpp_sel", 25), > + GATE_VDO00(CLK_VDO0_DISP_MUTEX0, "vdo0_disp_mutex0", "vpp_sel", 26), > + GATE_VDO00(CLK_VDO0_DISP_IL_ROT0, "vdo0_disp_il_rot0", "vpp_sel", 27), > + GATE_VDO00(CLK_VDO0_APB_BUS, "vdo0_apb_bus", "vpp_sel", 28), > + GATE_VDO00(CLK_VDO0_FAKE_ENG0, "vdo0_fake_eng0", "vpp_sel", 29), > + GATE_VDO00(CLK_VDO0_FAKE_ENG1, "vdo0_fake_eng1", "vpp_sel", 30), > + /* VDO01 */ > + GATE_VDO01(CLK_VDO0_DL_ASYNC0, "vdo0_dl_async0", "vpp_sel", 0), > + GATE_VDO01(CLK_VDO0_DL_ASYNC1, "vdo0_dl_async1", "vpp_sel", 1), > + GATE_VDO01(CLK_VDO0_DL_ASYNC2, "vdo0_dl_async2", "vpp_sel", 2), > + GATE_VDO01(CLK_VDO0_DL_ASYNC3, "vdo0_dl_async3", "vpp_sel", 3), > + GATE_VDO01(CLK_VDO0_DL_ASYNC4, "vdo0_dl_async4", "vpp_sel", 4), > + GATE_VDO01(CLK_VDO0_DISP_MONITOR0, "vdo0_disp_monitor0", "vpp_sel", 5), > + GATE_VDO01(CLK_VDO0_DISP_MONITOR1, "vdo0_disp_monitor1", "vpp_sel", 6), > + GATE_VDO01(CLK_VDO0_DISP_MONITOR2, "vdo0_disp_monitor2", "vpp_sel", 7), > + GATE_VDO01(CLK_VDO0_DISP_MONITOR3, "vdo0_disp_monitor3", "vpp_sel", 8), > + GATE_VDO01(CLK_VDO0_DISP_MONITOR4, "vdo0_disp_monitor4", "vpp_sel", 9), > + GATE_VDO01(CLK_VDO0_SMI_GALS, "vdo0_smi_gals", "vpp_sel", 10), > + GATE_VDO01(CLK_VDO0_SMI_COMMON, "vdo0_smi_common", "vpp_sel", 11), > + GATE_VDO01(CLK_VDO0_SMI_EMI, "vdo0_smi_emi", "vpp_sel", 12), > + GATE_VDO01(CLK_VDO0_SMI_IOMMU, "vdo0_smi_iommu", "vpp_sel", 13), > + GATE_VDO01(CLK_VDO0_SMI_LARB, "vdo0_smi_larb", "vpp_sel", 14), > + GATE_VDO01(CLK_VDO0_SMI_RSI, "vdo0_smi_rsi", "vpp_sel", 15), > + /* VDO02 */ > + GATE_VDO02(CLK_VDO0_DSI0_DSI, "vdo0_dsi0_dsi", "dsi_occ_sel", 0), > + GATE_VDO02(CLK_VDO0_DSI1_DSI, "vdo0_dsi1_dsi", "dsi_occ_sel", 8), > + GATE_VDO02(CLK_VDO0_DP_INTF0_DP_INTF, "vdo0_dp_intf0_dp_intf", "edp_sel", 16), > +}; > + > +static const struct mtk_clk_desc vdo0_desc = { > + .clks = vdo0_clks, > + .num_clks = ARRAY_SIZE(vdo0_clks), > +}; > + > +static const struct of_device_id of_match_clk_mt8195_vdo0[] = { > + { > + .compatible = "mediatek,mt8195-vdosys0", > + .data = &vdo0_desc, > + }, { > + /* sentinel */ > + } > +}; > + > +static struct platform_driver clk_mt8195_vdo0_drv = { > + .probe = mtk_clk_simple_probe, > + .driver = { > + .name = "clk-mt8195-vdo0", > + .of_match_table = of_match_clk_mt8195_vdo0, > + }, > +}; > + Can drop the empty line here. Overall the code looks good. ChenYu > +builtin_platform_driver(clk_mt8195_vdo0_drv); > -- > 2.18.0 > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Thu, Jun 17, 2021 at 7:10 AM Chun-Jie Chen <chun-jie.chen@mediatek.com> wrote: > > Add MT8195 vencsys clock providers > > Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> > --- > drivers/clk/mediatek/Kconfig | 6 +++ > drivers/clk/mediatek/Makefile | 1 + > drivers/clk/mediatek/clk-mt8195-venc.c | 71 ++++++++++++++++++++++++++ > 3 files changed, 78 insertions(+) > create mode 100644 drivers/clk/mediatek/clk-mt8195-venc.c > > diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig > index 1e89c68f6c6c..3352686d98cf 100644 > --- a/drivers/clk/mediatek/Kconfig > +++ b/drivers/clk/mediatek/Kconfig > @@ -660,6 +660,12 @@ config COMMON_CLK_MT8195_VDOSYS1 > help > This driver supports MediaTek MT8195 vdosys1 clocks. > > +config COMMON_CLK_MT8195_VENCSYS > + bool "Clock driver for MediaTek MT8195 vencsys" > + depends on COMMON_CLK_MT8195 > + help > + This driver supports MediaTek MT8195 vencsys clocks. > + > config COMMON_CLK_MT8516 > bool "Clock driver for MediaTek MT8516" > depends on ARCH_MEDIATEK || COMPILE_TEST > diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile > index 76c0fa837cb0..76a6b404e34b 100644 > --- a/drivers/clk/mediatek/Makefile > +++ b/drivers/clk/mediatek/Makefile > @@ -93,5 +93,6 @@ obj-$(CONFIG_COMMON_CLK_MT8195_NNASYS) += clk-mt8195-nna.o > obj-$(CONFIG_COMMON_CLK_MT8195_VDECSYS) += clk-mt8195-vdec.o > obj-$(CONFIG_COMMON_CLK_MT8195_VDOSYS0) += clk-mt8195-vdo0.o > obj-$(CONFIG_COMMON_CLK_MT8195_VDOSYS1) += clk-mt8195-vdo1.o > +obj-$(CONFIG_COMMON_CLK_MT8195_VENCSYS) += clk-mt8195-venc.o > obj-$(CONFIG_COMMON_CLK_MT8516) += clk-mt8516.o > obj-$(CONFIG_COMMON_CLK_MT8516_AUDSYS) += clk-mt8516-aud.o > diff --git a/drivers/clk/mediatek/clk-mt8195-venc.c b/drivers/clk/mediatek/clk-mt8195-venc.c > new file mode 100644 > index 000000000000..410ca69d5759 > --- /dev/null > +++ b/drivers/clk/mediatek/clk-mt8195-venc.c > @@ -0,0 +1,71 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +// Copyright (c) 2021 MediaTek Inc. > +// Author: Chun-Jie Chen <chun-jie.chen@mediatek.com> > + > +#include <linux/clk-provider.h> > +#include <linux/platform_device.h> > + > +#include "clk-mtk.h" > +#include "clk-gate.h" > + > +#include <dt-bindings/clock/mt8195-clk.h> > + > +static const struct mtk_gate_regs venc_cg_regs = { > + .set_ofs = 0x4, > + .clr_ofs = 0x8, > + .sta_ofs = 0x0, > +}; > + > +#define GATE_VENC(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &venc_cg_regs, _shift, &mtk_clk_gate_ops_setclr_inv) > + > +static const struct mtk_gate venc_clks[] = { > + GATE_VENC(CLK_VENC_LARB, "venc_larb", "venc_sel", 0), > + GATE_VENC(CLK_VENC_VENC, "venc_venc", "venc_sel", 4), > + GATE_VENC(CLK_VENC_JPGENC, "venc_jpgenc", "venc_sel", 8), > + GATE_VENC(CLK_VENC_JPGDEC, "venc_jpgdec", "venc_sel", 12), > + GATE_VENC(CLK_VENC_JPGDEC_C1, "venc_jpgdec_c1", "venc_sel", 16), > + GATE_VENC(CLK_VENC_GALS, "venc_gals", "venc_sel", 28), > +}; > + > +static const struct mtk_gate venc_core1_clks[] = { > + GATE_VENC(CLK_VENC_CORE1_LARB, "venc_core1_larb", "venc_sel", 0), > + GATE_VENC(CLK_VENC_CORE1_VENC, "venc_core1_venc", "venc_sel", 4), > + GATE_VENC(CLK_VENC_CORE1_JPGENC, "venc_core1_jpgenc", "venc_sel", 8), > + GATE_VENC(CLK_VENC_CORE1_JPGDEC, "venc_core1_jpgdec", "venc_sel", 12), > + GATE_VENC(CLK_VENC_CORE1_JPGDEC_C1, "venc_core1_jpgdec_c1", "venc_sel", 16), > + GATE_VENC(CLK_VENC_CORE1_GALS, "venc_core1_gals", "venc_sel", 28), > +}; So I'm not sure why there are two sets of clocks for each endpoint. Since this hardware block is not documented in the datasheet, maybe you could provide some explanation? What I'm wondering is if the first set is actually some clock gate for bus access and the second set is the main module clock that drives the internal logic. The general comments from the other patches apply as well. ChenYu > + > +static const struct mtk_clk_desc venc_desc = { > + .clks = venc_clks, > + .num_clks = ARRAY_SIZE(venc_clks), > +}; > + > +static const struct mtk_clk_desc venc_core1_desc = { > + .clks = venc_core1_clks, > + .num_clks = ARRAY_SIZE(venc_core1_clks), > +}; > + > +static const struct of_device_id of_match_clk_mt8195_venc[] = { > + { > + .compatible = "mediatek,mt8195-vencsys", > + .data = &venc_desc, > + }, { > + .compatible = "mediatek,mt8195-vencsys_core1", > + .data = &venc_core1_desc, > + }, { > + /* sentinel */ > + } > +}; > + > +static struct platform_driver clk_mt8195_venc_drv = { > + .probe = mtk_clk_simple_probe, > + .driver = { > + .name = "clk-mt8195-venc", > + .of_match_table = of_match_clk_mt8195_venc, > + }, > +}; > + > +builtin_platform_driver(clk_mt8195_venc_drv); > -- > 2.18.0 > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
Hi, On Thu, Jun 17, 2021 at 7:00 AM Chun-Jie Chen <chun-jie.chen@mediatek.com> wrote: > > Add MT8195 vppsys1 clock provider > > Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> > --- > drivers/clk/mediatek/Kconfig | 6 ++ > drivers/clk/mediatek/Makefile | 1 + > drivers/clk/mediatek/clk-mt8195-vpp1.c | 110 +++++++++++++++++++++++++ > 3 files changed, 117 insertions(+) > create mode 100644 drivers/clk/mediatek/clk-mt8195-vpp1.c > > diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig > index 2deef026fbb4..91b1c19da1ab 100644 > --- a/drivers/clk/mediatek/Kconfig > +++ b/drivers/clk/mediatek/Kconfig > @@ -672,6 +672,12 @@ config COMMON_CLK_MT8195_VPPSYS0 > help > This driver supports MediaTek MT8195 vppsys0 clocks. > > +config COMMON_CLK_MT8195_VPPSYS1 > + bool "Clock driver for MediaTek MT8195 vppsys1" > + depends on COMMON_CLK_MT8195 > + help > + This driver supports MediaTek MT8195 vppsys1 clocks. > + > config COMMON_CLK_MT8516 > bool "Clock driver for MediaTek MT8516" > depends on ARCH_MEDIATEK || COMPILE_TEST > diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile > index 34cd7f2c71ac..fbf57473bb91 100644 > --- a/drivers/clk/mediatek/Makefile > +++ b/drivers/clk/mediatek/Makefile > @@ -95,5 +95,6 @@ obj-$(CONFIG_COMMON_CLK_MT8195_VDOSYS0) += clk-mt8195-vdo0.o > obj-$(CONFIG_COMMON_CLK_MT8195_VDOSYS1) += clk-mt8195-vdo1.o > obj-$(CONFIG_COMMON_CLK_MT8195_VENCSYS) += clk-mt8195-venc.o > obj-$(CONFIG_COMMON_CLK_MT8195_VPPSYS0) += clk-mt8195-vpp0.o > +obj-$(CONFIG_COMMON_CLK_MT8195_VPPSYS1) += clk-mt8195-vpp1.o > obj-$(CONFIG_COMMON_CLK_MT8516) += clk-mt8516.o > obj-$(CONFIG_COMMON_CLK_MT8516_AUDSYS) += clk-mt8516-aud.o > diff --git a/drivers/clk/mediatek/clk-mt8195-vpp1.c b/drivers/clk/mediatek/clk-mt8195-vpp1.c > new file mode 100644 > index 000000000000..0650ba86d5b6 > --- /dev/null > +++ b/drivers/clk/mediatek/clk-mt8195-vpp1.c > @@ -0,0 +1,110 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +// Copyright (c) 2021 MediaTek Inc. > +// Author: Chun-Jie Chen <chun-jie.chen@mediatek.com> > + > +#include <linux/clk-provider.h> > +#include <linux/platform_device.h> > + > +#include "clk-mtk.h" > +#include "clk-gate.h" > + > +#include <dt-bindings/clock/mt8195-clk.h> > + > +static const struct mtk_gate_regs vpp10_cg_regs = { Again, please add a separator. > + .set_ofs = 0x104, > + .clr_ofs = 0x108, > + .sta_ofs = 0x100, > +}; > + > +static const struct mtk_gate_regs vpp11_cg_regs = { > + .set_ofs = 0x114, > + .clr_ofs = 0x118, > + .sta_ofs = 0x110, > +}; > + > +#define GATE_VPP10(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &vpp10_cg_regs, _shift, &mtk_clk_gate_ops_setclr) > + > +#define GATE_VPP11(_id, _name, _parent, _shift) \ > + GATE_MTK(_id, _name, _parent, &vpp11_cg_regs, _shift, &mtk_clk_gate_ops_setclr) > + > +static const struct mtk_gate vpp1_clks[] = { > + /* VPP10 */ > + GATE_VPP10(CLK_VPP1_SVPP1_MDP_OVL, "vpp1_svpp1_mdp_ovl", "vpp_sel", 0), > + GATE_VPP10(CLK_VPP1_SVPP1_MDP_TCC, "vpp1_svpp1_mdp_tcc", "vpp_sel", 1), > + GATE_VPP10(CLK_VPP1_SVPP1_MDP_WROT, "vpp1_svpp1_mdp_wrot", "vpp_sel", 2), > + GATE_VPP10(CLK_VPP1_SVPP1_VPP_PAD, "vpp1_svpp1_vpp_pad", "vpp_sel", 3), > + GATE_VPP10(CLK_VPP1_SVPP2_MDP_WROT, "vpp1_svpp2_mdp_wrot", "vpp_sel", 4), > + GATE_VPP10(CLK_VPP1_SVPP2_VPP_PAD, "vpp1_svpp2_vpp_pad", "vpp_sel", 5), > + GATE_VPP10(CLK_VPP1_SVPP3_MDP_WROT, "vpp1_svpp3_mdp_wrot", "vpp_sel", 6), > + GATE_VPP10(CLK_VPP1_SVPP3_VPP_PAD, "vpp1_svpp3_vpp_pad", "vpp_sel", 7), > + GATE_VPP10(CLK_VPP1_SVPP1_MDP_RDMA, "vpp1_svpp1_mdp_rdma", "vpp_sel", 8), > + GATE_VPP10(CLK_VPP1_SVPP1_MDP_FG, "vpp1_svpp1_mdp_fg", "vpp_sel", 9), > + GATE_VPP10(CLK_VPP1_SVPP2_MDP_RDMA, "vpp1_svpp2_mdp_rdma", "vpp_sel", 10), > + GATE_VPP10(CLK_VPP1_SVPP2_MDP_FG, "vpp1_svpp2_mdp_fg", "vpp_sel", 11), > + GATE_VPP10(CLK_VPP1_SVPP3_MDP_RDMA, "vpp1_svpp3_mdp_rdma", "vpp_sel", 12), > + GATE_VPP10(CLK_VPP1_SVPP3_MDP_FG, "vpp1_svpp3_mdp_fg", "vpp_sel", 13), > + GATE_VPP10(CLK_VPP1_VPP_SPLIT, "vpp1_vpp_split", "vpp_sel", 14), > + GATE_VPP10(CLK_VPP1_SVPP2_VDO0_DL_RELAY, "vpp1_svpp2_vdo0_dl_relay", "vpp_sel", 15), > + GATE_VPP10(CLK_VPP1_SVPP1_MDP_TDSHP, "vpp1_svpp1_mdp_tdshp", "vpp_sel", 16), > + GATE_VPP10(CLK_VPP1_SVPP1_MDP_COLOR, "vpp1_svpp1_mdp_color", "vpp_sel", 17), > + GATE_VPP10(CLK_VPP1_SVPP3_VDO1_DL_RELAY, "vpp1_svpp3_vdo1_dl_relay", "vpp_sel", 18), > + GATE_VPP10(CLK_VPP1_SVPP2_VPP_MERGE, "vpp1_svpp2_vpp_merge", "vpp_sel", 19), > + GATE_VPP10(CLK_VPP1_SVPP2_MDP_COLOR, "vpp1_svpp2_mdp_color", "vpp_sel", 20), > + GATE_VPP10(CLK_VPP1_VPPSYS1_GALS, "vpp1_vppsys1_gals", "vpp_sel", 21), > + GATE_VPP10(CLK_VPP1_SVPP3_VPP_MERGE, "vpp1_svpp3_vpp_merge", "vpp_sel", 22), > + GATE_VPP10(CLK_VPP1_SVPP3_MDP_COLOR, "vpp1_svpp3_mdp_color", "vpp_sel", 23), > + GATE_VPP10(CLK_VPP1_VPPSYS1_LARB, "vpp1_vppsys1_larb", "vpp_sel", 24), > + GATE_VPP10(CLK_VPP1_SVPP1_MDP_RSZ, "vpp1_svpp1_mdp_rsz", "vpp_sel", 25), > + GATE_VPP10(CLK_VPP1_SVPP1_MDP_HDR, "vpp1_svpp1_mdp_hdr", "vpp_sel", 26), > + GATE_VPP10(CLK_VPP1_SVPP1_MDP_AAL, "vpp1_svpp1_mdp_aal", "vpp_sel", 27), > + GATE_VPP10(CLK_VPP1_SVPP2_MDP_HDR, "vpp1_svpp2_mdp_hdr", "vpp_sel", 28), > + GATE_VPP10(CLK_VPP1_SVPP2_MDP_AAL, "vpp1_svpp2_mdp_aal", "vpp_sel", 29), > + GATE_VPP10(CLK_VPP1_DL_ASYNC, "vpp1_dl_async", "vpp_sel", 30), > + GATE_VPP10(CLK_VPP1_LARB5_FAKE_ENG, "vpp1_larb5_fake_eng", "vpp_sel", 31), > + /* VPP11 */ > + GATE_VPP11(CLK_VPP1_SVPP3_MDP_HDR, "vpp1_svpp3_mdp_hdr", "vpp_sel", 0), > + GATE_VPP11(CLK_VPP1_SVPP3_MDP_AAL, "vpp1_svpp3_mdp_aal", "vpp_sel", 1), > + GATE_VPP11(CLK_VPP1_SVPP2_VDO1_DL_RELAY, "vpp1_svpp2_vdo1_dl_relay", "vpp_sel", 2), > + GATE_VPP11(CLK_VPP1_LARB6_FAKE_ENG, "vpp1_larb6_fake_eng", "vpp_sel", 3), > + GATE_VPP11(CLK_VPP1_SVPP2_MDP_RSZ, "vpp1_svpp2_mdp_rsz", "vpp_sel", 4), > + GATE_VPP11(CLK_VPP1_SVPP3_MDP_RSZ, "vpp1_svpp3_mdp_rsz", "vpp_sel", 5), > + GATE_VPP11(CLK_VPP1_SVPP3_VDO0_DL_RELAY, "vpp1_svpp3_vdo0_dl_relay", "vpp_sel", 6), > + GATE_VPP11(CLK_VPP1_DISP_MUTEX, "vpp1_disp_mutex", "vpp_sel", 7), > + GATE_VPP11(CLK_VPP1_SVPP2_MDP_TDSHP, "vpp1_svpp2_mdp_tdshp", "vpp_sel", 8), > + GATE_VPP11(CLK_VPP1_SVPP3_MDP_TDSHP, "vpp1_svpp3_mdp_tdshp", "vpp_sel", 9), > + GATE_VPP11(CLK_VPP1_VPP0_DL1_RELAY, "vpp1_vpp0_dl1_relay", "vpp_sel", 10), > + GATE_VPP11(CLK_VPP1_HDMI_META, "vpp1_hdmi_meta", "hdmirx_p", 11), > + GATE_VPP11(CLK_VPP1_VPP_SPLIT_HDMI, "vpp1_vpp_split_hdmi", "hdmirx_p", 12), > + GATE_VPP11(CLK_VPP1_DGI_IN, "vpp1_dgi_in", "in_dgi", 13), > + GATE_VPP11(CLK_VPP1_DGI_OUT, "vpp1_dgi_out", "dgi_out_sel", 14), > + GATE_VPP11(CLK_VPP1_VPP_SPLIT_DGI, "vpp1_vpp_split_dgi", "dgi_out_sel", 15), > + GATE_VPP11(CLK_VPP1_VPP0_DL_ASYNC, "vpp1_vpp0_dl_async", "vpp_sel", 16), > + GATE_VPP11(CLK_VPP1_VPP0_DL_RELAY, "vpp1_vpp0_dl_relay", "vpp_sel", 17), > + GATE_VPP11(CLK_VPP1_VPP_SPLIT_26M, "vpp1_vpp_split_26m", "clk26m", 26), The last two aren't in the datasheet. Same general comments apply. ChenYu > +}; > + > +static const struct mtk_clk_desc vpp1_desc = { > + .clks = vpp1_clks, > + .num_clks = ARRAY_SIZE(vpp1_clks), > +}; > + > +static const struct of_device_id of_match_clk_mt8195_vpp1[] = { > + { > + .compatible = "mediatek,mt8195-vppsys1", > + .data = &vpp1_desc, > + }, { > + /* sentinel */ > + } > +}; > + > +static struct platform_driver clk_mt8195_vpp1_drv = { > + .probe = mtk_clk_simple_probe, > + .driver = { > + .name = "clk-mt8195-vpp1", > + .of_match_table = of_match_clk_mt8195_vpp1, > + }, > +}; > + > +builtin_platform_driver(clk_mt8195_vpp1_drv); > -- > 2.18.0 > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
Hi, On Thu, Jun 17, 2021 at 6:59 AM Chun-Jie Chen <chun-jie.chen@mediatek.com> wrote: > > Add MT8195 imp i2c wrapper clock providers > > Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> > --- > drivers/clk/mediatek/Kconfig | 6 ++ > drivers/clk/mediatek/Makefile | 1 + > .../clk/mediatek/clk-mt8195-imp_iic_wrap.c | 68 +++++++++++++++++++ > 3 files changed, 75 insertions(+) > create mode 100644 drivers/clk/mediatek/clk-mt8195-imp_iic_wrap.c > > diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig > index 5089bacdf0a5..ade85a52b7ed 100644 > --- a/drivers/clk/mediatek/Kconfig > +++ b/drivers/clk/mediatek/Kconfig > @@ -684,6 +684,12 @@ config COMMON_CLK_MT8195_WPESYS > help > This driver supports MediaTek MT8195 wpesys clocks. > > +config COMMON_CLK_MT8195_IMP_IIC_WRAP > + bool "Clock driver for MediaTek MT8195 imp_iic_wrap" > + depends on COMMON_CLK_MT8195 > + help > + This driver supports MediaTek MT8195 imp_iic_wrap clocks. > + General comments from other patches also apply. > config COMMON_CLK_MT8516 > bool "Clock driver for MediaTek MT8516" > depends on ARCH_MEDIATEK || COMPILE_TEST > diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile > index 32cfb0030d92..b10c6267ba98 100644 > --- a/drivers/clk/mediatek/Makefile > +++ b/drivers/clk/mediatek/Makefile > @@ -97,5 +97,6 @@ obj-$(CONFIG_COMMON_CLK_MT8195_VENCSYS) += clk-mt8195-venc.o > obj-$(CONFIG_COMMON_CLK_MT8195_VPPSYS0) += clk-mt8195-vpp0.o > obj-$(CONFIG_COMMON_CLK_MT8195_VPPSYS1) += clk-mt8195-vpp1.o > obj-$(CONFIG_COMMON_CLK_MT8195_WPESYS) += clk-mt8195-wpe.o > +obj-$(CONFIG_COMMON_CLK_MT8195_IMP_IIC_WRAP) += clk-mt8195-imp_iic_wrap.o > obj-$(CONFIG_COMMON_CLK_MT8516) += clk-mt8516.o > obj-$(CONFIG_COMMON_CLK_MT8516_AUDSYS) += clk-mt8516-aud.o > diff --git a/drivers/clk/mediatek/clk-mt8195-imp_iic_wrap.c b/drivers/clk/mediatek/clk-mt8195-imp_iic_wrap.c > new file mode 100644 > index 000000000000..efb62f484bbe > --- /dev/null > +++ b/drivers/clk/mediatek/clk-mt8195-imp_iic_wrap.c > @@ -0,0 +1,68 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +// Copyright (c) 2021 MediaTek Inc. > +// Author: Chun-Jie Chen <chun-jie.chen@mediatek.com> > + > +#include <linux/clk-provider.h> > +#include <linux/platform_device.h> > + > +#include "clk-mtk.h" > +#include "clk-gate.h" > + > +#include <dt-bindings/clock/mt8195-clk.h> > + > +static const struct mtk_gate_regs imp_iic_wrap_cg_regs = { > + .set_ofs = 0xe08, > + .clr_ofs = 0xe04, > + .sta_ofs = 0xe00, > +}; > + > +#define GATE_IMP_IIC_WRAP(_id, _name, _parent, _shift) \ > + GATE_MTK_FLAGS(_id, _name, _parent, &imp_iic_wrap_cg_regs, _shift, \ > + &mtk_clk_gate_ops_setclr, CLK_OPS_PARENT_ENABLE) > + > +static const struct mtk_gate imp_iic_wrap_s_clks[] = { > + GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_S_I2C5, "imp_iic_wrap_s_i2c5", "i2c_sel", 0), > + GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_S_I2C6, "imp_iic_wrap_s_i2c6", "i2c_sel", 1), > + GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_S_I2C7, "imp_iic_wrap_s_i2c7", "i2c_sel", 2), > +}; > + > +static const struct mtk_gate imp_iic_wrap_w_clks[] = { > + GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_W_I2C0, "imp_iic_wrap_w_i2c0", "i2c_sel", 0), > + GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_W_I2C1, "imp_iic_wrap_w_i2c1", "i2c_sel", 1), > + GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_W_I2C2, "imp_iic_wrap_w_i2c2", "i2c_sel", 2), > + GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_W_I2C3, "imp_iic_wrap_w_i2c3", "i2c_sel", 3), > + GATE_IMP_IIC_WRAP(CLK_IMP_IIC_WRAP_W_I2C4, "imp_iic_wrap_w_i2c4", "i2c_sel", 4), The datasheet doesn't provide the actual index numbers for each bit, but based on the address range groupings I'd say the numbering here is reasonable. ChenYu > +}; > + > +static const struct mtk_clk_desc imp_iic_wrap_s_desc = { > + .clks = imp_iic_wrap_s_clks, > + .num_clks = ARRAY_SIZE(imp_iic_wrap_s_clks), > +}; > + > +static const struct mtk_clk_desc imp_iic_wrap_w_desc = { > + .clks = imp_iic_wrap_w_clks, > + .num_clks = ARRAY_SIZE(imp_iic_wrap_w_clks), > +}; > + > +static const struct of_device_id of_match_clk_mt8195_imp_iic_wrap[] = { > + { > + .compatible = "mediatek,mt8195-imp_iic_wrap_s", > + .data = &imp_iic_wrap_s_desc, > + }, { > + .compatible = "mediatek,mt8195-imp_iic_wrap_w", > + .data = &imp_iic_wrap_w_desc, > + }, { > + /* sentinel */ > + } > +}; > + > +static struct platform_driver clk_mt8195_imp_iic_wrap_drv = { > + .probe = mtk_clk_simple_probe, > + .driver = { > + .name = "clk-mt8195-imp_iic_wrap", > + .of_match_table = of_match_clk_mt8195_imp_iic_wrap, > + }, > +}; > + > +builtin_platform_driver(clk_mt8195_imp_iic_wrap_drv); > -- > 2.18.0 > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
Hi, On Thu, Jun 17, 2021 at 6:49 AM Chun-Jie Chen <chun-jie.chen@mediatek.com> wrote: > > This patch adds the new binding documentation for system clock > and functional clock on Mediatek MT8195. > > Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> > --- > .../arm/mediatek/mediatek,mt8195-clock.yaml | 287 ++++++++++++++++++ > .../mediatek/mediatek,mt8195-sys-clock.yaml | 66 ++++ > 2 files changed, 353 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-clock.yaml > create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-sys-clock.yaml > > diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-clock.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-clock.yaml > new file mode 100644 > index 000000000000..21554b3515cf > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-clock.yaml > @@ -0,0 +1,287 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/arm/mediatek/mediatek,mt8195-clock.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: MediaTek Functional Clock Controller for MT8195 > + > +maintainers: > + - Chun-Jie Chen <chun-jie.chen@mediatek.com> > + > +description: > + The Mediatek functional clock controller provides various clocks on MT8195. > + > +properties: > + compatible: > + oneOf: > + - items: > + - enum: > + - mediatek,mt8195-nnasys > + - mediatek,mt8195-scp_adsp > + - mediatek,mt8195-audsys > + - mediatek,mt8195-audsys_src > + - mediatek,mt8195-imp_iic_wrap_s > + - mediatek,mt8195-imp_iic_wrap_w > + - mediatek,mt8195-mfgcfg > + - mediatek,mt8195-vppsys0 > + - mediatek,mt8195-wpesys > + - mediatek,mt8195-wpesys_vpp0 > + - mediatek,mt8195-wpesys_vpp1 > + - mediatek,mt8195-vppsys1 > + - mediatek,mt8195-imgsys > + - mediatek,mt8195-imgsys1_dip_top > + - mediatek,mt8195-imgsys1_dip_nr > + - mediatek,mt8195-imgsys1_wpe > + - mediatek,mt8195-ipesys > + - mediatek,mt8195-camsys > + - mediatek,mt8195-camsys_rawa > + - mediatek,mt8195-camsys_yuva > + - mediatek,mt8195-camsys_rawb > + - mediatek,mt8195-camsys_yuvb > + - mediatek,mt8195-camsys_mraw > + - mediatek,mt8195-ccusys > + - mediatek,mt8195-vdecsys_soc > + - mediatek,mt8195-vdecsys > + - mediatek,mt8195-vdecsys_core1 > + - mediatek,mt8195-apusys_pll > + - mediatek,mt8195-vencsys > + - mediatek,mt8195-vencsys_core1 > + - mediatek,mt8195-vdosys0 > + - mediatek,mt8195-vdosys1 > + reg: > + maxItems: 1 > + > + '#clock-cells': > + const: 1 > + > +required: > + - compatible > + - reg > + > +additionalProperties: false I think this really needs to describe some of the clock relations between the various clock controllers. For example, both "mediatek,mt8195-imp_iic_wrap_s" and "mediatek,mt8195-imp_iic_wrap_w" take the CLK_TOP_I2C_SEL clock from "mediatek,mt8195-topckgen", but it is not described. > + > +examples: > + - | > + nnasys: clock-controller@10211000 { > + compatible = "mediatek,mt8195-nnasys"; > + reg = <0x10211000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + scp_adsp: clock-controller@10720000 { > + compatible = "mediatek,mt8195-scp_adsp"; > + reg = <0x10720000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + audsys: clock-controller@10890000 { > + compatible = "mediatek,mt8195-audsys"; > + reg = <0x10890000 0x10000>; > + #clock-cells = <1>; > + }; > + > + - | > + audsys_src: clock-controller@108a0000 { > + compatible = "mediatek,mt8195-audsys_src"; > + reg = <0x108a0000 0x2000>; > + #clock-cells = <1>; > + }; > + > + - | > + imp_iic_wrap_s: clock-controller@11d03000 { > + compatible = "mediatek,mt8195-imp_iic_wrap_s"; > + reg = <0x11d03000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + imp_iic_wrap_w: clock-controller@11e05000 { > + compatible = "mediatek,mt8195-imp_iic_wrap_w"; > + reg = <0x11e05000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + mfgcfg: clock-controller@13fbf000 { > + compatible = "mediatek,mt8195-mfgcfg"; > + reg = <0x13fbf000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vppsys0: clock-controller@14000000 { > + compatible = "mediatek,mt8195-vppsys0"; > + reg = <0x14000000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + wpesys: clock-controller@14e00000 { > + compatible = "mediatek,mt8195-wpesys"; > + reg = <0x14e00000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + wpesys_vpp0: clock-controller@14e02000 { > + compatible = "mediatek,mt8195-wpesys_vpp0"; > + reg = <0x14e02000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + wpesys_vpp1: clock-controller@14e03000 { > + compatible = "mediatek,mt8195-wpesys_vpp1"; > + reg = <0x14e03000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vppsys1: clock-controller@14f00000 { > + compatible = "mediatek,mt8195-vppsys1"; > + reg = <0x14f00000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + imgsys: clock-controller@15000000 { > + compatible = "mediatek,mt8195-imgsys"; > + reg = <0x15000000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + imgsys1_dip_top: clock-controller@15110000 { > + compatible = "mediatek,mt8195-imgsys1_dip_top"; > + reg = <0x15110000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + imgsys1_dip_nr: clock-controller@15130000 { > + compatible = "mediatek,mt8195-imgsys1_dip_nr"; > + reg = <0x15130000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + imgsys1_wpe: clock-controller@15220000 { > + compatible = "mediatek,mt8195-imgsys1_wpe"; > + reg = <0x15220000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + ipesys: clock-controller@15330000 { > + compatible = "mediatek,mt8195-ipesys"; > + reg = <0x15330000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + camsys: clock-controller@16000000 { > + compatible = "mediatek,mt8195-camsys"; > + reg = <0x16000000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + camsys_rawa: clock-controller@1604f000 { > + compatible = "mediatek,mt8195-camsys_rawa"; > + reg = <0x1604f000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + camsys_yuva: clock-controller@1606f000 { > + compatible = "mediatek,mt8195-camsys_yuva"; > + reg = <0x1606f000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + camsys_rawb: clock-controller@1608f000 { > + compatible = "mediatek,mt8195-camsys_rawb"; > + reg = <0x1608f000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + camsys_yuvb: clock-controller@160af000 { > + compatible = "mediatek,mt8195-camsys_yuvb"; > + reg = <0x160af000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + camsys_mraw: clock-controller@16140000 { > + compatible = "mediatek,mt8195-camsys_mraw"; > + reg = <0x16140000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + ccusys: clock-controller@17200000 { > + compatible = "mediatek,mt8195-ccusys"; > + reg = <0x17200000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vdecsys_soc: clock-controller@1800f000 { > + compatible = "mediatek,mt8195-vdecsys_soc"; > + reg = <0x1800f000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vdecsys: clock-controller@1802f000 { > + compatible = "mediatek,mt8195-vdecsys"; > + reg = <0x1802f000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vdecsys_core1: clock-controller@1803f000 { > + compatible = "mediatek,mt8195-vdecsys_core1"; > + reg = <0x1803f000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + apusys_pll: clock-controller@190f3000 { > + compatible = "mediatek,mt8195-apusys_pll"; > + reg = <0x190f3000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vencsys: clock-controller@1a000000 { > + compatible = "mediatek,mt8195-vencsys"; > + reg = <0x1a000000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vencsys_core1: clock-controller@1b000000 { > + compatible = "mediatek,mt8195-vencsys_core1"; > + reg = <0x1b000000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vdosys0: clock-controller@1c01a000 { > + compatible = "mediatek,mt8195-vdosys0"; > + reg = <0x1c01a000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + vdosys1: clock-controller@1c100000 { > + compatible = "mediatek,mt8195-vdosys1"; > + reg = <0x1c100000 0x1000>; > + #clock-cells = <1>; > + }; > diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-sys-clock.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-sys-clock.yaml > new file mode 100644 > index 000000000000..ea379452ba91 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8195-sys-clock.yaml > @@ -0,0 +1,66 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/arm/mediatek/mediatek,mt8195-sys-clock.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: MediaTek System Clock Controller for MT8195 > + > +maintainers: > + - Chun-Jie Chen <chun-jie.chen@mediatek.com> > + > +description: > + The Mediatek system clock controller provides various clocks and system configuration > + like reset and bus protection on MT8195. > + > +properties: > + compatible: > + oneOf: > + - items: > + - enum: > + - mediatek,mt8195-topckgen > + - mediatek,mt8195-infracfg_ao > + - mediatek,mt8195-apmixedsys > + - mediatek,mt8195-pericfg_ao > + - const: syscon > + > + reg: > + maxItems: 1 > + > + '#clock-cells': > + const: 1 > + > +required: > + - compatible > + - reg > + > +additionalProperties: false Same thing for "mediatek,mt8195-topckgen". This clock controller takes a bunch of the PLLs from "mediatek,mt8195-apmixedsys" as well as an external oscillator, and muxes and divides them to produce various clock ouptuts. These clock parents are not described. In the external oscillator's case, the oscillator is described in the device tree, but the clock parent relationship is described in the clock driver in this series with a hard-coded clock name. This really applies to all clock controllers. Unless it includes some internal oscillator, it will always have some clock input used to produce other clock outputs. Regards ChenYu > + > +examples: > + - | > + topckgen: syscon@10000000 { > + compatible = "mediatek,mt8195-topckgen", "syscon"; > + reg = <0x10000000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + infracfg_ao: syscon@10001000 { > + compatible = "mediatek,mt8195-infracfg_ao", "syscon"; > + reg = <0x10001000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + apmixedsys: syscon@1000c000 { > + compatible = "mediatek,mt8195-apmixedsys", "syscon"; > + reg = <0x1000c000 0x1000>; > + #clock-cells = <1>; > + }; > + > + - | > + pericfg_ao: syscon@11003000 { > + compatible = "mediatek,mt8195-pericfg_ao", "syscon"; > + reg = <0x11003000 0x1000>; > + #clock-cells = <1>; > + }; > -- > 2.18.0 > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Tue, 2021-07-06 at 17:00 +0800, Chen-Yu Tsai wrote: > On Thu, Jun 17, 2021 at 6:59 AM Chun-Jie Chen > <chun-jie.chen@mediatek.com> wrote: > > > > Add MT8195 ccusys clock provider > > > > Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com> > > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> > > Also, I noticed that Mediatek drivers don't support the reset > controls found > in these clock controllers. Are there plans to add support for them? At present, we have no plan to support reset control in clock controllers because no request from sub system, but we have reset controller in MT8195 from TOPRGU [1]. [1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=521219 Best Regards, Chun-Jie