Message-ID: <20200911181914.GB20711@plvision.eu>
Date:   Fri, 11 Sep 2020 21:19:14 +0300
From:   Vadym Kochan <vadym.kochan@...ision.eu>
To:     netdev@...r.kernel.org, Russell King <linux@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>
Subject: [DISCUSS] sfp: sfp controller concept

Hi,

I'd like to discuss a concept of introduction additional entity into SFP
subsystem called SFP controller. But lets start with the issue.

Issue
=====

There are boards with SFP ports whose GPIO pins are not connected directly to
the SoC but to the I2C CPLD device which has ability to read/write statuses of
each SFP via I2C read/write commands.

Of course there is already a solution - implement GPIO chip and convert GPIO
numbers & states into internal representation (I2C registers). But it requires
additional GPIO-related handling like:

1) Describe GPIO pins and IN/OUT mapping in DTS

2) Consider this mapping also in CPLD driver

3) IRQ - for me this is not clear how to simulate
   sending IRQ via GPIO chip.

I started to think that may be it would be good to introduce
some generic sfp_controller which might be registered by such CPLD
driver which may provide states of connected modules through the
callback by mapping direct SFP states into it's CPLD registers and
call some sfp_state_update() which will trigger the SFP state
machine. So this driver will check/provide on direct SFP defined
states without considering the GPIO things.

How it may look
===============

Device tree:

sfp0: sfp0 {
        compatible = "sff,sfp";
        i2c-bus = <&i2c0_sfp0>;
        /* ref to controller device */
        ctl = <&cpld>;
        /* this index will be used by sfp controller */
        idx = <0>;
};

SFP controller interface:

There might be added generic sfp-ctl.c which implements the basic sfp controller infra:

    1) register/unregister sfp controller driver

    2) lookup sfp controller by fwnode on SFP node parsing/probing

The relation between modules might be:

    sfp.c <-> sfp-ctl.c <- driver <-> CPLD or some device

Flows:
1) CPLD driver prope:
    driver -> sfp_controller_register()

2) SFP instance probe:
    sfp.c -> sfp-ctl.c:sfp_controller_add_socket()
             creates assoctation between idx and sfp instance.
                                      
3) SFP get state:
    sfp.c -> sfp_ctl_get_state() -> sfp-ctl.c:sfp_controller_get_state() -> driver ops -> get_state

4) SFP state updated:
    driver -> sfp-ctl.c:sfp_controller_notify() -> sfp.c:sfp_state_update()
              finds struct sfp* instance by idx

------------------------------------------------------------------
/* public */

enum {
       GPIO_MODDEF0,
       GPIO_LOS,
       GPIO_TX_FAULT,
       GPIO_TX_DISABLE,
       GPIO_RATE_SELECT,
       GPIO_MAX,

       /* SFP controller should check/provide on these states */
       SFP_F_PRESENT = BIT(GPIO_MODDEF0),
       SFP_F_LOS = BIT(GPIO_LOS),
       SFP_F_TX_FAULT = BIT(GPIO_TX_FAULT),
       SFP_F_TX_DISABLE = BIT(GPIO_TX_DISABLE),
       SFP_F_RATE_SELECT = BIT(GPIO_RATE_SELECT),
};

struct sfp_controller_ops {
	unsigned int (*get_state)(struct sfp_controller *sfp_ctl, int sfp_idx);

	void (*set_state)(struct sfp_controller *sfp_ctl, int sfp_idx,
			  unsigned int state);
};

/* implemented by sfp-ctl.c */
struct sfp_controller *
sfp_controller_register(struct device *dev,
			struct sfp_controller_ops *sfp_ctl_ops,
			int flags);

/* implemented by sfp-ctl.c */
void sfp_controller_unregister(struct sfp_controller *sfp_ctl);

/* implemented by sfp.c */
sfp_state_update(struct sfp *sfp, int state);

/* internal */

/* implemented by sfp-ctl.c */
struct sfp_controller *sfp_controller_find_fwnode(struct fwnode_handle *fwnode);

/* implemented by sfp-ctl.c */
void sfp_controller_put(struct sfp_controller *ctl);

/* implemented by sfp-ctl.c */
unsigned int sfp_controller_get_state(struct sfp_controller *ctl, int idx);

/* implemented by sfp-ctl.c */
void sfp_controller_set_state(struct sfp_controller *ctl, int idx);

/* implemented by sfp-ctl.c */
/* This might be used as ability to notify changed SFP state to sfp-ctl.c by driver
which then may find struct sfp* instance by idx and call sfp_state_update()
which is handled by sfp.c */
void sfp_controller_notify(struct sfp_controller *ctl, int idx, int state);
--------------------------------------------------------------------

Currently I do not see how to properly define sfp_state_update(...) func
which may be triggered by sfp controller to notify SFP state machine. May be additional
interface is needed which may provide to controller the sfp* instance and it's idx:

int sfp_controller_add_socket(struct sfp_controller *ctl, struct sfp *sfp, int idx);

void sfp_controller_del_socket(struct sfp_controller *ctl, struct sfp *sfp);

so the driver may create mapping between struct sfp* and it's idx and call the:

sfp_state_update(struct sfp *sfp, int state);

Regards,
Vadym Kochan

Powered by blists - more mailing lists