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

[Biju Das <biju.das.jz@bp.renesas.com>]

Hi Laurent,

Thanks for the feedback.

> Subject: Re: [PATCH v5 01/11] i2c: Enhance i2c_new_ancillary_device API
> 
> Hi Biju,
> 
> On Thu, Jun 08, 2023 at 11:00:19AM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH v5 01/11] i2c: Enhance i2c_new_ancillary_device
> > > API On Thu, Jun 08, 2023 at 06:41:35AM +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
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > sorry for not being able to chime in earlier.
> > > > > >
> > > > > > > In Biju's particular use case, the i2c device responds to
> > > > > > > two addresses, which is the standard i2c ancillary use case.
> > > > > > > However, what's special
> > > > > >
> > > > > > Not quite. ancillary is used when a *driver* needs to take
> > > > > > care of two addresses. We already have devices bundling two
> > > > > > features into the same chip. I recall at least RTC + EEPROM
> > > > > > somewhere. And so far, we have been handling this by creating
> two nodes in DT and have proper binding docs.
> > > > > > I think this is cleaner. First, you can see in DT already what
> > > > > > the compound device really consists of. In this case, which
> > > > > > RTC and RTC driver is exactly needed. Second, the code added
> > > > > > here adds complexity to the I2C core with another layer of
> inderection for dummy devices.
> > > > >
> > > > > FYI, please see [1] and [2]
> > > > >
> > > > > As per DT maintainers, most of PMICs are described with one
> > > > > node, even though RTC is on separate address. According to them
> > > > > the DT schema allows multiple addresses for children.
> > > > > But currently we lacks implementation for that. The enhancement
> > > > > to this API allows that.
> > > > >
> > > > > > > As some resources are shared (knowledge about the clocks),
> > > > > > > splitting this in two distinct devices in DT (which is what
> > > > > > > Biju's initial patch series did) would need phandles to link
> both nodes together.
> > > > > > >
> > > > > > > Do you have a better idea how to represent this?
> > > > > >
> > > > > > Not sure if I understood this chip correctly, but maybe: The
> > > > > > PMIC driver exposes a clock gate which can be consumed by the
> RTC driver?
> > > >
> > > > Let me give me some details of this PMIC chip.
> > > >
> > > > PMIC device has 2 addresses "0x12:- PMIC" , "0x6f"- rtc.
> > > >
> > > > It has XIN, XOUT, INT# pins and a register for firmware revisions.
> > >
> > > Is the firmware revision register accessed through address 0x12
> > > (PMIC) or 0x6f (RTC) ?
> >
> > 0x12(PMIC).
> >
> > > > Based on the system design,
> > > >
> > > > If XIN and XOUT is connected to external crystal, Internal
> > > > oscillator is enabled for RTC. In this case we need to set the
> > > > oscillator bit to "0".
> > > >
> > > > If XIN is connected to external clock source, Internal oscillator
> > > > is disabled for RTC. In this case we need to set the oscillator
> > > > bit to "1".
> > >
> > > Same here, which address is the oscillator bit accessed through ?
> >
> > RTC (0x6F)--> to set oscillator bit.
> 
> And does the PMIC part depend on the oscillator bit being set correctly,
> or is that used for the RTC only ?

PMIC part does not. It is used only in RTC. 

Based on PMIC revision, we need to set the oscillator bit in RTC block
for PMIC rev a0 and rest of the PMIC chips.

On PMIC rev0, oscillator bit is inverted.

Cheers,
Biju
> 
> > > > If XIN and XOUT not connected RTC operation not possible.
> > > >
> > > > IRQ# (optional) functionality is shared between PMIC and RTC.
> > > > (PMIC fault for various bucks/LDOs/WDT/OTP/NVM and alarm
> condition).
> > >
> > > IRQs can be shared between multiple devices so this shouldn't be a
> > > problem.
> >
> > OK. How do we represent this IRQ in DT?
> 
> You can simply reference the same IRQ from the interrupts property of
> different DT nodes.
> 
> > > > The board, I have doesn't populate IRQ# pin. If needed some
> > > > customers can populate IRQ# pin and use it for PMIC fault and RTC
> alarm.
> > > >
> > > > Also, currently my board has PMIC rev a0 where oscillator bit is
> > > > inverted and internal oscillator is enabled (ie: XIN and XOUT is
> > > > connected to external crystal)
> 
> --
> Regards,
> 
> Laurent Pinchart