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

Mattijs Korpershoek <[email protected]>
Newsgroups gmane.comp.boot-loaders.u-boot
Message-ID <[email protected]>
Hi Anders,

Thank you for the patch.

On Thu, Jul 16, 2026 at 15:42, Anders Roxell <[email protected]> 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 <[email protected]>
> Signed-off-by: Jerome Forissier <[email protected]>
> Co-developed-by: Jens Wiklander <[email protected]>
> Signed-off-by: Jens Wiklander <[email protected]>
> Signed-off-by: Anders Roxell <[email protected]>
> ---
>  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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.