Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
References: <20220623211420.918875-1-aidanmacdonald.0x0@gmail.com>
 <20220623211420.918875-12-aidanmacdonald.0x0@gmail.com>
 <CGME20220701163330eucas1p13456e7757d9d2bc8d0aa35c16f143590@eucas1p1.samsung.com>
 <acaaf77f-3282-8544-dd3c-7915fc1a6a4f@samsung.com>
From:   Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
To:     Marek Szyprowski <m.szyprowski@samsung.com>
Cc:     broonie@kernel.org, gregkh@linuxfoundation.org, rafael@kernel.org,
        andy.shevchenko@gmail.com, mazziesaccount@gmail.com,
        linux-kernel@vger.kernel.org,
        Krzysztof Kozlowski <krzk@kernel.org>,
        'Linux Samsung SOC' <linux-samsung-soc@vger.kernel.org>
Subject: Re: [PATCH v2 11/12] regmap-irq: Add get_irq_reg() callback
In-reply-to: <acaaf77f-3282-8544-dd3c-7915fc1a6a4f@samsung.com>
Date:   Sun, 03 Jul 2022 12:06:58 +0100
Message-ID: <J6lhcwtTNjo9XCDWYcEz3QEWJBaJ34Qk@localhost>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk


Marek Szyprowski <m.szyprowski@samsung.com> writes:

> Hi All,
>
> On 23.06.2022 23:14, Aidan MacDonald wrote:
>> Replace the internal sub_irq_reg() function with a public callback
>> that drivers can use when they have more complex register layouts.
>> The default implementation is regmap_irq_get_irq_reg_linear(), used
>> if the chip doesn't provide its own callback.
>>
>> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
>> ---
>
> This patch landed in today's linux next-20220701 as commit bdf9b86cd3ad 
> ("regmap-irq: Add get_irq_reg() callback"). I've noticed that it causes 
> a regression on my test systems: the RTC alarm stopped working on all 
> boards with Samsung PMICs (drivers/mfd/sec*.c). There are no 
> warnings/oopses/etc. Waitng for the RTC alarm lasts forever, so it looks 
> that something is broken with interrupts. Reverting it on top of 
> linux-next fixes the issue.
>
> Unfortunately I'm going for a holidays for the whole next week and I'm 
> not able to analyze this issue further today. Krzysztof: maybe You will 
> be able to provide some more hints which regmap irq variant is broken?

Fortunately it's nothing complicated -- just a typo. Thanks for testing.

>>   drivers/base/regmap/regmap-irq.c | 126 ++++++++++++++++++++-----------
>>   include/linux/regmap.h           |  15 +++-
>>   2 files changed, 93 insertions(+), 48 deletions(-)
>> [..]
>> +unsigned int regmap_irq_get_irq_reg_linear(struct regmap_irq_chip_data *data,
>> +					   unsigned int base, int index)
>> +{
>> +	const struct regmap_irq_chip *chip = data->chip;
>> +	struct regmap *map = data->map;
>> +
>> +	/*
>> +	 * FIXME: This is for backward compatibility and should be removed
>> +	 * when not_fixed_stride is dropped (it's only used by qcom-pm8008).
>> +	 */
>> +	if (chip->not_fixed_stride && chip->sub_reg_offsets) {
>> +		struct regmap_irq_sub_irq_map *subreg;
>> +
>> +		subreg = &chip->sub_reg_offsets[0];
>> +		return base + subreg->offset[0];
>> +	}
>> +
>> +	return base + index * map->reg_stride * chip->irq_reg_stride;

chip->irq_reg_stride is usually 0. This should be data->irq_reg_stride,
which will be corrected to the default value of 1.

>> +}
>> +EXPORT_SYMBOL_GPL(regmap_irq_get_irq_reg_linear);
>> +
>>   /**
>>    * regmap_irq_set_type_config_simple() - Simple IRQ type configuration callback.
>>    * @buf: Buffer containing configuration register values, this is a 2D array of
>> @@ -862,6 +891,11 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
>
> Best regards