Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 3 Nov 2021 09:09:45 +0900
From:   Youngmin Nam <youngmin.nam@samsung.com>
To:     Mark Rutland <mark.rutland@arm.com>
Cc:     krzysztof.kozlowski@canonical.com, will@kernel.org,
        daniel.lezcano@linaro.org, tglx@linutronix.de,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-samsung-soc@vger.kernel.org, pullip.cho@samsung.com,
        hoony.yu@samsung.com, hajun.sung@samsung.com,
        myung-su.cha@samsung.com, kgene@kernel.org
Subject: Re: [PATCH v2 1/2] clocksource/drivers/exynos_mct_v2: introduce
 Exynos MCT version 2 driver for next Exynos SoC
Message-ID: <20211103000945.GA48132@perf>
MIME-Version: 1.0
In-Reply-To: <20211102102802.GA16545@C02TD0UTHF1T.local>
User-Agent: Mutt/1.9.4 (2018-02-28)
Content-Type: multipart/mixed;
        boundary="----ZBuuKJqGqetWuXeXLn-uUmRQxt4bB4GzF80pTq2gGxc9tq5b=_ae3f7_"
CMS-TYPE: 102P
DLP-Filter: Pass
References: <20211102001122.27516-1-youngmin.nam@samsung.com>
        <CGME20211101234500epcas2p2d0e5bc54615b635f6694bc1be4c89fb5@epcas2p2.samsung.com>
        <20211102001122.27516-2-youngmin.nam@samsung.com>
        <20211102102802.GA16545@C02TD0UTHF1T.local>
Precedence: bulk

------ZBuuKJqGqetWuXeXLn-uUmRQxt4bB4GzF80pTq2gGxc9tq5b=_ae3f7_
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline

On Tue, Nov 02, 2021 at 10:28:10AM +0000, Mark Rutland wrote:
> On Tue, Nov 02, 2021 at 09:11:21AM +0900, Youngmin Nam wrote:
> > Exynos MCT version 2 is composed of 1 FRC and 12 comparators.
> > There are no global timer and local timer anymore.
> > The 1 of 64bit FRC serves as "up-counter"(not "comparators").
> > The 12 comaprators(not "counter") can be used as per-cpu event timer
> > so that it can support upto 12 cores.
> > And a RTC source can be used as backup clock source.
> 
> [...]
> 
> > +static int exynos_mct_starting_cpu(unsigned int cpu)
> > +{
> > +	struct mct_clock_event_device *mevt = per_cpu_ptr(&percpu_mct_tick, cpu);
> > +	struct clock_event_device *evt = &mevt->evt;
> > +
> > +	snprintf(mevt->name, sizeof(mevt->name), "mct_comp%d", cpu);
> > +
> > +	evt->name = mevt->name;
> > +	evt->cpumask = cpumask_of(cpu);
> > +	evt->set_next_event = exynos_comp_set_next_event;
> > +	evt->set_state_periodic = mct_set_state_periodic;
> > +	evt->set_state_shutdown = mct_set_state_shutdown;
> > +	evt->set_state_oneshot = mct_set_state_shutdown;
> > +	evt->set_state_oneshot_stopped = mct_set_state_shutdown;
> > +	evt->tick_resume = mct_set_state_shutdown;
> > +	evt->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
> > +	evt->rating = 500;	/* use value higher than ARM arch timer */
> 
> Previously Will asked you to try CLOCK_EVT_FEAT_PERCPU here, and to set
> the C3STOP flag on the arch timer via the DT when necessary, rather than
> trying to override the arch timer like this:
> 
>   https://protect2.fireeye.com/v1/url?k=72526080-2dc9598b-7253ebcf-002590f5b904-ca603717c6462908&q=1&e=be56aa83-dbac-4639-913d-d388620fe3fc&u=https%3A%2F%2Flore.kernel.org%2Fr%2F20211027073458.GA22231%40willie-the-truck
> 
> There are a bunch of things that depend on the architected timer working
> as a clocksource (e.g. vdso, kvm), and it *should* work as a lock
> clockevent_device if configured correctly, and it's much more consistent
> with *everyone else* to use the arhcitected timer by default.
> 
> Please try as Will suggested above, so that this works from day one.
> 
> Thanks,
> Mark.
> 

Hi Mark.
It looks like you missed my previous mail.
https://lore.kernel.org/all/20211029035422.GA30523@perf/#t

Yes, I believe Will's suggestion definitely will work.
But that is for performance not functionality.
As a driver for new H/W IP I would like to confirm functionality first.

We need more time to test this feature with our exynos core power down feature.
And we need to do a various regression test whether there is another corner case or not.
So, how about we apply Will's suggetion later after the current patchset is merged first?
After doing our regression test with our exynos core power down feature, we can confirm this.

Thanks.

------ZBuuKJqGqetWuXeXLn-uUmRQxt4bB4GzF80pTq2gGxc9tq5b=_ae3f7_
Content-Type: text/plain; charset="utf-8"


------ZBuuKJqGqetWuXeXLn-uUmRQxt4bB4GzF80pTq2gGxc9tq5b=_ae3f7_--