Re: [RFC PATCH 1/7] mmc: sdhci: add quirk for broken 3.0V support
On Fri, Jun 20, 2014 at 05:35:22PM +0800, Vincent Yang wrote: This patch defines a quirk for platforms unable to enable 3.0V support. It is a preparation and will be used by Fujitsu SDHCI controller f_sdh30 driver. Signed-off-by: Vincent Yang vincent.y...@tw.fujitsu.com I don't think you need this patch. Instead, you can exclude 3V using the voltage-ranges = in the device tree. Thanks, Anton drivers/mmc/host/sdhci.c | 3 +++ include/linux/mmc/sdhci.h | 2 ++ 2 files changed, 5 insertions(+) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 47055f3..523075f 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -3069,6 +3069,9 @@ int sdhci_add_host(struct sdhci_host *host) } #endif /* CONFIG_REGULATOR */ + if (host-quirks2 SDHCI_QUIRK2_NO_3_0_V) + caps[0] = ~SDHCI_CAN_VDD_300; + /* * According to SD Host Controller spec v3.00, if the Host System * can afford more than 150mA, Host Driver should set XPC to 1. Also diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h index 08abe99..cac0958 100644 --- a/include/linux/mmc/sdhci.h +++ b/include/linux/mmc/sdhci.h @@ -98,6 +98,8 @@ struct sdhci_host { #define SDHCI_QUIRK2_BROKEN_HS200(16) /* Controller does not support DDR50 */ #define SDHCI_QUIRK2_BROKEN_DDR50(17) +/* The system physically doesn't support 3.0v, even if the host does */ +#define SDHCI_QUIRK2_NO_3_0_V(18) int irq;/* Device IRQ */ void __iomem *ioaddr; /* Mapped address */ -- 1.9.0 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3 V5] mmc:core: parse voltage from device-tree
On Wed, Aug 14, 2013 at 01:46:11PM +0800, Haijun Zhang wrote: Add function to support get voltage from device-tree. If there are voltage-range specified in device-tree node, this function will parse it and return the available voltage mask. Signed-off-by: Haijun Zhang haijun.zh...@freescale.com Acked-by: Anton Vorontsov an...@enomsg.org Thanks a lot! Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V3] mmc:of_spi: Update the code of getting voltage-ranges
On Mon, Aug 12, 2013 at 09:39:05AM +0800, Haijun Zhang wrote: Using function mmc_of_parse_voltage() to get voltage-ranges. Signed-off-by: Haijun Zhang haijun.zh...@freescale.com --- Acked-by: Anton Vorontsov an...@enomsg.org ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3 V3] mmc:esdhc: add support to get voltage from device-tree
On Mon, Aug 12, 2013 at 09:39:04AM +0800, Haijun Zhang wrote: Add suppport to get voltage from device-tree node for esdhc host, if voltage-ranges was specified in device-tree node we can get ocr_mask instead of read from host capacity register. If not voltages still can be get from host capacity register. Signed-off-by: Haijun Zhang haijun.zh...@freescale.com Acked-by: Anton Vorontsov an...@enomsg.org ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3 V3] mmc:sdhc: get voltage from sdhc host
On Mon, Aug 12, 2013 at 09:39:06AM +0800, Haijun Zhang wrote: We use host-ocr_mask to hold the voltage get from device-tree node, In case host-ocr_mask was available, we use host-ocr_mask as the final available voltage can be used by MMC/SD/SDIO card. Signed-off-by: Haijun Zhang haijun.zh...@freescale.com --- Reviewed-by: Anton Vorontsov an...@enomsg.org ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] mmc:of_spi: Update the code of getting voltage-ranges
On Wed, Jul 31, 2013 at 02:25:27PM +0800, Haijun Zhang wrote: int num_ranges; + u32 ocr_mask; int i; int ret = -EINVAL; @@ -102,26 +103,11 @@ struct mmc_spi_platform_data *mmc_spi_get_pdata(struct spi_device *spi) if (!oms) return NULL; - voltage_ranges = of_get_property(np, voltage-ranges, num_ranges); - num_ranges = num_ranges / sizeof(*voltage_ranges) / 2; - if (!voltage_ranges || !num_ranges) { - dev_err(dev, OF: voltage-ranges unspecified\n); + ocr_mask = mmc_of_parse_voltage(np); + if (ocr_mask = 0) ' 0' check for an unsigned type? :) I'd write just !ocr_mask... But other than that the patch looks good to me... Reviewed-by: Anton Vorontsov an...@enomsg.org Thanks! goto err_ocr; - } - - for (i = 0; i num_ranges; i++) { - const int j = i * 2; - u32 mask; - mask = mmc_vddrange_to_ocrmask(be32_to_cpu(voltage_ranges[j]), -be32_to_cpu(voltage_ranges[j + 1])); - if (!mask) { - ret = -EINVAL; - dev_err(dev, OF: voltage-range #%d is invalid\n, i); - goto err_ocr; - } - oms-pdata.ocr_mask |= mask; - } + oms-pdata.ocr_mask |= ocr_mask; for (i = 0; i ARRAY_SIZE(oms-gpios); i++) { enum of_gpio_flags gpio_flags; -- 1.8.0 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3 V2] mmc:core: parse voltage from device-tree
On Wed, Jul 31, 2013 at 02:25:25PM +0800, Haijun Zhang wrote: Add function to support get voltage from device-tree. If there are voltage-range specified in device-tree node, this function will parse it and return the avail voltage mask. Signed-off-by: Haijun Zhang haijun.zh...@freescale.com --- changes for v2: - Update the parameters of function drivers/mmc/core/core.c | 46 ++ include/linux/mmc/core.h | 1 + 2 files changed, 47 insertions(+) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 49a5bca..ce9c957 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -27,6 +27,7 @@ #include linux/fault-inject.h #include linux/random.h #include linux/slab.h +#include linux/of.h #include linux/mmc/card.h #include linux/mmc/host.h @@ -1196,6 +1197,51 @@ u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max) } EXPORT_SYMBOL(mmc_vddrange_to_ocrmask); +#ifdef CONFIG_OF + +/* This is not kernel-doc formatted comment for the function.. it should start with /**... + * mmc_of_parse_voltage - return mask of supported voltages + * @np: The device node need to be parsed. + * + * 1. Return zero: voltage-ranges unspecified in device-tree. + * 2. Return negative errno: voltage-range is invalid. This doesn't seem right... the function returns the unsigned mask... You can change the prototype of this func to something like this: int mmc_of_parse_voltage(struct device_node *np, u32 *mask); So the function will fill the mask and return 0 on success, and will return negtive errno on errors. + * 3. Return ocr_mask: a mask of voltages that parse from device-tree + * node can be provided to MMC/SD/SDIO devices. + */ + No need for this empty line... +u32 mmc_of_parse_voltage(struct device_node *np) +{ + const u32 *voltage_ranges; + int num_ranges, i; + u32 ocr_mask = 0; + + voltage_ranges = of_get_property(np, voltage-ranges, num_ranges); + num_ranges = num_ranges / sizeof(*voltage_ranges) / 2; + if (!voltage_ranges || !num_ranges) { + pr_info(%s: voltage-ranges unspecified\n, np-full_name); + return 0; + } + + for (i = 0; i num_ranges; i++) { + const int j = i * 2; + u32 mask; + + mask = mmc_vddrange_to_ocrmask(be32_to_cpu(voltage_ranges[j]), + be32_to_cpu(voltage_ranges[j + 1])); You lost some [pretty] formatting to line up the two arguments. :) + if (!mask) { + pr_err(%s: voltage-range #%d is invalid\n, + np-full_name, i); + return -EINVAL; + } + ocr_mask |= mask; + } + + return ocr_mask; +} +EXPORT_SYMBOL(mmc_of_parse_voltage); + +#endif /* CONFIG_OF */ + #ifdef CONFIG_REGULATOR /** diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index 443243b..e3f8fe3 100644 --- a/include/linux/mmc/core.h +++ b/include/linux/mmc/core.h @@ -209,5 +209,6 @@ static inline void mmc_claim_host(struct mmc_host *host) } extern u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max); +extern u32 mmc_of_parse_voltage(struct device_node *np); You need to add a 'struct device_node;' forward-declaration, otherwise non-OF code might fail to compile... Thanks, Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file
On Mon, Jul 22, 2013 at 09:41:34PM -0500, Scott Wood wrote: [...] +static void esdhc_get_voltage(struct sdhci_host *host, +struct platform_device *pdev) +{ +} Don't duplicate this code. Move it somewhere common and share it. [Haijun Wrote:] So, move it drivers/mmc/host/sdhci-pltfm.c and share it as Sdhc_get_voltage()? I'll let the MMC maintainer say what the appropriate place would be... Don't capitalize the function name, though. :-) Somewhere in drivers/mmc/core/core.c, near mmc_vddrange_to_ocrmask() would be most appropriate, IMO. #ifdef CONFIG_OF would be needed, though. Thanks, Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2 V2] mmc: esdhc: get voltage from dts file
On Thu, Jul 25, 2013 at 08:38:11AM +0800, Haijun Zhang wrote: Add voltage-range support in esdhc of T4, So we can choose to read voltages from dts file as one optional. If we can get a valid voltage-range from device node, we use this voltage as the final voltage support. Else we still read from capacity or from other provider. Signed-off-by: Haijun Zhang haijun.zh...@freescale.com Signed-off-by: Anton Vorontsov cbouatmai...@gmail.com Development process nitpick... The code originated from me, but I did not sign off this patch... Per Documentation/SubmittingPatches: The Signed-off-by: tag indicates that the signer was involved in the development of the patch, or that he/she was in the patch's delivery path. The order of the sign-off lines also has a meaning. Putting my sign off below yours means that I was not only involved in the development of the patch but also somehow approved the patch (but I did not :). [..] +void sdhci_get_voltage(struct platform_device *pdev) You still duplicate the code... Per my previous email, this should probably go into mmc/core (with the function renamed to something more generic, of course.) Thanks, Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/4 V2] mmc: esdhc: Add quirks to support T4240QDS board
On Tue, Jul 16, 2013 at 03:11:47AM +, Zhang Haijun-B42677 wrote: Hi, Anton You mean get voltage support from DTS? Or get voltage both from DTS and host capacity register? The logic might be that you first check device-tree, if it specifies voltage ranges, assume that DTS knows better, otherwise read capabilities from the register. Anton Thanks. Regards Haijun. -Original Message- From: linux-mmc-ow...@vger.kernel.org [mailto:linux-mmc- ow...@vger.kernel.org] On Behalf Of Anton Vorontsov Sent: Saturday, July 13, 2013 2:35 AM To: Wood Scott-B07421 Cc: Zhang Haijun-B42677; linux-...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; c...@laptop.org; Fleming Andy-AFLEMING; Wrobel Heinz-R39252 Subject: Re: [PATCH 3/4 V2] mmc: esdhc: Add quirks to support T4240QDS board On Mon, Jul 08, 2013 at 12:18:39PM -0500, Scott Wood wrote: On 07/08/2013 02:16:04 AM, Haijun Zhang wrote: On T4240QDS board controllers has an unusable ADMA engine, so use SDMA instead. Also 3.0v is support on T4240QDS board even if the capacity detailed only 1.8v support. Without this quirk SD card will declare voltage not support and Signed-off-by: Haijun Zhang haijun.zh...@freescale.com ...and what? Why is this board-specific? Isn't the controller part of the SoC, not part of the board? If it really is board-specific, could you be more detailed about why (ideally with an erratum number), and check the toplevel board compatible (or get the information from platform code) rather than modify the device tree? Yup, and if anything, I would recommend to reuse voltage-ranges property, it is already implemented for mmc spi driver, drivers/mmc/host/of_mmc_spi.c Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt Thanks, Anton -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/4 V2] mmc: esdhc: Add quirks to support T4240QDS board
On Mon, Jul 08, 2013 at 12:18:39PM -0500, Scott Wood wrote: On 07/08/2013 02:16:04 AM, Haijun Zhang wrote: On T4240QDS board controllers has an unusable ADMA engine, so use SDMA instead. Also 3.0v is support on T4240QDS board even if the capacity detailed only 1.8v support. Without this quirk SD card will declare voltage not support and Signed-off-by: Haijun Zhang haijun.zh...@freescale.com ...and what? Why is this board-specific? Isn't the controller part of the SoC, not part of the board? If it really is board-specific, could you be more detailed about why (ideally with an erratum number), and check the toplevel board compatible (or get the information from platform code) rather than modify the device tree? Yup, and if anything, I would recommend to reuse voltage-ranges property, it is already implemented for mmc spi driver, drivers/mmc/host/of_mmc_spi.c Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt Thanks, Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/mpc85xx: fix non-bootcpu cannot up after hibernation resume
Hi! On Tue, May 14, 2013 at 08:59:13AM +, Wang Dongsheng-B40534 wrote: I send to a wrong email address Anton Vorontsov avoront...@ru.mvista.com Add Anton Vorontsov anton.voront...@linaro.org to this email. I don't have any means to test it, but the patch itself looks good and the description makes sense. So, Reviewed-by: Anton Vorontsov an...@enomsg.org Thanks! Thanks all. -Original Message- From: Wang Dongsheng-B40534 Sent: Tuesday, May 14, 2013 4:06 PM To: avoront...@ru.mvista.com Cc: pau...@samba.org; r...@sisk.pl; b...@kernel.crashing.org; johan...@sipsolutions.net; Wood Scott-B07421; Li Yang-R58472; Zhao Chenhui-B35336; linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534 Subject: [PATCH] powerpc/mpc85xx: fix non-bootcpu cannot up after hibernation resume This problem belongs to the core synchronization issues. The cpu1 already updated spin_table values, but bootcore cannot get this value in time. After bootcpu hibiernation restore the pages. we are now running with the kernel data of the old kernel fully restored. if we reset the non-bootcpus that will be reset cache(tlb), the non-bootcpus will get new address(map virtual and physical address spaces). but bootcpu tlb cache still use boot kernel data, so we need to invalidate the bootcpu tlb cache make it to get new main memory data. log: Enabling non-boot CPUs ... smp_85xx_kick_cpu: timeout waiting for core 1 to reset smp: failed starting cpu 1 (rc -2) Error taking CPU1 up: -2 Signed-off-by: Wang Dongsheng dongsheng.w...@freescale.com diff --git a/arch/powerpc/kernel/swsusp_booke.S b/arch/powerpc/kernel/swsusp_booke.S index 11a3930..9503249 100644 --- a/arch/powerpc/kernel/swsusp_booke.S +++ b/arch/powerpc/kernel/swsusp_booke.S @@ -141,6 +141,19 @@ _GLOBAL(swsusp_arch_resume) lis r11,swsusp_save_area@h ori r11,r11,swsusp_save_area@l + /* +* The boot core get a virtual address, when the boot process, +* the virtual address corresponds to a physical address. After +* hibernation resume memory snapshots, The corresponding +* relationship between the virtual memory and physical memory +* might change again. We need to get a new page table. So we +* need to invalidate TLB after resume pages. +* +* Invalidations TLB Using tlbilx/tlbivax/MMUCSR0. +* tlbilx used here. +*/ + bl _tlbil_all + lwz r4,SL_SPRG0(r11) mtsprg 0,r4 lwz r4,SL_SPRG1(r11) -- 1.8.0 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/esdhc: enable the card insert/remove interrupt
Hello Huang, On Fri, Oct 26, 2012 at 02:42:36AM +, Huang Changming-R66093 wrote: For the current polling mode, driver will send CMD13 to poll the card status periodically , which will cause too many interrupts. Once I sent patches to detect the card when using polling mode last year: read the state register, instead of send CMD13. But, these patches were not accepted. Now I attach them for you. Was there any specific reason why the patches didn't get accepted? I very briefly looked at them, and they seem to be OK (there are a few cosmetic details I'd comment on, tho -- but please send them in a normal way (i.e. not as attachments). Thanks, Anton. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/esdhc: enable the card insert/remove interrupt
On Thu, Oct 25, 2012 at 10:05:44AM +, Huang Changming-R66093 wrote: Hi, Anton. Could you have any comment about it? If not, I will resend this patch with v2. Technically, the patch looks fine. But again, as far as I recall, the card detection logic was broken on the SOC level (it's actually very hard to break it on the board level -- it would either work or not, but in the eSDHC case it was just unreliable, again, if I recall everything correctly -- I might be wrong). Of course you know the hardware much better, so your words weight more, so you don't need my ack on the patch. :) Although there's a second issue: for P4080DS and mpc837x boards you still have the same problem with polling method, right? It is causing performance drops/freezes every like 100 ms, and that's why you want to avoid the polling. So, you've fixed some boards, but left others to suffer. Ideally, the best fix would be to also make the card polling cheap. Anyways, using (d) clause of the Reviewer's statement of oversight, I can easily give this: Reviewed-by: Anton Vorontsov cbouatmai...@gmail.com :) Thanks! [...] IIRC, the card detection is broken SOC-revision-wise, not board-wise. So the change seems wrong. Also, take a look at this: http://lkml.org/lkml/2010/7/14/127 I started the work but never finished, unfortunately it caused some regressions for non-FSL hardware... drivers/mmc/host/sdhci-of-esdhc.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c index ffc1226..5dc362f 100644 --- a/drivers/mmc/host/sdhci-of-esdhc.c +++ b/drivers/mmc/host/sdhci-of-esdhc.c @@ -196,6 +196,11 @@ static void esdhc_of_detect_limitation(struct platform_device *pdev, if (vvn == VENDOR_V_22) pdata-quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23; + /* P4080DS and MPC837XMDS board don't support interrupt mode */ + if (of_machine_is_compatible(fsl,mpc837xmds) || + of_machine_is_compatible(fsl,P4080DS)) + pdata-quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION; + iounmap(ioaddr); end: return; @@ -223,7 +228,7 @@ static struct sdhci_pltfm_data sdhci_esdhc_pdata = { * card detection could be handled via GPIO * eSDHC cannot support End Attribute in NOP ADMA descriptor */ - .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_CARD_DETECTION + .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_NO_CARD_NO_RESET | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC, .ops = sdhci_esdhc_ops, -- 1.7.9.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/esdhc: enable the card insert/remove interrupt
On Tue, Oct 23, 2012 at 03:01:17PM +0800, r66...@freescale.com wrote: From: Jerry Huang chang-ming.hu...@freescale.com The current eSDHC driver use the poll mode to detect if the SD/MMC card is inserted or removed, which will generate many interrupts and impact the performance. Therefore, change the default card detect to interrupt mode, if the board can't support this mode, we still use the poll mode. Signed-off-by: Jerry Huang chang-ming.hu...@freescale.com CC: Anton Vorontsov cbouatmai...@gmail.com CC: Chris Ball c...@laptop.org --- IIRC, the card detection is broken SOC-revision-wise, not board-wise. So the change seems wrong. Also, take a look at this: http://lkml.org/lkml/2010/7/14/127 I started the work but never finished, unfortunately it caused some regressions for non-FSL hardware... drivers/mmc/host/sdhci-of-esdhc.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c index ffc1226..5dc362f 100644 --- a/drivers/mmc/host/sdhci-of-esdhc.c +++ b/drivers/mmc/host/sdhci-of-esdhc.c @@ -196,6 +196,11 @@ static void esdhc_of_detect_limitation(struct platform_device *pdev, if (vvn == VENDOR_V_22) pdata-quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23; + /* P4080DS and MPC837XMDS board don't support interrupt mode */ + if (of_machine_is_compatible(fsl,mpc837xmds) || + of_machine_is_compatible(fsl,P4080DS)) + pdata-quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION; + iounmap(ioaddr); end: return; @@ -223,7 +228,7 @@ static struct sdhci_pltfm_data sdhci_esdhc_pdata = { * card detection could be handled via GPIO * eSDHC cannot support End Attribute in NOP ADMA descriptor */ - .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_CARD_DETECTION + .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_NO_CARD_NO_RESET | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC, .ops = sdhci_esdhc_ops, -- 1.7.9.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
On Tue, Sep 11, 2012 at 12:54:29AM -0700, Anton Vorontsov wrote: On Tue, Sep 11, 2012 at 03:12:44PM +0800, chang-ming.hu...@freescale.com wrote: From: Jerry Huang chang-ming.hu...@freescale.com Below SOCs don't support the cmd23 command for MMC card, therefore, disable it in device tree: P1020, P1021, P1022, P1024, P1025 and P4080 Signed-off-by: Jerry Huang chang-ming.hu...@freescale.com Acked-by: Anton Vorontsov cbouatmai...@gmail.com Btw, although the patch is trivial, I guess you still want to let know PowerPC folks about it. Adding Cc and copying the patch: - - - - From: Jerry Huang chang-ming.hu...@freescale.com Below SOCs don't support the cmd23 command for MMC card, therefore, disable it in device tree: P1020, P1021, P1022, P1024, P1025 and P4080 Signed-off-by: Jerry Huang chang-ming.hu...@freescale.com CC: Anton Vorontsov cbouatmai...@gmail.com --- arch/powerpc/boot/dts/fsl/p1020si-post.dtsi |1 + arch/powerpc/boot/dts/fsl/p1021si-post.dtsi |1 + arch/powerpc/boot/dts/fsl/p1022si-post.dtsi |1 + arch/powerpc/boot/dts/fsl/p4080si-post.dtsi |1 + 4 files changed, 4 insertions(+) diff --git a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi index 68cc5e7..793a30b 100644 --- a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi @@ -154,6 +154,7 @@ sdhc@2e000 { compatible = fsl,p1020-esdhc, fsl,esdhc; sdhci,auto-cmd12; + sdhci,no-cmd23; }; /include/ pq3-sec3.3-0.dtsi diff --git a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi index adb82fd..2b7fd2a 100644 --- a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi @@ -149,6 +149,7 @@ /include/ pq3-esdhc-0.dtsi sdhc@2e000 { sdhci,auto-cmd12; + sdhci,no-cmd23; }; /include/ pq3-sec3.3-0.dtsi diff --git a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi index 06216b8..2334a52 100644 --- a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi @@ -215,6 +215,7 @@ sdhc@2e000 { compatible = fsl,p1022-esdhc, fsl,esdhc; sdhci,auto-cmd12; + sdhci,no-cmd23; }; /include/ pq3-sec3.3-0.dtsi diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi index 8d35d2c..5b39952 100644 --- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi @@ -337,6 +337,7 @@ sdhc@114000 { voltage-ranges = 3300 3300; sdhci,auto-cmd12; + sdhci,no-cmd23; }; /include/ qoriq-i2c-0.dtsi -- 1.7.9.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
On Wed, Sep 12, 2012 at 03:19:18AM +, Huang Changming-R66093 wrote: [...] I don't think it is the best way to do it. For the VVN2.2 or older, some silicon support this feature (mpc8536 and p2020), but other silicones don't support it (e.g. p4080, p102x). Though, the current p5/p4/p3 has supported this feature, can we sure the future silicon support it? So I think the best way is to specify it in device tree as 'sdhci,auto-cmd12' In addition to your current patches, you could just add another patch that blacklists affected SOC revisions based on the info from PVR/SVR. For example, see gfar_detect_errata() in drivers/net/ethernet/freescale/gianfar.c. That way you could help users that don't have the newest device trees. Thanks, Anton. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/9] cpu: Introduce clear_tasks_mm_cpumask() helper
On Thu, Apr 26, 2012 at 04:59:11PM -0700, Andrew Morton wrote: [...] so its not like new tasks will ever get this cpu set in +* their mm mask. -- Peter Zijlstra +* Thus, we may use rcu_read_lock() here, instead of grabbing +* full-fledged tasklist_lock. +*/ + rcu_read_lock(); + for_each_process(p) { + struct task_struct *t; + + t = find_lock_task_mm(p); + if (!t) + continue; + cpumask_clear_cpu(cpu, mm_cpumask(t-mm)); + task_unlock(t); + } + rcu_read_unlock(); +} It is good that this code exists under CONFIG_HOTPLUG_CPU. Did you check that everything works correctly with CONFIG_HOTPLUG_CPU=n? Yeah, only the code under CONFIG_HOTPLUG_CPU calls the function, so it should be all fine. Thanks! -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] cpu: Document clear_tasks_mm_cpumask()
This patch adds more comments on clear_tasks_mm_cpumask, plus adds a runtime check: the function is only suitable for offlined CPUs, and if called inappropriately, the kernel should scream aloud. Suggested-by: Andrew Morton a...@linux-foundation.org Suggested-by: Peter Zijlstra a.p.zijls...@chello.nl Signed-off-by: Anton Vorontsov anton.voront...@linaro.org --- On Tue, May 01, 2012 at 12:45:33PM +0200, Peter Zijlstra wrote: On Thu, 2012-04-26 at 16:59 -0700, Andrew Morton wrote: +void clear_tasks_mm_cpumask(int cpu) The operation of this function was presumably obvious to you at the time you wrote it, but that isn't true of other people at later times. Please document it? [...] If someone tries to use this function for a different purpose, or copies-and-modifies it for a different purpose, we just shot them in the foot. They'd be pretty dumb to do that without reading the local comment, but still... Methinks something simple like: WARN_ON(cpu_online(cpu)); Ought to cure that worry, no? :-) Yeah, this is all good ideas, thanks. How about the following patch? kernel/cpu.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/kernel/cpu.c b/kernel/cpu.c index ecdf499..1bfa26f 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -13,6 +13,7 @@ #include linux/oom.h #include linux/rcupdate.h #include linux/export.h +#include linux/bug.h #include linux/kthread.h #include linux/stop_machine.h #include linux/mutex.h @@ -173,6 +174,18 @@ void __ref unregister_cpu_notifier(struct notifier_block *nb) } EXPORT_SYMBOL(unregister_cpu_notifier); +/** + * clear_tasks_mm_cpumask - Safely clear tasks' mm_cpumask for a CPU + * @cpu: a CPU id + * + * This function walks up all processes, finds a valid mm struct for + * each one and then clears a corresponding bit in mm's cpumask. While + * this all sounds trivial, there are various non-obvious corner cases, + * which this function tries to solve in a safe manner. + * + * Also note that the function uses a somewhat relaxed locking scheme, + * so it may be called only for an already offlined CPU. + */ void clear_tasks_mm_cpumask(int cpu) { struct task_struct *p; @@ -184,10 +197,15 @@ void clear_tasks_mm_cpumask(int cpu) * Thus, we may use rcu_read_lock() here, instead of grabbing * full-fledged tasklist_lock. */ + WARN_ON(cpu_online(cpu)); rcu_read_lock(); for_each_process(p) { struct task_struct *t; + /* +* Main thread might exit, but other threads may still have +* a valid mm. Find one. +*/ t = find_lock_task_mm(p); if (!t) continue; -- 1.7.9.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v3 0/9] Fixes for common mistakes w/ for_each_process and task-mm
Hi all, This is another resend of several task-mm fixes, the bugs I found during LMK code audit. Architectures were traverse the tasklist in an unsafe manner, plus there are a few cases of unsafe access to task-mm in general. There were no objections on the previous resend, and the final words were somewhere along the patches are fine line. In v3: - Dropped a controversal 'Make find_lock_task_mm() sparse-aware' patch; - Reword arm and sh commit messages, per Oleg Nesterov's suggestions; - Added an optimization trick in clear_tasks_mm_cpumask(): take only the rcu read lock, no need for the whole tasklist_lock. Suggested by Peter Zijlstra. In v2: - introduced a small helper in cpu.c: most arches duplicate the same [buggy] code snippet, so it's better to fix it and move the logic into a common function. Thanks, -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/9] cpu: Introduce clear_tasks_mm_cpumask() helper
Many architectures clear tasks' mm_cpumask like this: read_lock(tasklist_lock); for_each_process(p) { if (p-mm) cpumask_clear_cpu(cpu, mm_cpumask(p-mm)); } read_unlock(tasklist_lock); Depending on the context, the code above may have several problems, such as: 1. Working with task-mm w/o getting mm or grabing the task lock is dangerous as -mm might disappear (exit_mm() assigns NULL under task_lock(), so tasklist lock is not enough). 2. Checking for process-mm is not enough because process' main thread may exit or detach its mm via use_mm(), but other threads may still have a valid mm. This patch implements a small helper function that does things correctly, i.e.: 1. We take the task's lock while whe handle its mm (we can't use get_task_mm()/mmput() pair as mmput() might sleep); 2. To catch exited main thread case, we use find_lock_task_mm(), which walks up all threads and returns an appropriate task (with task lock held). Also, Per Peter Zijlstra's idea, now we don't grab tasklist_lock in the new helper, instead we take the rcu read lock. We can do this because the function is called after the cpu is taken down and marked offline, so no new tasks will get this cpu set in their mm mask. Signed-off-by: Anton Vorontsov anton.voront...@linaro.org --- include/linux/cpu.h |1 + kernel/cpu.c| 26 ++ 2 files changed, 27 insertions(+) diff --git a/include/linux/cpu.h b/include/linux/cpu.h index ee28844..d2ca49f 100644 --- a/include/linux/cpu.h +++ b/include/linux/cpu.h @@ -179,6 +179,7 @@ extern void put_online_cpus(void); #define hotcpu_notifier(fn, pri) cpu_notifier(fn, pri) #define register_hotcpu_notifier(nb) register_cpu_notifier(nb) #define unregister_hotcpu_notifier(nb) unregister_cpu_notifier(nb) +void clear_tasks_mm_cpumask(int cpu); int cpu_down(unsigned int cpu); #ifdef CONFIG_ARCH_CPU_PROBE_RELEASE diff --git a/kernel/cpu.c b/kernel/cpu.c index 2060c6e..ecdf499 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -10,6 +10,8 @@ #include linux/sched.h #include linux/unistd.h #include linux/cpu.h +#include linux/oom.h +#include linux/rcupdate.h #include linux/export.h #include linux/kthread.h #include linux/stop_machine.h @@ -171,6 +173,30 @@ void __ref unregister_cpu_notifier(struct notifier_block *nb) } EXPORT_SYMBOL(unregister_cpu_notifier); +void clear_tasks_mm_cpumask(int cpu) +{ + struct task_struct *p; + + /* +* This function is called after the cpu is taken down and marked +* offline, so its not like new tasks will ever get this cpu set in +* their mm mask. -- Peter Zijlstra +* Thus, we may use rcu_read_lock() here, instead of grabbing +* full-fledged tasklist_lock. +*/ + rcu_read_lock(); + for_each_process(p) { + struct task_struct *t; + + t = find_lock_task_mm(p); + if (!t) + continue; + cpumask_clear_cpu(cpu, mm_cpumask(t-mm)); + task_unlock(t); + } + rcu_read_unlock(); +} + static inline void check_for_tasks(int cpu) { struct task_struct *p; -- 1.7.9.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/9] arm: Use clear_tasks_mm_cpumask()
Checking for process-mm is not enough because process' main thread may exit or detach its mm via use_mm(), but other threads may still have a valid mm. To fix this we would need to use find_lock_task_mm(), which would walk up all threads and returns an appropriate task (with task lock held). clear_tasks_mm_cpumask() has this issue fixed, so let's use it. Suggested-by: Oleg Nesterov o...@redhat.com Signed-off-by: Anton Vorontsov anton.voront...@linaro.org --- arch/arm/kernel/smp.c |8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index addbbe8..e824ee4 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -130,7 +130,6 @@ static void percpu_timer_stop(void); int __cpu_disable(void) { unsigned int cpu = smp_processor_id(); - struct task_struct *p; int ret; ret = platform_cpu_disable(cpu); @@ -160,12 +159,7 @@ int __cpu_disable(void) flush_cache_all(); local_flush_tlb_all(); - read_lock(tasklist_lock); - for_each_process(p) { - if (p-mm) - cpumask_clear_cpu(cpu, mm_cpumask(p-mm)); - } - read_unlock(tasklist_lock); + clear_tasks_mm_cpumask(cpu); return 0; } -- 1.7.9.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/9] powerpc: Use clear_tasks_mm_cpumask()
Current CPU hotplug code has some task-mm handling issues: 1. Working with task-mm w/o getting mm or grabing the task lock is dangerous as -mm might disappear (exit_mm() assigns NULL under task_lock(), so tasklist lock is not enough). We can't use get_task_mm()/mmput() pair as mmput() might sleep, so we must take the task lock while handle its mm. 2. Checking for process-mm is not enough because process' main thread may exit or detach its mm via use_mm(), but other threads may still have a valid mm. To fix this we would need to use find_lock_task_mm(), which would walk up all threads and returns an appropriate task (with task lock held). clear_tasks_mm_cpumask() has all the issues fixed, so let's use it. Suggested-by: Oleg Nesterov o...@redhat.com Signed-off-by: Anton Vorontsov anton.voront...@linaro.org --- arch/powerpc/mm/mmu_context_nohash.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c index 5b63bd3..e779642 100644 --- a/arch/powerpc/mm/mmu_context_nohash.c +++ b/arch/powerpc/mm/mmu_context_nohash.c @@ -333,9 +333,7 @@ static int __cpuinit mmu_context_cpu_notify(struct notifier_block *self, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned int)(long)hcpu; -#ifdef CONFIG_HOTPLUG_CPU - struct task_struct *p; -#endif + /* We don't touch CPU 0 map, it's allocated at aboot and kept * around forever */ @@ -358,12 +356,7 @@ static int __cpuinit mmu_context_cpu_notify(struct notifier_block *self, stale_map[cpu] = NULL; /* We also clear the cpu_vm_mask bits of CPUs going away */ - read_lock(tasklist_lock); - for_each_process(p) { - if (p-mm) - cpumask_clear_cpu(cpu, mm_cpumask(p-mm)); - } - read_unlock(tasklist_lock); + clear_tasks_mm_cpumask(cpu); break; #endif /* CONFIG_HOTPLUG_CPU */ } -- 1.7.9.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 4/9] sh: Use clear_tasks_mm_cpumask()
Checking for process-mm is not enough because process' main thread may exit or detach its mm via use_mm(), but other threads may still have a valid mm. To fix this we would need to use find_lock_task_mm(), which would walk up all threads and returns an appropriate task (with task lock held). clear_tasks_mm_cpumask() has the issue fixed, so let's use it. Suggested-by: Oleg Nesterov o...@redhat.com Signed-off-by: Anton Vorontsov anton.voront...@linaro.org --- arch/sh/kernel/smp.c |7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/arch/sh/kernel/smp.c b/arch/sh/kernel/smp.c index eaebdf6..4664f76 100644 --- a/arch/sh/kernel/smp.c +++ b/arch/sh/kernel/smp.c @@ -123,7 +123,6 @@ void native_play_dead(void) int __cpu_disable(void) { unsigned int cpu = smp_processor_id(); - struct task_struct *p; int ret; ret = mp_ops-cpu_disable(cpu); @@ -153,11 +152,7 @@ int __cpu_disable(void) flush_cache_all(); local_flush_tlb_all(); - read_lock(tasklist_lock); - for_each_process(p) - if (p-mm) - cpumask_clear_cpu(cpu, mm_cpumask(p-mm)); - read_unlock(tasklist_lock); + clear_tasks_mm_cpumask(cpu); return 0; } -- 1.7.9.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 5/9] blackfin: A couple of task-mm handling fixes
The patch fixes two problems: 1. Working with task-mm w/o getting mm or grabing the task lock is dangerous as -mm might disappear (exit_mm() assigns NULL under task_lock(), so tasklist lock is not enough). We can't use get_task_mm()/mmput() pair as mmput() might sleep, so we have to take the task lock while handle its mm. 2. Checking for process-mm is not enough because process' main thread may exit or detach its mm via use_mm(), but other threads may still have a valid mm. To catch this we use find_lock_task_mm(), which walks up all threads and returns an appropriate task (with task lock held). Suggested-by: Oleg Nesterov o...@redhat.com Signed-off-by: Anton Vorontsov anton.voront...@linaro.org --- arch/blackfin/kernel/trace.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/arch/blackfin/kernel/trace.c b/arch/blackfin/kernel/trace.c index 44bbf2f..d08f0e3 100644 --- a/arch/blackfin/kernel/trace.c +++ b/arch/blackfin/kernel/trace.c @@ -10,6 +10,8 @@ #include linux/hardirq.h #include linux/thread_info.h #include linux/mm.h +#include linux/oom.h +#include linux/sched.h #include linux/uaccess.h #include linux/module.h #include linux/kallsyms.h @@ -28,7 +30,6 @@ void decode_address(char *buf, unsigned long address) struct task_struct *p; struct mm_struct *mm; unsigned long flags, offset; - unsigned char in_atomic = (bfin_read_IPEND() 0x10) || in_atomic(); struct rb_node *n; #ifdef CONFIG_KALLSYMS @@ -114,15 +115,15 @@ void decode_address(char *buf, unsigned long address) */ write_lock_irqsave(tasklist_lock, flags); for_each_process(p) { - mm = (in_atomic ? p-mm : get_task_mm(p)); - if (!mm) - continue; + struct task_struct *t; - if (!down_read_trylock(mm-mmap_sem)) { - if (!in_atomic) - mmput(mm); + t = find_lock_task_mm(p); + if (!t) continue; - } + + mm = t-mm; + if (!down_read_trylock(mm-mmap_sem)) + goto __continue; for (n = rb_first(mm-mm_rb); n; n = rb_next(n)) { struct vm_area_struct *vma; @@ -131,7 +132,7 @@ void decode_address(char *buf, unsigned long address) if (address = vma-vm_start address vma-vm_end) { char _tmpbuf[256]; - char *name = p-comm; + char *name = t-comm; struct file *file = vma-vm_file; if (file) { @@ -164,8 +165,7 @@ void decode_address(char *buf, unsigned long address) name, vma-vm_start, vma-vm_end); up_read(mm-mmap_sem); - if (!in_atomic) - mmput(mm); + task_unlock(t); if (buf[0] == '\0') sprintf(buf, [ %s ] dynamic memory, name); @@ -175,8 +175,8 @@ void decode_address(char *buf, unsigned long address) } up_read(mm-mmap_sem); - if (!in_atomic) - mmput(mm); +__continue: + task_unlock(t); } /* -- 1.7.9.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 6/9] blackfin: Fix possible deadlock in decode_address()
Oleg Nesterov found an interesting deadlock possibility: sysrq_showregs_othercpus() does smp_call_function(showacpu) and showacpu() show_stack()-decode_address(). Now suppose that IPI interrupts the task holding read_lock(tasklist). To fix this, blackfin should not grab the write_ variant of the tasklist lock, read_ one is enough. Suggested-by: Oleg Nesterov o...@redhat.com Signed-off-by: Anton Vorontsov anton.voront...@linaro.org --- arch/blackfin/kernel/trace.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/blackfin/kernel/trace.c b/arch/blackfin/kernel/trace.c index d08f0e3..f7f7a18 100644 --- a/arch/blackfin/kernel/trace.c +++ b/arch/blackfin/kernel/trace.c @@ -29,7 +29,7 @@ void decode_address(char *buf, unsigned long address) { struct task_struct *p; struct mm_struct *mm; - unsigned long flags, offset; + unsigned long offset; struct rb_node *n; #ifdef CONFIG_KALLSYMS @@ -113,7 +113,7 @@ void decode_address(char *buf, unsigned long address) * mappings of all our processes and see if we can't be a whee * bit more specific */ - write_lock_irqsave(tasklist_lock, flags); + read_lock(tasklist_lock); for_each_process(p) { struct task_struct *t; @@ -186,7 +186,7 @@ __continue: sprintf(buf, /* kernel dynamic memory */); done: - write_unlock_irqrestore(tasklist_lock, flags); + read_unlock(tasklist_lock); } #define EXPAND_LEN ((1 CONFIG_DEBUG_BFIN_HWTRACE_EXPAND_LEN) * 256 - 1) -- 1.7.9.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 7/9] um: Should hold tasklist_lock while traversing processes
Traversing the tasks requires holding tasklist_lock, otherwise it is unsafe. p.s. However, I'm not sure that calling os_kill_ptraced_process() in the atomic context is correct. It seem to work, but please take a closer look. Signed-off-by: Anton Vorontsov anton.voront...@linaro.org --- arch/um/kernel/reboot.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/um/kernel/reboot.c b/arch/um/kernel/reboot.c index 4d93dff..66d754c 100644 --- a/arch/um/kernel/reboot.c +++ b/arch/um/kernel/reboot.c @@ -4,6 +4,7 @@ */ #include linux/sched.h +#include linux/spinlock.h #include linux/slab.h #include kern_util.h #include os.h @@ -22,6 +23,7 @@ static void kill_off_processes(void) struct task_struct *p; int pid; + read_lock(tasklist_lock); for_each_process(p) { if (p-mm == NULL) continue; @@ -29,6 +31,7 @@ static void kill_off_processes(void) pid = p-mm-context.id.u.pid; os_kill_ptraced_process(pid, 1); } + read_unlock(tasklist_lock); } } -- 1.7.9.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 8/9] um: Fix possible race on task-mm
Checking for task-mm is dangerous as -mm might disappear (exit_mm() assigns NULL under task_lock(), so tasklist lock is not enough). We can't use get_task_mm()/mmput() pair as mmput() might sleep, so let's take the task lock while we care about its mm. Note that we should also use find_lock_task_mm() to check all process' threads for a valid mm, but for uml we'll do it in a separate patch. Signed-off-by: Anton Vorontsov anton.voront...@linaro.org --- arch/um/kernel/reboot.c |7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/arch/um/kernel/reboot.c b/arch/um/kernel/reboot.c index 66d754c..1411f4e 100644 --- a/arch/um/kernel/reboot.c +++ b/arch/um/kernel/reboot.c @@ -25,10 +25,13 @@ static void kill_off_processes(void) read_lock(tasklist_lock); for_each_process(p) { - if (p-mm == NULL) + task_lock(p); + if (!p-mm) { + task_unlock(p); continue; - + } pid = p-mm-context.id.u.pid; + task_unlock(p); os_kill_ptraced_process(pid, 1); } read_unlock(tasklist_lock); -- 1.7.9.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 9/9] um: Properly check all process' threads for a live mm
kill_off_processes() might miss a valid process, this is because checking for process-mm is not enough. Process' main thread may exit or detach its mm via use_mm(), but other threads may still have a valid mm. To catch this we use find_lock_task_mm(), which walks up all threads and returns an appropriate task (with task lock held). Suggested-by: Oleg Nesterov o...@redhat.com Signed-off-by: Anton Vorontsov anton.voront...@linaro.org --- arch/um/kernel/reboot.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/arch/um/kernel/reboot.c b/arch/um/kernel/reboot.c index 1411f4e..3d15243 100644 --- a/arch/um/kernel/reboot.c +++ b/arch/um/kernel/reboot.c @@ -6,6 +6,7 @@ #include linux/sched.h #include linux/spinlock.h #include linux/slab.h +#include linux/oom.h #include kern_util.h #include os.h #include skas.h @@ -25,13 +26,13 @@ static void kill_off_processes(void) read_lock(tasklist_lock); for_each_process(p) { - task_lock(p); - if (!p-mm) { - task_unlock(p); + struct task_struct *t; + + t = find_lock_task_mm(p); + if (!t) continue; - } - pid = p-mm-context.id.u.pid; - task_unlock(p); + pid = t-mm-context.id.u.pid; + task_unlock(t); os_kill_ptraced_process(pid, 1); } read_unlock(tasklist_lock); -- 1.7.9.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 7/9] um: Should hold tasklist_lock while traversing processes
On Mon, Apr 23, 2012 at 04:57:54PM +0200, Richard Weinberger wrote: On 23.04.2012 09:09, Anton Vorontsov wrote: Traversing the tasks requires holding tasklist_lock, otherwise it is unsafe. p.s. However, I'm not sure that calling os_kill_ptraced_process() in the atomic context is correct. It seem to work, but please take a closer look. Signed-off-by: Anton Vorontsovanton.voront...@linaro.org --- You forgot my Ack and I've already explained why os_kill_ptraced_process() is fine. Ouch, sorry! -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 01/10] cpu: Introduce clear_tasks_mm_cpumask() helper
Many architctures clear tasks' mm_cpumask like this: read_lock(tasklist_lock); for_each_process(p) { if (p-mm) cpumask_clear_cpu(cpu, mm_cpumask(p-mm)); } read_unlock(tasklist_lock); The code above has several problems, such as: 1. Working with task-mm w/o getting mm or grabing the task lock is dangerous as -mm might disappear (exit_mm() assigns NULL under task_lock(), so tasklist lock is not enough). 2. Checking for process-mm is not enough because process' main thread may exit or detach its mm via use_mm(), but other threads may still have a valid mm. This patch implements a small helper function that does things correctly, i.e.: 1. We take the task's lock while whe handle its mm (we can't use get_task_mm()/mmput() pair as mmput() might sleep); 2. To catch exited main thread case, we use find_lock_task_mm(), which walks up all threads and returns an appropriate task (with task lock held). Signed-off-by: Anton Vorontsov anton.voront...@linaro.org --- include/linux/cpu.h |1 + kernel/cpu.c| 18 ++ 2 files changed, 19 insertions(+) diff --git a/include/linux/cpu.h b/include/linux/cpu.h index 1f65875..941e865 100644 --- a/include/linux/cpu.h +++ b/include/linux/cpu.h @@ -171,6 +171,7 @@ extern void put_online_cpus(void); #define hotcpu_notifier(fn, pri) cpu_notifier(fn, pri) #define register_hotcpu_notifier(nb) register_cpu_notifier(nb) #define unregister_hotcpu_notifier(nb) unregister_cpu_notifier(nb) +void clear_tasks_mm_cpumask(int cpu); int cpu_down(unsigned int cpu); #ifdef CONFIG_ARCH_CPU_PROBE_RELEASE diff --git a/kernel/cpu.c b/kernel/cpu.c index 2060c6e..5255936 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -10,6 +10,7 @@ #include linux/sched.h #include linux/unistd.h #include linux/cpu.h +#include linux/oom.h #include linux/export.h #include linux/kthread.h #include linux/stop_machine.h @@ -171,6 +172,23 @@ void __ref unregister_cpu_notifier(struct notifier_block *nb) } EXPORT_SYMBOL(unregister_cpu_notifier); +void clear_tasks_mm_cpumask(int cpu) +{ + struct task_struct *p; + + read_lock(tasklist_lock); + for_each_process(p) { + struct task_struct *t; + + t = find_lock_task_mm(p); + if (!t) + continue; + cpumask_clear_cpu(cpu, mm_cpumask(t-mm)); + task_unlock(t); + } + read_unlock(tasklist_lock); +} + static inline void check_for_tasks(int cpu) { struct task_struct *p; -- 1.7.9.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit
On Fri, Sep 09, 2011 at 08:05:46PM +0800, Roy Zang wrote: From: Xu lei b33...@freescale.com Freescale eSDHC registers only support 32-bit accesses, this patch ensures that all Freescale eSDHC register accesses are 32-bit. Signed-off-by: Xu lei b33...@freescale.com Signed-off-by: Roy Zang tie-fei.z...@freescale.com Signed-off-by: Kumar Gala ga...@kernel.crashing.org --- The patch looks OK. Acked-by: Anton Vorontsov cbouatmai...@gmail.com [...] +static u8 esdhc_readb(struct sdhci_host *host, int reg) +{ + int base = reg ~0x3; + int shift = (reg 0x3) * 8; + u8 ret = (in_be32(host-ioaddr + base) shift) 0xff; return ret; } Though, I wonder if we could change sdhci_be32bs_read{b,w}, instead of making this local to eSDHC. The thing is: sdhci_be32bs_writeb() is using clrsetbits_be32, so the write variant already uses 32-bit accessors, so nothing should break if we switch sdhci_be32bs_readb() to in_be32(). But maybe it's safer if we do this in a separate patch, so that it could be easily reverted without impacting eSDHC if something actually breaks. You decide. :-) Thanks! -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] usb: Allocate pram dynamically.
On Tue, Aug 23, 2011 at 02:38:41PM +0200, Joakim Tjernlund wrote: MPC832x does not have enough MURAM to do fixed MURAM allocation. Change to dynamic allocation. Signed-off-by: Joakim Tjernlund joakim.tjernl...@transmode.se Acked-by: Anton Vorontsov cbouatmai...@gmail.com Thanks! p.s. You probably want to send this to Greg KH, + Cc linux-usb mailing list. -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2 v2] eSDHC: Fix errors when booting kernel with fsl esdhc
Hello, On Fri, Aug 12, 2011 at 09:44:26AM +, Zang Roy-R61911 wrote: [...] We try to not pollute generic sdhci.c driver with chip-specific quirks. Maybe you can do the fixups via IO accessors? Or by introducing some additional sdhci op? Anton, thanks for the comment, as we discussed, the original code use 8 bit byte operation, while in fact, on some powerpc platform, 32 bit operation is needed. should it be possible fixed by adding some wrapper in IO accessors or introduce additional sdhci op? I would do it in the IO accessors. Thanks, -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2 v2] eSDHC: Fix errors when booting kernel with fsl esdhc
On Fri, Jul 22, 2011 at 06:15:17PM +0800, Roy Zang wrote: [...] if (host-version = SDHCI_SPEC_200) { - ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL); - ctrl = ~SDHCI_CTRL_DMA_MASK; - if ((host-flags SDHCI_REQ_USE_DMA) - (host-flags SDHCI_USE_ADMA)) - ctrl |= SDHCI_CTRL_ADMA32; - else - ctrl |= SDHCI_CTRL_SDMA; - sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); + if (host-quirks SDHCI_QUIRK_QORIQ_PROCTL_WEIRD) { +#define ESDHCI_PROCTL_DMAS_MASK 0x0300 +#define ESDHCI_PROCTL_ADMA32 0x0200 +#define ESDHCI_PROCTL_SDMA 0x + ctrl = sdhci_readl(host, SDHCI_HOST_CONTROL); + ctrl = ~ESDHCI_PROCTL_DMAS_MASK; + if ((host-flags SDHCI_REQ_USE_DMA) + (host-flags SDHCI_USE_ADMA)) + ctrl |= ESDHCI_PROCTL_ADMA32; + else + ctrl |= ESDHCI_PROCTL_SDMA; + sdhci_writel(host, ctrl, SDHCI_HOST_CONTROL); + } else { + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL); + ctrl = ~SDHCI_CTRL_DMA_MASK; + if ((host-flags SDHCI_REQ_USE_DMA) + (host-flags SDHCI_USE_ADMA)) + ctrl |= SDHCI_CTRL_ADMA32; + else + ctrl |= SDHCI_CTRL_SDMA; + sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); We try to not pollute generic sdhci.c driver with chip-specific quirks. Maybe you can do the fixups via IO accessors? Or by introducing some additional sdhci op? [...] if (power != (unsigned short)-1) { switch (1 power) { +#define ESDHCI_FSL_POWER_MASK 0x40 +#define ESDHCI_FSL_POWER_1800x00 +#define ESDHCI_FSL_POWER_3000x40 Same here. The driver will rot quickly if everyone would start putting chip-specific quirks into sdhci.c. Please don't. Thanks, -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] eSDHC: fix incorrect default value of the capabilities register on P4080
On Tue, Jul 05, 2011 at 12:19:03PM +0800, Roy Zang wrote: P4080 eSDHC errata 12 describes incorrect default value of the the host controller capabilities register. The default value of the VS18 and VS30 fields in the host controller capabilities register (HOSTCAPBLT) are incorrect. The default of these bits should be zero instead of one in the eSDHC logic. This patch adds the workaround for these errata. Signed-off-by: Roy Zang tie-fei.z...@freescale.com --- drivers/mmc/host/sdhci-of-core.c |3 +++ drivers/mmc/host/sdhci.c |6 ++ include/linux/mmc/sdhci.h|4 3 files changed, 13 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c index fede43d..9bdd30d 100644 --- a/drivers/mmc/host/sdhci-of-core.c +++ b/drivers/mmc/host/sdhci-of-core.c @@ -182,6 +182,9 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev) if (of_device_is_compatible(np, fsl,esdhc)) host-quirks |= SDHCI_QUIRK_QORIQ_PROCTL_WEIRD; + if (of_device_is_compatible(np, fsl,p4080-esdhc)) + host-quirks |= SDHCI_QUIRK_QORIQ_HOSTCAPBLT_ONLY_VS33; Should really use voltage-ranges, not quirks. http://www.spinics.net/lists/linux-mmc/msg02785.html Thanks, -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] ppc/85xx: create a platform node for PCI EDAC device
On Thu, Jun 02, 2011 at 04:25:02PM +0400, Dmitry Eremin-Solenikov wrote: As a device for pci node isn't created, create a special platform_device for PCI EDAC device on MPC85xx. Signed-off-by: Dmitry Eremin-Solenikov dbarysh...@gmail.com --- arch/powerpc/sysdev/fsl_pci.c | 33 + 1 files changed, 33 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c index 68ca929..0e37259 100644 --- a/arch/powerpc/sysdev/fsl_pci.c +++ b/arch/powerpc/sysdev/fsl_pci.c @@ -381,6 +381,39 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary) return 0; } +int __init fsl_add_pci_err(void) static :-) +{ + struct device_node *np; + + for_each_node_by_type(np, pci) { + /* Only PCI, not PCI Express! */ + if (of_device_is_compatible(np, fsl,mpc8540-pci)) { + struct resource r[2]; How about '= {};' initializer instead of the '= NULL's down below? + + r[0].parent = NULL; + r[1].parent = NULL; Thanks, -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] ppc/85xx: create a platform node for PCI EDAC device
On Wed, Jun 01, 2011 at 04:28:11PM +0400, Dmitry Eremin-Solenikov wrote: [...] --- a/arch/powerpc/sysdev/fsl_pci.h +++ b/arch/powerpc/sysdev/fsl_pci.h @@ -92,6 +92,7 @@ extern int fsl_add_bridge(struct device_node *dev, int is_primary); extern void fsl_pcibios_fixup_bus(struct pci_bus *bus); extern int mpc83xx_add_bridge(struct device_node *dev); u64 fsl_pci_immrbar_base(struct pci_controller *hose); +int fsl_add_pci_err(void); With #ifdef CONFIG_PCI int fsl_add_pci_err(void); #else static inline int fsl_add_pci_err(void) { return -ENODEV; } #endif You won't need endless ifdefs in the board files: #ifdef CONFIG_PCI fsl_add_pci_err(); #endif Also, why not add this call to the fsl_add_bridge(), so you won't need to touch board files at all. Thanks, -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] ppc/85xx: create a platform node for PCI EDAC device
On Wed, Jun 01, 2011 at 06:55:35PM +0400, Dmitry Eremin-Solenikov wrote: On 6/1/11, Anton Vorontsov avoront...@mvista.com wrote: On Wed, Jun 01, 2011 at 04:28:11PM +0400, Dmitry Eremin-Solenikov wrote: [...] --- a/arch/powerpc/sysdev/fsl_pci.h +++ b/arch/powerpc/sysdev/fsl_pci.h @@ -92,6 +92,7 @@ extern int fsl_add_bridge(struct device_node *dev, int is_primary); extern void fsl_pcibios_fixup_bus(struct pci_bus *bus); extern int mpc83xx_add_bridge(struct device_node *dev); u64 fsl_pci_immrbar_base(struct pci_controller *hose); +int fsl_add_pci_err(void); With #ifdef CONFIG_PCI int fsl_add_pci_err(void); #else static inline int fsl_add_pci_err(void) { return -ENODEV; } #endif You won't need endless ifdefs in the board files: OK, will redo this patch. Btw, if you don't check return value of fsl_add_pci_err(), then it would probably make sense to return void. And if you do check it somewhere, be sure to include linux/errno.h for -ENODEV. :-) Thanks, -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [rtc-linux] Re: [PATCH] Add support for pt7c4338 (rtc device) in rtc-ds1307 driver
On Mon, May 30, 2011 at 02:29:58PM +, Tabi Timur-B04825 wrote: On Mon, May 30, 2011 at 3:24 AM, Wolfram Sang w.s...@pengutronix.de wrote: The first place where this should be mentioned is the datasheet of the pt-chip, so you might ask the producer to add this information (don't expect much to happen, though). It's true that the data sheet does not mention that it's identical to the DS1307, but that's still no excuse for not noticing it and writing a whole driver for it. :-( IIRC I asked you explicitly for the differences between the chips. If there are none, you can use the driver directly, right? :) Yes. The device tree node for the PT7C4338 device should just say /* The board has a PT7C4338, which is compatible with the DS1307 */ compatible = dallas,ds1307; While it seems to be 100% compatible, there could be chip-specific bugs or some interesting features that are hidden behind reserved bits and registers. So I think device tree should not lie about the chip model. Doing 'compatible = pericom,pt7c4338, dallas,ds1307' is perfectly fine. Note that today the several compatible entries approach gives you almost nothing, as you will need to add pt7c4338 entry into the driver anyway. I tried to improve this, i.e. make linux do OF-matching on the most generic compatible entry (the last one): http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg21196.html It was received coldly though: http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg22041.html http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg21273.html -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 5/7] mmc: sdhci: consolidate sdhci-of-esdhc and sdhci-esdhc-imx
On Thu, May 05, 2011 at 09:22:56PM +0800, Shawn Guo wrote: This patch is to consolidate SDHCI driver for Freescale eSDHC controller found on both MPCxxx and i.MX platforms. It merges sdhci-of-esdhc.c into sdhci-esdhc.c, so that the same pair of .probe/.remove hook works with eSDHC for two platforms. As the results, sdhci-of-esdhc.c and sdhci-esdhc.h are removed, and header esdhc.h containing the definition of esdhc_platform_data is put into the public folder. Signed-off-by: Shawn Guo shawn@linaro.org I'm not sure if that makes sense to merge these two. I'd rather vote for renaming sdhci-of-esdhc to sdhci-esdhc-mpc. :-) -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 3/7] mmc: sdhci: make sdhci-of device drivers self registered
On Thu, May 05, 2011 at 09:22:54PM +0800, Shawn Guo wrote: [...] - * Copyright (c) 2007 Freescale Semiconductor, Inc. - * Copyright (c) 2009 MontaVista Software, Inc. - * - * Authors: Xiaobo Xie x@freescale.com - * Anton Vorontsov avoront...@ru.mvista.com [...] -#ifdef CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER - -/* - * These accessors are designed for big endian hosts doing I/O to - * little endian controllers incorporating a 32-bit hardware byte swapper. - */ - -u32 sdhci_be32bs_readl(struct sdhci_host *host, int reg) -{ - return in_be32(host-ioaddr + reg); -} - -u16 sdhci_be32bs_readw(struct sdhci_host *host, int reg) -{ - return in_be16(host-ioaddr + (reg ^ 0x2)); -} - -u8 sdhci_be32bs_readb(struct sdhci_host *host, int reg) -{ - return in_8(host-ioaddr + (reg ^ 0x3)); -} - -void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg) -{ - out_be32(host-ioaddr + reg, val); -} - -void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg) -{ - struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); - int base = reg ~0x3; - int shift = (reg 0x2) * 8; - - switch (reg) { - case SDHCI_TRANSFER_MODE: - /* - * Postpone this write, we must do it together with a - * command write that is down below. - */ - pltfm_host-xfer_mode_shadow = val; - return; - case SDHCI_COMMAND: - sdhci_be32bs_writel(host, val 16 | pltfm_host-xfer_mode_shadow, - SDHCI_TRANSFER_MODE); - return; - } - clrsetbits_be32(host-ioaddr + base, 0x shift, val shift); -} - -void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg) -{ - int base = reg ~0x3; - int shift = (reg 0x3) * 8; - - clrsetbits_be32(host-ioaddr + base , 0xff shift, val shift); -} -#endif /* CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER */ [...] +#ifdef CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER +/* + * These accessors are designed for big endian hosts doing I/O to + * little endian controllers incorporating a 32-bit hardware byte swapper. + */ +u32 sdhci_be32bs_readl(struct sdhci_host *host, int reg) +{ + return in_be32(host-ioaddr + reg); +} + +u16 sdhci_be32bs_readw(struct sdhci_host *host, int reg) +{ + return in_be16(host-ioaddr + (reg ^ 0x2)); +} + +u8 sdhci_be32bs_readb(struct sdhci_host *host, int reg) +{ + return in_8(host-ioaddr + (reg ^ 0x3)); +} + +void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg) +{ + out_be32(host-ioaddr + reg, val); +} + +void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg) +{ + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + int base = reg ~0x3; + int shift = (reg 0x2) * 8; + + switch (reg) { + case SDHCI_TRANSFER_MODE: + /* + * Postpone this write, we must do it together with a + * command write that is down below. + */ + pltfm_host-xfer_mode_shadow = val; + return; + case SDHCI_COMMAND: + sdhci_be32bs_writel(host, val 16 | pltfm_host-xfer_mode_shadow, + SDHCI_TRANSFER_MODE); + return; + } + clrsetbits_be32(host-ioaddr + base, 0x shift, val shift); +} + +void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg) +{ + int base = reg ~0x3; + int shift = (reg 0x3) * 8; + + clrsetbits_be32(host-ioaddr + base , 0xff shift, val shift); +} +#endif /* CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER */ I noticed you're very careful wrt copyright/authorship notices, except for this case. How about retaining copyright stuff when you merge these two files? The patch itself looks great though. Acked-by: Anton Vorontsov cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 2/7] mmc: sdhci: eliminate sdhci_of_host and sdhci_of_data
On Thu, May 05, 2011 at 09:22:53PM +0800, Shawn Guo wrote: The patch migrates the use of sdhci_of_host and sdhci_of_data to sdhci_pltfm_host and sdhci_pltfm_data, so that the former pair can be eliminated. Signed-off-by: Shawn Guo shawn@linaro.org Reviewed-by: Grant Likely grant.lik...@secretlab.ca Acked-by: Anton Vorontsov cbouatmai...@gmail.com Thanks! -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/7] mmc: sdhci: make sdhci-pltfm device drivers self registered
On Thu, May 05, 2011 at 09:22:52PM +0800, Shawn Guo wrote: [...] +static int __devinit sdhci_cns3xxx_probe(struct platform_device *pdev) +{ + return sdhci_pltfm_register(pdev, sdhci_cns3xxx_pdata); +} + +static int __devexit sdhci_cns3xxx_remove(struct platform_device *pdev) +{ + return sdhci_pltfm_unregister(pdev); +} + +static struct platform_driver sdhci_cns3xxx_driver = { + .driver = { + .name = sdhci-cns3xxx, + .owner = THIS_MODULE, + }, + .probe = sdhci_cns3xxx_probe, + .remove = __devexit_p(sdhci_cns3xxx_remove), +#ifdef CONFIG_PM + .suspend= sdhci_pltfm_suspend, + .resume = sdhci_pltfm_resume, +#endif +}; + +static int __init sdhci_cns3xxx_init(void) +{ + return platform_driver_register(sdhci_cns3xxx_driver); +} +module_init(sdhci_cns3xxx_init); + +static void __exit sdhci_cns3xxx_exit(void) +{ + platform_driver_unregister(sdhci_cns3xxx_driver); +} +module_exit(sdhci_cns3xxx_exit); I don't think I like this duplicate code for each platform sub- driver. It's repetitive and annoying. :-/ But considering that ARM will be multiplatform soon, we don't want to have every mach-* stuff in the single sdhci-pltfm. So... OK. If that compiles, I'm fine with it. :-D Acked-by: Anton Vorontsov cbouatmai...@gmail.com Thanks! -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] gianfar: Fall back to software tcp/udp checksum on older controllers
Hello Alex, On Thu, Jan 27, 2011 at 12:14:21AM -0800, Alex Dubov wrote: As specified by errata eTSEC49 of MPC8548 and errata eTSEC12 of MPC83xx, older revisions of gianfar controllers will be unable to calculate a TCP/UDP packet checksum for some aligments of the appropriate FCB. This patch checks for FCB alignment on such controllers and falls back to software checksumming if the aligment is known to be bad. Signed-off-by: Alex Dubov oa...@yahoo.com --- This is my, somewhat different approach to Matthew Creech proposed solution. drivers/net/gianfar.c | 21 +++-- drivers/net/gianfar.h |1 + 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c index 5ed8f9f..b4f0e99 100644 --- a/drivers/net/gianfar.c +++ b/drivers/net/gianfar.c @@ -950,6 +950,11 @@ static void gfar_detect_errata(struct gfar_private *priv) (pvr == 0x80861010 (mod 0xfff9) == 0x80c0)) priv-errata |= GFAR_ERRATA_A002; + /* MPC8313 Rev 2.0, MPC8548 rev 2.0 */ + if ((pvr == 0x80850010 mod == 0x80b0 rev 0x0020) + || (pvr == 0x80210020 mod == 0x8030 rev == 0x0020)) Please indent it like the above: with two tabs. This is to keep things consistent. + priv-errata |= GFAR_ERRATA_12; + if (priv-errata) dev_info(dev, enabled errata workarounds, flags: 0x%x\n, priv-errata); @@ -2156,8 +2161,20 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev) /* Set up checksumming */ if (CHECKSUM_PARTIAL == skb-ip_summed) { fcb = gfar_add_fcb(skb); - lstatus |= BD_LFLAG(TXBD_TOE); - gfar_tx_checksum(skb, fcb); + switch (unlikely(gfar_has_errata(priv, GFAR_ERRATA_12)) + ? 1 : 0) { The switch construction is quite bizarre. ;-) Why not if (gfar_has_errata() (ulong)fcb % 0x20 18) { csum_help(); } else { lstatus |=... tx_csum(); } ? Thanks, + case 1: + /* as specified by errata */ + if (((unsigned long)fcb % 0x20) 0x18) { + __skb_pull(skb, GMAC_FCB_LEN); + skb_checksum_help(skb); + break; + } + /* otherwise, fall through */ + default: + lstatus |= BD_LFLAG(TXBD_TOE); + gfar_tx_checksum(skb, fcb); + } } -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of_mmc_spi: add card detect irq support
On Mon, Aug 30, 2010 at 11:49:08AM -0600, Grant Likely wrote: On Mon, Aug 30, 2010 at 10:38 AM, David Brownell davi...@pacbell.net wrote: Since I don't do OpenFirmware, let's hear from Grant on this one. Looks good to me. Acked-by: Grant Likely grant.lik...@secretlab.ca I wonder what happened with this patch? Thanks, -- Anton Vorontsov Email: cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts
On Tue, Nov 16, 2010 at 10:45:37PM +0100, Wolfram Sang wrote: Even worse, I seem to recall that I had once seen a manufacturer increasing the page-size from one charge to the next without changing the part-number, so I got this feeling you can't map pagesize to manufacturer/type which I still have. Sadly, this was long ago, so I can't proof it right now. Will try to dig up some datasheets when in the office tomorrow. Had a look, couldn't find anything :( And now? Well, it seems that there are enough people in pagesize camp anyway, I'd say just go ahead with the current approach. :-) It's an additional information, so won't do any harm anyway... Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts
On Mon, Nov 15, 2010 at 06:25:16PM +0100, Wolfram Sang wrote: Signed-off-by: Wolfram Sang w.s...@pengutronix.de --- arch/powerpc/boot/dts/pcm030.dts |1 + arch/powerpc/boot/dts/pcm032.dts |3 ++- 2 files changed, 3 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts index 8a4ec30..e7c36bc 100644 --- a/arch/powerpc/boot/dts/pcm030.dts +++ b/arch/powerpc/boot/dts/pcm030.dts @@ -259,6 +259,7 @@ eep...@52 { compatible = catalyst,24c32; reg = 0x52; + pagesize = 32; I think you'd better drop the pagesize property altogether, and instead make the compatible string more specific (if needed at all. are there any 'catalyst,24c32' chips with pagesize != 32?) Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] ucc_geth: Do not bring the whole IF down when TX failure.
On Fri, Nov 12, 2010 at 02:55:08PM +0100, Joakim Tjernlund wrote: ucc_geth_close lacks a cancel_work_sync(ugeth-timeout_work) to stop any outstanding processing of TX fail. However, one can not call cancel_work_sync without fixing the timeout function otherwise it will deadlock. This patch brings ucc_geth in line with gianfar: Don't bring the interface down and up, just reinit controller HW and PHY. Signed-off-by: Joakim Tjernlund joakim.tjernl...@transmode.se Looks sane, thanks! Reviewed-by: Anton Vorontsov cbouatmai...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] ucc_geth: Fix deadlock
On Fri, Nov 12, 2010 at 02:55:09PM +0100, Joakim Tjernlund wrote: This script: while [ 1==1 ] ; do ifconfig eth0 up; usleep 195 ;ifconfig eth0 down; dmesg -c ;done causes in just a second or two: INFO: task ifconfig:572 blocked for more than 120 seconds. [...] The reason appears to be ucc_geth_stop meets adjust_link as the PHY reports PHY changes. I belive adjust_link hangs somewhere, holding the PHY lock, because ucc_geth_stop disabled the controller HW. Fix is to stop the PHY before disabling the controller. Signed-off-by: Joakim Tjernlund joakim.tjernl...@transmode.se It's unclear where exactly adjust_link() hangs, but the patch looks as the right thing overall. Thanks! Reviewed-by: Anton Vorontsov cbouatmai...@gmail.com --- drivers/net/ucc_geth.c | 10 +++--- 1 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c index 6c254ed..06a5db3 100644 --- a/drivers/net/ucc_geth.c +++ b/drivers/net/ucc_geth.c @@ -2050,12 +2050,16 @@ static void ucc_geth_stop(struct ucc_geth_private *ugeth) ugeth_vdbg(%s: IN, __func__); + /* + * Tell the kernel the link is down. + * Must be done before disabling the controller + * or deadlock may happen. + */ + phy_stop(phydev); + /* Disable the controller */ ugeth_disable(ugeth, COMM_DIR_RX_AND_TX); - /* Tell the kernel the link is down */ - phy_stop(phydev); - /* Mask all interrupts */ out_be32(ugeth-uccf-p_uccm, 0x); ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] edac: mpc85xx: Add support for new MPCxxx/Pxxxx EDAC controllers (fix)
On Thu, Oct 07, 2010 at 01:18:19AM -0500, Kumar Gala wrote: [...] diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c index cfa86f7..b178cfa 100644 --- a/drivers/edac/mpc85xx_edac.c +++ b/drivers/edac/mpc85xx_edac.c @@ -652,7 +652,6 @@ static struct of_device_id mpc85xx_l2_err_of_match[] = { { .compatible = fsl,p1020-l2-cache-controller, }, { .compatible = fsl,p1021-l2-cache-controller, }, { .compatible = fsl,p2020-l2-cache-controller, }, - { .compatible = fsl,p4080-l2-cache-controller, }, {}, }; MODULE_DEVICE_TABLE(of, mpc85xx_l2_err_of_match); -- 1.7.0.5 Can you post a new patch as it doesn't look like this got merged by Andrew so we need to clean up after ourselves. It's already in Linus' tree. Thanks, - - - - commit cd1542c8197fc3c2eb3a8301505d5d9738fab1e4 Author: Anton Vorontsov avoront...@mvista.com Date: Tue Aug 10 18:03:21 2010 -0700 edac: mpc85xx: add support for new MPCxxx/P EDAC controllers Simply add proper IDs into the device table. Signed-off-by: Anton Vorontsov avoront...@mvista.com Cc: Scott Wood scottw...@freescale.com Cc: Peter Tyser pty...@xes-inc.com Cc: Dave Jiang dji...@mvista.com Cc: Doug Thompson dougthomp...@xmission.com Signed-off-by: Andrew Morton a...@linux-foundation.org Signed-off-by: Linus Torvalds torva...@linux-foundation.org diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c index fdbad55..af75e27 100644 --- a/drivers/edac/mpc85xx_edac.c +++ b/drivers/edac/mpc85xx_edac.c @@ -647,7 +647,10 @@ static struct of_device_id mpc85xx_l2_err_of_match[] = { { .compatible = fsl,mpc8555-l2-cache-controller, }, { .compatible = fsl,mpc8560-l2-cache-controller, }, { .compatible = fsl,mpc8568-l2-cache-controller, }, + { .compatible = fsl,mpc8569-l2-cache-controller, }, { .compatible = fsl,mpc8572-l2-cache-controller, }, + { .compatible = fsl,p1020-l2-cache-controller, }, + { .compatible = fsl,p1021-l2-cache-controller, }, { .compatible = fsl,p2020-l2-cache-controller, }, {}, }; @@ -1125,7 +1128,10 @@ static struct of_device_id mpc85xx_mc_err_of_match[] = { { .compatible = fsl,mpc8569-memory-controller, }, { .compatible = fsl,mpc8572-memory-controller, }, { .compatible = fsl,mpc8349-memory-controller, }, + { .compatible = fsl,p1020-memory-controller, }, + { .compatible = fsl,p1021-memory-controller, }, { .compatible = fsl,p2020-memory-controller, }, + { .compatible = fsl,p4080-memory-controller, }, {}, }; MODULE_DEVICE_TABLE(of, mpc85xx_mc_err_of_match); ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] edac: mpc85xx: Add support for new MPCxxx/Pxxxx EDAC controllers (fix)
On Thu, Oct 07, 2010 at 02:00:50AM -0500, Kumar Gala wrote: On Oct 7, 2010, at 1:37 AM, Kumar Gala wrote: @ -1125,7 +1128,10 @@ static struct of_device_id mpc85xx_mc_err_of_match[] = { { .compatible = fsl,mpc8569-memory-controller, }, { .compatible = fsl,mpc8572-memory-controller, }, { .compatible = fsl,mpc8349-memory-controller, }, + { .compatible = fsl,p1020-memory-controller, }, + { .compatible = fsl,p1021-memory-controller, }, { .compatible = fsl,p2020-memory-controller, }, + { .compatible = fsl,p4080-memory-controller, }, This line should be here ;) should NOT be here. Hm. Are you sure? I thought that only L2 cache controller is not applicable (and based on Scott's comment I removed the l2 cache compatible entry for p4080). But I guess memory-controller is somewhat similar to all other 85xx? If it's not, I can surely prepare a patch that removes p4080 entry. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 3/7] eSPI: add eSPI controller support
);' is sufficient. Also, I think there's no need for this wrapper nowadays (but splitting OF and real probe() stuff is still appropriate). +} + +static const struct of_device_id of_fsl_espi_match[] = { + { .compatible = fsl,mpc8536-espi }, + {} +}; +MODULE_DEVICE_TABLE(of, of_fsl_espi_match); + +static struct of_platform_driver fsl_espi_driver = { + .driver = { + .name = fsl_espi, + .owner = THIS_MODULE, + .of_match_table = of_fsl_espi_match, + }, + .probe = of_fsl_espi_probe, + .remove = __devexit_p(of_fsl_espi_remove), +}; + +static int __init fsl_espi_init(void) +{ + return of_register_platform_driver(fsl_espi_driver); +} +module_init(fsl_espi_init); + +static void __exit fsl_espi_exit(void) +{ + of_unregister_platform_driver(fsl_espi_driver); +} +module_exit(fsl_espi_exit); + +MODULE_AUTHOR(Mingkai Hu); +MODULE_DESCRIPTION(Enhanced Freescale SPI Driver); This sounds like that this is an enhanced version of the Freescale SPI driver, which it is not. ;-) +MODULE_LICENSE(GPL); diff --git a/drivers/spi/spi_fsl_lib.h b/drivers/spi/spi_fsl_lib.h index 6ae8949..9c81498 100644 --- a/drivers/spi/spi_fsl_lib.h +++ b/drivers/spi/spi_fsl_lib.h @@ -26,6 +26,7 @@ struct mpc8xxx_spi { /* rx tx bufs from the spi_transfer */ const void *tx; void *rx; + int len; I'd place the #ifdef CONFIG_SPI_ESPI, for documentation purposes. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 2/7] spi/mpc8xxx: refactor the common code for SPI/eSPI controller
On Thu, Sep 30, 2010 at 04:00:41PM +0800, Mingkai Hu wrote: [...] -static void mpc8xxx_spi_change_mode(struct spi_device *spi) +static void fsl_spi_change_mode(struct spi_device *spi) { struct mpc8xxx_spi *mspi = spi_master_get_devdata(spi-master); struct spi_mpc8xxx_cs *cs = spi-controller_state; - __be32 __iomem *mode = mspi-base-mode; + struct fsl_spi_reg *reg_base = (struct fsl_spi_reg *)mspi-reg_base; No need for these type casts (the same is for the whole patch). -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 6/7] mtd: m25p80: add a read function to read page by page
On Thu, Sep 30, 2010 at 11:41:40PM +0900, Grant Likely wrote: On Thu, Sep 30, 2010 at 11:16 PM, Grant Likely grant.lik...@secretlab.ca wrote: On Thu, Sep 30, 2010 at 7:46 PM, David Brownell davi...@pacbell.net wrote: --- On Thu, 9/30/10, Mingkai Hu mingkai...@freescale.com wrote: From: Mingkai Hu mingkai...@freescale.com Subject: [PATCH v3 6/7] mtd: m25p80: add a read function to read page by page NAK. We went over this before. Yes, I agree with David on this. If large transfers don't work, then it is the SPI master driver that is buggy. By the way, does this fix your problem? https://patchwork.kernel.org/patch/184752/ It shouldn't. AFAIK, eSPI is PIO-only controller, and the overrun fix is for the DMA mode. Thanks, p.s. Btw, in patch 3/7, is_dma_mapped argument of fsl_espi_bufs() is unneeded. -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/3] powerpc/fsl_soc.c: remove FSL USB platform code
On Tue, Sep 28, 2010 at 11:36:32AM +0200, Anatolij Gustschin wrote: This removed code will be replaced by simple of_platform driver for creation of FSL USB platform devices which is added by next patch of the series. Signed-off-by: Anatolij Gustschin ag...@denx.de Cc: Kumar Gala ga...@kernel.crashing.org Cc: Grant Likely grant.lik...@secretlab.ca --- This is not bisectable. You have to merge 1/3 and 2/3. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] spi_mpc8xxx: issue with using definition of pram in Device Tree
Hello, On Fri, Sep 24, 2010 at 09:20:27AM +0200, LEROY Christophe wrote: The issue is that cpm_muram_alloc_fixed() allocates memory from the general purpose muram area (from 0x0 to 0x1bff). Here we need to return a pointer to the parameter RAM, which is located somewhere starting at 0x1c00. It is not a dynamic allocation that is required here but only to point on the correct location in the parameter RAM. For the CPM2, I don't know. I'm working with a MPC866. Attached is a previous discussion on the subject where I explain a bit more in details the issue. The patch looks OK, I think. Doesn't explain why that worked on MPC8272 (CPM2) and MPC8560 (also CPM2) machines though. But here's my guess (I no longer have these boards to test it): On 8272 I used this node: + s...@4c0 { + #address-cells = 1; + #size-cells = 0; + compatible = fsl,cpm2-spi, fsl,spi; + reg = 0x11a80 0x40 0x89fc 0x2; On that SOC there are two muram data regions 0x0..0x2000 and 0x9000..0x9100. Note that we actually don't want data regions, and the only reason why that worked is that sysdev/cpm_common.c maps muram(0)..muram(max). Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3 v4] P4080/mtd: Only make elbc nand driver detect nand flash partitions
On Fri, Sep 17, 2010 at 03:01:08PM +0800, Roy Zang wrote: [...] +static struct mutex fsl_elbc_nand_mutex; + +static int __devinit fsl_elbc_nand_probe(struct platform_device *dev) { - struct fsl_lbc_regs __iomem *lbc = ctrl-regs; + struct fsl_lbc_regs __iomem *lbc; struct fsl_elbc_mtd *priv; struct resource res; + struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = NULL; No need for = NULL. [...] - ctrl-chips[bank] = priv; + mutex_init(fsl_elbc_nand_mutex); This may cause all sorts of misbehaviours, e.g. A: mutex_init(foo) A: mutex_lock(foo) B: mutex_init(foo) - destroyed A-context mutex. A: mutex_unlock(foo) - oops Instead of dynamically initializing the mutex, just define it with DEFINE_MUTEX() above. (Btw, #include linux/mutex.h is needed.) + + mutex_lock(fsl_elbc_nand_mutex); [...] -static int __devinit fsl_elbc_ctrl_init(struct fsl_elbc_ctrl *ctrl) +static int fsl_elbc_nand_remove(struct platform_device *dev) [...] + struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = fsl_lbc_ctrl_dev-nand; [...] + if (elbc_fcm_ctrl-chips[i]) + fsl_elbc_chip_remove(elbc_fcm_ctrl-chips[i]); [...] + fsl_lbc_ctrl_dev-nand = NULL; + kfree(elbc_fcm_ctrl); Will cause NULL dereference and/or use-after-free for other elbc nand instances. To avoid that, reference counting for elbc_fcm_ctrl is required. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3 v4] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices
On Fri, Sep 17, 2010 at 03:01:07PM +0800, Roy Zang wrote: [...] +struct fsl_lbc_ctrl { + /* device info */ + struct device *dev; Forward declaration for struct device is needed (and enough). + struct fsl_lbc_regs __iomem *regs; + int irq; + wait_queue_head_t irq_wait; #include linux/wait.h is needed for this. + spinlock_t lock; #include linux/spinlock.h [...] diff --git a/arch/powerpc/sysdev/fsl_lbc.c b/arch/powerpc/sysdev/fsl_lbc.c index dceb8d1..4920cd3 100644 --- a/arch/powerpc/sysdev/fsl_lbc.c +++ b/arch/powerpc/sysdev/fsl_lbc.c [...] +#include linux/slab.h +#include linux/of_platform.h I guess of_platform.h is not needed any longer. +#include linux/platform_device.h +#include linux/interrupt.h [...] default: return -EINVAL; @@ -143,14 +130,18 @@ EXPORT_SYMBOL(fsl_upm_find); * thus UPM pattern actually executed. Note that mar usage depends on the * pre-programmed AMX bits in the UPM RAM. */ + No need for this empty line. int fsl_upm_run_pattern(struct fsl_upm *upm, void __iomem *io_base, u32 mar) { int ret = 0; unsigned long flags; [...] +static int __devinit fsl_lbc_ctrl_probe(struct platform_device *dev) +{ + int ret; + + if (!dev-dev.of_node) { + dev_err(dev-dev, Device OF-Node is NULL); + return -EFAULT; + } + fsl_lbc_ctrl_dev = kzalloc(sizeof(*fsl_lbc_ctrl_dev), GFP_KERNEL); Btw, the code in the NAND driver checks for !fsl_lbc_ctrl_dev. So it might make sense to assign the global variable after the struct is fully initialized. + if (!fsl_lbc_ctrl_dev) + return -ENOMEM; + + dev_set_drvdata(dev-dev, fsl_lbc_ctrl_dev); + + spin_lock_init(fsl_lbc_ctrl_dev-lock); + init_waitqueue_head(fsl_lbc_ctrl_dev-irq_wait); + + fsl_lbc_ctrl_dev-regs = of_iomap(dev-dev.of_node, 0); + if (!fsl_lbc_ctrl_dev-regs) { + dev_err(dev-dev, failed to get memory region\n); + ret = -ENODEV; + goto err; + } + + fsl_lbc_ctrl_dev-irq = irq_of_parse_and_map(dev-dev.of_node, 0); + if (fsl_lbc_ctrl_dev-irq == NO_IRQ) { + dev_err(dev-dev, failed to get irq resource\n); + ret = -ENODEV; + goto err; + } + + fsl_lbc_ctrl_dev-dev = dev-dev; + + ret = fsl_lbc_ctrl_init(fsl_lbc_ctrl_dev); + if (ret 0) + goto err; + + ret = request_irq(fsl_lbc_ctrl_dev-irq, fsl_lbc_ctrl_irq, 0, + fsl-lbc, fsl_lbc_ctrl_dev); + if (ret != 0) { + dev_err(dev-dev, failed to install irq (%d)\n, + fsl_lbc_ctrl_dev-irq); + ret = fsl_lbc_ctrl_dev-irq; + goto err; + } + + return 0; + +err: + iounmap(fsl_lbc_ctrl_dev-regs); + kfree(fsl_lbc_ctrl_dev); + return ret; +} + +static const struct of_device_id fsl_lbc_match[] = { #include linux/mod_devicetable.h is needed for this. Plus, I think the patch is not runtime bisectable (i.e. you now do request_irq() here, but not removing it from the nand driver, so nand will fail to probe). Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3 v3] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices
On Thu, Sep 16, 2010 at 02:41:22PM +0800, Roy Zang wrote: [...] +static const struct platform_device_id fsl_lbc_match[] = { + { fsl,elbc, }, + { fsl,pq3-localbus, }, + { fsl,pq2-localbus, }, + { fsl,pq2pro-localbus, }, + {}, +}; + +static struct platform_driver fsl_lbc_ctrl_driver = { + .driver = { + .name = fsl-lbc, + }, + .probe = fsl_lbc_ctrl_probe, + .id_table = fsl_lbc_match, +}; No, it won't work that way (at least not w/o a device constructor somewhere in fsl_soc.c). Instead, you should write something like static const struct of_device_id fsl_lbc_match[] = { ... }; static struct platform_driver fsl_lbc_ctrl_driver = { .driver = { .name = fsl-lbc, .of_match_table = fsl_lbc_match; } ... }; (Note that platform_driver.driver has of_match_table nowadays -- that's what makes it possible to seamlessly transit from of_platform_driver to platform_driver.) The same applies for the second patch as well. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3 v3] P4080/mtd: Fix the freescale lbc issue with 36bit mode
On Thu, Sep 16, 2010 at 02:41:24PM +0800, Roy Zang wrote: From: Lan Chunhe-B25806 b25...@freescale.com When system uses 36bit physical address, res.start is 36bit physical address. But the function of in_be32 returns 32bit physical address. Then both of them compared each other is wrong. So by converting the address of res.start into the right format fixes this issue. Signed-off-by: Lan Chunhe-B25806 b25...@freescale.com Signed-off-by: Roy Zang tie-fei.z...@freescale.com --- arch/powerpc/include/asm/fsl_lbc.h |1 + arch/powerpc/sysdev/fsl_lbc.c | 23 ++- drivers/mtd/nand/fsl_elbc_nand.c |2 +- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/include/asm/fsl_lbc.h b/arch/powerpc/include/asm/fsl_lbc.h index db94698..5638b1e 100644 --- a/arch/powerpc/include/asm/fsl_lbc.h +++ b/arch/powerpc/include/asm/fsl_lbc.h @@ -246,6 +246,7 @@ struct fsl_upm { int width; }; +extern unsigned int fsl_lbc_addr(phys_addr_t addr_base); u32 here. Other than that, the patch looks good. Reviewed-by: Anton Vorontsov cbouatmai...@gmail.com Thanks! -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3 v3] P4080/mtd: Only make elbc nand driver detect nand flash partitions
On Thu, Sep 16, 2010 at 02:41:23PM +0800, Roy Zang wrote: [...] -static int __devinit fsl_elbc_chip_probe(struct fsl_elbc_ctrl *ctrl, - struct device_node *node) +/* + * Currently only one elbc probe is supported. + */ +static int __devinit fsl_elbc_nand_probe(struct platform_device *dev) { - struct fsl_lbc_regs __iomem *lbc = ctrl-regs; + struct fsl_lbc_regs __iomem *lbc; struct fsl_elbc_mtd *priv; struct resource res; + struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = NULL; [...] - ctrl-chips[bank] = priv; + if (fsl_lbc_ctrl_dev-nand == NULL) { + elbc_fcm_ctrl = kzalloc(sizeof(*elbc_fcm_ctrl), GFP_KERNEL); + if (!elbc_fcm_ctrl) { [...] + goto err; + } + fsl_lbc_ctrl_dev-nand = elbc_fcm_ctrl; + } + + elbc_fcm_ctrl-chips[bank] = priv; Again, this will oops on the second probe. And you still don't lock fsl_lbc_ctrl_dev-nand during check-then-assignment, so with parallel probing there will be a race. :-( We do have boards with several NAND chips (e.g. arch/powerpc/boot/dts/mpc8610_hpcd.dts), so these issues are all legitimate. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3 v3] P4080/mtd: Only make elbc nand driver detect nand flash partitions
On Thu, Sep 16, 2010 at 04:50:05PM +0800, Zang Roy-R61911 wrote: On Thu, Sep 16, 2010 at 02:41:23PM +0800, Roy Zang wrote: [...] -static int __devinit fsl_elbc_chip_probe(struct fsl_elbc_ctrl *ctrl, - struct device_node *node) +/* + * Currently only one elbc probe is supported. + */ +static int __devinit fsl_elbc_nand_probe(struct platform_device *dev) { - struct fsl_lbc_regs __iomem *lbc = ctrl-regs; + struct fsl_lbc_regs __iomem *lbc; struct fsl_elbc_mtd *priv; struct resource res; + struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = NULL; [...] - ctrl-chips[bank] = priv; + if (fsl_lbc_ctrl_dev-nand == NULL) { + elbc_fcm_ctrl = kzalloc(sizeof(*elbc_fcm_ctrl), GFP_KERNEL); + if (!elbc_fcm_ctrl) { [...] + goto err; + } + fsl_lbc_ctrl_dev-nand = elbc_fcm_ctrl; + } + + elbc_fcm_ctrl-chips[bank] = priv; Again, this will oops on the second probe. Why? Because of a NULL dereference (elbc_fcm_ctrl-). I understand that you don't have to believe me, but will you believe a compiler? oksana:~$ cat a.c #include stdio.h #include malloc.h char *foo; void probe(void) { char *bar = NULL; if (!foo) { bar = malloc(sizeof(*bar)); if (!bar) return; foo = bar; } *bar = 'a'; } int main(void) { probe(); probe(); return 0; } oksana:~$ gcc a.c ./a.out Segmentation fault -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3 v3] P4080/mtd: Only make elbc nand driver detect nand flash partitions
On Thu, Sep 16, 2010 at 11:14:48AM -0500, Scott Wood wrote: DEFINE_MUTEX(fsl_elbc_mutex); I'd place the mutex inside the fsl_lbc_ctrl_dev, i.e. fsl_lbc_ctrl_dev-nand_lock. This is to avoid more global variables. I wouldn't. If the lock is only meaningful to the NAND driver, it should be declared in the NAND driver. Besides, it's not any less of a global just because it's sitting inside a singleton struct. Perhaps it should be declared as a static local inside the probe function, if it's just to guard against this one race. OK, in that case better be persistent and not introduce fsl_lbc_ctrl_dev-nand at all, as it isn't used outside of the driver. Having fsl_lbc_ctrl_dev-nand and its lock elsewhere in the code makes no sense. Btw, even before this patch, it seems that the driver had all these bugs/races, i.e. ctrl-controller.lock was not used at all. Ugh. It is used, search nand_base.c for controller-lock. OK, now I see, the driver implements its own chip-controller (which is exactly what ctrl-controller is). Then we're fine. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3 v2][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices
On Fri, Sep 10, 2010 at 02:58:15PM +0800, Zang Roy-R61911 wrote: [...] +static struct of_platform_driver fsl_lbc_ctrl_driver = { Need linux/of_platform.h for this. It has been include by fsl_lbc.h-linux/of_platform.h- linux/platform_device.h Before submitting the patch, I have built and tested it. In Linux we try to include all the headers explicitly (except for asm/* if the same header name exists in linux/). That's to avoid problems if for some reason fsl_lbc.h will stop including of_plaform.h some day. Oh, and by the way, there is absolutely no reason to add linux/of_platform.h and interrupts.h into fsl_lbc.h, they're is simply not needed in fsl_lbc.h. Do you think I do not build the tree before I send out the patch? Nope, that's not what I think. I didn't say that the file won't build w/o these fixes, but they're still needed. + +static struct of_platform_driver fsl_lbc_ctrl_driver = { Need linux/of_platform.h for this. But you actually don't need of_platform_driver, as for the new code you can use platform_driver (and thus linux/platform_device.h). I'd prefer using of_platform_driver here for simplified code. Any special reason to use platform_device here? In the new kernels, of_platform_driver is almost a synonym of platform_driver, and 'of_platform_driver' stuff is soon to be deleted. You can use platform_driver just like of_platform_driver nowadays. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: How to define an I2C-to-SPI bridge device ?
On Fri, Sep 10, 2010 at 08:14:44PM +0200, André Schwarz wrote: [...] Does the device actually generate edge interrupts? Or is it a level irq device? If it is a level irq device, then the correct way to handle this is to disable the irq line so that the event can be handled at non-irq context, and then reenable it when finished. The irq is level-low active. Will do it via disable/re-enable then. FYI, In newer kernels you don't have to do it manually, there's request_threaded_irq() for this. -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/8] sdhci: Move real work out of an atomic context
On Thu, Sep 09, 2010 at 03:28:34AM +0100, Chris Ball wrote: [...] [7.372843] [b04193ce] __might_sleep+0xd9/0xe0 [7.387864] [b07260cc] mutex_lock+0x1c/0x2a [7.402576] [b06396e8] sdhci_led_control+0x1a/0x41 [7.417727] [b063bece] led_trigger_event+0x42/0x5c led_trigger_even grabs a readlock. :-( [7.432807] [b06326f8] mmc_request_done+0x56/0x6f [7.447597] [b063a2d1] sdhci_finish_work+0xc8/0xcd [7.462643] [b063a209] ? sdhci_finish_work+0x0/0xcd [7.477941] [b0432776] worker_thread+0x165/0x1ed [7.492856] [b063a209] ? sdhci_finish_work+0x0/0xcd [7.508204] [b0435591] ? autoremove_wake_function+0x0/0x34 [7.524178] [b0432611] ? worker_thread+0x0/0x1ed [7.538953] [b04352a0] kthread+0x63/0x68 [7.552659] [b043523d] ? kthread+0x0/0x68 [7.566349] [b0402cf6] kernel_thread_helper+0x6/0x10 [7.709931] udev: starting version 141 [7.940374] mmc2: new high speed SDHC card at address e4da [8.058165] mmcblk0: mmc2:e4da SU04G 3.69 GiB [8.135730] mmcblk0: p1 p2 Full dmesg is at http://chris.printf.net/anton-mutex-dmesg.txt. Anton, the kernel is 2.6.35.4-olpc plus your patchset from -mm. I can think about how to test on an upstream kernel instead, but perhaps your own tests simply didn't hit sdhci_led_control(). Yep, LEDS support was turned off. Andrew, if you want to drop this while the BUG() and potential performance regressions are worked out, I'd be happy to keep testing patches from Anton until it's without regressions here. Thanks Chris. I also think that it's better to drop these series now, and meanwhile I'll try to prepare another patchset. -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3 v2][MTD] P4080/mtd: Only make elbc nand driver detect nand flash partitions
, }, {} }; -static struct of_platform_driver fsl_elbc_ctrl_driver = { +static struct of_platform_driver fsl_elbc_nand_driver = { If you write of_platform_driver, you need linux/of_platform.h (which you removed in this patch). But I think that you need just 'struct platform_driver' here, and include linux/platform_device.h. .driver = { - .name = fsl-elbc, + .name = fsl,elbc-fcm-nand, .owner = THIS_MODULE, - .of_match_table = fsl_elbc_match, + .of_match_table = fsl_elbc_nand_match, }, - .probe = fsl_elbc_ctrl_probe, - .remove = fsl_elbc_ctrl_remove, + .probe = fsl_elbc_nand_probe, + .remove = fsl_elbc_nand_remove, }; Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 3/3][MTD] P4080/mtd: Fix the freescale lbc issue with 36bit mode
On Thu, Sep 09, 2010 at 06:20:32PM +0800, Roy Zang wrote: [...] /** + * fsl_lbc_addr - convert the base address + * @addr_base: base address of the memory bank + * + * This function converts a base address of lbc into the right format for the BR + * registers. If the SOC has eLBC then it returns 32bit physical address else + * it returns 34bit physical address for local bus(Example: MPC8641). + */ It returns 34bit physical address encoded in a 32 bit word, right? Because, IIRC, 'unsigned int' is always 32 bit. Worth mentioning this fact. +unsigned int fsl_lbc_addr(phys_addr_t addr_base) +{ + void *dev; struct device_node *np; + int compatible; + + dev = fsl_lbc_ctrl_dev-dev-of_node; + compatible = of_device_is_compatible(dev, fsl,elbc); + + if (compatible) + return addr_base 0x8000; + else + return (addr_base 0x08000ull) + | ((addr_base 0x3ull) 19); +} +EXPORT_SYMBOL(fsl_lbc_addr); Almost perfect. I'm not sure if 'unsigned int' is technically correct return type for this function though. I guess it should be u32. Also, the function may be a bit more understandable and shorter: u32 fsl_lbc_addr(phys_addr_t addr) { struct device_node *np = fsl_lbc_ctrl_dev-dev-of_node; u32 addrl = addr 0x8000; if (of_device_is_compatible(np, fsl,elbc)) return addrl; return addrl | ((addr 0x3ull) 19); } EXPORT_SYMBOL(fsl_lbc_addr); Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3 v2][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices
) + dev_err(ctrl-dev, Unknown error: + LTESR 0x%08X\n, status); + + return IRQ_HANDLED; + } + + return IRQ_NONE; +} + +/* + * fsl_lbc_ctrl_probe + * + * called by device layer when it finds a device matching + * one our driver can handled. This code allocates all of + * the resources needed for the controller only. The + * resources for the NAND banks themselves are allocated + * in the chip probe function. +*/ + +static int __devinit fsl_lbc_ctrl_probe(struct platform_device *ofdev, + const struct of_device_id *match) +{ + int ret; + + if (!ofdev-dev.of_node) { + dev_err(ofdev-dev, Device OF-Node is NULL); + return -EFAULT; + } + fsl_lbc_ctrl_dev = kzalloc(sizeof(*fsl_lbc_ctrl_dev), GFP_KERNEL); + if (!fsl_lbc_ctrl_dev) + return -ENOMEM; + + dev_set_drvdata(ofdev-dev, fsl_lbc_ctrl_dev); + + spin_lock_init(fsl_lbc_ctrl_dev-lock); + init_waitqueue_head(fsl_lbc_ctrl_dev-irq_wait); + + fsl_lbc_ctrl_dev-regs = of_iomap(ofdev-dev.of_node, 0); + if (!fsl_lbc_ctrl_dev-regs) { + dev_err(ofdev-dev, failed to get memory region\n); + ret = -ENODEV; + goto err; + } + + fsl_lbc_ctrl_dev-irq = irq_of_parse_and_map(ofdev-dev.of_node, 0); + if (fsl_lbc_ctrl_dev-irq == NO_IRQ) { + dev_err(ofdev-dev, failed to get irq resource\n); + ret = -ENODEV; + goto err; + } + + fsl_lbc_ctrl_dev-dev = ofdev-dev; + + ret = fsl_lbc_ctrl_init(fsl_lbc_ctrl_dev); + if (ret 0) + goto err; + + ret = request_irq(fsl_lbc_ctrl_dev-irq, fsl_lbc_ctrl_irq, 0, + fsl-lbc, fsl_lbc_ctrl_dev); + if (ret != 0) { + dev_err(ofdev-dev, failed to install irq (%d)\n, + fsl_lbc_ctrl_dev-irq); + ret = fsl_lbc_ctrl_dev-irq; + goto err; + } + + return 0; + +err: + iounmap(fsl_lbc_ctrl_dev-regs); + kfree(fsl_lbc_ctrl_dev); + return ret; +} + +static const struct of_device_id fsl_lbc_match[] = { + { .compatible = fsl,elbc, }, + { .compatible = fsl,pq3-localbus, }, + { .compatible = fsl,pq2-localbus, }, + { .compatible = fsl,pq2pro-localbus, }, + {}, +}; You need linux/mod_devicetable.h for this. + +static struct of_platform_driver fsl_lbc_ctrl_driver = { Need linux/of_platform.h for this. But you actually don't need of_platform_driver, as for the new code you can use platform_driver (and thus linux/platform_device.h). + .driver = { + .name = fsl-lbc, + .of_match_table = fsl_lbc_match, + }, + .probe = fsl_lbc_ctrl_probe, +}; + +static int __init fsl_lbc_init(void) +{ + return of_register_platform_driver(fsl_lbc_ctrl_driver); +} + No need for this empty line. +module_init(fsl_lbc_init); Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/8] sdhci: Move real work out of an atomic context
On Wed, Sep 08, 2010 at 10:37:41PM +0100, Chris Ball wrote: Hi Andrew, On Tue, Sep 07, 2010 at 03:38:13PM -0700, Andrew Morton wrote: I noticed no throughput drop neither with PIO transfers nor with DMA (tested on MPC8569E CPU), while latencies should be greatly improved. This patchset isn't causing any problems yet, but may do so in the future and will impact the validity of any testing. It seems to be kind of stuck. Should I drop it all? I suggest keeping it -- I'll find time to test it out here soon, and will keep it in mind as a possible regression cause. Thanks! Would be also great if you could point out which patch causes most of the performance drop (if any)? Albert, if you could find time, can you also bisect the patchset? I wouldn't want to buy Nintendo WII just to debug the perf regression. ;-) FWIW, I tried to disable multiblock read/writes and test with SD cards, and still didn't notice any performance drops. Maybe it's SDIO IRQs that cause the performance drop for the WII case, as we delay them a little bit? Or it could be the patch that introduces threaded IRQ handler in whole causes it. If so, I guess we'd need to move some of the processing to the real IRQ context, keeping the handler lockless (if possible) or introducing a very fine grained locking. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/8] sdhci: Move real work out of an atomic context
On Wed, Sep 08, 2010 at 11:05:48PM +0100, Chris Ball wrote: Hi Anton, On Thu, Sep 09, 2010 at 01:57:50AM +0400, Anton Vorontsov wrote: Thanks! Would be also great if you could point out which patch causes most of the performance drop (if any)? Albert, if you could find time, can you also bisect the patchset? I wouldn't want to buy Nintendo WII just to debug the perf regression. ;-) FWIW, I tried to disable multiblock read/writes and test with SD cards, and still didn't notice any performance drops. Maybe it's SDIO IRQs that cause the performance drop for the WII case, as we delay them a little bit? Or it could be the patch that introduces threaded IRQ handler in whole causes it. If so, I guess we'd need to move some of the processing to the real IRQ context, keeping the handler lockless (if possible) or introducing a very fine grained locking. I didn't know anything about a reported performance drop, and I don't think Andrew did either -- Albert's test results don't seem to have made it to this list, or anywhere else that I can see. Could you link to/repost his comments? (I'll be testing with libertas, so that will stress-test SDIO IRQs.) Sure thing, here are Albert's results. - Forwarded message from Albert Herranz albert_herr...@yahoo.es - Date: Mon, 02 Aug 2010 21:23:51 +0200 From: Albert Herranz albert_herr...@yahoo.es To: Anton Vorontsov cbouatmai...@gmail.com CC: a...@linux-foundation.org, mm-comm...@vger.kernel.org, ben-li...@fluff.org, m...@console-pimps.org, pie...@ossman.eu, w.s...@pengutronix.de, m...@bu3sch.de Subject: Re: + sdhci-use-work-structs-instead-of-tasklets.patch added to -mm tree Hi, Some initial numbers regarding performance. The patchset seems to cause a noticeable performance drop. I've run two iperf client tests (see the two invocations of iperf -c) and two iperf server tests (see iperf -s invocation). == 2.6.33 == $ iperf -c 192.168.1.130 Client connecting to 192.168.1.130, TCP port 5001 TCP window size: 16.0 KByte (default) [ 3] local 192.168.1.127 port 40119 connected with 192.168.1.130 port 5001 [ ID] Interval Transfer Bandwidth [ 3] 0.0-10.1 sec 1.05 MBytes872 Kbits/sec $ iperf -c 192.168.1.130 Client connecting to 192.168.1.130, TCP port 5001 TCP window size: 16.0 KByte (default) [ 3] local 192.168.1.127 port 40120 connected with 192.168.1.130 port 5001 [ ID] Interval Transfer Bandwidth [ 3] 0.0-10.0 sec 1.04 MBytes870 Kbits/sec $ iperf -s Server listening on TCP port 5001 TCP window size: 85.3 KByte (default) [ 4] local 192.168.1.127 port 5001 connected with 192.168.1.130 port 36691 [ ID] Interval Transfer Bandwidth [ 4] 0.0-10.2 sec 3.61 MBytes 2.98 Mbits/sec [ 5] local 192.168.1.127 port 5001 connected with 192.168.1.130 port 36692 [ 5] 0.0-10.1 sec 4.94 MBytes 4.09 Mbits/sec == 2.6.33 + sdhci: Move real work out of an atomic context patchset == $ iperf -c 192.168.1.130 Client connecting to 192.168.1.130, TCP port 5001 TCP window size: 16.0 KByte (default) [ 3] local 192.168.1.127 port 39210 connected with 192.168.1.130 port 5001 [ ID] Interval Transfer Bandwidth [ 3] 0.0-10.0 sec368 KBytes301 Kbits/sec $ iperf -c 192.168.1.130 Client connecting to 192.168.1.130, TCP port 5001 TCP window size: 16.0 KByte (default) [ 3] local 192.168.1.127 port 39211 connected with 192.168.1.130 port 5001 [ ID] Interval Transfer Bandwidth [ 3] 0.0-10.2 sec440 KBytes354 Kbits/sec $ iperf -s Server listening on TCP port 5001 TCP window size: 85.3 KByte (default) [ 4] local 192.168.1.127 port 5001 connected with 192.168.1.130 port 57833 [ ID] Interval Transfer Bandwidth [ 4] 0.0-10.2 sec 2.37 MBytes 1.95 Mbits/sec [ 5] local 192.168.1.127 port 5001 connected with 192.168.1.130 port 57834 [ 5] 0.0-10.2 sec 2.30 MBytes 1.90 Mbits/sec The subjective feeling is too that the system is slower. Cheers, Albert - End forwarded message - ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3][MTD] P4080/nand: Only make elbc nand driver detect nand flash partitions
On Mon, Sep 06, 2010 at 12:49:17PM +0800, Zang Roy-R61911 wrote: On Fri, Aug 06, 2010 at 10:51:35AM +0800, Roy Zang wrote: [...] +static struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl; + Are you sure that you want it as a global var? A bit scary change. Oh, you probably don't need it, as you can get it from fsl_lbc_ctrl_dev-nand? Get it form fsl_lbc_ctrl_dev-nand or assign it to fsl_lbc_ctrl_dev-nand in probe? I meant to get it from fsl_lbc_ctrl_dev-nand. I.e. in probe() you do: fsl_lbc_ctrl_dev-nand = elbc_fcm_ctrl, so you probably don't need the global var. (fsl_lbc_ctrl_dev seems to be global as well, duh. Well, one variable less in the global name space. But I'd probably use lbc_np-data to store the LBC private struct). Scott seem to be fine with it as there are probably no plans to to add several localbus controllers into the SoCs. But, I saw a custom board with two MPC82xx SoCs connected together, one as a master (core + peripheral devs), and other as a slave (its core was halted, and only slave's CPM peripheral devices were used by the master CPU). I think it is possible to connect two (or more) SoCs in a such way so that two or more LBC controllers would be visible for the Linux. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices
On Mon, Sep 06, 2010 at 05:24:35PM +0800, Zang Roy-R61911 wrote: [..] mxmr = fsl_lbc_ctrl_dev-regs-mcmr; That makes sense. A global or local variable for fsl_lbc_ctrl_dev-regs? Which one is better? The less global variables, the better. So, I'd vote for a local one. [...] +static int __devinit fsl_lbc_ctrl_probe(struct of_device *ofdev, + const struct of_device_id *match) +{ + int ret = 0; no need for the initial value here. Any harm? Probably not as gcc will likely optimize it away, but it's not needed, so why keep it there? habit. ;-) Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices
On Fri, Aug 06, 2010 at 10:51:34AM +0800, Roy Zang wrote: From: Lan Chunhe-B25806 b25...@freescale.com Move Freescale elbc interrupt from nand dirver to elbc driver. Then all elbc devices can use the interrupt instead of ONLY nand. Signed-off-by: Lan Chunhe-B25806 b25...@freescale.com Signed-off-by: Roy Zang tie-fei.z...@freescale.com --- send the patch to linux-...@lists.infradead.org it has been posted to linuxppc-...@ozlabs.org and do not get any comment. arch/powerpc/Kconfig |7 +- arch/powerpc/include/asm/fsl_lbc.h | 34 +- arch/powerpc/sysdev/fsl_lbc.c | 254 ++-- 3 files changed, 253 insertions(+), 42 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 2031a28..5155b53 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -703,9 +703,12 @@ config 4xx_SOC bool config FSL_LBC - bool + bool Freescale Local Bus support + depends on FSL_SOC help - Freescale Localbus support + Enables reporting of errors from the Freescale local bus + controller. Also contains some common code used by + drivers for specific local bus peripherals. config FSL_GTM bool [...] diff --git a/arch/powerpc/sysdev/fsl_lbc.c b/arch/powerpc/sysdev/fsl_lbc.c index dceb8d1..9c9e44f 100644 --- a/arch/powerpc/sysdev/fsl_lbc.c +++ b/arch/powerpc/sysdev/fsl_lbc.c @@ -5,6 +5,10 @@ * * Author: Anton Vorontsov avoront...@ru.mvista.com * + * Copyright (c) 2010 Freescale Semiconductor + * + * Authors: Jack Lan jack@freescale.com Would be prettier if you'd group copyright and authorship notices. I.e. Copyright 2008 MV Copyright 2010 FSL Authors: Anton Jack + * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or @@ -23,35 +27,8 @@ #include asm/fsl_lbc.h static spinlock_t fsl_lbc_lock = __SPIN_LOCK_UNLOCKED(fsl_lbc_lock); -static struct fsl_lbc_regs __iomem *fsl_lbc_regs; - -static char __initdata *compat_lbc[] = { - fsl,pq2-localbus, - fsl,pq2pro-localbus, - fsl,pq3-localbus, - fsl,elbc, -}; - -static int __init fsl_lbc_init(void) -{ - struct device_node *lbus; - int i; - - for (i = 0; i ARRAY_SIZE(compat_lbc); i++) { - lbus = of_find_compatible_node(NULL, NULL, compat_lbc[i]); - if (lbus) - goto found; - } - return -ENODEV; - -found: - fsl_lbc_regs = of_iomap(lbus, 0); - of_node_put(lbus); - if (!fsl_lbc_regs) - return -ENOMEM; - return 0; -} -arch_initcall(fsl_lbc_init); +struct fsl_lbc_ctrl *fsl_lbc_ctrl_dev; +EXPORT_SYMBOL(fsl_lbc_ctrl_dev); /** * fsl_lbc_find - find Localbus bank @@ -66,12 +43,12 @@ int fsl_lbc_find(phys_addr_t addr_base) { int i; - if (!fsl_lbc_regs) + if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev-regs) return -ENODEV; - for (i = 0; i ARRAY_SIZE(fsl_lbc_regs-bank); i++) { - __be32 br = in_be32(fsl_lbc_regs-bank[i].br); - __be32 or = in_be32(fsl_lbc_regs-bank[i].or); + for (i = 0; i ARRAY_SIZE(fsl_lbc_ctrl_dev-regs-bank); i++) { + __be32 br = in_be32(fsl_lbc_ctrl_dev-regs-bank[i].br); + __be32 or = in_be32(fsl_lbc_ctrl_dev-regs-bank[i].or); A dedicated variable for regs would make this much more readable? if (br BR_V (br or BR_BA) == addr_base) return i; @@ -99,17 +76,20 @@ int fsl_upm_find(phys_addr_t addr_base, struct fsl_upm *upm) if (bank 0) return bank; - br = in_be32(fsl_lbc_regs-bank[bank].br); + if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev-regs) + return -ENODEV; + + br = in_be32(fsl_lbc_ctrl_dev-regs-bank[bank].br); switch (br BR_MSEL) { case BR_MS_UPMA: - upm-mxmr = fsl_lbc_regs-mamr; + upm-mxmr = fsl_lbc_ctrl_dev-regs-mamr; Ditto, a very repetitive stuff, desires a variable for regs? break; case BR_MS_UPMB: - upm-mxmr = fsl_lbc_regs-mbmr; + upm-mxmr = fsl_lbc_ctrl_dev-regs-mbmr; break; case BR_MS_UPMC: - upm-mxmr = fsl_lbc_regs-mcmr; + upm-mxmr = fsl_lbc_ctrl_dev-regs-mcmr; break; default: return -EINVAL; @@ -143,14 +123,18 @@ EXPORT_SYMBOL(fsl_upm_find); * thus UPM pattern actually executed. Note that mar usage depends on the * pre-programmed AMX bits in the UPM RAM. */ + int fsl_upm_run_pattern(struct fsl_upm *upm, void __iomem *io_base, u32 mar) { int ret = 0; unsigned long flags; + if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev-regs
Re: [PATCH 3/3][MTD] P4080/nand: Fix the freescale lbc issue with 36bit mode
On Fri, Aug 06, 2010 at 10:51:36AM +0800, Roy Zang wrote: [...] /** + * convert_lbc_address - convert the base address + * @addr_base: base address of the memory bank + * + * This function converts a base address of lbc into the right format for the BR + * registers. If the SOC has eLBC then it returns 32bit physical address else + * it returns 34bit physical address for local bus(Example: MPC8641). + */ +unsigned int convert_lbc_address(phys_addr_t addr_base) +{ + void *dev; + int compatible; + + dev = of_find_node_by_name(NULL, localbus); Nope, you shouldn't do this. Never search by name. Also, aren't there already a global dev, which was found by the _probe() stuff? + if (!dev) { + printk(KERN_INFO fsl-lbc: can't find localbus node\n); + of_node_put(dev); + return 0; + } + + compatible = of_device_is_compatible(dev, fsl,elbc); + of_node_put(dev); + if (compatible) + return addr_base 0x8000; + else + return (addr_base 0x08000ull) \ + | ((addr_base 0x3ull) 19); +} +EXPORT_SYMBOL(convert_lbc_address); + +/** * fsl_lbc_find - find Localbus bank * @addr_base: base address of the memory bank * @@ -50,7 +80,8 @@ int fsl_lbc_find(phys_addr_t addr_base) __be32 br = in_be32(fsl_lbc_ctrl_dev-regs-bank[i].br); __be32 or = in_be32(fsl_lbc_ctrl_dev-regs-bank[i].or); - if (br BR_V (br or BR_BA) == addr_base) + if (br BR_V (br or BR_BA) \ No need for \ at the end of the line, keep == on the same line. + == convert_lbc_address(addr_base)) Would be prettier if you name it fsl_lbc_addr(). Keeps prefix the same for the rest of the file, plus makes it shorter (so there probably won't be any need for breaking the line). return i; } diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c index 7bbcb3f..0e8dc40 100644 --- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c @@ -838,7 +838,7 @@ static int __devinit fsl_elbc_nand_probe(struct of_device *dev, (in_be32(lbc-bank[bank].br) BR_MSEL) == BR_MS_FCM (in_be32(lbc-bank[bank].br) in_be32(lbc-bank[bank].or) BR_BA) - == res.start) + == convert_lbc_address(res.start)) break; if (bank = MAX_BANKS) { -- 1.5.6.5 Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3][MTD] P4080/nand: Only make elbc nand driver detect nand flash partitions
On Fri, Aug 06, 2010 at 10:51:35AM +0800, Roy Zang wrote: [...] +static struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl; + Are you sure that you want it as a global var? A bit scary change. Oh, you probably don't need it, as you can get it from fsl_lbc_ctrl_dev-nand? I wonder if Scott saw these patches? Cc'ed. -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: How to define an I2C-to-SPI bridge device ?
On Fri, Sep 03, 2010 at 10:36:19AM +0200, André Schwarz wrote: Hi, we're about to get new MPC8377 based hardware with various peripherals. There are two I2C-to-SPI bridge devices (NXP SC18IS602) and I'm not sure how to define a proper dts... Of course it's an easy thing creating 2 child nodes on the CPU's I2C device - but how can I represent the created SPI bus ? Um.. the same as the other SPI buses? I.e. i2c-controller { /* SOC I2C controller */ spi-controller { /* The I2C-to-SPI bridge */ spi-dev...@0 { }; spi-dev...@1 { }; }; }; Is the (possibly) required driver (of_sc18is60x_spi ?) supposed to be an I2C slave or an SPI host driver ? It should be an I2C driver that registers an SPI master (i.e. calls spi_alloc_master() and spi_register_master()). Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] [PPC] Motion-PRO: Added LED support for the Promess Motion-Pro board. The driver is based on the original version(http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg06694.htm
On Thu, Sep 02, 2010 at 12:20:31PM +0200, sposele...@emcraft.com wrote: [...] +config LEDS_MOTIONPRO + tristate Motionpro LEDs Support + depends on LEDS_CLASS + help + This option enables support for status and ready LEDs connected + to GPIO lines on Motionpro board. Why not expose these GPIOs via GPIOLIB[1] and use generic GPIO LEDs[2] driver, along with the timer LED trigger[3] for blinking? Thanks, [1] Documentation/gpio.txt Documentation/powerpc/dts-bindings/gpio/gpio.txt [2] drivers/leds/leds-gpio.c Documentation/powerpc/dts-bindings/gpio/led.txt [3] drivers/leds/ledtrig-timer.c -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] [PPC] Motion-PRO: Added LED support for the Promess Motion-Pro board. The driver is based on the original version(http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg06694.htm
On Thu, Sep 02, 2010 at 05:34:56PM +0400, Sergei Poselenov wrote: [...] + tristate Motionpro LEDs Support + depends on LEDS_CLASS + help + This option enables support for status and ready LEDs connected + to GPIO lines on Motionpro board. Why not expose these GPIOs via GPIOLIB[1] and use generic GPIO LEDs[2] driver, along with the timer LED trigger[3] for blinking? Thanks, [1] Documentation/gpio.txt Documentation/powerpc/dts-bindings/gpio/gpio.txt [2] drivers/leds/leds-gpio.c Documentation/powerpc/dts-bindings/gpio/led.txt [3] drivers/leds/ledtrig-timer.c Yes, this seem possible to implement (and thanks for pointing into this), however, the driver is already exists (actually, since 2007), so why to not add it to save efforts? - Faking PWM in the LEDs driver is just wrong thing to do. I don't see any other drivers doing this, and even if they were, they would need to be fixed; - This duplicates timer trigger functionality; - By writing (if there isn't any already) a generic GPIOLIB driver for the GPIO controller that you have, you could use these GPIOs not only for LEDs, but also for SPI, MDIO, I2C, MMC, and even raw NAND chips. I.e., by choosing the right methodology you save much more efforts in the long run. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] gpiolib: Add 'struct gpio_chip' forward declaration for !GPIOLIB case
On Tue, Aug 31, 2010 at 02:03:44AM -0600, Grant Likely wrote: On Tue, Aug 24, 2010 at 01:26:23PM +0400, Anton Vorontsov wrote: With CONFIG_GPIOLIB=n, the 'struct gpio_chip' is not declared, so the following pops up on PowerPC: cc1: warnings being treated as errors In file included from arch/powerpc/platforms/52xx/mpc52xx_common.c:19: include/linux/of_gpio.h:74: warning: 'struct gpio_chip' declared inside parameter list include/linux/of_gpio.h:74: warning: its scope is only this definition or declaration, which is probably not what you want include/linux/of_gpio.h:75: warning: 'struct gpio_chip' declared inside parameter list make[2]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error 1 This patch fixes the issue by providing the proper forward declaration. Signed-off-by: Anton Vorontsov cbouatmai...@gmail.com This doesn't actually solve the problem, and gpiochip should remain undefined when CONFIG_GPIOLIB=n to catch exactly these build failures. The real problem is that I merged a change into the mpc5200 code that required CONFIG_GPIOLIB be enabled without reflecting that requirement in Kconfig. No, look closer. The error is in of_gpio.h, and it's perfectly fine to include it w/o GPIOLIB=y. --- On Tue, Aug 24, 2010 at 04:26:08PM +1000, Benjamin Herrenschmidt wrote: I get that with my current stuff: cc1: warnings being treated as errors In file included from [..]/mpc52xx_common.c:19: of_gpio.h:74: error: ‘struct gpio_chip’ declared inside parameter list [...] make[3]: *** Waiting for unfinished jobs That's because with GPIOCHIP=n no one declares struct gpio_chip. It should be either of_gpio.h or gpio.h. Let's make it gpio.h, as this feels more generic, and we already have some !GPIOLIB handling in there. include/linux/gpio.h |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/include/linux/gpio.h b/include/linux/gpio.h index 03f616b..85207d2 100644 --- a/include/linux/gpio.h +++ b/include/linux/gpio.h @@ -15,6 +15,12 @@ struct device; /* + * Some code might rely on the declaration. Still, it is illegal + * to dereference it for !GPIOLIB case. + */ +struct gpio_chip; + +/* * Some platforms don't support the GPIO programming interface. * * In case some driver uses it anyway (it should normally have -- 1.7.0.5 -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] gpiolib: Add 'struct gpio_chip' forward declaration for !GPIOLIB case
On Tue, Aug 31, 2010 at 10:44:14AM -0600, Grant Likely wrote: On Tue, Aug 31, 2010 at 2:37 AM, Anton Vorontsov cbouatmai...@gmail.com wrote: On Tue, Aug 31, 2010 at 02:03:44AM -0600, Grant Likely wrote: On Tue, Aug 24, 2010 at 01:26:23PM +0400, Anton Vorontsov wrote: With CONFIG_GPIOLIB=n, the 'struct gpio_chip' is not declared, so the following pops up on PowerPC: cc1: warnings being treated as errors In file included from arch/powerpc/platforms/52xx/mpc52xx_common.c:19: include/linux/of_gpio.h:74: warning: 'struct gpio_chip' declared inside parameter list include/linux/of_gpio.h:74: warning: its scope is only this definition or declaration, which is probably not what you want include/linux/of_gpio.h:75: warning: 'struct gpio_chip' declared inside parameter list make[2]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error 1 This patch fixes the issue by providing the proper forward declaration. Signed-off-by: Anton Vorontsov cbouatmai...@gmail.com This doesn't actually solve the problem, and gpiochip should remain undefined when CONFIG_GPIOLIB=n to catch exactly these build failures. The real problem is that I merged a change into the mpc5200 code that required CONFIG_GPIOLIB be enabled without reflecting that requirement in Kconfig. No, look closer. The error is in of_gpio.h, and it's perfectly fine to include it w/o GPIOLIB=y. Looking even closer, we're both wrong. You're right I didn't look carefully enough, and the error is in of_gpio.h, not the .c file. However, it is not okay to get the definitions from of_gpio.h when CONFIG_GPIOLIB=n. If GPIOLIB, or more specifically OF_GPIO isn't set, then the of_gpio.h definitions should either not be defined, or should be defined as empty stubs (where appropriate). Grrr. Grant, look again, even closer than you did. They are stubs! #else /* CONFIG_OF_GPIO */ !OF_GPIO (or !GPIOLIB) case. /* Drivers may not strictly depend on the GPIO support, so let them link. */ static inline int of_get_gpio_flags(struct device_node *np, int index, enum of_gpio_flags *flags) { return -ENOSYS; } static inline unsigned int of_gpio_count(struct device_node *np) { return 0; } static inline void of_gpiochip_add(struct gpio_chip *gc) { } static inline void of_gpiochip_remove(struct gpio_chip *gc) { } #endif /* CONFIG_OF_GPIO */ The errors are triggered by the of_gpiochip_*() stubs, which are needed by the drivers/gpio/gpiolib.c. Do you want to add another #ifdef CONFIG_GPIOLIB around of_gpiochip_*()? That would be ugly. There's nothing wrong in providing the forward decls, you can't dereference it anyway (so you would still catch the bogus users). And at the same time it's will work great for these stubs. -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of_mmc_spi: add card detect irq support
Hello, The patch looks mostly good. A few cosmetic issues down below. On Mon, Aug 30, 2010 at 02:04:59PM +0200, Esben Haabendal wrote: Please add some change log, a couple of sentences would work. Signed-off-by: Esben Haabendal e...@doredevelopment.dk --- drivers/mmc/host/of_mmc_spi.c | 25 +++-- 1 files changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/of_mmc_spi.c b/drivers/mmc/host/of_mmc_spi.c index 1247e5d..e872b61 100644 --- a/drivers/mmc/host/of_mmc_spi.c +++ b/drivers/mmc/host/of_mmc_spi.c @@ -34,6 +34,7 @@ enum { struct of_mmc_spi { int gpios[NUM_GPIOS]; bool alow_gpios[NUM_GPIOS]; + int detect_irq; struct mmc_spi_platform_data pdata; }; @@ -61,6 +62,20 @@ static int of_mmc_spi_get_ro(struct device *dev) return of_mmc_spi_read_gpio(dev, WP_GPIO); } +static int of_mmc_spi_init(struct device *dev, + irqreturn_t (*irqhandler)(int, void *), void *mmc) +{ + struct of_mmc_spi *oms = to_of_mmc_spi(dev); Please add an empty line here. + return request_threaded_irq( + oms-detect_irq, NULL, irqhandler, 0, dev_name(dev), mmc); I'd write it this way: return request_threaded_irq(oms-detect_irq, NULL, irqhandler, 0, dev_name(dev), mmc); But that's a matter of taste. +} + +static void of_mmc_spi_exit(struct device *dev, void *mmc) +{ + struct of_mmc_spi *oms = to_of_mmc_spi(dev); Empty line. + free_irq(oms-detect_irq, mmc); +} + struct mmc_spi_platform_data *mmc_spi_get_pdata(struct spi_device *spi) { struct device *dev = spi-dev; @@ -121,8 +136,14 @@ struct mmc_spi_platform_data *mmc_spi_get_pdata(struct spi_device *spi) if (gpio_is_valid(oms-gpios[WP_GPIO])) oms-pdata.get_ro = of_mmc_spi_get_ro; - /* We don't support interrupts yet, let's poll. */ - oms-pdata.caps |= MMC_CAP_NEEDS_POLL; + oms-detect_irq = irq_of_parse_and_map(np, 0); + if (oms-detect_irq != NO_IRQ) { I'd write if (oms-detect_irq), which is a bit more natural (and still correct, 0 is the only invalid VIRQ number). + oms-pdata.init = of_mmc_spi_init; + oms-pdata.exit = of_mmc_spi_exit; + } + else { } else { Plus, please add an appropriate interrupts = bindings into Documentation/powerpc/dts-bindings/mmc-spi-slot.txt. And on the next resend, be sure to add Andrew Morton a...@linux-foundation.org, David Brownell dbrown...@users.sourceforge.net, and linux-...@vger.kernel.org the Cc list. Thanks! -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] gpiolib: Add 'struct gpio_chip' forward declaration for !GPIOLIB case
With CONFIG_GPIOLIB=n, the 'struct gpio_chip' is not declared, so the following pops up on PowerPC: cc1: warnings being treated as errors In file included from arch/powerpc/platforms/52xx/mpc52xx_common.c:19: include/linux/of_gpio.h:74: warning: 'struct gpio_chip' declared inside parameter list include/linux/of_gpio.h:74: warning: its scope is only this definition or declaration, which is probably not what you want include/linux/of_gpio.h:75: warning: 'struct gpio_chip' declared inside parameter list make[2]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error 1 This patch fixes the issue by providing the proper forward declaration. Signed-off-by: Anton Vorontsov cbouatmai...@gmail.com --- On Tue, Aug 24, 2010 at 04:26:08PM +1000, Benjamin Herrenschmidt wrote: I get that with my current stuff: cc1: warnings being treated as errors In file included from [..]/mpc52xx_common.c:19: of_gpio.h:74: error: ‘struct gpio_chip’ declared inside parameter list [...] make[3]: *** Waiting for unfinished jobs That's because with GPIOCHIP=n no one declares struct gpio_chip. It should be either of_gpio.h or gpio.h. Let's make it gpio.h, as this feels more generic, and we already have some !GPIOLIB handling in there. include/linux/gpio.h |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/include/linux/gpio.h b/include/linux/gpio.h index 03f616b..85207d2 100644 --- a/include/linux/gpio.h +++ b/include/linux/gpio.h @@ -15,6 +15,12 @@ struct device; /* + * Some code might rely on the declaration. Still, it is illegal + * to dereference it for !GPIOLIB case. + */ +struct gpio_chip; + +/* * Some platforms don't support the GPIO programming interface. * * In case some driver uses it anyway (it should normally have -- 1.7.0.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] powerpc/85xx: Cleanup QE initialization for MPC85xxMDS boards
On Wed, Aug 18, 2010 at 02:31:42PM -0500, Timur Tabi wrote: On Tue, Jun 8, 2010 at 2:55 PM, Anton Vorontsov avoront...@mvista.com wrote: The mpc85xx_mds_setup_arch() function is incomprehensible and unmaintainable. Factor out all QE specific stuff into mpc85xx_mds_qe_init() and mpc85xx_mds_reset_ucc_phys(). Also move QE stuff out of mpc85xx_mds_pic_init(). The diff is unreadable, but only because the code was so. ;-) It should be better now, and less indented. Signed-off-by: Anton Vorontsov avoront...@mvista.com --- This patch introduces breaks mpc85xx_smp_defconfig: CC arch/powerpc/platforms/85xx/mpc85xx_mds.o arch/powerpc/platforms/85xx/mpc85xx_mds.c: In function 'mpc85xx_mds_setup_arch': arch/powerpc/platforms/85xx/mpc85xx_mds.c:367: error: 'np' undeclared (first use in this function) arch/powerpc/platforms/85xx/mpc85xx_mds.c:367: error: (Each undeclared identifier is reported only once arch/powerpc/platforms/85xx/mpc85xx_mds.c:367: error: for each function it appears in.) Thanks for the report, apparently I tested my patch without CONFIG_PCI... But the issue should be already fixed by powerpc/85xx: Fix compile error in mpc85xx_mds.c http://patchwork.ozlabs.org/patch/60933/ (Though, not in Linus' tree yet.) Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: How to use mpc8xxx_gpio.c device driver
Hi, On Fri, Aug 13, 2010 at 03:29:11PM +0530, Ravi Gupta wrote: [...] Thanks for the reply. I had added the entries for gpio pin 9 for both controllers(I was not sure with controller's pin is connected to LED, but now I know it is pin no. 233 i.e 224+9) in the mpc8377_rdb.dts file. Below is a portion of my dts file, I have attached the complete dts file as attachment. i...@e000 { [...] * l...@0 { compatible = gpio-leds; label = hdd; gpios = gpio1 9 0; }; What kernel version you look at? Please see the latest kernel, it has MCU GPIO LED nodes already, and you can just add some additional nodes. http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=arch/powerpc/boot/dts/mpc8377_rdb.dts#l490 [...] Also I have enabled drivers/leds/leds-gpio.c in my kernel. To test whether the leds entires in dts file get attached to leds-gpio driver, I added printks in the probe function of the driver. static int gpio_led_probe(struct platform_device *pdev) { struct gpio_led_platform_data *pdata = pdev-dev.platform_data; struct gpio_led *cur_led; struct gpio_led_data *leds_data, *led_dat; int i, ret = 0; *printk(KERN_INFO led: inside gpio_led_probe.\n);* You have put the printk into the wrong function. It should have been of_gpio_leds_probe(): http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/leds/leds-gpio.c#l227 If you don't have that function then you use too old kernel. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: How to use mpc8xxx_gpio.c device driver
Hi, On Wed, Aug 11, 2010 at 06:57:16PM +0530, Ravi Gupta wrote: I am new to device driver development. I am trying to access the GPIO of MPC837xERDB eval board. I have upgraded its kernel to linux-2.6.28.9 and enable support for mpc8xxx_gpio.c. On boot up, it successfully detect two gpio controllers. Now my question is how I am going to use it to communicate with the gpio pins? Do I have to modify the code in mpc8xxx_gpio.c file to do whatever I want to do with gpios or I can use the standard gpio API provided in kernel ( gpio_request()/gpio_free() ). I also tries the standard kernel API, but it fails. Here is my code : No, you don't have to modify anything, and yes, you can use standard kernel GPIO API. #include linux/module.h #include linux/errno.h /* error codes */ #include linux/gpio.h static __init int sample_module_init(void) { int ret; int i; for (i=1; i32; i++) { ret = gpio_request(i, Sample Driver); Before issing gpio_request() you must get GPIO number from the of_get_gpio() or of_get_gpio_flags() calls (the _flags variant will also retreive flags such as 'active-low'). The calls assume that you have gpio = specifier in the device tree, see arch/powerpc/boot/dts/mpc8377_rdb.dts's leds node as an example. As you want GPIO LEDs, you can use drivers/leds/leds-gpio.c (see of_gpio_leds_probe() call, it gets gpio numbers via of_get_gpio_flags() and then requests them via gpio_request()). Also see Documentation/powerpc/dts-bindings/gpio/gpio.txt Documentation/powerpc/dts-bindings/gpio/led.txt if (ret) { printk(KERN_WARNING sample_driver: unable to request GPIO_PG%d\n, i); //return ret; } } return 0; } On Wed, Aug 11, 2010 at 07:49:40PM +0530, Ravi Gupta wrote: Also, when I try to export a gpio in sysfs echo 9 /sys/class/gpio/export The gpio numbers are global, i.e. GPIO controller base + GPIO number within the controller. [...] drwxr-xr-x3 root root0 Jan 1 00:00 gpiochip192 So, if you want GPIO9 within gpiochip192, you should issue echo 201 export. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] booting-without-of: Remove nonexistent chapters from TOC, fix numbering
Marvell and GPIO bindings live in their own files, so the TOC should not mention them. Also fix chapters numbering. Signed-off-by: Anton Vorontsov avoront...@mvista.com --- Documentation/powerpc/booting-without-of.txt | 31 + 1 files changed, 2 insertions(+), 29 deletions(-) diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt index 46d2210..3f454b7 100644 --- a/Documentation/powerpc/booting-without-of.txt +++ b/Documentation/powerpc/booting-without-of.txt @@ -49,40 +49,13 @@ Table of Contents f) MDIO on GPIOs g) SPI busses - VII - Marvell Discovery mv64[345]6x System Controller chips -1) The /system-controller node -2) Child nodes of /system-controller - a) Marvell Discovery MDIO bus - b) Marvell Discovery ethernet controller - c) Marvell Discovery PHY nodes - d) Marvell Discovery SDMA nodes - e) Marvell Discovery BRG nodes - f) Marvell Discovery CUNIT nodes - g) Marvell Discovery MPSCROUTING nodes - h) Marvell Discovery MPSCINTR nodes - i) Marvell Discovery MPSC nodes - j) Marvell Discovery Watch Dog Timer nodes - k) Marvell Discovery I2C nodes - l) Marvell Discovery PIC (Programmable Interrupt Controller) nodes - m) Marvell Discovery MPP (Multipurpose Pins) multiplexing nodes - n) Marvell Discovery GPP (General Purpose Pins) nodes - o) Marvell Discovery PCI host bridge node - p) Marvell Discovery CPU Error nodes - q) Marvell Discovery SRAM Controller nodes - r) Marvell Discovery PCI Error Handler nodes - s) Marvell Discovery Memory Controller nodes - - VIII - Specifying interrupt information for devices + VII - Specifying interrupt information for devices 1) interrupts property 2) interrupt-parent property 3) OpenPIC Interrupt Controllers 4) ISA Interrupt Controllers - IX - Specifying GPIO information for devices -1) gpios property -2) gpio-controller nodes - - X - Specifying device power management information (sleep property) + VIII - Specifying device power management information (sleep property) Appendix A - Sample SOC node for MPC8540 -- 1.7.0.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3 v2] dts: Add ESDHC weird voltage bits workaround
On Tue, Aug 03, 2010 at 11:11:12AM +0800, Roy Zang wrote: P4080 ESDHC controller does not support 1.8V and 3.0V voltage. but the host controller capabilities register wrongly set the bits. This patch adds the workaround to correct the weird voltage setting bits. Only 3.3V voltage is supported for P4080 ESDHC controller. Signed-off-by: Roy Zang tie-fei.z...@freescale.com Acked-by: Anton Vorontsov cbouatmai...@gmail.com Btw, where is implementation for the voltage-ranges handling? --- arch/powerpc/boot/dts/p4080ds.dts |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/boot/dts/p4080ds.dts b/arch/powerpc/boot/dts/p4080ds.dts index efa0091..2f0de24 100644 --- a/arch/powerpc/boot/dts/p4080ds.dts +++ b/arch/powerpc/boot/dts/p4080ds.dts @@ -280,6 +280,7 @@ reg = 0x114000 0x1000; interrupts = 48 2; interrupt-parent = mpic; + voltage-ranges = 3300 3300; sdhci,auto-cmd12; }; -- 1.5.6.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3 v2] dts: Add sdhci,auto-cmd12 field for p4080 device tree
On Tue, Aug 03, 2010 at 11:11:11AM +0800, Roy Zang wrote: Signed-off-by: Roy Zang tie-fei.z...@freescale.com --- Documentation/powerpc/dts-bindings/fsl/esdhc.txt |2 ++ arch/powerpc/boot/dts/p4080ds.dts|1 + 2 files changed, 3 insertions(+), 0 deletions(-) diff --git a/Documentation/powerpc/dts-bindings/fsl/esdhc.txt b/Documentation/powerpc/dts-bindings/fsl/esdhc.txt index 8a00407..64bcb8b 100644 --- a/Documentation/powerpc/dts-bindings/fsl/esdhc.txt +++ b/Documentation/powerpc/dts-bindings/fsl/esdhc.txt @@ -14,6 +14,8 @@ Required properties: reports inverted write-protect state; - sdhci,1-bit-only : (optional) specifies that a controller can only handle 1-bit data transfers. + - sdhci,auto-cmd12: (optional) specifies that a controller can +only handle auto CMD12. Acked-by: Anton Vorontsov cbouatmai...@gmail.com Example: diff --git a/arch/powerpc/boot/dts/p4080ds.dts b/arch/powerpc/boot/dts/p4080ds.dts index 6b29eab..efa0091 100644 --- a/arch/powerpc/boot/dts/p4080ds.dts +++ b/arch/powerpc/boot/dts/p4080ds.dts @@ -280,6 +280,7 @@ reg = 0x114000 0x1000; interrupts = 48 2; interrupt-parent = mpic; + sdhci,auto-cmd12; }; i...@118000 { -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3 v2] sdhci: Add auto CMD12 support for eSDHC driver
On Wed, Aug 04, 2010 at 07:02:56PM -0600, Grant Likely wrote: On Mon, Aug 2, 2010 at 9:11 PM, Roy Zang tie-fei.z...@freescale.com wrote: From: Jerry Huang chang-ming.hu...@freescale.com Add auto CMD12 command support for eSDHC driver. This is needed by P4080 and P1022 for block read/write. Manual asynchronous CMD12 abort operation causes protocol violations on these silicons. Signed-off-by: Jerry Huang chang-ming.hu...@freescale.com Signed-off-by: Roy Zang tie-fei.z...@freescale.com --- drivers/mmc/host/sdhci-of-core.c | 4 drivers/mmc/host/sdhci.c | 14 -- drivers/mmc/host/sdhci.h | 2 ++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index c6d1bd8..a92566e 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -817,8 +817,12 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host, WARN_ON(!host-data); mode = SDHCI_TRNS_BLK_CNT_EN; - if (data-blocks 1) - mode |= SDHCI_TRNS_MULTI; + if (data-blocks 1) { + if (host-quirks SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12) + mode |= SDHCI_TRNS_MULTI | SDHCI_TRNS_ACMD12; + else + mode |= SDHCI_TRNS_MULTI; nit: + mode |= SDHCI_TRNS_MULTI; + if (host-quirks SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12) + mode |= SDHCI_TRNS_ACMD12; Clearer, no? Much clearer. I would prefer the nit incorporated. Another nit: @@ -154,6 +154,10 @@ static int __devinit sdhci_of_probe(struct of_device *ofdev, host-ops = sdhci_of_data-ops; } + if (of_get_property(np, sdhci,auto-cmd12, NULL)) + host-quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12; + + ^^ No need for the two empty lines. if (of_get_property(np, sdhci,1-bit-only, NULL)) Though, technically the patch looks OK, feel free to add my Acked-by: Anton Vorontsov cbouatmai...@gmail.com on the next resend (if any). Thanks Roy! -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios
On Sat, Aug 07, 2010 at 06:40:22PM -0600, Grant Likely wrote: [...] static int __init mpc8xxx_add_gpiochips(void) { +const struct of_device_id *id; struct device_node *np; -for_each_compatible_node(np, NULL, fsl,mpc8349-gpio) -mpc8xxx_add_controller(np); - -for_each_compatible_node(np, NULL, fsl,mpc8572-gpio) -mpc8xxx_add_controller(np); - -for_each_compatible_node(np, NULL, fsl,mpc8610-gpio) +for_each_matching_node(np, mpc8xxx_gpio_ids) { +id = of_match_node(mpc8xxx_gpio_ids, np); +if (id) +np-data = id-data; mpc8xxx_add_controller(np); +} [...] Actually, there is absolutely no reason to keep mpc8xxx_add_gpiochip() as a separate function with the simplification of mpc8xxx_add_gpiochips(). I'd simplify the whole thing by merging the two functions together. You mean mpc8xxx_add_controller()? Putting 65-line function on a second indentation level, inside the for loop... sounds like a bad idea. -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc/85xx: Add P1021 PCI IDs and quirks
This is needed for proper PCI-E support on P1021 SoCs. Signed-off-by: Anton Vorontsov avoront...@mvista.com --- arch/powerpc/sysdev/fsl_pci.c |2 ++ include/linux/pci_ids.h |2 ++ 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c index 209384b..4ae9332 100644 --- a/arch/powerpc/sysdev/fsl_pci.c +++ b/arch/powerpc/sysdev/fsl_pci.c @@ -399,6 +399,8 @@ DECLARE_PCI_FIXUP_HEADER(0x1957, PCI_DEVICE_ID_P1013E, quirk_fsl_pcie_header); DECLARE_PCI_FIXUP_HEADER(0x1957, PCI_DEVICE_ID_P1013, quirk_fsl_pcie_header); DECLARE_PCI_FIXUP_HEADER(0x1957, PCI_DEVICE_ID_P1020E, quirk_fsl_pcie_header); DECLARE_PCI_FIXUP_HEADER(0x1957, PCI_DEVICE_ID_P1020, quirk_fsl_pcie_header); +DECLARE_PCI_FIXUP_HEADER(0x1957, PCI_DEVICE_ID_P1021E, quirk_fsl_pcie_header); +DECLARE_PCI_FIXUP_HEADER(0x1957, PCI_DEVICE_ID_P1021, quirk_fsl_pcie_header); DECLARE_PCI_FIXUP_HEADER(0x1957, PCI_DEVICE_ID_P1022E, quirk_fsl_pcie_header); DECLARE_PCI_FIXUP_HEADER(0x1957, PCI_DEVICE_ID_P1022, quirk_fsl_pcie_header); DECLARE_PCI_FIXUP_HEADER(0x1957, PCI_DEVICE_ID_P2010E, quirk_fsl_pcie_header); diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index c81eec4..fc987b4 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -2300,6 +2300,8 @@ #define PCI_DEVICE_ID_P20100x0079 #define PCI_DEVICE_ID_P1020E 0x0100 #define PCI_DEVICE_ID_P10200x0101 +#define PCI_DEVICE_ID_P1021E 0x0102 +#define PCI_DEVICE_ID_P10210x0103 #define PCI_DEVICE_ID_P1011E 0x0108 #define PCI_DEVICE_ID_P10110x0109 #define PCI_DEVICE_ID_P1022E 0x0110 -- 1.7.0.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] mmc_spi: Fix unterminated of_match_table
commit 2ffe8c5f323c3b9749bf7bc2375d909d20bdbb15 (of: refactor of_modalias_node() and remove explicit match table), introduced an unterminated of_match_table, which may cause kernel to oops. This patch fixes the issue by adding an empty device ID. Signed-off-by: Anton Vorontsov avoront...@mvista.com --- drivers/mmc/host/mmc_spi.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c index 7b0f3ef..1145ea0 100644 --- a/drivers/mmc/host/mmc_spi.c +++ b/drivers/mmc/host/mmc_spi.c @@ -1536,6 +1536,7 @@ static int __devexit mmc_spi_remove(struct spi_device *spi) #if defined(CONFIG_OF) static struct of_device_id mmc_spi_of_match_table[] __devinitdata = { { .compatible = mmc-spi-slot, }, + {}, }; #endif -- 1.7.0.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3 v2] mmc: Add ESDHC weird voltage bits workaround
On Mon, Aug 02, 2010 at 02:19:58PM +0800, Zang Roy-R61911 wrote: [...] For p4080 it will be 'voltage-ranges = 3200 3400;'. So, with voltage-ranges we can do fine grained VDD control without introducing anything new. why not voltage-ranges = 3300 3300; Right you are, both will be 3300. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Issues to access Compact Flash Card on MPC8360E
On Mon, Aug 02, 2010 at 07:44:22PM +0530, Atul Deshmukh wrote: Thanks a lot Anton, From the dts entry given below, local...@e0005000 { #address-cells = 2; #size-cells = 1; compatible = fsl,mpc8349e-localbus, fsl,pq2pro-localbus; reg = 0xe0005000 0xd8; ranges = 0x3 0x0 0xf000 0x210; p...@3,0 { compatible = fsl,mpc8349emitx-pata, ata-generic; reg = 0x3 0x0 0x10 0x3 0x20c 0x4; reg-shift = 1; pio-mode = 6; interrupts = 23 0x8; interrupt-parent = ipic; }; }; we can conclude that it uses ata-generic SATA/PATA controlelr driver which How did you come to this conclusion? From the node above it's IMHO pretty clear that IDE (PATA) is on the localbus. The driver is drivers/ata/pata_of_platform.c. controls PCI-based IDE-controller where we can plug in our CF card...Am I right??? Nope, no PCI involved. CF is almost* directly connected to the localbus. But in our design we don't use any controller we directly connects CF card to local bus where UPM controls it.. Yes, that's exactly how CF is done on MPC8349EmITX boards. Can you please explain how the interface is implemented in MPC8349.. Via localbus + UPM. * 'almost' is because there are some buffers and inverters, see schematics: http://www.freescale.com/files/32bit/hardware_tools/schematics/MPC8349EMITXESCH.pdf -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] edac: mpc85xx: Add support for new MPCxxx/Pxxxx EDAC controllers (fix)
On Wed, Jul 21, 2010 at 06:21:08PM -0500, Scott Wood wrote: [...] + { .compatible = fsl,p4080-l2-cache-controller, }, L2 on the p4080 is quite different from those other chips. It's part of the core, controlled by SPRs. erm, was that an ack or a nack? NACK, p4080 doesn't belong in this table, at least not its L2. L3 on p4080 is similar to L2 on these other chips, though, and it wouldn't take much to get this driver working on it -- but the match table entry should wait until the differences are accommodated. Signed-off-by: Anton Vorontsov avoront...@mvista.com --- Scott, thanks for catching this! Andrew, please merge this patch into edac-mpc85xx-add-support-for-new-mpcxxx-p-edac-controllers.patch Thanks! drivers/edac/mpc85xx_edac.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c index cfa86f7..b178cfa 100644 --- a/drivers/edac/mpc85xx_edac.c +++ b/drivers/edac/mpc85xx_edac.c @@ -652,7 +652,6 @@ static struct of_device_id mpc85xx_l2_err_of_match[] = { { .compatible = fsl,p1020-l2-cache-controller, }, { .compatible = fsl,p1021-l2-cache-controller, }, { .compatible = fsl,p2020-l2-cache-controller, }, - { .compatible = fsl,p4080-l2-cache-controller, }, {}, }; MODULE_DEVICE_TABLE(of, mpc85xx_l2_err_of_match); -- 1.7.0.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Which microcode patch for MPC870?
Hello, On Thu, Jul 29, 2010 at 11:40:21PM -0700, Shawn Jin wrote: Hi, Which microcode patch should be selected for MPC870? In the old 2.4 kernel, the CONFIG_UCODE_PATCH was selected. What's the corresponding config: CONFIG_USB_SOF_UCODE_PATCH or CONFIG_I2C_SPI_UCODE_PATCH or CONFIG_I2C_SPI_SMC1_UCODE_PATCH? Since my board doesn't have USB, I believe USB microcode is irrelevant here. So it comes down the other two choices. Of course do I really need the patch? My board has I2C and SMC1, but no SPI. I chose CONFIG_I2C_SPI_UCODE_PATCH as an experiment but got the following compilation error: These errors were fixed by http://patchwork.ozlabs.org/patch/58262/ and http://patchwork.ozlabs.org/patch/58263/ Thanks, CC arch/powerpc/sysdev/micropatch.o arch/powerpc/sysdev/micropatch.c: In function 'cpm_load_patch': arch/powerpc/sysdev/micropatch.c:629: error: expected '=', ',', ';', 'asm' or '__attribute__' before '*' token arch/powerpc/sysdev/micropatch.c:629: error: 'spp' undeclared (first use in this function) arch/powerpc/sysdev/micropatch.c:629: error: (Each undeclared identifier is reported only once arch/powerpc/sysdev/micropatch.c:629: error: for each function it appears in.) cc1: warnings being treated as errors arch/powerpc/sysdev/micropatch.c:630: warning: ISO C90 forbids mixed declarations and code arch/powerpc/sysdev/micropatch.c:671: error: 'spi_t' undeclared (first use in this function) arch/powerpc/sysdev/micropatch.c:671: error: expected expression before ')' token arch/powerpc/sysdev/micropatch.c:630: warning: unused variable 'smp' make[1]: *** [arch/powerpc/sysdev/micropatch.o] Error 1 Obviously there is no spi_t declaration in 2.6.33.5. So where is this spi_t declared? -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3 v2] mmc: Add ESDHC weird voltage bits workaround
On Fri, Jul 30, 2010 at 11:52:57AM +0800, Roy Zang wrote: P4080 ESDHC controller does not support 1.8V and 3.0V voltage. but the host controller capabilities register wrongly set the bits. This patch adds the workaround to correct the weird voltage setting bits. Signed-off-by: Roy Zang tie-fei.z...@freescale.com --- [...] diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c index 0c30242..1f3913d 100644 --- a/drivers/mmc/host/sdhci-of-core.c +++ b/drivers/mmc/host/sdhci-of-core.c @@ -164,6 +164,10 @@ static int __devinit sdhci_of_probe(struct of_device *ofdev, if (sdhci_of_wp_inverted(np)) host-quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT; + if (of_device_is_compatible(np, fsl,p4080-esdhc)) + host-quirks |= (SDHCI_QUIRK_QORIQ_NO_VDD_180 + |SDHCI_QUIRK_QORIQ_NO_VDD_300); + It should be two properties, something like sdhci,no-vdd-180 and sdhci,no-vdd-300. But it might be even better: we have voltage-ranges for mmc-spi case, see Documentation/powerpc/dts-bindings/mmc-spi-slot.txt. If voltage-ranges specified, then we use it, not capabilities register. For p4080 it will be 'voltage-ranges = 3200 3400;'. So, with voltage-ranges we can do fine grained VDD control without introducing anything new. As for implementation, you might just factor out voltage-ranges parsing from drivers/mmc/host/of_mmc_spi.c, and then in sdhci driver you could do. if (host-ocr_avail) mmc-ocr_avail = host-ocr_avail. clk = of_get_property(np, clock-frequency, size); if (clk size == sizeof(*clk) *clk) of_host-clock = *clk; diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 1424d08..a667790 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1699,6 +1699,14 @@ int sdhci_add_host(struct sdhci_host *host) caps = sdhci_readl(host, SDHCI_CAPABILITIES); + /* Workaround for P4080 host controller capabilities + * 1.8V and 3.0V do not supported*/ + if (host-quirks SDHCI_QUIRK_QORIQ_NO_VDD_180) The point of making NO_VDD stuff is to make these quirks chip-agnostic. Ideally, sdhci.c should never know about particular chips. So, you shouldn't name quirks with QORIQ. -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3] mmc: Auto CMD12 support for eSDHC driver
On Wed, Jul 28, 2010 at 01:42:33PM +0800, Roy Zang wrote: [...] + if (host-quirks SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12) + if (mrq-stop) { + mrq-data-stop = NULL; + mrq-stop = NULL; + } Please put additional curly braces for the first 'if' statement. host-mrq = mrq; diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index c846813..aa112aa 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -2,6 +2,7 @@ * linux/drivers/mmc/host/sdhci.h - Secure Digital Host Controller Interface driver * * Copyright (C) 2005-2008 Pierre Ossman, All Rights Reserved. + * Copyright 2010 Freescale Semiconductor, Inc. Wrong count of spaces after '*'. Also, according to git shortlog it's Freescale's first patch to sdhci core, and the patch is quite trivial. IANAL, but please refrain from adding authorship or copyright notices unless you have done some major contribution(s). * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -240,6 +241,8 @@ struct sdhci_host { #define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN(125) /* Controller cannot support End Attribute in NOP ADMA descriptor */ #define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC(126) +/* Controller uses Auto CMD12 command to stop the transfer */ +#define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12 (127) Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev