[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <04ed13f1-671f-7416-61d0-0bf452ae862e@canonical.com>
Date: Tue, 15 Mar 2022 11:49:59 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>
To: Chris Packham <chris.packham@...iedtelesis.co.nz>,
huziji@...vell.com, ulf.hansson@...aro.org, robh+dt@...nel.org,
davem@...emloft.net, kuba@...nel.org, linus.walleij@...aro.org,
catalin.marinas@....com, will@...nel.org, andrew@...n.ch,
gregory.clement@...tlin.com, sebastian.hesselbarth@...il.com,
adrian.hunter@...el.com, thomas.petazzoni@...tlin.com,
kostap@...vell.com, robert.marko@...tura.hr
Cc: linux-mmc@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
linux-gpio@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 4/8] pinctrl: mvebu: pinctrl driver for 98DX2530 SoC
On 14/03/2022 22:31, Chris Packham wrote:
> This pinctrl driver supports the 98DX25xx and 98DX35xx family of chips
> from Marvell. It is based on the Marvell SDK with additions for various
> (non-gpio) pin configurations based on the datasheet.
>
> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
> ---
>
> Notes:
> Changes in v2:
> - Make pinctrl a child of a syscon node like the armada-7k-pinctrl
>
> drivers/pinctrl/mvebu/Kconfig | 4 +
> drivers/pinctrl/mvebu/Makefile | 1 +
> drivers/pinctrl/mvebu/pinctrl-ac5.c | 226 ++++++++++++++++++++++++++++
> 3 files changed, 231 insertions(+)
> create mode 100644 drivers/pinctrl/mvebu/pinctrl-ac5.c
>
> diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig
> index 0d12894d3ee1..aa5883f09d7b 100644
> --- a/drivers/pinctrl/mvebu/Kconfig
> +++ b/drivers/pinctrl/mvebu/Kconfig
> @@ -45,6 +45,10 @@ config PINCTRL_ORION
> bool
> select PINCTRL_MVEBU
>
> +config PINCTRL_AC5
> + bool
> + select PINCTRL_MVEBU
> +
> config PINCTRL_ARMADA_37XX
> bool
> select GENERIC_PINCONF
> diff --git a/drivers/pinctrl/mvebu/Makefile b/drivers/pinctrl/mvebu/Makefile
> index cd082dca4482..23458ab17c53 100644
> --- a/drivers/pinctrl/mvebu/Makefile
> +++ b/drivers/pinctrl/mvebu/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_PINCTRL_ARMADA_CP110) += pinctrl-armada-cp110.o
> obj-$(CONFIG_PINCTRL_ARMADA_XP) += pinctrl-armada-xp.o
> obj-$(CONFIG_PINCTRL_ARMADA_37XX) += pinctrl-armada-37xx.o
> obj-$(CONFIG_PINCTRL_ORION) += pinctrl-orion.o
> +obj-$(CONFIG_PINCTRL_AC5) += pinctrl-ac5.o
> diff --git a/drivers/pinctrl/mvebu/pinctrl-ac5.c b/drivers/pinctrl/mvebu/pinctrl-ac5.c
> new file mode 100644
> index 000000000000..8bc0bbff7c1b
> --- /dev/null
> +++ b/drivers/pinctrl/mvebu/pinctrl-ac5.c
> @@ -0,0 +1,226 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Marvell ac5 pinctrl driver based on mvebu pinctrl core
> + *
> + * Copyright (C) 2021 Marvell
> + *
> + * Noam Liron <lnoam@...vell.com>
> + */
> +
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/pinctrl/pinctrl.h>
> +
> +#include "pinctrl-mvebu.h"
> +
> +static struct mvebu_mpp_mode ac5_mpp_modes[] = {
> + MPP_MODE(0,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "sdio", "d0"),
> + MPP_FUNCTION(2, "nand", "io4")),
> + MPP_MODE(1,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "sdio", "d1"),
> + MPP_FUNCTION(2, "nand", "io3")),
> + MPP_MODE(2,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "sdio", "d2"),
> + MPP_FUNCTION(2, "nand", "io2")),
> + MPP_MODE(3,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "sdio", "d3"),
> + MPP_FUNCTION(2, "nand", "io7")),
> + MPP_MODE(4,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "sdio", "d4"),
> + MPP_FUNCTION(2, "nand", "io6"),
> + MPP_FUNCTION(3, "uart3", "txd"),
> + MPP_FUNCTION(4, "uart2", "txd")),
> + MPP_MODE(5,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "sdio", "d5"),
> + MPP_FUNCTION(2, "nand", "io5"),
> + MPP_FUNCTION(3, "uart3", "rxd"),
> + MPP_FUNCTION(4, "uart2", "rxd")),
> + MPP_MODE(6,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "sdio", "d6"),
> + MPP_FUNCTION(2, "nand", "io0"),
> + MPP_FUNCTION(3, "i2c1", "sck")),
> + MPP_MODE(7,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "sdio", "d7"),
> + MPP_FUNCTION(2, "nand", "io1"),
> + MPP_FUNCTION(3, "i2c1", "sda")),
> + MPP_MODE(8,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "sdio", "clk"),
> + MPP_FUNCTION(2, "nand", "wen")),
> + MPP_MODE(9,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "sdio", "cmd"),
> + MPP_FUNCTION(2, "nand", "ale")),
> + MPP_MODE(10,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "sdio", "ds"),
> + MPP_FUNCTION(2, "nand", "cle")),
> + MPP_MODE(11,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "sdio", "rst"),
> + MPP_FUNCTION(2, "nand", "cen")),
> + MPP_MODE(12,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "spi0", "clk")),
> + MPP_MODE(13,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "spi0", "csn")),
> + MPP_MODE(14,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "spi0", "mosi")),
> + MPP_MODE(15,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "spi0", "miso")),
> + MPP_MODE(16,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "spi0", "wpn"),
> + MPP_FUNCTION(2, "nand", "ren"),
> + MPP_FUNCTION(3, "uart1", "txd")),
> + MPP_MODE(17,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "spi0", "hold"),
> + MPP_FUNCTION(2, "nand", "rb"),
> + MPP_FUNCTION(3, "uart1", "rxd")),
> + MPP_MODE(18,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(2, "uart2", "rxd")),
> + MPP_MODE(19,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(2, "uart2", "txd")),
> + MPP_MODE(20,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(2, "i2c1", "sck"),
> + MPP_FUNCTION(3, "spi1", "clk"),
> + MPP_FUNCTION(4, "uart3", "txd")),
> + MPP_MODE(21,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(2, "i2c1", "sda"),
> + MPP_FUNCTION(3, "spi1", "csn"),
> + MPP_FUNCTION(4, "uart3", "rxd")),
> + MPP_MODE(22,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(3, "spi1", "mosi")),
> + MPP_MODE(23,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(3, "spi1", "miso")),
> + MPP_MODE(24,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(2, "uart2", "txd")),
> + MPP_MODE(25,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(2, "uart2", "rxd")),
> + MPP_MODE(26,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "i2c0", "sck"),
> + MPP_FUNCTION(3, "uart3", "txd")),
> + MPP_MODE(27,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "i2c0", "sda"),
> + MPP_FUNCTION(3, "uart3", "rxd")),
> + MPP_MODE(28,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(3, "uart3", "txd")),
> + MPP_MODE(29,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(3, "uart3", "rxd")),
> + MPP_MODE(30,
> + MPP_FUNCTION(0, "gpio", NULL)),
> + MPP_MODE(31,
> + MPP_FUNCTION(0, "gpio", NULL)),
> + MPP_MODE(32,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "uart0", "txd")),
> + MPP_MODE(33,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(1, "uart0", "rxd")),
> + MPP_MODE(34,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(2, "uart3", "rxd")),
> + MPP_MODE(35,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(2, "uart3", "txd")),
> + MPP_MODE(36,
> + MPP_FUNCTION(0, "gpio", NULL)),
> + MPP_MODE(37,
> + MPP_FUNCTION(0, "gpio", NULL)),
> + MPP_MODE(38,
> + MPP_FUNCTION(0, "gpio", NULL)),
> + MPP_MODE(39,
> + MPP_FUNCTION(0, "gpio", NULL)),
> + MPP_MODE(40,
> + MPP_FUNCTION(0, "gpio", NULL)),
> + MPP_MODE(41,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(4, "uart2", "txd"),
> + MPP_FUNCTION(5, "i2c1", "sck")),
> + MPP_MODE(42,
> + MPP_FUNCTION(0, "gpio", NULL),
> + MPP_FUNCTION(4, "uart2", "rxd"),
> + MPP_FUNCTION(5, "i2c1", "sda")),
> + MPP_MODE(43,
> + MPP_FUNCTION(0, "gpio", NULL)),
> + MPP_MODE(44,
> + MPP_FUNCTION(0, "gpio", NULL)),
> + MPP_MODE(45,
> + MPP_FUNCTION(0, "gpio", NULL)),
> +};
> +
> +static struct mvebu_pinctrl_soc_info ac5_pinctrl_info;
You should not have static/file-scope variables, especially that it is
not actually used in that way.
> +
> +static const struct of_device_id ac5_pinctrl_of_match[] = {
> + {
> + .compatible = "marvell,ac5-pinctrl",
> + },
> + { },
> +};
> +
> +static const struct mvebu_mpp_ctrl ac5_mpp_controls[] = {
> + MPP_FUNC_CTRL(0, 45, NULL, mvebu_regmap_mpp_ctrl), };
> +
> +static struct pinctrl_gpio_range ac5_mpp_gpio_ranges[] = {
> + MPP_GPIO_RANGE(0, 0, 0, 46), };
> +
> +static int ac5_pinctrl_probe(struct platform_device *pdev)
> +{
> + struct mvebu_pinctrl_soc_info *soc = &ac5_pinctrl_info;
> + const struct of_device_id *match =
> + of_match_device(ac5_pinctrl_of_match, &pdev->dev);
Why is this needed? Unusual, dead-code.
> +
> + if (!match || !pdev->dev.parent)
> + return -ENODEV;
> +
> + soc->variant = 0; /* no variants for ac5 */
> + soc->controls = ac5_mpp_controls;
> + soc->ncontrols = ARRAY_SIZE(ac5_mpp_controls);
> + soc->gpioranges = ac5_mpp_gpio_ranges;
> + soc->ngpioranges = ARRAY_SIZE(ac5_mpp_gpio_ranges);
> + soc->modes = ac5_mpp_modes;
> + soc->nmodes = ac5_mpp_controls[0].npins;
> +
> + pdev->dev.platform_data = soc;
> +
> + return mvebu_pinctrl_simple_regmap_probe(pdev, pdev->dev.parent, 0);
> +}
> +
> +static struct platform_driver ac5_pinctrl_driver = {
> + .driver = {
> + .name = "ac5-pinctrl",
> + .of_match_table = of_match_ptr(ac5_pinctrl_of_match),
of_match_ptr() does not look correct for OF-only platform. This should
complain in W=1 compile tests on !OF config.
Best regards,
Krzysztof
Powered by blists - more mailing lists