| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250511141120.58941a45@jic23-huawei>
Date: Sun, 11 May 2025 14:11:20 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Jishnu Prakash <jishnu.prakash@....qualcomm.com>
Cc: robh@...nel.org, krzysztof.kozlowski@...aro.org, krzk+dt@...nel.org,
conor+dt@...nel.org, agross@...nel.org, andersson@...nel.org,
lumag@...nel.org, dmitry.baryshkov@....qualcomm.com,
konradybcio@...nel.org, daniel.lezcano@...aro.org, sboyd@...nel.org,
amitk@...nel.org, thara.gopinath@...il.com, lee@...nel.org,
rafael@...nel.org, subbaraman.narayanamurthy@....qualcomm.com,
david.collins@....qualcomm.com, anjelique.melendez@....qualcomm.com,
quic_kamalw@...cinc.com, rui.zhang@...el.com, lukasz.luba@....com,
devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org, cros-qcom-dts-watchers@...omium.org,
quic_skakitap@...cinc.com, neil.armstrong@...aro.org,
stephan.gerhold@...aro.org
Subject: Re: [PATCH V6 5/5] thermal: qcom: add support for PMIC5 Gen3 ADC
thermal monitoring
On Fri, 9 May 2025 16:39:59 +0530
Jishnu Prakash <jishnu.prakash@....qualcomm.com> wrote:
> Add support for ADC_TM part of PMIC5 Gen3.
>
> This is an auxiliary driver under the Gen3 ADC driver, which implements the
> threshold setting and interrupt generating functionalities of QCOM ADC_TM
> drivers, used to support thermal trip points.
>
> Signed-off-by: Jishnu Prakash <jishnu.prakash@....qualcomm.com>
Hi Jishnu,
A few minor things inline.
Jonathan
> diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5-gen3.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5-gen3.c
> new file mode 100644
> index 000000000000..c63822635f10
> --- /dev/null
> +++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5-gen3.c
> +static int adc_tm5_register_tzd(struct adc_tm5_gen3_chip *adc_tm5)
> +{
> + unsigned int i, channel;
> + struct thermal_zone_device *tzd;
> +
> + for (i = 0; i < adc_tm5->nchannels; i++) {
> + channel = V_CHAN(adc_tm5->chan_props[i].common_props);
> + tzd = devm_thermal_of_zone_register(adc_tm5->dev, channel,
> + &adc_tm5->chan_props[i],
> + &adc_tm_ops);
> +
> + if (IS_ERR(tzd)) {
> + if (PTR_ERR(tzd) == -ENODEV) {
> + dev_warn(adc_tm5->dev,
> + "thermal sensor on channel %d is not used\n",
> + channel);
> + continue;
> + }
> + return dev_err_probe(adc_tm5->dev, PTR_ERR(tzd),
> + "Error registering TZ zone:%ld for channel:%d\n",
> + PTR_ERR(tzd), channel);
> + }
> + adc_tm5->chan_props[i].tzd = tzd;
> + devm_thermal_add_hwmon_sysfs(adc_tm5->dev, tzd);
Can fail so unusual not to see an error check. Add a comment if intended.
> + }
> + return 0;
> +}
> +
> +static int adc_tm5_probe(struct auxiliary_device *aux_dev,
> + const struct auxiliary_device_id *id)
> +{
> + struct adc_tm5_gen3_chip *adc_tm5;
> + struct tm5_aux_dev_wrapper *aux_dev_wrapper;
> + struct device *dev = &aux_dev->dev;
> + int i, ret;
> +
> + adc_tm5 = devm_kzalloc(&aux_dev->dev, sizeof(*adc_tm5), GFP_KERNEL);
Use dev
> + if (!adc_tm5)
> + return -ENOMEM;
> +
> + aux_dev_wrapper = container_of(aux_dev, struct tm5_aux_dev_wrapper,
> + aux_dev);
> +
> + adc_tm5->dev = dev;
> + adc_tm5->dev_data = aux_dev_wrapper->dev_data;
> + adc_tm5->nchannels = aux_dev_wrapper->n_tm_channels;
> + adc_tm5->chan_props = devm_kcalloc(adc_tm5->dev, aux_dev_wrapper->n_tm_channels,
Might as well use dev here too.
> + sizeof(*adc_tm5->chan_props), GFP_KERNEL);
> + if (!adc_tm5->chan_props)
> + return -ENOMEM;
> +
> + for (i = 0; i < adc_tm5->nchannels; i++) {
> + adc_tm5->chan_props[i].common_props = aux_dev_wrapper->tm_props[i];
> + adc_tm5->chan_props[i].timer = MEAS_INT_1S;
> + adc_tm5->chan_props[i].sdam_index = (i + 1) / 8;
> + adc_tm5->chan_props[i].tm_chan_index = (i + 1) % 8;
> + adc_tm5->chan_props[i].chip = adc_tm5;
> + }
> +
> + ret = devm_add_action_or_reset(adc_tm5->dev, adc5_gen3_disable, adc_tm5);
> + if (ret)
> + return ret;
> +
> + INIT_WORK(&adc_tm5->tm_handler_work, tm_handler_work);
> +
> + /*
> + * Skipping first SDAM IRQ as it is requested in parent driver.
> + * If there is a TM violation on that IRQ, the parent driver calls
> + * the notifier (tm_event_notify) exposed from this driver to handle it.
> + */
> + for (i = 1; i < adc_tm5->dev_data->num_sdams; i++) {
> + ret = devm_request_threaded_irq(adc_tm5->dev,
> + adc_tm5->dev_data->base[i].irq,
> + NULL, adctm5_gen3_isr, IRQF_ONESHOT,
> + adc_tm5->dev_data->base[i].irq_name,
> + adc_tm5);
> + if (ret < 0)
> + return ret;
> + }
> +
> + /*
> + * This drvdata is only used in the function (adctm_event_handler)
> + * called by parent ADC driver in case of TM violation on the first SDAM.
> + */
> + auxiliary_set_drvdata(aux_dev, adc_tm5);
> +
> + ret = devm_add_action(adc_tm5->dev, adc5_gen3_clear_work, adc_tm5);
I'd add a comment on what this is undoing as normally devm clean up matches
something being started and there is no obvious sign of what that is here.
> + if (ret)
> + return ret;
> +
> + ret = adc_tm5_register_tzd(adc_tm5);
return adc_tm5...
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +static int __init adctm5_init_module(void)
> +{
> + return auxiliary_driver_register(&adctm5gen3_auxiliary_drv.adrv);
> +}
> +
> +static void __exit adctm5_exit_module(void)
> +{
> + auxiliary_driver_unregister(&adctm5gen3_auxiliary_drv.adrv);
> +}
> +
> +module_init(adctm5_init_module);
> +module_exit(adctm5_exit_module);
module_auxiliary_driver() not work for some reason?
> +
> +MODULE_DESCRIPTION("SPMI PMIC Thermal Monitor ADC driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("QCOM_SPMI_ADC5_GEN3");
Powered by blists - more mailing lists