linux-kernel - Re: [PATCH v5] gpio: Add MOXA ART GPIO driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACmBeS1NoyHWhS5ydONf3x_z8SQu5-vh_XzMazuY9-uYXdg_wQ@mail.gmail.com>
Date:	Mon, 14 Oct 2013 13:15:28 +0200
From:	Jonas Jensen <jonas.jensen@...il.com>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	Grant Likely <grant.likely@...aro.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"arm@...nel.org" <arm@...nel.org>, Arnd Bergmann <arnd@...db.de>,
	Mark Rutland <mark.rutland@....com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v5] gpio: Add MOXA ART GPIO driver

Thank you for the replies.

On 11 October 2013 17:44, Linus Walleij <linus.walleij@...aro.org> wrote:
>>     The register responsible for doing enable/disable is located
>>     at <0x98100100 0x4>, the clock register is very close at
>>     <0x98100000 0x34>.
>
> If we don't know we have to guess.
>
> This layout makes me think that the IO-window at 0x98100000 is
> a power-clock-and-reset controller. It contains some register
> to latch the pins enable/disable them, or if this is even a clock
> gate? Are you sure about this? Is it now a gated clock, simply,
> so that this bit should be handled in the clock driver, i.e.
> this bit gets set by clk_enable() from the GPIO driver?

The IO-window at 0x98100000 contains registers that are read to
determine PLL and APB clock frequencies. Sorry I don't know more than
that.

This is part of a pending patch adding the clock driver:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/203494.html

Arnd made a similar comment suggesting syscon back when MMC mapped the
same register:
https://groups.google.com/d/msg/linux.kernel/eeS7vhMWMAc/zNYhzyKilh8J

I think I prefer to have this in the clock driver opposed to using syscon.

The one downside I can see, individual control of the pins would be
lost? Does it make sense to enable all pins once? this is acceptable
for at least UC-7112-LX, it doesn't need to disable/enable pins beyond
the initial clk_enable().

I say the above because I currently have nothing that requires
individual pin control. However, I removed one line from MMC that
turned out to be unnecessary. That line directly access the "PMU"
register disabling pins 10-17 with the following comment:

"
/* change I/O multiplexing to SD, so the GPIO 17-10 will be fail */
moxart_gpio_mp_clear(0xff << 10);
"
http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/mmc/host/moxasd.c#619


>>     I don't think gpio_poweroff driver is right for this hardware
>>     because the pin is not connected to anything that can do reset.
>>     The old 2.6.9 kernel driver uses timer poll with eventual call
>>     to userspace.
>>
>>     To test that it works, I added gpio_poweroff anyway, modified
>>     with gpio_export() the pin can then be seen switching between
>>     0 and 1 (on "cat /sys/class/gpio/gpio25/value").
>
> Hmmmm not quite following this...

I'll try to elaborate. What happens in gpio_poweroff driver does not
look like something that can reset the hardware. Reset on UC-7112-LX
is implemented using the same register as the watchdog, in platform
code hooked up to arm_pm_restart.

The old sources "solved" this by polling the reset pin with eventual
call_usermodehelper (/sbin/reboot):
http://code.google.com/p/linux-2-6-9-moxart/source/browse/drivers/char/moxa_watchdog.c#174

What was previously in a kernel driver, would now be solved in userspace?

gpio_export() allowed me to verify the pin number, pressing reset
toggles the value.

Adding the gpio-leds driver, that pin was automatically exported to
sysfs, that got me thinking:

How do I export the reset button to sysfs? Should gpio_export() be
added to platform code?


from drivers/power/reset/gpio-poweroff.c:

static void gpio_poweroff_do_poweroff(void)
{
        BUG_ON(!gpio_is_valid(gpio_num));

        /* drive it active, also inactive->active edge */
        gpio_direction_output(gpio_num, !gpio_active_low);
        mdelay(100);
        /* drive inactive, also active->inactive edge */
        gpio_set_value(gpio_num, gpio_active_low);
        mdelay(100);

        /* drive it active, also inactive->active edge */
        gpio_set_value(gpio_num, !gpio_active_low);

        /* give it some time */
        mdelay(3000);

        WARN_ON(1);
}


Regards,
Jonas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/