Re: [PATCH] staging: gasket: formatting fixes
On Mon, Nov 12, 2018 at 1:27 PM Robert Deal wrote: > > Reformat arguments in a few functions in gasket_page_table.c to better > follow linux kernel formatting standards. > > Signed-off-by: Robert Deal > --- > drivers/staging/gasket/gasket_page_table.c | 24 ++ > 1 file changed, 11 insertions(+), 13 deletions(-) > > diff --git a/drivers/staging/gasket/gasket_page_table.c > b/drivers/staging/gasket/gasket_page_table.c > index f5253ba9a430..26755d9ca41d 100644 > --- a/drivers/staging/gasket/gasket_page_table.c > +++ b/drivers/staging/gasket/gasket_page_table.c > @@ -1088,9 +1088,9 @@ void gasket_page_table_reset(struct gasket_page_table > *pg_tbl) > } > > /* See gasket_page_table.h for description. */ > -int gasket_page_table_lookup_page( > - struct gasket_page_table *pg_tbl, ulong dev_addr, struct page **ppage, > - ulong *poffset) > +int gasket_page_table_lookup_page(struct gasket_page_table *pg_tbl, > + ulong dev_addr, struct page **ppage, > + ulong *poffset) > { > uint page_num; > struct gasket_page_table_entry *pte; > @@ -1134,9 +1134,9 @@ int gasket_page_table_lookup_page( > } > > /* See gasket_page_table.h for description. */ > -bool gasket_page_table_are_addrs_bad( > - struct gasket_page_table *pg_tbl, ulong host_addr, ulong dev_addr, > - ulong bytes) > +bool gasket_page_table_are_addrs_bad(struct gasket_page_table *pg_tbl, > +ulong host_addr, ulong dev_addr, > +ulong bytes) > { > if (host_addr & (PAGE_SIZE - 1)) { > dev_err(pg_tbl->device, > @@ -1150,8 +1150,8 @@ bool gasket_page_table_are_addrs_bad( > EXPORT_SYMBOL(gasket_page_table_are_addrs_bad); > > /* See gasket_page_table.h for description. */ > -bool gasket_page_table_is_dev_addr_bad( > - struct gasket_page_table *pg_tbl, ulong dev_addr, ulong bytes) > +bool gasket_page_table_is_dev_addr_bad(struct gasket_page_table *pg_tbl, > + ulong dev_addr, ulong bytes) > { > uint num_pages = bytes / PAGE_SIZE; > > @@ -1226,9 +1226,8 @@ int gasket_page_table_system_status(struct > gasket_page_table *page_table) > } > > /* Record the host_addr to coherent dma memory mapping. */ > -int gasket_set_user_virt( > - struct gasket_dev *gasket_dev, u64 size, dma_addr_t dma_address, > - ulong vma) > +int gasket_set_user_virt(struct gasket_dev *gasket_dev, u64 size, > +dma_addr_t dma_address, ulong vma) > { > int j; > struct gasket_page_table *pg_tbl; > @@ -1346,8 +1345,7 @@ int gasket_free_coherent_memory(struct gasket_dev > *gasket_dev, u64 size, > } > > /* Release all coherent memory. */ > -void gasket_free_coherent_memory_all( > - struct gasket_dev *gasket_dev, u64 index) > +void gasket_free_coherent_memory_all(struct gasket_dev *gasket_dev, u64 > index) > { > if (!gasket_dev->page_table[index]) > return; > -- > 2.17.1 > Acked-by: Todd Poynor Thanks!
RFC: staging: gasket: re-implement using UIO
On Mon, Sep 10, 2018 at 8:28 AM Ahmed S. Darwish wrote: > > The gasket in-kernel framework, recently introduced under staging, > re-implements what is already long-time provided by the UIO > subsystem, with extra PCI BAR remapping and MSI conveniences. > > Before moving it out of staging, make sure we add the new bits to > the UIO framework instead, then transform its signle client, the > Apex driver, to a proper UIO driver (uio_driver.h). > > Link: https://lkml.kernel.org/r/20180828103817.GB1397@do-kernel So I'm looking at this for reals now. The BAR mapping stuff is straightforward with the existing framework. Everything else could be done outside of UIO via the existing device interface, but figured I'd collect any opinions about adding the new bits to UIO. The Apex device has 13 MSIX interrupts. UIO does one IRQ per device. The PRUSS driver registers 8 instances of the UIO device with identical memory mappings but individual IRQs for its 8 interrupts. Currently gasket has userspace pass down an eventfd (via ioctl) for each interrupt it wants to watch. Is there interest in modifying UIO to handle multiple IRQs in some perhaps similar fashion? Speaking of ioctls, are those allowed here, or is sysfs or something else always required? The aforementioned multiple IRQ stuff probably maps nicely to sysfs (there's a small number of them easily represented as attributes), while DMA buffer mappings seem more problematic, but maybe somebody's thought of a good way to represent these already. And then we need to map buffers to our device. We could probably implement this via an IOMMU driver API for our custom MMU and hook that up to generic IOMMU support for UIO, which sounds like something a lot of drivers could use. There's a few other tidbits the driver does, including allocating coherent memory for userspace to share with the device, but that's probably enough for now. If anybody wants to squash any of the above as a non-starter for UIO or point things in a different direction, it's appreciated. Thanks, Todd
[PATCH 2/3] staging: gasket: remove debug logs in page table mapping calls
From: Todd Poynor Remove very noisy debug logs that also contain typos and incorrect output formats. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 24 -- 1 file changed, 24 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 5b398b7ba81d3..b7d460cf15fbc 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -477,7 +477,6 @@ static int gasket_perform_mapping(struct gasket_page_table *pg_tbl, for (i = 0; i < num_pages; i++) { page_addr = host_addr + i * PAGE_SIZE; offset = page_addr & (PAGE_SIZE - 1); - dev_dbg(pg_tbl->device, "%s i %d\n", __func__, i); if (is_coherent(pg_tbl, host_addr)) { u64 off = (u64)host_addr - @@ -506,22 +505,9 @@ static int gasket_perform_mapping(struct gasket_page_table *pg_tbl, ptes[i].dma_addr = dma_map_page(pg_tbl->device, page, 0, PAGE_SIZE, DMA_BIDIRECTIONAL); - dev_dbg(pg_tbl->device, - "%s i %d pte %p pfn %p -> mapped %llx\n", - __func__, i, &ptes[i], - (void *)page_to_pfn(page), - (unsigned long long)ptes[i].dma_addr); if (dma_mapping_error(pg_tbl->device, ptes[i].dma_addr)) { - dev_dbg(pg_tbl->device, - "%s i %d -> fail to map page %llx " - "[pfn %p ohys %p]\n", - __func__, i, - (unsigned long long)ptes[i].dma_addr, - (void *)page_to_pfn(page), - (void *)page_to_phys(page)); - if (gasket_release_page(ptes[i].page)) --pg_tbl->num_active_pages; @@ -892,11 +878,6 @@ static int gasket_alloc_extended_subtable(struct gasket_page_table *pg_tbl, pte->dma_addr = dma_map_page(pg_tbl->device, pte->page, 0, PAGE_SIZE, DMA_TO_DEVICE); if (dma_mapping_error(pg_tbl->device, pte->dma_addr)) { - dev_dbg(pg_tbl->device, - "%s: fail to map page [pfn %lx phys %llx]\n", - __func__, page_to_pfn(pte->page), - page_to_phys(pte->page)); - free_page(page_addr); vfree(pte->sublevel); memset(pte, 0, sizeof(struct gasket_page_table_entry)); @@ -1050,11 +1031,6 @@ int gasket_page_table_map(struct gasket_page_table *pg_tbl, ulong host_addr, } mutex_unlock(&pg_tbl->mutex); - - dev_dbg(pg_tbl->device, - "%s done: ha %llx daddr %llx num %d, ret %d\n", - __func__, (unsigned long long)host_addr, - (unsigned long long)dev_addr, num_pages, ret); return ret; } EXPORT_SYMBOL(gasket_page_table_map); -- 2.19.1.331.ge82ca0e54c-goog
[PATCH 3/3] staging: gasket: page_table: add mapping flags
From: Nick Ewalt This allows for more precise dma_direction in the dma_map_page requests. Also leaves room for adding more flags later. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket.h| 33 drivers/staging/gasket/gasket_ioctl.c | 62 +++ drivers/staging/gasket/gasket_page_table.c | 87 ++ drivers/staging/gasket/gasket_page_table.h | 4 +- 4 files changed, 141 insertions(+), 45 deletions(-) diff --git a/drivers/staging/gasket/gasket.h b/drivers/staging/gasket/gasket.h index a0f065c517a52..93e7af1551975 100644 --- a/drivers/staging/gasket/gasket.h +++ b/drivers/staging/gasket/gasket.h @@ -37,6 +37,31 @@ struct gasket_page_table_ioctl { u64 device_address; }; +/* + * Structure for ioctl mapping buffers with flags when using the Gasket + * page_table module. + */ +struct gasket_page_table_ioctl_flags { + struct gasket_page_table_ioctl base; + /* +* Flags indicating status and attribute requests from the host. +* NOTE: STATUS bit does not need to be set in this request. +* Set RESERVED bits to 0 to ensure backwards compatibility. +* +* Bitfields: +* [0] - STATUS: indicates if this entry/slot is free +*0 = PTE_FREE +*1 = PTE_INUSE +* [2:1] - DMA_DIRECTION: dma_data_direction requested by host +* 00 = DMA_BIDIRECTIONAL +* 01 = DMA_TO_DEVICE +* 10 = DMA_FROM_DEVICE +* 11 = DMA_NONE +* [31:3] - RESERVED +*/ + u32 flags; +}; + /* * Common structure for ioctls mapping and unmapping buffers when using the * Gasket page_table module. @@ -119,4 +144,12 @@ struct gasket_coherent_alloc_config_ioctl { #define GASKET_IOCTL_CONFIG_COHERENT_ALLOCATOR \ _IOWR(GASKET_IOCTL_BASE, 11, struct gasket_coherent_alloc_config_ioctl) +/* + * Tells the kernel to map size bytes at host_address to device_address in + * page_table_index page table. Passes flags to indicate additional attribute + * requests for the mapped memory. + */ +#define GASKET_IOCTL_MAP_BUFFER_FLAGS \ + _IOW(GASKET_IOCTL_BASE, 12, struct gasket_page_table_ioctl_flags) + #endif /* __GASKET_H__ */ diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c index 0ca48e688818f..e80f38509f47c 100644 --- a/drivers/staging/gasket/gasket_ioctl.c +++ b/drivers/staging/gasket/gasket_ioctl.c @@ -20,6 +20,7 @@ #define trace_gasket_ioctl_integer_data(x) #define trace_gasket_ioctl_eventfd_data(x, ...) #define trace_gasket_ioctl_page_table_data(x, ...) +#define trace_gasket_ioctl_page_table_flags_data(x, ...) #define trace_gasket_ioctl_config_coherent_allocator(x, ...) #endif @@ -130,29 +131,59 @@ static int gasket_partition_page_table( } /* Map a userspace buffer to a device virtual address. */ +static int gasket_map_buffers_common( + struct gasket_dev *gasket_dev, + struct gasket_page_table_ioctl_flags *pibuf) +{ + if (pibuf->base.page_table_index >= gasket_dev->num_page_tables) + return -EFAULT; + + if (gasket_page_table_are_addrs_bad(gasket_dev->page_table[pibuf->base.page_table_index], + pibuf->base.host_address, + pibuf->base.device_address, + pibuf->base.size)) + return -EINVAL; + + return gasket_page_table_map(gasket_dev->page_table[pibuf->base.page_table_index], +pibuf->base.host_address, +pibuf->base.device_address, +pibuf->base.size / PAGE_SIZE, +pibuf->flags); +} + static int gasket_map_buffers(struct gasket_dev *gasket_dev, struct gasket_page_table_ioctl __user *argp) { - struct gasket_page_table_ioctl ibuf; + struct gasket_page_table_ioctl_flags ibuf; - if (copy_from_user(&ibuf, argp, sizeof(struct gasket_page_table_ioctl))) + if (copy_from_user(&ibuf.base, argp, sizeof(struct gasket_page_table_ioctl))) return -EFAULT; - trace_gasket_ioctl_page_table_data(ibuf.page_table_index, ibuf.size, - ibuf.host_address, - ibuf.device_address); + ibuf.flags = 0; - if (ibuf.page_table_index >= gasket_dev->num_page_tables) + trace_gasket_ioctl_page_table_data(ibuf.base.page_table_index, + ibuf.base.size, + ibuf.base.host_address, +
[PATCH 0/3] staging: gasket: add DMA direction flags a.o.
From: Todd Poynor Add flags to page mapping ioctls that specify DMA directions other than bi-directional, avoiding unnecessary cache maintenance for read-only or write-only buffers. Remove some spammy / unhelpful / inaccurately formatted debug logs. Nick Ewalt (1): staging: gasket: page_table: add mapping flags Todd Poynor (2): staging: gasket: remove debug logs for callback invocation staging: gasket: remove debug logs in page table mapping calls drivers/staging/gasket/gasket.h| 33 ++ drivers/staging/gasket/gasket_core.c | 7 +- drivers/staging/gasket/gasket_ioctl.c | 62 +--- drivers/staging/gasket/gasket_page_table.c | 111 +++-- drivers/staging/gasket/gasket_page_table.h | 4 +- 5 files changed, 142 insertions(+), 75 deletions(-) -- 2.19.1.331.ge82ca0e54c-goog
[PATCH 1/3] staging: gasket: remove debug logs for callback invocation
From: Todd Poynor Debug logs for device-specific callback invocation aren't very useful, remove. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index f230bec76ae4e..a445d58fb3999 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -109,8 +109,6 @@ check_and_invoke_callback(struct gasket_dev *gasket_dev, { int ret = 0; - dev_dbg(gasket_dev->dev, "check_and_invoke_callback %p\n", - cb_function); if (cb_function) { mutex_lock(&gasket_dev->mutex); ret = cb_function(gasket_dev); @@ -126,11 +124,8 @@ gasket_check_and_invoke_callback_nolock(struct gasket_dev *gasket_dev, { int ret = 0; - if (cb_function) { - dev_dbg(gasket_dev->dev, - "Invoking device-specific callback.\n"); + if (cb_function) ret = cb_function(gasket_dev); - } return ret; } -- 2.19.1.331.ge82ca0e54c-goog
Re: [PATCH 01/11] staging: gasket: core: debug log updates
On Mon, Oct 15, 2018 at 12:34 AM Greg Kroah-Hartman wrote: > > On Sun, Oct 14, 2018 at 09:59:17PM -0700, Todd Poynor wrote: > > From: Todd Poynor > > > > Add debug logs for device enable/disable events, > > Why? > > What is going to need this? As one of the few people actually developing for Apex chips (but this may be more soon), I ran into a situation where I cared about this. But I usually cobble together custom debugging for development situations, and generally don't get debug logs for released-in-the-wild drivers, so I'm fine not including these or any other debug logs. It sounds like debug logs face a pretty high bar for acceptance. I'm happy to send a patch removing all of 'em from gasket/apex if that's preferred. > > remove logs for > > callbacks (the called functions can generate their own logs if needed). > > That's a different thing than "adding" them, so shouldn't this really be > 2 patches? If it was, I could have accepted this patch already, and > ignored the "add new logs" one :) Sure, the callbacks most frequently occur during disable/enable events and were linked in my brain, but will send a new patch to just remove the useless callback logs. > > thanks, > > greg k-h
Re: [PATCH v3] staging: gasket: Fix sparse "incorrect type in assignment" warnings.
On Wed, Oct 10, 2018 at 2:14 PM Laurence Rochfort wrote: > > Remove the coherent buffer __iomem cookie because the buffer is > allocated from dma_alloc_coherent(). > > warning: incorrect type in assignment (different address spaces) >expected unsigned char [noderef] [usertype] *virt_base >got void *[assigned] mem > warning: incorrect type in argument 3 (different address spaces) >expected void *cpu_addr >got unsigned char [noderef] [usertype] *virt_base > > Signed-off-by: Laurence Rochfort > --- > Changes in v3: > - Just remove the __iommem cookie, don't alter type. > > drivers/staging/gasket/gasket_core.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/gasket/gasket_core.h > b/drivers/staging/gasket/gasket_core.h > index 275fd0b..e62adcd 100644 > --- a/drivers/staging/gasket/gasket_core.h > +++ b/drivers/staging/gasket/gasket_core.h > @@ -231,7 +231,7 @@ struct gasket_coherent_buffer_desc { > /* Coherent buffer structure. */ > struct gasket_coherent_buffer { > /* Virtual base address. */ > - u8 __iomem *virt_base; > + u8 *virt_base; > > /* Physical base address. */ > ulong phys_base; > -- > 2.9.5 Reviewed-by: Todd Poynor Thanks!
Re: [PATCH 00/11] staging: gasket: fixes
On Mon, Oct 15, 2018 at 12:33 AM Greg Kroah-Hartman wrote: > > On Sun, Oct 14, 2018 at 09:59:16PM -0700, Todd Poynor wrote: > > From: Todd Poynor > > > > Various fixes for gasket/apex drivers. > > For some reason you seem to have ignored/missed this patch: > Subject: [PATCH v3] staging: gasket: Fix sparse "incorrect type in > assignment" warnings. > that was sent last week. > > Any specific reason? Coming off a 2 week vacation, I'm behind on everything. I saw you had commented already, but I'll take a look as well, sure. > > thanks, > > greg k-h -- Todd
[PATCH 07/11] staging: gasket: page_table: fix comment in components_to_dev_address
From: Nick Ewalt Comments in components_to_dev_address() describing examples are inconsistent, fix these to be accurate. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index a88f2ae0cee8b..ec9359576ea7c 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -687,13 +687,13 @@ static inline bool gasket_addr_is_simple(struct gasket_page_table *pg_tbl, * Convert (simple, page, offset) into a device address. * Examples: * Simple page 0, offset 32: - * Input (0, 0, 32), Output 0x20 + * Input (1, 0, 32), Output 0x20 * Simple page 1000, offset 511: - * Input (0, 1000, 512), Output 0x3E81FF + * Input (1, 1000, 511), Output 0x3E81FF * Extended page 0, offset 32: * Input (0, 0, 32), Output 0x800020 * Extended page 1000, offset 511: - * Input (1, 1000, 512), Output 0x8003E81FF + * Input (0, 1000, 511), Output 0x8003E81FF */ static ulong gasket_components_to_dev_address(struct gasket_page_table *pg_tbl, int is_simple, uint page_index, -- 2.19.0.605.g01d371f741-goog
[PATCH 04/11] staging: gasket: page_table: remove unnecessary PTE status set to free
From: Nick Ewalt Remove unnecessary ptes[i].status update in gasket_perform_unmapping. The vaaue will be cleared in the following memset. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 985a3a93499d5..d2e115d2dba30 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -623,7 +623,6 @@ static void gasket_perform_unmapping(struct gasket_page_table *pg_tbl, if (gasket_release_page(ptes[i].page)) --pg_tbl->num_active_pages; } - ptes[i].status = PTE_FREE; /* and clear the PTE. */ memset(&ptes[i], 0, sizeof(struct gasket_page_table_entry)); -- 2.19.0.605.g01d371f741-goog
[PATCH 11/11] staging: gasket: Update device virtual address comment
From: Nick Ewalt Add that number of page table entries and extended address bit offset are configurable. Update example virtual address format to be more consistent with typical usage. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index c2fbab74194f8..5b398b7ba81d3 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -10,10 +10,18 @@ * * This file assumes 4kB pages throughout; can be factored out when necessary. * - * Address format is as follows: + * There is a configurable number of page table entries, as well as a + * configurable bit index for the extended address flag. Both of these are + * specified in gasket_page_table_init through the page_table_config parameter. + * + * The following example assumes: + * page_table_config->total_entries = 8192 + * page_table_config->extended_bit = 63 + * + * Address format: * Simple addresses - those whose containing pages are directly placed in the * device's address translation registers - are laid out as: - * [ 63 - 40: Unused | 39 - 28: 0 | 27 - 12: page index | 11 - 0: page offset ] + * [ 63 - 25: 0 | 24 - 12: page index | 11 - 0: page offset ] * page index: The index of the containing page in the device's address * translation registers. * page offset: The index of the address into the containing page. @@ -21,7 +29,7 @@ * Extended address - those whose containing pages are contained in a second- * level page table whose address is present in the device's address translation * registers - are laid out as: - * [ 63 - 40: Unused | 39: flag | 38 - 37: 0 | 36 - 21: dev/level 0 index | + * [ 63: flag | 62 - 34: 0 | 33 - 21: dev/level 0 index | * 20 - 12: host/level 1 index | 11 - 0: page offset ] * flag:Marker indicating that this is an extended address. Always 1. * dev index: The index of the first-level page in the device's extended -- 2.19.0.605.g01d371f741-goog
[PATCH 02/11] staging: gasket: page table: return valid error code on map fail
From: Todd Poynor Return -EINVAL on mapping failures, instead of -1, which triggers a checkpatch error. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 964146f0df526..2e1de8ad4a2c6 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -514,13 +514,12 @@ static int gasket_perform_mapping(struct gasket_page_table *pg_tbl, (void *)page_to_pfn(page), (void *)page_to_phys(page)); - /* clean up */ if (gasket_release_page(ptes[i].page)) --pg_tbl->num_active_pages; memset(&ptes[i], 0, sizeof(struct gasket_page_table_entry)); - return -1; + return -EINVAL; } } @@ -1165,7 +1164,7 @@ int gasket_page_table_lookup_page( *ppage = NULL; *poffset = 0; mutex_unlock(&pg_tbl->mutex); - return -1; + return -EINVAL; } /* See gasket_page_table.h for description. */ -- 2.19.0.605.g01d371f741-goog
[PATCH 08/11] staging: gasket: page_table: simplify gasket_components_to_dev_address
From: Nick Ewalt Refactor gasket_components_to_dev_address to be faster and easier to understand. The old implementation was unnecessarily complex and masked the page_index for simple addresses but not extended ones. It makes the most sense for this function to perform no such masking. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 21 ++--- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index ec9359576ea7c..c2fbab74194f8 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -699,26 +699,9 @@ static ulong gasket_components_to_dev_address(struct gasket_page_table *pg_tbl, int is_simple, uint page_index, uint offset) { - ulong lvl0_index, lvl1_index; + ulong dev_addr = (page_index << GASKET_SIMPLE_PAGE_SHIFT) | offset; - if (is_simple) { - /* Return simple addresses directly. */ - lvl0_index = page_index & (pg_tbl->config.total_entries - 1); - return (lvl0_index << GASKET_SIMPLE_PAGE_SHIFT) | offset; - } - - /* -* This could be compressed into fewer statements, but -* A) the compiler should optimize it -* B) this is not slow -* C) this is an uncommon operation -* D) this is actually readable this way. -*/ - lvl0_index = page_index / GASKET_PAGES_PER_SUBTABLE; - lvl1_index = page_index & (GASKET_PAGES_PER_SUBTABLE - 1); - return (pg_tbl)->extended_flag | - (lvl0_index << GASKET_EXTENDED_LVL0_SHIFT) | - (lvl1_index << GASKET_EXTENDED_LVL1_SHIFT) | offset; + return is_simple ? dev_addr : (pg_tbl->extended_flag | dev_addr); } /* -- 2.19.0.605.g01d371f741-goog
[PATCH 01/11] staging: gasket: core: debug log updates
From: Todd Poynor Add debug logs for device enable/disable events, remove logs for callbacks (the called functions can generate their own logs if needed). Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index f230bec76ae4e..62a7515915e59 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -109,8 +109,6 @@ check_and_invoke_callback(struct gasket_dev *gasket_dev, { int ret = 0; - dev_dbg(gasket_dev->dev, "check_and_invoke_callback %p\n", - cb_function); if (cb_function) { mutex_lock(&gasket_dev->mutex); ret = cb_function(gasket_dev); @@ -126,11 +124,8 @@ gasket_check_and_invoke_callback_nolock(struct gasket_dev *gasket_dev, { int ret = 0; - if (cb_function) { - dev_dbg(gasket_dev->dev, - "Invoking device-specific callback.\n"); + if (cb_function) ret = cb_function(gasket_dev); - } return ret; } @@ -633,6 +628,7 @@ void gasket_disable_device(struct gasket_dev *gasket_dev) gasket_dev->internal_desc->driver_desc; int i; + dev_dbg(gasket_dev->dev, "disabling device\n"); /* Only delete the device if it has been successfully added. */ if (gasket_dev->dev_info.cdev_added) cdev_del(&gasket_dev->dev_info.cdev); @@ -1357,6 +1353,7 @@ int gasket_enable_device(struct gasket_dev *gasket_dev) const struct gasket_driver_desc *driver_desc = gasket_dev->internal_desc->driver_desc; + dev_dbg(gasket_dev->dev, "enabling device\n"); ret = gasket_interrupt_init(gasket_dev); if (ret) { dev_err(gasket_dev->dev, -- 2.19.0.605.g01d371f741-goog
[PATCH 09/11] staging: gasket: apex: fix sysfs_show
From: Nick Ewalt sysfs_show was incorrectly extracting the sysfs_attribute_type from the gasket_sysfs_attribute. This prevented dispatch from working properly. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/apex_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index 6dca3b177863c..3c7a13a4798ef 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -526,7 +526,7 @@ static ssize_t sysfs_show(struct device *device, struct device_attribute *attr, return -ENODEV; } - type = (enum sysfs_attribute_type)gasket_sysfs_get_attr(device, attr); + type = (enum sysfs_attribute_type)gasket_attr->data.attr_type; switch (type) { case ATTR_KERNEL_HIB_PAGE_TABLE_SIZE: ret = scnprintf(buf, PAGE_SIZE, "%u\n", -- 2.19.0.605.g01d371f741-goog
[PATCH 03/11] staging: gasket: page table: remove dead code in coherent mem alloc
From: Todd Poynor gasket_alloc_coherent_memory() has some unnecessary code related to out of memory checking that will never hit the condition checked, remove. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 2e1de8ad4a2c6..985a3a93499d5 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -1316,7 +1316,6 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size, GFP_KERNEL); if (!gasket_dev->page_table[index]->coherent_pages) goto nomem; - *dma_address = 0; gasket_dev->coherent_buffer.length_bytes = PAGE_SIZE * (num_pages); @@ -1331,15 +1330,12 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size, (u64)mem + j * PAGE_SIZE; } - if (*dma_address == 0) - goto nomem; return 0; nomem: - if (mem) { + if (mem) dma_free_coherent(gasket_get_device(gasket_dev), num_pages * PAGE_SIZE, mem, handle); - } kfree(gasket_dev->page_table[index]->coherent_pages); gasket_dev->page_table[index]->coherent_pages = NULL; -- 2.19.0.605.g01d371f741-goog
[PATCH 06/11] staging: gasket: page table: fixup error path allocating coherent mem
From: Nick Ewalt Correctly clean up data structure state in gasket_alloc_coherent_memory error path, to ensure no double free on the stale pointer value. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 9c2f8671216b5..a88f2ae0cee8b 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -1332,9 +1332,13 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size, return 0; nomem: - if (mem) + if (mem) { dma_free_coherent(gasket_get_device(gasket_dev), num_pages * PAGE_SIZE, mem, handle); + gasket_dev->coherent_buffer.length_bytes = 0; + gasket_dev->coherent_buffer.virt_base = NULL; + gasket_dev->coherent_buffer.phys_base = 0; + } kfree(gasket_dev->page_table[index]->coherent_pages); gasket_dev->page_table[index]->coherent_pages = NULL; -- 2.19.0.605.g01d371f741-goog
[PATCH 00/11] staging: gasket: fixes
From: Todd Poynor Various fixes for gasket/apex drivers. Nick Ewalt (8): staging: gasket: page_table: remove unnecessary PTE status set to free staging: gasket: page_table: rearrange gasket_page_table_entry staging: gasket: page table: fixup error path allocating coherent mem staging: gasket: page_table: fix comment in components_to_dev_address staging: gasket: page_table: simplify gasket_components_to_dev_address staging: gasket: apex: fix sysfs_show staging: gasket: sysfs: fix attribute release comment staging: gasket: Update device virtual address comment Todd Poynor (3): staging: gasket: core: debug log updates staging: gasket: page table: return valid error code on map fail staging: gasket: page table: remove dead code in coherent mem alloc drivers/staging/gasket/apex_driver.c | 2 +- drivers/staging/gasket/gasket_core.c | 9 +-- drivers/staging/gasket/gasket_page_table.c | 65 +- drivers/staging/gasket/gasket_sysfs.h | 4 +- 4 files changed, 33 insertions(+), 47 deletions(-) -- 2.19.0.605.g01d371f741-goog
[PATCH 10/11] staging: gasket: sysfs: fix attribute release comment
From: Nick Ewalt Comments for gasket_sysfs_get_attr() incorrectly describe reference release procedure. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_sysfs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/gasket/gasket_sysfs.h b/drivers/staging/gasket/gasket_sysfs.h index f32eaf89e056b..151e8edd28ea6 100644 --- a/drivers/staging/gasket/gasket_sysfs.h +++ b/drivers/staging/gasket/gasket_sysfs.h @@ -152,8 +152,8 @@ void gasket_sysfs_put_device_data(struct device *device, * Returns the Gasket sysfs attribute associated with the kernel device * attribute and device structure itself. Upon success, this call will take a * reference to internal sysfs data that must be released with a call to - * gasket_sysfs_get_device_data. While this reference is held, the underlying - * device sysfs information/structure will remain valid/will not be deleted. + * gasket_sysfs_put_attr. While this reference is held, the underlying device + * sysfs information/structure will remain valid/will not be deleted. */ struct gasket_sysfs_attribute * gasket_sysfs_get_attr(struct device *device, struct device_attribute *attr); -- 2.19.0.605.g01d371f741-goog
[PATCH 05/11] staging: gasket: page_table: rearrange gasket_page_table_entry
From: Nick Ewalt Rearrange gasket_page_table entry to reduce padding slop. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index d2e115d2dba30..9c2f8671216b5 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -103,12 +103,6 @@ struct gasket_page_table_entry { /* The status of this entry/slot: free or in use. */ enum pte_status status; - /* Address of the page in DMA space. */ - dma_addr_t dma_addr; - - /* Linux page descriptor for the page described by this structure. */ - struct page *page; - /* * Index for alignment into host vaddrs. * When a user specifies a host address for a mapping, that address may @@ -119,6 +113,12 @@ struct gasket_page_table_entry { */ int offset; + /* Address of the page in DMA space. */ + dma_addr_t dma_addr; + + /* Linux page descriptor for the page described by this structure. */ + struct page *page; + /* * If this is an extended and first-level entry, sublevel points * to the second-level entries underneath this entry. -- 2.19.0.605.g01d371f741-goog
[PATCH 00/10] staging: gasket: Apex uncloaks, plus fixes
From: Todd Poynor Add a description of the Gasket framework device Apex (it's a Google Edge TPU machine learning accelerator soon to be released), plus a handful of fixes. Nick Ewalt (7): staging: gasket: fix DMA direction for extended page tables staging: gasket: fix data page unmap DMA direction staging: gasket: page_table: don't unmap coherent pages staging: gasket: fix gasket_free_coherent_memory metadata frees staging: gasket: cleanup if dma_map_page fails in gasket_perform_mapping staging: gasket: page_table: use total_entries for max ext lvl0 page idx staging: gasket: page_table: handle failed dma_map_page Todd Poynor (3): staging: gasket: Kconfig: describe Apex as an Edge TPU device staging: gasket: interrupt: remove PCI-MSIX-specific status check staging: gasket: page table: use GFP_KERNEL for dma_alloc_coherent drivers/staging/gasket/Kconfig | 5 +- drivers/staging/gasket/gasket_interrupt.c | 5 -- drivers/staging/gasket/gasket_page_table.c | 61 +++--- 3 files changed, 45 insertions(+), 26 deletions(-) -- 2.19.0.397.gdd90340f6a-goog
[PATCH 01/10] staging: gasket: Kconfig: describe Apex as an Edge TPU device
From: Todd Poynor Add a brief description and URL for more information on the Apex device, an Edge TPU (Tensorflow Processing Unit) machine learning accelerator. Signed-off-by: Todd Poynor --- drivers/staging/gasket/Kconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/gasket/Kconfig b/drivers/staging/gasket/Kconfig index 970e299046c37..e82b85541f7ef 100644 --- a/drivers/staging/gasket/Kconfig +++ b/drivers/staging/gasket/Kconfig @@ -14,8 +14,9 @@ config STAGING_APEX_DRIVER tristate "Apex Driver" depends on STAGING_GASKET_FRAMEWORK help - This driver supports the Apex device. Say Y if you want to - include this driver in the kernel. + This driver supports the Apex Edge TPU device. See + https://cloud.google.com/edge-tpu/ for more information. + Say Y if you want to include this driver in the kernel. To compile this driver as a module, choose M here. The module will be called "apex". -- 2.19.0.397.gdd90340f6a-goog
[PATCH 07/10] staging: gasket: fix gasket_free_coherent_memory metadata frees
From: Nick Ewalt Free gasket_coherent_page_entries metadata memory, update data structures accordingly. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 33d98043953a5..c1ce8f984f8e0 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -1353,6 +1353,11 @@ int gasket_free_coherent_memory(struct gasket_dev *gasket_dev, u64 size, gasket_dev->coherent_buffer.virt_base = NULL; gasket_dev->coherent_buffer.phys_base = 0; } + + kfree(gasket_dev->page_table[index]->coherent_pages); + gasket_dev->page_table[index]->coherent_pages = NULL; + gasket_dev->page_table[index]->num_coherent_pages = 0; + return 0; } -- 2.19.0.397.gdd90340f6a-goog
[PATCH 10/10] staging: gasket: page_table: handle failed dma_map_page
From: Nick Ewalt Handle dma_map_page failing in gasket_alloc_extended_subtable: free memory, don't add invalid page table entry. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 8364b49f147c2..964146f0df526 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -902,6 +902,17 @@ static int gasket_alloc_extended_subtable(struct gasket_page_table *pg_tbl, /* Map the page into DMA space. */ pte->dma_addr = dma_map_page(pg_tbl->device, pte->page, 0, PAGE_SIZE, DMA_TO_DEVICE); + if (dma_mapping_error(pg_tbl->device, pte->dma_addr)) { + dev_dbg(pg_tbl->device, + "%s: fail to map page [pfn %lx phys %llx]\n", + __func__, page_to_pfn(pte->page), + page_to_phys(pte->page)); + + free_page(page_addr); + vfree(pte->sublevel); + memset(pte, 0, sizeof(struct gasket_page_table_entry)); + return -ENOMEM; + } /* make the addresses available to the device */ dma_addr = (pte->dma_addr + pte->offset) | GASKET_VALID_SLOT_FLAG; -- 2.19.0.397.gdd90340f6a-goog
[PATCH 06/10] staging: gasket: page_table: don't unmap coherent pages
From: Nick Ewalt Only call dma_unmap_page if there was an associated dma_map_page call. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 8fe27e7d1b53c..33d98043953a5 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -610,7 +610,7 @@ static void gasket_perform_unmapping(struct gasket_page_table *pg_tbl, /* release the address from the driver, */ if (ptes[i].status == PTE_INUSE) { - if (ptes[i].dma_addr) { + if (ptes[i].page && ptes[i].dma_addr) { dma_unmap_page(pg_tbl->device, ptes[i].dma_addr, PAGE_SIZE, DMA_BIDIRECTIONAL); } -- 2.19.0.397.gdd90340f6a-goog
[PATCH 08/10] staging: gasket: cleanup if dma_map_page fails in gasket_perform_mapping
From: Nick Ewalt Previously pages would have never been unmapped in this case. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 33 +- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index c1ce8f984f8e0..779ad2f23ef9b 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -433,6 +433,19 @@ static int is_coherent(struct gasket_page_table *pg_tbl, ulong host_addr) return min <= host_addr && host_addr < max; } +/* Safely return a page to the OS. */ +static bool gasket_release_page(struct page *page) +{ + if (!page) + return false; + + if (!PageReserved(page)) + SetPageDirty(page); + put_page(page); + + return true; +} + /* * Get and map last level page table buffers. * @@ -500,6 +513,13 @@ static int gasket_perform_mapping(struct gasket_page_table *pg_tbl, (unsigned long long)ptes[i].dma_addr, (void *)page_to_pfn(page), (void *)page_to_phys(page)); + + /* clean up */ + if (gasket_release_page(ptes[i].page)) + --pg_tbl->num_active_pages; + + memset(&ptes[i], 0, + sizeof(struct gasket_page_table_entry)); return -1; } } @@ -571,19 +591,6 @@ static int gasket_alloc_simple_entries(struct gasket_page_table *pg_tbl, return 0; } -/* Safely return a page to the OS. */ -static bool gasket_release_page(struct page *page) -{ - if (!page) - return false; - - if (!PageReserved(page)) - SetPageDirty(page); - put_page(page); - - return true; -} - /* * Unmap and release mapped pages. * The page table mutex must be held by the caller. -- 2.19.0.397.gdd90340f6a-goog
[PATCH 04/10] staging: gasket: fix DMA direction for extended page tables
From: Nick Ewalt Extended page tables should be mapped as DMA_TO_DEVICE, not bi-directional. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 71b77da2e18ca..e86bdc5fc79d2 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -320,7 +320,7 @@ static void gasket_free_extended_subtable(struct gasket_page_table *pg_tbl, if (pte->dma_addr) dma_unmap_page(pg_tbl->device, pte->dma_addr, PAGE_SIZE, - DMA_BIDIRECTIONAL); + DMA_TO_DEVICE); vfree(pte->sublevel); @@ -894,7 +894,7 @@ static int gasket_alloc_extended_subtable(struct gasket_page_table *pg_tbl, /* Map the page into DMA space. */ pte->dma_addr = dma_map_page(pg_tbl->device, pte->page, 0, PAGE_SIZE, -DMA_BIDIRECTIONAL); +DMA_TO_DEVICE); /* make the addresses available to the device */ dma_addr = (pte->dma_addr + pte->offset) | GASKET_VALID_SLOT_FLAG; -- 2.19.0.397.gdd90340f6a-goog
[PATCH 02/10] staging: gasket: interrupt: remove PCI-MSIX-specific status check
From: Todd Poynor Devices not using MSIX don't use the msix_initialized field, so don't require it to be set in the interrupt system status check. The general check for interrupts configured that follows can cover both MSIX and device-managed interrupts. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_interrupt.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c index 2cd262be65ca0..49d47afad64fa 100644 --- a/drivers/staging/gasket/gasket_interrupt.c +++ b/drivers/staging/gasket/gasket_interrupt.c @@ -478,11 +478,6 @@ int gasket_interrupt_system_status(struct gasket_dev *gasket_dev) return GASKET_STATUS_DEAD; } - if (!gasket_dev->interrupt_data->msix_configured) { - dev_dbg(gasket_dev->dev, "Interrupt not initialized\n"); - return GASKET_STATUS_LAMED; - } - if (gasket_dev->interrupt_data->num_configured != gasket_dev->interrupt_data->num_interrupts) { dev_dbg(gasket_dev->dev, -- 2.19.0.397.gdd90340f6a-goog
[PATCH 03/10] staging: gasket: page table: use GFP_KERNEL for dma_alloc_coherent
From: Todd Poynor Flags should be specified for dma_alloc_coherent() call. Use GFP_KERNEL, it's fine to sleep here. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 53492f4fad6aa..71b77da2e18ca 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -1287,7 +1287,7 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size, return -EINVAL; mem = dma_alloc_coherent(gasket_get_device(gasket_dev), -num_pages * PAGE_SIZE, &handle, 0); +num_pages * PAGE_SIZE, &handle, GFP_KERNEL); if (!mem) goto nomem; -- 2.19.0.397.gdd90340f6a-goog
[PATCH 05/10] staging: gasket: fix data page unmap DMA direction
From: Nick Ewalt The DMA direction supplied to dma_unmap_page should match the corresponding dma_map_page call, which is mapped bi-directional. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index e86bdc5fc79d2..8fe27e7d1b53c 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -612,7 +612,7 @@ static void gasket_perform_unmapping(struct gasket_page_table *pg_tbl, if (ptes[i].status == PTE_INUSE) { if (ptes[i].dma_addr) { dma_unmap_page(pg_tbl->device, ptes[i].dma_addr, - PAGE_SIZE, DMA_FROM_DEVICE); + PAGE_SIZE, DMA_BIDIRECTIONAL); } if (gasket_release_page(ptes[i].page)) --pg_tbl->num_active_pages; -- 2.19.0.397.gdd90340f6a-goog
[PATCH 09/10] staging: gasket: page_table: use total_entries for max ext lvl0 page idx
From: Nick Ewalt The maximum number of entries in the page table is configurable at initialization time and should be used in gasket_extended_lvl0_page_idx. Signed-off-by: Nick Ewalt Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 779ad2f23ef9b..8364b49f147c2 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -562,7 +562,7 @@ static ulong gasket_extended_lvl0_page_idx(struct gasket_page_table *pg_tbl, ulong dev_addr) { return (dev_addr >> GASKET_EXTENDED_LVL0_SHIFT) & - ((1 << GASKET_EXTENDED_LVL0_WIDTH) - 1); + (pg_tbl->config.total_entries - 1); } /* -- 2.19.0.397.gdd90340f6a-goog
Re: [GIT PULL] Staging/IIO driver patches for 4.19-rc1
On Tue, Aug 28, 2018 at 3:38 AM Ahmed S. Darwish wrote: >[...] > On Sat, 18 Aug 2018 17:57:24 +0200, Greg KH wrote: > [...] > > addition of some new IIO drivers. Also added was a "gasket" driver from > > Google that needs loads of work and the erofs filesystem. > > > > Why are we adding __a whole new in-kernel framework__ for > developing basic user-space drivers? > > We already have a frameowrk for that, and it's UIO. [1] The UIO > code is a very stable and simple subsystem; it's also heavily used > in the embedded industry.. Yeah, it's clear all the userspace I/O framework code needs to move to UIO with some patches to add agreed-upon new pieces. I think everyone agrees this code shouldn't move out of staging until that happens. A whole lot of work is needed on these drivers, and UIO conversion is on my list to address soon. > > I've looked at the gasket documentation [2], and the first user > of this new in-kernel API [3], and this is almost replicating UIO > it's not funny. [4] True, the gasket APIs adds some extra new > conveniences (PCI BAR re-mapping, MSI, ..), but there's no > technical reason this cannot be added to the UIO code instead. > > More-over, the exposed user-space API is just some ioctls. So if > google hase some shipped user-space code that is using this, > hopefully the driver can still be re-implemented through UIO > without changing these bits.. > > Thanks, > > [1] https://www.kernel.org/doc/html/v4.18/driver-api/uio-howto.html > [2] drivers/staging/gasket/gasket_core.h :: struct gasket_driver_desc > [3] drivers/staging/gasket/apex_driver.c > [4] include/linux/uio_driver.h > > -- > Darwi > http://darwish.chasingpointers.com Thanks -- Todd
Re: [PATCH 01/16] MAINTAINERS: Switch a maintainer for drivers/staging/gasket
On Thu, Aug 9, 2018 at 11:14 PM Greg Kroah-Hartman wrote: > > On Thu, Aug 09, 2018 at 08:40:06PM -0700, John Joseph wrote: > > Acked-by: John Joseph > > Why are you acking something you supposidly already signed-off on? Sorry, my fault, wasn't sure what the protocol was for these so I suggested he send an Acked-by as well, can drop that. > > > On Thu, Aug 9, 2018 at 8:20 PM, Todd Poynor wrote: > > > From: Todd Poynor > > > > > > Todd Poynor takes over for John Joseph. > > > > > > Signed-off-by: John Joseph > > > Signed-off-by: Todd Poynor > > Did you not really provide your signed-off-by here? We generated this together, the S-o-b is valid, > > totally confused, > > greg k-h
[PATCH 00/16] staging: gasket: return of the son of cleanups
From: Todd Poynor Remove extraneous memory barriers, refactor PCI-specific code in prep for platform devices in the near future, general cleanups, and make de facto maintainership official. Todd Poynor (16): MAINTAINERS: Switch a maintainer for drivers/staging/gasket staging: gasket: core: remove debug log that could crash staging: gasket: core: fix line continuation indent in gasket_alloc_dev staging: gasket: core: remove kobj_name param from gasket_alloc_dev staging: gasket: core: remove ftrace-style debug logs staging: gasket: remove gasket_exit() staging: gasket: page table: remove unnecessary NULL check staging: gasket: page table: use dma_mapping_error for error detection staging: gasket: core: switch to relaxed memory-mapped I/O staging: gasket: page table: remove extraneous memory barriers staging: gasket: core: factor out generic device add code from PCI code staging: gasket: core: factor out generic device remove code from PCI staging: gasket: core: rename lookup_internal_desc to be PCI-specific staging: gasket: interrupt: refactor PCI MSIX-specific handler code staging: gasket: interrupt: simplify interrupt init parameters staging: gasket: interrupt: remove unimplemented interrupt types MAINTAINERS| 2 +- drivers/staging/gasket/gasket_core.c | 138 +++-- drivers/staging/gasket/gasket_core.h | 19 +-- drivers/staging/gasket/gasket_interrupt.c | 105 ++-- drivers/staging/gasket/gasket_interrupt.h | 24 +--- drivers/staging/gasket/gasket_page_table.c | 24 ++-- 6 files changed, 124 insertions(+), 188 deletions(-) -- 2.18.0.597.ga71716f1ad-goog
[PATCH 07/16] staging: gasket: page table: remove unnecessary NULL check
From: Todd Poynor gasket_alloc_coherent_memory remove unnecessary NULL check for coherent_pages. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index d4c5f8aa7dd34..bd921dc6094de 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -1328,10 +1328,8 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size, num_pages * PAGE_SIZE, mem, handle); } - if (gasket_dev->page_table[index]->coherent_pages) { - kfree(gasket_dev->page_table[index]->coherent_pages); - gasket_dev->page_table[index]->coherent_pages = NULL; - } + kfree(gasket_dev->page_table[index]->coherent_pages); + gasket_dev->page_table[index]->coherent_pages = NULL; gasket_dev->page_table[index]->num_coherent_pages = 0; return -ENOMEM; } -- 2.18.0.597.ga71716f1ad-goog
[PATCH 03/16] staging: gasket: core: fix line continuation indent in gasket_alloc_dev
From: Todd Poynor Previous cleanups missed a case of multi-line function call with line continuation parameters not aligned per kernel style. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 37d14e30ffa21..3fb805204d700 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -231,8 +231,9 @@ static int gasket_alloc_dev(struct gasket_internal_desc *internal_desc, dev_info->devt = MKDEV(driver_desc->major, driver_desc->minor + gasket_dev->dev_idx); - dev_info->device = device_create(internal_desc->class, parent, - dev_info->devt, gasket_dev, dev_info->name); + dev_info->device = + device_create(internal_desc->class, parent, dev_info->devt, + gasket_dev, dev_info->name); /* cdev has not yet been added; cdev_added is 0 */ dev_info->gasket_dev_ptr = gasket_dev; -- 2.18.0.597.ga71716f1ad-goog
[PATCH 05/16] staging: gasket: core: remove ftrace-style debug logs
From: Todd Poynor Remove debug logs that only indicate the name of the entered function, in favor of using ftrace for function tracing style logs. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 5f54b3615f67c..0fe5b86b294c8 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1789,7 +1789,6 @@ static int __init gasket_init(void) { int i; - pr_debug("%s\n", __func__); mutex_lock(&g_mutex); for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) { g_descs[i].driver_desc = NULL; @@ -1804,7 +1803,6 @@ static int __init gasket_init(void) static void __exit gasket_exit(void) { - pr_debug("%s\n", __func__); } MODULE_DESCRIPTION("Google Gasket driver framework"); MODULE_VERSION(GASKET_FRAMEWORK_VERSION); -- 2.18.0.597.ga71716f1ad-goog
[PATCH 02/16] staging: gasket: core: remove debug log that could crash
From: Todd Poynor A debug log in gasket_alloc_dev() is issued regardless of whether the device pointer used returned success or error. The log isn't that useful anyway, remove it. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index d12ab560411f7..37d14e30ffa21 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -234,8 +234,6 @@ static int gasket_alloc_dev(struct gasket_internal_desc *internal_desc, dev_info->device = device_create(internal_desc->class, parent, dev_info->devt, gasket_dev, dev_info->name); - dev_dbg(dev_info->device, "Gasket device allocated.\n"); - /* cdev has not yet been added; cdev_added is 0 */ dev_info->gasket_dev_ptr = gasket_dev; /* ownership is all 0, indicating no owner or opens. */ -- 2.18.0.597.ga71716f1ad-goog
[PATCH 13/16] staging: gasket: core: rename lookup_internal_desc to be PCI-specific
From: Todd Poynor Rename lookup_internal_desc() to lookup_pci_internal_desc() to reflect use for PCI devices only, in prep for non-PCI devices in the future. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 5e048f6e16e12..99f3f11d75ce2 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -651,13 +651,13 @@ void gasket_disable_device(struct gasket_dev *gasket_dev) EXPORT_SYMBOL(gasket_disable_device); /* - * Registered descriptor lookup. + * Registered driver descriptor lookup for PCI devices. * * Precondition: Called with g_mutex held (to avoid a race on return). * Returns NULL if no matching device was found. */ static struct gasket_internal_desc * -lookup_internal_desc(struct pci_dev *pci_dev) +lookup_pci_internal_desc(struct pci_dev *pci_dev) { int i; @@ -1488,7 +1488,7 @@ int gasket_pci_add_device(struct pci_dev *pci_dev, dev_dbg(&pci_dev->dev, "add PCI gasket device\n"); mutex_lock(&g_mutex); - internal_desc = lookup_internal_desc(pci_dev); + internal_desc = lookup_pci_internal_desc(pci_dev); mutex_unlock(&g_mutex); if (!internal_desc) { dev_err(&pci_dev->dev, @@ -1536,7 +1536,7 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev) struct gasket_dev *gasket_dev = NULL; /* Find the device desc. */ mutex_lock(&g_mutex); - internal_desc = lookup_internal_desc(pci_dev); + internal_desc = lookup_pci_internal_desc(pci_dev); if (!internal_desc) { mutex_unlock(&g_mutex); return; -- 2.18.0.597.ga71716f1ad-goog
[PATCH 11/16] staging: gasket: core: factor out generic device add code from PCI code
From: Todd Poynor Split out generic gasket device add code from the code for adding a PCI gasket device, in prep for other gasket device types in the future. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 76 ++-- 1 file changed, 48 insertions(+), 28 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index aee819f379e9a..ce8ae226f82d9 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1419,6 +1419,48 @@ int gasket_enable_device(struct gasket_dev *gasket_dev) } EXPORT_SYMBOL(gasket_enable_device); +static int __gasket_add_device(struct device *parent_dev, + struct gasket_internal_desc *internal_desc, + struct gasket_dev **gasket_devp) +{ + int ret; + struct gasket_dev *gasket_dev; + const struct gasket_driver_desc *driver_desc = + internal_desc->driver_desc; + + ret = gasket_alloc_dev(internal_desc, parent_dev, &gasket_dev); + if (ret) + return ret; + if (IS_ERR(gasket_dev->dev_info.device)) { + dev_err(parent_dev, "Cannot create %s device %s [ret = %ld]\n", + driver_desc->name, gasket_dev->dev_info.name, + PTR_ERR(gasket_dev->dev_info.device)); + ret = -ENODEV; + goto free_gasket_dev; + } + + ret = gasket_sysfs_create_mapping(gasket_dev->dev_info.device, + gasket_dev); + if (ret) + goto remove_device; + + ret = gasket_sysfs_create_entries(gasket_dev->dev_info.device, + gasket_sysfs_generic_attrs); + if (ret) + goto remove_sysfs_mapping; + + *gasket_devp = gasket_dev; + return 0; + +remove_sysfs_mapping: + gasket_sysfs_remove_mapping(gasket_dev->dev_info.device); +remove_device: + device_destroy(internal_desc->class, gasket_dev->dev_info.devt); +free_gasket_dev: + gasket_free_dev(gasket_dev); + return ret; +} + /* * Add PCI gasket device. * @@ -1433,7 +1475,6 @@ int gasket_pci_add_device(struct pci_dev *pci_dev, int ret; struct gasket_internal_desc *internal_desc; struct gasket_dev *gasket_dev; - const struct gasket_driver_desc *driver_desc; struct device *parent; dev_dbg(&pci_dev->dev, "add PCI gasket device\n"); @@ -1447,29 +1488,15 @@ int gasket_pci_add_device(struct pci_dev *pci_dev, return -ENODEV; } - driver_desc = internal_desc->driver_desc; - parent = &pci_dev->dev; - ret = gasket_alloc_dev(internal_desc, parent, &gasket_dev); + ret = __gasket_add_device(parent, internal_desc, &gasket_dev); if (ret) return ret; - gasket_dev->pci_dev = pci_dev; - if (IS_ERR_OR_NULL(gasket_dev->dev_info.device)) { - pr_err("Cannot create %s device %s [ret = %ld]\n", - driver_desc->name, gasket_dev->dev_info.name, - PTR_ERR(gasket_dev->dev_info.device)); - ret = -ENODEV; - goto fail1; - } + gasket_dev->pci_dev = pci_dev; ret = gasket_setup_pci(pci_dev, gasket_dev); if (ret) - goto fail2; - - ret = gasket_sysfs_create_mapping(gasket_dev->dev_info.device, - gasket_dev); - if (ret) - goto fail3; + goto cleanup_pci; /* * Once we've created the mapping structures successfully, attempt to @@ -1480,23 +1507,16 @@ int gasket_pci_add_device(struct pci_dev *pci_dev, if (ret) { dev_err(gasket_dev->dev, "Cannot create sysfs pci link: %d\n", ret); - goto fail3; + goto cleanup_pci; } - ret = gasket_sysfs_create_entries(gasket_dev->dev_info.device, - gasket_sysfs_generic_attrs); - if (ret) - goto fail4; *gasket_devp = gasket_dev; return 0; -fail4: -fail3: - gasket_sysfs_remove_mapping(gasket_dev->dev_info.device); -fail2: +cleanup_pci: gasket_cleanup_pci(gasket_dev); + gasket_sysfs_remove_mapping(gasket_dev->dev_info.device); device_destroy(internal_desc->class, gasket_dev->dev_info.devt); -fail1: gasket_free_dev(gasket_dev); return ret; } -- 2.18.0.597.ga71716f1ad-goog
[PATCH 12/16] staging: gasket: core: factor out generic device remove code from PCI
From: Todd Poynor Separate code for generic parts of gasket device removal sequence from the PCI device removal code, in prep for non-PCI devices later. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index ce8ae226f82d9..5e048f6e16e12 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1461,6 +1461,14 @@ static int __gasket_add_device(struct device *parent_dev, return ret; } +static void __gasket_remove_device(struct gasket_internal_desc *internal_desc, + struct gasket_dev *gasket_dev) +{ + gasket_sysfs_remove_mapping(gasket_dev->dev_info.device); + device_destroy(internal_desc->class, gasket_dev->dev_info.devt); + gasket_free_dev(gasket_dev); +} + /* * Add PCI gasket device. * @@ -1515,9 +1523,7 @@ int gasket_pci_add_device(struct pci_dev *pci_dev, cleanup_pci: gasket_cleanup_pci(gasket_dev); - gasket_sysfs_remove_mapping(gasket_dev->dev_info.device); - device_destroy(internal_desc->class, gasket_dev->dev_info.devt); - gasket_free_dev(gasket_dev); + __gasket_remove_device(internal_desc, gasket_dev); return ret; } EXPORT_SYMBOL(gasket_pci_add_device); @@ -1528,7 +1534,6 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev) int i; struct gasket_internal_desc *internal_desc; struct gasket_dev *gasket_dev = NULL; - const struct gasket_driver_desc *driver_desc; /* Find the device desc. */ mutex_lock(&g_mutex); internal_desc = lookup_internal_desc(pci_dev); @@ -1538,8 +1543,6 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev) } mutex_unlock(&g_mutex); - driver_desc = internal_desc->driver_desc; - /* Now find the specific device */ mutex_lock(&internal_desc->mutex); for (i = 0; i < GASKET_DEV_MAX; i++) { @@ -1558,10 +1561,7 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev) internal_desc->driver_desc->name); gasket_cleanup_pci(gasket_dev); - - gasket_sysfs_remove_mapping(gasket_dev->dev_info.device); - device_destroy(internal_desc->class, gasket_dev->dev_info.devt); - gasket_free_dev(gasket_dev); + __gasket_remove_device(internal_desc, gasket_dev); } EXPORT_SYMBOL(gasket_pci_remove_device); -- 2.18.0.597.ga71716f1ad-goog
[PATCH 06/16] staging: gasket: remove gasket_exit()
From: Todd Poynor Remove now-empty gasket_exit() function. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 0fe5b86b294c8..aee819f379e9a 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1801,12 +1801,8 @@ static int __init gasket_init(void) return 0; } -static void __exit gasket_exit(void) -{ -} MODULE_DESCRIPTION("Google Gasket driver framework"); MODULE_VERSION(GASKET_FRAMEWORK_VERSION); MODULE_LICENSE("GPL v2"); MODULE_AUTHOR("Rob Springer "); module_init(gasket_init); -module_exit(gasket_exit); -- 2.18.0.597.ga71716f1ad-goog
[PATCH 08/16] staging: gasket: page table: use dma_mapping_error for error detection
From: Todd Poynor gasket_perform_mapping() call dma_mapping_error() to determine if mapping failed. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index bd921dc6094de..4d2499269499b 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -493,7 +493,8 @@ static int gasket_perform_mapping(struct gasket_page_table *pg_tbl, (void *)page_to_pfn(page), (unsigned long long)ptes[i].dma_addr); - if (ptes[i].dma_addr == -1) { + if (dma_mapping_error(pg_tbl->device, + ptes[i].dma_addr)) { dev_dbg(pg_tbl->device, "%s i %d -> fail to map page %llx " "[pfn %p ohys %p]\n", -- 2.18.0.597.ga71716f1ad-goog
[PATCH 14/16] staging: gasket: interrupt: refactor PCI MSIX-specific handler code
From: Todd Poynor Split interrupt handler into PCI MSIX-specific and generic functions, for adding non-MSIX handlers in the future. Move MSIX init code together,, out of generic init path. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_interrupt.c | 48 --- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c index 1cfbc120f2284..f94e4ea9a7ded 100644 --- a/drivers/staging/gasket/gasket_interrupt.c +++ b/drivers/staging/gasket/gasket_interrupt.c @@ -157,9 +157,22 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev) } } -static irqreturn_t gasket_msix_interrupt_handler(int irq, void *dev_id) +static void +gasket_handle_interrupt(struct gasket_interrupt_data *interrupt_data, + int interrupt_index) { struct eventfd_ctx *ctx; + + trace_gasket_interrupt_event(interrupt_data->name, interrupt_index); + ctx = interrupt_data->eventfd_ctxs[interrupt_index]; + if (ctx) + eventfd_signal(ctx, 1); + + ++(interrupt_data->interrupt_counts[interrupt_index]); +} + +static irqreturn_t gasket_msix_interrupt_handler(int irq, void *dev_id) +{ struct gasket_interrupt_data *interrupt_data = dev_id; int interrupt = -1; int i; @@ -175,14 +188,7 @@ static irqreturn_t gasket_msix_interrupt_handler(int irq, void *dev_id) pr_err("Received unknown irq %d\n", irq); return IRQ_HANDLED; } - trace_gasket_interrupt_event(interrupt_data->name, interrupt); - - ctx = interrupt_data->eventfd_ctxs[interrupt]; - if (ctx) - eventfd_signal(ctx, 1); - - ++(interrupt_data->interrupt_counts[interrupt]); - + gasket_handle_interrupt(interrupt_data, interrupt); return IRQ_HANDLED; } @@ -192,6 +198,12 @@ gasket_interrupt_msix_init(struct gasket_interrupt_data *interrupt_data) int ret = 1; int i; + interrupt_data->msix_entries = + kcalloc(interrupt_data->num_interrupts, + sizeof(struct msix_entry), GFP_KERNEL); + if (!interrupt_data->msix_entries) + return -ENOMEM; + for (i = 0; i < interrupt_data->num_interrupts; i++) { interrupt_data->msix_entries[i].entry = i; interrupt_data->msix_entries[i].vector = 0; @@ -343,20 +355,10 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, interrupt_data->num_configured = 0; interrupt_data->wire_interrupt_offsets = wire_int_offsets; - /* Allocate all dynamic structures. */ - interrupt_data->msix_entries = kcalloc(num_interrupts, - sizeof(struct msix_entry), - GFP_KERNEL); - if (!interrupt_data->msix_entries) { - kfree(interrupt_data); - return -ENOMEM; - } - interrupt_data->eventfd_ctxs = kcalloc(num_interrupts, sizeof(struct eventfd_ctx *), GFP_KERNEL); if (!interrupt_data->eventfd_ctxs) { - kfree(interrupt_data->msix_entries); kfree(interrupt_data); return -ENOMEM; } @@ -366,7 +368,6 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, GFP_KERNEL); if (!interrupt_data->interrupt_counts) { kfree(interrupt_data->eventfd_ctxs); - kfree(interrupt_data->msix_entries); kfree(interrupt_data); return -ENOMEM; } @@ -417,6 +418,7 @@ gasket_interrupt_msix_cleanup(struct gasket_interrupt_data *interrupt_data) if (interrupt_data->msix_configured) pci_disable_msix(interrupt_data->pci_dev); interrupt_data->msix_configured = 0; + kfree(interrupt_data->msix_entries); } int gasket_interrupt_reinit(struct gasket_dev *gasket_dev) @@ -448,10 +450,11 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev) } if (ret) { - /* Failing to setup MSIx will cause the device + /* Failing to setup interrupts will cause the device * to report GASKET_STATUS_LAMED, but is not fatal. */ - dev_warn(gasket_dev->dev, "Couldn't init msix: %d\n", ret); + dev_warn(gasket_dev->dev, "Couldn't reinit interrupts: %d\n", +ret); return 0; } @@ -497,7 +500,6 @@ void gasket_interrupt_cleanup(struct gasket_dev *gasket_dev) kfree(interrupt_data->interru
[PATCH 16/16] staging: gasket: interrupt: remove unimplemented interrupt types
From: Todd Poynor Interrupt types PCI_MSI and PLATFORM_WIRE are unused and unimplemented. Remove these. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.h | 11 drivers/staging/gasket/gasket_interrupt.c | 34 +-- 2 files changed, 1 insertion(+), 44 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h index fd7e75b765a6d..0203460f48957 100644 --- a/drivers/staging/gasket/gasket_core.h +++ b/drivers/staging/gasket/gasket_core.h @@ -50,8 +50,6 @@ enum gasket_interrupt_packing { /* Type of the interrupt supported by the device. */ enum gasket_interrupt_type { PCI_MSIX = 0, - PCI_MSI = 1, - PLATFORM_WIRE = 2, }; /* @@ -69,12 +67,6 @@ struct gasket_interrupt_desc { int packing; }; -/* Offsets to the wire interrupt handling registers */ -struct gasket_wire_interrupt_offsets { - u64 pending_bit_array; - u64 mask_array; -}; - /* * This enum is used to identify memory regions being part of the physical * memory that belongs to a device. @@ -384,9 +376,6 @@ struct gasket_driver_desc { */ struct gasket_coherent_buffer_desc coherent_buffer_description; - /* Offset of wire interrupt registers. */ - const struct gasket_wire_interrupt_offsets *wire_interrupt_offsets; - /* Interrupt type. (One of gasket_interrupt_type). */ int interrupt_type; diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c index eb5dfbe08e214..2cd262be65ca0 100644 --- a/drivers/staging/gasket/gasket_interrupt.c +++ b/drivers/staging/gasket/gasket_interrupt.c @@ -45,9 +45,6 @@ struct gasket_interrupt_data { /* The width of a single interrupt in a packed interrupt register. */ int pack_width; - /* offset of wire interrupt registers */ - const struct gasket_wire_interrupt_offsets *wire_interrupt_offsets; - /* * Design-wise, these elements should be bundled together, but * pci_enable_msix's interface requires that they be managed @@ -92,19 +89,6 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev) dev_dbg(gasket_dev->dev, "Running interrupt setup\n"); - if (interrupt_data->type == PLATFORM_WIRE || - interrupt_data->type == PCI_MSI) { - /* Nothing needs to be done for platform or PCI devices. */ - return; - } - - if (interrupt_data->type != PCI_MSIX) { - dev_dbg(gasket_dev->dev, - "Cannot handle unsupported interrupt type %d\n", - interrupt_data->type); - return; - } - /* Setup the MSIX table. */ for (i = 0; i < interrupt_data->num_interrupts; i++) { @@ -351,8 +335,6 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev) interrupt_data->interrupt_bar_index = driver_desc->interrupt_bar_index; interrupt_data->pack_width = driver_desc->interrupt_pack_width; interrupt_data->num_configured = 0; - interrupt_data->wire_interrupt_offsets = - driver_desc->wire_interrupt_offsets; interrupt_data->eventfd_ctxs = kcalloc(driver_desc->num_interrupts, sizeof(struct eventfd_ctx *), @@ -379,12 +361,7 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev) force_msix_interrupt_unmasking(gasket_dev); break; - case PCI_MSI: - case PLATFORM_WIRE: default: - dev_err(gasket_dev->dev, - "Cannot handle unsupported interrupt type %d\n", - interrupt_data->type); ret = -EINVAL; } @@ -439,12 +416,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev) force_msix_interrupt_unmasking(gasket_dev); break; - case PCI_MSI: - case PLATFORM_WIRE: default: - dev_dbg(gasket_dev->dev, - "Cannot handle unsupported interrupt type %d\n", - gasket_dev->interrupt_data->type); ret = -EINVAL; } @@ -489,12 +461,8 @@ void gasket_interrupt_cleanup(struct gasket_dev *gasket_dev) gasket_interrupt_msix_cleanup(interrupt_data); break; - case PCI_MSI: - case PLATFORM_WIRE: default: - dev_dbg(gasket_dev->dev, - "Cannot handle unsupported interrupt type %d\n", - interrupt_data->type); + break; } kfree(interrupt_data->interrupt_counts); -- 2.18.0.597.ga71716f1ad-goog
[PATCH 04/16] staging: gasket: core: remove kobj_name param from gasket_alloc_dev
From: Todd Poynor gasket_alloc_dev can retrieve the device name from the parent parameter, a separate parameter isn't needed for this. Rename the variable to better reflect its meaning, as the name of the parent device for which a gasket device is being allocated. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 17 - 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 3fb805204d700..5f54b3615f67c 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -189,26 +189,26 @@ static int gasket_find_dev_slot(struct gasket_internal_desc *internal_desc, * Returns 0 if successful, a negative error code otherwise. */ static int gasket_alloc_dev(struct gasket_internal_desc *internal_desc, - struct device *parent, struct gasket_dev **pdev, - const char *kobj_name) + struct device *parent, struct gasket_dev **pdev) { int dev_idx; const struct gasket_driver_desc *driver_desc = internal_desc->driver_desc; struct gasket_dev *gasket_dev; struct gasket_cdev_info *dev_info; + const char *parent_name = dev_name(parent); - pr_debug("Allocating a Gasket device %s.\n", kobj_name); + pr_debug("Allocating a Gasket device, parent %s.\n", parent_name); *pdev = NULL; - dev_idx = gasket_find_dev_slot(internal_desc, kobj_name); + dev_idx = gasket_find_dev_slot(internal_desc, parent_name); if (dev_idx < 0) return dev_idx; gasket_dev = *pdev = kzalloc(sizeof(*gasket_dev), GFP_KERNEL); if (!gasket_dev) { - pr_err("no memory for device %s\n", kobj_name); + pr_err("no memory for device, parent %s\n", parent_name); return -ENOMEM; } internal_desc->devs[dev_idx] = gasket_dev; @@ -217,7 +217,7 @@ static int gasket_alloc_dev(struct gasket_internal_desc *internal_desc, gasket_dev->internal_desc = internal_desc; gasket_dev->dev_idx = dev_idx; - snprintf(gasket_dev->kobj_name, GASKET_NAME_MAX, "%s", kobj_name); + snprintf(gasket_dev->kobj_name, GASKET_NAME_MAX, "%s", parent_name); gasket_dev->dev = get_device(parent); /* gasket_bar_data is uninitialized. */ gasket_dev->num_page_tables = driver_desc->num_page_tables; @@ -1431,13 +1431,12 @@ int gasket_pci_add_device(struct pci_dev *pci_dev, struct gasket_dev **gasket_devp) { int ret; - const char *kobj_name = dev_name(&pci_dev->dev); struct gasket_internal_desc *internal_desc; struct gasket_dev *gasket_dev; const struct gasket_driver_desc *driver_desc; struct device *parent; - pr_debug("add PCI device %s\n", kobj_name); + dev_dbg(&pci_dev->dev, "add PCI gasket device\n"); mutex_lock(&g_mutex); internal_desc = lookup_internal_desc(pci_dev); @@ -1451,7 +1450,7 @@ int gasket_pci_add_device(struct pci_dev *pci_dev, driver_desc = internal_desc->driver_desc; parent = &pci_dev->dev; - ret = gasket_alloc_dev(internal_desc, parent, &gasket_dev, kobj_name); + ret = gasket_alloc_dev(internal_desc, parent, &gasket_dev); if (ret) return ret; gasket_dev->pci_dev = pci_dev; -- 2.18.0.597.ga71716f1ad-goog
[PATCH 01/16] MAINTAINERS: Switch a maintainer for drivers/staging/gasket
From: Todd Poynor Todd Poynor takes over for John Joseph. Signed-off-by: John Joseph Signed-off-by: Todd Poynor --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index af64fe0f0b41f..f3466b5c50482 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5939,7 +5939,7 @@ F:Documentation/gcc-plugins.txt GASKET DRIVER FRAMEWORK M: Rob Springer -M: John Joseph +M: Todd Poynor M: Ben Chan S: Maintained F: drivers/staging/gasket/ -- 2.18.0.597.ga71716f1ad-goog
[PATCH 15/16] staging: gasket: interrupt: simplify interrupt init parameters
From: Todd Poynor Pass the gasket driver descriptor to the interrupt init function, rather than exploding out separate parameters from various fields of that structure. This allows us to make more localized changes to the types of interrupts supported (MSIX vs. wire, etc.) without affecting the calling sequence, and seems nicer for simplification purposes. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 8 +-- drivers/staging/gasket/gasket_interrupt.c | 27 +++ drivers/staging/gasket/gasket_interrupt.h | 24 +--- 3 files changed, 15 insertions(+), 44 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 99f3f11d75ce2..f230bec76ae4e 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1357,13 +1357,7 @@ int gasket_enable_device(struct gasket_dev *gasket_dev) const struct gasket_driver_desc *driver_desc = gasket_dev->internal_desc->driver_desc; - ret = gasket_interrupt_init(gasket_dev, driver_desc->name, - driver_desc->interrupt_type, - driver_desc->interrupts, - driver_desc->num_interrupts, - driver_desc->interrupt_pack_width, - driver_desc->interrupt_bar_index, - driver_desc->wire_interrupt_offsets); + ret = gasket_interrupt_init(gasket_dev); if (ret) { dev_err(gasket_dev->dev, "Critical failure to allocate interrupts: %d\n", ret); diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c index f94e4ea9a7ded..eb5dfbe08e214 100644 --- a/drivers/staging/gasket/gasket_interrupt.c +++ b/drivers/staging/gasket/gasket_interrupt.c @@ -331,31 +331,30 @@ static struct gasket_sysfs_attribute interrupt_sysfs_attrs[] = { GASKET_END_OF_ATTR_ARRAY, }; -int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, - int type, - const struct gasket_interrupt_desc *interrupts, - int num_interrupts, int pack_width, int bar_index, - const struct gasket_wire_interrupt_offsets *wire_int_offsets) +int gasket_interrupt_init(struct gasket_dev *gasket_dev) { int ret; struct gasket_interrupt_data *interrupt_data; + const struct gasket_driver_desc *driver_desc = + gasket_get_driver_desc(gasket_dev); interrupt_data = kzalloc(sizeof(struct gasket_interrupt_data), GFP_KERNEL); if (!interrupt_data) return -ENOMEM; gasket_dev->interrupt_data = interrupt_data; - interrupt_data->name = name; - interrupt_data->type = type; + interrupt_data->name = driver_desc->name; + interrupt_data->type = driver_desc->interrupt_type; interrupt_data->pci_dev = gasket_dev->pci_dev; - interrupt_data->num_interrupts = num_interrupts; - interrupt_data->interrupts = interrupts; - interrupt_data->interrupt_bar_index = bar_index; - interrupt_data->pack_width = pack_width; + interrupt_data->num_interrupts = driver_desc->num_interrupts; + interrupt_data->interrupts = driver_desc->interrupts; + interrupt_data->interrupt_bar_index = driver_desc->interrupt_bar_index; + interrupt_data->pack_width = driver_desc->interrupt_pack_width; interrupt_data->num_configured = 0; - interrupt_data->wire_interrupt_offsets = wire_int_offsets; + interrupt_data->wire_interrupt_offsets = + driver_desc->wire_interrupt_offsets; - interrupt_data->eventfd_ctxs = kcalloc(num_interrupts, + interrupt_data->eventfd_ctxs = kcalloc(driver_desc->num_interrupts, sizeof(struct eventfd_ctx *), GFP_KERNEL); if (!interrupt_data->eventfd_ctxs) { @@ -363,7 +362,7 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, return -ENOMEM; } - interrupt_data->interrupt_counts = kcalloc(num_interrupts, + interrupt_data->interrupt_counts = kcalloc(driver_desc->num_interrupts, sizeof(ulong), GFP_KERNEL); if (!interrupt_data->interrupt_counts) { diff --git a/drivers/staging/gasket/gasket_interrupt.h b/drivers/staging/gasket/gasket_interrupt.h index 835af439e96a9..85526a1374a1a 100644 --- a/drivers/staging/gasket/gasket_interrupt.h +++ b/drivers/staging/gask
[PATCH 09/16] staging: gasket: core: switch to relaxed memory-mapped I/O
From: Todd Poynor Use of readl() is deprecated; readl_relaxed() with appropriate memory barriers is preferred. Switch to relaxed reads and writes for better performance as well. Memory barriers required for I/O vs. normal memory access on Apex devices have already been explicitly coded in the page table routines. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h index 275fd0b345b6e..fd7e75b765a6d 100644 --- a/drivers/staging/gasket/gasket_core.h +++ b/drivers/staging/gasket/gasket_core.h @@ -590,25 +590,25 @@ const char *gasket_num_name_lookup(uint num, static inline ulong gasket_dev_read_64(struct gasket_dev *gasket_dev, int bar, ulong location) { - return readq(&gasket_dev->bar_data[bar].virt_base[location]); + return readq_relaxed(&gasket_dev->bar_data[bar].virt_base[location]); } static inline void gasket_dev_write_64(struct gasket_dev *dev, u64 value, int bar, ulong location) { - writeq(value, &dev->bar_data[bar].virt_base[location]); + writeq_relaxed(value, &dev->bar_data[bar].virt_base[location]); } static inline void gasket_dev_write_32(struct gasket_dev *dev, u32 value, int bar, ulong location) { - writel(value, &dev->bar_data[bar].virt_base[location]); + writel_relaxed(value, &dev->bar_data[bar].virt_base[location]); } static inline u32 gasket_dev_read_32(struct gasket_dev *dev, int bar, ulong location) { - return readl(&dev->bar_data[bar].virt_base[location]); + return readl_relaxed(&dev->bar_data[bar].virt_base[location]); } static inline void gasket_read_modify_write_64(struct gasket_dev *dev, int bar, -- 2.18.0.597.ga71716f1ad-goog
[PATCH 10/16] staging: gasket: page table: remove extraneous memory barriers
From: Todd Poynor Some explicit memory barriers in the page table code are not necessary, either because: (a) The barrier follows a non-relaxed MMIO access that already performs a read or write memory barrier. (b) The barrier follows DMA API calls for which the device-visible effects of IOMMU programming are guaranteed to be flushed to the IOMMU prior to the call returning, and doesn't need to sync with normal memory access. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 15 +-- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 4d2499269499b..53492f4fad6aa 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -317,8 +317,6 @@ static void gasket_free_extended_subtable(struct gasket_page_table *pg_tbl, /* Release the page table from the device */ writeq(0, slot); - /* Force sync around the address release. */ - mb(); if (pte->dma_addr) dma_unmap_page(pg_tbl->device, pte->dma_addr, PAGE_SIZE, @@ -504,8 +502,6 @@ static int gasket_perform_mapping(struct gasket_page_table *pg_tbl, (void *)page_to_phys(page)); return -1; } - /* Wait until the page is mapped. */ - mb(); } /* Make the DMA-space address available to the device. */ @@ -604,12 +600,13 @@ static void gasket_perform_unmapping(struct gasket_page_table *pg_tbl, */ for (i = 0; i < num_pages; i++) { /* release the address from the device, */ - if (is_simple_mapping || ptes[i].status == PTE_INUSE) + if (is_simple_mapping || ptes[i].status == PTE_INUSE) { writeq(0, &slots[i]); - else + } else { ((u64 __force *)slots)[i] = 0; - /* Force sync around the address release. */ - mb(); + /* sync above PTE update before updating mappings */ + wmb(); + } /* release the address from the driver, */ if (ptes[i].status == PTE_INUSE) { @@ -898,8 +895,6 @@ static int gasket_alloc_extended_subtable(struct gasket_page_table *pg_tbl, /* Map the page into DMA space. */ pte->dma_addr = dma_map_page(pg_tbl->device, pte->page, 0, PAGE_SIZE, DMA_BIDIRECTIONAL); - /* Wait until the page is mapped. */ - mb(); /* make the addresses available to the device */ dma_addr = (pte->dma_addr + pte->offset) | GASKET_VALID_SLOT_FLAG; -- 2.18.0.597.ga71716f1ad-goog
[PATCH 10/15] staging: gasket: apex: fold device add/remove logic inline
From: Todd Poynor Gasket device drivers are now in charge of the device add and remove sequences; the framework callbacks for these are deleted. Move the apex device add callback code to the probe function. Apex did not implement the removal callback. Signed-off-by: Todd Poynor --- drivers/staging/gasket/apex_driver.c | 69 +--- 1 file changed, 32 insertions(+), 37 deletions(-) diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index 1d8a100c52885..69ca7fb10eddc 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -448,37 +448,6 @@ static int apex_reset(struct gasket_dev *gasket_dev) return ret; } -static int apex_add_dev_cb(struct gasket_dev *gasket_dev) -{ - ulong page_table_ready, msix_table_ready; - int retries = 0; - - apex_reset(gasket_dev); - - while (retries < APEX_RESET_RETRY) { - page_table_ready = - gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX, - APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_INIT); - msix_table_ready = - gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX, - APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE_INIT); - if (page_table_ready && msix_table_ready) - break; - schedule_timeout(msecs_to_jiffies(APEX_RESET_DELAY)); - retries++; - } - - if (retries == APEX_RESET_RETRY) { - if (!page_table_ready) - dev_err(gasket_dev->dev, "Page table init timed out\n"); - if (!msix_table_ready) - dev_err(gasket_dev->dev, "MSI-X table init timed out\n"); - return -ETIMEDOUT; - } - - return 0; -} - /* * Check permissions for Apex ioctls. * Returns true if the current user may execute this ioctl, and false otherwise. @@ -626,6 +595,8 @@ static int apex_pci_probe(struct pci_dev *pci_dev, const struct pci_device_id *id) { int ret; + ulong page_table_ready, msix_table_ready; + int retries = 0; struct gasket_dev *gasket_dev; ret = pci_enable_device(pci_dev); @@ -644,15 +615,42 @@ static int apex_pci_probe(struct pci_dev *pci_dev, } pci_set_drvdata(pci_dev, gasket_dev); + apex_reset(gasket_dev); + + while (retries < APEX_RESET_RETRY) { + page_table_ready = + gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX, + APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_INIT); + msix_table_ready = + gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX, + APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE_INIT); + if (page_table_ready && msix_table_ready) + break; + schedule_timeout(msecs_to_jiffies(APEX_RESET_DELAY)); + retries++; + } + + if (retries == APEX_RESET_RETRY) { + if (!page_table_ready) + dev_err(gasket_dev->dev, "Page table init timed out\n"); + if (!msix_table_ready) + dev_err(gasket_dev->dev, "MSI-X table init timed out\n"); + ret = -ETIMEDOUT; + goto remove_device; + } + ret = gasket_enable_device(gasket_dev); if (ret) { dev_err(&pci_dev->dev, "error enabling gasket device\n"); - gasket_pci_remove_device(pci_dev); - pci_disable_device(pci_dev); - return ret; + goto remove_device; } return 0; + +remove_device: + gasket_pci_remove_device(pci_dev); + pci_disable_device(pci_dev); + return ret; } static void apex_pci_remove(struct pci_dev *pci_dev) @@ -697,9 +695,6 @@ static struct gasket_driver_desc apex_desc = { .interrupts = apex_interrupts, .interrupt_pack_width = 7, - .add_dev_cb = apex_add_dev_cb, - .remove_dev_cb = NULL, - .sysfs_setup_cb = apex_sysfs_setup_cb, .sysfs_cleanup_cb = NULL, -- 2.18.0.597.ga71716f1ad-goog
[PATCH 06/15] staging: gasket: apex: remove device enable and disable callbacks
From: Todd Poynor These are not implemented for apex, and are now being removed from the gasket framework. Signed-off-by: Todd Poynor --- drivers/staging/gasket/apex_driver.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index b47661442009d..e2bc06b5244f7 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -688,9 +688,6 @@ static struct gasket_driver_desc apex_desc = { .add_dev_cb = apex_add_dev_cb, .remove_dev_cb = NULL, - .enable_dev_cb = NULL, - .disable_dev_cb = NULL, - .sysfs_setup_cb = apex_sysfs_setup_cb, .sysfs_cleanup_cb = NULL, -- 2.18.0.597.ga71716f1ad-goog
[PATCH 11/15] staging: gasket: core: remove sysfs setup and cleanup callbacks
From: Todd Poynor Gasket device drivers now call into the gasket framework to initialize and de-initialize, rather than the other way around. The calling code can perform sysfs setup and cleanup actions without callbacks from the framework. Remove the sysfs setup and cleanup callbacks. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 10 -- drivers/staging/gasket/gasket_core.h | 18 -- 2 files changed, 28 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 0d76e18fcde5b..ace92f107ed58 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1489,18 +1489,9 @@ int gasket_pci_add_device(struct pci_dev *pci_dev, if (ret) goto fail4; - ret = check_and_invoke_callback(gasket_dev, - driver_desc->sysfs_setup_cb); - if (ret) { - dev_err(gasket_dev->dev, "Error in sysfs setup cb: %d\n", ret); - goto fail5; - } - *gasket_devp = gasket_dev; return 0; -fail5: - check_and_invoke_callback(gasket_dev, driver_desc->sysfs_cleanup_cb); fail4: fail3: gasket_sysfs_remove_mapping(gasket_dev->dev_info.device); @@ -1550,7 +1541,6 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev) gasket_cleanup_pci(gasket_dev); - check_and_invoke_callback(gasket_dev, driver_desc->sysfs_cleanup_cb); gasket_sysfs_remove_mapping(gasket_dev->dev_info.device); device_destroy(internal_desc->class, gasket_dev->dev_info.devt); gasket_free_dev(gasket_dev); diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h index 0ef0a2640f0fe..275fd0b345b6e 100644 --- a/drivers/staging/gasket/gasket_core.h +++ b/drivers/staging/gasket/gasket_core.h @@ -444,24 +444,6 @@ struct gasket_driver_desc { */ int (*device_close_cb)(struct gasket_dev *dev); - /* -* sysfs_setup_cb: Callback to set up driver-specific sysfs nodes. -* @dev: Pointer to the gasket_dev struct for this device. -* -* Called during the add gasket device call. -* -*/ - int (*sysfs_setup_cb)(struct gasket_dev *dev); - - /* -* sysfs_cleanup_cb: Callback to clean up driver-specific sysfs nodes. -* @dev: Pointer to the gasket_dev struct for this device. -* -* Called during device disable processing. -* -*/ - int (*sysfs_cleanup_cb)(struct gasket_dev *dev); - /* * get_mappable_regions_cb: Get descriptors of mappable device memory. * @gasket_dev: Pointer to the struct gasket_dev for this device. -- 2.18.0.597.ga71716f1ad-goog
[PATCH 12/15] staging: gasket: apex: move sysfs setup code to probe function
From: Todd Poynor The gasket framework no longer provides callbacks to the device driver for sysfs setup and teardown. Move the sysfs setup code to the device probe function. Apex does not implement sysfs cleanup code. Signed-off-by: Todd Poynor --- drivers/staging/gasket/apex_driver.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index 69ca7fb10eddc..55319619b2e66 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -568,12 +568,6 @@ static struct gasket_sysfs_attribute apex_sysfs_attrs[] = { GASKET_END_OF_ATTR_ARRAY }; -static int apex_sysfs_setup_cb(struct gasket_dev *gasket_dev) -{ - return gasket_sysfs_create_entries(gasket_dev->dev_info.device, - apex_sysfs_attrs); -} - /* On device open, perform a core reinit reset. */ static int apex_device_open_cb(struct gasket_dev *gasket_dev) { @@ -639,6 +633,11 @@ static int apex_pci_probe(struct pci_dev *pci_dev, goto remove_device; } + ret = gasket_sysfs_create_entries(gasket_dev->dev_info.device, + apex_sysfs_attrs); + if (ret) + dev_err(&pci_dev->dev, "error creating device sysfs entries\n"); + ret = gasket_enable_device(gasket_dev); if (ret) { dev_err(&pci_dev->dev, "error enabling gasket device\n"); @@ -695,9 +694,6 @@ static struct gasket_driver_desc apex_desc = { .interrupts = apex_interrupts, .interrupt_pack_width = 7, - .sysfs_setup_cb = apex_sysfs_setup_cb, - .sysfs_cleanup_cb = NULL, - .device_open_cb = apex_device_open_cb, .device_close_cb = apex_device_cleanup, -- 2.18.0.597.ga71716f1ad-goog
[PATCH 08/15] staging: gasket: apex: enable/disable gasket device from apex
From: Todd Poynor Gasket framework now places device drivers in charge of calling APIs to enable and disable gasket device operations. Make the appropriate calls from the apex driver. Signed-off-by: Todd Poynor --- drivers/staging/gasket/apex_driver.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index e2bc06b5244f7..1d8a100c52885 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -643,11 +643,23 @@ static int apex_pci_probe(struct pci_dev *pci_dev, return ret; } + pci_set_drvdata(pci_dev, gasket_dev); + ret = gasket_enable_device(gasket_dev); + if (ret) { + dev_err(&pci_dev->dev, "error enabling gasket device\n"); + gasket_pci_remove_device(pci_dev); + pci_disable_device(pci_dev); + return ret; + } + return 0; } static void apex_pci_remove(struct pci_dev *pci_dev) { + struct gasket_dev *gasket_dev = pci_get_drvdata(pci_dev); + + gasket_disable_device(gasket_dev); gasket_pci_remove_device(pci_dev); pci_disable_device(pci_dev); } -- 2.18.0.597.ga71716f1ad-goog
[PATCH 07/15] staging: gasket: core: let device driver enable/disable gasket device
From: Todd Poynor Move gasket device enable/disable functions from internal calls to external calls from the gasket device drivers. The device driver will call these functions at appropriate times in its processing, placing the device driver in control of this sequence and reducing the need for callbacks from framework back to the device drivers during the enable/disable sequences. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 22 -- drivers/staging/gasket/gasket_core.h | 6 ++ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index b070efaf0d41c..fad4883e6332c 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -628,7 +628,7 @@ static int gasket_add_cdev(struct gasket_cdev_info *dev_info, } /* Disable device operations. */ -static void gasket_disable_dev(struct gasket_dev *gasket_dev) +void gasket_disable_device(struct gasket_dev *gasket_dev) { const struct gasket_driver_desc *driver_desc = gasket_dev->internal_desc->driver_desc; @@ -649,6 +649,7 @@ static void gasket_disable_dev(struct gasket_dev *gasket_dev) } } } +EXPORT_SYMBOL(gasket_disable_device); /* * Registered descriptor lookup. @@ -1350,13 +1351,12 @@ static const struct file_operations gasket_file_ops = { }; /* Perform final init and marks the device as active. */ -static int gasket_enable_dev(struct gasket_internal_desc *internal_desc, -struct gasket_dev *gasket_dev) +int gasket_enable_device(struct gasket_dev *gasket_dev) { int tbl_idx; int ret; const struct gasket_driver_desc *driver_desc = - internal_desc->driver_desc; + gasket_dev->internal_desc->driver_desc; ret = gasket_interrupt_init(gasket_dev, driver_desc->name, driver_desc->interrupt_type, @@ -1418,13 +1418,15 @@ static int gasket_enable_dev(struct gasket_internal_desc *internal_desc, return 0; } +EXPORT_SYMBOL(gasket_enable_device); /* * Add PCI gasket device. * * Called by Gasket device probe function. - * Allocates device metadata, maps device memory, and calls gasket_enable_dev - * to prepare the device for active use. + * Allocates device metadata and maps device memory. The device driver must + * call gasket_enable_device after driver init is complete to place the device + * in active use. */ int gasket_pci_add_device(struct pci_dev *pci_dev, struct gasket_dev **gasket_devp) @@ -1500,13 +1502,6 @@ int gasket_pci_add_device(struct pci_dev *pci_dev, goto fail5; } - ret = gasket_enable_dev(internal_desc, gasket_dev); - if (ret) { - pr_err("cannot setup %s device\n", driver_desc->name); - gasket_disable_dev(gasket_dev); - goto fail5; - } - *gasket_devp = gasket_dev; return 0; @@ -1560,7 +1555,6 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev) dev_dbg(gasket_dev->dev, "remove %s PCI gasket device\n", internal_desc->driver_desc->name); - gasket_disable_dev(gasket_dev); gasket_cleanup_pci(gasket_dev); check_and_invoke_callback(gasket_dev, driver_desc->sysfs_cleanup_cb); diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h index 5d40bc7f52e91..9c143ebeba452 100644 --- a/drivers/staging/gasket/gasket_core.h +++ b/drivers/staging/gasket/gasket_core.h @@ -590,6 +590,12 @@ int gasket_pci_add_device(struct pci_dev *pci_dev, /* Remove a PCI gasket device. */ void gasket_pci_remove_device(struct pci_dev *pci_dev); +/* Enable a Gasket device. */ +int gasket_enable_device(struct gasket_dev *gasket_dev); + +/* Disable a Gasket device. */ +void gasket_disable_device(struct gasket_dev *gasket_dev); + /* * Reset the Gasket device. * @gasket_dev: Gasket device struct. -- 2.18.0.597.ga71716f1ad-goog
[PATCH 14/15] staging: gasket: apex: place in low power reset until opened
From: Todd Poynor The apex device is left out of reset mode at the end of device probe/initialize processing. Add a call to enter reset at the end of the sequence, triggering power gating and other low power features. Signed-off-by: Todd Poynor --- drivers/staging/gasket/apex_driver.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index 55319619b2e66..c747e9ca45186 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -644,6 +644,10 @@ static int apex_pci_probe(struct pci_dev *pci_dev, goto remove_device; } + /* Place device in low power mode until opened */ + if (allow_power_save) + apex_enter_reset(gasket_dev); + return 0; remove_device: -- 2.18.0.597.ga71716f1ad-goog
[PATCH 13/15] staging: gasket: core: protect against races during unregister
From: Todd Poynor Keep mutex held across the unregistration operation, until the driver_desc field of the global table is removed, to prevent a concurrent accessor from looking up the driver_desc while gasket_unregister_device() is in the processing of removing it. Reported-by: Guenter Roeck Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index ace92f107ed58..a6462b6d702f1 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1765,9 +1765,9 @@ void gasket_unregister_device(const struct gasket_driver_desc *driver_desc) break; } } - mutex_unlock(&g_mutex); if (!internal_desc) { + mutex_unlock(&g_mutex); pr_err("request to unregister unknown desc: %s, %d:%d\n", driver_desc->name, driver_desc->major, driver_desc->minor); @@ -1780,7 +1780,6 @@ void gasket_unregister_device(const struct gasket_driver_desc *driver_desc) class_destroy(internal_desc->class); /* Finally, effectively "remove" the driver. */ - mutex_lock(&g_mutex); g_descs[desc_idx].driver_desc = NULL; mutex_unlock(&g_mutex); -- 2.18.0.597.ga71716f1ad-goog
[PATCH 01/15] staging: gasket: sysfs: clean up state if ENOMEM removing mapping
From: Todd Poynor If kcalloc() returns NULL in put_mapping(), continue to clean up state, including dropping the reference on the struct device and free attribute memory. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_sysfs.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/staging/gasket/gasket_sysfs.c b/drivers/staging/gasket/gasket_sysfs.c index 56d62aea51118..fc45f0d13e87d 100644 --- a/drivers/staging/gasket/gasket_sysfs.c +++ b/drivers/staging/gasket/gasket_sysfs.c @@ -101,13 +101,12 @@ static void put_mapping(struct gasket_sysfs_mapping *mapping) files_to_remove = kcalloc(num_files_to_remove, sizeof(*files_to_remove), GFP_KERNEL); - if (!files_to_remove) { - mutex_unlock(&mapping->mutex); - return; - } - - for (i = 0; i < num_files_to_remove; i++) - files_to_remove[i] = mapping->attributes[i].attr; + if (files_to_remove) + for (i = 0; i < num_files_to_remove; i++) + files_to_remove[i] = + mapping->attributes[i].attr; + else + num_files_to_remove = 0; kfree(mapping->attributes); mapping->attributes = NULL; -- 2.18.0.597.ga71716f1ad-goog
[PATCH 15/15] staging: gasket: core: remove incorrect extraneous comment
From: Todd Poynor A copy-and-pasted comment from another code sequence is removed from gasket core init sequence. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index a6462b6d702f1..d12ab560411f7 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1792,7 +1792,6 @@ static int __init gasket_init(void) int i; pr_debug("%s\n", __func__); - /* Check for duplicates and find a free slot. */ mutex_lock(&g_mutex); for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) { g_descs[i].driver_desc = NULL; -- 2.18.0.597.ga71716f1ad-goog
[PATCH 03/15] staging: gasket: apex: move PCI core calls to apex driver
From: Todd Poynor Apex driver moves PCI core calls like probe, enable, and remove from gasket to apex. Call new functions in gasket to register apex as a PCI device to the gasket framework. Signed-off-by: Todd Poynor --- drivers/staging/gasket/apex_driver.c | 49 +++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index 42cef68eb4c19..b47661442009d 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -621,6 +622,36 @@ static void apex_pci_fixup_class(struct pci_dev *pdev) DECLARE_PCI_FIXUP_CLASS_HEADER(APEX_PCI_VENDOR_ID, APEX_PCI_DEVICE_ID, PCI_CLASS_NOT_DEFINED, 8, apex_pci_fixup_class); +static int apex_pci_probe(struct pci_dev *pci_dev, + const struct pci_device_id *id) +{ + int ret; + struct gasket_dev *gasket_dev; + + ret = pci_enable_device(pci_dev); + if (ret) { + dev_err(&pci_dev->dev, "error enabling PCI device\n"); + return ret; + } + + pci_set_master(pci_dev); + + ret = gasket_pci_add_device(pci_dev, &gasket_dev); + if (ret) { + dev_err(&pci_dev->dev, "error adding gasket device\n"); + pci_disable_device(pci_dev); + return ret; + } + + return 0; +} + +static void apex_pci_remove(struct pci_dev *pci_dev) +{ + gasket_pci_remove_device(pci_dev); + pci_disable_device(pci_dev); +} + static struct gasket_driver_desc apex_desc = { .name = "apex", .driver_version = APEX_DRIVER_VERSION, @@ -672,13 +703,29 @@ static struct gasket_driver_desc apex_desc = { .device_reset_cb = apex_reset, }; +static struct pci_driver apex_pci_driver = { + .name = "apex", + .probe = apex_pci_probe, + .remove = apex_pci_remove, + .id_table = apex_pci_ids, +}; + static int __init apex_init(void) { - return gasket_register_device(&apex_desc); + int ret; + + ret = gasket_register_device(&apex_desc); + if (ret) + return ret; + ret = pci_register_driver(&apex_pci_driver); + if (ret) + gasket_unregister_device(&apex_desc); + return ret; } static void apex_exit(void) { + pci_unregister_driver(&apex_pci_driver); gasket_unregister_device(&apex_desc); } MODULE_DESCRIPTION("Google Apex driver"); -- 2.18.0.597.ga71716f1ad-goog
[PATCH 04/15] staging: gasket: core: convert remaining info logs to debug
From: Todd Poynor Remaining info-level logs in gasket core converted to debug-level; the information is not needed during normal system operation. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 01cafe1ff6605..2741256eacfe8 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1819,7 +1819,7 @@ void gasket_unregister_device(const struct gasket_driver_desc *driver_desc) g_descs[desc_idx].driver_desc = NULL; mutex_unlock(&g_mutex); - pr_info("removed %s driver\n", driver_desc->name); + pr_debug("removed %s driver\n", driver_desc->name); } EXPORT_SYMBOL(gasket_unregister_device); @@ -1827,7 +1827,7 @@ static int __init gasket_init(void) { int i; - pr_info("Performing one-time init of the Gasket framework.\n"); + pr_debug("%s\n", __func__); /* Check for duplicates and find a free slot. */ mutex_lock(&g_mutex); for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) { @@ -1843,8 +1843,7 @@ static int __init gasket_init(void) static void __exit gasket_exit(void) { - /* No deinit/dealloc needed at present. */ - pr_info("Removing Gasket framework module.\n"); + pr_debug("%s\n", __func__); } MODULE_DESCRIPTION("Google Gasket driver framework"); MODULE_VERSION(GASKET_FRAMEWORK_VERSION); -- 2.18.0.597.ga71716f1ad-goog
[PATCH 09/15] staging: gasket: core: delete device add and remove callbacks
From: Todd Poynor Gasket device drivers are now in charge of orchestrating the device add and removal sequences, so the callbacks from the framework to the device drivers for these events are no longer needed. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 10 -- drivers/staging/gasket/gasket_core.h | 29 2 files changed, 39 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index fad4883e6332c..0d76e18fcde5b 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1468,12 +1468,6 @@ int gasket_pci_add_device(struct pci_dev *pci_dev, if (ret) goto fail2; - ret = check_and_invoke_callback(gasket_dev, driver_desc->add_dev_cb); - if (ret) { - dev_err(gasket_dev->dev, "Error in add device cb: %d\n", ret); - goto fail2; - } - ret = gasket_sysfs_create_mapping(gasket_dev->dev_info.device, gasket_dev); if (ret) @@ -1512,7 +1506,6 @@ int gasket_pci_add_device(struct pci_dev *pci_dev, gasket_sysfs_remove_mapping(gasket_dev->dev_info.device); fail2: gasket_cleanup_pci(gasket_dev); - check_and_invoke_callback(gasket_dev, driver_desc->remove_dev_cb); device_destroy(internal_desc->class, gasket_dev->dev_info.devt); fail1: gasket_free_dev(gasket_dev); @@ -1559,9 +1552,6 @@ void gasket_pci_remove_device(struct pci_dev *pci_dev) check_and_invoke_callback(gasket_dev, driver_desc->sysfs_cleanup_cb); gasket_sysfs_remove_mapping(gasket_dev->dev_info.device); - - check_and_invoke_callback(gasket_dev, driver_desc->remove_dev_cb); - device_destroy(internal_desc->class, gasket_dev->dev_info.devt); gasket_free_dev(gasket_dev); } diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h index 9c143ebeba452..0ef0a2640f0fe 100644 --- a/drivers/staging/gasket/gasket_core.h +++ b/drivers/staging/gasket/gasket_core.h @@ -302,12 +302,6 @@ struct gasket_dev { /* Hardware revision value for this device. */ int hardware_revision; - /* -* Device-specific data; allocated in gasket_driver_desc.add_dev_cb() -* and freed in gasket_driver_desc.remove_dev_cb(). -*/ - void *cb_data; - /* Protects access to per-device data (i.e. this structure). */ struct mutex mutex; @@ -415,29 +409,6 @@ struct gasket_driver_desc { int interrupt_pack_width; /* Driver callback functions - all may be NULL */ - /* -* add_dev_cb: Callback when a device is found. -* @dev: The gasket_dev struct for this driver instance. -* -* This callback should initialize the device-specific cb_data. -* Called when a device is found by the driver, -* before any BAR ranges have been mapped. If this call fails (returns -* nonzero), remove_dev_cb will be called. -* -*/ - int (*add_dev_cb)(struct gasket_dev *dev); - - /* -* remove_dev_cb: Callback for when a device is removed from the system. -* @dev: The gasket_dev struct for this driver instance. -* -* This callback should free data allocated in add_dev_cb. -* Called immediately before a device is unregistered by the driver. -* All framework-managed resources will have been cleaned up by the time -* this callback is invoked (PCI BARs, character devices, ...). -*/ - int (*remove_dev_cb)(struct gasket_dev *dev); - /* * device_open_cb: Callback for when a device node is opened in write * mode. -- 2.18.0.597.ga71716f1ad-goog
[PATCH 00/15] staging: gasket: unwrap pci core and more
From: Todd Poynor Stop wrapping PCI core calls like probe, enable, remove, etc. in the gasket framework, move these calls to the device driver instead. Have gasket drivers call into framework on init, enable, disable, etc. sequences, rather than the other way around. Remove the gasket-to-device callbacks associated with these sequences. Plus a few other fixes and cleanups. Todd Poynor (15): staging: gasket: sysfs: clean up state if ENOMEM removing mapping staging: gasket: core: move core PCI calls to device drivers staging: gasket: apex: move PCI core calls to apex driver staging: gasket: core: convert remaining info logs to debug staging: gasket: core: remove device enable and disable callbacks staging: gasket: apex: remove device enable and disable callbacks staging: gasket: core: let device driver enable/disable gasket device staging: gasket: apex: enable/disable gasket device from apex staging: gasket: core: delete device add and remove callbacks staging: gasket: apex: fold device add/remove logic inline staging: gasket: core: remove sysfs setup and cleanup callbacks staging: gasket: apex: move sysfs setup code to probe function staging: gasket: core: protect against races during unregister staging: gasket: apex: place in low power reset until opened staging: gasket: core: remove incorrect extraneous comment drivers/staging/gasket/apex_driver.c | 145 +- drivers/staging/gasket/gasket_core.c | 140 ++--- drivers/staging/gasket/gasket_core.h | 82 +++ drivers/staging/gasket/gasket_sysfs.c | 13 ++- 4 files changed, 148 insertions(+), 232 deletions(-) -- 2.18.0.597.ga71716f1ad-goog
[PATCH 05/15] staging: gasket: core: remove device enable and disable callbacks
From: Todd Poynor Device enable/disable operations are moving from being initiated through the gasket framework to being initiated by the gasket device driver. The driver can perform any processing needed for these operations before or after the calls into the framework. Neither of these callbacks are implemented for the only gasket driver upstream today, apex. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 9 - drivers/staging/gasket/gasket_core.h | 27 ++- 2 files changed, 2 insertions(+), 34 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 2741256eacfe8..b070efaf0d41c 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -648,8 +648,6 @@ static void gasket_disable_dev(struct gasket_dev *gasket_dev) gasket_page_table_cleanup(gasket_dev->page_table[i]); } } - - check_and_invoke_callback(gasket_dev, driver_desc->disable_dev_cb); } /* @@ -1408,13 +1406,6 @@ static int gasket_enable_dev(struct gasket_internal_desc *internal_desc, } gasket_dev->hardware_revision = ret; - ret = check_and_invoke_callback(gasket_dev, driver_desc->enable_dev_cb); - if (ret) { - dev_err(gasket_dev->dev, "Error in enable device cb: %d\n", - ret); - return ret; - } - /* device_status_cb returns a device status, not an error code. */ gasket_dev->status = gasket_get_hw_status(gasket_dev); if (gasket_dev->status == GASKET_STATUS_DEAD) diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h index 9f9bc66a0daa0..5d40bc7f52e91 100644 --- a/drivers/staging/gasket/gasket_core.h +++ b/drivers/staging/gasket/gasket_core.h @@ -473,34 +473,11 @@ struct gasket_driver_desc { */ int (*device_close_cb)(struct gasket_dev *dev); - /* -* enable_dev_cb: Callback immediately before enabling the device. -* @dev: Pointer to the gasket_dev struct for this driver instance. -* -* This callback is invoked after the device has been added and all BAR -* spaces mapped, immediately before registering and enabling the -* [character] device via cdev_add. If this call fails (returns -* nonzero), disable_dev_cb will be called. -* -* Note that cdev are initialized but not active -* (cdev_add has not yet been called) when this callback is invoked. -*/ - int (*enable_dev_cb)(struct gasket_dev *dev); - - /* -* disable_dev_cb: Callback immediately after disabling the device. -* @dev: Pointer to the gasket_dev struct for this driver instance. -* -* Called during device shutdown, immediately after disabling device -* operations via cdev_del. -*/ - int (*disable_dev_cb)(struct gasket_dev *dev); - /* * sysfs_setup_cb: Callback to set up driver-specific sysfs nodes. * @dev: Pointer to the gasket_dev struct for this device. * -* Called just before enable_dev_cb. +* Called during the add gasket device call. * */ int (*sysfs_setup_cb)(struct gasket_dev *dev); @@ -509,7 +486,7 @@ struct gasket_driver_desc { * sysfs_cleanup_cb: Callback to clean up driver-specific sysfs nodes. * @dev: Pointer to the gasket_dev struct for this device. * -* Called just before disable_dev_cb. +* Called during device disable processing. * */ int (*sysfs_cleanup_cb)(struct gasket_dev *dev); -- 2.18.0.597.ga71716f1ad-goog
[PATCH 02/15] staging: gasket: core: move core PCI calls to device drivers
From: Todd Poynor Remove gasket wrapping of PCI probe, enable, disable, and remove functions. Replace with calls to add and remove PCI gasket devices, to be called by the gasket device drivers. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 82 drivers/staging/gasket/gasket_core.h | 6 ++ 2 files changed, 28 insertions(+), 60 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 2d209e36cf372..01cafe1ff6605 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -51,9 +51,6 @@ struct gasket_internal_desc { /* Kernel-internal device class. */ struct class *class; - /* PCI subsystem metadata associated with this driver. */ - struct pci_driver pci; - /* Instantiated / present devices of this type. */ struct gasket_dev *devs[GASKET_DEV_MAX]; }; @@ -368,10 +365,10 @@ static void gasket_unmap_pci_bar(struct gasket_dev *dev, int bar_num) } /* - * Setup PCI & set up memory mapping for the specified device. + * Setup PCI memory mapping for the specified device. * - * Enables the PCI device, reads the BAR registers and sets up pointers to the - * device's memory mapped IO space. + * Reads the BAR registers and sets up pointers to the device's memory mapped + * IO space. * * Returns 0 on success and a negative value otherwise. */ @@ -380,14 +377,6 @@ static int gasket_setup_pci(struct pci_dev *pci_dev, { int i, mapped_bars, ret; - ret = pci_enable_device(pci_dev); - if (ret) { - dev_err(gasket_dev->dev, "cannot enable PCI device\n"); - return ret; - } - - pci_set_master(pci_dev); - for (i = 0; i < GASKET_NUM_BARS; i++) { ret = gasket_map_pci_bar(gasket_dev, i); if (ret) { @@ -402,19 +391,16 @@ static int gasket_setup_pci(struct pci_dev *pci_dev, for (i = 0; i < mapped_bars; i++) gasket_unmap_pci_bar(gasket_dev, i); - pci_disable_device(pci_dev); return -ENOMEM; } -/* Unmaps memory and cleans up PCI for the specified device. */ +/* Unmaps memory for the specified device. */ static void gasket_cleanup_pci(struct gasket_dev *gasket_dev) { int i; for (i = 0; i < GASKET_NUM_BARS; i++) gasket_unmap_pci_bar(gasket_dev, i); - - pci_disable_device(gasket_dev->pci_dev); } /* Determine the health of the Gasket device. */ @@ -1443,15 +1429,14 @@ static int gasket_enable_dev(struct gasket_internal_desc *internal_desc, } /* - * PCI subsystem probe function. - * - * Called when a Gasket device is found. Allocates device metadata, maps device - * memory, and calls gasket_enable_dev to prepare the device for active use. + * Add PCI gasket device. * - * Returns 0 if successful and a negative value otherwise. + * Called by Gasket device probe function. + * Allocates device metadata, maps device memory, and calls gasket_enable_dev + * to prepare the device for active use. */ -static int gasket_pci_probe(struct pci_dev *pci_dev, - const struct pci_device_id *id) +int gasket_pci_add_device(struct pci_dev *pci_dev, + struct gasket_dev **gasket_devp) { int ret; const char *kobj_name = dev_name(&pci_dev->dev); @@ -1460,13 +1445,14 @@ static int gasket_pci_probe(struct pci_dev *pci_dev, const struct gasket_driver_desc *driver_desc; struct device *parent; - pr_info("Add Gasket device %s\n", kobj_name); + pr_debug("add PCI device %s\n", kobj_name); mutex_lock(&g_mutex); internal_desc = lookup_internal_desc(pci_dev); mutex_unlock(&g_mutex); if (!internal_desc) { - pr_err("PCI probe called for unknown driver type\n"); + dev_err(&pci_dev->dev, + "PCI add device called for unknown driver type\n"); return -ENODEV; } @@ -1530,6 +1516,7 @@ static int gasket_pci_probe(struct pci_dev *pci_dev, goto fail5; } + *gasket_devp = gasket_dev; return 0; fail5: @@ -1545,14 +1532,10 @@ static int gasket_pci_probe(struct pci_dev *pci_dev, gasket_free_dev(gasket_dev); return ret; } +EXPORT_SYMBOL(gasket_pci_add_device); -/* - * PCI subsystem remove function. - * - * Called to remove a Gasket device. Finds the device in the device list and - * cleans up metadata. - */ -static void gasket_pci_remove(struct pci_dev *pci_dev) +/* Remove a PCI gasket device. */ +void gasket_pci_remove_device(struct pci_dev *pci_dev) { int i; struct gasket_internal_desc *internal_desc; @@ -1583,8 +1566,8 @@ static void gasket_pci_remove(struct pci_dev *pci_dev) if (!gasket_dev
[PATCH 0/4 v2] staging: gasket: cleanups du jour
From: Todd Poynor More cleanups for the gasket+apex drivers. Patched changed in v2 from v1: staging: gasket: core: print driver version code at registration time staging: gasket: core: move driver loaded log after error cases Above 2 patches replaced by new patch: staging: gasket: core: remove registration logs staging: gasket: core: device register debug log cleanups Drop explicit "gasket:" prefix, use pr_fmt instead (in following patch) staging: gasket: core: add subsystem and device info to error logs Renamed: staging: gasket: core: add subsystem and device info to logs Use pr_fmt for modname prefix on logs Revert "staging: gasket: core: hold reference to pci_dev while used" Fixup SHA format in commit text. Patches dropped from v2, already merged from v1: staging: gasket: apex: enable power save mode by default staging: gasket: remove "reset type" param from framework staging: gasket: apex: drop reset type param Todd Poynor (4): staging: gasket: core: remove registration logs staging: gasket: core: device register debug log cleanups staging: gasket: core: add subsystem and device info to logs Revert "staging: gasket: core: hold reference to pci_dev while used" drivers/staging/gasket/gasket_core.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) -- 2.18.0.597.ga71716f1ad-goog
[PATCH 1/4] staging: gasket: core: remove registration logs
From: Todd Poynor Remove logs for loading gasket drivers. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 2b75f100da4d3..fa477d0c3c74c 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1758,9 +1758,6 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) } mutex_unlock(&g_mutex); - pr_info("Loaded %s driver, framework version %s\n", - driver_desc->name, GASKET_FRAMEWORK_VERSION); - if (desc_idx == -1) { pr_err("Too many Gasket drivers loaded: %d\n", GASKET_FRAMEWORK_DESC_MAX); @@ -1808,7 +1805,6 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) goto fail2; } - pr_info("Driver registered successfully.\n"); return 0; fail2: -- 2.18.0.597.ga71716f1ad-goog
[PATCH 4/4] Revert "staging: gasket: core: hold reference to pci_dev while used"
From: Todd Poynor There's no need to take an additional reference on the pci_dev structure for the pointer copy saved in gasket data structures. This reverts commit: 8dd8a48b9a7d ("staging: gasket: core: hold reference to pci_dev while used") Reported-by: Dmitry Torokhov Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 93a4d9f08eaab..2d209e36cf372 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -255,7 +255,6 @@ static void gasket_free_dev(struct gasket_dev *gasket_dev) internal_desc->devs[gasket_dev->dev_idx] = NULL; mutex_unlock(&internal_desc->mutex); put_device(gasket_dev->dev); - pci_dev_put(gasket_dev->pci_dev); kfree(gasket_dev); } @@ -1477,7 +1476,7 @@ static int gasket_pci_probe(struct pci_dev *pci_dev, ret = gasket_alloc_dev(internal_desc, parent, &gasket_dev, kobj_name); if (ret) return ret; - gasket_dev->pci_dev = pci_dev_get(pci_dev); + gasket_dev->pci_dev = pci_dev; if (IS_ERR_OR_NULL(gasket_dev->dev_info.device)) { pr_err("Cannot create %s device %s [ret = %ld]\n", driver_desc->name, gasket_dev->dev_info.name, -- 2.18.0.597.ga71716f1ad-goog
[PATCH 3/4] staging: gasket: core: add subsystem and device info to logs
From: Todd Poynor Identify gasket as the subsystem printing various messages. Add the driver name to appropriate messages to indicate which driver has a problem. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 91db71c238804..93a4d9f08eaab 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -6,6 +6,9 @@ * * Copyright (C) 2018 Google, Inc. */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include "gasket_core.h" #include "gasket_interrupt.h" @@ -208,7 +211,7 @@ static int gasket_alloc_dev(struct gasket_internal_desc *internal_desc, gasket_dev = *pdev = kzalloc(sizeof(*gasket_dev), GFP_KERNEL); if (!gasket_dev) { - pr_err("no memory for device\n"); + pr_err("no memory for device %s\n", kobj_name); return -ENOMEM; } internal_desc->devs[dev_idx] = gasket_dev; @@ -1760,7 +1763,7 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) mutex_unlock(&g_mutex); if (desc_idx == -1) { - pr_err("Too many Gasket drivers loaded: %d\n", + pr_err("too many drivers loaded, max %d\n", GASKET_FRAMEWORK_DESC_MAX); return -EBUSY; } @@ -1790,7 +1793,8 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) ret = __pci_register_driver(&internal->pci, driver_desc->module, driver_desc->name); if (ret) { - pr_err("cannot register pci driver [ret=%d]\n", ret); + pr_err("cannot register %s pci driver [ret=%d]\n", + driver_desc->name, ret); goto fail1; } @@ -1798,7 +1802,8 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) driver_desc->minor), GASKET_DEV_MAX, driver_desc->name); if (ret) { - pr_err("cannot register char driver [ret=%d]\n", ret); + pr_err("cannot register %s char driver [ret=%d]\n", + driver_desc->name, ret); goto fail2; } -- 2.18.0.597.ga71716f1ad-goog
[PATCH 2/4] staging: gasket: core: device register debug log cleanups
From: Todd Poynor At device/driver registration time, convert a not-very-informative info message to a more informative debug message, drop some not overly helpful debug messages. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index fa477d0c3c74c..91db71c238804 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1735,7 +1735,8 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) int desc_idx = -1; struct gasket_internal_desc *internal; - pr_info("Initializing Gasket framework device\n"); + pr_debug("Loading %s driver version %s\n", driver_desc->name, +driver_desc->driver_version); /* Check for duplicates and find a free slot. */ mutex_lock(&g_mutex); @@ -1764,8 +1765,6 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) return -EBUSY; } - /* Internal structure setup. */ - pr_debug("Performing initial internal structure setup.\n"); internal = &g_descs[desc_idx]; mutex_init(&internal->mutex); memset(internal->devs, 0, sizeof(struct gasket_dev *) * GASKET_DEV_MAX); @@ -1788,7 +1787,6 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) * Not using pci_register_driver() (without underscores), as it * depends on KBUILD_MODNAME, and this is a shared file. */ - pr_debug("Registering PCI driver.\n"); ret = __pci_register_driver(&internal->pci, driver_desc->module, driver_desc->name); if (ret) { @@ -1796,7 +1794,6 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) goto fail1; } - pr_debug("Registering char driver.\n"); ret = register_chrdev_region(MKDEV(driver_desc->major, driver_desc->minor), GASKET_DEV_MAX, driver_desc->name); -- 2.18.0.597.ga71716f1ad-goog
[PATCH 4/8] staging: gasket: core: device register debug log cleanups
From: Todd Poynor At device/driver registration time, convert a not-very-informative info message to a more informative dbeug message, drop some not overly helpful debug messages. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 2160c2de78e77..36c077fffc41c 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1736,7 +1736,8 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) int desc_idx = -1; struct gasket_internal_desc *internal; - pr_info("Initializing Gasket framework device\n"); + pr_debug("gasket: Loading %s driver version %s\n", driver_desc->name, +driver_desc->driver_version); /* Check for duplicates and find a free slot. */ mutex_lock(&g_mutex); @@ -1765,8 +1766,6 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) return -EBUSY; } - /* Internal structure setup. */ - pr_debug("Performing initial internal structure setup.\n"); internal = &g_descs[desc_idx]; mutex_init(&internal->mutex); memset(internal->devs, 0, sizeof(struct gasket_dev *) * GASKET_DEV_MAX); @@ -1789,7 +1788,6 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) * Not using pci_register_driver() (without underscores), as it * depends on KBUILD_MODNAME, and this is a shared file. */ - pr_debug("Registering PCI driver.\n"); ret = __pci_register_driver(&internal->pci, driver_desc->module, driver_desc->name); if (ret) { @@ -1797,7 +1795,6 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) goto fail1; } - pr_debug("Registering char driver.\n"); ret = register_chrdev_region(MKDEV(driver_desc->major, driver_desc->minor), GASKET_DEV_MAX, driver_desc->name); -- 2.18.0.597.ga71716f1ad-goog
[PATCH 7/8] staging: gasket: apex: drop reset type param
From: Todd Poynor Apex doesn't implement different types of resets based on the reset type param passed through the gasket layer or from userspace via the gasket_reset ioctl. The reset type is dropped from the gasket framework in a previous patch due to a lack of present need and non-conforming use of this parameter by the framework. Drop the parameter from the apex driver as well. Signed-off-by: Todd Poynor --- drivers/staging/gasket/apex_driver.c | 20 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index 7fd0dd609d037..42cef68eb4c19 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -56,10 +56,6 @@ #define APEX_EXTENDED_SHIFT 63 /* Extended address bit position. */ -enum apex_reset_types { - APEX_CHIP_REINIT_RESET = 3, -}; - /* Check reset 120 times */ #define APEX_RESET_RETRY 120 /* Wait 100 ms between checks. Total 12 sec wait maximum. */ @@ -258,7 +254,7 @@ static int apex_get_status(struct gasket_dev *gasket_dev) } /* Enter GCB reset state. */ -static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type) +static int apex_enter_reset(struct gasket_dev *gasket_dev) { if (bypass_top_level) return 0; @@ -313,7 +309,7 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type) } /* Quit GCB reset state. */ -static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type) +static int apex_quit_reset(struct gasket_dev *gasket_dev) { u32 val0, val1; @@ -413,7 +409,7 @@ static int apex_device_cleanup(struct gasket_dev *gasket_dev) __func__, gasket_dev, hib_error, scalar_error); if (allow_power_save) - ret = apex_enter_reset(gasket_dev, APEX_CHIP_REINIT_RESET); + ret = apex_enter_reset(gasket_dev); return ret; } @@ -429,7 +425,7 @@ static bool is_gcb_in_reset(struct gasket_dev *gasket_dev) } /* Reset the hardware, then quit reset. Called on device open. */ -static int apex_reset(struct gasket_dev *gasket_dev, uint type) +static int apex_reset(struct gasket_dev *gasket_dev) { int ret; @@ -442,11 +438,11 @@ static int apex_reset(struct gasket_dev *gasket_dev, uint type) */ dev_dbg(gasket_dev->dev, "%s: toggle reset\n", __func__); - ret = apex_enter_reset(gasket_dev, type); + ret = apex_enter_reset(gasket_dev); if (ret) return ret; } - ret = apex_quit_reset(gasket_dev, type); + ret = apex_quit_reset(gasket_dev); return ret; } @@ -456,7 +452,7 @@ static int apex_add_dev_cb(struct gasket_dev *gasket_dev) ulong page_table_ready, msix_table_ready; int retries = 0; - apex_reset(gasket_dev, 0); + apex_reset(gasket_dev); while (retries < APEX_RESET_RETRY) { page_table_ready = @@ -611,7 +607,7 @@ static int apex_sysfs_setup_cb(struct gasket_dev *gasket_dev) /* On device open, perform a core reinit reset. */ static int apex_device_open_cb(struct gasket_dev *gasket_dev) { - return gasket_reset_nolock(gasket_dev, APEX_CHIP_REINIT_RESET); + return gasket_reset_nolock(gasket_dev); } static const struct pci_device_id apex_pci_ids[] = { -- 2.18.0.597.ga71716f1ad-goog
[PATCH 3/8] staging: gasket: core: move driver loaded log after error cases
From: Todd Poynor Gasket could claim to have loaded a driver and then print an error indicating it actually did not. Move the driver registration message after the last error check. Replace the existing "loaded successfully" message with this instead. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index e550c9060dcd2..2160c2de78e77 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1759,10 +1759,6 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) } mutex_unlock(&g_mutex); - pr_info("gasket: Loaded %s driver version %s, framework version %s\n", - driver_desc->name, driver_desc->driver_version, - GASKET_FRAMEWORK_VERSION); - if (desc_idx == -1) { pr_err("Too many Gasket drivers loaded: %d\n", GASKET_FRAMEWORK_DESC_MAX); @@ -1810,7 +1806,10 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) goto fail2; } - pr_info("Driver registered successfully.\n"); + pr_info("gasket: Loaded %s driver version %s, framework version %s\n", + driver_desc->name, driver_desc->driver_version, + GASKET_FRAMEWORK_VERSION); + return 0; fail2: -- 2.18.0.597.ga71716f1ad-goog
[PATCH 1/8] staging: gasket: apex: enable power save mode by default
From: Todd Poynor Set default value of allow_power_save parameter to enable power save mode, which is expected to be the state usually desired. Signed-off-by: Marty Faltesek Signed-off-by: Todd Poynor --- drivers/staging/gasket/apex_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index dfbff47b46086..7fd0dd609d037 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -233,7 +233,7 @@ static struct gasket_interrupt_desc apex_interrupts[] = { /* Allows device to enter power save upon driver close(). */ -static int allow_power_save; +static int allow_power_save = 1; /* Allows SW based clock gating. */ static int allow_sw_clock_gating; -- 2.18.0.597.ga71716f1ad-goog
[PATCH 0/8] staging: gasket: cleanups du jour
From: Todd Poynor More cleanups for the gasket+apex drivers. Todd Poynor (8): staging: gasket: apex: enable power save mode by default staging: gasket: core: print driver version code at registration time staging: gasket: core: move driver loaded log after error cases staging: gasket: core: device register debug log cleanups staging: gasket: core: add subsystem and device info to error logs staging: gasket: remove "reset type" param from framework staging: gasket: apex: drop reset type param Revert "staging: gasket: core: hold reference to pci_dev while used" drivers/staging/gasket/apex_driver.c | 22 - drivers/staging/gasket/gasket.h | 4 +- drivers/staging/gasket/gasket_constants.h | 2 +- drivers/staging/gasket/gasket_core.c | 56 +++ drivers/staging/gasket/gasket_core.h | 13 ++ drivers/staging/gasket/gasket_ioctl.c | 3 +- 6 files changed, 43 insertions(+), 57 deletions(-) -- 2.18.0.597.ga71716f1ad-goog
[PATCH 8/8] Revert "staging: gasket: core: hold reference to pci_dev while used"
From: Todd Poynor There's no need to take an additional reference on the pci_dev structure for the pointer copy saved in gasket data structures. This reverts commit 8dd8a48b9a7dae5493494a8603adddfdf1914716. Reported-by: Dmitry Torokhov Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 4e30b154b..8c5574305f13b 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -253,7 +253,6 @@ static void gasket_free_dev(struct gasket_dev *gasket_dev) internal_desc->devs[gasket_dev->dev_idx] = NULL; mutex_unlock(&internal_desc->mutex); put_device(gasket_dev->dev); - pci_dev_put(gasket_dev->pci_dev); kfree(gasket_dev); } @@ -1475,7 +1474,7 @@ static int gasket_pci_probe(struct pci_dev *pci_dev, ret = gasket_alloc_dev(internal_desc, parent, &gasket_dev, kobj_name); if (ret) return ret; - gasket_dev->pci_dev = pci_dev_get(pci_dev); + gasket_dev->pci_dev = pci_dev; if (IS_ERR_OR_NULL(gasket_dev->dev_info.device)) { pr_err("gasket: Cannot create %s device %s [ret = %ld]\n", driver_desc->name, gasket_dev->dev_info.name, -- 2.18.0.597.ga71716f1ad-goog
[PATCH 6/8] staging: gasket: remove "reset type" param from framework
From: Todd Poynor The "type of reset" parameter to the gasket device reset APIs isn't required by the only gasket device submitted upstream, apex. The framework documents the param as private to the device driver and a pass-through at the gasket layer, but the gasket core calls the device driver with a hardcoded reset type of zero, which is not documented as having a predefined meaning. In light of all this, remove the reset type parameter from the framework. Remove the reset ioctl reset type parameter, and bump the framework version number to reflect the interface change. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket.h | 4 ++-- drivers/staging/gasket/gasket_constants.h | 2 +- drivers/staging/gasket/gasket_core.c | 11 +-- drivers/staging/gasket/gasket_core.h | 13 +++-- drivers/staging/gasket/gasket_ioctl.c | 3 +-- 5 files changed, 12 insertions(+), 21 deletions(-) diff --git a/drivers/staging/gasket/gasket.h b/drivers/staging/gasket/gasket.h index 9f709f0c5a2bb..a0f065c517a52 100644 --- a/drivers/staging/gasket/gasket.h +++ b/drivers/staging/gasket/gasket.h @@ -52,8 +52,8 @@ struct gasket_coherent_alloc_config_ioctl { /* Base number for all Gasket-common IOCTLs */ #define GASKET_IOCTL_BASE 0xDC -/* Reset the device using the specified reset type. */ -#define GASKET_IOCTL_RESET _IOW(GASKET_IOCTL_BASE, 0, unsigned long) +/* Reset the device. */ +#define GASKET_IOCTL_RESET _IO(GASKET_IOCTL_BASE, 0) /* Associate the specified [event]fd with the specified interrupt. */ #define GASKET_IOCTL_SET_EVENTFD \ diff --git a/drivers/staging/gasket/gasket_constants.h b/drivers/staging/gasket/gasket_constants.h index 82ed3f21e8aed..50d87c7b178c2 100644 --- a/drivers/staging/gasket/gasket_constants.h +++ b/drivers/staging/gasket/gasket_constants.h @@ -3,7 +3,7 @@ #ifndef __GASKET_CONSTANTS_H__ #define __GASKET_CONSTANTS_H__ -#define GASKET_FRAMEWORK_VERSION "1.1.1" +#define GASKET_FRAMEWORK_VERSION "1.1.2" /* * The maximum number of simultaneous device types supported by the framework. diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 19331feb9b09f..4e30b154b 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1295,7 +1295,7 @@ static int gasket_release(struct inode *inode, struct file *file) ownership->owner = 0; /* Forces chip reset before we unmap the page tables. */ - driver_desc->device_reset_cb(gasket_dev, 0); + driver_desc->device_reset_cb(gasket_dev); for (i = 0; i < driver_desc->num_page_tables; ++i) { gasket_page_table_unmap_all(gasket_dev->page_table[i]); @@ -1623,18 +1623,18 @@ const char *gasket_num_name_lookup(uint num, } EXPORT_SYMBOL(gasket_num_name_lookup); -int gasket_reset(struct gasket_dev *gasket_dev, uint reset_type) +int gasket_reset(struct gasket_dev *gasket_dev) { int ret; mutex_lock(&gasket_dev->mutex); - ret = gasket_reset_nolock(gasket_dev, reset_type); + ret = gasket_reset_nolock(gasket_dev); mutex_unlock(&gasket_dev->mutex); return ret; } EXPORT_SYMBOL(gasket_reset); -int gasket_reset_nolock(struct gasket_dev *gasket_dev, uint reset_type) +int gasket_reset_nolock(struct gasket_dev *gasket_dev) { int ret; int i; @@ -1644,8 +1644,7 @@ int gasket_reset_nolock(struct gasket_dev *gasket_dev, uint reset_type) if (!driver_desc->device_reset_cb) return 0; - /* Perform a device reset of the requested type. */ - ret = driver_desc->device_reset_cb(gasket_dev, reset_type); + ret = driver_desc->device_reset_cb(gasket_dev); if (ret) { dev_dbg(gasket_dev->dev, "Device reset cb returned %d.\n", ret); diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h index 713bf42de41a4..67f5960943a8a 100644 --- a/drivers/staging/gasket/gasket_core.h +++ b/drivers/staging/gasket/gasket_core.h @@ -580,17 +580,12 @@ struct gasket_driver_desc { /* * device_reset_cb: Reset the hardware in question. * @dev: Pointer to the gasket_dev structure for this device. -* @type: Integer representing reset type. (All -* Gasket resets have an integer representing their type -* defined in (device)_ioctl.h; the specific resets are -* device-dependent, but are handled in the device-specific -* callback anyways.) * * Called by reset ioctls. This function should not * lock the gasket_dev mutex. It should return 0 on success * and an error on failure. */ - int (
[PATCH 5/8] staging: gasket: core: add subsystem and device info to error logs
From: Todd Poynor Identify gasket as the subsystem printing various error messages. Add the driver name to appropriate messages to indicate which driver has a problem. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 27 +++ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 36c077fffc41c..19331feb9b09f 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -160,7 +160,7 @@ static int gasket_find_dev_slot(struct gasket_internal_desc *internal_desc, for (i = 0; i < GASKET_DEV_MAX; i++) { if (internal_desc->devs[i] && strcmp(internal_desc->devs[i]->kobj_name, kobj_name) == 0) { - pr_err("Duplicate device %s\n", kobj_name); + pr_err("gasket: Duplicate device %s\n", kobj_name); mutex_unlock(&internal_desc->mutex); return -EBUSY; } @@ -173,7 +173,8 @@ static int gasket_find_dev_slot(struct gasket_internal_desc *internal_desc, } if (i == GASKET_DEV_MAX) { - pr_err("Too many registered devices; max %d\n", GASKET_DEV_MAX); + pr_err("gasket: Too many registered devices, max %d\n", + GASKET_DEV_MAX); mutex_unlock(&internal_desc->mutex); return -EBUSY; } @@ -208,7 +209,7 @@ static int gasket_alloc_dev(struct gasket_internal_desc *internal_desc, gasket_dev = *pdev = kzalloc(sizeof(*gasket_dev), GFP_KERNEL); if (!gasket_dev) { - pr_err("no memory for device\n"); + pr_err("gasket: no memory for device %s\n", kobj_name); return -ENOMEM; } internal_desc->devs[dev_idx] = gasket_dev; @@ -1464,7 +1465,7 @@ static int gasket_pci_probe(struct pci_dev *pci_dev, internal_desc = lookup_internal_desc(pci_dev); mutex_unlock(&g_mutex); if (!internal_desc) { - pr_err("PCI probe called for unknown driver type\n"); + pr_err("gasket: PCI probe called for unknown driver type\n"); return -ENODEV; } @@ -1476,7 +1477,7 @@ static int gasket_pci_probe(struct pci_dev *pci_dev, return ret; gasket_dev->pci_dev = pci_dev_get(pci_dev); if (IS_ERR_OR_NULL(gasket_dev->dev_info.device)) { - pr_err("Cannot create %s device %s [ret = %ld]\n", + pr_err("gasket: Cannot create %s device %s [ret = %ld]\n", driver_desc->name, gasket_dev->dev_info.name, PTR_ERR(gasket_dev->dev_info.device)); ret = -ENODEV; @@ -1523,7 +1524,7 @@ static int gasket_pci_probe(struct pci_dev *pci_dev, ret = gasket_enable_dev(internal_desc, gasket_dev); if (ret) { - pr_err("cannot setup %s device\n", driver_desc->name); + pr_err("gasket: cannot setup %s device\n", driver_desc->name); gasket_disable_dev(gasket_dev); goto fail5; } @@ -1743,7 +1744,7 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) { if (g_descs[i].driver_desc == driver_desc) { - pr_err("%s driver already loaded/registered\n", + pr_err("gasket: %s driver already loaded/registered\n", driver_desc->name); mutex_unlock(&g_mutex); return -EBUSY; @@ -1761,7 +1762,7 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) mutex_unlock(&g_mutex); if (desc_idx == -1) { - pr_err("Too many Gasket drivers loaded: %d\n", + pr_err("gasket: too many drivers loaded, max %d\n", GASKET_FRAMEWORK_DESC_MAX); return -EBUSY; } @@ -1778,7 +1779,7 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) class_create(driver_desc->module, driver_desc->name); if (IS_ERR(internal->class)) { - pr_err("Cannot register %s class [ret=%ld]\n", + pr_err("gasket: Cannot register %s class [ret=%ld]\n", driver_desc->name, PTR_ERR(internal->class)); ret = PTR_ERR(internal->class); goto unregister_gasket_driver; @@ -1791,7 +1792,8 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) ret = __
[PATCH 2/8] staging: gasket: core: print driver version code at registration time
From: Todd Poynor Print the driver version code in the kernel log at gasket driver registration time for informational purposes. Add "gasket:" prefix to make clear it is the gasket framework logging this information. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index f76f4a0ecbac6..e550c9060dcd2 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1759,8 +1759,9 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc) } mutex_unlock(&g_mutex); - pr_info("Loaded %s driver, framework version %s\n", - driver_desc->name, GASKET_FRAMEWORK_VERSION); + pr_info("gasket: Loaded %s driver version %s, framework version %s\n", + driver_desc->name, driver_desc->driver_version, + GASKET_FRAMEWORK_VERSION); if (desc_idx == -1) { pr_err("Too many Gasket drivers loaded: %d\n", -- 2.18.0.597.ga71716f1ad-goog
[PATCH 05/15] staging: gasket: TODO: remove entry for static function declarations
From: Todd Poynor The static function declarations are removed, remove the TODO file entry for this. Signed-off-by: Todd Poynor --- drivers/staging/gasket/TODO | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/gasket/TODO b/drivers/staging/gasket/TODO index 7f4c13ce021b..6aa2a7f6bc4b 100644 --- a/drivers/staging/gasket/TODO +++ b/drivers/staging/gasket/TODO @@ -1,6 +1,5 @@ This is a list of things that need to be done to get this driver out of the staging directory. -- Remove static function declarations. - Document sysfs files with Documentation/ABI/ entries. - Use misc interface instead of major number for driver version description. - Add descriptions of module_param's -- 2.18.0.345.g5c9ce644c3-goog
[PATCH 14/15] Revert "staging: gasket: page table: hold references to device and pci_dev"
From: Todd Poynor gasket_free_dev() is called only from driver PCI probe and remove function. It is guaranteed that that pci_dev structure is not going anywhere during that time; there is no need to take this additional reference. This reverts commit dd9d1502feea3c23d412f289aad79e1d4e86d45d. Reported-by: Dmitry Torokhov Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 13e1d0952a47..ed6ab3c5f038 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -280,7 +280,7 @@ int gasket_page_table_init(struct gasket_page_table **ppg_tbl, pg_tbl->extended_offset_reg = (u64 __iomem *)&bar_data->virt_base[page_table_config->extended_reg]; pg_tbl->device = get_device(device); - pg_tbl->pci_dev = pci_dev_get(pci_dev); + pg_tbl->pci_dev = pci_dev; dev_dbg(device, "Page table initialized successfully\n"); @@ -378,7 +378,6 @@ void gasket_page_table_cleanup(struct gasket_page_table *pg_tbl) pg_tbl->entries = NULL; put_device(pg_tbl->device); - pci_dev_put(pg_tbl->pci_dev); kfree(pg_tbl); } -- 2.18.0.345.g5c9ce644c3-goog
[PATCH 09/15] staging: gasket: sysfs: fix function param line continuation style
From: Todd Poynor Fix multi-line alignment formatting to look like: int ret = long_function_name(device, VARIABLE1, VARIABLE2, VARIABLE3, VARIABLE4); Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_sysfs.c | 26 +++--- drivers/staging/gasket/gasket_sysfs.h | 32 +-- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/drivers/staging/gasket/gasket_sysfs.c b/drivers/staging/gasket/gasket_sysfs.c index ef4eca02afa6..a4bfca43cd03 100644 --- a/drivers/staging/gasket/gasket_sysfs.c +++ b/drivers/staging/gasket/gasket_sysfs.c @@ -145,8 +145,8 @@ void gasket_sysfs_init(void) } } -int gasket_sysfs_create_mapping( - struct device *device, struct gasket_dev *gasket_dev) +int gasket_sysfs_create_mapping(struct device *device, + struct gasket_dev *gasket_dev) { struct gasket_sysfs_mapping *mapping; int map_idx = -1; @@ -210,8 +210,8 @@ int gasket_sysfs_create_mapping( return 0; } -int gasket_sysfs_create_entries( - struct device *device, const struct gasket_sysfs_attribute *attrs) +int gasket_sysfs_create_entries(struct device *device, + const struct gasket_sysfs_attribute *attrs) { int i; int ret; @@ -293,8 +293,8 @@ void gasket_sysfs_put_device_data(struct device *device, struct gasket_dev *dev) } EXPORT_SYMBOL(gasket_sysfs_put_device_data); -struct gasket_sysfs_attribute *gasket_sysfs_get_attr( - struct device *device, struct device_attribute *attr) +struct gasket_sysfs_attribute * +gasket_sysfs_get_attr(struct device *device, struct device_attribute *attr) { int i; int num_attrs; @@ -317,8 +317,8 @@ struct gasket_sysfs_attribute *gasket_sysfs_get_attr( } EXPORT_SYMBOL(gasket_sysfs_get_attr); -void gasket_sysfs_put_attr( - struct device *device, struct gasket_sysfs_attribute *attr) +void gasket_sysfs_put_attr(struct device *device, + struct gasket_sysfs_attribute *attr) { int i; int num_attrs; @@ -342,9 +342,9 @@ void gasket_sysfs_put_attr( } EXPORT_SYMBOL(gasket_sysfs_put_attr); -ssize_t gasket_sysfs_register_store( - struct device *device, struct device_attribute *attr, const char *buf, - size_t count) +ssize_t gasket_sysfs_register_store(struct device *device, + struct device_attribute *attr, + const char *buf, size_t count) { ulong parsed_value = 0; struct gasket_sysfs_mapping *mapping; @@ -386,8 +386,8 @@ ssize_t gasket_sysfs_register_store( gasket_attr->data.bar_address.offset); if (gasket_attr->write_callback) - gasket_attr->write_callback( - gasket_dev, gasket_attr, parsed_value); + gasket_attr->write_callback(gasket_dev, gasket_attr, + parsed_value); gasket_sysfs_put_attr(device, gasket_attr); put_mapping(mapping); diff --git a/drivers/staging/gasket/gasket_sysfs.h b/drivers/staging/gasket/gasket_sysfs.h index e9f4fad80461..f32eaf89e056 100644 --- a/drivers/staging/gasket/gasket_sysfs.h +++ b/drivers/staging/gasket/gasket_sysfs.h @@ -68,9 +68,9 @@ struct gasket_sysfs_attribute { * The callback should perform any logging necessary, as errors cannot * be returned from the callback. */ - void (*write_callback)( - struct gasket_dev *dev, struct gasket_sysfs_attribute *attr, - ulong value); + void (*write_callback)(struct gasket_dev *dev, + struct gasket_sysfs_attribute *attr, + ulong value); }; #define GASKET_SYSFS_RO(_name, _show_function, _attr_type) \ @@ -98,8 +98,8 @@ void gasket_sysfs_init(void); * If this function is not called before gasket_sysfs_create_entries, a warning * will be logged. */ -int gasket_sysfs_create_mapping( - struct device *device, struct gasket_dev *gasket_dev); +int gasket_sysfs_create_mapping(struct device *device, + struct gasket_dev *gasket_dev); /* * Creates bulk entries in sysfs. @@ -111,8 +111,8 @@ int gasket_sysfs_create_mapping( * gasket_sysfs_create_mapping had a legacy device, the entries will be created * for it, as well. */ -int gasket_sysfs_create_entries( - struct device *device, const struct gasket_sysfs_attribute *attrs); +int gasket_sysfs_create_entries(struct device *device, + const struct gasket_sysfs_attribute *attrs); /* * Removes a device mapping from the global table. @@ -141,8 +141,8 @@ struct gasket_dev *gasket_sysfs_get_device_data(struct device *device); * @device: Kernel device structure. * @dev: Gasket device descriptor
[PATCH 03/15] staging: gasket: interrupt: remove static function forward declarations
From: Todd Poynor Remove forward declarations of static functions, move code to avoid forward references, for kernel style. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_interrupt.c | 499 +++--- 1 file changed, 242 insertions(+), 257 deletions(-) diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c index 27fde991edc6..3079b59b122b 100644 --- a/drivers/staging/gasket/gasket_interrupt.c +++ b/drivers/staging/gasket/gasket_interrupt.c @@ -70,32 +70,259 @@ struct gasket_interrupt_data { int irq; }; -/* Function definitions. */ -static ssize_t interrupt_sysfs_show( - struct device *device, struct device_attribute *attr, char *buf); - -static irqreturn_t gasket_msix_interrupt_handler(int irq, void *dev_id); - /* Structures to display interrupt counts in sysfs. */ enum interrupt_sysfs_attribute_type { ATTR_INTERRUPT_COUNTS, }; +/* Set up device registers for interrupt handling. */ +static void gasket_interrupt_setup(struct gasket_dev *gasket_dev) +{ + int i; + int pack_shift; + ulong mask; + ulong value; + struct gasket_interrupt_data *interrupt_data = + gasket_dev->interrupt_data; + + if (!interrupt_data) { + dev_dbg(gasket_dev->dev, "Interrupt data is not initialized\n"); + return; + } + + dev_dbg(gasket_dev->dev, "Running interrupt setup\n"); + + if (interrupt_data->type == PLATFORM_WIRE || + interrupt_data->type == PCI_MSI) { + /* Nothing needs to be done for platform or PCI devices. */ + return; + } + + if (interrupt_data->type != PCI_MSIX) { + dev_dbg(gasket_dev->dev, + "Cannot handle unsupported interrupt type %d\n", + interrupt_data->type); + return; + } + + /* Setup the MSIX table. */ + + for (i = 0; i < interrupt_data->num_interrupts; i++) { + /* +* If the interrupt is not packed, we can write the index into +* the register directly. If not, we need to deal with a read- +* modify-write and shift based on the packing index. +*/ + dev_dbg(gasket_dev->dev, + "Setting up interrupt index %d with index 0x%llx and " + "packing %d\n", + interrupt_data->interrupts[i].index, + interrupt_data->interrupts[i].reg, + interrupt_data->interrupts[i].packing); + if (interrupt_data->interrupts[i].packing == UNPACKED) { + value = interrupt_data->interrupts[i].index; + } else { + switch (interrupt_data->interrupts[i].packing) { + case PACK_0: + pack_shift = 0; + break; + case PACK_1: + pack_shift = interrupt_data->pack_width; + break; + case PACK_2: + pack_shift = 2 * interrupt_data->pack_width; + break; + case PACK_3: + pack_shift = 3 * interrupt_data->pack_width; + break; + default: + dev_dbg(gasket_dev->dev, + "Found interrupt description with " + "unknown enum %d\n", + interrupt_data->interrupts[i].packing); + return; + } + + mask = ~(0x << pack_shift); + value = gasket_dev_read_64( + gasket_dev, + interrupt_data->interrupt_bar_index, + interrupt_data->interrupts[i].reg) & + mask; + value |= interrupt_data->interrupts[i].index +<< pack_shift; + } + gasket_dev_write_64(gasket_dev, value, + interrupt_data->interrupt_bar_index, + interrupt_data->interrupts[i].reg); + } +} + +static irqreturn_t gasket_msix_interrupt_handler(int irq, void *dev_id) +{ + struct eventfd_ctx *ctx; + struct gasket_interrupt_data *interrupt_data = dev_id; + int interrupt = -1; + int i; + + /* If this linear lookup is a problem, we can maintain a map/hash. *
[PATCH 08/15] staging: gasket: page table: fix function param line continuation style
From: Todd Poynor Fix multi-line alignment formatting to look like: int ret = long_function_name(device, VARIABLE1, VARIABLE2, VARIABLE3, VARIABLE4); Many of these TODO items were previously cleaned up during the conversion to standard logging functions. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 206 +++-- drivers/staging/gasket/gasket_page_table.h | 33 ++-- 2 files changed, 120 insertions(+), 119 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index aa036b2e8193..13e1d0952a47 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -215,11 +215,10 @@ struct gasket_page_table { }; /* See gasket_page_table.h for description. */ -int gasket_page_table_init( - struct gasket_page_table **ppg_tbl, - const struct gasket_bar_data *bar_data, - const struct gasket_page_table_config *page_table_config, - struct device *device, struct pci_dev *pci_dev) +int gasket_page_table_init(struct gasket_page_table **ppg_tbl, + const struct gasket_bar_data *bar_data, + const struct gasket_page_table_config *page_table_config, + struct device *device, struct pci_dev *pci_dev) { ulong bytes; struct gasket_page_table *pg_tbl; @@ -276,10 +275,10 @@ int gasket_page_table_init( pg_tbl->extended_flag = 0; } pg_tbl->num_active_pages = 0; - pg_tbl->base_slot = (u64 __iomem *)&( - bar_data->virt_base[page_table_config->base_reg]); - pg_tbl->extended_offset_reg = (u64 __iomem *)&( - bar_data->virt_base[page_table_config->extended_reg]); + pg_tbl->base_slot = + (u64 __iomem *)&bar_data->virt_base[page_table_config->base_reg]; + pg_tbl->extended_offset_reg = + (u64 __iomem *)&bar_data->virt_base[page_table_config->extended_reg]; pg_tbl->device = get_device(device); pg_tbl->pci_dev = pci_dev_get(pci_dev); @@ -292,8 +291,8 @@ int gasket_page_table_init( * Check if a range of PTEs is free. * The page table mutex must be held by the caller. */ -static bool gasket_is_pte_range_free( - struct gasket_page_table_entry *ptes, uint num_entries) +static bool gasket_is_pte_range_free(struct gasket_page_table_entry *ptes, +uint num_entries) { int i; @@ -309,9 +308,9 @@ static bool gasket_is_pte_range_free( * Free a second level page [sub]table. * The page table mutex must be held before this call. */ -static void gasket_free_extended_subtable( - struct gasket_page_table *pg_tbl, struct gasket_page_table_entry *pte, - u64 __iomem *slot) +static void gasket_free_extended_subtable(struct gasket_page_table *pg_tbl, + struct gasket_page_table_entry *pte, + u64 __iomem *slot) { /* Release the page table from the driver */ pte->status = PTE_FREE; @@ -337,8 +336,8 @@ static void gasket_free_extended_subtable( * Actually perform collection. * The page table mutex must be held by the caller. */ -static void gasket_page_table_garbage_collect_nolock( - struct gasket_page_table *pg_tbl) +static void +gasket_page_table_garbage_collect_nolock(struct gasket_page_table *pg_tbl) { struct gasket_page_table_entry *pte; u64 __iomem *slot; @@ -351,10 +350,10 @@ static void gasket_page_table_garbage_collect_nolock( pte < pg_tbl->entries + pg_tbl->config.total_entries; pte++, slot++) { if (pte->status == PTE_INUSE) { - if (gasket_is_pte_range_free( - pte->sublevel, GASKET_PAGES_PER_SUBTABLE)) - gasket_free_extended_subtable( - pg_tbl, pte, slot); + if (gasket_is_pte_range_free(pte->sublevel, +GASKET_PAGES_PER_SUBTABLE)) + gasket_free_extended_subtable(pg_tbl, pte, + slot); } } } @@ -384,8 +383,8 @@ void gasket_page_table_cleanup(struct gasket_page_table *pg_tbl) } /* See gasket_page_table.h for description. */ -int gasket_page_table_partition( - struct gasket_page_table *pg_tbl, uint num_simple_entries) +int gasket_page_table_partition(struct gasket_page_table *pg_tbl, + uint num_simple_entries) { int i, start; @@ -445,10 +444,10 @@ static int is_coherent(struct gasket_page_table *pg_tbl, ulong host_addr) * an extended mapping,
[PATCH 15/15] staging: gasket: page table: fix header file include guard symbol
From: Todd Poynor The include guard symbol for gasket_page_table.h is out-of-date. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.h b/drivers/staging/gasket/gasket_page_table.h index 00c06f050eb9..7b01b73ea3e7 100644 --- a/drivers/staging/gasket/gasket_page_table.h +++ b/drivers/staging/gasket/gasket_page_table.h @@ -9,8 +9,8 @@ * Copyright (C) 2018 Google, Inc. */ -#ifndef __GASKET_ADDR_TRNSL_H__ -#define __GASKET_ADDR_TRNSL_H__ +#ifndef __GASKET_PAGE_TABLE_H__ +#define __GASKET_PAGE_TABLE_H__ #include #include @@ -246,4 +246,4 @@ void gasket_free_coherent_memory_all(struct gasket_dev *gasket_dev, int gasket_set_user_virt(struct gasket_dev *gasket_dev, uint64_t size, dma_addr_t dma_address, ulong vma); -#endif +#endif /* __GASKET_PAGE_TABLE_H__ */ -- 2.18.0.345.g5c9ce644c3-goog
[PATCH 10/15] staging: gasket: interrupt: fix function param line continuation style
From: Todd Poynor Fix multi-line alignment formatting to look like: int ret = long_function_name(device, VARIABLE1, VARIABLE2, VARIABLE3, VARIABLE4); Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_interrupt.c | 73 +++ drivers/staging/gasket/gasket_interrupt.h | 19 +++--- 2 files changed, 43 insertions(+), 49 deletions(-) diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c index 3079b59b122b..09c3d0747af6 100644 --- a/drivers/staging/gasket/gasket_interrupt.c +++ b/drivers/staging/gasket/gasket_interrupt.c @@ -144,11 +144,10 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev) } mask = ~(0x << pack_shift); - value = gasket_dev_read_64( - gasket_dev, - interrupt_data->interrupt_bar_index, - interrupt_data->interrupts[i].reg) & - mask; + value = gasket_dev_read_64(gasket_dev, + interrupt_data->interrupt_bar_index, + interrupt_data->interrupts[i].reg); + value &= mask; value |= interrupt_data->interrupts[i].index << pack_shift; } @@ -187,8 +186,8 @@ static irqreturn_t gasket_msix_interrupt_handler(int irq, void *dev_id) return IRQ_HANDLED; } -static int gasket_interrupt_msix_init( - struct gasket_interrupt_data *interrupt_data) +static int +gasket_interrupt_msix_init(struct gasket_interrupt_data *interrupt_data) { int ret = 1; int i; @@ -210,10 +209,9 @@ static int gasket_interrupt_msix_init( interrupt_data->msix_configured = 1; for (i = 0; i < interrupt_data->num_interrupts; i++) { - ret = request_irq( - interrupt_data->msix_entries[i].vector, - gasket_msix_interrupt_handler, 0, interrupt_data->name, - interrupt_data); + ret = request_irq(interrupt_data->msix_entries[i].vector, + gasket_msix_interrupt_handler, 0, + interrupt_data->name, interrupt_data); if (ret) { dev_err(&interrupt_data->pci_dev->dev, @@ -250,25 +248,23 @@ static void force_msix_interrupt_unmasking(struct gasket_dev *gasket_dev) ulong location = APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE + MSIX_MASK_BIT_OFFSET + i * MSIX_VECTOR_SIZE; u32 mask = - gasket_dev_read_32( - gasket_dev, - gasket_dev->interrupt_data->interrupt_bar_index, - location); + gasket_dev_read_32(gasket_dev, + gasket_dev->interrupt_data->interrupt_bar_index, + location); if (!(mask & 1)) continue; /* Unmask the msix vector (clear 32 bits) */ - gasket_dev_write_32( - gasket_dev, 0, - gasket_dev->interrupt_data->interrupt_bar_index, - location); + gasket_dev_write_32(gasket_dev, 0, + gasket_dev->interrupt_data->interrupt_bar_index, + location); } #undef MSIX_VECTOR_SIZE #undef MSIX_MASK_BIT_OFFSET #undef APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE } -static ssize_t interrupt_sysfs_show( - struct device *device, struct device_attribute *attr, char *buf) +static ssize_t interrupt_sysfs_show(struct device *device, + struct device_attribute *attr, char *buf) { int i, ret; ssize_t written = 0, total_written = 0; @@ -318,22 +314,22 @@ static ssize_t interrupt_sysfs_show( } static struct gasket_sysfs_attribute interrupt_sysfs_attrs[] = { - GASKET_SYSFS_RO( - interrupt_counts, interrupt_sysfs_show, ATTR_INTERRUPT_COUNTS), + GASKET_SYSFS_RO(interrupt_counts, interrupt_sysfs_show, + ATTR_INTERRUPT_COUNTS), GASKET_END_OF_ATTR_ARRAY, }; -int gasket_interrupt_init( - struct gasket_dev *gasket_dev, const char *name, int type, - const struct gasket_interrupt_desc *interrupts, - int num_interrupts, int pack_width, int bar_index, - const struct gasket_wire_interrupt_offsets *wire_int_offsets) +int gasket_interrupt_init(struct gasket_dev *gas
[PATCH 11/15] staging: gasket: TODO: remove entry for multi-line alignment style
From: Todd Poynor Multi-line alignment formatting issues fixed, remove the TODO entry for this. Signed-off-by: Todd Poynor --- drivers/staging/gasket/TODO | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/gasket/TODO b/drivers/staging/gasket/TODO index 6aa2a7f6bc4b..6ff8e01b04cc 100644 --- a/drivers/staging/gasket/TODO +++ b/drivers/staging/gasket/TODO @@ -4,9 +4,6 @@ staging directory. - Use misc interface instead of major number for driver version description. - Add descriptions of module_param's - apex_get_status() should actually check status. -- Fix multi-line alignment formatting to look like: - int ret = long_function_name(device, VARIABLE1, VARIABLE2, - VARIABLE3, VARIABLE4); - "drivers" should never be dealing with "raw" sysfs calls or mess around with kobjects at all. The driver core should handle all of this for you automaically. There should not be a need for raw attribute macros. -- 2.18.0.345.g5c9ce644c3-goog
[PATCH 13/15] staging: gasket: core: use bool type for ns_capable result
From: Todd Poynor When gasket core was converted from using capable() to use ns_capable() instead, the type of the variable holding the result should have been converted from int to bool. Reported-by: Dmitry Torokhov Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 44344528cd88..f76f4a0ecbac 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1191,7 +1191,7 @@ static int gasket_open(struct inode *inode, struct file *filp) struct gasket_cdev_info *dev_info = container_of(inode->i_cdev, struct gasket_cdev_info, cdev); struct pid_namespace *pid_ns = task_active_pid_ns(current); - int is_root = ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN); + bool is_root = ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN); gasket_dev = dev_info->gasket_dev_ptr; driver_desc = gasket_dev->internal_desc->driver_desc; @@ -1270,7 +1270,7 @@ static int gasket_release(struct inode *inode, struct file *file) struct gasket_cdev_info *dev_info = container_of(inode->i_cdev, struct gasket_cdev_info, cdev); struct pid_namespace *pid_ns = task_active_pid_ns(current); - int is_root = ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN); + bool is_root = ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN); gasket_dev = dev_info->gasket_dev_ptr; driver_desc = gasket_dev->internal_desc->driver_desc; -- 2.18.0.345.g5c9ce644c3-goog
[PATCH 12/15] staging: gasket: apex: move driver-private defines out of apex.h
From: Todd Poynor apex.h is supposed to contain kernel-userspace interface definitions, but has a number of defines that are only used by apex_driver.c or are not used at all. Move driver implementation defines not shared with userspace to the driver source. Remove unused defines. Signed-off-by: Todd Poynor --- drivers/staging/gasket/apex.h| 62 +--- drivers/staging/gasket/apex_driver.c | 29 + 2 files changed, 31 insertions(+), 60 deletions(-) diff --git a/drivers/staging/gasket/apex.h b/drivers/staging/gasket/apex.h index d89cc2387b7d..3bbce5e4 100644 --- a/drivers/staging/gasket/apex.h +++ b/drivers/staging/gasket/apex.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0 */ /* - * Apex kernel-userspace interface definition(s). + * Apex kernel-userspace interface definitions. * * Copyright (C) 2018 Google, Inc. */ @@ -8,66 +8,8 @@ #define __APEX_H__ #include -#include -#include "gasket.h" - -/* Structural definitions/macros. */ -/* The number of PCI BARs. */ -#define APEX_NUM_BARS 3 - -/* Size of a memory page in bytes, and the related number of bits to shift. */ -#define APEX_PAGE_SHIFT 12 -#define APEX_PAGE_SIZE BIT(APEX_PAGE_SHIFT) - -#define APEX_EXTENDED_SHIFT 63 /* Extended address bit position. */ - -/* - * Addresses are 2^3=8 bytes each. Page in second level page table holds - * APEX_PAGE_SIZE/8 addresses. - */ -#define APEX_ADDR_SHIFT 3 -#define APEX_LEVEL_SHIFT (APEX_PAGE_SHIFT - APEX_ADDR_SHIFT) -#define APEX_LEVEL_SIZE BIT(APEX_LEVEL_SHIFT) - -#define APEX_PAGE_TABLE_MAX 65536 -#define APEX_SIMPLE_PAGE_MAX APEX_PAGE_TABLE_MAX -#define APEX_EXTENDED_PAGE_MAX (APEX_PAGE_TABLE_MAX << APEX_LEVEL_SHIFT) - -/* Check reset 120 times */ -#define APEX_RESET_RETRY 120 -/* Wait 100 ms between checks. Total 12 sec wait maximum. */ -#define APEX_RESET_DELAY 100 - -#define APEX_CHIP_INIT_DONE 2 -#define APEX_RESET_ACCEPTED 0 - -enum apex_reset_types { - APEX_CHIP_REINIT_RESET = 3, -}; - -/* Interrupt defines */ -/* Gasket device interrupts enums must be dense (i.e., no empty slots). */ -enum apex_interrupt { - APEX_INTERRUPT_INSTR_QUEUE = 0, - APEX_INTERRUPT_INPUT_ACTV_QUEUE = 1, - APEX_INTERRUPT_PARAM_QUEUE = 2, - APEX_INTERRUPT_OUTPUT_ACTV_QUEUE = 3, - APEX_INTERRUPT_SC_HOST_0 = 4, - APEX_INTERRUPT_SC_HOST_1 = 5, - APEX_INTERRUPT_SC_HOST_2 = 6, - APEX_INTERRUPT_SC_HOST_3 = 7, - APEX_INTERRUPT_TOP_LEVEL_0 = 8, - APEX_INTERRUPT_TOP_LEVEL_1 = 9, - APEX_INTERRUPT_TOP_LEVEL_2 = 10, - APEX_INTERRUPT_TOP_LEVEL_3 = 11, - APEX_INTERRUPT_FATAL_ERR = 12, - APEX_INTERRUPT_COUNT = 13, -}; - -/* - * Clock Gating ioctl. - */ +/* Clock Gating ioctl. */ struct apex_gate_clock_ioctl { /* Enter or leave clock gated state. */ u64 enable; diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index c0d3922e1d7c..dfbff47b4608 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -54,6 +54,17 @@ */ #define APEX_PAGE_TABLE_TOTAL_ENTRIES 8192 +#define APEX_EXTENDED_SHIFT 63 /* Extended address bit position. */ + +enum apex_reset_types { + APEX_CHIP_REINIT_RESET = 3, +}; + +/* Check reset 120 times */ +#define APEX_RESET_RETRY 120 +/* Wait 100 ms between checks. Total 12 sec wait maximum. */ +#define APEX_RESET_DELAY 100 + /* Enumeration of the supported sysfs entries. */ enum sysfs_attribute_type { ATTR_KERNEL_HIB_PAGE_TABLE_SIZE, @@ -133,6 +144,24 @@ static const struct gasket_mappable_region mappable_regions[NUM_REGIONS] = { static const struct gasket_mappable_region cm_mappable_regions[1] = { { 0x0, APEX_CH_MEM_BYTES } }; +/* Gasket device interrupts enums must be dense (i.e., no empty slots). */ +enum apex_interrupt { + APEX_INTERRUPT_INSTR_QUEUE = 0, + APEX_INTERRUPT_INPUT_ACTV_QUEUE = 1, + APEX_INTERRUPT_PARAM_QUEUE = 2, + APEX_INTERRUPT_OUTPUT_ACTV_QUEUE = 3, + APEX_INTERRUPT_SC_HOST_0 = 4, + APEX_INTERRUPT_SC_HOST_1 = 5, + APEX_INTERRUPT_SC_HOST_2 = 6, + APEX_INTERRUPT_SC_HOST_3 = 7, + APEX_INTERRUPT_TOP_LEVEL_0 = 8, + APEX_INTERRUPT_TOP_LEVEL_1 = 9, + APEX_INTERRUPT_TOP_LEVEL_2 = 10, + APEX_INTERRUPT_TOP_LEVEL_3 = 11, + APEX_INTERRUPT_FATAL_ERR = 12, + APEX_INTERRUPT_COUNT = 13, +}; + /* Interrupt descriptors for Apex */ static struct gasket_interrupt_desc apex_interrupts[] = { { -- 2.18.0.345.g5c9ce644c3-goog
[PATCH 07/15] staging: gasket: ioctl: fix function param line continuation style
From: Todd Poynor Fix multi-line alignment formatting to look like: int ret = long_function_name(device, VARIABLE1, VARIABLE2, VARIABLE3, VARIABLE4); Many of these TODO items were previously cleaned up during the conversion to standard logging functions. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_ioctl.c | 60 +-- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c index 134d45a281ac..d3397cc74e69 100644 --- a/drivers/staging/gasket/gasket_ioctl.c +++ b/drivers/staging/gasket/gasket_ioctl.c @@ -79,12 +79,12 @@ static int gasket_read_simple_page_table_size( if (ibuf.page_table_index >= gasket_dev->num_page_tables) return -EFAULT; - ibuf.size = gasket_page_table_num_simple_entries( - gasket_dev->page_table[ibuf.page_table_index]); + ibuf.size = + gasket_page_table_num_simple_entries(gasket_dev->page_table[ibuf.page_table_index]); - trace_gasket_ioctl_page_table_data( - ibuf.page_table_index, ibuf.size, ibuf.host_address, - ibuf.device_address); + trace_gasket_ioctl_page_table_data(ibuf.page_table_index, ibuf.size, + ibuf.host_address, + ibuf.device_address); if (copy_to_user(argp, &ibuf, sizeof(ibuf))) return -EFAULT; @@ -138,21 +138,21 @@ static int gasket_map_buffers(struct gasket_dev *gasket_dev, if (copy_from_user(&ibuf, argp, sizeof(struct gasket_page_table_ioctl))) return -EFAULT; - trace_gasket_ioctl_page_table_data( - ibuf.page_table_index, ibuf.size, ibuf.host_address, - ibuf.device_address); + trace_gasket_ioctl_page_table_data(ibuf.page_table_index, ibuf.size, + ibuf.host_address, + ibuf.device_address); if (ibuf.page_table_index >= gasket_dev->num_page_tables) return -EFAULT; - if (gasket_page_table_are_addrs_bad( - gasket_dev->page_table[ibuf.page_table_index], - ibuf.host_address, ibuf.device_address, ibuf.size)) + if (gasket_page_table_are_addrs_bad(gasket_dev->page_table[ibuf.page_table_index], + ibuf.host_address, + ibuf.device_address, ibuf.size)) return -EINVAL; - return gasket_page_table_map( - gasket_dev->page_table[ibuf.page_table_index], - ibuf.host_address, ibuf.device_address, ibuf.size / PAGE_SIZE); + return gasket_page_table_map(gasket_dev->page_table[ibuf.page_table_index], +ibuf.host_address, ibuf.device_address, +ibuf.size / PAGE_SIZE); } /* Unmap a userspace buffer from a device virtual address. */ @@ -164,16 +164,15 @@ static int gasket_unmap_buffers(struct gasket_dev *gasket_dev, if (copy_from_user(&ibuf, argp, sizeof(struct gasket_page_table_ioctl))) return -EFAULT; - trace_gasket_ioctl_page_table_data( - ibuf.page_table_index, ibuf.size, ibuf.host_address, - ibuf.device_address); + trace_gasket_ioctl_page_table_data(ibuf.page_table_index, ibuf.size, + ibuf.host_address, + ibuf.device_address); if (ibuf.page_table_index >= gasket_dev->num_page_tables) return -EFAULT; - if (gasket_page_table_is_dev_addr_bad( - gasket_dev->page_table[ibuf.page_table_index], - ibuf.device_address, ibuf.size)) + if (gasket_page_table_is_dev_addr_bad(gasket_dev->page_table[ibuf.page_table_index], + ibuf.device_address, ibuf.size)) return -EINVAL; gasket_page_table_unmap(gasket_dev->page_table[ibuf.page_table_index], @@ -197,8 +196,8 @@ static int gasket_config_coherent_allocator( sizeof(struct gasket_coherent_alloc_config_ioctl))) return -EFAULT; - trace_gasket_ioctl_config_coherent_allocator( - ibuf.enable, ibuf.size, ibuf.dma_address); + trace_gasket_ioctl_config_coherent_allocator(ibuf.enable, ibuf.size, +ibuf.dma_address); if (ibuf.page_table_index >= gasket_dev->num_page_tables) return -EFAULT; @@ -207,13 +206,13 @@ static int gasket_config_coherent_allocator( return -ENOMEM; if (ibuf.enable == 0) { - ret = gasket_free
[PATCH 06/15] staging: gasket: core: fix function param line continuation style
From: Todd Poynor Fix multi-line alignment formatting to look like: int ret = long_function_name(device, VARIABLE1, VARIABLE2, VARIABLE3, VARIABLE4); Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 294 +-- drivers/staging/gasket/gasket_core.h | 68 +++ 2 files changed, 179 insertions(+), 183 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index b5a7254fbfb3..44344528cd88 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -103,8 +103,9 @@ enum gasket_sysfs_attribute_type { }; /* Perform a standard Gasket callback. */ -static inline int check_and_invoke_callback( - struct gasket_dev *gasket_dev, int (*cb_function)(struct gasket_dev *)) +static inline int +check_and_invoke_callback(struct gasket_dev *gasket_dev, + int (*cb_function)(struct gasket_dev *)) { int ret = 0; @@ -119,8 +120,9 @@ static inline int check_and_invoke_callback( } /* Perform a standard Gasket callback without grabbing gasket_dev->mutex. */ -static inline int gasket_check_and_invoke_callback_nolock( - struct gasket_dev *gasket_dev, int (*cb_function)(struct gasket_dev *)) +static inline int +gasket_check_and_invoke_callback_nolock(struct gasket_dev *gasket_dev, + int (*cb_function)(struct gasket_dev *)) { int ret = 0; @@ -147,8 +149,8 @@ static int gasket_owned_by_current_tgid(struct gasket_cdev_info *info) * * Returns the located slot number on success or a negative number on failure. */ -static int gasket_find_dev_slot( - struct gasket_internal_desc *internal_desc, const char *kobj_name) +static int gasket_find_dev_slot(struct gasket_internal_desc *internal_desc, + const char *kobj_name) { int i; @@ -186,9 +188,9 @@ static int gasket_find_dev_slot( * * Returns 0 if successful, a negative error code otherwise. */ -static int gasket_alloc_dev( - struct gasket_internal_desc *internal_desc, struct device *parent, - struct gasket_dev **pdev, const char *kobj_name) +static int gasket_alloc_dev(struct gasket_internal_desc *internal_desc, + struct device *parent, struct gasket_dev **pdev, + const char *kobj_name) { int dev_idx; const struct gasket_driver_desc *driver_desc = @@ -228,7 +230,7 @@ static int gasket_alloc_dev( gasket_dev->dev_idx); dev_info->devt = MKDEV(driver_desc->major, driver_desc->minor + - gasket_dev->dev_idx); + gasket_dev->dev_idx); dev_info->device = device_create(internal_desc->class, parent, dev_info->devt, gasket_dev, dev_info->name); @@ -371,8 +373,8 @@ static void gasket_unmap_pci_bar(struct gasket_dev *dev, int bar_num) * * Returns 0 on success and a negative value otherwise. */ -static int gasket_setup_pci( - struct pci_dev *pci_dev, struct gasket_dev *gasket_dev) +static int gasket_setup_pci(struct pci_dev *pci_dev, + struct gasket_dev *gasket_dev) { int i, mapped_bars, ret; @@ -421,8 +423,8 @@ static int gasket_get_hw_status(struct gasket_dev *gasket_dev) const struct gasket_driver_desc *driver_desc = gasket_dev->internal_desc->driver_desc; - status = gasket_check_and_invoke_callback_nolock( - gasket_dev, driver_desc->device_status_cb); + status = gasket_check_and_invoke_callback_nolock(gasket_dev, + driver_desc->device_status_cb); if (status != GASKET_STATUS_ALIVE) { dev_dbg(gasket_dev->dev, "Hardware reported status %d.\n", status); @@ -437,8 +439,7 @@ static int gasket_get_hw_status(struct gasket_dev *gasket_dev) } for (i = 0; i < driver_desc->num_page_tables; ++i) { - status = gasket_page_table_system_status( - gasket_dev->page_table[i]); + status = gasket_page_table_system_status(gasket_dev->page_table[i]); if (status != GASKET_STATUS_ALIVE) { dev_dbg(gasket_dev->dev, "Page table %d reported status %d.\n", @@ -450,8 +451,10 @@ static int gasket_get_hw_status(struct gasket_dev *gasket_dev) return GASKET_STATUS_ALIVE; } -static ssize_t gasket_write_mappable_regions( - char *buf, const struct gasket_driver_desc *driver_desc, int bar_index) +static ssize_t +gasket_write_mappable_regions(char *buf, + const struct gasket_driver_desc *driver_desc, + int bar_i
[PATCH 00/15] staging: gasket: cleanups continue
From: Todd Poynor More cleanups for the gasket and apex drivers: finish up TODO items for static function forward declarations and multi-line alignment style, a couple of fixups for recent patch feedback, and a couple other issues found in the meantime. Todd Poynor (15): staging: gasket: core: remove static function forward declarations staging: gasket: ioctl: remove static function forward declarations staging: gasket: interrupt: remove static function forward declarations staging: gasket: pg tbl: remove static function forward declarations staging: gasket: TODO: remove entry for static function declarations staging: gasket: core: fix function param line continuation style staging: gasket: ioctl: fix function param line continuation style staging: gasket: page table: fix function param line continuation style staging: gasket: sysfs: fix function param line continuation style staging: gasket: interrupt: fix function param line continuation style staging: gasket: TODO: remove entry for multi-line alignment style staging: gasket: apex: move driver-private defines out of apex.h staging: gasket: core: use bool type for ns_capable result Revert "staging: gasket: page table: hold references to device and pci_dev" staging: gasket: page table: fix header file include guard symbol drivers/staging/gasket/TODO|4 - drivers/staging/gasket/apex.h | 62 +- drivers/staging/gasket/apex_driver.c | 29 + drivers/staging/gasket/gasket_core.c | 2040 ++-- drivers/staging/gasket/gasket_core.h | 68 +- drivers/staging/gasket/gasket_interrupt.c | 530 +++-- drivers/staging/gasket/gasket_interrupt.h | 19 +- drivers/staging/gasket/gasket_ioctl.c | 412 ++-- drivers/staging/gasket/gasket_page_table.c | 1434 +++--- drivers/staging/gasket/gasket_page_table.h | 39 +- drivers/staging/gasket/gasket_sysfs.c | 26 +- drivers/staging/gasket/gasket_sysfs.h | 32 +- 12 files changed, 2248 insertions(+), 2447 deletions(-) -- 2.18.0.345.g5c9ce644c3-goog
[PATCH 04/15] staging: gasket: pg tbl: remove static function forward declarations
From: Todd Poynor Remove forward declarations of static functions, move code to avoid forward references, for kernel style. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_page_table.c | 1433 ++-- 1 file changed, 684 insertions(+), 749 deletions(-) diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index b42f6637b909..aa036b2e8193 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -214,71 +214,6 @@ struct gasket_page_table { struct gasket_coherent_page_entry *coherent_pages; }; -/* Mapping declarations */ -static int gasket_map_simple_pages( - struct gasket_page_table *pg_tbl, ulong host_addr, - ulong dev_addr, uint num_pages); -static int gasket_map_extended_pages( - struct gasket_page_table *pg_tbl, ulong host_addr, - ulong dev_addr, uint num_pages); -static int gasket_perform_mapping( - struct gasket_page_table *pg_tbl, - struct gasket_page_table_entry *pte_base, u64 __iomem *att_base, - ulong host_addr, uint num_pages, int is_simple_mapping); - -static int gasket_alloc_simple_entries( - struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages); -static int gasket_alloc_extended_entries( - struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_entries); -static int gasket_alloc_extended_subtable( - struct gasket_page_table *pg_tbl, struct gasket_page_table_entry *pte, - u64 __iomem *att_reg); - -/* Unmapping declarations */ -static void gasket_page_table_unmap_nolock( - struct gasket_page_table *pg_tbl, ulong start_addr, uint num_pages); -static void gasket_page_table_unmap_all_nolock( - struct gasket_page_table *pg_tbl); -static void gasket_unmap_simple_pages( - struct gasket_page_table *pg_tbl, ulong start_addr, uint num_pages); -static void gasket_unmap_extended_pages( - struct gasket_page_table *pg_tbl, ulong start_addr, uint num_pages); -static void gasket_perform_unmapping( - struct gasket_page_table *pg_tbl, - struct gasket_page_table_entry *pte_base, u64 __iomem *att_base, - uint num_pages, int is_simple_mapping); - -static void gasket_free_extended_subtable( - struct gasket_page_table *pg_tbl, struct gasket_page_table_entry *pte, - u64 __iomem *att_reg); -static bool gasket_release_page(struct page *page); - -/* Other/utility declarations */ -static inline bool gasket_addr_is_simple( - struct gasket_page_table *pg_tbl, ulong addr); -static bool gasket_is_simple_dev_addr_bad( - struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages); -static bool gasket_is_extended_dev_addr_bad( - struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages); -static bool gasket_is_pte_range_free( - struct gasket_page_table_entry *pte, uint num_entries); -static void gasket_page_table_garbage_collect_nolock( - struct gasket_page_table *pg_tbl); - -/* Address format declarations */ -static ulong gasket_components_to_dev_address( - struct gasket_page_table *pg_tbl, int is_simple, uint page_index, - uint offset); -static int gasket_simple_page_idx( - struct gasket_page_table *pg_tbl, ulong dev_addr); -static ulong gasket_extended_lvl0_page_idx( - struct gasket_page_table *pg_tbl, ulong dev_addr); -static ulong gasket_extended_lvl1_page_idx( - struct gasket_page_table *pg_tbl, ulong dev_addr); - -static int is_coherent(struct gasket_page_table *pg_tbl, ulong host_addr); - -/* Public/exported functions */ /* See gasket_page_table.h for description. */ int gasket_page_table_init( struct gasket_page_table **ppg_tbl, @@ -353,6 +288,85 @@ int gasket_page_table_init( return 0; } +/* + * Check if a range of PTEs is free. + * The page table mutex must be held by the caller. + */ +static bool gasket_is_pte_range_free( + struct gasket_page_table_entry *ptes, uint num_entries) +{ + int i; + + for (i = 0; i < num_entries; i++) { + if (ptes[i].status != PTE_FREE) + return false; + } + + return true; +} + +/* + * Free a second level page [sub]table. + * The page table mutex must be held before this call. + */ +static void gasket_free_extended_subtable( + struct gasket_page_table *pg_tbl, struct gasket_page_table_entry *pte, + u64 __iomem *slot) +{ + /* Release the page table from the driver */ + pte->status = PTE_FREE; + + /* Release the page table from the device */ + writeq(0, slot); + /* Force sync around the address release. */ + mb(); + + if (pte->dma_addr) + dma_unmap_page(pg_tbl->device, pte->dma_addr, PAGE_SIZE, + DMA_BIDIRECTIONAL); + + vfree(pte->sublevel); + + if (pte->page) + free_page((ulong)page_address(pte->page)); + + mems
[PATCH 02/15] staging: gasket: ioctl: remove static function forward declarations
From: Todd Poynor Remove forward declarations of static functions, move code to avoid forward references, for kernel style. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_ioctl.c | 356 -- 1 file changed, 168 insertions(+), 188 deletions(-) diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c index 55bdd7bfac86..134d45a281ac 100644 --- a/drivers/staging/gasket/gasket_ioctl.c +++ b/drivers/staging/gasket/gasket_ioctl.c @@ -23,194 +23,6 @@ #define trace_gasket_ioctl_config_coherent_allocator(x, ...) #endif -static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd); -static int gasket_set_event_fd(struct gasket_dev *dev, - struct gasket_interrupt_eventfd __user *argp); -static int gasket_read_page_table_size( - struct gasket_dev *gasket_dev, - struct gasket_page_table_ioctl __user *argp); -static int gasket_read_simple_page_table_size( - struct gasket_dev *gasket_dev, - struct gasket_page_table_ioctl __user *argp); -static int gasket_partition_page_table( - struct gasket_dev *gasket_dev, - struct gasket_page_table_ioctl __user *argp); -static int gasket_map_buffers(struct gasket_dev *gasket_dev, - struct gasket_page_table_ioctl __user *argp); -static int gasket_unmap_buffers(struct gasket_dev *gasket_dev, - struct gasket_page_table_ioctl __user *argp); -static int gasket_config_coherent_allocator( - struct gasket_dev *gasket_dev, - struct gasket_coherent_alloc_config_ioctl __user *argp); - -/* - * standard ioctl dispatch function. - * @filp: File structure pointer describing this node usage session. - * @cmd: ioctl number to handle. - * @argp: ioctl-specific data pointer. - * - * Standard ioctl dispatcher; forwards operations to individual handlers. - */ -long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp) -{ - struct gasket_dev *gasket_dev; - unsigned long arg = (unsigned long)argp; - gasket_ioctl_permissions_cb_t ioctl_permissions_cb; - int retval; - - gasket_dev = (struct gasket_dev *)filp->private_data; - trace_gasket_ioctl_entry(gasket_dev->dev_info.name, cmd); - - ioctl_permissions_cb = gasket_get_ioctl_permissions_cb(gasket_dev); - if (ioctl_permissions_cb) { - retval = ioctl_permissions_cb(filp, cmd, argp); - if (retval < 0) { - trace_gasket_ioctl_exit(retval); - return retval; - } else if (retval == 0) { - trace_gasket_ioctl_exit(-EPERM); - return -EPERM; - } - } else if (!gasket_ioctl_check_permissions(filp, cmd)) { - trace_gasket_ioctl_exit(-EPERM); - dev_dbg(gasket_dev->dev, "ioctl cmd=%x noperm\n", cmd); - return -EPERM; - } - - /* Tracing happens in this switch statement for all ioctls with -* an integer argrument, but ioctls with a struct argument -* that needs copying and decoding, that tracing is done within -* the handler call. -*/ - switch (cmd) { - case GASKET_IOCTL_RESET: - trace_gasket_ioctl_integer_data(arg); - retval = gasket_reset(gasket_dev, arg); - break; - case GASKET_IOCTL_SET_EVENTFD: - retval = gasket_set_event_fd(gasket_dev, argp); - break; - case GASKET_IOCTL_CLEAR_EVENTFD: - trace_gasket_ioctl_integer_data(arg); - retval = gasket_interrupt_clear_eventfd( - gasket_dev->interrupt_data, (int)arg); - break; - case GASKET_IOCTL_PARTITION_PAGE_TABLE: - trace_gasket_ioctl_integer_data(arg); - retval = gasket_partition_page_table(gasket_dev, argp); - break; - case GASKET_IOCTL_NUMBER_PAGE_TABLES: - trace_gasket_ioctl_integer_data(gasket_dev->num_page_tables); - if (copy_to_user(argp, &gasket_dev->num_page_tables, -sizeof(uint64_t))) - retval = -EFAULT; - else - retval = 0; - break; - case GASKET_IOCTL_PAGE_TABLE_SIZE: - retval = gasket_read_page_table_size(gasket_dev, argp); - break; - case GASKET_IOCTL_SIMPLE_PAGE_TABLE_SIZE: - retval = gasket_read_simple_page_table_size(gasket_dev, argp); - break; - case GASKET_IOCTL_MAP_BUFFER: - retval = gasket_map_buffers(gasket_dev, argp); - break; - case GASKET_IOCTL_CONFIG_COHERENT_ALLOCATOR: - retval = gasket_config_coherent_allocator(gasket_dev, argp); - break; - case GASKET_IOCTL_U
[PATCH 01/15] staging: gasket: core: remove static function forward declarations
From: Todd Poynor Remove forward declarations of static functions, move code to avoid forward references, for kernel style. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 1900 +- 1 file changed, 922 insertions(+), 978 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index c00774059f9e..b5a7254fbfb3 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -67,61 +67,6 @@ enum do_map_region_status { DO_MAP_REGION_INVALID, }; -/* Function declarations; comments are with definitions. */ -static int __init gasket_init(void); -static void __exit gasket_exit(void); - -static int gasket_pci_probe( - struct pci_dev *pci_dev, const struct pci_device_id *id); -static void gasket_pci_remove(struct pci_dev *pci_dev); - -static int gasket_setup_pci(struct pci_dev *pci_dev, struct gasket_dev *dev); -static void gasket_cleanup_pci(struct gasket_dev *dev); - -static int gasket_map_pci_bar(struct gasket_dev *dev, int bar_num); -static void gasket_unmap_pci_bar(struct gasket_dev *dev, int bar_num); - -static int gasket_alloc_dev( - struct gasket_internal_desc *internal_desc, struct device *dev, - struct gasket_dev **pdev, const char *kobj_name); -static void gasket_free_dev(struct gasket_dev *dev); - -static int gasket_find_dev_slot( - struct gasket_internal_desc *internal_desc, const char *kobj_name); - -static int gasket_add_cdev( - struct gasket_cdev_info *dev_info, - const struct file_operations *file_ops, struct module *owner); - -static int gasket_enable_dev( - struct gasket_internal_desc *internal_desc, - struct gasket_dev *gasket_dev); -static void gasket_disable_dev(struct gasket_dev *gasket_dev); - -static struct gasket_internal_desc *lookup_internal_desc( - struct pci_dev *pci_dev); - -static ssize_t gasket_sysfs_data_show( - struct device *device, struct device_attribute *attr, char *buf); - -static int gasket_mmap(struct file *filp, struct vm_area_struct *vma); -static int gasket_open(struct inode *inode, struct file *file); -static int gasket_release(struct inode *inode, struct file *file); -static long gasket_ioctl(struct file *filp, uint cmd, ulong arg); - -static int gasket_mm_vma_bar_offset( - const struct gasket_dev *gasket_dev, const struct vm_area_struct *vma, - ulong *bar_offset); -static bool gasket_mm_get_mapping_addrs( - const struct gasket_mappable_region *region, ulong bar_offset, - ulong requested_length, struct gasket_mappable_region *mappable_region, - ulong *virt_offset); -static enum do_map_region_status do_map_region( - const struct gasket_dev *gasket_dev, struct vm_area_struct *vma, - struct gasket_mappable_region *map_region); - -static int gasket_get_hw_status(struct gasket_dev *gasket_dev); - /* Global data definitions. */ /* Mutex - only for framework-wide data. Other data should be protected by * finer-grained locks. @@ -157,48 +102,6 @@ enum gasket_sysfs_attribute_type { ATTR_USER_MEM_RANGES }; -/* File operations for all Gasket devices. */ -static const struct file_operations gasket_file_ops = { - .owner = THIS_MODULE, - .llseek = no_llseek, - .mmap = gasket_mmap, - .open = gasket_open, - .release = gasket_release, - .unlocked_ioctl = gasket_ioctl, -}; - -/* These attributes apply to all Gasket driver instances. */ -static const struct gasket_sysfs_attribute gasket_sysfs_generic_attrs[] = { - GASKET_SYSFS_RO(bar_offsets, gasket_sysfs_data_show, ATTR_BAR_OFFSETS), - GASKET_SYSFS_RO(bar_sizes, gasket_sysfs_data_show, ATTR_BAR_SIZES), - GASKET_SYSFS_RO(driver_version, gasket_sysfs_data_show, - ATTR_DRIVER_VERSION), - GASKET_SYSFS_RO(framework_version, gasket_sysfs_data_show, - ATTR_FRAMEWORK_VERSION), - GASKET_SYSFS_RO(device_type, gasket_sysfs_data_show, ATTR_DEVICE_TYPE), - GASKET_SYSFS_RO(revision, gasket_sysfs_data_show, - ATTR_HARDWARE_REVISION), - GASKET_SYSFS_RO(pci_address, gasket_sysfs_data_show, ATTR_PCI_ADDRESS), - GASKET_SYSFS_RO(status, gasket_sysfs_data_show, ATTR_STATUS), - GASKET_SYSFS_RO(is_device_owned, gasket_sysfs_data_show, - ATTR_IS_DEVICE_OWNED), - GASKET_SYSFS_RO(device_owner, gasket_sysfs_data_show, - ATTR_DEVICE_OWNER), - GASKET_SYSFS_RO(write_open_count, gasket_sysfs_data_show, - ATTR_WRITE_OPEN_COUNT), - GASKET_SYSFS_RO(reset_count, gasket_sysfs_data_show, ATTR_RESET_COUNT), - GASKET_SYSFS_RO(user_mem_ranges, gasket_sysfs_data_show, - ATTR_USER_MEM_RANGES), - GASKET_END_OF_ATTR_ARRAY -}; - -MODULE_DESCRIPTION("Google Gasket driver framework"); -MODULE_VERSION(GASKET_FRAMEWORK_VERSION); -MODULE_LICEN
Re: [PATCH 01/13] staging: gasket: core: hold reference to pci_dev while used
On Mon, Jul 30, 2018 at 11:19 PM Dmitry Torokhov wrote: > > On Sun, Jul 29, 2018 at 12:37 PM Todd Poynor wrote: > > > > From: Todd Poynor > > > > Hold a reference on the struct pci_dev while a pointer to it is held in > > the gasket data structures. > > > > Signed-off-by: Todd Poynor > > --- > > drivers/staging/gasket/gasket_core.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/gasket/gasket_core.c > > b/drivers/staging/gasket/gasket_core.c > > index 2b484d067c38a..b832a4f529f27 100644 > > --- a/drivers/staging/gasket/gasket_core.c > > +++ b/drivers/staging/gasket/gasket_core.c > > @@ -488,6 +488,7 @@ static void gasket_free_dev(struct gasket_dev > > *gasket_dev) > > internal_desc->devs[gasket_dev->dev_idx] = NULL; > > mutex_unlock(&internal_desc->mutex); > > put_device(gasket_dev->dev); > > + pci_dev_put(gasket_dev->pci_dev); > > gasket_free_dev() is called only from driver PCI probe and remove > function. I can assure you that that pci_dev structure is not going > anywhere, there is no need to take this additional reference. WIll fix, thanks. > > Thanks, > Dmitry
Re: [PATCH 04/13] staging: gasket: core: allow root access based on user namespace
Hi Dmitry, On Mon, Jul 30, 2018 at 10:57 AM Dmitry Torokhov wrote: > > Hi Todd, > > On Sun, Jul 29, 2018 at 12:37 PM Todd Poynor wrote: > > @@ -1064,7 +1067,8 @@ static int gasket_open(struct inode *inode, struct > > file *filp) > > char task_name[TASK_COMM_LEN]; > > struct gasket_cdev_info *dev_info = > > container_of(inode->i_cdev, struct gasket_cdev_info, cdev); > > - int is_root = capable(CAP_SYS_ADMIN); > > + struct pid_namespace *pid_ns = task_active_pid_ns(current); > > + int is_root = ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN); > > ns_capable() returns bool, why did you make is_root an integer? Gaah, I forgot to change the type of the existing var. Will fix, thanks -- Todd > > Thanks, > Dmitry
[PATCH 01/13] staging: gasket: core: hold reference to pci_dev while used
From: Todd Poynor Hold a reference on the struct pci_dev while a pointer to it is held in the gasket data structures. Signed-off-by: Todd Poynor --- drivers/staging/gasket/gasket_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 2b484d067c38a..b832a4f529f27 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -488,6 +488,7 @@ static void gasket_free_dev(struct gasket_dev *gasket_dev) internal_desc->devs[gasket_dev->dev_idx] = NULL; mutex_unlock(&internal_desc->mutex); put_device(gasket_dev->dev); + pci_dev_put(gasket_dev->pci_dev); kfree(gasket_dev); } @@ -565,6 +566,7 @@ static int gasket_pci_probe( ret = gasket_alloc_dev(internal_desc, parent, &gasket_dev, kobj_name); if (ret) return ret; + gasket_dev->pci_dev = pci_dev_get(pci_dev); if (IS_ERR_OR_NULL(gasket_dev->dev_info.device)) { pr_err("Cannot create %s device %s [ret = %ld]\n", driver_desc->name, gasket_dev->dev_info.name, @@ -572,7 +574,6 @@ static int gasket_pci_probe( ret = -ENODEV; goto fail1; } - gasket_dev->pci_dev = pci_dev; ret = gasket_setup_pci(pci_dev, gasket_dev); if (ret) @@ -703,7 +704,6 @@ static int gasket_setup_pci( { int i, mapped_bars, ret; - gasket_dev->pci_dev = pci_dev; ret = pci_enable_device(pci_dev); if (ret) { dev_err(gasket_dev->dev, "cannot enable PCI device\n"); -- 2.18.0.345.g5c9ce644c3-goog