On Mon, Mar 15, 2021 at 02:49:04PM -0700, Doug Anderson wrote: > Hi, > > On Fri, Mar 12, 2021 at 10:32 AM Matthias Kaehlcke <mka@xxxxxxxxxxxx> wrote: > > > > CoachZ rev3 uses a 100k NTC thermistor for the charger temperatures, > > instead of the 47k NTC that is stuffed in earlier revisions. Add .dts > > files for rev3. > > > > The 47k NTC currently isn't supported by the PM6150 ADC driver. > > Disable the charger thermal zone for rev1 and rev2 to avoid the use > > of bogus temperature values. > > > > Signed-off-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx> > > --- > > > > Changes in v2: > > - added CoachZ rev3 > > - updated subject and commit message > > > > arch/arm64/boot/dts/qcom/Makefile | 2 ++ > > .../boot/dts/qcom/sc7180-trogdor-coachz-r1.dts | 9 +++++++++ > > .../dts/qcom/sc7180-trogdor-coachz-r2-lte.dts | 4 ++-- > > .../boot/dts/qcom/sc7180-trogdor-coachz-r2.dts | 13 +++++++++++-- > > .../dts/qcom/sc7180-trogdor-coachz-r3-lte.dts | 18 ++++++++++++++++++ > > .../boot/dts/qcom/sc7180-trogdor-coachz-r3.dts | 15 +++++++++++++++ > > 6 files changed, 57 insertions(+), 4 deletions(-) > > So what you have here is good and we could land it. Feel free to add > my Reviewed-by tag if you want. > > ...but I want to propose an alternative. It turns out that these days > coachz-r1 and coachz-r2 are actually the same. The only reason both > exist is because <https://crrev.com/c/2733863> ("CHROMIUM: arm64: dts: > qcom: sc7180: add dmic_clk_en back") wasn't the proper inverse of > <https://crrev.com/c/2596726> ("CHROMIUM: arm64: dts: qcom: sc7180: > remove dmic_clk_en"). > > It sorta squashes two changes into one, but if you combined your > change with one that folded "-r1" into "-r2" it would actually make a > smaller / easier to understand change, essentially, it would be: > - just a rename of the "-r2" file to be "-r3" > - add "-rev2" into the list of compatibles in "-r1" file. > - add the "disable" into the "-r1" file. I agree, if rev1 and rev2 are the same in terms of the DT they should use the same file(s).
