[PATCH 1/4] amba: Export amba_bustype

[PATCH 1/4] amba: Export amba_bustype

[Kim Phillips]

This patch is provided in the context of allowing the Coresight driver
subsystem to be loaded as modules.  Coresight uses amba_bus in its call
to bus_find_device() in of_coresight_get_endpoint_device() when
searching for a configurable endpoint device.  This patch allows
Coresight to reference amba_bustype when built as a module.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Eric Auger <eric.auger@redhat.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Todd Kjos <tkjos@google.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Kim Phillips <kim.phillips@arm.com>
---
There was a prior patch submitted by Alex W. here:

https://lkml.org/lkml/2017/6/19/811

But I can't tell its fate - presume simply delayed?

Coresight uses amba_bus in its call to bus_find_device() here:

https://lxr.missinglinkelectronics.com/linux/drivers/hwtracing/coresight/of_coresight.c#L51

Grepping for bus_type and EXPORT shows other busses exporting their
type, so I don't think this is the wrong approach.  If, OTOH, Coresight
needs to do something differently, please comment.

 drivers/amba/bus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 594c228d2f02..12283bd06733 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -197,6 +197,7 @@ struct bus_type amba_bustype = {
 	.pm		= &amba_pm,
 	.force_dma	= true,
 };
+EXPORT_SYMBOL_GPL(amba_bustype);
 
 static int __init amba_init(void)
 {
-- 
2.16.2

[PATCH 1/4] amba: Export amba_bustype

[Kim Phillips]

This patch is provided in the context of allowing the Coresight driver
subsystem to be loaded as modules.  Coresight uses amba_bus in its call
to bus_find_device() in of_coresight_get_endpoint_device() when
searching for a configurable endpoint device.  This patch allows
Coresight to reference amba_bustype when built as a module.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Eric Auger <eric.auger@redhat.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Todd Kjos <tkjos@google.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Kim Phillips <kim.phillips@arm.com>
---
There was a prior patch submitted by Alex W. here:

https://lkml.org/lkml/2017/6/19/811

But I can't tell its fate - presume simply delayed?

Coresight uses amba_bus in its call to bus_find_device() here:

https://lxr.missinglinkelectronics.com/linux/drivers/hwtracing/coresight/of_coresight.c#L51

Grepping for bus_type and EXPORT shows other busses exporting their
type, so I don't think this is the wrong approach.  If, OTOH, Coresight
needs to do something differently, please comment.

 drivers/amba/bus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 594c228d2f02..12283bd06733 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -197,6 +197,7 @@ struct bus_type amba_bustype = {
 	.pm		= &amba_pm,
 	.force_dma	= true,
 };
+EXPORT_SYMBOL_GPL(amba_bustype);
 
 static int __init amba_init(void)
 {
-- 
2.16.2

Re: [PATCH 1/4] amba: Export amba_bustype

[Robin Murphy]

Hi Kim,

On 08/05/18 20:06, Kim Phillips wrote:
> This patch is provided in the context of allowing the Coresight driver
> subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> to bus_find_device() in of_coresight_get_endpoint_device() when
> searching for a configurable endpoint device.  This patch allows
> Coresight to reference amba_bustype when built as a module.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Todd Kjos <tkjos@google.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
> ---
> There was a prior patch submitted by Alex W. here:
> 
> https://lkml.org/lkml/2017/6/19/811
> 
> But I can't tell its fate - presume simply delayed?
> 
> Coresight uses amba_bus in its call to bus_find_device() here:
> 
> https://lxr.missinglinkelectronics.com/linux/drivers/hwtracing/coresight/of_coresight.c#L51
> 
> Grepping for bus_type and EXPORT shows other busses exporting their
> type, so I don't think this is the wrong approach.  If, OTOH, Coresight
> needs to do something differently, please comment.

Exposing raw bus_types is pretty ugly, but it is indeed the status quo, 
so this probably is the reasonable thing to do. I suppose an amba_bus 
equivalent of of_find_device_by_node() could be implemented, but for 
only a single potential user that doesn't seem particularly worthwhile, 
since unless some massive shake-up of how buses work comes along the 
bus_type will inevitably end up being exported for other reasons anyway. 
So, in the context of this series;

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

However, as a wild idea for sidestepping the issue completely (or at 
least keeping it within the CoreSight framework), at first glance it 
appears something like the below might be feasible, although I may well 
be missing some obvious reason why not.

Thanks,
Robin.

----->8-----
diff --git a/drivers/hwtracing/coresight/of_coresight.c 
b/drivers/hwtracing/coresight/of_coresight.c
index 7c375443ede6..2c3fdc9b63e6 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -27,28 +27,13 @@

  static int of_dev_node_match(struct device *dev, void *data)
  {
-	return dev->of_node == data;
+	return dev->parent->of_node == data;
  }

  static struct device *
  of_coresight_get_endpoint_device(struct device_node *endpoint)
  {
-	struct device *dev = NULL;
-
-	/*
-	 * If we have a non-configurable replicator, it will be found on the
-	 * platform bus.
-	 */
-	dev = bus_find_device(&platform_bus_type, NULL,
-			      endpoint, of_dev_node_match);
-	if (dev)
-		return dev;
-
-	/*
-	 * We have a configurable component - circle through the AMBA bus
-	 * looking for the device that matches the endpoint node.
-	 */
-	return bus_find_device(&amba_bustype, NULL,
+	return bus_find_device(&coresight_bustype, NULL,
  			       endpoint, of_dev_node_match);
  }

[PATCH 1/4] amba: Export amba_bustype

[Robin Murphy]

Hi Kim,

On 08/05/18 20:06, Kim Phillips wrote:
> This patch is provided in the context of allowing the Coresight driver
> subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> to bus_find_device() in of_coresight_get_endpoint_device() when
> searching for a configurable endpoint device.  This patch allows
> Coresight to reference amba_bustype when built as a module.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Todd Kjos <tkjos@google.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
> ---
> There was a prior patch submitted by Alex W. here:
> 
> https://lkml.org/lkml/2017/6/19/811
> 
> But I can't tell its fate - presume simply delayed?
> 
> Coresight uses amba_bus in its call to bus_find_device() here:
> 
> https://lxr.missinglinkelectronics.com/linux/drivers/hwtracing/coresight/of_coresight.c#L51
> 
> Grepping for bus_type and EXPORT shows other busses exporting their
> type, so I don't think this is the wrong approach.  If, OTOH, Coresight
> needs to do something differently, please comment.

Exposing raw bus_types is pretty ugly, but it is indeed the status quo, 
so this probably is the reasonable thing to do. I suppose an amba_bus 
equivalent of of_find_device_by_node() could be implemented, but for 
only a single potential user that doesn't seem particularly worthwhile, 
since unless some massive shake-up of how buses work comes along the 
bus_type will inevitably end up being exported for other reasons anyway. 
So, in the context of this series;

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

However, as a wild idea for sidestepping the issue completely (or at 
least keeping it within the CoreSight framework), at first glance it 
appears something like the below might be feasible, although I may well 
be missing some obvious reason why not.

Thanks,
Robin.

----->8-----
diff --git a/drivers/hwtracing/coresight/of_coresight.c 
b/drivers/hwtracing/coresight/of_coresight.c
index 7c375443ede6..2c3fdc9b63e6 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -27,28 +27,13 @@

  static int of_dev_node_match(struct device *dev, void *data)
  {
-	return dev->of_node == data;
+	return dev->parent->of_node == data;
  }

  static struct device *
  of_coresight_get_endpoint_device(struct device_node *endpoint)
  {
-	struct device *dev = NULL;
-
-	/*
-	 * If we have a non-configurable replicator, it will be found on the
-	 * platform bus.
-	 */
-	dev = bus_find_device(&platform_bus_type, NULL,
-			      endpoint, of_dev_node_match);
-	if (dev)
-		return dev;
-
-	/*
-	 * We have a configurable component - circle through the AMBA bus
-	 * looking for the device that matches the endpoint node.
-	 */
-	return bus_find_device(&amba_bustype, NULL,
+	return bus_find_device(&coresight_bustype, NULL,
  			       endpoint, of_dev_node_match);
  }

Re: [PATCH 1/4] amba: Export amba_bustype

[Mathieu Poirier]

On Wed, May 09, 2018 at 02:38:32PM +0100, Robin Murphy wrote:
> Hi Kim,
> 
> On 08/05/18 20:06, Kim Phillips wrote:
> >This patch is provided in the context of allowing the Coresight driver
> >subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> >to bus_find_device() in of_coresight_get_endpoint_device() when
> >searching for a configurable endpoint device.  This patch allows
> >Coresight to reference amba_bustype when built as a module.
> >
> >Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> >Cc: Alex Williamson <alex.williamson@redhat.com>
> >Cc: Eric Auger <eric.auger@redhat.com>
> >Cc: Russell King <linux@armlinux.org.uk>
> >Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >Cc: Todd Kjos <tkjos@google.com>
> >Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> >Cc: Thierry Reding <treding@nvidia.com>
> >Cc: Robin Murphy <robin.murphy@arm.com>
> >Signed-off-by: Kim Phillips <kim.phillips@arm.com>
> >---
> >There was a prior patch submitted by Alex W. here:
> >
> >https://lkml.org/lkml/2017/6/19/811
> >
> >But I can't tell its fate - presume simply delayed?
> >
> >Coresight uses amba_bus in its call to bus_find_device() here:
> >
> >https://lxr.missinglinkelectronics.com/linux/drivers/hwtracing/coresight/of_coresight.c#L51
> >
> >Grepping for bus_type and EXPORT shows other busses exporting their
> >type, so I don't think this is the wrong approach.  If, OTOH, Coresight
> >needs to do something differently, please comment.
> 
> Exposing raw bus_types is pretty ugly, but it is indeed the status quo, so
> this probably is the reasonable thing to do. I suppose an amba_bus
> equivalent of of_find_device_by_node() could be implemented, but for only a
> single potential user that doesn't seem particularly worthwhile, since
> unless some massive shake-up of how buses work comes along the bus_type will
> inevitably end up being exported for other reasons anyway. So, in the
> context of this series;
> 
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> 
> However, as a wild idea for sidestepping the issue completely (or at least
> keeping it within the CoreSight framework), at first glance it appears
> something like the below might be feasible, although I may well be missing
> some obvious reason why not.
> 
> Thanks,
> Robin.
> 
> ----->8-----
> diff --git a/drivers/hwtracing/coresight/of_coresight.c
> b/drivers/hwtracing/coresight/of_coresight.c
> index 7c375443ede6..2c3fdc9b63e6 100644
> --- a/drivers/hwtracing/coresight/of_coresight.c
> +++ b/drivers/hwtracing/coresight/of_coresight.c
> @@ -27,28 +27,13 @@
> 
>  static int of_dev_node_match(struct device *dev, void *data)
>  {
> -	return dev->of_node == data;
> +	return dev->parent->of_node == data;
>  }
> 
>  static struct device *
>  of_coresight_get_endpoint_device(struct device_node *endpoint)
>  {
> -	struct device *dev = NULL;
> -
> -	/*
> -	 * If we have a non-configurable replicator, it will be found on the
> -	 * platform bus.
> -	 */
> -	dev = bus_find_device(&platform_bus_type, NULL,
> -			      endpoint, of_dev_node_match);
> -	if (dev)
> -		return dev;
> -
> -	/*
> -	 * We have a configurable component - circle through the AMBA bus
> -	 * looking for the device that matches the endpoint node.
> -	 */
> -	return bus_find_device(&amba_bustype, NULL,
> +	return bus_find_device(&coresight_bustype, NULL,
>  			       endpoint, of_dev_node_match);
>  }

Hi Robin and thanks for the input.  

Your approach would work if all CS devices would be on the CS bus, which is not
the case at discovery time when of_coresight_get_endpoint_device() is called.

Mathieu

> 

[PATCH 1/4] amba: Export amba_bustype

[Mathieu Poirier]

On Wed, May 09, 2018 at 02:38:32PM +0100, Robin Murphy wrote:
> Hi Kim,
> 
> On 08/05/18 20:06, Kim Phillips wrote:
> >This patch is provided in the context of allowing the Coresight driver
> >subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> >to bus_find_device() in of_coresight_get_endpoint_device() when
> >searching for a configurable endpoint device.  This patch allows
> >Coresight to reference amba_bustype when built as a module.
> >
> >Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> >Cc: Alex Williamson <alex.williamson@redhat.com>
> >Cc: Eric Auger <eric.auger@redhat.com>
> >Cc: Russell King <linux@armlinux.org.uk>
> >Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >Cc: Todd Kjos <tkjos@google.com>
> >Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> >Cc: Thierry Reding <treding@nvidia.com>
> >Cc: Robin Murphy <robin.murphy@arm.com>
> >Signed-off-by: Kim Phillips <kim.phillips@arm.com>
> >---
> >There was a prior patch submitted by Alex W. here:
> >
> >https://lkml.org/lkml/2017/6/19/811
> >
> >But I can't tell its fate - presume simply delayed?
> >
> >Coresight uses amba_bus in its call to bus_find_device() here:
> >
> >https://lxr.missinglinkelectronics.com/linux/drivers/hwtracing/coresight/of_coresight.c#L51
> >
> >Grepping for bus_type and EXPORT shows other busses exporting their
> >type, so I don't think this is the wrong approach.  If, OTOH, Coresight
> >needs to do something differently, please comment.
> 
> Exposing raw bus_types is pretty ugly, but it is indeed the status quo, so
> this probably is the reasonable thing to do. I suppose an amba_bus
> equivalent of of_find_device_by_node() could be implemented, but for only a
> single potential user that doesn't seem particularly worthwhile, since
> unless some massive shake-up of how buses work comes along the bus_type will
> inevitably end up being exported for other reasons anyway. So, in the
> context of this series;
> 
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> 
> However, as a wild idea for sidestepping the issue completely (or at least
> keeping it within the CoreSight framework), at first glance it appears
> something like the below might be feasible, although I may well be missing
> some obvious reason why not.
> 
> Thanks,
> Robin.
> 
> ----->8-----
> diff --git a/drivers/hwtracing/coresight/of_coresight.c
> b/drivers/hwtracing/coresight/of_coresight.c
> index 7c375443ede6..2c3fdc9b63e6 100644
> --- a/drivers/hwtracing/coresight/of_coresight.c
> +++ b/drivers/hwtracing/coresight/of_coresight.c
> @@ -27,28 +27,13 @@
> 
>  static int of_dev_node_match(struct device *dev, void *data)
>  {
> -	return dev->of_node == data;
> +	return dev->parent->of_node == data;
>  }
> 
>  static struct device *
>  of_coresight_get_endpoint_device(struct device_node *endpoint)
>  {
> -	struct device *dev = NULL;
> -
> -	/*
> -	 * If we have a non-configurable replicator, it will be found on the
> -	 * platform bus.
> -	 */
> -	dev = bus_find_device(&platform_bus_type, NULL,
> -			      endpoint, of_dev_node_match);
> -	if (dev)
> -		return dev;
> -
> -	/*
> -	 * We have a configurable component - circle through the AMBA bus
> -	 * looking for the device that matches the endpoint node.
> -	 */
> -	return bus_find_device(&amba_bustype, NULL,
> +	return bus_find_device(&coresight_bustype, NULL,
>  			       endpoint, of_dev_node_match);
>  }

Hi Robin and thanks for the input.  

Your approach would work if all CS devices would be on the CS bus, which is not
the case at discovery time when of_coresight_get_endpoint_device() is called.

Mathieu

> 

Re: [PATCH 1/4] amba: Export amba_bustype

[Robin Murphy]

On 09/05/18 16:40, Mathieu Poirier wrote:
> On Wed, May 09, 2018 at 02:38:32PM +0100, Robin Murphy wrote:
>> Hi Kim,
>>
>> On 08/05/18 20:06, Kim Phillips wrote:
>>> This patch is provided in the context of allowing the Coresight driver
>>> subsystem to be loaded as modules.  Coresight uses amba_bus in its call
>>> to bus_find_device() in of_coresight_get_endpoint_device() when
>>> searching for a configurable endpoint device.  This patch allows
>>> Coresight to reference amba_bustype when built as a module.
>>>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Cc: Alex Williamson <alex.williamson@redhat.com>
>>> Cc: Eric Auger <eric.auger@redhat.com>
>>> Cc: Russell King <linux@armlinux.org.uk>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: Todd Kjos <tkjos@google.com>
>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>>> Cc: Thierry Reding <treding@nvidia.com>
>>> Cc: Robin Murphy <robin.murphy@arm.com>
>>> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
>>> ---
>>> There was a prior patch submitted by Alex W. here:
>>>
>>> https://lkml.org/lkml/2017/6/19/811
>>>
>>> But I can't tell its fate - presume simply delayed?
>>>
>>> Coresight uses amba_bus in its call to bus_find_device() here:
>>>
>>> https://lxr.missinglinkelectronics.com/linux/drivers/hwtracing/coresight/of_coresight.c#L51
>>>
>>> Grepping for bus_type and EXPORT shows other busses exporting their
>>> type, so I don't think this is the wrong approach.  If, OTOH, Coresight
>>> needs to do something differently, please comment.
>>
>> Exposing raw bus_types is pretty ugly, but it is indeed the status quo, so
>> this probably is the reasonable thing to do. I suppose an amba_bus
>> equivalent of of_find_device_by_node() could be implemented, but for only a
>> single potential user that doesn't seem particularly worthwhile, since
>> unless some massive shake-up of how buses work comes along the bus_type will
>> inevitably end up being exported for other reasons anyway. So, in the
>> context of this series;
>>
>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>>
>> However, as a wild idea for sidestepping the issue completely (or at least
>> keeping it within the CoreSight framework), at first glance it appears
>> something like the below might be feasible, although I may well be missing
>> some obvious reason why not.
>>
>> Thanks,
>> Robin.
>>
>> ----->8-----
>> diff --git a/drivers/hwtracing/coresight/of_coresight.c
>> b/drivers/hwtracing/coresight/of_coresight.c
>> index 7c375443ede6..2c3fdc9b63e6 100644
>> --- a/drivers/hwtracing/coresight/of_coresight.c
>> +++ b/drivers/hwtracing/coresight/of_coresight.c
>> @@ -27,28 +27,13 @@
>>
>>   static int of_dev_node_match(struct device *dev, void *data)
>>   {
>> -	return dev->of_node == data;
>> +	return dev->parent->of_node == data;
>>   }
>>
>>   static struct device *
>>   of_coresight_get_endpoint_device(struct device_node *endpoint)
>>   {
>> -	struct device *dev = NULL;
>> -
>> -	/*
>> -	 * If we have a non-configurable replicator, it will be found on the
>> -	 * platform bus.
>> -	 */
>> -	dev = bus_find_device(&platform_bus_type, NULL,
>> -			      endpoint, of_dev_node_match);
>> -	if (dev)
>> -		return dev;
>> -
>> -	/*
>> -	 * We have a configurable component - circle through the AMBA bus
>> -	 * looking for the device that matches the endpoint node.
>> -	 */
>> -	return bus_find_device(&amba_bustype, NULL,
>> +	return bus_find_device(&coresight_bustype, NULL,
>>   			       endpoint, of_dev_node_match);
>>   }
> 
> Hi Robin and thanks for the input.
> 
> Your approach would work if all CS devices would be on the CS bus, which is not
> the case at discovery time when of_coresight_get_endpoint_device() is called.

Ah, now I see that obvious thing I was indeed missing - I got mixed up 
and started thinking bus_add_device() only got called as part of driver 
probe, but of course it's actually much earlier in 
{platform,amba}_device_add(). That means my idea cannot in fact work at 
all, since both ends of any link would defer waiting for the other to 
probe (and thus call coresight_register()) first. Oh well, I guess 
poking amba_bustype really does remain the only reasonable answer.

Thanks,
Robin.

[PATCH 1/4] amba: Export amba_bustype

[Robin Murphy]

On 09/05/18 16:40, Mathieu Poirier wrote:
> On Wed, May 09, 2018 at 02:38:32PM +0100, Robin Murphy wrote:
>> Hi Kim,
>>
>> On 08/05/18 20:06, Kim Phillips wrote:
>>> This patch is provided in the context of allowing the Coresight driver
>>> subsystem to be loaded as modules.  Coresight uses amba_bus in its call
>>> to bus_find_device() in of_coresight_get_endpoint_device() when
>>> searching for a configurable endpoint device.  This patch allows
>>> Coresight to reference amba_bustype when built as a module.
>>>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Cc: Alex Williamson <alex.williamson@redhat.com>
>>> Cc: Eric Auger <eric.auger@redhat.com>
>>> Cc: Russell King <linux@armlinux.org.uk>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: Todd Kjos <tkjos@google.com>
>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>>> Cc: Thierry Reding <treding@nvidia.com>
>>> Cc: Robin Murphy <robin.murphy@arm.com>
>>> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
>>> ---
>>> There was a prior patch submitted by Alex W. here:
>>>
>>> https://lkml.org/lkml/2017/6/19/811
>>>
>>> But I can't tell its fate - presume simply delayed?
>>>
>>> Coresight uses amba_bus in its call to bus_find_device() here:
>>>
>>> https://lxr.missinglinkelectronics.com/linux/drivers/hwtracing/coresight/of_coresight.c#L51
>>>
>>> Grepping for bus_type and EXPORT shows other busses exporting their
>>> type, so I don't think this is the wrong approach.  If, OTOH, Coresight
>>> needs to do something differently, please comment.
>>
>> Exposing raw bus_types is pretty ugly, but it is indeed the status quo, so
>> this probably is the reasonable thing to do. I suppose an amba_bus
>> equivalent of of_find_device_by_node() could be implemented, but for only a
>> single potential user that doesn't seem particularly worthwhile, since
>> unless some massive shake-up of how buses work comes along the bus_type will
>> inevitably end up being exported for other reasons anyway. So, in the
>> context of this series;
>>
>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>>
>> However, as a wild idea for sidestepping the issue completely (or at least
>> keeping it within the CoreSight framework), at first glance it appears
>> something like the below might be feasible, although I may well be missing
>> some obvious reason why not.
>>
>> Thanks,
>> Robin.
>>
>> ----->8-----
>> diff --git a/drivers/hwtracing/coresight/of_coresight.c
>> b/drivers/hwtracing/coresight/of_coresight.c
>> index 7c375443ede6..2c3fdc9b63e6 100644
>> --- a/drivers/hwtracing/coresight/of_coresight.c
>> +++ b/drivers/hwtracing/coresight/of_coresight.c
>> @@ -27,28 +27,13 @@
>>
>>   static int of_dev_node_match(struct device *dev, void *data)
>>   {
>> -	return dev->of_node == data;
>> +	return dev->parent->of_node == data;
>>   }
>>
>>   static struct device *
>>   of_coresight_get_endpoint_device(struct device_node *endpoint)
>>   {
>> -	struct device *dev = NULL;
>> -
>> -	/*
>> -	 * If we have a non-configurable replicator, it will be found on the
>> -	 * platform bus.
>> -	 */
>> -	dev = bus_find_device(&platform_bus_type, NULL,
>> -			      endpoint, of_dev_node_match);
>> -	if (dev)
>> -		return dev;
>> -
>> -	/*
>> -	 * We have a configurable component - circle through the AMBA bus
>> -	 * looking for the device that matches the endpoint node.
>> -	 */
>> -	return bus_find_device(&amba_bustype, NULL,
>> +	return bus_find_device(&coresight_bustype, NULL,
>>   			       endpoint, of_dev_node_match);
>>   }
> 
> Hi Robin and thanks for the input.
> 
> Your approach would work if all CS devices would be on the CS bus, which is not
> the case at discovery time when of_coresight_get_endpoint_device() is called.

Ah, now I see that obvious thing I was indeed missing - I got mixed up 
and started thinking bus_add_device() only got called as part of driver 
probe, but of course it's actually much earlier in 
{platform,amba}_device_add(). That means my idea cannot in fact work at 
all, since both ends of any link would defer waiting for the other to 
probe (and thus call coresight_register()) first. Oh well, I guess 
poking amba_bustype really does remain the only reasonable answer.

Thanks,
Robin.

Re: [PATCH 1/4] amba: Export amba_bustype

[Kim Phillips]

On Wed, 9 May 2018 17:02:34 +0100
Robin Murphy <robin.murphy@arm.com> wrote:

> probe (and thus call coresight_register()) first. Oh well, I guess 
> poking amba_bustype really does remain the only reasonable answer.

Thanks all, I've added Robin's Reviewed-by, and submitted this patch to
RMK's patch tracking system, here:

http://www.armlinux.org.uk/developer/patches/viewpatch.php?id=8773/1

Cheers,

Kim

[PATCH 1/4] amba: Export amba_bustype

[Kim Phillips]

On Wed, 9 May 2018 17:02:34 +0100
Robin Murphy <robin.murphy@arm.com> wrote:

> probe (and thus call coresight_register()) first. Oh well, I guess 
> poking amba_bustype really does remain the only reasonable answer.

Thanks all, I've added Robin's Reviewed-by, and submitted this patch to
RMK's patch tracking system, here:

http://www.armlinux.org.uk/developer/patches/viewpatch.php?id=8773/1

Cheers,

Kim

Re: [PATCH 1/4] amba: Export amba_bustype

[Ulf Hansson]

On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
> This patch is provided in the context of allowing the Coresight driver
> subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> to bus_find_device() in of_coresight_get_endpoint_device() when
> searching for a configurable endpoint device.  This patch allows
> Coresight to reference amba_bustype when built as a module.

Sounds like you are fixing a bug, don't your want this to go for
stable and then also add a fixes tag?

>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Todd Kjos <tkjos@google.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Kim Phillips <kim.phillips@arm.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

[...]

Kind regards
Uffe

[PATCH 1/4] amba: Export amba_bustype

[Ulf Hansson]

On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
> This patch is provided in the context of allowing the Coresight driver
> subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> to bus_find_device() in of_coresight_get_endpoint_device() when
> searching for a configurable endpoint device.  This patch allows
> Coresight to reference amba_bustype when built as a module.

Sounds like you are fixing a bug, don't your want this to go for
stable and then also add a fixes tag?

>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Todd Kjos <tkjos@google.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Kim Phillips <kim.phillips@arm.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

[...]

Kind regards
Uffe

Re: [PATCH 1/4] amba: Export amba_bustype

[Kim Phillips]

On Tue, 15 May 2018 08:59:02 +0200
Ulf Hansson <ulf.hansson@linaro.org> wrote:

> On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
> > This patch is provided in the context of allowing the Coresight driver
> > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> > to bus_find_device() in of_coresight_get_endpoint_device() when
> > searching for a configurable endpoint device.  This patch allows
> > Coresight to reference amba_bustype when built as a module.
> 
> Sounds like you are fixing a bug, don't your want this to go for
> stable and then also add a fixes tag?

How do you consider this a bug fix?  What commit would the fixes tag
reference?  The introduction of the amba bus?  Not only aren't busses
required to export their bus_type, but that commit predates git.

Kim

[PATCH 1/4] amba: Export amba_bustype

[Kim Phillips]

On Tue, 15 May 2018 08:59:02 +0200
Ulf Hansson <ulf.hansson@linaro.org> wrote:

> On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
> > This patch is provided in the context of allowing the Coresight driver
> > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> > to bus_find_device() in of_coresight_get_endpoint_device() when
> > searching for a configurable endpoint device.  This patch allows
> > Coresight to reference amba_bustype when built as a module.
> 
> Sounds like you are fixing a bug, don't your want this to go for
> stable and then also add a fixes tag?

How do you consider this a bug fix?  What commit would the fixes tag
reference?  The introduction of the amba bus?  Not only aren't busses
required to export their bus_type, but that commit predates git.

Kim

Re: [PATCH 1/4] amba: Export amba_bustype

[Russell King - ARM Linux]

On Tue, May 15, 2018 at 08:59:02AM +0200, Ulf Hansson wrote:
> On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
> > This patch is provided in the context of allowing the Coresight driver
> > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> > to bus_find_device() in of_coresight_get_endpoint_device() when
> > searching for a configurable endpoint device.  This patch allows
> > Coresight to reference amba_bustype when built as a module.
> 
> Sounds like you are fixing a bug, don't your want this to go for
> stable and then also add a fixes tag?

What bug is this fixing exactly that would qualify it for stable
backporting?

The lack of an export is never a bug unless there is some existing
user which requires it.  This is not the case here.

What Kim is doing in his new patch series is making Coresight - which
is currently only available as either disabled or built-in - possible
to be loaded as a module.  This is a new feature, and in the process
of creating this new feature, Kim needs a symbol that wasn't previously
needed to be exported.

I think it would be hard to argue that Coresight not being available
as a module is a bug worthy of backporting to older kernels.

Therefore, it is not a bug, and it certainly does not qualify for
backporting to stable trees:

 - It must be obviously correct and tested.

Probably.

 - It cannot be bigger than 100 lines, with context.

Is.

 - It must fix only one thing.

Does.

 - It must fix a real bug that bothers people (not a, "This could be a
   problem..." type thing).

Nope.

 - It must fix a problem that causes a build error (but not for things
   marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
   security issue, or some "oh, that's not good" issue.  In short, something
   critical.

Nope, not in any stable tree.

 - Serious issues as reported by a user of a distribution kernel may also
   be considered if they fix a notable performance or interactivity issue.
   As these fixes are not as obvious and have a higher risk of a subtle
   regression they should only be submitted by a distribution kernel
   maintainer and include an addendum linking to a bugzilla entry if it
   exists and additional information on the user-visible impact.

Hasn't been.

 - New device IDs and quirks are also accepted.

Is not that.

 - No "theoretical race condition" issues, unless an explanation of how the
   race can be exploited is also provided.

Is not that.

 - It cannot contain any "trivial" fixes in it (spelling changes,
   whitespace cleanups, etc).

Doesn't (so okay.)

 - It must follow the
   :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
   rules.

Does.

 - It or an equivalent fix must already exist in Linus' tree (upstream).

Eventually.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 1/4] amba: Export amba_bustype

[Russell King - ARM Linux]

On Tue, May 15, 2018 at 08:59:02AM +0200, Ulf Hansson wrote:
> On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
> > This patch is provided in the context of allowing the Coresight driver
> > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> > to bus_find_device() in of_coresight_get_endpoint_device() when
> > searching for a configurable endpoint device.  This patch allows
> > Coresight to reference amba_bustype when built as a module.
> 
> Sounds like you are fixing a bug, don't your want this to go for
> stable and then also add a fixes tag?

What bug is this fixing exactly that would qualify it for stable
backporting?

The lack of an export is never a bug unless there is some existing
user which requires it.  This is not the case here.

What Kim is doing in his new patch series is making Coresight - which
is currently only available as either disabled or built-in - possible
to be loaded as a module.  This is a new feature, and in the process
of creating this new feature, Kim needs a symbol that wasn't previously
needed to be exported.

I think it would be hard to argue that Coresight not being available
as a module is a bug worthy of backporting to older kernels.

Therefore, it is not a bug, and it certainly does not qualify for
backporting to stable trees:

 - It must be obviously correct and tested.

Probably.

 - It cannot be bigger than 100 lines, with context.

Is.

 - It must fix only one thing.

Does.

 - It must fix a real bug that bothers people (not a, "This could be a
   problem..." type thing).

Nope.

 - It must fix a problem that causes a build error (but not for things
   marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
   security issue, or some "oh, that's not good" issue.  In short, something
   critical.

Nope, not in any stable tree.

 - Serious issues as reported by a user of a distribution kernel may also
   be considered if they fix a notable performance or interactivity issue.
   As these fixes are not as obvious and have a higher risk of a subtle
   regression they should only be submitted by a distribution kernel
   maintainer and include an addendum linking to a bugzilla entry if it
   exists and additional information on the user-visible impact.

Hasn't been.

 - New device IDs and quirks are also accepted.

Is not that.

 - No "theoretical race condition" issues, unless an explanation of how the
   race can be exploited is also provided.

Is not that.

 - It cannot contain any "trivial" fixes in it (spelling changes,
   whitespace cleanups, etc).

Doesn't (so okay.)

 - It must follow the
   :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
   rules.

Does.

 - It or an equivalent fix must already exist in Linus' tree (upstream).

Eventually.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH 1/4] amba: Export amba_bustype

[Russell King - ARM Linux]

On Tue, May 15, 2018 at 08:15:19AM -0500, Kim Phillips wrote:
> On Tue, 15 May 2018 08:59:02 +0200
> Ulf Hansson <ulf.hansson@linaro.org> wrote:
> 
> > On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
> > > This patch is provided in the context of allowing the Coresight driver
> > > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> > > to bus_find_device() in of_coresight_get_endpoint_device() when
> > > searching for a configurable endpoint device.  This patch allows
> > > Coresight to reference amba_bustype when built as a module.
> > 
> > Sounds like you are fixing a bug, don't your want this to go for
> > stable and then also add a fixes tag?
> 
> How do you consider this a bug fix?  What commit would the fixes tag
> reference?  The introduction of the amba bus?  Not only aren't busses
> required to export their bus_type, but that commit predates git.

I do not consider it a bug fix (see my reply to Ulf) and I certainly
do not think it should qualify for backporting to *stable* kernels.

While the impact on stable kernels of just this patch should be low,
that's not really the point: one of the requirements for stable kernels
is that patches should be _real_ bug fixes - stuff that affects people
using the kernel.

That is not the case in this instance - there is no problem with any
of the existing kernels with not having this symbol exported.

The only problem which we're aware of is with Coresight, and only then
when your patches to allow Coresight to be modular are merged.  That's
a new feature, and this new feature now requires a symbol that was not
previously required to be exported to now be exported.

So, the need for this export comes from your new feature, not from a
bug report that is affecting people.  As long as your new feature is
not backported (does it even qualify for backporting to stable kernels?)
then there is no problem with any existing stable kernel, and so there
is no requirement for it to be backported.

Hence, there's no need to Cc stable, and no need for a fixes tag.  It's
not a fix, it's a feature enhancement to permit modular code to use
bus_find_device() with this bus type which wasn't previously required.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 1/4] amba: Export amba_bustype

[Russell King - ARM Linux]

On Tue, May 15, 2018 at 08:15:19AM -0500, Kim Phillips wrote:
> On Tue, 15 May 2018 08:59:02 +0200
> Ulf Hansson <ulf.hansson@linaro.org> wrote:
> 
> > On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
> > > This patch is provided in the context of allowing the Coresight driver
> > > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> > > to bus_find_device() in of_coresight_get_endpoint_device() when
> > > searching for a configurable endpoint device.  This patch allows
> > > Coresight to reference amba_bustype when built as a module.
> > 
> > Sounds like you are fixing a bug, don't your want this to go for
> > stable and then also add a fixes tag?
> 
> How do you consider this a bug fix?  What commit would the fixes tag
> reference?  The introduction of the amba bus?  Not only aren't busses
> required to export their bus_type, but that commit predates git.

I do not consider it a bug fix (see my reply to Ulf) and I certainly
do not think it should qualify for backporting to *stable* kernels.

While the impact on stable kernels of just this patch should be low,
that's not really the point: one of the requirements for stable kernels
is that patches should be _real_ bug fixes - stuff that affects people
using the kernel.

That is not the case in this instance - there is no problem with any
of the existing kernels with not having this symbol exported.

The only problem which we're aware of is with Coresight, and only then
when your patches to allow Coresight to be modular are merged.  That's
a new feature, and this new feature now requires a symbol that was not
previously required to be exported to now be exported.

So, the need for this export comes from your new feature, not from a
bug report that is affecting people.  As long as your new feature is
not backported (does it even qualify for backporting to stable kernels?)
then there is no problem with any existing stable kernel, and so there
is no requirement for it to be backported.

Hence, there's no need to Cc stable, and no need for a fixes tag.  It's
not a fix, it's a feature enhancement to permit modular code to use
bus_find_device() with this bus type which wasn't previously required.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH 1/4] amba: Export amba_bustype

[Kim Phillips]

On Tue, 15 May 2018 14:48:31 +0100
Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

> On Tue, May 15, 2018 at 08:15:19AM -0500, Kim Phillips wrote:
> > On Tue, 15 May 2018 08:59:02 +0200
> > Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > 
> > > On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
> > > > This patch is provided in the context of allowing the Coresight driver
> > > > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> > > > to bus_find_device() in of_coresight_get_endpoint_device() when
> > > > searching for a configurable endpoint device.  This patch allows
> > > > Coresight to reference amba_bustype when built as a module.
> > > 
> > > Sounds like you are fixing a bug, don't your want this to go for
> > > stable and then also add a fixes tag?
> > 
> > How do you consider this a bug fix?  What commit would the fixes tag
> > reference?  The introduction of the amba bus?  Not only aren't busses
> > required to export their bus_type, but that commit predates git.
> 
> I do not consider it a bug fix (see my reply to Ulf) and I certainly

I agree this isn't a bug fix.

> The only problem which we're aware of is with Coresight, and only then
> when your patches to allow Coresight to be modular are merged.  That's

Just to clarify: the Coresight modularization patch depends on this
patch, so this patch is to be merged first.

Cheers,

Kim

[PATCH 1/4] amba: Export amba_bustype

[Kim Phillips]

On Tue, 15 May 2018 14:48:31 +0100
Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

> On Tue, May 15, 2018 at 08:15:19AM -0500, Kim Phillips wrote:
> > On Tue, 15 May 2018 08:59:02 +0200
> > Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > 
> > > On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
> > > > This patch is provided in the context of allowing the Coresight driver
> > > > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> > > > to bus_find_device() in of_coresight_get_endpoint_device() when
> > > > searching for a configurable endpoint device.  This patch allows
> > > > Coresight to reference amba_bustype when built as a module.
> > > 
> > > Sounds like you are fixing a bug, don't your want this to go for
> > > stable and then also add a fixes tag?
> > 
> > How do you consider this a bug fix?  What commit would the fixes tag
> > reference?  The introduction of the amba bus?  Not only aren't busses
> > required to export their bus_type, but that commit predates git.
> 
> I do not consider it a bug fix (see my reply to Ulf) and I certainly

I agree this isn't a bug fix.

> The only problem which we're aware of is with Coresight, and only then
> when your patches to allow Coresight to be modular are merged.  That's

Just to clarify: the Coresight modularization patch depends on this
patch, so this patch is to be merged first.

Cheers,

Kim

Re: [PATCH 1/4] amba: Export amba_bustype

[Mathieu Poirier]

On 15 May 2018 at 08:37, Kim Phillips <kim.phillips@arm.com> wrote:
> On Tue, 15 May 2018 14:48:31 +0100
> Russell King - ARM Linux <linux@armlinux.org.uk> wrote:
>
>> On Tue, May 15, 2018 at 08:15:19AM -0500, Kim Phillips wrote:
>> > On Tue, 15 May 2018 08:59:02 +0200
>> > Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> >
>> > > On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
>> > > > This patch is provided in the context of allowing the Coresight driver
>> > > > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
>> > > > to bus_find_device() in of_coresight_get_endpoint_device() when
>> > > > searching for a configurable endpoint device.  This patch allows
>> > > > Coresight to reference amba_bustype when built as a module.
>> > >
>> > > Sounds like you are fixing a bug, don't your want this to go for
>> > > stable and then also add a fixes tag?
>> >
>> > How do you consider this a bug fix?  What commit would the fixes tag
>> > reference?  The introduction of the amba bus?  Not only aren't busses
>> > required to export their bus_type, but that commit predates git.
>>
>> I do not consider it a bug fix (see my reply to Ulf) and I certainly
>
> I agree this isn't a bug fix.
>
>> The only problem which we're aware of is with Coresight, and only then
>> when your patches to allow Coresight to be modular are merged.  That's
>
> Just to clarify: the Coresight modularization patch depends on this
> patch, so this patch is to be merged first.

I think the whole feature should be merge at the same time and (with
Russell's ACK) probably easier through my tree.

>
> Cheers,
>
> Kim

[PATCH 1/4] amba: Export amba_bustype

[Mathieu Poirier]

On 15 May 2018 at 08:37, Kim Phillips <kim.phillips@arm.com> wrote:
> On Tue, 15 May 2018 14:48:31 +0100
> Russell King - ARM Linux <linux@armlinux.org.uk> wrote:
>
>> On Tue, May 15, 2018 at 08:15:19AM -0500, Kim Phillips wrote:
>> > On Tue, 15 May 2018 08:59:02 +0200
>> > Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> >
>> > > On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
>> > > > This patch is provided in the context of allowing the Coresight driver
>> > > > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
>> > > > to bus_find_device() in of_coresight_get_endpoint_device() when
>> > > > searching for a configurable endpoint device.  This patch allows
>> > > > Coresight to reference amba_bustype when built as a module.
>> > >
>> > > Sounds like you are fixing a bug, don't your want this to go for
>> > > stable and then also add a fixes tag?
>> >
>> > How do you consider this a bug fix?  What commit would the fixes tag
>> > reference?  The introduction of the amba bus?  Not only aren't busses
>> > required to export their bus_type, but that commit predates git.
>>
>> I do not consider it a bug fix (see my reply to Ulf) and I certainly
>
> I agree this isn't a bug fix.
>
>> The only problem which we're aware of is with Coresight, and only then
>> when your patches to allow Coresight to be modular are merged.  That's
>
> Just to clarify: the Coresight modularization patch depends on this
> patch, so this patch is to be merged first.

I think the whole feature should be merge at the same time and (with
Russell's ACK) probably easier through my tree.

>
> Cheers,
>
> Kim

Re: [PATCH 1/4] amba: Export amba_bustype

[Ulf Hansson]

On 15 May 2018 at 15:41, Russell King - ARM Linux <linux@armlinux.org.uk> wrote:
> On Tue, May 15, 2018 at 08:59:02AM +0200, Ulf Hansson wrote:
>> On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
>> > This patch is provided in the context of allowing the Coresight driver
>> > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
>> > to bus_find_device() in of_coresight_get_endpoint_device() when
>> > searching for a configurable endpoint device.  This patch allows
>> > Coresight to reference amba_bustype when built as a module.
>>
>> Sounds like you are fixing a bug, don't your want this to go for
>> stable and then also add a fixes tag?
>
> What bug is this fixing exactly that would qualify it for stable
> backporting?

That was my question, as when reading the changelog of $subject patch,
this isn't clear to me.

>
> The lack of an export is never a bug unless there is some existing
> user which requires it.  This is not the case here.
>
> What Kim is doing in his new patch series is making Coresight - which
> is currently only available as either disabled or built-in - possible
> to be loaded as a module.  This is a new feature, and in the process
> of creating this new feature, Kim needs a symbol that wasn't previously
> needed to be exported.

Thanks for clarifying, I did not review the entire series. The change
make perfect sense, no objections - and of course I agree it's not a
bug fix.

[...]

Kind regards
Uffe

[PATCH 1/4] amba: Export amba_bustype

[Ulf Hansson]

On 15 May 2018 at 15:41, Russell King - ARM Linux <linux@armlinux.org.uk> wrote:
> On Tue, May 15, 2018 at 08:59:02AM +0200, Ulf Hansson wrote:
>> On 8 May 2018 at 21:06, Kim Phillips <kim.phillips@arm.com> wrote:
>> > This patch is provided in the context of allowing the Coresight driver
>> > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
>> > to bus_find_device() in of_coresight_get_endpoint_device() when
>> > searching for a configurable endpoint device.  This patch allows
>> > Coresight to reference amba_bustype when built as a module.
>>
>> Sounds like you are fixing a bug, don't your want this to go for
>> stable and then also add a fixes tag?
>
> What bug is this fixing exactly that would qualify it for stable
> backporting?

That was my question, as when reading the changelog of $subject patch,
this isn't clear to me.

>
> The lack of an export is never a bug unless there is some existing
> user which requires it.  This is not the case here.
>
> What Kim is doing in his new patch series is making Coresight - which
> is currently only available as either disabled or built-in - possible
> to be loaded as a module.  This is a new feature, and in the process
> of creating this new feature, Kim needs a symbol that wasn't previously
> needed to be exported.

Thanks for clarifying, I did not review the entire series. The change
make perfect sense, no objections - and of course I agree it's not a
bug fix.

[...]

Kind regards
Uffe

Re: [PATCH 1/4] amba: Export amba_bustype

[Andy Shevchenko]

On Tue, May 8, 2018 at 10:06 PM, Kim Phillips <kim.phillips@arm.com> wrote:
> This patch is provided in the context of allowing the Coresight driver
> subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> to bus_find_device() in of_coresight_get_endpoint_device() when
> searching for a configurable endpoint device.  This patch allows
> Coresight to reference amba_bustype when built as a module.

> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -197,6 +197,7 @@ struct bus_type amba_bustype = {
>         .pm             = &amba_pm,
>         .force_dma      = true,
>  };
> +EXPORT_SYMBOL_GPL(amba_bustype);

Oh,

What wrong with the approach let's say similar to PCI bus?

Whenever you have a struct device you may use two helpers:

dev_is_pci() -> is the device of PCI bus type?
to_pci_dev() -> get's container of struct device for PCI bus case

-- 
With Best Regards,
Andy Shevchenko

[PATCH 1/4] amba: Export amba_bustype

[Andy Shevchenko]

On Tue, May 8, 2018 at 10:06 PM, Kim Phillips <kim.phillips@arm.com> wrote:
> This patch is provided in the context of allowing the Coresight driver
> subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> to bus_find_device() in of_coresight_get_endpoint_device() when
> searching for a configurable endpoint device.  This patch allows
> Coresight to reference amba_bustype when built as a module.

> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -197,6 +197,7 @@ struct bus_type amba_bustype = {
>         .pm             = &amba_pm,
>         .force_dma      = true,
>  };
> +EXPORT_SYMBOL_GPL(amba_bustype);

Oh,

What wrong with the approach let's say similar to PCI bus?

Whenever you have a struct device you may use two helpers:

dev_is_pci() -> is the device of PCI bus type?
to_pci_dev() -> get's container of struct device for PCI bus case

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/4] amba: Export amba_bustype

[Russell King - ARM Linux]

On Wed, May 16, 2018 at 12:16:28PM +0300, Andy Shevchenko wrote:
> On Tue, May 8, 2018 at 10:06 PM, Kim Phillips <kim.phillips@arm.com> wrote:
> > This patch is provided in the context of allowing the Coresight driver
> > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> > to bus_find_device() in of_coresight_get_endpoint_device() when
> > searching for a configurable endpoint device.  This patch allows
> > Coresight to reference amba_bustype when built as a module.
> 
> > --- a/drivers/amba/bus.c
> > +++ b/drivers/amba/bus.c
> > @@ -197,6 +197,7 @@ struct bus_type amba_bustype = {
> >         .pm             = &amba_pm,
> >         .force_dma      = true,
> >  };
> > +EXPORT_SYMBOL_GPL(amba_bustype);
> 
> Oh,
> 
> What wrong with the approach let's say similar to PCI bus?
> 
> Whenever you have a struct device you may use two helpers:
> 
> dev_is_pci() -> is the device of PCI bus type?
> to_pci_dev() -> get's container of struct device for PCI bus case

How does that help with bus_find_device() which requires the bus_type
structure for the type of devices to be searched?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 1/4] amba: Export amba_bustype

[Russell King - ARM Linux]

On Wed, May 16, 2018 at 12:16:28PM +0300, Andy Shevchenko wrote:
> On Tue, May 8, 2018 at 10:06 PM, Kim Phillips <kim.phillips@arm.com> wrote:
> > This patch is provided in the context of allowing the Coresight driver
> > subsystem to be loaded as modules.  Coresight uses amba_bus in its call
> > to bus_find_device() in of_coresight_get_endpoint_device() when
> > searching for a configurable endpoint device.  This patch allows
> > Coresight to reference amba_bustype when built as a module.
> 
> > --- a/drivers/amba/bus.c
> > +++ b/drivers/amba/bus.c
> > @@ -197,6 +197,7 @@ struct bus_type amba_bustype = {
> >         .pm             = &amba_pm,
> >         .force_dma      = true,
> >  };
> > +EXPORT_SYMBOL_GPL(amba_bustype);
> 
> Oh,
> 
> What wrong with the approach let's say similar to PCI bus?
> 
> Whenever you have a struct device you may use two helpers:
> 
> dev_is_pci() -> is the device of PCI bus type?
> to_pci_dev() -> get's container of struct device for PCI bus case

How does that help with bus_find_device() which requires the bus_type
structure for the type of devices to be searched?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH 1/4] amba: Export amba_bustype

[Robin Murphy]

On 16/05/18 10:18, Russell King - ARM Linux wrote:
> On Wed, May 16, 2018 at 12:16:28PM +0300, Andy Shevchenko wrote:
>> On Tue, May 8, 2018 at 10:06 PM, Kim Phillips <kim.phillips@arm.com> wrote:
>>> This patch is provided in the context of allowing the Coresight driver
>>> subsystem to be loaded as modules.  Coresight uses amba_bus in its call
>>> to bus_find_device() in of_coresight_get_endpoint_device() when
>>> searching for a configurable endpoint device.  This patch allows
>>> Coresight to reference amba_bustype when built as a module.
>>
>>> --- a/drivers/amba/bus.c
>>> +++ b/drivers/amba/bus.c
>>> @@ -197,6 +197,7 @@ struct bus_type amba_bustype = {
>>>          .pm             = &amba_pm,
>>>          .force_dma      = true,
>>>   };
>>> +EXPORT_SYMBOL_GPL(amba_bustype);
>>
>> Oh,
>>
>> What wrong with the approach let's say similar to PCI bus?
>>
>> Whenever you have a struct device you may use two helpers:
>>
>> dev_is_pci() -> is the device of PCI bus type?
>> to_pci_dev() -> get's container of struct device for PCI bus case
> 
> How does that help with bus_find_device() which requires the bus_type
> structure for the type of devices to be searched?
Not to mention that dev_is_pci() still relies on pci_bus_type itself 
being exported anyway.

Robin.

[PATCH 1/4] amba: Export amba_bustype

[Robin Murphy]

On 16/05/18 10:18, Russell King - ARM Linux wrote:
> On Wed, May 16, 2018 at 12:16:28PM +0300, Andy Shevchenko wrote:
>> On Tue, May 8, 2018 at 10:06 PM, Kim Phillips <kim.phillips@arm.com> wrote:
>>> This patch is provided in the context of allowing the Coresight driver
>>> subsystem to be loaded as modules.  Coresight uses amba_bus in its call
>>> to bus_find_device() in of_coresight_get_endpoint_device() when
>>> searching for a configurable endpoint device.  This patch allows
>>> Coresight to reference amba_bustype when built as a module.
>>
>>> --- a/drivers/amba/bus.c
>>> +++ b/drivers/amba/bus.c
>>> @@ -197,6 +197,7 @@ struct bus_type amba_bustype = {
>>>          .pm             = &amba_pm,
>>>          .force_dma      = true,
>>>   };
>>> +EXPORT_SYMBOL_GPL(amba_bustype);
>>
>> Oh,
>>
>> What wrong with the approach let's say similar to PCI bus?
>>
>> Whenever you have a struct device you may use two helpers:
>>
>> dev_is_pci() -> is the device of PCI bus type?
>> to_pci_dev() -> get's container of struct device for PCI bus case
> 
> How does that help with bus_find_device() which requires the bus_type
> structure for the type of devices to be searched?
Not to mention that dev_is_pci() still relies on pci_bus_type itself 
being exported anyway.

Robin.