diff mbox

[RFC,1/7] platform: add a device node

Message ID 1360442671-15216-2-git-send-email-javier.martinez@collabora.co.uk
State New
Headers show

Commit Message

Javier Martinez Canillas Feb. 9, 2013, 8:44 p.m. UTC
When using Device Trees, it is necessary to associate a
device node with a platform device.

Usually this device node has to used in the device probe
function (e.g: to initizalize the pinctrl pads assocaited
with the device).

So, platform code needs to pass a device node as a platform
device info to the platform device registration function.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/mach-imx/devices/platform-gpio-mxc.c |    2 +-
 arch/arm/mach-imx/devices/platform-imx-dma.c  |    4 ++--
 arch/arm/mach-imx/mach-armadillo5x0.c         |    2 +-
 arch/arm/mach-imx/mach-imx27_visstrim_m10.c   |    3 ++-
 arch/arm/mach-imx/mach-mx1ads.c               |    2 +-
 arch/arm/mach-nomadik/cpu-8815.c              |    2 +-
 arch/arm/mach-omap2/fb.c                      |    2 +-
 arch/arm/mach-omap2/gpmc-smsc911x.c           |    2 +-
 arch/arm/mach-ux500/board-mop500-audio.c      |    2 +-
 arch/arm/mach-ux500/devices-common.c          |    3 ++-
 arch/arm/mach-ux500/devices-db8500.h          |    2 +-
 arch/unicore32/kernel/puv3-core.c             |    2 +-
 arch/unicore32/kernel/puv3-nb0916.c           |    2 +-
 drivers/base/platform.c                       |    1 +
 drivers/leds/leds-gpio-register.c             |    2 +-
 drivers/virtio/virtio_mmio.c                  |    2 +-
 include/linux/platform_device.h               |    9 ++++++---
 sound/soc/samsung/i2s.c                       |    2 +-
 18 files changed, 26 insertions(+), 20 deletions(-)

Comments

Greg Kroah-Hartman Feb. 10, 2013, 1:02 a.m. UTC | #1
On Sat, Feb 09, 2013 at 09:44:25PM +0100, Javier Martinez Canillas wrote:
> When using Device Trees, it is necessary to associate a
> device node with a platform device.
> 
> Usually this device node has to used in the device probe
> function (e.g: to initizalize the pinctrl pads assocaited
> with the device).
> 
> So, platform code needs to pass a device node as a platform
> device info to the platform device registration function.

Really?  We don't already have enough other pointers in the platform
device structure that we need another one?

I don't buy it, sorry.  Any reason you didn't run this by Grant as well?

greg k-h
Javier Martinez Canillas Feb. 10, 2013, 1:49 a.m. UTC | #2
Hi Greg,

Thanks a lot for your feedback.

On 02/10/2013 02:02 AM, Greg Kroah-Hartman wrote:
> On Sat, Feb 09, 2013 at 09:44:25PM +0100, Javier Martinez Canillas wrote:
>> When using Device Trees, it is necessary to associate a
>> device node with a platform device.
>> 
>> Usually this device node has to used in the device probe
>> function (e.g: to initizalize the pinctrl pads assocaited
>> with the device).
>> 
>> So, platform code needs to pass a device node as a platform
>> device info to the platform device registration function.
> 
> Really?  We don't already have enough other pointers in the platform
> device structure that we need another one?
>

I knew this would be controversial and that's why I didn't mean it to be a patch
but a RFC :)

The problem basically is that you have to associate the platform device with its
corresponding DT device node because it can be used in the driver probe function.

But you can't do this before calling platform_device_register_resndata() since
the platform_device has not been allocated yet and you can't do it after since
by then the driver probe function has been executed already.

You could certainly pass the device node as a platform device resource by for
example storing the dev node pointer in the struct resource .start member or by
adding it to the structure (struct smsc911x_platform_config in this case) that
gets copied to the struct platform_device_info void *data.

Then in the driver probe function you could get the device node from either the
platform_data or a dev->resource.

But I expect most users of platform_device_register_resndata() will have a
similar issue as more and more platform drivers are migrated to DT.

So, to avoid each driver to do the same (encode and decode the DT device node
using platform_data or a device resource) I thought that it could make sense to
add yet another pointer to the struct platform_device_info structure and set:

pdev->dev.of_node = pdevinfo->of_node

inside platform_device_register_full().

> I don't buy it, sorry.  Any reason you didn't run this by Grant as well?
> 

No reason at all, it is just me being goofy. I thought that Grant was cc'ed
already and I just realized that he wasn't. cc'ing him as well.

> greg k-h
> 

Thanks a lot and best regards,
Javier
Russell King - ARM Linux Feb. 10, 2013, 9:37 a.m. UTC | #3
On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
> I knew this would be controversial and that's why I didn't mean it to be a patch
> but a RFC :)
> 
> The problem basically is that you have to associate the platform device with its
> corresponding DT device node because it can be used in the driver probe function.

When DT is being used, doesn't DT create the platform devices for you,
with the device node already set correctly?

Manually creating platform devices and adding DT nodes to it sounds like
the wrong thing to be doing.
Javier Martinez Canillas Feb. 10, 2013, 11:35 a.m. UTC | #4
On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>> I knew this would be controversial and that's why I didn't mean it to be a patch
>> but a RFC :)
>>
>> The problem basically is that you have to associate the platform device with its
>> corresponding DT device node because it can be used in the driver probe function.
>
> When DT is being used, doesn't DT create the platform devices for you,
> with the device node already set correctly?
>

Well they usually do but not always just like usually you have a
platform_device in your board code and call platform_device_register().

But in some configurations you can't just define the platform_device
required resources (I/O memory, IRQ, etc) as static platform data.
In some cases you need a level of indirection.

In this particular case a SMSC ethernet chip is connected to an
OMAP3 processor through its General-Purpose Memory Controller.

You can't just define the I/O memory used by the device since you first
need to request that address to the GPMC. The same happens with the
IRQ line since a OMAP GPIO pin is used so you have to first configure
the GPIO line as input.

So in platform code you call to gpmc_smsc911x_init() passing all the
GPMC specific configurations (GPIO used for IRQ, GPMC chip select, etc)

Then gpmc_smsc911x_init() does all the needed setup, fills a platform_data
for the real platform_device and calls  platform_device_register_resndata().

>From the smsc911x platform_driver probe function point of view it just have
resources and doesn't know if it's I/O memory is directly mapped or is
through a memory controller as the GPMC. It also doesn't know if the IRQ is
a GPIO or not.

Another approach could be extending the current SMSC LAN DT binding with
GPMC-specific properties so the platform driver could do this setup when the
device node is found and the probe function is called.

> Manually creating platform devices and adding DT nodes to it sounds like
> the wrong thing to be doing.
> --

Yes, I don't like this approach to much either but I didn't find a better way to
do it. That was the main point of this RFC.

I'm still learning about Device Trees so I hope that someone with more
experience with DT or more familiarized with OMAP GPMC and the SMSC
driver could take a step forward and help me giving a better idea.

Thanks a lot for your feedback and best regards,
Javier
Sascha Hauer Feb. 11, 2013, 8:16 a.m. UTC | #5
On Sun, Feb 10, 2013 at 12:35:43PM +0100, Javier Martinez Canillas wrote:
> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
> >> I knew this would be controversial and that's why I didn't mean it to be a patch
> >> but a RFC :)
> >>
> >> The problem basically is that you have to associate the platform device with its
> >> corresponding DT device node because it can be used in the driver probe function.
> >
> > When DT is being used, doesn't DT create the platform devices for you,
> > with the device node already set correctly?
> >
> 
> Well they usually do but not always just like usually you have a
> platform_device in your board code and call platform_device_register().
> 
> But in some configurations you can't just define the platform_device
> required resources (I/O memory, IRQ, etc) as static platform data.
> In some cases you need a level of indirection.
> 
> In this particular case a SMSC ethernet chip is connected to an
> OMAP3 processor through its General-Purpose Memory Controller.
> 
> You can't just define the I/O memory used by the device since you first
> need to request that address to the GPMC. The same happens with the
> IRQ line since a OMAP GPIO pin is used so you have to first configure
> the GPIO line as input.

For the gpio interrupt you can use, example taken from omap4-var-som.dts:

	interrupt-parent = <&gpio6>;
	interrupts = <11>; /* gpio line 171 */

Other architectures allow to specify the edge/level hi/low active
parameters from the devicetree aswell. The gpio direction should be
handled by the gpio driver as necessary, at least that's what done on
other architectures.

If the SMSC hangs on the GPMC then the SMSC should be a child node of
the GPMC. The GPMC would then act as a bus driver and configure the
chipselects and timings for its children automatically, maybe based
on timing information from the devicetree. I've never tried this before,
but I think that's the way things should be.

No need to manually create platform devices from the devicetree.

Sascha
Javier Martinez Canillas Feb. 11, 2013, 10:33 a.m. UTC | #6
On Mon, Feb 11, 2013 at 9:16 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Sun, Feb 10, 2013 at 12:35:43PM +0100, Javier Martinez Canillas wrote:
>> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>> >> I knew this would be controversial and that's why I didn't mean it to be a patch
>> >> but a RFC :)
>> >>
>> >> The problem basically is that you have to associate the platform device with its
>> >> corresponding DT device node because it can be used in the driver probe function.
>> >
>> > When DT is being used, doesn't DT create the platform devices for you,
>> > with the device node already set correctly?
>> >
>>
>> Well they usually do but not always just like usually you have a
>> platform_device in your board code and call platform_device_register().
>>
>> But in some configurations you can't just define the platform_device
>> required resources (I/O memory, IRQ, etc) as static platform data.
>> In some cases you need a level of indirection.
>>
>> In this particular case a SMSC ethernet chip is connected to an
>> OMAP3 processor through its General-Purpose Memory Controller.
>>
>> You can't just define the I/O memory used by the device since you first
>> need to request that address to the GPMC. The same happens with the
>> IRQ line since a OMAP GPIO pin is used so you have to first configure
>> the GPIO line as input.
>
> For the gpio interrupt you can use, example taken from omap4-var-som.dts:
>
>         interrupt-parent = <&gpio6>;
>         interrupts = <11>; /* gpio line 171 */
>
> Other architectures allow to specify the edge/level hi/low active
> parameters from the devicetree aswell. The gpio direction should be
> handled by the gpio driver as necessary, at least that's what done on
> other architectures.
>
> If the SMSC hangs on the GPMC then the SMSC should be a child node of
> the GPMC. The GPMC would then act as a bus driver and configure the
> chipselects and timings for its children automatically, maybe based
> on timing information from the devicetree. I've never tried this before,
> but I think that's the way things should be.
>

Hi Sasha,

The SMSC is already a child node of the GPMC in the device tree but instead
using the generic SMSC binding I added a GPMC-specific SMSC binding.

Since the SMSC binding doesn't have a chip select property and it expects
the I/O memory address to be explicitly defined in the reg property while
the GPMC needs to request this memory according to the chip select used.

> No need to manually create platform devices from the devicetree.
>

Thanks a lot for your feedback, I'll go back and think more about this and try
to solve this using a different approach that won't require changing
the platform
device info structure and reusing the generic SMSC LAN DT binding.

> Sascha
>

Best regards,
Javier
Sascha Hauer Feb. 11, 2013, 11:24 a.m. UTC | #7
On Mon, Feb 11, 2013 at 11:33:19AM +0100, Javier Martinez Canillas wrote:
> On Mon, Feb 11, 2013 at 9:16 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Sun, Feb 10, 2013 at 12:35:43PM +0100, Javier Martinez Canillas wrote:
> >> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
> >> <linux@arm.linux.org.uk> wrote:
> >> > On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
> >> >> I knew this would be controversial and that's why I didn't mean it to be a patch
> >> >> but a RFC :)
> >> >>
> >> >> The problem basically is that you have to associate the platform device with its
> >> >> corresponding DT device node because it can be used in the driver probe function.
> >> >
> >> > When DT is being used, doesn't DT create the platform devices for you,
> >> > with the device node already set correctly?
> >> >
> >>
> >> Well they usually do but not always just like usually you have a
> >> platform_device in your board code and call platform_device_register().
> >>
> >> But in some configurations you can't just define the platform_device
> >> required resources (I/O memory, IRQ, etc) as static platform data.
> >> In some cases you need a level of indirection.
> >>
> >> In this particular case a SMSC ethernet chip is connected to an
> >> OMAP3 processor through its General-Purpose Memory Controller.
> >>
> >> You can't just define the I/O memory used by the device since you first
> >> need to request that address to the GPMC. The same happens with the
> >> IRQ line since a OMAP GPIO pin is used so you have to first configure
> >> the GPIO line as input.
> >
> > For the gpio interrupt you can use, example taken from omap4-var-som.dts:
> >
> >         interrupt-parent = <&gpio6>;
> >         interrupts = <11>; /* gpio line 171 */
> >
> > Other architectures allow to specify the edge/level hi/low active
> > parameters from the devicetree aswell. The gpio direction should be
> > handled by the gpio driver as necessary, at least that's what done on
> > other architectures.
> >
> > If the SMSC hangs on the GPMC then the SMSC should be a child node of
> > the GPMC. The GPMC would then act as a bus driver and configure the
> > chipselects and timings for its children automatically, maybe based
> > on timing information from the devicetree. I've never tried this before,
> > but I think that's the way things should be.
> >
> 
> Hi Sasha,
> 
> The SMSC is already a child node of the GPMC in the device tree but instead
> using the generic SMSC binding I added a GPMC-specific SMSC binding.
> 
> Since the SMSC binding doesn't have a chip select property and it expects
> the I/O memory address to be explicitly defined in the reg property while
> the GPMC needs to request this memory according to the chip select used.

So you probably have this:

	gpmc {
		compatible = "ti,gpmc", "simple-bus";
		ranges;

		smsc911x {
			compatible = "smsc,91x";
		}
	}

If you remove the simple-bus property the gpmc devices would not be
probed. If then you add a driver which matches "ti,gpmc" you can
configure the chip selects in its probe callback. After this you
can call of_platform_populate() starting from the gpmc device node
to instantiate the gpmc child devices.

Please somebody interrupt me if I'm talking total rubbish here. I never
tried this and only assume it should work like this.

Sascha
Javier Martinez Canillas Feb. 11, 2013, 11:38 a.m. UTC | #8
On Mon, Feb 11, 2013 at 12:24 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Mon, Feb 11, 2013 at 11:33:19AM +0100, Javier Martinez Canillas wrote:
>> On Mon, Feb 11, 2013 at 9:16 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> > On Sun, Feb 10, 2013 at 12:35:43PM +0100, Javier Martinez Canillas wrote:
>> >> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
>> >> <linux@arm.linux.org.uk> wrote:
>> >> > On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>> >> >> I knew this would be controversial and that's why I didn't mean it to be a patch
>> >> >> but a RFC :)
>> >> >>
>> >> >> The problem basically is that you have to associate the platform device with its
>> >> >> corresponding DT device node because it can be used in the driver probe function.
>> >> >
>> >> > When DT is being used, doesn't DT create the platform devices for you,
>> >> > with the device node already set correctly?
>> >> >
>> >>
>> >> Well they usually do but not always just like usually you have a
>> >> platform_device in your board code and call platform_device_register().
>> >>
>> >> But in some configurations you can't just define the platform_device
>> >> required resources (I/O memory, IRQ, etc) as static platform data.
>> >> In some cases you need a level of indirection.
>> >>
>> >> In this particular case a SMSC ethernet chip is connected to an
>> >> OMAP3 processor through its General-Purpose Memory Controller.
>> >>
>> >> You can't just define the I/O memory used by the device since you first
>> >> need to request that address to the GPMC. The same happens with the
>> >> IRQ line since a OMAP GPIO pin is used so you have to first configure
>> >> the GPIO line as input.
>> >
>> > For the gpio interrupt you can use, example taken from omap4-var-som.dts:
>> >
>> >         interrupt-parent = <&gpio6>;
>> >         interrupts = <11>; /* gpio line 171 */
>> >
>> > Other architectures allow to specify the edge/level hi/low active
>> > parameters from the devicetree aswell. The gpio direction should be
>> > handled by the gpio driver as necessary, at least that's what done on
>> > other architectures.
>> >
>> > If the SMSC hangs on the GPMC then the SMSC should be a child node of
>> > the GPMC. The GPMC would then act as a bus driver and configure the
>> > chipselects and timings for its children automatically, maybe based
>> > on timing information from the devicetree. I've never tried this before,
>> > but I think that's the way things should be.
>> >
>>
>> Hi Sasha,
>>
>> The SMSC is already a child node of the GPMC in the device tree but instead
>> using the generic SMSC binding I added a GPMC-specific SMSC binding.
>>
>> Since the SMSC binding doesn't have a chip select property and it expects
>> the I/O memory address to be explicitly defined in the reg property while
>> the GPMC needs to request this memory according to the chip select used.
>
> So you probably have this:
>
>         gpmc {
>                 compatible = "ti,gpmc", "simple-bus";
>                 ranges;
>
>                 smsc911x {
>                         compatible = "smsc,91x";
>                 }
>         }
>

Yes

> If you remove the simple-bus property the gpmc devices would not be
> probed. If then you add a driver which matches "ti,gpmc" you can
> configure the chip selects in its probe callback. After this you
> can call of_platform_populate() starting from the gpmc device node
> to instantiate the gpmc child devices.
>
> Please somebody interrupt me if I'm talking total rubbish here. I never
> tried this and only assume it should work like this.
>

Nice, I haven't thought about that and I don't see why it shouldn't work.

I won't have time to work on this for at least the next 3 weeks but I'll try
once I have more free time and post another RFC if I got things working.

> Sascha
>

Thanks a lot and best regards,
Javier
Grant Likely Feb. 18, 2013, 1:33 p.m. UTC | #9
On Sun, Feb 10, 2013 at 9:37 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>> I knew this would be controversial and that's why I didn't mean it to be a patch
>> but a RFC :)
>>
>> The problem basically is that you have to associate the platform device with its
>> corresponding DT device node because it can be used in the driver probe function.
>
> When DT is being used, doesn't DT create the platform devices for you,
> with the device node already set correctly?
>
> Manually creating platform devices and adding DT nodes to it sounds like
> the wrong thing to be doing.

Right; I'd expect your platform device creation to go through the
of_platform_populate() route. What is your use-case?

g.
Grant Likely Feb. 18, 2013, 1:51 p.m. UTC | #10
On Sun, Feb 10, 2013 at 11:35 AM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>>> I knew this would be controversial and that's why I didn't mean it to be a patch
>>> but a RFC :)
>>>
>>> The problem basically is that you have to associate the platform device with its
>>> corresponding DT device node because it can be used in the driver probe function.
>>
>> When DT is being used, doesn't DT create the platform devices for you,
>> with the device node already set correctly?
>>
>
> Well they usually do but not always just like usually you have a
> platform_device in your board code and call platform_device_register().
>
> But in some configurations you can't just define the platform_device
> required resources (I/O memory, IRQ, etc) as static platform data.
> In some cases you need a level of indirection.
>
> In this particular case a SMSC ethernet chip is connected to an
> OMAP3 processor through its General-Purpose Memory Controller.
>
> You can't just define the I/O memory used by the device since you first
> need to request that address to the GPMC. The same happens with the
> IRQ line since a OMAP GPIO pin is used so you have to first configure
> the GPIO line as input.
>
> So in platform code you call to gpmc_smsc911x_init() passing all the
> GPMC specific configurations (GPIO used for IRQ, GPMC chip select, etc)
>
> Then gpmc_smsc911x_init() does all the needed setup, fills a platform_data
> for the real platform_device and calls  platform_device_register_resndata().
>
> From the smsc911x platform_driver probe function point of view it just have
> resources and doesn't know if it's I/O memory is directly mapped or is
> through a memory controller as the GPMC. It also doesn't know if the IRQ is
> a GPIO or not.

It's still the same difference as far as the device is concerned.
External bus chip-select lines are well understood. The key here is to
encode the CS line number into the reg property of the child node so
that the GPMC driver has the information it needs to configure the
chip selects. You do this by setting #address-cells to '2' in the GPMC
node, and  use the ranges property to map from the gpmc address space
to the cpu address space. Like so (if you had 2 Ethernet controllers)

        gpmc {
                #address-cells = <2>;  // First cell is CS#, second
cell is offset from CS base
                #size-cells = <1>;
                compatible = "ti,gpmc";
                ranges = <0 0 0xf1000000 0x1000>, //CS0 mapped to
0xf1000000..0xf1000fff
                                <1 0 0xf1001000 0x1000>; //CS1 mapped
to 0xf1001000..0xf1001fff

                ethernet@0,0 {
                        compatible = "smsc,91c111";
                        reg = <0 0 0x1000>;
                }
                ethernet@1,0 {
                        compatible = "smsc,91c111";
                        reg = <1 0 0x1000>;
               }
      }

The GPMC driver can use the information in the ranges property for
setting up the chip select mappings. For the smsc,91c111 driver the
mapping becomes transparent.

You can see another example of this in
arch/powerpc/boot/dts/media5200.dts in the localbus node.

g.
Javier Martinez Canillas Feb. 18, 2013, 1:56 p.m. UTC | #11
On 02/18/2013 02:51 PM, Grant Likely wrote:
> On Sun, Feb 10, 2013 at 11:35 AM, Javier Martinez Canillas
> <martinez.javier@gmail.com> wrote:
>> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>>> On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>>>> I knew this would be controversial and that's why I didn't mean it to be a patch
>>>> but a RFC :)
>>>>
>>>> The problem basically is that you have to associate the platform device with its
>>>> corresponding DT device node because it can be used in the driver probe function.
>>>
>>> When DT is being used, doesn't DT create the platform devices for you,
>>> with the device node already set correctly?
>>>
>>
>> Well they usually do but not always just like usually you have a
>> platform_device in your board code and call platform_device_register().
>>
>> But in some configurations you can't just define the platform_device
>> required resources (I/O memory, IRQ, etc) as static platform data.
>> In some cases you need a level of indirection.
>>
>> In this particular case a SMSC ethernet chip is connected to an
>> OMAP3 processor through its General-Purpose Memory Controller.
>>
>> You can't just define the I/O memory used by the device since you first
>> need to request that address to the GPMC. The same happens with the
>> IRQ line since a OMAP GPIO pin is used so you have to first configure
>> the GPIO line as input.
>>
>> So in platform code you call to gpmc_smsc911x_init() passing all the
>> GPMC specific configurations (GPIO used for IRQ, GPMC chip select, etc)
>>
>> Then gpmc_smsc911x_init() does all the needed setup, fills a platform_data
>> for the real platform_device and calls  platform_device_register_resndata().
>>
>> From the smsc911x platform_driver probe function point of view it just have
>> resources and doesn't know if it's I/O memory is directly mapped or is
>> through a memory controller as the GPMC. It also doesn't know if the IRQ is
>> a GPIO or not.
> 
> It's still the same difference as far as the device is concerned.
> External bus chip-select lines are well understood. The key here is to
> encode the CS line number into the reg property of the child node so
> that the GPMC driver has the information it needs to configure the
> chip selects. You do this by setting #address-cells to '2' in the GPMC
> node, and  use the ranges property to map from the gpmc address space
> to the cpu address space. Like so (if you had 2 Ethernet controllers)
> 
>         gpmc {
>                 #address-cells = <2>;  // First cell is CS#, second
> cell is offset from CS base
>                 #size-cells = <1>;
>                 compatible = "ti,gpmc";
>                 ranges = <0 0 0xf1000000 0x1000>, //CS0 mapped to
> 0xf1000000..0xf1000fff
>                                 <1 0 0xf1001000 0x1000>; //CS1 mapped
> to 0xf1001000..0xf1001fff
> 
>                 ethernet@0,0 {
>                         compatible = "smsc,91c111";
>                         reg = <0 0 0x1000>;
>                 }
>                 ethernet@1,0 {
>                         compatible = "smsc,91c111";
>                         reg = <1 0 0x1000>;
>                }
>       }
> 
> The GPMC driver can use the information in the ranges property for
> setting up the chip select mappings. For the smsc,91c111 driver the
> mapping becomes transparent.
> 
> You can see another example of this in
> arch/powerpc/boot/dts/media5200.dts in the localbus node.
> 
> g.
> 

Hello Grant,

Thanks a lot for your explanation. Now is very clear to me how this has to be
done. I'll work on an v2 of this patch-set that do it correctly and will resend.

Best regards,
Javier
diff mbox

Patch

diff --git a/arch/arm/mach-imx/devices/platform-gpio-mxc.c b/arch/arm/mach-imx/devices/platform-gpio-mxc.c
index 26483fa..4f4d8f9 100644
--- a/arch/arm/mach-imx/devices/platform-gpio-mxc.c
+++ b/arch/arm/mach-imx/devices/platform-gpio-mxc.c
@@ -28,5 +28,5 @@  struct platform_device *__init mxc_register_gpio(char *name, int id,
 	};
 
 	return platform_device_register_resndata(&mxc_aips_bus,
-			name, id, res, ARRAY_SIZE(res), NULL, 0);
+			name, id, res, ARRAY_SIZE(res), NULL, 0, NULL);
 }
diff --git a/arch/arm/mach-imx/devices/platform-imx-dma.c b/arch/arm/mach-imx/devices/platform-imx-dma.c
index ccdb5dc..1e3838c 100644
--- a/arch/arm/mach-imx/devices/platform-imx-dma.c
+++ b/arch/arm/mach-imx/devices/platform-imx-dma.c
@@ -28,7 +28,7 @@  struct platform_device __init __maybe_unused *imx_add_imx_dma(char *name,
 	};
 
 	return platform_device_register_resndata(&mxc_ahb_bus,
-			name, -1, res, ARRAY_SIZE(res), NULL, 0);
+			name, -1, res, ARRAY_SIZE(res), NULL, 0, NULL);
 }
 
 struct platform_device __init __maybe_unused *imx_add_imx_sdma(char *name,
@@ -47,5 +47,5 @@  struct platform_device __init __maybe_unused *imx_add_imx_sdma(char *name,
 	};
 
 	return platform_device_register_resndata(&mxc_ahb_bus, name,
-			-1, res, ARRAY_SIZE(res), pdata, sizeof(*pdata));
+			-1, res, ARRAY_SIZE(res), pdata, sizeof(*pdata), NULL);
 }
diff --git a/arch/arm/mach-imx/mach-armadillo5x0.c b/arch/arm/mach-imx/mach-armadillo5x0.c
index 59bd6b0..e664681 100644
--- a/arch/arm/mach-imx/mach-armadillo5x0.c
+++ b/arch/arm/mach-imx/mach-armadillo5x0.c
@@ -519,7 +519,7 @@  static void __init armadillo5x0_init(void)
 	platform_device_register_resndata(NULL, "physmap-flash", -1,
 			&armadillo5x0_nor_flash_resource, 1,
 			&armadillo5x0_nor_flash_pdata,
-			sizeof(armadillo5x0_nor_flash_pdata));
+			sizeof(armadillo5x0_nor_flash_pdata), NULL);
 
 	/* Register NAND Flash */
 	imx31_add_mxc_nand(&armadillo5x0_nand_board_info);
diff --git a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
index 318bd8d..5065f67 100644
--- a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
+++ b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
@@ -570,7 +570,8 @@  static void __init visstrim_m10_board_init(void)
 	imx_add_platform_device("mx27vis", 0, NULL, 0, &snd_mx27vis_pdata,
 				sizeof(snd_mx27vis_pdata));
 	platform_device_register_resndata(NULL, "soc-camera-pdrv", 0, NULL, 0,
-				      &iclink_tvp5150, sizeof(iclink_tvp5150));
+				      &iclink_tvp5150, sizeof(iclink_tvp5150),
+				      NULL);
 	gpio_led_register_device(0, &visstrim_m10_led_data);
 
 	/* Use mother board version to decide what video devices we shall use */
diff --git a/arch/arm/mach-imx/mach-mx1ads.c b/arch/arm/mach-imx/mach-mx1ads.c
index 06b4837..9ce64a5 100644
--- a/arch/arm/mach-imx/mach-mx1ads.c
+++ b/arch/arm/mach-imx/mach-mx1ads.c
@@ -118,7 +118,7 @@  static void __init mx1ads_init(void)
 	/* Physmap flash */
 	platform_device_register_resndata(NULL, "physmap-flash", 0,
 			&flash_resource, 1,
-			&mx1ads_flash_data, sizeof(mx1ads_flash_data));
+			&mx1ads_flash_data, sizeof(mx1ads_flash_data), NULL);
 
 	/* I2C */
 	i2c_register_board_info(0, mx1ads_i2c_devices,
diff --git a/arch/arm/mach-nomadik/cpu-8815.c b/arch/arm/mach-nomadik/cpu-8815.c
index 1273931..b117d6c 100644
--- a/arch/arm/mach-nomadik/cpu-8815.c
+++ b/arch/arm/mach-nomadik/cpu-8815.c
@@ -65,7 +65,7 @@  cpu8815_add_gpio(int id, resource_size_t addr, int irq,
 
 	return platform_device_register_resndata(NULL, "gpio", id,
 				resources, ARRAY_SIZE(resources),
-				pdata, sizeof(*pdata));
+				pdata, sizeof(*pdata), NULL);
 }
 
 void cpu8815_add_gpios(resource_size_t *base, int num, int irq,
diff --git a/arch/arm/mach-omap2/fb.c b/arch/arm/mach-omap2/fb.c
index 190ae49..ba2e5ae 100644
--- a/arch/arm/mach-omap2/fb.c
+++ b/arch/arm/mach-omap2/fb.c
@@ -81,7 +81,7 @@  static int __init omap_init_vrfb(void)
 	}
 
 	pdev = platform_device_register_resndata(NULL, "omapvrfb", -1,
-			res, num_res, NULL, 0);
+			res, num_res, NULL, 0, NULL);
 
 	if (IS_ERR(pdev))
 		return PTR_ERR(pdev);
diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c
index ef99011..5ce00ad2 100644
--- a/arch/arm/mach-omap2/gpmc-smsc911x.c
+++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
@@ -82,7 +82,7 @@  void __init gpmc_smsc911x_init(struct omap_smsc911x_platform_data *gpmc_cfg)
 
 	pdev = platform_device_register_resndata(NULL, "smsc911x", gpmc_cfg->id,
 		 gpmc_smsc911x_resources, ARRAY_SIZE(gpmc_smsc911x_resources),
-		 &gpmc_smsc911x_config, sizeof(gpmc_smsc911x_config));
+		 &gpmc_smsc911x_config, sizeof(gpmc_smsc911x_config), NULL);
 	if (!pdev) {
 		pr_err("Unable to register platform device\n");
 		gpio_free(gpmc_cfg->gpio_reset);
diff --git a/arch/arm/mach-ux500/board-mop500-audio.c b/arch/arm/mach-ux500/board-mop500-audio.c
index 7209db7..5ebbc65 100644
--- a/arch/arm/mach-ux500/board-mop500-audio.c
+++ b/arch/arm/mach-ux500/board-mop500-audio.c
@@ -129,7 +129,7 @@  static struct platform_device *db8500_add_msp_i2s(struct device *parent,
 		id, irq);
 	pdev = platform_device_register_resndata(parent, "ux500-msp-i2s", id,
 						res, ARRAY_SIZE(res),
-						pdata, sizeof(*pdata));
+						pdata, sizeof(*pdata), NULL);
 	if (!pdev) {
 		pr_err("Failed to register platform-device 'ux500-msp-i2s.%d'!\n",
 			id);
diff --git a/arch/arm/mach-ux500/devices-common.c b/arch/arm/mach-ux500/devices-common.c
index 16b5f71..dcbe6c2 100644
--- a/arch/arm/mach-ux500/devices-common.c
+++ b/arch/arm/mach-ux500/devices-common.c
@@ -42,7 +42,8 @@  dbx500_add_gpio(struct device *parent, int id, resource_size_t addr, int irq,
 		resources,
 		ARRAY_SIZE(resources),
 		pdata,
-		sizeof(*pdata));
+		sizeof(*pdata),
+		NULL);
 }
 
 void dbx500_add_gpios(struct device *parent, resource_size_t *base, int num,
diff --git a/arch/arm/mach-ux500/devices-db8500.h b/arch/arm/mach-ux500/devices-db8500.h
index a5e05f6..5a8ed73 100644
--- a/arch/arm/mach-ux500/devices-db8500.h
+++ b/arch/arm/mach-ux500/devices-db8500.h
@@ -26,7 +26,7 @@  db8500_add_ske_keypad(struct device *parent,
 	};
 
 	return platform_device_register_resndata(parent, "nmk-ske-keypad", -1,
-						 resources, 2, pdata, size);
+						 resources, 2, pdata, size, NULL);
 }
 
 static inline struct amba_device *
diff --git a/arch/unicore32/kernel/puv3-core.c b/arch/unicore32/kernel/puv3-core.c
index 254adee..8143a18 100644
--- a/arch/unicore32/kernel/puv3-core.c
+++ b/arch/unicore32/kernel/puv3-core.c
@@ -274,6 +274,6 @@  void __init puv3_core_init(void)
 	platform_device_register_simple("PKUnity-v3-AC97", -1, NULL, 0);
 	platform_device_register_resndata(&platform_bus, "musb_hdrc", -1,
 			puv3_usb_resources, ARRAY_SIZE(puv3_usb_resources),
-			&puv3_usb_plat, sizeof(puv3_usb_plat));
+			&puv3_usb_plat, sizeof(puv3_usb_plat), NULL);
 }
 
diff --git a/arch/unicore32/kernel/puv3-nb0916.c b/arch/unicore32/kernel/puv3-nb0916.c
index 181108b..b2a55cb 100644
--- a/arch/unicore32/kernel/puv3-nb0916.c
+++ b/arch/unicore32/kernel/puv3-nb0916.c
@@ -119,7 +119,7 @@  int __init mach_nb0916_init(void)
 
 	platform_device_register_resndata(&platform_bus, "physmap-flash", -1,
 			&physmap_flash_resource, 1,
-			&physmap_flash_data, sizeof(physmap_flash_data));
+			&physmap_flash_data, sizeof(physmap_flash_data), NULL);
 
 	if (request_irq(gpio_to_irq(GPI_LCD_CASE_OFF),
 		&nb0916_lcdcaseoff_handler,
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index c0b8df3..79ba66a 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -437,6 +437,7 @@  struct platform_device *platform_device_register_full(
 		goto err_alloc;
 
 	pdev->dev.parent = pdevinfo->parent;
+	pdev->dev.of_node = pdevinfo->of_node;
 	ACPI_HANDLE_SET(&pdev->dev, pdevinfo->acpi_node.handle);
 
 	if (pdevinfo->dma_mask) {
diff --git a/drivers/leds/leds-gpio-register.c b/drivers/leds/leds-gpio-register.c
index 1c4ed55..e1f1c15 100644
--- a/drivers/leds/leds-gpio-register.c
+++ b/drivers/leds/leds-gpio-register.c
@@ -34,7 +34,7 @@  struct platform_device *__init gpio_led_register_device(
 		return ERR_PTR(-ENOMEM);
 
 	ret = platform_device_register_resndata(NULL, "leds-gpio", id,
-			NULL, 0, &_pdata, sizeof(_pdata));
+			NULL, 0, &_pdata, sizeof(_pdata), NULL);
 	if (IS_ERR(ret))
 		kfree(_pdata.leds);
 
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 31f966f..0709ad6 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -566,7 +566,7 @@  static int vm_cmdline_set(const char *device,
 
 	pdev = platform_device_register_resndata(&vm_cmdline_parent,
 			"virtio-mmio", vm_cmdline_id++,
-			resources, ARRAY_SIZE(resources), NULL, 0);
+			resources, ARRAY_SIZE(resources), NULL, 0, NULL);
 	if (IS_ERR(pdev))
 		return PTR_ERR(pdev);
 
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index a9ded9a..29393bd 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -66,6 +66,7 @@  struct platform_device_info {
 		const void *data;
 		size_t size_data;
 		u64 dma_mask;
+		struct device_node *of_node;
 };
 extern struct platform_device *platform_device_register_full(
 		const struct platform_device_info *pdevinfo);
@@ -81,13 +82,14 @@  extern struct platform_device *platform_device_register_full(
  * @num: number of resources
  * @data: platform specific data for this platform device
  * @size: size of platform specific data
+ * @of_node: device node of this platform device
  *
  * Returns &struct platform_device pointer on success, or ERR_PTR() on error.
  */
 static inline struct platform_device *platform_device_register_resndata(
 		struct device *parent, const char *name, int id,
 		const struct resource *res, unsigned int num,
-		const void *data, size_t size) {
+		const void *data, size_t size, struct device_node *of_node) {
 
 	struct platform_device_info pdevinfo = {
 		.parent = parent,
@@ -98,6 +100,7 @@  static inline struct platform_device *platform_device_register_resndata(
 		.data = data,
 		.size_data = size,
 		.dma_mask = 0,
+		.of_node = of_node,
 	};
 
 	return platform_device_register_full(&pdevinfo);
@@ -130,7 +133,7 @@  static inline struct platform_device *platform_device_register_simple(
 		const struct resource *res, unsigned int num)
 {
 	return platform_device_register_resndata(NULL, name, id,
-			res, num, NULL, 0);
+			res, num, NULL, 0, NULL);
 }
 
 /**
@@ -154,7 +157,7 @@  static inline struct platform_device *platform_device_register_data(
 		const void *data, size_t size)
 {
 	return platform_device_register_resndata(parent, name, id,
-			NULL, 0, data, size);
+			NULL, 0, data, size, NULL);
 }
 
 extern struct platform_device *platform_device_alloc(const char *name, int id);
diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index d2d124f..e6eabb9 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -982,7 +982,7 @@  static struct i2s_dai *i2s_alloc_dai(struct platform_device *pdev, bool sec)
 	} else {	/* Create a new platform_device for Secondary */
 		i2s->pdev = platform_device_register_resndata(NULL,
 				pdev->name, pdev->id + SAMSUNG_I2S_SECOFF,
-				NULL, 0, NULL, 0);
+				NULL, 0, NULL, 0, NULL);
 		if (IS_ERR(i2s->pdev))
 			return NULL;
 	}