| Message ID | 20240117125527.23324-3-tzimmermann@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | firmware/sysfb: Track parent device for screen_info | expand |
Thomas Zimmermann <tzimmermann@suse.de> writes: > Add screen_info_get_pci_dev() to find the PCI device of an instance > of screen_info. Does nothing on systems without PCI bus. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- [...] > +struct pci_dev *screen_info_pci_dev(const struct screen_info *si) > +{ > + struct resource res[SCREEN_INFO_MAX_RESOURCES]; > + size_t i, numres; > + int ret; > + > + ret = screen_info_resources(si, res, ARRAY_SIZE(res)); > + if (ret < 0) > + return ERR_PTR(ret); > + numres = ret; > + I would just drop the ret variable and assign the screen_info_resources() return value to numres. I think that makes the code easier to follow. Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Hi Am 29.01.24 um 12:04 schrieb Javier Martinez Canillas: > Thomas Zimmermann <tzimmermann@suse.de> writes: > >> Add screen_info_get_pci_dev() to find the PCI device of an instance >> of screen_info. Does nothing on systems without PCI bus. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- > > [...] > >> +struct pci_dev *screen_info_pci_dev(const struct screen_info *si) >> +{ >> + struct resource res[SCREEN_INFO_MAX_RESOURCES]; >> + size_t i, numres; >> + int ret; >> + >> + ret = screen_info_resources(si, res, ARRAY_SIZE(res)); >> + if (ret < 0) >> + return ERR_PTR(ret); >> + numres = ret; >> + > > I would just drop the ret variable and assign the screen_info_resources() > return value to numres. I think that makes the code easier to follow. The value of ret could be an errno code. We would effectively return NULL for errors. And I just noticed that the function docs imply this. But NULL is also a valid value if there is no PCI device. I'd prefer to keep the errno-pointer around. Best regards Thomas > > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> >
Thomas Zimmermann <tzimmermann@suse.de> writes: > Hi > > Am 29.01.24 um 12:04 schrieb Javier Martinez Canillas: >> Thomas Zimmermann <tzimmermann@suse.de> writes: >> >>> Add screen_info_get_pci_dev() to find the PCI device of an instance >>> of screen_info. Does nothing on systems without PCI bus. >>> >>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>> --- >> >> [...] >> >>> +struct pci_dev *screen_info_pci_dev(const struct screen_info *si) >>> +{ >>> + struct resource res[SCREEN_INFO_MAX_RESOURCES]; >>> + size_t i, numres; >>> + int ret; >>> + >>> + ret = screen_info_resources(si, res, ARRAY_SIZE(res)); >>> + if (ret < 0) >>> + return ERR_PTR(ret); >>> + numres = ret; >>> + >> >> I would just drop the ret variable and assign the screen_info_resources() >> return value to numres. I think that makes the code easier to follow. > > The value of ret could be an errno code. We would effectively return > NULL for errors. And I just noticed that the function docs imply this. > But NULL is also a valid value if there is no PCI device. I'd prefer to > keep the errno-pointer around. > Yes. I meant making numres an int instead of size_t (SCREEN_INFO_MAX_RESOURCES is only 3 after all). That way you could just return ERR_PTR(numres) if is < 0. No strong preference, just think that the code is easier to read in that case.
diff --git a/drivers/video/Makefile b/drivers/video/Makefile index b929b664d52c..6bbf87c1b579 100644 --- a/drivers/video/Makefile +++ b/drivers/video/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_VIDEO_NOMODESET) += nomodeset.o obj-$(CONFIG_HDMI) += hdmi.o screen_info-y := screen_info_generic.o +screen_info-$(CONFIG_PCI) += screen_info_pci.o obj-$(CONFIG_VT) += console/ obj-$(CONFIG_FB_STI) += console/ diff --git a/drivers/video/screen_info_pci.c b/drivers/video/screen_info_pci.c new file mode 100644 index 000000000000..16fe4afa3377 --- /dev/null +++ b/drivers/video/screen_info_pci.c @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/pci.h> +#include <linux/screen_info.h> + +static struct pci_dev *__screen_info_pci_dev(struct resource *res) +{ + struct pci_dev *pdev; + + if (!(res->flags & IORESOURCE_MEM)) + return NULL; + + for_each_pci_dev(pdev) { + const struct resource *r; + + if ((pdev->class >> 16) != PCI_BASE_CLASS_DISPLAY) + continue; + + r = pci_find_resource(pdev, res); + if (r) + return pdev; + } + + return NULL; +} + +/** + * screen_info_pci_dev() - Return PCI parent device that contains screen_info's framebuffer + * @si: the screen_info + * + * Returns: + * The screen_info's parent device on success, or NULL otherwise. + */ +struct pci_dev *screen_info_pci_dev(const struct screen_info *si) +{ + struct resource res[SCREEN_INFO_MAX_RESOURCES]; + size_t i, numres; + int ret; + + ret = screen_info_resources(si, res, ARRAY_SIZE(res)); + if (ret < 0) + return ERR_PTR(ret); + numres = ret; + + for (i = 0; i < numres; ++i) { + struct pci_dev *pdev = __screen_info_pci_dev(&res[i]); + + if (pdev) + return pdev; + } + + return NULL; +} +EXPORT_SYMBOL(screen_info_pci_dev); diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h index d4d45395df19..746645b8ee83 100644 --- a/include/linux/screen_info.h +++ b/include/linux/screen_info.h @@ -9,6 +9,7 @@ */ #define SCREEN_INFO_MAX_RESOURCES 3 +struct pci_dev; struct resource; static inline bool __screen_info_has_lfb(unsigned int type) @@ -104,6 +105,15 @@ static inline unsigned int screen_info_video_type(const struct screen_info *si) int screen_info_resources(const struct screen_info *si, struct resource *r, size_t num); +#if defined(CONFIG_PCI) +struct pci_dev *screen_info_pci_dev(const struct screen_info *si); +#else +static inline struct pci_dev *screen_info_pci_dev(const struct screen_info *si) +{ + return NULL; +} +#endif + extern struct screen_info screen_info; #endif /* _SCREEN_INFO_H */
Add screen_info_get_pci_dev() to find the PCI device of an instance of screen_info. Does nothing on systems without PCI bus. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/video/Makefile | 1 + drivers/video/screen_info_pci.c | 54 +++++++++++++++++++++++++++++++++ include/linux/screen_info.h | 10 ++++++ 3 files changed, 65 insertions(+) create mode 100644 drivers/video/screen_info_pci.c