Re: [RESEND PATCH v7 2/2] mmc: OCTEON: Add host driver for OCTEON MMC controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/19/2016 03:09 PM, Arnd Bergmann wrote:
On Tuesday 19 April 2016 14:45:35 David Daney wrote:
On 04/19/2016 01:46 PM, Arnd Bergmann wrote:
On Thursday 31 March 2016 16:26:53 Matt Redfearn wrote:
+struct octeon_mmc_host {
+	u64	base;
+	u64	ndf_base;
+	u64	emm_cfg;
+	u64	n_minus_one;  /* OCTEON II workaround location */
+	int	last_slot;
+
+	struct semaphore mmc_serializer;

Please don't add any new semaphores to the kernel, use a mutex or
a completion instead.

The last time I checked, a mutex could not be used from interrupt context.

Since we are in interrupt context and we really want mutex-like behavior
here, it seems like a semaphore is just the thing we need.

I am not sure how completions would be of use, perhaps you could elaborate.

Completions are used when you have one thread waiting for an event,
which is often an interrupt: the process calls
wait_for_completion(&completion); and the interrupt handler calls
complete(&completion);

It seems that you are using the semaphore for two reasons here (I
only read it briefly so I may be wrong):
waiting for the interrupt handler and serializing against another
thread. In this case you need both a mutex (to guarantee mutual
exclusion) and a completion (to wait for the interrupt handler
to finish).


The way the MMC driver works is that the driver's .request() method is called to initiate a request. After .request() is finished, it returns back to the kernel so other work can be done.

From the interrupt handler, when the request is complete, the interrupt handler calls req->done(req); to terminate the whole thing.


  So we have:

  CPU-A                      CPU-B                  CPU-C

  octeon_mmc_request(0)        .                     .
     down()                    .                     .
     queue_request(0);         .                     .
     return;                   .                     .
  other_useful_work            .                     .
   .                           .                     .
   .                           .                     .
   .                           .                     .
  octeon_mmc_request(1)        .                     .
     down() -> blocks          .                     .
                            octeon_mmc_interrupt()   .
                                up() -> unblocks     .
     down() <-unblocks          req->done(0)         .
     queue_request(1);          return;              .
     return;                   .                     .
  other_useful_work            .                     .
   .                           .                octeon_mmc_interrupt()
   .                           .                     up()
   .                           .                     req->done(1)
   .                           .                     return;
   .                           .                     .


We don't want to have the thread on CPU-A wait around in an extra mutex or completion for the command to finish. The MMC core already has its own request waiting code, but it doesn't handle the concept of a slot. These commands can take hundreds or thousands of mS to terminate. The whole idea of the MMC framework is to queue the request and get back to doing other work ASAP.

In the case of this octeon_mmc driver we need to serialize the commands issued to multiple slots, for this we use the semaphore. If you don't like struct semaphore, we would have to invent a proprietary wait queue mechanism that has semantics nearly identical to struct semaphore, and people would complain that we are reinventing the semaphore.

It doesn't seem clean to cobble up multiple waiting structures (completion + mutex + logic that surely would contain errors) where a single (well debugged) struct semaphore does what we want.


David Daney


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux