Initialize optoe device via device tree

[insekt]

Hi, is it possible to initialize optoe device via device tree? From the source code it seems that it not possible but I'm not sure. Now I'm using this device tree and I see initialization of the optoe device itself but port_name property doesn't work.

i2c@0 {
    #address-cells = <1>;
    #size-cells = <0>;
    reg = <0>;

    optoe@50 {
        compatible = "oom,optoe2";
        reg = <0x50>;
        port_name = "0";
    };

[donboll]

Try removing “oom,” from the compatible line:

compatible = “optoe2”;

It turns out that the optoe kernel driver knows nothing about the oom python package. I don’t think “oom,optoe2” will match the driver.

Don

From: Mikhail Sokolov [mailto:notifications@github.com] Sent: Sunday, April 12, 2020 5:10 AM To: opencomputeproject/oom oom@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: [opencomputeproject/oom] Initialize optoe device via device tree (#17)

Hi, is it possible to initialize optoe device via device tree? From the source code it seems that it not possible but I'm not sure. Now I'm using this device tree and I see initialization of the optoe device itself but port_name property doesn't work.

i2c@0 {

address-cells = <1>;

    #size-cells = <0>;
    reg = <0>;

    optoe@50 {
           compatible = "oom,optoe2";
           reg = <0x50>;
           port_name = "0";
    };

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/opencomputeproject/oom/issues/17 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AD6VCJ64NK2YQDBJ6YFYSL3RMGVRVANCNFSM4MGMQT3A . https://github.com/notifications/beacon/AD6VCJ36RKQ7NTJGWPXP7WTRMGVRVA5CNFSM4MGMQT3KYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4I5L6H3Q.gif

[insekt]

Don, this property

compatible = “optoe2”;

defines only how many I2C addresses driver need to use and doesn't related to port_name.

BTW, it doesn't matter how to define it

compatible = “optoe2”;

or

compatible = “oom,optoe2”;

It seems like optoe driver hasn't been written to support device tree at all. It looks like a classic I2C driver without OF (https://elinux.org/Device_Tree_What_It_Is#Open_Firmware). Property "compatible" works based on underneath I2C driver infrastructure. I wasn't able to understand exactly how compatible property works with the current optoe driver. My findings ended up with this https://github.com/torvalds/linux/blob/f66ed1ebbfde37631fba289f7c399eaa70632abf/drivers/i2c/i2c-core-base.c#L93

But I did find a way how to get properties from device tree with a small modification to optoe_probe function. After some research I ended up with this code:

Two new variables.

    const char* cp = NULL;
    int len = 0;

Check for a device tree property

        /* Check for a device tree property */
        if (client->dev.of_node) {
            cp = of_get_property(client->dev.of_node, "port_name", &len);
            if ((cp == NULL) || (len == 0)) {
                strcpy(port_name, "unitialized");
            } else {
                strcpy(port_name, cp);              
            }           
        }

static int optoe_probe(struct i2c_client *client,
            const struct i2c_device_id *id)
{
    int err;
    int use_smbus = 0;
    struct optoe_platform_data chip;
    struct optoe_data *optoe;
    int num_addresses = 0;
    char port_name[MAX_PORT_NAME_LEN];
    const char* cp = NULL;
    int len = 0;

    if (client->addr != 0x50) {
        dev_dbg(&client->dev, "probe, bad i2c addr: 0x%x\n",
                      client->addr);
        err = -EINVAL;
        goto exit;
    }

    if (client->dev.platform_data) {
        chip = *(struct optoe_platform_data *)client->dev.platform_data;
        /* take the port name from the supplied platform data */
#ifdef EEPROM_CLASS
        strncpy(port_name, chip.eeprom_data->label, MAX_PORT_NAME_LEN);
#else
        memcpy(port_name, chip.port_name, MAX_PORT_NAME_LEN);
#endif
        dev_dbg(&client->dev,
            "probe, chip provided, flags:0x%x; name: %s\n",
            chip.flags, client->name);
    } else {
        if (!id->driver_data) {
            err = -ENODEV;
            goto exit;
        }
        dev_dbg(&client->dev, "probe, building chip\n");
        /* Check for a device tree property */
        if (client->dev.of_node) {
            cp = of_get_property(client->dev.of_node, "port_name", &len);
            if ((cp == NULL) || (len == 0)) {
                strcpy(port_name, "unitialized");
            } else {
                strcpy(port_name, cp);              
            }           
        }
...

I find this trick here: https://github.com/opennetworklinux/linux/blob/c1d55323367a3c45510794a0ebe2e706c1443541/3.2.65-1%2Bdeb7u2/patches/driver-eeprom-class.patch#L214

[insekt]

Now I can pass port_name via device tree. I work with Raspberry Pi. Here is device tree overlay and the config.

Overlay consist of 2 type of devices:

• I2C-mux (1 item) - I2C-mux (PCA9548) provide ability to put 8 devices with the same address to the I2C bus
• optoe (8 items) - SFP/SFP+ modules

Overlay:

// I2C Mux with optoe overlay for Raspberry Pi

/dts-v1/;
/plugin/;

/{
    compatible = "brcm,bcm2708";

    fragment@0 {
        target = <&i2c_arm>;
        __overlay__ {
            #address-cells = <1>;
            #size-cells = <0>;
            status = "okay";

            pca9548: mux@70 {
                compatible = "nxp,pca9548";
                reg = <0x70>;
                i2c-mux-idle-disconnect;
                #address-cells = <1>;
                #size-cells = <0>;

                i2c@0 {
                    #address-cells = <1>;
                    #size-cells = <0>;
                    reg = <0>;

                    optoe0: optoe@50 {
                        compatible = "optoe2";
                        reg = <0x50>;
                        port_name = "1";
                    };
                };
                i2c@1 {
                    #address-cells = <1>;
                    #size-cells = <0>;
                    reg = <1>;

                    optoe1: optoe@50 {
                        compatible = "optoe2";
                        reg = <0x50>;
                        port_name = "2";
                    };
                };
                i2c@2 {
                    #address-cells = <1>;
                    #size-cells = <0>;
                    reg = <2>;

                    optoe2: optoe@50 {
                        compatible = "optoe2";
                        reg = <0x50>;
                        port_name = "3";
                    };
                };
                i2c@3 {
                    #address-cells = <1>;
                    #size-cells = <0>;
                    reg = <3>;

                    optoe3: optoe@50 {
                        compatible = "optoe2";
                        reg = <0x50>;
                        port_name = "4";
                    };
                };
                i2c@4 {
                    #address-cells = <1>;
                    #size-cells = <0>;
                    reg = <4>;

                    optoe4: optoe@50 {
                        compatible = "optoe2";
                        reg = <0x50>;
                        port_name = "5";
                    };
                };
                i2c@5 {
                    #address-cells = <1>;
                    #size-cells = <0>;
                    reg = <5>;

                    optoe5: optoe@50 {
                        compatible = "optoe2";
                        reg = <0x50>;
                        port_name = "6";
                    };
                };
                i2c@6 {
                    #address-cells = <1>;
                    #size-cells = <0>;
                    reg = <6>;

                    optoe6: optoe@50 {
                        compatible = "optoe2";
                        reg = <0x50>;
                        port_name = "7";
                    };
                };
                i2c@7 {
                    #address-cells = <1>;
                    #size-cells = <0>;
                    reg = <7>;

                    optoe7: optoe@50 {
                        compatible = "optoe2";
                        reg = <0x50>;
                        port_name = "8";
                    };
                };
            };
        };
    };

    __overrides__ {
        addr = <&pca9548>,"reg:0";
        p0 = <&optoe0>,"port_name";
        p1 = <&optoe1>,"port_name";
        p2 = <&optoe2>,"port_name";
        p3 = <&optoe3>,"port_name";
        p4 = <&optoe4>,"port_name";
        p5 = <&optoe5>,"port_name";
        p6 = <&optoe6>,"port_name";
        p7 = <&optoe7>,"port_name";
    };
};

Should be compiled and place in the overlays dir.

dtc -O dtb -o optoe-overlay.dtbo -b 0 -@ optoe-overlay.dts
cp optoe-overlay.dtbo /boot/overlays/optoe.dtbo

From /boot/config.txt

dtoverlay=optoe:addr=0x75
dtoverlay=optoe:addr=0x74,p0=9,p1=10,p2=11,p3=12,p4=13,p5=14,p6=15,p7=16
dtoverlay=optoe:addr=0x73,p0=17,p1=18,p2=19,p3=20,p4=21,p5=22,p6=23,p7=24
dtoverlay=optoe:addr=0x72,p0=25,p1=26,p2=27,p3=28,p4=29,p5=30,p6=31,p7=32
dtoverlay=optoe:addr=0x71,p0=33,p1=34,p2=35,p3=36,p4=37,p5=38,p6=39,p7=40
dtoverlay=optoe:addr=0x70,p0=41,p1=42,p2=43,p3=44,p4=45,p5=46,p6=47,p7=48

1st line in config (dtoverlay=optoe:addr=0x75) doesn't have options, defaults from device tree are used.

This config if for 48 SFP/SFP+ modules connected 1 I2C bus over 6 I2C muxes.

[donboll]

Mikhail,

Thanks for the feedback and the code. You are right, “optoe driver hasn’t been written to support device tree at all”. Device Tree was not part of the environment I was working in, for any of the switches or OS implementations. No problem, I’m willing to add it.

Don, this property

compatible = “optoe2”;

defines only how many I2C addresses driver need to use and doesn't related to port_name.

BTW, it doesn't matter how to define it

compatible = “optoe2”;

or

compatible = “oom,optoe2”;

It looks like the compatible property is passed into the optoe probe function as the ‘name’ field in the i2c_client structure. As such, “oom,optoe2” will not be recognized by the probe function. “optoe2” will be recognized, and will cause the driver to treat the device as a “TWO_ADDR” device, ie an SFP part. So, “optoe1” is for QSFP, “optoe2” is for SFP, “optoe3” is for CMIS. No change required for that.

Your code for the port_name parameter looks good. I’ve tweaked it slightly. I prefer more descriptive variable names, and I used strncpy to copy in the port name. That way it can’t be overwritten by an overly long port name.

Here’s the diff for the proposed new version of optoe.c.

---

---

diff --git a/optoe/optoe.c b/optoe/optoe.c

index 16287fd..5fa6248 100644

--- a/optoe/optoe.c

+++ b/optoe/optoe.c

@@ -951,6 +951,8 @@ static int optoe_probe(struct i2c_client *client,

    struct optoe_data *optoe;

    int num_addresses = 0;

    char port_name[MAX_PORT_NAME_LEN];

• const char *of_port_name = NULL;

• int of_pn_len = 0;

  if (client->addr != 0x50) {
  
          dev_dbg(&client->dev, "probe, bad i2c addr: 0x%x\n",

@@ -976,7 +978,18 @@ static int optoe_probe(struct i2c_client *client,

                    goto exit;

            }

            dev_dbg(&client->dev, "probe, building chip\n");

• strcpy(port_name, "unitialized");

• / Check for a device tree property for port name /

• if (client->dev.of_node) {

• of_port_name = of_get_property(client->dev.of_node,

• "port_name", &of_pn_len);

• if ((of_port_name == NULL) || (of_pn_len == 0)) {

• strcpy(port_name, "unitialized");

• } else {

• strncpy(port_name, of_port_name,

• MAX_PORT_NAME_LEN);

• }

• }

          chip.flags = 0;

ifdef EEPROM_CLASS

            chip.eeprom_data = NULL;

---

I’ve compiled it, but I don’t have an environment right now that uses device tree, so I can’t test it. Can you put this on your PI system, and see if it appears to work? In particular, can you check the ‘dev_class’ and ‘port_name’ files for a few of the devices to confirm both were passed in correctly? ‘dev_class’ should be 1, 2 or 3, mapping to optoe1, optoe2 and optoe3.

Can you try a device tree that specifies at least one device each as optoe1, optoe2 and optoe3? You don’t have to try to actually use the devices labeled this way, just check to see if they are properly set through the device tree.

If you can send me the results, and those tests pass, I will commit this to the github oom repo.

Thanks

Don

From: Mikhail Sokolov [mailto:notifications@github.com] Sent: Sunday, May 03, 2020 8:29 AM To: opencomputeproject/oom oom@noreply.github.com Cc: donboll don@thebollingers.org; Comment comment@noreply.github.com Subject: Re: [opencomputeproject/oom] Initialize optoe device via device tree (#17)

Now I can pass port_name via device tree. I work with Raspberry Pi. Here is device tree overlay and the config.

Overlay consist of 2 type of devices:

• I2C-mux (1 item) - I2C-mux (PCA9548) provide ability to put 8 devices with the same address to the I2C bus.
• optoe (8 items) - SFP/SFP+ modules

Overlay:

// I2C Mux with optoe overlay for Raspberry Pi

/dts-v1/; /plugin/;

/{ compatible = "brcm,bcm2708";

    fragment@0 {
           target = <&i2c_arm>;
           __overlay__ {
                   #address-cells = <1>;
                   #size-cells = <0>;
                   status = "okay";

                   pca9548: mux@70 {
                           compatible = "nxp,pca9548";
                           reg = <0x70>;
                           i2c-mux-idle-disconnect;
                           #address-cells = <1>;
                           #size-cells = <0>;

                           i2c@0 {
                                  #address-cells = <1>;
                                  #size-cells = <0>;
                                  reg = <0>;

                                  optoe0: optoe@50 {
                                          compatible = "optoe2";
                                          reg = <0x50>;
                                          port_name = "1";
                                  };
                           };
                           i2c@1 {
                                  #address-cells = <1>;
                                  #size-cells = <0>;
                                  reg = <1>;

                                  optoe1: optoe@50 {
                                          compatible = "optoe2";
                                          reg = <0x50>;
                                          port_name = "2";
                                  };
                           };
                           i2c@2 {
                                  #address-cells = <1>;
                                  #size-cells = <0>;
                                  reg = <2>;

                                  optoe2: optoe@50 {
                                          compatible = "optoe2";
                                          reg = <0x50>;
                                          port_name = "3";
                                  };
                           };
                           i2c@3 {
                                  #address-cells = <1>;
                                  #size-cells = <0>;
                                  reg = <3>;

                                  optoe3: optoe@50 {
                                          compatible = "optoe2";
                                          reg = <0x50>;
                                          port_name = "4";
                                  };
                           };
                           i2c@4 {
                                  #address-cells = <1>;
                                  #size-cells = <0>;
                                  reg = <4>;

                                  optoe4: optoe@50 {
                                          compatible = "optoe2";
                                          reg = <0x50>;
                                          port_name = "5";
                                  };
                           };
                           i2c@5 {
                                  #address-cells = <1>;
                                  #size-cells = <0>;
                                  reg = <5>;

                                  optoe5: optoe@50 {
                                          compatible = "optoe2";
                                          reg = <0x50>;
                                          port_name = "6";
                                  };
                           };
                           i2c@6 {
                                  #address-cells = <1>;
                                  #size-cells = <0>;
                                  reg = <6>;

                                  optoe6: optoe@50 {
                                          compatible = "optoe2";
                                          reg = <0x50>;
                                          port_name = "7";
                                  };
                           };
                           i2c@7 {
                                  #address-cells = <1>;
                                  #size-cells = <0>;
                                  reg = <7>;

                                  optoe7: optoe@50 {
                                          compatible = "optoe2";
                                          reg = <0x50>;
                                          port_name = "8";
                                  };
                           };
                   };
           };
    };

    __overrides__ {
           addr = <&pca9548>,"reg:0";
           p0 = <&optoe0>,"port_name";
           p1 = <&optoe1>,"port_name";
           p2 = <&optoe2>,"port_name";
           p3 = <&optoe3>,"port_name";
           p4 = <&optoe4>,"port_name";
           p5 = <&optoe5>,"port_name";
           p6 = <&optoe6>,"port_name";
           p7 = <&optoe7>,"port_name";
    };

};

Should be compiled and place in the overlays dir.

dtc -O dtb -o optoe-overlay.dtbo -b 0 -@ optoe-overlay.dts cp optoe-overlay.dtbo /boot/overlays/optoe.dtbo

From /boot/config.txt

dtoverlay=optoe:addr=0x75 dtoverlay=optoe:addr=0x74,p0=9,p1=10,p2=11,p3=12,p4=13,p5=14,p6=15,p7=16 dtoverlay=optoe:addr=0x73,p0=17,p1=18,p2=19,p3=20,p4=21,p5=22,p6=23,p7=24 dtoverlay=optoe:addr=0x72,p0=25,p1=26,p2=27,p3=28,p4=29,p5=30,p6=31,p7=32 dtoverlay=optoe:addr=0x71,p0=33,p1=34,p2=35,p3=36,p4=37,p5=38,p6=39,p7=40 dtoverlay=optoe:addr=0x70,p0=41,p1=42,p2=43,p3=44,p4=45,p5=46,p6=47,p7=48

1st line in config (dtoverlay=optoe:addr=0x75) doesn't have options, defaults from device tree are used.

This config if for 48 SFP/SFP+ modules connected 1 I2C bus over 6 I2C muxes.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/opencomputeproject/oom/issues/17#issuecomment-623127302 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AD6VCJ72ID37GVTKYXSUUJDRPWES5ANCNFSM4MGMQT3A .

[donboll]

device tree support has been added in optoe, available as of August 12, 2021. See commit 3ef1e9f5.