RE: [PATCH v5] cpuidle: sunplus: Create cpuidle driver for sunplus sp7021

[Edwin Chiu 邱垂峰]

> -----Original Message-----
> From: Sudeep Holla <sudeep.holla@xxxxxxx>
> Sent: Monday, February 21, 2022 6:52 PM
> To: Edwin Chiu <edwinchiu0505tw@xxxxxxxxx>
> Cc: Edwin Chiu 邱垂峰 <edwin.chiu@xxxxxxxxxxx>; robh+dt@xxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; rafael@xxxxxxxxxx; Sudeep Holla <sudeep.holla@xxxxxxx>;
> daniel.lezcano@xxxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v5] cpuidle: sunplus: Create cpuidle driver for sunplus sp7021
> 
> On Mon, Feb 21, 2022 at 03:26:18PM +0800, Edwin Chiu wrote:
> > Create cpuidle driver for sunplus sp7021 chip
> >
> > Signed-off-by: Edwin Chiu <edwinchiu0505tw@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxx
> >  S:	Maintained
> >  F:	drivers/net/ethernet/dlink/sundance.c
> >
> > +SUNPLUS CPUIDLE DRIVER
> > +M:	Edwin Chiu <edwinchiu0505tw@xxxxxxxxx>
> > +S:	Maintained
> > +F:	arch/arm/mach-sunplus/cpuidle-sunplus.c
> > +F:	include/linux/platform_data/cpuidle-sunplus.h
> > +
> >  SUPERH
> >  M:	Yoshinori Sato <ysato@xxxxxxxxxxxxxxxxxxxx>
> >  M:	Rich Felker <dalias@xxxxxxxx>
> > 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;
> > +}
> 
> You really don't need a cpuidle driver to just WFI for any states.
> Add the driver when you have something non WFI in the suspend function.
> 
> > +static const struct of_device_id sp7021_idle_state_match[] = {
> > +	{ .compatible = "arm,idle-state", .data = sp7021_cpu_spc },
> > +	{ },
> > +};
> 
> This is better than adding new driver like you did in previous version.
> 
> I did a quick check but couldn't figure out. How do cpus get switched ON or OFF on this platform(for
> example during CPU hotplug) ?
> 
> --
> Regards,
> Sudeep


In this patch, I just want to submit cpuidle function.
So there have no cpu hotplug function now.



邱垂峰 EdwinChiu
智能運算專案
T: +886-3-5786005 ext.2590
edwin.chiu@xxxxxxxxxxx
300 新竹科學園區創新一路19號