Re: [RFC PATCH 1/7] mmc: sdhci: add quirk for broken 3.0V support

2014-06-20 Thread Anton Vorontsov
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

2013-08-22 Thread Anton Vorontsov
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

2013-08-22 Thread Anton Vorontsov
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

2013-08-22 Thread Anton Vorontsov
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

2013-08-22 Thread Anton Vorontsov
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

2013-08-08 Thread Anton Vorontsov
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

2013-08-08 Thread Anton Vorontsov
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

2013-07-26 Thread Anton Vorontsov
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

2013-07-26 Thread Anton Vorontsov
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

2013-07-15 Thread Anton Vorontsov
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

2013-07-12 Thread Anton Vorontsov
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

2013-05-23 Thread Anton Vorontsov
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

2012-10-26 Thread Anton Vorontsov
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

2012-10-25 Thread Anton Vorontsov
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

2012-10-23 Thread Anton Vorontsov
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

2012-09-11 Thread Anton Vorontsov
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

2012-09-11 Thread Anton Vorontsov
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

2012-05-04 Thread Anton Vorontsov
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()

2012-05-04 Thread Anton Vorontsov
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

2012-04-23 Thread Anton Vorontsov
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

2012-04-23 Thread Anton Vorontsov
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()

2012-04-23 Thread Anton Vorontsov
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()

2012-04-23 Thread Anton Vorontsov
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()

2012-04-23 Thread Anton Vorontsov
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

2012-04-23 Thread Anton Vorontsov
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()

2012-04-23 Thread Anton Vorontsov
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

2012-04-23 Thread Anton Vorontsov
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

2012-04-23 Thread Anton Vorontsov
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

2012-04-23 Thread Anton Vorontsov
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

2012-04-23 Thread Anton Vorontsov
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

2012-03-24 Thread Anton Vorontsov
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

2011-09-09 Thread Anton Vorontsov
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.

2011-08-23 Thread Anton Vorontsov
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

2011-08-12 Thread Anton Vorontsov
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

2011-07-22 Thread Anton Vorontsov
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

2011-07-05 Thread Anton Vorontsov
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

2011-06-02 Thread Anton Vorontsov
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

2011-06-01 Thread Anton Vorontsov
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

2011-06-01 Thread Anton Vorontsov
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

2011-05-30 Thread Anton Vorontsov
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

2011-05-05 Thread Anton Vorontsov
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

2011-05-05 Thread Anton Vorontsov
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

2011-05-05 Thread Anton Vorontsov
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

2011-05-05 Thread Anton Vorontsov
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

2011-01-27 Thread Anton Vorontsov
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

2010-12-28 Thread Anton Vorontsov
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

2010-11-16 Thread Anton Vorontsov
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

2010-11-15 Thread Anton Vorontsov
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.

2010-11-12 Thread Anton Vorontsov
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

2010-11-12 Thread Anton Vorontsov
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)

2010-10-07 Thread Anton Vorontsov
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)

2010-10-07 Thread Anton Vorontsov
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

2010-10-01 Thread Anton Vorontsov
);' 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

2010-10-01 Thread Anton Vorontsov
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

2010-09-30 Thread Anton Vorontsov
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

2010-09-28 Thread Anton Vorontsov
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

2010-09-24 Thread Anton Vorontsov
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

2010-09-20 Thread Anton Vorontsov
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

2010-09-20 Thread Anton Vorontsov
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

2010-09-16 Thread Anton Vorontsov
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

2010-09-16 Thread Anton Vorontsov
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

2010-09-16 Thread Anton Vorontsov
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

2010-09-16 Thread Anton Vorontsov
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

2010-09-16 Thread Anton Vorontsov
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

2010-09-10 Thread Anton Vorontsov
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 ?

2010-09-10 Thread Anton Vorontsov
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

2010-09-09 Thread Anton Vorontsov
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

2010-09-09 Thread Anton Vorontsov
,
   },
   {}
  };
  
 -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

2010-09-09 Thread Anton Vorontsov
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

2010-09-09 Thread Anton Vorontsov
)
 + 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

2010-09-08 Thread Anton Vorontsov
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

2010-09-08 Thread Anton Vorontsov
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

2010-09-06 Thread Anton Vorontsov
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

2010-09-06 Thread Anton Vorontsov
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

2010-09-03 Thread Anton Vorontsov
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

2010-09-03 Thread Anton Vorontsov
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

2010-09-03 Thread Anton Vorontsov
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 ?

2010-09-03 Thread Anton Vorontsov
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

2010-09-02 Thread Anton Vorontsov
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

2010-09-02 Thread Anton Vorontsov
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

2010-08-31 Thread Anton Vorontsov
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

2010-08-31 Thread Anton Vorontsov
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

2010-08-30 Thread Anton Vorontsov
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

2010-08-24 Thread Anton Vorontsov
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

2010-08-19 Thread Anton Vorontsov
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

2010-08-13 Thread Anton Vorontsov
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

2010-08-11 Thread Anton Vorontsov
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

2010-08-11 Thread Anton Vorontsov
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

2010-08-09 Thread Anton Vorontsov
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

2010-08-09 Thread Anton Vorontsov
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

2010-08-09 Thread Anton Vorontsov
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

2010-08-08 Thread Anton Vorontsov
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

2010-08-08 Thread Anton Vorontsov
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

2010-08-08 Thread Anton Vorontsov
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

2010-08-02 Thread Anton Vorontsov
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

2010-08-02 Thread Anton Vorontsov
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)

2010-08-01 Thread Anton Vorontsov
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?

2010-07-30 Thread Anton Vorontsov
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

2010-07-30 Thread Anton Vorontsov
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

2010-07-30 Thread Anton Vorontsov
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


  1   2   3   4   5   6   7   8   9   10   >