Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

Krzysztof Kozlowski
2026-02-06T08:12:52Z

On Fri, Feb 06, 2026 at 02:44:06AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
> + bcl->hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
> + bcl->hwmon_name,
> + bcl,
> + &bcl_hwmon_chip_info,
> + NULL);
> + if (IS_ERR(bcl->hwmon_dev)) {
> + dev_err(&pdev->dev, "Failed to register hwmon device: %ld\n",
> + PTR_ERR(bcl->hwmon_dev));
> + return PTR_ERR(bcl->hwmon_dev);
> + }
> +
> + dev_dbg(&pdev->dev, "BCL hwmon device with version: %u.%u registered\n",
> + bcl_get_version_major(bcl), bcl_get_version_minor(bcl));
> +
> + return 0;
> +}
> +
> +static const struct of_device_id bcl_match[] = {
> + {
> + .compatible = "qcom,bcl-v1",
> + .data = &pm7250b_data,

And that's the proof that your binding is just repeating downstream
bollocks pattern. Something like "v1" does not exist if you even here
claim this is not "v1", but pm7250.

> + }, {
> + .compatible = "qcom,bcl-v2",
> + .data = &pm8350_data,
> + }, {
> + .compatible = "qcom,bcl-v3-bmx",
> + .data = &pm8550b_data,
> + }, {
> + .compatible = "qcom,bcl-v3-wb",
> + .data = &pmw5100_data,
> + }, {
> + .compatible = "qcom,bcl-v3-core",
> + .data = &pm8550_data,
> + }, {
> + .compatible = "qcom,bcl-v4-bmx",
> + .data = &pmih010_data,
> + }, {
> + .compatible = "qcom,bcl-v4-wb",
> + .data = &pmw6100_data,
> + }, {
> + .compatible = "qcom,bcl-v4-core",
> + .data = &pmh010_data,
> + }, {
> + .compatible = "qcom,bcl-v4-pmv010",
> + .data = &pmv010_data,
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, bcl_match);
> +
> +static struct platform_driver bcl_driver = {
> + .probe = bcl_probe,
> + .driver = {
> + .name = BCL_DRIVER_NAME,

No, just use name here.

> + .of_match_table = bcl_match,
> + },
> +};
> +
> +MODULE_AUTHOR("Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>");
> +MODULE_DESCRIPTION("QCOM BCL HWMON driver");
> +module_platform_driver(bcl_driver);
> +MODULE_LICENSE("GPL");

...

> +
> +/**
> + * bcl_disable_irq - Generic helper to disable alarm irq
> + * @alarm: BCL level alarm data
> + */
> +static inline void bcl_disable_irq(struct bcl_alarm_data *alarm)

Why do you write actual functions/code in the headers?

Please follow standard C rules, which mean that the C unit contains code
intended for execution. Headers are only for declarations.

> +{
> + if (!alarm->irq_enabled)
> + return;
> + alarm->irq_enabled = false;
> + disable_irq_nosync(alarm->irq);
> + disable_irq_wake(alarm->irq);
> +}
> +
> +#endif /* __QCOM_BCL_HWMON_H__ */
>
> --
> 2.43.0
>