introduce support for early platform drivers
cover introduce support for early platform drivers
This series is a follow-up to the RFC[1] posted a couple days ago.
NOTE: this series applies on top of my recent patches[2] that move the previous implementation of early platform devices to arch/sh.
Problem:
Certain class of devices, such as timers, certain clock drivers and irq chip drivers need to be probed early in the boot sequence. The currently preferred approach is using one of the OF_DECLARE() macros. This however does not create a platform device which has many drawbacks - such as not being able to use devres routines, dev_ log functions or no way of deferring the init OF function if some other resources are missing.
For drivers that use both platform drivers and OF_DECLARE the situation is even more complicated as the code needs to take into account that there can possibly be no struct device present. For a specific use case that we're having problems with, please refer to the recent DaVinci common-clock conversion patches and the nasty workaround that this problem implies[3].
We also used to have an early platform drivers implementation but they were not integrated with the linux device model at all - they merely used the same data structures. The users could not use devres, defer probe and the early devices never became actual platform devices later on.
Proposed solution:
This series aims at solving this problem by (re-)introducing the concept of early platform drivers and devices - this time however in a way that seamlessly integrates with the existing platform drivers and also offers device-tree support.
The idea is to provide a way for users to probe devices early, while already being able to use devres, devices resources and properties and also deferred probing.
New structures are introduced: the early platform driver contains the early_probe callback which has the same signature as regular platform_device probe. This callback is called early on. The user can have both the early and regular probe speficied or only one of them and they both receive the same platform device object as argument. Any device data allocated early will be carried over to the normal probe.
The architecture code is responsible for calling early_platform_start() in which the early drivers will be registered and devices populated from DT.
Once the device and kobject mechanisms are ready, all early drivers and devices will be converted into real platform drivers and devices. Also: if any of the early platform registration functions will be called once early initialization is done, these functions will work like regular platform_device/driver ones.
Patches 1-9/12 introduce changes to existing code that are necessary before adding support for early drivers. Patch 10/12 contains the new framwork in an isolated file which can be compiled only if needed by the architecture.
Patch 11/12 contains a dummy early platform driver that serves as a code example and can be used for simple testing.
The last patch finally makes the of/platform code aware of early platform drivers.
If accepted, this new mechanism could potentially lead to consolidation of the code currently used by users of OF_DECLARE, since they could be converted to actual platform drivers.
[1] https://lkml.org/lkml/2018/4/26/657
[2] https://lkml.org/lkml/2018/4/30/547
[3] https://lkml.org/lkml/2018/4/26/1094
Bartosz Golaszewski (12):
platform/early: add a new field to struct device platform/early: don't WARN() on non-empty devres list for early
devices
platform/early: export platform_match() locally
platform: provide a separate function for initializing platform
devices
platform: export platform_device_release() locally
of: add a new flag for OF device nodes
of/platform: provide a separate routine for setting up device
resources
of/platform: provide a separate routine for device initialization platform/early: add an init section for early driver data platform/early: implement support for early platform drivers
misc: implement a dummy early platform driver of/platform: make the OF code aware of early platform drivers
drivers/base/Kconfig | 3 + drivers/base/Makefile | 1 + drivers/base/base.h | 4 + drivers/base/dd.c | 2 +- drivers/base/early.c | 332 ++++++++++++++++++++++++++++++ drivers/base/platform.c | 28 ++- drivers/misc/Kconfig | 8 + drivers/misc/Makefile | 2 + drivers/misc/dummy-early.c | 82 ++++++++ drivers/of/platform.c | 85 +++++--- include/asm-generic/vmlinux.lds.h | 11 + include/linux/device.h | 1 + include/linux/early_platform.h | 75 +++++++ include/linux/of.h | 1 + include/linux/of_platform.h | 2 + include/linux/platform_device.h | 2 + 16 files changed, 604 insertions(+), 35 deletions(-) create mode 100644 drivers/base/early.c create mode 100644 drivers/misc/dummy-early.c create mode 100644 include/linux/early_platform.h
Signature · 1 line
2.17.0
PATCH 1/12 platform/early: add a new field to struct device
From: Bartosz Golaszewski <[email protected]>
Add a new single-bit field to struct device which will be used to indicate that a device was probed early in the boot sequence.
This does not affect the size of struct device on any architecture I built on (ARM, ARM64, x86_64).
--- include/linux/device.h | 1 + 1 file changed, 1 insertion(+)
include/linux/device.h +1 -0
@@ -992,6 +992,7 @@ struct device { bool offline_disabled:1; bool offline:1; bool of_node_reused:1; + bool early:1; };static inline struct device *kobj_to_dev (struct kobject *kobj)
Signature · 1 line
2.17.0
Re: [PATCH 01/12] platform/early: add a new field to struct device
Quoted 9 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Add a new single-bit field to struct device which will be used to indicate that a device was probed early in the boot sequence.
This does not affect the size of struct device on any architecture I built on (ARM, ARM64,
x86_64).Signed-off-by Bartosz Golaszewski <[email protected]>
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
PATCH 2/12 platform/early: don't WARN() on non-empty devres list for early devices
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Early platform devices can have devres objects allocated in early_probe(). This is not a bug so don't dump stack in this case.
--- drivers/base/dd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/base/dd.c +1 -1
@@ -410,7 +410,7 @@ static int really_probe(struct device *dev, struct device_driver *drv)atomic_inc (&probe_count);pr_debug ("bus: '%s': %s: probing driver %s with device %s\n" , drv->bus->name, __func__, drv->name,dev_name (dev)); -WARN_ON (!list_empty (&dev->devres_head)); +WARN_ON (!dev->early && !list_empty (&dev->devres_head)); re_probe: dev->driver = drv;
Signature · 1 line
2.17.0
Re: [PATCH 02/12] platform/early: don't WARN() on non-empty devres list for early devices
Quoted 6 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Early platform devices can have devres objects allocated in
early_probe(). This is not a bug so don't dump stack in this case.Signed-off-by Bartosz Golaszewski <[email protected]>
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
PATCH 3/12 platform/early: export platform_match() locally
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
We will use this function to match devices in the early platform core code. Export it locally in drivers/base.
--- drivers/base/base.h | 3 +++ drivers/base/platform.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
drivers/base/base.h +3 -0
@@ -161,3 +161,6 @@ extern void device_links_driver_cleanup(struct device *dev);extern void device_links_no_driver (struct device *dev);extern bool device_links_busy (struct device *dev);extern void device_links_unbind_consumers (struct device *dev); + +/* Platform device helpers. */ +extern int platform_match (struct device *dev,struct device_driver *drv);
drivers/base/platform.c +1 -1
@@ -961,7 +961,7 @@ static const struct platform_device_id *platform_match_id(* and compare it against the name of the driver. Return whether they match * or not. */ -static int platform_match (struct device *dev,struct device_driver *drv) +int platform_match (struct device *dev,struct device_driver *drv) {struct platform_device *pdev =to_platform_device (dev);struct platform_driver *pdrv =to_platform_driver (drv);
Signature · 1 line
2.17.0
Re: [PATCH 03/12] platform/early: export platform_match() locally
Quoted 6 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
We will use this function to match devices in the early platform core code. Export it locally in
drivers/base.Signed-off-by Bartosz Golaszewski <[email protected]>
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
PATCH 4/12 platform: provide a separate function for initializing platform devices
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
The early platform driver framework will allocate platform devices using its own internal mechanisms. Provide a function that allows to initialize a platform device object without allocating it.
--- drivers/base/platform.c | 24 ++++++++++++++++++++---- include/linux/platform_device.h | 2 ++ 2 files changed, 22 insertions(+), 4 deletions(-)
drivers/base/platform.c +20 -4
@@ -252,6 +252,25 @@ static void platform_device_release(struct device *dev)kfree (pa); } +/** +* platform_device_init - initialize a platform device +* @pdev: platform device to initialize +* @name: base name of the device we're adding, NOTE: it's not copied +* @id: instance id +* +* Initiate an already allocated platform device. This routine does not set +* the release device callback. +*/ +void platform_device_init (struct platform_device *pdev, +const char *name,int id) +{ + pdev->name = name; + pdev->id = id; +device_initialize (&pdev->dev); +arch_setup_pdev_archdata (pdev); +} +EXPORT_SYMBOL_GPL (platform_device_init); +/** * platform_device_alloc - create a platform device * @name: base name of the device we're adding
@@ -267,11 +286,8 @@ struct platform_device *platform_device_alloc(const char *name, int id) pa =kzalloc (sizeof (*pa) +strlen (name) +1 ,GFP_KERNEL );if (pa) {strcpy (pa->name, name); - pa->pdev.name = pa->name; - pa->pdev.id = id; -device_initialize (&pa->pdev.dev); +platform_device_init (&pa->pdev, pa->name, id); pa->pdev.dev.release = platform_device_release; -arch_setup_pdev_archdata (&pa->pdev); }return pa ? &pa->pdev :NULL ;
include/linux/platform_device.h +2 -0
@@ -165,6 +165,8 @@ static inline struct platform_device *platform_device_register_data( NULL, 0, data, size); } +extern void platform_device_init (struct platform_device *pdev, +const char *name,int id);extern struct platform_device *platform_device_alloc (const char *name,int id); extern int platform_device_add_resources(struct platform_device *pdev, const struct resource *res,
Signature · 1 line
2.17.0
Re: [PATCH 04/12] platform: provide a separate function for initializing platform devices
Quoted 7 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
The early platform driver framework will allocate platform devices using its own internal mechanisms. Provide a function that allows to initialize a platform device object without allocating it.
Signed-off-by Bartosz Golaszewski <[email protected]>
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
PATCH 5/12 platform: export platform_device_release() locally
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
The early platform driver mechanism will use this function as the device release callback. Make it available in drivers/base.
--- drivers/base/base.h | 1 + drivers/base/platform.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
drivers/base/base.h +1 -0
@@ -164,3 +164,4 @@ extern void device_links_unbind_consumers(struct device *dev);/* Platform device helpers. */ extern int platform_match (struct device *dev,struct device_driver *drv); +extern void platform_device_release (struct device *dev);
drivers/base/platform.c +1 -1
@@ -239,7 +239,7 @@ void platform_device_put(struct platform_device *pdev) }EXPORT_SYMBOL_GPL (platform_device_put); -static void platform_device_release (struct device *dev) +void platform_device_release (struct device *dev) { struct platform_object *pa = container_of(dev, struct platform_object, pdev.dev);
Signature · 1 line
2.17.0
Re: [PATCH 05/12] platform: export platform_device_release() locally
Quoted 6 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
The early platform driver mechanism will use this function as the device release callback. Make it available in
drivers/base.Signed-off-by Bartosz Golaszewski <[email protected]>
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
PATCH 6/12 of: add a new flag for OF device nodes
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
This flag indicates that a device node has been populated early and that it needs different handling when called from of_platform_populate().
--- include/linux/of.h | 1 + 1 file changed, 1 insertion(+)
include/linux/of.h +1 -0
@@ -143,6 +143,7 @@ extern raw_spinlock_t devtree_lock;#define OF_DETACHED 2/* node has been detached from the device tree */ #define OF_POPULATED 3/* device already created for the node */ #define OF_POPULATED_BUS 4/* of_platform_populate recursed to children of this node */ +#define OF_POPULATED_EARLY 5/* device is already allocated */ #define OF_BAD_ADDR ((u64)-1)
Signature · 1 line
2.17.0
Re: [PATCH 06/12] of: add a new flag for OF device nodes
Quoted 7 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
This flag indicates that a device node has been populated early and that it needs different handling when called from
of_platform_populate().Signed-off-by Bartosz Golaszewski <[email protected]>
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
PATCH 7/12 of/platform: provide a separate routine for setting up device resources
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
The early platform driver framework will need to setup the device resources before the regular populating of the device tree happens.
Provide a separate function for that.
--- drivers/of/platform.c | 54 +++++++++++++++++++++++-------------- include/linux/of_platform.h | 2 ++ 2 files changed, 36 insertions(+), 20 deletions(-)
drivers/of/platform.c +34 -20
@@ -99,24 +99,12 @@ static void of_device_make_bus_id(struct device *dev) } } -/** -* of_device_alloc - Allocate and initialize an of_device -* @np: device node to assign to device -* @bus_id: Name to assign to the device. May be null to use default name. -* @parent: Parent device. -*/ -struct platform_device *of_device_alloc (struct device_node *np, -const char *bus_id, -struct device *parent) +int of_device_init_resources (struct platform_device *pdev, +struct device_node *np) { -struct platform_device *dev;int rc, i, num_reg =0 , num_irq;struct resource *res, temp_res; - dev =platform_device_alloc ("" ,PLATFORM_DEVID_NONE ); -if (!dev) -return NULL ; -/* count the io and irq resources */ while (of_address_to_resource (np, num_reg, &temp_res) ==0 ) num_reg++;
@@ -125,22 +113,48 @@ struct platform_device *of_device_alloc(struct device_node *np,/* Populate the resource table */ if (num_irq || num_reg) { res =kzalloc (sizeof (*res) * (num_irq + num_reg),GFP_KERNEL ); -if (!res) { -platform_device_put (dev); -return NULL ; - } +if (!res) +return -ENOMEM ; + + pdev->num_resources = num_reg + num_irq; + pdev->resource = res; - dev->num_resources = num_reg + num_irq; - dev->resource = res;for (i =0 ; i < num_reg; i++, res++) { rc =of_address_to_resource (np, i, res);WARN_ON (rc); } +if (of_irq_to_resource_table (np, res, num_irq) != num_irq)pr_debug ("not all legacy IRQ resources mapped for %s\n" , np->name); } +return 0 ; +} + +/** +* of_device_alloc - Allocate and initialize an of_device +* @np: device node to assign to device +* @bus_id: Name to assign to the device. May be null to use default name. +* @parent: Parent device. +*/ +struct platform_device *of_device_alloc (struct device_node *np, +const char *bus_id, +struct device *parent) +{ +struct platform_device *dev; +int rc; + + dev =platform_device_alloc ("" ,PLATFORM_DEVID_NONE ); +if (!dev) +return NULL ; + + rc =of_device_init_resources (dev, np); +if (rc) { +platform_device_put (dev); +return NULL ; + } + dev->dev.of_node =of_node_get (np); dev->dev.fwnode = &np->fwnode; dev->dev.parent = parent ? : &platform_bus;
include/linux/of_platform.h +2 -0
@@ -52,6 +52,8 @@ extern const struct of_device_id of_default_bus_match_table[];extern struct platform_device *of_device_alloc (struct device_node *np,const char *bus_id,struct device *parent); +extern int of_device_init_resources (struct platform_device *pdev, +struct device_node *np);#ifdef CONFIG_OF extern struct platform_device *of_find_device_by_node (struct device_node *np);#else
Signature · 1 line
2.17.0
Re: [PATCH 07/12] of/platform: provide a separate routine for setting up device resources
Quoted 8 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
The early platform driver framework will need to setup the device resources before the regular populating of the device tree happens.
Provide a separate function for that.
Signed-off-by Bartosz Golaszewski <[email protected]>
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
PATCH 8/12 of/platform: provide a separate routine for device initialization
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
The early platform device framework will need to initialize the platform device objects without them being allocated in of_device_alloc(). Provide a routine that allows it.
--- drivers/of/platform.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)
drivers/of/platform.c +14 -8
@@ -132,6 +132,19 @@ int of_device_init_resources(struct platform_device *pdev,return 0 ; } +static void of_device_init (struct platform_device *pdev,struct device_node *np, +const char *bus_id,struct device *parent) +{ + pdev->dev.of_node =of_node_get (np); + pdev->dev.fwnode = &np->fwnode; + pdev->dev.parent = parent ? : &platform_bus; + +if (bus_id) +dev_set_name (&pdev->dev,"%s" , bus_id); +else +of_device_make_bus_id (&pdev->dev); +} +/** * of_device_alloc - Allocate and initialize an of_device * @np: device node to assign to device
@@ -155,14 +168,7 @@ struct platform_device *of_device_alloc(struct device_node *np,return NULL ; } - dev->dev.of_node =of_node_get (np); - dev->dev.fwnode = &np->fwnode; - dev->dev.parent = parent ? : &platform_bus; - -if (bus_id) -dev_set_name (&dev->dev,"%s" , bus_id); -else -of_device_make_bus_id (&dev->dev); +of_device_init (dev, np, bus_id, parent);return dev; }
Signature · 1 line
2.17.0
Re: [PATCH 08/12] of/platform: provide a separate routine for device initialization
Quoted 7 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
The early platform device framework will need to initialize the platform device objects without them being allocated in
of_device_alloc(). Provide a routine that allows it.Signed-off-by Bartosz Golaszewski <[email protected]>
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
PATCH 9/12 platform/early: add an init section for early driver data
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Provide a separate section in which pointers to early platform driver structs will be stored.
--- include/asm-generic/vmlinux.lds.h | 11 +++++++++++ 1 file changed, 11 insertions(+)
include/asm-generic/vmlinux.lds.h +11 -0
@@ -214,6 +214,16 @@#define CPU_METHOD_OF_TABLES () OF_TABLE(CONFIG_SMP, cpu_method)#define CPUIDLE_METHOD_OF_TABLES () OF_TABLE(CONFIG_CPU_IDLE, cpuidle_method) +#ifdef CONFIG_EARLY_PLATFORM +#define EARLY_PLATFORM_DRIVERS_TABLE () \ + . = ALIGN(8); \ + VMLINUX_SYMBOL(__early_platform_drivers_table) = .; \ + KEEP(*(__early_platform_drivers_table)) \ + VMLINUX_SYMBOL(__early_platform_drivers_table_end) = .; +#else +#define EARLY_PLATFORM_DRIVERS_TABLE () +#endif +#ifdef CONFIG_ACPI #define ACPI_PROBE_TABLE (name) \ . = ALIGN(8); \
@@ -593,6 +603,7 @@ RESERVEDMEM_OF_TABLES() \ TIMER_OF_TABLES() \ IOMMU_OF_TABLES() \ + EARLY_PLATFORM_DRIVERS_TABLE() \ CPU_METHOD_OF_TABLES() \ CPUIDLE_METHOD_OF_TABLES() \KERNEL_DTB () \
Signature · 1 line
2.17.0
Re: [PATCH 09/12] platform/early: add an init section for early driver data
Hi Bartosz,
Quoted 6 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Provide a separate section in which pointers to early platform driver structs will be stored.
Signed-off-by Bartosz Golaszewski <[email protected]>
Thanks for your patch!
include/asm-generic/vmlinux.lds.h line 219
Quoted 9 lines
include/asm-generic/vmlinux.lds.h +3
-0@@ -214,6 +214,16 @@#define CPU_METHOD_OF_TABLES () OF_TABLE(CONFIG_SMP, cpu_method)#define CPUIDLE_METHOD_OF_TABLES () OF_TABLE(CONFIG_CPU_IDLE, cpuidle_method) +#ifdef CONFIG_EARLY_PLATFORM +#define EARLY_PLATFORM_DRIVERS_TABLE () \ + . = ALIGN(8); \
Should this use STRUCT_ALIGN() instead?
include/asm-generic/vmlinux.lds.h line 225
Quoted 6 lines
include/asm-generic/vmlinux.lds.h +6
-0+ VMLINUX_SYMBOL(__early_platform_drivers_table) = .; \ + KEEP(*(__early_platform_drivers_table)) \ + VMLINUX_SYMBOL(__early_platform_drivers_table_end) = .; +#else +#define EARLY_PLATFORM_DRIVERS_TABLE () +#endif
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 09/12] platform/early: add an init section for early driver data
2018-05-14 23:29 GMT+02:00 Geert Uytterhoeven <[email protected]>:
Quoted 18 lines
Hi Bartosz,
Quoted 6 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Provide a separate section in which pointers to early platform driver structs will be stored.
Signed-off-by Bartosz Golaszewski <[email protected]>Thanks for your patch!
Quoted 6 lines
include/asm-generic/vmlinux.lds.h +0
-0@@ -214,6 +214,16 @@#define CPU_METHOD_OF_TABLES () OF_TABLE(CONFIG_SMP, cpu_method)#define CPUIDLE_METHOD_OF_TABLES () OF_TABLE(CONFIG_CPU_IDLE, cpuidle_method)
Quoted 6 lines
include/asm-generic/vmlinux.lds.h line 219
Quoted 3 lines
include/asm-generic/vmlinux.lds.h +3
-0+#ifdef CONFIG_EARLY_PLATFORM +#define EARLY_PLATFORM_DRIVERS_TABLE () \ + . = ALIGN(8); \Should this use
STRUCT_ALIGN()instead?
No, we're only using it to store pointers to structs, not the actual struct early_platform_driver objects.
Quoted 9 lines
include/asm-generic/vmlinux.lds.h line 225
Quoted 6 lines
include/asm-generic/vmlinux.lds.h +6
-0+ VMLINUX_SYMBOL(__early_platform_drivers_table) = .; \ + KEEP(*(__early_platform_drivers_table)) \ + VMLINUX_SYMBOL(__early_platform_drivers_table_end) = .; +#else +#define EARLY_PLATFORM_DRIVERS_TABLE () +#endif Gr{oetje,eeting}s,
Quoted 8 lines
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Thanks,
Bartosz
PATCH 10/12 platform/early: implement support for early platform drivers
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
This introduces the core part of support for early platform drivers and devices.
--- drivers/base/Kconfig | 3 + drivers/base/Makefile | 1 + drivers/base/early.c | 332 +++++++++++++++++++++++++++++++++ drivers/misc/Makefile | 1 + include/linux/early_platform.h | 75 ++++++++ 5 files changed, 412 insertions(+) create mode 100644 drivers/base/early.c create mode 100644 include/linux/early_platform.h
drivers/base/Kconfig +3 -0
@@ -205,6 +205,9 @@ config DEBUG_TEST_DRIVER_REMOVE unusable. You should say N here unless you are explicitly looking to test this functionality. +config EARLY_PLATFORM + def_bool n + source "drivers/base/test/Kconfig" config SYS_HYPERVISOR
drivers/base/Makefile +1 -0
@@ -7,6 +7,7 @@ obj-y := component.o core.o bus.o dd.o syscore.o \ attribute_container.o transport_class.o \ topology.o container.o property.o cacheinfo.o \ devcon.o +obj-$(CONFIG_EARLY_PLATFORM) += early.o obj-$(CONFIG_DEVTMPFS) += devtmpfs.o obj-$(CONFIG_DMA_CMA) += dma-contiguous.o obj-y += power/
drivers/base/early.c new file +332 -0
332 lines not on this page · raw message
drivers/misc/Makefile +1 -0
@@ -57,3 +57,4 @@ obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o obj-$(CONFIG_OCXL) += ocxl/ obj-$(CONFIG_MISC_RTSX) += cardreader/ +CFLAGS_dummy-early.o := -DDEBUG
include/linux/early_platform.h new file +75 -0
75 lines not on this page · raw message
Signature · 1 line
2.17.0
Re: [PATCH 10/12] platform/early: implement support for early platform drivers
Quoted 5 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
This introduces the core part of support for early platform drivers and devices.
It looks like most of your prep patches are to separate the alloc and init of platform devices because you are essentially making early devices/drivers a sub-class. Maybe you could avoid doing that and simplify things a bit. Comments below based on doing that...
include/linux/early_platform.h line 24
Quoted 10 lines
include/linux/early_platform.h new file +10
-0+/** +* struct early_platform_driver +* +* @pdrv: real platform driver associated with this early platform driver +* @list: list head for the list of early platform drivers +* @early_probe: early probe callback +*/ +struct early_platform_driver { +struct platform_driver pdrv; +struct list_head list;
Couldn't you use an existing list in driver_private until you move over to the normal bus infra.
Quoted 1 line
+ int (*early_probe)(struct
platform_device*);
Just add this to platform_driver.
include/linux/early_platform.h line 38
Quoted 13 lines
include/linux/early_platform.h new file +13
-0+}; + +/** +* struct early_platform_device +* +* @pdev: real platform device associated with this early platform device +* @list: list head for the list of early platform devices +* @deferred: true if this device's early probe was deferred +* @deferred_drv: early platform driver with which this device was matched +*/ +struct early_platform_device { +struct platform_device pdev; +struct list_head list;
Use a list in device_private?
include/linux/early_platform.h line 40
Quoted 2 lines
include/linux/early_platform.h new file +2
-0+bool deferred; +struct early_platform_driver *deferred_drv;
Can't you use the existing deferred probe list?
Rob
Re: [PATCH 10/12] platform/early: implement support for early platform drivers
2018-05-14 15:37 GMT+02:00 Rob Herring <[email protected]>:
Quoted 12 lines · Bartosz Golaszewski
Quoted 5 lines
From: Bartosz Golaszewski <[email protected]>
This introduces the core part of support for early platform drivers and devices.
It looks like most of your prep patches are to separate the alloc and init of platform devices because you are essentially making early devices/drivers a sub-class. Maybe you could avoid doing that and simplify things a bit. Comments below based on doing that...
My aim was to change as little as possible for everybody else while fixing our problem. These changes are already controversial enough without risky reusing of existing fields in common structures. I was just afraid that there are too many intricacies for it to be safe.
Quoted 5 lines
Quoted 5 lines
+/**
+ * structearly_platform_driver
+ *
+ * @pdrv: real platform driver associated with this early platform driver
+ * @list: list head for the list of early platform drivers
Quoted 9 lines
include/linux/early_platform.h line 24
Quoted 5 lines
+ * @early_probe: early probe callback
+ */include/linux/early_platform.h new file +3
-0+struct early_platform_driver { +struct platform_driver pdrv; +struct list_head list;Couldn't you use an existing list in
driver_privateuntil you move over to the normal bus infra.
This is something that the previous implementation did. It was quite unreadable, so I decided to go with a separate list.
Quoted 4 lines
Quoted 1 line
+ int (*early_probe)(struct
platform_device*);Just add this to
platform_driver.
This would extend the structure for everybody else while there'll be very few such devices and not all systems would even require it.
Quoted 16 lines
include/linux/early_platform.h line 38
Quoted 13 lines
include/linux/early_platform.h new file +13
-0+}; + +/** +* struct early_platform_device +* +* @pdev: real platform device associated with this early platform device +* @list: list head for the list of early platform devices +* @deferred: true if this device's early probe was deferred +* @deferred_drv: early platform driver with which this device was matched +*/ +struct early_platform_device { +struct platform_device pdev; +struct list_head list;Use a list in
device_private?
Quoted 5 lines
include/linux/early_platform.h line 40
Quoted 2 lines
include/linux/early_platform.h new file +2
-0+bool deferred; +struct early_platform_driver *deferred_drv;Can't you use the existing deferred probe list?
I thought about it, but I was afraid there could be some timing issues with that and decided against it. The early deferral also doesn't work in a workque, but is synchronous instead.
Best regards,
Bartosz Golaszewski
Re: [PATCH 10/12] platform/early: implement support for early platform drivers
Quoted 14 lines · Bartosz Golaszewski
2018-05-14 15:37 GMT+02:00 Rob Herring <[email protected]>:
Quoted 12 lines · Bartosz Golaszewski
Quoted 5 lines
From: Bartosz Golaszewski <[email protected]>
This introduces the core part of support for early platform drivers and devices.
It looks like most of your prep patches are to separate the alloc and init of platform devices because you are essentially making early devices/drivers a sub-class. Maybe you could avoid doing that and simplify things a bit. Comments below based on doing that...
Quoted 4 lines
My aim was to change as little as possible for everybody else while fixing our problem. These changes are already controversial enough without risky reusing of existing fields in common structures. I was just afraid that there are too many intricacies for it to be safe.
I don't think those intricacies would go away just by having separate fields. Perhaps it would make things fail more explicitly. After all, I think it needs to be a very atomic operation when a device is switched.
Quoted 45 lines
Quoted 5 lines
Quoted 5 lines
+/**
+ * structearly_platform_driver
+ *
+ * @pdrv: real platform driver associated with this early platform driver
+ * @list: list head for the list of early platform driversQuoted 9 lines
include/linux/early_platform.h line 24
Quoted 5 lines
+ * @early_probe: early probe callback
+ */include/linux/early_platform.h new file +3
-0+struct early_platform_driver { +struct platform_driver pdrv; +struct list_head list;Couldn't you use an existing list in
driver_privateuntil you move over to the normal bus infra.This is something that the previous implementation did. It was quite unreadable, so I decided to go with a separate list.
Quoted 4 lines
Quoted 1 line
+ int (*early_probe)(struct
platform_device*);Just add this to
platform_driver.This would extend the structure for everybody else while there'll be very few such devices and not all systems would even require it.
Quoted 19 lines
include/linux/early_platform.h line 38
Quoted 13 lines
include/linux/early_platform.h new file +13
-0+}; + +/** +* struct early_platform_device +* +* @pdev: real platform device associated with this early platform device +* @list: list head for the list of early platform devices +* @deferred: true if this device's early probe was deferred +* @deferred_drv: early platform driver with which this device was matched +*/ +struct early_platform_device { +struct platform_device pdev; +struct list_head list;Use a list in
device_private?Quoted 2 lines
include/linux/early_platform.h new file +2
-0+bool deferred; +struct early_platform_driver *deferred_drv;
Quoted 6 lines
Quoted 2 lines
Can't you use the existing deferred probe list?
I thought about it, but I was afraid there could be some timing issues with that and decided against it. The early deferral also doesn't work in a workque, but is synchronous instead.
I didn't mean use the wq, but just the list fields. You'd still have the early list and normal list with different list heads. If you ever had a device wanting to be on both lists at the same time, then you've got major problems.
Rob
PATCH 11/12 misc: implement a dummy early platform driver
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Implement a very simple early platform driver. Its purpose is to show how such drivers can be registered and to emit a message when probed.
It can be then added to the device tree or machine code to verify that the early platform devices work as expected.
--- drivers/misc/Kconfig | 8 ++++ drivers/misc/Makefile | 1 + drivers/misc/dummy-early.c | 82 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 drivers/misc/dummy-early.c
drivers/misc/Kconfig +8 -0
@@ -91,6 +91,14 @@ config DUMMY_IRQ The sole purpose of this module is to help with debugging of systems on which spurious IRQs would happen on disabled IRQ vector. +config DUMMY_EARLY + bool "Dummy early platform driver" + select EARLY_PLATFORM + default n + help + This module's only function is to register itself with the early + platform device framework and be probed early in the boot process. + config IBM_ASM tristate "Device driver for IBM RSA service processor" depends on X86 && PCI && INPUT
drivers/misc/Makefile +1 -0
@@ -11,6 +11,7 @@ obj-$(CONFIG_INTEL_MID_PTI) += pti.o obj-$(CONFIG_ATMEL_SSC) += atmel-ssc.o obj-$(CONFIG_ATMEL_TCLIB) += atmel_tclib.o obj-$(CONFIG_DUMMY_IRQ) += dummy-irq.o +obj-$(CONFIG_DUMMY_EARLY) += dummy-early.o obj-$(CONFIG_ICS932S401) += ics932s401.o obj-$(CONFIG_LKDTM) += lkdtm/ obj-$(CONFIG_TIFM_CORE) += tifm_core.o
drivers/misc/dummy-early.c new file +82 -0
82 lines not on this page · raw message
Signature · 1 line
2.17.0
PATCH 12/12 of/platform: make the OF code aware of early platform drivers
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Check the relevant flag in the device node and skip the allocation part for devices that were populated early.
--- drivers/of/platform.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
drivers/of/platform.c +12 -3
@@ -21,6 +21,7 @@#include <linux/of_irq.h> #include <linux/of_platform.h> #include <linux/platform_device.h> +#include <linux/early_platform.h> const struct of_device_id of_default_bus_match_table[] = { { .compatible ="simple-bus" , },
@@ -196,9 +197,17 @@ static struct platform_device *of_platform_device_create_pdata( of_node_test_and_set_flag(np, OF_POPULATED))return NULL ; - dev =of_device_alloc (np, bus_id, parent); -if (!dev) - goto err_clear_flag; +if (of_node_check_flag (np,OF_POPULATED_EARLY )) { + dev =of_early_to_platform_device (np); +if (IS_ERR (dev)) + goto err_clear_flag; + +of_device_init (dev, np, bus_id, parent); + }else { + dev =of_device_alloc (np, bus_id, parent); +if (!dev) + goto err_clear_flag; + } dev->dev.bus = &platform_bus_type; dev->dev.platform_data = platform_data;
Signature · 1 line
2.17.0
Re: [PATCH 12/12] of/platform: make the OF code aware of early platform drivers
Hi Bartosz,
Quoted 6 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Check the relevant flag in the device node and skip the allocation part for devices that were populated early.
Signed-off-by Bartosz Golaszewski <[email protected]>
Thanks for your patch!
Quoted 11 lines
drivers/of/platform.c +1
-3@@ -196,9 +197,17 @@ static struct platform_device *of_platform_device_create_pdata( of_node_test_and_set_flag(np, OF_POPULATED))return NULL ; - dev =of_device_alloc (np, bus_id, parent); -if (!dev) - goto err_clear_flag; +if (of_node_check_flag (np,OF_POPULATED_EARLY )) {
drivers/of/platform.c static struct platform_device *of_platform_device_create_pdata( · line 210
Quoted 10 lines
drivers/of/platform.c +10
-0+ dev =of_early_to_platform_device (np); +if (IS_ERR (dev)) + goto err_clear_flag; + +of_device_init (dev, np, bus_id, parent); + }else { + dev =of_device_alloc (np, bus_id, parent); +if (!dev) + goto err_clear_flag; + }
The above may become cleaner if:
1. of_early_to_platform_device() would return NULL instead -ENOENT,
2. of_device_alloc() would be split in alloc and init phases, too.
Then you can do:
dev = of_node_check_flag(np, OF_POPULATED_EARLY)
? of_early_to_platform_device(np)
: __of_device_alloc(np, bus_id, parent);
if (!dev)
goto err_clear_flag;
of_device_init(dev, np, bus_id, parent);Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoted 5 lines · Bartosz Golaszewski
This series is a follow-up to the RFC[1] posted a couple days ago.
NOTE: this series applies on top of my recent patches[2] that move the previous implementation of early platform devices to
arch/sh.
Quoted 8 lines
Problem:
Certain class of devices, such as timers, certain clock drivers and irq chip drivers need to be probed early in the boot sequence. The currently preferred approach is using one of the
OF_DECLARE()macros. This however does not create a platform device which has many drawbacks - such as not being able to use devres routines,dev_log functions or no way of deferring the init OF function if some other resources are missing.
I skimmed though this and it doesn't look horrible (how's that for positive feedback? ;) ). But before going into the details, I think first there needs to be agreement this is the right direction.
The question does remain though as to whether this class of devices should be platform drivers. They can't be modules. They can't be hotplugged. Can they be runtime-pm enabled? So the advantage is ...
I assume that the clock maintainers had some reason to move clocks to be platform drivers. It's just not clear to me what that was.
Quoted 5 lines
For drivers that use both platform drivers and
OF_DECLAREthe situation is even more complicated as the code needs to take into account that there can possibly be no struct device present. For a specific use case that we're having problems with, please refer to the recent DaVinci common-clock conversion patches and the nasty workaround that this problem implies[3].
So devm_kzalloc will work with this solution? Why did we need devm_kzalloc in the first place? The clocks can never be removed and cleaning up on error paths is kind of pointless. The system would be hosed, right?
Quoted 23 lines
We also used to have an early platform drivers implementation but they were not integrated with the linux device model at all - they merely used the same data structures. The users could not use devres, defer probe and the early devices never became actual platform devices later on.
Proposed solution:
This series aims at solving this problem by (re-)introducing the concept of early platform drivers and devices - this time however in a way that seamlessly integrates with the existing platform drivers and also offers device-tree support.
The idea is to provide a way for users to probe devices early, while already being able to use devres, devices resources and properties and also deferred probing.
New structures are introduced: the early platform driver contains the
early_probecallback which has the same signature as regularplatform_deviceprobe. This callback is called early on. The user can have both the early and regular probe speficied or only one of them and they both receive the same platform device object as argument. Any device data allocated early will be carried over to the normal probe.
Quoted 2 lines
The architecture code is responsible for calling
early_platform_start()in which the early drivers will be registered and devices populated from DT.
Can we really do this in one spot for different devices (clk, timers, irq). The sequence is all very carefully crafted. Platform specific hooks is another thing to consider.
Quoted 4 lines
Once the device and kobject mechanisms are ready, all early drivers and devices will be converted into real platform drivers and devices. Also: if any of the early platform registration functions will be called once early initialization is done, these functions will work like regular platform_device/driver ones.
This could leave devices in a weird state. They've successfully probed early, but then are on the deferred list for normal probe for example.
Rob
2018-05-11 22:13 GMT+02:00 Rob Herring <[email protected]>:
Quoted 15 lines · Bartosz Golaszewski
Quoted 5 lines
This series is a follow-up to the RFC[1] posted a couple days ago.
NOTE: this series applies on top of my recent patches[2] that move the previous implementation of early platform devices to
arch/sh.Quoted 8 lines
Problem:
Certain class of devices, such as timers, certain clock drivers and irq chip drivers need to be probed early in the boot sequence. The currently preferred approach is using one of the
OF_DECLARE()macros. This however does not create a platform device which has many drawbacks - such as not being able to use devres routines,dev_log functions or no way of deferring the init OF function if some other resources are missing.
Quoted 8 lines
I skimmed though this and it doesn't look horrible (how's that for positive feedback? ;) ). But before going into the details, I think first there needs to be agreement this is the right direction.
The question does remain though as to whether this class of devices should be platform drivers. They can't be modules. They can't be hotplugged. Can they be runtime-pm enabled? So the advantage is ...
The main (but not the only) advantage for drivers that can both be platform drivers and OF_DECLARE drivers is that we get a single entry point and can reuse code without resorting to checking if (!dev). It results in more consistent code base. Another big advantage is consolidation of device tree and machine code for SoC drivers used in different boards of which some are still using board files and others are defined in DT (see: DaVinci).
Quoted 9 lines
I assume that the clock maintainers had some reason to move clocks to be platform drivers. It's just not clear to me what that was.
Quoted 5 lines
For drivers that use both platform drivers and
OF_DECLAREthe situation is even more complicated as the code needs to take into account that there can possibly be no struct device present. For a specific use case that we're having problems with, please refer to the recent DaVinci common-clock conversion patches and the nasty workaround that this problem implies[3].
Quoted 5 lines
So
devm_kzallocwill work with this solution? Why did we needdevm_kzallocin the first place? The clocks can never be removed and cleaning up on error paths is kind of pointless. The system would be hosed, right?
It depends - not all clocks are necessary for system to boot.
Quoted 23 lines
Quoted 23 lines
We also used to have an early platform drivers implementation but they were not integrated with the linux device model at all - they merely used the same data structures. The users could not use devres, defer probe and the early devices never became actual platform devices later on.
Proposed solution:
This series aims at solving this problem by (re-)introducing the concept of early platform drivers and devices - this time however in a way that seamlessly integrates with the existing platform drivers and also offers device-tree support.
The idea is to provide a way for users to probe devices early, while already being able to use devres, devices resources and properties and also deferred probing.
New structures are introduced: the early platform driver contains the
early_probecallback which has the same signature as regularplatform_deviceprobe. This callback is called early on. The user can have both the early and regular probe speficied or only one of them and they both receive the same platform device object as argument. Any device data allocated early will be carried over to the normal probe.
Quoted 7 lines
Quoted 2 lines
The architecture code is responsible for calling
early_platform_start()in which the early drivers will be registered and devices populated from DT.Can we really do this in one spot for different devices (clk, timers, irq). The sequence is all very carefully crafted. Platform specific hooks is another thing to consider.
This is why I added support for early probe deferral - so that we can stop caring for the order as long as the drivers are aware of other resources they need and we call early_platform_start() the moment the earliest of the early devices is needed.
Quoted 8 lines
Quoted 4 lines
Once the device and kobject mechanisms are ready, all early drivers and devices will be converted into real platform drivers and devices. Also: if any of the early platform registration functions will be called once early initialization is done, these functions will work like regular platform_device/driver ones.
This could leave devices in a weird state. They've successfully probed early, but then are on the deferred list for normal probe for example.
It's not really a weird state - once we're past the 'early' stage, the drivers' "earlyness" no longer plays any role. An early deferred device wouldn't be any different from other deferred devices. It's possible that we enabled some crucial resources in the early stage and then failed to fully probe the device later. I don't see a problem with that.
Best regards,
Bartosz Golaszewski
Quoted 42 lines · Bartosz Golaszewski
2018-05-11 22:13 GMT+02:00 Rob Herring <[email protected]>:
Quoted 15 lines · Bartosz Golaszewski
Quoted 5 lines
This series is a follow-up to the RFC[1] posted a couple days ago.
NOTE: this series applies on top of my recent patches[2] that move the previous implementation of early platform devices to
arch/sh.Quoted 8 lines
Problem:
Certain class of devices, such as timers, certain clock drivers and irq chip drivers need to be probed early in the boot sequence. The currently preferred approach is using one of the
OF_DECLARE()macros. This however does not create a platform device which has many drawbacks - such as not being able to use devres routines,dev_log functions or no way of deferring the init OF function if some other resources are missing.Quoted 8 lines
I skimmed though this and it doesn't look horrible (how's that for positive feedback? ;) ). But before going into the details, I think first there needs to be agreement this is the right direction.
The question does remain though as to whether this class of devices should be platform drivers. They can't be modules. They can't be hotplugged. Can they be runtime-pm enabled? So the advantage is ...
The main (but not the only) advantage for drivers that can both be platform drivers and
OF_DECLAREdrivers is that we get a single entry point and can reuse code without resorting to checking if (!dev). It results in more consistent code base. Another big advantage is consolidation of device tree and machine code for SoC drivers used in different boards of which some are still using board files and others are defined in DT (see: DaVinci).Quoted 9 lines
I assume that the clock maintainers had some reason to move clocks to be platform drivers. It's just not clear to me what that was.
Quoted 5 lines
For drivers that use both platform drivers and
OF_DECLAREthe situation is even more complicated as the code needs to take into account that there can possibly be no struct device present. For a specific use case that we're having problems with, please refer to the recent DaVinci common-clock conversion patches and the nasty workaround that this problem implies[3].
Quoted 7 lines
Quoted 5 lines
So
devm_kzallocwill work with this solution? Why did we needdevm_kzallocin the first place? The clocks can never be removed and cleaning up on error paths is kind of pointless. The system would be hosed, right?It depends - not all clocks are necessary for system to boot.
That doesn't matter. You have a single driver for all/most of the clocks, so the driver can't be removed.
Quoted 31 lines
Quoted 23 lines
Quoted 23 lines
We also used to have an early platform drivers implementation but they were not integrated with the linux device model at all - they merely used the same data structures. The users could not use devres, defer probe and the early devices never became actual platform devices later on.
Proposed solution:
This series aims at solving this problem by (re-)introducing the concept of early platform drivers and devices - this time however in a way that seamlessly integrates with the existing platform drivers and also offers device-tree support.
The idea is to provide a way for users to probe devices early, while already being able to use devres, devices resources and properties and also deferred probing.
New structures are introduced: the early platform driver contains the
early_probecallback which has the same signature as regularplatform_deviceprobe. This callback is called early on. The user can have both the early and regular probe speficied or only one of them and they both receive the same platform device object as argument. Any device data allocated early will be carried over to the normal probe.Quoted 7 lines
Quoted 2 lines
The architecture code is responsible for calling
early_platform_start()in which the early drivers will be registered and devices populated from DT.Can we really do this in one spot for different devices (clk, timers, irq). The sequence is all very carefully crafted. Platform specific hooks is another thing to consider.
Quoted 4 lines
This is why I added support for early probe deferral - so that we can stop caring for the order as long as the drivers are aware of other resources they need and we call
early_platform_start()the moment the earliest of the early devices is needed.
Deferred probe helps for inter-device dependencies, but I am more concerned about timing of trying to register clocksources, irqchips, etc. What happens if we probe drivers before the infrastructure is initialized?
Rob
Hi Rob,
Quoting Rob Herring (2018-05-14 06:20:57)
Quoted 49 lines · Bartosz Golaszewski
Quoted 48 lines
2018-05-11 22:13 GMT+02:00 Rob Herring <[email protected]>:
Quoted 15 lines · Bartosz Golaszewski
Quoted 5 lines
This series is a follow-up to the RFC[1] posted a couple days ago.
NOTE: this series applies on top of my recent patches[2] that move the previous implementation of early platform devices to
arch/sh.Quoted 8 lines
Problem:
Certain class of devices, such as timers, certain clock drivers and irq chip drivers need to be probed early in the boot sequence. The currently preferred approach is using one of the
OF_DECLARE()macros. This however does not create a platform device which has many drawbacks - such as not being able to use devres routines,dev_log functions or no way of deferring the init OF function if some other resources are missing.Quoted 8 lines
I skimmed though this and it doesn't look horrible (how's that for positive feedback? ;) ). But before going into the details, I think first there needs to be agreement this is the right direction.
The question does remain though as to whether this class of devices should be platform drivers. They can't be modules. They can't be hotplugged. Can they be runtime-pm enabled? So the advantage is ...
The main (but not the only) advantage for drivers that can both be platform drivers and
OF_DECLAREdrivers is that we get a single entry point and can reuse code without resorting to checking if (!dev). It results in more consistent code base. Another big advantage is consolidation of device tree and machine code for SoC drivers used in different boards of which some are still using board files and others are defined in DT (see: DaVinci).Quoted 14 lines
I assume that the clock maintainers had some reason to move clocks to be platform drivers. It's just not clear to me what that was.
Quoted 5 lines
For drivers that use both platform drivers and
OF_DECLAREthe situation is even more complicated as the code needs to take into account that there can possibly be no struct device present. For a specific use case that we're having problems with, please refer to the recent DaVinci common-clock conversion patches and the nasty workaround that this problem implies[3].So
devm_kzallocwill work with this solution? Why did we needdevm_kzallocin the first place? The clocks can never be removed and cleaning up on error paths is kind of pointless. The system would be hosed, right?
Quoted 4 lines
Quoted 1 line
It depends - not all clocks are necessary for system to boot.
That doesn't matter. You have a single driver for all/most of the clocks, so the driver can't be removed.
-ECANOFWORMS
A couple of quick rebuttals, but I imagine we're going to disagree on the platform_driver thing as a matter of taste no matter what...
1) There should be multiple clk drivers in a properly modeled system. Some folks still incorrectly put all clocks in a system into a single driver because That's How We Used To Do It, and some systems (simpler ones) really only have a single clock generator IP block.
Excepting those two reasons above, we really should have separate drivers for clocks controlled by the PMIC, for the (one or more) clock generator blocks inside of the AP/SoC, and then even more for the drivers that map to IP blocks that have their own clock gens.
Good examples of the latter are display controllers that generate their own PLL and pixel clock. Or MMC controllers that have a runtime-programmable clock divider. Examples of these are merged into mainline.
2) Stephen and I wanted clock drivers to actually be represented in the driver model. There were these gigantic clock drivers that exclusively used CLK_OF_DECLARE and they just sort of floated out there in the ether... no representation in sysfs, no struct device to map onto a clock controller struct, etc.
I'd be happy to hear why you think that platform_driver is a bad fit for a device driver that generally manages memory-mapped system resources that are part of the system glue and not really tied to a specific bus. Sounds like a good fit to me.
If platform_driver doesn't handle the early boot thing well, that tells me that we have a problem to solve in platform_driver, not in the clk subsystem or drivers.
3) Lots of clock controllers should be loadable modules. E.g. i2c clock expanders, potentially external PMIC-related drivers, external audio codecs, etc.
Again, repeating my point #1 above, just because many platforms have a monolithic clock driver does not mean that this is the Right Way to do it.
Best regards,
Mike
Quoted 43 lines
Quoted 32 lines
Quoted 24 lines
Quoted 24 lines
We also used to have an early platform drivers implementation but they were not integrated with the linux device model at all - they merely used the same data structures. The users could not use devres, defer probe and the early devices never became actual platform devices later on.
Proposed solution:
This series aims at solving this problem by (re-)introducing the concept of early platform drivers and devices - this time however in a way that seamlessly integrates with the existing platform drivers and also offers device-tree support.
The idea is to provide a way for users to probe devices early, while already being able to use devres, devices resources and properties and also deferred probing.
New structures are introduced: the early platform driver contains the
early_probecallback which has the same signature as regularplatform_deviceprobe. This callback is called early on. The user can have both the early and regular probe speficied or only one of them and they both receive the same platform device object as argument. Any device data allocated early will be carried over to the normal probe.Quoted 7 lines
Quoted 2 lines
The architecture code is responsible for calling
early_platform_start()in which the early drivers will be registered and devices populated from DT.Can we really do this in one spot for different devices (clk, timers, irq). The sequence is all very carefully crafted. Platform specific hooks is another thing to consider.
Quoted 4 lines
This is why I added support for early probe deferral - so that we can stop caring for the order as long as the drivers are aware of other resources they need and we call
early_platform_start()the moment the earliest of the early devices is needed.Deferred probe helps for inter-device dependencies, but I am more concerned about timing of trying to register clocksources, irqchips, etc. What happens if we probe drivers before the infrastructure is initialized?
Rob
Quoted 54 lines · Michael Turquette
Hi Rob,
Quoting Rob Herring (2018-05-14 06:20:57)
Quoted 51 lines · Bartosz Golaszewski
Quoted 49 lines
2018-05-11 22:13 GMT+02:00 Rob Herring <[email protected]>:
Quoted 15 lines · Bartosz Golaszewski
Quoted 5 lines
This series is a follow-up to the RFC[1] posted a couple days ago.
NOTE: this series applies on top of my recent patches[2] that move the previous implementation of early platform devices to
arch/sh.Quoted 8 lines
Problem:
Certain class of devices, such as timers, certain clock drivers and irq chip drivers need to be probed early in the boot sequence. The currently preferred approach is using one of the
OF_DECLARE()macros. This however does not create a platform device which has many drawbacks - such as not being able to use devres routines,dev_log functions or no way of deferring the init OF function if some other resources are missing.Quoted 8 lines
I skimmed though this and it doesn't look horrible (how's that for positive feedback? ;) ). But before going into the details, I think first there needs to be agreement this is the right direction.
The question does remain though as to whether this class of devices should be platform drivers. They can't be modules. They can't be hotplugged. Can they be runtime-pm enabled? So the advantage is ...
The main (but not the only) advantage for drivers that can both be platform drivers and
OF_DECLAREdrivers is that we get a single entry point and can reuse code without resorting to checking if (!dev). It results in more consistent code base. Another big advantage is consolidation of device tree and machine code for SoC drivers used in different boards of which some are still using board files and others are defined in DT (see: DaVinci).Quoted 9 lines
I assume that the clock maintainers had some reason to move clocks to be platform drivers. It's just not clear to me what that was.
Quoted 5 lines
For drivers that use both platform drivers and
OF_DECLAREthe situation is even more complicated as the code needs to take into account that there can possibly be no struct device present. For a specific use case that we're having problems with, please refer to the recent DaVinci common-clock conversion patches and the nasty workaround that this problem implies[3].Quoted 5 lines
So
devm_kzallocwill work with this solution? Why did we needdevm_kzallocin the first place? The clocks can never be removed and cleaning up on error paths is kind of pointless. The system would be hosed, right?It depends - not all clocks are necessary for system to boot.
Quoted 7 lines
Quoted 2 lines
That doesn't matter. You have a single driver for all/most of the clocks, so the driver can't be removed.
-ECANOFWORMSA couple of quick rebuttals, but I imagine we're going to disagree on the
platform_driverthing as a matter of taste no matter what...
It's really more should the clocksource, clockevents and primary interrupt controller be drivers. Let's get agreement on that first. If yes, then it probably does make sense that their dependencies are drivers too. If not, then making only the dependencies drivers doesn't seem right to me.
Quoted 9 lines
1) There should be multiple clk drivers in a properly modeled system. Some folks still incorrectly put all clocks in a system into a single driver because That's How We Used To Do It, and some systems (simpler ones) really only have a single clock generator IP block.
Excepting those two reasons above, we really should have separate drivers for clocks controlled by the PMIC, for the (one or more) clock generator blocks inside of the AP/SoC, and then even more for the drivers that map to IP blocks that have their own clock gens.
I agree those should be separate entities at least. But for a given h/w block, if you already have to use OF_DECLARE, why would you try to split that into OF_DECLARE and a driver? what advantage does putting non-boot essential clocks in a driver or transitioning to a driver get you?
And I've seen PMIC clocks could be inputs to the SoC's clock controller(s), so the dependencies get more complicated. Then does the PMIC driver and its dependencies need to be early drivers too?
Quoted 4 lines
Good examples of the latter are display controllers that generate their own PLL and pixel clock. Or MMC controllers that have a runtime-programmable clock divider. Examples of these are merged into mainline.
But those are drivers of types other than a clock controller that happen to register some clocks as well. I wasn't saying these cases can't or shouldn't be part of the driver model. Look at irqchips. We have some that use the driver model (e.g. every GPIO driver) and some that don't because there's no need (e.g. GIC).
Quoted 6 lines
2) Stephen and I wanted clock drivers to actually be represented in the driver model. There were these gigantic clock drivers that exclusively used
CLK_OF_DECLAREand they just sort of floated out there in the ether... no representation in sysfs, no struct device to map onto a clock controller struct, etc.
Quoted 8 lines
I'd be happy to hear why you think that
platform_driveris a bad fit for a device driver that generally manages memory-mapped system resources that are part of the system glue and not really tied to a specific bus. Sounds like a good fit to me.If
platform_driverdoesn't handle the early boot thing well, that tells me that we have a problem to solve inplatform_driver, not in the clk subsystem or drivers.
Doing things earlier is not the only way to solve the problems. Perhaps we need to figure out how to start things later. Maybe it's not feasible here, I don't know.
Quoted 7 lines
3) Lots of clock controllers should be loadable modules. E.g. i2c clock expanders, potentially external PMIC-related drivers, external audio codecs, etc.
Again, repeating my point #1 above, just because many platforms have a monolithic clock driver does not mean that this is the Right Way to do it.
And in those cases, I completely agree they should be part of a driver.
Rob
Hi Rob,
Quoted 59 lines · Rob Herring
Quoted 57 lines · Michael Turquette
Quoting Rob Herring (2018-05-14 06:20:57)
Quoted 49 lines · Bartosz Golaszewski
Quoted 48 lines
2018-05-11 22:13 GMT+02:00 Rob Herring <[email protected]>:
Quoted 15 lines · Bartosz Golaszewski
Quoted 5 lines
This series is a follow-up to the RFC[1] posted a couple days ago.
NOTE: this series applies on top of my recent patches[2] that move the previous implementation of early platform devices to
arch/sh.Quoted 8 lines
Problem:
Certain class of devices, such as timers, certain clock drivers and irq chip drivers need to be probed early in the boot sequence. The currently preferred approach is using one of the
OF_DECLARE()macros. This however does not create a platform device which has many drawbacks - such as not being able to use devres routines,dev_log functions or no way of deferring the init OF function if some other resources are missing.Quoted 8 lines
I skimmed though this and it doesn't look horrible (how's that for positive feedback? ;) ). But before going into the details, I think first there needs to be agreement this is the right direction.
The question does remain though as to whether this class of devices should be platform drivers. They can't be modules. They can't be hotplugged. Can they be runtime-pm enabled? So the advantage is ...
The main (but not the only) advantage for drivers that can both be platform drivers and
OF_DECLAREdrivers is that we get a single entry point and can reuse code without resorting to checking if (!dev). It results in more consistent code base. Another big advantage is consolidation of device tree and machine code for SoC drivers used in different boards of which some are still using board files and others are defined in DT (see: DaVinci).Quoted 14 lines
I assume that the clock maintainers had some reason to move clocks to be platform drivers. It's just not clear to me what that was.
Quoted 5 lines
For drivers that use both platform drivers and
OF_DECLAREthe situation is even more complicated as the code needs to take into account that there can possibly be no struct device present. For a specific use case that we're having problems with, please refer to the recent DaVinci common-clock conversion patches and the nasty workaround that this problem implies[3].So
devm_kzallocwill work with this solution? Why did we needdevm_kzallocin the first place? The clocks can never be removed and cleaning up on error paths is kind of pointless. The system would be hosed, right?Quoted 4 lines
Quoted 1 line
It depends - not all clocks are necessary for system to boot.
That doesn't matter. You have a single driver for all/most of the clocks, so the driver can't be removed.
-ECANOFWORMS
Quoted 8 lines
Quoted 2 lines
A couple of quick rebuttals, but I imagine we're going to disagree on the
platform_driverthing as a matter of taste no matter what...It's really more should the clocksource, clockevents and primary interrupt controller be drivers. Let's get agreement on that first. If yes, then it probably does make sense that their dependencies are drivers too. If not, then making only the dependencies drivers doesn't seem right to me.
Yes, they should all be drivers.
Assuming clocksources, clockevents, or primary interrupt controllers are special, and thus forcing everyone to use OF_DECLARE for the whole subsystem, doesn't fly everywhere.
Clockevents and interrupt controllers can have a module clock. All three can be part of a PM Domain, which requires a struct device to be handled properly.
Quoted 10 lines
Quoted 9 lines
1) There should be multiple clk drivers in a properly modeled system. Some folks still incorrectly put all clocks in a system into a single driver because That's How We Used To Do It, and some systems (simpler ones) really only have a single clock generator IP block.
Excepting those two reasons above, we really should have separate drivers for clocks controlled by the PMIC, for the (one or more) clock generator blocks inside of the AP/SoC, and then even more for the drivers that map to IP blocks that have their own clock gens.
Quoted 5 lines
I agree those should be separate entities at least. But for a given h/w block, if you already have to use
OF_DECLARE, why would you try to split that intoOF_DECLAREand a driver? what advantage does putting non-boot essential clocks in a driver or transitioning to a driver get you?
You may want to split it because of dependencies. OF_DECLARE doesn't handle EPROBE_DEFER, while some critical parts may be needed early.
Quoted 3 lines
And I've seen PMIC clocks could be inputs to the SoC's clock controller(s), so the dependencies get more complicated. Then does the PMIC driver and its dependencies need to be early drivers too?
Especially if there are clock loops, but that's something different. Cfr. cs2000 in arch/arm64/boot/dts/renesas/salvator-common.dtsi, which provides a clock from the main SoC, but also consumes a clock from the main SoC. The latter is modeled in DT as a fixed-clock as a workaround.
Quoted 10 lines
Quoted 4 lines
Good examples of the latter are display controllers that generate their own PLL and pixel clock. Or MMC controllers that have a runtime-programmable clock divider. Examples of these are merged into mainline.
But those are drivers of types other than a clock controller that happen to register some clocks as well. I wasn't saying these cases can't or shouldn't be part of the driver model. Look at irqchips. We have some that use the driver model (e.g. every GPIO driver) and some that don't because there's no need (e.g. GIC).
There is a need for using the driver model for the GIC. On some platforms, the GIC can be part of a clock and/or power domain.
For secondary GICs, that got fixed in commit 9c8edddfc9924cb4 ("irqchip/gic: Add platform driver for non-root GICs that require RPM"). For primary GICs, it's still not fixed, so we have to live with CLK_IS_CRITICAL, and rely on other critical devices in the same power domain to avoid the power domain being powered off.
Quoted 11 lines
Quoted 11 lines
2) Stephen and I wanted clock drivers to actually be represented in the driver model. There were these gigantic clock drivers that exclusively used
CLK_OF_DECLAREand they just sort of floated out there in the ether... no representation in sysfs, no struct device to map onto a clock controller struct, etc.I'd be happy to hear why you think that
platform_driveris a bad fit for a device driver that generally manages memory-mapped system resources that are part of the system glue and not really tied to a specific bus. Sounds like a good fit to me.
Quoted 7 lines
Quoted 3 lines
If
platform_driverdoesn't handle the early boot thing well, that tells me that we have a problem to solve inplatform_driver, not in the clk subsystem or drivers.Doing things earlier is not the only way to solve the problems. Perhaps we need to figure out how to start things later. Maybe it's not feasible here, I don't know.
The fixed probe order imposed by OF_DECLARE() limits this: if your OF_DECLARE() driver depends on something else, the latter must become an early device.
If all subsystems would use real devices, EPROBE_DEFER would handle most of it automatically.
Then, next step would be to avoid triggering EPROBE_DEFER, e.g. by analyzing dependencies in DT...
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoted 9 lines · Geert Uytterhoeven
Yes, they should all be drivers.
Assuming clocksources, clockevents, or primary interrupt controllers are special, and thus forcing everyone to use
OF_DECLAREfor the whole subsystem, doesn't fly everywhere.Clockevents and interrupt controllers can have a module clock. All three can be part of a PM Domain, which requires a struct device to be handled properly.
I agree it sure would be nice to have all these as drivers. I believe SoC specific clockevent using SoC specific clocks is the reason why some clocks cannot currently be drivers.
Regards,
Tony
pt., 11 maj 2018 o 18:21 Bartosz Golaszewski <[email protected]> napisał(a):
Quoted 108 lines
This series is a follow-up to the RFC[1] posted a couple days ago.
NOTE: this series applies on top of my recent patches[2] that move the previous implementation of early platform devices to
arch/sh.Problem:
Certain class of devices, such as timers, certain clock drivers and irq chip drivers need to be probed early in the boot sequence. The currently preferred approach is using one of the
OF_DECLARE()macros. This however does not create a platform device which has many drawbacks - such as not being able to use devres routines,dev_log functions or no way of deferring the init OF function if some other resources are missing.For drivers that use both platform drivers and
OF_DECLAREthe situation is even more complicated as the code needs to take into account that there can possibly be no struct device present. For a specific use case that we're having problems with, please refer to the recent DaVinci common-clock conversion patches and the nasty workaround that this problem implies[3].We also used to have an early platform drivers implementation but they were not integrated with the linux device model at all - they merely used the same data structures. The users could not use devres, defer probe and the early devices never became actual platform devices later on.
Proposed solution:
This series aims at solving this problem by (re-)introducing the concept of early platform drivers and devices - this time however in a way that seamlessly integrates with the existing platform drivers and also offers device-tree support.
The idea is to provide a way for users to probe devices early, while already being able to use devres, devices resources and properties and also deferred probing.
New structures are introduced: the early platform driver contains the
early_probecallback which has the same signature as regularplatform_deviceprobe. This callback is called early on. The user can have both the early and regular probe speficied or only one of them and they both receive the same platform device object as argument. Any device data allocated early will be carried over to the normal probe.The architecture code is responsible for calling
early_platform_start()in which the early drivers will be registered and devices populated from DT.Once the device and kobject mechanisms are ready, all early drivers and devices will be converted into real platform drivers and devices. Also: if any of the early platform registration functions will be called once early initialization is done, these functions will work like regular platform_device/driver ones.
Patches 1-9/12 introduce changes to existing code that are necessary before adding support for early drivers. Patch 10/12 contains the new framwork in an isolated file which can be compiled only if needed by the architecture.
Patch 11/12 contains a dummy early platform driver that serves as a code example and can be used for simple testing.
The last patch finally makes the of/platform code aware of early platform drivers.
If accepted, this new mechanism could potentially lead to consolidation of the code currently used by users of
OF_DECLARE, since they could be converted to actual platform drivers.[1] https://lkml.org/lkml/2018/4/26/657
[2] https://lkml.org/lkml/2018/4/30/547
[3] https://lkml.org/lkml/2018/4/26/1094Bartosz Golaszewski (12):
platform/early: add a new field to struct device platform/early: don'tWARN()on non-empty devres list for early
devices
platform/early: exportplatform_match()locally
platform: provide a separate function for initializing platform
devices
platform: exportplatform_device_release()locally
of: add a new flag for OF device nodes
of/platform: provide a separate routine for setting up device
resources
of/platform: provide a separate routine for device initialization platform/early: add an init section for early driver data platform/early: implement support for early platform drivers
misc: implement a dummy early platform driver of/platform: make the OF code aware of early platform driversdrivers/base/Kconfig | 3 + drivers/base/Makefile | 1 + drivers/base/base.h | 4 + drivers/base/dd.c | 2 +- drivers/base/early.c | 332 ++++++++++++++++++++++++++++++ drivers/base/platform.c | 28 ++- drivers/misc/Kconfig | 8 + drivers/misc/Makefile | 2 + drivers/misc/dummy-early.c | 82 ++++++++ drivers/of/platform.c | 85 +++++--- include/asm-generic/vmlinux.lds.h | 11 + include/linux/device.h | 1 + include/linux/early_platform.h | 75 +++++++ include/linux/of.h | 1 + include/linux/of_platform.h | 2 + include/linux/platform_device.h | 2 + 16 files changed, 604 insertions(+), 35 deletions(-) create mode 100644 drivers/base/early.c create mode 100644 drivers/misc/dummy-early.c create mode 100644 include/linux/early_platform.h
Quoted 3 lines
--
2.17.0
Hi
For anyone interested: I'll be doing a BoF about early platform drivers next week on Monday at 6:00pm during the ELCE 2018 in Edinburgh. It would be great to discuss this topic personally.
Best regards and hope to see you there
Bartosz Golaszewski