Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] arm64: dts: qcom: unpopulated ADC7_AMUX_THM5 channel breaks adp12v_voltage - [Medium] arm64: dts: qcom: invalid trip2 in pmc8280c-thermal breaks CPU throttling --
commit 1587f072e6febbe214d258d0ce37a60770079813 Author: Xilin Wu <[email protected]> arm64: dts: qcom: sc8280xp: add Radxa Dragon Q8B This commit adds initial device tree support for the Radxa Dragon Q8B single-board computer based on the Qualcomm Snapdragon 8cx Gen 3 SC8280XP platform. > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-radxa-dragon-q8b.dts > b/arch/arm64/boot/dts/qcom/sc8280xp-radxa-dragon-q8b.dts > new file mode 100644 > index 0000000000000..e0fae337ab667 > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-radxa-dragon-q8b.dts [ ... ] > + thermal-zones { [ ... ] > + pmc8280c-thermal { > + cooling-maps { > + map0 { > + trip = <&pmc8280c_alert0>; > + /* Throttle X1C cores to 2496MHz at > 105C */ > + cooling-device = <&cpu4 5 5>, > + <&cpu5 5 5>, > + <&cpu6 5 5>, > + <&cpu7 5 5>; > + }; > + }; > + > + trips { > + pmc8280c_alert0: trip2 { > + temperature = <105000>; > + hysteresis = <0>; > + type = "passive"; > + }; > + }; > + }; [Severity: Medium] Can this trip2 node in pmc8280c-thermal work as intended alongside the base definition in arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi? In sc8280xp-pmics.dtsi, pmc8280c_thermal already defines two trips on pmc8280c_temp_alarm (which has no io-channels property): pmc8280c_thermal: pmc8280c-thermal { polling-delay-passive = <100>; thermal-sensors = <&pmc8280c_temp_alarm>; trips { trip0 { temperature = <95000>; hysteresis = <0>; type = "passive"; }; trip1 { temperature = <115000>; hysteresis = <0>; type = "critical"; }; }; }; Appending trip2 at 105000 mC results in three trips ordered as 95000, 115000, and 105000. In drivers/thermal/qcom/qcom-spmi-temp-alarm.c: 1. On GEN2 rev2 hardware, qpnp_tm_init() -> qpnp_tm_gen2_rev2_configure_trip_temps() programs the three hardware stages in DTB order and then enforces strictly increasing thresholds: for (i = 1; i < STAGE_COUNT; i++) { if (chip->temp_thresh_map[i] <= chip->temp_thresh_map[i - 1]) { dev_err(chip->dev, "Threshold %d=%ld <= threshold %d=%ld\n", i, chip->temp_thresh_map[i], i - 1, chip->temp_thresh_map[i - 1]); return -EINVAL; } } Because threshold 2 (105000) is less than threshold 1 (115000), probe fails with -EINVAL. 2. On GEN2 rev0/rev1 hardware without an ADC channel, qpnp_tm_get_temp() -> qpnp_tm_update_temp_no_adc() only reports discrete stage temperatures (37000 mC in Stage 0, 97000 mC in Stage 1, and 117000 mC in Stage 2): if (stage_new > stage_old) { /* increasing stage, use lower bound */ chip->temp = qpnp_tm_decode_temp(chip, stage_new) + TEMP_STAGE_HYSTERESIS; } As temperature rises, the reported value jumps directly from 97000 mC to 117000 mC (crossing trip1 at 115000 mC), so trip2 at 105000 mC and its CPU cooling map will never activate before shutdown. Should pmc8280c-thermal override trip0 (or wire an ADC channel to pmc8280c_temp_alarm) instead of adding trip2? [ ... ] > + adp12v_voltage: voltage-divider-adp12v { > + compatible = "voltage-divider"; > + io-channels = <&pmk8280_vadc PM8350_ADC7_AMUX_THM5(1)>; > + #io-channel-cells = <1>; > + > + /* R470 (110K) in series with R471 (10K) to ground. */ > + full-ohms = <120000>; > + output-ohms = <10000>; > + }; [ ... ] > +&pmk8280_vadc { > + channel@108 { > + reg = <PM8350_ADC7_AMUX_THM5(1)>; > + qcom,hw-settle-time = <200>; > + qcom,pre-scaling = <1 1>; > + label = "adp12v_det"; > + }; [Severity: Medium] Will channel@108 and its consumers adp12v_voltage and iio-hwmon fail to probe at runtime when using PM8350_ADC7_AMUX_THM5(1)? In drivers/iio/adc/qcom-spmi-adc5.c, pmk8280_vadc (compatible "qcom,spmi-adc7") uses adc7_chans_pmic[]. That array defines [ADC7_AMUX_THM5_100K_PU] (0x48), but has no entry for unpulled [ADC7_AMUX_THM5] (0x08): static const struct adc5_channels adc7_chans_pmic[ADC5_MAX_CHANNEL] = { [ADC7_REF_GND] = ADC5_CHAN_VOLT("ref_gnd", 0, SCALE_HW_CALIB_DEFAULT) ... [ADC7_AMUX_THM5_100K_PU] = ADC5_CHAN_TEMP("amux_thm5_pu2", 0, SCALE_HW_CALIB_THERM_100K_PU_PM7) When adc5_get_fw_data() initializes channel@108 from adc7_chans_pmic[0x08], the zero-initialized entry leaves iio_chan->info_mask_separate as 0. When drivers/iio/afe/iio-rescale.c probes adp12v_voltage via rescale_probe() -> rescale_configure_channel(), it checks the source channel capabilities: if (iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) && (iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE) || iio_channel_has_info(schan, IIO_CHAN_INFO_OFFSET))) { dev_info(dev, "using raw+scale/offset source channel\n"); } else if (iio_channel_has_info(schan, IIO_CHAN_INFO_PROCESSED)) { dev_info(dev, "using processed channel\n"); rescale->chan_processed = true; } else { dev_err(dev, "source channel is not supported\n"); return -EINVAL; } Because neither IIO_CHAN_INFO_RAW nor IIO_CHAN_INFO_PROCESSED is set on the source channel, adp12v_voltage fails to probe with -EINVAL, and the downstream iio-hwmon node defers probe indefinitely. Does adc7_chans_pmic[] in drivers/iio/adc/qcom-spmi-adc5.c need an ADC7_AMUX_THM5 voltage channel entry before this channel can be used here? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
