From: Guenter Roeck <linux@roeck-us.net> To: Michael Walle <michael@walle.cc> Cc: linux-gpio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org, linux-pwm@vger.kernel.org, linux-watchdog@vger.kernel.org, linux-arm-kernel@lists.infradead.org, "Linus Walleij" <linus.walleij@linaro.org>, "Bartosz Golaszewski" <bgolaszewski@baylibre.com>, "Rob Herring" <robh+dt@kernel.org>, "Jean Delvare" <jdelvare@suse.com>, "Lee Jones" <lee.jones@linaro.org>, "Thierry Reding" <thierry.reding@gmail.com>, "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>, "Wim Van Sebroeck" <wim@linux-watchdog.org>, "Shawn Guo" <shawnguo@kernel.org>, "Li Yang" <leoyang.li@nxp.com>, "Thomas Gleixner" <tglx@linutronix.de>, "Jason Cooper" <jason@lakedaemon.net>, "Marc Zyngier" <maz@kernel.org>, "Guenter Roeck" <groeck7@gmail.com> Subject: Re: [PATCH 06/18] irqchip: add sl28cpld interrupt controller support Date: Wed, 18 Mar 2020 13:35:51 -0700 Message-ID: <d2ede8b3-75ab-36f3-bc7e-8f7169977fb7@roeck-us.net> (raw) In-Reply-To: <b9661afeda9fc6413b2fe392f254c685@walle.cc> On 3/18/20 10:06 AM, Michael Walle wrote: > Am 2020-03-18 17:53, schrieb Guenter Roeck: >> On 3/17/20 1:50 PM, Michael Walle wrote: >>> This patch adds support for the interrupt controller inside the sl28 >>> CPLD management controller. >>> >>> Signed-off-by: Michael Walle <michael@walle.cc> >>> --- >>> drivers/irqchip/Kconfig | 3 ++ >>> drivers/irqchip/Makefile | 1 + >>> drivers/irqchip/irq-sl28cpld.c | 92 ++++++++++++++++++++++++++++++++++ >>> drivers/mfd/Kconfig | 4 +- >>> 4 files changed, 98 insertions(+), 2 deletions(-) >>> create mode 100644 drivers/irqchip/irq-sl28cpld.c >>> >>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig >>> index 24fe08702ef7..3fd7415c8b55 100644 >>> --- a/drivers/irqchip/Kconfig >>> +++ b/drivers/irqchip/Kconfig >>> @@ -246,6 +246,9 @@ config RENESAS_RZA1_IRQC >>> Enable support for the Renesas RZ/A1 Interrupt Controller, to use up >>> to 8 external interrupts with configurable sense select. >>> >>> +config SL28CPLD_INTC >>> + bool >>> + >>> config ST_IRQCHIP >>> bool >>> select REGMAP >>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile >>> index eae0d78cbf22..0f4a37782609 100644 >>> --- a/drivers/irqchip/Makefile >>> +++ b/drivers/irqchip/Makefile >>> @@ -105,3 +105,4 @@ obj-$(CONFIG_MADERA_IRQ) += irq-madera.o >>> obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o >>> obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o >>> obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o >>> +obj-$(CONFIG_SL28CPLD_INTC) += irq-sl28cpld.o >>> diff --git a/drivers/irqchip/irq-sl28cpld.c b/drivers/irqchip/irq-sl28cpld.c >>> new file mode 100644 >>> index 000000000000..fa52ed79137b >>> --- /dev/null >>> +++ b/drivers/irqchip/irq-sl28cpld.c >>> @@ -0,0 +1,92 @@ >>> +// SPDX-License-Identifier: GPL-2.0-only >>> +/* >>> + * SMARC-sAL28 Interrupt core driver. >>> + * >>> + * Copyright 2019 Kontron Europe GmbH >>> + */ >>> + >>> +#include <linux/kernel.h> >>> +#include <linux/module.h> >>> +#include <linux/of.h> >>> +#include <linux/of_address.h> >>> +#include <linux/of_platform.h> >>> +#include <linux/i2c.h> >>> +#include <linux/regmap.h> >>> +#include <linux/interrupt.h> >>> +#include <linux/mfd/core.h> >>> + >>> +#define INTC_IE 0 >>> +#define INTC_IP 1 >>> + >>> +static const struct regmap_irq sl28cpld_irqs[] = { >>> + REGMAP_IRQ_REG_LINE(0, 8), >>> + REGMAP_IRQ_REG_LINE(1, 8), >>> + REGMAP_IRQ_REG_LINE(2, 8), >>> + REGMAP_IRQ_REG_LINE(3, 8), >>> + REGMAP_IRQ_REG_LINE(4, 8), >>> + REGMAP_IRQ_REG_LINE(5, 8), >>> + REGMAP_IRQ_REG_LINE(6, 8), >>> + REGMAP_IRQ_REG_LINE(7, 8), >>> +}; >>> + >>> +struct sl28cpld_intc { >>> + struct regmap *regmap; >>> + struct regmap_irq_chip chip; >>> + struct regmap_irq_chip_data *irq_data; >>> +}; >>> + >>> +static int sl28cpld_intc_probe(struct platform_device *pdev) >>> +{ >>> + struct sl28cpld_intc *irqchip; >>> + struct resource *res; >>> + unsigned int irq; >>> + int ret; >>> + >>> + irqchip = devm_kzalloc(&pdev->dev, sizeof(*irqchip), GFP_KERNEL); >>> + if (!irqchip) >>> + return -ENOMEM; >>> + >>> + if (!pdev->dev.parent) >>> + return -ENODEV; >>> + >>> + irqchip->regmap = dev_get_regmap(pdev->dev.parent, NULL); >>> + if (!irqchip->regmap) >>> + return -ENODEV; >>> + >>> + irq = platform_get_irq(pdev, 0); >>> + if (irq < 0) >>> + return irq; >>> + >>> + res = platform_get_resource(pdev, IORESOURCE_REG, 0); >>> + if (!res) >>> + return -EINVAL; >>> + >>> + irqchip->chip.name = "sl28cpld-intc"; >>> + irqchip->chip.irqs = sl28cpld_irqs; >>> + irqchip->chip.num_irqs = ARRAY_SIZE(sl28cpld_irqs); >>> + irqchip->chip.num_regs = 1; >>> + irqchip->chip.status_base = res->start + INTC_IP; >>> + irqchip->chip.mask_base = res->start + INTC_IE; >>> + irqchip->chip.mask_invert = true, >>> + irqchip->chip.ack_base = res->start + INTC_IP; >>> + >>> + ret = devm_regmap_add_irq_chip(&pdev->dev, irqchip->regmap, irq, >>> + IRQF_SHARED | IRQF_ONESHOT, 0, >>> + &irqchip->chip, &irqchip->irq_data); >>> + if (ret) >>> + return ret; >>> + dev_info(&pdev->dev, "registered IRQ %d\n", irq); >>> + >>> + return 0; >>> +} >>> + >>> +static struct platform_driver sl28cpld_intc_driver = { >>> + .probe = sl28cpld_intc_probe, >>> + .driver = { >>> + .name = "sl28cpld-intc", >>> + } >>> +}; >>> +module_platform_driver(sl28cpld_intc_driver); >>> + >>> +MODULE_DESCRIPTION("sl28cpld Interrupt Controller Driver"); >>> +MODULE_LICENSE("GPL"); >>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >>> index 01588c366476..4f741d640705 100644 >>> --- a/drivers/mfd/Kconfig >>> +++ b/drivers/mfd/Kconfig >>> @@ -2060,12 +2060,12 @@ config SGI_MFD_IOC3 >>> then say Y. Otherwise say N. >>> >>> config MFD_SL28CPLD >>> - tristate "Kontron sl28 core driver" >>> + bool "Kontron sl28 core driver" >> >> This is .... unusual. Why declare it tristate only to re-declare it >> bool in the next patch ? > > I though it was a good idea to have that gradually build up, esp. since > these patches might go through different reviewers/trees. That being said, > I'll change it though. > Gradually build up is fine, but that doesn't mean to do it one way first and change it later. This only results in feedback like mine - I2C=y just didn't make sense in the previous patch. Please keep in mind that doing things one way first and then changing them later only wastes people's time. It would be much better to mention in the previous patch that the symbol is declared bool because a subsequent patch introduces an interrupt controller driver which needs to be built-in. Thanks, Guenter >> It does explain the I2C=y, but I really think it should be bool from >> the start if it ends up >> there. > > Ok. > >> >>> depends on I2C=y >>> depends on OF >>> select REGMAP_I2C >>> select REGMAP_IRQ >>> - select SL28CPLD_IRQ >>> + select SL28CPLD_INTC >> >> What is the point of introducing SL28CPLD_IRQ in the first place ? > > oh damn. this is a left-over which slipped through. There should just > be a SL28CPLD_INTC. > > -michael > >> >>> select MFD_CORE >>> help >>> This option enables support for the board management controller >>>
next prev parent reply index Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-17 20:49 [PATCH 00/18] Add support for Kontron sl28cpld Michael Walle 2020-03-17 20:50 ` [PATCH 01/18] include/linux/ioport.h: add helper to define REG resource constructs Michael Walle 2020-03-17 20:50 ` [PATCH 02/18] mfd: mfd-core: Don't overwrite the dma_mask of the child device Michael Walle 2020-03-17 20:50 ` [PATCH 03/18] mfd: mfd-core: match device tree node against reg property Michael Walle 2020-03-17 20:50 ` [PATCH 04/18] dt-bindings: mfd: Add bindings for sl28cpld Michael Walle 2020-03-30 22:35 ` Rob Herring 2020-03-31 7:40 ` Michael Walle 2020-03-17 20:50 ` [PATCH 05/18] mfd: Add support for Kontron sl28cpld management controller Michael Walle 2020-03-18 3:28 ` Guenter Roeck 2020-03-18 16:38 ` Michael Walle 2020-03-17 20:50 ` [PATCH 06/18] irqchip: add sl28cpld interrupt controller support Michael Walle 2020-03-18 16:53 ` Guenter Roeck 2020-03-18 17:06 ` Michael Walle 2020-03-18 20:35 ` Guenter Roeck [this message] 2020-03-17 20:50 ` [PATCH 07/18] dt-bindings: watchdog: Add bindings for sl28cpld watchdog Michael Walle 2020-03-17 20:50 ` [PATCH 08/18] watchdog: add support " Michael Walle 2020-03-18 3:17 ` Guenter Roeck 2020-03-17 20:50 ` [PATCH 09/18] dt-bindings: pwm: Add bindings for sl28cpld PWM controller Michael Walle 2020-03-17 20:50 ` [PATCH 10/18] pwm: add support " Michael Walle 2020-03-17 20:50 ` [PATCH 11/18] dt-bindings: gpio: Add bindings for sl28cpld GPIO controller Michael Walle 2020-03-17 20:50 ` [PATCH 12/18] gpio: add support for the " Michael Walle 2020-03-18 9:14 ` Bartosz Golaszewski 2020-03-18 12:45 ` Michael Walle 2020-03-25 11:50 ` Bartosz Golaszewski 2020-03-26 20:05 ` Michael Walle 2020-03-27 10:20 ` Linus Walleij 2020-03-27 15:28 ` Michael Walle 2020-03-27 19:01 ` Linus Walleij 2020-03-30 11:21 ` Bartosz Golaszewski 2020-03-30 11:48 ` Michael Walle 2020-03-28 12:04 ` Michael Walle 2020-03-28 17:20 ` Michael Walle 2020-03-17 20:50 ` [PATCH 13/18] dt-bindings: hwmon: Add bindings for sl28cpld hardware monitoring Michael Walle 2020-03-17 20:50 ` [PATCH 14/18] hwmon: add support for the sl28cpld hardware monitoring controller Michael Walle 2020-03-18 3:27 ` Guenter Roeck 2020-03-18 16:32 ` Michael Walle 2020-03-17 20:50 ` [PATCH 15/18] arm64: dts: freescale: sl28: enable sl28cpld Michael Walle 2020-03-17 20:50 ` [PATCH 16/18] arm64: dts: freescale: sl28: map GPIOs to input events Michael Walle 2020-03-17 20:50 ` [PATCH 17/18] arm64: dts: freescale: sl28: enable LED support Michael Walle 2020-03-17 20:50 ` [PATCH 18/18] arm64: dts: freescale: sl28: enable fan support Michael Walle
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=d2ede8b3-75ab-36f3-bc7e-8f7169977fb7@roeck-us.net \ --to=linux@roeck-us.net \ --cc=bgolaszewski@baylibre.com \ --cc=devicetree@vger.kernel.org \ --cc=groeck7@gmail.com \ --cc=jason@lakedaemon.net \ --cc=jdelvare@suse.com \ --cc=lee.jones@linaro.org \ --cc=leoyang.li@nxp.com \ --cc=linus.walleij@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-gpio@vger.kernel.org \ --cc=linux-hwmon@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pwm@vger.kernel.org \ --cc=linux-watchdog@vger.kernel.org \ --cc=maz@kernel.org \ --cc=michael@walle.cc \ --cc=robh+dt@kernel.org \ --cc=shawnguo@kernel.org \ --cc=tglx@linutronix.de \ --cc=thierry.reding@gmail.com \ --cc=u.kleine-koenig@pengutronix.de \ --cc=wim@linux-watchdog.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Linux-Watchdog Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \ linux-watchdog@vger.kernel.org public-inbox-index linux-watchdog Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog AGPL code for this site: git clone https://public-inbox.org/public-inbox.git