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

[Rosen Penev]

On Mon, Apr 13, 2026 at 10:51 AM Justin Chen <justin.chen@xxxxxxxxxxxx> wrote:
>
>
>
> 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?
Of the allocation, yes since we need the size.

Of the dev_info, no. I can move it back.
>
> 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);
>