On Thu, Apr 17, 2025 at 05:12:35PM +0200, Ivan Vecera wrote:
> On 17. 04. 25 4:50 odp., Ivan Vecera wrote:
> > On 17. 04. 25 3:13 odp., Andrew Lunn wrote:
> > > On Wed, Apr 16, 2025 at 08:19:25PM +0200, Ivan Vecera wrote:
> > > > On Wed, Apr 16, 2025 at 7:11 PM Andrew Lunn <and...@lunn.ch> wrote:
...
> > > Anyway, look around. How many other MFD, well actually, any sort of
> > > driver at all, have a bunch of low level helpers as inline functions
> > > in a header? You are aiming to write a plain boring driver which looks
> > > like every other driver in Linux....
> >
> > Well, I took inline functions approach as this is safer than macro usage
> > and each register have own very simple implementation with type and
> > range control (in case of indexed registers).
> >
> > It is safer to use:
> > zl3073x_read_ref_config(..., &v);
> > ...
> > zl3073x_read_ref_config(..., &v);
> >
> > than:
> > zl3073x_read_reg8(..., ZL_REG_REF_CONFIG, &v);
> > ...
> > zl3073x_read_reg16(..., ZL_REG_REF_CONFIG, &v); /* wrong */
> >
> > With inline function defined for each register the mistake in the
> > example cannot happen and also compiler checks that 'v' has correct
> > type.
> >
> > > Think about your layering. What does the MFD need to offer to sub
> > > drivers so they can work? For lower registers, maybe just
> > > zl3073x_read_u8(), zl3073x_read_u16() & zl3073x_read_read_u32(). Write
> > > variants as well. Plus the API needed to safely access the mailbox.
> > > Export these using one of the EXPORT_SYMBOL_GPL() variants, so the sub
> > > drivers can access them. The #defines for the registers numbers can be
> > > placed into a shared header file.
> >
> > Would it be acceptable for you something like this:
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").
Dunno if it helps here, though.
--
With Best Regards,
Andy Shevchenko
