Re: [PATCH v4] usb: typec: intel_pmc_mux: Expose IOM port status to debugfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 5/22/2023 9:56 PM, Greg KH wrote:
On Mon, May 22, 2023 at 03:24:13PM +0530, Rajat Khandelwal wrote:
Hi,

On 5/22/2023 1:07 PM, Greg KH wrote:
On Mon, May 22, 2023 at 12:33:43PM +0530, Rajat Khandelwal wrote:
IOM status has a crucial role during debugging to check the
current state of the type-C port.
There are ways to fetch the status, but all those require the
IOM port status offset, which could change with platform.

Make a debugfs directory for intel_pmc_mux and expose the status
under it per port basis.

Signed-off-by: Rajat Khandelwal <rajat.khandelwal@xxxxxxxxxxxxxxx>
Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
---

v4:
1. Maintain a root directory for PMC module and incorporate devices
under it
2. Add the debugfs module under '/sys/kernel/debug/usb'
3. Use the platform device 'pmc->dev' to assign the device's name

v3: Allocate the debugfs directory name for the platform device with
its ACPI dev name included

v2:
1. Remove static declaration of the debugfs root for 'intel_pmc_mux'
2. Remove explicitly defined one-liner functions

   drivers/usb/typec/mux/intel_pmc_mux.c | 54 ++++++++++++++++++++++++++-
   1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
index 34e4188a40ff..f400094c76f2 100644
--- a/drivers/usb/typec/mux/intel_pmc_mux.c
+++ b/drivers/usb/typec/mux/intel_pmc_mux.c
@@ -15,6 +15,8 @@
   #include <linux/usb/typec_mux.h>
   #include <linux/usb/typec_dp.h>
   #include <linux/usb/typec_tbt.h>
+#include <linux/debugfs.h>
+#include <linux/usb.h>
   #include <asm/intel_scu_ipc.h>
@@ -143,8 +145,14 @@ struct pmc_usb {
   	struct acpi_device *iom_adev;
   	void __iomem *iom_base;
   	u32 iom_port_status_offset;
+
+#ifdef CONFIG_DEBUG_FS
+	struct dentry *dentry;
+#endif
No need for the #ifdef anymore, right?  In fact, I think it will break
the build if you have it this way and CONFIG_DEBUG_FS is not enabled,
right?
I guess you're right. Maybe it'd have been fine if the rest of the
debugfs stuff was also enclosed within the conditional macros.
Which is not needed and not good kernel coding style, so it's right that
they are not there.

Sure.


Anyways, removing it seems appropriate now.

Also, is it OK to send you v5 on the public list directly?
Where else would you send it?

I was under the impression that you'd again require the review
internally under Intel before releasing out to public list.
Anyways, sending out v5.

Thanks
Rajat


confused,

greg k-h



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux