linux-kernel - Re: [PATCH (resend)] mailbox: Add Altera mailbox driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFiDJ58etaJxO3csGzeTYq3j0a4=e-eu4MpTn3Xd8BtJmzssNA@mail.gmail.com>
Date:	Wed, 17 Dec 2014 10:30:41 +0800
From:	Ley Foon Tan <lftan@...era.com>
To:	Suman Anna <s-anna@...com>
Cc:	Dinh Nguyen <dinh.linux@...il.com>,
	Jassi Brar <jassisinghbrar@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	devicetree@...r.kernel.org
Subject: Re: [PATCH (resend)] mailbox: Add Altera mailbox driver

On Wed, Dec 17, 2014 at 3:53 AM, Suman Anna <s-anna@...com> wrote:

>>> I think this logic in general will conflict between interrupt and poll
>>> mode. send_data is supposed to return -EBUSY only in polling mode and
>>> not in interrupt mode.
>> What is the recommended error code for this case? BTW, omap-mailbox.c
>> also return -EBUSY if mailbox is full.
>
> Looks like the mbox core doesn't care about the error, it's just
> checking for non-success case. I made that comment based on the
> documentation in last_tx_done. OMAP mailbox is always interrupt driven,
> and it is very rare that we will ever hit the mbox full because of the
> Tx-ready interrupt driven Tx ticker and also the h/w fifo.
>
> I see that your mailbox is very similar to OMAP mailbox though without
> the h/w fifo, but you do have support for both polling and interrupt
> modes in your code. OMAP mailbox can do that as well, but it's
> inefficient to use polling so the driver is strictly interrupt driven.
Yes, you are right. So, I think -EBUSY still can use it in interrupt case.



>>>>> +
>>>>> +    mbox->mbox_base = devm_ioremap_resource(&pdev->dev, regs);
>>>>> +    if (IS_ERR(mbox->mbox_base))
>>>>> +            return PTR_ERR(mbox->mbox_base);
>>>>> +
>>>>> +    /* Check is it a sender or receiver? */
>>>>> +    mbox->is_sender = altera_mbox_is_sender(mbox);
>>>>> +
>>>>> +    mbox->irq = platform_get_irq(pdev, 0);
>>>>> +    if (mbox->irq >= 0)
>>>>> +            mbox->intr_mode = true;
>>>
>>> This seems to be a poor way of setting up the mode. Is it the same IP
>>> block but different integration on different SoCs? Or on the same SoC,
>>> and you can use it either by interrupt driven or by polling.
>> Yes, it can use interrupt or polling mode depends on hardware configurations.
>> It is a soft IP and can be configured with different hardware configurations.
>
> OK, the platform_get_irq suggests that this decision is made by having
> or not having the interrupts property in the DT node, is that the only
> way of differentiating this mode?
Yes, we don't have other DTS property to identify the interrupt mode.

Regards
Ley Foon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/