Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1414777 > unrolled thread

[PATCH 00/22] of: Introduce of_match_device()

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/22] of: Introduce of_match_device() Thierry Reding <thierry.reding@gmail.com> - 2016-06-06 10:50 +0200
    [PATCH 01/22] of: Implement of_match_device() Thierry Reding <thierry.reding@gmail.com> - 2016-06-06 10:50 +0200
    Re: [PATCH 00/22] of: Introduce of_match_device() Frank Rowand <frowand.list@gmail.com> - 2016-06-06 21:00 +0200

#1414777 — [PATCH 00/22] of: Introduce of_match_device()

From: Thierry Reding <treding@nvidia.com>

This series of patches introduces a common implementation of a function
that can be used in iterators (such as the bus_for_each_dev() or the
driver_for_each_device() functions) and that compare's a device's device
tree node with the passed in data. Numerous drivers and subsystems have
their own variant of this function, so a lot of duplication can be
removed by making use of the common implementation.

One thing that slightly bugs me about this is that it can't be used with
users of class_for_each_device() or class_find_device() because they get
passed a callback that takes a const void * rather than a void * for the
device tree node. I had at some point investigated to turn the remainder
of the iterators into taking const void *, but there are some drivers in
the kernel that want to modify the data passed to them, so it isn't easy
to switch.

An alternative would be to turn the const void * into void * for all the
callbacks of this sort, so that we can use a single set throughout the
tree, but I'm not sure how welcome that would be. I can give that a shot
if people think it worth, though.

Greg, any thoughts on this?

I've also kept the list of recipients very short, to get early feedback
and buy-in from Rob and Frank on the common implementation. I expect the
naming to allow for some good bike-shedding.

Russell, I've Cc'ed you explicitly on this because last dozen or so
patches touch users of component helpers and you've been busy working on
those lately. I wanted to give you a heads up on this and am looking for
your feedback on where you think this would fit in.

Merging this might get a little complicated, but I think it'd be easiest
to take it all through the DT tree. Perhaps we can hold off on the final
DRM and IOMMU patches since they are based on stuff currently in linux-
next only, and defer those to v4.9?

Thierry

Thierry Reding (22):
  of: Implement of_match_device()
  amba: tegra-ahb: Use of_device_match()
  coresight: Use of_device_match()
  i2c: core: Use of_device_match()
  net: cpsw-phy-sel: Use of_device_match()
  nvmem: core: Use of_device_match()
  of/platform: Use of_device_match()
  spi: Use of_device_match()
  of: mdio: Use of_device_match()
  usb: phy: am335x-control: Use of_device_match()
  usb: phy: isp1301: Use of_device_match()
  drm/hdlcd: Use of_device_match()
  drm/armada: Use of_device_match()
  drm/etnaviv: Use of_device_match()
  drm/hisilicon: Use of_device_match()
  drm/mediatek: Use of_device_match()
  drm/msm: Use of_device_match()
  drm/rockchip: Use of_device_match()
  drm/sti: Use of_device_match()
  drm/sun4i: Use of_device_match()
  drm/tilcdc: Use of_device_match()
  iommu/mediatek: Use of_device_match()

 drivers/amba/tegra-ahb.c                        | 11 ++---------
 drivers/gpu/drm/arm/hdlcd_drv.c                 |  8 ++------
 drivers/gpu/drm/armada/armada_drv.c             |  8 ++------
 drivers/gpu/drm/drm_mipi_dsi.c                  |  5 -----
 drivers/gpu/drm/etnaviv/etnaviv_drv.c           | 12 +++---------
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |  8 ++------
 drivers/gpu/drm/mediatek/mtk_drm_drv.c          |  8 ++------
 drivers/gpu/drm/msm/msm_drv.c                   | 13 +++----------
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c     | 12 +++---------
 drivers/gpu/drm/sti/sti_drv.c                   |  8 ++------
 drivers/gpu/drm/sun4i/sun4i_drv.c               | 12 ++----------
 drivers/gpu/drm/tilcdc/tilcdc_external.c        |  8 ++------
 drivers/hwtracing/coresight/of_coresight.c      | 10 +++-------
 drivers/i2c/i2c-core.c                          |  9 ++-------
 drivers/iommu/mtk_iommu.c                       |  8 ++------
 drivers/net/ethernet/ti/cpsw-phy-sel.c          |  9 ++-------
 drivers/nvmem/core.c                            |  8 ++------
 drivers/of/of_mdio.c                            |  9 ++-------
 drivers/of/platform.c                           |  7 +------
 drivers/spi/spi.c                               |  7 +------
 drivers/usb/phy/phy-am335x-control.c            | 10 +++-------
 drivers/usb/phy/phy-isp1301.c                   | 13 ++++---------
 include/linux/of_device.h                       |  5 +++++
 23 files changed, 52 insertions(+), 156 deletions(-)

-- 
2.8.3

[toc] | [next] | [standalone]

#1414780 — [PATCH 01/22] of: Implement of_match_device()

From: Thierry Reding <treding@nvidia.com>

Many drivers open-code a variant of this function to look up the device
representing a given device tree node. Provide a common implementation
that can be reused by all drivers to avoid duplication.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_mipi_dsi.c | 5 -----
 include/linux/of_device.h      | 5 +++++
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index f5d80839a90c..37f95f3b2dd2 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -77,11 +77,6 @@ static struct bus_type mipi_dsi_bus_type = {
 	.pm = &mipi_dsi_device_pm_ops,
 };
 
-static int of_device_match(struct device *dev, void *data)
-{
-	return dev->of_node == data;
-}
-
 /**
  * of_find_mipi_dsi_device_by_node() - find the MIPI DSI device matching a
  *    device tree node
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index cc7dd687a89d..ada83eac2ae2 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -102,4 +102,9 @@ static inline void of_dma_configure(struct device *dev, struct device_node *np)
 {}
 #endif /* CONFIG_OF */
 
+static inline int of_device_match(struct device *dev, void *data)
+{
+	return dev->of_node == data;
+}
+
 #endif /* _LINUX_OF_DEVICE_H */
-- 
2.8.3

[toc] | [prev] | [next] | [standalone]

#1415374

On 06/06/16 01:31, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> This series of patches introduces a common implementation of a function
> that can be used in iterators (such as the bus_for_each_dev() or the
> driver_for_each_device() functions) and that compare's a device's device
> tree node with the passed in data. Numerous drivers and subsystems have
> their own variant of this function, so a lot of duplication can be
> removed by making use of the common implementation.
> 
> One thing that slightly bugs me about this is that it can't be used with
> users of class_for_each_device() or class_find_device() because they get
> passed a callback that takes a const void * rather than a void * for the
> device tree node. I had at some point investigated to turn the remainder
> of the iterators into taking const void *, but there are some drivers in
> the kernel that want to modify the data passed to them, so it isn't easy
> to switch.
> 
> An alternative would be to turn the const void * into void * for all the
> callbacks of this sort, so that we can use a single set throughout the
> tree, but I'm not sure how welcome that would be. I can give that a shot
> if people think it worth, though.
> 
> Greg, any thoughts on this?
> 
> I've also kept the list of recipients very short, to get early feedback
> and buy-in from Rob and Frank on the common implementation. I expect the
> naming to allow for some good bike-shedding.

Common implementation looks fine to me.

My shot at bike-shedding is to make the function name reflect the argument
names of bus_find_device() and driver_find_device().  That would suggest
of_node_match_data().  Or if one wanted to be more concise, of_node_match()
or of_match_node().

But of_match_node() is already in wide use (for something else entirely).

And drivers/regulator/core.c is the one existing version of of_node_match():

  static int of_node_match(struct device *dev, const void *data)
  {
          return dev->of_node == data;
  }

This is an example of the 'const void *data' vs 'void *data' that you
mentioned, because this of_node_match() is used for class_find_device().
of_node_match() could be used since only one current usage of the
name would have to be changed to some other name.

So that brings me back to of_node_match_data() being my preference
because of_node_match() is entirely too similar to of_match_node(),
which seems like it would lead to confusion when I am reading code.
Then maybe a variant on that name with a 'const void *data' argument.
Like the longer than I would like of_node_match_const_data().

< snip >

-Frank

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web