On Thu, Apr 9, 2026 at 8:12 AM Lee Jones <[email protected]> wrote:
>
> On Mon, 30 Mar 2026, Rosen Penev wrote:
>
> > Use a flexible array member to combine kzalloc and kcalloc. This
> > required moving the struct lpg_channel definition up as flexible array
> > members require a full definition.
> >
> > Add __counted_by for extra runtime analysis.
> >
> > Signed-off-by: Rosen Penev <[email protected]>
> > ---
> > drivers/leds/rgb/leds-qcom-lpg.c | 117 +++++++++++++++----------------
> > 1 file changed, 56 insertions(+), 61 deletions(-)
> >
> > diff --git a/drivers/leds/rgb/leds-qcom-lpg.c
> > b/drivers/leds/rgb/leds-qcom-lpg.c
> > index f6061c47f863..83cedf4a0cbf 100644
> > --- a/drivers/leds/rgb/leds-qcom-lpg.c
> > +++ b/drivers/leds/rgb/leds-qcom-lpg.c
> > @@ -80,58 +80,8 @@
> > #define SDAM_PAUSE_HI_MULTIPLIER_OFFSET 0x8
> > #define SDAM_PAUSE_LO_MULTIPLIER_OFFSET 0x9
> >
> > -struct lpg_channel;
> > struct lpg_data;
> >
> > -/**
> > - * struct lpg - LPG device context
> > - * @dev: pointer to LPG device
> > - * @map: regmap for register access
> > - * @lock: used to synchronize LED and pwm callback requests
> > - * @pwm: PWM-chip object, if operating in PWM mode
> > - * @data: reference to version specific data
> > - * @lut_base: base address of the LUT block (optional)
> > - * @lut_size: number of entries in the LUT block
> > - * @lut_bitmap: allocation bitmap for LUT entries
> > - * @pbs_dev: PBS device
> > - * @lpg_chan_sdam: LPG SDAM peripheral device
> > - * @lut_sdam: LUT SDAM peripheral device
> > - * @pbs_en_bitmap: bitmap for tracking PBS triggers
> > - * @triled_base: base address of the TRILED block (optional)
> > - * @triled_src: power-source for the TRILED
> > - * @triled_has_atc_ctl: true if there is TRI_LED_ATC_CTL register
> > - * @triled_has_src_sel: true if there is TRI_LED_SRC_SEL register
> > - * @channels: list of PWM channels
> > - * @num_channels: number of @channels
> > - */
> > -struct lpg {
> > - struct device *dev;
> > - struct regmap *map;
> > -
> > - struct mutex lock;
> > -
> > - struct pwm_chip *pwm;
> > -
> > - const struct lpg_data *data;
> > -
> > - u32 lut_base;
> > - u32 lut_size;
> > - unsigned long *lut_bitmap;
> > -
> > - struct pbs_dev *pbs_dev;
> > - struct nvmem_device *lpg_chan_sdam;
> > - struct nvmem_device *lut_sdam;
> > - unsigned long pbs_en_bitmap;
> > -
> > - u32 triled_base;
> > - u32 triled_src;
> > - bool triled_has_atc_ctl;
> > - bool triled_has_src_sel;
> > -
> > - struct lpg_channel *channels;
> > - unsigned int num_channels;
> > -};
> > -
> > /**
> > * struct lpg_channel - per channel data
> > * @lpg: reference to parent lpg
> > @@ -216,6 +166,55 @@ struct lpg_led {
> > struct lpg_channel *channels[] __counted_by(num_channels);
> > };
> >
> > +/**
> > + * struct lpg - LPG device context
> > + * @dev: pointer to LPG device
> > + * @map: regmap for register access
> > + * @lock: used to synchronize LED and pwm callback requests
> > + * @pwm: PWM-chip object, if operating in PWM mode
> > + * @data: reference to version specific data
> > + * @lut_base: base address of the LUT block (optional)
> > + * @lut_size: number of entries in the LUT block
> > + * @lut_bitmap: allocation bitmap for LUT entries
> > + * @pbs_dev: PBS device
> > + * @lpg_chan_sdam: LPG SDAM peripheral device
> > + * @lut_sdam: LUT SDAM peripheral device
> > + * @pbs_en_bitmap: bitmap for tracking PBS triggers
> > + * @triled_base: base address of the TRILED block (optional)
> > + * @triled_src: power-source for the TRILED
> > + * @triled_has_atc_ctl: true if there is TRI_LED_ATC_CTL register
> > + * @triled_has_src_sel: true if there is TRI_LED_SRC_SEL register
> > + * @channels: list of PWM channels
> > + * @num_channels: number of @channels
> > + */
>
> Should we be reordering the kerneldoc descriptions for '@channels' and
> '@num_channels' here to correctly match the updated order in the struct below?
Yeah probably.
>
> > +struct lpg {
> > + struct device *dev;
> > + struct regmap *map;
> > +
> > + struct mutex lock;
> > +
> > + struct pwm_chip *pwm;
> > +
> > + const struct lpg_data *data;
> > +
> > + u32 lut_base;
> > + u32 lut_size;
> > + unsigned long *lut_bitmap;
> > +
> > + struct pbs_dev *pbs_dev;
> > + struct nvmem_device *lpg_chan_sdam;
> > + struct nvmem_device *lut_sdam;
> > + unsigned long pbs_en_bitmap;
> > +
> > + u32 triled_base;
> > + u32 triled_src;
> > + bool triled_has_atc_ctl;
> > + bool triled_has_src_sel;
> > +
> > + unsigned int num_channels;
> > + struct lpg_channel channels[] __counted_by(num_channels);
> > +};
> > +
> > /**
> > * struct lpg_channel_data - per channel initialization data
> > * @sdam_offset: Channel offset in LPG SDAM
> > @@ -1475,12 +1474,6 @@ static int lpg_init_channels(struct lpg *lpg)
> > struct lpg_channel *chan;
> > int i;
> >
> > - lpg->num_channels = data->num_channels;
> > - lpg->channels = devm_kcalloc(lpg->dev, data->num_channels,
> > - sizeof(struct lpg_channel), GFP_KERNEL);
> > - if (!lpg->channels)
> > - return -ENOMEM;
> > -
> > for (i = 0; i < data->num_channels; i++) {
> > chan = &lpg->channels[i];
> >
> > @@ -1603,18 +1596,20 @@ static int lpg_init_sdam(struct lpg *lpg)
> >
> > static int lpg_probe(struct platform_device *pdev)
> > {
> > + const struct lpg_data *data;
> > struct lpg *lpg;
> > int ret;
> > int i;
> >
> > - lpg = devm_kzalloc(&pdev->dev, sizeof(*lpg), GFP_KERNEL);
> > + data = of_device_get_match_data(&pdev->dev);
> > + if (!data)
> > + return -EINVAL;
> > +
> > + lpg = devm_kzalloc(&pdev->dev, struct_size(lpg, channels,
> > data->num_channels), GFP_KERNEL);
> > if (!lpg)
> > return -ENOMEM;
> >
> > - lpg->data = of_device_get_match_data(&pdev->dev);
>
> You just NULL-ptr-derefed yourself. Did you test this?
This can never return NULL. All compatible entries have a
corresponding data value pointing to a statically allocated struct.
>
> > - if (!lpg->data)
> > - return -EINVAL;
> > -
> > + lpg->num_channels = data->num_channels;
> > lpg->dev = &pdev->dev;
> > mutex_init(&lpg->lock);
> >
> > --
> > 2.53.0
> >
> >
>
> --
> Lee Jones [李琼斯]
