* Re: [PATCH v2] powerpc/powernv: Framework to log critical errors on powernv.
From: Michael Ellerman @ 2013-12-18 5:27 UTC (permalink / raw)
To: Deepthi Dharwar; +Cc: PowerPC email list
In-Reply-To: <52B13019.4010601@linux.vnet.ibm.com>
On Wed, 2013-12-18 at 10:48 +0530, Deepthi Dharwar wrote:
> Hi Micheal,
>
> Thanks for the review.
No worries.
> On 12/18/2013 08:13 AM, Michael Ellerman wrote:
> > On Mon, 2013-12-16 at 18:00 +0530, Deepthi Dharwar wrote:
> >> +/* All the information regarding an error/event to be reported
> >> + * needs to populate this structure using pre-defined interfaces
> >> + * only
> >> + */
> >> +struct opal_errorlog {
> >> +
> >> + uint16_t component_id;
> >> + uint8_t error_events_type:3;
> >
> > Bit field?
> >
> >> + uint8_t subsystem_id;
> >> +
> >> + uint8_t event_sev;
> >> + uint8_t event_subtype;
> >> + uint8_t usr_scn_count; /* User section count */
> >
> > user_section_count;
> >
> >> + uint8_t elog_origin;
> >> +
> >> + uint32_t usr_scn_size; /* User section size */
> >
> > user_section_size;
> >
> >> + uint32_t reason_code;
> >> + uint32_t additional_info[4];
> >> +
> >> + char usr_data_dump[OPAL_LOG_MAX_DUMP];
> >> +};
> >
> > It looks like this goes straight to Opal, should we be using __packed ?
>
> Yes, this goes straight into Opal. The structure is defined such that
> it is packed by default, this will not require compiler to pack bytes.
Sure, but the compiler might decide to lay it out differently for some reason.
You should use __packed.
Also bitfields are essentially a big "implementation defined behaviour" sign,
so I would avoid using the bitfield.
> >> diff --git a/arch/powerpc/platforms/powernv/opal-elog.c b/arch/powerpc/platforms/powernv/opal-elog.c
> >> index 58849d0..ade1e58 100644
> >> --- a/arch/powerpc/platforms/powernv/opal-elog.c
> >> +++ b/arch/powerpc/platforms/powernv/opal-elog.c
> >> @@ -22,7 +23,9 @@
> >> /* Maximum size of a single log on FSP is 16KB */
> >> #define OPAL_MAX_ERRLOG_SIZE 16384
> >>
> >> -/* maximu number of records powernv can hold */
> >> +#define USR_CHAR_ARRAY_FIXED_SIZE 4
> >
> > What is this?
>
> struct User data section is mapped to a buffer. As all the structures
> are padded, we need to subtract the same to do data manipulation.
> Make me need to re-word it or use __packed here.
Yeah that's still not really clear to me, so if you can do something that is
more obvious that would be good.
> >> @@ -272,6 +275,61 @@ static int init_err_log_buffer(void)
> >> return 0;
> >> }
> >>
> >> +/* Interface to be used by POWERNV to push the logs to FSP via Sapphire */
> >> +struct opal_errorlog *elog_create(uint8_t err_evt_type, uint16_t component_id,
> >> + uint8_t subsystem_id, uint8_t event_sev, uint8_t event_subtype,
> >> + uint32_t reason_code, uint32_t info0, uint32_t info1,
> >> + uint32_t info2, uint32_t info3)
> >
> >
> > A call to this function is going to be just a giant list of integer values, it
> > will not be easy to see at a glance which value goes in which field.
> >
> > I think you'd be better off with an elog_alloc() routine, and then you just do
> > the initialisation explicitly so that it's obvious which value goes where:
> >
> > elog->error_events_type = FOO;
> > elog->component_id = BAR;
> > elog->subsystem_id = ETC;
> >
>
> elog_create() will be called by all sub-systems on POWERNV platform to
> log events and errors. I feel we are better off passing all the required
> arguments to the interface than initialize explicitly.
> This would have a cleaner interface to error logging by
> 1) Removing huge amount of code duplication ( Each and every error/event
> to be reported needs to initialise fields of the opal_errorlog struct
> done many many times on POWERNV, results in redundant code )
It will be more lines of code, but it might be more readable code.
> 2) There are chances of missing out on initialising key fields if
> done by the user. Having an interface mandates the fields that
> needs to populated while logging error/events.
I can always pass 0 :)
We will see how it looks once there are some callers.
> >> +void commit_errorlog_to_fsp(struct opal_errorlog *buf)
> >> +{
> >> + opal_commit_log_to_fsp((void *)(vmalloc_to_pfn(buf) << PAGE_SHIFT));
> >
> > Can't fail?
>
> It is better to have a return here, atleast for the caller to know if
> opal handling of the same is successful or not. I will make the required
> change.
>
> >> + kfree(buf);
> >
> > It's a bit rude to free buf when the caller still has a pointer to it.
>
> Technically, after the error log has been committed, the user is not
> supposed to re-use or do anything with that buffer. I need to add
> checks in all my routines if(buf != NULL), to handle the case where
> the user by mistake is trying to use the same buffer pointer.
Why is the user not supposed to re-use it?
kfree()'ing the buffer doesn't prevent the caller from re-using it.
cheers
^ permalink raw reply
* Re: [PATCH v2] powerpc/powernv: Framework to log critical errors on powernv.
From: Deepthi Dharwar @ 2013-12-18 6:21 UTC (permalink / raw)
To: Michael Ellerman; +Cc: PowerPC email list
In-Reply-To: <1387344458.20735.2.camel@concordia>
On 12/18/2013 10:57 AM, Michael Ellerman wrote:
> On Wed, 2013-12-18 at 10:48 +0530, Deepthi Dharwar wrote:
>> Hi Micheal,
>>
>> Thanks for the review.
>
> No worries.
>
>> On 12/18/2013 08:13 AM, Michael Ellerman wrote:
>>> On Mon, 2013-12-16 at 18:00 +0530, Deepthi Dharwar wrote:
>>>> +/* All the information regarding an error/event to be reported
>>>> + * needs to populate this structure using pre-defined interfaces
>>>> + * only
>>>> + */
>>>> +struct opal_errorlog {
>>>> +
>>>> + uint16_t component_id;
>>>> + uint8_t error_events_type:3;
>>>
>>> Bit field?
>>>
>>>> + uint8_t subsystem_id;
>>>> +
>>>> + uint8_t event_sev;
>>>> + uint8_t event_subtype;
>>>> + uint8_t usr_scn_count; /* User section count */
>>>
>>> user_section_count;
>>>
>>>> + uint8_t elog_origin;
>>>> +
>>>> + uint32_t usr_scn_size; /* User section size */
>>>
>>> user_section_size;
>>>
>>>> + uint32_t reason_code;
>>>> + uint32_t additional_info[4];
>>>> +
>>>> + char usr_data_dump[OPAL_LOG_MAX_DUMP];
>>>> +};
>>
>>> It looks like this goes straight to Opal, should we be using __packed ?
>>
>> Yes, this goes straight into Opal. The structure is defined such that
>> it is packed by default, this will not require compiler to pack bytes.
>
> Sure, but the compiler might decide to lay it out differently for some reason.
> You should use __packed.
Ok.
> Also bitfields are essentially a big "implementation defined behaviour" sign,
> so I would avoid using the bitfield.
Yes, bitfields will be gone.
>>>> diff --git a/arch/powerpc/platforms/powernv/opal-elog.c b/arch/powerpc/platforms/powernv/opal-elog.c
>>>> index 58849d0..ade1e58 100644
>>>> --- a/arch/powerpc/platforms/powernv/opal-elog.c
>>>> +++ b/arch/powerpc/platforms/powernv/opal-elog.c
>>>> @@ -22,7 +23,9 @@
>>>> /* Maximum size of a single log on FSP is 16KB */
>>>> #define OPAL_MAX_ERRLOG_SIZE 16384
>>>>
>>>> -/* maximu number of records powernv can hold */
>>>> +#define USR_CHAR_ARRAY_FIXED_SIZE 4
>>>
>>> What is this?
>>
>> struct User data section is mapped to a buffer. As all the structures
>> are padded, we need to subtract the same to do data manipulation.
>> Make me need to re-word it or use __packed here.
>
> Yeah that's still not really clear to me, so if you can do something that is
> more obvious that would be good.
Sure.
>>>> @@ -272,6 +275,61 @@ static int init_err_log_buffer(void)
>>>> return 0;
>>>> }
>>>>
>>>> +/* Interface to be used by POWERNV to push the logs to FSP via Sapphire */
>>>> +struct opal_errorlog *elog_create(uint8_t err_evt_type, uint16_t component_id,
>>>> + uint8_t subsystem_id, uint8_t event_sev, uint8_t event_subtype,
>>>> + uint32_t reason_code, uint32_t info0, uint32_t info1,
>>>> + uint32_t info2, uint32_t info3)
>>>
>>>
>>> A call to this function is going to be just a giant list of integer values, it
>>> will not be easy to see at a glance which value goes in which field.
>>>
>>> I think you'd be better off with an elog_alloc() routine, and then you just do
>>> the initialisation explicitly so that it's obvious which value goes where:
>>>
>>> elog->error_events_type = FOO;
>>> elog->component_id = BAR;
>>> elog->subsystem_id = ETC;
>>>
>>
>> elog_create() will be called by all sub-systems on POWERNV platform to
>> log events and errors. I feel we are better off passing all the required
>> arguments to the interface than initialize explicitly.
>> This would have a cleaner interface to error logging by
>> 1) Removing huge amount of code duplication ( Each and every error/event
>> to be reported needs to initialise fields of the opal_errorlog struct
>> done many many times on POWERNV, results in redundant code )
>
> It will be more lines of code, but it might be more readable code.
>
>> 2) There are chances of missing out on initialising key fields if
>> done by the user. Having an interface mandates the fields that
>> needs to populated while logging error/events.
>
> I can always pass 0 :)
I was referring to more on the lines of missing unintentionally :)
> We will see how it looks once there are some callers.
Sure. I will retain it for now. Going forward once we start adding
users exploiting this interface, we can then take a call.
>
>>>> +void commit_errorlog_to_fsp(struct opal_errorlog *buf)
>>>> +{
>>>> + opal_commit_log_to_fsp((void *)(vmalloc_to_pfn(buf) << PAGE_SHIFT));
>>>
>>> Can't fail?
>>
>> It is better to have a return here, atleast for the caller to know if
>> opal handling of the same is successful or not. I will make the required
>> change.
>>
>>>> + kfree(buf);
>>>
>>> It's a bit rude to free buf when the caller still has a pointer to it.
>>
>> Technically, after the error log has been committed, the user is not
>> supposed to re-use or do anything with that buffer. I need to add
>> checks in all my routines if(buf != NULL), to handle the case where
>> the user by mistake is trying to use the same buffer pointer.
>
> Why is the user not supposed to re-use it?
>
> kfree()'ing the buffer doesn't prevent the caller from re-using it.
Release the memory. Assign pointer to NULL before returning.
All the error logging interfaces should have NULL check (to return).
User can't do much in that case.
Regards,
Deepthi
> cheers
>
>
>
>
^ permalink raw reply
* Re: [PATCH] powerpc: book3s: kvm: Don't abuse host r2 in exit path
From: Aneesh Kumar K.V @ 2013-12-18 7:35 UTC (permalink / raw)
To: agraf, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1384178387-22993-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
Hi Alex,
Any update on this ? We need this to got into 3.13.
-aneesh
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> We don't use PACATOC for PR. Avoid updating HOST_R2 with PR
> KVM mode when both HV and PR are enabled in the kernel. Without this we
> get the below crash
>
> (qemu)
> Unable to handle kernel paging request for data at address 0xffffffffffff8310
> Faulting instruction address: 0xc00000000001d5a4
> cpu 0x2: Vector: 300 (Data Access) at [c0000001dc53aef0]
> pc: c00000000001d5a4: .vtime_delta.isra.1+0x34/0x1d0
> lr: c00000000001d760: .vtime_account_system+0x20/0x60
> sp: c0000001dc53b170
> msr: 8000000000009032
> dar: ffffffffffff8310
> dsisr: 40000000
> current = 0xc0000001d76c62d0
> paca = 0xc00000000fef1100 softe: 0 irq_happened: 0x01
> pid = 4472, comm = qemu-system-ppc
> enter ? for help
> [c0000001dc53b200] c00000000001d760 .vtime_account_system+0x20/0x60
> [c0000001dc53b290] c00000000008d050 .kvmppc_handle_exit_pr+0x60/0xa50
> [c0000001dc53b340] c00000000008f51c kvm_start_lightweight+0xb4/0xc4
> [c0000001dc53b510] c00000000008cdf0 .kvmppc_vcpu_run_pr+0x150/0x2e0
> [c0000001dc53b9e0] c00000000008341c .kvmppc_vcpu_run+0x2c/0x40
> [c0000001dc53ba50] c000000000080af4 .kvm_arch_vcpu_ioctl_run+0x54/0x1b0
> [c0000001dc53bae0] c00000000007b4c8 .kvm_vcpu_ioctl+0x478/0x730
> [c0000001dc53bca0] c0000000002140cc .do_vfs_ioctl+0x4ac/0x770
> [c0000001dc53bd80] c0000000002143e8 .SyS_ioctl+0x58/0xb0
> [c0000001dc53be30] c000000000009e58 syscall_exit+0x0/0x98
> --- Exception: c00 (System Call) at 00001fffff960160
> SP (1ffffecbe3c0) is in userspace
>
> These changes were originally part of
> http://mid.gmane.org/20130806042205.GR19254@iris.ozlabs.ibm.com
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/kvm_book3s_asm.h | 1 +
> arch/powerpc/kernel/asm-offsets.c | 1 +
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 7 +++----
> 3 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s_asm.h b/arch/powerpc/include/asm/kvm_book3s_asm.h
> index 0bd9348..69fe837 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_asm.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_asm.h
> @@ -79,6 +79,7 @@ struct kvmppc_host_state {
> ulong vmhandler;
> ulong scratch0;
> ulong scratch1;
> + ulong scratch2;
> u8 in_guest;
> u8 restore_hid5;
> u8 napping;
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index 8e6ede6..841a4c8 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -583,6 +583,7 @@ int main(void)
> HSTATE_FIELD(HSTATE_VMHANDLER, vmhandler);
> HSTATE_FIELD(HSTATE_SCRATCH0, scratch0);
> HSTATE_FIELD(HSTATE_SCRATCH1, scratch1);
> + HSTATE_FIELD(HSTATE_SCRATCH2, scratch2);
> HSTATE_FIELD(HSTATE_IN_GUEST, in_guest);
> HSTATE_FIELD(HSTATE_RESTORE_HID5, restore_hid5);
> HSTATE_FIELD(HSTATE_NAPPING, napping);
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 339aa5e..16f7654 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -750,15 +750,14 @@ kvmppc_interrupt_hv:
> * guest CR, R12 saved in shadow VCPU SCRATCH1/0
> * guest R13 saved in SPRN_SCRATCH0
> */
> - /* abuse host_r2 as third scratch area; we get r2 from PACATOC(r13) */
> - std r9, HSTATE_HOST_R2(r13)
> + std r9, HSTATE_SCRATCH2(r13)
>
> lbz r9, HSTATE_IN_GUEST(r13)
> cmpwi r9, KVM_GUEST_MODE_HOST_HV
> beq kvmppc_bad_host_intr
> #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
> cmpwi r9, KVM_GUEST_MODE_GUEST
> - ld r9, HSTATE_HOST_R2(r13)
> + ld r9, HSTATE_SCRATCH2(r13)
> beq kvmppc_interrupt_pr
> #endif
> /* We're now back in the host but in guest MMU context */
> @@ -778,7 +777,7 @@ kvmppc_interrupt_hv:
> std r6, VCPU_GPR(R6)(r9)
> std r7, VCPU_GPR(R7)(r9)
> std r8, VCPU_GPR(R8)(r9)
> - ld r0, HSTATE_HOST_R2(r13)
> + ld r0, HSTATE_SCRATCH2(r13)
> std r0, VCPU_GPR(R9)(r9)
> std r10, VCPU_GPR(R10)(r9)
> std r11, VCPU_GPR(R11)(r9)
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/2] powerpc/p1022ds: fix rtc compatible string
From: Dongsheng Wang @ 2013-12-18 8:39 UTC (permalink / raw)
To: scottwood, Chang-Ming.Huang, roy.zang; +Cc: linuxppc-dev, Wang Dongsheng
From: Wang Dongsheng <dongsheng.wang@freescale.com>
RTC Hardware(ds3232) and rtc compatible string does not match.
Change "dallas,ds1339" to "dallas,ds3232".
Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
diff --git a/arch/powerpc/boot/dts/p1022ds.dtsi b/arch/powerpc/boot/dts/p1022ds.dtsi
index 873da35..5725058 100644
--- a/arch/powerpc/boot/dts/p1022ds.dtsi
+++ b/arch/powerpc/boot/dts/p1022ds.dtsi
@@ -146,7 +146,7 @@
*/
};
rtc@68 {
- compatible = "dallas,ds1339";
+ compatible = "dallas,ds3232";
reg = <0x68>;
};
adt7461@4c {
--
1.8.5
^ permalink raw reply related
* [PATCH 2/2] powerpc/p1022ds: add a interrupt for rtc node
From: Dongsheng Wang @ 2013-12-18 8:39 UTC (permalink / raw)
To: scottwood, Chang-Ming.Huang, roy.zang; +Cc: linuxppc-dev, Wang Dongsheng
In-Reply-To: <1387355964-39486-1-git-send-email-dongsheng.wang@freescale.com>
From: Wang Dongsheng <dongsheng.wang@freescale.com>
Add an external interrupt for rtc node.
Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
diff --git a/arch/powerpc/boot/dts/p1022ds.dtsi b/arch/powerpc/boot/dts/p1022ds.dtsi
index 5725058..957e0dc 100644
--- a/arch/powerpc/boot/dts/p1022ds.dtsi
+++ b/arch/powerpc/boot/dts/p1022ds.dtsi
@@ -148,6 +148,7 @@
rtc@68 {
compatible = "dallas,ds3232";
reg = <0x68>;
+ interrupts = <0x1 0x1 0 0>;
};
adt7461@4c {
compatible = "adi,adt7461";
--
1.8.5
^ permalink raw reply related
* Re: [PATCH] powerpc: Make 64-bit non-VMX __copy_tofrom_user bi-endian
From: Anton Blanchard @ 2013-12-18 10:15 UTC (permalink / raw)
To: benh, paulus, paulmck; +Cc: linuxppc-dev
In-Reply-To: <20131218092957.4a12cbcf@kryten>
Hi,
> [ This is a rare but nasty LE issue. Most of the time we use the
> POWER7 optimised __copy_tofrom_user_power7 loop, but when it hits an
> exception we fall back to the base __copy_tofrom_user loop. - Anton ]
To try and catch any screw ups in our ppc64 memcpy and copy_tofrom_user
loops, I wrote a quick test:
http://ozlabs.org/~anton/junkcode/validate_kernel_copyloops.tar.gz
"make check" runs through all source and destination alignments for a
range of sizes. It verifies the data was copied correctly and the
redzone before and after were untouched.
It tests:
copyuser_64
copyuser_power7
memcpy_64
memcpy_power7
memcpy_64 is currently unused on LE, but I added Paul McKenney's LE
fixes regardless. copyuser_64 has the same LE fix (posted yesterday).
All loops pass the test on both LE and BE.
Anton
^ permalink raw reply
* Re: [PATCH] powerpc: book3s: kvm: Don't abuse host r2 in exit path
From: Alexander Graf @ 2013-12-18 10:30 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Paul Mackerras, linuxppc-dev, kvm-ppc,
kvm@vger.kernel.org mailing list
In-Reply-To: <87lhzilign.fsf@linux.vnet.ibm.com>
On 18.12.2013, at 08:35, Aneesh Kumar K.V =
<aneesh.kumar@linux.vnet.ibm.com> wrote:
>=20
> Hi Alex,
>=20
> Any update on this ? We need this to got into 3.13.
Thanks, applied to for-3.13.
Alex
^ permalink raw reply
* Re: [PATCH v2 2/6] PCI/MSI: Factor out pci_get_msi_cap() interface
From: Bjorn Helgaas @ 2013-12-18 18:26 UTC (permalink / raw)
To: Mark Lord
Cc: Joerg Roedel, x86@kernel.org, linux-kernel@vger.kernel.org,
Michael Ellerman, linux-ide@vger.kernel.org, Alexander Gordeev,
Jan Beulich, linux-pci@vger.kernel.org, Tejun Heo, linuxppc-dev,
Ingo Molnar
In-Reply-To: <52442975.9000603@start.ca>
On Thu, Sep 26, 2013 at 08:32:53AM -0400, Mark Lord wrote:
> On 13-09-18 05:48 AM, Alexander Gordeev wrote:
> >
> > The last pattern makes most of sense to me and could be updated with a more
> > clear sequence - a call to (bit modified) pci_msix_table_size() followed
> > by a call to pci_enable_msix(). I think this pattern can effectively
> > supersede the currently recommended "loop" practice.
>
> The loop is still necessary, because there's a race between those two calls,
> so that pci_enable_msix() can still fail due to lack of MSIX slots.
Hi Mark,
Can you elaborate on this race? My understanding is that
pci_msix_table_size() depends only on the "Table Size" field in the MSI-X
Message Control register.
So if there's a concurrency problem here, it would have to be something
like "pci_enable_msix() may not be able to configure the requested number
of vectors because it has to allocate from a shared pool."
If that's the case, pci_msix_table_size() wouldn't be involved at all, and
the only question is how to coordinate between several drivers that each
call pci_enable_msix(). I think that would have to be resolved in some
arch hook used by the PCI core.
Maybe this is already taken care of; I just want to make sure we don't
overlook an issue here.
Bjorn
^ permalink raw reply
* Re: commit e38c0a1f breaks powerpc boards with uli1575 chip
From: Rob Herring @ 2013-12-18 18:40 UTC (permalink / raw)
To: Nikita Yushchenko, Arnd Bergmann, Thierry Reding, Grant Likely,
devicetree@vger.kernel.org, Kumar Gala
Cc: Alexey Lugovskoy, linuxppc-dev, linux-kernel, Dmitry Krivoschekov
In-Reply-To: <201312171135.38576@blacky.localdomain>
[fixed DT maillist address]
On 12/17/2013 01:35 AM, Nikita Yushchenko wrote:
> Hi
>
> While trying to make freescale p2020ds and mpc8572ds boards working with mainline kernel, I faced that commit
> e38c0a1f (Handle #address-cells > 2 specially) breaks things with these boards.
Good to see this broke in v3.7 and is just now found...
>
> Both these boards have uli1575 chip.
> Corresponding part in device tree is something like
>
> uli1575@0 {
> reg = <0x0 0x0 0x0 0x0 0x0>;
> #size-cells = <2>;
> #address-cells = <3>;
> ranges = <0x2000000 0x0 0x80000000
> 0x2000000 0x0 0x80000000
> 0x0 0x20000000
>
> 0x1000000 0x0 0x0
> 0x1000000 0x0 0x0
> 0x0 0x10000>;
> isa@1e {
> ...
>
> I.e. it has #address-cells = <3>
>
>
> With commit e38c0a1f reverted, devices under uli1575 are registered correctly, e.g. for rtc
>
> OF: ** translation for device /pcie@ffe09000/pcie@0/uli1575@0/isa@1e/rtc@70 **
> OF: bus is isa (na=2, ns=1) on /pcie@ffe09000/pcie@0/uli1575@0/isa@1e
> OF: translating address: 00000001 00000070
> OF: parent bus is default (na=3, ns=2) on /pcie@ffe09000/pcie@0/uli1575@0
> OF: walking ranges...
> OF: ISA map, cp=0, s=1000, da=70
> OF: parent translation for: 01000000 00000000 00000000
> OF: with offset: 70
> OF: one level translation: 00000000 00000000 00000070
> OF: parent bus is pci (na=3, ns=2) on /pcie@ffe09000/pcie@0
> OF: walking ranges...
> OF: default map, cp=a0000000, s=20000000, da=70
> OF: default map, cp=0, s=10000, da=70
> OF: parent translation for: 01000000 00000000 00000000
> OF: with offset: 70
> OF: one level translation: 01000000 00000000 00000070
> OF: parent bus is pci (na=3, ns=2) on /pcie@ffe09000
> OF: walking ranges...
> OF: PCI map, cp=0, s=10000, da=70
> OF: parent translation for: 01000000 00000000 00000000
> OF: with offset: 70
> OF: one level translation: 01000000 00000000 00000070
> OF: parent bus is default (na=2, ns=2) on /
> OF: walking ranges...
> OF: PCI map, cp=0, s=10000, da=70
> OF: parent translation for: 00000000 ffc10000
> OF: with offset: 70
> OF: one level translation: 00000000 ffc10070
> OF: reached root node
>
> With commit e38c0a1f in place, address translation fails:
>
> OF: ** translation for device /pcie@ffe09000/pcie@0/uli1575@0/isa@1e/rtc@70 **
> OF: bus is isa (na=2, ns=1) on /pcie@ffe09000/pcie@0/uli1575@0/isa@1e
> OF: translating address: 00000001 00000070
> OF: parent bus is default (na=3, ns=2) on /pcie@ffe09000/pcie@0/uli1575@0
> OF: walking ranges...
> OF: ISA map, cp=0, s=1000, da=70
> OF: parent translation for: 01000000 00000000 00000000
> OF: with offset: 70
> OF: one level translation: 00000000 00000000 00000070
> OF: parent bus is pci (na=3, ns=2) on /pcie@ffe09000/pcie@0
> OF: walking ranges...
> OF: default map, cp=a0000000, s=20000000, da=70
> OF: default map, cp=0, s=10000, da=70
> OF: not found !
>
> Either e38c0a1f should be reverted, or uli1575 (and perhaps other similar devices) have to be described in device
> trees differently.
Reverting would break Tegra PCIe, but you should not have to change the
DT either. So we need a solution.
Is this something like this sufficient to fix it?
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 4b9317b..378aebd 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -74,7 +74,7 @@ static u64 of_bus_default_map(__be32 *addr, const
__be32 *range,
* mapping doesn't specify a physical address. Rather, the address
* specifies an identifier that must match exactly.
*/
- if (na > 2 && memcmp(range, addr, na * 4) != 0)
+ if (na > 2 && memcmp(range, addr, (na - 2) * 4) != 0)
return OF_BAD_ADDR;
if (da < cp || da >= (cp + s))
^ permalink raw reply related
* Re: [RFC][PATCH v1] ASoC: fsl_ssi: Add DAI master mode support for SSI on i.MX series
From: Mark Brown @ 2013-12-18 18:59 UTC (permalink / raw)
To: Nicolin Chen; +Cc: alsa-devel, lgirdwood, tiwai, timur, perex, linuxppc-dev
In-Reply-To: <1386845085-21682-1-git-send-email-Guangyu.Chen@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 681 bytes --]
On Thu, Dec 12, 2013 at 06:44:45PM +0800, Nicolin Chen wrote:
> +/**
> + * fsl_ssi_set_dai_tdm_slot - set TDM slot number
> + *
> + * Note: This function can be only called when using SSI as DAI master
> + */
> +static int fsl_ssi_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai, u32 tx_mask,
> + u32 rx_mask, int slots, int slot_width)
> +{
> + struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai);
> + struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
> + u32 val;
I'm a bit concernred about what this is for and why it's required - is
it something that machine drivers have to call and if it is shouldn't
the driver be defaulting to a sensible configuration?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH] rapidio: add modular rapidio core build into powerpc and mips branches
From: Alexandre Bounine @ 2013-12-18 18:57 UTC (permalink / raw)
To: Andrew Morton, linux-kernel, linuxppc-dev
Cc: linux-mips, Ralf Baechle, Alexandre Bounine, Jean Delvare
Allow modular build option for RapidIO subsystem core in MIPS and PowerPC
architectural branches.
At this moment modular RapidIO subsystem build is enabled only for platforms
that use PCI/PCIe based RapidIO controllers (e.g. Tsi721).
Signed-off-by: Alexandre Bounine <alexandre.bounine@idt.com>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Jean Delvare <jdelvare@suse.de>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Li Yang <leoli@freescale.com>
Cc: linux-mips@linux-mips.org
---
arch/mips/Kconfig | 2 +-
arch/powerpc/Kconfig | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 650de39..e6a8a7a 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2442,7 +2442,7 @@ source "drivers/pcmcia/Kconfig"
source "drivers/pci/hotplug/Kconfig"
config RAPIDIO
- bool "RapidIO support"
+ tristate "RapidIO support"
depends on PCI
default n
help
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index b44b52c..992531f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -790,7 +790,7 @@ config HAS_RAPIDIO
default n
config RAPIDIO
- bool "RapidIO support"
+ tristate "RapidIO support"
depends on HAS_RAPIDIO || PCI
help
If you say Y here, the kernel will include drivers and
@@ -798,7 +798,7 @@ config RAPIDIO
config FSL_RIO
bool "Freescale Embedded SRIO Controller support"
- depends on RAPIDIO && HAS_RAPIDIO
+ depends on RAPIDIO = y && HAS_RAPIDIO
default "n"
---help---
Include support for RapidIO controller on Freescale embedded
--
1.7.8.4
^ permalink raw reply related
* Re: [PATCH v1 0/4] powerpc/512x: update COMMON_CLK support for MPC5125
From: Anatolij Gustschin @ 2013-12-18 19:53 UTC (permalink / raw)
To: Gerhard Sittig
Cc: Mike Turquette, Detlev Zundel, Matteo Facchinetti, Scott Wood,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <1386681097-14126-1-git-send-email-gsi@denx.de>
On Tue, 10 Dec 2013 14:11:33 +0100
Gerhard Sittig <gsi@denx.de> wrote:
...
> Gerhard Sittig (4):
> powerpc/512x: clk: minor comment updates
> powerpc/512x: clk: enforce even SDHC divider values
> powerpc/512x: clk: support MPC5121/5123/5125 SoC variants
> powerpc/512x: dts: add MPC5125 clock specs
>
> arch/powerpc/boot/dts/mpc5125twr.dts | 53 +++-
> arch/powerpc/include/asm/mpc5121.h | 7 +-
> arch/powerpc/platforms/512x/clock-commonclk.c | 369 +++++++++++++++++++++----
> include/dt-bindings/clock/mpc512x-clock.h | 9 +-
> 4 files changed, 386 insertions(+), 52 deletions(-)
Applied this series to mpc5xxx next. Thanks!
Anatolij
^ permalink raw reply
* Re: [PATCH v1 1/1] powerpc/512x: dts: remove misplaced IRQ spec from 'soc' node (5125)
From: Anatolij Gustschin @ 2013-12-18 19:54 UTC (permalink / raw)
To: Gerhard Sittig; +Cc: devicetree, linuxppc-dev
In-Reply-To: <1386669068-2477-1-git-send-email-gsi@denx.de>
On Tue, 10 Dec 2013 10:51:08 +0100
Gerhard Sittig <gsi@denx.de> wrote:
> the 'soc' node in the MPC5125 "tower" board .dts has an '#interrupt-cells'
> property although this node is not an interrupt controller
>
> remove this erroneously placed property because starting with v3.13-rc1
> lookup and resolution of 'interrupts' specs for peripherals gets misled
> (tries to use the 'soc' as the interrupt parent which fails), emits
> 'no irq domain found' WARN() messages and breaks the boot process
>
> [ best viewed with 'git diff -U5' to have DT node names in the context ]
>
> Cc: Anatolij Gustschin <agust@denx.de>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Gerhard Sittig <gsi@denx.de>
>
> ---
>
> note that this is not a resend of the previous MPC5121 fix, but instead
> is a fix for MPC5125 along the same lines of the MPC5121 fix
> ---
> arch/powerpc/boot/dts/mpc5125twr.dts | 1 -
> 1 file changed, 1 deletion(-)
Applied. Thanks!
Anatolij
^ permalink raw reply
* Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values
From: Alexander Graf @ 2013-12-18 21:44 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Paul Mackerras, linuxppc-dev, kvm-ppc,
kvm@vger.kernel.org mailing list
In-Reply-To: <1384178577-23721-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On 11.11.2013, at 15:02, Aneesh Kumar K.V =
<aneesh.kumar@linux.vnet.ibm.com> wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>=20
> Don't try to compute these values.
>=20
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>=20
> NOTE: I am not sure why we were originally computing dsisr and dar. So =
may be
> we need a variant of this patch. But with this and the additional =
patch
> "powerpc: book3s: PR: Enable Little Endian PR guest" I am able to get =
a Little Endian
> PR guest to boot.
It's quite easy to find out - git blame tells you all the history and =
points you to commit ca7f4203b.
commit ca7f4203b9b66e12d0d9968ff7dfe781f3a9695a
Author: Alexander Graf <agraf@suse.de>
Date: Wed Mar 24 21:48:28 2010 +0100
KVM: PPC: Implement alignment interrupt
Mac OS X has some applications - namely the Finder - that require =
alignment
interrupts to work properly. So we need to implement them.
But the spec for 970 and 750 also looks different. While 750 =
requires the
DSISR and DAR fields to reflect some instruction bits (DSISR) and =
the fault
address (DAR), the 970 declares this as an optional feature. So we =
need
to reconstruct DSISR and DAR manually.
Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Avi Kivity <avi@redhat.com>
Read this as "on 970, alignment interrupts don't give us DSISR and DAR =
of the faulting instruction" as otherwise I wouldn't have implemented =
it.
So this is clearly a nack on this patch :).
Alex
^ permalink raw reply
* RE: [PATCH v2 00/10] Kconfig: cleanup SERIO_I8042 dependencies
From: Luck, Tony @ 2013-12-18 21:59 UTC (permalink / raw)
To: Mark Salter, linux-kernel@vger.kernel.org
Cc: Yu, Fenghua, x86@kernel.org, Russell King,
linux-mips@linux-mips.org, linux-sh@vger.kernel.org,
Dmitry Torokhov, Ingo Molnar, Paul Mundt, Paul Mackerras,
linux-alpha@vger.kernel.org, sparclinux@vger.kernel.org,
linux-ia64@vger.kernel.org, Thomas Gleixner, Guan Xuetao,
linuxppc-dev@lists.ozlabs.org, David S. Miller,
linux-arm-kernel@lists.infradead.org, Richard Henderson
In-Reply-To: <1387295333-24684-1-git-send-email-msalter@redhat.com>
> This is v2 of the patch series. Changes from version 1:
>
> o Added acks. arm, ia64, and sh are only ones without acks.
ia64 bits look OK
Acked-by: Tony Luck <tony.luck@intel.com>
^ permalink raw reply
* Re: [PATCH v1 0/4] powerpc/512x: update COMMON_CLK support for MPC5125
From: Mike Turquette @ 2013-12-18 22:20 UTC (permalink / raw)
To: Gerhard Sittig, linuxppc-dev, linux-arm-kernel,
Anatolij Gustschin, Matteo Facchinetti
Cc: Scott Wood, Gerhard Sittig, Detlev Zundel
In-Reply-To: <1386681097-14126-1-git-send-email-gsi@denx.de>
Quoting Gerhard Sittig (2013-12-10 05:11:33)
> this series improves the previously introduced common clock support for
> MPC512x such that SoC variants 5123 and 5125 get addressed appropriately
> (MPC5125 turned out to be rather different from MPC5121 than I perceived
> before -- there is much more than "just two FECs and no MBX")
> =
> thus this series depends on "add COMMON_CLK support for PowerPC MPC512x"
> (v6 sent in <1385851897-23475-1-git-send-email-gsi@denx.de>, applicable
> on top of v3.13-rc1 or later, currently applied to mpc5xxx -next,
> available at git://git.denx.de/linux-2.6-agust.git next)
> =
> this series does not address the issue of outdated or missing device
> tree binding documentation for MPC512x peripherals -- that's the scope
> of a pending separate series
For the three clock patches:
Acked-by: Mike Turquette <mturquette@linaro.org>
> =
> v1 initial submission (2013-12-10)
> - enforce an even divider value for SDHC (on all MPC512x variants)
> - tell 5121/5123/5125 SoC variants apart and only register the
> appropriate set of clock items (i.e. refuse to access unused and
> reserved bits, and support those components which are only found on
> MPC5125)
> - update the MPC5125 "tower" board DTS (although the code still works in
> the absence of device tree clock specs)
> =
> the series passes 'checkpatch.pl --strict' except for two warnings which
> cannot get fixed because <linux/clk-provider.h> dictates the data type
> and "fixing" the warning would break the build
> =
> WARNING: static const char * array should probably be static const char=
* const
> #256: FILE: arch/powerpc/platforms/512x/clock-commonclk.c:500:
> +static const char *parent_names_mux0_spdif[] =3D {
> =
> WARNING: static const char * array should probably be static const char=
* const
> #260: FILE: arch/powerpc/platforms/512x/clock-commonclk.c:504:
> +static const char *parent_names_mux0_canin[] =3D {
> =
> total: 0 errors, 2 warnings, 0 checks, 495 lines checked
> =
> the series was build-tested, and was run-tested on the MPC5121 ADS board
> =
> Matteo, can you verify the crystal frequency in the DTS update, please?
> And that v3.13-rc kernels with v6 of the COMMON_CLK introduction for
> MPC512x plus this series for MPC5125 operate your peripherals, both with
> an updated device tree as well as with a former device tree that lacks
> clock specs? Thank you! Setting CONFIG_COMMON_CLK_DEBUG=3Dy in your
> .config and eyeballing /sys/kernel/debug/clk/clk_summary will help you.
> =
> Gerhard Sittig (4):
> powerpc/512x: clk: minor comment updates
> powerpc/512x: clk: enforce even SDHC divider values
> powerpc/512x: clk: support MPC5121/5123/5125 SoC variants
> powerpc/512x: dts: add MPC5125 clock specs
> =
> arch/powerpc/boot/dts/mpc5125twr.dts | 53 +++-
> arch/powerpc/include/asm/mpc5121.h | 7 +-
> arch/powerpc/platforms/512x/clock-commonclk.c | 369 +++++++++++++++++++=
++----
> include/dt-bindings/clock/mpc512x-clock.h | 9 +-
> 4 files changed, 386 insertions(+), 52 deletions(-)
> =
> -- =
> 1.7.10.4
>=20
^ permalink raw reply
* Re: [v3, 3/7] powerpc: enable the relocatable support for the fsl booke 32bit kernel
From: Scott Wood @ 2013-12-18 23:48 UTC (permalink / raw)
To: Kevin Hao; +Cc: linuxppc
In-Reply-To: <1375838315-27797-4-git-send-email-haokexin@gmail.com>
On Wed, Aug 07, 2013 at 09:18:31AM +0800, Kevin Hao wrote:
> This is based on the codes in the head_44x.S. The difference is that
> the init tlb size we used is 64M. With this patch we can only load the
> kernel at address between memstart_addr ~ memstart_addr + 64M. We will
> fix this restriction in the following patches.
Which following patch fixes the restriction? With all seven patches
applied, I was still only successful booting within 64M of memstart_addr.
-Scott
^ permalink raw reply
* Re: [2/2,v8] powerpc/fsl-booke: Add initial T104x_QDS board support
From: Scott Wood @ 2013-12-19 0:50 UTC (permalink / raw)
To: Prabhakar Kushwaha; +Cc: Poonam Aggrwal, linuxppc-dev, Priyanka Jain
In-Reply-To: <1380771889-2626-1-git-send-email-prabhakar@freescale.com>
On Thu, Oct 03, 2013 at 09:14:49AM +0530, Prabhakar Kushwaha wrote:
> diff --git a/arch/powerpc/boot/dts/t104xqds.dtsi b/arch/powerpc/boot/dts/t104xqds.dtsi
> new file mode 100644
> index 0000000..5a518b3
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/t104xqds.dtsi
[snip]
> +/ {
> + model = "fsl,T1040QDS";
> + compatible = "fsl,T1040QDS";
Remove model/compatible; they're supplied by the includer of this file.
> + nand@2,0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "fsl,ifc-nand";
> + reg = <0x2 0x0 0x10000>;
> +
> + partition@0 {
> + /* This location must not be altered */
> + /* 1MB for u-boot Bootloader Image */
> + reg = <0x0 0x00100000>;
> + label = "NAND U-Boot Image";
> + read-only;
> + };
> +
> + partition@100000 {
> + /* 1MB for DTB Image */
> + reg = <0x00100000 0x00100000>;
> + label = "NAND DTB Image";
> + };
> +
> + partition@200000 {
> + /* 10MB for Linux Kernel Image */
> + reg = <0x00200000 0x00A00000>;
> + label = "NAND Linux Kernel Image";
> + };
> +
> + partition@C00000 {
> + /* 500MB for Root file System Image */
> + reg = <0x00c00000 0x1F400000>;
> + label = "NAND RFS Image";
> + };
> + };
I'd really like to see us stop putting partition info in the dts files.
Alternatives are using the mtdparts command line option, or having U-Boot
use the mtdparts environment variable to add partition info to the dtb
(there's already code for this).
> + board-control@3,0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "fsl,tetra-fpga", "fsl,fpga-qixis";
> + reg = <3 0 0x300>;
> + };
Could you explain the relationship between "tetra" and "qixis"?
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values
From: Paul Mackerras @ 2013-12-19 2:26 UTC (permalink / raw)
To: Alexander Graf
Cc: linuxppc-dev, Aneesh Kumar K.V, kvm-ppc,
kvm@vger.kernel.org mailing list
In-Reply-To: <0AB88010-F2B7-44A1-8FA9-2A40079706BB@suse.de>
On Wed, Dec 18, 2013 at 10:44:08PM +0100, Alexander Graf wrote:
>
> On 11.11.2013, at 15:02, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
> > From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> >
> > Don't try to compute these values.
> >
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > ---
> >
> > NOTE: I am not sure why we were originally computing dsisr and dar. So may be
> > we need a variant of this patch. But with this and the additional patch
> > "powerpc: book3s: PR: Enable Little Endian PR guest" I am able to get a Little Endian
> > PR guest to boot.
>
> It's quite easy to find out - git blame tells you all the history and points you to commit ca7f4203b.
>
> commit ca7f4203b9b66e12d0d9968ff7dfe781f3a9695a
> Author: Alexander Graf <agraf@suse.de>
> Date: Wed Mar 24 21:48:28 2010 +0100
>
> KVM: PPC: Implement alignment interrupt
>
> Mac OS X has some applications - namely the Finder - that require alignment
> interrupts to work properly. So we need to implement them.
>
> But the spec for 970 and 750 also looks different. While 750 requires the
> DSISR and DAR fields to reflect some instruction bits (DSISR) and the fault
> address (DAR), the 970 declares this as an optional feature. So we need
> to reconstruct DSISR and DAR manually.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Avi Kivity <avi@redhat.com>
>
> Read this as "on 970, alignment interrupts don't give us DSISR and DAR of the faulting instruction" as otherwise I wouldn't have implemented it.
Although it's optional, all IBM POWER cpus, and as far as I know all
PowerPC cpus, set DAR on an alignment interrupt to the effective
address being accessed. You have a valid point regarding DSISR, but
it would be nice to skip the computations where either the host CPU
provides the bits, or the virtual CPU doesn't.
Paul.
^ permalink raw reply
* Re: [RFC][PATCH v1] ASoC: fsl_ssi: Add DAI master mode support for SSI on i.MX series
From: Nicolin Chen @ 2013-12-19 2:14 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel, lgirdwood, tiwai, timur, perex, linuxppc-dev
In-Reply-To: <20131218185952.GM31886@sirena.org.uk>
On Wed, Dec 18, 2013 at 06:59:52PM +0000, Mark Brown wrote:
> On Thu, Dec 12, 2013 at 06:44:45PM +0800, Nicolin Chen wrote:
>
> > +/**
> > + * fsl_ssi_set_dai_tdm_slot - set TDM slot number
> > + *
> > + * Note: This function can be only called when using SSI as DAI master
> > + */
> > +static int fsl_ssi_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai, u32 tx_mask,
> > + u32 rx_mask, int slots, int slot_width)
> > +{
> > + struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai);
> > + struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
> > + u32 val;
>
> I'm a bit concernred about what this is for and why it's required - is
> it something that machine drivers have to call and if it is shouldn't
> the driver be defaulting to a sensible configuration?
SSI can control how many slots to generate and which slot to send data. Yes,
the normal case, which should be defaulting to normal two slots I2S case, can
be configured by SSI driver itself as you mentioned. I'll add it to startup().
Then only those machine drivers using multiple slots (>2) need to call it.
Thank you for the comments.
Nicolin Chen
^ permalink raw reply
* Re: commit e38c0a1f breaks powerpc boards with uli1575 chip
From: Nikita Yushchenko @ 2013-12-19 4:42 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree@vger.kernel.org, Arnd Bergmann, Dmitry Krivoschekov,
Alexey Lugovskoy, Thierry Reding, linux-kernel, Grant Likely,
linuxppc-dev
In-Reply-To: <52B1EC15.5070606@gmail.com>
> Reverting would break Tegra PCIe, but you should not have to change the
> DT either. So we need a solution.
>
> Is this something like this sufficient to fix it?
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 4b9317b..378aebd 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -74,7 +74,7 @@ static u64 of_bus_default_map(__be32 *addr, const
> __be32 *range,
> * mapping doesn't specify a physical address. Rather, the
> address * specifies an identifier that must match exactly.
> */
> - if (na > 2 && memcmp(range, addr, na * 4) != 0)
> + if (na > 2 && memcmp(range, addr, (na - 2) * 4) != 0)
> return OF_BAD_ADDR;
>
> if (da < cp || da >= (cp + s))
No, this does not help.
I've dumped the actual content of 'range' and 'addr' at the failure point
(i.e. ar point that returns error with e38c0a1f but passes without
e38c0a1f ):
OF: default map, cp=0, s=10000, da=70
range: 01 00 00 00 00 00 00 00 00 00 00 00
addr: 00 00 00 00 00 00 00 00 00 00 00 70
Nikita
^ permalink raw reply
* Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values
From: Aneesh Kumar K.V @ 2013-12-19 7:02 UTC (permalink / raw)
To: Alexander Graf
Cc: Paul Mackerras, linuxppc-dev, kvm-ppc,
kvm@vger.kernel.org mailing list
In-Reply-To: <0AB88010-F2B7-44A1-8FA9-2A40079706BB@suse.de>
Alexander Graf <agraf@suse.de> writes:
> On 11.11.2013, at 15:02, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>
>> Don't try to compute these values.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>>
>> NOTE: I am not sure why we were originally computing dsisr and dar. So may be
>> we need a variant of this patch. But with this and the additional patch
>> "powerpc: book3s: PR: Enable Little Endian PR guest" I am able to get a Little Endian
>> PR guest to boot.
>
> It's quite easy to find out - git blame tells you all the history and points you to commit ca7f4203b.
>
> commit ca7f4203b9b66e12d0d9968ff7dfe781f3a9695a
> Author: Alexander Graf <agraf@suse.de>
> Date: Wed Mar 24 21:48:28 2010 +0100
>
> KVM: PPC: Implement alignment interrupt
>
> Mac OS X has some applications - namely the Finder - that require alignment
> interrupts to work properly. So we need to implement them.
>
> But the spec for 970 and 750 also looks different. While 750 requires the
> DSISR and DAR fields to reflect some instruction bits (DSISR) and the fault
> address (DAR), the 970 declares this as an optional feature. So we need
> to reconstruct DSISR and DAR manually.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Avi Kivity <avi@redhat.com>
>
> Read this as "on 970, alignment interrupts don't give us DSISR and DAR of the faulting instruction" as otherwise I wouldn't have implemented it.
>
> So this is clearly a nack on this patch :).
I can possibly do a if (cpu_has_feature(CPU_FTR_ARCH_201)). But do we need
to do that ? According to Paul we should always find DAR.
-aneesh
^ permalink raw reply
* [PATCH] powerpc/512x: dts: disable MPC5125 usb module
From: Matteo Facchinetti @ 2013-12-19 10:23 UTC (permalink / raw)
To: linuxppc-dev; +Cc: gsi, agust
USB controller pin-muxing is not initialized correctly and when system boot,
causes a kernel panic.
USB controller is also connected with a USB3320 ulpi tranciever and
DTS should be includes the correct dependency for initialize and activate
this component.
Signed-off-by: Matteo Facchinetti <matteo.facchinetti@sirius-es.it>
---
arch/powerpc/boot/dts/mpc5125twr.dts | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/boot/dts/mpc5125twr.dts b/arch/powerpc/boot/dts/mpc5125twr.dts
index 806479f..85452a7 100644
--- a/arch/powerpc/boot/dts/mpc5125twr.dts
+++ b/arch/powerpc/boot/dts/mpc5125twr.dts
@@ -230,6 +230,9 @@
};
usb@3000 {
+ /* TODO correct pinmux config and fix USB3320 ulpi dependency */
+ status = "disabled";
+
compatible = "fsl,mpc5121-usb2-dr";
reg = <0x3000 0x400>;
#address-cells = <1>;
--
1.8.3.2
^ permalink raw reply related
* Re: [RFC][PATCH v1] ASoC: fsl_ssi: Add DAI master mode support for SSI on i.MX series
From: Mark Brown @ 2013-12-19 10:48 UTC (permalink / raw)
To: Nicolin Chen; +Cc: alsa-devel, lgirdwood, tiwai, timur, perex, linuxppc-dev
In-Reply-To: <1386845085-21682-1-git-send-email-Guangyu.Chen@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 502 bytes --]
On Thu, Dec 12, 2013 at 06:44:45PM +0800, Nicolin Chen wrote:
> From: Nicolin Chen <b42378@freescale.com>
>
> This patch adds three main functions for DAI master mode: set_dai_fmt(),
> set_dai_sysclk() and set_dai_tdm_slot(), and one essential baud clock
> accordingly. After appending this patch, the fsl_ssi driver on i.MX series
> has the ability to derive LRCLK and BCLK from baud clock source so as to
> support some audio Codecs which can only be used in slave mode.
Applied, thanks.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/512x: dts: disable MPC5125 usb module
From: Gerhard Sittig @ 2013-12-19 12:30 UTC (permalink / raw)
To: Matteo Facchinetti; +Cc: agust, linuxppc-dev
In-Reply-To: <1387448639-11050-1-git-send-email-matteo.facchinetti@sirius-es.it>
On Thu, Dec 19, 2013 at 11:23 +0100, Matteo Facchinetti wrote:
>
> USB controller pin-muxing is not initialized correctly and when system boot,
> causes a kernel panic.
> USB controller is also connected with a USB3320 ulpi tranciever and
> DTS should be includes the correct dependency for initialize and activate
> this component.
>
> Signed-off-by: Matteo Facchinetti <matteo.facchinetti@sirius-es.it>
> ---
> arch/powerpc/boot/dts/mpc5125twr.dts | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/powerpc/boot/dts/mpc5125twr.dts b/arch/powerpc/boot/dts/mpc5125twr.dts
> index 806479f..85452a7 100644
> --- a/arch/powerpc/boot/dts/mpc5125twr.dts
> +++ b/arch/powerpc/boot/dts/mpc5125twr.dts
> @@ -230,6 +230,9 @@
> };
>
> usb@3000 {
> + /* TODO correct pinmux config and fix USB3320 ulpi dependency */
> + status = "disabled";
> +
> compatible = "fsl,mpc5121-usb2-dr";
> reg = <0x3000 0x400>;
> #address-cells = <1>;
> --
> 1.8.3.2
I agree on the change to the board dts file, but suggest to
reword the commit description for improved reception.
I feel it's worth trying to phrase the subject line, the commit
message, and the patch such that they can get considered
independently from each other, as not all of them are necessarily
available at the same time. Often they get looked up from
different perspectives, like terse listing first for orientation,
log with description then to determine whether to have a closer
look, the patch only at the end after the other checks told you
to look into more details. Assuming that they always show up in
combination may turn out to be inaccurate.
So I suggest some text along those lines:
at the moment the USB controller's pin muxing is not setup
correctly and causes a kernel panic upon system startup, so
disable the USB1 device tree node in the MPC5125 tower board
dts file
the USB controller is connected to an USB3320 ULPI transceiver
and the device tree should receive an update to reflect correct
dependencies and required initialization data before the USB1
node can get re-enabled
Does that sound correct to you? Does it reflect your intention,
or did I put something in wrong terms?
A minor nit would be that other reviewers in the past suggested
to put the 'status = "disabled"' line last in the list of
properties (right before optional children). I don't have strong
feelings about this. Putting it first might better reflect your
motivation of only re-enabling the node after fixing the lack or
inappropriateness of existing information first.
A different matter is that I'd suggest to re-work the MPC5125
device tree. It recently escaped my attention because it did not
share any information with the MPC5121 trees. Comparing the
MPC5125 board DTS with the MPC5121 DTS include file resulted in a
lot of unnecessary "differences" that turned out to be whitespace
or comment style only, or differences in the order of nodes.
There were only few real differences in the information, and the
MPC5125 device tree appears to only describe a subset of what the
SoC actually contains.
It may be worth looking into
- identifying common parts that are shared among the MPC5121 and
MPC5125 (my recent CCF update lists differences, but does not
explicitly list similarities, and is from the clocks
perspective and may not cover all of the SoC components)
- putting those common parts into .dtsi files if possible
- making the MPC5125 tower board reference the DTS includes,
sharing as much as possible with the other SoC variants
This may involve another split of the mpc5121.dtsi into what's
common to all MPC512x variants, and what's exclusive to MPC5121
only.
But that is a bigger task than the above quick adjustment, and is
not a required fix but just an improvement in maintainability or
completeness of information. So I suggest to pick your USB1
disabling for -next and 3.14 now, and to address the DTS cleanup
and sharing later.
virtually yours
Gerhard Sittig
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
^ 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