Re: [PATCH] usb: bdc: allocate phys with main struct

[Justin Chen]

On 4/10/26 9:12 PM, Rosen Penev wrote:
> Use a flexible array member to combine allocations and simplify code
> slightly. No need for a branch deciding whether to allocate or not.
>
> Add __counted_by for extra runtime analysis.
>
> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
> ---
>   drivers/usb/gadget/udc/bdc/bdc.h      |  2 +-
>   drivers/usb/gadget/udc/bdc/bdc_core.c | 20 +++++++-------------
>   2 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/bdc/bdc.h b/drivers/usb/gadget/udc/bdc/bdc.h
> index 2f4abf6f8f77..cc961161eb46 100644
> --- a/drivers/usb/gadget/udc/bdc/bdc.h
> +++ b/drivers/usb/gadget/udc/bdc/bdc.h
> @@ -409,7 +409,6 @@ struct bdc {
>   	spinlock_t	lock;
>   
>   	/* generic phy */
> -	struct phy      **phys;
>   	int num_phys;
>   	/* num of endpoints for a particular instantiation of IP */
>   	unsigned int num_eps;
> @@ -453,6 +452,7 @@ struct bdc {
>   	 */
>   	struct delayed_work	func_wake_notify;
>   	struct clk		*clk;
> +	struct phy		*phys[] __counted_by(num_phys);
>   };
>   
>   static inline u32 bdc_readl(void __iomem *base, u32 offset)
> diff --git a/drivers/usb/gadget/udc/bdc/bdc_core.c b/drivers/usb/gadget/udc/bdc/bdc_core.c
> index 438201dc96ca..4b16b85da450 100644
> --- a/drivers/usb/gadget/udc/bdc/bdc_core.c
> +++ b/drivers/usb/gadget/udc/bdc/bdc_core.c
> @@ -487,14 +487,20 @@ static int bdc_probe(struct platform_device *pdev)
>   	int irq;
>   	u32 temp;
>   	struct device *dev = &pdev->dev;
> +	int num_phys;
>   	int phy_num;
>   
>   	dev_dbg(dev, "%s()\n", __func__);
>   
> -	bdc = devm_kzalloc(dev, sizeof(*bdc), GFP_KERNEL);
> +	num_phys = of_count_phandle_with_args(dev->of_node,
> +						"phys", "#phy-cells");
> +	bdc = devm_kzalloc(dev, struct_size(bdc, phys, num_phys), GFP_KERNEL);
>   	if (!bdc)
>   		return -ENOMEM;
>   
> +	bdc->num_phys = num_phys;
> +	dev_info(dev, "Using %d phy(s)\n", bdc->num_phys);
> +

This feels like a step sideways instead of an improvement IMHO. And we 
are also moving the allocation and dev_info() print. Is there a reason 
to change the ordering?

Justin

>   	bdc->regs = devm_platform_ioremap_resource(pdev, 0);
>   	if (IS_ERR(bdc->regs))
>   		return PTR_ERR(bdc->regs);
> @@ -508,18 +514,6 @@ static int bdc_probe(struct platform_device *pdev)
>   	bdc->dev = dev;
>   	dev_dbg(dev, "bdc->regs: %p irq=%d\n", bdc->regs, bdc->irq);
>   
> -	bdc->num_phys = of_count_phandle_with_args(dev->of_node,
> -						"phys", "#phy-cells");
> -	if (bdc->num_phys > 0) {
> -		bdc->phys = devm_kcalloc(dev, bdc->num_phys,
> -					sizeof(struct phy *), GFP_KERNEL);
> -		if (!bdc->phys)
> -			return -ENOMEM;
> -	} else {
> -		bdc->num_phys = 0;
> -	}
> -	dev_info(dev, "Using %d phy(s)\n", bdc->num_phys);
> -
>   	for (phy_num = 0; phy_num < bdc->num_phys; phy_num++) {
>   		bdc->phys[phy_num] = devm_of_phy_get_by_index(
>   			dev, dev->of_node, phy_num);