[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <8834d309-19f2-466f-aed4-ab0f9e2df02d@collabora.com>
Date: Fri, 19 Jan 2024 13:55:17 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
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
Il 19/01/24 12:44, Dmitry Baryshkov ha scritto:
> 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.
>
Oh, whoops. Totally forgot to do that, sorry - thanks for pointing that out!
Done for v2.
> [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.
>
Done for v2.
>> +
>> + 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?
>
As far as I understand, yes.
Unfortunately, there's no datasheet.
>> +
>> + /* 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
>
>
Powered by blists - more mailing lists