Re: [PATCH v2] sdhci: Add support for hosts that are only capable of 1-bit transfers

2009-06-19 Thread Pierre Ossman
On Thu, 18 Jun 2009 00:14:08 +0400
Anton Vorontsov avoront...@ru.mvista.com wrote:

 Some hosts (hardware configurations, or particular SD/MMC slots) may
 not support 4-bit bus. For example, on MPC8569E-MDS boards we can
 switch between serial (1-bit only) and nibble (4-bit) modes, thought
 we have to disable more peripherals to work in 4-bit mode.
 
 Along with some small core changes, this patch modifies sdhci-of
 driver, so that now it looks for sdhci,1-bit-only property in the
 device-tree, and if specified we enable a proper quirk.
 
 Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
 ---

Patch merged.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] sdhci: Add support for hosts that are only capable of 1-bit transfers

2009-06-13 Thread Pierre Ossman
On Fri, 12 Jun 2009 00:15:45 +0400
Anton Vorontsov avoront...@ru.mvista.com wrote:

 Some hosts (hardware configurations, or particular SD/MMC slots) may
 not support 4-bit bus. For example, on MPC8569E-MDS boards we can
 switch between serial (1-bit only) and nibble (4-bit) modes, thought
 we have to disable more peripherals to work in 4-bit mode.
 
 Along with some small core changes, this patch modifies sdhci-of
 driver, so that now it looks for mode property in the device-tree.
 
 Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
 ---
 
 Pierre, I'm not sure if a quirk would be appropriate here. If so,
 I can redo the patch with FORCE_1_BIT_DATA quirk.
 

I'd prefer a quirk, yes. 4-bit support is mandated so this would be a
deviation from the spec and such should always be handled by quirks for
clarity.

(I do think it is silly that they made it mandatory though considering
the embedded market)

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] mmc: Fix the wrong accessor to HOSTVER register

2009-06-03 Thread Pierre Ossman
On Tue, 2 Jun 2009 21:26:44 +0400
Anton Vorontsov avoront...@ru.mvista.com wrote:

 On Wed, May 06, 2009 at 06:40:07PM +0800, Dave Liu wrote:
  Freescale eSDHC controller has the special order for
  the HOST version register. that is not same as the other's
  registers. The address of HOSTVER in spec is 0xFE, and
  we need use the in_be16(0xFE) to access it, not in_be16(0xFC).
  
  Signed-off-by: Dave Liu dave...@freescale.com
 
 Sorry for the delay Dave. This patch is surely
 
 Acked-by: Anton Vorontsov avoront...@ru.mvista.com
 

Queued.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] mmc: Add fsl,esdhc as a valid compatible to bind against

2009-05-22 Thread Pierre Ossman
On Fri,  8 May 2009 08:52:49 -0500
Kumar Gala ga...@kernel.crashing.org wrote:

 We plan to use fsl,esdhc going forward as the base compatible so update
 the driver to bind against it.
 
 Signed-off-by: Kumar Gala ga...@kernel.crashing.org
 ---

Applied.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH v3 0/11] FSL eSDHC support

2009-03-22 Thread Pierre Ossman
On Tue, 17 Mar 2009 00:13:06 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 Hi all,
 
 Here comes another version, let's hope that one is final. ;-)
 

I think so. I've queued it all for 2.6.30. Thanks :)

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 02/11] sdhci: Add support for bus-specific IO memory accessors

2009-03-08 Thread Pierre Ossman
On Wed, 4 Mar 2009 20:46:58 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 On Sat, Feb 21, 2009 at 04:57:57PM +0100, Pierre Ossman wrote:
  
  We can most likely do some micro-optimisation do make the compare part
  cheaper, but the point was to avoid a function call for all the
  properly implemented controllers out there. We could have a flag so
  that it only has to check host-flags, which will most likely be in the
  cache anyway.
  
  Overhead for eSDHC is not a concern in my book, what is interesting is
  how much this change slows things down for other controllers.
 
 OK, I see. Will the patch down below make you a little bit more happy
 wrt normal controllers? Two #ifdefs, but then there is absolutely
 zero overhead for the fully compliant SDHCI controllers.
 

I can't say this makes me happy either, but I think it's acceptable for
now so that we can move forward. I'd like a common code path for this
thing, but I think I'm going to have to put a bit more time into it
myself than I currently have available.

 (So far it's just on top of this series, but I can incorporate it
 into the sdhci: Add support for bus-specific IO memory accessors
 patch, if you like).
 

Please do. Have one patch add some code and another remove it in the
same set is just silly. :)

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 05/13] sdhci: Add support for card-detection polling

2009-03-08 Thread Pierre Ossman
On Wed, 4 Mar 2009 20:49:17 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 On Sat, Feb 21, 2009 at 04:58:21PM +0100, Pierre Ossman wrote:
  Just modify the if-clause and
  things will work.
 
 That would look horrid...
 
 if ((!(host-quirks  SDHCI_QUIRK_BROKEN_CARD_DETECTION) 
 !(sdhci_readl(host, SDHCI_PRESENT_STATE) 
 SDHCI_CARD_PRESENT)) ||
 (host-flags  SDHCI_DEVICE_DEAD)) {
 

There are worse ones in that code, but I see your point. :)

  Might want to add a comment also to make it more obvious what the
  if-clause does.
 
 Let's try to avoid the if-clause above? How about this:
 

Looks ok.

 @@ -1096,6 +1099,7 @@ out:
  static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
  {
   struct sdhci_host *host;
 + bool present;
   unsigned long flags;
  
   host = mmc_priv(mmc);

Can we use bool in the kernel?

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 07/13] sdhci: Add support for hosts with strict 32 bit addressing

2009-03-08 Thread Pierre Ossman
On Wed, 4 Mar 2009 20:48:45 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 But I see the point of confusion... Instead of teaching
 SDHCI core to work with 32 bits hosts, we'd better handle this
 in the eSDHC part, in the accessors.
 
 This is relatively trivial and should not cause much overhead
 (at least when using DMA), just a small state machine with
 the xfer mode register shadowed in software (plus, notice that
 this also handles BLOCK_SIZE, as I promised in another email):
 

Me like. Keeps my life a lot saner. :)

Just be aware that there is a remote risk of breakage as people hacking
on sdhci-core won't be aware of esdhc's, let's call it unique,
behaviour. Some testing now and then on your part would be prudent. :)

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 12/13] sdhci: Add quirk for controllers with max. block size up to 4096 bytes

2009-03-08 Thread Pierre Ossman
On Wed, 4 Mar 2009 20:47:44 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 
 I'll get rid of this particular patch, and put some BLOCK_SIZE
 magic into the writew accessor (to clean the DMA bits) instead.
 
 Though, I'll prepare another patch to force blksz to 2048, since
 eSDHC specifies 3 in the blksz capability bitfield, and that
 causes SDHCI core to fall back to the 512 byte blocks.
 

Ok.

  After all, is it ever used?
 
 Not sure, maybe `dd bs=' can use it? A bit lazy to check this
 right now, but from the quick tests, enabling/disabling blksz
 of 4096 bytes doesn't cause any performance change. At least
 with the ordinary SD cards.
 

Memory cards will not use this (at least not with the current
standards), as the block layer thinks in 512 byte blocks. Also, the
sector size propagates to user space in a way that causes filesystems
to behave differently, making cards incompatible with all other
operating systems (i.e. if we don't use 512 byte blocks).

So the only scenario where this might be used is SDIO, and I'm not sure
such big blocks are a win there either because of the overhead of
changing block size.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 11/11] mmc: Add OpenFirmware bindings for SDHCI driver

2009-03-08 Thread Pierre Ossman
On Thu, 5 Mar 2009 23:28:50 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 This patch adds a new driver: sdhci-of. The driver is similar to
 the sdhci-pci, it contains common probe code, and controller-specific
 ops and quirks.
 
 So far there are only Freescale eSDHC ops and quirks.
 
 Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
 Acked-by: Arnd Bergmann a...@arndb.de
 ---
  drivers/mmc/host/Kconfig|   10 ++
  drivers/mmc/host/Makefile   |1 +
  drivers/mmc/host/sdhci-of.c |  309 
 +++
  3 files changed, 320 insertions(+), 0 deletions(-)
  create mode 100644 drivers/mmc/host/sdhci-of.c
 

I'd like a MAINTAINERS entry for this (sub)driver.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 02/11] sdhci: Add support for bus-specific IO memory accessors

2009-02-21 Thread Pierre Ossman
On Fri, 13 Feb 2009 17:40:39 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 
 No, on eSDHC the registers are big-endian, 32-bit width, with, for
 example, two 16-bit logical registers packed into it.
 
 That is,
 
  0x4  0x5 0x6  0x7
 |:|
 | BLKCNT : BLKSZ  |
 |:|
  31  0
 
 ( The register looks wrong, right? BLKSZ should be at 0x4. But imagine
   that you swapped bytes in this 32 bit register... then the registers
   and their byte addresses will look normal. )
 
 So if we try to issue readw(SDHCI_BLOCK_SIZE), i.e. readw(0x4):
 
 - We'll read BLKCNT, while we wanted BLKSZ. This is because the
   address bits should be translated before we try word or byte
   reads/writes.
 - On powerpc read{l,w}() convert the read value from little-endian
   to big-endian byte order, which is wrong for our case (the
   register is big-endian already).
 
 That means that we have to convert address, but we don't want to
 convert the result of read/write ops.
 

*cries*

Now this is just incredibly horrible. Why the hell did they try to use
the sdhci interface and then do stupid things like this?

   +static inline void sdhci_writel(struct sdhci_host *host, u32 val, int 
   reg)
   +{
   + host-writel(host, val, reg);
   +}
  
  Having to override these are worst case scenario
 
 Hm. It's not a worst case scenario, it's a normal scenario for
 eSDHC. Why should we treat eSDHC as a second-class citizen?
 

Because it's complete and utter crap. Freescale has completely ignored
the basic register interface requirements of the SDHCI spec. Treating
eSDHC as a second-class citizen is generous IMO.

  as far as I'm
  concerned, so I'd prefer something like:
  
  if (!host-ops-writel)
  writel(host-ioaddr + reg, val);
  else
  host-ops-writel(host, val, reg);
 
 Surely the overhead isn't measurable... but why we purposely make
 things worse?
 

We can most likely do some micro-optimisation do make the compare part
cheaper, but the point was to avoid a function call for all the
properly implemented controllers out there. We could have a flag so
that it only has to check host-flags, which will most likely be in the
cache anyway.

Overhead for eSDHC is not a concern in my book, what is interesting is
how much this change slows things down for other controllers.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 03/13] sdhci: Split card-detection IRQs management from sdhci_init()

2009-02-21 Thread Pierre Ossman
On Fri, 13 Feb 2009 17:47:15 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 Card detection interrupts should be handled separately as they should
 not be enabled before mmc_add_host() returns and should be disabled
 before calling mmc_remove_host(). The same is for suspend and resume
 routines.
 
 sdhci_init() no longer enables card-detection irqs. Instead, two new
 functions implemented: sdhci_enable_card_detection() and
 sdhci_disable_card_detection().
 
 New sdhci_reinit() call implemented to behave the same way as the old
 sdhci_init().
 
 Also, this patch implements and uses few new helpers to manage IRQs in
 a more conveinient way, that is:
 
 - sdhci_clear_set_irqs()
 - sdhci_unmask_irqs()
 - sdhci_mask_irqs()
 - SDHCI_INT_ALL_MASK constant
 
 sdhci_enable_sdio_irq() converted to these new helpers, plus the
 helpers will be used by the subsequent patches.
 
 Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
 ---

That's a lot of indirection, but fair enough. :)

 @@ -1792,6 +1832,8 @@ int sdhci_add_host(struct sdhci_host *host)
  
   mmc_add_host(mmc);
  
 + sdhci_enable_card_detection(host);
 +
   printk(KERN_INFO %s: SDHCI controller on %s [%s] using %s%s\n,
   mmc_hostname(mmc), host-hw_name, dev_name(mmc_dev(mmc)),
   (host-flags  SDHCI_USE_ADMA)?A:,

There is a small race here, but I'm not sure it's worth dealing with.

 diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
 index e907441..45c8309 100644
 --- a/drivers/mmc/host/sdhci.h
 +++ b/drivers/mmc/host/sdhci.h
 @@ -124,6 +124,10 @@
   SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL | \
   SDHCI_INT_DATA_TIMEOUT | SDHCI_INT_DATA_CRC | \
   SDHCI_INT_DATA_END_BIT)
 +#define SDHCI_INT_ALL_MASK   (SDHCI_INT_CMD_MASK | SDHCI_INT_DATA_MASK | \
 + SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE | \
 + SDHCI_INT_CARD_INT | SDHCI_INT_ERROR | SDHCI_INT_BUS_POWER | \
 + SDHCI_INT_ACMD12ERR | SDHCI_INT_ADMA_ERROR)
  

In the context this is used, why not just use (unsigned)-1?

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 05/13] sdhci: Add support for card-detection polling

2009-02-21 Thread Pierre Ossman
On Fri, 13 Feb 2009 17:47:18 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 @@ -1110,13 +1113,18 @@ static void sdhci_request(struct mmc_host *mmc, 
 struct mmc_request *mrq)
  
   host-mrq = mrq;
  
 + if (host-quirks  SDHCI_QUIRK_BROKEN_CARD_DETECTION)
 + goto send;
 +
   if (!(sdhci_readl(host, SDHCI_PRESENT_STATE)  SDHCI_CARD_PRESENT)
   || (host-flags  SDHCI_DEVICE_DEAD)) {
   host-mrq-cmd-error = -ENOMEDIUM;
   tasklet_schedule(host-finish_tasklet);
 - } else
 - sdhci_send_command(host, mrq-cmd);
 -
 + goto out;
 + }
 +send:
 + sdhci_send_command(host, mrq-cmd);
 +out:
   mmiowb();
   spin_unlock_irqrestore(host-lock, flags);
  }

goto:s seem unnecessary here, and your patch is even incorrect as it
ignores the SDHCI_DEVICE_DEAD flag. Just modify the if-clause and
things will work.

Might want to add a comment also to make it more obvious what the
if-clause does.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 07/13] sdhci: Add support for hosts with strict 32 bit addressing

2009-02-21 Thread Pierre Ossman
On Fri, 13 Feb 2009 17:47:22 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 SDHCI driver must take special care when working with triggering
 registers on hosts with strict 32 bit addressing.
 
 In FSL eSDHC hosts all registers are 32 bit width, writing to the
 first half of any register will cause [undefined?] write the second
 half of the register. That is, 16 bit write to the TRANSFER_MODE
 register, makes hardware see a bogus write to the COMMAND register
 (these two registers are adjacent).
 
 This patch adds SDHCI_QUIRK_32BIT_REGISTERS quirk. When specified,
 the sdhci driver will try to pack all dangerous writes into single
 32 bit write transaction.
 
 Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
 ---

What about the other places where we have 16 and 8 bit registers?

-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 12/13] sdhci: Add quirk for controllers with max. block size up to 4096 bytes

2009-02-21 Thread Pierre Ossman
On Fri, 13 Feb 2009 17:47:39 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 @@ -831,7 +832,12 @@ static void sdhci_prepare_data(struct sdhci_host *host, 
 struct mmc_data *data)
   sdhci_set_transfer_irqs(host);
  
   /* We do not handle DMA boundaries, so set it to max (512 KiB) */
 - sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, data-blksz), SDHCI_BLOCK_SIZE);
 + if (host-quirks  SDHCI_QUIRK_MAX_BLK_SZ_4096)
 + blksz = data-blksz;
 + else
 + blksz = SDHCI_MAKE_BLKSZ(7, data-blksz);
 +
 + sdhci_writew(host, blksz, SDHCI_BLOCK_SIZE);
   sdhci_writew(host, data-blocks, SDHCI_BLOCK_COUNT);
  }
  

Hmm.. I seem to have overlooked this part previously. I guess they've
basically stripped out the DMA boundary stuff and used the bits for
other things?

At this point I'm leaning more towards simply not supporting their
extended block size. After all, is it ever used?

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 0/13] FSL eSDHC support

2009-02-21 Thread Pierre Ossman
On Fri, 20 Feb 2009 20:32:28 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 Hi all,
 
 Some updates for the eSDHC support:
 

I think the patches are coming along nicely. If we can just sort out
the accessors, then it should be ready for -next. It pokes around quite
a bit in the sdhci driver though, so I'd like it to stay there for one
cycle and (hopefully) be merged for 2.6.31.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 01/13] sdhci: Add quirk for controllers with no end-of-busy IRQ

2009-02-21 Thread Pierre Ossman
On Fri, 20 Feb 2009 20:33:08 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 From: Ben Dooks ben-li...@fluff.org
 
 The Samsung SDHCI (and FSL eSDHC) controller block seems to fail
 to generate an INT_DATA_END after the transfer has completed and
 the bus busy state finished.
 
 Changes in e809517f6fa5803a5a1cd56026f0e2190fc13d5c to use the
 new busy method are the cause of the behaviour change.
 
 Signed-off-by: Ben Dooks ben-li...@fluff.org
 Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
 ---

Any objections to me merging this right away? It is needed for another
controller.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH RFC 0/11] FSL eSDHC support: second call for comments

2009-02-08 Thread Pierre Ossman
On Fri, 6 Feb 2009 21:05:20 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 Hi all,
 
 There were only a few comments on the previous version. So, here is
 the second call for comments.
 

Yeah sorry, haven't really had time for looking over your patches
properly. See my first comments to relevant patches now though.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 02/11] sdhci: Add support for bus-specific IO memory accessors

2009-02-08 Thread Pierre Ossman
On Fri, 6 Feb 2009 21:06:45 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 Currently the SDHCI driver works with PCI accessors (write{l,b,w} and
 read{l,b,w}).
 
 With this patch drivers may change memory accessors, so that we can
 support hosts with weird IO memory access requirments.
 
 For example, in FSL eSDHC SDHCI hardware all registers are 32 bit
 width, with big-endian addressing. That is, readb(0x2f) should turn
 into readb(0x2c), and readw(0x2c) should be translated to
 le16_to_cpu(readw(0x2e)).
 
 Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
 ---

I was hoping we wouldn't have to do a lot of magic in the accessors
since the spec is rather clear on the register interface. :/

Let's see if I've understood this correctly.

1. The CPU is big-endian but the register are little-endian (as the
spec requires). I was under the impression that the read*/write*
accessor handled any endian conversion between the bus and the cpu? How
do e.g. PCI work on Sparc?

2. Register access must be done 32 bits at a time. Now this is just
broken and might cause big problems as some registers cannot just be
read and written back to. OTOH you refer to readw() in your example,
not readl(). What's the deal here?

 +static inline void sdhci_writel(struct sdhci_host *host, u32 val, int reg)
 +{
 + host-writel(host, val, reg);
 +}

Having to override these are worst case scenario as far as I'm
concerned, so I'd prefer something like:

if (!host-ops-writel)
writel(host-ioaddr + reg, val);
else
host-ops-writel(host, val, reg);

and maybe even a likely() up there.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 03/11] sdhci: Add type checking for IO memory accessors

2009-02-08 Thread Pierre Ossman
On Fri, 6 Feb 2009 21:06:48 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 A new restricted integer type introduced: sdhci_reg_t.
 
 Header file now specifies registers via sdhci_reg() inline function.
 Only one place (not counting sdhci_def_*() accessors) need to cast
 a register back to an offset, i.e. sdhci_finish_command().
 
 From now on sparse tool will warn about IO memory accessors misuses,
 for exampple:
 
 sdhci_writeb(host, SDHCI_TIMEOUT_CONTROL, count);
 
   CHECK   sdhci.c
 sdhci.c:614:21: warning: incorrect type in argument 2 (different base types)
 sdhci.c:614:21:expected unsigned char [unsigned] [usertype] val
 sdhci.c:614:21:got restricted int
 sdhci.c:614:44: warning: incorrect type in argument 3 (different base types)
 sdhci.c:614:44:expected restricted int [usertype] reg
 sdhci.c:614:44:got unsigned char [unsigned] [assigned] [usertype] count
 
 Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
 ---

Is this really a problem? It's a lot of noise in the code and I can't
really see this as a major issue, or even a minor one. :)

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 04/11] sdhci: Add support for card-detection polling

2009-02-08 Thread Pierre Ossman
On Fri, 6 Feb 2009 21:06:50 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 This patch adds SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk. When specified,
 sdhci driver will set MMC_CAP_NEEDS_POLL MMC host capability, and won't
 enable card insert/remove interrupts.
 
 This is needed for hosts with unreliable card detection, such as FSL
 eSDHC. The original eSDHC driver was tring to debounce card-detection
 IRQs by reading present state and disabling particular interrupts. But
 with this debouncing scheme I noticed that sometimes we miss card
 insertion/removal events.
 
 Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
 ---

I guess you need to fix the check at the start of the request function
as well.

 diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
 index b7a79a0..45c5f1f 100644
 --- a/drivers/mmc/host/sdhci.c
 +++ b/drivers/mmc/host/sdhci.c
 @@ -162,6 +162,9 @@ static void sdhci_init(struct sdhci_host *host)
   SDHCI_INT_DMA_END | SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE |
   SDHCI_INT_ADMA_ERROR;
  
 + if (host-quirks  SDHCI_QUIRK_BROKEN_CARD_DETECTION)
 + intmask = ~(SDHCI_INT_CARD_REMOVE | SDHCI_INT_CARD_INSERT);
 +

A matter of taste perhaps, but I think it would make more sense to not
add them in the first place than to add and then remove them.

Card detection interrupts should be handled separately anyway as they
should not be enabled before mmc_add_host() returns and should be
disabled before calling mmc_remove_host(). Patch welcome. ;)

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 07/11] sdhci: Add quirk to suppress PIO interrupts during DMA transfers

2009-02-08 Thread Pierre Ossman
On Fri, 6 Feb 2009 21:06:55 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 Some hosts (that is, FSL eSDHC) throw PIO interrupts during DMA
 transfers, this causes tons of unneeded interrupts, and thus highly
 degraded speed.
 
 This patch adds SDHCI_QUIRK_PIO_IRQS_DURING_DMA quirk. When specified,
 the sdhci driver will disable PIO interrupts during DMA transfers.
 
 Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
 ---

It's probably better to change the interrupt handling to only enable
relevant interrupts instead of having everything on constantly. Too
many quirks just makes the driver difficult to understand.

-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 08/11] sdhci: Add support for hosts that don't specify clocks in the cap. register

2009-02-08 Thread Pierre Ossman
On Fri, 6 Feb 2009 21:06:57 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 FSL eSDHC hosts don't provide clocks bits in the capabilities register,
 instead we're getting clocks values from the device tree.
 
 There is somewhat similar change[1] from Ben Dooks, the change adds
 callbacks for getting the clocks. But for eSDHC the callbacks are
 superfluous, since the clocks are static.
 
 [1] http://lkml.org/lkml/2008/12/2/157
 
 Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
 ---

As I told Ben, I prefer if we stick to the standard as much as
possible. So no external info unless the register is set to zero.

And since we know the Samsung chip needs callbacks, we might as well
add them here. It's not like this is a performance critical path.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 09/11] sdhci: Add set_clock callback

2009-02-08 Thread Pierre Ossman
On Fri, 6 Feb 2009 21:06:59 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 FSL eSDHC hosts have incompatible register map to manage the SDCLK.
 This patch adds set_clock callback so that drivers could overwrite
 set_clock behaviour.
 
 Similar patch[1] was posted by Ben Dooks, though in Ben's version the
 callback is named change_clock, plus the patch has some unrelated bits
 that makes the patch difficult to reuse.
 
 [1] http://lkml.org/lkml/2008/12/2/160
 
 Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
 ---

A set_clock() callback is reasonable as there might be a clock source
that needs to be set up, but completely overriding the normal routine
(i.e. the return) should be quirked IMO.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 10/11] sdhci: Add quirk for Freescale eSDHC controllers

2009-02-08 Thread Pierre Ossman
On Fri, 6 Feb 2009 21:07:01 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 This patch adds SDHCI_QUIRK_FSL quirk. The quirk is used to instruct
 the sdhci driver about various FSL eSDHC host incompatibilities:
 

No device quirks please. They should be for specific bugs, not lumping
things together like this. Otherwise we'll soon have an unmanageable
mess.

 1) FSL eSDHC controllers can support maximum block size up to 4096
bytes. The MBL (Maximum Block Length) field in the capabilities
register extended by one bit.
 
(Should we implement a dedicated quirk for this? I.e.
 SDHCI_QUIRK_MAX_BLK_SZ_4096?)
 

Yes please. It would have to mean always support 4096 though, not
turn reserved bit 18 into a block length bit.

 2) sdhci_init() is needed after error conditions.
 
(Can we safely do this for all controllers?)
 

Please investigate which part of sdhci_init() is needed. How does it
break without this?

 3) Small udelay is needed to make eSDHC work in PIO mode. Without
the delay reading causes endless interrupt storm, and writing
corrupts data. The first guess would be that we must wait for
some bit in some register, but I didn't find any reliable bits
that changes before and after the delay. Though, more investigation
on this is in my todo list.

Please try to investigate more, but if you cannot improve it further
then a specific quirk can be added.

Rgds
-- 
 -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.


signature.asc
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] mmc: Add driver for Freescale eSDHC controllers

2009-01-14 Thread Pierre Ossman
On Thu, 15 Jan 2009 02:56:05 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 
 Ah. I wonder why Freescale just didn't write some patches for sdhci,
 but copied the code instead...
 

Probably because it was quicker. Samsung did the same thing, but now
Ben Dooks has patches to properly support it via sdhci.

Rgds
-- 
 -- Pierre Ossman

  Linux kernel, MMC maintainerhttp://www.kernel.org
  rdesktop, core developer  http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] mmc: Add driver for Freescale eSDHC controllers

2009-01-14 Thread Pierre Ossman
On Thu, 15 Jan 2009 14:11:09 +0800
Liu Dave dave...@freescale.com wrote:

  Probably because it was quicker. Samsung did the same thing, but now
  Ben Dooks has patches to properly support it via sdhci.
 
 Pierre,
 What samsung driver are you talking? Could you point it to me?
 

I'm afraid I haven't seen any readily available sources for their
driver. If you meant Ben's version, you can find the latest revision
here:

http://marc.info/?t=12282326615

Unfortunately marc.info didn't sort out the threading, but you should
be able to find all the patches there or some other LKML archive.

Rgds
-- 
 -- Pierre Ossman

  Linux kernel, MMC maintainerhttp://www.kernel.org
  rdesktop, core developer  http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH v2] mmc: Add mmc_vddrange_to_ocrmask() helper function

2008-12-14 Thread Pierre Ossman
On Mon, 1 Dec 2008 14:53:20 +0300
Anton Vorontsov avoront...@ru.mvista.com wrote:

 
 Though, the $subject patch could be merged anytime as it doesn't
 depend on anything else. So, if you'll merge it earlier, that will
 make things a bit easier: -1 patch to resend. ;-)
 

Queued up. Will be sent once the merge window opens up.

Rgds
-- 
 -- Pierre Ossman

  Linux kernel, MMC maintainerhttp://www.kernel.org
  rdesktop, core developer  http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH v2] mmc: Add mmc_vddrange_to_ocrmask() helper function

2008-11-30 Thread Pierre Ossman
On Wed, 26 Nov 2008 22:54:17 +0300
Anton Vorontsov [EMAIL PROTECTED] wrote:

 This function sets the OCR mask bits according to provided voltage
 ranges. Will be used by the mmc_spi OpenFirmware bindings.
 
 Signed-off-by: Anton Vorontsov [EMAIL PROTECTED]
 ---
 
 Hi Pierre,
 
 Sorry for the delay.
 

This looks perfect. Just tell me when you want me to queue it up.

Rgds
-- 
 -- Pierre Ossman

  Linux kernel, MMC maintainerhttp://www.kernel.org
  rdesktop, core developer  http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 0/3 RFC] MMC SPI support for OpenFirmware platforms

2008-11-08 Thread Pierre Ossman
On Thu, 30 Oct 2008 22:55:46 +0300
Anton Vorontsov [EMAIL PROTECTED] wrote:

 
 Pierre, the approach is somewhat similar to this one:
 http://lkml.org/lkml/2008/5/26/135
 Posted few months ago.
 
 I know you don't like it, but I ask you to reconsider it. The
 I2C and SPI cases are similar, and recently we tried to write
 bindings for some I2C GPIO controllers.
 

This new version is a bit better in that you've generalised thing more.
I'd still prefer if we can have an interface where the driver doesn't
have to know that it is on an ACPI/OF/EFI/whatnot host, but I can live
with this model for now.

Rgds
-- 
 -- Pierre Ossman

  Linux kernel, MMC maintainerhttp://www.kernel.org
  rdesktop, core developer  http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 2/3] mmc: Add mmc_vddrange_to_ocrmask() helper function

2008-11-08 Thread Pierre Ossman
On Thu, 30 Oct 2008 22:56:32 +0300
Anton Vorontsov [EMAIL PROTECTED] wrote:

 +/**
 + * mmc_vddrange_to_ocrmask - Convert a voltage range to the OCR mask
 + * @vdd_min: minimum voltage value (mV)
 + * @vdd_max: maximum voltage value (mV)
 + * @mask:pointer to the mask
 + *

Why the pointer? Why not let the caller handle the aggregation? That
would be a lot safer.

 + /* fill the mask, from max bit to min bit */
 + while (vdd_max = vdd_min)
 + *mask |= 1  vdd_max--;
 + return 0;

Many cards get a bit uppity with a single bit set. If possible, try to
make this function set two bits when the voltage is right on the
boundary (e.g. 3.3V).

Rgds
-- 
 -- Pierre Ossman

  Linux kernel, MMC maintainerhttp://www.kernel.org
  rdesktop, core developer  http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] [MMC] mmc_spi: add polling support for the card detect line

2008-05-17 Thread Pierre Ossman
On Fri, 16 May 2008 20:50:57 +0400
Anton Vorontsov [EMAIL PROTECTED] wrote:

 Some boards do not use interrupts on the CD line, so we want to poll
 the CD and see if there was a change. 1 second poll interval seems
 resonable.
 

The idea isn't bad, but I'm not sure about the mechanism.

To poll a MMC slot, you do not really need a card detect at all. The
MMC layer can just shoot off some requests and see if anything
responds.  The PXA driver (if my memory serves me right) already does
this. So the best idea there would be to add this feature to the MMC
core and let the host indicate that it needs it via MMC_CAP_NEEDS_POLL
or something like that.

The card detection pin then becomes an optimisation, something that is
also useful in other ways. Have the host driver check the card detection
pin at the start of every request, and quickly fail it if there is no
card present.

That should give you what you want with much more flexibility for other
uses as well.

Rgds
-- 
 -- Pierre Ossman

  Linux kernel, MMC maintainerhttp://www.kernel.org
  rdesktop, core developer  http://www.rdesktop.org
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev