[PATCH v3 0/3] RTL9300 MDIO driver

[PATCH v3 0/3] RTL9300 MDIO driver

[Chris Packham]

This series adds a driver for the MDIO controller on the RTL9300 family
of devices. The controller is a little unique in that we can't access
the SMI interfaces directly. Instead we associate the SMI interface with
a switch port and use the port number to address the SMI bus in
software.

Lee has picked up the mfd binding change so I've dropped it from this round.

Chris Packham (3):
  dt-bindings: net: Add Realtek MDIO controller
  mips: dts: realtek: Add MDIO controller
  net: mdio: Add RTL9300 MDIO driver

 .../bindings/net/realtek,rtl9301-mdio.yaml    |  82 +++++
 arch/mips/boot/dts/realtek/rtl930x.dtsi       |   8 +
 drivers/net/mdio/Kconfig                      |   7 +
 drivers/net/mdio/Makefile                     |   1 +
 drivers/net/mdio/mdio-realtek-rtl.c           | 341 ++++++++++++++++++
 5 files changed, 439 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml
 create mode 100644 drivers/net/mdio/mdio-realtek-rtl.c

-- 
2.47.1

[PATCH v3 1/3] dt-bindings: net: Add Realtek MDIO controller

[Chris Packham]

Add dtschema for the MDIO controller found in the RTL9300 SoCs. The
controller is slightly unusual in that direct MDIO communication is not
possible. Instead, the SMI bus and PHY address are associated with a
switch port and the port number is used when talking to the PHY.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---

Notes:
    Changes in v3:
    - Add r-by from Connor
    Changes in v2:
    - None

 .../bindings/net/realtek,rtl9301-mdio.yaml    | 82 +++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml

diff --git a/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml
new file mode 100644
index 000000000000..95ed77ff8dcc
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/realtek,rtl9301-mdio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek RTL9300 MDIO Controller
+
+maintainers:
+  - Chris Packham <chris.packham@alliedtelesis.co.nz>
+
+allOf:
+  - $ref: mdio.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - realtek,rtl9302b-mdio
+              - realtek,rtl9302c-mdio
+              - realtek,rtl9303-mdio
+          - const: realtek,rtl9301-mdio
+      - const: realtek,rtl9301-mdio
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  reg:
+    maxItems: 1
+
+patternProperties:
+  '^ethernet-phy(@[a-f0-9]+)?':
+    type: object
+    $ref: ethernet-phy.yaml#
+
+    properties:
+      reg:
+        description:
+          The MDIO communication on the RTL9300 is abstracted by the switch. At
+          the software level communication uses the switch port to address the
+          PHY with the actual MDIO bus and address having been setup via the
+          realtek,smi-address property.
+
+      realtek,smi-address:
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+        description: SMI interface and address for the connected PHY
+        items:
+          - description: SMI interface number associated with the port.
+          - description: SMI address of the PHY for the port.
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    mdio@ca00 {
+        compatible = "realtek,rtl9301-mdio";
+        reg = <0xca00 0x200>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ethernet-phy@0 {
+            compatible = "ethernet-phy-ieee802.3-c45";
+            reg = <0>;
+            realtek,smi-address = <0 1>;
+        };
+
+        ethernet-phy@8 {
+            compatible = "ethernet-phy-ieee802.3-c45";
+            reg = <8>;
+            realtek,smi-address = <1 1>;
+        };
+    };
-- 
2.47.1

[PATCH v3 2/3] mips: dts: realtek: Add MDIO controller

[Chris Packham]

Add a device tree node for the MDIO controller on the RTL9300 chips.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v3:
    - None
    Changes in v2:
    - None

 arch/mips/boot/dts/realtek/rtl930x.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/mips/boot/dts/realtek/rtl930x.dtsi b/arch/mips/boot/dts/realtek/rtl930x.dtsi
index 17577457d159..5f74d121ce84 100644
--- a/arch/mips/boot/dts/realtek/rtl930x.dtsi
+++ b/arch/mips/boot/dts/realtek/rtl930x.dtsi
@@ -57,6 +57,14 @@ i2c1: i2c@388 {
 			#size-cells = <0>;
 			status = "disabled";
 		};
+
+		mdio0: mdio@ca00 {
+			compatible = "realtek,rtl9301-mdio";
+			reg = <0xca00 0x200>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
 	};
 };
 
-- 
2.47.1

[PATCH v3 3/3] net: mdio: Add RTL9300 MDIO driver

[Chris Packham]

Add a driver for the MDIO controller on the RTL9300 family of Ethernet
switches with integrated SoC. There are 4 physical SMI interfaces on the
RTL9300 but access is done using the switch ports so a single MDIO bus
is presented to the rest of the system.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v3:
    - Fix (another) off-by-one error
    Changes in v2:
    - Add clause 22 support
    - Remove commented out code
    - Formatting cleanup
    - Set MAX_PORTS correctly for MDIO interface
    - Fix off-by-one error in pn check

 drivers/net/mdio/Kconfig            |   7 +
 drivers/net/mdio/Makefile           |   1 +
 drivers/net/mdio/mdio-realtek-rtl.c | 341 ++++++++++++++++++++++++++++
 3 files changed, 349 insertions(+)
 create mode 100644 drivers/net/mdio/mdio-realtek-rtl.c

diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index 4a7a303be2f7..0c6240c4a7e9 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -185,6 +185,13 @@ config MDIO_IPQ8064
 	  This driver supports the MDIO interface found in the network
 	  interface units of the IPQ8064 SoC
 
+config MDIO_REALTEK_RTL
+	tristate "Realtek RTL9300 MDIO interface support"
+	depends on MACH_REALTEK_RTL || COMPILE_TEST
+	help
+	  This driver supports the MDIO interface found in the Realtek
+	  RTL9300 family of Ethernet switches with integrated SoC.
+
 config MDIO_REGMAP
 	tristate
 	help
diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
index 1015f0db4531..2cd8b491f301 100644
--- a/drivers/net/mdio/Makefile
+++ b/drivers/net/mdio/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_MDIO_MOXART)		+= mdio-moxart.o
 obj-$(CONFIG_MDIO_MSCC_MIIM)		+= mdio-mscc-miim.o
 obj-$(CONFIG_MDIO_MVUSB)		+= mdio-mvusb.o
 obj-$(CONFIG_MDIO_OCTEON)		+= mdio-octeon.o
+obj-$(CONFIG_MDIO_REALTEK_RTL)		+= mdio-realtek-rtl.o
 obj-$(CONFIG_MDIO_REGMAP)		+= mdio-regmap.o
 obj-$(CONFIG_MDIO_SUN4I)		+= mdio-sun4i.o
 obj-$(CONFIG_MDIO_THUNDER)		+= mdio-thunder.o
diff --git a/drivers/net/mdio/mdio-realtek-rtl.c b/drivers/net/mdio/mdio-realtek-rtl.c
new file mode 100644
index 000000000000..d61a9273e86f
--- /dev/null
+++ b/drivers/net/mdio/mdio-realtek-rtl.c
@@ -0,0 +1,341 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MDIO controller for RTL9300 switches with integrated SoC.
+ *
+ * The MDIO communication is abstracted by the switch. At the software level
+ * communication uses the switch port to address the PHY with the actual MDIO
+ * bus and address having been setup via the realtek,smi-address property.
+ */
+
+#include <linux/mdio.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#define SMI_GLB_CTRL			0x000
+#define   GLB_CTRL_INTF_SEL(intf)	BIT(16 + (intf))
+#define SMI_PORT0_15_POLLING_SEL	0x008
+#define SMI_ACCESS_PHY_CTRL_0		0x170
+#define SMI_ACCESS_PHY_CTRL_1		0x174
+#define   PHY_CTRL_RWOP			BIT(2)
+#define   PHY_CTRL_TYPE			BIT(1)
+#define   PHY_CTRL_CMD			BIT(0)
+#define   PHY_CTRL_FAIL			BIT(25)
+#define SMI_ACCESS_PHY_CTRL_2		0x178
+#define SMI_ACCESS_PHY_CTRL_3		0x17c
+#define SMI_PORT0_5_ADDR_CTRL		0x180
+
+#define MAX_PORTS       28
+#define MAX_SMI_BUSSES  4
+#define MAX_SMI_ADDR	0x1f
+
+struct realtek_mdio_priv {
+	struct regmap *regmap;
+	u8 smi_bus[MAX_PORTS];
+	u8 smi_addr[MAX_PORTS];
+	bool smi_bus_isc45[MAX_SMI_BUSSES];
+	u32 reg_base;
+};
+
+static int realtek_mdio_wait_ready(struct realtek_mdio_priv *priv)
+{
+	struct regmap *regmap = priv->regmap;
+	u32 reg_base = priv->reg_base;
+	u32 val;
+
+	return regmap_read_poll_timeout(regmap, reg_base + SMI_ACCESS_PHY_CTRL_1,
+					val, !(val & PHY_CTRL_CMD), 10, 500);
+}
+
+static int realtek_mdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
+{
+	struct realtek_mdio_priv *priv = bus->priv;
+	struct regmap *regmap = priv->regmap;
+	u32 reg_base = priv->reg_base;
+	u32 val;
+	int err;
+
+	err = realtek_mdio_wait_ready(priv);
+	if (err)
+		return err;
+
+	err = regmap_write(regmap, reg_base + SMI_ACCESS_PHY_CTRL_2, phy_id << 16);
+	if (err)
+		return err;
+
+	err = regmap_write(regmap, reg_base + SMI_ACCESS_PHY_CTRL_1,
+			   regnum << 20 |  0x1f << 15 | 0xfff << 3 | PHY_CTRL_CMD);
+	if (err)
+		return err;
+
+	err = realtek_mdio_wait_ready(priv);
+	if (err)
+		return err;
+
+	err = regmap_read(regmap, reg_base + SMI_ACCESS_PHY_CTRL_2, &val);
+	if (err)
+		return err;
+
+	return val & 0xffff;
+}
+
+static int realtek_mdio_write_c22(struct mii_bus *bus, int phy_id, int regnum, u16 value)
+{
+	struct realtek_mdio_priv *priv = bus->priv;
+	struct regmap *regmap = priv->regmap;
+	u32 reg_base = priv->reg_base;
+	u32 val;
+	int err;
+
+	err = realtek_mdio_wait_ready(priv);
+	if (err)
+		return err;
+
+	err = regmap_write(regmap, reg_base + SMI_ACCESS_PHY_CTRL_0, BIT(phy_id));
+	if (err)
+		return err;
+
+	err = regmap_write(regmap, reg_base + SMI_ACCESS_PHY_CTRL_2, value << 16);
+	if (err)
+		return err;
+
+	err = regmap_write(regmap, reg_base + SMI_ACCESS_PHY_CTRL_1,
+			   regnum << 20 |  0x1f << 15 | 0xfff << 3 | PHY_CTRL_RWOP | PHY_CTRL_CMD);
+	if (err)
+		return err;
+
+	err = regmap_read_poll_timeout(regmap, reg_base + SMI_ACCESS_PHY_CTRL_1,
+				       val, !(val & PHY_CTRL_CMD), 10, 100);
+	if (err)
+		return err;
+
+	if (val & PHY_CTRL_FAIL)
+		return -ENXIO;
+
+	return 0;
+}
+
+static int realtek_mdio_read_c45(struct mii_bus *bus, int phy_id, int dev_addr, int regnum)
+{
+	struct realtek_mdio_priv *priv = bus->priv;
+	struct regmap *regmap = priv->regmap;
+	u32 reg_base = priv->reg_base;
+	u32 val;
+	int err;
+
+	err = realtek_mdio_wait_ready(priv);
+	if (err)
+		return err;
+
+	err = regmap_write(regmap, reg_base + SMI_ACCESS_PHY_CTRL_2, phy_id << 16);
+	if (err)
+		return err;
+
+	err = regmap_write(regmap, reg_base + SMI_ACCESS_PHY_CTRL_3,
+			   dev_addr << 16 | (regnum & 0xffff));
+	if (err)
+		return err;
+
+	err = regmap_write(regmap, reg_base + SMI_ACCESS_PHY_CTRL_1,
+			   PHY_CTRL_TYPE | PHY_CTRL_CMD);
+	if (err)
+		return err;
+
+	err = realtek_mdio_wait_ready(priv);
+	if (err)
+		return err;
+
+	err = regmap_read(regmap, reg_base + SMI_ACCESS_PHY_CTRL_2, &val);
+	if (err)
+		return err;
+
+	return val & 0xffff;
+}
+
+static int realtek_mdio_write_c45(struct mii_bus *bus, int phy_id, int dev_addr,
+				  int regnum, u16 value)
+{
+	struct realtek_mdio_priv *priv = bus->priv;
+	struct regmap *regmap = priv->regmap;
+	u32 reg_base = priv->reg_base;
+	u32 val;
+	int err;
+
+	err = realtek_mdio_wait_ready(priv);
+	if (err)
+		return err;
+
+	err = regmap_write(regmap, reg_base + SMI_ACCESS_PHY_CTRL_0, BIT(phy_id));
+	if (err)
+		return err;
+
+	err = regmap_write(regmap, reg_base + SMI_ACCESS_PHY_CTRL_2, value << 16);
+	if (err)
+		return err;
+
+	err = regmap_write(regmap, reg_base + SMI_ACCESS_PHY_CTRL_3,
+			   dev_addr << 16 | (regnum & 0xffff));
+	if (err)
+		return err;
+
+	err = regmap_write(regmap, reg_base + SMI_ACCESS_PHY_CTRL_1,
+			   PHY_CTRL_RWOP | PHY_CTRL_TYPE | PHY_CTRL_CMD);
+	if (err)
+		return err;
+
+	err = regmap_read_poll_timeout(regmap, reg_base + SMI_ACCESS_PHY_CTRL_1,
+				       val, !(val & PHY_CTRL_CMD), 10, 100);
+	if (err)
+		return err;
+
+	if (val & PHY_CTRL_FAIL)
+		return -ENXIO;
+
+	return 0;
+}
+
+static int realtek_mdiobus_init(struct realtek_mdio_priv *priv)
+{
+	u32 glb_ctrl_mask = 0, glb_ctrl_val = 0;
+	struct regmap *regmap = priv->regmap;
+	u32 reg_base = priv->reg_base;
+	u32 port_addr[5] = { 0 };
+	u32 poll_sel[2] = { 0 };
+	int i, err;
+
+	/* Associate the port with the SMI interface and PHY */
+	for (i = 0; i < MAX_PORTS; i++) {
+		int pos;
+
+		if (priv->smi_bus[i] > 3)
+			continue;
+
+		pos = (i % 6) * 5;
+		port_addr[i / 6] |= priv->smi_addr[i] << pos;
+
+		pos = (i % 16) * 2;
+		poll_sel[i / 16] |= priv->smi_bus[i] << pos;
+	}
+
+	/* Put the interfaces into C45 mode if required */
+	for (i = 0; i < MAX_SMI_BUSSES; i++) {
+		if (priv->smi_bus_isc45[i]) {
+			glb_ctrl_mask |= GLB_CTRL_INTF_SEL(i);
+			glb_ctrl_val |= GLB_CTRL_INTF_SEL(i);
+		}
+	}
+
+	err = regmap_bulk_write(regmap, reg_base + SMI_PORT0_5_ADDR_CTRL,
+				port_addr, 5);
+	if (err)
+		return err;
+
+	err = regmap_bulk_write(regmap, reg_base + SMI_PORT0_15_POLLING_SEL,
+				poll_sel, 2);
+	if (err)
+		return err;
+
+	err = regmap_update_bits(regmap, reg_base + SMI_GLB_CTRL,
+				 glb_ctrl_mask, glb_ctrl_val);
+	if (err)
+		return err;
+
+	return 0;
+}
+
+static int realtek_mdiobus_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct realtek_mdio_priv *priv;
+	struct fwnode_handle *child;
+	struct mii_bus *bus;
+	int err;
+
+	bus = devm_mdiobus_alloc_size(dev, sizeof(*priv));
+	if (!bus)
+		return -ENOMEM;
+
+	bus->name = "Reaktek Switch MDIO Bus";
+	bus->read = realtek_mdio_read_c22;
+	bus->write = realtek_mdio_write_c22;
+	bus->read_c45 = realtek_mdio_read_c45;
+	bus->write_c45 =  realtek_mdio_write_c45;
+	bus->parent = dev;
+	priv = bus->priv;
+
+	priv->regmap = syscon_node_to_regmap(dev->parent->of_node);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	err = device_property_read_u32(dev, "reg", &priv->reg_base);
+	if (err)
+		return err;
+
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
+
+	device_for_each_child_node(dev, child) {
+		u32 pn, smi_addr[2];
+
+		err = fwnode_property_read_u32(child, "reg", &pn);
+		if (err)
+			return err;
+
+		if (pn >= MAX_PORTS)
+			return dev_err_probe(dev, -EINVAL, "illegal port number %d\n", pn);
+
+		err = fwnode_property_read_u32_array(child, "realtek,smi-address", smi_addr, 2);
+		if (err) {
+			smi_addr[0] = 0;
+			smi_addr[1] = pn;
+		}
+
+		if (smi_addr[0] >= MAX_SMI_BUSSES)
+			return dev_err_probe(dev, -EINVAL, "illegal smi bus number %d\n",
+					     smi_addr[0]);
+
+		if (smi_addr[1] > MAX_SMI_ADDR)
+			return dev_err_probe(dev, -EINVAL, "illegal smi addr %d\n", smi_addr[1]);
+
+		if (fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45"))
+			priv->smi_bus_isc45[smi_addr[0]] = true;
+
+		priv->smi_bus[pn] = smi_addr[0];
+		priv->smi_addr[pn] = smi_addr[1];
+	}
+
+	err = realtek_mdiobus_init(priv);
+	if (err)
+		return dev_err_probe(dev, err, "failed to initialise MDIO bus controller\n");
+
+	err = devm_of_mdiobus_register(dev, bus, dev->of_node);
+	if (err)
+		return dev_err_probe(dev, err, "cannot register MDIO bus\n");
+
+	return 0;
+}
+
+static const struct of_device_id realtek_mdio_ids[] = {
+	{ .compatible = "realtek,rtl9301-mdio" },
+	{ .compatible = "realtek,rtl9302b-mdio" },
+	{ .compatible = "realtek,rtl9302c-mdio" },
+	{ .compatible = "realtek,rtl9303-mdio" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, realtek_mdio_ids);
+
+static struct platform_driver rtl9300_mdio_driver = {
+	.probe = realtek_mdiobus_probe,
+	.driver = {
+		.name = "mdio-rtl9300",
+		.of_match_table = realtek_mdio_ids,
+	},
+};
+
+module_platform_driver(rtl9300_mdio_driver);
+
+MODULE_DESCRIPTION("RTL9300 MDIO driver");
+MODULE_LICENSE("GPL");
-- 
2.47.1

Re: [PATCH v3 1/3] dt-bindings: net: Add Realtek MDIO controller

[Daniel Golle]

On Wed, Dec 18, 2024 at 11:44:59AM +1300, Chris Packham wrote:
> [...]
> +patternProperties:
> +  '^ethernet-phy(@[a-f0-9]+)?':
> +    type: object
> +    $ref: ethernet-phy.yaml#
> +
> +    properties:
> +      reg:
> +        description:
> +          The MDIO communication on the RTL9300 is abstracted by the switch. At
> +          the software level communication uses the switch port to address the
> +          PHY with the actual MDIO bus and address having been setup via the
> +          realtek,smi-address property.
> +
> +      realtek,smi-address:
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        description: SMI interface and address for the connected PHY
> +        items:
> +          - description: SMI interface number associated with the port.
> +          - description: SMI address of the PHY for the port.

What speaks against describing the actual MDIO busses and addresses (ie.
the hardware) in Device Tree and setting up the hardware mapping between
ports and SMI bus and address by parsing the DSA switch port description
in the MDIO driver?

In that way you would not need the additional vendor-specific property
and on switches with high port density also avoid having to deal with
bogus "MDIO addresses" greater than 31.

Re: [PATCH v3 1/3] dt-bindings: net: Add Realtek MDIO controller

[Chris Packham]

Hi Daniel,

On 18/12/2024 12:54, Daniel Golle wrote:
> On Wed, Dec 18, 2024 at 11:44:59AM +1300, Chris Packham wrote:
>> [...]
>> +patternProperties:
>> +  '^ethernet-phy(@[a-f0-9]+)?':
>> +    type: object
>> +    $ref: ethernet-phy.yaml#
>> +
>> +    properties:
>> +      reg:
>> +        description:
>> +          The MDIO communication on the RTL9300 is abstracted by the switch. At
>> +          the software level communication uses the switch port to address the
>> +          PHY with the actual MDIO bus and address having been setup via the
>> +          realtek,smi-address property.
>> +
>> +      realtek,smi-address:
>> +        $ref: /schemas/types.yaml#/definitions/uint32-array
>> +        description: SMI interface and address for the connected PHY
>> +        items:
>> +          - description: SMI interface number associated with the port.
>> +          - description: SMI address of the PHY for the port.
> What speaks against describing the actual MDIO busses and addresses (ie.
> the hardware) in Device Tree and setting up the hardware mapping between
> ports and SMI bus and address by parsing the DSA switch port description
> in the MDIO driver?
>
> In that way you would not need the additional vendor-specific property
> and on switches with high port density also avoid having to deal with
> bogus "MDIO addresses" greater than 31.

I kind of expected this discussion.

The main problem is I couldn't figure out how to make that work. 
Ultimately I still need to write the MDIO bus arrangement to hardware 
and map back to a port number when doing MDIO accesses. Whether the 
device-tree uses port number as the reg property and a vendor property 
for the SMI address or does it the other way round I'd still need a 
vendor property. There's also a complication that I've got one MDIO 
controller that looks after more than one MDIO interface at the same 
time. I'll have a bit of a think about it over the holidays and see if I 
can come up with anything cleaner.