Re: [PATCH v3 2/2] soc: fsl: qbman: Use raw spinlock for cgr_lock

[Crystal Wood]

On Tue, 2023-04-04 at 10:55 -0400, Sean Anderson wrote:

> @@ -1456,11 +1456,11 @@ static void qm_congestion_task(struct work_struct
> *work)
>         union qm_mc_result *mcr;
>         struct qman_cgr *cgr;
>  
> -       spin_lock_irq(&p->cgr_lock);
> +       raw_spin_lock_irq(&p->cgr_lock);
>         qm_mc_start(&p->p);
>         qm_mc_commit(&p->p, QM_MCC_VERB_QUERYCONGESTION);
>         if (!qm_mc_result_timeout(&p->p, &mcr)) {
> -               spin_unlock_irq(&p->cgr_lock);
> +               raw_spin_unlock_irq(&p->cgr_lock);

qm_mc_result_timeout() spins with a timeout of 10 ms which is very
inappropriate for a raw lock.  What is the actual expected upper bound?

>                 dev_crit(p->config->dev, "QUERYCONGESTION timeout\n");
>                 qman_p_irqsource_add(p, QM_PIRQ_CSCI);
>                 return;
> @@ -1476,7 +1476,7 @@ static void qm_congestion_task(struct work_struct
> *work)
>         list_for_each_entry(cgr, &p->cgr_cbs, node)
>                 if (cgr->cb && qman_cgrs_get(&c, cgr->cgrid))
>                         cgr->cb(p, cgr, qman_cgrs_get(&rr, cgr->cgrid));
> -       spin_unlock_irq(&p->cgr_lock);
> +       raw_spin_unlock_irq(&p->cgr_lock);
>         qman_p_irqsource_add(p, QM_PIRQ_CSCI);
>  }

The callback loop is also a bit concerning...

-Crystal