* Re: [PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF
From: Michael Ellerman @ 2014-09-15 2:49 UTC (permalink / raw)
To: Gavin Shan; +Cc: Wei Yang, linuxppc-dev
In-Reply-To: <20140912050518.GA17933@shangw>
On Fri, 2014-09-12 at 15:05 +1000, Gavin Shan wrote:
> On Fri, Sep 12, 2014 at 01:55:23PM +1000, Michael Ellerman wrote:
> >On Thu, 2014-09-11 at 11:42 +0800, Wei Yang wrote:
> >> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> >> index 4a45ba8..403445e 100644
> >> --- a/arch/powerpc/kernel/eeh.c
> >> +++ b/arch/powerpc/kernel/eeh.c
> >> @@ -625,7 +625,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
> >> int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
> >> {
> >> struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
> >> - struct eeh_pe *pe = edev->pe;
> >> + struct eeh_pe *pe = edev ? edev->pe : NULL;
> >>
> >> if (!pe) {
> >> pr_err("%s: No PE found on PCI device %s\n",
> >
> >
> >We seem to do this or something similar in a few places. Is it worth having a
> >pci_dev_to_eeh_pe() inline?
> >
>
> Yes, maybe we just need a eeh_dev_to_pe() because converting
> pci_dev to eeh_dev is already coverred by pci_dev_to_eeh_dev().
>
> With eeh_dev_to_pe(), it looks like this:
>
> struct pci_dev *pdev;
> struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
> struct eeh_pe *pe = eeh_dev_to_pe(edev);
>
> Or another case:
>
> struct device_node *dn;
> struct eeh_dev *edev = of_node_to_eeh_dev(dn);
> struct eeh_pe *pe = eeh_dev_to_pe(edev);
Yeah I guess.
I saw a few places where we go from pci_dev to eeh_pe via a eeh_dev but then
don't use the eeh_dev at all. So for those it would make sense to have one
macro that does the full conversion from pci_dev to eeh_pe.
But if you think that's not very common then yeah a macro to do each stage is
fine.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: Add macros for the ibm_architecture_vec[] lengths
From: Michael Ellerman @ 2014-09-15 3:00 UTC (permalink / raw)
To: Stewart Smith; +Cc: linuxppc-dev
In-Reply-To: <m3a969sj08.fsf@oc8180480414.ibm.com>
On Tue, 2014-09-09 at 11:03 +1000, Stewart Smith wrote:
> Michael Ellerman <mpe@ellerman.id.au> writes:
> > The encoding of the lengths in the ibm_architecture_vec array is
> > "interesting" to say the least. It's non-obvious how the number of bytes
> > we provide relates to the length value.
> >
> > In fact we already got it wrong once, see 11e9ed43ca8a "Fix up
> > ibm_architecture_vec definition".
> >
> > So add some macros to make it (hopefully) clearer. These at least have
> > the property that the integer present in the code is equal to the number
> > of bytes that follows it.
> >
> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>
> Seems at least as correct as the code was before... so,
>
> Reviewed-by: Stewart Smith <stewart@linux.vnet.ibm.com>
>
> (not actually compiled or tested or anything, but my internal C
> preprocesser says it looks okay :)
Thanks for the review, it's one of those patches it would be easy to get wrong
because it's so simple in theory.
As a test I deliberately broke the NUM_VECTORS value, and .. my system booted
just fine! So I think I'll give this a bit more scrutiny and work out what's
going on there first :)
cheers
^ permalink raw reply
* Re: [PATCH v2 1/2] pseries: Fix endian issues in onlining cpu threads
From: Michael Ellerman @ 2014-09-15 3:01 UTC (permalink / raw)
To: Thomas Falcon; +Cc: Nathan Fontenot, linuxppc-dev
In-Reply-To: <1410549102-8410-1-git-send-email-tlfalcon@linux.vnet.ibm.com>
On Fri, 2014-09-12 at 14:11 -0500, Thomas Falcon wrote:
> The ibm,ppc-interrupt-server#s property is in big endian format.
> These values need to be converted when used by little endian
> architectures.
>
> Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
> ---
> Changes in v2:
>
> Followed suggestions from Michael Ellerman
> conversion of intserv values occur once
Thanks Tom.
I think I saw that you'd already discussed these with Nathan in another thread,
so I'll assume he's OK with them unless he says otherwise.
cheers
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/powernv: Add OPAL check token call
From: Michael Ellerman @ 2014-09-15 3:10 UTC (permalink / raw)
To: Michael Neuling; +Cc: Stewart Smith, linuxppc-dev
In-Reply-To: <1408423682-14297-1-git-send-email-mikey@neuling.org>
On Tue, 2014-08-19 at 14:47 +1000, Michael Neuling wrote:
> Currently there is no way to generically check if an OPAL call exists or not
> from the host kernel.
>
> This adds an OPAL call opal_check_token() which tells you if the given token is
> present in OPAL or not.
>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
> arch/powerpc/include/asm/opal.h | 7 +++++++
> arch/powerpc/platforms/powernv/opal-wrappers.S | 1 +
> 2 files changed, 8 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 86055e5..4593a93 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -135,6 +135,7 @@ struct opal_sg_list {
> #define OPAL_FLASH_MANAGE 77
> #define OPAL_FLASH_UPDATE 78
> #define OPAL_RESYNC_TIMEBASE 79
> +#define OPAL_CHECK_TOKEN 80
> #define OPAL_DUMP_INIT 81
> #define OPAL_DUMP_INFO 82
> #define OPAL_DUMP_READ 83
> @@ -417,6 +418,11 @@ struct opal_msg {
> __be64 params[8];
> };
>
> +enum OpalCheckTokenStatus {
> + OPAL_TOKEN_ABSENT = 0,
> + OPAL_TOKEN_PRESENT = 1
> +};
I don't see this used anywhere?
And NoCamelCase !
Yes I know there's lots in that file, but I didn't merge that :)
cheers
^ permalink raw reply
* Re: [PATCH 3/4] powerpc/powernv: Check OPAL elog calls exist before using
From: Michael Ellerman @ 2014-09-15 3:13 UTC (permalink / raw)
To: Michael Neuling; +Cc: Stewart Smith, linuxppc-dev
In-Reply-To: <1408423682-14297-3-git-send-email-mikey@neuling.org>
On Tue, 2014-08-19 at 14:48 +1000, Michael Neuling wrote:
> Check that the OPAL_ELOG_READ token exists before initalising the elog
> infrastructure.
>
> This avoids littering the OPAL console with:
> "OPAL: Called with bad token 74"
>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
> arch/powerpc/platforms/powernv/opal-elog.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-elog.c b/arch/powerpc/platforms/powernv/opal-elog.c
> index bbdb3ff..518fe95 100644
> --- a/arch/powerpc/platforms/powernv/opal-elog.c
> +++ b/arch/powerpc/platforms/powernv/opal-elog.c
> @@ -295,6 +295,10 @@ int __init opal_elog_init(void)
> {
> int rc = 0;
>
> + /* ELOG not supported by firmware */
> + if (!opal_check_token(OPAL_ELOG_READ))
> + return -1;
> +
The only caller does:
/* Setup error log interface */
rc = opal_elog_init();
/* Setup code update interface */
opal_flash_init();
/* Setup platform dump extract interface */
opal_platform_dump_init();
/* Setup system parameters interface */
opal_sys_param_init();
/* Setup message log interface. */
opal_msglog_init();
}
return 0;
}
So we may as well have it return void. Wanna send a follow-up patch for that.
cheers
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/powernv: Add OPAL check token call
From: Michael Neuling @ 2014-09-15 3:19 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Stewart Smith, linuxppc-dev
In-Reply-To: <1410750623.24738.9.camel@concordia>
On Mon, 2014-09-15 at 13:10 +1000, Michael Ellerman wrote:
> On Tue, 2014-08-19 at 14:47 +1000, Michael Neuling wrote:
> > Currently there is no way to generically check if an OPAL call exists o=
r not
> > from the host kernel.
> >=20
> > This adds an OPAL call opal_check_token() which tells you if the given =
token is
> > present in OPAL or not.
> >=20
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
> > ---
> > arch/powerpc/include/asm/opal.h | 7 +++++++
> > arch/powerpc/platforms/powernv/opal-wrappers.S | 1 +
> > 2 files changed, 8 insertions(+)
> >=20
> > diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm=
/opal.h
> > index 86055e5..4593a93 100644
> > --- a/arch/powerpc/include/asm/opal.h
> > +++ b/arch/powerpc/include/asm/opal.h
> > @@ -135,6 +135,7 @@ struct opal_sg_list {
> > #define OPAL_FLASH_MANAGE 77
> > #define OPAL_FLASH_UPDATE 78
> > #define OPAL_RESYNC_TIMEBASE 79
> > +#define OPAL_CHECK_TOKEN 80
> > #define OPAL_DUMP_INIT 81
> > #define OPAL_DUMP_INFO 82
> > #define OPAL_DUMP_READ 83
> > @@ -417,6 +418,11 @@ struct opal_msg {
> > __be64 params[8];
> > };
> > =20
> > +enum OpalCheckTokenStatus {
> > + OPAL_TOKEN_ABSENT =3D 0,
> > + OPAL_TOKEN_PRESENT =3D 1
> > +};
>=20
> I don't see this used anywhere?
>=20
> And NoCamelCase !
We can probably just delete the enum since we are just doing this in
code anyway:
if (!opal_check_token(OPAL_RTC_READ))
goto out;
OK? Or would you prefer the usage to read:
if (opal_check_token(OPAL_RTC_READ) =3D=3D OPAL_TOKEN_ABSENT)
goto out;
> Yes I know there's lots in that file, but I didn't merge that :)
Fickle bloody maintainers. I thought I never say this but... I want our
crazy Frenchman back! :-P
Mikey
^ permalink raw reply
* Re: [PATCH] rtc/tpo: Driver to support rtc and wakeup on PowerNV platform
From: Michael Ellerman @ 2014-09-15 3:28 UTC (permalink / raw)
To: Neelesh Gupta; +Cc: a.zummo, tglx, linuxppc-dev, rtc-linux
In-Reply-To: <20140910053232.11176.35140.stgit@localhost.localdomain>
On Wed, 2014-09-10 at 11:08 +0530, Neelesh Gupta wrote:
> The patch implements the OPAL rtc driver that binds with the rtc
> driver subsystem. The driver uses the platform device infrastructure
> to probe the rtc device and register it to rtc class framework. The
> 'wakeup' is supported depending upon the property 'has-tpo' present
> in the OF node. It provides a way to load the generic rtc driver in
> in the absence of an OPAL driver.
>
> The patch also moves the existing OPAL rtc get/set time interfaces to the
> new driver and exposes the necessary OPAL calls using EXPORT_SYMBOL_GPL.
>
> Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com>
> ---
>
> Note:
> This patch depends upon the below patch posted on 'linuxppc-dev@lists.ozlabs.org'
> [PATCH 1/4] powerpc/powernv: Add OPAL check token call
> Signed-off-by: Michael Neuling <mikey@neuling.org>
>
> arch/powerpc/include/asm/opal.h | 7 -
> arch/powerpc/kernel/time.c | 1
> arch/powerpc/platforms/powernv/opal-async.c | 3
> arch/powerpc/platforms/powernv/opal-rtc.c | 63 ++----
> arch/powerpc/platforms/powernv/opal-wrappers.S | 2
> arch/powerpc/platforms/powernv/opal.c | 6 +
> arch/powerpc/platforms/powernv/setup.c | 2
> drivers/rtc/Kconfig | 11 +
> drivers/rtc/Makefile | 1
> drivers/rtc/rtc-opal.c | 257 ++++++++++++++++++++++++
> 10 files changed, 305 insertions(+), 48 deletions(-)
> create mode 100644 drivers/rtc/rtc-opal.c
Alessandro, given this depends on other things in the powerpc tree do you mind
if we take this through the powerpc tree with your Ack?
Or I can create a topic branch with the required powerpc changes.
cheers
^ permalink raw reply
* Re: [RFC PATCH] powerpc/numa: add ability to disable and debug topology updates
From: Michael Ellerman @ 2014-09-15 5:05 UTC (permalink / raw)
To: Nishanth Aravamudan; +Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <20140909200913.GG22906@linux.vnet.ibm.com>
On Tue, 2014-09-09 at 13:09 -0700, Nishanth Aravamudan wrote:
> We have hit a few customer issues with the topology update code (VPHN
> and PRRN). It would be nice to be able to debug the notifications coming
> from the hypervisor in both cases to the LPAR, as well as to disable
> reacting to the notifications, to narrow down the source of the
> problems. Add a basic level of such functionality, similar to the numa=
> command-line parameter.
>
> Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
> ---
> This is pretty rough, but has been useful in the field already. I'm not
> sure if more information would be useful than this basic amount.
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 5ae8608ca9f5..6e3b9e3a2ab4 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -3370,6 +3370,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> e.g. base its process migration decisions on it.
> Default is on.
>
> + topology_updates= [KNL, PPC, NUMA]
> + Format: {off | debug}
> + Specify if the kernel should ignore (off) or
> + emit more information (debug) when the
> + hypervisor sends NUMA topology updates to an
> + LPAR.
Boot-time parameters are kind of a pain, not least because they require a reboot
to activate.
Does it really need to be a boot param, or could it be a debugfs or sysctl
flag? ie. do we need to disable it immediately at boot or would it be OK if it
was /etc/rc.local or similar that turned it off ?
It looks like arch_update_cpu_topology() is called quite early, from
init_sched_domains(), but in that case I don't see how
cpu_associativity_changes_mask can be non-zero, ie. we'll never do any work
via that path.
As far as the debug goes, we could just use pr_debug() with
CONFIG_DYNAMIC_DEBUG, it's not quite as easy to enable as a kernel parameter
but for the odd bit of debugging it should be fine.
I guess the downside there is you have to do some work before you know if
you're printing anything out. More below ...
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index d7737a542fd7..72c5ad313cbe 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -1160,6 +1160,28 @@ static int __init early_numa(char *p)
> }
> early_param("numa", early_numa);
>
> +static int topology_updates_enabled = 1;
> +static int topology_updates_debug = 0;
> +
> +static int __init early_topology_updates(char *p)
> +{
> + if (!p)
> + return 0;
> +
> + if (strstr(p, "off")) {
> + printk(KERN_INFO "Disabling topology updates\n");
> + topology_updates_enabled = 0;
> + }
Can you add a:
#define pr_fmt(fmt) "numa: " fmt
At the top of the file and then use pr_xxx() for these?
> + if (strstr(p, "debug")) {
> + printk(KERN_INFO "Enabling topology updates debug\n");
> + topology_updates_debug = 1;
> + }
> +
> + return 0;
> +}
> +early_param("topology_updates", early_topology_updates);
> +
> #ifdef CONFIG_MEMORY_HOTPLUG
> /*
> * Find the node associated with a hot added memory section for
> @@ -1546,6 +1568,9 @@ int arch_update_cpu_topology(void)
> struct device *dev;
> int weight, new_nid, i = 0;
>
> + if (!topology_updates_enabled)
> + return 0;
> +
> weight = cpumask_weight(&cpu_associativity_changes_mask);
> if (!weight)
> return 0;
> @@ -1610,6 +1635,25 @@ int arch_update_cpu_topology(void)
> *
> * And for the similar reason, we will skip all the following updating.
> */
The comment should stay attached to the check below of updated_cpus, ie. this
block should preceed the comment.
> + if (topology_updates_debug) {
> + char *buf = kmalloc_array(NR_CPUS*5, sizeof(char), GFP_KERNEL);
> + cpumask_scnprintf(buf, NR_CPUS*5, &updated_cpus);
> + printk(KERN_DEBUG "Topology update for the following CPUs:\n");
> + printk(KERN_DEBUG " %s\n", buf);
> + printk(KERN_DEBUG "cpumask_weight(&updated_cpus)) = %u\n",
> + cpumask_weight(&updated_cpus));
Do we really need to print the cpumask? The same information is available below
right, just in a more verbose form?
Assuming we can skip that we can just use pr_debug() for these I think and drop
the debug flag. Or is this a super hot-path that I'm not aware of?
> + if (cpumask_weight(&updated_cpus)) {
> + for (ud = &updates[0]; ud; ud = ud->next) {
> + printk(KERN_DEBUG "cpu %d moving from node %d "
> + "to %d\n", ud->cpu,
> + ud->old_nid, ud->new_nid);
> + }
> + }
> + kfree(buf);
> + }
> +
> if (!cpumask_weight(&updated_cpus))
> goto out;
>
> @@ -1807,8 +1851,10 @@ static const struct file_operations topology_ops = {
>
> static int topology_update_init(void)
> {
> - start_topology_update();
> - proc_create("powerpc/topology_updates", 0644, NULL, &topology_ops);
> + if (topology_updates_enabled) {
> + start_topology_update();
> + proc_create("powerpc/topology_updates", 0644, NULL, &topology_ops);
> + }
It was already broken, but can you fix the (lack of) error handling here please?
>
> return 0;
> }
cheers
^ permalink raw reply
* Re: [PATCH v2 2/2] pseries: Fix endian issues in cpu hot-removal
From: Bharata B Rao @ 2014-09-15 6:46 UTC (permalink / raw)
To: Thomas Falcon; +Cc: linuxppc-dev
In-Reply-To: <1410549102-8410-2-git-send-email-tlfalcon@linux.vnet.ibm.com>
On Sat, Sep 13, 2014 at 12:41 AM, Thomas Falcon
<tlfalcon@linux.vnet.ibm.com> wrote:
> When removing a cpu, this patch makes sure that values
> gotten from or passed to firmware are in the correct
> endian format.
>
> Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
> ---
> Changes in v2:
>
> Followed suggestions from Michael Ellerman:
> Conversion of intserv to cpu endian occurs once.
> Conversion of drc_index to cpu endian occurs once
> using of_property_read_u32.
> ---
> arch/powerpc/platforms/pseries/dlpar.c | 20 +++++++++++---------
> arch/powerpc/platforms/pseries/hotplug-cpu.c | 10 ++++++----
> 2 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platfo=
rms/pseries/dlpar.c
> index 9e9f30b2..343dfdf 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -444,7 +444,8 @@ static int dlpar_offline_cpu(struct device_node *dn)
> int rc =3D 0;
> unsigned int cpu;
> int len, nthreads, i;
> - const u32 *intserv;
> + const __be32 *intserv;
> + u32 thread;
>
> intserv =3D of_get_property(dn, "ibm,ppc-interrupt-server#s", &le=
n);
> if (!intserv)
> @@ -454,8 +455,9 @@ static int dlpar_offline_cpu(struct device_node *dn)
>
> cpu_maps_update_begin();
> for (i =3D 0; i < nthreads; i++) {
> + thread =3D be32_to_cpu(intserv[i]);
> for_each_present_cpu(cpu) {
> - if (get_hard_smp_processor_id(cpu) !=3D intserv[i=
])
> + if (get_hard_smp_processor_id(cpu) !=3D thread)
> continue;
>
> if (get_cpu_current_state(cpu) =3D=3D CPU_STATE_O=
FFLINE)
> @@ -477,14 +479,14 @@ static int dlpar_offline_cpu(struct device_node *dn=
)
> * Upgrade it's state to CPU_STATE_OFFLINE.
> */
> set_preferred_offline_state(cpu, CPU_STATE_OFFLIN=
E);
> - BUG_ON(plpar_hcall_norets(H_PROD, intserv[i])
> + BUG_ON(plpar_hcall_norets(H_PROD, thread)
> !=3D H_SU=
CCESS);
> __cpu_die(cpu);
> break;
> }
> if (cpu =3D=3D num_possible_cpus())
> printk(KERN_WARNING "Could not find cpu to offlin=
e "
> - "with physical id 0x%x\n", intserv[i]);
> + "with physical id 0x%x\n", thread);
> }
> cpu_maps_update_done();
>
> @@ -496,15 +498,15 @@ out:
> static ssize_t dlpar_cpu_release(const char *buf, size_t count)
> {
> struct device_node *dn;
> - const u32 *drc_index;
> + const u32 drc_index;
> int rc;
>
> dn =3D of_find_node_by_path(buf);
> if (!dn)
> return -EINVAL;
>
> - drc_index =3D of_get_property(dn, "ibm,my-drc-index", NULL);
> - if (!drc_index) {
> + rc =3D of_property_read_u32(dn, "ibm,my-drc-index", &drc_index);
Use of const for drc_index causes compilation problems.
Warning on Fedora 20 BE
arch/powerpc/platforms/pseries/dlpar.c: In function =E2=80=98dlpar_cpu_rele=
ase=E2=80=99:
arch/powerpc/platforms/pseries/dlpar.c:508:2: warning: passing
argument 3 of =E2=80=98of_property_read_u32=E2=80=99 discards =E2=80=98cons=
t=E2=80=99 qualifier from
pointer target type [enabled by default]
rc =3D of_property_read_u32(dn, "ibm,my-drc-index", &drc_index);
^
In file included from arch/powerpc/platforms/pseries/dlpar.c:18:0:
include/linux/of.h:699:19: note: expected =E2=80=98u32 *=E2=80=99 but argum=
ent is of
type =E2=80=98const u32 *=E2=80=99
static inline int of_property_read_u32(const struct device_node *np,
Error on Ubuntu 14.04 LE
arch/powerpc/platforms/pseries/dlpar.c: In function =E2=80=98dlpar_cpu_rele=
ase=E2=80=99:
arch/powerpc/platforms/pseries/dlpar.c:508:2: error: passing argument
3 of =E2=80=98of_property_read_u32=E2=80=99 discards =E2=80=98const=E2=80=
=99 qualifier from pointer
target type [-Werror]
rc =3D of_property_read_u32(dn, "ibm,my-drc-index", &drc_index);
^
In file included from arch/powerpc/platforms/pseries/dlpar.c:18:0:
include/linux/of.h:699:19: note: expected =E2=80=98u32 *=E2=80=99 but argum=
ent is of
type =E2=80=98const u32 *=E2=80=99
static inline int of_property_read_u32(const struct device_node *np,
^
cc1: all warnings being treated as errors
Regards,
Bharata.
^ permalink raw reply
* Re: [PATCH] cpuidle/powernv: Enter fastsleep on checking if deep idle states are allowed
From: Michael Ellerman @ 2014-09-15 6:59 UTC (permalink / raw)
To: Preeti U Murthy; +Cc: mikey, linux-pm, rjw, linux-kernel, linuxppc-dev
In-Reply-To: <20140912110131.7311.86417.stgit@preeti.in.ibm.com>
On Fri, 2014-09-12 at 16:31 +0530, Preeti U Murthy wrote:
> Today the procfs interface /proc/sys/kernel/powersave-nap is used to control
> entry into deep idle states beyond snooze. Check for the value of this
> parameter before entering fastsleep. We already do this check for nap in
> power7_idle().
>
> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
> ---
>
> drivers/cpuidle/cpuidle-powernv.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
> index a64be57..b8ba52e 100644
> --- a/drivers/cpuidle/cpuidle-powernv.c
> +++ b/drivers/cpuidle/cpuidle-powernv.c
> @@ -69,6 +69,12 @@ static int fastsleep_loop(struct cpuidle_device *dev,
> unsigned long old_lpcr = mfspr(SPRN_LPCR);
> unsigned long new_lpcr;
>
> + /*
> + * Verify if snooze is the only valid cpuidle state
> + */
> + if (!(powersave_nap > 0))
> + return index;
> +
> if (unlikely(system_state < SYSTEM_RUNNING))
> return index;
Doesn't the above mean we are just going to keep trying to go into fastsleep
again and again? Or does the idle code work out that it didn't work based on
the fact that we didn't sleep for the right period?
We were talking about getting rid of powersave_nap altogether, but I think we
decided we couldn't, I forget.
cheers
^ permalink raw reply
* [PATCH v2 1/1] pseries: Make CPU hotplug path endian safe
From: bharata.rao @ 2014-09-15 7:03 UTC (permalink / raw)
To: linux-kernel; +Cc: Thomas Falcon, linuxppc-dev, Bharata B Rao
From: Bharata B Rao <bharata@linux.vnet.ibm.com>
- ibm,rtas-configure-connector should treat the RTAS data as big endian.
- Treat ibm,ppc-interrupt-server#s as big-endian when setting
smp_processor_id during hotplug.
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
Changes in v2:
- Don't convert drc_index to BE in dlpar_configure_connector() but instead
convert in the caller dlpar_cpu_probe() so that migration path isn't
affected.
- Mark members of cc_workarea struct as __be32 instead of u32 (Thomas)
- Based on top of Thomas Falcon's two patches.
(http://patchwork.ozlabs.org/patch/388767/)
v1: http://patchwork.ozlabs.org/patch/386216/
arch/powerpc/platforms/pseries/dlpar.c | 22 +++++++++++-----------
arch/powerpc/platforms/pseries/hotplug-cpu.c | 4 ++--
arch/powerpc/platforms/pseries/pseries.h | 3 ++-
3 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 8718227..52aeb37 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -24,11 +24,11 @@
#include <asm/rtas.h>
struct cc_workarea {
- u32 drc_index;
- u32 zero;
- u32 name_offset;
- u32 prop_length;
- u32 prop_offset;
+ __be32 drc_index;
+ __be32 zero;
+ __be32 name_offset;
+ __be32 prop_length;
+ __be32 prop_offset;
};
void dlpar_free_cc_property(struct property *prop)
@@ -48,11 +48,11 @@ static struct property *dlpar_parse_cc_property(struct cc_workarea *ccwa)
if (!prop)
return NULL;
- name = (char *)ccwa + ccwa->name_offset;
+ name = (char *)ccwa + be32_to_cpu(ccwa->name_offset);
prop->name = kstrdup(name, GFP_KERNEL);
- prop->length = ccwa->prop_length;
- value = (char *)ccwa + ccwa->prop_offset;
+ prop->length = be32_to_cpu(ccwa->prop_length);
+ value = (char *)ccwa + be32_to_cpu(ccwa->prop_offset);
prop->value = kmemdup(value, prop->length, GFP_KERNEL);
if (!prop->value) {
dlpar_free_cc_property(prop);
@@ -78,7 +78,7 @@ static struct device_node *dlpar_parse_cc_node(struct cc_workarea *ccwa,
if (!dn)
return NULL;
- name = (char *)ccwa + ccwa->name_offset;
+ name = (char *)ccwa + be32_to_cpu(ccwa->name_offset);
dn->full_name = kasprintf(GFP_KERNEL, "%s/%s", path, name);
if (!dn->full_name) {
kfree(dn);
@@ -125,7 +125,7 @@ void dlpar_free_cc_nodes(struct device_node *dn)
#define CALL_AGAIN -2
#define ERR_CFG_USE -9003
-struct device_node *dlpar_configure_connector(u32 drc_index,
+struct device_node *dlpar_configure_connector(__be32 drc_index,
struct device_node *parent)
{
struct device_node *dn;
@@ -413,7 +413,7 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
if (!parent)
return -ENODEV;
- dn = dlpar_configure_connector(drc_index, parent);
+ dn = dlpar_configure_connector(cpu_to_be32(drc_index), parent);
if (!dn)
return -EINVAL;
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index b174fa7..5c375f9 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -247,7 +247,7 @@ static int pseries_add_processor(struct device_node *np)
unsigned int cpu;
cpumask_var_t candidate_mask, tmp;
int err = -ENOSPC, len, nthreads, i;
- const u32 *intserv;
+ const __be32 *intserv;
intserv = of_get_property(np, "ibm,ppc-interrupt-server#s", &len);
if (!intserv)
@@ -293,7 +293,7 @@ static int pseries_add_processor(struct device_node *np)
for_each_cpu(cpu, tmp) {
BUG_ON(cpu_present(cpu));
set_cpu_present(cpu, true);
- set_hard_smp_processor_id(cpu, *intserv++);
+ set_hard_smp_processor_id(cpu, be32_to_cpu(*intserv++));
}
err = 0;
out_unlock:
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 361add6..1796c54 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -56,7 +56,8 @@ extern void hvc_vio_init_early(void);
/* Dynamic logical Partitioning/Mobility */
extern void dlpar_free_cc_nodes(struct device_node *);
extern void dlpar_free_cc_property(struct property *);
-extern struct device_node *dlpar_configure_connector(u32, struct device_node *);
+extern struct device_node *dlpar_configure_connector(__be32,
+ struct device_node *);
extern int dlpar_attach_node(struct device_node *);
extern int dlpar_detach_node(struct device_node *);
--
1.7.11.7
^ permalink raw reply related
* [PATCH] powerpc/eeh: Fix kernel crash when passing through VF
From: Wei Yang @ 2014-09-15 8:08 UTC (permalink / raw)
To: gwshan, linuxppc-dev, mpe; +Cc: Wei Yang
In-Reply-To: <1410406921-8557-1-git-send-email-weiyang@linux.vnet.ibm.com>
When doing vfio passthrough a VF, the kernel will crash with following
message:
[ 442.656459] Unable to handle kernel paging request for data at address 0x00000060
[ 442.656593] Faulting instruction address: 0xc000000000038b88
[ 442.656706] Oops: Kernel access of bad area, sig: 11 [#1]
[ 442.656798] SMP NR_CPUS=1024 NUMA PowerNV
[ 442.656890] Modules linked in: vfio_pci mlx4_core nf_conntrack_netbios_ns nf_conntrack_broadcast ipt_MASQUERADE ip6t_REJECT xt_conntrack bnep bluetooth rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw tg3 nfsd be2net nfs_acl ses lockd ptp enclosure pps_core kvm_hv kvm_pr shpchp binfmt_misc kvm sunrpc uinput lpfc scsi_transport_fc ipr scsi_tgt [last unloaded: mlx4_core]
[ 442.658152] CPU: 40 PID: 14948 Comm: qemu-system-ppc Not tainted 3.10.42yw-pkvm+ #37
[ 442.658219] task: c000000f7e2a9a00 ti: c000000f6dc3c000 task.ti: c000000f6dc3c000
[ 442.658287] NIP: c000000000038b88 LR: c0000000004435a8 CTR: c000000000455bc0
[ 442.658352] REGS: c000000f6dc3f580 TRAP: 0300 Not tainted (3.10.42yw-pkvm+)
[ 442.658419] MSR: 9000000000009032 <SF,HV,EE,ME,IR,DR,RI> CR: 28004882 XER: 20000000
[ 442.658577] CFAR: c00000000000908c DAR: 0000000000000060 DSISR: 40000000 SOFTE: 1
GPR00: c0000000004435a8 c000000f6dc3f800 c0000000012b1c10 c00000000da24000
GPR04: 0000000000000003 0000000000001004 00000000000015b3 000000000000ffff
GPR08: c00000000127f5d8 0000000000000000 000000000000ffff 0000000000000000
GPR12: c000000000068078 c00000000fdd6800 000001003c320c80 000001003c3607f0
GPR16: 0000000000000001 00000000105480c8 000000001055aaa8 000001003c31ab18
GPR20: 000001003c10fb40 000001003c360ae8 000000001063bcf0 000000001063bdb0
GPR24: 000001003c15ed70 0000000010548f40 c000001fe5514c88 c000001fe5514cb0
GPR28: c00000000da24000 0000000000000000 c00000000da24000 0000000000000003
[ 442.659471] NIP [c000000000038b88] .pcibios_set_pcie_reset_state+0x28/0x130
[ 442.659530] LR [c0000000004435a8] .pci_set_pcie_reset_state+0x28/0x40
[ 442.659585] Call Trace:
[ 442.659610] [c000000f6dc3f800] [00000000000719e0] 0x719e0 (unreliable)
[ 442.659677] [c000000f6dc3f880] [c0000000004435a8] .pci_set_pcie_reset_state+0x28/0x40
[ 442.659757] [c000000f6dc3f900] [c000000000455bf8] .reset_fundamental+0x38/0x80
[ 442.659835] [c000000f6dc3f980] [c0000000004562a8] .pci_dev_specific_reset+0xa8/0xf0
[ 442.659913] [c000000f6dc3fa00] [c0000000004448c4] .__pci_dev_reset+0x44/0x430
[ 442.659980] [c000000f6dc3fab0] [c000000000444d5c] .pci_reset_function+0x7c/0xc0
[ 442.660059] [c000000f6dc3fb30] [d00000001c141ab8] .vfio_pci_open+0xe8/0x2b0 [vfio_pci]
[ 442.660139] [c000000f6dc3fbd0] [c000000000586c30] .vfio_group_fops_unl_ioctl+0x3a0/0x630
[ 442.660219] [c000000f6dc3fc90] [c000000000255fbc] .do_vfs_ioctl+0x4ec/0x7c0
[ 442.660286] [c000000f6dc3fd80] [c000000000256364] .SyS_ioctl+0xd4/0xf0
[ 442.660354] [c000000f6dc3fe30] [c000000000009e54] syscall_exit+0x0/0x98
[ 442.660420] Instruction dump:
[ 442.660454] 4bfffce9 4bfffee4 7c0802a6 fbc1fff0 fbe1fff8 f8010010 f821ff81 7c7e1b78
[ 442.660566] 7c9f2378 60000000 60000000 e93e02c8 <e8690060> 2fa30000 41de00c4 2b9f0002
[ 442.660679] ---[ end trace a64ac9546bcf0328 ]---
[ 442.660724]
The reason is current VF is not EEH enabled.
This patch introduces a marco to convert eeh_dev to eeh_pe. By doing so, it
will prevent converting with NULL pointer.
Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
V2 -> V3:
1. rebased on 3.17-rc4
2. introduce a marco
3. use this marco in several other places
V1 -> V2:
1. code style and patch subject adjustment
---
arch/powerpc/kernel/eeh.c | 4 ++--
arch/powerpc/kernel/eeh_pe.c | 2 +-
include/linux/pci.h | 5 +++++
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 59a64f8..0f1b637 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -410,7 +410,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
}
dn = eeh_dev_to_of_node(edev);
dev = eeh_dev_to_pci_dev(edev);
- pe = edev->pe;
+ pe = eeh_dev_to_pe(edev);
/* Access to IO BARs might get this far and still not want checking. */
if (!pe) {
@@ -634,7 +634,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
{
struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
- struct eeh_pe *pe = edev->pe;
+ struct eeh_pe *pe = eeh_dev_to_pe(edev);
if (!pe) {
pr_err("%s: No PE found on PCI device %s\n",
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 00e3844..5864017 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -428,7 +428,7 @@ int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
}
/* Remove the EEH device */
- pe = edev->pe;
+ pe = eeh_dev_to_pe(edev);
edev->pe = NULL;
list_del(&edev->list);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index fd03e819..9656f92 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1824,6 +1824,11 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
{
return pdev->dev.archdata.edev;
}
+
+static inline struct eeh_pe *eeh_dev_to_pe(struct eeh_dev* edev)
+{
+ return edev ? edev->pe : NULL;
+}
#endif
int pci_for_each_dma_alias(struct pci_dev *pdev,
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2 1/3] PCI/MSI/PPC: Remove arch_msi_check_device()
From: Alexander Gordeev @ 2014-09-15 8:34 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linux-pci, linuxppc-dev, linux-kernel
In-Reply-To: <1410748970.24738.3.camel@concordia>
On Mon, Sep 15, 2014 at 12:42:50PM +1000, Michael Ellerman wrote:
> On Sun, 2014-09-07 at 20:57 +0200, Alexander Gordeev wrote:
> > Moving MSI checks from arch_msi_check_device() function to
> > arch_setup_msi_irqs() function makes code more compact and
> > allows removing unnecessary hook arch_msi_check_device()
> > from generic MSI code.
> >
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: linux-pci@vger.kernel.org
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
>
> I already acked the previous version, but if you didn't want it that's fine :)
I do want it, but I thought you may not agree with the new changelog ;)
> cheers
>
>
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply
* Data Cache Push Parity Error
From: Jay_Chen @ 2014-09-15 8:15 UTC (permalink / raw)
To: linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 854 bytes --]
Hi all,
I am using P2020 with Linux kernel 3.8.13.
Sometimes my system encounters machine check exception and it shows “Caused by (from MCSR=20000000): Data Cache Push Parity Error”.
Could anyone show me some hints to dig into this issue? Any comment is welcome.
Thanks in advance.
Jay Chen…
This electronic mail transmission is intended only for the named recipient. It contains information which may be privileged,confidential and exempt from disclosure under applicable law. Dissemination, distribution, or copying of this communication by anyone other than the recipient or the recipient's agent is strictly prohibited. If this electronic mail transmission is received in error, Please notify us immediately and delete the message and all attachments of it from your computer system. Thank you for your cooperation.
[-- Attachment #2: Type: text/html, Size: 4054 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/iommu/ddw: Fix endianness
From: Alexey Kardashevskiy @ 2014-09-15 8:41 UTC (permalink / raw)
Cc: Alexander Graf, Nishanth Aravamudan, Gavin Shan, linux-kernel,
Paul Mackerras, Anton Blanchard, Alistair Popple, linuxppc-dev
In-Reply-To: <1410261723-21810-1-git-send-email-aik@ozlabs.ru>
On 09/09/2014 09:22 PM, Alexey Kardashevskiy wrote:
> rtas_call() accepts and returns values in CPU endianness.
>
> of_read_number() accepts big-endian values but create.addr_hi/lo returned
> by rtas_call() are in CPU endiannes.
>
> The dynamic_dma_window_prop struct defines all members as BE so let's
> make it true.
>
> struct dynamic_dma_window_prop {
> __be32 liobn; /* tce table number */
> __be64 dma_base; /* address hi,lo */
> __be32 tce_shift; /* ilog2(tce_page_size) */
> __be32 window_shift; /* ilog2(tce_window_size) */
> };
>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Ping, anyone?
> ---
> arch/powerpc/platforms/pseries/iommu.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index 7c1d77c..700020a 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -750,7 +750,7 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
> pr_debug("%s successfully cleared tces in window.\n",
> np->full_name);
>
> - ret = rtas_call(ddw_avail[2], 1, 1, NULL, liobn);
> + ret = rtas_call(be32_to_cpu(ddw_avail[2]), 1, 1, NULL, liobn);
> if (ret)
> pr_warning("%s: failed to remove direct window: rtas returned "
> "%d to ibm,remove-pe-dma-window(%x) %llx\n",
> @@ -842,7 +842,7 @@ static int query_ddw(struct pci_dev *dev, const u32 *ddw_avail,
> cfg_addr = edev->pe_config_addr;
> buid = edev->phb->buid;
>
> - ret = rtas_call(ddw_avail[0], 3, 5, (u32 *)query,
> + ret = rtas_call(be32_to_cpu(ddw_avail[0]), 3, 5, (u32 *)query,
> cfg_addr, BUID_HI(buid), BUID_LO(buid));
> dev_info(&dev->dev, "ibm,query-pe-dma-windows(%x) %x %x %x"
> " returned %d\n", ddw_avail[0], cfg_addr, BUID_HI(buid),
> @@ -874,8 +874,9 @@ static int create_ddw(struct pci_dev *dev, const u32 *ddw_avail,
>
> do {
> /* extra outputs are LIOBN and dma-addr (hi, lo) */
> - ret = rtas_call(ddw_avail[1], 5, 4, (u32 *)create, cfg_addr,
> - BUID_HI(buid), BUID_LO(buid), page_shift, window_shift);
> + ret = rtas_call(be32_to_cpu(ddw_avail[1]), 5, 4, (u32 *)create,
> + cfg_addr, BUID_HI(buid), BUID_LO(buid),
> + page_shift, window_shift);
> } while (rtas_busy_delay(ret));
> dev_info(&dev->dev,
> "ibm,create-pe-dma-window(%x) %x %x %x %x %x returned %d "
> @@ -972,11 +973,11 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> dev_dbg(&dev->dev, "no free dynamic windows");
> goto out_failed;
> }
> - if (be32_to_cpu(query.page_size) & 4) {
> + if (query.page_size & 4) {
> page_shift = 24; /* 16MB */
> - } else if (be32_to_cpu(query.page_size) & 2) {
> + } else if (query.page_size & 2) {
> page_shift = 16; /* 64kB */
> - } else if (be32_to_cpu(query.page_size) & 1) {
> + } else if (query.page_size & 1) {
> page_shift = 12; /* 4kB */
> } else {
> dev_dbg(&dev->dev, "no supported direct page size in mask %x",
> @@ -987,7 +988,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> /* verify the window * number of ptes will map the partition */
> /* check largest block * page size > max memory hotplug addr */
> max_addr = memory_hotplug_max();
> - if (be32_to_cpu(query.largest_available_block) < (max_addr >> page_shift)) {
> + if (query.largest_available_block < (max_addr >> page_shift)) {
> dev_dbg(&dev->dev, "can't map partiton max 0x%llx with %u "
> "%llu-sized pages\n", max_addr, query.largest_available_block,
> 1ULL << page_shift);
> @@ -1014,8 +1015,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> if (ret != 0)
> goto out_free_prop;
>
> - ddwprop->liobn = create.liobn;
> - ddwprop->dma_base = cpu_to_be64(of_read_number(&create.addr_hi, 2));
> + ddwprop->liobn = cpu_to_be32(create.liobn);
> + ddwprop->dma_base = cpu_to_be64(((u64)create.addr_hi << 32) |
> + create.addr_lo);
> ddwprop->tce_shift = cpu_to_be32(page_shift);
> ddwprop->window_shift = cpu_to_be32(len);
>
> @@ -1048,7 +1050,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> list_add(&window->list, &direct_window_list);
> spin_unlock(&direct_window_list_lock);
>
> - dma_addr = of_read_number(&create.addr_hi, 2);
> + dma_addr = be64_to_cpu(ddwprop->dma_base);
> goto out_unlock;
>
> out_free_window:
>
--
Alexey
^ permalink raw reply
* Re: [PATCH] cpuidle/powernv: Enter fastsleep on checking if deep idle states are allowed
From: Preeti U Murthy @ 2014-09-15 8:52 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, mikey, rjw, linux-kernel, linux-pm
In-Reply-To: <1410764352.32643.2.camel@concordia>
On 09/15/2014 12:29 PM, Michael Ellerman wrote:
> On Fri, 2014-09-12 at 16:31 +0530, Preeti U Murthy wrote:
>> Today the procfs interface /proc/sys/kernel/powersave-nap is used to control
>> entry into deep idle states beyond snooze. Check for the value of this
>> parameter before entering fastsleep. We already do this check for nap in
>> power7_idle().
>>
>> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>> ---
>>
>> drivers/cpuidle/cpuidle-powernv.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
>> index a64be57..b8ba52e 100644
>> --- a/drivers/cpuidle/cpuidle-powernv.c
>> +++ b/drivers/cpuidle/cpuidle-powernv.c
>> @@ -69,6 +69,12 @@ static int fastsleep_loop(struct cpuidle_device *dev,
>> unsigned long old_lpcr = mfspr(SPRN_LPCR);
>> unsigned long new_lpcr;
>>
>> + /*
>> + * Verify if snooze is the only valid cpuidle state
>> + */
>> + if (!(powersave_nap > 0))
>> + return index;
>> +
>> if (unlikely(system_state < SYSTEM_RUNNING))
>> return index;
>
> Doesn't the above mean we are just going to keep trying to go into fastsleep
> again and again? Or does the idle code work out that it didn't work based on
> the fact that we didn't sleep for the right period?
Thats right. The idle code figures that its judgment to enter fastsleep
was not correct and applies a correction factor to its future decisions
. This correction factor is intended to influence the cpuidle governor's
decision on choosing an idle state for the cpu based on the history of
wakeups. Hence a shallower idle state will be chosen here on in the
above circumstance.
>
> We were talking about getting rid of powersave_nap altogether, but I think we
> decided we couldn't, I forget.
Isn't this a helpful knob to disable cpuidle at runtime? Currently we
check the value of powersave_nap before entering both nap and fastsleep.
Regards
Preeti U Murthy
>
> cheers
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: [PATCH] powerpc/iommu/ddw: Fix endianness
From: Michael Ellerman @ 2014-09-15 9:18 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Alexander Graf, Nishanth Aravamudan, Gavin Shan, linux-kernel,
Paul Mackerras, Anton Blanchard, Alistair Popple, linuxppc-dev
In-Reply-To: <5416A64E.1060005@ozlabs.ru>
On Mon, 2014-09-15 at 18:41 +1000, Alexey Kardashevskiy wrote:
> On 09/09/2014 09:22 PM, Alexey Kardashevskiy wrote:
> > rtas_call() accepts and returns values in CPU endianness.
Sounds right.
> > of_read_number() accepts big-endian values but create.addr_hi/lo returned
> > by rtas_call() are in CPU endiannes.
Also sounds right.
> > The dynamic_dma_window_prop struct defines all members as BE so let's
> > make it true.
It does. But why does it do that? It seems to be allocated and setup in
enable_ddw() and then the only place I see it used is in
tce_setrange_multi_pSeriesLP()/tce_clearrange_multi_pSeriesLP(), which both
unpack it again. What am I missing?
> > struct dynamic_dma_window_prop {
> > __be32 liobn; /* tce table number */
> > __be64 dma_base; /* address hi,lo */
> > __be32 tce_shift; /* ilog2(tce_page_size) */
> > __be32 window_shift; /* ilog2(tce_window_size) */
> > };
We do read them from the device tree in find_existing_ddw_windows(), but if
that's the only place then the conversion to cpu endian should happen there.
> > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > index 7c1d77c..700020a 100644
> > --- a/arch/powerpc/platforms/pseries/iommu.c
> > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > @@ -750,7 +750,7 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
> > pr_debug("%s successfully cleared tces in window.\n",
> > np->full_name);
> >
> > - ret = rtas_call(ddw_avail[2], 1, 1, NULL, liobn);
> > + ret = rtas_call(be32_to_cpu(ddw_avail[2]), 1, 1, NULL, liobn);
The conversion should happen once where ever ddw_avail comes out of the device
tree, rather than everywhere it's used.
cheers
^ permalink raw reply
* Re: [PATCH V3] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Markus Pargmann @ 2014-09-15 10:05 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, timur, perex,
nicoleotsuka, broonie, linuxppc-dev, linux-kernel
In-Reply-To: <894383e00876763f22988fc5f3f9f232f939f923.1410517971.git.shengjiu.wang@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 5842 bytes --]
On Fri, Sep 12, 2014 at 06:35:15PM +0800, Shengjiu Wang wrote:
> Check if ipg clock is in clock-names property, then we can move the
> ipg clock enable and disable operation to startup and shutdown, that
> is only enable ipg clock when ssi is working and keep clock is disabled
> when ssi is in idle.
> But when the checking is failed, remain the clock control as before.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
> ---
> V3 change log:
> update patch according Nicolin and markus's comments
>
>
> sound/soc/fsl/fsl_ssi.c | 53 ++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 45 insertions(+), 8 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> index 2fc3e66..6d1dfd5 100644
> --- a/sound/soc/fsl/fsl_ssi.c
> +++ b/sound/soc/fsl/fsl_ssi.c
> @@ -169,6 +169,7 @@ struct fsl_ssi_private {
> u8 i2s_mode;
> bool use_dma;
> bool use_dual_fifo;
> + bool has_ipg_clk_name;
> unsigned int fifo_depth;
> struct fsl_ssi_rxtx_reg_val rxtx_reg_val;
>
> @@ -530,6 +531,11 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
> struct snd_soc_pcm_runtime *rtd = substream->private_data;
> struct fsl_ssi_private *ssi_private =
> snd_soc_dai_get_drvdata(rtd->cpu_dai);
> + int ret;
> +
> + ret = clk_prepare_enable(ssi_private->clk);
> + if (ret)
> + return ret;
>
> /* When using dual fifo mode, it is safer to ensure an even period
> * size. If appearing to an odd number while DMA always starts its
> @@ -544,6 +550,21 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
> }
>
> /**
> + * fsl_ssi_shutdown: shutdown the SSI
> + *
> + */
> +static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct fsl_ssi_private *ssi_private =
> + snd_soc_dai_get_drvdata(rtd->cpu_dai);
> +
> + clk_disable_unprepare(ssi_private->clk);
> +
> +}
> +
> +/**
> * fsl_ssi_set_bclk - configure Digital Audio Interface bit clock
> *
> * Note: This function can be only called when using SSI as DAI master
> @@ -1043,6 +1064,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)
>
> static const struct snd_soc_dai_ops fsl_ssi_dai_ops = {
> .startup = fsl_ssi_startup,
> + .shutdown = fsl_ssi_shutdown,
> .hw_params = fsl_ssi_hw_params,
> .hw_free = fsl_ssi_hw_free,
> .set_fmt = fsl_ssi_set_dai_fmt,
> @@ -1168,17 +1190,22 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
> u32 dmas[4];
> int ret;
>
> - ssi_private->clk = devm_clk_get(&pdev->dev, NULL);
> + if (ssi_private->has_ipg_clk_name)
> + ssi_private->clk = devm_clk_get(&pdev->dev, "ipg");
> + else
> + ssi_private->clk = devm_clk_get(&pdev->dev, NULL);
> if (IS_ERR(ssi_private->clk)) {
> ret = PTR_ERR(ssi_private->clk);
> dev_err(&pdev->dev, "could not get clock: %d\n", ret);
> return ret;
> }
>
> - ret = clk_prepare_enable(ssi_private->clk);
> - if (ret) {
> - dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", ret);
> - return ret;
> + if (!ssi_private->has_ipg_clk_name) {
> + ret = clk_prepare_enable(ssi_private->clk);
> + if (ret) {
> + dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", ret);
> + return ret;
> + }
> }
>
> /* For those SLAVE implementations, we ingore non-baudclk cases
> @@ -1236,8 +1263,9 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
> return 0;
>
> error_pcm:
> - clk_disable_unprepare(ssi_private->clk);
>
> + if (!ssi_private->has_ipg_clk_name)
> + clk_disable_unprepare(ssi_private->clk);
> return ret;
> }
>
> @@ -1246,7 +1274,8 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev,
> {
> if (!ssi_private->use_dma)
> imx_pcm_fiq_exit(pdev);
> - clk_disable_unprepare(ssi_private->clk);
> + if (!ssi_private->has_ipg_clk_name)
> + clk_disable_unprepare(ssi_private->clk);
> }
>
> static int fsl_ssi_probe(struct platform_device *pdev)
> @@ -1321,8 +1350,16 @@ static int fsl_ssi_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> + ret = of_property_match_string(np, "clock-names", "ipg");
> + if (ret < 0) {
> + ssi_private->has_ipg_clk_name = false;
> + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> &fsl_ssi_regconfig);
Sorry if I was unclear about that. My suggestion was to enable the clock
right here:
clk_prepare_enable(ssi_private->clk);
Then you can remove ssi_private->has_ipg_clk_name and all
clk_prepare_enable() and clk_disable_unprepare() from above. Also you
can move the devm_clk_get() into this block.
It seems you really want to implement this for devicetrees where the
"ipg" clock-name is missing, but I don't understand why? I really can't
see any benefit of adding all these clk_prepare_enable() calls for all
cornercases that may occure. For example the clocks for AC97 are still
missing in this version.
Best regards,
Markus
> + } else {
> + ssi_private->has_ipg_clk_name = true;
> + ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev,
> + "ipg", iomem, &fsl_ssi_regconfig);
> + }
> if (IS_ERR(ssi_private->regs)) {
> dev_err(&pdev->dev, "Failed to init register map\n");
> return PTR_ERR(ssi_private->regs);
> --
> 1.7.9.5
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH V3] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Shengjiu Wang @ 2014-09-15 10:22 UTC (permalink / raw)
To: Markus Pargmann
Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, timur, perex,
nicoleotsuka, broonie, linuxppc-dev, linux-kernel
In-Reply-To: <20140915100541.GD8844@pengutronix.de>
On Mon, Sep 15, 2014 at 12:05:41PM +0200, Markus Pargmann wrote:
> On Fri, Sep 12, 2014 at 06:35:15PM +0800, Shengjiu Wang wrote:
> > Check if ipg clock is in clock-names property, then we can move the
> > ipg clock enable and disable operation to startup and shutdown, that
> > is only enable ipg clock when ssi is working and keep clock is disabled
> > when ssi is in idle.
> > But when the checking is failed, remain the clock control as before.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
> > ---
> > V3 change log:
> > update patch according Nicolin and markus's comments
> >
> >
> > sound/soc/fsl/fsl_ssi.c | 53 ++++++++++++++++++++++++++++++++++++++++-------
> > 1 file changed, 45 insertions(+), 8 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> > index 2fc3e66..6d1dfd5 100644
> > --- a/sound/soc/fsl/fsl_ssi.c
> > +++ b/sound/soc/fsl/fsl_ssi.c
> > @@ -169,6 +169,7 @@ struct fsl_ssi_private {
> > u8 i2s_mode;
> > bool use_dma;
> > bool use_dual_fifo;
> > + bool has_ipg_clk_name;
> > unsigned int fifo_depth;
> > struct fsl_ssi_rxtx_reg_val rxtx_reg_val;
> >
> > @@ -530,6 +531,11 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
> > struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > struct fsl_ssi_private *ssi_private =
> > snd_soc_dai_get_drvdata(rtd->cpu_dai);
> > + int ret;
> > +
> > + ret = clk_prepare_enable(ssi_private->clk);
> > + if (ret)
> > + return ret;
> >
> > /* When using dual fifo mode, it is safer to ensure an even period
> > * size. If appearing to an odd number while DMA always starts its
> > @@ -544,6 +550,21 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
> > }
> >
> > /**
> > + * fsl_ssi_shutdown: shutdown the SSI
> > + *
> > + */
> > +static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
> > + struct snd_soc_dai *dai)
> > +{
> > + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > + struct fsl_ssi_private *ssi_private =
> > + snd_soc_dai_get_drvdata(rtd->cpu_dai);
> > +
> > + clk_disable_unprepare(ssi_private->clk);
> > +
> > +}
> > +
> > +/**
> > * fsl_ssi_set_bclk - configure Digital Audio Interface bit clock
> > *
> > * Note: This function can be only called when using SSI as DAI master
> > @@ -1043,6 +1064,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)
> >
> > static const struct snd_soc_dai_ops fsl_ssi_dai_ops = {
> > .startup = fsl_ssi_startup,
> > + .shutdown = fsl_ssi_shutdown,
> > .hw_params = fsl_ssi_hw_params,
> > .hw_free = fsl_ssi_hw_free,
> > .set_fmt = fsl_ssi_set_dai_fmt,
> > @@ -1168,17 +1190,22 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
> > u32 dmas[4];
> > int ret;
> >
> > - ssi_private->clk = devm_clk_get(&pdev->dev, NULL);
> > + if (ssi_private->has_ipg_clk_name)
> > + ssi_private->clk = devm_clk_get(&pdev->dev, "ipg");
> > + else
> > + ssi_private->clk = devm_clk_get(&pdev->dev, NULL);
> > if (IS_ERR(ssi_private->clk)) {
> > ret = PTR_ERR(ssi_private->clk);
> > dev_err(&pdev->dev, "could not get clock: %d\n", ret);
> > return ret;
> > }
> >
> > - ret = clk_prepare_enable(ssi_private->clk);
> > - if (ret) {
> > - dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", ret);
> > - return ret;
> > + if (!ssi_private->has_ipg_clk_name) {
> > + ret = clk_prepare_enable(ssi_private->clk);
> > + if (ret) {
> > + dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", ret);
> > + return ret;
> > + }
> > }
> >
> > /* For those SLAVE implementations, we ingore non-baudclk cases
> > @@ -1236,8 +1263,9 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
> > return 0;
> >
> > error_pcm:
> > - clk_disable_unprepare(ssi_private->clk);
> >
> > + if (!ssi_private->has_ipg_clk_name)
> > + clk_disable_unprepare(ssi_private->clk);
> > return ret;
> > }
> >
> > @@ -1246,7 +1274,8 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev,
> > {
> > if (!ssi_private->use_dma)
> > imx_pcm_fiq_exit(pdev);
> > - clk_disable_unprepare(ssi_private->clk);
> > + if (!ssi_private->has_ipg_clk_name)
> > + clk_disable_unprepare(ssi_private->clk);
> > }
> >
> > static int fsl_ssi_probe(struct platform_device *pdev)
> > @@ -1321,8 +1350,16 @@ static int fsl_ssi_probe(struct platform_device *pdev)
> > return -ENOMEM;
> > }
> >
> > - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > + ret = of_property_match_string(np, "clock-names", "ipg");
> > + if (ret < 0) {
> > + ssi_private->has_ipg_clk_name = false;
> > + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > &fsl_ssi_regconfig);
>
> Sorry if I was unclear about that. My suggestion was to enable the clock
> right here:
> clk_prepare_enable(ssi_private->clk);
>
> Then you can remove ssi_private->has_ipg_clk_name and all
> clk_prepare_enable() and clk_disable_unprepare() from above. Also you
> can move the devm_clk_get() into this block.
>
ipg clock not only need to be enabled when accessing register, but also
need to be enabled when ssi is working. So I add clk_enable in startup.
> It seems you really want to implement this for devicetrees where the
> "ipg" clock-name is missing, but I don't understand why? I really can't
> see any benefit of adding all these clk_prepare_enable() calls for all
> cornercases that may occure. For example the clocks for AC97 are still
> missing in this version.
When ipg clock-name is missing, I just remain the code logic as before.
So AC97 case is same as before too.
When have ipg clock-name, register it to regmap for accessing register,
and enabling ipg clock in startup also is useful for AC97.
I don't understand why you think clock for AC97 is still missing?
wang shengjiu
>
> Best regards,
>
> Markus
>
> > + } else {
> > + ssi_private->has_ipg_clk_name = true;
> > + ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev,
> > + "ipg", iomem, &fsl_ssi_regconfig);
> > + }
> > if (IS_ERR(ssi_private->regs)) {
> > dev_err(&pdev->dev, "Failed to init register map\n");
> > return PTR_ERR(ssi_private->regs);
> > --
> > 1.7.9.5
> >
> >
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH] powerpc/iommu/ddw: Fix endianness
From: Alexey Kardashevskiy @ 2014-09-15 10:26 UTC (permalink / raw)
To: Michael Ellerman
Cc: Alexander Graf, Nishanth Aravamudan, Gavin Shan, linux-kernel,
Paul Mackerras, Anton Blanchard, Alistair Popple, linuxppc-dev
In-Reply-To: <1410772688.3931.0.camel@concordia>
On 09/15/2014 07:18 PM, Michael Ellerman wrote:
> On Mon, 2014-09-15 at 18:41 +1000, Alexey Kardashevskiy wrote:
>> On 09/09/2014 09:22 PM, Alexey Kardashevskiy wrote:
>>> rtas_call() accepts and returns values in CPU endianness.
>
> Sounds right.
>
>>> of_read_number() accepts big-endian values but create.addr_hi/lo returned
>>> by rtas_call() are in CPU endiannes.
>
> Also sounds right.
>
>>> The dynamic_dma_window_prop struct defines all members as BE so let's
>>> make it true.
>
> It does. But why does it do that? It seems to be allocated and setup in
> enable_ddw() and then the only place I see it used is in
> tce_setrange_multi_pSeriesLP()/tce_clearrange_multi_pSeriesLP(), which both
> unpack it again. What am I missing?
I do not know why they are BE. I just know that create_ddw() returns
ddw_create_response struct which members are declared as BE but they are
not as rtas_call() already made them CPU-endian.
May be rtas_call() must not be used for structs. Or these structs must be
fixed to be CPU endian. Cannot choose what/how to fix here. Sure I still
can miss something here and it is all correct and I have to fix QEMU.
>>> struct dynamic_dma_window_prop {
>>> __be32 liobn; /* tce table number */
>>> __be64 dma_base; /* address hi,lo */
>>> __be32 tce_shift; /* ilog2(tce_page_size) */
>>> __be32 window_shift; /* ilog2(tce_window_size) */
>>> };
>
> We do read them from the device tree in find_existing_ddw_windows(), but if
> that's the only place then the conversion to cpu endian should happen there.
enable_ddw() kmalloc's ddwprop which is of this dynamic_dma_window_prop type.
Then enable_ddw() initializes properties of that ddwprop thing but does it
incorrectly.
Then it calls walk_system_ram_range() which eventually calls
tce_setrange_multi_pSeriesLP() to map every single page of guest's ram, the
dynamic_dma_window_prop struct pointer is a void* argument of that callback.
find_existing_ddw_windows() handles something called
"linux,direct64-ddr-window-info" which is not a part of DDW at all and it
is not from PAPR and this patch is not about it.
>>> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
>>> index 7c1d77c..700020a 100644
>>> --- a/arch/powerpc/platforms/pseries/iommu.c
>>> +++ b/arch/powerpc/platforms/pseries/iommu.c
>>> @@ -750,7 +750,7 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
>>> pr_debug("%s successfully cleared tces in window.\n",
>>> np->full_name);
>>>
>>> - ret = rtas_call(ddw_avail[2], 1, 1, NULL, liobn);
>>> + ret = rtas_call(be32_to_cpu(ddw_avail[2]), 1, 1, NULL, liobn);
>
> The conversion should happen once where ever ddw_avail comes out of the device
> tree, rather than everywhere it's used.
ddw_avail is a pointer to device tree property value:
ddw_avail = of_get_property(np, "ibm,ddw-applicable", &len);
This contains 3 tokens, only 2 of them are actually used by the DDW code in
its current state and each of them is used just once in the guest's
lifetime. Older guest kernels would use the "reset" extension token but
again - only once. I fail to see the point in caching CPU-endian values of
these tokens.
>
> cheers
>
>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH V3] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Markus Pargmann @ 2014-09-15 10:32 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, timur, perex,
nicoleotsuka, broonie, linuxppc-dev, linux-kernel
In-Reply-To: <20140915102224.GA23877@audiosh1>
[-- Attachment #1: Type: text/plain, Size: 6863 bytes --]
On Mon, Sep 15, 2014 at 06:22:27PM +0800, Shengjiu Wang wrote:
> On Mon, Sep 15, 2014 at 12:05:41PM +0200, Markus Pargmann wrote:
> > On Fri, Sep 12, 2014 at 06:35:15PM +0800, Shengjiu Wang wrote:
> > > Check if ipg clock is in clock-names property, then we can move the
> > > ipg clock enable and disable operation to startup and shutdown, that
> > > is only enable ipg clock when ssi is working and keep clock is disabled
> > > when ssi is in idle.
> > > But when the checking is failed, remain the clock control as before.
> > >
> > > Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
> > > ---
> > > V3 change log:
> > > update patch according Nicolin and markus's comments
> > >
> > >
> > > sound/soc/fsl/fsl_ssi.c | 53 ++++++++++++++++++++++++++++++++++++++++-------
> > > 1 file changed, 45 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> > > index 2fc3e66..6d1dfd5 100644
> > > --- a/sound/soc/fsl/fsl_ssi.c
> > > +++ b/sound/soc/fsl/fsl_ssi.c
> > > @@ -169,6 +169,7 @@ struct fsl_ssi_private {
> > > u8 i2s_mode;
> > > bool use_dma;
> > > bool use_dual_fifo;
> > > + bool has_ipg_clk_name;
> > > unsigned int fifo_depth;
> > > struct fsl_ssi_rxtx_reg_val rxtx_reg_val;
> > >
> > > @@ -530,6 +531,11 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
> > > struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > > struct fsl_ssi_private *ssi_private =
> > > snd_soc_dai_get_drvdata(rtd->cpu_dai);
> > > + int ret;
> > > +
> > > + ret = clk_prepare_enable(ssi_private->clk);
> > > + if (ret)
> > > + return ret;
> > >
> > > /* When using dual fifo mode, it is safer to ensure an even period
> > > * size. If appearing to an odd number while DMA always starts its
> > > @@ -544,6 +550,21 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
> > > }
> > >
> > > /**
> > > + * fsl_ssi_shutdown: shutdown the SSI
> > > + *
> > > + */
> > > +static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
> > > + struct snd_soc_dai *dai)
> > > +{
> > > + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > > + struct fsl_ssi_private *ssi_private =
> > > + snd_soc_dai_get_drvdata(rtd->cpu_dai);
> > > +
> > > + clk_disable_unprepare(ssi_private->clk);
> > > +
> > > +}
> > > +
> > > +/**
> > > * fsl_ssi_set_bclk - configure Digital Audio Interface bit clock
> > > *
> > > * Note: This function can be only called when using SSI as DAI master
> > > @@ -1043,6 +1064,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)
> > >
> > > static const struct snd_soc_dai_ops fsl_ssi_dai_ops = {
> > > .startup = fsl_ssi_startup,
> > > + .shutdown = fsl_ssi_shutdown,
> > > .hw_params = fsl_ssi_hw_params,
> > > .hw_free = fsl_ssi_hw_free,
> > > .set_fmt = fsl_ssi_set_dai_fmt,
> > > @@ -1168,17 +1190,22 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
> > > u32 dmas[4];
> > > int ret;
> > >
> > > - ssi_private->clk = devm_clk_get(&pdev->dev, NULL);
> > > + if (ssi_private->has_ipg_clk_name)
> > > + ssi_private->clk = devm_clk_get(&pdev->dev, "ipg");
> > > + else
> > > + ssi_private->clk = devm_clk_get(&pdev->dev, NULL);
> > > if (IS_ERR(ssi_private->clk)) {
> > > ret = PTR_ERR(ssi_private->clk);
> > > dev_err(&pdev->dev, "could not get clock: %d\n", ret);
> > > return ret;
> > > }
> > >
> > > - ret = clk_prepare_enable(ssi_private->clk);
> > > - if (ret) {
> > > - dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", ret);
> > > - return ret;
> > > + if (!ssi_private->has_ipg_clk_name) {
> > > + ret = clk_prepare_enable(ssi_private->clk);
> > > + if (ret) {
> > > + dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", ret);
> > > + return ret;
> > > + }
> > > }
> > >
> > > /* For those SLAVE implementations, we ingore non-baudclk cases
> > > @@ -1236,8 +1263,9 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
> > > return 0;
> > >
> > > error_pcm:
> > > - clk_disable_unprepare(ssi_private->clk);
> > >
> > > + if (!ssi_private->has_ipg_clk_name)
> > > + clk_disable_unprepare(ssi_private->clk);
> > > return ret;
> > > }
> > >
> > > @@ -1246,7 +1274,8 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev,
> > > {
> > > if (!ssi_private->use_dma)
> > > imx_pcm_fiq_exit(pdev);
> > > - clk_disable_unprepare(ssi_private->clk);
> > > + if (!ssi_private->has_ipg_clk_name)
> > > + clk_disable_unprepare(ssi_private->clk);
> > > }
> > >
> > > static int fsl_ssi_probe(struct platform_device *pdev)
> > > @@ -1321,8 +1350,16 @@ static int fsl_ssi_probe(struct platform_device *pdev)
> > > return -ENOMEM;
> > > }
> > >
> > > - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > > + ret = of_property_match_string(np, "clock-names", "ipg");
> > > + if (ret < 0) {
> > > + ssi_private->has_ipg_clk_name = false;
> > > + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > > &fsl_ssi_regconfig);
> >
> > Sorry if I was unclear about that. My suggestion was to enable the clock
> > right here:
> > clk_prepare_enable(ssi_private->clk);
> >
> > Then you can remove ssi_private->has_ipg_clk_name and all
> > clk_prepare_enable() and clk_disable_unprepare() from above. Also you
> > can move the devm_clk_get() into this block.
> >
>
> ipg clock not only need to be enabled when accessing register, but also
> need to be enabled when ssi is working. So I add clk_enable in startup.
>
> > It seems you really want to implement this for devicetrees where the
> > "ipg" clock-name is missing, but I don't understand why? I really can't
> > see any benefit of adding all these clk_prepare_enable() calls for all
> > cornercases that may occure. For example the clocks for AC97 are still
> > missing in this version.
>
> When ipg clock-name is missing, I just remain the code logic as before.
> So AC97 case is same as before too.
> When have ipg clock-name, register it to regmap for accessing register,
> and enabling ipg clock in startup also is useful for AC97.
>
> I don't understand why you think clock for AC97 is still missing?
Sorry got it all wrong, monday morning tiredness perhaps ;-).
The patch looks good for me at the second look. I will test it later
today and give you feedback.
Best regards,
Markus
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH V3] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Shengjiu Wang @ 2014-09-15 10:37 UTC (permalink / raw)
To: Markus Pargmann
Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, timur, perex,
nicoleotsuka, broonie, linuxppc-dev, linux-kernel
In-Reply-To: <20140915103213.GE8844@pengutronix.de>
On Mon, Sep 15, 2014 at 12:32:13PM +0200, Markus Pargmann wrote:
> On Mon, Sep 15, 2014 at 06:22:27PM +0800, Shengjiu Wang wrote:
> > On Mon, Sep 15, 2014 at 12:05:41PM +0200, Markus Pargmann wrote:
> > > On Fri, Sep 12, 2014 at 06:35:15PM +0800, Shengjiu Wang wrote:
> > > > Check if ipg clock is in clock-names property, then we can move the
> > > > ipg clock enable and disable operation to startup and shutdown, that
> > > > is only enable ipg clock when ssi is working and keep clock is disabled
> > > > when ssi is in idle.
> > > > But when the checking is failed, remain the clock control as before.
> > > >
> > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
> > > > ---
> > > > V3 change log:
> > > > update patch according Nicolin and markus's comments
> > > >
> > > >
> > > > sound/soc/fsl/fsl_ssi.c | 53 ++++++++++++++++++++++++++++++++++++++++-------
> > > > 1 file changed, 45 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> > > > index 2fc3e66..6d1dfd5 100644
> > > > --- a/sound/soc/fsl/fsl_ssi.c
> > > > +++ b/sound/soc/fsl/fsl_ssi.c
> > > > @@ -169,6 +169,7 @@ struct fsl_ssi_private {
> > > > u8 i2s_mode;
> > > > bool use_dma;
> > > > bool use_dual_fifo;
> > > > + bool has_ipg_clk_name;
> > > > unsigned int fifo_depth;
> > > > struct fsl_ssi_rxtx_reg_val rxtx_reg_val;
> > > >
> > > > @@ -530,6 +531,11 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
> > > > struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > > > struct fsl_ssi_private *ssi_private =
> > > > snd_soc_dai_get_drvdata(rtd->cpu_dai);
> > > > + int ret;
> > > > +
> > > > + ret = clk_prepare_enable(ssi_private->clk);
> > > > + if (ret)
> > > > + return ret;
> > > >
> > > > /* When using dual fifo mode, it is safer to ensure an even period
> > > > * size. If appearing to an odd number while DMA always starts its
> > > > @@ -544,6 +550,21 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
> > > > }
> > > >
> > > > /**
> > > > + * fsl_ssi_shutdown: shutdown the SSI
> > > > + *
> > > > + */
> > > > +static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
> > > > + struct snd_soc_dai *dai)
> > > > +{
> > > > + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > > > + struct fsl_ssi_private *ssi_private =
> > > > + snd_soc_dai_get_drvdata(rtd->cpu_dai);
> > > > +
> > > > + clk_disable_unprepare(ssi_private->clk);
> > > > +
> > > > +}
> > > > +
> > > > +/**
> > > > * fsl_ssi_set_bclk - configure Digital Audio Interface bit clock
> > > > *
> > > > * Note: This function can be only called when using SSI as DAI master
> > > > @@ -1043,6 +1064,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)
> > > >
> > > > static const struct snd_soc_dai_ops fsl_ssi_dai_ops = {
> > > > .startup = fsl_ssi_startup,
> > > > + .shutdown = fsl_ssi_shutdown,
> > > > .hw_params = fsl_ssi_hw_params,
> > > > .hw_free = fsl_ssi_hw_free,
> > > > .set_fmt = fsl_ssi_set_dai_fmt,
> > > > @@ -1168,17 +1190,22 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
> > > > u32 dmas[4];
> > > > int ret;
> > > >
> > > > - ssi_private->clk = devm_clk_get(&pdev->dev, NULL);
> > > > + if (ssi_private->has_ipg_clk_name)
> > > > + ssi_private->clk = devm_clk_get(&pdev->dev, "ipg");
> > > > + else
> > > > + ssi_private->clk = devm_clk_get(&pdev->dev, NULL);
> > > > if (IS_ERR(ssi_private->clk)) {
> > > > ret = PTR_ERR(ssi_private->clk);
> > > > dev_err(&pdev->dev, "could not get clock: %d\n", ret);
> > > > return ret;
> > > > }
> > > >
> > > > - ret = clk_prepare_enable(ssi_private->clk);
> > > > - if (ret) {
> > > > - dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", ret);
> > > > - return ret;
> > > > + if (!ssi_private->has_ipg_clk_name) {
> > > > + ret = clk_prepare_enable(ssi_private->clk);
> > > > + if (ret) {
> > > > + dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", ret);
> > > > + return ret;
> > > > + }
> > > > }
> > > >
> > > > /* For those SLAVE implementations, we ingore non-baudclk cases
> > > > @@ -1236,8 +1263,9 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
> > > > return 0;
> > > >
> > > > error_pcm:
> > > > - clk_disable_unprepare(ssi_private->clk);
> > > >
> > > > + if (!ssi_private->has_ipg_clk_name)
> > > > + clk_disable_unprepare(ssi_private->clk);
> > > > return ret;
> > > > }
> > > >
> > > > @@ -1246,7 +1274,8 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev,
> > > > {
> > > > if (!ssi_private->use_dma)
> > > > imx_pcm_fiq_exit(pdev);
> > > > - clk_disable_unprepare(ssi_private->clk);
> > > > + if (!ssi_private->has_ipg_clk_name)
> > > > + clk_disable_unprepare(ssi_private->clk);
> > > > }
> > > >
> > > > static int fsl_ssi_probe(struct platform_device *pdev)
> > > > @@ -1321,8 +1350,16 @@ static int fsl_ssi_probe(struct platform_device *pdev)
> > > > return -ENOMEM;
> > > > }
> > > >
> > > > - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > > > + ret = of_property_match_string(np, "clock-names", "ipg");
> > > > + if (ret < 0) {
> > > > + ssi_private->has_ipg_clk_name = false;
> > > > + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > > > &fsl_ssi_regconfig);
> > >
> > > Sorry if I was unclear about that. My suggestion was to enable the clock
> > > right here:
> > > clk_prepare_enable(ssi_private->clk);
> > >
> > > Then you can remove ssi_private->has_ipg_clk_name and all
> > > clk_prepare_enable() and clk_disable_unprepare() from above. Also you
> > > can move the devm_clk_get() into this block.
> > >
> >
> > ipg clock not only need to be enabled when accessing register, but also
> > need to be enabled when ssi is working. So I add clk_enable in startup.
> >
> > > It seems you really want to implement this for devicetrees where the
> > > "ipg" clock-name is missing, but I don't understand why? I really can't
> > > see any benefit of adding all these clk_prepare_enable() calls for all
> > > cornercases that may occure. For example the clocks for AC97 are still
> > > missing in this version.
> >
> > When ipg clock-name is missing, I just remain the code logic as before.
> > So AC97 case is same as before too.
> > When have ipg clock-name, register it to regmap for accessing register,
> > and enabling ipg clock in startup also is useful for AC97.
> >
> > I don't understand why you think clock for AC97 is still missing?
>
> Sorry got it all wrong, monday morning tiredness perhaps ;-).
>
> The patch looks good for me at the second look. I will test it later
> today and give you feedback.
>
> Best regards,
>
> Markus
Thank you very much. Your comments help me a lot. and wait your feedback.
best regards
wang shengjiu
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH] drivers: char: hw_random: printk replacement
From: Sudip Mukherjee @ 2014-09-15 12:03 UTC (permalink / raw)
To: Herbert Xu
Cc: Olof Johansson, linuxppc-dev, linux-kernel, linux-geode,
Matt Mackall
In-Reply-To: <20140915114123.GA4843@gondor.apana.org.au>
On Mon, Sep 15, 2014 at 07:41:23PM +0800, Herbert Xu wrote:
> On Thu, Aug 28, 2014 at 08:32:58PM +0530, Sudip Mukherjee wrote:
> > as pr_* macros are more preffered over printk, so printk replaced with corresponding pr_* macros
> >
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
>
> Your patch doesn't even build:
>
> CC [M] drivers/char/hw_random/intel-rng.o
> CC [M] drivers/char/hw_random/amd-rng.o
> CC [M] drivers/char/hw_random/via-rng.o
> drivers/char/hw_random/intel-rng.c: In function ‘intel_init_hw_struct’:
> drivers/char/hw_random/intel-rng.c:317:3: error: expected ‘)’ before ‘warning’
> drivers/char/hw_random/intel-rng.c:308:36: warning: unused variable ‘warning’ [-Wunused-variable]
> make[1]: *** [drivers/char/hw_random/intel-rng.o] Error 1
>
> Cheers,
> --
i am extremely sorry for the mess. after applying the patch i tested with allmodconfig
and seeing no error i thought it was ok. but now after your mail when i am verifying i
noticed that allmodconfig is not including it in the config and as a result it was not tested.
i will post a corrected v2 of the patch.
sorry again,
thanks
sudip
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] drivers: char: hw_random: printk replacement
From: Herbert Xu @ 2014-09-15 11:41 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Olof Johansson, linuxppc-dev, linux-kernel, linux-geode,
Matt Mackall
In-Reply-To: <1409238178-16496-1-git-send-email-sudipm.mukherjee@gmail.com>
On Thu, Aug 28, 2014 at 08:32:58PM +0530, Sudip Mukherjee wrote:
> as pr_* macros are more preffered over printk, so printk replaced with corresponding pr_* macros
>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
Your patch doesn't even build:
CC [M] drivers/char/hw_random/intel-rng.o
CC [M] drivers/char/hw_random/amd-rng.o
CC [M] drivers/char/hw_random/via-rng.o
drivers/char/hw_random/intel-rng.c: In function ‘intel_init_hw_struct’:
drivers/char/hw_random/intel-rng.c:317:3: error: expected ‘)’ before ‘warning’
drivers/char/hw_random/intel-rng.c:308:36: warning: unused variable ‘warning’ [-Wunused-variable]
make[1]: *** [drivers/char/hw_random/intel-rng.o] Error 1
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] cpuidle/powernv: Enter fastsleep on checking if deep idle states are allowed
From: Benjamin Herrenschmidt @ 2014-09-15 12:23 UTC (permalink / raw)
To: Preeti U Murthy; +Cc: mikey, linux-pm, rjw, linux-kernel, linuxppc-dev
In-Reply-To: <5416A8B0.8040603@linux.vnet.ibm.com>
On Mon, 2014-09-15 at 14:22 +0530, Preeti U Murthy wrote:
> > We were talking about getting rid of powersave_nap altogether, but I
> think we
> > decided we couldn't, I forget.
>
> Isn't this a helpful knob to disable cpuidle at runtime? Currently we
> check the value of powersave_nap before entering both nap and
> fastsleep.
It's useful when diagnosing some problems that can be caused by idle
states.
Cheers,
Ben.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox