[PATCH net-next 0/5] net: dsa: lan9303: Linting

[PATCH net-next 0/5] net: dsa: lan9303: Linting

[Egil Hjelmeland]

This series is non-functional. 
 - Correct some errors in comments and documentation.
Remove scripts/checkpatch.pl WARNINGs and most CHECKs:
 - Replace msleep(1) with usleep_range()
 - Remove unnecessary parentheses
 - Adjust indenting

Egil Hjelmeland (5):
  net: dsa: lan9303: Correct register names in comments
  net: dsa: lan9303: Fix syntax errors in device tree examples
  net: dsa: lan9303: Replace msleep(1) with usleep_range()
  net: dsa: lan9303: Remove unnecessary parentheses
  net: dsa: lan9303: Adjust indenting

 Documentation/devicetree/bindings/net/dsa/lan9303.txt | 4 ++--
 drivers/net/dsa/lan9303-core.c                        | 6 +++---
 drivers/net/dsa/lan9303_i2c.c                         | 2 +-
 drivers/net/dsa/lan9303_mdio.c                        | 2 +-
 include/linux/dsa/lan9303.h                           | 8 +++++---
 net/dsa/tag_lan9303.c                                 | 2 +-
 6 files changed, 13 insertions(+), 11 deletions(-)

-- 
2.11.0

[PATCH net-next 1/5] net: dsa: lan9303: Correct register names in comments

[Egil Hjelmeland]

Two comments refer to registers, but lack the LAN9303_ prefix.
Fix that.

Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
 include/linux/dsa/lan9303.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/linux/dsa/lan9303.h b/include/linux/dsa/lan9303.h
index 05d8d136baab..f48a85c377de 100644
--- a/include/linux/dsa/lan9303.h
+++ b/include/linux/dsa/lan9303.h
@@ -13,8 +13,8 @@ struct lan9303_phy_ops {
 #define LAN9303_NUM_ALR_RECORDS 512
 struct lan9303_alr_cache_entry {
 	u8  mac_addr[ETH_ALEN];
-	u8  port_map;           /* Bitmap of ports. Zero if unused entry */
-	u8  stp_override;       /* non zero if set ALR_DAT1_AGE_OVERRID */
+	u8  port_map;         /* Bitmap of ports. Zero if unused entry */
+	u8  stp_override;     /* non zero if set LAN9303_ALR_DAT1_AGE_OVERRID */
 };
 
 struct lan9303 {
@@ -28,7 +28,9 @@ struct lan9303 {
 	struct mutex indirect_mutex; /* protect indexed register access */
 	const struct lan9303_phy_ops *ops;
 	bool is_bridged; /* true if port 1 and 2 are bridged */
-	u32 swe_port_state; /* remember SWE_PORT_STATE while not bridged */
+
+	/* remember LAN9303_SWE_PORT_STATE while not bridged */
+	u32 swe_port_state;
 	/* LAN9303 do not offer reading specific ALR entry. Cache all
 	 * static entries in a flat table
 	 **/
-- 
2.11.0

[PATCH net-next 2/5] net: dsa: lan9303: Fix syntax errors in device tree examples

[Egil Hjelmeland]

Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
 Documentation/devicetree/bindings/net/dsa/lan9303.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/lan9303.txt b/Documentation/devicetree/bindings/net/dsa/lan9303.txt
index 4448d063ddf6..464d6bf87605 100644
--- a/Documentation/devicetree/bindings/net/dsa/lan9303.txt
+++ b/Documentation/devicetree/bindings/net/dsa/lan9303.txt
@@ -52,7 +52,7 @@ I2C managed mode:
 
 			port@1 { /* external port 1 */
 				reg = <1>;
-				label = "lan1;
+				label = "lan1";
 			};
 
 			port@2 { /* external port 2 */
@@ -89,7 +89,7 @@ MDIO managed mode:
 
 					port@1 { /* external port 1 */
 						reg = <1>;
-						label = "lan1;
+						label = "lan1";
 					};
 
 					port@2 { /* external port 2 */
-- 
2.11.0

[PATCH net-next 3/5] net: dsa: lan9303: Replace msleep(1) with usleep_range()

[Egil Hjelmeland]

Remove scripts/checkpatch.pl WARNING by replacing msleep(1) with usleep_range()

Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
 drivers/net/dsa/lan9303-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index c4afc8f1a66d..70ecd18a5e7d 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -284,7 +284,7 @@ static int lan9303_indirect_phy_wait_for_completion(struct lan9303 *chip)
 		}
 		if (!(reg & LAN9303_PMI_ACCESS_MII_BUSY))
 			return 0;
-		msleep(1);
+		usleep_range(1000, 2000);
 	}
 
 	return -EIO;
@@ -376,7 +376,7 @@ static int lan9303_switch_wait_for_completion(struct lan9303 *chip)
 		}
 		if (!(reg & LAN9303_SWITCH_CSR_CMD_BUSY))
 			return 0;
-		msleep(1);
+		usleep_range(1000, 2000);
 	}
 
 	return -EIO;
-- 
2.11.0

[PATCH net-next 4/5] net: dsa: lan9303: Remove unnecessary parentheses

[Egil Hjelmeland]

Remove scripts/checkpatch.pl CHECKs by remove unnecessary parentheses

Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
 drivers/net/dsa/lan9303-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 70ecd18a5e7d..b9a95f542f65 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -483,7 +483,7 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
 		return reg;
 	}
 
-	if ((reg != 0) && (reg != 0xffff))
+	if (reg != 0 && reg != 0xffff)
 		chip->phy_addr_sel_strap = 1;
 	else
 		chip->phy_addr_sel_strap = 0;
-- 
2.11.0

Re: [PATCH net-next 4/5] net: dsa: lan9303: Remove unnecessary parentheses

[Joe Perches]

On Fri, 2017-11-03 at 11:55 +0100, Egil Hjelmeland wrote:
> Remove scripts/checkpatch.pl CHECKs by remove unnecessary parentheses
[]
> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
[]
> @@ -483,7 +483,7 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
>  		return reg;
>  	}
>  
> -	if ((reg != 0) && (reg != 0xffff))
> +	if (reg != 0 && reg != 0xffff)
>  		chip->phy_addr_sel_strap = 1;
>  	else
>  		chip->phy_addr_sel_strap = 0;

phy_addr_sel_strap is currently bool.

If this is to be changed, it should be set
true or false.

My preference would be:

	chip->phy_addr_sel_strap = (reg != 0 && reg != 0xffff);

But perhaps its bool type should be converted
to int as this phy_addr_sel_strap is used as
int several times.

$ git grep phy_addr_sel_strap
drivers/net/dsa/lan9303-core.c: /* depending on the 'phy_addr_sel_strap' setting, the three phys are
drivers/net/dsa/lan9303-core.c:  * 'phy_addr_sel_strap' setting directly, so we need a test, which
drivers/net/dsa/lan9303-core.c:  * Special reg 18 of phy 3 reads as 0x0000, if 'phy_addr_sel_strap' is 0
drivers/net/dsa/lan9303-core.c:  * 0x0000, which means 'phy_addr_sel_strap' is 1 and the IDs are 1-2-3.
drivers/net/dsa/lan9303-core.c:         chip->phy_addr_sel_strap = 1;
drivers/net/dsa/lan9303-core.c:         chip->phy_addr_sel_strap = 0;
drivers/net/dsa/lan9303-core.c:         chip->phy_addr_sel_strap ? "1-2-3" : "0-1-2");
drivers/net/dsa/lan9303-core.c: int phy_base = chip->phy_addr_sel_strap;
drivers/net/dsa/lan9303-core.c: int phy_base = chip->phy_addr_sel_strap;
drivers/net/dsa/lan9303-core.c: if (port == chip->phy_addr_sel_strap) {
drivers/net/dsa/lan9303-core.c:         lan9303_phy_write(ds, chip->phy_addr_sel_strap + port,
drivers/net/dsa/lan9303-core.c: chip->ds->phys_mii_mask = chip->phy_addr_sel_strap ? 0xe : 0x7;
include/linux/dsa/lan9303.h:    bool phy_addr_sel_strap;

Re: [PATCH net-next 4/5] net: dsa: lan9303: Remove unnecessary parentheses

[Egil Hjelmeland]

On 03. nov. 2017 15:11, Joe Perches wrote:
> On Fri, 2017-11-03 at 11:55 +0100, Egil Hjelmeland wrote:
>> Remove scripts/checkpatch.pl CHECKs by remove unnecessary parentheses
> []
>> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> []
>> @@ -483,7 +483,7 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
>>   		return reg;
>>   	}
>>   
>> -	if ((reg != 0) && (reg != 0xffff))
>> +	if (reg != 0 && reg != 0xffff)
>>   		chip->phy_addr_sel_strap = 1;
>>   	else
>>   		chip->phy_addr_sel_strap = 0;
> 
> phy_addr_sel_strap is currently bool.
> 
> If this is to be changed, it should be set
> true or false.
> 
> My preference would be:
> 
> 	chip->phy_addr_sel_strap = (reg != 0 && reg != 0xffff);
> 
> But perhaps its bool type should be converted
> to int as this phy_addr_sel_strap is used as
> int several times.
> 

Hi Joe

I had not noticed that phy_addr_sel_strap is bool. I agree that is
misleading. Your suggestion is perhaps a bit "magic", but on the other
hand the magic is explained well in the comment above.

If there are no disagreements, I can do a v2 with that.

And thanks for teaching me about "git grep"!

Cheers
Egil

Re: [PATCH net-next 4/5] net: dsa: lan9303: Remove unnecessary parentheses

[Vivien Didelot]

Hi Egil,

Egil Hjelmeland <privat@egil-hjelmeland.no> writes:

> If there are no disagreements, I can do a v2 with that.
>
> And thanks for teaching me about "git grep"!

If you send a v2, you may want to address the other parenthesis
alignment issues found when running ./scripts/checkpatch -f on the
lan9303* files.

Applying this gives you a few more: https://patchwork.kernel.org/patch/10014913/

(you can also add my Reviewed-by tag on patches you didn't touch.)


Thanks,

        Vivien

Re: [PATCH net-next 4/5] net: dsa: lan9303: Remove unnecessary parentheses

[Egil Hjelmeland]

On 03. nov. 2017 15:54, Vivien Didelot wrote:
> Hi Egil,
> 
> Egil Hjelmeland <privat@egil-hjelmeland.no> writes:
> 
> If you send a v2, you may want to address the other parenthesis
> alignment issues found when running ./scripts/checkpatch -f on the
> lan9303* files.
> 

There is just one remaining alignment issue. Removing that would
require introducing an extra variable just for that purpose. I
don't think that makes the code more readable. So I will not do it.
If anybody else want to do it, fine, I will just watch in silence.

> Applying this gives you a few more: https://patchwork.kernel.org/patch/10014913/
> 
> (you can also add my Reviewed-by tag on patches you didn't touch.)
> 
> 
> Thanks,
> 
>          Vivien
> 

Egil

Re: [PATCH net-next 4/5] net: dsa: lan9303: Remove unnecessary parentheses

[Egil Hjelmeland]

On 03. nov. 2017 15:35, Egil Hjelmeland wrote:
> On 03. nov. 2017 15:11, Joe Perches wrote:
>> On Fri, 2017-11-03 at 11:55 +0100, Egil Hjelmeland wrote:
>>> Remove scripts/checkpatch.pl CHECKs by remove unnecessary parentheses
>> []
>>> diff --git a/drivers/net/dsa/lan9303-core.c 
>>> b/drivers/net/dsa/lan9303-core.c
>> []
>>> @@ -483,7 +483,7 @@ static int lan9303_detect_phy_setup(struct 
>>> lan9303 *chip)
>>>           return reg;
>>>       }
>>> -    if ((reg != 0) && (reg != 0xffff))
>>> +    if (reg != 0 && reg != 0xffff)
>>>           chip->phy_addr_sel_strap = 1;
>>>       else
>>>           chip->phy_addr_sel_strap = 0;
>>
>> phy_addr_sel_strap is currently bool.
>>
>> If this is to be changed, it should be set
>> true or false.
>>
>> My preference would be:
>>
>>     chip->phy_addr_sel_strap = (reg != 0 && reg != 0xffff);
>>
>> But perhaps its bool type should be converted
>> to int as this phy_addr_sel_strap is used as
>> int several times.
>>
> 
> Hi Joe
> 
> I had not noticed that phy_addr_sel_strap is bool. I agree that is
> misleading. Your suggestion is perhaps a bit "magic", but on the other
> hand the magic is explained well in the comment above.
> 
> If there are no disagreements, I can do a v2 with that.
> 
> And thanks for teaching me about "git grep"!
> 
> Cheers
> Egil
> 
Hi
I changed my mind slightly. I will just remove patch 4 in v2. In stead
deal with phy_addr_sel_strap in a separate post.

Because I think I want to rename phy_addr_sel_strap as well, plus some
other simplification. So starting to creep out of the "linting" scope.

Egil

[PATCH net-next 5/5] net: dsa: lan9303: Adjust indenting

[Egil Hjelmeland]

Remove scripts/checkpatch.pl CHECKs by adjusting indenting.

Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
 drivers/net/dsa/lan9303_i2c.c  | 2 +-
 drivers/net/dsa/lan9303_mdio.c | 2 +-
 net/dsa/tag_lan9303.c          | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/lan9303_i2c.c b/drivers/net/dsa/lan9303_i2c.c
index 24ec20f7f444..909a7e864246 100644
--- a/drivers/net/dsa/lan9303_i2c.c
+++ b/drivers/net/dsa/lan9303_i2c.c
@@ -50,7 +50,7 @@ static int lan9303_i2c_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	sw_dev->chip.regmap = devm_regmap_init_i2c(client,
-						&lan9303_i2c_regmap_config);
+						   &lan9303_i2c_regmap_config);
 	if (IS_ERR(sw_dev->chip.regmap)) {
 		ret = PTR_ERR(sw_dev->chip.regmap);
 		dev_err(&client->dev, "Failed to allocate register map: %d\n",
diff --git a/drivers/net/dsa/lan9303_mdio.c b/drivers/net/dsa/lan9303_mdio.c
index 0bc56b9900f9..cc9c2ea1c4fe 100644
--- a/drivers/net/dsa/lan9303_mdio.c
+++ b/drivers/net/dsa/lan9303_mdio.c
@@ -116,7 +116,7 @@ static int lan9303_mdio_probe(struct mdio_device *mdiodev)
 		return -ENOMEM;
 
 	sw_dev->chip.regmap = devm_regmap_init(&mdiodev->dev, NULL, sw_dev,
-						&lan9303_mdio_regmap_config);
+					       &lan9303_mdio_regmap_config);
 	if (IS_ERR(sw_dev->chip.regmap)) {
 		ret = PTR_ERR(sw_dev->chip.regmap);
 		dev_err(&mdiodev->dev, "regmap init failed: %d\n", ret);
diff --git a/net/dsa/tag_lan9303.c b/net/dsa/tag_lan9303.c
index e526c8967b98..5ba01fc3c6ba 100644
--- a/net/dsa/tag_lan9303.c
+++ b/net/dsa/tag_lan9303.c
@@ -88,7 +88,7 @@ static struct sk_buff *lan9303_xmit(struct sk_buff *skb, struct net_device *dev)
 }
 
 static struct sk_buff *lan9303_rcv(struct sk_buff *skb, struct net_device *dev,
-			struct packet_type *pt)
+				   struct packet_type *pt)
 {
 	u16 *lan9303_tag;
 	unsigned int source_port;
-- 
2.11.0

Re: [PATCH net-next 0/5] net: dsa: lan9303: Linting

[Vivien Didelot]

Egil Hjelmeland <privat@egil-hjelmeland.no> writes:

> This series is non-functional. 
>  - Correct some errors in comments and documentation.
> Remove scripts/checkpatch.pl WARNINGs and most CHECKs:
>  - Replace msleep(1) with usleep_range()
>  - Remove unnecessary parentheses
>  - Adjust indenting
>
> Egil Hjelmeland (5):
>   net: dsa: lan9303: Correct register names in comments
>   net: dsa: lan9303: Fix syntax errors in device tree examples
>   net: dsa: lan9303: Replace msleep(1) with usleep_range()
>   net: dsa: lan9303: Remove unnecessary parentheses
>   net: dsa: lan9303: Adjust indenting
>
>  Documentation/devicetree/bindings/net/dsa/lan9303.txt | 4 ++--
>  drivers/net/dsa/lan9303-core.c                        | 6 +++---
>  drivers/net/dsa/lan9303_i2c.c                         | 2 +-
>  drivers/net/dsa/lan9303_mdio.c                        | 2 +-
>  include/linux/dsa/lan9303.h                           | 8 +++++---
>  net/dsa/tag_lan9303.c                                 | 2 +-
>  6 files changed, 13 insertions(+), 11 deletions(-)

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>


Thanks!

        Vivien