Re: [PATCH] PCI/switchtec: Read all 64 bits of part_event_bitmap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2020-01-02 8:31 p.m., Sasha Levin wrote:
> On Thu, Jan 02, 2020 at 05:46:58PM -0700, Logan Gunthorpe wrote:
>>
>>
>> On 2020-01-02 5:18 p.m., Sasha Levin wrote:
>>> On Wed, Jan 01, 2020 at 06:04:06PM +0100, Greg Kroah-Hartman wrote:
>>>> On Thu, Dec 19, 2019 at 11:27:47AM -0700, Logan Gunthorpe wrote:
>>>>> commit 6acdf7e19b37cb3a9258603d0eab315079c19c5e upstream.
>>>>>
>>>>> The part_event_bitmap register is 64 bits wide, so read it with
>>>>> ioread64()
>>>>> instead of the 32-bit ioread32().
>>>>>
>>>>> Fixes: 52eabba5bcdb ("switchtec: Add IOCTLs to the Switchtec driver")
>>>>> Link:
>>>>> https://lore.kernel.org/r/20190910195833.3891-1-logang@xxxxxxxxxxxx
>>>>> Reported-by: Doug Meyer <dmeyer@xxxxxxxxxx>
>>>>> Signed-off-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
>>>>> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
>>>>> Cc: stable@xxxxxxxxxxxxxxx    # v4.12+
>>>>> Cc: Kelvin Cao <Kelvin.Cao@xxxxxxxxxxxxx>
>>>>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>>>>> ---
>>>>>
>>>>> ioread64() was introduced in v5.1 so the upstream patch won't
>>>>> compile on
>>>>> stable versions 4.14 or 4.19. This is the same patch but uses readq()
>>>>> which should be equivalent.
>>>>
>>>> Now queued up, thanks.
>>>
>>> Hey Logan,
>>>
>>> As Guenter has pointed out, readq() is only defined for 64 bits, so this
>>> breaks compilation in i386. I've dropped this backport for now, if you
>>> could fix it up we could queue it up again.
>>
>> Not quiet true. It is in fact defined for 32-bit architectures as two
>> readl() calls in "linux/io-64-nonatomic-lo-hi.h".
>>
>> So, unless I'm missing something the patch should be fine.
> 
> This is an actual error we're seeing:
> 
> drivers/pci/switch/switchtec.c: In function ‘ioctl_event_summary’:
> drivers/pci/switch/switchtec.c:636:18: error: implicit declaration of
> function ‘readq’; did you mean ‘readl’?
> [-Werror=implicit-function-declaration]
>  636 |  s.part_bitmap = readq(&stdev->mmio_sw_event->part_event_bitmap);
>      |                  ^~~~~
>      |                  readl
> cc1: some warnings being treated as errors
> make[1]: *** [scripts/Makefile.build:310:
> drivers/pci/switch/switchtec.o] Error 1
> make: *** [Makefile:1695: drivers/pci/switch/] Error 2
> 
> So the patch isn't fine :)
> 
> You're correct about linux/io-64-nonatomic-lo-hi.h, but sadly it isn't
> included in drivers/pci/switch/switchtec.c so it's not getting used.
> Something like the following has fixed compilation for me:

Oh, hmm, yes looks like we added that include in v5.0 so earlier
versions need it for that patch to be correct on non-64bit arches. Sigh.

Can you just add the include line to the patch or do you need me to send
a new one fixed up?

Thanks,

Logan



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux