| Message ID | 1557155521-30949-8-git-send-email-l.luba@partner.samsung.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Exynos5 Dynamic Memory Controller driver | expand |
On Mon, May 06, 2019 at 05:11:55PM +0200, Lukasz Luba wrote: > The patch adds description for DT binding for a new Exynos5422 Dynamic > Memory Controller device. > > Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com> > --- > .../bindings/memory-controllers/exynos5422-dmc.txt | 74 ++++++++++++++++++++++ > 1 file changed, 74 insertions(+) > create mode 100644 Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt > > diff --git a/Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt b/Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt > new file mode 100644 > index 0000000..be602a9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt > @@ -0,0 +1,74 @@ > +* Exynos5422 frequency and voltage scaling for Dynamic Memory Controller device > + > +The Samsung Exynos5422 SoC has DMC (Dynamic Memory Controller) to which the DRAM > +memory chips are connected. The driver is to monitor the controller in runtime > +and switch frequency and voltage. To monitor the usage of the controller in > +runtime, the driver uses the PPMU (Platform Performance Monitoring Unit), which > +is able to measure the current load of the memory. > +When 'userspace' governor is used for the driver, an application is able to > +switch the DMC and memory frequency. > + > +Required properties for DMC device for Exynos5422: > +- compatible: Should be "samsung,exynos5422-dmc". > +- clock-names : the name of clock used by the controller. > +- clocks : phandles for clock specified in "clock-names" property. Need to enumerate the clocks and their order. > +- devfreq-events : phandles for PPMU devices connected to this DMC. > +- vdd-supply : phandle for voltage regulator which is connected. > +- reg : registers of two CDREX controllers. > +- operating-points-v2 : phandle for OPPs described in v2 definition. > +- device-handle : phandle of the connected DRAM memory device. For more > + information please refer to Documentation Documentation... ? > +- devfreq-events : phandles of the PPMU events used by the controller. > +- samsung,syscon-chipid : phandle of the ChipID used by the controller. > +- samsung,syscon-clk : phandle of the clock register set used by the controller. Looks like a hack. Can't you get this from the clocks property? What is this for? > + > +Example: > + > + ppmu_dmc0_0: ppmu@10d00000 { > + compatible = "samsung,exynos-ppmu"; > + reg = <0x10d00000 0x2000>; > + clocks = <&clock CLK_PCLK_PPMU_DREX0_0>; > + clock-names = "ppmu"; > + events { > + ppmu_event_dmc0_0: ppmu-event3-dmc0_0 { > + event-name = "ppmu-event3-dmc0_0"; > + }; > + }; > + }; > + > + dmc: memory-controller@10c20000 { > + compatible = "samsung,exynos5422-dmc"; > + reg = <0x10c20000 0x100>, <0x10c30000 0x100>, > + clocks = <&clock CLK_FOUT_SPLL>, > + <&clock CLK_MOUT_SCLK_SPLL>, > + <&clock CLK_FF_DOUT_SPLL2>, > + <&clock CLK_FOUT_BPLL>, > + <&clock CLK_MOUT_BPLL>, > + <&clock CLK_SCLK_BPLL>, > + <&clock CLK_MOUT_MX_MSPLL_CCORE>, > + <&clock CLK_MOUT_MX_MSPLL_CCORE_PHY>, > + <&clock CLK_MOUT_MCLK_CDREX>, > + <&clock CLK_DOUT_CLK2X_PHY0>, > + <&clock CLK_CLKM_PHY0>, > + <&clock CLK_CLKM_PHY1>; > + clock-names = "fout_spll", > + "mout_sclk_spll", > + "ff_dout_spll2", > + "fout_bpll", > + "mout_bpll", > + "sclk_bpll", > + "mout_mx_mspll_ccore", > + "mout_mx_mspll_ccore_phy", > + "mout_mclk_cdrex", > + "dout_clk2x_phy0", > + "clkm_phy0", > + "clkm_phy1"; > + operating-points-v2 = <&dmc_opp_table>; > + devfreq-events = <&ppmu_event3_dmc0_0>, <&ppmu_event3_dmc0_1>, > + <&ppmu_event3_dmc1_0>, <&ppmu_event3_dmc1_1>; > + operating-points-v2 = <&dmc_opp_table>; > + device-handle = <&samsung_K3QF2F20DB>; > + vdd-supply = <&buck1_reg>; > + samsung,syscon-clk = <&clock>; > + samsung,syscon-chipid = <&chipid>; > + }; > -- > 2.7.4 >
On Tue, 7 May 2019 at 19:04, Rob Herring <robh@kernel.org> wrote: > > +- devfreq-events : phandles of the PPMU events used by the controller. > > +- samsung,syscon-chipid : phandle of the ChipID used by the controller. > > +- samsung,syscon-clk : phandle of the clock register set used by the controller. > > Looks like a hack. Can't you get this from the clocks property? What is > this for? Hi Rob, Lukasz uses these two syscon regmaps to read certain registers. For chipid he reads it to check the size of attached memory (only 2 GB version is supported). This indeed looks like a hack. However the second regmap (clk) is needed to get the timing data from registers from DMC clock driver address space. These are registers with memory timing so their data is not exposed anyway in common clk framework. Best regards, Krzysztof
Hi Rob, On 5/7/19 7:04 PM, Rob Herring wrote: > On Mon, May 06, 2019 at 05:11:55PM +0200, Lukasz Luba wrote: >> The patch adds description for DT binding for a new Exynos5422 Dynamic >> Memory Controller device. >> >> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com> >> --- >> .../bindings/memory-controllers/exynos5422-dmc.txt | 74 ++++++++++++++++++++++ >> 1 file changed, 74 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt >> >> diff --git a/Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt b/Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt >> new file mode 100644 >> index 0000000..be602a9 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt >> @@ -0,0 +1,74 @@ >> +* Exynos5422 frequency and voltage scaling for Dynamic Memory Controller device >> + >> +The Samsung Exynos5422 SoC has DMC (Dynamic Memory Controller) to which the DRAM >> +memory chips are connected. The driver is to monitor the controller in runtime >> +and switch frequency and voltage. To monitor the usage of the controller in >> +runtime, the driver uses the PPMU (Platform Performance Monitoring Unit), which >> +is able to measure the current load of the memory. >> +When 'userspace' governor is used for the driver, an application is able to >> +switch the DMC and memory frequency. >> + >> +Required properties for DMC device for Exynos5422: >> +- compatible: Should be "samsung,exynos5422-dmc". >> +- clock-names : the name of clock used by the controller. >> +- clocks : phandles for clock specified in "clock-names" property. > > Need to enumerate the clocks and their order. OK I will add a list here and above in 'clock-names' in the right order. > >> +- devfreq-events : phandles for PPMU devices connected to this DMC. >> +- vdd-supply : phandle for voltage regulator which is connected. >> +- reg : registers of two CDREX controllers. >> +- operating-points-v2 : phandle for OPPs described in v2 definition. >> +- device-handle : phandle of the connected DRAM memory device. For more >> + information please refer to Documentation > > Documentation... ? I should have changed it after I moved the lpddr3.txt files in the doc dir. I missed it, my apologies. > >> +- devfreq-events : phandles of the PPMU events used by the controller. >> +- samsung,syscon-chipid : phandle of the ChipID used by the controller. >> +- samsung,syscon-clk : phandle of the clock register set used by the controller. > > Looks like a hack. Can't you get this from the clocks property? What is > this for? As Krzysztof commented in the next message, the chipid register contains information about memory size and the clock register set has some registers from DMC (timings settings) which are not typical 'clocks'. Chanwoo suggested to use syscon regmap to access these clock registers safely. Krzysztof suggested to use syscon regmap similar to Sylwester's AVS driver while accessing chipid registers. That's why they are here. Both register sets contain needed information for proper operation of the driver. Regards, Lukasz > >> + >> +Example: >> + >> + ppmu_dmc0_0: ppmu@10d00000 { >> + compatible = "samsung,exynos-ppmu"; >> + reg = <0x10d00000 0x2000>; >> + clocks = <&clock CLK_PCLK_PPMU_DREX0_0>; >> + clock-names = "ppmu"; >> + events { >> + ppmu_event_dmc0_0: ppmu-event3-dmc0_0 { >> + event-name = "ppmu-event3-dmc0_0"; >> + }; >> + }; >> + }; >> + >> + dmc: memory-controller@10c20000 { >> + compatible = "samsung,exynos5422-dmc"; >> + reg = <0x10c20000 0x100>, <0x10c30000 0x100>, >> + clocks = <&clock CLK_FOUT_SPLL>, >> + <&clock CLK_MOUT_SCLK_SPLL>, >> + <&clock CLK_FF_DOUT_SPLL2>, >> + <&clock CLK_FOUT_BPLL>, >> + <&clock CLK_MOUT_BPLL>, >> + <&clock CLK_SCLK_BPLL>, >> + <&clock CLK_MOUT_MX_MSPLL_CCORE>, >> + <&clock CLK_MOUT_MX_MSPLL_CCORE_PHY>, >> + <&clock CLK_MOUT_MCLK_CDREX>, >> + <&clock CLK_DOUT_CLK2X_PHY0>, >> + <&clock CLK_CLKM_PHY0>, >> + <&clock CLK_CLKM_PHY1>; >> + clock-names = "fout_spll", >> + "mout_sclk_spll", >> + "ff_dout_spll2", >> + "fout_bpll", >> + "mout_bpll", >> + "sclk_bpll", >> + "mout_mx_mspll_ccore", >> + "mout_mx_mspll_ccore_phy", >> + "mout_mclk_cdrex", >> + "dout_clk2x_phy0", >> + "clkm_phy0", >> + "clkm_phy1"; >> + operating-points-v2 = <&dmc_opp_table>; >> + devfreq-events = <&ppmu_event3_dmc0_0>, <&ppmu_event3_dmc0_1>, >> + <&ppmu_event3_dmc1_0>, <&ppmu_event3_dmc1_1>; >> + operating-points-v2 = <&dmc_opp_table>; >> + device-handle = <&samsung_K3QF2F20DB>; >> + vdd-supply = <&buck1_reg>; >> + samsung,syscon-clk = <&clock>; >> + samsung,syscon-chipid = <&chipid>; >> + }; >> -- >> 2.7.4 >> > >
On 5/8/19 9:19 AM, Krzysztof Kozlowski wrote: > On Tue, 7 May 2019 at 19:04, Rob Herring <robh@kernel.org> wrote: >>> +- devfreq-events : phandles of the PPMU events used by the controller. >>> +- samsung,syscon-chipid : phandle of the ChipID used by the controller. >>> +- samsung,syscon-clk : phandle of the clock register set used by the controller. >> >> Looks like a hack. Can't you get this from the clocks property? What is >> this for? > > Hi Rob, > > Lukasz uses these two syscon regmaps to read certain registers. For > chipid he reads it to check the size of attached memory (only 2 GB > version is supported). This indeed looks like a hack. However the > second regmap (clk) is needed to get the timing data from registers > from DMC clock driver address space. These are registers with memory > timing so their data is not exposed anyway in common clk framework. > > Best regards, > Krzysztof Thank you Krzysztof for a fast response. I have also responded to Rob. I wouldn't call accessing chipid registers as a hack, though. The DMC registers do not contain information about the memory chip since it is in phase of production the board not the chip. Thus, chipid regs (which loads from e-fuses) are best place to put information about memory type/size. Regards, Lukasz
On Wed, 8 May 2019 at 11:45, Lukasz Luba <l.luba@partner.samsung.com> wrote: > > > On 5/8/19 9:19 AM, Krzysztof Kozlowski wrote: > > On Tue, 7 May 2019 at 19:04, Rob Herring <robh@kernel.org> wrote: > >>> +- devfreq-events : phandles of the PPMU events used by the controller. > >>> +- samsung,syscon-chipid : phandle of the ChipID used by the controller. > >>> +- samsung,syscon-clk : phandle of the clock register set used by the controller. > >> > >> Looks like a hack. Can't you get this from the clocks property? What is > >> this for? > > > > Hi Rob, > > > > Lukasz uses these two syscon regmaps to read certain registers. For > > chipid he reads it to check the size of attached memory (only 2 GB > > version is supported). This indeed looks like a hack. However the > > second regmap (clk) is needed to get the timing data from registers > > from DMC clock driver address space. These are registers with memory > > timing so their data is not exposed anyway in common clk framework. > > > > Best regards, > > Krzysztof > > Thank you Krzysztof for a fast response. I have also responded to Rob. > I wouldn't call accessing chipid registers as a hack, though. The DMC > registers do not contain information about the memory chip since it is > in phase of production the board not the chip. Thus, chipid regs (which > loads from e-fuses) are best place to put information about memory > type/size. By hack I meant that you have to read chipid instead of DTS... but as you pointed, the DTS could not match the real fused values so actually it makes sense to read them. Best regards, Krzysztof
On Wed, May 8, 2019 at 4:45 AM Lukasz Luba <l.luba@partner.samsung.com> wrote: > > > On 5/8/19 9:19 AM, Krzysztof Kozlowski wrote: > > On Tue, 7 May 2019 at 19:04, Rob Herring <robh@kernel.org> wrote: > >>> +- devfreq-events : phandles of the PPMU events used by the controller. > >>> +- samsung,syscon-chipid : phandle of the ChipID used by the controller. > >>> +- samsung,syscon-clk : phandle of the clock register set used by the controller. > >> > >> Looks like a hack. Can't you get this from the clocks property? What is > >> this for? > > > > Hi Rob, > > > > Lukasz uses these two syscon regmaps to read certain registers. For > > chipid he reads it to check the size of attached memory (only 2 GB > > version is supported). This indeed looks like a hack. However the > > second regmap (clk) is needed to get the timing data from registers > > from DMC clock driver address space. These are registers with memory > > timing so their data is not exposed anyway in common clk framework. Okay, please just explain what your accessing. Consider adding the offset as a cell in case stuff moves around on another chip. > > > > Best regards, > > Krzysztof > > Thank you Krzysztof for a fast response. I have also responded to Rob. > I wouldn't call accessing chipid registers as a hack, though. The DMC > registers do not contain information about the memory chip since it is > in phase of production the board not the chip. Thus, chipid regs (which > loads from e-fuses) are best place to put information about memory > type/size. For efuses, we have a binding (nvmem). Maybe you should use it. Rob
Hi Rob, On 5/8/19 10:35 PM, Rob Herring wrote: > On Wed, May 8, 2019 at 4:45 AM Lukasz Luba <l.luba@partner.samsung.com> wrote: >> >> >> On 5/8/19 9:19 AM, Krzysztof Kozlowski wrote: >>> On Tue, 7 May 2019 at 19:04, Rob Herring <robh@kernel.org> wrote: >>>>> +- devfreq-events : phandles of the PPMU events used by the controller. >>>>> +- samsung,syscon-chipid : phandle of the ChipID used by the controller. >>>>> +- samsung,syscon-clk : phandle of the clock register set used by the controller. >>>> >>>> Looks like a hack. Can't you get this from the clocks property? What is >>>> this for? >>> >>> Hi Rob, >>> >>> Lukasz uses these two syscon regmaps to read certain registers. For >>> chipid he reads it to check the size of attached memory (only 2 GB >>> version is supported). This indeed looks like a hack. However the >>> second regmap (clk) is needed to get the timing data from registers >>> from DMC clock driver address space. These are registers with memory >>> timing so their data is not exposed anyway in common clk framework. > > Okay, please just explain what your accessing. Consider adding the > offset as a cell in case stuff moves around on another chip. Good point. I will also have to regmap the registers and not take from 'clock' device. > >>> >>> Best regards, >>> Krzysztof >> >> Thank you Krzysztof for a fast response. I have also responded to Rob. >> I wouldn't call accessing chipid registers as a hack, though. The DMC >> registers do not contain information about the memory chip since it is >> in phase of production the board not the chip. Thus, chipid regs (which >> loads from e-fuses) are best place to put information about memory >> type/size. > > For efuses, we have a binding (nvmem). Maybe you should use it. I don't know about the design of a planned 'chipid' driver, which going to be sent to LKML in near future. Thank you for this information, I will talk with Bartek. Regards, Lukasz > > Rob > >
diff --git a/Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt b/Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt new file mode 100644 index 0000000..be602a9 --- /dev/null +++ b/Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt @@ -0,0 +1,74 @@ +* Exynos5422 frequency and voltage scaling for Dynamic Memory Controller device + +The Samsung Exynos5422 SoC has DMC (Dynamic Memory Controller) to which the DRAM +memory chips are connected. The driver is to monitor the controller in runtime +and switch frequency and voltage. To monitor the usage of the controller in +runtime, the driver uses the PPMU (Platform Performance Monitoring Unit), which +is able to measure the current load of the memory. +When 'userspace' governor is used for the driver, an application is able to +switch the DMC and memory frequency. + +Required properties for DMC device for Exynos5422: +- compatible: Should be "samsung,exynos5422-dmc". +- clock-names : the name of clock used by the controller. +- clocks : phandles for clock specified in "clock-names" property. +- devfreq-events : phandles for PPMU devices connected to this DMC. +- vdd-supply : phandle for voltage regulator which is connected. +- reg : registers of two CDREX controllers. +- operating-points-v2 : phandle for OPPs described in v2 definition. +- device-handle : phandle of the connected DRAM memory device. For more + information please refer to Documentation +- devfreq-events : phandles of the PPMU events used by the controller. +- samsung,syscon-chipid : phandle of the ChipID used by the controller. +- samsung,syscon-clk : phandle of the clock register set used by the controller. + +Example: + + ppmu_dmc0_0: ppmu@10d00000 { + compatible = "samsung,exynos-ppmu"; + reg = <0x10d00000 0x2000>; + clocks = <&clock CLK_PCLK_PPMU_DREX0_0>; + clock-names = "ppmu"; + events { + ppmu_event_dmc0_0: ppmu-event3-dmc0_0 { + event-name = "ppmu-event3-dmc0_0"; + }; + }; + }; + + dmc: memory-controller@10c20000 { + compatible = "samsung,exynos5422-dmc"; + reg = <0x10c20000 0x100>, <0x10c30000 0x100>, + clocks = <&clock CLK_FOUT_SPLL>, + <&clock CLK_MOUT_SCLK_SPLL>, + <&clock CLK_FF_DOUT_SPLL2>, + <&clock CLK_FOUT_BPLL>, + <&clock CLK_MOUT_BPLL>, + <&clock CLK_SCLK_BPLL>, + <&clock CLK_MOUT_MX_MSPLL_CCORE>, + <&clock CLK_MOUT_MX_MSPLL_CCORE_PHY>, + <&clock CLK_MOUT_MCLK_CDREX>, + <&clock CLK_DOUT_CLK2X_PHY0>, + <&clock CLK_CLKM_PHY0>, + <&clock CLK_CLKM_PHY1>; + clock-names = "fout_spll", + "mout_sclk_spll", + "ff_dout_spll2", + "fout_bpll", + "mout_bpll", + "sclk_bpll", + "mout_mx_mspll_ccore", + "mout_mx_mspll_ccore_phy", + "mout_mclk_cdrex", + "dout_clk2x_phy0", + "clkm_phy0", + "clkm_phy1"; + operating-points-v2 = <&dmc_opp_table>; + devfreq-events = <&ppmu_event3_dmc0_0>, <&ppmu_event3_dmc0_1>, + <&ppmu_event3_dmc1_0>, <&ppmu_event3_dmc1_1>; + operating-points-v2 = <&dmc_opp_table>; + device-handle = <&samsung_K3QF2F20DB>; + vdd-supply = <&buck1_reg>; + samsung,syscon-clk = <&clock>; + samsung,syscon-chipid = <&chipid>; + };
The patch adds description for DT binding for a new Exynos5422 Dynamic Memory Controller device. Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com> --- .../bindings/memory-controllers/exynos5422-dmc.txt | 74 ++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt