| Message ID | 1409576021-24828-2-git-send-email-anders.berg@avagotech.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
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
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?
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
> 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
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.
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
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.
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
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.
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");
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