| Message ID | 20240228-imx95-blk-ctl-v3-1-40ceba01a211@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support i.MX95 BLK CTL module clock features | expand |
On Wed, Feb 28, 2024 at 03:48:22PM +0800, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > i.MX95 includes BLK CTL module in several MIXes, such as VPU_CSR in > VPUMIX, BLK_CTRL_NETCMIX in NETCMIX, CAMERA_CSR in CAMERAMIX and etc. > > The BLK CTL module is used for various settings of a specific MIX, such > as clock, QoS and etc. > > This patch is to add some BLK CTL modules that has clock features. This sentence doesn't add anything you haven't already said. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > .../devicetree/bindings/clock/imx95-blk-ctl.yaml | 61 ++++++++++++++++++++++ > include/dt-bindings/clock/nxp,imx95-clock.h | 32 ++++++++++++ > 2 files changed, 93 insertions(+) > > diff --git a/Documentation/devicetree/bindings/clock/imx95-blk-ctl.yaml b/Documentation/devicetree/bindings/clock/imx95-blk-ctl.yaml > new file mode 100644 > index 000000000000..c8974b927bee > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/imx95-blk-ctl.yaml > @@ -0,0 +1,61 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/imx95-blk-ctl.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NXP i.MX95 Block Control > + > +maintainers: > + - Peng Fan <peng.fan@nxp.com> > + > +properties: > + compatible: > + items: > + - enum: > + - nxp,imx95-cameramix-csr > + - nxp,imx95-display-master-csr > + - nxp,imx95-dispmix-lvds-csr > + - nxp,imx95-dispmix-csr > + - nxp,imx95-netcmix-blk-ctrl > + - nxp,imx95-vpumix-csr > + - const: syscon > + > + reg: > + maxItems: 1 > + > + power-domains: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + '#clock-cells': > + const: 1 > + description: > + The clock consumer should specify the desired clock by having the clock > + ID in its "clocks" phandle cell. See > + include/dt-bindings/clock/nxp,imx95-clock.h > + > + mux-controller: > + type: object > + $ref: /schemas/mux/reg-mux.yaml > + > +required: > + - compatible > + - reg > + - '#clock-cells' > + > +additionalProperties: false > + > +examples: > + # Clock Control Module node: > + - | > + #include <dt-bindings/clock/nxp,imx95-clock.h> > + > + syscon@4c410000 { clock-controller@... As that is the main feature/function. > + compatible = "nxp,imx95-vpumix-csr", "syscon"; > + reg = <0x4c410000 0x10000>; > + #clock-cells = <1>; Please make the example as full as possible. For example, add mux-controller node. Do some of the blocks not have mux ctrl?
On 04/03/2024 15:39, Rob Herring wrote: > > As that is the main feature/function. > >> + compatible = "nxp,imx95-vpumix-csr", "syscon"; >> + reg = <0x4c410000 0x10000>; >> + #clock-cells = <1>; > > Please make the example as full as possible. For example, add > mux-controller node. Do some of the blocks not have mux ctrl? I asked for this in v2... We can keep asking and asking... Best regards, Krzysztof
.org; linux-kernel@vger.kernel.org; Peng Fan > <peng.fan@nxp.com> > Subject: Re: [PATCH v3 1/2] dt-bindindgs: clock: support NXP i.MX95 BLK CTL > module > > On Wed, Feb 28, 2024 at 03:48:22PM +0800, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > i.MX95 includes BLK CTL module in several MIXes, such as VPU_CSR in > > VPUMIX, BLK_CTRL_NETCMIX in NETCMIX, CAMERA_CSR in CAMERAMIX > and etc. > > > > The BLK CTL module is used for various settings of a specific MIX, > > such as clock, QoS and etc. > > > > This patch is to add some BLK CTL modules that has clock features. > > This sentence doesn't add anything you haven't already said. Will drop it. > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > .../devicetree/bindings/clock/imx95-blk-ctl.yaml | 61 > ++++++++++++++++++++++ > > include/dt-bindings/clock/nxp,imx95-clock.h | 32 ++++++++++++ > > 2 files changed, 93 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/clock/imx95-blk-ctl.yaml > > b/Documentation/devicetree/bindings/clock/imx95-blk-ctl.yaml > > new file mode 100644 > > index 000000000000..c8974b927bee > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/imx95-blk-ctl.yaml > > @@ -0,0 +1,61 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > +cetree.org%2Fschemas%2Fclock%2Fimx95-blk- > ctl.yaml%23&data=05%7C02%7Cp > > > +eng.fan%40nxp.com%7Cd0a6445fb5604872ec3a08dc3c58e0e9%7C686ea1 > d3bc2b4c > > > +6fa92cd99c5c301635%7C0%7C0%7C638451599621992781%7CUnknown% > 7CTWFpbGZsb > > > +3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn > 0%3D > > > +%7C0%7C%7C%7C&sdata=DJA1dYKc3f9Q5S%2Fa20O%2BJWgQU%2FsMiskY > RIykKzCRUok > > +%3D&reserved=0 > > +$schema: > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > +cetree.org%2Fmeta- > schemas%2Fcore.yaml%23&data=05%7C02%7Cpeng.fan%40nx > > > +p.com%7Cd0a6445fb5604872ec3a08dc3c58e0e9%7C686ea1d3bc2b4c6fa9 > 2cd99c5c > > > +301635%7C0%7C0%7C638451599622001649%7CUnknown%7CTWFpbGZs > b3d8eyJWIjoiM > > > +C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7 > C%7C%7 > > > +C&sdata=RvMgj7vtwJ3WMYsD3gEO9U8ZI2fRsPy6WVYhCOJ0EfI%3D&reserv > ed=0 > > + > > +title: NXP i.MX95 Block Control > > + > > +maintainers: > > + - Peng Fan <peng.fan@nxp.com> > > + > > +properties: > > + compatible: > > + items: > > + - enum: > > + - nxp,imx95-cameramix-csr > > + - nxp,imx95-display-master-csr > > + - nxp,imx95-dispmix-lvds-csr > > + - nxp,imx95-dispmix-csr > > + - nxp,imx95-netcmix-blk-ctrl > > + - nxp,imx95-vpumix-csr > > + - const: syscon > > + > > + reg: > > + maxItems: 1 > > + > > + power-domains: > > + maxItems: 1 > > + > > + clocks: > > + maxItems: 1 > > + > > + '#clock-cells': > > + const: 1 > > + description: > > + The clock consumer should specify the desired clock by having the > clock > > + ID in its "clocks" phandle cell. See > > + include/dt-bindings/clock/nxp,imx95-clock.h > > + > > + mux-controller: > > + type: object > > + $ref: /schemas/mux/reg-mux.yaml > > + > > +required: > > + - compatible > > + - reg > > + - '#clock-cells' > > + > > +additionalProperties: false > > + > > +examples: > > + # Clock Control Module node: > > + - | > > + #include <dt-bindings/clock/nxp,imx95-clock.h> > > + > > + syscon@4c410000 { > > clock-controller@... But this is a syscon, using clock-controller will trigger dt check warning. > > As that is the main feature/function. > > > + compatible = "nxp,imx95-vpumix-csr", "syscon"; > > + reg = <0x4c410000 0x10000>; > > + #clock-cells = <1>; > > Please make the example as full as possible. For example, add mux-controller > node. Do some of the blocks not have mux ctrl? Yes. The blk ctrl is not just for clock, some registers has mux ctrl, such as Pixel_link_sel. Thanks, Peng.
On 05/03/2024 05:13, Peng Fan wrote: >>> + >>> +examples: >>> + # Clock Control Module node: >>> + - | >>> + #include <dt-bindings/clock/nxp,imx95-clock.h> >>> + >>> + syscon@4c410000 { >> >> clock-controller@... > > But this is a syscon, using clock-controller will trigger dt > check warning. Which warning? >> >> As that is the main feature/function. >> >>> + compatible = "nxp,imx95-vpumix-csr", "syscon"; >>> + reg = <0x4c410000 0x10000>; >>> + #clock-cells = <1>; >> >> Please make the example as full as possible. For example, add mux-controller >> node. Do some of the blocks not have mux ctrl? > > Yes. The blk ctrl is not just for clock, some registers has mux ctrl, > such as Pixel_link_sel. Then mux-controller should not be allowed for them. Best regards, Krzysztof
> Subject: Re: [PATCH v3 1/2] dt-bindindgs: clock: support NXP i.MX95 BLK CTL > module > > On 05/03/2024 05:13, Peng Fan wrote: > >>> + > >>> +examples: > >>> + # Clock Control Module node: > >>> + - | > >>> + #include <dt-bindings/clock/nxp,imx95-clock.h> > >>> + > >>> + syscon@4c410000 { > >> > >> clock-controller@... > > > > But this is a syscon, using clock-controller will trigger dt check > > warning. > > Which warning? I just recalled that node with syscon in compatible string needs has syscon as node, I maybe wrong. > > >> > >> As that is the main feature/function. > >> > >>> + compatible = "nxp,imx95-vpumix-csr", "syscon"; > >>> + reg = <0x4c410000 0x10000>; > >>> + #clock-cells = <1>; > >> > >> Please make the example as full as possible. For example, add > >> mux-controller node. Do some of the blocks not have mux ctrl? > > > > Yes. The blk ctrl is not just for clock, some registers has mux ctrl, > > such as Pixel_link_sel. > > Then mux-controller should not be allowed for them. You mean I should not add mux-controller under the blk ctrl node? We have such a node in downstream tree, if mux-controller is not allowed, would you please suggest other approaches? display_master_csr: syscon@4ad10000 { compatible = "fsl,imx95-display-master-csr", "syscon"; reg = <0x0 0x4ad10000 0x0 0x10000>; #clock-cells = <1>; clocks = <&scmi_clk IMX95_CLK_CAMAPB>; power-domains = <&scmi_devpd IMX95_PD_CAMERA>; mux: mux-controller { compatible = "mmio-mux"; #mux-control-cells = <1>; mux-reg-masks = <0x4 0x00000001>; /* Pixel_link_sel */ idle-states = <0>; }; }; Thanks, Peng. > > Best regards, > Krzysztof
On 05/03/2024 08:18, Peng Fan wrote: >> Subject: Re: [PATCH v3 1/2] dt-bindindgs: clock: support NXP i.MX95 BLK CTL >> module >> >> On 05/03/2024 05:13, Peng Fan wrote: >>>>> + >>>>> +examples: >>>>> + # Clock Control Module node: >>>>> + - | >>>>> + #include <dt-bindings/clock/nxp,imx95-clock.h> >>>>> + >>>>> + syscon@4c410000 { >>>> >>>> clock-controller@... >>> >>> But this is a syscon, using clock-controller will trigger dt check >>> warning. >> >> Which warning? > > I just recalled that node with syscon in compatible string needs > has syscon as node, I maybe wrong. Just paste the warning, so we can think about it. > >> >>>> >>>> As that is the main feature/function. >>>> >>>>> + compatible = "nxp,imx95-vpumix-csr", "syscon"; >>>>> + reg = <0x4c410000 0x10000>; >>>>> + #clock-cells = <1>; >>>> >>>> Please make the example as full as possible. For example, add >>>> mux-controller node. Do some of the blocks not have mux ctrl? >>> >>> Yes. The blk ctrl is not just for clock, some registers has mux ctrl, >>> such as Pixel_link_sel. >> >> Then mux-controller should not be allowed for them. > > You mean I should not add mux-controller under the blk ctrl node? mux-controller is already there, isn't it? I am saying your binding is not precise. Your binding implies that ALL OF THEM have mux controller. You told me it is not true, so you have change the meaning of binding and disallow the mux-controller for the cases it is not applicable. Best regards, Krzysztof
On 05/03/2024 08:29, Krzysztof Kozlowski wrote: > On 05/03/2024 08:18, Peng Fan wrote: >>> Subject: Re: [PATCH v3 1/2] dt-bindindgs: clock: support NXP i.MX95 BLK CTL >>> module >>> >>> On 05/03/2024 05:13, Peng Fan wrote: >>>>>> + >>>>>> +examples: >>>>>> + # Clock Control Module node: >>>>>> + - | >>>>>> + #include <dt-bindings/clock/nxp,imx95-clock.h> >>>>>> + >>>>>> + syscon@4c410000 { >>>>> >>>>> clock-controller@... >>>> >>>> But this is a syscon, using clock-controller will trigger dt check >>>> warning. >>> >>> Which warning? >> >> I just recalled that node with syscon in compatible string needs >> has syscon as node, I maybe wrong. > > Just paste the warning, so we can think about it. > >> >>> >>>>> >>>>> As that is the main feature/function. >>>>> >>>>>> + compatible = "nxp,imx95-vpumix-csr", "syscon"; >>>>>> + reg = <0x4c410000 0x10000>; >>>>>> + #clock-cells = <1>; >>>>> >>>>> Please make the example as full as possible. For example, add >>>>> mux-controller node. Do some of the blocks not have mux ctrl? >>>> >>>> Yes. The blk ctrl is not just for clock, some registers has mux ctrl, >>>> such as Pixel_link_sel. >>> >>> Then mux-controller should not be allowed for them. >> >> You mean I should not add mux-controller under the blk ctrl node? > > mux-controller is already there, isn't it? I am saying your binding is > not precise. Your binding implies that ALL OF THEM have mux controller. > You told me it is not true, so you have change the meaning of binding > and disallow the mux-controller for the cases it is not applicable. > ... or create separate binding/schema for the variants with mux-controller. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/clock/imx95-blk-ctl.yaml b/Documentation/devicetree/bindings/clock/imx95-blk-ctl.yaml new file mode 100644 index 000000000000..c8974b927bee --- /dev/null +++ b/Documentation/devicetree/bindings/clock/imx95-blk-ctl.yaml @@ -0,0 +1,61 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/imx95-blk-ctl.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NXP i.MX95 Block Control + +maintainers: + - Peng Fan <peng.fan@nxp.com> + +properties: + compatible: + items: + - enum: + - nxp,imx95-cameramix-csr + - nxp,imx95-display-master-csr + - nxp,imx95-dispmix-lvds-csr + - nxp,imx95-dispmix-csr + - nxp,imx95-netcmix-blk-ctrl + - nxp,imx95-vpumix-csr + - const: syscon + + reg: + maxItems: 1 + + power-domains: + maxItems: 1 + + clocks: + maxItems: 1 + + '#clock-cells': + const: 1 + description: + The clock consumer should specify the desired clock by having the clock + ID in its "clocks" phandle cell. See + include/dt-bindings/clock/nxp,imx95-clock.h + + mux-controller: + type: object + $ref: /schemas/mux/reg-mux.yaml + +required: + - compatible + - reg + - '#clock-cells' + +additionalProperties: false + +examples: + # Clock Control Module node: + - | + #include <dt-bindings/clock/nxp,imx95-clock.h> + + syscon@4c410000 { + compatible = "nxp,imx95-vpumix-csr", "syscon"; + reg = <0x4c410000 0x10000>; + #clock-cells = <1>; + }; +... diff --git a/include/dt-bindings/clock/nxp,imx95-clock.h b/include/dt-bindings/clock/nxp,imx95-clock.h new file mode 100644 index 000000000000..09120e098a97 --- /dev/null +++ b/include/dt-bindings/clock/nxp,imx95-clock.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: GPL-2.0-only OR MIT */ +/* + * Copyright 2024 NXP + */ + +#ifndef __DT_BINDINGS_CLOCK_IMX95_H +#define __DT_BINDINGS_CLOCK_IMX95_H + +#define IMX95_CLK_DISPMIX_ENG0_SEL 0 +#define IMX95_CLK_DISPMIX_ENG1_SEL 1 +#define IMX95_CLK_DISPMIX_END 2 + +#define IMX95_CLK_DISPMIX_LVDS_PHY_DIV 0 +#define IMX95_CLK_DISPMIX_LVDS_CH0_GATE 1 +#define IMX95_CLK_DISPMIX_LVDS_CH1_GATE 2 +#define IMX95_CLK_DISPMIX_PIX_DI0_GATE 3 +#define IMX95_CLK_DISPMIX_PIX_DI1_GATE 4 +#define IMX95_CLK_DISPMIX_LVDS_CSR_END 5 + +#define IMX95_CLK_VPUBLK_WAVE 0 +#define IMX95_CLK_VPUBLK_JPEG_ENC 1 +#define IMX95_CLK_VPUBLK_JPEG_DEC 2 +#define IMX95_CLK_VPUBLK_END 3 + +#define IMX95_CLK_CAMBLK_CSI2_FOR0 0 +#define IMX95_CLK_CAMBLK_CSI2_FOR1 1 +#define IMX95_CLK_CAMBLK_ISP_AXI 2 +#define IMX95_CLK_CAMBLK_ISP_PIXEL 3 +#define IMX95_CLK_CAMBLK_ISP 4 +#define IMX95_CLK_CAMBLK_END 5 + +#endif /* __DT_BINDINGS_CLOCK_IMX95_H */