On Thu, 5 Sep 2019 at 14:22, Ludovic Barre <ludovic.Barre@xxxxxx> wrote:
>
> From: Ludovic Barre <ludovic.barre@xxxxxx>
>
> This patch adds busy_completion callback at mmci_host_ops
> to allow to define a specific busy completion by variant.
>
> The legacy code corresponding to busy completion used
> by ux500 variants is moved to ux500_busy_complete function.
>
> The busy_detect boolean property is replaced by
> busy_complete callback definition.
At this point I prefer to keep th busy_detect boolean property. It
makes the code a bit more consistent.
Although, I think in case busy_detect is set for the variant, the
variant also needs to assign the new ->busy_completion() callback. In
other words, we don't need to check for a valid callback in code if
busy_detect is set.
Otherwise, this looks good to me!
Kind regards
Uffe
>
> Signed-off-by: Ludovic Barre <ludovic.barre@xxxxxx>
> ---
> drivers/mmc/host/mmci.c | 142 +++++++++++++++++++++-------------------
> drivers/mmc/host/mmci.h | 3 +-
> 2 files changed, 76 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index c30319255dc2..e20164f4354d 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -44,6 +44,7 @@
> #define DRIVER_NAME "mmci-pl18x"
>
> static void mmci_variant_init(struct mmci_host *host);
> +static void ux500_variant_init(struct mmci_host *host);
> static void ux500v2_variant_init(struct mmci_host *host);
>
> static unsigned int fmax = 515633;
> @@ -175,7 +176,6 @@ static struct variant_data variant_ux500 = {
> .f_max = 100000000,
> .signal_direction = true,
> .pwrreg_clkgate = true,
> - .busy_detect = true,
> .busy_dpsm_flag = MCI_DPSM_ST_BUSYMODE,
> .busy_detect_flag = MCI_ST_CARDBUSY,
> .busy_detect_mask = MCI_ST_BUSYENDMASK,
> @@ -184,7 +184,7 @@ static struct variant_data variant_ux500 = {
> .irq_pio_mask = MCI_IRQ_PIO_MASK,
> .start_err = MCI_STARTBITERR,
> .opendrain = MCI_OD,
> - .init = mmci_variant_init,
> + .init = ux500_variant_init,
> };
>
> static struct variant_data variant_ux500v2 = {
> @@ -208,7 +208,6 @@ static struct variant_data variant_ux500v2 = {
> .f_max = 100000000,
> .signal_direction = true,
> .pwrreg_clkgate = true,
> - .busy_detect = true,
> .busy_dpsm_flag = MCI_DPSM_ST_BUSYMODE,
> .busy_detect_flag = MCI_ST_CARDBUSY,
> .busy_detect_mask = MCI_ST_BUSYENDMASK,
> @@ -610,6 +609,67 @@ static u32 ux500v2_get_dctrl_cfg(struct mmci_host *host)
> return MCI_DPSM_ENABLE | (host->data->blksz << 16);
> }
>
> +static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk)
> +{
> + void __iomem *base = host->base;
> +
> + /*
> + * Before unmasking for the busy end IRQ, confirm that the
> + * command was sent successfully. To keep track of having a
> + * command in-progress, waiting for busy signaling to end,
> + * store the status in host->busy_status.
> + *
> + * Note that, the card may need a couple of clock cycles before
> + * it starts signaling busy on DAT0, hence re-read the
> + * MMCISTATUS register here, to allow the busy bit to be set.
> + * Potentially we may even need to poll the register for a
> + * while, to allow it to be set, but tests indicates that it
> + * isn't needed.
> + */
> + if (!host->busy_status && !(status & err_msk) &&
> + (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) {
> + writel(readl(base + MMCIMASK0) |
> + host->variant->busy_detect_mask,
> + base + MMCIMASK0);
> +
> + host->busy_status = status & (MCI_CMDSENT | MCI_CMDRESPEND);
> + return false;
> + }
> +
> + /*
> + * If there is a command in-progress that has been successfully
> + * sent, then bail out if busy status is set and wait for the
> + * busy end IRQ.
> + *
> + * Note that, the HW triggers an IRQ on both edges while
> + * monitoring DAT0 for busy completion, but there is only one
> + * status bit in MMCISTATUS for the busy state. Therefore
> + * both the start and the end interrupts needs to be cleared,
> + * one after the other. So, clear the busy start IRQ here.
> + */
> + if (host->busy_status &&
> + (status & host->variant->busy_detect_flag)) {
> + writel(host->variant->busy_detect_mask, base + MMCICLEAR);
> + return false;
> + }
> +
> + /*
> + * If there is a command in-progress that has been successfully
> + * sent and the busy bit isn't set, it means we have received
> + * the busy end IRQ. Clear and mask the IRQ, then continue to
> + * process the command.
> + */
> + if (host->busy_status) {
> + writel(host->variant->busy_detect_mask, base + MMCICLEAR);
> +
> + writel(readl(base + MMCIMASK0) &
> + ~host->variant->busy_detect_mask, base + MMCIMASK0);
> + host->busy_status = 0;
> + }
> +
> + return true;
> +}
> +
> /*
> * All the DMA operation mode stuff goes inside this ifdef.
> * This assumes that you have a generic DMA device interface,
> @@ -953,9 +1013,16 @@ void mmci_variant_init(struct mmci_host *host)
> host->ops = &mmci_variant_ops;
> }
>
> +void ux500_variant_init(struct mmci_host *host)
> +{
> + host->ops = &mmci_variant_ops;
> + host->ops->busy_complete = ux500_busy_complete;
> +}
> +
> void ux500v2_variant_init(struct mmci_host *host)
> {
> host->ops = &mmci_variant_ops;
> + host->ops->busy_complete = ux500_busy_complete;
> host->ops->get_datactrl_cfg = ux500v2_get_dctrl_cfg;
> }
>
> @@ -1235,68 +1302,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
> return;
>
> /* Handle busy detection on DAT0 if the variant supports it. */
> - if (busy_resp && host->variant->busy_detect) {
> -
> - /*
> - * Before unmasking for the busy end IRQ, confirm that the
> - * command was sent successfully. To keep track of having a
> - * command in-progress, waiting for busy signaling to end,
> - * store the status in host->busy_status.
> - *
> - * Note that, the card may need a couple of clock cycles before
> - * it starts signaling busy on DAT0, hence re-read the
> - * MMCISTATUS register here, to allow the busy bit to be set.
> - * Potentially we may even need to poll the register for a
> - * while, to allow it to be set, but tests indicates that it
> - * isn't needed.
> - */
> - if (!host->busy_status && !(status & err_msk) &&
> - (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) {
> -
> - writel(readl(base + MMCIMASK0) |
> - host->variant->busy_detect_mask,
> - base + MMCIMASK0);
> -
> - host->busy_status =
> - status & (MCI_CMDSENT|MCI_CMDRESPEND);
> - return;
> - }
> -
> - /*
> - * If there is a command in-progress that has been successfully
> - * sent, then bail out if busy status is set and wait for the
> - * busy end IRQ.
> - *
> - * Note that, the HW triggers an IRQ on both edges while
> - * monitoring DAT0 for busy completion, but there is only one
> - * status bit in MMCISTATUS for the busy state. Therefore
> - * both the start and the end interrupts needs to be cleared,
> - * one after the other. So, clear the busy start IRQ here.
> - */
> - if (host->busy_status &&
> - (status & host->variant->busy_detect_flag)) {
> - writel(host->variant->busy_detect_mask,
> - host->base + MMCICLEAR);
> + if (busy_resp && host->ops->busy_complete)
> + if (!host->ops->busy_complete(host, status, err_msk))
> return;
> - }
> -
> - /*
> - * If there is a command in-progress that has been successfully
> - * sent and the busy bit isn't set, it means we have received
> - * the busy end IRQ. Clear and mask the IRQ, then continue to
> - * process the command.
> - */
> - if (host->busy_status) {
> -
> - writel(host->variant->busy_detect_mask,
> - host->base + MMCICLEAR);
> -
> - writel(readl(base + MMCIMASK0) &
> - ~host->variant->busy_detect_mask,
> - base + MMCIMASK0);
> - host->busy_status = 0;
> - }
> - }
>
> host->cmd = NULL;
>
> @@ -1537,7 +1545,7 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
> * clear the corresponding IRQ.
> */
> status &= readl(host->base + MMCIMASK0);
> - if (host->variant->busy_detect)
> + if (host->ops->busy_complete)
> writel(status & ~host->variant->busy_detect_mask,
> host->base + MMCICLEAR);
> else
> @@ -1605,7 +1613,7 @@ static void mmci_set_max_busy_timeout(struct mmc_host *mmc)
> struct mmci_host *host = mmc_priv(mmc);
> u32 max_busy_timeout = 0;
>
> - if (!host->variant->busy_detect)
> + if (!host->ops->busy_complete)
> return;
>
> if (host->variant->busy_timeout && mmc->actual_clock)
> @@ -1980,7 +1988,7 @@ static int mmci_probe(struct amba_device *dev,
> /*
> * Enable busy detection.
> */
> - if (variant->busy_detect) {
> + if (host->ops->busy_complete) {
> mmci_ops.card_busy = mmci_card_busy;
> /*
> * Not all variants have a flag to enable busy detection
> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
> index d8b7f6774e8f..733f9a035b06 100644
> --- a/drivers/mmc/host/mmci.h
> +++ b/drivers/mmc/host/mmci.h
> @@ -286,7 +286,6 @@ struct mmci_host;
> * @f_max: maximum clk frequency supported by the controller.
> * @signal_direction: input/out direction of bus signals can be indicated
> * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock
> - * @busy_detect: true if the variant supports busy detection on DAT0.
> * @busy_timeout: true if the variant starts data timer when the DPSM
> * enter in Wait_R or Busy state.
> * @busy_dpsm_flag: bitmask enabling busy detection in the DPSM
> @@ -334,7 +333,6 @@ struct variant_data {
> u32 f_max;
> u8 signal_direction:1;
> u8 pwrreg_clkgate:1;
> - u8 busy_detect:1;
> u8 busy_timeout:1;
> u32 busy_dpsm_flag;
> u32 busy_detect_flag;
> @@ -369,6 +367,7 @@ struct mmci_host_ops {
> void (*dma_error)(struct mmci_host *host);
> void (*set_clkreg)(struct mmci_host *host, unsigned int desired);
> void (*set_pwrreg)(struct mmci_host *host, unsigned int pwr);
> + bool (*busy_complete)(struct mmci_host *host, u32 status, u32 err_msk);
> };
>
> struct mmci_host {
> --
> 2.17.1
>
