On Thu, 09 Apr 2026, Rosen Penev wrote: > 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.
Where is lpg->data now set? -- Lee Jones [李琼斯]
