diff mbox

[U-Boot,1/8] net: zynq: Don't overwrite gem_rclk_ctrl with default value

Message ID 1483532844-20649-2-git-send-email-stefan.herbrechtsmeier@weidmueller.com
State Superseded
Delegated to: Michal Simek
Headers show

Commit Message

Herbrechtsmeier Dr.-Ing. , Stefan Jan. 4, 2017, 12:27 p.m. UTC
From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.de>

The gem[0-1]_rclk_ctrl registers control the source of the rx clock,
control and data signals and configure via ps7_init function. Don't
overwrite the register with the default value.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

 arch/arm/mach-zynq/slcr.c | 7 -------
 drivers/net/zynq_gem.c    | 9 ++-------
 2 files changed, 2 insertions(+), 14 deletions(-)

Comments

Michal Simek Jan. 10, 2017, 1:52 p.m. UTC | #1
On 4.1.2017 13:27, stefan.herbrechtsmeier@weidmueller.com wrote:
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.de>
> 
> The gem[0-1]_rclk_ctrl registers control the source of the rx clock,
> control and data signals and configure via ps7_init function. Don't
> overwrite the register with the default value.

TBH I don't think this is a win for us to relate on ps7_init
configuration. There are a lot of things there but I would like to
control this in this sw instead of psu_init.

Thanks,
Michal
Herbrechtsmeier Dr.-Ing. , Stefan Jan. 10, 2017, 2:28 p.m. UTC | #2
Hi Michal,

> -----Ursprüngliche Nachricht-----
> Von: Michal Simek [mailto:monstr@monstr.eu]
> Gesendet: Dienstag, 10. Januar 2017 14:53
> An: Herbrechtsmeier, Stefan; u-boot@lists.denx.de
> Cc: Herbrechtsmeier, Stefan; Michal Simek; Jagan Teki; Albert Aribaud;
> Joe Hershberger; Mike Looijmans
> Betreff: Re: [PATCH 1/8] net: zynq: Don't overwrite gem_rclk_ctrl with
> default value
> 
> On 4.1.2017 13:27, stefan.herbrechtsmeier@weidmueller.com wrote:
> > From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.de>
> >
> > The gem[0-1]_rclk_ctrl registers control the source of the rx clock,
> > control and data signals and configure via ps7_init function. Don't
> > overwrite the register with the default value.
> 
> TBH I don't think this is a win for us to relate on ps7_init
> configuration. There are a lot of things there but I would like to
> control this in this sw instead of psu_init.

At the moment the ps7_init only touch the register if the board doesn't use the default configuration. U-boot overwrite the register with the default value if 'xlnx,emio' isn't set in the device tree. It doesn't change the value to gem_rclk_ctrl if 'xlnx,emio' is set.

At the moment the clock configuration is managed by the ps7_init and only the gem_rclk_ctrl configuration is overwritten by u-boot. I don't think it is correct do overwrite some part of the clock tree configuration which is generated by Xilinx tools.

Regards,
  Stefan



Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 - 
Komplementärin: Weidmüller Interface Führungsgesellschaft mbH - 
Sitz: Detmold - Amtsgericht Lemgo HRB 3924; 
Geschäftsführer: José Carlos Álvarez Tobar, Elke Eckstein, Dr. Peter Köhler, Jörg Timmermann;
USt-ID-Nr. DE124599660
Michal Simek Jan. 10, 2017, 3:26 p.m. UTC | #3
+siva

On 10.1.2017 15:28, Stefan.Herbrechtsmeier@weidmueller.com wrote:
> Hi Michal,
> 
>> -----Ursprüngliche Nachricht-----
>> Von: Michal Simek [mailto:monstr@monstr.eu]
>> Gesendet: Dienstag, 10. Januar 2017 14:53
>> An: Herbrechtsmeier, Stefan; u-boot@lists.denx.de
>> Cc: Herbrechtsmeier, Stefan; Michal Simek; Jagan Teki; Albert Aribaud;
>> Joe Hershberger; Mike Looijmans
>> Betreff: Re: [PATCH 1/8] net: zynq: Don't overwrite gem_rclk_ctrl with
>> default value
>>
>> On 4.1.2017 13:27, stefan.herbrechtsmeier@weidmueller.com wrote:
>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.de>
>>>
>>> The gem[0-1]_rclk_ctrl registers control the source of the rx clock,
>>> control and data signals and configure via ps7_init function. Don't
>>> overwrite the register with the default value.
>>
>> TBH I don't think this is a win for us to relate on ps7_init
>> configuration. There are a lot of things there but I would like to
>> control this in this sw instead of psu_init.
> 
> At the moment the ps7_init only touch the register if the board doesn't use the default configuration. U-boot overwrite the register with the default value if 'xlnx,emio' isn't set in the device tree. It doesn't change the value to gem_rclk_ctrl if 'xlnx,emio' is set.
> 
> At the moment the clock configuration is managed by the ps7_init and only the gem_rclk_ctrl configuration is overwritten by u-boot. I don't think it is correct do overwrite some part of the clock tree configuration which is generated by Xilinx tools.

ok.

Siva: I don't have testcase for emio or any hw design to test this.
Do you have something which we can check?

Thanks,
Michal
diff mbox

Patch

diff --git a/arch/arm/mach-zynq/slcr.c b/arch/arm/mach-zynq/slcr.c
index 2d3bf2a..c1129cd 100644
--- a/arch/arm/mach-zynq/slcr.c
+++ b/arch/arm/mach-zynq/slcr.c
@@ -140,13 +140,6 @@  void zynq_slcr_gem_clk_setup(u32 gem_id, unsigned long clk_rate)
 	if (ret)
 		goto out;
 
-	if (gem_id) {
-		/* Configure GEM_RCLK_CTRL */
-		writel(1, &slcr_base->gem1_rclk_ctrl);
-	} else {
-		/* Configure GEM_RCLK_CTRL */
-		writel(1, &slcr_base->gem0_rclk_ctrl);
-	}
 	udelay(100000);
 out:
 	zynq_slcr_lock();
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
index d2e5e7c..9bfb89f 100644
--- a/drivers/net/zynq_gem.c
+++ b/drivers/net/zynq_gem.c
@@ -174,7 +174,6 @@  struct zynq_gem_priv {
 	u32 rxbd_current;
 	u32 rx_first_buf;
 	int phyaddr;
-	u32 emio;
 	int init;
 	struct zynq_gem_regs *iobase;
 	phy_interface_t interface;
@@ -467,10 +466,8 @@  static int zynq_gem_init(struct udevice *dev)
 		break;
 	}
 
-	/* Change the rclk and clk only not using EMIO interface */
-	if (!priv->emio)
-		zynq_slcr_gem_clk_setup((ulong)priv->iobase !=
-					ZYNQ_GEM_BASEADDR0, clk_rate);
+	zynq_slcr_gem_clk_setup((ulong)priv->iobase !=
+				ZYNQ_GEM_BASEADDR0, clk_rate);
 
 	setbits_le32(&regs->nwctrl, ZYNQ_GEM_NWCTRL_RXEN_MASK |
 					ZYNQ_GEM_NWCTRL_TXEN_MASK);
@@ -685,7 +682,6 @@  static int zynq_gem_ofdata_to_platdata(struct udevice *dev)
 	pdata->iobase = (phys_addr_t)dev_get_addr(dev);
 	priv->iobase = (struct zynq_gem_regs *)pdata->iobase;
 	/* Hardcode for now */
-	priv->emio = 0;
 	priv->phyaddr = -1;
 
 	priv->phy_of_handle = fdtdec_lookup_phandle(gd->fdt_blob,
@@ -703,7 +699,6 @@  static int zynq_gem_ofdata_to_platdata(struct udevice *dev)
 	}
 	priv->interface = pdata->phy_interface;
 
-	priv->emio = fdtdec_get_bool(gd->fdt_blob, dev->of_offset, "xlnx,emio");
 
 	printf("ZYNQ GEM: %lx, phyaddr %x, interface %s\n", (ulong)priv->iobase,
 	       priv->phyaddr, phy_string_for_interface(priv->interface));