* [PATCH V2] scsi: ufs-debugfs: Add error counters
@ 2020-12-16 18:51 Adrian Hunter
2020-12-16 20:16 ` Bean Huo
0 siblings, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2020-12-16 18:51 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, linux-kernel, Alim Akhtar, Avri Altman, Bean Huo,
Can Guo, Stanley Chu
People testing have a need to know how many errors might be occurring
over time. Add error counters and expose them via debugfs.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
Changes in V2:
Add missing '#include "ufs-debugfs.h"' in ufs-debugfs.c
Reported-by: kernel test robot <lkp@intel.com>
drivers/scsi/ufs/Makefile | 1 +
drivers/scsi/ufs/ufs-debugfs.c | 56 ++++++++++++++++++++++++++++++++++
drivers/scsi/ufs/ufs-debugfs.h | 22 +++++++++++++
drivers/scsi/ufs/ufshcd.c | 19 ++++++++++++
drivers/scsi/ufs/ufshcd.h | 5 +++
5 files changed, 103 insertions(+)
create mode 100644 drivers/scsi/ufs/ufs-debugfs.c
create mode 100644 drivers/scsi/ufs/ufs-debugfs.h
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 4679af1b564e..9ca32b1143cb 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -9,6 +9,7 @@ ufs_qcom-$(CONFIG_SCSI_UFS_CRYPTO) += ufs-qcom-ice.o
obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
ufshcd-core-y += ufshcd.o ufs-sysfs.o
+ufshcd-core-$(CONFIG_DEBUG_FS) += ufs-debugfs.o
ufshcd-core-$(CONFIG_SCSI_UFS_BSG) += ufs_bsg.o
ufshcd-core-$(CONFIG_SCSI_UFS_CRYPTO) += ufshcd-crypto.o
obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
diff --git a/drivers/scsi/ufs/ufs-debugfs.c b/drivers/scsi/ufs/ufs-debugfs.c
new file mode 100644
index 000000000000..e7e6e2dba346
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-debugfs.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2020 Intel Corporation
+
+#include <linux/debugfs.h>
+
+#include "ufs-debugfs.h"
+#include "ufshcd.h"
+
+static struct dentry *ufs_debugfs_root;
+
+void ufs_debugfs_init(void)
+{
+ ufs_debugfs_root = debugfs_create_dir("ufshcd", NULL);
+}
+
+void ufs_debugfs_exit(void)
+{
+ debugfs_remove_recursive(ufs_debugfs_root);
+}
+
+static int ufs_debugfs_stats_show(struct seq_file *s, void *data)
+{
+ struct ufs_hba *hba = s->private;
+ struct ufs_event_hist *e = hba->ufs_stats.event;
+
+#define PRT(fmt, typ) \
+ seq_printf(s, fmt, e[UFS_EVT_ ## typ].cnt)
+
+ PRT("PHY Adapter Layer errors (except LINERESET): %llu\n", PA_ERR);
+ PRT("Data Link Layer errors: %llu\n", DL_ERR);
+ PRT("Network Layer errors: %llu\n", NL_ERR);
+ PRT("Transport Layer errors: %llu\n", TL_ERR);
+ PRT("Generic DME errors: %llu\n", DME_ERR);
+ PRT("Auto-hibernate errors: %llu\n", AUTO_HIBERN8_ERR);
+ PRT("IS Fatal errors (CEFES, SBFES, HCFES, DFES): %llu\n", FATAL_ERR);
+ PRT("DME Link Startup errors: %llu\n", LINK_STARTUP_FAIL);
+ PRT("PM Resume errors: %llu\n", RESUME_ERR);
+ PRT("PM Suspend errors : %llu\n", SUSPEND_ERR);
+ PRT("Logical Unit Resets: %llu\n", DEV_RESET);
+ PRT("Host Resets: %llu\n", HOST_RESET);
+ PRT("SCSI command aborts: %llu\n", ABORT);
+#undef PRT
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(ufs_debugfs_stats);
+
+void ufs_debugfs_hba_init(struct ufs_hba *hba)
+{
+ hba->debugfs_root = debugfs_create_dir(dev_name(hba->dev), ufs_debugfs_root);
+ debugfs_create_file("stats", 0400, hba->debugfs_root, hba, &ufs_debugfs_stats_fops);
+}
+
+void ufs_debugfs_hba_exit(struct ufs_hba *hba)
+{
+ debugfs_remove_recursive(hba->debugfs_root);
+}
diff --git a/drivers/scsi/ufs/ufs-debugfs.h b/drivers/scsi/ufs/ufs-debugfs.h
new file mode 100644
index 000000000000..d5e744c7bd2d
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-debugfs.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2020 Intel Corporation
+ */
+
+#ifndef __UFS_DEBUGFS_H__
+#define __UFS_DEBUGFS_H__
+
+struct ufs_hba;
+
+#ifdef CONFIG_DEBUG_FS
+void ufs_debugfs_init(void);
+void ufs_debugfs_exit(void);
+void ufs_debugfs_hba_init(struct ufs_hba *hba);
+void ufs_debugfs_hba_exit(struct ufs_hba *hba);
+#else
+static inline void ufs_debugfs_init(void) {}
+static inline void ufs_debugfs_exit(void) {}
+static inline void ufs_debugfs_hba_init(struct ufs_hba *hba) {}
+static inline void ufs_debugfs_hba_exit(struct ufs_hba *hba) {}
+#endif
+
+#endif
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 82ad31781bc9..d8a3cf0cd6d5 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -20,6 +20,7 @@
#include "ufs_quirks.h"
#include "unipro.h"
#include "ufs-sysfs.h"
+#include "ufs-debugfs.h"
#include "ufs_bsg.h"
#include "ufshcd-crypto.h"
#include <asm/unaligned.h>
@@ -4540,6 +4541,7 @@ void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val)
e = &hba->ufs_stats.event[id];
e->val[e->pos] = val;
e->tstamp[e->pos] = ktime_get();
+ e->cnt += 1;
e->pos = (e->pos + 1) % UFS_EVENT_HIST_LENGTH;
ufshcd_vops_event_notify(hba, id, &val);
@@ -8334,6 +8336,8 @@ static int ufshcd_hba_init(struct ufs_hba *hba)
if (err)
goto out_disable_vreg;
+ ufs_debugfs_hba_init(hba);
+
hba->is_powered = true;
goto out;
@@ -8350,6 +8354,7 @@ static int ufshcd_hba_init(struct ufs_hba *hba)
static void ufshcd_hba_exit(struct ufs_hba *hba)
{
if (hba->is_powered) {
+ ufs_debugfs_hba_exit(hba);
ufshcd_variant_hba_exit(hba);
ufshcd_setup_vreg(hba, false);
ufshcd_suspend_clkscaling(hba);
@@ -9436,6 +9441,20 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
}
EXPORT_SYMBOL_GPL(ufshcd_init);
+static int __init ufshcd_core_init(void)
+{
+ ufs_debugfs_init();
+ return 0;
+}
+
+static void __exit ufshcd_core_exit(void)
+{
+ ufs_debugfs_exit();
+}
+
+module_init(ufshcd_core_init);
+module_exit(ufshcd_core_exit);
+
MODULE_AUTHOR("Santosh Yaragnavi <santosh.sy@samsung.com>");
MODULE_AUTHOR("Vinayak Holikatti <h.vinayak@samsung.com>");
MODULE_DESCRIPTION("Generic UFS host controller driver Core");
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index aa9ea3552323..8c51ce01517e 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -445,11 +445,13 @@ struct ufs_clk_scaling {
* @pos: index to indicate cyclic buffer position
* @reg: cyclic buffer for registers value
* @tstamp: cyclic buffer for time stamp
+ * @cnt: error counter
*/
struct ufs_event_hist {
int pos;
u32 val[UFS_EVENT_HIST_LENGTH];
ktime_t tstamp[UFS_EVENT_HIST_LENGTH];
+ unsigned long long cnt;
};
/**
@@ -817,6 +819,9 @@ struct ufs_hba {
u32 crypto_cfg_register;
struct blk_keyslot_manager ksm;
#endif
+#ifdef CONFIG_DEBUG_FS
+ struct dentry *debugfs_root;
+#endif
};
/* Returns true if clocks can be gated. Otherwise false */
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V2] scsi: ufs-debugfs: Add error counters
2020-12-16 18:51 [PATCH V2] scsi: ufs-debugfs: Add error counters Adrian Hunter
@ 2020-12-16 20:16 ` Bean Huo
2020-12-17 9:49 ` Adrian Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Bean Huo @ 2020-12-16 20:16 UTC (permalink / raw)
To: Adrian Hunter, Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, linux-kernel, Alim Akhtar, Avri Altman, Can Guo, Stanley Chu
On Wed, 2020-12-16 at 20:51 +0200, Adrian Hunter wrote:
> ufshcd_variant_hba_exit(hba);
> ufshcd_setup_vreg(hba, false);
> ufshcd_suspend_clkscaling(hba);
> @@ -9436,6 +9441,20 @@ int ufshcd_init(struct ufs_hba *hba, void
> __iomem *mmio_base, unsigned int irq)
> }
> EXPORT_SYMBOL_GPL(ufshcd_init);
>
> +static int __init ufshcd_core_init(void)
> +{
> + ufs_debugfs_init();
> + return 0;
> +}
> +
> +static void __exit ufshcd_core_exit(void)
> +{
> + ufs_debugfs_exit();
> +}
Hi, Adrian
The purpose of patch is acceptable, but I don't know why you choose
using ufshcd_core_* here.
Also. I don't know if module_init() is a proper way here.
thanks,
Bean
>
> n
> +
> +module_init(ufshcd_core_init);
> +module_exit(ufshcd_core_exit)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2] scsi: ufs-debugfs: Add error counters
2020-12-16 20:16 ` Bean Huo
@ 2020-12-17 9:49 ` Adrian Hunter
2020-12-17 11:06 ` Avri Altman
2020-12-17 22:57 ` Bean Huo
0 siblings, 2 replies; 6+ messages in thread
From: Adrian Hunter @ 2020-12-17 9:49 UTC (permalink / raw)
To: Bean Huo, Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, linux-kernel, Alim Akhtar, Avri Altman, Can Guo, Stanley Chu
On 16/12/20 10:16 pm, Bean Huo wrote:
> On Wed, 2020-12-16 at 20:51 +0200, Adrian Hunter wrote:
>> ufshcd_variant_hba_exit(hba);
>> ufshcd_setup_vreg(hba, false);
>> ufshcd_suspend_clkscaling(hba);
>> @@ -9436,6 +9441,20 @@ int ufshcd_init(struct ufs_hba *hba, void
>> __iomem *mmio_base, unsigned int irq)
>> }
>> EXPORT_SYMBOL_GPL(ufshcd_init);
>>
>> +static int __init ufshcd_core_init(void)
>> +{
>> + ufs_debugfs_init();
>> + return 0;
>> +}
>> +
>> +static void __exit ufshcd_core_exit(void)
>> +{
>> + ufs_debugfs_exit();
>> +}
>
> Hi, Adrian
Thanks for looking at the patch.
>
> The purpose of patch is acceptable, but I don't know why you choose
> using ufshcd_core_* here.
Do you mean you would like a different function name? 'ufshcd_init' is used
already. The module is called ufshcd-core, so ufshcd_core_* seems appropriate.
> Also. I don't know if module_init() is a proper way here.
Can you be more specific? It is normal to do module initialization in
module_init().
However I do see a problem. When builtin, initcalls are ordered according
to link order, but the Makefile does not have ufshcd-core at the top i.e.
$ cat drivers/scsi/ufs/Makefile
# SPDX-License-Identifier: GPL-2.0
# UFSHCD makefile
obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o
tc-dwc-g210.o
obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o
tc-dwc-g210.o
obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o
obj-$(CONFIG_SCSI_UFS_QCOM) += ufs_qcom.o
ufs_qcom-y += ufs-qcom.o
ufs_qcom-$(CONFIG_SCSI_UFS_CRYPTO) += ufs-qcom-ice.o
obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
ufshcd-core-y += ufshcd.o ufs-sysfs.o
ufshcd-core-$(CONFIG_DEBUG_FS) += ufs-debugfs.o
ufshcd-core-$(CONFIG_SCSI_UFS_BSG) += ufs_bsg.o
ufshcd-core-$(CONFIG_SCSI_UFS_CRYPTO) += ufshcd-crypto.o
obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
obj-$(CONFIG_SCSI_UFS_MEDIATEK) += ufs-mediatek.o
obj-$(CONFIG_SCSI_UFS_TI_J721E) += ti-j721e-ufs.o
Should be:
# SPDX-License-Identifier: GPL-2.0
# UFSHCD makefile
# Order here is important. ufshcd-core must initialize first.
ufshcd-core-y += ufshcd.o ufs-sysfs.o
ufshcd-core-$(CONFIG_DEBUG_FS) += ufs-debugfs.o
ufshcd-core-$(CONFIG_SCSI_UFS_BSG) += ufs_bsg.o
ufshcd-core-$(CONFIG_SCSI_UFS_CRYPTO) += ufshcd-crypto.o
obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o
tc-dwc-g210.o
obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o
tc-dwc-g210.o
obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o
obj-$(CONFIG_SCSI_UFS_QCOM) += ufs_qcom.o
ufs_qcom-y += ufs-qcom.o
ufs_qcom-$(CONFIG_SCSI_UFS_CRYPTO) += ufs-qcom-ice.o
obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
obj-$(CONFIG_SCSI_UFS_MEDIATEK) += ufs-mediatek.o
obj-$(CONFIG_SCSI_UFS_TI_J721E) += ti-j721e-ufs.o
What do you think?
>
> thanks,
> Bean
>
>>
>> n
>> +
>> +module_init(ufshcd_core_init);
>> +module_exit(ufshcd_core_exit)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH V2] scsi: ufs-debugfs: Add error counters
2020-12-17 9:49 ` Adrian Hunter
@ 2020-12-17 11:06 ` Avri Altman
2020-12-17 22:57 ` Bean Huo
1 sibling, 0 replies; 6+ messages in thread
From: Avri Altman @ 2020-12-17 11:06 UTC (permalink / raw)
To: Adrian Hunter, Bean Huo, Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, linux-kernel, Alim Akhtar, Can Guo, Stanley Chu
Please add my reviewed-by once you fix that.
Thanks,
Avri
> However I do see a problem. When builtin, initcalls are ordered according
> to link order, but the Makefile does not have ufshcd-core at the top i.e.
>
> $ cat drivers/scsi/ufs/Makefile
> # SPDX-License-Identifier: GPL-2.0
> # UFSHCD makefile
> obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o
> tc-dwc-g210.o
> obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o
> ufshcd-dwc.o
> tc-dwc-g210.o
> obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o
> obj-$(CONFIG_SCSI_UFS_QCOM) += ufs_qcom.o
> ufs_qcom-y += ufs-qcom.o
> ufs_qcom-$(CONFIG_SCSI_UFS_CRYPTO) += ufs-qcom-ice.o
> obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
> obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
> ufshcd-core-y += ufshcd.o ufs-sysfs.o
> ufshcd-core-$(CONFIG_DEBUG_FS) += ufs-debugfs.o
> ufshcd-core-$(CONFIG_SCSI_UFS_BSG) += ufs_bsg.o
> ufshcd-core-$(CONFIG_SCSI_UFS_CRYPTO) += ufshcd-crypto.o
> obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
> obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
> obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
> obj-$(CONFIG_SCSI_UFS_MEDIATEK) += ufs-mediatek.o
> obj-$(CONFIG_SCSI_UFS_TI_J721E) += ti-j721e-ufs.o
>
> Should be:
>
> # SPDX-License-Identifier: GPL-2.0
> # UFSHCD makefile
> # Order here is important. ufshcd-core must initialize first.
> ufshcd-core-y += ufshcd.o ufs-sysfs.o
> ufshcd-core-$(CONFIG_DEBUG_FS) += ufs-debugfs.o
> ufshcd-core-$(CONFIG_SCSI_UFS_BSG) += ufs_bsg.o
> ufshcd-core-$(CONFIG_SCSI_UFS_CRYPTO) += ufshcd-crypto.o
> obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o
> tc-dwc-g210.o
> obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o
> ufshcd-dwc.o
> tc-dwc-g210.o
> obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o
> obj-$(CONFIG_SCSI_UFS_QCOM) += ufs_qcom.o
> ufs_qcom-y += ufs-qcom.o
> ufs_qcom-$(CONFIG_SCSI_UFS_CRYPTO) += ufs-qcom-ice.o
> obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
> obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
> obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
> obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
> obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
> obj-$(CONFIG_SCSI_UFS_MEDIATEK) += ufs-mediatek.o
> obj-$(CONFIG_SCSI_UFS_TI_J721E) += ti-j721e-ufs.o
>
> What do you think?
>
> >
> > thanks,
> > Bean
> >
> >>
> >> n
> >> +
> >> +module_init(ufshcd_core_init);
> >> +module_exit(ufshcd_core_exit)
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2] scsi: ufs-debugfs: Add error counters
2020-12-17 9:49 ` Adrian Hunter
2020-12-17 11:06 ` Avri Altman
@ 2020-12-17 22:57 ` Bean Huo
2020-12-18 12:17 ` Adrian Hunter
1 sibling, 1 reply; 6+ messages in thread
From: Bean Huo @ 2020-12-17 22:57 UTC (permalink / raw)
To: Adrian Hunter, Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, linux-kernel, Alim Akhtar, Avri Altman, Can Guo, Stanley Chu
On Thu, 2020-12-17 at 11:49 +0200, Adrian Hunter wrote:
> >
> > The purpose of patch is acceptable, but I don't know why you choose
> > using ufshcd_core_* here.
>
> Do you mean you would like a different function name? 'ufshcd_init'
> is used
> already. The module is called ufshcd-core, so ufshcd_core_* seems
> appropriate.
>
> > Also. I don't know if module_init() is a proper way here.
>
> Can you be more specific? It is normal to do module initialization
> in
> module_init().
Hi Adrian
My concern that ufs_debugfs_init() is called in module_init(), but your
another debugfs initialization function ufs_debugfs_hba_init(hba)
called in the UFS host probe path.
If these two (module_init() and module_platform_driver())
initializaiton sequence always as your expectation: ufs_debugfs_init()-
->ufs_debugfs_hba_init(), that is fine, otherwise, it is better just
group them, make it simpler.
Thanks,
Bean
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2] scsi: ufs-debugfs: Add error counters
2020-12-17 22:57 ` Bean Huo
@ 2020-12-18 12:17 ` Adrian Hunter
0 siblings, 0 replies; 6+ messages in thread
From: Adrian Hunter @ 2020-12-18 12:17 UTC (permalink / raw)
To: Bean Huo, Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, linux-kernel, Alim Akhtar, Avri Altman, Can Guo, Stanley Chu
On 18/12/20 12:57 am, Bean Huo wrote:
> On Thu, 2020-12-17 at 11:49 +0200, Adrian Hunter wrote:
>>>
>>> The purpose of patch is acceptable, but I don't know why you choose
>>> using ufshcd_core_* here.
>>
>> Do you mean you would like a different function name? 'ufshcd_init'
>> is used
>> already. The module is called ufshcd-core, so ufshcd_core_* seems
>> appropriate.
>>
>>> Also. I don't know if module_init() is a proper way here.
>>
>> Can you be more specific? It is normal to do module initialization
>> in
>> module_init().
>
> Hi Adrian
> My concern that ufs_debugfs_init() is called in module_init(), but your
> another debugfs initialization function ufs_debugfs_hba_init(hba)
> called in the UFS host probe path.
It is a good question, but module dependencies and initcall ordering means
that won't happen. It is not unusual for modules to do initialization in
this way, that is completed before dependent modules initialize.
>
> If these two (module_init() and module_platform_driver())
> initializaiton sequence always as your expectation: ufs_debugfs_init()-
> ->ufs_debugfs_hba_init(), that is fine, otherwise, it is better just
> group them, make it simpler.
Unfortunately, doing it that way, calls to debugsfs_create_dir("ufshcd",
NULL) could race. Losers of the race will get an error, and will not get the
dentry they need to proceed. Preventing the race would require adding a
mutex. So the other way is simpler.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-18 12:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-16 18:51 [PATCH V2] scsi: ufs-debugfs: Add error counters Adrian Hunter
2020-12-16 20:16 ` Bean Huo
2020-12-17 9:49 ` Adrian Hunter
2020-12-17 11:06 ` Avri Altman
2020-12-17 22:57 ` Bean Huo
2020-12-18 12:17 ` Adrian Hunter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).