Hi Andrew,

Thank you for the feedback. You can find a (leaked) datasheet at:
https://github.com/libc0607/Realtek_switch_hacking/blob/files/RTL8231_Datasheet_1.2.pdf

On Fri, 2021-04-09 at 00:18 +0200, Andrew Lunn wrote:

quoted

- Providing no compatible for an MDIO child node is considered to
be equivalent
  to a c22 ethernet phy, so one must be provided. However, this
node is then
  not automatically probed.

It cannot be automatically probed, since register 2 and 3 do not
contain an ID, which PHYs do. So you need to explicitly list in on
the
MDIO bus, and when the of_mdiobus_register() is called, the device
will be instantiated.

Is it okay to provide a binding without a driver?

quoted

  If some code is required, where should this be put?
  Current devicetree structure:
    mdio-bus {
        compatible = "vendor,mdio";
        ...

        expander0: expander@0 {
            /*
             * Provide compatible for working registration of mdio
device.
             * Device probing happens in gpio1 node.
             */
            compatible = "realtek,rtl8231-expander";
            reg = <0>;
        };

    };
    gpio1 : ext_gpio {
        compatible = "realtek,rtl8231-mdio";
        gpio-controller;
        ...
    };

I don't understand this split. Why not

     mdio-bus {
         compatible = "vendor,mdio";
         ...
 
         expander0: expander@0 {
             /*
              * Provide compatible for working registration of mdio
device.
              * Device probing happens in gpio1 node.
              */
             compatible = "realtek,rtl8231-expander";
             reg = <0>;
             gpio-controller;
         };
     };

You can list whatever properties you need in the node. Ethernet
switches have interrupt-controller, embedded MDIO busses with PHYs on
them etc.

This is what I tried initially, but it doesn't seem to work. The node
is probably still added as an MDIO device, but rtl8231_gpio_probe()
doesn't appear to get called at all. I do agree it would be preferable
over the split specification.

Having another look, I see mdio_device_id is used for ethernet phys,
but like you said this requires and ID in registers 2 & 3. These
registers contain pin configuration on the RTL8231, so this can't be
used.
Registering as a phy_driver appears to have the same issue, although it
looks like I could use a custom match_phy_device(). I do feel like this
would be stretching the meaning of what a PHY is.

quoted

- MFD driver:
  The RTL8231 is not just a GPIO expander, but also a pin
controller and LED
  matrix controller. Regmap initialisation could probably be moved
to a parent
  MFD, with gpio, led, and pinctrl cells. Is this a hard
requirement if only a
  GPIO controller is provided?

You need to think about forward/backwards compatibility. You are
defining a binding now, which you need to keep. Do you see how an MFD
could be added without breaking backwards compatibility?

There are pin-/gpio-controllers that have the gpio and pinctrl nodes in
the device's root node. So I think adding pinctrl later shouldn't be an
issue. The LED matrix description would probably need a dedicated sub-
node. I'll see if I can write some preliminary bindings later today or
this weekend.

Best,
Sander