Re: [PATCH 28/57] media: Add ovxxxx_16bit_addr_reg_helpers.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 10, 2023 at 12:19:30PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 2/10/23 11:53, Andy Shevchenko wrote:
> > On Fri, Feb 10, 2023 at 12:47:55PM +0200, Sakari Ailus wrote:
> >> On Fri, Feb 10, 2023 at 12:29:19PM +0200, Laurent Pinchart wrote:
> >>> On Fri, Feb 10, 2023 at 12:21:15PM +0200, Sakari Ailus wrote:
> >>>> On Thu, Feb 09, 2023 at 06:11:12PM +0200, Laurent Pinchart wrote:
> > 
> > ...
> > 
> >>>> I took a look at this some time ago, too, and current regmap API is a poor
> >>>> fit for CCI devices. CCI works on top of e.g. both I²C and I3C so something
> >>>> on top of regmap is a better approach indeed.
> >>>
> >>> I'm confused, is regmap a poor fit, or a better approach ?
> >>
> >> I'm proposing having something on top of regmap, but not changing regmap
> >> itself.
> > 
> > I don't understand why we can't change regmap? regmap has a facility called
> > regmap bus which we can provide specifically for these types of devices. What's
> > wrong to see it done?
> 
> It is fairly easy to layer the few 16 and 24 bit register accesses over
> a standard regmap with 16 bit reg-address and 8 bit reg-data width using
> regmap_bulk_write() to still do the write in e.g. a single i2c-transfer.

I think we could also use regmap_raw_write().

> So if we want regmap for underlying physical layer independence, e.g.
> spi / i2c / i3c. we can just use standard regmap with a 
> cci_write_reg helper on top.

Agreed. We can start experimenting with this, and if somebody has use
cases outside of the camera sensor drivers space, we could later move
those helpers to regmap.

> I think that would be the most KISS solution here. One thing to also keep
> in mind is the amount of work necessary to convert existing sensor drivers.
> Also keeping in mind that it is not just the in tree sensor drivers, but
> also all out of tree sensor drivers which I have seen use similar constructs.

If this was the only issue to handle when porting drivers to mainline
and upstreaming them, I'd be happy :-)

> Requiring drivers to have a list / array of structs of all used register
> addresses + specifying the width per register address is not going to scale
> very poorly wrt converting all the code out there and I'm afraid that
> letting regmap somehow deal with the register-width issue is going to
> require something like this.

Did you mean "not going to scale very well" ? I'm not sure to understand
what you mean here.

-- 
Regards,

Laurent Pinchart




[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux