mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v2 1/2] include/phy: add driver_data to resume more of kernel code
@ 2021-10-13  7:53 Oleksij Rempel
  2021-10-13  7:53 ` [PATCH v2 2/2] net: phy: micrel: sync init code for ksz80xx variants with the kernel driver Oleksij Rempel
  0 siblings, 1 reply; 7+ messages in thread
From: Oleksij Rempel @ 2021-10-13  7:53 UTC (permalink / raw)
  To: barebox; +Cc: Oleksij Rempel

Add driver_data pointer to be able to port more of kernel code for
micrel phy.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 include/linux/phy.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index a4cda3e28d..202eeb4d44 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -212,6 +212,7 @@ struct phy_device {
  * phy_id_mask: Defines the important bits of the phy_id
  * features: A list of features (speed, duplex, etc) supported
  *   by this PHY
+ * @driver_data: Static driver data
  *
  * The drivers must implement config_aneg and read_status.  All
  * other functions are optional. Note that none of these
@@ -225,6 +226,7 @@ struct phy_driver {
 	u32 phy_id;
 	unsigned int phy_id_mask;
 	u32 features;
+	const void *driver_data;
 
 	/*
 	 * Called to initialize the PHY,
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 2/2] net: phy: micrel: sync init code for ksz80xx variants with the kernel driver
  2021-10-13  7:53 [PATCH v2 1/2] include/phy: add driver_data to resume more of kernel code Oleksij Rempel
@ 2021-10-13  7:53 ` Oleksij Rempel
  2021-10-13 10:25   ` Trent Piepho
  0 siblings, 1 reply; 7+ messages in thread
From: Oleksij Rempel @ 2021-10-13  7:53 UTC (permalink / raw)
  To: barebox; +Cc: Oleksij Rempel

Sync part of barebox micrel driver with the kernel v5.15-rc1.
This change will affect most of by barebox supported 100Mbit/ksz80xx PHY
variants and provide unified devicetree support for LED and clock configuration.

At same time, NAND and broadcast configuration parts are synced as well
to reduce the difference with the kernel driver.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/micrel.c | 249 +++++++++++++++++++++++++++++++++++----
 1 file changed, 228 insertions(+), 21 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index ea193c84a7..a605e06307 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -14,6 +14,7 @@
 
 #include <common.h>
 #include <init.h>
+#include <linux/clk.h>
 #include <linux/mii.h>
 #include <linux/ethtool.h>
 #include <linux/phy.h>
@@ -24,16 +25,18 @@
 /* Operation Mode Strap Override */
 #define MII_KSZPHY_OMSO				0x16
 #define KSZPHY_OMSO_B_CAST_OFF			BIT(9)
+#define KSZPHY_OMSO_NAND_TREE_ON		BIT(5)
 #define KSZPHY_OMSO_RMII_OVERRIDE		BIT(1)
 #define KSZPHY_OMSO_MII_OVERRIDE		BIT(0)
 
 /* general PHY control reg in vendor specific block. */
-#define	MII_KSZPHY_CTRL				0x1F
+#define	MII_KSZPHY_CTRL				0x1f
 /* bitmap of PHY register to set interrupt mode */
 #define KSZPHY_CTRL_INT_ACTIVE_HIGH		BIT(9)
 #define KSZ9021_CTRL_INT_ACTIVE_HIGH		BIT(14)
 #define KS8737_CTRL_INT_ACTIVE_HIGH		BIT(14)
 #define KSZ8051_RMII_50MHZ_CLK			BIT(7)
+#define KSZPHY_RMII_REF_CLK_SEL			BIT(7)
 
 /* PHY Control 1 */
 #define MII_KSZPHY_CTRL_1			0x1e
@@ -52,6 +55,47 @@
 
 #define PS_TO_REG				200
 
+struct kszphy_type {
+	u32 led_mode_reg;
+	bool has_broadcast_disable;
+	bool has_nand_tree_disable;
+	bool has_rmii_ref_clk_sel;
+};
+
+struct kszphy_priv {
+	const struct kszphy_type *type;
+	int led_mode;
+	bool rmii_ref_clk_sel;
+	bool rmii_ref_clk_sel_val;
+};
+
+static const struct kszphy_type ksz8001_type = {
+	.led_mode_reg		= MII_KSZPHY_CTRL_1,
+};
+
+static const struct kszphy_type ksz8021_type = {
+	.led_mode_reg		= MII_KSZPHY_CTRL,
+	.has_broadcast_disable	= true,
+	.has_nand_tree_disable	= true,
+	.has_rmii_ref_clk_sel	= true,
+};
+
+static const struct kszphy_type ksz8041_type = {
+	.led_mode_reg		= MII_KSZPHY_CTRL_1,
+};
+
+static const struct kszphy_type ksz8051_type = {
+	.led_mode_reg		= MII_KSZPHY_CTRL,
+	.has_nand_tree_disable	= true,
+};
+
+static const struct kszphy_type ksz8081_type = {
+	.led_mode_reg		= MII_KSZPHY_CTRL,
+	.has_broadcast_disable	= true,
+	.has_nand_tree_disable	= true,
+	.has_rmii_ref_clk_sel	= true,
+};
+
 static int kszphy_extended_write(struct phy_device *phydev,
 				u32 regnum, u16 val)
 {
@@ -66,6 +110,22 @@ static int kszphy_extended_read(struct phy_device *phydev,
 	return phy_read(phydev, MII_KSZPHY_EXTREG_READ);
 }
 
+static int kszphy_rmii_clk_sel(struct phy_device *phydev, bool val)
+{
+	int ctrl;
+
+	ctrl = phy_read(phydev, MII_KSZPHY_CTRL);
+	if (ctrl < 0)
+		return ctrl;
+
+	if (val)
+		ctrl |= KSZPHY_RMII_REF_CLK_SEL;
+	else
+		ctrl &= ~KSZPHY_RMII_REF_CLK_SEL;
+
+	return phy_write(phydev, MII_KSZPHY_CTRL, ctrl);
+}
+
 /* Handle LED mode, shift = position of first led mode bit, usually 4 or 14 */
 static int kszphy_led_mode(struct phy_device *phydev, int reg, int shift)
 {
@@ -83,37 +143,119 @@ static int kszphy_led_mode(struct phy_device *phydev, int reg, int shift)
 	return 0;
 }
 
-static int kszphy_config_init(struct phy_device *phydev)
+static int kszphy_setup_led(struct phy_device *phydev, u32 reg, int val)
 {
-	kszphy_led_mode(phydev, MII_KSZPHY_CTRL_1, 14);
+	const struct device_d *dev = &phydev->dev;
+	int rc, temp, shift;
 
-	return 0;
+	switch (reg) {
+	case MII_KSZPHY_CTRL_1:
+		shift = 14;
+		break;
+	case MII_KSZPHY_CTRL:
+		shift = 4;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	temp = phy_read(phydev, reg);
+	if (temp < 0) {
+		rc = temp;
+		goto out;
+	}
+
+	temp &= ~(3 << shift);
+	temp |= val << shift;
+	rc = phy_write(phydev, reg, temp);
+out:
+	if (rc < 0)
+		dev_err(dev, "failed to set led mode\n");
+
+	return rc;
 }
 
-static int ksz8021_config_init(struct phy_device *phydev)
+/* Disable PHY address 0 as the broadcast address, so that it can be used as a
+ * unique (non-broadcast) address on a shared bus.
+ */
+static int kszphy_broadcast_disable(struct phy_device *phydev)
 {
-	phy_set_bits(phydev, MII_KSZPHY_OMSO, KSZPHY_OMSO_B_CAST_OFF);
+	const struct device_d *dev = &phydev->dev;
+	int ret;
 
-	kszphy_led_mode(phydev, MII_KSZPHY_CTRL, 4);
+	ret = phy_read(phydev, MII_KSZPHY_OMSO);
+	if (ret < 0)
+		goto out;
 
-	return 0;
+	ret = phy_write(phydev, MII_KSZPHY_OMSO, ret | KSZPHY_OMSO_B_CAST_OFF);
+out:
+	if (ret)
+		dev_err(dev, "failed to disable broadcast address\n");
+
+	return ret;
 }
 
-static int ks8051_config_init(struct phy_device *phydev)
+static int kszphy_nand_tree_disable(struct phy_device *phydev)
 {
-	int regval;
+	const struct device_d *dev = &phydev->dev;
+	int ret;
+
+	ret = phy_read(phydev, MII_KSZPHY_OMSO);
+	if (ret < 0)
+		goto out;
+
+	if (!(ret & KSZPHY_OMSO_NAND_TREE_ON))
+		return 0;
+
+	ret = phy_write(phydev, MII_KSZPHY_OMSO,
+			ret & ~KSZPHY_OMSO_NAND_TREE_ON);
+out:
+	if (ret)
+		dev_err(dev, "failed to disable NAND tree mode\n");
+
+	return ret;
+}
+
+/* Some config bits need to be set again on resume, handle them here. */
+static int kszphy_config_reset(struct phy_device *phydev)
+{
+	struct kszphy_priv *priv = phydev->priv;
+	int ret;
 
-	if (phydev->dev_flags & MICREL_PHY_50MHZ_CLK) {
-		regval = phy_read(phydev, MII_KSZPHY_CTRL);
-		regval |= KSZ8051_RMII_50MHZ_CLK;
-		phy_write(phydev, MII_KSZPHY_CTRL, regval);
+	if (priv->rmii_ref_clk_sel) {
+		ret = kszphy_rmii_clk_sel(phydev, priv->rmii_ref_clk_sel_val);
+		if (ret) {
+			dev_err(&phydev->dev,
+				   "failed to set rmii reference clock\n");
+			return ret;
+		}
 	}
 
-	kszphy_led_mode(phydev, MII_KSZPHY_CTRL, 4);
+	if (priv->led_mode >= 0)
+		kszphy_setup_led(phydev, priv->type->led_mode_reg, priv->led_mode);
 
 	return 0;
 }
 
+static int kszphy_config_init(struct phy_device *phydev)
+{
+	struct kszphy_priv *priv = phydev->priv;
+	const struct kszphy_type *type;
+
+	if (!priv)
+		return 0;
+
+	type = priv->type;
+
+	if (type->has_broadcast_disable)
+		kszphy_broadcast_disable(phydev);
+
+	if (type->has_nand_tree_disable)
+		kszphy_nand_tree_disable(phydev);
+
+	return kszphy_config_reset(phydev);
+}
+
 static int ksz9021_load_values_from_of(struct phy_device *phydev,
 				       const struct device_node *of_node,
 				       u16 reg, const char *field[])
@@ -468,13 +610,66 @@ static int ksz8873mll_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int kszphy_probe(struct phy_device *phydev)
+{
+	struct device_d *dev = &phydev->dev;
+	struct device_node *np = dev->device_node;
+	struct phy_driver *drv = to_phy_driver(dev->driver);
+	const struct kszphy_type *type = drv->driver_data;
+	struct kszphy_priv *priv;
+	struct clk *clk;
+	int ret;
+
+	priv = xzalloc(sizeof(*priv));
+
+	phydev->priv = priv;
+
+	priv->type = type;
+
+	if (type->led_mode_reg) {
+		ret = of_property_read_u32(np, "micrel,led-mode",
+				&priv->led_mode);
+		if (ret)
+			priv->led_mode = -1;
+
+		if (priv->led_mode > 3) {
+			dev_err(dev, "invalid led mode: 0x%02x\n",
+				priv->led_mode);
+			priv->led_mode = -1;
+		}
+	} else {
+		priv->led_mode = -1;
+	}
+
+	clk = clk_get(dev, "rmii-ref");
+	/* NOTE: clk may be NULL if building without CONFIG_HAVE_CLK */
+	if (!IS_ERR_OR_NULL(clk)) {
+		unsigned long rate = clk_get_rate(clk);
+		bool rmii_ref_clk_sel_25_mhz;
+
+		priv->rmii_ref_clk_sel = type->has_rmii_ref_clk_sel;
+		rmii_ref_clk_sel_25_mhz = of_property_read_bool(np,
+				"micrel,rmii-reference-clock-select-25-mhz");
+
+		if (rate > 24500000 && rate < 25500000) {
+			priv->rmii_ref_clk_sel_val = rmii_ref_clk_sel_25_mhz;
+		} else if (rate > 49500000 && rate < 50500000) {
+			priv->rmii_ref_clk_sel_val = !rmii_ref_clk_sel_25_mhz;
+		} else {
+			dev_err(dev, "Clock rate out of range: %ld\n", rate);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static struct phy_driver ksphy_driver[] = {
 {
 	.phy_id		= PHY_ID_KS8737,
 	.phy_id_mask	= 0x00fffff0,
 	.drv.name	= "Micrel KS8737",
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
-	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 }, {
@@ -483,7 +678,9 @@ static struct phy_driver ksphy_driver[] = {
 	.drv.name	= "Micrel KSZ8021",
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause |
 			   SUPPORTED_Asym_Pause),
-	.config_init	= ksz8021_config_init,
+	.driver_data	= &ksz8021_type,
+	.probe		= kszphy_probe,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 }, {
@@ -492,7 +689,9 @@ static struct phy_driver ksphy_driver[] = {
 	.drv.name	= "Micrel KSZ8031",
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause |
 			   SUPPORTED_Asym_Pause),
-	.config_init	= ksz8021_config_init,
+	.driver_data	= &ksz8021_type,
+	.probe		= kszphy_probe,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 }, {
@@ -501,6 +700,8 @@ static struct phy_driver ksphy_driver[] = {
 	.drv.name	= "Micrel KSZ8041",
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause
 				| SUPPORTED_Asym_Pause),
+	.driver_data	= &ksz8041_type,
+	.probe		= kszphy_probe,
 	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
@@ -510,22 +711,28 @@ static struct phy_driver ksphy_driver[] = {
 	.drv.name	= "Micrel KSZ8051",
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause
 				| SUPPORTED_Asym_Pause),
-	.config_init	= ks8051_config_init,
+	.driver_data	= &ksz8051_type,
+	.probe		= kszphy_probe,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 }, {
 	.phy_id		= PHY_ID_KSZ8081,
 	.phy_id_mask	= MICREL_PHY_ID_MASK,
 	.drv.name	= "Micrel KSZ8081/91",
+	.driver_data	= &ksz8081_type,
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
-	.config_init	= ksz8021_config_init,
+	.probe		= kszphy_probe,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 }, {
 	.phy_id		= PHY_ID_KSZ8001,
-	.drv.name	= "Micrel KSZ8001 or KS8721",
 	.phy_id_mask	= 0x00ffffff,
+	.drv.name	= "Micrel KSZ8001 or KS8721",
+	.driver_data	= &ksz8001_type,
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
+	.probe		= kszphy_probe,
 	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] net: phy: micrel: sync init code for ksz80xx variants with the kernel driver
  2021-10-13  7:53 ` [PATCH v2 2/2] net: phy: micrel: sync init code for ksz80xx variants with the kernel driver Oleksij Rempel
@ 2021-10-13 10:25   ` Trent Piepho
  2021-10-13 11:19     ` Oleksij Rempel
  0 siblings, 1 reply; 7+ messages in thread
From: Trent Piepho @ 2021-10-13 10:25 UTC (permalink / raw)
  To: Oleksij Rempel; +Cc: Barebox List

On Wed, Oct 13, 2021 at 2:43 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>
> Sync part of barebox micrel driver with the kernel v5.15-rc1.
> This change will affect most of by barebox supported 100Mbit/ksz80xx PHY
> variants and provide unified devicetree support for LED and clock configuration.

I already added LED mode OF support to this driver.

>  drivers/net/phy/micrel.c | 249 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 228 insertions(+), 21 deletions(-)

227 added lines is a lot to do basically nothing not already done.

>  #define KSZ8051_RMII_50MHZ_CLK                 BIT(7)
> +#define KSZPHY_RMII_REF_CLK_SEL                        BIT(7)

These are the same bit in the same register!

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] net: phy: micrel: sync init code for ksz80xx variants with the kernel driver
  2021-10-13 10:25   ` Trent Piepho
@ 2021-10-13 11:19     ` Oleksij Rempel
  2021-10-14 20:32       ` Trent Piepho
  0 siblings, 1 reply; 7+ messages in thread
From: Oleksij Rempel @ 2021-10-13 11:19 UTC (permalink / raw)
  To: Trent Piepho; +Cc: Barebox List

On Wed, Oct 13, 2021 at 03:25:17AM -0700, Trent Piepho wrote:
> On Wed, Oct 13, 2021 at 2:43 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> >
> > Sync part of barebox micrel driver with the kernel v5.15-rc1.
> > This change will affect most of by barebox supported 100Mbit/ksz80xx PHY
> > variants and provide unified devicetree support for LED and clock configuration.
> 
> I already added LED mode OF support to this driver.

Yes, it was partially incorrect. It attempted to write to not existing or not
documented register of PHY_ID_KS8737.

This is the reason why I prefer to share driver code base with kernel,
where possible.

> >  drivers/net/phy/micrel.c | 249 +++++++++++++++++++++++++++++++++++----
> >  1 file changed, 228 insertions(+), 21 deletions(-)
> 
> 227 added lines is a lot to do basically nothing not already done.

I'm not sure what is your point

> >  #define KSZ8051_RMII_50MHZ_CLK                 BIT(7)
> > +#define KSZPHY_RMII_REF_CLK_SEL                        BIT(7)
> 
> These are the same bit in the same register!

Thank you! I'll fix it.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] net: phy: micrel: sync init code for ksz80xx variants with the kernel driver
  2021-10-13 11:19     ` Oleksij Rempel
@ 2021-10-14 20:32       ` Trent Piepho
  2021-10-15  6:36         ` Oleksij Rempel
  2021-10-15  7:11         ` Sascha Hauer
  0 siblings, 2 replies; 7+ messages in thread
From: Trent Piepho @ 2021-10-14 20:32 UTC (permalink / raw)
  To: Oleksij Rempel; +Cc: Barebox List

On Wed, Oct 13, 2021 at 4:19 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> On Wed, Oct 13, 2021 at 03:25:17AM -0700, Trent Piepho wrote:
> > On Wed, Oct 13, 2021 at 2:43 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > >
> > > Sync part of barebox micrel driver with the kernel v5.15-rc1.
> > > This change will affect most of by barebox supported 100Mbit/ksz80xx PHY
> > > variants and provide unified devicetree support for LED and clock configuration.
> >
> > I already added LED mode OF support to this driver.
>
> Yes, it was partially incorrect. It attempted to write to not existing or not
> documented register of PHY_ID_KS8737.

That is not true.  KS8737 would only attempt to set the led mode bits
if the device tree tries to set the led mode.  Since KS8737 does not
have a led mode selection, the device tree should not have this, and
there is no problem.

If you want a device tree validator, then use the yaml dts spec to do
this in the proper way, at build time.  The bootloader has the the
most constrained size of any part of a Linux system and is also the
most critical for device start up time.  It is the worst possible
place to put a dts validator.

> This is the reason why I prefer to share driver code base with kernel,
> where possible.

You again ignore there are two ways to do this.  Make the kernel match
Barebox where the Barebox code is better.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] net: phy: micrel: sync init code for ksz80xx variants with the kernel driver
  2021-10-14 20:32       ` Trent Piepho
@ 2021-10-15  6:36         ` Oleksij Rempel
  2021-10-15  7:11         ` Sascha Hauer
  1 sibling, 0 replies; 7+ messages in thread
From: Oleksij Rempel @ 2021-10-15  6:36 UTC (permalink / raw)
  To: Trent Piepho; +Cc: Barebox List

On Thu, Oct 14, 2021 at 01:32:27PM -0700, Trent Piepho wrote:
> On Wed, Oct 13, 2021 at 4:19 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > On Wed, Oct 13, 2021 at 03:25:17AM -0700, Trent Piepho wrote:
> > > On Wed, Oct 13, 2021 at 2:43 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > > >
> > > > Sync part of barebox micrel driver with the kernel v5.15-rc1.
> > > > This change will affect most of by barebox supported 100Mbit/ksz80xx PHY
> > > > variants and provide unified devicetree support for LED and clock configuration.
> > >
> > > I already added LED mode OF support to this driver.
> >
> > Yes, it was partially incorrect. It attempted to write to not existing or not
> > documented register of PHY_ID_KS8737.
> 
> That is not true.  KS8737 would only attempt to set the led mode bits
> if the device tree tries to set the led mode.  Since KS8737 does not
> have a led mode selection, the device tree should not have this, and
> there is no problem.
> 
> If you want a device tree validator, then use the yaml dts spec to do
> this in the proper way, at build time. 

Please, verify your suggestion.

> The bootloader has the the
> most constrained size of any part of a Linux system and is also the
> most critical for device start up time.  It is the worst possible
> place to put a dts validator.

After you'll spend time on verifying your previous suggestion. I'll be
able to explain, why this part has tiny problem.

> > This is the reason why I prefer to share driver code base with kernel,
> > where possible.
> 
> You again ignore there are two ways to do this.

Board code and phy fixups? Please reread my previous answer. But from
your yuml suggestion I hope to know main misunderstanding in this
discussion. 

> Make the kernel match Barebox where the Barebox code is better.

>From this discussion, i assume you are using linux kernel. This "bad"
kernel code runs on your system and many other devices in the world.
But, you actually do not care to send kernel patches. So, kernel code
quality is not a big issue for you? The code which actually runs on your
system?

Well, it looks like, your code quality concept seems to be limited by
definition of code size, at least for barebox. But, I hope, after you
understand the root of PHY related challenges, you'll understand why
most of your suggestion will work only on some limited amount of use
cases. 

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] net: phy: micrel: sync init code for ksz80xx variants with the kernel driver
  2021-10-14 20:32       ` Trent Piepho
  2021-10-15  6:36         ` Oleksij Rempel
@ 2021-10-15  7:11         ` Sascha Hauer
  1 sibling, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2021-10-15  7:11 UTC (permalink / raw)
  To: Trent Piepho; +Cc: Oleksij Rempel, Barebox List

On Thu, Oct 14, 2021 at 01:32:27PM -0700, Trent Piepho wrote:
> On Wed, Oct 13, 2021 at 4:19 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > On Wed, Oct 13, 2021 at 03:25:17AM -0700, Trent Piepho wrote:
> > > On Wed, Oct 13, 2021 at 2:43 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > > >
> > > > Sync part of barebox micrel driver with the kernel v5.15-rc1.
> > > > This change will affect most of by barebox supported 100Mbit/ksz80xx PHY
> > > > variants and provide unified devicetree support for LED and clock configuration.
> > >
> > > I already added LED mode OF support to this driver.
> >
> > Yes, it was partially incorrect. It attempted to write to not existing or not
> > documented register of PHY_ID_KS8737.
> 
> That is not true.  KS8737 would only attempt to set the led mode bits
> if the device tree tries to set the led mode.  Since KS8737 does not
> have a led mode selection, the device tree should not have this, and
> there is no problem.
> 
> If you want a device tree validator, then use the yaml dts spec to do
> this in the proper way, at build time.  The bootloader has the the
> most constrained size of any part of a Linux system and is also the
> most critical for device start up time.  It is the worst possible
> place to put a dts validator.
> 
> > This is the reason why I prefer to share driver code base with kernel,
> > where possible.
> 
> You again ignore there are two ways to do this.  Make the kernel match
> Barebox where the Barebox code is better.

Maybe Oleksij can split that patch up a little so that we can better see
which changes we want and which not.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-10-15  7:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-13  7:53 [PATCH v2 1/2] include/phy: add driver_data to resume more of kernel code Oleksij Rempel
2021-10-13  7:53 ` [PATCH v2 2/2] net: phy: micrel: sync init code for ksz80xx variants with the kernel driver Oleksij Rempel
2021-10-13 10:25   ` Trent Piepho
2021-10-13 11:19     ` Oleksij Rempel
2021-10-14 20:32       ` Trent Piepho
2021-10-15  6:36         ` Oleksij Rempel
2021-10-15  7:11         ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox