Re: [PATCH v5 64/75] usb: add helpers needed by the resynced DWC3 code

[Mattijs Korpershoek <mkorpershoek@kernel.org>]

Hi Anders,

Thank you for the patch.

On Thu, Jul 16, 2026 at 15:42, Anders Roxell <anders.roxell@linaro.org> wrote:

Patch title should include dm: in prefix, since we are touching driver model

> The resynced DWC3 code calls a few helpers that u-boot did not have.
> Add them first so the next commits can use them.
>
> Co-developed-by: Jerome Forissier <jerome.forissier@linaro.org>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> Co-developed-by: Jens Wiklander <jens.wiklander@linaro.org>
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
>  include/dm/device_compat.h | 13 +++++++++
>  include/dm/read.h          | 51 ++++++++++++++++++++++++++++++++++
>  include/linux/compat.h     | 15 ++++++++++
>  include/linux/usb/phy.h    | 56 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 135 insertions(+)
>
> diff --git a/include/dm/device_compat.h b/include/dm/device_compat.h
> index aa9a6fbb5e3f..6b31a72768fd 100644
> --- a/include/dm/device_compat.h
> +++ b/include/dm/device_compat.h
> @@ -119,4 +119,17 @@
>  #define dev_vdbg(dev, fmt, ...) \
>  	__dev_printk(LOGL_DEBUG_CONTENT, dev, fmt, ##__VA_ARGS__)
>  
> +#define dev_err_probe(dev, err, fmt, ...) \
> +	({ \
> +		int _err = (err); \
> +		if (_err != -EPROBE_DEFER) { \
> +			dev_err(dev, fmt, ##__VA_ARGS__); \
> +			dev_err(dev, "[err=%d]", _err); \
> +		} else { \
> +			dev_dbg(dev, fmt, ##__VA_ARGS__); \
> +			dev_dbg(dev, "[err=%d]", _err); \
> +		} \
> +		_err; \
> +	 })
> +
>  #endif
> diff --git a/include/dm/read.h b/include/dm/read.h
> index 12dcde6645c7..4a34afa81962 100644
> --- a/include/dm/read.h
> +++ b/include/dm/read.h
> @@ -1273,6 +1273,57 @@ static inline phy_interface_t dev_read_phy_mode(const struct udevice *dev)
>  
>  #endif /* CONFIG_DM_DEV_READ_INLINE */
>  
> +static inline int dev_count_u32(const struct udevice *dev,
> +					 const char *name)

$ ./scripts/checkpatch.pl --git
52e90cc8ccac0882f2ea160a731ebfe24adfa22a^..52e90cc8ccac0882f2ea160a731ebfe24adfa22a
seems to complain here:

CHECK: Alignment should match open parenthesis
#54: FILE: include/dm/read.h:1277:
+static inline int dev_count_u32(const struct udevice *dev,
+                                        const char *name)

We also see some other warnings later:

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#179: FILE: include/linux/usb/phy.h:59:
+                            unsigned mA);

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#199: FILE: include/linux/usb/phy.h:79:
+usb_phy_set_power(struct usb_phy *x, unsigned mA)

total: 0 errors, 2 warnings, 3 checks, 171 lines checked


> +{
> +	return dev_read_u32_array(dev, name, NULL, 0);
> +}
> +
> +/* Linux compatibility */
> +
> +#define device_property_count_u32 dev_count_u32
> +#define device_property_read_bool dev_read_bool
> +#define device_property_read_u16 dev_read_u16
> +#define device_property_read_u32_array dev_read_u32_array
> +#define device_property_read_u32 dev_read_u32
> +#define device_property_read_u8 dev_read_u8
> +
> +static inline int device_property_read_string(const struct udevice *dev,
> +					      const char *propname,
> +					      const char **val)
> +{
> +	*val = dev_read_string(dev, propname);
> +	if (!*val)
> +		return -ENOENT;
> +	return 0;
> +}
> +
> +static inline bool device_property_present(const struct udevice *dev,
> +					   const char *propname)
> +{
> +	return (dev_read_size(dev, propname) > 0);
> +}
> +
> +static inline int device_property_count_u8(const struct udevice *dev,
> +					   const char *propname)
> +{
> +	return dev_read_size(dev, propname);
> +}
> +
> +static inline int device_property_read_u8_array(const struct udevice *dev,
> +						const char *propname,
> +						u8 *out, size_t count)
> +{
> +	const u8 *ptr = dev_read_u8_array_ptr(dev, propname, count);
> +
> +	if (!ptr)
> +		return -EINVAL;
> +
> +	memcpy(out, ptr, count);
> +
> +	return 0;
> +}
> +
>  /**
>   * dev_for_each_subnode() - Helper function to iterate through subnodes
>   *
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index d4ba4d0088a0..16c02260c111 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -297,6 +297,7 @@ struct rw_semaphore { int i; };
>  #define up_write(...)			do { } while (0)
>  #define down_read(...)			do { } while (0)
>  #define up_read(...)			do { } while (0)
> +struct device_node;
>  struct device {
>  	struct device		*parent;
>  	struct class		*class;
> @@ -305,6 +306,7 @@ struct device {
>  	/* This is used from drivers/usb/musb-new subsystem only */
>  	void		*driver_data;	/* data private to the driver */
>  	void            *device_data;   /* data private to the device */
> +	struct device_node	*of_node; /* associated device tree node */
>  };
>  struct mutex { int i; };
>  struct kernel_param { int i; };
> @@ -402,4 +404,17 @@ typedef unsigned long dmaaddr_t;
>  #define free_irq(irq, data) do {} while (0)
>  #define request_irq(nr, f, flags, nm, data) 0
>  
> +/* From include/linux/reset.h */
> +
> +struct reset_control;
> +
> +static inline int reset_control_assert(struct reset_control *rstc)
> +{
> +	return 0;
> +}
> +
> +static inline int reset_control_deassert(struct reset_control *rstc)
> +{
> +	return 0;
> +}
>  #endif
> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
> index 14b2c7eb2e63..de1d0c82076f 100644
> --- a/include/linux/usb/phy.h
> +++ b/include/linux/usb/phy.h
> @@ -21,6 +21,13 @@ enum usb_phy_interface {
>  	USBPHY_INTERFACE_MODE_HSIC,
>  };
>  
> +/* associate a type with PHY */
> +enum usb_phy_type {
> +	USB_PHY_TYPE_UNDEFINED,
> +	USB_PHY_TYPE_USB2,
> +	USB_PHY_TYPE_USB3,
> +};

Can we splease split out the PHY parts to a separate patch?

It will make it easier to review.

Also, please mention from which linux version these code bits have been
imported from.


> +
>  #if CONFIG_IS_ENABLED(DM_USB)
>  /**
>   * usb_get_phy_mode - Get phy mode for given device_node
> @@ -37,4 +44,53 @@ static inline enum usb_phy_interface usb_get_phy_mode(ofnode node)
>  }
>  #endif
>  
> +struct usb_phy {
> +	struct device		*dev;
> +
> +	/* initialize/shutdown the phy */
> +	int     (*init)(struct usb_phy *x);
> +	void    (*shutdown)(struct usb_phy *x);
> +
> +	/* enable/disable VBUS */
> +	int     (*set_vbus)(struct usb_phy *x, int on);
> +
> +	/* effective for B devices, ignored for A-peripheral */
> +	int     (*set_power)(struct usb_phy *x,
> +			     unsigned mA);
> +};
> +
> +static inline int
> +usb_phy_init(struct usb_phy *x)
> +{
> +	if (x && x->init)
> +		return x->init(x);
> +
> +	return 0;
> +}
> +
> +static inline void
> +usb_phy_shutdown(struct usb_phy *x)
> +{
> +	if (x && x->shutdown)
> +		x->shutdown(x);
> +}
> +
> +static inline int
> +usb_phy_set_power(struct usb_phy *x, unsigned mA)
> +{
> +	if (!x)
> +		return 0;
> +
> +	/* TODO usb_phy_set_charger_current(x, mA); */

I'm ok for this being a TODO, but I'd like to see a justification of why
it's commented?

> +
> +	if (x->set_power)
> +		return x->set_power(x, mA);
> +	return 0;
> +}
> +
> +static inline int
> +usb_phy_set_suspend(struct usb_phy *x, int suspend)
> +{
> +	return 0;
> +}
>  #endif /* __LINUX_USB_PHY_H */
> -- 
> 2.53.0