[RFC] net: stmmac: socfpga: dt binding

[RFC] net: stmmac: socfpga: dt binding

[Steffen Trumtrar]

Hi!

I'm in the middle of getting the IEEE1588/PTP features of the GMAC up and
running. The problem I have is with the current binding however.

The documentation says:

Example:

gmac0: ethernet@ff700000 {
	compatible = "altr,socfpga-stmmac", "snps,dwmac-3.70a", "snps,dwmac";
	altr,sysmgr-syscon = <&sysmgr 0x60 0>;
	status = "disabled";
	reg = <0xff700000 0x2000>;
	interrupts = <0 115 4>;
	interrupt-names = "macirq";
	mac-address = [00 00 00 00 00 00];/* Filled in by U-Boot */
	clocks = <&emac_0_clk>;
	clock-names = "stmmaceth";
};

The problem is with the "altr,sysmgr-syscon" property. It was invented,
because the Arria10 and the Arria5/CycloneV have different register
offsets and shifts. This information is however not enough to describe all
properties of the hardware, but more of a crutch IMHO.
The shifts are for the PHYSEL bits. Of course now I need some
different shifts AND I need to access an additional register with different
shifts on Arria10 and CycloneV in the sysmgr.

So, the standard way of handling this is with a new compatible for each
type and getting rid of the register offset and shift in the property,
I guess. And test the DT and print a deprecation warning if the property
is still there.

Do you see any other/better way of fixing this?

As a side note: Is it reasonable to use the "clk_ptp_ref" property of the
stmmac to decide if the PTP features should be enabled or not?
As the hardware needs to be configured to use this clock, I think the
presence of this property is a good enough indicator.

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] net: stmmac: socfpga: dt binding

[Dinh Nguyen]

On 08/13/2015 02:41 PM, Steffen Trumtrar wrote:
> Hi!
> 
> I'm in the middle of getting the IEEE1588/PTP features of the GMAC up and
> running. The problem I have is with the current binding however.
> 
> The documentation says:
> 
> Example:
> 
> gmac0: ethernet@ff700000 {
> 	compatible = "altr,socfpga-stmmac", "snps,dwmac-3.70a", "snps,dwmac";
> 	altr,sysmgr-syscon = <&sysmgr 0x60 0>;
> 	status = "disabled";
> 	reg = <0xff700000 0x2000>;
> 	interrupts = <0 115 4>;
> 	interrupt-names = "macirq";
> 	mac-address = [00 00 00 00 00 00];/* Filled in by U-Boot */
> 	clocks = <&emac_0_clk>;
> 	clock-names = "stmmaceth";
> };
> 
> The problem is with the "altr,sysmgr-syscon" property. It was invented,
> because the Arria10 and the Arria5/CycloneV have different register

"altr,sysmgr-syscon" was invented before Arria10 was in the picture. It
was invented as a way to reach out and toggle registers that had a
direct impact to the ethernet IP, but was located in a separate IP block.

>From the documentation:

"- altr,sysmgr-syscon : Should be the phandle to the system manager node
that encompasses the glue register, the register offset, and the
register shift."

So "altr,sysmgr-syscon" is a pretty generic property that toggles the
system manager. At the time, I was only using it for the PHYSEL bits and
did not consider the PTP clock select offsets.

> offsets and shifts. This information is however not enough to describe all
> properties of the hardware, but more of a crutch IMHO.
> The shifts are for the PHYSEL bits. Of course now I need some
> different shifts AND I need to access an additional register with different
> shifts on Arria10 and CycloneV in the sysmgr.
> 
> So, the standard way of handling this is with a new compatible for each
> type and getting rid of the register offset and shift in the property,
> I guess. And test the DT and print a deprecation warning if the property
> is still there.
> 
> Do you see any other/better way of fixing this?
> 
> As a side note: Is it reasonable to use the "clk_ptp_ref" property of the
> stmmac to decide if the PTP features should be enabled or not?
> As the hardware needs to be configured to use this clock, I think the
> presence of this property is a good enough indicator.
> 

I think what you're trying to do is reach the PTP clock select bits that
are located in the system manager, right? Should this be done through
the clock manager instead of stmmac driver?

Dinh

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] net: stmmac: socfpga: dt binding

[Steffen Trumtrar]

Hi!

On Fri, Aug 14, 2015 at 10:40:28AM -0500, Dinh Nguyen wrote:
> On 08/13/2015 02:41 PM, Steffen Trumtrar wrote:
> > Hi!
> > 
> > I'm in the middle of getting the IEEE1588/PTP features of the GMAC up and
> > running. The problem I have is with the current binding however.
> > 
> > The documentation says:
> > 
> > Example:
> > 
> > gmac0: ethernet@ff700000 {
> > 	compatible = "altr,socfpga-stmmac", "snps,dwmac-3.70a", "snps,dwmac";
> > 	altr,sysmgr-syscon = <&sysmgr 0x60 0>;
> > 	status = "disabled";
> > 	reg = <0xff700000 0x2000>;
> > 	interrupts = <0 115 4>;
> > 	interrupt-names = "macirq";
> > 	mac-address = [00 00 00 00 00 00];/* Filled in by U-Boot */
> > 	clocks = <&emac_0_clk>;
> > 	clock-names = "stmmaceth";
> > };
> > 
> > The problem is with the "altr,sysmgr-syscon" property. It was invented,
> > because the Arria10 and the Arria5/CycloneV have different register
> 
> "altr,sysmgr-syscon" was invented before Arria10 was in the picture. It
> was invented as a way to reach out and toggle registers that had a
> direct impact to the ethernet IP, but was located in a separate IP block.
> 
> From the documentation:
> 
> "- altr,sysmgr-syscon : Should be the phandle to the system manager node
> that encompasses the glue register, the register offset, and the
> register shift."
> 
> So "altr,sysmgr-syscon" is a pretty generic property that toggles the
> system manager. At the time, I was only using it for the PHYSEL bits and
> did not consider the PTP clock select offsets.
> 

Hm, okay.

> > offsets and shifts. This information is however not enough to describe all
> > properties of the hardware, but more of a crutch IMHO.
> > The shifts are for the PHYSEL bits. Of course now I need some
> > different shifts AND I need to access an additional register with different
> > shifts on Arria10 and CycloneV in the sysmgr.
> > 
> > So, the standard way of handling this is with a new compatible for each
> > type and getting rid of the register offset and shift in the property,
> > I guess. And test the DT and print a deprecation warning if the property
> > is still there.
> > 
> > Do you see any other/better way of fixing this?
> > 
> > As a side note: Is it reasonable to use the "clk_ptp_ref" property of the
> > stmmac to decide if the PTP features should be enabled or not?
> > As the hardware needs to be configured to use this clock, I think the
> > presence of this property is a good enough indicator.
> > 
> 
> I think what you're trying to do is reach the PTP clock select bits that
> are located in the system manager, right? Should this be done through
> the clock manager instead of stmmac driver?
> 

That is also a possibility I guess. Although I'm not sure how that would
look like.
I also need to set the emac_1/emac_0 bits in the FPGA Interface group,
otherwise the clock doesn't reach the EMAC.
These are all random bits somewhere, so syscon is totally fine here. I
personally wouldn't like to specify all this stuff in the devicetree however.
The driver can/does know this stuff if it knows what device it is running on.

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html