Message ID | CAGzjT4f2agA30TGb8XFSRu57=--qyBt=bU5QWVTmU6LAGmwM8A@mail.gmail.com |
---|---|
State | New |
Headers | show |
Hello Jaccon, On Sun, Sep 11, 2011 at 07:34:25PM +0200, Jaccon Bastiaansen wrote: > >> > hmm, better switch the complete driver to be a platform driver and > >> > instead of the legacy probing let it create the corresponding device. > >> > > >> > >> What exactly do you mean with "switch the complete driver to be a > >> platform driver"? That I should remove all the legacy from the driver > >> (which would break the driver for users who don't use it as a platform > >> driver) or that I should add a new probe() function free of legacy > >> (which would duplicate code of the existing cs89x0_probe1() function)? > > Of course you should not break it for legacy users. Just instead of the > > legacy stuff just add a platform device with the respective values such > > that the platform driver is used in all cases. > > > > For extra points move the adding of the device to platform code. > > > > The platform device you mention is added by another patch that I also > sent on the the 7th of September (not directly to you, but to Sascha > and kernel@pengutronix.de). This patch was called: > > [PATCH] Add CS890x0 ethernet controller support to the i.MX21ADS > > In this patch I add a platform device to the i.MX21ADS evaluation > board. All the required settings (memory address and IRQ) that the > CS890x platform driver needs to run on the i.MX21ADS board are set in > the i.MX21ADS specific code. You find the patch below. It this the > approach you mean? not really. On some machines (e.g arm/imx/mx31ads) some magic constants are defined somewhere in arch/ to fill in netcard_portlist and cs8900_irq_map. On these machines the cs8900 driver does drive some hardware, but not via the platform bus. This is what i want to have changed. Best regards Uwe
Hello Uwe, 2011/9/11 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: > Hello Jaccon, > > On Sun, Sep 11, 2011 at 07:34:25PM +0200, Jaccon Bastiaansen wrote: >> >> > hmm, better switch the complete driver to be a platform driver and >> >> > instead of the legacy probing let it create the corresponding device. >> >> > >> >> >> >> What exactly do you mean with "switch the complete driver to be a >> >> platform driver"? That I should remove all the legacy from the driver >> >> (which would break the driver for users who don't use it as a platform >> >> driver) or that I should add a new probe() function free of legacy >> >> (which would duplicate code of the existing cs89x0_probe1() function)? >> > Of course you should not break it for legacy users. Just instead of the >> > legacy stuff just add a platform device with the respective values such >> > that the platform driver is used in all cases. >> > >> > For extra points move the adding of the device to platform code. >> > >> >> The platform device you mention is added by another patch that I also >> sent on the the 7th of September (not directly to you, but to Sascha >> and kernel@pengutronix.de). This patch was called: >> >> [PATCH] Add CS890x0 ethernet controller support to the i.MX21ADS >> >> In this patch I add a platform device to the i.MX21ADS evaluation >> board. All the required settings (memory address and IRQ) that the >> CS890x platform driver needs to run on the i.MX21ADS board are set in >> the i.MX21ADS specific code. You find the patch below. It this the >> approach you mean? > not really. On some machines (e.g arm/imx/mx31ads) some magic constants > are defined somewhere in arch/ to fill in netcard_portlist and > cs8900_irq_map. On these machines the cs8900 driver does drive some > hardware, but not via the platform bus. This is what i want to have > changed. > Ok, so you would like to see the #ifdefs for the IXDP2351, IXDP2X01, QQ2440 and MX31ADS in the cs890x driver replaced by platform_device definitions in the platform specific code of those platforms (in the same way as I have done for the i.MX21ADS platform)? Is this correct? > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Regards, Jaccon
Hello Jaccon, On Mon, Sep 12, 2011 at 12:52:52PM +0200, Jaccon Bastiaansen wrote: > 2011/9/11 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: > > On some machines (e.g arm/imx/mx31ads) some magic constants > > are defined somewhere in arch/ to fill in netcard_portlist and > > cs8900_irq_map. On these machines the cs8900 driver does drive some > > hardware, but not via the platform bus. This is what i want to have > > changed. > > > > Ok, so you would like to see the #ifdefs for the IXDP2351, IXDP2X01, > QQ2440 and MX31ADS in the cs890x driver replaced by platform_device > definitions in the platform specific code of those platforms (in the > same way as I have done for the i.MX21ADS platform)? Is this correct? More or less, yes. I would have started to keep the change local to the cs8900 driver, but moving the devices to arch is the final goal. The upside of first reworking the driver and only then move the devices out is that it might be better/easier to bisect and review. Best regards Uwe
Hello Domenico, 2011/9/28 Domenico Andreoli <cavokz@gmail.com>: > Hi Jaccon, > > On Mon, Sep 12, 2011 at 12:52:52PM +0200, Jaccon Bastiaansen wrote: >> >> Ok, so you would like to see the #ifdefs for the IXDP2351, IXDP2X01, >> QQ2440 and MX31ADS in the cs890x driver replaced by platform_device >> definitions in the platform specific code of those platforms (in the >> same way as I have done for the i.MX21ADS platform)? Is this correct? > > Feel free to completely dump the QQ2440 thing, the QQ2440 board support > is not in mainline and will not until it can be implemented mostly with > DT, I guess. At that time I will be happy to use your platform device > conversion - with the OF initialization that sombody will write ;) > > I will test it. > > cheers, > Domenico > This patch does what you describe and removes the QQ2440 board support. Regards, Jaccon
On Fri, Sep 30, 2011 at 11:01:10AM +0200, Jaccon Bastiaansen wrote: > Hello Domenico, > > 2011/9/28 Domenico Andreoli <cavokz@gmail.com>: > > Hi Jaccon, > > > > On Mon, Sep 12, 2011 at 12:52:52PM +0200, Jaccon Bastiaansen wrote: > >> > >> Ok, so you would like to see the #ifdefs for the IXDP2351, IXDP2X01, > >> QQ2440 and MX31ADS in the cs890x driver replaced by platform_device > >> definitions in the platform specific code of those platforms (in the > >> same way as I have done for the i.MX21ADS platform)? Is this correct? > > > > Feel free to completely dump the QQ2440 thing, the QQ2440 board support > > is not in mainline and will not until it can be implemented mostly with > > DT, I guess. At that time I will be happy to use your platform device > > conversion - with the OF initialization that sombody will write ;) I don't have Domenico's original mail... The QQ2440 thing got submitted into the patch system while the consolidation push was going on earlier in the year, so I ignored it (along with the other platforms which people submitted without regard to Linus' rant.) It's now up to the SoC maintainers whether they want to accept this stuff, and if so how to get it into mainline (probably via Arnd) - be that in their current form or if they think that a DT based implementation is better. I'll be discarding the four 'new' platforms (of which QQ2440 is one) which have been sitting in the patch system since March in the next few days.
On Sat, Oct 01, 2011 at 06:13:40PM +0100, Russell King - ARM Linux wrote: > On Fri, Sep 30, 2011 at 11:01:10AM +0200, Jaccon Bastiaansen wrote: > > Hello Domenico, > > > > 2011/9/28 Domenico Andreoli <cavokz@gmail.com>: > > > Hi Jaccon, > > > > > > On Mon, Sep 12, 2011 at 12:52:52PM +0200, Jaccon Bastiaansen wrote: > > >> > > >> Ok, so you would like to see the #ifdefs for the IXDP2351, IXDP2X01, > > >> QQ2440 and MX31ADS in the cs890x driver replaced by platform_device > > >> definitions in the platform specific code of those platforms (in the > > >> same way as I have done for the i.MX21ADS platform)? Is this correct? > > > > > > Feel free to completely dump the QQ2440 thing, the QQ2440 board support > > > is not in mainline and will not until it can be implemented mostly with > > > DT, I guess. At that time I will be happy to use your platform device > > > conversion - with the OF initialization that sombody will write ;) > > I don't have Domenico's original mail... yes, there was a problem on my side that I hope is now fixed. > It's now up to the SoC maintainers whether they want to accept this stuff, > and if so how to get it into mainline (probably via Arnd) - be that in > their current form or if they think that a DT based implementation is > better. > > I'll be discarding the four 'new' platforms (of which QQ2440 is one) which > have been sitting in the patch system since March in the next few days. thank you. Regards, Domenico
=================================== [PATCH] Add CS890x0 ethernet controller support to the i.MX21ADS Signed-off-by: Jaccon Bastiaansen <jaccon.bastiaansen@gmail.com> --- arch/arm/mach-imx/mach-mx21ads.c | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-imx/mach-mx21ads.c b/arch/arm/mach-imx/mach-mx21ads.c index 74ac889..62b4717 100644 --- a/arch/arm/mach-imx/mach-mx21ads.c +++ b/arch/arm/mach-imx/mach-mx21ads.c @@ -159,6 +159,26 @@ static struct platform_device mx21ads_nor_mtd_device = { .resource = &mx21ads_flash_resource, }; +static struct resource mx21ads_cs8900_resources[] = { + { + .start = (u32)MX21ADS_CS8900A_IOBASE_REG, + .end = (u32)MX21ADS_CS8900A_IOBASE_REG + 0x200000 - 1, + .flags = IORESOURCE_MEM, + }, + { + .start = MX21ADS_CS8900A_IRQ, + .end = MX21ADS_CS8900A_IRQ, + .flags = IORESOURCE_IRQ, + }, +}; + +static struct platform_device mx21ads_cs8900_device = { + .name = "cs89x0", + .id = 0, + .num_resources = ARRAY_SIZE(mx21ads_cs8900_resources), + .resource = mx21ads_cs8900_resources, +}; + static const struct imxuart_platform_data uart_pdata_rts __initconst = { .flags = IMXUART_HAVE_RTSCTS, }; @@ -275,6 +295,7 @@ static void __init mx21ads_map_io(void) static struct platform_device *platform_devices[] __initdata = { &mx21ads_nor_mtd_device, + &mx21ads_cs8900_device }; static void __init mx21ads_board_init(void)