Hi,
On 2-Dec-24 7:45 PM, Andy Shevchenko wrote:
> On Mon, Dec 02, 2024 at 08:34:01PM +0200, Ilpo Järvinen wrote:
>> On Sat, 16 Nov 2024, Hans de Goede wrote:
>
> ...
>
>>> +struct atla10_ec_battery_state {
>>> + u8 status; /* Using ACPI Battery spec status bits */
>>> + u8 capacity; /* Percent */
>>> + __le16 charge_now_mAh;
>>> + __le16 voltage_now_mV;
>>> + __le16 current_now_mA;
>>> + __le16 charge_full_mAh;
>>> + __le16 temp; /* centi degrees Celsius */
>>> +} __packed;
>>> +
>>> +struct atla10_ec_battery_info {
>>> + __le16 charge_full_design_mAh;
>>> + __le16 voltage_now_mV; /* Should be design voltage, but is not ? */
>>> + __le16 charge_full_design2_mAh;
>>> +} __packed;
>>
>> Both struct have only naturally aligned members. Why is __packed needed?
>
> Wouldn't the second one give sizeof() == 8 rather than 6? Sorry, my memory
> about this in C is always flaky.
That might be one way how things could go wrong, yes.
To answer Ilpo's original question: these structures represent
the on wire format, hence also the __le16 use and the __packed
is there to disable any possible compiler shenanigans.
I basically always add __packed to structures representing
hw memory / wire formats just to be sure.
Regards,
Hans
