[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA8EJpqa3_ZaC7xo+pWBovKEvJ5utxR7nS1vSRnyskw7GZf-wQ@mail.gmail.com>
Date: Fri, 6 Sep 2024 12:28:25 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Song Xue <quic_songxue@...cinc.com>
Cc: Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, linux-phy@...ts.infradead.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org, Neil Armstrong <neil.armstrong@...aro.org>
Subject: Re: [PATCH v2 2/2] phy: add NXP PTN3222 eUSB2 to USB2 redriver
On Fri, 6 Sept 2024 at 11:40, Song Xue <quic_songxue@...cinc.com> wrote:
>
>
>
> On 8/31/2024 7:45 AM, Dmitry Baryshkov wrote:
> > On Sat, 31 Aug 2024 at 02:13, Song Xue <quic_songxue@...cinc.com> wrote:
> >> On 8/30/2024 4:20 PM, Dmitry Baryshkov wrote:
> >>> The NXP PTN3222 is the single-port eUSB2 to USB2 redriver that performs
> >>> translation between eUSB2 and USB2 signalling schemes. It supports all
> >>> three data rates: Low Speed, Full Speed and High Speed.
> >>>
> >>> The reset state enables autonegotiation of the PHY role and of the data
> >>> rate, so no additional programming is required.
> >>>
> >>> Reviewed-by: Neil Armstrong <neil.armstrong@...aro.org>
> >>> Tested-by: Konrad Dybcio <konradybcio@...nel.org>
> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> >>> ---
> >>> drivers/phy/Kconfig | 11 ++++
> >>> drivers/phy/Makefile | 1 +
> >>> drivers/phy/phy-nxp-ptn3222.c | 123 ++++++++++++++++++++++++++++++++++++++++++
> >>> 3 files changed, 135 insertions(+)
> >
> > [trimmed]
> >
> >>> +
> >>> +MODULE_DESCRIPTION("NXP PTN3222 eUSB2 Redriver driver");
> >>> +MODULE_LICENSE("GPL");
> >>>
> >> The I2C driver just realizes the function on reset and PWR. What about
> >> other I2C driver function like I2C interface operations,
> >
> > I don't quite understand what you mean by this. Could you please clarify?
> >
> >> auto-suspend,
> >
> > I think you mean pm_runtime here. It's a valid case, but granted that
> > it should stay enabled when USB controller is enabled, the gain should
> > be pretty limited. I'll consider a followup patch implementing
> > pm_runtime for the sake of being able to disable I2C host if DWC3
> > controller disables the PHY.
> >
> >> remote wakeup,
> >
> > Not supported by design. PTN3222 doesn't have IRQ pins to report
> > events to the host.
> >
> >> memory maps etc.
> >
> > huh?
> >
> >> Who will enable these? I think it is not
> >> incomplete I2C driver, if on someday, ptn3222 is used as I2C device.
> >
> > Well, I'm using it as an I2C device.
> >
> Sorry for the delayed response.
> The functions I listed, such as auto-suspend and wake-up, are just
> examples. My main point is that a basic I2C driver should include
> fundamental functions like setting up the I2C bus, configuring the
> clock, and setting the SDA (data line) and SCL (clock line). A basic I2C
> driver shouldn’t be limited to enabling the power supply and reset pin,
> as these features can be handled by other drivers as well.
> If you implement these fundamental functions, I think it will be sufficient.
I think you have mixed two things. You are describing an I2C bus
device, which PTN3222 isn't. I2C clients do not have to setup
anything. SDA/SCL and clock frequency are handled by the I2C bus
drivers and by the I2C framework.
--
With best wishes
Dmitry
Powered by blists - more mailing lists