Re: [PATCH v2 1/6] usb: typec: ucsi: Remove unused fields from struct ucsi_connector_status

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

 



On Thu, Aug 22, 2024 at 02:24:17PM +0300, Heikki Krogerus wrote:
> Hi Abhishek,
> 
> I'm sorry to keep you waiting.
> 
> > You have me convinced on the "failing loudly" part but I'm still
> > confused about the "how".
> > 
> > Making sure we always check versions to access the bits makes me think
> > we need wrappers on casting to the rightly versioned connector status.
> > Should we be versioning access for everything that's not in UCSI 1.2
> > then?
> > 
> > Example:
> > 
> > struct ucsi_connector_status_raw {
> >   u8 bytes[19];
> > };
> > 
> > struct ucsi_connector_status_v1 {
> >   ...
> > };
> > 
> > struct ucsi_connector_status_v2 {
> >   ...
> > };
> > 
> > struct ucsi_connector_status_v1* get_connector_status_v1(struct
> > ucsi_connector *con) {
> >   return (struct ucsi_connector_status_v1 *)con->raw_status;
> > }
> > 
> > struct ucsi_connector_status_v2* get_connector_status_v2(struct
> > ucsi_connector *con) {
> >   return con->ucsi->version >= UCSI_VERSION_2_0 ? (struct
> > ucsi_connector_status_v2 *)&con->raw_status : NULL;
> > }
> > 
> > /* Read all bits supported by the current version. */
> > int ucsi_read_connector_status(struct ucsi_connector *con, struct
> > ucsi_connector_status_raw *raw_conn_status);
> 
> I'll take a look at this next week. Right now I have to focus on
> other tasks.

Sorry again for the delay. Today I sketched the idea that I have. I
think this problem would be the easiest to handle with field specific
helpers (see attached).

But at the same time I would like to get rid of all command specific
data structures. I've been planning that for some time now. We can do
that with a macro like that UCSI_FIELD(). The problem with those
structures is that if the fields in the structure don't align nicely
(like in case of GET_CONNECTOR_STATUS), we have to come up with custom
members, and that is not ideal at all. There are probable other
problems too.

I did not convert anything yet, I'll prepare a more complete RFC
tomorrow, but you should be able to get the idea from this.

thanks,

-- 
heikki
diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index c8c87377909d..972d7b364a3d 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -4,6 +4,7 @@
 #define __DRIVER_USB_TYPEC_UCSI_H
 
 #include <linux/bitops.h>
+#include <linux/bitfield.h>
 #include <linux/completion.h>
 #include <linux/device.h>
 #include <linux/power_supply.h>
@@ -352,6 +353,27 @@ struct ucsi_connector_status {
 #define   UCSI_CONSTAT_BC_TRICKLE_CHARGING	3
 } __packed;
 
+#define UCSI_CONNECTOR_CHANGE(_con)		UCSI_FIELD(&(_con)->status, 0, 16)
+#define UCSI_CONNECTOR_PWR_OPMODE(_con)		UCSI_FIELD(&(_con)->status, 16, 3)
+#define UCSI_CONNECTOR_CONNECTED(_con)		UCSI_FIELD(&(_con)->status, 19, 1)
+#define UCSI_CONNECTOR_PWR_DIR(_con)		UCSI_FIELD(&(_con)->status, 20, 1)
+#define UCSI_CONNECTOR_PARTNER_FLAGS(_con)	UCSI_FIELD(&(_con)->status, 21, 8)
+#define UCSI_CONNECTOR_PARTNER_TYPE(_con)	UCSI_FIELD(&(_con)->status, 29, 3)
+#define UCSI_CONNECTOR_RDO(_con)		UCSI_FIELD(&(_con)->status, 32, 32)
+#define UCSI_CONNECTOR_BC_STATUS(_con)		UCSI_FIELD(&(_con)->status, 64, 2)
+#define UCSI_CONNECTOR_PD_STATUS(_con)		UCSI_FIELD(&(_con)->status, 70, 16)
+
+#define UCSI_FIELD(data, offset, size)					\
+	({								\
+		u8 m = ((offset) % 8);					\
+		FIELD_GET((GENMASK((m + ((size) - 1)), m)),		\
+			  (*((u32 *)(&((u8 *)data)[((offset) / 8)]))));	\
+	})
+
+#define UCSI_FIELD_SAFE(ucsi, data, offset, size)			\
+	if (!WARN_ON(((offset) / 8) >= UCSI_MAX_DATA_LENGTH(ucsi)))	\
+		UCSI_FIELD(data, offset, size)
+
 /* -------------------------------------------------------------------------- */
 
 struct ucsi_debugfs_entry {
@@ -513,4 +535,73 @@ static inline void ucsi_debugfs_unregister(struct ucsi *ucsi) { }
 #define USB_TYPEC_NVIDIA_VLINK_DP_VDO	0x1
 #define USB_TYPEC_NVIDIA_VLINK_DBG_VDO	0x3
 
+/* -------------------------------------------------------------------------- */
+
+static __always_inline enum typec_orientation ucsi_orientation(struct ucsi_connector *con)
+{
+	if (WARN_ON(con->ucsi->version < UCSI_VERSION_2_0))
+		return TYPEC_ORIENTATION_NONE;
+	if (!UCSI_CONNECTOR_CONNECTED(con))
+		return TYPEC_ORIENTATION_NONE;
+	if (UCSI_FIELD(con, 86, 1))
+		return TYPEC_ORIENTATION_REVERSE;
+	return TYPEC_ORIENTATION_NORMAL;
+}
+
+static __always_inline int ucsi_sink_path_status(struct ucsi_connector *con)
+{
+	if (WARN_ON(con->ucsi->version < UCSI_VERSION_2_0))
+		return -EINVAL;
+	return UCSI_FIELD(con, 87, 1);
+}
+
+static inline int ucsi_reverse_current_protection_status(struct ucsi_connector *con)
+{
+	if (WARN_ON(con->ucsi->version < UCSI_VERSION_2_0))
+		return -EINVAL;
+	return UCSI_FIELD(con, 88, 1);
+}
+
+static inline int ucsi_power_reading_ready(struct ucsi_connector *con)
+{
+	if (WARN_ON(con->ucsi->version < UCSI_VERSION_2_0))
+		return -EINVAL;
+	return UCSI_FIELD(con, 89, 1);
+}
+
+static inline int ucsi_current_scale(struct ucsi_connector *con)
+{
+	if (WARN_ON(con->ucsi->version < UCSI_VERSION_2_0))
+		return -EINVAL;
+	return UCSI_FIELD(con, 90, 3) * 5; /* mA */
+}
+
+static inline int ucsi_peak_current(struct ucsi_connector *con)
+{
+	if (WARN_ON(con->ucsi->version < UCSI_VERSION_2_0))
+		return -EINVAL;
+	return UCSI_FIELD(con, 93, 16);
+}
+
+static inline int ucsi_average_current(struct ucsi_connector *con)
+{
+	if (WARN_ON(con->ucsi->version < UCSI_VERSION_2_0))
+		return -EINVAL;
+	return UCSI_FIELD(con, 109, 16);
+}
+
+static inline int ucsi_voltage_scale(struct ucsi_connector *con)
+{
+	if (WARN_ON(con->ucsi->version < UCSI_VERSION_2_0))
+		return -EINVAL;
+	return UCSI_FIELD(con, 125, 4) * 5; /* mV */
+}
+
+static inline int ucsi_voltage_reading(struct ucsi_connector *con)
+{
+	if (WARN_ON(con->ucsi->version < UCSI_VERSION_2_0))
+		return -EINVAL;
+	return UCSI_FIELD(con, 129, 16);
+}
+
 #endif /* __DRIVER_USB_TYPEC_UCSI_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