| Message ID | 20200504155950.29065-2-rayagonda.kokatanur@broadcom.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | add brcm iproc gpio driver | expand |
Hi Rayagonda, On Mon, 4 May 2020 at 10:00, Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com> wrote: > > Add gpio driver support for Broadcom iproc-based socs. > This seems to be missing a change log. Can I suggest using patman to make this automatic? > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com> > Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli at broadcom.com> > --- > drivers/gpio/Kconfig | 11 ++ > drivers/gpio/Makefile | 1 + > drivers/gpio/iproc_gpio.c | 275 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 287 insertions(+) > create mode 100644 drivers/gpio/iproc_gpio.c > Regards, Simon
Hi Simon, On Tue, May 5, 2020 at 12:57 AM Simon Glass <sjg at chromium.org> wrote: > > Hi Rayagonda, > > On Mon, 4 May 2020 at 10:00, Rayagonda Kokatanur > <rayagonda.kokatanur at broadcom.com> wrote: > > > > Add gpio driver support for Broadcom iproc-based socs. > > > > This seems to be missing a change log. Can I suggest using patman to > make this automatic? All change logs are present in the cover letter. Please let me know if I still need to have a change log for individual patch. > > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com> > > Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli at broadcom.com> > > --- > > drivers/gpio/Kconfig | 11 ++ > > drivers/gpio/Makefile | 1 + > > drivers/gpio/iproc_gpio.c | 275 ++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 287 insertions(+) > > create mode 100644 drivers/gpio/iproc_gpio.c > > > Regards, > Simon
Hi Rayagonda, On Mon, 4 May 2020 at 10:00, Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com> wrote: > > Add gpio driver support for Broadcom iproc-based socs. > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com> > Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli at broadcom.com> > --- > drivers/gpio/Kconfig | 11 ++ > drivers/gpio/Makefile | 1 + > drivers/gpio/iproc_gpio.c | 275 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 287 insertions(+) > create mode 100644 drivers/gpio/iproc_gpio.c > Reviewed-by: Simon Glass <sjg at chromium.org> nits and suggestions below > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 2081520f42..57d2cd2e75 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -135,6 +135,17 @@ config IMX_RGPIO2P > help > This driver supports i.MX7ULP Rapid GPIO2P controller. > > +config IPROC_GPIO > + bool "Broadcom iProc GPIO driver(without pinconf)" > + default n > + help > + The Broadcom iProc based SoCs- Cygnus, NS2, NS3, NSP and Stingray, > + use the same GPIO Controller IP hence this driver could be used > + for all. > + > + The Broadcom iProc based SoCs have multiple GPIO controllers and only > + the always-ON GPIO controller (CRMU/AON) is supported by this driver. > + > config HSDK_CREG_GPIO > bool "HSDK CREG GPIO griver" > depends on DM_GPIO > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > index 7638259007..5dc5849477 100644 > --- a/drivers/gpio/Makefile > +++ b/drivers/gpio/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_CORTINA_GPIO) += cortina_gpio.o > obj-$(CONFIG_INTEL_GPIO) += intel_gpio.o > obj-$(CONFIG_INTEL_ICH6_GPIO) += intel_ich6_gpio.o > obj-$(CONFIG_INTEL_BROADWELL_GPIO) += intel_broadwell_gpio.o > +obj-$(CONFIG_IPROC_GPIO) += iproc_gpio.o > obj-$(CONFIG_KIRKWOOD_GPIO) += kw_gpio.o > obj-$(CONFIG_KONA_GPIO) += kona_gpio.o > obj-$(CONFIG_MARVELL_GPIO) += mvgpio.o > diff --git a/drivers/gpio/iproc_gpio.c b/drivers/gpio/iproc_gpio.c > new file mode 100644 > index 0000000000..f75831d6e7 > --- /dev/null > +++ b/drivers/gpio/iproc_gpio.c > @@ -0,0 +1,275 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2020 Broadcom > + */ > + > +#include <common.h> > +#include <errno.h> > +#include <asm/gpio.h> > +#include <asm/io.h> > +#include <dm.h> > +#include <dm/device_compat.h> > +#include <dm/devres.h> > +#include <dm/pinctrl.h> Do you need that one? > + > +/* > + * There are five GPIO bank register. Each bank can configure max of 32 gpios. > + * BANK0 - gpios 0 to 31 > + * BANK1 - gpios 32 to 63 > + * BANK2 - gpios 64 to 95 > + * BANK3 - gpios 96 to 127 > + * BANK4 - gpios 128 to 150 > + * > + * Offset difference between consecutive bank register is 0x200 > + */ > +#define NGPIO_PER_BANK 32 > +#define GPIO_BANK_SIZE 0x200 > +#define GPIO_BANK(pin) ((pin) / NGPIO_PER_BANK) > +#define IPROC_GPIO_SHIFT(pin) ((pin) % NGPIO_PER_BANK) > +#define IPROC_GPIO_REG(pin, reg) (GPIO_BANK_SIZE * GPIO_BANK(pin) + (reg)) > + > +/* device register offset */ > +#define IPROC_GPIO_DATA_IN_OFFSET 0x00 > +#define IPROC_GPIO_DATA_OUT_OFFSET 0x04 > +#define IPROC_GPIO_OUT_EN_OFFSET 0x08 > + > +/** > + * struct iproc_gpio_pctrl_map - mapping between gpio and pinctrl specfied > + * using gpio-ranges parameter in dt. > + * @gpio_pin: start of gpio number in gpio-ranges > + * @pctrl_pin: start of pinctrl number in gpio-ranges > + * @npins: total number of pins in gpio-ranges > + * @node: list node > + */ > +struct iproc_gpio_pctrl_map { > + u32 gpio_pin; > + u32 pctrl_pin; > + u32 npins; > + struct list_head node; > +}; > + > +/** > + * struct iproc_gpio_pctrl_map - gpio device instance > + * @pinctrl_dev:pointer to pinctrl device > + * @gpiomap: list node having mapping between gpio and pinctrl > + * @base: I/O register base address of gpio device > + * @name: gpio device name, ex GPIO0, GPIO1 > + * @ngpios: total number of gpios > + */ > +struct iproc_gpio_platdata { > + struct udevice *pinctrl_dev; > + struct list_head gpiomap; > + void __iomem *base; > + char *name; > + u32 ngpios; > +}; > + > +/** > + * iproc_gpio_set_bit - set or clear one bit (corresponding to the GPIO pin) Please fit the title on a single line. You can add more detail later. E.g. iproc_gpio_set_bit() - set or clear one bit in an iproc GPIO register The bit relates to a GPIO pin > + * in a iproc GPIO register > + * > + * @iproc_gpio: Iproc GPIO device But here you are actually passing plat > + * @reg: register offset > + * @gpio: GPIO pin > + * @set: set or clear > + */ > +static inline void iproc_gpio_set_bit(struct iproc_gpio_platdata *plat, > + u32 reg, u32 gpio, bool set) > +{ > + u32 offset = IPROC_GPIO_REG(gpio, reg); > + u32 shift = IPROC_GPIO_SHIFT(gpio); > + > + clrsetbits_le32(plat->base + offset, BIT(shift), > + (set ? BIT(shift) : 0)); > +} > + > +static inline bool iproc_gpio_get_bit(struct iproc_gpio_platdata *plat, > + u32 reg, u32 gpio) > +{ > + u32 offset = IPROC_GPIO_REG(gpio, reg); > + u32 shift = IPROC_GPIO_SHIFT(gpio); > + > + return readl(plat->base + offset) & BIT(shift); > +} > + > +static u32 iproc_get_pctrl_from_gpio(struct iproc_gpio_platdata *plat, u32 gpio) function comment > +{ > + struct iproc_gpio_pctrl_map *range = NULL; > + struct list_head *pos, *tmp; > + u32 ret = 0; > + > + list_for_each_safe(pos, tmp, &plat->gpiomap) { > + range = list_entry(pos, struct iproc_gpio_pctrl_map, node); > + if (gpio == range->gpio_pin || > + gpio < (range->gpio_pin + range->npins)) { > + ret = range->pctrl_pin + (gpio - range->gpio_pin); > + break; > + } > + } > + > + return ret; > +} > + > +static int iproc_get_gpio_pctrl_mapping(struct udevice *dev) function comment > +{ > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > + struct iproc_gpio_pctrl_map *range = NULL; > + struct ofnode_phandle_args args; > + int index = 0, ret; > + > + for (;; index++) { > + ret = dev_read_phandle_with_args(dev, "gpio-ranges", > + NULL, 3, index, &args); > + if (ret) > + break; > + > + range = devm_kzalloc(dev, sizeof(*range), GFP_KERNEL); > + if (!range) > + return -ENOMEM; > + > + range->gpio_pin = args.args[0]; > + range->pctrl_pin = args.args[1]; > + range->npins = args.args[2]; > + list_add_tail(&range->node, &plat->gpiomap); > + } > + > + return 0; > +} > + > +static int iproc_gpio_request(struct udevice *dev, u32 gpio, const char *label) > +{ > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > + u32 pctrl; > + > + if (!plat->pinctrl_dev) comment why you are doing this return. I think it is because it is a top-level device? > + return 0; > + > + pctrl = iproc_get_pctrl_from_gpio(plat, gpio); > + > + return pinctrl_request(plat->pinctrl_dev, pctrl, 0); > +} > + > +static int iproc_gpio_direction_input(struct udevice *dev, u32 gpio) > +{ > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > + > + iproc_gpio_set_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio, false); You might consider dropping the IPROC_GPIO prefix on these, or at least the IPROC prefix. They are close to this file so there is no need for all the boilerplate. > + dev_dbg(dev, "gpio:%u set input\n", gpio); > + > + return 0; > +} > + > +static int iproc_gpio_direction_output(struct udevice *dev, u32 gpio, int value) > +{ > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > + > + iproc_gpio_set_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio, true); > + iproc_gpio_set_bit(plat, IPROC_GPIO_DATA_OUT_OFFSET, gpio, value); > + dev_dbg(dev, "gpio:%u set output, value:%d\n", gpio, value); > + > + return 0; > +} > + > +static int iproc_gpio_get_value(struct udevice *dev, u32 gpio) > +{ > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > + int value; > + > + value = iproc_gpio_get_bit(plat, IPROC_GPIO_DATA_IN_OFFSET, gpio); > + dev_dbg(dev, "gpio:%u get, value:%d\n", gpio, value); > + > + return value; > +} > + > +static int iproc_gpio_set_value(struct udevice *dev, u32 gpio, int value) > +{ > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > + > + if (iproc_gpio_get_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio)) > + iproc_gpio_set_bit(plat, IPROC_GPIO_DATA_OUT_OFFSET, gpio, > + value); > + > + dev_dbg(dev, "gpio:%u set, value:%d\n", gpio, value); > + return 0; > +} > + > +static int iproc_gpio_get_function(struct udevice *dev, u32 gpio) > +{ > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > + > + if (iproc_gpio_get_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio)) > + return GPIOF_OUTPUT; > + else > + return GPIOF_INPUT; > +} > + > +static int iproc_gpio_ofdata_to_platdata(struct udevice *dev) > +{ > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > + struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); > + int ret; > + char name[10]; > + > + plat->base = dev_read_addr_ptr(dev); > + if (!plat->base) { > + debug("%s: Failed to get base address\n", __func__); > + return -EINVAL; > + } > + > + ret = dev_read_u32(dev, "ngpios", &plat->ngpios); > + if (ret < 0) { > + dev_err(dev, "%s: Failed to get ngpios\n", __func__); Note that this adds size to the driver. If you don't care about that it is fine, otherwise consider using log_debug() > + return ret; > + } > + > + uclass_get_device_by_phandle(UCLASS_PINCTRL, dev, "gpio-ranges", > + &plat->pinctrl_dev); > + if (ret < 0) { > + dev_err(dev, "%s: Failed to get pinctrl phandle\n", __func__); > + return ret; > + } > + > + INIT_LIST_HEAD(&plat->gpiomap); > + ret = iproc_get_gpio_pctrl_mapping(dev); > + if (ret < 0) { > + dev_err(dev, "%s: Failed to get gpio to pctrl map ret(%d)\n", > + __func__, ret); > + return ret; > + } > + > + snprintf(name, sizeof(name), "GPIO%d", dev->req_seq); > + plat->name = strdup(name); > + if (!plat->name) > + return -ENOMEM; > + > + uc_priv->gpio_count = plat->ngpios; > + uc_priv->bank_name = plat->name; > + > + dev_info(dev, ":bank name(%s) base %p, #gpios %d\n", > + plat->name, plat->base, plat->ngpios); > + > + return 0; > +} > + > +static const struct dm_gpio_ops iproc_gpio_ops = { > + .request = iproc_gpio_request, > + .direction_input = iproc_gpio_direction_input, > + .direction_output = iproc_gpio_direction_output, > + .get_value = iproc_gpio_get_value, > + .set_value = iproc_gpio_set_value, > + .get_function = iproc_gpio_get_function, > +}; > + > +static const struct udevice_id iproc_gpio_ids[] = { > + { .compatible = "brcm,iproc-gpio" }, > + { } > +}; > + > +U_BOOT_DRIVER(iproc_gpio) = { > + .name = "iproc_gpio", > + .id = UCLASS_GPIO, > + .of_match = iproc_gpio_ids, > + .ops = &iproc_gpio_ops, > + .ofdata_to_platdata = iproc_gpio_ofdata_to_platdata, > + .platdata_auto_alloc_size = sizeof(struct iproc_gpio_platdata), > +}; > -- > 2.17.1 > Regards, Simon
Hi Rayagonda, On Tue, 5 May 2020 at 00:32, Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com> wrote: > > Hi Simon, > > On Tue, May 5, 2020 at 12:57 AM Simon Glass <sjg at chromium.org> wrote: > > > > Hi Rayagonda, > > > > On Mon, 4 May 2020 at 10:00, Rayagonda Kokatanur > > <rayagonda.kokatanur at broadcom.com> wrote: > > > > > > Add gpio driver support for Broadcom iproc-based socs. > > > > > > > This seems to be missing a change log. Can I suggest using patman to > > make this automatic? > > All change logs are present in the cover letter. > Please let me know if I still need to have a change log for individual patch. Yes please. As I mentioned, patman does this for you. - Simon > > > > > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com> > > > Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli at broadcom.com> > > > --- > > > drivers/gpio/Kconfig | 11 ++ > > > drivers/gpio/Makefile | 1 + > > > drivers/gpio/iproc_gpio.c | 275 ++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 287 insertions(+) > > > create mode 100644 drivers/gpio/iproc_gpio.c > > > > > Regards, > > Simon
Hi Simon, On Tue, May 5, 2020 at 9:32 PM Simon Glass <sjg at chromium.org> wrote: > > Hi Rayagonda, > > On Mon, 4 May 2020 at 10:00, Rayagonda Kokatanur > <rayagonda.kokatanur at broadcom.com> wrote: > > > > Add gpio driver support for Broadcom iproc-based socs. > > > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com> > > Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli at broadcom.com> > > --- > > drivers/gpio/Kconfig | 11 ++ > > drivers/gpio/Makefile | 1 + > > drivers/gpio/iproc_gpio.c | 275 ++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 287 insertions(+) > > create mode 100644 drivers/gpio/iproc_gpio.c > > > > Reviewed-by: Simon Glass <sjg at chromium.org> > > nits and suggestions below > > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > > index 2081520f42..57d2cd2e75 100644 > > --- a/drivers/gpio/Kconfig > > +++ b/drivers/gpio/Kconfig > > @@ -135,6 +135,17 @@ config IMX_RGPIO2P > > help > > This driver supports i.MX7ULP Rapid GPIO2P controller. > > > > +config IPROC_GPIO > > + bool "Broadcom iProc GPIO driver(without pinconf)" > > + default n > > + help > > + The Broadcom iProc based SoCs- Cygnus, NS2, NS3, NSP and Stingray, > > + use the same GPIO Controller IP hence this driver could be used > > + for all. > > + > > + The Broadcom iProc based SoCs have multiple GPIO controllers and only > > + the always-ON GPIO controller (CRMU/AON) is supported by this driver. > > + > > config HSDK_CREG_GPIO > > bool "HSDK CREG GPIO griver" > > depends on DM_GPIO > > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > > index 7638259007..5dc5849477 100644 > > --- a/drivers/gpio/Makefile > > +++ b/drivers/gpio/Makefile > > @@ -19,6 +19,7 @@ obj-$(CONFIG_CORTINA_GPIO) += cortina_gpio.o > > obj-$(CONFIG_INTEL_GPIO) += intel_gpio.o > > obj-$(CONFIG_INTEL_ICH6_GPIO) += intel_ich6_gpio.o > > obj-$(CONFIG_INTEL_BROADWELL_GPIO) += intel_broadwell_gpio.o > > +obj-$(CONFIG_IPROC_GPIO) += iproc_gpio.o > > obj-$(CONFIG_KIRKWOOD_GPIO) += kw_gpio.o > > obj-$(CONFIG_KONA_GPIO) += kona_gpio.o > > obj-$(CONFIG_MARVELL_GPIO) += mvgpio.o > > diff --git a/drivers/gpio/iproc_gpio.c b/drivers/gpio/iproc_gpio.c > > new file mode 100644 > > index 0000000000..f75831d6e7 > > --- /dev/null > > +++ b/drivers/gpio/iproc_gpio.c > > @@ -0,0 +1,275 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (C) 2020 Broadcom > > + */ > > + > > +#include <common.h> > > +#include <errno.h> > > +#include <asm/gpio.h> > > +#include <asm/io.h> > > +#include <dm.h> > > +#include <dm/device_compat.h> > > +#include <dm/devres.h> > > +#include <dm/pinctrl.h> > > Do you need that one? Yes #include <dm/pinctrl.h> is required for pinctrl_request() used in iproc_gpio_request() function. > > > + > > +/* > > + * There are five GPIO bank register. Each bank can configure max of 32 gpios. > > + * BANK0 - gpios 0 to 31 > > + * BANK1 - gpios 32 to 63 > > + * BANK2 - gpios 64 to 95 > > + * BANK3 - gpios 96 to 127 > > + * BANK4 - gpios 128 to 150 > > + * > > + * Offset difference between consecutive bank register is 0x200 > > + */ > > +#define NGPIO_PER_BANK 32 > > +#define GPIO_BANK_SIZE 0x200 > > +#define GPIO_BANK(pin) ((pin) / NGPIO_PER_BANK) > > +#define IPROC_GPIO_SHIFT(pin) ((pin) % NGPIO_PER_BANK) > > +#define IPROC_GPIO_REG(pin, reg) (GPIO_BANK_SIZE * GPIO_BANK(pin) + (reg)) > > + > > +/* device register offset */ > > +#define IPROC_GPIO_DATA_IN_OFFSET 0x00 > > +#define IPROC_GPIO_DATA_OUT_OFFSET 0x04 > > +#define IPROC_GPIO_OUT_EN_OFFSET 0x08 > > + > > +/** > > + * struct iproc_gpio_pctrl_map - mapping between gpio and pinctrl specfied > > + * using gpio-ranges parameter in dt. > > + * @gpio_pin: start of gpio number in gpio-ranges > > + * @pctrl_pin: start of pinctrl number in gpio-ranges > > + * @npins: total number of pins in gpio-ranges > > + * @node: list node > > + */ > > +struct iproc_gpio_pctrl_map { > > + u32 gpio_pin; > > + u32 pctrl_pin; > > + u32 npins; > > + struct list_head node; > > +}; > > + > > +/** > > + * struct iproc_gpio_pctrl_map - gpio device instance > > + * @pinctrl_dev:pointer to pinctrl device > > + * @gpiomap: list node having mapping between gpio and pinctrl > > + * @base: I/O register base address of gpio device > > + * @name: gpio device name, ex GPIO0, GPIO1 > > + * @ngpios: total number of gpios > > + */ > > +struct iproc_gpio_platdata { > > + struct udevice *pinctrl_dev; > > + struct list_head gpiomap; > > + void __iomem *base; > > + char *name; > > + u32 ngpios; > > +}; > > + > > +/** > > + * iproc_gpio_set_bit - set or clear one bit (corresponding to the GPIO pin) > > Please fit the title on a single line. You can add more detail later. E.g. > > iproc_gpio_set_bit() - set or clear one bit in an iproc GPIO register > > The bit relates to a GPIO pin > > > > > + * in a iproc GPIO register > > + * > > + * @iproc_gpio: Iproc GPIO device > > But here you are actually passing plat > > > + * @reg: register offset > > + * @gpio: GPIO pin > > + * @set: set or clear > > + */ > > +static inline void iproc_gpio_set_bit(struct iproc_gpio_platdata *plat, > > + u32 reg, u32 gpio, bool set) > > +{ > > + u32 offset = IPROC_GPIO_REG(gpio, reg); > > + u32 shift = IPROC_GPIO_SHIFT(gpio); > > + > > + clrsetbits_le32(plat->base + offset, BIT(shift), > > + (set ? BIT(shift) : 0)); > > +} > > + > > +static inline bool iproc_gpio_get_bit(struct iproc_gpio_platdata *plat, > > + u32 reg, u32 gpio) > > +{ > > + u32 offset = IPROC_GPIO_REG(gpio, reg); > > + u32 shift = IPROC_GPIO_SHIFT(gpio); > > + > > + return readl(plat->base + offset) & BIT(shift); > > +} > > + > > +static u32 iproc_get_pctrl_from_gpio(struct iproc_gpio_platdata *plat, u32 gpio) > > function comment > > > +{ > > + struct iproc_gpio_pctrl_map *range = NULL; > > + struct list_head *pos, *tmp; > > + u32 ret = 0; > > + > > + list_for_each_safe(pos, tmp, &plat->gpiomap) { > > + range = list_entry(pos, struct iproc_gpio_pctrl_map, node); > > + if (gpio == range->gpio_pin || > > + gpio < (range->gpio_pin + range->npins)) { > > + ret = range->pctrl_pin + (gpio - range->gpio_pin); > > + break; > > + } > > + } > > + > > + return ret; > > +} > > + > > +static int iproc_get_gpio_pctrl_mapping(struct udevice *dev) > > function comment > > > +{ > > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > > + struct iproc_gpio_pctrl_map *range = NULL; > > + struct ofnode_phandle_args args; > > + int index = 0, ret; > > + > > + for (;; index++) { > > + ret = dev_read_phandle_with_args(dev, "gpio-ranges", > > + NULL, 3, index, &args); > > + if (ret) > > + break; > > + > > + range = devm_kzalloc(dev, sizeof(*range), GFP_KERNEL); > > + if (!range) > > + return -ENOMEM; > > + > > + range->gpio_pin = args.args[0]; > > + range->pctrl_pin = args.args[1]; > > + range->npins = args.args[2]; > > + list_add_tail(&range->node, &plat->gpiomap); > > + } > > + > > + return 0; > > +} > > + > > +static int iproc_gpio_request(struct udevice *dev, u32 gpio, const char *label) > > +{ > > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > > + u32 pctrl; > > + > > + if (!plat->pinctrl_dev) > > comment why you are doing this return. I think it is because it is a > top-level device? > > > + return 0; > > + > > + pctrl = iproc_get_pctrl_from_gpio(plat, gpio); > > + > > + return pinctrl_request(plat->pinctrl_dev, pctrl, 0); > > +} > > + > > +static int iproc_gpio_direction_input(struct udevice *dev, u32 gpio) > > +{ > > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > > + > > + iproc_gpio_set_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio, false); > > You might consider dropping the IPROC_GPIO prefix on these, or at > least the IPROC prefix. They are close to this file so there is no > need for all the boilerplate. > > > + dev_dbg(dev, "gpio:%u set input\n", gpio); > > + > > + return 0; > > +} > > + > > +static int iproc_gpio_direction_output(struct udevice *dev, u32 gpio, int value) > > +{ > > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > > + > > + iproc_gpio_set_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio, true); > > + iproc_gpio_set_bit(plat, IPROC_GPIO_DATA_OUT_OFFSET, gpio, value); > > + dev_dbg(dev, "gpio:%u set output, value:%d\n", gpio, value); > > + > > + return 0; > > +} > > + > > +static int iproc_gpio_get_value(struct udevice *dev, u32 gpio) > > +{ > > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > > + int value; > > + > > + value = iproc_gpio_get_bit(plat, IPROC_GPIO_DATA_IN_OFFSET, gpio); > > + dev_dbg(dev, "gpio:%u get, value:%d\n", gpio, value); > > + > > + return value; > > +} > > + > > +static int iproc_gpio_set_value(struct udevice *dev, u32 gpio, int value) > > +{ > > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > > + > > + if (iproc_gpio_get_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio)) > > + iproc_gpio_set_bit(plat, IPROC_GPIO_DATA_OUT_OFFSET, gpio, > > + value); > > + > > + dev_dbg(dev, "gpio:%u set, value:%d\n", gpio, value); > > + return 0; > > +} > > + > > +static int iproc_gpio_get_function(struct udevice *dev, u32 gpio) > > +{ > > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > > + > > + if (iproc_gpio_get_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio)) > > + return GPIOF_OUTPUT; > > + else > > + return GPIOF_INPUT; > > +} > > + > > +static int iproc_gpio_ofdata_to_platdata(struct udevice *dev) > > +{ > > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); > > + struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); > > + int ret; > > + char name[10]; > > + > > + plat->base = dev_read_addr_ptr(dev); > > + if (!plat->base) { > > + debug("%s: Failed to get base address\n", __func__); > > + return -EINVAL; > > + } > > + > > + ret = dev_read_u32(dev, "ngpios", &plat->ngpios); > > + if (ret < 0) { > > + dev_err(dev, "%s: Failed to get ngpios\n", __func__); > > Note that this adds size to the driver. If you don't care about that > it is fine, otherwise consider using log_debug() Thanks for the info. Since it is an important error message and I feel it should be dev_err. Rest of the review comments will fix and send v6. Regards, Rayagonda > > > + return ret; > > + } > > + > > + uclass_get_device_by_phandle(UCLASS_PINCTRL, dev, "gpio-ranges", > > + &plat->pinctrl_dev); > > + if (ret < 0) { > > + dev_err(dev, "%s: Failed to get pinctrl phandle\n", __func__); > > + return ret; > > + } > > + > > + INIT_LIST_HEAD(&plat->gpiomap); > > + ret = iproc_get_gpio_pctrl_mapping(dev); > > + if (ret < 0) { > > + dev_err(dev, "%s: Failed to get gpio to pctrl map ret(%d)\n", > > + __func__, ret); > > + return ret; > > + } > > + > > + snprintf(name, sizeof(name), "GPIO%d", dev->req_seq); > > + plat->name = strdup(name); > > + if (!plat->name) > > + return -ENOMEM; > > + > > + uc_priv->gpio_count = plat->ngpios; > > + uc_priv->bank_name = plat->name; > > + > > + dev_info(dev, ":bank name(%s) base %p, #gpios %d\n", > > + plat->name, plat->base, plat->ngpios); > > + > > + return 0; > > +} > > + > > +static const struct dm_gpio_ops iproc_gpio_ops = { > > + .request = iproc_gpio_request, > > + .direction_input = iproc_gpio_direction_input, > > + .direction_output = iproc_gpio_direction_output, > > + .get_value = iproc_gpio_get_value, > > + .set_value = iproc_gpio_set_value, > > + .get_function = iproc_gpio_get_function, > > +}; > > + > > +static const struct udevice_id iproc_gpio_ids[] = { > > + { .compatible = "brcm,iproc-gpio" }, > > + { } > > +}; > > + > > +U_BOOT_DRIVER(iproc_gpio) = { > > + .name = "iproc_gpio", > > + .id = UCLASS_GPIO, > > + .of_match = iproc_gpio_ids, > > + .ops = &iproc_gpio_ops, > > + .ofdata_to_platdata = iproc_gpio_ofdata_to_platdata, > > + .platdata_auto_alloc_size = sizeof(struct iproc_gpio_platdata), > > +}; > > -- > > 2.17.1 > > > > Regards, > Simon
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 2081520f42..57d2cd2e75 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -135,6 +135,17 @@ config IMX_RGPIO2P help This driver supports i.MX7ULP Rapid GPIO2P controller. +config IPROC_GPIO + bool "Broadcom iProc GPIO driver(without pinconf)" + default n + help + The Broadcom iProc based SoCs- Cygnus, NS2, NS3, NSP and Stingray, + use the same GPIO Controller IP hence this driver could be used + for all. + + The Broadcom iProc based SoCs have multiple GPIO controllers and only + the always-ON GPIO controller (CRMU/AON) is supported by this driver. + config HSDK_CREG_GPIO bool "HSDK CREG GPIO griver" depends on DM_GPIO diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 7638259007..5dc5849477 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_CORTINA_GPIO) += cortina_gpio.o obj-$(CONFIG_INTEL_GPIO) += intel_gpio.o obj-$(CONFIG_INTEL_ICH6_GPIO) += intel_ich6_gpio.o obj-$(CONFIG_INTEL_BROADWELL_GPIO) += intel_broadwell_gpio.o +obj-$(CONFIG_IPROC_GPIO) += iproc_gpio.o obj-$(CONFIG_KIRKWOOD_GPIO) += kw_gpio.o obj-$(CONFIG_KONA_GPIO) += kona_gpio.o obj-$(CONFIG_MARVELL_GPIO) += mvgpio.o diff --git a/drivers/gpio/iproc_gpio.c b/drivers/gpio/iproc_gpio.c new file mode 100644 index 0000000000..f75831d6e7 --- /dev/null +++ b/drivers/gpio/iproc_gpio.c @@ -0,0 +1,275 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2020 Broadcom + */ + +#include <common.h> +#include <errno.h> +#include <asm/gpio.h> +#include <asm/io.h> +#include <dm.h> +#include <dm/device_compat.h> +#include <dm/devres.h> +#include <dm/pinctrl.h> + +/* + * There are five GPIO bank register. Each bank can configure max of 32 gpios. + * BANK0 - gpios 0 to 31 + * BANK1 - gpios 32 to 63 + * BANK2 - gpios 64 to 95 + * BANK3 - gpios 96 to 127 + * BANK4 - gpios 128 to 150 + * + * Offset difference between consecutive bank register is 0x200 + */ +#define NGPIO_PER_BANK 32 +#define GPIO_BANK_SIZE 0x200 +#define GPIO_BANK(pin) ((pin) / NGPIO_PER_BANK) +#define IPROC_GPIO_SHIFT(pin) ((pin) % NGPIO_PER_BANK) +#define IPROC_GPIO_REG(pin, reg) (GPIO_BANK_SIZE * GPIO_BANK(pin) + (reg)) + +/* device register offset */ +#define IPROC_GPIO_DATA_IN_OFFSET 0x00 +#define IPROC_GPIO_DATA_OUT_OFFSET 0x04 +#define IPROC_GPIO_OUT_EN_OFFSET 0x08 + +/** + * struct iproc_gpio_pctrl_map - mapping between gpio and pinctrl specfied + * using gpio-ranges parameter in dt. + * @gpio_pin: start of gpio number in gpio-ranges + * @pctrl_pin: start of pinctrl number in gpio-ranges + * @npins: total number of pins in gpio-ranges + * @node: list node + */ +struct iproc_gpio_pctrl_map { + u32 gpio_pin; + u32 pctrl_pin; + u32 npins; + struct list_head node; +}; + +/** + * struct iproc_gpio_pctrl_map - gpio device instance + * @pinctrl_dev:pointer to pinctrl device + * @gpiomap: list node having mapping between gpio and pinctrl + * @base: I/O register base address of gpio device + * @name: gpio device name, ex GPIO0, GPIO1 + * @ngpios: total number of gpios + */ +struct iproc_gpio_platdata { + struct udevice *pinctrl_dev; + struct list_head gpiomap; + void __iomem *base; + char *name; + u32 ngpios; +}; + +/** + * iproc_gpio_set_bit - set or clear one bit (corresponding to the GPIO pin) + * in a iproc GPIO register + * + * @iproc_gpio: Iproc GPIO device + * @reg: register offset + * @gpio: GPIO pin + * @set: set or clear + */ +static inline void iproc_gpio_set_bit(struct iproc_gpio_platdata *plat, + u32 reg, u32 gpio, bool set) +{ + u32 offset = IPROC_GPIO_REG(gpio, reg); + u32 shift = IPROC_GPIO_SHIFT(gpio); + + clrsetbits_le32(plat->base + offset, BIT(shift), + (set ? BIT(shift) : 0)); +} + +static inline bool iproc_gpio_get_bit(struct iproc_gpio_platdata *plat, + u32 reg, u32 gpio) +{ + u32 offset = IPROC_GPIO_REG(gpio, reg); + u32 shift = IPROC_GPIO_SHIFT(gpio); + + return readl(plat->base + offset) & BIT(shift); +} + +static u32 iproc_get_pctrl_from_gpio(struct iproc_gpio_platdata *plat, u32 gpio) +{ + struct iproc_gpio_pctrl_map *range = NULL; + struct list_head *pos, *tmp; + u32 ret = 0; + + list_for_each_safe(pos, tmp, &plat->gpiomap) { + range = list_entry(pos, struct iproc_gpio_pctrl_map, node); + if (gpio == range->gpio_pin || + gpio < (range->gpio_pin + range->npins)) { + ret = range->pctrl_pin + (gpio - range->gpio_pin); + break; + } + } + + return ret; +} + +static int iproc_get_gpio_pctrl_mapping(struct udevice *dev) +{ + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); + struct iproc_gpio_pctrl_map *range = NULL; + struct ofnode_phandle_args args; + int index = 0, ret; + + for (;; index++) { + ret = dev_read_phandle_with_args(dev, "gpio-ranges", + NULL, 3, index, &args); + if (ret) + break; + + range = devm_kzalloc(dev, sizeof(*range), GFP_KERNEL); + if (!range) + return -ENOMEM; + + range->gpio_pin = args.args[0]; + range->pctrl_pin = args.args[1]; + range->npins = args.args[2]; + list_add_tail(&range->node, &plat->gpiomap); + } + + return 0; +} + +static int iproc_gpio_request(struct udevice *dev, u32 gpio, const char *label) +{ + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); + u32 pctrl; + + if (!plat->pinctrl_dev) + return 0; + + pctrl = iproc_get_pctrl_from_gpio(plat, gpio); + + return pinctrl_request(plat->pinctrl_dev, pctrl, 0); +} + +static int iproc_gpio_direction_input(struct udevice *dev, u32 gpio) +{ + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); + + iproc_gpio_set_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio, false); + dev_dbg(dev, "gpio:%u set input\n", gpio); + + return 0; +} + +static int iproc_gpio_direction_output(struct udevice *dev, u32 gpio, int value) +{ + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); + + iproc_gpio_set_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio, true); + iproc_gpio_set_bit(plat, IPROC_GPIO_DATA_OUT_OFFSET, gpio, value); + dev_dbg(dev, "gpio:%u set output, value:%d\n", gpio, value); + + return 0; +} + +static int iproc_gpio_get_value(struct udevice *dev, u32 gpio) +{ + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); + int value; + + value = iproc_gpio_get_bit(plat, IPROC_GPIO_DATA_IN_OFFSET, gpio); + dev_dbg(dev, "gpio:%u get, value:%d\n", gpio, value); + + return value; +} + +static int iproc_gpio_set_value(struct udevice *dev, u32 gpio, int value) +{ + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); + + if (iproc_gpio_get_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio)) + iproc_gpio_set_bit(plat, IPROC_GPIO_DATA_OUT_OFFSET, gpio, + value); + + dev_dbg(dev, "gpio:%u set, value:%d\n", gpio, value); + return 0; +} + +static int iproc_gpio_get_function(struct udevice *dev, u32 gpio) +{ + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); + + if (iproc_gpio_get_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio)) + return GPIOF_OUTPUT; + else + return GPIOF_INPUT; +} + +static int iproc_gpio_ofdata_to_platdata(struct udevice *dev) +{ + struct iproc_gpio_platdata *plat = dev_get_platdata(dev); + struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); + int ret; + char name[10]; + + plat->base = dev_read_addr_ptr(dev); + if (!plat->base) { + debug("%s: Failed to get base address\n", __func__); + return -EINVAL; + } + + ret = dev_read_u32(dev, "ngpios", &plat->ngpios); + if (ret < 0) { + dev_err(dev, "%s: Failed to get ngpios\n", __func__); + return ret; + } + + uclass_get_device_by_phandle(UCLASS_PINCTRL, dev, "gpio-ranges", + &plat->pinctrl_dev); + if (ret < 0) { + dev_err(dev, "%s: Failed to get pinctrl phandle\n", __func__); + return ret; + } + + INIT_LIST_HEAD(&plat->gpiomap); + ret = iproc_get_gpio_pctrl_mapping(dev); + if (ret < 0) { + dev_err(dev, "%s: Failed to get gpio to pctrl map ret(%d)\n", + __func__, ret); + return ret; + } + + snprintf(name, sizeof(name), "GPIO%d", dev->req_seq); + plat->name = strdup(name); + if (!plat->name) + return -ENOMEM; + + uc_priv->gpio_count = plat->ngpios; + uc_priv->bank_name = plat->name; + + dev_info(dev, ":bank name(%s) base %p, #gpios %d\n", + plat->name, plat->base, plat->ngpios); + + return 0; +} + +static const struct dm_gpio_ops iproc_gpio_ops = { + .request = iproc_gpio_request, + .direction_input = iproc_gpio_direction_input, + .direction_output = iproc_gpio_direction_output, + .get_value = iproc_gpio_get_value, + .set_value = iproc_gpio_set_value, + .get_function = iproc_gpio_get_function, +}; + +static const struct udevice_id iproc_gpio_ids[] = { + { .compatible = "brcm,iproc-gpio" }, + { } +}; + +U_BOOT_DRIVER(iproc_gpio) = { + .name = "iproc_gpio", + .id = UCLASS_GPIO, + .of_match = iproc_gpio_ids, + .ops = &iproc_gpio_ops, + .ofdata_to_platdata = iproc_gpio_ofdata_to_platdata, + .platdata_auto_alloc_size = sizeof(struct iproc_gpio_platdata), +};