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

[Ivan Vecera]

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