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

[Wolfram Sang <wsa@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1885 bytes --]

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?

> 
> 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?

> 	};
> 
> 	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?

> 		compatible = "renesas,raa215300-isl1208-01";
> 		compatible = "renesas,raa215300-isl1208-a0";

I dunno the exact versions, but you probably get the idea.

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]