spi: Use DIV_ROUND_UP at appropriate places

PATCH New Axel Lin 4 participants 7 messages Jun 14, 2013 – Jun 17, 2013

Subsystems (2)

Arm i.MX and Layerscape SPI

lore

Share & tools

lore

Axel Lin

author

2013-06-14 07:59 UTC #0 lore raw

PATCH 1/1 spi: Use DIV_ROUND_UP at appropriate places

This change slightly improves readability.

Signed-off-by Axel Lin <[email protected]>

---
 drivers/spi/cf_qspi.c | 2 +-
 drivers/spi/mxc_spi.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

drivers/spi/cf_qspi.c +1 -1

index a37ac4e..06bcf91 100644
@@ -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 */

drivers/spi/mxc_spi.c +3 -3

index 5bed858..2ea3228 100644
@@ -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(&regs->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;

Signature · 1 line

1.8.1.2

Jagan Teki

2013-06-14 12:20 UTC #1 lore raw

PATCH 1/1 spi: Use DIV_ROUND_UP at appropriate places

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

Quoted 13 lines · Axel Lin

This change slightly improves readability.
Signed-off-by Axel Lin <[email protected]>
---
 drivers/spi/cf_qspi.c | 2 +-
 drivers/spi/mxc_spi.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/spi/cf_qspi.c +0 -0
index a37ac4e..06bcf91 100644

drivers/spi/cf_qspi.c int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, · line 174

Quoted 4 lines

drivers/spi/cf_qspi.c +0 -1

@@ -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.

Axel Lin

author

2013-06-14 13:09 UTC #2 lore raw

PATCH 1/1 spi: Use DIV_ROUND_UP at appropriate places

2013/6/14 Jagan Teki <[email protected]>:

Quoted 11 lines

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?

Quoted 4 lines

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.

Quoted 10 lines · Axel Lin

Quoted 8 lines
This change slightly improves readability.
Signed-off-by Axel Lin <[email protected]>
---
 drivers/spi/cf_qspi.c | 2 +-
 drivers/spi/mxc_spi.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

Quoted 11 lines

drivers/spi/cf_qspi.c int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, · line 174
Quoted 9 lines
drivers/spi/cf_qspi.c +0 -1
index a37ac4e..06bcf91 100644
@@ -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).

Quoted 4 lines

--
Thanks,
Jagan.

Jagan Teki

2013-06-14 13:18 UTC #3 lore raw

PATCH 1/1 spi: Use DIV_ROUND_UP at appropriate places

Quoted 10 lines · Axel Lin

2013/6/14 Jagan Teki <[email protected]>:
Quoted 9 lines
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>"

Quoted 5 lines

Quoted 2 lines
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.

Stefano Babic

2013-06-17 10:40 UTC #4 lore raw

PATCH 1/1 spi: Use DIV_ROUND_UP at appropriate places

Quoted 9 lines · Jagan Teki

Quoted 2 lines
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

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

Richard Retanubun

2013-06-14 13:24 UTC #5 lore raw

PATCH 1/1 spi: Use DIV_ROUND_UP at appropriate places

In reply to Axel Lin

Quoted 10 lines · Axel Lin

2013/6/14 Jagan Teki<[email protected]>:
Quoted 9 lines
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>"

Quoted 5 lines

Quoted 2 lines
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

Quoted 38 lines

Quoted 5 lines
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.
Quoted 10 lines · Axel Lin
Quoted 8 lines
This change slightly improves readability.
Signed-off-by Axel Lin<[email protected]>
---
  drivers/spi/cf_qspi.c | 2 +-
  drivers/spi/mxc_spi.c | 6 +++---
  2 files changed, 4 insertions(+), 4 deletions(-)
Quoted 11 lines
drivers/spi/cf_qspi.c int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, · line 174
Quoted 9 lines
drivers/spi/cf_qspi.c +0 -1
index a37ac4e..06bcf91 100644
@@ -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).
Quoted 4 lines
--
Thanks,
Jagan.

Jagan Teki

2013-06-14 13:28 UTC #6 lore raw

PATCH 1/1 spi: Use DIV_ROUND_UP at appropriate places

Quoted 20 lines · Richard Retanubun

Quoted 12 lines · Axel Lin
2013/6/14 Jagan Teki<[email protected]>:
Quoted 10 lines
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>"
Quoted 6 lines
Quoted 2 lines
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?

Quoted 7 lines

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.