* Re: [PATCH V7 04/17] PCI: Take additional IOV BAR alignment in sizing and assigning
From: Bjorn Helgaas @ 2014-09-09 20:09 UTC (permalink / raw)
To: Wei Yang
Cc: Benjamin Herrenschmidt, linux-pci@vger.kernel.org, Gavin Shan,
Mike Qiu, Guo Chao, linuxppc-dev
In-Reply-To: <20140820061402.GA8381@richard>
On Wed, Aug 20, 2014 at 12:14 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
> On Tue, Aug 19, 2014 at 09:08:41PM -0600, Bjorn Helgaas wrote:
>>On Thu, Jul 24, 2014 at 02:22:14PM +0800, Wei Yang wrote:
>>> At resource sizing/assigning stage, resources are divided into two lists,
>>> requested list and additional list, while the alignement of the additional
>>> IOV BAR is not taken into the sizeing and assigning procedure.
>>>
>>> This is reasonable in the original implementation, since IOV BAR's alignment is
>>> mostly the size of a PF BAR alignemt. This means the alignment is already taken
>>> into consideration. While this rule may be violated on some platform.
>>>
>>> This patch take the additional IOV BAR alignment in sizing and assigning stage
>>> explicitly.
>>>
>>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>> ---
>>> drivers/pci/setup-bus.c | 68 +++++++++++++++++++++++++++++++++++++++++------
>>> 1 file changed, 60 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>>> index a5a63ec..d83681f 100644
>>> --- a/drivers/pci/setup-bus.c
>>> +++ b/drivers/pci/setup-bus.c
>>> @@ -120,6 +120,28 @@ static resource_size_t get_res_add_size(struct list_head *head,
>>> return 0;
>>> }
>>>
>>> +static resource_size_t get_res_add_align(struct list_head *head,
>>> + struct resource *res)
>>> +{
>>> + struct pci_dev_resource *dev_res;
>>> +
>>> + list_for_each_entry(dev_res, head, list) {
>>> + if (dev_res->res == res) {
>>> + int idx = res - &dev_res->dev->resource[0];
>>> +
>>> + dev_printk(KERN_DEBUG, &dev_res->dev->dev,
>>> + "res[%d]=%pR get_res_add_align min_align %llx\n",
>>> + idx, dev_res->res,
>>> + (unsigned long long)dev_res->min_align);
>>> +
>>> + return dev_res->min_align;
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>
>>I see that you copied the structure of the existing get_res_add_size()
>>here. But I don't understand *that* function. It looks basically like
>>this:
>>
>> resource_size_t get_res_add_size(list, res)
>> {
>> list_for_each_entry(dev_res, head, list) {
>> if (dev_res->res == res)
>> return dev_res->add_size;
>> }
>> return 0;
>> }
>>
>>and we call it like this:
>>
>> dev_res->res->end += get_res_add_size(realloc_head, dev_res->res);
>>
>>So we start out with dev_res", pass in dev_res->res, search the
>>realloc_head list to find dev_res again, and return dev_res->add_size.
>>That looks equivalent to just:
>>
>> dev_res->res->end += dev_res->add_size;
>>
>>It looks like get_res_add_size() merely adds a printk and some complexity.
>>Am I missing something?
>>
>
> Let me try to explain it, if not correct, please let know :-)
>
> dev_res->res->end += get_res_add_size(realloc_head, dev_res->res);
>
> would be expanded to:
>
> dev_res->res->end += dev_res_1->add_size;
>
> with the dev_res_1 is another one from dev_res which is stored in realloc_head.
Yep, I see now.
>>I do see that there are other callers where we don't actually start with
>>dev_res, which makes it a little more complicated. But I think you should
>>either add something like this:
>>
>> struct pci_dev_resource *res_to_dev_res(list, res)
>> {
>> list_for_each_entry(dev_res, head, list) {
>> if (dev_res->res == res)
>> return dev_res;
>> }
>> return NULL;
>> }
>>
>
> Ok, we can extract the common part of these two functions.
>
>>which can be used to replace get_res_add_size() and get_res_add_align(), OR
>>figure out whether the dev_res of interest is always one we've just added.
>>If it is, maybe you can just make add_to_list() return the dev_res pointer
>>instead of an errno, and hang onto the pointer. I'd like that much better
>>if that's possible.
>>
>
> Sorry, I don't get this point.
Don't worry, it didn't make sense. I was thinking that we knew the
dev_res up front and didn't need to look it up, but that's not the
case.
Sorry it took me so long to respond to this; I'm a bit swamped dealing
with some regressions.
Bjorn
> add_to_list() is used to create the pci_dev_resource list, get_res_add_size()
> and get_res_add_align() is to retrieve the information in the list. I am not
> sure how to leverage add_to_list() in these two functions?
>
>>> +
>>> +
>>> /* Sort resources by alignment */
>>> static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head)
>>> {
>>> @@ -368,8 +390,9 @@ static void __assign_resources_sorted(struct list_head *head,
>>> LIST_HEAD(save_head);
>>> LIST_HEAD(local_fail_head);
>>> struct pci_dev_resource *save_res;
>>> - struct pci_dev_resource *dev_res, *tmp_res;
>>> + struct pci_dev_resource *dev_res, *tmp_res, *dev_res2;
>>> unsigned long fail_type;
>>> + resource_size_t add_align, align;
>>>
>>> /* Check if optional add_size is there */
>>> if (!realloc_head || list_empty(realloc_head))
>>> @@ -384,10 +407,31 @@ static void __assign_resources_sorted(struct list_head *head,
>>> }
>>>
>>> /* Update res in head list with add_size in realloc_head list */
>>> - list_for_each_entry(dev_res, head, list)
>>> + list_for_each_entry_safe(dev_res, tmp_res, head, list) {
>>> dev_res->res->end += get_res_add_size(realloc_head,
>>> dev_res->res);
>>>
>>> + if (!(dev_res->res->flags & IORESOURCE_STARTALIGN))
>>> + continue;
>>> +
>>> + add_align = get_res_add_align(realloc_head, dev_res->res);
>>> +
>>> + if (add_align > dev_res->res->start) {
>>> + dev_res->res->start = add_align;
>>> + dev_res->res->end = add_align +
>>> + resource_size(dev_res->res);
>>> +
>>> + list_for_each_entry(dev_res2, head, list) {
>>> + align = pci_resource_alignment(dev_res2->dev,
>>> + dev_res2->res);
>>> + if (add_align > align)
>>> + list_move_tail(&dev_res->list,
>>> + &dev_res2->list);
>>> + }
>>> + }
>>> +
>>> + }
>>> +
>>> /* Try updated head list with add_size added */
>>> assign_requested_resources_sorted(head, &local_fail_head);
>>>
>>> @@ -930,6 +974,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>> struct resource *b_res = find_free_bus_resource(bus,
>>> mask | IORESOURCE_PREFETCH, type);
>>> resource_size_t children_add_size = 0;
>>> + resource_size_t children_add_align = 0;
>>> + resource_size_t add_align = 0;
>>>
>>> if (!b_res)
>>> return -ENOSPC;
>>> @@ -954,6 +1000,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>> /* put SRIOV requested res to the optional list */
>>> if (realloc_head && i >= PCI_IOV_RESOURCES &&
>>> i <= PCI_IOV_RESOURCE_END) {
>>> + add_align = max(pci_resource_alignment(dev, r), add_align);
>>> r->end = r->start - 1;
>>> add_to_list(realloc_head, dev, r, r_size, 0/* don't care */);
>>> children_add_size += r_size;
>>> @@ -984,8 +1031,11 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>> if (order > max_order)
>>> max_order = order;
>>>
>>> - if (realloc_head)
>>> + if (realloc_head) {
>>> children_add_size += get_res_add_size(realloc_head, r);
>>> + children_add_align = get_res_add_align(realloc_head, r);
>>> + add_align = max(add_align, children_add_align);
>>> + }
>>> }
>>> }
>>>
>>> @@ -996,7 +1046,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>> add_size = children_add_size;
>>> size1 = (!realloc_head || (realloc_head && !add_size)) ? size0 :
>>> calculate_memsize(size, min_size, add_size,
>>> - resource_size(b_res), min_align);
>>> + resource_size(b_res), max(min_align, add_align));
>>> if (!size0 && !size1) {
>>> if (b_res->start || b_res->end)
>>> dev_info(&bus->self->dev, "disabling bridge window %pR to %pR (unused)\n",
>>> @@ -1008,10 +1058,12 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>> b_res->end = size0 + min_align - 1;
>>> b_res->flags |= IORESOURCE_STARTALIGN;
>>> if (size1 > size0 && realloc_head) {
>>> - add_to_list(realloc_head, bus->self, b_res, size1-size0, min_align);
>>> - dev_printk(KERN_DEBUG, &bus->self->dev, "bridge window %pR to %pR add_size %llx\n",
>>> - b_res, &bus->busn_res,
>>> - (unsigned long long)size1-size0);
>>> + add_to_list(realloc_head, bus->self, b_res, size1-size0,
>>> + max(min_align, add_align));
>>> + dev_printk(KERN_DEBUG, &bus->self->dev, "bridge window "
>>> + "%pR to %pR add_size %llx add_align %llx\n", b_res,
>>> + &bus->busn_res, (unsigned long long)size1-size0,
>>> + max(min_align, add_align));
>>
>>Factor out this "max(min_align, add_align)" thing so we don't have to
>>change these lines. Bonus points if you can also factor it out of the
>>calculate_memsize() call above. That one is a pretty complicated ternary
>>expression that should probably be turned into an "if" instead anyway.
>>
>
> Ok, I get your point. Let me make it more easy to read.
>
>>> }
>>> return 0;
>>> }
>>> --
>>> 1.7.9.5
>>>
>
> --
> Richard Yang
> Help you, Help me
>
^ permalink raw reply
* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Nicolin Chen @ 2014-09-09 20:27 UTC (permalink / raw)
To: Timur Tabi
Cc: Shengjiu Wang, alsa-devel, tiwai, linux-kernel, broonie,
lgirdwood, perex, Li.Xiubo, mpa, linuxppc-dev
In-Reply-To: <540F5D29.7040309@tabi.org>
On Tue, Sep 09, 2014 at 03:03:53PM -0500, Timur Tabi wrote:
> On 09/09/2014 02:59 PM, Nicolin Chen wrote:
> >+ /*
> >+ * Initially mark the clock to NULL for all platforms so that later
> >+ * clk_prepare_enable() will ignore and return 0 for non-clock cases.
> >+ */
> >+ ssi_private->clk = NULL;
>
> According to Mark, NULL is a valid clock, so this should be instead:
>
> ssi_private->clk = PTR_ERR(-EINVAL);
>
> although that doesn't sit well with me.
I guess Mark's comment is merely against the check for clk validation
because if talking about clk validation, we should check IS_ERR(clk)
rather than check !=NULL directly.
However, my approach doesn't need any check. The open() or pm_resume()
can just call clk_prepare_enable() directly. The __clk_enable() will
then handle the 'clk == NULL' case:
static int __clk_enable(struct clk *clk)
{
int ret = 0;
if (!clk)
return 0;
^ permalink raw reply
* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Timur Tabi @ 2014-09-09 20:37 UTC (permalink / raw)
To: Nicolin Chen
Cc: Shengjiu Wang, alsa-devel, tiwai, linux-kernel, broonie,
lgirdwood, perex, Li.Xiubo, mpa, linuxppc-dev
In-Reply-To: <20140909202710.GB5224@Asurada>
On 09/09/2014 03:27 PM, Nicolin Chen wrote:
> I guess Mark's comment is merely against the check for clk validation
> because if talking about clk validation, we should check IS_ERR(clk)
> rather than check !=NULL directly.
Ah, that makes sense now.
> However, my approach doesn't need any check. The open() or pm_resume()
> can just call clk_prepare_enable() directly. The __clk_enable() will
> then handle the 'clk == NULL' case:
Yes, I was thinking the same thing.
^ permalink raw reply
* Re: [PATCH 1/2 v6] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-09-09 21:12 UTC (permalink / raw)
To: Madhavan Srinivasan, benh, paulus, mpe; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1410282456-11287-2-git-send-email-maddy@linux.vnet.ibm.com>
On 09.09.14 19:07, Madhavan Srinivasan wrote:
> This patch adds kernel side support for software breakpoint.
> Design is that, by using an illegal instruction, we trap to hypervisor
> via Emulation Assistance interrupt, where we check for the illegal instruction
> and accordingly we return to Host or Guest. Patch also adds support for
> software breakpoint in PR KVM.
>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/kvm_ppc.h | 6 ++++++
> arch/powerpc/kvm/book3s.c | 3 ++-
> arch/powerpc/kvm/book3s_hv.c | 41 ++++++++++++++++++++++++++++++++++----
> arch/powerpc/kvm/book3s_pr.c | 3 +++
> arch/powerpc/kvm/emulate.c | 15 ++++++++++++++
> 5 files changed, 63 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index fb86a22..dd83c9a 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -38,6 +38,12 @@
> #include <asm/paca.h>
> #endif
>
> +/*
> + * KVMPPC_INST_SW_BREAKPOINT is debug Instruction
> + * for supporting software breakpoint.
> + */
> +#define KVMPPC_INST_SW_BREAKPOINT 0x00dddd00
> +
> enum emulation_result {
> EMULATE_DONE, /* no further processing */
> EMULATE_DO_MMIO, /* kvm_run filled with MMIO request */
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index dd03f6b..00e9c9f 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -778,7 +778,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> struct kvm_guest_debug *dbg)
> {
> - return -EINVAL;
> + vcpu->guest_debug = dbg->control;
> + return 0;
> }
>
> void kvmppc_decrementer_func(unsigned long data)
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 27cced9..000fbec 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -725,6 +725,30 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
> return kvmppc_hcall_impl_hv_realmode(cmd);
> }
>
> +static int kvmppc_emulate_debug_inst(struct kvm_run *run,
> + struct kvm_vcpu *vcpu)
> +{
> + u32 last_inst;
> +
> + if (kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
> + EMULATE_DONE) {
> + /*
> + * Fetch failed, so return to guest and
> + * try executing it again.
> + */
> + return RESUME_GUEST;
> + }
> +
> + if (last_inst == KVMPPC_INST_SW_BREAKPOINT) {
> + run->exit_reason = KVM_EXIT_DEBUG;
> + run->debug.arch.address = kvmppc_get_pc(vcpu);
> + return RESUME_HOST;
> + } else {
> + kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> + return RESUME_GUEST;
> + }
> +}
> +
> static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> struct task_struct *tsk)
> {
> @@ -807,12 +831,18 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> break;
> /*
> * This occurs if the guest executes an illegal instruction.
> - * We just generate a program interrupt to the guest, since
> - * we don't emulate any guest instructions at this stage.
> + * If the guest debug is disabled, generate a program interrupt
> + * to the guest. If guest debug is enabled, we need to check
> + * whether the instruction is a software breakpoint instruction.
> + * Accordingly return to Guest or Host.
> */
> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> - r = RESUME_GUEST;
> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) {
> + r = kvmppc_emulate_debug_inst(run, vcpu);
> + } else {
> + kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> + r = RESUME_GUEST;
> + }
> break;
> /*
> * This occurs if the guest (kernel or userspace), does something that
> @@ -922,6 +952,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
> long int i;
>
> switch (id) {
> + case KVM_REG_PPC_DEBUG_INST:
> + *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT);
> + break;
> case KVM_REG_PPC_HIOR:
> *val = get_reg_val(id, 0);
> break;
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index faffb27..6d73708 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
Very nice patch set :). The only thing we're missing now is that Book3S
PR does not allow sw breakpoints to arrive in user mode (MSR.PR == 1),
because there we're never going to arrive at kvmppc_emulate_instruction().
But this can come as a follow-up patch.
Alex
^ permalink raw reply
* Re: [PATCH 0/2 v6] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-09-09 21:15 UTC (permalink / raw)
To: Madhavan Srinivasan, benh, paulus, mpe; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1410282456-11287-1-git-send-email-maddy@linux.vnet.ibm.com>
On 09.09.14 19:07, Madhavan Srinivasan wrote:
> This patchset adds ppc64 server side support for software breakpoint
> and extends the use of illegal instruction as software
> breakpoint across ppc platform.
>
> Patch 1, adds kernel side support for software breakpoint.
> Design is that, by using an illegal instruction, we trap to
> hypervisor via Emulation Assistance interrupt, where we check
> for the illegal instruction and accordingly we return to Host
> or Guest. Patch also adds support for software breakpoint
> in PR KVM.
>
> Patch 2,extends the use of illegal instruction as software
> breakpoint instruction across the ppc platform. Patch extends
> booke program interrupt code to support software breakpoint.
Thanks, applied to kvm-ppc-queue.
Alex
^ permalink raw reply
* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Nicolin Chen @ 2014-09-09 21:09 UTC (permalink / raw)
To: Timur Tabi, Shengjiu Wang
Cc: alsa-devel, tiwai, Li.Xiubo, lgirdwood, perex, broonie, mpa,
linuxppc-dev, linux-kernel
In-Reply-To: <540F6506.4060302@tabi.org>
On Tue, Sep 09, 2014 at 03:37:26PM -0500, Timur Tabi wrote:
> >However, my approach doesn't need any check. The open() or pm_resume()
> >can just call clk_prepare_enable() directly. The __clk_enable() will
> >then handle the 'clk == NULL' case:
>
> Yes, I was thinking the same thing.
Because that's following your suggestion :)
@Shengjiu
Another thing I forgot to mention is we still need a return check for
clk_prepare_enable() which isn't in the current version. And I said
"doesn't need any check" is indicating the pre-check of the call.
Thank you
Nicolin
^ permalink raw reply
* Re: [PATCH v3] topology: add support for node_to_mem_node() to determine the fallback node
From: Andrew Morton @ 2014-09-10 0:11 UTC (permalink / raw)
To: Nishanth Aravamudan
Cc: Han Pingtian, Matt Mackall, David Rientjes, Pekka Enberg,
Linux Memory Management List, Paul Mackerras, Tejun Heo,
Joonsoo Kim, linuxppc-dev, Christoph Lameter, Wanpeng Li,
Anton Blanchard
In-Reply-To: <20140909190326.GD22906@linux.vnet.ibm.com>
On Tue, 9 Sep 2014 12:03:27 -0700 Nishanth Aravamudan <nacc@linux.vnet.ibm.com> wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> We need to determine the fallback node in slub allocator if the
> allocation target node is memoryless node. Without it, the SLUB wrongly
> select the node which has no memory and can't use a partial slab,
> because of node mismatch. Introduced function, node_to_mem_node(X), will
> return a node Y with memory that has the nearest distance. If X is
> memoryless node, it will return nearest distance node, but, if X is
> normal node, it will return itself.
>
> We will use this function in following patch to determine the fallback
> node.
>
> ...
>
> --- a/include/linux/topology.h
> +++ b/include/linux/topology.h
> @@ -119,11 +119,20 @@ static inline int numa_node_id(void)
> * Use the accessor functions set_numa_mem(), numa_mem_id() and cpu_to_mem().
This comment could be updated.
> */
> DECLARE_PER_CPU(int, _numa_mem_);
> +extern int _node_numa_mem_[MAX_NUMNODES];
>
> #ifndef set_numa_mem
> static inline void set_numa_mem(int node)
> {
> this_cpu_write(_numa_mem_, node);
> + _node_numa_mem_[numa_node_id()] = node;
> +}
> +#endif
> +
> +#ifndef node_to_mem_node
> +static inline int node_to_mem_node(int node)
> +{
> + return _node_numa_mem_[node];
> }
A wee bit of documentation wouldn't hurt.
How does node_to_mem_node(numa_node_id()) differ from numa_mem_id()?
If I'm reading things correctly, they should both always return the
same thing. If so, do we need both?
Will node_to_mem_node() ever actually be called with a node !=
numa_node_id()?
> #endif
>
> @@ -146,6 +155,7 @@ static inline int cpu_to_mem(int cpu)
> static inline void set_cpu_numa_mem(int cpu, int node)
> {
> per_cpu(_numa_mem_, cpu) = node;
> + _node_numa_mem_[cpu_to_node(cpu)] = node;
> }
> #endif
>
> @@ -159,6 +169,13 @@ static inline int numa_mem_id(void)
> }
> #endif
>
> +#ifndef node_to_mem_node
> +static inline int node_to_mem_node(int node)
> +{
> + return node;
> +}
> +#endif
> +
> #ifndef cpu_to_mem
> static inline int cpu_to_mem(int cpu)
> {
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 18cee0d4c8a2..0883c42936d4 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -85,6 +85,7 @@ EXPORT_PER_CPU_SYMBOL(numa_node);
> */
> DEFINE_PER_CPU(int, _numa_mem_); /* Kernel "local memory" node */
> EXPORT_PER_CPU_SYMBOL(_numa_mem_);
> +int _node_numa_mem_[MAX_NUMNODES];
How does this get updated as CPUs, memory and nodes are hot-added and
removed?
> #endif
>
^ permalink raw reply
* Re: [PATCH 2/3] slub: fallback to node_to_mem_node() node if allocating on memoryless node
From: Andrew Morton @ 2014-09-10 0:11 UTC (permalink / raw)
To: Nishanth Aravamudan
Cc: Han Pingtian, Matt Mackall, David Rientjes, Pekka Enberg,
Linux Memory Management List, Paul Mackerras, Tejun Heo,
Joonsoo Kim, linuxppc-dev, Christoph Lameter, Wanpeng Li,
Anton Blanchard
In-Reply-To: <20140909190514.GE22906@linux.vnet.ibm.com>
On Tue, 9 Sep 2014 12:05:14 -0700 Nishanth Aravamudan <nacc@linux.vnet.ibm.com> wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> Update the SLUB code to search for partial slabs on the nearest node
> with memory in the presence of memoryless nodes. Additionally, do not
> consider it to be an ALLOC_NODE_MISMATCH (and deactivate the slab) when
> a memoryless-node specified allocation goes off-node.
>
> ...
>
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1699,7 +1699,12 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
> struct kmem_cache_cpu *c)
> {
> void *object;
> - int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node;
> + int searchnode = node;
> +
> + if (node == NUMA_NO_NODE)
> + searchnode = numa_mem_id();
> + else if (!node_present_pages(node))
> + searchnode = node_to_mem_node(node);
I expect a call to node_to_mem_node() will always be preceded by a test
of node_present_pages(). Perhaps node_to_mem_node() should just do the
node_present_pages() call itself?
^ permalink raw reply
* Re: [PATCH v3] topology: add support for node_to_mem_node() to determine the fallback node
From: Nishanth Aravamudan @ 2014-09-10 0:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Han Pingtian, Matt Mackall, David Rientjes, Pekka Enberg,
Linux Memory Management List, Paul Mackerras, Tejun Heo,
Joonsoo Kim, linuxppc-dev, Christoph Lameter, Wanpeng Li,
Anton Blanchard
In-Reply-To: <20140909171115.75c7702c37dfb23b9e053636@linux-foundation.org>
On 09.09.2014 [17:11:15 -0700], Andrew Morton wrote:
> On Tue, 9 Sep 2014 12:03:27 -0700 Nishanth Aravamudan <nacc@linux.vnet.ibm.com> wrote:
>
> > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> >
> > We need to determine the fallback node in slub allocator if the
> > allocation target node is memoryless node. Without it, the SLUB wrongly
> > select the node which has no memory and can't use a partial slab,
> > because of node mismatch. Introduced function, node_to_mem_node(X), will
> > return a node Y with memory that has the nearest distance. If X is
> > memoryless node, it will return nearest distance node, but, if X is
> > normal node, it will return itself.
> >
> > We will use this function in following patch to determine the fallback
> > node.
> >
> > ...
> >
> > --- a/include/linux/topology.h
> > +++ b/include/linux/topology.h
> > @@ -119,11 +119,20 @@ static inline int numa_node_id(void)
> > * Use the accessor functions set_numa_mem(), numa_mem_id() and cpu_to_mem().
>
> This comment could be updated.
Will do, do you prefer a follow-on patch or one that replaces this one?
> > */
> > DECLARE_PER_CPU(int, _numa_mem_);
> > +extern int _node_numa_mem_[MAX_NUMNODES];
> >
> > #ifndef set_numa_mem
> > static inline void set_numa_mem(int node)
> > {
> > this_cpu_write(_numa_mem_, node);
> > + _node_numa_mem_[numa_node_id()] = node;
> > +}
> > +#endif
> > +
> > +#ifndef node_to_mem_node
> > +static inline int node_to_mem_node(int node)
> > +{
> > + return _node_numa_mem_[node];
> > }
>
> A wee bit of documentation wouldn't hurt.
>
> How does node_to_mem_node(numa_node_id()) differ from numa_mem_id()?
> If I'm reading things correctly, they should both always return the
> same thing. If so, do we need both?
That seems correct to me. The nearest memory node of this cpu's NUMA
node (node_to_mem_node(numa_node_id()) is always equal to the nearest
memory node (numa_mem_id()).
> Will node_to_mem_node() ever actually be called with a node !=
> numa_node_id()?
Well, it's a layering problem. The eventual callers of
node_to_mem_node() only have the requested NUMA node (if any) available.
I think because get_partial() __slab_alloc() allow for allocations for
any node, and that's where we see the slab deactivation issues, we need
to support this in the API.
In practice, it's probably that the node parameter is often
numa_node_id(), but we can't be sure of that in these call-paths,
afaict.
> > #endif
> >
> > @@ -146,6 +155,7 @@ static inline int cpu_to_mem(int cpu)
> > static inline void set_cpu_numa_mem(int cpu, int node)
> > {
> > per_cpu(_numa_mem_, cpu) = node;
> > + _node_numa_mem_[cpu_to_node(cpu)] = node;
> > }
> > #endif
> >
> > @@ -159,6 +169,13 @@ static inline int numa_mem_id(void)
> > }
> > #endif
> >
> > +#ifndef node_to_mem_node
> > +static inline int node_to_mem_node(int node)
> > +{
> > + return node;
> > +}
> > +#endif
> > +
> > #ifndef cpu_to_mem
> > static inline int cpu_to_mem(int cpu)
> > {
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 18cee0d4c8a2..0883c42936d4 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -85,6 +85,7 @@ EXPORT_PER_CPU_SYMBOL(numa_node);
> > */
> > DEFINE_PER_CPU(int, _numa_mem_); /* Kernel "local memory" node */
> > EXPORT_PER_CPU_SYMBOL(_numa_mem_);
> > +int _node_numa_mem_[MAX_NUMNODES];
>
> How does this get updated as CPUs, memory and nodes are hot-added and
> removed?
As CPUs are added, the architecture code in the CPU bringup will update
the NUMA topology. Memory and node hotplug are still open issues, I
mentioned the former in the cover letter. I should have mentioned it in
this commit message as well.
I do notice that Lee's commit message from 7aac78988551 ("numa:
introduce numa_mem_id()- effective local memory node id"):
"Generic initialization of 'numa_mem' occurs in __build_all_zonelists().
This will initialize the boot cpu at boot time, and all cpus on change
of numa_zonelist_order, or when node or memory hot-plug requires
zonelist rebuild. Archs that support memoryless nodes will need to
initialize 'numa_mem' for secondary cpus as they're brought on-line."
And since we update the _node_numa_mem_ value on set_cpu_numa_mem()
calls, which were already needed for numa_mem_id(), we might be covered.
Testing these cases (hotplug) is next in my plans.
Thanks,
Nish
^ permalink raw reply
* Re: [PATCH 2/3] slub: fallback to node_to_mem_node() node if allocating on memoryless node
From: Nishanth Aravamudan @ 2014-09-10 0:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Han Pingtian, Matt Mackall, David Rientjes, Pekka Enberg,
Linux Memory Management List, Paul Mackerras, Tejun Heo,
Joonsoo Kim, linuxppc-dev, Christoph Lameter, Wanpeng Li,
Anton Blanchard
In-Reply-To: <20140909171125.de9844579d55599c59260afb@linux-foundation.org>
On 09.09.2014 [17:11:25 -0700], Andrew Morton wrote:
> On Tue, 9 Sep 2014 12:05:14 -0700 Nishanth Aravamudan <nacc@linux.vnet.ibm.com> wrote:
>
> > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> >
> > Update the SLUB code to search for partial slabs on the nearest node
> > with memory in the presence of memoryless nodes. Additionally, do not
> > consider it to be an ALLOC_NODE_MISMATCH (and deactivate the slab) when
> > a memoryless-node specified allocation goes off-node.
> >
> > ...
> >
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -1699,7 +1699,12 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
> > struct kmem_cache_cpu *c)
> > {
> > void *object;
> > - int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node;
> > + int searchnode = node;
> > +
> > + if (node == NUMA_NO_NODE)
> > + searchnode = numa_mem_id();
> > + else if (!node_present_pages(node))
> > + searchnode = node_to_mem_node(node);
>
> I expect a call to node_to_mem_node() will always be preceded by a test
> of node_present_pages(). Perhaps node_to_mem_node() should just do the
> node_present_pages() call itself?
Really, we don't need that test here. We could always use the result of
node_to_mem_node() in the else. If memoryless nodes are not supported
(off in .config), then node_to_mem_node() trivially returns. If they are
supported, it returns the correct value for all nodes.
It's just an optimization (premature?) since we can avoid worrying (in
this path) about memoryless nodes if the node in question has memory.
And, in fact, in __slab_alloc(), we could do the following:
...
int searchnode = node;
if (node != NUMA_NO_NODE)
searchnode = node_to_mem_node(node);
if (node != searchnode &&
unlikely(!node_match(page, searchnode))) {
...
which would minimize the impact to non-memoryless node NUMA configs.
Does that seem better to you? I can add comments to this patch as well.
Thanks,
Nish
^ permalink raw reply
* Re: [PATCH v2] QE: move qe code from arch/powerpc to drivers/soc
From: Scott Wood @ 2014-09-09 23:26 UTC (permalink / raw)
To: Zhao Qiang-B45475
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
Xie Xiaobo-R63061
In-Reply-To: <88ded7369c0546aabeececed5f1a4c06@BLUPR03MB341.namprd03.prod.outlook.com>
On Tue, 2014-09-09 at 04:27 -0500, Zhao Qiang-B45475 wrote:
> On Fri, 2014-09-05 at 06:47 AM, Wood Scott wrote:
>
>
>
> > Subject: Re: [PATCH v2] QE: move qe code from arch/powerpc to drivers/soc
> >
> > On Thu, 2014-09-04 at 13:06 +0800, Zhao Qiang wrote:
> > > LS1 is arm cpu and it has qe ip block.
> > > move qe code from platform directory to public directory.
> > >
> > > QE is an IP block integrates several comunications peripheral
> > > controllers. It can implement a variety of applications, such as uart,
> > > usb and tdm and so on.
> > >
> > > Signed-off-by: Zhao Qiang <B45475@freescale.com>
> > > ---
> > > Changes for v2:
> > > - mv code to drivers/soc
> >
> > Who will be the maintainer of this code once it lives in drivers/soc,
> > especially once it is no longer used only by PPC?
>
> I have no idea about that, can you explain why you want to know who will be the maintainer.
Currently it falls within "Freescale PPC" which I maintain. This moves
it out of arch/powerpc and is the first step towards making it not be
PPC-specific. Whose tree are you expecting patches to
drivers/soc/fsl-qe to go through?
> > > diff --git a/drivers/soc/qe/Kconfig b/drivers/soc/qe/Kconfig new file
> > > mode 100644 index 0000000..8b03ca2
> > > --- /dev/null
> > > +++ b/drivers/soc/qe/Kconfig
> > > @@ -0,0 +1,45 @@
> > > +#
> > > +# QE Communication options
> > > +#
> > > +config QUICC_ENGINE
> > > + bool "Freescale QUICC Engine (QE) Support"
> > > + depends on FSL_SOC && PPC32
> > > + select PPC_LIB_RHEAP
> > > + select CRC32
> > > + help
> > > + The QUICC Engine (QE) is a new generation of communications
> > > + coprocessors on Freescale embedded CPUs (akin to CPM in older
> > chips).
> > > + Selecting this option means that you wish to build a kernel
> > > + for a machine with a QE coprocessor.
> > > +
> > > +config QE_GPIO
> > > + bool "QE GPIO support"
> > > + depends on QUICC_ENGINE
> > > + select ARCH_REQUIRE_GPIOLIB
> > > + help
> > > + Say Y here if you're going to use hardware that connects to the
> > > + QE GPIOs.
> > > +
> > > +config UCC_SLOW
> > > + bool
> > > + default y if SERIAL_QE
> > > + help
> > > + This option provides qe_lib support to UCC slow
> > > + protocols: UART, BISYNC, QMC
> > > +
> > > +config UCC_FAST
> > > + bool
> > > + default y if UCC_GETH
> > > + help
> > > + This option provides qe_lib support to UCC fast
> > > + protocols: HDLC, Ethernet, ATM, transparent
> > > +
> > > +config UCC
> > > + bool
> > > + default y if UCC_FAST || UCC_SLOW
> > > +
> > > +config QE_USB
> > > + bool
> > > + default y if USB_FSL_QE
> > > + help
> > > + QE USB Controller support
> >
> > First could we give these names better namespacing?
>
> Add FSL as prefix?
Yes.
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc/eeh: fix crashing when passing VF
From: Wei Yang @ 2014-09-10 2:26 UTC (permalink / raw)
To: Gavin Shan; +Cc: Wei Yang, benh, linuxppc-dev
In-Reply-To: <20140820020735.GA12789@shangw>
Hi, Ben
Sounds this is not merged in the mainline yet.
Would you like me sending a new version with those fix? Or you don't like
this?
On Wed, Aug 20, 2014 at 12:07:35PM +1000, Gavin Shan wrote:
>On Tue, Aug 19, 2014 at 10:27:09AM +0800, Wei Yang wrote:
>
>The subject would be "powerpc/eeh: Fix kernel crash when passing through VF".
>
>>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 is a quick fix for this problem.
>>
>>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>
>With all minor comments fixed:
>
>Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>
>>---
>> arch/powerpc/kernel/eeh.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>>index 0ba4392..d2d2130 100644
>>--- a/arch/powerpc/kernel/eeh.c
>>+++ b/arch/powerpc/kernel/eeh.c
>>@@ -630,7 +630,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;
>
>It would be:
>
> struct eeh_pe *pe = edev ? edev->pe : NULL;
>
>>
>> if (!pe) {
>> pr_err("%s: No PE found on PCI device %s\n",
>
>Thanks,
>Gavin
--
Richard Yang
Help you, Help me
^ permalink raw reply
* Re: [PATCH V7 04/17] PCI: Take additional IOV BAR alignment in sizing and assigning
From: Wei Yang @ 2014-09-10 3:27 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Wei Yang, Benjamin Herrenschmidt, linux-pci@vger.kernel.org,
Gavin Shan, Guo Chao, Mike Qiu, linuxppc-dev
In-Reply-To: <CAErSpo5ZSB-=aQtiM5OenfWWfn2b=TOwxM-BkpwxBGRnmMyAzw@mail.gmail.com>
On Tue, Sep 09, 2014 at 02:09:46PM -0600, Bjorn Helgaas wrote:
>On Wed, Aug 20, 2014 at 12:14 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
>> On Tue, Aug 19, 2014 at 09:08:41PM -0600, Bjorn Helgaas wrote:
>>>On Thu, Jul 24, 2014 at 02:22:14PM +0800, Wei Yang wrote:
>>>> At resource sizing/assigning stage, resources are divided into two lists,
>>>> requested list and additional list, while the alignement of the additional
>>>> IOV BAR is not taken into the sizeing and assigning procedure.
>>>>
>>>> This is reasonable in the original implementation, since IOV BAR's alignment is
>>>> mostly the size of a PF BAR alignemt. This means the alignment is already taken
>>>> into consideration. While this rule may be violated on some platform.
>>>>
>>>> This patch take the additional IOV BAR alignment in sizing and assigning stage
>>>> explicitly.
>>>>
>>>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>>> ---
>>>> drivers/pci/setup-bus.c | 68 +++++++++++++++++++++++++++++++++++++++++------
>>>> 1 file changed, 60 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>>>> index a5a63ec..d83681f 100644
>>>> --- a/drivers/pci/setup-bus.c
>>>> +++ b/drivers/pci/setup-bus.c
>>>> @@ -120,6 +120,28 @@ static resource_size_t get_res_add_size(struct list_head *head,
>>>> return 0;
>>>> }
>>>>
>>>> +static resource_size_t get_res_add_align(struct list_head *head,
>>>> + struct resource *res)
>>>> +{
>>>> + struct pci_dev_resource *dev_res;
>>>> +
>>>> + list_for_each_entry(dev_res, head, list) {
>>>> + if (dev_res->res == res) {
>>>> + int idx = res - &dev_res->dev->resource[0];
>>>> +
>>>> + dev_printk(KERN_DEBUG, &dev_res->dev->dev,
>>>> + "res[%d]=%pR get_res_add_align min_align %llx\n",
>>>> + idx, dev_res->res,
>>>> + (unsigned long long)dev_res->min_align);
>>>> +
>>>> + return dev_res->min_align;
>>>> + }
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>
>>>I see that you copied the structure of the existing get_res_add_size()
>>>here. But I don't understand *that* function. It looks basically like
>>>this:
>>>
>>> resource_size_t get_res_add_size(list, res)
>>> {
>>> list_for_each_entry(dev_res, head, list) {
>>> if (dev_res->res == res)
>>> return dev_res->add_size;
>>> }
>>> return 0;
>>> }
>>>
>>>and we call it like this:
>>>
>>> dev_res->res->end += get_res_add_size(realloc_head, dev_res->res);
>>>
>>>So we start out with dev_res", pass in dev_res->res, search the
>>>realloc_head list to find dev_res again, and return dev_res->add_size.
>>>That looks equivalent to just:
>>>
>>> dev_res->res->end += dev_res->add_size;
>>>
>>>It looks like get_res_add_size() merely adds a printk and some complexity.
>>>Am I missing something?
>>>
>>
>> Let me try to explain it, if not correct, please let know :-)
>>
>> dev_res->res->end += get_res_add_size(realloc_head, dev_res->res);
>>
>> would be expanded to:
>>
>> dev_res->res->end += dev_res_1->add_size;
>>
>> with the dev_res_1 is another one from dev_res which is stored in realloc_head.
>
>Yep, I see now.
>
>>>I do see that there are other callers where we don't actually start with
>>>dev_res, which makes it a little more complicated. But I think you should
>>>either add something like this:
>>>
>>> struct pci_dev_resource *res_to_dev_res(list, res)
>>> {
>>> list_for_each_entry(dev_res, head, list) {
>>> if (dev_res->res == res)
>>> return dev_res;
>>> }
>>> return NULL;
>>> }
>>>
>>
>> Ok, we can extract the common part of these two functions.
>>
>>>which can be used to replace get_res_add_size() and get_res_add_align(), OR
>>>figure out whether the dev_res of interest is always one we've just added.
>>>If it is, maybe you can just make add_to_list() return the dev_res pointer
>>>instead of an errno, and hang onto the pointer. I'd like that much better
>>>if that's possible.
>>>
>>
>> Sorry, I don't get this point.
>
>Don't worry, it didn't make sense. I was thinking that we knew the
>dev_res up front and didn't need to look it up, but that's not the
>case.
>
>Sorry it took me so long to respond to this; I'm a bit swamped dealing
>with some regressions.
:-) Never mind, those regressions are with higher priority then this new
feature.
And I found some bugs in this version during the test, and will merge those
fixes in the next version.
>
>Bjorn
>
>> add_to_list() is used to create the pci_dev_resource list, get_res_add_size()
>> and get_res_add_align() is to retrieve the information in the list. I am not
>> sure how to leverage add_to_list() in these two functions?
>>
>>>> +
>>>> +
>>>> /* Sort resources by alignment */
>>>> static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head)
>>>> {
>>>> @@ -368,8 +390,9 @@ static void __assign_resources_sorted(struct list_head *head,
>>>> LIST_HEAD(save_head);
>>>> LIST_HEAD(local_fail_head);
>>>> struct pci_dev_resource *save_res;
>>>> - struct pci_dev_resource *dev_res, *tmp_res;
>>>> + struct pci_dev_resource *dev_res, *tmp_res, *dev_res2;
>>>> unsigned long fail_type;
>>>> + resource_size_t add_align, align;
>>>>
>>>> /* Check if optional add_size is there */
>>>> if (!realloc_head || list_empty(realloc_head))
>>>> @@ -384,10 +407,31 @@ static void __assign_resources_sorted(struct list_head *head,
>>>> }
>>>>
>>>> /* Update res in head list with add_size in realloc_head list */
>>>> - list_for_each_entry(dev_res, head, list)
>>>> + list_for_each_entry_safe(dev_res, tmp_res, head, list) {
>>>> dev_res->res->end += get_res_add_size(realloc_head,
>>>> dev_res->res);
>>>>
>>>> + if (!(dev_res->res->flags & IORESOURCE_STARTALIGN))
>>>> + continue;
>>>> +
>>>> + add_align = get_res_add_align(realloc_head, dev_res->res);
>>>> +
>>>> + if (add_align > dev_res->res->start) {
>>>> + dev_res->res->start = add_align;
>>>> + dev_res->res->end = add_align +
>>>> + resource_size(dev_res->res);
>>>> +
>>>> + list_for_each_entry(dev_res2, head, list) {
>>>> + align = pci_resource_alignment(dev_res2->dev,
>>>> + dev_res2->res);
>>>> + if (add_align > align)
>>>> + list_move_tail(&dev_res->list,
>>>> + &dev_res2->list);
>>>> + }
>>>> + }
>>>> +
>>>> + }
>>>> +
>>>> /* Try updated head list with add_size added */
>>>> assign_requested_resources_sorted(head, &local_fail_head);
>>>>
>>>> @@ -930,6 +974,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>>> struct resource *b_res = find_free_bus_resource(bus,
>>>> mask | IORESOURCE_PREFETCH, type);
>>>> resource_size_t children_add_size = 0;
>>>> + resource_size_t children_add_align = 0;
>>>> + resource_size_t add_align = 0;
>>>>
>>>> if (!b_res)
>>>> return -ENOSPC;
>>>> @@ -954,6 +1000,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>>> /* put SRIOV requested res to the optional list */
>>>> if (realloc_head && i >= PCI_IOV_RESOURCES &&
>>>> i <= PCI_IOV_RESOURCE_END) {
>>>> + add_align = max(pci_resource_alignment(dev, r), add_align);
>>>> r->end = r->start - 1;
>>>> add_to_list(realloc_head, dev, r, r_size, 0/* don't care */);
>>>> children_add_size += r_size;
>>>> @@ -984,8 +1031,11 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>>> if (order > max_order)
>>>> max_order = order;
>>>>
>>>> - if (realloc_head)
>>>> + if (realloc_head) {
>>>> children_add_size += get_res_add_size(realloc_head, r);
>>>> + children_add_align = get_res_add_align(realloc_head, r);
>>>> + add_align = max(add_align, children_add_align);
>>>> + }
>>>> }
>>>> }
>>>>
>>>> @@ -996,7 +1046,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>>> add_size = children_add_size;
>>>> size1 = (!realloc_head || (realloc_head && !add_size)) ? size0 :
>>>> calculate_memsize(size, min_size, add_size,
>>>> - resource_size(b_res), min_align);
>>>> + resource_size(b_res), max(min_align, add_align));
>>>> if (!size0 && !size1) {
>>>> if (b_res->start || b_res->end)
>>>> dev_info(&bus->self->dev, "disabling bridge window %pR to %pR (unused)\n",
>>>> @@ -1008,10 +1058,12 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>>> b_res->end = size0 + min_align - 1;
>>>> b_res->flags |= IORESOURCE_STARTALIGN;
>>>> if (size1 > size0 && realloc_head) {
>>>> - add_to_list(realloc_head, bus->self, b_res, size1-size0, min_align);
>>>> - dev_printk(KERN_DEBUG, &bus->self->dev, "bridge window %pR to %pR add_size %llx\n",
>>>> - b_res, &bus->busn_res,
>>>> - (unsigned long long)size1-size0);
>>>> + add_to_list(realloc_head, bus->self, b_res, size1-size0,
>>>> + max(min_align, add_align));
>>>> + dev_printk(KERN_DEBUG, &bus->self->dev, "bridge window "
>>>> + "%pR to %pR add_size %llx add_align %llx\n", b_res,
>>>> + &bus->busn_res, (unsigned long long)size1-size0,
>>>> + max(min_align, add_align));
>>>
>>>Factor out this "max(min_align, add_align)" thing so we don't have to
>>>change these lines. Bonus points if you can also factor it out of the
>>>calculate_memsize() call above. That one is a pretty complicated ternary
>>>expression that should probably be turned into an "if" instead anyway.
>>>
>>
>> Ok, I get your point. Let me make it more easy to read.
>>
>>>> }
>>>> return 0;
>>>> }
>>>> --
>>>> 1.7.9.5
>>>>
>>
>> --
>> Richard Yang
>> Help you, Help me
>>
--
Richard Yang
Help you, Help me
^ permalink raw reply
* RE: [PATCH v2] QE: move qe code from arch/powerpc to drivers/soc
From: qiang.zhao @ 2014-09-10 3:38 UTC (permalink / raw)
To: Scott Wood
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
Xiaobo Xie
In-Reply-To: <1410305198.24184.336.camel@snotra.buserror.net>
T24gV2VkLCAyMDE0LTA5LTEwIGF0IDA3OjI3IC0wNTAwLCBXb29kIFNjb3R0LUIwNzQyMSB3cm90
ZToNCg0KPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBXb29kIFNjb3R0LUIw
NzQyMQ0KPiBTZW50OiBXZWRuZXNkYXksIFNlcHRlbWJlciAxMCwgMjAxNCA3OjI3IEFNDQo+IFRv
OiBaaGFvIFFpYW5nLUI0NTQ3NQ0KPiBDYzogTGkgWWFuZy1MZW8tUjU4NDcyOyBsaW51eHBwYy1k
ZXZAbGlzdHMub3psYWJzLm9yZzsgWGllIFhpYW9iby1SNjMwNjE7DQo+IGxpbnV4LWtlcm5lbEB2
Z2VyLmtlcm5lbC5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRDSCB2Ml0gUUU6IG1vdmUgcWUgY29k
ZSBmcm9tIGFyY2gvcG93ZXJwYyB0byBkcml2ZXJzL3NvYw0KPiANCj4gT24gVHVlLCAyMDE0LTA5
LTA5IGF0IDA0OjI3IC0wNTAwLCBaaGFvIFFpYW5nLUI0NTQ3NSB3cm90ZToNCj4gPiBPbiBGcmks
IDIwMTQtMDktMDUgYXQgMDY6NDcgQU0sIFdvb2QgU2NvdHQgd3JvdGU6DQo+ID4NCj4gPg0KPiA+
DQo+ID4gPiBTdWJqZWN0OiBSZTogW1BBVENIIHYyXSBRRTogbW92ZSBxZSBjb2RlIGZyb20gYXJj
aC9wb3dlcnBjIHRvDQo+ID4gPiBkcml2ZXJzL3NvYw0KPiA+ID4NCj4gPiA+IE9uIFRodSwgMjAx
NC0wOS0wNCBhdCAxMzowNiArMDgwMCwgWmhhbyBRaWFuZyB3cm90ZToNCj4gPiA+ID4gTFMxIGlz
IGFybSBjcHUgYW5kIGl0IGhhcyBxZSBpcCBibG9jay4NCj4gPiA+ID4gbW92ZSBxZSBjb2RlIGZy
b20gcGxhdGZvcm0gZGlyZWN0b3J5IHRvIHB1YmxpYyBkaXJlY3RvcnkuDQo+ID4gPiA+DQo+ID4g
PiA+IFFFIGlzIGFuIElQIGJsb2NrIGludGVncmF0ZXMgc2V2ZXJhbCBjb211bmljYXRpb25zIHBl
cmlwaGVyYWwNCj4gPiA+ID4gY29udHJvbGxlcnMuIEl0IGNhbiBpbXBsZW1lbnQgYSB2YXJpZXR5
IG9mIGFwcGxpY2F0aW9ucywgc3VjaCBhcw0KPiA+ID4gPiB1YXJ0LCB1c2IgYW5kIHRkbSBhbmQg
c28gb24uDQo+ID4gPiA+DQo+ID4gPiA+IFNpZ25lZC1vZmYtYnk6IFpoYW8gUWlhbmcgPEI0NTQ3
NUBmcmVlc2NhbGUuY29tPg0KPiA+ID4gPiAtLS0NCj4gPiA+ID4gQ2hhbmdlcyBmb3IgdjI6DQo+
ID4gPiA+IAktIG12IGNvZGUgdG8gZHJpdmVycy9zb2MNCj4gPiA+DQo+ID4gPiBXaG8gd2lsbCBi
ZSB0aGUgbWFpbnRhaW5lciBvZiB0aGlzIGNvZGUgb25jZSBpdCBsaXZlcyBpbg0KPiA+ID4gZHJp
dmVycy9zb2MsIGVzcGVjaWFsbHkgb25jZSBpdCBpcyBubyBsb25nZXIgdXNlZCBvbmx5IGJ5IFBQ
Qz8NCj4gPg0KPiA+IEkgaGF2ZSBubyBpZGVhIGFib3V0IHRoYXQsIGNhbiB5b3UgZXhwbGFpbiB3
aHkgeW91IHdhbnQgdG8ga25vdyB3aG8NCj4gd2lsbCBiZSB0aGUgbWFpbnRhaW5lci4NCj4gDQo+
IEN1cnJlbnRseSBpdCBmYWxscyB3aXRoaW4gIkZyZWVzY2FsZSBQUEMiIHdoaWNoIEkgbWFpbnRh
aW4uICBUaGlzIG1vdmVzDQo+IGl0IG91dCBvZiBhcmNoL3Bvd2VycGMgYW5kIGlzIHRoZSBmaXJz
dCBzdGVwIHRvd2FyZHMgbWFraW5nIGl0IG5vdCBiZQ0KPiBQUEMtc3BlY2lmaWMuICBXaG9zZSB0
cmVlIGFyZSB5b3UgZXhwZWN0aW5nIHBhdGNoZXMgdG8gZHJpdmVycy9zb2MvZnNsLXFlDQo+IHRv
IGdvIHRocm91Z2g/DQoNCkhvdyBhYm91dCBuZXR3b3JrIHRyZWU/DQoNCj4gDQo+ID4gPiA+IGRp
ZmYgLS1naXQgYS9kcml2ZXJzL3NvYy9xZS9LY29uZmlnIGIvZHJpdmVycy9zb2MvcWUvS2NvbmZp
ZyBuZXcNCj4gPiA+ID4gZmlsZSBtb2RlIDEwMDY0NCBpbmRleCAwMDAwMDAwLi44YjAzY2EyDQo+
ID4gPiA+IC0tLSAvZGV2L251bGwNCj4gPiA+ID4gKysrIGIvZHJpdmVycy9zb2MvcWUvS2NvbmZp
Zw0KPiA+ID4gPiBAQCAtMCwwICsxLDQ1IEBADQo+ID4gPiA+ICsjDQo+ID4gPiA+ICsjIFFFIENv
bW11bmljYXRpb24gb3B0aW9ucw0KPiA+ID4gPiArIw0KPiA+ID4gPiArY29uZmlnIFFVSUNDX0VO
R0lORQ0KPiA+ID4gPiArCWJvb2wgIkZyZWVzY2FsZSBRVUlDQyBFbmdpbmUgKFFFKSBTdXBwb3J0
Ig0KPiA+ID4gPiArCWRlcGVuZHMgb24gRlNMX1NPQyAmJiBQUEMzMg0KPiA+ID4gPiArCXNlbGVj
dCBQUENfTElCX1JIRUFQDQo+ID4gPiA+ICsJc2VsZWN0IENSQzMyDQo+ID4gPiA+ICsJaGVscA0K
PiA+ID4gPiArCSAgVGhlIFFVSUNDIEVuZ2luZSAoUUUpIGlzIGEgbmV3IGdlbmVyYXRpb24gb2Yg
Y29tbXVuaWNhdGlvbnMNCj4gPiA+ID4gKwkgIGNvcHJvY2Vzc29ycyBvbiBGcmVlc2NhbGUgZW1i
ZWRkZWQgQ1BVcyAoYWtpbiB0byBDUE0gaW4NCj4gb2xkZXINCj4gPiA+IGNoaXBzKS4NCj4gPiA+
ID4gKwkgIFNlbGVjdGluZyB0aGlzIG9wdGlvbiBtZWFucyB0aGF0IHlvdSB3aXNoIHRvIGJ1aWxk
IGEga2VybmVsDQo+ID4gPiA+ICsJICBmb3IgYSBtYWNoaW5lIHdpdGggYSBRRSBjb3Byb2Nlc3Nv
ci4NCj4gPiA+ID4gKw0KPiA+ID4gPiArY29uZmlnIFFFX0dQSU8NCj4gPiA+ID4gKwlib29sICJR
RSBHUElPIHN1cHBvcnQiDQo+ID4gPiA+ICsJZGVwZW5kcyBvbiBRVUlDQ19FTkdJTkUNCj4gPiA+
ID4gKwlzZWxlY3QgQVJDSF9SRVFVSVJFX0dQSU9MSUINCj4gPiA+ID4gKwloZWxwDQo+ID4gPiA+
ICsJICBTYXkgWSBoZXJlIGlmIHlvdSdyZSBnb2luZyB0byB1c2UgaGFyZHdhcmUgdGhhdCBjb25u
ZWN0cyB0bw0KPiB0aGUNCj4gPiA+ID4gKwkgIFFFIEdQSU9zLg0KPiA+ID4gPiArDQo+ID4gPiA+
ICtjb25maWcgVUNDX1NMT1cNCj4gPiA+ID4gKwlib29sDQo+ID4gPiA+ICsJZGVmYXVsdCB5IGlm
IFNFUklBTF9RRQ0KPiA+ID4gPiArCWhlbHANCj4gPiA+ID4gKwkgIFRoaXMgb3B0aW9uIHByb3Zp
ZGVzIHFlX2xpYiBzdXBwb3J0IHRvIFVDQyBzbG93DQo+ID4gPiA+ICsJICBwcm90b2NvbHM6IFVB
UlQsIEJJU1lOQywgUU1DDQo+ID4gPiA+ICsNCj4gPiA+ID4gK2NvbmZpZyBVQ0NfRkFTVA0KPiA+
ID4gPiArCWJvb2wNCj4gPiA+ID4gKwlkZWZhdWx0IHkgaWYgVUNDX0dFVEgNCj4gPiA+ID4gKwlo
ZWxwDQo+ID4gPiA+ICsJICBUaGlzIG9wdGlvbiBwcm92aWRlcyBxZV9saWIgc3VwcG9ydCB0byBV
Q0MgZmFzdA0KPiA+ID4gPiArCSAgcHJvdG9jb2xzOiBIRExDLCBFdGhlcm5ldCwgQVRNLCB0cmFu
c3BhcmVudA0KPiA+ID4gPiArDQo+ID4gPiA+ICtjb25maWcgVUNDDQo+ID4gPiA+ICsJYm9vbA0K
PiA+ID4gPiArCWRlZmF1bHQgeSBpZiBVQ0NfRkFTVCB8fCBVQ0NfU0xPVw0KPiA+ID4gPiArDQo+
ID4gPiA+ICtjb25maWcgUUVfVVNCDQo+ID4gPiA+ICsJYm9vbA0KPiA+ID4gPiArCWRlZmF1bHQg
eSBpZiBVU0JfRlNMX1FFDQo+ID4gPiA+ICsJaGVscA0KPiA+ID4gPiArCSAgUUUgVVNCIENvbnRy
b2xsZXIgc3VwcG9ydA0KPiA+ID4NCj4gPiA+IEZpcnN0IGNvdWxkIHdlIGdpdmUgdGhlc2UgbmFt
ZXMgYmV0dGVyIG5hbWVzcGFjaW5nPw0KPiA+DQo+ID4gQWRkIEZTTCBhcyBwcmVmaXg/DQo+IA0K
PiBZZXMuDQoNCklmIHRoZSBuYW1lcyBjaGFuZ2VkLCB0aGVyZSB3aWxsIGJlIHNvIGEgbG90IG9m
IGNoYW5nZXMuDQpDYW4gSSBzZW5kIGFub3RoZXIgcGF0Y2ggdG8gY2hhbmdlIGl0Pw0KDQo+IA0K
PiAtU2NvdHQNCj4gDQpSZWdhcmRzLA0KWmhhbyBRaWFuZw0K
^ permalink raw reply
* Re: [PATCH] powerpc: dts: t208x: Change T208x USB controller version
From: Scott Wood @ 2014-09-10 5:16 UTC (permalink / raw)
To: Badola Nikhil-B46172
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <5b5401b39f83458bb5ada39d13abee29@BN1PR0301MB0593.namprd03.prod.outlook.com>
On Tue, 2014-09-09 at 23:45 -0500, Badola Nikhil-B46172 wrote:
> >-----Original Message-----
> >From: Wood Scott-B07421
> >Sent: Friday, August 22, 2014 4:07 AM
> >To: Badola Nikhil-B46172
> >Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org
> >Subject: Re: [PATCH] powerpc: dts: t208x: Change T208x USB controller version
> >
> >On Thu, 2014-08-21 at 16:01 +0530, Nikhil Badola wrote:
> >> Change USB controller version to 2.5 in compatible string for
> >> T2080/T2081
> >>
> >> Signed-off-by: Nikhil Badola <nikhil.badola@freescale.com>
> >> ---
> >> Checkpatch warnings handled by commit
> >> 61a8c2c6fe71082de3ea8629589dcdd0cc5c3f02
> >
> >That checkpatch warning is known to have false positives in cases where the
> >binding says "<CHIPNAME>-device" or "device-<VERSION>". If you want to
> >update the binding to give an example with a version, that's fine, but
> >checkpatch shouldn't be why. We're not going to update the binding example
> >again to match a different version the next time one is added to a device tree...
> >
> >> Documentation: dts: fsl-usb: Document USB node compatible string for
> >IP version
> >>
> >> arch/powerpc/boot/dts/fsl/t2081si-post.dtsi | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
> >> b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
> >> index 97479f0..aecee96 100644
> >> --- a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
> >> +++ b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
> >> @@ -410,7 +410,7 @@
> >> /include/ "qoriq-gpio-3.dtsi"
> >> /include/ "qoriq-usb2-mph-0.dtsi"
> >> usb0: usb@210000 {
> >> - compatible = "fsl-usb2-mph-v2.4", "fsl-usb2-mph";
> >> + compatible = "fsl-usb2-mph-v2.5", "fsl-usb2-mph";
> >
> >This is an example of why it's better to rely on version registers when present.
> >
> >-Scott
> >
> Hi scott,
>
> I can see this patch in "superseded" state in patchwork. As per our discussion the IP version checking is to be done by compatible string only, so can I proceed to send this patch again?
Sorry, apparently I thought that
http://patchwork.ozlabs.org/patch/383670/ was a newer version of the
patch, and didn't notice they were for different chips. No need to
resend. Thanks for pointing this out.
-Scott
^ permalink raw reply
* RE: [PATCH] powerpc: dts: t208x: Change T208x USB controller version
From: nikhil.badola @ 2014-09-10 4:45 UTC (permalink / raw)
To: Scott Wood; +Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1408660593.6510.0.camel@snotra.buserror.net>
Pi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+RnJvbTogV29vZCBTY290dC1CMDc0MjENCj5T
ZW50OiBGcmlkYXksIEF1Z3VzdCAyMiwgMjAxNCA0OjA3IEFNDQo+VG86IEJhZG9sYSBOaWtoaWwt
QjQ2MTcyDQo+Q2M6IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOyBkZXZpY2V0cmVlQHZn
ZXIua2VybmVsLm9yZw0KPlN1YmplY3Q6IFJlOiBbUEFUQ0hdIHBvd2VycGM6IGR0czogdDIwOHg6
IENoYW5nZSBUMjA4eCBVU0IgY29udHJvbGxlciB2ZXJzaW9uDQo+DQo+T24gVGh1LCAyMDE0LTA4
LTIxIGF0IDE2OjAxICswNTMwLCBOaWtoaWwgQmFkb2xhIHdyb3RlOg0KPj4gQ2hhbmdlIFVTQiBj
b250cm9sbGVyIHZlcnNpb24gdG8gMi41IGluIGNvbXBhdGlibGUgc3RyaW5nIGZvcg0KPj4gVDIw
ODAvVDIwODENCj4+DQo+PiBTaWduZWQtb2ZmLWJ5OiBOaWtoaWwgQmFkb2xhIDxuaWtoaWwuYmFk
b2xhQGZyZWVzY2FsZS5jb20+DQo+PiAtLS0NCj4+IAlDaGVja3BhdGNoIHdhcm5pbmdzIGhhbmRs
ZWQgYnkgY29tbWl0DQo+PiA2MWE4YzJjNmZlNzEwODJkZTNlYTg2Mjk1ODlkY2RkMGNjNWMzZjAy
DQo+DQo+VGhhdCBjaGVja3BhdGNoIHdhcm5pbmcgaXMga25vd24gdG8gaGF2ZSBmYWxzZSBwb3Np
dGl2ZXMgaW4gY2FzZXMgd2hlcmUgdGhlDQo+YmluZGluZyBzYXlzICI8Q0hJUE5BTUU+LWRldmlj
ZSIgb3IgImRldmljZS08VkVSU0lPTj4iLiAgSWYgeW91IHdhbnQgdG8NCj51cGRhdGUgdGhlIGJp
bmRpbmcgdG8gZ2l2ZSBhbiBleGFtcGxlIHdpdGggYSB2ZXJzaW9uLCB0aGF0J3MgZmluZSwgYnV0
DQo+Y2hlY2twYXRjaCBzaG91bGRuJ3QgYmUgd2h5LiAgV2UncmUgbm90IGdvaW5nIHRvIHVwZGF0
ZSB0aGUgYmluZGluZyBleGFtcGxlDQo+YWdhaW4gdG8gbWF0Y2ggYSBkaWZmZXJlbnQgdmVyc2lv
biB0aGUgbmV4dCB0aW1lIG9uZSBpcyBhZGRlZCB0byBhIGRldmljZSB0cmVlLi4uDQo+DQo+PiAJ
RG9jdW1lbnRhdGlvbjogZHRzOiBmc2wtdXNiOiBEb2N1bWVudCBVU0Igbm9kZSBjb21wYXRpYmxl
IHN0cmluZyBmb3INCj5JUCB2ZXJzaW9uDQo+Pg0KPj4gIGFyY2gvcG93ZXJwYy9ib290L2R0cy9m
c2wvdDIwODFzaS1wb3N0LmR0c2kgfCA0ICsrLS0NCj4+ICAxIGZpbGUgY2hhbmdlZCwgMiBpbnNl
cnRpb25zKCspLCAyIGRlbGV0aW9ucygtKQ0KPj4NCj4+IGRpZmYgLS1naXQgYS9hcmNoL3Bvd2Vy
cGMvYm9vdC9kdHMvZnNsL3QyMDgxc2ktcG9zdC5kdHNpDQo+PiBiL2FyY2gvcG93ZXJwYy9ib290
L2R0cy9mc2wvdDIwODFzaS1wb3N0LmR0c2kNCj4+IGluZGV4IDk3NDc5ZjAuLmFlY2VlOTYgMTAw
NjQ0DQo+PiAtLS0gYS9hcmNoL3Bvd2VycGMvYm9vdC9kdHMvZnNsL3QyMDgxc2ktcG9zdC5kdHNp
DQo+PiArKysgYi9hcmNoL3Bvd2VycGMvYm9vdC9kdHMvZnNsL3QyMDgxc2ktcG9zdC5kdHNpDQo+
PiBAQCAtNDEwLDcgKzQxMCw3IEBADQo+PiAgL2luY2x1ZGUvICJxb3JpcS1ncGlvLTMuZHRzaSIN
Cj4+ICAvaW5jbHVkZS8gInFvcmlxLXVzYjItbXBoLTAuZHRzaSINCj4+ICAJdXNiMDogdXNiQDIx
MDAwMCB7DQo+PiAtCQljb21wYXRpYmxlID0gImZzbC11c2IyLW1waC12Mi40IiwgImZzbC11c2Iy
LW1waCI7DQo+PiArCQljb21wYXRpYmxlID0gImZzbC11c2IyLW1waC12Mi41IiwgImZzbC11c2Iy
LW1waCI7DQo+DQo+VGhpcyBpcyBhbiBleGFtcGxlIG9mIHdoeSBpdCdzIGJldHRlciB0byByZWx5
IG9uIHZlcnNpb24gcmVnaXN0ZXJzIHdoZW4gcHJlc2VudC4NCj4NCj4tU2NvdHQNCj4NCkhpIHNj
b3R0LA0KDQpJIGNhbiBzZWUgdGhpcyBwYXRjaCBpbiAic3VwZXJzZWRlZCIgc3RhdGUgaW4gcGF0
Y2h3b3JrLiBBcyBwZXIgb3VyIGRpc2N1c3Npb24gdGhlIElQIHZlcnNpb24gY2hlY2tpbmcgaXMg
dG8gYmUgZG9uZSBieSBjb21wYXRpYmxlIHN0cmluZyBvbmx5LCBzbyBjYW4gSSBwcm9jZWVkIHRv
IHNlbmQgdGhpcyBwYXRjaCBhZ2Fpbj8NCg==
^ permalink raw reply
* [PATCH] rtc/tpo: Driver to support rtc and wakeup on PowerNV platform
From: Neelesh Gupta @ 2014-09-10 5:38 UTC (permalink / raw)
To: linuxppc-dev, rtc-linux; +Cc: a.zummo, tglx
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.
Test results:
-------------
Host:
[root@tul169p1 linus]# ls -l /sys/class/rtc/
total 0
lrwxrwxrwx 1 root root 0 Aug 28 03:21 rtc0 -> ../../devices/opal-rtc/rtc/rtc0
[root@tul169p1 linus]#
[root@tul169p1 linus]# cat /sys/devices/opal-rtc/rtc/rtc0/time
08:22:16
[root@tul169p1 linus]# cat /sys/devices/opal-rtc/rtc/rtc0/time
08:23:30
[root@tul169p1 linus]# echo `date '+%s' -d '+ 3 minutes'` > /sys/class/rtc/rtc0/wakealarm
[root@tul169p1 linus]# cat /sys/class/rtc/rtc0/wakealarm
1409214402
[root@tul169p1 linus]#
FSP:
$ smgr mfgState
standby
$ rtim timeofday
System time is valid: 2014/08/28 08:25:16.128924
$ smgr mfgState
ipling
$
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
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 4593a93..215c560 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -152,6 +152,8 @@ struct opal_sg_list {
#define OPAL_HANDLE_HMI 98
#define OPAL_REGISTER_DUMP_REGION 101
#define OPAL_UNREGISTER_DUMP_REGION 102
+#define OPAL_WRITE_TPO 103
+#define OPAL_READ_TPO 104
#ifndef __ASSEMBLY__
@@ -790,6 +792,9 @@ int64_t opal_rtc_read(__be32 *year_month_day,
__be64 *hour_minute_second_millisecond);
int64_t opal_rtc_write(uint32_t year_month_day,
uint64_t hour_minute_second_millisecond);
+int64_t opal_tpo_read(uint64_t token, __be32 *year_mon_day, __be32 *hour_min);
+int64_t opal_tpo_write(uint64_t token, uint32_t year_mon_day,
+ uint32_t hour_min);
int64_t opal_cec_power_down(uint64_t request);
int64_t opal_cec_reboot(void);
int64_t opal_read_nvram(uint64_t buffer, uint64_t size, uint64_t offset);
@@ -960,8 +965,6 @@ extern int opal_async_wait_response(uint64_t token, struct opal_msg *msg);
extern int opal_get_sensor_data(u32 sensor_hndl, u32 *sensor_data);
struct rtc_time;
-extern int opal_set_rtc_time(struct rtc_time *tm);
-extern void opal_get_rtc_time(struct rtc_time *tm);
extern unsigned long opal_get_boot_time(void);
extern void opal_nvram_init(void);
extern void opal_flash_init(void);
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 368ab37..149dc80 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -989,6 +989,7 @@ void GregorianDay(struct rtc_time * tm)
tm->tm_wday = day % 7;
}
+EXPORT_SYMBOL_GPL(GregorianDay);
void to_tm(int tim, struct rtc_time * tm)
{
diff --git a/arch/powerpc/platforms/powernv/opal-async.c b/arch/powerpc/platforms/powernv/opal-async.c
index e462ab9..693b6cd 100644
--- a/arch/powerpc/platforms/powernv/opal-async.c
+++ b/arch/powerpc/platforms/powernv/opal-async.c
@@ -71,6 +71,7 @@ int opal_async_get_token_interruptible(void)
return token;
}
+EXPORT_SYMBOL_GPL(opal_async_get_token_interruptible);
int __opal_async_release_token(int token)
{
@@ -102,6 +103,7 @@ int opal_async_release_token(int token)
return 0;
}
+EXPORT_SYMBOL_GPL(opal_async_release_token);
int opal_async_wait_response(uint64_t token, struct opal_msg *msg)
{
@@ -120,6 +122,7 @@ int opal_async_wait_response(uint64_t token, struct opal_msg *msg)
return 0;
}
+EXPORT_SYMBOL_GPL(opal_async_wait_response);
static int opal_async_comp_event(struct notifier_block *nb,
unsigned long msg_type, void *msg)
diff --git a/arch/powerpc/platforms/powernv/opal-rtc.c b/arch/powerpc/platforms/powernv/opal-rtc.c
index b1885db..a3c1b43 100644
--- a/arch/powerpc/platforms/powernv/opal-rtc.c
+++ b/arch/powerpc/platforms/powernv/opal-rtc.c
@@ -15,6 +15,8 @@
#include <linux/bcd.h>
#include <linux/rtc.h>
#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
#include <asm/opal.h>
#include <asm/firmware.h>
@@ -49,11 +51,9 @@ unsigned long __init opal_get_boot_time(void)
else
mdelay(10);
}
- if (rc != OPAL_SUCCESS) {
- ppc_md.get_rtc_time = NULL;
- ppc_md.set_rtc_time = NULL;
+ if (rc != OPAL_SUCCESS)
return 0;
- }
+
y_m_d = be32_to_cpu(__y_m_d);
h_m_s_ms = be64_to_cpu(__h_m_s_ms);
opal_to_tm(y_m_d, h_m_s_ms, &tm);
@@ -61,49 +61,24 @@ unsigned long __init opal_get_boot_time(void)
tm.tm_hour, tm.tm_min, tm.tm_sec);
}
-void opal_get_rtc_time(struct rtc_time *tm)
+static __init int opal_time_init(void)
{
- long rc = OPAL_BUSY;
- u32 y_m_d;
- u64 h_m_s_ms;
- __be32 __y_m_d;
- __be64 __h_m_s_ms;
+ struct platform_device *pdev;
+ struct device_node *rtc;
- while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
- rc = opal_rtc_read(&__y_m_d, &__h_m_s_ms);
- if (rc == OPAL_BUSY_EVENT)
- opal_poll_events(NULL);
+ rtc = of_find_node_by_path("/ibm,opal/rtc");
+ if (rtc) {
+ pdev = of_platform_device_create(rtc, "opal-rtc", NULL);
+ of_node_put(rtc);
+ } else {
+ if (opal_check_token(OPAL_RTC_READ) == OPAL_TOKEN_PRESENT ||
+ opal_check_token(OPAL_READ_TPO) == OPAL_TOKEN_PRESENT)
+ pdev = platform_device_register_simple("opal-rtc", -1,
+ NULL, 0);
else
- mdelay(10);
+ return -ENODEV;
}
- if (rc != OPAL_SUCCESS)
- return;
- y_m_d = be32_to_cpu(__y_m_d);
- h_m_s_ms = be64_to_cpu(__h_m_s_ms);
- opal_to_tm(y_m_d, h_m_s_ms, tm);
-}
-
-int opal_set_rtc_time(struct rtc_time *tm)
-{
- long rc = OPAL_BUSY;
- u32 y_m_d = 0;
- u64 h_m_s_ms = 0;
-
- y_m_d |= ((u32)bin2bcd((tm->tm_year + 1900) / 100)) << 24;
- y_m_d |= ((u32)bin2bcd((tm->tm_year + 1900) % 100)) << 16;
- y_m_d |= ((u32)bin2bcd((tm->tm_mon + 1))) << 8;
- y_m_d |= ((u32)bin2bcd(tm->tm_mday));
- h_m_s_ms |= ((u64)bin2bcd(tm->tm_hour)) << 56;
- h_m_s_ms |= ((u64)bin2bcd(tm->tm_min)) << 48;
- h_m_s_ms |= ((u64)bin2bcd(tm->tm_sec)) << 40;
-
- while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
- rc = opal_rtc_write(y_m_d, h_m_s_ms);
- if (rc == OPAL_BUSY_EVENT)
- opal_poll_events(NULL);
- else
- mdelay(10);
- }
- return rc == OPAL_SUCCESS ? 0 : -EIO;
+ return PTR_ERR_OR_ZERO(pdev);
}
+machine_subsys_initcall(powernv, opal_time_init);
diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
index 5718855..3dfe93e95 100644
--- a/arch/powerpc/platforms/powernv/opal-wrappers.S
+++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
@@ -248,3 +248,5 @@ OPAL_CALL(opal_set_param, OPAL_SET_PARAM);
OPAL_CALL(opal_handle_hmi, OPAL_HANDLE_HMI);
OPAL_CALL(opal_register_dump_region, OPAL_REGISTER_DUMP_REGION);
OPAL_CALL(opal_unregister_dump_region, OPAL_UNREGISTER_DUMP_REGION);
+OPAL_CALL(opal_tpo_write, OPAL_WRITE_TPO);
+OPAL_CALL(opal_tpo_read, OPAL_READ_TPO);
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index b44eec3..c87f6d9 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -784,3 +784,9 @@ void opal_free_sg_list(struct opal_sg_list *sg)
sg = NULL;
}
}
+
+EXPORT_SYMBOL_GPL(opal_poll_events);
+EXPORT_SYMBOL_GPL(opal_rtc_read);
+EXPORT_SYMBOL_GPL(opal_rtc_write);
+EXPORT_SYMBOL_GPL(opal_tpo_read);
+EXPORT_SYMBOL_GPL(opal_tpo_write);
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 5a0e2dc..8179c26 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -257,8 +257,6 @@ static unsigned long pnv_memory_block_size(void)
static void __init pnv_setup_machdep_opal(void)
{
ppc_md.get_boot_time = opal_get_boot_time;
- ppc_md.get_rtc_time = opal_get_rtc_time;
- ppc_md.set_rtc_time = opal_set_rtc_time;
ppc_md.restart = pnv_restart;
ppc_md.power_off = pnv_power_off;
ppc_md.halt = pnv_halt;
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index a168e96..4229e00 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -949,6 +949,17 @@ config RTC_DRV_NUC900
If you say yes here you get support for the RTC subsystem of the
NUC910/NUC920 used in embedded systems.
+config RTC_DRV_OPAL
+ tristate "IBM OPAL RTC driver"
+ depends on PPC_POWERNV
+ default y
+ help
+ If you say yes here you get support for the PowerNV platform RTC
+ driver based on OPAL interfaces.
+
+ This driver can also be built as a module. If so, the module
+ will be called rtc-opal.
+
comment "on-CPU RTC drivers"
config RTC_DRV_DAVINCI
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 56f061c..c4e7d1e 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -91,6 +91,7 @@ obj-$(CONFIG_RTC_DRV_MSM6242) += rtc-msm6242.o
obj-$(CONFIG_RTC_DRV_MPC5121) += rtc-mpc5121.o
obj-$(CONFIG_RTC_DRV_MV) += rtc-mv.o
obj-$(CONFIG_RTC_DRV_NUC900) += rtc-nuc900.o
+obj-$(CONFIG_RTC_DRV_OPAL) += rtc-opal.o
obj-$(CONFIG_RTC_DRV_OMAP) += rtc-omap.o
obj-$(CONFIG_RTC_DRV_PALMAS) += rtc-palmas.o
obj-$(CONFIG_RTC_DRV_PCAP) += rtc-pcap.o
diff --git a/drivers/rtc/rtc-opal.c b/drivers/rtc/rtc-opal.c
new file mode 100644
index 0000000..583db51
--- /dev/null
+++ b/drivers/rtc/rtc-opal.c
@@ -0,0 +1,257 @@
+/*
+ * IBM OPAL RTC driver
+ * Copyright (C) 2014 IBM
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.
+ */
+
+#define DRVNAME "rtc-opal"
+#define pr_fmt(fmt) DRVNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/rtc.h>
+#include <linux/delay.h>
+#include <linux/bcd.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <asm/opal.h>
+#include <asm/firmware.h>
+
+static void opal_to_tm(u32 y_m_d, u64 h_m_s_ms, struct rtc_time *tm)
+{
+ tm->tm_year = ((bcd2bin(y_m_d >> 24) * 100) +
+ bcd2bin((y_m_d >> 16) & 0xff)) - 1900;
+ tm->tm_mon = bcd2bin((y_m_d >> 8) & 0xff) - 1;
+ tm->tm_mday = bcd2bin(y_m_d & 0xff);
+ tm->tm_hour = bcd2bin((h_m_s_ms >> 56) & 0xff);
+ tm->tm_min = bcd2bin((h_m_s_ms >> 48) & 0xff);
+ tm->tm_sec = bcd2bin((h_m_s_ms >> 40) & 0xff);
+
+ GregorianDay(tm);
+}
+
+static void tm_to_opal(struct rtc_time *tm, u32 *y_m_d, u64 *h_m_s_ms)
+{
+ *y_m_d |= ((u32)bin2bcd((tm->tm_year + 1900) / 100)) << 24;
+ *y_m_d |= ((u32)bin2bcd((tm->tm_year + 1900) % 100)) << 16;
+ *y_m_d |= ((u32)bin2bcd((tm->tm_mon + 1))) << 8;
+ *y_m_d |= ((u32)bin2bcd(tm->tm_mday));
+
+ *h_m_s_ms |= ((u64)bin2bcd(tm->tm_hour)) << 56;
+ *h_m_s_ms |= ((u64)bin2bcd(tm->tm_min)) << 48;
+ *h_m_s_ms |= ((u64)bin2bcd(tm->tm_sec)) << 40;
+}
+
+static int opal_get_rtc_time(struct device *dev, struct rtc_time *tm)
+{
+ long rc = OPAL_BUSY;
+ u32 y_m_d;
+ u64 h_m_s_ms;
+ __be32 __y_m_d;
+ __be64 __h_m_s_ms;
+
+ while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
+ rc = opal_rtc_read(&__y_m_d, &__h_m_s_ms);
+ if (rc == OPAL_BUSY_EVENT)
+ opal_poll_events(NULL);
+ else
+ msleep(10);
+ }
+
+ if (rc != OPAL_SUCCESS)
+ return -EIO;
+
+ y_m_d = be32_to_cpu(__y_m_d);
+ h_m_s_ms = be64_to_cpu(__h_m_s_ms);
+ opal_to_tm(y_m_d, h_m_s_ms, tm);
+
+ return 0;
+}
+
+static int opal_set_rtc_time(struct device *dev, struct rtc_time *tm)
+{
+ long rc = OPAL_BUSY;
+ u32 y_m_d = 0;
+ u64 h_m_s_ms = 0;
+
+ tm_to_opal(tm, &y_m_d, &h_m_s_ms);
+ while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
+ rc = opal_rtc_write(y_m_d, h_m_s_ms);
+ if (rc == OPAL_BUSY_EVENT)
+ opal_poll_events(NULL);
+ else
+ msleep(10);
+ }
+
+ return rc == OPAL_SUCCESS ? 0 : -EIO;
+}
+
+/*
+ * TPO Timed Power-On
+ *
+ * TPO get/set OPAL calls care about the hour and min and to make it consistent
+ * with the rtc utility time conversion functions, we use the 'u64' to store
+ * its value and perform bit shift by 32 before use..
+ */
+static int opal_get_tpo_time(struct device *dev, struct rtc_wkalrm *alarm)
+{
+ __be32 __y_m_d, __h_m;
+ struct opal_msg msg;
+ int rc, token;
+ u64 h_m_s_ms;
+ u32 y_m_d;
+
+ token = opal_async_get_token_interruptible();
+ if (token < 0) {
+ if (token != -ERESTARTSYS)
+ pr_err("Failed to get the async token\n");
+
+ return token;
+ }
+
+ rc = opal_tpo_read(token, &__y_m_d, &__h_m);
+ if (rc != OPAL_ASYNC_COMPLETION) {
+ rc = -EIO;
+ goto exit;
+ }
+
+ rc = opal_async_wait_response(token, &msg);
+ if (rc) {
+ rc = -EIO;
+ goto exit;
+ }
+
+ rc = be64_to_cpu(msg.params[1]);
+ if (rc != OPAL_SUCCESS) {
+ rc = -EIO;
+ goto exit;
+ }
+
+ y_m_d = be32_to_cpu(__y_m_d);
+ h_m_s_ms = ((u64)be32_to_cpu(__h_m) << 32);
+ opal_to_tm(y_m_d, h_m_s_ms, &alarm->time);
+
+exit:
+ opal_async_release_token(token);
+ return rc;
+}
+
+/* Set Timed Power-On */
+static int opal_set_tpo_time(struct device *dev, struct rtc_wkalrm *alarm)
+{
+ u64 h_m_s_ms = 0, token;
+ struct opal_msg msg;
+ u32 y_m_d = 0;
+ int rc;
+
+ tm_to_opal(&alarm->time, &y_m_d, &h_m_s_ms);
+
+ token = opal_async_get_token_interruptible();
+ if (token < 0) {
+ if (token != -ERESTARTSYS)
+ pr_err("Failed to get the async token\n");
+
+ return token;
+ }
+
+ /* TPO, we care about hour and minute */
+ rc = opal_tpo_write(token, y_m_d,
+ (u32)((h_m_s_ms >> 32) & 0xffff0000));
+ if (rc != OPAL_ASYNC_COMPLETION) {
+ rc = -EIO;
+ goto exit;
+ }
+
+ rc = opal_async_wait_response(token, &msg);
+ if (rc) {
+ rc = -EIO;
+ goto exit;
+ }
+
+ rc = be64_to_cpu(msg.params[1]);
+ if (rc != OPAL_SUCCESS)
+ rc = -EIO;
+
+exit:
+ opal_async_release_token(token);
+ return rc;
+}
+
+static const struct rtc_class_ops opal_rtc_ops = {
+ .read_time = opal_get_rtc_time,
+ .set_time = opal_set_rtc_time,
+ .read_alarm = opal_get_tpo_time,
+ .set_alarm = opal_set_tpo_time,
+};
+
+static int opal_rtc_probe(struct platform_device *pdev)
+{
+ struct rtc_device *rtc;
+
+ if (pdev->dev.of_node && of_get_property(pdev->dev.of_node, "has-tpo",
+ NULL))
+ device_set_wakeup_capable(&pdev->dev, true);
+
+ rtc = devm_rtc_device_register(&pdev->dev, DRVNAME, &opal_rtc_ops,
+ THIS_MODULE);
+
+ return PTR_ERR_OR_ZERO(rtc);
+}
+
+static const struct of_device_id opal_rtc_match[] = {
+ {
+ .compatible = "ibm,opal-rtc",
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(of, opal_rtc_match);
+
+static const struct platform_device_id opal_rtc_driver_ids[] = {
+ {
+ .name = "opal-rtc",
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, opal_rtc_driver_ids);
+
+static struct platform_driver opal_rtc_driver = {
+ .probe = opal_rtc_probe,
+ .id_table = opal_rtc_driver_ids,
+ .driver = {
+ .name = DRVNAME,
+ .owner = THIS_MODULE,
+ .of_match_table = opal_rtc_match,
+ },
+};
+
+static int __init opal_rtc_init(void)
+{
+ if (!firmware_has_feature(FW_FEATURE_OPAL))
+ return -ENODEV;
+
+ return platform_driver_register(&opal_rtc_driver);
+}
+
+static void __exit opal_rtc_exit(void)
+{
+ platform_driver_unregister(&opal_rtc_driver);
+}
+
+MODULE_AUTHOR("Neelesh Gupta <neelegup@linux.vnet.ibm.com>");
+MODULE_DESCRIPTION("IBM OPAL RTC driver");
+MODULE_LICENSE("GPL");
+
+module_init(opal_rtc_init);
+module_exit(opal_rtc_exit);
^ permalink raw reply related
* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Markus Pargmann @ 2014-09-10 6:21 UTC (permalink / raw)
To: Nicolin Chen
Cc: Shengjiu Wang, alsa-devel, lgirdwood, tiwai, linux-kernel,
broonie, timur, perex, Li.Xiubo, linuxppc-dev
In-Reply-To: <20140909183804.GA6944@Asurada>
[-- Attachment #1: Type: text/plain, Size: 1982 bytes --]
On Tue, Sep 09, 2014 at 11:38:05AM -0700, Nicolin Chen wrote:
> On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote:
> > @@ -1321,7 +1333,11 @@ static int fsl_ssi_probe(struct platform_device *pdev)
> > return -ENOMEM;
> > }
> >
> > - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > + if (ssi_private->soc->imx)
> > + ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev,
> > + "ipg", iomem, &fsl_ssi_regconfig);
> > + else
> > + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
>
> As Markus mentioned, the key point here is to be compatible with those
> non-clock-name platforms.
>
> I think it would be safer to keep the current code while adding an extra
> clk_disable_unprepare() at the end of probe() as a common routine. And
> meantime, make sure to have the call for imx only because it seems that
> the other platforms do not depend on the clock. //a bit guessing here :)
>
> Then we can get a patch like:
> open() {
> + clk_prepare_enable();
> ....
> }
>
> close() {
> ....
> + clk_disable_unprepare()
> }
>
> probe() {
> clk_get();
> clk_prepare_enable();
> ....
> if (xxx)
> - goto err_xx;
> + return ret;
> ....
> + clk_disable_unprepare();
> return 0;
> -err_xx:
> - clk_disable_unprepare()
> }
>
> remove() {
> ....
> - clk_disable_unprepare()
> }
If I remember correctly, there may be AC97 communication with the codec
before any substream is created. That's why we enable the SSI unit right
at the beginning for AC97 in fsl_ssi_setup_reg_vals(). So we need to
check for AC97 before disabling clocks.
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: [alsa-devel] [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Nicolin Chen @ 2014-09-10 6:42 UTC (permalink / raw)
To: Markus Pargmann
Cc: Shengjiu Wang, timur, alsa-devel, tiwai, Li.Xiubo, linux-kernel,
lgirdwood, broonie, linuxppc-dev
In-Reply-To: <20140910062118.GA26348@pengutronix.de>
On Wed, Sep 10, 2014 at 08:21:18AM +0200, Markus Pargmann wrote:
> On Tue, Sep 09, 2014 at 11:38:05AM -0700, Nicolin Chen wrote:
> > On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote:
> > > @@ -1321,7 +1333,11 @@ static int fsl_ssi_probe(struct platform_device *pdev)
> > > return -ENOMEM;
> > > }
> > >
> > > - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > > + if (ssi_private->soc->imx)
> > > + ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev,
> > > + "ipg", iomem, &fsl_ssi_regconfig);
> > > + else
> > > + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> >
> > As Markus mentioned, the key point here is to be compatible with those
> > non-clock-name platforms.
> >
> > I think it would be safer to keep the current code while adding an extra
> > clk_disable_unprepare() at the end of probe() as a common routine. And
> > meantime, make sure to have the call for imx only because it seems that
> > the other platforms do not depend on the clock. //a bit guessing here :)
> >
> > Then we can get a patch like:
> > open() {
> > + clk_prepare_enable();
> > ....
> > }
> >
> > close() {
> > ....
> > + clk_disable_unprepare()
> > }
> >
> > probe() {
> > clk_get();
> > clk_prepare_enable();
> > ....
> > if (xxx)
> > - goto err_xx;
> > + return ret;
> > ....
> > + clk_disable_unprepare();
> > return 0;
> > -err_xx:
> > - clk_disable_unprepare()
> > }
> >
> > remove() {
> > ....
> > - clk_disable_unprepare()
> > }
>
> If I remember correctly, there may be AC97 communication with the codec
> before any substream is created. That's why we enable the SSI unit right
> at the beginning for AC97 in fsl_ssi_setup_reg_vals(). So we need to
> check for AC97 before disabling clocks.
Thank you for the input. That's the exact part I couldn't be sure.
And I agree that adding a check for AC97 will be safer while this
may keeps itself removable if being unnecessary.
Thanks
Nicolin
^ permalink raw reply
* Re: [PATCH v2] cpufreq: powernv: Set the cpus to nominal frequency during reboot/kexec
From: shilpa @ 2014-09-10 6:57 UTC (permalink / raw)
To: David Laight, Viresh Kumar
Cc: Preeti U Murthy, linuxppc-dev@lists.ozlabs.org, Rafael J. Wysocki,
Linux Kernel Mailing List, linux-pm@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174849AD@AcuExch.aculab.com>
On 09/01/2014 02:42 PM, David Laight wrote:
>> Yes unlikely() should cover the whole if statement...
>
> Actually it probably shouldn't.
> You need to look at the generated code with each different set of 'unlikely()'
> to see how gcc processes them.
> In this case, if 'rebooting' is false you want to 'fall through' on a statically
> predicted 'not taken' branch. You don't ever care about the second clause.
> With an 'unlikely' covering the entire statement gcc could easily add a
> forwards conditional branch (that will be mis-predicted) for the 'rebooting' test.
>
> (Yes, I spent a lot of time getting gcc to generate branches that were
> correctly statically predicted for some code where every cycle mattered.)
>
> David
>
Hi David,
The objdup with an 'unlikely()' covering the entire if statement is as follows:
if (unlikely(rebooting && new_index != get_nominal_index()))
return -EBUSY;
1ac: 2f 89 00 00 cmpwi cr7,r9,0 /* compare rebooting,0 */
1b0: 40 de 00 4c bne- cr7,1fc <.powernv_cpufreq_target_index+0x7c>
The '-' in the instruction bne- specifies an unlikely branch. So gcc has
processed the first clause to be identified as an unlikely branch i.e,
branch to <1fc> (to test the second clause) is unlikely on 'rebooting' not
equal to 0.
1b4: 1f ff 00 0c mulli r31,r31,12
.
. <--- Set the frequency and return --->
.
.
1fc: 3d 22 00 00 addis r9,r2,0 /* test the second clause */
200: 3d 02 00 00 addis r8,r2,0
204: 81 49 00 00 lwz r10,0(r9)
208: 81 28 00 00 lwz r9,0(r8)
20c: 7d 29 50 50 subf r9,r9,r10
210: 7f 89 f8 00 cmpw cr7,r9,r31 /* compare new_index,nominal_index */
214: 41 9e ff a0 beq+ cr7,1b4 <.powernv_cpufreq_target_index+0x34>
The '+' in the instruction beq+ specifies a likely branch. The second clause
unlikely(new_index != get_nominal_index()) is processed to
likely(new_index == get_nominal_index()).
218: 38 60 ff f0 li r3,-16 /* return -EBUSY */
21c: 4b ff ff cc b 1e8 <.powernv_cpufreq_target_index+0x68>
So unlikely() covering the entire statement will not lead to a branch mis-prediction
for the 'rebooting' test. Having unlikely to cover both 'rebooting' and the second
clause we can avoid the branch miss prediction for the second clause. This is
advantageous for the code path powernv_cpufreq_target_index(policy,nominal_index)
which will be invoked by the reboot_notifier.
Thanks and Regards,
Shilpa
^ permalink raw reply
* Re: [PATCH v2 1/3] init/main.c: Give init_task a canary
From: Chuck Ebbert @ 2014-09-10 7:26 UTC (permalink / raw)
To: Aaron Tomlin
Cc: dzickus, jcastillo, riel, x86, akpm, peterz, bmr, prarit, oleg,
rostedt, linux-kernel, minchan, mingo, aneesh.kumar, akpm, hannes,
jgh, linuxppc-dev, tglx, pzijlstr
In-Reply-To: <1410255749-2956-2-git-send-email-atomlin@redhat.com>
On Tue, 9 Sep 2014 10:42:27 +0100
Aaron Tomlin <atomlin@redhat.com> wrote:
> +void task_stack_end_magic(struct task_struct *tsk)
> +{
> + unsigned long *stackend;
> +
> + stackend = end_of_stack(tsk);
> + *stackend = STACK_END_MAGIC; /* for overflow detection */
> +}
> +
For clarity this should probably be called set_task_stack_end_magic().
And has this been tested on parisc and metag, which use STACK_GROWSUP ?
I can't see how end_of_stack() as it's defined now could work on those archs.
^ permalink raw reply
* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Shengjiu Wang @ 2014-09-10 8:12 UTC (permalink / raw)
To: Nicolin Chen
Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, timur, perex, broonie,
mpa, linuxppc-dev, linux-kernel
In-Reply-To: <20140909183804.GA6944@Asurada>
On Tue, Sep 09, 2014 at 11:38:05AM -0700, Nicolin Chen wrote:
> On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote:
> > @@ -1321,7 +1333,11 @@ static int fsl_ssi_probe(struct platform_device *pdev)
> > return -ENOMEM;
> > }
> >
> > - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > + if (ssi_private->soc->imx)
> > + ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev,
> > + "ipg", iomem, &fsl_ssi_regconfig);
> > + else
> > + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
>
> As Markus mentioned, the key point here is to be compatible with those
> non-clock-name platforms.
>
> I think it would be safer to keep the current code while adding an extra
> clk_disable_unprepare() at the end of probe() as a common routine. And
> meantime, make sure to have the call for imx only because it seems that
> the other platforms do not depend on the clock. //a bit guessing here :)
>
> Then we can get a patch like:
> open() {
> + clk_prepare_enable();
> ....
> }
>
> close() {
> ....
> + clk_disable_unprepare()
> }
what is the open() and close()? do you mean the fsl_ssi_startup()
and fsl_ssi_shutdown()?
>
> probe() {
> clk_get();
> clk_prepare_enable();
> ....
> if (xxx)
> - goto err_xx;
> + return ret;
> ....
> + clk_disable_unprepare();
> return 0;
> -err_xx:
> - clk_disable_unprepare()
> }
If this probe() is fsl_ssi_imx_probe(), I think no need to add
clk_prepare_enable() or clk_disable_unprepare(), seems there is no
registers accessing in this probe.
>
> remove() {
> ....
> - clk_disable_unprepare()
> }
>
> As long as you make the subject clear as 'Don't enable core/ipg clock
> when SSI's idle', I'm sure you can make them within a single patch.
>
> And an alternative way for open() and close() is to put those code into
> pm_runtime_resume/suspend() instead (since we might have some internal
> code need to be added by using pm_runtime as well), which would make
> the further code neater IMO.
>
> Thank you
> Nicolin
^ permalink raw reply
* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Shengjiu Wang @ 2014-09-10 10:01 UTC (permalink / raw)
To: Nicolin Chen
Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, Timur Tabi, perex,
broonie, mpa, linuxppc-dev, linux-kernel
In-Reply-To: <20140909195928.GA5224@Asurada>
On Tue, Sep 09, 2014 at 12:59:29PM -0700, Nicolin Chen wrote:
> On Tue, Sep 09, 2014 at 02:37:42PM -0500, Timur Tabi wrote:
> > On 09/09/2014 01:38 PM, Nicolin Chen wrote:
> > >make sure to have the call for imx only because it seems that
> > >the other platforms do not depend on the clock.
> >
> > Although I doubt anyone will every add support for clocks to PowerPC "side"
> > of this driver, I would prefer to avoid IMX-specific changes. Instead, the
> > code should check if a clock is available. That's why I suggested this
> > change:
> >
> > - if (ssi_private->soc->imx)
> > + if (!IS_ERR(ssi_private->clk))
>
> Hmm.... I think the following change may be better?
>
> probe() {
> ....
> + /*
> + * Initially mark the clock to NULL for all platforms so that later
> + * clk_prepare_enable() will ignore and return 0 for non-clock cases.
> + */
> + ssi_private->clk = NULL;
> .....
> fsl_ssi_imx_probe();
> }
ssi_private is initialized to zero in beginning of probe. I think no need to
add this change here.
wang shengjiu
>
> In this way, all platforms, not confined to imx any more, will be able
> to call clk_prepare_enable(). Then we don't need an extra platform check
> before calling it.
^ permalink raw reply
* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Shengjiu Wang @ 2014-09-10 10:30 UTC (permalink / raw)
To: Markus Pargmann
Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, timur, perex,
Nicolin Chen, broonie, linuxppc-dev, linux-kernel
In-Reply-To: <20140910062118.GA26348@pengutronix.de>
On Wed, Sep 10, 2014 at 08:21:18AM +0200, Markus Pargmann wrote:
> On Tue, Sep 09, 2014 at 11:38:05AM -0700, Nicolin Chen wrote:
> > On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote:
> > > @@ -1321,7 +1333,11 @@ static int fsl_ssi_probe(struct platform_device *pdev)
> > > return -ENOMEM;
> > > }
> > >
> > > - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > > + if (ssi_private->soc->imx)
> > > + ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev,
> > > + "ipg", iomem, &fsl_ssi_regconfig);
> > > + else
> > > + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> >
> > As Markus mentioned, the key point here is to be compatible with those
> > non-clock-name platforms.
> >
> > I think it would be safer to keep the current code while adding an extra
> > clk_disable_unprepare() at the end of probe() as a common routine. And
> > meantime, make sure to have the call for imx only because it seems that
> > the other platforms do not depend on the clock. //a bit guessing here :)
> >
> > Then we can get a patch like:
> > open() {
> > + clk_prepare_enable();
> > ....
> > }
> >
> > close() {
> > ....
> > + clk_disable_unprepare()
> > }
> >
> > probe() {
> > clk_get();
> > clk_prepare_enable();
> > ....
> > if (xxx)
> > - goto err_xx;
> > + return ret;
> > ....
> > + clk_disable_unprepare();
> > return 0;
> > -err_xx:
> > - clk_disable_unprepare()
> > }
> >
> > remove() {
> > ....
> > - clk_disable_unprepare()
> > }
>
> If I remember correctly, there may be AC97 communication with the codec
> before any substream is created. That's why we enable the SSI unit right
> at the beginning for AC97 in fsl_ssi_setup_reg_vals(). So we need to
> check for AC97 before disabling clocks.
>
> Best regards,
>
> Markus
hi Markus
I think if clk_prepare_enable() in startup(), and clk_disable_unprepare()
in shutdown can meet this requirement, right?
done:
if (ssi_private->dai_fmt)
_fsl_ssi_set_dai_fmt(ssi_private, ssi_private->dai_fmt);
I find that in end of probe, there is setting of dai_fmt. Can we remove this?
because this setting need to enable ipg clock, and if ac97, ipg clock can't be
disabled.
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 V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Markus Pargmann @ 2014-09-10 10:53 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, timur, perex,
Nicolin Chen, broonie, linuxppc-dev, linux-kernel
In-Reply-To: <20140910103005.GB17326@audiosh1>
[-- Attachment #1: Type: text/plain, Size: 3104 bytes --]
Hi,
On Wed, Sep 10, 2014 at 06:30:06PM +0800, Shengjiu Wang wrote:
> On Wed, Sep 10, 2014 at 08:21:18AM +0200, Markus Pargmann wrote:
> > On Tue, Sep 09, 2014 at 11:38:05AM -0700, Nicolin Chen wrote:
> > > On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote:
> > > > @@ -1321,7 +1333,11 @@ static int fsl_ssi_probe(struct platform_device *pdev)
> > > > return -ENOMEM;
> > > > }
> > > >
> > > > - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > > > + if (ssi_private->soc->imx)
> > > > + ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev,
> > > > + "ipg", iomem, &fsl_ssi_regconfig);
> > > > + else
> > > > + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > >
> > > As Markus mentioned, the key point here is to be compatible with those
> > > non-clock-name platforms.
> > >
> > > I think it would be safer to keep the current code while adding an extra
> > > clk_disable_unprepare() at the end of probe() as a common routine. And
> > > meantime, make sure to have the call for imx only because it seems that
> > > the other platforms do not depend on the clock. //a bit guessing here :)
> > >
> > > Then we can get a patch like:
> > > open() {
> > > + clk_prepare_enable();
> > > ....
> > > }
> > >
> > > close() {
> > > ....
> > > + clk_disable_unprepare()
> > > }
> > >
> > > probe() {
> > > clk_get();
> > > clk_prepare_enable();
> > > ....
> > > if (xxx)
> > > - goto err_xx;
> > > + return ret;
> > > ....
> > > + clk_disable_unprepare();
> > > return 0;
> > > -err_xx:
> > > - clk_disable_unprepare()
> > > }
> > >
> > > remove() {
> > > ....
> > > - clk_disable_unprepare()
> > > }
> >
> > If I remember correctly, there may be AC97 communication with the codec
> > before any substream is created. That's why we enable the SSI unit right
> > at the beginning for AC97 in fsl_ssi_setup_reg_vals(). So we need to
> > check for AC97 before disabling clocks.
> >
> > Best regards,
> >
> > Markus
>
> hi Markus
>
> I think if clk_prepare_enable() in startup(), and clk_disable_unprepare()
> in shutdown can meet this requirement, right?
Yes that could work.
>
> done:
> if (ssi_private->dai_fmt)
> _fsl_ssi_set_dai_fmt(ssi_private, ssi_private->dai_fmt);
>
> I find that in end of probe, there is setting of dai_fmt. Can we remove this?
> because this setting need to enable ipg clock, and if ac97, ipg clock can't be
> disabled.
No you can't remove it. It is necessary for the DT property "fsl,mode".
Most dts do not have this property anymore because the sound cards are
setting the dai-fmt. But there are still some powerpc dts files that
contain that property.
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
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