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

Andy Shevchenko
2026-05-06T13:26:23Z

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