[1/2] gpio: Add driver for AXM55xx SSP chip selects
diff mbox

Message ID 1409576021-24828-2-git-send-email-anders.berg@avagotech.com
State Awaiting Upstream
Headers show

Commit Message

Anders Berg Sept. 1, 2014, 12:53 p.m. UTC
The AXM55xx device has a ARM PL022 SSP controller with an extension that
enables the control of up to five chip select signals. This driver implements
the logic to control these signals and exports them as GPIOs.

Signed-off-by: Anders Berg <anders.berg@avagotech.com>
---
 drivers/gpio/Kconfig          |   8 +++
 drivers/gpio/Makefile         |   1 +
 drivers/gpio/gpio-axxia-ssp.c | 128 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 137 insertions(+)
 create mode 100644 drivers/gpio/gpio-axxia-ssp.c

Comments

Linus Walleij Sept. 4, 2014, 5:04 p.m. UTC | #1
I need an ACK from Mark Brown on this before I apply.
The driver seems to do what it should, but I need Mark to
be OK with exporting the CS signals from an SSP/SPI block
to be exposed to Linux as GPIOs.

(Top-posting and quoting patch verbatim so Mark gets it.)

Yours,
Linus Walleij

On Mon, Sep 1, 2014 at 2:53 PM, Anders Berg <anders.berg@avagotech.com> wrote:
> The AXM55xx device has a ARM PL022 SSP controller with an extension that
> enables the control of up to five chip select signals. This driver implements
> the logic to control these signals and exports them as GPIOs.
>
> Signed-off-by: Anders Berg <anders.berg@avagotech.com>
> ---
>  drivers/gpio/Kconfig          |   8 +++
>  drivers/gpio/Makefile         |   1 +
>  drivers/gpio/gpio-axxia-ssp.c | 128 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 137 insertions(+)
>  create mode 100644 drivers/gpio/gpio-axxia-ssp.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 9de1515..c2f2686 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -112,6 +112,14 @@ config GPIO_MAX730X
>
>  comment "Memory mapped GPIO drivers:"
>
> +config GPIO_AXXIA_SSP
> +       tristate "AXM55xx SSP chip select pins as GPIO"
> +       depends on ARCH_AXXIA
> +       default y if SPI_MASTER
> +       select GPIO_GENERIC
> +       help
> +         Say yes here to get GPIO interface for AXM55xx SSP chip select signals.
> +
>  config GPIO_CLPS711X
>         tristate "CLPS711X GPIO support"
>         depends on ARCH_CLPS711X || COMPILE_TEST
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 5d024e3..01e1869 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_GPIO_ADP5520)    += gpio-adp5520.o
>  obj-$(CONFIG_GPIO_ADP5588)     += gpio-adp5588.o
>  obj-$(CONFIG_GPIO_AMD8111)     += gpio-amd8111.o
>  obj-$(CONFIG_GPIO_ARIZONA)     += gpio-arizona.o
> +obj-$(CONFIG_GPIO_AXXIA_SSP)   += gpio-axxia-ssp.o
>  obj-$(CONFIG_GPIO_BCM_KONA)    += gpio-bcm-kona.o
>  obj-$(CONFIG_GPIO_BT8XX)       += gpio-bt8xx.o
>  obj-$(CONFIG_GPIO_CLPS711X)    += gpio-clps711x.o
> diff --git a/drivers/gpio/gpio-axxia-ssp.c b/drivers/gpio/gpio-axxia-ssp.c
> new file mode 100644
> index 0000000..5884206
> --- /dev/null
> +++ b/drivers/gpio/gpio-axxia-ssp.c
> @@ -0,0 +1,128 @@
> +/*
> + * GPIO interface for SSP chip select outputs.
> + *
> + * Copyright (C) 2013 LSI Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/bitops.h>
> +#include <linux/init.h>
> +#include <linux/gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +
> +struct gpio_dev {
> +       void __iomem    *reg;
> +       struct gpio_chip gpio_chip;
> +};
> +
> +static int ssp_gpio_direction_out(struct gpio_chip *chip,
> +                                 unsigned offset, int value)
> +{
> +       /* Always outputs */
> +       return 0;
> +}
> +
> +static int ssp_gpio_direction_in(struct gpio_chip *chip, unsigned offset)
> +{
> +       return -ENXIO;
> +}
> +
> +static int ssp_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> +       struct gpio_dev *priv = dev_get_drvdata(chip->dev);
> +       u32 tmp;
> +
> +       tmp = readl(priv->reg);
> +       return !!(tmp & BIT(offset));
> +}
> +
> +static void ssp_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> +{
> +       struct gpio_dev *priv = dev_get_drvdata(chip->dev);
> +       u32 tmp;
> +
> +       tmp = readl(priv->reg);
> +       if (value)
> +               tmp |= BIT(offset);
> +       else
> +               tmp &= ~BIT(offset);
> +       writel(tmp, priv->reg);
> +}
> +
> +static int ssp_gpio_probe(struct platform_device *pdev)
> +{
> +       struct gpio_dev *priv;
> +       struct resource *io;
> +       struct gpio_chip *chip;
> +       int ret;
> +
> +       priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       priv->reg = devm_ioremap(&pdev->dev, io->start, resource_size(io));
> +       if (!priv->reg)
> +               return -ENXIO;
> +
> +       platform_set_drvdata(pdev, priv);
> +
> +       chip = &priv->gpio_chip;
> +       chip->dev = &pdev->dev;
> +#ifdef CONFIG_OF_GPIO
> +       chip->of_node = pdev->dev.of_node;
> +#endif
> +       chip->direction_input = ssp_gpio_direction_in;
> +       chip->direction_output = ssp_gpio_direction_out;
> +       chip->get = ssp_gpio_get;
> +       chip->set = ssp_gpio_set;
> +       chip->label = "ssp-gpio";
> +       chip->owner = THIS_MODULE;
> +       chip->base = -1;
> +       chip->ngpio = 5;
> +
> +       /* Set all 5 chip-selects high (de-asserted) by default */
> +       writel(0x1f, priv->reg);
> +
> +       ret = gpiochip_add(chip);
> +       if (ret < 0)
> +               dev_err(&pdev->dev, "failed to register chip, %d\n", ret);
> +
> +       return ret;
> +}
> +
> +static int ssp_gpio_remove(struct platform_device *pdev)
> +{
> +       struct gpio_dev *priv = dev_get_drvdata(&pdev->dev);
> +
> +       return gpiochip_remove(&priv->gpio_chip);
> +}
> +
> +static const struct of_device_id ssp_gpio_id_table[] = {
> +       { .compatible = "lsi,ssp-gpio" },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(platform, ssp_gpio_id_table);
> +
> +static struct platform_driver ssp_gpio_driver = {
> +       .driver = {
> +               .name = "ssp-gpio",
> +               .owner = THIS_MODULE,
> +               .of_match_table = ssp_gpio_id_table
> +       },
> +       .probe = ssp_gpio_probe,
> +       .remove = ssp_gpio_remove,
> +};
> +
> +module_platform_driver(ssp_gpio_driver);
> +
> +MODULE_AUTHOR("LSI Corporation");
> +MODULE_DESCRIPTION("GPIO interface for SSP chip selects");
> +MODULE_LICENSE("GPL");
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Sept. 4, 2014, 5:06 p.m. UTC | #2
On Thu, Sep 04, 2014 at 07:04:44PM +0200, Linus Walleij wrote:
> I need an ACK from Mark Brown on this before I apply.
> The driver seems to do what it should, but I need Mark to
> be OK with exporting the CS signals from an SSP/SPI block
> to be exposed to Linux as GPIOs.
> 
> (Top-posting and quoting patch verbatim so Mark gets it.)

I don't see any sort of reasoning for this?
Linus Walleij Sept. 4, 2014, 5:32 p.m. UTC | #3
On Thu, Sep 4, 2014 at 7:06 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Sep 04, 2014 at 07:04:44PM +0200, Linus Walleij wrote:
>> I need an ACK from Mark Brown on this before I apply.
>> The driver seems to do what it should, but I need Mark to
>> be OK with exporting the CS signals from an SSP/SPI block
>> to be exposed to Linux as GPIOs.
>>
>> (Top-posting and quoting patch verbatim so Mark gets it.)
>
> I don't see any sort of reasoning for this?

Maybe there was some earlier, but this is an excellent opportunity
to repeat it... So Anders: why do you want to do this?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anders Berg Sept. 5, 2014, 9:05 a.m. UTC | #4
> On Thu, Sep 4, 2014 at 7:06 PM, Mark Brown <broonie@kernel.org> wrote:
>> On Thu, Sep 04, 2014 at 07:04:44PM +0200, Linus Walleij wrote:
>>> I need an ACK from Mark Brown on this before I apply.
>>> The driver seems to do what it should, but I need Mark to
>>> be OK with exporting the CS signals from an SSP/SPI block
>>> to be exposed to Linux as GPIOs.
>>>
>>> (Top-posting and quoting patch verbatim so Mark gets it.)
>>
>> I don't see any sort of reasoning for this?
>
> Maybe there was some earlier, but this is an excellent opportunity
> to repeat it... So Anders: why do you want to do this?
>

Well, the problem with adding this to the spi-pl022 driver (as a
vendor specific extension) is that this IP block unfortunately isn't
distinguishable from the standard ARM PL022 implementation (same
values in the PrimeCell identification registers). That's why I went
down the GPIO path...

/Anders
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux Sept. 5, 2014, 9:47 a.m. UTC | #5
On Mon, Sep 01, 2014 at 02:53:40PM +0200, Anders Berg wrote:
> The AXM55xx device has a ARM PL022 SSP controller with an extension that
> enables the control of up to five chip select signals. This driver implements
> the logic to control these signals and exports them as GPIOs.
> 
> Signed-off-by: Anders Berg <anders.berg@avagotech.com>

So, what happens if the GPIO driver binds first, followed by the SSP
device?  Please test that scenario out before this patch gets merged.
Anders Berg Sept. 5, 2014, 1:40 p.m. UTC | #6
On Fri, Sep 5, 2014 at 11:47 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Sep 01, 2014 at 02:53:40PM +0200, Anders Berg wrote:
>> The AXM55xx device has a ARM PL022 SSP controller with an extension that
>> enables the control of up to five chip select signals. This driver implements
>> the logic to control these signals and exports them as GPIOs.
>>
>> Signed-off-by: Anders Berg <anders.berg@avagotech.com>
>
> So, what happens if the GPIO driver binds first, followed by the SSP
> device?  Please test that scenario out before this patch gets merged.
>

Both scenarios work.
/Anders
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Sept. 6, 2014, 1:53 p.m. UTC | #7
On Fri, Sep 05, 2014 at 11:05:37AM +0200, Anders Berg wrote:

> Well, the problem with adding this to the spi-pl022 driver (as a
> vendor specific extension) is that this IP block unfortunately isn't
> distinguishable from the standard ARM PL022 implementation (same
> values in the PrimeCell identification registers). That's why I went
> down the GPIO path...

Would it not be more straightforward to just register it as a platform
device in this case, or use a separate OF ID in the DT?  We appear to
specify a compatible string when registering AMBA devices even if we are
capable of identifying using the ID registers.
Anders Berg Sept. 8, 2014, 11:02 a.m. UTC | #8
On Sat, Sep 6, 2014 at 3:53 PM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Sep 05, 2014 at 11:05:37AM +0200, Anders Berg wrote:
>
>> Well, the problem with adding this to the spi-pl022 driver (as a
>> vendor specific extension) is that this IP block unfortunately isn't
>> distinguishable from the standard ARM PL022 implementation (same
>> values in the PrimeCell identification registers). That's why I went
>> down the GPIO path...
>
> Would it not be more straightforward to just register it as a platform
> device in this case, or use a separate OF ID in the DT?  We appear to
> specify a compatible string when registering AMBA devices even if we are
> capable of identifying using the ID registers.

I'll drop this gpio patch for now and submit a patch on the pl022
driver instead. I read the arm,primecell binding and learned that
there is a property (arm,primecell-periphid) that can be used to
override the HW PrimeCell ID. Seems like something to use here, right?

/Anders
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Sept. 8, 2014, 11:35 a.m. UTC | #9
On Mon, Sep 08, 2014 at 01:02:32PM +0200, Anders Berg wrote:
> On Sat, Sep 6, 2014 at 3:53 PM, Mark Brown <broonie@kernel.org> wrote:

> > Would it not be more straightforward to just register it as a platform
> > device in this case, or use a separate OF ID in the DT?  We appear to
> > specify a compatible string when registering AMBA devices even if we are
> > capable of identifying using the ID registers.

> I'll drop this gpio patch for now and submit a patch on the pl022
> driver instead. I read the arm,primecell binding and learned that
> there is a property (arm,primecell-periphid) that can be used to
> override the HW PrimeCell ID. Seems like something to use here, right?

Possibly, though if there's no hardware ID to override to that might not
be the best option.  You could always define a property that's specific
to the pl022 driver flagging that it's got these extra chip selects though.

Patch
diff mbox

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 9de1515..c2f2686 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -112,6 +112,14 @@  config GPIO_MAX730X
 
 comment "Memory mapped GPIO drivers:"
 
+config GPIO_AXXIA_SSP
+	tristate "AXM55xx SSP chip select pins as GPIO"
+	depends on ARCH_AXXIA
+	default y if SPI_MASTER
+	select GPIO_GENERIC
+	help
+	  Say yes here to get GPIO interface for AXM55xx SSP chip select signals.
+
 config GPIO_CLPS711X
 	tristate "CLPS711X GPIO support"
 	depends on ARCH_CLPS711X || COMPILE_TEST
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 5d024e3..01e1869 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -18,6 +18,7 @@  obj-$(CONFIG_GPIO_ADP5520)	+= gpio-adp5520.o
 obj-$(CONFIG_GPIO_ADP5588)	+= gpio-adp5588.o
 obj-$(CONFIG_GPIO_AMD8111)	+= gpio-amd8111.o
 obj-$(CONFIG_GPIO_ARIZONA)	+= gpio-arizona.o
+obj-$(CONFIG_GPIO_AXXIA_SSP)	+= gpio-axxia-ssp.o
 obj-$(CONFIG_GPIO_BCM_KONA)	+= gpio-bcm-kona.o
 obj-$(CONFIG_GPIO_BT8XX)	+= gpio-bt8xx.o
 obj-$(CONFIG_GPIO_CLPS711X)	+= gpio-clps711x.o
diff --git a/drivers/gpio/gpio-axxia-ssp.c b/drivers/gpio/gpio-axxia-ssp.c
new file mode 100644
index 0000000..5884206
--- /dev/null
+++ b/drivers/gpio/gpio-axxia-ssp.c
@@ -0,0 +1,128 @@ 
+/*
+ * GPIO interface for SSP chip select outputs.
+ *
+ * Copyright (C) 2013 LSI Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/bitops.h>
+#include <linux/init.h>
+#include <linux/gpio.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/of.h>
+
+struct gpio_dev {
+	void __iomem    *reg;
+	struct gpio_chip gpio_chip;
+};
+
+static int ssp_gpio_direction_out(struct gpio_chip *chip,
+				  unsigned offset, int value)
+{
+	/* Always outputs */
+	return 0;
+}
+
+static int ssp_gpio_direction_in(struct gpio_chip *chip, unsigned offset)
+{
+	return -ENXIO;
+}
+
+static int ssp_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+	struct gpio_dev *priv = dev_get_drvdata(chip->dev);
+	u32 tmp;
+
+	tmp = readl(priv->reg);
+	return !!(tmp & BIT(offset));
+}
+
+static void ssp_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+	struct gpio_dev *priv = dev_get_drvdata(chip->dev);
+	u32 tmp;
+
+	tmp = readl(priv->reg);
+	if (value)
+		tmp |= BIT(offset);
+	else
+		tmp &= ~BIT(offset);
+	writel(tmp, priv->reg);
+}
+
+static int ssp_gpio_probe(struct platform_device *pdev)
+{
+	struct gpio_dev *priv;
+	struct resource *io;
+	struct gpio_chip *chip;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->reg = devm_ioremap(&pdev->dev, io->start, resource_size(io));
+	if (!priv->reg)
+		return -ENXIO;
+
+	platform_set_drvdata(pdev, priv);
+
+	chip = &priv->gpio_chip;
+	chip->dev = &pdev->dev;
+#ifdef CONFIG_OF_GPIO
+	chip->of_node = pdev->dev.of_node;
+#endif
+	chip->direction_input = ssp_gpio_direction_in;
+	chip->direction_output = ssp_gpio_direction_out;
+	chip->get = ssp_gpio_get;
+	chip->set = ssp_gpio_set;
+	chip->label = "ssp-gpio";
+	chip->owner = THIS_MODULE;
+	chip->base = -1;
+	chip->ngpio = 5;
+
+	/* Set all 5 chip-selects high (de-asserted) by default */
+	writel(0x1f, priv->reg);
+
+	ret = gpiochip_add(chip);
+	if (ret < 0)
+		dev_err(&pdev->dev, "failed to register chip, %d\n", ret);
+
+	return ret;
+}
+
+static int ssp_gpio_remove(struct platform_device *pdev)
+{
+	struct gpio_dev *priv = dev_get_drvdata(&pdev->dev);
+
+	return gpiochip_remove(&priv->gpio_chip);
+}
+
+static const struct of_device_id ssp_gpio_id_table[] = {
+	{ .compatible = "lsi,ssp-gpio" },
+	{}
+};
+MODULE_DEVICE_TABLE(platform, ssp_gpio_id_table);
+
+static struct platform_driver ssp_gpio_driver = {
+	.driver = {
+		.name = "ssp-gpio",
+		.owner = THIS_MODULE,
+		.of_match_table = ssp_gpio_id_table
+	},
+	.probe = ssp_gpio_probe,
+	.remove = ssp_gpio_remove,
+};
+
+module_platform_driver(ssp_gpio_driver);
+
+MODULE_AUTHOR("LSI Corporation");
+MODULE_DESCRIPTION("GPIO interface for SSP chip selects");
+MODULE_LICENSE("GPL");