Hi Conor, Rajeev,

> +static int svc_i3c_master_handle_ibi(struct svc_i3c_master *master,
> +                                  struct i3c_dev_desc *dev)
> +{
> +     struct svc_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
> +     struct i3c_ibi_slot *slot;
> +     unsigned int count;
> +     u32 mdatactrl;
> +     int ret = 0;
> +     u8 *buf;
> +
> +     spin_lock(&master->ibi.lock);
> +
> +     slot = i3c_generic_ibi_get_free_slot(data->ibi_pool);
> +     if (!slot) {
> +             ret = -ENOSPC;
> +             goto unlock;
> +     }
> +
> +     slot->len = 0;
> +     buf = slot->data;
> +     while (readl(master->regs + SVC_I3C_MSTATUS) & SVC_I3C_MINT_RXPEND) {
> +             mdatactrl = readl(master->regs + SVC_I3C_MDATACTRL);
> +             count = SVC_I3C_MDATACTRL_RXCOUNT(mdatactrl);
> +             readsl(master->regs + SVC_I3C_MRDATAB, buf, count);

After discussing with Boris, I have a question about the design: is
there a way to differentiate, from a software point of view, from the
data coming as "IBI" and "regular data"?

Let's say the master initiates a read.
The moment after, an IBI is triggered.
The hanlde_ibi() helper is called to read the IBI payload.
While the IBI interrupted the read operation, it also interrupted the
master dequeuing process of the bytes already in the FIFO. From a
software perspective, we might end up reading a "regular byte" from the
handle_ibi() helper expecting an "IBI byte".

Is there some kind of double queue feature which I missed? Or perhaps a
way to discriminate the origin of the data?

> +             slot->len += count;
> +             buf += count;
> +     }
> +
> +     i3c_master_queue_ibi(dev, slot);
> +
> +unlock:
> +     spin_unlock(&master->ibi.lock);
> +     svc_i3c_master_emit_stop(master);
> +     svc_i3c_master_flush_fifo(master);
> +
> +     return ret;
> +}
> +
> +static void svc_i3c_master_ack_ibi(struct svc_i3c_master *master,
> +                                bool mandatory_byte)
> +{
> +     unsigned int ibi_ack_nack;
> +
> +     ibi_ack_nack = SVC_I3C_MCTRL_REQUEST_IBI_ACKNACK;
> +     if (mandatory_byte)
> +             ibi_ack_nack |= SVC_I3C_MCTRL_IBIRESP_ACK_WITH_BYTE;
> +     else
> +             ibi_ack_nack |= SVC_I3C_MCTRL_IBIRESP_ACK_WITHOUT_BYTE;
> +
> +     writel(ibi_ack_nack, master->regs + SVC_I3C_MCTRL);
> +}
> +
> +static void svc_i3c_master_nack_ibi(struct svc_i3c_master *master)
> +{
> +     writel(SVC_I3C_MCTRL_REQUEST_IBI_ACKNACK |
> +            SVC_I3C_MCTRL_IBIRESP_NACK,
> +            master->regs + SVC_I3C_MCTRL);
> +}
> +
> +static irqreturn_t svc_i3c_master_irq_handler(int irq, void *dev_id)
> +{
> +     struct svc_i3c_master *master = (struct svc_i3c_master *)dev_id;
> +     u32 active = readl(master->regs + SVC_I3C_MINTMASKED);
> +     u32 status = readl(master->regs + SVC_I3C_MSTATUS);
> +     unsigned int ibitype = SVC_I3C_MSTATUS_IBITYPE(status);
> +     unsigned int ibiaddr = SVC_I3C_MSTATUS_IBIADDR(status);
> +     struct i3c_dev_desc *dev;
> +     bool rdata;
> +
> +     if (active & SVC_I3C_MINT_SLVSTART) {
> +             writel(SVC_I3C_MINT_SLVSTART, master->regs + SVC_I3C_MSTATUS);
> +             writel(SVC_I3C_MCTRL_REQUEST_AUTO_IBI |
> +                    SVC_I3C_MCTRL_IBIRESP_MANUAL,
> +                    master->regs + SVC_I3C_MCTRL);
> +             return IRQ_HANDLED;
> +     }
> +
> +     if (!(active & SVC_I3C_MINT_IBIWON))
> +             return IRQ_NONE;
> +
> +     writel(SVC_I3C_MINT_IBIWON, master->regs + SVC_I3C_MSTATUS);
> +
> +     switch (ibitype) {
> +     case SVC_I3C_MSTATUS_IBITYPE_IBI:
> +             dev = svc_i3c_master_dev_from_addr(master, ibiaddr);
> +             if (WARN_ON(!dev)) {
> +                     svc_i3c_master_nack_ibi(master);
> +                     break;
> +             }
> +
> +             rdata = dev->info.bcr & I3C_BCR_IBI_PAYLOAD;
> +             svc_i3c_master_ack_ibi(master, rdata);
> +             if (rdata) {
> +                     svc_i3c_master_disable_interrupts(master);
> +                     return IRQ_WAKE_THREAD;
> +             }
> +
> +             break;
> +     case SVC_I3C_MSTATUS_IBITYPE_MASTER_REQUEST:
> +             svc_i3c_master_nack_ibi(master);
> +             break;
> +     case SVC_I3C_MSTATUS_IBITYPE_HOT_JOIN:
> +             svc_i3c_master_ack_ibi(master, false);
> +             queue_work(master->base.wq, &master->hj_work);
> +             break;
> +     default:
> +             return IRQ_NONE;
> +     }
> +
> +     return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t svc_i3c_master_threaded_handler(int irq, void *dev_id)
> +{
> +     struct svc_i3c_master *master = (struct svc_i3c_master *)dev_id;
> +     u32 status = readl(master->regs + SVC_I3C_MSTATUS);
> +     unsigned int ibiaddr = SVC_I3C_MSTATUS_IBIADDR(status);
> +     struct i3c_dev_desc *dev;
> +
> +     dev = svc_i3c_master_dev_from_addr(master, ibiaddr);
> +     if (WARN_ON(!dev)) {
> +             svc_i3c_master_emit_stop(master);
> +             svc_i3c_master_flush_fifo(master);
> +             return IRQ_HANDLED;
> +     }
> +
> +     svc_i3c_master_handle_ibi(master, dev);
> +     svc_i3c_master_enable_interrupts(master,
> +                                      SVC_I3C_MINT_SLVSTART |
> +                                      SVC_I3C_MINT_IBIWON);
> +
> +     return IRQ_HANDLED;
> +}

Thanks,
Miquèl

Reply via email to