On 1/14/25 11:56 AM, André Draszik wrote:
Hi Bart,
On Tue, 2025-01-14 at 09:55 -0800, Bart Van Assche wrote:
On 1/14/25 8:16 AM, André Draszik wrote:
+/**
- ufshcd_scsi_host_put_callback - deallocate underlying Scsi_Host and
thereby the Host Bus Adapter (HBA)
- @host: pointer to SCSI host
- */
+static void ufshcd_scsi_host_put_callback(void *host) +{
- scsi_host_put(host);
+}
Please rename ufshcd_scsi_host_put_callback() such that the function name makes clear when this function is called instead of what the function does.
Would you have a suggestion for such a name? Something like ufshcd_driver_release_action()?
Unless I'm misunderstanding you, I believe most drivers use a function name that says what the function does, e.g. dell_wmi_ddv_debugfs_remove (just as a completely random example out of many).
If going by when it is called and if applying this principle throughout ufshcd, then there can only ever be one such function in ufshcd, as all devm_add_action() callback actions happen at driver release, which surely isn't what you mean.
You probably meant something different?
I meant what I wrote in my previous email: to chose another name for ufshcd_scsi_host_put_callback() only. Having a function name that duplicates the function body leaves readers of the code guessing from where the function is called. BTW, naming callbacks after their call site is a normal practice as far as I know. From ufs-qcom.c:
static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = { .name = "qcom", .init = ufs_qcom_init, .exit = ufs_qcom_exit, .get_ufs_hci_version = ufs_qcom_get_ufs_hci_version, .clk_scale_notify = ufs_qcom_clk_scale_notify, .setup_clocks = ufs_qcom_setup_clocks, .hce_enable_notify = ufs_qcom_hce_enable_notify, .link_startup_notify = ufs_qcom_link_startup_notify, .pwr_change_notify = ufs_qcom_pwr_change_notify, .apply_dev_quirks = ufs_qcom_apply_dev_quirks, .fixup_dev_quirks = ufs_qcom_fixup_dev_quirks, .suspend = ufs_qcom_suspend, .resume = ufs_qcom_resume, .dbg_register_dump = ufs_qcom_dump_dbg_regs, .device_reset = ufs_qcom_device_reset, .config_scaling_param = ufs_qcom_config_scaling_param, .reinit_notify = ufs_qcom_reinit_notify, .mcq_config_resource = ufs_qcom_mcq_config_resource, .get_hba_mac = ufs_qcom_get_hba_mac, .op_runtime_config = ufs_qcom_op_runtime_config, .get_outstanding_cqs = ufs_qcom_get_outstanding_cqs, .config_esi = ufs_qcom_config_esi, };
Bart.