Re: [PATCH V3 11/11] ARM: delete struct sys_timer
On 11/20/2012 09:43 PM, Haojian Zhuang wrote:
> On Tue, Nov 20, 2012 at 2:31 AM, Stephen Warren wrote:
>> From: Stephen Warren
>>
>> Now that the only field in struct sys_timer is .init, delete the struct,
>> and replace the machine descriptor .timer field with the initialization
>> function itself.
>>
>> This will enable moving timer drivers into drivers/clocksource without
>> having to place a public prototype of each struct sys_timer object into
>> include/linux; the intent is to create a single of_clocksource_init()
>> function that determines which timer driver to initialize by scanning
>> the device dtree, much like the proposed irqchip_init() at:
>> http://www.spinics.net/lists/arm-kernel/msg203686.html
...
> I checked the patch for mach-mmp.
>
> @@ -69,7 +65,7 @@ static const char *mmp_dt_board_compat[] __initdata = {
> DT_MACHINE_START(PXA168_DT, "Marvell PXA168 (Device Tree Support)")
> .map_io = mmp_map_io,
> .init_irq = mmp_dt_irq_init,
> - .timer = &mmp_dt_timer,
> + .init_time = mmp_dt_init_timer,
> .init_machine = pxa168_dt_init,
> .dt_compat = mmp_dt_board_compat,
> MACHINE_END
> @@ -77,7 +73,7 @@ MACHINE_END
> DT_MACHINE_START(PXA910_DT, "Marvell PXA910 (Device Tree Support)")
> .map_io = mmp_map_io,
> .init_irq = mmp_dt_irq_init,
> - .timer = &mmp_dt_timer,
> + .init_time = mmp_dt_timer_init,
> .init_machine = pxa910_dt_init,
> .dt_compat = mmp_dt_board_compat,
> MACHINE_END
>
> This first init_time is assigned by mmp_dt_init_timer. But the second
> init_time is
> assigned by mmp_dt_timer_init. I think it's a typo error. Could you
> help to fix this?
Thanks, I've fixed that up locally.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3 10/11] ARM: remove struct sys_timer suspend and resume fields
On 11/21/2012 01:28 AM, Linus Walleij wrote:
> Oh and there was this comment/TODO:
>
> On Mon, Nov 19, 2012 at 7:31 PM, Stephen Warren wrote:
>
>> @@ -17,15 +17,6 @@
>> * Initialise the kernels jiffy timer source, claim interrupt
>> * using setup_irq. This is called early on during initialisation
>> * while interrupts are still disabled on the local CPU.
>> - * - suspend
>> - * Suspend the kernel jiffy timer source, if necessary. This
>> - * is called with interrupts disabled, after all normal devices
>> - * have been suspended. If no action is required, set this to
>> - * NULL.
>> - * - resume
>> - * Resume the kernel jiffy timer source, if necessary. This
>> - * is called with interrupts disabled before any normal devices
>> - * are resumed. If no action is required, set this to NULL.
>> * - offset
>> * Return the timer offset in microseconds since the last timer
>> * interrupt. Note: this must take account of any unprocessed
>> @@ -33,8 +24,6 @@
>> */
>> struct sys_timer {
>> void(*init)(void);
>> - void(*suspend)(void);
>> - void(*resume)(void);
>> };
>
> So from the above it is quite clear that the sys_timer is breaking
> the suspend_noirq/resume_noirq naming convention from
> runtime PM as IRQs are disabled on these paths.
>
> The same goes for struct clock_event_device ...
>
> So while this looks just as bad after as before the patch,
> we should take a mental notice to rename the .suspend
> and .resume hooks in the clock_event_device to
> .suspend_noirq and .resume_noirq at some point.
>
> I was thinking that if your patch set is introducing a
> plethora of new users of these hooks we should maybe
> stick a patch at the beginning of the series renaming the
> hooks to *_noirq, but if it's a major obstacle it can surely wait.
I think I'd prefer to keep that rename separate and do it later; while
this series does introduce a few new users of the type, there are many
more pre-existing users. Renaming all the users would end up making this
series potentially conflict with addition of new users in tip.git or
elsewhere, whereas any possible current conflicts from this series
should be resolvable in arm-soc pretty easily I hope, so I'd rather
create a separate series that does the rename, and probably apply it to
tip.git, probably for 3.10?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] memory: tegra30: Fix warning w/o PM_SLEEP
On 11/21/2012 05:42 AM, Hiroshi Doyu wrote: > Fix build warning w/o PM_SLEEP. Reviewed-by: Stephen Warren I assume Greg will take this through his tree. > Change-Id: I713f7dd697f1ad9fb1fee56b389b0d6c45bb540c But that shouldn't be there though... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] arm: tegra: select correct parent clk for pll_p
On 11/20/2012 12:29 AM, Sivaram Nair wrote: > For Tegra30, pll_p clk's parent is wrongly specified as clk_m instead of > pll_ref in the tegra30_clk_init_table and this is resulting in a > boot-time warning. This patch fixes this by correcting the clk init > table. Thanks, applied. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] arm: tegra: fix comment in dsib clk set_parent
On 11/21/2012 04:42 AM, Sivaram Nair wrote: > Since the clk framework has already taken necessary locks before calling > into the arch clk ops code, no further locks are needed while setting > the parent of dsib clk. This patch removes a comment that indicated > otherwise, and yet did not take any locks. Thanks, applied. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] amba: tegra-ahb: Fix warning w/o PM_SLEEP
On 11/21/2012 05:41 AM, Hiroshi Doyu wrote: > Fix build warning w/o PM_SLEEP. > > Change-Id: Id5b7d089de835025b5708fa345a75990fd976363 Thanks, applied, with commit description fixed up to remove the Change-Id. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] ARM: dt: tegra: cardhu: Add drm components
On 11/20/2012 12:27 AM, Mark Zhang wrote: > This patch adds the rgb node which is needed by > tegra drm driver. This doesn't seem to include all the GPIOs/regulators/... required to enable the panel and backlight. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC v2 5/8] ARM: tegra: Add auxiliary data for nvhost
On 11/26/2012 06:19 AM, Terje Bergstrom wrote:
> Add SoC specific auxiliary data to host1x and gr2d. nvhost uses
> this data.
>
> Signed-off-by: Terje Bergstrom
> Signed-off-by: Arto Merilainen
Arto's S-o-b really should be first and yours last since you're (Terje)
the one who touched the patches last.
> diff --git a/arch/arm/mach-tegra/board-dt-tegra20.c
> b/arch/arm/mach-tegra/board-dt-tegra20.c
I think none of the changes the board-dt-tegra*.c should be made.
AUXDATA is a temporary measure to keep things working during the
transition to device tree. We want to remove entries from the AUXDATA
tables rather than add them. The only thing that's stopping us from
doing so right now is the lack of DT-based clock lookups, which hence
require devices to have a specific name.
> +static struct nvhost_device_data tegra_host1x_info = {
> + .clocks = { {"host1x", UINT_MAX} },
> +static struct nvhost_device_data tegra_gr2d_info = {
> + .clocks = { {"gr2d", UINT_MAX, true}, {"epp", UINT_MAX, true} },
Clock names shouldn't be passed in platform data; instead, clk_get()
should be passed the device object and device-relative (i.e. not global)
clock name. I expect if the driver is fixed to make this change, the
changes to tegra*_clocks_data.c won't be needed either.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC v2 5/8] ARM: tegra: Add auxiliary data for nvhost
On 11/26/2012 11:33 PM, Terje Bergström wrote:
> On 27.11.2012 01:39, Stephen Warren wrote:
>> Clock names shouldn't be passed in platform data; instead, clk_get()
>> should be passed the device object and device-relative (i.e. not global)
>> clock name. I expect if the driver is fixed to make this change, the
>> changes to tegra*_clocks_data.c won't be needed either.
>
> Isn't this code doing exactly that - getting a device relative clock,
> nvhost_module_init() in nvhost.acm.c:
>
> (...)
> /* initialize clocks to known state */
> while (pdata->clocks[i].name && i < NVHOST_MODULE_MAX_CLOCKS) {
> long rate = pdata->clocks[i].default_rate;
> struct clk *c;
>
> c = devm_clk_get(&dev->dev, pdata->clocks[i].name);
The line above is getting the (device-relative) clock name from platform
data, rather than using some fixed name as it should be.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH V7 3/7] arm64: use new common dtc rule
From: Stephen Warren The current rules have the .dtb files build in a different directory from the .dts files. This patch changes arm64 to use the generic dtb rule which builds .dtb files in the same directory as the source .dts. This requires moving parts of arch/arm64/boot/Makefile into newly created arch/arm64/boot/dts/Makefile, and updating arch/arm64/Makefile to call the new Makefile. Cc: Catalin Marinas Cc: linux-arm-ker...@lists.infradead.org Signed-off-by: Stephen Warren --- v7: New patch. --- arch/arm64/Makefile |2 +- arch/arm64/boot/Makefile |5 - arch/arm64/boot/dts/Makefile |5 + 3 files changed, 6 insertions(+), 6 deletions(-) create mode 100644 arch/arm64/boot/dts/Makefile diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile index 364191f..fd3d4a1 100644 --- a/arch/arm64/Makefile +++ b/arch/arm64/Makefile @@ -54,7 +54,7 @@ zinstall install: vmlinux $(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $@ %.dtb: - $(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@ + $(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) $(boot)/dts/$@ # We use MRPROPER_FILES and CLEAN_FILES now archclean: diff --git a/arch/arm64/boot/Makefile b/arch/arm64/boot/Makefile index eca209b..5a0e3ab 100644 --- a/arch/arm64/boot/Makefile +++ b/arch/arm64/boot/Makefile @@ -22,9 +22,6 @@ $(obj)/Image: vmlinux FORCE $(obj)/Image.gz: $(obj)/Image FORCE $(call if_changed,gzip) -$(obj)/%.dtb: $(src)/dts/%.dts - $(call cmd,dtc) - install: $(obj)/Image $(CONFIG_SHELL) $(srctree)/$(src)/install.sh $(KERNELRELEASE) \ $(obj)/Image System.map "$(INSTALL_PATH)" @@ -32,5 +29,3 @@ install: $(obj)/Image zinstall: $(obj)/Image.gz $(CONFIG_SHELL) $(srctree)/$(src)/install.sh $(KERNELRELEASE) \ $(obj)/Image.gz System.map "$(INSTALL_PATH)" - -clean-files += *.dtb diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile new file mode 100644 index 000..801e2d7 --- /dev/null +++ b/arch/arm64/boot/dts/Makefile @@ -0,0 +1,5 @@ +targets += dtbs + +dtbs: $(addprefix $(obj)/, $(dtb-y)) + +clean-files := *.dtb -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V7 6/7] microblaze: use new common dtc rule
From: Stephen Warren The current rules have the .dtb files build in a different directory from the .dts files. This patch changes microblaze to use the generic dtb rule which builds .dtb files in the same directory as the source .dts. This requires moving parts of arch/microblaze/boot/Makefile into newly created arch/microblaze/boot/dts/Makefile, and updating arch/microblaze/Makefile to call the new Makefile. linked_dtb.S is also moved into boot/dts/ since it's used by rules that were moved. Cc: Michal Simek Cc: microblaze-ucli...@itee.uq.edu.au Signed-off-by: Stephen Warren --- v7: New patch. --- arch/microblaze/Makefile |2 +- arch/microblaze/boot/Makefile | 19 +-- arch/microblaze/boot/dts/Makefile | 22 ++ arch/microblaze/boot/dts/linked_dtb.S |2 ++ arch/microblaze/boot/linked_dtb.S |3 --- 5 files changed, 26 insertions(+), 22 deletions(-) create mode 100644 arch/microblaze/boot/dts/Makefile create mode 100644 arch/microblaze/boot/dts/linked_dtb.S delete mode 100644 arch/microblaze/boot/linked_dtb.S diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile index b23c40e..d26fb90 100644 --- a/arch/microblaze/Makefile +++ b/arch/microblaze/Makefile @@ -57,7 +57,7 @@ boot := arch/microblaze/boot DTB:=$(subst simpleImage.,,$(filter simpleImage.%, $(MAKECMDGOALS))) ifneq ($(DTB),) - core-y += $(boot)/ + core-y += $(boot)/dts/ endif # defines filename extension depending memory management type diff --git a/arch/microblaze/boot/Makefile b/arch/microblaze/boot/Makefile index fa83ea4..80fe54f 100644 --- a/arch/microblaze/boot/Makefile +++ b/arch/microblaze/boot/Makefile @@ -2,21 +2,10 @@ # arch/microblaze/boot/Makefile # -obj-y += linked_dtb.o - targets := linux.bin linux.bin.gz simpleImage.% OBJCOPYFLAGS := -R .note -R .comment -R .note.gnu.build-id -O binary -# Ensure system.dtb exists -$(obj)/linked_dtb.o: $(obj)/system.dtb - -# Generate system.dtb from $(DTB).dtb -ifneq ($(DTB),system) -$(obj)/system.dtb: $(obj)/$(DTB).dtb - $(call if_changed,cp) -endif - $(obj)/linux.bin: vmlinux FORCE $(call if_changed,objcopy) $(call if_changed,uimage) @@ -45,10 +34,4 @@ $(obj)/simpleImage.%: vmlinux FORCE @echo 'Kernel: $@ is ready' ' (#'`cat .version`')' -# Rule to build device tree blobs -DTC_FLAGS := -p 1024 - -$(obj)/%.dtb: $(src)/dts/%.dts FORCE - $(call if_changed_dep,dtc) - -clean-files += *.dtb simpleImage.*.unstrip linux.bin.ub +clean-files += simpleImage.*.unstrip linux.bin.ub diff --git a/arch/microblaze/boot/dts/Makefile b/arch/microblaze/boot/dts/Makefile new file mode 100644 index 000..c3b3a5d --- /dev/null +++ b/arch/microblaze/boot/dts/Makefile @@ -0,0 +1,22 @@ +# +# arch/microblaze/boot/Makefile +# + +obj-y += linked_dtb.o + +# Ensure system.dtb exists +$(obj)/linked_dtb.o: $(obj)/system.dtb + +# Generate system.dtb from $(DTB).dtb +ifneq ($(DTB),system) +$(obj)/system.dtb: $(obj)/$(DTB).dtb + $(call if_changed,cp) +endif + +quiet_cmd_cp = CP $< $@$2 + cmd_cp = cat $< >$@$2 || (rm -f $@ && echo false) + +# Rule to build device tree blobs +DTC_FLAGS := -p 1024 + +clean-files += *.dtb diff --git a/arch/microblaze/boot/dts/linked_dtb.S b/arch/microblaze/boot/dts/linked_dtb.S new file mode 100644 index 000..23345af --- /dev/null +++ b/arch/microblaze/boot/dts/linked_dtb.S @@ -0,0 +1,2 @@ +.section __fdt_blob,"a" +.incbin "arch/microblaze/boot/dts/system.dtb" diff --git a/arch/microblaze/boot/linked_dtb.S b/arch/microblaze/boot/linked_dtb.S deleted file mode 100644 index cb2b537..000 --- a/arch/microblaze/boot/linked_dtb.S +++ /dev/null @@ -1,3 +0,0 @@ -.section __fdt_blob,"a" -.incbin "arch/microblaze/boot/system.dtb" - -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V7 5/7] c6x: use new common dtc rule
From: Stephen Warren
The current rules have the .dtb files build in a different directory
from the .dts files. This patch changes c6x to use the generic dtb
rule which builds .dtb files in the same directory as the source .dts.
This requires moving parts of arch/c6x/boot/Makefile into newly created
arch/c6x/boot/dts/Makefile, and updating arch/c6x/Makefile to call the
new Makefile. linked_dtb.S is also moved into boot/dts/ since it's used
by rules that were moved.
Cc: Mark Salter
Cc: Aurelien Jacquiot
Cc: linux-c6x-...@linux-c6x.org
Signed-off-by: Stephen Warren
---
v7: New patch.
---
arch/c6x/Makefile|2 +-
arch/c6x/boot/Makefile | 20
arch/c6x/boot/{ => dts}/Makefile | 14 ++
arch/c6x/boot/dts/linked_dtb.S |2 ++
arch/c6x/boot/linked_dtb.S |2 --
5 files changed, 5 insertions(+), 35 deletions(-)
copy arch/c6x/boot/{ => dts}/Makefile (50%)
create mode 100644 arch/c6x/boot/dts/linked_dtb.S
delete mode 100644 arch/c6x/boot/linked_dtb.S
diff --git a/arch/c6x/Makefile b/arch/c6x/Makefile
index a9eb959..e72eb34 100644
--- a/arch/c6x/Makefile
+++ b/arch/c6x/Makefile
@@ -41,7 +41,7 @@ DTB:=$(subst dtbImage.,,$(filter dtbImage.%, $(MAKECMDGOALS)))
export DTB
ifneq ($(DTB),)
-core-y += $(boot)/
+core-y += $(boot)/dts/
endif
# With make 3.82 we cannot mix normal and wildcard targets
diff --git a/arch/c6x/boot/Makefile b/arch/c6x/boot/Makefile
index 6891257..8734abe 100644
--- a/arch/c6x/boot/Makefile
+++ b/arch/c6x/boot/Makefile
@@ -6,25 +6,5 @@ OBJCOPYFLAGS_vmlinux.bin := -O binary
$(obj)/vmlinux.bin: vmlinux FORCE
$(call if_changed,objcopy)
-DTC_FLAGS ?= -p 1024
-
-ifneq ($(DTB),)
-obj-y += linked_dtb.o
-endif
-
-$(obj)/%.dtb: $(src)/dts/%.dts FORCE
- $(call if_changed_dep,dtc)
-
-quiet_cmd_cp = CP $< $@$2
- cmd_cp = cat $< >$@$2 || (rm -f $@ && echo false)
-
-# Generate builtin.dtb from $(DTB).dtb
-$(obj)/builtin.dtb: $(obj)/$(DTB).dtb
- $(call if_changed,cp)
-
-$(obj)/linked_dtb.o: $(obj)/builtin.dtb
-
$(obj)/dtbImage.%: vmlinux
$(call if_changed,objcopy)
-
-clean-files := $(obj)/*.dtb
diff --git a/arch/c6x/boot/Makefile b/arch/c6x/boot/dts/Makefile
similarity index 50%
copy from arch/c6x/boot/Makefile
copy to arch/c6x/boot/dts/Makefile
index 6891257..c7528b0 100644
--- a/arch/c6x/boot/Makefile
+++ b/arch/c6x/boot/dts/Makefile
@@ -1,20 +1,13 @@
#
-# Makefile for bootable kernel images
+# Makefile for device trees
#
-OBJCOPYFLAGS_vmlinux.bin := -O binary
-$(obj)/vmlinux.bin: vmlinux FORCE
- $(call if_changed,objcopy)
-
DTC_FLAGS ?= -p 1024
ifneq ($(DTB),)
obj-y += linked_dtb.o
endif
-$(obj)/%.dtb: $(src)/dts/%.dts FORCE
- $(call if_changed_dep,dtc)
-
quiet_cmd_cp = CP $< $@$2
cmd_cp = cat $< >$@$2 || (rm -f $@ && echo false)
@@ -24,7 +17,4 @@ $(obj)/builtin.dtb: $(obj)/$(DTB).dtb
$(obj)/linked_dtb.o: $(obj)/builtin.dtb
-$(obj)/dtbImage.%: vmlinux
- $(call if_changed,objcopy)
-
-clean-files := $(obj)/*.dtb
+clean-files := *.dtb
diff --git a/arch/c6x/boot/dts/linked_dtb.S b/arch/c6x/boot/dts/linked_dtb.S
new file mode 100644
index 000..cf347f1
--- /dev/null
+++ b/arch/c6x/boot/dts/linked_dtb.S
@@ -0,0 +1,2 @@
+.section __fdt_blob,"a"
+.incbin "arch/c6x/boot/dts/builtin.dtb"
diff --git a/arch/c6x/boot/linked_dtb.S b/arch/c6x/boot/linked_dtb.S
deleted file mode 100644
index 57a4454..000
--- a/arch/c6x/boot/linked_dtb.S
+++ /dev/null
@@ -1,2 +0,0 @@
-.section __fdt_blob,"a"
-.incbin "arch/c6x/boot/builtin.dtb"
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH V7 7/7] xtensa: use new common dtc rule
From: Stephen Warren The current rules have the .dtb files build in a different directory from the .dts files. This patch changes xtensa to use the generic dtb rule which builds .dtb files in the same directory as the source .dts. This requires moving parts of arch/xtensa/boot/Makefile into newly created arch/xtensa/boot/dts/Makefile, and updating arch/xtensa/Makefile to call the new Makefile. Cc: Chris Zankel Cc: Max Filippov Cc: linux-xte...@linux-xtensa.org Signed-off-by: Stephen Warren --- v7: New patch. --- arch/xtensa/Makefile |4 ++-- arch/xtensa/boot/Makefile | 10 -- arch/xtensa/boot/dts/Makefile | 13 + 3 files changed, 15 insertions(+), 12 deletions(-) create mode 100644 arch/xtensa/boot/dts/Makefile diff --git a/arch/xtensa/Makefile b/arch/xtensa/Makefile index b8eb819..b0646fd 100644 --- a/arch/xtensa/Makefile +++ b/arch/xtensa/Makefile @@ -80,7 +80,7 @@ core-y+= $(buildvar) $(buildplf) libs-y += arch/xtensa/lib/ $(LIBGCC) -core-$(CONFIG_BUILTIN_DTB_BOOL) += arch/xtensa/boot/ +core-$(CONFIG_BUILTIN_DTB_BOOL) += arch/xtensa/boot/dts/ boot := arch/xtensa/boot @@ -92,7 +92,7 @@ zImage: vmlinux $(Q)$(MAKE) $(build)=$(boot) $@ %.dtb: - $(Q)$(MAKE) $(build)=$(boot) $(boot)/$@ + $(Q)$(MAKE) $(build)=$(boot)/dts $(boot)/dts/$@ define archhelp @echo '* zImage - Compressed kernel image (arch/xtensa/boot/images/zImage.*)' diff --git a/arch/xtensa/boot/Makefile b/arch/xtensa/boot/Makefile index 5d21c21..6be27fb 100644 --- a/arch/xtensa/boot/Makefile +++ b/arch/xtensa/boot/Makefile @@ -25,16 +25,6 @@ bootdir-$(CONFIG_XTENSA_PLATFORM_ISS) += boot-elf bootdir-$(CONFIG_XTENSA_PLATFORM_XT2000) += boot-redboot boot-elf boot-uboot bootdir-$(CONFIG_XTENSA_PLATFORM_XTAVNET)+= boot-redboot boot-elf boot-uboot - -BUILTIN_DTB := $(patsubst "%",%,$(CONFIG_BUILTIN_DTB)).dtb.o -obj-$(CONFIG_BUILTIN_DTB_BOOL) += $(BUILTIN_DTB) - -# Rule to build device tree blobs -$(obj)/%.dtb: $(src)/dts/%.dts FORCE - $(call if_changed_dep,dtc) - -clean-files := *.dtb.S - zImage Image: $(bootdir-y) $(bootdir-y): $(addprefix $(obj)/,$(subdir-y)) \ diff --git a/arch/xtensa/boot/dts/Makefile b/arch/xtensa/boot/dts/Makefile new file mode 100644 index 000..0b5581a --- /dev/null +++ b/arch/xtensa/boot/dts/Makefile @@ -0,0 +1,13 @@ +# +# arch/xtensa/boot/dts/Makefile +# +# This file is subject to the terms and conditions of the GNU General Public +# License. See the file "COPYING" in the main directory of this archive +# for more details. +# +# + +BUILTIN_DTB := $(patsubst "%",%,$(CONFIG_BUILTIN_DTB)).dtb.o +obj-$(CONFIG_BUILTIN_DTB_BOOL) += $(BUILTIN_DTB) + +clean-files := *.dtb.S -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V7 4/7] openrisc: use new common dtc rule
From: Stephen Warren
The current rules have the .dtb files build in a different directory
from the .dts files. This patch changes openrisc to use the generic dtb
rule which builds .dtb files in the same directory as the source .dts.
This requires renaming arch/openrisc/boot/Makefile to
arch/openrisc/boot/dts/Makefile, and updating arch/openrisc/Makefile to
call the new Makefile.
Cc: Jonas Bonn
Cc: li...@lists.openrisc.net
Signed-off-by: Stephen Warren
---
v7: New patch.
---
arch/openrisc/Makefile|2 +-
arch/openrisc/boot/{ => dts}/Makefile |5 -
2 files changed, 1 insertion(+), 6 deletions(-)
rename arch/openrisc/boot/{ => dts}/Makefile (75%)
diff --git a/arch/openrisc/Makefile b/arch/openrisc/Makefile
index 966886c..4739b83 100644
--- a/arch/openrisc/Makefile
+++ b/arch/openrisc/Makefile
@@ -50,6 +50,6 @@ BUILTIN_DTB := y
else
BUILTIN_DTB := n
endif
-core-$(BUILTIN_DTB) += arch/openrisc/boot/
+core-$(BUILTIN_DTB) += arch/openrisc/boot/dts/
all: vmlinux
diff --git a/arch/openrisc/boot/Makefile b/arch/openrisc/boot/dts/Makefile
similarity index 75%
rename from arch/openrisc/boot/Makefile
rename to arch/openrisc/boot/dts/Makefile
index 0995835..b092d30 100644
--- a/arch/openrisc/boot/Makefile
+++ b/arch/openrisc/boot/dts/Makefile
@@ -1,5 +1,3 @@
-
-
ifneq '$(CONFIG_OPENRISC_BUILTIN_DTB)' '""'
BUILTIN_DTB := $(patsubst "%",%,$(CONFIG_OPENRISC_BUILTIN_DTB)).dtb.o
else
@@ -10,6 +8,3 @@ obj-y += $(BUILTIN_DTB)
clean-files := *.dtb.S
#DTC_FLAGS ?= -p 1024
-
-$(obj)/%.dtb: $(src)/dts/%.dts FORCE
- $(call if_changed_dep,dtc)
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH V7 1/7] kbuild: centralize .dts->.dtb rule
From: Stephen Warren
All architectures that use cmd_dtc do so in almost the same way. Create
a central build rule to avoid duplication. The one difference is that
most current uses of dtc build $(obj)/%.dtb from $(src)/dts/%.dts rather
than building the .dtb in the same directory as the .dts file. This
difference will be eliminated arch-by-arch in future patches.
MIPS is the exception here; it already uses the exact same rule as the
new common rule, so the duplicate is removed in this patch to avoid any
conflict. arch/mips changes courtesy of Ralf Baechle.
Update Documentation/kbuild to remove the explicit call to cmd_dtc from
the example, now that the rule exists in a centralized location.
Cc: Arnd Bergmann
Cc: linux-arm-ker...@lists.infradead.org
Cc: Olof Johansson
Cc: Russell King
Cc: Catalin Marinas
Cc: Jonas Bonn
Cc: li...@lists.openrisc.net
Cc: Aurelien Jacquiot
Cc: linux-c6x-...@linux-c6x.org
Cc: Mark Salter
Cc: Michal Simek
Cc: microblaze-ucli...@itee.uq.edu.au
Cc: Chris Zankel
Cc: linux-xte...@linux-xtensa.org
Cc: Max Filippov
Signed-off-by: Ralf Baechle
Signed-off-by: Stephen Warren
---
This is based on next-20121126.
I've split out this dtc rule cleanup as a separate patch series.
Hopefully it can be applied without too much controversy, then I'll move
back to discussing running cpp over *.dts.
v7:
* Build *.dtb from *.dts not src/*.dts.
* Removed all arch/ updates except MIPS.
v6: Added arch/{arm64,microblaze,mips} updates.
v5: Updated Documentation/kbuild.
v4: No change.
v3: No change.
v2: New patch.
---
Documentation/kbuild/makefiles.txt | 15 ---
arch/mips/cavium-octeon/Makefile |3 ---
arch/mips/lantiq/dts/Makefile |3 ---
arch/mips/netlogic/dts/Makefile|3 ---
scripts/Makefile.lib |3 +++
5 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/Documentation/kbuild/makefiles.txt
b/Documentation/kbuild/makefiles.txt
index ec9ae67..14c3f4f 100644
--- a/Documentation/kbuild/makefiles.txt
+++ b/Documentation/kbuild/makefiles.txt
@@ -1175,15 +1175,16 @@ When kbuild executes, the following steps are followed
(roughly):
in an init section in the image. Platform code *must* copy the
blob to non-init memory prior to calling unflatten_device_tree().
- Example:
- #arch/x86/platform/ce4100/Makefile
- clean-files := *dtb.S
+ To use this command, simply add *.dtb into obj-y or targets, or make
+ some other target depend on %.dtb
- DTC_FLAGS := -p 1024
- obj-y += foo.dtb.o
+ A central rule exists to create $(obj)/%.dtb from $(src)/%.dts;
+ architecture Makefiles do no need to explicitly write out that rule.
- $(obj)/%.dtb: $(src)/%.dts
- $(call cmd,dtc)
+ Example:
+ targets += $(dtb-y)
+ clean-files += *.dtb
+ DTC_FLAGS ?= -p 1024
--- 6.8 Custom kbuild commands
diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile
index bc96e29..6e927cf 100644
--- a/arch/mips/cavium-octeon/Makefile
+++ b/arch/mips/cavium-octeon/Makefile
@@ -24,9 +24,6 @@ DTB_FILES = $(patsubst %.dts, %.dtb, $(DTS_FILES))
obj-y += $(patsubst %.dts, %.dtb.o, $(DTS_FILES))
-$(obj)/%.dtb: $(src)/%.dts FORCE
- $(call if_changed_dep,dtc)
-
# Let's keep the .dtb files around in case we want to look at them.
.SECONDARY: $(addprefix $(obj)/, $(DTB_FILES))
diff --git a/arch/mips/lantiq/dts/Makefile b/arch/mips/lantiq/dts/Makefile
index 674fca4..6fa72dd 100644
--- a/arch/mips/lantiq/dts/Makefile
+++ b/arch/mips/lantiq/dts/Makefile
@@ -1,4 +1 @@
obj-$(CONFIG_DT_EASY50712) := easy50712.dtb.o
-
-$(obj)/%.dtb: $(obj)/%.dts
- $(call if_changed,dtc)
diff --git a/arch/mips/netlogic/dts/Makefile b/arch/mips/netlogic/dts/Makefile
index 67ae3fe2..d117d46 100644
--- a/arch/mips/netlogic/dts/Makefile
+++ b/arch/mips/netlogic/dts/Makefile
@@ -1,4 +1 @@
obj-$(CONFIG_DT_XLP_EVP) := xlp_evp.dtb.o
-
-$(obj)/%.dtb: $(obj)/%.dts
- $(call if_changed,dtc)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 0be6f11..bdf42fd 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -266,6 +266,9 @@ $(obj)/%.dtb.S: $(obj)/%.dtb
quiet_cmd_dtc = DTC $@
cmd_dtc = $(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 $(DTC_FLAGS) -d
$(depfile) $<
+$(obj)/%.dtb: $(src)/%.dts FORCE
+ $(call if_changed_dep,dtc)
+
# Bzip2
# ---
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH V7 2/7] ARM: dt: change .dtb build rules to build in dts directory
From: Grant Likely The current rules have the .dtb files build in a different directory from the .dts files. The only reason for this is that it was what PowerPC has done historically. This patch changes ARM to use the generic dtb rule which builds .dtb files in the same directory as the source .dts. Cc: Russell King Cc: Arnd Bergmann Cc: Olof Johansson Cc: linux-arm-ker...@lists.infradead.org Signed-off-by: Grant Likely [swarren: added rm command for old stale .dtb files] Signed-off-by: Stephen Warren --- v7: New patch. --- arch/arm/Makefile |4 ++-- arch/arm/boot/Makefile | 12 arch/arm/boot/dts/Makefile |8 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/arch/arm/Makefile b/arch/arm/Makefile index 1ec5f67..5f92252 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -291,10 +291,10 @@ zinstall uinstall install: vmlinux $(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $@ %.dtb: scripts - $(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@ + $(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) $(boot)/dts/$@ dtbs: scripts - $(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@ + $(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) dtbs # We use MRPROPER_FILES and CLEAN_FILES now archclean: diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile index 9137df5..abfce28 100644 --- a/arch/arm/boot/Makefile +++ b/arch/arm/boot/Makefile @@ -15,8 +15,6 @@ ifneq ($(MACHINE),) include $(srctree)/$(MACHINE)/Makefile.boot endif -include $(srctree)/arch/arm/boot/dts/Makefile - # Note: the following conditions must always be true: # ZRELADDR == virt_to_phys(PAGE_OFFSET + TEXT_OFFSET) # PARAMS_PHYS must be within 4MB of ZRELADDR @@ -59,16 +57,6 @@ $(obj)/zImage: $(obj)/compressed/vmlinux FORCE endif -targets += $(dtb-y) - -# Rule to build device tree blobs -$(obj)/%.dtb: $(src)/dts/%.dts FORCE - $(call if_changed_dep,dtc) - -$(obj)/dtbs: $(addprefix $(obj)/, $(dtb-y)) - -clean-files := *.dtb - ifneq ($(LOADADDR),) UIMAGE_LOADADDR=$(LOADADDR) else diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index a17d5ab..cb217f8 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -136,4 +136,12 @@ dtb-$(CONFIG_ARCH_VT8500) += vt8500-bv07.dtb \ wm8650-mid.dtb dtb-$(CONFIG_ARCH_ZYNQ) += zynq-zc702.dtb +targets += dtbs endif + +dtbs: $(addprefix $(obj)/, $(dtb-y)) + # *.dtb used to be generated in the directory above. Clean out the + # old build results so people don't accidentally use them. + rm -f $(obj)/../*.dtb + +clean-files := *.dtb -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC v2 8/8] drm: tegra: Add gr2d device
On 11/28/2012 07:45 AM, Terje Bergström wrote: > On 28.11.2012 16:06, Lucas Stach wrote: >> Why do even need/use dma-buf for this use case? This is all one DRM >> device, even if we separate host1x and gr2d as implementation modules. > > I didn't want to implement dependency to drm gem objects in nvhost, but > we have thought about doing that. dma-buf brings quite a lot of > overhead, so implementing support for gem buffers would make the > sequence a bit leaner. > > nvhost already has infra to support multiple memory managers. > >> So standard way of doing this is: >> 1. create gem object for pushbuffer >> 2. create fake mmap offset for gem obj >> 3. map pushbuf using the fake offset on the drm device >> 4. at submit time zap the mapping >> >> You need this logic anyway, as normally we don't rely on userspace to >> sync gpu and cpu, but use the kernel to handle the concurrency issues. > > Taking a step back - 2D streams are actually very short, in the order of > <100 bytes. Just copying them to kernel space would actually be faster > than doing MMU operations. I'm not sure it's a good idea to have one buffer submission mechanism for the 2D class and another for the 3D/... class, nor to bet that 2D streams will always be short. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] ARM: tegra: bus_notifier registers IOMMU devices(was: How to specify IOMMU'able devices in DT)
On 11/28/2012 06:48 AM, Hiroshi Doyu wrote:
> Hiroshi Doyu wrote @ Mon, 24 Sep 2012 14:50:14 +0300
> (EEST):
> ...
> On Mon, 2012-09-24 at 12:04 +0300, Hiroshi Doyu wrote:
>> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
>> index a1a7225..9eae3be 100644
>> --- a/drivers/base/platform.c
>> +++ b/drivers/base/platform.c
>> @@ -21,6 +21,8 @@
>> #include
>> #include
>>
>> +#include
>> +
>> #include "base.h"
>>
>> #define to_platform_driver(drv)(container_of((drv), struct
>> platform_driver, \
>> @@ -305,8 +307,19 @@ int platform_device_add(struct platform_device
>> *pdev)
>> dev_name(&pdev->dev), dev_name(pdev->dev.parent));
>>
>> ret = device_add(&pdev->dev);
>> - if (ret == 0)
>> - return ret;
>> + if (ret)
>> + goto failed;
>> +
>> +#ifdef CONFIG_PLATFORM_ENABLE_IOMMU
>> + if (platform_bus_type.map && !pdev->dev.archdata.mapping) {
>> + ret = arm_iommu_attach_device(&pdev->dev,
>> + platform_bus_type.map);
>> + if (ret)
>> + goto failed;
>
> This is horrible ... you're adding an architecture specific callback
> into our generic code; that's really a no-no. If the concept of
> CONFIG_PLATFORM_ENABE_IOMMU is useful to more than just arm, then this
> could become a generic callback.
As mentioned in the original, this is a heck to explain what is
needed. I am looking for some generic solution for how to specify
IOMMU info for each platform devices. I'm guessing that some other SoC
may have the similar requirements on the above. As you mentioned, this
solution should be a generic, not arch specific.
>>>
>>> Please read more about bus notifiers. IMHO a good example is provided in
>>> the following thread:
>>> http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg12238.html
>>
>> This bus notifier seems enough flexible to afford the variation of
>> IOMMU map info, like Tegra ASID, which could be platform-specific, and
>> the other could be common too. There's already iommu_bus_notifier
>> too. I'll try to implement something base on this.
>
> Experimentally implemented as below. With the followig patch, each
> device could specify its own map in DT, and automatically the device
> would be attached to the map.
>
> There is a case that some devices share a map. This patch doesn't
> suppor such case yet.
>
> From 8cb75bb6f3a8535a077e0e85265f87c1f1289bfd Mon Sep 17 00:00:00 2001
> From: Hiroshi Doyu
> Date: Wed, 28 Nov 2012 14:47:04 +0200
> Subject: [PATCH 1/1] ARM: tegra: bus_notifier registers IOMMU devices
>
> platform_bus notifier registers IOMMU devices if dma-window is
> specified.
>
> Its format is:
> dma-window = <"start" "size">;
> ex)
> dma-window = <0x12345000 0x8000>;
>
> Signed-off-by: Hiroshi Doyu
> ---
> arch/arm/mach-tegra/board-dt-tegra30.c | 40
>
Shouldn't this patch be to the IOMMU driver itself, not the core Tegra code?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V7 1/7] kbuild: centralize .dts->.dtb rule
On 11/27/2012 04:29 PM, Stephen Warren wrote: > All architectures that use cmd_dtc do so in almost the same way. Create > a central build rule to avoid duplication. The one difference is that > most current uses of dtc build $(obj)/%.dtb from $(src)/dts/%.dts rather > than building the .dtb in the same directory as the .dts file. This > difference will be eliminated arch-by-arch in future patches. > > MIPS is the exception here; it already uses the exact same rule as the > new common rule, so the duplicate is removed in this patch to avoid any > conflict. arch/mips changes courtesy of Ralf Baechle. > > Update Documentation/kbuild to remove the explicit call to cmd_dtc from > the example, now that the rule exists in a centralized location. Ben, Paul, Following this patch (http://lkml.org/lkml/2012/11/27/555), I posted a series of patches to convert almost all architectures to using the centralized rule. The one architecture I didn't convert was PowerPC. I didn't convert it because arch/powerpc/boot/Makefile contains a large number of rules (to generate *Image.% where % is a board name) that depend on %.dtb, which is expected to be in arch/powerpc/boot rather than arch/powerpc/boot/dts. Now, I guess it's possible to convert them all to expect the .dtb files to be in dts/ and also have arch/powerpc/boot/Makefile call make in boot/dts/ to make each required .dtb file. However, the patch would be a bit larger than all the other architecture patches. Do you want me to do that conversion, or would you rather I leave PowerPC alone? Thanks for any feedback. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] drm: tegra: Add maintainers entry
On 11/28/2012 12:18 PM, Thierry Reding wrote: > Add myself as the maintainer of the NVIDIA Tegra DRM driver. Aside from one comment below, Acked-by: Stephen Warren > +L: dri-de...@lists.freedesktop.org Should linux-te...@vger.kernel.org also be CC'd so that everything Tegra-related goes to one place? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: tegra: moving stuff away from mach/clk.h
On 11/28/2012 11:12 PM, Prashant Gaikwad wrote: > On Tuesday 27 November 2012 12:29 PM, Sivaram Nair wrote: >> This patch moves some stuff away from mach/clk.h to other mach-tegra >> files. This is part of the efforts to get rid of mach/clk.h which in >> turn will help to enable single zImage. >> >> Signed-off-by: Sivaram Nair >> --- >> arch/arm/mach-tegra/clock.c|1 + >> arch/arm/mach-tegra/clock.h| 12 ++-- >> arch/arm/mach-tegra/include/mach/clk.h | 11 --- >> 3 files changed, 11 insertions(+), 13 deletions(-) > > This patch will not add any real value since I have removed clk_cfg_ex > functionality in clock code rework. > It's just that I will have to rebase my changes on this. > > Reviewed-by: Prashant Gaikwad In that case, I may as well not apply patch 2/2 at all, and I'll just take your clock code cleanup instead. I assume that will be posted for inclusion into 3.9? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC v2 1/8] video: tegra: Add nvhost driver
On 11/29/2012 03:21 AM, Terje Bergström wrote:
> On 28.11.2012 23:23, Thierry Reding wrote:
...
>>> + regs = platform_get_resource(dev, IORESOURCE_MEM, 0);
>>> + intr0 = platform_get_resource(dev, IORESOURCE_IRQ, 0);
>>> + intr1 = platform_get_resource(dev, IORESOURCE_IRQ, 1);
>>> +
>>> + if (!regs || !intr0 || !intr1) {
>>
>> I prefer to have these checked for explicitly, one by one for
>> readability and potentially more useful diagnostics.
>
> Can do.
>
>> Also you should be using platform_get_irq() for interrupts. Furthermore
>> the host1x DT node (and the TRM) name the interrupts "syncpt" and
>> "general", so maybe those would be more useful variable names than
>> "intr0" and "intr1".
>>
>> But since you don't use them anyway they shouldn't be part of this
>> patch.
>
> True. I might also as well delete the general interrupt altogether, as
> we don't use it for any real purpose.
Do make sure the interrupts still are part of the DT binding though, so
that the binding fully describes the HW, and the interrupt is available
to retrieve if we ever do use it in the future.
>>> + for (i = 0; i < pdata->num_clks; i++)
>>> + clk_prepare_enable(pdata->clk[i]);
>>> + nvhost_syncpt_reset(&host->syncpt);
>>> + for (i = 0; i < pdata->num_clks; i++)
>>> + clk_disable_unprepare(pdata->clk[i]);
>>
>> Stephen already hinted at this when discussing the AUXDATA. You should
>> explicitly request the clocks.
>
> I'm not too happy about that idea. The clock management code is generic
> for all modules, and that's why it's driven by a data structure. Now
> Stephen and you ask me to unroll the loops and make copies of the code
> to facilitate different modules and different SoCs.
You can still create tables of clocks inside the driver and loop over
them. So, loop unrolling isn't related to my comments at least. It's
just that clk_get() shouldn't take its parameters from platform data.
But if these are clocks for (arbitrary) child modules (that may or may
not exist dynamically), why aren't the drivers for the child modules
managing them?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC v2 1/8] video: tegra: Add nvhost driver
On 11/29/2012 04:47 AM, Thierry Reding wrote: > On Thu, Nov 29, 2012 at 12:21:04PM +0200, Terje Bergström wrote: >> On 28.11.2012 23:23, Thierry Reding wrote: >>> This could be problematic. Since drivers/video and >>> drivers/gpu/drm are separate trees, this would entail a >>> continuous burden on keeping both trees synchronized. While I >>> realize that eventually it might be better to put the host1x >>> driver in a separate place to accomodate for its use by other >>> subsystems, I'm not sure moving it here right away is the best >>> approach. >> >> I understand your point, but I hope also that we'd end up with >> something that can be used as basis for the downstream kernel to >> migrate to upstream stack. >> >> The key point here is to make the API between nvhost and tegradrm >> as small and robust to changes as possible. > > I agree. But I also fear that there will be changes eventually and > having both go in via different tree requires those trees to be > merged in a specific order to avoid breakage should the API change. > This will be particularly ugly in linux-next. > > That's why I explicitly proposed to take this into > drivers/gpu/drm/tegra for the time being, until we can be > reasonably sure that the API is fixed. Then I'm fine with moving it > wherever seems the best fit. Even then there might be the > occasional dependency, but they should get fewer and fewer as the > code matures. It is acceptable for one maintainer to ack patches, and another maintainer to merge a series that touches both "their own" code and code owned by another tree. This should of course only be needed when inter-module APIs change; changes to code within a module shouldn't require this. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC v2 2/8] video: tegra: Add syncpoint wait and interrupts
On 11/29/2012 01:44 AM, Thierry Reding wrote:
> On Mon, Nov 26, 2012 at 03:19:08PM +0200, Terje Bergstrom wrote:
>> diff --git a/drivers/video/tegra/host/host1x/host1x_intr.c
>> b/drivers/video/tegra/host/host1x/host1x_intr.c
> [...]
>> +/* Spacing between sync registers */ +#define REGISTER_STRIDE 4
>
> Erm... no. The usual way you should be doing this is either make
> the register definitions account for the stride or use accessors
> that apply the stride. You should be doing the latter anyway to
> make accesses. For example:
>
> static inline void host1x_syncpt_writel(struct host1x *host1x,
> unsigned long value, unsigned long offset) { writel(value,
> host1x->regs + SYNCPT_BASE + offset); }
>
> static inline unsigned long host1x_syncpt_readl(struct host1x
> *host1x, unsigned long offset) { return readl(host1x->regs +
> SYNCPT_BASE + offset); }
>
> Alternatively, if you want to pass the register index instead of
> the offset, you can use just multiply the offset in that function:
>
> writel(value, host1x->regs + SYNCPT_BASE + (offset << 2));
>
> The same can also be done with the non-syncpt registers.
It seems like reasonable documentation to replace "<< 2" with "*
REGISTER_STRIDE" here.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: manual merge of the bcm2835 tree with the arm-soc tree
On 03/17/2013 10:04 PM, Stephen Rothwell wrote:
> Hi Stephen,
>
> Today's linux-next merge of the bcm2835 tree got a conflict in
> arch/arm/Kconfig between commit 15bc1fe67f66 ("ARM: cns3xxx:
> enable multiplatform support") from the arm-soc tree and commit
> f1ac922dec7e ("ARM: bcm2835: convert to multi-platform") from the
> bcm2835 tree.
Thanks. The resolution for the two bcm2835/arm-soc conflicts looks
fine. These conflicts should go away once I send bcm2835 pull requests
to arm-soc for inclusion in 3.10.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: manual merge of the tegra tree with the arm-soc tree
On 03/17/2013 10:31 PM, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the tegra tree got a conflict in
> drivers/clocksource/tegra20_timer.c between commit 1d16cfb3aeba
> ("clocksource: tegra20: use the device_node pointer passed to
> init") from the arm-soc tree and commit 6f88fb8af6c6 ("clocksource:
> tegra: move to of_clk_get") from the tegra tree.
>
> I fixed it up (see below) and can carry the fix as necessary (no
> action is required).
Thanks. This resolution looks fine.
Arnd/Olof, do you want me to rebase the Tegra branch onto some arm-soc
branch to resolve this conflict, or are you happy to simply resolve it
as below when pulling the Tegra branches into arm-soc for 3.10?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/7] kbuild: create an "include chroot" for DT bindings
On 03/18/2013 09:13 AM, Shawn Guo wrote: > On Wed, Mar 06, 2013 at 11:22:57AM -0700, Stephen Warren wrote: >> From: Stephen Warren >> >> The recent dtc+cpp support allows header files and C pre-processor >> defines/macros to be used when compiling device tree files. These >> headers will typically define various constants that are part of the >> device tree bindings. >> >> The original patch which set up the dtc+cpp include path only considered >> using those headers from device tree files. However, most are also >> useful for kernel code which needs to interpret the device tree. >> >> In both the DT files and the kernel, I'd like to include the DT-related >> headers in the same way, for example, . >> That will simplify any text which discusses the DT header locations. >> >> Creating a for kernel source to use is as simple as >> placing files into include/dt-bindings/. >> >> However, when compiling DT files, the include path should be restricted >> so that only the dt-bindings path is available; arbitrary kernel headers >> shouldn't be exposed. For this reason, create a specific include >> directory for use by dtc+cpp, and symlink dt-bindings from there to the >> actual location of include/dt-bindings/. For want of a better location, >> place this "include chroot" into the existing dts/ directory. >> >> arch/*/boot/dts/include/dt-bindings -> ../../../../../include/dt-bindings >> >> Some headers used by device tree files may not be useful to the kernel; >> they may be used simply to aid in constructing the DT file (e.g. macros >> to create a node), but not define any information that the kernel needs >> to share. These may be placed directly into arch/*/boot/dts/ along with >> the DT files themselves. >> >> Cc: Shawn Guo >> Cc: Hiroshi Doyu >> Acked-by: Michal Marek >> Acked-by: Shawn Guo >> Acked-by: Rob Herring >> Signed-off-by: Stephen Warren >> --- >> This first patch was previously posted separately as V2. I'm including >> it here to keep it with the whole series. I hope this series can be >> applied as a topic branch in arm-soc. > > So no comments on the series means that we can start maintain it on > a topic branch, so that people can base their works on it? I was thinking of reposting the series today due to lack of responses, so that the content was freshly available in people's inboxes. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] regmap: core: Split out in place value parsing
On 03/03/2013 06:14 PM, Mark Brown wrote: > Currently the value parsing operations both return the parsed value and > modify the passed buffer. This precludes their use in places like the cache > code so split out the in place modification into a new parse_inplace() > operation. Mark, This change seems to break audio on my Tegra system with a WM8903 CODEC. In next-20130318, reverting this plus "regmap: cache: Store caches in native register format where possible" which depends on it does solve the problem. Looking at the raw WM8903 registers using i2cdump shows that for example without these patches, register 39h is set to 0x0020, but with them it's set to 0x0021, so the headphones are muted. In both cases, the regmap debugfs file thinks the value last written is 0xa1 (the 0x80 bit isn't actually state stored in HW, so the only relevant difference is in the LSB). There are numerous other differences in i2cdump output. It took a very quick look at the patch and couldn't see anything actively wrong. I wonder if one of the existing unconverted users of .parse_val() relies on the in-place modification of the buffer as well as getting the result back, and so should have been converted to calling both .parse_inplace() and then .parse_val()? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mfd: palmas: initialise client->of_node for dummy created client
On 03/19/2013 02:58 AM, Laxman Dewangan wrote: > Palmas device have three different i2c addresses. The device creates > the two new dummy i2c clients for accessing the register by using > primary client adapter. This new dummy i2c client have their of_node > as NULL. > > The dummy i2c client is used for registering interrupt and on this, > it creates irq domain handle. This created irq domain handle has > their of_node as NULL. It seems like part of the solution here is to modify the i2c_client object itself so that it can directly support devices that have multiple I2C addresses; instead of 1 i2c_client representing 1 address, 1 i2c_client could represent a list of addresses, that list being populated directly from the list contained in the top-level node's reg property. That way, you wouldn't need any dummy i2c_clients, which would avoid this issue. If the I2C device itself has multiple I2C addresses, they really should all be explicitly listed in the device tree reg property. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mfd: palmas: initialise client->of_node for dummy created client
On 03/19/2013 10:52 AM, Mark Brown wrote: > On Tue, Mar 19, 2013 at 09:44:24AM -0600, Stephen Warren wrote: > >> It seems like part of the solution here is to modify the >> i2c_client object itself so that it can directly support devices >> that have multiple I2C addresses; instead of 1 i2c_client >> representing 1 address, 1 i2c_client could represent a list of >> addresses, that list being populated directly from the list >> contained in the top-level node's reg property. That way, you >> wouldn't need any dummy i2c_clients, which would avoid this >> issue. > > This does then make it more complicated for all users of I2C as > they need to become aware of such devices. Not sure that's a > win... Presumably the existing APIs would work identically, and additional APIs would be added for the complex case? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Doc: dt: i2c: tegra: add dt binding for i2c-tegra
On 03/18/2013 03:35 AM, Laxman Dewangan wrote: > Add documentation for device tree binding of NVIDIA's Tegra i2c > controller driver. > > Describing all compatible values used for diffenent Tegra SoCs > in details in this documentation. I have applied this to Tegra's for-3.10/dt branch. I fixed a couple of typos and capitalization errors. I also trimmed the patch to include only a single example; the description in the text is enough to get the rest right, so there's no need to include exhaustive examples. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: tegra114: add speedo-based process identification
On 03/18/2013 05:17 AM, Danny Huang wrote: > Add speedo-based process identifictaion for Tegra114. I have applied this to Tegra's for-3.10/soc branch, with one addition below: > diff --git a/arch/arm/mach-tegra/tegra114_speedo.c > b/arch/arm/mach-tegra/tegra114_speedo.c > +#include Here, I also added: #include That's not needed in the Tegra for-next branch (which is based on 3.9-rc1), but is needed when merged into linux-next, and hence will presumably be needed once this is submitted into 3.10. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ 12/75] USB: EHCI: work around silicon bug in Intels EHCI controllers
On 03/18/2013 03:06 PM, Greg Kroah-Hartman wrote: > 3.8-stable review patch. If anyone has any objections, please let me know. This patch causes reboot/shutdown to fail on Tegra-based systems in 3.9-rc3. I reported this at: http://www.spinics.net/lists/linux-usb/msg82518.html although there's been no discussion there yet, and I haven't had time to investigate exactly what the problem is. > -- > > From: Alan Stern > > commit 6402c796d3b4205d3d7296157956c5100a05d7d6 upstream. > > This patch (as1660) works around a hardware problem present in some > (if not all) Intel EHCI controllers. After a QH has been unlinked > from the async schedule and the corresponding IAA interrupt has > occurred, the controller is not supposed access the QH and its qTDs. > There certainly shouldn't be any more DMA writes to those structures. > Nevertheless, Intel's controllers have been observed to perform a > final writeback to the QH's overlay region and to the most recent qTD. > For more information and a test program to determine whether this > problem is present in a particular controller, see > > http://marc.info/?l=linux-usb&m=135492071812265&w=2 > http://marc.info/?l=linux-usb&m=136182570800963&w=2 > > This patch works around the problem by always waiting for two IAA > cycles when unlinking an async QH. The extra IAA delay gives the > controller time to perform its final writeback. > > Surprisingly enough, the effects of this silicon bug have gone > undetected until quite recently. More through luck than anything > else, it hasn't caused any apparent problems. However, it does > interact badly with the path that follows this one, so it needs to be > addressed. > > This is the first part of a fix for the regression reported at: > > https://bugs.launchpad.net/bugs/1088733 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra: fuse: add fuctions to read speedo id and process id
On 03/18/2013 08:33 PM, Danny Huang wrote: > Add functions to read the speedo and process id of both cpu and soc. > There might be some drivers need the information as well. What code wants to use these functions? It'd be best to submit this patch with that code, so that something actually uses the functions, and they don't look like dead code. In the past, functions similar to this used to exist. However, they were removed and replaced by direct access to the underlying variables. Do we actually need functions for this, or can code simply read from the variables instead? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/7] usb: phy: tegra: Add error handling & clean up.
On 03/18/2013 06:29 AM, Venu Byravarasu wrote: > Check return values from all GPIO APIs and handle errors accordingly. > Remove clk_disable_unprepare which is no more needed. Patches 6 and 7 each fail checkpatch with "WARNING: line over 80 characters". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/7] USB: PHY: Tegra: registering TEGRA USB PHY as platform driver
On 03/18/2013 06:29 AM, Venu Byravarasu wrote: > As part of this series, apart from patch containing changes to register TEGRA > USB PHY driver as platform driver, prepared below patches: > 1. Re-arranging & adding new DT properties. > 2. Getting various params from DT properties added. > 3. code clean up. Venu, I'm curious whether these patches were tested at all. I have found at least two significant problems with trivial testing: 1) "reboot" or "shutdown -h now" both cause the following crash, with or without any USB devices plugged in (or ever having been plugged in): > [ 355.836288] Unable to handle kernel NULL pointer dereference at virtual > address 0028 > [ 355.847961] pgd = ed62 > [ 355.854093] [0028] *pgd= ... > [ 356.146728] [] (tegra_ehci_hcd_shutdown+0x18/0x2c) from > [] (platform_drv_shutdown+0x18/0x1c) > [ 356.160379] [] (platform_drv_shutdown+0x18/0x1c) from > [] (device_shutdown+0x34/0x188) > [ 356.173464] [] (device_shutdown+0x34/0x188) from [] > (kernel_restart_prepare+0x34/0x3c) > [ 356.186668] [] (kernel_restart_prepare+0x34/0x3c) from > [] (kernel_restart+0xc/0x4c) > [ 356.199637] [] (kernel_restart+0xc/0x4c) from [] > (sys_reboot+0x1ac/0x1d8) > [ 356.211704] [] (sys_reboot+0x1ac/0x1d8) from [] > (ret_fast_syscall+0x0/0x30) > [ 356.223965] Code: ebfe4b27 e5903000 e24300e8 e5133044 (e5933028) > [ 356.233896] ---[ end trace 088d89482b4af176 ]--- 2) The first time enumeration USB devices is attempted on a port fails. For devices that are plugged in at boot, this means that to get them working, they must be unplugged and re-plugged after boot. For devices that are not plugged in at boot, this means they must be plugged, unplugged, and then plugged in again. This is obviously problematic in and of itself. This is especially true for boards like Harmony that have a built-in USB hub and network chip. I didn't actually test this, but I assume that they cannot be made to work at all with this patch series, since they cannot be unplugged. The failed enumeration is accompanied by the following message: [2.451530] hub 3-0:1.0: unable to enumerate USB device on port 1 Both of these problems reproduce on at least boards Ventana and Seaboard(Springbank), although I assume that all boards are affected. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/7] ARM: tegra: update device trees for USB binding rework
On 03/18/2013 06:29 AM, Venu Byravarasu wrote: > This patch updates all Tegra board files so that they contain all the > properties required by the updated USB DT binding. Note that this patch > only adds the new properties and does not yet remove the old properties, > in order to maintain bisectability. The old properties will be removed > once the driver has been updated to assume the new bindings. > > Signed-off-by: Venu Byravarasu > --- > arch/arm/boot/dts/tegra20-colibri-512.dtsi |4 +++ > arch/arm/boot/dts/tegra20-harmony.dts |8 +++--- > arch/arm/boot/dts/tegra20-iris-512.dts |4 +++ > arch/arm/boot/dts/tegra20-paz00.dts|8 +++--- > arch/arm/boot/dts/tegra20-seaboard.dts | 13 +++--- > arch/arm/boot/dts/tegra20-trimslice.dts| 12 +++--- > arch/arm/boot/dts/tegra20-ventana.dts |7 +++-- > arch/arm/boot/dts/tegra20.dtsi | 32 +-- > 8 files changed, 57 insertions(+), 31 deletions(-) I think you forgot to update arch/arm/boot/dts/tegra20-whistler.dts in this patch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] usb: phy: tegra: Get PHY mode using DT
On 03/18/2013 06:29 AM, Venu Byravarasu wrote:
> Added a new PHY mode to support OTG.
> Obtained Tegra USB PHY mode using DT property.
> diff --git a/drivers/usb/phy/tegra_usb_phy.c b/drivers/usb/phy/tegra_usb_phy.c
> @@ -713,6 +712,16 @@ struct tegra_usb_phy *tegra_usb_phy_open(struct device
> *dev, int instance,
> else
> phy->is_ulpi_phy = true;
>
> + err = of_property_match_string(np, "dr_mode", "otg");
> + if (err < 0) {
> + err = of_property_match_string(np, "dr_mode", "gadget");
> + if (err < 0)
The binding says the 3 legal values for this property are "host",
"peripheral", or "otg". This agrees with the usage in
Documentation/devicetree/bindings/usb/fsl-usb.txt and
drivers/usb/host/fsl-mph-dr-of.c. So, "gadget" is not something the code
should be checking for.
I'm sure I pointed this out before.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/7] usb: phy: tegra: get ULPI reset GPIO info using DT.
On 03/18/2013 06:29 AM, Venu Byravarasu wrote: > As GPIO information is avail through DT, used it to get Tegra ULPI > reset GPIO number. Added a new member to tegra_usb_phy structure to > store this number. > diff --git a/drivers/usb/phy/tegra_usb_phy.c b/drivers/usb/phy/tegra_usb_phy.c > - gpio_direction_output(config->reset_gpio, 0); > + gpio_direction_output(phy->reset_gpio, 0); > msleep(5); > - gpio_direction_output(config->reset_gpio, 1); > + gpio_direction_output(phy->reset_gpio, 1); That implies that the PHY reset signal is active-low. This should be represented in the GPIO flags in the device tree. In other words, instead of e.g.: nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */ you want: nvidia,phy-reset-gpio = <&gpio 169 1>; /* gpio PV1 */ Flag 1 means active-low. See Documentation/devicetree/bindings/gpio/nvidia,tegra20-gpio.txt. This is a bug in the current device tree content, although it has no effect since no code currently uses the GPIO flags from DT. I suggest creating a separate patch to fix this, and inserting it between patch 1 and 2 of the series. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/7] usb: phy: tegra: Add error handling & clean up.
On 03/18/2013 06:29 AM, Venu Byravarasu wrote: > Check return values from all GPIO APIs and handle errors accordingly. > Remove clk_disable_unprepare which is no more needed. The call to clk_disable_unprepare is incorrect in the current code. The way you worded that, it sounds like it's no longer needed because of the changes made in this patch. I would re-write that last sentence as: Remove the call to clk_disable_unprepare(); this function does not prepare or enable the clock, so the error path should not disable or unprepare it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/7] usb: phy: registering tegra USB PHY as platform driver
On 03/18/2013 06:29 AM, Venu Byravarasu wrote:
> Registered tegra USB PHY as a separate platform driver.
>
> To synchronize host controller and PHY initialization, used deferred
> probe mechanism. As PHY should be initialized before EHCI starts running,
> deferred probe of Tegra EHCI driver till PHY probe gets completed.
>
> Got rid of instance number based handling in host driver.
>
> Made use of DT params to get the PHY Pad registers.
>
> Merged tegra_phy_init into tegra_usb_phy_init.
> diff --git a/drivers/usb/phy/tegra_usb_phy.c b/drivers/usb/phy/tegra_usb_phy.c
> static void tegra_usb_phy_close(struct usb_phy *x)
> {
> struct tegra_usb_phy *phy = container_of(x, struct tegra_usb_phy,
> u_phy);
>
> if (phy->is_ulpi_phy)
> clk_put(phy->clk);
phy->clk is obtained using devm_clk_get(). This typically means you
never need to clk_put() it, and if for some reason you really have to,
you should use devm_clk_put() instead of plain clk_put().
> clk_put(phy->pll_u);
Same here.
> @@ -774,23 +667,53 @@ struct tegra_usb_phy *tegra_usb_phy_open(struct device
> *dev, int instance,
> + if (phy->is_ulpi_phy) {
> + phy->clk = devm_clk_get(phy->dev, "ulpi-link");
> + if (IS_ERR(phy->clk)) {
> + pr_err("%s: can't get ulpi clock\n", __func__);
> + err = PTR_ERR(phy->clk);
> + goto fail;
> +
> + }
> +
> + err = gpio_request(phy->reset_gpio, "ulpi_phy_reset_b");
I think you can use devm_gpio_request() here to simplify the error-handling.
> + if (err < 0) {
> + dev_err(phy->dev, "request failed for gpio: %d\n",
> +phy->reset_gpio);
> + goto fail;
> + }
> +
> + err = gpio_direction_output(phy->reset_gpio, 0);
> + if (err < 0) {
> + dev_err(phy->dev, "gpio %d direction not set to
> output\n",
> +phy->reset_gpio);
> + goto cleanup_gpio_req;
> + }
>
> - return phy;
> + phy->ulpi = otg_ulpi_create(&ulpi_viewport_access_ops, 0);
> + if (!phy->ulpi) {
> + dev_err(phy->dev, "otg_ulpi_create returned err\n");
> + err = -ENOMEM;
> + goto cleanup_gpio_req;
> + }
>
> -err1:
> + phy->ulpi->io_priv = phy->regs + ULPI_VIEWPORT;
> + } else {
> + err = utmip_pad_open(phy);
> + if (err < 0)
> + goto fail;
> + }
I wonder why in the ULPI case, all the code is inline here, whereas in
the UTMI case, this simply calls a function. Wouldn't it be more
consistent to have the following code here:
if (phy->is_ulpi_phy)
err = ulpi_open();
else
err = utmip_open();
if (err)
goto fail;
> +static int tegra_usb_phy_probe(struct platform_device *pdev)
Hmmm. Note that in order to make deferred probe work correctly, all the
gpio_request(), clk_get(), etc. calls that acquire resources from other
drivers must happen here in probe() and not in tegra_usb_phy_open().
> + err = of_property_match_string(np, "dr_mode", "otg");
> + if (err < 0) {
> + err = of_property_match_string(np, "dr_mode", "gadget");
Again, use "peripheral", not "gadget".
> +struct usb_phy *tegra_usb_get_phy(struct device_node *dn)
> +{
> + struct device *dev;
> + struct tegra_usb_phy *tegra_phy;
> +
> + dev = driver_find_device(&tegra_usb_phy_driver.driver, NULL, dn,
> + tegra_usb_phy_match);
> + if (!dev)
> + return ERR_PTR(-EPROBE_DEFER);
> +
> + tegra_phy = dev_get_drvdata(dev);
> +
> + return &tegra_phy->u_phy;
> +}
I think you need a module_get() somewhere in there, and also need to add
a tegra_usb_put_phy() function too, so you can call module_put() from it.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: add table lookup to mux
On 03/19/2013 03:33 PM, Mike Turquette wrote: > Quoting Peter De Schrijver (2013-03-12 11:42:23) >> Add a table lookup feature to the mux clock. Also allow arbitrary masks >> instead of the width. This will be used by some clocks on Tegra114. Also >> adapt the tegra periph clk because it uses struct clk_mux directly. >> >> Signed-off-by: Peter De Schrijver > > Do you actually need arbitrary masks instead of a continuous bitfield? > Or does this change just make it easier for you to convert existing > data? Yes, Peter mentioned somewhere that Tegra apparently has some registers where the mux bits are split up into non-contiguous regions of a register. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: add table lookup to mux
On 03/19/2013 03:37 PM, Mike Turquette wrote: > Quoting Stephen Warren (2013-03-12 11:53:46) >> On 03/12/2013 12:42 PM, Peter De Schrijver wrote: >>> Add a table lookup feature to the mux clock. Also allow arbitrary masks >>> instead of the width. This will be used by some clocks on Tegra114. Also >>> adapt the tegra periph clk because it uses struct clk_mux directly. >>> >>> Signed-off-by: Peter De Schrijver >>> >>> -- >>> Mike, >>> >>> This is the same patch I posted before which implements a table lookup >>> feature for the mux clock. I squashed both the changes to clk-mux.c and >>> tegra/clk.h together in order to make the patch bisectable. >> >> For the record in this thread, this is a dependency of the Tegra114 >> clock driver. Can this patch be added to a stable branch in the clk or >> arm-soc trees, which I can merge before applying the Tegra114 clock >> driver? Thanks. > > Yes. I don't have an immutable branch today, but I will have one by > -rc4. It will be named clk-for-3.10. Perhaps this series could be merged into an immutable topic branch of its own. That could then be merged into your clk-for-3.10, and Tegra's branch, and reduce the number of commits that get merged into the Tegra tree from outside? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Regression] process hangs due to "USB: EHCI: work around silicon bug in Intels EHCI controllers"
On 03/19/2013 06:03 PM, Andreas Bombe wrote: > The named commit (6402c796d3) causes a process to hang indefinitely in > usb_kill_urb(). Reverting it fixes the problem. The bug also prevents > suspend/shutdown/reboot from completing, presumably due to the hanging > process. > > (Cc'ing Stephen Warren because I only found his report after I bisected > it myself. Do you also see a blocked process?) Yes, I believe the symptoms we're both seeing are identical. > This is the hanging process on my system: > [ 240.364085] INFO: task colord-sane:3619 blocked for more than 120 seconds. ... > [ 240.364118] Call Trace: > [ 240.364127] [] schedule+0x65/0x67 > [ 240.364132] [] usb_kill_urb+0xb5/0xd3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: tegra114: add speedo-based process identification
On 03/19/2013 07:42 PM, Danny Huang wrote: > On Wed, 2013-03-20 at 01:54 +0800, Stephen Warren wrote: >> On 03/18/2013 05:17 AM, Danny Huang wrote: >>> Add speedo-based process identifictaion for Tegra114. >> >> I have applied this to Tegra's for-3.10/soc branch, with one addition below: >> >>> diff --git a/arch/arm/mach-tegra/tegra114_speedo.c >>> b/arch/arm/mach-tegra/tegra114_speedo.c >> >>> +#include >> >> Here, I also added: >> >> #include >> >> That's not needed in the Tegra for-next branch (which is based on >> 3.9-rc1), but is needed when merged into linux-next, and hence will >> presumably be needed once this is submitted into 3.10. > > Thanks. Does it mean that we have to include the bug.h for all newly > created file for 3.10? It's only needed in files that use its contents. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra: fuse: add fuctions to read speedo id and process id
On 03/19/2013 07:39 PM, Danny Huang wrote: > On Wed, 2013-03-20 at 02:05 +0800, Stephen Warren wrote: >> On 03/18/2013 08:33 PM, Danny Huang wrote: >>> Add functions to read the speedo and process id of both cpu and soc. >>> There might be some drivers need the information as well. >> >> What code wants to use these functions? It'd be best to submit this >> patch with that code, so that something actually uses the functions, and >> they don't look like dead code. > > There are some developing drivers that are located in drivers folder > needs those information. Which drivers? > I think that they can't include a header file in mach-tegra folder True. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] regulator: palmas: add input supply names
On 03/20/2013 06:54 AM, Laxman Dewangan wrote: > On Wednesday 20 March 2013 06:01 PM, Mark Brown wrote: >> * PGP Signed by an unknown key >> >> On Mon, Mar 18, 2013 at 02:59:48PM +0530, Laxman Dewangan wrote: >> >> There's something odd about several recent patch serieses you've posted, >> they end up with patch 2 before instead of after patch 1. Anyway... > > I generally send a patches on single git-send command > git send-emal --to=ldewan...@nvidia.com .patch 0001.patch Don't do that; send *.patch. If you don't, then patches 2..n don't end up being "in-reply-to" patch 1, so they won't show up as a single email thread. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BUG: pinmux: forbid mux_usecount to be set at UINT_MAX
On 03/20/2013 05:31 AM, Richard Genoud wrote:
> If pin_free is called on a pin already freed, mux_usecount is set to
> UINT_MAX which is really a bad idea.
> This will silently ignore a double call to pin_free
Shouldn't we WARN_ON(this case)?
> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> if (!gpio_range) {
> - desc->mux_usecount--;
> - if (desc->mux_usecount)
> + if (1 == desc->mux_usecount)
> + desc->mux_usecount = 0;
> + else
> return NULL;
What if desc-mux_usecount was 2; this patch prevents the use-count from
being decremented to 1 in this case. Shouldn't this be:
if (!gpio_range) {
+ if (WARN_ON(!desc->mux_usecount))
+ return NULL;
desc->mux_usecount--;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BUG: [RFC] pinctrl: pins are freed 2 times in pinctrl_bind_pins
On 03/20/2013 05:31 AM, Richard Genoud wrote: > If the function pinctrl_select_state() fails because one pin is already > taken elsewhere, pinmux_enable_setting makes all the necessary pin_free > calls (and not more than necessary). > The problem here is that devm_pinctrl_put() will be called on the pin > group, and each pin in this group has already been freed. > > Example: > If a i2c function has already sucessfully taken pins 5 and 6. > And now, pinctrl_bind_pins() is called for function PHY (pins 3 4 5 6 7). > pinmux_enable_setting() will fail AND call pin_free on necessary pins. > But if devm_pinctrl_put() is called, it will call again pin_free on pins > 3 4 5 6 7. > So, the pins 5 and 6 will be released (and pins 3 4 7 double freed). > Which means that even if the i2c function has claim the pins, they will > be available for other functions. > > This patch simply doesn't call devm_pinctrl_put when > pinctrl_select_state fails, but I'm not sure it's the right thing to do. The correct fix here is not to skip the call to devm_pinctrl_put(), since that undoes a lot of other things besides the current state selection. Instead, pinctrl_select_state_locked() needs to be fixed so that: a) Change "p->state = state;" to "p->state = NULL;" or similar, to indicate that no state is selected. (Please validate if a NULL value in that variable will cause problems elsewhere) b) Add back the assignment "p->state = state;" at the end of the function, if no error occurred. c) Fix the list_for_each_entry() call that applies all the settings for the new state so that if it fails, it undoes everything that it's applied so far. That's the hard part, unless there's a list_for_each_entry_before_the_current_one_that_list_for_each_entry_iterated_over_already() macro! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] regulator: palmas: add input supply names
On 03/20/2013 10:38 AM, Mark Brown wrote: > On Wed, Mar 20, 2013 at 10:07:17AM -0600, Stephen Warren wrote: >> On 03/20/2013 06:54 AM, Laxman Dewangan wrote: > >>> I generally send a patches on single git-send command git >>> send-emal --to=ldewan...@nvidia.com .patch 0001.patch > >> Don't do that; send *.patch. If you don't, then patches 2..n >> don't end up being "in-reply-to" patch 1, so they won't show up >> as a single email thread. > > The two should be equivalent (though the glob is much easier to > type)? The result of the globbing ought to be what Laxman is typing > by hand assuming that he's getting the order correct. If you send *.patch at once, git send-email adds an in-reply-to header to the email which sets up the threading. If you send the patches 1-by-1, this header isn't added, since git send-email doesn't have a clue what message ID it chose for patch 1. Unless you pass the --in-reply-to command-line option, that is. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] regulator: palmas: add input supply names
On 03/20/2013 10:52 AM, Mark Brown wrote: > On Wed, Mar 20, 2013 at 10:42:26AM -0600, Stephen Warren wrote: >> On 03/20/2013 10:38 AM, Mark Brown wrote: >>> On Wed, Mar 20, 2013 at 10:07:17AM -0600, Stephen Warren >>> wrote: >>>> On 03/20/2013 06:54 AM, Laxman Dewangan wrote: > >>>>> I generally send a patches on single git-send command git >>>>> send-emal --to=ldewan...@nvidia.com .patch 0001.patch > >>>> Don't do that; send *.patch. If you don't, then patches 2..n >>>> don't end up being "in-reply-to" patch 1, so they won't show >>>> up as a single email thread. > >>> The two should be equivalent (though the glob is much easier >>> to type)? The result of the globbing ought to be what Laxman is >>> typing by hand assuming that he's getting the order correct. > >> If you send *.patch at once, git send-email adds an in-reply-to >> header to the email which sets up the threading. If you send the >> patches 1-by-1, this header isn't added, since git send-email >> doesn't have a clue what message ID it chose for patch 1. Unless >> you pass the --in-reply-to command-line option, that is. > > Indeed - I think you may have quoted the wrong bit of Laxman's > mail there? The bit you're replying to is doing it as a single > command, though he did talk about doing this in multiple commands > elsewhere in his mail. Oh right yes. I'd seen .patch and not that there were multiple entries in the command-line that listed (I assume) all the files. Sorry for the noise. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BUG: pinmux: forbid mux_usecount to be set at UINT_MAX
On 03/20/2013 10:59 AM, Richard Genoud wrote:
> 2013/3/20 Stephen Warren :
>> On 03/20/2013 05:31 AM, Richard Genoud wrote:
>>> If pin_free is called on a pin already freed, mux_usecount is set to
>>> UINT_MAX which is really a bad idea.
>>> This will silently ignore a double call to pin_free
>>
>> Shouldn't we WARN_ON(this case)?
> yes indeed, it may be better to issue a big warning because AFAIK
> that's not normal.
>
>>
>>> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
>>
>>> if (!gpio_range) {
>>> - desc->mux_usecount--;
>>> - if (desc->mux_usecount)
>>> + if (1 == desc->mux_usecount)
>>> + desc->mux_usecount = 0;
>>> + else
>>> return NULL;
>>
>> What if desc-mux_usecount was 2; this patch prevents the use-count from
>> being decremented to 1 in this case. Shouldn't this be:
>>
>> if (!gpio_range) {
>> + if (WARN_ON(!desc->mux_usecount))
>> + return NULL;
>> desc->mux_usecount--;
>
> Well, I'm not very familiar with this code, but can mux_usecount be
> higher than 1 ?
Possibly not, but isn't that more something that the
resource-acquisition code (i.e. whatever does mux_usecount++) should
enforce; the cleanup code should probably support all cases in case the
enforcement rules change in the future. Either that, or convert the
field to a bool so it's clear what the range is.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/7] ARM: tegra: finalize USB EHCI and PHY bindings
On 03/20/2013 06:15 AM, Venu Byravarasu wrote: >> -Original Message- >> From: kishon [mailto:kis...@ti.com] >> Sent: Wednesday, March 20, 2013 4:49 PM >> To: Venu Byravarasu >> Cc: gre...@linuxfoundation.org; st...@rowland.harvard.edu; >> ba...@ti.com; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; >> swar...@wwwdotorg.org; linux-te...@vger.kernel.org; devicetree- >> disc...@lists.ozlabs.org >> Subject: Re: [PATCH 1/7] ARM: tegra: finalize USB EHCI and PHY bindings >> >> Hi, >> >> On Monday 18 March 2013 05:59 PM, Venu Byravarasu wrote: >>> The existing Tegra USB bindings have a few issues: >>> >>> 1) Many properties are documented as being part of the EHCI controller >>> node, yet they apply more to the PHY device. They should be moved. >>> >>> 2) Some registers in PHY1 are shared with PHY3, and hence PHY3 needs a >>> reg entry to point at PHY1's register space. We can't assume the PHY1 >>> driver is present, so the PHY3 driver will directly access those >>> registers. >>> >>> 3) The list of clocks required by the PHY was missing some required >>> entries. >>> >>> This patch fixes the binding definition to resolve these issues. >>> >>> Signed-off-by: Venu Byravarasu >>> --- >>> .../bindings/usb/nvidia,tegra20-ehci.txt | 27 >>> +++ >>> .../bindings/usb/nvidia,tegra20-usb-phy.txt| 27 >> +-- >>> 2 files changed, 29 insertions(+), 25 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra20-ehci.txt >> b/Documentation/devicetree/bindings/usb/nvidia,tegra20-ehci.txt >>> index 34c9528..df09330 100644 >>> --- a/Documentation/devicetree/bindings/usb/nvidia,tegra20-ehci.txt >>> +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra20-ehci.txt >>> @@ -6,27 +6,10 @@ Practice : Universal Serial Bus" with the following >> modifications >>> and additions : >>> >>> Required properties : >>> - - compatible : Should be "nvidia,tegra20-ehci" for USB controllers >>> - used in host mode. >>> - - phy_type : Should be one of "ulpi" or "utmi". >>> >>> Optional properties: >>> - - dr_mode : dual role mode. Indicates the working mode for > >>> index 6bdaba2..7ceccd3 100644 >>> --- a/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt >>> +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt >>> @@ -4,8 +4,24 @@ The device node for Tegra SOC USB PHY: >>> >>> Required properties : >>> + - phy_type : Should be one of "utmi", "ulpi" or "hsic". >> >> dt property names generally dont have "_". > > Thanks Kishon, for your comments. > Is it mandatory or optional? > If it is mandatory, then I might have to change dr_mode as well along with > phy_type. This rule is basically mandatory for *new* property definitions. However, both phy_type and dr_mode are properties that already exist and have historical precedence for their naming. So, I don't think we can change them. In fact, IIRC, someone even posted some patches to provide a common set of parsing functions for those properties, and I assume those patches use the existing names with _ in them. The patches aren't applied yet though, I think. (Venu, we already discussed this rule downstream) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/7] ARM: tegra: update device trees for USB binding rework
On 03/20/2013 05:23 AM, kishon wrote:
> Hi,
>
> On Monday 18 March 2013 05:59 PM, Venu Byravarasu wrote:
>> This patch updates all Tegra board files so that they contain all the
>> properties required by the updated USB DT binding. Note that this patch
>> only adds the new properties and does not yet remove the old properties,
>> in order to maintain bisectability. The old properties will be removed
>> once the driver has been updated to assume the new bindings.
>>
>> Signed-off-by: Venu Byravarasu
>> ---
>> arch/arm/boot/dts/tegra20-colibri-512.dtsi |4 +++
>> arch/arm/boot/dts/tegra20-harmony.dts |8 +++---
>> arch/arm/boot/dts/tegra20-iris-512.dts |4 +++
>> arch/arm/boot/dts/tegra20-paz00.dts|8 +++---
>> arch/arm/boot/dts/tegra20-seaboard.dts | 13 +++---
>> arch/arm/boot/dts/tegra20-trimslice.dts| 12 +++---
>> arch/arm/boot/dts/tegra20-ventana.dts |7 +++--
>> arch/arm/boot/dts/tegra20.dtsi | 32
>> +--
>> 8 files changed, 57 insertions(+), 31 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/tegra20-colibri-512.dtsi
>> b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
>> index cb73e62..af5a7ae 100644
>> --- a/arch/arm/boot/dts/tegra20-colibri-512.dtsi
>> +++ b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
>> @@ -443,6 +443,10 @@
>> nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */
>> };
>>
>> +usb-phy@c5004000 {
>
> This node doesn't have a *reg* property. So "@c5004000" is not needed.
> This comment applies to all the nodes which doesn't have *reg* property.
Yes it does have a reg property.
The node itself is first defined in tegra20.dtsi, and does contain a reg
property there.
tegra20-colibri-512.dtsi (and many other files in this patch) include
tegra20.dtsi, and simply add additional board-specific properties to the
existing node, and should not re-iterate properties that already exist.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/7] USB: PHY: Tegra: registering TEGRA USB PHY as platform driver
On 03/20/2013 06:12 AM, Venu Byravarasu wrote: >> -Original Message- >> From: Venu Byravarasu >> Sent: Wednesday, March 20, 2013 11:30 AM >> To: 'Stephen Warren' >> Cc: gre...@linuxfoundation.org; st...@rowland.harvard.edu; >> ba...@ti.com; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; >> linux-te...@vger.kernel.org; devicetree-disc...@lists.ozlabs.org >> Subject: RE: [PATCH 0/7] USB: PHY: Tegra: registering TEGRA USB PHY as >> platform driver >> >>> -Original Message- >>> From: Stephen Warren [mailto:swar...@wwwdotorg.org] >>> Sent: Wednesday, March 20, 2013 1:22 AM >>> To: Venu Byravarasu >>> Cc: gre...@linuxfoundation.org; st...@rowland.harvard.edu; >>> ba...@ti.com; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; >>> linux-te...@vger.kernel.org; devicetree-disc...@lists.ozlabs.org >>> Subject: Re: [PATCH 0/7] USB: PHY: Tegra: registering TEGRA USB PHY as >>> platform driver >>> >>> On 03/18/2013 06:29 AM, Venu Byravarasu wrote: >>>> As part of this series, apart from patch containing changes to register >>> TEGRA >>>> USB PHY driver as platform driver, prepared below patches: >>>> 1. Re-arranging & adding new DT properties. >>>> 2. Getting various params from DT properties added. >>>> 3. code clean up. >>> >>> Venu, I'm curious whether these patches were tested at all. I have found >>> at least two significant problems with trivial testing: >> >> Stephen, >> Initially started testing after applying each and every patch. >> Like that tested till first 5 patches. >> As did not see any issues till then, applied rest 2 patches at once and >> tested >> with that. >> Though did not see mouse getting vbus on the 1st boot, Vbus was coming >> fine after disconnect and connect. >> Hence did not test thereafter. >> >> After checking your current mail, tried now and observed that there seems to >> be some real issue with patch#7 only. (As tried now after applying till >> patch# >> 6 and did not see this issue). >> Will debug further on patch#7 and update with proper fix after addressing >> your other comments. > > Debugged further and found that the issue is because of > http://marc.info/?l=linux-arm-kernel&m=135890098024987&w=2 > On reverting that patch and applying it on top of patch#7, able to see > enumeration working fine. > > Anyhow, will take care of your other comments and merge this change with > patch#7 and resend > for review. Venu, we already discussed this downstream, and I pointed out that patch is /extremely/ unlikely to cause any issue. The clk_set_rate() call returns an error since the requested clock rate is not supported. The clk_prepare_enable() shouldn't have any effect because if it did, the only possible meaning is that the clock is otherwise turned off because it has no users, and that would prevent far more than USB from operating correctly. Please fully debug this problem and root-cause it. I'm not reverting that EMC clock patch without a complete explanation. There's little point reposting the patches until you've found the problem. If we did have to revert that patch, then we would need to redefine the DT bindings for Tegra USB to add that clock into the list of required clocks, since otherwise it could not clk_get(dev, "emc"). But the USB driver shouldn't be touching non-USB clocks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/7] usb: phy: registering tegra USB PHY as platform driver
On 03/20/2013 06:43 AM, Venu Byravarasu wrote:
> Stephen Warren wrote at Wednesday, March 20, 2013 1:51 AM:
>> On 03/18/2013 06:29 AM, Venu Byravarasu wrote:
>>> Registered tegra USB PHY as a separate platform driver.
>>> diff --git a/drivers/usb/phy/tegra_usb_phy.c
>>> b/drivers/usb/phy/tegra_usb_phy.c
>>> +static int tegra_usb_phy_probe(struct platform_device *pdev)
>>
>> Hmmm. Note that in order to make deferred probe work correctly, all the
>> gpio_request(), clk_get(), etc. calls that acquire resources from other
>> drivers must happen here in probe() and not in tegra_usb_phy_open().
>
> In present code tegra_usb_phy_open() is indirectly called via usb_phy_init()
> from ehci-tegra.c.
> Between obtaining PHY handle (and hence getting into deferred probe, when it
> is not available)
> and calling usb_phy_init, no other USB registers were accessed. Hence I was
> doing this way.
>
> Do you still think it would be better to move gpio and clk APIs to probe?
Yes.
What's happening right now is that the PHY driver potentially completes
probe() before its resources are available.
This just happens not to break anything because the EHCI driver's probe
ends up getting deferred until some other PHY driver function can
acquire those resources, so the USB port as a whole isn't registered
until the resources are available.
However, this still means that the PHY driver could be suspended after
e.g. a driver that provides a GPIO it uses, since the PHY's probe
completed before the GPIO driver's probe.
Hence, this could easily cause some form of suspend/resume or perhaps
even shutdown/reboot problem.
>>> +struct usb_phy *tegra_usb_get_phy(struct device_node *dn)
>>> +{
>>> + struct device *dev;
>>> + struct tegra_usb_phy *tegra_phy;
>>> +
>>> + dev = driver_find_device(&tegra_usb_phy_driver.driver, NULL, dn,
>>> +tegra_usb_phy_match);
>>> + if (!dev)
>>> + return ERR_PTR(-EPROBE_DEFER);
>>> +
>>> + tegra_phy = dev_get_drvdata(dev);
>>> +
>>> + return &tegra_phy->u_phy;
>>> +}
>>
>> I think you need a module_get() somewhere in there, and also need to add
>> a tegra_usb_put_phy() function too, so you can call module_put() from it.
>
> Am not clear on why to add module_get here. Can you plz provide more details?
Actually, I guess it isn't needed, although slightly by accident perhaps:
If ehci-tegra.c and phy-usb-tegra.c are built as modules, they'll be
separate modules. Since ehci-tegra.c calls into phy-usb-tegra.c, you
need to make sure that phy-usb-tegra.c stays loaded any time
ehci-tegra.c is loaded.
Right now, this is ensured because ehci-tegra.c references functions in
phy-usb-tegra.c by symbol name, so a dependency exists. So, I guess
everything actually works out without the module_get(). So, no changes
are needed.
After this series is applied, I believe that tegra_usb_get_phy() is the
last function that ehci-tegra.c references by symbol name. Eventually,
that function will be replaced by devm_of_phy_get_byname() (see Kishon's
generic PHY API patch series), so there won't be any symbol name
dependencies, so some other mechanism must be used to ensure the PHY
driver stays loaded while the EHCI driver is using it. At that point,
the implementation of devm_of_phy_get_byname() should be calling
module_get(), so again no changes should be required to your patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/7] USB: PHY: Tegra: registering TEGRA USB PHY as platform driver
On 03/20/2013 11:36 AM, Stephen Warren wrote:
> On 03/20/2013 06:12 AM, Venu Byravarasu wrote:
>> Venu Byravarasu wrote at Wednesday, March 20, 2013 11:30 AM:
>>> Stephen Warren wrote at Wednesday, March 20, 2013 1:22 AM:
>>>> On 03/18/2013 06:29 AM, Venu Byravarasu wrote:
>>>>> As part of this series, apart from patch containing changes to register
>>>> TEGRA
>>>>> USB PHY driver as platform driver, prepared below patches:
>>>>> 1. Re-arranging & adding new DT properties.
>>>>> 2. Getting various params from DT properties added.
>>>>> 3. code clean up.
>>>>
>>>> Venu, I'm curious whether these patches were tested at all. I have found
>>>> at least two significant problems with trivial testing:
>>>
>>> Stephen,
>>> Initially started testing after applying each and every patch.
>>> Like that tested till first 5 patches.
>>> As did not see any issues till then, applied rest 2 patches at once and
>>> tested
>>> with that.
>>> Though did not see mouse getting vbus on the 1st boot, Vbus was coming
>>> fine after disconnect and connect.
>>> Hence did not test thereafter.
>>>
>>> After checking your current mail, tried now and observed that there seems to
>>> be some real issue with patch#7 only. (As tried now after applying till
>>> patch#
>>> 6 and did not see this issue).
>>> Will debug further on patch#7 and update with proper fix after addressing
>>> your other comments.
>>
>> Debugged further and found that the issue is because of
>> http://marc.info/?l=linux-arm-kernel&m=135890098024987&w=2
>> On reverting that patch and applying it on top of patch#7, able to see
>> enumeration working fine.
>>
>> Anyhow, will take care of your other comments and merge this change with
>> patch#7 and resend
>> for review.
>
> Venu, we already discussed this downstream, and I pointed out that patch
> is /extremely/ unlikely to cause any issue.
OK, so I found that reverting that patch does "solve" the issue, but
it's got nothing to do with that patch being wrong. There is some memory
corruption issue in your patches that is hidden if that patch is reverted.
In other words, on both Springbank (which you don't have, and has an EMC
scaling table so that the EMC frequency probably varies with CPU load)
and Ventana (which is what I assume you're testing on, and has no EMC
table, so the EMC clock frequency is fixed):
1) Tegra's for-next branch: Works fine.
2) (1) plus your patches: Broken as I described above.
3) (2) plus revert 9304512 "usb: host: tegra: don't touch EMC clock":
works fine on Ventana, somewhat works on Seaboard; lots of "can't
enumerate USB bus" messages, but eventually succeeds.
The really interesting thing is that if I take (2) above plus *just* the
following patch:
> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> index 772fa29..7499240 100644
> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c
> @@ -44,6 +44,7 @@ struct tegra_ehci_hcd {
> struct ehci_hcd *ehci;
> struct tegra_usb_phy *phy;
> struct clk *clk;
> + struct clk *emc_clk;
> struct usb_phy *transceiver;
> int host_resumed;
> int port_resuming;
Then it works fine on both Ventana and Seaboard.
To me, this says that there's nothing at all wrong with 9304512 "usb:
host: tegra: don't touch EMC clock", but rather there is some memory
corruption going on in your patches series, and adding that extra field
in the struct just hides the effect of the memory corruption.
Please debug and fix that. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] ARM: tegra: dalmore: add dt node for TPS65090's regulators
On 03/20/2013 07:44 AM, Laxman Dewangan wrote:
> NVIDIA's Tegra114 platform, Dalmore, uses the TPS65090 as
> secondary PMICs which mainly act as voltage switch controlled
> by I2C communication.
> diff --git a/arch/arm/boot/dts/tegra114-dalmore.dts
> b/arch/arm/boot/dts/tegra114-dalmore.dts
> @@ -747,4 +823,18 @@
> bus-width = <8>;
> status = "okay";
> };
> + regulators {
> + compatible = "simple-bus";
> + #address-cells = <1>;
You need a blank line between the nodes there.
There a a number of typos and capitalization issues in the various
commit descriptions.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: enable Tegra114 based platform PMICs
On 03/20/2013 07:44 AM, Laxman Dewangan wrote: > NVIDIA's Tegra114 have two reference platform, Dalmore and Pluto. > Dalmore uses the following PMICs: > - TPS65913 as primary PMIC. > - TPS65090 as secondary PMIC used for switch regulators and battery charging. > - TPS51632 for cpu regulator. > > Pluto uses the TPS65913 as the PMIC. > > Enable config variable of these PMICs for Dalomore and Pluto. Just FYI, I would apply the DT changes to Tegra's for-3.10/dt branch, and the defconfig change to Tegra's for-3.10/defconfig branch. The /dt branch will get merged first, and the /defconfig after it. Hence, you would typically want to send patches in that order. It doesn't really make any practical difference in this case though. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 5/6] ARM: dts: omap: update usb_otg_hs data
On 03/20/2013 03:12 AM, Kishon Vijay Abraham I wrote: > Updated the usb_otg_hs dt data to include the *phy* and *phy-names* > binding in order for the driver to use the new generic PHY framework. > Also updated the Documentation to include the binding information. > diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt > b/Documentation/devicetree/bindings/usb/omap-usb.txt > index abce256..3d6f9f6 100644 > --- a/Documentation/devicetree/bindings/usb/omap-usb.txt > +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt > @@ -19,6 +19,9 @@ OMAP MUSB GLUE > - power : Should be "50". This signifies the controller can supply upto > 100mA when operating in host mode. > - usb-phy : the phandle for the PHY device > + - phy : the phandle for the PHY device (used by generic PHY framework) > + - phy-names : the names of the PHY corresponding to the PHYs present in the > + *phy* phandle. If the intent is for those properties to be generic and used by any DT binding that refers to a PHY node, I think you'd want to define those properties in e.g. Documentation/devicetree/bindings/phy/phy.txt, just like common clock/GPIO/... properties are defined in standalone common files. I think you want to require that DT nodes that represent PHYs have a #phy-cells property, and that the format of the phy property be <&phy_phandle phy_specifier*>, where #phy-cells in the referenced node defines how many cells are part of phy_specifier*, just like (almost) any other DT property that references another node by phandle. That way, if a single DT node represents a HW block that implements e.g. 3 PHYs, it can use #phy-cells = <1>, and the referencing phy property can include a cell that indicates which of those 3 PHYs is being referenced. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] regmap: core: Split out in place value parsing
On 03/19/2013 10:50 AM, Mark Brown wrote:
> On Mon, Mar 18, 2013 at 05:41:49PM -0600, Stephen Warren wrote:
>
>> It took a very quick look at the patch and couldn't see anything
>> actively wrong. I wonder if one of the existing unconverted users
>> of .parse_val() relies on the in-place modification of the buffer
>> as well as getting the result back, and so should have been
>> converted to calling both .parse_inplace() and then
>> .parse_val()?
>
> Possibly, though you'd have thought that if it were just that one
> of the other users would have noticed - my primary development
> board uses regmap extensively for example. Does seem the most
> likely option though. Can't test anything again until Friday
> sadly.
>
> Might also be some unusual code path WM8903 exercises, though again
> it's pretty simple.
I haven't thought through why the patch in question causes/exposes the
issue yet, but I have found out what the problem is.
_regmap_bus_raw_write() formats the value into work_buf + reg_bytes +
pad_bytes, i.e. work_buf + 1.
For the first regmap_write() that the WM8903 driver does, work_buf is
now xx 89 03.
_regmap_raw_write() then memcpy()s from val (i.e. work_buf + 1) to
workbuf, and parses the value to send to the caching code:
> if (!map->cache_bypass && map->format.parse_val) { unsigned int
> ival; int val_bytes = map->format.val_bytes; for (i = 0; i <
> val_len / val_bytes; i++) { memcpy(map->work_buf, val + (i *
> val_bytes), val_bytes); ival =
> map->format.parse_val(map->work_buf);
This corrupts that value at work_buf + 1 since it overlaps the copy
destination at work_buf. work_buf is now 89 03 03.
_regmap_raw_write() then formats the register address into work_buf,
yielding 00 03 03.
That data stream is then sent over I2C, causing a write to register 0
(correct) of value 03 03 (rather than 89 03).
If I comment out the memcpy, and instead pass the value address
directly to .parse_val(), everything works again.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] regmap: core: Split out in place value parsing
On 03/20/2013 04:19 PM, Stephen Warren wrote:
> On 03/19/2013 10:50 AM, Mark Brown wrote:
>> On Mon, Mar 18, 2013 at 05:41:49PM -0600, Stephen Warren wrote:
>>
>>> It took a very quick look at the patch and couldn't see anything
>>> actively wrong. I wonder if one of the existing unconverted users
>>> of .parse_val() relies on the in-place modification of the buffer
>>> as well as getting the result back, and so should have been
>>> converted to calling both .parse_inplace() and then
>>> .parse_val()?
>>
>> Possibly, though you'd have thought that if it were just that one
>> of the other users would have noticed - my primary development
>> board uses regmap extensively for example. Does seem the most
>> likely option though. Can't test anything again until Friday
>> sadly.
>>
>> Might also be some unusual code path WM8903 exercises, though again
>> it's pretty simple.
>
> I haven't thought through why the patch in question causes/exposes the
> issue yet, but I have found out what the problem is.
>
> _regmap_bus_raw_write() formats the value into work_buf + reg_bytes +
> pad_bytes, i.e. work_buf + 1.
>
> For the first regmap_write() that the WM8903 driver does, work_buf is
> now xx 89 03.
>
> _regmap_raw_write() then memcpy()s from val (i.e. work_buf + 1) to
> workbuf, and parses the value to send to the caching code:
>
>> if (!map->cache_bypass && map->format.parse_val) { unsigned int
>> ival; int val_bytes = map->format.val_bytes; for (i = 0; i <
>> val_len / val_bytes; i++) { memcpy(map->work_buf, val + (i *
>> val_bytes), val_bytes); ival =
>> map->format.parse_val(map->work_buf);
>
> This corrupts that value at work_buf + 1 since it overlaps the copy
> destination at work_buf. work_buf is now 89 03 03.
Ah, here's why it used to work:
When parse_val used to both return the value and modify the buffer in
place, parse_val used to re-write the buffer from 89 03 03 to 03 89 03,
which is the same content that it originally had before the memcpy, at
least for the region that holds the value rather than the address, which
is all that's relevant since the address gets over-written later. The
combination of the memcpy-down-by-1-byte followed by swap-first-2-bytes,
just accidentally happened to leave the buffer in a valid state. Of
course, this only works for certain combinations of register address and
value sizes, I think...
So, I think that the fix I mentioned below really is valid, and what's
more is technically needed irrespective of whether "regmap: core: Split
out in place value parsing" is applied. I'll assume so and send out that
patch.
> _regmap_raw_write() then formats the register address into work_buf,
> yielding 00 03 03.
>
> That data stream is then sent over I2C, causing a write to register 0
> (correct) of value 03 03 (rather than 89 03).
>
> If I comment out the memcpy, and instead pass the value address
> directly to .parse_val(), everything works again.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] regmap: don't corrupt work buffer in _regmap_raw_write()
From: Stephen Warren
_regmap_raw_write() contains code to call regcache_write() to write
values to the cache. That code calls memcpy() to copy the value data to
the start of the work_buf. However, at least when _regmap_raw_write() is
called from _regmap_bus_raw_write(), the value data is in the work_buf,
and this memcpy() operation may over-write part of that value data,
depending on the value of reg_bytes + pad_bytes. At least when using
reg_bytes==1 and pad_bytes==0, corruption of the value data does occur.
To solve this, remove the memcpy() operation, and modify the subsequent
.parse_val() call to parse the original value buffer directly.
At least in the case of 8-bit register address and 16-bit values, and
writes of single registers at a time, this memcpy-then-parse combination
used to cancel each-other out; for a work-buffer containing xx 89 03,
the memcpy changed it to 89 03 03, and the parse_val changed it back to
89 89 03, thus leaving the value uncorrupted. This appears completely
accidental though. Since commit 8a819ff "regmap: core: Split out in
place value parsing", .parse_val only returns the parsed value, and does
not modify the buffer, and hence does not (accidentally) undo the
corruption caused by memcpy(). This caused bogus values to get written
to HW, thus preventing e.g. audio playback on systems with a WM8903
CODEC. This patch fixes that.
Cc: Laxman Dewangan
Signed-off-by: Stephen Warren
---
drivers/base/regmap/regmap.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index fed91ac..1ab4498 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -963,8 +963,7 @@ static int _regmap_raw_write(struct regmap *map, unsigned
int reg,
unsigned int ival;
int val_bytes = map->format.val_bytes;
for (i = 0; i < val_len / val_bytes; i++) {
- memcpy(map->work_buf, val + (i * val_bytes), val_bytes);
- ival = map->format.parse_val(map->work_buf);
+ ival = map->format.parse_val(val + (i * val_bytes));
ret = regcache_write(map, reg + (i * map->reg_stride),
ival);
if (ret) {
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] regmap: include linux/sched.h to fix build
From: Stephen Warren This fixes: drivers/base/regmap/regmap.c: In function 'regmap_async_complete_cb': drivers/base/regmap/regmap.c:1656:3: error: 'TASK_NORMAL' undeclared (first use in this function) drivers/base/regmap/regmap.c:1656:3: note: each undeclared identifier is reported only once for each function it appears in drivers/base/regmap/regmap.c: In function 'regmap_async_complete': drivers/base/regmap/regmap.c:1688:2: error: 'TASK_UNINTERRUPTIBLE' undeclared (first use in this function) drivers/base/regmap/regmap.c:1688:2: error: implicit declaration of function 'schedule' An alternative might be to adjust linux/wait.h to include linux/sched.h, but since that hasn't been done before, I assume we're consciously avoiding doing that. Signed-off-by: Stephen Warren --- drivers/base/regmap/regmap.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index bee8560..ebd1f22 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -16,6 +16,7 @@ #include #include #include +#include #define CREATE_TRACE_POINTS #include -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] cpufreq: TEGRA: Set policy->cpus from driver->init()
On 01/31/2013 11:40 PM, Viresh Kumar wrote: > For multicore SoC's, with cores sharing clock line, we are required to set > policy->cpus and policy->related_cpus with mask of cpus. > > With following patch, we need to set policy->cpus with mask of all possible > cpus > and policy->related_cpus would be filled automatically by the cpufreq core. > > commit 4948b355e90080cd5ec1e91189f65a01e4186ef2 > Author: Viresh Kumar > Date: Tue Jan 29 14:39:08 2013 + > > cpufreq: Simplify cpufreq_add_dev() > > Current Tegra driver fills only ->related_cpus and not ->cpus, which looks to > be > incorrect. Lets fix it. Joseph Lo reviewed/tested this and it looks fine, so, Acked-by: Stephen Warren -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: CPU hotplug hang due to "swap: make each swap partition have one address_space"
On 02/03/2013 07:36 PM, Shaohua Li wrote: > On Fri, Feb 01, 2013 at 10:02:33PM -0700, Stephen Warren wrote: >> Shaohua, >> >> In next-20130128, commit 174f064 "swap: make each swap partition have >> one address_space" (from the mm/akpm tree) appears causes a hang/RCU >> stall for me when hot-unplugging a CPU. > > does this one work for you? > http://marc.info/?l=linux-mm&m=135929599505624&w=2 > Or try a more recent linux-next. The patch is in akpm's tree. Yes, that patch fixes the issue for me, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] ARM: tegra: Fix build error for gic update
On 02/04/2013 04:08 AM, Hiroshi Doyu wrote: > Fix build error in board-dt-tegra114.c(next-20130204) Hiroshi, which tree should this patch be applied to? I assume it needs to go through arm-soc since that's where the Tegra and GIC patches meet? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv5,RESEND 7/8] ARM: tegra: Add board data and 2D clocks
On 02/04/2013 04:26 AM, Thierry Reding wrote: > On Tue, Jan 15, 2013 at 01:44:03PM +0200, Terje Bergstrom wrote: >> Add a driver alias gr2d for Tegra 2D device, and assign a >> duplicate of 2D clock to that driver alias. >> >> Signed-off-by: Terje Bergstrom --- >> arch/arm/mach-tegra/board-dt-tegra20.c|1 + >> arch/arm/mach-tegra/board-dt-tegra30.c|1 + >> arch/arm/mach-tegra/tegra20_clocks_data.c |2 +- >> arch/arm/mach-tegra/tegra30_clocks_data.c |1 + 4 files >> changed, 4 insertions(+), 1 deletion(-) > > With Prashant's clock rework patches now merged this patch can be > dropped. Assuming this series is applied for 3.10 and not earlier, yes. I'd certainly recommend applying for 3.10 not 3.9; the dependencies to apply this for 3.9 given the AUXDATA/... requirements would be too painful. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: next-20130204 - bisected slab problem to "slab: Common constants for kmalloc boundaries"
On 02/04/2013 09:22 AM, James Hogan wrote:
> Hi,
>
> I've hit boot problems in next-20130204 on Meta:
>
> META213-Thread0 DSP [LogF] kobject (4fc03980): tried to init an initialized
> object, something is seriously wrong.
> META213-Thread0 DSP [LogF]
> META213-Thread0 DSP [LogF] Call trace:
> META213-Thread0 DSP [LogF] [<4000888c>] _show_stack+0x68/0x7c
> META213-Thread0 DSP [LogF] [<400088b4>] _dump_stack+0x14/0x28
> META213-Thread0 DSP [LogF] [<40103794>] _kobject_init+0x58/0x9c
> META213-Thread0 DSP [LogF] [<40103810>] _kobject_create+0x38/0x64
> META213-Thread0 DSP [LogF] [<40103eac>] _kobject_create_and_add+0x14/0x8c
> META213-Thread0 DSP [LogF] [<40190ac4>] _mnt_init+0xd8/0x220
> META213-Thread0 DSP [LogF] [<40190508>] _vfs_caches_init+0xb0/0x160
...
> I've bisected it to the following commit:
>
> commit 95a05b428cc675694321c8f762591984f3fd2b1e
> Author: Christoph Lameter
> Date: Thu Jan 10 19:14:19 2013 +
>
> slab: Common constants for kmalloc boundaries
>
> Standardize the constants that describe the smallest and largest
> object kept in the kmalloc arrays for SLAB and SLUB.
>
> Differentiate between the maximum size for which a slab cache is used
> (KMALLOC_MAX_CACHE_SIZE) and the maximum allocatable size
> (KMALLOC_MAX_SIZE, KMALLOC_MAX_ORDER).
>
> Signed-off-by: Christoph Lameter
> Signed-off-by: Pekka Enberg
I see the same problem on ARM. I believe it's because of the changes to
the calculation of MALLOC_SHIFT_LOW.
The old code was:
#if defined(ARCH_DMA_MINALIGN) && ARCH_DMA_MINALIGN > 8
#define KMALLOC_MIN_SIZE ARCH_DMA_MINALIGN
#else
#ifdef CONFIG_SLAB
#define KMALLOC_MIN_SIZE 32
#else
#define KMALLOC_MIN_SIZE 8
#endif
#endif
#define KMALLOC_SHIFT_LOW ilog2(KMALLOC_MIN_SIZE)
Here, KMALLOC_SHIFT_LOW and KMALLOC_MIN_SIZE are always consistent/related.
The new code is:
#define KMALLOC_SHIFT_LOW 5
...
#if defined(ARCH_DMA_MINALIGN) && ARCH_DMA_MINALIGN > 8
#define KMALLOC_MIN_SIZE ARCH_DMA_MINALIGN
#else
#define KMALLOC_MIN_SIZE (1 << KMALLOC_SHIFT_LOW)
#endif
Here, if defined(ARCH_DMA_MINALIGN), then KMALLOC_MIN_SIZE isn't
relative-to/derived-from KMALLOC_SHIFT_LOW, so the two may become
inconsistent.
On my ARM system at least, CONFIG_ARM_L1_CACHE_SHIFT_6 is set since I
have an ARMv7 CPU (see arch/arm/mm/Kconfig), which causes
CONFIG_ARM_L1_CACHE_SHIFT=6, then:
> arch/arm/include/asm/cache.h:7:#define L1_CACHE_SHIFT
> CONFIG_ARM_L1_CACHE_SHIFT
> arch/arm/include/asm/cache.h:8:#define L1_CACHE_BYTES (1 <<
> L1_CACHE_SHIFT)
> arch/arm/include/asm/cache.h:17:#define ARCH_DMA_MINALIGN L1_CACHE_BYTES
... hence that case triggers.
I also see that in most parts of the patch, SLUB_PAGE_SHIFT was replaced
with (KMALLOC_SHIFT_HIGH + 1), or equivalently tests were changed from <
to <=:
> - size <= SLUB_MAX_SIZE && !(flags & SLUB_DMA)) {
> + size <= KMALLOC_MAX_CACHE_SIZE && !(flags & SLUB_DMA)) {
However, the following doesn't seem to have that adjustment:
> diff --git a/mm/slub.c b/mm/slub.c
> index ba2ca53..d0f72ee 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2775,7 +2775,7 @@ init_kmem_cache_node(struct kmem_cache_node *n)
> static inline int alloc_kmem_cache_cpus(struct kmem_cache *s)
> {
> BUILD_BUG_ON(PERCPU_DYNAMIC_EARLY_SIZE <
> - SLUB_PAGE_SHIFT * sizeof(struct kmem_cache_cpu));
> + KMALLOC_SHIFT_HIGH * sizeof(struct kmem_cache_cpu));
Should that also be (KMALLOC_SHIFT_HIGH + 1)?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: CPU hotplug hang due to "swap: make each swap partition have one address_space"
On 02/04/2013 09:57 AM, Stephen Warren wrote: > On 02/03/2013 07:36 PM, Shaohua Li wrote: >> On Fri, Feb 01, 2013 at 10:02:33PM -0700, Stephen Warren wrote: >>> Shaohua, >>> >>> In next-20130128, commit 174f064 "swap: make each swap partition have >>> one address_space" (from the mm/akpm tree) appears causes a hang/RCU >>> stall for me when hot-unplugging a CPU. >> >> does this one work for you? >> http://marc.info/?l=linux-mm&m=135929599505624&w=2 >> Or try a more recent linux-next. The patch is in akpm's tree. > > Yes, that patch fixes the issue for me, thanks. I notice this patch is only in Andrew's mmots tree, not mmotm, and hence isn't included in linux-next. Do you know when it'll be "promoted" and show up in linux-next? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 09/10] clk: tegra: Implement clocks for Tegra114
On 02/04/2013 03:45 AM, Peter De Schrijver wrote: > On Mon, Feb 04, 2013 at 08:08:55AM +0100, Prashant Gaikwad wrote: >>> + /* xusb_hs_src */ >>> + val = readl(clk_base + CLK_SOURCE_XUSB_SS_SRC); >>> + val |= BIT(25); /* always select PLLU_60M */ >>> + writel(val, clk_base + CLK_SOURCE_XUSB_SS_SRC); >>> + >>> + clk = clk_register_fixed_factor(NULL, "xusb_hs_src", "pll_u_60M", 0, >>> + 1, 1); >>> + clks[xusb_hs_src] = clk; >>> + >> >> With device tree we can directly use pll_u_60M, no need to register >> clock with fixed factor 1. > > This is true for now. In the future these clocks will need to be dvfs aware > though. I think it makes sense to have a separate clock then? Why does DVFS-awareness require it to be a different clock/ID? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 07/10] ARM: tegra: Define Tegra114 CAR binding
On 02/01/2013 03:18 AM, Peter De Schrijver wrote: > The device tree binding models Tegra114 CAR (Clock And Reset) as a single > monolithic clock provider. ... > +- #clock-cells : Should be 1. > + In clock consumers, this cell represents the clock ID exposed by the CAR. ... > + 222pll_u > + 223pll_u_480M > + 224pll_u_60M > + 225pll_u_48M > + 226pll_u_12M I notice here that we have separate clock IDs for the various PLL u outputs. We don't have this on Tegra20 or Tegra30, but I wonder if we should have them there too? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 01/10] clk: tegra: Refactor PLL programming code
On 02/01/2013 03:18 AM, Peter De Schrijver wrote: > Refactor the PLL programming code to make it useable by the new PLL types > introduced by Tegra114. I tested this series on Tegra20 and Tegra30 for regressions and found none. Note that I didn't test on Tegra114. The series, Tested-by: Stephen Warren Also, I briefly reviewed the code and aside from comments that have already been made, so the series, Reviewed-by: Stephen Warren -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clocksource: tegra: move to of_clk_get
On 02/04/2013 06:40 AM, Peter De Schrijver wrote: > The new clockframework introduced DT IDs for each clock. To be able to remove > the device registrations, this driver needs to be updated to use the DT IDs. > Note that the actual removal of the clk_register_clkdev() calls will be done > in a later series. This seems fine for 3.10, but I wonder about dependencies. Will there be a bunch of other similar changes? I guess this patch is a dependency of some changes to drivers/clk/tegra/ to remove the non-DT clock registrations? It'd be good to get a clear idea of all the dependencies, so we can see what's the best tree to apply the changes to; perhaps this should go through the drivers/clk tree, or Tegra's clk driver changes need to go through the Tegra tree again in 3.10? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: next-20130204 - bisected slab problem to "slab: Common constants for kmalloc boundaries"
On 02/05/2013 09:36 AM, Christoph Lameter wrote: > OK I was able to reproduce it by setting ARCH_DMA_MINALIGN in slab.h. This > patch fixes it here: > > > Subject: slab: Handle ARCH_DMA_MINALIGN correctly > > A fixed KMALLOC_SHIFT_LOW does not work for arches with higher alignment > requirements. > > Determine KMALLOC_SHIFT_LOW from ARCH_DMA_MINALIGN instead. Tested-by: Stephen Warren > +/* > + * Some archs want to perform DMA into kmalloc caches and need a guaranteed > + * alignment larger than the alignment of a 64-bit integer. > + * Setting ARCH_KMALLOC_MINALIGN in arch headers allows that. > + */ > +#if defined(ARCH_DMA_MINALIGN) && ARCH_DMA_MINALIGN > 8 > +#define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN > +#define KMALLOC_MIN_SIZE ARCH_DMA_MINALIGN I might be tempted to drop that #define of KMALLOC_MIN_SIZE ... > +#define KMALLOC_SHIFT_LOW ilog2(ARCH_DMA_MINALIGN) > +#else > +#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long) > +#endif > +#ifndef KMALLOC_MIN_SIZE > #define KMALLOC_MIN_SIZE (1 << KMALLOC_SHIFT_LOW) > #endif ... and simply drop the ifdef around that #define instead. That way, KMALLOC_MIN_SIZE is always defined in one place, and derived from KMALLOC_SHIFT_LOW; the logic will just set KMALLOC_SHIFT_LOW based on the various conditions. This seems a little safer to me; fewer conditions and less code to update if anything changes. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V8] kbuild: create a rule to run the pre-processor on *.dts files
From: Stephen Warren Create cmd_dtc_cpp to run the C pre-processor on *.dts file before passing them to dtc for final compilation. This allows the use of #define and #include within the .dts file. Acked-by: Simon Glass Acked-by: Jean-Christophe PLAGNIOL-VILLARD Acked-by: Michal Marek Acked-by: Srinivas Kandagatla Signed-off-by: Stephen Warren --- Grant, back in mid-November, you said you'd make a decision on this in the next couple of days, but I think this got overlooked. v8: Add -undef to the cpp command-line, to prevent e.g. "linux" and "unix" being defined; symbols which can appear in the device tree. v7: Build *.dtb from *.dts not src/*.dts. v6: No change. v5: * Update Documentation/kbuild for the new command and rule. v4: * Use -x assembler-with-cpp so pre-defined macros are set up so that #included header files know to only use cpp syntax, not C syntax. * Define __DTS__ for similar reasons. * use $(CPP) not $(CC) -E, and use $(cpp_flags). * Save the pre-processed results so they can be easily inspected when debugging build issues. * The use of -x assembler-with-cpp causes cpp to recognize directives in column 1 only. Hence, there's no need to escape property names that begin with #. Hence, there's no need for separate skeleton.dtsi and skeleton.dtsip. Maintain a separate file extension and build rule so that CPP-usage is opt-in. In particular, when using CPP, #include must be used rather than /include/ so that dependencies work. v3: Pass "-x c" not "-xc" to cpp. v2: Place make %.dtb: %.dtsp rule into Makefile.lib. --- Documentation/kbuild/makefiles.txt | 23 +++ scripts/Makefile.lib | 10 ++ 2 files changed, 33 insertions(+) diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt index 14c3f4f..5198b74 100644 --- a/Documentation/kbuild/makefiles.txt +++ b/Documentation/kbuild/makefiles.txt @@ -1186,6 +1186,29 @@ When kbuild executes, the following steps are followed (roughly): clean-files += *.dtb DTC_FLAGS ?= -p 1024 +dtc_cpp + This is just like dtc as describe above, except that the C pre- + processor is invoked upon the .dtsp file before compiling the result + with dtc. + + In order for build dependencies to work, all files compiled using + dtc_cpp must use the C pre-processor's #include functionality and not + dtc's /include/ functionality. + + Using the C pre-processor allows use of #define to create named + constants. In turn, the #defines will typically appear in a header + file, which may be shared with regular C code. Since the dtc language + represents a data structure rather than code in C syntax, similar + restrictions are placed on a header file included by a device tree + file as for a header file included by an assembly language file. + In particular, the C pre-processor is passed -x assembler-with-cpp, + which sets macro __ASSEMBLY__. __DTS__ is also set. These allow header + files to restrict their content to that compatible with device tree + source. + + A central rule exists to create $(obj)/%.dtb from $(src)/%.dtsp; + architecture Makefiles do no need to explicitly write out that rule. + --- 6.8 Custom kbuild commands When kbuild is executing with KBUILD_VERBOSE=0, then only a shorthand diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index bdf42fd..7910229 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -269,6 +269,16 @@ cmd_dtc = $(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 $(DTC_FLAGS) -d $(depfile $(obj)/%.dtb: $(src)/%.dts FORCE $(call if_changed_dep,dtc) +dtc-tmp = $(subst $(comma),_,$(dot-target).dts) + +quiet_cmd_dtc_cpp = DTC+CPP $@ +cmd_dtc_cpp = $(CPP) $(cpp_flags) -x assembler-with-cpp -undef -D__DTS__ \ + -o $(dtc-tmp) $< ; \ + $(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 $(DTC_FLAGS) $(dtc-tmp) + +$(obj)/%.dtb: $(src)/%.dtsp FORCE + $(call if_changed_dep,dtc_cpp) + # Bzip2 # --- -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/3] drm/exynos: Get HDMI version from device tree
n 02/05/2013 04:42 PM, Sean Paul wrote:
> Use the compatible string in the device tree to determine which
> registers/functions to use in the HDMI driver. Also changes the
> references from v13 to 4210 and v14 to 4212 to reflect the IP
> block version instead of the HDMI version.
> diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
> b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> #ifdef CONFIG_OF
> static struct of_device_id hdmi_match_types[] = {
> {
> - .compatible = "samsung,exynos5-hdmi",
> - .data = (void *)HDMI_TYPE14,
> + .compatible = "samsung,exynos4-hdmi",
> }, {
> /* end node */
> }
Why not fill in all the "base" compatible values there (I think you need
this anyway so that DTs don't all have to be compatible with
samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS*
values, then ...
> @@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
> +
> + if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi"))
> + hdata->version |= HDMI_VER_EXYNOS4210;
> + if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi"))
> + hdata->version |= HDMI_VER_EXYNOS4212;
> + if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi"))
> + hdata->version |= HDMI_VER_EXYNOS5250;
Instead of that, do roughly:
match = of_match_device(hdmi_match_types, &pdev->dev);
if (match)
hdata->version |= (int)match->data;
That way, it's all table-based. Any future additions to
hdmi_match_types[] won't require another if statement to be added to
probe().
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/3] drm/exynos: Get HDMI version from device tree
On 02/05/2013 05:37 PM, Sean Paul wrote:
> On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren wrote:
>> n 02/05/2013 04:42 PM, Sean Paul wrote:
>>> Use the compatible string in the device tree to determine which
>>> registers/functions to use in the HDMI driver. Also changes the
>>> references from v13 to 4210 and v14 to 4212 to reflect the IP
>>> block version instead of the HDMI version.
>>
>>> diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
>>> b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
>>
>> Binding looks sane to me.
>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>
>>> #ifdef CONFIG_OF
>>> static struct of_device_id hdmi_match_types[] = {
>>> {
>>> - .compatible = "samsung,exynos5-hdmi",
>>> - .data = (void *)HDMI_TYPE14,
>>> + .compatible = "samsung,exynos4-hdmi",
>>> }, {
>>> /* end node */
>>> }
>>
>> Why not fill in all the "base" compatible values there (I think you need
>> this anyway so that DTs don't all have to be compatible with
>> samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS*
>> values, then ...
>>
>
> At the moment, all DTs have to be compatible with exynos4-hdmi since
> it provides the base for the current driver. The driver uses 4210 and
> 4212 to differentiate between different register addresses and
> features, but most things are just exynos4-hdmi compatible.
The DT nodes should include only the compatible values that the HW is
actually compatible with. If the HW isn't compatible with exynos4-hdmi,
that value shouldn't be in the compatible property, but instead whatever
the "base" value that the HW really is compatible with. The driver can
support multiple "base" compatible values from this table.
>>> @@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
>>
>>> +
>>> + if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi"))
>>> + hdata->version |= HDMI_VER_EXYNOS4210;
>>> + if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi"))
>>> + hdata->version |= HDMI_VER_EXYNOS4212;
>>> + if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi"))
>>> + hdata->version |= HDMI_VER_EXYNOS5250;
>>
>> Instead of that, do roughly:
>>
>> match = of_match_device(hdmi_match_types, &pdev->dev);
>> if (match)
>> hdata->version |= (int)match->data;
>>
>> That way, it's all table-based. Any future additions to
>> hdmi_match_types[] won't require another if statement to be added to
>> probe().
>
> I don't think it's that easy. of_match_device returns the first match
> from the device table, so I'd still need to iterate through the
> matches. I could still break this out into a table, but I don't think
> of_match_device is the right way to probe it.
You shouldn't have to iterate over multiple matches. of_match_device()
is supposed to return the match for the first entry in the compatible
property, then if there was no match, move on to looking at the next
entry in the compatible property, etc. In practice, I think it's still
not implemented quite correctly for this, but you can make it work by
putting the newest compatible value first in the match table.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/3] drm/exynos: Get HDMI version from device tree
On 02/05/2013 05:56 PM, Sean Paul wrote:
> On Tue, Feb 5, 2013 at 4:42 PM, Stephen Warren wrote:
>> On 02/05/2013 05:37 PM, Sean Paul wrote:
>>> On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren
>>> wrote:
>>>> n 02/05/2013 04:42 PM, Sean Paul wrote:
>>>>> Use the compatible string in the device tree to determine which
>>>>> registers/functions to use in the HDMI driver. Also changes the
>>>>> references from v13 to 4210 and v14 to 4212 to reflect the IP
>>>>> block version instead of the HDMI version.
>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
>>>>> b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
>>>>
>>>> Binding looks sane to me.
>>>>
>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>>>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>>>
>>>>> #ifdef CONFIG_OF
>>>>> static struct of_device_id hdmi_match_types[] = {
>>>>> {
>>>>> - .compatible = "samsung,exynos5-hdmi",
>>>>> - .data = (void *)HDMI_TYPE14,
>>>>> + .compatible = "samsung,exynos4-hdmi",
>>>>> }, {
>>>>> /* end node */
>>>>> }
>>>>
>>>> Why not fill in all the "base" compatible values there (I think you need
>>>> this anyway so that DTs don't all have to be compatible with
>>>> samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS*
>>>> values, then ...
>>>>
>>>
>>> At the moment, all DTs have to be compatible with exynos4-hdmi since
>>> it provides the base for the current driver. The driver uses 4210 and
>>> 4212 to differentiate between different register addresses and
>>> features, but most things are just exynos4-hdmi compatible.
>>
>> The DT nodes should include only the compatible values that the HW is
>> actually compatible with. If the HW isn't compatible with exynos4-hdmi,
>> that value shouldn't be in the compatible property, but instead whatever
>> the "base" value that the HW really is compatible with. The driver can
>> support multiple "base" compatible values from this table.
>
> All devices that use this driver are compatible, at some level, with
> exynos4-hdmi, so I think its usage is correct here.
But can a driver that only knows about the original exynos4-hdmi operate
any of the HW correctly without any additional knowledge? If not, the
new HW isn't compatible with the old.
>>>>> @@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device
>>>>> *pdev)
>>>>
>>>>> +
>>>>> + if (of_device_is_compatible(dev->of_node,
>>>>> "samsung,exynos4210-hdmi"))
>>>>> + hdata->version |= HDMI_VER_EXYNOS4210;
>>>>> + if (of_device_is_compatible(dev->of_node,
>>>>> "samsung,exynos4212-hdmi"))
>>>>> + hdata->version |= HDMI_VER_EXYNOS4212;
>>>>> + if (of_device_is_compatible(dev->of_node,
>>>>> "samsung,exynos5250-hdmi"))
>>>>> + hdata->version |= HDMI_VER_EXYNOS5250;
>>>>
>>>> Instead of that, do roughly:
>>>>
>>>> match = of_match_device(hdmi_match_types, &pdev->dev);
>>>> if (match)
>>>> hdata->version |= (int)match->data;
>>>>
>>>> That way, it's all table-based. Any future additions to
>>>> hdmi_match_types[] won't require another if statement to be added to
>>>> probe().
>>>
>>> I don't think it's that easy. of_match_device returns the first match
>>> from the device table, so I'd still need to iterate through the
>>> matches. I could still break this out into a table, but I don't think
>>> of_match_device is the right way to probe it.
>>
>> You shouldn't have to iterate over multiple matches. of_match_device()
>> is supposed to return the match for the first entry in the compatible
>> property, then if there was no match, move on to looking at the next
>> entry in the compatible property, etc. In practice, I think it's still
>> not implemented quite correctly for this, but you can make it work by
>> putting the newest compatible value first in the match tabl
Re: [PATCH] clk: tegra: initialise parent of uart clocks
On 02/06/2013 05:22 AM, Peter De Schrijver wrote: > On Wed, Feb 06, 2013 at 11:47:41AM +0100, Laxman Dewangan wrote: >> Initialise the parent of UARTs to PLLP and disabling clock by >> default. >> > > I wonder if we should move the parent definitions to DT at some point. I think that's what Prashant is going to work on next, right? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: tegra: initialise parent of uart clocks
On 02/06/2013 03:47 AM, Laxman Dewangan wrote: > Initialise the parent of UARTs to PLLP OK > and disabling clock by default. Hmm. Only the clocks initialized by the new entries you added are marked disabled (or rather, not actively enabled; if they're enabled already, they won't be disabled). We should treat all UARTs equally. Historically we've needed to enable the serial clocks forcibly since the regular serial driver didn't call clk_get() or clk_prepare_enable() on any clocks, but I notice that it does now, since sometime in kernel 3.8. As such, I think you can modify all the UART entries in these tables to have the enable/state field set to false (0). Can you try that and check that it works for the serial console ports? Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] ARM: tegra: Fix build error w/ ARCH_TEGRA_114_SOC w/o ARCH_TEGRA_3x_SOC
On 02/06/2013 09:10 AM, Hiroshi Doyu wrote: > This patch fixes the build error when ARCH_TEGRA_114_SOC is enabled > and ARCH_TEGRA_3x_SOC is disabled. > > This provides Tegra114 with its own tegra114_init_early() instead of > making use of tegra30_init_early() so that T114 build doesn't depend > on T3x anymore. Olof, Arnd, This patch fixes the (well, an) issue for me. Can you apply it directly to the arm-soc tree. I believe it should apply on top of the tegra-for-3.9-soc-t114 pull request I sent. Let me know if you need a resend, although it was CC'd to LAKML, so I assume you have it in your email history. BTW, I anticipate a few more fixes coming down the line: 1) Clock-related patch from Laxman so that UART C clock on Cardhu is set up correctly for Bluetooth. v1 was posted already, but I think a v2 is needed here. 2) Another fix for Tegra20+Tegra114 w/o Tegra30 build fix from Hiroshi, related to drivers/amba/tegra-ahb.c, although there aren't any dependencies there, so perhaps that can go through Russell since he maintains that tree. 3) Something related to fb2229a "ARM: 7634/1: uncompress debug support for multiplatform build" since that breaks tegra_defconfig, but I haven't had a chance to look into that yet. 4) Something related to a boot failure related to crash in tegra_timer_interrupt(), which I'm just about to go investigate. Hopefully that's it... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: Modify leap year test for more simpler way
On 02/06/2013 06:00 AM, Haojian Zhuang wrote: > On Wed, Feb 6, 2013 at 8:43 PM, wrote: >> On 2013년 02월 06일 20:42, Venu Byravarasu wrote: >>> By definition, leap year is one, which is a divisible by 4 & 400, excluding >>> multiples of 100s. >>> Hence I feel this patch is not correct. >> >> No, I think you might misunderstood the it's meaning. The former code checks >> whether if it is multiple of 4 or not. Formal mathematical way to verify >> multiple of 4 >> is just checks the last two digits are multiple of 4. This '(!year % 4) && >> (year % 100)' >> part does it. But with only that checking, it may miss the case of multiple >> of 400 which >> is also multiple of 4. Then my modification checks in hexadecimal, whether >> if number >> has any of 1st and 2nd bit with value 1. Because any number which has all >> bits above >> the 3rd can be divided with 4(2^2). >> (e.g. 44(0b101100) = 2^5+2^3+2^2 = 2^2(2^3 + 2 + 1)) >> So It does same things with less instructions. > > I still can't understand your logic. > > Please check whether 200 year is leap year. > > 200(decimal) = 2b11001000 > > !(200 & 0x3) = 1 (Your condition said that 200 year is a leap year.) > > According to this logic in below. > if year mod 4 = 0 and year mod 100 <> 0 or year mod 400 = 0, then > it's a leap year. > > This tells us that 200 year isn't a leap year because 200 mod 100 == > 0. So who is wrong? The rule is: it's a leap year if divisible by 4, unless it's divisible by 100, but actually also including years divisible by 400. So, the current code is correct, and the patch is wrong. http://en.wikipedia.org/wiki/Leap_year#Algorithm -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] memory: tegra_ahb_enable_smmu() depends on TEGRA_IOMMU_SMMU
On 02/06/2013 11:34 AM, Hiroshi Doyu wrote: > New SoC, Tegra114 also uses SMMU. Change tegra_ahb_enable_smmu()'s > dependency from ARCH_TEGRA_3x_SOC to TEGRA_IOMMU_SMMU. No need to edit > whenever a new Tegra SoC comes. > > The following combination caused build error, which this patch fixes. > > CONFIG_ARCH_TEGRA_2x_SOC=y > \# CONFIG_ARCH_TEGRA_3x_SOC is not set > CONFIG_ARCH_TEGRA_114_SOC=y > > drivers/iommu/tegra-smmu.c:485: undefined reference to 'tegra_ahb_enable_smmu' Tested-by: Stephen Warren Russell, I assume Hiroshi should upload this to your patch tracker? It's needed for 3.9. Hiroshi, for reference, see: http://www.arm.linux.org.uk/developer/patches/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 4/4] arm: Add generic timer broadcast support
On 01/14/2013 10:05 AM, Mark Rutland wrote:
> Implement timer_broadcast for the arm architecture, allowing for the use
> of clock_event_device_drivers decoupled from the timer tick broadcast
> mechanism.
Mark, this patch is now in next-20130206 and causes a crash during boot
on Tegra. The reason appears to be because of:
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> @@ -524,7 +524,6 @@ static void __cpuinit percpu_timer_setup(void)
> struct clock_event_device *evt = &per_cpu(percpu_clockevent, cpu);
>
> evt->cpumask = cpumask_of(cpu);
> - evt->broadcast = smp_timer_broadcast;
After that change, evt->broadcast is never assigned, and hence is NULL.
Yet elsewhere in kernel/time/tick-broadcast.c it's used unconditionally:
static void tick_do_broadcast(struct cpumask *mask)
...
if (!cpumask_empty(mask)) {
...
td = &per_cpu(tick_cpu_device, cpumask_first(mask));
td->evtdev->broadcast(mask);
Now perhaps the Tegra timer driver simply isn't being set up correctly,
so the bug is there... But the only other place I can find where
->broadcast is assigned is in tick_device_uses_broadcast() which only
does it for "non-functional" timers, which doesn't apply to Tegra's timer.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/14] mfd: ab8500: provide a irq_set_type() function
On 02/05/2013 12:48 PM, Linus Walleij wrote:
> From: Lee Jones
>
> In the AB8500 IRQ mask and unmask functions, we rely on testing for
> IRQ_TYPE_EDGE_RISING and IRQ_TYPE_EDGE_FALLING interrupts to
> physically mask and unmask the correct interrupt lines. In order
> for us to do that, the trigger needs to be set in the associated
> flags. However, unless a irq_set_type() function pointer is passed
> when registering the IRQ chip, the IRQ subsystem will refuse to do
> it. For that reason, we're providing one.
> diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
> +static int ab8500_irq_set_type(struct irq_data *data, unsigned int type)
> +{
> + return 0;
> +}
I think patch 0 implied that only rising/falling edges can be detected,
not levels? If so, should that function validate that the requested type
can be supported?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/14] mfd: ab8500: ensure new AB8500 pinctrl driver is probed correctly
On 02/05/2013 12:48 PM, Linus Walleij wrote:
> From: Lee Jones
>
> The old, BROKEN AB8500 GPIO driver has been revamped as a shiny
> new pinctrl driver and has been renamed as such. So, if we would
> like to make use of it, we need to register it via its new name.
> diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
> static struct mfd_cell ab8500_devs[] = {
> {
> - .name = "ab8500-gpio",
> + .name = "pinctrl-ab8500",
> .of_compatible = "stericsson,ab8500-gpio",
> },
I assume that the GPIO/pinctrl driver has already been renamed in some
unrelated patch, so this patch is just fixing it so it works again?
If I'm not understanding correctly, doesn't this patch break "git
bisect", since it's only renaming it in the MFD core and not in the
driver that this expects to instantiate?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] pinctrl/abx500: use direct IRQ defines
On 02/05/2013 12:48 PM, Linus Walleij wrote:
> From: Linus Walleij
>
> Make it harder to do mistakes by introducing the actual
> defined ABx500 IRQ number into the IRQ cluster definitions.
> Deduct cluster offset from the GPIO offset to make each
> cluster coherent.
Shouldn't this patch be squashed into the previous patch to avoid churn?
> static struct abx500_pinctrl_soc_data ab9540_soc = {
> @@ -273,8 +273,7 @@ static int abx500_gpio_to_irq(struct gpio_chip *chip,
> unsigned offset)
> - hwirq = gpio + cluster->to_irq;
> -
> + hwirq = gpio - cluster->start + cluster->to_irq;
> return irq_create_mapping(pct->parent->domain, hwirq);
In particular, this change implies that the previous patch was simply
incorrect, although I haven't really thought about it in detail.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/14] ARM: ux500: enable AB8500 GPIO for HREF
On 02/05/2013 12:48 PM, Linus Walleij wrote:
> From: Lee Jones
>
> The AB8500 GPIO driver has been un-BROKEN and rewritten as a pinctrl
> driver. Now that it's back in use, let's ensure that it's available
> when booting HREF with Device Tree enabled.
> diff --git a/arch/arm/boot/dts/hrefprev60.dts
> b/arch/arm/boot/dts/hrefprev60.dts
> soc-u9500 {
> + prcmu@80157000 {
> + ab8500@5 {
> + ab8500-gpio {
> + compatible = "stericsson,ab8500-gpio";
The MFDs I've looked at (which admittedly might not be that many) all
have the top-level chip described in device tree, but not all the
component sub-devices, since they're all a static part of the top-level
chip. Instead, the top-level MFD instantiates all the sub-devices
itself. I'm curious why this MFD device works differently - do many work
like this?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 4/4] arm: Add generic timer broadcast support
On 02/07/2013 03:04 AM, Mark Rutland wrote:
> Hi Stephen,
>
> Sorry about this; I'm to blame for the bug.
>
> On Wed, Feb 06, 2013 at 08:51:43PM +, Stephen Warren wrote:
>> On 01/14/2013 10:05 AM, Mark Rutland wrote:
>>> Implement timer_broadcast for the arm architecture, allowing for the use
>>> of clock_event_device_drivers decoupled from the timer tick broadcast
>>> mechanism.
>>
>> Mark, this patch is now in next-20130206 and causes a crash during boot
>> on Tegra. The reason appears to be because of:
>>
>>> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
>>
>>> @@ -524,7 +524,6 @@ static void __cpuinit percpu_timer_setup(void)
>>> struct clock_event_device *evt = &per_cpu(percpu_clockevent, cpu);
>>>
>>> evt->cpumask = cpumask_of(cpu);
>>> - evt->broadcast = smp_timer_broadcast;
>>
>> After that change, evt->broadcast is never assigned, and hence is NULL.
>> Yet elsewhere in kernel/time/tick-broadcast.c it's used unconditionally:
>>
>> static void tick_do_broadcast(struct cpumask *mask)
>> ...
>> if (!cpumask_empty(mask)) {
>> ...
>> td = &per_cpu(tick_cpu_device, cpumask_first(mask));
>> td->evtdev->broadcast(mask);
>>
>> Now perhaps the Tegra timer driver simply isn't being set up correctly,
>> so the bug is there... But the only other place I can find where
>> ->broadcast is assigned is in tick_device_uses_broadcast() which only
>> does it for "non-functional" timers, which doesn't apply to Tegra's timer.
>>
>
> The intent of 12ad100046: "clockevents: Add generic timer broadcast function"
> was to setup the broadcast function both for non-functional/dummy timers and
> those that stop in low-power states (CLOCK_EVT_FEAT_C3STOP). I missed the
> CLOCK_EVT_FEAT_C3STOP case.
>
> I believe the patch below will fix this for Tegra and any other platforms
> where
> broadcast is required in low power states.
>
> Stephen, could you test this for Tegra?
Tested-by: Stephen Warren
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 4/4] arm: Add generic timer broadcast support
On 02/07/2013 04:40 AM, Santosh Shilimkar wrote:
> Mark,
>
> On Thursday 07 February 2013 03:34 PM, Mark Rutland wrote:
>> Hi Stephen,
>>
>> Sorry about this; I'm to blame for the bug.
>>
>> On Wed, Feb 06, 2013 at 08:51:43PM +, Stephen Warren wrote:
>>> On 01/14/2013 10:05 AM, Mark Rutland wrote:
>>>> Implement timer_broadcast for the arm architecture, allowing for the
>>>> use
>>>> of clock_event_device_drivers decoupled from the timer tick broadcast
>>>> mechanism.
>>>
>>> Mark, this patch is now in next-20130206 and causes a crash during boot
>>> on Tegra. The reason appears to be because of:
>>>
>>>> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
>>>
>>>> @@ -524,7 +524,6 @@ static void __cpuinit percpu_timer_setup(void)
>>>> struct clock_event_device *evt = &per_cpu(percpu_clockevent,
>>>> cpu);
>>>>
>>>> evt->cpumask = cpumask_of(cpu);
>>>> -evt->broadcast = smp_timer_broadcast;
>>>
>>> After that change, evt->broadcast is never assigned, and hence is NULL.
>>> Yet elsewhere in kernel/time/tick-broadcast.c it's used unconditionally:
>>>
>>> static void tick_do_broadcast(struct cpumask *mask)
>>> ...
>>> if (!cpumask_empty(mask)) {
>>> ...
>>> td = &per_cpu(tick_cpu_device, cpumask_first(mask));
>>> td->evtdev->broadcast(mask);
>>>
>>> Now perhaps the Tegra timer driver simply isn't being set up correctly,
>>> so the bug is there... But the only other place I can find where
>>> ->broadcast is assigned is in tick_device_uses_broadcast() which only
>>> does it for "non-functional" timers, which doesn't apply to Tegra's
>>> timer.
>>
>> The intent of 12ad100046: "clockevents: Add generic timer broadcast
>> function"
>> was to setup the broadcast function both for non-functional/dummy
>> timers and
>> those that stop in low-power states (CLOCK_EVT_FEAT_C3STOP). I missed the
>> CLOCK_EVT_FEAT_C3STOP case.
>
> I am not sure this exactly the case. Because in my testing, the C3STOP
> path was exercised already. And if the C3STOP is used then notifiers
> calls are expected for switching of clock-events to broadcast mode.
>
> And dummy broad-cast hook should come into picture only if the per CPU
> local timer clock-event are not registered.
>
> I just tried "next-20130207" on OMAP and I see the broad-cast mechanism
> works and didn't observe any crash.
...
>> I believe the patch below will fix this for Tegra and any other
>> platforms where
>> broadcast is required in low power states.
>>
> Am not sure you really need that patch unless and until am missing a
> scenario in my test.
>
> Stephan,
>
> Do we have crash log seen on Tegra ? Which tegra CPUIDLE driver
> is being used when crash is seen ?
The crash log is below. It's simply because td->evt->broadcast is NULL
because it wasn't ever set.
I'm using the CPUIDLE driver in arch/arm/mach-tegra/cpuidle*.c.
> [ 4.179156] Unable to handle kernel NULL pointer dereference at virtual
> address
> [ 4.211200] pgd = c0004000
> [ 4.237688] [] *pgd=
> [ 4.264868] Internal error: Oops: 8005 [#1] PREEMPT SMP ARM
> [ 4.294396] Modules linked in:
> [ 4.320940] CPU: 0 Not tainted (3.8.0-rc6-next-20130206-2-g2aaa5ff #6)
> [ 4.352125] PC is at 0x0
> [ 4.378346] LR is at tick_do_broadcast.constprop.3+0x74/0xb0
> [ 4.407786] pc : [<>] lr : [] psr: 2193
> [ 4.407786] sp : c06d7d60 ip : 0002 fp : 0004
> [ 4.466633] r10: c06d2ef8 r9 : c06def08 r8 : c06dec98
> [ 4.495457] r7 : r6 : f8b8d230 r5 : c073d598 r4 :
> [ 4.525825] r3 : r2 : 008ac000 r1 : 0004 r0 : c073d5a4
> [ 4.556192] Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
> [ 4.587865] Control: 10c5387d Table: 404a DAC: 0015
> [ 4.617983] Process swapper/0 (pid: 0, stack limit = 0xc06d6238)
> [ 4.648314] Stack: (0xc06d7d60 to 0xc06d8000)
> [ 4.677145] 7d60: 0002 7fff c0063c28 c06f17c0
> f8b8d230
> [ 4.710387] 7d80: c0029b40 c06f1780 ee015ad0 0049
> c07186a7 ee015a80
> [ 4.743652] 7da0: 0001 c0228c2c c0228c08 c0079184 f8b66130
> ee2dc8e8 ee015a80
> [ 4.776806] 7dc0: ee015ad0 c06f1780 fe000100 c06d8080 c06d6000 c04e01ac
> c0079304
> [ 4.810088] 7de0: ee015a80 ee015ad0 c06d7ee8 c007bf38 c007bebc 0049
>
Re: [PATCH v3] usb: phy: moving all PHY API definitions to usb/phy directory
On 02/07/2013 06:38 AM, kishon wrote: > On Thursday 07 February 2013 05:02 PM, Venu Byravarasu wrote: >> As drivers/usb/otg/otg.c contains most of the PHY related APIs which >> are not OTG specific, moving them to more logical place under >> drivers/usb/phy. >> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile >> index b13faa1..886be13 100644 >> --- a/drivers/usb/phy/Makefile >> +++ b/drivers/usb/phy/Makefile >> @@ -4,6 +4,7 @@ >> >> ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG >> >> +obj-$(CONFIG_USB_COMMON)+= phy.o > > I think we should have a separate config for PHYs. There might be > controllers which dont need separate PHY. But we can have that as a > separate patch later since we might want to change the Kconfig of other > UDC drivers. >> +/* >> + * phy.c -- USB PHY utility code >> + * >> + * Copyright (C) 2004 Texas Instruments > > You can update this to 2013. I'm not sure if anyone other that TI can do that? >> +EXPORT_SYMBOL_GPL(usb_bind_phy); >> + >> + > > Unnecessary spaces here. Thanks for the review. It was very difficult to find you review comments since you quoted the entire patch rather than just the few pieces you were commenting on... > You might want to do this patch after applying *[PATCH] usb: otg: use > try_module_get in all usb_get_phy functions and add missing module_put* > by Marc. Venu, please make sure you include the changes in that patch in your Tegra USB/PHY driver rework too, before you post it upstream. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] pinctrl/abx500: use direct IRQ defines
On 02/07/2013 02:01 AM, Lee Jones wrote:
> I don't see myself on cc. Was that intentional?
The original patch was that way; I assume git send-email only CC'd you
on patches written by you.
> I quite like the idea of this.
>
> Stephen,
>
> It doesn't mean the other patch was wrong, it just transfers the math.
Ah, I see. The issue is that the code below clearly calculates the hwirq
differently, and it wasn't immediately obvious that this part of the
patch for example:
> struct abx500_gpio_irq_cluster ab8500_gpio_irq_cluster[] = {
> - GPIO_IRQ_CLUSTER(6, 13, 34),
> - GPIO_IRQ_CLUSTER(24, 25, 24),
> - GPIO_IRQ_CLUSTER(36, 41, 14),
> + GPIO_IRQ_CLUSTER(6, 13, AB8500_INT_GPIO6R),
> + GPIO_IRQ_CLUSTER(24, 25, AB8500_INT_GPIO24R),
> + GPIO_IRQ_CLUSTER(36, 41, AB8500_INT_GPIO36R),
> };
... actually changes the values in the table (AB8500_INT_GPIO6R is 40,
so when using that value, you need to subtract of the value 6 for the
base to get the original 34).
> I wouldn't squash it into mine. I like the transition and the
> possibility to revert it if there's been some mistake.
>
> (not to say there is one, but just in case.)
>
> Sent from my mobile Linux device.
>
> On Feb 7, 2013 12:14 AM, "Stephen Warren" <mailto:swar...@wwwdotorg.org>> wrote:
>
> On 02/05/2013 12:48 PM, Linus Walleij wrote:
> > From: Linus Walleij <mailto:linus.wall...@linaro.org>>
> >
> > Make it harder to do mistakes by introducing the actual
> > defined ABx500 IRQ number into the IRQ cluster definitions.
> > Deduct cluster offset from the GPIO offset to make each
> > cluster coherent.
>
> Shouldn't this patch be squashed into the previous patch to avoid churn?
>
> > static struct abx500_pinctrl_soc_data ab9540_soc = {
>
> > @@ -273,8 +273,7 @@ static int abx500_gpio_to_irq(struct gpio_chip
> *chip, unsigned offset)
>
> > - hwirq = gpio + cluster->to_irq;
> > -
> > + hwirq = gpio - cluster->start + cluster->to_irq;
> > return
> irq_create_mapping(pct->parent->domain, hwirq);
>
> In particular, this change implies that the previous patch was simply
> incorrect, although I haven't really thought about it in detail.
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> <mailto:majord...@vger.kernel.org>
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/10] USB: EHCI: make ehci-msm a separate driver
On 02/07/2013 10:34 AM, manjunath.gou...@linaro.org wrote: > Separate the Qualcomm On-Chip host controller driver from ehci-hcd host code > into its own driver module. > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig > @@ -209,7 +210,7 @@ config USB_EHCI_TEGRA > boolean "NVIDIA Tegra HCD support" > depends on USB_EHCI_HCD && ARCH_TEGRA > select USB_EHCI_ROOT_HUB_TT > - help > + --- help--- > This driver enables support for the internal USB Host Controllers > found in NVIDIA Tegra SoCs. The controllers are EHCI compliant. That seems like an unrelated change. The indentation and space after first "---" looks wrong too. It looks pretty random which of "help" and "---help---" is used in this file; if you're going to fix that, I think fix them all, but in a separate patch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: tegra: Implement locking for super clock
On 02/07/2013 11:57 AM, Mike Turquette wrote:
> Quoting Peter De Schrijver (2013-02-07 08:24:14)
>> Although tegra_clk_register_super_mux() has a lock parameter, the lock is not
>> actually used by the code. Fixed with this patch.
>>
>> Signed-off-by: Peter De Schrijver
>> ---
>> drivers/clk/tegra/clk-super.c | 18 +++---
>> 1 files changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/tegra/clk-super.c b/drivers/clk/tegra/clk-super.c
>> index 7ad48a8..2fd924d 100644
>> --- a/drivers/clk/tegra/clk-super.c
>> +++ b/drivers/clk/tegra/clk-super.c
>> @@ -73,7 +73,12 @@ static int clk_super_set_parent(struct clk_hw *hw, u8
>> index)
>> {
>> struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
>> u32 val, state;
>> + int err = 0;
>> u8 parent_index, shift;
>> + unsigned long flags = 0;
>
> I don't think initializing flags to zero is necessary but it is not a
> big deal. Is gcc throwing a warning? Otherwise:
>
> Acked-by: Mike Turquette
I've applied this to Tegra's for-3.9/soc-ccf-fixes branch. If you want
to repost given Mike's comments, I can take an updated version.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: tegra: local arrays should be static
On 02/07/2013 10:49 AM, Mike Turquette wrote: > Quoting Peter De Schrijver (2013-02-07 08:30:36) >> cclk_g_parents, cclk_lp_parents and sclk_parents are only accessed from >> within >> clk-tegra30.c. Declare them static to avoid namespace polution. >> >> Signed-off-by: Peter De Schrijver > > Acked-by: Mike Turquette I've applied this to Tegra's for-3.9/soc-ccf-fixes branch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: tegra: Add missing spinlock for hclk and pclk
On 02/07/2013 10:49 AM, Mike Turquette wrote: > Quoting Peter De Schrijver (2013-02-07 08:37:35) >> The hclk and pclk clocks are controlled by the same register. Hence a lock is >> required to avoid corruption. >> >> Signed-off-by: Peter De Schrijver > > I assume this is going through the tegra tree with the other ccf > patches, so: > > Acked-by: Mike Turquette I've applied this to Tegra's for-3.9/soc-ccf-fixes branch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] clocksource: tegra20: use the device_node pointer passed to init
On 02/07/2013 12:09 PM, Rob Herring wrote: > From: Rob Herring > > We've already matched the node, so use the node pointer passed in. The rtc > init was intermingled with the timer init, so split this out to a separate > init function. The series, Reviewed-by: Stephen Warren Patches 1,4: Tested-by: Stephen Warren One thing I wonder re: patch 4 - I know someone (I think Hiroshi, now CC'd) planned to refactor drivers/clocksource/tegra20_timer.c to enhance it for Tegra114. I'd like to check with him that the refactoring in this patch won't impede that at all. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] pinctrl/abx500: use direct IRQ defines
On 02/08/2013 01:25 AM, Lee Jones wrote:
> On Thu, 07 Feb 2013, Stephen Warren wrote:
>
>> On 02/07/2013 02:01 AM, Lee Jones wrote:
>>> I don't see myself on cc. Was that intentional?
>>
>> The original patch was that way; I assume git send-email only CC'd you
>> on patches written by you.
>
> No, I didn't send this patch at all.
>
> I was asking Linus if he ment to CC me, as I thought I should have been.
>
>>> I quite like the idea of this.
>>>
>>> Stephen,
>>>
>>> It doesn't mean the other patch was wrong, it just transfers the math.
>>
>> Ah, I see. The issue is that the code below clearly calculates the hwirq
>> differently, and it wasn't immediately obvious that this part of the
>> patch for example:
>>
>>> struct abx500_gpio_irq_cluster ab8500_gpio_irq_cluster[] = {
>>> - GPIO_IRQ_CLUSTER(6, 13, 34),
>>> - GPIO_IRQ_CLUSTER(24, 25, 24),
>>> - GPIO_IRQ_CLUSTER(36, 41, 14),
>>> + GPIO_IRQ_CLUSTER(6, 13, AB8500_INT_GPIO6R),
>>> + GPIO_IRQ_CLUSTER(24, 25, AB8500_INT_GPIO24R),
>>> + GPIO_IRQ_CLUSTER(36, 41, AB8500_INT_GPIO36R),
>>> };
>>
>> ... actually changes the values in the table (AB8500_INT_GPIO6R is 40,
>> so when using that value, you need to subtract of the value 6 for the
>> base to get the original 34).
>
> Yes, I see how that may of looked if you didn't see the other change.
>
> So you're happy?
Yes.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
On 02/08/2013 05:29 AM, Marc Dietrich wrote:
> Hiroshi,
>
> Am Freitag, 8. Februar 2013, 09:29:31 schrieb Hiroshi Doyu:
>> Refactored tegra{20,30,114}_init_early() so that we have the unified
>> tegra_init_early().
>> diff --git a/arch/arm/mach-tegra/hotplug.c b/arch/arm/mach-tegra/hotplug.c
>> +void __init tegra_hotplug_init(void)
>> {
>> +switch (tegra_chip_id) {
>> +#ifdef CONFIG_ARCH_TEGRA_2x_SOC
>> +case TEGRA20:
>> +tegra_hotplug_shutdown = tegra20_hotplug_shutdown;
>> +break;
>> #endif
>> +#if defined(CONFIG_ARCH_TEGRA_3x_SOC)
>> +case TEGRA30:
>> +tegra_hotplug_shutdown = tegra30_hotplug_shutdown;
>> +break;
>> #endif
>> +default:
>> +BUG_ON(IS_ENABLED(CONFIG_HOTPLUG_CPU));
>> +break;
>> +}
>> +}
>
> are these ifdefs really needed? Multisoc kernels will enable them all anyway
> and there is a case structure which protects the assignments. Also the
> hotplug
> functions are very tiny, so there shouldn't be a big loss.
The files that contain/implement those functions are separate for each
SoC and only included in the build when the individual SoCs are enabled.
While multi-platform SoCs do make sense for distros, we also very
specifically want to support the case where only Tegra, and only a
single Tegra SoC, is enabled, hence this separation.
(and Tegra doesn't support multi-platform yet; maybe in another kernel
release or two)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
