Re: FW: [RFC 3/3] CMDQ: Mediatek CMDQ driver

From: Daniel Kurtz <djkurtz@chromium.org>
To: Horng-Shyang Liao <hs.liao@mediatek.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>,
	"open list:OPEN FIRMWARE AND..." <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"moderated list:ARM/Mediatek SoC support" 
	<linux-mediatek@lists.infradead.org>,
	srv_heupstream <srv_heupstream@mediatek.com>,
	Sascha Hauer <kernel@pengutronix.de>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Nicolas Boichat <drinkcat@chromium.org>,
	CK HU <ck.hu@mediatek.com>, cawa cheng <cawa.cheng@mediatek.com>,
	Bibby Hsieh <bibby.hsieh@mediatek.com>,
	YT Shen <yt.shen@mediatek.com>,
	Daoyuan Huang <daoyuan.huang@mediatek.com>,
	Damon Chu <damon.chu@mediatek.com>,
	Josh-YC Liu <josh-yc.liu@mediatek.com>,
	Glory Hung <glory.hung@mediatek.com>,
	Yong Wu <yong.wu@mediatek.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Rob Herring <robh+dt@kernel.org>
Subject: Re: FW: [RFC 3/3] CMDQ: Mediatek CMDQ driver
Date: Wed, 3 Feb 2016 14:40:47 +0800	[thread overview]
Message-ID: <CAGS+omABD93f3X0pfiFgaxa9Lob2txkJfUUomUvs+hiRkxSuHw@mail.gmail.com> (raw)
In-Reply-To: <1454479333.11967.12.camel@mtksdaap41>

> Hi Dan,
>
> Thanks for your comment.
> This solution looks good to me.
> I will change it as your suggestion.
>
> But, I have a question about 'mask out the provided *device virtual*
> address'.
> Are lower 16-bits (or 24-bits for JUMP op) of device virtual address the
> same as device physical address?

I'm not sure.  But I doubt it we can rely on this.
My guess would be that the ioremap only preserves the lower 12 bits
(4k page size).

> If not, we still need to pass in physical address into CMDQ driver.

Or, instead of the iommu/slot approach, we can just provide a
registration function for the gce driver.
Each gce consumer could then have a simple gce node, with no slot/address:

  mediatek,gce = <&gce>;

Then on probe, the gce consumer could pass in its (struct device *) to
gce_register_device().  gce_register_device() could then access the
device's of_node to extract its physical address range, and look up
its physical address in its table of per-soc of
"device_address:gce_subsys_address" entries.  If the physical address
is in a valid subsys ranges, the gce_register_device would cache the
subsys address, and an offset in a (struct gce_consumer).
gce_register_device() could then add this struct to a struct list_head
of gce_consumers, and finally return a pointer to it back to the
caller.

Later, the gce consumer could pass in ths (struct gce_consumer *) when
make gce calls, along with the *offset* (not the physical address or
virtual address) for the register that it wishes to access.  Then the
gce driver can simply use the gce_consumer->subsys entry to create a
gce address from the passed in offset.

This will keep the binding very simple, and would remove the need to
convert from device virtual to physical addresses by the gce consumer,
but require a little more per-gce-consumer setup.

-Dan