[PATCH] phy: omap-usb2: add explicit PHY comparator API

[PATCH] phy: omap-usb2: add explicit PHY comparator API

[Ivaylo Dimitrov]

The existing omap_usb2_set_comparator() API locates the OMAP USB2 PHY
instance by calling usb_get_phy(USB_PHY_TYPE_USB2), assuming there is a
single USB2 PHY registered in the system.

This assumption no longer holds on systems with multiple USB2 PHY
providers. In such cases, the global lookup may return a different USB2
PHY instance, causing the OMAP driver to perform an invalid container_of()
conversion when accessing its private data.

Introduce omap_usb2_set_phy_comparator(), allowing callers to explicitly
specify the OMAP USB2 PHY instance to associate with a phy_companion.
The new helper validates that the supplied USB PHY is an OMAP USB2 PHY
before accessing its private data.

The existing omap_usb2_set_comparator() API is kept unchanged for
existing users.


Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 drivers/phy/ti/phy-omap-usb2.c | 32 ++++++++++++++++++++++++++++++++
 include/linux/phy/omap_usb.h   |  7 +++++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/phy/ti/phy-omap-usb2.c b/drivers/phy/ti/phy-omap-usb2.c
index 1eb252604441..ac31b693fce7 100644
--- a/drivers/phy/ti/phy-omap-usb2.c
+++ b/drivers/phy/ti/phy-omap-usb2.c
@@ -70,6 +70,8 @@ struct omap_usb {
 
 #define	phy_to_omapusb(x)	container_of((x), struct omap_usb, phy)
 
+static int omap_usb_set_host(struct usb_otg *otg, struct usb_bus *host);
+
 struct usb_phy_data {
 	const char *label;
 	u8 flags;
@@ -113,6 +115,36 @@ int omap_usb2_set_comparator(struct phy_companion *comparator)
 }
 EXPORT_SYMBOL_GPL(omap_usb2_set_comparator);
 
+/**
+ * omap_usb2_set_phy_comparator() - associate a comparator with an OMAP USB2 PHY
+ *
+ * @omap_phy: OMAP USB2 PHY instance
+ * @comparator: companion PHY providing comparator operations
+ *
+ * The phy companion driver should call this API with the companion PHY
+ * implementation containing the callbacks required by @omap_phy.
+ *
+ * For use by phy companion drivers.
+ */
+int omap_usb2_set_phy_comparator(struct usb_phy *omap_phy,
+				 struct phy_companion *comparator)
+{
+	struct omap_usb	*phy;
+
+	if (!omap_phy)
+		return -EINVAL;
+
+	if (WARN_ON(!omap_phy->otg ||
+		    omap_phy->otg->set_host != omap_usb_set_host))
+		return -EINVAL;
+
+	phy = phy_to_omapusb(omap_phy);
+	phy->comparator = comparator;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(omap_usb2_set_phy_comparator);
+
 static int omap_usb_set_vbus(struct usb_otg *otg, bool enabled)
 {
 	struct omap_usb *phy = phy_to_omapusb(otg->usb_phy);
diff --git a/include/linux/phy/omap_usb.h b/include/linux/phy/omap_usb.h
index e23b52df93ec..96f9792c6ce6 100644
--- a/include/linux/phy/omap_usb.h
+++ b/include/linux/phy/omap_usb.h
@@ -15,11 +15,18 @@
 
 #if defined(CONFIG_OMAP_USB2) || defined(CONFIG_OMAP_USB2_MODULE)
 extern int omap_usb2_set_comparator(struct phy_companion *comparator);
+extern int omap_usb2_set_phy_comparator(struct usb_phy *omap_phy,
+					struct phy_companion *comparator);
 #else
 static inline int omap_usb2_set_comparator(struct phy_companion *comparator)
 {
 	return -ENODEV;
 }
+static inline int omap_usb2_set_phy_comparator(struct usb_phy *omap_phy,
+					       struct phy_companion *comparator)
+{
+	return -ENODEV;
+}
 #endif
 
 #endif /* __DRIVERS_OMAP_USB_H */
-- 
2.39.5


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

Re: [PATCH] phy: omap-usb2: add explicit PHY comparator API

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Type validation relies on function pointer equality, which is unsafe under Identical Code Folding (ICF).

Pre-existing issues:
- [High] A Time-Of-Check to Time-Of-Use (TOCTOU) race condition exists when accessing `phy->comparator`.
- [High] The legacy `omap_usb2_set_comparator()` function remains vulnerable to an invalid `container_of()` conversion on multi-PHY systems.
--

--- Patch [1]: [PATCH] phy: omap-usb2: add explicit PHY comparator API ---
commit 4b00e72aaba66e3ef6417d57ee7a7916dac1cd3d
Author: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>

phy: omap-usb2: add explicit PHY comparator API

The existing omap_usb2_set_comparator() API assumes a single USB2 PHY,
leading to invalid container_of() casts on multi-PHY systems. This commit
introduces omap_usb2_set_phy_comparator() for explicit PHY instance
association while keeping the legacy API intact for existing users.

> diff --git a/drivers/phy/ti/phy-omap-usb2.c b/drivers/phy/ti/phy-omap-usb2.c
> index 1eb252604441b..ac31b693fce7b 100644
> --- a/drivers/phy/ti/phy-omap-usb2.c
> +++ b/drivers/phy/ti/phy-omap-usb2.c
[ ... ]
> @@ -113,6 +115,36 @@ int omap_usb2_set_comparator(struct phy_companion *comparator)
>  }
>  EXPORT_SYMBOL_GPL(omap_usb2_set_comparator);

[Severity: High]
This is a pre-existing issue, but does leaving this legacy API without type
validation defeat the purpose of fixing the multi-PHY issue?

If an existing caller on a multi-PHY system uses this API and usb_get_phy()
returns a non-OMAP PHY, it will still perform an invalid container_of() cast:

drivers/phy/ti/phy-omap-usb2.c:omap_usb2_set_comparator() {
    ...
    struct usb_phy *x = usb_get_phy(USB_PHY_TYPE_USB2);
    ...
    phy = phy_to_omapusb(x);
    phy->comparator = comparator;
    ...
}

Could this lead to arbitrary memory corruption when phy->comparator is
overwritten on a foreign PHY structure?

> +/**
> + * omap_usb2_set_phy_comparator() - associate a comparator with an OMAP USB2 PHY
> + *
> + * @omap_phy: OMAP USB2 PHY instance
> + * @comparator: companion PHY providing comparator operations
> + *
> + * The phy companion driver should call this API with the companion PHY
> + * implementation containing the callbacks required by @omap_phy.
> + *
> + * For use by phy companion drivers.
> + */
> +int omap_usb2_set_phy_comparator(struct usb_phy *omap_phy,
> +				 struct phy_companion *comparator)
> +{
> +	struct omap_usb	*phy;
> +
> +	if (!omap_phy)
> +		return -EINVAL;
> +
> +	if (WARN_ON(!omap_phy->otg ||
> +		    omap_phy->otg->set_host != omap_usb_set_host))
> +		return -EINVAL;

[Severity: Medium]
Is it safe to rely on function pointer equality for type validation here?

If the kernel is built with Link Time Optimization and Identical Code Folding
(e.g., Clang LTO --icf=all), omap_usb_set_host could be folded into the
same memory address as a structurally identical set_host implementation
in another PHY driver.

Under these conditions, could this check falsely pass for an incorrect PHY,
leading to an invalid container_of() cast?

> +
> +	phy = phy_to_omapusb(omap_phy);
> +	phy->comparator = comparator;

[Severity: High]
This is a pre-existing issue, but does modifying the shared phy->comparator
pointer locklessly create a race condition with OTG events?

Looking at omap_usb_set_vbus() and omap_usb_start_srp(), the comparator
pointer is checked and then dereferenced without locks or READ_ONCE():

drivers/phy/ti/phy-omap-usb2.c:omap_usb_set_vbus() {
    ...
    if (!phy->comparator || !phy->comparator->set_vbus)
        return -ENODEV;

    return phy->comparator->set_vbus(phy->comparator, enabled);
}

If an OTG event occurs concurrently with a device unbind that clears the
comparator pointer, could this result in a use-after-free or NULL pointer
dereference?

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(omap_usb2_set_phy_comparator);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722053618.602702-1-ivo.g.dimitrov.75@gmail.com?part=1

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

Re: [PATCH] phy: omap-usb2: add explicit PHY comparator API

[Ivaylo Dimitrov]

Gentle ping on this patch. It's been about five weeks since v1, and I 
haven't seen any feedback yet.

The patch is still applicable to current linux-next. Please let me know 
if the approach looks reasonable, or if there are any concerns I should 
address.

Thanks,
Ivo

On 22.07.26 г. 8:36 ч., Ivaylo Dimitrov wrote:
> The existing omap_usb2_set_comparator() API locates the OMAP USB2 PHY
> instance by calling usb_get_phy(USB_PHY_TYPE_USB2), assuming there is a
> single USB2 PHY registered in the system.
> 
> This assumption no longer holds on systems with multiple USB2 PHY
> providers. In such cases, the global lookup may return a different USB2
> PHY instance, causing the OMAP driver to perform an invalid container_of()
> conversion when accessing its private data.
> 
> Introduce omap_usb2_set_phy_comparator(), allowing callers to explicitly
> specify the OMAP USB2 PHY instance to associate with a phy_companion.
> The new helper validates that the supplied USB PHY is an OMAP USB2 PHY
> before accessing its private data.
> 
> The existing omap_usb2_set_comparator() API is kept unchanged for
> existing users.
> 
> 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> ---
>   drivers/phy/ti/phy-omap-usb2.c | 32 ++++++++++++++++++++++++++++++++
>   include/linux/phy/omap_usb.h   |  7 +++++++
>   2 files changed, 39 insertions(+)
> 
> diff --git a/drivers/phy/ti/phy-omap-usb2.c b/drivers/phy/ti/phy-omap-usb2.c
> index 1eb252604441..ac31b693fce7 100644
> --- a/drivers/phy/ti/phy-omap-usb2.c
> +++ b/drivers/phy/ti/phy-omap-usb2.c
> @@ -70,6 +70,8 @@ struct omap_usb {
>   
>   #define	phy_to_omapusb(x)	container_of((x), struct omap_usb, phy)
>   
> +static int omap_usb_set_host(struct usb_otg *otg, struct usb_bus *host);
> +
>   struct usb_phy_data {
>   	const char *label;
>   	u8 flags;
> @@ -113,6 +115,36 @@ int omap_usb2_set_comparator(struct phy_companion *comparator)
>   }
>   EXPORT_SYMBOL_GPL(omap_usb2_set_comparator);
>   
> +/**
> + * omap_usb2_set_phy_comparator() - associate a comparator with an OMAP USB2 PHY
> + *
> + * @omap_phy: OMAP USB2 PHY instance
> + * @comparator: companion PHY providing comparator operations
> + *
> + * The phy companion driver should call this API with the companion PHY
> + * implementation containing the callbacks required by @omap_phy.
> + *
> + * For use by phy companion drivers.
> + */
> +int omap_usb2_set_phy_comparator(struct usb_phy *omap_phy,
> +				 struct phy_companion *comparator)
> +{
> +	struct omap_usb	*phy;
> +
> +	if (!omap_phy)
> +		return -EINVAL;
> +
> +	if (WARN_ON(!omap_phy->otg ||
> +		    omap_phy->otg->set_host != omap_usb_set_host))
> +		return -EINVAL;
> +
> +	phy = phy_to_omapusb(omap_phy);
> +	phy->comparator = comparator;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(omap_usb2_set_phy_comparator);
> +
>   static int omap_usb_set_vbus(struct usb_otg *otg, bool enabled)
>   {
>   	struct omap_usb *phy = phy_to_omapusb(otg->usb_phy);
> diff --git a/include/linux/phy/omap_usb.h b/include/linux/phy/omap_usb.h
> index e23b52df93ec..96f9792c6ce6 100644
> --- a/include/linux/phy/omap_usb.h
> +++ b/include/linux/phy/omap_usb.h
> @@ -15,11 +15,18 @@
>   
>   #if defined(CONFIG_OMAP_USB2) || defined(CONFIG_OMAP_USB2_MODULE)
>   extern int omap_usb2_set_comparator(struct phy_companion *comparator);
> +extern int omap_usb2_set_phy_comparator(struct usb_phy *omap_phy,
> +					struct phy_companion *comparator);
>   #else
>   static inline int omap_usb2_set_comparator(struct phy_companion *comparator)
>   {
>   	return -ENODEV;
>   }
> +static inline int omap_usb2_set_phy_comparator(struct usb_phy *omap_phy,
> +					       struct phy_companion *comparator)
> +{
> +	return -ENODEV;
> +}
>   #endif
>   
>   #endif /* __DRIVERS_OMAP_USB_H */

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

Re: [PATCH] phy: omap-usb2: add explicit PHY comparator API

[Vinod Koul]

On 22-07-26, 08:36, Ivaylo Dimitrov wrote:
> The existing omap_usb2_set_comparator() API locates the OMAP USB2 PHY
> instance by calling usb_get_phy(USB_PHY_TYPE_USB2), assuming there is a
> single USB2 PHY registered in the system.
> 
> This assumption no longer holds on systems with multiple USB2 PHY
> providers. In such cases, the global lookup may return a different USB2
> PHY instance, causing the OMAP driver to perform an invalid container_of()
> conversion when accessing its private data.
> 
> Introduce omap_usb2_set_phy_comparator(), allowing callers to explicitly
> specify the OMAP USB2 PHY instance to associate with a phy_companion.
> The new helper validates that the supplied USB PHY is an OMAP USB2 PHY
> before accessing its private data.

Well the name can cause ambiguity with old API. Second I would like to
see users of this API as well

> 
> The existing omap_usb2_set_comparator() API is kept unchanged for
> existing users.
> 
> 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> ---
>  drivers/phy/ti/phy-omap-usb2.c | 32 ++++++++++++++++++++++++++++++++
>  include/linux/phy/omap_usb.h   |  7 +++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/drivers/phy/ti/phy-omap-usb2.c b/drivers/phy/ti/phy-omap-usb2.c
> index 1eb252604441..ac31b693fce7 100644
> --- a/drivers/phy/ti/phy-omap-usb2.c
> +++ b/drivers/phy/ti/phy-omap-usb2.c
> @@ -70,6 +70,8 @@ struct omap_usb {
>  
>  #define	phy_to_omapusb(x)	container_of((x), struct omap_usb, phy)
>  
> +static int omap_usb_set_host(struct usb_otg *otg, struct usb_bus *host);
> +
>  struct usb_phy_data {
>  	const char *label;
>  	u8 flags;
> @@ -113,6 +115,36 @@ int omap_usb2_set_comparator(struct phy_companion *comparator)
>  }
>  EXPORT_SYMBOL_GPL(omap_usb2_set_comparator);
>  
> +/**
> + * omap_usb2_set_phy_comparator() - associate a comparator with an OMAP USB2 PHY
> + *
> + * @omap_phy: OMAP USB2 PHY instance
> + * @comparator: companion PHY providing comparator operations
> + *
> + * The phy companion driver should call this API with the companion PHY
> + * implementation containing the callbacks required by @omap_phy.
> + *
> + * For use by phy companion drivers.
> + */
> +int omap_usb2_set_phy_comparator(struct usb_phy *omap_phy,
> +				 struct phy_companion *comparator)
> +{
> +	struct omap_usb	*phy;
> +
> +	if (!omap_phy)
> +		return -EINVAL;
> +
> +	if (WARN_ON(!omap_phy->otg ||
> +		    omap_phy->otg->set_host != omap_usb_set_host))
> +		return -EINVAL;
> +
> +	phy = phy_to_omapusb(omap_phy);
> +	phy->comparator = comparator;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(omap_usb2_set_phy_comparator);
> +
>  static int omap_usb_set_vbus(struct usb_otg *otg, bool enabled)
>  {
>  	struct omap_usb *phy = phy_to_omapusb(otg->usb_phy);
> diff --git a/include/linux/phy/omap_usb.h b/include/linux/phy/omap_usb.h
> index e23b52df93ec..96f9792c6ce6 100644
> --- a/include/linux/phy/omap_usb.h
> +++ b/include/linux/phy/omap_usb.h
> @@ -15,11 +15,18 @@
>  
>  #if defined(CONFIG_OMAP_USB2) || defined(CONFIG_OMAP_USB2_MODULE)
>  extern int omap_usb2_set_comparator(struct phy_companion *comparator);
> +extern int omap_usb2_set_phy_comparator(struct usb_phy *omap_phy,
> +					struct phy_companion *comparator);
>  #else
>  static inline int omap_usb2_set_comparator(struct phy_companion *comparator)
>  {
>  	return -ENODEV;
>  }
> +static inline int omap_usb2_set_phy_comparator(struct usb_phy *omap_phy,
> +					       struct phy_companion *comparator)
> +{
> +	return -ENODEV;
> +}
>  #endif
>  
>  #endif /* __DRIVERS_OMAP_USB_H */
> -- 
> 2.39.5

-- 
~Vinod

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

Re: [PATCH] phy: omap-usb2: add explicit PHY comparator API

[Ivaylo Dimitrov]

On 27.08.26 г. 18:51 ч., Vinod Koul wrote:
> On 22-07-26, 08:36, Ivaylo Dimitrov wrote:
>> The existing omap_usb2_set_comparator() API locates the OMAP USB2 PHY
>> instance by calling usb_get_phy(USB_PHY_TYPE_USB2), assuming there is a
>> single USB2 PHY registered in the system.
>>
>> This assumption no longer holds on systems with multiple USB2 PHY
>> providers. In such cases, the global lookup may return a different USB2
>> PHY instance, causing the OMAP driver to perform an invalid container_of()
>> conversion when accessing its private data.
>>
>> Introduce omap_usb2_set_phy_comparator(), allowing callers to explicitly
>> specify the OMAP USB2 PHY instance to associate with a phy_companion.
>> The new helper validates that the supplied USB PHY is an OMAP USB2 PHY
>> before accessing its private data.
> 
> Well the name can cause ambiguity with old API. Second I would like to
> see users of this API as well
> 

Would `omap_usb2_set_comparator_for_phy()` be clearer? Or could you
suggest a better name?

Regarding the user of the API, let me provide some background.

Currently, the cpcap-charger driver uses the old 
omap_usb2_set_comparator() API. On mapphone devices there are two
USB-PHY devices, phy_omap_usb2 and phy_cpcap_usb. The old API selects
the first USB2 PHY probed, which is not guaranteed to be the OMAP PHY.

I have also added DCP detection and extcon support to the CPCAP PHY, and
I'm working on proper charging current limiting in cpcap-charger. For 
that, the charger driver needs to be able to explicitly select the OMAP
PHY from DT and register for its PHY events, in order to properly limit
the current in case of DCP/gadget/OTG connection.

With the DT configuration, cpcap-charger has the specific PHY instance
available and can use the new API. For legacy configurations without a
PHY specified in DT, it will continue to use the existing API.

The relevant changes are:

DCP detection/extcon support in the CPCAP PHY:
https://lkml.org/lkml/2026/7/11/600

`cpcap-charger` changes (currently out-of-tree):
https://git.maemo.org/leste-upstream-forks/droid4-linux/commit/0261cbb724d875ee4aa2e5af34caead39e5e47fb

I initially kept the cpcap-charger change separate since the two
patches look somewhat unrelated. I also wanted to see whether the
approach in this patch is acceptable in principle before preparing the
cpcap-charger changes for upstreaming.

Would you prefer me to send a series with the $subject patch and the
cpcap-charger changes, including the required DT schema changes?

Thanks and regards,
Ivo



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