Re: [PATCH] mfd: macsmc: Fix key count endianness annotation
"Joshua Peisach" <[email protected]> Sun, 19 Jul 2026 11:28:25 -0400
| Newsgroups | dev.linux.lists.asahi,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Sun Jul 19, 2026 at 11:15 AM EDT, Sven Peter wrote:
>
>
> On 7/19/26 17:02, Joshua Peisach wrote:
>> On Sun Jul 19, 2026 at 9:00 AM EDT, Sven Peter wrote:
>>> SMC firmware returns the value of the #KEY key in big-endian unlike mos=
t
>>> other keys. Reading it through apple_smc_read_u32() into a plain u32
>>> and then converting with be32_to_cpu() makes sparse complain:
>>>
>>> =C2=A0 drivers/mfd/macsmc.c:462:26: sparse: cast to restricted __be32
>>>
>>> Read the raw value into a __be32 using apple_smc_read() instead.
>>>
>>> Fixes: e038d985c982 ("mfd: Add Apple Silicon System Management=20
>>> Controller")
>>> Reported-by: kernel test robot <[email protected]>
>>> Closes:=20
>>> https://lore.kernel.org/oe-kbuild-all/[email protected]=
om/
>>> Signed-off-by: Sven Peter <[email protected]>
>>> ---
>>> =C2=A0drivers/mfd/macsmc.c | 8 +++++---
>>> =C2=A01 file changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mfd/macsmc.c b/drivers/mfd/macsmc.c
>>> index 358feec2d088..514cba7dc897 100644
>>> --- a/drivers/mfd/macsmc.c
>>> +++ b/drivers/mfd/macsmc.c
>>> @@ -410,7 +410,7 @@ static int apple_smc_probe(struct platform_device=
=20
>>> *pdev)
>>> =C2=A0{
>>> =C2=A0=C2=A0=C2=A0=C2=A0 struct device *dev =3D &pdev->dev;
>>> =C2=A0=C2=A0=C2=A0=C2=A0 struct apple_smc *smc;
>>> -=C2=A0=C2=A0=C2=A0 u32 count;
>>> +=C2=A0=C2=A0=C2=A0 __be32 count;
>>> =C2=A0=C2=A0=C2=A0=C2=A0 int ret;
>>>
>>> =C2=A0=C2=A0=C2=A0=C2=A0 smc =3D devm_kzalloc(dev, sizeof(*smc), GFP_KE=
RNEL);
>>> @@ -461,8 +461,10 @@ static int apple_smc_probe(struct=20
>>> platform_device *pdev)
>>> =C2=A0=C2=A0=C2=A0=C2=A0 dev_set_drvdata(&pdev->dev, smc);
>>> =C2=A0=C2=A0=C2=A0=C2=A0 BLOCKING_INIT_NOTIFIER_HEAD(&smc->event_handle=
rs);
>>>
>>> -=C2=A0=C2=A0=C2=A0 ret =3D apple_smc_read_u32(smc, SMC_KEY(#KEY), &cou=
nt);
>>> -=C2=A0=C2=A0=C2=A0 if (ret)
>>> +=C2=A0=C2=A0=C2=A0 ret =3D apple_smc_read(smc, SMC_KEY(#KEY), &count, =
sizeof(count));
>>> +=C2=A0=C2=A0=C2=A0 if (ret >=3D 0 && ret !=3D sizeof(count))
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ret =3D -EINVAL;
>>> +=C2=A0=C2=A0=C2=A0 if (ret < 0)
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return dev_err_probe(s=
mc->dev, ret, "Failed to get key count");
>>> =C2=A0=C2=A0=C2=A0=C2=A0 smc->key_count =3D be32_to_cpu(count);
>>
>> This makes sense, it just feels weird reading because u32 is.. not a
>> be32 (that gets passed into be32_to_cpu).
>
> count is __be32 now (the SMC firmware returns that value as big-endian=20
> because Apple likes to think different) and we read that using=20
> apple_smc_read now which just reads raw bytes. be32_to_cpu then converts=
=20
> that to a u32, i.e. the type of smc->key_count. There's no functional=20
> change here. What's weird about that?
>
>
> Sven
Pfft, brain fart. I thought you were going other way around.
Reviewed-by: Joshua Peisach <[email protected]>