Re: [PATCH v10 03/20] dlb: add resource and device initialization
flat view
From: Greg KH <gregkh@linuxfoundation.org>
Date: 2021-03-09 09:24:53
On Wed, Feb 10, 2021 at 11:54:06AM -0600, Mike Ximing Chen wrote:
quoted hunk ↗ jump to hunk
--- a/drivers/misc/dlb/dlb_hw_types.h +++ b/drivers/misc/dlb/dlb_hw_types.h@@ -5,6 +5,13 @@ #define __DLB_HW_TYPES_H #include <linux/io.h> +#include <linux/types.h> + +#include "dlb_bitmap.h" + +#define BITS_SET(x, val, mask) (x = ((x) & ~(mask)) \ + | (((val) << (mask##_LOC)) & (mask))) +#define BITS_GET(x, mask) (((x) & (mask)) >> (mask##_LOC))
Why not use the built-in kernel functions for this? Why are you creating your own?
quoted hunk ↗ jump to hunk
-static void -dlb_pf_unmap_pci_bar_space(struct dlb *dlb, struct pci_dev *pdev) +static void dlb_pf_unmap_pci_bar_space(struct dlb *dlb, struct pci_dev *pdev)
Why reformat code here, and not do it right the first time around?
quoted hunk ↗ jump to hunk
{ pcim_iounmap(pdev, dlb->hw.csr_kva); pcim_iounmap(pdev, dlb->hw.func_kva); } -static int -dlb_pf_map_pci_bar_space(struct dlb *dlb, struct pci_dev *pdev) +static int dlb_pf_map_pci_bar_space(struct dlb *dlb, struct pci_dev *pdev)
Same here.
quoted hunk ↗ jump to hunk
{ dlb->hw.func_kva = pcim_iomap_table(pdev)[DLB_FUNC_BAR]; dlb->hw.func_phys_addr = pci_resource_start(pdev, DLB_FUNC_BAR);@@ -40,6 +42,59 @@ dlb_pf_map_pci_bar_space(struct dlb *dlb, struct pci_dev *pdev) return 0; } +/*******************************/ +/****** Driver management ******/ +/*******************************/ + +static int dlb_pf_init_driver_state(struct dlb *dlb) +{ + mutex_init(&dlb->resource_mutex); + + return 0;
If this can not fail, why is this not just void?
quoted hunk ↗ jump to hunk
diff --git a/drivers/misc/dlb/dlb_resource.h b/drivers/misc/dlb/dlb_resource.h new file mode 100644 index 000000000000..2229813d9c45 --- /dev/null +++ b/drivers/misc/dlb/dlb_resource.h
Why do you have lots of little .h files and not just one simple .h file for the driver? That makes it much easier to maintain over time, right? thanks, greg k-h