On Wed, Aug 28, 2024 at 08:32:29AM -0500, Nishanth Menon wrote:

On 13:57-20240828, Mark Brown wrote:

quoted

On Wed, Aug 28, 2024 at 07:10:08AM -0500, Nishanth Menon wrote:

quoted

quoted

Fixes: 0ec74ad3c157 ("regmap: rework ->max_register handling")

quoted

In what sense is this a fix?

The max_register was 0x0 was clearly a corner case. The fix done for
remap  should have cleaned up the users of max_register to maintain the
behavior. That is just my opinion.

No, apart from the fact that catching all possible regmap users would be
rather hard it's entirely optional for regmaps to specify a maxium
register.

quoted

really does not seem like a good idea - unless you've done an audit of
every single syscon to make sure they do explicitly specify a maximum
register, and confirmed that this can't be specified via DT, then it's
going to break things.

I understand the risk - but having a consistent max_register definition
is important - key here is that in regmap, max_register is valid if:
a) max_register not being 0
b) if max_register is 0, it is valid only if max_register_is_0 is set to
true.

When syscon sets the max_register, it operates correctly for num_reg > 1
however, when reg_size == 1, you don't get the checks that you
get when num_regs > 1. That is inconsistent behavior.

It might help if you can clarify why you think an inconsistent behavior
is correct for syscon?

Like I say specifying a maximum register is entirely optional, not
everyone wants that feature and if you don't use the debugfs interface
or the flat cache it doesn't *super* matter.  With 0 as default it's
always going to be awkward to describe a maximum register of 0 while
allowing that to be optional, fortunately very few devices have a single
register.