[U-Boot] [PATCH] spi: Use DIV_ROUND_UP at appropriate places

This change slightly improves readability.
Signed-off-by: Axel Lin axel.lin@ingics.com --- drivers/spi/cf_qspi.c | 2 +- drivers/spi/mxc_spi.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cf_qspi.c b/drivers/spi/cf_qspi.c index a37ac4e..06bcf91 100644 --- a/drivers/spi/cf_qspi.c +++ b/drivers/spi/cf_qspi.c @@ -171,7 +171,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, volatile qspi_t *qspi = dev->regs; u8 *txbuf = (u8 *)dout; u8 *rxbuf = (u8 *)din; - u32 count = ((bitlen / 8) + (bitlen % 8 ? 1 : 0)); + u32 count = DIV_ROUND_UP(bitlen, 8); u32 n, i = 0;
/* Sanitize arguments */ diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 5bed858..2ea3228 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -224,7 +224,7 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen, const u8 *dout, u8 *din, unsigned long flags) { struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave); - int nbytes = (bitlen + 7) / 8; + int nbytes = DIV_ROUND_UP(bitlen, 8); u32 data, cnt, i; struct cspi_regs *regs = (struct cspi_regs *)mxcs->base;
@@ -294,7 +294,7 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen, /* Transfer completed, clear any pending request */ reg_write(®s->stat, MXC_CSPICTRL_TC | MXC_CSPICTRL_RXOVF);
- nbytes = (bitlen + 7) / 8; + nbytes = DIV_ROUND_UP(bitlen, 8);
cnt = nbytes % 32;
@@ -330,7 +330,7 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen, int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, void *din, unsigned long flags) { - int n_bytes = (bitlen + 7) / 8; + int n_bytes = DIV_ROUND_UP(bitlen, 8); int n_bits; int ret; u32 blk_size;

Hi,
IMHO: Can you please use the proper commit header prefix.
I am just sharing my thoughts, ignore this if you know it already. I followed below syntax. "<main_module>: <sub_module>: <sub_sub_module>: <COMMIT_HEADER>"
Ex: for this commit (seems like this commit changes two drivers) spi: cf_qspi | mxc_spi: Use DIV_ROUND_UP at appropriate places
if you send same changes individually. spi: cf_qspi: Use DIV_ROUND_UP at appropriate places spi: mxc_spi: Use DIV_ROUND_UP at appropriate places
On Fri, Jun 14, 2013 at 1:29 PM, Axel Lin axel.lin@ingics.com wrote:
This change slightly improves readability.
Signed-off-by: Axel Lin axel.lin@ingics.com
drivers/spi/cf_qspi.c | 2 +- drivers/spi/mxc_spi.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cf_qspi.c b/drivers/spi/cf_qspi.c index a37ac4e..06bcf91 100644 --- a/drivers/spi/cf_qspi.c +++ b/drivers/spi/cf_qspi.c @@ -171,7 +171,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, volatile qspi_t *qspi = dev->regs; u8 *txbuf = (u8 *)dout; u8 *rxbuf = (u8 *)din;
u32 count = ((bitlen / 8) + (bitlen % 8 ? 1 : 0));
Was this equivalent to (bitlen + 7) / 8 ?
-- Thanks, Jagan.

2013/6/14 Jagan Teki jagannadh.teki@gmail.com:
Hi,
IMHO: Can you please use the proper commit header prefix.
I am just sharing my thoughts, ignore this if you know it already. I followed below syntax. "<main_module>: <sub_module>: <sub_sub_module>: <COMMIT_HEADER>"
Ex: for this commit (seems like this commit changes two drivers) spi: cf_qspi | mxc_spi: Use DIV_ROUND_UP at appropriate places
This does not scale. What if a (trivial) patch touches 10 drivers?
if you send same changes individually. spi: cf_qspi: Use DIV_ROUND_UP at appropriate places spi: mxc_spi: Use DIV_ROUND_UP at appropriate places
I was thinking doing so may add maintainer's burden. (well for this case with 2 patches, it's not a problem) But since you prefer sending a fix per driver, I'll resend the patches.
On Fri, Jun 14, 2013 at 1:29 PM, Axel Lin axel.lin@ingics.com wrote:
This change slightly improves readability.
Signed-off-by: Axel Lin axel.lin@ingics.com
drivers/spi/cf_qspi.c | 2 +- drivers/spi/mxc_spi.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cf_qspi.c b/drivers/spi/cf_qspi.c index a37ac4e..06bcf91 100644 --- a/drivers/spi/cf_qspi.c +++ b/drivers/spi/cf_qspi.c @@ -171,7 +171,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, volatile qspi_t *qspi = dev->regs; u8 *txbuf = (u8 *)dout; u8 *rxbuf = (u8 *)din;
u32 count = ((bitlen / 8) + (bitlen % 8 ? 1 : 0));
Was this equivalent to (bitlen + 7) / 8 ?
Yes. And that is the point of this patch ( to handle divide-round-up in a uniform way).
-- Thanks, Jagan.

On Fri, Jun 14, 2013 at 6:39 PM, Axel Lin axel.lin@ingics.com wrote:
2013/6/14 Jagan Teki jagannadh.teki@gmail.com:
Hi,
IMHO: Can you please use the proper commit header prefix.
I am just sharing my thoughts, ignore this if you know it already. I followed below syntax. "<main_module>: <sub_module>: <sub_sub_module>: <COMMIT_HEADER>"
Ex: for this commit (seems like this commit changes two drivers) spi: cf_qspi | mxc_spi: Use DIV_ROUND_UP at appropriate places
This does not scale. What if a (trivial) patch touches 10 drivers?
Yes, you are correct if the changes is common to across the all drivers or If the changes which are more in 10 spi drivers out of 20 or > 10 for your example - use "spi: " and I think it's good to use above syntax as you changesd two drivers.
I am just showing the possibilities of using the coding guidelines, nothing more.
-- Thanks, Jagan.

On 14/06/2013 15:18, Jagan Teki wrote:
This does not scale. What if a (trivial) patch touches 10 drivers?
Yes, you are correct if the changes is common to across the all drivers or If the changes which are more in 10 spi drivers out of 20 or > 10 for your example - use "spi: " and I think it's good to use above syntax as you changesd two drivers.
I am just showing the possibilities of using the coding guidelines, nothing more.
There is another issue: patman recognizes each <subpart>: and checks for an alias to find the e-mail to send patches, and stops to go on if it is not found. In this example, you can use patman if there is an entry for spi: (and this is you as SPI custodian) and also for mxc_spi:, and usually there is no maintainer for each driver. I agree that using a long chain does not scale, and I think the hidden rule to use subsystem: <subject> works well in most cases, Axel did in this patchset.
Regards, Stefano

On 14/06/13 09:09 AM, Axel Lin wrote:
2013/6/14 Jagan Tekijagannadh.teki@gmail.com:
Hi,
IMHO: Can you please use the proper commit header prefix.
I am just sharing my thoughts, ignore this if you know it already. I followed below syntax. "<main_module>:<sub_module>:<sub_sub_module>:<COMMIT_HEADER>"
Ex: for this commit (seems like this commit changes two drivers) spi: cf_qspi | mxc_spi: Use DIV_ROUND_UP at appropriate places
This does not scale. What if a (trivial) patch touches 10 drivers?
Depends how trivial it is I guess. I can see benefits to both approaches.
This one is simple enough that I am okay if it is grouped.
Just for my own education, how do I correctly ACK just the part I know (mcf_qspi in this case) in a grouped patch? Is there a way to automate my "Signed-off-by:" via e-mail?
-- Richard Retanubun
if you send same changes individually. spi: cf_qspi: Use DIV_ROUND_UP at appropriate places spi: mxc_spi: Use DIV_ROUND_UP at appropriate places
I was thinking doing so may add maintainer's burden. (well for this case with 2 patches, it's not a problem) But since you prefer sending a fix per driver, I'll resend the patches.
On Fri, Jun 14, 2013 at 1:29 PM, Axel Linaxel.lin@ingics.com wrote:
This change slightly improves readability.
Signed-off-by: Axel Linaxel.lin@ingics.com
drivers/spi/cf_qspi.c | 2 +- drivers/spi/mxc_spi.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cf_qspi.c b/drivers/spi/cf_qspi.c index a37ac4e..06bcf91 100644 --- a/drivers/spi/cf_qspi.c +++ b/drivers/spi/cf_qspi.c @@ -171,7 +171,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, volatile qspi_t *qspi = dev->regs; u8 *txbuf = (u8 *)dout; u8 *rxbuf = (u8 *)din;
u32 count = ((bitlen / 8) + (bitlen % 8 ? 1 : 0));
Was this equivalent to (bitlen + 7) / 8 ?
Yes. And that is the point of this patch ( to handle divide-round-up in a uniform way).
-- Thanks, Jagan.

On Fri, Jun 14, 2013 at 6:54 PM, Richard Retanubun richardretanubun@ruggedcom.com wrote:
On 14/06/13 09:09 AM, Axel Lin wrote:
2013/6/14 Jagan Tekijagannadh.teki@gmail.com:
Hi,
IMHO: Can you please use the proper commit header prefix.
I am just sharing my thoughts, ignore this if you know it already. I followed below syntax. "<main_module>:<sub_module>:<sub_sub_module>:<COMMIT_HEADER>"
Ex: for this commit (seems like this commit changes two drivers) spi: cf_qspi | mxc_spi: Use DIV_ROUND_UP at appropriate places
This does not scale. What if a (trivial) patch touches 10 drivers?
Depends how trivial it is I guess. I can see benefits to both approaches.
This one is simple enough that I am okay if it is grouped.
Just for my own education, how do I correctly ACK just the part I know (mcf_qspi in this case) in a grouped patch? Is there a way to automate my "Signed-off-by:" via e-mail?
Usually normal mail server like gmail or outlook doesn' update these s-o-b or a-b use thunderbread mail logic, that will automate when u did these changes.
-- Thanks, Jagan.
participants (4)
-
Axel Lin
-
Jagan Teki
-
Richard Retanubun
-
Stefano Babic