[PATCH] leds: qcom-lpg: allocate channels with main struct

[PATCH] leds: qcom-lpg: allocate channels with main struct

[Rosen Penev]

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 <rosenp@gmail.com>
---
 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
+ */
+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);
-	if (!lpg->data)
-		return -EINVAL;
-
+	lpg->num_channels = data->num_channels;
 	lpg->dev = &pdev->dev;
 	mutex_init(&lpg->lock);
 
-- 
2.53.0

Re: [PATCH] leds: qcom-lpg: allocate channels with main struct

[Lee Jones]

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 <rosenp@gmail.com>
> ---
>  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?

> +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?

> -	if (!lpg->data)
> -		return -EINVAL;
> -
> +	lpg->num_channels = data->num_channels;
>  	lpg->dev = &pdev->dev;
>  	mutex_init(&lpg->lock);
>  
> -- 
> 2.53.0
> 
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH] leds: qcom-lpg: allocate channels with main struct

[Rosen Penev]

On Thu, Apr 9, 2026 at 8:12 AM Lee Jones <lee@kernel.org> 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 <rosenp@gmail.com>
> > ---
> >  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 [李琼斯]

Re: [PATCH] leds: qcom-lpg: allocate channels with main struct

[Lee Jones]

On Thu, 09 Apr 2026, Rosen Penev wrote:

> On Thu, Apr 9, 2026 at 8:12 AM Lee Jones <lee@kernel.org> 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 <rosenp@gmail.com>
> > > ---
> > >  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 [李琼斯]