linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Using device tree overlays in Linux
@ 2019-04-04  1:50 Chris Packham
  2019-04-06  6:07 ` Rob Herring
  2019-04-08  1:05 ` Frank Rowand
  0 siblings, 2 replies; 8+ messages in thread
From: Chris Packham @ 2019-04-04  1:50 UTC (permalink / raw)
  To: pantelis.antoniou, frowand.list, robh+dt
  Cc: devicetree, linux-kernel, Hamish Martin

Hi,

I'm implementing support for some modular Linux based systems using 
device tree overlays. The code is working but it seems a little more 
fiddly that than it should be so I'm wondering if I'm doing it right.

An example of what I'm doing is


arch/arm/boot/dts/Makefile:
DTC_FLAGS_myboard += -@

drivers/foo/Makefile:
obj-y += myplugin.dtb.o
obj-y += mydriver.o

drivers/foo/myplugin.dts:
/dts-v1/;
/plugin/;
/{
	fragment@0 {
		target = <&i2c0>;
		__overlay__ {
			gpio@74 {
				compatible = "nxp,pca9539";
				reg = <0x74>
			};
		};
	};
};

drivers/foo/mydriver.c:
extern uint8_t __dtb_myplugin_begin[];
extern uint8_t __dtb_myplugin_end[];

int mydriver_probe(struct platform_device *pdev)
{
	u32 size = __dtb_myplugin_end - __dtb_myplugin_begin;
	int overlay_id;
	int ret;
	
	ret = of_overlay_fdt_apply(__dtb_myplugin_begin,
				   size, &overlay_id);
	return ret;
}


The first issue is that I need to add -@ to the DTC_FLAGS for my board 
dtb. I kind of understand that I only need -@ if my overlay targets 
something symbolic so I might not need it but I was surprised that there 
wasn't a Kconfig option that makes this happen automatically.

externing things in C files makes checkpatch.pl complain. I see the 
of/unittests.c and rcar_du_of.c hide this with a macro. I was again 
surprised that there wasn't a common macro to declare these.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Using device tree overlays in Linux
  2019-04-04  1:50 Using device tree overlays in Linux Chris Packham
@ 2019-04-06  6:07 ` Rob Herring
  2019-04-08  1:05 ` Frank Rowand
  1 sibling, 0 replies; 8+ messages in thread
From: Rob Herring @ 2019-04-06  6:07 UTC (permalink / raw)
  To: Chris Packham
  Cc: pantelis.antoniou, frowand.list, devicetree, linux-kernel, Hamish Martin

On Thu, Apr 04, 2019 at 01:50:20AM +0000, Chris Packham wrote:
> Hi,
> 
> I'm implementing support for some modular Linux based systems using 
> device tree overlays. The code is working but it seems a little more 
> fiddly that than it should be so I'm wondering if I'm doing it right.
> 
> An example of what I'm doing is
> 
> 
> arch/arm/boot/dts/Makefile:
> DTC_FLAGS_myboard += -@
> 
> drivers/foo/Makefile:
> obj-y += myplugin.dtb.o
> obj-y += mydriver.o
> 
> drivers/foo/myplugin.dts:
> /dts-v1/;
> /plugin/;
> /{
> 	fragment@0 {
> 		target = <&i2c0>;
> 		__overlay__ {
> 			gpio@74 {
> 				compatible = "nxp,pca9539";
> 				reg = <0x74>
> 			};
> 		};
> 	};
> };
> 
> drivers/foo/mydriver.c:
> extern uint8_t __dtb_myplugin_begin[];
> extern uint8_t __dtb_myplugin_end[];
> 
> int mydriver_probe(struct platform_device *pdev)
> {
> 	u32 size = __dtb_myplugin_end - __dtb_myplugin_begin;
> 	int overlay_id;
> 	int ret;
> 	
> 	ret = of_overlay_fdt_apply(__dtb_myplugin_begin,
> 				   size, &overlay_id);
> 	return ret;
> }
> 
> 
> The first issue is that I need to add -@ to the DTC_FLAGS for my board 
> dtb. I kind of understand that I only need -@ if my overlay targets 
> something symbolic so I might not need it but I was surprised that there 
> wasn't a Kconfig option that makes this happen automatically.

Whether overlays make sense or are needed are per board.

You could add a kconfig entry that drivers which depend on overlays 
select, but turning on '-@' has to be per board (or SoC family if the 
SoC maintainer is okay with that).

> externing things in C files makes checkpatch.pl complain. I see the 
> of/unittests.c and rcar_du_of.c hide this with a macro. I was again 
> surprised that there wasn't a common macro to declare these.

Feel free to propose something. There just aren't that many cases that 
anyone has cared what checkpatch says.

Rob


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Using device tree overlays in Linux
  2019-04-04  1:50 Using device tree overlays in Linux Chris Packham
  2019-04-06  6:07 ` Rob Herring
@ 2019-04-08  1:05 ` Frank Rowand
  2019-04-08  1:27   ` Chris Packham
  1 sibling, 1 reply; 8+ messages in thread
From: Frank Rowand @ 2019-04-08  1:05 UTC (permalink / raw)
  To: Chris Packham, pantelis.antoniou, robh+dt
  Cc: devicetree, linux-kernel, Hamish Martin

Hi Chris,

On 4/3/19 6:50 PM, Chris Packham wrote:
> Hi,
> 
> I'm implementing support for some modular Linux based systems using 
> device tree overlays. The code is working but it seems a little more 
> fiddly that than it should be so I'm wondering if I'm doing it right.

Let me start by saying that I strongly discourage using device tree
overlays in the Linux kernel until the support is more baked.  For
some info on how unbaked overlays are, see:

   https://elinux.org/Frank%27s_Evolving_Overlay_Thoughts

You should consider applying overlays in the Linux kernel to be
fragile at best.

If you can not figure out how to solve your needs without using
overlays, then having the boot loader apply the overlay instead
of the kernel applying the overlay avoids most of the issues.


> An example of what I'm doing is
> 
> 
> arch/arm/boot/dts/Makefile:
> DTC_FLAGS_myboard += -@
> 
> drivers/foo/Makefile:
> obj-y += myplugin.dtb.o
> obj-y += mydriver.o
> 
> drivers/foo/myplugin.dts:
> /dts-v1/;
> /plugin/;
> /{
> 	fragment@0 {
> 		target = <&i2c0>;
> 		__overlay__ {
> 			gpio@74 {
> 				compatible = "nxp,pca9539";
> 				reg = <0x74>
> 			};
> 		};
> 	};
> };

The fragment and __overlay__ nodes are implementation, subject to
change, and should not be hand coded.  The dtc compiler has added
features to allow a more generic source code.  The above should be:

// SPDX-License-Identifier: GPL-2.0
/dts-v1/;
/plugin/;

&i2c0 {
	gpio@74 {
		compatible = "nxp,pca9539";
		reg = <0x74>;
	};
};

(Not compiled, not tested.)

The rcar overlay sources were merged before dtc was updated to
handle this new syntax, so they are not a good example for
how to write an overlay.


> drivers/foo/mydriver.c:
> extern uint8_t __dtb_myplugin_begin[];
> extern uint8_t __dtb_myplugin_end[];
> 
> int mydriver_probe(struct platform_device *pdev)
> {
> 	u32 size = __dtb_myplugin_end - __dtb_myplugin_begin;
> 	int overlay_id;
> 	int ret;
> 	
> 	ret = of_overlay_fdt_apply(__dtb_myplugin_begin,
> 				   size, &overlay_id);
> 	return ret;
> }

I'm guessing that you have simplified your use case to make it easier to
describe (thank you for that).  But that also makes it harder to understand
the use case, and whether this is a good solution.  Can you describe your
use case in some more detail?


> The first issue is that I need to add -@ to the DTC_FLAGS for my board 
> dtb. I kind of understand that I only need -@ if my overlay targets 
> something symbolic so I might not need it but I was surprised that there 
> wasn't a Kconfig option that makes this happen automatically.
> 
> externing things in C files makes checkpatch.pl complain. I see the 
> of/unittests.c and rcar_du_of.c hide this with a macro. I was again 
> surprised that there wasn't a common macro to declare these.

unittests.c should not be used as a model of how to code for devicetree.
There are some hacks that are needed to be able to test, but should not
be used by normal drivers.

The rcar use of overlays is a temporary hack to provide backward
compatibility.  The intent is to drop this code once the backward
compatibility window ends.

The long-term intent (once enough of the overlay code is in place) is
to provide an overlay loader than can apply an overlay .dtb from a file.

-Frank

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Using device tree overlays in Linux
  2019-04-08  1:05 ` Frank Rowand
@ 2019-04-08  1:27   ` Chris Packham
  2019-04-09 19:19     ` Frank Rowand
  2019-04-09 19:33     ` Frank Rowand
  0 siblings, 2 replies; 8+ messages in thread
From: Chris Packham @ 2019-04-08  1:27 UTC (permalink / raw)
  To: Frank Rowand, pantelis.antoniou, robh+dt
  Cc: devicetree, linux-kernel, Hamish Martin

Hi Frank,

On 8/04/19 1:05 PM, Frank Rowand wrote:
> Hi Chris,
> 
> On 4/3/19 6:50 PM, Chris Packham wrote:
>> Hi,
>>
>> I'm implementing support for some modular Linux based systems using
>> device tree overlays. The code is working but it seems a little more
>> fiddly that than it should be so I'm wondering if I'm doing it right.
> 
> Let me start by saying that I strongly discourage using device tree
> overlays in the Linux kernel until the support is more baked.  For
> some info on how unbaked overlays are, see:
> 
>     https://elinux.org/Frank%27s_Evolving_Overlay_Thoughts
> 
> You should consider applying overlays in the Linux kernel to be
> fragile at best.
> 
> If you can not figure out how to solve your needs without using
> overlays, then having the boot loader apply the overlay instead
> of the kernel applying the overlay avoids most of the issues.
> 

Consider us beta-testers :).

We've got a few modular systems that have miscellaneous devices that we 
want to be hot-swapping at run time. For the most part the devices in 
question are i2c based. We want to use overlays as it saves manually 
instantiating devices and avoids needing specific knowledge of the base 
platform.

>> An example of what I'm doing is
>>
>>
>> arch/arm/boot/dts/Makefile:
>> DTC_FLAGS_myboard += -@
>>
>> drivers/foo/Makefile:
>> obj-y += myplugin.dtb.o
>> obj-y += mydriver.o
>>
>> drivers/foo/myplugin.dts:
>> /dts-v1/;
>> /plugin/;
>> /{
>> 	fragment@0 {
>> 		target = <&i2c0>;
>> 		__overlay__ {
>> 			gpio@74 {
>> 				compatible = "nxp,pca9539";
>> 				reg = <0x74>
>> 			};
>> 		};
>> 	};
>> };
> 
> The fragment and __overlay__ nodes are implementation, subject to
> change, and should not be hand coded.  The dtc compiler has added
> features to allow a more generic source code.  The above should be:
> 
> // SPDX-License-Identifier: GPL-2.0
> /dts-v1/;
> /plugin/;
> 
> &i2c0 {
> 	gpio@74 {
> 		compatible = "nxp,pca9539";
> 		reg = <0x74>;
> 	};
> };
> 
> (Not compiled, not tested.)
> 
> The rcar overlay sources were merged before dtc was updated to
> handle this new syntax, so they are not a good example for
> how to write an overlay.

OK thanks. I suspected as much. I'll update my code based on your example.

>> drivers/foo/mydriver.c:
>> extern uint8_t __dtb_myplugin_begin[];
>> extern uint8_t __dtb_myplugin_end[];
>>
>> int mydriver_probe(struct platform_device *pdev)
>> {
>> 	u32 size = __dtb_myplugin_end - __dtb_myplugin_begin;
>> 	int overlay_id;
>> 	int ret;
>> 	
>> 	ret = of_overlay_fdt_apply(__dtb_myplugin_begin,
>> 				   size, &overlay_id);
>> 	return ret;
>> }
> 
> I'm guessing that you have simplified your use case to make it easier to
> describe (thank you for that).  But that also makes it harder to understand
> the use case, and whether this is a good solution.  Can you describe your
> use case in some more detail?

As I said above we have a few different modular systems we're trying to 
support. But they all do basically the same thing.

The insertion of a module is detected based on a gpio line being asserted.

Then we identify what was just inserted, the details vary a little per 
platform but in one instance we load an overlay that has instantiates an 
i2c eeprom identifying the module.

Once we know which specific module has been inserted based on the data 
in the eeprom we load another overlay that instantiates the rest of the 
devices on that module.

>> The first issue is that I need to add -@ to the DTC_FLAGS for my board
>> dtb. I kind of understand that I only need -@ if my overlay targets
>> something symbolic so I might not need it but I was surprised that there
>> wasn't a Kconfig option that makes this happen automatically.
>>
>> externing things in C files makes checkpatch.pl complain. I see the
>> of/unittests.c and rcar_du_of.c hide this with a macro. I was again
>> surprised that there wasn't a common macro to declare these.
> 
> unittests.c should not be used as a model of how to code for devicetree.
> There are some hacks that are needed to be able to test, but should not
> be used by normal drivers.
> 
> The rcar use of overlays is a temporary hack to provide backward
> compatibility.  The intent is to drop this code once the backward
> compatibility window ends.

Yes I suspected as much.

> The long-term intent (once enough of the overlay code is in place) is
> to provide an overlay loader than can apply an overlay .dtb from a file.

Would that be a kernel helper or driven via udev?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Using device tree overlays in Linux
  2019-04-08  1:27   ` Chris Packham
@ 2019-04-09 19:19     ` Frank Rowand
  2019-04-09 20:54       ` Chris Packham
  2019-04-09 19:33     ` Frank Rowand
  1 sibling, 1 reply; 8+ messages in thread
From: Frank Rowand @ 2019-04-09 19:19 UTC (permalink / raw)
  To: Chris Packham, pantelis.antoniou, robh+dt
  Cc: devicetree, linux-kernel, Hamish Martin

On 4/7/19 6:27 PM, Chris Packham wrote:
> Hi Frank,
> 
> On 8/04/19 1:05 PM, Frank Rowand wrote:
>> Hi Chris,
>>
>> On 4/3/19 6:50 PM, Chris Packham wrote:
>>> Hi,
>>>
>>> I'm implementing support for some modular Linux based systems using
>>> device tree overlays. The code is working but it seems a little more
>>> fiddly that than it should be so I'm wondering if I'm doing it right.
>>
>> Let me start by saying that I strongly discourage using device tree
>> overlays in the Linux kernel until the support is more baked.  For
>> some info on how unbaked overlays are, see:
>>
>>     https://elinux.org/Frank%27s_Evolving_Overlay_Thoughts
>>
>> You should consider applying overlays in the Linux kernel to be
>> fragile at best.
>>
>> If you can not figure out how to solve your needs without using
>> overlays, then having the boot loader apply the overlay instead
>> of the kernel applying the overlay avoids most of the issues.
>>
> 
> Consider us beta-testers :).

You should read the elinux.org page more carefully.  The support
isn't even alpha stage.  It is at the level of proof of concept.

I'll update that page to explicitly say the code is proof of
concept.

< snip >

I'll reply to the rest of the email separately.

-Frank

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Using device tree overlays in Linux
  2019-04-08  1:27   ` Chris Packham
  2019-04-09 19:19     ` Frank Rowand
@ 2019-04-09 19:33     ` Frank Rowand
  2019-04-09 20:47       ` Chris Packham
  1 sibling, 1 reply; 8+ messages in thread
From: Frank Rowand @ 2019-04-09 19:33 UTC (permalink / raw)
  To: Chris Packham, pantelis.antoniou, robh+dt
  Cc: devicetree, linux-kernel, Hamish Martin

On 4/7/19 6:27 PM, Chris Packham wrote:
> Hi Frank,
> 
> On 8/04/19 1:05 PM, Frank Rowand wrote:
>> Hi Chris,
>>
>> On 4/3/19 6:50 PM, Chris Packham wrote:
>>> Hi,
>>>
>>> I'm implementing support for some modular Linux based systems using
>>> device tree overlays. The code is working but it seems a little more
>>> fiddly that than it should be so I'm wondering if I'm doing it right.
>>
>> Let me start by saying that I strongly discourage using device tree
>> overlays in the Linux kernel until the support is more baked.  For
>> some info on how unbaked overlays are, see:
>>
>>     https://elinux.org/Frank%27s_Evolving_Overlay_Thoughts
>>
>> You should consider applying overlays in the Linux kernel to be
>> fragile at best.
>>
>> If you can not figure out how to solve your needs without using
>> overlays, then having the boot loader apply the overlay instead
>> of the kernel applying the overlay avoids most of the issues.
>>
> 
> Consider us beta-testers :).
> 
> We've got a few modular systems that have miscellaneous devices that we 
> want to be hot-swapping at run time. For the most part the devices in 
> question are i2c based. We want to use overlays as it saves manually 
> instantiating devices and avoids needing specific knowledge of the base 
> platform.

I want to make sure we are using a common vocabulary.

By hot-swap, do you mean physically plugging in or removing the device
_while the kernel is booted_?

If this is the case, then my suggestion of using the boot loader to
apply the devicetree overlay is not sufficient.


>>> An example of what I'm doing is
>>>
>>>
>>> arch/arm/boot/dts/Makefile:
>>> DTC_FLAGS_myboard += -@
>>>
>>> drivers/foo/Makefile:
>>> obj-y += myplugin.dtb.o
>>> obj-y += mydriver.o
>>>
>>> drivers/foo/myplugin.dts:
>>> /dts-v1/;
>>> /plugin/;
>>> /{
>>> 	fragment@0 {
>>> 		target = <&i2c0>;
>>> 		__overlay__ {
>>> 			gpio@74 {
>>> 				compatible = "nxp,pca9539";
>>> 				reg = <0x74>
>>> 			};
>>> 		};
>>> 	};
>>> };
>>
>> The fragment and __overlay__ nodes are implementation, subject to
>> change, and should not be hand coded.  The dtc compiler has added
>> features to allow a more generic source code.  The above should be:
>>
>> // SPDX-License-Identifier: GPL-2.0
>> /dts-v1/;
>> /plugin/;
>>
>> &i2c0 {
>> 	gpio@74 {
>> 		compatible = "nxp,pca9539";
>> 		reg = <0x74>;
>> 	};
>> };
>>
>> (Not compiled, not tested.)
>>
>> The rcar overlay sources were merged before dtc was updated to
>> handle this new syntax, so they are not a good example for
>> how to write an overlay.
> 
> OK thanks. I suspected as much. I'll update my code based on your example.
> 
>>> drivers/foo/mydriver.c:
>>> extern uint8_t __dtb_myplugin_begin[];
>>> extern uint8_t __dtb_myplugin_end[];
>>>
>>> int mydriver_probe(struct platform_device *pdev)
>>> {
>>> 	u32 size = __dtb_myplugin_end - __dtb_myplugin_begin;
>>> 	int overlay_id;
>>> 	int ret;
>>> 	
>>> 	ret = of_overlay_fdt_apply(__dtb_myplugin_begin,
>>> 				   size, &overlay_id);
>>> 	return ret;
>>> }
>>
>> I'm guessing that you have simplified your use case to make it easier to
>> describe (thank you for that).  But that also makes it harder to understand
>> the use case, and whether this is a good solution.  Can you describe your
>> use case in some more detail?
> 
> As I said above we have a few different modular systems we're trying to 
> support. But they all do basically the same thing.
> 
> The insertion of a module is detected based on a gpio line being asserted.
> 
> Then we identify what was just inserted, the details vary a little per 
> platform but in one instance we load an overlay that has instantiates an 
> i2c eeprom identifying the module.
> 
> Once we know which specific module has been inserted based on the data 
> in the eeprom we load another overlay that instantiates the rest of the 
> devices on that module.

As an alternate solution, could nodes for all of the possible modules be
included in the devicetree, but disabled?  Then as modules are detected, the
appropriate nodes could be enabled.  (This is a conceptual proposal - the
implementation details might encounter issues, some of which are similar
to the issues overlays face.)


>>> The first issue is that I need to add -@ to the DTC_FLAGS for my board
>>> dtb. I kind of understand that I only need -@ if my overlay targets
>>> something symbolic so I might not need it but I was surprised that there
>>> wasn't a Kconfig option that makes this happen automatically.
>>>
>>> externing things in C files makes checkpatch.pl complain. I see the
>>> of/unittests.c and rcar_du_of.c hide this with a macro. I was again
>>> surprised that there wasn't a common macro to declare these.
>>
>> unittests.c should not be used as a model of how to code for devicetree.
>> There are some hacks that are needed to be able to test, but should not
>> be used by normal drivers.
>>
>> The rcar use of overlays is a temporary hack to provide backward
>> compatibility.  The intent is to drop this code once the backward
>> compatibility window ends.
> 
> Yes I suspected as much.
> 
>> The long-term intent (once enough of the overlay code is in place) is
>> to provide an overlay loader than can apply an overlay .dtb from a file.
> 
> Would that be a kernel helper or driven via udev?

The implementation is TBD.  I would prefer to defer the implementation
discussion until we are closer to being willing to accept a loader.

-Frank


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Using device tree overlays in Linux
  2019-04-09 19:33     ` Frank Rowand
@ 2019-04-09 20:47       ` Chris Packham
  0 siblings, 0 replies; 8+ messages in thread
From: Chris Packham @ 2019-04-09 20:47 UTC (permalink / raw)
  To: Frank Rowand, pantelis.antoniou, robh+dt
  Cc: devicetree, linux-kernel, Hamish Martin

On 10/04/19 7:33 AM, Frank Rowand wrote:
> On 4/7/19 6:27 PM, Chris Packham wrote:
>> Hi Frank,
>>
>> On 8/04/19 1:05 PM, Frank Rowand wrote:
>>> Hi Chris,
>>>
>>> On 4/3/19 6:50 PM, Chris Packham wrote:
>>>> Hi,
>>>>
>>>> I'm implementing support for some modular Linux based systems using
>>>> device tree overlays. The code is working but it seems a little more
>>>> fiddly that than it should be so I'm wondering if I'm doing it right.
>>>
>>> Let me start by saying that I strongly discourage using device tree
>>> overlays in the Linux kernel until the support is more baked.  For
>>> some info on how unbaked overlays are, see:
>>>
>>>      https://elinux.org/Frank%27s_Evolving_Overlay_Thoughts
>>>
>>> You should consider applying overlays in the Linux kernel to be
>>> fragile at best.
>>>
>>> If you can not figure out how to solve your needs without using
>>> overlays, then having the boot loader apply the overlay instead
>>> of the kernel applying the overlay avoids most of the issues.
>>>
>>
>> Consider us beta-testers :).
>>
>> We've got a few modular systems that have miscellaneous devices that we
>> want to be hot-swapping at run time. For the most part the devices in
>> question are i2c based. We want to use overlays as it saves manually
>> instantiating devices and avoids needing specific knowledge of the base
>> platform.
> 
> I want to make sure we are using a common vocabulary.
> 
> By hot-swap, do you mean physically plugging in or removing the device
> _while the kernel is booted_?

Yes that's what I mean. The modules can be physically changed while the 
kernel is running. The more common term might be hot-plug but for 
whatever reason hot-swap has stuck as a term from our pre-linux days.

If you don't mind the advertising this is the kind of system I'm talking 
about:

https://www.alliedtelesis.com/products/switches/x908-gen2

Those modules can be replaced at run time and have various i2c and mdio 
connections running to the base unit.


> If this is the case, then my suggestion of using the boot loader to
> apply the devicetree overlay is not sufficient.

Correct. We did have another system that was still modular but not 
hot-swappable (basically a late binding assembly). For that having the 
bootloader manipulate the DTS before booting the kernel worked quite well.

>>>> An example of what I'm doing is
>>>>
>>>>
>>>> arch/arm/boot/dts/Makefile:
>>>> DTC_FLAGS_myboard += -@
>>>>
>>>> drivers/foo/Makefile:
>>>> obj-y += myplugin.dtb.o
>>>> obj-y += mydriver.o
>>>>
>>>> drivers/foo/myplugin.dts:
>>>> /dts-v1/;
>>>> /plugin/;
>>>> /{
>>>> 	fragment@0 {
>>>> 		target = <&i2c0>;
>>>> 		__overlay__ {
>>>> 			gpio@74 {
>>>> 				compatible = "nxp,pca9539";
>>>> 				reg = <0x74>
>>>> 			};
>>>> 		};
>>>> 	};
>>>> };
>>>
>>> The fragment and __overlay__ nodes are implementation, subject to
>>> change, and should not be hand coded.  The dtc compiler has added
>>> features to allow a more generic source code.  The above should be:
>>>
>>> // SPDX-License-Identifier: GPL-2.0
>>> /dts-v1/;
>>> /plugin/;
>>>
>>> &i2c0 {
>>> 	gpio@74 {
>>> 		compatible = "nxp,pca9539";
>>> 		reg = <0x74>;
>>> 	};
>>> };
>>>
>>> (Not compiled, not tested.)
>>>
>>> The rcar overlay sources were merged before dtc was updated to
>>> handle this new syntax, so they are not a good example for
>>> how to write an overlay.
>>
>> OK thanks. I suspected as much. I'll update my code based on your example.
>>

That worked quite well thanks. I'd be happy to add something to the 
documentation as an example if you'd like.

>>>> drivers/foo/mydriver.c:
>>>> extern uint8_t __dtb_myplugin_begin[];
>>>> extern uint8_t __dtb_myplugin_end[];
>>>>
>>>> int mydriver_probe(struct platform_device *pdev)
>>>> {
>>>> 	u32 size = __dtb_myplugin_end - __dtb_myplugin_begin;
>>>> 	int overlay_id;
>>>> 	int ret;
>>>> 	
>>>> 	ret = of_overlay_fdt_apply(__dtb_myplugin_begin,
>>>> 				   size, &overlay_id);
>>>> 	return ret;
>>>> }
>>>
>>> I'm guessing that you have simplified your use case to make it easier to
>>> describe (thank you for that).  But that also makes it harder to understand
>>> the use case, and whether this is a good solution.  Can you describe your
>>> use case in some more detail?
>>
>> As I said above we have a few different modular systems we're trying to
>> support. But they all do basically the same thing.
>>
>> The insertion of a module is detected based on a gpio line being asserted.
>>
>> Then we identify what was just inserted, the details vary a little per
>> platform but in one instance we load an overlay that has instantiates an
>> i2c eeprom identifying the module.
>>
>> Once we know which specific module has been inserted based on the data
>> in the eeprom we load another overlay that instantiates the rest of the
>> devices on that module.
> 
> As an alternate solution, could nodes for all of the possible modules be
> included in the devicetree, but disabled?  Then as modules are detected, the
> appropriate nodes could be enabled.  (This is a conceptual proposal - the
> implementation details might encounter issues, some of which are similar
> to the issues overlays face.)

That is what I've ended up doing for the eeprom. It means I only have to 
juggle one overlay.

Having all the possible nodes is doable but it becomes as complicated as 
manually instantiating the devices, either way you have to know about 
what devices are there and where they fit on the base platform.

>>>> The first issue is that I need to add -@ to the DTC_FLAGS for my board
>>>> dtb. I kind of understand that I only need -@ if my overlay targets
>>>> something symbolic so I might not need it but I was surprised that there
>>>> wasn't a Kconfig option that makes this happen automatically.
>>>>
>>>> externing things in C files makes checkpatch.pl complain. I see the
>>>> of/unittests.c and rcar_du_of.c hide this with a macro. I was again
>>>> surprised that there wasn't a common macro to declare these.
>>>
>>> unittests.c should not be used as a model of how to code for devicetree.
>>> There are some hacks that are needed to be able to test, but should not
>>> be used by normal drivers.
>>>
>>> The rcar use of overlays is a temporary hack to provide backward
>>> compatibility.  The intent is to drop this code once the backward
>>> compatibility window ends.
>>
>> Yes I suspected as much.
>>
>>> The long-term intent (once enough of the overlay code is in place) is
>>> to provide an overlay loader than can apply an overlay .dtb from a file.
>>
>> Would that be a kernel helper or driven via udev?
> 
> The implementation is TBD.  I would prefer to defer the implementation
> discussion until we are closer to being willing to accept a loader.
> 
> -Frank
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Using device tree overlays in Linux
  2019-04-09 19:19     ` Frank Rowand
@ 2019-04-09 20:54       ` Chris Packham
  0 siblings, 0 replies; 8+ messages in thread
From: Chris Packham @ 2019-04-09 20:54 UTC (permalink / raw)
  To: Frank Rowand, pantelis.antoniou, robh+dt
  Cc: devicetree, linux-kernel, Hamish Martin

On 10/04/19 7:19 AM, Frank Rowand wrote:
> On 4/7/19 6:27 PM, Chris Packham wrote:
>> Hi Frank,
>>
>> On 8/04/19 1:05 PM, Frank Rowand wrote:
>>> Hi Chris,
>>>
>>> On 4/3/19 6:50 PM, Chris Packham wrote:
>>>> Hi,
>>>>
>>>> I'm implementing support for some modular Linux based systems using
>>>> device tree overlays. The code is working but it seems a little more
>>>> fiddly that than it should be so I'm wondering if I'm doing it right.
>>>
>>> Let me start by saying that I strongly discourage using device tree
>>> overlays in the Linux kernel until the support is more baked.  For
>>> some info on how unbaked overlays are, see:
>>>
>>>      https://elinux.org/Frank%27s_Evolving_Overlay_Thoughts
>>>
>>> You should consider applying overlays in the Linux kernel to be
>>> fragile at best.
>>>
>>> If you can not figure out how to solve your needs without using
>>> overlays, then having the boot loader apply the overlay instead
>>> of the kernel applying the overlay avoids most of the issues.
>>>
>>
>> Consider us beta-testers :).
> 
> You should read the elinux.org page more carefully.  The support
> isn't even alpha stage.  It is at the level of proof of concept.
> 
> I'll update that page to explicitly say the code is proof of
> concept.

pre-alpha-testers then. We've actually been using the early overlay 
support for a while and it's suited our needs quite well. Aside from the 
internal API changes as we move to the latest kernel the current support 
also seems to meet our needs.

Both Hamish and I are quite keen to see this move forward so please 
don't hesitate to ask us if there are things we might be able to help 
with. Even if it's just trying something out on our hardware platforms.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-04-09 20:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-04  1:50 Using device tree overlays in Linux Chris Packham
2019-04-06  6:07 ` Rob Herring
2019-04-08  1:05 ` Frank Rowand
2019-04-08  1:27   ` Chris Packham
2019-04-09 19:19     ` Frank Rowand
2019-04-09 20:54       ` Chris Packham
2019-04-09 19:33     ` Frank Rowand
2019-04-09 20:47       ` Chris Packham

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).