diff mbox

[OpenWrt-Devel] b53: fix overriding port 8 state (if it is connected to CPU)

Message ID 1426669356-10799-1-git-send-email-zajec5@gmail.com
State Changes Requested
Delegated to: Jonas Gorski
Headers show

Commit Message

Rafał Miłecki March 18, 2015, 9:02 a.m. UTC
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 .../generic/files/drivers/net/phy/b53/b53_common.c | 23 +++++++++++++++++++++-
 .../generic/files/drivers/net/phy/b53/b53_regs.h   |  1 +
 2 files changed, 23 insertions(+), 1 deletion(-)

Comments

Jonas Gorski March 18, 2015, 10:28 a.m. UTC | #1
On Wed, Mar 18, 2015 at 10:02 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  .../generic/files/drivers/net/phy/b53/b53_common.c | 23 +++++++++++++++++++++-
>  .../generic/files/drivers/net/phy/b53/b53_regs.h   |  1 +
>  2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/target/linux/generic/files/drivers/net/phy/b53/b53_common.c b/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
> index e44d194..4597742 100644
> --- a/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
> +++ b/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
> @@ -525,7 +525,7 @@ static int b53_switch_reset(struct b53_device *dev)
>                                 return -EINVAL;
>                         }
>                 }
> -       } else if ((is531x5(dev) || is5301x(dev)) && dev->sw_dev.cpu_port == B53_CPU_PORT) {
> +       } else if (is531x5(dev) && dev->sw_dev.cpu_port == B53_CPU_PORT) {
>                 u8 mii_port_override;
>
>                 b53_read8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
> @@ -533,6 +533,27 @@ static int b53_switch_reset(struct b53_device *dev)
>                 b53_write8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
>                            mii_port_override | PORT_OVERRIDE_EN |
>                            PORT_OVERRIDE_LINK);
> +       } else if (is5301x(dev)) {
> +               /*
> +                * CPU interface attached to port 8 requires specific handling.
> +                * It uses different overriding register and extra ports 5 and 7
> +                * need to be configured as well.
> +                */
> +               if (dev->sw_dev.cpu_port == 8) {
> +                       u8 mii_port_override;
> +
> +                       b53_read8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
> +                                 &mii_port_override);
> +                       mii_port_override |= PORT_OVERRIDE_LINK |
> +                                            PORT_OVERRIDE_RX_FLOW |
> +                                            PORT_OVERRIDE_TX_FLOW |
> +                                            PORT_OVERRIDE_SPEED_2000M |
> +                                            PORT_OVERRIDE_EN;
> +                       b53_write8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
> +                                  mii_port_override);
> +               } else {
> +                       pr_warn("overriding CPU port other than 8 is not supported yet\n");
> +               }
>         }
>
>         b53_enable_mib(dev);

How about

@@ -530,9 +530,16 @@ static int b53_switch_reset(struct b53_device *dev)

                b53_read8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
                          &mii_port_override);
+
+               mii_port_override |= PORT_OVERRIDE_LINK | PORT_OVERRIDE_EN;
+
+               if (is5301x(dev))
+                       mii_port_override |= PORT_OVERRIDE_RX_FLOW |
+                                            PORT_OVERRIDE_TX_FLOW |
+                                            PORT_OVERRIDE_SPEED_2000M;
+
                b53_write8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
-                          mii_port_override | PORT_OVERRIDE_EN |
-                          PORT_OVERRIDE_LINK);
+                          mii_port_override);
        }

        b53_enable_mib(dev);

instead of creating a full new branch?



Jonas
Rafał Miłecki March 18, 2015, 11:09 a.m. UTC | #2
On 18 March 2015 at 11:28, Jonas Gorski <jogo@openwrt.org> wrote:
> On Wed, Mar 18, 2015 at 10:02 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> ---
>>  .../generic/files/drivers/net/phy/b53/b53_common.c | 23 +++++++++++++++++++++-
>>  .../generic/files/drivers/net/phy/b53/b53_regs.h   |  1 +
>>  2 files changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/linux/generic/files/drivers/net/phy/b53/b53_common.c b/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
>> index e44d194..4597742 100644
>> --- a/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
>> +++ b/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
>> @@ -525,7 +525,7 @@ static int b53_switch_reset(struct b53_device *dev)
>>                                 return -EINVAL;
>>                         }
>>                 }
>> -       } else if ((is531x5(dev) || is5301x(dev)) && dev->sw_dev.cpu_port == B53_CPU_PORT) {
>> +       } else if (is531x5(dev) && dev->sw_dev.cpu_port == B53_CPU_PORT) {
>>                 u8 mii_port_override;
>>
>>                 b53_read8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
>> @@ -533,6 +533,27 @@ static int b53_switch_reset(struct b53_device *dev)
>>                 b53_write8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
>>                            mii_port_override | PORT_OVERRIDE_EN |
>>                            PORT_OVERRIDE_LINK);
>> +       } else if (is5301x(dev)) {
>> +               /*
>> +                * CPU interface attached to port 8 requires specific handling.
>> +                * It uses different overriding register and extra ports 5 and 7
>> +                * need to be configured as well.
>> +                */
>> +               if (dev->sw_dev.cpu_port == 8) {
>> +                       u8 mii_port_override;
>> +
>> +                       b53_read8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
>> +                                 &mii_port_override);
>> +                       mii_port_override |= PORT_OVERRIDE_LINK |
>> +                                            PORT_OVERRIDE_RX_FLOW |
>> +                                            PORT_OVERRIDE_TX_FLOW |
>> +                                            PORT_OVERRIDE_SPEED_2000M |
>> +                                            PORT_OVERRIDE_EN;
>> +                       b53_write8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
>> +                                  mii_port_override);
>> +               } else {
>> +                       pr_warn("overriding CPU port other than 8 is not supported yet\n");
>> +               }
>>         }
>>
>>         b53_enable_mib(dev);
>
> How about
>
> @@ -530,9 +530,16 @@ static int b53_switch_reset(struct b53_device *dev)
>
>                 b53_read8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
>                           &mii_port_override);
> +
> +               mii_port_override |= PORT_OVERRIDE_LINK | PORT_OVERRIDE_EN;
> +
> +               if (is5301x(dev))
> +                       mii_port_override |= PORT_OVERRIDE_RX_FLOW |
> +                                            PORT_OVERRIDE_TX_FLOW |
> +                                            PORT_OVERRIDE_SPEED_2000M;
> +
>                 b53_write8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
> -                          mii_port_override | PORT_OVERRIDE_EN |
> -                          PORT_OVERRIDE_LINK);
> +                          mii_port_override);
>         }
>
>         b53_enable_mib(dev);
>
> instead of creating a full new branch?

As the comment says, this code for BCM5301X will be extended. This is
because of this 2000M speed, which requires configuring 3 ports. It
seems communication between switch and CPU interface with such speed
couldn't be handled with only a single port. Broadcom decided to use 3
ports in total for that.

I don't have that code ready and I also don't like patch bombs, so I
started with this simple change. However because of further
development plans I vote for a separated branch.
Jonas Gorski March 30, 2015, 8:32 p.m. UTC | #3
On Wed, Mar 18, 2015 at 12:09 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> On 18 March 2015 at 11:28, Jonas Gorski <jogo@openwrt.org> wrote:
>> On Wed, Mar 18, 2015 at 10:02 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
>>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>>> ---
>>>  .../generic/files/drivers/net/phy/b53/b53_common.c | 23 +++++++++++++++++++++-
>>>  .../generic/files/drivers/net/phy/b53/b53_regs.h   |  1 +
>>>  2 files changed, 23 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target/linux/generic/files/drivers/net/phy/b53/b53_common.c b/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
>>> index e44d194..4597742 100644
>>> --- a/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
>>> +++ b/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
>>> @@ -525,7 +525,7 @@ static int b53_switch_reset(struct b53_device *dev)
>>>                                 return -EINVAL;
>>>                         }
>>>                 }
>>> -       } else if ((is531x5(dev) || is5301x(dev)) && dev->sw_dev.cpu_port == B53_CPU_PORT) {
>>> +       } else if (is531x5(dev) && dev->sw_dev.cpu_port == B53_CPU_PORT) {
>>>                 u8 mii_port_override;
>>>
>>>                 b53_read8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
>>> @@ -533,6 +533,27 @@ static int b53_switch_reset(struct b53_device *dev)
>>>                 b53_write8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
>>>                            mii_port_override | PORT_OVERRIDE_EN |
>>>                            PORT_OVERRIDE_LINK);
>>> +       } else if (is5301x(dev)) {
>>> +               /*
>>> +                * CPU interface attached to port 8 requires specific handling.
>>> +                * It uses different overriding register and extra ports 5 and 7
>>> +                * need to be configured as well.
>>> +                */
>>> +               if (dev->sw_dev.cpu_port == 8) {
>>> +                       u8 mii_port_override;
>>> +
>>> +                       b53_read8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
>>> +                                 &mii_port_override);
>>> +                       mii_port_override |= PORT_OVERRIDE_LINK |
>>> +                                            PORT_OVERRIDE_RX_FLOW |
>>> +                                            PORT_OVERRIDE_TX_FLOW |
>>> +                                            PORT_OVERRIDE_SPEED_2000M |
>>> +                                            PORT_OVERRIDE_EN;
>>> +                       b53_write8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
>>> +                                  mii_port_override);
>>> +               } else {
>>> +                       pr_warn("overriding CPU port other than 8 is not supported yet\n");
>>> +               }
>>>         }
>>>
>>>         b53_enable_mib(dev);
>>
>> How about
>>
>> @@ -530,9 +530,16 @@ static int b53_switch_reset(struct b53_device *dev)
>>
>>                 b53_read8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
>>                           &mii_port_override);
>> +
>> +               mii_port_override |= PORT_OVERRIDE_LINK | PORT_OVERRIDE_EN;
>> +
>> +               if (is5301x(dev))
>> +                       mii_port_override |= PORT_OVERRIDE_RX_FLOW |
>> +                                            PORT_OVERRIDE_TX_FLOW |
>> +                                            PORT_OVERRIDE_SPEED_2000M;
>> +
>>                 b53_write8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
>> -                          mii_port_override | PORT_OVERRIDE_EN |
>> -                          PORT_OVERRIDE_LINK);
>> +                          mii_port_override);
>>         }
>>
>>         b53_enable_mib(dev);
>>
>> instead of creating a full new branch?
>
> As the comment says, this code for BCM5301X will be extended. This is
> because of this 2000M speed, which requires configuring 3 ports. It
> seems communication between switch and CPU interface with such speed
> couldn't be handled with only a single port. Broadcom decided to use 3
> ports in total for that.
>
> I don't have that code ready and I also don't like patch bombs, so I
> started with this simple change. However because of further
> development plans I vote for a separated branch.

Sorry for taking so long to respond, the kernel-size issue on bcm63xx
took longer and had more side effects than expected.

Then let's keep it simple for now and move it into a separate branch
if you actually have the code ready. At least for me, my future code
never stays that way how I imagined it will be and it tends to look a
lot different when done.


Jonas
diff mbox

Patch

diff --git a/target/linux/generic/files/drivers/net/phy/b53/b53_common.c b/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
index e44d194..4597742 100644
--- a/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
+++ b/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
@@ -525,7 +525,7 @@  static int b53_switch_reset(struct b53_device *dev)
 				return -EINVAL;
 			}
 		}
-	} else if ((is531x5(dev) || is5301x(dev)) && dev->sw_dev.cpu_port == B53_CPU_PORT) {
+	} else if (is531x5(dev) && dev->sw_dev.cpu_port == B53_CPU_PORT) {
 		u8 mii_port_override;
 
 		b53_read8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
@@ -533,6 +533,27 @@  static int b53_switch_reset(struct b53_device *dev)
 		b53_write8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
 			   mii_port_override | PORT_OVERRIDE_EN |
 			   PORT_OVERRIDE_LINK);
+	} else if (is5301x(dev)) {
+		/*
+		 * CPU interface attached to port 8 requires specific handling.
+		 * It uses different overriding register and extra ports 5 and 7
+		 * need to be configured as well.
+		 */
+		if (dev->sw_dev.cpu_port == 8) {
+			u8 mii_port_override;
+
+			b53_read8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
+				  &mii_port_override);
+			mii_port_override |= PORT_OVERRIDE_LINK |
+					     PORT_OVERRIDE_RX_FLOW |
+					     PORT_OVERRIDE_TX_FLOW |
+					     PORT_OVERRIDE_SPEED_2000M |
+					     PORT_OVERRIDE_EN;
+			b53_write8(dev, B53_CTRL_PAGE, B53_PORT_OVERRIDE_CTRL,
+				   mii_port_override);
+		} else {
+			pr_warn("overriding CPU port other than 8 is not supported yet\n");
+		}
 	}
 
 	b53_enable_mib(dev);
diff --git a/target/linux/generic/files/drivers/net/phy/b53/b53_regs.h b/target/linux/generic/files/drivers/net/phy/b53/b53_regs.h
index 6d71493..129946d 100644
--- a/target/linux/generic/files/drivers/net/phy/b53/b53_regs.h
+++ b/target/linux/generic/files/drivers/net/phy/b53/b53_regs.h
@@ -86,6 +86,7 @@ 
 #define   PORT_OVERRIDE_RV_MII_25	BIT(4) /* BCM5325 only */
 #define   PORT_OVERRIDE_RX_FLOW		BIT(4)
 #define   PORT_OVERRIDE_TX_FLOW		BIT(5)
+#define   PORT_OVERRIDE_SPEED_2000M	BIT(6) /* BCM5301X only, requires setting 1000M */
 #define   PORT_OVERRIDE_EN		BIT(7) /* Use the register contents */
 
 /* Power-down mode control */