Re: drivers/platform/x86/apple-gmux.c:276:15: sparse: sparse: cast to restricted __be32

[Hans de Goede]

Hi,

On 5/16/23 13:31, Orlando Chamberlain wrote:
> Hi,
> 
>> On 16 May 2023, at 8:27 pm, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>>
>> Hi,
>>
>>> On 5/16/23 12:16, kernel test robot wrote:
>>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>>> head:   f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
>>> commit: 0c18184de990e63f708b090bcb9fc6c0fbc427cd platform/x86: apple-gmux: support MMIO gmux on T2 Macs
>>> date:   9 weeks ago
>>> config: i386-randconfig-s001-20230515
>>> compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
>>> reproduce:
>>>        # apt-get install sparse
>>>        # sparse version: v0.6.4-39-gce1a6720-dirty
>>>        # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0c18184de990e63f708b090bcb9fc6c0fbc427cd
>>>        git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>>>        git fetch --no-tags linus master
>>>        git checkout 0c18184de990e63f708b090bcb9fc6c0fbc427cd
>>>        # save the config file
>>>        mkdir build_dir && cp config build_dir/.config
>>>        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 olddefconfig
>>>        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/platform/x86/
>>>
>>> If you fix the issue, kindly add following tag where applicable
>>> | Reported-by: kernel test robot <lkp@xxxxxxxxx>
>>> | Link: https://lore.kernel.org/oe-kbuild-all/202305161712.5l3f4iI4-lkp@xxxxxxxxx/
>>
>> <snip>
>>
>>> vim +276 drivers/platform/x86/apple-gmux.c
>>>
>>>   265    
>>>   266    static u32 gmux_mmio_read32(struct apple_gmux_data *gmux_data, int port)
>>>   267    {
>>>   268        u32 val;
>>>   269    
>>>   270        mutex_lock(&gmux_data->index_lock);
>>>   271        gmux_mmio_wait(gmux_data);
>>>   272        iowrite8((port & 0xff), gmux_data->iomem_base + GMUX_MMIO_PORT_SELECT);
>>>   273        iowrite8(GMUX_MMIO_READ | sizeof(val),
>>>   274            gmux_data->iomem_base + GMUX_MMIO_COMMAND_SEND);
>>>   275        gmux_mmio_wait(gmux_data);
>>>> 276        val = be32_to_cpu(ioread32(gmux_data->iomem_base));
>>
>> Ok, so sparse does not like this line.
>>
>>>   277        mutex_unlock(&gmux_data->index_lock);
>>>   278    
>>>   279        return val;
>>>   280    }
>>>   281    
>>>   282    static void gmux_mmio_write32(struct apple_gmux_data *gmux_data, int port,
>>>   283                       u32 val)
>>>   284    {
>>>   285        mutex_lock(&gmux_data->index_lock);
>>>> 286        iowrite32(cpu_to_be32(val), gmux_data->iomem_base);
>>
>> Nor this line. But this is what we want (convert to/from be32 to CPU
>> when reading/writing).
>>
>> There is iowrite32be() but that always unconditionally swabs
>> the byte order independent of the CPU byte-order.
>>
>> Now this is an x86 driver so always swapping is fine, still
>> I wonder if there is a better option here then using
>> iowrite32be() and ioread32be() ?
> 
> Thanks for finding those functions. I can't think of anything better unless there's a simple way to change sparse's rules so that it will be happy with code like this.

Ok, can you please submit a pathc to switch to iowrite32be() and
ioread32be() then (when you have time to work on this) ?

Regards,

Hans






>>>   287        iowrite8(port & 0xff, gmux_data->iomem_base + GMUX_MMIO_PORT_SELECT);
>>>   288        iowrite8(GMUX_MMIO_WRITE | sizeof(val),
>>>   289            gmux_data->iomem_base + GMUX_MMIO_COMMAND_SEND);
>>>   290        gmux_mmio_wait(gmux_data);
>>>   291        mutex_unlock(&gmux_data->index_lock);
>>>   292    }
>>>   293    
>>>
>>
>