linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Guru Das Srinagesh <gurus@codeaurora.org>
To: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
Cc: "Markus.Elfring@web.de" <Markus.Elfring@web.de>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"lee.jones@linaro.org" <lee.jones@linaro.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"linux@roeck-us.net" <linux@roeck-us.net>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"subbaram@codeaurora.org" <subbaram@codeaurora.org>,
	"collinsd@codeaurora.org" <collinsd@codeaurora.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"joe@perches.com" <joe@perches.com>,
	"bjorn.andersson@linaro.org" <bjorn.andersson@linaro.org>,
	"aghayal@codeaurora.org" <aghayal@codeaurora.org>,
	"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>
Subject: Re: [RFC PATCH v3 1/3] regmap-irq: Extend sub-irq to support non-fixed reg strides
Date: Mon, 12 Apr 2021 11:08:05 -0700	[thread overview]
Message-ID: <20210412180805.GA18661@codeaurora.org> (raw)
In-Reply-To: <d78cefad64d528e7c894c153950e4b4d2a18b300.camel@fi.rohmeurope.com>

Hi Matti,

On Mon, Apr 12, 2021 at 11:08:49AM +0000, Vaittinen, Matti wrote:
> Hi,
> 
> On Mon, 2021-04-12 at 09:05 +0300, Matti Vaittinen wrote:
> > Hi All,
> > 
> > On Wed, 2021-03-10 at 16:39 -0800, Guru Das Srinagesh wrote:
> > > Qualcomm's MFD chips have a top level interrupt status register and
> > > sub-irqs (peripherals).  When a bit in the main status register
> > > goes
> > > high, it means that the peripheral corresponding to that bit has an
> > > unserviced interrupt. If the bit is not set, this means that the
> > > corresponding peripheral does not.
> > > 
> > > Commit a2d21848d9211d ("regmap: regmap-irq: Add main status
> > > register
> > > support") introduced the sub-irq logic that is currently applied
> > > only
> > > when reading status registers, but not for any other functions like
> > > acking
> > > or masking. Extend the use of sub-irq to all other functions, with
> > > two
> > > caveats regarding the specification of offsets:
> > > 
> > > - Each member of the sub_reg_offsets array should be of length 1
> > > - The specified offsets should be the unequal strides for each sub-
> > > irq
> > >   device.
> > > 
> > > In QCOM's case, all the *_base registers are to be configured to
> > > the
> > > base addresses of the first sub-irq group, with offsets of each
> > > subsequent group calculated as a difference from these addresses.
> > > 
> > > Continuing from the example mentioned in the cover letter:
> > > 
> > > 	/*
> > > 	 * Address of MISC_INT_MASK		= 0x1011
> > > 	 * Address of TEMP_ALARM_INT_MASK	= 0x2011
> > > 	 * Address of GPIO01_INT_MASK		= 0x3011
> > > 	 *
> > > 	 * Calculate offsets as:
> > > 	 * offset_0 = 0x1011 - 0x1011 = 0       (to access MISC's
> > > 	 * 					 registers)
> > > 	 * offset_1 = 0x2011 - 0x1011 = 0x1000
> > > 	 * offset_2 = 0x3011 - 0x1011 = 0x2000
> > > 	 */
> > > 
> > > 	static unsigned int sub_unit0_offsets[] = {0};
> > > 	static unsigned int sub_unit1_offsets[] = {0x1000};
> > > 	static unsigned int sub_unit2_offsets[] = {0x2000};
> > > 
> > > 	static struct regmap_irq_sub_irq_map chip_sub_irq_offsets[] = {
> > > 		REGMAP_IRQ_MAIN_REG_OFFSET(sub_unit0_offsets),
> > > 		REGMAP_IRQ_MAIN_REG_OFFSET(sub_unit0_offsets),
> > > 		REGMAP_IRQ_MAIN_REG_OFFSET(sub_unit0_offsets),
> > > 	};
> > > 
> > > 	static struct regmap_irq_chip chip_irq_chip = {
> > > 	--------8<--------
> > > 	.not_fixed_stride = true,
> > > 	.mask_base	  = MISC_INT_MASK,
> > > 	.type_base	  = MISC_INT_TYPE,
> > > 	.ack_base	  = MISC_INT_ACK,
> > > 	.sub_reg_offsets  = chip_sub_irq_offsets,
> > > 	--------8<--------
> > > 	};
> > > 
> > > Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
> > > ---
> > >  drivers/base/regmap/regmap-irq.c | 81 ++++++++++++++++++++++++++--
> > > ------------
> > >  include/linux/regmap.h           |  7 ++++
> > >  2 files changed, 60 insertions(+), 28 deletions(-)
> > > 
> > > diff --git a/drivers/base/regmap/regmap-irq.c
> > > b/drivers/base/regmap/regmap-irq.c
> > > index 19db764..e1d8fc9e 100644
> > > --- a/drivers/base/regmap/regmap-irq.c
> > > +++ b/drivers/base/regmap/regmap-irq.c
> > > @@ -45,6 +45,27 @@ struct regmap_irq_chip_data {
> > >  	bool clear_status:1;
> > >  };
> > > 
> > 
> > Sorry that I am late with the "review" but I only now noticed this
> > change when I was following the references from PM8008 PMIC patch
> > mail.
> > 
> > 
> > >  
> > > +static int sub_irq_reg(struct regmap_irq_chip_data *data,
> > > +		       unsigned int base_reg, int i)
> > 
> > Do I read this correctly - this function should map the main status
> > bit
> > (given in i) to the (sub)IRQ register, right? How does this work for
> > cases where one bit corresponds to more than one sub-register? Or do
> > I
> > misunderstand the purpose of this function? (This is the case with
> > both
> > the BD70528 and BD71828).
> 
> I did some quick test with BD71815 which I had at home. And it seems to
> be I did indeed misunderstand this :) This is not for converting the
> main-IRQ bits to sub-irq registers - this is for getting the sub-IRQ
> register address based on the 'sub IRQ register index'.

Yes, that's right. With this change, the sub-irq concept which was
initially introduced to map the main-irq bits to sub-irq registers is
being extended and repurposed to cover the specific memory layout
described in the commit message and cover letter. 

I've updated the comment block in the header file for `sub_reg_offsets`
to make this clarification as well. 

The sub_irq_reg() function will not break existing functionality because
the crux of it will get executed only if not_fixed_stride is set.

> 
> So I do apologize the noise, it seems all is well and everything
> (except my self confidence) keeps working as it did :)
> 
> Thanks for the improvement Guru Das!

Thanks for testing this out and providing confirmation, Matti :)

Thank you.

Guru Das.

  reply	other threads:[~2021-04-12 18:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-11  0:39 [RFC PATCH v3 0/3] Add support for Qualcomm MFD PMIC register layout Guru Das Srinagesh
2021-03-11  0:39 ` [RFC PATCH v3 1/3] regmap-irq: Extend sub-irq to support non-fixed reg strides Guru Das Srinagesh
2021-04-12  6:05   ` Matti Vaittinen
2021-04-12 11:08     ` Vaittinen, Matti
2021-04-12 18:08       ` Guru Das Srinagesh [this message]
2021-03-11  0:39 ` [RFC PATCH v3 2/3] regmap-irq: Add support for POLARITY_HI and POLARITY_LO config regs Guru Das Srinagesh
2021-03-12 12:19   ` Mark Brown
2021-03-15 20:33     ` Guru Das Srinagesh
2021-03-15 20:36       ` Guru Das Srinagesh
2021-03-17 20:42       ` Mark Brown
2021-03-20  2:37         ` Guru Das Srinagesh
2021-03-11  0:39 ` [RFC PATCH v3 3/3] regmap-irq: Modify type_buf handling for IRQ_TYPE_LEVEL_* Guru Das Srinagesh
2021-03-18 18:33 ` (subset) [RFC PATCH v3 0/3] Add support for Qualcomm MFD PMIC register layout Mark Brown
2021-03-18 18:36   ` Mark Brown

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=20210412180805.GA18661@codeaurora.org \
    --to=gurus@codeaurora.org \
    --cc=Markus.Elfring@web.de \
    --cc=Matti.Vaittinen@fi.rohmeurope.com \
    --cc=aghayal@codeaurora.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=broonie@kernel.org \
    --cc=collinsd@codeaurora.org \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=lee.jones@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=robh+dt@kernel.org \
    --cc=subbaram@codeaurora.org \
    --subject='Re: [RFC PATCH v3 1/3] regmap-irq: Extend sub-irq to support non-fixed reg strides' \
    /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

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).