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.
> +#if 0
> +#define octeon_mmc_dbg trace_printk
> +#else
> +static inline void octeon_mmc_dbg(const char *s, ...) { }
> +#endif
Remove this and use dev_dbg() or pr_debug(), it does the same thing.
> +static irqreturn_t octeon_mmc_interrupt(int irq, void *dev_id)
This function is rather long, can you split it up a bit for
readability?
> +{
> + struct octeon_mmc_host *host = dev_id;
> + union cvmx_mio_emm_int emm_int;
> + struct mmc_request *req;
> + bool host_done;
> + union cvmx_mio_emm_rsp_sts rsp_sts;
> + unsigned long flags = 0;
> +
> + if (host->need_irq_handler_lock)
> + spin_lock_irqsave(&host->irq_handler_lock, flags);
> + else
> + __acquire(&host->irq_handler_lock);
The locking seems odd, why do you only sometimes have to take the lock,
and why do you disable interrupts from within the irq handler?
Arnd
--
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
