On Wed, Jul 21, 2010 at 6:05 PM, Mike Frysinger [off-list ref] wrote:

On Wed, Jul 21, 2010 at 11:16, Lennert Buytenhek wrote:

quoted

On Wed, Jul 21, 2010 at 09:37:22AM -0400, Mike Frysinger wrote:

quoted

+static int ksz8893m_setup(struct dsa_switch *ds)
+{
+     int i;
+     int ret;
+
+     ret = ksz8893m_switch_reset(ds);
+     if (ret < 0)
+             return ret;

It's pretty ugly that the mdiobus is passed in via the normal means,
but a reference to the SPI bus to use is just stuffed into some global
variable.

Can you not access all registers via MII?

it depends on the host mdio bus.  if it supports the semi-standard
behavior of toggling the OP field of MDIO frames, then yes, you can do
it via MII.  but i dont think the current mdio framework in the kernel
keeps track of that functionality, so there isnt a way in the driver
to say "is this possible, else fall back to SPI".

Are you referring to SMI ?

certainly the part that was used to develop this driver does not
support this behavior thus SPI is the only way of accessing the
extended registers.  i guess the driver could be extended so that
people could pick which mode they want to program the registers via
platform resources, but we have no way of testing that, so i say let
the person who actually can use & wants that functionality implement
it.

quoted

(If not, struct dsa_chip_data will need go be extended with another
struct device pointer that we can use to find the spi bus with.)

if the framework supports, i can convert the driver to it, but i'm not
sure we have the time atm to tackle reworking common frameworks.

If someone tackles this, it would be nice that they bear in mind that
phylib's drivers also need spi/smi/i2c register access.

quoted

quoted

+static int ksz8893m_port_to_phy_addr(int port)
+{
+     if (port >= 1 && port <= KSZ8893M_PORT_NUM)
+             return port;
+
+     pr_warning("use default phy addr 3\n");
+     return 3;

Does this ever happen?  You should just be able to return -1 here, IMHO.

i dont recall seeing a warning, but presumably if it it did occur,
something else needs fixing.  so -1 is OK.

I would remove this.

-- 
Karl