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 */