| Message ID | 8651585c91ee40f6ab3e30b893bcfb90fe5c2c25.1515639336.git.ryder.lee@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 31, 2018 at 03:42:44PM +0800, Ryder Lee wrote: > As the MediaTek audio hardware block that expose functionalities that are > handled by separate subsystems in the kernel, and there are registers that > are shared between related drivers. > > Switch the current device to an MFD device, add more descriptions about the > subsystem and modify example accordingly. > > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com> > --- > .../bindings/arm/mediatek/mediatek,audsys.txt | 37 ++++++++++++++++++---- > 1 file changed, 30 insertions(+), 7 deletions(-) > > diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt > index 9b8f578..677af40 100644 > --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt > @@ -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"; This should have some SoC specific compatible. > +- 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. There's no register range associated with the clocks? If there is, add a reg property. > > +AFE components: > +------- > +For common binding part and usage, refer to > +../sonud/mt2701-afe-pcm.txt. > + > Example: > > -audsys: audsys@11220000 { > - compatible = "mediatek,mt7622-audsys", "syscon"; > - reg = <0 0x11220000 0 0x1000>; > - #clock-cells = <1>; > -}; > + audio-subsystem@11220000 { > + compatible = "syscon", "simple-mfd"; > + reg = <0 0x11220000 0 0x2000>; > + > + audsys: clock { > + compatible = "mediatek,mt2701-audsys"; > + #clock-cells = <1>; > + }; > + > + ... > + }; > -- > 1.9.1 >
On Mon, 2018-02-05 at 00:08 -0600, Rob Herring wrote: > On Wed, Jan 31, 2018 at 03:42:44PM +0800, Ryder Lee wrote: > > As the MediaTek audio hardware block that expose functionalities that are > > handled by separate subsystems in the kernel, and there are registers that > > are shared between related drivers. > > > > Switch the current device to an MFD device, add more descriptions about the > > subsystem and modify example accordingly. > > > > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com> > > --- > > .../bindings/arm/mediatek/mediatek,audsys.txt | 37 ++++++++++++++++++---- > > 1 file changed, 30 insertions(+), 7 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt > > index 9b8f578..677af40 100644 > > --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt > > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt > > @@ -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"; > > This should have some SoC specific compatible. As we don't have a specific driver (compatible string) for it and if we need to add that I think the term '*-audsys' is very suitable here, but unfortunately, it has already picked for clock driver (see child node). I also took ../../marvell/*-system-controller.txt as examples for my case. Thus, I'm not sure should we still need a new one here? > > +- 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. > > There's no register range associated with the clocks? If there is, add a > reg property. No, we don't need reg property here, as the two sub-drivers will obtain the regmap which is propagated from the parent. Thanks > > > > +AFE components: > > +------- > > +For common binding part and usage, refer to > > +../sonud/mt2701-afe-pcm.txt. > > + > > Example: > > > > -audsys: audsys@11220000 { > > - compatible = "mediatek,mt7622-audsys", "syscon"; > > - reg = <0 0x11220000 0 0x1000>; > > - #clock-cells = <1>; > > -}; > > + audio-subsystem@11220000 { > > + compatible = "syscon", "simple-mfd"; > > + reg = <0 0x11220000 0 0x2000>; > > + > > + audsys: clock { > > + compatible = "mediatek,mt2701-audsys"; > > + #clock-cells = <1>; > > + }; > > + > > + ... > > + }; > > -- > > 1.9.1 > > > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Mon, Feb 5, 2018 at 3:11 AM, Ryder Lee <ryder.lee@mediatek.com> wrote: > On Mon, 2018-02-05 at 00:08 -0600, Rob Herring wrote: >> On Wed, Jan 31, 2018 at 03:42:44PM +0800, Ryder Lee wrote: >> > As the MediaTek audio hardware block that expose functionalities that are >> > handled by separate subsystems in the kernel, and there are registers that >> > are shared between related drivers. >> > >> > Switch the current device to an MFD device, add more descriptions about the >> > subsystem and modify example accordingly. >> > >> > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com> >> > --- >> > .../bindings/arm/mediatek/mediatek,audsys.txt | 37 ++++++++++++++++++---- >> > 1 file changed, 30 insertions(+), 7 deletions(-) >> > >> > diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt >> > index 9b8f578..677af40 100644 >> > --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt >> > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt >> > @@ -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"; >> >> This should have some SoC specific compatible. > > As we don't have a specific driver (compatible string) for it and if we > need to add that I think the term '*-audsys' is very suitable here, but > unfortunately, it has already picked for clock driver (see child node). Perhaps the clocks block should have "-clk" on the end or something. Why do you really need to change this in the first place? You don't have to have DT child nodes to create child (struct) devices and child drivers. > > I also took ../../marvell/*-system-controller.txt as examples for my > case. Thus, I'm not sure should we still need a new one here? > >> > +- 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. >> >> There's no register range associated with the clocks? If there is, add a >> reg property. > > No, we don't need reg property here, as the two sub-drivers will obtain > the regmap which is propagated from the parent. I know regmap doesn't need it. That's not what I asked. If you have a range of registers for the clocks, then define that in reg. Only if the clock control bits are mixed up with other functions within the same registers, then you can omit it. Rob
On Wed, 2018-02-07 at 18:49 -0600, Rob Herring wrote: > >> > diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt > >> > index 9b8f578..677af40 100644 > >> > --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt > >> > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt > >> > @@ -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"; > >> > >> This should have some SoC specific compatible. > > > > As we don't have a specific driver (compatible string) for it and if we > > need to add that I think the term '*-audsys' is very suitable here, but > > unfortunately, it has already picked for clock driver (see child node). > > Perhaps the clocks block should have "-clk" on the end or something. > > Why do you really need to change this in the first place? You don't > have to have DT child nodes to create child (struct) devices and child > drivers. > I'm not sure I get your point. Did you mean that we could have a parent (which represents clock and its compatible is something like "*-audsys","syscon","simple-mfd") with a subnode for audio function? If so, there's no special reason. I just think it more specifically to let people know we have an MFD (top node) and it has two sub-functions (children) share the same region. > >> > +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. > >> > >> There's no register range associated with the clocks? If there is, add a > >> reg property. > > > > No, we don't need reg property here, as the two sub-drivers will obtain > > the regmap which is propagated from the parent. > > I know regmap doesn't need it. That's not what I asked. If you have a > range of registers for the clocks, then define that in reg. Only if > the clock control bits are mixed up with other functions within the > same registers, then you can omit it. > Oh okay, I didn't read the mail well. We have four or five registers for the clocks, but one of them (offset:0x634) is mixed up with the audio function (bit 17-19), so I omit it. Ryder.
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt index 9b8f578..677af40 100644 --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt @@ -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@11220000 { - compatible = "mediatek,mt7622-audsys", "syscon"; - reg = <0 0x11220000 0 0x1000>; - #clock-cells = <1>; -}; + audio-subsystem@11220000 { + compatible = "syscon", "simple-mfd"; + reg = <0 0x11220000 0 0x2000>; + + audsys: clock { + compatible = "mediatek,mt2701-audsys"; + #clock-cells = <1>; + }; + + ... + };
As the MediaTek audio hardware block that expose functionalities that are handled by separate subsystems in the kernel, and there are registers that are shared between related drivers. Switch the current device to an MFD device, add more descriptions about the subsystem and modify example accordingly. Signed-off-by: Ryder Lee <ryder.lee@mediatek.com> --- .../bindings/arm/mediatek/mediatek,audsys.txt | 37 ++++++++++++++++++---- 1 file changed, 30 insertions(+), 7 deletions(-)