Re: [PATCH net-next 2/6] net: dsa: mt7530: Extend device data ready for adding a new hardware
From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2019-12-12 03:45:56
Also in:
linux-devicetree, linux-mediatek, lkml
On 12/10/2019 12:14 AM, Landen Chao wrote:
Add a structure holding required operations for each device such as device initialization, PHY port read or write, a checker whether PHY interface is supported on a certain port, MAC port setup for either bus pad or a specific PHY interface. The patch is done for ready adding a new hardware MT7531. Signed-off-by: Landen Chao <redacted> Signed-off-by: Sean Wang <sean.wang@mediatek.com> ---
[snip]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index ed1ec10ec62b..9a648d1f5d09 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c@@ -425,7 +425,7 @@ mt7530_fdb_write(struct mt7530_priv *priv, u16 vid, } static int -mt7530_pad_clk_setup(struct dsa_switch *ds, int mode) +mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t mode)
Here you pass a phy_interface_t argument but not in mt7632_pad_clk_setup().
+static int
+mt7530_pad_setup(struct dsa_switch *ds, const struct phylink_link_state *state)
+{
+ struct mt7530_priv *priv = ds->priv;
+
+ /* Setup TX circuit incluing relevant PAD and driving */
+ mt7530_pad_clk_setup(ds, state->interface);
+
+ if (priv->id == ID_MT7530) {
+ /* Setup RX circuit, relevant PAD and driving on the
+ * host which must be placed after the setup on the
+ * device side is all finished.
+ */
+ mt7623_pad_clk_setup(ds);Would not it make sense to pass it down here as well for consistency? [snip]
quoted hunk ↗ jump to hunk
@@ -1660,8 +1768,19 @@ mt7530_probe(struct mdio_device *mdiodev) /* Get the hardware identifier from the devicetree node. * We will need it for some of the clock and regulator setup. */ - priv->id = (unsigned int)(unsigned long) - of_device_get_match_data(&mdiodev->dev); + priv->info = of_device_get_match_data(&mdiodev->dev); + if (!priv->info) + return -EINVAL; + + /* Sanity check if these required device operstaions are filled + * properly.
Typo: operations. Other than that, this looks okay to me. -- Florian