Attempting to clean-up W=1 kernel builds, which are currently
overwhelmingly riddled with niggly little warnings.
Lee Jones (20):
misc: pti: Repair kerneldoc formatting issues
misc: pti: Remove unparsable empty line in function header
misc: habanalabs: firmware_if: Add missing 'fw_name' and 'dst' entries
to function header
misc: habanalabs: pci: Fix a variety of kerneldoc issues
misc: habanalabs: irq: Repair kerneldoc formatting issues
misc: habanalabs: goya: Omit pointless check ensuring addr is >=0
misc: habanalabs: pci: Scrub documentation for non-present function
argument
misc: habanalabs: goya: goya_coresight: Remove set but unused variable
'val'
misc: habanalabs: gaudi: Remove ill placed asterisk from kerneldoc
header
misc: habanalabs: gaudi: gaudi_security: Repair incorrectly named
function arg
misc: enclosure: Fix some kerneldoc anomalies
misc: lattice-ecp3-config: Remove set but clearly unused variable
'ret'
misc: pch_phub: Provide descriptions for 'chip' argument
misc: pch_phub: Remove superfluous descriptions to non-existent args
'offset_address'
misc: enclosure: Update enclosure_remove_device() documentation to
match reality
misc: genwqe: card_base: Remove set but unused variable 'rc'
misc: genwqe: card_base: Do not pass unused argument 'fatal_err'
misc: genwqe: card_base: Whole host of kerneldoc fixes
misc: genwqe: card_dev: Whole host of kerneldoc fixes
misc: genwqe: card_utils: Whole a plethora of documentation issues
drivers/misc/enclosure.c | 8 ++--
drivers/misc/genwqe/card_base.c | 46 ++++++++++---------
drivers/misc/genwqe/card_dev.c | 24 +++++++---
drivers/misc/genwqe/card_utils.c | 30 ++++++++++--
drivers/misc/habanalabs/firmware_if.c | 3 ++
drivers/misc/habanalabs/gaudi/gaudi.c | 2 +-
.../misc/habanalabs/gaudi/gaudi_security.c | 3 +-
drivers/misc/habanalabs/goya/goya.c | 16 +++----
drivers/misc/habanalabs/goya/goya_coresight.c | 3 +-
drivers/misc/habanalabs/irq.c | 22 ++++-----
drivers/misc/habanalabs/pci.c | 13 ++++--
drivers/misc/lattice-ecp3-config.c | 19 ++++----
drivers/misc/pch_phub.c | 9 +++-
drivers/misc/pti.c | 11 ++---
14 files changed, 126 insertions(+), 83 deletions(-)
--
2.25.1
'fatal_err' is taken as an argument to a static function which is only
invoked once. During this invocation 'fatal_err' is not set. There
doesn't appear to be a good reason to keep it around.
Also fixes the following W=1 kernel build warning:
drivers/misc/genwqe/card_base.c:588: warning: Function parameter or member 'fatal_err' not described in 'genwqe_recover_card'
Cc: Michael Jung <[email protected]>
Cc: Michael Ruettger <[email protected]>
Cc: Frank Haverkamp <[email protected]>
Cc: Joerg-Stephan Vogt <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/genwqe/card_base.c | 18 +++---------------
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/misc/genwqe/card_base.c b/drivers/misc/genwqe/card_base.c
index bceebf49de2d5..809a6f46f6de3 100644
--- a/drivers/misc/genwqe/card_base.c
+++ b/drivers/misc/genwqe/card_base.c
@@ -569,30 +569,18 @@ static int genwqe_stop(struct genwqe_dev *cd)
/**
* genwqe_recover_card() - Try to recover the card if it is possible
- *
- * If fatal_err is set no register access is possible anymore. It is
- * likely that genwqe_start fails in that situation. Proper error
- * handling is required in this case.
+ * @cd: GenWQE device information
*
* genwqe_bus_reset() will cause the pci code to call genwqe_remove()
* and later genwqe_probe() for all virtual functions.
*/
-static int genwqe_recover_card(struct genwqe_dev *cd, int fatal_err)
+static int genwqe_recover_card(struct genwqe_dev *cd)
{
int rc;
struct pci_dev *pci_dev = cd->pci_dev;
genwqe_stop(cd);
- /*
- * Make sure chip is not reloaded to maintain FFDC. Write SLU
- * Reset Register, CPLDReset field to 0.
- */
- if (!fatal_err) {
- cd->softreset = 0x70ull;
- __genwqe_writeq(cd, IO_SLC_CFGREG_SOFTRESET, cd->softreset);
- }
-
rc = genwqe_bus_reset(cd);
if (rc != 0) {
dev_err(&pci_dev->dev,
@@ -958,7 +946,7 @@ static int genwqe_health_thread(void *data)
cd->card_state = GENWQE_CARD_FATAL_ERROR;
- rc = genwqe_recover_card(cd, 0);
+ rc = genwqe_recover_card(cd);
if (rc < 0) {
/* FIXME Card is unusable and needs unbind! */
goto fatal_error;
--
2.25.1
hl_pci_bars_map() has a miss-typed argument name in the function
description. hl_pci_elbi_write() was missing documented arguments.
The headers for functions hl_pci_bars_unmap(), hl_pci_elbi_write()
and hl_pci_reset_link_through_bridge() were written in kerneldoc
format, but lack the kerneldoc identifier '/**'. Let's promote
them so they can gain access to the checker.
These changes fix the following W=1 kernel build warnings:
drivers/misc/habanalabs/pci.c:27: warning: Function parameter or member 'name' not described in 'hl_pci_bars_map'
drivers/misc/habanalabs/pci.c:27: warning: Excess function parameter 'bar_name' description in 'hl_pci_bars_map'
drivers/misc/habanalabs/pci.c:147: warning: Function parameter or member 'addr' not described in 'hl_pci_iatu_write'
drivers/misc/habanalabs/pci.c:147: warning: Function parameter or member 'data' not described in 'hl_pci_iatu_write'
drivers/misc/habanalabs/pci.c:324: warning: Excess function parameter 'dma_mask' description in 'hl_pci_set_dma_mask'
Cc: Oded Gabbay <[email protected]>
Cc: Tomer Tayar <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/habanalabs/pci.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/misc/habanalabs/pci.c b/drivers/misc/habanalabs/pci.c
index 9f634ef6f5b37..24073e8eb8df9 100644
--- a/drivers/misc/habanalabs/pci.c
+++ b/drivers/misc/habanalabs/pci.c
@@ -15,7 +15,7 @@
/**
* hl_pci_bars_map() - Map PCI BARs.
* @hdev: Pointer to hl_device structure.
- * @bar_name: Array of BAR names.
+ * @name: Array of BAR names.
* @is_wc: Array with flag per BAR whether a write-combined mapping is needed.
*
* Request PCI regions and map them to kernel virtual addresses.
@@ -61,7 +61,7 @@ int hl_pci_bars_map(struct hl_device *hdev, const char * const name[3],
return rc;
}
-/*
+/**
* hl_pci_bars_unmap() - Unmap PCI BARS.
* @hdev: Pointer to hl_device structure.
*
@@ -80,9 +80,11 @@ static void hl_pci_bars_unmap(struct hl_device *hdev)
pci_release_regions(pdev);
}
-/*
+/**
* hl_pci_elbi_write() - Write through the ELBI interface.
* @hdev: Pointer to hl_device structure.
+ * @addr: Address to write to
+ * @data: Data to write
*
* Return: 0 on success, negative value for failure.
*/
@@ -140,6 +142,8 @@ static int hl_pci_elbi_write(struct hl_device *hdev, u64 addr, u32 data)
/**
* hl_pci_iatu_write() - iatu write routine.
* @hdev: Pointer to hl_device structure.
+ * @addr: Address to write to
+ * @data: Data to write
*
* Return: 0 on success, negative value for failure.
*/
@@ -161,7 +165,7 @@ int hl_pci_iatu_write(struct hl_device *hdev, u32 addr, u32 data)
return 0;
}
-/*
+/**
* hl_pci_reset_link_through_bridge() - Reset PCI link.
* @hdev: Pointer to hl_device structure.
*/
--
2.25.1
audi_pb_set_block()'s argument 'base' was incorrectly named 'block' in
its function header.
Fixes the following W=1 kernel build warning(s):
drivers/misc/habanalabs/gaudi/gaudi_security.c:454: warning: Function parameter or member 'base' not described in 'gaudi_pb_set_block'
drivers/misc/habanalabs/gaudi/gaudi_security.c:454: warning: Excess function parameter 'block' description in 'gaudi_pb_set_block'
Cc: Oded Gabbay <[email protected]>
Cc: Tomer Tayar <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/habanalabs/gaudi/gaudi_security.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/misc/habanalabs/gaudi/gaudi_security.c b/drivers/misc/habanalabs/gaudi/gaudi_security.c
index 6a351e31fa6af..abdd5ed8f2cf6 100644
--- a/drivers/misc/habanalabs/gaudi/gaudi_security.c
+++ b/drivers/misc/habanalabs/gaudi/gaudi_security.c
@@ -447,8 +447,7 @@ static u64 gaudi_rr_hbw_mask_high_ar_regs[GAUDI_NUMBER_OF_RR_REGS] = {
* gaudi_set_block_as_protected - set the given block as protected
*
* @hdev: pointer to hl_device structure
- * @block: block base address
- *
+ * @base: block base address
*/
static void gaudi_pb_set_block(struct hl_device *hdev, u64 base)
{
--
2.25.1
W=1 kernel builds report a lack of descriptions for various
function arguments. In reality they are documented, but the
formatting was not as expected '@.*:'. Instead, '-'s were
used as separators.
This change fixes the following warnings:
drivers/misc/pti.c:748: warning: Function parameter or member 'port' not described in 'pti_port_activate'
drivers/misc/pti.c:748: warning: Function parameter or member 'tty' not described in 'pti_port_activate'
drivers/misc/pti.c:765: warning: Function parameter or member 'port' not described in 'pti_port_shutdown'
drivers/misc/pti.c:793: warning: Function parameter or member 'pdev' not described in 'pti_pci_probe'
drivers/misc/pti.c:793: warning: Function parameter or member 'ent' not described in 'pti_pci_probe'
Cc: J Freyensee <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/pti.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/pti.c b/drivers/misc/pti.c
index 07e9da7918ebd..e19988766aa69 100644
--- a/drivers/misc/pti.c
+++ b/drivers/misc/pti.c
@@ -733,8 +733,8 @@ static struct console pti_console = {
* pti_port_activate()- Used to start/initialize any items upon
* first opening of tty_port().
*
- * @port- The tty port number of the PTI device.
- * @tty- The tty struct associated with this device.
+ * @port: The tty port number of the PTI device.
+ * @tty: The tty struct associated with this device.
*
* Returns:
* always returns 0
@@ -754,7 +754,7 @@ static int pti_port_activate(struct tty_port *port, struct tty_struct *tty)
* pti_port_shutdown()- Used to stop/shutdown any items upon the
* last tty port close.
*
- * @port- The tty port number of the PTI device.
+ * @port: The tty port number of the PTI device.
*
* Notes: The primary purpose of the PTI tty port 0 is to hook
* the syslog daemon to it; thus this port will be open for a
@@ -780,8 +780,8 @@ static const struct tty_port_operations tty_port_ops = {
* pti_pci_probe()- Used to detect pti on the pci bus and set
* things up in the driver.
*
- * @pdev- pci_dev struct values for pti.
- * @ent- pci_device_id struct for pti driver.
+ * @pdev: pci_dev struct values for pti.
+ * @ent: pci_device_id struct for pti driver.
*
* Returns:
* 0 for success
--
2.25.1
The kerneldoc tooling/parsers/validators get confused if non-
standard formatting is used. The first line after the kerneldoc
identifier '/**' must not be blank else the following warnings
will be issued:
drivers/misc/pti.c:902: warning: Cannot understand *
on line 902 - I thought it was a doc line
Cc: J Freyensee <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/pti.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/misc/pti.c b/drivers/misc/pti.c
index e19988766aa69..7236ae527b19e 100644
--- a/drivers/misc/pti.c
+++ b/drivers/misc/pti.c
@@ -898,7 +898,6 @@ static struct pci_driver pti_pci_driver = {
};
/**
- *
* pti_init()- Overall entry/init call to the pti driver.
* It starts the registration process with the kernel.
*
--
2.25.1
Probably a copy 'n' paste error, 'offset_address' has never been
part of the pch_phub_{read,write}_gbe_mac_addr() functions.
Squashes the following W=1 warnings:
drivers/misc/pch_phub.c:450: warning: Excess function parameter 'offset_address' description in 'pch_phub_read_gbe_mac_addr'
drivers/misc/pch_phub.c:462: warning: Excess function parameter 'offset_address' description in 'pch_phub_write_gbe_mac_addr'
Cc: Masayuki Ohtak <[email protected]>
Cc: Tomoya MORINAGA <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/pch_phub.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/misc/pch_phub.c b/drivers/misc/pch_phub.c
index 95ba82a8625b0..b27d826132915 100644
--- a/drivers/misc/pch_phub.c
+++ b/drivers/misc/pch_phub.c
@@ -448,7 +448,6 @@ static int pch_phub_gbe_serial_rom_conf_mp(struct pch_phub_reg *chip)
/**
* pch_phub_read_gbe_mac_addr() - Read Gigabit Ethernet MAC address
- * @offset_address: Gigabit Ethernet MAC address offset value.
* @chip: Pointer to the PHUB register structure
* @data: Buffer of the Gigabit Ethernet MAC address value.
*/
@@ -461,7 +460,6 @@ static void pch_phub_read_gbe_mac_addr(struct pch_phub_reg *chip, u8 *data)
/**
* pch_phub_write_gbe_mac_addr() - Write MAC address
- * @offset_address: Gigabit Ethernet MAC address offset value.
* @chip: Pointer to the PHUB register structure
* @data: Gigabit Ethernet MAC address value.
*/
--
2.25.1
For some reason (probably copy 'n' paste) kerneldoc descriptions
were missing for all instances of 'chip'. Providing them squashes
the following W=1 kernel build warnings:
drivers/misc/pch_phub.c:145: warning: Function parameter or member 'chip' not described in 'pch_phub_read_modify_write_reg'
drivers/misc/pch_phub.c:282: warning: Function parameter or member 'chip' not described in 'pch_phub_read_serial_rom'
drivers/misc/pch_phub.c:296: warning: Function parameter or member 'chip' not described in 'pch_phub_write_serial_rom'
drivers/misc/pch_phub.c:334: warning: Function parameter or member 'chip' not described in 'pch_phub_read_serial_rom_val'
drivers/misc/pch_phub.c:350: warning: Function parameter or member 'chip' not described in 'pch_phub_write_serial_rom_val'
drivers/misc/pch_phub.c:450: warning: Function parameter or member 'chip' not described in 'pch_phub_read_gbe_mac_addr'
drivers/misc/pch_phub.c:462: warning: Function parameter or member 'chip' not described in 'pch_phub_write_gbe_mac_addr'
Cc: Masayuki Ohtak <[email protected]>
Cc: Tomoya MORINAGA <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/pch_phub.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/misc/pch_phub.c b/drivers/misc/pch_phub.c
index 60828af7506a3..95ba82a8625b0 100644
--- a/drivers/misc/pch_phub.c
+++ b/drivers/misc/pch_phub.c
@@ -135,6 +135,7 @@ static DEFINE_MUTEX(pch_phub_mutex);
/**
* pch_phub_read_modify_write_reg() - Reading modifying and writing register
+ * @chip: Pointer to the PHUB register structure
* @reg_addr_offset: Register offset address value.
* @data: Writing value.
* @mask: Mask value.
@@ -274,6 +275,7 @@ static void pch_phub_restore_reg_conf(struct pci_dev *pdev)
/**
* pch_phub_read_serial_rom() - Reading Serial ROM
+ * @chip: Pointer to the PHUB register structure
* @offset_address: Serial ROM offset address to read.
* @data: Read buffer for specified Serial ROM value.
*/
@@ -288,6 +290,7 @@ static void pch_phub_read_serial_rom(struct pch_phub_reg *chip,
/**
* pch_phub_write_serial_rom() - Writing Serial ROM
+ * @chip: Pointer to the PHUB register structure
* @offset_address: Serial ROM offset address.
* @data: Serial ROM value to write.
*/
@@ -326,6 +329,7 @@ static int pch_phub_write_serial_rom(struct pch_phub_reg *chip,
/**
* pch_phub_read_serial_rom_val() - Read Serial ROM value
+ * @chip: Pointer to the PHUB register structure
* @offset_address: Serial ROM address offset value.
* @data: Serial ROM value to read.
*/
@@ -342,6 +346,7 @@ static void pch_phub_read_serial_rom_val(struct pch_phub_reg *chip,
/**
* pch_phub_write_serial_rom_val() - writing Serial ROM value
+ * @chip: Pointer to the PHUB register structure
* @offset_address: Serial ROM address offset value.
* @data: Serial ROM value.
*/
@@ -444,6 +449,7 @@ static int pch_phub_gbe_serial_rom_conf_mp(struct pch_phub_reg *chip)
/**
* pch_phub_read_gbe_mac_addr() - Read Gigabit Ethernet MAC address
* @offset_address: Gigabit Ethernet MAC address offset value.
+ * @chip: Pointer to the PHUB register structure
* @data: Buffer of the Gigabit Ethernet MAC address value.
*/
static void pch_phub_read_gbe_mac_addr(struct pch_phub_reg *chip, u8 *data)
@@ -456,6 +462,7 @@ static void pch_phub_read_gbe_mac_addr(struct pch_phub_reg *chip, u8 *data)
/**
* pch_phub_write_gbe_mac_addr() - Write MAC address
* @offset_address: Gigabit Ethernet MAC address offset value.
+ * @chip: Pointer to the PHUB register structure
* @data: Gigabit Ethernet MAC address value.
*/
static int pch_phub_write_gbe_mac_addr(struct pch_phub_reg *chip, u8 *data)
--
2.25.1
W=1 kernel builds report a lack of descriptions for various
function arguments. In reality they are documented, but the
formatting was not as expected '@.*:'. Instead, '-'s were
used as separators.
While we're here, the headers for functions various functions
were written in kerneldoc format, but lack the kerneldoc
identifier '/**'. Let's promote them so they can gain access
to the checker.
This change fixes the following W=1 warnings:
drivers/misc/habanalabs/irq.c:24: warning: Function parameter or member 'eq_work' not described in 'hl_eqe_work'
drivers/misc/habanalabs/irq.c:24: warning: Function parameter or member 'hdev' not described in 'hl_eqe_work'
drivers/misc/habanalabs/irq.c:24: warning: Function parameter or member 'eq_entry' not described in 'hl_eqe_work'
Cc: Oded Gabbay <[email protected]>
Cc: Tomer Tayar <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/habanalabs/irq.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/misc/habanalabs/irq.c b/drivers/misc/habanalabs/irq.c
index 4e77a73857793..6981d67153b11 100644
--- a/drivers/misc/habanalabs/irq.c
+++ b/drivers/misc/habanalabs/irq.c
@@ -13,9 +13,9 @@
* struct hl_eqe_work - This structure is used to schedule work of EQ
* entry and armcp_reset event
*
- * @eq_work - workqueue object to run when EQ entry is received
- * @hdev - pointer to device structure
- * @eq_entry - copy of the EQ entry
+ * @eq_work: workqueue object to run when EQ entry is received
+ * @hdev: pointer to device structure
+ * @eq_entry: copy of the EQ entry
*/
struct hl_eqe_work {
struct work_struct eq_work;
@@ -23,7 +23,7 @@ struct hl_eqe_work {
struct hl_eq_entry eq_entry;
};
-/*
+/**
* hl_cq_inc_ptr - increment ci or pi of cq
*
* @ptr: the current ci or pi value of the completion queue
@@ -39,7 +39,7 @@ inline u32 hl_cq_inc_ptr(u32 ptr)
return ptr;
}
-/*
+/**
* hl_eq_inc_ptr - increment ci of eq
*
* @ptr: the current ci value of the event queue
@@ -66,7 +66,7 @@ static void irq_handle_eqe(struct work_struct *work)
kfree(eqe_work);
}
-/*
+/**
* hl_irq_handler_cq - irq handler for completion queue
*
* @irq: irq number
@@ -142,7 +142,7 @@ irqreturn_t hl_irq_handler_cq(int irq, void *arg)
return IRQ_HANDLED;
}
-/*
+/**
* hl_irq_handler_eq - irq handler for event queue
*
* @irq: irq number
@@ -206,7 +206,7 @@ irqreturn_t hl_irq_handler_eq(int irq, void *arg)
return IRQ_HANDLED;
}
-/*
+/**
* hl_cq_init - main initialization function for an cq object
*
* @hdev: pointer to device structure
@@ -238,7 +238,7 @@ int hl_cq_init(struct hl_device *hdev, struct hl_cq *q, u32 hw_queue_id)
return 0;
}
-/*
+/**
* hl_cq_fini - destroy completion queue
*
* @hdev: pointer to device structure
@@ -269,7 +269,7 @@ void hl_cq_reset(struct hl_device *hdev, struct hl_cq *q)
memset((void *) (uintptr_t) q->kernel_address, 0, HL_CQ_SIZE_IN_BYTES);
}
-/*
+/**
* hl_eq_init - main initialization function for an event queue object
*
* @hdev: pointer to device structure
@@ -297,7 +297,7 @@ int hl_eq_init(struct hl_device *hdev, struct hl_eq *q)
return 0;
}
-/*
+/**
* hl_eq_fini - destroy event queue
*
* @hdev: pointer to device structure
--
2.25.1
Firstly some missing function argument documentation, then some
whch are present, but are incorrectly named.
Fixes the following W=1 kernel build warnings:
drivers/misc/enclosure.c:115: warning: Function parameter or member 'name' not described in 'enclosure_register'
drivers/misc/enclosure.c:115: warning: Function parameter or member 'cb' not described in 'enclosure_register'
drivers/misc/enclosure.c:283: warning: Function parameter or member 'number' not described in 'enclosure_component_alloc'
drivers/misc/enclosure.c:283: warning: Excess function parameter 'num' description in 'enclosure_component_alloc'
drivers/misc/enclosure.c:363: warning: Function parameter or member 'component' not described in 'enclosure_add_device'
drivers/misc/enclosure.c:363: warning: Excess function parameter 'num' description in 'enclosure_add_device'
drivers/misc/enclosure.c:398: warning: Function parameter or member 'dev' not described in 'enclosure_remove_device'
drivers/misc/enclosure.c:398: warning: Excess function parameter 'num' description in 'enclosure_remove_device'
Cc: James Bottomley <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/enclosure.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 3c2d405bc79b9..e8eba52750b34 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -103,7 +103,9 @@ EXPORT_SYMBOL_GPL(enclosure_for_each_device);
* enclosure_register - register device as an enclosure
*
* @dev: device containing the enclosure
+ * @name: chosen device name
* @components: number of components in the enclosure
+ * @cb: platform call-backs
*
* This sets up the device for being an enclosure. Note that @dev does
* not have to be a dedicated enclosure device. It may be some other type
@@ -266,7 +268,7 @@ static const struct attribute_group *enclosure_component_groups[];
/**
* enclosure_component_alloc - prepare a new enclosure component
* @edev: the enclosure to add the component
- * @num: the device number
+ * @number: the device number
* @type: the type of component being added
* @name: an optional name to appear in sysfs (leave NULL if none)
*
@@ -347,7 +349,7 @@ EXPORT_SYMBOL_GPL(enclosure_component_register);
/**
* enclosure_add_device - add a device as being part of an enclosure
* @edev: the enclosure device being added to.
- * @num: the number of the component
+ * @component: the number of the component
* @dev: the device being added
*
* Declares a real device to reside in slot (or identifier) @num of an
--
2.25.1
No attempt to check the return value of RREG32() has been made
since the call was introduced a year ago.
Fixes W=1 kernel build warning:
drivers/misc/habanalabs/goya/goya_coresight.c: In function ‘goya_debug_coresight’:
drivers/misc/habanalabs/goya/goya_coresight.c:643:6: warning: variable ‘val’ set but not used [-Wunused-but-set-variable]
643 | u32 val;
| ^~~
Cc: Oded Gabbay <[email protected]>
Cc: Tomer Tayar <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/habanalabs/goya/goya_coresight.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/misc/habanalabs/goya/goya_coresight.c b/drivers/misc/habanalabs/goya/goya_coresight.c
index 1258724ea5106..aa51fc71f0a1f 100644
--- a/drivers/misc/habanalabs/goya/goya_coresight.c
+++ b/drivers/misc/habanalabs/goya/goya_coresight.c
@@ -640,7 +640,6 @@ static int goya_config_spmu(struct hl_device *hdev,
int goya_debug_coresight(struct hl_device *hdev, void *data)
{
struct hl_debug_params *params = data;
- u32 val;
int rc = 0;
switch (params->op) {
@@ -672,7 +671,7 @@ int goya_debug_coresight(struct hl_device *hdev, void *data)
}
/* Perform read from the device to flush all configuration */
- val = RREG32(mmPCIE_DBI_DEVICE_ID_VENDOR_ID_REG);
+ RREG32(mmPCIE_DBI_DEVICE_ID_VENDOR_ID_REG);
return rc;
}
--
2.25.1
It's odd for the return value to be assigned to a variable so many
times, but never actually checked, but this has been the case since
the driver's inception in 2012. If it hasn't caused any issues by
now, it's probably unlikely to. Let's take it out, at least until
someone finds a reason to start using it.
Fixes the following W=1 kernel build warning:
drivers/misc/lattice-ecp3-config.c: In function ‘firmware_load’:
drivers/misc/lattice-ecp3-config.c:70:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
70 | int ret;
| ^~~
Cc: Stefan Roese <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/lattice-ecp3-config.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/misc/lattice-ecp3-config.c b/drivers/misc/lattice-ecp3-config.c
index 884485c3f7232..5eaf74447ca1e 100644
--- a/drivers/misc/lattice-ecp3-config.c
+++ b/drivers/misc/lattice-ecp3-config.c
@@ -67,7 +67,6 @@ static void firmware_load(const struct firmware *fw, void *context)
struct spi_device *spi = (struct spi_device *)context;
struct fpga_data *data = spi_get_drvdata(spi);
u8 *buffer;
- int ret;
u8 txbuf[8];
u8 rxbuf[8];
int rx_len = 8;
@@ -92,7 +91,7 @@ static void firmware_load(const struct firmware *fw, void *context)
/* Trying to speak with the FPGA via SPI... */
txbuf[0] = FPGA_CMD_READ_ID;
- ret = spi_write_then_read(spi, txbuf, 8, rxbuf, rx_len);
+ spi_write_then_read(spi, txbuf, 8, rxbuf, rx_len);
jedec_id = get_unaligned_be32(&rxbuf[4]);
dev_dbg(&spi->dev, "FPGA JTAG ID=%08x\n", jedec_id);
@@ -110,7 +109,7 @@ static void firmware_load(const struct firmware *fw, void *context)
dev_info(&spi->dev, "FPGA %s detected\n", ecp3_dev[i].name);
txbuf[0] = FPGA_CMD_READ_STATUS;
- ret = spi_write_then_read(spi, txbuf, 8, rxbuf, rx_len);
+ spi_write_then_read(spi, txbuf, 8, rxbuf, rx_len);
status = get_unaligned_be32(&rxbuf[4]);
dev_dbg(&spi->dev, "FPGA Status=%08x\n", status);
@@ -130,20 +129,20 @@ static void firmware_load(const struct firmware *fw, void *context)
memcpy(buffer + 4, fw->data, fw->size);
txbuf[0] = FPGA_CMD_REFRESH;
- ret = spi_write(spi, txbuf, 4);
+ spi_write(spi, txbuf, 4);
txbuf[0] = FPGA_CMD_WRITE_EN;
- ret = spi_write(spi, txbuf, 4);
+ spi_write(spi, txbuf, 4);
txbuf[0] = FPGA_CMD_CLEAR;
- ret = spi_write(spi, txbuf, 4);
+ spi_write(spi, txbuf, 4);
/*
* Wait for FPGA memory to become cleared
*/
for (i = 0; i < FPGA_CLEAR_LOOP_COUNT; i++) {
txbuf[0] = FPGA_CMD_READ_STATUS;
- ret = spi_write_then_read(spi, txbuf, 8, rxbuf, rx_len);
+ spi_write_then_read(spi, txbuf, 8, rxbuf, rx_len);
status = get_unaligned_be32(&rxbuf[4]);
if (status == FPGA_STATUS_CLEARED)
break;
@@ -160,13 +159,13 @@ static void firmware_load(const struct firmware *fw, void *context)
}
dev_info(&spi->dev, "Configuring the FPGA...\n");
- ret = spi_write(spi, buffer, fw->size + 8);
+ spi_write(spi, buffer, fw->size + 8);
txbuf[0] = FPGA_CMD_WRITE_DIS;
- ret = spi_write(spi, txbuf, 4);
+ spi_write(spi, txbuf, 4);
txbuf[0] = FPGA_CMD_READ_STATUS;
- ret = spi_write_then_read(spi, txbuf, 8, rxbuf, rx_len);
+ spi_write_then_read(spi, txbuf, 8, rxbuf, rx_len);
status = get_unaligned_be32(&rxbuf[4]);
dev_dbg(&spi->dev, "FPGA Status=%08x\n", status);
--
2.25.1
enclosure_remove_device() hasn't taken an 'int component for over a decade.
Instead use kerneldoc to describe the 'struct device' actually passed in.
Fixes the following W=1 kernel build warning(s):
drivers/misc/enclosure.c:400: warning: Function parameter or member 'dev' not described in 'enclosure_remove_device'
drivers/misc/enclosure.c:400: warning: Excess function parameter 'num' description in 'enclosure_remove_device'
Cc: James Bottomley <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/enclosure.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index e8eba52750b34..f950d0155876a 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -391,7 +391,7 @@ EXPORT_SYMBOL_GPL(enclosure_add_device);
/**
* enclosure_remove_device - remove a device from an enclosure
* @edev: the enclosure device
- * @num: the number of the component to remove
+ * @dev: device to remove/put
*
* Returns zero on success or an error.
*
--
2.25.1
Seeing as 'addr' is unsigned, it would be impossible for the assigned
value to be anything other than zero or positive.
Squashes the following W=1 warnings:
drivers/misc/habanalabs/goya/goya.c: In function ‘goya_debugfs_read32’:
drivers/misc/habanalabs/goya/goya.c:3945:19: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
3945 | } else if ((addr >= DRAM_PHYS_BASE) &&
| ^~
drivers/misc/habanalabs/goya/goya.c: In function ‘goya_debugfs_write32’:
drivers/misc/habanalabs/goya/goya.c:4002:19: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
4002 | } else if ((addr >= DRAM_PHYS_BASE) &&
| ^~
drivers/misc/habanalabs/goya/goya.c: In function ‘goya_debugfs_read64’:
drivers/misc/habanalabs/goya/goya.c:4047:19: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
4047 | } else if ((addr >= DRAM_PHYS_BASE) &&
| ^~
drivers/misc/habanalabs/goya/goya.c: In function ‘goya_debugfs_write64’:
drivers/misc/habanalabs/goya/goya.c:4091:19: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
4091 | } else if ((addr >= DRAM_PHYS_BASE) &&
| ^~
drivers/misc/habanalabs/pci.c:328: warning: Excess function parameter 'dma_mask' description in 'hl_pci_set_dma_mask'
drivers/misc/habanalabs/goya/goya_coresight.c: In function ‘goya_debug_coresight’:
drivers/misc/habanalabs/goya/goya_coresight.c:643:6: warning: variable ‘val’ set but not used [-Wunused-but-set-variable]
643 | u32 val;
| ^~~
Cc: Oded Gabbay <[email protected]>
Cc: Tomer Tayar <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/habanalabs/goya/goya.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/misc/habanalabs/goya/goya.c b/drivers/misc/habanalabs/goya/goya.c
index 0d2952bb58dfb..a4a20e27ed3b4 100644
--- a/drivers/misc/habanalabs/goya/goya.c
+++ b/drivers/misc/habanalabs/goya/goya.c
@@ -3942,8 +3942,7 @@ static int goya_debugfs_read32(struct hl_device *hdev, u64 addr, u32 *val)
*val = readl(hdev->pcie_bar[SRAM_CFG_BAR_ID] +
(addr - SRAM_BASE_ADDR));
- } else if ((addr >= DRAM_PHYS_BASE) &&
- (addr < DRAM_PHYS_BASE + hdev->asic_prop.dram_size)) {
+ } else if (addr < DRAM_PHYS_BASE + hdev->asic_prop.dram_size) {
u64 bar_base_addr = DRAM_PHYS_BASE +
(addr & ~(prop->dram_pci_bar_size - 0x1ull));
@@ -3999,8 +3998,7 @@ static int goya_debugfs_write32(struct hl_device *hdev, u64 addr, u32 val)
writel(val, hdev->pcie_bar[SRAM_CFG_BAR_ID] +
(addr - SRAM_BASE_ADDR));
- } else if ((addr >= DRAM_PHYS_BASE) &&
- (addr < DRAM_PHYS_BASE + hdev->asic_prop.dram_size)) {
+ } else if (addr < DRAM_PHYS_BASE + hdev->asic_prop.dram_size) {
u64 bar_base_addr = DRAM_PHYS_BASE +
(addr & ~(prop->dram_pci_bar_size - 0x1ull));
@@ -4044,9 +4042,8 @@ static int goya_debugfs_read64(struct hl_device *hdev, u64 addr, u64 *val)
*val = readq(hdev->pcie_bar[SRAM_CFG_BAR_ID] +
(addr - SRAM_BASE_ADDR));
- } else if ((addr >= DRAM_PHYS_BASE) &&
- (addr <=
- DRAM_PHYS_BASE + hdev->asic_prop.dram_size - sizeof(u64))) {
+ } else if (addr <=
+ DRAM_PHYS_BASE + hdev->asic_prop.dram_size - sizeof(u64)) {
u64 bar_base_addr = DRAM_PHYS_BASE +
(addr & ~(prop->dram_pci_bar_size - 0x1ull));
@@ -4088,9 +4085,8 @@ static int goya_debugfs_write64(struct hl_device *hdev, u64 addr, u64 val)
writeq(val, hdev->pcie_bar[SRAM_CFG_BAR_ID] +
(addr - SRAM_BASE_ADDR));
- } else if ((addr >= DRAM_PHYS_BASE) &&
- (addr <=
- DRAM_PHYS_BASE + hdev->asic_prop.dram_size - sizeof(u64))) {
+ } else if (addr <=
+ DRAM_PHYS_BASE + hdev->asic_prop.dram_size - sizeof(u64)) {
u64 bar_base_addr = DRAM_PHYS_BASE +
(addr & ~(prop->dram_pci_bar_size - 0x1ull));
--
2.25.1
W=1 kernel builds report a lack of description of gaudi_set_asic_funcs()'s
'hdev' argument. In reality it is documented, but the formatting
was not as expected '@.*:'. Instead, there was a misplaced asterisk
which was confusing the kerneldoc validator.
Squashes the following W=1 warning:
drivers/misc/habanalabs/gaudi/gaudi.c:6746: warning: Function parameter or member 'hdev' not described in 'gaudi_set_asic_funcs'
Cc: Oded Gabbay <[email protected]>
Cc: Tomer Tayar <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/habanalabs/gaudi/gaudi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/habanalabs/gaudi/gaudi.c b/drivers/misc/habanalabs/gaudi/gaudi.c
index 61f88e9884ce9..bdb9c0080c464 100644
--- a/drivers/misc/habanalabs/gaudi/gaudi.c
+++ b/drivers/misc/habanalabs/gaudi/gaudi.c
@@ -6739,7 +6739,7 @@ static const struct hl_asic_funcs gaudi_funcs = {
/**
* gaudi_set_asic_funcs - set GAUDI function pointers
*
- * @*hdev: pointer to hl_device structure
+ * @hdev: pointer to hl_device structure
*
*/
void gaudi_set_asic_funcs(struct hl_device *hdev)
--
2.25.1
Variable 'rc' hasn't been checked since the driver's inception
in 2013. If it hasn't caused any issues since then, it's unlikely
to in the future. Let's take it out for now.
Fixes the following W=1 kernel build warning(s):
drivers/misc/genwqe/card_base.c: In function ‘genwqe_health_check_stop’:
/home/lee/projects/linux/kernel/drivers/misc/genwqe/card_base.c:1046:6: warning: variable ‘rc’ set but not used [-Wunused-but-set-variable]
1046 | int rc;
| ^~
Cc: Michael Jung <[email protected]>
Cc: Michael Ruettger <[email protected]>
Cc: Frank Haverkamp <[email protected]>
Cc: Joerg-Stephan Vogt <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/genwqe/card_base.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/misc/genwqe/card_base.c b/drivers/misc/genwqe/card_base.c
index 1dc6c7c5cbce9..bceebf49de2d5 100644
--- a/drivers/misc/genwqe/card_base.c
+++ b/drivers/misc/genwqe/card_base.c
@@ -1043,12 +1043,10 @@ static int genwqe_health_thread_running(struct genwqe_dev *cd)
static int genwqe_health_check_stop(struct genwqe_dev *cd)
{
- int rc;
-
if (!genwqe_health_thread_running(cd))
return -EIO;
- rc = kthread_stop(cd->health_thread);
+ kthread_stop(cd->health_thread);
cd->health_thread = NULL;
return 0;
}
--
2.25.1
Including; add missing documentation for function arguments, re-ordering
of #defines i.e. not placed between kerneldoc headers and the functions
they are documenting, demotion of file header/comment from kerneldoc
format and removal of documentation for non-existent args.
Fixes the following W=1 kernel build warnings:
drivers/misc/genwqe/card_dev.c:33: warning: Function parameter or member 'cd' not described in 'genwqe_open_files'
drivers/misc/genwqe/card_dev.c:98: warning: Function parameter or member 'virt_addr' not described in 'genwqe_search_pin'
drivers/misc/genwqe/card_dev.c:98: warning: Excess function parameter 'dma_addr' description in 'genwqe_search_pin'
drivers/misc/genwqe/card_dev.c:154: warning: Function parameter or member 'virt_addr' not described in '__genwqe_search_mapping'
drivers/misc/genwqe/card_dev.c:256: warning: Function parameter or member 'cd' not described in 'genwqe_kill_fasync'
drivers/misc/genwqe/card_dev.c:256: warning: Function parameter or member 'sig' not described in 'genwqe_kill_fasync'
drivers/misc/genwqe/card_dev.c:387: warning: Function parameter or member 'vma' not described in 'genwqe_vma_close'
drivers/misc/genwqe/card_dev.c:430: warning: Function parameter or member 'filp' not described in 'genwqe_mmap'
drivers/misc/genwqe/card_dev.c:430: warning: Function parameter or member 'vma' not described in 'genwqe_mmap'
drivers/misc/genwqe/card_dev.c:495: warning: Excess function parameter 'cd' description in 'FLASH_BLOCK'
drivers/misc/genwqe/card_dev.c:495: warning: Excess function parameter 'load' description in 'FLASH_BLOCK'
drivers/misc/genwqe/card_dev.c:827: warning: Function parameter or member 'cfile' not described in 'ddcb_cmd_cleanup'
drivers/misc/genwqe/card_dev.c:827: warning: Function parameter or member 'req' not described in 'ddcb_cmd_cleanup'
drivers/misc/genwqe/card_dev.c:854: warning: Function parameter or member 'cfile' not described in 'ddcb_cmd_fixups'
drivers/misc/genwqe/card_dev.c:854: warning: Function parameter or member 'req' not described in 'ddcb_cmd_fixups'
drivers/misc/genwqe/card_dev.c:984: warning: Function parameter or member 'cfile' not described in 'genwqe_execute_ddcb'
drivers/misc/genwqe/card_dev.c:984: warning: Function parameter or member 'cmd' not described in 'genwqe_execute_ddcb'
drivers/misc/genwqe/card_dev.c:1350: warning: Function parameter or member 'cd' not described in 'genwqe_device_remove'
Cc: Michael Jung <[email protected]>
Cc: Michael Ruettger <[email protected]>
Cc: Frank Haverkamp <[email protected]>
Cc: Joerg-Stephan Vogt <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/genwqe/card_dev.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/misc/genwqe/card_dev.c b/drivers/misc/genwqe/card_dev.c
index 040a0bda31254..55fc5b80e649f 100644
--- a/drivers/misc/genwqe/card_dev.c
+++ b/drivers/misc/genwqe/card_dev.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* IBM Accelerator Family 'GenWQE'
*
* (C) Copyright IBM Corp. 2013
@@ -87,7 +87,7 @@ static int genwqe_del_pin(struct genwqe_file *cfile, struct dma_mapping *m)
* @cfile: Descriptor of opened file
* @u_addr: User virtual address
* @size: Size of buffer
- * @dma_addr: DMA address to be updated
+ * @virt_addr: Virtual address to be updated
*
* Return: Pointer to the corresponding mapping NULL if not found
*/
@@ -144,6 +144,7 @@ static void __genwqe_del_mapping(struct genwqe_file *cfile,
* @u_addr: user virtual address
* @size: size of buffer
* @dma_addr: DMA address to be updated
+ * @virt_addr: Virtual address to be updated
* Return: Pointer to the corresponding mapping NULL if not found
*/
static struct dma_mapping *__genwqe_search_mapping(struct genwqe_file *cfile,
@@ -249,6 +250,8 @@ static void genwqe_remove_pinnings(struct genwqe_file *cfile)
/**
* genwqe_kill_fasync() - Send signal to all processes with open GenWQE files
+ * @cd: GenWQE device information
+ * @sig: Signal to send out
*
* E.g. genwqe_send_signal(cd, SIGIO);
*/
@@ -380,6 +383,7 @@ static void genwqe_vma_open(struct vm_area_struct *vma)
/**
* genwqe_vma_close() - Called each time when vma is unmapped
+ * @vma: VMA area to close
*
* Free memory which got allocated by GenWQE mmap().
*/
@@ -416,6 +420,8 @@ static const struct vm_operations_struct genwqe_vma_ops = {
/**
* genwqe_mmap() - Provide contignous buffers to userspace
+ * @filp: File pointer (unused)
+ * @vma: VMA area to map
*
* We use mmap() to allocate contignous buffers used for DMA
* transfers. After the buffer is allocated we remap it to user-space
@@ -484,16 +490,15 @@ static int genwqe_mmap(struct file *filp, struct vm_area_struct *vma)
return rc;
}
+#define FLASH_BLOCK 0x40000 /* we use 256k blocks */
+
/**
* do_flash_update() - Excute flash update (write image or CVPD)
- * @cd: genwqe device
+ * @cfile: Descriptor of opened file
* @load: details about image load
*
* Return: 0 if successful
*/
-
-#define FLASH_BLOCK 0x40000 /* we use 256k blocks */
-
static int do_flash_update(struct genwqe_file *cfile,
struct genwqe_bitstream *load)
{
@@ -820,6 +825,8 @@ static int genwqe_unpin_mem(struct genwqe_file *cfile, struct genwqe_mem *m)
/**
* ddcb_cmd_cleanup() - Remove dynamically created fixup entries
+ * @cfile: Descriptor of opened file
+ * @req: DDCB work request
*
* Only if there are any. Pinnings are not removed.
*/
@@ -844,6 +851,8 @@ static int ddcb_cmd_cleanup(struct genwqe_file *cfile, struct ddcb_requ *req)
/**
* ddcb_cmd_fixups() - Establish DMA fixups/sglists for user memory references
+ * @cfile: Descriptor of opened file
+ * @req: DDCB work request
*
* Before the DDCB gets executed we need to handle the fixups. We
* replace the user-space addresses with DMA addresses or do
@@ -974,6 +983,8 @@ static int ddcb_cmd_fixups(struct genwqe_file *cfile, struct ddcb_requ *req)
/**
* genwqe_execute_ddcb() - Execute DDCB using userspace address fixups
+ * @cfile: Descriptor of opened file
+ * @cmd: Command identifier (passed from user)
*
* The code will build up the translation tables or lookup the
* contignous memory allocation table to find the right translations
@@ -1339,6 +1350,7 @@ static int genwqe_inform_and_stop_processes(struct genwqe_dev *cd)
/**
* genwqe_device_remove() - Remove genwqe's char device
+ * @cd: GenWQE device information
*
* This function must be called after the client devices are removed
* because it will free the major/minor number range for the genwqe
--
2.25.1
From missing documentation for function arguments, to promotion
obvious kerneldoc headers and incorrectly named arguments.
Fixes the following W=1 warnings:
drivers/misc/genwqe/card_base.c:175: warning: Function parameter or member 'cd' not described in 'genwqe_bus_reset'
drivers/misc/genwqe/card_base.c:272: warning: Function parameter or member 'cd' not described in 'genwqe_recovery_on_fatal_gfir_required'
drivers/misc/genwqe/card_base.c:293: warning: Function parameter or member 'cd' not described in 'genwqe_T_psec'
drivers/misc/genwqe/card_base.c:314: warning: Function parameter or member 'cd' not described in 'genwqe_setup_pf_jtimer'
drivers/misc/genwqe/card_base.c:334: warning: Function parameter or member 'cd' not described in 'genwqe_setup_vf_jtimer'
drivers/misc/genwqe/card_base.c:557: warning: Function parameter or member 'cd' not described in 'genwqe_stop'
drivers/misc/genwqe/card_base.c:617: warning: Function parameter or member 'cd' not described in 'genwqe_fir_checking'
drivers/misc/genwqe/card_base.c:760: warning: Function parameter or member 'pci_dev' not described in 'genwqe_pci_fundamental_reset'
drivers/misc/genwqe/card_base.c:889: warning: Function parameter or member 'data' not described in 'genwqe_health_thread'
drivers/misc/genwqe/card_base.c:1046: warning: Function parameter or member 'cd' not described in 'genwqe_pci_setup'
drivers/misc/genwqe/card_base.c:1131: warning: Function parameter or member 'cd' not described in 'genwqe_pci_remove'
drivers/misc/genwqe/card_base.c:1151: warning: Function parameter or member 'pci_dev' not described in 'genwqe_probe'
drivers/misc/genwqe/card_base.c:1151: warning: Function parameter or member 'id' not described in 'genwqe_probe'
drivers/misc/genwqe/card_base.c:1151: warning: Excess function parameter 'pdev' description in 'genwqe_probe'
drivers/misc/genwqe/card_base.c:1207: warning: Function parameter or member 'pci_dev' not described in 'genwqe_remove'
drivers/misc/genwqe/card_base.c:1336: warning: Function parameter or member 'dev' not described in 'genwqe_devnode'
drivers/misc/genwqe/card_base.c:1336: warning: Function parameter or member 'mode' not described in 'genwqe_devnode'
Cc: Michael Jung <[email protected]>
Cc: Michael Ruettger <[email protected]>
Cc: Frank Haverkamp <[email protected]>
Cc: Joerg-Stephan Vogt <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/misc/genwqe/card_base.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/genwqe/card_base.c b/drivers/misc/genwqe/card_base.c
index 809a6f46f6de3..93d2ed91c85b2 100644
--- a/drivers/misc/genwqe/card_base.c
+++ b/drivers/misc/genwqe/card_base.c
@@ -165,6 +165,7 @@ static void genwqe_dev_free(struct genwqe_dev *cd)
/**
* genwqe_bus_reset() - Card recovery
+ * @cd: GenWQE device information
*
* pci_reset_function() will recover the device and ensure that the
* registers are accessible again when it completes with success. If
@@ -262,6 +263,7 @@ static void genwqe_tweak_hardware(struct genwqe_dev *cd)
/**
* genwqe_recovery_on_fatal_gfir_required() - Version depended actions
+ * @cd: GenWQE device information
*
* Bitstreams older than 2013-02-17 have a bug where fatal GFIRs must
* be ignored. This is e.g. true for the bitstream we gave to the card
@@ -280,6 +282,7 @@ int genwqe_flash_readback_fails(struct genwqe_dev *cd)
/**
* genwqe_T_psec() - Calculate PF/VF timeout register content
+ * @cd: GenWQE device information
*
* Note: From a design perspective it turned out to be a bad idea to
* use codes here to specifiy the frequency/speed values. An old
@@ -303,6 +306,7 @@ static int genwqe_T_psec(struct genwqe_dev *cd)
/**
* genwqe_setup_pf_jtimer() - Setup PF hardware timeouts for DDCB execution
+ * @cd: GenWQE device information
*
* Do this _after_ card_reset() is called. Otherwise the values will
* vanish. The settings need to be done when the queues are inactive.
@@ -329,6 +333,7 @@ static bool genwqe_setup_pf_jtimer(struct genwqe_dev *cd)
/**
* genwqe_setup_vf_jtimer() - Setup VF hardware timeouts for DDCB execution
+ * @cd: GenWQE device information
*/
static bool genwqe_setup_vf_jtimer(struct genwqe_dev *cd)
{
@@ -543,6 +548,7 @@ static int genwqe_start(struct genwqe_dev *cd)
/**
* genwqe_stop() - Stop card operation
+ * @cd: GenWQE device information
*
* Recovery notes:
* As long as genwqe_thread runs we might access registers during
@@ -606,6 +612,7 @@ static int genwqe_health_check_cond(struct genwqe_dev *cd, u64 *gfir)
/**
* genwqe_fir_checking() - Check the fault isolation registers of the card
+ * @cd: GenWQE device information
*
* If this code works ok, can be tried out with help of the genwqe_poke tool:
* sudo ./tools/genwqe_poke 0x8 0xfefefefefef
@@ -750,6 +757,7 @@ static u64 genwqe_fir_checking(struct genwqe_dev *cd)
/**
* genwqe_pci_fundamental_reset() - trigger a PCIe fundamental reset on the slot
+ * @pci_dev: PCI device information struct
*
* Note: pci_set_pcie_reset_state() is not implemented on all archs, so this
* reset method will not work in all cases.
@@ -814,8 +822,9 @@ static int genwqe_platform_recovery(struct genwqe_dev *cd)
return rc;
}
-/*
+/**
* genwqe_reload_bistream() - reload card bitstream
+ * @cd: GenWQE device information
*
* Set the appropriate register and call fundamental reset to reaload the card
* bitstream.
@@ -868,6 +877,7 @@ static int genwqe_reload_bistream(struct genwqe_dev *cd)
/**
* genwqe_health_thread() - Health checking thread
+ * @data: GenWQE device information
*
* This thread is only started for the PF of the card.
*
@@ -1041,6 +1051,7 @@ static int genwqe_health_check_stop(struct genwqe_dev *cd)
/**
* genwqe_pci_setup() - Allocate PCIe related resources for our card
+ * @cd: GenWQE device information
*/
static int genwqe_pci_setup(struct genwqe_dev *cd)
{
@@ -1126,6 +1137,7 @@ static int genwqe_pci_setup(struct genwqe_dev *cd)
/**
* genwqe_pci_remove() - Free PCIe related resources for our card
+ * @cd: GenWQE device information
*/
static void genwqe_pci_remove(struct genwqe_dev *cd)
{
@@ -1140,7 +1152,8 @@ static void genwqe_pci_remove(struct genwqe_dev *cd)
/**
* genwqe_probe() - Device initialization
- * @pdev: PCI device information struct
+ * @pci_dev: PCI device information struct
+ * @id: PCI device ID
*
* Callable for multiple cards. This function is called on bind.
*
@@ -1200,6 +1213,7 @@ static int genwqe_probe(struct pci_dev *pci_dev,
/**
* genwqe_remove() - Called when device is removed (hot-plugable)
+ * @pci_dev: PCI device information struct
*
* Or when driver is unloaded respecitively when unbind is done.
*/
@@ -1219,8 +1233,10 @@ static void genwqe_remove(struct pci_dev *pci_dev)
genwqe_dev_free(cd);
}
-/*
+/**
* genwqe_err_error_detected() - Error detection callback
+ * @pci_dev: PCI device information struct
+ * @state: PCI channel state
*
* This callback is called by the PCI subsystem whenever a PCI bus
* error is detected.
@@ -1328,6 +1344,8 @@ static struct pci_driver genwqe_driver = {
/**
* genwqe_devnode() - Set default access mode for genwqe devices.
+ * @dev: Pointer to device (unused)
+ * @mode: Carrier to pass-back given mode (permissions)
*
* Default mode should be rw for everybody. Do not change default
* device name.
--
2.25.1
On Mon, Jun 29, 2020 at 5:04 PM Lee Jones <[email protected]> wrote:
>
> audi_pb_set_block()'s argument 'base' was incorrectly named 'block' in
gaudi_pb_set_block()'s
> its function header.
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/misc/habanalabs/gaudi/gaudi_security.c:454: warning: Function parameter or member 'base' not described in 'gaudi_pb_set_block'
> drivers/misc/habanalabs/gaudi/gaudi_security.c:454: warning: Excess function parameter 'block' description in 'gaudi_pb_set_block'
>
> Cc: Oded Gabbay <[email protected]>
> Cc: Tomer Tayar <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/misc/habanalabs/gaudi/gaudi_security.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/misc/habanalabs/gaudi/gaudi_security.c b/drivers/misc/habanalabs/gaudi/gaudi_security.c
> index 6a351e31fa6af..abdd5ed8f2cf6 100644
> --- a/drivers/misc/habanalabs/gaudi/gaudi_security.c
> +++ b/drivers/misc/habanalabs/gaudi/gaudi_security.c
> @@ -447,8 +447,7 @@ static u64 gaudi_rr_hbw_mask_high_ar_regs[GAUDI_NUMBER_OF_RR_REGS] = {
> * gaudi_set_block_as_protected - set the given block as protected
> *
> * @hdev: pointer to hl_device structure
> - * @block: block base address
> - *
> + * @base: block base address
> */
> static void gaudi_pb_set_block(struct hl_device *hdev, u64 base)
> {
> --
> 2.25.1
>
With the above fix, This patch is:
Reviewed-by: Oded Gabbay <[email protected]>
On Mon, Jun 29, 2020 at 5:04 PM Lee Jones <[email protected]> wrote:
>
> No attempt to check the return value of RREG32() has been made
> since the call was introduced a year ago.
>
> Fixes W=1 kernel build warning:
>
> drivers/misc/habanalabs/goya/goya_coresight.c: In function ‘goya_debug_coresight’:
> drivers/misc/habanalabs/goya/goya_coresight.c:643:6: warning: variable ‘val’ set but not used [-Wunused-but-set-variable]
> 643 | u32 val;
> | ^~~
>
> Cc: Oded Gabbay <[email protected]>
> Cc: Tomer Tayar <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/misc/habanalabs/goya/goya_coresight.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/misc/habanalabs/goya/goya_coresight.c b/drivers/misc/habanalabs/goya/goya_coresight.c
> index 1258724ea5106..aa51fc71f0a1f 100644
> --- a/drivers/misc/habanalabs/goya/goya_coresight.c
> +++ b/drivers/misc/habanalabs/goya/goya_coresight.c
> @@ -640,7 +640,6 @@ static int goya_config_spmu(struct hl_device *hdev,
> int goya_debug_coresight(struct hl_device *hdev, void *data)
> {
> struct hl_debug_params *params = data;
> - u32 val;
> int rc = 0;
>
> switch (params->op) {
> @@ -672,7 +671,7 @@ int goya_debug_coresight(struct hl_device *hdev, void *data)
> }
>
> /* Perform read from the device to flush all configuration */
> - val = RREG32(mmPCIE_DBI_DEVICE_ID_VENDOR_ID_REG);
> + RREG32(mmPCIE_DBI_DEVICE_ID_VENDOR_ID_REG);
>
> return rc;
> }
> --
> 2.25.1
>
This patch is:
Reviewed-by: Oded Gabbay <[email protected]>
On Mon, Jun 29, 2020 at 5:04 PM Lee Jones <[email protected]> wrote:
>
> W=1 kernel builds report a lack of description of gaudi_set_asic_funcs()'s
> 'hdev' argument. In reality it is documented, but the formatting
> was not as expected '@.*:'. Instead, there was a misplaced asterisk
> which was confusing the kerneldoc validator.
>
> Squashes the following W=1 warning:
>
> drivers/misc/habanalabs/gaudi/gaudi.c:6746: warning: Function parameter or member 'hdev' not described in 'gaudi_set_asic_funcs'
>
> Cc: Oded Gabbay <[email protected]>
> Cc: Tomer Tayar <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/misc/habanalabs/gaudi/gaudi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/habanalabs/gaudi/gaudi.c b/drivers/misc/habanalabs/gaudi/gaudi.c
> index 61f88e9884ce9..bdb9c0080c464 100644
> --- a/drivers/misc/habanalabs/gaudi/gaudi.c
> +++ b/drivers/misc/habanalabs/gaudi/gaudi.c
> @@ -6739,7 +6739,7 @@ static const struct hl_asic_funcs gaudi_funcs = {
> /**
> * gaudi_set_asic_funcs - set GAUDI function pointers
> *
> - * @*hdev: pointer to hl_device structure
> + * @hdev: pointer to hl_device structure
> *
> */
> void gaudi_set_asic_funcs(struct hl_device *hdev)
> --
> 2.25.1
>
This patch is:
Reviewed-by: Oded Gabbay <[email protected]>
On Mon, Jun 29, 2020 at 5:04 PM Lee Jones <[email protected]> wrote:
>
> W=1 kernel builds report a lack of descriptions for various
> function arguments. In reality they are documented, but the
> formatting was not as expected '@.*:'. Instead, '-'s were
> used as separators.
>
> While we're here, the headers for functions various functions
> were written in kerneldoc format, but lack the kerneldoc
> identifier '/**'. Let's promote them so they can gain access
> to the checker.
>
> This change fixes the following W=1 warnings:
>
> drivers/misc/habanalabs/irq.c:24: warning: Function parameter or member 'eq_work' not described in 'hl_eqe_work'
> drivers/misc/habanalabs/irq.c:24: warning: Function parameter or member 'hdev' not described in 'hl_eqe_work'
> drivers/misc/habanalabs/irq.c:24: warning: Function parameter or member 'eq_entry' not described in 'hl_eqe_work'
>
> Cc: Oded Gabbay <[email protected]>
> Cc: Tomer Tayar <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/misc/habanalabs/irq.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/misc/habanalabs/irq.c b/drivers/misc/habanalabs/irq.c
> index 4e77a73857793..6981d67153b11 100644
> --- a/drivers/misc/habanalabs/irq.c
> +++ b/drivers/misc/habanalabs/irq.c
> @@ -13,9 +13,9 @@
> * struct hl_eqe_work - This structure is used to schedule work of EQ
> * entry and armcp_reset event
> *
> - * @eq_work - workqueue object to run when EQ entry is received
> - * @hdev - pointer to device structure
> - * @eq_entry - copy of the EQ entry
> + * @eq_work: workqueue object to run when EQ entry is received
> + * @hdev: pointer to device structure
> + * @eq_entry: copy of the EQ entry
> */
> struct hl_eqe_work {
> struct work_struct eq_work;
> @@ -23,7 +23,7 @@ struct hl_eqe_work {
> struct hl_eq_entry eq_entry;
> };
>
> -/*
> +/**
> * hl_cq_inc_ptr - increment ci or pi of cq
> *
> * @ptr: the current ci or pi value of the completion queue
> @@ -39,7 +39,7 @@ inline u32 hl_cq_inc_ptr(u32 ptr)
> return ptr;
> }
>
> -/*
> +/**
> * hl_eq_inc_ptr - increment ci of eq
> *
> * @ptr: the current ci value of the event queue
> @@ -66,7 +66,7 @@ static void irq_handle_eqe(struct work_struct *work)
> kfree(eqe_work);
> }
>
> -/*
> +/**
> * hl_irq_handler_cq - irq handler for completion queue
> *
> * @irq: irq number
> @@ -142,7 +142,7 @@ irqreturn_t hl_irq_handler_cq(int irq, void *arg)
> return IRQ_HANDLED;
> }
>
> -/*
> +/**
> * hl_irq_handler_eq - irq handler for event queue
> *
> * @irq: irq number
> @@ -206,7 +206,7 @@ irqreturn_t hl_irq_handler_eq(int irq, void *arg)
> return IRQ_HANDLED;
> }
>
> -/*
> +/**
> * hl_cq_init - main initialization function for an cq object
> *
> * @hdev: pointer to device structure
> @@ -238,7 +238,7 @@ int hl_cq_init(struct hl_device *hdev, struct hl_cq *q, u32 hw_queue_id)
> return 0;
> }
>
> -/*
> +/**
> * hl_cq_fini - destroy completion queue
> *
> * @hdev: pointer to device structure
> @@ -269,7 +269,7 @@ void hl_cq_reset(struct hl_device *hdev, struct hl_cq *q)
> memset((void *) (uintptr_t) q->kernel_address, 0, HL_CQ_SIZE_IN_BYTES);
> }
>
> -/*
> +/**
> * hl_eq_init - main initialization function for an event queue object
> *
> * @hdev: pointer to device structure
> @@ -297,7 +297,7 @@ int hl_eq_init(struct hl_device *hdev, struct hl_eq *q)
> return 0;
> }
>
> -/*
> +/**
> * hl_eq_fini - destroy event queue
> *
> * @hdev: pointer to device structure
> --
> 2.25.1
>
This patch is:
Reviewed-by: Oded Gabbay <[email protected]>
On 2020-06-29 16:04, Lee Jones wrote:
> Variable 'rc' hasn't been checked since the driver's inception
> in 2013. If it hasn't caused any issues since then, it's unlikely
> to in the future. Let's take it out for now.
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/misc/genwqe/card_base.c: In function
> ‘genwqe_health_check_stop’:
>
> /home/lee/projects/linux/kernel/drivers/misc/genwqe/card_base.c:1046:6:
> warning: variable ‘rc’ set but not used
> [-Wunused-but-set-variable]
> 1046 | int rc;
> | ^~
>
> Cc: Michael Jung <[email protected]>
> Cc: Michael Ruettger <[email protected]>
> Cc: Frank Haverkamp <[email protected]>
> Cc: Joerg-Stephan Vogt <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/misc/genwqe/card_base.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/misc/genwqe/card_base.c
> b/drivers/misc/genwqe/card_base.c
> index 1dc6c7c5cbce9..bceebf49de2d5 100644
> --- a/drivers/misc/genwqe/card_base.c
> +++ b/drivers/misc/genwqe/card_base.c
> @@ -1043,12 +1043,10 @@ static int genwqe_health_thread_running(struct
> genwqe_dev *cd)
>
> static int genwqe_health_check_stop(struct genwqe_dev *cd)
> {
> - int rc;
> -
> if (!genwqe_health_thread_running(cd))
> return -EIO;
>
> - rc = kthread_stop(cd->health_thread);
> + kthread_stop(cd->health_thread);
> cd->health_thread = NULL;
> return 0;
> }
Good idea. Let's remove it Thanks for the contribution.
Signed-off-by: Frank Haverkamp <[email protected]>
On 2020-06-29 16:04, Lee Jones wrote:
> From missing documentation for function arguments, to promotion
> obvious kerneldoc headers and incorrectly named arguments.
>
> Fixes the following W=1 warnings:
>
> drivers/misc/genwqe/card_base.c:175: warning: Function parameter or
> member 'cd' not described in 'genwqe_bus_reset'
> drivers/misc/genwqe/card_base.c:272: warning: Function parameter or
> member 'cd' not described in 'genwqe_recovery_on_fatal_gfir_required'
> drivers/misc/genwqe/card_base.c:293: warning: Function parameter or
> member 'cd' not described in 'genwqe_T_psec'
> drivers/misc/genwqe/card_base.c:314: warning: Function parameter or
> member 'cd' not described in 'genwqe_setup_pf_jtimer'
> drivers/misc/genwqe/card_base.c:334: warning: Function parameter or
> member 'cd' not described in 'genwqe_setup_vf_jtimer'
> drivers/misc/genwqe/card_base.c:557: warning: Function parameter or
> member 'cd' not described in 'genwqe_stop'
> drivers/misc/genwqe/card_base.c:617: warning: Function parameter or
> member 'cd' not described in 'genwqe_fir_checking'
> drivers/misc/genwqe/card_base.c:760: warning: Function parameter or
> member 'pci_dev' not described in 'genwqe_pci_fundamental_reset'
> drivers/misc/genwqe/card_base.c:889: warning: Function parameter or
> member 'data' not described in 'genwqe_health_thread'
> drivers/misc/genwqe/card_base.c:1046: warning: Function parameter or
> member 'cd' not described in 'genwqe_pci_setup'
> drivers/misc/genwqe/card_base.c:1131: warning: Function parameter or
> member 'cd' not described in 'genwqe_pci_remove'
> drivers/misc/genwqe/card_base.c:1151: warning: Function parameter or
> member 'pci_dev' not described in 'genwqe_probe'
> drivers/misc/genwqe/card_base.c:1151: warning: Function parameter or
> member 'id' not described in 'genwqe_probe'
> drivers/misc/genwqe/card_base.c:1151: warning: Excess function
> parameter 'pdev' description in 'genwqe_probe'
> drivers/misc/genwqe/card_base.c:1207: warning: Function parameter or
> member 'pci_dev' not described in 'genwqe_remove'
> drivers/misc/genwqe/card_base.c:1336: warning: Function parameter or
> member 'dev' not described in 'genwqe_devnode'
> drivers/misc/genwqe/card_base.c:1336: warning: Function parameter or
> member 'mode' not described in 'genwqe_devnode'
>
> Cc: Michael Jung <[email protected]>
> Cc: Michael Ruettger <[email protected]>
> Cc: Frank Haverkamp <[email protected]>
> Cc: Joerg-Stephan Vogt <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/misc/genwqe/card_base.c | 24 +++++++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/misc/genwqe/card_base.c
> b/drivers/misc/genwqe/card_base.c
> index 809a6f46f6de3..93d2ed91c85b2 100644
> --- a/drivers/misc/genwqe/card_base.c
> +++ b/drivers/misc/genwqe/card_base.c
> @@ -165,6 +165,7 @@ static void genwqe_dev_free(struct genwqe_dev *cd)
>
> /**
> * genwqe_bus_reset() - Card recovery
> + * @cd: GenWQE device information
> *
> * pci_reset_function() will recover the device and ensure that the
> * registers are accessible again when it completes with success. If
> @@ -262,6 +263,7 @@ static void genwqe_tweak_hardware(struct genwqe_dev
> *cd)
>
> /**
> * genwqe_recovery_on_fatal_gfir_required() - Version depended actions
> + * @cd: GenWQE device information
> *
> * Bitstreams older than 2013-02-17 have a bug where fatal GFIRs must
> * be ignored. This is e.g. true for the bitstream we gave to the card
> @@ -280,6 +282,7 @@ int genwqe_flash_readback_fails(struct genwqe_dev
> *cd)
>
> /**
> * genwqe_T_psec() - Calculate PF/VF timeout register content
> + * @cd: GenWQE device information
> *
> * Note: From a design perspective it turned out to be a bad idea to
> * use codes here to specifiy the frequency/speed values. An old
> @@ -303,6 +306,7 @@ static int genwqe_T_psec(struct genwqe_dev *cd)
>
> /**
> * genwqe_setup_pf_jtimer() - Setup PF hardware timeouts for DDCB
> execution
> + * @cd: GenWQE device information
> *
> * Do this _after_ card_reset() is called. Otherwise the values will
> * vanish. The settings need to be done when the queues are inactive.
> @@ -329,6 +333,7 @@ static bool genwqe_setup_pf_jtimer(struct
> genwqe_dev *cd)
>
> /**
> * genwqe_setup_vf_jtimer() - Setup VF hardware timeouts for DDCB
> execution
> + * @cd: GenWQE device information
> */
> static bool genwqe_setup_vf_jtimer(struct genwqe_dev *cd)
> {
> @@ -543,6 +548,7 @@ static int genwqe_start(struct genwqe_dev *cd)
>
> /**
> * genwqe_stop() - Stop card operation
> + * @cd: GenWQE device information
> *
> * Recovery notes:
> * As long as genwqe_thread runs we might access registers during
> @@ -606,6 +612,7 @@ static int genwqe_health_check_cond(struct
> genwqe_dev *cd, u64 *gfir)
>
> /**
> * genwqe_fir_checking() - Check the fault isolation registers of the
> card
> + * @cd: GenWQE device information
> *
> * If this code works ok, can be tried out with help of the
> genwqe_poke tool:
> * sudo ./tools/genwqe_poke 0x8 0xfefefefefef
> @@ -750,6 +757,7 @@ static u64 genwqe_fir_checking(struct genwqe_dev
> *cd)
>
> /**
> * genwqe_pci_fundamental_reset() - trigger a PCIe fundamental reset
> on the slot
> + * @pci_dev: PCI device information struct
> *
> * Note: pci_set_pcie_reset_state() is not implemented on all archs,
> so this
> * reset method will not work in all cases.
> @@ -814,8 +822,9 @@ static int genwqe_platform_recovery(struct
> genwqe_dev *cd)
> return rc;
> }
>
> -/*
> +/**
> * genwqe_reload_bistream() - reload card bitstream
> + * @cd: GenWQE device information
> *
> * Set the appropriate register and call fundamental reset to reaload
> the card
> * bitstream.
> @@ -868,6 +877,7 @@ static int genwqe_reload_bistream(struct genwqe_dev
> *cd)
>
> /**
> * genwqe_health_thread() - Health checking thread
> + * @data: GenWQE device information
> *
> * This thread is only started for the PF of the card.
> *
> @@ -1041,6 +1051,7 @@ static int genwqe_health_check_stop(struct
> genwqe_dev *cd)
>
> /**
> * genwqe_pci_setup() - Allocate PCIe related resources for our card
> + * @cd: GenWQE device information
> */
> static int genwqe_pci_setup(struct genwqe_dev *cd)
> {
> @@ -1126,6 +1137,7 @@ static int genwqe_pci_setup(struct genwqe_dev
> *cd)
>
> /**
> * genwqe_pci_remove() - Free PCIe related resources for our card
> + * @cd: GenWQE device information
> */
> static void genwqe_pci_remove(struct genwqe_dev *cd)
> {
> @@ -1140,7 +1152,8 @@ static void genwqe_pci_remove(struct genwqe_dev
> *cd)
>
> /**
> * genwqe_probe() - Device initialization
> - * @pdev: PCI device information struct
> + * @pci_dev: PCI device information struct
> + * @id: PCI device ID
> *
> * Callable for multiple cards. This function is called on bind.
> *
> @@ -1200,6 +1213,7 @@ static int genwqe_probe(struct pci_dev *pci_dev,
>
> /**
> * genwqe_remove() - Called when device is removed (hot-plugable)
> + * @pci_dev: PCI device information struct
> *
> * Or when driver is unloaded respecitively when unbind is done.
> */
> @@ -1219,8 +1233,10 @@ static void genwqe_remove(struct pci_dev
> *pci_dev)
> genwqe_dev_free(cd);
> }
>
> -/*
> +/**
> * genwqe_err_error_detected() - Error detection callback
> + * @pci_dev: PCI device information struct
> + * @state: PCI channel state
> *
> * This callback is called by the PCI subsystem whenever a PCI bus
> * error is detected.
> @@ -1328,6 +1344,8 @@ static struct pci_driver genwqe_driver = {
>
> /**
> * genwqe_devnode() - Set default access mode for genwqe devices.
> + * @dev: Pointer to device (unused)
> + * @mode: Carrier to pass-back given mode (permissions)
> *
> * Default mode should be rw for everybody. Do not change default
> * device name.
Thanks for adding the documentation.
Signed-off-by: Frank Haverkamp <[email protected]>
On 2020-06-29 16:04, Lee Jones wrote:
> Including; add missing documentation for function arguments,
> re-ordering
> of #defines i.e. not placed between kerneldoc headers and the functions
> they are documenting, demotion of file header/comment from kerneldoc
> format and removal of documentation for non-existent args.
>
> Fixes the following W=1 kernel build warnings:
>
> drivers/misc/genwqe/card_dev.c:33: warning: Function parameter or
> member 'cd' not described in 'genwqe_open_files'
> drivers/misc/genwqe/card_dev.c:98: warning: Function parameter or
> member 'virt_addr' not described in 'genwqe_search_pin'
> drivers/misc/genwqe/card_dev.c:98: warning: Excess function parameter
> 'dma_addr' description in 'genwqe_search_pin'
> drivers/misc/genwqe/card_dev.c:154: warning: Function parameter or
> member 'virt_addr' not described in '__genwqe_search_mapping'
> drivers/misc/genwqe/card_dev.c:256: warning: Function parameter or
> member 'cd' not described in 'genwqe_kill_fasync'
> drivers/misc/genwqe/card_dev.c:256: warning: Function parameter or
> member 'sig' not described in 'genwqe_kill_fasync'
> drivers/misc/genwqe/card_dev.c:387: warning: Function parameter or
> member 'vma' not described in 'genwqe_vma_close'
> drivers/misc/genwqe/card_dev.c:430: warning: Function parameter or
> member 'filp' not described in 'genwqe_mmap'
> drivers/misc/genwqe/card_dev.c:430: warning: Function parameter or
> member 'vma' not described in 'genwqe_mmap'
> drivers/misc/genwqe/card_dev.c:495: warning: Excess function
> parameter 'cd' description in 'FLASH_BLOCK'
> drivers/misc/genwqe/card_dev.c:495: warning: Excess function
> parameter 'load' description in 'FLASH_BLOCK'
> drivers/misc/genwqe/card_dev.c:827: warning: Function parameter or
> member 'cfile' not described in 'ddcb_cmd_cleanup'
> drivers/misc/genwqe/card_dev.c:827: warning: Function parameter or
> member 'req' not described in 'ddcb_cmd_cleanup'
> drivers/misc/genwqe/card_dev.c:854: warning: Function parameter or
> member 'cfile' not described in 'ddcb_cmd_fixups'
> drivers/misc/genwqe/card_dev.c:854: warning: Function parameter or
> member 'req' not described in 'ddcb_cmd_fixups'
> drivers/misc/genwqe/card_dev.c:984: warning: Function parameter or
> member 'cfile' not described in 'genwqe_execute_ddcb'
> drivers/misc/genwqe/card_dev.c:984: warning: Function parameter or
> member 'cmd' not described in 'genwqe_execute_ddcb'
> drivers/misc/genwqe/card_dev.c:1350: warning: Function parameter or
> member 'cd' not described in 'genwqe_device_remove'
>
> Cc: Michael Jung <[email protected]>
> Cc: Michael Ruettger <[email protected]>
> Cc: Frank Haverkamp <[email protected]>
> Cc: Joerg-Stephan Vogt <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/misc/genwqe/card_dev.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/misc/genwqe/card_dev.c
> b/drivers/misc/genwqe/card_dev.c
> index 040a0bda31254..55fc5b80e649f 100644
> --- a/drivers/misc/genwqe/card_dev.c
> +++ b/drivers/misc/genwqe/card_dev.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/**
> +/*
> * IBM Accelerator Family 'GenWQE'
> *
> * (C) Copyright IBM Corp. 2013
> @@ -87,7 +87,7 @@ static int genwqe_del_pin(struct genwqe_file *cfile,
> struct dma_mapping *m)
> * @cfile: Descriptor of opened file
> * @u_addr: User virtual address
> * @size: Size of buffer
> - * @dma_addr: DMA address to be updated
> + * @virt_addr: Virtual address to be updated
> *
> * Return: Pointer to the corresponding mapping NULL if not found
> */
> @@ -144,6 +144,7 @@ static void __genwqe_del_mapping(struct genwqe_file
> *cfile,
> * @u_addr: user virtual address
> * @size: size of buffer
> * @dma_addr: DMA address to be updated
> + * @virt_addr: Virtual address to be updated
> * Return: Pointer to the corresponding mapping NULL if not found
> */
> static struct dma_mapping *__genwqe_search_mapping(struct genwqe_file
> *cfile,
> @@ -249,6 +250,8 @@ static void genwqe_remove_pinnings(struct
> genwqe_file *cfile)
>
> /**
> * genwqe_kill_fasync() - Send signal to all processes with open
> GenWQE files
> + * @cd: GenWQE device information
> + * @sig: Signal to send out
> *
> * E.g. genwqe_send_signal(cd, SIGIO);
> */
> @@ -380,6 +383,7 @@ static void genwqe_vma_open(struct vm_area_struct
> *vma)
>
> /**
> * genwqe_vma_close() - Called each time when vma is unmapped
> + * @vma: VMA area to close
> *
> * Free memory which got allocated by GenWQE mmap().
> */
> @@ -416,6 +420,8 @@ static const struct vm_operations_struct
> genwqe_vma_ops = {
>
> /**
> * genwqe_mmap() - Provide contignous buffers to userspace
> + * @filp: File pointer (unused)
> + * @vma: VMA area to map
> *
> * We use mmap() to allocate contignous buffers used for DMA
> * transfers. After the buffer is allocated we remap it to user-space
> @@ -484,16 +490,15 @@ static int genwqe_mmap(struct file *filp, struct
> vm_area_struct *vma)
> return rc;
> }
>
> +#define FLASH_BLOCK 0x40000 /* we use 256k blocks */
> +
> /**
> * do_flash_update() - Excute flash update (write image or CVPD)
> - * @cd: genwqe device
> + * @cfile: Descriptor of opened file
> * @load: details about image load
> *
> * Return: 0 if successful
> */
> -
> -#define FLASH_BLOCK 0x40000 /* we use 256k blocks */
> -
> static int do_flash_update(struct genwqe_file *cfile,
> struct genwqe_bitstream *load)
> {
> @@ -820,6 +825,8 @@ static int genwqe_unpin_mem(struct genwqe_file
> *cfile, struct genwqe_mem *m)
>
> /**
> * ddcb_cmd_cleanup() - Remove dynamically created fixup entries
> + * @cfile: Descriptor of opened file
> + * @req: DDCB work request
> *
> * Only if there are any. Pinnings are not removed.
> */
> @@ -844,6 +851,8 @@ static int ddcb_cmd_cleanup(struct genwqe_file
> *cfile, struct ddcb_requ *req)
>
> /**
> * ddcb_cmd_fixups() - Establish DMA fixups/sglists for user memory
> references
> + * @cfile: Descriptor of opened file
> + * @req: DDCB work request
> *
> * Before the DDCB gets executed we need to handle the fixups. We
> * replace the user-space addresses with DMA addresses or do
> @@ -974,6 +983,8 @@ static int ddcb_cmd_fixups(struct genwqe_file
> *cfile, struct ddcb_requ *req)
>
> /**
> * genwqe_execute_ddcb() - Execute DDCB using userspace address fixups
> + * @cfile: Descriptor of opened file
> + * @cmd: Command identifier (passed from user)
> *
> * The code will build up the translation tables or lookup the
> * contignous memory allocation table to find the right translations
> @@ -1339,6 +1350,7 @@ static int
> genwqe_inform_and_stop_processes(struct genwqe_dev *cd)
>
> /**
> * genwqe_device_remove() - Remove genwqe's char device
> + * @cd: GenWQE device information
> *
> * This function must be called after the client devices are removed
> * because it will free the major/minor number range for the genwqe
Ok.
Signed-off-by: Frank Haverkamp <[email protected]>
On 2020-06-29 16:04, Lee Jones wrote:
> 'fatal_err' is taken as an argument to a static function which is only
> invoked once. During this invocation 'fatal_err' is not set. There
> doesn't appear to be a good reason to keep it around.
>
> Also fixes the following W=1 kernel build warning:
>
> drivers/misc/genwqe/card_base.c:588: warning: Function parameter or
> member 'fatal_err' not described in 'genwqe_recover_card'
>
> Cc: Michael Jung <[email protected]>
> Cc: Michael Ruettger <[email protected]>
> Cc: Frank Haverkamp <[email protected]>
> Cc: Joerg-Stephan Vogt <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/misc/genwqe/card_base.c | 18 +++---------------
> 1 file changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/misc/genwqe/card_base.c
> b/drivers/misc/genwqe/card_base.c
> index bceebf49de2d5..809a6f46f6de3 100644
> --- a/drivers/misc/genwqe/card_base.c
> +++ b/drivers/misc/genwqe/card_base.c
> @@ -569,30 +569,18 @@ static int genwqe_stop(struct genwqe_dev *cd)
>
> /**
> * genwqe_recover_card() - Try to recover the card if it is possible
> - *
> - * If fatal_err is set no register access is possible anymore. It is
> - * likely that genwqe_start fails in that situation. Proper error
> - * handling is required in this case.
> + * @cd: GenWQE device information
> *
> * genwqe_bus_reset() will cause the pci code to call genwqe_remove()
> * and later genwqe_probe() for all virtual functions.
> */
> -static int genwqe_recover_card(struct genwqe_dev *cd, int fatal_err)
> +static int genwqe_recover_card(struct genwqe_dev *cd)
> {
> int rc;
> struct pci_dev *pci_dev = cd->pci_dev;
>
> genwqe_stop(cd);
>
> - /*
> - * Make sure chip is not reloaded to maintain FFDC. Write SLU
> - * Reset Register, CPLDReset field to 0.
> - */
> - if (!fatal_err) {
> - cd->softreset = 0x70ull;
> - __genwqe_writeq(cd, IO_SLC_CFGREG_SOFTRESET, cd->softreset);
> - }
> -
> rc = genwqe_bus_reset(cd);
> if (rc != 0) {
> dev_err(&pci_dev->dev,
> @@ -958,7 +946,7 @@ static int genwqe_health_thread(void *data)
>
> cd->card_state = GENWQE_CARD_FATAL_ERROR;
>
> - rc = genwqe_recover_card(cd, 0);
> + rc = genwqe_recover_card(cd);
> if (rc < 0) {
> /* FIXME Card is unusable and needs unbind! */
> goto fatal_error;
I think this one I want to keep. Since fatal_err is 0, !fatal_error is 1
and the register write is actually executed.
I also want to keep the parameter even though it is only used with 0.
The register bit causes a less drastic recovery, but if we would
discover that that is not working ok, we rather discard the debug data
we get in this case instead of letting the recovery fail. I think it
does not hurt to keep it. Maybe you can add a comment?
Thanks
Frank
On Tue, 30 Jun 2020, haver wrote:
> On 2020-06-29 16:04, Lee Jones wrote:
> > Variable 'rc' hasn't been checked since the driver's inception
> > in 2013. If it hasn't caused any issues since then, it's unlikely
> > to in the future. Let's take it out for now.
> >
> > Fixes the following W=1 kernel build warning(s):
> >
> > drivers/misc/genwqe/card_base.c: In function
> > ‘genwqe_health_check_stop’:
> >
> > /home/lee/projects/linux/kernel/drivers/misc/genwqe/card_base.c:1046:6:
> > warning: variable ‘rc’ set but not used
> > [-Wunused-but-set-variable]
> > 1046 | int rc;
> > | ^~
> >
> > Cc: Michael Jung <[email protected]>
> > Cc: Michael Ruettger <[email protected]>
> > Cc: Frank Haverkamp <[email protected]>
> > Cc: Joerg-Stephan Vogt <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/misc/genwqe/card_base.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/misc/genwqe/card_base.c
> > b/drivers/misc/genwqe/card_base.c
> > index 1dc6c7c5cbce9..bceebf49de2d5 100644
> > --- a/drivers/misc/genwqe/card_base.c
> > +++ b/drivers/misc/genwqe/card_base.c
> > @@ -1043,12 +1043,10 @@ static int genwqe_health_thread_running(struct
> > genwqe_dev *cd)
> >
> > static int genwqe_health_check_stop(struct genwqe_dev *cd)
> > {
> > - int rc;
> > -
> > if (!genwqe_health_thread_running(cd))
> > return -EIO;
> >
> > - rc = kthread_stop(cd->health_thread);
> > + kthread_stop(cd->health_thread);
> > cd->health_thread = NULL;
> > return 0;
> > }
>
> Good idea. Let's remove it Thanks for the contribution.
No problem, and you are welcome.
> Signed-off-by: Frank Haverkamp <[email protected]>
Just as an aside, this should be Acked-by, unless you either
contributed to the patch directly or are in the delivery path i.e. you
plan to pick the patch and send it to, say Linus, via a pull-request.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
On 2020-06-30 09:42, Lee Jones wrote:
> On Tue, 30 Jun 2020, haver wrote:
>
>> On 2020-06-29 16:04, Lee Jones wrote:
>> > Variable 'rc' hasn't been checked since the driver's inception
>> > in 2013. If it hasn't caused any issues since then, it's unlikely
>> > to in the future. Let's take it out for now.
>> >
>> > Fixes the following W=1 kernel build warning(s):
>> >
>> > drivers/misc/genwqe/card_base.c: In function
>> > ‘genwqe_health_check_stop’:
>> >
>> > /home/lee/projects/linux/kernel/drivers/misc/genwqe/card_base.c:1046:6:
>> > warning: variable ‘rc’ set but not used
>> > [-Wunused-but-set-variable]
>> > 1046 | int rc;
>> > | ^~
>> >
>> > Cc: Michael Jung <[email protected]>
>> > Cc: Michael Ruettger <[email protected]>
>> > Cc: Frank Haverkamp <[email protected]>
>> > Cc: Joerg-Stephan Vogt <[email protected]>
>> > Signed-off-by: Lee Jones <[email protected]>
>> > ---
>> > drivers/misc/genwqe/card_base.c | 4 +---
>> > 1 file changed, 1 insertion(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/misc/genwqe/card_base.c
>> > b/drivers/misc/genwqe/card_base.c
>> > index 1dc6c7c5cbce9..bceebf49de2d5 100644
>> > --- a/drivers/misc/genwqe/card_base.c
>> > +++ b/drivers/misc/genwqe/card_base.c
>> > @@ -1043,12 +1043,10 @@ static int genwqe_health_thread_running(struct
>> > genwqe_dev *cd)
>> >
>> > static int genwqe_health_check_stop(struct genwqe_dev *cd)
>> > {
>> > - int rc;
>> > -
>> > if (!genwqe_health_thread_running(cd))
>> > return -EIO;
>> >
>> > - rc = kthread_stop(cd->health_thread);
>> > + kthread_stop(cd->health_thread);
>> > cd->health_thread = NULL;
>> > return 0;
>> > }
>>
>> Good idea. Let's remove it Thanks for the contribution.
>
> No problem, and you are welcome.
>
>> Signed-off-by: Frank Haverkamp <[email protected]>
>
> Just as an aside, this should be Acked-by, unless you either
> contributed to the patch directly or are in the delivery path i.e. you
> plan to pick the patch and send it to, say Linus, via a pull-request.
Right. Thanks for reminding me. Feel free to send it yourself.
When was the documentation checking introduced? At the time we
contributed the code there was no such checking.
Acked-by: Frank Haverkamp <[email protected]>
On Tue, 30 Jun 2020, haver wrote:
> On 2020-06-30 09:42, Lee Jones wrote:
> > On Tue, 30 Jun 2020, haver wrote:
> >
> > > On 2020-06-29 16:04, Lee Jones wrote:
> > > > Variable 'rc' hasn't been checked since the driver's inception
> > > > in 2013. If it hasn't caused any issues since then, it's unlikely
> > > > to in the future. Let's take it out for now.
> > > >
> > > > Fixes the following W=1 kernel build warning(s):
> > > >
> > > > drivers/misc/genwqe/card_base.c: In function
> > > > ‘genwqe_health_check_stop’:
> > > >
> > > > /home/lee/projects/linux/kernel/drivers/misc/genwqe/card_base.c:1046:6:
> > > > warning: variable ‘rc’ set but not used
> > > > [-Wunused-but-set-variable]
> > > > 1046 | int rc;
> > > > | ^~
> > > >
> > > > Cc: Michael Jung <[email protected]>
> > > > Cc: Michael Ruettger <[email protected]>
> > > > Cc: Frank Haverkamp <[email protected]>
> > > > Cc: Joerg-Stephan Vogt <[email protected]>
> > > > Signed-off-by: Lee Jones <[email protected]>
> > > > ---
> > > > drivers/misc/genwqe/card_base.c | 4 +---
> > > > 1 file changed, 1 insertion(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/misc/genwqe/card_base.c
> > > > b/drivers/misc/genwqe/card_base.c
> > > > index 1dc6c7c5cbce9..bceebf49de2d5 100644
> > > > --- a/drivers/misc/genwqe/card_base.c
> > > > +++ b/drivers/misc/genwqe/card_base.c
> > > > @@ -1043,12 +1043,10 @@ static int genwqe_health_thread_running(struct
> > > > genwqe_dev *cd)
> > > >
> > > > static int genwqe_health_check_stop(struct genwqe_dev *cd)
> > > > {
> > > > - int rc;
> > > > -
> > > > if (!genwqe_health_thread_running(cd))
> > > > return -EIO;
> > > >
> > > > - rc = kthread_stop(cd->health_thread);
> > > > + kthread_stop(cd->health_thread);
> > > > cd->health_thread = NULL;
> > > > return 0;
> > > > }
> > >
> > > Good idea. Let's remove it Thanks for the contribution.
> >
> > No problem, and you are welcome.
> >
> > > Signed-off-by: Frank Haverkamp <[email protected]>
> >
> > Just as an aside, this should be Acked-by, unless you either
> > contributed to the patch directly or are in the delivery path i.e. you
> > plan to pick the patch and send it to, say Linus, via a pull-request.
>
> Right. Thanks for reminding me. Feel free to send it yourself.
> When was the documentation checking introduced? At the time we
> contributed the code there was no such checking.
The checking scripts have been in place for many years, but it looks
like doc checking became part of EXTRA_GCC_CHECKS builds in 2017:
> commit 3a025e1d1c2ea42fa497c9c6b21c284e0f69e28b
Author: Matthew Wilcox <[email protected]>
Date: Mon Nov 20 10:40:40 2017 -0800
Add optional check for bad kernel-doc comments
Implement a '-none' output mode for kernel-doc which will only output
warning messages, and suppresses the warning message about there being
no kernel-doc in the file.
If the build has requested additional warnings, automatically check all
.c files. This patch does not check .h files. Enabling the warning
by default would add about 1300 warnings, so it's default off for now.
People who care can use this to check they didn't break the docs and
maybe we'll get all the warnings fixed and be able to enable this check
by default in the future.
Signed-off-by: Matthew Wilcox <[email protected]>
Signed-off-by: Jonathan Corbet <[email protected]>
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 2e3a10e79ca9e..aceac0ba07451 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -108,6 +108,10 @@ ifneq ($(KBUILD_CHECKSRC),0)
endif
endif
+ifneq ($(KBUILD_ENABLE_EXTRA_GCC_CHECKS),)
+ cmd_checkdoc = $(srctree)/scripts/kernel-doc -none $< ;
+endif
+
# Do section mismatch analysis for each module/built-in.o
ifdef CONFIG_DEBUG_SECTION_MISMATCH
cmd_secanalysis = ; scripts/mod/modpost $@
@@ -289,6 +293,7 @@ define rule_cc_o_c
$(call echo-cmd,checksrc) $(cmd_checksrc) \
$(call cmd_and_fixdep,cc_o_c) \
$(cmd_modversions_c) \
+ $(cmd_checkdoc) \
$(call echo-cmd,objtool) $(cmd_objtool) \
$(call echo-cmd,record_mcount) $(cmd_record_mcount)
endef
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
On Tue, 30 Jun 2020, haver wrote:
> On 2020-06-29 16:04, Lee Jones wrote:
> > 'fatal_err' is taken as an argument to a static function which is only
> > invoked once. During this invocation 'fatal_err' is not set. There
> > doesn't appear to be a good reason to keep it around.
> >
> > Also fixes the following W=1 kernel build warning:
> >
> > drivers/misc/genwqe/card_base.c:588: warning: Function parameter or
> > member 'fatal_err' not described in 'genwqe_recover_card'
> >
> > Cc: Michael Jung <[email protected]>
> > Cc: Michael Ruettger <[email protected]>
> > Cc: Frank Haverkamp <[email protected]>
> > Cc: Joerg-Stephan Vogt <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/misc/genwqe/card_base.c | 18 +++---------------
> > 1 file changed, 3 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/misc/genwqe/card_base.c
> > b/drivers/misc/genwqe/card_base.c
> > index bceebf49de2d5..809a6f46f6de3 100644
> > --- a/drivers/misc/genwqe/card_base.c
> > +++ b/drivers/misc/genwqe/card_base.c
> > @@ -569,30 +569,18 @@ static int genwqe_stop(struct genwqe_dev *cd)
> >
> > /**
> > * genwqe_recover_card() - Try to recover the card if it is possible
> > - *
> > - * If fatal_err is set no register access is possible anymore. It is
> > - * likely that genwqe_start fails in that situation. Proper error
> > - * handling is required in this case.
> > + * @cd: GenWQE device information
> > *
> > * genwqe_bus_reset() will cause the pci code to call genwqe_remove()
> > * and later genwqe_probe() for all virtual functions.
> > */
> > -static int genwqe_recover_card(struct genwqe_dev *cd, int fatal_err)
> > +static int genwqe_recover_card(struct genwqe_dev *cd)
> > {
> > int rc;
> > struct pci_dev *pci_dev = cd->pci_dev;
> >
> > genwqe_stop(cd);
> >
> > - /*
> > - * Make sure chip is not reloaded to maintain FFDC. Write SLU
> > - * Reset Register, CPLDReset field to 0.
> > - */
> > - if (!fatal_err) {
> > - cd->softreset = 0x70ull;
> > - __genwqe_writeq(cd, IO_SLC_CFGREG_SOFTRESET, cd->softreset);
> > - }
> > -
> > rc = genwqe_bus_reset(cd);
> > if (rc != 0) {
> > dev_err(&pci_dev->dev,
> > @@ -958,7 +946,7 @@ static int genwqe_health_thread(void *data)
> >
> > cd->card_state = GENWQE_CARD_FATAL_ERROR;
> >
> > - rc = genwqe_recover_card(cd, 0);
> > + rc = genwqe_recover_card(cd);
> > if (rc < 0) {
> > /* FIXME Card is unusable and needs unbind! */
> > goto fatal_error;
>
> I think this one I want to keep. Since fatal_err is 0, !fatal_error is 1 and
> the register write is actually executed.
Ah yes, good catch.
What if we *always* did instead then?
> I also want to keep the parameter even though it is only used with 0. The
> register bit causes a less drastic recovery, but if we would discover that
> that is not working ok, we rather discard the debug data we get in this case
> instead of letting the recovery fail. I think it does not hurt to keep it.
I'm not 100% against it, but it is dead code.
> Maybe you can add a comment?
If you really want to keep it, I can just update the kerneldoc
instead.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
On 2020-06-30 11:10, Lee Jones wrote:
> On Tue, 30 Jun 2020, haver wrote:
>
>> On 2020-06-29 16:04, Lee Jones wrote:
>> > 'fatal_err' is taken as an argument to a static function which is only
>> > invoked once. During this invocation 'fatal_err' is not set. There
>> > doesn't appear to be a good reason to keep it around.
>> >
>> > Also fixes the following W=1 kernel build warning:
>> >
>> > drivers/misc/genwqe/card_base.c:588: warning: Function parameter or
>> > member 'fatal_err' not described in 'genwqe_recover_card'
>> >
>> > Cc: Michael Jung <[email protected]>
>> > Cc: Michael Ruettger <[email protected]>
>> > Cc: Frank Haverkamp <[email protected]>
>> > Cc: Joerg-Stephan Vogt <[email protected]>
>> > Signed-off-by: Lee Jones <[email protected]>
>> > ---
>> > drivers/misc/genwqe/card_base.c | 18 +++---------------
>> > 1 file changed, 3 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/drivers/misc/genwqe/card_base.c
>> > b/drivers/misc/genwqe/card_base.c
>> > index bceebf49de2d5..809a6f46f6de3 100644
>> > --- a/drivers/misc/genwqe/card_base.c
>> > +++ b/drivers/misc/genwqe/card_base.c
>> > @@ -569,30 +569,18 @@ static int genwqe_stop(struct genwqe_dev *cd)
>> >
>> > /**
>> > * genwqe_recover_card() - Try to recover the card if it is possible
>> > - *
>> > - * If fatal_err is set no register access is possible anymore. It is
>> > - * likely that genwqe_start fails in that situation. Proper error
>> > - * handling is required in this case.
>> > + * @cd: GenWQE device information
>> > *
>> > * genwqe_bus_reset() will cause the pci code to call genwqe_remove()
>> > * and later genwqe_probe() for all virtual functions.
>> > */
>> > -static int genwqe_recover_card(struct genwqe_dev *cd, int fatal_err)
>> > +static int genwqe_recover_card(struct genwqe_dev *cd)
>> > {
>> > int rc;
>> > struct pci_dev *pci_dev = cd->pci_dev;
>> >
>> > genwqe_stop(cd);
>> >
>> > - /*
>> > - * Make sure chip is not reloaded to maintain FFDC. Write SLU
>> > - * Reset Register, CPLDReset field to 0.
>> > - */
>> > - if (!fatal_err) {
>> > - cd->softreset = 0x70ull;
>> > - __genwqe_writeq(cd, IO_SLC_CFGREG_SOFTRESET, cd->softreset);
>> > - }
>> > -
>> > rc = genwqe_bus_reset(cd);
>> > if (rc != 0) {
>> > dev_err(&pci_dev->dev,
>> > @@ -958,7 +946,7 @@ static int genwqe_health_thread(void *data)
>> >
>> > cd->card_state = GENWQE_CARD_FATAL_ERROR;
>> >
>> > - rc = genwqe_recover_card(cd, 0);
>> > + rc = genwqe_recover_card(cd);
>> > if (rc < 0) {
>> > /* FIXME Card is unusable and needs unbind! */
>> > goto fatal_error;
>>
>> I think this one I want to keep. Since fatal_err is 0, !fatal_error is
>> 1 and
>> the register write is actually executed.
>
> Ah yes, good catch.
>
> What if we *always* did instead then?
I knew you would ask that ;-).
>
>> I also want to keep the parameter even though it is only used with 0.
>> The
>> register bit causes a less drastic recovery, but if we would discover
>> that
>> that is not working ok, we rather discard the debug data we get in
>> this case
>> instead of letting the recovery fail. I think it does not hurt to keep
>> it.
>
> I'm not 100% against it, but it is dead code.
>
>> Maybe you can add a comment?
>
> If you really want to keep it, I can just update the kerneldoc
> instead.
I prefer that option. I want to indicate that there are two possible
ways to recover on a problem. If we delete the currently not exploited
parameter, this information gets lost.
Regards
Frank
On Tue, 30 Jun 2020, haver wrote:
> On 2020-06-30 11:10, Lee Jones wrote:
> > On Tue, 30 Jun 2020, haver wrote:
> >
> > > On 2020-06-29 16:04, Lee Jones wrote:
> > > > 'fatal_err' is taken as an argument to a static function which is only
> > > > invoked once. During this invocation 'fatal_err' is not set. There
> > > > doesn't appear to be a good reason to keep it around.
> > > >
> > > > Also fixes the following W=1 kernel build warning:
> > > >
> > > > drivers/misc/genwqe/card_base.c:588: warning: Function parameter or
> > > > member 'fatal_err' not described in 'genwqe_recover_card'
> > > >
> > > > Cc: Michael Jung <[email protected]>
> > > > Cc: Michael Ruettger <[email protected]>
> > > > Cc: Frank Haverkamp <[email protected]>
> > > > Cc: Joerg-Stephan Vogt <[email protected]>
> > > > Signed-off-by: Lee Jones <[email protected]>
> > > > ---
> > > > drivers/misc/genwqe/card_base.c | 18 +++---------------
> > > > 1 file changed, 3 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/drivers/misc/genwqe/card_base.c
> > > > b/drivers/misc/genwqe/card_base.c
> > > > index bceebf49de2d5..809a6f46f6de3 100644
> > > > --- a/drivers/misc/genwqe/card_base.c
> > > > +++ b/drivers/misc/genwqe/card_base.c
> > > > @@ -569,30 +569,18 @@ static int genwqe_stop(struct genwqe_dev *cd)
> > > >
> > > > /**
> > > > * genwqe_recover_card() - Try to recover the card if it is possible
> > > > - *
> > > > - * If fatal_err is set no register access is possible anymore. It is
> > > > - * likely that genwqe_start fails in that situation. Proper error
> > > > - * handling is required in this case.
> > > > + * @cd: GenWQE device information
> > > > *
> > > > * genwqe_bus_reset() will cause the pci code to call genwqe_remove()
> > > > * and later genwqe_probe() for all virtual functions.
> > > > */
> > > > -static int genwqe_recover_card(struct genwqe_dev *cd, int fatal_err)
> > > > +static int genwqe_recover_card(struct genwqe_dev *cd)
> > > > {
> > > > int rc;
> > > > struct pci_dev *pci_dev = cd->pci_dev;
> > > >
> > > > genwqe_stop(cd);
> > > >
> > > > - /*
> > > > - * Make sure chip is not reloaded to maintain FFDC. Write SLU
> > > > - * Reset Register, CPLDReset field to 0.
> > > > - */
> > > > - if (!fatal_err) {
> > > > - cd->softreset = 0x70ull;
> > > > - __genwqe_writeq(cd, IO_SLC_CFGREG_SOFTRESET, cd->softreset);
> > > > - }
> > > > -
> > > > rc = genwqe_bus_reset(cd);
> > > > if (rc != 0) {
> > > > dev_err(&pci_dev->dev,
> > > > @@ -958,7 +946,7 @@ static int genwqe_health_thread(void *data)
> > > >
> > > > cd->card_state = GENWQE_CARD_FATAL_ERROR;
> > > >
> > > > - rc = genwqe_recover_card(cd, 0);
> > > > + rc = genwqe_recover_card(cd);
> > > > if (rc < 0) {
> > > > /* FIXME Card is unusable and needs unbind! */
> > > > goto fatal_error;
> > >
> > > I think this one I want to keep. Since fatal_err is 0, !fatal_error
> > > is 1 and
> > > the register write is actually executed.
> >
> > Ah yes, good catch.
> >
> > What if we *always* did instead then?
>
> I knew you would ask that ;-).
>
> >
> > > I also want to keep the parameter even though it is only used with
> > > 0. The
> > > register bit causes a less drastic recovery, but if we would
> > > discover that
> > > that is not working ok, we rather discard the debug data we get in
> > > this case
> > > instead of letting the recovery fail. I think it does not hurt to
> > > keep it.
> >
> > I'm not 100% against it, but it is dead code.
> >
> > > Maybe you can add a comment?
> >
> > If you really want to keep it, I can just update the kerneldoc
> > instead.
>
> I prefer that option. I want to indicate that there are two possible ways to
> recover on a problem. If we delete the currently not exploited parameter,
> this information gets lost.
That's fine. Will fix.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog