linux-kernel - Re: [PATCH v2 3/3] mailbox: mediatek: Add mtk-apu-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] [day] [month] [year] [list]

Message-ID: <CAGXv+5E98VZnf_PL2kEF=VxytNVyCEV4iY=aEnEoZz0Z7AJiTg@mail.gmail.com>
Date: Fri, 3 Jan 2025 12:56:30 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: Jassi Brar <jassisinghbrar@...il.com>
Cc: "Karl.Li" <karl.li@...iatek.com>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-mediatek@...ts.infradead.org, Chungying Lu <chungying.lu@...iatek.com>, 
	Chien-Chih Tseng <Chien-Chih.Tseng@...iatek.com>, Andy Teng <Andy.Teng@...iatek.com>, 
	Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v2 3/3] mailbox: mediatek: Add mtk-apu-mailbox driver

On Fri, Jan 3, 2025 at 12:30 PM Jassi Brar <jassisinghbrar@...il.com> wrote:
>
> On Sun, Dec 29, 2024 at 9:46 PM Karl.Li <karl.li@...iatek.com> wrote:
> ....
> > +
> > +static irqreturn_t mtk_apu_mailbox_irq(int irq, void *data)
> > +{
> > +       struct mbox_chan *chan = data;
> > +       struct mtk_apu_mailbox *apu_mbox = get_mtk_apu_mailbox(chan->mbox);
> > +       struct mbox_chan *link = &apu_mbox->mbox.chans[0];
> > +       u8 data_cnt = fls(readl(apu_mbox->regs + MTK_APU_MBOX_OUTBOX_IRQ));
> > +
> > +       memcpy_fromio(apu_mbox->msgs.data, apu_mbox->regs + MTK_APU_MBOX_OUTBOX,
> > +                     sizeof(*apu_mbox->msgs.data) * data_cnt);
> > +
> > +       mbox_chan_received_data(link, apu_mbox->msgs.data);
> > +
> > +       return IRQ_WAKE_THREAD;
> > +}
> > +
> You don't seem to do anything that 'ack' the irq line. So if you merge
> this into mtk_apu_mailbox_irq_thread() and do
>  devm_request_threaded_irq(dev, irq,   NULL,
> mtk_apu_mailbox_irq_thread,  IRQF_ONESHOT,  ...);
> you can avoid adding a new api and keep your client code simple.
> mbox_chan_received_data() is strictly between the client and
> controller driver. If it is not called from an atomic context, feel
> free to do memcpy() from it.

That sounds great. Could we perhaps update the API documentation and
drop the "atomic" requirement, or update it to something you describe
here as "between the client and controller driver".

Disclosure: I've been doing pre-reviews for this series and we were
operating under the assumption that mbox_chan_received_data() and
its callbacks must work under atomic contexts.


Thanks
ChenYu