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

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

Hi Wolfram,

Thanks for the feedback.

> Subject: Re: [PATCH v5 01/11] i2c: Enhance i2c_new_ancillary_device API
> 
> Hi Biju,
> 
> > DT-Maintainers suggestion:
> > [1]
> > raa215300: pmic@12 {
> > 	compatible = "renesas,raa215300";
> > 	reg = <0x12>, <0x6f>;
> > 	reg-names = "main", "rtc";
> >
> > 	clocks = <&x2>;
> > 	clock-names = "xin";
> > 	/* Add Optional shared IRQ resource and share it to child and handle
> > it both in parent and child */ };
> 
> Would this binding allow to not use the RTC if the second reg is missing?
> What are the advantages of not enabling RTC? Saving power?

Some use case, just use PMIC for regulators, reset and STR(suspend to RAM) as it may have
RTC support in SoC. for eg: RZ/N1.

> 
> >
> > Laurent/Wolfram suggestion to split it into two nodes and get rid of
> this patch:
> > [2]
> > 	raa215300: pmic @12 {
> > 		compatible = "renesas,raa215300";
> > 		reg = <0x12>;
> >
> > 		/* Add Optional shared IRQ */
> > 		renesas,raa215300-rtc = <&rtc_raa215300>; /* Parse the handle
> and
> > Enable RTC , if present.*/
> 
> Thinking more about this: DT is hardware description, so the RTC should
> always be described in DT. If the RTC is actually activated is more a
> configuration thing, or? Brainstorming: maybe the PMIC driver could try to
> find the node with reg == 0x6f and see if firmware has enabled it or not?

In-built RTC is always present on the PMIC chips. 

But RTC is enabled or not disabled is based on system design. 

If X1N/XOUT both grounded means you cannot use RTC.

If XIN connected to external clock source means internal oscillator is disabled.

If XIN/XOUT connected to external crystal means internal oscillator is enabled.

> 
> > 	};
> >
> > 	rtc_raa215300: rtc@6f {
> > 		compatible = "renesas,raa215300-isl1208";
> > 		reg = <0x6f>;
> >
> > 		/* Add Optional shared IRQ */
> > 		clocks = <&x2>;
> > 		clock-names = "xin";
> > 		renesas,raa215300-pmic = <&pmic>; /* Parse the handle to get
> PMIC version to check Oscillator bit is inverted or not */
> > 	};
> 
> I have been scratching my head around this and wondered about one thing.
> The RTC driver needs to know if the oscillator bit is inverted. AFAIU this
> depends on the version of the PMIC (which includes the RTC). So, can't we
> simply encode the version in the compatible string?

PMIC is a component on the SoM module. So SoM's may have different PMIC versions.
I need to support all PMIC version with single image.

If we encode the version in the compatible string means, we need to detect PMIC version in bootloader and update the rtc compatible and merge with kernel device tree by bootloader.

Again, we need to define 1 extra compatible "renesas,raa215300-isl1208-a0"
in DT documentation for describing A0 chip that has inverted oscillator bit.

Cheers,
Biju

> 
> > 		compatible = "renesas,raa215300-isl1208-01";
> > 		compatible = "renesas,raa215300-isl1208-a0";
> 
> I dunno the exact versions, but you probably get the idea.
> 
> Happy hacking,
> 
>    Wolfram