RE: [PATCH 2/2] fsl/mpic_timer: make mpic_timer to support deep sleep feature
-Original Message- From: Wood Scott-B07421 Sent: Wednesday, April 16, 2014 5:06 AM To: Wang Dongsheng-B40534 Cc: Jin Zhengxiong-R64188; Li Yang-Leo-R58472; Zhao Chenhui-B35336; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH 2/2] fsl/mpic_timer: make mpic_timer to support deep sleep feature On Mon, 2014-04-14 at 22:23 -0500, Wang Dongsheng-B40534 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, April 15, 2014 7:36 AM To: Wang Dongsheng-B40534 Cc: Jin Zhengxiong-R64188; Li Yang-Leo-R58472; Zhao Chenhui-B35336; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH 2/2] fsl/mpic_timer: make mpic_timer to support deep sleep feature On Mon, 2014-04-14 at 10:24 +0800, Dongsheng Wang wrote: From: Wang Dongsheng dongsheng.w...@freescale.com At T104x platfrom the timer clock will be changed when system going to deep sleep. Could you elaborate on what is changing and why? Okay. +#include asm/mpc85xx.h #include asm/mpic_timer.h So much for, The driver currently is only tested on fsl chip, but it can potentially support other global timers complying to OpenPIC standard. #define FSL_GLOBAL_TIMER 0x1 @@ -71,8 +74,10 @@ struct timer_group_priv { struct timer_regs __iomem *regs; struct mpic_timer timer[TIMERS_PER_GROUP]; struct list_headnode; + unsigned long idle; unsigned inttimerfreq; - unsigned intidle; Why? Um... It shouldn't be happened...i will remove this. + unsigned intsuspended_timerfreq; + unsigned intresume_timerfreq; unsigned intflags; spinlock_t lock; void __iomem*group_tcr; @@ -88,6 +93,7 @@ static struct cascade_priv cascade_timer[] = { }; static LIST_HEAD(timer_group_list); +static int switch_freq_flag; Needs documentation, and based on _flag it should probably be a bool. Okay. static void convert_ticks_to_time(struct timer_group_priv *priv, const u64 ticks, struct timeval *time) @@ -423,6 +429,33 @@ struct mpic_timer *mpic_request_timer(irq_handler_t fn, void *dev, } EXPORT_SYMBOL(mpic_request_timer); +static void timer_group_get_suspended_freq(struct timer_group_priv +*priv) { + struct device_node *np; + + np = of_find_compatible_node(NULL, NULL, fsl,qoriq-clockgen-2.0); + if (!np) { + pr_err(mpic timer: Missing clockgen device node.\n); Why is it an error to not have a 2.0 QorIQ clockgen? This will affect the accuracy of the timer. But not means the timer cannot work. Maybe you are right, this pr_err should be a pr_warn. What I mean is, what if the mpic timer driver is used with deep sleep on a different chip such as mpc8536? Only T104x has this feature, other platform will not be effect. I will remove this pr_err. + return; + } + + of_property_read_u32(np, clock-frequency, priv- suspended_timerfreq); + of_node_put(np); Shouldn't this go through the clock API? Sorry, I'm not clear about clock API, you mean fsl_get_sys_freq()? Or ? No, I mean drivers/clk/ Though I suppose manually reading clock-frequency is OK, as the clock-frequency on the clockgen node predated the introduction of clock bindings to the device tree. Don't use fsl_get_sys_freq(). No, we cannot use drivers/clk/. Because clk-ppc-corenet.c only support corenet platform. +static int need_to_switch_freq(void) +{ + u32 svr; + + svr = mfspr(SPRN_SVR); + if (SVR_SOC_VER(svr) == SVR_T1040 || + SVR_SOC_VER(svr) == SVR_T1042) + return 1; Explain why this is specific to T104x. Mpic timer freq will be change when system going to deep sleep. So we need to recalculate the time. Do any other chips with deep sleep have this problem? Only has on T104x right now. Regards, -Dongsheng -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] mtd: m25p80: Modify the name of mtd_info
On Fri, Mar 21, 2014 at 07:16:18PM +0800, Hou Zhiqiang wrote: To specify spi flash layouts by mtdparts=... in cmdline, we must give mtd_info a fixed name,because the cmdlinepart's parser will match the name of mtd_info given in cmdline. Now, if use DT, the mtd_info's name will be spi-dev-name. It consists of spi_master-bus_num, and the spi_master-bus_num maybe dynamically fetched. So, in this case, replace the component bus_num with thei physical address of spi master. Signed-off-by: Hou Zhiqiang b48...@freescale.com --- drivers/mtd/devices/m25p80.c | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c index 7eda71d..64450a2 100644 --- a/drivers/mtd/devices/m25p80.c +++ b/drivers/mtd/devices/m25p80.c @@ -30,6 +30,7 @@ #include linux/mtd/cfi.h #include linux/mtd/mtd.h #include linux/mtd/partitions.h +#include linux/of_address.h #include linux/of_platform.h #include linux/spi/spi.h @@ -934,9 +935,11 @@ static int m25p_probe(struct spi_device *spi) struct flash_platform_data *data; struct m25p *flash; struct flash_info *info; - unsignedi; + unsignedi, ret; struct mtd_part_parser_data ppdata; struct device_node *np = spi-dev.of_node; + struct resource res; + struct device_node *mnp = spi-master-dev.of_node; /* Platform data helps sort out which chip type we have, as * well as how this board partitions it. If we don't have @@ -1009,8 +1012,17 @@ static int m25p_probe(struct spi_device *spi) if (data data-name) flash-mtd.name = data-name; - else - flash-mtd.name = dev_name(spi-dev); + else{ + ret = of_address_to_resource(mnp, 0, res); You're making a lot assumptions about the SPI master/device relationship -- that the master has a device-node; that the first resource of the master is its physical address (this is not guaranteed by DT semantics). Please do not do this. + if (ret) { + dev_err(spi-dev, failed to get spi master resource\n); + return ret; This is wrong. It breaks all !CONFIG_OF cases which didn't specify a name via platform data. + } + flash-mtd.name = kasprintf(GFP_KERNEL, spi%x.%d, + (unsigned)res.start, spi-chip_select); + if (!flash-mtd.name) + return -ENOMEM; + } flash-mtd.type = MTD_NORFLASH; flash-mtd.writesize = 1; Brian ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [v3] mtd: m25p80: Modify the name of mtd_info
On Sat, Mar 22, 2014 at 1:55 AM, Hou Zhiqiang b48...@freescale.com wrote: v3: Fix a bug, matching unsigned long long with %llx. v2: 1. Fix some code style issue. 2. Cast physical address to unsigned long long. I missed v2 and v3 and commented on v1 instead, sorry. Same comments apply here though. Also, can you please include PATCH in the subject? (e.g., [PATCH v3]) See Documentation/SubmittingPatches, item 11. Thanks, Brian ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v4] powerpc: kvm: make _PAGE_NUMA take effect
On 15.04.14 10:33, Liu Ping Fan wrote: Numa fault is a method which help to achieve auto numa balancing. When such a page fault takes place, the page fault handler will check whether the page is placed correctly. If not, migration should be involved to cut down the distance between the cpu and pages. A pte with _PAGE_NUMA help to implement numa fault. It means not to allow the MMU to access the page directly. So a page fault is triggered and numa fault handler gets the opportunity to run checker. As for the access of MMU, we need special handling for the powernv's guest. When we mark a pte with _PAGE_NUMA, we already call mmu_notifier to invalidate it in guest's htab, but when we tried to re-insert them, we firstly try to map it in real-mode. Only after this fails, we fallback to virt mode, and most of important, we run numa fault handler in virt mode. This patch guards the way of real-mode to ensure that if a pte is marked with _PAGE_NUMA, it will NOT be mapped in real mode, instead, it will be mapped in virt mode and have the opportunity to be checked with placement. Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com Reviewed-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com Thanks, applied to for-3.15. Alex ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/1] powerpc: Increase COMMAND_LINE_SIZE to 2048 from 512.
On 04/15/2014 05:49 AM, Benjamin Herrenschmidt wrote: On Mon, 2014-04-14 at 14:58 -0400, Joseph Salisbury wrote: After further review, it appears ppc does not actually use the define in the ppc headers but uses the common generic default(include/uapi/asm-generic/setup.h). COMMAND_LINE_SIZE should probably become a kernel config option. Do folks agree that is the correct thing to do? If so, I can re-work the patch. No objection on my side. Make sure you remove any unused arch define while at it. Cheers, Ben. Hi Ben, I can think of two ways to add the new config option. One would be to have a large entry in ~/arch/Kconfig, with a default COMMAND_LINE_SIZE line for each architecture. The other way would be to have the default value for COMMAND_LINE_SIZE in the architecture sub-directory Kconfig file: ~/arch/powerpc/Kconfig for example. Do you have a preference for either way? Thanks, Joe ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/2] fsl/corenet_generic: add a particular initialization for platform
On Tue, 2014-04-15 at 21:58 -0500, Wang Dongsheng-B40534 wrote: -Original Message- From: Wood Scott-B07421 Sent: Wednesday, April 16, 2014 3:39 AM To: Wang Dongsheng-B40534 Cc: Jin Zhengxiong-R64188; haoke...@gmail.com; Kushwaha Prabhakar-B32579; linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH v2 1/2] fsl/corenet_generic: add a particular initialization for platform On Tue, 2014-04-15 at 13:53 +0800, Dongsheng Wang wrote: From: Wang Dongsheng dongsheng.w...@freescale.com Corenet_generic is a generic platform initialization. Those based on the corenet_generic board maybe need a particular initialize to enable/set some IP-Blocks. So add Fix Generic Initialization to solve this kind of special cases. I still don't understand what you mean by fix. What are you fixing, or what is fixed? There is no need for adding an infrastructure layer here. Just add a new piece of code for t104x diu, and have it be called by an appropriate initfunc. fix is means to handle some boards those based on corenet_generic config file, But those boards may need some special handle. Perhaps these used to handle special feature codes not have an appropriate initfunc we cannot *just find* an appropriate place, I'm not asking you to just find anything. I'm asking you to add an initfunc in a standalone file. if more and more boards need to do this, at that time maybe *initfunc* looks very complicated. They would each have their own initfunc. There is no reason to tie this in with anything else. --- a/arch/powerpc/platforms/85xx/Kconfig +++ b/arch/powerpc/platforms/85xx/Kconfig @@ -269,6 +269,17 @@ config CORENET_GENERIC The following boards are supported for both 32bit and 64bit kernel: P5020 DS and P5040 DS +config FIX_GENERIC_PLATFORM_INIT + bool Fix Generic Initialization + depends on CORENET_GENERIC Why does this depend on CORENET_GENERIC? Because CORENET_GENERIC is a multiboards file, This is designed to handle this situation. This DIU code is going to be just as applicable to a custom T104x board which may or may not use CORENET_GENERIC. + default y No. Why not? This will not increase any redundant operations if there is not any boards need fix. You can see my fix.c code. default y should not be used for hardware specific code. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH RFC v11 5/6] dma: mpc512x: add device tree binding document
On Tue, 2014-04-15 at 14:54 +0400, Alexander Popov wrote: Introduce a device tree binding document for the MPC512x DMA controller Signed-off-by: Gerhard Sittig g...@denx.de Signed-off-by: Alexander Popov a13xp0p0...@gmail.com I'm not certain whether the attribution is right. Is the S-o-b appropriate when the patch is not from me? As I've stated before, it's OK if you pick up and extend what I provide, but please don't pretend that I wrote what you did, and don't pretend that I ACKed or passed along your submission when I didn't. This binding certainly needs further improvement to become a good one. As I've communicated in the past, I was rather ignorant back then when I wrote v1 and v2 of the RFC. We have learned something in the meantime. Though I admit having gone silent after several review iterations. Assumed you would pick up information that showed up several times on public lists. --- /dev/null +++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt @@ -0,0 +1,51 @@ +* Freescale MPC512x and MPC8308 DMA Controller + +The DMA controller in the Freescale MPC512x and MPC8308 SoCs can move +blocks of memory contents between memory and peripherals or +from memory to memory. + +Refer to the Generic DMA Controller and DMA request bindings in +the dma/dma.txt file for a more detailed description of binding. + +* DMA controller + +Required properties: +- compatible: Should be one of + fsl,mpc5121-dma + fsl,mpc8308-dma, fsl,mpc5121-dma is this a duplicate? looks funny, needs a fix or is it a requirement that for MPC8308 you need to provide both compatible strings? that would be wrong, as MPC8308 certainly is not an MPC5121 a quick search reveals: the drivers/dma/mpc512x_dma.c Linux driver implementation is wrong, it should match on both strings; expecting the MPC8308 to disguise as an MPC5121 when it's not is inappropriate (and only went unnoticed because of missing bindings, I guess) +- reg: Address and size of the DMA controller's register set +- interrupts: Interrupt for the DMA controller. Generic interrupt client node + is described in interrupt-controller/interrupts.txt 'interrupts' only works in combinations with 'interrupt-parent', that actual .dts files don't have the latter in the nodes is an implementation detail but not a binding's requirement and an alternative method of specifying interrupts was introduced recently, a reference to the common binding without naming one specific property name could be most appropriate + +Optional properties: +- #dma-cells: The length of the DMA specifier, must be 1 since + the DMA controller uses a fixed assignment of request lines + per channel. Refer to dma/dma.txt for the detailed description + of this property I'm afraid that a generic/common document does not and cannot describe the specific semantics of this provider's cells this binding should explicitly mention that the number of cells needs to be one, and that this one cell is the DMA channel (which translates to peripheral request line), because these assigments are fixed in hardware + +Example: + + dma0: dma@14000 { + compatible = fsl,mpc5121-dma; + reg = 0x14000 0x1800; + interrupts = 65 0x8; + #dma-cells = 1; + }; + +* DMA client the DMA provider's binding probably need not discuss client specs, a reference to the common binding should suffice if it's appropriate at all virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev