On Mon, May 01, 2023 at 01:11:05PM +0100, Bryan O'Donoghue wrote:
> This commit adds a QCOM PMIC TCPM driver with an initial pm8150b
> block.
>
> The driver is layered as follows:
>
> qcom_pmic_typec.c : Responsible for registering with TCPM and arbitrates
> access to the Type-C and PDPHY hardware blocks in one
> place. This presents a single TCPM device to device to
> the Linux TCPM layer.
>
> qcom_pmic_typec_pdphy.c: Responsible for interfacing with the PDPHY hardware and
> processing power-delivery related calls from TCPM.
> This hardware binding can be extended to
> facilitate similar hardware in different PMICs.
>
> qcom_pmic_typec_port.c: Responsible for notifying and processing Type-C
> related calls from TCPM. Similar to the pdphy this
> layer can be extended to handle the specifics of
> different Qualcomm PMIC Type-C port managers.
>
> This code provides all of the same functionality as the existing
> qcom typec driver plus power-delivery as well.
>
> As a result commit 6c8cf3695176 ("usb: typec: Add QCOM PMIC typec detection
> driver") can be deleted entirely.
>
> References code from Jonathan Marek, Jack Pham, Wesley Cheng, Hemant Kumar,
> Guru Das Srinagesh and Ashay Jaiswal.
>
> Acked-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>
Couple of nits below.
[ ... ]
> +
> +static void qcom_pmic_typec_pdphy_pd_receive(struct pmic_typec_pdphy *pmic_typec_pdphy)
> +{
> + struct device *dev = pmic_typec_pdphy->dev;
> + struct pd_message msg;
> + unsigned int size, rx_status;
> + unsigned long flags;
> + int ret;
> +
> + spin_lock_irqsave(&pmic_typec_pdphy->lock, flags);
> +
> + ret = regmap_read(pmic_typec_pdphy->regmap,
> + pmic_typec_pdphy->base + USB_PDPHY_RX_SIZE_REG, &size);
> + if (ret)
> + goto done;
> +
> + /* Hardware requires +1 of the real read value to be passed */
> + if ((size < 1 || size > (sizeof(msg.payload) + 1))) {
if (size < 1 || size > sizeof(msg.payload) + 1) {
would have been sufficient.
[...]
> +int qcom_pmic_typec_port_get_cc(struct pmic_typec_port *pmic_typec_port,
> + enum typec_cc_status *cc1,
> + enum typec_cc_status *cc2)
> +{
> + struct device *dev = pmic_typec_port->dev;
> + unsigned int misc, val;
> + bool attached;
> + int ret = 0;
> +
> + ret = regmap_read(pmic_typec_port->regmap,
> + pmic_typec_port->base + TYPEC_MISC_STATUS_REG, &misc);
> + if (ret)
> + goto done;
> +
> + attached = !!(misc & CC_ATTACHED);
> +
> + if (pmic_typec_port->debouncing_cc) {
> + ret = -EBUSY;
> + goto done;
> + }
> +
> + *cc1 = TYPEC_CC_OPEN;
> + *cc2 = TYPEC_CC_OPEN;
> +
> + if (!(attached))
> + goto done;
Excess () around attached
Guenter
