linux-kernel - Re: [PATCH 1/3 v3] pinctrl: ADI PIN control driver for the GPIO controller on bf54x and bf60x.

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: <CACRpkdZpHk31zPPn9OeXp3=EH=H6p3BKmikN=7TwLhsR+c1aiA@mail.gmail.com>
Date:	Thu, 29 Aug 2013 10:02:38 +0200
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Sonic Zhang <sonic.adi@...il.com>
Cc:	Grant Likely <grant.likely@...aro.org>,
	Steven Miao <realmz6@...il.com>,
	Stephen Warren <swarren@...dotorg.org>,
	LKML <linux-kernel@...r.kernel.org>,
	adi-buildroot-devel@...ts.sourceforge.net,
	Sonic Zhang <sonic.zhang@...log.com>
Subject: Re: [PATCH 1/3 v3] pinctrl: ADI PIN control driver for the GPIO
 controller on bf54x and bf60x.

On Wed, Aug 21, 2013 at 8:30 AM, Sonic Zhang <sonic.adi@...il.com> wrote:

> From: Sonic Zhang <sonic.zhang@...log.com>
>
> The new ADI GPIO2 controller was introduced since the BF548 and BF60x
> processors. It differs a lot from the old one on BF5xx processors. So,
> create a pinctrl driver under the pinctrl framework.
(...)

The v3 is a huge improvement! Keep going.

I will not repeat any of Stephens review comments, I just
saw this:

> +static const unsigned uart1_pins[] = {
> +       GPIO_PH0, GPIO_PH1,
> +#ifdef CONFIG_BFIN_UART1_CTSRTS
> +       GPIO_PE9, GPIO_PE10,
> +#endif
(...)
> +static const unsigned atapi_pins[] = {
> +       GPIO_PH2, GPIO_PJ3, GPIO_PJ4, GPIO_PJ5, GPIO_PJ6,
> +       GPIO_PJ7, GPIO_PJ8, GPIO_PJ9, GPIO_PJ10,
> +       GPIO_PG5, GPIO_PG6, GPIO_PG7,
> +#ifdef CONFIG_BF548_ATAPI_ALTERNATIVE_PORT
> +       GPIO_PF0, GPIO_PF1, GPIO_PF2, GPIO_PF3, GPIO_PF4, GPIO_PF5, GPIO_PF6,
> +       GPIO_PF7, GPIO_PF8, GPIO_PF9, GPIO_PF10, GPIO_PF11, GPIO_PF12,
> +       GPIO_PF13, GPIO_PF14, GPIO_PF15, GPIO_PG2, GPIO_PG3, GPIO_PG4,
> +#endif

(...)
> +static const struct adi_pin_group adi_pin_groups[] = {
(...)
> +       ADI_PIN_GROUP("uart1grp", uart1_pins),
(...)
> +       ADI_PIN_GROUP("atapigrp", atapi_pins),
> +};

This is not how we do it. Do not use Kconfig to spefify how the SoC
is utilized. This shall be done at runtime.

What you want to do for UART0 is specify one group for the TX/RX
pair and another group for the CTS/RTS pair like this:

static const unsigned uart1_rxtx_pins[] = {
       GPIO_PH0, GPIO_PH1,
};

static const unsigned uart1_rtscts_pins[] = {
       GPIO_PE9, GPIO_PE10,
};

static const struct adi_pin_group adi_pin_groups[] = {
(...)
        ADI_PIN_GROUP("uart1txrxgrp", uart1_rxtx_pins),
        ADI_PIN_GROUP("uart1rtsctsgrp", uart1_ctsrts_pins),
       (...)
};

(And vice versa for the atapi pins, create two groups.)

If you want to use all four pins on UART1 in a system, you
specify this in you map (in platform data or DTS), so that
e.g. the state "default" will be activate *both* uart1txrxgrp
and uart1rtscts groups.

This is typically done with two entries in the map.

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