Excluding pins from Linux control

Excluding pins from Linux control

[Stefan Wahren]

Hi,

on BCM2835 (Raspberry Pi) the firmware on the VideoCore and Linux on the ARM core share a common pin "space". Depending on the board revision the VideoCore needs to controls some pins, which shouldn't be claimed by Linux on the ARM.

What is the right way to exclude control on Linux for those pins in order to avoid possible harmful operations?

I tought of this [1] but only for pins.

[1] - http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Regards
Stefan
--
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

Re: Excluding pins from Linux control

[Linus Walleij]

On Mon, Feb 27, 2017 at 7:02 PM, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:

> on BCM2835 (Raspberry Pi) the firmware on the VideoCore and Linux on the ARM core share
> a common pin "space". Depending on the board revision the VideoCore needs to controls
> some pins, which shouldn't be claimed by Linux on the ARM.
>
> What is the right way to exclude control on Linux for those pins in order to avoid possible
> harmful operations?

Do you want to do this statically (at boot) or dynamically (at runtime)?

If you want to do it statically, at boot time:
Just add some NOOP function (i.e. a function that result in
zero register writes or anything) named "videocore-reserved" or
something to the driver in  drivers/pinctrl/bcm/pinctrl-bcm2835.c,
make it applicable to the affected pins, making it possible
to create a state that will combine the function "videocore-reserved"
with these pins, resulting in them being exclusively used for
that.

Then in the videocore device tree node (I assume there is
something like such) create a pinctrl state that just take all these
pins with this dummy function.

(If the videocore does not even have a device tree node, hog all
those pins to that function using hogs as described in
Documentaion/pinctrl.txt, essentially just take the pins for the
pin controller itself.)

If you want to do it at runtime, you need some communication link
to the videocore (I guess) and then it is essentially the same
approach but done using proper states reflecting whether the
videocore is actively using them or not, but for the simple case
even this would be easiest done using the static approach above.
You may need a dynamic solution if you want to remux the
pins to ground when going to sleep for example, then something
must move the pins from "default" to "sleep" state and back,
and that needs to be the videocore driver in the kernel I guess.

Yours,
Linus Walleij
--
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

Re: Excluding pins from Linux control

[Stefan Wahren]

Hi Linus,

> Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> hat am 14. März 2017 um 15:39 geschrieben:
> 
> 
> On Mon, Feb 27, 2017 at 7:02 PM, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:
> 
> > on BCM2835 (Raspberry Pi) the firmware on the VideoCore and Linux on the ARM core share
> > a common pin "space". Depending on the board revision the VideoCore needs to controls
> > some pins, which shouldn't be claimed by Linux on the ARM.
> >
> > What is the right way to exclude control on Linux for those pins in order to avoid possible
> > harmful operations?
> 
> Do you want to do this statically (at boot) or dynamically (at runtime)?

statically

> 
> If you want to do it statically, at boot time:
> Just add some NOOP function (i.e. a function that result in
> zero register writes or anything) named "videocore-reserved" or
> something to the driver in  drivers/pinctrl/bcm/pinctrl-bcm2835.c,
> make it applicable to the affected pins, making it possible
> to create a state that will combine the function "videocore-reserved"
> with these pins, resulting in them being exclusively used for
> that.

If you speak of a NOOP function, do you mean a "dummy" pin function which is configured by the device tree? Like extending the "brcm,function" property from the existing binding?

> 
> Then in the videocore device tree node (I assume there is
> something like such) create a pinctrl state that just take all these
> pins with this dummy function.

Is there a comparable example in the existing drivers?

Thanks
Stefan
--
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

Re: Excluding pins from Linux control

[Linus Walleij]

On Tue, Mar 14, 2017 at 9:09 PM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> Linus Walleij <linus.walleij@linaro.org> hat am 14. März 2017 um 15:39 geschrieben:

>> If you want to do it statically, at boot time:
>> Just add some NOOP function (i.e. a function that result in
>> zero register writes or anything) named "videocore-reserved" or
>> something to the driver in  drivers/pinctrl/bcm/pinctrl-bcm2835.c,
>> make it applicable to the affected pins, making it possible
>> to create a state that will combine the function "videocore-reserved"
>> with these pins, resulting in them being exclusively used for
>> that.
>
> If you speak of a NOOP function, do you mean a "dummy" pin function
> which is configured by the device tree? Like extending the "brcm,function"
> property from the existing binding?

No, a function for videocore defined by the driver in
drivers/pinctrl/bcm/pinctrl-bcm2835.c
and then used by the device tree.

Looking at the BCM2835 syntax (hope I get it right), in the end you should
be able to do this:

    videocore: videocore {
        brcm,pins = <12, 13, 14, 15, 16, 17, 18, 19, 20>;
        brcm,function = <BCM2835_FSEL_VIDEOCORE>;
    };

So BCM2835_FSEL_VIDEOCORE need to be defined in the
DT include and made available as a muxing option inside the driver, with
the effect that nothing really happens when you select it.

The point is that the pinctrl core will then regard the pin as taken
(this can be verified in debugfs) and then no other function can go
in and use the pin by mistake.

Something like this:

>From d188c03c1fba7d1223c1dafcb8b977e383298ef4 Mon Sep 17 00:00:00 2001
From: Linus Walleij <linus.walleij@linaro.org>
Date: Wed, 15 Mar 2017 09:45:37 +0100
Subject: [PATCH] Stab at videocore function

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 9 +++++++++
 include/dt-bindings/pinctrl/bcm2835.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 85d009112864..213ca5117bd3 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -41,6 +41,7 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
+#include <dt-bindings/pinctrl/bcm2835.h>

 #define MODULE_NAME "pinctrl-bcm2835"
 #define BCM2835_NUM_GPIOS 54
@@ -226,6 +227,9 @@ enum bcm2835_fsel {
     BCM2835_FSEL_MASK = 0x7,
 };

+/* This function can no be configured in the hardware register */
+#define BCM2835_FSEL_VIDEOCORE 8
+
 static const char * const bcm2835_functions[BCM2835_FSEL_COUNT] = {
     [BCM2835_FSEL_GPIO_IN] = "gpio_in",
     [BCM2835_FSEL_GPIO_OUT] = "gpio_out",
@@ -235,6 +239,7 @@ static const char * const
bcm2835_functions[BCM2835_FSEL_COUNT] = {
     [BCM2835_FSEL_ALT3] = "alt3",
     [BCM2835_FSEL_ALT4] = "alt4",
     [BCM2835_FSEL_ALT5] = "alt5",
+    [BCM2835_FSEL_VIDEOCORE] = "videocore",
 };

 static const char * const irq_type_names[] = {
@@ -879,6 +884,10 @@ static int bcm2835_pmx_set(struct pinctrl_dev *pctldev,
 {
     struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);

+    /* This pin is set as used by the videocore - do nothing */
+    if (func_selector == BCM2835_FSEL_VIDEOCORE)
+        return 0;
+
     bcm2835_pinctrl_fsel_set(pc, group_selector, func_selector);

     return 0;
diff --git a/include/dt-bindings/pinctrl/bcm2835.h
b/include/dt-bindings/pinctrl/bcm2835.h
index e4e4fdf5d38f..2de2a2eb2fbc 100644
--- a/include/dt-bindings/pinctrl/bcm2835.h
+++ b/include/dt-bindings/pinctrl/bcm2835.h
@@ -23,6 +23,7 @@
 #define BCM2835_FSEL_ALT1    5
 #define BCM2835_FSEL_ALT2    6
 #define BCM2835_FSEL_ALT3    7
+#define BCM2835_FSEL_VIDEOCORE    8

 /* brcm,pull property */
 #define BCM2835_PUD_OFF        0
-- 
2.9.3

>> Then in the videocore device tree node (I assume there is
>> something like such) create a pinctrl state that just take all these
>> pins with this dummy function.
>
> Is there a comparable example in the existing drivers?

A lot of drivers use functions much more granular than the GPIO,
ALT0, ALT1 ... etc used by bcm2835.

Yours,
Linus Walleij

Re: Excluding pins from Linux control

[Stefan Wahren]

> Linus Walleij <linus.walleij@linaro.org> hat am 15. März 2017 um 09:46 geschrieben:
> 
> 
> On Tue, Mar 14, 2017 at 9:09 PM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> >> Linus Walleij <linus.walleij@linaro.org> hat am 14. März 2017 um 15:39 geschrieben:
> 
> >> If you want to do it statically, at boot time:
> >> Just add some NOOP function (i.e. a function that result in
> >> zero register writes or anything) named "videocore-reserved" or
> >> something to the driver in  drivers/pinctrl/bcm/pinctrl-bcm2835.c,
> >> make it applicable to the affected pins, making it possible
> >> to create a state that will combine the function "videocore-reserved"
> >> with these pins, resulting in them being exclusively used for
> >> that.
> >
> > If you speak of a NOOP function, do you mean a "dummy" pin function
> > which is configured by the device tree? Like extending the "brcm,function"
> > property from the existing binding?
> 
> No, a function for videocore defined by the driver in
> drivers/pinctrl/bcm/pinctrl-bcm2835.c
> and then used by the device tree.
> 
> Looking at the BCM2835 syntax (hope I get it right), in the end you should
> be able to do this:
> 
>     videocore: videocore {
>         brcm,pins = <12, 13, 14, 15, 16, 17, 18, 19, 20>;
>         brcm,function = <BCM2835_FSEL_VIDEOCORE>;
>     };
> 
> So BCM2835_FSEL_VIDEOCORE need to be defined in the
> DT include and made available as a muxing option inside the driver, with
> the effect that nothing really happens when you select it.
> 
> The point is that the pinctrl core will then regard the pin as taken
> (this can be verified in debugfs) and then no other function can go
> in and use the pin by mistake.
> 
> Something like this:
> 
> From d188c03c1fba7d1223c1dafcb8b977e383298ef4 Mon Sep 17 00:00:00 2001
> From: Linus Walleij <linus.walleij@linaro.org>
> Date: Wed, 15 Mar 2017 09:45:37 +0100
> Subject: [PATCH] Stab at videocore function
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Again, thanks a lot

Stefan

> 
> Yours,
> Linus Walleij