[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aAEhb-6QV2m21pm2@smile.fi.intel.com>
Date: Thu, 17 Apr 2025 18:42:39 +0300
From: Andy Shevchenko <andy@...nel.org>
To: Ivan Vecera <ivecera@...hat.com>
Cc: Andrew Lunn <andrew@...n.ch>, netdev@...r.kernel.org,
Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
Jiri Pirko <jiri@...nulli.us>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Prathosh Satish <Prathosh.Satish@...rochip.com>,
Lee Jones <lee@...nel.org>, Kees Cook <kees@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Michal Schmidt <mschmidt@...hat.com>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v3 net-next 3/8] mfd: Add Microchip ZL3073x support
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 <andrew@...n.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
Powered by blists - more mailing lists