From patchwork Fri Jun 20 18:55:16 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ashwin Chaugule X-Patchwork-Id: 32295 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-pb0-f70.google.com (mail-pb0-f70.google.com [209.85.160.70]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id D062F203C2 for ; Fri, 20 Jun 2014 18:55:19 +0000 (UTC) Received: by mail-pb0-f70.google.com with SMTP id rp16sf14244544pbb.1 for ; Fri, 20 Jun 2014 11:55:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mime-version:in-reply-to:references :date:message-id:subject:from:to:cc:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :list-post:list-help:list-archive:list-unsubscribe:content-type; bh=vloKX6yGsovkZOGnK1k9NWlwlnH6pWvB5ewRKcxUgE4=; b=ZuoLRm4orwhheZxPzIb5aKmLxM+T87IJwq4+J//7By49T7ggZatwTBUXffZZ8OfE4v iEl3KQOq+FoftKbSSLiMXfiwbiPZNStCQDby+iqTnkSY8/7MqKg1XCx6rnPaYecVDsWG CwZ+u/LlFJvXAkwak3H13OE8s09UA14PSvVFSVad4G27A3z7FoddZcglFGsKDnLyQBxW uuADjcZS13x1K3mSYq3p9uLn4IF18fd+YsEYrPqhzCzd9oaiCQuB/jkRmS802pgf/boN lWWsik+kq5T6R0S+EjkJaeywpigw36mQ/q2PxBJcQno4ebi1acZHlOGxYH+ZxvGxtyj1 Jbsg== X-Gm-Message-State: ALoCoQkF1QcVACaqdseKuTE0tTpQfDhL8OQrIPlZ+jNBMwLUKpgh25idGzohn2lg6P5wDdTBJIwj X-Received: by 10.66.164.167 with SMTP id yr7mr2076582pab.15.1403290519071; Fri, 20 Jun 2014 11:55:19 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.27.131 with SMTP id 3ls1076685qgx.9.gmail; Fri, 20 Jun 2014 11:55:18 -0700 (PDT) X-Received: by 10.221.69.68 with SMTP id yb4mr2006331vcb.49.1403290518736; Fri, 20 Jun 2014 11:55:18 -0700 (PDT) Received: from mail-ve0-f172.google.com (mail-ve0-f172.google.com [209.85.128.172]) by mx.google.com with ESMTPS id u4si4598648vcs.52.2014.06.20.11.55.18 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 20 Jun 2014 11:55:18 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.128.172 as permitted sender) client-ip=209.85.128.172; Received: by mail-ve0-f172.google.com with SMTP id jz11so3995091veb.3 for ; Fri, 20 Jun 2014 11:55:18 -0700 (PDT) X-Received: by 10.220.43.193 with SMTP id x1mr1974269vce.57.1403290518609; Fri, 20 Jun 2014 11:55:18 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.221.37.5 with SMTP id tc5csp43799vcb; Fri, 20 Jun 2014 11:55:18 -0700 (PDT) X-Received: by 10.180.189.234 with SMTP id gl10mr6221957wic.56.1403290517552; Fri, 20 Jun 2014 11:55:17 -0700 (PDT) Received: from mail-wg0-f46.google.com (mail-wg0-f46.google.com [74.125.82.46]) by mx.google.com with ESMTPS id t11si3642163wib.5.2014.06.20.11.55.17 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 20 Jun 2014 11:55:17 -0700 (PDT) Received-SPF: pass (google.com: domain of ashwin.chaugule@linaro.org designates 74.125.82.46 as permitted sender) client-ip=74.125.82.46; Received: by mail-wg0-f46.google.com with SMTP id y10so4116459wgg.5 for ; Fri, 20 Jun 2014 11:55:16 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.194.200.3 with SMTP id jo3mr6067966wjc.110.1403290516503; Fri, 20 Jun 2014 11:55:16 -0700 (PDT) Received: by 10.217.67.6 with HTTP; Fri, 20 Jun 2014 11:55:16 -0700 (PDT) In-Reply-To: References: <1402591692-7736-1-git-send-email-ashwin.chaugule@linaro.org> <1402591692-7736-2-git-send-email-ashwin.chaugule@linaro.org> <4476889.LvjMyVbGZ8@wuerfel> Date: Fri, 20 Jun 2014 14:55:16 -0400 Message-ID: Subject: Re: [RFC v2 1/3] Mailbox: Add support for ACPI From: Ashwin Chaugule To: Arnd Bergmann Cc: "linux-arm-kernel@lists.infradead.org" , linux-acpi@vger.kernel.org, Patch Tracking , Jassi Brar , rjw@rjwysocki.net, "linaro-acpi@lists.linaro.org" , Mark Brown X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: ashwin.chaugule@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.128.172 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Hello, On 12 June 2014 13:14, Ashwin Chaugule wrote: > On 12 June 2014 13:02, Arnd Bergmann wrote: >> On Thursday 12 June 2014 12:48:10 Ashwin Chaugule wrote: >>> >>> +#ifndef CONFIG_ACPI >>> if (!mbox->of_xlate) >>> mbox->of_xlate = of_mbox_index_xlate; >>> - >>> +#endif >>> mutex_lock(&con_mutex); >>> list_add_tail(&mbox->node, &mbox_cons); >>> mutex_unlock(&con_mutex); >>> >> >> You can't do #ifndef here, the driver must still work if >> both OF and ACPI are enabled. > > Ok. Here we could skip the macro altogether, since of_xlate wont be > called with ACPI anyway. > > Will need to look into using/faking mbox->dev to remove the other ifndefs. So, in order to get an mbox->dev for ACPI platforms, we'd need an entry in the DSDT table. That seems rather pointless, since the DSDT is reserved for devices and is supposed to be OS agnostic. Since the mailbox controller itself is not really a "device" with a resource descriptor, I dont see the point in adding a dummy DSDT entry for the sake of getting this `struct device`. Also, I'm told adding new entries to this table requires registering a unique 4 character identifier and approval from some committees. If there are other ways to get this structure I'd like to hear about it. The other alternative would be to piggy back on the ACPI CPU detection code, which looks for the ACPI0007 device node in the DSDT and use that as the mbox controller device. This node is already registered and is an established method to detect CPUs. But I'm not sure what happens when CPUs are hotplugged off, we surely dont want mailbox clients such as PCC to break. The third alternative is to ignore the dev refcounts for ACPI as shown in this totally untested patch: ----------------8<---------------------------------------- >From 6fe6e583f0b23b08643a4a85545a9a5338b9b1a0 Mon Sep 17 00:00:00 2001 From: Ashwin Chaugule Date: Wed, 11 Jun 2014 16:09:35 -0400 Subject: [PATCH] Mailbox: Add support for ACPI The current mailbox framework only supports DT based bindings. Add another mechanism for mailbox clients to register with mailbox controllers and request for specific mailbox channels. This enables usage of the mailbox framework on kernels with ACPI support. Signed-off-by: Ashwin Chaugule --- drivers/mailbox/mailbox.c | 177 ++++++++++++++++++++++++++----------- include/linux/mailbox_client.h | 2 +- include/linux/mailbox_controller.h | 1 + 3 files changed, 129 insertions(+), 51 deletions(-) diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c index d83d12c..e2704f5 100644 --- a/drivers/mailbox/mailbox.c +++ b/drivers/mailbox/mailbox.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -273,100 +274,166 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg) } EXPORT_SYMBOL_GPL(mbox_send_message); -/** - * mbox_request_channel - Request a mailbox channel. - * @cl: Identity of the client requesting the channel. - * - * The Client specifies its requirements and capabilities while asking for - * a mailbox channel by name. It can't be called from atomic context. - * The channel is exclusively allocated and can't be used by another - * client before the owner calls mbox_free_channel. - * After assignment, any packet received on this channel will be - * handed over to the client via the 'rx_callback'. - * - * Return: Pointer to the channel assigned to the client if successful. - * ERR_PTR for request failure. - */ -struct mbox_chan *mbox_request_channel(struct mbox_client *cl) +static int init_channel(struct mbox_chan *chan, + struct mbox_client *cl) +{ + unsigned long flags; + int ret; + + if (!chan) { + pr_err("No mailbox channel specified\n"); + return -EINVAL; + } + + spin_lock_irqsave(&chan->lock, flags); + chan->msg_free = 0; + chan->msg_count = 0; + chan->active_req = NULL; + chan->cl = cl; + + if (!cl->tx_tout) /* wait for ever */ + cl->tx_tout = msecs_to_jiffies(3600000); + else + cl->tx_tout = msecs_to_jiffies(cl->tx_tout); + if (chan->txdone_method == TXDONE_BY_POLL + && cl->knows_txdone) + chan->txdone_method |= TXDONE_BY_ACK; + spin_unlock_irqrestore(&chan->lock, flags); + + ret = chan->mbox->ops->startup(chan); + if (ret) { + pr_err("Unable to startup the chan\n"); + mbox_free_channel(chan); + chan = ERR_PTR(ret); + } + + return ret; +} + +static int get_acpi_mbox_chan(struct mbox_client *cl, + struct mbox_chan **chan) +{ + struct mbox_controller *mbox; + int chan_id, ret, len; + char *chan_ptr; + + if (!cl->chan_name) + return -ENODEV; + + list_for_each_entry(mbox, &mbox_cons, node) { + if (mbox->name) { + /* + * The cl->chan_name has the format => controller:channel + * as described in mailbox_client.h + */ + + len = strlen(mbox->name); + chan_ptr = cl->chan_name + len + 1; + + ret = kstrtou32(cl->chan_name, 0, &chan_id); + + if (ret < 0) { + pr_err("Err while parsing mailbox:%s channel idx\n", + mbox->name); + continue; + } + + if (!strncmp(cl->chan_name, mbox->name, len)) { + *chan = &mbox->chans[chan_id]; + return init_channel(*chan, cl); + } + } + } + + return -ENODEV; +} + +static int get_of_mbox_chan(struct mbox_client *cl, + struct mbox_chan **chan) { struct device *dev = cl->dev; struct mbox_controller *mbox; struct of_phandle_args spec; - struct mbox_chan *chan; - unsigned long flags; int count, i, ret; if (!dev || !dev->of_node) { pr_err("%s: No owner device node\n", __func__); - return ERR_PTR(-ENODEV); + return -ENODEV; } count = of_property_count_strings(dev->of_node, "mbox-names"); if (count < 0) { pr_err("%s: mbox-names property of node '%s' missing\n", __func__, dev->of_node->full_name); - return ERR_PTR(-ENODEV); + return -ENODEV; } - - mutex_lock(&con_mutex); - - ret = -ENODEV; for (i = 0; i < count; i++) { const char *s; if (of_property_read_string_index(dev->of_node, - "mbox-names", i, &s)) + "mbox-names", i, &s)) continue; if (strcmp(cl->chan_name, s)) continue; if (of_parse_phandle_with_args(dev->of_node, - "mbox", "#mbox-cells", i, &spec)) + "mbox", "#mbox-cells", i, &spec)) continue; - chan = NULL; list_for_each_entry(mbox, &mbox_cons, node) if (mbox->dev->of_node == spec.np) { - chan = mbox->of_xlate(mbox, &spec); + *chan = mbox->of_xlate(mbox, &spec); break; } of_node_put(spec.np); - if (!chan) + if (!(*chan)) continue; ret = -EBUSY; - if (!chan->cl && try_module_get(mbox->dev->driver->owner)) + if (!(*chan)->cl && acpi_disabled && + try_module_get(mbox->dev->driver->owner)) break; } if (i == count) { mutex_unlock(&con_mutex); - return ERR_PTR(ret); + return ret; } - spin_lock_irqsave(&chan->lock, flags); - chan->msg_free = 0; - chan->msg_count = 0; - chan->active_req = NULL; - chan->cl = cl; - if (!cl->tx_tout) /* wait for ever */ - cl->tx_tout = msecs_to_jiffies(3600000); + return init_channel(*chan, cl); +} + +/** + * mbox_request_channel - Request a mailbox channel. + * @cl: Identity of the client requesting the channel. + * + * The Client specifies its requirements and capabilities while asking for + * a mailbox channel by name. It can't be called from atomic context. + * The channel is exclusively allocated and can't be used by another + * client before the owner calls mbox_free_channel. + * After assignment, any packet received on this channel will be + * handed over to the client via the 'rx_callback'. + * + * Return: Pointer to the channel assigned to the client if successful. + * ERR_PTR for request failure. + */ +struct mbox_chan *mbox_request_channel(struct mbox_client *cl) +{ + struct mbox_chan *chan = NULL; + int ret; + + mutex_lock(&con_mutex); + + if (acpi_disabled) + ret = get_of_mbox_chan(cl, &chan); else - cl->tx_tout = msecs_to_jiffies(cl->tx_tout); - if (chan->txdone_method == TXDONE_BY_POLL - && cl->knows_txdone) - chan->txdone_method |= TXDONE_BY_ACK; - spin_unlock_irqrestore(&chan->lock, flags); + ret = get_acpi_mbox_chan(cl, &chan); - ret = chan->mbox->ops->startup(chan); - if (ret) { - pr_err("Unable to startup the chan\n"); - mbox_free_channel(chan); - chan = ERR_PTR(ret); - } + if (ret) + pr_err("No mailbox channels found\n"); mutex_unlock(&con_mutex); return chan; @@ -394,7 +461,9 @@ void mbox_free_channel(struct mbox_chan *chan) if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK)) chan->txdone_method = TXDONE_BY_POLL; - module_put(chan->mbox->dev->driver->owner); + if (chan->mbox->dev) + module_put(chan->mbox->dev->driver->owner); + spin_unlock_irqrestore(&chan->lock, flags); } EXPORT_SYMBOL_GPL(mbox_free_channel); @@ -422,7 +491,15 @@ int mbox_controller_register(struct mbox_controller *mbox) int i, txdone; /* Sanity check */ - if (!mbox || !mbox->dev || !mbox->ops || !mbox->num_chans) + if (!mbox || !mbox->ops || !mbox->num_chans) + return -EINVAL; + + /* + * For ACPI platforms, to get mbox->dev, we'd need to + * have a fake meaningless entry in the DSDT for the + * mailbox controller. + */ + if (acpi_disabled && !mbox->dev) return -EINVAL; if (mbox->txdone_irq) diff --git a/include/linux/mailbox_client.h b/include/linux/mailbox_client.h index bbac2d2..716fbae 100644 --- a/include/linux/mailbox_client.h +++ b/include/linux/mailbox_client.h @@ -29,7 +29,7 @@ struct mbox_chan; */ struct mbox_client { struct device *dev; - const char *chan_name; + char *chan_name; void (*rx_callback)(struct mbox_client *cl, void *mssg); void (*tx_done)(struct mbox_client *cl, void *mssg, enum mbox_result r); bool tx_block; diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h index cf81e80..06476ef 100644 --- a/include/linux/mailbox_controller.h +++ b/include/linux/mailbox_controller.h @@ -78,6 +78,7 @@ struct mbox_controller { unsigned period; /* Hook to add to the global controller list */ struct list_head node; + char *name; } __aligned(32); /*