lkml.org 
[lkml]   [2022]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v5] cpuidle: sunplus: Create cpuidle driver for sunplus sp7021
From
On 01/03/2022 10:30, Edwin Chiu 邱垂峰 wrote:
>
>
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Tuesday, February 22, 2022 12:48 AM
>> To: Edwin Chiu <edwinchiu0505tw@gmail.com>; Edwin Chiu 邱垂峰 <edwin.chiu@sunplus.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 v5] cpuidle: sunplus: Create cpuidle driver for sunplus sp7021
>>
>> On 21/02/2022 08:26, Edwin Chiu wrote:
>>> Create cpuidle driver for sunplus sp7021 chip
>>>
>>> Signed-off-by: Edwin Chiu <edwinchiu0505tw@gmail.com>
>>> ---
>>> Changes in v3
>>> - Rearrangement #include sequence
>>> - Change remark style to /*~*/
>>> - Align author email address to same as sob
>>> - Optimal code
>>> Changes in v4
>>> - According Rob Herringrobh's comment
>>> There is no need for this binding.
>>> Just wanting a different driver is not a reason
>>> for a duplicate schema.
>>> So remove yaml file and submit driver again.
>>> Changes in v5
>>> - According Krzysztof's comment
>>> 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.
>>> According Sudeep's comment
>>> In short NACK for any dedicated driver for this platform,
>>> use the generic cpuidle-arm driver with appropriate platform hooks
>>> Create cpuidle-sunplus.c in arch/arm/mach-sunplus/
>>> for hook generic cpuidle-arm driver
>>>
>>> MAINTAINERS | 6 ++
>>> arch/arm/mach-sunplus/cpuidle-sunplus.c | 88 +++++++++++++++++
>>> include/linux/platform_data/cpuidle-sunplus.h | 12 ++++
>>> 3 files changed, 106 insertions(+)
>>> create mode 100644 arch/arm/mach-sunplus/cpuidle-sunplus.c
>>> create mode 100644 include/linux/platform_data/cpuidle-sunplus.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS index e0dca8f..5c96428 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -18252,6 +18252,12 @@ L: netdev@vger.kernel.org
>>> S: Maintained
>>> F: drivers/net/ethernet/dlink/sundance.c
>>>
>>> +SUNPLUS CPUIDLE DRIVER
>>> +M: Edwin Chiu <edwinchiu0505tw@gmail.com>
>>> +S: Maintained
>>> +F: arch/arm/mach-sunplus/cpuidle-sunplus.c
>>> +F: include/linux/platform_data/cpuidle-sunplus.h
>>> +
>>> SUPERH
>>> M: Yoshinori Sato <ysato@users.sourceforge.jp>
>>> M: Rich Felker <dalias@libc.org>
>>> diff --git a/arch/arm/mach-sunplus/cpuidle-sunplus.c
>>> b/arch/arm/mach-sunplus/cpuidle-sunplus.c
>>> new file mode 100644
>>> index 0000000..e9d9738
>>> --- /dev/null
>>> +++ b/arch/arm/mach-sunplus/cpuidle-sunplus.c
>>> @@ -0,0 +1,88 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * SP7021 cpu idle Driver.
>>> + * Copyright (C) Sunplus Tech / Tibbo Tech.
>>> + */
>>> +#define pr_fmt(fmt) "CPUidle arm: " fmt
>>> +
>>> +#include <linux/cpuidle.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/platform_data/cpuidle-sunplus.h>
>>> +
>>> +#include <asm/cpuidle.h>
>>> +
>>> +typedef int (*idle_fn)(void);
>>> +
>>> +static DEFINE_PER_CPU(idle_fn*, sp7021_idle_ops);
>>> +
>>> +static int sp7021_cpuidle_enter(unsigned long index) {
>>> + return __this_cpu_read(sp7021_idle_ops)[index]();
>>> +}
>>> +static int sp7021_cpu_spc(void)
>>> +{
>>> + cpu_v7_do_idle(); /* idle to WFI */
>>> + return 0;
>>> +}
>>> +static const struct of_device_id sp7021_idle_state_match[] = {
>>> + { .compatible = "arm,idle-state", .data = sp7021_cpu_spc },
>>> + { },
>>> +};
>>
>> This is confusing. You want to have two drivers to bind to the same compatible? As I wrote in the
>> previous messages, you should simply use arm,idle-state just like few other architectures.
>>
>>
>> Best regards,
>> Krzysztof
>
>
> The patch v5 implemented according your comment.
> Used common part of arm,idle-state.
> Create new enable-method for cpuidle.ops function.
> It only have arm cpuidle driver exist now, no two drivers to bind to the same compatible.
>
> What do you mean " simply use arm,idle-state just like few other architectures "?
>

I mean, do it similarly (by using arm,idle-state and other related
properties) to for example ti,am4372/ti,am3352.

Best regards,
Krzysztof

\
 
 \ /
  Last update: 2022-03-01 12:34    [W:0.128 / U:1.428 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site