Hi Thomas, Am Donnerstag, 27. Oktober 2022, 16:04:34 CEST schrieb Thomas Zimmermann: > * PGP Signed: 10/27/2022 at 04:04:34 PM > > Hi > > Am 27.10.22 um 15:07 schrieb Alexander Stein: > > Hello Thomas, > > > > Am Donnerstag, 27. Oktober 2022, 13:57:06 CEST schrieb Thomas Zimmermann: > >> Properties of 32-bit integers are returned from the OF device tree > >> as type __be32. Cast PCI vendor and device IDs from __be32 to u32 > >> before comparing them to constants. Fixes sparse warnings shown below. > >> > >> drivers/gpu/drm/tiny/ofdrm.c:237:17: warning: restricted __be32 > >> degrades > >> > >> to integer drivers/gpu/drm/tiny/ofdrm.c:238:18: warning: restricted > >> __be32 > >> degrades to integer drivers/gpu/drm/tiny/ofdrm.c:238:54: warning: > >> restricted __be32 degrades to integer > >> > >> See [1] for the bug report. > >> > >> Reported-by: kernel test robot <lkp@xxxxxxxxx> > >> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > >> Link: > >> https://lore.kernel.org/dri-devel/202210192208.D888I6X7-lkp@xxxxxxxxx/ # > >> [1] --- > >> > >> drivers/gpu/drm/tiny/ofdrm.c | 5 ++++- > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c > >> index 0e1cc2369afcc..0da8b248ccc6e 100644 > >> --- a/drivers/gpu/drm/tiny/ofdrm.c > >> +++ b/drivers/gpu/drm/tiny/ofdrm.c > >> @@ -231,8 +231,11 @@ static u64 display_get_address_of(struct drm_device > >> *dev, struct device_node *of return address; > >> > >> } > >> > >> -static bool is_avivo(__be32 vendor, __be32 device) > >> +static bool is_avivo(__be32 vendor_id, __be32 device_id) > >> > >> { > >> > >> + u32 vendor = (__force u32)vendor_id; > >> + u32 device = (__force u32)device_id; > > > > I don't have much context, but just from reading this, shouldn't this be > > be32_to_cpu() instead? > > I should have explained that in the commit message. The values are > supposed to be in big endian. We compare to PCI ids. The code originally > was taken from [1], which does the right thing. The next version will > add this info to the commit message. > > Best regards > Thomas > > [1] > https://elixir.bootlin.com/linux/v6.0.5/source/drivers/video/fbdev/offb.c#L3 > 57 Thanks for the link. The commit message for that original change [2] indicates this was a fix on a Powerstation which happens to be a PowerPC aka big-endian. This this happens to work as intended, but not on little-endian machines. vendor_id is __be32 (big-endian) and you compare this to PCI_VENDOR_ID_ATI (0x1002) which is native-endian. I guess you see the problem. See also [3] where a property is converted properly. Thinking about it, instead of calling 'is_avivo(*vendor_p, *device_p)' I'd prefer something like 'is_avivo(be32_to_cpup(vendor_p), be32_to_cpup(device_p))', so there is no need to pass __be32 around. Best regards, Alexander [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ commit/?id=57a20d8fb0d2a05abe40abd6bb29e3f923721f1b [3] https://elixir.bootlin.com/linux/v6.0.5/source/drivers/bus/fsl-mc/fsl-mc-bus.c#L1027 > > Best regards, > > Alexander > > > >> + > >> > >> /* This will match most R5xx */ > >> return (vendor == PCI_VENDOR_ID_ATI) && > >> > >> ((device >= PCI_VENDOR_ID_ATI_R520 && device < 0x7800) ||
