* [PATCH 0/5] mtd: st_spi_fsm: Sweep-up remaining blocking-issues
@ 2014-03-20 11:11 Lee Jones
2014-03-20 11:11 ` [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments Lee Jones
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Lee Jones @ 2014-03-20 11:11 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, computersforpeace
Cc: linux-mtd, dwmw2, lee.jones, kernel, angus.clark
Brian,
Sincerest apologies for missing these before.
I hope this now covers all of points you raised.
drivers/mtd/devices/st_spi_fsm.c | 33 +++++++++------------------------
1 file changed, 9 insertions(+), 24 deletions(-)
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments
2014-03-20 11:11 [PATCH 0/5] mtd: st_spi_fsm: Sweep-up remaining blocking-issues Lee Jones
@ 2014-03-20 11:11 ` Lee Jones
2014-03-20 11:25 ` Joe Perches
2014-03-20 11:11 ` [PATCH 2/5] mtd: st_spi_fsm: Avoid duplicating MTD core code Lee Jones
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Lee Jones @ 2014-03-20 11:11 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, computersforpeace
Cc: linux-mtd, dwmw2, lee.jones, kernel, angus.clark
Reported-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/mtd/devices/st_spi_fsm.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index aefd48d..dcc1dc3 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -785,8 +785,7 @@ static void stfsm_wait_seq(struct stfsm *fsm)
dev_err(fsm->dev, "timeout on sequence completion\n");
}
-static void stfsm_read_fifo(struct stfsm *fsm, uint32_t *buf,
- const uint32_t size)
+static void stfsm_read_fifo(struct stfsm *fsm, uint32_t *buf, uint32_t size)
{
uint32_t remaining = size >> 2;
uint32_t avail;
@@ -811,8 +810,7 @@ static void stfsm_read_fifo(struct stfsm *fsm, uint32_t *buf,
}
}
-static int stfsm_write_fifo(struct stfsm *fsm,
- const uint32_t *buf, const uint32_t size)
+static int stfsm_write_fifo(struct stfsm *fsm, uint32_t *buf, uint32_t size)
{
uint32_t words = size >> 2;
@@ -1544,8 +1542,8 @@ static int stfsm_read(struct stfsm *fsm, uint8_t *buf, uint32_t size,
return 0;
}
-static int stfsm_write(struct stfsm *fsm, const uint8_t *const buf,
- const uint32_t size, const uint32_t offset)
+static int stfsm_write(struct stfsm *fsm, const uint8_t *buf,
+ uint32_t size, uint32_t offset)
{
struct stfsm_seq *seq = &fsm->stfsm_seq_write;
uint32_t data_pads;
@@ -1670,7 +1668,7 @@ static int stfsm_mtd_read(struct mtd_info *mtd, loff_t from, size_t len,
return 0;
}
-static int stfsm_erase_sector(struct stfsm *fsm, const uint32_t offset)
+static int stfsm_erase_sector(struct stfsm *fsm, uint32_t offset)
{
struct stfsm_seq *seq = &stfsm_seq_erase_sector;
int ret;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/5] mtd: st_spi_fsm: Avoid duplicating MTD core code
2014-03-20 11:11 [PATCH 0/5] mtd: st_spi_fsm: Sweep-up remaining blocking-issues Lee Jones
2014-03-20 11:11 ` [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments Lee Jones
@ 2014-03-20 11:11 ` Lee Jones
2014-03-20 11:11 ` [PATCH 3/5] mtd: st_spi_fsm: Correct vendor name spelling issue - missing "M" Lee Jones
` (3 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Lee Jones @ 2014-03-20 11:11 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, computersforpeace
Cc: linux-mtd, dwmw2, lee.jones, kernel, angus.clark
Reported-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/mtd/devices/st_spi_fsm.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index dcc1dc3..27f9ec9 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -1728,14 +1728,6 @@ static int stfsm_mtd_write(struct mtd_info *mtd, loff_t to, size_t len,
dev_dbg(fsm->dev, "%s to 0x%08x, len %zd\n", __func__, (u32)to, len);
- *retlen = 0;
-
- if (!len)
- return 0;
-
- if (to + len > mtd->size)
- return -EINVAL;
-
/* Offset within page */
page_offs = to % FLASH_PAGESIZE;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/5] mtd: st_spi_fsm: Correct vendor name spelling issue - missing "M"
2014-03-20 11:11 [PATCH 0/5] mtd: st_spi_fsm: Sweep-up remaining blocking-issues Lee Jones
2014-03-20 11:11 ` [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments Lee Jones
2014-03-20 11:11 ` [PATCH 2/5] mtd: st_spi_fsm: Avoid duplicating MTD core code Lee Jones
@ 2014-03-20 11:11 ` Lee Jones
2014-03-20 11:11 ` [PATCH 4/5] mtd: st_spi_fsm: Allow loop to run at least once before giving up CPU Lee Jones
` (2 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Lee Jones @ 2014-03-20 11:11 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, computersforpeace
Cc: linux-mtd, dwmw2, lee.jones, kernel, angus.clark
Reported-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/mtd/devices/st_spi_fsm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 27f9ec9..9de753d 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -3,7 +3,7 @@
*
* Author: Angus Clark <angus.clark@st.com>
*
- * Copyright (C) 2010-2014 STicroelectronics Limited
+ * Copyright (C) 2010-2014 STMicroelectronics Limited
*
* JEDEC probe based on drivers/mtd/devices/m25p80.c
*
--
1.8.3.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/5] mtd: st_spi_fsm: Allow loop to run at least once before giving up CPU
2014-03-20 11:11 [PATCH 0/5] mtd: st_spi_fsm: Sweep-up remaining blocking-issues Lee Jones
` (2 preceding siblings ...)
2014-03-20 11:11 ` [PATCH 3/5] mtd: st_spi_fsm: Correct vendor name spelling issue - missing "M" Lee Jones
@ 2014-03-20 11:11 ` Lee Jones
2014-03-20 11:11 ` [PATCH 5/5] mtd: st_spi_fsm: Succinctly reorganise .remove() Lee Jones
2014-03-20 11:29 ` [PATCH 0/5] mtd: st_spi_fsm: Sweep-up remaining blocking-issues Brian Norris
5 siblings, 0 replies; 18+ messages in thread
From: Lee Jones @ 2014-03-20 11:11 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, computersforpeace
Cc: linux-mtd, dwmw2, lee.jones, kernel, angus.clark
Reported-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/mtd/devices/st_spi_fsm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 9de753d..921b44a 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -860,8 +860,6 @@ static uint8_t stfsm_wait_busy(struct stfsm *fsm)
*/
deadline = jiffies + FLASH_MAX_BUSY_WAIT;
while (!timeout) {
- cond_resched();
-
if (time_after_eq(jiffies, deadline))
timeout = 1;
@@ -880,6 +878,8 @@ static uint8_t stfsm_wait_busy(struct stfsm *fsm)
if (!timeout)
/* Restart */
writel(seq->seq_cfg, fsm->base + SPI_FAST_SEQ_CFG);
+
+ cond_resched();
}
dev_err(fsm->dev, "timeout on wait_busy\n");
--
1.8.3.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/5] mtd: st_spi_fsm: Succinctly reorganise .remove()
2014-03-20 11:11 [PATCH 0/5] mtd: st_spi_fsm: Sweep-up remaining blocking-issues Lee Jones
` (3 preceding siblings ...)
2014-03-20 11:11 ` [PATCH 4/5] mtd: st_spi_fsm: Allow loop to run at least once before giving up CPU Lee Jones
@ 2014-03-20 11:11 ` Lee Jones
2014-03-20 11:29 ` [PATCH 0/5] mtd: st_spi_fsm: Sweep-up remaining blocking-issues Brian Norris
5 siblings, 0 replies; 18+ messages in thread
From: Lee Jones @ 2014-03-20 11:11 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, computersforpeace
Cc: linux-mtd, dwmw2, lee.jones, kernel, angus.clark
Reported-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/mtd/devices/st_spi_fsm.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 921b44a..bea1416 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -2081,13 +2081,8 @@ static int stfsm_probe(struct platform_device *pdev)
static int stfsm_remove(struct platform_device *pdev)
{
struct stfsm *fsm = platform_get_drvdata(pdev);
- int err;
- err = mtd_device_unregister(&fsm->mtd);
- if (err)
- return err;
-
- return 0;
+ return mtd_device_unregister(&fsm->mtd);
}
static struct of_device_id stfsm_match[] = {
--
1.8.3.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments
2014-03-20 11:11 ` [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments Lee Jones
@ 2014-03-20 11:25 ` Joe Perches
2014-03-20 11:41 ` Lee Jones
0 siblings, 1 reply; 18+ messages in thread
From: Joe Perches @ 2014-03-20 11:25 UTC (permalink / raw)
To: Lee Jones
Cc: angus.clark, kernel, linux-kernel, linux-mtd, computersforpeace,
dwmw2, linux-arm-kernel
On Thu, 2014-03-20 at 11:11 +0000, Lee Jones wrote:
> Reported-by: Brian Norris <computersforpeace@gmail.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
Why are these useless?
Even if the object code produced is the same,
these serve as information to the human reader
about how the arguments are used.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/5] mtd: st_spi_fsm: Sweep-up remaining blocking-issues
2014-03-20 11:11 [PATCH 0/5] mtd: st_spi_fsm: Sweep-up remaining blocking-issues Lee Jones
` (4 preceding siblings ...)
2014-03-20 11:11 ` [PATCH 5/5] mtd: st_spi_fsm: Succinctly reorganise .remove() Lee Jones
@ 2014-03-20 11:29 ` Brian Norris
5 siblings, 0 replies; 18+ messages in thread
From: Brian Norris @ 2014-03-20 11:29 UTC (permalink / raw)
To: Lee Jones
Cc: angus.clark, kernel, linux-kernel, linux-mtd, dwmw2, linux-arm-kernel
On Thu, Mar 20, 2014 at 11:11:42AM +0000, Lee Jones wrote:
> Sincerest apologies for missing these before.
>
> I hope this now covers all of points you raised.
Looks good. Thanks for following up. Pushed all to l2-mtd.git.
Thanks,
Brian
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments
2014-03-20 11:25 ` Joe Perches
@ 2014-03-20 11:41 ` Lee Jones
2014-03-20 11:48 ` Joe Perches
0 siblings, 1 reply; 18+ messages in thread
From: Lee Jones @ 2014-03-20 11:41 UTC (permalink / raw)
To: Joe Perches
Cc: angus.clark, kernel, linux-kernel, linux-mtd, computersforpeace,
dwmw2, linux-arm-kernel
On Thu, 20 Mar 2014, Joe Perches wrote:
> On Thu, 2014-03-20 at 11:11 +0000, Lee Jones wrote:
> > Reported-by: Brian Norris <computersforpeace@gmail.com>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
>
> Why are these useless?
>
> Even if the object code produced is the same,
> these serve as information to the human reader
> about how the arguments are used.
https://lkml.org/lkml/2014/3/20/69
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments
2014-03-20 11:41 ` Lee Jones
@ 2014-03-20 11:48 ` Joe Perches
2014-03-20 11:54 ` Brian Norris
0 siblings, 1 reply; 18+ messages in thread
From: Joe Perches @ 2014-03-20 11:48 UTC (permalink / raw)
To: Lee Jones
Cc: angus.clark, kernel, linux-kernel, linux-mtd, computersforpeace,
dwmw2, linux-arm-kernel
On Thu, 2014-03-20 at 11:41 +0000, Lee Jones wrote:
> On Thu, 20 Mar 2014, Joe Perches wrote:
>
> > On Thu, 2014-03-20 at 11:11 +0000, Lee Jones wrote:
> > > Reported-by: Brian Norris <computersforpeace@gmail.com>
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >
> > Why are these useless?
> >
> > Even if the object code produced is the same,
> > these serve as information to the human reader
> > about how the arguments are used.
>
> https://lkml.org/lkml/2014/3/20/69
>
>From that email:
suggestion:
const uint8_t *const buf => const uint8_t *buf
const uint32_t size => uint32_t size
const uint32_t offset => uint32_t offset
What was done:
-static int stfsm_write_fifo(struct stfsm *fsm,
- const uint32_t *buf, const uint32_t size)
+static int stfsm_write_fifo(struct stfsm *fsm, uint32_t *buf, uint32_t size)
Note the removal of the const from uint32_t *buf
Why?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments
2014-03-20 11:48 ` Joe Perches
@ 2014-03-20 11:54 ` Brian Norris
2014-03-20 12:03 ` Lee Jones
0 siblings, 1 reply; 18+ messages in thread
From: Brian Norris @ 2014-03-20 11:54 UTC (permalink / raw)
To: Joe Perches
Cc: angus.clark, kernel, Lee Jones, linux-kernel, linux-mtd, dwmw2,
linux-arm-kernel
On Thu, Mar 20, 2014 at 04:48:10AM -0700, Joe Perches wrote:
> On Thu, 2014-03-20 at 11:41 +0000, Lee Jones wrote:
> > On Thu, 20 Mar 2014, Joe Perches wrote:
> > > On Thu, 2014-03-20 at 11:11 +0000, Lee Jones wrote:
> > > > Reported-by: Brian Norris <computersforpeace@gmail.com>
> > > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > >
> > > Why are these useless?
> > >
> > > Even if the object code produced is the same,
> > > these serve as information to the human reader
> > > about how the arguments are used.
> >
> > https://lkml.org/lkml/2014/3/20/69
> >
>
> From that email:
>
> suggestion:
>
> const uint8_t *const buf => const uint8_t *buf
> const uint32_t size => uint32_t size
> const uint32_t offset => uint32_t offset
>
> What was done:
>
> -static int stfsm_write_fifo(struct stfsm *fsm,
> - const uint32_t *buf, const uint32_t size)
> +static int stfsm_write_fifo(struct stfsm *fsm, uint32_t *buf, uint32_t size)
>
> Note the removal of the const from uint32_t *buf
Good catch. That wasn't my intention.
> Why?
I can let Lee speak for himself. But I'm thinking I'll squash this diff
in, if no one minds:
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index bea1416aad07..1957d7c8e185 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -810,7 +810,8 @@ static void stfsm_read_fifo(struct stfsm *fsm, uint32_t *buf, uint32_t size)
}
}
-static int stfsm_write_fifo(struct stfsm *fsm, uint32_t *buf, uint32_t size)
+static int stfsm_write_fifo(struct stfsm *fsm, const uint32_t *buf,
+ uint32_t size)
{
uint32_t words = size >> 2;
@@ -1806,7 +1807,7 @@ out1:
return ret;
}
-static void stfsm_read_jedec(struct stfsm *fsm, uint8_t *const jedec)
+static void stfsm_read_jedec(struct stfsm *fsm, uint8_t *jedec)
{
const struct stfsm_seq *seq = &stfsm_seq_read_jedec;
uint32_t tmp[2];
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments
2014-03-20 11:54 ` Brian Norris
@ 2014-03-20 12:03 ` Lee Jones
2014-03-20 12:05 ` Brian Norris
2014-03-20 12:13 ` Joe Perches
0 siblings, 2 replies; 18+ messages in thread
From: Lee Jones @ 2014-03-20 12:03 UTC (permalink / raw)
To: Brian Norris
Cc: angus.clark, kernel, linux-kernel, linux-mtd, Joe Perches, dwmw2,
linux-arm-kernel
> Good catch. That wasn't my intention.
+1
> > Why?
An oversight.
> I can let Lee speak for himself. But I'm thinking I'll squash this diff
> in, if no one minds:
>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Patch looks good to me:
Acked-by: Lee Jones <lee.jones@linaro.org>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments
2014-03-20 12:03 ` Lee Jones
@ 2014-03-20 12:05 ` Brian Norris
2014-03-20 12:13 ` Joe Perches
1 sibling, 0 replies; 18+ messages in thread
From: Brian Norris @ 2014-03-20 12:05 UTC (permalink / raw)
To: Lee Jones
Cc: Angus CLARK, kernel, Linux Kernel, linux-mtd, Joe Perches,
David Woodhouse, linux-arm-kernel
On Thu, Mar 20, 2014 at 5:03 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
>
> Patch looks good to me:
> Acked-by: Lee Jones <lee.jones@linaro.org>
Squashed into $SUBJECT patch and pushed to l2-mtd.git.
Brian
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments
2014-03-20 12:03 ` Lee Jones
2014-03-20 12:05 ` Brian Norris
@ 2014-03-20 12:13 ` Joe Perches
2014-03-20 12:41 ` Brian Norris
2014-03-20 13:29 ` Lee Jones
1 sibling, 2 replies; 18+ messages in thread
From: Joe Perches @ 2014-03-20 12:13 UTC (permalink / raw)
To: Lee Jones
Cc: angus.clark, kernel, linux-kernel, linux-mtd, Brian Norris,
dwmw2, linux-arm-kernel
On Thu, 2014-03-20 at 12:03 +0000, Lee Jones wrote:
> > Good catch. That wasn't my intention.
> > > Why?
> An oversight.
That's still not an explanation.
Why, unless cast away by the code itself, is
const removal a good thing?
It does serve as an indication to a reader what
the code does with the argument.
About the only reason I can think of arguing in
favor of removal is inconsistent application of
const within the module.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments
2014-03-20 12:13 ` Joe Perches
@ 2014-03-20 12:41 ` Brian Norris
2014-03-20 12:44 ` Joe Perches
2014-03-20 13:29 ` Lee Jones
1 sibling, 1 reply; 18+ messages in thread
From: Brian Norris @ 2014-03-20 12:41 UTC (permalink / raw)
To: Joe Perches
Cc: Angus CLARK, kernel, Lee Jones, Linux Kernel, linux-mtd,
David Woodhouse, linux-arm-kernel
On Thu, Mar 20, 2014 at 5:13 AM, Joe Perches <joe@perches.com> wrote:
> On Thu, 2014-03-20 at 12:03 +0000, Lee Jones wrote:
>> > Good catch. That wasn't my intention.
>> > > Why?
>> An oversight.
>
> That's still not an explanation.
>
> Why, unless cast away by the code itself, is
> const removal a good thing?
It's not so much removal as it is review of the initial driver merge.
I'd contend that const was applied somewhat thoughtlessly originally,
and it didn't really serve a good purpose.
> It does serve as an indication to a reader what
> the code does with the argument.
>
> About the only reason I can think of arguing in
> favor of removal is inconsistent application of
> const within the module.
That's one good reason. And not only consistency within the modules,
but consistency within the subsystem (and the kernel at large,
really). There's rarely a case of a const function parameter. And I'm
sure there are numerous function parameters which could potentially be
marked 'const'.
I also don't think that a function parameter is the right place to
mark const like this. Function arguments are always pass-by-value, so
this 'const' tells users (callers) nothing useful. It only provides
useless constraints on what the function can do with its copy of the
parameter.
Brian
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments
2014-03-20 12:41 ` Brian Norris
@ 2014-03-20 12:44 ` Joe Perches
2014-03-20 16:58 ` Brian Norris
0 siblings, 1 reply; 18+ messages in thread
From: Joe Perches @ 2014-03-20 12:44 UTC (permalink / raw)
To: Brian Norris
Cc: Angus CLARK, kernel, Lee Jones, Linux Kernel, linux-mtd,
David Woodhouse, linux-arm-kernel
On Thu, 2014-03-20 at 05:41 -0700, Brian Norris wrote:
> I also don't think that a function parameter is the right place to
> mark const like this. Function arguments are always pass-by-value, so
> this 'const' tells users (callers) nothing useful. It only provides
> useless constraints on what the function can do with its copy of the
> parameter.
Again, that's not useless information.
And as you've seen, just making these changes
can be error prone.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments
2014-03-20 12:13 ` Joe Perches
2014-03-20 12:41 ` Brian Norris
@ 2014-03-20 13:29 ` Lee Jones
1 sibling, 0 replies; 18+ messages in thread
From: Lee Jones @ 2014-03-20 13:29 UTC (permalink / raw)
To: Joe Perches
Cc: angus.clark, kernel, linux-kernel, linux-mtd, Brian Norris,
dwmw2, linux-arm-kernel
> > > Good catch. That wasn't my intention.
> > > > Why?
> > An oversight.
>
> That's still not an explanation.
>
> Why, unless cast away by the code itself, is
> const removal a good thing?
>
> It does serve as an indication to a reader what
> the code does with the argument.
>
> About the only reason I can think of arguing in
> favor of removal is inconsistent application of
> const within the module.
Your qualm is not with me in this instance. I don't ever mind holding
my hands up to mistakes I make or standing up for decisions I believe
in, but in this case I'm merely satisfying the wishes of a maintainer
I'm submitting code to.
I am strongly un-opinionated about these particular changes.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments
2014-03-20 12:44 ` Joe Perches
@ 2014-03-20 16:58 ` Brian Norris
0 siblings, 0 replies; 18+ messages in thread
From: Brian Norris @ 2014-03-20 16:58 UTC (permalink / raw)
To: Joe Perches
Cc: Angus CLARK, kernel, Lee Jones, Linux Kernel, linux-mtd,
David Woodhouse, linux-arm-kernel
On Thu, Mar 20, 2014 at 05:44:45AM -0700, Joe Perches wrote:
> On Thu, 2014-03-20 at 05:41 -0700, Brian Norris wrote:
> > I also don't think that a function parameter is the right place to
> > mark const like this. Function arguments are always pass-by-value, so
> > this 'const' tells users (callers) nothing useful. It only provides
> > useless constraints on what the function can do with its copy of the
> > parameter.
>
> Again, that's not useless information.
For local, pass-by-value function arguments (i.e., constant data, or
constant pointers to data), I respectfully disagree. (For "pointers to
constant data", I completely agree that the 'const' info is useful.)
> And as you've seen, just making these changes
> can be error prone.
Thank you for catching our mistake now. But I don't think that is
relevant; just because you caught an error doesn't mean that the change
(primarily for consistency's sake) should be avoided entirely.
Unless you have a more convincing argument, this code will remain as-is.
Regards,
Brian
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2014-03-20 16:58 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-20 11:11 [PATCH 0/5] mtd: st_spi_fsm: Sweep-up remaining blocking-issues Lee Jones
2014-03-20 11:11 ` [PATCH 1/5] mtd: st_spi_fsm: Remove useless consts from function arguments Lee Jones
2014-03-20 11:25 ` Joe Perches
2014-03-20 11:41 ` Lee Jones
2014-03-20 11:48 ` Joe Perches
2014-03-20 11:54 ` Brian Norris
2014-03-20 12:03 ` Lee Jones
2014-03-20 12:05 ` Brian Norris
2014-03-20 12:13 ` Joe Perches
2014-03-20 12:41 ` Brian Norris
2014-03-20 12:44 ` Joe Perches
2014-03-20 16:58 ` Brian Norris
2014-03-20 13:29 ` Lee Jones
2014-03-20 11:11 ` [PATCH 2/5] mtd: st_spi_fsm: Avoid duplicating MTD core code Lee Jones
2014-03-20 11:11 ` [PATCH 3/5] mtd: st_spi_fsm: Correct vendor name spelling issue - missing "M" Lee Jones
2014-03-20 11:11 ` [PATCH 4/5] mtd: st_spi_fsm: Allow loop to run at least once before giving up CPU Lee Jones
2014-03-20 11:11 ` [PATCH 5/5] mtd: st_spi_fsm: Succinctly reorganise .remove() Lee Jones
2014-03-20 11:29 ` [PATCH 0/5] mtd: st_spi_fsm: Sweep-up remaining blocking-issues Brian Norris
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).