Re: [PATCH net-next v4 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-12-04 Thread Steve Douthit
On 12/3/18 6:46 PM, Florian Fainelli wrote:
> Yes, we have been discussing that topic with Andrew and have a few ideas
> on how that could be achieved, but not code to use that at the moment.
> One of the idea was to finally allow enslaving the DSA master network
> device, that way you could assign specific ports to a specific CPU/DSA
> master network port within the switch, though that requires setting up a
> bridge. Would that work for your use case?

If bridge here means a port based vlan, then yes that works.  If we're
talking about something that shows up under brctl, then that's less
ideal.

It's probably time to split the thread for any further discussion since
this isn't really relevant to intel-wired-lan anymore.


Re: [PATCH net-next v4 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-12-04 Thread Andrew Lunn
> Yes the current solution whereby we need to get a hold on the network
> device's struct device reference is not quite ideal, AFAIR, Andrew has
> had the same problem.

Yes, it is not nice, and there is a race with systemd renaming the
interfaces using its naming rules. We need a way to lookup an
interface based on a bus location, or similar, not by name.

  Andrew


Re: [PATCH net-next v4 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-12-03 Thread Florian Fainelli
On 12/3/18 3:42 PM, Steve Douthit wrote:
>> Not directly related to this patch series, are you using the legacy or
>> "new" way of passing platform data in order to register the DSA switch?
>> Since you mentioned 6390, I would assume this must be the "new" way of
>> registering DSA devices with mdio_boardinfo etc. In that case, have you
>> found yourself limited by the current dsa_chip_platform_data?
> 
> With the new method I had an off-by-one error where the 'enabled_ports'
> bitmask and 'port_names' array didn't match up in my first attempt.
> That's definitely my fault, but the loop that searches for the "cpu"
> string didn't like it and segfaulted.
> 
> I've got something of a chicken-and-the-egg problem between where I'm
> deferring while waiting for netdevs to show up, and registering the
> mdio_board_info that needs those same pointers.  For testing I exported
> mdiobus_setup_mdiodev_from_board_info() and mdiobus_create_device() so I
> could call the setup function again when the data was actually ready.

Yes the current solution whereby we need to get a hold on the network
device's struct device reference is not quite ideal, AFAIR, Andrew has
had the same problem.

> 
> It'd be nice to have more than one "cpu" port on a switch and have some
> way to associate downstream and "cpu" ports.  Not sure exactly what that
> would look like, and it's not something I'm going to try and tackle at
> the moment, but it's one for the wish list.

Yes, we have been discussing that topic with Andrew and have a few ideas
on how that could be achieved, but not code to use that at the moment.
One of the idea was to finally allow enslaving the DSA master network
device, that way you could assign specific ports to a specific CPU/DSA
master network port within the switch, though that requires setting up a
bridge. Would that work for your use case?
--
Florian


Re: [PATCH net-next v4 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-12-03 Thread Steve Douthit
> Not directly related to this patch series, are you using the legacy or
> "new" way of passing platform data in order to register the DSA switch?
> Since you mentioned 6390, I would assume this must be the "new" way of
> registering DSA devices with mdio_boardinfo etc. In that case, have you
> found yourself limited by the current dsa_chip_platform_data?

With the new method I had an off-by-one error where the 'enabled_ports'
bitmask and 'port_names' array didn't match up in my first attempt.
That's definitely my fault, but the loop that searches for the "cpu"
string didn't like it and segfaulted.

I've got something of a chicken-and-the-egg problem between where I'm
deferring while waiting for netdevs to show up, and registering the
mdio_board_info that needs those same pointers.  For testing I exported
mdiobus_setup_mdiodev_from_board_info() and mdiobus_create_device() so I
could call the setup function again when the data was actually ready.

It'd be nice to have more than one "cpu" port on a switch and have some
way to associate downstream and "cpu" ports.  Not sure exactly what that
would look like, and it's not something I'm going to try and tackle at
the moment, but it's one for the wish list.


Re: [PATCH net-next v4 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-12-03 Thread Florian Fainelli
On 12/3/18 12:14 PM, Steve Douthit wrote:
> Changes from v3 -> v4
> * Remove unecessary pointer casts
> * Fix copy/paste issues in comments
> * Simplify setting of swfw semaphore flags
> * Collect Reviewed-by: tags
> 
> Changes from v2 -> v3
> * Added warnings about interactions between this code and PHY polling
>   unit to the commit messages.
> 
> Changes from v1 -> v2
> [PATCH 1/2] ixgbe: register a mdiobus
> * Add intel-wired-...@lists.osuosl.org to CC list, see
> * select MII in Kconfig (thanks to the kbuild bot)
> * Only call mdiobus_regsiter for single x500em_a device
> * Use readx_poll_timeout() in ixgbe_msca_cmd()
> * Register different bus->read/write callbacks in ixgbe_mii_bus_init()
>   so there's device id check on every access
> * Use device pci_name() in bus->id instead of parent bridge's name
> 
> [PATCH 2/2] ixgbe: use mii_bus to handle MII related ioctls
> * Only use mdiobus_read/write for adapters that registered a mdiobus

Not directly related to this patch series, are you using the legacy or
"new" way of passing platform data in order to register the DSA switch?
Since you mentioned 6390, I would assume this must be the "new" way of
registering DSA devices with mdio_boardinfo etc. In that case, have you
found yourself limited by the current dsa_chip_platform_data?

> 
> Stephen Douthit (2):
>   ixgbe: register a mdiobus
>   ixgbe: use mii_bus to handle MII related ioctls
> 
>  drivers/net/ethernet/intel/Kconfig|   1 +
>  drivers/net/ethernet/intel/ixgbe/ixgbe.h  |   2 +
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  23 ++
>  drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 307 ++
>  drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |   2 +
>  5 files changed, 335 insertions(+)
> 


-- 
Florian


[PATCH net-next v4 0/2] Add mii_bus to ixgbe driver for dsa devs

2018-12-03 Thread Steve Douthit
Changes from v3 -> v4
* Remove unecessary pointer casts
* Fix copy/paste issues in comments
* Simplify setting of swfw semaphore flags
* Collect Reviewed-by: tags

Changes from v2 -> v3
* Added warnings about interactions between this code and PHY polling
  unit to the commit messages.

Changes from v1 -> v2
[PATCH 1/2] ixgbe: register a mdiobus
* Add intel-wired-...@lists.osuosl.org to CC list, see
* select MII in Kconfig (thanks to the kbuild bot)
* Only call mdiobus_regsiter for single x500em_a device
* Use readx_poll_timeout() in ixgbe_msca_cmd()
* Register different bus->read/write callbacks in ixgbe_mii_bus_init()
  so there's device id check on every access
* Use device pci_name() in bus->id instead of parent bridge's name

[PATCH 2/2] ixgbe: use mii_bus to handle MII related ioctls
* Only use mdiobus_read/write for adapters that registered a mdiobus

Stephen Douthit (2):
  ixgbe: register a mdiobus
  ixgbe: use mii_bus to handle MII related ioctls

 drivers/net/ethernet/intel/Kconfig|   1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe.h  |   2 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  23 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 307 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |   2 +
 5 files changed, 335 insertions(+)

-- 
2.17.2