MIME-Version: 1.0
References: <20211215160906.17451-1-semen.protsenko@linaro.org>
 <20211215160906.17451-7-semen.protsenko@linaro.org> <8c1dbcda-ce01-81c9-b34a-f64b6f61c868@canonical.com>
 <CAPLW+4ndeokx3WiYaK_3ooe0J+BQe8Dx7QCecA7Deowk0AdxnA@mail.gmail.com> <73c5a527-2d5d-8524-b067-f9128055ff10@canonical.com>
In-Reply-To: <73c5a527-2d5d-8524-b067-f9128055ff10@canonical.com>
From:   Sam Protsenko <semen.protsenko@linaro.org>
Date:   Fri, 17 Dec 2021 17:56:24 +0200
Message-ID: <CAPLW+4n+iVwhVB06vwBKxesaeuRo0kgXEZvqPQo6RbOiZO3Vbw@mail.gmail.com>
Subject: Re: [PATCH 6/7] arm64: dts: exynos: Add initial Exynos850 SoC support
To:     Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Sylwester Nawrocki <s.nawrocki@samsung.com>,
        Jaewon Kim <jaewon02.kim@samsung.com>,
        Chanho Park <chanho61.park@samsung.com>,
        David Virag <virag.david003@gmail.com>,
        Youngmin Nam <youngmin.nam@samsung.com>,
        Tomasz Figa <tomasz.figa@gmail.com>,
        Chanwoo Choi <cw00.choi@samsung.com>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        Daniel Palmer <daniel@0x0f.com>,
        Hao Fang <fanghao11@huawei.com>,
        linux-arm-kernel@lists.infradead.org,
        linux-samsung-soc@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 17 Dec 2021 at 10:21, Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> On 16/12/2021 20:40, Sam Protsenko wrote:
> > On Wed, 15 Dec 2021 at 18:47, Krzysztof Kozlowski
> > <krzysztof.kozlowski@canonical.com> wrote:
> >>
>
> (...)
>
> >>> +             serial0 = &serial_0;
> >>> +             serial1 = &serial_1;
> >>> +             serial2 = &serial_2;
> >>> +             i2c0 = &i2c_0;
> >>> +             i2c1 = &i2c_1;
> >>> +             i2c2 = &i2c_2;
> >>> +             i2c3 = &i2c_3;
> >>> +             i2c4 = &i2c_4;
> >>> +             i2c5 = &i2c_5;
> >>> +             i2c6 = &i2c_6;
> >>> +             i2c7 = &hsi2c_0;
> >>> +             i2c8 = &hsi2c_1;
> >>> +             i2c9 = &hsi2c_2;
> >>> +             i2c10 = &hsi2c_3;
> >>> +             i2c11 = &hsi2c_4;
> >>> +     };
> >>> +
> >>> +     arm-pmu {
> >>> +             compatible = "arm,cortex-a55-pmu";
> >>> +             interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>,
> >>> +                          <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>,
> >>> +                          <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>,
> >>> +                          <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>,
> >>> +                          <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
> >>> +                          <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>,
> >>> +                          <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>,
> >>> +                          <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>;
> >>> +             interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>,
> >>> +                                  <&cpu4>, <&cpu5>, <&cpu6>, <&cpu7>;
> >>> +     };
> >>> +
> >>> +     /* Main system clock (XTCXO); external, must be 26 MHz */
> >>> +     oscclk: clock-oscclk {
> >>> +             compatible = "fixed-clock";
> >>> +             clock-output-names = "oscclk";
> >>> +             #clock-cells = <0>;
> >>> +     };
> >>> +
> >>> +     /* RTC clock (XrtcXTI); external, must be 32.768 kHz */
> >>
> >> This clock is usually provided by PMIC, so instead I expect updating
> >> s2mps11-clk driver. It's not correct to mock it with fixed-clock, but in
> >> some cases might be needed. Then I would need an explanation and maybe a
> >> TODO note.
> >>
> >> I wonder if we already discussed this...
> >>
> >
> > Don't really remember discussing that. That's actually something new
> > for me :) I was planning to add PMIC support as a part of separate
> > activity later, it might not be so easy: S2MPU12 uses I3C connection.
> > And RTC clock is not handled even in downstream kernel. So I'll have
> > to implement that by PMIC datasheet. I'll keep some TODO comment for
> > now, hope it's ok with you?
>
> Assuming it is really coming from the PMIC (should be visible in the
> board schematics), it should be using s2mps11-clk. I am fine with
> keeping fixed-clock now + TODO note, but please move it to the board
> DTS. It's not the property of the SoC.
>

Yes, I checked, RTC clock is coming from PMIC. Moved "rtcclk" clock to
board file, and corresponding clock properties for "rtc" and "cmu_hsi"
nodes as well. Will send v4 soon.

> >
> >>> +     rtcclk: clock-rtcclk {> +               compatible = "fixed-clock";
> >>> +             clock-output-names = "rtcclk";
> >>> +             #clock-cells = <0>;
> >>> +     };
> >>> +
>
>
> Best regards,
> Krzysztof