ARM: OMAP3+: hwmod: Add AM33XX HWMOD data for davinci_mdio

Would you please take this bugfix for 3.7-rc2? The suggestion to mail you came from Toni Lindgren. The context where it came from is here:

http://lists.arm.linux.org.uk/lurker/message/20121015.191630.bdae3c50.en.html

Thanks,
Richard

----- Forwarded message from Richard Cochran <[email protected]> -----

Date: Mon, 15 Oct 2012 21:16:32 +0200
From: Richard Cochran <[email protected]>
To: netdev at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org, Arnd Bergmann <[email protected]>,
David Miller <[email protected]>,
Russell King <[email protected]>,
Mugunthan V N <[email protected]>
Subject: [PATCH 2/5] ARM: OMAP3+: hwmod: Add AM33XX HWMOD data for davinci_mdio
X-Mailer: git-send-email 1.7.2.5

From: Mugunthan V N <[email protected]>

This patch adds minimal hwmod support for davinci mdio driver. This patch requires rework on parent child relation between cpsw and davinci mdio hwmod data to support runtime PM.

Signed-off-by Mugunthan V N <[email protected]>

---
 arch/arm/mach-omap2/omap_hwmod_33xx_data.c |   34 ++++++++++++++++++++++++++-
 1 files changed, 32 insertions(+), 2 deletions(-)

arch/arm/mach-omap2/omap_hwmod_33xx_data.c +32 -2

index 59d5c1c..f96bbc0 100644
@@ -650,8 +650,7 @@ static struct omap_hwmod_class_sysconfig am33xx_cpgmac_sysc = {
 	.rev_offs	= 0x0,
 	.sysc_offs	= 0x8,
 	.syss_offs	= 0x4,
-	.sysc_flags	= (SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE |
-			   SYSS_HAS_RESET_STATUS),
+	.sysc_flags	= (SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE),
 	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | MSTANDBY_FORCE |
 			   MSTANDBY_NO),
 	.sysc_fields	= &omap_hwmod_sysc_type3,

@@ -682,6 +681,8 @@ static struct omap_hwmod am33xx_cpgmac0_hwmod = {
 			.modulemode	= MODULEMODE_SWCTRL,
 		},
 	},
+	.flags		= (HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY |
+			   HWMOD_INIT_NO_RESET | HWMOD_INIT_NO_IDLE),
 };
 
 /*

@@ -2510,6 +2511,34 @@ static struct omap_hwmod_addr_space am33xx_elm_addr_space[] = {
 	{ }
 };
 
+/* mdio class */
+static struct omap_hwmod_class am33xx_mdio_hwmod_class = {
+	.name		= "davinci_mdio",
+};
+
+struct omap_hwmod_addr_space am33xx_mdio_addr_space[] = {
+	{
+		.pa_start	= 0x4A101000,
+		.pa_end		= 0x4A101000 + SZ_256 - 1,
+		.flags		= ADDR_MAP_ON_INIT,
+	},
+	{ }
+};
+
+static struct omap_hwmod am33xx_mdio_hwmod = {
+	.name		= "davinci_mdio",
+	.class		= &am33xx_mdio_hwmod_class,
+	.clkdm_name	= "cpsw_125mhz_clkdm",
+	.main_clk	= "cpsw_125mhz_gclk",
+};
+
+struct omap_hwmod_ocp_if am33xx_cpgmac0__mdio = {
+	.master		= &am33xx_cpgmac0_hwmod,
+	.slave		= &am33xx_mdio_hwmod,
+	.addr		= am33xx_mdio_addr_space,
+	.user		= OCP_USER_MPU,
+};
+
 static struct omap_hwmod_ocp_if am33xx_l4_ls__elm = {
 	.master		= &am33xx_l4_ls_hwmod,
 	.slave		= &am33xx_elm_hwmod,

@@ -3371,6 +3400,7 @@ static struct omap_hwmod_ocp_if *am33xx_hwmod_ocp_ifs[] __initdata = {
 	&am33xx_l3_main__tptc2,
 	&am33xx_l3_s__usbss,
 	&am33xx_l4_hs__cpgmac0,
+	&am33xx_cpgmac0__mdio,
 	NULL,
 };
 

Signature · 4 lines

1.7.2.5


----- End forwarded message -----

Hi Richard

On Wed, 17 Oct 2012, Richard Cochran wrote:

Quoted 4 lines

Would you please take this bugfix for 3.7-rc2? The suggestion to mail you came from Toni Lindgren. The context where it came from is here:

http://lists.arm.linux.org.uk/lurker/message/20121015.191630.bdae3c50.en.html

This patch appears to add a new feature, correct? I don't think the CPSW could have worked in the past without this data present. So it looks to me like this is 3.8 material, unless Tony would like it to go in sooner?

Quoted 7 lines · Paul Walmsley

Hi Richard

Quoted 3 lines · Richard Cochran

Would you please take this bugfix for 3.7-rc2? The suggestion to mail you came from Toni Lindgren. The context where it came from is here:

Quoted 5 lines

Quoted 1 line

http://lists.arm.linux.org.uk/lurker/message/20121015.191630.bdae3c50.en.html

This patch appears to add a new feature, correct? I don't think the CPSW could have worked in the past without this data present. So it looks to me like this is 3.8 material, unless Tony would like it to go in sooner?

Yeah unless it fixes something, we should just queue it for v3.8 merge window.

So there has been this cpsw driver since v3.4-rc1~177^2~5

df82859 netdev: driver: ethernet: Add TI CPSW driver

and four people signed off on it, so it must have been working at one point.

Since the device tree make-over, the driver is a derelict, and thus the present patch is fixing a regression.

I just want the already merged driver to work with the vanilla kernel. Is that too much to ask?

It doesn't necessarily mean that the driver is usable in that kernel release.

Either way, the patch is likely to make it into the mainline kernel. It's just that it will probably take a few weeks longer than you might like.

I *do* mind if y'all go merging all kinds of code that has never been tested. Those who submit and their reviewers should ensure that the code is actually working. Before buying hardware or starting projects, I often check what is in mainline. I expect that mainline has been at least minimally tested.

Maybe that is just me, or maybe people working on the ARM tree have different expectations.

Probably the driver was submitted before any SoC integration support was available. Grepping for 'cpsw' under arch/ turns up only AM33xx. AM335x didn't have device enumeration support in the mainline kernel until 3.7, via commit a2cfc509bc4eeef9f5c4607b1203f17f22ea2a36 ("ARM: OMAP3+: hwmod:
Add AM33XX HWMOD data"). So I don't see how it could have worked with mainline.

But it seems that the CPSW has never worked in the mainline kernel. So this particular patch is not fixing a regression. Therefore we shouldn't send it upstream during the -rc time period. Instead we'll schedule it to be sent a few weeks later, during the 3.8 merge window.

You say it will be working in 3.8, after 3.7 comes out, in about December.

Sorry to harp on this so much, but I try out my patches with the tip of the appropriate tree before sending them, or when I cannot, at least I put into the log "compile tested only" or something like that.

I wrote that we'll schedule the SoC integration patch for sending upstream for 3.8. This does not necessarily mean that our upstreams will take it, or that it will result in a working CPSW.

Quoted 7 lines · Paul Walmsley

Probably the driver was submitted before any SoC integration support was available. Grepping for 'cpsw' under arch/ turns up only AM33xx. AM335x didn't have device enumeration support in the mainline kernel until 3.7, via commit a2cfc509bc4eeef9f5c4607b1203f17f22ea2a36 ("ARM: OMAP3+: hwmod:
Add AM33XX HWMOD data"). So I don't see how it could have worked with mainline.

...

Quoted 4 lines

But it seems that the CPSW has never worked in the mainline kernel. So this particular patch is not fixing a regression. Therefore we shouldn't send it upstream during the -rc time period. Instead we'll schedule it to be sent a few weeks later, during the 3.8 merge window.

Let's think about this some more:

The driver's commit is from March and is included in v3.4-rc1.

You say it will be working in 3.8, after 3.7 comes out, in about December.

So how long does it take between merging code and actually getting it working?

Sorry to harp on this so much, but I try out my patches with the tip of the appropriate tree before sending them, or when I cannot, at least I put into the log "compile tested only" or something like that.

Thanks,
Richard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Another important point is, this driver is also required and used for Davinci family of devices (arch/mach/mach-davinci/).

Quoted 3 lines · Vaibhav Hiremath

Another important point is, this driver is also required and used for Davinci family of devices (arch/mach/mach-davinci/).

That is really beside the point. If the code isn't ready yet, then don't merge it.

When I asked about the beaglebone, I was given the impression that it will be ready for v3.7-rc1. But, as I know realize, at the current rate, it might not even be ready for v3.8.

I don't mind waiting, but please make sure that whatever lands into a release is really, truly working.

Quoted 8 lines · Richard Cochran

Quoted 3 lines · Vaibhav Hiremath

Another important point is, this driver is also required and used for Davinci family of devices (arch/mach/mach-davinci/).

That is really beside the point. If the code isn't ready yet, then don't merge it.

Quoted 6 lines

When I asked about the beaglebone, I was given the impression that it will be ready for v3.7-rc1. But, as I know realize, at the current rate, it might not even be ready for v3.8.

I don't mind waiting, but please make sure that whatever lands into a release is really, truly working.

Indeed. This has been a problem with many of the TI patches in general. People are working on separate product trees and then produce patches for the mainline kernel that are poorly tested.

The requirement should be: Do your patch on an omap board then send the patch from your omap board after booting your board with the patch :)

Quoted 12 lines · Richard Cochran

Quoted 3 lines · Vaibhav Hiremath

Another important point is, this driver is also required and used for Davinci family of devices (arch/mach/mach-davinci/).

That is really beside the point. If the code isn't ready yet, then don't merge it.

When I asked about the beaglebone, I was given the impression that it will be ready for v3.7-rc1. But, as I know realize, at the current rate, it might not even be ready for v3.8.

Quoted 2 lines

I don't mind waiting, but please make sure that whatever lands into a release is really, truly working.

Indeed. This has been a problem with many of the TI patches in general. People are working on separate product trees and then produce patches for the mainline kernel that are poorly tested.

Quoted 13 lines · Tony Lindgren

Quoted 12 lines · Richard Cochran

Quoted 3 lines · Vaibhav Hiremath

Another important point is, this driver is also required and used for Davinci family of devices (arch/mach/mach-davinci/).

That is really beside the point. If the code isn't ready yet, then don't merge it.

When I asked about the beaglebone, I was given the impression that it will be ready for v3.7-rc1. But, as I know realize, at the current rate, it might not even be ready for v3.8.

Quoted 7 lines

Quoted 2 lines

I don't mind waiting, but please make sure that whatever lands into a release is really, truly working.

Indeed. This has been a problem with many of the TI patches in general. People are working on separate product trees and then produce patches for the mainline kernel that are poorly tested.

Tony,

It may not be true always, as we always work simultaneously and there are high chances that some patches/development is dependent on others from functionality perspective (especially baseport), but still they are independent modules (may be for other devices).

Lets take a example of AM33xx and OMAP5 here, we started submitting baseport patches to the list (almost 6-8 months now), not all the patches gets accepted together in one shot.
As you know, when we started pushing AM33xx and OMAP5 baseport patches, we were at the stage where both DT and hwmod was required. First attempt for board file submission did not went through, since we decided to force all new devices migrate to DT, right?

So the criteria initially was, build should not break and submit patches step-by-step.

Now, with baseport patches submitted to list, individual developer can also start submitting patches to the respective driver's list, making sure that, driver doesn't change irrespective of platform. I do not see anything wrong with this, as we always consider driver independent.

In this particular case, note that, all the patches Richard posted recently are AM33xx SoC integration specific patches only.

Quoted 29 lines · Hiremath, Vaibhav

Quoted 13 lines · Tony Lindgren

Quoted 12 lines · Richard Cochran

Quoted 3 lines · Vaibhav Hiremath

Another important point is, this driver is also required and used for Davinci family of devices (arch/mach/mach-davinci/).

That is really beside the point. If the code isn't ready yet, then don't merge it.

When I asked about the beaglebone, I was given the impression that it will be ready for v3.7-rc1. But, as I know realize, at the current rate, it might not even be ready for v3.8.

Quoted 7 lines

Quoted 2 lines

I don't mind waiting, but please make sure that whatever lands into a release is really, truly working.

Indeed. This has been a problem with many of the TI patches in general. People are working on separate product trees and then produce patches for the mainline kernel that are poorly tested.

Tony,

It may not be true always, as we always work simultaneously and there are high chances that some patches/development is dependent on others from functionality perspective (especially baseport), but still they are independent modules (may be for other devices).

Quoted 7 lines

Lets take a example of AM33xx and OMAP5 here, we started submitting baseport patches to the list (almost 6-8 months now), not all the patches gets accepted together in one shot.
As you know, when we started pushing AM33xx and OMAP5 baseport patches, we were at the stage where both DT and hwmod was required. First attempt for board file submission did not went through, since we decided to force all new devices migrate to DT, right?

I think you guys have done a pretty good job with the am33xx patches in general to get the core omap changes merged.

Quoted 8 lines

So the criteria initially was, build should not break and submit patches step-by-step.

Now, with baseport patches submitted to list, individual developer can also start submitting patches to the respective driver's list, making sure that, driver doesn't change irrespective of platform. I do not see anything wrong with this, as we always consider driver independent.

Quoted 2 lines

In this particular case, note that, all the patches Richard posted recently are AM33xx SoC integration specific patches only.

Yes now the core omap patches are merged, and people want to use the devices with mainline kernel. That's usually good news :)

Now the expectation has grown to the level where people expect everything in the Mainline kernel and not in some custom kernel release.

Quoted 3 lines · Vaibhav

Now the expectation has grown to the level where people expect everything in the Mainline kernel and not in some custom kernel release.

You are right, except for the "has grown" part. I have always expected the official kernel releases to be working and complete, without the kind of half baked stumps that the embedded vendors are pushing.

In my experience with Freescale (and now TI also), the vendor supports Linux on their platform by providing a heavily patched, quick and dirty, customized kernel that is kinda, sorta working, somehow. This provides a way for their customers to get started with the development kits right away, and as such I have no problem with it.

What I do have a problem with is the fact the vendors then drop the ball and don't follow through by getting their changes into mainline. This makes a huge problem for the vendor's customers later on, when the choice becomes either stay with the dead end vendor kernel, or try to get mainline working all by themselves.

And so I am pleased to hear that the Ethernet ports on the am335x will be working in v3.8. Better late than never.

Quoted 3 lines · Vaibhav Hiremath

Another important point is, this driver is also required and used for Davinci family of devices (arch/mach/mach-davinci/).

That is really beside the point. If the code isn't ready yet, then don't merge it.

When I asked about the beaglebone, I was given the impression that it will be ready for v3.7-rc1. But, as I know realize, at the current rate, it might not even be ready for v3.8.

I don't mind waiting, but please make sure that whatever lands into a release is really, truly working.

Thanks,
Richard

I understand, and as you mentioned we are not fully there at v3.7-rc1 with all the drivers/module support, due to all known reasons. Its good that with v3.7-rc2, Beaglebone boots up out of box from mainline.

Quoted 4 lines · Vaibhav

I understand, and as you mentioned we are not fully there at v3.7-rc1 with all the drivers/module support, due to all known reasons. Its good that with v3.7-rc2, Beaglebone boots up out of box from mainline.

Can you say whether we have working Ethernet on the beaglebone in v3.7?

Thanks,
Richard

Quoted 8 lines · Paul Walmsley

Quoted 7 lines

Hi Richard

Quoted 3 lines · Richard Cochran

Would you please take this bugfix for 3.7-rc2? The suggestion to mail you came from Toni Lindgren. The context where it came from is here:

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

http://lists.arm.linux.org.uk/lurker/message/20121015.191630.bdae3c50.en.html

This patch appears to add a new feature, correct? I don't think the CPSW could have worked in the past without this data present. So it looks to me like this is 3.8 material, unless Tony would like it to go in sooner?

Yeah unless it fixes something, we should just queue it for v3.8 merge window.

So there has been this cpsw driver since v3.4-rc1~177^2~5

df82859 netdev: driver: ethernet: Add TI CPSW driver

and four people signed off on it, so it must have been working at one point. Since the device tree make-over, the driver is a derelict, and thus the present patch is fixing a regression.

Paul,

Would you please take this bugfix for 3.7-rc2? The suggestion to mail you came from Toni Lindgren. The context where it came from is here:

http://lists.arm.linux.org.uk/lurker/message/20121015.191630.bdae3c50.en.html

Thanks,
Richard

----- Forwarded message from Richard Cochran <[email protected]> -----

Date: Mon, 15 Oct 2012 21:16:32 +0200
From: Richard Cochran <[email protected]>
To: netdev at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org, Arnd Bergmann <[email protected]>,
David Miller <[email protected]>,
Russell King <[email protected]>,
Mugunthan V N <[email protected]>
Subject: [PATCH 2/5] ARM: OMAP3+: hwmod: Add AM33XX HWMOD data for davinci_mdio
X-Mailer: git-send-email 1.7.2.5

From: Mugunthan V N <[email protected]>

This patch adds minimal hwmod support for davinci mdio driver. This patch requires rework on parent child relation between cpsw and davinci mdio hwmod data to support runtime PM.

Signed-off-by Mugunthan V N <[email protected]>

---
 arch/arm/mach-omap2/omap_hwmod_33xx_data.c |   34 ++++++++++++++++++++++++++-
 1 files changed, 32 insertions(+), 2 deletions(-)

arch/arm/mach-omap2/omap_hwmod_33xx_data.c +1 -2

index 59d5c1c..f96bbc0 100644
@@ -650,8 +650,7 @@ static struct omap_hwmod_class_sysconfig am33xx_cpgmac_sysc = {
 	.rev_offs	= 0x0,
 	.sysc_offs	= 0x8,
 	.syss_offs	= 0x4,
-	.sysc_flags	= (SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE |
-			   SYSS_HAS_RESET_STATUS),
+	.sysc_flags	= (SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE),
 	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | MSTANDBY_FORCE |
 			   MSTANDBY_NO),
 	.sysc_fields	= &omap_hwmod_sysc_type3,

arch/arm/mach-omap2/omap_hwmod_33xx_data.c +29 -0

 };
 
 /*

@@ -2510,6 +2511,34 @@ static struct omap_hwmod_addr_space am33xx_elm_addr_space[] = {
 	{ }
 };
 
+/* mdio class */
+static struct omap_hwmod_class am33xx_mdio_hwmod_class = {
+	.name		= "davinci_mdio",
+};
+
+struct omap_hwmod_addr_space am33xx_mdio_addr_space[] = {
+	{
+		.pa_start	= 0x4A101000,
+		.pa_end		= 0x4A101000 + SZ_256 - 1,
+		.flags		= ADDR_MAP_ON_INIT,
+	},
+	{ }
+};
+
+static struct omap_hwmod am33xx_mdio_hwmod = {
+	.name		= "davinci_mdio",
+	.class		= &am33xx_mdio_hwmod_class,
+	.clkdm_name	= "cpsw_125mhz_clkdm",
+	.main_clk	= "cpsw_125mhz_gclk",
+};
+
+struct omap_hwmod_ocp_if am33xx_cpgmac0__mdio = {
+	.master		= &am33xx_cpgmac0_hwmod,
+	.slave		= &am33xx_mdio_hwmod,
+	.addr		= am33xx_mdio_addr_space,
+	.user		= OCP_USER_MPU,
+};
+
 static struct omap_hwmod_ocp_if am33xx_l4_ls__elm = {
 	.master		= &am33xx_l4_ls_hwmod,
 	.slave		= &am33xx_elm_hwmod,

@@ -3371,6 +3400,7 @@ static struct omap_hwmod_ocp_if *am33xx_hwmod_ocp_ifs[] __initdata = {
 	&am33xx_l3_main__tptc2,
 	&am33xx_l3_s__usbss,
 	&am33xx_l4_hs__cpgmac0,
+	&am33xx_cpgmac0__mdio,
 	NULL,
 };
 

2. There is HW bug as far as idle (in turn ocp reset) is concerned, and needs special handling for this. I already have patch for this, which I will submit it to the list.

http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;h=a28da2ac98f173f97dbbb46be1ce0a3879f21a11

Couple of reasons,

1. These flags were added for validation, so I expect cpsw driver should work without these flags set. I tested it mounting NFS as rootFS and it works for me.

2. There is HW bug as far as idle (in turn ocp reset) is concerned, and needs special handling for this. I already have patch for this, which I will submit it to the list.

http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;h=a28da2ac98f173f97dbbb46be1ce0a3879f21a11

Note: Sorry for long commit description, it was screwed up by someone while git push.

I have pushed the tested branch to (linux-omap/master + CPSW patches) -
https://github.com/hvaibhav/am335x-linux/tree/am335x-upstream-staging

Hi Vaibhav

Quoted 17 lines · Vaibhav Hiremath

Couple of reasons,

1. These flags were added for validation, so I expect cpsw driver should work without these flags set. I tested it mounting NFS as rootFS and it works for me.

2. There is HW bug as far as idle (in turn ocp reset) is concerned, and needs special handling for this. I already have patch for this, which I will submit it to the list.

http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;h=a28da2ac98f173f97dbbb46be1ce0a3879f21a11

Note: Sorry for long commit description, it was screwed up by someone while git push.

Quoted 2 lines

I have pushed the tested branch to (linux-omap/master + CPSW patches) -
https://github.com/hvaibhav/am335x-linux/tree/am335x-upstream-staging

Just checking on this one. I don't mean to rush you, but if you have some patches you'd like to see integrated for 3.8 here, it would be good to get them posted before the end of the week.