[PATCH v5 01/11] i2c: Enhance i2c_new_ancillary_device API

[Biju Das]

Hi Laurent,

Thanks for the feedback.

> Subject: Re: [PATCH v5 01/11] i2c: Enhance i2c_new_ancillary_device API
> 
> On Tue, Jun 13, 2023 at 07:31:46PM +0000, Biju Das wrote:
> > > Subject: RE: [PATCH v5 01/11] i2c: Enhance i2c_new_ancillary_device
> > > API
> > > > Subject: RE: [PATCH v5 01/11] i2c: Enhance
> > > > i2c_new_ancillary_device API
> > > > > Subject: Re: [PATCH v5 01/11] i2c: Enhance
> > > > > i2c_new_ancillary_device API
> > > > >
> > > > > Hi everyone,
> > > > >
> > > > > > Perhaps we should first think through what an ancillary device
> > > > > > really is.  My understanding is that it is used to talk to
> > > > > > secondary addresses of a multi-address I2C slave device.
> > > > >
> > > > > As I mentioned somewhere before, this is not the case. Ancillary
> > > > > devices are when one *driver* handles more than one address.
> > > > > Everything else has been handled differently in the past (for
> > > > > all the uses I am aware of).
> > > > >
> > > > > Yet, I have another idea which is so simple that I wonder if it
> > > > > maybe has already been discussed so far?
> > > > >
> > > > > * have two regs in the bindings
> > > >
> > > > OK, it is inline with DT maintainers expectation as it is matching
> > > > with real hw as single device node having two regs.
> > > >
> > > > > * use the second reg with i2c_new_client_device to instantiate the
> > > > >   RTC sibling. 'struct i2c_board_info', which is one parameter,
> should
> > > > >   have enough options to pass data, e.g it has a software_node.
> > > >
> > > > OK, I can see the below can be passed from PMIC to new client
> device.
> > > >
> > > > 	client->addr = info->addr;
> > > >
> > > > 	client->init_irq = info->irq;
> > > >
> > > > >
> > > > > Should work or did I miss something here?
> > > >
> > > > I guess it will work. We instantiate appropriate device based On
> > > > PMIC revision and slave address and IRQ resource passed through
> > > > 'struct i2c_board_info'
> > > >
> > > > Will check this and update you.
> > >
> > > info.irq = irq; -->Irq fine
> > > info.addr = addr; -->slave address fine size = strscpy(info.type,
> > > name, sizeof(info.type)); -->instantiation based on PMIC version
> > > fine.
> > >
> > > 1) How do we share clk details on instantiated device to find is it
> > > connected to external crystal or external clock source? as we cannot
> > > pass of_node between PMIC and "i2c_board_info" as it results in
> > > pinctrl failure. info->platformdata and
> > > Client->dev.platformdata to retrieve this info??
> >
> > Or
> >
> > I2C instantiation based on actual oscillator bit value, ie, two
> > i2c_device_id's with one for setting oscillator bit and another for
> > clearing oscillator bit
> >
> > PMIC driver parses the clock details. Based on firmware version and
> > clock, It instantiates either i2c_device_id with setting oscillator
> > bit or clearing oscillator bit.
> 
> I don't like that hack. I still think that two DT nodes is the best
> option, I think you're trying hard to hack around a problem that is
> actually not a problem.

Why do you think it is a hack? I believe rather it is actual solution

PMIC is a single device, with 2 regs, clocks, pinctrl and IRQ properties.
So it will be represented as single node with single compatible.

By instating a client device, we are sharing the relevant resources to RTC device driver.

Cheers,
Biju