Re: [igt-dev] [PATCH i-g-t 2/3] lib/igt_device: add igt_map_bar_region

[Katarzyna Dec <katarzyna.dec@intel.com>]

On Thu, Apr 04, 2019 at 03:18:04PM +0200, Daniel Mrzyglod wrote:
> This function use sysfs to map particular mmio region.
s/use/uses/
> 
> Signed-off-by: Daniel Mrzyglod <daniel.t.mrzyglod@intel.com>
> ---
>  lib/igt_device.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  lib/igt_device.h |  1 +
>  2 files changed, 42 insertions(+)
> 
> diff --git a/lib/igt_device.c b/lib/igt_device.c
> index d3934efb..805109e5 100644
> --- a/lib/igt_device.c
> +++ b/lib/igt_device.c
> @@ -220,3 +220,44 @@ struct pci_device *igt_device_get_pci_device(int fd)
>  
>  	return pci_dev;
>  }
> +
> +/**
> + * igt_device_map_pci_bar_region:
> + *
> + * @fd: the device
> + * @mmio_bar: bar to be mapped
> + * @mmio_size: bar size
> + *
> + * Returns:
> + * The pointer for mmapped bar
> + */
> +void *igt_device_map_pci_bar_region(int fd, int  mmio_bar, int mmio_size)
> +{
> +	struct igt_pci_addr pci_addr;
> +	char filepath[4096];
It would be better to have some variable here instead of 'magic' number.
> +	int newfd;
> +	void *igt_mmio = NULL;
> +
> +	if (igt_device_get_pci_addr(fd, &pci_addr)) {
> +		igt_warn("Unable to find device PCI address\n");
> +		return NULL;
> +	}
> +
> +	sprintf(filepath, "/sys/devices/pci%.4x\:%.2x/%.4x\:%.2x\:%.2x.%.1x/resource%.1x",
Even without warning from compiler this sprintf looks buggy. Is there any way we
can to it in a way to be sure it can be less error prone?
> +			pci_addr.domain,
> +			pci_addr.bus,
> +			pci_addr.domain,
> +			pci_addr.bus,
> +			pci_addr.device,
> +			pci_addr.function,
> +			mmio_bar);
> +
> +
> +	newfd = open(filepath, O_RDWR | O_SYNC);
> +	igt_mmio = mmap(0, mmio_size, PROT_READ | PROT_WRITE, MAP_SHARED, newfd, 0);
> +
> +	igt_fail_on_f(igt_mmio == MAP_FAILED,
> +		      "Couldn't map MMIO region\n");
> +
You're not closing fd on function end.
Kasia
> +	return igt_mmio;
> +}
> diff --git a/lib/igt_device.h b/lib/igt_device.h
> index 860b3a8a..6ffc1d5e 100644
> --- a/lib/igt_device.h
> +++ b/lib/igt_device.h
> @@ -34,5 +34,6 @@ void igt_device_drop_master(int fd);
>  
>  int igt_device_get_card_index(int fd);
>  struct pci_device *igt_device_get_pci_device(int fd);
> +void *igt_device_map_pci_bar_region(int fd, int  mmio_bar, int mmio_size);
>  
>  #endif /* __IGT_DEVICE_H__ */
> -- 
> 2.20.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev