lkml.org 
[lkml]   [2014]   [Jun]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCHv5 2/4] mailbox: Introduce framework for mailbox
From
On Mon, Jun 2, 2014 at 8:44 PM, Matt Porter <mporter@linaro.org> wrote:
> On Fri, May 30, 2014 at 11:01:55AM +0530, Jassi Brar wrote:
>
>> Being more specific to your platform, I think you need some server
>> code (mailbox's client) that every driver (like clock, pmu, pinmux
>> etc) registers with to send messages to remote and receive commands
>> from remote ... perhaps by registering some filter to sort out
>> messages for each driver.
>
> Right, and here's where you hit on the problem. This server you mention
> is not a piece of hardware, it would be a software construct. As such, it
> doesn't fit into the DT binding as it exists. It's probably best to
> illustrate in DT syntax.
>
> If I were to represent the hardware relationship in the DT binding now
> it would look like this:
>
> ---
> cpm: mailbox@deadbeef {
> compatible = "brcm,bcm-cpm-mailbox";
> reg = <...>;
> #mbox-cells <1>;
> interrupts = <...>;
> };
>
> /* clock complex */
> ccu {
> compatible = "brcm,bcm-foo-ccu";
> mbox = <&cpm CPM_SYSTEM_CHANNEL>;
> mbox-names = "system";
> /* leaving out other mailboxes for brevity */
> #clock-cells <1>;
> clock-output-names = "bar",
> "baz";
> };
>
> pmu {
> compatible = "brcm,bcm-foo-pmu"
> mbox = <&cpm CPM_SYSTEM_CHANNEL>;
> mbox-names = "system";
> };
>
> pinmux {
> compatible = "brcm,bcm-foo-pinctrl";
> mbox = <&cpm CPM_SYSTEM_CHANNEL>;
> mbox-names = "system";
> };
> ---
Yeah, I too don't think its a good idea.


> What we would need to do is completely ignore this information in each
> of the of the client drivers associated with the clock, pmu, and pinmux
> devices. This IPC server would need to be instantiated and get the
> mailbox information from some source. mbox_request_channel() only works
> when the client has an of_node with the mbox-names property present.
> Let's say we follow this model and represent it in DT:
>
> --
> cpm: mailbox@deadbeef {
> compatible = "brcm,bcm-cpm-mailbox";
> reg = <...>;
> #mbox-cells <1>;
> interrupts = <...>;
> };
>
> cpm_ipc {
> compatible = "brcm,bcm-cpm-ipc";
> mbox = <&cpm CPM_SYSTEM_CHANNEL>;
> mbox-names = "system";
> /* leaving out other mailboxes for brevity */
> };
> ---
>
> This would allow an ipc driver to exclusively own this system channel,
> but now we've invented a binding that doesn't reflect the hardware at
> all. It's describing software so I don't believe the DT maintainers will
> allow this type of construct.
>
Must the server node specify MMIO and an IRQ, to be acceptable? Like ...

cpm_ipc : cpm@deadbeef {
compatible = "brcm,bcm-cpm-ipc";
/* reg = <0xdeadbeef 0x100>; */
/* interrupts = <0 123 4>; */
mbox = <&cpm CPM_SYSTEM_CHANNEL>;
mbox-names = "system";
};

cpm_ipc already specifies a hardware resource (mbox) that its driver
needs, I think that should be enough reason. If it were some purely
soft property for the driver like
mode = "poll"; //or "irq"
then the node wouldn't be justified because that is the job of a
build-time config or run-time module option.

Regards,
-Jassi


\
 
 \ /
  Last update: 2014-06-02 19:41    [W:0.199 / U:5.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site