linux-kernel - Re: [PATCH RFC v2 1/8] pinctrl: Add driver for Zynq

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: <CACRpkdaPiXiDegibiaqGdFxApowNEmbG0L4RDJVv-t+xhGBmjA@mail.gmail.com>
Date:	Tue, 28 Oct 2014 15:53:12 +0100
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Soren Brinkmann <soren.brinkmann@...inx.com>
Cc:	Michal Simek <michal.simek@...inx.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Steffen Trumtrar <s.trumtrar@...gutronix.de>
Subject: Re: [PATCH RFC v2 1/8] pinctrl: Add driver for Zynq

On Thu, Oct 16, 2014 at 7:11 PM, Soren Brinkmann
<soren.brinkmann@...inx.com> wrote:

> Signed-off-by: Soren Brinkmann <soren.brinkmann@...inx.com>
> ---
> changes since RFC:
>  - use syscon/regmap to access registers in SLCR space
>  - add pinctrl to zc702 DT
>  - rebase to 3.18: rename enable -> set_mux
>  - add kernel-doc
>  - support pinconf
>    - supported attributes
>      - pin-bias: pull up, tristate, disable
>      - slew-rate: 0 == slow, 1 == fast; generic pinconf does not display
>        argument

Great progress!!

(...)
> +++ b/arch/arm/mach-zynq/Kconfig
> @@ -9,6 +9,7 @@ config ARCH_ZYNQ
>         select HAVE_ARM_TWD if SMP
>         select ICST
>         select MFD_SYSCON
> +       select PINCTRL

Don't you also want to select PINCTRL_ZYNQ or is it
really optional?

>         select SOC_BUS
>         help
>           Support for Xilinx Zynq ARM Cortex A9 Platform

Please split these machine changes into a separate patch. It is hitting
a totally different subsystem.

(...)
> +++ b/drivers/pinctrl/pinctrl-zynq.c
(...)
> +static const struct pinctrl_ops zynq_pctrl_ops = {
> +       .get_groups_count = zynq_pctrl_get_groups_count,
> +       .get_group_name = zynq_pctrl_get_group_name,
> +       .get_group_pins = zynq_pctrl_get_group_pins,
> +       .dt_node_to_map = pinconf_generic_dt_node_to_map_group,
> +       .dt_free_map = pinctrl_utils_dt_free_map
> +};

Nice use of generic functions!

> +static int zynq_pinconf_cfg_get(struct pinctrl_dev *pctldev,
> +                               unsigned pin,
> +                               unsigned long *config)
> +{
> +       u32 reg;
> +       int ret;
> +       unsigned int arg = 0;
> +       unsigned int param = pinconf_to_config_param(*config);
> +       struct zynq_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> +
> +       if (pin > 53)
> +               return -ENOTSUPP;

53 looks a bit magical? #define or comment here to explain what's
going on?

Apart from these small things this looks like merge material.

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/