* [PATCH net] net: phy: dp83867: Fix various styling and space issues
@ 2020-09-03 14:15 Dan Murphy
2020-09-03 16:34 ` Florian Fainelli
0 siblings, 1 reply; 6+ messages in thread
From: Dan Murphy @ 2020-09-03 14:15 UTC (permalink / raw)
To: davem, andrew, f.fainelli, hkallweit1; +Cc: netdev, linux-kernel, Dan Murphy
Fix spacing issues reported for misaligned switch..case and extra new
lines.
Also updated the file header to comply with networking commet style.
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
drivers/net/phy/dp83867.c | 47 ++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 25 deletions(-)
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index cd7032628a28..f182a8d767c6 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -1,6 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
-/*
- * Driver for the Texas Instruments DP83867 PHY
+/* Driver for the Texas Instruments DP83867 PHY
*
* Copyright (C) 2015 Texas Instruments Inc.
*/
@@ -35,7 +34,7 @@
#define DP83867_CFG4_SGMII_ANEG_MASK (BIT(5) | BIT(6))
#define DP83867_CFG4_SGMII_ANEG_TIMER_11MS (3 << 5)
#define DP83867_CFG4_SGMII_ANEG_TIMER_800US (2 << 5)
-#define DP83867_CFG4_SGMII_ANEG_TIMER_2US (1 << 5)
+#define DP83867_CFG4_SGMII_ANEG_TIMER_2US BIT(5)
#define DP83867_CFG4_SGMII_ANEG_TIMER_16MS (0 << 5)
#define DP83867_RGMIICTL 0x0032
@@ -113,7 +112,6 @@
#define DP83867_RGMII_RX_CLK_DELAY_SHIFT 0
#define DP83867_RGMII_RX_CLK_DELAY_INV (DP83867_RGMII_RX_CLK_DELAY_MAX + 1)
-
/* IO_MUX_CFG bits */
#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MASK 0x1f
#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX 0x0
@@ -384,22 +382,22 @@ static int dp83867_set_downshift(struct phy_device *phydev, u8 cnt)
DP83867_DOWNSHIFT_EN);
switch (cnt) {
- case DP83867_DOWNSHIFT_1_COUNT:
- count = DP83867_DOWNSHIFT_1_COUNT_VAL;
- break;
- case DP83867_DOWNSHIFT_2_COUNT:
- count = DP83867_DOWNSHIFT_2_COUNT_VAL;
- break;
- case DP83867_DOWNSHIFT_4_COUNT:
- count = DP83867_DOWNSHIFT_4_COUNT_VAL;
- break;
- case DP83867_DOWNSHIFT_8_COUNT:
- count = DP83867_DOWNSHIFT_8_COUNT_VAL;
- break;
- default:
- phydev_err(phydev,
- "Downshift count must be 1, 2, 4 or 8\n");
- return -EINVAL;
+ case DP83867_DOWNSHIFT_1_COUNT:
+ count = DP83867_DOWNSHIFT_1_COUNT_VAL;
+ break;
+ case DP83867_DOWNSHIFT_2_COUNT:
+ count = DP83867_DOWNSHIFT_2_COUNT_VAL;
+ break;
+ case DP83867_DOWNSHIFT_4_COUNT:
+ count = DP83867_DOWNSHIFT_4_COUNT_VAL;
+ break;
+ case DP83867_DOWNSHIFT_8_COUNT:
+ count = DP83867_DOWNSHIFT_8_COUNT_VAL;
+ break;
+ default:
+ phydev_err(phydev,
+ "Downshift count must be 1, 2, 4 or 8\n");
+ return -EINVAL;
}
val = DP83867_DOWNSHIFT_EN;
@@ -411,7 +409,7 @@ static int dp83867_set_downshift(struct phy_device *phydev, u8 cnt)
}
static int dp83867_get_tunable(struct phy_device *phydev,
- struct ethtool_tunable *tuna, void *data)
+ struct ethtool_tunable *tuna, void *data)
{
switch (tuna->id) {
case ETHTOOL_PHY_DOWNSHIFT:
@@ -422,7 +420,7 @@ static int dp83867_get_tunable(struct phy_device *phydev,
}
static int dp83867_set_tunable(struct phy_device *phydev,
- struct ethtool_tunable *tuna, const void *data)
+ struct ethtool_tunable *tuna, const void *data)
{
switch (tuna->id) {
case ETHTOOL_PHY_DOWNSHIFT:
@@ -524,11 +522,10 @@ static int dp83867_of_init(struct phy_device *phydev)
dp83867->io_impedance = -1; /* leave at default */
dp83867->rxctrl_strap_quirk = of_property_read_bool(of_node,
- "ti,dp83867-rxctrl-strap-quirk");
+ "ti,dp83867-rxctrl-strap-quirk");
dp83867->sgmii_ref_clk_en = of_property_read_bool(of_node,
- "ti,sgmii-ref-clock-output-enable");
-
+ "ti,sgmii-ref-clock-output-enable");
dp83867->rx_id_delay = DP83867_RGMII_RX_CLK_DELAY_INV;
ret = of_property_read_u32(of_node, "ti,rx-internal-delay",
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net] net: phy: dp83867: Fix various styling and space issues
2020-09-03 14:15 [PATCH net] net: phy: dp83867: Fix various styling and space issues Dan Murphy
@ 2020-09-03 16:34 ` Florian Fainelli
2020-09-03 16:41 ` Dan Murphy
0 siblings, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2020-09-03 16:34 UTC (permalink / raw)
To: Dan Murphy, davem, andrew, hkallweit1; +Cc: netdev, linux-kernel
On 9/3/2020 7:15 AM, Dan Murphy wrote:
> Fix spacing issues reported for misaligned switch..case and extra new
> lines.
>
> Also updated the file header to comply with networking commet style.
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> drivers/net/phy/dp83867.c | 47 ++++++++++++++++++---------------------
> 1 file changed, 22 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> index cd7032628a28..f182a8d767c6 100644
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c
> @@ -1,6 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0
> -/*
> - * Driver for the Texas Instruments DP83867 PHY
> +/* Driver for the Texas Instruments DP83867 PHY
> *
> * Copyright (C) 2015 Texas Instruments Inc.
> */
> @@ -35,7 +34,7 @@
> #define DP83867_CFG4_SGMII_ANEG_MASK (BIT(5) | BIT(6))
> #define DP83867_CFG4_SGMII_ANEG_TIMER_11MS (3 << 5)
> #define DP83867_CFG4_SGMII_ANEG_TIMER_800US (2 << 5)
> -#define DP83867_CFG4_SGMII_ANEG_TIMER_2US (1 << 5)
> +#define DP83867_CFG4_SGMII_ANEG_TIMER_2US BIT(5)
Now the definitions are inconsistent, you would want to drop this one
and stick to the existing style.
The rest of the changes look good, so with that fixed, and the subject
correct to "net-next" (this is no bug fix material), you can add:
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] net: phy: dp83867: Fix various styling and space issues
2020-09-03 16:34 ` Florian Fainelli
@ 2020-09-03 16:41 ` Dan Murphy
2020-09-03 16:51 ` Florian Fainelli
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Dan Murphy @ 2020-09-03 16:41 UTC (permalink / raw)
To: Florian Fainelli, davem, andrew, hkallweit1; +Cc: netdev, linux-kernel
Florian
On 9/3/20 11:34 AM, Florian Fainelli wrote:
>
>
> On 9/3/2020 7:15 AM, Dan Murphy wrote:
>> Fix spacing issues reported for misaligned switch..case and extra new
>> lines.
>>
>> Also updated the file header to comply with networking commet style.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>> drivers/net/phy/dp83867.c | 47 ++++++++++++++++++---------------------
>> 1 file changed, 22 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
>> index cd7032628a28..f182a8d767c6 100644
>> --- a/drivers/net/phy/dp83867.c
>> +++ b/drivers/net/phy/dp83867.c
>> @@ -1,6 +1,5 @@
>> // SPDX-License-Identifier: GPL-2.0
>> -/*
>> - * Driver for the Texas Instruments DP83867 PHY
>> +/* Driver for the Texas Instruments DP83867 PHY
>> *
>> * Copyright (C) 2015 Texas Instruments Inc.
>> */
>> @@ -35,7 +34,7 @@
>> #define DP83867_CFG4_SGMII_ANEG_MASK (BIT(5) | BIT(6))
>> #define DP83867_CFG4_SGMII_ANEG_TIMER_11MS (3 << 5)
>> #define DP83867_CFG4_SGMII_ANEG_TIMER_800US (2 << 5)
>> -#define DP83867_CFG4_SGMII_ANEG_TIMER_2US (1 << 5)
>> +#define DP83867_CFG4_SGMII_ANEG_TIMER_2US BIT(5)
>
> Now the definitions are inconsistent, you would want to drop this one
> and stick to the existing style.
OK I was a little conflicted making that change due to the reasons you
mentioned. But if that is an acceptable warning I am ok with it.
>
> The rest of the changes look good, so with that fixed, and the subject
> correct to "net-next" (this is no bug fix material), you can add:
>
I will have to reapply this to the net-next to make sure it applies
cleanly there. But not an issue.
Dan
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] net: phy: dp83867: Fix various styling and space issues
2020-09-03 16:41 ` Dan Murphy
@ 2020-09-03 16:51 ` Florian Fainelli
2020-09-03 16:54 ` Joe Perches
2020-09-03 20:42 ` Andrew Lunn
2 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2020-09-03 16:51 UTC (permalink / raw)
To: Dan Murphy, davem, andrew, hkallweit1; +Cc: netdev, linux-kernel
On 9/3/2020 9:41 AM, Dan Murphy wrote:
> Florian
>
> On 9/3/20 11:34 AM, Florian Fainelli wrote:
>>
>>
>> On 9/3/2020 7:15 AM, Dan Murphy wrote:
>>> Fix spacing issues reported for misaligned switch..case and extra new
>>> lines.
>>>
>>> Also updated the file header to comply with networking commet style.
>>>
>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>> ---
>>> drivers/net/phy/dp83867.c | 47 ++++++++++++++++++---------------------
>>> 1 file changed, 22 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
>>> index cd7032628a28..f182a8d767c6 100644
>>> --- a/drivers/net/phy/dp83867.c
>>> +++ b/drivers/net/phy/dp83867.c
>>> @@ -1,6 +1,5 @@
>>> // SPDX-License-Identifier: GPL-2.0
>>> -/*
>>> - * Driver for the Texas Instruments DP83867 PHY
>>> +/* Driver for the Texas Instruments DP83867 PHY
>>> *
>>> * Copyright (C) 2015 Texas Instruments Inc.
>>> */
>>> @@ -35,7 +34,7 @@
>>> #define DP83867_CFG4_SGMII_ANEG_MASK (BIT(5) | BIT(6))
>>> #define DP83867_CFG4_SGMII_ANEG_TIMER_11MS (3 << 5)
>>> #define DP83867_CFG4_SGMII_ANEG_TIMER_800US (2 << 5)
>>> -#define DP83867_CFG4_SGMII_ANEG_TIMER_2US (1 << 5)
>>> +#define DP83867_CFG4_SGMII_ANEG_TIMER_2US BIT(5)
>>
>> Now the definitions are inconsistent, you would want to drop this one
>> and stick to the existing style.
>
> OK I was a little conflicted making that change due to the reasons you
> mentioned. But if that is an acceptable warning I am ok with it.
IMHO, if the are no obvious incorrect styles, and using 1 << x is not,
it just may not be the preferred style (and there is quite frankly a ton
of those patterns in the kernel), then ignoring the checkpatch.pl
warning is fine. After all this is a tool, not an absolute truth by any
means, but again, others may disagree.
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] net: phy: dp83867: Fix various styling and space issues
2020-09-03 16:41 ` Dan Murphy
2020-09-03 16:51 ` Florian Fainelli
@ 2020-09-03 16:54 ` Joe Perches
2020-09-03 20:42 ` Andrew Lunn
2 siblings, 0 replies; 6+ messages in thread
From: Joe Perches @ 2020-09-03 16:54 UTC (permalink / raw)
To: Dan Murphy, Florian Fainelli, davem, andrew, hkallweit1
Cc: netdev, linux-kernel
On Thu, 2020-09-03 at 11:41 -0500, Dan Murphy wrote:
> On 9/3/20 11:34 AM, Florian Fainelli wrote:
> > On 9/3/2020 7:15 AM, Dan Murphy wrote:
> > > Fix spacing issues reported for misaligned switch..case and extra new
> > > lines.
[]
> > > diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
[]
> > > @@ -35,7 +34,7 @@
> > > #define DP83867_CFG4_SGMII_ANEG_MASK (BIT(5) | BIT(6))
> > > #define DP83867_CFG4_SGMII_ANEG_TIMER_11MS (3 << 5)
> > > #define DP83867_CFG4_SGMII_ANEG_TIMER_800US (2 << 5)
> > > -#define DP83867_CFG4_SGMII_ANEG_TIMER_2US (1 << 5)
> > > +#define DP83867_CFG4_SGMII_ANEG_TIMER_2US BIT(5)
> > Now the definitions are inconsistent, you would want to drop this one
> > and stick to the existing style.
>
> OK I was a little conflicted making that change due to the reasons you
> mentioned. But if that is an acceptable warning I am ok with it.
Maybe use GENMASK for DP83867_CFG4_SGMII_ANEG_MASK instead.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] net: phy: dp83867: Fix various styling and space issues
2020-09-03 16:41 ` Dan Murphy
2020-09-03 16:51 ` Florian Fainelli
2020-09-03 16:54 ` Joe Perches
@ 2020-09-03 20:42 ` Andrew Lunn
2 siblings, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2020-09-03 20:42 UTC (permalink / raw)
To: Dan Murphy; +Cc: Florian Fainelli, davem, hkallweit1, netdev, linux-kernel
> > > #define DP83867_CFG4_SGMII_ANEG_MASK (BIT(5) | BIT(6))
> > > #define DP83867_CFG4_SGMII_ANEG_TIMER_11MS (3 << 5)
> > > #define DP83867_CFG4_SGMII_ANEG_TIMER_800US (2 << 5)
> > > -#define DP83867_CFG4_SGMII_ANEG_TIMER_2US (1 << 5)
> > > +#define DP83867_CFG4_SGMII_ANEG_TIMER_2US BIT(5)
> >
> > Now the definitions are inconsistent, you would want to drop this one
> > and stick to the existing style.
>
> OK I was a little conflicted making that change due to the reasons you
> mentioned. But if that is an acceptable warning I am ok with it.
Hi Dan
I work around this by using hex:
#define DP83867_CFG4_SGMII_ANEG_TIMER_11MS (0x3 << 5)
#define DP83867_CFG4_SGMII_ANEG_TIMER_800US (0x2 << 5)
#define DP83867_CFG4_SGMII_ANEG_TIMER_2US (0x1 << 5)
checkpatch does not complain about that.
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-03 20:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-03 14:15 [PATCH net] net: phy: dp83867: Fix various styling and space issues Dan Murphy
2020-09-03 16:34 ` Florian Fainelli
2020-09-03 16:41 ` Dan Murphy
2020-09-03 16:51 ` Florian Fainelli
2020-09-03 16:54 ` Joe Perches
2020-09-03 20:42 ` Andrew Lunn
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).