All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: "André Draszik" <andre.draszik@linaro.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>,
	"Vinod Koul" <vkoul@kernel.org>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Alim Akhtar" <alim.akhtar@samsung.com>,
	"Peter Griffin" <peter.griffin@linaro.org>,
	linux-phy@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH] phy: exynos5-usbdrd: Fix broken USB on Exynos5422 (TYPEC dependency)
Date: Sun, 16 Feb 2025 10:42:09 +0100	[thread overview]
Message-ID: <23e8f626-e512-4e80-b6b8-8ff073bd37e6@kernel.org> (raw)
In-Reply-To: <bc292f121afaacb29fdc7d600c72cb3008b6b56a.camel@linaro.org>

On 15/02/2025 17:20, André Draszik wrote:
> Hi Krzysztof,
> 
> On Sat, 2025-02-15 at 10:41 +0100, Krzysztof Kozlowski wrote:
>> Older Exynos designs, like Exynos5422, do not have USB Type-C and the
>> USB DRD PHY does not really depend on Type-C for these devices at all.
>> Incorrectly added dependency on CONFIG_TYPEC caused this driver to be
>> missing for exynos_defconfig and as result Exynos5422-based boards like
>> Hardkernel Odroid HC1 failed to probe USB.
>>
>> Drop incorrect dependency and rely on module to be reachable by the
>> compiler.
>>
>> Reported-by: Krzysztof Kozlowski <krzk@kernel.org>
>> Closes: https://krzk.eu/#/builders/21/builds/6139
>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Closes: https://lore.kernel.org/all/3c0b77e6-357d-453e-8b63-4757c3231bde@samsung.com/
>> Fixes: 09dc674295a3 ("phy: exynos5-usbdrd: subscribe to orientation notifier if required")
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> ---
>>
>> Patch for issue in linux-next
>> ---
>>  drivers/phy/samsung/Kconfig              | 1 -
>>  drivers/phy/samsung/phy-exynos5-usbdrd.c | 2 +-
>>  2 files changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
>> index 7fba571c0e2b..e2330b0894d6 100644
>> --- a/drivers/phy/samsung/Kconfig
>> +++ b/drivers/phy/samsung/Kconfig
>> @@ -81,7 +81,6 @@ config PHY_EXYNOS5_USBDRD
>>  	tristate "Exynos5 SoC series USB DRD PHY driver"
>>  	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
>>  	depends on HAS_IOMEM
>> -	depends on TYPEC || (TYPEC=n && COMPILE_TEST)
> 
> This line ensures that PHY_EXYNOS5_USBDRD changes to M if
> TYPEC is M.


I know what it does. But it is not the correct way to express optional
dependency. COMPILE_TEST makes no sense here. Unless this was not meant
to be optional dependency, but then it is wrong because none of older
(or many other) devices depend on typec.

> 
>>  	depends on USB_DWC3_EXYNOS
>>  	select GENERIC_PHY
>>  	select MFD_SYSCON
>> diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
>> index ff2436f11d68..e8a9fef22107 100644
>> --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
>> +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
>> @@ -1456,7 +1456,7 @@ static int exynos5_usbdrd_setup_notifiers(struct exynos5_usbdrd_phy *phy_drd)
>>  {
>>  	int ret;
>>  
>> -	if (!IS_ENABLED(CONFIG_TYPEC))
>> +	if (!IS_REACHABLE(CONFIG_TYPEC))
> 
> On arm64, the defconfig has TYPEC as module (while PHY_EXYNOS5_USBDRD
> defaults to y above), and therefore all following code is becomes
> disabled with your change on arm64.

In terms of defconfig, this could be fixed as simple as changing it to
module. This should be module for arm64, anyway.

In terms of users, that's indeed tricky runtime debugging issue, so
probably we need separate USBDRD_WITH_TYPEC symbol.

> 
> Can we find a different solution to unbreak arm32 and keep arm64
> defconfig working as intended?
> 
> Cheers,
> Andre'
> 


Best regards,
Krzysztof


WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <krzk@kernel.org>
To: "André Draszik" <andre.draszik@linaro.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>,
	"Vinod Koul" <vkoul@kernel.org>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Alim Akhtar" <alim.akhtar@samsung.com>,
	"Peter Griffin" <peter.griffin@linaro.org>,
	linux-phy@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH] phy: exynos5-usbdrd: Fix broken USB on Exynos5422 (TYPEC dependency)
Date: Sun, 16 Feb 2025 10:42:09 +0100	[thread overview]
Message-ID: <23e8f626-e512-4e80-b6b8-8ff073bd37e6@kernel.org> (raw)
In-Reply-To: <bc292f121afaacb29fdc7d600c72cb3008b6b56a.camel@linaro.org>

On 15/02/2025 17:20, André Draszik wrote:
> Hi Krzysztof,
> 
> On Sat, 2025-02-15 at 10:41 +0100, Krzysztof Kozlowski wrote:
>> Older Exynos designs, like Exynos5422, do not have USB Type-C and the
>> USB DRD PHY does not really depend on Type-C for these devices at all.
>> Incorrectly added dependency on CONFIG_TYPEC caused this driver to be
>> missing for exynos_defconfig and as result Exynos5422-based boards like
>> Hardkernel Odroid HC1 failed to probe USB.
>>
>> Drop incorrect dependency and rely on module to be reachable by the
>> compiler.
>>
>> Reported-by: Krzysztof Kozlowski <krzk@kernel.org>
>> Closes: https://krzk.eu/#/builders/21/builds/6139
>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Closes: https://lore.kernel.org/all/3c0b77e6-357d-453e-8b63-4757c3231bde@samsung.com/
>> Fixes: 09dc674295a3 ("phy: exynos5-usbdrd: subscribe to orientation notifier if required")
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> ---
>>
>> Patch for issue in linux-next
>> ---
>>  drivers/phy/samsung/Kconfig              | 1 -
>>  drivers/phy/samsung/phy-exynos5-usbdrd.c | 2 +-
>>  2 files changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
>> index 7fba571c0e2b..e2330b0894d6 100644
>> --- a/drivers/phy/samsung/Kconfig
>> +++ b/drivers/phy/samsung/Kconfig
>> @@ -81,7 +81,6 @@ config PHY_EXYNOS5_USBDRD
>>  	tristate "Exynos5 SoC series USB DRD PHY driver"
>>  	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
>>  	depends on HAS_IOMEM
>> -	depends on TYPEC || (TYPEC=n && COMPILE_TEST)
> 
> This line ensures that PHY_EXYNOS5_USBDRD changes to M if
> TYPEC is M.


I know what it does. But it is not the correct way to express optional
dependency. COMPILE_TEST makes no sense here. Unless this was not meant
to be optional dependency, but then it is wrong because none of older
(or many other) devices depend on typec.

> 
>>  	depends on USB_DWC3_EXYNOS
>>  	select GENERIC_PHY
>>  	select MFD_SYSCON
>> diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
>> index ff2436f11d68..e8a9fef22107 100644
>> --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
>> +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
>> @@ -1456,7 +1456,7 @@ static int exynos5_usbdrd_setup_notifiers(struct exynos5_usbdrd_phy *phy_drd)
>>  {
>>  	int ret;
>>  
>> -	if (!IS_ENABLED(CONFIG_TYPEC))
>> +	if (!IS_REACHABLE(CONFIG_TYPEC))
> 
> On arm64, the defconfig has TYPEC as module (while PHY_EXYNOS5_USBDRD
> defaults to y above), and therefore all following code is becomes
> disabled with your change on arm64.

In terms of defconfig, this could be fixed as simple as changing it to
module. This should be module for arm64, anyway.

In terms of users, that's indeed tricky runtime debugging issue, so
probably we need separate USBDRD_WITH_TYPEC symbol.

> 
> Can we find a different solution to unbreak arm32 and keep arm64
> defconfig working as intended?
> 
> Cheers,
> Andre'
> 


Best regards,
Krzysztof

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

  reply	other threads:[~2025-02-16 10:02 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-15  9:41 [PATCH] phy: exynos5-usbdrd: Fix broken USB on Exynos5422 (TYPEC dependency) Krzysztof Kozlowski
2025-02-15  9:41 ` Krzysztof Kozlowski
2025-02-15 16:20 ` André Draszik
2025-02-15 16:20   ` André Draszik
2025-02-16  9:42   ` Krzysztof Kozlowski [this message]
2025-02-16  9:42     ` Krzysztof Kozlowski
2025-02-27  5:56 ` Vinod Koul
2025-02-27  5:56   ` Vinod Koul
2025-02-27  7:02   ` Krzysztof Kozlowski
2025-02-27  7:02     ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=23e8f626-e512-4e80-b6b8-8ff073bd37e6@kernel.org \
    --to=krzk@kernel.org \
    --cc=alim.akhtar@samsung.com \
    --cc=andre.draszik@linaro.org \
    --cc=kishon@kernel.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=peter.griffin@linaro.org \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.