* Re: [PATCH] powerpc/iommu: Remove unused pci_iommu_init() and pci_direct_iommu_init()
From: Benjamin Herrenschmidt @ 2013-06-11 21:27 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20130611195705.17794.68833.stgit@bhelgaas-glaptop>
On Tue, 2013-06-11 at 13:57 -0600, Bjorn Helgaas wrote:
> pci_iommu_init() and pci_direct_iommu_init() are not referenced anywhere,
> so remove them.
Ah indeed, some old stuff...
Thanks,
Ben.
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> arch/powerpc/include/asm/iommu.h | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
> index cbfe678..e670f73 100644
> --- a/arch/powerpc/include/asm/iommu.h
> +++ b/arch/powerpc/include/asm/iommu.h
> @@ -125,13 +125,6 @@ extern void iommu_init_early_pSeries(void);
> extern void iommu_init_early_dart(void);
> extern void iommu_init_early_pasemi(void);
>
> -#ifdef CONFIG_PCI
> -extern void pci_iommu_init(void);
> -extern void pci_direct_iommu_init(void);
> -#else
> -static inline void pci_iommu_init(void) { }
> -#endif
> -
> extern void alloc_dart_table(void);
> #if defined(CONFIG_PPC64) && defined(CONFIG_PM)
> static inline void iommu_save(void)
^ permalink raw reply
* Re: [PATCH -V7 09/18] powerpc: Switch 16GB and 16MB explicit hugepages to a different page table format
From: Scott Wood @ 2013-06-11 22:50 UTC (permalink / raw)
To: Scott Wood; +Cc: linux-mm, linuxppc-dev, paulus, Aneesh Kumar K.V, dwg
In-Reply-To: <1370984023.18413.30@snotra>
On 06/11/2013 03:53:43 PM, Scott Wood wrote:
> On 06/08/2013 11:57:48 AM, Aneesh Kumar K.V wrote:
>> With the config shared I am not finding anything wrong, but I can't =20
>> test
>> these configs. Also can you confirm what you bisect this to
>>=20
>> e2b3d202d1dba8f3546ed28224ce485bc50010be
>> powerpc: Switch 16GB and 16MB explicit hugepages to a different page =20
>> table format
>=20
>>=20
>> or
>>=20
>> cf9427b85e90bb1ff90e2397ff419691d983c68b "powerpc: New hugepage =20
>> directory format"
>=20
> It's e2b3d202d1dba8f3546ed28224ce485bc50010be.
>=20
> It turned out to be the change from "pmd_none" to =20
> "pmd_none_or_clear_bad". Making that change triggers the "bad pmd" =20
> messages even when applied to v3.9 -- so we had bad pmds all along, =20
> undetected. Now I get to figure out why. :-(
So, for both pud and pgd we only call "or_clear_bad" when is_hugepd =20
returns false. Why is it OK to do it unconditionally for pmd?
-Scott=
^ permalink raw reply
* Re: [PATCH 23/27] powernv/opal: Notifier for OPAL events
From: Benjamin Herrenschmidt @ 2013-06-12 0:32 UTC (permalink / raw)
To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1370417668-16832-24-git-send-email-shangw@linux.vnet.ibm.com>
On Wed, 2013-06-05 at 15:34 +0800, Gavin Shan wrote:
> The patch intends to implement the notifier for variable OPAL events.
> It's notable that the notifier can be disabled dynamically. Also, the
> notifier could be fired upon incoming OPAL interrupts, or enabling
> the OPAL notifier.
"This patch implements a notifier to receive a notification on OPAL
event mask changes." is probably better. No need to blurb about
enable/disable, however add something along the lines of
"The notifier is only called as a result of an OPAL interrupt, which
will happen upon reception of FSP messages or PCI errors. Any event
mask change detected as a result of opal_poll_events() will not result
in a notifier call.
With OPALv3, opal_poll_event() will not clear interrupt conditions from
the FSP however, even if it consumes the messages (and thus updates the
event mask). Thus the interrupt notifier is a reliable way to get
the completion for FSP based OPAL operations. The specific list will
be added to the header file.
> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/opal.h | 3 +
> arch/powerpc/platforms/powernv/opal.c | 79 ++++++++++++++++++++++++++++++++-
> 2 files changed, 81 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 2880797..64e7c84 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -644,6 +644,9 @@ extern void hvc_opal_init_early(void);
> extern int early_init_dt_scan_opal(unsigned long node, const char *uname,
> int depth, void *data);
>
> +extern int opal_notifier_register(uint64_t mask, void (*cb)(uint64_t));
> +extern void opal_notifier_enable(bool enable);
Make it two functions
opal_enable_notifier() vs. opal_disable_notifier()
> extern int opal_get_chars(uint32_t vtermno, char *buf, int count);
> extern int opal_put_chars(uint32_t vtermno, const char *buf, int total_len);
>
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index 628c564..9bbbf93 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -26,11 +26,20 @@ struct opal {
> u64 entry;
> } opal;
>
> +struct opal_cb {
> + struct list_head list;
> + uint64_t mask;
> + void (*cb)(uint64_t);
> +};
> +
> static struct device_node *opal_node;
> static DEFINE_SPINLOCK(opal_write_lock);
> extern u64 opal_mc_secondary_handler[];
> static unsigned int *opal_irqs;
> static unsigned int opal_irq_count;
> +static LIST_HEAD(opal_notifier);
> +static DEFINE_SPINLOCK(opal_notifier_lock);
> +static atomic_t opal_notifier_hold = ATOMIC_INIT(0);
>
> int __init early_init_dt_scan_opal(unsigned long node,
> const char *uname, int depth, void *data)
> @@ -95,6 +104,74 @@ static int __init opal_register_exception_handlers(void)
>
> early_initcall(opal_register_exception_handlers);
>
> +int opal_notifier_register(uint64_t mask, void (*cb)(uint64_t))
> +{
> + unsigned long flags;
> + struct opal_cb *p, *tmp;
> +
> + if (!mask || !cb) {
> + pr_warning("%s: Invalid argument (%llx, %p)!\n",
> + __func__, mask, cb);
> + return -EINVAL;
> + }
> +
> + p = kzalloc(sizeof(*p), GFP_KERNEL);
> + if (!p) {
> + pr_warning("%s: Out of memory (%llx, %p)!\n",
> + __func__, mask, cb);
> + return -ENOMEM;
> + }
> + p->mask = mask;
> + p->cb = cb;
> +
> + spin_lock_irqsave(&opal_notifier_lock, flags);
> + list_for_each_entry(tmp, &opal_notifier, list) {
> + if (tmp->cb == cb || tmp->mask & mask) {
> + pr_warning("%s: Duplicate evnet handler (%llx, %p)\n",
> + __func__, tmp->mask, tmp->cb);
> + spin_unlock_irqrestore(&opal_notifier_lock, flags);
> + kfree(p);
> + return -EEXIST;
> + }
> + }
Don't bother with checking the list already. This is not useful. Also
it's fine for two things to listen on the same event.
> +
> + list_add_tail(&p->list, &opal_notifier);
> + spin_unlock_irqrestore(&opal_notifier_lock, flags);
> +
> + return 0;
> +}
> +
> +static void opal_do_notifier(uint64_t events)
> +{
> + struct opal_cb *tmp;
> +
> + if (atomic_read(&opal_notifier_hold))
> + return;
> + if (!events)
> + return;
> +
> + list_for_each_entry(tmp, &opal_notifier, list) {
> + if (events & tmp->mask)
> + tmp->cb(events & tmp->mask);
> + }
> +}
My idea was to call this if the event bit has changed since the last
time we called opal_do_notifier. IE. Use a static last_notified_mask
and do something like
changed_mask = last_notified_mask ^ events;
list_for_each_entry(tmp, &opal_notifier, list) {
if (changed_mask & tmp->mask)
tmp->cb(events);
Also, always pass the whole events to the callback, no point in
filtering.
BTW, "tmp" isn't a nice name here.
> +void opal_notifier_enable(bool enable)
> +{
> + int64_t rc;
> + uint64_t evt = 0;
> +
> + if (enable) {
> + atomic_set(&opal_notifier_hold, 0);
> +
> + /* Process pending events */
> + rc = opal_poll_events(&evt);
> + if (rc == OPAL_SUCCESS && evt)
> + opal_do_notifier(evt);
> + } else
> + atomic_set(&opal_notifier_hold, 1);
> +}
As I said, two functions.
> int opal_get_chars(uint32_t vtermno, char *buf, int count)
> {
> s64 len, rc;
> @@ -297,7 +374,7 @@ static irqreturn_t opal_interrupt(int irq, void *data)
>
> opal_handle_interrupt(virq_to_hw(irq), &events);
>
> - /* XXX TODO: Do something with the events */
> + opal_do_notifier(events);
>
> return IRQ_HANDLED;
> }
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 0/4 v3] KVM: PPC: IOMMU in-kernel handling
From: Benjamin Herrenschmidt @ 2013-06-12 3:14 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: kvm, linux-kernel, kvm-ppc, Alexander Graf, Paul Mackerras,
linuxppc-dev, David Gibson
In-Reply-To: <1370412673-1345-1-git-send-email-aik@ozlabs.ru>
On Wed, 2013-06-05 at 16:11 +1000, Alexey Kardashevskiy wrote:
> Ben, ping! :)
>
> This series has tiny fixes (capability and ioctl numbers,
> changed documentation, compile errors in some configuration).
> More details are in the commit messages.
> Rebased on v3.10-rc4.
Alex, I assume you'll merge that once I ack it ?
Cheers,
Ben.
>
> Alexey Kardashevskiy (4):
> KVM: PPC: Add support for multiple-TCE hcalls
> powerpc: Prepare to support kernel handling of IOMMU map/unmap
> KVM: PPC: Add support for IOMMU in-kernel handling
> KVM: PPC: Add hugepage support for IOMMU in-kernel handling
>
> Documentation/virtual/kvm/api.txt | 45 +++
> arch/powerpc/include/asm/kvm_host.h | 7 +
> arch/powerpc/include/asm/kvm_ppc.h | 40 ++-
> arch/powerpc/include/asm/pgtable-ppc64.h | 4 +
> arch/powerpc/include/uapi/asm/kvm.h | 7 +
> arch/powerpc/kvm/book3s_64_vio.c | 398 ++++++++++++++++++++++++-
> arch/powerpc/kvm/book3s_64_vio_hv.c | 471 ++++++++++++++++++++++++++++--
> arch/powerpc/kvm/book3s_hv.c | 39 +++
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 6 +
> arch/powerpc/kvm/book3s_pr_papr.c | 37 ++-
> arch/powerpc/kvm/powerpc.c | 15 +
> arch/powerpc/mm/init_64.c | 77 ++++-
> include/uapi/linux/kvm.h | 3 +
> 13 files changed, 1121 insertions(+), 28 deletions(-)
>
^ permalink raw reply
* Re: [PATCH 23/27] powernv/opal: Notifier for OPAL events
From: Gavin Shan @ 2013-06-12 3:15 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Gavin Shan
In-Reply-To: <1370997149.8250.158.camel@pasglop>
On Wed, Jun 12, 2013 at 10:32:29AM +1000, Benjamin Herrenschmidt wrote:
>On Wed, 2013-06-05 at 15:34 +0800, Gavin Shan wrote:
>> The patch intends to implement the notifier for variable OPAL events.
>> It's notable that the notifier can be disabled dynamically. Also, the
>> notifier could be fired upon incoming OPAL interrupts, or enabling
>> the OPAL notifier.
>
>"This patch implements a notifier to receive a notification on OPAL
>event mask changes." is probably better. No need to blurb about
>enable/disable, however add something along the lines of
>
>"The notifier is only called as a result of an OPAL interrupt, which
>will happen upon reception of FSP messages or PCI errors. Any event
>mask change detected as a result of opal_poll_events() will not result
>in a notifier call.
>
>With OPALv3, opal_poll_event() will not clear interrupt conditions from
>the FSP however, even if it consumes the messages (and thus updates the
>event mask). Thus the interrupt notifier is a reliable way to get
>the completion for FSP based OPAL operations. The specific list will
>be added to the header file.
>
>
Thanks, Ben. Will update the changelog accordingly.
>> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/include/asm/opal.h | 3 +
>> arch/powerpc/platforms/powernv/opal.c | 79 ++++++++++++++++++++++++++++++++-
>> 2 files changed, 81 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
>> index 2880797..64e7c84 100644
>> --- a/arch/powerpc/include/asm/opal.h
>> +++ b/arch/powerpc/include/asm/opal.h
>> @@ -644,6 +644,9 @@ extern void hvc_opal_init_early(void);
>> extern int early_init_dt_scan_opal(unsigned long node, const char *uname,
>> int depth, void *data);
>>
>> +extern int opal_notifier_register(uint64_t mask, void (*cb)(uint64_t));
>> +extern void opal_notifier_enable(bool enable);
>
>Make it two functions
>
>opal_enable_notifier() vs. opal_disable_notifier()
>
Ok. Will do.
>> extern int opal_get_chars(uint32_t vtermno, char *buf, int count);
>> extern int opal_put_chars(uint32_t vtermno, const char *buf, int total_len);
>>
>> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
>> index 628c564..9bbbf93 100644
>> --- a/arch/powerpc/platforms/powernv/opal.c
>> +++ b/arch/powerpc/platforms/powernv/opal.c
>> @@ -26,11 +26,20 @@ struct opal {
>> u64 entry;
>> } opal;
>>
>> +struct opal_cb {
>> + struct list_head list;
>> + uint64_t mask;
>> + void (*cb)(uint64_t);
>> +};
>> +
>> static struct device_node *opal_node;
>> static DEFINE_SPINLOCK(opal_write_lock);
>> extern u64 opal_mc_secondary_handler[];
>> static unsigned int *opal_irqs;
>> static unsigned int opal_irq_count;
>> +static LIST_HEAD(opal_notifier);
>> +static DEFINE_SPINLOCK(opal_notifier_lock);
>> +static atomic_t opal_notifier_hold = ATOMIC_INIT(0);
>>
>> int __init early_init_dt_scan_opal(unsigned long node,
>> const char *uname, int depth, void *data)
>> @@ -95,6 +104,74 @@ static int __init opal_register_exception_handlers(void)
>>
>> early_initcall(opal_register_exception_handlers);
>>
>> +int opal_notifier_register(uint64_t mask, void (*cb)(uint64_t))
>> +{
>> + unsigned long flags;
>> + struct opal_cb *p, *tmp;
>> +
>> + if (!mask || !cb) {
>> + pr_warning("%s: Invalid argument (%llx, %p)!\n",
>> + __func__, mask, cb);
>> + return -EINVAL;
>> + }
>> +
>> + p = kzalloc(sizeof(*p), GFP_KERNEL);
>> + if (!p) {
>> + pr_warning("%s: Out of memory (%llx, %p)!\n",
>> + __func__, mask, cb);
>> + return -ENOMEM;
>> + }
>> + p->mask = mask;
>> + p->cb = cb;
>> +
>> + spin_lock_irqsave(&opal_notifier_lock, flags);
>> + list_for_each_entry(tmp, &opal_notifier, list) {
>> + if (tmp->cb == cb || tmp->mask & mask) {
>> + pr_warning("%s: Duplicate evnet handler (%llx, %p)\n",
>> + __func__, tmp->mask, tmp->cb);
>> + spin_unlock_irqrestore(&opal_notifier_lock, flags);
>> + kfree(p);
>> + return -EEXIST;
>> + }
>> + }
>
>Don't bother with checking the list already. This is not useful. Also
>it's fine for two things to listen on the same event.
>
Ok. Will update in next revision.
>> +
>> + list_add_tail(&p->list, &opal_notifier);
>> + spin_unlock_irqrestore(&opal_notifier_lock, flags);
>> +
>> + return 0;
>> +}
>> +
>> +static void opal_do_notifier(uint64_t events)
>> +{
>> + struct opal_cb *tmp;
>> +
>> + if (atomic_read(&opal_notifier_hold))
>> + return;
>> + if (!events)
>> + return;
>> +
>> + list_for_each_entry(tmp, &opal_notifier, list) {
>> + if (events & tmp->mask)
>> + tmp->cb(events & tmp->mask);
>> + }
>> +}
>
>My idea was to call this if the event bit has changed since the last
>time we called opal_do_notifier. IE. Use a static last_notified_mask
>and do something like
>
> changed_mask = last_notified_mask ^ events;
>
> list_for_each_entry(tmp, &opal_notifier, list) {
> if (changed_mask & tmp->mask)
> tmp->cb(events);
>
>Also, always pass the whole events to the callback, no point in
>filtering.
>
>BTW, "tmp" isn't a nice name here.
>
Ok. Will update in next revision:
- Allow multiple "clients" for same event.
- Make the variable "tmp" to have better name.
>> +void opal_notifier_enable(bool enable)
>> +{
>> + int64_t rc;
>> + uint64_t evt = 0;
>> +
>> + if (enable) {
>> + atomic_set(&opal_notifier_hold, 0);
>> +
>> + /* Process pending events */
>> + rc = opal_poll_events(&evt);
>> + if (rc == OPAL_SUCCESS && evt)
>> + opal_do_notifier(evt);
>> + } else
>> + atomic_set(&opal_notifier_hold, 1);
>> +}
>
>As I said, two functions.
>
Ok.
>> int opal_get_chars(uint32_t vtermno, char *buf, int count)
>> {
>> s64 len, rc;
>> @@ -297,7 +374,7 @@ static irqreturn_t opal_interrupt(int irq, void *data)
>>
>> opal_handle_interrupt(virq_to_hw(irq), &events);
>>
>> - /* XXX TODO: Do something with the events */
>> + opal_do_notifier(events);
>>
>> return IRQ_HANDLED;
>> }
>
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH v3 00/27] EEH Support for PowerNV platform
From: Gavin Shan @ 2013-06-12 3:18 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Gavin Shan
In-Reply-To: <1370936784.8250.96.camel@pasglop>
On Tue, Jun 11, 2013 at 05:46:24PM +1000, Benjamin Herrenschmidt wrote:
>On Wed, 2013-06-05 at 15:34 +0800, Gavin Shan wrote:
>> Initially, the series of patches is built based on 3.10.RC1 and the patchset
>> doesn't intend to enable EEH functionality for PHB3 for now. Obviously, PHB3
>> EEH support on PowerNV platform is something to do in future.
>
>One thing missing here is a first patch that moves the eeh core out of
>platform/pseries or things will simply not build if CONFIG_PPC_PSERIES
>isn't enabled :-)
>
>Move the whole lot to arch/powerpc/kernel
>
Ok. Will make the first patch to do it in next revision :-)
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH] powerpc/sysfs: disable hotplug for the boot cpu
From: Benjamin Herrenschmidt @ 2013-06-12 3:25 UTC (permalink / raw)
To: Zhao Chenhui; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20130603104315.GA1365@localhost.localdomain>
On Mon, 2013-06-03 at 18:43 +0800, Zhao Chenhui wrote:
> On Sat, Jun 01, 2013 at 07:49:44AM +1000, Benjamin Herrenschmidt wrote:
> > On Tue, 2013-05-28 at 15:59 +0800, Zhao Chenhui wrote:
> > > Some features depend on the boot cpu, for instance, hibernate/suspend.
> > > So disable hotplug for the boot cpu.
> >
> > Don't we have code to "move" the boot CPU around when that happens ?
> >
> > Ben.
> >
>
> Currently, the code in generic_cpu_disable() likes this:
>
> if (cpu == boot_cpuid)
> return -EBUSY;
But the code in pseries/hotplug-cpu.c doesn't, we just "move" the boot
CPU around when that happens. Any reason we can't do that generically ?
Cheers,
Ben.
> If the dying cpu is the boot cpu, it will return -EBUSY. In the subsequent error handling,
> cpu_notify_nofail(CPU_DOWN_FAILED) in _cpu_down() will be called. Unfortunately, some
> cpu notifier callbacks handled CPU_DOWN_PREPARE, but not CPU_DOWN_FAILED, such as sched_cpu_inactive().
> So it will cause issues.
>
> If we set the hotpluggable for the boot cpu, we can prevent user applications from disabling the boot cpu.
>
> -Chenhui
>
> > > Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> > > ---
> > > arch/powerpc/kernel/sysfs.c | 4 +++-
> > > 1 files changed, 3 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> > > index e68a845..294b1c4e 100644
> > > --- a/arch/powerpc/kernel/sysfs.c
> > > +++ b/arch/powerpc/kernel/sysfs.c
> > > @@ -655,8 +655,10 @@ static int __init topology_init(void)
> > > * CPU. For instance, the boot cpu might never be valid
> > > * for hotplugging.
> > > */
> > > - if (ppc_md.cpu_die)
> > > + if (ppc_md.cpu_die && cpu != boot_cpuid)
> > > c->hotpluggable = 1;
> > > + else
> > > + c->hotpluggable = 0;
> > >
> > > if (cpu_online(cpu) || c->hotpluggable) {
> > > register_cpu(c, cpu);
> >
> >
> >
^ permalink raw reply
* Re: [PATCH 15/27] powerpc/eeh: I/O chip EEH state retrieval
From: Gavin Shan @ 2013-06-12 3:32 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Gavin Shan
In-Reply-To: <1370936224.8250.93.camel@pasglop>
On Tue, Jun 11, 2013 at 05:37:04PM +1000, Benjamin Herrenschmidt wrote:
>On Wed, 2013-06-05 at 15:34 +0800, Gavin Shan wrote:
>> The patch adds I/O chip backend to retrieve the state for the
>> indicated PE. While the PE state is temperarily unavailable,
>> we return the default wait time (1000ms).
>>
>> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/platforms/powernv/eeh-ioda.c | 102 ++++++++++++++++++++++++++++-
>> 1 files changed, 101 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>> index e24622e..3c72321 100644
>> --- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>> @@ -125,10 +125,110 @@ static int ioda_eeh_set_option(struct eeh_pe *pe, int option)
>> return ret;
>> }
>>
>> +/**
>> + * ioda_eeh_get_state - Retrieve the state of PE
>> + * @pe: EEH PE
>> + * @state: return value
>> + *
>> + * The PE's state should be retrieved from the PEEV, PEST
>> + * IODA tables. Since the OPAL has exported the function
>> + * to do it, it'd better to use that.
>> + */
>> +static int ioda_eeh_get_state(struct eeh_pe *pe, int *state)
>> +{
>
>So everywhere you have this "state" argument which isn't a state but a delay ...
>
>Moreover you only initialize it in one specific case and leave it otherwise
>uninitialized....
>
>At the very least, init it to 0 by default as to not leave a dangling
>"return argument" like that. However, I still have a problem with it:
>
Ok. I will update accordingly in upper layer (eeh-powernv.c)
- Initialize it to value "0".
- If necessary, return 1 second.
>> + case OPAL_EEH_STOPPED_TEMP_UNAVAIL:
>> + result |= EEH_STATE_UNAVAILABLE;
>> + if (state)
>> + *state = 1000;
>> + break;
>
>This is the *only* case where we return anything here. Why do we bother
>then and not have the upper layer simply wait one second whenever it gets
>a temp unavailable result (btw, you didn't differenciate temp unavailable
>from permanently unavailable in your API).
>
We already defferentiated the permanent/temp availibility through the
return value from the function:
- EEH_STATE_UNAVAILABLE: temporary unavailibility
- EEH_STATE_NOT_SUPPORT: permanent unavailibility
The EEH core will handle the return value (from the function) accordingly.
>This has impacts on patch 18/27 which I'll cover here:
>
>> +/**
>> + * powernv_eeh_set_option - Initialize EEH or MMIO/DMA reenable
>> + * @pe: EEH PE
>> + * @option: operation to be issued
>> + *
>> + * The function is used to control the EEH functionality globally.
>> + * Currently, following options are support according to PAPR:
>> + * Enable EEH, Disable EEH, Enable MMIO and Enable DMA
>> + */
>> +static int powernv_eeh_set_option(struct eeh_pe *pe, int option)
>> +{
>> + struct pci_controller *hose = pe->phb;
>> + struct pnv_phb *phb = hose->private_data;
>> + int ret = -EEXIST;
>> +
>> + /*
>> + * What we need do is pass it down for hardware
>> + * implementation to handle it.
>> + */
>> + if (phb->eeh_ops && phb->eeh_ops->set_option)
>> + ret = phb->eeh_ops->set_option(pe, option);
>> +
>> + return ret;
>> +}
>
>Should we implement something here ? IE. Should we look into
>disabling freezing in the PHB via the firmware ? Or we just don't care ?
>
We just don't care. If EEH functionality has been disabled, we shouldn't
run into the code.
>> +/**
>> + * powernv_eeh_get_pe_addr - Retrieve PE address
>> + * @pe: EEH PE
>> + *
>> + * Retrieve the PE address according to the given tranditional
>> + * PCI BDF (Bus/Device/Function) address.
>> + */
>> +static int powernv_eeh_get_pe_addr(struct eeh_pe *pe)
>> +{
>> + return pe->addr;
>> +}
>>
>> +/**
>> + * powernv_eeh_get_state - Retrieve PE state
>> + * @pe: EEH PE
>> + * @state: return value
>> + *
>> + * Retrieve the state of the specified PE. For IODA-compitable
>> + * platform, it should be retrieved from IODA table. Therefore,
>> + * we prefer passing down to hardware implementation to handle
>> + * it.
>> + */
>> +static int powernv_eeh_get_state(struct eeh_pe *pe, int *state)
>> +{
>> + struct pci_controller *hose = pe->phb;
>> + struct pnv_phb *phb = hose->private_data;
>> + int ret = EEH_STATE_NOT_SUPPORT;
>> +
>> + if (phb->eeh_ops && phb->eeh_ops->get_state)
>> + ret = phb->eeh_ops->get_state(pe, state);
>> +
>> + return ret;
>> +}
>
>Same comments about "state" which is really "delay" and is probably
>not necessary at all ...
>
We need the "delay" in future to support PowerKVM guest. If the
specified PE is being reset, we rely on the delay to hold the
powerkvm guest for a while until the PE reset is done.
>> +/**
>> + * powernv_eeh_reset - Reset the specified PE
>> + * @pe: EEH PE
>> + * @option: reset option
>> + *
>> + * Reset the specified PE
>> + */
>> +static int powernv_eeh_reset(struct eeh_pe *pe, int option)
>> +{
>> + struct pci_controller *hose = pe->phb;
>> + struct pnv_phb *phb = hose->private_data;
>> + int ret = -EEXIST;
>> +
>> + if (phb->eeh_ops && phb->eeh_ops->reset)
>> + ret = phb->eeh_ops->reset(pe, option);
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * powernv_eeh_wait_state - Wait for PE state
>> + * @pe: EEH PE
>> + * @max_wait: maximal period in microsecond
>> + *
>> + * Wait for the state of associated PE. It might take some time
>> + * to retrieve the PE's state.
>> + */
>> +static int powernv_eeh_wait_state(struct eeh_pe *pe, int max_wait)
>> +{
>> + int ret;
>> + int mwait;
>> +
>> + while (1) {
>> + ret = powernv_eeh_get_state(pe, &mwait);
>> +
>> + /*
>> + * If the PE's state is temporarily unavailable,
>> + * we have to wait for the specified time. Otherwise,
>> + * the PE's state will be returned immediately.
>> + */
>> + if (ret != EEH_STATE_UNAVAILABLE)
>> + return ret;
>
>So here we do a compare, while ret is actually a bit mask ...
>
>In fact, ret should be named state_mask or something like that for clarity
>and you should do a bit test here. Also do you want to diffenciate
>permanent unavailability from temp. unavailability ?
>
>> + max_wait -= mwait;
>
>You decrement max_wait but never test it or use it. You probably mean to
>
> - Limit mwait to max_wait
> - If mwait is 0, return
>
Yeah, I will change the code accordingly in next revision.
>> + msleep(mwait);
>> + }
>> +
>> + return EEH_STATE_NOT_SUPPORT;
>> +}
>> +
>> +/**
>> + * powernv_eeh_get_log - Retrieve error log
>> + * @pe: EEH PE
>> + * @severity: temporary or permanent error log
>> + * @drv_log: driver log to be combined with retrieved error log
>> + * @len: length of driver log
>> + *
>> + * Retrieve the temporary or permanent error from the PE.
>> + */
>> +static int powernv_eeh_get_log(struct eeh_pe *pe, int severity,
>> + char *drv_log, unsigned long len)
>> +{
>> + struct pci_controller *hose = pe->phb;
>> + struct pnv_phb *phb = hose->private_data;
>> + int ret = -EEXIST;
>> +
>> + if (phb->eeh_ops && phb->eeh_ops->get_log)
>> + ret = phb->eeh_ops->get_log(pe, severity, drv_log, len);
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * powernv_eeh_configure_bridge - Configure PCI bridges in the indicated PE
>> + * @pe: EEH PE
>> + *
>> + * The function will be called to reconfigure the bridges included
>> + * in the specified PE so that the mulfunctional PE would be recovered
>> + * again.
>> + */
>> +static int powernv_eeh_configure_bridge(struct eeh_pe *pe)
>> +{
>> + struct pci_controller *hose = pe->phb;
>> + struct pnv_phb *phb = hose->private_data;
>> + int ret = 0;
>> +
>> + if (phb->eeh_ops && phb->eeh_ops->configure_bridge)
>> + ret = phb->eeh_ops->configure_bridge(pe);
>> +
>> + return ret;
>> +}
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH 17/27] powerpc/eeh: I/O chip PE log and bridge setup
From: Gavin Shan @ 2013-06-12 3:33 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Gavin Shan
In-Reply-To: <1370936279.8250.94.camel@pasglop>
On Tue, Jun 11, 2013 at 05:37:59PM +1000, Benjamin Herrenschmidt wrote:
>On Wed, 2013-06-05 at 15:34 +0800, Gavin Shan wrote:
>> The patch adds backends to retrieve error log and configure p2p
>> bridges for the indicated PE.
>>
>> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>> ---
>
>> +/**
>> + * ioda_eeh_configure_bridge - Configure the PCI bridges for the indicated PE
>> + * @pe: EEH PE
>> + *
>> + * For particular PE, it might have included PCI bridges. In order
>> + * to make the PE work properly, those PCI bridges should be configured
>> + * correctly. However, we need do nothing on P7IOC since the reset
>> + * function will do everything that should be covered by the function.
>> + */
>> +static int ioda_eeh_configure_bridge(struct eeh_pe *pe)
>> +{
>> + return 0;
>
>Does it now ?
>
>IE. Who reconfigures the windows and other config space bits of P2P
>bridges ? Or is this handled elsewhere in Linux or in the upper levels
>of EEH ? Or is that only needed for the PHB ?
>
The EEH core already coverred it, so we needn't do anything here.
If we're going to reset the PHB, firmware will re-initialize the
PHB and the left will be coverred by EEH core.
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH 15/27] powerpc/eeh: I/O chip EEH state retrieval
From: Benjamin Herrenschmidt @ 2013-06-12 4:19 UTC (permalink / raw)
To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <20130612033203.GA10000@shangw.(null)>
On Wed, 2013-06-12 at 11:32 +0800, Gavin Shan wrote:
> >Same comments about "state" which is really "delay" and is probably
> >not necessary at all ...
> >
>
> We need the "delay" in future to support PowerKVM guest. If the
> specified PE is being reset, we rely on the delay to hold the
> powerkvm guest for a while until the PE reset is done.
Do we ? Can't we just rely on "temp unavailble" result and wait 1s when
that happens (then try again) ?
IE, A delay associated with a state doesn't make that much sense
semantically speaking. With a state *transition* maybe but this isn't
what this function is about...
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 1/5] ibmebus: convert of_platform_driver to platform_driver
From: Benjamin Herrenschmidt @ 2013-06-12 5:29 UTC (permalink / raw)
To: Rob Herring
Cc: Roland Dreier, Arnd Bergmann, linux-rdma, netdev,
devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org,
Christoph Raisch, Hoang-Nam Nguyen, Thadeu Lima de Souza Cascardo,
Grant Likely, Paul Mackerras, Sean Hefty, linuxppc-dev,
Hal Rosenstock
In-Reply-To: <CAL_JsqKXt-gFL4y_k-_dFktpk1JD_S2G+hUsoOavw8QugFMOKA@mail.gmail.com>
On Wed, 2013-05-22 at 07:26 -0500, Rob Herring wrote:
> > git://sources.calxeda.com/kernel/linux.git of-platform-removal
>
> Ben,
>
> Did you have a chance to test this? I want to get this into -next.
I tested the one in for-next (sorry for the high latency). Works fine.
Ack.
Cheers,
Ben.
^ permalink raw reply
* [PATCH] of: Fix locking vs. interrupts
From: Benjamin Herrenschmidt @ 2013-06-12 5:39 UTC (permalink / raw)
To: Thomas Gleixner
Cc: devicetree-discuss, linuxppc-dev, David Miller, Linux Kernel list
The OF code uses irqsafe locks everywhere except in a handful of functions
for no obvious reasons. Since the conversion from the old rwlocks, this
now triggers lockdep warnings when used at interrupt time. At least one
driver (ibmvscsi) seems to be doing that from softirq context.
This converts the few non-irqsafe locks into irqsafe ones, making them
consistent with the rest of the code.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <stable@vger.kernel.org> [v3.9+]
---
Note: It's silly to access the device-tree at interrupt time in most cases,
and we should probably fix ibmvscsi, but for the time being, let's fix the
obvious bug. Thomas, this can probably still go into 3.10... If not, I've
CCed stable.
diff --git a/arch/sparc/kernel/prom_common.c b/arch/sparc/kernel/prom_common.c
index 9f20566..79cc0d1 100644
--- a/arch/sparc/kernel/prom_common.c
+++ b/arch/sparc/kernel/prom_common.c
@@ -54,6 +54,7 @@ EXPORT_SYMBOL(of_set_property_mutex);
int of_set_property(struct device_node *dp, const char *name, void *val, int len)
{
struct property **prevp;
+ unsigned long flags;
void *new_val;
int err;
@@ -64,7 +65,7 @@ int of_set_property(struct device_node *dp, const char *name, void *val, int len
err = -ENODEV;
mutex_lock(&of_set_property_mutex);
- raw_spin_lock(&devtree_lock);
+ raw_spin_lock_irqsave(&devtree_lock, flags);
prevp = &dp->properties;
while (*prevp) {
struct property *prop = *prevp;
@@ -91,7 +92,7 @@ int of_set_property(struct device_node *dp, const char *name, void *val, int len
}
prevp = &(*prevp)->next;
}
- raw_spin_unlock(&devtree_lock);
+ raw_spin_unlock_irqrestore(&devtree_lock, flags);
mutex_unlock(&of_set_property_mutex);
/* XXX Upate procfs if necessary... */
diff --git a/drivers/of/base.c b/drivers/of/base.c
index f53b992..951452c 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -192,14 +192,15 @@ EXPORT_SYMBOL(of_find_property);
struct device_node *of_find_all_nodes(struct device_node *prev)
{
struct device_node *np;
+ unsigned long flags;
- raw_spin_lock(&devtree_lock);
+ raw_spin_lock_irqsave(&devtree_lock, flags);
np = prev ? prev->allnext : of_allnodes;
for (; np != NULL; np = np->allnext)
if (of_node_get(np))
break;
of_node_put(prev);
- raw_spin_unlock(&devtree_lock);
+ raw_spin_unlock_irqrestore(&devtree_lock, flags);
return np;
}
EXPORT_SYMBOL(of_find_all_nodes);
@@ -421,8 +422,9 @@ struct device_node *of_get_next_available_child(const struct device_node *node,
struct device_node *prev)
{
struct device_node *next;
+ unsigned long flags;
- raw_spin_lock(&devtree_lock);
+ raw_spin_lock_irqsave(&devtree_lock, flags);
next = prev ? prev->sibling : node->child;
for (; next; next = next->sibling) {
if (!__of_device_is_available(next))
@@ -431,7 +433,7 @@ struct device_node *of_get_next_available_child(const struct device_node *node,
break;
}
of_node_put(prev);
- raw_spin_unlock(&devtree_lock);
+ raw_spin_unlock_irqrestore(&devtree_lock, flags);
return next;
}
EXPORT_SYMBOL(of_get_next_available_child);
@@ -735,13 +737,14 @@ EXPORT_SYMBOL_GPL(of_modalias_node);
struct device_node *of_find_node_by_phandle(phandle handle)
{
struct device_node *np;
+ unsigned long flags;
- raw_spin_lock(&devtree_lock);
+ raw_spin_lock_irqsave(&devtree_lock, flags);
for (np = of_allnodes; np; np = np->allnext)
if (np->phandle == handle)
break;
of_node_get(np);
- raw_spin_unlock(&devtree_lock);
+ raw_spin_unlock_irqrestore(&devtree_lock, flags);
return np;
}
EXPORT_SYMBOL(of_find_node_by_phandle);
^ permalink raw reply related
* Re: [PATCH -V7 09/18] powerpc: Switch 16GB and 16MB explicit hugepages to a different page table format
From: Aneesh Kumar K.V @ 2013-06-12 6:30 UTC (permalink / raw)
To: Scott Wood, Scott Wood; +Cc: linux-mm, paulus, linuxppc-dev, dwg
In-Reply-To: <1370991027.18413.33@snotra>
Scott Wood <scottwood@freescale.com> writes:
> On 06/11/2013 03:53:43 PM, Scott Wood wrote:
>> On 06/08/2013 11:57:48 AM, Aneesh Kumar K.V wrote:
>>> With the config shared I am not finding anything wrong, but I can't
>>> test
>>> these configs. Also can you confirm what you bisect this to
>>>
>>> e2b3d202d1dba8f3546ed28224ce485bc50010be
>>> powerpc: Switch 16GB and 16MB explicit hugepages to a different page
>>> table format
>>
>>>
>>> or
>>>
>>> cf9427b85e90bb1ff90e2397ff419691d983c68b "powerpc: New hugepage
>>> directory format"
>>
>> It's e2b3d202d1dba8f3546ed28224ce485bc50010be.
>>
>> It turned out to be the change from "pmd_none" to
>> "pmd_none_or_clear_bad". Making that change triggers the "bad pmd"
>> messages even when applied to v3.9 -- so we had bad pmds all along,
>> undetected. Now I get to figure out why. :-(
>
> So, for both pud and pgd we only call "or_clear_bad" when is_hugepd
> returns false. Why is it OK to do it unconditionally for pmd?
>
Ok, that could be the issue. Now the reason why we want to call
pmd_clear is to take care of explicit hugepage pte saved in the
pmd slot. We should already find the slot cleared otherwise it
is a corruption. How about the below ? The current code is broken
in that we will never take that free_hugepd_range call at all.
commit a09f59fe477242a3ebd153e618a705ac8f6c1b89
Author: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Date: Wed Jun 12 11:32:58 2013 +0530
powerpc: Fix bad pmd error with FSL config
FSL uses the hugepd at PMD level and don't encode pte directly
at the pmd level. So it will find the lower bits of pmd set
and the pmd_bad check throws error. Infact the current code
will never take the free_hugepd_range call at all because it will
clear the pmd if it find a hugepd pointer.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index f2f01fd..315fbd4 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -536,19 +536,28 @@ static void hugetlb_free_pmd_range(struct mmu_gather *tlb, pud_t *pud,
do {
pmd = pmd_offset(pud, addr);
next = pmd_addr_end(addr, end);
- if (pmd_none_or_clear_bad(pmd))
- continue;
+ if (!is_hugepd(pmd)) {
+ /*
+ * if it is not hugepd pointer, we should already find
+ * it cleared.
+ */
+ if (!pmd_none_or_clear_bad(pmd))
+ WARN_ON(1);
+ } else {
+ if (pmd_none(*pmd))
+ continue;
#ifdef CONFIG_PPC_FSL_BOOK3E
- /*
- * Increment next by the size of the huge mapping since
- * there may be more than one entry at this level for a
- * single hugepage, but all of them point to
- * the same kmem cache that holds the hugepte.
- */
- next = addr + (1 << hugepd_shift(*(hugepd_t *)pmd));
+ /*
+ * Increment next by the size of the huge mapping since
+ * there may be more than one entry at this level for a
+ * single hugepage, but all of them point to
+ * the same kmem cache that holds the hugepte.
+ */
+ next = addr + (1 << hugepd_shift(*(hugepd_t *)pmd));
#endif
- free_hugepd_range(tlb, (hugepd_t *)pmd, PMD_SHIFT,
- addr, next, floor, ceiling);
+ free_hugepd_range(tlb, (hugepd_t *)pmd, PMD_SHIFT,
+ addr, next, floor, ceiling);
+ }
} while (addr = next, addr != end);
start &= PUD_MASK;
^ permalink raw reply related
* [PATCH] serial/mpc52xx_uart: fix kernel panic when system reboot
From: Matteo Facchinetti @ 2013-06-12 7:21 UTC (permalink / raw)
To: linux-serial, gregkh; +Cc: agust, linuxppc-dev
This bug appear when a second PSC based driver appends an interrupt
routine to the FIFO controller shared interrupt (like spi-mpc512x-psc).
When reboot, uart_shutdown() remove the serial console interrupt handler
while spi-mpc512x-psc isr is still activate and cause the following kernel
panic:
The system is going down for reboot NOW!rpc (ttyPSC0) (Mon Jun 10 12:26:07 20
INIT: Sending processirq 40: nobody cared (try booting with the "irqpoll" option)
CPU: 0 PID: 0 Comm: swapper Not tainted 3.10.0-rc4-next-20130607-00001-ga0bceb3-dirty #5
Call Trace:
[cfff9f00] [c0007910] show_stack+0x48/0x150 (unreliable)
[cfff9f40] [c005ae60] __report_bad_irq.isra.6+0x34/0xe0
[cfff9f60] [c005b194] note_interrupt+0x214/0x26c
[cfff9f90] [c00590fc] handle_irq_event_percpu+0xd0/0x1bc
[cfff9fd0] [c005921c] handle_irq_event+0x34/0x54
[cfff9fe0] [c005b8f4] handle_level_irq+0x90/0xf4
[cfff9ff0] [c000cb98] call_handle_irq+0x18/0x28
[c050dd60] [c000575c] do_IRQ+0xcc/0x124
[c050dd90] [c000eb04] ret_from_except+0x0/0x14
--- Exception: 501 at __do_softirq+0x84/0x17c
LR = __do_softirq+0x20/0x17c
[c050de50] [c0045e7c] ktime_get+0x148/0x160 (unreliable)
[c050dea0] [c001eccc] irq_exit+0x94/0xb8
[c050deb0] [c00056e0] do_IRQ+0x50/0x124
[c050dee0] [c000eb04] ret_from_except+0x0/0x14
--- Exception: 501 at cpu_startup_entry+0x5c/0x134
LR = cpu_startup_entry+0xc0/0x134
[c050dfa0] [c0045170] cpu_startup_entry+0xac/0x134 (unreliable)
[c050dfc0] [c04b9d70] start_kernel+0x2d4/0x2e8
[c050dff0] [00003438] 0x3438
handlers:
[<c02ae388>] mpc512x_psc_spi_isr
Disabling IRQ #40
Disabling serial console interrupt before free_irq()
prevents this error.
Signed-off-by: Matteo Facchinetti <matteo.facchinetti@sirius-es.it>
---
drivers/tty/serial/mpc52xx_uart.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/tty/serial/mpc52xx_uart.c b/drivers/tty/serial/mpc52xx_uart.c
index 9ba1945..e1280a2 100644
--- a/drivers/tty/serial/mpc52xx_uart.c
+++ b/drivers/tty/serial/mpc52xx_uart.c
@@ -1058,6 +1058,9 @@ mpc52xx_uart_shutdown(struct uart_port *port)
if (psc_ops->clock)
psc_ops->clock(port, 0);
+ /* Disable interrupt */
+ psc_ops->cw_disable_ints(port);
+
/* Release interrupt */
free_irq(port->irq, port);
}
--
1.8.1.2
^ permalink raw reply related
* Re: [BUG] PCI related panic on powerpc based board with 3.10-rcX
From: Rojhalat Ibrahim @ 2013-06-12 8:19 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, linux-kernel, Michael Guntsche
In-Reply-To: <1370971739.18413.27@snotra>
On Tuesday 11 June 2013 12:28:59 Scott Wood wrote:
> On 06/11/2013 12:09:42 PM, Michael Guntsche wrote:
> > On Tue, Jun 11, 2013 at 7:00 PM, Scott Wood <scottwood@freescale.com>
> >
> > wrote:
> > > On 06/11/2013 02:24:28 AM, Rojhalat Ibrahim wrote:
> > >> On Monday 10 June 2013 17:52:33 Scott Wood wrote:
> > >> > On 06/10/2013 12:07:43 PM, Michael Guntsche wrote:
> > >> > > Good evening,
> > >> > >
> > >> > > This patch does not fix the problem, during boot the kernel
> >
> > still
> >
> > >> > > panics. I had a closer look at the commit and the following
> >
> > patch
> >
> > >> > > fixes it for me....
> > >> > >
> > >> > > diff --git a/arch/powerpc/sysdev/fsl_pci.c
> > >> > > b/arch/powerpc/sysdev/fsl_pci.c
> > >> > > index 028ac1f..21b687f 100644
> > >> > > --- a/arch/powerpc/sysdev/fsl_pci.c
> > >> > > +++ b/arch/powerpc/sysdev/fsl_pci.c
> > >> > > @@ -814,7 +814,7 @@ int __init mpc83xx_add_bridge(struct
> >
> > device_node
> >
> > >> > > *dev)
> > >> > >
> > >> > > if (ret)
> > >> > >
> > >> > > goto err0;
> > >> > >
> > >> > > } else {
> > >> > >
> > >> > > - fsl_setup_indirect_pci(hose, rsrc_cfg.start,
> > >> > > + setup_indirect_pci(hose, rsrc_cfg.start,
> > >> > >
> > >> > > rsrc_cfg.start + 4, 0);
> > >> > >
> > >> > > }
> > >> >
> > >> > The only difference here is that you're not setting hose->ops to
> > >> > fsl_indirect_pci_ops. Do you know why that is helping, and what
> > >> > hose->ops is set to instead?
> > >> >
> > >> > -Scott
> > >>
> > >> The difference is only the read function in hose->ops, which is
> >
> > set to
> >
> > >> indirect_read_config instead of fsl_indirect_read_config.
> > >>
> > >> fsl_indirect_read_config calls fsl_pcie_check_link, which is where
> >
> > the
> >
> > >> Oops
> > >> occurs.
> > >
> > > Why is fsl_pcie_check_link being called for non-PCIe buses?
> > >
> > >> Mike, can you find out where exactly in fsl_pcie_check_link the
> >
> > bad access
> >
> > >> happens? Enabling CONFIG_DEBUG_BUGVERBOSE might help.
> > >
> > > Why does it matter? You shouldn't be calling that function at all.
> > >
> > > -Scott
> >
> > For the record BUGVERBOSE is already set with this build so this is
> > the most detailed trace I get. And regarding Scott's remark, maybe I
> > was not clear enough in my first report. This is a PCI only board so I
> > also wondered about the call to fsl_pcie_check_link in the first
> > place.
>
> Yes, I figured it was non-PCIe because the code change that you said
> helped was on the non-PCIe branch of the if/else. Generally it's good
> to explicitly mention the chip you're using, though.
>
> fsl_setup_indirect_pci should be renamed to fsl_setup_indirect_pcie.
> Your patch above should be applied, and fsl_setup_indirect_pcie should
> be moved into the booke/86xx ifdef to avoid an unused function warning.
>
> -Scott
How about this patch? It uses setup_indirect_pci for the PCI case in
mpc83xx_add_bridge. Additionally it adds a check in fsl_setup_indirect_pci
to only use the modified read function in case of PCIe.
Rojhalat
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 028ac1f..45670df 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -97,22 +97,23 @@ static int fsl_indirect_read_config(struct pci_bus *bus, unsigned int devfn,
return indirect_read_config(bus, devfn, offset, len, val);
}
-static struct pci_ops fsl_indirect_pci_ops =
+static struct pci_ops fsl_indirect_pcie_ops =
{
.read = fsl_indirect_read_config,
.write = indirect_write_config,
};
+#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
+
static void __init fsl_setup_indirect_pci(struct pci_controller* hose,
resource_size_t cfg_addr,
resource_size_t cfg_data, u32 flags)
{
setup_indirect_pci(hose, cfg_addr, cfg_data, flags);
- hose->ops = &fsl_indirect_pci_ops;
+ if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) /* PCIe */
+ hose->ops = &fsl_indirect_pcie_ops;
}
-#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
-
#define MAX_PHYS_ADDR_BITS 40
static u64 pci64_dma_offset = 1ull << MAX_PHYS_ADDR_BITS;
@@ -814,8 +815,8 @@ int __init mpc83xx_add_bridge(struct device_node *dev)
if (ret)
goto err0;
} else {
- fsl_setup_indirect_pci(hose, rsrc_cfg.start,
- rsrc_cfg.start + 4, 0);
+ setup_indirect_pci(hose, rsrc_cfg.start,
+ rsrc_cfg.start + 4, 0);
}
printk(KERN_INFO "Found FSL PCI host bridge at 0x%016llx. "
^ permalink raw reply related
* [PATCH 07/11] cpufreq: powerpc: CBE_RAS: select CPU_FREQ_TABLE
From: Viresh Kumar @ 2013-06-12 8:15 UTC (permalink / raw)
To: rjw
Cc: robin.randhawa, arnd, linux-pm, Viresh Kumar, patches,
Liviu.Dudau, linux-kernel, cpufreq, linaro-kernel,
Steve.Bannister, arvind.chauhan, linuxppc-dev,
charles.garcia-tobin
In-Reply-To: <cover.1371022830.git.viresh.kumar@linaro.org>
CPUFreq driver of this platform uses APIs from freq_table.c and so must select
CPU_FREQ_TABLE.
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/Kconfig.powerpc | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/cpufreq/Kconfig.powerpc b/drivers/cpufreq/Kconfig.powerpc
index 9c926ca..68c1abc 100644
--- a/drivers/cpufreq/Kconfig.powerpc
+++ b/drivers/cpufreq/Kconfig.powerpc
@@ -1,6 +1,7 @@
config CPU_FREQ_CBE
tristate "CBE frequency scaling"
depends on CBE_RAS && PPC_CELL
+ select CPU_FREQ_TABLE
default m
help
This adds the cpufreq driver for Cell BE processors.
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related
* Re: [PATCH] of: Fix locking vs. interrupts
From: Thomas Gleixner @ 2013-06-12 8:25 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: devicetree-discuss, linuxppc-dev, David Miller, Linux Kernel list
In-Reply-To: <1371015544.8250.182.camel@pasglop>
On Wed, 12 Jun 2013, Benjamin Herrenschmidt wrote:
> The OF code uses irqsafe locks everywhere except in a handful of functions
> for no obvious reasons. Since the conversion from the old rwlocks, this
> now triggers lockdep warnings when used at interrupt time. At least one
> driver (ibmvscsi) seems to be doing that from softirq context.
>
> This converts the few non-irqsafe locks into irqsafe ones, making them
> consistent with the rest of the code.
Fun. https://lkml.org/lkml/2013/2/4/416 seems to have got lost
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: <stable@vger.kernel.org> [v3.9+]
Acked-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>
> Note: It's silly to access the device-tree at interrupt time in most cases,
> and we should probably fix ibmvscsi, but for the time being, let's fix the
Right.
> obvious bug. Thomas, this can probably still go into 3.10... If not, I've
> CCed stable.
Should go through Grant I think.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH] of: Fix locking vs. interrupts
From: Benjamin Herrenschmidt @ 2013-06-12 8:56 UTC (permalink / raw)
To: Thomas Gleixner
Cc: devicetree-discuss, linuxppc-dev, David Miller, Linux Kernel list
In-Reply-To: <alpine.LFD.2.02.1306121023150.22970@ionos>
On Wed, 2013-06-12 at 10:25 +0200, Thomas Gleixner wrote:
> On Wed, 12 Jun 2013, Benjamin Herrenschmidt wrote:
>
> > The OF code uses irqsafe locks everywhere except in a handful of functions
> > for no obvious reasons. Since the conversion from the old rwlocks, this
> > now triggers lockdep warnings when used at interrupt time. At least one
> > driver (ibmvscsi) seems to be doing that from softirq context.
> >
> > This converts the few non-irqsafe locks into irqsafe ones, making them
> > consistent with the rest of the code.
>
> Fun. https://lkml.org/lkml/2013/2/4/416 seems to have got lost
>
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > CC: <stable@vger.kernel.org> [v3.9+]
>
> Acked-by: Thomas Gleixner <tglx@linutronix.de>
>
> > ---
> >
> > Note: It's silly to access the device-tree at interrupt time in most cases,
> > and we should probably fix ibmvscsi, but for the time being, let's fix the
>
> Right.
>
> > obvious bug. Thomas, this can probably still go into 3.10... If not, I've
> > CCed stable.
>
> Should go through Grant I think.
Right, thinko. Sent to you due to the bug being exposed by your
conversion to spinlocks. Anyway, Grant got it now.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] of: Fix locking vs. interrupts
From: Grant Likely @ 2013-06-12 9:06 UTC (permalink / raw)
To: Thomas Gleixner, Benjamin Herrenschmidt
Cc: devicetree-discuss, linuxppc-dev, David Miller, Linux Kernel list
In-Reply-To: <alpine.LFD.2.02.1306121023150.22970@ionos>
On Wed, 12 Jun 2013 10:25:56 +0200 (CEST), Thomas Gleixner <tglx@linutronix.de> wrote:
> On Wed, 12 Jun 2013, Benjamin Herrenschmidt wrote:
>
> > The OF code uses irqsafe locks everywhere except in a handful of functions
> > for no obvious reasons. Since the conversion from the old rwlocks, this
> > now triggers lockdep warnings when used at interrupt time. At least one
> > driver (ibmvscsi) seems to be doing that from softirq context.
> >
> > This converts the few non-irqsafe locks into irqsafe ones, making them
> > consistent with the rest of the code.
>
> Fun. https://lkml.org/lkml/2013/2/4/416 seems to have got lost
>
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > CC: <stable@vger.kernel.org> [v3.9+]
>
> Acked-by: Thomas Gleixner <tglx@linutronix.de>
>
> > ---
> >
> > Note: It's silly to access the device-tree at interrupt time in most cases,
> > and we should probably fix ibmvscsi, but for the time being, let's fix the
>
> Right.
>
> > obvious bug. Thomas, this can probably still go into 3.10... If not, I've
> > CCed stable.
>
> Should go through Grant I think.
Applied, thanks.
g.
^ permalink raw reply
* Re: [PATCH] of: Fix locking vs. interrupts
From: David Miller @ 2013-06-12 9:15 UTC (permalink / raw)
To: grant.likely; +Cc: devicetree-discuss, tglx, linuxppc-dev, linux-kernel
In-Reply-To: <20130612090612.7E4ED3E0A56@localhost>
From: Grant Likely <grant.likely@secretlab.ca>
Date: Wed, 12 Jun 2013 10:06:12 +0100
> On Wed, 12 Jun 2013 10:25:56 +0200 (CEST), Thomas Gleixner <tglx@linutronix.de> wrote:
>> On Wed, 12 Jun 2013, Benjamin Herrenschmidt wrote:
>>
>> > The OF code uses irqsafe locks everywhere except in a handful of functions
>> > for no obvious reasons. Since the conversion from the old rwlocks, this
>> > now triggers lockdep warnings when used at interrupt time. At least one
>> > driver (ibmvscsi) seems to be doing that from softirq context.
>> >
>> > This converts the few non-irqsafe locks into irqsafe ones, making them
>> > consistent with the rest of the code.
>>
>> Fun. https://lkml.org/lkml/2013/2/4/416 seems to have got lost
>>
>> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> > CC: <stable@vger.kernel.org> [v3.9+]
>>
>> Acked-by: Thomas Gleixner <tglx@linutronix.de>
>>
>> > ---
>> >
>> > Note: It's silly to access the device-tree at interrupt time in most cases,
>> > and we should probably fix ibmvscsi, but for the time being, let's fix the
>>
>> Right.
>>
>> > obvious bug. Thomas, this can probably still go into 3.10... If not, I've
>> > CCed stable.
>>
>> Should go through Grant I think.
>
> Applied, thanks.
FWIW:
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* help with PCI RTL8139 on mpc5200
From: scinteeb @ 2013-06-12 9:52 UTC (permalink / raw)
To: linuxppc-dev
Hello,
First of all I want to mention that I am quite new on the development at
this level on the mpc5200 based boards. It might be that I'll ask dumb or
wrong questions so I want to apologize for this.
I am using a custom board based on icecube 5200 reference card but with an
additional PCI RTL8139 network interface.
1. I did compile the latest u-boot and at that level the PCI is detecting
the interface and the u-boot is adding it in the network list as I can see
as interfaces FEC and RTL8139#0.
2. I have compiled the latest released kernel 3.9.4 which is having support
for mpc5200 based devices. The kernel is able to identify the RTL8139
interface, is loading properly the 8139too driver, I can change the settings
of the interface, but when I am trying to setup the IP address I am getting
an SIOCSIFFLAGS error.
The investigations showed that the reason for the error is that the IRQ 0
that is detected by the system as being used by the interface cannot be
used. The request_irq function in the rtl8139 is failing because the
can_request_irq is returning 0.
The interesting boot messages that I found while booting the kernel and
might be helpful are:
[ 0.226240] mpc52xx_irqhost_map: Critical IRQ #3 is unsupported! Nopping
it.
...
[ 1.404745] 8139too 0000:01:0c.0 eth1: RealTek RTL8139 at 0xca086000,
00:e0:4c:00:00:08, IRQ 0
Any idea on how to address the situation?
Thanks in advance,
Bogdan.
--
View this message in context: http://linuxppc.10917.n7.nabble.com/help-with-PCI-RTL8139-on-mpc5200-tp72284.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.
^ permalink raw reply
* [PATCH] powerpc: Fix bad pmd error with book3E config
From: Aneesh Kumar K.V @ 2013-06-12 10:30 UTC (permalink / raw)
To: benh, scottwood; +Cc: linuxppc-dev, Aneesh Kumar K.V
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Book3E uses the hugepd at PMD level and don't encode pte directly
at the pmd level. So it will find the lower bits of pmd set
and the pmd_bad check throws error. Infact the current code
will never take the free_hugepd_range call at all because it will
clear the pmd if it find a hugepd pointer.
Reported-by: Scott Wood <scottwood@freescale.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/mm/hugetlbpage.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index f2f01fd..0d3d3ee 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -536,19 +536,26 @@ static void hugetlb_free_pmd_range(struct mmu_gather *tlb, pud_t *pud,
do {
pmd = pmd_offset(pud, addr);
next = pmd_addr_end(addr, end);
- if (pmd_none_or_clear_bad(pmd))
- continue;
+ if (!is_hugepd(pmd)) {
+ /*
+ * if it is not hugepd pointer, we should already find
+ * it cleared.
+ */
+ if (!pmd_none_or_clear_bad(pmd))
+ WARN_ON(1);
+ } else {
#ifdef CONFIG_PPC_FSL_BOOK3E
- /*
- * Increment next by the size of the huge mapping since
- * there may be more than one entry at this level for a
- * single hugepage, but all of them point to
- * the same kmem cache that holds the hugepte.
- */
- next = addr + (1 << hugepd_shift(*(hugepd_t *)pmd));
+ /*
+ * Increment next by the size of the huge mapping since
+ * there may be more than one entry at this level for a
+ * single hugepage, but all of them point to
+ * the same kmem cache that holds the hugepte.
+ */
+ next = addr + (1 << hugepd_shift(*(hugepd_t *)pmd));
#endif
- free_hugepd_range(tlb, (hugepd_t *)pmd, PMD_SHIFT,
- addr, next, floor, ceiling);
+ free_hugepd_range(tlb, (hugepd_t *)pmd, PMD_SHIFT,
+ addr, next, floor, ceiling);
+ }
} while (addr = next, addr != end);
start &= PUD_MASK;
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH 1/5] ibmebus: convert of_platform_driver to platform_driver
From: Grant Likely @ 2013-06-12 11:40 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Rob Herring
Cc: Roland Dreier, Arnd Bergmann, linux-rdma, netdev,
devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org,
Christoph Raisch, Hoang-Nam Nguyen, Thadeu Lima de Souza Cascardo,
Paul Mackerras, Sean Hefty, linuxppc-dev, Hal Rosenstock
In-Reply-To: <1371014978.8250.175.camel@pasglop>
On Wed, 12 Jun 2013 15:29:38 +1000, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Wed, 2013-05-22 at 07:26 -0500, Rob Herring wrote:
> > > git://sources.calxeda.com/kernel/linux.git of-platform-removal
> >
> > Ben,
> >
> > Did you have a chance to test this? I want to get this into -next.
>
> I tested the one in for-next (sorry for the high latency). Works fine.
>
> Ack.
Added acks and applied to my for-next branch. Thanks!
g.
^ permalink raw reply
* Re: [PATCH] MDIO: FSL_PQ_MDIO: Fix bug on incorrect offset of tbipa register
From: Timur Tabi @ 2013-06-12 13:04 UTC (permalink / raw)
To: Oded Gabbay, benh, paulus, leoli, galak, B38951, Dongsheng.Wang,
bigeasy, stef.van.os, davem
Cc: netdev, linuxppc-dev, linux-kernel
In-Reply-To: <1371041258-15298-1-git-send-email-ogabbay@advaoptical.com>
Oded Gabbay wrote:
> Note: This patch may break MDIO functionallity of some old Freescale's SoC
> until Freescale will fix their device tree files. Basically, every device tree
> which contains an mdio device that is compatible to "fsl,gianfar-tbi" should be
> examined.
I haven't had a chance to review the patch in detail, but I can tell you
that breaking compatibility with older device trees is unacceptable. You
need to add some code, even if it's an ugly hack, to support those trees.
--
Timur Tabi
^ permalink raw reply
* Re: [PATCH] MDIO: FSL_PQ_MDIO: Fix bug on incorrect offset of tbipa register
From: Oded Gabbay @ 2013-06-12 13:09 UTC (permalink / raw)
To: Timur Tabi
Cc: stef.van.os, bigeasy, Dongsheng.Wang, paulus, netdev,
linuxppc-dev, davem, linux-kernel, B38951
In-Reply-To: <51B871CB.1090808@tabi.org>
Oded Gabbay wrote:
>> Note: This patch may break MDIO functionallity of some old
>> Freescale's SoC
>> until Freescale will fix their device tree files. Basically, every
>> device tree
>> which contains an mdio device that is compatible to "fsl,gianfar-tbi"
>> should be
>> examined.
>
> On 06/12/2013 04:04 PM, Timur Tabi wrote:
> I haven't had a chance to review the patch in detail, but I can tell
> you that breaking compatibility with older device trees is
> unacceptable. You need to add some code, even if it's an ugly hack,
> to support those trees.
>
I generally agree with this statement except that without this patch,
almost ALL of Freescale's SoC that uses "fsl,gianfar-tbi" are broken,
including the older ones. At least this patch fixes some of the device
trees. Because I'm not working at Freescale, I have a very limited
access to a few SoC which I could test this patch on. I think it is
Freescale's responsibility to release a complementary patch to fix the
rest of the SoC device trees.
Oded
^ 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