Skip to contentSkip to conversation

Staging: octeon: Use preferred kernel type

PATCH Superseded aybuke ozdemir 2 participants 2 messages Oct 1, 2015 Staging

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 Staging: octeon: Use preferred kernel type

This patch "uint*_t" type instead of "u*" type was used. checkpatch.pl issue in ethernet.c file.

---
Changes since v1:
* change commit explain message.
* all uint*_t type changed in one commit.
* add my surname in user name part.

---
 drivers/staging/octeon/ethernet.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/staging/octeon/ethernet.c +4 -4
index 7274fda..c9183e0 100644
@@ -382,11 +382,11 @@ static int cvm_oct_set_mac_filter(struct net_device *dev)
 	    && (cvmx_helper_interface_get_mode(interface) !=
 		CVMX_HELPER_INTERFACE_MODE_SPI)) {
 		int i;
-		uint8_t *ptr = dev->dev_addr;
-		uint64_t mac = 0;
+		u8 *ptr = dev->dev_addr;
+		u64 mac = 0;
 
 		for (i = 0; i < 6; i++)
-			mac = (mac << 8) | (uint64_t)ptr[i];
+			mac = (mac << 8) | (u64)ptr[i];
 
 		gmx_cfg.u64 =
 		    cvmx_read_csr(CVMX_GMXX_PRTX_CFG(index, interface));
@@ -835,7 +835,7 @@ static int cvm_oct_probe(struct platform_device *pdev)
 				cvm_oct_device[priv->port] = dev;
 				fau -=
 				    cvmx_pko_get_num_queues(priv->port) *
-				    sizeof(uint32_t);
+				    sizeof(u32);
 				queue_delayed_work(cvm_oct_poll_queue,
 						&priv->port_periodic_work, HZ);
 			}
Signature · 1 line
2.1.4
#1 lore raw
Quoted 4 lines · On Thursday 01 October 2015 11:05:41 aybuke ozdemir wrote:

This patch "uint*_t" type instead of "u*" type was used. checkpatch.pl issue in ethernet.c file.

Each point is a bit better this time, but can be improved further:

Quoted 2 lines

Changes since v1:
* change commit explain message.

For the best changelog messages, start out by stating what the driver does before your patch, and why that is wrong. This is the most important part to justify why the patch should be included. Only in the last sentence of the patch, describe what you did instead, which can normally be really brief.

[In more complex cases than this when there are multiple ways to address the problem, it can make sense to also list alternative approaches and why you picked one over the others.]

Quoted 1 line

* all uint*_t type changed in one commit.

This is basically ok, but I'd go further here and do the same thing for the other three files using those types in the same directory. As a good rule, when you fix one problem, look around similar places (same file, same directory, same subsystem) for how common the problem is first, and then decide on how far you want to go. Finding the right balance takes a bit of experience, as you don't want the patches to become too complex or too simple.

The command

git grep -w 'u\?int[0-9]*_t' drivers/staging/octeon/

lists 5 files and 19 instances in this driver, which is large enough that you could do it as a series of five patches, but still small enough to do it as a single patch because the replacement is very obvious.

If you decide to do only one file initially, you can mention in the changelog (below the --- line) that you intend to address the problem in the other four files after this has been accepted. That avoids the obvious question why you didn't do them all at once.

Quoted 1 line

* add my surname in user name part.

The spelling of your name still seems wrong, I would expect that you use the same capitalization as most other people and write it as "Aybuke Ozdemir" or "Aybüke Özdemir" rather than "aybuke ozdemir", whichever you want people to use when they write about you.

Quoted 24 lines
---
 drivers/staging/octeon/ethernet.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/staging/octeon/ethernet.c +3 -3
index 7274fda..c9183e0 100644
@@ -382,11 +382,11 @@ static int cvm_oct_set_mac_filter(struct net_device *dev)
 	    && (cvmx_helper_interface_get_mode(interface) !=
 		CVMX_HELPER_INTERFACE_MODE_SPI)) {
 		int i;
-		uint8_t *ptr = dev->dev_addr;
-		uint64_t mac = 0;
+		u8 *ptr = dev->dev_addr;
+		u64 mac = 0;
 
 		for (i = 0; i < 6; i++)
-			mac = (mac << 8) | (uint64_t)ptr[i];
+			mac = (mac << 8) | (u64)ptr[i];
 
 		gmx_cfg.u64 =
 		    cvmx_read_csr(CVMX_GMXX_PRTX_CFG(index, interface));

drivers/staging/octeon/ethernet.c static int cvm_oct_probe(struct platform_device *pdev) · line 841

Quoted 8 lines
drivers/staging/octeon/ethernet.c +1 -1
@@ -835,7 +835,7 @@ static int cvm_oct_probe(struct platform_device *pdev)
 				cvm_oct_device[priv->port] = dev;
 				fau -=
 				    cvmx_pko_get_num_queues(priv->port) *
-				    sizeof(uint32_t);
+				    sizeof(u32);
 				queue_delayed_work(cvm_oct_poll_queue,
 						&priv->port_periodic_work, HZ);
 			}

The changes look completely fine.

Arnd