linux-kernel - Re: [PATCH v2 2/2] usb: typec: add support for the nb7vpq904m Type-C Linear Redriver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZIGLQ31CXN1VuVDI@kuha.fi.intel.com>
Date:   Thu, 8 Jun 2023 11:03:15 +0300
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     neil.armstrong@...aro.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        linux-arm-msm@...r.kernel.org, linux-usb@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] usb: typec: add support for the nb7vpq904m Type-C
 Linear Redriver

Hi,

On Wed, Jun 07, 2023 at 09:51:09AM +0200, neil.armstrong@...aro.org wrote:
> +static int nb7vpq904m_set(struct nb7vpq904m *nb7)
> +{
> +	bool reverse = (nb7->orientation == TYPEC_ORIENTATION_REVERSE);
> +
> +	switch (nb7->mode) {
> +	case TYPEC_STATE_SAFE:
> +		regmap_write(nb7->regmap, GEN_DEV_SET_REG,
> +			     GEN_DEV_SET_CHIP_EN |
> +			     GEN_DEV_SET_CHNA_EN |
> +			     GEN_DEV_SET_CHNB_EN |
> +			     GEN_DEV_SET_CHNC_EN |
> +			     GEN_DEV_SET_CHND_EN |
> +			     FIELD_PREP(GEN_DEV_SET_OP_MODE_MASK,
> +					GEN_DEV_SET_OP_MODE_USB));
> +		nb7vpq904m_set_channel(nb7, NB7_CHNA, false);
> +		nb7vpq904m_set_channel(nb7, NB7_CHNB, false);
> +		nb7vpq904m_set_channel(nb7, NB7_CHNC, false);
> +		nb7vpq904m_set_channel(nb7, NB7_CHND, false);
> +		regmap_write(nb7->regmap, AUX_CC_REG, 0x2);
> +
> +		return 0;
> +
> +	case TYPEC_STATE_USB:
> +		/*
> +		 * Normal Orientation (CC1)
> +		 * A -> USB RX
> +		 * B -> USB TX
> +		 * C -> X
> +		 * D -> X
> +		 * Flipped Orientation (CC2)
> +		 * A -> X
> +		 * B -> X
> +		 * C -> USB TX
> +		 * D -> USB RX
> +		 *
> +		 * Reversed if data lanes are swapped
> +		 */
> +		if (reverse ^ nb7->swap_data_lanes) {
> +			regmap_write(nb7->regmap, GEN_DEV_SET_REG,
> +				     GEN_DEV_SET_CHIP_EN |
> +				     GEN_DEV_SET_CHNA_EN |
> +				     GEN_DEV_SET_CHNB_EN |
> +				     FIELD_PREP(GEN_DEV_SET_OP_MODE_MASK,
> +						GEN_DEV_SET_OP_MODE_USB));
> +			nb7vpq904m_set_channel(nb7, NB7_CHNA, false);
> +			nb7vpq904m_set_channel(nb7, NB7_CHNB, false);
> +		} else {
> +			regmap_write(nb7->regmap, GEN_DEV_SET_REG,
> +				     GEN_DEV_SET_CHIP_EN |
> +				     GEN_DEV_SET_CHNC_EN |
> +				     GEN_DEV_SET_CHND_EN |
> +				     FIELD_PREP(GEN_DEV_SET_OP_MODE_MASK,
> +						GEN_DEV_SET_OP_MODE_USB));
> +			nb7vpq904m_set_channel(nb7, NB7_CHNC, false);
> +			nb7vpq904m_set_channel(nb7, NB7_CHND, false);
> +		}
> +		regmap_write(nb7->regmap, AUX_CC_REG, 0x2);
> +
> +		return 0;
> +
> +	default:
> +		if (nb7->svid != USB_TYPEC_DP_SID)
> +			return -EINVAL;
> +
> +		break;
> +	}
> +
> +	/* DP Altmode Setup */
> +
> +	regmap_write(nb7->regmap, AUX_CC_REG, reverse ? 0x1 : 0x0);
> +
> +	switch (nb7->mode) {
> +	case TYPEC_DP_STATE_C:
> +	case TYPEC_DP_STATE_E:
> +		/*
> +		 * Normal Orientation (CC1)
> +		 * A -> DP3
> +		 * B -> DP2
> +		 * C -> DP1
> +		 * D -> DP0
> +		 * Flipped Orientation (CC2)
> +		 * A -> DP0
> +		 * B -> DP1
> +		 * C -> DP2
> +		 * D -> DP3
> +		 */
> +		regmap_write(nb7->regmap, GEN_DEV_SET_REG,
> +			     GEN_DEV_SET_CHIP_EN |
> +			     GEN_DEV_SET_CHNA_EN |
> +			     GEN_DEV_SET_CHNB_EN |
> +			     GEN_DEV_SET_CHNC_EN |
> +			     GEN_DEV_SET_CHND_EN |
> +			     FIELD_PREP(GEN_DEV_SET_OP_MODE_MASK,
> +					GEN_DEV_SET_OP_MODE_DP_4LANE));
> +		nb7vpq904m_set_channel(nb7, NB7_CHNA, true);
> +		nb7vpq904m_set_channel(nb7, NB7_CHNB, true);
> +		nb7vpq904m_set_channel(nb7, NB7_CHNC, true);
> +		nb7vpq904m_set_channel(nb7, NB7_CHND, true);
> +		break;
> +
> +	case TYPEC_DP_STATE_D:
> +	case TYPEC_DP_STATE_F:
> +		regmap_write(nb7->regmap, GEN_DEV_SET_REG,
> +			     GEN_DEV_SET_CHIP_EN |
> +			     GEN_DEV_SET_CHNA_EN |
> +			     GEN_DEV_SET_CHNB_EN |
> +			     GEN_DEV_SET_CHNC_EN |
> +			     GEN_DEV_SET_CHND_EN |
> +			     FIELD_PREP(GEN_DEV_SET_OP_MODE_MASK,
> +					reverse ^ nb7->swap_data_lanes ?
> +						GEN_DEV_SET_OP_MODE_DP_CC2
> +						: GEN_DEV_SET_OP_MODE_DP_CC1));
> +
> +		/*
> +		 * Normal Orientation (CC1)
> +		 * A -> USB RX
> +		 * B -> USB TX
> +		 * C -> DP1
> +		 * D -> DP0
> +		 * Flipped Orientation (CC2)
> +		 * A -> DP0
> +		 * B -> DP1
> +		 * C -> USB TX
> +		 * D -> USB RX
> +		 *
> +		 * Reversed if data lanes are swapped
> +		 */
> +		if (nb7->swap_data_lanes) {
> +			nb7vpq904m_set_channel(nb7, NB7_CHNA, !reverse);
> +			nb7vpq904m_set_channel(nb7, NB7_CHNB, !reverse);
> +			nb7vpq904m_set_channel(nb7, NB7_CHNC, reverse);
> +			nb7vpq904m_set_channel(nb7, NB7_CHND, reverse);
> +		} else {
> +			nb7vpq904m_set_channel(nb7, NB7_CHNA, reverse);
> +			nb7vpq904m_set_channel(nb7, NB7_CHNB, reverse);
> +			nb7vpq904m_set_channel(nb7, NB7_CHNC, !reverse);
> +			nb7vpq904m_set_channel(nb7, NB7_CHND, !reverse);
> +		}
> +		break;
> +
> +	default:
> +		return -ENOTSUPP;

That should be -EOPNOTSUPP.

> +	}
> +
> +	return 0;
> +}
> +
> +static int nb7vpq904m_sw_set(struct typec_switch_dev *sw,
> +			      enum typec_orientation orientation)

While at it, one line should be enough for this one.

thanks,

-- 
heikki