[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA8EJprroFtKvCeZtP_FySr5d6v4dogYQKFQvzPJLpwWBuMrKw@mail.gmail.com>
Date: Fri, 19 Jan 2024 13:44:06 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: gregkh@...uxfoundation.org, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
heikki.krogerus@...ux.intel.com, matthias.bgg@...il.com,
neil.armstrong@...aro.org, andersson@...nel.org, nathan@...nel.org,
luca.weiss@...rphone.com, tianping.fang@...iatek.com,
linux-usb@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, kernel@...labora.com
Subject: Re: [PATCH 2/2] usb: typec: mux: Add ITE IT5205 Alternate Mode
Passive MUX driver
On Fri, 19 Jan 2024 at 12:46, AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com> wrote:
>
> The ITE IT5202 is a USB Type-C Alternate Mode Passive MUX, used for
> muxing the SBU lines of a Type-C port with DisplayPort altmode and
> also providing an orientation switch.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> ---
> drivers/usb/typec/mux/Kconfig | 10 ++
> drivers/usb/typec/mux/Makefile | 1 +
> drivers/usb/typec/mux/it5205.c | 292 +++++++++++++++++++++++++++++++++
> 3 files changed, 303 insertions(+)
> create mode 100644 drivers/usb/typec/mux/it5205.c
>
> diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig
> index d2cb5e733e57..399c7b0983df 100644
> --- a/drivers/usb/typec/mux/Kconfig
> +++ b/drivers/usb/typec/mux/Kconfig
> @@ -36,6 +36,16 @@ config TYPEC_MUX_INTEL_PMC
> control the USB role switch and also the multiplexer/demultiplexer
> switches used with USB Type-C Alternate Modes.
>
> +config TYPEC_MUX_IT5205
> + tristate "ITE IT5205 Type-C USB Alt Mode Passive MUX driver"
> + depends on I2C
> + select REGMAP_I2C
> + help
> + Driver for the ITE IT5205 Type-C USB Alternate Mode Passive MUX
> + which provides support for muxing DisplayPort and sideband signals
> + on a common USB Type-C connector.
> + If compiled as a module, the module will be named it5205.
> +
> config TYPEC_MUX_NB7VPQ904M
> tristate "On Semiconductor NB7VPQ904M Type-C redriver driver"
> depends on I2C
> diff --git a/drivers/usb/typec/mux/Makefile b/drivers/usb/typec/mux/Makefile
> index 57dc9ac6f8dc..bb96f30267af 100644
> --- a/drivers/usb/typec/mux/Makefile
> +++ b/drivers/usb/typec/mux/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_TYPEC_MUX_FSA4480) += fsa4480.o
> obj-$(CONFIG_TYPEC_MUX_GPIO_SBU) += gpio-sbu-mux.o
> obj-$(CONFIG_TYPEC_MUX_PI3USB30532) += pi3usb30532.o
> obj-$(CONFIG_TYPEC_MUX_INTEL_PMC) += intel_pmc_mux.o
> +obj-$(CONFIG_TYPEC_MUX_IT5205) += it5205.o
> obj-$(CONFIG_TYPEC_MUX_NB7VPQ904M) += nb7vpq904m.o
> obj-$(CONFIG_TYPEC_MUX_PTN36502) += ptn36502.o
> obj-$(CONFIG_TYPEC_MUX_WCD939X_USBSS) += wcd939x-usbss.o
> diff --git a/drivers/usb/typec/mux/it5205.c b/drivers/usb/typec/mux/it5205.c
> new file mode 100644
> index 000000000000..99203b8a086d
> --- /dev/null
> +++ b/drivers/usb/typec/mux/it5205.c
> @@ -0,0 +1,292 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ITE IT5205 Type-C USB alternate mode passive mux
> + *
> + * Copyright (c) 2020 MediaTek Inc.
> + * Copyright (c) 2024 Collabora Ltd.
> + * AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> + *
> + */
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/mutex.h>
> +#include <linux/of_platform.h>
> +#include <linux/regmap.h>
> +#include <linux/usb/typec.h>
> +#include <linux/usb/typec_mux.h>
> +#include <linux/usb/typec_dp.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio.h>
> +#include <linux/usb/tcpm.h>
I'd say, it is a usual custom to sort this list.
[skipped]
> +
> +static int it5205_mux_set(struct typec_mux_dev *mux, struct typec_mux_state *state)
> +{
> + struct it5205 *it = typec_mux_get_drvdata(mux);
> + u8 val;
> +
> + switch (state->mode) {
> + case TYPEC_STATE_USB:
> + val = IT5205_USB;
> + break;
> + case TYPEC_DP_STATE_C:
> + fallthrough;
> + case TYPEC_DP_STATE_E:
> + val = IT5205_DP;
> + break;
> + case TYPEC_DP_STATE_D:
> + val = IT5205_DP_USB;
> + break;
> + case TYPEC_STATE_SAFE:
> + fallthrough;
> + default:
> + val = 0;
> + break;
> + }
Please add a check for state->altmode. All states at TYPEC_STATE_MODAL
and above (which includes TYPEC_DP_STATE_[CDE]) are only relevant with
connection to the particular typec->altmode SVID.
> +
> + return regmap_update_bits(it->regmap, IT5205_REG_MUXCR,
> + IT5205_DP_USB_CTRL_MASK, val);
> +}
> +
> +static irqreturn_t it5205_irq_handler(int irq, void *data)
> +{
> + struct it5205 *it = data;
> + int ret;
> + u32 val;
> +
> + ret = regmap_read(it->regmap, IT5205_REG_ISR, &val);
> + if (ret)
> + return IRQ_NONE;
> +
> + if (val & IT5205_ISR_CSBU_OVP) {
> + dev_warn(&it->client->dev, "Overvoltage detected!\n");
Will it cut the voltage automatically?
> +
> + /* Reset CSBU */
> + regmap_update_bits(it->regmap, IT5205_REG_CSBUSR,
> + IT5205_CSBUSR_SWITCH, 0);
> + regmap_update_bits(it->regmap, IT5205_REG_CSBUSR,
> + IT5205_CSBUSR_SWITCH, IT5205_CSBUSR_SWITCH);
> + }
> +
> + return IRQ_HANDLED;
> +}
The rest LGTM
--
With best wishes
Dmitry
Powered by blists - more mailing lists