Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 8 Jun 2023 11:03:15 +0300
From:   Heikki Krogerus <heikki.krogerus@linux.intel.com>
To:     neil.armstrong@linaro.org
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] usb: typec: add support for the nb7vpq904m Type-C
 Linear Redriver
Message-ID: <ZIGLQ31CXN1VuVDI@kuha.fi.intel.com>
References: <20230601-topic-sm8x50-upstream-redriver-v2-0-dda89b22b1c0@linaro.org>
 <20230601-topic-sm8x50-upstream-redriver-v2-2-dda89b22b1c0@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230601-topic-sm8x50-upstream-redriver-v2-2-dda89b22b1c0@linaro.org>
Precedence: bulk

Hi,

On Wed, Jun 07, 2023 at 09:51:09AM +0200, neil.armstrong@linaro.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