[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9b58a985-3d63-42bb-9a76-e5b04a4b6012@kernel.org>
Date: Sun, 16 Feb 2025 10:44:08 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>,
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>, Alim Akhtar <alim.akhtar@...sung.com>,
Philipp Zabel <p.zabel@...gutronix.de>
Cc: linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org,
linux-phy@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 3/4] phy: samsung: add Exynos2200 SNPS eUSB2 driver
On 16/02/2025 10:41, Ivaylo Ivanov wrote:
> On 2/16/25 11:26, Krzysztof Kozlowski wrote:
>> On 15/02/2025 13:24, Ivaylo Ivanov wrote:
>>> The Exynos2200 SoC uses Synopsis eUSB2 PHY for USB 2.0. Add a new
>>> driver for it.
>>>
>>> eUSB2 on Exynos SoCs is usually paired alongside a USB PHY controller.
>>> Currently the driver is modelled to take and enable/disable the usb phy
>>> controller when needed.
>>>
>>> The driver is based on information from downstream drivers.
>>>
>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>
>>> ---
>>> drivers/phy/samsung/Kconfig | 13 +
>>> drivers/phy/samsung/Makefile | 1 +
>>> .../phy/samsung/phy-exynos2200-snps-eusb2.c | 351 ++++++++++++++++++
>>> 3 files changed, 365 insertions(+)
>>> create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c
>>>
>>> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
>>> index e2330b089..f62285254 100644
>>> --- a/drivers/phy/samsung/Kconfig
>>> +++ b/drivers/phy/samsung/Kconfig
>>> @@ -77,6 +77,19 @@ config PHY_S5PV210_USB2
>>> particular SoC is compiled in the driver. In case of S5PV210 two phys
>>> are available - device and host.
>>>
>>> +config PHY_EXYNOS2200_SNPS_EUSB2
>>> + tristate "Exynos2200 eUSB 2.0 PHY driver"
>>> + depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
>>> + depends on HAS_IOMEM
>>> + depends on USB_DWC3_EXYNOS
>>
>> How does it depend? What are you using from DWC3?
>
> Can drop, I guess.
>
>>
>>> + select GENERIC_PHY
>>> + select MFD_SYSCON
>> Where do you use it?
>
> Remained from USBCON driver.
>
>>
>>> + default y
>>> + help
>>> + Enable USBCON PHY support for Exynos2200 SoC.
>>> + This driver provides PHY interface for eUSB 2.0 controller
>>> + present on Exynos5 SoC series.
>>> +
>>> config PHY_EXYNOS5_USBDRD
>>> tristate "Exynos5 SoC series USB DRD PHY driver"
>>> depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
>>> diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile
>>> index fea1f96d0..90b84c7fc 100644
>>> --- a/drivers/phy/samsung/Makefile
>>> +++ b/drivers/phy/samsung/Makefile
>>> @@ -14,5 +14,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o
>>> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o
>>> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o
>>> phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2) += phy-s5pv210-usb2.o
>>> +obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2) += phy-exynos2200-snps-eusb2.o
>> Entire driver looks like repeating existing qcom-snps-eusb2.
>
> It's the same IP, but implemented differently on a different platform. At
> the very least, the register layout is different.
I checked few registers, looked very the same. Same blocks from synopsys
have the common register layouts.
>
>> You need to
>> integrate the changes, not create duplicated driver.
>
> I can do that, but it would be come a bit cluttered, won't it? Depends on
> if we want to follow the current oem-provided initialization sequence, or
> try and fully reuse what we have in there.
I think it duplicates a lot, so it won't be clutter. We have many
drivers having common code and per-variant ops.
Best regards,
Krzysztof
Powered by blists - more mailing lists