Re: [PATCH net-next v4 3/8] mfd: Add Microchip ZL3073x support

[Ivan Vecera <ivecera@redhat.com>]

On 24. 04. 25 6:34 odp., Andrew Lunn wrote:
>> +++ b/drivers/mfd/zl3073x-regs.h
>> @@ -0,0 +1,74 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#ifndef __ZL3073X_REGS_H
>> +#define __ZL3073X_REGS_H
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/bits.h>
>> +
>> +/*
>> + * Register address structure:
>> + * ===========================
>> + *  25        19 18   16 15     7 6           0
>> + * +-------------------------------------------+
>> + * | max_offset | width |  page  | page_offset |
>> + * +-------------------------------------------+
>> + *
>> + * page_offset ... <0x00..0x7F>
>> + * page .......... HW page number
>> + * size .......... register byte size (1, 2, 4 or 6)
>> + * max_offset .... maximal offset for indexed registers
>> + *                 (for non-indexed regs max_offset == page_offset)
>> + */
> 
> Something i missed earlier. This does not really describe
> hardware. The upper half is meta data about the register, which you
> encode into the register number.
> 
> How many other Linux drivers do you know about which does this?

This was proposed by Andy S.

Cite:
V4L2 (or media subsystem) solve the problem by providing a common
helpers for reading and writing tons of different registers in cameras.
See the commit 613cbb91e9ce ("media: Add MIPI CCI register access helper
functions").

They encode register address and size in register value. I have just
extend this approach to cover indexed registers. The max_offset is for
sanity during access to such registers, potential access out of
bounds is detected and error returned.

One can use just two simple functions for both register types:
zl3073x_read_reg(zldev, ZL_REG_NONIDX1, &value);
zl3073x_read_reg(zldev, ZL_REG_IDX1(idx), &value);

> 
> Also width vs size.

I'm sorry, just a typo during reworking.


Ivan