| Message ID | 20231026081514.3610343-2-Delphine_CC_Chiu@Wiwynn.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | LTC4286 and LTC4287 driver support | expand |
Hey, On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote: > Add a device tree bindings for ltc4286 driver. Bindings are for devices, not for drivers. > > Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > > Changelog: > v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6 > - Add type for adi,vrange-select-25p6 > - Revise rsense-micro-ohms to shunt-resistor-micro-ohms > --- > .../bindings/hwmon/lltc,ltc4286.yaml | 50 +++++++++++++++++++ > MAINTAINERS | 10 ++++ > 2 files changed, 60 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > new file mode 100644 > index 000000000000..17022de657bb > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > @@ -0,0 +1,50 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: LTC4286 power monitors > + > +maintainers: > + - Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > + > +properties: > + compatible: > + enum: > + - lltc,ltc4286 > + - lltc,ltc4287 I don't recall seeing an answer to Guenter about this ltc4287 device: https://lore.kernel.org/all/22f6364c-611c-ffb6-451c-9ddc20418d0a@roeck-us.net/ > + > + reg: > + maxItems: 1 > + > + adi,vrange-select-25p6: > + description: > + This property is a bool parameter to represent the > + voltage range is 25.6 or not for this chip. 25.6 what? Volts? microvolts? What about Guenter's suggestion to name this so that it better matches the other, similar properties? Cheers, Conor.
On 10/26/23 07:25, Conor Dooley wrote: > Hey, > > On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote: >> Add a device tree bindings for ltc4286 driver. > > Bindings are for devices, not for drivers. > >> >> Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> >> >> Changelog: >> v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6 >> - Add type for adi,vrange-select-25p6 >> - Revise rsense-micro-ohms to shunt-resistor-micro-ohms >> --- >> .../bindings/hwmon/lltc,ltc4286.yaml | 50 +++++++++++++++++++ >> MAINTAINERS | 10 ++++ >> 2 files changed, 60 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml >> >> diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml >> new file mode 100644 >> index 000000000000..17022de657bb >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml >> @@ -0,0 +1,50 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: LTC4286 power monitors >> + >> +maintainers: >> + - Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> >> + >> +properties: >> + compatible: >> + enum: >> + - lltc,ltc4286 >> + - lltc,ltc4287 > > I don't recall seeing an answer to Guenter about this ltc4287 device: > https://lore.kernel.org/all/22f6364c-611c-ffb6-451c-9ddc20418d0a@roeck-us.net/ > At least the chip does officially exist now, and a datasheet is available. https://www.analog.com/en/products/ltc4287.html It shows that coefficients for the telemetry commands are different, meaning that indeed both chips need to be explicitly referenced in the properties description (and handled in the driver, which proves my point of needing a datasheet before accepting such a driver). >> + >> + reg: >> + maxItems: 1 >> + >> + adi,vrange-select-25p6: >> + description: >> + This property is a bool parameter to represent the >> + voltage range is 25.6 or not for this chip. > > 25.6 what? Volts? microvolts? > What about Guenter's suggestion to name this so that it better matches > the other, similar properties? > I still would prefer one of the more common properties. I still prefer adi,vrange-high-enable. Guenter
On Thu, Oct 26, 2023 at 08:09:52AM -0700, Guenter Roeck wrote: > On 10/26/23 07:25, Conor Dooley wrote: > > Hey, > > > > On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote: > > > Add a device tree bindings for ltc4286 driver. > > > > Bindings are for devices, not for drivers. > > > > > > > > Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > > > > > > Changelog: > > > v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6 > > > - Add type for adi,vrange-select-25p6 > > > - Revise rsense-micro-ohms to shunt-resistor-micro-ohms > > > --- > > > .../bindings/hwmon/lltc,ltc4286.yaml | 50 +++++++++++++++++++ > > > MAINTAINERS | 10 ++++ > > > 2 files changed, 60 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > > > new file mode 100644 > > > index 000000000000..17022de657bb > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > > > @@ -0,0 +1,50 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: LTC4286 power monitors > > > + > > > +maintainers: > > > + - Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > > > + > > > +properties: > > > + compatible: > > > + enum: > > > + - lltc,ltc4286 > > > + - lltc,ltc4287 > > > > I don't recall seeing an answer to Guenter about this ltc4287 device: > > https://lore.kernel.org/all/22f6364c-611c-ffb6-451c-9ddc20418d0a@roeck-us.net/ > > > > At least the chip does officially exist now, and a datasheet is available. > > https://www.analog.com/en/products/ltc4287.html > > It shows that coefficients for the telemetry commands are different, > meaning that indeed both chips need to be explicitly referenced > in the properties description (and handled in the driver, which proves > my point of needing a datasheet before accepting such a driver). Oh neat, would've been good if that'd been mentioned! > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + adi,vrange-select-25p6: > > > + description: > > > + This property is a bool parameter to represent the > > > + voltage range is 25.6 or not for this chip. > > > > 25.6 what? Volts? microvolts? > > What about Guenter's suggestion to name this so that it better matches > > the other, similar properties? > > > > I still would prefer one of the more common properties. > I still prefer adi,vrange-high-enable. I think, from reading the previous version, that this is actually the lower voltage range, as there is a 102.x $unit range too that is the default in the hardware. Obviously, the use of the property could be inverted to match that naming however. Cheers, Conor.
On 10/26/23 08:26, Conor Dooley wrote: > On Thu, Oct 26, 2023 at 08:09:52AM -0700, Guenter Roeck wrote: >> On 10/26/23 07:25, Conor Dooley wrote: >>> Hey, >>> >>> On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote: >>>> Add a device tree bindings for ltc4286 driver. >>> >>> Bindings are for devices, not for drivers. >>> >>>> >>>> Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> >>>> >>>> Changelog: >>>> v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6 >>>> - Add type for adi,vrange-select-25p6 >>>> - Revise rsense-micro-ohms to shunt-resistor-micro-ohms >>>> --- >>>> .../bindings/hwmon/lltc,ltc4286.yaml | 50 +++++++++++++++++++ >>>> MAINTAINERS | 10 ++++ >>>> 2 files changed, 60 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml >>>> new file mode 100644 >>>> index 000000000000..17022de657bb >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml >>>> @@ -0,0 +1,50 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: LTC4286 power monitors >>>> + >>>> +maintainers: >>>> + - Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> >>>> + >>>> +properties: >>>> + compatible: >>>> + enum: >>>> + - lltc,ltc4286 >>>> + - lltc,ltc4287 >>> >>> I don't recall seeing an answer to Guenter about this ltc4287 device: >>> https://lore.kernel.org/all/22f6364c-611c-ffb6-451c-9ddc20418d0a@roeck-us.net/ >>> >> >> At least the chip does officially exist now, and a datasheet is available. >> >> https://www.analog.com/en/products/ltc4287.html >> >> It shows that coefficients for the telemetry commands are different, >> meaning that indeed both chips need to be explicitly referenced >> in the properties description (and handled in the driver, which proves >> my point of needing a datasheet before accepting such a driver). > > Oh neat, would've been good if that'd been mentioned! > Actually, turns out there is some contradiction in the LTC4286 datasheet. It mentions different coefficients in different places. It is all but impossible to determine if the datasheet is wrong or if the chip uses a variety of coefficients unless one has a real chip available. Sigh :-(. >>>> + >>>> + reg: >>>> + maxItems: 1 >>>> + >>>> + adi,vrange-select-25p6: >>>> + description: >>>> + This property is a bool parameter to represent the >>>> + voltage range is 25.6 or not for this chip. >>> >>> 25.6 what? Volts? microvolts? >>> What about Guenter's suggestion to name this so that it better matches >>> the other, similar properties? >>> >> >> I still would prefer one of the more common properties. >> I still prefer adi,vrange-high-enable. > > I think, from reading the previous version, that this is actually the > lower voltage range, as there is a 102.x $unit range too that is the > default in the hardware. Obviously, the use of the property could be > inverted to match that naming however. > It needs to be programmed either way because it is unknown how the chip has been programmed before. That means some action is needed no matter if the property is provided or not. Sure, we could add something like adi,vrange-low-enable. Not sure if that would be any better. Actually, after looking at the datasheets, I'd be more interested in the impact of other configuration buts such as VPWR_SELECT which selects the voltage used for power calculations, or all the settings in MFR_ADC_CONFIG. The datasheet doesn't really explain (or I can't figure out how it does) how the different configurations affect the reported telemetry values. But that is a question for the driver, not for the property description. Guenter
Hi Delphine, kernel test robot noticed the following build warnings: [auto build test WARNING on groeck-staging/hwmon-next] [also build test WARNING on linus/master v6.6-rc7 next-20231026] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Delphine-CC-Chiu/dt-bindings-hwmon-Add-lltc-ltc4286-driver-bindings/20231026-161739 base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next patch link: https://lore.kernel.org/r/20231026081514.3610343-2-Delphine_CC_Chiu%40Wiwynn.com patch subject: [PATCH v2 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver bindings reproduce: (https://download.01.org/0day-ci/archive/20231027/202310271540.4uI1Fgxe-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202310271540.4uI1Fgxe-lkp@intel.com/ All warnings (new ones prefixed by >>): >> Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/hwmon/ltc4286.rst >> MAINTAINERS:27681: WARNING: unknown document: ../devicetree/bindings/hwmon/ltc4286
> -----Original Message----- > From: Conor Dooley <conor@kernel.org> > Sent: Thursday, October 26, 2023 10:26 PM > To: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@wiwynn.com> > Cc: patrick@stwcx.xyz; Jean Delvare <jdelvare@suse.com>; Guenter Roeck > <linux@roeck-us.net>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > Jonathan Corbet <corbet@lwn.net>; linux-i2c@vger.kernel.org; > linux-hwmon@vger.kernel.org; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org > Subject: Re: [PATCH v2 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver > bindings > > Hey, > > On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote: > > Add a device tree bindings for ltc4286 driver. > > Bindings are for devices, not for drivers. > > > > > Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > > > > Changelog: > > v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6 > > - Add type for adi,vrange-select-25p6 > > - Revise rsense-micro-ohms to shunt-resistor-micro-ohms > > --- > > .../bindings/hwmon/lltc,ltc4286.yaml | 50 > +++++++++++++++++++ > > MAINTAINERS | 10 ++++ > > 2 files changed, 60 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > > > > diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > > b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > > new file mode 100644 > > index 000000000000..17022de657bb > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > > @@ -0,0 +1,50 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: LTC4286 power monitors > > + > > +maintainers: > > + - Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > > + > > +properties: > > + compatible: > > + enum: > > + - lltc,ltc4286 > > + - lltc,ltc4287 > > I don't recall seeing an answer to Guenter about this ltc4287 device: > https://lore.kernel.org/all/22f6364c-611c-ffb6-451c-9ddc20418d0a@roeck-us. > net/ > > > + > > + reg: > > + maxItems: 1 > > + > > + adi,vrange-select-25p6: > > + description: > > + This property is a bool parameter to represent the > > + voltage range is 25.6 or not for this chip. > > 25.6 what? Volts? microvolts? 25.6 volts > What about Guenter's suggestion to name this so that it better matches the > other, similar properties? > > Cheers, > Conor.
> -----Original Message----- > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck > Sent: Thursday, October 26, 2023 11:10 PM > To: Conor Dooley <conor@kernel.org>; Delphine_CC_Chiu/WYHQ/Wiwynn > <Delphine_CC_Chiu@wiwynn.com> > Cc: patrick@stwcx.xyz; Jean Delvare <jdelvare@suse.com>; Rob Herring > <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > Jonathan Corbet <corbet@lwn.net>; linux-i2c@vger.kernel.org; > linux-hwmon@vger.kernel.org; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org > Subject: Re: [PATCH v2 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver > bindings > > Security Reminder: Please be aware that this email is sent by an external > sender. > > On 10/26/23 07:25, Conor Dooley wrote: > > Hey, > > > > On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote: > >> Add a device tree bindings for ltc4286 driver. > > > > Bindings are for devices, not for drivers. > > > >> > >> Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > >> > >> Changelog: > >> v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6 > >> - Add type for adi,vrange-select-25p6 > >> - Revise rsense-micro-ohms to shunt-resistor-micro-ohms > >> --- > >> .../bindings/hwmon/lltc,ltc4286.yaml | 50 > +++++++++++++++++++ > >> MAINTAINERS | 10 ++++ > >> 2 files changed, 60 insertions(+) > >> create mode 100644 > >> Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > >> > >> diff --git > >> a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > >> b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > >> new file mode 100644 > >> index 000000000000..17022de657bb > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > >> @@ -0,0 +1,50 @@ > >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > >> +--- > >> +$id: > >> +http://dev/ > >> > +icetree.org%2Fschemas%2Fhwmon%2Flltc%2Cltc4286.yaml%23&data=05%7C > 01% > >> > +7CWayne_SC_Liu%40wiwynn.com%7Cb250e206c9ef48fbc1c108dbd6359e51 > %7Cda6 > >> > +e0628fc834caf9dd273061cbab167%7C0%7C0%7C638339298041650948%7CU > nknown > >> > +%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW > wiL > >> > +CJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=gGQEDl33zRIJbfbinu2%2Bi2cC > Ay6y0o > >> +DSLzBpLL7hA%2F8%3D&reserved=0 > >> +$schema: > >> +http://dev/ > >> > +icetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7CWayne_S > C_Li > >> > +u%40wiwynn.com%7Cb250e206c9ef48fbc1c108dbd6359e51%7Cda6e0628fc8 > 34caf > >> > +9dd273061cbab167%7C0%7C0%7C638339298041650948%7CUnknown%7CT > WFpbGZsb3 > >> > +d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0 > %3D > >> > +%7C3000%7C%7C%7C&sdata=nl1IM1HpYptsJOHfiuXtKFmD%2FVlGMCW1IkoK > HYco0sk > >> +%3D&reserved=0 > >> + > >> +title: LTC4286 power monitors > >> + > >> +maintainers: > >> + - Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > >> + > >> +properties: > >> + compatible: > >> + enum: > >> + - lltc,ltc4286 > >> + - lltc,ltc4287 > > > > I don't recall seeing an answer to Guenter about this ltc4287 device: > > https://lore/ > > .kernel.org%2Fall%2F22f6364c-611c-ffb6-451c-9ddc20418d0a%40roeck-us.n > e > > > t%2F&data=05%7C01%7CWayne_SC_Liu%40wiwynn.com%7Cb250e206c9ef48f > bc1c108 > > > dbd6359e51%7Cda6e0628fc834caf9dd273061cbab167%7C0%7C0%7C6383392 > 9804165 > > > 0948%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz > IiLCJBT > > > iI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=OpwfD3rBS0vQBF > jUhszrMg > > 4mq581jU7gx54Ln8V3gUA%3D&reserved=0 > > > > At least the chip does officially exist now, and a datasheet is available. > > https://www.ana/ > log.com%2Fen%2Fproducts%2Fltc4287.html&data=05%7C01%7CWayne_SC_Li > u%40wiwynn.com%7Cb250e206c9ef48fbc1c108dbd6359e51%7Cda6e0628fc83 > 4caf9dd273061cbab167%7C0%7C0%7C638339298041650948%7CUnknown%7 > CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL > CJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=CwT16Uipl8RymnFZ1WuBJzUJv > fLCKdK3VlTcTBN0xxk%3D&reserved=0 > > It shows that coefficients for the telemetry commands are different, meaning > that indeed both chips need to be explicitly referenced in the properties > description (and handled in the driver, which proves my point of needing a > datasheet before accepting such a driver). We will check the difference of coefficients for the telemetry commands with vendor. > > >> + > >> + reg: > >> + maxItems: 1 > >> + > >> + adi,vrange-select-25p6: > >> + description: > >> + This property is a bool parameter to represent the > >> + voltage range is 25.6 or not for this chip. > > > > 25.6 what? Volts? microvolts? > > What about Guenter's suggestion to name this so that it better matches > > the other, similar properties? > > > > I still would prefer one of the more common properties. > I still prefer adi,vrange-high-enable. > > Guenter
> -----Original Message----- > From: Conor Dooley <conor@kernel.org> > Sent: Thursday, October 26, 2023 11:26 PM > To: Guenter Roeck <linux@roeck-us.net> > Cc: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@wiwynn.com>; > patrick@stwcx.xyz; Jean Delvare <jdelvare@suse.com>; Rob Herring > <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > Jonathan Corbet <corbet@lwn.net>; linux-i2c@vger.kernel.org; > linux-hwmon@vger.kernel.org; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org > Subject: Re: [PATCH v2 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver > bindings > > On Thu, Oct 26, 2023 at 08:09:52AM -0700, Guenter Roeck wrote: > > On 10/26/23 07:25, Conor Dooley wrote: > > > Hey, > > > > > > On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote: > > > > Add a device tree bindings for ltc4286 driver. > > > > > > Bindings are for devices, not for drivers. > > > > > > > > > > > Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > > > > > > > > Changelog: > > > > v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6 > > > > - Add type for adi,vrange-select-25p6 > > > > - Revise rsense-micro-ohms to shunt-resistor-micro-ohms > > > > --- > > > > .../bindings/hwmon/lltc,ltc4286.yaml | 50 > +++++++++++++++++++ > > > > MAINTAINERS | 10 ++++ > > > > 2 files changed, 60 insertions(+) > > > > create mode 100644 > > > > Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > > > > b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > > > > new file mode 100644 > > > > index 000000000000..17022de657bb > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > > > > @@ -0,0 +1,50 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML > > > > +1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: LTC4286 power monitors > > > > + > > > > +maintainers: > > > > + - Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > > > > + > > > > +properties: > > > > + compatible: > > > > + enum: > > > > + - lltc,ltc4286 > > > > + - lltc,ltc4287 > > > > > > I don't recall seeing an answer to Guenter about this ltc4287 device: > > > https://lore.kernel.org/all/22f6364c-611c-ffb6-451c-9ddc20418d0a@roe > > > ck-us.net/ > > > > > > > At least the chip does officially exist now, and a datasheet is available. > > > > https://www.analog.com/en/products/ltc4287.html > > > > It shows that coefficients for the telemetry commands are different, > > meaning that indeed both chips need to be explicitly referenced in the > > properties description (and handled in the driver, which proves my > > point of needing a datasheet before accepting such a driver). > > Oh neat, would've been good if that'd been mentioned! > > > > > + > > > > + reg: > > > > + maxItems: 1 > > > > + > > > > + adi,vrange-select-25p6: > > > > + description: > > > > + This property is a bool parameter to represent the > > > > + voltage range is 25.6 or not for this chip. > > > > > > 25.6 what? Volts? microvolts? > > > What about Guenter's suggestion to name this so that it better > > > matches the other, similar properties? > > > > > > > I still would prefer one of the more common properties. > > I still prefer adi,vrange-high-enable. > > I think, from reading the previous version, that this is actually the lower voltage > range, as there is a 102.x $unit range too that is the default in the hardware. > Obviously, the use of the property could be inverted to match that naming > however. We will use adi,vrange-low-enable instead of adi,vrange-select-25p6 > > Cheers, > Conor.
> -----Original Message----- > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck > Sent: Friday, October 27, 2023 12:49 AM > To: Conor Dooley <conor@kernel.org> > Cc: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@wiwynn.com>; > patrick@stwcx.xyz; Jean Delvare <jdelvare@suse.com>; Rob Herring > <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > Jonathan Corbet <corbet@lwn.net>; linux-i2c@vger.kernel.org; > linux-hwmon@vger.kernel.org; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org > Subject: Re: [PATCH v2 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver > bindings > > Security Reminder: Please be aware that this email is sent by an external > sender. > > On 10/26/23 08:26, Conor Dooley wrote: > > On Thu, Oct 26, 2023 at 08:09:52AM -0700, Guenter Roeck wrote: > >> On 10/26/23 07:25, Conor Dooley wrote: > >>> Hey, > >>> > >>> On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote: > >>>> Add a device tree bindings for ltc4286 driver. > >>> > >>> Bindings are for devices, not for drivers. > >>> > >>>> > >>>> Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > >>>> > >>>> Changelog: > >>>> v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6 > >>>> - Add type for adi,vrange-select-25p6 > >>>> - Revise rsense-micro-ohms to shunt-resistor-micro-ohms > >>>> --- > >>>> .../bindings/hwmon/lltc,ltc4286.yaml | 50 > +++++++++++++++++++ > >>>> MAINTAINERS | 10 ++++ > >>>> 2 files changed, 60 insertions(+) > >>>> create mode 100644 > >>>> Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > >>>> > >>>> diff --git > >>>> a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > >>>> b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > >>>> new file mode 100644 > >>>> index 000000000000..17022de657bb > >>>> --- /dev/null > >>>> +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml > >>>> @@ -0,0 +1,50 @@ > >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML > >>>> +1.2 > >>>> +--- > >>>> +$id: > >>>> +https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fd > >>>> > +evicetree.org%2Fschemas%2Fhwmon%2Flltc%2Cltc4286.yaml%23&data=05% > 7 > >>>> > +C01%7CWayne_SC_Liu%40wiwynn.com%7Cf555db9618454d17e0b508dbd64 > 36721 > >>>> > +%7Cda6e0628fc834caf9dd273061cbab167%7C0%7C0%7C6383393572109674 > 95%7 > >>>> > +CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB > TiI > >>>> > +6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=cPOOqJ5y3K%2B > D%2Frsp > >>>> +gVhpKDIC0gC5nKBbRp7Up0OxDpM%3D&reserved=0 > >>>> +$schema: > >>>> +https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fd > >>>> > +evicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7CWayne > _S > >>>> > +C_Liu%40wiwynn.com%7Cf555db9618454d17e0b508dbd6436721%7Cda6e06 > 28fc > >>>> > +834caf9dd273061cbab167%7C0%7C0%7C638339357210967495%7CUnknown > %7CTW > >>>> > +FpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX > >>>> > +VCI6Mn0%3D%7C3000%7C%7C%7C&sdata=HjOhDm8bwPaNpWgumCSlak%2 > FiqoagwZc > >>>> +0eb95J1wnKUE%3D&reserved=0 > >>>> + > >>>> +title: LTC4286 power monitors > >>>> + > >>>> +maintainers: > >>>> + - Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > >>>> + > >>>> +properties: > >>>> + compatible: > >>>> + enum: > >>>> + - lltc,ltc4286 > >>>> + - lltc,ltc4287 > >>> > >>> I don't recall seeing an answer to Guenter about this ltc4287 device: > >>> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo > >>> > re.kernel.org%2Fall%2F22f6364c-611c-ffb6-451c-9ddc20418d0a%40roeck-u > >>> > s.net%2F&data=05%7C01%7CWayne_SC_Liu%40wiwynn.com%7Cf555db96184 > 54d17 > >>> > e0b508dbd6436721%7Cda6e0628fc834caf9dd273061cbab167%7C0%7C0%7C6 > 38339 > >>> > 357210967495%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQI > joiV2l > >>> > uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=oekIX > %2FlP > >>> %2B%2F4KhrSlK8Xp1zR0fdX1Emmr0Ox5GPS9Dz0%3D&reserved=0 > >>> > >> > >> At least the chip does officially exist now, and a datasheet is available. > >> > >> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww > >> .analog.com%2Fen%2Fproducts%2Fltc4287.html&data=05%7C01%7CWayne > _SC_Li > >> > u%40wiwynn.com%7Cf555db9618454d17e0b508dbd6436721%7Cda6e0628fc8 > 34caf9 > >> > dd273061cbab167%7C0%7C0%7C638339357210967495%7CUnknown%7CTWF > pbGZsb3d8 > >> > eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D > %7C > >> > 3000%7C%7C%7C&sdata=FqRheGXFi%2FmSH3cnRZ7eSbwD3JfZj8powcGBCJcP > wWQ%3D& > >> reserved=0 > >> > >> It shows that coefficients for the telemetry commands are different, > >> meaning that indeed both chips need to be explicitly referenced in > >> the properties description (and handled in the driver, which proves > >> my point of needing a datasheet before accepting such a driver). > > > > Oh neat, would've been good if that'd been mentioned! > > > > Actually, turns out there is some contradiction in the LTC4286 datasheet. > It mentions different coefficients in different places. It is all but impossible to > determine if the datasheet is wrong or if the chip uses a variety of coefficients > unless one has a real chip available. Sigh :-(. We are not the chip vendor, but we could forward your question to vendor. Could you point out the exact places (which pages) where are the contradiction in LTC4286 datasheet? > > >>>> + > >>>> + reg: > >>>> + maxItems: 1 > >>>> + > >>>> + adi,vrange-select-25p6: > >>>> + description: > >>>> + This property is a bool parameter to represent the > >>>> + voltage range is 25.6 or not for this chip. > >>> > >>> 25.6 what? Volts? microvolts? > >>> What about Guenter's suggestion to name this so that it better > >>> matches the other, similar properties? > >>> > >> > >> I still would prefer one of the more common properties. > >> I still prefer adi,vrange-high-enable. > > > > I think, from reading the previous version, that this is actually the > > lower voltage range, as there is a 102.x $unit range too that is the > > default in the hardware. Obviously, the use of the property could be > > inverted to match that naming however. > > > > It needs to be programmed either way because it is unknown how the chip has > been programmed before. That means some action is needed no matter if the > property is provided or not. > > Sure, we could add something like adi,vrange-low-enable. Not sure if that > would be any better. > > Actually, after looking at the datasheets, I'd be more interested in the impact > of other configuration buts such as VPWR_SELECT which selects the voltage > used for power calculations, or all the settings in MFR_ADC_CONFIG. The > datasheet doesn't really explain (or I can't figure out how it does) how the > different configurations affect the reported telemetry values. But that is a > question for the driver, not for the property description. We have sent an e-mail about this question. > > Guenter
On 10/30/23 23:25, Delphine_CC_Chiu/WYHQ/Wiwynn wrote: [ ... ] >> >> Actually, turns out there is some contradiction in the LTC4286 datasheet. >> It mentions different coefficients in different places. It is all but impossible to >> determine if the datasheet is wrong or if the chip uses a variety of coefficients >> unless one has a real chip available. Sigh :-(. > We are not the chip vendor, but we could forward your question to vendor. > Could you point out the exact places (which pages) where are the contradiction in LTC4286 datasheet? > See "PMBUS COMMAND SUMMARY", default values: "IOUT_OC_WARN_LIMIT" says "21.3 mV/RSENSE" "PIN_OP_WARN_LIMIT" says "2.8/RSENSE" This seems to contradict "Table 8. PMBus M, B, and R Parameters". But then, reading it again (and again), I think it is just an odd and confusing way of trying to describe the 0x7fff default register values. Sorry for the noise. Guenter
diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml new file mode 100644 index 000000000000..17022de657bb --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml @@ -0,0 +1,50 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: LTC4286 power monitors + +maintainers: + - Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> + +properties: + compatible: + enum: + - lltc,ltc4286 + - lltc,ltc4287 + + reg: + maxItems: 1 + + adi,vrange-select-25p6: + description: + This property is a bool parameter to represent the + voltage range is 25.6 or not for this chip. + type: boolean + + shunt-resistor-micro-ohms: + description: + Resistor value in micro-Ohm + +required: + - compatible + - reg + - shunt-resistor-micro-ohms + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + power-sensor@40 { + compatible = "lltc,ltc4286"; + reg = <0x40>; + adi,vrange-select-25p6; + shunt-resistor-micro-ohms = <300>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 668d1e24452d..89e5fff4bba9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12466,6 +12466,16 @@ S: Maintained F: Documentation/hwmon/ltc4261.rst F: drivers/hwmon/ltc4261.c +LTC4286 HARDWARE MONITOR DRIVER +M: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> +L: linux-i2c@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml +F: Documentation/devicetree/bindings/hwmon/ltc4286.rst +F: drivers/hwmon/pmbus/Kconfig +F: drivers/hwmon/pmbus/Makefile +F: drivers/hwmon/pmbus/ltc4286.c + LTC4306 I2C MULTIPLEXER DRIVER M: Michael Hennerich <michael.hennerich@analog.com> L: linux-i2c@vger.kernel.org
Add a device tree bindings for ltc4286 driver. Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> Changelog: v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6 - Add type for adi,vrange-select-25p6 - Revise rsense-micro-ohms to shunt-resistor-micro-ohms --- .../bindings/hwmon/lltc,ltc4286.yaml | 50 +++++++++++++++++++ MAINTAINERS | 10 ++++ 2 files changed, 60 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml