Re: [PATCH-Android] [android/system/media] File renamed to avoid build errors using GCC 4.5/4.6.
Thanks Jassi..
Resending the patch (created using -M)
Signed-off-by: Sachin Kamat
Change-Id: I39652f14b362c42ebc2ceb37952d8e57cf89692c
---
opensles/libopensles/Android.mk|2 +-
.../{IAndroidEffect.c => IAndroidEffect.cpp} |0
2 files changed, 1 insertions(+), 1 deletions(-)
rename opensles/libopensles/{IAndroidEffect.c => IAndroidEffect.cpp} (100%)
diff --git a/opensles/libopensles/Android.mk
b/opensles/libopensles/Android.mk
index 64e9b6f..f965d3e 100644
--- a/opensles/libopensles/Android.mk
+++ b/opensles/libopensles/Android.mk
@@ -56,7 +56,7 @@ LOCAL_SRC_FILES:= \
CEngine.c \
COutputMix.c \
IAndroidConfiguration.c \
-IAndroidEffect.c \
+IAndroidEffect.cpp\
IAndroidEffectCapabilities.c \
IAndroidEffectSend.c \
IBassBoost.c \
diff --git a/opensles/libopensles/IAndroidEffect.c
b/opensles/libopensles/IAndroidEffect.cpp
similarity index 100%
rename from opensles/libopensles/IAndroidEffect.c
rename to opensles/libopensles/IAndroidEffect.cpp
--
1.7.1
**
On 16 March 2011 11:03, Jassi Brar wrote:
> On Wed, Mar 16, 2011 at 10:46 AM, Sachin Kamat
> wrote:
> > Signed-off-by: Sachin Kamat
> >
> > Change-Id: I39652f14b362c42ebc2ceb37952d8e57cf89692c
> > ---
> > opensles/libopensles/Android.mk |2 +-
> > opensles/libopensles/IAndroidEffect.c | 130
> ---
> > opensles/libopensles/IAndroidEffect.cpp | 130
> +++
> > 3 files changed, 131 insertions(+), 131 deletions(-)
> > delete mode 100644 opensles/libopensles/IAndroidEffect.c
> > create mode 100644 opensles/libopensles/IAndroidEffect.cpp
>
> If it's purely file rename, using '-M' flag with git format-patch
> gives much more manageable patch.
>
--
With warm regards,
Sachin
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH-Android] [android/system/media] File renamed to avoid build errors using GCC 4.5/4.6.
On Wed, Mar 16, 2011 at 10:46 AM, Sachin Kamat wrote: > Signed-off-by: Sachin Kamat > > Change-Id: I39652f14b362c42ebc2ceb37952d8e57cf89692c > --- > opensles/libopensles/Android.mk | 2 +- > opensles/libopensles/IAndroidEffect.c | 130 > --- > opensles/libopensles/IAndroidEffect.cpp | 130 > +++ > 3 files changed, 131 insertions(+), 131 deletions(-) > delete mode 100644 opensles/libopensles/IAndroidEffect.c > create mode 100644 opensles/libopensles/IAndroidEffect.cpp If it's purely file rename, using '-M' flag with git format-patch gives much more manageable patch. ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH-Android] [android/external/skia] Modified to avoid build errors using GCC 4.5/4.6.
Signed-off-by: Sachin Kamat
Change-Id: Ibebec39dfd1e083a052870927a4129fdeca7cf8a
---
src/images/SkFlipPixelRef.cpp|2 +-
src/images/SkImageRef_GlobalPool.cpp |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/images/SkFlipPixelRef.cpp b/src/images/SkFlipPixelRef.cpp
index 8d0f15a..bdf4d0a 100644
--- a/src/images/SkFlipPixelRef.cpp
+++ b/src/images/SkFlipPixelRef.cpp
@@ -74,7 +74,7 @@ SkPixelRef* SkFlipPixelRef::Create(SkFlattenableReadBuffer&
buffer) {
return SkNEW_ARGS(SkFlipPixelRef, (buffer));
}
-static SkPixelRef::Registrar::Registrar reg("SkFlipPixelRef",
+static SkPixelRef::Registrar reg("SkFlipPixelRef",
SkFlipPixelRef::Create);
///
diff --git a/src/images/SkImageRef_GlobalPool.cpp
b/src/images/SkImageRef_GlobalPool.cpp
index 1f0bc43..15d3f0d 100644
--- a/src/images/SkImageRef_GlobalPool.cpp
+++ b/src/images/SkImageRef_GlobalPool.cpp
@@ -50,7 +50,7 @@ SkPixelRef*
SkImageRef_GlobalPool::Create(SkFlattenableReadBuffer& buffer) {
return SkNEW_ARGS(SkImageRef_GlobalPool, (buffer));
}
-static SkPixelRef::Registrar::Registrar reg("SkImageRef_GlobalPool",
+static SkPixelRef::Registrar reg("SkImageRef_GlobalPool",
SkImageRef_GlobalPool::Create);
///
--
1.7.1
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH-Android] [android/build] Disable Werror option to avoid build errors using GCC 4.5/4.6.
Signed-off-by: Sachin Kamat Change-Id: I471e831d569b37345752d788f5a213ffd6616690 --- core/config.mk |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/core/config.mk b/core/config.mk index a76aec8..45100fb 100644 --- a/core/config.mk +++ b/core/config.mk @@ -98,7 +98,7 @@ COMMON_JAVA_PACKAGE_SUFFIX := .jar COMMON_ANDROID_PACKAGE_SUFFIX := .apk # list of flags to turn specific warnings in to errors -TARGET_ERROR_FLAGS := -Werror=return-type -Werror=non-virtual-dtor -Werror=address -Werror=sequence-point +TARGET_ERROR_FLAGS := # TODO: do symbol compression TARGET_COMPRESS_MODULE_SYMBOLS := false -- 1.7.1 ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH-Android] [android/system/media] File renamed to avoid build errors using GCC 4.5/4.6.
Signed-off-by: Sachin Kamat
Change-Id: I39652f14b362c42ebc2ceb37952d8e57cf89692c
---
opensles/libopensles/Android.mk |2 +-
opensles/libopensles/IAndroidEffect.c | 130 ---
opensles/libopensles/IAndroidEffect.cpp | 130 +++
3 files changed, 131 insertions(+), 131 deletions(-)
delete mode 100644 opensles/libopensles/IAndroidEffect.c
create mode 100644 opensles/libopensles/IAndroidEffect.cpp
diff --git a/opensles/libopensles/Android.mk b/opensles/libopensles/Android.mk
index 64e9b6f..f965d3e 100644
--- a/opensles/libopensles/Android.mk
+++ b/opensles/libopensles/Android.mk
@@ -56,7 +56,7 @@ LOCAL_SRC_FILES:= \
CEngine.c \
COutputMix.c \
IAndroidConfiguration.c \
-IAndroidEffect.c \
+IAndroidEffect.cpp\
IAndroidEffectCapabilities.c \
IAndroidEffectSend.c \
IBassBoost.c \
diff --git a/opensles/libopensles/IAndroidEffect.c
b/opensles/libopensles/IAndroidEffect.c
deleted file mode 100644
index b934c15..000
--- a/opensles/libopensles/IAndroidEffect.c
+++ /dev/null
@@ -1,130 +0,0 @@
-/*
- * Copyright (C) 2010 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-/* Android Effect implementation */
-
-#include "sles_allinclusive.h"
-
-
-static SLresult IAndroidEffect_CreateEffect(SLAndroidEffectItf self,
-SLInterfaceID effectImplementationId) {
-
-SL_ENTER_INTERFACE
-
-IAndroidEffect *this = (IAndroidEffect *) self;
-if (SL_OBJECTID_AUDIOPLAYER == IObjectToObjectID(this->mThis)) {
-CAudioPlayer *ap = (CAudioPlayer *)this->mThis;
-if (NULL != ap->mAudioTrack) {
-result = android_genericFx_createEffect(this,
effectImplementationId, ap->mSessionId);
-} else {
-result = SL_RESULT_RESOURCE_ERROR;
-}
-} else if (SL_OBJECTID_OUTPUTMIX == IObjectToObjectID(this->mThis)) {
-result = android_genericFx_createEffect(this, effectImplementationId,
-android::AudioSystem::SESSION_OUTPUT_MIX);
-} else {
-// the interface itself is invalid because it is not attached to an
AudioPlayer or
-// an OutputMix
-result = SL_RESULT_PARAMETER_INVALID;
-}
-
-SL_LEAVE_INTERFACE
-}
-
-
-static SLresult IAndroidEffect_ReleaseEffect(SLAndroidEffectItf self,
-SLInterfaceID effectImplementationId) {
-
-SL_ENTER_INTERFACE
-
-IAndroidEffect *this = (IAndroidEffect *) self;
-result = android_genericFx_releaseEffect(this, effectImplementationId);
-
-SL_LEAVE_INTERFACE
-}
-
-
-static SLresult IAndroidEffect_SetEnabled(SLAndroidEffectItf self,
-SLInterfaceID effectImplementationId, SLboolean enabled) {
-
-SL_ENTER_INTERFACE
-
-IAndroidEffect *this = (IAndroidEffect *) self;
-result = android_genericFx_setEnabled(this, effectImplementationId,
enabled);
-
-SL_LEAVE_INTERFACE
-}
-
-
-static SLresult IAndroidEffect_IsEnabled(SLAndroidEffectItf self,
-SLInterfaceID effectImplementationId, SLboolean * pEnabled) {
-
-SL_ENTER_INTERFACE
-
-IAndroidEffect *this = (IAndroidEffect *) self;
-result = android_genericFx_isEnabled(this, effectImplementationId,
pEnabled);
-
-SL_LEAVE_INTERFACE
-}
-
-
-static SLresult IAndroidEffect_SendCommand(SLAndroidEffectItf self,
-SLInterfaceID effectImplementationId, SLuint32 command, SLuint32
commandSize,
-void* pCommand, SLuint32 *replySize, void *pReply) {
-
-SL_ENTER_INTERFACE
-
-IAndroidEffect *this = (IAndroidEffect *) self;
-result = android_genericFx_sendCommand(this, effectImplementationId,
command, commandSize,
-pCommand, replySize, pReply);
-
-SL_LEAVE_INTERFACE
-}
-
-
-static const struct SLAndroidEffectItf_ IAndroidEffect_Itf = {
-IAndroidEffect_CreateEffect,
-IAndroidEffect_ReleaseEffect,
-IAndroidEffect_SetEnabled,
-IAndroidEffect_IsEnabled,
-IAndroidEffect_SendCommand
-};
-
-void IAndroidEffect_init(void *self)
-{
-IAndroidEffect *this = (IAndroidEffect *) self;
-this->mItf = &IAndroidEffect_Itf;
-#ifndef TARGET_SIMULATOR
-this->mEffects = new android::KeyedVector();
-#endif
-}
-
-void IAndroidEffect_deinit(void *self)
-{
-IAndroidEffect *this = (IAndroidEffect *) self;
-#ifndef TARGET_SIMULATO
[PATCH-Android] [android/external/webkit] Modified to avoid build errors using GCC 4.5/4.6.
Signed-off-by: Sachin Kamat
Change-Id: I2cd4ccdf7edc39b025ef39bd5db0154c616aa5a9
---
WebKit/android/nav/FindCanvas.cpp |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/WebKit/android/nav/FindCanvas.cpp
b/WebKit/android/nav/FindCanvas.cpp
index d8e908b..15cce43 100644
--- a/WebKit/android/nav/FindCanvas.cpp
+++ b/WebKit/android/nav/FindCanvas.cpp
@@ -98,7 +98,7 @@ GlyphSet::~GlyphSet() {
// part of mLowerGlyphs
}
-GlyphSet::GlyphSet& GlyphSet::operator=(GlyphSet& src) {
+GlyphSet& GlyphSet::operator=(GlyphSet& src) {
mTypeface = src.mTypeface;
mCount = src.mCount;
if (mCount > MAX_STORAGE_COUNT) {
--
1.7.1
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH-Android] Patches to build Android using GCC 4.5/4.6.
Hi, The next set of patches will help in building Android source code out of the box using Linaro GCC 4.5/4.6 toolchains. The patches basically try to circumvent the build problem caused by stricter GCC 4.5/4.6. These patches are based on remotes/m/linaro_android_2.3.2 branch. Regards, Sachin ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH-Android] [android/frameworks/base] Modified to eliminate build errors using GCC 4.5/4.6.
Signed-off-by: Sachin Kamat
Change-Id: Id31f863427cd3a7d5d5abced71de78a76ae3d028
---
libs/utils/RefBase.cpp |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libs/utils/RefBase.cpp b/libs/utils/RefBase.cpp
index 0bd1af4..d28b751 100644
--- a/libs/utils/RefBase.cpp
+++ b/libs/utils/RefBase.cpp
@@ -480,7 +480,7 @@ void RefBase::weakref_type::printRefs() const
void RefBase::weakref_type::trackMe(bool enable, bool retain)
{
-static_cast(this)->trackMe(enable, retain);
+static_cast(this)->trackMe(enable, retain);
}
RefBase::weakref_type* RefBase::createWeak(const void* id) const
--
1.7.1
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH-Android] [android/libcore] Disable Werror option to avoid build errors using GCC 4.5/4.6.
Signed-off-by: Sachin Kamat Change-Id: I2c4699686ada362b43b836ace61636fc000720d3 --- NativeCode.mk |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/NativeCode.mk b/NativeCode.mk index 4c6ffa5..7b56aef 100644 --- a/NativeCode.mk +++ b/NativeCode.mk @@ -85,7 +85,7 @@ core_static_libraries := $(sort $(LOCAL_STATIC_LIBRARIES)) include $(CLEAR_VARS) -LOCAL_CFLAGS += -Wall -Wextra -Werror +LOCAL_CFLAGS += -Wall -Wextra ifeq ($(TARGET_ARCH),arm) # Ignore "note: the mangling of 'va_list' has changed in GCC 4.4" -- 1.7.1 ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH-Android] [android/external/elfcopy] Modified to eliminate build errors using GCC 4.5/4.6.
Signed-off-by: Sachin Kamat Change-Id: I01286388a1bef915cb06adae862d9f3a0397b059 --- elfcopy.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/elfcopy.c b/elfcopy.c index ed1e6ce..da054c7 100644 --- a/elfcopy.c +++ b/elfcopy.c @@ -2456,7 +2456,6 @@ update_symbol_values(Elf *elf, GElf_Ehdr *ehdr, out why and also figure out whether the zero value should have been adjusted, after all. */ -ASSERT(!(shdr_info[sym->st_shndx].shdr.sh_flags & SHF_ALLOC)); ASSERT(shdr_info[i].shdr.sh_type == SHT_SYMTAB); } -- 1.7.1 ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH-Android] [android/frameworks/opt/emoji] Disable Werror option to avoid build errors using GCC 4.5/4.6.
Signed-off-by: Sachin Kamat Change-Id: I61a9d5c9b5aef8358a04d0e1ac8964f905f0970a --- Android.mk |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Android.mk b/Android.mk index 331d794..181f115 100644 --- a/Android.mk +++ b/Android.mk @@ -32,7 +32,7 @@ ifneq ($(TARGET_SIMULATOR),true) LOCAL_SHARED_LIBRARIES += libdl endif -LOCAL_CFLAGS += -Wall -Werror +LOCAL_CFLAGS += -Wall LOCAL_LDLIBS += -lpthread -- 1.7.1 ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH-Android] [android/dalvik] Disable Werror option to avoid build errors using GCC 4.5/4.6.
Signed-off-by: Sachin Kamat Change-Id: I8fa29112df35fea05d034ef4d08559135c5e327a --- vm/Dvm.mk |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/vm/Dvm.mk b/vm/Dvm.mk index 0867ffa..7b8a39d 100644 --- a/vm/Dvm.mk +++ b/vm/Dvm.mk @@ -264,7 +264,6 @@ MTERP_ARCH_KNOWN := false ifeq ($(dvm_arch),arm) #dvm_arch_variant := armv7-a #LOCAL_CFLAGS += -march=armv7-a -mfloat-abi=softfp -mfpu=vfp - LOCAL_CFLAGS += -Werror MTERP_ARCH_KNOWN := true # Select architecture-specific sources (armv4t, armv5te etc.) LOCAL_SRC_FILES += \ -- 1.7.1 ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: LAVA integration effort for Android validation
On 16 March 2011 10:03, Michael Hudson-Doyle wrote: > On Tue, 15 Mar 2011 11:17:14 -0500, Paul Larson > wrote: > > > I like statically IP. How do we know which board would get which IP? > > > One idea would be to set up a DNS for internal board names. e.g. > > > panda01.internal.network would resolve to our preallocated static IP > > > for board "panda01". > > > > > > This could then be used by the dispatcher to resolve the IP and pass > > > it to the board through kernel cmdline. > > > > > > I hadn't really thought of passing it via cmdline. If we need to bring > it > > up statically, we could possibly do that, or possibly just make the > > necessary adjustments before booting the image. What I was proposing, if > > the image is already set up to dhcp by default, we could run a dhcp > server > > on the control node with the static ip assignments already in place for > each > > board. > > How would you do this? Most boards don't have a fixed MAC address. You > can set up a dhcp-name but that (AIUI) means a different rootfs for each > board and then you might as well just allocate a static IP... > > Also, some boards of the same type have the same MAC as well.. > > That way they always get the same address that we pre-allocated for > > that board. We'll also need to make sure that the control node has a > valid > > ip on both the internal and the external network. > > Cheers, > mwh > > ___ > linaro-dev mailing list > linaro-dev@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-dev > -- With warm regards, Sachin ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V4 1/5] arm/dt: add basic mx51 device tree support
On Tue, Mar 15, 2011 at 9:34 PM, Jason Hui wrote: > Hi, Grant, > > On Tue, Mar 15, 2011 at 3:03 PM, Grant Likely > wrote: >> Hi Jason, >> >> Minor comments below. >> >> On Thu, Mar 10, 2011 at 12:59:41PM +0800, Jason Liu wrote: >>> Signed-off-by: Jason Liu >>> Signed-off-by: Jason Liu >> >> This looks wrong. You should only have one s-o-b line. Use one email >> addr or the other. Not both. > > I just take the same approach as this link: > https://lkml.org/lkml/2010/12/17/363 > If you think it's not applicable, I can change it. Yeah, I don't think that's right. A s-o-b is a personal assertion that the patch is to the best of your knowledge that you have the right to submit it for inclusion in the kernel (see section 12 of Documentation/SubmittingPatches). It doesn't make any statements about who owns the copyright on the patch or other issues of corporate ownership. Companies may have policies about which email address employees use when signing off, but that isn't what the s-o-b protocol is for. Since there isn't more than one of you, you should only have one s-o-b line. :-) Paul, since your email was presented as evidence, would you care to offer a counter-argument? :-) g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: LAVA integration effort for Android validation
> How would you do this? Most boards don't have a fixed MAC address. You > can set up a dhcp-name but that (AIUI) means a different rootfs for each > board and then you might as well just allocate a static IP... I know this is a problem on some boards, but I thought it had been fixed on some, and some required something in uboot, which I hoped we could set in advance, to make it work. That's one reason I hadn't moved to this yet, I want to make sure it's actually possible first. But if you have a different idea, I'm open to suggestions. :) Thanks, Paul Larson ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: LAVA integration effort for Android validation
On Tue, 15 Mar 2011 11:17:14 -0500, Paul Larson wrote: > > I like statically IP. How do we know which board would get which IP? > > One idea would be to set up a DNS for internal board names. e.g. > > panda01.internal.network would resolve to our preallocated static IP > > for board "panda01". > > > > This could then be used by the dispatcher to resolve the IP and pass > > it to the board through kernel cmdline. > > > > I hadn't really thought of passing it via cmdline. If we need to bring it > up statically, we could possibly do that, or possibly just make the > necessary adjustments before booting the image. What I was proposing, if > the image is already set up to dhcp by default, we could run a dhcp server > on the control node with the static ip assignments already in place for each > board. How would you do this? Most boards don't have a fixed MAC address. You can set up a dhcp-name but that (AIUI) means a different rootfs for each board and then you might as well just allocate a static IP... > That way they always get the same address that we pre-allocated for > that board. We'll also need to make sure that the control node has a valid > ip on both the internal and the external network. Cheers, mwh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V4 1/5] arm/dt: add basic mx51 device tree support
Hi, Grant,
On Tue, Mar 15, 2011 at 3:03 PM, Grant Likely wrote:
> Hi Jason,
>
> Minor comments below.
>
> On Thu, Mar 10, 2011 at 12:59:41PM +0800, Jason Liu wrote:
>> Signed-off-by: Jason Liu
>> Signed-off-by: Jason Liu
>
> This looks wrong. You should only have one s-o-b line. Use one email
> addr or the other. Not both.
I just take the same approach as this link: https://lkml.org/lkml/2010/12/17/363
If you think it's not applicable, I can change it.
>
>> ---
>> arch/arm/mach-mx5/Kconfig | 8
>> arch/arm/mach-mx5/Makefile | 1 +
>> arch/arm/mach-mx5/board-dt.c | 65
>> +++
>> arch/arm/mach-mx5/clock-mx51-mx53.c | 43 -
>> arch/arm/plat-mxc/include/mach/common.h | 1 +
>> 5 files changed, 117 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
>> index de4fa99..6438f87 100644
>> --- a/arch/arm/mach-mx5/Kconfig
>> +++ b/arch/arm/mach-mx5/Kconfig
>> @@ -47,6 +47,14 @@ config MACH_MX51_BABBAGE
>> u-boot. This includes specific configurations for the board and its
>> peripherals.
>>
>> +config MACH_MX51_DT
>> + bool "Generic MX51 board (FDT support)"
>> + select USE_OF
>> + select SOC_IMX51
>> + help
>> + Support for generic Freescale i.MX51 boards using Flattened Device
>> + Tree.
>> +
>> config MACH_MX51_3DS
>> bool "Support MX51PDK (3DS)"
>> select SOC_IMX51
>> diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
>> index 0d43be9..540697e 100644
>> --- a/arch/arm/mach-mx5/Makefile
>> +++ b/arch/arm/mach-mx5/Makefile
>> @@ -18,3 +18,4 @@ obj-$(CONFIG_MACH_EUKREA_CPUIMX51SD) += board-cpuimx51sd.o
>> obj-$(CONFIG_MACH_EUKREA_MBIMXSD51_BASEBOARD) += eukrea_mbimxsd-baseboard.o
>> obj-$(CONFIG_MACH_MX51_EFIKAMX) += board-mx51_efikamx.o
>> obj-$(CONFIG_MACH_MX50_RDP) += board-mx50_rdp.o
>> +obj-$(CONFIG_MACH_MX51_DT) += board-dt.o
>> diff --git a/arch/arm/mach-mx5/board-dt.c b/arch/arm/mach-mx5/board-dt.c
>> new file mode 100644
>> index 000..19c60a4
>> --- /dev/null
>> +++ b/arch/arm/mach-mx5/board-dt.c
>> @@ -0,0 +1,65 @@
>> +/*
>> + * Copyright 2011 Linaro Ltd.
>> + * Copyright 2011 Freescale Semiconductor, Inc.
>> + *
>> + * The code contained herein is licensed under the GNU General Public
>> + * License. You may obtain a copy of the GNU General Public License
>> + * Version 2 or later at the following locations:
>> + *
>> + * http://www.opensource.org/licenses/gpl-license.html
>> + * http://www.gnu.org/copyleft/gpl.html
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include "devices.h"
>> +
>> +static struct of_device_id mx51_dt_match_table[] __initdata = {
>> + { .compatible = "simple-bus", },
>> + {}
>> +};
>> +
>> +static void __init mx51_dt_board_init(void)
>> +{
>> + of_platform_bus_probe(NULL, mx51_dt_match_table, NULL);
>> +}
>> +
>> +static void __init mx51_dt_timer_init(void)
>> +{
>> + mx51_clocks_init(32768, 2400, 22579200, 0);
>> + mx5_clk_dt_init();
>> +}
>> +
>> +static struct sys_timer mxc_timer = {
>> + .init = mx51_dt_timer_init,
>> +};
>> +
>> +static const char *mx51_dt_board_compat[] = {
>> + "fsl,mx51-babbage",
>> + NULL
>> +};
>> +
>> +DT_MACHINE_START(MX51_DT, "Freescale MX51 (Flattened Device Tree)")
>> + .boot_params = PHYS_OFFSET + 0x100,
>
> You should be able to drop the .boot_params line.
OK,
>
>> + .map_io = mx51_map_io,
>> + .init_irq = mx51_init_irq,
>> + .init_machine = mx51_dt_board_init,
>> + .dt_compat = mx51_dt_board_compat,
>> + .timer = &mxc_timer,
>> +MACHINE_END
>> diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c
>> b/arch/arm/mach-mx5/clock-mx51-mx53.c
>> index 0a19e75..dedb7f9 100644
>> --- a/arch/arm/mach-mx5/clock-mx51-mx53.c
>> +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
>> @@ -15,13 +15,19 @@
>> #include
>> #include
>> #include
>> -
>> +#include
>> #include
>>
>> #include
>> #include
>> #include
>>
>> +#ifdef CONFIG_OF
>> +#include
>> +#include
>> +#include
>> +#endif /* CONFIG_OF */
>
> You can drop the #ifdef CONFIG_OF here. linux/of*.h is safe to
> include when CONFIG_OF is not selected.
OK,
>
>> +
>> #include "crm_regs.h"
>>
>> /* External clock values passed-in by the board code */
>> @@ -1432,3 +1438,38 @@ int __init mx53_clocks_init(unsigned long ckil,
>> unsigned long osc,
>> MX53_INT_GPT);
>> return 0;
>> }
>> +
>> +#ifdef CONFIG_OF
>> +static struct clk *mx5_dt_clk_get(struct device_node *np,
>> + const char *output_id, void *data)
>> +{
>> + return data;
>> +}
>> +
>> +static __init void mx5_dt_scan_clks(void)
>> +{
>> + struct device_node *node;
Re: [PATCH V4 5/5] net/fec: add device tree matching support
Hi, Grant,
On Tue, Mar 15, 2011 at 3:14 PM, Grant Likely wrote:
> On Thu, Mar 10, 2011 at 12:59:45PM +0800, Jason Liu wrote:
>> Signed-off-by: Jason Liu
>> Signed-off-by: Jason Liu
>> ---
>> drivers/net/fec.c | 13 +
>> 1 files changed, 13 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
>> index 02cdd71..fcb9768 100644
>> --- a/drivers/net/fec.c
>> +++ b/drivers/net/fec.c
>> @@ -45,6 +45,9 @@
>> #include
>> #include
>>
>> +#include
>> +#include
>> +
>
> Should be mixed in with the rest of the linux/*.h includes (don't put
> a blank line between them.
OK,
>
>> #include
>>
>> #ifndef CONFIG_ARM
>> @@ -1523,6 +1526,13 @@ static const struct dev_pm_ops fec_pm_ops = {
>> };
>> #endif
>>
>> +#ifdef CONFIG_OF
>> +static struct of_device_id fec_matches[] = {
>> + { .compatible = "fsl,imx-fec" },
>
> Must have documentation for this binding in
> Documentation/devicetree/bindings before I can pick this up. Same
> goes for the uart driver patch.
OK, I will write one documentation for it and the same with uart.
>
> Also, I recommend being more specific on the compatible property.
> fsl,imx51-fec would be better. Newer parts can claim compatibility
> with this one if you're concerned about supporting multiple parts.
>
> ie. for imx 53, this would be appropriate:
>
> compatible = "fsl,imx53-fec", "fsl,imx51-fec";
OK,
>
>> + {},
>> +};
>> +#endif
>> +
>> static struct platform_driver fec_driver = {
>> .driver = {
>> .name = DRIVER_NAME,
>> @@ -1530,6 +1540,9 @@ static struct platform_driver fec_driver = {
>> #ifdef CONFIG_PM
>> .pm = &fec_pm_ops,
>> #endif
>> +#ifdef CONFIG_OF
>> + .of_match_table = fec_matches,
>> +#endif
>> },
>> .id_table = fec_devtype,
>> .probe = fec_probe,
>> --
>> 1.7.1
>>
>>
>> ___
>> linaro-dev mailing list
>> linaro-dev@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V4 3/5] serial/imx: parse from device tree support
Hi, Grant,
On Tue, Mar 15, 2011 at 3:07 PM, Grant Likely wrote:
> Minor comments, but otherwise goes first.
>
> On Thu, Mar 10, 2011 at 12:59:43PM +0800, Jason Liu wrote:
>> Signed-off-by: Jason Liu
>> Signed-off-by: Jason Liu
>> Signed-off-by: Jeremy Kerr
>
> Jeremy's s-o-b should go at the top of the list. He started it, and
> then you took over. s-o-b lines is the chain of people who have
> handled a patch, and so it should reflect the order that they handled
> it.
OK, get it.
>
>> ---
>> drivers/tty/serial/imx.c | 79
>> --
>> 1 files changed, 69 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index dfcf4b1..c9483d1 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -53,6 +53,9 @@
>> #include
>> #include
>>
>> +#include
>> +#include
>> +
>
> These should be mixed in with the rest of the includes.
OK,
>
>> /* Register definitions */
>> #define URXD0 0x0 /* Receiver Register */
>> #define URTX0 0x40 /* Transmitter Register */
>> @@ -1224,6 +1227,53 @@ static int serial_imx_resume(struct platform_device
>> *dev)
>> return 0;
>> }
>>
>> +#ifdef CONFIG_OF
>> +static int serial_imx_probe_dt(struct imx_port *sport,
>> + struct platform_device *pdev)
>> +{
>> + struct device_node *node = pdev->dev.of_node;
>> + static int line;
>> +
>> + if (!node)
>> + return -ENODEV;
>> +
>> + if (of_get_property(node, "fsl,has-rts-cts", NULL))
>> + sport->have_rtscts = 1;
>> +
>> + if (of_get_property(node, "fsl,irda-mode", NULL))
>> + sport->use_irda = 1;
>> +
>> + sport->port.line = line++;
>> +
>> + return 0;
>> +}
>> +#else
>> +static int serial_imx_probe_dt(struct imx_port *sport,
>> + struct platform_device *pdev)
>> +{
>> + return -ENODEV;
>> +}
>> +#endif
>> +
>> +static int serial_imx_probe_pdata(struct imx_port *sport,
>> + struct platform_device *pdev)
>> +{
>> + struct imxuart_platform_data *pdata = pdev->dev.platform_data;
>> +
>> + if (!pdata)
>> + return 0;
>> +
>> + if (pdata->flags & IMXUART_HAVE_RTSCTS)
>> + sport->have_rtscts = 1;
>> +
>> +#ifdef CONFIG_IRDA
>> + if (pdata->flags & IMXUART_IRDA)
>> + sport->use_irda = 1;
>> +#endif
>> + sport->port.line = pdev->id;
>> +
>> + return 0;
>> +}
>> static int serial_imx_probe(struct platform_device *pdev)
>> {
>> struct imx_port *sport;
>> @@ -1236,6 +1286,12 @@ static int serial_imx_probe(struct platform_device
>> *pdev)
>> if (!sport)
>> return -ENOMEM;
>>
>> + ret = serial_imx_probe_dt(sport, pdev);
>> + if (ret == -ENODEV)
>> + ret = serial_imx_probe_pdata(sport, pdev);
>> + if (ret)
>> + goto free;
>> +
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> if (!res) {
>> ret = -ENODEV;
>> @@ -1260,7 +1316,6 @@ static int serial_imx_probe(struct platform_device
>> *pdev)
>> sport->port.fifosize = 32;
>> sport->port.ops = &imx_pops;
>> sport->port.flags = UPF_BOOT_AUTOCONF;
>> - sport->port.line = pdev->id;
>> init_timer(&sport->timer);
>> sport->timer.function = imx_timeout;
>> sport->timer.data = (unsigned long)sport;
>> @@ -1274,17 +1329,13 @@ static int serial_imx_probe(struct platform_device
>> *pdev)
>>
>> sport->port.uartclk = clk_get_rate(sport->clk);
>>
>> - imx_ports[pdev->id] = sport;
>> + if (imx_ports[sport->port.line]) {
>> + ret = -EBUSY;
>> + goto clkput;
>> + }
>> + imx_ports[sport->port.line] = sport;
>>
>> pdata = pdev->dev.platform_data;
>> - if (pdata && (pdata->flags & IMXUART_HAVE_RTSCTS))
>> - sport->have_rtscts = 1;
>> -
>> -#ifdef CONFIG_IRDA
>> - if (pdata && (pdata->flags & IMXUART_IRDA))
>> - sport->use_irda = 1;
>> -#endif
>> -
>> if (pdata && pdata->init) {
>> ret = pdata->init(pdev);
>> if (ret)
>> @@ -1336,6 +1387,11 @@ static int serial_imx_remove(struct platform_device
>> *pdev)
>> return 0;
>> }
>>
>> +static struct of_device_id imx_uart_matches[] = {
>> + { .compatible = "fsl,imx51-uart" },
>> + {},
>> +};
>> +
>> static struct platform_driver serial_imx_driver = {
>> .probe = serial_imx_probe,
>> .remove = serial_imx_remove,
>> @@ -1345,6 +1401,9 @@ static struct platform_driver serial_imx_driver = {
>> .driver = {
>> .name = "imx-uart",
>> .owner = THIS_MODULE,
>> +#if defined(CONFIG_OF)
>> + .of_match_table = imx_uart_matches,
>> +#endif
>> },
>> };
>>
>> --
>> 1.7.1
>>
>
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: Better reviews for the same cost in gcc-linaro
On 15 March 2011 22:44, Loïc Minier wrote: >> > 7) merge to trunk (with the inevitable ChangeLog merge failure >> > that you mentioned). > > bzr has plugins to merge changelog entries for some types of > changelogs; I wonder whether we could use these here. Another option > would be to generate a GNU ChangeLog from the bzr log at release time > as we do for linaro-image-tools for instance. spiv just put up a proposal[1] to ship the changelog-merging code in bzr core. If that doesn't handle gcc's particular ChangeLog form, please file a bug against bzr with an example and tagged linaro. [1] https://code.launchpad.net/~spiv/bzr/add-changelog-merge/+merge/53380 -- Martin ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: linaro android build system issues
On Fri, 11 Mar 2011 17:23:45 +1300, Michael Hudson-Doyle wrote: > Hi all, > > For some reason the jenkins instance that powers the android build > system isn't auto provisioning slaves from ec2, so builds are being > queued but not executed. If you notice this happen and it's blocking > you, get an admin (asac, loic, patryk) to go to > https://android-build.linaro.org/jenkins/computer/ and click the > "provision via ec2" button, and a slave will be started and run the > builds. You can click it more than once to get multiple slaves to clear > the queue more quickly. The slaves will be de-provisioned after they > have been idle for a little while, so we don't need to manually clean up > after ourselves. > > I'll try and figure out what's going on and fix it next week. I've figured it out (jenkins was counting _all_ instances running in the account it has the credentials for, not just those it launched, against its instance cap) and have worked around it for now (raised the instance cap) pending a better fix (using security groups to identify instances jenkins has launched). Cheers, mwh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: Better reviews for the same cost in gcc-linaro
On Tue, Mar 15, 2011 at 11:53 PM, Andrew Stubbs wrote: > Richard may know all this, but I'm just going to post anyway in case some > people reading can benefit from some tips. > > I find that bzr is slow - there's no getting around it, but there are some > tricks that can help. > > On 14/03/11 11:12, Richard Sandiford wrote: >> >> My concern was that, again with my way of working, the process is: >> >> 1) develop fix >> 2) branch trunk (creating a whole new gcc source tree, so quite slow) > > This is going to take a short while however you cut it, but doing it the > naive way is *very* slow. > > So, make sure you use "init-repo". This creates a top-level ".bzr" directory > that can be shared between many branches. This cuts down on network usage, > and speeds up local branching also. > > To set it up: > > mkdir my_work_dir > cd my_work_dir > bzr init-repo . > bzr branch lp:gcc-linaro my_1st_branch > bzr branch lp:gcc-linaro my_2nd_branch > > > Basically, any branches you create within sub-directories of "my_work_dir" > have shared history, so there's no need to waste time duplicating it. > > I believe hard-linking should be a win too, but I don't use it much: > > bzr branch --hardlink my_1st_branch my_2nd_branch > > You might also try experimenting with local stacked branches (so the new one > has only shallow history), but I'm not sure there's any advantage if you use > a shared repo: > > bzr branch --stacked my_1st_branch my_2nd_branch > >> 3) apply fix to new branch, with ChangeLog entry >> 4) publish new branch in laucnhpad > > I find "bzr push" is quite fast, but there's a special gotcha - it always > stacks new feature branches on top of the gcc-linaro/4.5 branch (more > accurately, the "focus of development" branch), and if you're working with > 4.4 or 4.6, that means there quite a lot of difference to upload. > > You can fix this by telling it what branch to stack on: > > bzr push > --stacked-on=bzr+ssh://bazaar.launchpad.net/~linaro-toolchain-dev/gcc-linaro/4.6 > lp:~/gcc-linaro/ > > Unfortunately, the "lp:..." form doesn't work with --stacked-on. :( > >> 5) wait for branch to be processed by launchpad (only a few minutes, >> nothing major) >> 6) ask for a review >> 7) merge to trunk (with the inevitable ChangeLog merge failure >> that you mentioned). >> >> whereas the upstream way would be: >> >> 1) develop fix >> 2) ask for a review, attaching patch >> 3) apply patch to trunk, with ChangeLog entry >> >> The upstream way feels much simpler, and avoids the merge failure hassle. > > True. If you prefer, by all means include the ChangeLog entry in the merge > request, and it can be inserted into ChangeLog.linaro at merge time. > > It makes no real difference to me, but it does mean that if there is anybody > out there pulling toolchains from feature branches, the ChangeLogs won't be > helpful. I doubt that anybody does that??? > >>> Short story is that we have a better tool than svn, so feature >>> branches may make some use cases overall easier and more transparent. >> >> Well, as you say, the size of GCC and its history is pushing the limits >> of bzr a bit. For bug-fixing and committing, I actually find quilt+svn >> to be a fair bit more productive than bzr, and that's even with Andrew >> doing the heavy work on merging. > > I'd say that calling bzr a better tool than svn is pushing it a bit. It has > some nice features, and it works better than svn for launchpad's purposes, > but I'd still rather use SVN or, better still, git. If bzr wasn't such a dog > to use (for large projects), it would be as good as SVN or GIT, but it would > not be "better" - just different. (Svn was lacking a good merge tool, but I > believe that's fixed now?) I prefer bzr over svn for this project for reasons that are better discussed over a beer... I've updated the BzrTips page on the wiki: https://wiki.linaro.org/WorkingGroups/ToolChain/BzrTips with links out to Andrew's, Loic's, and Martin's emails. If these tips work for you, please edit the wiki. I've also updated the BzrIssues page at: https://wiki.linaro.org/MichaelHope/Sandbox/BzrIssues with the performance numbers from earlier in the thread. -- Michael ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 5/7] mmc: support sdhci-esdhc-imx as an OF device
On Mon, Mar 14, 2011 at 10:25:57PM +0800, Shawn Guo wrote:
> Signed-off-by: Shawn Guo
dt support can be added directly to sdchi-pltfm.c drivers now. There
is no longer any need to use sdhci-of-core.c any more. For an
example, see the patch titled "of/tegra: add sdhci device tree
handling" in my devicetree/test branch.
http://git.secretlab.ca/?p=linux-2.6.git;a=commit;h=1437bd1d5a1fec2dcf17d9fde73b385569c82084
g.
> ---
> drivers/mmc/host/Kconfig |7 ---
> drivers/mmc/host/Makefile|3 ++-
> drivers/mmc/host/sdhci-of-core.c |3 +++
> drivers/mmc/host/sdhci-of.h |1 +
> 4 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index afe8c6f..9746ec1 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -133,12 +133,13 @@ config MMC_SDHCI_CNS3XXX
> If unsure, say N.
>
> config MMC_SDHCI_ESDHC_IMX
> - bool "SDHCI platform support for the Freescale eSDHC i.MX controller"
> - depends on MMC_SDHCI_PLTFM && (ARCH_MX25 || ARCH_MX35 || ARCH_MX5)
> + bool "SDHCI support for the Freescale eSDHC i.MX controller"
> + depends on ARCH_MX25 || ARCH_MX35 || ARCH_MX5
> + depends on MMC_SDHCI_PLTFM || MMC_SDHCI_OF
> select MMC_SDHCI_IO_ACCESSORS
> help
> This selects the Freescale eSDHC controller support on the platform
> - bus, found on platforms like mx35/51.
> + or OF bus, found on platforms like mx35/51.
>
> If unsure, say N.
>
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index e834fb2..cca8dfd 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -39,7 +39,6 @@ obj-$(CONFIG_MMC_USHC) += ushc.o
> obj-$(CONFIG_MMC_SDHCI_PLTFM)+= sdhci-platform.o
> sdhci-platform-y := sdhci-pltfm.o
> sdhci-platform-$(CONFIG_MMC_SDHCI_CNS3XXX) += sdhci-cns3xxx.o
> -sdhci-platform-$(CONFIG_MMC_SDHCI_ESDHC_IMX) += sdhci-esdhc-imx.o
> sdhci-platform-$(CONFIG_MMC_SDHCI_DOVE) += sdhci-dove.o
> sdhci-platform-$(CONFIG_MMC_SDHCI_TEGRA) += sdhci-tegra.o
>
> @@ -48,6 +47,8 @@ sdhci-of-y := sdhci-of-core.o
> sdhci-of-$(CONFIG_MMC_SDHCI_OF_ESDHC)+= sdhci-of-esdhc.o
> sdhci-of-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o
>
> +obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX)+= sdhci-esdhc-imx.o
> +
> ifeq ($(CONFIG_CB710_DEBUG),y)
> CFLAGS-cb710-mmc+= -DDEBUG
> endif
> diff --git a/drivers/mmc/host/sdhci-of-core.c
> b/drivers/mmc/host/sdhci-of-core.c
> index 0b32ad7..d803fa5 100644
> --- a/drivers/mmc/host/sdhci-of-core.c
> +++ b/drivers/mmc/host/sdhci-of-core.c
> @@ -32,6 +32,9 @@
> #include "sdhci.h"
>
> static const struct of_device_id sdhci_of_match[] = {
> +#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
> + { .compatible = "fsl,imx-esdhc", .data = &sdhci_esdhc_imx_data, },
> +#endif
> #ifdef CONFIG_MMC_SDHCI_OF_ESDHC
> { .compatible = "fsl,mpc8379-esdhc", .data = &sdhci_esdhc_data, },
> { .compatible = "fsl,mpc8536-esdhc", .data = &sdhci_esdhc_data, },
> diff --git a/drivers/mmc/host/sdhci-of.h b/drivers/mmc/host/sdhci-of.h
> index e88fe2e..90fb2eb 100644
> --- a/drivers/mmc/host/sdhci-of.h
> +++ b/drivers/mmc/host/sdhci-of.h
> @@ -31,6 +31,7 @@ extern void sdhci_be32bs_writel(struct sdhci_host *host,
> u32 val, int reg);
> extern void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg);
> extern void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg);
>
> +extern struct sdhci_data sdhci_esdhc_imx_data;
> extern struct sdhci_data sdhci_esdhc_data;
> extern struct sdhci_data sdhci_hlwd_data;
>
> --
> 1.7.1
>
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 4/7] mmc: consolidate sdhci_pltfm_data and sdhci_of_data into one
On Mon, Mar 14, 2011 at 10:25:56PM +0800, Shawn Guo wrote:
> This patch is motivated by the work of supporting sdhci-esdhc-imx as
> an OF device. The sdhci-esdhc-imx driver was well designed to be
> able to work with either platform or OF bus, with a little effort to
> decouple the driver from sdhci_pltfm_data.
>
> Like sdhci_ops works for both platform and OF sdhci driver, the patch
> demonstrates that sdhci_pltfm_data and sdhci_of_data can be
> consolidated into sdhci_data, which should work for both. As one of
> the results, header linux/mmc/sdhci-pltfm.h can be deleted.
>
> Signed-off-by: Shawn Guo
I like the push to unify DT and non-DT usage with the SDHCI drivers,
but I'm not convinced on the approach. Actually, that's more a
comment on the existing code than it is on the this patch.
I don't like the way that sdhci-pltfm.c and sdhci-of-core.c each take
responsibility for the .probe hook on each of the implementations.
Doing it that way means that each implementation needs to add a set of
hooks into those core files protected by #ifdefs based on whether or
not the driver is enabled. It also means that loading one driver
means that all the configured sdhci drivers get loaded into the
kernel. This seems backwards.
>From what I can see, sdhci-pltfm.c and sdhci-of-core.c both provide
useful common functions that would be used by all sdhci host drivers.
The interface would be a lot cleaner if those routines were exported
for use by other modules, and each driver registered its own probe
hook. It would keep all the driver specific stuff out of
sdhci_pltfm_ids and I think it would result in a cleaner interface
overall.
Here is how I would do it:
1) break the bulk of the sdhci_pltfm.c and sdhci_of_core.c .probe and
.remove hooks out into separate functions; callable by other modules
2) for each driver, add its own probe and remove hooks which calls
into the new functions created in step 1. This would eliminate the
need for the ->exit and ->init callbacks since they would get rolled
into the new .probe and .remove callbacks
3) finally, when all drivers are removed; eliminate the driver
registration from sdhci_pltfm.c and sdhci_of_core.c because there
wouldn't be any users left.
drivers/mmc/host/sdhci-pxa.c appears to be a good example of how I
think sdhci platform_drivers should be structured.
At the same time, the functionality from sdhci_of_core.c could easily
be migrated into sdhci_pltfm.c.
g.
> ---
> drivers/mmc/host/sdhci-cns3xxx.c |3 +-
> drivers/mmc/host/sdhci-dove.c |2 +-
> drivers/mmc/host/sdhci-esdhc-imx.c | 19 ++---
> drivers/mmc/host/sdhci-of-core.c | 50
> +++-
> drivers/mmc/host/sdhci-of-esdhc.c | 28 +++-
> drivers/mmc/host/sdhci-of-hlwd.c | 20 --
> drivers/mmc/host/sdhci-of.h|9 +-
> drivers/mmc/host/sdhci-pltfm.c | 47 -
> drivers/mmc/host/sdhci-pltfm.h | 18 ++--
> drivers/mmc/host/sdhci-tegra.c | 10 +++---
> drivers/mmc/host/sdhci.h | 15 +++
> include/linux/mmc/sdhci-pltfm.h| 35 -
> 12 files changed, 123 insertions(+), 133 deletions(-)
> delete mode 100644 include/linux/mmc/sdhci-pltfm.h
>
> diff --git a/drivers/mmc/host/sdhci-cns3xxx.c
> b/drivers/mmc/host/sdhci-cns3xxx.c
> index 9ebd1d7..6d494d5 100644
> --- a/drivers/mmc/host/sdhci-cns3xxx.c
> +++ b/drivers/mmc/host/sdhci-cns3xxx.c
> @@ -15,7 +15,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include "sdhci.h"
> #include "sdhci-pltfm.h"
> @@ -86,7 +85,7 @@ static struct sdhci_ops sdhci_cns3xxx_ops = {
> .set_clock = sdhci_cns3xxx_set_clock,
> };
>
> -struct sdhci_pltfm_data sdhci_cns3xxx_pdata = {
> +struct sdhci_data sdhci_cns3xxx_data = {
> .ops = &sdhci_cns3xxx_ops,
> .quirks = SDHCI_QUIRK_BROKEN_DMA |
> SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
> index 2aeef4f..7d14402 100644
> --- a/drivers/mmc/host/sdhci-dove.c
> +++ b/drivers/mmc/host/sdhci-dove.c
> @@ -61,7 +61,7 @@ static struct sdhci_ops sdhci_dove_ops = {
> .read_l = sdhci_dove_readl,
> };
>
> -struct sdhci_pltfm_data sdhci_dove_pdata = {
> +struct sdhci_data sdhci_dove_data = {
> .ops= &sdhci_dove_ops,
> .quirks = SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER |
> SDHCI_QUIRK_NO_BUSY_IRQ |
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 9b82910..dfd1ccb 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -16,7 +16,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include "sdhci.h"
> #include "sdhci-pltfm.h"
> @@ -86,21 +85,21 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8
> val, int reg)
> esdhc_clrset_le(host, 0xff, val, r
Re: Yet another memory provider: can linaro organize a meeting?
On Tue, Mar 15, 2011 at 12:07 PM, Robert Fekete wrote: > On 8 March 2011 20:23, Laurent Pinchart > wrote: >> Hi Andy, >> >> On Tuesday 08 March 2011 20:12:45 Andy Walls wrote: >>> On Tue, 2011-03-08 at 16:52 +0100, Laurent Pinchart wrote: >>> >>> [snip] >>> >>> > > > It really shouldn't be that hard to get everyone involved together >>> > > > and settle on a single solution (either based on an existing >>> > > > proposal or create a 'the best of' vendor-neutral solution). >>> > > >>> > > "Single" might be making the problem impossibly hard to solve well. >>> > > One-size-fits-all solutions have a tendency to fall short on meeting >>> > > someone's critical requirement. I will agree that "less than n", for >>> > > some small n, is certainly desirable. >>> > > >>> > > The memory allocators and managers are ideally satisfying the >>> > > requirements imposed by device hardware, what userspace applications >>> > > are expected to do with the buffers, and system performance. (And >>> > > maybe the platform architecture, I/O bus, and dedicated video memory?) >>> > >>> > In the embedded world, a very common use case is to capture video data >>> > from an ISP (V4L2+MC), process it in a DSP (V4L2+M2M, tidspbridge, ...) >>> > and display it on the GPU (OpenGL/ES). We need to be able to share a >>> > data buffer between the ISP and the DSP, and another buffer between the >>> > DSP and the GPU. If processing is not required, sharing a data buffer >>> > between the ISP and the GPU is required. Achieving zero-copy requires a >>> > single memory management solution used by the ISP, the DSP and the GPU. >>> >>> Ah. I guess I misunderstood what was meant by "memory provider" to some >>> extent. >>> >>> So what I read is a common way of providing in kernel persistent buffers >>> (buffer objects? buffer entities?) for drivers and userspace >>> applications to pass around by reference (no copies). Userspace may or >>> may not want to see the contents of the buffer objects. >> >> Exactly. How that memory is allocated in irrelevant here, and we can have >> several different allocators as long as the buffer objects can be managed >> through a single API. That API will probably have to expose buffer properties >> related to allocation, in order for all components in the system to verify >> that the buffers are suitable for their needs, but the allocation process >> itself is irrelevant. >> >>> So I understand now why a single solution is desirable. >> > > Exactly, > > It is important to know that there are 3 topics of discussion which > all are a separate topic of its own: > > 1. The actual memory allocator > 2. In-kernel API > 3. Userland API > > Explained: > 1. This is how you acquire the actual physical or virtual memory, > defrag, swap, etc. This can be enhanced by CMA, hotswap, memory > regions or whatever and the main topic for a system wide memory > allocator does not deal much with how this is done. > 2. In-kernel API is important from a device driver point of view in > order to resolve buffers, pin memory when used(enable defrag when > unpinned) > 3. Userland API deals with alloc/free, import/export(IPC), security, > and set-domain capabilities among others and is meant to pass buffers > between processes in userland and enable no-copy data paths. > > We need to resolve 2. and 3. > > GEM/TTM is mentioned in this thread and there is an overlap of what is > happening within DRM/DRI/GEM/TTM/KMS and V4L2. The whole idea behind > DRM is to have one device driver for everything (well at least 2D/3D, > video codecs, display output/composition), while on a SoC all this is > on several drivers/IP's. A V4L2 device cannot resolve a GEM handle. > GEM only lives inside one DRM device (AFAIK). GEM is also mainly for > "dedicated memory-less" graphics cards while TTM mainly targets > advanced Graphics Card with dedicated memory. From a SoC point of view > DRM looks very "fluffy" and not quite slimmed for an embedded device, > and you cannot get GEM/TTM without bringing in all of DRM/DRI. KMS on > the other hand is very attractive as a framebuffer device replacer. It > is not an easy task to decide on a multimedia user interface for a SoC > vendor. Modern GPUs are basically an SoC: 3D engine, video decode, hdmi packet engines, audio, dma engine, display blocks, etc. with a shared memory controller. Also the AMD fusion and Intel moorestown SoCs are not too different from ARM-based SoCs and we are supporting them with the drm. I expect we'll see the x86 and ARM/MIPS based SoCs continue to get closer together. What are you basing your "fluffy" statement on? We recently merged a set of patches from qualcomm to support platform devices in the drm and Dave added support for USB devices. Qualcomm also has an open source drm for their snapdragon GPUs (although the userspace driver is closed) and they are using that on their SoCs. > > Uniting the frameworks within the kernel will likely fail(too big of a > task) but a common system w
Re: Yet another memory provider: can linaro organize a meeting?
On Tue, Mar 15, 2011 at 9:07 AM, Robert Fekete wrote: > On 8 March 2011 20:23, Laurent Pinchart > wrote: > > Hi Andy, > > > > On Tuesday 08 March 2011 20:12:45 Andy Walls wrote: > >> On Tue, 2011-03-08 at 16:52 +0100, Laurent Pinchart wrote: > >> > >> [snip] > >> > >> > > > It really shouldn't be that hard to get everyone involved together > >> > > > and settle on a single solution (either based on an existing > >> > > > proposal or create a 'the best of' vendor-neutral solution). > >> > > > >> > > "Single" might be making the problem impossibly hard to solve well. > >> > > One-size-fits-all solutions have a tendency to fall short on meeting > >> > > someone's critical requirement. I will agree that "less than n", > for > >> > > some small n, is certainly desirable. > >> > > > >> > > The memory allocators and managers are ideally satisfying the > >> > > requirements imposed by device hardware, what userspace applications > >> > > are expected to do with the buffers, and system performance. (And > >> > > maybe the platform architecture, I/O bus, and dedicated video > memory?) > >> > > >> > In the embedded world, a very common use case is to capture video data > >> > from an ISP (V4L2+MC), process it in a DSP (V4L2+M2M, tidspbridge, > ...) > >> > and display it on the GPU (OpenGL/ES). We need to be able to share a > >> > data buffer between the ISP and the DSP, and another buffer between > the > >> > DSP and the GPU. If processing is not required, sharing a data buffer > >> > between the ISP and the GPU is required. Achieving zero-copy requires > a > >> > single memory management solution used by the ISP, the DSP and the > GPU. > >> > >> Ah. I guess I misunderstood what was meant by "memory provider" to some > >> extent. > >> > >> So what I read is a common way of providing in kernel persistent buffers > >> (buffer objects? buffer entities?) for drivers and userspace > >> applications to pass around by reference (no copies). Userspace may or > >> may not want to see the contents of the buffer objects. > > > > Exactly. How that memory is allocated in irrelevant here, and we can have > > several different allocators as long as the buffer objects can be managed > > through a single API. That API will probably have to expose buffer > properties > > related to allocation, in order for all components in the system to > verify > > that the buffers are suitable for their needs, but the allocation process > > itself is irrelevant. > > > >> So I understand now why a single solution is desirable. > > > > Exactly, > > It is important to know that there are 3 topics of discussion which > all are a separate topic of its own: > > 1. The actual memory allocator > 2. In-kernel API > 3. Userland API > > Explained: > 1. This is how you acquire the actual physical or virtual memory, > defrag, swap, etc. This can be enhanced by CMA, hotswap, memory > regions or whatever and the main topic for a system wide memory > allocator does not deal much with how this is done. > 2. In-kernel API is important from a device driver point of view in > order to resolve buffers, pin memory when used(enable defrag when > unpinned) > 3. Userland API deals with alloc/free, import/export(IPC), security, > and set-domain capabilities among others and is meant to pass buffers > between processes in userland and enable no-copy data paths. > > We need to resolve 2. and 3. > > GEM/TTM is mentioned in this thread and there is an overlap of what is > happening within DRM/DRI/GEM/TTM/KMS and V4L2. The whole idea behind > DRM is to have one device driver for everything (well at least 2D/3D, > video codecs, display output/composition), while on a SoC all this is > on several drivers/IP's. A V4L2 device cannot resolve a GEM handle. > GEM only lives inside one DRM device (AFAIK). GEM is also mainly for > "dedicated memory-less" graphics cards while TTM mainly targets > advanced Graphics Card with dedicated memory. From a SoC point of view > DRM looks very "fluffy" and not quite slimmed for an embedded device, > and you cannot get GEM/TTM without bringing in all of DRM/DRI. KMS on > the other hand is very attractive as a framebuffer device replacer. It > is not an easy task to decide on a multimedia user interface for a SoC > vendor. > > Uniting the frameworks within the kernel will likely fail(too big of a > task) but a common system wide memory manager would for sure make life > easier enabling the possibility to pass buffers between drivers(and > user-land as well). In order for No-copy to work on a system level the > general multimedia infrastructure in User-land (i.e. > Gstreamer/X11/wayland/stagefright/flingers/etc) must also be aware of > this memory manager and manage handles accordingly. This > infrastructure in user-land puts the requirements on the User land API > (1.). > > I know that STE and ARM has a vision to have a hwmem/ump alike API and > that Linaro is one place to resolve this. As Jesse Barker mentioned > earlier Linaro has work ongoin
Re: LAVA integration effort for Android validation
> I like statically IP. How do we know which board would get which IP? > One idea would be to set up a DNS for internal board names. e.g. > panda01.internal.network would resolve to our preallocated static IP > for board "panda01". > > This could then be used by the dispatcher to resolve the IP and pass > it to the board through kernel cmdline. > > I hadn't really thought of passing it via cmdline. If we need to bring it up statically, we could possibly do that, or possibly just make the necessary adjustments before booting the image. What I was proposing, if the image is already set up to dhcp by default, we could run a dhcp server on the control node with the static ip assignments already in place for each board. That way they always get the same address that we pre-allocated for that board. We'll also need to make sure that the control node has a valid ip on both the internal and the external network. Thanks, Paul Larson ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: Getting rid of update-alternative in cross toolchain packages
On Tue, Mar 15, 2011 at 01:00:04PM +0100, Marcin Juszkiewicz wrote: > Some time ago I looked at cross toolchain packages to find how they > handle installation of several versions at same time. It was done by > using 'update-alternatives' tool and was broken. We fixed it during > Ubuntu/Maverick cycle but "u-a" is still in use (just versions are used > now to take care of priority so latest gcc is default one). > But this is different then native gcc which is selected by gcc-defaults > package - /usr/bin/gcc is symlink to /usr/bin/gcc-DEFAULTVERSION (where > DEFAULTVERSION value depends on architecture and distribution). Ok, we > have gcc-defaults-armel-cross in Ubuntu but it takes care only of > depending on default versions of cross toolchain components. > I filled a bug [1] about it and discussed it with Matthias Klose. The > proper way would consists those steps: > - new cross packages will use "u-a remove" to get rid of old alternative > stuff (in postinst and prerm) > - new gcc-defaults-armel-cross package will provide > /usr/bin/arm-linux-gnueabi-APP symlinks > - new gcc-defaults-armel-cross will also conflicts with older versions > of cross toolchain packages From the bug report: 4. alter postinst/prerm of gcc-4.[45]-armel-cross to remove old u-a: TODO This should be done in the preinst instead. Rationale: update-alternatives is part of dpkg, so doesn't require any Pre-Depends; and a dependency (such as the gcc-arm-linux-gnueabi dependency on gcc-4.5-arm-linux-gnueabi) does not prevent one package from being unpacked before the postinst of another package it depends on has been run. So if you do this in the postinst, you get: - gcc-4.5-arm-linux-gnueabi unpacked (enforced by the Conflicts: from gcc-arm-linux-gnueabi) - gcc-arm-linux-gnueabi unpacked; overwrites /usr/bin/arm-linux-gnueabi-gcc symlink - gcc-4.5-arm-linux-gnueabi configured; u-a remove runs, removing the symlink - gcc-arm-linux-gnueabi configured - but the gcc symlink is now missing Otherwise, this looks ok. -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. Ubuntu Developerhttp://www.debian.org/ slanga...@ubuntu.com vor...@debian.org signature.asc Description: Digital signature ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: Yet another memory provider: can linaro organize a meeting?
On 8 March 2011 20:23, Laurent Pinchart wrote: > Hi Andy, > > On Tuesday 08 March 2011 20:12:45 Andy Walls wrote: >> On Tue, 2011-03-08 at 16:52 +0100, Laurent Pinchart wrote: >> >> [snip] >> >> > > > It really shouldn't be that hard to get everyone involved together >> > > > and settle on a single solution (either based on an existing >> > > > proposal or create a 'the best of' vendor-neutral solution). >> > > >> > > "Single" might be making the problem impossibly hard to solve well. >> > > One-size-fits-all solutions have a tendency to fall short on meeting >> > > someone's critical requirement. I will agree that "less than n", for >> > > some small n, is certainly desirable. >> > > >> > > The memory allocators and managers are ideally satisfying the >> > > requirements imposed by device hardware, what userspace applications >> > > are expected to do with the buffers, and system performance. (And >> > > maybe the platform architecture, I/O bus, and dedicated video memory?) >> > >> > In the embedded world, a very common use case is to capture video data >> > from an ISP (V4L2+MC), process it in a DSP (V4L2+M2M, tidspbridge, ...) >> > and display it on the GPU (OpenGL/ES). We need to be able to share a >> > data buffer between the ISP and the DSP, and another buffer between the >> > DSP and the GPU. If processing is not required, sharing a data buffer >> > between the ISP and the GPU is required. Achieving zero-copy requires a >> > single memory management solution used by the ISP, the DSP and the GPU. >> >> Ah. I guess I misunderstood what was meant by "memory provider" to some >> extent. >> >> So what I read is a common way of providing in kernel persistent buffers >> (buffer objects? buffer entities?) for drivers and userspace >> applications to pass around by reference (no copies). Userspace may or >> may not want to see the contents of the buffer objects. > > Exactly. How that memory is allocated in irrelevant here, and we can have > several different allocators as long as the buffer objects can be managed > through a single API. That API will probably have to expose buffer properties > related to allocation, in order for all components in the system to verify > that the buffers are suitable for their needs, but the allocation process > itself is irrelevant. > >> So I understand now why a single solution is desirable. > Exactly, It is important to know that there are 3 topics of discussion which all are a separate topic of its own: 1. The actual memory allocator 2. In-kernel API 3. Userland API Explained: 1. This is how you acquire the actual physical or virtual memory, defrag, swap, etc. This can be enhanced by CMA, hotswap, memory regions or whatever and the main topic for a system wide memory allocator does not deal much with how this is done. 2. In-kernel API is important from a device driver point of view in order to resolve buffers, pin memory when used(enable defrag when unpinned) 3. Userland API deals with alloc/free, import/export(IPC), security, and set-domain capabilities among others and is meant to pass buffers between processes in userland and enable no-copy data paths. We need to resolve 2. and 3. GEM/TTM is mentioned in this thread and there is an overlap of what is happening within DRM/DRI/GEM/TTM/KMS and V4L2. The whole idea behind DRM is to have one device driver for everything (well at least 2D/3D, video codecs, display output/composition), while on a SoC all this is on several drivers/IP's. A V4L2 device cannot resolve a GEM handle. GEM only lives inside one DRM device (AFAIK). GEM is also mainly for "dedicated memory-less" graphics cards while TTM mainly targets advanced Graphics Card with dedicated memory. From a SoC point of view DRM looks very "fluffy" and not quite slimmed for an embedded device, and you cannot get GEM/TTM without bringing in all of DRM/DRI. KMS on the other hand is very attractive as a framebuffer device replacer. It is not an easy task to decide on a multimedia user interface for a SoC vendor. Uniting the frameworks within the kernel will likely fail(too big of a task) but a common system wide memory manager would for sure make life easier enabling the possibility to pass buffers between drivers(and user-land as well). In order for No-copy to work on a system level the general multimedia infrastructure in User-land (i.e. Gstreamer/X11/wayland/stagefright/flingers/etc) must also be aware of this memory manager and manage handles accordingly. This infrastructure in user-land puts the requirements on the User land API (1.). I know that STE and ARM has a vision to have a hwmem/ump alike API and that Linaro is one place to resolve this. As Jesse Barker mentioned earlier Linaro has work ongoing on this topic (https://wiki.linaro.org/WorkingGroups/Middleware/Graphics/Projects/UnifiedMemoryManagement) and a V4L2 brainstorming meeting in Warsaw will likely bring this up as well. And Gstreamer is also looking at this from a user-land point of view. ARM
Re: LAVA integration effort for Android validation
On Tue, Mar 15, 2011 at 4:09 PM, Paul Larson wrote: > On Mon, Mar 14, 2011 at 11:22 AM, Jeremy Chang > wrote: >> >> Hi, list: >> I am working on Validation part for Android, in which mainly >> inclusive of integrating existing benchmark and testing suites into >> abrek, like 0xbench and android CTS. For now in my thought, if these >> benchmark/testing can integrate with abrek, then it should be no much >> problem or extra effort for integrating further into LAVA. > > As discussed before, I think abrek is probably not the right choice for > running tests under android. Abrek was designed to work on linaro images > running python in the client, specifically it was designed to run under > Linaro or Ubuntu images, before we had even considered android at all. We > probably need some kind of mechanism for running the tests from the server > over adb instead, then outputting the results into a json bundle that the > dashboard can understand. we talked about this on IRC. I pointed Jeremy to our lp:lava branch and explained to him the theory of how things work. We agreed to first write a few example job descriptions and then decide what features a generic testsuite framework would require (if we need this at all). Also we might do a prototype to learn how things would work if we implemented the actions for android deploy, boot and test run. > >> >> Though I have some questions regarding to validation. >> >> How to detect the early fail through serial console? like >> detecting the failure during kernel booting stage also before running >> init or getting the shell. I think this will be a job issued by the >> dispatcher though I don't know how this checking mechanism will be >> done. Would there be anything here needed to do specifically for >> Android? > > Right, the dispatcher currently handles this. We are using pexpect to drive > things over the serial console. So once the system is booting, we expect to > see a valid shell prompt within some reasonable timeout period. If we > don't, then we know the boot failed. I expect the bit that detects the > shell prompt may need to be changed somewhat for android. Perhaps for > android we need to wait for a valid adb devices output? or a working adb > shell? yes, its a timeout thing. we should give the adb connect a reasonable timeout. In this way we can guess that the boot failed in a bad way as no network came up. > >> >> How to connect the devices in the farm? I checked the wiki page, >> Platform/Validation/Specs/HardwareSetup [1]. The network will be used. >> I am thinking what's needed for setting up a network environment for >> this? The device is needed to fetch a fixed ip by dhcp just after >> booting up? USB gadget is an alternative for Android and in most >> situation could be more convenient for personal testing. adb (Android >> Debug Bridge, running on host side) can connect to a device through >> USB or TCP/IP. > > USB gadget is hard for us, if we have a lot of machines. Network would be > better, but can we rely in it to be working? Maybe we just make that a > requirement for running android tests. I don't currently have it set up, > but my current thinking is that we want to define a new network for the > validation machines, perhaps 10.1.x.x, and set up static assignments for > them in a dhcp server running on the control node. This way, they could > either be configured statically, or via dhcp. yes, USB gadget is not that great for our current setup. I like statically IP. How do we know which board would get which IP? One idea would be to set up a DNS for internal board names. e.g. panda01.internal.network would resolve to our preallocated static IP for board "panda01". This could then be used by the dispatcher to resolve the IP and pass it to the board through kernel cmdline. -- - Alexander ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: gcc: Thumb interworking and weakly linked functions
Aneesh V wrote:
> I was trying to build u-boot in Thumb2 for OMAP4. Everything was fine
> until I added some patches recently. One of these patches introduced an
> API (let's say foo()) that has a weakly linked alias(let's say
> __foo()) and a strongly linked implementation(the real foo()) in an
> assembly file.
>
> Although I give -mthumb and -mthumb-interwork for all the files,
> apparently GCC generates ARM code for assembly files. In the final
> image foobar() calls foo() using a BL. Since foobar() is in Thumb and
> foo() in ARM, it ends up crashing. Looks like foobar() assumed foo()
> to be Thumb because __foo() is Thumb.
I'm unable to reproduce this. Do you have a complete test case?
I've tried with the following small example:
foo1.c:
extern void foo (void) __attribute__ ((weak, alias ("__foo")));
void __foo (void)
{
}
int main (void)
{
foo ();
}
foo2.S:
.text
.align 2
.global foo
.type foo, %function
foo:
push{r7}
add r7, sp, #0
mov sp, r7
pop {r7}
bx lr
.size foo, .-foo
When building just "gcc foo1.c", I get:
835c <__foo>:
835c: b480push{r7}
835e: af00add r7, sp, #0
8360: 46bdmov sp, r7
8362: bc80pop {r7}
8364: 4770bx lr
8366: bf00nop
8368 :
8368: b580push{r7, lr}
836a: af00add r7, sp, #0
836c: f7ff fff6 bl 835c <__foo>
8370: 4618mov r0, r3
8372: bd80pop {r7, pc}
When building both files "gcc foo1.c foo2.S", I get instead:
8368 :
8368: b580push{r7, lr}
836a: af00add r7, sp, #0
836c: f000 e802 blx 8374
8370: 4618mov r0, r3
8372: bd80pop {r7, pc}
8374 :
8374: e92d0080push{r7}
8378: e28d7000add r7, sp, #0
837c: e1a0d007mov sp, r7
8380: e8bd0080pop {r7}
8384: e12fff1ebx lr
So it seems to me the linker is handling this correctly ...
(This is on Ubuntu Natty using system gcc and binutils.)
Mit freundlichen Gruessen / Best Regards
Ulrich Weigand
--
Dr. Ulrich Weigand | Phone: +49-7031/16-3727
STSM, GNU compiler and toolchain for Linux on System z and Cell/B.E.
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter | Geschäftsführung: Dirk
Wittkopp
Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht
Stuttgart, HRB 243294
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: LAVA integration effort for Android validation
On Mon, Mar 14, 2011 at 11:22 AM, Jeremy Chang wrote: > Hi, list: >I am working on Validation part for Android, in which mainly > inclusive of integrating existing benchmark and testing suites into > abrek, like 0xbench and android CTS. For now in my thought, if these > benchmark/testing can integrate with abrek, then it should be no much > problem or extra effort for integrating further into LAVA. > As discussed before, I think abrek is probably not the right choice for running tests under android. Abrek was designed to work on linaro images running python in the client, specifically it was designed to run under Linaro or Ubuntu images, before we had even considered android at all. We probably need some kind of mechanism for running the tests from the server over adb instead, then outputting the results into a json bundle that the dashboard can understand. > >Though I have some questions regarding to validation. > >How to detect the early fail through serial console? like > detecting the failure during kernel booting stage also before running > init or getting the shell. I think this will be a job issued by the > dispatcher though I don't know how this checking mechanism will be > done. Would there be anything here needed to do specifically for > Android? > Right, the dispatcher currently handles this. We are using pexpect to drive things over the serial console. So once the system is booting, we expect to see a valid shell prompt within some reasonable timeout period. If we don't, then we know the boot failed. I expect the bit that detects the shell prompt may need to be changed somewhat for android. Perhaps for android we need to wait for a valid adb devices output? or a working adb shell? > >How to connect the devices in the farm? I checked the wiki page, > Platform/Validation/Specs/HardwareSetup [1]. The network will be used. > I am thinking what's needed for setting up a network environment for > this? The device is needed to fetch a fixed ip by dhcp just after > booting up? USB gadget is an alternative for Android and in most > situation could be more convenient for personal testing. adb (Android > Debug Bridge, running on host side) can connect to a device through > USB or TCP/IP. > USB gadget is hard for us, if we have a lot of machines. Network would be better, but can we rely in it to be working? Maybe we just make that a requirement for running android tests. I don't currently have it set up, but my current thinking is that we want to define a new network for the validation machines, perhaps 10.1.x.x, and set up static assignments for them in a dhcp server running on the control node. This way, they could either be configured statically, or via dhcp. Thanks, Paul Larson ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH] [android/devices/linaro/common] Correct permission, owner and group on files in the root tarball
On Tue, Mar 15, 2011 at 2:35 PM, Patrik Ryd wrote: > On 15 March 2011 12:28, Alexander Sack wrote: >> >> would be great to have a more verbose git comment as of why we are >> doing this. e.g. what was the problem and what was the solution. also >> why do we need to have a copy of mktarball.sh rather than fixing that >> one directly. > > This patch fixes the permission problem in our root tarball. I think we > should try come > to a conclusion on how we are going to handle the kernel for the different > builds first > and then start building the boot tarball instead of our root tarball. I agree. Let's get our boottarball/kernel story sorted out this week. I would like to see us being able to repo sync our various kernels and use the android platform build system to build the kernel + the boottarballs. In this way we can reuse our build service more or less without modifications. Anyway, until that happens, feel free to push this patch! -- - Alexander ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH] [android/devices/linaro/common] Correct permission, owner and group on files in the root tarball
On 15 March 2011 12:28, Alexander Sack wrote:
> would be great to have a more verbose git comment as of why we are
> doing this. e.g. what was the problem and what was the solution. also
> why do we need to have a copy of mktarball.sh rather than fixing that
> one directly.
>
This patch fixes the permission problem in our root tarball. I think we
should try come
to a conclusion on how we are going to handle the kernel for the different
builds first
and then start building the boot tarball instead of our root tarball.
When we use the boot tarball it make sense moving this correction (for the
rootfs) into
the android version of mktarball.sh and contribute it.
> On Fri, Mar 11, 2011 at 4:05 PM, Patrik Ryd wrote:
> > Fix for LP #731780.
> > ---
> > tasks/mktarball.sh | 60
>
> > tasks/tarballs.mk |4 ++-
> > 2 files changed, 63 insertions(+), 1 deletions(-)
> > create mode 100755 tasks/mktarball.sh
> >
> > diff --git a/tasks/mktarball.sh b/tasks/mktarball.sh
> > new file mode 100755
> > index 000..622ff47
> > --- /dev/null
> > +++ b/tasks/mktarball.sh
> > @@ -0,0 +1,60 @@
> > +#!/bin/bash
> > +
> > +# This is a modified copy of build/tools/mktarball.sh
> > +
> > +# $1: path to fs_get_stats program
> > +# $2: start dir
> > +# $3: subdir to tar up (from $2)
> > +# $4: target tar name
> > +# $5: target tarball name (usually $(3).bz2)
> > +
> > +if [ $# -ne 5 ]; then
> > +echo "Error: wrong number of arguments in cmd: $0 $* "
> > +exit 1
> > +fi
> > +
> > +fs_get_stats=`readlink -f $1`
> > +start_dir=`readlink -f $2`
> > +dir_to_tar=$3
> > +target_tar=`readlink -f $4`
> > +target_tarball=`readlink -f $5`
> > +
> > +cd $2
> > +
> > +#tar --no-recursion -cvf ${target_tar} ${dir_to_tar}
> > +rm ${target_tar} > /dev/null 2>&1
> > +
> > +# do dirs first
> > +subdirs=`find ${dir_to_tar} -type d -print`
> > +files=`find ${dir_to_tar} \! -type d -print`
> > +for f in ${subdirs} ${files} ; do
> > +curr_perms=`stat -c 0%a $f`
> > +[ -d "$f" ] && is_dir=1 || is_dir=0
> > +f2=`echo ${f#*/}`
> > +new_info=`${fs_get_stats} ${curr_perms} ${is_dir} ${f2}`
> > +new_uid=`echo ${new_info} | awk '{print $1;}'`
> > +new_gid=`echo ${new_info} | awk '{print $2;}'`
> > +new_perms=`echo ${new_info} | awk '{print $3;}'`
> > +#echo "$f: dir: $is_dir curr: $curr_perms uid: $new_uid gid:
> $new_gid "\
> > +# "perms: $new_perms"
> > +tar --no-recursion --numeric-owner --owner $new_uid \
> > +--group $new_gid --mode $new_perms -p -rf ${target_tar} ${f}
> > +done
> > +
> > +if [ $? -eq 0 ] ; then
> > +case "${target_tarball}" in
> > +*.bz2 )
> > +bzip2 -c ${target_tar} > ${target_tarball}
> > +;;
> > +*.gz )
> > +gzip -c ${target_tar} > ${target_tarball}
> > +;;
> > +esac
> > +success=$?
> > +[ $success -eq 0 ] || rm -f ${target_tarball}
> > +rm -f ${target_tar}
> > +exit $success
> > +fi
> > +
> > +rm -f ${target_tar}
> > +exit 1
> > diff --git a/tasks/tarballs.mk b/tasks/tarballs.mk
> > index e569e98..e6354fc 100644
> > --- a/tasks/tarballs.mk
> > +++ b/tasks/tarballs.mk
> > @@ -2,11 +2,13 @@
> > # Trigger build of tar balls for the linaro boards
> > #
> >
> > +LINARO_MKTARBALL := device/linaro/common/tasks/mktarball.sh
> > +
> > ###
> > ## root tarball
> > define build-roottarball-target
> > $(hide) echo "Target root fs tarball:"
> $(INSTALLED_ROOTTARBALL_TARGET)
> > -$(hide) $(MKTARBALL) $(FS_GET_STATS) \
> > +$(hide) $(LINARO_MKTARBALL) $(FS_GET_STATS) \
> > $(PRODUCT_OUT)/root . $(PRIVATE_ROOT_TAR) \
> > $(INSTALLED_ROOTTARBALL_TARGET)
> > endef
> > --
> > 1.7.1
> >
> >
> > ___
> > linaro-dev mailing list
> > linaro-dev@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/linaro-dev
> >
>
>
>
> --
>
> - Alexander
>
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Getting rid of update-alternative in cross toolchain packages
Some time ago I looked at cross toolchain packages to find how they handle installation of several versions at same time. It was done by using 'update-alternatives' tool and was broken. We fixed it during Ubuntu/Maverick cycle but "u-a" is still in use (just versions are used now to take care of priority so latest gcc is default one). But this is different then native gcc which is selected by gcc-defaults package - /usr/bin/gcc is symlink to /usr/bin/gcc-DEFAULTVERSION (where DEFAULTVERSION value depends on architecture and distribution). Ok, we have gcc-defaults-armel-cross in Ubuntu but it takes care only of depending on default versions of cross toolchain components. I filled a bug [1] about it and discussed it with Matthias Klose. The proper way would consists those steps: - new cross packages will use "u-a remove" to get rid of old alternative stuff (in postinst and prerm) - new gcc-defaults-armel-cross package will provide /usr/bin/arm-linux-gnueabi-APP symlinks - new gcc-defaults-armel-cross will also conflicts with older versions of cross toolchain packages Getting rid of symlinks was accepted during Emdebian sprint so such change will be done in gcc-4.[3456] source packages. gcc-defaults for other cross architectures then armel will need to be done for Emdebian. What do you think about such plan? 1. https://bugs.launchpad.net/bugs/676454 ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH] arm/dt: Add dtb make rule
2011/3/15 Rob Herring : > Grant, > > On 03/10/2011 01:46 PM, Rob Herring wrote: >> >> From: Rob Herring >> >> Add a make rule to compile dt blobs for ARM. >> >> Signed-off-by: Rob Herring > > Can you pick this one up in your ARM tree. Test result on i.MX51 babbage board: gcl/linux-2.6$ make babbage.dtb ARCH=arm CROSS_COMPILE=arm-none-linux-gnueabi- DTC arch/arm/boot/babbage.dtb DTC: dts->dtb on file "arch/arm/boot/dts/babbage.dts" Tested-by: Jason Liu Jason > > Rob > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: Better reviews for the same cost in gcc-linaro
On Tue, Mar 15, 2011, Andrew Stubbs wrote: > I find that bzr is slow - there's no getting around it, but there > are some tricks that can help. Thanks for sharing your tips; also noteworthy, this tip by Martin Pool: http://lists.linaro.org/pipermail/linaro-dev/2011-March/003282.html > I find "bzr push" is quite fast, but there's a special gotcha - it > always stacks new feature branches on top of the gcc-linaro/4.5 > branch (more accurately, the "focus of development" branch), and if > you're working with 4.4 or 4.6, that means there quite a lot of > difference to upload. Actually, I have a cron jobs which pushes the revisions of the 4.4 branch in the 4.5 repo on Launchpad; I just realized that this was still an issue from your email, so I've looked at the script again and it wasn't dealing with 4.6; I've added a step to copy the 4.6 revs into the 4.5 repo every night, so that should make bzr push fast again for you. (I'm still pushing the revs though, will take a while.) > > 5) wait for branch to be processed by launchpad (only a few minutes, > > nothing major) I don't :-) > > 7) merge to trunk (with the inevitable ChangeLog merge failure > > that you mentioned). bzr has plugins to merge changelog entries for some types of changelogs; I wonder whether we could use these here. Another option would be to generate a GNU ChangeLog from the bzr log at release time as we do for linaro-image-tools for instance. -- Loïc Minier ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH] [android/devices/linaro/common] Correct permission, owner and group on files in the root tarball
would be great to have a more verbose git comment as of why we are
doing this. e.g. what was the problem and what was the solution. also
why do we need to have a copy of mktarball.sh rather than fixing that
one directly.
On Fri, Mar 11, 2011 at 4:05 PM, Patrik Ryd wrote:
> Fix for LP #731780.
> ---
> tasks/mktarball.sh | 60
>
> tasks/tarballs.mk | 4 ++-
> 2 files changed, 63 insertions(+), 1 deletions(-)
> create mode 100755 tasks/mktarball.sh
>
> diff --git a/tasks/mktarball.sh b/tasks/mktarball.sh
> new file mode 100755
> index 000..622ff47
> --- /dev/null
> +++ b/tasks/mktarball.sh
> @@ -0,0 +1,60 @@
> +#!/bin/bash
> +
> +# This is a modified copy of build/tools/mktarball.sh
> +
> +# $1: path to fs_get_stats program
> +# $2: start dir
> +# $3: subdir to tar up (from $2)
> +# $4: target tar name
> +# $5: target tarball name (usually $(3).bz2)
> +
> +if [ $# -ne 5 ]; then
> + echo "Error: wrong number of arguments in cmd: $0 $* "
> + exit 1
> +fi
> +
> +fs_get_stats=`readlink -f $1`
> +start_dir=`readlink -f $2`
> +dir_to_tar=$3
> +target_tar=`readlink -f $4`
> +target_tarball=`readlink -f $5`
> +
> +cd $2
> +
> +#tar --no-recursion -cvf ${target_tar} ${dir_to_tar}
> +rm ${target_tar} > /dev/null 2>&1
> +
> +# do dirs first
> +subdirs=`find ${dir_to_tar} -type d -print`
> +files=`find ${dir_to_tar} \! -type d -print`
> +for f in ${subdirs} ${files} ; do
> + curr_perms=`stat -c 0%a $f`
> + [ -d "$f" ] && is_dir=1 || is_dir=0
> + f2=`echo ${f#*/}`
> + new_info=`${fs_get_stats} ${curr_perms} ${is_dir} ${f2}`
> + new_uid=`echo ${new_info} | awk '{print $1;}'`
> + new_gid=`echo ${new_info} | awk '{print $2;}'`
> + new_perms=`echo ${new_info} | awk '{print $3;}'`
> +# echo "$f: dir: $is_dir curr: $curr_perms uid: $new_uid gid: $new_gid "\
> +# "perms: $new_perms"
> + tar --no-recursion --numeric-owner --owner $new_uid \
> + --group $new_gid --mode $new_perms -p -rf ${target_tar} ${f}
> +done
> +
> +if [ $? -eq 0 ] ; then
> + case "${target_tarball}" in
> + *.bz2 )
> + bzip2 -c ${target_tar} > ${target_tarball}
> + ;;
> + *.gz )
> + gzip -c ${target_tar} > ${target_tarball}
> + ;;
> + esac
> + success=$?
> + [ $success -eq 0 ] || rm -f ${target_tarball}
> + rm -f ${target_tar}
> + exit $success
> +fi
> +
> +rm -f ${target_tar}
> +exit 1
> diff --git a/tasks/tarballs.mk b/tasks/tarballs.mk
> index e569e98..e6354fc 100644
> --- a/tasks/tarballs.mk
> +++ b/tasks/tarballs.mk
> @@ -2,11 +2,13 @@
> # Trigger build of tar balls for the linaro boards
> #
>
> +LINARO_MKTARBALL := device/linaro/common/tasks/mktarball.sh
> +
> ###
> ## root tarball
> define build-roottarball-target
> $(hide) echo "Target root fs tarball:" $(INSTALLED_ROOTTARBALL_TARGET)
> - $(hide) $(MKTARBALL) $(FS_GET_STATS) \
> + $(hide) $(LINARO_MKTARBALL) $(FS_GET_STATS) \
> $(PRODUCT_OUT)/root . $(PRIVATE_ROOT_TAR) \
> $(INSTALLED_ROOTTARBALL_TARGET)
> endef
> --
> 1.7.1
>
>
> ___
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
--
- Alexander
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: Better reviews for the same cost in gcc-linaro
Richard may know all this, but I'm just going to post anyway in case some people reading can benefit from some tips. I find that bzr is slow - there's no getting around it, but there are some tricks that can help. On 14/03/11 11:12, Richard Sandiford wrote: My concern was that, again with my way of working, the process is: 1) develop fix 2) branch trunk (creating a whole new gcc source tree, so quite slow) This is going to take a short while however you cut it, but doing it the naive way is *very* slow. So, make sure you use "init-repo". This creates a top-level ".bzr" directory that can be shared between many branches. This cuts down on network usage, and speeds up local branching also. To set it up: mkdir my_work_dir cd my_work_dir bzr init-repo . bzr branch lp:gcc-linaro my_1st_branch bzr branch lp:gcc-linaro my_2nd_branch Basically, any branches you create within sub-directories of "my_work_dir" have shared history, so there's no need to waste time duplicating it. I believe hard-linking should be a win too, but I don't use it much: bzr branch --hardlink my_1st_branch my_2nd_branch You might also try experimenting with local stacked branches (so the new one has only shallow history), but I'm not sure there's any advantage if you use a shared repo: bzr branch --stacked my_1st_branch my_2nd_branch 3) apply fix to new branch, with ChangeLog entry 4) publish new branch in laucnhpad I find "bzr push" is quite fast, but there's a special gotcha - it always stacks new feature branches on top of the gcc-linaro/4.5 branch (more accurately, the "focus of development" branch), and if you're working with 4.4 or 4.6, that means there quite a lot of difference to upload. You can fix this by telling it what branch to stack on: bzr push --stacked-on=bzr+ssh://bazaar.launchpad.net/~linaro-toolchain-dev/gcc-linaro/4.6 lp:~/gcc-linaro/ Unfortunately, the "lp:..." form doesn't work with --stacked-on. :( 5) wait for branch to be processed by launchpad (only a few minutes, nothing major) 6) ask for a review 7) merge to trunk (with the inevitable ChangeLog merge failure that you mentioned). whereas the upstream way would be: 1) develop fix 2) ask for a review, attaching patch 3) apply patch to trunk, with ChangeLog entry The upstream way feels much simpler, and avoids the merge failure hassle. True. If you prefer, by all means include the ChangeLog entry in the merge request, and it can be inserted into ChangeLog.linaro at merge time. It makes no real difference to me, but it does mean that if there is anybody out there pulling toolchains from feature branches, the ChangeLogs won't be helpful. I doubt that anybody does that??? Short story is that we have a better tool than svn, so feature branches may make some use cases overall easier and more transparent. Well, as you say, the size of GCC and its history is pushing the limits of bzr a bit. For bug-fixing and committing, I actually find quilt+svn to be a fair bit more productive than bzr, and that's even with Andrew doing the heavy work on merging. I'd say that calling bzr a better tool than svn is pushing it a bit. It has some nice features, and it works better than svn for launchpad's purposes, but I'd still rather use SVN or, better still, git. If bzr wasn't such a dog to use (for large projects), it would be as good as SVN or GIT, but it would not be "better" - just different. (Svn was lacking a good merge tool, but I believe that's fixed now?) Quilt+svn is OK, but my personal preference is stgit+svn. :) Anyway, enough of the opinion piece, I hope the bzr stuff helps somebody. Andrew ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 2/2 android/device/linaro/common] mount mmc partitions instead of mtd partitions
On Tue, Mar 15, 2011 at 10:17 AM, Jim Huang wrote: > On 15 March 2011 16:29, Alexander Sack wrote: >> On Mon, Mar 14, 2011 at 3:37 PM, Jeremy Chang >> wrote: >>> All partitions from mmc is expected. >>> This depends on Jim Huang's patch "init: support mmc device mount" in >>> android/system/core to work. >> >> what happened with the "by-name" approach? Is this something we plan to do >> next? >> > > hi Alexander, > > Yes, that is the next patch I would like to submit. At present, > Jeremy and I just make sure > the minimal-but-working patches landed in Linaro first. OK, thanks for the update. I think once we start working on the by-name patches we should involve android-platform to see if they like that approach as well (and prepare eventual upstreaming). -- - Alexander ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 2/2 android/device/linaro/common] mount mmc partitions instead of mtd partitions
On 15 March 2011 16:29, Alexander Sack wrote: > On Mon, Mar 14, 2011 at 3:37 PM, Jeremy Chang wrote: >> All partitions from mmc is expected. >> This depends on Jim Huang's patch "init: support mmc device mount" in >> android/system/core to work. > > what happened with the "by-name" approach? Is this something we plan to do > next? > hi Alexander, Yes, that is the next patch I would like to submit. At present, Jeremy and I just make sure the minimal-but-working patches landed in Linaro first. Sincerely, -jserv ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 2/2 android/device/linaro/common] mount mmc partitions instead of mtd partitions
On Mon, Mar 14, 2011 at 3:37 PM, Jeremy Chang wrote: > All partitions from mmc is expected. > > This depends on Jim Huang's patch "init: support mmc device mount" in > android/system/core to work. > > Signed-off-by: Jeremy Chang > --- > init.rc | 12 ++-- > 1 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/init.rc b/init.rc > index b6a92c5..a3f72e1 100644 > --- a/init.rc > +++ b/init.rc > @@ -90,12 +90,12 @@ loglevel 3 > write /dev/cpuctl/bg_non_interactive/cpu.shares 52 > > on fs > -# mount mtd partitions > - # Mount /system rw first to give the filesystem a chance to save > a checkpoint > - mount yaffs2 mtd@system /system > - mount yaffs2 mtd@system /system ro remount > - mount yaffs2 mtd@userdata /data nosuid nodev > - mount yaffs2 mtd@cache /cache nosuid nodev > +# mount mmc partitions > + mount ext4 mmc@blk0p3 /system > + mount ext4 mmc@blk0p3 /system ro remount > + mount ext4 mmc@blk0p5 /cache > + mount ext4 mmc@blk0p6 /data > + mount ext4 mmc@blk0p7 /sdcard what happened with the "by-name" approach? Is this something we plan to do next? -- - Alexander ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 2/5] arm/mxc: add clk members to ease dt clock support
On Tue, Mar 15, 2011 at 02:02:56AM -0600, Grant Likely wrote:
> On Tue, Mar 15, 2011 at 03:50:29PM +0800, Shawn Guo wrote:
> > On Tue, Mar 15, 2011 at 01:41:01AM -0600, Grant Likely wrote:
> > > On Tue, Mar 08, 2011 at 11:56:34AM +0800, Shawn Guo wrote:
> > > > On Mon, Mar 07, 2011 at 10:53:37AM -0700, Grant Likely wrote:
> > > > > On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo
> > > > > wrote:
> > > > > > The 'rate' is added for fixed-clock support, while 'pll_base' is for
> > > > > > pll clock. These two particular type of clocks are supposed to be
> > > > > > gracefully supported by the common clk api when it gets ready.
> > > > >
> > > > > How does the current imx clock code handle fixed and pll clocks?
> > > >
> > > > For fixed-clock, the current code gets several variables holding the
> > > > rate and then return the rate from several get_rate functions.
> > > >
> > > > static unsigned long external_high_reference, external_low_reference;
> > > > static unsigned long oscillator_reference, ckih2_reference;
> > > >
> > > > static unsigned long get_high_reference_clock_rate(struct clk *clk)
> > > > {
> > > > return external_high_reference;
> > > > }
> > > >
> > > > static unsigned long get_low_reference_clock_rate(struct clk *clk)
> > > > {
> > > > return external_low_reference;
> > > > }
> > > >
> > > > static unsigned long get_oscillator_reference_clock_rate(struct clk
> > > > *clk)
> > > > {
> > > > return oscillator_reference;
> > > > }
> > > >
> > > > static unsigned long get_ckih2_reference_clock_rate(struct clk *clk)
> > > > {
> > > > return ckih2_reference;
> > > > }
> > > >
> > > > With this new rate member added, all these can be consolidated into one.
> > > >
> > > > For base address of pll, the current code uses the reference to clocks
> > > > statically defined to know which pll is the one.
> > > >
> > > > static inline void __iomem *_mx51_get_pll_base(struct clk *pll)
> > > > {
> > > > #ifdef CONFIG_OF
> > > > return pll->pll_base;
> > > > #else
> > > > if (pll == &pll1_main_clk)
> > > > return MX51_DPLL1_BASE;
> > > > else if (pll == &pll2_sw_clk)
> > > > return MX51_DPLL2_BASE;
> > > > else if (pll == &pll3_sw_clk)
> > > > return MX51_DPLL3_BASE;
> > > > else
> > > > BUG();
> > > >
> > > > return NULL;
> > > > #endif
> > >
> > > Be careful about stuff like this. Remember that enabling CONFIG_OF
> > > must *not break* board support that does not use the device tree. The
> > > above #ifdef block will break existing users.
> > >
> > Though the code has been killed in the latest version I just sent
> > yesterday I sent last night, I do not understand how it will break
> > the existing users. The existing code is:
> >
> > static inline void __iomem *_mx51_get_pll_base(struct clk *pll)
> > {
> > if (pll == &pll1_main_clk)
> > return MX51_DPLL1_BASE;
> > else if (pll == &pll2_sw_clk)
> > return MX51_DPLL2_BASE;
> > else if (pll == &pll3_sw_clk)
> > return MX51_DPLL3_BASE;
> > else
> > BUG();
> >
> > return NULL;
> > }
>
> What you wrote wrapped the current implementation with #ifdef
> CONFIG_OF ... #else [existing code] #endif. That says to me that when
> CONFIG_OF is enabled, the old code gets compiled out, which means the
> function no longer works on non-dt platforms.
>
> The goal is to support both dt and non-dt machines with a single
> kernel image.
>
Ah, I missed this point. Thanks.
--
Regards,
Shawn
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 5/5] of/clock: eliminate function __of_clk_get_from_provider
On Tue, Mar 15, 2011 at 03:59:16PM +0800, Shawn Guo wrote:
> On Tue, Mar 15, 2011 at 01:54:05AM -0600, Grant Likely wrote:
> > On Mon, Mar 14, 2011 at 09:18:42PM +0800, Shawn Guo wrote:
> > > With the platform clock support, the 'struct clk' should have been
> > > associated with device_node->data. So the use of function
> > > __of_clk_get_from_provider can be eliminated.
> > >
> > > Signed-off-by: Shawn Guo
> > > ---
> > > drivers/of/clock.c | 23 ++-
> > > 1 files changed, 2 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/of/clock.c b/drivers/of/clock.c
> > > index 7b5ea67..f124d0a 100644
> > > --- a/drivers/of/clock.c
> > > +++ b/drivers/of/clock.c
> > > @@ -71,24 +71,6 @@ void of_clk_del_provider(struct device_node *np,
> > > mutex_unlock(&of_clk_lock);
> > > }
> > >
> > > -static struct clk *__of_clk_get_from_provider(struct device_node *np,
> > > const char *clk_output)
> > > -{
> > > - struct of_clk_provider *provider;
> > > - struct clk *clk = NULL;
> > > -
> > > - /* Check if we have such a provider in our array */
> > > - mutex_lock(&of_clk_lock);
> > > - list_for_each_entry(provider, &of_clk_providers, link) {
> > > - if (provider->node == np)
> > > - clk = provider->get(np, clk_output, provider->data);
> > > - if (clk)
> > > - break;
> > > - }
> > > - mutex_unlock(&of_clk_lock);
> > > -
> > > - return clk;
> > > -}
> > > -
> > > struct clk *of_clk_get(struct device *dev, const char *id)
> > > {
> > > struct device_node *provnode;
> > > @@ -123,9 +105,8 @@ struct clk *of_clk_get(struct device *dev, const char
> > > *id)
> > > __func__, prop_name, dev->of_node->full_name);
> > > return NULL;
> > > }
> > > - clk = __of_clk_get_from_provider(provnode, prop);
> > > - if (clk)
> > > - dev_dbg(dev, "Using clock from %s\n", provnode->full_name);
> > > +
> > > + clk = provnode->data;
> >
> > Where is the device_node->data pointer getting set?
> >
> +#define ADD_CLK_LOOKUP() \
> + do {\
> + node->data = clk; \
> ^
> + \
> + cl->dev_id = dev_id;\
> + cl->clk = clk; \
> + clkdev_add(cl); \
> + \
> + return 0; \
> + \
> + out_kfree: \
> + kfree(cl); \
> + return ret; \
> + } while (0)
Yeah... don't do that! :-)
g.
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 2/5] arm/mxc: add clk members to ease dt clock support
On Tue, Mar 15, 2011 at 03:50:29PM +0800, Shawn Guo wrote:
> On Tue, Mar 15, 2011 at 01:41:01AM -0600, Grant Likely wrote:
> > On Tue, Mar 08, 2011 at 11:56:34AM +0800, Shawn Guo wrote:
> > > On Mon, Mar 07, 2011 at 10:53:37AM -0700, Grant Likely wrote:
> > > > On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo wrote:
> > > > > The 'rate' is added for fixed-clock support, while 'pll_base' is for
> > > > > pll clock. These two particular type of clocks are supposed to be
> > > > > gracefully supported by the common clk api when it gets ready.
> > > >
> > > > How does the current imx clock code handle fixed and pll clocks?
> > >
> > > For fixed-clock, the current code gets several variables holding the
> > > rate and then return the rate from several get_rate functions.
> > >
> > > static unsigned long external_high_reference, external_low_reference;
> > > static unsigned long oscillator_reference, ckih2_reference;
> > >
> > > static unsigned long get_high_reference_clock_rate(struct clk *clk)
> > > {
> > > return external_high_reference;
> > > }
> > >
> > > static unsigned long get_low_reference_clock_rate(struct clk *clk)
> > > {
> > > return external_low_reference;
> > > }
> > >
> > > static unsigned long get_oscillator_reference_clock_rate(struct clk *clk)
> > > {
> > > return oscillator_reference;
> > > }
> > >
> > > static unsigned long get_ckih2_reference_clock_rate(struct clk *clk)
> > > {
> > > return ckih2_reference;
> > > }
> > >
> > > With this new rate member added, all these can be consolidated into one.
> > >
> > > For base address of pll, the current code uses the reference to clocks
> > > statically defined to know which pll is the one.
> > >
> > > static inline void __iomem *_mx51_get_pll_base(struct clk *pll)
> > > {
> > > #ifdef CONFIG_OF
> > > return pll->pll_base;
> > > #else
> > > if (pll == &pll1_main_clk)
> > > return MX51_DPLL1_BASE;
> > > else if (pll == &pll2_sw_clk)
> > > return MX51_DPLL2_BASE;
> > > else if (pll == &pll3_sw_clk)
> > > return MX51_DPLL3_BASE;
> > > else
> > > BUG();
> > >
> > > return NULL;
> > > #endif
> >
> > Be careful about stuff like this. Remember that enabling CONFIG_OF
> > must *not break* board support that does not use the device tree. The
> > above #ifdef block will break existing users.
> >
> Though the code has been killed in the latest version I just sent
> yesterday I sent last night, I do not understand how it will break
> the existing users. The existing code is:
>
> static inline void __iomem *_mx51_get_pll_base(struct clk *pll)
> {
> if (pll == &pll1_main_clk)
> return MX51_DPLL1_BASE;
> else if (pll == &pll2_sw_clk)
> return MX51_DPLL2_BASE;
> else if (pll == &pll3_sw_clk)
> return MX51_DPLL3_BASE;
> else
> BUG();
>
> return NULL;
> }
What you wrote wrapped the current implementation with #ifdef
CONFIG_OF ... #else [existing code] #endif. That says to me that when
CONFIG_OF is enabled, the old code gets compiled out, which means the
function no longer works on non-dt platforms.
The goal is to support both dt and non-dt machines with a single
kernel image.
g.
>
> --
> Regards,
> Shawn
>
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 3/7] mmc: make the reference to sdhci_tegra_dt_pdata conditional
On Mon, Mar 14, 2011 at 10:25:55PM +0800, Shawn Guo wrote:
> Wrap tegra dt_id with CONFIG_MMC_SDHCI_TEGRA to make the reference to
> sdhci_tegra_dt_pdata conditional, otherwise it will stop build for
> other mmc driver when OF is enabled.
>
> Signed-off-by: Shawn Guo
Looks right to me.
g.
> ---
> drivers/mmc/host/sdhci-pltfm.c |2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> index ccc04ac..4125fbf 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -52,7 +52,9 @@ static struct sdhci_ops sdhci_pltfm_ops = {
> #if defined(CONFIG_OF)
> #include
> static const struct of_device_id sdhci_dt_ids[] = {
> +#ifdef CONFIG_MMC_SDHCI_TEGRA
> { .compatible = "nvidia,tegra250-sdhci", .data = &sdhci_tegra_dt_pdata
> },
> +#endif
> { }
> };
> MODULE_DEVICE_TABLE(platform, sdhci_dt_ids);
> --
> 1.7.1
>
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 5/5] of/clock: eliminate function __of_clk_get_from_provider
On Tue, Mar 15, 2011 at 01:54:05AM -0600, Grant Likely wrote:
> On Mon, Mar 14, 2011 at 09:18:42PM +0800, Shawn Guo wrote:
> > With the platform clock support, the 'struct clk' should have been
> > associated with device_node->data. So the use of function
> > __of_clk_get_from_provider can be eliminated.
> >
> > Signed-off-by: Shawn Guo
> > ---
> > drivers/of/clock.c | 23 ++-
> > 1 files changed, 2 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/of/clock.c b/drivers/of/clock.c
> > index 7b5ea67..f124d0a 100644
> > --- a/drivers/of/clock.c
> > +++ b/drivers/of/clock.c
> > @@ -71,24 +71,6 @@ void of_clk_del_provider(struct device_node *np,
> > mutex_unlock(&of_clk_lock);
> > }
> >
> > -static struct clk *__of_clk_get_from_provider(struct device_node *np,
> > const char *clk_output)
> > -{
> > - struct of_clk_provider *provider;
> > - struct clk *clk = NULL;
> > -
> > - /* Check if we have such a provider in our array */
> > - mutex_lock(&of_clk_lock);
> > - list_for_each_entry(provider, &of_clk_providers, link) {
> > - if (provider->node == np)
> > - clk = provider->get(np, clk_output, provider->data);
> > - if (clk)
> > - break;
> > - }
> > - mutex_unlock(&of_clk_lock);
> > -
> > - return clk;
> > -}
> > -
> > struct clk *of_clk_get(struct device *dev, const char *id)
> > {
> > struct device_node *provnode;
> > @@ -123,9 +105,8 @@ struct clk *of_clk_get(struct device *dev, const char
> > *id)
> > __func__, prop_name, dev->of_node->full_name);
> > return NULL;
> > }
> > - clk = __of_clk_get_from_provider(provnode, prop);
> > - if (clk)
> > - dev_dbg(dev, "Using clock from %s\n", provnode->full_name);
> > +
> > + clk = provnode->data;
>
> Where is the device_node->data pointer getting set?
>
+#define ADD_CLK_LOOKUP() \
+ do {\
+ node->data = clk; \
^
+ \
+ cl->dev_id = dev_id;\
+ cl->clk = clk; \
+ clkdev_add(cl); \
+ \
+ return 0; \
+ \
+ out_kfree: \
+ kfree(cl); \
+ return ret; \
+ } while (0)
> In general the ->data pointer of struct device_node should be avoided.
> There are no strong rules about its usage which means there is a very
> real risk that another driver or subsystem will try to use it for a
> different purpose.
>
> Iterating over the whole device tree is safer, and it really isn't
> very expensive. If you really want to store the struct_clk pointer in
> the device node, then it would be better to add a struct clk * field
> to struct device_node.
>
--
Regards,
Shawn
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 2/7] arm/dt: add pad configurations for mx51 babbage
On Mon, Mar 14, 2011 at 10:20:16AM -0500, Rob Herring wrote:
> Shawn,
>
> On 03/14/2011 09:25 AM, Shawn Guo wrote:
> >The pad configuration is something common between dt and non-dt
> >kernel, so it can be copied from non-dt code directly.
> >
> >Signed-off-by: Shawn Guo
> >---
> > arch/arm/mach-mx5/board-dt.c | 94
> > ++
> > 1 files changed, 94 insertions(+), 0 deletions(-)
> >
> >diff --git a/arch/arm/mach-mx5/board-dt.c b/arch/arm/mach-mx5/board-dt.c
> >index 45d1e37..4850251 100644
> >--- a/arch/arm/mach-mx5/board-dt.c
> >+++ b/arch/arm/mach-mx5/board-dt.c
> >@@ -31,6 +31,97 @@
> >
> > #include "devices.h"
> >
> >+static iomux_v3_cfg_t mx51babbage_pads[] = {
> >+/* UART1 */
> >+MX51_PAD_UART1_RXD__UART1_RXD,
> >+MX51_PAD_UART1_TXD__UART1_TXD,
> >+MX51_PAD_UART1_RTS__UART1_RTS,
> >+MX51_PAD_UART1_CTS__UART1_CTS,
> >+
> >+/* UART2 */
> >+MX51_PAD_UART2_RXD__UART2_RXD,
> >+MX51_PAD_UART2_TXD__UART2_TXD,
> >+
> >+/* UART3 */
> >+MX51_PAD_EIM_D25__UART3_RXD,
> >+MX51_PAD_EIM_D26__UART3_TXD,
> >+MX51_PAD_EIM_D27__UART3_RTS,
> >+MX51_PAD_EIM_D24__UART3_CTS,
> >+
> >+/* I2C1 */
> >+MX51_PAD_EIM_D16__I2C1_SDA,
> >+MX51_PAD_EIM_D19__I2C1_SCL,
> >+
> >+/* I2C2 */
> >+MX51_PAD_KEY_COL4__I2C2_SCL,
> >+MX51_PAD_KEY_COL5__I2C2_SDA,
> >+
> >+/* HSI2C */
> >+MX51_PAD_I2C1_CLK__I2C1_CLK,
> >+MX51_PAD_I2C1_DAT__I2C1_DAT,
> >+
> >+/* USB HOST1 */
> >+MX51_PAD_USBH1_CLK__USBH1_CLK,
> >+MX51_PAD_USBH1_DIR__USBH1_DIR,
> >+MX51_PAD_USBH1_NXT__USBH1_NXT,
> >+MX51_PAD_USBH1_DATA0__USBH1_DATA0,
> >+MX51_PAD_USBH1_DATA1__USBH1_DATA1,
> >+MX51_PAD_USBH1_DATA2__USBH1_DATA2,
> >+MX51_PAD_USBH1_DATA3__USBH1_DATA3,
> >+MX51_PAD_USBH1_DATA4__USBH1_DATA4,
> >+MX51_PAD_USBH1_DATA5__USBH1_DATA5,
> >+MX51_PAD_USBH1_DATA6__USBH1_DATA6,
> >+MX51_PAD_USBH1_DATA7__USBH1_DATA7,
> >+
> >+/* USB HUB reset line*/
> >+MX51_PAD_GPIO1_7__GPIO1_7,
> >+
> >+/* FEC */
> >+MX51_PAD_EIM_EB2__FEC_MDIO,
> >+MX51_PAD_EIM_EB3__FEC_RDATA1,
> >+MX51_PAD_EIM_CS2__FEC_RDATA2,
> >+MX51_PAD_EIM_CS3__FEC_RDATA3,
> >+MX51_PAD_EIM_CS4__FEC_RX_ER,
> >+MX51_PAD_EIM_CS5__FEC_CRS,
> >+MX51_PAD_NANDF_RB2__FEC_COL,
> >+MX51_PAD_NANDF_RB3__FEC_RX_CLK,
> >+MX51_PAD_NANDF_D9__FEC_RDATA0,
> >+MX51_PAD_NANDF_D8__FEC_TDATA0,
> >+MX51_PAD_NANDF_CS2__FEC_TX_ER,
> >+MX51_PAD_NANDF_CS3__FEC_MDC,
> >+MX51_PAD_NANDF_CS4__FEC_TDATA1,
> >+MX51_PAD_NANDF_CS5__FEC_TDATA2,
> >+MX51_PAD_NANDF_CS6__FEC_TDATA3,
> >+MX51_PAD_NANDF_CS7__FEC_TX_EN,
> >+MX51_PAD_NANDF_RDY_INT__FEC_TX_CLK,
> >+
> >+/* FEC PHY reset line */
> >+MX51_PAD_EIM_A20__GPIO2_14,
> >+
> >+/* SD 1 */
> >+MX51_PAD_SD1_CMD__SD1_CMD,
> >+MX51_PAD_SD1_CLK__SD1_CLK,
> >+MX51_PAD_SD1_DATA0__SD1_DATA0,
> >+MX51_PAD_SD1_DATA1__SD1_DATA1,
> >+MX51_PAD_SD1_DATA2__SD1_DATA2,
> >+MX51_PAD_SD1_DATA3__SD1_DATA3,
> >+
> >+/* SD 2 */
> >+MX51_PAD_SD2_CMD__SD2_CMD,
> >+MX51_PAD_SD2_CLK__SD2_CLK,
> >+MX51_PAD_SD2_DATA0__SD2_DATA0,
> >+MX51_PAD_SD2_DATA1__SD2_DATA1,
> >+MX51_PAD_SD2_DATA2__SD2_DATA2,
> >+MX51_PAD_SD2_DATA3__SD2_DATA3,
> >+
> >+/* eCSPI1 */
> >+MX51_PAD_CSPI1_MISO__ECSPI1_MISO,
> >+MX51_PAD_CSPI1_MOSI__ECSPI1_MOSI,
> >+MX51_PAD_CSPI1_SCLK__ECSPI1_SCLK,
> >+MX51_PAD_CSPI1_SS0__GPIO4_24,
> >+MX51_PAD_CSPI1_SS1__GPIO4_25,
> >+};
>
> This data already exists, so you should not duplicate it here.
>
> Iomux setup is a good candidate for a DT binding as it is just data,
> but I never came up with a good solution that was not bloated with a
> 32-bit value for every setting of each pin.
Yes, it makes perfect sense to encode the pin mux settings into the
dt. It will require some engineering and creativity to develop a good
binding.
g.
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 5/5] of/clock: eliminate function __of_clk_get_from_provider
On Mon, Mar 14, 2011 at 09:18:42PM +0800, Shawn Guo wrote:
> With the platform clock support, the 'struct clk' should have been
> associated with device_node->data. So the use of function
> __of_clk_get_from_provider can be eliminated.
>
> Signed-off-by: Shawn Guo
> ---
> drivers/of/clock.c | 23 ++-
> 1 files changed, 2 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/of/clock.c b/drivers/of/clock.c
> index 7b5ea67..f124d0a 100644
> --- a/drivers/of/clock.c
> +++ b/drivers/of/clock.c
> @@ -71,24 +71,6 @@ void of_clk_del_provider(struct device_node *np,
> mutex_unlock(&of_clk_lock);
> }
>
> -static struct clk *__of_clk_get_from_provider(struct device_node *np, const
> char *clk_output)
> -{
> - struct of_clk_provider *provider;
> - struct clk *clk = NULL;
> -
> - /* Check if we have such a provider in our array */
> - mutex_lock(&of_clk_lock);
> - list_for_each_entry(provider, &of_clk_providers, link) {
> - if (provider->node == np)
> - clk = provider->get(np, clk_output, provider->data);
> - if (clk)
> - break;
> - }
> - mutex_unlock(&of_clk_lock);
> -
> - return clk;
> -}
> -
> struct clk *of_clk_get(struct device *dev, const char *id)
> {
> struct device_node *provnode;
> @@ -123,9 +105,8 @@ struct clk *of_clk_get(struct device *dev, const char *id)
> __func__, prop_name, dev->of_node->full_name);
> return NULL;
> }
> - clk = __of_clk_get_from_provider(provnode, prop);
> - if (clk)
> - dev_dbg(dev, "Using clock from %s\n", provnode->full_name);
> +
> + clk = provnode->data;
Where is the device_node->data pointer getting set?
In general the ->data pointer of struct device_node should be avoided.
There are no strong rules about its usage which means there is a very
real risk that another driver or subsystem will try to use it for a
different purpose.
Iterating over the whole device tree is safer, and it really isn't
very expensive. If you really want to store the struct_clk pointer in
the device node, then it would be better to add a struct clk * field
to struct device_node.
g.
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 2/5] arm/mxc: add clk members to ease dt clock support
On Tue, Mar 15, 2011 at 01:41:01AM -0600, Grant Likely wrote:
> On Tue, Mar 08, 2011 at 11:56:34AM +0800, Shawn Guo wrote:
> > On Mon, Mar 07, 2011 at 10:53:37AM -0700, Grant Likely wrote:
> > > On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo wrote:
> > > > The 'rate' is added for fixed-clock support, while 'pll_base' is for
> > > > pll clock. These two particular type of clocks are supposed to be
> > > > gracefully supported by the common clk api when it gets ready.
> > >
> > > How does the current imx clock code handle fixed and pll clocks?
> >
> > For fixed-clock, the current code gets several variables holding the
> > rate and then return the rate from several get_rate functions.
> >
> > static unsigned long external_high_reference, external_low_reference;
> > static unsigned long oscillator_reference, ckih2_reference;
> >
> > static unsigned long get_high_reference_clock_rate(struct clk *clk)
> > {
> > return external_high_reference;
> > }
> >
> > static unsigned long get_low_reference_clock_rate(struct clk *clk)
> > {
> > return external_low_reference;
> > }
> >
> > static unsigned long get_oscillator_reference_clock_rate(struct clk *clk)
> > {
> > return oscillator_reference;
> > }
> >
> > static unsigned long get_ckih2_reference_clock_rate(struct clk *clk)
> > {
> > return ckih2_reference;
> > }
> >
> > With this new rate member added, all these can be consolidated into one.
> >
> > For base address of pll, the current code uses the reference to clocks
> > statically defined to know which pll is the one.
> >
> > static inline void __iomem *_mx51_get_pll_base(struct clk *pll)
> > {
> > #ifdef CONFIG_OF
> > return pll->pll_base;
> > #else
> > if (pll == &pll1_main_clk)
> > return MX51_DPLL1_BASE;
> > else if (pll == &pll2_sw_clk)
> > return MX51_DPLL2_BASE;
> > else if (pll == &pll3_sw_clk)
> > return MX51_DPLL3_BASE;
> > else
> > BUG();
> >
> > return NULL;
> > #endif
>
> Be careful about stuff like this. Remember that enabling CONFIG_OF
> must *not break* board support that does not use the device tree. The
> above #ifdef block will break existing users.
>
Though the code has been killed in the latest version I just sent
yesterday I sent last night, I do not understand how it will break
the existing users. The existing code is:
static inline void __iomem *_mx51_get_pll_base(struct clk *pll)
{
if (pll == &pll1_main_clk)
return MX51_DPLL1_BASE;
else if (pll == &pll2_sw_clk)
return MX51_DPLL2_BASE;
else if (pll == &pll3_sw_clk)
return MX51_DPLL3_BASE;
else
BUG();
return NULL;
}
--
Regards,
Shawn
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 3/5] arm/dt: mx51: dynamically add clocks per dt nodes
On Mon, Mar 14, 2011 at 09:18:40PM +0800, Shawn Guo wrote:
> This patch is to change the static clock creating and registering to
> the dynamic way, which scans dt clock nodes, associate clk with
> device_node, and then add them to clkdev accordingly.
>
> It's a pretty straight translation from non-dt clock code to dt one,
> and it does not really change any actual clock code.
>
> Signed-off-by: Shawn Guo
> ---
> arch/arm/mach-mx5/clock-mx51-mx53.c | 1401
> ++-
> 1 files changed, 1387 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c
> b/arch/arm/mach-mx5/clock-mx51-mx53.c
> index dedb7f9..c3ec7f6 100644
> --- a/arch/arm/mach-mx5/clock-mx51-mx53.c
> +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
> @@ -49,6 +49,30 @@ static struct clk emi_fast_clk;
> static struct clk ipu_clk;
> static struct clk mipi_hsc1_clk;
>
> +#ifdef CONFIG_OF
> +/*
> + * The pointers are defined to save the references to the clocks
> + * dynamically created, and then could be used to replace those
> + * static references in non-dt clock code.
> + */
> +static struct clk *osc_clk_dt;
> +static struct clk *pll1_main_clk_dt;
> +static struct clk *pll1_sw_clk_dt;
> +static struct clk *pll2_sw_clk_dt;
> +static struct clk *pll3_sw_clk_dt;
> +static struct clk *lp_apm_clk_dt;
> +static struct clk *periph_apm_clk_dt;
> +static struct clk *main_bus_clk_dt;
> +static struct clk *ipg_clk_dt;
> +static struct clk *ipg_per_clk_dt;
> +static struct clk *cpu_clk_dt;
> +static struct clk *iim_clk_dt;
> +static struct clk *usboh3_clk_dt;
> +static struct clk *usb_phy1_clk_dt;
> +static struct clk *esdhc1_clk_dt;
> +static struct clk *esdhc2_clk_dt;
> +#endif
Heh, yeah this seems sub-optimal. It would be better to share common
clock initialization between dt and non-dt boards, even if it means
that a lot of the clocks are node described in the device tree (at
least for the on-chip SoC clocks; board-accessible clocks still need
to appear in the device tree though.
> +
> #define MAX_DPLL_WAIT_TRIES 1000 /* 1000 * udelay(1) = 1ms */
>
> /* calculate best pre and post dividers to get the required divider */
> @@ -163,10 +187,18 @@ static inline void __iomem *_mx53_get_pll_base(struct
> clk *pll)
> return NULL;
> }
>
> +#ifdef CONFIG_OF
> +static void __iomem *dt_mx51_get_pll_base(struct clk *pll);
> +#endif
> +
> static inline void __iomem *_get_pll_base(struct clk *pll)
> {
> if (cpu_is_mx51())
> +#ifdef CONFIG_OF
> + return dt_mx51_get_pll_base(pll);
> +#else
> return _mx51_get_pll_base(pll);
> +#endif
If you do it this way, you need to make sure dt_mx51_get_pll_base()
falls back to _mx51_get_pll_base() when a dt is not provided.
g.
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 2/5] arm/mxc: add clk members to ease dt clock support
On Tue, Mar 08, 2011 at 11:56:34AM +0800, Shawn Guo wrote:
> On Mon, Mar 07, 2011 at 10:53:37AM -0700, Grant Likely wrote:
> > On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo wrote:
> > > The 'rate' is added for fixed-clock support, while 'pll_base' is for
> > > pll clock. These two particular type of clocks are supposed to be
> > > gracefully supported by the common clk api when it gets ready.
> >
> > How does the current imx clock code handle fixed and pll clocks?
>
> For fixed-clock, the current code gets several variables holding the
> rate and then return the rate from several get_rate functions.
>
> static unsigned long external_high_reference, external_low_reference;
> static unsigned long oscillator_reference, ckih2_reference;
>
> static unsigned long get_high_reference_clock_rate(struct clk *clk)
> {
> return external_high_reference;
> }
>
> static unsigned long get_low_reference_clock_rate(struct clk *clk)
> {
> return external_low_reference;
> }
>
> static unsigned long get_oscillator_reference_clock_rate(struct clk *clk)
> {
> return oscillator_reference;
> }
>
> static unsigned long get_ckih2_reference_clock_rate(struct clk *clk)
> {
> return ckih2_reference;
> }
>
> With this new rate member added, all these can be consolidated into one.
>
> For base address of pll, the current code uses the reference to clocks
> statically defined to know which pll is the one.
>
> static inline void __iomem *_mx51_get_pll_base(struct clk *pll)
> {
> #ifdef CONFIG_OF
> return pll->pll_base;
> #else
> if (pll == &pll1_main_clk)
> return MX51_DPLL1_BASE;
> else if (pll == &pll2_sw_clk)
> return MX51_DPLL2_BASE;
> else if (pll == &pll3_sw_clk)
> return MX51_DPLL3_BASE;
> else
> BUG();
>
> return NULL;
> #endif
Be careful about stuff like this. Remember that enabling CONFIG_OF
must *not break* board support that does not use the device tree. The
above #ifdef block will break existing users.
g.
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 3/5] arm/dt: mx51: dynamically add gpt and uart related clocks per dt nodes
On Tue, Mar 08, 2011 at 12:22:10AM +0800, Shawn Guo wrote:
> This patch is to change the static clock creating and registering to
> the dynamic way, which scans dt clock nodes, associate clk with
> device_node, and then add them to clkdev accordingly.
>
> Signed-off-by: Shawn Guo
> ---
> arch/arm/mach-mx5/clock-mx51-mx53.c | 436
> +--
> 1 files changed, 422 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c
> b/arch/arm/mach-mx5/clock-mx51-mx53.c
> index dedb7f9..1940171 100644
> --- a/arch/arm/mach-mx5/clock-mx51-mx53.c
> +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
> @@ -135,6 +135,9 @@ static inline u32 _get_mux(struct clk *parent, struct clk
> *m0,
>
> static inline void __iomem *_mx51_get_pll_base(struct clk *pll)
> {
> +#ifdef CONFIG_OF
> + return pll->pll_base;
> +#else
> if (pll == &pll1_main_clk)
> return MX51_DPLL1_BASE;
> else if (pll == &pll2_sw_clk)
> @@ -145,6 +148,7 @@ static inline void __iomem *_mx51_get_pll_base(struct clk
> *pll)
> BUG();
>
> return NULL;
> +#endif
> }
>
> static inline void __iomem *_mx53_get_pll_base(struct clk *pll)
> @@ -1439,33 +1443,437 @@ int __init mx53_clocks_init(unsigned long ckil,
> unsigned long osc,
> return 0;
> }
>
> +/*
> + * Dynamically create and register clks per dt nodes
> + */
> #ifdef CONFIG_OF
> -static struct clk *mx5_dt_clk_get(struct device_node *np,
> - const char *output_id, void *data)
> +
> +#define ALLOC_CLK_LOOKUP() \
> + struct clk_lookup *cl; \
> + struct clk *clk;\
> + int ret;\
> + \
> + do {\
> + cl = kzalloc(sizeof(*cl) + sizeof(*clk), GFP_KERNEL); \
> + if (!cl)\
> + return -ENOMEM; \
> + clk = (struct clk *) (cl + 1); \
> + \
> + clk->parent = mx5_get_source_clk(node); \
> + clk->secondary = mx5_get_source_clk(node); \
> + } while (0)
> +
> +#define ADD_CLK_LOOKUP() \
> + do {\
> + node->data = clk; \
> + cl->dev_id = of_get_property(node, \
> + "clock-outputs", NULL); \
> + cl->con_id = of_get_property(node, \
> + "clock-alias", NULL); \
> + if (!cl->dev_id && !cl->con_id) { \
> + ret = -EINVAL; \
> + goto out_kfree; \
> + } \
> + cl->clk = clk; \
> + clkdev_add(cl); \
> + \
> + return 0; \
> + \
> + out_kfree: \
> + kfree(cl); \
> + return ret; \
> + } while (0)
Yikes! Doing this as a macro will be a nightmare for debugging.
This needs refactoring into functions.
> +
> +static unsigned long get_fixed_clk_rate(struct clk *clk)
> {
> - return data;
> + return clk->rate;
> }
>
> -static __init void mx5_dt_scan_clks(void)
> +static __init int mx5_scan_fixed_clks(void)
> {
> struct device_node *node;
> + struct clk_lookup *cl;
> struct clk *clk;
> - const char *id;
> - int rc;
> + const __be32 *rate;
> + int ret = 0;
>
> - for_each_compatible_node(node, NULL, "clock") {
> - id = of_get_property(node, "clock-outputs", NULL);
> - if (!id)
> + for_each_compatible_node(node, NULL, "fixed-clock") {
> + cl = kzalloc(sizeof(*cl) + sizeof(*clk), GFP_KERNEL);
> + if (!cl) {
> + ret = -ENOMEM;
> + break;
> + }
> + clk = (struct clk *) (cl + 1);
> +
> + rate = of_get_property(node, "clock-frequency", NULL);
> +
Re: [PATCH V4 5/5] net/fec: add device tree matching support
On Thu, Mar 10, 2011 at 12:59:45PM +0800, Jason Liu wrote:
> Signed-off-by: Jason Liu
> Signed-off-by: Jason Liu
> ---
> drivers/net/fec.c | 13 +
> 1 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 02cdd71..fcb9768 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -45,6 +45,9 @@
> #include
> #include
>
> +#include
> +#include
> +
Should be mixed in with the rest of the linux/*.h includes (don't put
a blank line between them.
> #include
>
> #ifndef CONFIG_ARM
> @@ -1523,6 +1526,13 @@ static const struct dev_pm_ops fec_pm_ops = {
> };
> #endif
>
> +#ifdef CONFIG_OF
> +static struct of_device_id fec_matches[] = {
> + { .compatible = "fsl,imx-fec" },
Must have documentation for this binding in
Documentation/devicetree/bindings before I can pick this up. Same
goes for the uart driver patch.
Also, I recommend being more specific on the compatible property.
fsl,imx51-fec would be better. Newer parts can claim compatibility
with this one if you're concerned about supporting multiple parts.
ie. for imx 53, this would be appropriate:
compatible = "fsl,imx53-fec", "fsl,imx51-fec";
> + {},
> +};
> +#endif
> +
> static struct platform_driver fec_driver = {
> .driver = {
> .name = DRIVER_NAME,
> @@ -1530,6 +1540,9 @@ static struct platform_driver fec_driver = {
> #ifdef CONFIG_PM
> .pm = &fec_pm_ops,
> #endif
> +#ifdef CONFIG_OF
> + .of_match_table = fec_matches,
> +#endif
> },
> .id_table = fec_devtype,
> .probe = fec_probe,
> --
> 1.7.1
>
>
> ___
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V4 4/5] net/fec: check id_entry pointer before using it
On Thu, Mar 10, 2011 at 12:59:44PM +0800, Jason Liu wrote:
> The id_entry will possibly be NULL, So, need check
> id_entry and make sure it not NULL before using it.
>
> Signed-off-by: Jason Liu
> Signed-off-by: Jason Liu
Other than the double s-o-b lines, this patch looks good to me.
Acked-by: Grant Likely
g.
> ---
> drivers/net/fec.c | 12 ++--
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 2a71373..02cdd71 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -293,7 +293,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct
> net_device *dev)
>* the system that it's running on. As the result, driver has to
>* swap every frame going to and coming from the controller.
>*/
> - if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
> + if (id_entry && id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
> swap_buffer(bufaddr, skb->len);
>
> /* Save skb pointer */
> @@ -529,7 +529,7 @@ fec_enet_rx(struct net_device *dev)
> dma_unmap_single(NULL, bdp->cbd_bufaddr, bdp->cbd_datlen,
> DMA_FROM_DEVICE);
>
> - if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
> + if (id_entry && id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
> swap_buffer(data, pkt_len);
>
> /* This does 16 byte alignment, exactly what we need.
> @@ -808,7 +808,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
>* mdio interface in board design, and need to be configured by
>* fec0 mii_bus.
>*/
> - if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && pdev->id) {
> + if (id_entry && (id_entry->driver_data & FEC_QUIRK_ENET_MAC) &&
> pdev->id) {
> /* fec1 uses fec0 mii_bus */
> fep->mii_bus = fec0_mii_bus;
> return 0;
> @@ -851,7 +851,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
> goto err_out_free_mdio_irq;
>
> /* save fec0 mii_bus */
> - if (id_entry->driver_data & FEC_QUIRK_ENET_MAC)
> + if (id_entry && id_entry->driver_data & FEC_QUIRK_ENET_MAC)
> fec0_mii_bus = fep->mii_bus;
>
> return 0;
> @@ -1238,7 +1238,7 @@ fec_restart(struct net_device *dev, int duplex)
>* enet-mac reset will reset mac address registers too,
>* so need to reconfigure it.
>*/
> - if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
> + if (id_entry && id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
> memcpy(&temp_mac, dev->dev_addr, ETH_ALEN);
> writel(cpu_to_be32(temp_mac[0]), fep->hwp + FEC_ADDR_LOW);
> writel(cpu_to_be32(temp_mac[1]), fep->hwp + FEC_ADDR_HIGH);
> @@ -1294,7 +1294,7 @@ fec_restart(struct net_device *dev, int duplex)
>* The phy interface and speed need to get configured
>* differently on enet-mac.
>*/
> - if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
> + if (id_entry && id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
> val = readl(fep->hwp + FEC_R_CNTRL);
>
> /* MII or RMII */
> --
> 1.7.1
>
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V4 3/5] serial/imx: parse from device tree support
Minor comments, but otherwise goes first.
On Thu, Mar 10, 2011 at 12:59:43PM +0800, Jason Liu wrote:
> Signed-off-by: Jason Liu
> Signed-off-by: Jason Liu
> Signed-off-by: Jeremy Kerr
Jeremy's s-o-b should go at the top of the list. He started it, and
then you took over. s-o-b lines is the chain of people who have
handled a patch, and so it should reflect the order that they handled
it.
> ---
> drivers/tty/serial/imx.c | 79
> --
> 1 files changed, 69 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index dfcf4b1..c9483d1 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -53,6 +53,9 @@
> #include
> #include
>
> +#include
> +#include
> +
These should be mixed in with the rest of the includes.
> /* Register definitions */
> #define URXD0 0x0 /* Receiver Register */
> #define URTX0 0x40 /* Transmitter Register */
> @@ -1224,6 +1227,53 @@ static int serial_imx_resume(struct platform_device
> *dev)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static int serial_imx_probe_dt(struct imx_port *sport,
> + struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + static int line;
> +
> + if (!node)
> + return -ENODEV;
> +
> + if (of_get_property(node, "fsl,has-rts-cts", NULL))
> + sport->have_rtscts = 1;
> +
> + if (of_get_property(node, "fsl,irda-mode", NULL))
> + sport->use_irda = 1;
> +
> + sport->port.line = line++;
> +
> + return 0;
> +}
> +#else
> +static int serial_imx_probe_dt(struct imx_port *sport,
> + struct platform_device *pdev)
> +{
> + return -ENODEV;
> +}
> +#endif
> +
> +static int serial_imx_probe_pdata(struct imx_port *sport,
> + struct platform_device *pdev)
> +{
> + struct imxuart_platform_data *pdata = pdev->dev.platform_data;
> +
> + if (!pdata)
> + return 0;
> +
> + if (pdata->flags & IMXUART_HAVE_RTSCTS)
> + sport->have_rtscts = 1;
> +
> +#ifdef CONFIG_IRDA
> + if (pdata->flags & IMXUART_IRDA)
> + sport->use_irda = 1;
> +#endif
> + sport->port.line = pdev->id;
> +
> + return 0;
> +}
> static int serial_imx_probe(struct platform_device *pdev)
> {
> struct imx_port *sport;
> @@ -1236,6 +1286,12 @@ static int serial_imx_probe(struct platform_device
> *pdev)
> if (!sport)
> return -ENOMEM;
>
> + ret = serial_imx_probe_dt(sport, pdev);
> + if (ret == -ENODEV)
> + ret = serial_imx_probe_pdata(sport, pdev);
> + if (ret)
> + goto free;
> +
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!res) {
> ret = -ENODEV;
> @@ -1260,7 +1316,6 @@ static int serial_imx_probe(struct platform_device
> *pdev)
> sport->port.fifosize = 32;
> sport->port.ops = &imx_pops;
> sport->port.flags = UPF_BOOT_AUTOCONF;
> - sport->port.line = pdev->id;
> init_timer(&sport->timer);
> sport->timer.function = imx_timeout;
> sport->timer.data = (unsigned long)sport;
> @@ -1274,17 +1329,13 @@ static int serial_imx_probe(struct platform_device
> *pdev)
>
> sport->port.uartclk = clk_get_rate(sport->clk);
>
> - imx_ports[pdev->id] = sport;
> + if (imx_ports[sport->port.line]) {
> + ret = -EBUSY;
> + goto clkput;
> + }
> + imx_ports[sport->port.line] = sport;
>
> pdata = pdev->dev.platform_data;
> - if (pdata && (pdata->flags & IMXUART_HAVE_RTSCTS))
> - sport->have_rtscts = 1;
> -
> -#ifdef CONFIG_IRDA
> - if (pdata && (pdata->flags & IMXUART_IRDA))
> - sport->use_irda = 1;
> -#endif
> -
> if (pdata && pdata->init) {
> ret = pdata->init(pdev);
> if (ret)
> @@ -1336,6 +1387,11 @@ static int serial_imx_remove(struct platform_device
> *pdev)
> return 0;
> }
>
> +static struct of_device_id imx_uart_matches[] = {
> + { .compatible = "fsl,imx51-uart" },
> + {},
> +};
> +
> static struct platform_driver serial_imx_driver = {
> .probe = serial_imx_probe,
> .remove = serial_imx_remove,
> @@ -1345,6 +1401,9 @@ static struct platform_driver serial_imx_driver = {
> .driver = {
> .name = "imx-uart",
> .owner = THIS_MODULE,
> +#if defined(CONFIG_OF)
> + .of_match_table = imx_uart_matches,
> +#endif
> },
> };
>
> --
> 1.7.1
>
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V4 1/5] arm/dt: add basic mx51 device tree support
Hi Jason,
Minor comments below.
On Thu, Mar 10, 2011 at 12:59:41PM +0800, Jason Liu wrote:
> Signed-off-by: Jason Liu
> Signed-off-by: Jason Liu
This looks wrong. You should only have one s-o-b line. Use one email
addr or the other. Not both.
> ---
> arch/arm/mach-mx5/Kconfig |8
> arch/arm/mach-mx5/Makefile |1 +
> arch/arm/mach-mx5/board-dt.c| 65
> +++
> arch/arm/mach-mx5/clock-mx51-mx53.c | 43 -
> arch/arm/plat-mxc/include/mach/common.h |1 +
> 5 files changed, 117 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
> index de4fa99..6438f87 100644
> --- a/arch/arm/mach-mx5/Kconfig
> +++ b/arch/arm/mach-mx5/Kconfig
> @@ -47,6 +47,14 @@ config MACH_MX51_BABBAGE
> u-boot. This includes specific configurations for the board and its
> peripherals.
>
> +config MACH_MX51_DT
> + bool "Generic MX51 board (FDT support)"
> + select USE_OF
> + select SOC_IMX51
> + help
> + Support for generic Freescale i.MX51 boards using Flattened Device
> + Tree.
> +
> config MACH_MX51_3DS
> bool "Support MX51PDK (3DS)"
> select SOC_IMX51
> diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
> index 0d43be9..540697e 100644
> --- a/arch/arm/mach-mx5/Makefile
> +++ b/arch/arm/mach-mx5/Makefile
> @@ -18,3 +18,4 @@ obj-$(CONFIG_MACH_EUKREA_CPUIMX51SD) += board-cpuimx51sd.o
> obj-$(CONFIG_MACH_EUKREA_MBIMXSD51_BASEBOARD) += eukrea_mbimxsd-baseboard.o
> obj-$(CONFIG_MACH_MX51_EFIKAMX) += board-mx51_efikamx.o
> obj-$(CONFIG_MACH_MX50_RDP) += board-mx50_rdp.o
> +obj-$(CONFIG_MACH_MX51_DT) += board-dt.o
> diff --git a/arch/arm/mach-mx5/board-dt.c b/arch/arm/mach-mx5/board-dt.c
> new file mode 100644
> index 000..19c60a4
> --- /dev/null
> +++ b/arch/arm/mach-mx5/board-dt.c
> @@ -0,0 +1,65 @@
> +/*
> + * Copyright 2011 Linaro Ltd.
> + * Copyright 2011 Freescale Semiconductor, Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "devices.h"
> +
> +static struct of_device_id mx51_dt_match_table[] __initdata = {
> + { .compatible = "simple-bus", },
> + {}
> +};
> +
> +static void __init mx51_dt_board_init(void)
> +{
> + of_platform_bus_probe(NULL, mx51_dt_match_table, NULL);
> +}
> +
> +static void __init mx51_dt_timer_init(void)
> +{
> + mx51_clocks_init(32768, 2400, 22579200, 0);
> + mx5_clk_dt_init();
> +}
> +
> +static struct sys_timer mxc_timer = {
> + .init = mx51_dt_timer_init,
> +};
> +
> +static const char *mx51_dt_board_compat[] = {
> + "fsl,mx51-babbage",
> + NULL
> +};
> +
> +DT_MACHINE_START(MX51_DT, "Freescale MX51 (Flattened Device Tree)")
> + .boot_params = PHYS_OFFSET + 0x100,
You should be able to drop the .boot_params line.
> + .map_io = mx51_map_io,
> + .init_irq = mx51_init_irq,
> + .init_machine = mx51_dt_board_init,
> + .dt_compat= mx51_dt_board_compat,
> + .timer= &mxc_timer,
> +MACHINE_END
> diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c
> b/arch/arm/mach-mx5/clock-mx51-mx53.c
> index 0a19e75..dedb7f9 100644
> --- a/arch/arm/mach-mx5/clock-mx51-mx53.c
> +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
> @@ -15,13 +15,19 @@
> #include
> #include
> #include
> -
> +#include
> #include
>
> #include
> #include
> #include
>
> +#ifdef CONFIG_OF
> +#include
> +#include
> +#include
> +#endif /* CONFIG_OF */
You can drop the #ifdef CONFIG_OF here. linux/of*.h is safe to
include when CONFIG_OF is not selected.
> +
> #include "crm_regs.h"
>
> /* External clock values passed-in by the board code */
> @@ -1432,3 +1438,38 @@ int __init mx53_clocks_init(unsigned long ckil,
> unsigned long osc,
> MX53_INT_GPT);
> return 0;
> }
> +
> +#ifdef CONFIG_OF
> +static struct clk *mx5_dt_clk_get(struct device_node *np,
> + const char *output_id, void *data)
> +{
> + return data;
> +}
> +
> +static __init void mx5_dt_scan_clks(void)
> +{
> + struct device_node *node;
> + struct clk *clk;
> + const char *id;
> + int rc;
> +
> + for_each_compatible_node(node, NULL, "clock") {
> + id = of_get_property(node, "clock-outputs", NULL);
> + if (!id)
> + continue;
> +
> + clk = clk_get_sys(id, NULL);
> + if (IS_ERR(clk))
> + continue;
> +
> +
