Re: [PATCH 00/13] Discover and probe dependencies
Am 18.06.2015 um 16:49 schrieb Russell King - ARM Linux: I think you may need to pick a better example to illustrate your point. :) It's easy to describe: Sort driver probe calls and forget about the whole device-stuff. The real target is to sort the driver probe calls and ideally the whole device stuff could be ignored, leaving it as however it may work. Unfortunately, that's not as easy to implement as it sounds. ;) I've too hit the device-stuff very late in my driver-probe-ordering experiment and just did it somehow, because I already was at the limit of the time I wanted to spend. ;) So to repeat myself, I think the first target has to be to identify drivers either without having to call their initcalls at all, or by making sure their initcalls just register and do nothing else (what I've called "well-done"). Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/13] Discover and probe dependencies
On 18 June 2015 at 11:42, Andrzej Hajda wrote: > Hi Tomeu, > > I have few comments about the design. > > On 06/17/2015 03:42 PM, Tomeu Vizoso wrote: >> Hi, >> >> this is another attempt at preventing deferred probe from obscuring why your >> devices aren't probing and from delaying to the end of the boot process the >> probe of the device you care the most. >> >> The major differences with my previous approach [0] are: >> >> * Dependencies are probed before the target is probed, so we don't have >> nested >> probe() calls, avoiding a series of deadlock situations. >> >> * Dependencies could be stored and reused for other purposes such as for >> passing resources to drivers ala devm_probe, or for warning when a device >> is >> going to be unbound and has dependencies active, etc. > > With this approach we should assume many things, for example: > 1. Dependencies are explicitly described in firmware (dts/dtb). > It will not work for example with lookup tables present in > gpios/clocks/regulators. Yes, but my understanding is that we are moving towards having the hw described in fwnode (so DT, ACPI and board files), and these dependencies are part of the hw description. > 2. Provider create/register their resources only during probe. > It is not always the case - for example componentized drivers in > probe often > calls only component_add, the real initialization is performed in > bind callback. We don't really try to bind the actual provider, but the platform device from which it derives, assuming that once that platform device finishes probing, the actual provider will have been probed as well. >From what I gather from reading drivers/of/platform.c this assumption has to hold for DT-based machines, but I'm not so sure about others. > 3. Dependencies are mandatory, ie without it driver will not be able to > successfully finish > the probe. > It should be not true. Sometimes device will require given resource > only in specific > scenario, or it can still probe successfully and ask for the > resource later. > I can also imagine that firmware can describe more information than > given driver require, > some resources even if they are present in the dts, will be not > requested by the driver, it > can be the case of drivers providing limited functionality, or just > obsolete bindings. > > > I have also more general design objection, which should not be > necessarily true: > Device node describes piece of the hardware which should be mainly > interpreted by the driver. > Parsing it in external code [1] violates this idea. Additionally we will > have the same information > parsed and interpreted in two different places (discovery framework and > the driver), > it does not look good to me. > But as I said earlier it is just my opinion, not a solid evidence :) Yeah, I see that concern, but actually the code that interprets dependencies are the subsystem cores, not the drivers themselves. It's true that this approach introduces some code duplication that the on-demand series doesn't, but I'm not sure it would be a clear win to refactor that duplication away. > [1]: I know that for example clk_get is also located in external > framework but currently the driver > decides that it should call clk_get, my_private_clk_get, > other_framework_clk_get or do not call it at all, > this framework assumes that it will be always clk_get, or at least > something compatible with it at binding level. Yeah, in this series I assume that if the gpio bindings say that phandles in properties with names ending in -gpios, then any phandles in properties with that name scheme should point to gpiochips. I have done some grepping and for this and all the other generic subsystems this seems to hold true (there aren't properties that follow any of those name schemes and that contain some other information). Regards, Tomeu > Regards > Andrzej > > >> >> * I have tried to keep it firmware-agnostic. The previous approach (on-demand >> probing) could be done like this as well, but would require adding fwnode >> APIs to all affected subsystems first. >> >> I have only implemented the class.get_dependencies() callback for the GPIO >> subsystem and for the host1x bus because that's all that was needed on my >> Tegra >> Chromebook to avoid deferred probes, but if this approach is deemed >> worthwhile >> I will add more implementations so that deferred probes are avoided on the >> other boards I have access to. >> >> [0] http://thread.gmane.org/gmane.linux.kernel.gpio/8465 >> >> Thanks, >> >> Tomeu >> >> Tomeu Vizoso (13): >> gpiolib: Fix docs for gpiochip_add_pingroup_range >> driver-core: defer all probes until late_initcall >> ARM: tegra: Add gpio-ranges property >> pinctrl: tegra: Only set the gpio range if needed >> driver core: fix docbook for device_private.device >> of/platform: Set fwnode field for new devices >> driver-core: Add class.get_dependencies() callback >>
Re: [PATCH 00/13] Discover and probe dependencies
On Thu, Jun 18, 2015 at 03:14:31PM +0200, Andrzej Hajda wrote: > Lets look at more real example: we have HDMI encoder which can > use some video and audio resources provided by some video and audio > drivers. If we know that our machine will work without sound we can > disable audio drivers but we can expect video should still work, ie > HDMI driver should successfully probe even if audio resources are > not available. That already happens today, if you structure the driver appropriately. I really don't buy into the crap argument that "all struct device must be created by DT" - it is perfectly acceptable for a device driver to declare and register its own sub-devices where it's appropraite for it to do so. Since audio _requires_ video on HDMI to work (audio fundamentally depends on a working video setup), it is perfectly acceptable for a HDMI video driver to register a struct device for its audio driver, and to pass details that the audio driver may need. What is not acceptable is to duplicate the HDMI drivers device_node into the child devices: this creates a situation where the generic device model can match the HDMI driver with its child device. So this is a big no no. The model I refer to above is something which I have, and others have implemented for HDMI devices, and it's a completely reasonable model. Remember, DT is about describing the hardware. If you have a HDMI encoder, that's one hardware block, and there should be one entry describing it in DT. Just because in Linux we may decide to separate them into two separate drivers, and therefore two separate struct device's is an implementation detail, and is not a reason to medle with the hardware model in DT. I think you may need to pick a better example to illustrate your point. :) -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/13] Discover and probe dependencies
On 18 June 2015 at 15:14, Andrzej Hajda wrote: > On 06/18/2015 12:36 PM, Mark Brown wrote: >> On Thu, Jun 18, 2015 at 11:42:01AM +0200, Andrzej Hajda wrote: >> >> There's something really messed up with how your mailer is word wrapping >> paragraphs, might want to check it out - it looks like it's making lines >> that are longer than 80 columns and then randomly breaking them 80 >> columns in rather than reflowing things. >> >>> 3. Dependencies are mandatory, ie without it driver will not be able to >>> successfully finish >>> the probe. >>> It should be not true. Sometimes device will require given resource >>> only in specific >>> scenario, or it can still probe successfully and ask for the >>> resource later. >>> I can also imagine that firmware can describe more information than >>> given driver require, >>> some resources even if they are present in the dts, will be not >>> requested by the driver, it >>> can be the case of drivers providing limited functionality, or just >>> obsolete bindings. >> Well, this isn't clear - the model here seems to be that the >> dependencies are those that are explicitly listed for a given platform. >> For those our current expectation is that we will try to control them >> and will defer probe until the resources that are mapped in on the >> platform are present so this doesn't seem like a change. > IMO current assumption is that we CAN TRY to control them > and we MAY defer probe until resource is present, nothing mandatory. > > Lets look at more real example: we have HDMI encoder which can > use some video and audio resources provided by some video and audio > drivers. If we know that our machine will work without sound we can > disable audio drivers but we can expect video should still work, ie HDMI > driver should successfully probe even if audio resources are not available. > > I had an impression that in this patchset the device wont be probed > until all dependencies are present, but after looking at the code it > does not seems to be true - in case dependency cannot be probed > warning will be printed but the probe will continue, so it should handle > scenario above properly. Yeah, I think deferred probe is a very good way to make sure that we get a working system at the end. It's just that as we probe devices in a random order, we'd get a lot of noise if we printed something every time a device failed to find a resource and deferred its probe. You can see these series as just changing the order in which we probe devices, to one in which dependencies are probed before the device that depends on them. > The only minor issue is that we will see > sometimes misleading message about missing dependencies. Yes, that's why they are currently pr_debug and not pr_warn. I think that once we have tables specifying the resources needed by each driver (ala devm_probe), we can know which are optional and print appropriate warnings accordingly. Thanks, Tomeu > Regards > Andrzej > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/13] Discover and probe dependencies
On 06/18/2015 12:36 PM, Mark Brown wrote: > On Thu, Jun 18, 2015 at 11:42:01AM +0200, Andrzej Hajda wrote: > > There's something really messed up with how your mailer is word wrapping > paragraphs, might want to check it out - it looks like it's making lines > that are longer than 80 columns and then randomly breaking them 80 > columns in rather than reflowing things. > >> 3. Dependencies are mandatory, ie without it driver will not be able to >> successfully finish >> the probe. >> It should be not true. Sometimes device will require given resource >> only in specific >> scenario, or it can still probe successfully and ask for the >> resource later. >> I can also imagine that firmware can describe more information than >> given driver require, >> some resources even if they are present in the dts, will be not >> requested by the driver, it >> can be the case of drivers providing limited functionality, or just >> obsolete bindings. > Well, this isn't clear - the model here seems to be that the > dependencies are those that are explicitly listed for a given platform. > For those our current expectation is that we will try to control them > and will defer probe until the resources that are mapped in on the > platform are present so this doesn't seem like a change. IMO current assumption is that we CAN TRY to control them and we MAY defer probe until resource is present, nothing mandatory. Lets look at more real example: we have HDMI encoder which can use some video and audio resources provided by some video and audio drivers. If we know that our machine will work without sound we can disable audio drivers but we can expect video should still work, ie HDMI driver should successfully probe even if audio resources are not available. I had an impression that in this patchset the device wont be probed until all dependencies are present, but after looking at the code it does not seems to be true - in case dependency cannot be probed warning will be printed but the probe will continue, so it should handle scenario above properly. The only minor issue is that we will see sometimes misleading message about missing dependencies. Regards Andrzej -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/13] Discover and probe dependencies
On Thu, Jun 18, 2015 at 11:42:01AM +0200, Andrzej Hajda wrote: There's something really messed up with how your mailer is word wrapping paragraphs, might want to check it out - it looks like it's making lines that are longer than 80 columns and then randomly breaking them 80 columns in rather than reflowing things. > 3. Dependencies are mandatory, ie without it driver will not be able to > successfully finish > the probe. > It should be not true. Sometimes device will require given resource > only in specific > scenario, or it can still probe successfully and ask for the > resource later. > I can also imagine that firmware can describe more information than > given driver require, > some resources even if they are present in the dts, will be not > requested by the driver, it > can be the case of drivers providing limited functionality, or just > obsolete bindings. Well, this isn't clear - the model here seems to be that the dependencies are those that are explicitly listed for a given platform. For those our current expectation is that we will try to control them and will defer probe until the resources that are mapped in on the platform are present so this doesn't seem like a change. signature.asc Description: Digital signature
Re: [PATCH 00/13] Discover and probe dependencies
On Thu, Jun 18, 2015 at 11:42:01AM +0200, Andrzej Hajda wrote:
> 2. Provider create/register their resources only during probe.
> It is not always the case - for example componentized drivers in
> probe often
> calls only component_add, the real initialization is performed in
> bind callback.
bind is called in _a_ probe callback, but it may not be the probe callback
associated with the device. (It'll be the final component's callback.)
I'm willing to be less critical of componentised drivers claiming their
resources in ->probe _iff_ they separate their data structures, like:
struct foo_priv {
void *base;
struct clk *clk;
struct dma_chan *chan;
... other resource data ...;
struct foo_runtime {
... runtime data ...
} rt;
};
and then, in their bind callback, they memset the foo_runtime structure
to zero, to ensure that the driver always re-binds in the same state as
the first bind.
I've seen too many lax drivers over the years that this is a point I'm
very insistant on: either componentised drivers don't do any resource
claiming in their probe function, or they take steps to ensure non-
resource struct members are properly separated such that they can
guarantee that they aren't going to accidentally use something from a
previous binding.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/13] Discover and probe dependencies
Hi Tomeu, I have few comments about the design. On 06/17/2015 03:42 PM, Tomeu Vizoso wrote: > Hi, > > this is another attempt at preventing deferred probe from obscuring why your > devices aren't probing and from delaying to the end of the boot process the > probe of the device you care the most. > > The major differences with my previous approach [0] are: > > * Dependencies are probed before the target is probed, so we don't have nested > probe() calls, avoiding a series of deadlock situations. > > * Dependencies could be stored and reused for other purposes such as for > passing resources to drivers ala devm_probe, or for warning when a device is > going to be unbound and has dependencies active, etc. With this approach we should assume many things, for example: 1. Dependencies are explicitly described in firmware (dts/dtb). It will not work for example with lookup tables present in gpios/clocks/regulators. 2. Provider create/register their resources only during probe. It is not always the case - for example componentized drivers in probe often calls only component_add, the real initialization is performed in bind callback. 3. Dependencies are mandatory, ie without it driver will not be able to successfully finish the probe. It should be not true. Sometimes device will require given resource only in specific scenario, or it can still probe successfully and ask for the resource later. I can also imagine that firmware can describe more information than given driver require, some resources even if they are present in the dts, will be not requested by the driver, it can be the case of drivers providing limited functionality, or just obsolete bindings. I have also more general design objection, which should not be necessarily true: Device node describes piece of the hardware which should be mainly interpreted by the driver. Parsing it in external code [1] violates this idea. Additionally we will have the same information parsed and interpreted in two different places (discovery framework and the driver), it does not look good to me. But as I said earlier it is just my opinion, not a solid evidence :) [1]: I know that for example clk_get is also located in external framework but currently the driver decides that it should call clk_get, my_private_clk_get, other_framework_clk_get or do not call it at all, this framework assumes that it will be always clk_get, or at least something compatible with it at binding level. Regards Andrzej > > * I have tried to keep it firmware-agnostic. The previous approach (on-demand > probing) could be done like this as well, but would require adding fwnode > APIs to all affected subsystems first. > > I have only implemented the class.get_dependencies() callback for the GPIO > subsystem and for the host1x bus because that's all that was needed on my > Tegra > Chromebook to avoid deferred probes, but if this approach is deemed worthwhile > I will add more implementations so that deferred probes are avoided on the > other boards I have access to. > > [0] http://thread.gmane.org/gmane.linux.kernel.gpio/8465 > > Thanks, > > Tomeu > > Tomeu Vizoso (13): > gpiolib: Fix docs for gpiochip_add_pingroup_range > driver-core: defer all probes until late_initcall > ARM: tegra: Add gpio-ranges property > pinctrl: tegra: Only set the gpio range if needed > driver core: fix docbook for device_private.device > of/platform: Set fwnode field for new devices > driver-core: Add class.get_dependencies() callback > gpio: sysfs: implement class.get_dependencies() > gpu: host1x: implement class.get_dependencies() > driver-core: add for_each_class() > device property: add fwnode_get_parent() > device property: add fwnode_get_name() > driver-core: probe dependencies before probing > > arch/arm/boot/dts/tegra114.dtsi | 1 + > arch/arm/boot/dts/tegra124.dtsi | 1 + > arch/arm/boot/dts/tegra20.dtsi | 1 + > arch/arm/boot/dts/tegra30.dtsi | 1 + > drivers/base/base.h | 4 +- > drivers/base/class.c| 16 + > drivers/base/dd.c | 128 > +++- > drivers/base/property.c | 38 > drivers/gpio/gpiolib-sysfs.c| 81 + > drivers/gpio/gpiolib.c | 2 +- > drivers/gpu/host1x/dev.c| 47 +++ > drivers/of/platform.c | 1 + > drivers/pinctrl/pinctrl-tegra.c | 19 +- > include/acpi/acpi_bus.h | 5 ++ > include/linux/acpi.h| 5 ++ > include/linux/device.h | 6 ++ > include/linux/fwnode.h | 5 ++ > include/linux/property.h| 4 ++ > 18 files changed, 361 insertions(+), 4 deletions(-) > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at
Re: [PATCH 00/13] Discover and probe dependencies
On 06/18/2015 12:36 PM, Mark Brown wrote: On Thu, Jun 18, 2015 at 11:42:01AM +0200, Andrzej Hajda wrote: There's something really messed up with how your mailer is word wrapping paragraphs, might want to check it out - it looks like it's making lines that are longer than 80 columns and then randomly breaking them 80 columns in rather than reflowing things. 3. Dependencies are mandatory, ie without it driver will not be able to successfully finish the probe. It should be not true. Sometimes device will require given resource only in specific scenario, or it can still probe successfully and ask for the resource later. I can also imagine that firmware can describe more information than given driver require, some resources even if they are present in the dts, will be not requested by the driver, it can be the case of drivers providing limited functionality, or just obsolete bindings. Well, this isn't clear - the model here seems to be that the dependencies are those that are explicitly listed for a given platform. For those our current expectation is that we will try to control them and will defer probe until the resources that are mapped in on the platform are present so this doesn't seem like a change. IMO current assumption is that we CAN TRY to control them and we MAY defer probe until resource is present, nothing mandatory. Lets look at more real example: we have HDMI encoder which can use some video and audio resources provided by some video and audio drivers. If we know that our machine will work without sound we can disable audio drivers but we can expect video should still work, ie HDMI driver should successfully probe even if audio resources are not available. I had an impression that in this patchset the device wont be probed until all dependencies are present, but after looking at the code it does not seems to be true - in case dependency cannot be probed warning will be printed but the probe will continue, so it should handle scenario above properly. The only minor issue is that we will see sometimes misleading message about missing dependencies. Regards Andrzej -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/13] Discover and probe dependencies
Am 18.06.2015 um 16:49 schrieb Russell King - ARM Linux: I think you may need to pick a better example to illustrate your point. :) It's easy to describe: Sort driver probe calls and forget about the whole device-stuff. The real target is to sort the driver probe calls and ideally the whole device stuff could be ignored, leaving it as however it may work. Unfortunately, that's not as easy to implement as it sounds. ;) I've too hit the device-stuff very late in my driver-probe-ordering experiment and just did it somehow, because I already was at the limit of the time I wanted to spend. ;) So to repeat myself, I think the first target has to be to identify drivers either without having to call their initcalls at all, or by making sure their initcalls just register and do nothing else (what I've called well-done). Regards, Alexander Holler -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/13] Discover and probe dependencies
On 18 June 2015 at 15:14, Andrzej Hajda a.ha...@samsung.com wrote: On 06/18/2015 12:36 PM, Mark Brown wrote: On Thu, Jun 18, 2015 at 11:42:01AM +0200, Andrzej Hajda wrote: There's something really messed up with how your mailer is word wrapping paragraphs, might want to check it out - it looks like it's making lines that are longer than 80 columns and then randomly breaking them 80 columns in rather than reflowing things. 3. Dependencies are mandatory, ie without it driver will not be able to successfully finish the probe. It should be not true. Sometimes device will require given resource only in specific scenario, or it can still probe successfully and ask for the resource later. I can also imagine that firmware can describe more information than given driver require, some resources even if they are present in the dts, will be not requested by the driver, it can be the case of drivers providing limited functionality, or just obsolete bindings. Well, this isn't clear - the model here seems to be that the dependencies are those that are explicitly listed for a given platform. For those our current expectation is that we will try to control them and will defer probe until the resources that are mapped in on the platform are present so this doesn't seem like a change. IMO current assumption is that we CAN TRY to control them and we MAY defer probe until resource is present, nothing mandatory. Lets look at more real example: we have HDMI encoder which can use some video and audio resources provided by some video and audio drivers. If we know that our machine will work without sound we can disable audio drivers but we can expect video should still work, ie HDMI driver should successfully probe even if audio resources are not available. I had an impression that in this patchset the device wont be probed until all dependencies are present, but after looking at the code it does not seems to be true - in case dependency cannot be probed warning will be printed but the probe will continue, so it should handle scenario above properly. Yeah, I think deferred probe is a very good way to make sure that we get a working system at the end. It's just that as we probe devices in a random order, we'd get a lot of noise if we printed something every time a device failed to find a resource and deferred its probe. You can see these series as just changing the order in which we probe devices, to one in which dependencies are probed before the device that depends on them. The only minor issue is that we will see sometimes misleading message about missing dependencies. Yes, that's why they are currently pr_debug and not pr_warn. I think that once we have tables specifying the resources needed by each driver (ala devm_probe), we can know which are optional and print appropriate warnings accordingly. Thanks, Tomeu Regards Andrzej -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/13] Discover and probe dependencies
On 18 June 2015 at 11:42, Andrzej Hajda a.ha...@samsung.com wrote: Hi Tomeu, I have few comments about the design. On 06/17/2015 03:42 PM, Tomeu Vizoso wrote: Hi, this is another attempt at preventing deferred probe from obscuring why your devices aren't probing and from delaying to the end of the boot process the probe of the device you care the most. The major differences with my previous approach [0] are: * Dependencies are probed before the target is probed, so we don't have nested probe() calls, avoiding a series of deadlock situations. * Dependencies could be stored and reused for other purposes such as for passing resources to drivers ala devm_probe, or for warning when a device is going to be unbound and has dependencies active, etc. With this approach we should assume many things, for example: 1. Dependencies are explicitly described in firmware (dts/dtb). It will not work for example with lookup tables present in gpios/clocks/regulators. Yes, but my understanding is that we are moving towards having the hw described in fwnode (so DT, ACPI and board files), and these dependencies are part of the hw description. 2. Provider create/register their resources only during probe. It is not always the case - for example componentized drivers in probe often calls only component_add, the real initialization is performed in bind callback. We don't really try to bind the actual provider, but the platform device from which it derives, assuming that once that platform device finishes probing, the actual provider will have been probed as well. From what I gather from reading drivers/of/platform.c this assumption has to hold for DT-based machines, but I'm not so sure about others. 3. Dependencies are mandatory, ie without it driver will not be able to successfully finish the probe. It should be not true. Sometimes device will require given resource only in specific scenario, or it can still probe successfully and ask for the resource later. I can also imagine that firmware can describe more information than given driver require, some resources even if they are present in the dts, will be not requested by the driver, it can be the case of drivers providing limited functionality, or just obsolete bindings. I have also more general design objection, which should not be necessarily true: Device node describes piece of the hardware which should be mainly interpreted by the driver. Parsing it in external code [1] violates this idea. Additionally we will have the same information parsed and interpreted in two different places (discovery framework and the driver), it does not look good to me. But as I said earlier it is just my opinion, not a solid evidence :) Yeah, I see that concern, but actually the code that interprets dependencies are the subsystem cores, not the drivers themselves. It's true that this approach introduces some code duplication that the on-demand series doesn't, but I'm not sure it would be a clear win to refactor that duplication away. [1]: I know that for example clk_get is also located in external framework but currently the driver decides that it should call clk_get, my_private_clk_get, other_framework_clk_get or do not call it at all, this framework assumes that it will be always clk_get, or at least something compatible with it at binding level. Yeah, in this series I assume that if the gpio bindings say that phandles in properties with names ending in -gpios, then any phandles in properties with that name scheme should point to gpiochips. I have done some grepping and for this and all the other generic subsystems this seems to hold true (there aren't properties that follow any of those name schemes and that contain some other information). Regards, Tomeu Regards Andrzej * I have tried to keep it firmware-agnostic. The previous approach (on-demand probing) could be done like this as well, but would require adding fwnode APIs to all affected subsystems first. I have only implemented the class.get_dependencies() callback for the GPIO subsystem and for the host1x bus because that's all that was needed on my Tegra Chromebook to avoid deferred probes, but if this approach is deemed worthwhile I will add more implementations so that deferred probes are avoided on the other boards I have access to. [0] http://thread.gmane.org/gmane.linux.kernel.gpio/8465 Thanks, Tomeu Tomeu Vizoso (13): gpiolib: Fix docs for gpiochip_add_pingroup_range driver-core: defer all probes until late_initcall ARM: tegra: Add gpio-ranges property pinctrl: tegra: Only set the gpio range if needed driver core: fix docbook for device_private.device of/platform: Set fwnode field for new devices driver-core: Add class.get_dependencies() callback gpio: sysfs: implement class.get_dependencies() gpu: host1x: implement class.get_dependencies() driver-core: add
Re: [PATCH 00/13] Discover and probe dependencies
On Thu, Jun 18, 2015 at 03:14:31PM +0200, Andrzej Hajda wrote: Lets look at more real example: we have HDMI encoder which can use some video and audio resources provided by some video and audio drivers. If we know that our machine will work without sound we can disable audio drivers but we can expect video should still work, ie HDMI driver should successfully probe even if audio resources are not available. That already happens today, if you structure the driver appropriately. I really don't buy into the crap argument that all struct device must be created by DT - it is perfectly acceptable for a device driver to declare and register its own sub-devices where it's appropraite for it to do so. Since audio _requires_ video on HDMI to work (audio fundamentally depends on a working video setup), it is perfectly acceptable for a HDMI video driver to register a struct device for its audio driver, and to pass details that the audio driver may need. What is not acceptable is to duplicate the HDMI drivers device_node into the child devices: this creates a situation where the generic device model can match the HDMI driver with its child device. So this is a big no no. The model I refer to above is something which I have, and others have implemented for HDMI devices, and it's a completely reasonable model. Remember, DT is about describing the hardware. If you have a HDMI encoder, that's one hardware block, and there should be one entry describing it in DT. Just because in Linux we may decide to separate them into two separate drivers, and therefore two separate struct device's is an implementation detail, and is not a reason to medle with the hardware model in DT. I think you may need to pick a better example to illustrate your point. :) -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/13] Discover and probe dependencies
On Thu, Jun 18, 2015 at 11:42:01AM +0200, Andrzej Hajda wrote:
2. Provider create/register their resources only during probe.
It is not always the case - for example componentized drivers in
probe often
calls only component_add, the real initialization is performed in
bind callback.
bind is called in _a_ probe callback, but it may not be the probe callback
associated with the device. (It'll be the final component's callback.)
I'm willing to be less critical of componentised drivers claiming their
resources in -probe _iff_ they separate their data structures, like:
struct foo_priv {
void *base;
struct clk *clk;
struct dma_chan *chan;
... other resource data ...;
struct foo_runtime {
... runtime data ...
} rt;
};
and then, in their bind callback, they memset the foo_runtime structure
to zero, to ensure that the driver always re-binds in the same state as
the first bind.
I've seen too many lax drivers over the years that this is a point I'm
very insistant on: either componentised drivers don't do any resource
claiming in their probe function, or they take steps to ensure non-
resource struct members are properly separated such that they can
guarantee that they aren't going to accidentally use something from a
previous binding.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/13] Discover and probe dependencies
Hi Tomeu, I have few comments about the design. On 06/17/2015 03:42 PM, Tomeu Vizoso wrote: Hi, this is another attempt at preventing deferred probe from obscuring why your devices aren't probing and from delaying to the end of the boot process the probe of the device you care the most. The major differences with my previous approach [0] are: * Dependencies are probed before the target is probed, so we don't have nested probe() calls, avoiding a series of deadlock situations. * Dependencies could be stored and reused for other purposes such as for passing resources to drivers ala devm_probe, or for warning when a device is going to be unbound and has dependencies active, etc. With this approach we should assume many things, for example: 1. Dependencies are explicitly described in firmware (dts/dtb). It will not work for example with lookup tables present in gpios/clocks/regulators. 2. Provider create/register their resources only during probe. It is not always the case - for example componentized drivers in probe often calls only component_add, the real initialization is performed in bind callback. 3. Dependencies are mandatory, ie without it driver will not be able to successfully finish the probe. It should be not true. Sometimes device will require given resource only in specific scenario, or it can still probe successfully and ask for the resource later. I can also imagine that firmware can describe more information than given driver require, some resources even if they are present in the dts, will be not requested by the driver, it can be the case of drivers providing limited functionality, or just obsolete bindings. I have also more general design objection, which should not be necessarily true: Device node describes piece of the hardware which should be mainly interpreted by the driver. Parsing it in external code [1] violates this idea. Additionally we will have the same information parsed and interpreted in two different places (discovery framework and the driver), it does not look good to me. But as I said earlier it is just my opinion, not a solid evidence :) [1]: I know that for example clk_get is also located in external framework but currently the driver decides that it should call clk_get, my_private_clk_get, other_framework_clk_get or do not call it at all, this framework assumes that it will be always clk_get, or at least something compatible with it at binding level. Regards Andrzej * I have tried to keep it firmware-agnostic. The previous approach (on-demand probing) could be done like this as well, but would require adding fwnode APIs to all affected subsystems first. I have only implemented the class.get_dependencies() callback for the GPIO subsystem and for the host1x bus because that's all that was needed on my Tegra Chromebook to avoid deferred probes, but if this approach is deemed worthwhile I will add more implementations so that deferred probes are avoided on the other boards I have access to. [0] http://thread.gmane.org/gmane.linux.kernel.gpio/8465 Thanks, Tomeu Tomeu Vizoso (13): gpiolib: Fix docs for gpiochip_add_pingroup_range driver-core: defer all probes until late_initcall ARM: tegra: Add gpio-ranges property pinctrl: tegra: Only set the gpio range if needed driver core: fix docbook for device_private.device of/platform: Set fwnode field for new devices driver-core: Add class.get_dependencies() callback gpio: sysfs: implement class.get_dependencies() gpu: host1x: implement class.get_dependencies() driver-core: add for_each_class() device property: add fwnode_get_parent() device property: add fwnode_get_name() driver-core: probe dependencies before probing arch/arm/boot/dts/tegra114.dtsi | 1 + arch/arm/boot/dts/tegra124.dtsi | 1 + arch/arm/boot/dts/tegra20.dtsi | 1 + arch/arm/boot/dts/tegra30.dtsi | 1 + drivers/base/base.h | 4 +- drivers/base/class.c| 16 + drivers/base/dd.c | 128 +++- drivers/base/property.c | 38 drivers/gpio/gpiolib-sysfs.c| 81 + drivers/gpio/gpiolib.c | 2 +- drivers/gpu/host1x/dev.c| 47 +++ drivers/of/platform.c | 1 + drivers/pinctrl/pinctrl-tegra.c | 19 +- include/acpi/acpi_bus.h | 5 ++ include/linux/acpi.h| 5 ++ include/linux/device.h | 6 ++ include/linux/fwnode.h | 5 ++ include/linux/property.h| 4 ++ 18 files changed, 361 insertions(+), 4 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/13] Discover and probe dependencies
On Thu, Jun 18, 2015 at 11:42:01AM +0200, Andrzej Hajda wrote: There's something really messed up with how your mailer is word wrapping paragraphs, might want to check it out - it looks like it's making lines that are longer than 80 columns and then randomly breaking them 80 columns in rather than reflowing things. 3. Dependencies are mandatory, ie without it driver will not be able to successfully finish the probe. It should be not true. Sometimes device will require given resource only in specific scenario, or it can still probe successfully and ask for the resource later. I can also imagine that firmware can describe more information than given driver require, some resources even if they are present in the dts, will be not requested by the driver, it can be the case of drivers providing limited functionality, or just obsolete bindings. Well, this isn't clear - the model here seems to be that the dependencies are those that are explicitly listed for a given platform. For those our current expectation is that we will try to control them and will defer probe until the resources that are mapped in on the platform are present so this doesn't seem like a change. signature.asc Description: Digital signature
