[Skiboot] [PATCH v8 2/2] Add SBE driver support

[Benjamin Herrenschmidt]

On Thu, 2018-04-19 at 13:58 +0530, Vasant Hegde wrote:
> +}
> +
> +static void __p9_sbe_interrupt(struct p9_sbe *sbe)
> +{
> +       bool has_response = false;
> +       int rc;
> +       u64 data = 0, val;
> +       struct p9_sbe_msg *msg = NULL;
>  
>         /* Read doorbell register */
> -       rc = xscom_read(chip_id, PSU_HOST_DOORBELL_REG_RW, &data);
> +       rc = xscom_read(sbe->chip_id, PSU_HOST_DOORBELL_REG_RW, &data);
>         if (rc) {
>                 prlog(PR_ERR, "Failed to read SBE to Host doorbell register "
> -                     "[chip id = %x]\n", chip_id);
> -               goto clr_interrupt;
> +                     "[chip id = %x]\n", sbe->chip_id);
> +               p9_sbe_reg_dump(sbe->chip_id);
> +               return;
>         }
>  
> -       /* SBE passtrhough command, call prd handler */
> -       if (data & SBE_HOST_PASSTHROUGH) {
> -               prd_sbe_passthrough(chip_id);
> +       /* Called from poller path, nothing to process */
> +       if (!data)
> +               return;
> +
> +       /* Read SBE response before clearing doorbell register */
> +       if (data & SBE_HOST_RESPONSE_WAITING) {
> +               if (!list_empty(&sbe->msg_list)) {
> +                       msg = list_top(&sbe->msg_list, struct p9_sbe_msg, link);
> +                       p9_sbe_handle_response(sbe->chip_id, msg);
> +                       has_response = true;
> +               }
>         }

I still think you should process the ACKs before you look at responses.

Didn't we also say we should have a separate variable for responses in
this function ?

Generally I still prefer if we handle one bit at a time amd loop & re-
read & retry, as discussed.

> -clr_interrupt:
> -       /* Clears all the bits */
> -       rc = xscom_write(chip_id, PSU_HOST_DOORBELL_REG_AND,
> -                        SBE_HOST_RESPONSE_CLEAR);
> +       /* Clear doorbell register */
> +       val = SBE_HOST_RESPONSE_MASK & ~data;
> +       rc = xscom_write(sbe->chip_id, PSU_HOST_DOORBELL_REG_AND, val);
>         if (rc) {
>                 prlog(PR_ERR, "Failed to clear SBE to Host doorbell "
> -                     "register [chip id = %x]\n", chip_id);
> +                     "interrupt [chip id = %x]\n", sbe->chip_id);
> +               return;
> +       }
> +
> +       /* SBE came back from reset */
> +       if (data & SBE_HOST_RESET) {
> +               prlog(PR_NOTICE, "Back from reset [chip id = %x]\n", sbe->chip_id);
> +               /* Reset SBE MBOX state */
> +               sbe->state = sbe_mbox_idle;
> +
> +               /* Reset message state */
> +               if (!list_empty(&sbe->msg_list)) {
> +                       msg = list_top(&sbe->msg_list, struct p9_sbe_msg, link);
> +                       msg->state = sbe_msg_queued;
> +                }
> +               goto next_msg;
> +       }
> +
> +       /* SBE passthrough command, call prd handler */
> +       if (data & SBE_HOST_PASSTHROUGH) {
> +               prd_sbe_passthrough(sbe->chip_id);
> +       }
> +
> +       /* Process ACK message before response */
> +       if (data & SBE_HOST_MSG_READ) {
> +               p9_sbe_send_complete(sbe);
>         }
> +
> +       /* Got response */
> +       if (has_response) {
> +               /* Reset SBE MBOX state */
> +               sbe->state = sbe_mbox_idle;
> +               p9_sbe_msg_complete(sbe, msg, sbe_msg_done);
> +       }
> +
> +next_msg:
> +       p9_sbe_process_queue(sbe);
> +}
> +