All of lore.kernel.org
 help / color / mirror / Atom feed
From: leo.yan@linaro.org (Leo Yan)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 2/3] mailbox: Hi6220: add mailbox driver
Date: Thu, 20 Aug 2015 21:44:38 +0800	[thread overview]
Message-ID: <20150820134438.GA18629@leoy-linaro> (raw)
In-Reply-To: <55D568E2.9060505@hisilicon.com>

Hi Yiping,

Thanks for review, please see below comments.

On Thu, Aug 20, 2015 at 01:42:58PM +0800, YiPing Xu wrote:
> On 2015/8/20 10:53, Leo Yan wrote:
> >Add driver for Hi6220 mailbox, the mailbox communicates with MCU; for
> >sending data, it can support two methods for low level implementation:
> >one is to use interrupt as acknowledge, another is automatic mode which
> >without any acknowledge. These two methods have been supported in the
> >driver. For receiving data, it will depend on the interrupt to notify
> >the channel has incoming message; enhance rx channel's message queue,
> >which is based on the code in drivers/mailbox/omap-mailbox.c.
> >
> >Now mailbox driver is used to send message to MCU to control dynamic
> >voltage and frequency scaling for CPU, GPU and DDR.
> >
> >Signed-off-by: Leo Yan <leo.yan@linaro.org>

[...]

> >+static int hi6220_mbox_send_data(struct mbox_chan *chan, void *msg)
> >+{
> >+	struct hi6220_mbox_chan *mchan = chan->con_priv;
> >+	struct hi6220_mbox *mbox = mchan->parent;
> >+	int irq = mchan->remote_irq;
> >+	u32 *buf = msg;
> >+	unsigned long flags;
> >+	int i;
> >+
> >+	hi6220_mbox_set_status(mchan, HI6220_MBOX_STATUS_TX);
> 
> 	hi6220_mbox_set_status is called in send_data context, and it is
> also called in hi6220_mbox_rx_interrupt and
> hi6220_mbox_tx_interrupt.
> 
> 	no race condition here?

Have thought this question yet when writing code; it will _NOT_
introduce race condition according to below implementation details:

- Every channel have its own slot, so there have no race condition
  between channels;
- The channel is unidirectional; so it will be only for tx or rx at
  the same time;
- The channel operation is sequential, so it have the sequence as:
  For tx: startup -> send_data -> tx_irq (for ack);
  For rx: startup -> rx_irq -> recv_data;

Thanks,
Leo Yan

WARNING: multiple messages have this Message-ID (diff)
From: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
To: YiPing Xu <xuyiping-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	Ian Campbell
	<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
	galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>,
	Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>,
	Jassi Brar
	<jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Bintian Wang
	<bintian.wang-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>,
	haojian.zhuang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	Wei Xu <xuwei5-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	guodong.xu-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	Jian Zhang <zhangjian001-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>,
	Zhenwei Wang
	<Zhenwei.wang-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>,
	Haoju Mo <mohaoju-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>,
	Dan Zhao <dan.zhao-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>,
	kongfei-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org,
	Guangyue Zeng
	<zengguangyue-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Subject: Re: [PATCH v2 2/3] mailbox: Hi6220: add mailbox driver
Date: Thu, 20 Aug 2015 21:44:38 +0800	[thread overview]
Message-ID: <20150820134438.GA18629@leoy-linaro> (raw)
In-Reply-To: <55D568E2.9060505-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>

Hi Yiping,

Thanks for review, please see below comments.

On Thu, Aug 20, 2015 at 01:42:58PM +0800, YiPing Xu wrote:
> On 2015/8/20 10:53, Leo Yan wrote:
> >Add driver for Hi6220 mailbox, the mailbox communicates with MCU; for
> >sending data, it can support two methods for low level implementation:
> >one is to use interrupt as acknowledge, another is automatic mode which
> >without any acknowledge. These two methods have been supported in the
> >driver. For receiving data, it will depend on the interrupt to notify
> >the channel has incoming message; enhance rx channel's message queue,
> >which is based on the code in drivers/mailbox/omap-mailbox.c.
> >
> >Now mailbox driver is used to send message to MCU to control dynamic
> >voltage and frequency scaling for CPU, GPU and DDR.
> >
> >Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

[...]

> >+static int hi6220_mbox_send_data(struct mbox_chan *chan, void *msg)
> >+{
> >+	struct hi6220_mbox_chan *mchan = chan->con_priv;
> >+	struct hi6220_mbox *mbox = mchan->parent;
> >+	int irq = mchan->remote_irq;
> >+	u32 *buf = msg;
> >+	unsigned long flags;
> >+	int i;
> >+
> >+	hi6220_mbox_set_status(mchan, HI6220_MBOX_STATUS_TX);
> 
> 	hi6220_mbox_set_status is called in send_data context, and it is
> also called in hi6220_mbox_rx_interrupt and
> hi6220_mbox_tx_interrupt.
> 
> 	no race condition here?

Have thought this question yet when writing code; it will _NOT_
introduce race condition according to below implementation details:

- Every channel have its own slot, so there have no race condition
  between channels;
- The channel is unidirectional; so it will be only for tx or rx at
  the same time;
- The channel operation is sequential, so it have the sequence as:
  For tx: startup -> send_data -> tx_irq (for ack);
  For rx: startup -> rx_irq -> recv_data;

Thanks,
Leo Yan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Leo Yan <leo.yan@linaro.org>
To: YiPing Xu <xuyiping@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	galak@codeaurora.org, Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Jassi Brar <jassisinghbrar@gmail.com>,
	Bintian Wang <bintian.wang@huawei.com>,
	haojian.zhuang@linaro.org, Wei Xu <xuwei5@hisilicon.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, guodong.xu@linaro.org,
	Jian Zhang <zhangjian001@hisilicon.com>,
	Zhenwei Wang <Zhenwei.wang@hisilicon.com>,
	Haoju Mo <mohaoju@hisilicon.com>,
	Dan Zhao <dan.zhao@hisilicon.com>,
	kongfei@hisilicon.com, Guangyue Zeng <zengguangyue@hisilicon.com>
Subject: Re: [PATCH v2 2/3] mailbox: Hi6220: add mailbox driver
Date: Thu, 20 Aug 2015 21:44:38 +0800	[thread overview]
Message-ID: <20150820134438.GA18629@leoy-linaro> (raw)
In-Reply-To: <55D568E2.9060505@hisilicon.com>

Hi Yiping,

Thanks for review, please see below comments.

On Thu, Aug 20, 2015 at 01:42:58PM +0800, YiPing Xu wrote:
> On 2015/8/20 10:53, Leo Yan wrote:
> >Add driver for Hi6220 mailbox, the mailbox communicates with MCU; for
> >sending data, it can support two methods for low level implementation:
> >one is to use interrupt as acknowledge, another is automatic mode which
> >without any acknowledge. These two methods have been supported in the
> >driver. For receiving data, it will depend on the interrupt to notify
> >the channel has incoming message; enhance rx channel's message queue,
> >which is based on the code in drivers/mailbox/omap-mailbox.c.
> >
> >Now mailbox driver is used to send message to MCU to control dynamic
> >voltage and frequency scaling for CPU, GPU and DDR.
> >
> >Signed-off-by: Leo Yan <leo.yan@linaro.org>

[...]

> >+static int hi6220_mbox_send_data(struct mbox_chan *chan, void *msg)
> >+{
> >+	struct hi6220_mbox_chan *mchan = chan->con_priv;
> >+	struct hi6220_mbox *mbox = mchan->parent;
> >+	int irq = mchan->remote_irq;
> >+	u32 *buf = msg;
> >+	unsigned long flags;
> >+	int i;
> >+
> >+	hi6220_mbox_set_status(mchan, HI6220_MBOX_STATUS_TX);
> 
> 	hi6220_mbox_set_status is called in send_data context, and it is
> also called in hi6220_mbox_rx_interrupt and
> hi6220_mbox_tx_interrupt.
> 
> 	no race condition here?

Have thought this question yet when writing code; it will _NOT_
introduce race condition according to below implementation details:

- Every channel have its own slot, so there have no race condition
  between channels;
- The channel is unidirectional; so it will be only for tx or rx at
  the same time;
- The channel operation is sequential, so it have the sequence as:
  For tx: startup -> send_data -> tx_irq (for ack);
  For rx: startup -> rx_irq -> recv_data;

Thanks,
Leo Yan

  reply	other threads:[~2015-08-20 13:44 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-20  2:53 [PATCH v2 0/3] mailbox: hisilicon: add Hi6220 mailbox driver Leo Yan
2015-08-20  2:53 ` Leo Yan
2015-08-20  2:53 ` [PATCH v2 1/3] dt-bindings: mailbox: Document " Leo Yan
2015-08-20  2:53   ` Leo Yan
2015-08-20  2:53 ` [PATCH v2 2/3] mailbox: Hi6220: add " Leo Yan
2015-08-20  2:53   ` Leo Yan
2015-08-20  5:42   ` YiPing Xu
2015-08-20  5:42     ` YiPing Xu
2015-08-20  5:42     ` YiPing Xu
2015-08-20 13:44     ` Leo Yan [this message]
2015-08-20 13:44       ` Leo Yan
2015-08-20 13:44       ` Leo Yan
2015-08-20  2:53 ` [PATCH v2 3/3] arm64: dts: add Hi6220 mailbox node Leo Yan
2015-08-20  2:53   ` Leo Yan
2015-08-20  2:53   ` Leo Yan
2015-08-24 10:00   ` Mark Rutland
2015-08-24 10:00     ` Mark Rutland
2015-08-24 10:00     ` Mark Rutland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150820134438.GA18629@leoy-linaro \
    --to=leo.yan@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.