Re: [PATCH v3 2/3] remoteproc/keystone: Add a remoteproc driver for Keystone 2 DSPs

Hi,

On Mon, Jun 26, 2017 at 8:55 AM Suman Anna [off-list ref] wrote:

Hi Bjorn,

On 06/25/2017 03:15 PM, Bjorn Andersson wrote:

quoted

On Tue 13 Jun 16:45 PDT 2017, Suman Anna wrote:

quoted

+static int keystone_rproc_start(struct rproc *rproc)
+{
+    struct keystone_rproc *ksproc = rproc->priv;
+    int ret;
+
+    INIT_WORK(&ksproc->workqueue, handle_event);
+
+    ret = request_irq(ksproc->irq_ring, keystone_rproc_vring_interrupt, 0,
+                      dev_name(ksproc->dev), ksproc);
+    if (ret) {
+            dev_err(ksproc->dev, "failed to enable vring interrupt, ret = %d\n",
+                    ret);
+            goto out;
+    }
+
+    ret = request_irq(ksproc->irq_fault, keystone_rproc_exception_interrupt,
+                      0, dev_name(ksproc->dev), ksproc);
+    if (ret) {
+            dev_err(ksproc->dev, "failed to enable exception interrupt, ret = %d\n",
+                    ret);
+            goto free_vring_irq;
+    }

I do prefer that your request any resources during probe() and
potentially enable/disable them here. If below concern about using a
GPIO driver is cleared already I'll take it as is though.

[..]

quoted

+static void keystone_rproc_kick(struct rproc *rproc, int vqid)
+{
+    struct keystone_rproc *ksproc = rproc->priv;
+
+    if (WARN_ON(ksproc->kick_gpio < 0))
+            return;
+
+    gpio_set_value(ksproc->kick_gpio, 1);
+}
+

This doesn't sound like a gpio-controller and the GPIO maintainer did
reject an attempt by me to use the GPIO framework to abstract a similar
thing. Do you already have this driver upstream or have you clarified
with the maintainer that the GPIO framework is an acceptable abstraction
for this?

Yeah, this has been upstream since quite some time. See commit
2134cb997f2f ("gpio: syscon: reuse for keystone 2 socs").

Sorry for the thread necromancy, but was it intentional that the
driver only parses DT to find this "GPIO" but never requests/reserves
it? I would like to drop of_get_named_gpio_flags() in favor of a more
standard dev_get_gpiod().

Thanks.

-- 
Dmitry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel