| Message ID | 20230117032729.9578-1-peter@n8pjl.ca (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | wifi: rsi: Avoid defines prefixed with CONFIG | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Peter Lafreniere <peter@n8pjl.ca> writes: > To avoid confusion, it is best to only define CONFIG_* macros in Kconfig > files. Here we change the name of one define, which causes no change to > functionality. > > Signed-off-by: Peter Lafreniere <peter@n8pjl.ca> > --- [...] > --- a/drivers/net/wireless/rsi/rsi_hal.h > +++ b/drivers/net/wireless/rsi/rsi_hal.h > @@ -69,7 +69,7 @@ > #define EOF_REACHED 'E' > #define CHECK_CRC 'K' > #define POLLING_MODE 'P' > -#define CONFIG_AUTO_READ_MODE 'R' > +#define CONFIGURE_AUTO_READ_MODE 'R' I would prefer to add a prefix instead, for example RSI_CONFIG_AUTO_READ_MODE or something like that.
That would work better, but to me it seems odd to have one define with a prefix and the others without. I could change them all, but that seems like excessive churn for the very minor 'issue' that it fixes. After rereadig rsi_load_9113_firmware(), it seems like just dropping the CONFIG_ would be a reasonable change that doesn't affect readability. Cheers, Peter
Peter Lafreniere <peter@n8pjl.ca> writes: > That would work better, but to me it seems odd to have one define > with a prefix and the others without. I could change them all, but > that seems like excessive churn for the very minor 'issue' that > it fixes. > > After rereadig rsi_load_9113_firmware(), it seems like just dropping > the CONFIG_ would be a reasonable change that doesn't affect readability. Yeah, removing CONFIG_ looks good as well. But please add quotes into your email. We get A LOT of email so having proper quotes is important, otherwise we don't know what you are answering to.
diff --git a/drivers/net/wireless/rsi/rsi_91x_hal.c b/drivers/net/wireless/rsi/rsi_91x_hal.c index c7460fbba014..d2e02d5da3c0 100644 --- a/drivers/net/wireless/rsi/rsi_91x_hal.c +++ b/drivers/net/wireless/rsi/rsi_91x_hal.c @@ -894,7 +894,7 @@ static int rsi_load_9113_firmware(struct rsi_hw *adapter) struct ta_metadata *metadata_p; int status; - status = bl_cmd(adapter, CONFIG_AUTO_READ_MODE, CMD_PASS, + status = bl_cmd(adapter, CONFIGURE_AUTO_READ_MODE, CMD_PASS, "AUTO_READ_CMD"); if (status < 0) return status; @@ -984,7 +984,7 @@ static int rsi_load_9113_firmware(struct rsi_hw *adapter) } rsi_dbg(ERR_ZONE, "Firmware upgrade failed\n"); - status = bl_cmd(adapter, CONFIG_AUTO_READ_MODE, CMD_PASS, + status = bl_cmd(adapter, CONFIGURE_AUTO_READ_MODE, CMD_PASS, "AUTO_READ_MODE"); if (status) goto fail; diff --git a/drivers/net/wireless/rsi/rsi_hal.h b/drivers/net/wireless/rsi/rsi_hal.h index 5b07262a9740..e1d9a4676f44 100644 --- a/drivers/net/wireless/rsi/rsi_hal.h +++ b/drivers/net/wireless/rsi/rsi_hal.h @@ -69,7 +69,7 @@ #define EOF_REACHED 'E' #define CHECK_CRC 'K' #define POLLING_MODE 'P' -#define CONFIG_AUTO_READ_MODE 'R' +#define CONFIGURE_AUTO_READ_MODE 'R' #define JUMP_TO_ZERO_PC 'J' #define FW_LOADING_SUCCESSFUL 'S' #define LOADING_INITIATED '1'
To avoid confusion, it is best to only define CONFIG_* macros in Kconfig files. Here we change the name of one define, which causes no change to functionality. Signed-off-by: Peter Lafreniere <peter@n8pjl.ca> --- drivers/net/wireless/rsi/rsi_91x_hal.c | 4 ++-- drivers/net/wireless/rsi/rsi_hal.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)