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 Thu, Jan 02, 2020 at 10:43:22PM -0700, Logan Gunthorpe wrote:
> 
> 
> 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?

A new one fixed up would be great, thanks.

greg k-h



[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