Message ID | 20190921151835.770263-1-martin.blumenstingl@googlemail.com (mailing list archive) |
---|---|
Headers | show |
Series | add the DDR clock controller on Meson8 and Meson8b | expand |
On Sat 21 Sep 2019 at 17:18, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > Meson8 and Meson8b SoCs embed a DDR clock controller in their MMCBUS > registers. This series: > - adds support for this DDR clock controller (patches 0 and 1) > - wires up the DDR PLL as input for two audio clocks (patches 2 and 3) Have you been able to validate somehow that DDR rate calculated by CCF is the actual rate that gets to the audio clocks ? While I understand the interest for completeness, I suspect the having the DDR clock as an audio parent was just for debugging purpose. IOW, I'm not sure if adding this parent is useful to an actual audio use case. As far as audio would be concerned, I think we are better of without this parent. > - adds the DDR clock controller to meson8.dtsi and meson8b.dtsi > Could you please separate the driver and DT series in the future ? Those take different paths and are meant for different maintainers. > Special thanks go out to Alexandre Mergnat for switching the Amlogic > clock drivers over to parent_hws and parent_data. That made this series > a lot easier for me! > > This series depends on my other series from [0]: > "provide the XTAL clock via OF on Meson8/8b/8m2" > > > [0] https://patchwork.kernel.org/cover/11155515/ > > > Martin Blumenstingl (6): > dt-bindings: clock: add the Amlogic Meson8 DDR clock controller > binding > clk: meson: add a driver for the Meson8/8b/8m2 DDR clock controller > clk: meson: meson8b: use of_clk_hw_register to register the clocks > clk: meson: meson8b: add the ddr_pll input for the audio clocks > ARM: dts: meson8: add the DDR clock controller > ARM: dts: meson8b: add the DDR clock controller > > .../clock/amlogic,meson8-ddr-clkc.yaml | 50 ++++++ > arch/arm/boot/dts/meson8.dtsi | 13 +- > arch/arm/boot/dts/meson8b.dtsi | 13 +- > drivers/clk/meson/Makefile | 2 +- > drivers/clk/meson/meson8-ddr.c | 153 ++++++++++++++++++ > drivers/clk/meson/meson8b.c | 36 ++--- > include/dt-bindings/clock/meson8-ddr-clkc.h | 4 + > 7 files changed, 245 insertions(+), 26 deletions(-) > create mode 100644 Documentation/devicetree/bindings/clock/amlogic,meson8-ddr-clkc.yaml > create mode 100644 drivers/clk/meson/meson8-ddr.c > create mode 100644 include/dt-bindings/clock/meson8-ddr-clkc.h > > -- > 2.23.0
Hi Jerome, On Mon, Sep 23, 2019 at 12:06 PM Jerome Brunet <jbrunet@baylibre.com> wrote: > > On Sat 21 Sep 2019 at 17:18, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > > Meson8 and Meson8b SoCs embed a DDR clock controller in their MMCBUS > > registers. This series: > > - adds support for this DDR clock controller (patches 0 and 1) > > - wires up the DDR PLL as input for two audio clocks (patches 2 and 3) > > Have you been able to validate somehow that DDR rate calculated by CCF > is the actual rate that gets to the audio clocks ? no, I haven't been able to validate this (yet) > While I understand the interest for completeness, I suspect the having > the DDR clock as an audio parent was just for debugging purpose. IOW, > I'm not sure if adding this parent is useful to an actual audio use > case. As far as audio would be concerned, I think we are better of > without this parent. there at least three other (potential) consumers of the ddr_pll clocks on the 32-bit SoCs: - CPU clock mux [0] - clk81 mux [1] - USB PHY [2] I have not validated any of these either > > - adds the DDR clock controller to meson8.dtsi and meson8b.dtsi > > > > Could you please separate the driver and DT series in the future ? Those > take different paths and are meant for different maintainers. sure - so far Kevin has been doing a great job of still tracking these but I'm happy to split this into two patchsets Martin [0] https://github.com/endlessm/u-boot-meson/blob/345ee7eb02903f5ecb1173ffb2cd36666e44ebed/board/amlogic/m8b_m201_v1/firmware/timming.c#L441 [1] https://github.com/endlessm/u-boot-meson/blob/345ee7eb02903f5ecb1173ffb2cd36666e44ebed/board/amlogic/m8b_m201_v1/firmware/timming.c#L452 [2] https://github.com/endlessm/u-boot-meson/blob/f1ee03e3f7547d03e1478cc1fc967a9e5a121d92/arch/arm/cpu/aml_meson/m8/firmware/usb_boot/platform.c#L22
On Mon 23 Sep 2019 at 22:49, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > Hi Jerome, > > On Mon, Sep 23, 2019 at 12:06 PM Jerome Brunet <jbrunet@baylibre.com> wrote: >> >> On Sat 21 Sep 2019 at 17:18, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: >> >> > Meson8 and Meson8b SoCs embed a DDR clock controller in their MMCBUS >> > registers. This series: >> > - adds support for this DDR clock controller (patches 0 and 1) >> > - wires up the DDR PLL as input for two audio clocks (patches 2 and 3) >> >> Have you been able to validate somehow that DDR rate calculated by CCF >> is the actual rate that gets to the audio clocks ? > no, I haven't been able to validate this (yet) > >> While I understand the interest for completeness, I suspect the having >> the DDR clock as an audio parent was just for debugging purpose. IOW, >> I'm not sure if adding this parent is useful to an actual audio use >> case. As far as audio would be concerned, I think we are better of >> without this parent. > there at least three other (potential) consumers of the ddr_pll clocks > on the 32-bit SoCs: > - CPU clock mux [0] > - clk81 mux [1] > - USB PHY [2] > > I have not validated any of these either > Then I would suggest to leave patch 4 out until we can somehow validate this. > > > Martin > > > [0] https://github.com/endlessm/u-boot-meson/blob/345ee7eb02903f5ecb1173ffb2cd36666e44ebed/board/amlogic/m8b_m201_v1/firmware/timming.c#L441 > [1] https://github.com/endlessm/u-boot-meson/blob/345ee7eb02903f5ecb1173ffb2cd36666e44ebed/board/amlogic/m8b_m201_v1/firmware/timming.c#L452 > [2] https://github.com/endlessm/u-boot-meson/blob/f1ee03e3f7547d03e1478cc1fc967a9e5a121d92/arch/arm/cpu/aml_meson/m8/firmware/usb_boot/platform.c#L22