switch to MFD device for MediaTek audio subsystem
Subsystems (3)
v3 cover switch to MFD device for MediaTek audio subsystem
Hi,
This series switches the clock driver to adapt the new MFD parent. I still keep the previous version of the bindings (patch 4/5) for discussion.
Rob, let me know if there is any problem with this.
changes since v3:
- Rebase to v4.16.
- Modify clock driver for the sake of the backward compatibility.
- Rewrite commit message of patch 4/5 to make it more specific.
changes since v2:
- Drop useless changes in clk-mt7622-aud.c.
- Revise binding text:
- Add more information about audio subsystem.
- Separate clock node and AFE node.
- Update license header.
changes since v1:
- To avoid writing an MFD driver, we add "simple-mfd" in the audsys binding.
- Move three top clocks to audio driver [1] as we remove mfd/mtk-audsys.c in v1.
Ryder Lee (5):
clk: mediatek: update missing clock data for MT7622 audsys
clk: mediatek: modify MT7622 audsys to adapt MFD device
clk: mediatek: add audsys support for MT2701
dt-bindings: clock: mediatek: update audsys documentation to adapt MFD
device
arm: dts: mediatek: add audio-subsystem node for both MT2701 and
MT7623
.../bindings/arm/mediatek/mediatek,audsys.txt | 37 +++- arch/arm/boot/dts/mt2701.dtsi | 192 ++++++++++---------- arch/arm/boot/dts/mt7623.dtsi | 193 ++++++++++----------- drivers/clk/mediatek/Kconfig | 6 + drivers/clk/mediatek/Makefile | 1 + drivers/clk/mediatek/clk-mt2701-aud.c | 179 +++++++++++++++++++ drivers/clk/mediatek/clk-mt7622-aud.c | 8 +- include/dt-bindings/clock/mt7622-clk.h | 3 +- 8 files changed, 414 insertions(+), 205 deletions(-) create mode 100644 drivers/clk/mediatek/clk-mt2701-aud.c
Signature · 1 line
1.9.1
PATCH v3 1/5 clk: mediatek: update missing clock data for MT7622 audsys
Add missing clock data 'CLK_AUDIO_AFE_CONN' for MT7622 audsys.
--- drivers/clk/mediatek/clk-mt7622-aud.c | 1 + include/dt-bindings/clock/mt7622-clk.h | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-)
drivers/clk/mediatek/clk-mt7622-aud.c +1 -0
@@ -106,6 +106,7 @@GATE_AUDIO1 (CLK_AUDIO_INTDIR ,"audio_intdir" ,"intdir_sel" ,20 ),GATE_AUDIO1 (CLK_AUDIO_A1SYS ,"audio_a1sys" ,"a1sys_hp_sel" ,21 ),GATE_AUDIO1 (CLK_AUDIO_A2SYS ,"audio_a2sys" ,"a2sys_hp_sel" ,22 ), +GATE_AUDIO1 (CLK_AUDIO_AFE_CONN ,"audio_afe_conn" ,"a1sys_hp_sel" ,23 ),/* AUDIO2 */ GATE_AUDIO2 (CLK_AUDIO_UL1 ,"audio_ul1" ,"a1sys_hp_sel" ,0 ), GATE_AUDIO2(CLK_AUDIO_UL2, "audio_ul2", "a1sys_hp_sel", 1),
include/dt-bindings/clock/mt7622-clk.h +2 -1
@@ -235,7 +235,8 @@#define CLK_AUDIO_MEM_ASRC3 43#define CLK_AUDIO_MEM_ASRC4 44#define CLK_AUDIO_MEM_ASRC5 45 -#define CLK_AUDIO_NR_CLK 46 +#define CLK_AUDIO_AFE_CONN 46 +#define CLK_AUDIO_NR_CLK 47/* SSUSBSYS */
Signature · 1 line
1.9.1
Re: [PATCH v3 1/5] clk: mediatek: update missing clock data for MT7622 audsys
Quoted 8 lines · Ryder Lee
Add missing clock data '
CLK_AUDIO_AFE_CONN' for MT7622 audsys.Signed-off-by Ryder Lee <[email protected]>Reviewed-by Rob Herring <[email protected]>--- drivers/clk/mediatek/clk-mt7622-aud.c | 1 + include/dt-bindings/clock/mt7622-clk.h | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-)>
Quoted 27 lines
drivers/clk/mediatek/clk-mt7622-aud.c +1
-0@@ -106,6 +106,7 @@GATE_AUDIO1 (CLK_AUDIO_INTDIR ,"audio_intdir" ,"intdir_sel" ,20 ),GATE_AUDIO1 (CLK_AUDIO_A1SYS ,"audio_a1sys" ,"a1sys_hp_sel" ,21 ),GATE_AUDIO1 (CLK_AUDIO_A2SYS ,"audio_a2sys" ,"a2sys_hp_sel" ,22 ), +GATE_AUDIO1 (CLK_AUDIO_AFE_CONN ,"audio_afe_conn" ,"a1sys_hp_sel" ,23 ),/* AUDIO2 */ GATE_AUDIO2 (CLK_AUDIO_UL1 ,"audio_ul1" ,"a1sys_hp_sel" ,0 ), GATE_AUDIO2(CLK_AUDIO_UL2, "audio_ul2", "a1sys_hp_sel", 1),include/dt-bindings/clock/mt7622-clk.h +2
-1@@ -235,7 +235,8 @@#define CLK_AUDIO_MEM_ASRC3 43#define CLK_AUDIO_MEM_ASRC4 44#define CLK_AUDIO_MEM_ASRC5 45 -#define CLK_AUDIO_NR_CLK 46 +#define CLK_AUDIO_AFE_CONN 46 +#define CLK_AUDIO_NR_CLK 47/* SSUSBSYS */
PATCH v3 2/5 clk: mediatek: modify MT7622 audsys to adapt MFD device
In reply to Ryder LeeAs the new MFD device is in place, switch probing method to adapt it.
--- drivers/clk/mediatek/clk-mt7622-aud.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/clk/mediatek/clk-mt7622-aud.c +6 -1
@@ -142,11 +142,16 @@ static int clk_mt7622_audiosys_init(struct platform_device *pdev) {struct clk_onecell_data *clk_data;struct device_node *node = pdev->dev.of_node; +struct device_node *pnode = pdev->dev.parent->of_node;int r; clk_data =mtk_alloc_clk_data (CLK_AUDIO_NR_CLK ); -mtk_clk_register_gates (node, audio_clks,ARRAY_SIZE (audio_clks), +/* Check if called from MFD */ +if (!pnode) + pnode = node; + +mtk_clk_register_gates (pnode, audio_clks,ARRAY_SIZE (audio_clks), clk_data); r =of_clk_add_provider (node, of_clk_src_onecell_get, clk_data);
Signature · 1 line
1.9.1
Matthias Brugger
maintainerRe: [PATCH v3 2/5] clk: mediatek: modify MT7622 audsys to adapt MFD device
Quoted 14 lines · Ryder Lee
As the new MFD device is in place, switch probing method to adapt it.
Signed-off-by Ryder Lee <[email protected]>--- drivers/clk/mediatek/clk-mt7622-aud.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/clk/mediatek/clk-mt7622-aud.c +0
-0@@ -142,11 +142,16 @@ static int clk_mt7622_audiosys_init(struct platform_device *pdev) {struct clk_onecell_data *clk_data;
drivers/clk/mediatek/clk-mt7622-aud.c static int clk_mt7622_audiosys_init(struct platform_device *pdev) · line 152
Quoted 10 lines
drivers/clk/mediatek/clk-mt7622-aud.c +4
-1struct device_node *node = pdev->dev.of_node; +struct device_node *pnode = pdev->dev.parent->of_node;int r; clk_data =mtk_alloc_clk_data (CLK_AUDIO_NR_CLK ); - mtk_clk_register_gates(node, audio_clks, ARRAY_SIZE(audio_clks), +/* Check if called from MFD */ +if (!pnode) + pnode = node;
This is needed for backwards compatibility, correct?
Please change the comment accordingly. The same holds for mt2701 driver.
Thanks,
Matthias
Quoted 6 lines
drivers/clk/mediatek/clk-mt7622-aud.c +2
-0+ +mtk_clk_register_gates (pnode, audio_clks,ARRAY_SIZE (audio_clks), clk_data); r =of_clk_add_provider (node, of_clk_src_onecell_get, clk_data);
PATCH v3 3/5 clk: mediatek: add audsys support for MT2701
In reply to Ryder LeeAdd clock driver support for MT2701 audsys.
--- drivers/clk/mediatek/Kconfig | 6 ++ drivers/clk/mediatek/Makefile | 1 + drivers/clk/mediatek/clk-mt2701-aud.c | 179 ++++++++++++++++++++++++++++++++++ 3 files changed, 186 insertions(+) create mode 100644 drivers/clk/mediatek/clk-mt2701-aud.c
drivers/clk/mediatek/Kconfig +6 -0
@@ -54,6 +54,12 @@ config COMMON_CLK_MT2701_BDPSYS ---help--- This driver supports MediaTek MT2701 bdpsys clocks. +config COMMON_CLK_MT2701_AUDSYS + bool "Clock driver for Mediatek MT2701 audsys" + depends on COMMON_CLK_MT2701 + ---help--- + This driver supports Mediatek MT2701 audsys clocks. + config COMMON_CLK_MT2712 bool "Clock driver for MediaTek MT2712" depends on (ARCH_MEDIATEK && ARM64) || COMPILE_TEST
drivers/clk/mediatek/Makefile +1 -0
@@ -6,6 +6,7 @@ obj-$(CONFIG_COMMON_CLK_MT6797_MMSYS) += clk-mt6797-mm.o obj-$(CONFIG_COMMON_CLK_MT6797_VDECSYS) += clk-mt6797-vdec.o obj-$(CONFIG_COMMON_CLK_MT6797_VENCSYS) += clk-mt6797-venc.o obj-$(CONFIG_COMMON_CLK_MT2701) += clk-mt2701.o +obj-$(CONFIG_COMMON_CLK_MT2701_AUDSYS) += clk-mt2701-aud.o obj-$(CONFIG_COMMON_CLK_MT2701_BDPSYS) += clk-mt2701-bdp.o obj-$(CONFIG_COMMON_CLK_MT2701_ETHSYS) += clk-mt2701-eth.o obj-$(CONFIG_COMMON_CLK_MT2701_HIFSYS) += clk-mt2701-hif.o
drivers/clk/mediatek/clk-mt2701-aud.c new file +179 -0
179 lines not on this page · raw message
Signature · 1 line
1.9.1
PATCH v3 4/5 dt-bindings: clock: mediatek: update audsys documentation to adapt MFD device
In reply to Ryder LeeThe MediaTek audio hardware block that exposes functionalities that are handled by separate subsystems in the kernel. These functions are all mapped somewhere at 0x112xxxxx, and there are some control bits are mixed up with other functions within the same registers.
Therefore, it's better to switch the AUDSYS to MFD device for the sake of avoiding adding individual nodes with the overlapping regions for those devices.
This patch adds a top level node "simple-mfd" to represent whole subsystem, which consists of two portions - audio componets and clock part.
--- .../bindings/arm/mediatek/mediatek,audsys.txt | 37 ++++++++++++++++++---- 1 file changed, 30 insertions(+), 7 deletions(-)
Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt +30 -7
@@ -1,22 +1,45 @@ -MediaTek AUDSYS controller +MediaTek Audio Subsystem ============================ +The audio subsystem is one of the multi-function blocks of MediaTek SoCs. +It contains a system controller, which provides a number registers giving +access to two features: AUDSYS clocks and Audio Front End (AFE) components. +For the top level node: +- compatible: must be: "syscon", "simple-mfd"; +- reg: register area of the Audio Subsystem + +Required sub-nodes: + +AUDSYS clocks: +------- The MediaTek AUDSYS controller provides various clocks to the system. Required Properties: - compatible: Should be one of: - - "mediatek,mt7622-audsys", "syscon" + - "mediatek,mt2701-audsys"; + - "mediatek,mt7622-audsys"; - #clock-cells: Must be 1 The AUDSYS controller uses the common clk binding from Documentation/devicetree/bindings/clock/clock-bindings.txt The available clocks are defined in dt-bindings/clock/mt*-clk.h. +AFE components: +------- +For common binding part and usage, refer to +../sonud/mt2701-afe-pcm.txt. + Example: -audsys: audsys at 11220000 { - compatible = "mediatek,mt7622-audsys", "syscon"; - reg = <0 0x11220000 0 0x1000>; - #clock-cells = <1>; -}; + audio-subsystem at 11220000 { + compatible = "syscon", "simple-mfd"; + reg = <0 0x11220000 0 0x2000>; + + audsys: clock { + compatible = "mediatek,mt2701-audsys"; + #clock-cells = <1>; + }; + + ... + };
Signature · 1 line
1.9.1
Re: [PATCH v3 4/5] dt-bindings: clock: mediatek: update audsys documentation to adapt MFD device
Quoted 4 lines · Ryder Lee
The MediaTek audio hardware block that exposes functionalities that are handled by separate subsystems in the kernel. These functions are all mapped somewhere at 0x112xxxxx, and there are some control bits are mixed up with other functions within the same registers.
I still don't think this change is necessary.
Just because a hardware block in DT maps to different subsystems in a particular OS doesn't mean you need a DT node for each OS subsystem. What we have subsystems for changes over time and DT shouldn't really be changing based on that. And DT is not the only way to instantiate drivers.
Rob
[SPAM]Re: [PATCH v3 4/5] dt-bindings: clock: mediatek: update audsys documentation to adapt MFD device
Quoted 6 lines · Rob Herring
Quoted 4 lines · Ryder Lee
The MediaTek audio hardware block that exposes functionalities that are handled by separate subsystems in the kernel. These functions are all mapped somewhere at 0x112xxxxx, and there are some control bits are mixed up with other functions within the same registers.
Quoted 8 lines
I still don't think this change is necessary.
Just because a hardware block in DT maps to different subsystems in a particular OS doesn't mean you need a DT node for each OS subsystem. What we have subsystems for changes over time and DT shouldn't really be changing based on that. And DT is not the only way to instantiate drivers.
Apart right now we have the definition of both functions. The other location is here:../sonud/mt2701-afe-pcm.txt. The ways I could come up with are:
1. Add a dummy MFD driver (need to think a new compatible or just use '*-audsys' which has already been picked by clock driver) to instantiate two sub-devices through id_table and mfd_cell.
2. For the sake of simplification - add a new compatible "simple-mfd".
3. The last thing - keep two nodes separated/independent. (x)
I'm not sure which one is better.
@Lee @Matthias: What do you suggest?
Ryder.
[SPAM]Re: [PATCH v3 4/5] dt-bindings: clock: mediatek: update audsys documentation to adapt MFD device
Quoted 16 lines · Ryder Lee
Quoted 6 lines · Rob Herring
Quoted 4 lines · Ryder Lee
The MediaTek audio hardware block that exposes functionalities that are handled by separate subsystems in the kernel. These functions are all mapped somewhere at 0x112xxxxx, and there are some control bits are mixed up with other functions within the same registers.
Quoted 8 lines
I still don't think this change is necessary.
Just because a hardware block in DT maps to different subsystems in a particular OS doesn't mean you need a DT node for each OS subsystem. What we have subsystems for changes over time and DT shouldn't really be changing based on that. And DT is not the only way to instantiate drivers.
Quoted 3 lines
Apart right now we have the definition of both functions. The other location is here:../sonud/mt2701-afe-pcm.txt. The ways I could come up with are:
There are several problems you need to fix. First, "mediatek,mt2701-audsys" is not documented. It is only used in the example. Second, bindings/arm/mediatek/mediatek,audsys.txt should move to bindings/sound/ if it is only audio related functions. Or perhaps just combine the 2 documents because it is all inconsistent currently.
The 2 documents are inconsistent as to what is the relationship of -audsys and -audio (afe) nodes. mt2701-afe-pcm.txt shows that the AFE is already a child of -audsys. The -audsys node should have #clock-cells. It should also not be a simple-mfd (another inconsistency in the binding) because it needs to probe first to provide clocks to child nodes, and then trigger probing the child nodes.
Rob
Quoted 18 lines
1. Add a dummy MFD driver (need to think a new compatible or just use '*-audsys' which has already been picked by clock driver) to instantiate two sub-devices through
id_tableandmfd_cell.2. For the sake of simplification - add a new compatible "simple-mfd".
3. The last thing - keep two nodes separated/independent. (x)
I'm not sure which one is better.
@Lee @Matthias: What do you suggest?
Ryder.
[SPAM]Re: [PATCH v3 4/5] dt-bindings: clock: mediatek: update audsys documentation to adapt MFD device
Quoted 21 lines · Rob Herring
Quoted 16 lines · Ryder Lee
Quoted 6 lines · Rob Herring
Quoted 4 lines · Ryder Lee
The MediaTek audio hardware block that exposes functionalities that are handled by separate subsystems in the kernel. These functions are all mapped somewhere at 0x112xxxxx, and there are some control bits are mixed up with other functions within the same registers.
Quoted 8 lines
I still don't think this change is necessary.
Just because a hardware block in DT maps to different subsystems in a particular OS doesn't mean you need a DT node for each OS subsystem. What we have subsystems for changes over time and DT shouldn't really be changing based on that. And DT is not the only way to instantiate drivers.
Quoted 3 lines
Apart right now we have the definition of both functions. The other location is here:../sonud/mt2701-afe-pcm.txt. The ways I could come up with are:
Quoted 5 lines
There are several problems you need to fix. First, "mediatek,mt2701-audsys" is not documented. It is only used in the example. Second,
bindings/arm/mediatek/mediatek,audsys.txtshould move to bindings/sound/ if it is only audio related functions. Or perhaps just combine the 2 documents because it is all inconsistent currently.
Because the series crossed subsystems but didn't apply at the same time.
Quoted 7 lines
The 2 documents are inconsistent as to what is the relationship of -audsys and -audio (afe) nodes. mt2701-afe-pcm.txt shows that the AFE is already a child of -audsys. The -audsys node should have #clock-cells. It should also not be a simple-mfd (another inconsistency in the binding) because it needs to probe first to provide clocks to child nodes, and then trigger probing the child nodes.
This is the 1st version I sent before, and the clock parts still under review :( . But yes, the 2 inconsistent documents should be fixed - this may depend on what we end up doing with the DT appearance.
IMHO, apart from overlapping regions with other functions I didn't see any difference between audsys and other clock drivers (providers).
For the sake of uniformity, I make the 2 sub-devices parallel and move "simple-mfd" to the top, and the sequences should actually be handled through "probe deferral mechanism" - that would make this kind of situations much easier to manage.
BTW, I could make the AFE driver be instantiated/probed from the clock driver but this seems superfluous to me. Just make sure is this what you want?
Ryder
[SPAM]Re: [PATCH v3 4/5] dt-bindings: clock: mediatek: update audsys documentation to adapt MFD device
Quoted 42 lines · Ryder Lee
Quoted 21 lines · Rob Herring
Quoted 16 lines · Ryder Lee
Quoted 6 lines · Rob Herring
Quoted 4 lines · Ryder Lee
The MediaTek audio hardware block that exposes functionalities that are handled by separate subsystems in the kernel. These functions are all mapped somewhere at 0x112xxxxx, and there are some control bits are mixed up with other functions within the same registers.
Quoted 8 lines
I still don't think this change is necessary.
Just because a hardware block in DT maps to different subsystems in a particular OS doesn't mean you need a DT node for each OS subsystem. What we have subsystems for changes over time and DT shouldn't really be changing based on that. And DT is not the only way to instantiate drivers.
Quoted 3 lines
Apart right now we have the definition of both functions. The other location is here:../sonud/mt2701-afe-pcm.txt. The ways I could come up with are:
Quoted 5 lines
There are several problems you need to fix. First, "mediatek,mt2701-audsys" is not documented. It is only used in the example. Second,
bindings/arm/mediatek/mediatek,audsys.txtshould move to bindings/sound/ if it is only audio related functions. Or perhaps just combine the 2 documents because it is all inconsistent currently.Because the series crossed subsystems but didn't apply at the same time.
Quoted 7 lines
The 2 documents are inconsistent as to what is the relationship of -audsys and -audio (afe) nodes. mt2701-afe-pcm.txt shows that the AFE is already a child of -audsys. The -audsys node should have #clock-cells. It should also not be a simple-mfd (another inconsistency in the binding) because it needs to probe first to provide clocks to child nodes, and then trigger probing the child nodes.
This is the 1st version I sent before, and the clock parts still under review :( . But yes, the 2 inconsistent documents should be fixed - this may depend on what we end up doing with the DT appearance.
Quoted 7 lines
IMHO, apart from overlapping regions with other functions I didn't see any difference between audsys and other clock drivers (providers).
For the sake of uniformity, I make the 2 sub-devices parallel and move "simple-mfd" to the top, and the sequences should actually be handled through "probe deferral mechanism" - that would make this kind of situations much easier to manage.
If a child node has a dependency on the parent, probe deferral is not the correct solution. The parent should probe first and control probing of the chlidren. "simple-mfd" is intended for cases where there is no dependency on the parent node.
Quoted 3 lines
BTW, I could make the AFE driver be instantiated/probed from the clock driver but this seems superfluous to me. Just make sure is this what you want?
I would think it would be the other way around. The AFE driver creates some clocks. Whether that's a separate driver or a single driver is a kernel issue that has nothing to do with the binding. IIRC, there's several examples in display controllers and camera interfaces where those drivers are also clock providers.
Rob
[SPAM]Re: [PATCH v3 4/5] dt-bindings: clock: mediatek: update audsys documentation to adapt MFD device
Quoted 51 lines · Rob Herring
Quoted 42 lines · Ryder Lee
Quoted 21 lines · Rob Herring
Quoted 16 lines · Ryder Lee
Quoted 6 lines · Rob Herring
Quoted 4 lines · Ryder Lee
The MediaTek audio hardware block that exposes functionalities that are handled by separate subsystems in the kernel. These functions are all mapped somewhere at 0x112xxxxx, and there are some control bits are mixed up with other functions within the same registers.
Quoted 8 lines
I still don't think this change is necessary.
Just because a hardware block in DT maps to different subsystems in a particular OS doesn't mean you need a DT node for each OS subsystem. What we have subsystems for changes over time and DT shouldn't really be changing based on that. And DT is not the only way to instantiate drivers.
Quoted 3 lines
Apart right now we have the definition of both functions. The other location is here:../sonud/mt2701-afe-pcm.txt. The ways I could come up with are:
Quoted 5 lines
There are several problems you need to fix. First, "mediatek,mt2701-audsys" is not documented. It is only used in the example. Second,
bindings/arm/mediatek/mediatek,audsys.txtshould move to bindings/sound/ if it is only audio related functions. Or perhaps just combine the 2 documents because it is all inconsistent currently.Because the series crossed subsystems but didn't apply at the same time.
Quoted 7 lines
The 2 documents are inconsistent as to what is the relationship of -audsys and -audio (afe) nodes. mt2701-afe-pcm.txt shows that the AFE is already a child of -audsys. The -audsys node should have #clock-cells. It should also not be a simple-mfd (another inconsistency in the binding) because it needs to probe first to provide clocks to child nodes, and then trigger probing the child nodes.
This is the 1st version I sent before, and the clock parts still under review :( . But yes, the 2 inconsistent documents should be fixed - this may depend on what we end up doing with the DT appearance.
Quoted 7 lines
IMHO, apart from overlapping regions with other functions I didn't see any difference between audsys and other clock drivers (providers).
For the sake of uniformity, I make the 2 sub-devices parallel and move "simple-mfd" to the top, and the sequences should actually be handled through "probe deferral mechanism" - that would make this kind of situations much easier to manage.
Quoted 5 lines
If a child node has a dependency on the parent, probe deferral is not the correct solution. The parent should probe first and control probing of the chlidren. "simple-mfd" is intended for cases where there is no dependency on the parent node.
I think the device hierarchy for AUDSYS has to be depicted more appropriately at first before we keep going to discuss the topic.
------- clock-controller - AUDSYS ---------- audio-controller - ------- other controllers not being modeled yet
Currently, AUDSYS just works as a container containing all the devices present within the range reserved for audio relevant function and even partial registers and bits for these devices are mixing within some of the range. And for clock controller, they're just all being used to provide to the audio-controller or other functional devices under AUDSYS internally.
Thus, deferral actually happens between siblings, rather than between parent and child. For example, the probe deferral Ryder said is pointing to that audio-controller needs to wait until all resources like clock resources all in ready state.
I'm not fully sure MFD can be used properly to describe such kind of device hierarchy, but it's really worth having a good way to describe the composition of hardware blocks precisely in device tree and to ease extension device support under AUDSYS for future.
There is also the same thing happened in MMSYS as [1] stated.
[1] https://lkml.org/lkml/2017/11/14/669
Quoted 16 lines
Quoted 3 lines
BTW, I could make the AFE driver be instantiated/probed from the clock driver but this seems superfluous to me. Just make sure is this what you want?
I would think it would be the other way around. The AFE driver creates some clocks. Whether that's a separate driver or a single driver is a kernel issue that has nothing to do with the binding. IIRC, there's several examples in display controllers and camera interfaces where those drivers are also clock providers.
Rob
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
[SPAM]Re: [PATCH v3 4/5] dt-bindings: clock: mediatek: update audsys documentation to adapt MFD device
In reply to Rob HerringQuoted 28 lines · Rob Herring
Quoted 20 lines
Quoted 5 lines
There are several problems you need to fix. First, "mediatek,mt2701-audsys" is not documented. It is only used in the example. Second,
bindings/arm/mediatek/mediatek,audsys.txtshould move to bindings/sound/ if it is only audio related functions. Or perhaps just combine the 2 documents because it is all inconsistent currently.Because the series crossed subsystems but didn't apply at the same time.
Quoted 7 lines
The 2 documents are inconsistent as to what is the relationship of -audsys and -audio (afe) nodes. mt2701-afe-pcm.txt shows that the AFE is already a child of -audsys. The -audsys node should have #clock-cells. It should also not be a simple-mfd (another inconsistency in the binding) because it needs to probe first to provide clocks to child nodes, and then trigger probing the child nodes.
This is the 1st version I sent before, and the clock parts still under review :( . But yes, the 2 inconsistent documents should be fixed - this may depend on what we end up doing with the DT appearance.
Quoted 7 lines
IMHO, apart from overlapping regions with other functions I didn't see any difference between audsys and other clock drivers (providers).
For the sake of uniformity, I make the 2 sub-devices parallel and move "simple-mfd" to the top, and the sequences should actually be handled through "probe deferral mechanism" - that would make this kind of situations much easier to manage.
Quoted 4 lines
If a child node has a dependency on the parent, probe deferral is not the correct solution. The parent should probe first and control probing of the chlidren. "simple-mfd" is intended for cases where there is no dependency on the parent node.
In my opinion, they are just the provider-consumer relationship not the parent-child relationship, and we leverage this mechanism for most CCF users. Please correct me if I'm wrong.
That's why I sent the a new one to separate them (but I forgot to modify mt2701-afe-pcm.txt).
Quoted 9 lines
Quoted 3 lines
BTW, I could make the AFE driver be instantiated/probed from the clock driver but this seems superfluous to me. Just make sure is this what you want?
I would think it would be the other way around. The AFE driver creates some clocks. Whether that's a separate driver or a single driver is a kernel issue that has nothing to do with the binding. IIRC, there's several examples in display controllers and camera interfaces where those drivers are also clock providers.
I've look at some display drivers, but couldn't find a good case. The different is, that the driver I saw provides clocks to other submodules, while the AFE is just a user. So it's odd if we register AFE itself as a clock provider (and use the clocks it generates).
PATCH v3 5/5 arm: dts: mediatek: add audio-subsystem node for both MT2701 and MT7623
In reply to Ryder LeeAdd audio-subsystem node and its clock support for both MT2701/MT7623. Then modify afe node to adapt it.
--- arch/arm/boot/dts/mt2701.dtsi | 192 ++++++++++++++++++++--------------------- arch/arm/boot/dts/mt7623.dtsi | 193 +++++++++++++++++++++--------------------- 2 files changed, 189 insertions(+), 196 deletions(-)
arch/arm/boot/dts/mt2701.dtsi +94 -98
@@ -426,104 +426,100 @@ status = "disabled"; }; - afe: audio-controller at 11220000 { - compatible = "mediatek,mt2701-audio"; - reg = <0 0x11220000 0 0x2000>, - <0 0x112a0000 0 0x20000>; - interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_LOW>, - <GIC_SPI 132 IRQ_TYPE_LEVEL_LOW>; - interrupt-names = "afe", "asys"; - power-domains = <&scpsys MT2701_POWER_DOMAIN_IFR_MSC>; - - clocks = <&infracfg CLK_INFRA_AUDIO>, - <&topckgen CLK_TOP_AUD_MUX1_SEL>, - <&topckgen CLK_TOP_AUD_MUX2_SEL>, - <&topckgen CLK_TOP_AUD_MUX1_DIV>, - <&topckgen CLK_TOP_AUD_MUX2_DIV>, - <&topckgen CLK_TOP_AUD_48K_TIMING>, - <&topckgen CLK_TOP_AUD_44K_TIMING>, - <&topckgen CLK_TOP_AUDPLL_MUX_SEL>, - <&topckgen CLK_TOP_APLL_SEL>, - <&topckgen CLK_TOP_AUD1PLL_98M>, - <&topckgen CLK_TOP_AUD2PLL_90M>, - <&topckgen CLK_TOP_HADDS2PLL_98M>, - <&topckgen CLK_TOP_HADDS2PLL_294M>, - <&topckgen CLK_TOP_AUDPLL>, - <&topckgen CLK_TOP_AUDPLL_D4>, - <&topckgen CLK_TOP_AUDPLL_D8>, - <&topckgen CLK_TOP_AUDPLL_D16>, - <&topckgen CLK_TOP_AUDPLL_D24>, - <&topckgen CLK_TOP_AUDINTBUS_SEL>, - <&clk26m>, - <&topckgen CLK_TOP_SYSPLL1_D4>, - <&topckgen CLK_TOP_AUD_K1_SRC_SEL>, - <&topckgen CLK_TOP_AUD_K2_SRC_SEL>, - <&topckgen CLK_TOP_AUD_K3_SRC_SEL>, - <&topckgen CLK_TOP_AUD_K4_SRC_SEL>, - <&topckgen CLK_TOP_AUD_K5_SRC_SEL>, - <&topckgen CLK_TOP_AUD_K6_SRC_SEL>, - <&topckgen CLK_TOP_AUD_K1_SRC_DIV>, - <&topckgen CLK_TOP_AUD_K2_SRC_DIV>, - <&topckgen CLK_TOP_AUD_K3_SRC_DIV>, - <&topckgen CLK_TOP_AUD_K4_SRC_DIV>, - <&topckgen CLK_TOP_AUD_K5_SRC_DIV>, - <&topckgen CLK_TOP_AUD_K6_SRC_DIV>, - <&topckgen CLK_TOP_AUD_I2S1_MCLK>, - <&topckgen CLK_TOP_AUD_I2S2_MCLK>, - <&topckgen CLK_TOP_AUD_I2S3_MCLK>, - <&topckgen CLK_TOP_AUD_I2S4_MCLK>, - <&topckgen CLK_TOP_AUD_I2S5_MCLK>, - <&topckgen CLK_TOP_AUD_I2S6_MCLK>, - <&topckgen CLK_TOP_ASM_M_SEL>, - <&topckgen CLK_TOP_ASM_H_SEL>, - <&topckgen CLK_TOP_UNIVPLL2_D4>, - <&topckgen CLK_TOP_UNIVPLL2_D2>, - <&topckgen CLK_TOP_SYSPLL_D5>; - - clock-names = "infra_sys_audio_clk", - "top_audio_mux1_sel", - "top_audio_mux2_sel", - "top_audio_mux1_div", - "top_audio_mux2_div", - "top_audio_48k_timing", - "top_audio_44k_timing", - "top_audpll_mux_sel", - "top_apll_sel", - "top_aud1_pll_98M", - "top_aud2_pll_90M", - "top_hadds2_pll_98M", - "top_hadds2_pll_294M", - "top_audpll", - "top_audpll_d4", - "top_audpll_d8", - "top_audpll_d16", - "top_audpll_d24", - "top_audintbus_sel", - "clk_26m", - "top_syspll1_d4", - "top_aud_k1_src_sel", - "top_aud_k2_src_sel", - "top_aud_k3_src_sel", - "top_aud_k4_src_sel", - "top_aud_k5_src_sel", - "top_aud_k6_src_sel", - "top_aud_k1_src_div", - "top_aud_k2_src_div", - "top_aud_k3_src_div", - "top_aud_k4_src_div", - "top_aud_k5_src_div", - "top_aud_k6_src_div", - "top_aud_i2s1_mclk", - "top_aud_i2s2_mclk", - "top_aud_i2s3_mclk", - "top_aud_i2s4_mclk", - "top_aud_i2s5_mclk", - "top_aud_i2s6_mclk", - "top_asm_m_sel", - "top_asm_h_sel", - "top_univpll2_d4", - "top_univpll2_d2", - "top_syspll_d5"; + audio-subsystem at 11220000 { + compatible = "syscon", "simple-mfd"; + reg = <0 0x11220000 0 0x2000>; + + audsys: clock { + compatible = "mediatek,mt2701-audsys"; + #clock-cells = <1>; + }; + + afe: audio { + compatible = "mediatek,mt2701-audio"; + interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_LOW>, + <GIC_SPI 132 IRQ_TYPE_LEVEL_LOW>; + interrupt-names = "afe", "asys"; + power-domains = <&scpsys MT2701_POWER_DOMAIN_IFR_MSC>; + + clocks = <&infracfg CLK_INFRA_AUDIO>, + <&topckgen CLK_TOP_AUD_MUX1_SEL>, + <&topckgen CLK_TOP_AUD_MUX2_SEL>, + <&topckgen CLK_TOP_AUD_48K_TIMING>, + <&topckgen CLK_TOP_AUD_44K_TIMING>, + <&topckgen CLK_TOP_AUD_K1_SRC_SEL>, + <&topckgen CLK_TOP_AUD_K2_SRC_SEL>, + <&topckgen CLK_TOP_AUD_K3_SRC_SEL>, + <&topckgen CLK_TOP_AUD_K4_SRC_SEL>, + <&topckgen CLK_TOP_AUD_K1_SRC_DIV>, + <&topckgen CLK_TOP_AUD_K2_SRC_DIV>, + <&topckgen CLK_TOP_AUD_K3_SRC_DIV>, + <&topckgen CLK_TOP_AUD_K4_SRC_DIV>, + <&topckgen CLK_TOP_AUD_I2S1_MCLK>, + <&topckgen CLK_TOP_AUD_I2S2_MCLK>, + <&topckgen CLK_TOP_AUD_I2S3_MCLK>, + <&topckgen CLK_TOP_AUD_I2S4_MCLK>, + <&audsys CLK_AUD_I2SO1>, + <&audsys CLK_AUD_I2SO2>, + <&audsys CLK_AUD_I2SO3>, + <&audsys CLK_AUD_I2SO4>, + <&audsys CLK_AUD_I2SIN1>, + <&audsys CLK_AUD_I2SIN2>, + <&audsys CLK_AUD_I2SIN3>, + <&audsys CLK_AUD_I2SIN4>, + <&audsys CLK_AUD_ASRCO1>, + <&audsys CLK_AUD_ASRCO2>, + <&audsys CLK_AUD_ASRCO3>, + <&audsys CLK_AUD_ASRCO4>, + <&audsys CLK_AUD_AFE>, + <&audsys CLK_AUD_AFE_CONN>, + <&audsys CLK_AUD_A1SYS>, + <&audsys CLK_AUD_A2SYS>, + <&audsys CLK_AUD_AFE_MRGIF>; + + clock-names = "infra_sys_audio_clk", + "top_audio_mux1_sel", + "top_audio_mux2_sel", + "top_audio_a1sys_hp", + "top_audio_a2sys_hp", + "i2s0_src_sel", + "i2s1_src_sel", + "i2s2_src_sel", + "i2s3_src_sel", + "i2s0_src_div", + "i2s1_src_div", + "i2s2_src_div", + "i2s3_src_div", + "i2s0_mclk_en", + "i2s1_mclk_en", + "i2s2_mclk_en", + "i2s3_mclk_en", + "i2so0_hop_ck", + "i2so1_hop_ck", + "i2so2_hop_ck", + "i2so3_hop_ck", + "i2si0_hop_ck", + "i2si1_hop_ck", + "i2si2_hop_ck", + "i2si3_hop_ck", + "asrc0_out_ck", + "asrc1_out_ck", + "asrc2_out_ck", + "asrc3_out_ck", + "audio_afe_pd", + "audio_afe_conn_pd", + "audio_a1sys_pd", + "audio_a2sys_pd", + "audio_mrgif_pd"; + + assigned-clocks = <&topckgen CLK_TOP_AUD_MUX1_SEL>, + <&topckgen CLK_TOP_AUD_MUX2_SEL>, + <&topckgen CLK_TOP_AUD_MUX1_DIV>, + <&topckgen CLK_TOP_AUD_MUX2_DIV>; + assigned-clock-parents = <&topckgen CLK_TOP_AUD1PLL_98M>, + <&topckgen CLK_TOP_AUD2PLL_90M>; + assigned-clock-rates = <0>, <0>, <49152000>, <45158400>; + }; }; mmsys: syscon at 14000000 {
arch/arm/boot/dts/mt7623.dtsi +95 -98
200 lines not on this page · raw message
Signature · 1 line
1.9.1