lkml.org 
[lkml]   [2022]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v4] cpuidle: sunplus: Create cpuidle driver for sunplus sp7021
Date



> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Sent: Monday, February 14, 2022 2:45 PM
> To: Edwin Chiu 邱垂峰 <edwin.chiu@sunplus.com>; Edwin Chiu <edwinchiu0505tw@gmail.com>;
> robh+dt@kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; rafael@kernel.org;
> daniel.lezcano@linaro.org; linux-pm@vger.kernel.org
> Subject: Re: [PATCH v4] cpuidle: sunplus: Create cpuidle driver for sunplus sp7021
>
> On 14/02/2022 03:55, Edwin Chiu 邱垂峰 wrote:
> > Hi Krzysztof:
> >
> > Please see below answer.
> >
> >>> +static struct cpuidle_driver sp7021_idle_driver = {
> >>> + .name = "sp7021_idle",
> >>> + .owner = THIS_MODULE,
> >>> + /*
> >>> + * State at index 0 is standby wfi and considered standard
> >>> + * on all ARM platforms. If in some platforms simple wfi
> >>> + * can't be used as "state 0", DT bindings must be implemented
> >>> + * to work around this issue and allow installing a special
> >>> + * handler for idle state index 0.
> >>> + */
> >>> + .states[0] = {
> >>> + .enter = sp7021_enter_idle_state,
> >>> + .exit_latency = 1,
> >>> + .target_residency = 1,
> >>> + .power_usage = UINT_MAX,
> >>> + .name = "WFI",
> >>> + .desc = "ARM WFI",
> >>
> >> I have impression that there is no point in having custom driver with WFI...
> >>
> >> Still the main question from Daniel and Sudeep stays: why do you need
> >> this? You copied exactly cpuildle-arm driver, there is nothing
> >> different here. At least I could not spot differences. Maybe except that you use cpu_v7_do_idle
> explicitly.
> >>
> >> Unfortunately I cannot understand the explanation here:
> >> https://lore.kernel.org/all/0812c44f777d4026b79df2e3698294be@sphcmbx0
> >> 2.sunplus.com.tw/ Why exactly cpuidle-arm does not work in your case?
> >>
> > Edwin=> I mean cpuidle-arm driver can't directly use with no modified.
> > If someone want to use cpuidle-arm driver, below modification seems necessary.
> >
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > Static int sp7021_cpuidle_suspend_enter(unsigned long index) {~}
> > Static int __init sp7021_cpuidle_init(struct device_node *cpu_node, int cpu) {~}
> > Static const struct cpuidle_ops sc_smp_ops __initconst = {
> > .suspend = sp7021_cpuidle_suspend_enter,
> > .init = sp7021_cpuidle_init,
> > };
> > CPUIDLE_METHOD_OF_DECLARE(sc_smp, "sunplus,sc-smp", &sc_smp_ops); //declare
> enable method
> >
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ~~~~~~~~~~
> >
> > But change cpuilde-arm.c for sunplus driver seems not reasonable.
> > That is why I want to submit cpuidle-sunplus.c
> > Althought sunplus cpuidle only come in WFI, but it can complete the cpuidle framework.
>
> I don't think it is correct. You can use cpuidle-arm, because it is being always initialized with
> device_initcall(). You either use appropriate compatible in DT or add your compatible to cpuidle-arm.
>
> Even if this did not work, then the solution is to use common parts, not to duplicate entire driver.
> Duplicating is not acceptable.
>
> Best regards,
> Krzysztof


I do used compatible = "arm,idle-state" in DT and enable generic arm cpuidle driver in menuconfig.
But there have mount driver fail message due to no cpuidle_ops functions.
That is why I need added patch code to complete cpuidle driver.
According your comment, I will try to use common parts and hook some custom code, later.
Thanks.

邱垂峰 EdwinChiu
智能運算專案
T: +886-3-5786005 ext.2590
edwin.chiu@sunplus.com
300 新竹科學園區創新一路19號
\
 
 \ /
  Last update: 2022-02-18 09:11    [W:0.101 / U:0.948 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site