Re: [PATCH 06/13] thermal: mediatek: add pmic thermal support

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Wed, May 06, 2026 at 11:22:15AM +0000, Roman Vivchar wrote:
> On Tuesday, May 5th, 2026 at 11:16 AM, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Mon, May 04, 2026 at 09:24:58PM +0300, Roman Vivchar via B4 Relay wrote:

...

> > > +struct mtk_pmic_sensor {
> > > +	struct mtk_pmic_thermal *mt;
> > > +	int id;
> > > +	struct iio_channel *adc_channel;
> > > +	struct thermal_zone_device *tzdev;
> > > +};
> > 
> > Can you confirm with `pahole` that this is the best layout (taking into account
> > the use in the below data structure)?
> > 
> > > +struct mtk_pmic_thermal {
> > > +	struct device *dev;
> > > +	struct regmap *regmap;
> > > +	struct mtk_pmic_sensor sensors[MAX_SENSORS];
> > > +
> > > +	s32 t_slope1;
> > > +	s32 t_slope2;
> > > +	s32 t_intercept;
> > > +
> > > +	const struct mtk_thermal_data *data;
> > > +};
> 
> On the ARMv7 it shouldn't be an issue, because pointer size equals to
> the s32 or int. However, I've reordered the fields to group pointers
> and integers together.
> 
> struct mtk_pmic_sensor {
> 	struct mtk_pmic_thermal *  mt;                   /*     0     4 */
> 	struct iio_channel *       adc_channel;          /*     4     4 */
> 	struct thermal_zone_device * tzdev;              /*     8     4 */
> 	int                        id;                   /*    12     4 */
> 
> 	/* size: 16, cachelines: 1, members: 4 */
> 	/* last cacheline: 16 bytes */
> };
> 
> struct mtk_pmic_thermal {
> 	struct device *            dev;                  /*     0     4 */
> 	struct regmap *            regmap;               /*     4     4 */
> 	const struct mtk_thermal_data  * data;           /*     8     4 */
> 	s32                        t_slope1;             /*    12     4 */
> 	s32                        t_slope2;             /*    16     4 */
> 	s32                        t_intercept;          /*    20     4 */
> 	struct mtk_pmic_sensor     sensors[1];           /*    24    16 */
> 
> 	/* size: 40, cachelines: 1, members: 7 */
> 	/* last cacheline: 40 bytes */
> };
> 
> The compiler will still add some padding on the AArch64 though.
> 
> struct mtk_pmic_sensor {
> 	struct mtk_pmic_thermal *  mt;                   /*     0     8 */
> 	struct iio_channel *       adc_channel;          /*     8     8 */
> 	struct thermal_zone_device * tzdev;              /*    16     8 */
> 	int                        id;                   /*    24     4 */
> 
> 	/* size: 32, cachelines: 1, members: 4 */
> 	/* padding: 4 */
> 	/* last cacheline: 32 bytes */
> };
> 
> struct mtk_pmic_thermal {
> 	struct device *            dev;                  /*     0     8 */
> 	struct regmap *            regmap;               /*     8     8 */
> 	const struct mtk_thermal_data  * data;           /*    16     8 */
> 	s32                        t_slope1;             /*    24     4 */
> 	s32                        t_slope2;             /*    28     4 */
> 	s32                        t_intercept;          /*    32     4 */
> 
> 	/* XXX 4 bytes hole, try to pack */

^^^^

> 	struct mtk_pmic_sensor     sensors[1];           /*    40    32 */
> 
> 	/* size: 72, cachelines: 2, members: 7 */
> 	/* sum members: 68, holes: 1, sum holes: 4 */
> 	/* last cacheline: 8 bytes */
> };
> 
> Is this good enough?

In the last it seems moving the s32 members to be the last will removes
the 4-byte hole.


-- 
With Best Regards,
Andy Shevchenko