Messages in this thread | | | Date | Wed, 27 Apr 2022 02:57:13 +0300 | From | Vladimir Oltean <> | Subject | Re: [RFC v1 1/3] net: dsa: mt753x: make reset optional |
| |
On Tue, Apr 26, 2022 at 03:49:22PM +0200, Frank Wunderlich wrote: > From: Frank Wunderlich <frank-w@public-files.de> > > Currently a reset line is required, but on BPI-R2-Pro board > this reset is shared with the gmac and prevents the switch to > be initialized because mdio is not ready fast enough after > the reset. > > So make the reset optional to allow shared reset lines.
What does it mean "to allow shared reset lines"? Allow as in "allow them to sit there, unused"?
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de> > --- > drivers/net/dsa/mt7530.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > index 19f0035d4410..ccf4cb944167 100644 > --- a/drivers/net/dsa/mt7530.c > +++ b/drivers/net/dsa/mt7530.c > @@ -2134,7 +2134,7 @@ mt7530_setup(struct dsa_switch *ds) > reset_control_assert(priv->rstc); > usleep_range(1000, 1100); > reset_control_deassert(priv->rstc); > - } else { > + } else if (priv->reset) {
I don't really understand this patch. gpiod_set_value_cansleep() can tolerate NULL GPIO descriptors.
> gpiod_set_value_cansleep(priv->reset, 0); > usleep_range(1000, 1100); > gpiod_set_value_cansleep(priv->reset, 1); > @@ -2276,7 +2276,7 @@ mt7531_setup(struct dsa_switch *ds) > reset_control_assert(priv->rstc); > usleep_range(1000, 1100); > reset_control_deassert(priv->rstc); > - } else { > + } else if (priv->reset) { > gpiod_set_value_cansleep(priv->reset, 0); > usleep_range(1000, 1100); > gpiod_set_value_cansleep(priv->reset, 1); > @@ -3272,8 +3272,7 @@ mt7530_probe(struct mdio_device *mdiodev) > priv->reset = devm_gpiod_get_optional(&mdiodev->dev, "reset", > GPIOD_OUT_LOW); > if (IS_ERR(priv->reset)) { > - dev_err(&mdiodev->dev, "Couldn't get our reset line\n"); > - return PTR_ERR(priv->reset); > + dev_warn(&mdiodev->dev, "Couldn't get our reset line\n");
I certainly don't understand why you're suppressing the pointer-encoded errors here. The function used is devm_gpiod_get_optional(), which returns NULL for a missing reset-gpios, not IS_ERR(something). The IS_ERR(something) is actually important to not ignore, maybe it's IS_ERR(-EPROBE_DEFER). And this change breaks waiting for the descriptor to become available.
> } > } > > -- > 2.25.1 >
So what doesn't work without this patch, exactly?
| |