linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthias Brugger <matthias.bgg@gmail.com>
To: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>,
	Lee Jones <lee.jones@linaro.org>,
	Rob Herring <robh+dt@kernel.org>, Mark Brown <broonie@kernel.org>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Eddie Huang <eddie.huang@mediatek.com>,
	Sean Wang <sean.wang@mediatek.com>,
	Alessandro Zummo <a.zummo@towertech.it>,
	Kate Stewart <kstewart@linuxfoundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Richard Fontana <rfontana@redhat.com>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-rtc@vger.kernel.org, srv_heupstream@mediatek.com
Subject: Re: [PATCH v5 02/10] mfd: mt6397: extract irq related code from core driver
Date: Fri, 23 Aug 2019 16:53:45 +0200	[thread overview]
Message-ID: <d32fed91-5d7a-a6fb-535b-fca9e311df42@gmail.com> (raw)
In-Reply-To: <1566531931-9772-3-git-send-email-hsin-hsiung.wang@mediatek.com>



On 23/08/2019 05:45, Hsin-Hsiung Wang wrote:
> In order to support different types of irq design, we decide to add
> separate irq drivers for different design and keep mt6397 mfd core
> simple and reusable to all generations of PMICs so far.
> 
> Acked-for-mfd-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
> ---
>  drivers/mfd/Makefile            |   3 +-
>  drivers/mfd/mt6397-core.c       | 146 --------------------------------
>  drivers/mfd/mt6397-irq.c        | 181 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/mt6397/core.h |   9 ++
>  4 files changed, 192 insertions(+), 147 deletions(-)
>  create mode 100644 drivers/mfd/mt6397-irq.c
> 
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index f026ada..9a96325 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -241,7 +241,8 @@ obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
>  obj-$(CONFIG_INTEL_SOC_PMIC_BXTWC)	+= intel_soc_pmic_bxtwc.o
>  obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC)	+= intel_soc_pmic_chtwc.o
>  obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI)	+= intel_soc_pmic_chtdc_ti.o
> -obj-$(CONFIG_MFD_MT6397)	+= mt6397-core.o
> +mt6397-objs	:= mt6397-core.o mt6397-irq.o
> +obj-$(CONFIG_MFD_MT6397)	+= mt6397.o
>  
>  obj-$(CONFIG_MFD_ALTERA_A10SR)	+= altera-a10sr.o
>  obj-$(CONFIG_MFD_ALTERA_SYSMGR) += altera-sysmgr.o
> diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> index c070862..93c8881 100644
> --- a/drivers/mfd/mt6397-core.c
> +++ b/drivers/mfd/mt6397-core.c
> @@ -18,10 +18,6 @@
>  #define MT6397_RTC_BASE		0xe000
>  #define MT6397_RTC_SIZE		0x3e
>  
> -#define MT6323_CHIP_ID		0x23
> -#define MT6391_CHIP_ID		0x91
> -#define MT6397_CHIP_ID		0x97
> -
>  static const struct resource mt6397_rtc_resources[] = {
>  	{
>  		.start = MT6397_RTC_BASE,
> @@ -86,148 +82,6 @@
>  	}
>  };
>  
> -static void mt6397_irq_lock(struct irq_data *data)
> -{
> -	struct mt6397_chip *mt6397 = irq_data_get_irq_chip_data(data);
> -
> -	mutex_lock(&mt6397->irqlock);
> -}
> -
> -static void mt6397_irq_sync_unlock(struct irq_data *data)
> -{
> -	struct mt6397_chip *mt6397 = irq_data_get_irq_chip_data(data);
> -
> -	regmap_write(mt6397->regmap, mt6397->int_con[0],
> -		     mt6397->irq_masks_cur[0]);
> -	regmap_write(mt6397->regmap, mt6397->int_con[1],
> -		     mt6397->irq_masks_cur[1]);
> -
> -	mutex_unlock(&mt6397->irqlock);
> -}
> -
> -static void mt6397_irq_disable(struct irq_data *data)
> -{
> -	struct mt6397_chip *mt6397 = irq_data_get_irq_chip_data(data);
> -	int shift = data->hwirq & 0xf;
> -	int reg = data->hwirq >> 4;
> -
> -	mt6397->irq_masks_cur[reg] &= ~BIT(shift);
> -}
> -
> -static void mt6397_irq_enable(struct irq_data *data)
> -{
> -	struct mt6397_chip *mt6397 = irq_data_get_irq_chip_data(data);
> -	int shift = data->hwirq & 0xf;
> -	int reg = data->hwirq >> 4;
> -
> -	mt6397->irq_masks_cur[reg] |= BIT(shift);
> -}
> -
> -#ifdef CONFIG_PM_SLEEP
> -static int mt6397_irq_set_wake(struct irq_data *irq_data, unsigned int on)
> -{
> -	struct mt6397_chip *mt6397 = irq_data_get_irq_chip_data(irq_data);
> -	int shift = irq_data->hwirq & 0xf;
> -	int reg = irq_data->hwirq >> 4;
> -
> -	if (on)
> -		mt6397->wake_mask[reg] |= BIT(shift);
> -	else
> -		mt6397->wake_mask[reg] &= ~BIT(shift);
> -
> -	return 0;
> -}
> -#else
> -#define mt6397_irq_set_wake NULL
> -#endif
> -
> -static struct irq_chip mt6397_irq_chip = {
> -	.name = "mt6397-irq",
> -	.irq_bus_lock = mt6397_irq_lock,
> -	.irq_bus_sync_unlock = mt6397_irq_sync_unlock,
> -	.irq_enable = mt6397_irq_enable,
> -	.irq_disable = mt6397_irq_disable,
> -	.irq_set_wake = mt6397_irq_set_wake,
> -};
> -
> -static void mt6397_irq_handle_reg(struct mt6397_chip *mt6397, int reg,
> -		int irqbase)
> -{
> -	unsigned int status;
> -	int i, irq, ret;
> -
> -	ret = regmap_read(mt6397->regmap, reg, &status);
> -	if (ret) {
> -		dev_err(mt6397->dev, "Failed to read irq status: %d\n", ret);
> -		return;
> -	}
> -
> -	for (i = 0; i < 16; i++) {
> -		if (status & BIT(i)) {
> -			irq = irq_find_mapping(mt6397->irq_domain, irqbase + i);
> -			if (irq)
> -				handle_nested_irq(irq);
> -		}
> -	}
> -
> -	regmap_write(mt6397->regmap, reg, status);
> -}
> -
> -static irqreturn_t mt6397_irq_thread(int irq, void *data)
> -{
> -	struct mt6397_chip *mt6397 = data;
> -
> -	mt6397_irq_handle_reg(mt6397, mt6397->int_status[0], 0);
> -	mt6397_irq_handle_reg(mt6397, mt6397->int_status[1], 16);
> -
> -	return IRQ_HANDLED;
> -}
> -
> -static int mt6397_irq_domain_map(struct irq_domain *d, unsigned int irq,
> -					irq_hw_number_t hw)
> -{
> -	struct mt6397_chip *mt6397 = d->host_data;
> -
> -	irq_set_chip_data(irq, mt6397);
> -	irq_set_chip_and_handler(irq, &mt6397_irq_chip, handle_level_irq);
> -	irq_set_nested_thread(irq, 1);
> -	irq_set_noprobe(irq);
> -
> -	return 0;
> -}
> -
> -static const struct irq_domain_ops mt6397_irq_domain_ops = {
> -	.map = mt6397_irq_domain_map,
> -};
> -
> -static int mt6397_irq_init(struct mt6397_chip *mt6397)
> -{
> -	int ret;
> -
> -	mutex_init(&mt6397->irqlock);
> -
> -	/* Mask all interrupt sources */
> -	regmap_write(mt6397->regmap, mt6397->int_con[0], 0x0);
> -	regmap_write(mt6397->regmap, mt6397->int_con[1], 0x0);
> -
> -	mt6397->irq_domain = irq_domain_add_linear(mt6397->dev->of_node,
> -		MT6397_IRQ_NR, &mt6397_irq_domain_ops, mt6397);
> -	if (!mt6397->irq_domain) {
> -		dev_err(mt6397->dev, "could not create irq domain\n");
> -		return -ENOMEM;
> -	}
> -
> -	ret = devm_request_threaded_irq(mt6397->dev, mt6397->irq, NULL,
> -		mt6397_irq_thread, IRQF_ONESHOT, "mt6397-pmic", mt6397);
> -	if (ret) {
> -		dev_err(mt6397->dev, "failed to register irq=%d; err: %d\n",
> -			mt6397->irq, ret);
> -		return ret;
> -	}
> -
> -	return 0;
> -}
> -
>  #ifdef CONFIG_PM_SLEEP
>  static int mt6397_irq_suspend(struct device *dev)
>  {
> diff --git a/drivers/mfd/mt6397-irq.c b/drivers/mfd/mt6397-irq.c
> new file mode 100644
> index 0000000..b2d3ce1
> --- /dev/null
> +++ b/drivers/mfd/mt6397-irq.c
> @@ -0,0 +1,181 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (c) 2019 MediaTek Inc.
> +
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/mt6323/core.h>
> +#include <linux/mfd/mt6323/registers.h>
> +#include <linux/mfd/mt6397/core.h>
> +#include <linux/mfd/mt6397/registers.h>
> +
> +static void mt6397_irq_lock(struct irq_data *data)
> +{
> +	struct mt6397_chip *mt6397 = irq_data_get_irq_chip_data(data);
> +
> +	mutex_lock(&mt6397->irqlock);
> +}
> +
> +static void mt6397_irq_sync_unlock(struct irq_data *data)
> +{
> +	struct mt6397_chip *mt6397 = irq_data_get_irq_chip_data(data);
> +
> +	regmap_write(mt6397->regmap, mt6397->int_con[0],
> +		     mt6397->irq_masks_cur[0]);
> +	regmap_write(mt6397->regmap, mt6397->int_con[1],
> +		     mt6397->irq_masks_cur[1]);
> +
> +	mutex_unlock(&mt6397->irqlock);
> +}
> +
> +static void mt6397_irq_disable(struct irq_data *data)
> +{
> +	struct mt6397_chip *mt6397 = irq_data_get_irq_chip_data(data);
> +	int shift = data->hwirq & 0xf;
> +	int reg = data->hwirq >> 4;
> +
> +	mt6397->irq_masks_cur[reg] &= ~BIT(shift);
> +}
> +
> +static void mt6397_irq_enable(struct irq_data *data)
> +{
> +	struct mt6397_chip *mt6397 = irq_data_get_irq_chip_data(data);
> +	int shift = data->hwirq & 0xf;
> +	int reg = data->hwirq >> 4;
> +
> +	mt6397->irq_masks_cur[reg] |= BIT(shift);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mt6397_irq_set_wake(struct irq_data *irq_data, unsigned int on)
> +{
> +	struct mt6397_chip *mt6397 = irq_data_get_irq_chip_data(irq_data);
> +	int shift = irq_data->hwirq & 0xf;
> +	int reg = irq_data->hwirq >> 4;
> +
> +	if (on)
> +		mt6397->wake_mask[reg] |= BIT(shift);
> +	else
> +		mt6397->wake_mask[reg] &= ~BIT(shift);
> +
> +	return 0;
> +}
> +#else
> +#define mt6397_irq_set_wake NULL
> +#endif
> +
> +static struct irq_chip mt6397_irq_chip = {
> +	.name = "mt6397-irq",
> +	.irq_bus_lock = mt6397_irq_lock,
> +	.irq_bus_sync_unlock = mt6397_irq_sync_unlock,
> +	.irq_enable = mt6397_irq_enable,
> +	.irq_disable = mt6397_irq_disable,
> +	.irq_set_wake = mt6397_irq_set_wake,
> +};
> +
> +static void mt6397_irq_handle_reg(struct mt6397_chip *mt6397, int reg,
> +				  int irqbase)
> +{
> +	unsigned int status;
> +	int i, irq, ret;
> +
> +	ret = regmap_read(mt6397->regmap, reg, &status);
> +	if (ret) {
> +		dev_err(mt6397->dev, "Failed to read irq status: %d\n", ret);
> +		return;
> +	}
> +
> +	for (i = 0; i < 16; i++) {
> +		if (status & BIT(i)) {
> +			irq = irq_find_mapping(mt6397->irq_domain, irqbase + i);
> +			if (irq)
> +				handle_nested_irq(irq);
> +		}
> +	}
> +
> +	regmap_write(mt6397->regmap, reg, status);
> +}
> +
> +static irqreturn_t mt6397_irq_thread(int irq, void *data)
> +{
> +	struct mt6397_chip *mt6397 = data;
> +
> +	mt6397_irq_handle_reg(mt6397, mt6397->int_status[0], 0);
> +	mt6397_irq_handle_reg(mt6397, mt6397->int_status[1], 16);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int mt6397_irq_domain_map(struct irq_domain *d, unsigned int irq,
> +				 irq_hw_number_t hw)
> +{
> +	struct mt6397_chip *mt6397 = d->host_data;
> +
> +	irq_set_chip_data(irq, mt6397);
> +	irq_set_chip_and_handler(irq, &mt6397_irq_chip, handle_level_irq);
> +	irq_set_nested_thread(irq, 1);
> +	irq_set_noprobe(irq);
> +
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops mt6397_irq_domain_ops = {
> +	.map = mt6397_irq_domain_map,
> +};
> +
> +int mt6397_irq_init(struct mt6397_chip *chip)
> +{
> +	int ret;
> +
> +	mutex_init(&chip->irqlock);
> +
> +	switch (chip->chip_id) {
> +	case MT6323_CHIP_ID:
> +		chip->int_con[0] = MT6323_INT_CON0;
> +		chip->int_con[1] = MT6323_INT_CON1;
> +		chip->int_status[0] = MT6323_INT_STATUS0;
> +		chip->int_status[1] = MT6323_INT_STATUS1;
> +		break;
> +
> +	case MT6391_CHIP_ID:
> +	case MT6397_CHIP_ID:
> +		chip->int_con[0] = MT6397_INT_CON0;
> +		chip->int_con[1] = MT6397_INT_CON1;
> +		chip->int_status[0] = MT6397_INT_STATUS0;
> +		chip->int_status[1] = MT6397_INT_STATUS1;
> +		break;
> +

Just stumbled over this in linux-next. I personally would prefer to have two
patches, one that moves the code and another one that adds the switch etc.

This way it would be much easier to realize this change.
Not sure if this is still possible, because it seems to be in one of Lee's
repositories already. (and if Lee thinks the same as I do, of course)

Regards,
Matthias


> +	default:
> +		dev_err(chip->dev, "unsupported chip: 0x%x\n", chip->chip_id);
> +		return -ENODEV;
> +	}
> +
> +	/* Mask all interrupt sources */
> +	regmap_write(chip->regmap, chip->int_con[0], 0x0);
> +	regmap_write(chip->regmap, chip->int_con[1], 0x0);
> +
> +	chip->irq_domain = irq_domain_add_linear(chip->dev->of_node,
> +						 MT6397_IRQ_NR,
> +						 &mt6397_irq_domain_ops,
> +						 chip);
> +	if (!chip->irq_domain) {
> +		dev_err(chip->dev, "could not create irq domain\n");
> +		return -ENOMEM;
> +	}
> +
> +	ret = devm_request_threaded_irq(chip->dev, chip->irq, NULL,
> +					mt6397_irq_thread, IRQF_ONESHOT,
> +					"mt6397-pmic", chip);
> +	if (ret) {
> +		dev_err(chip->dev, "failed to register irq=%d; err: %d\n",
> +			chip->irq, ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> diff --git a/include/linux/mfd/mt6397/core.h b/include/linux/mfd/mt6397/core.h
> index 25a95e7..9320c2a 100644
> --- a/include/linux/mfd/mt6397/core.h
> +++ b/include/linux/mfd/mt6397/core.h
> @@ -7,6 +7,12 @@
>  #ifndef __MFD_MT6397_CORE_H__
>  #define __MFD_MT6397_CORE_H__
>  
> +enum chip_id {
> +	MT6323_CHIP_ID = 0x23,
> +	MT6391_CHIP_ID = 0x91,
> +	MT6397_CHIP_ID = 0x97,
> +};
> +
>  enum mt6397_irq_numbers {
>  	MT6397_IRQ_SPKL_AB = 0,
>  	MT6397_IRQ_SPKR_AB,
> @@ -54,6 +60,9 @@ struct mt6397_chip {
>  	u16 irq_masks_cache[2];
>  	u16 int_con[2];
>  	u16 int_status[2];
> +	u16 chip_id;
>  };
>  
> +int mt6397_irq_init(struct mt6397_chip *chip);
> +
>  #endif /* __MFD_MT6397_CORE_H__ */
> 

  parent reply	other threads:[~2019-08-23 14:53 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-23  3:45 [PATCH v5 00/10] Add Support for MediaTek PMIC MT6358 Hsin-Hsiung Wang
2019-08-23  3:45 ` [PATCH v5 01/10] mfd: mt6397: clean up code Hsin-Hsiung Wang
2019-08-23  3:45 ` [PATCH v5 02/10] mfd: mt6397: extract irq related code from core driver Hsin-Hsiung Wang
2019-08-23 12:13   ` [BUG] " Frank Wunderlich
2019-08-23 14:56     ` Matthias Brugger
2019-08-23 15:26       ` Frank Wunderlich
2019-08-23 15:42         ` Matthias Brugger
2019-08-23 17:16           ` Aw: " Frank Wunderlich
2019-08-29  6:24             ` Hsin-hsiung Wang
2019-09-19 18:59               ` Frank Wunderlich
2019-09-30 19:44               ` Aw: " Frank Wunderlich
2019-08-23 15:35       ` Frank Wunderlich
2019-08-23 15:53       ` Frank Wunderlich
2019-08-23 14:53   ` Matthias Brugger [this message]
2019-08-23  3:45 ` [PATCH v5 03/10] mfd: mt6397: modify suspend/resume behavior Hsin-Hsiung Wang
2019-08-23 15:12   ` Matthias Brugger
2019-08-23  3:45 ` [PATCH v5 04/10] dt-bindings: mfd: Add compatible for the MediaTek MT6358 PMIC Hsin-Hsiung Wang
2019-08-23  3:45 ` [PATCH v5 05/10] regulator: Add document for MT6358 regulator Hsin-Hsiung Wang
2019-08-28 10:44   ` Mark Brown
2019-08-28 13:13   ` Applied "regulator: Add document for MT6358 regulator" to the regulator tree Mark Brown
2019-08-23  3:45 ` [PATCH v5 06/10] mfd: Add support for the MediaTek MT6358 PMIC Hsin-Hsiung Wang
2019-12-02  8:06   ` Pi-Hsun Shih
2019-12-06  8:55     ` Hsin-hsiung Wang
2019-08-23  3:45 ` [PATCH v5 07/10] regulator: mt6358: Add support for MT6358 regulator Hsin-Hsiung Wang
2019-08-28 10:45   ` Mark Brown
2019-08-29  6:09     ` Hsin-hsiung Wang
2019-08-28 13:13   ` Applied "regulator: mt6358: Add support for MT6358 regulator" to the regulator tree Mark Brown
2019-08-23  3:45 ` [PATCH v5 08/10] arm64: dts: mt6358: add PMIC MT6358 related nodes Hsin-Hsiung Wang
2019-08-23  3:45 ` [PATCH v5 09/10] rtc: mt6397: fix alarm register overwrite Hsin-Hsiung Wang
2019-08-23 15:35   ` Matthias Brugger
2019-08-29  5:34     ` Hsin-hsiung Wang
2019-08-23  3:45 ` [PATCH v5 10/10] rtc: mt6397: Add support for the MediaTek MT6358 RTC Hsin-Hsiung Wang

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=d32fed91-5d7a-a6fb-535b-fca9e311df42@gmail.com \
    --to=matthias.bgg@gmail.com \
    --cc=a.zummo@towertech.it \
    --cc=alexandre.belloni@bootlin.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=eddie.huang@mediatek.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hsin-hsiung.wang@mediatek.com \
    --cc=kstewart@linuxfoundation.org \
    --cc=lee.jones@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-rtc@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=rfontana@redhat.com \
    --cc=robh+dt@kernel.org \
    --cc=sean.wang@mediatek.com \
    --cc=srv_heupstream@mediatek.com \
    --cc=tglx@linutronix.de \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).