| Message ID | 1487796452-25951-3-git-send-email-philipp.tomsich@theobroma-systems.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
Hi Philipp, On 22 February 2017 at 13:47, Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > Currently, driver binding stops once it encounters the first > compatible driver that doesn't refuse to bind. However, there are > cases where a single node will need to be handled by multiple driver > classes. For those cases we provide a configurable option to continue > to bind after the first driver has been found. > > The first use cases for this are from the DM conversion of the sunxi > (Allwinner) architecture: > * pinctrl (UCLASS_PINCTRL) and gpio (UCLASS_GPIO) drivers need to > bind against a single node > * clock (UCLASS_CLK) and reset (UCLASS_RESET) drivers also need to > bind against a single node Does linux work this way? Another approach would be to have a separate MISC driver with two children, one pinctrl, one clk. > > Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> > --- > drivers/core/Kconfig | 14 ++++++++++++++ > drivers/core/lists.c | 12 +++++++++++- > 2 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig > index 8749561..913101c 100644 > --- a/drivers/core/Kconfig > +++ b/drivers/core/Kconfig > @@ -31,6 +31,20 @@ config DM_WARN > This will cause dm_warn() to be compiled out - it will do nothing > when called. > > +config DM_ALLOW_MULTIPLE_DRIVERS > + bool "Allow multiple drivers to bind for one node" > + depends on DM > + default n You should be able to drop this line. > + help > + The driver model in U-Boot originally did not allow multiple > + drivers to bind for a single device node. > + > + If enabled, multiple drivers can now bind for a single node > + by using the same compatible string for matching: lists_bind_fdt() > + will assume that binding multiple drivers is desirable, if the > + caller does not request the pointer to the udevice structure to > + be returned (i.e. if devp is NULL). Please update the function documentation in the header file. > + > config DM_DEVICE_REMOVE > bool "Support device removal" > depends on DM > diff --git a/drivers/core/lists.c b/drivers/core/lists.c > index 23b6ba7..52efe69 100644 > --- a/drivers/core/lists.c > +++ b/drivers/core/lists.c > @@ -166,7 +166,11 @@ int lists_bind_fdt(struct udevice *parent, const void *blob, int offset, > dm_dbg(" - attempt to match compatible string '%s'\n", > compat); > > - for (entry = driver; entry != driver + n_ents; entry++) { > + entry = driver; > +#if defined(CONFIG_DM_ALLOW_MULTIPLE_DRIVERS) > + allow_more_matches: > +#endif > + for (; entry != driver + n_ents; entry++) { > ret = driver_check_compatible(entry->of_match, &id, > compat); > if (!ret) > @@ -190,6 +194,12 @@ int lists_bind_fdt(struct udevice *parent, const void *blob, int offset, > found = true; > if (devp) > *devp = dev; > +#if defined(CONFIG_DM_ALLOW_MULTIPLE_DRIVERS) Can you make this a variable, e.g. with allow_multiple = IS_ENABLED(DM_ALLOW_MULTIPLE_DRIVERS)? I'd prefer not to add #ifdefs in this file. > + else { > + entry++; > + goto allow_more_matches; Is it possible to loop without using goto? > + } > +#endif > } > break; > } > -- > 1.9.1 > Regards, Simon
Hi Simon, > On 03 Mar 2017, at 05:52, Simon Glass <sjg@chromium.org> wrote: > > Hi Philipp, > > On 22 February 2017 at 13:47, Philipp Tomsich > <philipp.tomsich@theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>> wrote: >> Currently, driver binding stops once it encounters the first >> compatible driver that doesn't refuse to bind. However, there are >> cases where a single node will need to be handled by multiple driver >> classes. For those cases we provide a configurable option to continue >> to bind after the first driver has been found. >> >> The first use cases for this are from the DM conversion of the sunxi >> (Allwinner) architecture: >> * pinctrl (UCLASS_PINCTRL) and gpio (UCLASS_GPIO) drivers need to >> bind against a single node >> * clock (UCLASS_CLK) and reset (UCLASS_RESET) drivers also need to >> bind against a single node > > Does linux work this way? Another approach would be to have a separate > MISC driver with two children, one pinctrl, one clk. The linux CLK driver creates and registers a reset-controller; the PINCTRL driver does the same with the gpio-controller. Similar code to do this is easily possible in U-Boot … see sunxi_pctrl_bind_gpio(…) in [PATCH v2 1/6] of this series. However, binding multiple times makes for much simpler code and allows to keep driver data in separate drivers. Regards, Philipp.
Hi, On 3 March 2017 at 03:52, Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > Hi Simon, > > On 03 Mar 2017, at 05:52, Simon Glass <sjg@chromium.org> wrote: > > Hi Philipp, > > On 22 February 2017 at 13:47, Philipp Tomsich > <philipp.tomsich@theobroma-systems.com> wrote: > > Currently, driver binding stops once it encounters the first > compatible driver that doesn't refuse to bind. However, there are > cases where a single node will need to be handled by multiple driver > classes. For those cases we provide a configurable option to continue > to bind after the first driver has been found. > > The first use cases for this are from the DM conversion of the sunxi > (Allwinner) architecture: > * pinctrl (UCLASS_PINCTRL) and gpio (UCLASS_GPIO) drivers need to > bind against a single node > * clock (UCLASS_CLK) and reset (UCLASS_RESET) drivers also need to > bind against a single node > > > Does linux work this way? Another approach would be to have a separate > MISC driver with two children, one pinctrl, one clk. > > > The linux CLK driver creates and registers a reset-controller; the PINCTRL > driver > does the same with the gpio-controller. Similar code to do this is easily > possible in > U-Boot … see sunxi_pctrl_bind_gpio(…) in [PATCH v2 1/6] of this series. > > However, binding multiple times makes for much simpler code and allows to > keep > driver data in separate drivers. My question was more whether Linux registers multiple drivers with one device node. It's just not something I expected. I'm not really convinced on this. It will break the current one-to-one relationship, and functions like device_get_child_by_of_offset(). I think it would be better to have a MISC driver with two children. Regards, Simon
Hi Simon, Am Sonntag, 12. März 2017, 14:21:35 CET schrieb Simon Glass: > On 3 March 2017 at 03:52, Dr. Philipp Tomsich > <philipp.tomsich@theobroma-systems.com> wrote: > > On 03 Mar 2017, at 05:52, Simon Glass <sjg@chromium.org> wrote: > > On 22 February 2017 at 13:47, Philipp Tomsich > > <philipp.tomsich@theobroma-systems.com> wrote: > > > > Currently, driver binding stops once it encounters the first > > compatible driver that doesn't refuse to bind. However, there are > > cases where a single node will need to be handled by multiple driver > > classes. For those cases we provide a configurable option to continue > > to bind after the first driver has been found. > > > > The first use cases for this are from the DM conversion of the sunxi > > (Allwinner) architecture: > > * pinctrl (UCLASS_PINCTRL) and gpio (UCLASS_GPIO) drivers need to > > > > bind against a single node > > > > * clock (UCLASS_CLK) and reset (UCLASS_RESET) drivers also need to > > > > bind against a single node > > > > Does linux work this way? Another approach would be to have a separate > > MISC driver with two children, one pinctrl, one clk. > > > > > > The linux CLK driver creates and registers a reset-controller; the PINCTRL > > driver > > does the same with the gpio-controller. Similar code to do this is easily > > possible in > > U-Boot … see sunxi_pctrl_bind_gpio(…) in [PATCH v2 1/6] of this series. > > > > However, binding multiple times makes for much simpler code and allows to > > keep > > driver data in separate drivers. > > My question was more whether Linux registers multiple drivers with one > device node. It's just not something I expected. > > I'm not really convinced on this. It will break the current one-to-one > relationship, and functions like device_get_child_by_of_offset(). I > think it would be better to have a MISC driver with two children. In the regular device model the Linux kernel also generally has a one-to-one model. There are special cases, like clock and timer init using special handling, or things like "syscon" which acts like more of a flag and can live next to a regular device. Therefore things like the Rockchip clock controller create the reset controller included in the CRU block. A behaviour the uboot clk driver mimics. Also doesn't allowing multiple drivers to sit on top of a node create contention on who gets access to registers? At least in the kernel multiple mappings of registers are generally not favoured. Heiko
On Sun, Mar 12, 2017 at 02:21:35PM -0600, Simon Glass wrote: > Hi, > > On 3 March 2017 at 03:52, Dr. Philipp Tomsich > <philipp.tomsich@theobroma-systems.com> wrote: > > Hi Simon, > > > > On 03 Mar 2017, at 05:52, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Philipp, > > > > On 22 February 2017 at 13:47, Philipp Tomsich > > <philipp.tomsich@theobroma-systems.com> wrote: > > > > Currently, driver binding stops once it encounters the first > > compatible driver that doesn't refuse to bind. However, there are > > cases where a single node will need to be handled by multiple driver > > classes. For those cases we provide a configurable option to continue > > to bind after the first driver has been found. > > > > The first use cases for this are from the DM conversion of the sunxi > > (Allwinner) architecture: > > * pinctrl (UCLASS_PINCTRL) and gpio (UCLASS_GPIO) drivers need to > > bind against a single node > > * clock (UCLASS_CLK) and reset (UCLASS_RESET) drivers also need to > > bind against a single node > > > > > > Does linux work this way? Another approach would be to have a separate > > MISC driver with two children, one pinctrl, one clk. > > > > > > The linux CLK driver creates and registers a reset-controller; the PINCTRL > > driver > > does the same with the gpio-controller. Similar code to do this is easily > > possible in > > U-Boot … see sunxi_pctrl_bind_gpio(…) in [PATCH v2 1/6] of this series. > > > > However, binding multiple times makes for much simpler code and allows to > > keep > > driver data in separate drivers. > > My question was more whether Linux registers multiple drivers with one > device node. It's just not something I expected. It does, but it's not really what we're doing in the linux driver. It has one driver, with one device, but registering into multiple frameworks. Maxime
Hi, On 20 March 2017 at 01:08, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Sun, Mar 12, 2017 at 02:21:35PM -0600, Simon Glass wrote: >> Hi, >> >> On 3 March 2017 at 03:52, Dr. Philipp Tomsich >> <philipp.tomsich@theobroma-systems.com> wrote: >> > Hi Simon, >> > >> > On 03 Mar 2017, at 05:52, Simon Glass <sjg@chromium.org> wrote: >> > >> > Hi Philipp, >> > >> > On 22 February 2017 at 13:47, Philipp Tomsich >> > <philipp.tomsich@theobroma-systems.com> wrote: >> > >> > Currently, driver binding stops once it encounters the first >> > compatible driver that doesn't refuse to bind. However, there are >> > cases where a single node will need to be handled by multiple driver >> > classes. For those cases we provide a configurable option to continue >> > to bind after the first driver has been found. >> > >> > The first use cases for this are from the DM conversion of the sunxi >> > (Allwinner) architecture: >> > * pinctrl (UCLASS_PINCTRL) and gpio (UCLASS_GPIO) drivers need to >> > bind against a single node >> > * clock (UCLASS_CLK) and reset (UCLASS_RESET) drivers also need to >> > bind against a single node >> > >> > >> > Does linux work this way? Another approach would be to have a separate >> > MISC driver with two children, one pinctrl, one clk. >> > >> > >> > The linux CLK driver creates and registers a reset-controller; the PINCTRL >> > driver >> > does the same with the gpio-controller. Similar code to do this is easily >> > possible in >> > U-Boot … see sunxi_pctrl_bind_gpio(…) in [PATCH v2 1/6] of this series. >> > >> > However, binding multiple times makes for much simpler code and allows to >> > keep >> > driver data in separate drivers. >> >> My question was more whether Linux registers multiple drivers with one >> device node. It's just not something I expected. > > It does, but it's not really what we're doing in the linux driver. It > has one driver, with one device, but registering into multiple > frameworks. I believe that the U-Boot equivalent of this is to have a parent device with several children each in its own uclass. Regards, Simon
diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig index 8749561..913101c 100644 --- a/drivers/core/Kconfig +++ b/drivers/core/Kconfig @@ -31,6 +31,20 @@ config DM_WARN This will cause dm_warn() to be compiled out - it will do nothing when called. +config DM_ALLOW_MULTIPLE_DRIVERS + bool "Allow multiple drivers to bind for one node" + depends on DM + default n + help + The driver model in U-Boot originally did not allow multiple + drivers to bind for a single device node. + + If enabled, multiple drivers can now bind for a single node + by using the same compatible string for matching: lists_bind_fdt() + will assume that binding multiple drivers is desirable, if the + caller does not request the pointer to the udevice structure to + be returned (i.e. if devp is NULL). + config DM_DEVICE_REMOVE bool "Support device removal" depends on DM diff --git a/drivers/core/lists.c b/drivers/core/lists.c index 23b6ba7..52efe69 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -166,7 +166,11 @@ int lists_bind_fdt(struct udevice *parent, const void *blob, int offset, dm_dbg(" - attempt to match compatible string '%s'\n", compat); - for (entry = driver; entry != driver + n_ents; entry++) { + entry = driver; +#if defined(CONFIG_DM_ALLOW_MULTIPLE_DRIVERS) + allow_more_matches: +#endif + for (; entry != driver + n_ents; entry++) { ret = driver_check_compatible(entry->of_match, &id, compat); if (!ret) @@ -190,6 +194,12 @@ int lists_bind_fdt(struct udevice *parent, const void *blob, int offset, found = true; if (devp) *devp = dev; +#if defined(CONFIG_DM_ALLOW_MULTIPLE_DRIVERS) + else { + entry++; + goto allow_more_matches; + } +#endif } break; }
Currently, driver binding stops once it encounters the first compatible driver that doesn't refuse to bind. However, there are cases where a single node will need to be handled by multiple driver classes. For those cases we provide a configurable option to continue to bind after the first driver has been found. The first use cases for this are from the DM conversion of the sunxi (Allwinner) architecture: * pinctrl (UCLASS_PINCTRL) and gpio (UCLASS_GPIO) drivers need to bind against a single node * clock (UCLASS_CLK) and reset (UCLASS_RESET) drivers also need to bind against a single node Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> --- drivers/core/Kconfig | 14 ++++++++++++++ drivers/core/lists.c | 12 +++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-)