On 09/04/2025 16:42, Ivan Vecera wrote:
> Add several macros to access device registers. These macros
> defines a couple of static inline functions to ease an access
> device registers. There are two types of registers, the 1st type
> is a simple one that is defined by an address and size and the 2nd
> type is indexed register that is defined by base address, type,
> number of register instances and address stride between instances.
>
> Examples:
> __ZL3073X_REG_DEF(reg1, 0x1234, 4, u32);
> __ZL3073X_REG_IDX_DEF(idx_reg2, 0x1234, 2, u16, 4, 0x10);
Why can't you use standard FIELD_ macros? Why inventing the wheel again?
>
> this defines the following functions:
> int zl3073x_read_reg1(struct zl3073x_dev *dev, u32 *value);
> int zl3073x_write_reg1(struct zl3073x_dev *dev, u32 value);
> int zl3073x_read_idx_reg2(struct zl3073x_dev *dev, unsigned int idx,
> u32 *value);
> int zl3073x_write_idx_reg2(struct zl3073x_dev *dev, unsigned int idx,
> u32 value);
Do not copy code into commit msg. I asked about this last time. Explain
why do you need it, why existing API is not good.
>
> There are also several shortcut macros to define registers with
> certain bit widths: 8, 16, 32 and 48 bits.
>
> Signed-off-by: Ivan Vecera <ivec...@redhat.com>
> ---
...
> + *
> + * Note that these functions have to be called with the device lock
> + * taken.
> + */
> +#define __ZL3073X_REG_IDX_DEF(_name, _addr, _len, _type, _num, _stride)
> \
> +typedef _type zl3073x_##_name##_t; \
> +static inline __maybe_unused \
> +int zl3073x_read_##_name(struct zl3073x_dev *zldev, unsigned int idx,
> \
> + _type * value) \
> +{ \
> + WARN_ON(idx >= (_num)); \
No need to cause panic reboots. Either review your code so this does not
happen or properly handle.
Best regards,
Krzysztof
