* [PATCH 0/2] drm/ofdrm: Fix sparse warnings @ 2022-10-27 11:57 Thomas Zimmermann 2022-10-27 11:57 ` [PATCH 1/2] drm/ofdrm: Cast PCI IDs to u32 for comparing Thomas Zimmermann 2022-10-27 11:57 ` [PATCH 2/2] drm/ofdrm: Cast error pointers to void __iomem * Thomas Zimmermann 0 siblings, 2 replies; 7+ messages in thread From: Thomas Zimmermann @ 2022-10-27 11:57 UTC (permalink / raw) To: javierm, airlied, daniel; +Cc: Thomas Zimmermann, dri-devel Fix two types of sparse warnings in ofdrm. Reported by the LKP bot. Thomas Zimmermann (2): drm/ofdrm: Cast PCI IDs to u32 for comparing drm/ofdrm: Cast error pointers to void __iomem * drivers/gpu/drm/tiny/ofdrm.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) base-commit: 746559738f1335241ea686566cb654847c20d7a4 -- 2.38.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] drm/ofdrm: Cast PCI IDs to u32 for comparing 2022-10-27 11:57 [PATCH 0/2] drm/ofdrm: Fix sparse warnings Thomas Zimmermann @ 2022-10-27 11:57 ` Thomas Zimmermann 2022-10-27 13:07 ` Alexander Stein 2022-10-27 11:57 ` [PATCH 2/2] drm/ofdrm: Cast error pointers to void __iomem * Thomas Zimmermann 1 sibling, 1 reply; 7+ messages in thread From: Thomas Zimmermann @ 2022-10-27 11:57 UTC (permalink / raw) To: javierm, airlied, daniel; +Cc: Thomas Zimmermann, dri-devel, kernel test robot 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@intel.com> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Link: https://lore.kernel.org/dri-devel/202210192208.D888I6X7-lkp@intel.com/ # [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; + /* This will match most R5xx */ return (vendor == PCI_VENDOR_ID_ATI) && ((device >= PCI_VENDOR_ID_ATI_R520 && device < 0x7800) || -- 2.38.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/ofdrm: Cast PCI IDs to u32 for comparing 2022-10-27 11:57 ` [PATCH 1/2] drm/ofdrm: Cast PCI IDs to u32 for comparing Thomas Zimmermann @ 2022-10-27 13:07 ` Alexander Stein 2022-10-27 14:04 ` Thomas Zimmermann 0 siblings, 1 reply; 7+ messages in thread From: Alexander Stein @ 2022-10-27 13:07 UTC (permalink / raw) To: Thomas Zimmermann Cc: javierm, dri-devel, Thomas Zimmermann, kernel test robot 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@intel.com> > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Link: https://lore.kernel.org/dri-devel/202210192208.D888I6X7-lkp@intel.com/ > # [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? Best regards, Alexander > + > /* This will match most R5xx */ > return (vendor == PCI_VENDOR_ID_ATI) && > ((device >= PCI_VENDOR_ID_ATI_R520 && device < 0x7800) || ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/ofdrm: Cast PCI IDs to u32 for comparing 2022-10-27 13:07 ` Alexander Stein @ 2022-10-27 14:04 ` Thomas Zimmermann 2022-10-28 6:33 ` Alexander Stein 0 siblings, 1 reply; 7+ messages in thread From: Thomas Zimmermann @ 2022-10-27 14:04 UTC (permalink / raw) To: Alexander Stein; +Cc: dri-devel, javierm, kernel test robot [-- Attachment #1.1: Type: text/plain, Size: 2423 bytes --] 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@intel.com> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> Link: https://lore.kernel.org/dri-devel/202210192208.D888I6X7-lkp@intel.com/ >> # [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#L357 > > Best regards, > Alexander > >> + >> /* This will match most R5xx */ >> return (vendor == PCI_VENDOR_ID_ATI) && >> ((device >= PCI_VENDOR_ID_ATI_R520 && device < 0x7800) || > > > > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/ofdrm: Cast PCI IDs to u32 for comparing 2022-10-27 14:04 ` Thomas Zimmermann @ 2022-10-28 6:33 ` Alexander Stein 2022-10-28 6:59 ` Thomas Zimmermann 0 siblings, 1 reply; 7+ messages in thread From: Alexander Stein @ 2022-10-28 6:33 UTC (permalink / raw) To: Thomas Zimmermann; +Cc: dri-devel, javierm, kernel test robot 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@intel.com> > >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > >> Link: > >> https://lore.kernel.org/dri-devel/202210192208.D888I6X7-lkp@intel.com/ # > >> [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) || ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/ofdrm: Cast PCI IDs to u32 for comparing 2022-10-28 6:33 ` Alexander Stein @ 2022-10-28 6:59 ` Thomas Zimmermann 0 siblings, 0 replies; 7+ messages in thread From: Thomas Zimmermann @ 2022-10-28 6:59 UTC (permalink / raw) To: Alexander Stein; +Cc: javierm, dri-devel, kernel test robot [-- Attachment #1.1: Type: text/plain, Size: 3799 bytes --] Hi Am 28.10.22 um 08:33 schrieb Alexander Stein: > 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@intel.com> >>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>>> Link: >>>> https://lore.kernel.org/dri-devel/202210192208.D888I6X7-lkp@intel.com/ # >>>> [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. Makes sense. Thanks for reviewing. Best regards Thomas > > 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) || > > > > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] drm/ofdrm: Cast error pointers to void __iomem * 2022-10-27 11:57 [PATCH 0/2] drm/ofdrm: Fix sparse warnings Thomas Zimmermann 2022-10-27 11:57 ` [PATCH 1/2] drm/ofdrm: Cast PCI IDs to u32 for comparing Thomas Zimmermann @ 2022-10-27 11:57 ` Thomas Zimmermann 1 sibling, 0 replies; 7+ messages in thread From: Thomas Zimmermann @ 2022-10-27 11:57 UTC (permalink / raw) To: javierm, airlied, daniel; +Cc: Thomas Zimmermann, dri-devel, kernel test robot Cast error pointers when returning them as void __iomem *. Fixes a number of Sparse warnings, such as the ones shown below. ../drivers/gpu/drm/tiny/ofdrm.c:439:31: warning: incorrect type in return expression (different address spaces) ../drivers/gpu/drm/tiny/ofdrm.c:439:31: expected void [noderef] __iomem * ../drivers/gpu/drm/tiny/ofdrm.c:439:31: got void * ../drivers/gpu/drm/tiny/ofdrm.c:442:31: warning: incorrect type in return expression (different address spaces) ../drivers/gpu/drm/tiny/ofdrm.c:442:31: expected void [noderef] __iomem * ../drivers/gpu/drm/tiny/ofdrm.c:442:31: got void * See [1] for the bug report. Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Link: https://lore.kernel.org/dri-devel/202210200016.yiQzPIy0-lkp@intel.com/ # [1] --- drivers/gpu/drm/tiny/ofdrm.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c index 0da8b248ccc6e..b358da3840ad0 100644 --- a/drivers/gpu/drm/tiny/ofdrm.c +++ b/drivers/gpu/drm/tiny/ofdrm.c @@ -436,21 +436,21 @@ static void __iomem *get_cmap_address_of(struct ofdrm_device *odev, struct devic if (!addr_p) addr_p = of_get_address(of_node, bar_no, &max_size, &flags); if (!addr_p) - return ERR_PTR(-ENODEV); + return (void __iomem *)ERR_PTR(-ENODEV); if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0) - return ERR_PTR(-ENODEV); + return (void __iomem *)ERR_PTR(-ENODEV); if ((offset + size) >= max_size) - return ERR_PTR(-ENODEV); + return (void __iomem *)ERR_PTR(-ENODEV); address = of_translate_address(of_node, addr_p); if (address == OF_BAD_ADDR) - return ERR_PTR(-ENODEV); + return (void __iomem *)ERR_PTR(-ENODEV); mem = devm_ioremap(dev->dev, address + offset, size); if (!mem) - return ERR_PTR(-ENOMEM); + return (void __iomem *)ERR_PTR(-ENOMEM); return mem; } @@ -468,7 +468,7 @@ static void __iomem *ofdrm_mach64_cmap_ioremap(struct ofdrm_device *odev, cmap_base = devm_ioremap(dev->dev, address, 0x1000); if (!cmap_base) - return ERR_PTR(-ENOMEM); + return (void __iomem *)ERR_PTR(-ENOMEM); return cmap_base; } @@ -627,11 +627,11 @@ static void __iomem *ofdrm_qemu_cmap_ioremap(struct ofdrm_device *odev, address = of_translate_address(of_node, io_of_addr); if (address == OF_BAD_ADDR) - return ERR_PTR(-ENODEV); + return (void __iomem *)ERR_PTR(-ENODEV); cmap_base = devm_ioremap(dev->dev, address + 0x3c8, 2); if (!cmap_base) - return ERR_PTR(-ENOMEM); + return (void __iomem *)ERR_PTR(-ENOMEM); return cmap_base; } -- 2.38.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-10-28 7:00 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-10-27 11:57 [PATCH 0/2] drm/ofdrm: Fix sparse warnings Thomas Zimmermann 2022-10-27 11:57 ` [PATCH 1/2] drm/ofdrm: Cast PCI IDs to u32 for comparing Thomas Zimmermann 2022-10-27 13:07 ` Alexander Stein 2022-10-27 14:04 ` Thomas Zimmermann 2022-10-28 6:33 ` Alexander Stein 2022-10-28 6:59 ` Thomas Zimmermann 2022-10-27 11:57 ` [PATCH 2/2] drm/ofdrm: Cast error pointers to void __iomem * Thomas Zimmermann
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.