2023-06-26 19:27:45

by Jerry Ray

[permalink] [raw]
Subject: [PATCH] net: dsa: microchip: phy reg access 0x10-0x1f

With the recent patch to promote phy register accesses to 32-bits for the
range >=0x10, it is also necessary to expand the allowed register address
table for the affected devices. These three register sets use
ksz9477_dev_ops and are therefore affected by the change. The address
ranges 0xN120-0xN13f map to the phy register address 0x10-0x1f. There is
no reason to exclude any register accesses within this space.

on June 20, 2023
commit 5c844d57aa78 ("net: dsa: microchip: fix writes to phy registers >= 0x10")

Signed-off-by: Jerry Ray <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 65 ++++++--------------------
1 file changed, 13 insertions(+), 52 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 813b91a816bb..b7ce48147a38 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -509,10 +509,7 @@ static const struct regmap_range ksz8563_valid_regs[] = {
regmap_reg_range(0x1030, 0x1030),
regmap_reg_range(0x1100, 0x1111),
regmap_reg_range(0x111a, 0x111d),
- regmap_reg_range(0x1122, 0x1127),
- regmap_reg_range(0x112a, 0x112b),
- regmap_reg_range(0x1136, 0x1139),
- regmap_reg_range(0x113e, 0x113f),
+ regmap_reg_range(0x1120, 0x113f),
regmap_reg_range(0x1400, 0x1401),
regmap_reg_range(0x1403, 0x1403),
regmap_reg_range(0x1410, 0x1417),
@@ -539,10 +536,7 @@ static const struct regmap_range ksz8563_valid_regs[] = {
regmap_reg_range(0x2030, 0x2030),
regmap_reg_range(0x2100, 0x2111),
regmap_reg_range(0x211a, 0x211d),
- regmap_reg_range(0x2122, 0x2127),
- regmap_reg_range(0x212a, 0x212b),
- regmap_reg_range(0x2136, 0x2139),
- regmap_reg_range(0x213e, 0x213f),
+ regmap_reg_range(0x2120, 0x213f),
regmap_reg_range(0x2400, 0x2401),
regmap_reg_range(0x2403, 0x2403),
regmap_reg_range(0x2410, 0x2417),
@@ -635,10 +629,7 @@ static const struct regmap_range ksz9477_valid_regs[] = {
regmap_reg_range(0x1030, 0x1030),
regmap_reg_range(0x1100, 0x1115),
regmap_reg_range(0x111a, 0x111f),
- regmap_reg_range(0x1122, 0x1127),
- regmap_reg_range(0x112a, 0x112b),
- regmap_reg_range(0x1136, 0x1139),
- regmap_reg_range(0x113e, 0x113f),
+ regmap_reg_range(0x1120, 0x113f),
regmap_reg_range(0x1400, 0x1401),
regmap_reg_range(0x1403, 0x1403),
regmap_reg_range(0x1410, 0x1417),
@@ -669,10 +660,7 @@ static const struct regmap_range ksz9477_valid_regs[] = {
regmap_reg_range(0x2030, 0x2030),
regmap_reg_range(0x2100, 0x2115),
regmap_reg_range(0x211a, 0x211f),
- regmap_reg_range(0x2122, 0x2127),
- regmap_reg_range(0x212a, 0x212b),
- regmap_reg_range(0x2136, 0x2139),
- regmap_reg_range(0x213e, 0x213f),
+ regmap_reg_range(0x2120, 0x213f),
regmap_reg_range(0x2400, 0x2401),
regmap_reg_range(0x2403, 0x2403),
regmap_reg_range(0x2410, 0x2417),
@@ -703,10 +691,7 @@ static const struct regmap_range ksz9477_valid_regs[] = {
regmap_reg_range(0x3030, 0x3030),
regmap_reg_range(0x3100, 0x3115),
regmap_reg_range(0x311a, 0x311f),
- regmap_reg_range(0x3122, 0x3127),
- regmap_reg_range(0x312a, 0x312b),
- regmap_reg_range(0x3136, 0x3139),
- regmap_reg_range(0x313e, 0x313f),
+ regmap_reg_range(0x3120, 0x313f),
regmap_reg_range(0x3400, 0x3401),
regmap_reg_range(0x3403, 0x3403),
regmap_reg_range(0x3410, 0x3417),
@@ -737,10 +722,7 @@ static const struct regmap_range ksz9477_valid_regs[] = {
regmap_reg_range(0x4030, 0x4030),
regmap_reg_range(0x4100, 0x4115),
regmap_reg_range(0x411a, 0x411f),
- regmap_reg_range(0x4122, 0x4127),
- regmap_reg_range(0x412a, 0x412b),
- regmap_reg_range(0x4136, 0x4139),
- regmap_reg_range(0x413e, 0x413f),
+ regmap_reg_range(0x4120, 0x413f),
regmap_reg_range(0x4400, 0x4401),
regmap_reg_range(0x4403, 0x4403),
regmap_reg_range(0x4410, 0x4417),
@@ -771,10 +753,7 @@ static const struct regmap_range ksz9477_valid_regs[] = {
regmap_reg_range(0x5030, 0x5030),
regmap_reg_range(0x5100, 0x5115),
regmap_reg_range(0x511a, 0x511f),
- regmap_reg_range(0x5122, 0x5127),
- regmap_reg_range(0x512a, 0x512b),
- regmap_reg_range(0x5136, 0x5139),
- regmap_reg_range(0x513e, 0x513f),
+ regmap_reg_range(0x5120, 0x513f),
regmap_reg_range(0x5400, 0x5401),
regmap_reg_range(0x5403, 0x5403),
regmap_reg_range(0x5410, 0x5417),
@@ -897,10 +876,7 @@ static const struct regmap_range ksz9896_valid_regs[] = {
regmap_reg_range(0x1030, 0x1030),
regmap_reg_range(0x1100, 0x1115),
regmap_reg_range(0x111a, 0x111f),
- regmap_reg_range(0x1122, 0x1127),
- regmap_reg_range(0x112a, 0x112b),
- regmap_reg_range(0x1136, 0x1139),
- regmap_reg_range(0x113e, 0x113f),
+ regmap_reg_range(0x1120, 0x113f),
regmap_reg_range(0x1400, 0x1401),
regmap_reg_range(0x1403, 0x1403),
regmap_reg_range(0x1410, 0x1417),
@@ -927,10 +903,7 @@ static const struct regmap_range ksz9896_valid_regs[] = {
regmap_reg_range(0x2030, 0x2030),
regmap_reg_range(0x2100, 0x2115),
regmap_reg_range(0x211a, 0x211f),
- regmap_reg_range(0x2122, 0x2127),
- regmap_reg_range(0x212a, 0x212b),
- regmap_reg_range(0x2136, 0x2139),
- regmap_reg_range(0x213e, 0x213f),
+ regmap_reg_range(0x2120, 0x213f),
regmap_reg_range(0x2400, 0x2401),
regmap_reg_range(0x2403, 0x2403),
regmap_reg_range(0x2410, 0x2417),
@@ -957,10 +930,7 @@ static const struct regmap_range ksz9896_valid_regs[] = {
regmap_reg_range(0x3030, 0x3030),
regmap_reg_range(0x3100, 0x3115),
regmap_reg_range(0x311a, 0x311f),
- regmap_reg_range(0x3122, 0x3127),
- regmap_reg_range(0x312a, 0x312b),
- regmap_reg_range(0x3136, 0x3139),
- regmap_reg_range(0x313e, 0x313f),
+ regmap_reg_range(0x3120, 0x313f),
regmap_reg_range(0x3400, 0x3401),
regmap_reg_range(0x3403, 0x3403),
regmap_reg_range(0x3410, 0x3417),
@@ -987,10 +957,7 @@ static const struct regmap_range ksz9896_valid_regs[] = {
regmap_reg_range(0x4030, 0x4030),
regmap_reg_range(0x4100, 0x4115),
regmap_reg_range(0x411a, 0x411f),
- regmap_reg_range(0x4122, 0x4127),
- regmap_reg_range(0x412a, 0x412b),
- regmap_reg_range(0x4136, 0x4139),
- regmap_reg_range(0x413e, 0x413f),
+ regmap_reg_range(0x4120, 0x413f),
regmap_reg_range(0x4400, 0x4401),
regmap_reg_range(0x4403, 0x4403),
regmap_reg_range(0x4410, 0x4417),
@@ -1017,10 +984,7 @@ static const struct regmap_range ksz9896_valid_regs[] = {
regmap_reg_range(0x5030, 0x5030),
regmap_reg_range(0x5100, 0x5115),
regmap_reg_range(0x511a, 0x511f),
- regmap_reg_range(0x5122, 0x5127),
- regmap_reg_range(0x512a, 0x512b),
- regmap_reg_range(0x5136, 0x5139),
- regmap_reg_range(0x513e, 0x513f),
+ regmap_reg_range(0x5120, 0x513f),
regmap_reg_range(0x5400, 0x5401),
regmap_reg_range(0x5403, 0x5403),
regmap_reg_range(0x5410, 0x5417),
@@ -1047,10 +1011,7 @@ static const struct regmap_range ksz9896_valid_regs[] = {
regmap_reg_range(0x6030, 0x6030),
regmap_reg_range(0x6100, 0x6115),
regmap_reg_range(0x611a, 0x611f),
- regmap_reg_range(0x6122, 0x6127),
- regmap_reg_range(0x612a, 0x612b),
- regmap_reg_range(0x6136, 0x6139),
- regmap_reg_range(0x613e, 0x613f),
+ regmap_reg_range(0x6120, 0x613f),
regmap_reg_range(0x6300, 0x6301),
regmap_reg_range(0x6400, 0x6401),
regmap_reg_range(0x6403, 0x6403),
--
2.25.1



2023-06-26 20:00:59

by Horatiu Vultur

[permalink] [raw]
Subject: Re: [PATCH] net: dsa: microchip: phy reg access 0x10-0x1f

The 06/26/2023 11:20, Jerry Ray wrote:

Hi Jerry,

This seems like a patch for net-next which seems to be closed now.
Please hold back this patch until the window opens again in like 2 weeks.

Also you need to add in the subject which tree are you targeting.
In your case is net-next then it should be something like:
[PATCH net-next] net: dsa: microchip: phy reg access 0x10-0x1f

> With the recent patch to promote phy register accesses to 32-bits for the
> range >=0x10, it is also necessary to expand the allowed register address
> table for the affected devices. These three register sets use
> ksz9477_dev_ops and are therefore affected by the change. The address
> ranges 0xN120-0xN13f map to the phy register address 0x10-0x1f. There is
> no reason to exclude any register accesses within this space.
>
> on June 20, 2023
> commit 5c844d57aa78 ("net: dsa: microchip: fix writes to phy registers >= 0x10")

This is just a small thing but as you need to send a new version, you can
write something like this:
---
With the recent patch [0] to promote ...

[0] 5c844d57aa78 ("net: dsa: microchip: fix writes to phy registers >= 0x10")
---

Or:

---
With the commit 5c844d57aa78 ("net: dsa: microchip: fix writes to phy
registers >= 0x10") which promotes phy ...
---

Just to make it a little bit more clear that the commit that you posted
at the end refers to the patch that you mention to at the beginning of
the commit message.

>
> Signed-off-by: Jerry Ray <[email protected]>
> ---
> drivers/net/dsa/microchip/ksz_common.c | 65 ++++++--------------------
> 1 file changed, 13 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 813b91a816bb..b7ce48147a38 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -509,10 +509,7 @@ static const struct regmap_range ksz8563_valid_regs[] = {
> regmap_reg_range(0x1030, 0x1030),
> regmap_reg_range(0x1100, 0x1111),
> regmap_reg_range(0x111a, 0x111d),
> - regmap_reg_range(0x1122, 0x1127),
> - regmap_reg_range(0x112a, 0x112b),
> - regmap_reg_range(0x1136, 0x1139),
> - regmap_reg_range(0x113e, 0x113f),
> + regmap_reg_range(0x1120, 0x113f),
> regmap_reg_range(0x1400, 0x1401),
> regmap_reg_range(0x1403, 0x1403),
> regmap_reg_range(0x1410, 0x1417),
> @@ -539,10 +536,7 @@ static const struct regmap_range ksz8563_valid_regs[] = {
> regmap_reg_range(0x2030, 0x2030),
> regmap_reg_range(0x2100, 0x2111),
> regmap_reg_range(0x211a, 0x211d),
> - regmap_reg_range(0x2122, 0x2127),
> - regmap_reg_range(0x212a, 0x212b),
> - regmap_reg_range(0x2136, 0x2139),
> - regmap_reg_range(0x213e, 0x213f),
> + regmap_reg_range(0x2120, 0x213f),
> regmap_reg_range(0x2400, 0x2401),
> regmap_reg_range(0x2403, 0x2403),
> regmap_reg_range(0x2410, 0x2417),
> @@ -635,10 +629,7 @@ static const struct regmap_range ksz9477_valid_regs[] = {
> regmap_reg_range(0x1030, 0x1030),
> regmap_reg_range(0x1100, 0x1115),
> regmap_reg_range(0x111a, 0x111f),
> - regmap_reg_range(0x1122, 0x1127),
> - regmap_reg_range(0x112a, 0x112b),
> - regmap_reg_range(0x1136, 0x1139),
> - regmap_reg_range(0x113e, 0x113f),
> + regmap_reg_range(0x1120, 0x113f),
> regmap_reg_range(0x1400, 0x1401),
> regmap_reg_range(0x1403, 0x1403),
> regmap_reg_range(0x1410, 0x1417),
> @@ -669,10 +660,7 @@ static const struct regmap_range ksz9477_valid_regs[] = {
> regmap_reg_range(0x2030, 0x2030),
> regmap_reg_range(0x2100, 0x2115),
> regmap_reg_range(0x211a, 0x211f),
> - regmap_reg_range(0x2122, 0x2127),
> - regmap_reg_range(0x212a, 0x212b),
> - regmap_reg_range(0x2136, 0x2139),
> - regmap_reg_range(0x213e, 0x213f),
> + regmap_reg_range(0x2120, 0x213f),
> regmap_reg_range(0x2400, 0x2401),
> regmap_reg_range(0x2403, 0x2403),
> regmap_reg_range(0x2410, 0x2417),
> @@ -703,10 +691,7 @@ static const struct regmap_range ksz9477_valid_regs[] = {
> regmap_reg_range(0x3030, 0x3030),
> regmap_reg_range(0x3100, 0x3115),
> regmap_reg_range(0x311a, 0x311f),
> - regmap_reg_range(0x3122, 0x3127),
> - regmap_reg_range(0x312a, 0x312b),
> - regmap_reg_range(0x3136, 0x3139),
> - regmap_reg_range(0x313e, 0x313f),
> + regmap_reg_range(0x3120, 0x313f),
> regmap_reg_range(0x3400, 0x3401),
> regmap_reg_range(0x3403, 0x3403),
> regmap_reg_range(0x3410, 0x3417),
> @@ -737,10 +722,7 @@ static const struct regmap_range ksz9477_valid_regs[] = {
> regmap_reg_range(0x4030, 0x4030),
> regmap_reg_range(0x4100, 0x4115),
> regmap_reg_range(0x411a, 0x411f),
> - regmap_reg_range(0x4122, 0x4127),
> - regmap_reg_range(0x412a, 0x412b),
> - regmap_reg_range(0x4136, 0x4139),
> - regmap_reg_range(0x413e, 0x413f),
> + regmap_reg_range(0x4120, 0x413f),
> regmap_reg_range(0x4400, 0x4401),
> regmap_reg_range(0x4403, 0x4403),
> regmap_reg_range(0x4410, 0x4417),
> @@ -771,10 +753,7 @@ static const struct regmap_range ksz9477_valid_regs[] = {
> regmap_reg_range(0x5030, 0x5030),
> regmap_reg_range(0x5100, 0x5115),
> regmap_reg_range(0x511a, 0x511f),
> - regmap_reg_range(0x5122, 0x5127),
> - regmap_reg_range(0x512a, 0x512b),
> - regmap_reg_range(0x5136, 0x5139),
> - regmap_reg_range(0x513e, 0x513f),
> + regmap_reg_range(0x5120, 0x513f),
> regmap_reg_range(0x5400, 0x5401),
> regmap_reg_range(0x5403, 0x5403),
> regmap_reg_range(0x5410, 0x5417),
> @@ -897,10 +876,7 @@ static const struct regmap_range ksz9896_valid_regs[] = {
> regmap_reg_range(0x1030, 0x1030),
> regmap_reg_range(0x1100, 0x1115),
> regmap_reg_range(0x111a, 0x111f),
> - regmap_reg_range(0x1122, 0x1127),
> - regmap_reg_range(0x112a, 0x112b),
> - regmap_reg_range(0x1136, 0x1139),
> - regmap_reg_range(0x113e, 0x113f),
> + regmap_reg_range(0x1120, 0x113f),
> regmap_reg_range(0x1400, 0x1401),
> regmap_reg_range(0x1403, 0x1403),
> regmap_reg_range(0x1410, 0x1417),
> @@ -927,10 +903,7 @@ static const struct regmap_range ksz9896_valid_regs[] = {
> regmap_reg_range(0x2030, 0x2030),
> regmap_reg_range(0x2100, 0x2115),
> regmap_reg_range(0x211a, 0x211f),
> - regmap_reg_range(0x2122, 0x2127),
> - regmap_reg_range(0x212a, 0x212b),
> - regmap_reg_range(0x2136, 0x2139),
> - regmap_reg_range(0x213e, 0x213f),
> + regmap_reg_range(0x2120, 0x213f),
> regmap_reg_range(0x2400, 0x2401),
> regmap_reg_range(0x2403, 0x2403),
> regmap_reg_range(0x2410, 0x2417),
> @@ -957,10 +930,7 @@ static const struct regmap_range ksz9896_valid_regs[] = {
> regmap_reg_range(0x3030, 0x3030),
> regmap_reg_range(0x3100, 0x3115),
> regmap_reg_range(0x311a, 0x311f),
> - regmap_reg_range(0x3122, 0x3127),
> - regmap_reg_range(0x312a, 0x312b),
> - regmap_reg_range(0x3136, 0x3139),
> - regmap_reg_range(0x313e, 0x313f),
> + regmap_reg_range(0x3120, 0x313f),
> regmap_reg_range(0x3400, 0x3401),
> regmap_reg_range(0x3403, 0x3403),
> regmap_reg_range(0x3410, 0x3417),
> @@ -987,10 +957,7 @@ static const struct regmap_range ksz9896_valid_regs[] = {
> regmap_reg_range(0x4030, 0x4030),
> regmap_reg_range(0x4100, 0x4115),
> regmap_reg_range(0x411a, 0x411f),
> - regmap_reg_range(0x4122, 0x4127),
> - regmap_reg_range(0x412a, 0x412b),
> - regmap_reg_range(0x4136, 0x4139),
> - regmap_reg_range(0x413e, 0x413f),
> + regmap_reg_range(0x4120, 0x413f),
> regmap_reg_range(0x4400, 0x4401),
> regmap_reg_range(0x4403, 0x4403),
> regmap_reg_range(0x4410, 0x4417),
> @@ -1017,10 +984,7 @@ static const struct regmap_range ksz9896_valid_regs[] = {
> regmap_reg_range(0x5030, 0x5030),
> regmap_reg_range(0x5100, 0x5115),
> regmap_reg_range(0x511a, 0x511f),
> - regmap_reg_range(0x5122, 0x5127),
> - regmap_reg_range(0x512a, 0x512b),
> - regmap_reg_range(0x5136, 0x5139),
> - regmap_reg_range(0x513e, 0x513f),
> + regmap_reg_range(0x5120, 0x513f),
> regmap_reg_range(0x5400, 0x5401),
> regmap_reg_range(0x5403, 0x5403),
> regmap_reg_range(0x5410, 0x5417),
> @@ -1047,10 +1011,7 @@ static const struct regmap_range ksz9896_valid_regs[] = {
> regmap_reg_range(0x6030, 0x6030),
> regmap_reg_range(0x6100, 0x6115),
> regmap_reg_range(0x611a, 0x611f),
> - regmap_reg_range(0x6122, 0x6127),
> - regmap_reg_range(0x612a, 0x612b),
> - regmap_reg_range(0x6136, 0x6139),
> - regmap_reg_range(0x613e, 0x613f),
> + regmap_reg_range(0x6120, 0x613f),
> regmap_reg_range(0x6300, 0x6301),
> regmap_reg_range(0x6400, 0x6401),
> regmap_reg_range(0x6403, 0x6403),
> --
> 2.25.1
>

--
/Horatiu

2023-06-26 23:01:58

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH] net: dsa: microchip: phy reg access 0x10-0x1f

Hi Horațiu,

On Mon, Jun 26, 2023 at 09:26:43PM +0200, Horatiu Vultur wrote:
> The 06/26/2023 11:20, Jerry Ray wrote:
>
> Hi Jerry,
>
> This seems like a patch for net-next which seems to be closed now.
> Please hold back this patch until the window opens again in like 2 weeks.
>
> Also you need to add in the subject which tree are you targeting.
> In your case is net-next then it should be something like:
> [PATCH net-next] net: dsa: microchip: phy reg access 0x10-0x1f
>
> > With the recent patch to promote phy register accesses to 32-bits for the
> > range >=0x10, it is also necessary to expand the allowed register address
> > table for the affected devices. These three register sets use
> > ksz9477_dev_ops and are therefore affected by the change. The address
> > ranges 0xN120-0xN13f map to the phy register address 0x10-0x1f. There is
> > no reason to exclude any register accesses within this space.
> >
> > on June 20, 2023
> > commit 5c844d57aa78 ("net: dsa: microchip: fix writes to phy registers >= 0x10")
>
> This is just a small thing but as you need to send a new version, you can
> write something like this:
> ---
> With the recent patch [0] to promote ...
>
> [0] 5c844d57aa78 ("net: dsa: microchip: fix writes to phy registers >= 0x10")
> ---
>
> Or:
>
> ---
> With the commit 5c844d57aa78 ("net: dsa: microchip: fix writes to phy
> registers >= 0x10") which promotes phy ...
> ---
>
> Just to make it a little bit more clear that the commit that you posted
> at the end refers to the patch that you mention to at the beginning of
> the commit message.

I think these signs intend to denote a Fixes: tag. For that, we have a
fairly standard way of generating them:

$ cat ~/.gitconfig
...
[core]
abbrev = 12
[pretty]
fixes = Fixes: %h (\"%s\")

$ git show 5c844d57aa78 --pretty=fixes
Fixes: 5c844d57aa78 ("net: dsa: microchip: fix writes to phy registers >= 0x10")

That line should be pasted in the commit message next to (no empty lines
in between) the other tags like reviews and sign offs.

If Jerry can prove a problem with the existing code structure from
net-next (any PHY writes to registers >= 0x10), then the patch does not
need to wait until net-next reopens. It needs to wait until the net-next
pull request is sent to Linus, then this can be resent towards 'net.git'.
That problem needs to be described in the commit message.

If there's no problem that could potentially have a user-visible impact
in the existing code and PHY driver (those PHY writes are unreachable),
then I agree with you, the change can wait and should be resent when
net-next reopens.