Gianfar ethernet device

Gianfar ethernet device

[Jon Smirl]

Which platforms have the Gianfar ethernet device? The gfar code in
fsl_soc.c is getting built on the mpc5200 and it doesn't have the
device.

-- 
Jon Smirl
jonsmirl@gmail.com

Re: Gianfar ethernet device

[Jon Smirl]

Does this patch add the right ifdefs in fsl_soc.c to make these
drivers build on their proper platforms? As an experiment I have
disabled platform bus on the mpc5200 and I'm only using of_platform
bus. Turning off platform bus is exposing a lot of code that is
getting built into my mpc5200 kernel that is supporting devices not
available on the platform.

A few more fixups in some other drivers and I should have mpc5200
working without platform bus.

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index d6ef264..7017510 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -130,6 +130,7 @@ u32 get_baudrate(void)
 EXPORT_SYMBOL(get_baudrate);
 #endif /* CONFIG_CPM2 */

+#if defined(CONFIG_PPC_83xx) || defined(CONFIG_PPC_85xx) ||
defined(CONFIG_PPC_86xx)
 static int __init gfar_mdio_of_init(void)
 {
 	struct device_node *np;
@@ -317,6 +318,7 @@ err:
 }

 arch_initcall(gfar_of_init);
+#endif

 #ifdef CONFIG_PPC_83xx
 static int __init mpc83xx_wdt_init(void)
@@ -379,7 +381,7 @@ nodev:
 }

 arch_initcall(mpc83xx_wdt_init);
-#endif
+

 static enum fsl_usb2_phy_modes determine_usb_phy(const char *phy_type)
 {
@@ -542,6 +544,7 @@ err:
 }

 arch_initcall(fsl_usb_of_init);
+#endif

 #ifndef CONFIG_PPC_CPM_NEW_BINDING
 #ifdef CONFIG_CPM2
@@ -1085,6 +1088,7 @@ arch_initcall(cpm_smc_uart_of_init);
 #endif /* CONFIG_8xx */
 #endif /* CONFIG_PPC_CPM_NEW_BINDING */

+#if defined(CONFIG_PPC_83xx)
 int __init fsl_spi_init(struct spi_board_info *board_infos,
 			unsigned int num_board_infos,
 			void (*activate_cs)(u8 cs, u8 polarity),
@@ -1174,6 +1178,7 @@ err:

 	return spi_register_board_info(board_infos, num_board_infos);
 }
+#endif

 #if defined(CONFIG_PPC_85xx) || defined(CONFIG_PPC_86xx)
 static __be32 __iomem *rstcr;

-- 
Jon Smirl
jonsmirl@gmail.com

Re: Gianfar ethernet device

[Benjamin Herrenschmidt]

On Sun, 2007-11-11 at 18:30 -0500, Jon Smirl wrote:
> Does this patch add the right ifdefs in fsl_soc.c to make these
> drivers build on their proper platforms? As an experiment I have
> disabled platform bus on the mpc5200 and I'm only using of_platform
> bus. Turning off platform bus is exposing a lot of code that is
> getting built into my mpc5200 kernel that is supporting devices not
> available on the platform.
> 
> A few more fixups in some other drivers and I should have mpc5200
> working without platform bus.

That sort of per-platform ifdefs is just WRONG.

On the other hand, if you feel that such common code deserves not being
built on all platforms, then what you can do is define a Kconfig option,
such as CONFIG_PPC_CAN_HAVE_GIANFAR, that gets select'ed by the
platforms that can have a Gianfar and use -that- for the ifdef.

Ben.

Re: Gianfar ethernet device

[Jon Smirl]

On 11/11/07, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> On Sun, 2007-11-11 at 18:30 -0500, Jon Smirl wrote:
> > Does this patch add the right ifdefs in fsl_soc.c to make these
> > drivers build on their proper platforms? As an experiment I have
> > disabled platform bus on the mpc5200 and I'm only using of_platform
> > bus. Turning off platform bus is exposing a lot of code that is
> > getting built into my mpc5200 kernel that is supporting devices not
> > available on the platform.
> >
> > A few more fixups in some other drivers and I should have mpc5200
> > working without platform bus.
>
> That sort of per-platform ifdefs is just WRONG.
>
> On the other hand, if you feel that such common code deserves not being
> built on all platforms, then what you can do is define a Kconfig option,
> such as CONFIG_PPC_CAN_HAVE_GIANFAR, that gets select'ed by the
> platforms that can have a Gianfar and use -that- for the ifdef.

The real solution is that gianfar support belongs in a device driver,
not in a common file. That whole fsl_soc.c file is a catch-all of
things that belong in device drivers. I haven't looked at every line
in it, but 90%+ of the code should be moved into device drivers.

I'm preparing a patch that moves the i2c driver out of fsl_soc.c and
into i2c_mpc.c.


-- 
Jon Smirl
jonsmirl@gmail.com

Re: Gianfar ethernet device

[Jon Smirl]

BTW, this exercise of disabling platform bus is turning up some real bugs.

There are several tests in the kernel like this:

        if (!shost->shost_gendev.parent)
                shost->shost_gendev.parent = dev ? dev : &platform_bus;

	if (adap->dev.parent == NULL) {
		adap->dev.parent = &platform_bus;
		pr_debug("I2C adapter driver [%s] forgot to specify "
			 "physical device\n", adap->name);
	}

This doesn't do the right thing when the device is on of_platform_bus
instead of platform_bus. What's the right fix for these? Shouldn't the
call error out instead of trying to fix up the parent? Then fix the
code to set the parent right when the device was created.

-- 
Jon Smirl
jonsmirl@gmail.com

Re: Gianfar ethernet device

[Benjamin Herrenschmidt]

> The real solution is that gianfar support belongs in a device driver,
> not in a common file. That whole fsl_soc.c file is a catch-all of
> things that belong in device drivers. I haven't looked at every line
> in it, but 90%+ of the code should be moved into device drivers.
> 
> I'm preparing a patch that moves the i2c driver out of fsl_soc.c and
> into i2c_mpc.c.

The problem is how do you instanciate it. I'm working on some solution
for that but it's not there yet.

Ben.

Re: Gianfar ethernet device

[Jon Smirl]

On 11/11/07, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> > The real solution is that gianfar support belongs in a device driver,
> > not in a common file. That whole fsl_soc.c file is a catch-all of
> > things that belong in device drivers. I haven't looked at every line
> > in it, but 90%+ of the code should be moved into device drivers.
> >
> > I'm preparing a patch that moves the i2c driver out of fsl_soc.c and
> > into i2c_mpc.c.
>
> The problem is how do you instantiate it. I'm working on some solution
> for that but it's not there yet.

Are there powerpc platforms without device trees?

Standard of_platform driver works fine to instantiate the i2c driver
on mpc5200.

static struct of_device_id mpc_i2c_of_match[] = {
	{
		.compatible	= "fsl-i2c",
	},
};
MODULE_DEVICE_TABLE(of, mpc_i2c_of_match);

/* Structure for a device driver */
static struct of_platform_driver mpc_i2c_driver = {
	.match_table	= mpc_i2c_of_match,
	.probe		= mpc_i2c_probe,
	.remove		= __devexit_p(mpc_i2c_remove),
	.driver		= {
		.owner	= THIS_MODULE,
		.name	= DRV_NAME,
	},
};

static int __init mpc_i2c_init(void)
{
	int rv;

	rv = of_register_platform_driver(&mpc_i2c_driver);
	if (rv) {
		printk(KERN_ERR DRV_NAME " of_register_platform_driver failed (%i)\n", rv);
		return rv;
	}
	return 0;
}

module_init(mpc_i2c_init);

----------------------------------------------------------------------------------------------

i2c and alsa soc core instantiate like this, asoc v2 is not in tree
yet. These cores used to create platform drivers, but that was wrong,
they don't have any hardware associated with them.

static int __init i2c_init(void)
{
	int retval;

	retval = bus_register(&i2c_bus_type);
	if (retval)
		return retval;
	return class_register(&i2c_adapter_class);
}

subsys_initcall(i2c_init);

-- 
Jon Smirl
jonsmirl@gmail.com

Re: Gianfar ethernet device

[Benjamin Herrenschmidt]

On Sun, 2007-11-11 at 20:16 -0500, Jon Smirl wrote:
> 
> Are there powerpc platforms without device trees?

Not in arch/powerpc.

Ben.

Re: Gianfar ethernet device

[Jon Smirl]

On 11/11/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> On 11/11/07, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> >
> > > The real solution is that gianfar support belongs in a device driver,
> > > not in a common file. That whole fsl_soc.c file is a catch-all of
> > > things that belong in device drivers. I haven't looked at every line
> > > in it, but 90%+ of the code should be moved into device drivers.
> > >
> > > I'm preparing a patch that moves the i2c driver out of fsl_soc.c and
> > > into i2c_mpc.c.
> >
> > The problem is how do you instantiate it. I'm working on some solution
> > for that but it's not there yet.

What's an example of a device that can't be instantiated?

>
> Are there powerpc platforms without device trees?
>
> Standard of_platform driver works fine to instantiate the i2c driver
> on mpc5200.
>
> static struct of_device_id mpc_i2c_of_match[] = {
>         {
>                 .compatible     = "fsl-i2c",
>         },
> };
> MODULE_DEVICE_TABLE(of, mpc_i2c_of_match);
>
> /* Structure for a device driver */
> static struct of_platform_driver mpc_i2c_driver = {
>         .match_table    = mpc_i2c_of_match,
>         .probe          = mpc_i2c_probe,
>         .remove         = __devexit_p(mpc_i2c_remove),
>         .driver         = {
>                 .owner  = THIS_MODULE,
>                 .name   = DRV_NAME,
>         },
> };
>
> static int __init mpc_i2c_init(void)
> {
>         int rv;
>
>         rv = of_register_platform_driver(&mpc_i2c_driver);
>         if (rv) {
>                 printk(KERN_ERR DRV_NAME " of_register_platform_driver failed (%i)\n", rv);
>                 return rv;
>         }
>         return 0;
> }
>
> module_init(mpc_i2c_init);
>
> ----------------------------------------------------------------------------------------------
>
> i2c and alsa soc core instantiate like this, asoc v2 is not in tree
> yet. These cores used to create platform drivers, but that was wrong,
> they don't have any hardware associated with them.
>
> static int __init i2c_init(void)
> {
>         int retval;
>
>         retval = bus_register(&i2c_bus_type);
>         if (retval)
>                 return retval;
>         return class_register(&i2c_adapter_class);
> }
>
> subsys_initcall(i2c_init);
>
> --
> Jon Smirl
> jonsmirl@gmail.com
>


-- 
Jon Smirl
jonsmirl@gmail.com