On 6/1/23 14:13, Lukasz Majewski wrote:
Hi Marek,

On 6/1/23 12:00, Lukasz Majewski wrote:
Some devices, when configured in bootstrap to 'no cpu' mode require
PHY manual reset to get them operational and responding to reading
their ID registers.

Without this step - the PHYLIB probing will fail.

In more details - the bootstrap configuration from switch must be
read. The value of CONFIG Data1 (0x71) of Scratch and Misc register
is read to check if 'no_cpu' and 'addr4' bits were set.

Signed-off-by: Lukasz Majewski <lu...@denx.de>
Reviewed-by: Ramon Fried <rfried....@gmail.com>

---

   drivers/net/phy/mv88e61xx.c | 63
+++++++++++++++++++++++++++++++++++-- 1 file changed, 61
insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mv88e61xx.c
b/drivers/net/phy/mv88e61xx.c index 69a87bead469..cf8f5e833e82
100644 --- a/drivers/net/phy/mv88e61xx.c
+++ b/drivers/net/phy/mv88e61xx.c
@@ -194,6 +194,17 @@ struct mv88e61xx_phy_priv {
        u8 phy_ctrl1_en_det_width; /* Width of 'EDet' bit field */
        u8 phy_ctrl1_en_det_ctrl;  /* 'EDet' control value */
        u8 direct_access;          /* Access switch device
directly */
+       /*
+        * Bootstrap configuration:
+        *
+        * If addr4 = 1 device is accessible from 0x10 address on
MDIO bus.
+        */
+       u8 addr4;
+       /*
+        * If no_cpu = 1 switch is automatically setup, otherwise
PHY reset is
+        * required from CPU for normal operation.
+        */
+       u8 no_cpu;
   };
static inline int smi_cmd(int cmd, int addr, int reg)
@@ -1218,6 +1229,33 @@ U_BOOT_PHY_DRIVER(mv88e6071) = {
        .shutdown = &genphy_shutdown,
   };
+static int mv88e61xx_read_bootstrap(struct phy_device *phydev)
+{
+       struct mv88e61xx_phy_priv *priv = phydev->priv;
+       struct mii_dev *mdio_bus = priv->mdio_bus;
+       int val;
+
+       /* mv88e6020 - ID = 0x0200 (REG 3 on non PHY port) */
+       if (priv->id == PORT_SWITCH_ID_6020) {
+               /* Prepare to read scratch and misc register */
+               mdio_bus->write(mdio_bus, priv->global2, 0,
+                               0x1a /*MV_SCRATCH_MISC*/,
+                               (0x71 /*MV_CONFIG_DATA1*/ << 8));

Introduce macros for these magic values.

Frankly speaking, this is more readable than macros as it is in sync
with vendor's documentation.

Sorry, I am not buying this argument. Random constant 0x123 is NOT more readable than the standard that the rest of the U-Boot code base uses which is:

#define MV_SCRATCH_MISC 0x1a

and then call(MV_SCRATCH_MISC...) .

Hard-coding random magic register offsets into the code is just sloppy and leads to duplication. The mv88e61xx driver does not do it either, so this patch is violating its coding style, see the top 200 or so lines of the driver. Do not do it.

To make it clear, until this is fixed, NAK .

In other words - it is easier to find this information in documentation
when presented like above.

No

Reply via email to